Merge pull request #19469 from mozilla/FXA-12225

vpomerleau · web-flow · commit 517913247b53 · 2025-09-22T21:49:30.000-07:00
task(settings) Wrap add recovery phone and replace recovery with MFA guard
diff --git a/packages/functional-tests/tests/resetPassword/resetPassword2FA.spec.ts b/packages/functional-tests/tests/resetPassword/resetPassword2FA.spec.ts
@@ -418,6 +418,8 @@ test.describe('reset password with recovery phone', () => {
     await settings.totp.addRecoveryPhoneButton.click();
     await page.waitForURL(/recovery_phone\/setup/);
 
+    await settings.confirmMfaGuard(credentials.email);
+
     await expect(recoveryPhone.addHeader()).toBeVisible();
 
     await recoveryPhone.enterPhoneNumber(testNumber);
@@ -504,6 +506,8 @@ test.describe('reset password with recovery phone', () => {
     await settings.totp.addRecoveryPhoneButton.click();
     await page.waitForURL(/recovery_phone\/setup/);
 
+    await settings.confirmMfaGuard(credentials.email);
+
     await expect(recoveryPhone.addHeader()).toBeVisible();
 
     await recoveryPhone.enterPhoneNumber(testNumber);
diff --git a/packages/functional-tests/tests/settings/recoveryPhone.spec.ts b/packages/functional-tests/tests/settings/recoveryPhone.spec.ts
@@ -37,13 +37,12 @@ test.describe('severity-1 #smoke', () => {
       await signInAccount(target, page, settings, signin, credentials);
 
       await settings.goto();
-      const totpCredentials = await setup2faWithBackupCodeChoice(
-        settings,
-        totp
-      );
+      await setup2faWithBackupCodeChoice(settings, totp);
       await expect(settings.totp.status).toHaveText('Enabled');
       await settings.totp.addRecoveryPhoneButton.click();
 
+      await settings.confirmMfaGuard(credentials.email);
+
       await expect(recoveryPhone.addHeader()).toBeVisible();
 
       await recoveryPhone.enterPhoneNumber('1234567890');
@@ -65,13 +64,12 @@ test.describe('severity-1 #smoke', () => {
       await signInAccount(target, page, settings, signin, credentials);
 
       await settings.goto();
-      const totpCredentials = await setup2faWithBackupCodeChoice(
-        settings,
-        totp
-      );
+      await setup2faWithBackupCodeChoice(settings, totp);
       await expect(settings.totp.status).toHaveText('Enabled');
       await settings.totp.addRecoveryPhoneButton.click();
 
+      await settings.confirmMfaGuard(credentials.email);
+
       await expect(recoveryPhone.addHeader()).toBeVisible();
 
       await recoveryPhone.enterPhoneNumber(testNumber);
@@ -119,10 +117,7 @@ test.describe('severity-1 #smoke', () => {
       await signInAccount(target, page, settings, signin, credentials);
 
       await settings.goto();
-      const totpCredentials = await setup2faWithBackupCodeChoice(
-        settings,
-        totp
-      );
+      await setup2faWithBackupCodeChoice(settings, totp);
       await expect(settings.totp.status).toHaveText('Enabled');
       await addRecoveryPhone(
         settings,
@@ -744,6 +739,8 @@ async function addRecoveryPhone(
   await settings.totp.addRecoveryPhoneButton.click();
   await page.waitForURL(/recovery_phone\/setup/);
 
+  await settings.confirmMfaGuard(credentials.email);
+
   await expect(recoveryPhone.addHeader()).toBeVisible();
 
   await recoveryPhone.enterPhoneNumber(target.smsClient.getPhoneNumber());
diff --git a/packages/fxa-auth-client/lib/client.ts b/packages/fxa-auth-client/lib/client.ts
@@ -2392,7 +2392,7 @@ export default class AuthClient {
   }
 
   /**
-   * Tries to register a recovery phone number
+   * Tries to register a recovery phone number. Important, this must be used for inline_recovery flow!
    *
    * @param sessionToken The user's current session token
    * @param phoneNumber The phone number to register. Should be E.164 format
@@ -2411,6 +2411,26 @@ export default class AuthClient {
     );
   }
 
+  /**
+   * Tries to register a recovery phone number
+   *
+   * @param jwt The users MFA jwt
+   * @param phoneNumber The phone number to register. Should be E.164 format
+   * @param headers
+   */
+  async recoveryPhoneCreateWithJwt(
+    jwt: string,
+    phoneNumber: string,
+    headers?: Headers
+  ): Promise<{ nationalFormat?: string; success: boolean }> {
+    return this.jwtPost(
+      '/mfa/recovery_phone/create',
+      jwt,
+      { phoneNumber },
+      headers
+    );
+  }
+
   /**
    * Checks to see if a recovery phone is available in the user's region.
    * @param sessionToken The user's current session token
diff --git a/packages/fxa-auth-server/lib/routes/recovery-phone.ts b/packages/fxa-auth-server/lib/routes/recovery-phone.ts
@@ -1037,6 +1037,32 @@ export const recoveryPhoneRoutes = (
         return recoveryPhoneHandler.setupPhoneNumber(request);
       },
     },
+    {
+      method: 'POST',
+      path: '/mfa/recovery_phone/create',
+      options: {
+        pre: [{ method: featureEnabledCheck }],
+        auth: {
+          strategy: 'mfa',
+          scope: ['mfa:2fa'],
+          payload: false,
+        },
+        validate: {
+          payload: isA.object({
+            phoneNumber: isA.string().regex(E164_NUMBER).required(),
+          }),
+        },
+      },
+      handler: function (request: AuthRequest) {
+        return routes
+          .find(
+            (route) =>
+              route.path === '/v1/recovery_phone/create' &&
+              route.method === 'POST'
+          )
+          ?.handler(request);
+      },
+    },
     {
       method: 'POST',
       path: '/recovery_phone/available',
diff --git a/packages/fxa-settings/src/components/Settings/Page2faReplaceBackupCodes/index.tsx b/packages/fxa-settings/src/components/Settings/Page2faReplaceBackupCodes/index.tsx
@@ -21,6 +21,7 @@ import VerifiedSessionGuard from '../VerifiedSessionGuard';
 import FlowSetup2faBackupCodeDownload from '../FlowSetup2faBackupCodeDownload';
 import FlowSetup2faBackupCodeConfirm from '../FlowSetup2faBackupCodeConfirm';
 import { MfaGuard } from '../MfaGuard';
+import { isInvalidJwtError } from '../../../lib/mfa-guard-utils';
 
 export const PageMfaGuard2faReplaceBackupCodes = (
   props: RouteComponentProps
@@ -96,7 +97,7 @@ export const Page2faReplaceBackupCodes = (_: RouteComponentProps) => {
 
       alertSuccessAndGoHome();
     } catch (e) {
-      if (e && e.code === 401 && e.errno === 110) {
+      if (isInvalidJwtError(e)) {
         // JWT invalid/expired
         errorHandler(e);
         return;
diff --git a/packages/fxa-settings/src/components/Settings/Page2faSetup/index.tsx b/packages/fxa-settings/src/components/Settings/Page2faSetup/index.tsx
@@ -186,6 +186,7 @@ const Page2faSetup = (_: RouteComponentProps) => {
   };
 
   const handleVerifyPhone = async (phoneNumberInput: string) => {
+    // TODO: Switch to addRecoveryPhoneJwt once page is wrapped with MfaGuard
     const { nationalFormat } = await account.addRecoveryPhone(phoneNumberInput);
     setPhoneData({
       phoneNumber: phoneNumberInput,
@@ -194,6 +195,7 @@ const Page2faSetup = (_: RouteComponentProps) => {
   };
 
   const handleResendSms = async () => {
+    // TODO: Switch to addRecoveryPhoneWithJwt once page is wrapped with MfaGuard
     await account.addRecoveryPhone(phoneData.phoneNumber);
   };
 
diff --git a/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.test.tsx b/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.test.tsx
@@ -48,15 +48,20 @@ describe('PageRecoveryPhoneSetup', () => {
 
   it('add recovery phone with successful submission renders step 2', async () => {
     const user = userEvent.setup();
-    const addRecoveryPhone = jest
+    const addRecoveryPhoneWithJwt = jest
       .fn()
       .mockResolvedValueOnce(mockSuccessResponse);
-    renderWithRouter(<Subject account={{ addRecoveryPhone }} />);
+
+    renderWithRouter(<Subject account={{ addRecoveryPhoneWithJwt }} />);
 
     await completeStepOne(user);
-    await waitFor(() => expect(addRecoveryPhone).toHaveBeenCalledTimes(1));
     await waitFor(() =>
-      expect(addRecoveryPhone).toHaveBeenCalledWith(MOCK_FULL_PHONE_NUMBER)
+      expect(addRecoveryPhoneWithJwt).toHaveBeenCalledTimes(1)
+    );
+    await waitFor(() =>
+      expect(addRecoveryPhoneWithJwt).toHaveBeenCalledWith(
+        MOCK_FULL_PHONE_NUMBER
+      )
     );
     expect(
       screen.queryByText(/You’ll get a text message from Mozilla/i)
@@ -69,12 +74,12 @@ describe('PageRecoveryPhoneSetup', () => {
     const confirmRecoveryPhone = jest
       .fn()
       .mockResolvedValueOnce(mockSuccessResponse);
-    const addRecoveryPhone = jest
+    const addRecoveryPhoneWithJwt = jest
       .fn()
       .mockResolvedValueOnce(mockSuccessResponse)
       .mockResolvedValueOnce(mockSuccessResponse);
     renderWithRouter(
-      <Subject account={{ confirmRecoveryPhone, addRecoveryPhone }} />
+      <Subject account={{ confirmRecoveryPhone, addRecoveryPhoneWithJwt }} />
     );
 
     await completeStepOne(user);
@@ -88,21 +93,29 @@ describe('PageRecoveryPhoneSetup', () => {
       })
     );
 
-    await waitFor(() => expect(addRecoveryPhone).toHaveBeenCalledTimes(2));
-    expect(addRecoveryPhone).toHaveBeenNthCalledWith(1, MOCK_FULL_PHONE_NUMBER);
-    expect(addRecoveryPhone).toHaveBeenNthCalledWith(2, MOCK_FULL_PHONE_NUMBER);
+    await waitFor(() =>
+      expect(addRecoveryPhoneWithJwt).toHaveBeenCalledTimes(2)
+    );
+    expect(addRecoveryPhoneWithJwt).toHaveBeenNthCalledWith(
+      1,
+      MOCK_FULL_PHONE_NUMBER
+    );
+    expect(addRecoveryPhoneWithJwt).toHaveBeenNthCalledWith(
+      2,
+      MOCK_FULL_PHONE_NUMBER
+    );
   });
 
   it('at step 2, allows code confirm', async () => {
     const user = userEvent.setup();
     const confirmRecoveryPhone = jest
       .fn()
       .mockResolvedValueOnce(mockSuccessResponse);
-    const addRecoveryPhone = jest
+    const addRecoveryPhoneWithJwt = jest
       .fn()
       .mockResolvedValueOnce(mockSuccessResponse);
     renderWithRouter(
-      <Subject account={{ confirmRecoveryPhone, addRecoveryPhone }} />
+      <Subject account={{ confirmRecoveryPhone, addRecoveryPhoneWithJwt }} />
     );
 
     await completeStepOne(user);
@@ -127,11 +140,11 @@ describe('PageRecoveryPhoneSetup', () => {
     const confirmRecoveryPhone = jest
       .fn()
       .mockResolvedValueOnce(mockSuccessResponse);
-    const addRecoveryPhone = jest
+    const addRecoveryPhoneWithJwt = jest
       .fn()
       .mockResolvedValueOnce(mockSuccessResponse);
     renderWithRouter(
-      <Subject account={{ confirmRecoveryPhone, addRecoveryPhone }} />
+      <Subject account={{ confirmRecoveryPhone, addRecoveryPhoneWithJwt }} />
     );
 
     await waitFor(() =>
@@ -152,11 +165,11 @@ describe('PageRecoveryPhoneSetup', () => {
     const confirmRecoveryPhone = jest
       .fn()
       .mockResolvedValueOnce(mockSuccessResponse);
-    const addRecoveryPhone = jest
+    const addRecoveryPhoneWithJwt = jest
       .fn()
       .mockResolvedValueOnce(mockSuccessResponse);
     renderWithRouter(
-      <Subject account={{ confirmRecoveryPhone, addRecoveryPhone }} />
+      <Subject account={{ confirmRecoveryPhone, addRecoveryPhoneWithJwt }} />
     );
 
     await completeStepOne(user);
diff --git a/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.tsx b/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.tsx
@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import React, { useState } from 'react';
+import { useErrorHandler } from 'react-error-boundary';
 import { useNavigateWithQuery } from '../../../lib/hooks/useNavigateWithQuery';
 import { SETTINGS_PATH } from '../../../constants';
 import { useAccount, useFtlMsgResolver } from '../../../models';
@@ -11,14 +12,25 @@ import FlowSetupRecoveryPhoneConfirmCode from '../FlowSetupRecoveryPhoneConfirmC
 import FlowSetupRecoveryPhoneSubmitNumber from '../FlowSetupRecoveryPhoneSubmitNumber';
 import { RouteComponentProps, useLocation } from '@reach/router';
 import { RecoveryPhoneSetupReason } from '../../../lib/types';
+import { MfaGuard } from '../MfaGuard';
+import { isInvalidJwtError } from '../../../lib/mfa-guard-utils';
 
 const numberOfSteps = 2;
 
+export const MfaGuardPageRecoveryPhoneSetup = (props: RouteComponentProps) => {
+  return (
+    <MfaGuard requiredScope="2fa">
+      <PageRecoveryPhoneSetup {...props} />
+    </MfaGuard>
+  );
+};
+
 export const PageRecoveryPhoneSetup = (_: RouteComponentProps) => {
   const ftlMsgResolver = useFtlMsgResolver();
   const navigateWithQuery = useNavigateWithQuery();
   const account = useAccount();
   const location = useLocation();
+  const errorHandler = useErrorHandler();
   const reason: RecoveryPhoneSetupReason =
     (location as any)?.state?.reason ?? RecoveryPhoneSetupReason.setup;
 
@@ -82,7 +94,17 @@ export const PageRecoveryPhoneSetup = (_: RouteComponentProps) => {
     // one code can be valid at the same time if the user clicks “resend code” to
     // account for SMS transmission delay. (This will change in FXA-11039)
     // try/catch is in the component that calls this function
-    await account.addRecoveryPhone(phoneData.phoneNumber);
+    try {
+      await account.addRecoveryPhoneWithJwt(phoneData.phoneNumber);
+    } catch (e) {
+      if (isInvalidJwtError(e)) {
+        // JWT invalid/expired
+        errorHandler(e);
+        return;
+      }
+
+      throw e;
+    }
   };
 
   const verifyRecoveryCode = async (code: string) => {
@@ -97,11 +119,22 @@ export const PageRecoveryPhoneSetup = (_: RouteComponentProps) => {
 
   const verifyPhoneNumber = async (phoneNumberInput: string) => {
     // try/catch is in the component that calls this function
-    const { nationalFormat } = await account.addRecoveryPhone(phoneNumberInput);
-    setPhoneData({
-      phoneNumber: phoneNumberInput,
-      nationalFormat,
-    });
+    try {
+      const { nationalFormat } =
+        await account.addRecoveryPhoneWithJwt(phoneNumberInput);
+      setPhoneData({
+        phoneNumber: phoneNumberInput,
+        nationalFormat,
+      });
+    } catch (e) {
+      if (isInvalidJwtError(e)) {
+        // JWT invalid/expired
+        errorHandler(e);
+        return;
+      }
+
+      throw e;
+    }
   };
 
   return (
@@ -118,7 +151,7 @@ export const PageRecoveryPhoneSetup = (_: RouteComponentProps) => {
             verifyPhoneNumber,
             currentStep,
             numberOfSteps,
-            reason
+            reason,
           }}
         />
       )}
@@ -139,7 +172,7 @@ export const PageRecoveryPhoneSetup = (_: RouteComponentProps) => {
             verifyRecoveryCode,
             currentStep,
             numberOfSteps,
-            reason
+            reason,
           }}
         />
       )}
diff --git a/packages/fxa-settings/src/components/Settings/PageSecondaryEmailAdd/index.tsx b/packages/fxa-settings/src/components/Settings/PageSecondaryEmailAdd/index.tsx
@@ -17,6 +17,7 @@ import { getErrorFtlId } from '../../../lib/error-utils';
 import { MfaGuard } from '../MfaGuard';
 import { useErrorHandler } from 'react-error-boundary';
 import VerifiedSessionGuard from '../VerifiedSessionGuard';
+import { isInvalidJwtError } from '../../../lib/mfa-guard-utils';
 
 export const PageSecondaryEmailAdd = (_: RouteComponentProps) => {
   usePageViewEvent('settings.emails');
@@ -45,7 +46,7 @@ export const PageSecondaryEmailAdd = (_: RouteComponentProps) => {
         await account.createSecondaryEmail(email);
         navigateWithQuery('emails/verify', { state: { email }, replace: true });
       } catch (e) {
-        if (e && e.code === 401 && e.errno === 110) {
+        if (isInvalidJwtError(e)) {
           // JWT invalid/expired
           errorHandler(e);
           return;
diff --git a/packages/fxa-settings/src/components/Settings/PageSecondaryEmailVerify/index.tsx b/packages/fxa-settings/src/components/Settings/PageSecondaryEmailVerify/index.tsx
diff --git a/packages/fxa-settings/src/components/Settings/index.test.tsx b/packages/fxa-settings/src/components/Settings/index.test.tsx
diff --git a/packages/fxa-settings/src/components/Settings/index.tsx b/packages/fxa-settings/src/components/Settings/index.tsx
diff --git a/packages/fxa-settings/src/models/Account.ts b/packages/fxa-settings/src/models/Account.ts
