[PM-21797] Require userID for keyService's getUserKeyFromStorage (#14855)

* require userID for keyService's getUserKeyFromStorage

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

@@ -128,10 +128,11 @@ export abstract class KeyService {
    * @param keySuffix The desired version of the user's key to retrieve
    * @param userId The desired user
    * @returns The user key
+   * @throws Error when userId is null or undefined.
    */
   abstract getUserKeyFromStorage(
     keySuffix: KeySuffixOptions,
-    userId?: string,
+    userId: string,
   ): Promise<UserKey | null>;
 
   /**

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

@@ -14,6 +14,7 @@ import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/ke
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
+import { KeySuffixOptions } from "@bitwarden/common/platform/enums";
 import { Encrypted } from "@bitwarden/common/platform/interfaces/encrypted";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { EncString, EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string";
@@ -896,4 +897,84 @@ describe("keyService", () => {
       });
     });
   });
+
+  describe("getUserKeyFromStorage", () => {
+    let mockUserKey: UserKey;
+    let validateUserKeySpy: jest.SpyInstance;
+
+    beforeEach(() => {
+      mockUserKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey;
+      validateUserKeySpy = jest.spyOn(keyService, "validateUserKey");
+    });
+
+    afterEach(() => {
+      validateUserKeySpy.mockRestore();
+    });
+
+    describe("input validation", () => {
+      const invalidUserIdTestCases = [
+        { keySuffix: KeySuffixOptions.Auto, userId: null as unknown as UserId },
+        { keySuffix: KeySuffixOptions.Auto, userId: undefined as unknown as UserId },
+        { keySuffix: KeySuffixOptions.Pin, userId: null as unknown as UserId },
+        { keySuffix: KeySuffixOptions.Pin, userId: undefined as unknown as UserId },
+      ];
+
+      test.each(invalidUserIdTestCases)(
+        "throws when keySuffix is $keySuffix and userId is $userId",
+        async ({ keySuffix, userId }) => {
+          await expect(keyService.getUserKeyFromStorage(keySuffix, userId)).rejects.toThrow(
+            "UserId is required",
+          );
+        },
+      );
+    });
+
+    describe("with Pin keySuffix", () => {
+      it("returns null and doesn't validate the key", async () => {
+        const result = await keyService.getUserKeyFromStorage(KeySuffixOptions.Pin, mockUserId);
+
+        expect(result).toBeNull();
+        expect(validateUserKeySpy).not.toHaveBeenCalled();
+      });
+    });
+
+    describe("with Auto keySuffix", () => {
+      it("returns validated key from storage when key exists and is valid", async () => {
+        stateService.getUserKeyAutoUnlock.mockResolvedValue(mockUserKey.keyB64);
+        validateUserKeySpy.mockResolvedValue(true);
+
+        const result = await keyService.getUserKeyFromStorage(KeySuffixOptions.Auto, mockUserId);
+
+        expect(result).toEqual(mockUserKey);
+        expect(validateUserKeySpy).toHaveBeenCalledWith(mockUserKey, mockUserId);
+        expect(stateService.getUserKeyAutoUnlock).toHaveBeenCalledWith({
+          userId: mockUserId,
+        });
+      });
+
+      it("returns null when no key is found in storage", async () => {
+        stateService.getUserKeyAutoUnlock.mockResolvedValue(null as unknown as string);
+
+        const result = await keyService.getUserKeyFromStorage(KeySuffixOptions.Auto, mockUserId);
+
+        expect(result).toBeNull();
+        expect(validateUserKeySpy).not.toHaveBeenCalled();
+      });
+
+      it("clears stored keys when userKey validation fails", async () => {
+        stateService.getUserKeyAutoUnlock.mockResolvedValue(mockUserKey.keyB64);
+        validateUserKeySpy.mockResolvedValue(false);
+
+        const result = await keyService.getUserKeyFromStorage(KeySuffixOptions.Auto, mockUserId);
+
+        expect(result).toEqual(mockUserKey);
+        expect(validateUserKeySpy).toHaveBeenCalledWith(mockUserKey, mockUserId);
+        expect(logService.warning).toHaveBeenCalledWith("Invalid key, throwing away stored keys");
+        expect(pinService.clearPinKeyEncryptedUserKeyEphemeral).toHaveBeenCalledWith(mockUserId);
+        expect(stateService.setUserKeyAutoUnlock).toHaveBeenCalledWith(null, {
+          userId: mockUserId,
+        });
+      });
+    });
+  });
 });

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

@@ -178,11 +178,10 @@ export class DefaultKeyService implements KeyServiceAbstraction {
 
   async getUserKeyFromStorage(
     keySuffix: KeySuffixOptions,
-    userId?: UserId,
+    userId: UserId,
   ): Promise<UserKey | null> {
-    userId ??= await firstValueFrom(this.stateProvider.activeUserId$);
     if (userId == null) {
-      throw new Error("No active user id found.");
+      throw new Error("UserId is required");
     }
 
     const userKey = await this.getKeyFromStorage(keySuffix, userId);