From 875efd4a0d1f54fa98f6bd4d19976ad58fc5a19a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20=C5=9Al=C4=99zak?= Date: Wed, 21 Oct 2026 14:45:22 +0200 Subject: [PATCH 1/4] reorder mfa configuration flow --- .../ConfigureMfaPage/ConfigureMfaPage.tsx | 16 +++---- .../ConfigureMfaTimeoutProvider.tsx | 6 +-- .../hooks/useConfigureMfaStore.test.ts | 21 +++----- .../hooks/useConfigureMfaStore.tsx | 48 +++++-------------- .../hooks/useLeaveVerification.ts | 23 +++++++++ .../verify/ConfigureMfaVerify.tsx | 38 ++++++++++----- .../ConfigureSelectVerificationStep.tsx | 24 ++++++++-- .../ConfigureVerifyEmailStep.tsx | 16 ++++--- .../ConfigureVerifyFido2Step.tsx | 15 +++--- .../ConfigureVerifyOidcStep.tsx | 14 ++++-- .../ConfigureVerifyTotpStep.tsx | 13 +++-- 11 files changed, 131 insertions(+), 103 deletions(-) create mode 100644 new-ui/src/pages/full/ConfigureMfaPage/hooks/useLeaveVerification.ts diff --git a/new-ui/src/pages/full/ConfigureMfaPage/ConfigureMfaPage.tsx b/new-ui/src/pages/full/ConfigureMfaPage/ConfigureMfaPage.tsx index 697870a1a..c567176c9 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/ConfigureMfaPage.tsx +++ b/new-ui/src/pages/full/ConfigureMfaPage/ConfigureMfaPage.tsx @@ -7,6 +7,7 @@ import { } from './components/ConfigureMfaTimeoutProvider'; import { useConfigureMfaStore } from './hooks/useConfigureMfaStore'; import { ConfigureMfaVerify } from './verify/ConfigureMfaVerify'; +import { ConfigureSelectMethodsStep } from './verify/ConfigureSelectMethodsStep/ConfigureSelectMethodsStep'; import { ConfigureMfaWizard } from './wizard/ConfigureMfaWizard'; export const ConfigureMfaPage = () => { @@ -19,10 +20,8 @@ export const ConfigureMfaPage = () => { const ConfigureMfaContent = () => { const navigate = useNavigate(); - // a late authorization on the selection screen waits for the picks - const inWizard = useConfigureMfaStore( - (s) => s.authorized && isPresent(s.selectedMethods), - ); + const authorized = useConfigureMfaStore((s) => s.authorized); + const methodsSelected = useConfigureMfaStore((s) => isPresent(s.selectedMethods)); const handleSessionExpired = useConfigureMfaSessionExpired(); const leave = useCallback(() => { @@ -31,12 +30,13 @@ const ConfigureMfaContent = () => { return ( <> - {inWizard && ( - - )} - {!inWizard && ( + {!authorized && ( )} + {authorized && !methodsSelected && } + {authorized && methodsSelected && ( + + )} ); }; diff --git a/new-ui/src/pages/full/ConfigureMfaPage/components/ConfigureMfaTimeoutProvider.tsx b/new-ui/src/pages/full/ConfigureMfaPage/components/ConfigureMfaTimeoutProvider.tsx index b2453a9ef..b432004e4 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/components/ConfigureMfaTimeoutProvider.tsx +++ b/new-ui/src/pages/full/ConfigureMfaPage/components/ConfigureMfaTimeoutProvider.tsx @@ -27,12 +27,10 @@ export const useConfigureMfaSessionExpired = (): (() => void) => { export const ConfigureMfaTimeoutProvider = ({ children }: PropsWithChildren) => { const [expired, setExpired] = useState(false); const deadline = useConfigureMfaStore((s) => s.deadline); - // Picking only the email fallback opens on Finish before verification, hence the auth check. const settled = useConfigureMfaStore( (s) => - s.authorized && - (s.activeStep === ConfigureMfaStep.RecoveryCodes || - s.activeStep === ConfigureMfaStep.Finish), + s.activeStep === ConfigureMfaStep.RecoveryCodes || + s.activeStep === ConfigureMfaStep.Finish, ); const expire = useCallback(() => setExpired(true), []); diff --git a/new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.test.ts b/new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.test.ts index 41f63b005..e516b0bd9 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.test.ts +++ b/new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.test.ts @@ -54,9 +54,7 @@ describe('applyAuthorization', () => { }); }); - it('waits on the selection when the answer lands after Back', () => { - useConfigureMfaStore.getState().selectMethods([MfaMethod.Fido2]); - useConfigureMfaStore.getState().backFromVerification(); + it('authorizes before any factor is picked', () => { applyAuthorization(sessionId, authorizeResult); const state = useConfigureMfaStore.getState(); @@ -66,9 +64,7 @@ describe('applyAuthorization', () => { expect(state.deadline).not.toBeNull(); }); - it('sets up the picks confirmed after a late answer', () => { - useConfigureMfaStore.getState().selectMethods([MfaMethod.Fido2]); - useConfigureMfaStore.getState().backFromVerification(); + it('sets up the picks made after authorization', () => { applyAuthorization(sessionId, authorizeResult); useConfigureMfaStore.getState().selectMethods([MfaMethod.Fido2]); @@ -77,9 +73,7 @@ describe('applyAuthorization', () => { expect(state.deadline).not.toBeNull(); }); - it('finishes when the pick confirmed after a late answer is empty', () => { - useConfigureMfaStore.getState().selectMethods([MfaMethod.Fido2]); - useConfigureMfaStore.getState().backFromVerification(); + it('finishes when the pick made after authorization is empty', () => { applyAuthorization(sessionId, authorizeResult); useConfigureMfaStore.getState().selectMethods([]); @@ -88,15 +82,14 @@ describe('applyAuthorization', () => { expect(state.deadline).toBeNull(); }); - it('keeps the deadline of an unauthorized session with an empty pick', () => { - useConfigureMfaStore.setState({ deadline: '2030-01-01T00:00:00.000Z' }); - useConfigureMfaStore.getState().selectMethods([]); + it('returns to the verification picker on Back', () => { + useConfigureMfaStore.getState().selectVerificationMethod(MfaMethod.Email); + useConfigureMfaStore.getState().backFromVerification(); - expect(useConfigureMfaStore.getState().deadline).toBe('2030-01-01T00:00:00.000Z'); + expect(useConfigureMfaStore.getState().verificationMethod).toBeNull(); }); it('drops an answer for another session', () => { - useConfigureMfaStore.getState().selectMethods([MfaMethod.Fido2]); applyAuthorization('session-2', authorizeResult); expect(useConfigureMfaStore.getState().authorized).toBe(false); diff --git a/new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.tsx b/new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.tsx index fdf06de92..03b306660 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.tsx +++ b/new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.tsx @@ -39,10 +39,10 @@ type StoreValues = { configuredMethods: MfaMethodValue[]; /** Factors this run has set up, which is what the wizard steps through. */ completedMethods: MfaMethodValue[]; - /** Null until the selection step is done. Empty is valid, the email fallback configures - * a factor on its own. */ + /** Null until the selection step, which follows authorization, is done. Empty is valid, + * the email fallback configures a factor on its own. */ selectedMethods: MfaMethodValue[] | null; - /** Pre-ticked in the selection step, from the entry point or an earlier pass. */ + /** Pre-ticked in the selection step, from the entry point. */ initialSelection: MfaMethodValue[]; /** most preferred first, from the session since an older Core rejects some factors */ verificationMethods: MfaVerificationMethod[]; @@ -137,8 +137,6 @@ interface Store extends StoreValues { ) => void; selectMethods: (methods: MfaMethodValue[]) => void; selectVerificationMethod: (method: MfaVerificationMethod) => void; - /** Keeps the current picks ticked and the session alive. */ - backToSelection: () => void; backFromVerification: () => void; /** The fresh deadline bounds every setup still to come, not just the next one. */ authorize: (response: MfaConfigAuthorizeResult) => void; @@ -186,46 +184,22 @@ export const useConfigureMfaStore = create()( return { selectedMethods: methods, activeStep: firstStep(next), - // picked after a late authorization, so the deadline was kept for it - ...(current.authorized && { - deadline: sessionDeadline(next, current.deadline), - }), + deadline: sessionDeadline(next, current.deadline), }; }); }, selectVerificationMethod: (method) => { set({ verificationMethod: method }); }, - backToSelection: () => { - set((current) => ({ - initialSelection: current.selectedMethods ?? current.initialSelection, - selectedMethods: null, - verificationMethod: null, - activeStep: defaults.activeStep, - })); - }, backFromVerification: () => { - if (isPresent(get().verificationMethod)) { - set({ verificationMethod: null }); - return; - } - get().backToSelection(); + set({ verificationMethod: null }); }, authorize: (response) => { - set((current) => { - const deadline = dayjs.unix(response.deadline_timestamp).toISOString(); - // a late answer can land after Back, the steps wait for its Continue - if (!isPresent(current.selectedMethods)) { - return { authorized: true, recoveryCodes: response.recovery_codes, deadline }; - } - // The fallback enables email as it verifies, so only this authorization issues codes. - const next = { ...current, recoveryCodes: response.recovery_codes }; - return { - authorized: true, - recoveryCodes: response.recovery_codes, - deadline: sessionDeadline(next, deadline), - activeStep: firstStep(next), - }; + // The fallback enables email as it verifies, so only this authorization issues codes. + set({ + authorized: true, + recoveryCodes: response.recovery_codes, + deadline: dayjs.unix(response.deadline_timestamp).toISOString(), }); }, factorConfigured: (method, recoveryCodes) => { @@ -263,7 +237,7 @@ export const useConfigureMfaStore = create()( name: 'configure-mfa-store', storage: createJSONStorage(() => sessionStorage), // Bumped on every shape change: a stored session is never resumable across one. - version: 13, + version: 14, }, ), ); diff --git a/new-ui/src/pages/full/ConfigureMfaPage/hooks/useLeaveVerification.ts b/new-ui/src/pages/full/ConfigureMfaPage/hooks/useLeaveVerification.ts new file mode 100644 index 000000000..5fbd23c05 --- /dev/null +++ b/new-ui/src/pages/full/ConfigureMfaPage/hooks/useLeaveVerification.ts @@ -0,0 +1,23 @@ +import { useMutation } from '@tanstack/react-query'; +import { useCallback } from 'react'; +import { discardMfaConfiguration, useConfigureMfaStore } from './useConfigureMfaStore'; + +/** Verification opens the flow, so leaving it ends the session unless a method picker came first. */ +export const useLeaveVerification = (onCancel: () => void) => { + const hasPicker = useConfigureMfaStore((s) => s.verificationMethods.length > 1); + + const { mutate: cancel, isPending: isLeaving } = useMutation({ + mutationFn: discardMfaConfiguration, + onSettled: onCancel, + }); + + const leave = useCallback(() => { + if (hasPicker) { + useConfigureMfaStore.getState().backFromVerification(); + return; + } + cancel(); + }, [hasPicker, cancel]); + + return { leaveText: hasPicker ? 'Back' : 'Cancel', leave, isLeaving }; +}; diff --git a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureMfaVerify.tsx b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureMfaVerify.tsx index 6b54141e4..c1fbfef40 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureMfaVerify.tsx +++ b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureMfaVerify.tsx @@ -2,7 +2,6 @@ import { MfaMethod } from '../../../../shared/rust-api/types'; import { isPresent } from '../../../../shared/utils/isPresent'; import { useConfigureMfaStore } from '../hooks/useConfigureMfaStore'; import type { MfaVerificationMethod } from '../types'; -import { ConfigureSelectMethodsStep } from './ConfigureSelectMethodsStep/ConfigureSelectMethodsStep'; import { ConfigureSelectVerificationStep } from './ConfigureSelectVerificationStep/ConfigureSelectVerificationStep'; import { ConfigureVerifyEmailStep } from './ConfigureVerifyEmailStep/ConfigureVerifyEmailStep'; import { ConfigureVerifyFido2Step } from './ConfigureVerifyFido2Step/ConfigureVerifyFido2Step'; @@ -14,18 +13,13 @@ type Props = { onSessionExpired: () => void; }; -/** Picks what the wizard sets up, then verifies the session with an existing factor. */ +/** Verifies the session with an existing factor before anything can be picked to set up. */ export const ConfigureMfaVerify = ({ onCancel, onSessionExpired }: Props) => { const candidates = useConfigureMfaStore((s) => s.verificationMethods); - const methodsSelected = useConfigureMfaStore((s) => isPresent(s.selectedMethods)); const verificationMethod = useConfigureMfaStore((s) => s.verificationMethod); - if (!methodsSelected) { - return ; - } - if (candidates.length > 1 && !isPresent(verificationMethod)) { - return ; + return ; } // for the type only, a session with nothing to verify with is refused at start @@ -34,12 +28,32 @@ export const ConfigureMfaVerify = ({ onCancel, onSessionExpired }: Props) => { switch (method) { case MfaMethod.Totp: - return ; + return ( + + ); case MfaMethod.Email: - return ; + return ( + + ); case MfaMethod.Fido2: - return ; + return ( + + ); case MfaMethod.Oidc: - return ; + return ( + + ); } }; diff --git a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureSelectVerificationStep/ConfigureSelectVerificationStep.tsx b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureSelectVerificationStep/ConfigureSelectVerificationStep.tsx index 891111769..e9582e3c0 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureSelectVerificationStep/ConfigureSelectVerificationStep.tsx +++ b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureSelectVerificationStep/ConfigureSelectVerificationStep.tsx @@ -1,3 +1,4 @@ +import { useMutation } from '@tanstack/react-query'; import { useState } from 'react'; import { Button } from '../../../../../shared/components/Button/Button'; import { ButtonVariant } from '../../../../../shared/components/Button/types'; @@ -5,17 +6,29 @@ import { Controls } from '../../../../../shared/components/Controls/Controls'; import { FullPageTitle } from '../../../../../shared/components/FullPageTitle/FullPageTitle'; import { FullPage } from '../../../../../shared/layouts/FullPage/FullPage'; import { ConfigureMfaVerificatorFactorSelector } from '../../components/ConfigureMfaVerificatorFactorSelector/ConfigureMfaVerificatorFactorSelector'; -import { useConfigureMfaStore } from '../../hooks/useConfigureMfaStore'; +import { + discardMfaConfiguration, + useConfigureMfaStore, +} from '../../hooks/useConfigureMfaStore'; import type { MfaVerificationMethod } from '../../types'; import '../style.scss'; import './style.scss'; +interface Props { + onCancel: () => void; +} + /** shown only when the session offers more than one method */ -export const ConfigureSelectVerificationStep = () => { +export const ConfigureSelectVerificationStep = ({ onCancel }: Props) => { const methods = useConfigureMfaStore((s) => s.verificationMethods); const [selected, setSelected] = useState(methods[0]); + const { mutate: cancel, isPending: isCancelling } = useMutation({ + mutationFn: discardMfaConfiguration, + onSettled: onCancel, + }); + return ( {