From 8ee19658327b2b26dd9e328741ef657421dc113b Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Fri, 15 Mar 2024 10:28:34 -0500 Subject: [PATCH] Revert "Use global state for biometric prompt cancel storage (#8328)" (#8351) This reverts commit 770f782a16a26f602db5659b6b71837b73270aee. --- libs/common/spec/fake-account-service.ts | 3 -- .../biometric-state.service.spec.ts | 47 ++----------------- .../biometrics/biometric-state.service.ts | 43 ++--------------- .../biometrics/biometric.state.spec.ts | 2 +- .../platform/biometrics/biometric.state.ts | 3 +- libs/common/src/state-migrations/migrate.ts | 6 +-- ...ete-orphaned-biometric-prompt-data.spec.ts | 28 ----------- ...8-delete-orphaned-biometric-prompt-data.ts | 23 --------- 8 files changed, 14 insertions(+), 141 deletions(-) delete mode 100644 libs/common/src/state-migrations/migrations/38-delete-orphaned-biometric-prompt-data.spec.ts delete mode 100644 libs/common/src/state-migrations/migrations/38-delete-orphaned-biometric-prompt-data.ts diff --git a/libs/common/spec/fake-account-service.ts b/libs/common/spec/fake-account-service.ts index 2f33d9cf02..1364127f65 100644 --- a/libs/common/spec/fake-account-service.ts +++ b/libs/common/spec/fake-account-service.ts @@ -70,9 +70,6 @@ export class FakeAccountService implements AccountService { } async switchAccount(userId: UserId): Promise { - const next = - userId == null ? null : { id: userId, ...this.accountsSubject["_buffer"]?.[0]?.[userId] }; - this.activeAccountSubject.next(next); await this.mock.switchAccount(userId); } } diff --git a/libs/common/src/platform/biometrics/biometric-state.service.spec.ts b/libs/common/src/platform/biometrics/biometric-state.service.spec.ts index ee8da8ddf3..716ad627c1 100644 --- a/libs/common/src/platform/biometrics/biometric-state.service.spec.ts +++ b/libs/common/src/platform/biometrics/biometric-state.service.spec.ts @@ -1,7 +1,7 @@ import { firstValueFrom } from "rxjs"; -import { makeEncString, trackEmissions } from "../../../spec"; -import { FakeAccountService, mockAccountServiceWith } from "../../../spec/fake-account-service"; +import { makeEncString } from "../../../spec"; +import { mockAccountServiceWith } from "../../../spec/fake-account-service"; import { FakeSingleUserState } from "../../../spec/fake-state"; import { FakeStateProvider } from "../../../spec/fake-state-provider"; import { UserId } from "../../types/guid"; @@ -23,11 +23,10 @@ describe("BiometricStateService", () => { const userId = "userId" as UserId; const encClientKeyHalf = makeEncString(); const encryptedClientKeyHalf = encClientKeyHalf.encryptedString; - let accountService: FakeAccountService; + const accountService = mockAccountServiceWith(userId); let stateProvider: FakeStateProvider; beforeEach(() => { - accountService = mockAccountServiceWith(userId); stateProvider = new FakeStateProvider(accountService); sut = new DefaultBiometricStateService(stateProvider); @@ -146,13 +145,6 @@ describe("BiometricStateService", () => { }); describe("setPromptCancelled", () => { - let existingState: Record; - - beforeEach(() => { - existingState = { ["otherUser" as UserId]: false }; - stateProvider.global.getFake(PROMPT_CANCELLED).stateSubject.next(existingState); - }); - test("observable is updated", async () => { await sut.setPromptCancelled(); @@ -162,39 +154,10 @@ describe("BiometricStateService", () => { it("updates state", async () => { await sut.setPromptCancelled(); - const nextMock = stateProvider.global.getFake(PROMPT_CANCELLED).nextMock; - expect(nextMock).toHaveBeenCalledWith({ ...existingState, [userId]: true }); + const nextMock = stateProvider.activeUser.getFake(PROMPT_CANCELLED).nextMock; + expect(nextMock).toHaveBeenCalledWith([userId, true]); expect(nextMock).toHaveBeenCalledTimes(1); }); - - it("throws when called with no active user", async () => { - await accountService.switchAccount(null); - await expect(sut.setPromptCancelled()).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("resetPromptCancelled", () => { - it("deletes all prompt cancelled state", async () => { - await sut.resetPromptCancelled(); - - 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.setPromptCancelled(); - - await sut.resetPromptCancelled(); - - expect(emissions).toEqual([false, true, false]); - }); }); describe("setPromptAutomatically", () => { diff --git a/libs/common/src/platform/biometrics/biometric-state.service.ts b/libs/common/src/platform/biometrics/biometric-state.service.ts index 40d157df65..2047d137b5 100644 --- a/libs/common/src/platform/biometrics/biometric-state.service.ts +++ b/libs/common/src/platform/biometrics/biometric-state.service.ts @@ -1,4 +1,4 @@ -import { Observable, firstValueFrom, map, combineLatest } from "rxjs"; +import { Observable, firstValueFrom, map } from "rxjs"; import { UserId } from "../../types/guid"; import { EncryptedString, EncString } from "../models/domain/enc-string"; @@ -107,7 +107,7 @@ export class DefaultBiometricStateService implements BiometricStateService { private requirePasswordOnStartState: ActiveUserState; private encryptedClientKeyHalfState: ActiveUserState; private dismissedRequirePasswordOnStartCalloutState: ActiveUserState; - private promptCancelledState: GlobalState>; + private promptCancelledState: ActiveUserState; private promptAutomaticallyState: ActiveUserState; private fingerprintValidatedState: GlobalState; biometricUnlockEnabled$: Observable; @@ -138,15 +138,8 @@ export class DefaultBiometricStateService implements BiometricStateService { this.dismissedRequirePasswordOnStartCallout$ = this.dismissedRequirePasswordOnStartCalloutState.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.promptCancelledState = this.stateProvider.getActive(PROMPT_CANCELLED); + this.promptCancelled$ = this.promptCancelledState.state$.pipe(map(Boolean)); this.promptAutomaticallyState = this.stateProvider.getActive(PROMPT_AUTOMATICALLY); this.promptAutomatically$ = this.promptAutomaticallyState.state$.pipe(map(Boolean)); @@ -209,15 +202,6 @@ export class DefaultBiometricStateService implements BiometricStateService { async logout(userId: UserId): Promise { await this.stateProvider.getUser(userId, ENCRYPTED_CLIENT_KEY_HALF).update(() => null); - await this.promptCancelledState.update( - (record) => { - delete record[userId]; - return record; - }, - { - shouldUpdate: (record) => record[userId] == true, - }, - ); await this.stateProvider.getUser(userId, PROMPT_CANCELLED).update(() => null); // Persist auto prompt setting through logout // Persist dismissed require password on start callout through logout @@ -228,24 +212,7 @@ export class DefaultBiometricStateService implements BiometricStateService { } async setPromptCancelled(): Promise { - 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; - }, - }, - ); + await this.promptCancelledState.update(() => true); } async resetPromptCancelled(): Promise { diff --git a/libs/common/src/platform/biometrics/biometric.state.spec.ts b/libs/common/src/platform/biometrics/biometric.state.spec.ts index 420a0fb86e..a3b110c77c 100644 --- a/libs/common/src/platform/biometrics/biometric.state.spec.ts +++ b/libs/common/src/platform/biometrics/biometric.state.spec.ts @@ -14,7 +14,7 @@ import { describe.each([ [ENCRYPTED_CLIENT_KEY_HALF, "encryptedClientKeyHalf"], [DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT, true], - [PROMPT_CANCELLED, { userId1: true, userId2: false }], + [PROMPT_CANCELLED, true], [PROMPT_AUTOMATICALLY, true], [REQUIRE_PASSWORD_ON_START, true], [BIOMETRIC_UNLOCK_ENABLED, true], diff --git a/libs/common/src/platform/biometrics/biometric.state.ts b/libs/common/src/platform/biometrics/biometric.state.ts index aa16e14baa..a5041ca8d0 100644 --- a/libs/common/src/platform/biometrics/biometric.state.ts +++ b/libs/common/src/platform/biometrics/biometric.state.ts @@ -1,4 +1,3 @@ -import { UserId } from "../../types/guid"; import { EncryptedString } from "../models/domain/enc-string"; import { KeyDefinition, BIOMETRIC_SETTINGS_DISK } from "../state"; @@ -57,7 +56,7 @@ export const DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT = new KeyDefinition( +export const PROMPT_CANCELLED = new KeyDefinition( BIOMETRIC_SETTINGS_DISK, "promptCancelled", { diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index bb93e6295e..77a35ccb87 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -33,7 +33,6 @@ import { DomainSettingsMigrator } from "./migrations/34-move-domain-settings-to- import { MoveThemeToStateProviderMigrator } from "./migrations/35-move-theme-to-state-providers"; import { VaultSettingsKeyMigrator } from "./migrations/36-move-show-card-and-identity-to-state-provider"; import { AvatarColorMigrator } from "./migrations/37-move-avatar-color-to-state-providers"; -import { DeleteBiometricPromptCancelledData } from "./migrations/38-delete-orphaned-biometric-prompt-data"; import { RemoveEverBeenUnlockedMigrator } from "./migrations/4-remove-ever-been-unlocked"; import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys"; import { RemoveLegacyEtmKeyMigrator } from "./migrations/6-remove-legacy-etm-key"; @@ -43,7 +42,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 2; -export const CURRENT_VERSION = 38; +export const CURRENT_VERSION = 37; export type MinVersion = typeof MIN_VERSION; export function createMigrationBuilder() { @@ -83,8 +82,7 @@ export function createMigrationBuilder() { .with(DomainSettingsMigrator, 33, 34) .with(MoveThemeToStateProviderMigrator, 34, 35) .with(VaultSettingsKeyMigrator, 35, 36) - .with(AvatarColorMigrator, 36, 37) - .with(DeleteBiometricPromptCancelledData, 37, CURRENT_VERSION); + .with(AvatarColorMigrator, 36, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/common/src/state-migrations/migrations/38-delete-orphaned-biometric-prompt-data.spec.ts b/libs/common/src/state-migrations/migrations/38-delete-orphaned-biometric-prompt-data.spec.ts deleted file mode 100644 index d6083e91fe..0000000000 --- a/libs/common/src/state-migrations/migrations/38-delete-orphaned-biometric-prompt-data.spec.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { runMigrator } from "../migration-helper.spec"; -import { IRREVERSIBLE } from "../migrator"; - -import { DeleteBiometricPromptCancelledData } from "./38-delete-orphaned-biometric-prompt-data"; - -describe("MoveThemeToStateProviders", () => { - const sut = new DeleteBiometricPromptCancelledData(37, 38); - - describe("migrate", () => { - it("deletes promptCancelled from all users", async () => { - const output = await runMigrator(sut, { - authenticatedAccounts: ["user-1", "user-2"], - "user_user-1_biometricSettings_promptCancelled": true, - "user_user-2_biometricSettings_promptCancelled": false, - }); - - expect(output).toEqual({ - authenticatedAccounts: ["user-1", "user-2"], - }); - }); - }); - - describe("rollback", () => { - it("is irreversible", async () => { - await expect(runMigrator(sut, {}, "rollback")).rejects.toThrow(IRREVERSIBLE); - }); - }); -}); diff --git a/libs/common/src/state-migrations/migrations/38-delete-orphaned-biometric-prompt-data.ts b/libs/common/src/state-migrations/migrations/38-delete-orphaned-biometric-prompt-data.ts deleted file mode 100644 index 8498528f24..0000000000 --- a/libs/common/src/state-migrations/migrations/38-delete-orphaned-biometric-prompt-data.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { KeyDefinitionLike, MigrationHelper } from "../migration-helper"; -import { IRREVERSIBLE, Migrator } from "../migrator"; - -export const PROMPT_CANCELLED: KeyDefinitionLike = { - key: "promptCancelled", - stateDefinition: { name: "biometricSettings" }, -}; - -export class DeleteBiometricPromptCancelledData extends Migrator<37, 38> { - async migrate(helper: MigrationHelper): Promise { - await Promise.all( - (await helper.getAccounts()).map(async ({ userId }) => { - if (helper.getFromUser(userId, PROMPT_CANCELLED) != null) { - await helper.removeFromUser(userId, PROMPT_CANCELLED); - } - }), - ); - } - - async rollback(helper: MigrationHelper): Promise { - throw IRREVERSIBLE; - } -}