From d0542e9d88ca8ef5d8b0b8daf21bdb149d40145c Mon Sep 17 00:00:00 2001 From: Dan Share Date: Mon, 16 Feb 2026 08:53:40 +0000 Subject: [PATCH] [Browser MFA] Add BrowserMFARequestID to CreateAuthenticateChallenge --- lib/auth/auth.go | 34 +++++++++ lib/auth/browser_mfa_test.go | 141 ++++++++++++++++++++++++++++++++++- lib/auth/mfatypes/proto.go | 32 ++++++++ lib/web/mfa.go | 22 ++++-- 4 files changed, 222 insertions(+), 7 deletions(-) create mode 100644 lib/auth/mfatypes/proto.go diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 8a43bfff61546..7fb5f2c05cda0 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -1463,6 +1463,11 @@ type Server struct { // normally be fetched from the GitHub API. Used for testing. GithubUserAndTeamsOverride func() (*GithubUserResponse, []GithubTeamResponse, error) + // ObserveBrowserMFAChallengeExtensionsForTesting, if set, is called with the resolved + // challenge extensions when a BrowserMFARequestID is provided to + // CreateAuthenticateChallenge. Used for testing. + ObserveBrowserMFAChallengeExtensionsForTesting func(*mfav1.ChallengeExtensions) + // AWSRolesAnywhereCreateSessionOverride overrides the AWS Roles Anywhere Create Session API wrapper with a mocked one. // Used for testing. AWSRolesAnywhereCreateSessionOverride func(ctx context.Context, req createsession.CreateSessionRequest) (*createsession.CreateSessionResponse, error) @@ -4254,6 +4259,35 @@ func (a *Server) CreateAuthenticateChallenge(ctx context.Context, req *proto.Cre return nil } + // If a BrowserMFARequestID is provided, look up the request and apply its challenge extensions. + if req.BrowserMFARequestID != "" { + if req.ChallengeExtensions != nil { + return nil, trace.BadParameter("challenge extensions must not be set when a browser MFA request ID is provided") + } + + browserMFAReq, err := a.GetSSOMFASession(ctx, req.BrowserMFARequestID) + if err != nil { + a.logger.WarnContext(ctx, "Failed to read MFA session for browser MFA request", "error", err) + return nil, trace.AccessDenied("invalid browser MFA request") + } + + chalExts := browserMFAReq.ChallengeExtensions + if chalExts == nil { + a.logger.WarnContext(ctx, "MFA session for browser MFA recorded with empty challenge extensions", "request_id", req.BrowserMFARequestID) + return nil, trace.BadParameter("stored session lacks challenge extensions") + } + + // Replace the challenge extensions with the ones found in the SSO MFA object. + // These are the ones from the original tsh request. + challengeExtensions = mfatypes.ChallengeExtensionsToProto(chalExts) + + // Used for testing. If observer function is set, call it with + // challenge extensions from SSO MFA session. + if a.ObserveBrowserMFAChallengeExtensionsForTesting != nil { + a.ObserveBrowserMFAChallengeExtensionsForTesting(challengeExtensions) + } + } + switch req.GetRequest().(type) { case *proto.CreateAuthenticateChallengeRequest_UserCredentials: username = req.GetUserCredentials().GetUsername() diff --git a/lib/auth/browser_mfa_test.go b/lib/auth/browser_mfa_test.go index 4896335004cfe..9b61af8403b4b 100644 --- a/lib/auth/browser_mfa_test.go +++ b/lib/auth/browser_mfa_test.go @@ -27,6 +27,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/gravitational/teleport/api/client/proto" mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/auth/authtest" @@ -221,7 +222,9 @@ func TestCompleteBrowserMFAChallenge(t *testing.T) { ConnectorID: "test-connector", ConnectorType: "test", ChallengeExtensions: &mfatypes.ChallengeExtensions{ - Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + UserVerificationRequirement: "required", }, } err := a.UpsertSSOMFASessionData(ctx, session) @@ -314,3 +317,139 @@ func TestCompleteBrowserMFAChallenge(t *testing.T) { }) } } + +func TestCreateAuthenticateChallenge_BrowserMFARequestID(t *testing.T) { + t.Parallel() + ctx := t.Context() + + testServer, err := authtest.NewTestServer(authtest.ServerConfig{ + Auth: authtest.AuthServerConfig{ + Dir: t.TempDir(), + }, + }) + require.NoError(t, err) + t.Cleanup(func() { assert.NoError(t, testServer.Close()) }) + + a := testServer.Auth() + + userCreds, err := createUserWithSecondFactors(testServer.TLS) + require.NoError(t, err) + + userCredsRequest := &proto.CreateAuthenticateChallengeRequest_UserCredentials{ + UserCredentials: &proto.UserCredentials{ + Username: userCreds.username, + Password: userCreds.password, + }, + } + + tests := []struct { + name string + setup func(t *testing.T) + request *proto.CreateAuthenticateChallengeRequest + checkError func(t *testing.T, err error) + wantExtensions *mfav1.ChallengeExtensions + }{ + { + name: "NOK invalid browser MFA request ID", + request: &proto.CreateAuthenticateChallengeRequest{ + Request: userCredsRequest, + BrowserMFARequestID: "non-existent-id", + }, + checkError: func(t *testing.T, err error) { + assert.ErrorAs(t, err, new(*trace.AccessDeniedError), "CreateAuthenticateChallenge error mismatch") + assert.ErrorContains(t, err, "invalid browser MFA request") + }, + }, + { + name: "NOK challenge extensions set with browser MFA request ID", + request: &proto.CreateAuthenticateChallengeRequest{ + Request: userCredsRequest, + BrowserMFARequestID: "some-request-id", + ChallengeExtensions: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }, + }, + checkError: func(t *testing.T, err error) { + assert.ErrorAs(t, err, new(*trace.BadParameterError), "CreateAuthenticateChallenge error mismatch") + assert.ErrorContains(t, err, "challenge extensions must not be set") + }, + }, + { + name: "OK browser MFA challenge extensions applied from SSO MFA session", + setup: func(t *testing.T) { + session := &services.SSOMFASessionData{ + RequestID: "test-request-1", + Username: userCreds.username, + ConnectorID: "Browser", + ConnectorType: "Browser", + ChallengeExtensions: &mfatypes.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + UserVerificationRequirement: "required", + }, + } + err := a.UpsertSSOMFASessionData(ctx, session) + require.NoError(t, err) + }, + request: &proto.CreateAuthenticateChallengeRequest{ + Request: userCredsRequest, + BrowserMFARequestID: "test-request-1", + }, + wantExtensions: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + UserVerificationRequirement: "required", + }, + }, + { + name: "NOK nil challenge extensions", + setup: func(t *testing.T) { + session := &services.SSOMFASessionData{ + RequestID: "test-request-2", + Username: userCreds.username, + ConnectorID: "Browser", + ConnectorType: "Browser", + ChallengeExtensions: nil, + } + err := a.UpsertSSOMFASessionData(ctx, session) + require.NoError(t, err) + }, + request: &proto.CreateAuthenticateChallengeRequest{ + Request: userCredsRequest, + BrowserMFARequestID: "test-request-2", + }, + checkError: func(t *testing.T, err error) { + assert.ErrorAs(t, err, new(*trace.BadParameterError), "CreateAuthenticateChallenge error mismatch") + assert.ErrorContains(t, err, "stored session lacks challenge extensions") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.setup != nil { + tt.setup(t) + } + + var gotExtensions *mfav1.ChallengeExtensions + a.ObserveBrowserMFAChallengeExtensionsForTesting = func(ext *mfav1.ChallengeExtensions) { + gotExtensions = ext + } + + challenge, err := a.CreateAuthenticateChallenge(ctx, tt.request) + + if tt.checkError != nil { + tt.checkError(t, err) + return + } + + require.NoError(t, err) + require.NotNil(t, challenge) + assert.NotNil(t, challenge.WebauthnChallenge, "expected WebAuthn challenge to be present") + + if tt.wantExtensions != nil { + require.Equal(t, tt.wantExtensions, gotExtensions) + } + }) + } +} diff --git a/lib/auth/mfatypes/proto.go b/lib/auth/mfatypes/proto.go new file mode 100644 index 0000000000000..ff7c5f389b03c --- /dev/null +++ b/lib/auth/mfatypes/proto.go @@ -0,0 +1,32 @@ +// Teleport +// Copyright (C) 2026 Gravitational, Inc. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package mfatypes + +import mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" + +// ChallengeExtensionsToProto converts a ChallengeExtensions to its protobuf representation. +func ChallengeExtensionsToProto(chalExts *ChallengeExtensions) *mfav1.ChallengeExtensions { + if chalExts == nil { + return nil + } + + return &mfav1.ChallengeExtensions{ + Scope: chalExts.Scope, + AllowReuse: chalExts.AllowReuse, + UserVerificationRequirement: chalExts.UserVerificationRequirement, + } +} diff --git a/lib/web/mfa.go b/lib/web/mfa.go index 979649352150e..c4213e7dab836 100644 --- a/lib/web/mfa.go +++ b/lib/web/mfa.go @@ -141,6 +141,7 @@ type CreateAuthenticateChallengeRequest struct { ChallengeAllowReuse bool `json:"challenge_allow_reuse"` UserVerificationRequirement string `json:"user_verification_requirement"` ProxyAddress string `json:"proxy_address"` + BrowserMFARequestID string `json:"browser_mfa_request_id"` } // createAuthenticateChallengeHandle creates and returns MFA authentication challenges for the user in context (logged in user). @@ -218,18 +219,27 @@ func (h *Handler) createAuthenticateChallengeHandle(w http.ResponseWriter, r *ht query.Set("channel_id", channelID) ssoClientRedirectURL.RawQuery = query.Encode() - chal, err := clt.CreateAuthenticateChallenge(ctx, &proto.CreateAuthenticateChallengeRequest{ - Request: &proto.CreateAuthenticateChallengeRequest_ContextUser{ - ContextUser: &proto.ContextUser{}, - }, - MFARequiredCheck: mfaRequiredCheckProto, - ChallengeExtensions: &mfav1.ChallengeExtensions{ + // If BrowserMFARequestID is set, don't set the challenge extensions. + // They will be gotten from the stored MFASession on the backend and + // applied to the challenge. + var challengeExtensions *mfav1.ChallengeExtensions + if req.BrowserMFARequestID == "" { + challengeExtensions = &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope(req.ChallengeScope), AllowReuse: allowReuse, UserVerificationRequirement: req.UserVerificationRequirement, + } + } + + chal, err := clt.CreateAuthenticateChallenge(ctx, &proto.CreateAuthenticateChallengeRequest{ + Request: &proto.CreateAuthenticateChallengeRequest_ContextUser{ + ContextUser: &proto.ContextUser{}, }, + MFARequiredCheck: mfaRequiredCheckProto, + ChallengeExtensions: challengeExtensions, SSOClientRedirectURL: ssoClientRedirectURL.String(), ProxyAddress: req.ProxyAddress, + BrowserMFARequestID: req.BrowserMFARequestID, }) if err != nil { return nil, trace.Wrap(err)