Closed typerandom closed 7 years ago
Awesome work @typerandom! 🔥
Kicking the tires a little bit:
One other thought: The Device Added success screen doesn't have a clear call to action. To newbies it may be non-obvious that you have to click away to close.
The Forgot Password flow also has this issue, which I bugged in #144
Another other thought: I think this message could be simplified.
End-users don't care about Factors or Accounts. "This phone number has already been added to your account" is cleaner IMO.
Thanks @nbarbettini for the detailed review! We still need to add Client API endpoints to handle the "you already have an unverified factor of that type/phone number" story.
Just want to give an update here! Thanks @nbarbettini for all the feedback! That was really awesome! I've been working today on fixing these things and I have completed most of them.
My update here so far.
This bug has been fixed.
Yes, this is currently a limitation in the API. We’d need to have support for GET /factors
in or to be able to detect any existing unverified factors to be able to rechallenge them.
This has been fixed.
This has been fixed.
The window now autocloses after 5 seconds with the text “Device added - You’ll soon be redirected to your application.”
This has been fixed.
This is still being worked on. But expect to have this a little bit later today.
Great progress @typerandom! đź’Ż
Thanks @typerandom ! Here is my review, some of which we have already discussed on slack:
[x] The enrollment success view could use some love, it should read "your factor has been setup successfully", and it should not auto-close. The user should close the modal after they have grokked the message.
[ ] I have to press the enter button twice to submit the phone number form when I am enrolling with SMS. The first press has no affect.
[x] If my access token has expired, but I try to create a factor, the API errors and the MFA view just sits there. A check might be useful here, otherwise let's just assume the developer has already handled the logout state and the user can't access the MFA workflow?
[x] When creating a GA factor, we need to set a unique 4-character string for the issuer
field, so that the user can disambiguate between our GA entry and other entries that they may already exist in the app for their email address. This is a workaround until we modify the MFA Policy to allow the developer to specify the issuer for the application. We already did this for ID Site, please use this function to create a nice alphanumeric string. Then it will look like this for the end-user in their GA app:
[x] Following the last point, the factor select view also needs some love. Here are the points, and a screenshot that shows how I think this could look:
factor_select
response at this step, to further re-enforce that these are factors that the user already has.Regarding:
This seems to be related to autocomplete. I'm looking into how to fix it. Could you just verify that it only occurs when you select something from autocomplete and not when you enter it manually?
Also, thanks for the feedback! It was really good and I agree with all of the changes.
How to verify
In order to activate the MFA workflow, you need to configure the mfa policy resource of your application as shown below:
Fixes #56