[PM-23619] Remove getPrivateKey from the key service and update consumers (#15784)

* remove getPrivateKey from keyService

* Update consumer code

* Increase unit test coverage

apps/web/src/app/auth/emergency-access/services/emergency-access.service.spec.ts

@@ -2,6 +2,7 @@
 // @ts-strict-ignore
 import { MockProxy } from "jest-mock-extended";
 import mock from "jest-mock-extended/lib/Mock";
+import { of } from "rxjs";
 
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
@@ -14,9 +15,9 @@ import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
 import { CsprngArray } from "@bitwarden/common/types/csprng";
 import { UserId } from "@bitwarden/common/types/guid";
-import { UserKey, MasterKey } from "@bitwarden/common/types/key";
+import { UserKey, MasterKey, UserPrivateKey } from "@bitwarden/common/types/key";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
-import { KdfType, KeyService } from "@bitwarden/key-management";
+import { Argon2KdfConfig, KdfType, KeyService, PBKDF2KdfConfig } from "@bitwarden/key-management";
 
 import { EmergencyAccessStatusType } from "../enums/emergency-access-status-type";
 import { EmergencyAccessType } from "../enums/emergency-access-type";
@@ -26,6 +27,7 @@ import {
   EmergencyAccessGranteeDetailsResponse,
   EmergencyAccessGrantorDetailsResponse,
   EmergencyAccessTakeoverResponse,
+  EmergencyAccessViewResponse,
 } from "../response/emergency-access.response";
 
 import { EmergencyAccessApiService } from "./emergency-access-api.service";
@@ -142,88 +144,306 @@ describe("EmergencyAccessService", () => {
     });
   });
 
+  describe("getViewOnlyCiphers", () => {
+    const params = {
+      id: "emergency-access-id",
+      activeUserId: Utils.newGuid() as UserId,
+    };
+
+    it("throws an error is the active user's private key isn't available", async () => {
+      keyService.userPrivateKey$.mockReturnValue(of(null));
+
+      await expect(
+        emergencyAccessService.getViewOnlyCiphers(params.id, params.activeUserId),
+      ).rejects.toThrow("Active user does not have a private key, cannot get view only ciphers.");
+    });
+
+    it("should return decrypted and sorted ciphers", async () => {
+      const emergencyAccessViewResponse = {
+        keyEncrypted: "mockKeyEncrypted",
+        ciphers: [
+          { id: "cipher1", name: "encryptedName1" },
+          { id: "cipher2", name: "encryptedName2" },
+        ],
+      } as EmergencyAccessViewResponse;
+
+      const mockEncryptedCipher1 = {
+        id: "cipher1",
+        decrypt: jest.fn().mockResolvedValue({ id: "cipher1", decrypted: true }),
+      };
+      const mockEncryptedCipher2 = {
+        id: "cipher2",
+        decrypt: jest.fn().mockResolvedValue({ id: "cipher2", decrypted: true }),
+      };
+      emergencyAccessViewResponse.ciphers.map = jest.fn().mockImplementation(() => {
+        return [mockEncryptedCipher1, mockEncryptedCipher2];
+      });
+      cipherService.getLocaleSortingFunction.mockReturnValue((a: any, b: any) =>
+        a.id.localeCompare(b.id),
+      );
+      emergencyAccessApiService.postEmergencyAccessView.mockResolvedValue(
+        emergencyAccessViewResponse,
+      );
+
+      const mockPrivateKey = new Uint8Array(64) as UserPrivateKey;
+      keyService.userPrivateKey$.mockReturnValue(of(mockPrivateKey));
+
+      const mockDecryptedGrantorUserKey = new SymmetricCryptoKey(new Uint8Array(64));
+      encryptService.decapsulateKeyUnsigned.mockResolvedValueOnce(mockDecryptedGrantorUserKey);
+      const mockGrantorUserKey = mockDecryptedGrantorUserKey as UserKey;
+
+      const result = await emergencyAccessService.getViewOnlyCiphers(
+        params.id,
+        params.activeUserId,
+      );
+
+      expect(result).toEqual([
+        { id: "cipher1", decrypted: true },
+        { id: "cipher2", decrypted: true },
+      ]);
+      expect(mockEncryptedCipher1.decrypt).toHaveBeenCalledWith(mockGrantorUserKey);
+      expect(mockEncryptedCipher2.decrypt).toHaveBeenCalledWith(mockGrantorUserKey);
+      expect(emergencyAccessApiService.postEmergencyAccessView).toHaveBeenCalledWith(params.id);
+      expect(keyService.userPrivateKey$).toHaveBeenCalledWith(params.activeUserId);
+      expect(encryptService.decapsulateKeyUnsigned).toHaveBeenCalledWith(
+        new EncString(emergencyAccessViewResponse.keyEncrypted),
+        mockPrivateKey,
+      );
+      expect(cipherService.getLocaleSortingFunction).toHaveBeenCalled();
+    });
+  });
+
   describe("takeover", () => {
-    const mockId = "emergencyAccessId";
-    const mockEmail = "emergencyAccessEmail";
-    const mockName = "emergencyAccessName";
+    const params = {
+      id: "emergencyAccessId",
+      masterPassword: "mockPassword",
+      email: "emergencyAccessEmail",
+      activeUserId: Utils.newGuid() as UserId,
+    };
+
+    const takeoverResponse = {
+      keyEncrypted: "EncryptedKey",
+      kdf: KdfType.PBKDF2_SHA256,
+      kdfIterations: 500,
+    } as EmergencyAccessTakeoverResponse;
+
+    const userPrivateKey = new Uint8Array(64) as UserPrivateKey;
+    const mockMasterKey = new SymmetricCryptoKey(new Uint8Array(64) as CsprngArray) as MasterKey;
+    const mockMasterKeyHash = "mockMasterKeyHash";
+    let mockGrantorUserKey: UserKey;
+
+    // must mock [UserKey, EncString] return from keyService.encryptUserKeyWithMasterKey
+    // where UserKey is the decrypted grantor user key
+    const mockMasterKeyEncryptedUserKey = new EncString(
+      EncryptionType.AesCbc256_HmacSha256_B64,
+      "mockMasterKeyEncryptedUserKey",
+    );
+
+    beforeEach(() => {
+      emergencyAccessApiService.postEmergencyAccessTakeover.mockResolvedValueOnce(takeoverResponse);
+      keyService.userPrivateKey$.mockReturnValue(of(userPrivateKey));
+
+      const mockDecryptedGrantorUserKey = new SymmetricCryptoKey(new Uint8Array(64));
+      encryptService.decapsulateKeyUnsigned.mockResolvedValueOnce(mockDecryptedGrantorUserKey);
+      mockGrantorUserKey = mockDecryptedGrantorUserKey as UserKey;
+
+      keyService.makeMasterKey.mockResolvedValueOnce(mockMasterKey);
+      keyService.hashMasterKey.mockResolvedValueOnce(mockMasterKeyHash);
+      keyService.encryptUserKeyWithMasterKey.mockResolvedValueOnce([
+        mockGrantorUserKey,
+        mockMasterKeyEncryptedUserKey,
+      ]);
+    });
 
     it("posts a new password when decryption succeeds", async () => {
       // Arrange
-      emergencyAccessApiService.postEmergencyAccessTakeover.mockResolvedValueOnce({
-        keyEncrypted: "EncryptedKey",
-        kdf: KdfType.PBKDF2_SHA256,
-        kdfIterations: 500,
-      } as EmergencyAccessTakeoverResponse);
-
-      const mockDecryptedGrantorUserKey = new Uint8Array(64);
-      keyService.getPrivateKey.mockResolvedValue(new Uint8Array(64));
-      encryptService.decapsulateKeyUnsigned.mockResolvedValueOnce(
-        new SymmetricCryptoKey(mockDecryptedGrantorUserKey),
-      );
-
-      const mockMasterKey = new SymmetricCryptoKey(new Uint8Array(64) as CsprngArray) as MasterKey;
-
-      keyService.makeMasterKey.mockResolvedValueOnce(mockMasterKey);
-
-      const mockMasterKeyHash = "mockMasterKeyHash";
-      keyService.hashMasterKey.mockResolvedValueOnce(mockMasterKeyHash);
-
-      // must mock [UserKey, EncString] return from keyService.encryptUserKeyWithMasterKey
-      // where UserKey is the decrypted grantor user key
-      const mockMasterKeyEncryptedUserKey = new EncString(
-        EncryptionType.AesCbc256_HmacSha256_B64,
-        "mockMasterKeyEncryptedUserKey",
-      );
-
-      const mockUserKey = new SymmetricCryptoKey(mockDecryptedGrantorUserKey) as UserKey;
-
-      keyService.encryptUserKeyWithMasterKey.mockResolvedValueOnce([
-        mockUserKey,
-        mockMasterKeyEncryptedUserKey,
-      ]);
+      const expectedKdfConfig = new PBKDF2KdfConfig(takeoverResponse.kdfIterations);
 
       const expectedEmergencyAccessPasswordRequest = new EmergencyAccessPasswordRequest();
       expectedEmergencyAccessPasswordRequest.newMasterPasswordHash = mockMasterKeyHash;
       expectedEmergencyAccessPasswordRequest.key = mockMasterKeyEncryptedUserKey.encryptedString;
 
       // Act
-      await emergencyAccessService.takeover(mockId, mockEmail, mockName);
+      await emergencyAccessService.takeover(
+        params.id,
+        params.masterPassword,
+        params.email,
+        params.activeUserId,
+      );
 
       // Assert
+      expect(keyService.userPrivateKey$).toHaveBeenCalledWith(params.activeUserId);
+      expect(encryptService.decapsulateKeyUnsigned).toHaveBeenCalledWith(
+        new EncString(takeoverResponse.keyEncrypted),
+        userPrivateKey,
+      );
+      expect(keyService.makeMasterKey).toHaveBeenCalledWith(
+        params.masterPassword,
+        params.email,
+        expectedKdfConfig,
+      );
+      expect(keyService.hashMasterKey).toHaveBeenCalledWith(params.masterPassword, mockMasterKey);
+      expect(keyService.encryptUserKeyWithMasterKey).toHaveBeenCalledWith(
+        mockMasterKey,
+        mockGrantorUserKey,
+      );
       expect(emergencyAccessApiService.postEmergencyAccessPassword).toHaveBeenCalledWith(
-        mockId,
+        params.id,
         expectedEmergencyAccessPasswordRequest,
       );
     });
 
-    it("should not post a new password if decryption fails", async () => {
-      encryptService.rsaDecrypt.mockResolvedValueOnce(null);
-      emergencyAccessApiService.postEmergencyAccessTakeover.mockResolvedValueOnce({
+    it("uses argon2 KDF if takeover response is argon2", async () => {
+      const argon2TakeoverResponse = {
         keyEncrypted: "EncryptedKey",
-        kdf: KdfType.PBKDF2_SHA256,
-        kdfIterations: 500,
-      } as EmergencyAccessTakeoverResponse);
-      keyService.getPrivateKey.mockResolvedValue(new Uint8Array(64));
+        kdf: KdfType.Argon2id,
+        kdfIterations: 3,
+        kdfMemory: 64,
+        kdfParallelism: 4,
+      } as EmergencyAccessTakeoverResponse;
+      emergencyAccessApiService.postEmergencyAccessTakeover.mockReset();
+      emergencyAccessApiService.postEmergencyAccessTakeover.mockResolvedValueOnce(
+        argon2TakeoverResponse,
+      );
+
+      const expectedKdfConfig = new Argon2KdfConfig(
+        argon2TakeoverResponse.kdfIterations,
+        argon2TakeoverResponse.kdfMemory,
+        argon2TakeoverResponse.kdfParallelism,
+      );
+
+      const expectedEmergencyAccessPasswordRequest = new EmergencyAccessPasswordRequest();
+      expectedEmergencyAccessPasswordRequest.newMasterPasswordHash = mockMasterKeyHash;
+      expectedEmergencyAccessPasswordRequest.key = mockMasterKeyEncryptedUserKey.encryptedString;
+
+      await emergencyAccessService.takeover(
+        params.id,
+        params.masterPassword,
+        params.email,
+        params.activeUserId,
+      );
+
+      expect(keyService.userPrivateKey$).toHaveBeenCalledWith(params.activeUserId);
+      expect(encryptService.decapsulateKeyUnsigned).toHaveBeenCalledWith(
+        new EncString(argon2TakeoverResponse.keyEncrypted),
+        userPrivateKey,
+      );
+      expect(keyService.makeMasterKey).toHaveBeenCalledWith(
+        params.masterPassword,
+        params.email,
+        expectedKdfConfig,
+      );
+      expect(keyService.hashMasterKey).toHaveBeenCalledWith(params.masterPassword, mockMasterKey);
+      expect(keyService.encryptUserKeyWithMasterKey).toHaveBeenCalledWith(
+        mockMasterKey,
+        mockGrantorUserKey,
+      );
+      expect(emergencyAccessApiService.postEmergencyAccessPassword).toHaveBeenCalledWith(
+        params.id,
+        expectedEmergencyAccessPasswordRequest,
+      );
+    });
+
+    it("throws an error if masterKeyEncryptedUserKey is not found", async () => {
+      keyService.encryptUserKeyWithMasterKey.mockReset();
+      keyService.encryptUserKeyWithMasterKey.mockResolvedValueOnce(null);
+      const expectedKdfConfig = new PBKDF2KdfConfig(takeoverResponse.kdfIterations);
 
       await expect(
-        emergencyAccessService.takeover(mockId, mockEmail, mockName),
-      ).rejects.toThrowError("Failed to decrypt grantor key");
+        emergencyAccessService.takeover(
+          params.id,
+          params.masterPassword,
+          params.email,
+          params.activeUserId,
+        ),
+      ).rejects.toThrow("masterKeyEncryptedUserKey not found");
 
+      expect(keyService.userPrivateKey$).toHaveBeenCalledWith(params.activeUserId);
+      expect(encryptService.decapsulateKeyUnsigned).toHaveBeenCalledWith(
+        new EncString(takeoverResponse.keyEncrypted),
+        userPrivateKey,
+      );
+      expect(keyService.makeMasterKey).toHaveBeenCalledWith(
+        params.masterPassword,
+        params.email,
+        expectedKdfConfig,
+      );
+      expect(keyService.hashMasterKey).toHaveBeenCalledWith(params.masterPassword, mockMasterKey);
+      expect(keyService.encryptUserKeyWithMasterKey).toHaveBeenCalledWith(
+        mockMasterKey,
+        mockGrantorUserKey,
+      );
+      expect(emergencyAccessApiService.postEmergencyAccessPassword).not.toHaveBeenCalled();
+    });
+
+    it("should not post a new password if decryption fails", async () => {
+      emergencyAccessApiService.postEmergencyAccessTakeover.mockResolvedValueOnce(takeoverResponse);
+      encryptService.decapsulateKeyUnsigned.mockReset();
+      encryptService.decapsulateKeyUnsigned.mockResolvedValueOnce(null);
+
+      await expect(
+        emergencyAccessService.takeover(
+          params.id,
+          params.masterPassword,
+          params.email,
+          params.activeUserId,
+        ),
+      ).rejects.toThrow("Failed to decrypt grantor key");
+
+      expect(keyService.userPrivateKey$).toHaveBeenCalledWith(params.activeUserId);
+      expect(encryptService.decapsulateKeyUnsigned).toHaveBeenCalledWith(
+        new EncString(takeoverResponse.keyEncrypted),
+        userPrivateKey,
+      );
+      expect(keyService.makeMasterKey).not.toHaveBeenCalled();
+      expect(keyService.hashMasterKey).not.toHaveBeenCalled();
+      expect(keyService.encryptUserKeyWithMasterKey).not.toHaveBeenCalled();
+      expect(emergencyAccessApiService.postEmergencyAccessPassword).not.toHaveBeenCalled();
+    });
+
+    it("should not post a new password if decryption throws", async () => {
+      encryptService.decapsulateKeyUnsigned.mockReset();
+      encryptService.decapsulateKeyUnsigned.mockImplementationOnce(() => {
+        throw new Error("Failed to unwrap grantor key");
+      });
+
+      await expect(
+        emergencyAccessService.takeover(
+          params.id,
+          params.masterPassword,
+          params.email,
+          params.activeUserId,
+        ),
+      ).rejects.toThrowError("Failed to unwrap grantor key");
+
+      expect(keyService.userPrivateKey$).toHaveBeenCalledWith(params.activeUserId);
+      expect(encryptService.decapsulateKeyUnsigned).toHaveBeenCalledWith(
+        new EncString(takeoverResponse.keyEncrypted),
+        userPrivateKey,
+      );
+      expect(keyService.makeMasterKey).not.toHaveBeenCalled();
+      expect(keyService.hashMasterKey).not.toHaveBeenCalled();
+      expect(keyService.encryptUserKeyWithMasterKey).not.toHaveBeenCalled();
       expect(emergencyAccessApiService.postEmergencyAccessPassword).not.toHaveBeenCalled();
     });
 
     it("should throw an error if the users private key cannot be retrieved", async () => {
-      emergencyAccessApiService.postEmergencyAccessTakeover.mockResolvedValueOnce({
-        keyEncrypted: "EncryptedKey",
-        kdf: KdfType.PBKDF2_SHA256,
-        kdfIterations: 500,
-      } as EmergencyAccessTakeoverResponse);
-      keyService.getPrivateKey.mockResolvedValue(null);
+      keyService.userPrivateKey$.mockReturnValue(of(null));
 
-      await expect(emergencyAccessService.takeover(mockId, mockEmail, mockName)).rejects.toThrow(
-        "user does not have a private key",
-      );
+      await expect(
+        emergencyAccessService.takeover(
+          params.id,
+          params.masterPassword,
+          params.email,
+          params.activeUserId,
+        ),
+      ).rejects.toThrow("user does not have a private key");
 
+      expect(keyService.userPrivateKey$).toHaveBeenCalledWith(params.activeUserId);
+      expect(encryptService.decapsulateKeyUnsigned).not.toHaveBeenCalled();
+      expect(keyService.makeMasterKey).not.toHaveBeenCalled();
+      expect(keyService.hashMasterKey).not.toHaveBeenCalled();
+      expect(keyService.encryptUserKeyWithMasterKey).not.toHaveBeenCalled();
       expect(emergencyAccessApiService.postEmergencyAccessPassword).not.toHaveBeenCalled();
     });
   });

apps/web/src/app/auth/emergency-access/services/emergency-access.service.ts

@@ -1,4 +1,5 @@
 import { Injectable } from "@angular/core";
+import { firstValueFrom } from "rxjs";
 
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { PolicyData } from "@bitwarden/common/admin-console/models/data/policy.data";
@@ -237,11 +238,14 @@ export class EmergencyAccessService
    * Gets the grantor ciphers for an emergency access in view mode.
    * Intended for grantee.
    * @param id emergency access id
+   * @param activeUserId the user id of the active user
    */
-  async getViewOnlyCiphers(id: string): Promise<CipherView[]> {
+  async getViewOnlyCiphers(id: string, activeUserId: UserId): Promise<CipherView[]> {
     const response = await this.emergencyAccessApiService.postEmergencyAccessView(id);
 
-    const activeUserPrivateKey = await this.keyService.getPrivateKey();
+    const activeUserPrivateKey = await firstValueFrom(
+      this.keyService.userPrivateKey$(activeUserId),
+    );
 
     if (activeUserPrivateKey == null) {
       throw new Error("Active user does not have a private key, cannot get view only ciphers.");
@@ -264,11 +268,14 @@ export class EmergencyAccessService
    * @param id emergency access id
    * @param masterPassword new master password
    * @param email email address of grantee (must be consistent or login will fail)
+   * @param activeUserId the user id of the active user
    */
-  async takeover(id: string, masterPassword: string, email: string) {
+  async takeover(id: string, masterPassword: string, email: string, activeUserId: UserId) {
     const takeoverResponse = await this.emergencyAccessApiService.postEmergencyAccessTakeover(id);
 
-    const activeUserPrivateKey = await this.keyService.getPrivateKey();
+    const activeUserPrivateKey = await firstValueFrom(
+      this.keyService.userPrivateKey$(activeUserId),
+    );
 
     if (activeUserPrivateKey == null) {
       throw new Error("Active user does not have a private key, cannot complete a takeover.");
@@ -312,9 +319,7 @@ export class EmergencyAccessService
     request.newMasterPasswordHash = masterKeyHash;
     request.key = encKey[1].encryptedString;
 
-    // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-    // eslint-disable-next-line @typescript-eslint/no-floating-promises
-    this.emergencyAccessApiService.postEmergencyAccessPassword(id, request);
+    await this.emergencyAccessApiService.postEmergencyAccessPassword(id, request);
   }
 
   private async getEmergencyAccessData(): Promise<EmergencyAccessGranteeDetailsResponse[]> {

apps/web/src/app/auth/settings/emergency-access/takeover/emergency-access-takeover-dialog.component.ts

@@ -115,10 +115,12 @@ export class EmergencyAccessTakeoverDialogComponent implements OnInit {
     this.parentSubmittingBehaviorSubject.next(true);
 
     try {
+      const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
       await this.emergencyAccessService.takeover(
         this.dialogData.emergencyAccessId,
         passwordInputResult.newPassword,
         this.dialogData.grantorEmail,
+        activeUserId,
       );
     } catch (e) {
       this.logService.error(e);

apps/web/src/app/auth/settings/emergency-access/view/emergency-access-view.component.ts

@@ -2,6 +2,8 @@ import { Component, OnInit } from "@angular/core";
 import { ActivatedRoute, Router } from "@angular/router";
 import { firstValueFrom } from "rxjs";
 
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
+import { getUserId } from "@bitwarden/common/auth/services/account.service";
 import { EmergencyAccessId } from "@bitwarden/common/types/guid";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { DialogService } from "@bitwarden/components";
@@ -27,6 +29,7 @@ export class EmergencyAccessViewComponent implements OnInit {
     private route: ActivatedRoute,
     private emergencyAccessService: EmergencyAccessService,
     private dialogService: DialogService,
+    private accountService: AccountService,
   ) {}
 
   async ngOnInit() {
@@ -37,7 +40,8 @@ export class EmergencyAccessViewComponent implements OnInit {
     }
 
     this.id = qParams.id;
-    this.ciphers = await this.emergencyAccessService.getViewOnlyCiphers(qParams.id);
+    const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
+    this.ciphers = await this.emergencyAccessService.getViewOnlyCiphers(qParams.id, userId);
     this.loaded = true;
   }