Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI/okta duo push notification #11442

Merged
merged 15 commits into from
May 6, 2021
Merged

Conversation

Monkeychip
Copy link
Contributor

@Monkeychip Monkeychip commented Apr 22, 2021

This PR adds a push notification for Okta authentication. There have been issues with people waiting for the loader, not knowing that it was actually the loader waiting on them to hit their okta push notifications.

Functionality: If you select the auth method "okta" a function called delayAuthMessageReminder is fired concurrently while the authenticate concurrency task is also running. If after five seconds the authenticate concurrency task is still running a message appears to remind folks to check their push notifications. If the auth method takes less than five seconds (ex: you have okta setup with no MFA and its successful) you won't see the message reminder.

Here is a gif of a situation in which we wait 90 seconds for someone to hit their push notifications.

Here is a gif of a situation where it takes less than 5 seconds to login to okta and you do not see the message.

Here is a screenshot of the message:

Note:
While I was in there, I removed the overlay loader per Design's request— we show this with a grey out readOnly button that has a spinning loading inside of it. I also cleaned up the look of the AlertInline component that display messages for the JWT method and for under "More options". The messages now match the font-size and padding that Design had specked the Okta notification message for.

@Monkeychip Monkeychip added this to the 1.8 milestone Apr 22, 2021
@vercel vercel bot temporarily deployed to Preview – vault April 22, 2021 23:27 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 22, 2021 23:27 Inactive
@vercel vercel bot temporarily deployed to Preview – vault April 22, 2021 23:28 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 22, 2021 23:28 Inactive
@kalafut
Copy link
Contributor

kalafut commented Apr 23, 2021

Hi @Monkeychip. I'd like to chat about this since the design has moved a bit from the original idea of displaying a message after a certain amount of time.

Copy link
Contributor

@hashishaw hashishaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple questions here, likely not blocking but I'd like to hear your thoughts before I approve!

ui/app/components/auth-form.js Show resolved Hide resolved
ui/app/styles/core/message.scss Outdated Show resolved Hide resolved
@@ -105,4 +105,17 @@ module('Acceptance | auth', function(hooks) {
await visit('/vault/access');
assert.dom('[data-test-allow-expiration="true"]').doesNotExist('hides beacon when the api is used again');
});

test('it shows the push notification warning only for okta auth method after submit', async function(assert) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

Copy link
Contributor

@hashishaw hashishaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Thanks 🚀

@Monkeychip Monkeychip merged commit 43ad454 into master May 6, 2021
@Monkeychip Monkeychip deleted the ui/okta-duo-push-notification branch May 6, 2021 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants