[PM-21443] Require userId for KeyService's everHadUserKey$ (#14712)

* Require userId for KeyService's everHadUserKey$

* handle null active user in tdeDecryptionRequiredGuard

libs/angular/src/auth/guards/lock.guard.spec.ts

@@ -44,7 +44,7 @@ describe("lockGuard", () => {
 
     const keyService: MockProxy<KeyService> = mock<KeyService>();
     keyService.isLegacyUser.mockResolvedValue(setupParams.isLegacyUser);
-    keyService.everHadUserKey$ = of(setupParams.everHadUserKey);
+    keyService.everHadUserKey$.mockReturnValue(of(setupParams.everHadUserKey));
 
     const platformUtilService: MockProxy<PlatformUtilsService> = mock<PlatformUtilsService>();
     platformUtilService.getClientType.mockReturnValue(setupParams.clientType);

libs/angular/src/auth/guards/lock.guard.ts

@@ -84,7 +84,7 @@ export function lockGuard(): CanActivateFn {
     }
 
     // If authN user with TDE directly navigates to lock, reject that navigation
-    const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$);
+    const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$(activeUser.id));
     if (tdeEnabled && !everHadUserKey) {
       return false;
     }

libs/angular/src/auth/guards/redirect.guard.ts

@@ -2,8 +2,10 @@ import { inject } from "@angular/core";
 import { CanActivateFn, Router } from "@angular/router";
 import { firstValueFrom } from "rxjs";
 
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
+import { getUserId } from "@bitwarden/common/auth/services/account.service";
 import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { KeyService } from "@bitwarden/key-management";
@@ -33,6 +35,7 @@ export function redirectGuard(overrides: Partial<RedirectRoutes> = {}): CanActiv
     const authService = inject(AuthService);
     const keyService = inject(KeyService);
     const deviceTrustService = inject(DeviceTrustServiceAbstraction);
+    const accountService = inject(AccountService);
     const logService = inject(LogService);
     const router = inject(Router);
 
@@ -49,7 +52,8 @@ export function redirectGuard(overrides: Partial<RedirectRoutes> = {}): CanActiv
     // If locked, TDE is enabled, and the user hasn't decrypted yet, then redirect to the
     // login decryption options component.
     const tdeEnabled = await firstValueFrom(deviceTrustService.supportsDeviceTrust$);
-    const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$);
+    const userId = await firstValueFrom(accountService.activeAccount$.pipe(getUserId));
+    const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$(userId));
     if (authStatus === AuthenticationStatus.Locked && tdeEnabled && !everHadUserKey) {
       logService.info(
         "Sending user to TDE decryption options. AuthStatus is %s. TDE support is %s. Ever had user key is %s.",

libs/angular/src/auth/guards/tde-decryption-required.guard.spec.ts

@@ -0,0 +1,107 @@
+import { TestBed } from "@angular/core/testing";
+import { Router, provideRouter } from "@angular/router";
+import { mock } from "jest-mock-extended";
+import { BehaviorSubject, of } from "rxjs";
+
+import { EmptyComponent } from "@bitwarden/angular/platform/guard/feature-flag.guard.spec";
+import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service";
+import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
+import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
+import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction";
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { UserId } from "@bitwarden/common/types/guid";
+import { KeyService } from "@bitwarden/key-management";
+
+import { tdeDecryptionRequiredGuard } from "./tde-decryption-required.guard";
+
+describe("tdeDecryptionRequiredGuard", () => {
+  const activeUser: Account = {
+    id: "fake_user_id" as UserId,
+    email: "test@email.com",
+    emailVerified: true,
+    name: "Test User",
+  };
+
+  const setup = (
+    activeUser: Account | null,
+    authStatus: AuthenticationStatus | null = null,
+    tdeEnabled: boolean = false,
+    everHadUserKey: boolean = false,
+  ) => {
+    const accountService = mock<AccountService>();
+    const authService = mock<AuthService>();
+    const keyService = mock<KeyService>();
+    const deviceTrustService = mock<DeviceTrustServiceAbstraction>();
+    const logService = mock<LogService>();
+
+    accountService.activeAccount$ = new BehaviorSubject<Account | null>(activeUser);
+    if (authStatus !== null) {
+      authService.getAuthStatus.mockResolvedValue(authStatus);
+    }
+    keyService.everHadUserKey$.mockReturnValue(of(everHadUserKey));
+    deviceTrustService.supportsDeviceTrust$ = of(tdeEnabled);
+
+    const testBed = TestBed.configureTestingModule({
+      providers: [
+        { provide: AccountService, useValue: accountService },
+        { provide: AuthService, useValue: authService },
+        { provide: KeyService, useValue: keyService },
+        { provide: DeviceTrustServiceAbstraction, useValue: deviceTrustService },
+        { provide: LogService, useValue: logService },
+        provideRouter([
+          { path: "", component: EmptyComponent },
+          {
+            path: "protected-route",
+            component: EmptyComponent,
+            canActivate: [tdeDecryptionRequiredGuard()],
+          },
+        ]),
+      ],
+    });
+
+    return {
+      router: testBed.inject(Router),
+    };
+  };
+
+  it("redirects to root when the active account is null", async () => {
+    const { router } = setup(null, null);
+    await router.navigate(["protected-route"]);
+    expect(router.url).toBe("/");
+  });
+
+  test.each([AuthenticationStatus.Unlocked, AuthenticationStatus.LoggedOut])(
+    "redirects to root when the user isn't locked",
+    async (authStatus) => {
+      const { router } = setup(activeUser, authStatus);
+
+      await router.navigate(["protected-route"]);
+
+      expect(router.url).toBe("/");
+    },
+  );
+
+  it("redirects to root when TDE is not enabled", async () => {
+    const { router } = setup(activeUser, AuthenticationStatus.Locked, false, true);
+
+    await router.navigate(["protected-route"]);
+
+    expect(router.url).toBe("/");
+  });
+
+  it("redirects to root when user has had a user key", async () => {
+    const { router } = setup(activeUser, AuthenticationStatus.Locked, true, true);
+
+    await router.navigate(["protected-route"]);
+
+    expect(router.url).toBe("/");
+  });
+
+  it("allows access when user is locked, TDE is enabled, and user has never had a user key", async () => {
+    const { router } = setup(activeUser, AuthenticationStatus.Locked, true, false);
+
+    const result = await router.navigate(["protected-route"]);
+    expect(result).toBe(true);
+    expect(router.url).toBe("/protected-route");
+  });
+});

libs/angular/src/auth/guards/tde-decryption-required.guard.ts

@@ -5,8 +5,9 @@ import {
   RouterStateSnapshot,
   CanActivateFn,
 } from "@angular/router";
-import { firstValueFrom } from "rxjs";
+import { firstValueFrom, map } from "rxjs";
 
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
 import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction";
@@ -24,12 +25,18 @@ export function tdeDecryptionRequiredGuard(): CanActivateFn {
     const authService = inject(AuthService);
     const keyService = inject(KeyService);
     const deviceTrustService = inject(DeviceTrustServiceAbstraction);
+    const accountService = inject(AccountService);
     const logService = inject(LogService);
     const router = inject(Router);
 
+    const userId = await firstValueFrom(accountService.activeAccount$.pipe(map((a) => a?.id)));
+    if (userId == null) {
+      return router.createUrlTree(["/"]);
+    }
+
     const authStatus = await authService.getAuthStatus();
     const tdeEnabled = await firstValueFrom(deviceTrustService.supportsDeviceTrust$);
-    const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$);
+    const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$(userId));
 
     // We need to determine if we should bypass the decryption options and send the user to the vault.
     // The ONLY time that we want to send a user to the decryption options is when:

libs/angular/src/auth/guards/unauth.guard.spec.ts

@@ -2,7 +2,7 @@ import { TestBed } from "@angular/core/testing";
 import { Router } from "@angular/router";
 import { RouterTestingModule } from "@angular/router/testing";
 import { MockProxy, mock } from "jest-mock-extended";
-import { BehaviorSubject } from "rxjs";
+import { BehaviorSubject, of } from "rxjs";
 
 import { EmptyComponent } from "@bitwarden/angular/platform/guard/feature-flag.guard.spec";
 import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service";
@@ -43,7 +43,7 @@ describe("UnauthGuard", () => {
       authService.authStatusFor$.mockReturnValue(activeAccountStatusObservable);
     }
 
-    keyService.everHadUserKey$ = new BehaviorSubject<boolean>(everHadUserKey);
+    keyService.everHadUserKey$.mockReturnValue(of(everHadUserKey));
     deviceTrustService.supportsDeviceTrustByUserId$.mockReturnValue(
       new BehaviorSubject<boolean>(tdeEnabled),
     );

libs/angular/src/auth/guards/unauth.guard.ts

@@ -50,7 +50,7 @@ async function unauthGuard(
   const tdeEnabled = await firstValueFrom(
     deviceTrustService.supportsDeviceTrustByUserId$(activeUser.id),
   );
-  const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$);
+  const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$(activeUser.id));
 
   // If locked, TDE is enabled, and the user hasn't decrypted yet, then redirect to the
   // login decryption options component.

libs/key-management/src/abstractions/key.service.ts

@@ -85,11 +85,13 @@ export abstract class KeyService {
    * (such as auto, biometrics, or pin)
    */
   abstract refreshAdditionalKeys(): Promise<void>;
+
   /**
-   * Observable value that returns whether or not the currently active user has ever had auser key,
+   * Observable value that returns whether or not the user has ever had a userKey,
    * i.e. has ever been unlocked/decrypted. This is key for differentiating between TDE locked and standard locked states.
    */
-  abstract everHadUserKey$: Observable<boolean>;
+  abstract everHadUserKey$(userId: UserId): Observable<boolean>;
+
   /**
    * Retrieves the user key
    * @param userId The desired user

libs/key-management/src/key.service.spec.ts

@@ -34,7 +34,6 @@ import {
   FakeAccountService,
   mockAccountServiceWith,
   FakeStateProvider,
-  FakeActiveUserState,
   FakeSingleUserState,
 } from "@bitwarden/common/spec";
 import { CsprngArray } from "@bitwarden/common/types/csprng";
@@ -190,28 +189,28 @@ describe("keyService", () => {
   });
 
   describe("everHadUserKey$", () => {
-    let everHadUserKeyState: FakeActiveUserState<boolean>;
+    let everHadUserKeyState: FakeSingleUserState<boolean>;
 
     beforeEach(() => {
-      everHadUserKeyState = stateProvider.activeUser.getFake(USER_EVER_HAD_USER_KEY);
+      everHadUserKeyState = stateProvider.singleUser.getFake(mockUserId, USER_EVER_HAD_USER_KEY);
     });
 
     it("should return true when stored value is true", async () => {
       everHadUserKeyState.nextState(true);
 
-      expect(await firstValueFrom(keyService.everHadUserKey$)).toBe(true);
+      expect(await firstValueFrom(keyService.everHadUserKey$(mockUserId))).toBe(true);
     });
 
     it("should return false when stored value is false", async () => {
       everHadUserKeyState.nextState(false);
 
-      expect(await firstValueFrom(keyService.everHadUserKey$)).toBe(false);
+      expect(await firstValueFrom(keyService.everHadUserKey$(mockUserId))).toBe(false);
     });
 
     it("should return false when stored value is null", async () => {
       everHadUserKeyState.nextState(null);
 
-      expect(await firstValueFrom(keyService.everHadUserKey$)).toBe(false);
+      expect(await firstValueFrom(keyService.everHadUserKey$(mockUserId))).toBe(false);
     });
   });
 

libs/key-management/src/key.service.ts

@@ -41,7 +41,7 @@ import {
   USER_EVER_HAD_USER_KEY,
   USER_KEY,
 } from "@bitwarden/common/platform/services/key-state/user-key.state";
-import { ActiveUserState, StateProvider } from "@bitwarden/common/platform/state";
+import { StateProvider } from "@bitwarden/common/platform/state";
 import { CsprngArray } from "@bitwarden/common/types/csprng";
 import { OrganizationId, ProviderId, UserId } from "@bitwarden/common/types/guid";
 import {
@@ -63,10 +63,6 @@ import {
 import { KdfConfig } from "./models/kdf-config";
 
 export class DefaultKeyService implements KeyServiceAbstraction {
-  private readonly activeUserEverHadUserKey: ActiveUserState<boolean>;
-
-  readonly everHadUserKey$: Observable<boolean>;
-
   readonly activeUserOrgKeys$: Observable<Record<OrganizationId, OrgKey>>;
 
   constructor(
@@ -82,10 +78,6 @@ export class DefaultKeyService implements KeyServiceAbstraction {
     protected stateProvider: StateProvider,
     protected kdfConfigService: KdfConfigService,
   ) {
-    // User Key
-    this.activeUserEverHadUserKey = stateProvider.getActive(USER_EVER_HAD_USER_KEY);
-    this.everHadUserKey$ = this.activeUserEverHadUserKey.state$.pipe(map((x) => x ?? false));
-
     this.activeUserOrgKeys$ = this.stateProvider.activeUserId$.pipe(
       switchMap((userId) => (userId != null ? this.orgKeys$(userId) : NEVER)),
     ) as Observable<Record<OrganizationId, OrgKey>>;
@@ -141,6 +133,12 @@ export class DefaultKeyService implements KeyServiceAbstraction {
     await this.setUserKey(key, activeUserId);
   }
 
+  everHadUserKey$(userId: UserId): Observable<boolean> {
+    return this.stateProvider
+      .getUser(userId, USER_EVER_HAD_USER_KEY)
+      .state$.pipe(map((x) => x ?? false));
+  }
+
   getInMemoryUserKeyFor$(userId: UserId): Observable<UserKey> {
     return this.stateProvider.getUserState$(USER_KEY, userId);
   }