Merge pull request #19486 from mozilla/FXA-12425

bug(auth, tests): Issue AAL2 token for password reset with recovery key

Author: dschom

packages/functional-tests/tests/resetPassword/resetPassword2FA.spec.ts

@@ -116,7 +116,7 @@ test.describe('severity-1 #smoke', () => {
     credentials.password = newPassword;
   });
 
-  test('can reset password with recovery key and 2FA enabled but not prompted', async ({
+  test('can reset password with recovery key without 2FA prompt', async ({
     target,
     pages: { signin, resetPassword, settings, totp, recoveryKey },
     testAccountTracker,
@@ -183,6 +183,82 @@ test.describe('severity-1 #smoke', () => {
     credentials.password = newPassword;
   });
 
+  test('can reset password with recovery key then delete account', async ({
+    target,
+    pages: {
+      page,
+      signin,
+      resetPassword,
+      settings,
+      totp,
+      recoveryKey,
+      deleteAccount,
+    },
+    testAccountTracker,
+  }) => {
+    const credentials = await testAccountTracker.signUp();
+    const newPassword = testAccountTracker.generatePassword();
+
+    await signin.goto();
+    await signin.fillOutEmailFirstForm(credentials.email);
+    await signin.fillOutPasswordForm(credentials.password);
+
+    // Enable 2FA
+    await expect(settings.settingsHeading).toBeVisible();
+    await expect(settings.totp.status).toHaveText('Disabled');
+
+    await settings.totp.addButton.click();
+    await totp.setUpTwoStepAuthWithQrAndBackupCodesChoice();
+
+    await expect(settings.settingsHeading).toBeVisible();
+    await expect(settings.alertBar).toHaveText(
+      'Two-step authentication has been enabled'
+    );
+    await expect(settings.totp.status).toHaveText('Enabled');
+
+    // Create recovery key
+    await settings.recoveryKey.createButton.click();
+    const key = await recoveryKey.createRecoveryKey(
+      credentials.password,
+      'hint'
+    );
+
+    // Verify status as 'enabled'
+    await expect(settings.settingsHeading).toBeVisible();
+    await expect(settings.recoveryKey.status).toHaveText('Enabled');
+
+    await settings.signOut();
+
+    await resetPassword.goto();
+
+    await resetPassword.fillOutEmailForm(credentials.email);
+
+    const code = await target.emailClient.getResetPasswordCode(
+      credentials.email
+    );
+    await resetPassword.fillOutResetPasswordCodeForm(code);
+
+    // Not prompted for 2FA during reset password
+    await resetPassword.fillOutRecoveryKeyForm(key);
+    await resetPassword.fillOutNewPasswordForm(newPassword);
+
+    await expect(resetPassword.passwordResetPasswordSaved).toBeVisible();
+
+    await resetPassword.continueWithoutDownloadingRecoveryKey();
+    await resetPassword.recoveryKeyFinishButton.click();
+
+    await expect(settings.settingsHeading).toBeVisible();
+
+    // Recovery key has been consumed and a new one created
+    await expect(settings.recoveryKey.status).toHaveText('Enabled');
+
+    // Account deletion requires AAL match
+    // If AAL does not match, account deletion fails with 'unconfirmed session' error
+    await settings.deleteAccountButton.click();
+    await deleteAccount.deleteAccount(newPassword);
+    await expect(page.getByText('Account deleted successfully')).toBeVisible();
+  });
+
   test('can reset password with 2FA and forgot recovery key', async ({
     page,
     target,

packages/fxa-auth-server/lib/routes/account.ts

@@ -1044,7 +1044,7 @@ export class AccountHandler {
       // they have to verify the token.
       const hasTotpToken = await this.otpUtils.hasTotpToken(accountRecord);
       if (hasTotpToken) {
-        // User has enabled TOTP, no way around it, they must verify TOTP token
+        // User has enabled TOTP, they must verify TOTP token unless they are using recovery key
         verificationMethod = 'totp-2fa';
       } else if (!hasTotpToken && verificationMethod === 'totp-2fa') {
         // Error if requesting TOTP verification with TOTP not setup
@@ -1766,6 +1766,12 @@ export class AccountHandler {
             sessionToken.id,
             accountResetToken.verificationMethod
           );
+        } else if (recoveryKeyId && hasTotpToken) {
+          // If account has TOTP but recovery key is used,
+          // we elevate to AAL2 to match account AAL.
+          // Recovery key is considered a 2FA method.
+          // Only elevate if account has TOTP, otherwise session AAL > account AAL.
+          await this.db.verifyTokensWithMethod(sessionToken.id, 'totp-2fa');
         }
 
         return await request.propagateMetricsContext(

packages/fxa-auth-server/test/local/routes/account.js

@@ -439,6 +439,43 @@ describe('/account/reset', () => {
         null,
         'db.createSessionToken was passed correct form factor'
       );
+
+      // Token is not verified with TOTP-2FA method (AAL2) if account does not have TOTP
+      assert.equal(
+        mockDB.verifyTokensWithMethod.callCount,
+        0,
+        'db.verifyTokensWithMethod was not called'
+      );
+    });
+  });
+
+  describe('reset account with account recovery key, TOTP enabled', () => {
+    let res;
+    beforeEach(() => {
+      mockDB.totpToken = sinon.spy(() => {
+        return Promise.resolve({
+          verified: true,
+          enabled: true,
+        });
+      });
+      mockRequest.payload.wrapKb = hexString(32);
+      mockRequest.payload.recoveryKeyId = hexString(16);
+      return runTest(route, mockRequest, (result) => (res = result));
+    });
+
+    it('should return response', () => {
+      assert.ok(res.sessionToken, 'return sessionToken');
+      assert.ok(res.keyFetchToken, 'return keyFetchToken');
+    });
+
+    it('should verify token with TOTP-2FA method (AAL2) if account has TOTP', () => {
+      assert.equal(
+        mockDB.verifyTokensWithMethod.callCount,
+        1,
+        'db.verifyTokensWithMethod was called once'
+      );
+      const verifyArgs = mockDB.verifyTokensWithMethod.args[0];
+      assert.equal(verifyArgs[1], 'totp-2fa');
     });
   });