diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 05fded6bcaf..71b95ec6057 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -19,6 +19,7 @@ export enum FeatureFlag { PM23801_PrefetchPasswordPrelogin = "pm-23801-prefetch-password-prelogin", PM27086_UpdateAuthenticationApisForInputPassword = "pm-27086-update-authentication-apis-for-input-password", SafariAccountSwitching = "pm-5594-safari-account-switching", + PM31088_MasterPasswordServiceEmitSalt = "pm-31088-master-password-service-emit-salt", /* Autofill */ UseUndeterminedCipherScenarioTriggeringLogic = "undetermined-cipher-scenario-logic", @@ -143,6 +144,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.PM23801_PrefetchPasswordPrelogin]: FALSE, [FeatureFlag.PM27086_UpdateAuthenticationApisForInputPassword]: FALSE, [FeatureFlag.SafariAccountSwitching]: FALSE, + [FeatureFlag.PM31088_MasterPasswordServiceEmitSalt]: FALSE, /* Billing */ [FeatureFlag.TrialPaymentOptional]: FALSE, diff --git a/libs/common/src/key-management/master-password/services/master-password.service.spec.ts b/libs/common/src/key-management/master-password/services/master-password.service.spec.ts index f72ae0e7c5e..4a96dedf024 100644 --- a/libs/common/src/key-management/master-password/services/master-password.service.spec.ts +++ b/libs/common/src/key-management/master-password/services/master-password.service.spec.ts @@ -17,8 +17,11 @@ import { mockAccountServiceWith, } from "../../../../spec"; import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason"; +import { FeatureFlag } from "../../../enums/feature-flag.enum"; +import { ServerConfig } from "../../../platform/abstractions/config/server-config"; import { LogService } from "../../../platform/abstractions/log.service"; import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; +import { USER_SERVER_CONFIG } from "../../../platform/services/config/default-config.service"; import { UserId } from "../../../types/guid"; import { MasterKey, UserKey } from "../../../types/key"; import { KeyGenerationService } from "../../crypto"; @@ -92,14 +95,52 @@ describe("MasterPasswordService", () => { sut.saltForUser$(null as unknown as UserId); }).toThrow("userId is null or undefined."); }); + // Removable with unwinding of PM31088_MasterPasswordServiceEmitSalt it("throws when userid present but not in account service", async () => { await expect( firstValueFrom(sut.saltForUser$("00000000-0000-0000-0000-000000000001" as UserId)), ).rejects.toThrow("Cannot read properties of undefined (reading 'email')"); }); - it("returns salt", async () => { - const salt = await firstValueFrom(sut.saltForUser$(userId)); - expect(salt).toBeDefined(); + // Removable with unwinding of PM31088_MasterPasswordServiceEmitSalt + it("returns email-derived salt for legacy path", async () => { + const result = await firstValueFrom(sut.saltForUser$(userId)); + // mockAccountServiceWith defaults email to "email" + expect(result).toBe("email" as MasterPasswordSalt); + }); + + describe("saltForUser$ master password unlock data migration path", () => { + // Flagged with PM31088_MasterPasswordServiceEmitSalt PM-31088 + beforeEach(() => { + stateProvider.singleUser.getFake(userId, USER_SERVER_CONFIG).nextState({ + featureStates: { + [FeatureFlag.PM31088_MasterPasswordServiceEmitSalt]: true, + }, + } as unknown as ServerConfig); + }); + + // Unwinding should promote these tests as part of saltForUser suite. + it("returns salt from master password unlock data", async () => { + const expectedSalt = "custom-salt" as MasterPasswordSalt; + const unlockData = new MasterPasswordUnlockData( + expectedSalt, + new PBKDF2KdfConfig(600_000), + makeEncString().toSdk() as MasterKeyWrappedUserKey, + ); + stateProvider.singleUser + .getFake(userId, MASTER_PASSWORD_UNLOCK_KEY) + .nextState(unlockData.toJSON()); + + const result = await firstValueFrom(sut.saltForUser$(userId)); + expect(result).toBe(expectedSalt); + }); + + it("throws when master password unlock data is null", async () => { + stateProvider.singleUser.getFake(userId, MASTER_PASSWORD_UNLOCK_KEY).nextState(null); + + await expect(firstValueFrom(sut.saltForUser$(userId))).rejects.toThrow( + "Master password unlock data not found for user.", + ); + }); }); }); diff --git a/libs/common/src/key-management/master-password/services/master-password.service.ts b/libs/common/src/key-management/master-password/services/master-password.service.ts index 28d4f58d7dc..f1a074ff14c 100644 --- a/libs/common/src/key-management/master-password/services/master-password.service.ts +++ b/libs/common/src/key-management/master-password/services/master-password.service.ts @@ -1,6 +1,6 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { firstValueFrom, map, Observable } from "rxjs"; +import { firstValueFrom, iif, map, Observable, switchMap } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { assertNonNullish } from "@bitwarden/common/auth/utils"; @@ -12,8 +12,10 @@ import { KdfConfig } from "@bitwarden/key-management"; import { PureCrypto } from "@bitwarden/sdk-internal"; import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason"; +import { FeatureFlag, getFeatureFlagValue } from "../../../enums/feature-flag.enum"; import { LogService } from "../../../platform/abstractions/log.service"; import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; +import { USER_SERVER_CONFIG } from "../../../platform/services/config/default-config.service"; import { MASTER_PASSWORD_DISK, MASTER_PASSWORD_MEMORY, @@ -102,9 +104,29 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr saltForUser$(userId: UserId): Observable { assertNonNullish(userId, "userId"); - return this.accountService.accounts$.pipe( - map((accounts) => accounts[userId].email), - map((email) => this.emailToSalt(email)), + + // Note: We can't use the config service as an abstraction here because it creates a circular dependency: ConfigService -> ConfigApiService -> ApiService -> VaultTimeoutSettingsService -> KeyService -> MP service. + return this.stateProvider.getUser(userId, USER_SERVER_CONFIG).state$.pipe( + map((serverConfig) => + getFeatureFlagValue(serverConfig, FeatureFlag.PM31088_MasterPasswordServiceEmitSalt), + ), + switchMap((enabled) => + iif( + () => enabled, + this.masterPasswordUnlockData$(userId).pipe( + map((unlockData) => { + if (unlockData == null) { + throw new Error("Master password unlock data not found for user."); + } + return unlockData.salt; + }), + ), + this.accountService.accounts$.pipe( + map((accounts) => accounts[userId].email), + map((email) => this.emailToSalt(email)), + ), + ), + ), ); }