feat(mosaic): reverify SMS and authenticator MFA setup

.changeset/mfa-setup-reverification.md

@@ -0,0 +1,2 @@
+---
+---

packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-interactions.feature.test.tsx

@@ -92,22 +92,23 @@ describe('User profile MFA interaction boundaries', () => {
     expect(await screen.findByRole('img', { name: /QR code/i })).toBeVisible();
   });
 
-  it('keeps the dialog closed when dismissed while authenticator preparation is pending', async () => {
+  it('keeps the dialog open until authenticator preparation responds', async () => {
     const fapi = await renderMfa();
     const held = holdRequests('post', '/v1/me/totp');
     const user = userEvent.setup();
     await user.click(screen.getByRole('button', { name: 'Add verification method' }));
     await user.click(screen.getByRole('button', { name: /Authenticator app/ }));
     await waitFor(() => expect(held.requests).toHaveLength(1));
-    await user.keyboard('{Escape}');
-    await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull());
-    held.release();
-    await waitFor(() => expect(fapi.mfa.totpCreations).toBe(1));
-    expect(screen.queryByRole('dialog')).toBeNull();
-    await user.click(screen.getByRole('button', { name: 'Add verification method' }));
-    expect(await screen.findByRole('dialog', { name: 'Add 2-step verification' })).toHaveAccessibleDescription(
-      'Choose a verification method',
-    );
+    const dialog = screen.getByRole('dialog');
+    try {
+      expect(within(dialog).queryByRole('button', { name: 'Close' })).toBeNull();
+      await user.keyboard('{Escape}');
+      expect(dialog).toBeVisible();
+    } finally {
+      held.release();
+    }
+    expect(await screen.findByRole('img', { name: /QR code/i })).toBeVisible();
+    expect(fapi.mfa.totpCreations).toBe(1);
   });
 
   it('shows backup codes when dismissed while authenticator verification is pending', async () => {

packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx

@@ -9,6 +9,7 @@ import {
   fapiUrl,
   holdRequests,
   serveFapi,
+  VERIFICATION_CODE,
   worker,
 } from '../../../../__tests__/feature/fake-fapi';
 import { fapiClient, fapiPhoneNumber, fapiSession, fapiUser } from '../../../../__tests__/feature/fapi';
@@ -370,9 +371,10 @@ describe('User profile MFA management', () => {
     }
     expect(screen.getByRole('button', { name: 'Copy and close' })).toBeVisible();
     expect(screen.getByRole('button', { name: 'Copy and close' })).toHaveAttribute('aria-disabled', 'true');
-    await waitFor(() => expect(screen.getByRole('button', { name: 'Close' })).toHaveFocus());
+    expect(screen.queryByRole('button', { name: 'Close' })).toBeNull();
     held.release();
     await waitFor(() => expect(screen.getByText('CODE0100')).toBeVisible());
+    expect(screen.getByRole('button', { name: 'Close' })).toBeVisible();
     for (const name of ['Print', 'Download', 'Copy and close']) {
       expect(screen.getByRole('button', { name })).toBeEnabled();
     }
@@ -481,16 +483,144 @@ describe('User profile MFA management', () => {
     await regenerate(user);
     await waitFor(() => expect(first.requests).toHaveLength(1));
     const dialog = screen.getByRole('dialog');
-    await user.keyboard('{Escape}');
-    expect(dialog).toBeVisible();
-    first.release();
+    try {
+      expect(within(dialog).queryByRole('button', { name: 'Close' })).toBeNull();
+      await user.keyboard('{Escape}');
+      expect(dialog).toBeVisible();
+    } finally {
+      first.release();
+    }
 
     expect(await within(dialog).findByText('Enter your current password to continue')).toBeVisible();
     await user.type(screen.getByLabelText('Password'), 'hunter2{Enter}');
     await screen.findByRole('list', { name: 'Backup codes' });
     expect(fapi.mfa.backupCodeCreations).toBe(1);
   });
 
+  const startSms = async (user: ReturnType<typeof userEvent.setup>) => {
+    await user.click(screen.getByRole('button', { name: 'Add verification method' }));
+    await user.click(screen.getByRole('button', { name: /SMS verification/ }));
+  };
+
+  it('verifies the session before reserving an existing phone for SMS MFA', async () => {
+    const fapi = await renderMfa(undefined, mfaEnvironment(), { verification: { secrets: { password: 'hunter2' } } });
+    demandReverificationOnce('/v1/me/phone_numbers/phone_1');
+    const user = userEvent.setup();
+
+    await startSms(user);
+    await user.click(screen.getByRole('button', { name: 'Continue' }));
+    expect(await screen.findByText('Enter your current password to continue')).toBeVisible();
+    await waitFor(() => expect(screen.getByLabelText('Password')).toHaveFocus());
+    await user.keyboard('hunter2{Enter}');
+
+    expect(await screen.findByText('CODE0001')).toBeVisible();
+    expect(fapi.mfa.phoneUpdates).toContainEqual({ id: 'phone_1', reserved: true, default: undefined });
+  });
+
+  it('keeps SMS code verification open until its reservation challenge can be cancelled', async () => {
+    const unverified = fapiPhoneNumber({ ...phone, verification: { ...phone.verification, status: 'unverified' } });
+    const fapi = await renderMfa(fapiUser({ id: 'user_1', phone_numbers: [unverified] }), mfaEnvironment(), {
+      verification: { secrets: { password: 'hunter2' } },
+    });
+    demandReverificationOnce('/v1/me/phone_numbers/phone_1');
+    const user = userEvent.setup();
+
+    await startSms(user);
+    await user.click(screen.getByRole('button', { name: 'Continue' }));
+    const code = await screen.findByRole('textbox', { name: 'Verification code' });
+    const attempt = holdRequests('post', '/v1/me/phone_numbers/phone_1/attempt_verification');
+    await user.type(code, VERIFICATION_CODE);
+    await waitFor(() => expect(attempt.requests).toHaveLength(1));
+    const dialog = screen.getByRole('dialog');
+    try {
+      expect(within(dialog).queryByRole('button', { name: 'Close' })).toBeNull();
+      await user.keyboard('{Escape}');
+      expect(dialog).toBeVisible();
+    } finally {
+      attempt.release();
+    }
+
+    expect(await within(dialog).findByText('Enter your current password to continue')).toBeVisible();
+    await user.click(within(dialog).getByRole('button', { name: 'Close' }));
+    await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull());
+    expect(fapi.mfa.phoneAttempts).toEqual([{ id: 'phone_1', code: VERIFICATION_CODE }]);
+    expect(fapi.mfa.phoneUpdates).toEqual([]);
+    expect(screen.queryByRole('alert')).toBeNull();
+  });
+
+  it('verifies the session before creating a phone for SMS MFA, then sends its code', async () => {
+    const fapi = await renderMfa(fapiUser({ id: 'user_1' }), mfaEnvironment(), {
+      verification: { secrets: { password: 'hunter2' } },
+    });
+    demandReverificationOnce('/v1/me/phone_numbers');
+    const user = userEvent.setup();
+
+    await startSms(user);
+    await user.type(screen.getByRole('textbox', { name: 'Phone' }), '5555550303');
+    await user.click(screen.getByRole('button', { name: 'Send code' }));
+    expect(await screen.findByText('Enter your current password to continue')).toBeVisible();
+    await waitFor(() => expect(screen.getByLabelText('Password')).toHaveFocus());
+    await user.keyboard('hunter2{Enter}');
+
+    await waitFor(() => expect(fapi.mfa.phonePreparations).toHaveLength(1));
+    expect(fapi.mfa.phoneCreations).toHaveLength(1);
+    expect(await screen.findByRole('button', { name: 'Verify' })).toBeVisible();
+  });
+
+  it('lets the user cancel SMS MFA verification quietly and add the method again', async () => {
+    const fapi = await renderMfa(undefined, mfaEnvironment(), { verification: { secrets: { password: 'hunter2' } } });
+    demandReverificationOnce('/v1/me/phone_numbers/phone_1');
+    const user = userEvent.setup();
+
+    await startSms(user);
+    await user.click(screen.getByRole('button', { name: 'Continue' }));
+    await screen.findByText('Enter your current password to continue');
+    await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Close' }));
+
+    await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull());
+    expect(fapi.mfa.phoneUpdates).toEqual([]);
+    expect(screen.queryByRole('alert')).toBeNull();
+
+    await startSms(user);
+    await user.click(screen.getByRole('button', { name: 'Continue' }));
+    expect(await screen.findByText('CODE0001')).toBeVisible();
+  });
+
+  it('verifies the session before starting authenticator setup', async () => {
+    const fapi = await renderMfa(undefined, mfaEnvironment(), { verification: { secrets: { password: 'hunter2' } } });
+    demandReverificationOnce('/v1/me/totp');
+    const user = userEvent.setup();
+
+    await user.click(screen.getByRole('button', { name: 'Add verification method' }));
+    await user.click(screen.getByRole('button', { name: /Authenticator app/ }));
+    expect(await screen.findByText('Enter your current password to continue')).toBeVisible();
+    await waitFor(() => expect(screen.getByLabelText('Password')).toHaveFocus());
+    await user.keyboard('hunter2{Enter}');
+
+    expect(await screen.findByRole('img', { name: /QR code/i })).toBeVisible();
+    expect(fapi.mfa.totpCreations).toBe(1);
+  });
+
+  it('lets the user cancel authenticator verification quietly and start setup again', async () => {
+    const fapi = await renderMfa(undefined, mfaEnvironment(), { verification: { secrets: { password: 'hunter2' } } });
+    demandReverificationOnce('/v1/me/totp');
+    const user = userEvent.setup();
+
+    await user.click(screen.getByRole('button', { name: 'Add verification method' }));
+    await user.click(screen.getByRole('button', { name: /Authenticator app/ }));
+    await screen.findByText('Enter your current password to continue');
+    await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Close' }));
+
+    await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull());
+    expect(fapi.mfa.totpCreations).toBe(0);
+    expect(screen.queryByRole('alert')).toBeNull();
+
+    await user.click(screen.getByRole('button', { name: 'Add verification method' }));
+    await user.click(screen.getByRole('button', { name: /Authenticator app/ }));
+    expect(await screen.findByRole('img', { name: /QR code/i })).toBeVisible();
+    expect(fapi.mfa.totpCreations).toBe(1);
+  });
+
   it('copies server-issued backup codes and closes their one-time surface', async () => {
     const fapi = await renderMfa();
     const write = vi.spyOn(navigator.clipboard, 'writeText').mockResolvedValue();

packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts

@@ -6,6 +6,7 @@ import { setup } from '../../../machine/setup';
 import type { DoneInvokeEvent } from '../../../machine/types';
 import { useMachine } from '../../../machine/use-machine';
 import { toLocalizableError } from '../../../utils/errors';
+import type { ReverificationController } from '../../reverification';
 import type { UserProfileMfaAddableMethod, UserProfileMfaModel } from './user-profile-mfa-section.types';
 import type { UserProfileMfaSectionViewProps } from './user-profile-mfa-section.view';
 import type { UserProfileMfaSetupViewProps } from './user-profile-mfa-setup.view';
@@ -61,6 +62,7 @@ type MfaEvent =
 
 const { createMachine, assign, fromPromise } = setup<MfaContext, MfaEvent>();
 const RESEND_COOLDOWN_SECONDS = 30;
+const REVERIFIABLE_ACTIONS: readonly MfaAction[] = ['generate', 'sms', 'prepareAuthenticator'];
 const tick = { actions: assign(context => ({ resendSeconds: Math.max(0, context.resendSeconds - 1) })) };
 
 const mfaMachine = createMachine({
@@ -413,21 +415,27 @@ function createSetupProps(
   send: MfaSend,
   actions: MfaSetupActions,
   errorText: ReturnType<typeof useErrorText>,
+  reverification: ReverificationController,
 ): UserProfileMfaSetupViewProps {
   const flow = context.flow;
   const sms = flow.kind === 'sms' ? flow : undefined;
   const authenticator = flow.kind === 'authenticator' ? flow : undefined;
   const backup = flow.kind === 'backup' ? flow : undefined;
+  const isReverifying =
+    context.pending !== undefined && REVERIFIABLE_ACTIONS.includes(context.pending) && reverification.visible;
+  let step: UserProfileMfaSetupViewProps['step'];
+  if (isReverifying) {
+    step = 'verify';
+  } else if (flow.kind === 'closed') {
+    step = 'select';
+  } else if (flow.kind === 'backup') {
+    step = 'backup-codes';
+  } else {
+    step = flow.kind;
+  }
   return {
-    step:
-      flow.kind === 'closed'
-        ? 'select'
-        : flow.kind === 'backup'
-          ? context.pending === 'generate' && model.backupCodesReverification.visible
-            ? 'verify'
-            : 'backup-codes'
-          : flow.kind,
-    reverification: model.backupCodesReverification,
+    step,
+    reverification,
     methods: model.addableMethods,
     onSelect: actions.select,
     onBack: actions.back,
@@ -521,6 +529,11 @@ export function useUserProfileMfaController(model: ReadyModel) {
     }
   };
 
+  // The machine guarantees that only one is running/visible at a time
+  const reverification =
+    [model.createPhoneReverification, model.enrollSmsReverification, model.authenticatorReverification].find(
+      controller => controller.visible,
+    ) ?? model.backupCodesReverification;
   const setupProps = createSetupProps(
     model,
     context,
@@ -539,6 +552,7 @@ export function useUserProfileMfaController(model: ReadyModel) {
       print,
     },
     errorText,
+    reverification,
   );
 
   const sectionProps: UserProfileMfaSectionViewProps = {
@@ -552,24 +566,25 @@ export function useUserProfileMfaController(model: ReadyModel) {
   };
 
   const showAddTrigger = model.addableMethods.length > 0;
-  const reverification = model.backupCodesReverification;
   const reverifying = setupProps.step === 'verify';
+  const hasPendingReverification = context.pending !== undefined && REVERIFIABLE_ACTIONS.includes(context.pending);
+  const canClose = !hasPendingReverification || Boolean(reverification.onCancel);
 
   return {
     dialogOpen,
     showAddTrigger,
     showAddControl: showAddTrigger || dialogOpen,
     reverifying,
     onDialogOpenChange: (open: boolean) => {
-      if (!open && context.pending === 'generate') {
-        if (!reverification.onCancel) {
-          return;
-        }
-        reverification.onCancel();
+      if (!open && !canClose) {
+        return;
+      }
+      if (!open && hasPendingReverification) {
+        reverification.onCancel?.();
       }
       send({ type: open ? 'OPEN' : 'CLOSE' });
     },
-    showCloseButton: !reverifying || Boolean(reverification.onCancel),
+    showCloseButton: canClose,
     sectionProps,
     setupProps,
   };

packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.model.ts

@@ -112,7 +112,11 @@ function createSmsActions(
   userId: string,
   sessionId: string,
   refresh: () => Promise<void>,
-  makeDefault: (phone: PhoneNumberResource) => Promise<unknown>,
+  calls: {
+    makeDefault: (phone: PhoneNumberResource) => Promise<unknown>;
+    createPhone: (current: UserResource, phoneNumber: string) => Promise<PhoneNumberResource>;
+    reservePhone: (phone: PhoneNumberResource) => Promise<PhoneNumberResource>;
+  },
 ): Pick<ReadyModel, 'findOrCreatePhone' | 'enrollSms' | 'resendSms' | 'setDefault'> {
   const currentPhone = (phoneId: string) => {
     const current = requireIdentity(clerk, userId, sessionId);
@@ -129,7 +133,7 @@ function createSmsActions(
       const existing = current.phoneNumbers.find(
         phone => !phone.reservedForSecondFactor && phone.phoneNumber === phoneNumber,
       );
-      return summarizePhone(existing ?? (await current.createPhoneNumber({ phoneNumber })));
+      return summarizePhone(existing ?? (await calls.createPhone(current, phoneNumber)));
     },
     enrollSms: async (phoneId, code): Promise<SmsEnrollmentResult> => {
       let phone = currentPhone(phoneId);
@@ -144,7 +148,7 @@ function createSmsActions(
       if (phone.reservedForSecondFactor || !configuredFactors(environment).includes('phone_code')) {
         throw phoneUnavailable();
       }
-      const result = await phone.setReservedForSecondFactor({ reserved: true });
+      const result = await calls.reservePhone(phone);
       await refresh().catch(() => undefined);
       return { status: 'complete', backupCodes: result.backupCodes ?? [] };
     },
@@ -156,7 +160,7 @@ function createSmsActions(
       if (phone.verification.status !== 'verified' || !phone.reservedForSecondFactor) {
         throw phoneUnavailable();
       }
-      await makeDefault(phone);
+      await calls.makeDefault(phone);
       await refresh().catch(() => undefined);
     },
   };
@@ -196,6 +200,15 @@ export function useUserProfileMfaModel(): UserProfileMfaModel {
   const [createBackupCode, backupCodesReverification] = useReverificationFlow((current: UserResource) =>
     current.createBackupCode(),
   );
+  const [createPhone, createPhoneReverification] = useReverificationFlow((current: UserResource, phoneNumber: string) =>
+    current.createPhoneNumber({ phoneNumber }),
+  );
+  const [reservePhone, enrollSmsReverification] = useReverificationFlow((phone: PhoneNumberResource) =>
+    phone.setReservedForSecondFactor({ reserved: true }),
+  );
+  const [createTotp, authenticatorReverification] = useReverificationFlow((current: UserResource) =>
+    current.createTOTP(),
+  );
 
   if (!userLoaded || !sessionLoaded || !environment) {
     return { status: 'loading' };
@@ -221,7 +234,9 @@ export function useUserProfileMfaModel(): UserProfileMfaModel {
     await current.reload();
   };
 
-  const smsActions = smsEnabled ? createSmsActions(clerk, environment, userId, sessionId, refresh, makeDefault) : {};
+  const smsActions = smsEnabled
+    ? createSmsActions(clerk, environment, userId, sessionId, refresh, { makeDefault, createPhone, reservePhone })
+    : {};
 
   return {
     status: 'ready',
@@ -239,7 +254,7 @@ export function useUserProfileMfaModel(): UserProfileMfaModel {
           if (current.totpEnabled || !configuredFactors(environment).includes('totp')) {
             throw new ClerkRuntimeError('Authenticator is unavailable.', { code: 'mfa_authenticator_unavailable' });
           }
-          const result = await current.createTOTP();
+          const result = await createTotp(current);
           return { secret: result.secret ?? '', uri: result.uri ?? '' };
         }
       : undefined,
@@ -268,5 +283,8 @@ export function useUserProfileMfaModel(): UserProfileMfaModel {
     removeReverification,
     setDefaultReverification,
     backupCodesReverification,
+    createPhoneReverification,
+    enrollSmsReverification,
+    authenticatorReverification,
   };
 }