Merge pull request #19472 from mozilla/FXA-12398

task(settings,auth): Wrap remove account recovery key with MFA guard

Author: dschom

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

@@ -348,6 +348,21 @@ export default class AuthClient {
     return this.request('POST', path, payload, headers);
   }
 
+  private async jwtDelete(
+    path: string,
+    jwt: string,
+    payload: any,
+    headers?: Headers
+  ) {
+    const authorization = 'Bearer ' + jwt;
+    if (!headers) {
+      headers = new Headers({ authorization });
+    } else {
+      headers.set('authorization', authorization);
+    }
+    return this.request('DELETE', path, payload, headers);
+  }
+
   private async jwtPut(
     path: string,
     jwt: string,
@@ -2410,6 +2425,9 @@ export default class AuthClient {
     return accountData;
   }
 
+  /**
+   * @deprecated Use deleteRecoveryKeyWithJwt instead
+   */
   async deleteRecoveryKey(sessionToken: hexstring, headers?: Headers) {
     return this.hawkRequest(
       'DELETE',
@@ -2421,6 +2439,16 @@ export default class AuthClient {
     );
   }
 
+  /**
+   * Removes the recovery key from the account
+   * @param jwt The MFA jwt for auth
+   * @param headers
+   * @returns
+   */
+  async deleteRecoveryKeyWithJwt(jwt: string, headers?: Headers) {
+    return this.jwtDelete('/mfa/recoveryKey', jwt, {}, headers);
+  }
+
   async recoveryKeyExists(
     sessionToken: hexstring | undefined,
     email: string | undefined,

packages/fxa-auth-server/docs/swagger/recovery-key-api.ts

@@ -60,6 +60,15 @@ const RECOVERYKEY_DELETE = {
   ],
 };
 
+const MFA_RECOVERY_KEY_DELETE = {
+  ...TAGS_RECOVERY_KEY,
+  description: '/recoveryKey',
+  notes: [
+    '🔒 Authenticated with MFA JWT (scope: mfa:recovery_key)',
+    "This route remove an account's account recovery key. When the key is removed, it can no longer be used to restore an account's kB.",
+  ],
+};
+
 const RECOVERYKEY_VERIFY_POST = {
   ...TAGS_RECOVERY_KEY,
   description: '/recoveryKey/verify',
@@ -92,6 +101,7 @@ const RECOVERYKEY_HINT_POST = {
 
 const API_DOCS = {
   RECOVERYKEY_DELETE,
+  MFA_RECOVERY_KEY_DELETE,
   RECOVERYKEY_EXISTS_POST,
   RECOVERYKEY_POST,
   MFA_RECOVERY_KEY_POST,

packages/fxa-auth-server/lib/routes/recovery-key.js

@@ -470,6 +470,26 @@ module.exports = (
         return {};
       },
     },
+    {
+      method: 'DELETE',
+      path: '/mfa/recoveryKey',
+      options: {
+        ...RECOVERY_KEY_DOCS.MFA_RECOVERY_KEY_DELETE,
+        auth: {
+          strategy: 'mfa',
+          scope: ['mfa:recovery_key'],
+          payload: false,
+        },
+      },
+      async handler(request) {
+        return routes
+          .find(
+            (route) =>
+              route.path === '/v1/recoveryKey' && route.method === 'DELETE'
+          )
+          .handler(request);
+      },
+    },
   ];
 
   return routes;

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

@@ -9,11 +9,22 @@ import { mockAppContext, renderWithRouter } from '../../../models/mocks';
 import { Account, AppContext } from '../../../models';
 import * as Metrics from '../../../lib/metrics';
 import { Config } from '../../../lib/config';
+import AuthClient from 'fxa-auth-client/browser';
+import * as cache from '../../../lib/cache';
 
 jest.mock('../../../lib/metrics', () => ({
   logViewEvent: jest.fn(),
 }));
 
+jest.mock('../../../lib/cache', () => ({
+  ...jest.requireActual('../../../lib/cache'),
+  sessionToken: jest.fn(),
+  currentAccount: jest.fn(),
+}));
+
+const sessionToken = 'session-123';
+const jwt = 'jwt-123';
+
 const accountHasRecoveryKey = {
   hasPassword: true,
   recoveryKey: { exists: true },
@@ -29,11 +40,22 @@ const accountWithoutPassword = {
   recoveryKey: { exists: false },
 } as unknown as Account;
 
+const authClient = {} as unknown as AuthClient;
+
 const renderWithContext = (
   account: Partial<Account>,
   config?: Partial<Config>
 ) => {
-  const context = { account: account as Account, config: config as Config };
+  const context = {
+    authClient,
+    account: account as Account,
+    config: config as Config,
+  };
+
+  (cache.currentAccount as jest.Mock).mockReturnValue(account);
+  (cache.sessionToken as jest.Mock).mockReturnValue(sessionToken);
+  cache.JwtTokenCache.getToken = jest.fn().mockReturnValue(jwt);
+  cache.JwtTokenCache.hasToken = jest.fn().mockReturnValue(true);
 
   renderWithRouter(
     <AppContext.Provider value={mockAppContext(context)}>
@@ -103,7 +125,7 @@ describe('UnitRowRecoveryKey', () => {
       const accountHasRecoveryKeyWithDeleteSuccess = {
         hasPassword: true,
         recoveryKey: { exists: true },
-        deleteRecoveryKey: jest.fn().mockResolvedValue(true),
+        deleteRecoveryKeyWithJwt: jest.fn().mockResolvedValue(true),
       } as unknown as Account;
 
       renderWithContext(accountHasRecoveryKeyWithDeleteSuccess);
@@ -124,7 +146,7 @@ describe('UnitRowRecoveryKey', () => {
       const accountHasRecoveryKeyWithDeleteFailure = {
         hasPassword: true,
         recoveryKey: { exists: true },
-        deleteRecoveryKey: jest.fn().mockRejectedValue(false),
+        deleteRecoveryKeyWithJwt: jest.fn().mockRejectedValue(false),
       } as unknown as Account;
 
       renderWithContext(accountHasRecoveryKeyWithDeleteFailure);

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

@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import React, { useCallback, useState } from 'react';
+import { useErrorHandler } from 'react-error-boundary';
 import { useBooleanState } from 'fxa-react/lib/hooks';
 import { useAccount, useAlertBar, useFtlMsgResolver } from '../../../models';
 import { logViewEvent } from '../../../lib/metrics';
@@ -13,9 +14,12 @@ import { ButtonIconTrash } from '../ButtonIcon';
 import { SETTINGS_PATH } from '../../../constants';
 import { FtlMsg } from 'fxa-react/lib/utils';
 import GleanMetrics from '../../../lib/glean';
+import { isInvalidJwtError } from '../../../lib/mfa-guard-utils';
+import { MfaGuard } from '../MfaGuard';
 
 export const UnitRowRecoveryKey = () => {
   const account = useAccount();
+  const errorHandler = useErrorHandler();
 
   const recoveryKey = account.recoveryKey.exists;
   const alertBar = useAlertBar();
@@ -25,7 +29,7 @@ export const UnitRowRecoveryKey = () => {
 
   const deleteRecoveryKey = useCallback(async () => {
     try {
-      await account.deleteRecoveryKey();
+      await account.deleteRecoveryKeyWithJwt();
       hideModal();
       alertBar.success(
         ftlMsgResolver.getMsg(
@@ -36,6 +40,13 @@ export const UnitRowRecoveryKey = () => {
       logViewEvent('flow.settings.account-recovery', 'confirm-revoke.success');
     } catch (e) {
       hideModal();
+
+      if (isInvalidJwtError(e)) {
+        // JWT invalid/expired
+        errorHandler(e);
+        return;
+      }
+
       alertBar.error(
         ftlMsgResolver.getMsg(
           'rk-remove-error-2',
@@ -46,7 +57,7 @@ export const UnitRowRecoveryKey = () => {
     } finally {
       setIsDeleting(false);
     }
-  }, [account, hideModal, alertBar, ftlMsgResolver]);
+  }, [account, hideModal, alertBar, ftlMsgResolver, errorHandler]);
 
   const localizedDeleteRKIconButton = ftlMsgResolver.getMsg(
     'unit-row-recovery-key-delete-icon-button-title',
@@ -120,40 +131,42 @@ export const UnitRowRecoveryKey = () => {
             );
           }}
         >
-          <Modal
-            onDismiss={() => {
-              hideModal();
-            }}
-            onConfirm={() => {
-              setIsDeleting(true);
-              deleteRecoveryKey();
-              logViewEvent(
-                'flow.settings.account-recovery',
-                'confirm-revoke.submit'
-              );
-            }}
-            confirmBtnClassName="cta-caution cta-base-p"
-            confirmText={ftlMsgResolver.getMsg('rk-action-remove', 'Remove')}
-            headerId="recovery-key-header"
-            descId="recovery-key-desc"
-            isLoading={isDeleting}
-          >
-            <FtlMsg id="rk-remove-modal-heading-1">
-              <h2
-                id="recovery-key-header"
-                className="font-bold text-xl text-center mb-2"
-              >
-                Remove account recovery key?
-              </h2>
-            </FtlMsg>
-            <FtlMsg id="rk-remove-modal-content-1">
-              <p className="my-6 text-center">
-                In the event you reset your password, you wonʼt be able to use
-                your account recovery key to access your data. You canʼt undo
-                this action.
-              </p>
-            </FtlMsg>
-          </Modal>
+          <MfaGuard requiredScope="recovery_key">
+            <Modal
+              onDismiss={() => {
+                hideModal();
+              }}
+              onConfirm={() => {
+                setIsDeleting(true);
+                deleteRecoveryKey();
+                logViewEvent(
+                  'flow.settings.account-recovery',
+                  'confirm-revoke.submit'
+                );
+              }}
+              confirmBtnClassName="cta-caution cta-base-p"
+              confirmText={ftlMsgResolver.getMsg('rk-action-remove', 'Remove')}
+              headerId="recovery-key-header"
+              descId="recovery-key-desc"
+              isLoading={isDeleting}
+            >
+              <FtlMsg id="rk-remove-modal-heading-1">
+                <h2
+                  id="recovery-key-header"
+                  className="font-bold text-xl text-center mb-2"
+                >
+                  Remove account recovery key?
+                </h2>
+              </FtlMsg>
+              <FtlMsg id="rk-remove-modal-content-1">
+                <p className="my-6 text-center">
+                  In the event you reset your password, you wonʼt be able to use
+                  your account recovery key to access your data. You canʼt undo
+                  this action.
+                </p>
+              </FtlMsg>
+            </Modal>
+          </MfaGuard>
         </VerifiedSessionGuard>
       )}
     </UnitRow>

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

@@ -1143,10 +1143,9 @@ export class Account implements AccountData {
     await this.refresh('backupCodes');
   }
 
-  async deleteRecoveryKey() {
-    await this.withLoadingStatus(
-      this.authClient.deleteRecoveryKey(sessionToken()!)
-    );
+  async deleteRecoveryKeyWithJwt() {
+    const jwt = this.getCachedJwtByScope('recovery_key');
+    await this.withLoadingStatus(this.authClient.deleteRecoveryKeyWithJwt(jwt));
     const cache = this.apolloClient.cache;
     cache.modify({
       id: cache.identify({ __typename: 'Account' }),
@@ -1613,7 +1612,7 @@ export class Account implements AccountData {
    * @param scope MfaScope
    * @returns JWT token string
    */
-  private getCachedJwtByScope(scope: MfaScope) {
+  getCachedJwtByScope(scope: MfaScope) {
     const token = sessionToken();
     if (!token) {
       throw AuthUiErrors.INVALID_TOKEN;