From d1c8bded3cc40e62e62624e352b6be817c5673c3 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Sat, 31 Oct 2026 11:09:22 -0600 Subject: [PATCH 1/6] test(mosaic): assert OTP reuse and request ordering --- .../__tests__/reverification.feature.test.tsx | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx b/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx index c66274726c7..bf73851f1ae 100644 --- a/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx +++ b/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx @@ -822,9 +822,11 @@ describe('Reverification', () => { await useAnotherMethod(user); const second = holdRequests('post', PREPARE_FIRST); await user.click(await screen.findByRole('button', { name: 'Email code to alice@work.example' })); - await waitFor(() => expect(second.requests).toHaveLength(1)); + await settle(); + expect(second.requests).toHaveLength(0); prepare.fail('preparation_failed', 'Could not send your code'); + await waitFor(() => expect(second.requests).toHaveLength(1)); await settle(); expect(screen.getByRole('button', { name: 'Email code to alice@work.example' })).toBeVisible(); expect(screen.queryByText('Could not send your code')).toBeNull(); @@ -846,10 +848,12 @@ describe('Reverification', () => { const next = holdRequests('post', PREPARE_FIRST); await user.click(screen.getByRole('button', { name: 'Run action' })); - expect(await screen.findByText(STEP.emailCode)).toBeVisible(); - await waitFor(() => expect(next.requests).toHaveLength(1)); + await settle(); + expect(next.requests).toHaveLength(0); prepare.fail('preparation_failed', 'Could not send your code'); + expect(await screen.findByText(STEP.emailCode)).toBeVisible(); + await waitFor(() => expect(next.requests).toHaveLength(1)); await settle(); expect(screen.getByText(STEP.emailCode)).toBeVisible(); expect(screen.queryByText('Could not send your code')).toBeNull(); @@ -1239,14 +1243,11 @@ describe('Reverification', () => { expect(await screen.findByText(STEP.password)).toBeVisible(); expect(firstPrepare.requests).toHaveLength(1); - // Picking an email code shows a pending row and disables the other actions while the code is sent const secondPrepare = holdRequests('post', PREPARE_FIRST); await useAnotherMethod(user); await user.click(await screen.findByRole('button', { name: 'Email code to alice@example.com' })); - await waitFor(() => expect(secondPrepare.requests).toHaveLength(1)); - expect(method('Send SMS code to +1 555 0100')).toBeDisabled(); - expect(method('Back')).toBeDisabled(); - secondPrepare.release(); + await screen.findByRole('group', { name: 'Verification code' }); + expect(secondPrepare.requests).toHaveLength(0); // The code step opens with an empty field and no error once the code is sent await screen.findByRole('group', { name: 'Verification code' }); @@ -1257,6 +1258,10 @@ describe('Reverification', () => { // Picking a phone code sends the SMS code through the first factor endpoints await useAnotherMethod(user); await user.click(await screen.findByRole('button', { name: 'Send SMS code to +1 555 0100' })); + await waitFor(() => expect(secondPrepare.requests).toHaveLength(1)); + expect(method('Continue with your password')).toBeDisabled(); + expect(method('Back')).toBeDisabled(); + secondPrepare.release(); expect(await screen.findByText(STEP.phoneCode)).toBeVisible(); expect(fapi.verification.firstFactorVerification?.strategy).toBe('phone_code'); @@ -1636,14 +1641,12 @@ describe('Reverification', () => { await waitFor(() => expect(outcome()).toBe('rejected: reverification_cancelled')); await untilClosed(); - // The new challenge opens from the first step with empty fields await user.click(screen.getByRole('button', { name: 'Run action' })); - expect(await screen.findByText(STEP.password)).toBeVisible(); - expect(passwordField()).toHaveValue(''); - expect(passwordField()).toBeEnabled(); + await settle(); + expect(screen.queryByText(STEP.password)).toBeNull(); - // The late response from the dismissed attempt has no effect attempt.release(); + expect(await screen.findByText(STEP.password)).toBeVisible(); await settle(); expect(passwordField()).toHaveValue(''); expect(passwordField()).toBeEnabled(); @@ -1714,10 +1717,14 @@ describe('Reverification', () => { await startVerification(); expect(await screen.findByText(STEP.password)).toBeVisible(); + const newStart = holdRequests('post', '/v1/client/sessions/sess_2/verify'); await act(() => clerk.setActive({ session: 'sess_2' })); await untilClosed(); await waitFor(() => expect(outcome()).toBe('rejected: reverification_cancelled')); + await settle(); + newStart.release(); + expect(newStart.requests).toHaveLength(0); }); it('keeps the challenge open while the session is briefly unset during a navigation', async () => { From 052411191cf99b63a70e5d8a5c98c6ce0a796a26 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Sat, 31 Oct 2026 11:09:45 -0600 Subject: [PATCH 2/6] fix(mosaic): reuse valid OTP preparation across method navigation --- .changeset/quiet-bees-verify.md | 5 + .../feature/fake-fapi/verification.ts | 47 +- ...everification-preparation.feature.test.tsx | 537 ++++++++++++++++++ .../reverification.controller.test.ts | 67 +++ .../reverification.controller.ts | 283 ++++++--- .../reverification/reverification.model.tsx | 61 +- .../reverification/reverification.types.ts | 9 +- .../use-reverification-with-state.ts | 11 +- 8 files changed, 905 insertions(+), 115 deletions(-) create mode 100644 .changeset/quiet-bees-verify.md create mode 100644 packages/mosaic/src/features/reverification/__tests__/reverification-preparation.feature.test.tsx create mode 100644 packages/mosaic/src/features/reverification/__tests__/reverification.controller.test.ts diff --git a/.changeset/quiet-bees-verify.md b/.changeset/quiet-bees-verify.md new file mode 100644 index 00000000000..eab094f0621 --- /dev/null +++ b/.changeset/quiet-bees-verify.md @@ -0,0 +1,5 @@ +--- +'@clerk/mosaic': patch +--- + +Reuse an unexpired verification code when switching back to the same email address or phone number without attempting another verification method. Returning to a code preserves its resend countdown, and pending code sends finish before another method is prepared or submitted. diff --git a/packages/mosaic/src/__tests__/feature/fake-fapi/verification.ts b/packages/mosaic/src/__tests__/feature/fake-fapi/verification.ts index 6bc62122ef4..e41ed349aa3 100644 --- a/packages/mosaic/src/__tests__/feature/fake-fapi/verification.ts +++ b/packages/mosaic/src/__tests__/feature/fake-fapi/verification.ts @@ -19,11 +19,16 @@ export interface FakeVerificationState { firstFactors: SignInFirstFactorJSON[]; secondFactors: SignInSecondFactorJSON[]; secrets: Partial>; + codesByIdentification: Partial>; + firstFactorCode: string | null; + secondFactorCode: string | null; firstFactorVerification: VerificationJSON | null; secondFactorVerification: VerificationJSON | null; } -export type FakeVerificationSeed = Partial>; +export type FakeVerificationSeed = Partial< + Pick +>; export interface VerificationHost { client: ClientJSON; @@ -37,6 +42,9 @@ export function createVerificationState(seed: FakeVerificationSeed = {}): FakeVe firstFactors: [{ strategy: 'password' }], secondFactors: [], secrets: {}, + codesByIdentification: {}, + firstFactorCode: null, + secondFactorCode: null, firstFactorVerification: null, secondFactorVerification: null, ...seed, @@ -195,7 +203,18 @@ export function verificationHandlers(state: VerificationHost, url: (path: string return verificationErrors.missingResource(idParam); } } - setStageVerification(state, stage, fapiVerification(strategy)); + if (strategy === 'email_code' || strategy === 'phone_code') { + const identification = body.get(strategy === 'email_code' ? 'email_address_id' : 'phone_number_id'); + const code = + (identification ? state.verification.codesByIdentification[identification] : undefined) ?? + state.verification.secrets[strategy] ?? + null; + state.verification = + stage === 'first' + ? { ...state.verification, firstFactorCode: code } + : { ...state.verification, secondFactorCode: code }; + } + setStageVerification(state, stage, fapiVerification(strategy, { expire_at: Date.now() + 600_000 })); return verificationReply(state, session); }; @@ -214,15 +233,33 @@ export function verificationHandlers(state: VerificationHost, url: (path: string if (PREPARABLE_STRATEGIES.includes(strategy) && prepared?.strategy !== strategy) { return verificationErrors.notSent(); } - if (strategy !== 'passkey' && state.verification.secrets[strategy as VerificationSecretStrategy] === undefined) { + if ( + PREPARABLE_STRATEGIES.includes(strategy) && + prepared && + prepared.expire_at !== null && + prepared.expire_at <= Date.now() + ) { + return apiError(422, 'verification_expired', 'This code has expired.'); + } + const secret = + strategy === 'email_code' || strategy === 'phone_code' + ? stage === 'first' + ? state.verification.firstFactorCode + : state.verification.secondFactorCode + : strategy === 'password' || strategy === 'totp' || strategy === 'backup_code' + ? state.verification.secrets[strategy] + : undefined; + if (strategy !== 'passkey' && secret === undefined) { throw new Error(`fake FAPI: no secret seeded for strategy "${strategy}"`); } if (strategy === 'password') { + setStageVerification(state, stage, fapiVerification('password')); + state.verification.firstFactorCode = null; if (body.get('password') !== state.verification.secrets.password) { return verificationErrors.passwordIncorrect(); } } else if (strategy !== 'passkey') { - if (body.get('code') !== state.verification.secrets[strategy as VerificationSecretStrategy]) { + if (body.get('code') !== secret) { return verificationErrors.codeIncorrect(); } } @@ -253,6 +290,8 @@ export function verificationHandlers(state: VerificationHost, url: (path: string status: level === 'second_factor' ? 'needs_second_factor' : 'needs_first_factor', firstFactorVerification: null, secondFactorVerification: null, + firstFactorCode: null, + secondFactorCode: null, }; return verificationReply(state, session); }), diff --git a/packages/mosaic/src/features/reverification/__tests__/reverification-preparation.feature.test.tsx b/packages/mosaic/src/features/reverification/__tests__/reverification-preparation.feature.test.tsx new file mode 100644 index 00000000000..df9f129c3b0 --- /dev/null +++ b/packages/mosaic/src/features/reverification/__tests__/reverification-preparation.feature.test.tsx @@ -0,0 +1,537 @@ +import { reverificationError } from '@clerk/shared/authorization-errors'; +import { ClerkWebAuthnError } from '@clerk/shared/error'; +import type { VerificationJSON } from '@clerk/shared/types'; +import { act, screen, waitFor, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { http, HttpResponse } from 'msw'; +import { useState } from 'react'; +import { describe, expect, it, vi } from 'vitest'; + +import { fapiUrl, holdRequests, serveFapi, worker } from '../../../__tests__/feature/fake-fapi'; +import type { FakeVerificationSeed } from '../../../__tests__/feature/fake-fapi/verification'; +import { + fapiClient, + fapiEmailAddress, + fapiEnvironment, + fapiSession, + fapiSessionVerification, + fapiUser, + fapiVerification, +} from '../../../__tests__/feature/fapi'; +import { renderWithClerk } from '../../../__tests__/feature/render'; +import { Card } from '../../../components/card'; +import { Reverification, useReverificationFlow } from '../reverification'; + +const PREPARE_FIRST = '/v1/client/sessions/sess_1/verify/prepare_first_factor'; +const EMAIL_STEP = 'Enter the code sent to your email to continue'; +const PASSWORD_STEP = 'Enter your current password to continue'; +const CODE = '123456'; +const PHONE_CODE = '234567'; +const WORK_CODE = '654321'; +const EMAIL_METHOD = 'Email code to alice@example.com'; +const PHONE_METHOD = 'Send SMS code to +1 555 0100'; +const PHONE_STEP = 'Enter the code sent to your phone to continue'; +const PREPARE_SECOND = '/v1/client/sessions/sess_1/verify/prepare_second_factor'; +const emailFactor = { + strategy: 'email_code', + email_address_id: 'idn_alice', + safe_identifier: 'alice@example.com', +} as const; +const passwordFactor = { strategy: 'password' } as const; +const phoneFactor = { strategy: 'phone_code', phone_number_id: 'idn_phone', safe_identifier: '+1 555 0100' } as const; +const workFactor = { + strategy: 'email_code', + email_address_id: 'idn_work', + safe_identifier: 'alice@work.example', +} as const; + +function Host({ action }: { action: () => Promise }) { + const [run, controller] = useReverificationFlow(action); + const [outcome, setOutcome] = useState(''); + return ( + <> + + {outcome} + + + {controller.onCancel ? ( + + ) : null} + + + ); +} + +async function renderPreparation( + action = vi.fn().mockResolvedValueOnce(reverificationError()).mockResolvedValue({ done: true }), + verification: FakeVerificationSeed = {}, +) { + const alice = fapiUser({ + email_addresses: [fapiEmailAddress({ id: 'idn_alice', email_address: 'alice@example.com' })], + }); + const session = fapiSession({ id: 'sess_1', user: alice }); + const fapi = serveFapi({ + client: fapiClient([session]), + environment: fapiEnvironment({ display_config: { preferred_sign_in_strategy: 'otp' } }), + verification: { + firstFactors: [emailFactor, passwordFactor], + secrets: { email_code: CODE, phone_code: PHONE_CODE, password: 'hunter2', totp: CODE, backup_code: 'backup-1' }, + codesByIdentification: { idn_alice: CODE, idn_work: WORK_CODE, idn_phone: PHONE_CODE }, + ...verification, + }, + }); + const view = await renderWithClerk(); + return { ...view, fapi, action, session }; +} + +async function selectMethod(user: ReturnType, name: string) { + await user.click(screen.getByRole('button', { name: 'Use another method' })); + await user.click(await screen.findByRole('button', { name })); +} + +async function pasteCode(user: ReturnType, code: string) { + const [first] = within(screen.getByRole('group', { name: 'Verification code' })).getAllByRole('textbox'); + if (!first) { + throw new Error('Missing code input'); + } + await user.click(first); + await user.paste(code); +} + +async function startPrepared(options: FakeVerificationSeed = {}) { + const view = await renderPreparation(undefined, options); + const prepares = holdRequests('post', PREPARE_FIRST); + prepares.release(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Run action' })); + await vi.waitFor(() => expect(view.fapi.verification.firstFactorVerification?.strategy).toBe('email_code')); + return { ...view, prepares, user }; +} + +describe('Reverification preparation', () => { + it('reuses the email code after visiting password without attempting it', async () => { + const { fapi } = await renderPreparation(); + const prepare = holdRequests('post', PREPARE_FIRST); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Run action' })); + await waitFor(() => expect(prepare.requests).toHaveLength(1)); + prepare.release(); + await waitFor(() => expect(fapi.verification.firstFactorVerification?.strategy).toBe('email_code')); + await selectMethod(user, 'Continue with your password'); + expect(await screen.findByText(PASSWORD_STEP)).toBeVisible(); + + const duplicate = holdRequests('post', PREPARE_FIRST); + await selectMethod(user, 'Email code to alice@example.com'); + await new Promise(resolve => setTimeout(resolve, 100)); + duplicate.release(); + expect(duplicate.requests).toHaveLength(0); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + const [first] = within(screen.getByRole('group', { name: 'Verification code' })).getAllByRole('textbox'); + if (!first) { + throw new Error('Missing code input'); + } + await user.click(first); + await user.paste(CODE); + await waitFor(() => expect(screen.getByLabelText('Outcome')).toHaveTextContent('resolved')); + }); + + it('keeps a pending email preparation alive across a visit to password', async () => { + await renderPreparation(); + const prepare = holdRequests('post', PREPARE_FIRST); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Run action' })); + await waitFor(() => expect(prepare.requests).toHaveLength(1)); + await selectMethod(user, 'Continue with your password'); + expect(await screen.findByText(PASSWORD_STEP)).toBeVisible(); + await selectMethod(user, 'Email code to alice@example.com'); + await new Promise(resolve => setTimeout(resolve, 100)); + prepare.release(); + expect(prepare.requests).toHaveLength(1); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + }); + + it.each(['sent', 'failed'])( + 'waits for a pending preparation before submitting password when the send is %s', + async result => { + await renderPreparation(); + const prepare = holdRequests('post', PREPARE_FIRST); + const attempt = holdRequests('post', '/v1/client/sessions/sess_1/verify/attempt_first_factor'); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Run action' })); + await waitFor(() => expect(prepare.requests).toHaveLength(1)); + await selectMethod(user, 'Continue with your password'); + await user.type(screen.getByLabelText('Password'), 'hunter2{Enter}'); + await new Promise(resolve => setTimeout(resolve, 100)); + const prematureAttempts = attempt.requests.length; + if (result === 'sent') { + prepare.release(); + } else { + prepare.fail('preparation_failed', 'Old send failed'); + } + attempt.release(); + expect(prematureAttempts).toBe(0); + await waitFor(() => expect(screen.getByLabelText('Outcome')).toHaveTextContent('resolved')); + expect(attempt.requests).toHaveLength(1); + }, + ); + + it.each(['before', 'after'])('drains a cancelled preparation when the next start is released %s it', async order => { + await renderPreparation(vi.fn().mockResolvedValue(reverificationError())); + const oldPrepare = holdRequests('post', PREPARE_FIRST); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Run action' })); + await waitFor(() => expect(oldPrepare.requests).toHaveLength(1)); + await user.click(screen.getByRole('button', { name: 'Dismiss' })); + await waitFor(() => expect(screen.getByLabelText('Outcome')).toHaveTextContent('rejected')); + const start = holdRequests('post', '/v1/client/sessions/sess_1/verify'); + const nextPrepare = holdRequests('post', PREPARE_FIRST); + await user.click(screen.getByRole('button', { name: 'Run action' })); + await new Promise(resolve => setTimeout(resolve, 100)); + const prematureStarts = start.requests.length; + if (order === 'before') { + start.release(); + oldPrepare.release(); + } else { + oldPrepare.release(); + await waitFor(() => expect(start.requests).toHaveLength(1)); + start.release(); + } + nextPrepare.release(); + expect(prematureStarts).toBe(0); + await waitFor(() => expect(nextPrepare.requests).toHaveLength(1)); + expect(start.requests).toHaveLength(1); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + }); + + it.each([10_000, 30_000, 90_000])('preserves the dispatch cooldown when returning after %i ms', async elapsed => { + vi.useFakeTimers({ toFake: ['Date', 'setInterval', 'clearInterval'] }); + try { + const { prepares, user } = await startPrepared(); + await selectMethod(user, 'Continue with your password'); + await act(() => vi.advanceTimersByTimeAsync(elapsed)); + await selectMethod(user, EMAIL_METHOD); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + expect(prepares.requests).toHaveLength(1); + const resend = screen.getByRole('button', { name: /Resend/ }); + if (elapsed < 30_000) { + expect(resend).toBeDisabled(); + expect(resend).toHaveTextContent('Resend (20)'); + } else { + expect(resend).toBeEnabled(); + } + } finally { + vi.useRealTimers(); + } + }); + + it.each([0, 1000])('prepares again when returning %i ms after server expiry', async afterExpiry => { + vi.useFakeTimers({ toFake: ['Date', 'setInterval', 'clearInterval'] }); + try { + const { prepares, user, fapi } = await startPrepared(); + const expiresAt = fapi.verification.firstFactorVerification?.expire_at; + if (typeof expiresAt !== 'number') { + throw new Error('Missing server expiry'); + } + await selectMethod(user, 'Continue with your password'); + vi.setSystemTime(expiresAt + afterExpiry); + await selectMethod(user, EMAIL_METHOD); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + expect(prepares.requests).toHaveLength(2); + expect(screen.getByRole('button', { name: /Resend \(30\)/ })).toBeDisabled(); + await pasteCode(user, CODE); + await waitFor(() => expect(screen.getByLabelText('Outcome')).toHaveTextContent('resolved')); + } finally { + vi.useRealTimers(); + } + }); + + it('replaces the sole challenge on email A to B to A and rejects B code on A', async () => { + const { prepares, user } = await startPrepared({ firstFactors: [emailFactor, workFactor] }); + await selectMethod(user, 'Email code to alice@work.example'); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + await selectMethod(user, EMAIL_METHOD); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + expect(prepares.requests).toHaveLength(3); + expect( + await Promise.all( + prepares.requests.map(async request => new URLSearchParams(await request.text()).get('email_address_id')), + ), + ).toEqual(['idn_alice', 'idn_work', 'idn_alice']); + await pasteCode(user, WORK_CODE); + expect(await screen.findByText('Incorrect code')).toBeVisible(); + await user.keyboard('{Backspace>6/}'); + await pasteCode(user, CODE); + await waitFor(() => expect(screen.getByLabelText('Outcome')).toHaveTextContent('resolved')); + }); + + it.each(['password', 'code'] as const)('prepares again after an unsuccessful %s attempt', async strategy => { + const { prepares, user } = await startPrepared(); + if (strategy === 'password') { + await selectMethod(user, 'Continue with your password'); + await user.type(screen.getByLabelText('Password'), 'wrong{Enter}'); + expect(await screen.findByText(/Password is incorrect/)).toBeVisible(); + } else { + await pasteCode(user, '000000'); + expect(await screen.findByText('Incorrect code')).toBeVisible(); + await selectMethod(user, 'Continue with your password'); + } + await selectMethod(user, EMAIL_METHOD); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + expect(prepares.requests).toHaveLength(2); + await pasteCode(user, CODE); + await waitFor(() => expect(screen.getByLabelText('Outcome')).toHaveTextContent('resolved')); + }); + + it.each(['totp', 'backup_code'] as const)('reuses SMS after a failed %s attempt', async strategy => { + const { fapi } = await renderPreparation(undefined, { secondFactors: [phoneFactor, { strategy }] }); + const prepares = holdRequests('post', PREPARE_SECOND); + prepares.release(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Run action' })); + if (strategy === 'totp') { + expect(await screen.findByText('Enter the code generated by your authenticator app to continue')).toBeVisible(); + await selectMethod(user, PHONE_METHOD); + } + expect(await screen.findByText(PHONE_STEP)).toBeVisible(); + await waitFor(() => expect(fapi.verification.secondFactorVerification?.strategy).toBe('phone_code')); + await selectMethod(user, strategy === 'totp' ? 'Use your authenticator app' : 'Use a backup code'); + if (strategy === 'totp') { + await pasteCode(user, '000000'); + } else { + await user.type(screen.getByRole('textbox'), 'wrong{Enter}'); + } + expect(await screen.findByText('Incorrect code')).toBeVisible(); + await selectMethod(user, PHONE_METHOD); + expect(await screen.findByText(PHONE_STEP)).toBeVisible(); + expect(prepares.requests).toHaveLength(1); + await pasteCode(user, PHONE_CODE); + await waitFor(() => expect(screen.getByLabelText('Outcome')).toHaveTextContent('resolved')); + }); + + it('prepares the same phone separately for the first and second factors', async () => { + const action = vi + .fn() + .mockResolvedValueOnce(reverificationError({ level: 'multi_factor', afterMinutes: 10 })) + .mockResolvedValue({ done: true }); + await renderPreparation(action, { firstFactors: [phoneFactor], secondFactors: [phoneFactor] }); + const first = holdRequests('post', PREPARE_FIRST); + const second = holdRequests('post', PREPARE_SECOND); + first.release(); + second.release(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Run action' })); + expect(await screen.findByText(PHONE_STEP)).toBeVisible(); + await pasteCode(user, PHONE_CODE); + await waitFor(() => expect(second.requests).toHaveLength(1)); + expect(first.requests).toHaveLength(1); + await pasteCode(user, PHONE_CODE); + await waitFor(() => expect(screen.getByLabelText('Outcome')).toHaveTextContent('resolved')); + }); + + it.each([ + { label: 'missing expiry', value: { expire_at: null } }, + { label: 'invalid expiry', value: { expire_at: Number.MAX_VALUE } }, + { label: 'expired verification', value: { status: 'expired' } }, + { label: 'failed verification', value: { status: 'failed' } }, + { label: 'verified verification', value: { status: 'verified' } }, + { label: 'different strategy', value: { strategy: 'phone_code' } }, + ] satisfies Array<{ label: string; value: Partial }>)( + 'does not reuse $label', + async ({ value }) => { + const { fapi, session } = await renderPreparation(); + worker.use( + http.post( + fapiUrl(PREPARE_FIRST), + () => { + const verification = fapiVerification('email_code', value); + fapi.verification.firstFactorVerification = verification; + fapi.verification.firstFactorCode = CODE; + return HttpResponse.json({ + response: fapiSessionVerification(session, { + first_factor_verification: verification, + supported_first_factors: fapi.verification.firstFactors, + }), + client: fapi.client, + }); + }, + { once: true }, + ), + ); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Run action' })); + await waitFor(() => expect(fapi.verification.firstFactorVerification).not.toBeNull()); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + await selectMethod(user, 'Continue with your password'); + const next = holdRequests('post', PREPARE_FIRST); + next.release(); + await selectMethod(user, EMAIL_METHOD); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + expect(next.requests).toHaveLength(1); + await pasteCode(user, CODE); + await waitFor(() => expect(screen.getByLabelText('Outcome')).toHaveTextContent('resolved')); + }, + ); + + it('serializes two successful preparations and accepts only the selected destination code', async () => { + await renderPreparation(undefined, { firstFactors: [emailFactor, workFactor] }); + const first = holdRequests('post', PREPARE_FIRST); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Run action' })); + await waitFor(() => expect(first.requests).toHaveLength(1)); + const second = holdRequests('post', PREPARE_FIRST); + await selectMethod(user, 'Email code to alice@work.example'); + await new Promise(resolve => setTimeout(resolve, 100)); + const prematurePrepares = second.requests.length; + first.release(); + await waitFor(() => expect(second.requests).toHaveLength(1)); + second.release(); + expect(prematurePrepares).toBe(0); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + await pasteCode(user, CODE); + expect(await screen.findByText('Incorrect code')).toBeVisible(); + await user.keyboard('{Backspace>6/}'); + await pasteCode(user, WORK_CODE); + await waitFor(() => expect(screen.getByLabelText('Outcome')).toHaveTextContent('resolved')); + }); + + it('resends after reusing a valid preparation and recovers when resend fails', async () => { + vi.useFakeTimers({ toFake: ['Date', 'setInterval', 'clearInterval'] }); + try { + const { prepares, user } = await startPrepared(); + await selectMethod(user, 'Continue with your password'); + await act(() => vi.advanceTimersByTimeAsync(31_000)); + await selectMethod(user, EMAIL_METHOD); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + expect(prepares.requests).toHaveLength(1); + const resend = holdRequests('post', PREPARE_FIRST); + await user.click(screen.getByRole('button', { name: 'Resend' })); + await vi.waitFor(() => expect(resend.requests).toHaveLength(1)); + expect(screen.getByRole('button', { name: /Resend \(30\)/ })).toBeDisabled(); + resend.fail('preparation_failed', 'Resend failed'); + expect(await screen.findByText('Resend failed')).toBeVisible(); + expect(screen.getByRole('button', { name: 'Resend' })).toBeEnabled(); + const retry = holdRequests('post', PREPARE_FIRST); + retry.release(); + await user.click(screen.getByRole('button', { name: 'Resend' })); + await vi.waitFor(() => expect(retry.requests).toHaveLength(1)); + expect(screen.queryByText('Resend failed')).toBeNull(); + expect(screen.getByRole('button', { name: /Resend \(30\)/ })).toBeDisabled(); + await pasteCode(user, CODE); + await waitFor(() => expect(screen.getByLabelText('Outcome')).toHaveTextContent('resolved')); + } finally { + vi.useRealTimers(); + } + }); + + it('allows the currently sent code when expiry metadata is absent', async () => { + const { fapi, session } = await renderPreparation(); + worker.use( + http.post( + fapiUrl(PREPARE_FIRST), + () => { + const verification = fapiVerification('email_code', { expire_at: null }); + fapi.verification.firstFactorVerification = verification; + fapi.verification.firstFactorCode = CODE; + return HttpResponse.json({ + response: fapiSessionVerification(session, { + first_factor_verification: verification, + supported_first_factors: fapi.verification.firstFactors, + }), + client: fapi.client, + }); + }, + { once: true }, + ), + ); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Run action' })); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + await pasteCode(user, CODE); + await waitFor(() => expect(screen.getByLabelText('Outcome')).toHaveTextContent('resolved')); + }); + + it('clears preparation when the host unmounts and mounts again', async () => { + const { rerender, prepares, user, action } = await startPrepared(); + action.mockResolvedValueOnce(reverificationError()); + rerender( + , + ); + await user.click(screen.getByRole('button', { name: 'Run action' })); + await waitFor(() => expect(prepares.requests).toHaveLength(2)); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + await pasteCode(user, CODE); + await waitFor(() => expect(screen.getByLabelText('Outcome')).toHaveTextContent('resolved')); + }); + + it('does not start or prepare for a different session while the original preparation drains', async () => { + const { fapi, clerk } = await renderPreparation(); + const secondSession = fapiSession({ id: 'sess_2', user: fapiUser({ id: 'user_2' }) }); + fapi.client.sessions.push(secondSession); + const first = holdRequests('post', PREPARE_FIRST); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Run action' })); + await waitFor(() => expect(first.requests).toHaveLength(1)); + const otherStart = holdRequests('post', '/v1/client/sessions/sess_2/verify'); + otherStart.release(); + await act(() => clerk.setActive({ session: 'sess_2' })); + await waitFor(() => expect(screen.getByLabelText('Outcome')).toHaveTextContent('rejected')); + first.release(); + await new Promise(resolve => setTimeout(resolve, 100)); + expect(otherStart.requests).toHaveLength(0); + expect(first.requests).toHaveLength(1); + expect(clerk.session?.id).toBe('sess_2'); + expect(screen.queryByRole('button', { name: 'Dismiss' })).toBeNull(); + }); + + it('prepares again after the passkey prompt is aborted', async () => { + const { clerk } = await renderPreparation(undefined, { firstFactors: [emailFactor, { strategy: 'passkey' }] }); + const credential = vi.fn().mockResolvedValue({ + publicKeyCredential: null, + error: new ClerkWebAuthnError('Aborted', { code: 'passkey_operation_aborted' }), + }); + clerk.__internal_getPublicCredentials = credential; + Object.defineProperty(navigator, 'webdriver', { value: false, configurable: true }); + try { + const prepares = holdRequests('post', PREPARE_FIRST); + prepares.release(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Run action' })); + expect(await screen.findByText(/Using your passkey confirms/)).toBeVisible(); + await selectMethod(user, EMAIL_METHOD); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + await selectMethod(user, 'Use your passkey'); + await user.click(screen.getByRole('button', { name: 'Continue' })); + await waitFor(() => expect(credential).toHaveBeenCalledOnce()); + await waitFor(() => expect(screen.getByRole('button', { name: 'Use another method' })).toBeEnabled()); + await selectMethod(user, EMAIL_METHOD); + expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); + expect( + await Promise.all( + prepares.requests.map(async request => new URLSearchParams(await request.text()).get('strategy')), + ), + ).toEqual(['email_code', 'passkey', 'email_code']); + await pasteCode(user, CODE); + await waitFor(() => expect(screen.getByLabelText('Outcome')).toHaveTextContent('resolved')); + } finally { + Reflect.deleteProperty(navigator, 'webdriver'); + } + }); +}); diff --git a/packages/mosaic/src/features/reverification/__tests__/reverification.controller.test.ts b/packages/mosaic/src/features/reverification/__tests__/reverification.controller.test.ts new file mode 100644 index 00000000000..c1458a72c6f --- /dev/null +++ b/packages/mosaic/src/features/reverification/__tests__/reverification.controller.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { deferred, tick } from '../../../__tests__/async'; +import { createActor } from '../../../machine/create-actor'; +import { type ReverificationDeps, reverificationMachine } from '../reverification.controller'; +import type { ReverificationPreparation } from '../reverification.model'; +import type { ReverificationPreparableMethod } from '../reverification.types'; + +const email: ReverificationPreparableMethod = { + id: 'email_code:alice', + stage: 'first', + strategy: 'email_code', + emailAddressId: 'alice', + identifier: 'alice@example.com', +}; +const work: ReverificationPreparableMethod = { + id: 'email_code:work', + stage: 'first', + strategy: 'email_code', + emailAddressId: 'work', + identifier: 'alice@work.example', +}; + +function deps(): ReverificationDeps { + return { + start: vi.fn(async () => ({ status: 'needs_first_factor', methods: [email, work], startingMethod: email })), + prepare: vi.fn(async () => ({ status: 'unverified', strategy: 'email_code', expiresAt: Date.now() + 60_000 })), + attempt: vi.fn(async () => ({ status: 'complete', methods: [], startingMethod: null })), + finish: vi.fn(async () => {}), + }; +} + +describe('reverification operation ordering', () => { + it.each(['unavailable', 'failed', 'retrying'] as const)('starts a new active scope directly from %s', async value => { + const effects = deps(); + const actor = createActor(reverificationMachine, { + snapshot: { value, context: { deps: effects, scope: { runId: Symbol(), sessionId: 'session' } } }, + }).start(); + actor.setContext({ requestedScope: { runId: Symbol(), sessionId: 'session' } }); + actor.send({ type: 'START' }); + await tick(); + expect(effects.start).toHaveBeenCalledOnce(); + actor.stop(); + }); + + it('coalesces A to B to A selections while A is pending', async () => { + const effects = deps(); + const pending = deferred(); + effects.prepare = vi.fn(() => pending.promise); + const actor = createActor(reverificationMachine, { + context: { deps: effects, requestedScope: { runId: Symbol(), sessionId: 'session' } }, + }).start(); + actor.send({ type: 'START' }); + await tick(); + actor.send({ type: 'SHOW_METHODS' }); + actor.send({ type: 'SELECT_METHOD', id: work.id }); + actor.send({ type: 'SELECT_METHOD', id: email.id }); + pending.resolve({ status: 'unverified', strategy: 'email_code', expiresAt: Date.now() + 60_000 }); + await tick(); + expect(effects.prepare).toHaveBeenCalledExactlyOnceWith(email); + actor.send({ type: 'TYPE', value: '123456' }); + actor.send({ type: 'SUBMIT' }); + await tick(); + expect(effects.attempt).toHaveBeenCalledExactlyOnceWith(email, '123456'); + actor.stop(); + }); +}); diff --git a/packages/mosaic/src/features/reverification/reverification.controller.ts b/packages/mosaic/src/features/reverification/reverification.controller.ts index 5706c5392fb..5ed1d57592d 100644 --- a/packages/mosaic/src/features/reverification/reverification.controller.ts +++ b/packages/mosaic/src/features/reverification/reverification.controller.ts @@ -8,10 +8,16 @@ import { setup } from '../../machine/setup'; import type { DoneInvokeEvent, ErrorInvokeEvent, StateConfig } from '../../machine/types'; import { useMachine } from '../../machine/use-machine'; import { toLocalizableError } from '../../utils/errors'; -import type { ReverificationActiveModel, ReverificationModel } from './reverification.model'; +import type { + ReverificationActiveModel, + ReverificationModel, + ReverificationPreparation, + ReverificationScope, +} from './reverification.model'; import type { ReverificationErrorReason, ReverificationMethod, + ReverificationPreparableMethod, ReverificationResult, ReverificationStep, ReverificationViewProps, @@ -38,7 +44,22 @@ type Overlay = Extract; export type ReverificationDeps = Pick; +type RetainedPreparation = { + method: ReverificationPreparableMethod; + expiresAt: number; + resendAvailableAt: number; +}; + interface ReverificationContext { + scope: ReverificationScope | null; + requestedScope: ReverificationScope | null; + operationScope: ReverificationScope | null; + retainedPreparation: RetainedPreparation | null; + preparation: { + method: ReverificationPreparableMethod; + resendAvailableAt: number; + presentation: 'factor' | 'picker'; + } | null; inputValue: string; error: ErrorDescription | undefined; direction: FlowDirection; @@ -78,10 +99,6 @@ const unseatedDeps: ReverificationDeps = { export const RESEND_COOLDOWN_MS = 30_000; -function lockResend(): Pick { - return { resendAvailableAt: Date.now() + RESEND_COOLDOWN_MS }; -} - function unlockResend(): Pick { return { resendAvailableAt: undefined }; } @@ -101,8 +118,8 @@ function selectMethod(ctx: ReverificationContext, id: string): Partial { @@ -113,7 +130,108 @@ function submit(ctx: ReverificationContext): Promise { return ctx.deps.attempt(method, ctx.inputValue); } +function sameMethod(a: ReverificationMethod | null, b: ReverificationMethod | null): boolean { + return Boolean(a && b && a.stage === b.stage && a.strategy === b.strategy && a.id === b.id); +} + +function beginPreparation(ctx: ReverificationContext): Partial { + const method = ctx.activeMethod; + if (!method || !needsPrepare(method)) { + return {}; + } + const resendAvailableAt = Date.now() + RESEND_COOLDOWN_MS; + return { + retainedPreparation: null, + resendAvailableAt, + preparation: { method, resendAvailableAt, presentation: topOverlay(ctx) === 'method-picker' ? 'picker' : 'factor' }, + }; +} + +function sameScope(a: ReverificationScope | null, b: ReverificationScope | null): boolean { + return a?.runId === b?.runId && a?.sessionId === b?.sessionId; +} + +function settledRun(ctx: ReverificationContext) { + if (!ctx.scope) { + return 'inactive' as const; + } + if (!sameScope(ctx.scope, ctx.operationScope)) { + return 'starting' as const; + } + return undefined; +} + +function clearRun(): Partial { + return { + scope: null, + retainedPreparation: null, + activeMethod: null, + methods: [], + inputValue: '', + error: undefined, + submitRequested: false, + overlay: [], + frozenActiveMethodId: undefined, + ...unlockResend(), + }; +} + +const bindRun = assign(ctx => ({ ...clearRun(), scope: ctx.requestedScope })); +const drainingEvents = { + RESET: { actions: assign(clearRun) }, + START: { actions: bindRun }, +}; +const afterDrain = [ + { guard: (ctx: ReverificationContext) => settledRun(ctx) === 'inactive', target: 'inactive' as const }, + { guard: (ctx: ReverificationContext) => settledRun(ctx) === 'starting', target: 'starting' as const }, +]; + +function settlePreparation( + ctx: ReverificationContext, + result: { status: 'sent'; verification: ReverificationPreparation } | { status: 'failed'; error: unknown }, +) { + const drained = settledRun(ctx); + if (drained) { + return { target: drained }; + } + const preparation = ctx.preparation; + const method = ctx.activeMethod; + if (!preparation || !method) { + return { target: 'inactive' as const }; + } + if (needsPrepare(method) && !sameMethod(method, preparation.method)) { + return { target: 'preparing' as const }; + } + const matches = sameMethod(method, preparation.method); + const verification = result.status === 'sent' ? result.verification : null; + const retainedPreparation = + verification?.status === 'unverified' && + verification.strategy === preparation.method.strategy && + verification.expiresAt !== null && + Number.isFinite(verification.expiresAt) && + verification.expiresAt > Date.now() + ? { + method: preparation.method, + expiresAt: verification.expiresAt, + resendAvailableAt: preparation.resendAvailableAt, + } + : null; + return { + target: + ctx.submitRequested && (result.status === 'sent' || !matches) ? ('submitting' as const) : ('verifying' as const), + context: { + retainedPreparation, + preparation: null, + resendAvailableAt: result.status === 'sent' ? preparation.resendAvailableAt : undefined, + overlay: preparation.presentation === 'picker' ? [] : ctx.overlay, + error: matches && result.status === 'failed' ? toLocalizableError(result.error) : ctx.error, + submitRequested: false, + }, + }; +} + const applyResult = assign>((_, event) => ({ + retainedPreparation: null, methods: event.output.methods, activeMethod: event.output.startingMethod, inputValue: '', @@ -122,6 +240,7 @@ const applyResult = assign>((_, event) => direction: 1, })); const afterResult = [ + ...afterDrain, { guard: (_: ReverificationContext, event: DoneInvokeEvent) => event.output.status === 'complete', @@ -140,7 +259,7 @@ const afterResult = [ return Boolean(method && needsPrepare(method)); }, target: 'preparing' as const, - actions: [applyResult, assign(() => lockResend())], + actions: applyResult, }, { target: 'verifying' as const, actions: applyResult }, ]; @@ -175,28 +294,39 @@ const factorEvents = { guard: ctx => ctx.overlay.length > 0, actions: assign(ctx => ({ overlay: ctx.overlay.slice(0, -1), direction: -1 as const })), }, - SELECT_METHOD: [ - { - target: 'methodPickerPreparing', - guard: (ctx, event) => { - const method = ctx.methods.find(candidate => candidate.id === event.id); - return topOverlay(ctx) === 'method-picker' && Boolean(method && needsPrepare(method)); + SELECT_METHOD: ({ context: ctx, event }) => { + if (topOverlay(ctx) !== 'method-picker') { + return; + } + const method = ctx.methods.find(candidate => candidate.id === event.id); + if (!method) { + return; + } + const retained = ctx.retainedPreparation; + const reusable = retained && sameMethod(method, retained.method) && retained.expiresAt > Date.now(); + const prepare = needsPrepare(method) && !reusable; + return { + target: prepare ? 'preparing' : 'verifying', + context: { + ...selectMethod(ctx, event.id), + overlay: prepare ? ctx.overlay : [], + resendAvailableAt: reusable ? retained.resendAvailableAt : ctx.resendAvailableAt, }, - actions: assign((ctx, event) => selectMethod(ctx, event.id)), - }, - { - target: 'verifying', - guard: ctx => topOverlay(ctx) === 'method-picker', - actions: assign((ctx, event) => ({ ...selectMethod(ctx, event.id), overlay: [] })), - }, - ], + }; + }, RESET: 'inactive', + START: { target: 'starting', actions: bindRun }, } satisfies NonNullable['on']>; export const reverificationMachine = createMachine({ id: 'reverification', initial: 'inactive', context: { + scope: null, + requestedScope: null, + operationScope: null, + retainedPreparation: null, + preparation: null, inputValue: '', error: undefined, direction: 1, @@ -211,73 +341,45 @@ export const reverificationMachine = createMachine({ }, states: { inactive: { - entry: assign(() => ({ - inputValue: '', - error: undefined, - submitRequested: false, - overlay: [], - ...unlockResend(), - })), - on: { START: 'starting' }, + entry: assign(() => ({ ...clearRun(), preparation: null, operationScope: null })), + on: { START: { target: 'starting', actions: bindRun } }, }, starting: { - on: { RESET: 'inactive' }, + entry: assign(ctx => ({ operationScope: ctx.scope })), + on: drainingEvents, invoke: fromPromise(ctx => ctx.deps.start(), { onDone: afterResult, - onError: 'failed', + onError: [...afterDrain, { target: 'failed' }], }), }, - /* - Preparing a factor behind the scenes (e.g. sending an OTP) - - This shows the factor card with optimistic UI, so it needs to handle the same actions - as verifying, but if the user submits, we queue that up until after prepare resolves - */ preparing: { + entry: assign(ctx => ({ ...beginPreparation(ctx), operationScope: ctx.scope })), on: { ...factorEvents, + ...drainingEvents, + SELECT_METHOD: { + guard: (ctx, event) => + topOverlay(ctx) === 'method-picker' && ctx.methods.some(method => method.id === event.id), + actions: assign((ctx, event) => { + const selection = selectMethod(ctx, event.id); + const sending = selection.activeMethod && needsPrepare(selection.activeMethod); + return { + ...selection, + overlay: sending ? ctx.overlay : [], + preparation: ctx.preparation ? { ...ctx.preparation, presentation: sending ? 'picker' : 'factor' } : null, + }; + }), + }, SUBMIT: { guard: ctx => ctx.overlay.length === 0, actions: assign(() => ({ submitRequested: true })), }, }, invoke: fromPromise(prepareActive, { - onDone: [ - { - guard: (ctx: ReverificationContext) => ctx.submitRequested, - target: 'submitting' as const, - actions: assign(() => ({ submitRequested: false })), - }, - { target: 'verifying' as const }, - ], - onError: { - target: 'verifying', - actions: assign((_, event) => ({ - error: toLocalizableError(event.error), - submitRequested: false, - ...unlockResend(), - })), - }, - }), - }, - - /* - When selecting a method from the picker, we prepare before going to the factor card - - We don't use optimistic UI here simply because we're in a better position to show - some feedback that does not feel janky. The view handles disabling inputs while preparing. - */ - methodPickerPreparing: { - entry: assign(() => lockResend()), - on: { RESET: 'inactive' }, - invoke: fromPromise(prepareActive, { - onDone: { target: 'verifying', actions: assign(() => ({ overlay: [] })) }, - onError: { - target: 'verifying', - actions: assign((_, event) => ({ error: toLocalizableError(event.error), overlay: [], ...unlockResend() })), - }, + onDone: ({ context, event }) => settlePreparation(context, { status: 'sent', verification: event.output }), + onError: ({ context, event }) => settlePreparation(context, { status: 'failed', error: event.error }), }), }, @@ -293,7 +395,6 @@ export const reverificationMachine = createMachine({ Boolean(ctx.activeMethod && needsPrepare(ctx.activeMethod)) && (ctx.resendAvailableAt === undefined || Date.now() >= ctx.resendAvailableAt), actions: assign(() => ({ - ...lockResend(), inputValue: '', error: undefined, submitRequested: false, @@ -303,10 +404,18 @@ export const reverificationMachine = createMachine({ }, submitting: { - on: { RESET: 'inactive' }, + entry: assign(ctx => ({ + operationScope: ctx.scope, + retainedPreparation: + ctx.activeMethod?.strategy === 'totp' || ctx.activeMethod?.strategy === 'backup_code' + ? ctx.retainedPreparation + : null, + })), + on: drainingEvents, invoke: fromPromise(submit, { onDone: afterResult, onError: [ + ...afterDrain, { guard: (_: ReverificationContext, event: ErrorInvokeEvent) => isPasskeyAbortedError(event.error), target: 'verifying', @@ -321,25 +430,26 @@ export const reverificationMachine = createMachine({ }, unavailable: { - on: { RESET: 'inactive' }, + on: { RESET: 'inactive', START: { target: 'starting', actions: bindRun } }, }, failed: { - on: { RESET: 'inactive' }, + on: { RESET: 'inactive', START: { target: 'starting', actions: bindRun } }, }, finishing: { - on: { RESET: 'inactive' }, + entry: assign(ctx => ({ retainedPreparation: null, operationScope: ctx.scope })), + on: drainingEvents, invoke: fromPromise(ctx => ctx.deps.finish(), { - onDone: 'retrying', - onError: 'failed', + onDone: [...afterDrain, { target: 'retrying' }], + onError: [...afterDrain, { target: 'failed' }], }), }, // This means we are retrying the action after successful reverification // This state is left by the model returning to inactive once the retry has completed retrying: { - on: { RESET: 'inactive' }, + on: { RESET: 'inactive', START: { target: 'starting', actions: bindRun } }, }, }, }); @@ -420,6 +530,7 @@ function useReverificationState(model: ReverificationModel): ReverificationContr active ? { context: { + requestedScope: active.scope, supportEmail: active.supportEmail, deps: active, }, @@ -445,8 +556,8 @@ function useReverificationState(model: ReverificationModel): ReverificationContr ? model.cancel : undefined; const onCancel = cancelModel; - const needsStart = active !== null && snapshot.value === 'inactive'; - const needsReset = model.status === 'inactive' && snapshot.value !== 'inactive'; + const needsStart = active !== null && !sameScope(active.scope, snapshot.context.scope); + const needsReset = model.status === 'inactive' && snapshot.context.scope !== null; /* eslint-disable react-you-might-not-need-an-effect/no-event-handler -- Syncs the machine with the model status, which changes outside this component. */ useEffect(() => { if (needsStart) { @@ -484,9 +595,13 @@ function useReverificationState(model: ReverificationModel): ReverificationContr // If we are currently on the alternative methods screen and preparing a factor, activeMethod will // have transitioned to the factor we are now preparing, so the one we want to hide is the old one - const excludeId = state === 'methodPickerPreparing' ? context.frozenActiveMethodId : activeMethod?.id; + const excludeId = + state === 'preparing' && context.preparation?.presentation === 'picker' + ? context.frozenActiveMethodId + : activeMethod?.id; const methods = context.methods.filter(method => method.id !== excludeId); - const pendingMethodId = state === 'methodPickerPreparing' ? activeMethod?.id : undefined; + const pendingMethodId = + state === 'preparing' && context.preparation?.presentation === 'picker' ? activeMethod?.id : undefined; const view: ReverificationViewProps = { step, diff --git a/packages/mosaic/src/features/reverification/reverification.model.tsx b/packages/mosaic/src/features/reverification/reverification.model.tsx index 6d15d121b76..181469a4098 100644 --- a/packages/mosaic/src/features/reverification/reverification.model.tsx +++ b/packages/mosaic/src/features/reverification/reverification.model.tsx @@ -4,6 +4,7 @@ import type { SessionVerificationFirstFactor, SessionVerificationResource, SessionVerificationSecondFactor, + VerificationResource, } from '@clerk/shared/types'; import { isWebAuthnSupported } from '@clerk/shared/webauthn'; @@ -18,11 +19,18 @@ import type { } from './reverification.types'; import { pickStartingMethod } from './reverification.utils'; +export type ReverificationPreparation = Pick & { + expiresAt: number | null; +}; + +export type ReverificationScope = { runId: symbol; sessionId: string }; + export type ReverificationActiveModel = { + scope: ReverificationScope; status: 'active'; supportEmail: string; start: () => Promise; - prepare: (method: ReverificationPreparableMethod) => Promise; + prepare: (method: ReverificationPreparableMethod) => Promise; attempt: (method: ReverificationMethod, value: string) => Promise; finish: () => Promise; cancel: () => void; @@ -124,7 +132,12 @@ export function useReverificationModel(reverificationState: ReverificationState) const { level, cancel, complete } = reverificationState; - if (!session || !environment || supportEmail === undefined) { + if ( + !session || + !environment || + supportEmail === undefined || + (reverificationState.sessionId !== null && session.id !== reverificationState.sessionId) + ) { return { status: 'loading', cancel }; } @@ -136,31 +149,33 @@ export function useReverificationModel(reverificationState: ReverificationState) return { status: 'active', + scope: { runId: reverificationState.runId, sessionId: session.id }, supportEmail, start: async () => handleResponse(await session.startVerification({ level: level ?? 'second_factor' })), cancel, prepare: async method => { - switch (method.strategy) { - case 'email_code': - await session.prepareFirstFactorVerification({ - strategy: 'email_code', - emailAddressId: method.emailAddressId, - }); - return; - case 'phone_code': - if (method.stage === 'second') { - await session.prepareSecondFactorVerification({ - strategy: 'phone_code', - phoneNumberId: method.phoneNumberId, - }); - return; - } - await session.prepareFirstFactorVerification({ - strategy: 'phone_code', - phoneNumberId: method.phoneNumberId, - }); - return; - } + const resource = + method.strategy === 'email_code' + ? await session.prepareFirstFactorVerification({ + strategy: 'email_code', + emailAddressId: method.emailAddressId, + }) + : method.stage === 'second' + ? await session.prepareSecondFactorVerification({ + strategy: 'phone_code', + phoneNumberId: method.phoneNumberId, + }) + : await session.prepareFirstFactorVerification({ + strategy: 'phone_code', + phoneNumberId: method.phoneNumberId, + }); + const verification = + method.stage === 'first' ? resource.firstFactorVerification : resource.secondFactorVerification; + return { + status: verification.status, + strategy: verification.strategy, + expiresAt: verification.expireAt?.getTime() ?? null, + }; }, attempt: async (method, value) => { switch (method.strategy) { diff --git a/packages/mosaic/src/features/reverification/reverification.types.ts b/packages/mosaic/src/features/reverification/reverification.types.ts index ada8be5fc0d..618b780f2bd 100644 --- a/packages/mosaic/src/features/reverification/reverification.types.ts +++ b/packages/mosaic/src/features/reverification/reverification.types.ts @@ -46,7 +46,14 @@ export type ReverificationViewProps = ReverificationViewFields & { export type ReverificationState = | { phase: 'inactive' } - | { phase: 'active'; complete: () => void; cancel: () => void; level: SessionVerificationLevel | undefined } + | { + phase: 'active'; + runId: symbol; + sessionId: string | null; + complete: () => void; + cancel: () => void; + level: SessionVerificationLevel | undefined; + } | { phase: 'retrying' }; export type ReverificationResult = { diff --git a/packages/mosaic/src/features/reverification/use-reverification-with-state.ts b/packages/mosaic/src/features/reverification/use-reverification-with-state.ts index 156c8602a96..c5e2c7eb76a 100644 --- a/packages/mosaic/src/features/reverification/use-reverification-with-state.ts +++ b/packages/mosaic/src/features/reverification/use-reverification-with-state.ts @@ -16,6 +16,7 @@ type RuntimeOperation = | { status: 'requesting'; promise: Promise } | { status: 'active'; + runId: symbol; promise: Promise; sessionId: string | null; complete: () => void; @@ -71,9 +72,9 @@ export function useReverificationWithState( runtimeRef.current.sessionId = session?.id ?? null; }); - const completeChallenge = useCallback(() => { + const completeChallenge = useCallback((runId: symbol) => { const operation = runtimeRef.current.operation; - if (operation.status !== 'active') { + if (operation.status !== 'active' || operation.runId !== runId) { return; } runtimeRef.current.operation = { status: 'retrying', promise: operation.promise }; @@ -100,8 +101,10 @@ export function useReverificationWithState( return; } + const runId = Symbol(); runtimeRef.current.operation = { status: 'active', + runId, promise: operation.promise, sessionId: runtimeRef.current.sessionId, complete, @@ -110,8 +113,10 @@ export function useReverificationWithState( setReverificationState({ phase: 'active', + runId, + sessionId: runtimeRef.current.sessionId, level, - complete: completeChallenge, + complete: () => completeChallenge(runId), cancel: cancelChallenge, }); }, From 26b42313f8bf77298570dd93f10bb941ef7eaab5 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Sat, 31 Oct 2026 12:58:17 -0600 Subject: [PATCH 3/6] test(mosaic): share reverification feature fixtures --- ...everification-preparation.feature.test.tsx | 85 ++++----------- .../__tests__/reverification.feature.test.tsx | 101 ++++-------------- .../__tests__/reverification.fixtures.tsx | 94 ++++++++++++++++ 3 files changed, 134 insertions(+), 146 deletions(-) create mode 100644 packages/mosaic/src/features/reverification/__tests__/reverification.fixtures.tsx diff --git a/packages/mosaic/src/features/reverification/__tests__/reverification-preparation.feature.test.tsx b/packages/mosaic/src/features/reverification/__tests__/reverification-preparation.feature.test.tsx index df9f129c3b0..6218a8ef47d 100644 --- a/packages/mosaic/src/features/reverification/__tests__/reverification-preparation.feature.test.tsx +++ b/packages/mosaic/src/features/reverification/__tests__/reverification-preparation.feature.test.tsx @@ -4,14 +4,12 @@ import type { VerificationJSON } from '@clerk/shared/types'; import { act, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { http, HttpResponse } from 'msw'; -import { useState } from 'react'; import { describe, expect, it, vi } from 'vitest'; import { fapiUrl, holdRequests, serveFapi, worker } from '../../../__tests__/feature/fake-fapi'; import type { FakeVerificationSeed } from '../../../__tests__/feature/fake-fapi/verification'; import { fapiClient, - fapiEmailAddress, fapiEnvironment, fapiSession, fapiSessionVerification, @@ -19,83 +17,44 @@ import { fapiVerification, } from '../../../__tests__/feature/fapi'; import { renderWithClerk } from '../../../__tests__/feature/render'; -import { Card } from '../../../components/card'; -import { Reverification, useReverificationFlow } from '../reverification'; +import { + aliceSession, + CODE, + emailFactor, + HoldingHost, + passwordFactor, + phoneFactor, + PREPARE_FIRST, + PREPARE_SECOND, + SECRETS, + STEP, + workEmailFactor, +} from './reverification.fixtures'; -const PREPARE_FIRST = '/v1/client/sessions/sess_1/verify/prepare_first_factor'; -const EMAIL_STEP = 'Enter the code sent to your email to continue'; -const PASSWORD_STEP = 'Enter your current password to continue'; -const CODE = '123456'; +const EMAIL_STEP = STEP.emailCode; +const PASSWORD_STEP = STEP.password; const PHONE_CODE = '234567'; const WORK_CODE = '654321'; const EMAIL_METHOD = 'Email code to alice@example.com'; const PHONE_METHOD = 'Send SMS code to +1 555 0100'; -const PHONE_STEP = 'Enter the code sent to your phone to continue'; -const PREPARE_SECOND = '/v1/client/sessions/sess_1/verify/prepare_second_factor'; -const emailFactor = { - strategy: 'email_code', - email_address_id: 'idn_alice', - safe_identifier: 'alice@example.com', -} as const; -const passwordFactor = { strategy: 'password' } as const; -const phoneFactor = { strategy: 'phone_code', phone_number_id: 'idn_phone', safe_identifier: '+1 555 0100' } as const; -const workFactor = { - strategy: 'email_code', - email_address_id: 'idn_work', - safe_identifier: 'alice@work.example', -} as const; - -function Host({ action }: { action: () => Promise }) { - const [run, controller] = useReverificationFlow(action); - const [outcome, setOutcome] = useState(''); - return ( - <> - - {outcome} - - - {controller.onCancel ? ( - - ) : null} - - - ); -} +const PHONE_STEP = STEP.phoneCode; async function renderPreparation( action = vi.fn().mockResolvedValueOnce(reverificationError()).mockResolvedValue({ done: true }), verification: FakeVerificationSeed = {}, ) { - const alice = fapiUser({ - email_addresses: [fapiEmailAddress({ id: 'idn_alice', email_address: 'alice@example.com' })], - }); - const session = fapiSession({ id: 'sess_1', user: alice }); + const session = aliceSession; const fapi = serveFapi({ client: fapiClient([session]), environment: fapiEnvironment({ display_config: { preferred_sign_in_strategy: 'otp' } }), verification: { firstFactors: [emailFactor, passwordFactor], - secrets: { email_code: CODE, phone_code: PHONE_CODE, password: 'hunter2', totp: CODE, backup_code: 'backup-1' }, + secrets: { ...SECRETS, phone_code: PHONE_CODE }, codesByIdentification: { idn_alice: CODE, idn_work: WORK_CODE, idn_phone: PHONE_CODE }, ...verification, }, }); - const view = await renderWithClerk(); + const view = await renderWithClerk(); return { ...view, fapi, action, session }; } @@ -261,7 +220,7 @@ describe('Reverification preparation', () => { }); it('replaces the sole challenge on email A to B to A and rejects B code on A', async () => { - const { prepares, user } = await startPrepared({ firstFactors: [emailFactor, workFactor] }); + const { prepares, user } = await startPrepared({ firstFactors: [emailFactor, workEmailFactor] }); await selectMethod(user, 'Email code to alice@work.example'); expect(await screen.findByText(EMAIL_STEP)).toBeVisible(); await selectMethod(user, EMAIL_METHOD); @@ -388,7 +347,7 @@ describe('Reverification preparation', () => { ); it('serializes two successful preparations and accepts only the selected destination code', async () => { - await renderPreparation(undefined, { firstFactors: [emailFactor, workFactor] }); + await renderPreparation(undefined, { firstFactors: [emailFactor, workEmailFactor] }); const first = holdRequests('post', PREPARE_FIRST); const user = userEvent.setup(); await user.click(screen.getByRole('button', { name: 'Run action' })); @@ -469,7 +428,7 @@ describe('Reverification preparation', () => { const { rerender, prepares, user, action } = await startPrepared(); action.mockResolvedValueOnce(reverificationError()); rerender( - , diff --git a/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx b/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx index bf73851f1ae..837dafe9292 100644 --- a/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx +++ b/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx @@ -21,15 +21,25 @@ import { Card } from '../../../components/card'; import type { MosaicLocalization } from '../../../localization'; import { MosaicProvider } from '../../../mosaic-provider'; import { Reverification, useReverificationFlow } from '../reverification'; +import { + aliceSession, + ATTEMPT_FIRST, + ATTEMPT_SECOND, + BACKUP_CODE, + CODE, + emailFactor, + errorCode, + HoldingHost, + PASSWORD, + passwordFactor, + phoneFactor, + PREPARE_FIRST, + SECRETS, + STEP, + VERIFY, + workEmailFactor, +} from './reverification.fixtures'; -const alice = fapiUser({ - id: 'user_1', - first_name: 'Alice', - last_name: 'Smith', - email_addresses: [fapiEmailAddress({ id: 'idn_alice', email_address: 'alice@example.com' })], -}); - -const aliceSession = fapiSession({ id: 'sess_1', user: alice }); const bobSession = fapiSession({ id: 'sess_2', user: fapiUser({ @@ -39,35 +49,9 @@ const bobSession = fapiSession({ }), }); -const VERIFY = '/v1/client/sessions/sess_1/verify'; -const PREPARE_FIRST = `${VERIFY}/prepare_first_factor`; -const ATTEMPT_FIRST = `${VERIFY}/attempt_first_factor`; -const ATTEMPT_SECOND = `${VERIFY}/attempt_second_factor`; const TOUCH = '/v1/client/sessions/sess_1/touch'; -const PASSWORD = 'hunter2'; -const CODE = '123456'; const WRONG_CODE = '000000'; -const BACKUP_CODE = 'backup-1'; - -const SECRETS = { password: PASSWORD, email_code: CODE, phone_code: CODE, totp: CODE, backup_code: BACKUP_CODE }; - -const passwordFactor: SignInFirstFactorJSON = { strategy: 'password' }; -const emailFactor: SignInFirstFactorJSON = { - strategy: 'email_code', - email_address_id: 'idn_alice', - safe_identifier: 'alice@example.com', -}; -const workEmailFactor: SignInFirstFactorJSON = { - strategy: 'email_code', - email_address_id: 'idn_work', - safe_identifier: 'alice@work.example', -}; -const phoneFactor: SignInFirstFactorJSON = { - strategy: 'phone_code', - phone_number_id: 'idn_phone', - safe_identifier: '+1 555 0100', -}; const passkeyFactor: SignInFirstFactorJSON = { strategy: 'passkey' }; const enterpriseSsoFactor = { strategy: 'enterprise_sso', @@ -86,14 +70,6 @@ const secondPhoneFactor: SignInSecondFactorJSON = { const TITLE = 'Verification required'; const NO_FACTORS_TITLE = 'Cannot verify your account'; const GENERIC_ERROR_TITLE = 'Something went wrong'; -const STEP = { - password: 'Enter your current password to continue', - emailCode: 'Enter the code sent to your email to continue', - phoneCode: 'Enter the code sent to your phone to continue', - totp: 'Enter the code generated by your authenticator app to continue', - backupCode: 'Enter the backup code you received when setting up two-step authentication', - passkey: /Using your passkey confirms your identity/, -}; function signedIn(overrides: FakeFapiSeed = {}): FakeFapiSeed { return { client: fapiClient([aliceSession]), ...overrides }; @@ -111,13 +87,6 @@ function guardedAction(config?: ReverificationConfig) { return vi.fn().mockResolvedValueOnce(reverificationError(config)).mockResolvedValue({ done: true }); } -function errorCode(error: unknown) { - if (error instanceof Error) { - return 'code' in error && typeof error.code === 'string' ? error.code : error.message; - } - return 'unknown'; -} - function Host({ action, onOutcome }: { action: () => Promise; onOutcome?: (outcome: string) => void }) { const [run, reverification] = useReverificationFlow(action); const [outcome, setOutcome] = useState(''); @@ -171,40 +140,6 @@ function Host({ action, onOutcome }: { action: () => Promise; onOutcome ); } -function HoldingHost({ action }: { action: () => Promise }) { - const [run, reverification] = useReverificationFlow(action); - const [outcome, setOutcome] = useState(''); - - return ( - <> - - {outcome} - {String(reverification.visible)} - - - {reverification.onCancel ? ( - - ) : null} - - - ); -} - function renderReverification( action = guardedAction(), seed: FakeFapiSeed = signedIn(), diff --git a/packages/mosaic/src/features/reverification/__tests__/reverification.fixtures.tsx b/packages/mosaic/src/features/reverification/__tests__/reverification.fixtures.tsx new file mode 100644 index 00000000000..f9cece7b7eb --- /dev/null +++ b/packages/mosaic/src/features/reverification/__tests__/reverification.fixtures.tsx @@ -0,0 +1,94 @@ +import type { SignInFirstFactorJSON } from '@clerk/shared/types'; +import { useState } from 'react'; + +import { fapiEmailAddress, fapiSession, fapiUser } from '../../../__tests__/feature/fapi'; +import { Card } from '../../../components/card'; +import { Reverification, useReverificationFlow } from '../reverification'; + +const alice = fapiUser({ + id: 'user_1', + first_name: 'Alice', + last_name: 'Smith', + email_addresses: [fapiEmailAddress({ id: 'idn_alice', email_address: 'alice@example.com' })], +}); + +export const aliceSession = fapiSession({ id: 'sess_1', user: alice }); + +export const VERIFY = '/v1/client/sessions/sess_1/verify'; +export const PREPARE_FIRST = `${VERIFY}/prepare_first_factor`; +export const PREPARE_SECOND = `${VERIFY}/prepare_second_factor`; +export const ATTEMPT_FIRST = `${VERIFY}/attempt_first_factor`; +export const ATTEMPT_SECOND = `${VERIFY}/attempt_second_factor`; + +export const PASSWORD = 'hunter2'; +export const CODE = '123456'; +export const BACKUP_CODE = 'backup-1'; + +export const SECRETS = { password: PASSWORD, email_code: CODE, phone_code: CODE, totp: CODE, backup_code: BACKUP_CODE }; + +export const passwordFactor: SignInFirstFactorJSON = { strategy: 'password' }; +export const emailFactor: SignInFirstFactorJSON = { + strategy: 'email_code', + email_address_id: 'idn_alice', + safe_identifier: 'alice@example.com', +}; +export const workEmailFactor: SignInFirstFactorJSON = { + strategy: 'email_code', + email_address_id: 'idn_work', + safe_identifier: 'alice@work.example', +}; +export const phoneFactor: SignInFirstFactorJSON = { + strategy: 'phone_code', + phone_number_id: 'idn_phone', + safe_identifier: '+1 555 0100', +}; + +export const STEP = { + password: 'Enter your current password to continue', + emailCode: 'Enter the code sent to your email to continue', + phoneCode: 'Enter the code sent to your phone to continue', + totp: 'Enter the code generated by your authenticator app to continue', + backupCode: 'Enter the backup code you received when setting up two-step authentication', + passkey: /Using your passkey confirms your identity/, +}; + +export function errorCode(error: unknown) { + if (error instanceof Error) { + return 'code' in error && typeof error.code === 'string' ? error.code : error.message; + } + return 'unknown'; +} + +export function HoldingHost({ action }: { action: () => Promise }) { + const [run, reverification] = useReverificationFlow(action); + const [outcome, setOutcome] = useState(''); + + return ( + <> + + {outcome} + {String(reverification.visible)} + + + {reverification.onCancel ? ( + + ) : null} + + + ); +} From b5722b95abf0e6ca3e3c51d6a6008f8f99346166 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Sat, 31 Oct 2026 12:58:27 -0600 Subject: [PATCH 4/6] refactor(mosaic): isolate reverification request draining --- .../reverification.controller.test.ts | 176 ++++++++++- .../reverification.controller.ts | 279 +++++++++--------- .../reverification/reverification.model.tsx | 20 +- .../reverification/reverification.utils.ts | 4 + 4 files changed, 320 insertions(+), 159 deletions(-) diff --git a/packages/mosaic/src/features/reverification/__tests__/reverification.controller.test.ts b/packages/mosaic/src/features/reverification/__tests__/reverification.controller.test.ts index c1458a72c6f..cda3905e86c 100644 --- a/packages/mosaic/src/features/reverification/__tests__/reverification.controller.test.ts +++ b/packages/mosaic/src/features/reverification/__tests__/reverification.controller.test.ts @@ -4,7 +4,11 @@ import { deferred, tick } from '../../../__tests__/async'; import { createActor } from '../../../machine/create-actor'; import { type ReverificationDeps, reverificationMachine } from '../reverification.controller'; import type { ReverificationPreparation } from '../reverification.model'; -import type { ReverificationPreparableMethod } from '../reverification.types'; +import type { + ReverificationMethod, + ReverificationPreparableMethod, + ReverificationResult, +} from '../reverification.types'; const email: ReverificationPreparableMethod = { id: 'email_code:alice', @@ -23,9 +27,11 @@ const work: ReverificationPreparableMethod = { function deps(): ReverificationDeps { return { - start: vi.fn(async () => ({ status: 'needs_first_factor', methods: [email, work], startingMethod: email })), - prepare: vi.fn(async () => ({ status: 'unverified', strategy: 'email_code', expiresAt: Date.now() + 60_000 })), - attempt: vi.fn(async () => ({ status: 'complete', methods: [], startingMethod: null })), + start: vi.fn(() => + Promise.resolve({ status: 'needs_first_factor', methods: [email, work], startingMethod: email }), + ), + prepare: vi.fn(() => Promise.resolve({ reusableUntil: Date.now() + 60_000 })), + attempt: vi.fn(() => Promise.resolve({ status: 'complete', methods: [], startingMethod: null })), finish: vi.fn(async () => {}), }; } @@ -36,8 +42,7 @@ describe('reverification operation ordering', () => { const actor = createActor(reverificationMachine, { snapshot: { value, context: { deps: effects, scope: { runId: Symbol(), sessionId: 'session' } } }, }).start(); - actor.setContext({ requestedScope: { runId: Symbol(), sessionId: 'session' } }); - actor.send({ type: 'START' }); + actor.send({ type: 'START', scope: { runId: Symbol(), sessionId: 'session' } }); await tick(); expect(effects.start).toHaveBeenCalledOnce(); actor.stop(); @@ -45,17 +50,22 @@ describe('reverification operation ordering', () => { it('coalesces A to B to A selections while A is pending', async () => { const effects = deps(); + const password: ReverificationMethod = { id: 'password', stage: 'first', strategy: 'password' }; + effects.start = vi.fn(() => + Promise.resolve({ status: 'needs_first_factor', methods: [password, email, work], startingMethod: password }), + ); const pending = deferred(); effects.prepare = vi.fn(() => pending.promise); const actor = createActor(reverificationMachine, { - context: { deps: effects, requestedScope: { runId: Symbol(), sessionId: 'session' } }, + context: { deps: effects }, }).start(); - actor.send({ type: 'START' }); + actor.send({ type: 'START', scope: { runId: Symbol(), sessionId: 'session' } }); await tick(); actor.send({ type: 'SHOW_METHODS' }); + actor.send({ type: 'SELECT_METHOD', id: email.id }); actor.send({ type: 'SELECT_METHOD', id: work.id }); actor.send({ type: 'SELECT_METHOD', id: email.id }); - pending.resolve({ status: 'unverified', strategy: 'email_code', expiresAt: Date.now() + 60_000 }); + pending.resolve({ reusableUntil: Date.now() + 60_000 }); await tick(); expect(effects.prepare).toHaveBeenCalledExactlyOnceWith(email); actor.send({ type: 'TYPE', value: '123456' }); @@ -65,3 +75,151 @@ describe('reverification operation ordering', () => { actor.stop(); }); }); + +describe('reverification interrupted requests', () => { + it.each(['start', 'prepare', 'attempt', 'finish'] as const)( + 'waits for an interrupted %s before starting the latest run', + async operation => { + for (const fails of [false, true]) { + const effects = deps(); + const result = deferred(); + const preparation = deferred(); + const finish = deferred(); + if (operation === 'start') { + effects.start = vi.fn(() => result.promise); + } + if (operation === 'prepare') { + effects.prepare = vi.fn(() => preparation.promise); + } + if (operation === 'attempt') { + effects.attempt = vi.fn(() => result.promise); + } + if (operation === 'finish') { + effects.finish = vi.fn(() => finish.promise); + } + const actor = createActor(reverificationMachine, { context: { deps: effects } }).start(); + const start = () => { + const scope = { runId: Symbol(), sessionId: 'session' }; + actor.send({ type: 'START', scope }); + return scope; + }; + start(); + await tick(); + if (operation === 'attempt' || operation === 'finish') { + actor.send({ type: 'SUBMIT' }); + await tick(); + } + actor.send({ type: 'RESET' }); + start(); + actor.send({ type: 'RESET' }); + const latest = start(); + await tick(); + expect(effects.start).toHaveBeenCalledOnce(); + const nextStart = vi.fn(deps().start); + actor.setContext({ deps: { ...deps(), start: nextStart } }); + if (operation === 'prepare') { + if (fails) { + preparation.reject(new Error('stale prepare')); + } else { + preparation.resolve({ reusableUntil: Date.now() + 60_000 }); + } + } else if (operation === 'finish') { + if (fails) { + finish.reject(new Error('stale finish')); + } else { + finish.resolve(); + } + } else { + if (fails) { + result.reject(new Error('stale result')); + } else { + result.resolve({ status: 'complete', methods: [], startingMethod: null }); + } + } + await tick(); + expect(nextStart).toHaveBeenCalledOnce(); + expect(actor.getSnapshot().context.scope).toBe(latest); + expect(actor.getSnapshot().context.error).toBeUndefined(); + expect(effects.finish).toHaveBeenCalledTimes(operation === 'finish' ? 1 : 0); + actor.stop(); + } + }, + ); + + it('checking whether a method can be selected never sends a code', async () => { + const effects = deps(); + const actor = createActor(reverificationMachine, { context: { deps: effects } }).start(); + actor.send({ type: 'START', scope: { runId: Symbol(), sessionId: 'session' } }); + await tick(); + actor.send({ type: 'SHOW_METHODS' }); + expect(actor.can({ type: 'SELECT_METHOD', id: work.id })).toBe(true); + expect(actor.can({ type: 'SELECT_METHOD', id: work.id })).toBe(true); + expect(effects.prepare).toHaveBeenCalledExactlyOnceWith(email); + actor.stop(); + }); +}); + +describe('reverification preparation overlays', () => { + it('blocks leaving a pending picker and preserves help opened over the factor', async () => { + const effects = deps(); + const pending = deferred(); + effects.prepare = vi.fn(() => pending.promise); + const actor = createActor(reverificationMachine, { context: { deps: effects } }).start(); + actor.send({ type: 'START', scope: { runId: Symbol(), sessionId: 'session' } }); + await tick(); + actor.send({ type: 'SHOW_HELP' }); + expect(actor.getSnapshot().context.overlay).toEqual(['help']); + expect(actor.can({ type: 'BACK' })).toBe(true); + actor.send({ type: 'BACK' }); + actor.send({ type: 'SHOW_METHODS' }); + actor.send({ type: 'SELECT_METHOD', id: work.id }); + expect(actor.can({ type: 'BACK' })).toBe(false); + expect(actor.can({ type: 'SHOW_HELP' })).toBe(false); + actor.send({ type: 'BACK' }); + actor.send({ type: 'SHOW_HELP' }); + expect(actor.getSnapshot().context.overlay).toEqual(['method-picker']); + actor.stop(); + }); + + it('keeps help visible when the send behind it settles', async () => { + const effects = deps(); + const pending = deferred(); + effects.prepare = vi.fn(() => pending.promise); + const actor = createActor(reverificationMachine, { context: { deps: effects } }).start(); + actor.send({ type: 'START', scope: { runId: Symbol(), sessionId: 'session' } }); + await tick(); + actor.send({ type: 'SHOW_HELP' }); + pending.resolve({ reusableUntil: Date.now() + 60_000 }); + await tick(); + expect(actor.getSnapshot().context.overlay).toEqual(['help']); + expect(actor.getSnapshot().context.operation).toBeNull(); + actor.send({ type: 'BACK' }); + expect(actor.getSnapshot().context.overlay).toEqual([]); + actor.stop(); + }); +}); + +it.each([false, true])( + 'keeps a picker opened over the original pending factor when the send settles (failure %s)', + async fails => { + const effects = deps(); + const pending = deferred(); + effects.prepare = vi.fn(() => pending.promise); + const actor = createActor(reverificationMachine, { context: { deps: effects } }).start(); + actor.send({ type: 'START', scope: { runId: Symbol(), sessionId: 'session' } }); + await tick(); + actor.send({ type: 'SHOW_METHODS' }); + expect(actor.can({ type: 'BACK' })).toBe(true); + expect(actor.can({ type: 'SHOW_HELP' })).toBe(true); + if (fails) { + pending.reject({ code: 'form_identifier_not_found', message: 'Unknown identifier' }); + } else { + pending.resolve({ reusableUntil: Date.now() + 60_000 }); + } + await tick(); + expect(actor.getSnapshot().context.overlay).toEqual(['method-picker']); + actor.send({ type: 'BACK' }); + expect(actor.getSnapshot().context.overlay).toEqual([]); + actor.stop(); + }, +); diff --git a/packages/mosaic/src/features/reverification/reverification.controller.ts b/packages/mosaic/src/features/reverification/reverification.controller.ts index 5ed1d57592d..30850000d76 100644 --- a/packages/mosaic/src/features/reverification/reverification.controller.ts +++ b/packages/mosaic/src/features/reverification/reverification.controller.ts @@ -8,12 +8,7 @@ import { setup } from '../../machine/setup'; import type { DoneInvokeEvent, ErrorInvokeEvent, StateConfig } from '../../machine/types'; import { useMachine } from '../../machine/use-machine'; import { toLocalizableError } from '../../utils/errors'; -import type { - ReverificationActiveModel, - ReverificationModel, - ReverificationPreparation, - ReverificationScope, -} from './reverification.model'; +import type { ReverificationActiveModel, ReverificationModel, ReverificationScope } from './reverification.model'; import type { ReverificationErrorReason, ReverificationMethod, @@ -22,7 +17,7 @@ import type { ReverificationStep, ReverificationViewProps, } from './reverification.types'; -import { needsPrepare, otpChannelFor } from './reverification.utils'; +import { invalidatesPreparedCode, needsPrepare, otpChannelFor } from './reverification.utils'; type ReverificationControllerState = | { status: 'idle'; onCancel?: undefined } @@ -50,16 +45,20 @@ type RetainedPreparation = { resendAvailableAt: number; }; +type PreparationSettlement = { + method: ReverificationPreparableMethod; + resendAvailableAt: number; +} & ({ status: 'sent'; reusableUntil: number | null } | { status: 'failed'; error: unknown }); + +type Operation = + | { kind: 'result'; promise: Promise } + | { kind: 'prepare'; promise: Promise } + | { kind: 'finish'; promise: Promise }; + interface ReverificationContext { scope: ReverificationScope | null; - requestedScope: ReverificationScope | null; - operationScope: ReverificationScope | null; retainedPreparation: RetainedPreparation | null; - preparation: { - method: ReverificationPreparableMethod; - resendAvailableAt: number; - presentation: 'factor' | 'picker'; - } | null; + operation: Operation | null; inputValue: string; error: ErrorDescription | undefined; direction: FlowDirection; @@ -74,7 +73,7 @@ interface ReverificationContext { } type ReverificationEvent = - | { type: 'START' } + | { type: 'START'; scope: ReverificationScope } | { type: 'RESET' } | { type: 'TYPE'; value: string } | { type: 'SUBMIT' } @@ -107,19 +106,11 @@ function isPasskeyAbortedError(error: unknown): boolean { return isClerkRuntimeError(error) && error.code === 'passkey_operation_aborted'; } -function selectMethod(ctx: ReverificationContext, id: string): Partial { - return { - activeMethod: ctx.methods.find(method => method.id === id) ?? ctx.activeMethod, - inputValue: '', - error: undefined, - direction: 1, - submitRequested: false, - }; -} - -function prepareActive(ctx: ReverificationContext) { - const preparation = ctx.preparation; - return preparation ? ctx.deps.prepare(preparation.method) : Promise.reject(new Error('No preparation')); +function resultPromise(ctx: ReverificationContext): Promise { + if (ctx.operation?.kind !== 'result') { + throw new Error('Expected a reverification result operation'); + } + return ctx.operation.promise; } function submit(ctx: ReverificationContext): Promise { @@ -137,13 +128,17 @@ function sameMethod(a: ReverificationMethod | null, b: ReverificationMethod | nu function beginPreparation(ctx: ReverificationContext): Partial { const method = ctx.activeMethod; if (!method || !needsPrepare(method)) { - return {}; + throw new Error('Expected a preparable reverification method'); } const resendAvailableAt = Date.now() + RESEND_COOLDOWN_MS; + const promise = ctx.deps.prepare(method).then( + result => ({ status: 'sent', method, resendAvailableAt, ...result }), + (error: unknown) => ({ status: 'failed', method, resendAvailableAt, error }), + ); return { retainedPreparation: null, resendAvailableAt, - preparation: { method, resendAvailableAt, presentation: topOverlay(ctx) === 'method-picker' ? 'picker' : 'factor' }, + operation: { kind: 'prepare', promise }, }; } @@ -151,16 +146,6 @@ function sameScope(a: ReverificationScope | null, b: ReverificationScope | null) return a?.runId === b?.runId && a?.sessionId === b?.sessionId; } -function settledRun(ctx: ReverificationContext) { - if (!ctx.scope) { - return 'inactive' as const; - } - if (!sameScope(ctx.scope, ctx.operationScope)) { - return 'starting' as const; - } - return undefined; -} - function clearRun(): Partial { return { scope: null, @@ -176,54 +161,40 @@ function clearRun(): Partial { }; } -const bindRun = assign(ctx => ({ ...clearRun(), scope: ctx.requestedScope })); -const drainingEvents = { - RESET: { actions: assign(clearRun) }, - START: { actions: bindRun }, -}; -const afterDrain = [ - { guard: (ctx: ReverificationContext) => settledRun(ctx) === 'inactive', target: 'inactive' as const }, - { guard: (ctx: ReverificationContext) => settledRun(ctx) === 'starting', target: 'starting' as const }, -]; +const bindRun = assign>((_, event) => ({ + ...clearRun(), + scope: event.scope, +})); +const runEvents = { + RESET: 'inactive', + START: { target: 'starting', actions: bindRun }, +} as const; +const interruptionEvents = { + RESET: { target: 'draining', actions: assign(clearRun) }, + START: { target: 'draining', actions: bindRun }, +} as const; +const clearOperation = assign(() => ({ operation: null })); +const resumeRun = ({ context }: { context: ReverificationContext }) => ({ + target: context.scope ? ('starting' as const) : ('inactive' as const), +}); -function settlePreparation( - ctx: ReverificationContext, - result: { status: 'sent'; verification: ReverificationPreparation } | { status: 'failed'; error: unknown }, -) { - const drained = settledRun(ctx); - if (drained) { - return { target: drained }; - } - const preparation = ctx.preparation; +function settlePreparation(ctx: ReverificationContext, result: PreparationSettlement) { const method = ctx.activeMethod; - if (!preparation || !method) { - return { target: 'inactive' as const }; - } - if (needsPrepare(method) && !sameMethod(method, preparation.method)) { + if (method && needsPrepare(method) && !sameMethod(method, result.method)) { return { target: 'preparing' as const }; } - const matches = sameMethod(method, preparation.method); - const verification = result.status === 'sent' ? result.verification : null; - const retainedPreparation = - verification?.status === 'unverified' && - verification.strategy === preparation.method.strategy && - verification.expiresAt !== null && - Number.isFinite(verification.expiresAt) && - verification.expiresAt > Date.now() - ? { - method: preparation.method, - expiresAt: verification.expiresAt, - resendAvailableAt: preparation.resendAvailableAt, - } - : null; + const matches = sameMethod(method, result.method); return { target: ctx.submitRequested && (result.status === 'sent' || !matches) ? ('submitting' as const) : ('verifying' as const), context: { - retainedPreparation, - preparation: null, - resendAvailableAt: result.status === 'sent' ? preparation.resendAvailableAt : undefined, - overlay: preparation.presentation === 'picker' ? [] : ctx.overlay, + operation: null, + retainedPreparation: + result.status === 'sent' && result.reusableUntil !== null + ? { method: result.method, expiresAt: result.reusableUntil, resendAvailableAt: result.resendAvailableAt } + : null, + resendAvailableAt: result.status === 'sent' ? result.resendAvailableAt : undefined, + overlay: pendingPicker(ctx) ? [] : ctx.overlay, error: matches && result.status === 'failed' ? toLocalizableError(result.error) : ctx.error, submitRequested: false, }, @@ -231,6 +202,7 @@ function settlePreparation( } const applyResult = assign>((_, event) => ({ + operation: null, retainedPreparation: null, methods: event.output.methods, activeMethod: event.output.startingMethod, @@ -240,7 +212,6 @@ const applyResult = assign>((_, event) => direction: 1, })); const afterResult = [ - ...afterDrain, { guard: (_: ReverificationContext, event: DoneInvokeEvent) => event.output.status === 'complete', @@ -268,6 +239,14 @@ function topOverlay(ctx: ReverificationContext): Overlay | undefined { return ctx.overlay[ctx.overlay.length - 1]; } +function pendingPicker(ctx: ReverificationContext): boolean { + return ( + ctx.operation?.kind === 'prepare' && + topOverlay(ctx) === 'method-picker' && + ctx.activeMethod?.id !== ctx.frozenActiveMethodId + ); +} + const factorEvents = { TYPE: { actions: assign((_, event) => ({ inputValue: event.value, error: undefined })), @@ -283,7 +262,7 @@ const factorEvents = { })), }, SHOW_HELP: { - guard: ctx => topOverlay(ctx) !== 'help', + guard: ctx => topOverlay(ctx) !== 'help' && !pendingPicker(ctx), actions: assign(ctx => ({ overlay: [...ctx.overlay, 'help' as const], direction: 1 as const, @@ -291,7 +270,7 @@ const factorEvents = { })), }, BACK: { - guard: ctx => ctx.overlay.length > 0, + guard: ctx => ctx.overlay.length > 0 && !pendingPicker(ctx), actions: assign(ctx => ({ overlay: ctx.overlay.slice(0, -1), direction: -1 as const })), }, SELECT_METHOD: ({ context: ctx, event }) => { @@ -302,20 +281,29 @@ const factorEvents = { if (!method) { return; } + const selection = { + activeMethod: method, + inputValue: '', + error: undefined, + direction: 1 as const, + submitRequested: false, + }; + if (ctx.operation?.kind === 'prepare') { + return { context: { ...selection, overlay: needsPrepare(method) ? ctx.overlay : [] } }; + } const retained = ctx.retainedPreparation; const reusable = retained && sameMethod(method, retained.method) && retained.expiresAt > Date.now(); const prepare = needsPrepare(method) && !reusable; return { target: prepare ? 'preparing' : 'verifying', context: { - ...selectMethod(ctx, event.id), + ...selection, overlay: prepare ? ctx.overlay : [], resendAvailableAt: reusable ? retained.resendAvailableAt : ctx.resendAvailableAt, }, }; }, - RESET: 'inactive', - START: { target: 'starting', actions: bindRun }, + ...runEvents, } satisfies NonNullable['on']>; export const reverificationMachine = createMachine({ @@ -323,10 +311,8 @@ export const reverificationMachine = createMachine({ initial: 'inactive', context: { scope: null, - requestedScope: null, - operationScope: null, retainedPreparation: null, - preparation: null, + operation: null, inputValue: '', error: undefined, direction: 1, @@ -341,46 +327,39 @@ export const reverificationMachine = createMachine({ }, states: { inactive: { - entry: assign(() => ({ ...clearRun(), preparation: null, operationScope: null })), + entry: assign(() => ({ ...clearRun(), operation: null })), on: { START: { target: 'starting', actions: bindRun } }, }, starting: { - entry: assign(ctx => ({ operationScope: ctx.scope })), - on: drainingEvents, - invoke: fromPromise(ctx => ctx.deps.start(), { + entry: assign(ctx => ({ operation: { kind: 'result', promise: ctx.deps.start() } })), + on: interruptionEvents, + invoke: fromPromise(resultPromise, { onDone: afterResult, - onError: [...afterDrain, { target: 'failed' }], + onError: { target: 'failed', actions: clearOperation }, }), }, preparing: { - entry: assign(ctx => ({ ...beginPreparation(ctx), operationScope: ctx.scope })), + entry: assign(beginPreparation), on: { ...factorEvents, - ...drainingEvents, - SELECT_METHOD: { - guard: (ctx, event) => - topOverlay(ctx) === 'method-picker' && ctx.methods.some(method => method.id === event.id), - actions: assign((ctx, event) => { - const selection = selectMethod(ctx, event.id); - const sending = selection.activeMethod && needsPrepare(selection.activeMethod); - return { - ...selection, - overlay: sending ? ctx.overlay : [], - preparation: ctx.preparation ? { ...ctx.preparation, presentation: sending ? 'picker' : 'factor' } : null, - }; - }), - }, + ...interruptionEvents, SUBMIT: { guard: ctx => ctx.overlay.length === 0, actions: assign(() => ({ submitRequested: true })), }, }, - invoke: fromPromise(prepareActive, { - onDone: ({ context, event }) => settlePreparation(context, { status: 'sent', verification: event.output }), - onError: ({ context, event }) => settlePreparation(context, { status: 'failed', error: event.error }), - }), + invoke: fromPromise( + ctx => { + const operation = ctx.operation; + if (operation?.kind !== 'prepare') { + throw new Error('Expected preparation'); + } + return operation.promise; + }, + { onDone: ({ context, event }) => settlePreparation(context, event.output) }, + ), }, verifying: { @@ -405,51 +384,73 @@ export const reverificationMachine = createMachine({ submitting: { entry: assign(ctx => ({ - operationScope: ctx.scope, + operation: { kind: 'result', promise: submit(ctx) }, retainedPreparation: - ctx.activeMethod?.strategy === 'totp' || ctx.activeMethod?.strategy === 'backup_code' - ? ctx.retainedPreparation - : null, + ctx.activeMethod && !invalidatesPreparedCode(ctx.activeMethod) ? ctx.retainedPreparation : null, })), - on: drainingEvents, - invoke: fromPromise(submit, { + on: interruptionEvents, + invoke: fromPromise(resultPromise, { onDone: afterResult, onError: [ - ...afterDrain, { guard: (_: ReverificationContext, event: ErrorInvokeEvent) => isPasskeyAbortedError(event.error), target: 'verifying', - actions: assign(() => ({ error: undefined })), + actions: assign(() => ({ operation: null, error: undefined })), }, { target: 'verifying', - actions: assign((_, event) => ({ error: toLocalizableError(event.error) })), + actions: assign((_, event) => ({ operation: null, error: toLocalizableError(event.error) })), }, ], }), }, unavailable: { - on: { RESET: 'inactive', START: { target: 'starting', actions: bindRun } }, + on: runEvents, }, failed: { - on: { RESET: 'inactive', START: { target: 'starting', actions: bindRun } }, + on: runEvents, }, finishing: { - entry: assign(ctx => ({ retainedPreparation: null, operationScope: ctx.scope })), - on: drainingEvents, - invoke: fromPromise(ctx => ctx.deps.finish(), { - onDone: [...afterDrain, { target: 'retrying' }], - onError: [...afterDrain, { target: 'failed' }], - }), + entry: assign(ctx => ({ retainedPreparation: null, operation: { kind: 'finish', promise: ctx.deps.finish() } })), + on: interruptionEvents, + invoke: fromPromise( + ctx => { + const operation = ctx.operation; + if (operation?.kind !== 'finish') { + throw new Error('Expected finishing operation'); + } + return operation.promise; + }, + { + onDone: { target: 'retrying', actions: clearOperation }, + onError: { target: 'failed', actions: clearOperation }, + }, + ), + }, + + draining: { + on: { + RESET: { actions: assign(clearRun) }, + START: { actions: bindRun }, + }, + invoke: fromPromise( + ctx => { + if (!ctx.operation) { + throw new Error('Expected an operation to drain'); + } + return ctx.operation.promise; + }, + { onDone: resumeRun, onError: resumeRun }, + ), }, // This means we are retrying the action after successful reverification // This state is left by the model returning to inactive once the retry has completed retrying: { - on: { RESET: 'inactive', START: { target: 'starting', actions: bindRun } }, + on: runEvents, }, }, }); @@ -530,7 +531,6 @@ function useReverificationState(model: ReverificationModel): ReverificationContr active ? { context: { - requestedScope: active.scope, supportEmail: active.supportEmail, deps: active, }, @@ -560,12 +560,12 @@ function useReverificationState(model: ReverificationModel): ReverificationContr const needsReset = model.status === 'inactive' && snapshot.context.scope !== null; /* eslint-disable react-you-might-not-need-an-effect/no-event-handler -- Syncs the machine with the model status, which changes outside this component. */ useEffect(() => { - if (needsStart) { - send({ type: 'START' }); + if (needsStart && active) { + send({ type: 'START', scope: active.scope }); } else if (needsReset) { send({ type: 'RESET' }); } - }, [needsStart, needsReset, send]); + }, [needsStart, needsReset, active, send]); /* eslint-enable react-you-might-not-need-an-effect/no-event-handler */ const inactive = model.status === 'inactive'; @@ -595,13 +595,10 @@ function useReverificationState(model: ReverificationModel): ReverificationContr // If we are currently on the alternative methods screen and preparing a factor, activeMethod will // have transitioned to the factor we are now preparing, so the one we want to hide is the old one - const excludeId = - state === 'preparing' && context.preparation?.presentation === 'picker' - ? context.frozenActiveMethodId - : activeMethod?.id; + const isPendingPicker = pendingPicker(context); + const excludeId = isPendingPicker ? context.frozenActiveMethodId : activeMethod?.id; const methods = context.methods.filter(method => method.id !== excludeId); - const pendingMethodId = - state === 'preparing' && context.preparation?.presentation === 'picker' ? activeMethod?.id : undefined; + const pendingMethodId = isPendingPicker ? activeMethod?.id : undefined; const view: ReverificationViewProps = { step, diff --git a/packages/mosaic/src/features/reverification/reverification.model.tsx b/packages/mosaic/src/features/reverification/reverification.model.tsx index 181469a4098..4ef3d2453ae 100644 --- a/packages/mosaic/src/features/reverification/reverification.model.tsx +++ b/packages/mosaic/src/features/reverification/reverification.model.tsx @@ -4,7 +4,6 @@ import type { SessionVerificationFirstFactor, SessionVerificationResource, SessionVerificationSecondFactor, - VerificationResource, } from '@clerk/shared/types'; import { isWebAuthnSupported } from '@clerk/shared/webauthn'; @@ -19,9 +18,7 @@ import type { } from './reverification.types'; import { pickStartingMethod } from './reverification.utils'; -export type ReverificationPreparation = Pick & { - expiresAt: number | null; -}; +export type ReverificationPreparation = { reusableUntil: number | null }; export type ReverificationScope = { runId: symbol; sessionId: string }; @@ -171,11 +168,16 @@ export function useReverificationModel(reverificationState: ReverificationState) }); const verification = method.stage === 'first' ? resource.firstFactorVerification : resource.secondFactorVerification; - return { - status: verification.status, - strategy: verification.strategy, - expiresAt: verification.expireAt?.getTime() ?? null, - }; + const expiresAt = verification.expireAt?.getTime(); + const reusableUntil = + verification.status === 'unverified' && + verification.strategy === method.strategy && + expiresAt !== undefined && + Number.isFinite(expiresAt) && + expiresAt > Date.now() + ? expiresAt + : null; + return { reusableUntil }; }, attempt: async (method, value) => { switch (method.strategy) { diff --git a/packages/mosaic/src/features/reverification/reverification.utils.ts b/packages/mosaic/src/features/reverification/reverification.utils.ts index 42f07b27c3e..61867a65114 100644 --- a/packages/mosaic/src/features/reverification/reverification.utils.ts +++ b/packages/mosaic/src/features/reverification/reverification.utils.ts @@ -24,6 +24,10 @@ export function needsPrepare(method: ReverificationMethod): method is Reverifica return method.strategy === 'email_code' || method.strategy === 'phone_code'; } +export function invalidatesPreparedCode(method: ReverificationMethod): boolean { + return method.strategy !== 'totp' && method.strategy !== 'backup_code'; +} + function pickStartingFirstFactor( methods: readonly ReverificationMethod[], preferredSignInStrategy: PreferredSignInStrategy | undefined, From 6f2c70bc17f8fa34be18baefbf4f27a2b3c6eab7 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Sat, 31 Oct 2026 13:04:53 -0600 Subject: [PATCH 5/6] refactor(mosaic): use shared clock for OTP countdown --- .../reverification.controller.ts | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/packages/mosaic/src/features/reverification/reverification.controller.ts b/packages/mosaic/src/features/reverification/reverification.controller.ts index 30850000d76..64809416c05 100644 --- a/packages/mosaic/src/features/reverification/reverification.controller.ts +++ b/packages/mosaic/src/features/reverification/reverification.controller.ts @@ -1,7 +1,8 @@ import { isClerkRuntimeError } from '@clerk/shared/error'; -import { useEffect, useMemo, useState } from 'react'; +import { useEffect, useMemo } from 'react'; import type { FlowDirection } from '../../components/flow'; +import { useNow } from '../../hooks/use-now'; import type { ErrorDescription } from '../../localization'; import { useErrorText, useMessages } from '../../localization'; import { setup } from '../../machine/setup'; @@ -538,19 +539,11 @@ function useReverificationState(model: ReverificationModel): ReverificationContr : undefined, ); - const [now, setNow] = useState(() => Date.now()); const resendAvailableAt = snapshot.context.resendAvailableAt; + const now = useNow({ updateInterval: resendAvailableAt === undefined ? undefined : 1000 }).getTime(); const canResend = resendAvailableAt === undefined || now >= resendAvailableAt; const countingDown = !canResend; - useEffect(() => { - if (!countingDown) { - return; - } - const id = window.setInterval(() => setNow(Date.now()), 1000); - return () => window.clearInterval(id); - }, [countingDown, resendAvailableAt]); - const cancelModel = (model.status === 'loading' || model.status === 'active') && snapshot.value !== 'retrying' ? model.cancel @@ -622,12 +615,6 @@ function useReverificationState(model: ReverificationModel): ReverificationContr otpChannel: activeMethod ? otpChannelFor(activeMethod.strategy) : undefined, onResend: () => send({ type: 'RESEND' }), canResend, - // We clamp this to 30s because the first render after locking resend - // will have the old `now` state set, which would result in a value above - // 30s. There are other solutions like reading Date.now() in render and - // letting the interval only retrigger render, but that makes the render - // impure which is something the React compiler would warn about. - // useSyncExternalStore feels unnecessarily complex here resendRemainingSeconds: countingDown && resendAvailableAt ? Math.min(RESEND_COOLDOWN_MS / 1000, Math.max(0, Math.ceil((resendAvailableAt - now) / 1000))) From b5d4fae447740236ee6b686b4a6a27dbb79b8bf7 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Sat, 31 Oct 2026 13:21:34 -0600 Subject: [PATCH 6/6] test(mosaic): consolidate reverification overlay coverage --- .../reverification.controller.test.ts | 65 ------------------- .../__tests__/reverification.feature.test.tsx | 11 +++- 2 files changed, 10 insertions(+), 66 deletions(-) diff --git a/packages/mosaic/src/features/reverification/__tests__/reverification.controller.test.ts b/packages/mosaic/src/features/reverification/__tests__/reverification.controller.test.ts index cda3905e86c..ff1eeb8ecb1 100644 --- a/packages/mosaic/src/features/reverification/__tests__/reverification.controller.test.ts +++ b/packages/mosaic/src/features/reverification/__tests__/reverification.controller.test.ts @@ -158,68 +158,3 @@ describe('reverification interrupted requests', () => { actor.stop(); }); }); - -describe('reverification preparation overlays', () => { - it('blocks leaving a pending picker and preserves help opened over the factor', async () => { - const effects = deps(); - const pending = deferred(); - effects.prepare = vi.fn(() => pending.promise); - const actor = createActor(reverificationMachine, { context: { deps: effects } }).start(); - actor.send({ type: 'START', scope: { runId: Symbol(), sessionId: 'session' } }); - await tick(); - actor.send({ type: 'SHOW_HELP' }); - expect(actor.getSnapshot().context.overlay).toEqual(['help']); - expect(actor.can({ type: 'BACK' })).toBe(true); - actor.send({ type: 'BACK' }); - actor.send({ type: 'SHOW_METHODS' }); - actor.send({ type: 'SELECT_METHOD', id: work.id }); - expect(actor.can({ type: 'BACK' })).toBe(false); - expect(actor.can({ type: 'SHOW_HELP' })).toBe(false); - actor.send({ type: 'BACK' }); - actor.send({ type: 'SHOW_HELP' }); - expect(actor.getSnapshot().context.overlay).toEqual(['method-picker']); - actor.stop(); - }); - - it('keeps help visible when the send behind it settles', async () => { - const effects = deps(); - const pending = deferred(); - effects.prepare = vi.fn(() => pending.promise); - const actor = createActor(reverificationMachine, { context: { deps: effects } }).start(); - actor.send({ type: 'START', scope: { runId: Symbol(), sessionId: 'session' } }); - await tick(); - actor.send({ type: 'SHOW_HELP' }); - pending.resolve({ reusableUntil: Date.now() + 60_000 }); - await tick(); - expect(actor.getSnapshot().context.overlay).toEqual(['help']); - expect(actor.getSnapshot().context.operation).toBeNull(); - actor.send({ type: 'BACK' }); - expect(actor.getSnapshot().context.overlay).toEqual([]); - actor.stop(); - }); -}); - -it.each([false, true])( - 'keeps a picker opened over the original pending factor when the send settles (failure %s)', - async fails => { - const effects = deps(); - const pending = deferred(); - effects.prepare = vi.fn(() => pending.promise); - const actor = createActor(reverificationMachine, { context: { deps: effects } }).start(); - actor.send({ type: 'START', scope: { runId: Symbol(), sessionId: 'session' } }); - await tick(); - actor.send({ type: 'SHOW_METHODS' }); - expect(actor.can({ type: 'BACK' })).toBe(true); - expect(actor.can({ type: 'SHOW_HELP' })).toBe(true); - if (fails) { - pending.reject({ code: 'form_identifier_not_found', message: 'Unknown identifier' }); - } else { - pending.resolve({ reusableUntil: Date.now() + 60_000 }); - } - await tick(); - expect(actor.getSnapshot().context.overlay).toEqual(['method-picker']); - actor.send({ type: 'BACK' }); - expect(actor.getSnapshot().context.overlay).toEqual([]); - actor.stop(); - }, -); diff --git a/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx b/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx index 837dafe9292..963ebadc751 100644 --- a/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx +++ b/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx @@ -677,12 +677,20 @@ describe('Reverification', () => { }, ); - it('keeps the cooldown of a code sent while the picker is open', async () => { + it.each(['picker', 'help'] as const)('keeps the cooldown of a code sent while the %s is open', async step => { const { prepare, user } = await startSendingCode(); await useAnotherMethod(user); + if (step === 'help') { + await user.click(await screen.findByRole('button', { name: 'Get help' })); + expect(await screen.findByText(/If you have trouble verifying your account/)).toBeVisible(); + } prepare.release(); await settle(); + if (step === 'help') { + expect(screen.getByText(/If you have trouble verifying your account/)).toBeVisible(); + await back(user); + } expect(screen.getByRole('button', { name: 'Continue with your password' })).toBeVisible(); await back(user); @@ -1196,6 +1204,7 @@ describe('Reverification', () => { await waitFor(() => expect(secondPrepare.requests).toHaveLength(1)); expect(method('Continue with your password')).toBeDisabled(); expect(method('Back')).toBeDisabled(); + expect(method('Get help')).toBeDisabled(); secondPrepare.release(); expect(await screen.findByText(STEP.phoneCode)).toBeVisible(); expect(fapi.verification.firstFactorVerification?.strategy).toBe('phone_code');