mirror of
https://github.com/bitwarden/browser
synced 2026-01-21 11:53:34 +00:00
[PM-29928] Fix biometrics status check when native messaging permission is missing (#18154)
* Dont check biometrics status when nativeMessaging permission isn't granted * Increase polling interval and add unit tests
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import { Component } from "@angular/core";
|
||||
import { ComponentFixture, TestBed } from "@angular/core/testing";
|
||||
import { ComponentFixture, fakeAsync, TestBed, tick } from "@angular/core/testing";
|
||||
import { By } from "@angular/platform-browser";
|
||||
import { ActivatedRoute } from "@angular/router";
|
||||
import { mock } from "jest-mock-extended";
|
||||
@@ -37,7 +37,12 @@ import { UserId } from "@bitwarden/common/types/guid";
|
||||
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
|
||||
import { DialogService, ToastService } from "@bitwarden/components";
|
||||
import { newGuid } from "@bitwarden/guid";
|
||||
import { BiometricStateService, BiometricsService, KeyService } from "@bitwarden/key-management";
|
||||
import {
|
||||
BiometricStateService,
|
||||
BiometricsService,
|
||||
BiometricsStatus,
|
||||
KeyService,
|
||||
} from "@bitwarden/key-management";
|
||||
|
||||
import { BrowserApi } from "../../../platform/browser/browser-api";
|
||||
import BrowserPopupUtils from "../../../platform/browser/browser-popup-utils";
|
||||
@@ -64,6 +69,7 @@ describe("AccountSecurityComponent", () => {
|
||||
const apiService = mock<ApiService>();
|
||||
const billingService = mock<BillingAccountProfileStateService>();
|
||||
const biometricStateService = mock<BiometricStateService>();
|
||||
const biometricsService = mock<BiometricsService>();
|
||||
const configService = mock<ConfigService>();
|
||||
const dialogService = mock<DialogService>();
|
||||
const keyService = mock<KeyService>();
|
||||
@@ -75,6 +81,7 @@ describe("AccountSecurityComponent", () => {
|
||||
const validationService = mock<ValidationService>();
|
||||
const vaultNudgesService = mock<NudgesService>();
|
||||
const vaultTimeoutSettingsService = mock<VaultTimeoutSettingsService>();
|
||||
const mockI18nService = mock<I18nService>();
|
||||
|
||||
// Mock subjects to control the phishing detection observables
|
||||
let phishingAvailableSubject: BehaviorSubject<boolean>;
|
||||
@@ -91,14 +98,14 @@ describe("AccountSecurityComponent", () => {
|
||||
provide: BillingAccountProfileStateService,
|
||||
useValue: billingService,
|
||||
},
|
||||
{ provide: BiometricsService, useValue: mock<BiometricsService>() },
|
||||
{ provide: BiometricsService, useValue: biometricsService },
|
||||
{ provide: BiometricStateService, useValue: biometricStateService },
|
||||
{ provide: CipherService, useValue: mock<CipherService>() },
|
||||
{ provide: CollectionService, useValue: mock<CollectionService>() },
|
||||
{ provide: ConfigService, useValue: configService },
|
||||
{ provide: DialogService, useValue: dialogService },
|
||||
{ provide: EnvironmentService, useValue: mock<EnvironmentService>() },
|
||||
{ provide: I18nService, useValue: mock<I18nService>() },
|
||||
{ provide: I18nService, useValue: mockI18nService },
|
||||
{ provide: KeyService, useValue: keyService },
|
||||
{ provide: LockService, useValue: lockService },
|
||||
{ provide: LogService, useValue: mock<LogService>() },
|
||||
@@ -153,6 +160,7 @@ describe("AccountSecurityComponent", () => {
|
||||
pinServiceAbstraction.isPinSet.mockResolvedValue(false);
|
||||
configService.getFeatureFlag$.mockReturnValue(of(false));
|
||||
billingService.hasPremiumPersonally$.mockReturnValue(of(true));
|
||||
mockI18nService.t.mockImplementation((key) => `${key}-used-i18n`);
|
||||
|
||||
policyService.policiesByType$.mockReturnValue(of([null]));
|
||||
|
||||
@@ -459,4 +467,118 @@ describe("AccountSecurityComponent", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("biometrics polling timer", () => {
|
||||
let browserApiSpy: jest.SpyInstance;
|
||||
|
||||
beforeEach(() => {
|
||||
browserApiSpy = jest.spyOn(BrowserApi, "permissionsGranted");
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
component.ngOnDestroy();
|
||||
});
|
||||
|
||||
it("disables biometric control when canEnableBiometricUnlock is false", fakeAsync(async () => {
|
||||
biometricsService.canEnableBiometricUnlock.mockResolvedValue(false);
|
||||
|
||||
await component.ngOnInit();
|
||||
tick();
|
||||
|
||||
expect(component.form.controls.biometric.disabled).toBe(true);
|
||||
}));
|
||||
|
||||
it("enables biometric control when canEnableBiometricUnlock is true", fakeAsync(async () => {
|
||||
biometricsService.canEnableBiometricUnlock.mockResolvedValue(true);
|
||||
|
||||
await component.ngOnInit();
|
||||
tick();
|
||||
|
||||
expect(component.form.controls.biometric.disabled).toBe(false);
|
||||
}));
|
||||
|
||||
it("skips status check when nativeMessaging permission is not granted and not Safari", fakeAsync(async () => {
|
||||
biometricsService.canEnableBiometricUnlock.mockResolvedValue(true);
|
||||
browserApiSpy.mockResolvedValue(false);
|
||||
platformUtilsService.isSafari.mockReturnValue(false);
|
||||
|
||||
await component.ngOnInit();
|
||||
tick();
|
||||
|
||||
expect(biometricsService.getBiometricsStatusForUser).not.toHaveBeenCalled();
|
||||
expect(component.biometricUnavailabilityReason).toBeUndefined();
|
||||
}));
|
||||
|
||||
it("checks biometrics status when nativeMessaging permission is granted", fakeAsync(async () => {
|
||||
biometricsService.canEnableBiometricUnlock.mockResolvedValue(true);
|
||||
browserApiSpy.mockResolvedValue(true);
|
||||
platformUtilsService.isSafari.mockReturnValue(false);
|
||||
biometricsService.getBiometricsStatusForUser.mockResolvedValue(
|
||||
BiometricsStatus.DesktopDisconnected,
|
||||
);
|
||||
|
||||
await component.ngOnInit();
|
||||
tick();
|
||||
|
||||
expect(biometricsService.getBiometricsStatusForUser).toHaveBeenCalledWith(mockUserId);
|
||||
}));
|
||||
|
||||
it("should check status on Safari", fakeAsync(async () => {
|
||||
biometricsService.canEnableBiometricUnlock.mockResolvedValue(true);
|
||||
browserApiSpy.mockResolvedValue(false);
|
||||
platformUtilsService.isSafari.mockReturnValue(true);
|
||||
biometricsService.getBiometricsStatusForUser.mockResolvedValue(
|
||||
BiometricsStatus.DesktopDisconnected,
|
||||
);
|
||||
|
||||
await component.ngOnInit();
|
||||
tick();
|
||||
|
||||
expect(biometricsService.getBiometricsStatusForUser).toHaveBeenCalledWith(mockUserId);
|
||||
}));
|
||||
|
||||
test.each([
|
||||
[
|
||||
BiometricsStatus.DesktopDisconnected,
|
||||
"biometricsStatusHelptextDesktopDisconnected-used-i18n",
|
||||
],
|
||||
[
|
||||
BiometricsStatus.NotEnabledInConnectedDesktopApp,
|
||||
"biometricsStatusHelptextNotEnabledInDesktop-used-i18n",
|
||||
],
|
||||
[
|
||||
BiometricsStatus.HardwareUnavailable,
|
||||
"biometricsStatusHelptextHardwareUnavailable-used-i18n",
|
||||
],
|
||||
])(
|
||||
"sets expected unavailability reason for %s status when biometric not available",
|
||||
fakeAsync(async (biometricStatus: BiometricsStatus, expected: string) => {
|
||||
biometricsService.canEnableBiometricUnlock.mockResolvedValue(false);
|
||||
browserApiSpy.mockResolvedValue(true);
|
||||
platformUtilsService.isSafari.mockReturnValue(false);
|
||||
biometricsService.getBiometricsStatusForUser.mockResolvedValue(biometricStatus);
|
||||
|
||||
await component.ngOnInit();
|
||||
tick();
|
||||
|
||||
expect(component.biometricUnavailabilityReason).toBe(expected);
|
||||
}),
|
||||
);
|
||||
|
||||
it("should not set unavailability reason for error statuses when biometric is available", fakeAsync(async () => {
|
||||
biometricsService.canEnableBiometricUnlock.mockResolvedValue(true);
|
||||
browserApiSpy.mockResolvedValue(true);
|
||||
platformUtilsService.isSafari.mockReturnValue(false);
|
||||
biometricsService.getBiometricsStatusForUser.mockResolvedValue(
|
||||
BiometricsStatus.DesktopDisconnected,
|
||||
);
|
||||
|
||||
await component.ngOnInit();
|
||||
tick();
|
||||
|
||||
// Status is DesktopDisconnected but biometric IS available, so don't show error
|
||||
expect(component.biometricUnavailabilityReason).toBe("");
|
||||
component.ngOnDestroy();
|
||||
}));
|
||||
});
|
||||
});
|
||||
|
||||
@@ -149,6 +149,7 @@ export class AccountSecurityComponent implements OnInit, OnDestroy {
|
||||
|
||||
protected refreshTimeoutSettings$ = new BehaviorSubject<void>(undefined);
|
||||
private destroy$ = new Subject<void>();
|
||||
private readonly BIOMETRICS_POLLING_INTERVAL = 2000;
|
||||
|
||||
constructor(
|
||||
private accountService: AccountService,
|
||||
@@ -264,10 +265,9 @@ export class AccountSecurityComponent implements OnInit, OnDestroy {
|
||||
};
|
||||
this.form.patchValue(initialValues, { emitEvent: false });
|
||||
|
||||
timer(0, 1000)
|
||||
timer(0, this.BIOMETRICS_POLLING_INTERVAL)
|
||||
.pipe(
|
||||
switchMap(async () => {
|
||||
const status = await this.biometricsService.getBiometricsStatusForUser(activeAccount.id);
|
||||
const biometricSettingAvailable = await this.biometricsService.canEnableBiometricUnlock();
|
||||
if (!biometricSettingAvailable) {
|
||||
this.form.controls.biometric.disable({ emitEvent: false });
|
||||
@@ -275,6 +275,15 @@ export class AccountSecurityComponent implements OnInit, OnDestroy {
|
||||
this.form.controls.biometric.enable({ emitEvent: false });
|
||||
}
|
||||
|
||||
// Biometrics status shouldn't be checked if permissions are needed.
|
||||
const needsPermissionPrompt =
|
||||
!(await BrowserApi.permissionsGranted(["nativeMessaging"])) &&
|
||||
!this.platformUtilsService.isSafari();
|
||||
if (needsPermissionPrompt) {
|
||||
return;
|
||||
}
|
||||
|
||||
const status = await this.biometricsService.getBiometricsStatusForUser(activeAccount.id);
|
||||
if (status === BiometricsStatus.DesktopDisconnected && !biometricSettingAvailable) {
|
||||
this.biometricUnavailabilityReason = this.i18nService.t(
|
||||
"biometricsStatusHelptextDesktopDisconnected",
|
||||
|
||||
Reference in New Issue
Block a user