Merge pull request #15863 from mozilla/FXA-8419

fix(react): Prevent preemptive reset password email from sending with accountResetToken tweaks

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

@@ -507,7 +507,7 @@ export default class AuthClient {
     code: string,
     passwordForgotToken: hexstring,
     options: {
-      accountResetWithoutRecoveryKey?: boolean;
+      accountResetWithRecoveryKey?: boolean;
     } = {},
     headers: Headers = new Headers()
   ) {

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

@@ -623,10 +623,41 @@ export class Account implements AccountData {
     return result;
   }
 
+  /**
+   * TODO: Remove this once we update the GQL endpoint to accept the
+   *  `accountResetWithRecoveryKey` option and fix graphql-api not reporting
+   *  the correct IP address.
+   *
+   * @param token passwordForgotToken
+   * @param code code
+   * @param accountResetWithRecoveryKey is account being reset with recovery key?
+   * */
+  async passwordForgotVerifyCode(
+    token: string,
+    code: string,
+    accountResetWithRecoveryKey = false
+  ): Promise<string> {
+    // TODO: There is a bug in Backbone and React reset PW around `accountResetWithRecoveryKey`.
+    // We attempt to validate the `code` and `token` provided here, but because the
+    // `accountResetToken` can only be fetched once but uses the `accountResetWithRecoveryKey`
+    // option, auth-server fails to send an email if the user has a recovery key, attempts
+    // to use it unsuccessfully, and then goes through a normal reset via the link back
+    // to a normal reset if a user can't use their key.
+    const { accountResetToken } =
+      await this.authClient.passwordForgotVerifyCode(code, token, {
+        accountResetWithRecoveryKey,
+      });
+    return accountResetToken;
+  }
+
   /**
    * Verify a passwordForgotToken, which returns an accountResetToken that can
    * be used to perform the actual password reset.
    *
+   * NOTE! and TODO: this is currently unused. We need to update the GQL
+   * endpoint to accept the `accountResetWithRecoveryKey` option and
+   * fix graphql-api not reporting the correct IP address.
+   *
    * @param token passwordForgotToken
    * @param code code
    */
@@ -669,7 +700,8 @@ export class Account implements AccountData {
     token: string,
     code: string,
     email: string,
-    newPassword: string
+    newPassword: string,
+    resetToken?: string
   ): Promise<any> {
     try {
       // TODO: Temporary workaround (use auth-client directly) for GraphQL not
@@ -678,10 +710,8 @@ export class Account implements AccountData {
       //   token,
       //   code
       // );
-      const { accountResetToken } =
-        await this.authClient.passwordForgotVerifyCode(code, token, {
-          accountResetWithoutRecoveryKey: true,
-        });
+      const accountResetToken =
+        resetToken || (await this.passwordForgotVerifyCode(token, code));
       const {
         data: { accountReset },
       } = await this.apolloClient.mutate({

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

@@ -70,9 +70,7 @@ const accountWithValidResetToken = {
     recoveryData: 'mockRecoveryData',
     recoveryKeyId: MOCK_RECOVERY_KEY_ID,
   }),
-  verifyPasswordForgotToken: jest
-    .fn()
-    .mockResolvedValue({ accountResetToken: MOCK_RESET_TOKEN }),
+  passwordForgotVerifyCode: jest.fn().mockResolvedValue(MOCK_RESET_TOKEN),
 } as unknown as Account;
 
 const renderSubject = ({
@@ -127,7 +125,7 @@ describe('PageAccountRecoveryConfirmKey', () => {
   it('renders the component as expected when provided with an expired link', async () => {
     const accountWithTokenError = {
       resetPasswordStatus: jest.fn().mockResolvedValue(false),
-      verifyPasswordForgotToken: jest.fn().mockImplementation(() => {
+      passwordForgotVerifyCode: jest.fn().mockImplementation(() => {
         throw AuthUiErrors.INVALID_TOKEN;
       }),
     } as unknown as Account;
@@ -287,9 +285,7 @@ describe('PageAccountRecoveryConfirmKey', () => {
   it('submits successfully after invalid recovery key submission', async () => {
     const accountWithKeyInvalidOnce = {
       resetPasswordStatus: jest.fn().mockResolvedValue(true),
-      verifyPasswordForgotToken: jest
-        .fn()
-        .mockResolvedValue({ accountResetToken: MOCK_RESET_TOKEN }),
+      passwordForgotVerifyCode: jest.fn().mockResolvedValue(MOCK_RESET_TOKEN),
       getRecoveryKeyBundle: jest
         .fn()
         .mockImplementationOnce(() => {
@@ -316,17 +312,18 @@ describe('PageAccountRecoveryConfirmKey', () => {
       screen.getByRole('button', { name: 'Confirm account recovery key' })
     );
 
-    // only ever calls `verifyPasswordForgotToken` once despite number of submissions
+    // only ever calls `passwordForgotVerifyCode` once despite number of submissions
     await waitFor(() =>
       expect(
-        accountWithKeyInvalidOnce.verifyPasswordForgotToken
+        accountWithKeyInvalidOnce.passwordForgotVerifyCode
       ).toHaveBeenCalledTimes(1)
     );
     expect(
-      accountWithKeyInvalidOnce.verifyPasswordForgotToken
+      accountWithKeyInvalidOnce.passwordForgotVerifyCode
     ).toHaveBeenCalledWith(
       mockCompleteResetPasswordParams.token,
-      mockCompleteResetPasswordParams.code
+      mockCompleteResetPasswordParams.code,
+      true
     );
     expect(
       accountWithKeyInvalidOnce.getRecoveryKeyBundle

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

@@ -146,9 +146,10 @@ const AccountRecoveryConfirmKey = ({
       try {
         let resetToken = fetchedResetToken;
         if (!resetToken) {
-          const { accountResetToken } = await account.verifyPasswordForgotToken(
+          const accountResetToken = await account.passwordForgotVerifyCode(
             token,
-            code
+            code,
+            true
           );
           setFetchedResetToken(accountResetToken);
           resetToken = accountResetToken;
@@ -308,7 +309,10 @@ const AccountRecoveryConfirmKey = ({
           to={`/complete_reset_password${location.search}`}
           className="link-blue text-sm"
           id="lost-recovery-key"
-          state={{ lostRecoveryKey: true }}
+          state={{
+            lostRecoveryKey: true,
+            accountResetToken: fetchedResetToken,
+          }}
           onClick={() => {
             logViewEvent(
               'flow',

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

@@ -24,6 +24,7 @@ import {
   mockAppContext,
   renderWithRouter,
 } from '../../../models/mocks';
+import { MOCK_RESET_TOKEN } from '../../mocks';
 // import { getFtlBundle, testAllL10n } from 'fxa-react/lib/test-utils';
 // import { FluentBundle } from '@fluent/bundle';
 
@@ -43,6 +44,7 @@ jest.mock('../../../lib/metrics', () => ({
 
 let account: Account;
 let lostRecoveryKey: boolean;
+let accountResetToken: string | undefined;
 const mockNavigate = jest.fn();
 
 const mockSearchParams = {
@@ -61,6 +63,7 @@ const mockLocation = () => {
     search,
     state: {
       lostRecoveryKey,
+      accountResetToken,
     },
   };
 };
@@ -355,7 +358,8 @@ describe('CompleteResetPassword page', () => {
         token,
         code,
         emailToHashWith,
-        PASSWORD
+        PASSWORD,
+        undefined
       );
     });
     it('submits with email if emailToHashWith is missing', async () => {
@@ -367,7 +371,24 @@ describe('CompleteResetPassword page', () => {
         token,
         code,
         email,
-        PASSWORD
+        PASSWORD,
+        undefined
+      );
+    });
+
+    it('submits with accountResetToken if available', async () => {
+      lostRecoveryKey = true;
+      accountResetToken = MOCK_RESET_TOKEN;
+      render(<Subject />, account);
+      const { token, emailToHashWith, code } = mockCompleteResetPasswordParams;
+
+      await enterPasswordAndSubmit();
+      expect(account.completeResetPassword).toHaveBeenCalledWith(
+        token,
+        code,
+        emailToHashWith,
+        PASSWORD,
+        MOCK_RESET_TOKEN
       );
     });
 

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

@@ -86,7 +86,13 @@ const CompleteResetPassword = ({
    * If users clicked the link leading back to this page from `account_recovery_confirm_key`,
    * we assume the user has lost the key and pass along a `lostRecoveryKey` flag
    * so we don't perform the check and redirect again.
+   *
    * If the link is -not- valid, we render link expired or link damaged.
+   *
+   * Additionally, the user can submit an invalid account recovery key and receive back
+   * an `accountResetToken`, then follow the link back to this page. In this case, we
+   * should _not_ check if the 'token' parameter is valid, since it will be invalid after
+   * this token is provided to us.
    */
   useEffect(() => {
     const checkPasswordForgotToken = async (token: string) => {
@@ -147,12 +153,19 @@ const CompleteResetPassword = ({
       setShowLoadingSpinner(false);
       logPageViewEvent(viewName, REACT_ENTRYPOINT);
     };
-    checkPasswordForgotToken(linkModel.token);
+    // If a user comes from AccountRecoveryConfirmKey and attempted a recovery key
+    // submission, 'token' was already verified and used to fetch the reset token
+    if (!location.state?.accountResetToken) {
+      checkPasswordForgotToken(linkModel.token);
+    } else {
+      renderCompleteResetPassword();
+    }
   }, [
     account,
     navigate,
     location.search,
     location.state?.lostRecoveryKey,
+    location.state?.accountResetToken,
     linkModel.email,
     linkModel.token,
     setLinkStatus,
@@ -187,7 +200,8 @@ const CompleteResetPassword = ({
           token,
           code,
           emailToUse,
-          newPassword
+          newPassword,
+          location.state?.accountResetToken
         );
 
         /* NOTE: Session check/totp check must come after completeResetPassword since those
@@ -255,6 +269,7 @@ const CompleteResetPassword = ({
     [
       account,
       integration,
+      location.state?.accountResetToken,
       alertSuccessAndNavigate,
       ftlMsgResolver,
       setLinkStatus,

packages/fxa-settings/src/pages/ResetPassword/CompleteResetPassword/interfaces.ts

@@ -24,6 +24,7 @@ export type CompleteResetPasswordSubmitData = {
 
 export interface CompleteResetPasswordLocationState {
   lostRecoveryKey: boolean;
+  accountResetToken: string;
 }
 
 export interface CompleteResetPasswordParams {

packages/fxa-settings/src/pages/mocks.ts

@@ -12,5 +12,6 @@ export const MOCK_SERVICE = MozServices.FirefoxMonitor;
 export const MOCK_SESSION_TOKEN = 'sessionToken';
 export const MOCK_UNWRAP_BKEY = 'unwrapBKey';
 export const MOCK_KEY_FETCH_TOKEN = 'keyFetchToken';
+export const MOCK_RESET_TOKEN = 'mockResetToken';
 export const MOCK_AUTH_AT = 12345;
 export const MOCK_PASSWORD = 'notYourAveragePassW0Rd';