Skip to content

LG-9214: mfa selection priority#8130

Merged
mdiarra3 merged 11 commits intomainfrom
LG-9214-mfa-selection-priority
Apr 12, 2023
Merged

LG-9214: mfa selection priority#8130
mdiarra3 merged 11 commits intomainfrom
LG-9214-mfa-selection-priority

Conversation

@mdiarra3
Copy link
Contributor

@mdiarra3 mdiarra3 commented Apr 4, 2023

🎫 Ticket

Lg-9214

🛠 Summary of changes

Sets up MFA selection order test for A/B testing. add logging on buckets to query for the testing.

@mdiarra3 mdiarra3 marked this pull request as ready for review April 5, 2023 18:26
@mdiarra3 mdiarra3 requested a review from a team April 5, 2023 18:26
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Functionally works great 👍 Most comments are minor / stylistic.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remind me why we have two different controllers for MFA setup? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ugh something I want to refactor, the mfa_selection controller is for the controller to select/add additional mfa methods after the prompt if a user selects a single MFA method on account creation. I think originally the mfa selection had a few extra things that were extra/annoying (showing/graying out already selected mfa method, as well as two factor authentication setup controller will redirect you to account page if you have one authentication method already). We can definitely make this a lot cleaner.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you want to write a ticket summarizing the problem and maybe we can get that work prioritized?

@mdiarra3 mdiarra3 merged commit a9318b3 into main Apr 12, 2023
@mdiarra3 mdiarra3 deleted the LG-9214-mfa-selection-priority branch April 12, 2023 17:41
@solipet solipet mentioned this pull request Apr 13, 2023
jc-gsa pushed a commit that referenced this pull request Apr 19, 2023
* changelog: Upcoming Features, Authentication, Add capability for ab testing to see if changing priority affects user selection

* add additional params to loggin params

* add bucket

* rubocop and other specs

* fix linting

* analytics events

* fix up rspec

* LG-9214: address comments

* rubocop
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