Skip to content

LG-10022: Add second MFA reminder screen#9124

Merged
aduth merged 1 commit intomainfrom
aduth-lg-10022-mfa-encouragement
Sep 18, 2023
Merged

LG-10022: Add second MFA reminder screen#9124
aduth merged 1 commit intomainfrom
aduth-lg-10022-mfa-encouragement

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Aug 31, 2023

🎫 Ticket

LG-10022

🛠 Summary of changes

Adds a new MFA reminder screen that shows if the user only has a single MFA method, upon the first of either (a) the user has signed in 10 or more times or (b) their account is 30 days old.

📜 Testing Plan

It's easiest to test by setting an artificially low config override for sign-ins or account page, e.g. in config/application.yml:

second_mfa_reminder_sign_in_count: 2
  1. Go to http://localhost:3000
  2. Create an account
  3. After account creation, sign out
  4. Sign in
  5. Observe that you are prompted with the screen in screenshots below if (and only if) you had set up a single method during account creation
  6. Click one of the action buttons
  7. Observe that:
    a. If you chose to set up another MFA method, you're redirected to the MFA setup screen, with a "Cancel" link at the bottom
    b. If you chose to Continue, you're redirected to your account dashboard (or to the partner if coming from a partner authentication request)
  8. Sign out
  9. Sign in
  10. Observe that you are not prompted with the screen in screenshots below regardless of which action you took at Step 6

👀 Screenshots

Screenshot 2023-08-31 at 10 22 25 AM

@aduth aduth force-pushed the aduth-lg-10022-mfa-encouragement branch from 91c50b6 to a6899fd Compare September 6, 2023 19:32
@aduth aduth marked this pull request as ready for review September 6, 2023 19:33
@aduth aduth requested a review from a team September 6, 2023 19:33
@aduth aduth marked this pull request as draft September 8, 2023 13:15
@aduth aduth marked this pull request as draft September 8, 2023 13:15
@aduth
Copy link
Contributor Author

aduth commented Sep 8, 2023

Another thing to sort out while draft: I need to add more logging to satisfy the AC:

  • Log when the user successfully sets up MFA after clicking upsell Edit: Added in 87236b1

@aduth aduth force-pushed the aduth-lg-10022-mfa-encouragement branch from e1e1efb to a6fa689 Compare September 8, 2023 14:08
@aduth aduth marked this pull request as ready for review September 8, 2023 19:18
@aduth aduth changed the title LG-10022: Add second MFA encouragement screen LG-10022: Add second MFA reminder screen Sep 11, 2023
@aduth aduth force-pushed the aduth-lg-10022-mfa-encouragement branch from 87236b1 to 9b1d883 Compare September 11, 2023 12:16
@aduth
Copy link
Contributor Author

aduth commented Sep 15, 2023

Since I expect this page will get a lot of traffic and we're adding new routes which won't exist on old boxes during 50/50 state, I think I'm going to add a temporary feature flag so that we can get the code fully deployed before turning this on, to handle that more gracefully.

@aduth
Copy link
Contributor Author

aduth commented Sep 15, 2023

I found an issue here where clicking "Continue to [SP]" looks like it just refreshes the page when authenticating with a partner with whom the user has already consented. I'm wondering if it's an issue where we're expecting to redirect to after_sign_in_path_for, which would try immediately concluding the authentication via sp_session_request_url_with_updated_params, but since this is initiated by a PUT (not a POST), it doesn't work as expected. I think we've seen issues like this in the past with trying to redirect to completion URLs 😕

@aduth
Copy link
Contributor Author

aduth commented Sep 15, 2023

I think we've seen issues like this in the past with trying to redirect to completion URLs 😕

More specifically there's a CSP violation when trying to POST back to the partner like this.

@aduth aduth marked this pull request as draft September 15, 2023 13:34
@aduth
Copy link
Contributor Author

aduth commented Sep 15, 2023

I found an issue here where clicking "Continue to [SP]" looks like it just refreshes the page when authenticating with a partner with whom the user has already consented.

This should be fixed now in 84b028a, the primary revision being the inclusion of apply_secure_headers_override.

@aduth aduth marked this pull request as ready for review September 15, 2023 13:43
@aduth aduth force-pushed the aduth-lg-10022-mfa-encouragement branch from 456de2c to 4dcb848 Compare September 18, 2023 13:56
changelog: User-Facing Improvements, MFA Setup, Add second MFA encouragement screen for single-MFA accounts
@aduth aduth force-pushed the aduth-lg-10022-mfa-encouragement branch from 4dcb848 to 059f918 Compare September 18, 2023 14:15
@aduth aduth merged commit b329bdd into main Sep 18, 2023
@aduth aduth deleted the aduth-lg-10022-mfa-encouragement branch September 18, 2023 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants