Skip to content

Support SSO MFA for headless#53741

Merged
atburke merged 1 commit intomasterfrom
atburke/headless-sso-mfa
Apr 7, 2025
Merged

Support SSO MFA for headless#53741
atburke merged 1 commit intomasterfrom
atburke/headless-sso-mfa

Conversation

@atburke
Copy link
Copy Markdown
Contributor

@atburke atburke commented Apr 4, 2025

This change adds support for SSO MFA in headless mode.

Follow-up to #53166.

Changelog: Added support for SSO MFA as a headless MFA method

@atburke atburke requested review from Joerger, rosstimothy and zmb3 April 4, 2025 23:31
@github-actions github-actions Bot added rfd Request for Discussion size/sm ui labels Apr 4, 2025
@github-actions github-actions Bot requested review from kimlisa and rudream April 4, 2025 23:32
Comment thread lib/web/headless.go Outdated
Comment thread rfd/0180-sso-mfa.md Outdated
This change adds support for SSO MFA in headless mode.
@atburke atburke force-pushed the atburke/headless-sso-mfa branch from 5e50b0d to 2bd6edf Compare April 7, 2025 18:25
@atburke atburke enabled auto-merge April 7, 2025 18:26
@atburke atburke added this pull request to the merge queue Apr 7, 2025
Merged via the queue into master with commit b4b79e3 Apr 7, 2025
43 checks passed
@atburke atburke deleted the atburke/headless-sso-mfa branch April 7, 2025 19:46
atburke added a commit that referenced this pull request May 7, 2025
This change adds support for SSO MFA in headless mode.

Co-authored-by: joerger <bjoerger@goteleport.com>
github-merge-queue Bot pushed a commit that referenced this pull request May 7, 2025
This change adds support for SSO MFA in headless mode.

Co-authored-by: joerger <bjoerger@goteleport.com>
Comment thread lib/web/headless.go
Comment on lines +68 to +77
if req.MFAResponse == nil && req.WebauthnAssertionResponse != nil {
req.MFAResponse = &client.MFAChallengeResponse{
WebauthnResponse: req.WebauthnAssertionResponse,
}
}

mfaResp, err := req.MFAResponse.GetOptionalMFAResponseProtoReq()
if err != nil {
return nil, trace.Wrap(err)
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this might have introduced a regression where rejecting a headless login causes a panic (#58864).

2026-01-27T15:38:10.522+01:00 INFO  "http: panic serving 127.0.0.1:49464: runtime error: invalid memory address or nil pointer dereference

goroutine 6747436 [running]:
net/http.(*conn).serve.func1()
	net/http/server.go:1943 +0xb4
panic({0x10ee85f20?, 0x116b50be0?})
	runtime/panic.go:783 +0x120
github.com/gravitational/teleport/lib/client.(*MFAChallengeResponse).GetOptionalMFAResponseProtoReq(0x14002ee0640?)
	github.com/gravitational/teleport/lib/client/weblogin.go:139 +0x1c
github.com/gravitational/teleport/lib/web.(*Handler).putHeadlessState(0x14002ee0640?, {0x110703b50?, 0x14003ac6cc0?}, 0x14002ee0640, {0x14001a28940?, 0x19?, 0x14003639360?}, 0x14003639360)
	github.com/gravitational/teleport/lib/web/headless.go:74 +0xfc
github.com/gravitational/teleport/lib/web.(*Handler).bindDefaultEndpoints.(*Handler).WithAuth.func168({0x1106fe860, 0x14004992060}, 0x14002ee0640, {0x14001a28940, 0x1, 0x1})
	github.com/gravitational/teleport/lib/web/apiserver.go:5198 +0xb4
github.com/gravitational/teleport/lib/web.(*Handler).bindDefaultEndpoints.(*Handler).WithAuth.MakeHandler.MakeHandlerWithErrorWriter.func373({0x1106fe860, 0x14004992060}, 0x14002ee0640, {0x14001a28940, 0x1, 0x1})
…

I suppose req.MFAResponse might be nil but we attempt to read it on line 74.

Copy link
Copy Markdown
Member

@ravicious ravicious Jan 27, 2026

Choose a reason for hiding this comment

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

The Web UI uses the same endpoint to approve and reject a headless request, but rejecting a request should not require MFA.

Edit: Fix here. #63189

headlessSsoAccept(mfa: MfaState, transactionId: string) {
return mfa.getChallengeResponse().then((res: MfaChallengeResponse) => {
const request = {
action: 'accept',
mfaResponse: res,
// TODO(Joerger): DELETE IN v19.0.0, new clients send mfaResponse.
webauthnAssertionResponse: res.webauthn_response,
};
return api.put(cfg.getHeadlessSsoPath(transactionId), request);
});
},
headlessSSOReject(transactionId: string) {
const request = {
action: 'denied',
};
return api.put(cfg.getHeadlessSsoPath(transactionId), request);
},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rfd Request for Discussion size/sm ui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants