diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 954598de9c..f5642f45b2 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -1277,7 +1277,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: ChangeKdfService, useClass: DefaultChangeKdfService, - deps: [MasterPasswordServiceAbstraction, KeyService, KdfConfigService, ChangeKdfApiService], + deps: [ChangeKdfApiService, SdkService], }), safeProvider({ provide: AuthRequestServiceAbstraction, diff --git a/libs/common/src/key-management/kdf/change-kdf-service.spec.ts b/libs/common/src/key-management/kdf/change-kdf-service.spec.ts index 07cba1cc7e..c7df90f479 100644 --- a/libs/common/src/key-management/kdf/change-kdf-service.spec.ts +++ b/libs/common/src/key-management/kdf/change-kdf-service.spec.ts @@ -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(); - const masterPasswordService = mock(); - const keyService = mock(); - const kdfConfigService = mock(); + const sdkService = mock(); - let sut: DefaultChangeKdfService = mock(); + 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"); }); }); }); diff --git a/libs/common/src/key-management/kdf/change-kdf-service.ts b/libs/common/src/key-management/kdf/change-kdf-service.ts index d8bc3e21c1..64fbd1fce0 100644 --- a/libs/common/src/key-management/kdf/change-kdf-service.ts +++ b/libs/common/src/key-management/kdf/change-kdf-service.ts @@ -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 { 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); diff --git a/libs/common/src/key-management/master-password/types/master-password.types.ts b/libs/common/src/key-management/master-password/types/master-password.types.ts index 416b296623..19c8c49c11 100644 --- a/libs/common/src/key-management/master-password/types/master-password.types.ts +++ b/libs/common/src/key-management/master-password/types/master-password.types.ts @@ -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, + }; +} diff --git a/libs/key-management/src/index.ts b/libs/key-management/src/index.ts index d21b79540e..fb551b92cd 100644 --- a/libs/key-management/src/index.ts +++ b/libs/key-management/src/index.ts @@ -16,6 +16,7 @@ export { Argon2KdfConfig, KdfConfig, DEFAULT_KDF_CONFIG, + fromSdkKdfConfig, } from "./models/kdf-config"; export { KdfConfigService } from "./abstractions/kdf-config.service"; export { DefaultKdfConfigService } from "./kdf-config.service"; diff --git a/libs/key-management/src/models/kdf-config.ts b/libs/key-management/src/models/kdf-config.ts index a2ed8a2250..3296247710 100644 --- a/libs/key-management/src/models/kdf-config.ts +++ b/libs/key-management/src/models/kdf-config.ts @@ -145,4 +145,18 @@ export class Argon2KdfConfig { } } +export function fromSdkKdfConfig(sdkKdf: Kdf): KdfConfig { + if ("pBKDF2" in sdkKdf) { + return new PBKDF2KdfConfig(sdkKdf.pBKDF2.iterations); + } else if ("argon2id" in sdkKdf) { + return new Argon2KdfConfig( + sdkKdf.argon2id.iterations, + sdkKdf.argon2id.memory, + sdkKdf.argon2id.parallelism, + ); + } else { + throw new Error("Unsupported KDF type"); + } +} + export const DEFAULT_KDF_CONFIG = new PBKDF2KdfConfig(PBKDF2KdfConfig.ITERATIONS.defaultValue); diff --git a/package-lock.json b/package-lock.json index 2c1ca659d9..4e3707a60e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,7 +23,7 @@ "@angular/platform-browser": "19.2.14", "@angular/platform-browser-dynamic": "19.2.14", "@angular/router": "19.2.14", - "@bitwarden/sdk-internal": "0.2.0-main.311", + "@bitwarden/sdk-internal": "0.2.0-main.315", "@electron/fuses": "1.8.0", "@emotion/css": "11.13.5", "@koa/multer": "4.0.0", @@ -4690,9 +4690,9 @@ "link": true }, "node_modules/@bitwarden/sdk-internal": { - "version": "0.2.0-main.311", - "resolved": "https://registry.npmjs.org/@bitwarden/sdk-internal/-/sdk-internal-0.2.0-main.311.tgz", - "integrity": "sha512-zJdQykNMFOyivpNaCB9jc85wZ1ci2HM8/E4hI+yS7FgRm0sRigK5rieF3+xRjiq7pEsZSD8AucR+u/XK9ADXiw==", + "version": "0.2.0-main.315", + "resolved": "https://registry.npmjs.org/@bitwarden/sdk-internal/-/sdk-internal-0.2.0-main.315.tgz", + "integrity": "sha512-hdpFRLrDYSJ6+cNXfMyHdTgg/xIePIlEUSn4JWzwru4PvTcEkkFwGJM3L2LoUqTdNMiDQlr0UjDahopT+C2r0g==", "license": "GPL-3.0", "dependencies": { "type-fest": "^4.41.0" diff --git a/package.json b/package.json index 5f2bb4fdfe..4acf038b65 100644 --- a/package.json +++ b/package.json @@ -159,7 +159,7 @@ "@angular/platform-browser": "19.2.14", "@angular/platform-browser-dynamic": "19.2.14", "@angular/router": "19.2.14", - "@bitwarden/sdk-internal": "0.2.0-main.311", + "@bitwarden/sdk-internal": "0.2.0-main.315", "@electron/fuses": "1.8.0", "@emotion/css": "11.13.5", "@koa/multer": "4.0.0",