From ccb1aaf70079678b9b6850a3b61cacea9c0b9e92 Mon Sep 17 00:00:00 2001 From: joerger Date: Fri, 13 Dec 2024 16:58:23 -0800 Subject: [PATCH] Fix error handling in Web MFA flow. --- .../ChangePasswordWizard.tsx | 159 ++++++++------- .../wizards/AddAuthDeviceWizard.tsx | 66 +++--- .../wizards/DeleteAuthDeviceWizard.tsx | 45 ++-- .../wizards/ReauthenticateStep.tsx | 48 ++--- .../TestConnection/TestConnection.tsx | 8 +- .../TestConnection/TestConnection.tsx | 4 +- .../Server/TestConnection/TestConnection.tsx | 2 +- .../ReAuthenticate/ReAuthenticate.tsx | 65 +++--- .../ReAuthenticate/useReAuthenticate.ts | 193 ++++++++++-------- .../teleport/src/services/auth/auth.ts | 4 +- 10 files changed, 300 insertions(+), 294 deletions(-) diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx index 56fda8f9cd90c..0d180ef5b4d10 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 { 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, { useEffect, 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,11 +31,15 @@ import { requiredPassword, } from 'shared/components/Validation/rules'; import { useAsync } from 'shared/hooks/useAsync'; +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 { @@ -44,7 +47,6 @@ import { MfaOption, WebauthnAssertionResponse, } from 'teleport/services/mfa'; -import useReAuthenticate from 'teleport/components/ReAuthenticate/useReAuthenticate'; export interface ChangePasswordWizardProps { hasPasswordless: boolean; @@ -59,46 +61,15 @@ export function ChangePasswordWizard({ }: ChangePasswordWizardProps) { 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, initReauthOptions] = useAsync(async () => { - let mfaOptions = await getMfaChallengeOptions(); - const reauthOptions = getReauthOptions(mfaOptions, hasPasswordless); - setReauthMethod(reauthOptions[0].value); - return reauthOptions; + const reauthState = useReAuthenticate({ + challengeScope: MfaChallengeScope.CHANGE_PASSWORD, + onMfaResponse: async mfaResponse => + setWebauthnResponse(mfaResponse.webauthn_response), }); - useEffect(() => { - initReauthOptions(); - }, []); - 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 ( @@ -165,11 +135,10 @@ export type ChangePasswordWizardStepProps = StepComponentProps & ChangePasswordStepProps; interface ReauthenticateStepProps { - reauthOptions: ReauthenticationOption[]; + hasPasswordless: boolean; reauthMethod: ReauthenticationMethod; - onReauthMethodChange(method: ReauthenticationMethod): void; - submitWithPasswordless(): Promise; - submitWithMfa(mfaType?: DeviceType): Promise; + setReauthMethod(method: ReauthenticationMethod): void; + reauthState: ReauthState; onClose(): void; } @@ -178,28 +147,45 @@ export function ReauthenticateStep({ refCallback, stepIndex, flowLength, - reauthOptions, + hasPasswordless, reauthMethod, - onReauthMethodChange, - submitWithPasswordless, - submitWithMfa, + setReauthMethod, + reauthState: { + initAttempt, + mfaOptions, + submitWithMfa, + clearSubmitAttempt, + submitAttempt, + }, onClose, }: ChangePasswordWizardStepProps) { - const [reauthAttempt, reauthenticate] = useAsync( + 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) => { - switch (reauthMethod) { - case 'passwordless': - await submitWithPasswordless(); - break; - case 'totp': - // totp is handled in the ChangePasswordStep - break; - default: - await submitWithMfa(reauthMethod); - break; - } - next(); - } + // totp is handled in the ChangePasswordStep + if (reauthMethod === 'totp') next(); + + const mfaType = + reauthMethod === 'passwordless' ? 'webauthn' : reauthMethod; + const deviceUsage = + reauthMethod === 'passwordless' ? 'passwordless' : 'mfa'; + + submitWithMfa(mfaType, deviceUsage).then(([, err]) => { + if (!err) throw next(); + }); + }, + [submitWithMfa, next] ); const onReauthenticate = (e: React.FormEvent) => { @@ -207,6 +193,38 @@ export function ReauthenticateStep({ 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 ( @@ -216,20 +234,23 @@ export function ReauthenticateStep({ title="Verify Identity" /> - {reauthAttempt.status === 'error' && ( - {reauthAttempt.statusText} + {submitAttempt.status === 'error' && ( + {submitAttempt.statusText} )} Verification Method
onReauthenticate(e)}> { + setReauthMethod(o as DeviceType); + clearSubmitAttempt(); + }} /> diff --git a/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx b/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx index 6a029fd78c7d5..bbce32df63f98 100644 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx +++ b/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx @@ -24,7 +24,7 @@ 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'; @@ -76,13 +76,17 @@ export function AddAuthDeviceWizard({ const [privilegeToken, setPrivilegeToken] = useState(); const [credential, setCredential] = useState(null); - const { attempt, clearAttempt, getMfaChallengeOptions, submitWithMfa } = - useReAuthenticate({ - challengeScope: MfaChallengeScope.MANAGE_DEVICES, - onMfaResponse: mfaResponse => { - auth.createPrivilegeToken(mfaResponse).then(setPrivilegeToken); - }, - }); + 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), + }); // Choose a new device type from the options available for the given 2fa type. // irrelevant if usage === 'passkey'. @@ -91,32 +95,23 @@ export function AddAuthDeviceWizard({ registerMfaOptions[0].value ); - // 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 [reauthMfaOptions, getMfaOptions] = useAsync(async () => { - const reauthMfaOptions = await getMfaChallengeOptions(); - - // registering first device does not require reauth, just get a privilege token. - // - // TODO(Joerger): v19.0.0 - // Registering first device does not require a privilege token anymore, - // but the existing web register endpoint requires privilege token. - // We have a new endpoint "/v1/webapi/users/privilege" which does not - // require token, but can't be used until v19 for backwards compatibility. - if (reauthMfaOptions.length === 0) { - await auth.createPrivilegeToken().then(setPrivilegeToken); - } - - return reauthMfaOptions; - }); - + // 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(() => { - getMfaOptions(); - }, []); + if (reauthState.mfaOptions?.length === 0) { + auth.createPrivilegeToken().then(setPrivilegeToken); + } + }, [reauthState.mfaOptions]); // Handle potential error states first. - switch (reauthMfaOptions.status) { + switch (reauthState.initAttempt.status) { case 'processing': return ( @@ -124,7 +119,7 @@ export function AddAuthDeviceWizard({ ); case 'error': - return ; + return ; case 'success': break; default: @@ -141,16 +136,13 @@ export function AddAuthDeviceWizard({ 0 + reauthState.mfaOptions.length > 0 ? 'withReauthentication' : 'withoutReauthentication' } // Step properties mfaRegisterOptions={registerMfaOptions} - mfaChallengeOptions={reauthMfaOptions.data} - reauthAttempt={attempt} - clearReauthAttempt={clearAttempt} - submitWithMfa={submitWithMfa} + reauthState={reauthState} usage={usage} privilegeToken={privilegeToken} credential={credential} diff --git a/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.tsx b/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.tsx index 0ee99393df36b..13d6e8a67ae73 100644 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.tsx +++ b/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.tsx @@ -20,16 +20,12 @@ 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, { useEffect, useState } from 'react'; +import { StepComponentProps, StepHeader, StepSlider } from 'design/StepSlider'; +import { useState } from 'react'; import useAttempt from 'shared/hooks/useAttemptNext'; import Box from 'design/Box'; -import { StepHeader } from 'design/StepSlider'; - -import { useAsync } from 'shared/hooks/useAsync'; - import Indicator from 'design/Indicator'; import useTeleport from 'teleport/useTeleport'; @@ -60,29 +56,19 @@ export function DeleteAuthDeviceWizard({ }: DeleteAuthDeviceWizardProps) { const [privilegeToken, setPrivilegeToken] = useState(''); - const { attempt, clearAttempt, getMfaChallengeOptions, submitWithMfa } = - 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" - // can be used after v19 backwards compatibly. - auth.createPrivilegeToken(mfaResponse).then(setPrivilegeToken); - }, - }); - - const [challengeOptions, getChallengeOptions] = useAsync(async () => { - return getMfaChallengeOptions(); + 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), }); - useEffect(() => { - getChallengeOptions(); - }, []); - // Handle potential error states first. - switch (challengeOptions.status) { + switch (reauthState.initAttempt.status) { case 'processing': return ( @@ -90,7 +76,7 @@ export function DeleteAuthDeviceWizard({ ); case 'error': - return ; + return ; case 'success': break; default: @@ -108,10 +94,7 @@ export function DeleteAuthDeviceWizard({ flows={wizardFlows} currFlow="default" // Step properties - reauthAttempt={attempt} - clearReauthAttempt={clearAttempt} - mfaChallengeOptions={challengeOptions.data} - submitWithMfa={submitWithMfa} + reauthState={reauthState} deviceToDelete={deviceToDelete} privilegeToken={privilegeToken} onClose={onClose} diff --git a/web/packages/teleport/src/Account/ManageDevices/wizards/ReauthenticateStep.tsx b/web/packages/teleport/src/Account/ManageDevices/wizards/ReauthenticateStep.tsx index 806d54c910919..5dd0fd28eafb5 100644 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/ReauthenticateStep.tsx +++ b/web/packages/teleport/src/Account/ManageDevices/wizards/ReauthenticateStep.tsx @@ -20,24 +20,19 @@ 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 { StepComponentProps, StepHeader } from 'design/StepSlider'; import Box from 'design/Box'; -import { Attempt } from 'shared/hooks/useAttemptNext'; - +import { ReauthState } from 'teleport/components/ReAuthenticate/useReAuthenticate'; import { DeviceType } from 'teleport/services/mfa'; -import { MfaOption } from 'teleport/services/mfa'; export type ReauthenticateStepProps = StepComponentProps & { - reauthAttempt: Attempt; - clearReauthAttempt(): void; - mfaChallengeOptions: MfaOption[]; - submitWithMfa(mfaType?: DeviceType, otpCode?: string): Promise; + reauthState: ReauthState; onClose(): void; }; @@ -46,14 +41,11 @@ export function ReauthenticateStep({ refCallback, stepIndex, flowLength, - mfaChallengeOptions, - reauthAttempt, - clearReauthAttempt, - submitWithMfa, + reauthState: { mfaOptions, submitWithMfa, submitAttempt, clearSubmitAttempt }, onClose, }: ReauthenticateStepProps) { const [otpCode, setOtpCode] = useState(''); - const [mfaOption, setMfaOption] = useState(mfaChallengeOptions[0].value); + const [mfaOption, setMfaOption] = useState(mfaOptions[0].value); const onOtpCodeChanged = (e: React.ChangeEvent) => { setOtpCode(e.target.value); @@ -65,11 +57,11 @@ export function ReauthenticateStep({ ) => { e.preventDefault(); if (!validator.validate()) return; - submitWithMfa(mfaOption, otpCode).then(next); + submitWithMfa(mfaOption, 'mfa', otpCode).then(([, err]) => { + if (!err) next(); + }); }; - const errorMessage = getReauthenticationErrorMessage(reauthAttempt); - return (
@@ -79,14 +71,16 @@ export function ReauthenticateStep({ title="Verify Identity" /> - {errorMessage && {errorMessage}} + {submitAttempt.status === 'error' && ( + {submitAttempt.statusText} + )} {mfaOption && Multi-factor type} {({ validator }) => ( onReauthenticate(e, validator)}> { setMfaOption(o as DeviceType); - clearReauthAttempt(); + clearSubmitAttempt(); }} /> {mfaOption === 'totp' && ( @@ -106,7 +100,7 @@ export function ReauthenticateStep({ value={otpCode} placeholder="123 456" onChange={onOtpCodeChanged} - readonly={reauthAttempt.status === 'processing'} + readonly={submitAttempt.status === 'processing'} /> )} @@ -130,15 +124,3 @@ export function ReauthenticateStep({
); } - -function getReauthenticationErrorMessage(attempt: Attempt): string { - 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; - } - } -} 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/Kubernetes/TestConnection/TestConnection.tsx b/web/packages/teleport/src/Discover/Kubernetes/TestConnection/TestConnection.tsx index 6933383168cd6..d2c3a0812f48e 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 54195cb3e6cc1..403b38feab3af 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.tsx b/web/packages/teleport/src/components/ReAuthenticate/ReAuthenticate.tsx index ac293d4c7da7d..7add4a659fb55 100644 --- a/web/packages/teleport/src/components/ReAuthenticate/ReAuthenticate.tsx +++ b/web/packages/teleport/src/components/ReAuthenticate/ReAuthenticate.tsx @@ -16,34 +16,32 @@ * along with this program. If not, see . */ -import React, { useEffect, useState } from 'react'; import { - Flex, Box, - Text, 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 { Alert, 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 { useAsync } from 'shared/hooks/useAsync'; +import Validation, { Validator } from 'shared/components/Validation'; +import { requiredToken } from 'shared/components/Validation/rules'; import { MfaOption } from 'teleport/services/mfa'; import useReAuthenticate, { - ReauthState, ReauthProps, + ReauthState, } from './useReAuthenticate'; export type Props = ReauthProps & { @@ -61,26 +59,21 @@ export type State = ReauthState & { export function ReAuthenticate({ onClose, - attempt, - clearAttempt, - getMfaChallengeOptions, + initAttempt, + mfaOptions, submitWithMfa, + submitAttempt, + clearSubmitAttempt, }: State) { const [otpCode, setOtpToken] = useState(''); const [mfaOption, setMfaOption] = useState(); - const [challengeOptions, getChallengeOptions] = useAsync(async () => { - const mfaOptions = await getMfaChallengeOptions(); - setMfaOption(mfaOptions[0]); - return mfaOptions; - }); - useEffect(() => { - getChallengeOptions(); - }, []); + if (mfaOptions?.length) setMfaOption(mfaOptions[0]); + }, [mfaOptions]); // Handle potential error states first. - switch (challengeOptions.status) { + switch (initAttempt.status) { case 'processing': return ( @@ -88,16 +81,20 @@ export function ReAuthenticate({ ); case 'error': - return ; + return ; case 'success': break; default: return null; } - function onSubmit(e: React.MouseEvent) { + function onReauthenticate( + e: React.MouseEvent, + validator: Validator + ) { e.preventDefault(); - submitWithMfa(mfaOption.value, otpCode); + if (!validator.validate()) return; + submitWithMfa(mfaOption.value, 'mfa', otpCode); } return ( @@ -119,9 +116,9 @@ export function ReAuthenticate({ two-factor devices before performing this action. - {attempt.status === 'failed' && ( + {submitAttempt.status === 'error' && ( - {attempt.statusText} + {submitAttempt.statusText} )} @@ -130,15 +127,15 @@ export function ReAuthenticate({ width="60%" label="Two-factor Type" value={mfaOption} - options={challengeOptions.data} + 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} /> @@ -151,7 +148,7 @@ export function ReAuthenticate({ value={otpCode} onChange={e => setOtpToken(e.target.value)} placeholder="123 456" - readonly={attempt.status === 'processing'} + readonly={submitAttempt.status === 'processing'} mb={0} /> )} @@ -160,8 +157,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 7c4b11d132164..ed8c73f3fe6da 100644 --- a/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts +++ b/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts @@ -16,112 +16,141 @@ * along with this program. If not, see . */ -import { useState } from 'react'; -import useAttempt, { Attempt } from 'shared/hooks/useAttemptNext'; +import { useCallback, useEffect, useState } from 'react'; +import { Attempt, makeEmptyAttempt, useAsync } from 'shared/hooks/useAsync'; import auth from 'teleport/services/auth'; import { MfaChallengeScope } from 'teleport/services/auth/auth'; import { - getMfaChallengeOptions as getChallengeOptions, DeviceType, + DeviceUsage, + getMfaChallengeOptions, MfaAuthenticateChallenge, MfaChallengeResponse, MfaOption, } from 'teleport/services/mfa'; -export default function useReAuthenticate(props: ReauthProps): ReauthState { - // Note that attempt state "success" is not used or required. - // After the user submits, the control is passed back - // to the caller who is responsible for rendering the `ReAuthenticate` - // component. - const { attempt, setAttempt } = useAttempt(''); - - const [challenge, setMfaChallenge] = useState(null); - - // Provide a custom error handler to catch a webauthn frontend error that occurs - // on Firefox and replace it with a more helpful error message. - const handleError = (err: Error) => { - 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.', - }); - return; - } else { - setAttempt({ status: 'failed', statusText: err.message }); - return; - } - }; - - async function getMfaChallenge() { - if (challenge) { - return challenge; - } +export default function useReAuthenticate({ + challengeScope, + onMfaResponse, +}: ReauthProps): ReauthState { + const [mfaOptions, setMfaOptions] = useState(); + const [challengeState, setChallengeState] = useState(); - return auth.getMfaChallenge({ scope: props.challengeScope }).then(chal => { - setMfaChallenge(chal); - return chal; + const [initAttempt, init] = useAsync(async () => { + const challenge = await auth.getMfaChallenge({ + scope: challengeScope, }); - } - - function clearMfaChallenge() { - setMfaChallenge(null); - } - - function getMfaChallengeOptions() { - return getMfaChallenge().then(getChallengeOptions); - } - - function submitWithMfa(mfaType?: DeviceType, totp_code?: string) { - setAttempt({ status: 'processing' }); - return getMfaChallenge() - .then(chal => auth.getMfaChallengeResponse(chal, mfaType, totp_code)) - .then(props.onMfaResponse) - .finally(clearMfaChallenge) - .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: '' }); + 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)); + } + + try { + await onMfaResponse(response); + } finally { + // once onMfaResponse is called, assume the challenge + // has been consumed and clear the state. + setChallengeState(null); + } + }, + [getChallenge, onMfaResponse] + ) + ); + + function clearSubmitAttempt() { + setSubmitAttempt(makeEmptyAttempt()); } return { - attempt, - clearAttempt, - getMfaChallenge, - getMfaChallengeOptions, + initAttempt, + mfaOptions, submitWithMfa, - submitWithPasswordless, + submitAttempt, + clearSubmitAttempt, }; } export type ReauthProps = { challengeScope: MfaChallengeScope; - onMfaResponse(res: MfaChallengeResponse): void; + onMfaResponse(res: MfaChallengeResponse): Promise; }; export type ReauthState = { - attempt: Attempt; - clearAttempt: () => void; - getMfaChallenge: () => Promise; - getMfaChallengeOptions: () => Promise; - submitWithMfa: (mfaType?: DeviceType, totp_code?: string) => Promise; - submitWithPasswordless: () => Promise; + initAttempt: Attempt; + mfaOptions: MfaOption[]; + submitWithMfa: ( + mfaType?: DeviceType, + deviceUsage?: DeviceUsage, + totpCode?: string + ) => Promise<[void, Error]>; + submitAttempt: Attempt; + clearSubmitAttempt: () => void; +}; + +type challengeState = { + challenge: MfaAuthenticateChallenge; + deviceUsage: DeviceUsage; }; + +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'; + } + + return err.message; +} diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index 45cadd4a8fdf0..3724f1dc8b056 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -328,8 +328,8 @@ const auth = { 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, + secondFactorToken: existingMfaResponse?.totp_code, + webauthnAssertionResponse: existingMfaResponse?.webauthn_response, }); },