[PM-23626] Require userId for makeOrgKey on the key service (#15864)

* Update key service

* Update consumers

* Add unit test coverage for consumer services

* Add unit test coverage for organization-billing service

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

@@ -259,11 +259,12 @@ export abstract class KeyService {
   /**
    * Creates a new organization key and encrypts it with the user's public key.
    * This method can also return Provider keys for creating new Provider users.
-   *
-   * @throws Error when no active user or user have no public key
-   * @returns The new encrypted org key and the decrypted key itself
+   * @param userId The user id of the target user's public key to use.
+   * @throws Error when userId is null or undefined.
+   * @throws Error when no public key is found for the target user.
+   * @returns The new encrypted OrgKey | ProviderKey and the decrypted key itself
    */
-  abstract makeOrgKey<T extends OrgKey | ProviderKey>(): Promise<[EncString, T]>;
+  abstract makeOrgKey<T extends OrgKey | ProviderKey>(userId: UserId): Promise<[EncString, T]>;
   /**
    * Sets the user's encrypted private key in storage and
    * clears the decrypted private key from memory

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

@@ -39,7 +39,13 @@ import {
 } from "@bitwarden/common/spec";
 import { CsprngArray } from "@bitwarden/common/types/csprng";
 import { OrganizationId, UserId } from "@bitwarden/common/types/guid";
-import { UserKey, MasterKey } from "@bitwarden/common/types/key";
+import {
+  UserKey,
+  MasterKey,
+  UserPublicKey,
+  OrgKey,
+  ProviderKey,
+} from "@bitwarden/common/types/key";
 
 import { KdfConfigService } from "./abstractions/kdf-config.service";
 import { UserPrivateKeyDecryptionFailedError } from "./abstractions/key.service";
@@ -1029,6 +1035,66 @@ describe("keyService", () => {
     });
   });
 
+  describe("makeOrgKey", () => {
+    const mockUserPublicKey = new Uint8Array(64) as UserPublicKey;
+    const shareKey = new SymmetricCryptoKey(new Uint8Array(64));
+    const mockEncapsulatedKey = new EncString("mockEncapsulatedKey");
+
+    beforeEach(() => {
+      keyService.userPublicKey$ = jest
+        .fn()
+        .mockReturnValueOnce(new BehaviorSubject(mockUserPublicKey));
+      keyGenerationService.createKey.mockResolvedValue(shareKey);
+      encryptService.encapsulateKeyUnsigned.mockResolvedValue(mockEncapsulatedKey);
+    });
+
+    it("creates a new OrgKey and encapsulates it with the user's public key", async () => {
+      const result = await keyService.makeOrgKey<OrgKey>(mockUserId);
+
+      expect(result).toEqual([mockEncapsulatedKey, shareKey as OrgKey]);
+      expect(keyService.userPublicKey$).toHaveBeenCalledWith(mockUserId);
+      expect(keyGenerationService.createKey).toHaveBeenCalledWith(512);
+      expect(encryptService.encapsulateKeyUnsigned).toHaveBeenCalledWith(
+        shareKey,
+        mockUserPublicKey,
+      );
+    });
+
+    it("creates a new ProviderKey and encapsulates it with the user's public key", async () => {
+      const result = await keyService.makeOrgKey<ProviderKey>(mockUserId);
+
+      expect(result).toEqual([mockEncapsulatedKey, shareKey as ProviderKey]);
+      expect(keyService.userPublicKey$).toHaveBeenCalledWith(mockUserId);
+      expect(keyGenerationService.createKey).toHaveBeenCalledWith(512);
+      expect(encryptService.encapsulateKeyUnsigned).toHaveBeenCalledWith(
+        shareKey,
+        mockUserPublicKey,
+      );
+    });
+
+    test.each([null as unknown as UserId, undefined as unknown as UserId])(
+      "throws when the provided userId is %s",
+      async (userId) => {
+        await expect(keyService.makeOrgKey(userId)).rejects.toThrow("UserId is required");
+
+        expect(keyService.userPublicKey$).not.toHaveBeenCalled();
+        expect(keyGenerationService.createKey).not.toHaveBeenCalled();
+        expect(encryptService.encapsulateKeyUnsigned).not.toHaveBeenCalled();
+      },
+    );
+
+    it("throws if the user's public key is not found", async () => {
+      keyService.userPublicKey$ = jest.fn().mockReturnValueOnce(new BehaviorSubject(null));
+
+      await expect(keyService.makeOrgKey(mockUserId)).rejects.toThrow(
+        "No public key found for user " + mockUserId,
+      );
+
+      expect(keyGenerationService.createKey).not.toHaveBeenCalled();
+      expect(encryptService.encapsulateKeyUnsigned).not.toHaveBeenCalled();
+    });
+  });
+
   describe("userEncryptionKeyPair$", () => {
     type SetupKeysParams = {
       makeMasterKey: boolean;

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

@@ -446,19 +446,17 @@ export class DefaultKeyService implements KeyServiceAbstraction {
     await this.stateProvider.setUserState(USER_ENCRYPTED_PROVIDER_KEYS, null, userId);
   }
 
-  // TODO: Make userId required
-  async makeOrgKey<T extends OrgKey | ProviderKey>(userId?: UserId): Promise<[EncString, T]> {
-    const shareKey = await this.keyGenerationService.createKey(512);
-    userId ??= await firstValueFrom(this.stateProvider.activeUserId$);
+  async makeOrgKey<T extends OrgKey | ProviderKey>(userId: UserId): Promise<[EncString, T]> {
     if (userId == null) {
-      throw new Error("No active user found.");
+      throw new Error("UserId is required");
     }
 
     const publicKey = await firstValueFrom(this.userPublicKey$(userId));
     if (publicKey == null) {
-      throw new Error("No public key found.");
+      throw new Error("No public key found for user " + userId);
     }
 
+    const shareKey = await this.keyGenerationService.createKey(512);
     const encShareKey = await this.encryptService.encapsulateKeyUnsigned(shareKey, publicKey);
     return [encShareKey, shareKey as T];
   }