From bc61212969fa5f3a389424b2b2c8f87b882d236b Mon Sep 17 00:00:00 2001 From: Thomas Cauquil Date: Wed, 13 Dec 2023 19:16:22 +0100 Subject: [PATCH] [PM-4090] Fix: Automatic biometric authentication no longer worked (Resolves: #6134) (#6400) * chore: improve FR localization * fix: Automatic biometric authentication no longer worked * chore: add tests for the fixed file * Revert "chore: improve FR localization" This reverts commit 957cbee9b3293fad62e3741ee19556e7aff19f8b. * tests: fixes after resolved conflicts * chore(review): delete fluffy-spoon from tests * chore(review): resolve warnings from tests --- apps/desktop/src/auth/lock.component.spec.ts | 394 +++++++++++++++++++ apps/desktop/src/auth/lock.component.ts | 35 +- 2 files changed, 417 insertions(+), 12 deletions(-) create mode 100644 apps/desktop/src/auth/lock.component.spec.ts diff --git a/apps/desktop/src/auth/lock.component.spec.ts b/apps/desktop/src/auth/lock.component.spec.ts new file mode 100644 index 0000000000..fa94731957 --- /dev/null +++ b/apps/desktop/src/auth/lock.component.spec.ts @@ -0,0 +1,394 @@ +import { NO_ERRORS_SCHEMA } from "@angular/core"; +import { ComponentFixture, TestBed, fakeAsync, tick } from "@angular/core/testing"; +import { ActivatedRoute } from "@angular/router"; +import { MockProxy, mock } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { LockComponent as BaseLockComponent } from "@bitwarden/angular/auth/components/lock.component"; +import { I18nPipe } from "@bitwarden/angular/platform/pipes/i18n.pipe"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service"; +import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; +import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; +import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { DeviceTrustCryptoServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust-crypto.service.abstraction"; +import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; +import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; +import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; +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 { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.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 +const isWindowVisibleMock = jest.fn(); +(global as any).ipc = { + platform: { + biometric: { + enabled: jest.fn(), + }, + isWindowVisible: isWindowVisibleMock, + }, +}; + +describe("LockComponent", () => { + let component: LockComponent; + let fixture: ComponentFixture; + let stateServiceMock: MockProxy; + let messagingServiceMock: MockProxy; + let broadcasterServiceMock: MockProxy; + let platformUtilsServiceMock: MockProxy; + let activatedRouteMock: MockProxy; + + beforeEach(() => { + stateServiceMock = mock(); + stateServiceMock.activeAccount$ = of(null); + + messagingServiceMock = mock(); + broadcasterServiceMock = mock(); + platformUtilsServiceMock = mock(); + + activatedRouteMock = mock(); + activatedRouteMock.queryParams = mock(); + + TestBed.configureTestingModule({ + declarations: [LockComponent, I18nPipe], + providers: [ + { + provide: I18nService, + useValue: mock(), + }, + { + provide: PlatformUtilsService, + useValue: platformUtilsServiceMock, + }, + { + provide: MessagingService, + useValue: messagingServiceMock, + }, + { + provide: CryptoService, + useValue: mock(), + }, + { + provide: VaultTimeoutService, + useValue: mock(), + }, + { + provide: VaultTimeoutSettingsService, + useValue: mock(), + }, + { + provide: EnvironmentService, + useValue: mock(), + }, + { + provide: ElectronStateService, + useValue: stateServiceMock, + }, + { + provide: ApiService, + useValue: mock(), + }, + { + provide: ActivatedRoute, + useValue: activatedRouteMock, + }, + { + provide: BroadcasterService, + useValue: broadcasterServiceMock, + }, + { + provide: PolicyApiServiceAbstraction, + useValue: mock(), + }, + { + provide: InternalPolicyService, + useValue: mock(), + }, + { + provide: PasswordStrengthServiceAbstraction, + useValue: mock(), + }, + { + provide: LogService, + useValue: mock(), + }, + { + provide: DialogService, + useValue: mock(), + }, + { + provide: DeviceTrustCryptoServiceAbstraction, + useValue: mock(), + }, + { + provide: UserVerificationService, + useValue: mock(), + }, + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + }); + + beforeEach(() => { + fixture = TestBed.createComponent(LockComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + jest.clearAllMocks(); + }); + + describe("ngOnInit", () => { + it("should call super.ngOnInit() once", async () => { + const superNgOnInitSpy = jest.spyOn(BaseLockComponent.prototype, "ngOnInit"); + await component.ngOnInit(); + expect(superNgOnInitSpy).toHaveBeenCalledTimes(1); + }); + + it('should set "autoPromptBiometric" to true if "stateService.getDisableAutoBiometricsPrompt()" resolves to false', async () => { + stateServiceMock.getDisableAutoBiometricsPrompt.mockResolvedValue(false); + + 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); + + await component.ngOnInit(); + expect(component["autoPromptBiometric"]).toBe(false); + }); + + it('should set "biometricReady" to true if "stateService.getBiometricReady()" resolves to true', async () => { + component["canUseBiometric"] = jest.fn().mockResolvedValue(true); + + await component.ngOnInit(); + expect(component["biometricReady"]).toBe(true); + }); + + it('should set "biometricReady" to false if "stateService.getBiometricReady()" resolves to false', async () => { + component["canUseBiometric"] = jest.fn().mockResolvedValue(false); + + await component.ngOnInit(); + expect(component["biometricReady"]).toBe(false); + }); + + it("should call displayBiometricUpdateWarning", async () => { + component["displayBiometricUpdateWarning"] = jest.fn(); + await component.ngOnInit(); + expect(component["displayBiometricUpdateWarning"]).toHaveBeenCalledTimes(1); + }); + + it("should call delayedAskForBiometric", async () => { + component["delayedAskForBiometric"] = jest.fn(); + await component.ngOnInit(); + expect(component["delayedAskForBiometric"]).toHaveBeenCalledTimes(1); + expect(component["delayedAskForBiometric"]).toHaveBeenCalledWith(500); + }); + + it("should call delayedAskForBiometric when queryParams change", async () => { + activatedRouteMock.queryParams = of({ promptBiometric: true }); + component["delayedAskForBiometric"] = jest.fn(); + await component.ngOnInit(); + + expect(component["delayedAskForBiometric"]).toHaveBeenCalledTimes(1); + expect(component["delayedAskForBiometric"]).toHaveBeenCalledWith(500); + }); + + it("should call messagingService.send", async () => { + await component.ngOnInit(); + expect(messagingServiceMock.send).toHaveBeenCalledWith("getWindowIsFocused"); + }); + + describe("broadcasterService.subscribe", () => { + it('should call onWindowHidden() when "broadcasterService.subscribe" is called with "windowHidden"', async () => { + component["onWindowHidden"] = jest.fn(); + await component.ngOnInit(); + broadcasterServiceMock.subscribe.mock.calls[0][1]({ command: "windowHidden" }); + expect(component["onWindowHidden"]).toHaveBeenCalledTimes(1); + }); + + it('should call focusInput() when "broadcasterService.subscribe" is called with "windowIsFocused" is true and deferFocus is false', async () => { + component["focusInput"] = jest.fn(); + component["deferFocus"] = null; + await component.ngOnInit(); + broadcasterServiceMock.subscribe.mock.calls[0][1]({ + command: "windowIsFocused", + windowIsFocused: true, + } as any); + expect(component["deferFocus"]).toBe(false); + expect(component["focusInput"]).toHaveBeenCalledTimes(1); + }); + + it('should not call focusInput() when "broadcasterService.subscribe" is called with "windowIsFocused" is true and deferFocus is true', async () => { + component["focusInput"] = jest.fn(); + component["deferFocus"] = null; + await component.ngOnInit(); + broadcasterServiceMock.subscribe.mock.calls[0][1]({ + command: "windowIsFocused", + windowIsFocused: false, + } as any); + expect(component["deferFocus"]).toBe(true); + expect(component["focusInput"]).toHaveBeenCalledTimes(0); + }); + + it('should call focusInput() when "broadcasterService.subscribe" is called with "windowIsFocused" is true and deferFocus is true', async () => { + component["focusInput"] = jest.fn(); + component["deferFocus"] = true; + await component.ngOnInit(); + broadcasterServiceMock.subscribe.mock.calls[0][1]({ + command: "windowIsFocused", + windowIsFocused: true, + } as any); + expect(component["deferFocus"]).toBe(false); + expect(component["focusInput"]).toHaveBeenCalledTimes(1); + }); + + it('should not call focusInput() when "broadcasterService.subscribe" is called with "windowIsFocused" is false and deferFocus is true', async () => { + component["focusInput"] = jest.fn(); + component["deferFocus"] = true; + await component.ngOnInit(); + broadcasterServiceMock.subscribe.mock.calls[0][1]({ + command: "windowIsFocused", + windowIsFocused: false, + } as any); + expect(component["deferFocus"]).toBe(true); + expect(component["focusInput"]).toHaveBeenCalledTimes(0); + }); + }); + }); + + describe("ngOnDestroy", () => { + it("should call super.ngOnDestroy()", () => { + const superNgOnDestroySpy = jest.spyOn(BaseLockComponent.prototype, "ngOnDestroy"); + component.ngOnDestroy(); + expect(superNgOnDestroySpy).toHaveBeenCalledTimes(1); + }); + + it("should call broadcasterService.unsubscribe()", () => { + component.ngOnDestroy(); + expect(broadcasterServiceMock.unsubscribe).toHaveBeenCalledTimes(1); + }); + }); + + describe("focusInput", () => { + it('should call "focus" on #pin input if pinEnabled is true', () => { + component["pinEnabled"] = true; + global.document.getElementById = jest.fn().mockReturnValue({ focus: jest.fn() }); + component["focusInput"](); + expect(global.document.getElementById).toHaveBeenCalledWith("pin"); + }); + + it('should call "focus" on #masterPassword input if pinEnabled is false', () => { + component["pinEnabled"] = false; + global.document.getElementById = jest.fn().mockReturnValue({ focus: jest.fn() }); + component["focusInput"](); + expect(global.document.getElementById).toHaveBeenCalledWith("masterPassword"); + }); + }); + + describe("delayedAskForBiometric", () => { + beforeEach(() => { + component["supportsBiometric"] = true; + component["autoPromptBiometric"] = true; + }); + + it('should wait for "delay" milliseconds', fakeAsync(async () => { + const delaySpy = jest.spyOn(global, "setTimeout"); + component["delayedAskForBiometric"](5000); + + tick(4000); + component["biometricAsked"] = false; + + tick(1000); + component["biometricAsked"] = true; + + expect(delaySpy).toHaveBeenCalledWith(expect.any(Function), 5000); + })); + + it('should return; if "params" is defined and "params.promptBiometric" is false', fakeAsync(async () => { + component["delayedAskForBiometric"](5000, { promptBiometric: false }); + tick(5000); + expect(component["biometricAsked"]).toBe(false); + })); + + it('should not return; if "params" is defined and "params.promptBiometric" is true', fakeAsync(async () => { + component["delayedAskForBiometric"](5000, { promptBiometric: true }); + tick(5000); + expect(component["biometricAsked"]).toBe(true); + })); + + it('should not return; if "params" is undefined', fakeAsync(async () => { + component["delayedAskForBiometric"](5000); + tick(5000); + expect(component["biometricAsked"]).toBe(true); + })); + + it('should return; if "supportsBiometric" is false', fakeAsync(async () => { + component["supportsBiometric"] = false; + component["delayedAskForBiometric"](5000); + tick(5000); + expect(component["biometricAsked"]).toBe(false); + })); + + it('should return; if "autoPromptBiometric" is false', fakeAsync(async () => { + component["autoPromptBiometric"] = false; + component["delayedAskForBiometric"](5000); + tick(5000); + expect(component["biometricAsked"]).toBe(false); + })); + + it("should call unlockBiometric() if biometricAsked is false and window is visible", fakeAsync(async () => { + isWindowVisibleMock.mockResolvedValue(true); + component["unlockBiometric"] = jest.fn(); + component["biometricAsked"] = false; + component["delayedAskForBiometric"](5000); + tick(5000); + + expect(component["unlockBiometric"]).toHaveBeenCalledTimes(1); + })); + + it("should not call unlockBiometric() if biometricAsked is false and window is not visible", fakeAsync(async () => { + isWindowVisibleMock.mockResolvedValue(false); + component["unlockBiometric"] = jest.fn(); + component["biometricAsked"] = false; + component["delayedAskForBiometric"](5000); + tick(5000); + + expect(component["unlockBiometric"]).toHaveBeenCalledTimes(0); + })); + + it("should not call unlockBiometric() if biometricAsked is true", fakeAsync(async () => { + isWindowVisibleMock.mockResolvedValue(true); + component["unlockBiometric"] = jest.fn(); + component["biometricAsked"] = true; + + component["delayedAskForBiometric"](5000); + tick(5000); + + expect(component["unlockBiometric"]).toHaveBeenCalledTimes(0); + })); + }); + + describe("canUseBiometric", () => { + it("should call getUserId() on stateService", async () => { + stateServiceMock.getUserId.mockResolvedValue("userId"); + await component["canUseBiometric"](); + + expect(ipc.platform.biometric.enabled).toHaveBeenCalledWith("userId"); + }); + }); + + it('onWindowHidden() should set "showPassword" to false', () => { + component["showPassword"] = true; + component["onWindowHidden"](); + expect(component["showPassword"]).toBe(false); + }); +}); diff --git a/apps/desktop/src/auth/lock.component.ts b/apps/desktop/src/auth/lock.component.ts index 91a60557fb..3f62df7dd1 100644 --- a/apps/desktop/src/auth/lock.component.ts +++ b/apps/desktop/src/auth/lock.component.ts @@ -1,5 +1,6 @@ import { Component, NgZone } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; +import { switchMap } from "rxjs"; import { LockComponent as BaseLockComponent } from "@bitwarden/angular/auth/components/lock.component"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; @@ -31,6 +32,8 @@ const BroadcasterSubscriptionId = "LockComponent"; export class LockComponent extends BaseLockComponent { private deferFocus: boolean = null; protected biometricReady = false; + private biometricAsked = false; + private autoPromptBiometric = false; constructor( router: Router, @@ -78,23 +81,14 @@ export class LockComponent extends BaseLockComponent { async ngOnInit() { await super.ngOnInit(); - const autoPromptBiometric = !(await this.stateService.getDisableAutoBiometricsPrompt()); + this.autoPromptBiometric = !(await this.stateService.getDisableAutoBiometricsPrompt()); this.biometricReady = await this.canUseBiometric(); await this.displayBiometricUpdateWarning(); - // eslint-disable-next-line rxjs-angular/prefer-takeuntil - this.route.queryParams.subscribe((params) => { - setTimeout(async () => { - if (!params.promptBiometric || !this.supportsBiometric || !autoPromptBiometric) { - return; - } + this.delayedAskForBiometric(500); + this.route.queryParams.pipe(switchMap((params) => this.delayedAskForBiometric(500, params))); - if (await ipc.platform.isWindowVisible()) { - this.unlockBiometric(); - } - }, 1000); - }); this.broadcasterService.subscribe(BroadcasterSubscriptionId, async (message: any) => { this.ngZone.run(() => { switch (message.command) { @@ -128,6 +122,23 @@ export class LockComponent extends BaseLockComponent { this.showPassword = false; } + private async delayedAskForBiometric(delay: number, params?: any) { + await new Promise((resolve) => setTimeout(resolve, delay)); + + if (params && !params.promptBiometric) { + return; + } + + if (!this.supportsBiometric || !this.autoPromptBiometric || this.biometricAsked) { + return; + } + + this.biometricAsked = true; + if (await ipc.platform.isWindowVisible()) { + this.unlockBiometric(); + } + } + private async canUseBiometric() { const userId = await this.stateService.getUserId(); return await ipc.platform.biometric.enabled(userId);