mirror of
https://github.com/bitwarden/browser
synced 2025-12-17 00:33:44 +00:00
Ps/fix biometric prompt error on close (#8353)
* Fix error on close due to context differences in background Desktop background does not have active user information. Also, we want to delete _all_ prompt cancelled data, not just that for the active user. Storing this on global and manipulating observables to active achieves this without needing any user information in the background. * Remove potentially orphaned data * Throw nice error if prompt cancelled used without active user * Register migration * split prompt cancelled reset to user-specific and global
This commit is contained in:
@@ -1,8 +1,8 @@
|
||||
import { firstValueFrom } from "rxjs";
|
||||
|
||||
import { makeEncString } from "../../../spec";
|
||||
import { mockAccountServiceWith } from "../../../spec/fake-account-service";
|
||||
import { FakeSingleUserState } from "../../../spec/fake-state";
|
||||
import { makeEncString, trackEmissions } from "../../../spec";
|
||||
import { FakeAccountService, mockAccountServiceWith } from "../../../spec/fake-account-service";
|
||||
import { FakeGlobalState, FakeSingleUserState } from "../../../spec/fake-state";
|
||||
import { FakeStateProvider } from "../../../spec/fake-state-provider";
|
||||
import { UserId } from "../../types/guid";
|
||||
import { EncryptedString } from "../models/domain/enc-string";
|
||||
@@ -23,10 +23,11 @@ describe("BiometricStateService", () => {
|
||||
const userId = "userId" as UserId;
|
||||
const encClientKeyHalf = makeEncString();
|
||||
const encryptedClientKeyHalf = encClientKeyHalf.encryptedString;
|
||||
const accountService = mockAccountServiceWith(userId);
|
||||
let accountService: FakeAccountService;
|
||||
let stateProvider: FakeStateProvider;
|
||||
|
||||
beforeEach(() => {
|
||||
accountService = mockAccountServiceWith(userId);
|
||||
stateProvider = new FakeStateProvider(accountService);
|
||||
|
||||
sut = new DefaultBiometricStateService(stateProvider);
|
||||
@@ -145,19 +146,89 @@ describe("BiometricStateService", () => {
|
||||
});
|
||||
|
||||
describe("setPromptCancelled", () => {
|
||||
let existingState: Record<UserId, boolean>;
|
||||
|
||||
beforeEach(() => {
|
||||
existingState = { ["otherUser" as UserId]: false };
|
||||
stateProvider.global.getFake(PROMPT_CANCELLED).stateSubject.next(existingState);
|
||||
});
|
||||
|
||||
test("observable is updated", async () => {
|
||||
await sut.setPromptCancelled();
|
||||
await sut.setUserPromptCancelled();
|
||||
|
||||
expect(await firstValueFrom(sut.promptCancelled$)).toBe(true);
|
||||
});
|
||||
|
||||
it("updates state", async () => {
|
||||
await sut.setPromptCancelled();
|
||||
await sut.setUserPromptCancelled();
|
||||
|
||||
const nextMock = stateProvider.activeUser.getFake(PROMPT_CANCELLED).nextMock;
|
||||
expect(nextMock).toHaveBeenCalledWith([userId, true]);
|
||||
const nextMock = stateProvider.global.getFake(PROMPT_CANCELLED).nextMock;
|
||||
expect(nextMock).toHaveBeenCalledWith({ ...existingState, [userId]: true });
|
||||
expect(nextMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("throws when called with no active user", async () => {
|
||||
await accountService.switchAccount(null);
|
||||
await expect(sut.setUserPromptCancelled()).rejects.toThrow(
|
||||
"Cannot update biometric prompt cancelled state without an active user",
|
||||
);
|
||||
const nextMock = stateProvider.global.getFake(PROMPT_CANCELLED).nextMock;
|
||||
expect(nextMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("resetAllPromptCancelled", () => {
|
||||
it("deletes all prompt cancelled state", async () => {
|
||||
await sut.resetAllPromptCancelled();
|
||||
|
||||
const nextMock = stateProvider.global.getFake(PROMPT_CANCELLED).nextMock;
|
||||
expect(nextMock).toHaveBeenCalledWith(null);
|
||||
expect(nextMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("updates observable to false", async () => {
|
||||
const emissions = trackEmissions(sut.promptCancelled$);
|
||||
|
||||
await sut.setUserPromptCancelled();
|
||||
|
||||
await sut.resetAllPromptCancelled();
|
||||
|
||||
expect(emissions).toEqual([false, true, false]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("resetUserPromptCancelled", () => {
|
||||
let existingState: Record<UserId, boolean>;
|
||||
let state: FakeGlobalState<Record<UserId, boolean>>;
|
||||
|
||||
beforeEach(async () => {
|
||||
await accountService.switchAccount(userId);
|
||||
existingState = { [userId]: true, ["otherUser" as UserId]: false };
|
||||
state = stateProvider.global.getFake(PROMPT_CANCELLED);
|
||||
state.stateSubject.next(existingState);
|
||||
});
|
||||
|
||||
it("deletes specified user prompt cancelled state", async () => {
|
||||
await sut.resetUserPromptCancelled("otherUser" as UserId);
|
||||
|
||||
expect(state.nextMock).toHaveBeenCalledWith({ [userId]: true });
|
||||
expect(state.nextMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("deletes active user when called with no user", async () => {
|
||||
await sut.resetUserPromptCancelled();
|
||||
|
||||
expect(state.nextMock).toHaveBeenCalledWith({ ["otherUser" as UserId]: false });
|
||||
expect(state.nextMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("updates observable to false", async () => {
|
||||
const emissions = trackEmissions(sut.promptCancelled$);
|
||||
|
||||
await sut.resetUserPromptCancelled();
|
||||
|
||||
expect(emissions).toEqual([true, false]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("setPromptAutomatically", () => {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { Observable, firstValueFrom, map } from "rxjs";
|
||||
import { Observable, firstValueFrom, map, combineLatest } from "rxjs";
|
||||
|
||||
import { UserId } from "../../types/guid";
|
||||
import { EncryptedString, EncString } from "../models/domain/enc-string";
|
||||
@@ -81,13 +81,18 @@ export abstract class BiometricStateService {
|
||||
*/
|
||||
abstract setDismissedRequirePasswordOnStartCallout(): Promise<void>;
|
||||
/**
|
||||
* Updates the active user's state to reflect that they've cancelled the biometric prompt this lock.
|
||||
* Updates the active user's state to reflect that they've cancelled the biometric prompt.
|
||||
*/
|
||||
abstract setPromptCancelled(): Promise<void>;
|
||||
abstract setUserPromptCancelled(): Promise<void>;
|
||||
/**
|
||||
* Resets the active user's state to reflect that they haven't cancelled the biometric prompt this lock.
|
||||
* Resets the given user's state to reflect that they haven't cancelled the biometric prompt.
|
||||
* @param userId the user to reset the prompt cancelled state for. If not provided, the currently active user will be used.
|
||||
*/
|
||||
abstract resetPromptCancelled(): Promise<void>;
|
||||
abstract resetUserPromptCancelled(userId?: UserId): Promise<void>;
|
||||
/**
|
||||
* Resets all user's state to reflect that they haven't cancelled the biometric prompt.
|
||||
*/
|
||||
abstract resetAllPromptCancelled(): Promise<void>;
|
||||
/**
|
||||
* Updates the currently active user's setting for auto prompting for biometrics on application start and lock
|
||||
* @param prompt Whether or not to prompt for biometrics on application start.
|
||||
@@ -107,7 +112,7 @@ export class DefaultBiometricStateService implements BiometricStateService {
|
||||
private requirePasswordOnStartState: ActiveUserState<boolean>;
|
||||
private encryptedClientKeyHalfState: ActiveUserState<EncryptedString | undefined>;
|
||||
private dismissedRequirePasswordOnStartCalloutState: ActiveUserState<boolean>;
|
||||
private promptCancelledState: ActiveUserState<boolean>;
|
||||
private promptCancelledState: GlobalState<Record<UserId, boolean>>;
|
||||
private promptAutomaticallyState: ActiveUserState<boolean>;
|
||||
private fingerprintValidatedState: GlobalState<boolean>;
|
||||
biometricUnlockEnabled$: Observable<boolean>;
|
||||
@@ -138,8 +143,15 @@ export class DefaultBiometricStateService implements BiometricStateService {
|
||||
this.dismissedRequirePasswordOnStartCallout$ =
|
||||
this.dismissedRequirePasswordOnStartCalloutState.state$.pipe(map(Boolean));
|
||||
|
||||
this.promptCancelledState = this.stateProvider.getActive(PROMPT_CANCELLED);
|
||||
this.promptCancelled$ = this.promptCancelledState.state$.pipe(map(Boolean));
|
||||
this.promptCancelledState = this.stateProvider.getGlobal(PROMPT_CANCELLED);
|
||||
this.promptCancelled$ = combineLatest([
|
||||
this.stateProvider.activeUserId$,
|
||||
this.promptCancelledState.state$,
|
||||
]).pipe(
|
||||
map(([userId, record]) => {
|
||||
return record?.[userId] ?? false;
|
||||
}),
|
||||
);
|
||||
this.promptAutomaticallyState = this.stateProvider.getActive(PROMPT_AUTOMATICALLY);
|
||||
this.promptAutomatically$ = this.promptAutomaticallyState.state$.pipe(map(Boolean));
|
||||
|
||||
@@ -202,7 +214,7 @@ export class DefaultBiometricStateService implements BiometricStateService {
|
||||
|
||||
async logout(userId: UserId): Promise<void> {
|
||||
await this.stateProvider.getUser(userId, ENCRYPTED_CLIENT_KEY_HALF).update(() => null);
|
||||
await this.stateProvider.getUser(userId, PROMPT_CANCELLED).update(() => null);
|
||||
await this.resetUserPromptCancelled(userId);
|
||||
// Persist auto prompt setting through logout
|
||||
// Persist dismissed require password on start callout through logout
|
||||
}
|
||||
@@ -211,11 +223,41 @@ export class DefaultBiometricStateService implements BiometricStateService {
|
||||
await this.dismissedRequirePasswordOnStartCalloutState.update(() => true);
|
||||
}
|
||||
|
||||
async setPromptCancelled(): Promise<void> {
|
||||
await this.promptCancelledState.update(() => true);
|
||||
async resetUserPromptCancelled(userId: UserId): Promise<void> {
|
||||
await this.stateProvider.getGlobal(PROMPT_CANCELLED).update(
|
||||
(data, activeUserId) => {
|
||||
delete data[userId ?? activeUserId];
|
||||
return data;
|
||||
},
|
||||
{
|
||||
combineLatestWith: this.stateProvider.activeUserId$,
|
||||
shouldUpdate: (data, activeUserId) => data?.[userId ?? activeUserId] != null,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
async resetPromptCancelled(): Promise<void> {
|
||||
async setUserPromptCancelled(): Promise<void> {
|
||||
await this.promptCancelledState.update(
|
||||
(record, userId) => {
|
||||
record ??= {};
|
||||
record[userId] = true;
|
||||
return record;
|
||||
},
|
||||
{
|
||||
combineLatestWith: this.stateProvider.activeUserId$,
|
||||
shouldUpdate: (_, userId) => {
|
||||
if (userId == null) {
|
||||
throw new Error(
|
||||
"Cannot update biometric prompt cancelled state without an active user",
|
||||
);
|
||||
}
|
||||
return true;
|
||||
},
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
async resetAllPromptCancelled(): Promise<void> {
|
||||
await this.promptCancelledState.update(() => null);
|
||||
}
|
||||
|
||||
|
||||
@@ -14,7 +14,7 @@ import {
|
||||
describe.each([
|
||||
[ENCRYPTED_CLIENT_KEY_HALF, "encryptedClientKeyHalf"],
|
||||
[DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT, true],
|
||||
[PROMPT_CANCELLED, true],
|
||||
[PROMPT_CANCELLED, { userId1: true, userId2: false }],
|
||||
[PROMPT_AUTOMATICALLY, true],
|
||||
[REQUIRE_PASSWORD_ON_START, true],
|
||||
[BIOMETRIC_UNLOCK_ENABLED, true],
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { UserId } from "../../types/guid";
|
||||
import { EncryptedString } from "../models/domain/enc-string";
|
||||
import { KeyDefinition, BIOMETRIC_SETTINGS_DISK } from "../state";
|
||||
|
||||
@@ -56,7 +57,7 @@ export const DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT = new KeyDefinition<boo
|
||||
* 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.
|
||||
*/
|
||||
export const PROMPT_CANCELLED = new KeyDefinition<boolean>(
|
||||
export const PROMPT_CANCELLED = KeyDefinition.record<boolean, UserId>(
|
||||
BIOMETRIC_SETTINGS_DISK,
|
||||
"promptCancelled",
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user