fix(mosaic): align MFA dialog close controls with pending actions

Author: austincalvelage

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

@@ -100,9 +100,13 @@ describe('User profile MFA interaction boundaries', () => {
     await user.click(screen.getByRole('button', { name: /Authenticator app/ }));
     await waitFor(() => expect(held.requests).toHaveLength(1));
     const dialog = screen.getByRole('dialog');
-    await user.keyboard('{Escape}');
-    expect(dialog).toBeVisible();
-    held.release();
+    try {
+      expect(within(dialog).queryByRole('button', { name: 'Close' })).toBeNull();
+      await user.keyboard('{Escape}');
+      expect(dialog).toBeVisible();
+    } finally {
+      held.release();
+    }
     expect(await screen.findByRole('img', { name: /QR code/i })).toBeVisible();
     expect(fapi.mfa.totpCreations).toBe(1);
   });

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

@@ -9,6 +9,7 @@ import {
   fapiUrl,
   holdRequests,
   serveFapi,
+  VERIFICATION_CODE,
   worker,
 } from '../../../../__tests__/feature/fake-fapi';
 import { fapiClient, fapiPhoneNumber, fapiSession, fapiUser } from '../../../../__tests__/feature/fapi';
@@ -370,9 +371,10 @@ describe('User profile MFA management', () => {
     }
     expect(screen.getByRole('button', { name: 'Copy and close' })).toBeVisible();
     expect(screen.getByRole('button', { name: 'Copy and close' })).toHaveAttribute('aria-disabled', 'true');
-    await waitFor(() => expect(screen.getByRole('button', { name: 'Close' })).toHaveFocus());
+    expect(screen.queryByRole('button', { name: 'Close' })).toBeNull();
     held.release();
     await waitFor(() => expect(screen.getByText('CODE0100')).toBeVisible());
+    expect(screen.getByRole('button', { name: 'Close' })).toBeVisible();
     for (const name of ['Print', 'Download', 'Copy and close']) {
       expect(screen.getByRole('button', { name })).toBeEnabled();
     }
@@ -481,9 +483,13 @@ describe('User profile MFA management', () => {
     await regenerate(user);
     await waitFor(() => expect(first.requests).toHaveLength(1));
     const dialog = screen.getByRole('dialog');
-    await user.keyboard('{Escape}');
-    expect(dialog).toBeVisible();
-    first.release();
+    try {
+      expect(within(dialog).queryByRole('button', { name: 'Close' })).toBeNull();
+      await user.keyboard('{Escape}');
+      expect(dialog).toBeVisible();
+    } finally {
+      first.release();
+    }
 
     expect(await within(dialog).findByText('Enter your current password to continue')).toBeVisible();
     await user.type(screen.getByLabelText('Password'), 'hunter2{Enter}');
@@ -511,6 +517,37 @@ describe('User profile MFA management', () => {
     expect(fapi.mfa.phoneUpdates).toContainEqual({ id: 'phone_1', reserved: true, default: undefined });
   });
 
+  it('keeps SMS code verification open until its reservation challenge can be cancelled', async () => {
+    const unverified = fapiPhoneNumber({ ...phone, verification: { ...phone.verification, status: 'unverified' } });
+    const fapi = await renderMfa(fapiUser({ id: 'user_1', phone_numbers: [unverified] }), mfaEnvironment(), {
+      verification: { secrets: { password: 'hunter2' } },
+    });
+    demandReverificationOnce('/v1/me/phone_numbers/phone_1');
+    const user = userEvent.setup();
+
+    await startSms(user);
+    await user.click(screen.getByRole('button', { name: 'Continue' }));
+    const code = await screen.findByRole('textbox', { name: 'Verification code' });
+    const attempt = holdRequests('post', '/v1/me/phone_numbers/phone_1/attempt_verification');
+    await user.type(code, VERIFICATION_CODE);
+    await waitFor(() => expect(attempt.requests).toHaveLength(1));
+    const dialog = screen.getByRole('dialog');
+    try {
+      expect(within(dialog).queryByRole('button', { name: 'Close' })).toBeNull();
+      await user.keyboard('{Escape}');
+      expect(dialog).toBeVisible();
+    } finally {
+      attempt.release();
+    }
+
+    expect(await within(dialog).findByText('Enter your current password to continue')).toBeVisible();
+    await user.click(within(dialog).getByRole('button', { name: 'Close' }));
+    await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull());
+    expect(fapi.mfa.phoneAttempts).toEqual([{ id: 'phone_1', code: VERIFICATION_CODE }]);
+    expect(fapi.mfa.phoneUpdates).toEqual([]);
+    expect(screen.queryByRole('alert')).toBeNull();
+  });
+
   it('verifies the session before creating a phone for SMS MFA, then sends its code', async () => {
     const fapi = await renderMfa(fapiUser({ id: 'user_1' }), mfaEnvironment(), {
       verification: { secrets: { password: 'hunter2' } },

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

@@ -62,7 +62,7 @@ type MfaEvent =
 
 const { createMachine, assign, fromPromise } = setup<MfaContext, MfaEvent>();
 const RESEND_COOLDOWN_SECONDS = 30;
-const REVERIFIABLE_ACTIONS: readonly (MfaAction | undefined)[] = ['generate', 'sms', 'prepareAuthenticator'];
+const REVERIFIABLE_ACTIONS: readonly MfaAction[] = ['generate', 'sms', 'prepareAuthenticator'];
 const tick = { actions: assign(context => ({ resendSeconds: Math.max(0, context.resendSeconds - 1) })) };
 
 const mfaMachine = createMachine({
@@ -421,15 +421,20 @@ function createSetupProps(
   const sms = flow.kind === 'sms' ? flow : undefined;
   const authenticator = flow.kind === 'authenticator' ? flow : undefined;
   const backup = flow.kind === 'backup' ? flow : undefined;
-  const isReverifying = REVERIFIABLE_ACTIONS.includes(context.pending) && reverification.visible;
+  const isReverifying =
+    context.pending !== undefined && REVERIFIABLE_ACTIONS.includes(context.pending) && reverification.visible;
+  let step: UserProfileMfaSetupViewProps['step'];
+  if (isReverifying) {
+    step = 'verify';
+  } else if (flow.kind === 'closed') {
+    step = 'select';
+  } else if (flow.kind === 'backup') {
+    step = 'backup-codes';
+  } else {
+    step = flow.kind;
+  }
   return {
-    step: isReverifying
-      ? 'verify'
-      : flow.kind === 'closed'
-        ? 'select'
-        : flow.kind === 'backup'
-          ? 'backup-codes'
-          : flow.kind,
+    step,
     reverification,
     methods: model.addableMethods,
     onSelect: actions.select,
@@ -561,22 +566,24 @@ export function useUserProfileMfaController(model: ReadyModel) {
 
   const showAddTrigger = model.addableMethods.length > 0;
   const reverifying = setupProps.step === 'verify';
+  const hasPendingReverification = context.pending !== undefined && REVERIFIABLE_ACTIONS.includes(context.pending);
+  const canClose = !hasPendingReverification || Boolean(reverification.onCancel);
 
   return {
     dialogOpen,
     showAddTrigger,
     showAddControl: showAddTrigger || dialogOpen,
     reverifying,
     onDialogOpenChange: (open: boolean) => {
-      if (!open && REVERIFIABLE_ACTIONS.includes(context.pending)) {
-        if (!reverification.onCancel) {
-          return;
-        }
-        reverification.onCancel();
+      if (!open && !canClose) {
+        return;
+      }
+      if (!open && hasPendingReverification) {
+        reverification.onCancel?.();
       }
       send({ type: open ? 'OPEN' : 'CLOSE' });
     },
-    showCloseButton: !reverifying || Boolean(reverification.onCancel),
+    showCloseButton: canClose,
     sectionProps,
     setupProps,
   };