diff --git a/e b/e index 37c824d54ecfb..c49f24872778b 160000 --- a/e +++ b/e @@ -1 +1 @@ -Subproject commit 37c824d54ecfbc951412ee8542daab802b93fa75 +Subproject commit c49f24872778b6fa4a690364ce4592564347f551 diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 43e4c7bd179bd..5733e8b7c3754 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -920,7 +920,10 @@ func (h *Handler) bindDefaultEndpoints() { // MFA private endpoints. h.GET("/webapi/mfa/devices", h.WithAuth(h.getMFADevicesHandle)) + h.DELETE("/webapi/mfa/devices", h.WithAuth(h.deleteMFADeviceHandle)) h.POST("/webapi/mfa/authenticatechallenge", h.WithAuth(h.createAuthenticateChallengeHandle)) + h.POST("/webapi/mfa/registerchallenge", h.WithAuth(h.createRegisterChallengeHandle)) + h.POST("/webapi/mfa/devices", h.WithAuth(h.addMFADeviceHandle)) // DEPRECATED in favor of mfa/authenticatechallenge. // TODO(bl-nero): DELETE IN 17.0.0 diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index 1925f67e964c2..e8bc808c3d2be 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -5251,24 +5251,24 @@ func TestCreateRegisterChallenge(t *testing.T) { tests := []struct { name string - req *createRegisterChallengeRequest + req *createRegisterChallengeWithTokenRequest assertChallenge func(t *testing.T, c *client.MFARegisterChallenge) }{ { name: "totp", - req: &createRegisterChallengeRequest{ + req: &createRegisterChallengeWithTokenRequest{ DeviceType: "totp", }, }, { name: "webauthn", - req: &createRegisterChallengeRequest{ + req: &createRegisterChallengeWithTokenRequest{ DeviceType: "webauthn", }, }, { name: "passwordless", - req: &createRegisterChallengeRequest{ + req: &createRegisterChallengeWithTokenRequest{ DeviceType: "webauthn", DeviceUsage: "passwordless", }, diff --git a/lib/web/mfa.go b/lib/web/mfa.go index 9116966a9ed40..2ab9bfa281636 100644 --- a/lib/web/mfa.go +++ b/lib/web/mfa.go @@ -75,6 +75,41 @@ func (h *Handler) deleteMFADeviceWithTokenHandle(w http.ResponseWriter, r *http. return OK(), nil } +type deleteMfaDeviceRequest struct { + // DeviceName is the name of the device to delete. + DeviceName string `json:"deviceName"` + // ExistingMFAResponse is an MFA challenge response from an existing device. + // Not required if the user has no existing devices. + ExistingMFAResponse *client.MFAChallengeResponse `json:"existingMfaResponse"` +} + +// deleteMFADeviceHandle deletes an mfa device for the user defined in the `token`, given as a query parameter. +func (h *Handler) deleteMFADeviceHandle(w http.ResponseWriter, r *http.Request, p httprouter.Params, c *SessionContext) (interface{}, error) { + var req deleteMfaDeviceRequest + if err := httplib.ReadJSON(r, &req); err != nil { + return nil, trace.Wrap(err) + } + + mfaResponse, err := req.ExistingMFAResponse.GetOptionalMFAResponseProtoReq() + if err != nil { + return nil, trace.Wrap(err) + } + + clt, err := c.GetClient() + if err != nil { + return nil, trace.Wrap(err) + } + + if err := clt.DeleteMFADeviceSync(r.Context(), &proto.DeleteMFADeviceSyncRequest{ + DeviceName: req.DeviceName, + ExistingMFAResponse: mfaResponse, + }); err != nil { + return nil, trace.Wrap(err) + } + + return OK(), nil +} + type addMFADeviceRequest struct { // PrivilegeTokenID is privilege token id. PrivilegeTokenID string `json:"tokenId"` @@ -203,7 +238,7 @@ func (h *Handler) createAuthenticateChallengeWithTokenHandle(w http.ResponseWrit return makeAuthenticateChallenge(chal), nil } -type createRegisterChallengeRequest struct { +type createRegisterChallengeWithTokenRequest struct { // DeviceType is the type of MFA device to get a register challenge for. DeviceType string `json:"deviceType"` // DeviceUsage is the intended usage of the device (MFA, Passwordless, etc). @@ -214,7 +249,7 @@ type createRegisterChallengeRequest struct { // createRegisterChallengeWithTokenHandle creates and returns MFA register challenges for a new device for the specified device type. func (h *Handler) createRegisterChallengeWithTokenHandle(w http.ResponseWriter, r *http.Request, p httprouter.Params) (interface{}, error) { - var req createRegisterChallengeRequest + var req createRegisterChallengeWithTokenRequest if err := httplib.ReadJSON(r, &req); err != nil { return nil, trace.Wrap(err) } @@ -256,6 +291,72 @@ func (h *Handler) createRegisterChallengeWithTokenHandle(w http.ResponseWriter, return resp, nil } +type createRegisterChallengeRequest struct { + // DeviceType is the type of MFA device to get a register challenge for. + DeviceType string `json:"deviceType"` + // DeviceUsage is the intended usage of the device (MFA, Passwordless, etc). + // It mimics the proto.DeviceUsage enum. + // Defaults to MFA. + DeviceUsage string `json:"deviceUsage"` + // ExistingMFAResponse is an MFA challenge response from an existing device. + // Not required if the user has no existing devices. + ExistingMFAResponse *client.MFAChallengeResponse `json:"existingMfaResponse"` +} + +// createRegisterChallengeHandle creates and returns MFA register challenges for a new device for the specified device type. +func (h *Handler) createRegisterChallengeHandle(w http.ResponseWriter, r *http.Request, p httprouter.Params, c *SessionContext) (interface{}, error) { + var req createRegisterChallengeRequest + if err := httplib.ReadJSON(r, &req); err != nil { + return nil, trace.Wrap(err) + } + + var deviceType proto.DeviceType + switch req.DeviceType { + case "totp": + deviceType = proto.DeviceType_DEVICE_TYPE_TOTP + case "webauthn": + deviceType = proto.DeviceType_DEVICE_TYPE_WEBAUTHN + default: + return nil, trace.BadParameter("MFA device type %q unsupported", req.DeviceType) + } + + deviceUsage, err := getDeviceUsage(req.DeviceUsage) + if err != nil { + return nil, trace.Wrap(err) + } + + mfaResponse, err := req.ExistingMFAResponse.GetOptionalMFAResponseProtoReq() + if err != nil { + return nil, trace.Wrap(err) + } + + clt, err := c.GetClient() + if err != nil { + return nil, trace.Wrap(err) + } + + chal, err := clt.CreateRegisterChallenge(r.Context(), &proto.CreateRegisterChallengeRequest{ + DeviceType: deviceType, + DeviceUsage: deviceUsage, + ExistingMFAResponse: mfaResponse, + }) + if err != nil { + return nil, trace.Wrap(err) + } + + resp := &client.MFARegisterChallenge{} + switch chal.GetRequest().(type) { + case *proto.MFARegisterChallenge_TOTP: + resp.TOTP = &client.TOTPRegisterChallenge{ + QRCode: chal.GetTOTP().GetQRCode(), + } + case *proto.MFARegisterChallenge_Webauthn: + resp.Webauthn = wantypes.CredentialCreationFromProto(chal.GetWebauthn()) + } + + return resp, nil +} + func getDeviceUsage(reqUsage string) (proto.DeviceUsage, error) { var deviceUsage proto.DeviceUsage switch strings.ToLower(reqUsage) { diff --git a/lib/web/users.go b/lib/web/users.go index 88fa8907bafce..f96d0b4405e10 100644 --- a/lib/web/users.go +++ b/lib/web/users.go @@ -30,6 +30,7 @@ import ( "github.com/gravitational/teleport/api/mfa" "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/web/ui" ) @@ -251,10 +252,15 @@ func deleteUser(r *http.Request, params httprouter.Params, m userAPIGetter, user } type privilegeTokenRequest struct { + // TODO(Joerger): DELETE IN v19.0.0 in favor of ExistingMFAResponse // SecondFactorToken is the totp code. SecondFactorToken string `json:"secondFactorToken"` + // TODO(Joerger): DELETE IN v19.0.0 in favor of ExistingMFAResponse // WebauthnResponse is the response from authenticators. WebauthnResponse *wantypes.CredentialAssertionResponse `json:"webauthnAssertionResponse"` + // ExistingMFAResponse is an MFA challenge response from an existing device. + // Not required if the user has no existing devices. + ExistingMFAResponse *client.MFAChallengeResponse `json:"existingMfaResponse"` } // createPrivilegeTokenHandle creates and returns a privilege token. @@ -275,6 +281,12 @@ func (h *Handler) createPrivilegeTokenHandle(w http.ResponseWriter, r *http.Requ protoReq.ExistingMFAResponse = &proto.MFAAuthenticateResponse{Response: &proto.MFAAuthenticateResponse_Webauthn{ Webauthn: wantypes.CredentialAssertionResponseToProto(req.WebauthnResponse), }} + case req.ExistingMFAResponse != nil: + var err error + protoReq.ExistingMFAResponse, err = req.ExistingMFAResponse.GetOptionalMFAResponseProtoReq() + if err != nil { + return nil, trace.Wrap(err) + } default: // Can be empty, which means user did not have a second factor registered. } diff --git a/web/packages/teleport/src/Account/Account.story.tsx b/web/packages/teleport/src/Account/Account.story.tsx index 00cee6454d8f1..2710c711d1741 100644 --- a/web/packages/teleport/src/Account/Account.story.tsx +++ b/web/packages/teleport/src/Account/Account.story.tsx @@ -87,7 +87,7 @@ export const LoadingDevicesFailed = () => ( ); export const RemoveDialog = () => ( - + ); export const RestrictedTokenCreateProcessing = () => ( @@ -110,7 +110,6 @@ export const RestrictedTokenCreateFailed = () => ( ); const props: AccountProps = { - token: '', onAddDevice: () => null, fetchDevicesAttempt: { status: 'success' }, createRestrictedTokenAttempt: { status: '' }, @@ -179,7 +178,6 @@ const props: AccountProps = { }, ], onDeviceAdded: () => {}, - isReauthenticationRequired: false, addDeviceWizardVisible: false, closeAddDeviceWizard: () => {}, passwordState: PasswordState.PASSWORD_STATE_SET, diff --git a/web/packages/teleport/src/Account/Account.test.tsx b/web/packages/teleport/src/Account/Account.test.tsx index 3ca45002ffaee..e8e3ae37b9ea0 100644 --- a/web/packages/teleport/src/Account/Account.test.tsx +++ b/web/packages/teleport/src/Account/Account.test.tsx @@ -16,8 +16,8 @@ * along with this program. If not, see . */ -import { render, screen, waitFor } from 'design/utils/testing'; import { within } from '@testing-library/react'; +import { render, screen, waitFor } from 'design/utils/testing'; import userEvent from '@testing-library/user-event'; @@ -27,9 +27,9 @@ import TeleportContext from 'teleport/teleportContext'; import { AccountPage as Account } from 'teleport/Account/Account'; import cfg from 'teleport/config'; import { createTeleportContext } from 'teleport/mocks/contexts'; -import { PasswordState } from 'teleport/services/user'; import auth from 'teleport/services/auth/auth'; import MfaService, { MfaDevice } from 'teleport/services/mfa'; +import { PasswordState } from 'teleport/services/user'; const defaultAuthType = cfg.auth.second_factor; const defaultPasswordless = cfg.auth.allowPasswordless; @@ -189,6 +189,14 @@ test('password change', async () => { const ctx = createTeleportContext(); ctx.storeUser.setState({ passwordState: PasswordState.PASSWORD_STATE_UNSET }); jest.spyOn(ctx.mfaService, 'fetchDevices').mockResolvedValue([]); + jest.spyOn(auth, 'getMfaChallenge').mockResolvedValue({ + webauthnPublicKey: {} as PublicKeyCredentialRequestOptions, + totpChallenge: true, + }); + jest.spyOn(auth, 'getMfaChallengeResponse').mockResolvedValueOnce({}); + jest + .spyOn(auth, 'createPrivilegeToken') + .mockResolvedValueOnce('privilege-token'); jest.spyOn(auth, 'changePassword').mockResolvedValueOnce(undefined); await renderComponent(ctx); @@ -198,6 +206,9 @@ test('password change', async () => { // Change the password await user.click(screen.getByRole('button', { name: 'Change Password' })); + await waitFor(async () => { + await user.click(screen.getByRole('button', { name: 'Next' })); + }); await user.type(screen.getByLabelText('Current Password'), 'old-password'); await user.type(screen.getByLabelText('New Password'), 'asdfasdfasdfasdf'); await user.type( @@ -231,8 +242,8 @@ test('loading state', async () => { expect( within(screen.getByTestId('mfa-list')).getByTestId('indicator-wrapper') ).toBeVisible(); - expect(screen.getByText(/add a passkey/i)).toBeDisabled(); - expect(screen.getByText(/add mfa/i)).toBeDisabled(); + expect(screen.getByText(/add a passkey/i)).toBeVisible(); + expect(screen.getByText(/add mfa/i)).toBeVisible(); expect( screen.queryByTestId('passwordless-state-pill') ).not.toBeInTheDocument(); @@ -244,10 +255,11 @@ test('adding an MFA device', async () => { const ctx = createTeleportContext(); jest.spyOn(ctx.mfaService, 'fetchDevices').mockResolvedValue([testPasskey]); jest.spyOn(auth, 'getMfaChallenge').mockResolvedValue({ - webauthnPublicKey: null, + webauthnPublicKey: {} as PublicKeyCredentialRequestOptions, totpChallenge: true, ssoChallenge: null, }); + jest.spyOn(auth, 'getMfaChallengeResponse').mockResolvedValueOnce({}); jest .spyOn(auth, 'createNewWebAuthnDevice') .mockResolvedValueOnce(dummyCredential); @@ -255,16 +267,22 @@ test('adding an MFA device', async () => { .spyOn(MfaService.prototype, 'saveNewWebAuthnDevice') .mockResolvedValueOnce(undefined); jest - .spyOn(auth, 'createPrivilegeTokenWithWebauthn') - .mockResolvedValueOnce('webauthn-privilege-token'); + .spyOn(auth, 'createPrivilegeToken') + .mockResolvedValueOnce('privilege-token'); cfg.auth.second_factor = 'on'; await renderComponent(ctx); await user.click(screen.getByRole('button', { name: 'Add MFA' })); - await user.click(screen.getByRole('button', { name: 'Verify my identity' })); - await user.click( - screen.getByRole('button', { name: 'Create an MFA method' }) - ); + await waitFor(async () => { + await user.click( + screen.getByRole('button', { name: 'Verify my identity' }) + ); + }); + await waitFor(async () => { + await user.click( + screen.getByRole('button', { name: 'Create an MFA method' }) + ); + }); await user.type(screen.getByLabelText('MFA Method Name'), 'new-mfa'); // The final assertion can be accidentally made irrelevant if the button name @@ -276,7 +294,7 @@ test('adding an MFA device', async () => { addRequest: { deviceName: 'new-mfa', deviceUsage: 'mfa', - tokenId: 'webauthn-privilege-token', + tokenId: 'privilege-token', }, }); expect( @@ -288,6 +306,12 @@ test('adding a passkey', async () => { const user = userEvent.setup(); const ctx = createTeleportContext(); jest.spyOn(ctx.mfaService, 'fetchDevices').mockResolvedValue([testMfaMethod]); + jest.spyOn(auth, 'getMfaChallenge').mockResolvedValue({ + webauthnPublicKey: {} as PublicKeyCredentialRequestOptions, + totpChallenge: true, + ssoChallenge: null, + }); + jest.spyOn(auth, 'getMfaChallengeResponse').mockResolvedValueOnce({}); jest .spyOn(auth, 'createNewWebAuthnDevice') .mockResolvedValueOnce(dummyCredential); @@ -295,14 +319,18 @@ test('adding a passkey', async () => { .spyOn(MfaService.prototype, 'saveNewWebAuthnDevice') .mockResolvedValueOnce(undefined); jest - .spyOn(auth, 'createPrivilegeTokenWithWebauthn') - .mockResolvedValueOnce('webauthn-privilege-token'); + .spyOn(auth, 'createPrivilegeToken') + .mockResolvedValueOnce('privilege-token'); cfg.auth.second_factor = 'on'; cfg.auth.allowPasswordless = true; await renderComponent(ctx); await user.click(screen.getByRole('button', { name: 'Add a Passkey' })); - await user.click(screen.getByRole('button', { name: 'Verify my identity' })); + await waitFor(async () => { + await user.click( + screen.getByRole('button', { name: 'Verify my identity' }) + ); + }); await user.click(screen.getByRole('button', { name: 'Create a passkey' })); await user.type(screen.getByLabelText('Passkey Nickname'), 'new-passkey'); @@ -315,7 +343,7 @@ test('adding a passkey', async () => { addRequest: { deviceName: 'new-passkey', deviceUsage: 'passwordless', - tokenId: 'webauthn-privilege-token', + tokenId: 'privilege-token', }, }); expect( @@ -328,13 +356,14 @@ test('removing an MFA method', async () => { const ctx = createTeleportContext(); jest.spyOn(ctx.mfaService, 'fetchDevices').mockResolvedValue([testMfaMethod]); jest.spyOn(auth, 'getMfaChallenge').mockResolvedValue({ - webauthnPublicKey: null, - totpChallenge: false, + webauthnPublicKey: {} as PublicKeyCredentialRequestOptions, + totpChallenge: true, ssoChallenge: null, }); + jest.spyOn(auth, 'getMfaChallengeResponse').mockResolvedValueOnce({}); jest - .spyOn(auth, 'createPrivilegeTokenWithWebauthn') - .mockResolvedValueOnce('webauthn-privilege-token'); + .spyOn(auth, 'createPrivilegeToken') + .mockResolvedValueOnce('privilege-token'); jest .spyOn(MfaService.prototype, 'removeDevice') .mockResolvedValueOnce(undefined); @@ -352,7 +381,7 @@ test('removing an MFA method', async () => { await user.click(deleteStep.getByRole('button', { name: 'Delete' })); expect(ctx.mfaService.removeDevice).toHaveBeenCalledWith( - 'webauthn-privilege-token', + 'privilege-token', 'touch_id' ); expect(screen.queryByTestId('delete-step')).not.toBeInTheDocument(); diff --git a/web/packages/teleport/src/Account/Account.tsx b/web/packages/teleport/src/Account/Account.tsx index 9cd8c47720e2b..db0fbff3d46a6 100644 --- a/web/packages/teleport/src/Account/Account.tsx +++ b/web/packages/teleport/src/Account/Account.tsx @@ -105,14 +105,12 @@ export interface AccountProps extends ManageDevicesState, AccountPageProps { export function Account({ devices, - token, onAddDevice, onRemoveDevice, onDeviceAdded, onDeviceRemoved, deviceToRemove, fetchDevicesAttempt, - createRestrictedTokenAttempt, addDeviceWizardVisible, hideRemoveDevice, closeAddDeviceWizard, @@ -127,11 +125,8 @@ export function Account({ }: AccountProps) { const passkeys = devices.filter(d => d.usage === 'passwordless'); const mfaDevices = devices.filter(d => d.usage === 'mfa'); - const disableAddDevice = - createRestrictedTokenAttempt.status === 'processing' || - fetchDevicesAttempt.status !== 'success'; - const disableAddPasskey = disableAddDevice || !canAddPasskeys; - const disableAddMfa = disableAddDevice || !canAddMfa; + const disableAddPasskey = !canAddPasskeys; + const disableAddMfa = !canAddMfa; let mfaPillState = undefined; if (fetchDevicesAttempt.status !== 'processing') { @@ -140,7 +135,6 @@ export function Account({ const [notifications, setNotifications] = useState([]); const [prevFetchStatus, setPrevFetchStatus] = useState(''); - const [prevTokenStatus, setPrevTokenStatus] = useState(''); function addNotification(severity: NotificationSeverity, content: string) { setNotifications(n => [ @@ -165,13 +159,6 @@ export function Account({ } } - if (prevTokenStatus !== createRestrictedTokenAttempt.status) { - setPrevTokenStatus(createRestrictedTokenAttempt.status); - if (createRestrictedTokenAttempt.status === 'failed') { - addNotification('error', createRestrictedTokenAttempt.statusText); - } - } - function onPasswordChange() { addNotification('info', 'Your password has been changed.'); onPasswordChangeCb(); @@ -220,9 +207,6 @@ export function Account({ {!isSso && ( @@ -287,8 +269,6 @@ export function Account({ {deviceToRemove && ( . */ -import React from 'react'; - import Dialog from 'design/Dialog'; +import { makeEmptyAttempt } from 'shared/hooks/useAsync'; -import { createTeleportContext } from 'teleport/mocks/contexts'; import { ContextProvider } from 'teleport'; +import { createTeleportContext } from 'teleport/mocks/contexts'; -import { MfaDevice, WebauthnAssertionResponse } from 'teleport/services/mfa'; +import { + MFA_OPTION_SSO_DEFAULT, + MFA_OPTION_TOTP, + MFA_OPTION_WEBAUTHN, + WebauthnAssertionResponse, +} from 'teleport/services/mfa'; + +import { ReauthState } from 'teleport/components/ReAuthenticate/useReAuthenticate'; import { ChangePasswordStep, ChangePasswordWizardStepProps, ReauthenticateStep, - createReauthOptions, } from './ChangePasswordWizard'; export default { @@ -60,44 +65,16 @@ export function ChangePasswordWithPasswordlessVerification() { } export function ChangePasswordWithMfaDeviceVerification() { - return ; + return ; } export function ChangePasswordWithOtpVerification() { - return ; + return ; } -const devices: MfaDevice[] = [ - { - id: '1', - description: 'Hardware Key', - name: 'touch_id', - registeredDate: new Date(1628799417000), - lastUsedDate: new Date(1628799417000), - type: 'webauthn', - usage: 'passwordless', - }, - { - id: '2', - description: 'Hardware Key', - name: 'solokey', - registeredDate: new Date(1623722252000), - lastUsedDate: new Date(1623981452000), - type: 'webauthn', - usage: 'mfa', - }, - { - id: '3', - description: 'Authenticator App', - name: 'iPhone', - registeredDate: new Date(1618711052000), - lastUsedDate: new Date(1626472652000), - type: 'totp', - usage: 'mfa', - }, -]; - -const defaultReauthOptions = createReauthOptions('optional', true, devices); +export function ChangePasswordWithSsoVerification() { + return ; +} const stepProps = { // StepComponentProps @@ -109,14 +86,20 @@ const stepProps = { refCallback: () => {}, // Shared props - reauthMethod: 'mfaDevice', + reauthMethod: 'passwordless', onClose() {}, onSuccess() {}, // ReauthenticateStepProps - reauthOptions: defaultReauthOptions, - onReauthMethodChange() {}, - onWebauthnResponse() {}, + hasPasswordless: true, + setReauthMethod: () => {}, + reauthState: { + initAttempt: { status: 'success' }, + mfaOptions: [MFA_OPTION_WEBAUTHN, MFA_OPTION_TOTP, MFA_OPTION_SSO_DEFAULT], + submitWithMfa: async () => null, + submitAttempt: makeEmptyAttempt(), + clearSubmitAttempt: () => {}, + } as ReauthState, // ChangePasswordStepProps webauthnResponse: {} as WebauthnAssertionResponse, diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx index d23469ead98fd..e7212af322784 100644 --- a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx +++ b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx @@ -17,18 +17,23 @@ */ import { render, screen } from 'design/utils/testing'; -import React from 'react'; -import { within } from '@testing-library/react'; +import { waitFor, within } from '@testing-library/react'; import { userEvent, UserEvent } from '@testing-library/user-event'; import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; -import { MfaChallengeResponse } from 'teleport/services/mfa'; +import { + MFA_OPTION_TOTP, + MFA_OPTION_WEBAUTHN, + MfaChallengeResponse, +} from 'teleport/services/mfa'; import { ChangePasswordWizardProps, - createReauthOptions, + getReauthOptions, + REAUTH_OPTION_PASSKEY, + REAUTH_OPTION_WEBAUTHN, } from './ChangePasswordWizard'; import { ChangePasswordWizard } from '.'; @@ -52,29 +57,10 @@ const dummyChallengeResponse: MfaChallengeResponse = { let user: UserEvent; let onSuccess: jest.Mock; -function twice(arr) { - return [...arr, ...arr]; -} - -// Repeat devices twice to make sure we support multiple devices of the same -// type and purpose. -const deviceCases = { - all: twice([ - { type: 'totp', usage: 'mfa' }, - { type: 'webauthn', usage: 'mfa' }, - { type: 'webauthn', usage: 'passwordless' }, - ]), - authApps: twice([{ type: 'totp', usage: 'mfa' }]), - mfaDevices: twice([{ type: 'webauthn', usage: 'mfa' }]), - passkeys: twice([{ type: 'webauthn', usage: 'passwordless' }]), -}; - function TestWizard(props: Partial = {}) { return ( {}} onSuccess={onSuccess} {...props} @@ -86,7 +72,10 @@ beforeEach(() => { user = userEvent.setup(); onSuccess = jest.fn(); - jest.spyOn(auth, 'getMfaChallenge').mockResolvedValueOnce(undefined); + jest.spyOn(auth, 'getMfaChallenge').mockResolvedValue({ + totpChallenge: true, + webauthnPublicKey: {} as PublicKeyCredentialRequestOptions, + }); jest .spyOn(auth, 'getMfaChallengeResponse') .mockResolvedValueOnce(dummyChallengeResponse); @@ -99,11 +88,15 @@ describe('with passwordless reauthentication', () => { async function reauthenticate() { render(); - const reauthenticateStep = within( - screen.getByTestId('reauthenticate-step') - ); - await user.click(reauthenticateStep.getByText('Passkey')); - await user.click(reauthenticateStep.getByText('Next')); + await waitFor(() => { + expect(screen.getByTestId('reauthenticate-step')).toBeInTheDocument(); + }); + expect(auth.getMfaChallenge).toHaveBeenCalledWith({ + scope: MfaChallengeScope.CHANGE_PASSWORD, + }); + + await user.click(screen.getByText('Passkey')); + await user.click(screen.getByText('Next')); expect(auth.getMfaChallenge).toHaveBeenCalledWith({ scope: MfaChallengeScope.CHANGE_PASSWORD, userVerificationRequirement: 'required', @@ -128,8 +121,10 @@ describe('with passwordless reauthentication', () => { expect(auth.changePassword).toHaveBeenCalledWith({ oldPassword: '', newPassword: 'new-pass1234', - secondFactorToken: '', - webauthnResponse: dummyChallengeResponse.webauthn_response, + mfaResponse: { + totp_code: '', + webauthn_response: dummyChallengeResponse.webauthn_response, + }, }); expect(onSuccess).toHaveBeenCalled(); }); @@ -191,15 +186,16 @@ describe('with WebAuthn MFA reauthentication', () => { async function reauthenticate() { render(); - const reauthenticateStep = within( - screen.getByTestId('reauthenticate-step') - ); - await user.click(reauthenticateStep.getByText('MFA Device')); - await user.click(reauthenticateStep.getByText('Next')); + await waitFor(() => { + expect(screen.getByTestId('reauthenticate-step')).toBeInTheDocument(); + }); expect(auth.getMfaChallenge).toHaveBeenCalledWith({ scope: MfaChallengeScope.CHANGE_PASSWORD, - userVerificationRequirement: 'discouraged', }); + + await user.click(screen.getByText('Security Key')); + await user.click(screen.getByText('Next')); + expect(auth.getMfaChallengeResponse).toHaveBeenCalled(); } it('changes password', async () => { @@ -223,8 +219,10 @@ describe('with WebAuthn MFA reauthentication', () => { expect(auth.changePassword).toHaveBeenCalledWith({ oldPassword: 'current-pass', newPassword: 'new-pass1234', - secondFactorToken: '', - webauthnResponse: dummyChallengeResponse.webauthn_response, + mfaResponse: { + totp_code: '', + webauthn_response: dummyChallengeResponse.webauthn_response, + }, }); expect(onSuccess).toHaveBeenCalled(); }); @@ -293,12 +291,11 @@ describe('with OTP MFA reauthentication', () => { async function reauthenticate() { render(); - const reauthenticateStep = within( - screen.getByTestId('reauthenticate-step') - ); - await user.click(reauthenticateStep.getByText('Authenticator App')); - await user.click(reauthenticateStep.getByText('Next')); - expect(auth.getMfaChallenge).not.toHaveBeenCalled(); + await waitFor(() => { + expect(screen.getByTestId('reauthenticate-step')).toBeInTheDocument(); + }); + await user.click(screen.getByText('Authenticator App')); + await user.click(screen.getByText('Next')); } it('changes password', async () => { @@ -326,7 +323,9 @@ describe('with OTP MFA reauthentication', () => { expect(auth.changePassword).toHaveBeenCalledWith({ oldPassword: 'current-pass', newPassword: 'new-pass1234', - secondFactorToken: '654321', + mfaResponse: { + totp_code: '654321', + }, }); expect(onSuccess).toHaveBeenCalled(); }); @@ -402,121 +401,17 @@ describe('with OTP MFA reauthentication', () => { }); }); -describe('without reauthentication', () => { - it('changes password', async () => { - render(); - - const changePasswordStep = within( - screen.getByTestId('change-password-step') - ); - await user.type( - changePasswordStep.getByLabelText('Current Password'), - 'current-pass' - ); - await user.type( - changePasswordStep.getByLabelText('New Password'), - 'new-pass1234' - ); - await user.type( - changePasswordStep.getByLabelText('Confirm Password'), - 'new-pass1234' - ); - await user.click(changePasswordStep.getByText('Save Changes')); - expect(auth.getMfaChallenge).not.toHaveBeenCalled(); - expect(auth.changePassword).toHaveBeenCalledWith({ - oldPassword: 'current-pass', - newPassword: 'new-pass1234', - webauthnResponse: undefined, - secondFactorToken: '', - }); - expect(onSuccess).toHaveBeenCalled(); - }); - - it('cancels changing password', async () => { - render(); - - const changePasswordStep = within( - screen.getByTestId('change-password-step') - ); - await user.type( - changePasswordStep.getByLabelText('New Password'), - 'new-pass1234' - ); - await user.type( - changePasswordStep.getByLabelText('Confirm Password'), - 'new-pass1234' - ); - await user.click(changePasswordStep.getByText('Cancel')); - expect(auth.changePassword).not.toHaveBeenCalled(); - expect(onSuccess).not.toHaveBeenCalled(); - }); - - it('validates the password form', async () => { - render(); - - const changePasswordStep = within( - screen.getByTestId('change-password-step') - ); - await user.type( - changePasswordStep.getByLabelText('New Password'), - 'new-pass123' - ); - await user.type( - changePasswordStep.getByLabelText('Confirm Password'), - 'new-pass123' - ); - await user.click(changePasswordStep.getByText('Save Changes')); - expect(auth.changePassword).not.toHaveBeenCalled(); - expect(onSuccess).not.toHaveBeenCalled(); - expect(changePasswordStep.getByLabelText('New Password')).toBeInvalid(); - expect( - changePasswordStep.getByLabelText('New Password') - ).toHaveAccessibleDescription('Enter at least 12 characters'); - expect(changePasswordStep.getByLabelText('Current Password')).toBeInvalid(); - expect( - changePasswordStep.getByLabelText('Current Password') - ).toHaveAccessibleDescription('Current Password is required'); - - await user.type( - changePasswordStep.getByLabelText('Current Password'), - 'current-pass' - ); - await user.type( - changePasswordStep.getByLabelText('New Password'), - 'new-pass1234' - ); - await user.click(changePasswordStep.getByText('Save Changes')); - expect(auth.changePassword).not.toHaveBeenCalled(); - expect(onSuccess).not.toHaveBeenCalled(); - expect(changePasswordStep.getByLabelText('Confirm Password')).toBeInvalid(); - expect( - changePasswordStep.getByLabelText('Confirm Password') - ).toHaveAccessibleDescription('Password does not match'); - }); -}); - test.each` - auth2faType | passwordless | deviceCase | methods - ${'otp'} | ${false} | ${'all'} | ${['otp']} - ${'off'} | ${false} | ${'all'} | ${[]} - ${'optional'} | ${false} | ${'all'} | ${['mfaDevice', 'otp']} - ${'on'} | ${false} | ${'all'} | ${['mfaDevice', 'otp']} - ${'webauthn'} | ${false} | ${'all'} | ${['mfaDevice']} - ${'optional'} | ${true} | ${'all'} | ${['passwordless', 'mfaDevice', 'otp']} - ${'on'} | ${true} | ${'all'} | ${['passwordless', 'mfaDevice', 'otp']} - ${'webauthn'} | ${true} | ${'all'} | ${['passwordless', 'mfaDevice']} - ${'optional'} | ${true} | ${'authApps'} | ${['otp']} - ${'optional'} | ${true} | ${'mfaDevices'} | ${['mfaDevice']} - ${'optional'} | ${true} | ${'passkeys'} | ${['passwordless']} + mfaOptions | hasPasswordless | reauthOptions + ${[MFA_OPTION_TOTP]} | ${false} | ${[MFA_OPTION_TOTP]} + ${[MFA_OPTION_WEBAUTHN]} | ${false} | ${[REAUTH_OPTION_WEBAUTHN]} + ${[MFA_OPTION_TOTP, MFA_OPTION_WEBAUTHN]} | ${false} | ${[MFA_OPTION_TOTP, REAUTH_OPTION_WEBAUTHN]} + ${[MFA_OPTION_WEBAUTHN]} | ${true} | ${[REAUTH_OPTION_PASSKEY, REAUTH_OPTION_WEBAUTHN]} + ${[MFA_OPTION_TOTP, MFA_OPTION_WEBAUTHN]} | ${true} | ${[REAUTH_OPTION_PASSKEY, MFA_OPTION_TOTP, REAUTH_OPTION_WEBAUTHN]} `( - 'createReauthOptions: auth2faType=$auth2faType, passwordless=$passwordless, devices=$deviceCase', - ({ auth2faType, passwordless, methods, deviceCase }) => { - const devices = deviceCases[deviceCase]; - const reauthMethods = createReauthOptions( - auth2faType, - passwordless, - devices - ).map(o => o.value); - expect(reauthMethods).toEqual(methods); + 'getReauthOptions: mfaOptions=$mfaOptions, hasPasswordless=$hasPasswordless, devices=$deviceCase', + ({ mfaOptions, hasPasswordless, reauthOptions }) => { + const gotReauthOptions = getReauthOptions(mfaOptions, hasPasswordless); + expect(gotReauthOptions).toEqual(reauthOptions); } ); diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx index 3a66e65a070ce..a5eb1e8fc7204 100644 --- a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx +++ b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx @@ -16,14 +16,13 @@ * along with this program. If not, see . */ -import styled from 'styled-components'; -import { OutlineDanger } from 'design/Alert/Alert'; +import { Alert, OutlineDanger } from 'design/Alert/Alert'; import { ButtonPrimary, ButtonSecondary } from 'design/Button'; import Dialog from 'design/Dialog'; import Flex from 'design/Flex'; import { RadioGroup } from 'design/RadioGroup'; -import { StepComponentProps, StepSlider, StepHeader } from 'design/StepSlider'; -import React, { useState } from 'react'; +import { StepComponentProps, StepHeader, StepSlider } from 'design/StepSlider'; +import React, { useCallback, useEffect, useState } from 'react'; import FieldInput from 'shared/components/FieldInput'; import Validation, { Validator } from 'shared/components/Validation'; import { @@ -32,43 +31,44 @@ import { requiredPassword, } from 'shared/components/Validation/rules'; import { useAsync } from 'shared/hooks/useAsync'; -import { Auth2faType } from 'shared/services'; +import styled from 'styled-components'; import Box from 'design/Box'; +import Indicator from 'design/Indicator'; + +import useReAuthenticate, { + ReauthState, +} from 'teleport/components/ReAuthenticate/useReAuthenticate'; import { ChangePasswordReq } from 'teleport/services/auth'; import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; -import { MfaDevice, WebauthnAssertionResponse } from 'teleport/services/mfa'; +import { + DeviceType, + MfaOption, + WebauthnAssertionResponse, +} from 'teleport/services/mfa'; export interface ChangePasswordWizardProps { - /** MFA type setting, as configured in the cluster's configuration. */ - auth2faType: Auth2faType; - /** Determines whether the cluster allows passwordless login. */ - passwordlessEnabled: boolean; - /** A list of available authentication devices. */ - devices: MfaDevice[]; + hasPasswordless: boolean; onClose(): void; onSuccess(): void; } export function ChangePasswordWizard({ - auth2faType, - passwordlessEnabled, - devices, + hasPasswordless, onClose, onSuccess, }: ChangePasswordWizardProps) { - const reauthOptions = createReauthOptions( - auth2faType, - passwordlessEnabled, - devices - ); - const [reauthMethod, setReauthMethod] = useState( - reauthOptions[0]?.value - ); const [webauthnResponse, setWebauthnResponse] = useState(); - const reauthRequired = reauthOptions.length > 0; + + const reauthState = useReAuthenticate({ + challengeScope: MfaChallengeScope.CHANGE_PASSWORD, + onMfaResponse: async mfaResponse => + setWebauthnResponse(mfaResponse.webauthn_response), + }); + + const [reauthMethod, setReauthMethod] = useState(); return ( @@ -95,56 +93,41 @@ export function ChangePasswordWizard({ ); } -type ReauthenticationMethod = 'passwordless' | 'mfaDevice' | 'otp'; +type ReauthenticationMethod = 'passwordless' | DeviceType; type ReauthenticationOption = { value: ReauthenticationMethod; label: string; }; -export function createReauthOptions( - auth2faType: Auth2faType, - passwordlessEnabled: boolean, - devices: MfaDevice[] -) { - const options: ReauthenticationOption[] = []; - - const methodsAllowedByDevices = {}; - for (const d of devices) { - methodsAllowedByDevices[reauthMethodForDevice(d)] = true; - } - - if (passwordlessEnabled && 'passwordless' in methodsAllowedByDevices) { - options.push({ value: 'passwordless', label: 'Passkey' }); - } +export const REAUTH_OPTION_WEBAUTHN: ReauthenticationOption = { + value: 'webauthn', + label: 'Security Key', +}; - const mfaEnabled = auth2faType === 'on' || auth2faType === 'optional'; +export const REAUTH_OPTION_PASSKEY: ReauthenticationOption = { + value: 'passwordless', + label: 'Passkey', +}; - if ( - (auth2faType === 'webauthn' || mfaEnabled) && - 'mfaDevice' in methodsAllowedByDevices - ) { - options.push({ value: 'mfaDevice', label: 'MFA Device' }); - } +export function getReauthOptions( + mfaOptions: MfaOption[], + hasPasswordless: boolean +) { + // Be more specific about the WebAuthn device type (it's not a passkey). + const reauthOptions = mfaOptions.map((o: ReauthenticationOption) => + o.value === 'webauthn' ? REAUTH_OPTION_WEBAUTHN : o + ); - if ( - (auth2faType === 'otp' || mfaEnabled) && - 'otp' in methodsAllowedByDevices - ) { - options.push({ value: 'otp', label: 'Authenticator App' }); + // Add passwordless as the default if available. + if (hasPasswordless) { + reauthOptions.unshift(REAUTH_OPTION_PASSKEY); } - return options; -} - -/** Returns the reauthentication method supported by a given device. */ -function reauthMethodForDevice(d: MfaDevice): ReauthenticationMethod { - if (d.usage === 'passwordless') return 'passwordless'; - return d.type === 'totp' ? 'otp' : 'mfaDevice'; + return reauthOptions; } const wizardFlows = { withReauthentication: [ReauthenticateStep, ChangePasswordStep], - withoutReauthentication: [ChangePasswordStep], }; export type ChangePasswordWizardStepProps = StepComponentProps & @@ -152,10 +135,10 @@ export type ChangePasswordWizardStepProps = StepComponentProps & ChangePasswordStepProps; interface ReauthenticateStepProps { - reauthOptions: ReauthenticationOption[]; + hasPasswordless: boolean; reauthMethod: ReauthenticationMethod; - onReauthMethodChange(method: ReauthenticationMethod): void; - onWebauthnResponse(res: WebauthnAssertionResponse): void; + setReauthMethod(method: ReauthenticationMethod): void; + reauthState: ReauthState; onClose(): void; } @@ -164,37 +147,84 @@ export function ReauthenticateStep({ refCallback, stepIndex, flowLength, - reauthOptions, + hasPasswordless, reauthMethod, - onReauthMethodChange, - onWebauthnResponse, + setReauthMethod, + reauthState: { + initAttempt, + mfaOptions, + submitWithMfa, + clearSubmitAttempt, + submitAttempt, + }, onClose, }: ChangePasswordWizardStepProps) { - const [reauthenticateAttempt, reauthenticate] = useAsync( - async (m: ReauthenticationMethod) => { - if (m === 'passwordless' || m === 'mfaDevice') { - const challenge = await auth.getMfaChallenge({ - scope: MfaChallengeScope.CHANGE_PASSWORD, - userVerificationRequirement: - m === 'passwordless' ? 'required' : 'discouraged', - }); - - const response = await auth.getMfaChallengeResponse( - challenge, - 'webauthn' - ); - - // TODO(Joerger): handle non-webauthn response. - onWebauthnResponse(response.webauthn_response); - } - next(); - } + const [reauthOptions, initReauthOptions] = useAsync( + useCallback(async () => { + const reauthOptions = getReauthOptions(mfaOptions, hasPasswordless); + setReauthMethod(reauthOptions[0].value); + return reauthOptions; + }, [hasPasswordless, mfaOptions, setReauthMethod]) + ); + + useEffect(() => { + initReauthOptions(); + }, [initReauthOptions]); + + const reauthenticate = useCallback( + async (reauthMethod: ReauthenticationMethod) => { + // totp is handled in the ChangePasswordStep + if (reauthMethod === 'totp') return next(); + + const mfaType = + reauthMethod === 'passwordless' ? 'webauthn' : reauthMethod; + const deviceUsage = + reauthMethod === 'passwordless' ? 'passwordless' : 'mfa'; + + submitWithMfa(mfaType, deviceUsage).then(([, err]) => { + if (!err) next(); + }); + }, + [submitWithMfa, next] ); + const onReauthenticate = (e: React.FormEvent) => { e.preventDefault(); reauthenticate(reauthMethod); }; + // Handle potential error states first. + switch (initAttempt.status) { + case 'processing': + return ( + + + + ); + case 'error': + return ; + case 'success': + break; + default: + return null; + } + + // Handle potential error states first. + switch (reauthOptions.status) { + case 'processing': + return ( + + + + ); + case 'error': + return ; + case 'success': + break; + default: + return null; + } + return ( @@ -204,20 +234,23 @@ export function ReauthenticateStep({ title="Verify Identity" /> - {reauthenticateAttempt.status === 'error' && ( - {reauthenticateAttempt.statusText} + {submitAttempt.status === 'error' && ( + {submitAttempt.statusText} )} Verification Method
onReauthenticate(e)}> { + setReauthMethod(o as DeviceType); + clearSubmitAttempt(); + }} /> @@ -252,9 +285,9 @@ export function ChangePasswordStep({ const [oldPassword, setOldPassword] = useState(''); const [newPassword, setNewPassword] = useState(''); const [newPassConfirmed, setNewPassConfirmed] = useState(''); - const [authCode, setAuthCode] = useState(''); + const [otpCode, setOtpCode] = useState(''); const onAuthCodeChanged = (e: React.ChangeEvent) => { - setAuthCode(e.target.value); + setOtpCode(e.target.value); }; const [changePasswordAttempt, changePassword] = useAsync( async (req: ChangePasswordReq) => { @@ -269,7 +302,7 @@ export function ChangePasswordStep({ setOldPassword(''); setNewPassword(''); setNewPassConfirmed(''); - setAuthCode(''); + setOtpCode(''); } async function onSubmit( @@ -282,8 +315,10 @@ export function ChangePasswordStep({ await changePassword({ oldPassword, newPassword, - secondFactorToken: authCode, - webauthnResponse, + mfaResponse: { + totp_code: otpCode, + webauthn_response: webauthnResponse, + }, }); } @@ -328,14 +363,14 @@ export function ChangePasswordStep({ type="password" placeholder="Confirm Password" /> - {reauthMethod === 'otp' && ( + {reauthMethod === 'totp' && ( ([]); const [deviceToRemove, setDeviceToRemove] = useState(); - const [token, setToken] = useState(''); const fetchDevicesAttempt = useAttempt(''); const [newDeviceUsage, setNewDeviceUsage] = useState('passwordless'); @@ -38,8 +35,6 @@ export default function useManageDevices(ctx: Ctx) { // the user has no devices yet and thus can't authenticate using the ReAuthenticate dialog const createRestrictedTokenAttempt = useAttempt(''); - const isReauthenticationRequired = !token; - function fetchDevices() { fetchDevicesAttempt.run(() => ctx.mfaService.fetchDevices().then(setDevices) @@ -48,30 +43,12 @@ export default function useManageDevices(ctx: Ctx) { async function onAddDevice(usage: DeviceUsage) { setNewDeviceUsage(usage); - const challenge = await auth.getMfaChallenge({ - scope: MfaChallengeScope.MANAGE_DEVICES, - }); - // If the user doesn't receieve any challenges from the backend, that means - // they have no valid devices to be challenged and should instead use a privilege token - // to add a new device. - // TODO (avatus): add SSO challenge here as well when we add SSO for MFA - // TODO(Joerger): privilege token is no longer required to add first device. - if (!challenge) { - createRestrictedTokenAttempt.run(() => - auth.createRestrictedPrivilegeToken().then(token => { - setToken(token); - setAddDeviceWizardVisible(true); - }) - ); - } else { - setAddDeviceWizardVisible(true); - } + setAddDeviceWizardVisible(true); } function onDeviceAdded() { fetchDevices(); setAddDeviceWizardVisible(false); - setToken(null); } function onRemoveDevice(device: MfaDevice) { @@ -95,7 +72,6 @@ export default function useManageDevices(ctx: Ctx) { return { devices, - token, onAddDevice, onRemoveDevice, onDeviceAdded, @@ -103,7 +79,6 @@ export default function useManageDevices(ctx: Ctx) { deviceToRemove, fetchDevicesAttempt: fetchDevicesAttempt.attempt, createRestrictedTokenAttempt: createRestrictedTokenAttempt.attempt, - isReauthenticationRequired, addDeviceWizardVisible, hideRemoveDevice, closeAddDeviceWizard, diff --git a/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.story.tsx b/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.story.tsx index 46b2bfc235e02..92590ac5b5cdc 100644 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.story.tsx +++ b/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.story.tsx @@ -16,20 +16,25 @@ * along with this program. If not, see . */ -import React from 'react'; - -import { Auth2faType } from 'shared/services'; - import Dialog from 'design/Dialog'; -import { http, HttpResponse, delay } from 'msw'; +import { delay, http, HttpResponse } from 'msw'; + +import { makeEmptyAttempt } from 'shared/hooks/useAsync'; -import { createTeleportContext } from 'teleport/mocks/contexts'; import { ContextProvider } from 'teleport/index'; +import { createTeleportContext } from 'teleport/mocks/contexts'; import cfg from 'teleport/config'; -import { DeviceUsage } from 'teleport/services/mfa'; +import { + DeviceUsage, + MFA_OPTION_SSO_DEFAULT, + MFA_OPTION_TOTP, + MFA_OPTION_WEBAUTHN, +} from 'teleport/services/mfa'; + +import { ReauthState } from 'teleport/components/ReAuthenticate/useReAuthenticate'; import { AddAuthDeviceWizardStepProps, @@ -62,25 +67,14 @@ export function ReauthenticateLimitedOptions() { return ( ); } -export function ReauthenticateNoOptions() { - return ; -} - export function CreatePasskey() { return ; } @@ -92,7 +86,9 @@ export function CreateMfaHardwareDevice() { } export function CreateMfaAppQrCodeLoading() { - return ; + return ( + + ); } CreateMfaAppQrCodeLoading.parameters = { msw: { @@ -105,10 +101,12 @@ CreateMfaAppQrCodeLoading.parameters = { }, }; -export function CreateMfaAppQrCodeFailed() { - return ; +export function CreateAuthenticatorAppQrCodeFailed() { + return ( + + ); } -CreateMfaAppQrCodeFailed.parameters = { +CreateAuthenticatorAppQrCodeFailed.parameters = { msw: { handlers: [ http.post( @@ -128,10 +126,12 @@ CreateMfaAppQrCodeFailed.parameters = { const dummyQrCode = 'iVBORw0KGgoAAAANSUhEUgAAAB0AAAAdAQMAAABsXfVMAAAABlBMVEUAAAD///+l2Z/dAAAAAnRSTlP//8i138cAAAAJcEhZcwAACxIAAAsSAdLdfvwAAABrSURBVAiZY/gPBAxoxAcxh3qG71fv1zN8iQ8EEReBRACQ+H4ZKPZBFCj7/3v9f4aPU9vqGX4kFtUzfG5mBLK2aNUz/PM3AsmqAk2RNQTquLYLqDdG/z/QlGAgES4CFLu4GygrXF2Pbi+IAADZqFQFAjXZWgAAAABJRU5ErkJggg=='; -export function CreateMfaApp() { - return ; +export function CreateAuthenticatorApp() { + return ( + + ); } -CreateMfaApp.parameters = { +CreateAuthenticatorApp.parameters = { msw: { handlers: [ http.post( @@ -153,7 +153,7 @@ export function SaveMfaHardwareDevice() { } export function SaveMfaAuthenticatorApp() { - return ; + return ; } const stepProps: AddAuthDeviceWizardStepProps = { @@ -165,35 +165,26 @@ const stepProps: AddAuthDeviceWizardStepProps = { flowLength: 1, refCallback: () => {}, - // Other props + // Shared props privilegeToken: 'privilege-token', - usage: 'passwordless' as DeviceUsage, - auth2faType: 'on' as Auth2faType, - credential: { id: 'cred-id', type: 'public-key' }, - newMfaDeviceType: 'webauthn' as Auth2faType, - devices: [ - { - id: '1', - description: 'Authenticator App', - name: 'gizmo', - registeredDate: new Date(1628799417000), - lastUsedDate: new Date(1628799417000), - type: 'totp', - usage: 'mfa', - }, - { - id: '2', - description: 'Hardware Key', - name: 'key', - registeredDate: new Date(1623722252000), - lastUsedDate: new Date(1623981452000), - type: 'webauthn', - usage: 'mfa', - }, - ], - onNewMfaDeviceTypeChange: () => {}, - onDeviceCreated: () => {}, - onAuthenticated: () => {}, + newMfaDeviceType: 'webauthn', onClose: () => {}, onSuccess: () => {}, + usage: 'passwordless' as DeviceUsage, + + // Reauth props + reauthState: { + mfaOptions: [MFA_OPTION_WEBAUTHN, MFA_OPTION_TOTP, MFA_OPTION_SSO_DEFAULT], + submitWithMfa: async () => null, + submitAttempt: makeEmptyAttempt(), + clearSubmitAttempt: () => {}, + } as ReauthState, + + // Create props + mfaRegisterOptions: [MFA_OPTION_WEBAUTHN, MFA_OPTION_TOTP], + onDeviceCreated: () => {}, + onNewMfaDeviceTypeChange: () => {}, + + // Save props + credential: { id: 'cred-id', type: 'public-key' }, }; 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 3d0c6bbb6bf43..96a2fc05a404f 100644 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.test.tsx +++ b/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.test.tsx @@ -17,20 +17,17 @@ */ import { render, screen } from 'design/utils/testing'; -import React from 'react'; -import { within } from '@testing-library/react'; +import { waitFor } from '@testing-library/react'; import { userEvent, UserEvent } from '@testing-library/user-event'; -import TeleportContext from 'teleport/teleportContext'; import { ContextProvider } from 'teleport'; -import MfaService from 'teleport/services/mfa'; import auth from 'teleport/services/auth'; +import MfaService from 'teleport/services/mfa'; +import TeleportContext from 'teleport/teleportContext'; import { AddAuthDeviceWizardStepProps } from './AddAuthDeviceWizard'; -import { deviceCases } from './deviceCases'; - import { AddAuthDeviceWizard } from '.'; const dummyCredential: Credential = { id: 'cred-id', type: 'public-key' }; @@ -49,14 +46,6 @@ beforeEach(() => { jest .spyOn(MfaService.prototype, 'saveNewWebAuthnDevice') .mockResolvedValueOnce(undefined); - jest - .spyOn(auth, 'createPrivilegeTokenWithWebauthn') - .mockResolvedValueOnce('webauthn-privilege-token'); - jest - .spyOn(auth, 'createPrivilegeTokenWithTotp') - .mockImplementationOnce(token => - Promise.resolve(`totp-privilege-token-${token}`) - ); jest.spyOn(auth, 'createMfaRegistrationChallenge').mockResolvedValueOnce({ qrCode: 'dummy-qr-code', webauthnPublicKey: {} as PublicKeyCredentialCreationOptions, @@ -73,8 +62,7 @@ function TestWizard(props: Partial = {}) { {}} onSuccess={onSuccess} {...props} @@ -84,25 +72,30 @@ function TestWizard(props: Partial = {}) { } describe('flow without reauthentication', () => { + beforeEach(() => { + jest.spyOn(auth, 'getMfaChallenge').mockResolvedValueOnce({}); + jest + .spyOn(auth, 'createPrivilegeToken') + .mockResolvedValueOnce('privilege-token'); + }); + test('adds a passkey', async () => { render( ); - const createStep = within(screen.getByTestId('create-step')); - await user.click( - createStep.getByRole('button', { name: 'Create a passkey' }) - ); + await waitFor(() => { + expect(screen.getByTestId('create-step')).toBeInTheDocument(); + }); + await user.click(screen.getByRole('button', { name: 'Create a passkey' })); expect(auth.createNewWebAuthnDevice).toHaveBeenCalledWith({ tokenId: 'privilege-token', deviceUsage: 'passwordless', }); - const saveStep = within(screen.getByTestId('save-step')); - await user.type(saveStep.getByLabelText('Passkey Nickname'), 'new-passkey'); - await user.click( - saveStep.getByRole('button', { name: 'Save the Passkey' }) - ); + 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: { @@ -117,20 +110,22 @@ describe('flow without reauthentication', () => { test('adds a WebAuthn MFA', async () => { render(); - const createStep = within(screen.getByTestId('create-step')); - await user.click(createStep.getByLabelText('Hardware Device')); + await waitFor(() => { + expect(screen.getByTestId('create-step')).toBeInTheDocument(); + }); + await user.click(screen.getByLabelText('Security Key')); await user.click( - createStep.getByRole('button', { name: 'Create an MFA method' }) + screen.getByRole('button', { name: 'Create an MFA method' }) ); expect(auth.createNewWebAuthnDevice).toHaveBeenCalledWith({ tokenId: 'privilege-token', deviceUsage: 'mfa', }); - const saveStep = within(screen.getByTestId('save-step')); - await user.type(saveStep.getByLabelText('MFA Method Name'), 'new-mfa'); + expect(screen.getByTestId('save-step')).toBeInTheDocument(); + await user.type(screen.getByLabelText('MFA Method Name'), 'new-mfa'); await user.click( - saveStep.getByRole('button', { name: 'Save the MFA method' }) + screen.getByRole('button', { name: 'Save the MFA method' }) ); expect(ctx.mfaService.saveNewWebAuthnDevice).toHaveBeenCalledWith({ credential: dummyCredential, @@ -146,21 +141,24 @@ describe('flow without reauthentication', () => { test('adds an authenticator app', async () => { render(); - const createStep = within(screen.getByTestId('create-step')); - await user.click(createStep.getByLabelText('Authenticator App')); - expect(createStep.getByRole('img')).toHaveAttribute( + await waitFor(() => { + expect(screen.getByTestId('create-step')).toBeInTheDocument(); + }); + + await user.click(screen.getByLabelText('Authenticator App')); + expect(screen.getByRole('img')).toHaveAttribute( 'src', 'data:image/png;base64,dummy-qr-code' ); await user.click( - createStep.getByRole('button', { name: 'Create an MFA method' }) + screen.getByRole('button', { name: 'Create an MFA method' }) ); - const saveStep = within(screen.getByTestId('save-step')); - await user.type(saveStep.getByLabelText('MFA Method Name'), 'new-mfa'); - await user.type(saveStep.getByLabelText(/Authenticator Code/), '345678'); + expect(screen.getByTestId('save-step')).toBeInTheDocument(); + await user.type(screen.getByLabelText('MFA Method Name'), 'new-mfa'); + await user.type(screen.getByLabelText(/Authenticator Code/), '345678'); await user.click( - saveStep.getByRole('button', { name: 'Save the MFA method' }) + screen.getByRole('button', { name: 'Save the MFA method' }) ); expect(ctx.mfaService.addNewTotpDevice).toHaveBeenCalledWith({ tokenId: 'privilege-token', @@ -172,34 +170,44 @@ describe('flow without reauthentication', () => { }); describe('flow with reauthentication', () => { + beforeEach(() => { + jest.spyOn(auth, 'getMfaChallenge').mockResolvedValueOnce({ + totpChallenge: true, + webauthnPublicKey: {} as PublicKeyCredentialRequestOptions, + }); + jest.spyOn(auth, 'getMfaChallengeResponse').mockResolvedValueOnce({}); + jest + .spyOn(auth, 'createPrivilegeToken') + .mockResolvedValueOnce('privilege-token'); + }); + test('adds a passkey with WebAuthn reauthentication', async () => { render(); - const reauthenticateStep = within( - screen.getByTestId('reauthenticate-step') - ); - await user.click(reauthenticateStep.getByText('Verify my identity')); + await waitFor(() => { + expect(screen.getByTestId('reauthenticate-step')).toBeInTheDocument(); + }); - const createStep = within(screen.getByTestId('create-step')); - await user.click( - createStep.getByRole('button', { name: 'Create a passkey' }) - ); + await user.click(screen.getByText('Verify my identity')); + + await waitFor(() => { + expect(screen.getByTestId('create-step')).toBeInTheDocument(); + }); + await user.click(screen.getByRole('button', { name: 'Create a passkey' })); expect(auth.createNewWebAuthnDevice).toHaveBeenCalledWith({ - tokenId: 'webauthn-privilege-token', + tokenId: 'privilege-token', deviceUsage: 'passwordless', }); - const saveStep = within(screen.getByTestId('save-step')); - await user.type(saveStep.getByLabelText('Passkey Nickname'), 'new-passkey'); - await user.click( - saveStep.getByRole('button', { name: 'Save the Passkey' }) - ); + 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: 'webauthn-privilege-token', + tokenId: 'privilege-token', }, }); expect(onSuccess).toHaveBeenCalled(); @@ -208,90 +216,45 @@ describe('flow with reauthentication', () => { test('adds a passkey with OTP reauthentication', async () => { render(); - const reauthenticateStep = within( - screen.getByTestId('reauthenticate-step') - ); - await user.click(reauthenticateStep.getByText('Authenticator App')); - await user.type( - reauthenticateStep.getByLabelText('Authenticator Code'), - '654987' - ); - await user.click(reauthenticateStep.getByText('Verify my identity')); + await waitFor(() => { + expect(screen.getByTestId('reauthenticate-step')).toBeInTheDocument(); + }); - const createStep = within(screen.getByTestId('create-step')); - await user.click( - createStep.getByRole('button', { name: 'Create a passkey' }) - ); + await user.click(screen.getByText('Authenticator App')); + await user.type(screen.getByLabelText('Authenticator Code'), '654987'); + await user.click(screen.getByText('Verify my identity')); + + await waitFor(() => { + expect(screen.getByTestId('create-step')).toBeInTheDocument(); + }); + await user.click(screen.getByRole('button', { name: 'Create a passkey' })); expect(auth.createNewWebAuthnDevice).toHaveBeenCalledWith({ - tokenId: 'totp-privilege-token-654987', + tokenId: 'privilege-token', deviceUsage: 'passwordless', }); - const saveStep = within(screen.getByTestId('save-step')); - await user.type(saveStep.getByLabelText('Passkey Nickname'), 'new-passkey'); - await user.click( - saveStep.getByRole('button', { name: 'Save the Passkey' }) - ); + 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: 'totp-privilege-token-654987', + tokenId: 'privilege-token', }, }); expect(onSuccess).toHaveBeenCalled(); }); - test('shows all authentication options', async () => { - render( - - ); - - const reauthenticateStep = within( - screen.getByTestId('reauthenticate-step') - ); - expect( - reauthenticateStep.queryByLabelText(/passkey or security key/i) - ).toBeVisible(); - expect( - reauthenticateStep.queryByLabelText(/authenticator app/i) - ).toBeVisible(); - }); + test('shows reauthentication options', async () => { + render(); - test('limits authentication options to devices owned', async () => { - render( - - ); + await waitFor(() => { + expect(screen.getByTestId('reauthenticate-step')).toBeInTheDocument(); + }); - const reauthenticateStep = within( - screen.getByTestId('reauthenticate-step') - ); - expect( - reauthenticateStep.queryByLabelText(/passkey or security key/i) - ).not.toBeInTheDocument(); - expect( - reauthenticateStep.queryByLabelText(/authenticator app/i) - ).toBeVisible(); + expect(screen.queryByLabelText(/passkey or security key/i)).toBeVisible(); + expect(screen.queryByLabelText(/authenticator app/i)).toBeVisible(); }); - - test.each` - auth2faType | deviceCase | error - ${'otp'} | ${'mfaDevices'} | ${/authenticator app is required/i} - ${'webauthn'} | ${'authApps'} | ${/passkey or security key is required/i} - ${'on'} | ${'none'} | ${/identity verification is required/i} - `( - 'shows an error if no way to authenticate for MFA type "$auth2faType"', - async ({ auth2faType, deviceCase, error }) => { - render( - - ); - - expect(screen.getByText(error)).toBeVisible(); - } - ); }); diff --git a/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx b/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx index 79cabebb57a1d..bbce32df63f98 100644 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx +++ b/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -import { OutlineDanger } from 'design/Alert/Alert'; +import { Alert, OutlineDanger } from 'design/Alert/Alert'; import { ButtonPrimary, ButtonSecondary } from 'design/Button'; import Dialog from 'design/Dialog'; import Flex from 'design/Flex'; @@ -24,14 +24,13 @@ import Image from 'design/Image'; import Indicator from 'design/Indicator'; import { RadioGroup } from 'design/RadioGroup'; import { StepComponentProps, StepSlider } from 'design/StepSlider'; -import React, { useState, useEffect, FormEvent } from 'react'; +import React, { FormEvent, useEffect, useState } from 'react'; import FieldInput from 'shared/components/FieldInput'; import Validation, { Validator } from 'shared/components/Validation'; import { requiredField } from 'shared/components/Validation/rules'; import { useAsync } from 'shared/hooks/useAsync'; import useAttempt from 'shared/hooks/useAttemptNext'; import { Auth2faType } from 'shared/services'; -import createMfaOptions, { MfaOption } from 'shared/utils/createMfaOptions'; import Box from 'design/Box'; @@ -39,10 +38,17 @@ import { StepHeader } from 'design/StepSlider'; import { P } from 'design/Text/Text'; -import auth from 'teleport/services/auth/auth'; +import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; import useTeleport from 'teleport/useTeleport'; -import { DeviceUsage, MfaDevice } from 'teleport/services/mfa'; +import { + DeviceType, + DeviceUsage, + getMfaRegisterOptions, + MfaOption, +} from 'teleport/services/mfa'; + +import useReAuthenticate from 'teleport/components/ReAuthenticate/useReAuthenticate'; import { PasskeyBlurb } from '../../../components/Passkeys/PasskeyBlurb'; @@ -56,40 +62,69 @@ interface AddAuthDeviceWizardProps { usage: DeviceUsage; /** MFA type setting, as configured in the cluster's configuration. */ auth2faType: Auth2faType; - /** - * A list of user's devices, used for computing the list of available identity - * verification options. - */ - devices: MfaDevice[]; - /** - * A privilege token that may have been created previously; if present, the - * reauthentication step will be skipped. - */ - privilegeToken?: string; onClose(): void; onSuccess(): void; } /** A wizard for adding MFA and passkey devices. */ export function AddAuthDeviceWizard({ - privilegeToken: privilegeTokenProp = '', usage, auth2faType, - devices, onClose, onSuccess, }: AddAuthDeviceWizardProps) { - const reauthRequired = !privilegeTokenProp; - const [privilegeToken, setPrivilegeToken] = useState(privilegeTokenProp); + const [privilegeToken, setPrivilegeToken] = useState(); const [credential, setCredential] = useState(null); - const mfaOptions = createMfaOptions({ - auth2faType, - required: true, + const reauthState = useReAuthenticate({ + challengeScope: MfaChallengeScope.MANAGE_DEVICES, + onMfaResponse: mfaResponse => + // TODO(Joerger): Instead of getting a privilege token, we should get + // // a register challenge with the mfa response directly. For good UX, this would + // // require some refactoring to the flow so the user can choose a device type before + // // completing an mfa check and getting an otp/webauthn register challenge, or + // // allowing the backend to return a flexible register challenge + // await auth.createPrivilegeToken(mfaResponse).then(setPrivilegeToken); + auth.createPrivilegeToken(mfaResponse).then(setPrivilegeToken), }); - /** A new MFA device type, irrelevant if usage === 'passkey'. */ - const [newMfaDeviceType, setNewMfaDeviceType] = useState(mfaOptions[0].value); + // Choose a new device type from the options available for the given 2fa type. + // irrelevant if usage === 'passkey'. + const registerMfaOptions = getMfaRegisterOptions(auth2faType); + const [newMfaDeviceType, setNewMfaDeviceType] = useState( + registerMfaOptions[0].value + ); + + // If the user has no mfa devices registered, they can create a privilege token + // without an mfa response. + // + // TODO(Joerger): v19.0.0 + // A user without devices can register their first device without a privilege token + // too, but the existing web register endpoint requires privilege token. + // We have a new endpoint "/v1/webapi/users/devices" which does not + // require token, but can't be used until v19 for backwards compatibility. + // Once in use, we can leave privilege token empty here. + useEffect(() => { + if (reauthState.mfaOptions?.length === 0) { + auth.createPrivilegeToken().then(setPrivilegeToken); + } + }, [reauthState.mfaOptions]); + + // Handle potential error states first. + switch (reauthState.initAttempt.status) { + case 'processing': + return ( + + + + ); + case 'error': + return ; + case 'success': + break; + default: + return null; + } return ( 0 + ? 'withReauthentication' + : 'withoutReauthentication' } // Step properties + mfaRegisterOptions={registerMfaOptions} + reauthState={reauthState} usage={usage} - auth2faType={auth2faType} privilegeToken={privilegeToken} credential={credential} newMfaDeviceType={newMfaDeviceType} - devices={devices} onClose={onClose} - onAuthenticated={setPrivilegeToken} onNewMfaDeviceTypeChange={setNewMfaDeviceType} onDeviceCreated={setCredential} onSuccess={onSuccess} @@ -131,10 +167,10 @@ export type AddAuthDeviceWizardStepProps = StepComponentProps & SaveKeyStepProps; interface CreateDeviceStepProps { usage: DeviceUsage; - auth2faType: Auth2faType; + mfaRegisterOptions: MfaOption[]; privilegeToken: string; - newMfaDeviceType: Auth2faType; - onNewMfaDeviceTypeChange(o: Auth2faType): void; + newMfaDeviceType: DeviceType; + onNewMfaDeviceTypeChange(o: DeviceType): void; onClose(): void; onDeviceCreated(c: Credential): void; } @@ -146,9 +182,9 @@ export function CreateDeviceStep({ stepIndex, flowLength, usage, - auth2faType, privilegeToken, newMfaDeviceType, + mfaRegisterOptions, onNewMfaDeviceTypeChange, onClose, onDeviceCreated, @@ -158,6 +194,7 @@ export function CreateDeviceStep({ if (usage === 'passwordless' || newMfaDeviceType === 'webauthn') { createPasskeyAttempt.run(async () => { const credential = await auth.createNewWebAuthnDevice({ + // TODO(Joerger): Skip privilege token step, just pass in mfa response. tokenId: privilegeToken, deviceUsage: usage, }); @@ -193,7 +230,7 @@ export function CreateDeviceStep({ )} {usage === 'mfa' && ( - // Be more specific about the WebAuthn device type (it's not a passkey). - o.value === 'webauthn' ? { ...o, label: 'Hardware Device' } : o + // Be more specific about the WebAuthn device type (it's not a passkey). + mfaRegisterOptions = mfaRegisterOptions.map((o: MfaOption) => + o.value === 'webauthn' ? { ...o, label: 'Security Key' } : o ); return ( @@ -243,17 +277,17 @@ function CreateMfaBox({ Multi-factor type { - onNewMfaDeviceTypeChange(o as Auth2faType); + onNewMfaDeviceTypeChange(o as DeviceType); }} /> - {newMfaDeviceType === 'otp' && ( + {newMfaDeviceType === 'totp' && ( )} @@ -308,7 +342,7 @@ interface SaveKeyStepProps { privilegeToken: string; credential: Credential; usage: DeviceUsage; - newMfaDeviceType: Auth2faType; + newMfaDeviceType: DeviceType; onSuccess(): void; } @@ -396,7 +430,7 @@ export function SaveDeviceStep({ readonly={saveAttempt.attempt.status === 'processing'} /> - {usage === 'mfa' && newMfaDeviceType === 'otp' && ( + {usage === 'mfa' && newMfaDeviceType === 'totp' && ( . */ -import React from 'react'; - import Dialog from 'design/Dialog'; -import { createTeleportContext } from 'teleport/mocks/contexts'; +import { makeEmptyAttempt } from 'shared/hooks/useAsync'; + import { ContextProvider } from 'teleport/index'; +import { createTeleportContext } from 'teleport/mocks/contexts'; -import { MfaDevice } from 'teleport/services/mfa'; +import { + MFA_OPTION_SSO_DEFAULT, + MFA_OPTION_TOTP, + MFA_OPTION_WEBAUTHN, + MfaDevice, +} from 'teleport/services/mfa'; + +import { ReauthState } from 'teleport/components/ReAuthenticate/useReAuthenticate'; import { DeleteAuthDeviceWizardStepProps, @@ -100,12 +107,17 @@ const stepProps: DeleteAuthDeviceWizardStepProps = { flowLength: 2, refCallback: () => {}, - // Other props - devices: [dummyHardwareDevice, dummyPasskey], + // Reauth props + reauthState: { + mfaOptions: [MFA_OPTION_WEBAUTHN, MFA_OPTION_TOTP, MFA_OPTION_SSO_DEFAULT], + submitWithMfa: async () => null, + submitAttempt: makeEmptyAttempt(), + clearSubmitAttempt: () => {}, + } as ReauthState, + + // Delete props deviceToDelete: dummyPasskey, privilegeToken: 'privilege-token', - auth2faType: 'optional', - onAuthenticated: () => {}, onClose: () => {}, onSuccess: () => {}, }; 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 8062e09294b1c..dd780c4f3996f 100644 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.test.tsx +++ b/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.test.tsx @@ -18,7 +18,7 @@ import { render, screen } from 'design/utils/testing'; -import { within } from '@testing-library/react'; +import { waitFor } from '@testing-library/react'; import { userEvent, UserEvent } from '@testing-library/user-event'; import TeleportContext from 'teleport/teleportContext'; @@ -28,7 +28,7 @@ import auth from 'teleport/services/auth'; import { DeleteAuthDeviceWizardStepProps } from './DeleteAuthDeviceWizard'; -import { dummyPasskey, dummyHardwareDevice, deviceCases } from './deviceCases'; +import { dummyPasskey, dummyHardwareDevice } from './deviceCases'; import { DeleteAuthDeviceWizard } from '.'; @@ -41,14 +41,14 @@ beforeEach(() => { user = userEvent.setup(); onSuccess = jest.fn(); + jest.spyOn(auth, 'getMfaChallenge').mockResolvedValueOnce({ + totpChallenge: true, + webauthnPublicKey: {} as PublicKeyCredentialRequestOptions, + }); + jest.spyOn(auth, 'getMfaChallengeResponse').mockResolvedValueOnce({}); jest - .spyOn(auth, 'createPrivilegeTokenWithWebauthn') - .mockResolvedValueOnce('webauthn-privilege-token'); - jest - .spyOn(auth, 'createPrivilegeTokenWithTotp') - .mockImplementationOnce(token => - Promise.resolve(`totp-privilege-token-${token}`) - ); + .spyOn(auth, 'createPrivilegeToken') + .mockResolvedValueOnce('privilege-token'); jest .spyOn(MfaService.prototype, 'removeDevice') .mockResolvedValueOnce(undefined); @@ -60,9 +60,7 @@ function TestWizard(props: Partial) { return ( {}} onSuccess={onSuccess} {...props} @@ -74,14 +72,16 @@ function TestWizard(props: Partial) { test('deletes a device with WebAuthn reauthentication', async () => { render(); - const reauthenticateStep = within(screen.getByTestId('reauthenticate-step')); - await user.click(reauthenticateStep.getByText('Verify my identity')); + await waitFor(() => { + expect(screen.getByTestId('reauthenticate-step')).toBeInTheDocument(); + }); + await user.click(screen.getByText('Verify my identity')); - const deleteStep = within(screen.getByTestId('delete-step')); - await user.click(deleteStep.getByRole('button', { name: 'Delete' })); + expect(screen.getByTestId('delete-step')).toBeInTheDocument(); + await user.click(screen.getByRole('button', { name: 'Delete' })); expect(ctx.mfaService.removeDevice).toHaveBeenCalledWith( - 'webauthn-privilege-token', + 'privilege-token', 'TouchID' ); expect(onSuccess).toHaveBeenCalled(); @@ -90,19 +90,18 @@ test('deletes a device with WebAuthn reauthentication', async () => { test('deletes a device with OTP reauthentication', async () => { render(); - const reauthenticateStep = within(screen.getByTestId('reauthenticate-step')); - await user.click(reauthenticateStep.getByText('Authenticator App')); - await user.type( - reauthenticateStep.getByLabelText('Authenticator Code'), - '654987' - ); - await user.click(reauthenticateStep.getByText('Verify my identity')); + await waitFor(() => { + expect(screen.getByTestId('reauthenticate-step')).toBeInTheDocument(); + }); + await user.click(screen.getByText('Authenticator App')); + await user.type(screen.getByLabelText('Authenticator Code'), '654987'); + await user.click(screen.getByText('Verify my identity')); - const deleteStep = within(screen.getByTestId('delete-step')); - await user.click(deleteStep.getByRole('button', { name: 'Delete' })); + expect(screen.getByTestId('delete-step')).toBeInTheDocument(); + await user.click(screen.getByRole('button', { name: 'Delete' })); expect(ctx.mfaService.removeDevice).toHaveBeenCalledWith( - 'totp-privilege-token-654987', + 'privilege-token', 'TouchID' ); }); @@ -125,13 +124,13 @@ test.each([ async ({ device, title, message }) => { render(); - const reauthenticateStep = within( - screen.getByTestId('reauthenticate-step') - ); - await user.click(reauthenticateStep.getByText('Verify my identity')); + await waitFor(() => { + expect(screen.getByTestId('reauthenticate-step')).toBeInTheDocument(); + }); + await user.click(screen.getByText('Verify my identity')); - const deleteStep = within(screen.getByTestId('delete-step')); - expect(deleteStep.getByText(title)).toBeVisible(); - expect(deleteStep.getByText(message)).toBeVisible(); + expect(screen.getByTestId('delete-step')).toBeInTheDocument(); + expect(screen.getByText(title)).toBeVisible(); + expect(screen.getByText(message)).toBeVisible(); } ); diff --git a/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.tsx b/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.tsx index 44460e9cb6c6a..13d6e8a67ae73 100644 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.tsx +++ b/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.tsx @@ -16,36 +16,32 @@ * along with this program. If not, see . */ -import { OutlineDanger } from 'design/Alert/Alert'; +import { Alert, OutlineDanger } from 'design/Alert/Alert'; import { ButtonSecondary, ButtonWarning } from 'design/Button'; import Dialog from 'design/Dialog'; import Flex from 'design/Flex'; -import { StepComponentProps, StepSlider } from 'design/StepSlider'; -import React, { useState } from 'react'; +import { StepComponentProps, StepHeader, StepSlider } from 'design/StepSlider'; +import { useState } from 'react'; import useAttempt from 'shared/hooks/useAttemptNext'; -import { Auth2faType } from 'shared/services'; import Box from 'design/Box'; -import { StepHeader } from 'design/StepSlider'; +import Indicator from 'design/Indicator'; import useTeleport from 'teleport/useTeleport'; import { MfaDevice } from 'teleport/services/mfa'; +import useReAuthenticate from 'teleport/components/ReAuthenticate/useReAuthenticate'; + +import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; + import { ReauthenticateStep, ReauthenticateStepProps, } from './ReauthenticateStep'; interface DeleteAuthDeviceWizardProps { - /** MFA type setting, as configured in the cluster's configuration. */ - auth2faType: Auth2faType; - /** - * A list of user's devices, used for computing the list of available identity - * verification options. - */ - devices: MfaDevice[]; /** Device to be removed. */ deviceToDelete: MfaDevice; onClose(): void; @@ -54,14 +50,39 @@ interface DeleteAuthDeviceWizardProps { /** A wizard for deleting MFA and passkey devices. */ export function DeleteAuthDeviceWizard({ - auth2faType, - devices, deviceToDelete, onClose, onSuccess, }: DeleteAuthDeviceWizardProps) { const [privilegeToken, setPrivilegeToken] = useState(''); + const reauthState = useReAuthenticate({ + challengeScope: MfaChallengeScope.MANAGE_DEVICES, + onMfaResponse: mfaResponse => + // TODO(Joerger): v19.0.0 + // Devices can be deleted with an MFA response, so exchanging it for a + // privilege token adds an unnecessary API call. The device deletion + // endpoint requires a token, but the new endpoint "DELETE: /webapi/mfa/devices" + // does not and can be used in v19 backwards compatibly. + auth.createPrivilegeToken(mfaResponse).then(setPrivilegeToken), + }); + + // Handle potential error states first. + switch (reauthState.initAttempt.status) { + case 'processing': + return ( + + + + ); + case 'error': + return ; + case 'success': + break; + default: + return null; + } + return ( diff --git a/web/packages/teleport/src/Account/ManageDevices/wizards/ReauthenticateStep.test.tsx b/web/packages/teleport/src/Account/ManageDevices/wizards/ReauthenticateStep.test.tsx deleted file mode 100644 index 88f18c1d724ef..0000000000000 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/ReauthenticateStep.test.tsx +++ /dev/null @@ -1,44 +0,0 @@ -/** - * 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 { createReauthOptions } from './ReauthenticateStep'; -import { deviceCases } from './deviceCases'; - -test.each` - auth2faType | deviceCase | methods - ${'otp'} | ${'all'} | ${['otp']} - ${'off'} | ${'all'} | ${[]} - ${'optional'} | ${'all'} | ${['webauthn', 'otp']} - ${'on'} | ${'all'} | ${['webauthn', 'otp']} - ${'webauthn'} | ${'all'} | ${['webauthn']} - ${'optional'} | ${'authApps'} | ${['otp']} - ${'optional'} | ${'mfaDevices'} | ${['webauthn']} - ${'optional'} | ${'passkeys'} | ${['webauthn']} - ${'on'} | ${'none'} | ${[]} - ${'webauthn'} | ${'authApps'} | ${[]} - ${'otp'} | ${'mfaDevices'} | ${[]} -`( - 'createReauthOptions: auth2faType=$auth2faType, devices=$deviceCase', - ({ auth2faType, methods, deviceCase }) => { - const devices = deviceCases[deviceCase]; - const reauthMethods = createReauthOptions(auth2faType, devices).map( - o => o.value - ); - expect(reauthMethods).toEqual(methods); - } -); diff --git a/web/packages/teleport/src/Account/ManageDevices/wizards/ReauthenticateStep.tsx b/web/packages/teleport/src/Account/ManageDevices/wizards/ReauthenticateStep.tsx index 22f53a16cc49f..5dd0fd28eafb5 100644 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/ReauthenticateStep.tsx +++ b/web/packages/teleport/src/Account/ManageDevices/wizards/ReauthenticateStep.tsx @@ -20,54 +20,35 @@ import { OutlineDanger } from 'design/Alert/Alert'; import { ButtonPrimary, ButtonSecondary } from 'design/Button'; import Flex from 'design/Flex'; import { RadioGroup } from 'design/RadioGroup'; -import React, { useState, FormEvent } from 'react'; +import { StepComponentProps, StepHeader } from 'design/StepSlider'; +import React, { FormEvent, useState } from 'react'; import FieldInput from 'shared/components/FieldInput'; import Validation, { Validator } from 'shared/components/Validation'; import { requiredField } from 'shared/components/Validation/rules'; -import { Auth2faType } from 'shared/services'; -import createMfaOptions, { MfaOption } from 'shared/utils/createMfaOptions'; -import { StepComponentProps, StepHeader } from 'design/StepSlider'; import Box from 'design/Box'; -import { Attempt } from 'shared/hooks/useAttemptNext'; - -import useReAuthenticate from 'teleport/components/ReAuthenticate/useReAuthenticate'; -import { MfaDevice } from 'teleport/services/mfa'; +import { ReauthState } from 'teleport/components/ReAuthenticate/useReAuthenticate'; +import { DeviceType } from 'teleport/services/mfa'; export type ReauthenticateStepProps = StepComponentProps & { - auth2faType: Auth2faType; - devices: MfaDevice[]; - onAuthenticated(privilegeToken: string): void; + reauthState: ReauthState; onClose(): void; }; + export function ReauthenticateStep({ next, refCallback, stepIndex, flowLength, - auth2faType, - devices, + reauthState: { mfaOptions, submitWithMfa, submitAttempt, clearSubmitAttempt }, onClose, - onAuthenticated: onAuthenticatedProp, }: ReauthenticateStepProps) { - const onAuthenticated = (privilegeToken: string) => { - onAuthenticatedProp(privilegeToken); - next(); - }; - const { attempt, clearAttempt, submitWithTotp, submitWithWebauthn } = - useReAuthenticate({ - onAuthenticated, - }); - const mfaOptions = createReauthOptions(auth2faType, devices); - - const [mfaOption, setMfaOption] = useState( - mfaOptions[0]?.value - ); - const [authCode, setAuthCode] = useState(''); + const [otpCode, setOtpCode] = useState(''); + const [mfaOption, setMfaOption] = useState(mfaOptions[0].value); - const onAuthCodeChanged = (e: React.ChangeEvent) => { - setAuthCode(e.target.value); + const onOtpCodeChanged = (e: React.ChangeEvent) => { + setOtpCode(e.target.value); }; const onReauthenticate = ( @@ -76,20 +57,11 @@ export function ReauthenticateStep({ ) => { e.preventDefault(); if (!validator.validate()) return; - if (mfaOption === 'webauthn') { - submitWithWebauthn(); - } - if (mfaOption === 'otp') { - submitWithTotp(authCode); - } + submitWithMfa(mfaOption, 'mfa', otpCode).then(([, err]) => { + if (!err) next(); + }); }; - const errorMessage = getReauthenticationErrorMessage( - auth2faType, - mfaOptions.length, - attempt - ); - return (
@@ -99,7 +71,9 @@ export function ReauthenticateStep({ title="Verify Identity" /> - {errorMessage && {errorMessage}} + {submitAttempt.status === 'error' && ( + {submitAttempt.statusText} + )} {mfaOption && Multi-factor type} {({ validator }) => ( @@ -113,20 +87,20 @@ export function ReauthenticateStep({ gap={3} mb={4} onChange={o => { - setMfaOption(o as Auth2faType); - clearAttempt(); + setMfaOption(o as DeviceType); + clearSubmitAttempt(); }} /> - {mfaOption === 'otp' && ( + {mfaOption === 'totp' && ( )} @@ -150,64 +124,3 @@ export function ReauthenticateStep({
); } -function getReauthenticationErrorMessage( - auth2faType: Auth2faType, - numMfaOptions: number, - attempt: Attempt -): string { - if (numMfaOptions === 0) { - switch (auth2faType) { - case 'on': - return ( - "Identity verification is required, but you don't have any" + - 'passkeys or MFA methods registered. This may mean that the' + - 'server configuration has changed. Please contact your ' + - 'administrator.' - ); - case 'otp': - return ( - 'Identity verification using authenticator app is required, but ' + - "you don't have any authenticator apps registered. This may mean " + - 'that the server configuration has changed. Please contact your ' + - 'administrator.' - ); - case 'webauthn': - return ( - 'Identity verification using a passkey or security key is required, but ' + - "you don't have any such devices registered. This may mean " + - 'that the server configuration has changed. Please contact your ' + - 'administrator.' - ); - case 'optional': - case 'off': - // This error message is not useful, but this condition should never - // happen, and if it does, it means something is broken, and we don't - // have a clue anyway. - return 'Unable to verify identity'; - default: - auth2faType satisfies never; - } - } - - if (attempt.status === 'failed') { - // This message relies on the status message produced by the auth server in - // lib/auth/Server.checkOTP function. Please keep these in sync. - if (attempt.statusText === 'invalid totp token') { - return 'Invalid authenticator code'; - } else { - return attempt.statusText; - } - } -} - -export function createReauthOptions( - auth2faType: Auth2faType, - devices: MfaDevice[] -): MfaOption[] { - return createMfaOptions({ auth2faType, required: true }).filter( - ({ value }) => { - const deviceType = value === 'otp' ? 'totp' : value; - return devices.some(({ type }) => type === deviceType); - } - ); -} diff --git a/web/packages/teleport/src/Account/PasswordBox.tsx b/web/packages/teleport/src/Account/PasswordBox.tsx index d8d99ea287075..ea136156d0120 100644 --- a/web/packages/teleport/src/Account/PasswordBox.tsx +++ b/web/packages/teleport/src/Account/PasswordBox.tsx @@ -33,14 +33,12 @@ import { ChangePasswordWizard } from './ChangePasswordWizard'; import { StatePill, AuthMethodState } from './StatePill'; export interface PasswordBoxProps { - changeDisabled: boolean; devices: MfaDevice[]; passwordState: PasswordState; onPasswordChange: () => void; } export function PasswordBox({ - changeDisabled, devices, passwordState, onPasswordChange, @@ -67,10 +65,7 @@ export function PasswordBox({ } icon={} actions={ - setDialogOpen(true)} - > + setDialogOpen(true)}> Change Password } @@ -78,9 +73,10 @@ export function PasswordBox({ {dialogOpen && ( dev.usage === 'passwordless') + } onClose={() => setDialogOpen(false)} onSuccess={onSuccess} /> diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx index 1b5ae9e6a780a..eddce1b82a260 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx @@ -170,13 +170,13 @@ export function TestConnection(props: AgentStepProps) { {showMfaDialog && ( - testConnection({ + onMfaResponse={async res => { + await testConnection({ login: selectedLoginOpt.value, sshPrincipalSelectionMode, mfaResponse: res, - }) - } + }); + }} onClose={cancelMfaDialog} challengeScope={MfaChallengeScope.USER_SESSION} /> diff --git a/web/packages/teleport/src/Discover/Database/TestConnection/TestConnection.tsx b/web/packages/teleport/src/Discover/Database/TestConnection/TestConnection.tsx index d57a15037a21f..f9162dafd01be 100644 --- a/web/packages/teleport/src/Discover/Database/TestConnection/TestConnection.tsx +++ b/web/packages/teleport/src/Discover/Database/TestConnection/TestConnection.tsx @@ -117,7 +117,7 @@ export function TestConnection() { {showMfaDialog && ( testConnection(validator, res)} + onMfaResponse={async res => testConnection(validator, res)} onClose={cancelMfaDialog} challengeScope={MfaChallengeScope.USER_SESSION} /> diff --git a/web/packages/teleport/src/Discover/Kubernetes/TestConnection/TestConnection.tsx b/web/packages/teleport/src/Discover/Kubernetes/TestConnection/TestConnection.tsx index 1c0807560a700..a43fddf1c22a4 100644 --- a/web/packages/teleport/src/Discover/Kubernetes/TestConnection/TestConnection.tsx +++ b/web/packages/teleport/src/Discover/Kubernetes/TestConnection/TestConnection.tsx @@ -101,7 +101,9 @@ export function TestConnection({ {showMfaDialog && ( testConnection(makeTestConnRequest(), res)} + onMfaResponse={async res => + testConnection(makeTestConnRequest(), res) + } onClose={cancelMfaDialog} challengeScope={MfaChallengeScope.USER_SESSION} /> diff --git a/web/packages/teleport/src/Discover/Server/TestConnection/TestConnection.tsx b/web/packages/teleport/src/Discover/Server/TestConnection/TestConnection.tsx index f72d45081e132..7569906b05cb3 100644 --- a/web/packages/teleport/src/Discover/Server/TestConnection/TestConnection.tsx +++ b/web/packages/teleport/src/Discover/Server/TestConnection/TestConnection.tsx @@ -87,7 +87,7 @@ export function TestConnection(props: AgentStepProps) { {showMfaDialog && ( testConnection(selectedOpt.value, res)} + onMfaResponse={async res => testConnection(selectedOpt.value, res)} onClose={cancelMfaDialog} challengeScope={MfaChallengeScope.USER_SESSION} /> diff --git a/web/packages/teleport/src/components/ReAuthenticate/ReAuthenticate.story.tsx b/web/packages/teleport/src/components/ReAuthenticate/ReAuthenticate.story.tsx index 088bedae810a2..80cee8f465ffb 100644 --- a/web/packages/teleport/src/components/ReAuthenticate/ReAuthenticate.story.tsx +++ b/web/packages/teleport/src/components/ReAuthenticate/ReAuthenticate.story.tsx @@ -16,10 +16,16 @@ * along with this program. If not, see . */ -import React from 'react'; +import { makeEmptyAttempt } from 'shared/hooks/useAsync'; -import { State } from './useReAuthenticate'; -import { ReAuthenticate } from './ReAuthenticate'; +import { + MFA_OPTION_SSO_DEFAULT, + MFA_OPTION_TOTP, + MFA_OPTION_WEBAUTHN, +} from 'teleport/services/mfa'; + +import { ReAuthenticate, State } from './ReAuthenticate'; +import { ReauthState } from './useReAuthenticate'; export default { title: 'Teleport/ReAuthenticate', @@ -28,23 +34,38 @@ export default { export const Loaded = () => ; export const Processing = () => ( - + ); export const Failed = () => ( ); const props: State = { - attempt: { status: '' }, - clearAttempt: () => null, - submitWithTotp: () => null, - submitWithWebauthn: () => null, - preferredMfaType: 'webauthn', + reauthState: { + initAttempt: { status: 'success' }, + mfaOptions: [MFA_OPTION_WEBAUTHN, MFA_OPTION_TOTP, MFA_OPTION_SSO_DEFAULT], + submitWithMfa: async () => null, + submitAttempt: makeEmptyAttempt(), + clearSubmitAttempt: () => {}, + } as ReauthState, + onClose: () => null, - auth2faType: 'on', - actionText: 'performing this action', }; diff --git a/web/packages/teleport/src/components/ReAuthenticate/ReAuthenticate.tsx b/web/packages/teleport/src/components/ReAuthenticate/ReAuthenticate.tsx index c344262c7fd62..6dfc8a0f96f3c 100644 --- a/web/packages/teleport/src/components/ReAuthenticate/ReAuthenticate.tsx +++ b/web/packages/teleport/src/components/ReAuthenticate/ReAuthenticate.tsx @@ -16,55 +16,88 @@ * along with this program. If not, see . */ -import React, { useState } from 'react'; -import { Flex, Box, Text, ButtonPrimary, ButtonSecondary } from 'design'; +import { + Box, + ButtonPrimary, + ButtonSecondary, + Flex, + Indicator, + Text, +} from 'design'; +import { Alert, Danger } from 'design/Alert'; import Dialog, { - DialogHeader, - DialogTitle, DialogContent, DialogFooter, + DialogHeader, + DialogTitle, } from 'design/Dialog'; -import { Danger } from 'design/Alert'; -import Validation from 'shared/components/Validation'; -import { requiredToken } from 'shared/components/Validation/rules'; +import React, { useEffect, useState } from 'react'; import FieldInput from 'shared/components/FieldInput'; import FieldSelect from 'shared/components/FieldSelect'; -import createMfaOptions, { MfaOption } from 'shared/utils/createMfaOptions'; +import Validation, { Validator } from 'shared/components/Validation'; +import { requiredToken } from 'shared/components/Validation/rules'; + +import { MfaOption } from 'teleport/services/mfa'; -import useReAuthenticate, { State, Props } from './useReAuthenticate'; +import useReAuthenticate, { + ReauthProps, + ReauthState, +} from './useReAuthenticate'; + +export type Props = ReauthProps & { + onClose: () => void; +}; export default function Container(props: Props) { const state = useReAuthenticate(props); - return ; + return ; } +export type State = { + reauthState: ReauthState; + onClose: () => void; +}; + export function ReAuthenticate({ - attempt, - clearAttempt, - submitWithTotp, - submitWithWebauthn, onClose, - auth2faType, - preferredMfaType, - actionText, + reauthState: { + initAttempt, + mfaOptions, + submitWithMfa, + submitAttempt, + clearSubmitAttempt, + }, }: State) { - const [otpToken, setOtpToken] = useState(''); - const mfaOptions = createMfaOptions({ - auth2faType: auth2faType, - preferredType: preferredMfaType, - required: true, - }); - const [mfaOption, setMfaOption] = useState(mfaOptions[0]); + const [otpCode, setOtpToken] = useState(''); + const [mfaOption, setMfaOption] = useState(); - function onSubmit(e: React.MouseEvent) { - e.preventDefault(); + useEffect(() => { + if (mfaOptions?.length) setMfaOption(mfaOptions[0]); + }, [mfaOptions]); - if (mfaOption?.value === 'webauthn') { - submitWithWebauthn(); - } - if (mfaOption?.value === 'otp') { - submitWithTotp(otpToken); - } + // Handle potential error states first. + switch (initAttempt.status) { + case 'processing': + return ( + + + + ); + case 'error': + return ; + case 'success': + break; + default: + return null; + } + + function onReauthenticate( + e: React.MouseEvent, + validator: Validator + ) { + e.preventDefault(); + if (!validator.validate()) return; + submitWithMfa(mfaOption.value, 'mfa', otpCode); } return ( @@ -83,12 +116,12 @@ export function ReAuthenticate({ Verify your identity You must verify your identity with one of your existing - two-factor devices before {actionText}. + two-factor devices before performing this action. - {attempt.status === 'failed' && ( + {submitAttempt.status === 'error' && ( - {attempt.statusText} + {submitAttempt.statusText} )} @@ -100,25 +133,25 @@ export function ReAuthenticate({ options={mfaOptions} onChange={(o: MfaOption) => { setMfaOption(o); - clearAttempt(); + clearSubmitAttempt(); }} data-testid="mfa-select" mr={3} mb={0} - isDisabled={attempt.status === 'processing'} + isDisabled={submitAttempt.status === 'processing'} elevated={true} /> - {mfaOption.value === 'otp' && ( + {mfaOption?.value === 'totp' && ( setOtpToken(e.target.value)} placeholder="123 456" - readonly={attempt.status === 'processing'} + readonly={submitAttempt.status === 'processing'} mb={0} /> )} @@ -127,8 +160,8 @@ export function ReAuthenticate({ validator.validate() && onSubmit(e)} - disabled={attempt.status === 'processing'} + onClick={e => onReauthenticate(e, validator)} + disabled={submitAttempt.status === 'processing'} mr={3} mt={3} type="submit" diff --git a/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts b/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts index 7b2746de0a25c..ed8c73f3fe6da 100644 --- a/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts +++ b/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts @@ -16,128 +16,141 @@ * along with this program. If not, see . */ -import useAttempt from 'shared/hooks/useAttemptNext'; +import { useCallback, useEffect, useState } from 'react'; +import { Attempt, makeEmptyAttempt, useAsync } from 'shared/hooks/useAsync'; -import cfg from 'teleport/config'; import auth from 'teleport/services/auth'; import { MfaChallengeScope } from 'teleport/services/auth/auth'; -import type { MfaChallengeResponse } from 'teleport/services/mfa'; - -// useReAuthenticate will have different "submit" behaviors depending on: -// - If prop field `onMfaResponse` is defined, after a user submits, the -// function `onMfaResponse` is called with the user's MFA response. -// - If prop field `onAuthenticated` is defined, after a user submits, the -// user's MFA response are submitted with the request to get a privilege -// token, and after successfully obtaining the token, the function -// `onAuthenticated` will be called with this token. -export default function useReAuthenticate(props: Props) { - const { onClose, actionText = defaultActionText } = props; - - // Note that attempt state "success" is not used or required. - // After the user submits, the control is passed back - // to the caller who is reponsible for rendering the `ReAuthenticate` - // component. - const { attempt, setAttempt, handleError } = useAttempt(''); - - function submitWithTotp(secondFactorToken: string) { - if ('onMfaResponse' in props) { - props.onMfaResponse({ totp_code: secondFactorToken }); - return; - } - - setAttempt({ status: 'processing' }); - auth - .createPrivilegeTokenWithTotp(secondFactorToken) - .then(props.onAuthenticated) - .catch(handleError); - } +import { + DeviceType, + DeviceUsage, + getMfaChallengeOptions, + MfaAuthenticateChallenge, + MfaChallengeResponse, + MfaOption, +} from 'teleport/services/mfa'; + +export default function useReAuthenticate({ + challengeScope, + onMfaResponse, +}: ReauthProps): ReauthState { + const [mfaOptions, setMfaOptions] = useState(); + const [challengeState, setChallengeState] = useState(); + + const [initAttempt, init] = useAsync(async () => { + const challenge = await auth.getMfaChallenge({ + scope: challengeScope, + }); + + setChallengeState({ challenge, deviceUsage: 'mfa' }); + setMfaOptions(getMfaChallengeOptions(challenge)); + }); + + useEffect(() => { + init(); + }, []); + + const getChallenge = useCallback( + async (deviceUsage: DeviceUsage = 'mfa') => { + if (challengeState?.deviceUsage === deviceUsage) { + return challengeState.challenge; + } + + // If the challenge state is empty, used, or has different args, + // retrieve a new mfa challenge and set it in the state. + const challenge = await auth.getMfaChallenge({ + scope: challengeScope, + userVerificationRequirement: + deviceUsage === 'passwordless' ? 'required' : 'discouraged', + }); + setChallengeState({ + challenge, + deviceUsage, + }); + return challenge; + }, + [challengeState, challengeScope] + ); + + const [submitAttempt, submitWithMfa, setSubmitAttempt] = useAsync( + useCallback( + async ( + mfaType?: DeviceType, + deviceUsage?: DeviceUsage, + totpCode?: string + ) => { + const challenge = await getChallenge(deviceUsage); + + let response: MfaChallengeResponse; + try { + response = await auth.getMfaChallengeResponse( + challenge, + mfaType, + totpCode + ); + } catch (err) { + throw new Error(getReAuthenticationErrorMessage(err)); + } - function submitWithWebauthn() { - setAttempt({ status: 'processing' }); - - if ('onMfaResponse' in props) { - auth - .getMfaChallenge({ scope: props.challengeScope }) - .then(challenge => auth.getMfaChallengeResponse(challenge, 'webauthn')) - .catch(handleError); - - return; - } - - auth - .createPrivilegeTokenWithWebauthn() - .then(props.onAuthenticated) - .catch((err: Error) => { - // This catches a webauthn frontend error that occurs on Firefox and replaces it with a more helpful error message. - if ( - err.message.includes('attempt was made to use an object that is not') - ) { - setAttempt({ - status: 'failed', - statusText: - 'The two-factor device you used is not registered on this account. You must verify using a device that has already been registered.', - }); - } else { - setAttempt({ status: 'failed', statusText: err.message }); + try { + await onMfaResponse(response); + } finally { + // once onMfaResponse is called, assume the challenge + // has been consumed and clear the state. + setChallengeState(null); } - }); - } + }, + [getChallenge, onMfaResponse] + ) + ); - function clearAttempt() { - setAttempt({ status: '' }); + function clearSubmitAttempt() { + setSubmitAttempt(makeEmptyAttempt()); } return { - attempt, - clearAttempt, - submitWithTotp, - submitWithWebauthn, - auth2faType: cfg.getAuth2faType(), - preferredMfaType: cfg.getPreferredMfaType(), - actionText, - onClose, + initAttempt, + mfaOptions, + submitWithMfa, + submitAttempt, + clearSubmitAttempt, }; } -const defaultActionText = 'performing this action'; - -type BaseProps = { - onClose?: () => void; - /** - * The text that will be appended to the text in the re-authentication dialog. - * - * Default value: "performing this action" - * - * Example: If `actionText` is set to "registering a new device" then the dialog will say - * "You must verify your identity with one of your existing two-factor devices before registering a new device." - * - * */ - actionText?: string; +export type ReauthProps = { + challengeScope: MfaChallengeScope; + onMfaResponse(res: MfaChallengeResponse): Promise; }; -// MfaResponseProps defines a function -// that accepts a MFA response. No -// authentication has been done at this point. -type MfaResponseProps = BaseProps & { - onMfaResponse(res: MfaChallengeResponse): void; - /** - * The MFA challenge scope of the action to perform, as defined in webauthn.proto. - */ - challengeScope: MfaChallengeScope; - onAuthenticated?: never; +export type ReauthState = { + initAttempt: Attempt; + mfaOptions: MfaOption[]; + submitWithMfa: ( + mfaType?: DeviceType, + deviceUsage?: DeviceUsage, + totpCode?: string + ) => Promise<[void, Error]>; + submitAttempt: Attempt; + clearSubmitAttempt: () => void; }; -// DefaultProps defines a function that -// accepts a privilegeTokenId that is only -// obtained after MFA response has been -// validated. -type DefaultProps = BaseProps & { - onAuthenticated(privilegeTokenId: string): void; - onMfaResponse?: never; - challengeScope?: never; +type challengeState = { + challenge: MfaAuthenticateChallenge; + deviceUsage: DeviceUsage; }; -export type Props = MfaResponseProps | DefaultProps; +function getReAuthenticationErrorMessage(err: Error): string { + if (err.message.includes('attempt was made to use an object that is not')) { + // Catch a webauthn frontend error that occurs on Firefox and replace it with a more helpful error message. + return 'The two-factor device you used is not registered on this account. You must verify using a device that has already been registered.'; + } + + if (err.message === 'invalid totp token') { + // This message relies on the status message produced by the auth server in + // lib/auth/Server.checkOTP function. Please keep these in sync. + return 'Invalid authenticator code'; + } -export type State = ReturnType; + return err.message; +} diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index f0f30f7356b6d..3724f1dc8b056 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -208,17 +208,12 @@ const auth = { }); }, - changePassword({ - oldPassword, - newPassword, - secondFactorToken, - webauthnResponse, - }: ChangePasswordReq) { + changePassword({ oldPassword, newPassword, mfaResponse }: ChangePasswordReq) { const data = { old_password: base64EncodeUnicode(oldPassword), new_password: base64EncodeUnicode(newPassword), - second_factor_token: secondFactorToken, - webauthnAssertionResponse: webauthnResponse, + second_factor_token: mfaResponse.totp_code, + webauthnAssertionResponse: mfaResponse.webauthn_response, }; return api.put(cfg.api.changeUserPasswordPath, data); @@ -259,10 +254,6 @@ const auth = { return api.put(cfg.getHeadlessSsoPath(transactionId), request); }, - createPrivilegeTokenWithTotp(secondFactorToken: string) { - return api.post(cfg.api.createPrivilegeTokenPath, { secondFactorToken }); - }, - // getChallenge gets an MFA challenge for the provided parameters. If is_mfa_required_req // is provided and it is found that MFA is not required, returns null instead. async getMfaChallenge( @@ -332,18 +323,27 @@ const auth = { ); }, - // TODO(Joerger): Combine with otp endpoint. + createPrivilegeToken(existingMfaResponse?: MfaChallengeResponse) { + return api.post(cfg.api.createPrivilegeTokenPath, { + existingMfaResponse, + // TODO(Joerger): DELETE IN v19.0.0 + // Also provide totp/webauthn response in backwards compatible format. + secondFactorToken: existingMfaResponse?.totp_code, + webauthnAssertionResponse: existingMfaResponse?.webauthn_response, + }); + }, + + // TODO(Joerger): Delete once no longer used by /e createPrivilegeTokenWithWebauthn() { - // Creating privilege tokens always expects the MANAGE_DEVICES webauthn scope. return auth .getMfaChallenge({ scope: MfaChallengeScope.MANAGE_DEVICES }) .then(auth.getMfaChallengeResponse) - .then(res => - api.post(cfg.api.createPrivilegeTokenPath, { - // TODO(Joerger): Handle non-webauthn challenges. - webauthnAssertionResponse: res.webauthn_response, - }) - ); + .then(mfaResp => auth.createPrivilegeToken(mfaResp)); + }, + + // TODO(Joerger): Delete once no longer used by /e + createPrivilegeTokenWithTotp(secondFactorToken: string) { + return api.post(cfg.api.createPrivilegeTokenPath, { secondFactorToken }); }, createRestrictedPrivilegeToken() { diff --git a/web/packages/teleport/src/services/auth/types.ts b/web/packages/teleport/src/services/auth/types.ts index 7c74f666d9db1..57fb003ab7975 100644 --- a/web/packages/teleport/src/services/auth/types.ts +++ b/web/packages/teleport/src/services/auth/types.ts @@ -18,7 +18,7 @@ import { EventMeta } from 'teleport/services/userEvent'; -import { DeviceUsage, WebauthnAssertionResponse } from '../mfa'; +import { DeviceUsage, MfaChallengeResponse } from '../mfa'; import { IsMfaRequiredRequest, MfaChallengeScope } from './auth'; @@ -73,8 +73,7 @@ export type CreateAuthenticateChallengeRequest = { export type ChangePasswordReq = { oldPassword: string; newPassword: string; - secondFactorToken: string; - webauthnResponse?: WebauthnAssertionResponse; + mfaResponse?: MfaChallengeResponse; }; export type CreateNewHardwareDeviceRequest = { diff --git a/web/packages/teleport/src/services/mfa/mfaOptions.test.ts b/web/packages/teleport/src/services/mfa/mfaOptions.test.ts index 5c430a05be8a8..81eec4be87a80 100644 --- a/web/packages/teleport/src/services/mfa/mfaOptions.test.ts +++ b/web/packages/teleport/src/services/mfa/mfaOptions.test.ts @@ -94,7 +94,7 @@ describe('test retrieving mfa options from MFA Challenge', () => { webauthnPublicKey: {} as PublicKeyCredentialRequestOptions, ssoChallenge: Object.create(SSOChallenge), }, - expect: ['webauthn', 'totp', 'sso'], + expect: ['webauthn', 'sso', 'totp'], }, ]; diff --git a/web/packages/teleport/src/services/mfa/mfaOptions.ts b/web/packages/teleport/src/services/mfa/mfaOptions.ts index 4bbe1dceb65f1..96510d31e668f 100644 --- a/web/packages/teleport/src/services/mfa/mfaOptions.ts +++ b/web/packages/teleport/src/services/mfa/mfaOptions.ts @@ -20,6 +20,7 @@ import { Auth2faType } from 'shared/services'; import { DeviceType, MfaAuthenticateChallenge, SSOChallenge } from './types'; +// returns mfa challenge options in order of preferences: WebAuthn > SSO > TOTP. export function getMfaChallengeOptions(mfaChallenge: MfaAuthenticateChallenge) { const mfaOptions: MfaOption[] = []; @@ -27,12 +28,12 @@ export function getMfaChallengeOptions(mfaChallenge: MfaAuthenticateChallenge) { mfaOptions.push(MFA_OPTION_WEBAUTHN); } - if (mfaChallenge?.totpChallenge) { - mfaOptions.push(MFA_OPTION_TOTP); + if (mfaChallenge?.ssoChallenge) { + mfaOptions.push(getSsoMfaOption(mfaChallenge.ssoChallenge)); } - if (mfaChallenge?.ssoChallenge) { - mfaOptions.push(getSsoOption(mfaChallenge.ssoChallenge)); + if (mfaChallenge?.totpChallenge) { + mfaOptions.push(MFA_OPTION_TOTP); } return mfaOptions; @@ -57,22 +58,28 @@ export type MfaOption = { label: string; }; -const MFA_OPTION_WEBAUTHN: MfaOption = { +export const MFA_OPTION_WEBAUTHN: MfaOption = { value: 'webauthn', label: 'Passkey or Security Key', }; -const MFA_OPTION_TOTP: MfaOption = { +export const MFA_OPTION_TOTP: MfaOption = { value: 'totp', label: 'Authenticator App', }; -const getSsoOption = (ssoChallenge: SSOChallenge): MfaOption => { +// SSO MFA option used in tests. +export const MFA_OPTION_SSO_DEFAULT: MfaOption = { + value: 'sso', + label: 'SSO', +}; + +const getSsoMfaOption = (ssoChallenge: SSOChallenge): MfaOption => { return { value: 'sso', label: - ssoChallenge.device?.displayName || - ssoChallenge.device?.connectorId || + ssoChallenge?.device?.displayName || + ssoChallenge?.device?.connectorId || 'SSO', }; };