From 8a7bfefad32a7fe11f203a5d1f257cfe27c06114 Mon Sep 17 00:00:00 2001 From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> Date: Thu, 13 Nov 2025 14:06:56 -0600 Subject: [PATCH] [PM-26498] Add proofOfDecryption method to MasterPasswordUnlockService (#17322) * Add proofOfDecryption method to MasterPasswordUnlockService --- .../service-container/service-container.ts | 1 + .../src/services/jslib-services.module.ts | 2 +- .../master-password-unlock.service.ts | 14 +++ ...ult-master-password-unlock.service.spec.ts | 94 ++++++++++++++++++- .../default-master-password-unlock.service.ts | 40 ++++++++ 5 files changed, 148 insertions(+), 3 deletions(-) diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index c9f1d11210b..ebfb76eab2f 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -495,6 +495,7 @@ export class ServiceContainer { this.masterPasswordUnlockService = new DefaultMasterPasswordUnlockService( this.masterPasswordService, this.keyService, + this.logService, ); this.appIdService = new AppIdService(this.storageService, this.logService); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 97aa1869575..18c21024a6a 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -1087,7 +1087,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: MasterPasswordUnlockService, useClass: DefaultMasterPasswordUnlockService, - deps: [InternalMasterPasswordServiceAbstraction, KeyService], + deps: [InternalMasterPasswordServiceAbstraction, KeyService, LogService], }), safeProvider({ provide: KeyConnectorServiceAbstraction, diff --git a/libs/common/src/key-management/master-password/abstractions/master-password-unlock.service.ts b/libs/common/src/key-management/master-password/abstractions/master-password-unlock.service.ts index 4448206b2f6..63ee7d69719 100644 --- a/libs/common/src/key-management/master-password/abstractions/master-password-unlock.service.ts +++ b/libs/common/src/key-management/master-password/abstractions/master-password-unlock.service.ts @@ -7,7 +7,21 @@ export abstract class MasterPasswordUnlockService { * Unlocks the user's account using the master password. * @param masterPassword The master password provided by the user. * @param userId The ID of the active user. + * @throws If the master password provided is null/undefined/empty. + * @throws If the userId provided is null/undefined. + * @throws if the masterPasswordUnlockData for the user is not found. + * @throws If unwrapping the user key fails. * @returns the user's decrypted userKey. */ abstract unlockWithMasterPassword(masterPassword: string, userId: UserId): Promise; + + /** + * For the given master password and user ID, verifies whether the user can decrypt their user key stored in state. + * @param masterPassword The master password provided by the user. + * @param userId The ID of the active user. + * @throws If the master password provided is null/undefined/empty. + * @throws If the userId provided is null/undefined. + * @returns true if the userKey can be decrypted, false otherwise. + */ + abstract proofOfDecryption(masterPassword: string, userId: UserId): Promise; } diff --git a/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.spec.ts b/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.spec.ts index 75668e8e6bd..1c95090f04b 100644 --- a/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.spec.ts +++ b/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.spec.ts @@ -4,6 +4,8 @@ import { of } from "rxjs"; import { newGuid } from "@bitwarden/guid"; // eslint-disable-next-line no-restricted-imports import { Argon2KdfConfig, KeyService } from "@bitwarden/key-management"; +import { LogService } from "@bitwarden/logging"; +import { CryptoError } from "@bitwarden/sdk-internal"; import { UserId } from "@bitwarden/user-core"; import { HashPurpose } from "../../../platform/enums"; @@ -23,6 +25,7 @@ describe("DefaultMasterPasswordUnlockService", () => { let masterPasswordService: MockProxy; let keyService: MockProxy; + let logService: MockProxy; const mockMasterPassword = "testExample"; const mockUserId = newGuid() as UserId; @@ -41,8 +44,9 @@ describe("DefaultMasterPasswordUnlockService", () => { beforeEach(() => { masterPasswordService = mock(); keyService = mock(); + logService = mock(); - sut = new DefaultMasterPasswordUnlockService(masterPasswordService, keyService); + sut = new DefaultMasterPasswordUnlockService(masterPasswordService, keyService, logService); masterPasswordService.masterPasswordUnlockData$.mockReturnValue( of(mockMasterPasswordUnlockData), @@ -73,7 +77,7 @@ describe("DefaultMasterPasswordUnlockService", () => { ); test.each([null as unknown as UserId, undefined as unknown as UserId])( - "throws when the provided master password is %s", + "throws when the provided userID is %s", async (userId) => { await expect(sut.unlockWithMasterPassword(mockMasterPassword, userId)).rejects.toThrow( "User ID is required", @@ -151,4 +155,90 @@ describe("DefaultMasterPasswordUnlockService", () => { expect(masterPasswordService.setMasterKey).not.toHaveBeenCalled(); }); }); + + describe("proofOfDecryption", () => { + test.each([null as unknown as string, undefined as unknown as string, ""])( + "throws when the provided master password is %s", + async (masterPassword) => { + await expect(sut.proofOfDecryption(masterPassword, mockUserId)).rejects.toThrow( + "Master password is required", + ); + expect(masterPasswordService.masterPasswordUnlockData$).not.toHaveBeenCalled(); + expect( + masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData, + ).not.toHaveBeenCalled(); + }, + ); + + test.each([null as unknown as UserId, undefined as unknown as UserId])( + "throws when the provided userID is %s", + async (userId) => { + await expect(sut.proofOfDecryption(mockMasterPassword, userId)).rejects.toThrow( + "User ID is required", + ); + }, + ); + + it("returns false when the user doesn't have masterPasswordUnlockData", async () => { + masterPasswordService.masterPasswordUnlockData$.mockReturnValue(of(null)); + + const result = await sut.proofOfDecryption(mockMasterPassword, mockUserId); + + expect(result).toBe(false); + expect(masterPasswordService.masterPasswordUnlockData$).toHaveBeenCalledWith(mockUserId); + expect( + masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData, + ).not.toHaveBeenCalled(); + expect(logService.warning).toHaveBeenCalledWith( + `[DefaultMasterPasswordUnlockService] No master password unlock data found for user ${mockUserId} returning false.`, + ); + }); + + it("returns true when the master password is correct", async () => { + const result = await sut.proofOfDecryption(mockMasterPassword, mockUserId); + + expect(result).toBe(true); + expect(masterPasswordService.masterPasswordUnlockData$).toHaveBeenCalledWith(mockUserId); + expect(masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData).toHaveBeenCalledWith( + mockMasterPassword, + mockMasterPasswordUnlockData, + ); + }); + + it("returns false when the master password is incorrect", async () => { + const error = new Error("Incorrect password") as CryptoError; + error.name = "CryptoError"; + error.variant = "InvalidKey"; + masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData.mockRejectedValue(error); + + const result = await sut.proofOfDecryption(mockMasterPassword, mockUserId); + + expect(result).toBe(false); + expect(masterPasswordService.masterPasswordUnlockData$).toHaveBeenCalledWith(mockUserId); + expect(masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData).toHaveBeenCalledWith( + mockMasterPassword, + mockMasterPasswordUnlockData, + ); + expect(logService.debug).toHaveBeenCalledWith( + `[DefaultMasterPasswordUnlockService] Error during proof of decryption for user ${mockUserId} returning false: ${error}`, + ); + }); + + it("returns false when a generic error occurs", async () => { + const error = new Error("Generic error"); + masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData.mockRejectedValue(error); + + const result = await sut.proofOfDecryption(mockMasterPassword, mockUserId); + + expect(result).toBe(false); + expect(masterPasswordService.masterPasswordUnlockData$).toHaveBeenCalledWith(mockUserId); + expect(masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData).toHaveBeenCalledWith( + mockMasterPassword, + mockMasterPasswordUnlockData, + ); + expect(logService.error).toHaveBeenCalledWith( + `[DefaultMasterPasswordUnlockService] Unexpected error during proof of decryption for user ${mockUserId} returning false: ${error}`, + ); + }); + }); }); diff --git a/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.ts b/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.ts index 87114000abf..89a87403e49 100644 --- a/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.ts +++ b/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.ts @@ -2,6 +2,8 @@ import { firstValueFrom } from "rxjs"; // eslint-disable-next-line no-restricted-imports import { KeyService } from "@bitwarden/key-management"; +import { LogService } from "@bitwarden/logging"; +import { isCryptoError } from "@bitwarden/sdk-internal"; import { UserId } from "@bitwarden/user-core"; import { HashPurpose } from "../../../platform/enums"; @@ -14,6 +16,7 @@ export class DefaultMasterPasswordUnlockService implements MasterPasswordUnlockS constructor( private readonly masterPasswordService: InternalMasterPasswordServiceAbstraction, private readonly keyService: KeyService, + private readonly logService: LogService, ) {} async unlockWithMasterPassword(masterPassword: string, userId: UserId): Promise { @@ -37,6 +40,43 @@ export class DefaultMasterPasswordUnlockService implements MasterPasswordUnlockS return userKey; } + async proofOfDecryption(masterPassword: string, userId: UserId): Promise { + this.validateInput(masterPassword, userId); + + try { + const masterPasswordUnlockData = await firstValueFrom( + this.masterPasswordService.masterPasswordUnlockData$(userId), + ); + + if (masterPasswordUnlockData == null) { + this.logService.warning( + `[DefaultMasterPasswordUnlockService] No master password unlock data found for user ${userId} returning false.`, + ); + return false; + } + + const userKey = await this.masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData( + masterPassword, + masterPasswordUnlockData, + ); + + return userKey != null; + } catch (error) { + // masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData is expected to throw if the password is incorrect. + // Currently this throws CryptoError:InvalidKey if decrypting the user key fails at all. + if (isCryptoError(error) && error.variant === "InvalidKey") { + this.logService.debug( + `[DefaultMasterPasswordUnlockService] Error during proof of decryption for user ${userId} returning false: ${error}`, + ); + } else { + this.logService.error( + `[DefaultMasterPasswordUnlockService] Unexpected error during proof of decryption for user ${userId} returning false: ${error}`, + ); + } + return false; + } + } + private validateInput(masterPassword: string, userId: UserId): void { if (masterPassword == null || masterPassword === "") { throw new Error("Master password is required");