mirror of
https://github.com/bitwarden/browser
synced 2026-02-19 02:44:01 +00:00
[PM-31088] saltForUser should emit salt from master password unlock data (#18976)
* feat(salt-for-user) [PM-31088]: Add feature flag for saltForUser. * feat(salt-for-user) [PM-31088]: Flag saltForUser logic to return unlockdata.salt or emailToSalt. * test(salt-for-user) [PM-31088]: Update tests to include coverage for new behavior.
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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.",
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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<MasterPasswordSalt> {
|
||||
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)),
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user