From f83dcf2b24665780089f9300defffaeb16e5f2f5 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 7 Mar 2024 19:41:56 -0600 Subject: [PATCH] Move fingerprint validated to biometric state povider (#8058) --- .../browser/src/background/main.background.ts | 1 + .../background/nativeMessaging.background.ts | 12 ++------ .../src/popup/settings/settings.component.ts | 2 +- .../src/app/services/services.module.ts | 1 + .../biometric-state.service.spec.ts | 30 +++++++++++++++++++ .../biometrics/biometric-state.service.ts | 21 ++++++++++++- .../biometrics/biometric.state.spec.ts | 4 ++- .../platform/biometrics/biometric.state.ts | 11 +++++++ .../src/platform/services/system.service.ts | 7 +++-- 9 files changed, 74 insertions(+), 15 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index c5608dc830..ea51fa8805 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -814,6 +814,7 @@ export default class MainBackground { this.stateService, this.autofillSettingsService, this.vaultTimeoutSettingsService, + this.biometricStateService, ); // Other fields diff --git a/apps/browser/src/background/nativeMessaging.background.ts b/apps/browser/src/background/nativeMessaging.background.ts index e4fb46d960..2717a7b2b5 100644 --- a/apps/browser/src/background/nativeMessaging.background.ts +++ b/apps/browser/src/background/nativeMessaging.background.ts @@ -84,10 +84,6 @@ export class NativeMessagingBackground { private authService: AuthService, private biometricStateService: BiometricStateService, ) { - // 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 - this.stateService.setBiometricFingerprintValidated(false); - if (chrome?.permissions?.onAdded) { // Reload extension to activate nativeMessaging chrome.permissions.onAdded.addListener((permissions) => { @@ -100,9 +96,7 @@ export class NativeMessagingBackground { async connect() { this.appId = await this.appIdService.getAppId(); - // 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 - this.stateService.setBiometricFingerprintValidated(false); + await this.biometricStateService.setFingerprintValidated(false); return new Promise((resolve, reject) => { this.port = BrowserApi.connectNative("com.8bit.bitwarden"); @@ -148,9 +142,7 @@ export class NativeMessagingBackground { if (this.validatingFingerprint) { this.validatingFingerprint = false; - // 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 - this.stateService.setBiometricFingerprintValidated(true); + await this.biometricStateService.setFingerprintValidated(true); } this.sharedSecret = new SymmetricCryptoKey(decrypted); this.secureSetupResolve(); diff --git a/apps/browser/src/popup/settings/settings.component.ts b/apps/browser/src/popup/settings/settings.component.ts index f622cffd3e..c83c7b5e72 100644 --- a/apps/browser/src/popup/settings/settings.component.ts +++ b/apps/browser/src/popup/settings/settings.component.ts @@ -415,7 +415,7 @@ export class SettingsComponent implements OnInit { ]); } else { await this.biometricStateService.setBiometricUnlockEnabled(false); - await this.stateService.setBiometricFingerprintValidated(false); + await this.biometricStateService.setFingerprintValidated(false); } } diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index 2fde9744b9..3b50b6ced7 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -126,6 +126,7 @@ const RELOAD_CALLBACK = new InjectionToken<() => any>("RELOAD_CALLBACK"); StateServiceAbstraction, AutofillSettingsServiceAbstraction, VaultTimeoutSettingsService, + BiometricStateService, ], }, { 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 54471a25cd..716ad627c1 100644 --- a/libs/common/src/platform/biometrics/biometric-state.service.spec.ts +++ b/libs/common/src/platform/biometrics/biometric-state.service.spec.ts @@ -12,6 +12,7 @@ import { BIOMETRIC_UNLOCK_ENABLED, DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT, ENCRYPTED_CLIENT_KEY_HALF, + FINGERPRINT_VALIDATED, PROMPT_AUTOMATICALLY, PROMPT_CANCELLED, REQUIRE_PASSWORD_ON_START, @@ -67,6 +68,19 @@ describe("BiometricStateService", () => { }); }); + describe("fingerprintValidated$", () => { + it("emits when the fingerprint validated state changes", async () => { + const state = stateProvider.global.getFake(FINGERPRINT_VALIDATED); + state.stateSubject.next(undefined); + + expect(await firstValueFrom(sut.fingerprintValidated$)).toBe(false); + + state.stateSubject.next(true); + + expect(await firstValueFrom(sut.fingerprintValidated$)).toEqual(true); + }); + }); + describe("setEncryptedClientKeyHalf", () => { it("updates encryptedClientKeyHalf$", async () => { await sut.setEncryptedClientKeyHalf(encClientKeyHalf); @@ -207,4 +221,20 @@ describe("BiometricStateService", () => { expect(await sut.getBiometricUnlockEnabled(userId)).toBe(false); }); }); + + describe("setFingerprintValidated", () => { + it("updates fingerprintValidated$", async () => { + await sut.setFingerprintValidated(true); + + expect(await firstValueFrom(sut.fingerprintValidated$)).toBe(true); + }); + + it("updates state", async () => { + await sut.setFingerprintValidated(true); + + expect(stateProvider.global.getFake(FINGERPRINT_VALIDATED).nextMock).toHaveBeenCalledWith( + true, + ); + }); + }); }); diff --git a/libs/common/src/platform/biometrics/biometric-state.service.ts b/libs/common/src/platform/biometrics/biometric-state.service.ts index b00090eb26..2047d137b5 100644 --- a/libs/common/src/platform/biometrics/biometric-state.service.ts +++ b/libs/common/src/platform/biometrics/biometric-state.service.ts @@ -2,7 +2,7 @@ import { Observable, firstValueFrom, map } from "rxjs"; import { UserId } from "../../types/guid"; import { EncryptedString, EncString } from "../models/domain/enc-string"; -import { ActiveUserState, StateProvider } from "../state"; +import { ActiveUserState, GlobalState, StateProvider } from "../state"; import { BIOMETRIC_UNLOCK_ENABLED, @@ -11,6 +11,7 @@ import { DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT, PROMPT_AUTOMATICALLY, PROMPT_CANCELLED, + FINGERPRINT_VALIDATED, } from "./biometric.state"; export abstract class BiometricStateService { @@ -49,6 +50,10 @@ export abstract class BiometricStateService { * tracks the currently active user */ promptAutomatically$: Observable; + /** + * Whether or not IPC fingerprint has been validated by the user this session. + */ + fingerprintValidated$: Observable; /** * Updates the require password on start state for the currently active user. @@ -88,6 +93,11 @@ export abstract class BiometricStateService { * @param prompt Whether or not to prompt for biometrics on application start. */ abstract setPromptAutomatically(prompt: boolean): Promise; + /** + * Updates whether or not IPC has been validated by the user this session + * @param validated the value to save + */ + abstract setFingerprintValidated(validated: boolean): Promise; abstract logout(userId: UserId): Promise; } @@ -99,12 +109,14 @@ export class DefaultBiometricStateService implements BiometricStateService { private dismissedRequirePasswordOnStartCalloutState: ActiveUserState; private promptCancelledState: ActiveUserState; private promptAutomaticallyState: ActiveUserState; + private fingerprintValidatedState: GlobalState; biometricUnlockEnabled$: Observable; encryptedClientKeyHalf$: Observable; requirePasswordOnStart$: Observable; dismissedRequirePasswordOnStartCallout$: Observable; promptCancelled$: Observable; promptAutomatically$: Observable; + fingerprintValidated$: Observable; constructor(private stateProvider: StateProvider) { this.biometricUnlockEnabledState = this.stateProvider.getActive(BIOMETRIC_UNLOCK_ENABLED); @@ -130,6 +142,9 @@ export class DefaultBiometricStateService implements BiometricStateService { this.promptCancelled$ = this.promptCancelledState.state$.pipe(map(Boolean)); this.promptAutomaticallyState = this.stateProvider.getActive(PROMPT_AUTOMATICALLY); this.promptAutomatically$ = this.promptAutomaticallyState.state$.pipe(map(Boolean)); + + this.fingerprintValidatedState = this.stateProvider.getGlobal(FINGERPRINT_VALIDATED); + this.fingerprintValidated$ = this.fingerprintValidatedState.state$.pipe(map(Boolean)); } async setBiometricUnlockEnabled(enabled: boolean): Promise { @@ -207,6 +222,10 @@ export class DefaultBiometricStateService implements BiometricStateService { async setPromptAutomatically(prompt: boolean): Promise { await this.promptAutomaticallyState.update(() => prompt); } + + async setFingerprintValidated(validated: boolean): Promise { + await this.fingerprintValidatedState.update(() => validated); + } } function encryptedClientKeyHalfToEncString( diff --git a/libs/common/src/platform/biometrics/biometric.state.spec.ts b/libs/common/src/platform/biometrics/biometric.state.spec.ts index 4f79f8da73..a3b110c77c 100644 --- a/libs/common/src/platform/biometrics/biometric.state.spec.ts +++ b/libs/common/src/platform/biometrics/biometric.state.spec.ts @@ -5,6 +5,7 @@ import { BIOMETRIC_UNLOCK_ENABLED, DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT, ENCRYPTED_CLIENT_KEY_HALF, + FINGERPRINT_VALIDATED, PROMPT_AUTOMATICALLY, PROMPT_CANCELLED, REQUIRE_PASSWORD_ON_START, @@ -16,7 +17,8 @@ describe.each([ [PROMPT_CANCELLED, true], [PROMPT_AUTOMATICALLY, true], [REQUIRE_PASSWORD_ON_START, true], - [BIOMETRIC_UNLOCK_ENABLED, "test"], + [BIOMETRIC_UNLOCK_ENABLED, true], + [FINGERPRINT_VALIDATED, true], ])( "deserializes state %s", ( diff --git a/libs/common/src/platform/biometrics/biometric.state.ts b/libs/common/src/platform/biometrics/biometric.state.ts index 8796366c88..a5041ca8d0 100644 --- a/libs/common/src/platform/biometrics/biometric.state.ts +++ b/libs/common/src/platform/biometrics/biometric.state.ts @@ -74,3 +74,14 @@ export const PROMPT_AUTOMATICALLY = new KeyDefinition( deserializer: (obj) => obj, }, ); + +/** + * Stores whether or not IPC handshake has been validated this session. + */ +export const FINGERPRINT_VALIDATED = new KeyDefinition( + BIOMETRIC_SETTINGS_DISK, + "fingerprintValidated", + { + deserializer: (obj) => obj, + }, +); diff --git a/libs/common/src/platform/services/system.service.ts b/libs/common/src/platform/services/system.service.ts index 06f9fcf8fb..d19390c45e 100644 --- a/libs/common/src/platform/services/system.service.ts +++ b/libs/common/src/platform/services/system.service.ts @@ -9,6 +9,7 @@ import { MessagingService } from "../abstractions/messaging.service"; import { PlatformUtilsService } from "../abstractions/platform-utils.service"; import { StateService } from "../abstractions/state.service"; import { SystemService as SystemServiceAbstraction } from "../abstractions/system.service"; +import { BiometricStateService } from "../biometrics/biometric-state.service"; import { Utils } from "../misc/utils"; export class SystemService implements SystemServiceAbstraction { @@ -23,6 +24,7 @@ export class SystemService implements SystemServiceAbstraction { private stateService: StateService, private autofillSettingsService: AutofillSettingsServiceAbstraction, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, + private biometricStateService: BiometricStateService, ) {} async startProcessReload(authService: AuthService): Promise { @@ -54,8 +56,9 @@ export class SystemService implements SystemServiceAbstraction { } private async executeProcessReload() { - const biometricLockedFingerprintValidated = - await this.stateService.getBiometricFingerprintValidated(); + const biometricLockedFingerprintValidated = await firstValueFrom( + this.biometricStateService.fingerprintValidated$, + ); if (!biometricLockedFingerprintValidated) { clearInterval(this.reloadInterval); this.reloadInterval = null;