From 531df4807a30fc4f191b1358aaa7edd495b606ee Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 20 Oct 2026 23:22:43 -0600 Subject: [PATCH 1/6] feat(mosaic): reverify sessions before SMS and authenticator MFA setup --- .changeset/mfa-setup-reverification.md | 2 + ...-profile-mfa-interactions.feature.test.tsx | 13 ++- ...er-profile-mfa-management.feature.test.tsx | 90 +++++++++++++++++++ .../user-profile-mfa-section.controller.ts | 27 +++--- .../user-profile-mfa-section.model.ts | 30 +++++-- .../user-profile-mfa-section.types.ts | 3 + 6 files changed, 141 insertions(+), 24 deletions(-) create mode 100644 .changeset/mfa-setup-reverification.md diff --git a/.changeset/mfa-setup-reverification.md b/.changeset/mfa-setup-reverification.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mfa-setup-reverification.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-interactions.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-interactions.feature.test.tsx index 52460d42121..f7a823e518b 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-interactions.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-interactions.feature.test.tsx @@ -92,22 +92,19 @@ describe('User profile MFA interaction boundaries', () => { expect(await screen.findByRole('img', { name: /QR code/i })).toBeVisible(); }); - it('keeps the dialog closed when dismissed while authenticator preparation is pending', async () => { + it('keeps the dialog open until authenticator preparation responds', async () => { const fapi = await renderMfa(); const held = holdRequests('post', '/v1/me/totp'); const user = userEvent.setup(); await user.click(screen.getByRole('button', { name: 'Add verification method' })); await user.click(screen.getByRole('button', { name: /Authenticator app/ })); await waitFor(() => expect(held.requests).toHaveLength(1)); + const dialog = screen.getByRole('dialog'); await user.keyboard('{Escape}'); - await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + expect(dialog).toBeVisible(); held.release(); - await waitFor(() => expect(fapi.mfa.totpCreations).toBe(1)); - expect(screen.queryByRole('dialog')).toBeNull(); - await user.click(screen.getByRole('button', { name: 'Add verification method' })); - expect(await screen.findByRole('dialog', { name: 'Add 2-step verification' })).toHaveAccessibleDescription( - 'Choose a verification method', - ); + expect(await screen.findByRole('img', { name: /QR code/i })).toBeVisible(); + expect(fapi.mfa.totpCreations).toBe(1); }); it('shows backup codes when dismissed while authenticator verification is pending', async () => { diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx index c90e2e9d762..fefad38cd19 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx @@ -491,6 +491,96 @@ describe('User profile MFA management', () => { expect(fapi.mfa.backupCodeCreations).toBe(1); }); + const startSms = async (user: ReturnType) => { + await user.click(screen.getByRole('button', { name: 'Add verification method' })); + await user.click(screen.getByRole('button', { name: /SMS verification/ })); + }; + + it('verifies the session before reserving an existing phone for SMS MFA', async () => { + const fapi = await renderMfa(undefined, mfaEnvironment(), { verification: { secrets: { password: 'hunter2' } } }); + demandReverificationOnce('/v1/me/phone_numbers/phone_1'); + const user = userEvent.setup(); + + await startSms(user); + await user.click(screen.getByRole('button', { name: 'Continue' })); + expect(await screen.findByText('Enter your current password to continue')).toBeVisible(); + await user.type(screen.getByLabelText('Password'), 'hunter2{Enter}'); + + expect(await screen.findByText('CODE0001')).toBeVisible(); + expect(fapi.mfa.phoneUpdates).toContainEqual({ id: 'phone_1', reserved: true, default: undefined }); + }); + + it('verifies the session before creating a phone for SMS MFA, then sends its code', async () => { + const fapi = await renderMfa(fapiUser({ id: 'user_1' }), mfaEnvironment(), { + verification: { secrets: { password: 'hunter2' } }, + }); + demandReverificationOnce('/v1/me/phone_numbers'); + const user = userEvent.setup(); + + await startSms(user); + await user.type(screen.getByRole('textbox', { name: 'Phone' }), '5555550303'); + await user.click(screen.getByRole('button', { name: 'Send code' })); + expect(await screen.findByText('Enter your current password to continue')).toBeVisible(); + await user.type(screen.getByLabelText('Password'), 'hunter2{Enter}'); + + await waitFor(() => expect(fapi.mfa.phonePreparations).toHaveLength(1)); + expect(fapi.mfa.phoneCreations).toHaveLength(1); + expect(await screen.findByRole('button', { name: 'Verify' })).toBeVisible(); + }); + + it('lets the user cancel SMS MFA verification quietly and add the method again', async () => { + const fapi = await renderMfa(undefined, mfaEnvironment(), { verification: { secrets: { password: 'hunter2' } } }); + demandReverificationOnce('/v1/me/phone_numbers/phone_1'); + const user = userEvent.setup(); + + await startSms(user); + await user.click(screen.getByRole('button', { name: 'Continue' })); + await screen.findByText('Enter your current password to continue'); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Close' })); + + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + expect(fapi.mfa.phoneUpdates).toEqual([]); + expect(screen.queryByRole('alert')).toBeNull(); + + await startSms(user); + await user.click(screen.getByRole('button', { name: 'Continue' })); + expect(await screen.findByText('CODE0001')).toBeVisible(); + }); + + it('verifies the session before starting authenticator setup', async () => { + const fapi = await renderMfa(undefined, mfaEnvironment(), { verification: { secrets: { password: 'hunter2' } } }); + demandReverificationOnce('/v1/me/totp'); + const user = userEvent.setup(); + + await user.click(screen.getByRole('button', { name: 'Add verification method' })); + await user.click(screen.getByRole('button', { name: /Authenticator app/ })); + expect(await screen.findByText('Enter your current password to continue')).toBeVisible(); + await user.type(screen.getByLabelText('Password'), 'hunter2{Enter}'); + + expect(await screen.findByRole('img', { name: /QR code/i })).toBeVisible(); + expect(fapi.mfa.totpCreations).toBe(1); + }); + + it('lets the user cancel authenticator verification quietly and start setup again', async () => { + const fapi = await renderMfa(undefined, mfaEnvironment(), { verification: { secrets: { password: 'hunter2' } } }); + demandReverificationOnce('/v1/me/totp'); + const user = userEvent.setup(); + + await user.click(screen.getByRole('button', { name: 'Add verification method' })); + await user.click(screen.getByRole('button', { name: /Authenticator app/ })); + await screen.findByText('Enter your current password to continue'); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Close' })); + + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + expect(fapi.mfa.totpCreations).toBe(0); + expect(screen.queryByRole('alert')).toBeNull(); + + await user.click(screen.getByRole('button', { name: 'Add verification method' })); + await user.click(screen.getByRole('button', { name: /Authenticator app/ })); + expect(await screen.findByRole('img', { name: /QR code/i })).toBeVisible(); + expect(fapi.mfa.totpCreations).toBe(1); + }); + it('copies server-issued backup codes and closes their one-time surface', async () => { const fapi = await renderMfa(); const write = vi.spyOn(navigator.clipboard, 'writeText').mockResolvedValue(); diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts index 0c0f90bbc16..aa80fa46a0f 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts @@ -6,6 +6,7 @@ import { setup } from '../../../machine/setup'; import type { DoneInvokeEvent } from '../../../machine/types'; import { useMachine } from '../../../machine/use-machine'; import { toLocalizableError } from '../../../utils/errors'; +import type { ReverificationController } from '../../reverification'; import type { UserProfileMfaAddableMethod, UserProfileMfaModel } from './user-profile-mfa-section.types'; import type { UserProfileMfaSectionViewProps } from './user-profile-mfa-section.view'; import type { UserProfileMfaSetupViewProps } from './user-profile-mfa-setup.view'; @@ -61,6 +62,7 @@ type MfaEvent = const { createMachine, assign, fromPromise } = setup(); const RESEND_COOLDOWN_SECONDS = 30; +const REVERIFIABLE_ACTIONS: readonly (MfaAction | undefined)[] = ['generate', 'sms', 'prepareAuthenticator']; const tick = { actions: assign(context => ({ resendSeconds: Math.max(0, context.resendSeconds - 1) })) }; const mfaMachine = createMachine({ @@ -413,6 +415,7 @@ function createSetupProps( send: MfaSend, actions: MfaSetupActions, errorText: ReturnType, + reverification: ReverificationController, ): UserProfileMfaSetupViewProps { const flow = context.flow; const sms = flow.kind === 'sms' ? flow : undefined; @@ -420,14 +423,14 @@ function createSetupProps( const backup = flow.kind === 'backup' ? flow : undefined; return { step: - flow.kind === 'closed' - ? 'select' - : flow.kind === 'backup' - ? context.pending === 'generate' && model.backupCodesReverification.visible - ? 'verify' - : 'backup-codes' - : flow.kind, - reverification: model.backupCodesReverification, + REVERIFIABLE_ACTIONS.includes(context.pending) && reverification.visible + ? 'verify' + : flow.kind === 'closed' + ? 'select' + : flow.kind === 'backup' + ? 'backup-codes' + : flow.kind, + reverification, methods: model.addableMethods, onSelect: actions.select, onBack: actions.back, @@ -521,6 +524,10 @@ export function useUserProfileMfaController(model: ReadyModel) { } }; + const reverification = + [model.createPhoneReverification, model.enrollSmsReverification, model.authenticatorReverification].find( + controller => controller.visible, + ) ?? model.backupCodesReverification; const setupProps = createSetupProps( model, context, @@ -539,6 +546,7 @@ export function useUserProfileMfaController(model: ReadyModel) { print, }, errorText, + reverification, ); const sectionProps: UserProfileMfaSectionViewProps = { @@ -552,7 +560,6 @@ export function useUserProfileMfaController(model: ReadyModel) { }; const showAddTrigger = model.addableMethods.length > 0; - const reverification = model.backupCodesReverification; const reverifying = setupProps.step === 'verify'; return { @@ -561,7 +568,7 @@ export function useUserProfileMfaController(model: ReadyModel) { showAddControl: showAddTrigger || dialogOpen, reverifying, onDialogOpenChange: (open: boolean) => { - if (!open && context.pending === 'generate') { + if (!open && REVERIFIABLE_ACTIONS.includes(context.pending)) { if (!reverification.onCancel) { return; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.model.ts b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.model.ts index 888aaf68166..8c712130fb0 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.model.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.model.ts @@ -112,7 +112,11 @@ function createSmsActions( userId: string, sessionId: string, refresh: () => Promise, - makeDefault: (phone: PhoneNumberResource) => Promise, + calls: { + makeDefault: (phone: PhoneNumberResource) => Promise; + createPhone: (current: UserResource, phoneNumber: string) => Promise; + reservePhone: (phone: PhoneNumberResource) => Promise; + }, ): Pick { const currentPhone = (phoneId: string) => { const current = requireIdentity(clerk, userId, sessionId); @@ -129,7 +133,7 @@ function createSmsActions( const existing = current.phoneNumbers.find( phone => !phone.reservedForSecondFactor && phone.phoneNumber === phoneNumber, ); - return summarizePhone(existing ?? (await current.createPhoneNumber({ phoneNumber }))); + return summarizePhone(existing ?? (await calls.createPhone(current, phoneNumber))); }, enrollSms: async (phoneId, code): Promise => { let phone = currentPhone(phoneId); @@ -144,7 +148,7 @@ function createSmsActions( if (phone.reservedForSecondFactor || !configuredFactors(environment).includes('phone_code')) { throw phoneUnavailable(); } - const result = await phone.setReservedForSecondFactor({ reserved: true }); + const result = await calls.reservePhone(phone); await refresh().catch(() => undefined); return { status: 'complete', backupCodes: result.backupCodes ?? [] }; }, @@ -156,7 +160,7 @@ function createSmsActions( if (phone.verification.status !== 'verified' || !phone.reservedForSecondFactor) { throw phoneUnavailable(); } - await makeDefault(phone); + await calls.makeDefault(phone); await refresh().catch(() => undefined); }, }; @@ -196,6 +200,15 @@ export function useUserProfileMfaModel(): UserProfileMfaModel { const [createBackupCode, backupCodesReverification] = useReverificationFlow((current: UserResource) => current.createBackupCode(), ); + const [createPhone, createPhoneReverification] = useReverificationFlow((current: UserResource, phoneNumber: string) => + current.createPhoneNumber({ phoneNumber }), + ); + const [reservePhone, enrollSmsReverification] = useReverificationFlow((phone: PhoneNumberResource) => + phone.setReservedForSecondFactor({ reserved: true }), + ); + const [createTotp, authenticatorReverification] = useReverificationFlow((current: UserResource) => + current.createTOTP(), + ); if (!userLoaded || !sessionLoaded || !environment) { return { status: 'loading' }; @@ -221,7 +234,9 @@ export function useUserProfileMfaModel(): UserProfileMfaModel { await current.reload(); }; - const smsActions = smsEnabled ? createSmsActions(clerk, environment, userId, sessionId, refresh, makeDefault) : {}; + const smsActions = smsEnabled + ? createSmsActions(clerk, environment, userId, sessionId, refresh, { makeDefault, createPhone, reservePhone }) + : {}; return { status: 'ready', @@ -239,7 +254,7 @@ export function useUserProfileMfaModel(): UserProfileMfaModel { if (current.totpEnabled || !configuredFactors(environment).includes('totp')) { throw new ClerkRuntimeError('Authenticator is unavailable.', { code: 'mfa_authenticator_unavailable' }); } - const result = await current.createTOTP(); + const result = await createTotp(current); return { secret: result.secret ?? '', uri: result.uri ?? '' }; } : undefined, @@ -268,5 +283,8 @@ export function useUserProfileMfaModel(): UserProfileMfaModel { removeReverification, setDefaultReverification, backupCodesReverification, + createPhoneReverification, + enrollSmsReverification, + authenticatorReverification, }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.types.ts b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.types.ts index 2d87c6a0196..3581282f5c2 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.types.ts @@ -49,4 +49,7 @@ export type UserProfileMfaModel = removeReverification: ReverificationController; setDefaultReverification: ReverificationController; backupCodesReverification: ReverificationController; + createPhoneReverification: ReverificationController; + enrollSmsReverification: ReverificationController; + authenticatorReverification: ReverificationController; }; From 24348ab99e9dc14a746ce0f4deb2b840b5c300ea Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 21 Oct 2026 10:19:48 -0600 Subject: [PATCH 2/6] test(mosaic): cover MFA setup reverification focus --- .../user-profile-mfa-management.feature.test.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx index fefad38cd19..9eb429dad9c 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx @@ -504,7 +504,8 @@ describe('User profile MFA management', () => { await startSms(user); await user.click(screen.getByRole('button', { name: 'Continue' })); expect(await screen.findByText('Enter your current password to continue')).toBeVisible(); - await user.type(screen.getByLabelText('Password'), 'hunter2{Enter}'); + await waitFor(() => expect(screen.getByLabelText('Password')).toHaveFocus()); + await user.keyboard('hunter2{Enter}'); expect(await screen.findByText('CODE0001')).toBeVisible(); expect(fapi.mfa.phoneUpdates).toContainEqual({ id: 'phone_1', reserved: true, default: undefined }); @@ -521,7 +522,8 @@ describe('User profile MFA management', () => { await user.type(screen.getByRole('textbox', { name: 'Phone' }), '5555550303'); await user.click(screen.getByRole('button', { name: 'Send code' })); expect(await screen.findByText('Enter your current password to continue')).toBeVisible(); - await user.type(screen.getByLabelText('Password'), 'hunter2{Enter}'); + await waitFor(() => expect(screen.getByLabelText('Password')).toHaveFocus()); + await user.keyboard('hunter2{Enter}'); await waitFor(() => expect(fapi.mfa.phonePreparations).toHaveLength(1)); expect(fapi.mfa.phoneCreations).toHaveLength(1); @@ -555,7 +557,8 @@ describe('User profile MFA management', () => { await user.click(screen.getByRole('button', { name: 'Add verification method' })); await user.click(screen.getByRole('button', { name: /Authenticator app/ })); expect(await screen.findByText('Enter your current password to continue')).toBeVisible(); - await user.type(screen.getByLabelText('Password'), 'hunter2{Enter}'); + await waitFor(() => expect(screen.getByLabelText('Password')).toHaveFocus()); + await user.keyboard('hunter2{Enter}'); expect(await screen.findByRole('img', { name: /QR code/i })).toBeVisible(); expect(fapi.mfa.totpCreations).toBe(1); From 7ae152d7d8bc71d3dbe380914c112d12d9854d51 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 21 Oct 2026 10:20:08 -0600 Subject: [PATCH 3/6] fix(mosaic): focus MFA setup reverification fields --- .../user-profile-mfa-setup.view.tsx | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-setup.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-setup.view.tsx index 363cb957392..12541dbb03b 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-setup.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-setup.view.tsx @@ -1,4 +1,4 @@ -import { Flow } from '../../../components/flow'; +import { Flow, useFlowAutoFocus } from '../../../components/flow'; import type { ReverificationController } from '../../reverification'; import { Reverification } from '../../reverification'; import type { UserProfileAddAuthenticatorViewProps } from './user-profile-add-authenticator.view'; @@ -60,10 +60,21 @@ export function UserProfileMfaSetupView(props: UserProfileMfaSetupViewProps) { /> - + )} ); } + +function ReverifyStep(props: { controller: ReverificationController }) { + const initialFocusRef = useFlowAutoFocus(); + + return ( + + ); +} From ae4e285eb774df1b7d24c66bf2aaa40377926ea4 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 21 Oct 2026 12:03:30 -0600 Subject: [PATCH 4/6] refactor(mosaic): name the MFA reverification condition --- .../user-profile-mfa-section.controller.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts index aa80fa46a0f..05560a87a4a 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts @@ -421,15 +421,15 @@ function createSetupProps( const sms = flow.kind === 'sms' ? flow : undefined; const authenticator = flow.kind === 'authenticator' ? flow : undefined; const backup = flow.kind === 'backup' ? flow : undefined; + const isReverifying = REVERIFIABLE_ACTIONS.includes(context.pending) && reverification.visible; return { - step: - REVERIFIABLE_ACTIONS.includes(context.pending) && reverification.visible - ? 'verify' - : flow.kind === 'closed' - ? 'select' - : flow.kind === 'backup' - ? 'backup-codes' - : flow.kind, + step: isReverifying + ? 'verify' + : flow.kind === 'closed' + ? 'select' + : flow.kind === 'backup' + ? 'backup-codes' + : flow.kind, reverification, methods: model.addableMethods, onSelect: actions.select, From 7256f0fde92bb669b9c498b131fea900acc780dd Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Thu, 22 Oct 2026 12:36:57 -0600 Subject: [PATCH 5/6] fix(mosaic): align MFA dialog close controls with pending actions --- ...-profile-mfa-interactions.feature.test.tsx | 10 +++-- ...er-profile-mfa-management.feature.test.tsx | 45 +++++++++++++++++-- .../user-profile-mfa-section.controller.ts | 37 ++++++++------- 3 files changed, 70 insertions(+), 22 deletions(-) diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-interactions.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-interactions.feature.test.tsx index f7a823e518b..52c3128f777 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-interactions.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-interactions.feature.test.tsx @@ -100,9 +100,13 @@ describe('User profile MFA interaction boundaries', () => { await user.click(screen.getByRole('button', { name: /Authenticator app/ })); await waitFor(() => expect(held.requests).toHaveLength(1)); const dialog = screen.getByRole('dialog'); - await user.keyboard('{Escape}'); - expect(dialog).toBeVisible(); - held.release(); + try { + expect(within(dialog).queryByRole('button', { name: 'Close' })).toBeNull(); + await user.keyboard('{Escape}'); + expect(dialog).toBeVisible(); + } finally { + held.release(); + } expect(await screen.findByRole('img', { name: /QR code/i })).toBeVisible(); expect(fapi.mfa.totpCreations).toBe(1); }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx index 9eb429dad9c..059eecc91c5 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx @@ -9,6 +9,7 @@ import { fapiUrl, holdRequests, serveFapi, + VERIFICATION_CODE, worker, } from '../../../../__tests__/feature/fake-fapi'; import { fapiClient, fapiPhoneNumber, fapiSession, fapiUser } from '../../../../__tests__/feature/fapi'; @@ -370,9 +371,10 @@ describe('User profile MFA management', () => { } expect(screen.getByRole('button', { name: 'Copy and close' })).toBeVisible(); expect(screen.getByRole('button', { name: 'Copy and close' })).toHaveAttribute('aria-disabled', 'true'); - await waitFor(() => expect(screen.getByRole('button', { name: 'Close' })).toHaveFocus()); + expect(screen.queryByRole('button', { name: 'Close' })).toBeNull(); held.release(); await waitFor(() => expect(screen.getByText('CODE0100')).toBeVisible()); + expect(screen.getByRole('button', { name: 'Close' })).toBeVisible(); for (const name of ['Print', 'Download', 'Copy and close']) { expect(screen.getByRole('button', { name })).toBeEnabled(); } @@ -481,9 +483,13 @@ describe('User profile MFA management', () => { await regenerate(user); await waitFor(() => expect(first.requests).toHaveLength(1)); const dialog = screen.getByRole('dialog'); - await user.keyboard('{Escape}'); - expect(dialog).toBeVisible(); - first.release(); + try { + expect(within(dialog).queryByRole('button', { name: 'Close' })).toBeNull(); + await user.keyboard('{Escape}'); + expect(dialog).toBeVisible(); + } finally { + first.release(); + } expect(await within(dialog).findByText('Enter your current password to continue')).toBeVisible(); await user.type(screen.getByLabelText('Password'), 'hunter2{Enter}'); @@ -511,6 +517,37 @@ describe('User profile MFA management', () => { expect(fapi.mfa.phoneUpdates).toContainEqual({ id: 'phone_1', reserved: true, default: undefined }); }); + it('keeps SMS code verification open until its reservation challenge can be cancelled', async () => { + const unverified = fapiPhoneNumber({ ...phone, verification: { ...phone.verification, status: 'unverified' } }); + const fapi = await renderMfa(fapiUser({ id: 'user_1', phone_numbers: [unverified] }), mfaEnvironment(), { + verification: { secrets: { password: 'hunter2' } }, + }); + demandReverificationOnce('/v1/me/phone_numbers/phone_1'); + const user = userEvent.setup(); + + await startSms(user); + await user.click(screen.getByRole('button', { name: 'Continue' })); + const code = await screen.findByRole('textbox', { name: 'Verification code' }); + const attempt = holdRequests('post', '/v1/me/phone_numbers/phone_1/attempt_verification'); + await user.type(code, VERIFICATION_CODE); + await waitFor(() => expect(attempt.requests).toHaveLength(1)); + const dialog = screen.getByRole('dialog'); + try { + expect(within(dialog).queryByRole('button', { name: 'Close' })).toBeNull(); + await user.keyboard('{Escape}'); + expect(dialog).toBeVisible(); + } finally { + attempt.release(); + } + + expect(await within(dialog).findByText('Enter your current password to continue')).toBeVisible(); + await user.click(within(dialog).getByRole('button', { name: 'Close' })); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + expect(fapi.mfa.phoneAttempts).toEqual([{ id: 'phone_1', code: VERIFICATION_CODE }]); + expect(fapi.mfa.phoneUpdates).toEqual([]); + expect(screen.queryByRole('alert')).toBeNull(); + }); + it('verifies the session before creating a phone for SMS MFA, then sends its code', async () => { const fapi = await renderMfa(fapiUser({ id: 'user_1' }), mfaEnvironment(), { verification: { secrets: { password: 'hunter2' } }, diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts index 05560a87a4a..487e33941ce 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts @@ -62,7 +62,7 @@ type MfaEvent = const { createMachine, assign, fromPromise } = setup(); const RESEND_COOLDOWN_SECONDS = 30; -const REVERIFIABLE_ACTIONS: readonly (MfaAction | undefined)[] = ['generate', 'sms', 'prepareAuthenticator']; +const REVERIFIABLE_ACTIONS: readonly MfaAction[] = ['generate', 'sms', 'prepareAuthenticator']; const tick = { actions: assign(context => ({ resendSeconds: Math.max(0, context.resendSeconds - 1) })) }; const mfaMachine = createMachine({ @@ -421,15 +421,20 @@ function createSetupProps( const sms = flow.kind === 'sms' ? flow : undefined; const authenticator = flow.kind === 'authenticator' ? flow : undefined; const backup = flow.kind === 'backup' ? flow : undefined; - const isReverifying = REVERIFIABLE_ACTIONS.includes(context.pending) && reverification.visible; + const isReverifying = + context.pending !== undefined && REVERIFIABLE_ACTIONS.includes(context.pending) && reverification.visible; + let step: UserProfileMfaSetupViewProps['step']; + if (isReverifying) { + step = 'verify'; + } else if (flow.kind === 'closed') { + step = 'select'; + } else if (flow.kind === 'backup') { + step = 'backup-codes'; + } else { + step = flow.kind; + } return { - step: isReverifying - ? 'verify' - : flow.kind === 'closed' - ? 'select' - : flow.kind === 'backup' - ? 'backup-codes' - : flow.kind, + step, reverification, methods: model.addableMethods, onSelect: actions.select, @@ -561,6 +566,8 @@ export function useUserProfileMfaController(model: ReadyModel) { const showAddTrigger = model.addableMethods.length > 0; const reverifying = setupProps.step === 'verify'; + const hasPendingReverification = context.pending !== undefined && REVERIFIABLE_ACTIONS.includes(context.pending); + const canClose = !hasPendingReverification || Boolean(reverification.onCancel); return { dialogOpen, @@ -568,15 +575,15 @@ export function useUserProfileMfaController(model: ReadyModel) { showAddControl: showAddTrigger || dialogOpen, reverifying, onDialogOpenChange: (open: boolean) => { - if (!open && REVERIFIABLE_ACTIONS.includes(context.pending)) { - if (!reverification.onCancel) { - return; - } - reverification.onCancel(); + if (!open && !canClose) { + return; + } + if (!open && hasPendingReverification) { + reverification.onCancel?.(); } send({ type: open ? 'OPEN' : 'CLOSE' }); }, - showCloseButton: !reverifying || Boolean(reverification.onCancel), + showCloseButton: canClose, sectionProps, setupProps, }; From ad2f324def52b9b0576ce9b189cd3624fc9f83f5 Mon Sep 17 00:00:00 2001 From: Austin Calvelage Date: Thu, 22 Oct 2026 13:13:00 -0600 Subject: [PATCH 6/6] Update packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Höglund --- .../user-profile-mfa-section.controller.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts index 487e33941ce..62286ba4c01 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts @@ -529,6 +529,7 @@ export function useUserProfileMfaController(model: ReadyModel) { } }; + // The machine guarantees that only one is running/visible at a time const reverification = [model.createPhoneReverification, model.enrollSmsReverification, model.authenticatorReverification].find( controller => controller.visible,