diff --git a/apps/browser/src/auth/popup/lock.component.ts b/apps/browser/src/auth/popup/lock.component.ts index 42c2d44e31..7aa5a96b4e 100644 --- a/apps/browser/src/auth/popup/lock.component.ts +++ b/apps/browser/src/auth/popup/lock.component.ts @@ -1,5 +1,6 @@ import { Component, NgZone } from "@angular/core"; import { Router } from "@angular/router"; +import { firstValueFrom } from "rxjs"; import { LockComponent as BaseLockComponent } from "@bitwarden/angular/auth/components/lock.component"; import { PinCryptoServiceAbstraction } from "@bitwarden/auth/common"; @@ -19,6 +20,7 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service" import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { DialogService } from "@bitwarden/components"; @@ -59,6 +61,7 @@ export class LockComponent extends BaseLockComponent { userVerificationService: UserVerificationService, pinCryptoService: PinCryptoServiceAbstraction, private routerService: BrowserRouterService, + biometricStateService: BiometricStateService, ) { super( router, @@ -80,6 +83,7 @@ export class LockComponent extends BaseLockComponent { deviceTrustCryptoService, userVerificationService, pinCryptoService, + biometricStateService, ); this.successRoute = "/tabs/current"; this.isInitialLockScreen = (window as any).previousPopupUrl == null; @@ -100,8 +104,9 @@ export class LockComponent extends BaseLockComponent { async ngOnInit() { await super.ngOnInit(); - const disableAutoBiometricsPrompt = - (await this.stateService.getDisableAutoBiometricsPrompt()) ?? true; + const disableAutoBiometricsPrompt = await firstValueFrom( + this.biometricStateService.promptAutomatically$, + ); window.setTimeout(async () => { document.getElementById(this.pinEnabled ? "pin" : "masterPassword")?.focus(); diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 3df24d40fe..d742e2ebe6 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1,3 +1,5 @@ +import { firstValueFrom } from "rxjs"; + import { PinCryptoServiceAbstraction, PinCryptoService, @@ -64,6 +66,10 @@ import { ObservableStorageService, } from "@bitwarden/common/platform/abstractions/storage.service"; import { SystemService as SystemServiceAbstraction } from "@bitwarden/common/platform/abstractions/system.service"; +import { + BiometricStateService, + DefaultBiometricStateService, +} from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { StateFactory } from "@bitwarden/common/platform/factories/state-factory"; import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state"; import { AppIdService } from "@bitwarden/common/platform/services/app-id.service"; @@ -280,6 +286,7 @@ export default class MainBackground { individualVaultExportService: IndividualVaultExportServiceAbstraction; organizationVaultExportService: OrganizationVaultExportServiceAbstraction; vaultSettingsService: VaultSettingsServiceAbstraction; + biometricStateService: BiometricStateService; // Passed to the popup for Safari to workaround issues with theming, downloading, etc. backgroundWindow = window; @@ -321,7 +328,7 @@ export default class MainBackground { } }; - const logoutCallback = async (expired: boolean, userId?: string) => + const logoutCallback = async (expired: boolean, userId?: UserId) => await this.logout(expired, userId); this.messagingService = this.popupOnlyContext @@ -386,6 +393,7 @@ export default class MainBackground { this.stateProvider, this.accountService, ); + this.biometricStateService = new DefaultBiometricStateService(this.stateProvider); const migrationRunner = new MigrationRunner( this.storageService, @@ -1043,7 +1051,9 @@ export default class MainBackground { } } - async logout(expired: boolean, userId?: string) { + async logout(expired: boolean, userId?: UserId) { + userId ??= (await firstValueFrom(this.accountService.activeAccount$))?.id; + await this.eventUploadService.uploadEvents(userId); await Promise.all([ @@ -1058,6 +1068,7 @@ export default class MainBackground { this.vaultTimeoutSettingsService.clear(userId), this.keyConnectorService.clear(), this.vaultFilterService.clear(), + this.biometricStateService.logout(userId), // We intentionally do not clear the autofillSettingsService ]); diff --git a/apps/browser/src/popup/settings/settings.component.ts b/apps/browser/src/popup/settings/settings.component.ts index 703d2802a3..1914d2583a 100644 --- a/apps/browser/src/popup/settings/settings.component.ts +++ b/apps/browser/src/popup/settings/settings.component.ts @@ -31,6 +31,7 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { DialogService } from "@bitwarden/components"; import { SetPinComponent } from "../../auth/popup/components/set-pin.component"; @@ -101,6 +102,7 @@ export class SettingsComponent implements OnInit { private userVerificationService: UserVerificationService, private dialogService: DialogService, private changeDetectorRef: ChangeDetectorRef, + private biometricStateService: BiometricStateService, ) { this.accountSwitcherEnabled = enableAccountSwitching(); } @@ -176,7 +178,9 @@ export class SettingsComponent implements OnInit { ), pin: pinStatus !== "DISABLED", biometric: await this.vaultTimeoutSettingsService.isBiometricLockSet(), - enableAutoBiometricsPrompt: !(await this.stateService.getDisableAutoBiometricsPrompt()), + enableAutoBiometricsPrompt: await firstValueFrom( + this.biometricStateService.promptAutomatically$, + ), }; this.form.patchValue(initialValues); // Emit event to initialize `pairwise` operator @@ -416,8 +420,8 @@ export class SettingsComponent implements OnInit { } async updateAutoBiometricsPrompt() { - await this.stateService.setDisableAutoBiometricsPrompt( - !this.form.value.enableAutoBiometricsPrompt, + await this.biometricStateService.setPromptAutomatically( + this.form.value.enableAutoBiometricsPrompt, ); } diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index e9acb7a5af..fb24d98292 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -39,6 +39,7 @@ import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/s import { ClientType } from "@bitwarden/common/enums"; import { ConfigApiServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config-api.service.abstraction"; import { KeyGenerationService as KeyGenerationServiceAbstraction } from "@bitwarden/common/platform/abstractions/key-generation.service"; +import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { KeySuffixOptions, LogLevelType } from "@bitwarden/common/platform/enums"; import { StateFactory } from "@bitwarden/common/platform/factories/state-factory"; import { Account } from "@bitwarden/common/platform/models/domain/account"; @@ -205,6 +206,7 @@ export class Main { derivedStateProvider: DerivedStateProvider; stateProvider: StateProvider; loginStrategyService: LoginStrategyServiceAbstraction; + biometricStateService: BiometricStateService; constructor() { let p = null; @@ -627,6 +629,7 @@ export class Main { this.collectionService.clear(userId as UserId), this.policyService.clear(userId), this.passwordGenerationService.clear(), + this.biometricStateService.logout(userId as UserId), ]); await this.stateService.clean(); process.env.BW_SESSION = null; diff --git a/apps/desktop/src/app/accounts/settings.component.ts b/apps/desktop/src/app/accounts/settings.component.ts index 10b7d20d38..4c5c2060f8 100644 --- a/apps/desktop/src/app/accounts/settings.component.ts +++ b/apps/desktop/src/app/accounts/settings.component.ts @@ -16,6 +16,7 @@ import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.se import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { ThemeType, KeySuffixOptions } from "@bitwarden/common/platform/enums"; import { Utils } from "@bitwarden/common/platform/misc/utils"; @@ -23,7 +24,6 @@ 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", @@ -110,7 +110,7 @@ export class SettingsComponent implements OnInit { private i18nService: I18nService, private platformUtilsService: PlatformUtilsService, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, - private stateService: ElectronStateService, + private stateService: StateService, private messagingService: MessagingService, private cryptoService: CryptoService, private modalService: ModalService, @@ -242,7 +242,7 @@ export class SettingsComponent implements OnInit { ), pin: this.userHasPinSet, biometric: await this.vaultTimeoutSettingsService.isBiometricLockSet(), - autoPromptBiometrics: !(await this.stateService.getDisableAutoBiometricsPrompt()), + autoPromptBiometrics: await firstValueFrom(this.biometricStateService.promptAutomatically$), requirePasswordOnStart: await firstValueFrom( this.biometricStateService.requirePasswordOnStart$, ), @@ -453,9 +453,9 @@ export class SettingsComponent implements OnInit { // Recommended settings for Windows Hello this.form.controls.requirePasswordOnStart.setValue(true); this.form.controls.autoPromptBiometrics.setValue(false); - await this.stateService.setDisableAutoBiometricsPrompt(true); + await this.biometricStateService.setPromptAutomatically(false); await this.biometricStateService.setRequirePasswordOnStart(true); - await this.stateService.setDismissedBiometricRequirePasswordOnStart(); + await this.biometricStateService.setDismissedRequirePasswordOnStartCallout(); } await this.cryptoService.refreshAdditionalKeys(); @@ -475,10 +475,9 @@ export class SettingsComponent implements OnInit { // require password on start must be disabled if auto prompt biometrics is enabled this.form.controls.requirePasswordOnStart.setValue(false); await this.updateRequirePasswordOnStart(); - - await this.stateService.setDisableAutoBiometricsPrompt(null); + await this.biometricStateService.setPromptAutomatically(true); } else { - await this.stateService.setDisableAutoBiometricsPrompt(true); + await this.biometricStateService.setPromptAutomatically(false); } } @@ -492,7 +491,7 @@ export class SettingsComponent implements OnInit { } else { await this.biometricStateService.setRequirePasswordOnStart(false); } - await this.stateService.setDismissedBiometricRequirePasswordOnStart(); + await this.biometricStateService.setDismissedRequirePasswordOnStartCallout(); await this.cryptoService.refreshAdditionalKeys(); } diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index aa650b7073..c7b140dd1f 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -38,6 +38,7 @@ import { MessagingService } from "@bitwarden/common/platform/abstractions/messag import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { SystemService } from "@bitwarden/common/platform/abstractions/system.service"; +import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; import { UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; @@ -147,6 +148,7 @@ export class AppComponent implements OnInit, OnDestroy { private userVerificationService: UserVerificationService, private configService: ConfigServiceAbstraction, private dialogService: DialogService, + private biometricStateService: BiometricStateService, ) {} ngOnInit() { @@ -576,6 +578,7 @@ export class AppComponent implements OnInit, OnDestroy { await this.vaultTimeoutSettingsService.clear(userBeingLoggedOut); await this.policyService.clear(userBeingLoggedOut); await this.keyConnectorService.clear(); + await this.biometricStateService.logout(userBeingLoggedOut as UserId); preLogoutActiveUserId = this.activeUserId; await this.stateService.clean({ userId: userBeingLoggedOut }); diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index 06ac9e1deb..274564489c 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -57,7 +57,6 @@ import { ElectronRendererMessagingService } from "../../platform/services/electr import { ElectronRendererSecureStorageService } from "../../platform/services/electron-renderer-secure-storage.service"; import { ElectronRendererStorageService } from "../../platform/services/electron-renderer-storage.service"; import { ElectronStateService } from "../../platform/services/electron-state.service"; -import { ElectronStateService as ElectronStateServiceAbstraction } from "../../platform/services/electron-state.service.abstraction"; import { I18nRendererService } from "../../platform/services/i18n.renderer.service"; import { EncryptedMessageHandlerService } from "../../services/encrypted-message-handler.service"; import { NativeMessageHandlerService } from "../../services/native-message-handler.service"; @@ -140,10 +139,6 @@ const RELOAD_CALLBACK = new InjectionToken<() => any>("RELOAD_CALLBACK"); STATE_SERVICE_USE_CACHE, ], }, - { - provide: ElectronStateServiceAbstraction, - useExisting: StateServiceAbstraction, - }, { provide: FileDownloadService, useClass: DesktopFileDownloadService, diff --git a/apps/desktop/src/auth/lock.component.spec.ts b/apps/desktop/src/auth/lock.component.spec.ts index f428bda50e..6ecf93deb8 100644 --- a/apps/desktop/src/auth/lock.component.spec.ts +++ b/apps/desktop/src/auth/lock.component.spec.ts @@ -21,12 +21,11 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { DialogService } from "@bitwarden/components"; -import { ElectronStateService } from "../platform/services/electron-state.service.abstraction"; - import { LockComponent } from "./lock.component"; // ipc mock global @@ -43,14 +42,15 @@ const isWindowVisibleMock = jest.fn(); describe("LockComponent", () => { let component: LockComponent; let fixture: ComponentFixture; - let stateServiceMock: MockProxy; + let stateServiceMock: MockProxy; + const biometricStateService = mock(); let messagingServiceMock: MockProxy; let broadcasterServiceMock: MockProxy; let platformUtilsServiceMock: MockProxy; let activatedRouteMock: MockProxy; - beforeEach(() => { - stateServiceMock = mock(); + beforeEach(async () => { + stateServiceMock = mock(); stateServiceMock.activeAccount$ = of(null); messagingServiceMock = mock(); @@ -60,9 +60,11 @@ describe("LockComponent", () => { activatedRouteMock = mock(); activatedRouteMock.queryParams = mock(); - // 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 - TestBed.configureTestingModule({ + biometricStateService.dismissedRequirePasswordOnStartCallout$ = of(false); + biometricStateService.promptAutomatically$ = of(false); + biometricStateService.promptCancelled$ = of(false); + + await TestBed.configureTestingModule({ declarations: [LockComponent, I18nPipe], providers: [ { @@ -94,7 +96,7 @@ describe("LockComponent", () => { useValue: mock(), }, { - provide: ElectronStateService, + provide: StateService, useValue: stateServiceMock, }, { @@ -143,18 +145,19 @@ describe("LockComponent", () => { }, { provide: BiometricStateService, - useValue: mock(), + useValue: biometricStateService, }, ], schemas: [NO_ERRORS_SCHEMA], }).compileComponents(); - }); - beforeEach(() => { fixture = TestBed.createComponent(LockComponent); component = fixture.componentInstance; fixture.detectChanges(); - jest.clearAllMocks(); + }); + + afterEach(() => { + jest.resetAllMocks(); }); describe("ngOnInit", () => { @@ -164,15 +167,15 @@ describe("LockComponent", () => { expect(superNgOnInitSpy).toHaveBeenCalledTimes(1); }); - it('should set "autoPromptBiometric" to true if "stateService.getDisableAutoBiometricsPrompt()" resolves to false', async () => { - stateServiceMock.getDisableAutoBiometricsPrompt.mockResolvedValue(false); + it('should set "autoPromptBiometric" to true if "biometricState.promptAutomatically$" resolves to true', async () => { + biometricStateService.promptAutomatically$ = of(true); await component.ngOnInit(); expect(component["autoPromptBiometric"]).toBe(true); }); - it('should set "autoPromptBiometric" to false if "stateService.getDisableAutoBiometricsPrompt()" resolves to true', async () => { - stateServiceMock.getDisableAutoBiometricsPrompt.mockResolvedValue(true); + it('should set "autoPromptBiometric" to false if "biometricState.promptAutomatically$" resolves to false', async () => { + biometricStateService.promptAutomatically$ = of(false); await component.ngOnInit(); expect(component["autoPromptBiometric"]).toBe(false); diff --git a/apps/desktop/src/auth/lock.component.ts b/apps/desktop/src/auth/lock.component.ts index 3f462b395d..e88ce17ca5 100644 --- a/apps/desktop/src/auth/lock.component.ts +++ b/apps/desktop/src/auth/lock.component.ts @@ -1,6 +1,6 @@ import { Component, NgZone } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; -import { switchMap } from "rxjs"; +import { firstValueFrom, switchMap } from "rxjs"; import { LockComponent as BaseLockComponent } from "@bitwarden/angular/auth/components/lock.component"; import { PinCryptoServiceAbstraction } from "@bitwarden/auth/common"; @@ -19,12 +19,11 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { DialogService } from "@bitwarden/components"; -import { ElectronStateService } from "../platform/services/electron-state.service.abstraction"; - const BroadcasterSubscriptionId = "LockComponent"; @Component({ @@ -46,7 +45,7 @@ export class LockComponent extends BaseLockComponent { vaultTimeoutService: VaultTimeoutService, vaultTimeoutSettingsService: VaultTimeoutSettingsService, environmentService: EnvironmentService, - protected override stateService: ElectronStateService, + protected override stateService: StateService, apiService: ApiService, private route: ActivatedRoute, private broadcasterService: BroadcasterService, @@ -59,7 +58,7 @@ export class LockComponent extends BaseLockComponent { deviceTrustCryptoService: DeviceTrustCryptoServiceAbstraction, userVerificationService: UserVerificationService, pinCryptoService: PinCryptoServiceAbstraction, - private biometricStateService: BiometricStateService, + biometricStateService: BiometricStateService, ) { super( router, @@ -81,12 +80,15 @@ export class LockComponent extends BaseLockComponent { deviceTrustCryptoService, userVerificationService, pinCryptoService, + biometricStateService, ); } async ngOnInit() { await super.ngOnInit(); - this.autoPromptBiometric = !(await this.stateService.getDisableAutoBiometricsPrompt()); + this.autoPromptBiometric = await firstValueFrom( + this.biometricStateService.promptAutomatically$, + ); this.biometricReady = await this.canUseBiometric(); await this.displayBiometricUpdateWarning(); @@ -140,7 +142,7 @@ export class LockComponent extends BaseLockComponent { return; } - if (await this.stateService.getBiometricPromptCancelled()) { + if (await firstValueFrom(this.biometricStateService.promptCancelled$)) { return; } @@ -162,7 +164,7 @@ export class LockComponent extends BaseLockComponent { } private async displayBiometricUpdateWarning(): Promise { - if (await this.stateService.getDismissedBiometricRequirePasswordOnStart()) { + if (await firstValueFrom(this.biometricStateService.dismissedRequirePasswordOnStartCallout$)) { return; } @@ -179,10 +181,10 @@ export class LockComponent extends BaseLockComponent { await this.biometricStateService.setRequirePasswordOnStart(response); if (response) { - await this.stateService.setDisableAutoBiometricsPrompt(true); + await this.biometricStateService.setPromptAutomatically(false); } this.supportsBiometric = await this.canUseBiometric(); - await this.stateService.setDismissedBiometricRequirePasswordOnStart(); + await this.biometricStateService.setDismissedRequirePasswordOnStartCallout(); } } diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index 5c18814a0b..f64ae8e589 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -3,6 +3,7 @@ import * as path from "path"; import { app } from "electron"; import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; +import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { DefaultBiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { StateFactory } from "@bitwarden/common/platform/factories/state-factory"; import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state"; @@ -44,7 +45,7 @@ export class Main { memoryStorageService: MemoryStorageService; memoryStorageForStateProviders: MemoryStorageServiceForStateProviders; messagingService: ElectronMainMessagingService; - stateService: ElectronStateService; + stateService: StateService; environmentService: EnvironmentService; desktopCredentialStorageListener: DesktopCredentialStorageListener; migrationRunner: MigrationRunner; @@ -146,8 +147,11 @@ export class Main { false, // Do not use disk caching because this will get out of sync with the renderer service ); + const biometricStateService = new DefaultBiometricStateService(stateProvider); + this.windowMain = new WindowMain( this.stateService, + biometricStateService, this.logService, this.storageService, (arg) => this.processDeepLink(arg), @@ -169,8 +173,6 @@ export class Main { this.updaterMain, ); - const biometricStateService = new DefaultBiometricStateService(stateProvider); - this.biometricsService = new BiometricsService( this.i18nService, this.windowMain, diff --git a/apps/desktop/src/main/window.main.ts b/apps/desktop/src/main/window.main.ts index 64eca116dd..3154a8ccc1 100644 --- a/apps/desktop/src/main/window.main.ts +++ b/apps/desktop/src/main/window.main.ts @@ -8,6 +8,7 @@ import { WindowState } from "@bitwarden/common/models/domain/window-state"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service"; +import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { cleanUserAgent, @@ -36,6 +37,7 @@ export class WindowMain { constructor( private stateService: StateService, + private biometricStateService: BiometricStateService, private logService: LogService, private storageService: AbstractStorageService, private argvCallback: (argv: string[]) => void = null, @@ -90,11 +92,9 @@ export class WindowMain { // This method will be called when Electron is shutting // down the application. - app.on("before-quit", () => { + app.on("before-quit", async () => { // Allow biometric to auto-prompt on reload - // 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.setBiometricPromptCancelled(false); + await this.biometricStateService.resetPromptCancelled(); this.isQuitting = true; }); 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 124b7db914..c7f48aac22 100644 --- a/apps/desktop/src/platform/main/biometric/biometrics.service.spec.ts +++ b/apps/desktop/src/platform/main/biometric/biometrics.service.spec.ts @@ -3,11 +3,11 @@ import { mock, MockProxy } from "jest-mock-extended"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { UserId } from "@bitwarden/common/types/guid"; import { WindowMain } from "../../../main/window.main"; -import { ElectronStateService } from "../../services/electron-state.service.abstraction"; import BiometricDarwinMain from "./biometric.darwin.main"; import BiometricWindowsMain from "./biometric.windows.main"; @@ -24,7 +24,7 @@ jest.mock("@bitwarden/desktop-native", () => { describe("biometrics tests", function () { const i18nService = mock(); const windowMain = mock(); - const stateService = mock(); + const stateService = mock(); const logService = mock(); const messagingService = mock(); const biometricStateService = mock(); diff --git a/apps/desktop/src/platform/main/biometric/biometrics.service.ts b/apps/desktop/src/platform/main/biometric/biometrics.service.ts index 2a6577a2ee..301de1a994 100644 --- a/apps/desktop/src/platform/main/biometric/biometrics.service.ts +++ b/apps/desktop/src/platform/main/biometric/biometrics.service.ts @@ -1,11 +1,11 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { UserId } from "@bitwarden/common/types/guid"; import { WindowMain } from "../../../main/window.main"; -import { ElectronStateService } from "../../services/electron-state.service.abstraction"; import { BiometricsServiceAbstraction, OsBiometricService } from "./biometrics.service.abstraction"; @@ -16,7 +16,7 @@ export class BiometricsService implements BiometricsServiceAbstraction { constructor( private i18nService: I18nService, private windowMain: WindowMain, - private stateService: ElectronStateService, + private stateService: StateService, private logService: LogService, private messagingService: MessagingService, private platform: NodeJS.Platform, diff --git a/apps/desktop/src/platform/services/electron-crypto.service.spec.ts b/apps/desktop/src/platform/services/electron-crypto.service.spec.ts index 58ea5c412e..ba86df1d9b 100644 --- a/apps/desktop/src/platform/services/electron-crypto.service.spec.ts +++ b/apps/desktop/src/platform/services/electron-crypto.service.spec.ts @@ -6,6 +6,7 @@ import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt. import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/key-generation.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { makeEncString } from "@bitwarden/common/spec"; @@ -19,7 +20,6 @@ import { } from "../../../../../libs/common/spec/fake-account-service"; import { ElectronCryptoService } from "./electron-crypto.service"; -import { ElectronStateService } from "./electron-state.service.abstraction"; describe("electronCryptoService", () => { let sut: ElectronCryptoService; @@ -29,7 +29,7 @@ describe("electronCryptoService", () => { const encryptService = mock(); const platformUtilService = mock(); const logService = mock(); - const stateService = mock(); + const stateService = mock(); let accountService: FakeAccountService; let stateProvider: FakeStateProvider; const biometricStateService = mock(); diff --git a/apps/desktop/src/platform/services/electron-crypto.service.ts b/apps/desktop/src/platform/services/electron-crypto.service.ts index 184be5e1fb..a7e46dfb1a 100644 --- a/apps/desktop/src/platform/services/electron-crypto.service.ts +++ b/apps/desktop/src/platform/services/electron-crypto.service.ts @@ -4,6 +4,7 @@ import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt. import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/key-generation.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { KeySuffixOptions } from "@bitwarden/common/platform/enums"; import { Utils } from "@bitwarden/common/platform/misc/utils"; @@ -15,8 +16,6 @@ import { CsprngString } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; import { UserKey, MasterKey } from "@bitwarden/common/types/key"; -import { ElectronStateService } from "./electron-state.service.abstraction"; - export class ElectronCryptoService extends CryptoService { constructor( keyGenerationService: KeyGenerationService, @@ -24,7 +23,7 @@ export class ElectronCryptoService extends CryptoService { encryptService: EncryptService, platformUtilsService: PlatformUtilsService, logService: LogService, - protected override stateService: ElectronStateService, + stateService: StateService, accountService: AccountService, stateProvider: StateProvider, private biometricStateService: BiometricStateService, diff --git a/apps/desktop/src/platform/services/electron-state.service.abstraction.ts b/apps/desktop/src/platform/services/electron-state.service.abstraction.ts deleted file mode 100644 index 26b1a25f57..0000000000 --- a/apps/desktop/src/platform/services/electron-state.service.abstraction.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; -import { StorageOptions } from "@bitwarden/common/platform/models/domain/storage-options"; - -import { Account } from "../../models/account"; - -export abstract class ElectronStateService extends StateService { - getDismissedBiometricRequirePasswordOnStart: (options?: StorageOptions) => Promise; - setDismissedBiometricRequirePasswordOnStart: (options?: StorageOptions) => Promise; -} diff --git a/apps/desktop/src/platform/services/electron-state.service.ts b/apps/desktop/src/platform/services/electron-state.service.ts index 85182c02da..f4399221d2 100644 --- a/apps/desktop/src/platform/services/electron-state.service.ts +++ b/apps/desktop/src/platform/services/electron-state.service.ts @@ -7,12 +7,7 @@ import { DeviceKey } from "@bitwarden/common/types/key"; import { Account } from "../../models/account"; -import { ElectronStateService as ElectronStateServiceAbstraction } from "./electron-state.service.abstraction"; - -export class ElectronStateService - extends BaseStateService - implements ElectronStateServiceAbstraction -{ +export class ElectronStateService extends BaseStateService { private partialKeys = { deviceKey: "_deviceKey", }; @@ -23,24 +18,6 @@ export class ElectronStateService await super.addAccount(account); } - async getDismissedBiometricRequirePasswordOnStart(options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - return account?.settings?.dismissedBiometricRequirePasswordOnStartCallout; - } - - async setDismissedBiometricRequirePasswordOnStart(options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - account.settings.dismissedBiometricRequirePasswordOnStartCallout = true; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - override async getDeviceKey(options?: StorageOptions): Promise { options = this.reconcileOptions(options, await this.defaultSecureStorageOptions()); if (options?.userId == null) { diff --git a/apps/desktop/src/services/encrypted-message-handler.service.ts b/apps/desktop/src/services/encrypted-message-handler.service.ts index 106ef8e6a6..e38339d5ad 100644 --- a/apps/desktop/src/services/encrypted-message-handler.service.ts +++ b/apps/desktop/src/services/encrypted-message-handler.service.ts @@ -5,6 +5,7 @@ import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherType } from "@bitwarden/common/vault/enums"; @@ -24,11 +25,10 @@ import { GenerateResponse } from "../models/native-messaging/encrypted-message-r import { MessageResponseData } from "../models/native-messaging/encrypted-message-responses/message-response-data"; import { SuccessStatusResponse } from "../models/native-messaging/encrypted-message-responses/success-status-response"; import { UserStatusErrorResponse } from "../models/native-messaging/encrypted-message-responses/user-status-error-response"; -import { ElectronStateService } from "../platform/services/electron-state.service"; export class EncryptedMessageHandlerService { constructor( - private stateService: ElectronStateService, + private stateService: StateService, private authService: AuthService, private cipherService: CipherService, private policyService: PolicyService, diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index 25a8675d83..1a81ce555b 100644 --- a/apps/web/src/app/app.component.ts +++ b/apps/web/src/app/app.component.ts @@ -20,7 +20,9 @@ import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.se import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; +import { UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service"; import { InternalFolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; @@ -82,6 +84,7 @@ export class AppComponent implements OnDestroy, OnInit { private keyConnectorService: KeyConnectorService, private configService: ConfigServiceAbstraction, private dialogService: DialogService, + private biometricStateService: BiometricStateService, ) {} ngOnInit() { @@ -265,6 +268,7 @@ export class AppComponent implements OnDestroy, OnInit { this.policyService.clear(userId), this.passwordGenerationService.clear(), this.keyConnectorService.clear(), + this.biometricStateService.logout(userId as UserId), ]); this.searchService.clearIndex(); diff --git a/apps/web/src/app/auth/lock.component.ts b/apps/web/src/app/auth/lock.component.ts index 7088623aaf..c4f8d276bb 100644 --- a/apps/web/src/app/auth/lock.component.ts +++ b/apps/web/src/app/auth/lock.component.ts @@ -17,6 +17,7 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service" import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { DialogService } from "@bitwarden/components"; @@ -45,6 +46,7 @@ export class LockComponent extends BaseLockComponent { deviceTrustCryptoService: DeviceTrustCryptoServiceAbstraction, userVerificationService: UserVerificationService, pinCryptoService: PinCryptoServiceAbstraction, + biometricStateService: BiometricStateService, ) { super( router, @@ -66,6 +68,7 @@ export class LockComponent extends BaseLockComponent { deviceTrustCryptoService, userVerificationService, pinCryptoService, + biometricStateService, ); } diff --git a/libs/angular/src/auth/components/lock.component.ts b/libs/angular/src/auth/components/lock.component.ts index 3bc46b86ad..d5da3e826e 100644 --- a/libs/angular/src/auth/components/lock.component.ts +++ b/libs/angular/src/auth/components/lock.component.ts @@ -23,6 +23,7 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service" import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { HashPurpose, KeySuffixOptions } from "@bitwarden/common/platform/enums"; import { PinLockType } from "@bitwarden/common/services/vault-timeout/vault-timeout-settings.service"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; @@ -73,6 +74,7 @@ export class LockComponent implements OnInit, OnDestroy { protected deviceTrustCryptoService: DeviceTrustCryptoServiceAbstraction, protected userVerificationService: UserVerificationService, protected pinCryptoService: PinCryptoServiceAbstraction, + protected biometricStateService: BiometricStateService, ) {} async ngOnInit() { @@ -117,7 +119,7 @@ export class LockComponent implements OnInit, OnDestroy { return; } - await this.stateService.setBiometricPromptCancelled(true); + await this.biometricStateService.setPromptCancelled(); const userKey = await this.cryptoService.getUserKeyFromStorage(KeySuffixOptions.Biometric); if (userKey) { @@ -274,7 +276,7 @@ export class LockComponent implements OnInit, OnDestroy { private async doContinue(evaluatePasswordAfterUnlock: boolean) { await this.stateService.setEverBeenUnlocked(true); - await this.stateService.setBiometricPromptCancelled(false); + await this.biometricStateService.resetPromptCancelled(); this.messagingService.send("unlocked"); if (evaluatePasswordAfterUnlock) { diff --git a/libs/common/spec/fake-state.ts b/libs/common/spec/fake-state.ts index 96f3d41dd1..38a28b1f02 100644 --- a/libs/common/spec/fake-state.ts +++ b/libs/common/spec/fake-state.ts @@ -66,6 +66,7 @@ export class FakeGlobalState implements GlobalState { }); updateMock = this.update as jest.MockedFunction; + /** Tracks update values resolved by `FakeState.update` */ nextMock = jest.fn(); get state$() { @@ -128,6 +129,7 @@ export class FakeSingleUserState implements SingleUserState { updateMock = this.update as jest.MockedFunction; + /** Tracks update values resolved by `FakeState.update` */ nextMock = jest.fn(); private _keyDefinition: KeyDefinition | null = null; get keyDefinition() { @@ -190,6 +192,7 @@ export class FakeActiveUserState implements ActiveUserState { updateMock = this.update as jest.MockedFunction; + /** Tracks update values resolved by `FakeState.update` */ nextMock = jest.fn(); private _keyDefinition: KeyDefinition | null = null; diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 8de781674f..2611acf5c3 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -169,18 +169,6 @@ export abstract class StateService { * @deprecated For migration purposes only, use setUserKeyBiometric instead */ setCryptoMasterKeyBiometric: (value: BiometricKey, options?: StorageOptions) => Promise; - /** - * Gets a flag for if the biometrics process has been cancelled. - * Process reload occurs when biometrics is cancelled, so we store to disk to prevent - * it from reprompting and creating a loop. - */ - getBiometricPromptCancelled: (options?: StorageOptions) => Promise; - /** - * Sets a flag for if the biometrics process has been cancelled. - * Process reload occurs when biometrics is cancelled, so we store to disk to prevent - * it from reprompting and creating a loop. - */ - setBiometricPromptCancelled: (value: boolean, options?: StorageOptions) => Promise; getDecryptedCiphers: (options?: StorageOptions) => Promise; setDecryptedCiphers: (value: CipherView[], options?: StorageOptions) => Promise; getDecryptedPasswordGenerationHistory: ( @@ -218,8 +206,6 @@ export abstract class StateService { setDefaultUriMatch: (value: UriMatchType, options?: StorageOptions) => Promise; getDisableAddLoginNotification: (options?: StorageOptions) => Promise; setDisableAddLoginNotification: (value: boolean, options?: StorageOptions) => Promise; - getDisableAutoBiometricsPrompt: (options?: StorageOptions) => Promise; - setDisableAutoBiometricsPrompt: (value: boolean, options?: StorageOptions) => Promise; getDisableBadgeCounter: (options?: StorageOptions) => Promise; setDisableBadgeCounter: (value: boolean, options?: StorageOptions) => Promise; getDisableChangedPasswordNotification: (options?: StorageOptions) => Promise; 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 5752f69271..44be94983f 100644 --- a/libs/common/src/platform/biometrics/biometric-state.service.spec.ts +++ b/libs/common/src/platform/biometrics/biometric-state.service.spec.ts @@ -8,7 +8,13 @@ import { UserId } from "../../types/guid"; import { EncryptedString } from "../models/domain/enc-string"; import { BiometricStateService, DefaultBiometricStateService } from "./biometric-state.service"; -import { ENCRYPTED_CLIENT_KEY_HALF, REQUIRE_PASSWORD_ON_START } from "./biometric.state"; +import { + DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT, + ENCRYPTED_CLIENT_KEY_HALF, + PROMPT_AUTOMATICALLY, + PROMPT_CANCELLED, + REQUIRE_PASSWORD_ON_START, +} from "./biometric.state"; describe("BiometricStateService", () => { let sut: BiometricStateService; @@ -96,4 +102,56 @@ describe("BiometricStateService", () => { expect(await sut.getRequirePasswordOnStart(userId)).toBe(true); }); }); + + describe("require password on start callout", () => { + it("should be false when not set", async () => { + expect(await firstValueFrom(sut.dismissedRequirePasswordOnStartCallout$)).toBe(false); + }); + + it("should be true when set", async () => { + await sut.setDismissedRequirePasswordOnStartCallout(); + + expect(await firstValueFrom(sut.dismissedRequirePasswordOnStartCallout$)).toBe(true); + }); + + it("should update disk state", async () => { + await sut.setDismissedRequirePasswordOnStartCallout(); + + expect( + stateProvider.activeUser.getFake(DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT).nextMock, + ).toHaveBeenCalledWith([userId, true]); + }); + }); + + describe("prompt cancelled", () => { + test("observable should be updated", async () => { + await sut.setPromptCancelled(); + + expect(await firstValueFrom(sut.promptCancelled$)).toBe(true); + }); + + it("should update state with set", async () => { + await sut.setPromptCancelled(); + + const nextMock = stateProvider.activeUser.getFake(PROMPT_CANCELLED).nextMock; + expect(nextMock).toHaveBeenCalledWith([userId, true]); + expect(nextMock).toHaveBeenCalledTimes(1); + }); + }); + + describe("prompt automatically", () => { + test("observable should be updated", async () => { + await sut.setPromptAutomatically(true); + + expect(await firstValueFrom(sut.promptAutomatically$)).toBe(true); + }); + + it("should update state with setPromptAutomatically", async () => { + await sut.setPromptAutomatically(true); + + const nextMock = stateProvider.activeUser.getFake(PROMPT_AUTOMATICALLY).nextMock; + expect(nextMock).toHaveBeenCalledWith([userId, true]); + expect(nextMock).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/libs/common/src/platform/biometrics/biometric-state.service.ts b/libs/common/src/platform/biometrics/biometric-state.service.ts index 0007a5f210..7033184dab 100644 --- a/libs/common/src/platform/biometrics/biometric-state.service.ts +++ b/libs/common/src/platform/biometrics/biometric-state.service.ts @@ -4,7 +4,13 @@ import { UserId } from "../../types/guid"; import { EncryptedString, EncString } from "../models/domain/enc-string"; import { ActiveUserState, StateProvider } from "../state"; -import { ENCRYPTED_CLIENT_KEY_HALF, REQUIRE_PASSWORD_ON_START } from "./biometric.state"; +import { + ENCRYPTED_CLIENT_KEY_HALF, + REQUIRE_PASSWORD_ON_START, + DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT, + PROMPT_AUTOMATICALLY, + PROMPT_CANCELLED, +} from "./biometric.state"; export abstract class BiometricStateService { /** @@ -20,6 +26,24 @@ export abstract class BiometricStateService { * tracks the currently active user */ requirePasswordOnStart$: Observable; + /** + * Indicates the user has been warned about the security implications of using biometrics and, depending on the OS, + * + * tracks the currently active user. + */ + dismissedRequirePasswordOnStartCallout$: Observable; + /** + * Whether the user has cancelled the biometric prompt. + * + * tracks the currently active user + */ + promptCancelled$: Observable; + /** + * Whether the user has elected to automatically prompt for biometrics. + * + * tracks the currently active user + */ + promptAutomatically$: Observable; /** * Updates the require password on start state for the currently active user. @@ -32,13 +56,38 @@ export abstract class BiometricStateService { abstract getEncryptedClientKeyHalf(userId: UserId): Promise; abstract getRequirePasswordOnStart(userId: UserId): Promise; abstract removeEncryptedClientKeyHalf(userId: UserId): Promise; + /** + * Updates the active user's state to reflect that they've been warned about requiring password on start. + */ + abstract setDismissedRequirePasswordOnStartCallout(): Promise; + /** + * Updates the active user's state to reflect that they've cancelled the biometric prompt this lock. + */ + abstract setPromptCancelled(): Promise; + /** + * Resets the active user's state to reflect that they haven't cancelled the biometric prompt this lock. + */ + abstract resetPromptCancelled(): Promise; + /** + * 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. + */ + abstract setPromptAutomatically(prompt: boolean): Promise; + + abstract logout(userId: UserId): Promise; } export class DefaultBiometricStateService implements BiometricStateService { private requirePasswordOnStartState: ActiveUserState; private encryptedClientKeyHalfState: ActiveUserState; + private dismissedRequirePasswordOnStartCalloutState: ActiveUserState; + private promptCancelledState: ActiveUserState; + private promptAutomaticallyState: ActiveUserState; encryptedClientKeyHalf$: Observable; requirePasswordOnStart$: Observable; + dismissedRequirePasswordOnStartCallout$: Observable; + promptCancelled$: Observable; + promptAutomatically$: Observable; constructor(private stateProvider: StateProvider) { this.requirePasswordOnStartState = this.stateProvider.getActive(REQUIRE_PASSWORD_ON_START); @@ -50,6 +99,17 @@ export class DefaultBiometricStateService implements BiometricStateService { 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((v) => !!v)); + + this.promptCancelledState = this.stateProvider.getActive(PROMPT_CANCELLED); + this.promptCancelled$ = this.promptCancelledState.state$.pipe(map((v) => !!v)); + this.promptAutomaticallyState = this.stateProvider.getActive(PROMPT_AUTOMATICALLY); + this.promptAutomatically$ = this.promptAutomaticallyState.state$.pipe(map((v) => !!v)); } async setRequirePasswordOnStart(value: boolean): Promise { @@ -97,6 +157,25 @@ export class DefaultBiometricStateService implements BiometricStateService { async logout(userId: UserId): Promise { await this.stateProvider.getUser(userId, ENCRYPTED_CLIENT_KEY_HALF).update(() => null); + await this.stateProvider.getUser(userId, PROMPT_CANCELLED).update(() => null); + // Persist auto prompt setting through logout + // Persist dismissed require password on start callout through logout + } + + async setDismissedRequirePasswordOnStartCallout(): Promise { + await this.dismissedRequirePasswordOnStartCalloutState.update(() => true); + } + + async setPromptCancelled(): Promise { + await this.promptCancelledState.update(() => true); + } + + async resetPromptCancelled(): Promise { + await this.promptCancelledState.update(() => null); + } + + async setPromptAutomatically(prompt: boolean): Promise { + await this.promptAutomaticallyState.update(() => prompt); } } diff --git a/libs/common/src/platform/biometrics/biometric.state.spec.ts b/libs/common/src/platform/biometrics/biometric.state.spec.ts index e36b5f434f..c868a5b927 100644 --- a/libs/common/src/platform/biometrics/biometric.state.spec.ts +++ b/libs/common/src/platform/biometrics/biometric.state.spec.ts @@ -1,25 +1,36 @@ -import { ENCRYPTED_CLIENT_KEY_HALF, REQUIRE_PASSWORD_ON_START } from "./biometric.state"; +import { EncryptedString } from "../models/domain/enc-string"; +import { KeyDefinition } from "../state"; -describe("require password on start", () => { - const sut = REQUIRE_PASSWORD_ON_START; +import { + DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT, + ENCRYPTED_CLIENT_KEY_HALF, + PROMPT_AUTOMATICALLY, + PROMPT_CANCELLED, + REQUIRE_PASSWORD_ON_START, +} from "./biometric.state"; - it("should deserialize require password on start state", () => { - const requirePasswordOnStart = "requirePasswordOnStart"; - - const result = sut.deserializer(JSON.parse(JSON.stringify(requirePasswordOnStart))); - - expect(result).toEqual(requirePasswordOnStart); - }); -}); - -describe("encrypted client key half", () => { - const sut = ENCRYPTED_CLIENT_KEY_HALF; - - it("should deserialize encrypted client key half state", () => { - const encryptedClientKeyHalf = "encryptedClientKeyHalf"; - - const result = sut.deserializer(JSON.parse(JSON.stringify(encryptedClientKeyHalf))); - - expect(result).toEqual(encryptedClientKeyHalf); - }); -}); +describe.each([ + [ENCRYPTED_CLIENT_KEY_HALF, "encryptedClientKeyHalf"], + [DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT, true], + [PROMPT_CANCELLED, true], + [PROMPT_AUTOMATICALLY, true], + [REQUIRE_PASSWORD_ON_START, true], +])( + "deserializes state %s", + ( + ...args: [KeyDefinition, EncryptedString] | [KeyDefinition, boolean] + ) => { + it("should deserialize state", () => { + const [keyDefinition, state] = args; + // Need to type check to avoid TS error due to array values being unions instead of guaranteed tuple pairs + if (typeof state === "boolean") { + const deserialized = keyDefinition.deserializer(JSON.parse(JSON.stringify(state))); + expect(deserialized).toEqual(state); + return; + } else { + const deserialized = keyDefinition.deserializer(JSON.parse(JSON.stringify(state))); + expect(deserialized).toEqual(state); + } + }); + }, +); diff --git a/libs/common/src/platform/biometrics/biometric.state.ts b/libs/common/src/platform/biometrics/biometric.state.ts index 2c231b9e76..84a4c13a5f 100644 --- a/libs/common/src/platform/biometrics/biometric.state.ts +++ b/libs/common/src/platform/biometrics/biometric.state.ts @@ -28,3 +28,38 @@ export const ENCRYPTED_CLIENT_KEY_HALF = new KeyDefinition( deserializer: (obj) => obj, }, ); + +/** + * 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 KeyDefinition( + BIOMETRIC_SETTINGS_DISK, + "dismissedBiometricRequirePasswordOnStartCallout", + { + deserializer: (obj) => obj, + }, +); + +/** + * 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( + BIOMETRIC_SETTINGS_DISK, + "promptCancelled", + { + deserializer: (obj) => obj, + }, +); + +/** + * Stores whether the user has elected to automatically prompt for biometric unlock on application start. + */ +export const PROMPT_AUTOMATICALLY = new KeyDefinition( + BIOMETRIC_SETTINGS_DISK, + "promptAutomatically", + { + deserializer: (obj) => obj, + }, +); diff --git a/libs/common/src/platform/models/domain/account.ts b/libs/common/src/platform/models/domain/account.ts index 40fc2e623b..9cafabbda4 100644 --- a/libs/common/src/platform/models/domain/account.ts +++ b/libs/common/src/platform/models/domain/account.ts @@ -203,7 +203,6 @@ export class AccountSettings { biometricUnlock?: boolean; clearClipboard?: number; defaultUriMatch?: UriMatchType; - disableAutoBiometricsPrompt?: boolean; disableBadgeCounter?: boolean; disableGa?: boolean; dontShowCardsCurrentTab?: boolean; @@ -227,7 +226,6 @@ export class AccountSettings { avatarColor?: string; smOnboardingTasks?: Record>; trustDeviceChoiceForDecryption?: boolean; - biometricPromptCancelled?: boolean; /** @deprecated July 2023, left for migration purposes*/ pinProtected?: EncryptionPair = new EncryptionPair(); diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index 96005ef394..050948293b 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -775,24 +775,6 @@ export class StateService< await this.saveSecureStorageKey(partialKeys.biometricKey, value, options); } - async getBiometricPromptCancelled(options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - return account?.settings?.biometricPromptCancelled; - } - - async setBiometricPromptCancelled(value: boolean, options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - account.settings.biometricPromptCancelled = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - @withPrototypeForArrayMembers(CipherView, CipherView.fromJSON) async getDecryptedCiphers(options?: StorageOptions): Promise { return ( @@ -928,24 +910,6 @@ export class StateService< ); } - async getDisableAutoBiometricsPrompt(options?: StorageOptions): Promise { - return ( - (await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) - ?.settings?.disableAutoBiometricsPrompt ?? false - ); - } - - async setDisableAutoBiometricsPrompt(value: boolean, options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - account.settings.disableAutoBiometricsPrompt = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - async getDisableBadgeCounter(options?: StorageOptions): Promise { return ( (await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index 72e8b34edb..7192c6e070 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -17,6 +17,7 @@ import { RequirePasswordOnStartMigrator } from "./migrations/19-migrate-require- import { PrivateKeyMigrator } from "./migrations/20-move-private-key-to-state-providers"; import { CollectionMigrator } from "./migrations/21-move-collections-state-to-state-provider"; import { CollapsedGroupingsMigrator } from "./migrations/22-move-collapsed-groupings-to-state-provider"; +import { MoveBiometricPromptsToStateProviders } from "./migrations/23-move-biometric-prompts-to-state-providers"; import { FixPremiumMigrator } from "./migrations/3-fix-premium"; import { RemoveEverBeenUnlockedMigrator } from "./migrations/4-remove-ever-been-unlocked"; import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys"; @@ -27,7 +28,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 2; -export const CURRENT_VERSION = 22; +export const CURRENT_VERSION = 23; export type MinVersion = typeof MIN_VERSION; export function createMigrationBuilder() { @@ -52,7 +53,8 @@ export function createMigrationBuilder() { .with(RequirePasswordOnStartMigrator, 18, 19) .with(PrivateKeyMigrator, 19, 20) .with(CollectionMigrator, 20, 21) - .with(CollapsedGroupingsMigrator, 21, CURRENT_VERSION); + .with(CollapsedGroupingsMigrator, 21, 22) + .with(MoveBiometricPromptsToStateProviders, 22, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/common/src/state-migrations/migrations/23-move-biometric-prompts-to-state-providers.spec.ts b/libs/common/src/state-migrations/migrations/23-move-biometric-prompts-to-state-providers.spec.ts new file mode 100644 index 0000000000..d041517ac6 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/23-move-biometric-prompts-to-state-providers.spec.ts @@ -0,0 +1,131 @@ +import { MockProxy, any } from "jest-mock-extended"; + +import { MigrationHelper } from "../migration-helper"; +import { mockMigrationHelper } from "../migration-helper.spec"; + +import { + MoveBiometricPromptsToStateProviders, + DISMISSED_BIOMETRIC_REQUIRE_PASSWORD_ON_START_CALLOUT, + PROMPT_AUTOMATICALLY, +} from "./23-move-biometric-prompts-to-state-providers"; + +function exampleJSON() { + return { + global: { + otherStuff: "otherStuff1", + }, + authenticatedAccounts: ["user-1", "user-2", "user-3"], + "user-1": { + settings: { + disableAutoBiometricsPrompt: false, + dismissedBiometricRequirePasswordOnStartCallout: true, + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }, + "user-2": { + otherStuff: "otherStuff4", + }, + }; +} + +function rollbackJSON() { + return { + "user_user-1_biometricSettings_dismissedBiometricRequirePasswordOnStartCallout": true, + "user_user-1_biometricSettings_promptAutomatically": "false", + global: { + otherStuff: "otherStuff1", + }, + authenticatedAccounts: ["user-1", "user-2", "user-3"], + "user-1": { + settings: { + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }, + "user-2": { + otherStuff: "otherStuff4", + }, + }; +} + +describe("MoveBiometricPromptsToStateProviders migrator", () => { + let helper: MockProxy; + let sut: MoveBiometricPromptsToStateProviders; + + describe("migrate", () => { + beforeEach(() => { + helper = mockMigrationHelper(exampleJSON(), 22); + sut = new MoveBiometricPromptsToStateProviders(22, 23); + }); + + it("should remove biometricUnlock, dismissedBiometricRequirePasswordOnStartCallout, and biometricEncryptionClientKeyHalf from all accounts", async () => { + await sut.migrate(helper); + expect(helper.set).toHaveBeenCalledTimes(2); + expect(helper.set).toHaveBeenCalledWith("user-1", { + settings: { + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }); + expect(helper.set).toHaveBeenCalledWith("user-2", { + otherStuff: "otherStuff4", + }); + }); + + it("should set dismissedBiometricRequirePasswordOnStartCallout value for account that have it", async () => { + await sut.migrate(helper); + + expect(helper.setToUser).toHaveBeenCalledWith( + "user-1", + DISMISSED_BIOMETRIC_REQUIRE_PASSWORD_ON_START_CALLOUT, + true, + ); + }); + + it("should not call extra setToUser", async () => { + await sut.migrate(helper); + + expect(helper.setToUser).toHaveBeenCalledTimes(2); + }); + }); + + describe("rollback", () => { + beforeEach(() => { + helper = mockMigrationHelper(rollbackJSON(), 23); + sut = new MoveBiometricPromptsToStateProviders(22, 23); + }); + + it.each([DISMISSED_BIOMETRIC_REQUIRE_PASSWORD_ON_START_CALLOUT, PROMPT_AUTOMATICALLY])( + "should null out new values %s", + async (keyDefinition) => { + await sut.rollback(helper); + + expect(helper.setToUser).toHaveBeenCalledWith("user-1", keyDefinition, null); + }, + ); + + it("should add explicit value back to accounts", async () => { + await sut.rollback(helper); + + expect(helper.set).toHaveBeenCalledTimes(1); + expect(helper.set).toHaveBeenCalledWith("user-1", { + settings: { + disableAutoBiometricsPrompt: false, + dismissedBiometricRequirePasswordOnStartCallout: true, + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }); + }); + + it.each(["user-2", "user-3"])( + "should not try to restore values to missing accounts", + async (userId) => { + await sut.rollback(helper); + + expect(helper.set).not.toHaveBeenCalledWith(userId, any()); + }, + ); + }); +}); diff --git a/libs/common/src/state-migrations/migrations/23-move-biometric-prompts-to-state-providers.ts b/libs/common/src/state-migrations/migrations/23-move-biometric-prompts-to-state-providers.ts new file mode 100644 index 0000000000..f95ccaf7ff --- /dev/null +++ b/libs/common/src/state-migrations/migrations/23-move-biometric-prompts-to-state-providers.ts @@ -0,0 +1,99 @@ +import { KeyDefinitionLike, MigrationHelper } from "../migration-helper"; +import { Migrator } from "../migrator"; + +type ExpectedAccountType = { + settings?: { + disableAutoBiometricsPrompt?: boolean; + dismissedBiometricRequirePasswordOnStartCallout?: boolean; + }; +}; + +// prompt cancelled is refreshed on every app start/quit/unlock, so we don't need to migrate it + +export const DISMISSED_BIOMETRIC_REQUIRE_PASSWORD_ON_START_CALLOUT: KeyDefinitionLike = { + key: "dismissedBiometricRequirePasswordOnStartCallout", + stateDefinition: { name: "biometricSettings" }, +}; + +export const PROMPT_AUTOMATICALLY: KeyDefinitionLike = { + key: "promptAutomatically", + stateDefinition: { name: "biometricSettings" }, +}; + +export class MoveBiometricPromptsToStateProviders extends Migrator<22, 23> { + async migrate(helper: MigrationHelper): Promise { + const legacyAccounts = await helper.getAccounts(); + + await Promise.all( + legacyAccounts.map(async ({ userId, account }) => { + if (account == null) { + return; + } + // Move account data + + if (account?.settings?.dismissedBiometricRequirePasswordOnStartCallout != null) { + await helper.setToUser( + userId, + DISMISSED_BIOMETRIC_REQUIRE_PASSWORD_ON_START_CALLOUT, + account.settings.dismissedBiometricRequirePasswordOnStartCallout, + ); + } + + if (account?.settings?.disableAutoBiometricsPrompt != null) { + await helper.setToUser( + userId, + PROMPT_AUTOMATICALLY, + !account.settings.disableAutoBiometricsPrompt, + ); + } + + // Delete old account data + delete account?.settings?.dismissedBiometricRequirePasswordOnStartCallout; + delete account?.settings?.disableAutoBiometricsPrompt; + await helper.set(userId, account); + }), + ); + } + + async rollback(helper: MigrationHelper): Promise { + async function rollbackUser(userId: string, account: ExpectedAccountType) { + let updatedAccount = false; + + const userDismissed = await helper.getFromUser( + userId, + DISMISSED_BIOMETRIC_REQUIRE_PASSWORD_ON_START_CALLOUT, + ); + + if (userDismissed) { + account ??= {}; + account.settings ??= {}; + + updatedAccount = true; + account.settings.dismissedBiometricRequirePasswordOnStartCallout = userDismissed; + await helper.setToUser(userId, DISMISSED_BIOMETRIC_REQUIRE_PASSWORD_ON_START_CALLOUT, null); + } + + const userPromptAutomatically = await helper.getFromUser( + userId, + PROMPT_AUTOMATICALLY, + ); + + if (userPromptAutomatically != null) { + account ??= {}; + account.settings ??= {}; + + updatedAccount = true; + account.settings.disableAutoBiometricsPrompt = !userPromptAutomatically; + await helper.setToUser(userId, PROMPT_AUTOMATICALLY, null); + } + + if (updatedAccount) { + await helper.set(userId, account); + } + } + + const accounts = await helper.getAccounts(); + + await Promise.all(accounts.map(({ userId, account }) => rollbackUser(userId, account))); + } +}