From 4866eaa2ec3950683cd4a878e6c5c86365c4aabb Mon Sep 17 00:00:00 2001 From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> Date: Thu, 8 Jan 2026 11:09:13 -0600 Subject: [PATCH] [PM-23618] Require masterKey on makeUserKey (#17244) --- .../src/abstractions/key.service.ts | 6 ++--- libs/key-management/src/key.service.spec.ts | 26 +++++++++++++++++++ libs/key-management/src/key.service.ts | 14 +++------- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/libs/key-management/src/abstractions/key.service.ts b/libs/key-management/src/abstractions/key.service.ts index feb4a38ac27..6cf44544422 100644 --- a/libs/key-management/src/abstractions/key.service.ts +++ b/libs/key-management/src/abstractions/key.service.ts @@ -129,11 +129,11 @@ export abstract class KeyService { /** * Generates a new user key * @deprecated Interacting with the master key directly is prohibited. Use {@link makeUserKeyV1} instead. - * @throws Error when master key is null and there is no active user - * @param masterKey The user's master key. When null, grabs master key from active user. + * @throws Error when master key is null or undefined. + * @param masterKey The user's master key. * @returns A new user key and the master key protected version of it */ - abstract makeUserKey(masterKey: MasterKey | null): Promise<[UserKey, EncString]>; + abstract makeUserKey(masterKey: MasterKey): Promise<[UserKey, EncString]>; /** * Generates a new user key for a V1 user * Note: This will be replaced by a higher level function to initialize a whole users cryptographic state in the near future. diff --git a/libs/key-management/src/key.service.spec.ts b/libs/key-management/src/key.service.spec.ts index c0af62fe6e9..c0a0ab62347 100644 --- a/libs/key-management/src/key.service.spec.ts +++ b/libs/key-management/src/key.service.spec.ts @@ -177,6 +177,32 @@ describe("keyService", () => { }); }); + describe("makeUserKey", () => { + test.each([null as unknown as MasterKey, undefined as unknown as MasterKey])( + "throws when the provided masterKey is %s", + async (masterKey) => { + await expect(keyService.makeUserKey(masterKey)).rejects.toThrow("MasterKey is required"); + }, + ); + + it("encrypts the user key with the master key", async () => { + const mockUserKey = makeSymmetricCryptoKey(64); + const mockEncryptedUserKey = makeEncString("encryptedUserKey"); + + keyGenerationService.createKey.mockResolvedValue(mockUserKey); + encryptService.wrapSymmetricKey.mockResolvedValue(mockEncryptedUserKey); + const stretchedMasterKey = new SymmetricCryptoKey(new Uint8Array(64)); + keyGenerationService.stretchKey.mockResolvedValue(stretchedMasterKey); + + const result = await keyService.makeUserKey(makeSymmetricCryptoKey(32)); + + expect(encryptService.wrapSymmetricKey).toHaveBeenCalledWith(mockUserKey, stretchedMasterKey); + expect(keyGenerationService.createKey).toHaveBeenCalledWith(512); + expect(result[0]).toBe(mockUserKey); + expect(result[1]).toBe(mockEncryptedUserKey); + }); + }); + describe("everHadUserKey$", () => { let everHadUserKeyState: FakeSingleUserState; diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts index 621a8135d1e..8cb072a4c2a 100644 --- a/libs/key-management/src/key.service.ts +++ b/libs/key-management/src/key.service.ts @@ -204,17 +204,9 @@ export class DefaultKeyService implements KeyServiceAbstraction { return (await firstValueFrom(this.stateProvider.getUserState$(USER_KEY, userId))) != null; } - async makeUserKey(masterKey: MasterKey | null): Promise<[UserKey, EncString]> { - if (masterKey == null) { - const userId = await firstValueFrom(this.stateProvider.activeUserId$); - if (userId == null) { - throw new Error("No active user id found."); - } - - masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); - } - if (masterKey == null) { - throw new Error("No Master Key found."); + async makeUserKey(masterKey: MasterKey): Promise<[UserKey, EncString]> { + if (!masterKey) { + throw new Error("MasterKey is required"); } const newUserKey = await this.keyGenerationService.createKey(512);