Skip to content

Support SSO MFA for headless#53166

Closed
Joerger wants to merge 2 commits intomasterfrom
joerger/headless-sso-mfa
Closed

Support SSO MFA for headless#53166
Joerger wants to merge 2 commits intomasterfrom
joerger/headless-sso-mfa

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Mar 18, 2025

Changelog: Support SSO MFA as an MFA method for headless.

Copy link
Copy Markdown
Collaborator

@zmb3 zmb3 left a comment

Choose a reason for hiding this comment

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

Can we get some test coverage, or at least additions to the manual test plan?

@@ -219,9 +219,7 @@ const auth = {
},

headlessSSOGet(transactionId: string) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

While we're here, it should be headlessSsoGet.

@@ -234,10 +232,12 @@ const auth = {
headlessSSOAccept(transactionId: string) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we're changing headlessSSOGet then we should also rename this to headlessSsoAccept.

default:
err = trace.BadParameter("MFA response of type %T is not supported for headless authentication", mfaResp.Response)
emitHeadlessLoginEvent(ctx, events.UserHeadlessLoginApprovedFailureCode, a.authServer.emitter, headlessAuthn, err)
return err
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't we still be returning this error?

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.

3 participants