-
Notifications
You must be signed in to change notification settings - Fork 115
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
LG-10687 Break Up MFA Presenters #9211
Conversation
app/presenters/two_factor_authentication/set_up_selection_presenter.rb
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good!
Tested locally and no problems noted.
Is this pull request intended as a proof-of-concept limited to a single authentication method (TOTP) with the expectation that the rest will be ported in follow-up pull requests? |
Yes, this is the initial PR with proof of concept and base options available, and then we can do follow up requests per pull requests for each additional authentication method. @aduth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍 Couple minor comments.
🎫 Ticket
LG-10687
🛠 Summary of changes
This would create the workflow for creating Presenters for the Options presenter, with Sign in and Set up presenters to decouple the logic into seperate presenters
📜 Testing Plan
Adding MFA options and selecting a bunch of MFA options still works as expected on account creation
For Sign in, users can look at list of MFA options when clicking choose another authentication method, and can still sign in without issue.