diff --git a/api/constants/constants.go b/api/constants/constants.go index 263cce8bf938e..6a678ed0121fd 100644 --- a/api/constants/constants.go +++ b/api/constants/constants.go @@ -67,6 +67,9 @@ const ( // Github means authentication will happen remotely using a Github connector. Github = "github" + // BrowserMFA is for CLI flows that delegate MFA to the browser. + BrowserMFA = "browser_mfa" + // HumanDateFormatSeconds is a human readable date formatting with seconds HumanDateFormatSeconds = "Jan 2 2006 15:04:05 UTC" diff --git a/api/proto/teleport/legacy/types/mfa_device.proto b/api/proto/teleport/legacy/types/mfa_device.proto index 821159488de2d..e8a1804da3c0f 100644 --- a/api/proto/teleport/legacy/types/mfa_device.proto +++ b/api/proto/teleport/legacy/types/mfa_device.proto @@ -128,6 +128,6 @@ message SSOMFADevice { } // BrowserMFADevice is a synthetic MFA device that is made available if a user -// has at least one WebAuthn device and no SSO setup. This message doesn't +// has at least one WebAuthn device and no SSO MFA setup. This message doesn't // require any fields, it just needs to exist so it can be an MFA option. message BrowserMFADevice {} diff --git a/api/types/mfa_device.pb.go b/api/types/mfa_device.pb.go index 5f40d59b02b0c..21f21e81267c3 100644 --- a/api/types/mfa_device.pb.go +++ b/api/types/mfa_device.pb.go @@ -382,7 +382,7 @@ func (m *SSOMFADevice) XXX_DiscardUnknown() { var xxx_messageInfo_SSOMFADevice proto.InternalMessageInfo // BrowserMFADevice is a synthetic MFA device that is made available if a user -// has at least one WebAuthn device and no SSO setup. This message doesn't +// has at least one WebAuthn device and no SSO MFA setup. This message doesn't // require any fields, it just needs to exist so it can be an MFA option. type BrowserMFADevice struct { XXX_NoUnkeyedLiteral struct{} `json:"-"` diff --git a/gen/proto/ts/teleport/legacy/types/mfa_device_pb.ts b/gen/proto/ts/teleport/legacy/types/mfa_device_pb.ts index 3c29c116220d0..59d0c2b98263c 100644 --- a/gen/proto/ts/teleport/legacy/types/mfa_device_pb.ts +++ b/gen/proto/ts/teleport/legacy/types/mfa_device_pb.ts @@ -258,7 +258,7 @@ export interface SSOMFADevice { } /** * BrowserMFADevice is a synthetic MFA device that is made available if a user - * has at least one WebAuthn device and no SSO setup. This message doesn't + * has at least one WebAuthn device and no SSO MFA setup. This message doesn't * require any fields, it just needs to exist so it can be an MFA option. * * @generated from protobuf message types.BrowserMFADevice diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 7fb5f2c05cda0..b6d22e3ad2e37 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -4346,7 +4346,16 @@ func (a *Server) CreateAuthenticateChallenge(ctx context.Context, req *proto.Cre } } - challenges, err := a.mfaAuthChallenge(ctx, username, req.SSOClientRedirectURL, req.ProxyAddress, challengeExtensions) + // When completing a Browser MFA flow, only a WebAuthn challenge is needed, so + // clear the redirect URL so SSO and Browser MFA challenges are not generated. + clientRedirectURL := "" + if req.BrowserMFARequestID == "" { + // Both SSO and Browser MFA redirect URLs point to the same callback server on tsh. + // So we can take either one and generate an auth challenge with it. + clientRedirectURL = cmp.Or(req.SSOClientRedirectURL, req.BrowserMFATSHRedirectURL) + } + + challenges, err := a.mfaAuthChallenge(ctx, username, clientRedirectURL, req.ProxyAddress, challengeExtensions) if err != nil { // Do not obfuscate config-related errors. if errors.Is(err, types.ErrPasswordlessRequiresWebauthn) || errors.Is(err, types.ErrPasswordlessDisabledBySettings) { @@ -7882,7 +7891,7 @@ func (a *Server) isMFARequired(ctx context.Context, checker services.AccessCheck // mfaAuthChallenge constructs an MFAAuthenticateChallenge for all MFA devices // registered by the user. -func (a *Server) mfaAuthChallenge(ctx context.Context, user, ssoClientRedirectURL, proxyAddress string, challengeExtensions *mfav1.ChallengeExtensions) (*proto.MFAAuthenticateChallenge, error) { +func (a *Server) mfaAuthChallenge(ctx context.Context, user, clientRedirectURL, proxyAddress string, challengeExtensions *mfav1.ChallengeExtensions) (*proto.MFAAuthenticateChallenge, error) { isPasswordless := challengeExtensions.Scope == mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN // Check what kind of MFA is enabled. @@ -7893,6 +7902,7 @@ func (a *Server) mfaAuthChallenge(ctx context.Context, user, ssoClientRedirectUR enableTOTP := apref.IsSecondFactorTOTPAllowed() enableWebauthn := apref.IsSecondFactorWebauthnAllowed() enableSSO := apref.IsSecondFactorSSOAllowed() + enableBrowserMFA := apref.GetAllowCLIAuthViaBrowser() // Fetch configurations. The IsSecondFactor*Allowed calls above already // include the necessary checks of config empty, disabled, etc. @@ -7988,13 +7998,13 @@ func (a *Server) mfaAuthChallenge(ctx context.Context, user, ssoClientRedirectUR // If the user has an SSO device and the client provided a redirect URL to handle // the MFA SSO flow, create an SSO challenge. - if enableSSO && groupedDevs.SSO != nil && ssoClientRedirectURL != "" { + if enableSSO && groupedDevs.SSO != nil && clientRedirectURL != "" { if challenge.SSOChallenge, err = a.BeginSSOMFAChallenge( ctx, mfatypes.BeginSSOMFAChallengeParams{ User: user, SSO: groupedDevs.SSO.GetSso(), - SSOClientRedirectURL: ssoClientRedirectURL, + SSOClientRedirectURL: clientRedirectURL, ProxyAddress: proxyAddress, Ext: challengeExtensions, }, @@ -8003,6 +8013,23 @@ func (a *Server) mfaAuthChallenge(ctx context.Context, user, ssoClientRedirectUR } } + // If the user has a WebAuthn device, return a Browser MFA challenge. This + // challenge is useful in cases where a user only has a browser-associated + // WebAuthn device, but is trying to MFA via a CLI tool (tsh, tctl etc.) + if enableBrowserMFA && groupedDevs.Browser != nil && clientRedirectURL != "" { + if challenge.BrowserMFAChallenge, err = a.BeginBrowserMFAChallenge( + ctx, + mfatypes.BeginBrowserMFAChallengeParams{ + User: user, + BrowserMFATSHRedirectURL: clientRedirectURL, + ProxyAddress: proxyAddress, + Ext: challengeExtensions, + }, + ); err != nil { + return nil, trace.Wrap(err) + } + } + clusterName, err := a.GetClusterName(ctx) if err != nil { return nil, trace.Wrap(err) @@ -8029,6 +8056,7 @@ type devicesByType struct { TOTP bool Webauthn []*types.MFADevice SSO *types.MFADevice + Browser *types.MFADevice } func groupByDeviceType(devs []*types.MFADevice) devicesByType { @@ -8047,6 +8075,18 @@ func groupByDeviceType(devs []*types.MFADevice) devicesByType { logger.WarnContext(context.Background(), "Skipping MFA device with unknown type", "device_type", logutils.TypeAttr(dev.Device)) } } + + // Create a synthetic Browser device if the user has a WebAuthn device. + // This enables browser-based MFA for users who have WebAuthn/passkey devices. + if len(res.Webauthn) > 0 { + res.Browser = &types.MFADevice{ + Id: "browser", + Device: &types.MFADevice_Browser{ + Browser: &types.BrowserMFADevice{}, + }, + } + } + return res } diff --git a/lib/auth/browser_mfa.go b/lib/auth/browser_mfa.go index 740efc941341a..4220c371af3e2 100644 --- a/lib/auth/browser_mfa.go +++ b/lib/auth/browser_mfa.go @@ -20,12 +20,18 @@ import ( "context" "net/url" + "github.com/google/uuid" "github.com/gravitational/trace" + "github.com/gravitational/teleport/api/client/proto" + "github.com/gravitational/teleport/api/constants" webauthnpb "github.com/gravitational/teleport/api/types/webauthn" "github.com/gravitational/teleport/lib/auth/internal/browsermfa" + "github.com/gravitational/teleport/lib/auth/mfatypes" wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" "github.com/gravitational/teleport/lib/authz" + "github.com/gravitational/teleport/lib/client/sso" + "github.com/gravitational/teleport/lib/services" ) // CompleteBrowserMFAChallenge completes an MFA challenge response by returning the redirect URL with encrypted response. @@ -49,7 +55,7 @@ func (a *Server) CompleteBrowserMFAChallenge(ctx context.Context, requestID stri } // Valid WebAuthn response, encrypt and return it - u, err := url.Parse(mfaSession.ClientRedirectURL) + u, err := url.Parse(mfaSession.TSHRedirectURL) if err != nil { return "", trace.Wrap(err) } @@ -62,3 +68,39 @@ func (a *Server) CompleteBrowserMFAChallenge(ctx context.Context, requestID stri return clientRedirectURL, nil } + +// BeginBrowserMFAChallenge creates a new Browser MFA auth request and session +// data for the given params and stores it in the backend. +func (a *Server) BeginBrowserMFAChallenge(ctx context.Context, params mfatypes.BeginBrowserMFAChallengeParams) (*proto.BrowserMFAChallenge, error) { + if err := sso.ValidateClientRedirect(params.BrowserMFATSHRedirectURL, sso.CeremonyTypeMFA, nil); err != nil { + return nil, trace.Wrap(err, InvalidClientRedirectErrorMessage) + } + + requestID := uuid.NewString() + browserChal := &proto.BrowserMFAChallenge{ + RequestId: requestID, + } + + sessionData := &services.MFASessionData{ + Username: params.User, + RequestID: requestID, + ConnectorID: constants.BrowserMFA, + ConnectorType: constants.BrowserMFA, + TSHRedirectURL: params.BrowserMFATSHRedirectURL, + ChallengeExtensions: &mfatypes.ChallengeExtensions{ + Scope: params.Ext.Scope, + AllowReuse: params.Ext.AllowReuse, + }, + SourceCluster: params.SourceCluster, + TargetCluster: params.TargetCluster, + Payload: &mfatypes.SessionIdentifyingPayload{ + SSHSessionID: params.SIP.GetSshSessionId(), + }, + } + + if err := a.UpsertMFASessionData(ctx, sessionData); err != nil { + return nil, trace.Wrap(err) + } + + return browserChal, nil +} diff --git a/lib/auth/browser_mfa_test.go b/lib/auth/browser_mfa_test.go index 9b61af8403b4b..537df63cee028 100644 --- a/lib/auth/browser_mfa_test.go +++ b/lib/auth/browser_mfa_test.go @@ -24,11 +24,15 @@ import ( "github.com/google/uuid" "github.com/gravitational/trace" + "github.com/jonboulle/clockwork" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/gravitational/teleport/api/client/proto" + "github.com/gravitational/teleport/api/constants" mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/auth/authtest" "github.com/gravitational/teleport/lib/auth/internal/browsermfa" @@ -39,6 +43,83 @@ import ( "github.com/gravitational/teleport/lib/services" ) +const browserMFARedirectURL = "http://localhost:12345/callback?secret_key=test-key" + +type testEnv struct { + server *authtest.Server + auth *auth.Server + clock *clockwork.FakeClock + authPref types.AuthPreference + webauthnUser types.User + webauthnDev *types.MFADevice +} + +func newBrowserMFATestEnv(t *testing.T) testEnv { + t.Helper() + ctx := t.Context() + + fakeClock := clockwork.NewFakeClock() + testServer, err := authtest.NewTestServer(authtest.ServerConfig{ + Auth: authtest.AuthServerConfig{ + Dir: t.TempDir(), + Clock: fakeClock, + }, + }) + require.NoError(t, err) + t.Cleanup(func() { assert.NoError(t, testServer.Close()) }) + + a := testServer.Auth() + + // Register a proxy server so getProxyPublicAddr returns a valid address. + proxy, err := types.NewServer("test-proxy", types.KindProxy, types.ServerSpecV2{ + PublicAddrs: []string{"proxy.example.com:443"}, + }) + require.NoError(t, err) + err = a.UpsertProxy(ctx, proxy) + require.NoError(t, err) + + // Enable WebAuthn support. + authPref, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{ + Type: constants.Local, + SecondFactor: constants.SecondFactorWebauthn, + Webauthn: &types.Webauthn{ + RPID: "localhost", + }, + AllowCLIAuthViaBrowser: types.NewBoolOption(true), + }) + require.NoError(t, err) + _, err = a.UpsertAuthPreference(ctx, authPref) + require.NoError(t, err) + + // Create a user with a WebAuthn device. + webauthnUser, _, err := authtest.CreateUserAndRole(a, "webauthn-user", []string{"role"}, nil) + require.NoError(t, err) + + // Add a WebAuthn device for the webauthn user. + webauthnDev, err := types.NewMFADevice("webauthn-device", "webauthn-device-id", fakeClock.Now(), &types.MFADevice_Webauthn{ + Webauthn: &types.WebauthnDevice{ + CredentialId: []byte("credential-id"), + PublicKeyCbor: []byte("public-key"), + AttestationType: "none", + Aaguid: []byte("aaguid"), + SignatureCounter: 0, + ResidentKey: false, + }, + }) + require.NoError(t, err) + err = a.UpsertMFADevice(ctx, webauthnUser.GetName(), webauthnDev) + require.NoError(t, err) + + return testEnv{ + server: testServer, + auth: a, + clock: fakeClock, + authPref: authPref, + webauthnUser: webauthnUser, + webauthnDev: webauthnDev, + } +} + func TestEncryptBrowserMFAResponse(t *testing.T) { t.Parallel() @@ -216,18 +297,18 @@ func TestCompleteBrowserMFAChallenge(t *testing.T) { requestID := uuid.NewString() redirectURL := "http://127.0.0.1:62972/callback?secret_key=" + secretKey.String() session := &services.SSOMFASessionData{ - RequestID: requestID, - Username: "other-user", // mismatch username - ClientRedirectURL: redirectURL, - ConnectorID: "test-connector", - ConnectorType: "test", + RequestID: requestID, + Username: "other-user", // mismatch username + TSHRedirectURL: redirectURL, + ConnectorID: "test-connector", + ConnectorType: "test", ChallengeExtensions: &mfatypes.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, UserVerificationRequirement: "required", }, } - err := a.UpsertSSOMFASessionData(ctx, session) + err := a.UpsertMFASessionData(ctx, session) require.NoError(t, err) return requestID }, @@ -241,16 +322,16 @@ func TestCompleteBrowserMFAChallenge(t *testing.T) { setupSession: func(t *testing.T) string { requestID := uuid.NewString() session := &services.SSOMFASessionData{ - RequestID: requestID, - Username: username, - ClientRedirectURL: "://invalid-url", - ConnectorID: "test-connector", - ConnectorType: "test", + RequestID: requestID, + Username: username, + TSHRedirectURL: "://invalid-url", + ConnectorID: "test-connector", + ConnectorType: "test", ChallengeExtensions: &mfatypes.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }, } - err := a.UpsertSSOMFASessionData(ctx, session) + err := a.UpsertMFASessionData(ctx, session) require.NoError(t, err) return requestID }, @@ -264,16 +345,16 @@ func TestCompleteBrowserMFAChallenge(t *testing.T) { requestID := uuid.NewString() redirectURL := "http://127.0.0.1:62972/callback?secret_key=" + secretKey.String() session := &services.SSOMFASessionData{ - RequestID: requestID, - Username: username, - ClientRedirectURL: redirectURL, - ConnectorID: "test-connector", - ConnectorType: "test", + RequestID: requestID, + Username: username, + TSHRedirectURL: redirectURL, + ConnectorID: "test-connector", + ConnectorType: "test", ChallengeExtensions: &mfatypes.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, }, } - err := a.UpsertSSOMFASessionData(ctx, session) + err := a.UpsertMFASessionData(ctx, session) require.NoError(t, err) return requestID }, @@ -388,7 +469,7 @@ func TestCreateAuthenticateChallenge_BrowserMFARequestID(t *testing.T) { UserVerificationRequirement: "required", }, } - err := a.UpsertSSOMFASessionData(ctx, session) + err := a.UpsertMFASessionData(ctx, session) require.NoError(t, err) }, request: &proto.CreateAuthenticateChallengeRequest{ @@ -411,7 +492,7 @@ func TestCreateAuthenticateChallenge_BrowserMFARequestID(t *testing.T) { ConnectorType: "Browser", ChallengeExtensions: nil, } - err := a.UpsertSSOMFASessionData(ctx, session) + err := a.UpsertMFASessionData(ctx, session) require.NoError(t, err) }, request: &proto.CreateAuthenticateChallengeRequest{ @@ -427,6 +508,7 @@ func TestCreateAuthenticateChallenge_BrowserMFARequestID(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + if tt.setup != nil { tt.setup(t) } @@ -453,3 +535,207 @@ func TestCreateAuthenticateChallenge_BrowserMFARequestID(t *testing.T) { }) } } + +func TestBrowserMFAChallengeCreation(t *testing.T) { + t.Parallel() + ctx := t.Context() + + env := newBrowserMFATestEnv(t) + a := env.auth + + // Create a standard user without MFA devices. + standardUser, _, err := authtest.CreateUserAndRole(a, "standard", []string{"role"}, nil) + require.NoError(t, err) + + // Create a fake SAML user with SSO MFA enabled who shouldn't get Browser MFA challenge + // because they don't have webauthn + samlUser, samlRole, err := authtest.CreateUserAndRole(a, "saml-user", []string{"role"}, nil) + require.NoError(t, err) + + // Create a fake SAML user with SSO MFA enabled and a webauthn device, who will get Browser MFA + samlUserWithWebauthn, samlWebauthnRole, err := authtest.CreateUserAndRole(a, "saml-webauthn-user", []string{"role"}, nil) + require.NoError(t, err) + err = a.UpsertMFADevice(ctx, samlUserWithWebauthn.GetName(), env.webauthnDev) + require.NoError(t, err) + + samlConnector, err := types.NewSAMLConnector("saml", types.SAMLConnectorSpecV2{ + AssertionConsumerService: "http://localhost:65535/acs", + Issuer: "test", + SSO: "https://localhost:65535/sso", + AttributesToRoles: []types.AttributeMapping{ + {Name: "groups", Value: "admin", Roles: []string{samlRole.GetName(), samlWebauthnRole.GetName()}}, + }, + MFASettings: &types.SAMLConnectorMFASettings{ + Enabled: true, + Issuer: "test", + Sso: "https://localhost:65535/sso", + }, + }) + require.NoError(t, err) + _, err = a.UpsertSAMLConnector(ctx, samlConnector) + require.NoError(t, err) + + loginExt := &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + } + + for _, tt := range []struct { + name string + username string + setup func(t *testing.T) + challengeRequest *proto.CreateAuthenticateChallengeRequest + checkError func(t *testing.T, err error) + assertChallenge func(t *testing.T, chal *proto.MFAAuthenticateChallenge) + }{ + { + name: "NOK user without WebAuthn devices", + username: standardUser.GetName(), + challengeRequest: &proto.CreateAuthenticateChallengeRequest{ + ChallengeExtensions: loginExt, + BrowserMFATSHRedirectURL: browserMFARedirectURL, + }, + assertChallenge: func(t *testing.T, chal *proto.MFAAuthenticateChallenge) { + assert.Nil(t, chal.BrowserMFAChallenge, "should not return Browser MFA challenge for user without WebAuthn devices") + }, + }, + { + name: "NOK BrowserMFATSHRedirectURL not provided", + username: env.webauthnUser.GetName(), + challengeRequest: &proto.CreateAuthenticateChallengeRequest{ + ChallengeExtensions: loginExt, + BrowserMFATSHRedirectURL: "", + }, + assertChallenge: func(t *testing.T, chal *proto.MFAAuthenticateChallenge) { + assert.Nil(t, chal.BrowserMFAChallenge, "should not return Browser MFA challenge when BrowserMFATSHRedirectURL is empty") + }, + }, + { + name: "NOK Browser authentication disabled by auth preference", + username: env.webauthnUser.GetName(), + challengeRequest: &proto.CreateAuthenticateChallengeRequest{ + ChallengeExtensions: loginExt, + BrowserMFATSHRedirectURL: browserMFARedirectURL, + }, + setup: func(t *testing.T) { + // Disable Browser MFA + env.authPref.SetAllowCLIAuthViaBrowser(false) + _, err = a.UpsertAuthPreference(ctx, env.authPref) + require.NoError(t, err) + t.Cleanup(func() { + env.authPref.SetAllowCLIAuthViaBrowser(true) + _, err = a.UpsertAuthPreference(ctx, env.authPref) + assert.NoError(t, err) + }) + }, + assertChallenge: func(t *testing.T, chal *proto.MFAAuthenticateChallenge) { + assert.Nil(t, chal.BrowserMFAChallenge, "should not return Browser MFA challenge when AllowCLIAuthViaBrowser is false") + }, + }, + { + name: "NOK SSO MFA user without webauthn should not get Browser MFA", + username: samlUser.GetName(), + challengeRequest: &proto.CreateAuthenticateChallengeRequest{ + ChallengeExtensions: loginExt, + BrowserMFATSHRedirectURL: browserMFARedirectURL, + }, + assertChallenge: func(t *testing.T, chal *proto.MFAAuthenticateChallenge) { + assert.Nil(t, chal.BrowserMFAChallenge, "SSO MFA users should not get Browser MFA challenge when webauthn not available") + }, + }, + { + name: "OK SSO MFA user gets Browser MFA when webauthn available", + username: samlUserWithWebauthn.GetName(), + challengeRequest: &proto.CreateAuthenticateChallengeRequest{ + ChallengeExtensions: loginExt, + BrowserMFATSHRedirectURL: browserMFARedirectURL, + }, + assertChallenge: func(t *testing.T, chal *proto.MFAAuthenticateChallenge) { + assert.NotNil(t, chal.BrowserMFAChallenge, "expected Browser MFA challenge to be returned") + assert.NotEmpty(t, chal.BrowserMFAChallenge.RequestId, "request ID should be generated") + + sd, err := a.GetSSOMFASessionData(ctx, chal.BrowserMFAChallenge.RequestId) + require.NoError(t, err) + assert.Equal(t, &services.MFASessionData{ + RequestID: chal.BrowserMFAChallenge.RequestId, + Username: samlUserWithWebauthn.GetName(), + ConnectorID: constants.BrowserMFA, + ConnectorType: constants.BrowserMFA, + TSHRedirectURL: browserMFARedirectURL, + ChallengeExtensions: &mfatypes.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }, + Payload: &mfatypes.SessionIdentifyingPayload{}, + }, sd) + }, + }, + { + name: "OK WebAuthn user gets Browser MFA challenge", + username: env.webauthnUser.GetName(), + challengeRequest: &proto.CreateAuthenticateChallengeRequest{ + ChallengeExtensions: loginExt, + BrowserMFATSHRedirectURL: browserMFARedirectURL, + }, + assertChallenge: func(t *testing.T, chal *proto.MFAAuthenticateChallenge) { + require.NotNil(t, chal.BrowserMFAChallenge, "expected Browser MFA challenge to be returned") + assert.NotEmpty(t, chal.BrowserMFAChallenge.RequestId, "request ID should be generated") + + // Find SSO MFA session data tied to the challenge. + // Browser MFA reuses the SSO MFA session data storage. + sd, err := a.GetSSOMFASessionData(ctx, chal.BrowserMFAChallenge.RequestId) + require.NoError(t, err) + assert.Equal(t, &services.MFASessionData{ + RequestID: chal.BrowserMFAChallenge.RequestId, + Username: env.webauthnUser.GetName(), + ConnectorID: constants.BrowserMFA, + ConnectorType: constants.BrowserMFA, + TSHRedirectURL: browserMFARedirectURL, + ChallengeExtensions: &mfatypes.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }, + Payload: &mfatypes.SessionIdentifyingPayload{}, + }, sd) + }, + }, + { + name: "OK allow reuse", + username: env.webauthnUser.GetName(), + challengeRequest: &proto.CreateAuthenticateChallengeRequest{ + ChallengeExtensions: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_USER_SESSION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + BrowserMFATSHRedirectURL: browserMFARedirectURL, + }, + assertChallenge: func(t *testing.T, chal *proto.MFAAuthenticateChallenge) { + require.NotNil(t, chal.BrowserMFAChallenge, "expected Browser MFA challenge to be returned") + + // We should find SSO MFA session data tied to the challenge by request ID. + sd, err := a.GetSSOMFASessionData(ctx, chal.BrowserMFAChallenge.RequestId) + require.NoError(t, err) + assert.Equal(t, mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, sd.ChallengeExtensions.AllowReuse) + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + userClient, err := env.server.NewClient(authtest.TestUser(tt.username)) + require.NoError(t, err) + + if tt.setup != nil { + tt.setup(t) + } + chal, err := userClient.CreateAuthenticateChallenge(ctx, tt.challengeRequest) + + if tt.checkError != nil { + require.Error(t, err) + tt.checkError(t, err) + return + } + + require.NoError(t, err) + require.NotNil(t, chal) + if tt.assertChallenge != nil { + tt.assertChallenge(t, chal) + } + }) + } +} diff --git a/lib/auth/grpcserver.go b/lib/auth/grpcserver.go index 0ccc2cc39ab7c..1e95f5daec05d 100644 --- a/lib/auth/grpcserver.go +++ b/lib/auth/grpcserver.go @@ -2588,7 +2588,14 @@ func (g *GRPCServer) DeleteRole(ctx context.Context, req *authpb.DeleteRoleReque func doMFAPresenceChallenge(ctx context.Context, actx *grpcContext, stream authpb.AuthService_MaintainSessionPresenceServer, challengeReq *authpb.PresenceMFAChallengeRequest) error { user := actx.User.GetName() chalExt := &mfav1pb.ChallengeExtensions{Scope: mfav1pb.ChallengeScope_CHALLENGE_SCOPE_USER_SESSION} - authChallenge, err := actx.authServer.mfaAuthChallenge(ctx, user, challengeReq.SSOClientRedirectURL, challengeReq.ProxyAddress, chalExt) + + // Both SSO and Browser MFA redirect URLs point to the same callback server on tsh. + // So we can take either one and generate an auth challenge with it. + clientRedirectURL := challengeReq.SSOClientRedirectURL + if clientRedirectURL == "" { + clientRedirectURL = challengeReq.BrowserMFATSHRedirectURL + } + authChallenge, err := actx.authServer.mfaAuthChallenge(ctx, user, clientRedirectURL, challengeReq.ProxyAddress, chalExt) if err != nil { return trace.Wrap(err) } diff --git a/lib/auth/mfatypes/browser.go b/lib/auth/mfatypes/browser.go new file mode 100644 index 0000000000000..93b93d2244919 --- /dev/null +++ b/lib/auth/mfatypes/browser.go @@ -0,0 +1,35 @@ +// 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" +) + +// BeginBrowserMFAChallengeParams contains parameters for lib/auth/Server.BeginBrowserMFAChallenge. This struct is in this +// package in order to avoid a circular dependency between lib/auth and lib/auth/mfa/mfav1. +// TODO(cthach)/TODO(danielashare): Move params struct back to lib/auth package after SSO MFA device support is added to lib/auth/authtest +// (https://github.com/gravitational/teleport/issues/62271) and the lib/auth/mfa/mfav1.AuthServer interface is updated. +type BeginBrowserMFAChallengeParams struct { + User string + BrowserMFATSHRedirectURL string + ProxyAddress string + Ext *mfav1.ChallengeExtensions + SIP *mfav1.SessionIdentifyingPayload + SourceCluster string + TargetCluster string +} diff --git a/lib/auth/sso_mfa.go b/lib/auth/sso_mfa.go index 58b92e4c3d6ee..80bf040d7e836 100644 --- a/lib/auth/sso_mfa.go +++ b/lib/auth/sso_mfa.go @@ -73,7 +73,16 @@ func (a *Server) BeginSSOMFAChallenge(ctx context.Context, params mfatypes.Begin return nil, trace.BadParameter("unsupported sso connector type %v", params.SSO.ConnectorType) } - if err := a.upsertSSOMFASession(ctx, params.User, chal.RequestId, params.SSO.ConnectorId, params.SSO.ConnectorType, params.Ext, params.SIP, params.SourceCluster, params.TargetCluster); err != nil { + if err := a.upsertMFASession(ctx, upsertMFASessionParams{ + user: params.User, + sessionID: chal.RequestId, + connectorID: params.SSO.ConnectorId, + connectorType: params.SSO.ConnectorType, + ext: params.Ext, + sip: params.SIP, + sourceCluster: params.SourceCluster, + targetCluster: params.TargetCluster, + }); err != nil { return nil, trace.Wrap(err) } @@ -147,47 +156,62 @@ func (a *Server) VerifySSOMFASession(ctx context.Context, username, sessionID, t }, nil } -// upsertSSOMFASession upserts a new unverified SSO MFA session for the given username, -// sessionID, connector details, and challenge extensions. -func (a *Server) upsertSSOMFASession(ctx context.Context, user string, sessionID string, connectorID string, connectorType string, ext *mfav1.ChallengeExtensions, sip *mfav1.SessionIdentifyingPayload, sourceCluster string, targetCluster string) error { - data := &services.SSOMFASessionData{ - Username: user, - RequestID: sessionID, - ConnectorID: connectorID, - ConnectorType: connectorType, +// upsertMFASessionParams are the parameters for upsertMFASession. +type upsertMFASessionParams struct { + user string + sessionID string + connectorID string + connectorType string + tshRedirectURL string + ext *mfav1.ChallengeExtensions + sip *mfav1.SessionIdentifyingPayload + sourceCluster string + targetCluster string +} + +// upsertMFASession upserts a new unverified MFA session for the given username, +// sessionID, connector details, and challenge extensions. This is used by both +// SSO MFA and Browser MFA. +func (a *Server) upsertMFASession(ctx context.Context, params upsertMFASessionParams) error { + data := &services.MFASessionData{ + Username: params.user, + RequestID: params.sessionID, + ConnectorID: params.connectorID, + ConnectorType: params.connectorType, + TSHRedirectURL: params.tshRedirectURL, ChallengeExtensions: &mfatypes.ChallengeExtensions{ - Scope: ext.Scope, - AllowReuse: ext.AllowReuse, + Scope: params.ext.Scope, + AllowReuse: params.ext.AllowReuse, }, - SourceCluster: sourceCluster, - TargetCluster: targetCluster, + SourceCluster: params.sourceCluster, + TargetCluster: params.targetCluster, } - if sip != nil { + if params.sip != nil { data.Payload = &mfatypes.SessionIdentifyingPayload{ - SSHSessionID: sip.GetSshSessionId(), + SSHSessionID: params.sip.GetSshSessionId(), } } - return trace.Wrap(a.UpsertSSOMFASessionData(ctx, data)) + return trace.Wrap(a.UpsertMFASessionData(ctx, data)) } -// UpsertSSOMFASessionWithToken upserts the given SSO MFA session with a random mfa token. -func (a *Server) UpsertSSOMFASessionWithToken(ctx context.Context, sd *services.SSOMFASessionData) (token string, err error) { +// UpsertMFASessionWithToken upserts the given SSO MFA session with a random mfa token. +func (a *Server) UpsertMFASessionWithToken(ctx context.Context, sd *services.MFASessionData) (token string, err error) { sd.Token, err = utils.CryptoRandomHex(defaults.TokenLenBytes) if err != nil { return "", trace.Wrap(err) } - if err := a.UpsertSSOMFASessionData(ctx, sd); err != nil { + if err := a.UpsertMFASessionData(ctx, sd); err != nil { return "", trace.Wrap(err) } return sd.Token, nil } -// GetSSOMFASession returns the SSO MFA session for the given username and sessionID. -func (a *Server) GetSSOMFASession(ctx context.Context, sessionID string) (*services.SSOMFASessionData, error) { +// GetMFASession returns the MFA session for the given username and sessionID. +func (a *Server) GetMFASession(ctx context.Context, sessionID string) (*services.MFASessionData, error) { sd, err := a.GetSSOMFASessionData(ctx, sessionID) if err != nil { return nil, trace.Wrap(err) @@ -195,3 +219,12 @@ func (a *Server) GetSSOMFASession(ctx context.Context, sessionID string) (*servi return sd, nil } + +// TODO(danielashare): Remove these wrapper functions once `e` points to the renamed versions +func (a *Server) UpsertSSOMFASessionWithToken(ctx context.Context, sd *services.MFASessionData) (token string, err error) { + return a.UpsertMFASessionWithToken(ctx, sd) +} + +func (a *Server) GetSSOMFASession(ctx context.Context, sessionID string) (*services.MFASessionData, error) { + return a.GetMFASession(ctx, sessionID) +} diff --git a/lib/auth/sso_mfa_test.go b/lib/auth/sso_mfa_test.go index 6822b706ea5a8..1820a4f770f65 100644 --- a/lib/auth/sso_mfa_test.go +++ b/lib/auth/sso_mfa_test.go @@ -276,7 +276,7 @@ func TestSSOMFAChallenge_Creation(t *testing.T) { // We should find non validated SSO MFA session data tied to the challenge by auth request ID. sd, err := a.GetSSOMFASessionData(ctx, chal.SSOChallenge.RequestId) require.NoError(t, err) - assert.Equal(t, &services.SSOMFASessionData{ + assert.Equal(t, &services.MFASessionData{ RequestID: chal.SSOChallenge.RequestId, Username: samlUser.GetName(), ConnectorID: samlConnector.GetName(), @@ -315,7 +315,7 @@ func TestSSOMFAChallenge_Creation(t *testing.T) { // We should find non validated SSO MFA session data tied to the challenge by auth request ID. sd, err := a.GetSSOMFASessionData(ctx, chal.SSOChallenge.RequestId) require.NoError(t, err) - assert.Equal(t, &services.SSOMFASessionData{ + assert.Equal(t, &services.MFASessionData{ RequestID: chal.SSOChallenge.RequestId, Username: oidcUser.GetName(), ConnectorID: oidcConnector.GetName(), @@ -455,7 +455,7 @@ func TestSSOMFAChallenge_Validation(t *testing.T) { for _, tt := range []struct { name string username string - sd *services.SSOMFASessionData + sd *services.MFASessionData ssoResponse *proto.SSOResponse requiredExtensions *mfav1.ChallengeExtensions assertValidation func(t *testing.T, mad *authz.MFAAuthData, err error) @@ -485,7 +485,7 @@ func TestSSOMFAChallenge_Validation(t *testing.T) { { name: "NOK mismatch user", username: samlUser.GetName(), - sd: &services.SSOMFASessionData{ + sd: &services.MFASessionData{ RequestID: "request1", Username: "wrong-user", ConnectorID: samlConnector.GetName(), @@ -509,7 +509,7 @@ func TestSSOMFAChallenge_Validation(t *testing.T) { { name: "NOK mismatch token", username: samlUser.GetName(), - sd: &services.SSOMFASessionData{ + sd: &services.MFASessionData{ RequestID: "request2", Username: samlUser.GetName(), ConnectorID: samlConnector.GetName(), @@ -533,7 +533,7 @@ func TestSSOMFAChallenge_Validation(t *testing.T) { { name: "NOK non validated session data", username: samlUser.GetName(), - sd: &services.SSOMFASessionData{ + sd: &services.MFASessionData{ RequestID: "request2", Username: samlUser.GetName(), ConnectorID: samlConnector.GetName(), @@ -556,7 +556,7 @@ func TestSSOMFAChallenge_Validation(t *testing.T) { { name: "NOK mismatch scope", username: samlUser.GetName(), - sd: &services.SSOMFASessionData{ + sd: &services.MFASessionData{ RequestID: "request3", Username: samlUser.GetName(), ConnectorID: samlConnector.GetName(), @@ -580,7 +580,7 @@ func TestSSOMFAChallenge_Validation(t *testing.T) { { name: "NOK reuse not allowed", username: samlUser.GetName(), - sd: &services.SSOMFASessionData{ + sd: &services.MFASessionData{ RequestID: "request4", Username: samlUser.GetName(), ConnectorID: samlConnector.GetName(), @@ -606,7 +606,7 @@ func TestSSOMFAChallenge_Validation(t *testing.T) { { name: "NOK sso mfa not enabled by auth connector", username: noMFASAMLUser.GetName(), - sd: &services.SSOMFASessionData{ + sd: &services.MFASessionData{ RequestID: "request5", Username: noMFASAMLUser.GetName(), ConnectorID: noMFASAMLConnector.GetName(), @@ -630,7 +630,7 @@ func TestSSOMFAChallenge_Validation(t *testing.T) { { name: "NOK non sso user", username: standardUser.GetName(), - sd: &services.SSOMFASessionData{ + sd: &services.MFASessionData{ RequestID: "request6", Username: standardUser.GetName(), ConnectorID: samlConnector.GetName(), @@ -654,7 +654,7 @@ func TestSSOMFAChallenge_Validation(t *testing.T) { { name: "OK sso user", username: samlUser.GetName(), - sd: &services.SSOMFASessionData{ + sd: &services.MFASessionData{ RequestID: "request7", Username: samlUser.GetName(), ConnectorID: samlConnector.GetName(), @@ -684,7 +684,7 @@ func TestSSOMFAChallenge_Validation(t *testing.T) { { name: "OK sso user allow reuse", username: samlUser.GetName(), - sd: &services.SSOMFASessionData{ + sd: &services.MFASessionData{ RequestID: "request8", Username: samlUser.GetName(), ConnectorID: samlConnector.GetName(), @@ -715,7 +715,7 @@ func TestSSOMFAChallenge_Validation(t *testing.T) { } { t.Run(tt.name, func(t *testing.T) { if tt.sd != nil { - err := a.UpsertSSOMFASessionData(ctx, tt.sd) + err := a.UpsertMFASessionData(ctx, tt.sd) require.NoError(t, err) } diff --git a/lib/client/weblogin.go b/lib/client/weblogin.go index 979290521a501..4dc979f2815da 100644 --- a/lib/client/weblogin.go +++ b/lib/client/weblogin.go @@ -112,6 +112,10 @@ type MFAChallengeRequest struct { Pass string `json:"pass"` // Passwordless explicitly requests a passwordless/usernameless challenge. Passwordless bool `json:"passwordless"` + // BrowserMFATSHRedirectURL is the redirect url that tsh provides with a + // secret key that the server will use to return a WebAuthn response to. + // Format: http://localhost:12345/callback?secret_key=X + BrowserMFATSHRedirectURL string `json:"browser_mfa_tsh_redirect_url,omitempty"` } // MFAChallengeResponse holds the response to a MFA challenge. diff --git a/lib/services/identity.go b/lib/services/identity.go index c0c448ac135a0..9e98ec05a78a7 100644 --- a/lib/services/identity.go +++ b/lib/services/identity.go @@ -268,13 +268,13 @@ type Identity interface { // GetGithubAuthRequest retrieves Github auth request by the token GetGithubAuthRequest(ctx context.Context, stateToken string) (*types.GithubAuthRequest, error) - // UpsertSSOMFASessionData creates or updates SSO MFA session data in + // UpsertMFASessionData creates or updates SSO/Browser MFA session data in // storage, for the purpose of later verifying an MFA authentication attempt. - // SSO MFA session data is expected to expire according to backend settings. - UpsertSSOMFASessionData(ctx context.Context, sd *SSOMFASessionData) error + // MFA session data is expected to expire according to backend settings. + UpsertMFASessionData(ctx context.Context, sd *MFASessionData) error // GetSSOMFASessionData retrieves SSO MFA session data by ID. - GetSSOMFASessionData(ctx context.Context, sessionID string) (*SSOMFASessionData, error) + GetSSOMFASessionData(ctx context.Context, sessionID string) (*MFASessionData, error) // DeleteSSOMFASessionData deletes SSO MFA session data by ID. DeleteSSOMFASessionData(ctx context.Context, sessionID string) error diff --git a/lib/services/local/users.go b/lib/services/local/users.go index a2fc1d0798309..9ee754c3eb022 100644 --- a/lib/services/local/users.go +++ b/lib/services/local/users.go @@ -1943,7 +1943,7 @@ func (s *IdentityService) GetSSODiagnosticInfo(ctx context.Context, authKind str return &req, nil } -func (s *IdentityService) UpsertSSOMFASessionData(ctx context.Context, sd *services.SSOMFASessionData) error { +func (s *IdentityService) UpsertMFASessionData(ctx context.Context, sd *services.MFASessionData) error { switch { case sd == nil: return trace.BadParameter("missing parameter sd") @@ -1969,7 +1969,7 @@ func (s *IdentityService) UpsertSSOMFASessionData(ctx context.Context, sd *servi return trace.Wrap(err) } -func (s *IdentityService) GetSSOMFASessionData(ctx context.Context, sessionID string) (*services.SSOMFASessionData, error) { +func (s *IdentityService) GetSSOMFASessionData(ctx context.Context, sessionID string) (*services.MFASessionData, error) { if sessionID == "" { return nil, trace.BadParameter("missing parameter sessionID") } @@ -1978,7 +1978,7 @@ func (s *IdentityService) GetSSOMFASessionData(ctx context.Context, sessionID st if err != nil { return nil, trace.Wrap(err) } - sd := &services.SSOMFASessionData{} + sd := &services.MFASessionData{} return sd, trace.Wrap(json.Unmarshal(item.Value, sd)) } diff --git a/lib/services/local/users_test.go b/lib/services/local/users_test.go index 9ece1ff5da017..ce77da0c81a71 100644 --- a/lib/services/local/users_test.go +++ b/lib/services/local/users_test.go @@ -1753,13 +1753,13 @@ func TestIdentityService_SSOMFASessionDataCRUD(t *testing.T) { identity := newIdentityService(t, clockwork.NewFakeClock()) // Verify create. - sd := &services.SSOMFASessionData{ + sd := &services.MFASessionData{ RequestID: "request", Username: "alice", ConnectorID: "saml", ConnectorType: "saml", } - err := identity.UpsertSSOMFASessionData(ctx, sd) + err := identity.UpsertMFASessionData(ctx, sd) require.NoError(t, err) // Verify read. @@ -1771,7 +1771,7 @@ func TestIdentityService_SSOMFASessionDataCRUD(t *testing.T) { // Verify update. sd.Token = "token" - err = identity.UpsertSSOMFASessionData(ctx, sd) + err = identity.UpsertMFASessionData(ctx, sd) require.NoError(t, err) got, err = identity.GetSSOMFASessionData(ctx, sd.RequestID) require.NoError(t, err) diff --git a/lib/services/sso_mfa.go b/lib/services/sso_mfa.go index b197150cbd39c..e8528690f45e4 100644 --- a/lib/services/sso_mfa.go +++ b/lib/services/sso_mfa.go @@ -20,9 +20,12 @@ package services import "github.com/gravitational/teleport/lib/auth/mfatypes" -// SSOMFASessionData SSO MFA Session data. -type SSOMFASessionData struct { - // RequestID is the ID of the corresponding SSO Auth request, which is used to +// TODO(danielashare): Remove alias once `e` no longer references it +type SSOMFASessionData = MFASessionData + +// MFASessionData is MFA Session data for SSO MFA or Browser MFA. +type MFASessionData struct { + // RequestID is the ID of the corresponding Auth request, which is used to // identity this session. RequestID string `json:"request_id,omitempty"` // Username is the Teleport username. @@ -41,6 +44,7 @@ type SSOMFASessionData struct { SourceCluster string `json:"source_cluster,omitempty"` // TargetCluster is the optional cluster where the authentication is targeted. TargetCluster string `json:"target_cluster,omitempty"` - // ClientRedirectURL is the redirect URL for the client - ClientRedirectURL string `json:"client_redirect_url,omitempty"` + // TSHRedirectURL is the redirect URL used to return a WebAuthn response back to tsh. + // This is used exclusively by Browser MFA. + TSHRedirectURL string `json:"tsh_redirect_url,omitempty"` } diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index e14ee68b49247..73f0e1b3d8aae 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -3114,6 +3114,7 @@ func (h *Handler) getResetPasswordToken(ctx context.Context, tokenID string) (an // // {"user": "alex", "pass": "abcdef123456"} // {"passwordless": true} +// {"user": "alex", "pass": "abcdef123456", "BrowserMFATSHRedirectURL": "http://localhost:12345/callback?secret_key=X"} // // Successful response: // @@ -3145,6 +3146,8 @@ func (h *Handler) mfaLoginBegin(w http.ResponseWriter, r *http.Request, p httpro mfaReq.ChallengeExtensions = &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, } + + mfaReq.BrowserMFATSHRedirectURL = req.BrowserMFATSHRedirectURL } mfaChallenge, err := h.auth.proxyClient.CreateAuthenticateChallenge(r.Context(), mfaReq)