fix(recovery-code-login): [PM-18474] Fix for Recovery Code Login (#13497)

* fix(recovery-code-login): [PM-18474] Fix for Recovery Code Login - Fixed the recovery code login to work with the new device verification notice flow.

* test(recovery-code-login): [PM-18474] Fix for Recovery Code Login - Tests added.

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

@@ -0,0 +1,133 @@
+import { ComponentFixture, TestBed } from "@angular/core/testing";
+import { Router } from "@angular/router";
+import { mock, MockProxy } from "jest-mock-extended";
+
+import {
+  LoginStrategyServiceAbstraction,
+  LoginSuccessHandlerService,
+  PasswordLoginCredentials,
+} from "@bitwarden/auth/common";
+import { ApiService } from "@bitwarden/common/abstractions/api.service";
+import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result";
+import { TwoFactorRecoveryRequest } from "@bitwarden/common/auth/models/request/two-factor-recovery.request";
+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 { ToastService } from "@bitwarden/components";
+import { KeyService } from "@bitwarden/key-management";
+import { I18nPipe } from "@bitwarden/ui-common";
+import { NewDeviceVerificationNoticeService } from "@bitwarden/vault";
+
+import { RecoverTwoFactorComponent } from "./recover-two-factor.component";
+
+describe("RecoverTwoFactorComponent", () => {
+  let component: RecoverTwoFactorComponent;
+  let fixture: ComponentFixture<RecoverTwoFactorComponent>;
+
+  // Mock Services
+  let mockRouter: MockProxy<Router>;
+  let mockApiService: MockProxy<ApiService>;
+  let mockPlatformUtilsService: MockProxy<PlatformUtilsService>;
+  let mockI18nService: MockProxy<I18nService>;
+  let mockKeyService: MockProxy<KeyService>;
+  let mockLoginStrategyService: MockProxy<LoginStrategyServiceAbstraction>;
+  let mockToastService: MockProxy<ToastService>;
+  let mockConfigService: MockProxy<ConfigService>;
+  let mockLoginSuccessHandlerService: MockProxy<LoginSuccessHandlerService>;
+  let mockLogService: MockProxy<LogService>;
+  let mockNewDeviceVerificationNoticeService: MockProxy<NewDeviceVerificationNoticeService>;
+
+  beforeEach(() => {
+    mockRouter = mock<Router>();
+    mockApiService = mock<ApiService>();
+    mockPlatformUtilsService = mock<PlatformUtilsService>();
+    mockI18nService = mock<I18nService>();
+    mockKeyService = mock<KeyService>();
+    mockLoginStrategyService = mock<LoginStrategyServiceAbstraction>();
+    mockToastService = mock<ToastService>();
+    mockConfigService = mock<ConfigService>();
+    mockLoginSuccessHandlerService = mock<LoginSuccessHandlerService>();
+    mockLogService = mock<LogService>();
+    mockNewDeviceVerificationNoticeService = mock<NewDeviceVerificationNoticeService>();
+
+    TestBed.configureTestingModule({
+      declarations: [RecoverTwoFactorComponent],
+      providers: [
+        { provide: Router, useValue: mockRouter },
+        { provide: ApiService, useValue: mockApiService },
+        { provide: PlatformUtilsService, mockPlatformUtilsService },
+        { provide: I18nService, useValue: mockI18nService },
+        { provide: KeyService, useValue: mockKeyService },
+        { provide: LoginStrategyServiceAbstraction, useValue: mockLoginStrategyService },
+        { provide: ToastService, useValue: mockToastService },
+        { provide: ConfigService, useValue: mockConfigService },
+        { provide: LoginSuccessHandlerService, useValue: mockLoginSuccessHandlerService },
+        { provide: LogService, useValue: mockLogService },
+        {
+          provide: NewDeviceVerificationNoticeService,
+          useValue: mockNewDeviceVerificationNoticeService,
+        },
+      ],
+      imports: [I18nPipe],
+    });
+
+    fixture = TestBed.createComponent(RecoverTwoFactorComponent);
+    component = fixture.componentInstance;
+  });
+
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
+  describe("handleRecoveryLogin", () => {
+    it("should log in successfully and navigate to the two-factor settings page", async () => {
+      // Arrange
+      const request = new TwoFactorRecoveryRequest();
+      request.recoveryCode = "testRecoveryCode";
+      request.email = "test@example.com";
+
+      const authResult = new AuthResult();
+      mockLoginStrategyService.logIn.mockResolvedValue(authResult);
+
+      // Act
+      await component["handleRecoveryLogin"](request);
+
+      // Assert
+      expect(mockLoginStrategyService.logIn).toHaveBeenCalledWith(
+        expect.any(PasswordLoginCredentials),
+      );
+      expect(mockToastService.showToast).toHaveBeenCalledWith({
+        variant: "success",
+        title: "",
+        message: mockI18nService.t("youHaveBeenLoggedIn"),
+      });
+      expect(
+        mockNewDeviceVerificationNoticeService.updateNewDeviceVerificationSkipNoticeState,
+      ).toHaveBeenCalledWith(authResult.userId, true);
+      expect(mockRouter.navigate).toHaveBeenCalledWith(["/settings/security/two-factor"]);
+    });
+
+    it("should handle login errors and redirect to login page", async () => {
+      // Arrange
+      const request = new TwoFactorRecoveryRequest();
+      request.recoveryCode = "testRecoveryCode";
+      request.email = "test@example.com";
+
+      const error = new Error("Login failed");
+      mockLoginStrategyService.logIn.mockRejectedValue(error);
+
+      // Act
+      await component["handleRecoveryLogin"](request);
+
+      // Assert
+      expect(mockLogService.error).toHaveBeenCalledWith(
+        "Error logging in automatically: ",
+        error.message,
+      );
+      expect(mockRouter.navigate).toHaveBeenCalledWith(["/login"], {
+        queryParams: { email: request.email },
+      });
+    });
+  });
+});

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

@@ -18,6 +18,7 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service"
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import { ToastService } from "@bitwarden/components";
 import { KeyService } from "@bitwarden/key-management";
+import { NewDeviceVerificationNoticeService } from "@bitwarden/vault";
 
 @Component({
   selector: "app-recover-two-factor",
@@ -51,6 +52,7 @@ export class RecoverTwoFactorComponent implements OnInit {
     private configService: ConfigService,
     private loginSuccessHandlerService: LoginSuccessHandlerService,
     private logService: LogService,
+    private newDeviceVerificationNoticeService: NewDeviceVerificationNoticeService,
   ) {}
 
   async ngOnInit() {
@@ -134,6 +136,14 @@ export class RecoverTwoFactorComponent implements OnInit {
       });
 
       await this.loginSuccessHandlerService.run(authResult.userId);
+
+      // Before routing, set the state to skip the new device notification. This is a temporary
+      // fix and will be cleaned up in PM-18485.
+      await this.newDeviceVerificationNoticeService.updateNewDeviceVerificationSkipNoticeState(
+        authResult.userId,
+        true,
+      );
+
       await this.router.navigate(["/settings/security/two-factor"]);
     } catch (error) {
       // If login errors, redirect to login page per product. Don't show error
@@ -141,30 +151,4 @@ export class RecoverTwoFactorComponent implements OnInit {
       await this.router.navigate(["/login"], { queryParams: { email: request.email } });
     }
   }
-
-  /**
-   * Extracts an error message from the error object.
-   */
-  private extractErrorMessage(error: unknown): string {
-    let errorMessage: string = this.i18nService.t("unexpectedError");
-    if (error && typeof error === "object" && "validationErrors" in error) {
-      const validationErrors = error.validationErrors;
-      if (validationErrors && typeof validationErrors === "object") {
-        errorMessage = Object.keys(validationErrors)
-          .map((key) => {
-            const messages = (validationErrors as Record<string, string | string[]>)[key];
-            return Array.isArray(messages) ? messages.join(" ") : messages;
-          })
-          .join(" ");
-      }
-    } else if (
-      error &&
-      typeof error === "object" &&
-      "message" in error &&
-      typeof error.message === "string"
-    ) {
-      errorMessage = error.message;
-    }
-    return errorMessage;
-  }
 }

libs/angular/src/vault/guards/new-device-verification-notice.guard.spec.ts

@@ -36,6 +36,7 @@ describe("NewDeviceVerificationNoticeGuard", () => {
   const isSelfHost = jest.fn().mockReturnValue(false);
   const getProfileTwoFactorEnabled = jest.fn().mockResolvedValue(false);
   const noticeState$ = jest.fn().mockReturnValue(new BehaviorSubject(null));
+  const skipState$ = jest.fn().mockReturnValue(new BehaviorSubject(null));
   const getProfileCreationDate = jest.fn().mockResolvedValue(eightDaysAgo);
   const hasMasterPasswordAndMasterKeyHash = jest.fn().mockResolvedValue(true);
   const getUserSSOBound = jest.fn().mockResolvedValue(false);
@@ -50,12 +51,13 @@ describe("NewDeviceVerificationNoticeGuard", () => {
     hasMasterPasswordAndMasterKeyHash.mockClear();
     getUserSSOBound.mockClear();
     getUserSSOBoundAdminOwner.mockClear();
+    skipState$.mockClear();
 
     TestBed.configureTestingModule({
       providers: [
         { provide: Router, useValue: { createUrlTree } },
         { provide: ConfigService, useValue: { getFeatureFlag } },
-        { provide: NewDeviceVerificationNoticeService, useValue: { noticeState$ } },
+        { provide: NewDeviceVerificationNoticeService, useValue: { noticeState$, skipState$ } },
         { provide: AccountService, useValue: { activeAccount$ } },
         { provide: PlatformUtilsService, useValue: { isSelfHost } },
         { provide: UserVerificationService, useValue: { hasMasterPasswordAndMasterKeyHash } },
@@ -144,6 +146,14 @@ describe("NewDeviceVerificationNoticeGuard", () => {
     expect(await newDeviceGuard()).toBe(true);
   });
 
+  it("returns `true` when the skip state value is set to true", async () => {
+    skipState$.mockReturnValueOnce(new BehaviorSubject(true));
+
+    expect(await newDeviceGuard()).toBe(true);
+    expect(skipState$.mock.calls[0][0]).toBe("account-id");
+    expect(skipState$.mock.calls.length).toBe(1);
+  });
+
   describe("SSO bound", () => {
     beforeEach(() => {
       getFeatureFlag.mockImplementation((key) => {

libs/angular/src/vault/guards/new-device-verification-notice.guard.ts

@@ -44,6 +44,11 @@ export const NewDeviceVerificationNoticeGuard: CanActivateFn = async (
     return router.createUrlTree(["/login"]);
   }
 
+  // Currently used by the auth recovery login flow and will get cleaned up in PM-18485.
+  if (await firstValueFrom(newDeviceVerificationNoticeService.skipState$(currentAcct.id))) {
+    return true;
+  }
+
   try {
     const isSelfHosted = platformUtilsService.isSelfHost();
     const userIsSSOUser = await ssoAppliesToUser(

libs/vault/src/services/new-device-verification-notice.service.spec.ts

@@ -14,6 +14,7 @@ import {
   NewDeviceVerificationNoticeService,
   NewDeviceVerificationNotice,
   NEW_DEVICE_VERIFICATION_NOTICE_KEY,
+  SKIP_NEW_DEVICE_VERIFICATION_NOTICE,
 } from "./new-device-verification-notice.service";
 
 describe("New Device Verification Notice", () => {
@@ -21,6 +22,7 @@ describe("New Device Verification Notice", () => {
   const userId = Utils.newGuid() as UserId;
   let newDeviceVerificationService: NewDeviceVerificationNoticeService;
   let mockNoticeState: FakeSingleUserState<NewDeviceVerificationNotice>;
+  let mockSkipState: FakeSingleUserState<boolean>;
   let stateProvider: FakeStateProvider;
   let accountService: FakeAccountService;
 
@@ -28,6 +30,7 @@ describe("New Device Verification Notice", () => {
     accountService = mockAccountServiceWith(userId);
     stateProvider = new FakeStateProvider(accountService);
     mockNoticeState = stateProvider.singleUser.getFake(userId, NEW_DEVICE_VERIFICATION_NOTICE_KEY);
+    mockSkipState = stateProvider.singleUser.getFake(userId, SKIP_NEW_DEVICE_VERIFICATION_NOTICE);
     newDeviceVerificationService = new NewDeviceVerificationNoticeService(stateProvider);
   });
 
@@ -82,4 +85,28 @@ describe("New Device Verification Notice", () => {
       expect(result).toEqual(newState);
     });
   });
+
+  describe("skipNotice state", () => {
+    it("emits skip notice state", async () => {
+      const shouldSkip = true;
+      await stateProvider.setUserState(SKIP_NEW_DEVICE_VERIFICATION_NOTICE, shouldSkip, userId);
+
+      const result = await firstValueFrom(newDeviceVerificationService.skipState$(userId));
+
+      expect(result).toBe(shouldSkip);
+    });
+
+    it("should update the skip notice state", async () => {
+      const initialSkipState = false;
+      const updatedSkipState = true;
+      mockSkipState.nextState(initialSkipState);
+      await newDeviceVerificationService.updateNewDeviceVerificationSkipNoticeState(
+        userId,
+        updatedSkipState,
+      );
+
+      const result = await firstValueFrom(newDeviceVerificationService.skipState$(userId));
+      expect(result).toBe(updatedSkipState);
+    });
+  });
 });

libs/vault/src/services/new-device-verification-notice.service.ts

@@ -42,6 +42,15 @@ export const NEW_DEVICE_VERIFICATION_NOTICE_KEY =
     },
   );
 
+export const SKIP_NEW_DEVICE_VERIFICATION_NOTICE = new UserKeyDefinition<boolean>(
+  NEW_DEVICE_VERIFICATION_NOTICE,
+  "shouldSkip",
+  {
+    deserializer: (data: boolean) => data,
+    clearOn: ["logout"],
+  },
+);
+
 @Injectable()
 export class NewDeviceVerificationNoticeService {
   constructor(private stateProvider: StateProvider) {}
@@ -62,4 +71,19 @@ export class NewDeviceVerificationNoticeService {
       return { ...newState };
     });
   }
+
+  private skipState(userId: UserId): SingleUserState<boolean> {
+    return this.stateProvider.getUser(userId, SKIP_NEW_DEVICE_VERIFICATION_NOTICE);
+  }
+
+  skipState$(userId: UserId): Observable<boolean | null> {
+    return this.skipState(userId).state$;
+  }
+
+  async updateNewDeviceVerificationSkipNoticeState(
+    userId: UserId,
+    shouldSkip: boolean,
+  ): Promise<void> {
+    await this.skipState(userId).update(() => shouldSkip);
+  }
 }