From c8c1ed42baa2c665d6b98d7640c1e5910f35b4f1 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 15 Feb 2024 15:29:29 -0500 Subject: [PATCH] [PM-5537] Remove Unecessary Biometric State (#7762) * Create state for biometric client key halves * Move enc string util to central utils * Provide biometric state through service * Use biometric state to track client key half * Create migration for client key half * Ensure client key half is removed on logout * Remove account data for client key half * Remove unnecessary key definition likes * Remove moved state from account * Fix null-conditional operator failure * Simplify migration * Remove lame test * Fix test type * Add migrator * Remove state that is never read. * Remove unnecessary biometric state We don't need to determine platform in desktop background, it can be done in the UI at any time. * Fix merge * Use platform utils to identify OS desktop type --- .../background/nativeMessaging.background.ts | 2 - .../src/popup/settings/settings.component.ts | 2 - .../src/app/accounts/settings.component.ts | 27 ++++++++-- apps/desktop/src/auth/lock.component.ts | 11 ++++ apps/desktop/src/main.ts | 3 -- .../main/biometric/biometric.darwin.main.ts | 11 +--- .../main/biometric/biometric.windows.main.ts | 7 --- .../biometrics.service.abstraction.ts | 2 - .../main/biometric/biometrics.service.spec.ts | 12 +---- .../main/biometric/biometrics.service.ts | 4 -- .../src/auth/components/lock.component.ts | 2 - .../platform/abstractions/state.service.ts | 6 --- .../platform/models/domain/global-state.ts | 4 -- .../src/platform/services/state.service.ts | 52 ------------------- 14 files changed, 36 insertions(+), 109 deletions(-) diff --git a/apps/browser/src/background/nativeMessaging.background.ts b/apps/browser/src/background/nativeMessaging.background.ts index 6177763d9c..ece5536c56 100644 --- a/apps/browser/src/background/nativeMessaging.background.ts +++ b/apps/browser/src/background/nativeMessaging.background.ts @@ -296,8 +296,6 @@ export class NativeMessagingBackground { switch (message.command) { case "biometricUnlock": { - await this.stateService.setBiometricAwaitingAcceptance(null); - if (message.response === "not enabled") { this.messagingService.send("showDialog", { title: { key: "biometricsNotEnabledTitle" }, diff --git a/apps/browser/src/popup/settings/settings.component.ts b/apps/browser/src/popup/settings/settings.component.ts index f15b25d92d..703d2802a3 100644 --- a/apps/browser/src/popup/settings/settings.component.ts +++ b/apps/browser/src/popup/settings/settings.component.ts @@ -369,14 +369,12 @@ export class SettingsComponent implements OnInit { const awaitDesktopDialogRef = AwaitDesktopDialogComponent.open(this.dialogService); const awaitDesktopDialogClosed = firstValueFrom(awaitDesktopDialogRef.closed); - await this.stateService.setBiometricAwaitingAcceptance(true); await this.cryptoService.refreshAdditionalKeys(); await Promise.race([ awaitDesktopDialogClosed.then(async (result) => { if (result !== true) { this.form.controls.biometric.setValue(false); - await this.stateService.setBiometricAwaitingAcceptance(null); } }), this.platformUtilsService diff --git a/apps/desktop/src/app/accounts/settings.component.ts b/apps/desktop/src/app/accounts/settings.component.ts index e88e10e1be..10b7d20d38 100644 --- a/apps/desktop/src/app/accounts/settings.component.ts +++ b/apps/desktop/src/app/accounts/settings.component.ts @@ -24,6 +24,7 @@ import { DialogService } from "@bitwarden/components"; import { SetPinComponent } from "../../auth/components/set-pin.component"; import { flagEnabled } from "../../platform/flags"; import { ElectronStateService } from "../../platform/services/electron-state.service.abstraction"; + @Component({ selector: "app-settings", templateUrl: "settings.component.html", @@ -39,9 +40,7 @@ export class SettingsComponent implements OnInit { themeOptions: any[]; clearClipboardOptions: any[]; supportsBiometric: boolean; - biometricText: string; additionalBiometricSettingsText: string; - autoPromptBiometricsText: string; showAlwaysShowDock = false; requireEnableTray = false; showDuckDuckGoIntegrationOption = false; @@ -275,12 +274,10 @@ export class SettingsComponent implements OnInit { this.showMinToTray = this.platformUtilsService.getDevice() !== DeviceType.LinuxDesktop; this.showAlwaysShowDock = this.platformUtilsService.getDevice() === DeviceType.MacOsDesktop; this.supportsBiometric = await this.platformUtilsService.supportsBiometric(); - this.biometricText = await this.stateService.getBiometricText(); this.additionalBiometricSettingsText = this.biometricText === "unlockWithTouchId" ? "additionalTouchIdSettings" : "additionalWindowsHelloSettings"; - this.autoPromptBiometricsText = await this.stateService.getNoAutoPromptBiometricsText(); this.previousVaultTimeout = this.form.value.vaultTimeout; this.refreshTimeoutSettings$ @@ -667,4 +664,26 @@ export class SettingsComponent implements OnInit { this.destroy$.next(); this.destroy$.complete(); } + + get biometricText() { + switch (this.platformUtilsService.getDevice()) { + case DeviceType.MacOsDesktop: + return "unlockWithTouchId"; + case DeviceType.WindowsDesktop: + return "unlockWithWindowsHello"; + default: + throw new Error("Unsupported platform"); + } + } + + get autoPromptBiometricsText() { + switch (this.platformUtilsService.getDevice()) { + case DeviceType.MacOsDesktop: + return "autoPromptTouchId"; + case DeviceType.WindowsDesktop: + return "autoPromptWindowsHello"; + default: + throw new Error("Unsupported platform"); + } + } } diff --git a/apps/desktop/src/auth/lock.component.ts b/apps/desktop/src/auth/lock.component.ts index 6af535eda4..3f462b395d 100644 --- a/apps/desktop/src/auth/lock.component.ts +++ b/apps/desktop/src/auth/lock.component.ts @@ -185,4 +185,15 @@ export class LockComponent extends BaseLockComponent { await this.stateService.setDismissedBiometricRequirePasswordOnStart(); } } + + get biometricText() { + switch (this.platformUtilsService.getDevice()) { + case DeviceType.MacOsDesktop: + return "unlockWithTouchId"; + case DeviceType.WindowsDesktop: + return "unlockWithWindowsHello"; + default: + throw new Error("Unsupported platform"); + } + } } diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index d98bd88e0e..5c18814a0b 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -225,9 +225,6 @@ export class Main { } this.powerMonitorMain.init(); await this.updaterMain.init(); - if (this.biometricsService != null) { - await this.biometricsService.init(); - } if ( (await this.stateService.getEnableBrowserIntegration()) || diff --git a/apps/desktop/src/platform/main/biometric/biometric.darwin.main.ts b/apps/desktop/src/platform/main/biometric/biometric.darwin.main.ts index ae1c9dd0cf..169ee871c7 100644 --- a/apps/desktop/src/platform/main/biometric/biometric.darwin.main.ts +++ b/apps/desktop/src/platform/main/biometric/biometric.darwin.main.ts @@ -1,21 +1,12 @@ import { systemPreferences } from "electron"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { passwords } from "@bitwarden/desktop-native"; import { OsBiometricService } from "./biometrics.service.abstraction"; export default class BiometricDarwinMain implements OsBiometricService { - constructor( - private i18nservice: I18nService, - private stateService: StateService, - ) {} - - async init() { - await this.stateService.setBiometricText("unlockWithTouchId"); - await this.stateService.setNoAutoPromptBiometricsText("autoPromptTouchId"); - } + constructor(private i18nservice: I18nService) {} async osSupportsBiometric(): Promise { return systemPreferences.canPromptTouchID(); diff --git a/apps/desktop/src/platform/main/biometric/biometric.windows.main.ts b/apps/desktop/src/platform/main/biometric/biometric.windows.main.ts index 99ebe79702..b715e852b3 100644 --- a/apps/desktop/src/platform/main/biometric/biometric.windows.main.ts +++ b/apps/desktop/src/platform/main/biometric/biometric.windows.main.ts @@ -5,7 +5,6 @@ import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/sym import { biometrics, passwords } from "@bitwarden/desktop-native"; import { WindowMain } from "../../../main/window.main"; -import { ElectronStateService } from "../../services/electron-state.service.abstraction"; import { OsBiometricService } from "./biometrics.service.abstraction"; @@ -21,15 +20,9 @@ export default class BiometricWindowsMain implements OsBiometricService { constructor( private i18nService: I18nService, private windowMain: WindowMain, - private stateService: ElectronStateService, private logService: LogService, ) {} - async init() { - await this.stateService.setBiometricText("unlockWithWindowsHello"); - await this.stateService.setNoAutoPromptBiometricsText("autoPromptWindowsHello"); - } - async osSupportsBiometric(): Promise { return await biometrics.available(); } diff --git a/apps/desktop/src/platform/main/biometric/biometrics.service.abstraction.ts b/apps/desktop/src/platform/main/biometric/biometrics.service.abstraction.ts index 2112fc7cf5..2d5c1d19eb 100644 --- a/apps/desktop/src/platform/main/biometric/biometrics.service.abstraction.ts +++ b/apps/desktop/src/platform/main/biometric/biometrics.service.abstraction.ts @@ -1,5 +1,4 @@ export abstract class BiometricsServiceAbstraction { - init: () => Promise; osSupportsBiometric: () => Promise; canAuthBiometric: ({ service, @@ -26,7 +25,6 @@ export abstract class BiometricsServiceAbstraction { } export interface OsBiometricService { - init: () => Promise; osSupportsBiometric: () => Promise; authenticateBiometric: () => Promise; getBiometricKey: ( diff --git a/apps/desktop/src/platform/main/biometric/biometrics.service.spec.ts b/apps/desktop/src/platform/main/biometric/biometrics.service.spec.ts index 5b81ff9986..124b7db914 100644 --- a/apps/desktop/src/platform/main/biometric/biometrics.service.spec.ts +++ b/apps/desktop/src/platform/main/biometric/biometrics.service.spec.ts @@ -43,13 +43,7 @@ describe("biometrics tests", function () { const mockService = mock(); (sut as any).platformSpecificService = mockService; - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - sut.init(); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - sut.setEncryptionKeyHalf({ service: "test", key: "test", value: "test" }); - expect(mockService.init).toBeCalled(); + await sut.setEncryptionKeyHalf({ service: "test", key: "test", value: "test" }); await sut.canAuthBiometric({ service: "test", key: "test", userId }); expect(mockService.osSupportsBiometric).toBeCalled(); @@ -111,9 +105,6 @@ describe("biometrics tests", function () { innerService = mock(); (sut as any).platformSpecificService = innerService; - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - sut.init(); }); it("should return false if client key half is required and not provided", async () => { @@ -128,7 +119,6 @@ describe("biometrics tests", function () { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises sut.setEncryptionKeyHalf({ service: "test", key: "test", value: "test" }); - expect(innerService.init).toBeCalled(); await sut.canAuthBiometric({ service: "test", key: "test", userId }); expect(innerService.osSupportsBiometric).toBeCalled(); diff --git a/apps/desktop/src/platform/main/biometric/biometrics.service.ts b/apps/desktop/src/platform/main/biometric/biometrics.service.ts index dadd4713dc..2a6577a2ee 100644 --- a/apps/desktop/src/platform/main/biometric/biometrics.service.ts +++ b/apps/desktop/src/platform/main/biometric/biometrics.service.ts @@ -58,10 +58,6 @@ export class BiometricsService implements BiometricsServiceAbstraction { this.platformSpecificService = new NoopBiometricsService(); } - async init() { - return await this.platformSpecificService.init(); - } - async osSupportsBiometric() { return await this.platformSpecificService.osSupportsBiometric(); } diff --git a/libs/angular/src/auth/components/lock.component.ts b/libs/angular/src/auth/components/lock.component.ts index fc8c6eac20..3bc46b86ad 100644 --- a/libs/angular/src/auth/components/lock.component.ts +++ b/libs/angular/src/auth/components/lock.component.ts @@ -41,7 +41,6 @@ export class LockComponent implements OnInit, OnDestroy { formPromise: Promise; supportsBiometric: boolean; biometricLock: boolean; - biometricText: string; protected successRoute = "vault"; protected forcePasswordResetRoute = "update-temp-password"; @@ -343,7 +342,6 @@ export class LockComponent implements OnInit, OnDestroy { (await this.vaultTimeoutSettingsService.isBiometricLockSet()) && ((await this.cryptoService.hasUserKeyStored(KeySuffixOptions.Biometric)) || !this.platformUtilsService.supportsSecureStorage()); - this.biometricText = await this.stateService.getBiometricText(); this.email = await this.stateService.getEmail(); this.webVaultHostname = await this.environmentService.getHost(); diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index b71344e6ce..d6673c27ba 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -69,12 +69,8 @@ export abstract class StateService { setApiKeyClientSecret: (value: string, options?: StorageOptions) => Promise; getAutoConfirmFingerPrints: (options?: StorageOptions) => Promise; setAutoConfirmFingerprints: (value: boolean, options?: StorageOptions) => Promise; - getBiometricAwaitingAcceptance: (options?: StorageOptions) => Promise; - setBiometricAwaitingAcceptance: (value: boolean, options?: StorageOptions) => Promise; getBiometricFingerprintValidated: (options?: StorageOptions) => Promise; setBiometricFingerprintValidated: (value: boolean, options?: StorageOptions) => Promise; - getBiometricText: (options?: StorageOptions) => Promise; - setBiometricText: (value: string, options?: StorageOptions) => Promise; getBiometricUnlock: (options?: StorageOptions) => Promise; setBiometricUnlock: (value: boolean, options?: StorageOptions) => Promise; getCanAccessPremium: (options?: StorageOptions) => Promise; @@ -378,8 +374,6 @@ export abstract class StateService { setMinimizeOnCopyToClipboard: (value: boolean, options?: StorageOptions) => Promise; getNeverDomains: (options?: StorageOptions) => Promise<{ [id: string]: unknown }>; setNeverDomains: (value: { [id: string]: unknown }, options?: StorageOptions) => Promise; - getNoAutoPromptBiometricsText: (options?: StorageOptions) => Promise; - setNoAutoPromptBiometricsText: (value: string, options?: StorageOptions) => Promise; getOpenAtLogin: (options?: StorageOptions) => Promise; setOpenAtLogin: (value: boolean, options?: StorageOptions) => Promise; getOrganizationInvitation: (options?: StorageOptions) => Promise; diff --git a/libs/common/src/platform/models/domain/global-state.ts b/libs/common/src/platform/models/domain/global-state.ts index d5b00cc5d6..3efa3028c6 100644 --- a/libs/common/src/platform/models/domain/global-state.ts +++ b/libs/common/src/platform/models/domain/global-state.ts @@ -11,15 +11,11 @@ export class GlobalState { window?: WindowState = new WindowState(); twoFactorToken?: string; disableFavicon?: boolean; - biometricAwaitingAcceptance?: boolean; biometricFingerprintValidated?: boolean; vaultTimeout?: number; vaultTimeoutAction?: string; loginRedirect?: any; mainWindowSize?: number; - enableBiometrics?: boolean; - biometricText?: string; - noAutoPromptBiometricsText?: string; enableTray?: boolean; enableMinimizeToTray?: boolean; enableCloseToTray?: boolean; diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index 38174a79fe..004b9eff83 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -372,24 +372,6 @@ export class StateService< ); } - async getBiometricAwaitingAcceptance(options?: StorageOptions): Promise { - return ( - (await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) - ?.biometricAwaitingAcceptance ?? false - ); - } - - async setBiometricAwaitingAcceptance(value: boolean, options?: StorageOptions): Promise { - const globals = await this.getGlobals( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - globals.biometricAwaitingAcceptance = value; - await this.saveGlobals( - globals, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - async getBiometricFingerprintValidated(options?: StorageOptions): Promise { return ( (await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) @@ -408,23 +390,6 @@ export class StateService< ); } - async getBiometricText(options?: StorageOptions): Promise { - return ( - await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions())) - )?.biometricText; - } - - async setBiometricText(value: string, options?: StorageOptions): Promise { - const globals = await this.getGlobals( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - globals.biometricText = value; - await this.saveGlobals( - globals, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - async getBiometricUnlock(options?: StorageOptions): Promise { return ( (await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) @@ -1989,23 +1954,6 @@ export class StateService< ); } - async getNoAutoPromptBiometricsText(options?: StorageOptions): Promise { - return ( - await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions())) - )?.noAutoPromptBiometricsText; - } - - async setNoAutoPromptBiometricsText(value: string, options?: StorageOptions): Promise { - const globals = await this.getGlobals( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - globals.noAutoPromptBiometricsText = value; - await this.saveGlobals( - globals, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - async getOpenAtLogin(options?: StorageOptions): Promise { return ( (await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions())))