diff --git a/lib/client/weblogin.go b/lib/client/weblogin.go index 7edf946c0e39f..c3415e340417d 100644 --- a/lib/client/weblogin.go +++ b/lib/client/weblogin.go @@ -113,6 +113,8 @@ type MFAChallengeResponse struct { WebauthnResponse *wantypes.CredentialAssertionResponse `json:"webauthn_response,omitempty"` // SSOResponse is a response from an SSO MFA flow. SSOResponse *SSOResponse `json:"sso_response"` + // TODO(Joerger): DELETE IN v19.0.0, WebauthnResponse used instead. + WebauthnAssertionResponse *wantypes.CredentialAssertionResponse `json:"webauthnAssertionResponse"` } // SSOResponse is a json compatible [proto.SSOResponse]. @@ -124,25 +126,57 @@ type SSOResponse struct { // GetOptionalMFAResponseProtoReq converts response to a type proto.MFAAuthenticateResponse, // if there were any responses set. Otherwise returns nil. func (r *MFAChallengeResponse) GetOptionalMFAResponseProtoReq() (*proto.MFAAuthenticateResponse, error) { - if r.TOTPCode != "" && r.WebauthnResponse != nil { + var availableResponses int + if r.TOTPCode != "" { + availableResponses++ + } + if r.WebauthnResponse != nil { + availableResponses++ + } + if r.SSOResponse != nil { + availableResponses++ + } + + if availableResponses > 1 { return nil, trace.BadParameter("only one MFA response field can be set") } - if r.TOTPCode != "" { + switch { + case r.WebauthnResponse != nil: + return &proto.MFAAuthenticateResponse{Response: &proto.MFAAuthenticateResponse_Webauthn{ + Webauthn: wantypes.CredentialAssertionResponseToProto(r.WebauthnResponse), + }}, nil + case r.SSOResponse != nil: + return &proto.MFAAuthenticateResponse{Response: &proto.MFAAuthenticateResponse_SSO{ + SSO: &proto.SSOResponse{ + RequestId: r.SSOResponse.RequestID, + Token: r.SSOResponse.Token, + }, + }}, nil + case r.TOTPCode != "": return &proto.MFAAuthenticateResponse{Response: &proto.MFAAuthenticateResponse_TOTP{ TOTP: &proto.TOTPResponse{Code: r.TOTPCode}, }}, nil - } - - if r.WebauthnResponse != nil { + case r.WebauthnAssertionResponse != nil: return &proto.MFAAuthenticateResponse{Response: &proto.MFAAuthenticateResponse_Webauthn{ - Webauthn: wantypes.CredentialAssertionResponseToProto(r.WebauthnResponse), + Webauthn: wantypes.CredentialAssertionResponseToProto(r.WebauthnAssertionResponse), }}, nil } return nil, nil } +// ParseMFAChallengeResponse parses [MFAChallengeResponse] from JSON and returns it as a [proto.MFAAuthenticateResponse]. +func ParseMFAChallengeResponse(mfaResponseJSON []byte) (*proto.MFAAuthenticateResponse, error) { + var resp MFAChallengeResponse + if err := json.Unmarshal(mfaResponseJSON, &resp); err != nil { + return nil, trace.Wrap(err) + } + + protoResp, err := resp.GetOptionalMFAResponseProtoReq() + return protoResp, trace.Wrap(err) +} + // CreateSSHCertReq is passed by tsh to authenticate a local user without MFA // and receive short-lived certificates. type CreateSSHCertReq struct { diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index a81932f586de4..451f5668a14a2 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -2766,7 +2766,7 @@ func (h *Handler) mfaLoginBegin(w http.ResponseWriter, r *http.Request, p httpro return nil, trace.AccessDenied("invalid credentials") } - return makeAuthenticateChallenge(mfaChallenge), nil + return makeAuthenticateChallenge(mfaChallenge, "" /*channelID*/), nil } // mfaLoginFinish completes the MFA login ceremony, returning a new SSH @@ -4857,16 +4857,12 @@ func parseMFAResponseFromRequest(r *http.Request) error { // context and returned. func contextWithMFAResponseFromRequestHeader(ctx context.Context, requestHeader http.Header) (context.Context, error) { if mfaResponseJSON := requestHeader.Get("Teleport-MFA-Response"); mfaResponseJSON != "" { - var resp mfaResponse - if err := json.Unmarshal([]byte(mfaResponseJSON), &resp); err != nil { + mfaResp, err := client.ParseMFAChallengeResponse([]byte(mfaResponseJSON)) + if err != nil { return nil, trace.Wrap(err) } - return mfa.ContextWithMFAResponse(ctx, &proto.MFAAuthenticateResponse{ - Response: &proto.MFAAuthenticateResponse_Webauthn{ - Webauthn: wantypes.CredentialAssertionResponseToProto(resp.WebauthnAssertionResponse), - }, - }), nil + return mfa.ContextWithMFAResponse(ctx, mfaResp), nil } return ctx, nil diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index f3a71ebb78945..2816f2f92b7bb 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -5573,10 +5573,6 @@ func TestCreateAppSession_RequireSessionMFA(t *testing.T) { require.NoError(t, err) mfaResp, err := webauthnDev.SolveAuthn(chal) require.NoError(t, err) - mfaRespJSON, err := json.Marshal(mfaResponse{ - WebauthnAssertionResponse: wantypes.CredentialAssertionResponseFromProto(mfaResp.GetWebauthn()), - }) - require.NoError(t, err) // Extract the session ID and bearer token for the current session. rawCookie := *pack.cookies[0] @@ -5610,7 +5606,9 @@ func TestCreateAppSession_RequireSessionMFA(t *testing.T) { PublicAddr: "panel.example.com", ClusterName: "localhost", }, - MFAResponse: string(mfaRespJSON), + MFAResponse: client.MFAChallengeResponse{ + WebauthnAssertionResponse: wantypes.CredentialAssertionResponseFromProto(mfaResp.GetWebauthn()), + }, }, expectMFAVerified: true, }, diff --git a/lib/web/apps.go b/lib/web/apps.go index 8ae0dc5525468..0facc0436d03a 100644 --- a/lib/web/apps.go +++ b/lib/web/apps.go @@ -22,7 +22,6 @@ package web import ( "context" - "encoding/json" "net/http" "sort" @@ -33,7 +32,7 @@ import ( "github.com/gravitational/teleport/api/client/proto" apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" - wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" + "github.com/gravitational/teleport/lib/client" "github.com/gravitational/teleport/lib/httplib" "github.com/gravitational/teleport/lib/reversetunnelclient" "github.com/gravitational/teleport/lib/utils" @@ -191,7 +190,10 @@ type CreateAppSessionRequest struct { // AWSRole is the AWS role ARN when accessing AWS management console. AWSRole string `json:"arn,omitempty"` // MFAResponse is an optional MFA response used to create an MFA verified app session. - MFAResponse string `json:"mfa_response"` + MFAResponse client.MFAChallengeResponse `json:"mfaResponse"` + // TODO(Joerger): DELETE IN v19.0.0 + // Backwards compatible version of MFAResponse + MFAResponseJSON string `json:"mfa_response"` } // CreateAppSessionResponse is a response to POST /v1/webapi/sessions/app @@ -230,17 +232,16 @@ func (h *Handler) createAppSession(w http.ResponseWriter, r *http.Request, p htt } } - var mfaProtoResponse *proto.MFAAuthenticateResponse - if req.MFAResponse != "" { - var resp mfaResponse - if err := json.Unmarshal([]byte(req.MFAResponse), &resp); err != nil { - return nil, trace.Wrap(err) - } + mfaResponse, err := req.MFAResponse.GetOptionalMFAResponseProtoReq() + if err != nil { + return nil, trace.Wrap(err) + } - mfaProtoResponse = &proto.MFAAuthenticateResponse{ - Response: &proto.MFAAuthenticateResponse_Webauthn{ - Webauthn: wantypes.CredentialAssertionResponseToProto(resp.WebauthnAssertionResponse), - }, + // Fallback to backwards compatible mfa response. + if mfaResponse == nil && req.MFAResponseJSON != "" { + mfaResponse, err = client.ParseMFAChallengeResponse([]byte(req.MFAResponseJSON)) + if err != nil { + return nil, trace.Wrap(err) } } @@ -263,7 +264,7 @@ func (h *Handler) createAppSession(w http.ResponseWriter, r *http.Request, p htt PublicAddr: result.App.GetPublicAddr(), ClusterName: result.ClusterName, AWSRoleARN: req.AWSRole, - MFAResponse: mfaProtoResponse, + MFAResponse: mfaResponse, AppName: result.App.GetName(), URI: result.App.GetURI(), ClientAddr: r.RemoteAddr, diff --git a/lib/web/files.go b/lib/web/files.go index 53248258dd034..1c48dbf4f745e 100644 --- a/lib/web/files.go +++ b/lib/web/files.go @@ -20,7 +20,6 @@ package web import ( "context" - "encoding/json" "errors" "net/http" "time" @@ -35,7 +34,6 @@ import ( "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/api/utils/sshutils" "github.com/gravitational/teleport/lib/auth/authclient" - wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" "github.com/gravitational/teleport/lib/client" "github.com/gravitational/teleport/lib/multiplexer" "github.com/gravitational/teleport/lib/reversetunnelclient" @@ -56,8 +54,8 @@ type fileTransferRequest struct { remoteLocation string // filename is a file name filename string - // webauthn is an optional parameter that contains a webauthn response string used to issue single use certs - webauthn string + // mfaResponse is an optional parameter that contains an mfa response string used to issue single use certs + mfaResponse string // fileTransferRequestID is used to find a FileTransferRequest on a session fileTransferRequestID string // moderatedSessonID is an ID of a moderated session that has completed a @@ -74,11 +72,25 @@ func (h *Handler) transferFile(w http.ResponseWriter, r *http.Request, p httprou remoteLocation: query.Get("location"), filename: query.Get("filename"), namespace: defaults.Namespace, - webauthn: query.Get("webauthn"), + mfaResponse: query.Get("mfaResponse"), fileTransferRequestID: query.Get("fileTransferRequestId"), moderatedSessionID: query.Get("moderatedSessionId"), } + // Check for old query parameter, uses the same data structure. + // TODO(Joerger): DELETE IN v19.0.0 + if req.mfaResponse == "" { + req.mfaResponse = query.Get("webauthn") + } + + var mfaResponse *proto.MFAAuthenticateResponse + if req.mfaResponse != "" { + var err error + if mfaResponse, err = client.ParseMFAChallengeResponse([]byte(req.mfaResponse)); err != nil { + return nil, trace.Wrap(err) + } + } + // Send an error if only one of these params has been sent. Both should exist or not exist together if (req.fileTransferRequestID != "") != (req.moderatedSessionID != "") { return nil, trace.BadParameter("fileTransferRequestId and moderatedSessionId must both be included in the same request.") @@ -107,7 +119,7 @@ func (h *Handler) transferFile(w http.ResponseWriter, r *http.Request, p httprou return nil, trace.Wrap(err) } - if mfaReq.Required && query.Get("webauthn") == "" { + if mfaReq.Required && mfaResponse == nil { return nil, trace.AccessDenied("MFA required for file transfer") } @@ -135,8 +147,8 @@ func (h *Handler) transferFile(w http.ResponseWriter, r *http.Request, p httprou return nil, trace.Wrap(err) } - if req.webauthn != "" { - err = ft.issueSingleUseCert(req.webauthn, r, tc) + if req.mfaResponse != "" { + err = ft.issueSingleUseCert(mfaResponse, r, tc) if err != nil { return nil, trace.Wrap(err) } @@ -216,21 +228,10 @@ func (f *fileTransfer) createClient(req fileTransferRequest, httpReq *http.Reque return tc, nil } -type mfaResponse struct { - // WebauthnResponse is the response from authenticators. - WebauthnAssertionResponse *wantypes.CredentialAssertionResponse `json:"webauthnAssertionResponse"` -} - // issueSingleUseCert will take an assertion response sent from a solved challenge in the web UI // and use that to generate a cert. This cert is added to the Teleport Client as an authmethod that // can be used to connect to a node. -func (f *fileTransfer) issueSingleUseCert(webauthn string, httpReq *http.Request, tc *client.TeleportClient) error { - var mfaResp mfaResponse - err := json.Unmarshal([]byte(webauthn), &mfaResp) - if err != nil { - return trace.Wrap(err) - } - +func (f *fileTransfer) issueSingleUseCert(mfaResponse *proto.MFAAuthenticateResponse, httpReq *http.Request, tc *client.TeleportClient) error { pk, err := keys.ParsePrivateKey(f.sctx.cfg.Session.GetSSHPriv()) if err != nil { return trace.Wrap(err) @@ -241,11 +242,7 @@ func (f *fileTransfer) issueSingleUseCert(webauthn string, httpReq *http.Request SSHPublicKey: pk.MarshalSSHPublicKey(), Username: f.sctx.GetUser(), Expires: time.Now().Add(time.Minute).UTC(), - MFAResponse: &proto.MFAAuthenticateResponse{ - Response: &proto.MFAAuthenticateResponse_Webauthn{ - Webauthn: wantypes.CredentialAssertionResponseToProto(mfaResp.WebauthnAssertionResponse), - }, - }, + MFAResponse: mfaResponse, }) if err != nil { return trace.Wrap(err) diff --git a/lib/web/mfa.go b/lib/web/mfa.go index 485a4eff460bc..c59b0ae10cbd7 100644 --- a/lib/web/mfa.go +++ b/lib/web/mfa.go @@ -21,8 +21,10 @@ package web import ( "context" "net/http" + "net/url" "strings" + "github.com/google/uuid" "github.com/gravitational/trace" "github.com/julienschmidt/httprouter" @@ -201,6 +203,22 @@ func (h *Handler) createAuthenticateChallengeHandle(w http.ResponseWriter, r *ht allowReuse = mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES } + // Prepare an sso client redirect URL in case the user has an SSO MFA device. + ssoClientRedirectURL, err := url.Parse(sso.WebMFARedirect) + if err != nil { + return nil, trace.Wrap(err) + } + + // id is used by the front end to differentiate between separate ongoing SSO challenges. + id, err := uuid.NewRandom() + if err != nil { + return nil, trace.Wrap(err) + } + channelID := id.String() + query := ssoClientRedirectURL.Query() + query.Set("channel_id", channelID) + ssoClientRedirectURL.RawQuery = query.Encode() + chal, err := clt.CreateAuthenticateChallenge(r.Context(), &proto.CreateAuthenticateChallengeRequest{ Request: &proto.CreateAuthenticateChallengeRequest_ContextUser{ ContextUser: &proto.ContextUser{}, @@ -211,13 +229,13 @@ func (h *Handler) createAuthenticateChallengeHandle(w http.ResponseWriter, r *ht AllowReuse: allowReuse, UserVerificationRequirement: req.UserVerificationRequirement, }, - SSOClientRedirectURL: sso.WebMFARedirect, + SSOClientRedirectURL: ssoClientRedirectURL.String(), }) if err != nil { return nil, trace.Wrap(err) } - return makeAuthenticateChallenge(chal), nil + return makeAuthenticateChallenge(chal, channelID), nil } // createAuthenticateChallengeWithTokenHandle creates and returns MFA authenticate challenges for the user defined in token. @@ -235,7 +253,7 @@ func (h *Handler) createAuthenticateChallengeWithTokenHandle(w http.ResponseWrit return nil, trace.Wrap(err) } - return makeAuthenticateChallenge(chal), nil + return makeAuthenticateChallenge(chal, "" /*channelID*/), nil } type createRegisterChallengeWithTokenRequest struct { @@ -581,7 +599,7 @@ func (h *Handler) checkMFARequired(ctx context.Context, req *isMFARequiredReques } // makeAuthenticateChallenge converts proto to JSON format. -func makeAuthenticateChallenge(protoChal *proto.MFAAuthenticateChallenge) *client.MFAAuthenticateChallenge { +func makeAuthenticateChallenge(protoChal *proto.MFAAuthenticateChallenge, ssoChannelID string) *client.MFAAuthenticateChallenge { chal := &client.MFAAuthenticateChallenge{ TOTPChallenge: protoChal.GetTOTP() != nil, } @@ -590,6 +608,7 @@ func makeAuthenticateChallenge(protoChal *proto.MFAAuthenticateChallenge) *clien } if protoChal.GetSSOChallenge() != nil { chal.SSOChallenge = client.SSOChallengeFromProto(protoChal.GetSSOChallenge()) + chal.SSOChallenge.ChannelID = ssoChannelID } return chal } diff --git a/lib/web/mfajson/mfajson.go b/lib/web/mfajson/mfajson.go index 70abb8ecfec32..2105b0178b3a9 100644 --- a/lib/web/mfajson/mfajson.go +++ b/lib/web/mfajson/mfajson.go @@ -28,7 +28,7 @@ import ( "github.com/gravitational/teleport/lib/client" ) -// TODO(Joerger): DELETE IN v18.0.0 and use client.MFAChallengeResponse instead. +// TODO(Joerger): DELETE IN v19.0.0 and use client.MFAChallengeResponse instead. // Before v17, the WebUI sends a flattened webauthn response instead of a full // MFA challenge response. Newer WebUI versions v17+ will send both for // backwards compatibility. @@ -45,33 +45,17 @@ func Decode(b []byte, typ string) (*authproto.MFAAuthenticateResponse, error) { return nil, trace.Wrap(err) } - switch { - case resp.CredentialAssertionResponse != nil: - return &authproto.MFAAuthenticateResponse{ - Response: &authproto.MFAAuthenticateResponse_Webauthn{ - Webauthn: wantypes.CredentialAssertionResponseToProto(resp.CredentialAssertionResponse), - }, - }, nil - case resp.WebauthnResponse != nil: - return &authproto.MFAAuthenticateResponse{ - Response: &authproto.MFAAuthenticateResponse_Webauthn{ - Webauthn: wantypes.CredentialAssertionResponseToProto(resp.WebauthnResponse), - }, - }, nil - case resp.SSOResponse != nil: - return &authproto.MFAAuthenticateResponse{ - Response: &authproto.MFAAuthenticateResponse_SSO{ - SSO: &authproto.SSOResponse{ - RequestId: resp.SSOResponse.RequestID, - Token: resp.SSOResponse.Token, - }, - }, - }, nil - case resp.TOTPCode != "": - // Note: we can support TOTP through the websocket if desired, we just need to add - // a TOTP prompt modal and flip the switch here. - return nil, trace.BadParameter("totp is not supported in the WebUI") - default: + // Move flattened webauthn response into resp. + resp.MFAChallengeResponse.WebauthnAssertionResponse = resp.CredentialAssertionResponse + + protoResp, err := resp.GetOptionalMFAResponseProtoReq() + if err != nil { + return nil, trace.Wrap(err) + } + + if protoResp == nil { return nil, trace.BadParameter("invalid MFA response from web") } + + return protoResp, trace.Wrap(err) } diff --git a/lib/web/password.go b/lib/web/password.go index 6ae5923787d7e..824c8b00ecb5a 100644 --- a/lib/web/password.go +++ b/lib/web/password.go @@ -108,5 +108,5 @@ func (h *Handler) createAuthenticateChallengeWithPassword(w http.ResponseWriter, return nil, trace.Wrap(err) } - return makeAuthenticateChallenge(chal), nil + return makeAuthenticateChallenge(chal, "" /*channelID*/), nil } diff --git a/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.test.tsx b/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.test.tsx index 96a2fc05a404f..b4fb5a0303fe2 100644 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.test.tsx +++ b/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.test.tsx @@ -23,7 +23,7 @@ import { userEvent, UserEvent } from '@testing-library/user-event'; import { ContextProvider } from 'teleport'; import auth from 'teleport/services/auth'; -import MfaService from 'teleport/services/mfa'; +import MfaService, { SsoChallenge } from 'teleport/services/mfa'; import TeleportContext from 'teleport/teleportContext'; import { AddAuthDeviceWizardStepProps } from './AddAuthDeviceWizard'; @@ -170,11 +170,16 @@ describe('flow without reauthentication', () => { }); describe('flow with reauthentication', () => { + const dummyMfaChallenge = { + totpChallenge: true, + webauthnPublicKey: {} as PublicKeyCredentialRequestOptions, + ssoChallenge: {} as SsoChallenge, + }; + beforeEach(() => { - jest.spyOn(auth, 'getMfaChallenge').mockResolvedValueOnce({ - totpChallenge: true, - webauthnPublicKey: {} as PublicKeyCredentialRequestOptions, - }); + jest + .spyOn(auth, 'getMfaChallenge') + .mockResolvedValueOnce(dummyMfaChallenge); jest.spyOn(auth, 'getMfaChallengeResponse').mockResolvedValueOnce({}); jest .spyOn(auth, 'createPrivilegeToken') @@ -194,6 +199,11 @@ describe('flow with reauthentication', () => { expect(screen.getByTestId('create-step')).toBeInTheDocument(); }); await user.click(screen.getByRole('button', { name: 'Create a passkey' })); + expect(auth.getMfaChallengeResponse).toHaveBeenCalledWith( + dummyMfaChallenge, + 'webauthn', + '' + ); expect(auth.createNewWebAuthnDevice).toHaveBeenCalledWith({ tokenId: 'privilege-token', deviceUsage: 'passwordless', @@ -228,6 +238,46 @@ describe('flow with reauthentication', () => { expect(screen.getByTestId('create-step')).toBeInTheDocument(); }); await user.click(screen.getByRole('button', { name: 'Create a passkey' })); + expect(auth.getMfaChallengeResponse).toHaveBeenCalledWith( + dummyMfaChallenge, + 'totp', + '654987' + ); + expect(auth.createNewWebAuthnDevice).toHaveBeenCalledWith({ + tokenId: 'privilege-token', + deviceUsage: 'passwordless', + }); + + expect(screen.getByTestId('save-step')).toBeInTheDocument(); + await user.type(screen.getByLabelText('Passkey Nickname'), 'new-passkey'); + await user.click(screen.getByRole('button', { name: 'Save the Passkey' })); + expect(ctx.mfaService.saveNewWebAuthnDevice).toHaveBeenCalledWith({ + credential: dummyCredential, + addRequest: { + deviceName: 'new-passkey', + deviceUsage: 'passwordless', + tokenId: 'privilege-token', + }, + }); + expect(onSuccess).toHaveBeenCalled(); + }); + + test('adds a passkey with SSO reauthentication', async () => { + render(); + + await waitFor(() => { + expect(screen.getByTestId('reauthenticate-step')).toBeInTheDocument(); + }); + await user.click(screen.getByText('SSO')); + await user.click(screen.getByText('Verify my identity')); + + expect(screen.getByTestId('create-step')).toBeInTheDocument(); + await user.click(screen.getByRole('button', { name: 'Create a passkey' })); + expect(auth.getMfaChallengeResponse).toHaveBeenCalledWith( + dummyMfaChallenge, + 'sso', + '' + ); expect(auth.createNewWebAuthnDevice).toHaveBeenCalledWith({ tokenId: 'privilege-token', deviceUsage: 'passwordless', diff --git a/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.test.tsx b/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.test.tsx index dd780c4f3996f..c4e77e1365df7 100644 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.test.tsx +++ b/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.test.tsx @@ -23,7 +23,7 @@ import { userEvent, UserEvent } from '@testing-library/user-event'; import TeleportContext from 'teleport/teleportContext'; import { ContextProvider } from 'teleport'; -import MfaService from 'teleport/services/mfa'; +import MfaService, { SsoChallenge } from 'teleport/services/mfa'; import auth from 'teleport/services/auth'; import { DeleteAuthDeviceWizardStepProps } from './DeleteAuthDeviceWizard'; @@ -36,15 +36,18 @@ let ctx: TeleportContext; let user: UserEvent; let onSuccess: jest.Mock; +const dummyMfaChallenge = { + totpChallenge: true, + webauthnPublicKey: {} as PublicKeyCredentialRequestOptions, + ssoChallenge: {} as SsoChallenge, +}; + beforeEach(() => { ctx = new TeleportContext(); user = userEvent.setup(); onSuccess = jest.fn(); - jest.spyOn(auth, 'getMfaChallenge').mockResolvedValueOnce({ - totpChallenge: true, - webauthnPublicKey: {} as PublicKeyCredentialRequestOptions, - }); + jest.spyOn(auth, 'getMfaChallenge').mockResolvedValueOnce(dummyMfaChallenge); jest.spyOn(auth, 'getMfaChallengeResponse').mockResolvedValueOnce({}); jest .spyOn(auth, 'createPrivilegeToken') @@ -80,6 +83,11 @@ test('deletes a device with WebAuthn reauthentication', async () => { expect(screen.getByTestId('delete-step')).toBeInTheDocument(); await user.click(screen.getByRole('button', { name: 'Delete' })); + expect(auth.getMfaChallengeResponse).toHaveBeenCalledWith( + dummyMfaChallenge, + 'webauthn', + '' + ); expect(ctx.mfaService.removeDevice).toHaveBeenCalledWith( 'privilege-token', 'TouchID' @@ -100,6 +108,34 @@ test('deletes a device with OTP reauthentication', async () => { expect(screen.getByTestId('delete-step')).toBeInTheDocument(); await user.click(screen.getByRole('button', { name: 'Delete' })); + expect(auth.getMfaChallengeResponse).toHaveBeenCalledWith( + dummyMfaChallenge, + 'totp', + '654987' + ); + expect(ctx.mfaService.removeDevice).toHaveBeenCalledWith( + 'privilege-token', + 'TouchID' + ); +}); + +test('deletes a device with SSO reauthentication', async () => { + render(); + + await waitFor(() => { + expect(screen.getByTestId('reauthenticate-step')).toBeInTheDocument(); + }); + await user.click(screen.getByText('SSO')); + await user.click(screen.getByText('Verify my identity')); + + expect(screen.getByTestId('delete-step')).toBeInTheDocument(); + await user.click(screen.getByRole('button', { name: 'Delete' })); + + expect(auth.getMfaChallengeResponse).toHaveBeenCalledWith( + dummyMfaChallenge, + 'sso', + '' + ); expect(ctx.mfaService.removeDevice).toHaveBeenCalledWith( 'privilege-token', 'TouchID' diff --git a/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx b/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx index ae561d4950532..6b9e7fdf3400b 100644 --- a/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx +++ b/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx @@ -16,13 +16,13 @@ * along with this program. If not, see . */ -import { render, waitFor, screen } from 'design/utils/testing'; +import { render, screen, waitFor } from 'design/utils/testing'; import { createMemoryHistory } from 'history'; import { Router } from 'react-router'; import { Route } from 'teleport/components/Router'; -import api from 'teleport/services/api'; import cfg from 'teleport/config'; +import api from 'teleport/services/api'; import service from 'teleport/services/apps'; import { AppLauncher } from './AppLauncher'; diff --git a/web/packages/teleport/src/AppLauncher/AppLauncher.tsx b/web/packages/teleport/src/AppLauncher/AppLauncher.tsx index 97d3559bb6365..78db3d6733f2d 100644 --- a/web/packages/teleport/src/AppLauncher/AppLauncher.tsx +++ b/web/packages/teleport/src/AppLauncher/AppLauncher.tsx @@ -26,8 +26,11 @@ import { AccessDenied } from 'design/CardError'; import useAttempt from 'shared/hooks/useAttemptNext'; -import { UrlLauncherParams } from 'teleport/config'; +import AuthnDialog from 'teleport/components/AuthnDialog'; +import { CreateAppSessionParams, UrlLauncherParams } from 'teleport/config'; +import { useMfa } from 'teleport/lib/useMfa'; import service from 'teleport/services/apps'; +import { MfaChallengeScope } from 'teleport/services/auth/auth'; export function AppLauncher() { const { attempt, setAttempt } = useAttempt('processing'); @@ -37,6 +40,19 @@ export function AppLauncher() { const queryParams = new URLSearchParams(search); const isRedirectFlow = queryParams.get('required-apps'); + const mfa = useMfa({ + req: { + scope: MfaChallengeScope.USER_SESSION, + isMfaRequiredRequest: { + app: { + fqdn: pathParams.fqdn, + cluster_name: pathParams.clusterId, + public_addr: pathParams.publicAddr, + }, + }, + }, + }); + const createAppSession = useCallback(async (params: UrlLauncherParams) => { let fqdn = params.fqdn; const port = location.port ? `:${location.port}` : ''; @@ -101,7 +117,10 @@ export function AppLauncher() { if (params.arn) { params.arn = decodeURIComponent(params.arn); } - const session = await service.createAppSession(params); + + const createAppSessionParams = params as CreateAppSessionParams; + createAppSessionParams.mfaResponse = await mfa.getChallengeResponse(); + const session = await service.createAppSession(createAppSessionParams); // Set all the fields expected by server to validate request. const url = getXTeleportAuthUrl({ fqdn, port }); @@ -142,11 +161,16 @@ export function AppLauncher() { createAppSession(pathParams); }, [pathParams]); - if (attempt.status === 'failed') { - return ; - } - - return ; + return ( +
+ {attempt.status === 'failed' ? ( + + ) : ( + + )} + +
+ ); } export function AppLauncherProcessing() { diff --git a/web/packages/teleport/src/Console/DocumentDb/DocumentDb.tsx b/web/packages/teleport/src/Console/DocumentDb/DocumentDb.tsx index 0d6d333141b2a..6c024edfe7331 100644 --- a/web/packages/teleport/src/Console/DocumentDb/DocumentDb.tsx +++ b/web/packages/teleport/src/Console/DocumentDb/DocumentDb.tsx @@ -15,20 +15,20 @@ * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . */ -import { useRef, useEffect } from 'react'; +import { useEffect, useRef } from 'react'; -import { useTheme } from 'styled-components'; import { Box, Indicator } from 'design'; +import { useTheme } from 'styled-components'; -import * as stores from 'teleport/Console/stores/types'; import { Terminal, TerminalRef } from 'teleport/Console/DocumentSsh/Terminal'; -import { useMfa } from 'teleport/lib/useMfa'; +import * as stores from 'teleport/Console/stores/types'; +import { useMfaTty } from 'teleport/lib/useMfa'; import Document from 'teleport/Console/Document'; import AuthnDialog from 'teleport/components/AuthnDialog'; -import { useDbSession } from './useDbSession'; import { ConnectDialog } from './ConnectDialog'; +import { useDbSession } from './useDbSession'; type Props = { visible: boolean; @@ -38,11 +38,11 @@ type Props = { export function DocumentDb({ doc, visible }: Props) { const terminalRef = useRef(); const { tty, status, closeDocument, sendDbConnectData } = useDbSession(doc); - const mfa = useMfa(tty); + const mfa = useMfaTty(tty); useEffect(() => { // when switching tabs or closing tabs, focus on visible terminal terminalRef.current?.focus(); - }, [visible, mfa.requested, status]); + }, [visible, mfa, status]); const theme = useTheme(); return ( @@ -52,7 +52,7 @@ export function DocumentDb({ doc, visible }: Props) { )} - {mfa.requested && } + {status === 'waiting' && ( (); const { tty, status, closeDocument, sendKubeExecData } = useKubeExecSession(doc); - const mfa = useMfa(tty); + const mfa = useMfaTty(tty); useEffect(() => { // when switching tabs or closing tabs, focus on visible terminal terminalRef.current?.focus(); - }, [visible, mfa.requested]); + }, [visible, mfa.challenge]); const theme = useTheme(); const terminal = ( @@ -63,7 +63,7 @@ export default function DocumentKubeExec({ doc, visible }: Props) { )} - {mfa.requested && } + {status === 'waiting-for-exec-data' && ( diff --git a/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx b/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx index c378216dd66fb..4902d90845bf1 100644 --- a/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx +++ b/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx @@ -16,31 +16,32 @@ * along with this program. If not, see . */ -import { useRef, useEffect, useState, useCallback } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { useTheme } from 'styled-components'; -import { Indicator, Box } from 'design'; +import { Box, Indicator } from 'design'; import { - FileTransferActionBar, FileTransfer, - FileTransferRequests, + FileTransferActionBar, FileTransferContextProvider, + FileTransferRequests, } from 'shared/components/FileTransfer'; import { TerminalSearch } from 'shared/components/TerminalSearch'; import * as stores from 'teleport/Console/stores'; import AuthnDialog from 'teleport/components/AuthnDialog'; -import { useMfa } from 'teleport/lib/useMfa'; +import { useMfa, useMfaTty } from 'teleport/lib/useMfa'; +import { MfaChallengeScope } from 'teleport/services/auth/auth'; import Document from '../Document'; import { useConsoleContext } from '../consoleContextProvider'; import { Terminal, TerminalRef } from './Terminal'; -import useSshSession from './useSshSession'; import { useFileTransfer } from './useFileTransfer'; +import useSshSession from './useSshSession'; export default function DocumentSshWrapper(props: PropTypes) { return ( @@ -56,13 +57,15 @@ function DocumentSsh({ doc, visible }: PropTypes) { const terminalRef = useRef(); const { tty, status, closeDocument, session } = useSshSession(doc); const [showSearch, setShowSearch] = useState(false); - const mfa = useMfa(tty); - const { - getMfaResponseAttempt, - getDownloader, - getUploader, - fileTransferRequests, - } = useFileTransfer(tty, session, doc, mfa.addMfaToScpUrls); + + const ttyMfa = useMfaTty(tty); + const ftMfa = useMfa({ + isMfaRequired: ttyMfa.required, + req: { + scope: MfaChallengeScope.USER_SESSION, + }, + }); + const ft = useFileTransfer(tty, session, doc, ftMfa); const theme = useTheme(); function handleCloseFileTransfer() { @@ -75,8 +78,13 @@ function DocumentSsh({ doc, visible }: PropTypes) { useEffect(() => { // when switching tabs or closing tabs, focus on visible terminal - terminalRef.current?.focus(); - }, [visible, mfa.requested]); + if ( + ttyMfa.attempt.status === 'processing' || + ftMfa.attempt.status === 'processing' + ) { + terminalRef.current?.focus(); + } + }, [visible, ttyMfa.attempt.status, ftMfa.attempt.status]); const onSearchClose = useCallback(() => { setShowSearch(false); @@ -110,21 +118,15 @@ function DocumentSsh({ doc, visible }: PropTypes) { } beforeClose={() => window.confirm('Are you sure you want to cancel file transfers?') } - errorText={ - getMfaResponseAttempt.status === 'failed' - ? getMfaResponseAttempt.statusText - : null - } afterClose={handleCloseFileTransfer} transferHandlers={{ - getDownloader, - getUploader, + ...ft, }} /> @@ -143,7 +145,8 @@ function DocumentSsh({ doc, visible }: PropTypes) { )} - {mfa.requested && } + + {status === 'initialized' && terminal} ); diff --git a/web/packages/teleport/src/Console/DocumentSsh/useFileTransfer.ts b/web/packages/teleport/src/Console/DocumentSsh/useFileTransfer.ts index 90c3625a902cf..92c4c9976a198 100644 --- a/web/packages/teleport/src/Console/DocumentSsh/useFileTransfer.ts +++ b/web/packages/teleport/src/Console/DocumentSsh/useFileTransfer.ts @@ -16,18 +16,21 @@ * along with this program. If not, see . */ -import { useEffect, useState, useCallback } from 'react'; +import { useCallback, useEffect, useState } from 'react'; import { useFileTransferContext } from 'shared/components/FileTransfer'; -import Tty from 'teleport/lib/term/tty'; +import { DocumentSsh } from 'teleport/Console/stores'; import { EventType } from 'teleport/lib/term/enums'; +import Tty from 'teleport/lib/term/tty'; import { Session } from 'teleport/services/session'; -import { DocumentSsh } from 'teleport/Console/stores'; + +import cfg from 'teleport/config'; + +import { MfaState } from 'teleport/lib/useMfa'; import { useConsoleContext } from '../consoleContextProvider'; import { getHttpFileTransferHandlers } from './httpFileTransferHandlers'; -import useGetScpUrl from './useGetScpUrl'; export type FileTransferRequest = { sid: string; @@ -51,7 +54,7 @@ export const useFileTransfer = ( tty: Tty, session: Session, currentDoc: DocumentSsh, - addMfaToScpUrls: boolean + mfa: MfaState ) => { const { filesStore } = useFileTransferContext(); const startTransfer = filesStore.start; @@ -60,8 +63,6 @@ export const useFileTransfer = ( const [fileTransferRequests, setFileTransferRequests] = useState< FileTransferRequest[] >([]); - const { getScpUrl, attempt: getMfaResponseAttempt } = - useGetScpUrl(addMfaToScpUrls); const { clusterId, serverId, login } = currentDoc; const download = useCallback( @@ -70,7 +71,8 @@ export const useFileTransfer = ( abortController: AbortController, moderatedSessionParams?: ModeratedSessionParams ) => { - const url = await getScpUrl({ + const mfaResponse = await mfa.getChallengeResponse(); + const url = cfg.getScpUrl({ location, clusterId, serverId, @@ -78,7 +80,9 @@ export const useFileTransfer = ( filename: location, moderatedSessionId: moderatedSessionParams?.moderatedSessionId, fileTransferRequestId: moderatedSessionParams?.fileRequestId, + mfaResponse, }); + if (!url) { // if we return nothing here, the file transfer will not be added to the // file transfer list. If we add it to the list, the file will continue to @@ -88,7 +92,7 @@ export const useFileTransfer = ( } return getHttpFileTransferHandlers().download(url, abortController); }, - [clusterId, login, serverId, getScpUrl] + [clusterId, login, serverId, mfa] ); const upload = useCallback( @@ -98,7 +102,9 @@ export const useFileTransfer = ( abortController: AbortController, moderatedSessionParams?: ModeratedSessionParams ) => { - const url = await getScpUrl({ + const mfaResponse = await mfa.getChallengeResponse(); + + const url = cfg.getScpUrl({ location, clusterId, serverId, @@ -106,6 +112,7 @@ export const useFileTransfer = ( filename: file.name, moderatedSessionId: moderatedSessionParams?.moderatedSessionId, fileTransferRequestId: moderatedSessionParams?.fileRequestId, + mfaResponse, }); if (!url) { // if we return nothing here, the file transfer will not be added to the @@ -116,7 +123,7 @@ export const useFileTransfer = ( } return getHttpFileTransferHandlers().upload(url, file, abortController); }, - [clusterId, serverId, login, getScpUrl] + [clusterId, serverId, login, mfa] ); /* @@ -256,7 +263,6 @@ export const useFileTransfer = ( return { fileTransferRequests, - getMfaResponseAttempt, getUploader, getDownloader, }; diff --git a/web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts b/web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts deleted file mode 100644 index 478ccbcc5fa59..0000000000000 --- a/web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts +++ /dev/null @@ -1,66 +0,0 @@ -/** - * Teleport - * Copyright (C) 2023 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 . - */ - -import { useCallback } from 'react'; -import useAttempt from 'shared/hooks/useAttemptNext'; - -import cfg, { UrlScpParams } from 'teleport/config'; -import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; - -export default function useGetScpUrl(addMfaToScpUrls: boolean) { - const { setAttempt, attempt, handleError } = useAttempt(''); - - const getScpUrl = useCallback( - async (params: UrlScpParams) => { - setAttempt({ - status: 'processing', - statusText: '', - }); - if (!addMfaToScpUrls) { - return cfg.getScpUrl(params); - } - try { - const challenge = await auth.getMfaChallenge({ - scope: MfaChallengeScope.USER_SESSION, - }); - - const response = await auth.getMfaChallengeResponse( - challenge, - 'webauthn' - ); - - setAttempt({ - status: 'success', - statusText: '', - }); - return cfg.getScpUrl({ - webauthn: response.webauthn_response, - ...params, - }); - } catch (error) { - handleError(error); - } - }, - [addMfaToScpUrls, handleError, setAttempt] - ); - - return { - getScpUrl, - attempt, - }; -} diff --git a/web/packages/teleport/src/DesktopSession/DesktopSession.story.tsx b/web/packages/teleport/src/DesktopSession/DesktopSession.story.tsx index 97606b1ea3b86..e401ab43de9f1 100644 --- a/web/packages/teleport/src/DesktopSession/DesktopSession.story.tsx +++ b/web/packages/teleport/src/DesktopSession/DesktopSession.story.tsx @@ -16,16 +16,16 @@ * along with this program. If not, see . */ -import { useState } from 'react'; import { ButtonPrimary } from 'design/Button'; +import { useState } from 'react'; import { NotificationItem } from 'shared/components/Notification'; import { throttle } from 'shared/utils/highbar'; import { TdpClient, TdpClientEvent } from 'teleport/lib/tdp'; import { makeDefaultMfaState } from 'teleport/lib/useMfa'; -import { State } from './useDesktopSession'; import { DesktopSession } from './DesktopSession'; +import { State } from './useDesktopSession'; export default { title: 'Teleport/DesktopSession', @@ -261,14 +261,17 @@ export const WebAuthnPrompt = () => ( }} wsConnection={{ status: 'open' }} mfa={{ - errorText: '', - requested: true, - setErrorText: () => null, - addMfaToScpUrls: false, - onWebauthnAuthenticate: () => null, - onSsoAuthenticate: () => null, - webauthnPublicKey: null, - ssoChallenge: null, + ...makeDefaultMfaState(), + attempt: { + status: 'processing', + statusText: '', + data: null, + }, + challenge: { + webauthnPublicKey: { + challenge: new ArrayBuffer(1), + }, + }, }} /> ); diff --git a/web/packages/teleport/src/DesktopSession/DesktopSession.tsx b/web/packages/teleport/src/DesktopSession/DesktopSession.tsx index f5105f7d0246e..851c72b769fe4 100644 --- a/web/packages/teleport/src/DesktopSession/DesktopSession.tsx +++ b/web/packages/teleport/src/DesktopSession/DesktopSession.tsx @@ -184,12 +184,10 @@ export function DesktopSession(props: State) { const MfaDialog = ({ mfa }: { mfa: MfaState }) => { return ( { - mfa.setErrorText( - 'This session requires multi factor authentication to continue. Please hit "Retry" and follow the prompts given by your browser to complete authentication.' - ); - }} + mfaState={mfa} + replaceErrorText={ + 'This session requires multi factor authentication to continue. Please hit try again and follow the prompts given by your browser to complete authentication.' + } /> ); }; @@ -294,7 +292,7 @@ const nextScreenState = ( // Otherwise, calculate a new screen state. const showAnotherSessionActive = showAnotherSessionActiveDialog; - const showMfa = webauthn.requested; + const showMfa = webauthn.challenge; const showAlert = fetchAttempt.status === 'failed' || // Fetch attempt failed tdpConnection.status === 'failed' || // TDP connection failed diff --git a/web/packages/teleport/src/DesktopSession/useDesktopSession.tsx b/web/packages/teleport/src/DesktopSession/useDesktopSession.tsx index 1f642d38d8d96..f14482669f471 100644 --- a/web/packages/teleport/src/DesktopSession/useDesktopSession.tsx +++ b/web/packages/teleport/src/DesktopSession/useDesktopSession.tsx @@ -22,7 +22,7 @@ import { useParams } from 'react-router'; import useAttempt from 'shared/hooks/useAttemptNext'; import { ButtonState } from 'teleport/lib/tdp'; -import { useMfa } from 'teleport/lib/useMfa'; +import { useMfaTty } from 'teleport/lib/useMfa'; import desktopService from 'teleport/services/desktops'; import userService from 'teleport/services/user'; @@ -130,7 +130,7 @@ export default function useDesktopSession() { }); const tdpClient = clientCanvasProps.tdpClient; - const mfa = useMfa(tdpClient); + const mfa = useMfaTty(tdpClient); const onShareDirectory = () => { try { diff --git a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.story.tsx b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.story.tsx index 0e493d383efb4..7137b983a4d23 100644 --- a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.story.tsx +++ b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.story.tsx @@ -26,21 +26,27 @@ export default { export const LoadedWithMultipleOptions = () => { const props: Props = { - ...defaultProps, - mfa: { - ...defaultProps.mfa, - ssoChallenge: { - redirectUrl: 'hi', - requestId: '123', - channelId: '123', - device: { - connectorId: '123', - connectorType: 'saml', - displayName: 'Okta', - }, + mfaState: { + ...makeDefaultMfaState(), + attempt: { + status: 'processing', + statusText: '', + data: null, }, - webauthnPublicKey: { - challenge: new ArrayBuffer(1), + challenge: { + ssoChallenge: { + redirectUrl: 'hi', + requestId: '123', + channelId: '123', + device: { + connectorId: '123', + connectorType: 'saml', + displayName: 'Okta', + }, + }, + webauthnPublicKey: { + challenge: new ArrayBuffer(1), + }, }, }, }; @@ -49,29 +55,35 @@ export const LoadedWithMultipleOptions = () => { export const LoadedWithSingleOption = () => { const props: Props = { - ...defaultProps, - mfa: { - ...defaultProps.mfa, - webauthnPublicKey: { - challenge: new ArrayBuffer(1), + mfaState: { + ...makeDefaultMfaState(), + attempt: { + status: 'processing', + statusText: '', + data: null, + }, + challenge: { + webauthnPublicKey: { + challenge: new ArrayBuffer(1), + }, }, }, }; return ; }; -export const Error = () => { +export const LoadedWithError = () => { + const err = new Error('Something went wrong'); const props: Props = { - ...defaultProps, - mfa: { - ...defaultProps.mfa, - errorText: 'Something went wrong', + mfaState: { + ...makeDefaultMfaState(), + attempt: { + status: 'error', + statusText: err.message, + error: err, + data: null, + }, }, }; return ; }; - -const defaultProps: Props = { - mfa: makeDefaultMfaState(), - onCancel: () => null, -}; diff --git a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.test.tsx b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.test.tsx index 516c021c8d452..34be98660bc39 100644 --- a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.test.tsx +++ b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.test.tsx @@ -16,15 +16,15 @@ * along with this program. If not, see . */ -import { render, screen, fireEvent } from 'design/utils/testing'; +import { fireEvent, render, screen } from 'design/utils/testing'; import { makeDefaultMfaState, MfaState } from 'teleport/lib/useMfa'; -import { SSOChallenge } from 'teleport/services/mfa'; +import { getMfaChallengeOptions, SsoChallenge } from 'teleport/services/mfa'; import AuthnDialog from './AuthnDialog'; -const mockSsoChallenge: SSOChallenge = { +const mockSsoChallenge: SsoChallenge = { redirectUrl: 'url', requestId: '123', device: { @@ -51,8 +51,17 @@ describe('AuthnDialog', () => { }); test('renders single option dialog', () => { - const mfa = makeMockState({ ssoChallenge: mockSsoChallenge }); - render(); + const mfa = makeMockState({ + challenge: { + ssoChallenge: mockSsoChallenge, + }, + attempt: { + status: 'processing', + statusText: '', + data: null, + }, + }); + render(); expect(screen.getByText('Verify Your Identity')).toBeInTheDocument(); expect( @@ -63,13 +72,22 @@ describe('AuthnDialog', () => { }); test('renders multi option dialog', () => { - const mfa = makeMockState({ + const challenge = { ssoChallenge: mockSsoChallenge, webauthnPublicKey: { challenge: new ArrayBuffer(1), }, + }; + const mfa = makeMockState({ + options: getMfaChallengeOptions(challenge), + challenge, + attempt: { + status: 'processing', + statusText: '', + data: null, + }, }); - render(); + render(); expect(screen.getByText('Verify Your Identity')).toBeInTheDocument(); expect( @@ -83,8 +101,16 @@ describe('AuthnDialog', () => { test('displays error text when provided', () => { const errorText = 'Authentication failed'; - const mfa = makeMockState({ errorText }); - render(); + const mfa = makeMockState({ + challenge: {}, + attempt: { + status: 'error', + statusText: errorText, + data: null, + error: new Error(errorText), + }, + }); + render(); expect(screen.getByTestId('danger-alert')).toBeInTheDocument(); expect(screen.getByText(errorText)).toBeInTheDocument(); @@ -92,23 +118,37 @@ describe('AuthnDialog', () => { test('sso button renders with callback', async () => { const mfa = makeMockState({ - ssoChallenge: mockSsoChallenge, - onSsoAuthenticate: jest.fn(), + challenge: { + ssoChallenge: mockSsoChallenge, + }, + attempt: { + status: 'processing', + statusText: '', + data: null, + }, + submit: jest.fn(), }); - render(); + render(); const ssoButton = screen.getByText('Okta'); fireEvent.click(ssoButton); - expect(mfa.onSsoAuthenticate).toHaveBeenCalledTimes(1); + expect(mfa.submit).toHaveBeenCalledTimes(1); }); test('webauthn button renders with callback', async () => { const mfa = makeMockState({ - webauthnPublicKey: { challenge: new ArrayBuffer(0) }, - onWebauthnAuthenticate: jest.fn(), + challenge: { + webauthnPublicKey: { challenge: new ArrayBuffer(0) }, + }, + attempt: { + status: 'processing', + statusText: '', + data: null, + }, + submit: jest.fn(), }); - render(); + render(); const webauthn = screen.getByText('Passkey or MFA Device'); fireEvent.click(webauthn); - expect(mfa.onWebauthnAuthenticate).toHaveBeenCalledTimes(1); + expect(mfa.submit).toHaveBeenCalledTimes(1); }); }); diff --git a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx index 0a301b1f16c43..1b862601beaf1 100644 --- a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx +++ b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx @@ -16,76 +16,101 @@ * along with this program. If not, see . */ -import Dialog, { DialogContent } from 'design/Dialog'; import { Danger } from 'design/Alert'; -import { FingerprintSimple, Cross } from 'design/Icon'; +import Dialog, { DialogContent } from 'design/Dialog'; +import { Cross, FingerprintSimple } from 'design/Icon'; -import { Text, ButtonSecondary, Flex, ButtonIcon, H2 } from 'design'; +import { ButtonIcon, ButtonSecondary, Flex, H2, Text } from 'design'; import { guessProviderType } from 'shared/components/ButtonSso'; import { SSOIcon } from 'shared/components/ButtonSso/ButtonSso'; import { MfaState } from 'teleport/lib/useMfa'; +import { MFA_OPTION_TOTP } from 'teleport/services/mfa'; + +export type Props = { + mfaState: MfaState; + replaceErrorText?: string; + onClose?: () => void; +}; + +export default function AuthnDialog({ + mfaState: { options, challenge, submit, attempt, resetAttempt }, + replaceErrorText, + onClose, +}: Props) { + if (!challenge && attempt.status !== 'error') return; -export default function AuthnDialog({ mfa, onCancel }: Props) { - let hasMultipleOptions = mfa.ssoChallenge && mfa.webauthnPublicKey; + // TODO(Joerger): TOTP should be pretty easy to support here with a small button -> form flow. + const onlyTotpAvailable = + options?.length === 1 && options[0] === MFA_OPTION_TOTP; return ( ({ width: '400px' })} open={true}>

Verify Your Identity

- + { + resetAttempt(); + onClose(); + }} + > - {mfa.errorText && ( + {onlyTotpAvailable && ( - {mfa.errorText} + { + 'Authenticator app is not currently supported for this action, please register a passkey or a security key to continue.' + } + + )} + {attempt.status === 'error' && ( + + {replaceErrorText || attempt.statusText} )} - {hasMultipleOptions + {options?.length > 1 ? 'Select one of the following methods to verify your identity:' : 'Select the method below to verify your identity:'} - - {mfa.ssoChallenge && ( - - - {mfa.ssoChallenge.device.displayName || - mfa.ssoChallenge.device.connectorId} - - )} - {mfa.webauthnPublicKey && ( - - - Passkey or MFA Device - - )} - + {challenge && ( + + {challenge.ssoChallenge && ( + submit('sso')} + gap={2} + block + > + + {challenge.ssoChallenge.device.displayName || + challenge.ssoChallenge.device.connectorId} + + )} + {challenge.webauthnPublicKey && ( + submit('webauthn')} + gap={2} + block + > + + Passkey or MFA Device + + )} + + )} ); } - -export type Props = { - mfa: MfaState; - onCancel: () => void; -}; diff --git a/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts b/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts index ed8c73f3fe6da..62291b5a1f6a7 100644 --- a/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts +++ b/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts @@ -38,12 +38,15 @@ export default function useReAuthenticate({ const [mfaOptions, setMfaOptions] = useState(); const [challengeState, setChallengeState] = useState(); + function setMfaChallenge(challenge: MfaAuthenticateChallenge) { + setChallengeState({ challenge, deviceUsage: 'mfa' }); + } + const [initAttempt, init] = useAsync(async () => { const challenge = await auth.getMfaChallenge({ scope: challengeScope, }); - - setChallengeState({ challenge, deviceUsage: 'mfa' }); + setMfaChallenge(challenge); setMfaOptions(getMfaChallengeOptions(challenge)); }); @@ -112,6 +115,7 @@ export default function useReAuthenticate({ return { initAttempt, mfaOptions, + setMfaChallenge, submitWithMfa, submitAttempt, clearSubmitAttempt, @@ -126,6 +130,7 @@ export type ReauthProps = { export type ReauthState = { initAttempt: Attempt; mfaOptions: MfaOption[]; + setMfaChallenge: (challenge: MfaAuthenticateChallenge) => void; submitWithMfa: ( mfaType?: DeviceType, deviceUsage?: DeviceUsage, diff --git a/web/packages/teleport/src/config.ts b/web/packages/teleport/src/config.ts index f69a468f121fe..5d1792b2abb3d 100644 --- a/web/packages/teleport/src/config.ts +++ b/web/packages/teleport/src/config.ts @@ -17,9 +17,7 @@ */ import { generatePath } from 'react-router'; - import { IncludedResourceMode } from 'shared/components/UnifiedResources'; - import { mergeDeep } from 'shared/utils/highbar'; import { @@ -43,10 +41,10 @@ import type { import type { SortType } from 'teleport/services/agents'; import type { KubeResourceKind } from 'teleport/services/kube/types'; -import type { WebauthnAssertionResponse } from 'teleport/services/mfa'; import type { RecordingType } from 'teleport/services/recordings'; import type { ParticipantMode } from 'teleport/services/session'; import type { YamlSupportedResourceKind } from 'teleport/services/yaml/types'; +import type { MfaChallengeResponse } from './services/mfa'; const cfg = { /** @deprecated Use cfg.edition instead. */ @@ -885,20 +883,25 @@ const cfg = { }); }, - getScpUrl({ webauthn, ...params }: UrlScpParams) { + getScpUrl({ mfaResponse, ...params }: UrlScpParams) { let path = generatePath(cfg.api.scp, { ...params, }); - if (!webauthn) { + if (!mfaResponse) { return path; } // non-required MFA will mean this param is undefined and generatePath doesn't like undefined // or optional params. So we append it ourselves here. Its ok to be undefined when sent to the server // as the existence of this param is what will issue certs - return `${path}&webauthn=${JSON.stringify({ - webauthnAssertionResponse: webauthn, + + // TODO(Joerger): DELETE IN v19.0.0 + // We include webauthn for backwards compatibility. + path = `${path}&webauthn=${JSON.stringify({ + webauthnAssertionResponse: mfaResponse.webauthn_response, })}`; + + return `${path}&mfaResponse=${JSON.stringify(mfaResponse)}`; }, getRenewTokenUrl() { @@ -1246,6 +1249,14 @@ export interface UrlAppParams { arn?: string; } +export interface CreateAppSessionParams { + fqdn: string; + clusterId?: string; + publicAddr?: string; + arn?: string; + mfaResponse?: MfaChallengeResponse; +} + export interface UrlScpParams { clusterId: string; serverId: string; @@ -1254,7 +1265,7 @@ export interface UrlScpParams { filename: string; moderatedSessionId?: string; fileTransferRequestId?: string; - webauthn?: WebauthnAssertionResponse; + mfaResponse?: MfaChallengeResponse; } export interface UrlSshParams { diff --git a/web/packages/teleport/src/lib/EventEmitterMfaSender.ts b/web/packages/teleport/src/lib/EventEmitterMfaSender.ts index da30f1201e0c9..2753251121061 100644 --- a/web/packages/teleport/src/lib/EventEmitterMfaSender.ts +++ b/web/packages/teleport/src/lib/EventEmitterMfaSender.ts @@ -18,10 +18,7 @@ import { EventEmitter } from 'events'; -import { - MfaChallengeResponse, - WebauthnAssertionResponse, -} from 'teleport/services/mfa'; +import { MfaChallengeResponse } from 'teleport/services/mfa'; class EventEmitterMfaSender extends EventEmitter { constructor() { @@ -32,15 +29,6 @@ class EventEmitterMfaSender extends EventEmitter { sendChallengeResponse(data: MfaChallengeResponse) { throw new Error('Not implemented'); } - - // TODO (avatus) DELETE IN 18 - /** - * @deprecated Use sendChallengeResponse instead. - */ - // eslint-disable-next-line @typescript-eslint/no-unused-vars - sendWebAuthn(data: WebauthnAssertionResponse) { - throw new Error('Not implemented'); - } } export { EventEmitterMfaSender }; diff --git a/web/packages/teleport/src/lib/tdp/client.ts b/web/packages/teleport/src/lib/tdp/client.ts index ca18c58744124..b6ab1264b185d 100644 --- a/web/packages/teleport/src/lib/tdp/client.ts +++ b/web/packages/teleport/src/lib/tdp/client.ts @@ -57,7 +57,6 @@ import type { SyncKeys, SharedDirectoryTruncateResponse, } from './codec'; -import type { WebauthnAssertionResponse } from 'teleport/services/mfa'; export enum TdpClientEvent { TDP_CLIENT_SCREEN_SPEC = 'tdp client screen spec', @@ -624,14 +623,6 @@ export default class Client extends EventEmitterMfaSender { this.send(this.codec.encodeClipboardData(clipboardData)); } - sendWebAuthn(data: WebauthnAssertionResponse) { - const msg = this.codec.encodeMfaJson({ - mfaType: 'n', - jsonString: JSON.stringify(data), - }); - this.send(msg); - } - addSharedDirectory(sharedDirectory: FileSystemDirectoryHandle) { try { this.sdManager.add(sharedDirectory); diff --git a/web/packages/teleport/src/lib/term/tty.ts b/web/packages/teleport/src/lib/term/tty.ts index 3e924ff466f3f..a78fafb1ebd0d 100644 --- a/web/packages/teleport/src/lib/term/tty.ts +++ b/web/packages/teleport/src/lib/term/tty.ts @@ -19,7 +19,6 @@ import Logger from 'shared/libs/logger'; import { EventEmitterMfaSender } from 'teleport/lib/EventEmitterMfaSender'; -import { WebauthnAssertionResponse } from 'teleport/services/mfa'; import { AuthenticatedWebSocket } from 'teleport/lib/AuthenticatedWebSocket'; import { MfaChallengeResponse } from 'teleport/services/mfa'; @@ -88,7 +87,7 @@ class Tty extends EventEmitterMfaSender { // but to be backward compatible, we need to still spread the existing webauthn only fields // as "top level" fields so old proxies can still respond to webauthn challenges. // in 19, we can just pass "data" without this extra step - // TODO (avatus): DELETE IN 18 + // TODO (avatus): DELETE IN 19.0.0 const backwardCompatibleData = { ...data.webauthn_response, ...data, @@ -100,16 +99,6 @@ class Tty extends EventEmitterMfaSender { this.socket.send(bytearray); } - // TODO (avatus) DELETE IN 18 - /** - * @deprecated Use sendChallengeResponse instead. - */ - sendWebAuthn(data: WebauthnAssertionResponse) { - const encoded = this._proto.encodeChallengeResponse(JSON.stringify(data)); - const bytearray = new Uint8Array(encoded); - this.socket.send(bytearray); - } - sendKubeExecData(data: KubeExecData) { const encoded = this._proto.encodeKubeExecData(JSON.stringify(data)); const bytearray = new Uint8Array(encoded); diff --git a/web/packages/teleport/src/lib/useMfa.test.tsx b/web/packages/teleport/src/lib/useMfa.test.tsx new file mode 100644 index 0000000000000..886fbff25a662 --- /dev/null +++ b/web/packages/teleport/src/lib/useMfa.test.tsx @@ -0,0 +1,246 @@ +/** + * Teleport + * Copyright (C) 2024 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 . + */ + +import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; + +import { renderHook, waitFor } from '@testing-library/react'; +import { useState } from 'react'; + +import { CreateAuthenticateChallengeRequest } from 'teleport/services/auth'; +import { + MFA_OPTION_WEBAUTHN, + MfaAuthenticateChallenge, + MfaChallengeResponse, +} from 'teleport/services/mfa'; + +import { useMfa } from './useMfa'; + +const mockChallenge: MfaAuthenticateChallenge = { + webauthnPublicKey: {} as PublicKeyCredentialRequestOptions, +}; + +const mockResponse: MfaChallengeResponse = { + webauthn_response: { + id: 'cred-id', + type: 'public-key', + extensions: { + appid: true, + }, + rawId: 'rawId', + response: { + authenticatorData: 'authenticatorData', + clientDataJSON: 'clientDataJSON', + signature: 'signature', + userHandle: 'userHandle', + }, + }, +}; + +const mockChallengeReq: CreateAuthenticateChallengeRequest = { + scope: MfaChallengeScope.USER_SESSION, + isMfaRequiredRequest: { + node: { + node_name: 'node', + login: 'login', + }, + }, +}; + +describe('useMfa', () => { + beforeEach(() => { + jest.spyOn(console, 'error').mockImplementation(); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + test('mfa required', async () => { + jest.spyOn(auth, 'getMfaChallenge').mockResolvedValueOnce(mockChallenge); + jest + .spyOn(auth, 'getMfaChallengeResponse') + .mockResolvedValueOnce(mockResponse); + const { result: mfa } = renderHook(() => + useMfa({ + req: mockChallengeReq, + }) + ); + + const respPromise = mfa.current.getChallengeResponse(); + await waitFor(() => { + expect(auth.getMfaChallenge).toHaveBeenCalledWith(mockChallengeReq); + }); + + expect(mfa.current.options).toEqual([MFA_OPTION_WEBAUTHN]); + expect(mfa.current.required).toEqual(true); + expect(mfa.current.challenge).toEqual(mockChallenge); + expect(mfa.current.attempt.status).toEqual('processing'); + + await mfa.current.submit('webauthn'); + await waitFor(() => { + expect(auth.getMfaChallengeResponse).toHaveBeenCalledWith( + mockChallenge, + 'webauthn', + undefined + ); + }); + + const resp = await respPromise; + expect(resp).toEqual(mockResponse); + expect(mfa.current.challenge).toEqual(null); + expect(mfa.current.attempt.status).toEqual('success'); + }); + + test('mfa not required', async () => { + jest.spyOn(auth, 'getMfaChallenge').mockResolvedValue(null); + + const { result: mfa } = renderHook(() => + useMfa({ + req: mockChallengeReq, + }) + ); + + // If a challenge is not returned, an empty mfa response should be returned + // early and the requirement changed to false for future calls. + const resp = await mfa.current.getChallengeResponse(); + expect(auth.getMfaChallenge).toHaveBeenCalledWith(mockChallengeReq); + expect(resp).toEqual(undefined); + await waitFor(() => expect(mfa.current.required).toEqual(false)); + }); + + test('adaptable mfa requirement state', async () => { + jest.spyOn(auth, 'getMfaChallenge').mockResolvedValue(null); + + let isMfaRequired: boolean; + let setMfaRequired: (b: boolean) => void; + + let req: CreateAuthenticateChallengeRequest; + let setReq: (r: CreateAuthenticateChallengeRequest) => void; + + const { result: mfa } = renderHook(() => { + [isMfaRequired, setMfaRequired] = useState(null); + [req, setReq] = + useState(mockChallengeReq); + + return useMfa({ + req: req, + isMfaRequired: isMfaRequired, + }); + }); + + // mfaRequired should change when the isMfaRequired arg changes, allowing + // callers to propagate mfa required late (e.g. per-session MFA for file transfers) + setMfaRequired(false); + await waitFor(() => expect(mfa.current.required).toEqual(false)); + + setMfaRequired(true); + await waitFor(() => expect(mfa.current.required).toEqual(true)); + + setMfaRequired(null); + await waitFor(() => expect(mfa.current.required).toEqual(null)); + + // If isMfaRequiredRequest changes, the mfaRequired value should be reset. + setReq({ + ...mockChallengeReq, + isMfaRequiredRequest: { + admin_action: {}, + }, + }); + await waitFor(() => expect(mfa.current.required).toEqual(null)); + }); + + test('mfa challenge error', async () => { + const err = new Error('an error has occurred'); + jest.spyOn(auth, 'getMfaChallenge').mockImplementation(() => { + throw err; + }); + + const { result: mfa } = renderHook(() => useMfa({})); + + await expect(mfa.current.getChallengeResponse).rejects.toThrow(err); + await waitFor(() => { + expect(mfa.current.attempt).toEqual({ + status: 'error', + statusText: err.message, + error: err, + data: null, + }); + }); + }); + + test('mfa response error', async () => { + const err = new Error('an error has occurred'); + jest.spyOn(auth, 'getMfaChallenge').mockResolvedValueOnce(mockChallenge); + jest.spyOn(auth, 'getMfaChallengeResponse').mockImplementation(async () => { + throw err; + }); + + const { result: mfa } = renderHook(() => + useMfa({ + req: mockChallengeReq, + }) + ); + + const respPromise = mfa.current.getChallengeResponse(); + await waitFor(() => { + expect(auth.getMfaChallenge).toHaveBeenCalledWith(mockChallengeReq); + }); + await mfa.current.submit('webauthn'); + + await waitFor(() => { + expect(mfa.current.attempt).toEqual({ + status: 'error', + statusText: err.message, + error: err, + data: null, + }); + }); + + // After an error, the mfa response promise remains in an unresolved state, + // allowing for retries. + jest + .spyOn(auth, 'getMfaChallengeResponse') + .mockResolvedValueOnce(mockResponse); + await mfa.current.submit('webauthn'); + expect(await respPromise).toEqual(mockResponse); + }); + + test('reset mfa attempt', async () => { + jest.spyOn(auth, 'getMfaChallenge').mockResolvedValue(mockChallenge); + const { result: mfa } = renderHook(() => + useMfa({ + req: mockChallengeReq, + }) + ); + + const respPromise = mfa.current.getChallengeResponse(); + await waitFor(() => { + expect(auth.getMfaChallenge).toHaveBeenCalled(); + }); + + mfa.current.resetAttempt(); + + await expect(respPromise).rejects.toThrow( + new Error('MFA attempt cancelled by user') + ); + + await waitFor(() => { + expect(mfa.current.attempt.status).toEqual('error'); + }); + }); +}); diff --git a/web/packages/teleport/src/lib/useMfa.ts b/web/packages/teleport/src/lib/useMfa.ts index 664016790e002..50f852c3768e3 100644 --- a/web/packages/teleport/src/lib/useMfa.ts +++ b/web/packages/teleport/src/lib/useMfa.ts @@ -16,220 +16,208 @@ * along with this program. If not, see . */ -import { useState, useEffect, useCallback } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; +import { Attempt, makeEmptyAttempt, useAsync } from 'shared/hooks/useAsync'; import { EventEmitterMfaSender } from 'teleport/lib/EventEmitterMfaSender'; import { TermEvent } from 'teleport/lib/term/enums'; -import { parseMfaChallengeJson as parseMfaChallenge } from 'teleport/services/mfa/makeMfa'; import { - MfaAuthenticateChallengeJson, - SSOChallenge, -} from 'teleport/services/mfa'; + CreateAuthenticateChallengeRequest, + parseMfaChallengeJson, +} from 'teleport/services/auth'; import auth from 'teleport/services/auth/auth'; +import { + DeviceType, + getMfaChallengeOptions, + MfaAuthenticateChallenge, + MfaChallengeResponse, + MfaOption, +} from 'teleport/services/mfa'; -export function useMfa(emitterSender: EventEmitterMfaSender): MfaState { - const [state, setState] = useState<{ - errorText: string; - addMfaToScpUrls: boolean; - webauthnPublicKey: PublicKeyCredentialRequestOptions; - ssoChallenge: SSOChallenge; - totpChallenge: boolean; - }>({ - addMfaToScpUrls: false, - errorText: '', - webauthnPublicKey: null, - ssoChallenge: null, - totpChallenge: false, - }); - - function clearChallenges() { - setState(prevState => ({ - ...prevState, - totpChallenge: false, - webauthnPublicKey: null, - ssoChallenge: null, - })); - } - - function onSsoAuthenticate() { - if (!state.ssoChallenge) { - setState(prevState => ({ - ...prevState, - errorText: 'Invalid or missing SSO challenge', - })); - return; - } - - // try to center the screen - const width = 1045; - const height = 550; - const left = (screen.width - width) / 2; - const top = (screen.height - height) / 2; - - // these params will open a tiny window. - const params = `width=${width},height=${height},left=${left},top=${top}`; - window.open(state.ssoChallenge.redirectUrl, '_blank', params); - } - - function onWebauthnAuthenticate() { - if (!window.PublicKeyCredential) { - const errorText = - 'This browser does not support WebAuthn required for hardware tokens, \ - please try the latest version of Chrome, Firefox or Safari.'; - - setState({ - ...state, - errorText, - }); - return; - } - - auth - .getMfaChallengeResponse({ - webauthnPublicKey: state.webauthnPublicKey, - }) - .then(res => { - setState(prevState => ({ - ...prevState, - errorText: '', - webauthnPublicKey: null, - })); - emitterSender.sendWebAuthn(res.webauthn_response); - }) - .catch((err: Error) => { - setErrorText(err.message); - }); - } - - const waitForSsoChallengeResponse = useCallback( - async ( - ssoChallenge: SSOChallenge, - abortSignal: AbortSignal - ): Promise => { - const channel = new BroadcastChannel(ssoChallenge.channelId); +export type MfaProps = { + req?: CreateAuthenticateChallengeRequest; + isMfaRequired?: boolean | null; +}; - try { - const event = await waitForMessage(channel, abortSignal); - emitterSender.sendChallengeResponse({ - sso_response: { - requestId: ssoChallenge.requestId, - token: event.data.mfaToken, - }, +type mfaResponsePromiseWithResolvers = { + promise: Promise; + resolve: (v: MfaChallengeResponse) => void; + reject: (v?: any) => void; +}; + +/** + * Use the returned object to request MFA checks with a shared state. + * When MFA authentication is in progress, the object's properties can + * be used to display options to the user and prompt for them to complete + * the MFA check. + */ +export function useMfa({ req, isMfaRequired }: MfaProps): MfaState { + const [mfaRequired, setMfaRequired] = useState(); + const [options, setMfaOptions] = useState(); + const [challenge, setMfaChallenge] = useState(); + + const mfaResponsePromiseWithResolvers = + useRef(); + + useEffect(() => { + setMfaRequired(isMfaRequired); + }, [isMfaRequired]); + + useEffect(() => { + setMfaRequired(null); + }, [req?.isMfaRequiredRequest]); + + // getResponse is used to initiate MFA authentication. + // 1. Check if MFA is required by getting a new MFA challenge + // 2. If MFA is required, set the challenge in the MFA state and wait for it to + // be resolved by the caller. + // 3. The caller sees the mfa challenge set in state and submits an mfa response + // request with arguments provided by the user (mfa type, otp code). + // 4. Receive the mfa response through the mfaResponsePromise ref and return it. + // + // The caller should also display errors seen in attempt. + + const [attempt, getResponse, setMfaAttempt] = useAsync( + useCallback( + async (challenge?: MfaAuthenticateChallenge) => { + // If a previous call determined that MFA is not required, this is a noop. + if (mfaRequired === false) return; + + challenge = challenge ? challenge : await auth.getMfaChallenge(req); + if (!challenge) { + setMfaRequired(false); + return; + } + + // Set mfa requirement and options after we get a challenge for the first time. + if (!mfaRequired) setMfaRequired(true); + if (!options) setMfaOptions(getMfaChallengeOptions(challenge)); + + // Prepare a new promise to collect the mfa response retrieved + // through the submit function. + let resolve, reject; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; }); - clearChallenges(); - } catch (error) { - if (error.name !== 'AbortError') { - throw error; + + mfaResponsePromiseWithResolvers.current = { + promise, + resolve, + reject, + }; + + setMfaChallenge(challenge); + try { + return await promise; + } finally { + mfaResponsePromiseWithResolvers.current = null; + setMfaChallenge(null); } - } finally { - channel.close(); - } + }, + [req, mfaResponsePromiseWithResolvers, options, mfaRequired] + ) + ); + + const resetAttempt = () => { + if (mfaResponsePromiseWithResolvers.current) + mfaResponsePromiseWithResolvers.current.reject( + new Error('MFA attempt cancelled by user') + ); + mfaResponsePromiseWithResolvers.current = null; + setMfaChallenge(null); + setMfaAttempt(makeEmptyAttempt()); + }; + + const getChallengeResponse = useCallback( + async (challenge?: MfaAuthenticateChallenge) => { + const [resp, err] = await getResponse(challenge); + if (err) throw err; + return resp; }, - [emitterSender] + [getResponse] ); - useEffect(() => { - let ssoChallengeAbortController: AbortController | undefined; - const challengeHandler = (challengeJson: string) => { - const challenge = JSON.parse( - challengeJson - ) as MfaAuthenticateChallengeJson; - - const { webauthnPublicKey, ssoChallenge, totpChallenge } = - parseMfaChallenge(challenge); - - setState(prevState => ({ - ...prevState, - addMfaToScpUrls: true, - ssoChallenge, - webauthnPublicKey, - totpChallenge, - })); - - if (ssoChallenge) { - ssoChallengeAbortController?.abort(); - ssoChallengeAbortController = new AbortController(); - void waitForSsoChallengeResponse( - ssoChallenge, - ssoChallengeAbortController.signal + const submit = useCallback( + async (mfaType?: DeviceType, totpCode?: string) => { + if (!mfaResponsePromiseWithResolvers.current) { + throw new Error('submit called without an in flight MFA attempt'); + } + + try { + await mfaResponsePromiseWithResolvers.current.resolve( + await auth.getMfaChallengeResponse(challenge, mfaType, totpCode) ); + } catch (err) { + setMfaAttempt({ + data: null, + status: 'error', + statusText: err.message, + error: err, + }); } + }, + [challenge, mfaResponsePromiseWithResolvers, setMfaAttempt] + ); + + return { + required: mfaRequired, + options, + challenge, + getChallengeResponse, + submit, + attempt, + resetAttempt, + }; +} + +export function useMfaTty(emitterSender: EventEmitterMfaSender): MfaState { + const [mfaRequired, setMfaRequired] = useState(false); + + const mfa = useMfa({ isMfaRequired: mfaRequired }); + + useEffect(() => { + const challengeHandler = async (challengeJson: string) => { + // set Mfa required for other uses of this MfaState (e.g. file transfers) + setMfaRequired(true); + + const challenge = parseMfaChallengeJson(JSON.parse(challengeJson)); + const resp = await mfa.getChallengeResponse(challenge); + emitterSender.sendChallengeResponse(resp); }; emitterSender?.on(TermEvent.MFA_CHALLENGE, challengeHandler); - return () => { - ssoChallengeAbortController?.abort(); emitterSender?.removeListener(TermEvent.MFA_CHALLENGE, challengeHandler); }; - }, [emitterSender, waitForSsoChallengeResponse]); - - function setErrorText(newErrorText: string) { - setState(prevState => ({ ...prevState, errorText: newErrorText })); - } - - // if any challenge exists, requested is true - const requested = !!( - state.webauthnPublicKey || - state.totpChallenge || - state.ssoChallenge - ); + }, [mfa, emitterSender]); - return { - requested, - onWebauthnAuthenticate, - onSsoAuthenticate, - addMfaToScpUrls: state.addMfaToScpUrls, - setErrorText, - errorText: state.errorText, - webauthnPublicKey: state.webauthnPublicKey, - ssoChallenge: state.ssoChallenge, - }; + return mfa; } export type MfaState = { - onWebauthnAuthenticate: () => void; - onSsoAuthenticate: () => void; - setErrorText: (errorText: string) => void; - errorText: string; - requested: boolean; - addMfaToScpUrls: boolean; - webauthnPublicKey: PublicKeyCredentialRequestOptions; - ssoChallenge: SSOChallenge; + required: boolean; + options: MfaOption[]; + challenge: MfaAuthenticateChallenge; + // Generally you wouldn't pass in a challenge, unless you already + // have one handy, e.g. from a terminal websocket message. + getChallengeResponse: ( + challenge?: MfaAuthenticateChallenge + ) => Promise; + submit: (mfaType?: DeviceType, totpCode?: string) => Promise; + attempt: Attempt; + resetAttempt: () => void; }; // used for testing export function makeDefaultMfaState(): MfaState { return { - onWebauthnAuthenticate: () => null, - onSsoAuthenticate: () => null, - setErrorText: () => null, - errorText: '', - requested: false, - addMfaToScpUrls: false, - webauthnPublicKey: null, - ssoChallenge: null, + required: true, + options: null, + challenge: null, + getChallengeResponse: async () => null, + submit: () => null, + attempt: makeEmptyAttempt(), + resetAttempt: () => null, }; } - -function waitForMessage( - channel: BroadcastChannel, - abortSignal: AbortSignal -): Promise { - return new Promise((resolve, reject) => { - // Create the event listener - function eventHandler(e: MessageEvent) { - // Remove the event listener after it triggers - channel.removeEventListener('message', eventHandler); - // Resolve the promise with the event object - resolve(e); - } - - // Add the event listener - channel.addEventListener('message', eventHandler); - abortSignal.onabort = e => { - channel.removeEventListener('message', eventHandler); - reject(e); - }; - }); -} diff --git a/web/packages/teleport/src/services/api/api.test.ts b/web/packages/teleport/src/services/api/api.test.ts index b9689eeb4210b..af362e602bc4e 100644 --- a/web/packages/teleport/src/services/api/api.test.ts +++ b/web/packages/teleport/src/services/api/api.test.ts @@ -16,8 +16,6 @@ * along with this program. If not, see . */ -import { MfaChallengeResponse } from '../mfa'; - import api, { MFA_HEADER, defaultRequestOptions, @@ -28,7 +26,7 @@ import api, { describe('api.fetch', () => { const mockedFetch = jest.spyOn(global, 'fetch').mockResolvedValue({} as any); // we don't care about response - const mfaResp: MfaChallengeResponse = { + const mfaResp = { webauthn_response: { id: 'some-id', type: 'some-type', @@ -104,6 +102,7 @@ describe('api.fetch', () => { ...defaultRequestOptions.headers, ...getAuthHeaders(), [MFA_HEADER]: JSON.stringify({ + ...mfaResp, webauthnAssertionResponse: mfaResp.webauthn_response, }), }, @@ -124,6 +123,7 @@ describe('api.fetch', () => { ...customOpts.headers, ...getAuthHeaders(), [MFA_HEADER]: JSON.stringify({ + ...mfaResp, webauthnAssertionResponse: mfaResp.webauthn_response, }), }, diff --git a/web/packages/teleport/src/services/api/api.ts b/web/packages/teleport/src/services/api/api.ts index 1048c3333e11c..02f1c4ffbb21c 100644 --- a/web/packages/teleport/src/services/api/api.ts +++ b/web/packages/teleport/src/services/api/api.ts @@ -237,8 +237,8 @@ const api = { * If customOptions field is not provided, only fields defined in * `defaultRequestOptions` will be used. * - * @param webauthnResponse if defined (eg: `fetchJsonWithMfaAuthnRetry`) - * will add a custom MFA header field that will hold the webauthn response. + * @param mfaResponse if defined (eg: `fetchJsonWithMfaAuthnRetry`) + * will add a custom MFA header field that will hold the mfaResponse. */ fetch( url: string, @@ -258,7 +258,9 @@ const api = { if (mfaResponse) { options.headers[MFA_HEADER] = JSON.stringify({ - // TODO(Joerger): Handle non-webauthn response. + ...mfaResponse, + // TODO(Joerger): DELETE IN v19.0.0. + // We include webauthnAssertionResponse for backwards compatibility. webauthnAssertionResponse: mfaResponse.webauthn_response, }); } diff --git a/web/packages/teleport/src/services/apps/apps.ts b/web/packages/teleport/src/services/apps/apps.ts index d64f37414a872..268a48915aa2b 100644 --- a/web/packages/teleport/src/services/apps/apps.ts +++ b/web/packages/teleport/src/services/apps/apps.ts @@ -16,11 +16,13 @@ * along with this program. If not, see . */ -import api from 'teleport/services/api'; -import cfg, { UrlAppParams, UrlResourcesParams } from 'teleport/config'; +import cfg, { + CreateAppSessionParams, + UrlAppParams, + UrlResourcesParams, +} from 'teleport/config'; import { ResourcesResponse } from 'teleport/services/agents'; - -import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; +import api from 'teleport/services/api'; import makeApp from './makeApps'; import { App } from './types'; @@ -41,31 +43,14 @@ const service = { }); }, - async createAppSession(params: UrlAppParams) { - const resolveApp = { - fqdn: params.fqdn, - cluster_name: params.clusterId, - public_addr: params.publicAddr, - }; - - // Prompt for MFA if per-session MFA is required for this app. - const challenge = await auth.getMfaChallenge({ - scope: MfaChallengeScope.USER_SESSION, - allowReuse: false, - isMfaRequiredRequest: { - app: resolveApp, - }, - }); - - const resp = await auth.getMfaChallengeResponse(challenge); - + async createAppSession(params: CreateAppSessionParams) { const createAppSession = { - ...resolveApp, - arn: params.arn, - // TODO(Joerger): Handle non-webauthn response. - mfa_response: resp + ...params, + // TODO(Joerger): DELETE IN v19.0.0. + // We include a string version of the MFA response for backwards compatibility. + mfa_response: params.mfaResponse ? JSON.stringify({ - webauthnAssertionResponse: resp.webauthn_response, + webauthnAssertionResponse: params.mfaResponse.webauthn_response, }) : null, }; diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index 3724f1dc8b056..6480e41931ff2 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -23,6 +23,7 @@ import { DeviceUsage, MfaAuthenticateChallenge, MfaChallengeResponse, + SsoChallenge, } from 'teleport/services/mfa'; import { CaptureEvent, userEventService } from 'teleport/services/userEvent'; @@ -289,6 +290,8 @@ const auth = { mfaType = 'totp'; } else if (challenge.webauthnPublicKey) { mfaType = 'webauthn'; + } else if (challenge.ssoChallenge) { + mfaType = 'sso'; } } @@ -296,6 +299,10 @@ const auth = { return auth.getWebAuthnChallengeResponse(challenge.webauthnPublicKey); } + if (mfaType === 'sso') { + return auth.getSsoChallengeResponse(challenge.ssoChallenge); + } + if (mfaType === 'totp') { return { totp_code: totpCode, @@ -333,6 +340,51 @@ const auth = { }); }, + // TODO(Joerger): Delete once no longer used by /e + async getSsoChallengeResponse( + challenge: SsoChallenge + ): Promise { + const abortController = new AbortController(); + + auth.openSsoChallengeRedirect(challenge, abortController); + return await auth.waitForSsoChallengeResponse( + challenge, + abortController.signal + ); + }, + + openSsoChallengeRedirect( + { redirectUrl }: SsoChallenge, + abortController?: AbortController + ) { + // try to center the screen + const width = 1045; + const height = 550; + const left = (screen.width - width) / 2; + const top = (screen.height - height) / 2; + + // these params will open a tiny window. + const params = `width=${width},height=${height},left=${left},top=${top}`; + const w = window.open(redirectUrl, '_blank', params); + + // If the redirect URL window is closed prematurely, abort. + w.onclose = abortController?.abort; + }, + + async waitForSsoChallengeResponse( + { channelId, requestId }: SsoChallenge, + abortSignal: AbortSignal + ): Promise { + const channel = new BroadcastChannel(channelId); + const msg = await waitForMessage(channel, abortSignal); + return { + sso_response: { + requestId, + token: msg.data.mfaToken, + }, + }; + }, + // TODO(Joerger): Delete once no longer used by /e createPrivilegeTokenWithWebauthn() { return auth @@ -430,6 +482,30 @@ function base64EncodeUnicode(str: string) { ); } +function waitForMessage( + channel: BroadcastChannel, + abortSignal: AbortSignal +): Promise { + return new Promise((resolve, reject) => { + // Create the event listener + function eventHandler(e: MessageEvent) { + // Remove the event listener after it triggers + channel.removeEventListener('message', eventHandler); + // Resolve the promise with the event object + resolve(e); + } + + // Add the event listener + channel.addEventListener('message', eventHandler); + + // Close the event listener early if aborted. + abortSignal.onabort = e => { + channel.removeEventListener('message', eventHandler); + reject(e); + }; + }); +} + export default auth; export type IsMfaRequiredRequest = diff --git a/web/packages/teleport/src/services/mfa/mfaOptions.ts b/web/packages/teleport/src/services/mfa/mfaOptions.ts index 96510d31e668f..283feb83eb71f 100644 --- a/web/packages/teleport/src/services/mfa/mfaOptions.ts +++ b/web/packages/teleport/src/services/mfa/mfaOptions.ts @@ -18,7 +18,7 @@ import { Auth2faType } from 'shared/services'; -import { DeviceType, MfaAuthenticateChallenge, SSOChallenge } from './types'; +import { DeviceType, MfaAuthenticateChallenge, SsoChallenge } from './types'; // returns mfa challenge options in order of preferences: WebAuthn > SSO > TOTP. export function getMfaChallengeOptions(mfaChallenge: MfaAuthenticateChallenge) { @@ -74,7 +74,7 @@ export const MFA_OPTION_SSO_DEFAULT: MfaOption = { label: 'SSO', }; -const getSsoMfaOption = (ssoChallenge: SSOChallenge): MfaOption => { +const getSsoMfaOption = (ssoChallenge: SsoChallenge): MfaOption => { return { value: 'sso', label: diff --git a/web/packages/teleport/src/services/mfa/types.ts b/web/packages/teleport/src/services/mfa/types.ts index 382d7831f82fe..f8c0787544d08 100644 --- a/web/packages/teleport/src/services/mfa/types.ts +++ b/web/packages/teleport/src/services/mfa/types.ts @@ -51,7 +51,7 @@ export type SaveNewHardwareDeviceRequest = { }; export type MfaAuthenticateChallengeJson = { - sso_challenge?: SSOChallenge; + sso_challenge?: SsoChallenge; totp_challenge?: boolean; webauthn_challenge?: { publicKey: PublicKeyCredentialRequestOptionsJSON; @@ -59,12 +59,12 @@ export type MfaAuthenticateChallengeJson = { }; export type MfaAuthenticateChallenge = { - ssoChallenge?: SSOChallenge; + ssoChallenge?: SsoChallenge; totpChallenge?: boolean; webauthnPublicKey?: PublicKeyCredentialRequestOptions; }; -export type SSOChallenge = { +export type SsoChallenge = { channelId: string; redirectUrl: string; requestId: string;