-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Unify and simplify MFA Ceremony helpers #46986
Conversation
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, no major comments on my part.
Please take a look at the test failures.
28edc50
to
09dbce8
Compare
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.
LGTM.
* Refactor MFA ceremony helpers. * Refactor session MFA ceremony to use new MFA ceremony helpers. * Simplify calls to NewMFACeremony. * Remove remaining usage of tc.PromptMFA in favor of Ceremony. * Rename prompt constructor. * Add godoc to ceremony; update tests. * Cleanup. * Resolve comments; fix tests. * Update comments. * Fix test. * Fix lint.
* Refactor MFA ceremony helpers. * Refactor session MFA ceremony to use new MFA ceremony helpers. * Simplify calls to NewMFACeremony. * Remove remaining usage of tc.PromptMFA in favor of Ceremony. * Rename prompt constructor. * Add godoc to ceremony; update tests. * Cleanup. * Resolve comments; fix tests. * Update comments. * Fix test. * Fix lint.
There are many more hard to fix conflicts with the v15 backport than I would have hoped., so I'm going to hold off on it unless absolutely necessary. |
* Refactor MFA ceremony helpers. * Refactor session MFA ceremony to use new MFA ceremony helpers. * Simplify calls to NewMFACeremony. * Remove remaining usage of tc.PromptMFA in favor of Ceremony. * Rename prompt constructor. * Add godoc to ceremony; update tests. * Cleanup. * Resolve comments; fix tests. * Update comments. * Fix test. * Fix lint.
This is a follow up to the previous MFA Ceremony PR to unify MFA ceremonies with the same ceremony helpers. I also took steps to simplify the ceremony helpers. This change will make it easier to apply MFA ceremony changes, such as SSO MFA to one spot rather than several slightly modified spots.
This PR should have no functional impact.