Merge pull request #19458 from mozilla/FXA-12234

task(settings): Add MFA guard to remove 2FA SMS backup

Author: dschom

packages/functional-tests/tests/settings/recoveryPhone.spec.ts

@@ -99,6 +99,7 @@ test.describe('severity-1 #smoke', () => {
       await expect(settings.alertBar).toHaveText('Recovery phone added');
 
       await settings.totp.removeRecoveryPhoneButton.click();
+
       await page.waitForURL(/recovery_phone\/remove/);
       await page.getByRole('button', { name: 'Remove phone number' }).click();
 

packages/fxa-auth-client/lib/client.ts

@@ -1531,14 +1531,16 @@ export default class AuthClient {
     } = {},
     headers?: Headers
   ): Promise<SignedInAccountData> {
-
-    const oldCredentials = await this.sessionReauth(sessionToken, options.reauthEmail || email, oldPassword, {
-      keys: true
-    }, headers);
-    const oldCredentialsAuth = await crypto.getCredentials(
-      email,
-      oldPassword
+    const oldCredentials = await this.sessionReauth(
+      sessionToken,
+      options.reauthEmail || email,
+      oldPassword,
+      {
+        keys: true,
+      },
+      headers
     );
+    const oldCredentialsAuth = await crypto.getCredentials(email, oldPassword);
     const oldAuthPW = oldCredentialsAuth.authPW;
 
     const keys = await this.accountKeys(
@@ -1547,10 +1549,7 @@ export default class AuthClient {
       headers
     );
 
-    const newCredentials = await crypto.getCredentials(
-      email,
-      newPassword
-    );
+    const newCredentials = await crypto.getCredentials(email, newPassword);
 
     const wrapKb = crypto.unwrapKB(keys.kB, newCredentials.unwrapBKey);
     const authPW = newCredentials.authPW;
@@ -1600,8 +1599,8 @@ export default class AuthClient {
       if (
         error &&
         error.email &&
-        error.errno === ERRORS.INCORRECT_EMAIL_CASE
-        && !options.skipCaseError
+        error.errno === ERRORS.INCORRECT_EMAIL_CASE &&
+        !options.skipCaseError
       ) {
         options.skipCaseError = true;
         options.reauthEmail = email;
@@ -2979,7 +2978,7 @@ export default class AuthClient {
   }
 
   /**
-   * Removes a recovery phone from the user's account
+   * @deprecated Use recoveryPhoneDeleteWithJwt instead
    *
    * @param sessionToken The user's current session token
    * @param headers
@@ -2988,6 +2987,17 @@ export default class AuthClient {
     return this.sessionDelete('/recovery_phone', sessionToken, {}, headers);
   }
 
+  /**
+   * Disables 2FA Protection on the account.
+   *
+   * @param jwt - required, must be a verified session token
+   * @param headers - Optional additional headers for the request
+   * @returns A promise that resolves when the 2FA has been removed
+   */
+  async recoveryPhoneDeleteWithJwt(jwt: string, headers?: Headers) {
+    return this.jwtDelete('/mfa/recovery_phone', jwt, {}, headers);
+  }
+
   /**
    * Gets status of the recovery phone on the users account.
    * @param sessionToken The user's current session token

packages/fxa-auth-server/lib/routes/recovery-phone.ts

@@ -1193,6 +1193,26 @@ export const recoveryPhoneRoutes = (
         return recoveryPhoneHandler.destroy(request);
       },
     },
+    {
+      method: 'DELETE',
+      path: '/mfa/recovery_phone',
+      options: {
+        auth: {
+          strategy: 'mfa',
+          scope: ['mfa:2fa'],
+          payload: false,
+        },
+        response: {},
+      },
+      handler: async function (request: AuthRequest) {
+        return routes
+          .find(
+            (route) =>
+              route.path === '/v1/recovery_phone' && route.method === 'DELETE'
+          )
+          ?.handler(request);
+      },
+    },
     {
       method: 'GET',
       path: '/recovery_phone',

packages/fxa-settings/src/components/Settings/PageRecoveryPhoneRemove/index.stories.tsx

@@ -5,7 +5,7 @@
 import React from 'react';
 import { Meta } from '@storybook/react';
 import { withLocalization } from 'fxa-react/lib/storybooks';
-import PageRecoveryPhoneRemove from '.';
+import { PageRecoveryPhoneRemove } from '.';
 import { LocationProvider } from '@reach/router';
 import { mockAppContext, MOCK_ACCOUNT } from '../../../models/mocks';
 import { Account, AppContext } from '../../../models';

packages/fxa-settings/src/components/Settings/PageRecoveryPhoneRemove/index.test.tsx

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import React from 'react';
-import PageRecoveryPhoneRemove from '.';
+import { PageRecoveryPhoneRemove } from '.';
 import { act, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import { Account, AppContext } from '../../../models';

packages/fxa-settings/src/components/Settings/PageRecoveryPhoneRemove/index.tsx

@@ -15,6 +15,7 @@ import FlowContainer from '../FlowContainer';
 import { getLocalizedErrorMessage } from '../../../lib/error-utils';
 import GleanMetrics from '../../../lib/glean';
 import { useNavigateWithQuery } from '../../../lib/hooks/useNavigateWithQuery';
+import { MfaGuard } from '../MfaGuard';
 
 // TODO, update this link with #section-heading once the SUMO article is updated (FXA-10918)
 const sumoTwoStepLink = (
@@ -26,7 +27,7 @@ const sumoTwoStepLink = (
   </LinkExternal>
 );
 
-const PageRecoveryPhoneRemove = (props: RouteComponentProps) => {
+export const PageRecoveryPhoneRemove = (props: RouteComponentProps) => {
   const navigateWithQuery = useNavigateWithQuery();
   const account = useAccount();
   const alertBar = useAlertBar();
@@ -140,4 +141,10 @@ const PageRecoveryPhoneRemove = (props: RouteComponentProps) => {
   );
 };
 
-export default PageRecoveryPhoneRemove;
+export const PageMfaGuardRecoveryPhoneRemove = (props: RouteComponentProps) => {
+  return (
+    <MfaGuard requiredScope="2fa">
+      <PageRecoveryPhoneRemove {...props} />
+    </MfaGuard>
+  );
+};

packages/fxa-settings/src/components/Settings/index.test.tsx

@@ -197,9 +197,9 @@ describe('Settings App', () => {
   });
   it('routes to PageChangePassword', async () => {
     // Suppress MFA guard for this test
-    mockMfaGuard.mockImplementationOnce(({ children }: { children: ReactNode }) => (
-      <>{children}</>
-    ));
+    mockMfaGuard.mockImplementationOnce(
+      ({ children }: { children: ReactNode }) => <>{children}</>
+    );
 
     // Mock the JWT token cache to suppress MFA guard
     const mockJwtTokenCache = {
@@ -373,6 +373,11 @@ describe('Settings App', () => {
         route: '/change_password',
         hasPassword: true,
       },
+      {
+        pageName: 'PageRecoveryPhoneRemove',
+        route: '/recovery_phone/remove',
+        hasPassword: true,
+      },
     ];
 
     it.each(guardedRoutes)(

packages/fxa-settings/src/components/Settings/index.tsx

@@ -33,7 +33,7 @@ import { currentAccount } from '../../lib/cache';
 import { hasAccount, setCurrentAccount } from '../../lib/storage-utils';
 import GleanMetrics from '../../lib/glean';
 import Head from 'fxa-react/components/Head';
-import PageRecoveryPhoneRemove from './PageRecoveryPhoneRemove';
+import { PageMfaGuardRecoveryPhoneRemove } from './PageRecoveryPhoneRemove';
 import { SettingsIntegration } from './interfaces';
 import { useNavigateWithQuery } from '../../lib/hooks/useNavigateWithQuery';
 import MfaGuardPage2faChange from './Page2faChange';
@@ -197,7 +197,7 @@ export const Settings = ({
           <Redirect from="/avatar/change" to="/settings/avatar/" noThrow />
 
           <MfaGuardPageRecoveryPhoneSetup path="/recovery_phone/setup" />
-          <PageRecoveryPhoneRemove path="/recovery_phone/remove" />
+          <PageMfaGuardRecoveryPhoneRemove path="/recovery_phone/remove" />
 
           <PageMfaGuardTestWithAuthClient path="/mfa_guard/test/auth_client" />
           <PageMfaGuardTestWithGql path="/mfa_guard/test/gql" />

packages/fxa-settings/src/models/Account.ts

@@ -1566,8 +1566,9 @@ export class Account implements AccountData {
   }
 
   async removeRecoveryPhone() {
+    const jwt = this.getCachedJwtByScope('2fa');
     const result = await this.withLoadingStatus(
-      this.authClient.recoveryPhoneDelete(sessionToken()!)
+      this.authClient.recoveryPhoneDeleteWithJwt(jwt)
     );
     return result;
   }