1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-10 13:23:34 +00:00

cleanup remove password on start for desktop (#15957)

This commit is contained in:
Maciej Zieniuk
2025-08-22 23:14:02 +02:00
committed by GitHub
parent 65230a9c3e
commit 5b402d9c38
9 changed files with 5 additions and 302 deletions

View File

@@ -97,31 +97,6 @@
</label>
</div>
</div>
<div
class="form-group"
*ngIf="
supportsBiometric &&
this.form.value.biometric &&
(userHasMasterPassword || (this.form.value.pin && userHasPinSet)) &&
false
"
>
<div class="checkbox form-group-child">
<label for="requirePasswordOnStart">
<input
id="requirePasswordOnStart"
type="checkbox"
formControlName="requirePasswordOnStart"
(change)="updateRequirePasswordOnStart()"
/>
@if (pinEnabled$ | async) {
{{ "requirePasswordOnStart" | i18n }}
} @else {
{{ "requirePasswordWithoutPinOnStart" | i18n }}
}
</label>
</div>
</div>
</ng-container>
</div>
</div>

View File

@@ -157,7 +157,6 @@ describe("SettingsComponent", () => {
);
vaultTimeoutSettingsService.isBiometricLockSet.mockResolvedValue(false);
biometricStateService.promptAutomatically$ = of(false);
biometricStateService.requirePasswordOnStart$ = of(false);
autofillSettingsServiceAbstraction.clearClipboardDelay$ = of(null);
desktopSettingsService.minimizeOnCopy$ = of(false);
desktopSettingsService.trayEnabled$ = of(false);
@@ -378,48 +377,14 @@ describe("SettingsComponent", () => {
});
describe("when updating to false", () => {
let updateRequirePasswordOnStartSpy: jest.SpyInstance;
beforeEach(() => {
updateRequirePasswordOnStartSpy = jest
.spyOn(component, "updateRequirePasswordOnStart")
.mockImplementation(() => Promise.resolve());
});
it("updates requires password on start when the user doesn't have a MP and has requirePasswordOnStart on", async () => {
it("sets the pin form control to false and clears vault timeout", async () => {
await component.ngOnInit();
component.form.controls.requirePasswordOnStart.setValue(true, { emitEvent: false });
component.userHasMasterPassword = false;
await component.updatePinHandler(false);
expect(component.form.controls.pin.value).toBe(false);
expect(component.form.controls.requirePasswordOnStart.value).toBe(false);
expect(updateRequirePasswordOnStartSpy).toHaveBeenCalled();
expect(vaultTimeoutSettingsService.clear).toHaveBeenCalled();
expect(messagingService.send).toHaveBeenCalledWith("redrawMenu");
});
test.each([
[true, true],
[false, true],
[false, false],
])(
`doesn't updates requires password on start when the user's requirePasswordOnStart is %s and userHasMasterPassword is %s`,
async (requirePasswordOnStart, userHasMasterPassword) => {
await component.ngOnInit();
component.form.controls.requirePasswordOnStart.setValue(requirePasswordOnStart, {
emitEvent: false,
});
component.userHasMasterPassword = userHasMasterPassword;
await component.updatePinHandler(false);
expect(component.form.controls.pin.value).toBe(false);
expect(component.form.controls.requirePasswordOnStart.value).toBe(requirePasswordOnStart);
expect(updateRequirePasswordOnStartSpy).not.toHaveBeenCalled();
expect(vaultTimeoutSettingsService.clear).toHaveBeenCalled();
expect(messagingService.send).toHaveBeenCalledWith("redrawMenu");
},
);
});
});
@@ -512,11 +477,8 @@ describe("SettingsComponent", () => {
await component.updateBiometricHandler(true);
expect(biometricStateService.setBiometricUnlockEnabled).toHaveBeenCalledWith(true);
expect(component.form.controls.requirePasswordOnStart.value).toBe(true);
expect(component.form.controls.autoPromptBiometrics.value).toBe(false);
expect(biometricStateService.setPromptAutomatically).toHaveBeenCalledWith(false);
expect(biometricStateService.setRequirePasswordOnStart).toHaveBeenCalledWith(true);
expect(biometricStateService.setDismissedRequirePasswordOnStartCallout).toHaveBeenCalled();
expect(keyService.refreshAdditionalKeys).toHaveBeenCalledWith(mockUserId);
expect(component.form.controls.biometric.value).toBe(true);
expect(messagingService.send).toHaveBeenCalledWith("redrawMenu");
@@ -533,11 +495,8 @@ describe("SettingsComponent", () => {
await component.updateBiometricHandler(true);
expect(biometricStateService.setBiometricUnlockEnabled).toHaveBeenCalledWith(true);
expect(component.form.controls.requirePasswordOnStart.value).toBe(true);
expect(component.form.controls.autoPromptBiometrics.value).toBe(false);
expect(biometricStateService.setPromptAutomatically).toHaveBeenCalledWith(false);
expect(biometricStateService.setRequirePasswordOnStart).toHaveBeenCalledWith(true);
expect(biometricStateService.setDismissedRequirePasswordOnStartCallout).toHaveBeenCalled();
expect(keyService.refreshAdditionalKeys).toHaveBeenCalledWith(mockUserId);
expect(component.form.controls.biometric.value).toBe(true);
expect(messagingService.send).toHaveBeenCalledWith("redrawMenu");

View File

@@ -135,7 +135,6 @@ export class SettingsComponent implements OnInit, OnDestroy {
pin: [null as boolean | null],
biometric: false,
autoPromptBiometrics: false,
requirePasswordOnStart: false,
// Account Preferences
clearClipboard: [null],
minimizeOnCopyToClipboard: false,
@@ -350,9 +349,6 @@ export class SettingsComponent implements OnInit, OnDestroy {
pin: this.userHasPinSet,
biometric: await this.vaultTimeoutSettingsService.isBiometricLockSet(),
autoPromptBiometrics: await firstValueFrom(this.biometricStateService.promptAutomatically$),
requirePasswordOnStart: await firstValueFrom(
this.biometricStateService.requirePasswordOnStart$,
),
clearClipboard: await firstValueFrom(this.autofillSettingsService.clearClipboardDelay$),
minimizeOnCopyToClipboard: await firstValueFrom(this.desktopSettingsService.minimizeOnCopy$),
enableFavicons: await firstValueFrom(this.domainSettingsService.showFavicons$),
@@ -557,16 +553,7 @@ export class SettingsComponent implements OnInit, OnDestroy {
this.userHasPinSet = await firstValueFrom(dialogRef.closed);
this.form.controls.pin.setValue(this.userHasPinSet, { emitEvent: false });
}
if (!value) {
// If user turned off PIN without having a MP and has biometric + require MP/PIN on restart enabled
if (this.form.value.requirePasswordOnStart && !this.userHasMasterPassword) {
// then must turn that off to prevent user from getting into bad state
this.form.controls.requirePasswordOnStart.setValue(false);
await this.updateRequirePasswordOnStart();
}
} else {
const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
await this.vaultTimeoutSettingsService.clear(userId);
}
@@ -617,18 +604,12 @@ export class SettingsComponent implements OnInit, OnDestroy {
await this.biometricStateService.setBiometricUnlockEnabled(true);
if (this.isWindows) {
// Recommended settings for Windows Hello
this.form.controls.requirePasswordOnStart.setValue(true);
this.form.controls.autoPromptBiometrics.setValue(false);
await this.biometricStateService.setPromptAutomatically(false);
await this.biometricStateService.setRequirePasswordOnStart(true);
await this.biometricStateService.setDismissedRequirePasswordOnStartCallout();
} else if (this.isLinux) {
// Similar to Windows
this.form.controls.requirePasswordOnStart.setValue(true);
this.form.controls.autoPromptBiometrics.setValue(false);
await this.biometricStateService.setPromptAutomatically(false);
await this.biometricStateService.setRequirePasswordOnStart(true);
await this.biometricStateService.setDismissedRequirePasswordOnStartCallout();
}
await this.keyService.refreshAdditionalKeys(activeUserId);
@@ -644,30 +625,12 @@ export class SettingsComponent implements OnInit, OnDestroy {
async updateAutoPromptBiometrics() {
if (this.form.value.autoPromptBiometrics) {
// require password on start must be disabled if auto prompt biometrics is enabled
this.form.controls.requirePasswordOnStart.setValue(false);
await this.updateRequirePasswordOnStart();
await this.biometricStateService.setPromptAutomatically(true);
} else {
await this.biometricStateService.setPromptAutomatically(false);
}
}
async updateRequirePasswordOnStart() {
if (this.form.value.requirePasswordOnStart) {
// auto prompt biometrics must be disabled if require password on start is enabled
this.form.controls.autoPromptBiometrics.setValue(false);
await this.updateAutoPromptBiometrics();
await this.biometricStateService.setRequirePasswordOnStart(true);
} else {
await this.biometricStateService.setRequirePasswordOnStart(false);
}
await this.biometricStateService.setDismissedRequirePasswordOnStartCallout();
const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
await this.keyService.refreshAdditionalKeys(userId);
}
async saveFavicons() {
await this.domainSettingsService.setShowFavicons(this.form.value.enableFavicons);
this.messagingService.send("refreshCiphers");

View File

@@ -198,11 +198,6 @@ export default class OsBiometricsServiceLinux implements OsBiometricService {
userId: UserId,
key: SymmetricCryptoKey,
): Promise<Uint8Array | null> {
const requireClientKeyHalf = await this.biometricStateService.getRequirePasswordOnStart(userId);
if (!requireClientKeyHalf) {
return null;
}
if (this.clientKeyHalves.has(userId)) {
return this.clientKeyHalves.get(userId) || null;
}
@@ -227,12 +222,10 @@ export default class OsBiometricsServiceLinux implements OsBiometricService {
}
async getBiometricsFirstUnlockStatusForUser(userId: UserId): Promise<BiometricsStatus> {
const requireClientKeyHalf = await this.biometricStateService.getRequirePasswordOnStart(userId);
const clientKeyHalfB64 = this.clientKeyHalves.get(userId);
const clientKeyHalfSatisfied = !requireClientKeyHalf || !!clientKeyHalfB64;
if (!clientKeyHalfSatisfied) {
if (this.clientKeyHalves.has(userId)) {
return BiometricsStatus.Available;
} else {
return BiometricsStatus.UnlockNeeded;
}
return BiometricsStatus.Available;
}
}

View File

@@ -1846,12 +1846,6 @@
"autoPromptTouchId": {
"message": "Ask for Touch ID on app start"
},
"requirePasswordOnStart": {
"message": "Require password or PIN on app start"
},
"requirePasswordWithoutPinOnStart": {
"message": "Require password on app start"
},
"lockWithMasterPassOnRestart1": {
"message": "Lock with master password on restart"
},

View File

@@ -6,7 +6,6 @@ import {
trackEmissions,
FakeStateProvider,
FakeGlobalState,
FakeSingleUserState,
FakeAccountService,
mockAccountServiceWith,
} from "@bitwarden/common/spec";
@@ -15,12 +14,10 @@ import { UserId } from "@bitwarden/common/types/guid";
import { BiometricStateService, DefaultBiometricStateService } from "./biometric-state.service";
import {
BIOMETRIC_UNLOCK_ENABLED,
DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT,
ENCRYPTED_CLIENT_KEY_HALF,
FINGERPRINT_VALIDATED,
PROMPT_AUTOMATICALLY,
PROMPT_CANCELLED,
REQUIRE_PASSWORD_ON_START,
} from "./biometric.state";
describe("BiometricStateService", () => {
@@ -42,22 +39,6 @@ describe("BiometricStateService", () => {
jest.resetAllMocks();
});
describe("requirePasswordOnStart$", () => {
it("emits when the require password on start state changes", async () => {
const state = stateProvider.activeUser.getFake(REQUIRE_PASSWORD_ON_START);
state.nextState(true);
expect(await firstValueFrom(sut.requirePasswordOnStart$)).toBe(true);
});
it("emits false when the require password on start state is undefined", async () => {
const state = stateProvider.activeUser.getFake(REQUIRE_PASSWORD_ON_START);
state.nextState(undefined as unknown as boolean);
expect(await firstValueFrom(sut.requirePasswordOnStart$)).toBe(false);
});
});
describe("encryptedClientKeyHalf$", () => {
it("emits when the encryptedClientKeyHalf state changes", async () => {
const state = stateProvider.activeUser.getFake(ENCRYPTED_CLIENT_KEY_HALF);
@@ -95,61 +76,6 @@ describe("BiometricStateService", () => {
});
});
describe("setRequirePasswordOnStart", () => {
it("updates the requirePasswordOnStart$", async () => {
await sut.setRequirePasswordOnStart(true);
expect(await firstValueFrom(sut.requirePasswordOnStart$)).toBe(true);
});
it("removes the encryptedClientKeyHalf when the set value is false", async () => {
await sut.setEncryptedClientKeyHalf(encClientKeyHalf, userId);
await sut.setRequirePasswordOnStart(false);
const keyHalfState = stateProvider.getUser(
userId,
ENCRYPTED_CLIENT_KEY_HALF,
) as FakeSingleUserState<EncryptedString>;
expect(await firstValueFrom(keyHalfState.state$)).toBe(null);
expect(keyHalfState.nextMock).toHaveBeenCalledWith(null);
});
it("does not remove the encryptedClientKeyHalf when the value is true", async () => {
await sut.setEncryptedClientKeyHalf(encClientKeyHalf);
await sut.setRequirePasswordOnStart(true);
expect(await firstValueFrom(sut.encryptedClientKeyHalf$)).toEqual(encClientKeyHalf);
});
});
describe("getRequirePasswordOnStart", () => {
it("returns the requirePasswordOnStart state value", async () => {
stateProvider.singleUser.mockFor(userId, REQUIRE_PASSWORD_ON_START, true);
expect(await sut.getRequirePasswordOnStart(userId)).toBe(true);
});
});
describe("require password on start callout", () => {
it("is false when not set", async () => {
expect(await firstValueFrom(sut.dismissedRequirePasswordOnStartCallout$)).toBe(false);
});
it("is true when set", async () => {
await sut.setDismissedRequirePasswordOnStartCallout();
expect(await firstValueFrom(sut.dismissedRequirePasswordOnStartCallout$)).toBe(true);
});
it("updates disk state when called", async () => {
await sut.setDismissedRequirePasswordOnStartCallout();
expect(
stateProvider.activeUser.getFake(DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT).nextMock,
).toHaveBeenCalledWith([userId, true]);
});
});
describe("setPromptCancelled", () => {
let existingState: Record<UserId, boolean>;

View File

@@ -10,8 +10,6 @@ import { UserId } from "@bitwarden/common/types/guid";
import {
BIOMETRIC_UNLOCK_ENABLED,
ENCRYPTED_CLIENT_KEY_HALF,
REQUIRE_PASSWORD_ON_START,
DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT,
PROMPT_AUTOMATICALLY,
PROMPT_CANCELLED,
FINGERPRINT_VALIDATED,
@@ -30,18 +28,6 @@ export abstract class BiometricStateService {
* Tracks the currently active user
*/
abstract encryptedClientKeyHalf$: Observable<EncString | null>;
/**
* whether or not a password is required on first unlock after opening the application
*
* tracks the currently active user
*/
abstract requirePasswordOnStart$: Observable<boolean>;
/**
* Indicates the user has been warned about the security implications of using biometrics and, depending on the OS,
*
* tracks the currently active user.
*/
abstract dismissedRequirePasswordOnStartCallout$: Observable<boolean>;
/**
* Whether the user has cancelled the biometric prompt.
*
@@ -59,14 +45,6 @@ export abstract class BiometricStateService {
*/
abstract fingerprintValidated$: Observable<boolean>;
/**
* Updates the require password on start state for the currently active user.
*
* If false, the encrypted client key half will be removed.
* @param value whether or not a password is required on first unlock after opening the application
*/
abstract setRequirePasswordOnStart(value: boolean): Promise<void>;
/**
* Updates the biometric unlock enabled state for the currently active user.
* @param enabled whether or not to store a biometric key to unlock the vault
@@ -83,15 +61,6 @@ export abstract class BiometricStateService {
abstract getEncryptedClientKeyHalf(userId: UserId): Promise<EncString | null>;
abstract getRequirePasswordOnStart(userId: UserId): Promise<boolean>;
abstract removeEncryptedClientKeyHalf(userId: UserId): Promise<void>;
/**
* Updates the active user's state to reflect that they've been warned about requiring password on start.
*/
abstract setDismissedRequirePasswordOnStartCallout(): Promise<void>;
/**
* Updates the active user's state to reflect that they've cancelled the biometric prompt.
*/
@@ -129,17 +98,13 @@ export abstract class BiometricStateService {
export class DefaultBiometricStateService implements BiometricStateService {
private biometricUnlockEnabledState: ActiveUserState<boolean>;
private requirePasswordOnStartState: ActiveUserState<boolean>;
private encryptedClientKeyHalfState: ActiveUserState<EncryptedString>;
private dismissedRequirePasswordOnStartCalloutState: ActiveUserState<boolean>;
private promptCancelledState: GlobalState<Record<UserId, boolean>>;
private promptAutomaticallyState: ActiveUserState<boolean>;
private fingerprintValidatedState: GlobalState<boolean>;
private lastProcessReloadState: GlobalState<Date>;
biometricUnlockEnabled$: Observable<boolean>;
encryptedClientKeyHalf$: Observable<EncString | null>;
requirePasswordOnStart$: Observable<boolean>;
dismissedRequirePasswordOnStartCallout$: Observable<boolean>;
promptCancelled$: Observable<boolean>;
promptAutomatically$: Observable<boolean>;
fingerprintValidated$: Observable<boolean>;
@@ -149,22 +114,11 @@ export class DefaultBiometricStateService implements BiometricStateService {
this.biometricUnlockEnabledState = this.stateProvider.getActive(BIOMETRIC_UNLOCK_ENABLED);
this.biometricUnlockEnabled$ = this.biometricUnlockEnabledState.state$.pipe(map(Boolean));
this.requirePasswordOnStartState = this.stateProvider.getActive(REQUIRE_PASSWORD_ON_START);
this.requirePasswordOnStart$ = this.requirePasswordOnStartState.state$.pipe(
map((value) => !!value),
);
this.encryptedClientKeyHalfState = this.stateProvider.getActive(ENCRYPTED_CLIENT_KEY_HALF);
this.encryptedClientKeyHalf$ = this.encryptedClientKeyHalfState.state$.pipe(
map(encryptedClientKeyHalfToEncString),
);
this.dismissedRequirePasswordOnStartCalloutState = this.stateProvider.getActive(
DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT,
);
this.dismissedRequirePasswordOnStartCallout$ =
this.dismissedRequirePasswordOnStartCalloutState.state$.pipe(map(Boolean));
this.promptCancelledState = this.stateProvider.getGlobal(PROMPT_CANCELLED);
this.promptCancelled$ = combineLatest([
this.stateProvider.activeUserId$,
@@ -194,22 +148,6 @@ export class DefaultBiometricStateService implements BiometricStateService {
);
}
async setRequirePasswordOnStart(value: boolean): Promise<void> {
let currentActiveId: UserId | undefined = undefined;
await this.requirePasswordOnStartState.update(
(_, [userId]) => {
currentActiveId = userId;
return value;
},
{
combineLatestWith: this.requirePasswordOnStartState.combinedState$,
},
);
if (!value && currentActiveId) {
await this.removeEncryptedClientKeyHalf(currentActiveId);
}
}
async setEncryptedClientKeyHalf(encryptedKeyHalf: EncString, userId?: UserId): Promise<void> {
const value = encryptedKeyHalf?.encryptedString ?? null;
if (userId) {
@@ -219,16 +157,6 @@ export class DefaultBiometricStateService implements BiometricStateService {
}
}
async removeEncryptedClientKeyHalf(userId: UserId): Promise<void> {
await this.stateProvider.getUser(userId, ENCRYPTED_CLIENT_KEY_HALF).update(() => null);
}
async getRequirePasswordOnStart(userId: UserId): Promise<boolean> {
return !!(await firstValueFrom(
this.stateProvider.getUser(userId, REQUIRE_PASSWORD_ON_START).state$,
));
}
async getEncryptedClientKeyHalf(userId: UserId): Promise<EncString | null> {
return await firstValueFrom(
this.stateProvider
@@ -244,10 +172,6 @@ export class DefaultBiometricStateService implements BiometricStateService {
// Persist dismissed require password on start callout through logout
}
async setDismissedRequirePasswordOnStartCallout(): Promise<void> {
await this.dismissedRequirePasswordOnStartCalloutState.update(() => true);
}
async resetUserPromptCancelled(userId: UserId): Promise<void> {
await this.stateProvider.getGlobal(PROMPT_CANCELLED).update(
(data, activeUserId) => {

View File

@@ -2,20 +2,16 @@ import { KeyDefinition, UserKeyDefinition } from "@bitwarden/common/platform/sta
import {
BIOMETRIC_UNLOCK_ENABLED,
DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT,
ENCRYPTED_CLIENT_KEY_HALF,
FINGERPRINT_VALIDATED,
PROMPT_AUTOMATICALLY,
PROMPT_CANCELLED,
REQUIRE_PASSWORD_ON_START,
} from "./biometric.state";
describe.each([
[ENCRYPTED_CLIENT_KEY_HALF, "encryptedClientKeyHalf"],
[DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT, true],
[PROMPT_CANCELLED, { userId1: true, userId2: false }],
[PROMPT_AUTOMATICALLY, true],
[REQUIRE_PASSWORD_ON_START, true],
[BIOMETRIC_UNLOCK_ENABLED, true],
[FINGERPRINT_VALIDATED, true],
])(

View File

@@ -18,20 +18,6 @@ export const BIOMETRIC_UNLOCK_ENABLED = new UserKeyDefinition<boolean>(
},
);
/**
* Boolean indicating the user has elected to require a password to use their biometric key upon starting the application.
*
* A true setting controls whether {@link ENCRYPTED_CLIENT_KEY_HALF} is set.
*/
export const REQUIRE_PASSWORD_ON_START = new UserKeyDefinition<boolean>(
BIOMETRIC_SETTINGS_DISK,
"requirePasswordOnStart",
{
deserializer: (value: any) => value,
clearOn: [],
},
);
/**
* If the user has elected to require a password on first unlock of an application instance, this key will store the
* encrypted client key half used to unlock the vault.
@@ -48,19 +34,6 @@ export const ENCRYPTED_CLIENT_KEY_HALF = new UserKeyDefinition<EncryptedString>(
},
);
/**
* Indicates the user has been warned about the security implications of using biometrics and, depending on the OS,
* recommended to require a password on first unlock of an application instance.
*/
export const DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT = new UserKeyDefinition<boolean>(
BIOMETRIC_SETTINGS_DISK,
"dismissedBiometricRequirePasswordOnStartCallout",
{
deserializer: (obj) => obj,
clearOn: [],
},
);
/**
* Stores whether the user has elected to cancel the biometric prompt. This is stored on disk due to process-reload
* wiping memory state. We don't want to prompt the user again if they've elected to cancel.