diff --git a/.changeset/fresh-otp-preparation.md b/.changeset/fresh-otp-preparation.md new file mode 100644 index 00000000000..5dcd7745562 --- /dev/null +++ b/.changeset/fresh-otp-preparation.md @@ -0,0 +1,5 @@ +--- +'@clerk/mosaic': patch +--- + +Preserve verification code delivery errors and resend cooldowns when navigating to other methods or help and back in UserProfile. diff --git a/packages/mosaic/src/__tests__/feature/fake-fapi-isolation.feature.test.tsx b/packages/mosaic/src/__tests__/feature/fake-fapi-isolation.feature.test.tsx index 9ed44df552f..f73dd43ba0c 100644 --- a/packages/mosaic/src/__tests__/feature/fake-fapi-isolation.feature.test.tsx +++ b/packages/mosaic/src/__tests__/feature/fake-fapi-isolation.feature.test.tsx @@ -44,4 +44,17 @@ describe('fake FAPI isolation between tests', () => { expect(await clerk.session?.getToken()).toEqual(expect.any(String)); expect(document.cookie).not.toMatch(/(?:^|; )__session/); }); + + it('clears authentication cookies when the test iframe leaves the page', async () => { + const user = fapiUser({ id: 'user_1' }); + serveFapi({ client: fapiClient([fapiSession({ id: 'sess_1', user })]) }); + await renderWithClerk(
, { standardBrowser: true }); + window.dispatchEvent(new Event('focus')); + await new Promise(resolve => setTimeout(resolve, 0)); + expect(document.cookie).toContain('__session='); + + window.dispatchEvent(new PageTransitionEvent('pagehide')); + + expect(document.cookie).toBe(''); + }); }); 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 8fb2a015672..c010c91de98 100644 --- a/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx +++ b/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx @@ -665,6 +665,170 @@ describe('Reverification', () => { expect(screen.getByRole('button', { name: /Resend/ })).toBeEnabled(); }); + describe('when the user leaves the step while the code is being sent', () => { + async function startSendingCode( + firstFactors: SignInFirstFactorJSON[] = [emailFactor, passwordFactor], + action = guardedAction(), + ) { + await renderReverification(action, verificationSeed({ firstFactors }, { environment: preferring('otp') })); + const prepare = holdRequests('post', PREPARE_FIRST); + const user = await startVerification(); + expect(await screen.findByText(STEP.emailCode)).toBeVisible(); + await waitFor(() => expect(prepare.requests).toHaveLength(1)); + return { prepare, user }; + } + + const back = (user: User) => user.click(screen.getByRole('button', { name: 'Back' })); + + it.each(['picker', 'help'] as const)( + 'shows a failure that arrives on the %s step once the user returns', + 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.fail('preparation_failed', 'Could not send your code'); + await settle(); + if (step === 'help') { + expect(screen.getByText(/If you have trouble verifying your account/)).toBeVisible(); + await back(user); + } + expect(await screen.findByRole('button', { name: 'Continue with your password' })).toBeVisible(); + await back(user); + + // The step shows the failure and the user can resend immediately + expect(await screen.findByText(STEP.emailCode)).toBeVisible(); + expect(await screen.findByText('Could not send your code')).toBeVisible(); + expect(prepare.requests).toHaveLength(1); + const retry = holdRequests('post', PREPARE_FIRST); + await user.click(screen.getByRole('button', { name: /Resend/ })); + await waitFor(() => expect(retry.requests).toHaveLength(1)); + expect(screen.queryByText('Could not send your code')).toBeNull(); + expect(screen.getByRole('button', { name: /Resend \(\d+\)/ })).toBeDisabled(); + retry.release(); + }, + ); + + it('keeps the cooldown of a code sent while the picker is open', async () => { + const { prepare, user } = await startSendingCode(); + await useAnotherMethod(user); + + prepare.release(); + await settle(); + expect(screen.getByRole('button', { name: 'Continue with your password' })).toBeVisible(); + await back(user); + + expect(await screen.findByText(STEP.emailCode)).toBeVisible(); + expect(screen.getByRole('button', { name: /Resend \(\d+\)/ })).toBeDisabled(); + expect(prepare.requests).toHaveLength(1); + }); + + it.each(['sent', 'not sent'] as const)( + 'holds a submit made after returning from the picker until the code is %s', + async result => { + const { prepare, user } = await startSendingCode(); + const attempt = holdRequests('post', ATTEMPT_FIRST); + await useAnotherMethod(user); + await back(user); + expect(await screen.findByText(STEP.emailCode)).toBeVisible(); + + await pasteCode(user, CODE); + await settle(); + expect(attempt.requests).toHaveLength(0); + + if (result === 'sent') { + prepare.release(); + await waitFor(() => expect(attempt.requests).toHaveLength(1)); + attempt.release(); + await untilResolved(); + } else { + prepare.fail('preparation_failed', 'Could not send your code'); + expect(await screen.findByText('Could not send your code')).toBeVisible(); + expect(attempt.requests).toHaveLength(0); + expect(screen.getByRole('button', { name: /Resend/ })).toBeEnabled(); + attempt.release(); + } + expect(prepare.requests).toHaveLength(1); + }, + ); + + it('drops a held submit when the user opens the picker', async () => { + const { prepare, user } = await startSendingCode(); + const attempt = holdRequests('post', ATTEMPT_FIRST); + await pasteCode(user, CODE); + await useAnotherMethod(user); + + prepare.release(); + await settle(); + expect(screen.getByRole('button', { name: 'Continue with your password' })).toBeVisible(); + expect(attempt.requests).toHaveLength(0); + + // The typed code is still there on return + await back(user); + expect(await screen.findByText(STEP.emailCode)).toBeVisible(); + expect(slotValues()).toEqual(CODE.split('')); + expect(attempt.requests).toHaveLength(0); + expect(prepare.requests).toHaveLength(1); + attempt.release(); + }); + + it('ignores a late failure after the user picks the password', async () => { + const { prepare, user } = await startSendingCode(); + await useAnotherMethod(user); + await user.click(await screen.findByRole('button', { name: 'Continue with your password' })); + expect(await screen.findByText(STEP.password)).toBeVisible(); + + prepare.fail('preparation_failed', 'Could not send your code'); + await settle(); + expect(screen.getByText(STEP.password)).toBeVisible(); + expect(screen.queryByText('Could not send your code')).toBeNull(); + }); + + it('ignores a late failure after the user picks another email address', async () => { + const { prepare, user } = await startSendingCode([emailFactor, workEmailFactor]); + 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)); + + prepare.fail('preparation_failed', 'Could not send your code'); + await settle(); + expect(screen.getByRole('button', { name: 'Email code to alice@work.example' })).toBeVisible(); + expect(screen.queryByText('Could not send your code')).toBeNull(); + expect(prepare.requests).toHaveLength(1); + expect(second.requests).toHaveLength(1); + second.release(); + }); + + it('ignores a late failure after the challenge is dismissed from the picker and run again', async () => { + const action = vi + .fn() + .mockResolvedValueOnce(reverificationError()) + .mockResolvedValueOnce(reverificationError()); + const { prepare, user } = await startSendingCode(undefined, action); + await useAnotherMethod(user); + await dismiss(user); + await waitFor(() => expect(outcome()).toBe('rejected: reverification_cancelled')); + await untilClosed(); + + 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)); + + prepare.fail('preparation_failed', 'Could not send your code'); + await settle(); + expect(screen.getByText(STEP.emailCode)).toBeVisible(); + expect(screen.queryByText('Could not send your code')).toBeNull(); + expect(screen.getByRole('button', { name: /Resend \(\d+\)/ })).toBeDisabled(); + expect(prepare.requests).toHaveLength(1); + next.release(); + }); + }); + it('resends a code once the cooldown has passed', async () => { const { fapi } = await renderReverification(guardedAction(), verificationSeed({ firstFactors: [emailFactor] })); vi.useFakeTimers({ toFake: ['Date', 'setInterval', 'clearInterval'] }); diff --git a/packages/mosaic/src/features/reverification/reverification.controller.ts b/packages/mosaic/src/features/reverification/reverification.controller.ts index 46665d6216a..d50f5d013a6 100644 --- a/packages/mosaic/src/features/reverification/reverification.controller.ts +++ b/packages/mosaic/src/features/reverification/reverification.controller.ts @@ -35,7 +35,7 @@ export type ReverificationController = ReverificationControllerState & { visible: boolean; }; -type OverlayFrom = 'factor' | 'method-picker'; +type Overlay = Extract; export type ReverificationDeps = Pick; @@ -48,7 +48,7 @@ interface ReverificationContext { resendAvailableAt: number | undefined; submitRequested: boolean; frozenActiveMethodId: string | undefined; - overlayFrom: OverlayFrom; + overlay: readonly Overlay[]; supportEmail: string; deps: ReverificationDeps; } @@ -144,28 +144,51 @@ const afterResult = [ { target: 'verifying' as const, actions: applyResult }, ]; +function topOverlay(ctx: ReverificationContext): Overlay | undefined { + return ctx.overlay[ctx.overlay.length - 1]; +} + const factorEvents = { TYPE: { actions: assign((_, event) => ({ inputValue: event.value, errors: [] })), }, SHOW_METHODS: { - target: 'methodPicker', - guard: ctx => ctx.methods.filter(method => method.id !== ctx.activeMethod?.id).length > 0, + guard: ctx => + ctx.overlay.length === 0 && ctx.methods.filter(method => method.id !== ctx.activeMethod?.id).length > 0, actions: assign(ctx => ({ + overlay: ['method-picker' as const], direction: 1 as const, - overlayFrom: 'factor' as const, submitRequested: false, frozenActiveMethodId: ctx.activeMethod?.id, })), }, SHOW_HELP: { - target: 'help', - actions: assign(() => ({ + guard: ctx => topOverlay(ctx) !== 'help', + actions: assign(ctx => ({ + overlay: [...ctx.overlay, 'help' as const], direction: 1 as const, - overlayFrom: 'factor' as const, submitRequested: false, })), }, + BACK: { + 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)); + }, + 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', } satisfies NonNullable['on']>; @@ -181,7 +204,7 @@ export const reverificationMachine = createMachine({ resendAvailableAt: undefined, submitRequested: false, frozenActiveMethodId: undefined, - overlayFrom: 'factor', + overlay: [], supportEmail: '', deps: unseatedDeps, }, @@ -191,6 +214,7 @@ export const reverificationMachine = createMachine({ inputValue: '', errors: [], submitRequested: false, + overlay: [], ...unlockResend(), })), on: { START: 'starting' }, @@ -214,6 +238,7 @@ export const reverificationMachine = createMachine({ on: { ...factorEvents, SUBMIT: { + guard: ctx => ctx.overlay.length === 0, actions: assign(() => ({ submitRequested: true })), }, }, @@ -247,10 +272,10 @@ export const reverificationMachine = createMachine({ entry: assign(() => lockResend()), on: { RESET: 'inactive' }, invoke: fromPromise(prepareActive, { - onDone: 'verifying', + onDone: { target: 'verifying', actions: assign(() => ({ overlay: [] })) }, onError: { target: 'verifying', - actions: assign((_, event) => ({ errors: toLocalizableErrors(event.error), ...unlockResend() })), + actions: assign((_, event) => ({ errors: toLocalizableErrors(event.error), overlay: [], ...unlockResend() })), }, }), }, @@ -259,10 +284,11 @@ export const reverificationMachine = createMachine({ always: [{ guard: ctx => ctx.activeMethod === null, target: 'unavailable' }], on: { ...factorEvents, - SUBMIT: 'submitting', + SUBMIT: { target: 'submitting', guard: ctx => ctx.overlay.length === 0 }, RESEND: { target: 'preparing', guard: ctx => + ctx.overlay.length === 0 && Boolean(ctx.activeMethod && needsPrepare(ctx.activeMethod)) && (ctx.resendAvailableAt === undefined || Date.now() >= ctx.resendAvailableAt), actions: assign(() => ({ @@ -293,42 +319,6 @@ export const reverificationMachine = createMachine({ }), }, - methodPicker: { - on: { - SELECT_METHOD: [ - { - target: 'methodPickerPreparing', - guard: (ctx, event) => { - const method = ctx.methods.find(candidate => candidate.id === event.id); - return Boolean(method && needsPrepare(method)); - }, - actions: assign((ctx, event) => selectMethod(ctx, event.id)), - }, - { target: 'verifying', actions: assign((ctx, event) => selectMethod(ctx, event.id)) }, - ], - SHOW_HELP: { - target: 'help', - actions: assign(() => ({ direction: 1 as const, overlayFrom: 'method-picker' as const })), - }, - BACK: { target: 'verifying', actions: assign(() => ({ direction: -1 as const })) }, - RESET: 'inactive', - }, - }, - - help: { - on: { - BACK: [ - { - target: 'methodPicker', - guard: ctx => ctx.overlayFrom === 'method-picker', - actions: assign(() => ({ direction: -1 as const })), - }, - { target: 'verifying', actions: assign(() => ({ direction: -1 as const })) }, - ], - RESET: 'inactive', - }, - }, - unavailable: { on: { RESET: 'inactive' }, }, @@ -377,12 +367,13 @@ function factorStep(method: ReverificationMethod): ReverificationStep { return 'otp'; } -function viewStep(value: string, method: ReverificationMethod | null): ReverificationStep | undefined { - if (value === 'methodPicker' || value === 'methodPickerPreparing') { - return 'method-picker'; - } - if (value === 'help') { - return 'help'; +function viewStep( + value: string, + overlay: Overlay | undefined, + method: ReverificationMethod | null, +): ReverificationStep | undefined { + if (overlay) { + return overlay; } if ( value === 'verifying' || @@ -493,7 +484,7 @@ function useReverificationState(model: ReverificationModel): ReverificationContr return { status: 'error', reason: 'generic', onCancel }; } - const step = viewStep(state, activeMethod); + const step = viewStep(state, topOverlay(context), activeMethod); if (!step) { return { status: 'loading', onCancel }; diff --git a/packages/mosaic/vitest.setup.browser.mts b/packages/mosaic/vitest.setup.browser.mts index 9688e433d04..ae98521db28 100644 --- a/packages/mosaic/vitest.setup.browser.mts +++ b/packages/mosaic/vitest.setup.browser.mts @@ -64,15 +64,21 @@ beforeEach(() => { window.focus(); }); +function clearCookies() { + for (const cookie of document.cookie.split('; ').filter(Boolean)) { + document.cookie = `${cookie.split('=')[0]}=; max-age=0; path=/`; + } +} + +window.addEventListener('pagehide', clearCookies); + afterEach(() => { cleanup(); __resetClerkQueryClientForTest(); for (const channel of [...openChannels]) { channel.close(); } - for (const cookie of document.cookie.split('; ').filter(Boolean)) { - document.cookie = `${cookie.split('=')[0]}=; max-age=0; path=/`; - } + clearCookies(); localStorage.clear(); sessionStorage.clear(); worker.resetHandlers();