-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: SSO MFA - Incorporate SSO into cli mfa method preferences #47839
feat: SSO MFA - Incorporate SSO into cli mfa method preferences #47839
Conversation
This pull request is automatically being deployed by Amplify Hosting (learn more). |
c824e93
to
2420095
Compare
9df809b
to
c95d128
Compare
b673e04
to
913a35c
Compare
@@ -101,40 +107,71 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng | |||
|
|||
promptOTP := chal.TOTP != nil | |||
promptWebauthn := chal.WebauthnChallenge != nil | |||
promptSSO := false // TODO(Joerger): check for SSO challenge once added in separate PR. |
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.
I do appreciate the small change but it looks like we are mainly adding dead code to this file. Would it make sense to do a bit more here, so at least we can test the "SSO not implemented" error? Or maybe move some changes to a PR where they are effective?
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.
Sure, in that case I'll merge this into #46982
Part of the implementation of #44699
Implements the UX proposed in the RFD for CLI prompts.
Depends on #47874