1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-16 08:13:42 +00:00

[PM-23246] Add unlock with master password unlock data for lock component (#16204)

* Add unlocking with MasterPasswordUnlockData for angular lock component
This commit is contained in:
Thomas Avery
2025-10-15 11:56:46 -05:00
committed by GitHub
parent 76e4870aa3
commit aa9a276591
15 changed files with 1249 additions and 129 deletions

View File

@@ -33,6 +33,7 @@ export enum FeatureFlag {
PrivateKeyRegeneration = "pm-12241-private-key-regeneration",
EnrollAeadOnKeyRotation = "enroll-aead-on-key-rotation",
ForceUpdateKDFSettings = "pm-18021-force-update-kdf-settings",
UnlockWithMasterPasswordUnlockData = "pm-23246-unlock-with-master-password-unlock-data",
/* Tools */
DesktopSendUIRefresh = "desktop-send-ui-refresh",
@@ -110,6 +111,7 @@ export const DefaultFeatureFlagValue = {
[FeatureFlag.PrivateKeyRegeneration]: FALSE,
[FeatureFlag.EnrollAeadOnKeyRotation]: FALSE,
[FeatureFlag.ForceUpdateKDFSettings]: FALSE,
[FeatureFlag.UnlockWithMasterPasswordUnlockData]: FALSE,
/* Platform */
[FeatureFlag.IpcChannelFramework]: FALSE,

View File

@@ -0,0 +1,13 @@
import { UserId } from "@bitwarden/user-core";
import { UserKey } from "../../../types/key";
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.
* @returns the user's decrypted userKey.
*/
abstract unlockWithMasterPassword(masterPassword: string, userId: UserId): Promise<UserKey>;
}

View File

@@ -171,4 +171,12 @@ export abstract class InternalMasterPasswordServiceAbstraction extends MasterPas
masterPasswordUnlockData: MasterPasswordUnlockData,
userId: UserId,
): Promise<void>;
/**
* An observable that emits the master password unlock data for the target user.
* @param userId The user ID.
* @throws If the user ID is null or undefined.
* @returns An observable that emits the master password unlock data or null if not found.
*/
abstract masterPasswordUnlockData$(userId: UserId): Observable<MasterPasswordUnlockData | null>;
}

View File

@@ -0,0 +1,154 @@
import { mock, MockProxy } from "jest-mock-extended";
import { of } from "rxjs";
import { newGuid } from "@bitwarden/guid";
// eslint-disable-next-line no-restricted-imports
import { Argon2KdfConfig, KeyService } from "@bitwarden/key-management";
import { UserId } from "@bitwarden/user-core";
import { HashPurpose } from "../../../platform/enums";
import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key";
import { MasterKey, UserKey } from "../../../types/key";
import { InternalMasterPasswordServiceAbstraction } from "../abstractions/master-password.service.abstraction";
import {
MasterKeyWrappedUserKey,
MasterPasswordSalt,
MasterPasswordUnlockData,
} from "../types/master-password.types";
import { DefaultMasterPasswordUnlockService } from "./default-master-password-unlock.service";
describe("DefaultMasterPasswordUnlockService", () => {
let sut: DefaultMasterPasswordUnlockService;
let masterPasswordService: MockProxy<InternalMasterPasswordServiceAbstraction>;
let keyService: MockProxy<KeyService>;
const mockMasterPassword = "testExample";
const mockUserId = newGuid() as UserId;
const mockUserKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey;
const mockMasterPasswordUnlockData: MasterPasswordUnlockData = new MasterPasswordUnlockData(
"user@example.com" as MasterPasswordSalt,
new Argon2KdfConfig(100000, 64, 1),
"encryptedMasterKeyWrappedUserKey" as MasterKeyWrappedUserKey,
);
//Legacy data for tests
const mockMasterKey = new SymmetricCryptoKey(new Uint8Array(32)) as MasterKey;
const mockKeyHash = "localKeyHash";
beforeEach(() => {
masterPasswordService = mock<InternalMasterPasswordServiceAbstraction>();
keyService = mock<KeyService>();
sut = new DefaultMasterPasswordUnlockService(masterPasswordService, keyService);
masterPasswordService.masterPasswordUnlockData$.mockReturnValue(
of(mockMasterPasswordUnlockData),
);
masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData.mockResolvedValue(mockUserKey);
// Legacy state mocking
keyService.makeMasterKey.mockResolvedValue(mockMasterKey);
keyService.hashMasterKey.mockResolvedValue(mockKeyHash);
});
afterEach(() => {
jest.resetAllMocks();
});
describe("unlockWithMasterPassword", () => {
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.unlockWithMasterPassword(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 master password is %s",
async (userId) => {
await expect(sut.unlockWithMasterPassword(mockMasterPassword, userId)).rejects.toThrow(
"User ID is required",
);
},
);
it("throws an error when the user doesn't have masterPasswordUnlockData", async () => {
masterPasswordService.masterPasswordUnlockData$.mockReturnValue(of(null));
await expect(sut.unlockWithMasterPassword(mockMasterPassword, mockUserId)).rejects.toThrow(
"Master password unlock data was not found for the user " + mockUserId,
);
expect(masterPasswordService.masterPasswordUnlockData$).toHaveBeenCalledWith(mockUserId);
expect(
masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData,
).not.toHaveBeenCalled();
});
it("returns userKey successfully", async () => {
const result = await sut.unlockWithMasterPassword(mockMasterPassword, mockUserId);
expect(result).toEqual(mockUserKey);
expect(masterPasswordService.masterPasswordUnlockData$).toHaveBeenCalledWith(mockUserId);
expect(masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData).toHaveBeenCalledWith(
mockMasterPassword,
mockMasterPasswordUnlockData,
);
});
it("sets legacy state on success", async () => {
const result = await sut.unlockWithMasterPassword(mockMasterPassword, mockUserId);
expect(result).toEqual(mockUserKey);
expect(masterPasswordService.masterPasswordUnlockData$).toHaveBeenCalledWith(mockUserId);
expect(masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData).toHaveBeenCalledWith(
mockMasterPassword,
mockMasterPasswordUnlockData,
);
expect(keyService.makeMasterKey).toHaveBeenCalledWith(
mockMasterPassword,
mockMasterPasswordUnlockData.salt,
mockMasterPasswordUnlockData.kdf,
);
expect(keyService.hashMasterKey).toHaveBeenCalledWith(
mockMasterPassword,
mockMasterKey,
HashPurpose.LocalAuthorization,
);
expect(masterPasswordService.setMasterKeyHash).toHaveBeenCalledWith(mockKeyHash, mockUserId);
expect(masterPasswordService.setMasterKey).toHaveBeenCalledWith(mockMasterKey, mockUserId);
});
it("throws an error if masterKey construction fails", async () => {
keyService.makeMasterKey.mockResolvedValue(null as unknown as MasterKey);
await expect(sut.unlockWithMasterPassword(mockMasterPassword, mockUserId)).rejects.toThrow(
"Master key could not be created to set legacy master password state.",
);
expect(masterPasswordService.masterPasswordUnlockData$).toHaveBeenCalledWith(mockUserId);
expect(masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData).toHaveBeenCalledWith(
mockMasterPassword,
mockMasterPasswordUnlockData,
);
expect(keyService.makeMasterKey).toHaveBeenCalledWith(
mockMasterPassword,
mockMasterPasswordUnlockData.salt,
mockMasterPasswordUnlockData.kdf,
);
expect(keyService.hashMasterKey).not.toHaveBeenCalled();
expect(masterPasswordService.setMasterKeyHash).not.toHaveBeenCalled();
expect(masterPasswordService.setMasterKey).not.toHaveBeenCalled();
});
});
});

View File

@@ -0,0 +1,75 @@
import { firstValueFrom } from "rxjs";
// eslint-disable-next-line no-restricted-imports
import { KeyService } from "@bitwarden/key-management";
import { UserId } from "@bitwarden/user-core";
import { HashPurpose } from "../../../platform/enums";
import { UserKey } from "../../../types/key";
import { MasterPasswordUnlockService } from "../abstractions/master-password-unlock.service";
import { InternalMasterPasswordServiceAbstraction } from "../abstractions/master-password.service.abstraction";
import { MasterPasswordUnlockData } from "../types/master-password.types";
export class DefaultMasterPasswordUnlockService implements MasterPasswordUnlockService {
constructor(
private readonly masterPasswordService: InternalMasterPasswordServiceAbstraction,
private readonly keyService: KeyService,
) {}
async unlockWithMasterPassword(masterPassword: string, userId: UserId): Promise<UserKey> {
this.validateInput(masterPassword, userId);
const masterPasswordUnlockData = await firstValueFrom(
this.masterPasswordService.masterPasswordUnlockData$(userId),
);
if (masterPasswordUnlockData == null) {
throw new Error("Master password unlock data was not found for the user " + userId);
}
const userKey = await this.masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData(
masterPassword,
masterPasswordUnlockData,
);
await this.setLegacyState(masterPassword, masterPasswordUnlockData, userId);
return userKey;
}
private validateInput(masterPassword: string, userId: UserId): void {
if (masterPassword == null || masterPassword === "") {
throw new Error("Master password is required");
}
if (userId == null) {
throw new Error("User ID is required");
}
}
// Previously unlocking had the side effect of setting the masterKey and masterPasswordHash in state.
// This is to preserve that behavior, once masterKey and masterPasswordHash state is removed this should be removed as well.
private async setLegacyState(
masterPassword: string,
masterPasswordUnlockData: MasterPasswordUnlockData,
userId: UserId,
): Promise<void> {
const masterKey = await this.keyService.makeMasterKey(
masterPassword,
masterPasswordUnlockData.salt,
masterPasswordUnlockData.kdf,
);
if (!masterKey) {
throw new Error("Master key could not be created to set legacy master password state.");
}
const localKeyHash = await this.keyService.hashMasterKey(
masterPassword,
masterKey,
HashPurpose.LocalAuthorization,
);
await this.masterPasswordService.setMasterKeyHash(localKeyHash, userId);
await this.masterPasswordService.setMasterKey(masterKey, userId);
}
}

View File

@@ -119,4 +119,8 @@ export class FakeMasterPasswordService implements InternalMasterPasswordServiceA
): Promise<void> {
return this.mock.setMasterPasswordUnlockData(masterPasswordUnlockData, userId);
}
masterPasswordUnlockData$(userId: UserId): Observable<MasterPasswordUnlockData | null> {
return this.mock.masterPasswordUnlockData$(userId);
}
}

View File

@@ -1,6 +1,5 @@
import { mock, MockProxy } from "jest-mock-extended";
import * as rxjs from "rxjs";
import { firstValueFrom, of } from "rxjs";
import { firstValueFrom } from "rxjs";
import { Jsonify } from "type-fest";
import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-load.service";
@@ -10,6 +9,7 @@ import { Argon2KdfConfig, KdfConfig, KdfType, PBKDF2KdfConfig } from "@bitwarden
import {
FakeAccountService,
FakeStateProvider,
makeSymmetricCryptoKey,
mockAccountServiceWith,
} from "../../../../spec";
@@ -17,7 +17,6 @@ import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-pa
import { LogService } from "../../../platform/abstractions/log.service";
import { StateService } from "../../../platform/abstractions/state.service";
import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key";
import { StateProvider } from "../../../platform/state";
import { UserId } from "../../../types/guid";
import { MasterKey, UserKey } from "../../../types/key";
import { KeyGenerationService } from "../../crypto";
@@ -30,25 +29,30 @@ import {
MasterPasswordUnlockData,
} from "../types/master-password.types";
import { MASTER_PASSWORD_UNLOCK_KEY, MasterPasswordService } from "./master-password.service";
import {
FORCE_SET_PASSWORD_REASON,
MASTER_KEY_ENCRYPTED_USER_KEY,
MASTER_PASSWORD_UNLOCK_KEY,
MasterPasswordService,
} from "./master-password.service";
describe("MasterPasswordService", () => {
let sut: MasterPasswordService;
let stateProvider: MockProxy<StateProvider>;
let stateService: MockProxy<StateService>;
let keyGenerationService: MockProxy<KeyGenerationService>;
let encryptService: MockProxy<EncryptService>;
let logService: MockProxy<LogService>;
let cryptoFunctionService: MockProxy<CryptoFunctionService>;
let accountService: FakeAccountService;
let stateProvider: FakeStateProvider;
const userId = "00000000-0000-0000-0000-000000000000" as UserId;
const mockUserState = {
state$: of(null),
update: jest.fn().mockResolvedValue(null),
};
const kdfPBKDF2: KdfConfig = new PBKDF2KdfConfig(600_000);
const kdfArgon2: KdfConfig = new Argon2KdfConfig(4, 64, 3);
const salt = "test@bitwarden.com" as MasterPasswordSalt;
const userKey = makeSymmetricCryptoKey(64, 2) as UserKey;
const testUserKey: SymmetricCryptoKey = makeSymmetricCryptoKey(64, 1);
const testMasterKey: MasterKey = makeSymmetricCryptoKey(32, 2);
const testStretchedMasterKey: SymmetricCryptoKey = makeSymmetricCryptoKey(64, 3);
@@ -58,17 +62,13 @@ describe("MasterPasswordService", () => {
"2.gbauOANURUHqvhLTDnva1A==|nSW+fPumiuTaDB/s12+JO88uemV6rhwRSR+YR1ZzGr5j6Ei3/h+XEli2Unpz652NlZ9NTuRpHxeOqkYYJtp7J+lPMoclgteXuAzUu9kqlRc=|DeUFkhIwgkGdZA08bDnDqMMNmZk21D+H5g8IostPKAY=";
beforeEach(() => {
stateProvider = mock<StateProvider>();
stateService = mock<StateService>();
keyGenerationService = mock<KeyGenerationService>();
encryptService = mock<EncryptService>();
logService = mock<LogService>();
cryptoFunctionService = mock<CryptoFunctionService>();
accountService = mockAccountServiceWith(userId);
stateProvider.getUser.mockReturnValue(mockUserState as any);
mockUserState.update.mockReset();
stateProvider = new FakeStateProvider(accountService);
sut = new MasterPasswordService(
stateProvider,
@@ -88,6 +88,10 @@ describe("MasterPasswordService", () => {
});
});
afterEach(() => {
jest.resetAllMocks();
});
describe("saltForUser$", () => {
it("throws when userid not present", async () => {
expect(() => {
@@ -111,12 +115,10 @@ describe("MasterPasswordService", () => {
await sut.setForceSetPasswordReason(reason, userId);
expect(stateProvider.getUser).toHaveBeenCalled();
expect(mockUserState.update).toHaveBeenCalled();
// Call the update function to verify it returns the correct reason
const updateFn = mockUserState.update.mock.calls[0][0];
expect(updateFn(null)).toBe(reason);
const state = await firstValueFrom(
stateProvider.getUser(userId, FORCE_SET_PASSWORD_REASON).state$,
);
expect(state).toEqual(reason);
});
it("throws an error if reason is null", async () => {
@@ -132,31 +134,29 @@ describe("MasterPasswordService", () => {
});
it("does not overwrite AdminForcePasswordReset with other reasons except None", async () => {
jest
.spyOn(sut, "forceSetPasswordReason$")
.mockReturnValue(of(ForceSetPasswordReason.AdminForcePasswordReset));
jest
.spyOn(rxjs, "firstValueFrom")
.mockResolvedValue(ForceSetPasswordReason.AdminForcePasswordReset);
stateProvider.singleUser
.getFake(userId, FORCE_SET_PASSWORD_REASON)
.nextState(ForceSetPasswordReason.AdminForcePasswordReset);
await sut.setForceSetPasswordReason(ForceSetPasswordReason.WeakMasterPassword, userId);
expect(mockUserState.update).not.toHaveBeenCalled();
const state = await firstValueFrom(
stateProvider.getUser(userId, FORCE_SET_PASSWORD_REASON).state$,
);
expect(state).toEqual(ForceSetPasswordReason.AdminForcePasswordReset);
});
it("allows overwriting AdminForcePasswordReset with None", async () => {
jest
.spyOn(sut, "forceSetPasswordReason$")
.mockReturnValue(of(ForceSetPasswordReason.AdminForcePasswordReset));
jest
.spyOn(rxjs, "firstValueFrom")
.mockResolvedValue(ForceSetPasswordReason.AdminForcePasswordReset);
stateProvider.singleUser
.getFake(userId, FORCE_SET_PASSWORD_REASON)
.nextState(ForceSetPasswordReason.AdminForcePasswordReset);
await sut.setForceSetPasswordReason(ForceSetPasswordReason.None, userId);
expect(mockUserState.update).toHaveBeenCalled();
const state = await firstValueFrom(
stateProvider.getUser(userId, FORCE_SET_PASSWORD_REASON).state$,
);
expect(state).toEqual(ForceSetPasswordReason.None);
});
});
describe("decryptUserKeyWithMasterKey", () => {
@@ -227,10 +227,10 @@ describe("MasterPasswordService", () => {
await sut.setMasterKeyEncryptedUserKey(encryptedKey, userId);
expect(stateProvider.getUser).toHaveBeenCalled();
expect(mockUserState.update).toHaveBeenCalled();
const updateFn = mockUserState.update.mock.calls[0][0];
expect(updateFn(null)).toEqual(encryptedKey.toJSON());
const state = await firstValueFrom(
stateProvider.getUser(userId, MASTER_KEY_ENCRYPTED_USER_KEY).state$,
);
expect(state).toEqual(encryptedKey.toJSON());
});
});
@@ -328,11 +328,6 @@ describe("MasterPasswordService", () => {
});
describe("setMasterPasswordUnlockData", () => {
const kdfPBKDF2: KdfConfig = new PBKDF2KdfConfig(600_000);
const kdfArgon2: KdfConfig = new Argon2KdfConfig(4, 64, 3);
const salt = "test@bitwarden.com" as MasterPasswordSalt;
const userKey = makeSymmetricCryptoKey(64, 2) as UserKey;
it.each([kdfPBKDF2, kdfArgon2])(
"sets the master password unlock data kdf %o in the state",
async (kdfConfig) => {
@@ -345,11 +340,10 @@ describe("MasterPasswordService", () => {
await sut.setMasterPasswordUnlockData(masterPasswordUnlockData, userId);
expect(stateProvider.getUser).toHaveBeenCalledWith(userId, MASTER_PASSWORD_UNLOCK_KEY);
expect(mockUserState.update).toHaveBeenCalled();
const updateFn = mockUserState.update.mock.calls[0][0];
expect(updateFn(null)).toEqual(masterPasswordUnlockData.toJSON());
const state = await firstValueFrom(
stateProvider.getUser(userId, MASTER_PASSWORD_UNLOCK_KEY).state$,
);
expect(state).toEqual(masterPasswordUnlockData.toJSON());
},
);
@@ -373,6 +367,40 @@ describe("MasterPasswordService", () => {
});
});
describe("masterPasswordUnlockData$", () => {
test.each([null as unknown as UserId, undefined as unknown as UserId])(
"throws when the provided userId is %s",
async (userId) => {
expect(() => sut.masterPasswordUnlockData$(userId)).toThrow("userId is null or undefined.");
},
);
it("returns null when no data is set", async () => {
stateProvider.singleUser.getFake(userId, MASTER_PASSWORD_UNLOCK_KEY).nextState(null);
const result = await firstValueFrom(sut.masterPasswordUnlockData$(userId));
expect(result).toBeNull();
});
it.each([kdfPBKDF2, kdfArgon2])(
"returns the master password unlock data for kdf %o from state",
async (kdfConfig) => {
const masterPasswordUnlockData = await sut.makeMasterPasswordUnlockData(
"test-password",
kdfConfig,
salt,
userKey,
);
await sut.setMasterPasswordUnlockData(masterPasswordUnlockData, userId);
const result = await firstValueFrom(sut.masterPasswordUnlockData$(userId));
expect(result).toEqual(masterPasswordUnlockData.toJSON());
},
);
});
describe("MASTER_PASSWORD_UNLOCK_KEY", () => {
it("has the correct configuration", () => {
expect(MASTER_PASSWORD_UNLOCK_KEY.stateDefinition).toBeDefined();

View File

@@ -50,7 +50,7 @@ const MASTER_KEY_HASH = new UserKeyDefinition<string>(MASTER_PASSWORD_DISK, "mas
});
/** Disk to persist through lock */
const MASTER_KEY_ENCRYPTED_USER_KEY = new UserKeyDefinition<EncryptedString>(
export const MASTER_KEY_ENCRYPTED_USER_KEY = new UserKeyDefinition<EncryptedString>(
MASTER_PASSWORD_DISK,
"masterKeyEncryptedUserKey",
{
@@ -60,7 +60,7 @@ const MASTER_KEY_ENCRYPTED_USER_KEY = new UserKeyDefinition<EncryptedString>(
);
/** Disk to persist through lock and account switches */
const FORCE_SET_PASSWORD_REASON = new UserKeyDefinition<ForceSetPasswordReason>(
export const FORCE_SET_PASSWORD_REASON = new UserKeyDefinition<ForceSetPasswordReason>(
MASTER_PASSWORD_DISK,
"forceSetPasswordReason",
{
@@ -344,4 +344,10 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr
.getUser(userId, MASTER_PASSWORD_UNLOCK_KEY)
.update(() => masterPasswordUnlockData.toJSON());
}
masterPasswordUnlockData$(userId: UserId): Observable<MasterPasswordUnlockData | null> {
assertNonNullish(userId, "userId");
return this.stateProvider.getUser(userId, MASTER_PASSWORD_UNLOCK_KEY).state$;
}
}