From 525e9f64fb6171d2fd33f4b0d6d27b692890c3c8 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Wed, 12 Aug 2026 13:51:34 -0400 Subject: [PATCH 1/3] feat(salt): [PM-27060] - Added in logic so the prelogin response will have the salt present for key derivation when using the sdk for prelogin. --- .../password-login.strategy.ts | 21 ++++++++++++++++++- .../default-password-prelogin.service.ts | 2 +- .../password-prelogin.model.ts | 17 ++++++++------- .../password-prelogin.response.ts | 15 +++++-------- 4 files changed, 36 insertions(+), 19 deletions(-) diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.ts b/libs/auth/src/common/login-strategies/password-login.strategy.ts index 787df39c517b..bcf31728a76a 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.ts @@ -17,6 +17,7 @@ import { PasswordPreloginData, PasswordPreloginService, } from "@bitwarden/common/auth/password-prelogin"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { UserId } from "@bitwarden/common/types/guid"; @@ -132,7 +133,25 @@ export class PasswordLoginStrategy extends LoginStrategy { ): Promise { // if we have prefetched prelogin data, use it if (preFetchedPreloginData) { - return this.keyService.makeMasterKey(masterPassword, email, preFetchedPreloginData.kdfConfig); + // If we are using the sdk to fetch the prelogin, then use the salt that is used. By not + // using the salt when the feature flag is off, this gives us the ability to turn off using + // the salt in the event of bad normalization occurring during the transition. + const useSdkForPrelogin = await this.configService.getFeatureFlag( + FeatureFlag.PM27060_PasswordPreloginFromSdk, + ); + if (useSdkForPrelogin) { + return this.keyService.makeMasterKey( + masterPassword, + preFetchedPreloginData.salt, + preFetchedPreloginData.kdfConfig, + ); + } else { + return this.keyService.makeMasterKey( + masterPassword, + email, + preFetchedPreloginData.kdfConfig, + ); + } } // No prefetched data — fetch now. PasswordPreloginData.fromResponse validates the KDF config. diff --git a/libs/common/src/auth/password-prelogin/default-password-prelogin.service.ts b/libs/common/src/auth/password-prelogin/default-password-prelogin.service.ts index 78ad82b2c71c..306e50782d55 100644 --- a/libs/common/src/auth/password-prelogin/default-password-prelogin.service.ts +++ b/libs/common/src/auth/password-prelogin/default-password-prelogin.service.ts @@ -76,6 +76,6 @@ export class DefaultPasswordPreloginService implements PasswordPreloginService { const sdkResponse: SdkPasswordPreloginResponse = await loginClient.get_password_prelogin(email); const kdfConfig = fromSdkKdfConfig(sdkResponse.kdf); kdfConfig.validateKdfConfigForPrelogin(); - return new PasswordPreloginData(kdfConfig); + return new PasswordPreloginData(kdfConfig, sdkResponse.salt); } } diff --git a/libs/common/src/auth/password-prelogin/password-prelogin.model.ts b/libs/common/src/auth/password-prelogin/password-prelogin.model.ts index 74e03e57511f..eb733a704e52 100644 --- a/libs/common/src/auth/password-prelogin/password-prelogin.model.ts +++ b/libs/common/src/auth/password-prelogin/password-prelogin.model.ts @@ -9,7 +9,10 @@ import { PasswordPreloginResponse } from "./password-prelogin.response"; * Contains the KDF configuration needed to derive the master key from the user's master password. */ export class PasswordPreloginData { - constructor(readonly kdfConfig: KdfConfig) {} + constructor( + readonly kdfConfig: KdfConfig, + readonly salt: string, + ) {} /** * Creates a PasswordPreloginData instance from a prelogin API response. @@ -17,14 +20,14 @@ export class PasswordPreloginData { */ static fromResponse(response: PasswordPreloginResponse): PasswordPreloginData { const kdfConfig = - response.kdf === KdfType.PBKDF2_SHA256 - ? new PBKDF2KdfConfig(response.kdfIterations) + response.kdfSettings.kdfType === KdfType.PBKDF2_SHA256 + ? new PBKDF2KdfConfig(response.kdfSettings.iterations) : new Argon2KdfConfig( - response.kdfIterations, - response.kdfMemory!, - response.kdfParallelism!, + response.kdfSettings.iterations, + response.kdfSettings.memory!, + response.kdfSettings.parallelism!, ); kdfConfig.validateKdfConfigForPrelogin(); - return new PasswordPreloginData(kdfConfig); + return new PasswordPreloginData(kdfConfig, response.salt); } } diff --git a/libs/common/src/auth/password-prelogin/password-prelogin.response.ts b/libs/common/src/auth/password-prelogin/password-prelogin.response.ts index a8ef72b29a1b..beab39e6931d 100644 --- a/libs/common/src/auth/password-prelogin/password-prelogin.response.ts +++ b/libs/common/src/auth/password-prelogin/password-prelogin.response.ts @@ -1,20 +1,15 @@ // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. -// eslint-disable-next-line no-restricted-imports -import { KdfType } from "@bitwarden/key-management"; +import { KdfConfigResponse } from "../../key-management/models/response/kdf-config.response"; import { BaseResponse } from "../../models/response/base.response"; export class PasswordPreloginResponse extends BaseResponse { - kdf: KdfType; - kdfIterations: number; - kdfMemory?: number; - kdfParallelism?: number; + kdfSettings: KdfConfigResponse; + salt: string; constructor(response: any) { super(response); - this.kdf = this.getResponseProperty("Kdf"); - this.kdfIterations = this.getResponseProperty("KdfIterations"); - this.kdfMemory = this.getResponseProperty("KdfMemory"); - this.kdfParallelism = this.getResponseProperty("KdfParallelism"); + this.kdfSettings = this.getResponseProperty("KdfSettings"); + this.salt = this.getResponseProperty("Salt"); } } From 97aebb2bc09f571c948f414d6957652a7f732770 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Wed, 12 Aug 2026 14:52:19 -0400 Subject: [PATCH 2/3] fix(salt): [PM-27060] - One more place needed logic added. --- .../password-login.strategy.ts | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.ts b/libs/auth/src/common/login-strategies/password-login.strategy.ts index 0f170f914fef..2e4b45883431 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.ts @@ -137,14 +137,16 @@ export class PasswordLoginStrategy extends LoginStrategy { email: string, preFetchedPreloginData?: PasswordPreloginData, ): Promise { + const useSdkForPrelogin = await this.configService.getFeatureFlag( + FeatureFlag.PM27060_PasswordPreloginFromSdk, + ); + // if we have prefetched prelogin data, use it if (preFetchedPreloginData) { - // If we are using the sdk to fetch the prelogin, then use the salt that is used. By not - // using the salt when the feature flag is off, this gives us the ability to turn off using - // the salt in the event of bad normalization occurring during the transition. - const useSdkForPrelogin = await this.configService.getFeatureFlag( - FeatureFlag.PM27060_PasswordPreloginFromSdk, - ); + // If we are using the sdk to fetch the prelogin data, only then do we want to + // use the salt that is passed back from the prelogin response in building the master key. + // This gives us the ability to turn off the feature of using the returned salt from salt + // in the event of bad normalization occurring during the transition. if (useSdkForPrelogin) { return this.legacyCompatKeyService.makeMasterKey( masterPassword, @@ -167,7 +169,23 @@ export class PasswordLoginStrategy extends LoginStrategy { throw new Error("KDF config is required"); } - return this.legacyCompatKeyService.makeMasterKey(masterPassword, email, preloginData.kdfConfig); + // If we are using the sdk to fetch the prelogin data, only then do we want to + // use the salt that is passed back from the prelogin response in building the master key. + // This gives us the ability to turn off the feature of using the returned salt from salt + // in the event of bad normalization occurring during the transition. + if (useSdkForPrelogin) { + return this.legacyCompatKeyService.makeMasterKey( + masterPassword, + preloginData.salt, + preloginData.kdfConfig, + ); + } else { + return this.legacyCompatKeyService.makeMasterKey( + masterPassword, + email, + preloginData.kdfConfig, + ); + } } private async evaluateMasterPasswordIfRequired( From 41847d199072e156fab54bc5b96c65773f9bd7da Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Wed, 12 Aug 2026 15:12:52 -0400 Subject: [PATCH 3/3] test(salt): [PM-27060] - Added tests. --- .../login-strategies/login.strategy.spec.ts | 2 +- .../password-login.strategy.spec.ts | 153 ++++++++++++++++-- .../login-strategy.service.spec.ts | 4 +- .../default-password-prelogin.service.spec.ts | 52 +++++- .../password-prelogin-api.service.spec.ts | 40 +++-- .../password-prelogin.model.spec.ts | 116 ++++++++++--- .../password-prelogin.model.ts | 20 +-- .../password-prelogin.response.ts | 2 +- 8 files changed, 329 insertions(+), 60 deletions(-) diff --git a/libs/auth/src/common/login-strategies/login.strategy.spec.ts b/libs/auth/src/common/login-strategies/login.strategy.spec.ts index bc96e1997532..53e5a19c3985 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -184,7 +184,7 @@ describe("LoginStrategy", () => { tokenService.decodeAccessToken.calledWith(accessToken).mockResolvedValue(decodedToken); passwordPreloginService.getPreloginData$.mockReturnValue( - of(new PasswordPreloginData(PBKDF2KdfConfig.createDefault())), + of(new PasswordPreloginData(PBKDF2KdfConfig.createDefault(), "prelogin-salt")), ); legacyCompatKeyService.makeMasterKey.mockResolvedValue({} as any); diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts index aa6534233c5a..8c24b7ee1de0 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts @@ -17,6 +17,7 @@ import { } from "@bitwarden/common/auth/password-prelogin"; import { TwoFactorService } from "@bitwarden/common/auth/two-factor"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { FakeMasterPasswordService } from "@bitwarden/common/key-management/master-password/services/fake-master-password.service"; @@ -51,6 +52,9 @@ import { PasswordLoginStrategy, PasswordLoginStrategyData } from "./password-log const email = "hello@world.com"; const masterPassword = "password"; const hashedPassword = "HASHED_PASSWORD"; +// Deliberately not equal to `email`. The whole point of PM-27060 is that the server dictates the +// KDF salt, so a fixture where salt === email would pass no matter which one the strategy used. +const preloginSalt = "server.normalized+salt@world.com"; const masterKey = new SymmetricCryptoKey( Utils.fromB64ToArray( "N2KWjlLpfi5uHjv+YcfUKIpZ1l+W+6HRensmIqD+BFYBf6N/dvFpJfWwYnVBdgFCK2tJTAIMLhqzIQQEUmGFgg==", @@ -62,6 +66,17 @@ const masterPasswordPolicyResponse = new MasterPasswordPolicyResponse({ EnforceOnLogin: true, MinLength: 8, }); +const kdfConfig = PBKDF2KdfConfig.createDefault(); + +function credentialsWithPrefetchedData(salt: string = preloginSalt) { + return new PasswordLoginCredentials( + email, + masterPassword, + undefined, + undefined, + new PasswordPreloginData(kdfConfig, salt), + ); +} describe("PasswordLoginStrategy", () => { let accountService: FakeAccountService; @@ -127,10 +142,14 @@ describe("PasswordLoginStrategy", () => { }); passwordPreloginService.getPreloginData$.mockReturnValue( - of(new PasswordPreloginData(PBKDF2KdfConfig.createDefault())), + of(new PasswordPreloginData(PBKDF2KdfConfig.createDefault(), preloginSalt)), ); legacyCompatKeyService.makeMasterKey.mockResolvedValue(masterKey); + // Default to the flag off so the pre-PM-27060 behavior stays the baseline; tests that exercise + // the SDK prelogin path opt in explicitly. + configService.getFeatureFlag.mockResolvedValue(false); + legacyCompatKeyService.hashMasterKey .calledWith(masterPassword, expect.anything()) .mockResolvedValue(hashedPassword); @@ -231,14 +250,7 @@ describe("PasswordLoginStrategy", () => { }); it("does not call getPreloginData$ when preFetchedPreloginData is provided", async () => { - const preloginData = new PasswordPreloginData(PBKDF2KdfConfig.createDefault()); - const credentialsWithPrefetch = new PasswordLoginCredentials( - email, - masterPassword, - undefined, - undefined, - preloginData, - ); + const credentialsWithPrefetch = credentialsWithPrefetchedData(); await passwordLoginStrategy.logIn(credentialsWithPrefetch); @@ -260,6 +272,129 @@ describe("PasswordLoginStrategy", () => { expect(passwordPreloginService.clearCache).toHaveBeenCalledTimes(1); }); + + // PM-27060: when prelogin comes from the SDK, the server dictates the KDF salt and the client + // must derive the master key from it rather than from the email the user typed. The flag gates + // this so it can be switched off if normalization diverges during the transition. + describe("salt selection", () => { + it("reads the PM27060_PasswordPreloginFromSdk flag", async () => { + await passwordLoginStrategy.logIn(credentials); + + expect(configService.getFeatureFlag).toHaveBeenCalledWith( + FeatureFlag.PM27060_PasswordPreloginFromSdk, + ); + }); + + describe("when the flag is on", () => { + beforeEach(() => { + configService.getFeatureFlag.mockResolvedValue(true); + }); + + it("derives the master key from the prelogin salt for prefetched data", async () => { + await passwordLoginStrategy.logIn(credentialsWithPrefetchedData()); + + expect(legacyCompatKeyService.makeMasterKey).toHaveBeenCalledWith( + masterPassword, + preloginSalt, + kdfConfig, + ); + expect(legacyCompatKeyService.makeMasterKey).not.toHaveBeenCalledWith( + masterPassword, + email, + expect.anything(), + ); + }); + + it("derives the master key from the prelogin salt for freshly fetched data", async () => { + // credentials from the outer beforeEach carries no prefetched data, so the strategy + // fetches via passwordPreloginService, which is stubbed to return preloginSalt. + await passwordLoginStrategy.logIn(credentials); + + expect(legacyCompatKeyService.makeMasterKey).toHaveBeenCalledWith( + masterPassword, + preloginSalt, + PBKDF2KdfConfig.createDefault(), + ); + expect(legacyCompatKeyService.makeMasterKey).not.toHaveBeenCalledWith( + masterPassword, + email, + expect.anything(), + ); + }); + + it("passes the salt through verbatim without re-normalizing it", async () => { + // The server owns normalization under PM-27060. Trimming or lower-casing here would + // produce a different master key than the one the vault was encrypted with. + const unnormalizedSalt = " MiXeD.Case@World.Com "; + + await passwordLoginStrategy.logIn(credentialsWithPrefetchedData(unnormalizedSalt)); + + expect(legacyCompatKeyService.makeMasterKey).toHaveBeenCalledWith( + masterPassword, + unnormalizedSalt, + kdfConfig, + ); + }); + + it("passes an absent salt straight through", async () => { + // PM-28143: Salt is nullable server-side during the transition. This documents current + // behavior — the strategy does not fall back to the email and does not throw. + // Built inline rather than via the helper, whose default parameter would substitute + // preloginSalt for an explicit undefined. + const noSaltCredentials = new PasswordLoginCredentials( + email, + masterPassword, + undefined, + undefined, + new PasswordPreloginData(kdfConfig, undefined as unknown as string), + ); + + await passwordLoginStrategy.logIn(noSaltCredentials); + + expect(legacyCompatKeyService.makeMasterKey).toHaveBeenCalledWith( + masterPassword, + undefined, + kdfConfig, + ); + }); + }); + + describe("when the flag is off", () => { + beforeEach(() => { + configService.getFeatureFlag.mockResolvedValue(false); + }); + + it("derives the master key from the entered email for prefetched data", async () => { + await passwordLoginStrategy.logIn(credentialsWithPrefetchedData()); + + expect(legacyCompatKeyService.makeMasterKey).toHaveBeenCalledWith( + masterPassword, + email, + kdfConfig, + ); + expect(legacyCompatKeyService.makeMasterKey).not.toHaveBeenCalledWith( + masterPassword, + preloginSalt, + expect.anything(), + ); + }); + + it("derives the master key from the entered email for freshly fetched data", async () => { + await passwordLoginStrategy.logIn(credentials); + + expect(legacyCompatKeyService.makeMasterKey).toHaveBeenCalledWith( + masterPassword, + email, + PBKDF2KdfConfig.createDefault(), + ); + expect(legacyCompatKeyService.makeMasterKey).not.toHaveBeenCalledWith( + masterPassword, + preloginSalt, + expect.anything(), + ); + }); + }); + }); }); describe("evaluateMasterPasswordIfRequired", () => { diff --git a/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts b/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts index 82c792ff9f58..316b65b03daf 100644 --- a/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts +++ b/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts @@ -64,7 +64,7 @@ import { UserDecryptionOptionsService } from "../user-decryption-options/user-de import { LoginStrategyService } from "./login-strategy.service"; import { CacheData } from "./login-strategy.state"; -const argon2PreloginData = new PasswordPreloginData(new Argon2KdfConfig(2, 16, 1)); +const argon2PreloginData = new PasswordPreloginData(new Argon2KdfConfig(2, 16, 1), "prelogin-salt"); describe("LoginStrategyService", () => { let sut: LoginStrategyService; @@ -162,7 +162,7 @@ describe("LoginStrategyService", () => { }); passwordPreloginService.getPreloginData$.mockReturnValue( - of(new PasswordPreloginData(PBKDF2KdfConfig.createDefault())), + of(new PasswordPreloginData(PBKDF2KdfConfig.createDefault(), "prelogin-salt")), ); legacyCompatKeyService.makeMasterKey.mockResolvedValue({} as any); diff --git a/libs/common/src/auth/password-prelogin/default-password-prelogin.service.spec.ts b/libs/common/src/auth/password-prelogin/default-password-prelogin.service.spec.ts index 6c73d1449717..d793805162ea 100644 --- a/libs/common/src/auth/password-prelogin/default-password-prelogin.service.spec.ts +++ b/libs/common/src/auth/password-prelogin/default-password-prelogin.service.spec.ts @@ -36,17 +36,26 @@ describe("DefaultPasswordPreloginService", () => { const emailB = "b@example.com"; const identityUrl = "https://identity.bitwarden.com"; + // The API and SDK paths return different salts so tests can prove which source was used. + const apiSalt = "api-salt"; + const sdkSalt = "sdk-salt"; + // PBKDF2 is used as a stand-in throughout; KDF type coverage is in password-prelogin.model.spec.ts. const response = new PasswordPreloginResponse({ - Kdf: 0, - KdfIterations: PBKDF2KdfConfig.ITERATIONS.defaultValue, + KdfSettings: { KdfType: 0, Iterations: PBKDF2KdfConfig.ITERATIONS.defaultValue }, + Salt: apiSalt, }); const sdkResponse: SdkPasswordPreloginResponse = { kdf: { pBKDF2: { iterations: PBKDF2KdfConfig.ITERATIONS.defaultValue } }, - salt: "test-salt", + salt: sdkSalt, }; const expectedData = new PasswordPreloginData( new PBKDF2KdfConfig(PBKDF2KdfConfig.ITERATIONS.defaultValue), + apiSalt, + ); + const expectedSdkData = new PasswordPreloginData( + new PBKDF2KdfConfig(PBKDF2KdfConfig.ITERATIONS.defaultValue), + sdkSalt, ); beforeEach(() => { @@ -93,10 +102,45 @@ describe("DefaultPasswordPreloginService", () => { const result = await firstValueFrom(sut.getPreloginData$(email)); - expect(result).toEqual(expectedData); + expect(result).toEqual(expectedSdkData); expect(apiService.getPreloginData).not.toHaveBeenCalled(); }); + it("carries the SDK-supplied salt onto the model when the flag is on", async () => { + configService.getFeatureFlag.mockResolvedValue(true); + + const result = await firstValueFrom(sut.getPreloginData$(email)); + + // PasswordLoginStrategy derives the master key from this salt when the flag is on, so it + // must come from the SDK response rather than the email the caller passed in. + expect(result.salt).toBe(sdkSalt); + expect(result.salt).not.toBe(email); + }); + + it("carries the API-supplied salt onto the model when the flag is off", async () => { + const result = await firstValueFrom(sut.getPreloginData$(email)); + + // The salt is still mapped when the flag is off — PasswordLoginStrategy simply ignores it + // and derives from the entered email instead. + expect(result.salt).toBe(apiSalt); + }); + + it("emits an undefined salt when the SDK omits one", async () => { + // PM-28143: salt is nullable while the server transition is in flight. Documents current + // behavior — the service does not substitute a fallback. + configService.getFeatureFlag.mockResolvedValue(true); + sdkService.client.auth + .mockDeep() + .login.mockDeep() + .get_password_prelogin.mockResolvedValue({ + kdf: { pBKDF2: { iterations: PBKDF2KdfConfig.ITERATIONS.defaultValue } }, + } as SdkPasswordPreloginResponse); + + const result = await firstValueFrom(sut.getPreloginData$(email)); + + expect(result.salt).toBeUndefined(); + }); + it("checks the feature flag with the expected key", async () => { await firstValueFrom(sut.getPreloginData$(email)); diff --git a/libs/common/src/auth/password-prelogin/password-prelogin-api.service.spec.ts b/libs/common/src/auth/password-prelogin/password-prelogin-api.service.spec.ts index 14cb522151ff..195f062984f8 100644 --- a/libs/common/src/auth/password-prelogin/password-prelogin-api.service.spec.ts +++ b/libs/common/src/auth/password-prelogin/password-prelogin-api.service.spec.ts @@ -6,6 +6,7 @@ import { of } from "rxjs"; import { Argon2KdfConfig, PBKDF2KdfConfig } from "@bitwarden/key-management"; import { ApiService } from "../../abstractions/api.service"; +import { KdfConfigResponse } from "../../key-management/models/response/kdf-config.response"; import { Environment, EnvironmentService } from "../../platform/abstractions/environment.service"; import { PasswordPreloginApiService } from "./password-prelogin-api.service"; @@ -18,6 +19,13 @@ describe("PasswordPreloginApiService", () => { let sut: PasswordPreloginApiService; const identityUrl = "https://identity.example.com"; + const salt = "user@example.com"; + + // KdfConfigResponse validates on construction, so every payload needs a well-formed KdfSettings. + const pbkdf2Payload = { + KdfSettings: { KdfType: 0, Iterations: PBKDF2KdfConfig.ITERATIONS.defaultValue }, + Salt: salt, + }; beforeEach(() => { apiService = mock(); @@ -37,7 +45,7 @@ describe("PasswordPreloginApiService", () => { describe("getPreloginData", () => { it("calls apiService.send with correct parameters", async () => { const request = new PasswordPreloginRequest("user@example.com"); - apiService.send.mockResolvedValue({}); + apiService.send.mockResolvedValue(pbkdf2Payload); await sut.getPreloginData(request); @@ -53,31 +61,33 @@ describe("PasswordPreloginApiService", () => { it("returns a PreloginResponse", async () => { const request = new PasswordPreloginRequest("user@example.com"); - apiService.send.mockResolvedValue({ - Kdf: 0, - KdfIterations: PBKDF2KdfConfig.ITERATIONS.defaultValue, - }); + apiService.send.mockResolvedValue(pbkdf2Payload); const result = await sut.getPreloginData(request); expect(result).toBeInstanceOf(PasswordPreloginResponse); }); - it("maps kdf fields from the api response", async () => { + it("maps kdf settings and salt from the api response", async () => { const request = new PasswordPreloginRequest("user@example.com"); apiService.send.mockResolvedValue({ - Kdf: 1, - KdfIterations: Argon2KdfConfig.ITERATIONS.defaultValue, - KdfMemory: Argon2KdfConfig.MEMORY.defaultValue, - KdfParallelism: Argon2KdfConfig.PARALLELISM.defaultValue, + KdfSettings: { + KdfType: 1, + Iterations: Argon2KdfConfig.ITERATIONS.defaultValue, + Memory: Argon2KdfConfig.MEMORY.defaultValue, + Parallelism: Argon2KdfConfig.PARALLELISM.defaultValue, + }, + Salt: salt, }); const result = await sut.getPreloginData(request); - expect(result.kdf).toBe(1); - expect(result.kdfIterations).toBe(Argon2KdfConfig.ITERATIONS.defaultValue); - expect(result.kdfMemory).toBe(Argon2KdfConfig.MEMORY.defaultValue); - expect(result.kdfParallelism).toBe(Argon2KdfConfig.PARALLELISM.defaultValue); + expect(result.kdfSettings).toBeInstanceOf(KdfConfigResponse); + expect(result.kdfSettings.kdfType).toBe(1); + expect(result.kdfSettings.iterations).toBe(Argon2KdfConfig.ITERATIONS.defaultValue); + expect(result.kdfSettings.memory).toBe(Argon2KdfConfig.MEMORY.defaultValue); + expect(result.kdfSettings.parallelism).toBe(Argon2KdfConfig.PARALLELISM.defaultValue); + expect(result.salt).toBe(salt); }); it("uses the identity url from the environment", async () => { @@ -89,7 +99,7 @@ describe("PasswordPreloginApiService", () => { sut = new PasswordPreloginApiService(apiService, environmentService); const request = new PasswordPreloginRequest("user@example.com"); - apiService.send.mockResolvedValue({}); + apiService.send.mockResolvedValue(pbkdf2Payload); await sut.getPreloginData(request); diff --git a/libs/common/src/auth/password-prelogin/password-prelogin.model.spec.ts b/libs/common/src/auth/password-prelogin/password-prelogin.model.spec.ts index e0524359619f..c712c7668a20 100644 --- a/libs/common/src/auth/password-prelogin/password-prelogin.model.spec.ts +++ b/libs/common/src/auth/password-prelogin/password-prelogin.model.spec.ts @@ -5,23 +5,35 @@ import { Argon2KdfConfig, PBKDF2KdfConfig } from "@bitwarden/key-management"; import { PasswordPreloginData } from "./password-prelogin.model"; import { PasswordPreloginResponse } from "./password-prelogin.response"; +const salt = "user@example.com"; + describe("PasswordPreloginData", () => { describe("fromResponse", () => { it.each([ { description: "PBKDF2", - response: { Kdf: 0, KdfIterations: PBKDF2KdfConfig.ITERATIONS.defaultValue }, + response: { + KdfSettings: { + KdfType: 0, + Iterations: PBKDF2KdfConfig.ITERATIONS.defaultValue, + }, + Salt: salt, + }, expected: new PasswordPreloginData( new PBKDF2KdfConfig(PBKDF2KdfConfig.ITERATIONS.defaultValue), + salt, ), }, { description: "Argon2", response: { - Kdf: 1, - KdfIterations: Argon2KdfConfig.ITERATIONS.defaultValue, - KdfMemory: Argon2KdfConfig.MEMORY.defaultValue, - KdfParallelism: Argon2KdfConfig.PARALLELISM.defaultValue, + KdfSettings: { + KdfType: 1, + Iterations: Argon2KdfConfig.ITERATIONS.defaultValue, + Memory: Argon2KdfConfig.MEMORY.defaultValue, + Parallelism: Argon2KdfConfig.PARALLELISM.defaultValue, + }, + Salt: salt, }, expected: new PasswordPreloginData( new Argon2KdfConfig( @@ -29,6 +41,7 @@ describe("PasswordPreloginData", () => { Argon2KdfConfig.MEMORY.defaultValue, Argon2KdfConfig.PARALLELISM.defaultValue, ), + salt, ), }, ])("maps a $description response to a PasswordPreloginData", ({ response, expected }) => { @@ -37,10 +50,60 @@ describe("PasswordPreloginData", () => { expect(result).toEqual(expected); }); + it("carries the server-supplied salt through to the model", () => { + const serverSalt = " Normalized.Salt@Example.com "; + + const result = PasswordPreloginData.fromResponse( + new PasswordPreloginResponse({ + KdfSettings: { KdfType: 0, Iterations: PBKDF2KdfConfig.ITERATIONS.defaultValue }, + Salt: serverSalt, + }), + ); + + // The salt is passed through verbatim — normalization is the server's/SDK's responsibility, + // and re-normalizing here would defeat the point of asking the server for it. + expect(result.salt).toBe(serverSalt); + }); + + it("maps a camelCase response, matching the casing the server actually serializes", () => { + const result = PasswordPreloginData.fromResponse( + new PasswordPreloginResponse({ + kdfSettings: { kdfType: 0, iterations: PBKDF2KdfConfig.ITERATIONS.defaultValue }, + salt, + }), + ); + + expect(result).toEqual( + new PasswordPreloginData( + new PBKDF2KdfConfig(PBKDF2KdfConfig.ITERATIONS.defaultValue), + salt, + ), + ); + }); + + it("returns an undefined salt when the server omits it", () => { + // PM-28143: Salt is nullable server-side while the transition is in flight. This documents + // today's behavior — the undefined flows through untouched. PasswordLoginStrategy only reads + // it when PM27060_PasswordPreloginFromSdk is on. + const result = PasswordPreloginData.fromResponse( + new PasswordPreloginResponse({ + KdfSettings: { KdfType: 0, Iterations: PBKDF2KdfConfig.ITERATIONS.defaultValue }, + }), + ); + + expect(result.salt).toBeUndefined(); + expect(result.kdfConfig).toEqual( + new PBKDF2KdfConfig(PBKDF2KdfConfig.ITERATIONS.defaultValue), + ); + }); + it.each([ { description: "PBKDF2 iterations below minimum", - response: { Kdf: 0, KdfIterations: PBKDF2KdfConfig.PRELOGIN_ITERATIONS_MIN - 1 }, + response: { + KdfSettings: { KdfType: 0, Iterations: PBKDF2KdfConfig.PRELOGIN_ITERATIONS_MIN - 1 }, + Salt: salt, + }, expectedError: new RegExp( `PBKDF2 iterations must be at least ${PBKDF2KdfConfig.PRELOGIN_ITERATIONS_MIN}`, ), @@ -48,10 +111,13 @@ describe("PasswordPreloginData", () => { { description: "Argon2 iterations below minimum", response: { - Kdf: 1, - KdfIterations: Argon2KdfConfig.PRELOGIN_ITERATIONS_MIN - 1, - KdfMemory: Argon2KdfConfig.MEMORY.defaultValue, - KdfParallelism: Argon2KdfConfig.PARALLELISM.defaultValue, + KdfSettings: { + KdfType: 1, + Iterations: Argon2KdfConfig.PRELOGIN_ITERATIONS_MIN - 1, + Memory: Argon2KdfConfig.MEMORY.defaultValue, + Parallelism: Argon2KdfConfig.PARALLELISM.defaultValue, + }, + Salt: salt, }, expectedError: new RegExp( `Argon2 iterations must be at least ${Argon2KdfConfig.PRELOGIN_ITERATIONS_MIN}`, @@ -60,10 +126,13 @@ describe("PasswordPreloginData", () => { { description: "Argon2 memory below minimum", response: { - Kdf: 1, - KdfIterations: Argon2KdfConfig.ITERATIONS.defaultValue, - KdfMemory: Argon2KdfConfig.PRELOGIN_MEMORY_MIN - 1, - KdfParallelism: Argon2KdfConfig.PARALLELISM.defaultValue, + KdfSettings: { + KdfType: 1, + Iterations: Argon2KdfConfig.ITERATIONS.defaultValue, + Memory: Argon2KdfConfig.PRELOGIN_MEMORY_MIN - 1, + Parallelism: Argon2KdfConfig.PARALLELISM.defaultValue, + }, + Salt: salt, }, expectedError: new RegExp( `Argon2 memory must be at least ${Argon2KdfConfig.PRELOGIN_MEMORY_MIN} MiB`, @@ -72,10 +141,13 @@ describe("PasswordPreloginData", () => { { description: "Argon2 parallelism below minimum", response: { - Kdf: 1, - KdfIterations: Argon2KdfConfig.ITERATIONS.defaultValue, - KdfMemory: Argon2KdfConfig.MEMORY.defaultValue, - KdfParallelism: Argon2KdfConfig.PRELOGIN_PARALLELISM_MIN - 1, + KdfSettings: { + KdfType: 1, + Iterations: Argon2KdfConfig.ITERATIONS.defaultValue, + Memory: Argon2KdfConfig.MEMORY.defaultValue, + Parallelism: Argon2KdfConfig.PRELOGIN_PARALLELISM_MIN - 1, + }, + Salt: salt, }, expectedError: new RegExp( `Argon2 parallelism must be at least ${Argon2KdfConfig.PRELOGIN_PARALLELISM_MIN}`, @@ -86,5 +158,13 @@ describe("PasswordPreloginData", () => { PasswordPreloginData.fromResponse(new PasswordPreloginResponse(response)), ).toThrow(expectedError); }); + + it("throws when the response omits KdfSettings entirely", () => { + // KdfConfigResponse validates the payload on construction, so a server that hasn't shipped + // the PM-28143 change fails loudly here rather than deriving a key from an undefined KDF. + expect(() => new PasswordPreloginResponse({ Salt: salt })).toThrow( + "KDF config response does not contain a valid KDF type", + ); + }); }); }); diff --git a/libs/common/src/auth/password-prelogin/password-prelogin.model.ts b/libs/common/src/auth/password-prelogin/password-prelogin.model.ts index eb733a704e52..d083ad87ffc9 100644 --- a/libs/common/src/auth/password-prelogin/password-prelogin.model.ts +++ b/libs/common/src/auth/password-prelogin/password-prelogin.model.ts @@ -1,16 +1,23 @@ // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports -import { Argon2KdfConfig, KdfConfig, KdfType, PBKDF2KdfConfig } from "@bitwarden/key-management"; +import { KdfConfig } from "@bitwarden/key-management"; import { PasswordPreloginResponse } from "./password-prelogin.response"; /** * Domain model representing the server's prelogin response for password-based authentication. - * Contains the KDF configuration needed to derive the master key from the user's master password. + * Contains the KDF configuration and salt needed to derive the master key from the user's master + * password. */ export class PasswordPreloginData { constructor( readonly kdfConfig: KdfConfig, + /** + * The salt the server dictates for master key derivation. Only consumed when + * {@link FeatureFlag.PM27060_PasswordPreloginFromSdk} is on; callers otherwise derive from the + * user-entered email. Nullable while PM-28143 is in flight — the server does not populate it on + * every deployment. + */ readonly salt: string, ) {} @@ -19,14 +26,7 @@ export class PasswordPreloginData { * @param response The raw API response from the prelogin endpoint. */ static fromResponse(response: PasswordPreloginResponse): PasswordPreloginData { - const kdfConfig = - response.kdfSettings.kdfType === KdfType.PBKDF2_SHA256 - ? new PBKDF2KdfConfig(response.kdfSettings.iterations) - : new Argon2KdfConfig( - response.kdfSettings.iterations, - response.kdfSettings.memory!, - response.kdfSettings.parallelism!, - ); + const kdfConfig = response.kdfSettings.toKdfConfig(); kdfConfig.validateKdfConfigForPrelogin(); return new PasswordPreloginData(kdfConfig, response.salt); } diff --git a/libs/common/src/auth/password-prelogin/password-prelogin.response.ts b/libs/common/src/auth/password-prelogin/password-prelogin.response.ts index beab39e6931d..8aa1af3518c9 100644 --- a/libs/common/src/auth/password-prelogin/password-prelogin.response.ts +++ b/libs/common/src/auth/password-prelogin/password-prelogin.response.ts @@ -9,7 +9,7 @@ export class PasswordPreloginResponse extends BaseResponse { constructor(response: any) { super(response); - this.kdfSettings = this.getResponseProperty("KdfSettings"); + this.kdfSettings = new KdfConfigResponse(this.getResponseProperty("KdfSettings")); this.salt = this.getResponseProperty("Salt"); } }