fix(2fa-recovery-code-error): [Auth/PM-19885] Better error handling when 2FA recovery code is invalid (#16145)

Implements better error handling when a user enters an invalid 2FA recovery code. Upon entering an invalid code:
- Keep the user on the `/recover-2fa` page (This also makes it so the incorrect code remains in the form field so the user can see what they entered, if they mistyped the code, etc.)
- Show an inline error: "Invalid recovery code"

Author: rr-bw

apps/web/src/app/auth/recover-two-factor.component.html

@@ -28,7 +28,6 @@
     <bit-label>{{ "recoveryCodeTitle" | i18n }}</bit-label>
     <input bitInput type="text" formControlName="recoveryCode" appInputVerbatim />
   </bit-form-field>
-  <hr />
   <div class="tw-flex tw-gap-2">
     <button type="submit" bitButton bitFormButton buttonType="primary" [block]="true">
       {{ "submit" | i18n }}

apps/web/src/app/auth/recover-two-factor.component.spec.ts

@@ -9,10 +9,12 @@ import {
 } from "@bitwarden/auth/common";
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result";
+import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
 import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
+import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service";
 import { ToastService } from "@bitwarden/components";
 import { KeyService } from "@bitwarden/key-management";
 import { I18nPipe } from "@bitwarden/ui-common";
@@ -34,6 +36,7 @@ describe("RecoverTwoFactorComponent", () => {
   let mockConfigService: MockProxy<ConfigService>;
   let mockLoginSuccessHandlerService: MockProxy<LoginSuccessHandlerService>;
   let mockLogService: MockProxy<LogService>;
+  let mockValidationService: MockProxy<ValidationService>;
 
   beforeEach(() => {
     mockRouter = mock<Router>();
@@ -46,6 +49,7 @@ describe("RecoverTwoFactorComponent", () => {
     mockConfigService = mock<ConfigService>();
     mockLoginSuccessHandlerService = mock<LoginSuccessHandlerService>();
     mockLogService = mock<LogService>();
+    mockValidationService = mock<ValidationService>();
 
     TestBed.configureTestingModule({
       declarations: [RecoverTwoFactorComponent],
@@ -60,6 +64,7 @@ describe("RecoverTwoFactorComponent", () => {
         { provide: ConfigService, useValue: mockConfigService },
         { provide: LoginSuccessHandlerService, useValue: mockLoginSuccessHandlerService },
         { provide: LogService, useValue: mockLogService },
+        { provide: ValidationService, useValue: mockValidationService },
       ],
       imports: [I18nPipe],
       // FIXME(PM-18598): Replace unknownElements and unknownProperties with actual imports
@@ -70,16 +75,21 @@ describe("RecoverTwoFactorComponent", () => {
     component = fixture.componentInstance;
   });
 
-  afterEach(() => {
-    jest.resetAllMocks();
-  });
-
   describe("handleRecoveryLogin", () => {
+    let email: string;
+    let recoveryCode: string;
+
+    beforeEach(() => {
+      email = "[email protected]";
+      recoveryCode = "testRecoveryCode";
+    });
+
+    afterEach(() => {
+      jest.resetAllMocks();
+    });
+
     it("should log in successfully and navigate to the two-factor settings page", async () => {
       // Arrange
-      const email = "[email protected]";
-      const recoveryCode = "testRecoveryCode";
-
       const authResult = new AuthResult();
       mockLoginStrategyService.logIn.mockResolvedValue(authResult);
 
@@ -98,14 +108,16 @@ describe("RecoverTwoFactorComponent", () => {
       expect(mockRouter.navigate).toHaveBeenCalledWith(["/settings/security/two-factor"]);
     });
 
-    it("should handle login errors and redirect to login page", async () => {
+    it("should log an error and set an inline error on the recoveryCode form control upon receiving an ErrorResponse due to an invalid token", async () => {
       // Arrange
-      const email = "[email protected]";
-      const recoveryCode = "testRecoveryCode";
-
-      const error = new Error("Login failed");
+      const error = new ErrorResponse("mockError", 400);
+      error.message = "Two-step token is invalid";
       mockLoginStrategyService.logIn.mockRejectedValue(error);
 
+      const recoveryCodeControl = component.formGroup.get("recoveryCode");
+      jest.spyOn(recoveryCodeControl, "setErrors");
+      mockI18nService.t.mockReturnValue("Invalid recovery code");
+
       // Act
       await component["loginWithRecoveryCode"](email, recoveryCode);
 
@@ -114,9 +126,43 @@ describe("RecoverTwoFactorComponent", () => {
         "Error logging in automatically: ",
         error.message,
       );
-      expect(mockRouter.navigate).toHaveBeenCalledWith(["/login"], {
-        queryParams: { email: email },
+      expect(recoveryCodeControl.setErrors).toHaveBeenCalledWith({
+        invalidRecoveryCode: { message: "Invalid recovery code" },
       });
     });
+
+    it("should log an error and show validation but not set an inline error on the recoveryCode form control upon receiving some other ErrorResponse", async () => {
+      // Arrange
+      const error = new ErrorResponse("mockError", 400);
+      error.message = "Some other error";
+      mockLoginStrategyService.logIn.mockRejectedValue(error);
+
+      const recoveryCodeControl = component.formGroup.get("recoveryCode");
+      jest.spyOn(recoveryCodeControl, "setErrors");
+
+      // Act
+      await component["loginWithRecoveryCode"](email, recoveryCode);
+
+      // Assert
+      expect(mockLogService.error).toHaveBeenCalledWith(
+        "Error logging in automatically: ",
+        error.message,
+      );
+      expect(mockValidationService.showError).toHaveBeenCalledWith(error.message);
+      expect(recoveryCodeControl.setErrors).not.toHaveBeenCalled();
+    });
+
+    it("should log an error and show validation upon receiving a non-ErrorResponse error", async () => {
+      // Arrange
+      const error = new Error("Generic error");
+      mockLoginStrategyService.logIn.mockRejectedValue(error);
+
+      // Act
+      await component["loginWithRecoveryCode"](email, recoveryCode);
+
+      // Assert
+      expect(mockLogService.error).toHaveBeenCalledWith("Error logging in automatically: ", error);
+      expect(mockValidationService.showError).toHaveBeenCalledWith(error);
+    });
   });
 });

apps/web/src/app/auth/recover-two-factor.component.ts

@@ -1,4 +1,5 @@
-import { Component, OnInit } from "@angular/core";
+import { Component, DestroyRef, OnInit } from "@angular/core";
+import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
 import { FormControl, FormGroup, Validators } from "@angular/forms";
 import { Router } from "@angular/router";
 
@@ -9,8 +10,10 @@ import {
 } from "@bitwarden/auth/common";
 import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type";
 import { TokenTwoFactorRequest } from "@bitwarden/common/auth/models/request/identity-token/token-two-factor.request";
+import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service";
 import { ToastService } from "@bitwarden/components";
 
 @Component({
@@ -19,7 +22,7 @@ import { ToastService } from "@bitwarden/components";
   standalone: false,
 })
 export class RecoverTwoFactorComponent implements OnInit {
-  protected formGroup = new FormGroup({
+  formGroup = new FormGroup({
     email: new FormControl("", [Validators.required]),
     masterPassword: new FormControl("", [Validators.required]),
     recoveryCode: new FormControl("", [Validators.required]),
@@ -31,16 +34,23 @@ export class RecoverTwoFactorComponent implements OnInit {
   recoveryCodeMessage = "";
 
   constructor(
+    private destroyRef: DestroyRef,
     private router: Router,
     private i18nService: I18nService,
     private loginStrategyService: LoginStrategyServiceAbstraction,
     private toastService: ToastService,
     private loginSuccessHandlerService: LoginSuccessHandlerService,
     private logService: LogService,
+    private validationService: ValidationService,
   ) {}
 
   async ngOnInit() {
     this.recoveryCodeMessage = this.i18nService.t("logInBelowUsingYourSingleUseRecoveryCode");
+
+    // Clear any existing recovery code inline error when user updates the form
+    this.formGroup.valueChanges.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(() => {
+      this.formGroup.get("recoveryCode")?.setErrors(null);
+    });
   }
 
   get email(): string {
@@ -99,10 +109,21 @@ export class RecoverTwoFactorComponent implements OnInit {
       await this.loginSuccessHandlerService.run(authResult.userId);
 
       await this.router.navigate(["/settings/security/two-factor"]);
-    } catch (error) {
-      // If login errors, redirect to login page per product. Don't show error
-      this.logService.error("Error logging in automatically: ", (error as Error).message);
-      await this.router.navigate(["/login"], { queryParams: { email: email } });
+    } catch (error: unknown) {
+      if (error instanceof ErrorResponse) {
+        this.logService.error("Error logging in automatically: ", error.message);
+
+        if (error.message.includes("Two-step token is invalid")) {
+          this.formGroup.get("recoveryCode")?.setErrors({
+            invalidRecoveryCode: { message: this.i18nService.t("invalidRecoveryCode") },
+          });
+        } else {
+          this.validationService.showError(error.message);
+        }
+      } else {
+        this.logService.error("Error logging in automatically: ", error);
+        this.validationService.showError(error);
+      }
     }
   }
 }

apps/web/src/locales/en/messages.json

@@ -1517,6 +1517,9 @@
   "recoveryCodeTitle": {
     "message": "Recovery code"
   },
+  "invalidRecoveryCode": {
+    "message": "Invalid recovery code"
+  },
   "authenticatorAppTitle": {
     "message": "Authenticator app"
   },