[PM-5362] Add MP Service (attempt #2) (#8619)

* create mp and kdf service

* update mp service interface to not rely on active user

* rename observable methods

* update crypto service with new MP service

* add master password service to login strategies
- make fake service for easier testing
- fix crypto service tests

* update auth service and finish strategies

* auth request refactors

* more service refactors and constructor updates

* setMasterKey refactors

* remove master key methods from crypto service

* remove master key and hash from state service

* missed fixes

* create migrations and fix references

* fix master key imports

* default force set password reason to none

* add password reset reason observable factory to service

* remove kdf changes and migrate only disk data

* update migration number

* fix sync service deps

* use disk for force set password state

* fix desktop migration

* fix sso test

* fix tests

* fix more tests

* fix even more tests

* fix even more tests

* fix cli

* remove kdf service abstraction

* add missing deps for browser

* fix merge conflicts

* clear reset password reason on lock or logout

* fix tests

* fix other tests

* add jsdocs to abstraction

* use state provider in crypto service

* inverse master password service factory

* add clearOn to master password service

* add parameter validation to master password service

* add component level userId

* add missed userId

* migrate key hash

* fix login strategy service

* delete crypto master key from account

* migrate master key encrypted user key

* rename key hash to master key hash

* use mp service for getMasterKeyEncryptedUserKey

* fix tests

* fix user key decryption logic

* add clear methods to mp service

* fix circular dep and encryption issue

* fix test

* remove extra account service call

* use EncString in state provider

* fix tests

* return to using encrypted string for serialization

libs/common/src/auth/abstractions/master-password.service.abstraction.ts

@@ -0,0 +1,82 @@
+import { Observable } from "rxjs";
+
+import { EncString } from "../../platform/models/domain/enc-string";
+import { UserId } from "../../types/guid";
+import { MasterKey } from "../../types/key";
+import { ForceSetPasswordReason } from "../models/domain/force-set-password-reason";
+
+export abstract class MasterPasswordServiceAbstraction {
+  /**
+   * An observable that emits if the user is being forced to set a password on login and why.
+   * @param userId The user ID.
+   * @throws If the user ID is missing.
+   */
+  abstract forceSetPasswordReason$: (userId: UserId) => Observable<ForceSetPasswordReason>;
+  /**
+   * An observable that emits the master key for the user.
+   * @param userId The user ID.
+   * @throws If the user ID is missing.
+   */
+  abstract masterKey$: (userId: UserId) => Observable<MasterKey>;
+  /**
+   * An observable that emits the master key hash for the user.
+   * @param userId The user ID.
+   * @throws If the user ID is missing.
+   */
+  abstract masterKeyHash$: (userId: UserId) => Observable<string>;
+  /**
+   * Returns the master key encrypted user key for the user.
+   * @param userId The user ID.
+   * @throws If the user ID is missing.
+   */
+  abstract getMasterKeyEncryptedUserKey: (userId: UserId) => Promise<EncString>;
+}
+
+export abstract class InternalMasterPasswordServiceAbstraction extends MasterPasswordServiceAbstraction {
+  /**
+   * Set the master key for the user.
+   * Note: Use {@link clearMasterKey} to clear the master key.
+   * @param masterKey The master key.
+   * @param userId The user ID.
+   * @throws If the user ID or master key is missing.
+   */
+  abstract setMasterKey: (masterKey: MasterKey, userId: UserId) => Promise<void>;
+  /**
+   * Clear the master key for the user.
+   * @param userId The user ID.
+   * @throws If the user ID is missing.
+   */
+  abstract clearMasterKey: (userId: UserId) => Promise<void>;
+  /**
+   * Set the master key hash for the user.
+   * Note: Use {@link clearMasterKeyHash} to clear the master key hash.
+   * @param masterKeyHash The master key hash.
+   * @param userId The user ID.
+   * @throws If the user ID or master key hash is missing.
+   */
+  abstract setMasterKeyHash: (masterKeyHash: string, userId: UserId) => Promise<void>;
+  /**
+   * Clear the master key hash for the user.
+   * @param userId The user ID.
+   * @throws If the user ID is missing.
+   */
+  abstract clearMasterKeyHash: (userId: UserId) => Promise<void>;
+
+  /**
+   * Set the master key encrypted user key for the user.
+   * @param encryptedKey The master key encrypted user key.
+   * @param userId The user ID.
+   * @throws If the user ID or encrypted key is missing.
+   */
+  abstract setMasterKeyEncryptedUserKey: (encryptedKey: EncString, userId: UserId) => Promise<void>;
+  /**
+   * Set the force set password reason for the user.
+   * @param reason The reason the user is being forced to set a password.
+   * @param userId The user ID.
+   * @throws If the user ID or reason is missing.
+   */
+  abstract setForceSetPasswordReason: (
+    reason: ForceSetPasswordReason,
+    userId: UserId,
+  ) => Promise<void>;
+}

libs/common/src/auth/services/key-connector.service.spec.ts

@@ -21,6 +21,7 @@ import {
   CONVERT_ACCOUNT_TO_KEY_CONNECTOR,
   KeyConnectorService,
 } from "./key-connector.service";
+import { FakeMasterPasswordService } from "./master-password/fake-master-password.service";
 import { TokenService } from "./token.service";
 
 describe("KeyConnectorService", () => {
@@ -36,6 +37,7 @@ describe("KeyConnectorService", () => {
   let stateProvider: FakeStateProvider;
 
   let accountService: FakeAccountService;
+  let masterPasswordService: FakeMasterPasswordService;
 
   const mockUserId = Utils.newGuid() as UserId;
   const mockOrgId = Utils.newGuid() as OrganizationId;
@@ -47,10 +49,13 @@ describe("KeyConnectorService", () => {
   beforeEach(() => {
     jest.clearAllMocks();
 
+    masterPasswordService = new FakeMasterPasswordService();
     accountService = mockAccountServiceWith(mockUserId);
     stateProvider = new FakeStateProvider(accountService);
 
     keyConnectorService = new KeyConnectorService(
+      accountService,
+      masterPasswordService,
       cryptoService,
       apiService,
       tokenService,
@@ -214,7 +219,10 @@ describe("KeyConnectorService", () => {
 
       // Assert
       expect(apiService.getMasterKeyFromKeyConnector).toHaveBeenCalledWith(url);
-      expect(cryptoService.setMasterKey).toHaveBeenCalledWith(masterKey);
+      expect(masterPasswordService.mock.setMasterKey).toHaveBeenCalledWith(
+        masterKey,
+        expect.any(String),
+      );
     });
 
     it("should handle errors thrown during the process", async () => {
@@ -241,10 +249,10 @@ describe("KeyConnectorService", () => {
       // Arrange
       const organization = organizationData(true, true, "https://key-connector-url.com", 2, false);
       const masterKey = getMockMasterKey();
+      masterPasswordService.masterKeySubject.next(masterKey);
       const keyConnectorRequest = new KeyConnectorUserKeyRequest(masterKey.encKeyB64);
 
       jest.spyOn(keyConnectorService, "getManagingOrganization").mockResolvedValue(organization);
-      jest.spyOn(cryptoService, "getMasterKey").mockResolvedValue(masterKey);
       jest.spyOn(apiService, "postUserKeyToKeyConnector").mockResolvedValue();
 
       // Act
@@ -252,7 +260,6 @@ describe("KeyConnectorService", () => {
 
       // Assert
       expect(keyConnectorService.getManagingOrganization).toHaveBeenCalled();
-      expect(cryptoService.getMasterKey).toHaveBeenCalled();
       expect(apiService.postUserKeyToKeyConnector).toHaveBeenCalledWith(
         organization.keyConnectorUrl,
         keyConnectorRequest,
@@ -268,8 +275,8 @@ describe("KeyConnectorService", () => {
       const error = new Error("Failed to post user key to key connector");
       organizationService.getAll.mockResolvedValue([organization]);
 
+      masterPasswordService.masterKeySubject.next(masterKey);
       jest.spyOn(keyConnectorService, "getManagingOrganization").mockResolvedValue(organization);
-      jest.spyOn(cryptoService, "getMasterKey").mockResolvedValue(masterKey);
       jest.spyOn(apiService, "postUserKeyToKeyConnector").mockRejectedValue(error);
       jest.spyOn(logService, "error");
 
@@ -280,7 +287,6 @@ describe("KeyConnectorService", () => {
         // Assert
         expect(logService.error).toHaveBeenCalledWith(error);
         expect(keyConnectorService.getManagingOrganization).toHaveBeenCalled();
-        expect(cryptoService.getMasterKey).toHaveBeenCalled();
         expect(apiService.postUserKeyToKeyConnector).toHaveBeenCalledWith(
           organization.keyConnectorUrl,
           keyConnectorRequest,

libs/common/src/auth/services/key-connector.service.ts

@@ -16,7 +16,9 @@ import {
   UserKeyDefinition,
 } from "../../platform/state";
 import { MasterKey } from "../../types/key";
+import { AccountService } from "../abstractions/account.service";
 import { KeyConnectorService as KeyConnectorServiceAbstraction } from "../abstractions/key-connector.service";
+import { InternalMasterPasswordServiceAbstraction } from "../abstractions/master-password.service.abstraction";
 import { TokenService } from "../abstractions/token.service";
 import { KdfConfig } from "../models/domain/kdf-config";
 import { KeyConnectorUserKeyRequest } from "../models/request/key-connector-user-key.request";
@@ -45,6 +47,8 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
   private usesKeyConnectorState: ActiveUserState<boolean>;
   private convertAccountToKeyConnectorState: ActiveUserState<boolean>;
   constructor(
+    private accountService: AccountService,
+    private masterPasswordService: InternalMasterPasswordServiceAbstraction,
     private cryptoService: CryptoService,
     private apiService: ApiService,
     private tokenService: TokenService,
@@ -78,7 +82,8 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
 
   async migrateUser() {
     const organization = await this.getManagingOrganization();
-    const masterKey = await this.cryptoService.getMasterKey();
+    const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
+    const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId));
     const keyConnectorRequest = new KeyConnectorUserKeyRequest(masterKey.encKeyB64);
 
     try {
@@ -99,7 +104,8 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
       const masterKeyResponse = await this.apiService.getMasterKeyFromKeyConnector(url);
       const keyArr = Utils.fromB64ToArray(masterKeyResponse.key);
       const masterKey = new SymmetricCryptoKey(keyArr) as MasterKey;
-      await this.cryptoService.setMasterKey(masterKey);
+      const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
+      await this.masterPasswordService.setMasterKey(masterKey, userId);
     } catch (e) {
       this.handleKeyConnectorError(e);
     }
@@ -136,7 +142,8 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
       kdfConfig,
     );
     const keyConnectorRequest = new KeyConnectorUserKeyRequest(masterKey.encKeyB64);
-    await this.cryptoService.setMasterKey(masterKey);
+    const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
+    await this.masterPasswordService.setMasterKey(masterKey, userId);
 
     const userKey = await this.cryptoService.makeUserKey(masterKey);
     await this.cryptoService.setUserKey(userKey[0]);

libs/common/src/auth/services/master-password/fake-master-password.service.ts

@@ -0,0 +1,64 @@
+import { mock } from "jest-mock-extended";
+import { ReplaySubject, Observable } from "rxjs";
+
+import { EncString } from "../../../platform/models/domain/enc-string";
+import { UserId } from "../../../types/guid";
+import { MasterKey } from "../../../types/key";
+import { InternalMasterPasswordServiceAbstraction } from "../../abstractions/master-password.service.abstraction";
+import { ForceSetPasswordReason } from "../../models/domain/force-set-password-reason";
+
+export class FakeMasterPasswordService implements InternalMasterPasswordServiceAbstraction {
+  mock = mock<InternalMasterPasswordServiceAbstraction>();
+
+  // eslint-disable-next-line rxjs/no-exposed-subjects -- test class
+  masterKeySubject = new ReplaySubject<MasterKey>(1);
+  // eslint-disable-next-line rxjs/no-exposed-subjects -- test class
+  masterKeyHashSubject = new ReplaySubject<string>(1);
+  // eslint-disable-next-line rxjs/no-exposed-subjects -- test class
+  forceSetPasswordReasonSubject = new ReplaySubject<ForceSetPasswordReason>(1);
+
+  constructor(initialMasterKey?: MasterKey, initialMasterKeyHash?: string) {
+    this.masterKeySubject.next(initialMasterKey);
+    this.masterKeyHashSubject.next(initialMasterKeyHash);
+  }
+
+  masterKey$(userId: UserId): Observable<MasterKey> {
+    return this.masterKeySubject.asObservable();
+  }
+
+  setMasterKey(masterKey: MasterKey, userId: UserId): Promise<void> {
+    return this.mock.setMasterKey(masterKey, userId);
+  }
+
+  clearMasterKey(userId: UserId): Promise<void> {
+    return this.mock.clearMasterKey(userId);
+  }
+
+  masterKeyHash$(userId: UserId): Observable<string> {
+    return this.masterKeyHashSubject.asObservable();
+  }
+
+  getMasterKeyEncryptedUserKey(userId: UserId): Promise<EncString> {
+    return this.mock.getMasterKeyEncryptedUserKey(userId);
+  }
+
+  setMasterKeyEncryptedUserKey(encryptedKey: EncString, userId: UserId): Promise<void> {
+    return this.mock.setMasterKeyEncryptedUserKey(encryptedKey, userId);
+  }
+
+  setMasterKeyHash(masterKeyHash: string, userId: UserId): Promise<void> {
+    return this.mock.setMasterKeyHash(masterKeyHash, userId);
+  }
+
+  clearMasterKeyHash(userId: UserId): Promise<void> {
+    return this.mock.clearMasterKeyHash(userId);
+  }
+
+  forceSetPasswordReason$(userId: UserId): Observable<ForceSetPasswordReason> {
+    return this.forceSetPasswordReasonSubject.asObservable();
+  }
+
+  setForceSetPasswordReason(reason: ForceSetPasswordReason, userId: UserId): Promise<void> {
+    return this.mock.setForceSetPasswordReason(reason, userId);
+  }
+}

libs/common/src/auth/services/master-password/master-password.service.ts

@@ -0,0 +1,140 @@
+import { firstValueFrom, map, Observable } from "rxjs";
+
+import { EncryptedString, EncString } from "../../../platform/models/domain/enc-string";
+import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key";
+import {
+  MASTER_PASSWORD_DISK,
+  MASTER_PASSWORD_MEMORY,
+  StateProvider,
+  UserKeyDefinition,
+} from "../../../platform/state";
+import { UserId } from "../../../types/guid";
+import { MasterKey } from "../../../types/key";
+import { InternalMasterPasswordServiceAbstraction } from "../../abstractions/master-password.service.abstraction";
+import { ForceSetPasswordReason } from "../../models/domain/force-set-password-reason";
+
+/** Memory since master key shouldn't be available on lock */
+const MASTER_KEY = new UserKeyDefinition<MasterKey>(MASTER_PASSWORD_MEMORY, "masterKey", {
+  deserializer: (masterKey) => SymmetricCryptoKey.fromJSON(masterKey) as MasterKey,
+  clearOn: ["lock", "logout"],
+});
+
+/** Disk since master key hash is used for unlock */
+const MASTER_KEY_HASH = new UserKeyDefinition<string>(MASTER_PASSWORD_DISK, "masterKeyHash", {
+  deserializer: (masterKeyHash) => masterKeyHash,
+  clearOn: ["logout"],
+});
+
+/** Disk to persist through lock */
+const MASTER_KEY_ENCRYPTED_USER_KEY = new UserKeyDefinition<EncryptedString>(
+  MASTER_PASSWORD_DISK,
+  "masterKeyEncryptedUserKey",
+  {
+    deserializer: (key) => key,
+    clearOn: ["logout"],
+  },
+);
+
+/** Disk to persist through lock and account switches */
+const FORCE_SET_PASSWORD_REASON = new UserKeyDefinition<ForceSetPasswordReason>(
+  MASTER_PASSWORD_DISK,
+  "forceSetPasswordReason",
+  {
+    deserializer: (reason) => reason,
+    clearOn: ["logout"],
+  },
+);
+
+export class MasterPasswordService implements InternalMasterPasswordServiceAbstraction {
+  constructor(private stateProvider: StateProvider) {}
+
+  masterKey$(userId: UserId): Observable<MasterKey> {
+    if (userId == null) {
+      throw new Error("User ID is required.");
+    }
+    return this.stateProvider.getUser(userId, MASTER_KEY).state$;
+  }
+
+  masterKeyHash$(userId: UserId): Observable<string> {
+    if (userId == null) {
+      throw new Error("User ID is required.");
+    }
+    return this.stateProvider.getUser(userId, MASTER_KEY_HASH).state$;
+  }
+
+  forceSetPasswordReason$(userId: UserId): Observable<ForceSetPasswordReason> {
+    if (userId == null) {
+      throw new Error("User ID is required.");
+    }
+    return this.stateProvider
+      .getUser(userId, FORCE_SET_PASSWORD_REASON)
+      .state$.pipe(map((reason) => reason ?? ForceSetPasswordReason.None));
+  }
+
+  // TODO: Remove this method and decrypt directly in the service instead
+  async getMasterKeyEncryptedUserKey(userId: UserId): Promise<EncString> {
+    if (userId == null) {
+      throw new Error("User ID is required.");
+    }
+    const key = await firstValueFrom(
+      this.stateProvider.getUser(userId, MASTER_KEY_ENCRYPTED_USER_KEY).state$,
+    );
+    return EncString.fromJSON(key);
+  }
+
+  async setMasterKey(masterKey: MasterKey, userId: UserId): Promise<void> {
+    if (masterKey == null) {
+      throw new Error("Master key is required.");
+    }
+    if (userId == null) {
+      throw new Error("User ID is required.");
+    }
+    await this.stateProvider.getUser(userId, MASTER_KEY).update((_) => masterKey);
+  }
+
+  async clearMasterKey(userId: UserId): Promise<void> {
+    if (userId == null) {
+      throw new Error("User ID is required.");
+    }
+    await this.stateProvider.getUser(userId, MASTER_KEY).update((_) => null);
+  }
+
+  async setMasterKeyHash(masterKeyHash: string, userId: UserId): Promise<void> {
+    if (masterKeyHash == null) {
+      throw new Error("Master key hash is required.");
+    }
+    if (userId == null) {
+      throw new Error("User ID is required.");
+    }
+    await this.stateProvider.getUser(userId, MASTER_KEY_HASH).update((_) => masterKeyHash);
+  }
+
+  async clearMasterKeyHash(userId: UserId): Promise<void> {
+    if (userId == null) {
+      throw new Error("User ID is required.");
+    }
+    await this.stateProvider.getUser(userId, MASTER_KEY_HASH).update((_) => null);
+  }
+
+  async setMasterKeyEncryptedUserKey(encryptedKey: EncString, userId: UserId): Promise<void> {
+    if (encryptedKey == null) {
+      throw new Error("Encrypted Key is required.");
+    }
+    if (userId == null) {
+      throw new Error("User ID is required.");
+    }
+    await this.stateProvider
+      .getUser(userId, MASTER_KEY_ENCRYPTED_USER_KEY)
+      .update((_) => encryptedKey.toJSON() as EncryptedString);
+  }
+
+  async setForceSetPasswordReason(reason: ForceSetPasswordReason, userId: UserId): Promise<void> {
+    if (reason == null) {
+      throw new Error("Reason is required.");
+    }
+    if (userId == null) {
+      throw new Error("User ID is required.");
+    }
+    await this.stateProvider.getUser(userId, FORCE_SET_PASSWORD_REASON).update((_) => reason);
+  }
+}

libs/common/src/auth/services/user-verification/user-verification.service.ts

@@ -10,7 +10,10 @@ import { LogService } from "../../../platform/abstractions/log.service";
 import { PlatformUtilsService } from "../../../platform/abstractions/platform-utils.service";
 import { StateService } from "../../../platform/abstractions/state.service";
 import { KeySuffixOptions } from "../../../platform/enums/key-suffix-options.enum";
+import { UserId } from "../../../types/guid";
 import { UserKey } from "../../../types/key";
+import { AccountService } from "../../abstractions/account.service";
+import { InternalMasterPasswordServiceAbstraction } from "../../abstractions/master-password.service.abstraction";
 import { UserVerificationApiServiceAbstraction } from "../../abstractions/user-verification/user-verification-api.service.abstraction";
 import { UserVerificationService as UserVerificationServiceAbstraction } from "../../abstractions/user-verification/user-verification.service.abstraction";
 import { VerificationType } from "../../enums/verification-type";
@@ -35,6 +38,8 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
   constructor(
     private stateService: StateService,
     private cryptoService: CryptoService,
+    private accountService: AccountService,
+    private masterPasswordService: InternalMasterPasswordServiceAbstraction,
     private i18nService: I18nService,
     private userVerificationApiService: UserVerificationApiServiceAbstraction,
     private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction,
@@ -107,7 +112,8 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
     if (verification.type === VerificationType.OTP) {
       request.otp = verification.secret;
     } else {
-      let masterKey = await this.cryptoService.getMasterKey();
+      const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
+      let masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId));
       if (!masterKey && !alreadyHashed) {
         masterKey = await this.cryptoService.makeMasterKey(
           verification.secret,
@@ -164,7 +170,8 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
   private async verifyUserByMasterPassword(
     verification: MasterPasswordVerification,
   ): Promise<boolean> {
-    let masterKey = await this.cryptoService.getMasterKey();
+    const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
+    let masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId));
     if (!masterKey) {
       masterKey = await this.cryptoService.makeMasterKey(
         verification.secret,
@@ -181,7 +188,7 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
       throw new Error(this.i18nService.t("invalidMasterPassword"));
     }
     // TODO: we should re-evaluate later on if user verification should have the side effect of modifying state. Probably not.
-    await this.cryptoService.setMasterKey(masterKey);
+    await this.masterPasswordService.setMasterKey(masterKey, userId);
     return true;
   }
 
@@ -230,9 +237,10 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
   }
 
   async hasMasterPasswordAndMasterKeyHash(userId?: string): Promise<boolean> {
+    userId ??= (await firstValueFrom(this.accountService.activeAccount$))?.id;
     return (
       (await this.hasMasterPassword(userId)) &&
-      (await this.cryptoService.getMasterKeyHash()) != null
+      (await firstValueFrom(this.masterPasswordService.masterKeyHash$(userId as UserId))) != null
     );
   }