From b4e63c1e1eacb3b4d2f62d9502c4573dd132a8eb Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 27 Nov 2024 16:56:45 -0800 Subject: [PATCH] Refactor ChangePasswordWizard to use useReauthenticate. --- .../ChangePasswordWizard.story.tsx | 14 +- .../ChangePasswordWizard.test.tsx | 5 +- .../ChangePasswordWizard.tsx | 190 +++++++++--------- .../wizards/AddAuthDeviceWizard.test.tsx | 2 +- .../wizards/AddAuthDeviceWizard.tsx | 2 +- .../wizards/ReauthenticateStep.tsx | 10 +- .../ReAuthenticate/ReAuthenticate.story.tsx | 1 + .../ReAuthenticate/useReAuthenticate.ts | 17 ++ .../teleport/src/services/auth/auth.ts | 12 +- .../teleport/src/services/auth/types.ts | 5 +- 10 files changed, 133 insertions(+), 125 deletions(-) diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.story.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.story.tsx index 07283ae6c1c8d..fce7e73aab09e 100644 --- a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.story.tsx +++ b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.story.tsx @@ -23,13 +23,9 @@ import Dialog from 'design/Dialog'; import { createTeleportContext } from 'teleport/mocks/contexts'; import { ContextProvider } from 'teleport'; -import { MfaDevice } from 'teleport/services/mfa'; +import { getMfaRegisterOptions, MfaDevice } from 'teleport/services/mfa'; -import { - ChangePasswordStep, - ReauthenticateStep, - createReauthOptions, -} from './ChangePasswordWizard'; +import { ChangePasswordStep, ReauthenticateStep } from './ChangePasswordWizard'; export default { title: 'teleport/Account/Manage Devices/Change Password Wizard', @@ -96,8 +92,6 @@ const devices: MfaDevice[] = [ }, ]; -const defaultReauthOptions = createReauthOptions('optional', true, devices); - const stepProps = { // StepComponentProps next() {}, @@ -108,8 +102,8 @@ const stepProps = { refCallback: () => {}, // Other props - reauthOptions: defaultReauthOptions, - reauthMethod: defaultReauthOptions[0].value, + reauthOptions: getMfaRegisterOptions('on'), + reauthMethod: 'webauthn', credential: { id: '', type: '' }, onReauthMethodChange() {}, onAuthenticated() {}, diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx index 419f89da430cd..dbf22789306de 100644 --- a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx +++ b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx @@ -26,10 +26,7 @@ import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; import { MfaChallengeResponse } from 'teleport/services/mfa'; -import { - ChangePasswordWizardProps, - createReauthOptions, -} from './ChangePasswordWizard'; +import { ChangePasswordWizardProps } from './ChangePasswordWizard'; import { ChangePasswordWizard } from '.'; diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx index 1a152f2ddcfbf..e5a0ff26feb5b 100644 --- a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx +++ b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx @@ -17,13 +17,13 @@ */ 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 React, { useEffect, useState } from 'react'; import FieldInput from 'shared/components/FieldInput'; import Validation, { Validator } from 'shared/components/Validation'; import { @@ -32,17 +32,21 @@ import { requiredPassword, } from 'shared/components/Validation/rules'; import { useAsync } from 'shared/hooks/useAsync'; -import { Auth2faType } from 'shared/services'; import Box from 'design/Box'; import { ChangePasswordReq } from 'teleport/services/auth'; import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; -import { MfaDevice } from 'teleport/services/mfa'; +import { + DeviceType, + MfaDevice, + WebauthnAssertionResponse, +} from 'teleport/services/mfa'; +import useReAuthenticate from 'teleport/components/ReAuthenticate/useReAuthenticate'; +import { Attempt } from 'shared/hooks/useAttemptNext'; +import Indicator from 'design/Indicator'; 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. */ @@ -52,22 +56,66 @@ export interface ChangePasswordWizardProps { } export function ChangePasswordWizard({ - auth2faType, passwordlessEnabled, devices, onClose, onSuccess, }: ChangePasswordWizardProps) { - const reauthOptions = createReauthOptions( - auth2faType, - passwordlessEnabled, - devices - ); - const [reauthMethod, setReauthMethod] = useState( - reauthOptions[0]?.value - ); - const [credential, setCredential] = useState(); - const reauthRequired = reauthOptions.length > 0; + const [webauthnResponse, setWebauthnResponse] = + useState(); + const { getMfaChallengeOptions, submitWithMfa, submitWithPasswordless } = + useReAuthenticate({ + challengeScope: MfaChallengeScope.CHANGE_PASSWORD, + onMfaResponse: mfaResponse => { + setWebauthnResponse(mfaResponse.webauthn_response); + }, + }); + + // Attempt to get an MFA challenge for an existing device. If the challenge is + // empty, the user has no existing device (e.g. SSO user) and can register their + // first device without re-authentication. + const [reauthOptions, getReauthOptions] = useAsync(async () => { + let reauthOptions: ReauthenticationOption[] = + await getMfaChallengeOptions(); + + // Be more specific about the WebAuthn device type (it's not a passkey). + reauthOptions = reauthOptions.map((o: ReauthenticationOption) => + o.value === 'webauthn' ? { ...o, label: 'Security Key' } : o + ); + + // Add passwordless as the default if available. + if ( + passwordlessEnabled && + devices.some(dev => dev.usage === 'passwordless') + ) { + reauthOptions.unshift({ value: 'passwordless', label: 'Passkey' }); + } + + setReauthMethod(reauthOptions[0].value); + return reauthOptions; + }); + + useEffect(() => { + getReauthOptions(); + }, []); + + const [reauthMethod, setReauthMethod] = useState(); + + // Handle potential error states first. + switch (reauthOptions.status) { + case 'processing': + return ( + + + + ); + case 'error': + return ; + case 'success': + break; + default: + return null; + } return ( @@ -94,56 +141,14 @@ 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' }); - } - - const mfaEnabled = auth2faType === 'on' || auth2faType === 'optional'; - - if ( - (auth2faType === 'webauthn' || mfaEnabled) && - 'mfaDevice' in methodsAllowedByDevices - ) { - options.push({ value: 'mfaDevice', label: 'MFA Device' }); - } - - if ( - (auth2faType === 'otp' || mfaEnabled) && - 'otp' in methodsAllowedByDevices - ) { - options.push({ value: 'otp', label: 'Authenticator App' }); - } - - 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'; -} - const wizardFlows = { withReauthentication: [ReauthenticateStep, ChangePasswordStep], - withoutReauthentication: [ChangePasswordStep], }; type ChangePasswordWizardStepProps = StepComponentProps & @@ -154,7 +159,8 @@ interface ReauthenticateStepProps { reauthOptions: ReauthenticationOption[]; reauthMethod: ReauthenticationMethod; onReauthMethodChange(method: ReauthenticationMethod): void; - onAuthenticated(res: Credential): void; + submitWithPasswordless(): Promise; + submitWithMfa(mfaType?: DeviceType): Promise; onClose(): void; } @@ -166,26 +172,24 @@ export function ReauthenticateStep({ reauthOptions, reauthMethod, onReauthMethodChange, - onAuthenticated, + submitWithPasswordless, + submitWithMfa, 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); - - // TODO(Joerger): handle non-webauthn response. - onAuthenticated(response.webauthn_response); + const [reauthAttempt, reauthenticate] = useAsync( + async (reauthMethod: ReauthenticationMethod) => { + switch (reauthMethod) { + case 'passwordless': + await submitWithPasswordless(); + break; + case 'webauthn': + await submitWithMfa('webauthn'); + break; } next(); } ); + const onReauthenticate = (e: React.FormEvent) => { e.preventDefault(); reauthenticate(reauthMethod); @@ -200,8 +204,8 @@ export function ReauthenticateStep({ title="Verify Identity" /> - {reauthenticateAttempt.status === 'error' && ( - {reauthenticateAttempt.statusText} + {reauthAttempt.status === 'error' && ( + {reauthAttempt.statusText} )} Verification Method
onReauthenticate(e)}> @@ -229,7 +233,7 @@ export function ReauthenticateStep({ } interface ChangePasswordStepProps { - credential: Credential; + webauthnResponse: WebauthnAssertionResponse; reauthMethod: ReauthenticationMethod; onClose(): void; onSuccess(): void; @@ -240,7 +244,7 @@ export function ChangePasswordStep({ prev, stepIndex, flowLength, - credential, + webauthnResponse, reauthMethod, onClose, onSuccess, @@ -248,9 +252,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) => { @@ -265,7 +269,7 @@ export function ChangePasswordStep({ setOldPassword(''); setNewPassword(''); setNewPassConfirmed(''); - setAuthCode(''); + setOtpCode(''); } async function onSubmit( @@ -278,8 +282,10 @@ export function ChangePasswordStep({ await changePassword({ oldPassword, newPassword, - secondFactorToken: authCode, - credential, + mfaResponse: { + totp_code: otpCode, + webauthn_response: webauthnResponse, + }, }); } @@ -324,14 +330,14 @@ export function ChangePasswordStep({ type="password" placeholder="Confirm Password" /> - {reauthMethod === 'otp' && ( + {reauthMethod === 'totp' && ( { render(); const createStep = within(screen.getByTestId('create-step')); - await user.click(createStep.getByLabelText('Hardware Device')); + await user.click(createStep.getByLabelText('Security Key')); await user.click( createStep.getByRole('button', { name: 'Create an MFA method' }) ); diff --git a/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx b/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx index be27e91c3c008..d08e4e4e75c80 100644 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx +++ b/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx @@ -267,7 +267,7 @@ function CreateMfaBox({ }) { // Be more specific about the WebAuthn device type (it's not a passkey). mfaRegisterOptions = mfaRegisterOptions.map((o: MfaOption) => - o.value === 'webauthn' ? { ...o, label: 'Hardware Device' } : o + o.value === 'webauthn' ? { ...o, label: 'Security Key' } : o ); return ( diff --git a/web/packages/teleport/src/Account/ManageDevices/wizards/ReauthenticateStep.tsx b/web/packages/teleport/src/Account/ManageDevices/wizards/ReauthenticateStep.tsx index b7e703daef72c..c36e133e73c7b 100644 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/ReauthenticateStep.tsx +++ b/web/packages/teleport/src/Account/ManageDevices/wizards/ReauthenticateStep.tsx @@ -46,9 +46,9 @@ export function ReauthenticateStep({ refCallback, stepIndex, flowLength, - reauthAttempt: attempt, mfaChallengeOptions, - clearReauthAttempt: clearAttempt, + reauthAttempt, + clearReauthAttempt, submitWithMfa, onClose, }: ReauthenticateStepProps) { @@ -68,7 +68,7 @@ export function ReauthenticateStep({ submitWithMfa(mfaOption, otpCode).then(next); }; - const errorMessage = getReauthenticationErrorMessage(attempt); + const errorMessage = getReauthenticationErrorMessage(reauthAttempt); return (
@@ -94,7 +94,7 @@ export function ReauthenticateStep({ mb={4} onChange={o => { setMfaOption(o as DeviceType); - clearAttempt(); + clearReauthAttempt(); }} /> {mfaOption === 'totp' && ( @@ -106,7 +106,7 @@ export function ReauthenticateStep({ value={otpCode} placeholder="123 456" onChange={onOtpCodeChanged} - readonly={attempt.status === 'processing'} + readonly={reauthAttempt.status === 'processing'} /> )} diff --git a/web/packages/teleport/src/components/ReAuthenticate/ReAuthenticate.story.tsx b/web/packages/teleport/src/components/ReAuthenticate/ReAuthenticate.story.tsx index 6e63786a6fd72..e2d4c896b9380 100644 --- a/web/packages/teleport/src/components/ReAuthenticate/ReAuthenticate.story.tsx +++ b/web/packages/teleport/src/components/ReAuthenticate/ReAuthenticate.story.tsx @@ -44,5 +44,6 @@ const props: Props = { getMfaChallenge: () => null, getMfaChallengeOptions: async () => getMfaRegisterOptions('on'), submitWithMfa: (mfaType?: DeviceType, totp_code?: string) => null, + submitWithPasswordless: () => null, onClose: () => null, }; diff --git a/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts b/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts index 4d9e991277827..1e7d1337db3f2 100644 --- a/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts +++ b/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts @@ -102,6 +102,21 @@ export default function useReAuthenticate(props: ReauthProps): ReauthState { .catch(handleError); } + function submitWithPasswordless() { + setAttempt({ status: 'processing' }); + // Always get a new passwordless challenge, the challenge stored in state is for mfa + // and will also be overwritten in the backend by the passwordless challenge. + return auth + .getMfaChallenge({ + scope: props.challengeScope, + userVerificationRequirement: 'required', + }) + .then(chal => auth.getMfaChallengeResponse(chal, 'webauthn')) + .then(props.onMfaResponse) + .finally(clearMfaChallenge) + .catch(handleError); + } + function clearAttempt() { setAttempt({ status: '' }); } @@ -112,6 +127,7 @@ export default function useReAuthenticate(props: ReauthProps): ReauthState { getMfaChallenge, getMfaChallengeOptions, submitWithMfa, + submitWithPasswordless, }; } @@ -128,4 +144,5 @@ export type ReauthState = { getMfaChallenge: () => Promise; getMfaChallengeOptions: () => Promise; submitWithMfa: (mfaType?: DeviceType, totp_code?: string) => Promise; + submitWithPasswordless: () => Promise; }; diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index 8d55c36666470..9790d30773900 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -210,18 +210,12 @@ const auth = { }); }, - changePassword({ - oldPassword, - newPassword, - secondFactorToken, - credential, - }: ChangePasswordReq) { + changePassword({ oldPassword, newPassword, mfaResponse }: ChangePasswordReq) { const data = { old_password: base64EncodeUnicode(oldPassword), new_password: base64EncodeUnicode(newPassword), - second_factor_token: secondFactorToken, - webauthnAssertionResponse: - credential && makeWebauthnAssertionResponse(credential), + second_factor_token: mfaResponse.totp_code, + webauthnAssertionResponse: mfaResponse.webauthn_response, }; return api.put(cfg.api.changeUserPasswordPath, data); diff --git a/web/packages/teleport/src/services/auth/types.ts b/web/packages/teleport/src/services/auth/types.ts index ae9818ef2ebb7..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 } 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; - credential?: Credential; + mfaResponse?: MfaChallengeResponse; }; export type CreateNewHardwareDeviceRequest = {