diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 4df9937b62deb..2b842cb277a7f 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -4314,7 +4314,7 @@ func (a *Server) CreateAuthenticateChallenge(ctx context.Context, req *proto.Cre 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) + browserMFAReq, err := a.GetMFASession(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") diff --git a/lib/auth/browser_mfa.go b/lib/auth/browser_mfa.go index 4220c371af3e2..6c4279756b700 100644 --- a/lib/auth/browser_mfa.go +++ b/lib/auth/browser_mfa.go @@ -38,7 +38,7 @@ import ( func (a *Server) CompleteBrowserMFAChallenge(ctx context.Context, requestID string, webauthnResponse *webauthnpb.CredentialAssertionResponse) (string, error) { const notFoundErrMsg = "mfa session data not found" // Retrieve the MFA session - mfaSession, err := a.GetSSOMFASession(ctx, requestID) + mfaSession, err := a.GetMFASession(ctx, requestID) if trace.IsNotFound(err) { return "", trace.AccessDenied("%s", notFoundErrMsg) } else if err != nil { @@ -88,8 +88,9 @@ func (a *Server) BeginBrowserMFAChallenge(ctx context.Context, params mfatypes.B ConnectorType: constants.BrowserMFA, TSHRedirectURL: params.BrowserMFATSHRedirectURL, ChallengeExtensions: &mfatypes.ChallengeExtensions{ - Scope: params.Ext.Scope, - AllowReuse: params.Ext.AllowReuse, + Scope: params.Ext.Scope, + AllowReuse: params.Ext.AllowReuse, + UserVerificationRequirement: params.Ext.UserVerificationRequirement, }, SourceCluster: params.SourceCluster, TargetCluster: params.TargetCluster, diff --git a/lib/auth/browser_mfa_test.go b/lib/auth/browser_mfa_test.go index 2c30cad7aea87..3ccb7ed4ca5f8 100644 --- a/lib/auth/browser_mfa_test.go +++ b/lib/auth/browser_mfa_test.go @@ -284,7 +284,7 @@ func TestCompleteBrowserMFAChallenge(t *testing.T) { setupSession: func(t *testing.T) string { requestID := uuid.NewString() redirectURL := "http://127.0.0.1:62972/callback?secret_key=" + secretKey.String() - session := &services.SSOMFASessionData{ + session := &services.MFASessionData{ RequestID: requestID, Username: "other-user", // mismatch username TSHRedirectURL: redirectURL, @@ -309,7 +309,7 @@ func TestCompleteBrowserMFAChallenge(t *testing.T) { name: "NOK invalid redirect URL in session", setupSession: func(t *testing.T) string { requestID := uuid.NewString() - session := &services.SSOMFASessionData{ + session := &services.MFASessionData{ RequestID: requestID, Username: username, TSHRedirectURL: "://invalid-url", @@ -332,7 +332,7 @@ func TestCompleteBrowserMFAChallenge(t *testing.T) { setupSession: func(t *testing.T) string { requestID := uuid.NewString() redirectURL := "http://127.0.0.1:62972/callback?secret_key=" + secretKey.String() - session := &services.SSOMFASessionData{ + session := &services.MFASessionData{ RequestID: requestID, Username: username, TSHRedirectURL: redirectURL, @@ -713,6 +713,7 @@ func TestBrowserMFAChallengeCreation(t *testing.T) { if tt.setup != nil { tt.setup(t) } + chal, err := userClient.CreateAuthenticateChallenge(ctx, tt.challengeRequest) if tt.checkError != nil { @@ -729,3 +730,108 @@ func TestBrowserMFAChallengeCreation(t *testing.T) { }) } } + +func TestBrowserMFAChallenge_Validation(t *testing.T) { + t.Parallel() + ctx := t.Context() + + env := newBrowserMFATestEnv(t) + a := env.auth + + for _, tt := range []struct { + name string + sd *services.MFASessionData + requestID string + checkError func(t *testing.T, err error) + assertValidation func(t *testing.T, sd *services.MFASessionData) + }{ + { + name: "NOK session data not found", + sd: nil, + requestID: "nonexistent-request", + checkError: func(t *testing.T, err error) { + require.Error(t, err, "should fail when session data not found") + }, + }, + { + name: "OK session data retrieved correctly", + sd: &services.MFASessionData{ + RequestID: "request1", + Username: env.webauthnUser.GetName(), + ConnectorID: constants.BrowserMFA, + ConnectorType: constants.BrowserMFA, + TSHRedirectURL: browserMFARedirectURL, + ChallengeExtensions: &mfatypes.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }, + }, + requestID: "request1", + assertValidation: func(t *testing.T, sd *services.MFASessionData) { + require.NotNil(t, sd) + assert.Equal(t, "request1", sd.RequestID) + assert.Equal(t, env.webauthnUser.GetName(), sd.Username) + assert.Equal(t, constants.BrowserMFA, sd.ConnectorID) + assert.Equal(t, constants.BrowserMFA, sd.ConnectorType) + assert.Equal(t, browserMFARedirectURL, sd.TSHRedirectURL) + assert.Equal(t, mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, sd.ChallengeExtensions.Scope) + }, + }, + { + name: "OK session data with allow reuse", + sd: &services.MFASessionData{ + RequestID: "request2", + Username: env.webauthnUser.GetName(), + ConnectorID: constants.BrowserMFA, + ConnectorType: constants.BrowserMFA, + TSHRedirectURL: browserMFARedirectURL, + ChallengeExtensions: &mfatypes.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_USER_SESSION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + }, + requestID: "request2", + assertValidation: func(t *testing.T, sd *services.MFASessionData) { + require.NotNil(t, sd) + assert.Equal(t, mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, sd.ChallengeExtensions.AllowReuse) + }, + }, + { + name: "OK session data with admin action scope", + sd: &services.MFASessionData{ + RequestID: "request3", + Username: env.webauthnUser.GetName(), + ConnectorID: constants.BrowserMFA, + ConnectorType: constants.BrowserMFA, + TSHRedirectURL: browserMFARedirectURL, + ChallengeExtensions: &mfatypes.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + }, + requestID: "request3", + assertValidation: func(t *testing.T, sd *services.MFASessionData) { + require.NotNil(t, sd) + assert.Equal(t, mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, sd.ChallengeExtensions.Scope) + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + if tt.sd != nil { + err := a.UpsertMFASessionData(ctx, tt.sd) + require.NoError(t, err) + } + + sd, err := a.GetSSOMFASessionData(ctx, tt.requestID) + + if tt.checkError != nil { + require.Error(t, err) + tt.checkError(t, err) + return + } + + require.NoError(t, err) + if tt.assertValidation != nil { + tt.assertValidation(t, sd) + } + }) + } +} diff --git a/lib/client/mfa/cli.go b/lib/client/mfa/cli.go index 3c8a8a384f7f0..4c2fedcdf303b 100644 --- a/lib/client/mfa/cli.go +++ b/lib/client/mfa/cli.go @@ -240,12 +240,12 @@ func (c *CLIPrompt) Run(ctx context.Context, chal *proto.MFAAuthenticateChalleng slog.DebugContext(ctx, "Disabling SSO MFA: SSO MFA ceremony not available (this is likely a bug)") } - if state.promptBrowser && (!c.cfg.WebauthnSupported || c.cfg.MFACeremony == nil) { + if state.promptBrowser && (chal.WebauthnChallenge == nil || c.cfg.MFACeremony == nil) { state.promptBrowser = false slog.DebugContext( ctx, - "Disabling Browser MFA: cluster needs to support Webauthn and client needs to support SSO MFA Ceremony", - "webauthn_supported", c.cfg.WebauthnSupported, + "Disabling Browser MFA: user needs at least one webauthn device and client needs to support SSO MFA Ceremony", + "webauthn_available", chal.WebauthnChallenge != nil, "mfa_ceremony_available (if false, this is a bug)", c.cfg.MFACeremony != nil, ) } diff --git a/lib/client/sso/ceremony.go b/lib/client/sso/ceremony.go index 0e03a51e397ca..f405069f7fe65 100644 --- a/lib/client/sso/ceremony.go +++ b/lib/client/sso/ceremony.go @@ -25,7 +25,9 @@ import ( "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/mfa" + apiutils "github.com/gravitational/teleport/api/utils" "github.com/gravitational/teleport/lib/auth/authclient" + wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" ) // Ceremony is a customizable SSO login ceremony. @@ -107,6 +109,7 @@ type MFACeremony struct { ProxyAddress string HandleRedirect func(ctx context.Context, redirectURL string) error GetCallbackMFAToken func(ctx context.Context) (string, error) + GetCallbackWebauthn func(ctx context.Context) (*wantypes.CredentialAssertionResponse, error) } // GetClientCallbackURL returns the client callback URL. @@ -119,30 +122,60 @@ func (m *MFACeremony) GetProxyAddress() string { return m.ProxyAddress } -// Run the SSO MFA ceremony. +// Run the SSO/Browser MFA ceremony. func (m *MFACeremony) Run(ctx context.Context, chal *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) { - // TODO(danielashare): Remove when Browser MFA challenge handling is implemented - if chal.SSOChallenge == nil { - return nil, trace.BadParameter("no SSO challenge provided") - } + // The proxy will only ever return one of SSO or Browser challenge. However, + // check for SSO challenge first as it takes priority over browser MFA. + switch { + case chal.SSOChallenge != nil: + if err := m.HandleRedirect(ctx, chal.SSOChallenge.RedirectUrl); err != nil { + return nil, trace.Wrap(err) + } - if err := m.HandleRedirect(ctx, chal.SSOChallenge.RedirectUrl); err != nil { - return nil, trace.Wrap(err) - } + mfaToken, err := m.GetCallbackMFAToken(ctx) + if err != nil { + return nil, trace.Wrap(err) + } - mfaToken, err := m.GetCallbackMFAToken(ctx) - if err != nil { - return nil, trace.Wrap(err) - } + return &proto.MFAAuthenticateResponse{ + Response: &proto.MFAAuthenticateResponse_SSO{ + SSO: &proto.SSOResponse{ + RequestId: chal.SSOChallenge.RequestId, + Token: mfaToken, + }, + }, + }, nil + case chal.BrowserMFAChallenge != nil: + redirectURL, err := apiutils.ParseURL(m.ProxyAddress) + if err != nil { + return nil, trace.Wrap(err) + } + if redirectURL == nil { + return nil, trace.BadParameter("proxy address is required for browser MFA") + } + redirectURL.Scheme = "https" + redirectURL.Path = WebBrowserMFAPath + chal.BrowserMFAChallenge.RequestId - return &proto.MFAAuthenticateResponse{ - Response: &proto.MFAAuthenticateResponse_SSO{ - SSO: &proto.SSOResponse{ - RequestId: chal.SSOChallenge.RequestId, - Token: mfaToken, + if err := m.HandleRedirect(ctx, redirectURL.String()); err != nil { + return nil, trace.Wrap(err) + } + + webauthnResp, err := m.GetCallbackWebauthn(ctx) + if err != nil { + return nil, trace.Wrap(err) + } + + return &proto.MFAAuthenticateResponse{ + Response: &proto.MFAAuthenticateResponse_Browser{ + Browser: &proto.BrowserMFAResponse{ + RequestId: chal.BrowserMFAChallenge.RequestId, + WebauthnResponse: wantypes.CredentialAssertionResponseToProto(webauthnResp), + }, }, - }, - }, nil + }, nil + default: + return nil, trace.BadParameter("no SSO or Browser challenge provided") + } } // Close closes resources associated with the SSO MFA ceremony. @@ -172,6 +205,18 @@ func NewCLIMFACeremony(rd *Redirector) *MFACeremony { return loginResp.MFAToken, nil }, + GetCallbackWebauthn: func(ctx context.Context) (*wantypes.CredentialAssertionResponse, error) { + loginResp, err := rd.WaitForResponse(ctx) + if err != nil { + return nil, trace.Wrap(err) + } + + if loginResp.BrowserMFAWebauthnResponse == nil { + return nil, trace.BadParameter("login response for Browser MFA flow missing WebAuthn response") + } + + return loginResp.BrowserMFAWebauthnResponse, nil + }, } } @@ -197,5 +242,17 @@ func NewConnectMFACeremony(rd *Redirector) mfa.CallbackCeremony { return loginResp.MFAToken, nil }, + GetCallbackWebauthn: func(ctx context.Context) (*wantypes.CredentialAssertionResponse, error) { + loginResp, err := rd.WaitForResponse(ctx) + if err != nil { + return nil, trace.Wrap(err) + } + + if loginResp.BrowserMFAWebauthnResponse == nil { + return nil, trace.BadParameter("login response for Browser MFA flow missing WebAuthn response") + } + + return loginResp.BrowserMFAWebauthnResponse, nil + }, } } diff --git a/lib/client/sso/ceremony_test.go b/lib/client/sso/ceremony_test.go index 6f0a7289d1482..0309ad1f9dafb 100644 --- a/lib/client/sso/ceremony_test.go +++ b/lib/client/sso/ceremony_test.go @@ -231,7 +231,7 @@ const postform = ` func TestCLICeremony_MFA(t *testing.T) { const token = "sso-mfa-token" - const requestID = "soo-mfa-request-id" + const requestID = "sso-mfa-request-id" ctx := context.Background() mockProxy := newMockProxy(t) @@ -297,3 +297,7 @@ func TestCLICeremony_MFA(t *testing.T) { assert.Equal(t, token, mfaResponse.GetSSO().Token) assert.Equal(t, requestID, mfaResponse.GetSSO().RequestId) } + +// TODO(danielashare): Add full Browser MFA ceremony test once server-side +// Browser MFA functions have been merged. Test similar to above that tests +// CLI output, redirect handling, and MFA response. diff --git a/lib/client/weblogin.go b/lib/client/weblogin.go index feed289b27dbc..672394ee544da 100644 --- a/lib/client/weblogin.go +++ b/lib/client/weblogin.go @@ -136,6 +136,12 @@ type SSOResponse struct { Token string `json:"token,omitempty"` } +// BrowserMFAResponse is a json compatible [proto.BrowserMFAResponse]. +type BrowserMFAResponse struct { + RequestID string `json:"requestId,omitempty"` + WebauthnResponse *wantypes.CredentialAssertionResponse `json:"webauthnResponse,omitempty"` +} + // GetOptionalMFAResponseProtoReq converts response to a type proto.MFAAuthenticateResponse, // if there were any responses set. Otherwise returns nil. func (r *MFAChallengeResponse) GetOptionalMFAResponseProtoReq() (*proto.MFAAuthenticateResponse, error) { @@ -261,6 +267,9 @@ type AuthenticateSSHUserRequest struct { WebauthnChallengeResponse *wantypes.CredentialAssertionResponse `json:"webauthn_challenge_response"` // TOTPCode is a code from the TOTP device. TOTPCode string `json:"totp_code"` + // BrowserMFAResponse is a response from a Browser MFA flow containing a + // webauthn response. + BrowserMFAResponse *BrowserMFAResponse `json:"browser_response,omitempty"` // UserPublicKeys is embedded and holds user SSH and TLS public keys that // should be used as the subject of issued certificates, and optional // hardware key attestation statements for each key. @@ -470,6 +479,13 @@ func BrowserChallengeToProto(browserChal *BrowserMFAChallenge) *proto.BrowserMFA } } +// BrowserChallengeFromProto converts a BrowserChallenge to json compatible format +func BrowserChallengeFromProto(browserChal *proto.BrowserMFAChallenge) *BrowserMFAChallenge { + return &BrowserMFAChallenge{ + RequestID: browserChal.RequestId, + } +} + // initClient creates a new client to the HTTPS web proxy. func initClient(proxyAddr string, insecure bool, pool *x509.CertPool, extraHeaders map[string]string, opts ...roundtrip.ClientParam) (*WebClient, *url.URL, error) { log := slog.With(teleport.ComponentKey, teleport.ComponentClient) @@ -685,6 +701,11 @@ func SSHAgentMFALogin(ctx context.Context, login SSHLoginMFA) (*authclient.CLILo challengeResp.TOTPCode = r.TOTP.Code case *proto.MFAAuthenticateResponse_Webauthn: challengeResp.WebauthnChallengeResponse = wantypes.CredentialAssertionResponseFromProto(r.Webauthn) + case *proto.MFAAuthenticateResponse_Browser: + challengeResp.BrowserMFAResponse = &BrowserMFAResponse{ + RequestID: r.Browser.RequestId, + WebauthnResponse: wantypes.CredentialAssertionResponseFromProto(r.Browser.WebauthnResponse), + } default: // No challenge was sent, so we send back just username/password. } @@ -726,6 +747,9 @@ func newMFALoginCeremony(clt *WebClient, login SSHLoginMFA) *mfa.Ceremony { if challenge.WebauthnChallenge != nil { chal.WebauthnChallenge = wantypes.CredentialAssertionToProto(challenge.WebauthnChallenge) } + if challenge.BrowserMFAChallenge != nil { + chal.BrowserMFAChallenge = BrowserChallengeToProto(challenge.BrowserMFAChallenge) + } return chal, nil }, PromptConstructor: login.MFAPromptConstructor, diff --git a/lib/web/mfa.go b/lib/web/mfa.go index c4213e7dab836..09bf06bacf75e 100644 --- a/lib/web/mfa.go +++ b/lib/web/mfa.go @@ -556,5 +556,8 @@ func makeAuthenticateChallenge(protoChal *proto.MFAAuthenticateChallenge, ssoCha chal.SSOChallenge = client.SSOChallengeFromProto(protoChal.GetSSOChallenge()) chal.SSOChallenge.ChannelID = ssoChannelID } + if protoChal.GetBrowserMFAChallenge() != nil { + chal.BrowserMFAChallenge = client.BrowserChallengeFromProto(protoChal.GetBrowserMFAChallenge()) + } return chal }