mirror of
https://github.com/bitwarden/browser
synced 2025-12-06 00:13:28 +00:00
[PM-24683] Move change kdf service to SDK implementation (#16001)
* Add new mp service api * Fix tests * Add test coverage * Add newline * Fix type * Rename to "unwrapUserKeyFromMasterPasswordUnlockData" * Fix build * Fix build on cli * Fix linting * Re-sort spec * Add tests * Fix test and build issues * Fix build * Clean up * Remove introduced function * Clean up comments * Fix abstract class types * Fix comments * Cleanup * Cleanup * Update libs/common/src/key-management/master-password/types/master-password.types.ts Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> * Update libs/common/src/key-management/master-password/services/master-password.service.ts Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> * Update libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> * Update libs/common/src/key-management/master-password/types/master-password.types.ts Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> * Update libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> * Add comments * Fix build * Add arg null check * Cleanup * Fix build * Fix build on browser * Implement KDF change service * Deprecate encryptUserKeyWithMasterKey * Update libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> * Update libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> * Update libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> * Add tests for null params * Fix builds * Cleanup and deprecate more functions * Fix formatting * Prettier * Clean up * Update libs/key-management/src/abstractions/key.service.ts Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> * Make emailToSalt private and expose abstract saltForUser * Add tests * Add docs * Fix build * Fix tests * Fix tests * Address feedback and fix primitive obsession * Consolidate active account checks in change kdf confirmation component * Update libs/common/src/key-management/kdf/services/change-kdf-service.spec.ts Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> * Add defensive parameter checks * Add tests * Add comment for follow-up epic * Move change kdf service, remove abstraction and add api service * Fix test * Drop redundant null check * Address feedback * Add throw on empty password * Fix tests * Mark change kdf service as internal * Add abstract classes * Switch to abstraction * Move change kdf to sdk * Update tests * Fix tests * Clean up sdk mapping * Clean up tests * Check the argument to make_update_kdf * Fix mock data * Fix tests * Fix relative imports --------- Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com>
This commit is contained in:
Bernd Schoolmann
GitHub
@@ -1,14 +1,14 @@
|
||||
import { mock } from "jest-mock-extended";
|
||||
import { of } from "rxjs";
|
||||
|
||||
import { KdfRequest } from "@bitwarden/common/models/request/kdf.request";
|
||||
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
import { UserKey } from "@bitwarden/common/types/key";
|
||||
// eslint-disable-next-line no-restricted-imports
|
||||
import { KdfConfigService, KeyService, PBKDF2KdfConfig } from "@bitwarden/key-management";
|
||||
import { PBKDF2KdfConfig } from "@bitwarden/key-management";
|
||||
|
||||
import { MasterPasswordServiceAbstraction } from "../master-password/abstractions/master-password.service.abstraction";
|
||||
import { makeEncString } from "../../../spec";
|
||||
import { KdfRequest } from "../../models/request/kdf.request";
|
||||
import { SdkService } from "../../platform/abstractions/sdk/sdk.service";
|
||||
import { UserId } from "../../types/guid";
|
||||
import { EncString } from "../crypto/models/enc-string";
|
||||
import {
|
||||
MasterKeyWrappedUserKey,
|
||||
MasterPasswordAuthenticationHash,
|
||||
@@ -21,35 +21,63 @@ import { DefaultChangeKdfService } from "./change-kdf-service";
|
||||
|
||||
describe("ChangeKdfService", () => {
|
||||
const changeKdfApiService = mock<ChangeKdfApiService>();
|
||||
const masterPasswordService = mock<MasterPasswordServiceAbstraction>();
|
||||
const keyService = mock<KeyService>();
|
||||
const kdfConfigService = mock<KdfConfigService>();
|
||||
const sdkService = mock<SdkService>();
|
||||
|
||||
let sut: DefaultChangeKdfService = mock<DefaultChangeKdfService>();
|
||||
let sut: DefaultChangeKdfService;
|
||||
|
||||
const mockUserKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey;
|
||||
const mockOldKdfConfig = new PBKDF2KdfConfig(100000);
|
||||
const mockNewKdfConfig = new PBKDF2KdfConfig(200000);
|
||||
const mockOldKdfConfig = new PBKDF2KdfConfig(100000);
|
||||
const mockOldHash = "oldHash" as MasterPasswordAuthenticationHash;
|
||||
const mockNewHash = "newHash" as MasterPasswordAuthenticationHash;
|
||||
const mockUserId = "00000000-0000-0000-0000-000000000000" as UserId;
|
||||
const mockSalt = "test@bitwarden.com" as MasterPasswordSalt;
|
||||
const mockWrappedUserKey = "wrappedUserKey";
|
||||
const mockWrappedUserKey: EncString = makeEncString("wrappedUserKey");
|
||||
|
||||
const mockSdkClient = {
|
||||
crypto: jest.fn().mockReturnValue({
|
||||
make_update_kdf: jest.fn(),
|
||||
}),
|
||||
};
|
||||
const mockRef = {
|
||||
value: mockSdkClient,
|
||||
[Symbol.dispose]: jest.fn(),
|
||||
};
|
||||
const mockSdk = {
|
||||
take: jest.fn().mockReturnValue(mockRef),
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
sut = new DefaultChangeKdfService(
|
||||
masterPasswordService,
|
||||
keyService,
|
||||
kdfConfigService,
|
||||
changeKdfApiService,
|
||||
);
|
||||
sdkService.userClient$ = jest.fn((userId: UserId) => of(mockSdk)) as any;
|
||||
sut = new DefaultChangeKdfService(changeKdfApiService, sdkService);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
jest.resetAllMocks();
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
describe("updateUserKdfParams", () => {
|
||||
const mockUpdateKdfResult = {
|
||||
masterPasswordAuthenticationData: {
|
||||
kdf: mockNewKdfConfig.toSdkConfig(),
|
||||
salt: mockSalt,
|
||||
masterPasswordAuthenticationHash: mockNewHash,
|
||||
},
|
||||
masterPasswordUnlockData: {
|
||||
kdf: mockNewKdfConfig.toSdkConfig(),
|
||||
salt: mockSalt,
|
||||
masterKeyWrappedUserKey: mockWrappedUserKey.encryptedString,
|
||||
},
|
||||
oldMasterPasswordAuthenticationData: {
|
||||
kdf: mockOldKdfConfig.toSdkConfig(),
|
||||
salt: mockSalt,
|
||||
masterPasswordAuthenticationHash: mockOldHash,
|
||||
},
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
mockSdkClient.crypto().make_update_kdf.mockReturnValue(mockUpdateKdfResult);
|
||||
});
|
||||
|
||||
it("should throw an error if masterPassword is null", async () => {
|
||||
await expect(
|
||||
sut.updateUserKdfParams(null as unknown as string, mockNewKdfConfig, mockUserId),
|
||||
@@ -90,61 +118,31 @@ describe("ChangeKdfService", () => {
|
||||
).rejects.toThrow("userId");
|
||||
});
|
||||
|
||||
it("should throw an error if userKey is null", async () => {
|
||||
keyService.userKey$.mockReturnValueOnce(of(null));
|
||||
masterPasswordService.saltForUser$.mockReturnValueOnce(of(mockSalt));
|
||||
kdfConfigService.getKdfConfig$.mockReturnValueOnce(of(mockOldKdfConfig));
|
||||
it("should throw an error if SDK is not available", async () => {
|
||||
sdkService.userClient$ = jest.fn().mockReturnValue(of(null)) as any;
|
||||
|
||||
await expect(
|
||||
sut.updateUserKdfParams("masterPassword", mockNewKdfConfig, mockUserId),
|
||||
).rejects.toThrow();
|
||||
).rejects.toThrow("SDK not available");
|
||||
});
|
||||
|
||||
it("should throw an error if salt is null", async () => {
|
||||
keyService.userKey$.mockReturnValueOnce(of(mockUserKey));
|
||||
masterPasswordService.saltForUser$.mockReturnValueOnce(of(null));
|
||||
kdfConfigService.getKdfConfig$.mockReturnValueOnce(of(mockOldKdfConfig));
|
||||
await expect(
|
||||
sut.updateUserKdfParams("masterPassword", mockNewKdfConfig, mockUserId),
|
||||
).rejects.toThrow("Failed to get salt");
|
||||
});
|
||||
it("should call SDK update_kdf with correct parameters", async () => {
|
||||
const masterPassword = "masterPassword";
|
||||
|
||||
it("should throw an error if oldKdfConfig is null", async () => {
|
||||
keyService.userKey$.mockReturnValueOnce(of(mockUserKey));
|
||||
masterPasswordService.saltForUser$.mockReturnValueOnce(of(mockSalt));
|
||||
kdfConfigService.getKdfConfig$.mockReturnValueOnce(of(null));
|
||||
await expect(
|
||||
sut.updateUserKdfParams("masterPassword", mockNewKdfConfig, mockUserId),
|
||||
).rejects.toThrow("Failed to get oldKdfConfig");
|
||||
});
|
||||
await sut.updateUserKdfParams(masterPassword, mockNewKdfConfig, mockUserId);
|
||||
|
||||
it("should call apiService.send with correct parameters", async () => {
|
||||
keyService.userKey$.mockReturnValueOnce(of(mockUserKey));
|
||||
masterPasswordService.saltForUser$.mockReturnValueOnce(of(mockSalt));
|
||||
kdfConfigService.getKdfConfig$.mockReturnValueOnce(of(mockOldKdfConfig));
|
||||
|
||||
masterPasswordService.makeMasterPasswordAuthenticationData
|
||||
.mockResolvedValueOnce({
|
||||
salt: mockSalt,
|
||||
kdf: mockOldKdfConfig,
|
||||
masterPasswordAuthenticationHash: mockOldHash,
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
salt: mockSalt,
|
||||
kdf: mockNewKdfConfig,
|
||||
masterPasswordAuthenticationHash: mockNewHash,
|
||||
});
|
||||
|
||||
masterPasswordService.makeMasterPasswordUnlockData.mockResolvedValueOnce(
|
||||
new MasterPasswordUnlockData(
|
||||
mockSalt,
|
||||
mockNewKdfConfig,
|
||||
mockWrappedUserKey as MasterKeyWrappedUserKey,
|
||||
),
|
||||
expect(mockSdkClient.crypto().make_update_kdf).toHaveBeenCalledWith(
|
||||
masterPassword,
|
||||
mockNewKdfConfig.toSdkConfig(),
|
||||
);
|
||||
});
|
||||
|
||||
await sut.updateUserKdfParams("masterPassword", mockNewKdfConfig, mockUserId);
|
||||
it("should call changeKdfApiService.updateUserKdfParams with correct request", async () => {
|
||||
const masterPassword = "masterPassword";
|
||||
|
||||
const expected = new KdfRequest(
|
||||
await sut.updateUserKdfParams(masterPassword, mockNewKdfConfig, mockUserId);
|
||||
|
||||
const expectedRequest = new KdfRequest(
|
||||
{
|
||||
salt: mockSalt,
|
||||
kdf: mockNewKdfConfig,
|
||||
@@ -153,15 +151,38 @@ describe("ChangeKdfService", () => {
|
||||
new MasterPasswordUnlockData(
|
||||
mockSalt,
|
||||
mockNewKdfConfig,
|
||||
mockWrappedUserKey as MasterKeyWrappedUserKey,
|
||||
mockWrappedUserKey.encryptedString as MasterKeyWrappedUserKey,
|
||||
),
|
||||
).authenticateWith({
|
||||
);
|
||||
expectedRequest.authenticateWith({
|
||||
salt: mockSalt,
|
||||
kdf: mockOldKdfConfig,
|
||||
masterPasswordAuthenticationHash: mockOldHash,
|
||||
});
|
||||
|
||||
expect(changeKdfApiService.updateUserKdfParams).toHaveBeenCalledWith(expected);
|
||||
expect(changeKdfApiService.updateUserKdfParams).toHaveBeenCalledWith(expectedRequest);
|
||||
});
|
||||
|
||||
it("should properly dispose of SDK resources", async () => {
|
||||
const masterPassword = "masterPassword";
|
||||
jest.spyOn(mockNewKdfConfig, "toSdkConfig").mockReturnValue({} as any);
|
||||
|
||||
await sut.updateUserKdfParams(masterPassword, mockNewKdfConfig, mockUserId);
|
||||
|
||||
expect(mockRef[Symbol.dispose]).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should handle SDK errors properly", async () => {
|
||||
const masterPassword = "masterPassword";
|
||||
const sdkError = new Error("SDK update_kdf failed");
|
||||
jest.spyOn(mockNewKdfConfig, "toSdkConfig").mockReturnValue({} as any);
|
||||
mockSdkClient.crypto().make_update_kdf.mockImplementation(() => {
|
||||
throw sdkError;
|
||||
});
|
||||
|
||||
await expect(
|
||||
sut.updateUserKdfParams(masterPassword, mockNewKdfConfig, mockUserId),
|
||||
).rejects.toThrow("SDK update_kdf failed");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,55 +1,56 @@
|
||||
import { firstValueFrom, map } from "rxjs";
|
||||
|
||||
import { assertNonNullish } from "@bitwarden/common/auth/utils";
|
||||
import { KdfRequest } from "@bitwarden/common/models/request/kdf.request";
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
// eslint-disable-next-line no-restricted-imports
|
||||
import { KdfConfig, KdfConfigService, KeyService } from "@bitwarden/key-management";
|
||||
import { KdfConfig } from "@bitwarden/key-management";
|
||||
|
||||
import { MasterPasswordServiceAbstraction } from "../master-password/abstractions/master-password.service.abstraction";
|
||||
import { firstValueFromOrThrow } from "../utils";
|
||||
import { KdfRequest } from "../../models/request/kdf.request";
|
||||
import { SdkService } from "../../platform/abstractions/sdk/sdk.service";
|
||||
import {
|
||||
fromSdkAuthenticationData,
|
||||
MasterPasswordAuthenticationData,
|
||||
MasterPasswordUnlockData,
|
||||
} from "../master-password/types/master-password.types";
|
||||
|
||||
import { ChangeKdfApiService } from "./change-kdf-api.service.abstraction";
|
||||
import { ChangeKdfService } from "./change-kdf-service.abstraction";
|
||||
|
||||
export class DefaultChangeKdfService implements ChangeKdfService {
|
||||
constructor(
|
||||
private masterPasswordService: MasterPasswordServiceAbstraction,
|
||||
private keyService: KeyService,
|
||||
private kdfConfigService: KdfConfigService,
|
||||
private changeKdfApiService: ChangeKdfApiService,
|
||||
private sdkService: SdkService,
|
||||
) {}
|
||||
|
||||
async updateUserKdfParams(masterPassword: string, kdf: KdfConfig, userId: UserId): Promise<void> {
|
||||
assertNonNullish(masterPassword, "masterPassword");
|
||||
assertNonNullish(kdf, "kdf");
|
||||
assertNonNullish(userId, "userId");
|
||||
const updateKdfResult = await firstValueFrom(
|
||||
this.sdkService.userClient$(userId).pipe(
|
||||
map((sdk) => {
|
||||
if (!sdk) {
|
||||
throw new Error("SDK not available");
|
||||
}
|
||||
|
||||
const userKey = await firstValueFromOrThrow(this.keyService.userKey$(userId), "userKey");
|
||||
const salt = await firstValueFromOrThrow(
|
||||
this.masterPasswordService.saltForUser$(userId),
|
||||
"salt",
|
||||
);
|
||||
const oldKdfConfig = await firstValueFromOrThrow(
|
||||
this.kdfConfigService.getKdfConfig$(userId),
|
||||
"oldKdfConfig",
|
||||
using ref = sdk.take();
|
||||
|
||||
const updateKdfResponse = ref.value
|
||||
.crypto()
|
||||
.make_update_kdf(masterPassword, kdf.toSdkConfig());
|
||||
return updateKdfResponse;
|
||||
}),
|
||||
),
|
||||
);
|
||||
|
||||
const oldAuthenticationData =
|
||||
await this.masterPasswordService.makeMasterPasswordAuthenticationData(
|
||||
masterPassword,
|
||||
oldKdfConfig,
|
||||
salt,
|
||||
);
|
||||
const authenticationData =
|
||||
await this.masterPasswordService.makeMasterPasswordAuthenticationData(
|
||||
masterPassword,
|
||||
kdf,
|
||||
salt,
|
||||
);
|
||||
const unlockData = await this.masterPasswordService.makeMasterPasswordUnlockData(
|
||||
masterPassword,
|
||||
kdf,
|
||||
salt,
|
||||
userKey,
|
||||
const authenticationData: MasterPasswordAuthenticationData = fromSdkAuthenticationData(
|
||||
updateKdfResult.masterPasswordAuthenticationData,
|
||||
);
|
||||
const unlockData: MasterPasswordUnlockData = MasterPasswordUnlockData.fromSdk(
|
||||
updateKdfResult.masterPasswordUnlockData,
|
||||
);
|
||||
const oldAuthenticationData: MasterPasswordAuthenticationData = fromSdkAuthenticationData(
|
||||
updateKdfResult.oldMasterPasswordAuthenticationData,
|
||||
);
|
||||
|
||||
const request = new KdfRequest(authenticationData, unlockData);
|
||||
|
||||
@@ -1,8 +1,18 @@
|
||||
import { Jsonify, Opaque } from "type-fest";
|
||||
|
||||
// eslint-disable-next-line no-restricted-imports
|
||||
import { Argon2KdfConfig, KdfConfig, KdfType, PBKDF2KdfConfig } from "@bitwarden/key-management";
|
||||
import { EncString } from "@bitwarden/sdk-internal";
|
||||
import {
|
||||
fromSdkKdfConfig,
|
||||
Argon2KdfConfig,
|
||||
KdfConfig,
|
||||
KdfType,
|
||||
PBKDF2KdfConfig,
|
||||
} from "@bitwarden/key-management";
|
||||
import {
|
||||
EncString,
|
||||
MasterPasswordUnlockData as SdkMasterPasswordUnlockData,
|
||||
MasterPasswordAuthenticationData as SdkMasterPasswordAuthenticationData,
|
||||
} from "@bitwarden/sdk-internal";
|
||||
|
||||
/**
|
||||
* The Base64-encoded master password authentication hash, that is sent to the server for authentication.
|
||||
@@ -24,6 +34,14 @@ export class MasterPasswordUnlockData {
|
||||
readonly masterKeyWrappedUserKey: MasterKeyWrappedUserKey,
|
||||
) {}
|
||||
|
||||
static fromSdk(sdkData: SdkMasterPasswordUnlockData): MasterPasswordUnlockData {
|
||||
return new MasterPasswordUnlockData(
|
||||
sdkData.salt as MasterPasswordSalt,
|
||||
fromSdkKdfConfig(sdkData.kdf),
|
||||
sdkData.masterKeyWrappedUserKey as MasterKeyWrappedUserKey,
|
||||
);
|
||||
}
|
||||
|
||||
toJSON(): any {
|
||||
return {
|
||||
salt: this.salt,
|
||||
@@ -55,3 +73,14 @@ export type MasterPasswordAuthenticationData = {
|
||||
kdf: KdfConfig;
|
||||
masterPasswordAuthenticationHash: MasterPasswordAuthenticationHash;
|
||||
};
|
||||
|
||||
export function fromSdkAuthenticationData(
|
||||
sdkData: SdkMasterPasswordAuthenticationData,
|
||||
): MasterPasswordAuthenticationData {
|
||||
return {
|
||||
salt: sdkData.salt as MasterPasswordSalt,
|
||||
kdf: fromSdkKdfConfig(sdkData.kdf),
|
||||
masterPasswordAuthenticationHash:
|
||||
sdkData.masterPasswordAuthenticationHash as MasterPasswordAuthenticationHash,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user