Skip to content

LG-10050: Prevent MFA setup validation error from persisting#8984

Merged
aduth merged 2 commits intomainfrom
aduth-lg-10050-required-field-error
Aug 14, 2023
Merged

LG-10050: Prevent MFA setup validation error from persisting#8984
aduth merged 2 commits intomainfrom
aduth-lg-10050-required-field-error

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Aug 11, 2023

🎫 Ticket

LG-10050

🛠 Summary of changes

Updates validation logic for required MFA selection during account creation to prevent the "You must select an authentication method" from lingering unexpectedly after making a selection.

This includes some general refactoring to validation and error handling logic, to reinforce expectations around the shape and value formats of expected parameters and how (and where) error messages are computed.

The fix itself is using flash.now instead of flash where a redirect is not involved.

📜 Testing Plan

  1. Go to http://localhost:3000
  2. Click "Create an account"
  3. Complete account creation up to MFA selection screen
  4. Click "Submit" without selecting an MFA method
  5. Observe the validation error "Select an authentication method"
  6. Select an MFA method (e.g. phone) and click "Submit"

Before: The error message would persist onto the MFA setup screen.
After: The error message does not persist onto the MFA setup screen.

👀 Screenshots

Before After
image image

@aduth aduth requested a review from a team August 11, 2023 14:32
aduth added 2 commits August 11, 2023 10:57
changelog: Bug Fixes, MFA Setup, Fix issue where missing selection error message lingered longer than expected
Copy link
Contributor

@mitchellhenke mitchellhenke left a comment

Choose a reason for hiding this comment

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

Hm, this is another one that would have been caught by Rails/ActionControllerFlashBeforeRender

Was mentioned in #8961, but not enabled due to false positives. Maybe we should consider enabling it?

@aduth aduth force-pushed the aduth-lg-10050-required-field-error branch from 305113e to f7bba6c Compare August 11, 2023 14:57
@aduth
Copy link
Contributor Author

aduth commented Aug 11, 2023

Was mentioned in #8961, but not enabled due to false positives. Maybe we should consider enabling it?

Maybe! I'd be curious to test to see what the false positives look like.

Edit: For posterity, see #8986

@aduth aduth merged commit bb1c3c5 into main Aug 14, 2023
@aduth aduth deleted the aduth-lg-10050-required-field-error branch August 14, 2023 12:10
@jmdembe jmdembe mentioned this pull request Aug 15, 2023
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.

2 participants