fix(react): Password reset unexpected error fixes, with and without r…

…ecovery key

Because:
* There's an error when attempting to reset a password with an account recovery key, and other issues were uncovered during the fix

This commit:
* Fixes logic around accountResetToken, ensuring we only fetch getRecoveryBundle once with the same token
* Requests, receives, and sets sessionToken from account recovery reset and normal PW reset
* Uses this session token to perform other requests that must use authClient for now due to 'sessionToken.mustVerify' checks

Closes FXA-7189

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

@@ -89,7 +89,6 @@ export async function generateRecoveryKey(
  */
 export async function decryptRecoveryKeyData(
   recoveryKey: Uint8Array,
-  recoveryKeyId: string,
   recoveryData: string,
   uid: hexstring
 ): Promise<DecryptedRecoveryKeyData> {

packages/fxa-content-server/app/scripts/models/account.js

@@ -74,6 +74,9 @@ const DEFAULTS = _.extend(
     verificationReason: undefined,
     totpVerified: undefined,
     providerUid: undefined,
+
+    // TODO before merge: probably don't need this anymore with the getOldSettingsData fn
+    authAt: undefined,
   },
   PERSISTENT
 );

packages/fxa-graphql-api/src/gql/account.resolver.ts

@@ -502,14 +502,18 @@ export class AccountResolver {
     @GqlXHeaders() headers: Headers,
     @Args('input', { type: () => AccountResetInput })
     input: AccountResetInput
-  ) {
-    return this.authAPI.accountReset(
+  ): Promise<AccountResetPayload> {
+    const result = await this.authAPI.accountReset(
       input.email,
       input.newPassword,
       input.accountResetToken,
       input.options,
       headers
     );
+    return {
+      clientMutationId: input.clientMutationId,
+      ...result,
+    };
   }
 
   @Mutation((returns) => SignedUpAccountPayload, {

packages/fxa-settings/src/lib/cache.ts

@@ -14,6 +14,22 @@ export interface OldSettingsData {
   metricsEnabled?: boolean;
 }
 
+export function getOldSettingsData({
+  uid,
+  sessionToken,
+  alertText,
+  displayName,
+  metricsEnabled,
+}: Record<string, any>): OldSettingsData {
+  return {
+    uid,
+    sessionToken,
+    alertText,
+    displayName,
+    metricsEnabled,
+  };
+}
+
 type LocalAccount = OldSettingsData | undefined;
 type LocalAccounts = Record<hexstring, LocalAccount> | undefined;
 

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

@@ -11,7 +11,7 @@ import AuthClient, {
   generateRecoveryKey,
   getRecoveryKeyIdByUid,
 } from 'fxa-auth-client/browser';
-import { currentAccount, sessionToken } from '../lib/cache';
+import { currentAccount, getOldSettingsData, sessionToken } from '../lib/cache';
 import firefox from '../lib/channels/firefox';
 import Storage from '../lib/storage';
 import random from '../lib/random';
@@ -712,16 +712,29 @@ export class Account implements AccountData {
         token,
         code
       );
-      await this.apolloClient.mutate({
+      const {
+        data: { accountReset },
+      } = await this.apolloClient.mutate({
         mutation: gql`
           mutation accountReset($input: AccountResetInput!) {
             accountReset(input: $input) {
               clientMutationId
+              sessionToken
+              uid
             }
           }
         `,
-        variables: { input: { accountResetToken, email, newPassword } },
+        variables: {
+          input: {
+            accountResetToken,
+            email,
+            newPassword,
+            options: { sessionToken: true },
+          },
+        },
       });
+      currentAccount(getOldSettingsData(accountReset));
+      sessionToken(accountReset.sessionToken);
     } catch (err) {
       const errno = (err as ApolloError).graphQLErrors[0].extensions?.errno;
       if (errno && AuthUiErrorNos[errno]) {
@@ -897,6 +910,26 @@ export class Account implements AccountData {
     );
   }
 
+  /* TODO: Remove this and use GQL instead. We can't check for verified sessions
+   * unless you've already got one (oof) in at least the PW reset flow due to
+   * sessionToken.mustVerify which was added here: https://github.com/mozilla/fxa/pull/7512
+   * The unverified session token returned by a password reset contains `mustVerify`
+   * which causes the 'Must verify' error to be thrown and a redirect to occur */
+  async isSessionVerifiedAuthClient() {
+    const { state } = await this.withLoadingStatus(
+      this.authClient.sessionStatus(sessionToken()!)
+    );
+    return state === 'verified';
+  }
+
+  // TODO: Same as isSessionVerifiedAuthClient
+  async hasTotpAuthClient() {
+    const { verified } = await this.withLoadingStatus(
+      this.authClient.checkTotpTokenExists(sessionToken()!)
+    );
+    return verified;
+  }
+
   async generateRecoveryCodes(count: number, length: number) {
     const recoveryCodes: string[] = [];
     const gen = random.base32(length);
@@ -1234,9 +1267,11 @@ export class Account implements AccountData {
       opts.emailToHashWith,
       opts.password,
       opts.recoveryKeyId,
-      { kB: opts.kB }
+      { kB: opts.kB },
+      { sessionToken: true }
     );
-    currentAccount(data);
+    currentAccount(currentAccount(getOldSettingsData(data)));
+    sessionToken(data.sessionToken);
     const cache = this.apolloClient.cache;
     cache.modify({
       id: cache.identify({ __typename: 'Account' }),

packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryConfirmKey/index.test.tsx

@@ -4,7 +4,7 @@
 
 import React from 'react';
 import '@testing-library/jest-dom/extend-expect';
-import { fireEvent, render, screen } from '@testing-library/react';
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
 // import { getFtlBundle, testAllL10n } from 'fxa-react/lib/test-utils';
 // import { FluentBundle } from '@fluent/bundle';
 import { usePageViewEvent, logViewEvent } from '../../../lib/metrics';
@@ -257,6 +257,47 @@ describe('PageAccountRecoveryConfirmKey', () => {
         }
       );
     });
+
+    it('submits successfully after invalid recovery key submission', async () => {
+      account = {
+        ...account,
+        getRecoveryKeyBundle: jest
+          .fn()
+          .mockImplementationOnce(() => {
+            return Promise.reject(new Error('Oh noes'));
+          })
+          .mockResolvedValue({
+            recoveryData: 'mockRecoveryData',
+            recoveryKeyId: MOCK_RECOVERY_KEY_ID,
+          }),
+      } as unknown as Account;
+
+      render(<Subject {...{ account }} />);
+      await typeByLabelText('Enter account recovery key')(MOCK_RECOVERY_KEY);
+      fireEvent.click(
+        screen.getByRole('button', { name: 'Confirm account recovery key' })
+      );
+      await screen.findByText('Invalid account recovery key');
+
+      fireEvent.click(
+        screen.getByRole('button', { name: 'Confirm account recovery key' })
+      );
+
+      // only ever calls `verifyPasswordForgotToken` once despite number of submissions
+      await waitFor(() =>
+        expect(account.verifyPasswordForgotToken).toHaveBeenCalledTimes(1)
+      );
+      expect(account.verifyPasswordForgotToken).toHaveBeenCalledWith(
+        mockToken,
+        mockCode
+      );
+      expect(account.getRecoveryKeyBundle).toHaveBeenCalledTimes(2);
+      expect(account.getRecoveryKeyBundle).toHaveBeenCalledWith(
+        MOCK_RESET_TOKEN,
+        MOCK_RECOVERY_KEY,
+        mockUid
+      );
+    });
   });
 
   describe('emits metrics events', () => {

packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryConfirmKey/index.tsx

@@ -102,9 +102,8 @@ const AccountRecoveryConfirmKey = (_: RouteComponentProps) => {
       const decodedRecoveryKey = base32Decode(recoveryKey, 'Crockford');
       const uint8RecoveryKey = new Uint8Array(decodedRecoveryKey);
 
-      const decryptedData = await decryptRecoveryKeyData(
+      const { kB } = await decryptRecoveryKeyData(
         uint8RecoveryKey,
-        recoveryKeyId,
         recoveryData,
         uid
       );
@@ -113,7 +112,7 @@ const AccountRecoveryConfirmKey = (_: RouteComponentProps) => {
         state: {
           accountResetToken,
           recoveryKeyId,
-          kB: decryptedData.kB,
+          kB,
         },
       });
     },
@@ -123,21 +122,17 @@ const AccountRecoveryConfirmKey = (_: RouteComponentProps) => {
   const checkRecoveryKey = useCallback(
     async ({ recoveryKey, token, code, email, uid }: SubmitData) => {
       try {
-        if (!fetchedResetToken) {
+        let resetToken = fetchedResetToken;
+        if (!resetToken) {
           const { accountResetToken } = await account.verifyPasswordForgotToken(
             token,
             code
           );
           setFetchedResetToken(accountResetToken);
-          await getRecoveryBundleAndNavigate({
-            accountResetToken,
-            recoveryKey,
-            uid,
-            email,
-          });
+          resetToken = accountResetToken;
         }
         await getRecoveryBundleAndNavigate({
-          accountResetToken: fetchedResetToken,
+          accountResetToken: resetToken,
           recoveryKey,
           uid,
           email,

packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryResetPassword/index.test.tsx

@@ -38,6 +38,13 @@ let mockOauthClient = mocks.mockOauthClient();
 let mockNavigate = mocks.mockNavigate();
 let mockNotifier = mocks.mockNotifier();
 
+const mockUseNavigateWithoutRerender = jest.fn();
+
+jest.mock('../../../lib/hooks/useNavigateWithoutRerender', () => ({
+  __esModule: true,
+  default: () => mockUseNavigateWithoutRerender,
+}));
+
 jest.mock('../../../models/hooks', () => {
   return {
     __esModule: true,
@@ -290,6 +297,39 @@ describe('AccountRecoveryResetPassword page', () => {
           expect(notifyFirefoxOfLogin).toHaveBeenCalled();
         });
       });
+
+      describe('navigateAway', () => {
+        // Not needed once this page doesn't use `hardNavigateToContentServer`
+        const originalWindow = window.location;
+        beforeAll(() => {
+          // @ts-ignore
+          delete window.location;
+          window.location = { ...originalWindow, href: '' };
+        });
+        beforeEach(() => {
+          window.location.href = originalWindow.href;
+        });
+        afterAll(() => {
+          window.location = originalWindow;
+        });
+
+        it('account has TOTP', async () => {
+          // todo
+
+          expect(window.location.href).toBe('/signin_totp_code');
+        });
+
+        it('account does not have TOTP', async () => {
+          // todo
+
+          expect(mockUseNavigateWithoutRerender).toHaveBeenCalledWith(
+            '/reset_password_verified',
+            {
+              replace: true,
+            }
+          );
+        });
+      });
     });
   });
 

packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryResetPassword/index.tsx

@@ -9,7 +9,7 @@ import {
   useLocation,
   useNavigate,
 } from '@reach/router';
-import { FtlMsg } from 'fxa-react/lib/utils';
+import { FtlMsg, hardNavigateToContentServer } from 'fxa-react/lib/utils';
 import { useForm } from 'react-hook-form';
 
 import AppLayout from '../../../components/AppLayout';
@@ -252,10 +252,10 @@ const AccountRecoveryResetPassword = ({
           verificationInfo.emailToHashWith || verificationInfo.email,
       };
 
-      const [sessionisVerified] = await Promise.all([
-        account.isSessionVerified(),
-        account.resetPasswordWithRecoveryKey(options),
-      ]);
+      await account.resetPasswordWithRecoveryKey(options);
+      // must come after completeResetPassword since that receives the sessionToken
+      // required for this check
+      const sessionIsVerified = await account.isSessionVerifiedAuthClient();
 
       // FOLLOW-UP: Functionality not yet available.
       await account.setLastLogin(Date.now());
@@ -268,11 +268,11 @@ const AccountRecoveryResetPassword = ({
 
       switch (integration.type) {
         case IntegrationType.SyncDesktop:
-          notifyFirefoxOfLogin(account, sessionisVerified);
+          notifyFirefoxOfLogin(account, sessionIsVerified);
           break;
         case IntegrationType.OAuth:
           if (
-            sessionisVerified &&
+            sessionIsVerified &&
             // a user can only redirect back to the relier from the original tab
             // to avoid two tabs redirecting.
             isOriginalTab()
@@ -314,18 +314,20 @@ const AccountRecoveryResetPassword = ({
     setBannerState(BannerState.PasswordResetSuccess);
   }
 
-  function navigateAway() {
-    setUserPreference('account-recovery', account.recoveryKey);
+  async function navigateAway() {
+    // TODO / refactor: the initial account result with useAccount() does not
+    // contain account data due to no session token. Users receive the session
+    // token on PW reset, so we can query here for it.
+    const hasTotp = await account.hasTotpAuthClient();
+
+    setUserPreference('account-recovery', false);
     logViewEvent(viewName, 'recovery-key-consume.success');
 
-    // When users reset a PW with their recovery key we always want to navigate to
-    // the page encouraging them to generate another. If they don't have a verified
-    // session, clicking on any link taking them into Settings should take them to
-    // `signin_token_code` to verify their session first, then take them to Settings.
-    // We don't prompt them for a TOTP code if enabled, unlike a regular password reset,
-    // because posession of the primary email to use the reset link while using a
-    // recovery key is sufficient proof of ownership.
-    navigate(`/reset_password_with_recovery_key_verified?${location.search}`);
+    if (hasTotp) {
+      hardNavigateToContentServer(`/signin_totp_code${location.search}`);
+    } else {
+      navigate(`/reset_password_with_recovery_key_verified${location.search}`);
+    }
   }
 
   function alertValidationError(err: ModelValidationErrors) {