Add state for everHadUserKey (#7208)

* Migrate ever had user key * Add DI for state providers * Add state for everHadUserKey * Use ever had user key migrator Co-authored-by: SmithThe4th <gsmithwalter@gmail.com> Co-authored-by: Carlos Gonçalves <LRNcardozoWDF@users.noreply.github.com> Co-authored-by: Jason Ng <Jcory.ng@gmail.com> * Fix test from merge * Prefer stored observables to getters getters create a new observable every time they're called, whereas one set in the constructor is created only once. * Fix another merge issue * Fix cli background build --------- Co-authored-by: SmithThe4th <gsmithwalter@gmail.com> Co-authored-by: Carlos Gonçalves <LRNcardozoWDF@users.noreply.github.com> Co-authored-by: Jason Ng <Jcory.ng@gmail.com>
2025-12-14 15:23:33 +00:00 · 2024-01-10 11:51:45 -05:00
parent 211d7a2626
commit 46a3834f46
21 changed files with 404 additions and 100 deletions
--- a/libs/common/src/platform/abstractions/crypto.service.ts
+++ b/libs/common/src/platform/abstractions/crypto.service.ts
@@ -1,3 +1,5 @@
+import { Observable } from "rxjs";
+
 import { ProfileOrganizationResponse } from "../../admin-console/models/response/profile-organization.response";
 import { ProfileProviderOrganizationResponse } from "../../admin-console/models/response/profile-provider-organization.response";
 import { ProfileProviderResponse } from "../../admin-console/models/response/profile-provider.response";
@@ -29,14 +31,12 @@ export abstract class CryptoService {
   * kicking off a refresh of any additional keys
   * (such as auto, biometrics, or pin)
   */
-  /**
-   * Check if the current sessions has ever had a user key, i.e. has ever been unlocked/decrypted.
-   * This is key for differentiating between TDE locked and standard locked states.
-   * @param userId The desired user
-   * @returns True if the current session has ever had a user key
-   */
-  getEverHadUserKey: (userId?: string) => Promise<boolean>;
  refreshAdditionalKeys: () => Promise<void>;
+  /**
+   * Observable value that returns whether or not the currently active user has ever had auser key,
+   * i.e. has ever been unlocked/decrypted. This is key for differentiating between TDE locked and standard locked states.
+   */
+  everHadUserKey$: Observable<boolean>;
  /**
   * Retrieves the user key
   * @param userId The desired user
--- a/libs/common/src/platform/abstractions/state.service.ts
+++ b/libs/common/src/platform/abstractions/state.service.ts
@@ -393,8 +393,6 @@ export abstract class StateService<T extends Account = Account> {
  setEquivalentDomains: (value: string, options?: StorageOptions) => Promise<void>;
  getEventCollection: (options?: StorageOptions) => Promise<EventData[]>;
  setEventCollection: (value: EventData[], options?: StorageOptions) => Promise<void>;
-  getEverHadUserKey: (options?: StorageOptions) => Promise<boolean>;
-  setEverHadUserKey: (value: boolean, options?: StorageOptions) => Promise<void>;
  getEverBeenUnlocked: (options?: StorageOptions) => Promise<boolean>;
  setEverBeenUnlocked: (value: boolean, options?: StorageOptions) => Promise<void>;
  getForceSetPasswordReason: (options?: StorageOptions) => Promise<ForceSetPasswordReason>;
--- a/libs/common/src/platform/models/domain/account.ts
+++ b/libs/common/src/platform/models/domain/account.ts
@@ -207,7 +207,6 @@ export class AccountProfile {
  emailVerified?: boolean;
  entityId?: string;
  entityType?: string;
-  everHadUserKey?: boolean;
  everBeenUnlocked?: boolean;
  forceSetPasswordReason?: ForceSetPasswordReason;
  hasPremiumPersonally?: boolean;
--- a/libs/common/src/platform/services/crypto.service.spec.ts
+++ b/libs/common/src/platform/services/crypto.service.spec.ts
@@ -1,11 +1,17 @@
-import { mock, mockReset } from "jest-mock-extended";
+import { mock } from "jest-mock-extended";
+import { firstValueFrom } from "rxjs";

+import { FakeAccountService, mockAccountServiceWith } from "../../../spec/fake-account-service";
+import { FakeActiveUserState, FakeSingleUserState } from "../../../spec/fake-state";
+import { FakeStateProvider } from "../../../spec/fake-state-provider";
 import { CsprngArray } from "../../types/csprng";
+import { UserId } from "../../types/guid";
 import { CryptoFunctionService } from "../abstractions/crypto-function.service";
 import { EncryptService } from "../abstractions/encrypt.service";
 import { LogService } from "../abstractions/log.service";
 import { PlatformUtilsService } from "../abstractions/platform-utils.service";
 import { StateService } from "../abstractions/state.service";
+import { Utils } from "../misc/utils";
 import { EncString } from "../models/domain/enc-string";
 import {
  MasterKey,
@@ -13,7 +19,7 @@ import {
  SymmetricCryptoKey,
  UserKey,
 } from "../models/domain/symmetric-crypto-key";
-import { CryptoService } from "../services/crypto.service";
+import { CryptoService, USER_EVER_HAD_USER_KEY } from "../services/crypto.service";

 describe("cryptoService", () => {
  let cryptoService: CryptoService;
@@ -23,15 +29,14 @@ describe("cryptoService", () => {
  const platformUtilService = mock<PlatformUtilsService>();
  const logService = mock<LogService>();
  const stateService = mock<StateService>();
+  let stateProvider: FakeStateProvider;

-  const mockUserId = "mock user id";
+  const mockUserId = Utils.newGuid() as UserId;
+  let accountService: FakeAccountService;

  beforeEach(() => {
-    mockReset(cryptoFunctionService);
-    mockReset(encryptService);
-    mockReset(platformUtilService);
-    mockReset(logService);
-    mockReset(stateService);
+    accountService = mockAccountServiceWith(mockUserId);
+    stateProvider = new FakeStateProvider(accountService);

    cryptoService = new CryptoService(
      cryptoFunctionService,
@@ -39,9 +44,15 @@ describe("cryptoService", () => {
      platformUtilService,
      logService,
      stateService,
+      accountService,
+      stateProvider,
    );
  });

+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
  it("instantiates", () => {
    expect(cryptoService).not.toBeFalsy();
  });
@@ -117,12 +128,49 @@ describe("cryptoService", () => {
    });
  });

+  describe("everHadUserKey$", () => {
+    let everHadUserKeyState: FakeActiveUserState<boolean>;
+
+    beforeEach(() => {
+      everHadUserKeyState = stateProvider.activeUser.getFake(USER_EVER_HAD_USER_KEY);
+    });
+
+    it("should return true when stored value is true", async () => {
+      everHadUserKeyState.nextState(true);
+
+      expect(await firstValueFrom(cryptoService.everHadUserKey$)).toBe(true);
+    });
+
+    it("should return false when stored value is false", async () => {
+      everHadUserKeyState.nextState(false);
+
+      expect(await firstValueFrom(cryptoService.everHadUserKey$)).toBe(false);
+    });
+
+    it("should return false when stored value is null", async () => {
+      everHadUserKeyState.nextState(null);
+
+      expect(await firstValueFrom(cryptoService.everHadUserKey$)).toBe(false);
+    });
+  });
+
  describe("setUserKey", () => {
    let mockUserKey: UserKey;
+    let everHadUserKeyState: FakeSingleUserState<boolean>;

    beforeEach(() => {
      const mockRandomBytes = new Uint8Array(64) as CsprngArray;
      mockUserKey = new SymmetricCryptoKey(mockRandomBytes) as UserKey;
+      everHadUserKeyState = stateProvider.singleUser.getFake(mockUserId, USER_EVER_HAD_USER_KEY);
+
+      // Initialize storage
+      everHadUserKeyState.nextState(null);
+    });
+
+    it("should set everHadUserKey if key is not null to true", async () => {
+      await cryptoService.setUserKey(mockUserKey, mockUserId);
+
+      expect(await firstValueFrom(everHadUserKeyState.state$)).toBe(true);
    });

    describe("Auto Key refresh", () => {
--- a/libs/common/src/platform/services/crypto.service.ts
+++ b/libs/common/src/platform/services/crypto.service.ts
@@ -1,12 +1,15 @@
 import * as bigInt from "big-integer";
+import { firstValueFrom, map } from "rxjs";

 import { EncryptedOrganizationKeyData } from "../../admin-console/models/data/encrypted-organization-key.data";
 import { BaseEncryptedOrganizationKey } from "../../admin-console/models/domain/encrypted-organization-key";
 import { ProfileOrganizationResponse } from "../../admin-console/models/response/profile-organization.response";
 import { ProfileProviderOrganizationResponse } from "../../admin-console/models/response/profile-provider-organization.response";
 import { ProfileProviderResponse } from "../../admin-console/models/response/profile-provider.response";
+import { AccountService } from "../../auth/abstractions/account.service";
 import { KdfConfig } from "../../auth/models/domain/kdf-config";
 import { Utils } from "../../platform/misc/utils";
+import { UserId } from "../../types/guid";
 import { CryptoFunctionService } from "../abstractions/crypto-function.service";
 import { CryptoService as CryptoServiceAbstraction } from "../abstractions/crypto.service";
 import { EncryptService } from "../abstractions/encrypt.service";
@@ -36,34 +39,48 @@ import {
  SymmetricCryptoKey,
  UserKey,
 } from "../models/domain/symmetric-crypto-key";
+import { ActiveUserState, CRYPTO_DISK, KeyDefinition, StateProvider } from "../state";
+
+export const USER_EVER_HAD_USER_KEY = new KeyDefinition<boolean>(CRYPTO_DISK, "everHadUserKey", {
+  deserializer: (obj) => obj,
+});

 export class CryptoService implements CryptoServiceAbstraction {
+  private activeUserEverHadUserKey: ActiveUserState<boolean>;
+
+  readonly everHadUserKey$;
+
  constructor(
    protected cryptoFunctionService: CryptoFunctionService,
    protected encryptService: EncryptService,
    protected platformUtilService: PlatformUtilsService,
    protected logService: LogService,
    protected stateService: StateService,
-  ) {}
+    protected accountService: AccountService,
+    protected stateProvider: StateProvider,
+  ) {
+    this.activeUserEverHadUserKey = stateProvider.getActive(USER_EVER_HAD_USER_KEY);

-  async setUserKey(key: UserKey, userId?: string): Promise<void> {
+    this.everHadUserKey$ = this.activeUserEverHadUserKey.state$.pipe(map((x) => x ?? false));
+  }
+
+  async setUserKey(key: UserKey, userId?: UserId): Promise<void> {
+    // TODO: make this non-nullable in signature
+    userId ??= (await firstValueFrom(this.accountService.activeAccount$))?.id;
    if (key != null) {
-      await this.stateService.setEverHadUserKey(true, { userId: userId });
+      // Key should never be null anyway
+      this.stateProvider.getUser(userId, USER_EVER_HAD_USER_KEY).update(() => true);
    }
    await this.stateService.setUserKey(key, { userId: userId });
    await this.storeAdditionalKeys(key, userId);
  }

-  async getEverHadUserKey(userId?: string): Promise<boolean> {
-    return await this.stateService.getEverHadUserKey({ userId: userId });
-  }
-
  async refreshAdditionalKeys(): Promise<void> {
    const key = await this.getUserKey();
    await this.setUserKey(key);
  }

-  async getUserKey(userId?: string): Promise<UserKey> {
+  async getUserKey(userId?: UserId): Promise<UserKey> {
    let userKey = await this.stateService.getUserKey({ userId: userId });
    if (userKey) {
      return userKey;
@@ -79,13 +96,13 @@ export class CryptoService implements CryptoServiceAbstraction {
    }
  }

-  async isLegacyUser(masterKey?: MasterKey, userId?: string): Promise<boolean> {
+  async isLegacyUser(masterKey?: MasterKey, userId?: UserId): Promise<boolean> {
    return await this.validateUserKey(
      (masterKey ?? (await this.getMasterKey(userId))) as unknown as UserKey,
    );
  }

-  async getUserKeyWithLegacySupport(userId?: string): Promise<UserKey> {
+  async getUserKeyWithLegacySupport(userId?: UserId): Promise<UserKey> {
    const userKey = await this.getUserKey(userId);
    if (userKey) {
      return userKey;
@@ -96,7 +113,7 @@ export class CryptoService implements CryptoServiceAbstraction {
    return (await this.getMasterKey(userId)) as unknown as UserKey;
  }

-  async getUserKeyFromStorage(keySuffix: KeySuffixOptions, userId?: string): Promise<UserKey> {
+  async getUserKeyFromStorage(keySuffix: KeySuffixOptions, userId?: UserId): Promise<UserKey> {
    const userKey = await this.getKeyFromStorage(keySuffix, userId);
    if (userKey) {
      if (!(await this.validateUserKey(userKey))) {
@@ -113,11 +130,11 @@ export class CryptoService implements CryptoServiceAbstraction {
    );
  }

-  async hasUserKeyInMemory(userId?: string): Promise<boolean> {
+  async hasUserKeyInMemory(userId?: UserId): Promise<boolean> {
    return (await this.stateService.getUserKey({ userId: userId })) != null;
  }

-  async hasUserKeyStored(keySuffix: KeySuffixOptions, userId?: string): Promise<boolean> {
+  async hasUserKeyStored(keySuffix: KeySuffixOptions, userId?: UserId): Promise<boolean> {
    return (await this.getKeyFromStorage(keySuffix, userId)) != null;
  }

@@ -131,14 +148,14 @@ export class CryptoService implements CryptoServiceAbstraction {
    return this.buildProtectedSymmetricKey(masterKey, newUserKey);
  }

-  async clearUserKey(clearStoredKeys = true, userId?: string): Promise<void> {
+  async clearUserKey(clearStoredKeys = true, userId?: UserId): Promise<void> {
    await this.stateService.setUserKey(null, { userId: userId });
    if (clearStoredKeys) {
      await this.clearAllStoredUserKeys(userId);
    }
  }

-  async clearStoredUserKey(keySuffix: KeySuffixOptions, userId?: string): Promise<void> {
+  async clearStoredUserKey(keySuffix: KeySuffixOptions, userId?: UserId): Promise<void> {
    if (keySuffix === KeySuffixOptions.Auto) {
      this.stateService.setUserKeyAutoUnlock(null, { userId: userId });
      this.clearDeprecatedKeys(KeySuffixOptions.Auto, userId);
@@ -149,15 +166,15 @@ export class CryptoService implements CryptoServiceAbstraction {
    }
  }

-  async setMasterKeyEncryptedUserKey(userKeyMasterKey: string, userId?: string): Promise<void> {
+  async setMasterKeyEncryptedUserKey(userKeyMasterKey: string, userId?: UserId): Promise<void> {
    await this.stateService.setMasterKeyEncryptedUserKey(userKeyMasterKey, { userId: userId });
  }

-  async setMasterKey(key: MasterKey, userId?: string): Promise<void> {
+  async setMasterKey(key: MasterKey, userId?: UserId): Promise<void> {
    await this.stateService.setMasterKey(key, { userId: userId });
  }

-  async getMasterKey(userId?: string): Promise<MasterKey> {
+  async getMasterKey(userId?: UserId): Promise<MasterKey> {
    let masterKey = await this.stateService.getMasterKey({ userId: userId });
    if (!masterKey) {
      masterKey = (await this.stateService.getCryptoMasterKey({ userId: userId })) as MasterKey;
@@ -170,7 +187,7 @@ export class CryptoService implements CryptoServiceAbstraction {
    return masterKey;
  }

-  async getOrDeriveMasterKey(password: string, userId?: string) {
+  async getOrDeriveMasterKey(password: string, userId?: UserId) {
    let masterKey = await this.getMasterKey(userId);
    return (masterKey ||= await this.makeMasterKey(
      password,
@@ -195,7 +212,7 @@ export class CryptoService implements CryptoServiceAbstraction {
    return (await this.makeKey(password, email, kdf, KdfConfig)) as MasterKey;
  }

-  async clearMasterKey(userId?: string): Promise<void> {
+  async clearMasterKey(userId?: UserId): Promise<void> {
    await this.stateService.setMasterKey(null, { userId: userId });
  }

@@ -210,7 +227,7 @@ export class CryptoService implements CryptoServiceAbstraction {
  async decryptUserKeyWithMasterKey(
    masterKey: MasterKey,
    userKey?: EncString,
-    userId?: string,
+    userId?: UserId,
  ): Promise<UserKey> {
    masterKey ||= await this.getMasterKey(userId);
    if (masterKey == null) {
@@ -275,7 +292,7 @@ export class CryptoService implements CryptoServiceAbstraction {
    return await this.stateService.getKeyHash();
  }

-  async clearMasterKeyHash(userId?: string): Promise<void> {
+  async clearMasterKeyHash(userId?: UserId): Promise<void> {
    return await this.stateService.setKeyHash(null, { userId: userId });
  }

@@ -389,7 +406,7 @@ export class CryptoService implements CryptoServiceAbstraction {
    return this.buildProtectedSymmetricKey(key, newSymKey);
  }

-  async clearOrgKeys(memoryOnly?: boolean, userId?: string): Promise<void> {
+  async clearOrgKeys(memoryOnly?: boolean, userId?: UserId): Promise<void> {
    await this.stateService.setDecryptedOrganizationKeys(null, { userId: userId });
    if (!memoryOnly) {
      await this.stateService.setEncryptedOrganizationKeys(null, { userId: userId });
@@ -452,7 +469,7 @@ export class CryptoService implements CryptoServiceAbstraction {
    return providerKeys;
  }

-  async clearProviderKeys(memoryOnly?: boolean, userId?: string): Promise<void> {
+  async clearProviderKeys(memoryOnly?: boolean, userId?: UserId): Promise<void> {
    await this.stateService.setDecryptedProviderKeys(null, { userId: userId });
    if (!memoryOnly) {
      await this.stateService.setEncryptedProviderKeys(null, { userId: userId });
@@ -537,7 +554,7 @@ export class CryptoService implements CryptoServiceAbstraction {
    return [publicB64, privateEnc];
  }

-  async clearKeyPair(memoryOnly?: boolean, userId?: string): Promise<void[]> {
+  async clearKeyPair(memoryOnly?: boolean, userId?: UserId): Promise<void[]> {
    const keysToClear: Promise<void>[] = [
      this.stateService.setDecryptedPrivateKey(null, { userId: userId }),
      this.stateService.setPublicKey(null, { userId: userId }),
@@ -553,7 +570,7 @@ export class CryptoService implements CryptoServiceAbstraction {
    return (await this.stretchKey(pinKey)) as PinKey;
  }

-  async clearPinKeys(userId?: string): Promise<void> {
+  async clearPinKeys(userId?: UserId): Promise<void> {
    await this.stateService.setPinKeyEncryptedUserKey(null, { userId: userId });
    await this.stateService.setPinKeyEncryptedUserKeyEphemeral(null, { userId: userId });
    await this.stateService.setProtectedPin(null, { userId: userId });
@@ -613,7 +630,7 @@ export class CryptoService implements CryptoServiceAbstraction {
    return new SymmetricCryptoKey(randomBytes) as CipherKey;
  }

-  async clearKeys(userId?: string): Promise<any> {
+  async clearKeys(userId?: UserId): Promise<any> {
    await this.clearUserKey(true, userId);
    await this.clearMasterKeyHash(userId);
    await this.clearOrgKeys(false, userId);
@@ -776,7 +793,7 @@ export class CryptoService implements CryptoServiceAbstraction {
   * @param key The user key
   * @param userId The desired user
   */
-  protected async storeAdditionalKeys(key: UserKey, userId?: string) {
+  protected async storeAdditionalKeys(key: UserKey, userId?: UserId) {
    const storeAuto = await this.shouldStoreKey(KeySuffixOptions.Auto, userId);
    if (storeAuto) {
      await this.stateService.setUserKeyAutoUnlock(key.keyB64, { userId: userId });
@@ -802,7 +819,7 @@ export class CryptoService implements CryptoServiceAbstraction {
   * ephemeral version.
   * @param key The user key
   */
-  protected async storePinKey(key: UserKey, userId?: string) {
+  protected async storePinKey(key: UserKey, userId?: UserId) {
    const pin = await this.encryptService.decryptToUtf8(
      new EncString(await this.stateService.getProtectedPin({ userId: userId })),
      key,
@@ -822,7 +839,7 @@ export class CryptoService implements CryptoServiceAbstraction {
    }
  }

-  protected async shouldStoreKey(keySuffix: KeySuffixOptions, userId?: string) {
+  protected async shouldStoreKey(keySuffix: KeySuffixOptions, userId?: UserId) {
    let shouldStoreKey = false;
    switch (keySuffix) {
      case KeySuffixOptions.Auto: {
@@ -841,7 +858,7 @@ export class CryptoService implements CryptoServiceAbstraction {

  protected async getKeyFromStorage(
    keySuffix: KeySuffixOptions,
-    userId?: string,
+    userId?: UserId,
  ): Promise<UserKey> {
    if (keySuffix === KeySuffixOptions.Auto) {
      const userKey = await this.stateService.getUserKeyAutoUnlock({ userId: userId });
@@ -889,7 +906,7 @@ export class CryptoService implements CryptoServiceAbstraction {
    }
  }

-  protected async clearAllStoredUserKeys(userId?: string): Promise<void> {
+  protected async clearAllStoredUserKeys(userId?: UserId): Promise<void> {
    await this.stateService.setUserKeyAutoUnlock(null, { userId: userId });
    await this.stateService.setPinKeyEncryptedUserKeyEphemeral(null, { userId: userId });
  }
@@ -984,7 +1001,7 @@ export class CryptoService implements CryptoServiceAbstraction {
  // These methods support migrating the old keys to the new ones.
  // TODO: Remove after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3475)

-  async clearDeprecatedKeys(keySuffix: KeySuffixOptions, userId?: string) {
+  async clearDeprecatedKeys(keySuffix: KeySuffixOptions, userId?: UserId) {
    if (keySuffix === KeySuffixOptions.Auto) {
      await this.stateService.setCryptoMasterKeyAuto(null, { userId: userId });
    } else if (keySuffix === KeySuffixOptions.Pin) {
@@ -993,7 +1010,7 @@ export class CryptoService implements CryptoServiceAbstraction {
    }
  }

-  async migrateAutoKeyIfNeeded(userId?: string) {
+  async migrateAutoKeyIfNeeded(userId?: UserId) {
    const oldAutoKey = await this.stateService.getCryptoMasterKeyAuto({ userId: userId });
    if (!oldAutoKey) {
      return;
--- a/libs/common/src/platform/services/state.service.ts
+++ b/libs/common/src/platform/services/state.service.ts
@@ -2093,24 +2093,6 @@ export class StateService<
    );
  }

-  async getEverHadUserKey(options?: StorageOptions): Promise<boolean> {
-    return (
-      (await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions())))
-        ?.profile?.everHadUserKey ?? false
-    );
-  }
-
-  async setEverHadUserKey(value: boolean, options?: StorageOptions): Promise<void> {
-    const account = await this.getAccount(
-      this.reconcileOptions(options, await this.defaultOnDiskOptions()),
-    );
-    account.profile.everHadUserKey = value;
-    await this.saveAccount(
-      account,
-      this.reconcileOptions(options, await this.defaultOnDiskOptions()),
-    );
-  }
-
  async getEverBeenUnlocked(options?: StorageOptions): Promise<boolean> {
    return (
      (await this.getAccount(this.reconcileOptions(options, await this.defaultInMemoryOptions())))
--- a/libs/common/src/platform/state/state-definitions.ts
+++ b/libs/common/src/platform/state/state-definitions.ts
@@ -18,3 +18,5 @@ import { StateDefinition } from "./state-definition";
 */

 export const ACCOUNT_MEMORY = new StateDefinition("account", "memory");
+
+export const CRYPTO_DISK = new StateDefinition("crypto", "disk");