[PM-18038] Fix safari using outdated biometrics protocol (#13287)

* Fix safari using outdated biometrics protocol * Remove logging * Remove log * Move canEnableBiometricUnlock to biometric service * Fix build * Add tests * Fix type error * Attempt to fix build * Fix build * Fix test failure
2025-12-11 22:03:36 +00:00 · 2025-03-31 16:59:01 +02:00
parent 22039d038d
commit 15738f16ae
19 changed files with 264 additions and 39 deletions
--- a/apps/browser/src/auth/popup/settings/account-security.component.ts
+++ b/apps/browser/src/auth/popup/settings/account-security.component.ts
@@ -233,11 +233,7 @@ export class AccountSecurityComponent implements OnInit, OnDestroy {
      .pipe(
        switchMap(async () => {
          const status = await this.biometricsService.getBiometricsStatusForUser(activeAccount.id);
-          const biometricSettingAvailable =
-            !(await BrowserApi.permissionsGranted(["nativeMessaging"])) ||
-            (status !== BiometricsStatus.DesktopDisconnected &&
-              status !== BiometricsStatus.NotEnabledInConnectedDesktopApp) ||
-            (await this.vaultTimeoutSettingsService.isBiometricLockSet());
+          const biometricSettingAvailable = await this.biometricsService.canEnableBiometricUnlock();
          if (!biometricSettingAvailable) {
            this.form.controls.biometric.disable({ emitEvent: false });
          } else {
@@ -256,6 +252,13 @@ export class AccountSecurityComponent implements OnInit, OnDestroy {
              "biometricsStatusHelptextNotEnabledInDesktop",
              activeAccount.email,
            );
+          } else if (
+            status === BiometricsStatus.HardwareUnavailable &&
+            !biometricSettingAvailable
+          ) {
+            this.biometricUnavailabilityReason = this.i18nService.t(
+              "biometricsStatusHelptextHardwareUnavailable",
+            );
          } else {
            this.biometricUnavailabilityReason = "";
          }
--- a/apps/browser/src/background/main.background.ts
+++ b/apps/browser/src/background/main.background.ts
@@ -672,14 +672,6 @@ export default class MainBackground {
      this.kdfConfigService,
    );

-    this.biometricsService = new BackgroundBrowserBiometricsService(
-      runtimeNativeMessagingBackground,
-      this.logService,
-      this.keyService,
-      this.biometricStateService,
-      this.messagingService,
-    );
-
    this.appIdService = new AppIdService(this.storageService, this.logService);

    this.userDecryptionOptionsService = new UserDecryptionOptionsService(this.stateProvider);
@@ -699,6 +691,15 @@ export default class MainBackground {
      VaultTimeoutStringType.OnRestart, // default vault timeout
    );

+    this.biometricsService = new BackgroundBrowserBiometricsService(
+      runtimeNativeMessagingBackground,
+      this.logService,
+      this.keyService,
+      this.biometricStateService,
+      this.messagingService,
+      this.vaultTimeoutSettingsService,
+    );
+
    this.apiService = new ApiService(
      this.tokenService,
      this.platformUtilsService,
--- a/apps/browser/src/background/nativeMessaging.background.ts
+++ b/apps/browser/src/background/nativeMessaging.background.ts
@@ -120,9 +120,15 @@ export class NativeMessagingBackground {
      this.connecting = true;

      const connectedCallback = () => {
-        this.logService.info(
-          "[Native Messaging IPC] Connection to Bitwarden Desktop app established!",
-        );
+        if (!this.platformUtilsService.isSafari()) {
+          this.logService.info(
+            "[Native Messaging IPC] Connection to Bitwarden Desktop app established!",
+          );
+        } else {
+          this.logService.info(
+            "[Native Messaging IPC] Connection to Safari swift module established!",
+          );
+        }
        this.connected = true;
        this.connecting = false;
        resolve();
@@ -131,6 +137,7 @@ export class NativeMessagingBackground {
      // Safari has a bundled native component which is always available, no need to
      // check if the desktop app is running.
      if (this.platformUtilsService.isSafari()) {
+        this.isConnectedToOutdatedDesktopClient = false;
        connectedCallback();
      }

@@ -428,7 +435,9 @@ export class NativeMessagingBackground {
    }

    if (this.callbacks.has(messageId)) {
-      this.callbacks.get(messageId)!.resolver(message);
+      const callback = this.callbacks!.get(messageId)!;
+      this.callbacks.delete(messageId);
+      callback.resolver(message);
    } else {
      this.logService.info("[Native Messaging IPC] Received message without a callback", message);
    }
--- a/apps/browser/src/background/runtime.background.ts
+++ b/apps/browser/src/background/runtime.background.ts
@@ -78,6 +78,7 @@ export default class RuntimeBackground {
        BiometricsCommands.GetBiometricsStatus,
        BiometricsCommands.UnlockWithBiometricsForUser,
        BiometricsCommands.GetBiometricsStatusForUser,
+        BiometricsCommands.CanEnableBiometricUnlock,
        "getUseTreeWalkerApiForPageDetailsCollectionFeatureFlag",
        "getInlineMenuFieldQualificationFeatureFlag",
        "getUserPremiumStatus",
@@ -201,6 +202,9 @@ export default class RuntimeBackground {
      case BiometricsCommands.GetBiometricsStatusForUser: {
        return await this.main.biometricsService.getBiometricsStatusForUser(msg.userId);
      }
+      case BiometricsCommands.CanEnableBiometricUnlock: {
+        return await this.main.biometricsService.canEnableBiometricUnlock();
+      }
      case "getUseTreeWalkerApiForPageDetailsCollectionFeatureFlag": {
        return await this.configService.getFeatureFlag(
          FeatureFlag.UseTreeWalkerApiForPageDetailsCollection,
--- a/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.spec.ts
+++ b/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.spec.ts
@@ -0,0 +1,61 @@
+import { mock } from "jest-mock-extended";
+
+import { VaultTimeoutSettingsService } from "@bitwarden/common/key-management/vault-timeout";
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
+import { KeyService, BiometricStateService, BiometricsStatus } from "@bitwarden/key-management";
+
+import { NativeMessagingBackground } from "../../background/nativeMessaging.background";
+
+import { BackgroundBrowserBiometricsService } from "./background-browser-biometrics.service";
+
+describe("background browser biometrics service tests", function () {
+  let service: BackgroundBrowserBiometricsService;
+
+  const nativeMessagingBackground = mock<NativeMessagingBackground>();
+  const logService = mock<LogService>();
+  const keyService = mock<KeyService>();
+  const biometricStateService = mock<BiometricStateService>();
+  const messagingService = mock<MessagingService>();
+  const vaultTimeoutSettingsService = mock<VaultTimeoutSettingsService>();
+
+  beforeEach(() => {
+    jest.resetAllMocks();
+    service = new BackgroundBrowserBiometricsService(
+      () => nativeMessagingBackground,
+      logService,
+      keyService,
+      biometricStateService,
+      messagingService,
+      vaultTimeoutSettingsService,
+    );
+  });
+
+  describe("canEnableBiometricUnlock", () => {
+    const table: [BiometricsStatus, boolean, boolean][] = [
+      // status, already enabled, expected
+
+      // if the setting is not already on, it should only be possible to enable it if biometrics are available
+      [BiometricsStatus.Available, false, true],
+      [BiometricsStatus.HardwareUnavailable, false, false],
+      [BiometricsStatus.NotEnabledInConnectedDesktopApp, false, false],
+      [BiometricsStatus.DesktopDisconnected, false, false],
+
+      // if the setting is already on, it should always be possible to disable it
+      [BiometricsStatus.Available, true, true],
+      [BiometricsStatus.HardwareUnavailable, true, true],
+      [BiometricsStatus.NotEnabledInConnectedDesktopApp, true, true],
+      [BiometricsStatus.DesktopDisconnected, true, true],
+    ];
+    test.each(table)(
+      "status: %s, already enabled: %s, expected: %s",
+      async (status, alreadyEnabled, expected) => {
+        service.getBiometricsStatus = jest.fn().mockResolvedValue(status);
+        vaultTimeoutSettingsService.isBiometricLockSet.mockResolvedValue(alreadyEnabled);
+        const result = await service.canEnableBiometricUnlock();
+
+        expect(result).toBe(expected);
+      },
+    );
+  });
+});
--- a/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts
+++ b/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts
@@ -1,5 +1,6 @@
 import { Injectable } from "@angular/core";

+import { VaultTimeoutSettingsService } from "@bitwarden/common/key-management/vault-timeout";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
@@ -25,6 +26,7 @@ export class BackgroundBrowserBiometricsService extends BiometricsService {
    private keyService: KeyService,
    private biometricStateService: BiometricStateService,
    private messagingService: MessagingService,
+    private vaultTimeoutSettingsService: VaultTimeoutSettingsService,
  ) {
    super();
  }
@@ -169,4 +171,14 @@ export class BackgroundBrowserBiometricsService extends BiometricsService {
  }

  async setShouldAutopromptNow(value: boolean): Promise<void> {}
+  async canEnableBiometricUnlock(): Promise<boolean> {
+    const status = await this.getBiometricsStatus();
+    const isBiometricsAlreadyEnabled = await this.vaultTimeoutSettingsService.isBiometricLockSet();
+    const statusAllowsBiometric =
+      status !== BiometricsStatus.DesktopDisconnected &&
+      status !== BiometricsStatus.NotEnabledInConnectedDesktopApp &&
+      status !== BiometricsStatus.HardwareUnavailable;
+
+    return statusAllowsBiometric || isBiometricsAlreadyEnabled;
+  }
 }
--- a/apps/browser/src/key-management/biometrics/foreground-browser-biometrics.spec.ts
+++ b/apps/browser/src/key-management/biometrics/foreground-browser-biometrics.spec.ts
@@ -0,0 +1,60 @@
+import { mock } from "jest-mock-extended";
+
+import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
+
+import { BrowserApi } from "../../platform/browser/browser-api";
+
+import { ForegroundBrowserBiometricsService } from "./foreground-browser-biometrics";
+
+jest.mock("../../platform/browser/browser-api", () => ({
+  BrowserApi: {
+    sendMessageWithResponse: jest.fn(),
+    permissionsGranted: jest.fn(),
+  },
+}));
+
+describe("foreground browser biometrics service tests", function () {
+  const platformUtilsService = mock<PlatformUtilsService>();
+
+  beforeEach(() => {
+    jest.resetAllMocks();
+  });
+
+  describe("canEnableBiometricUnlock", () => {
+    const table: [boolean, boolean, boolean, boolean][] = [
+      // canEnableBiometricUnlock from background, native permission granted, isSafari, expected
+
+      // needs permission prompt; always allowed
+      [true, false, false, true],
+      [false, false, false, true],
+
+      // is safari; depends on the status that the background service reports
+      [false, false, true, false],
+      [true, false, true, true],
+
+      // native permissions granted; depends on the status that the background service reports
+      [false, true, false, false],
+      [true, true, false, true],
+
+      // should never happen since safari does not use the permissions
+      [false, true, true, false],
+      [true, true, true, true],
+    ];
+    test.each(table)(
+      "canEnableBiometric: %s, native permission granted: %s, isSafari: %s, expected: %s",
+      async (canEnableBiometricUnlockBackground, granted, isSafari, expected) => {
+        const service = new ForegroundBrowserBiometricsService(platformUtilsService);
+
+        (BrowserApi.permissionsGranted as jest.Mock).mockResolvedValue(granted);
+        (BrowserApi.sendMessageWithResponse as jest.Mock).mockResolvedValue({
+          result: canEnableBiometricUnlockBackground,
+        });
+        platformUtilsService.isSafari.mockReturnValue(isSafari);
+
+        const result = await service.canEnableBiometricUnlock();
+
+        expect(result).toBe(expected);
+      },
+    );
+  });
+});
--- a/apps/browser/src/key-management/biometrics/foreground-browser-biometrics.ts
+++ b/apps/browser/src/key-management/biometrics/foreground-browser-biometrics.ts
@@ -1,3 +1,4 @@
+import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
 import { UserId } from "@bitwarden/common/types/guid";
 import { UserKey } from "@bitwarden/common/types/key";
@@ -8,6 +9,10 @@ import { BrowserApi } from "../../platform/browser/browser-api";
 export class ForegroundBrowserBiometricsService extends BiometricsService {
  shouldAutopromptNow = true;

+  constructor(private platformUtilsService: PlatformUtilsService) {
+    super();
+  }
+
  async authenticateWithBiometrics(): Promise<boolean> {
    const response = await BrowserApi.sendMessageWithResponse<{
      result: boolean;
@@ -52,4 +57,19 @@ export class ForegroundBrowserBiometricsService extends BiometricsService {
  async setShouldAutopromptNow(value: boolean): Promise<void> {
    this.shouldAutopromptNow = value;
  }
+
+  async canEnableBiometricUnlock(): Promise<boolean> {
+    const needsPermissionPrompt =
+      !(await BrowserApi.permissionsGranted(["nativeMessaging"])) &&
+      !this.platformUtilsService.isSafari();
+    return (
+      needsPermissionPrompt ||
+      (
+        await BrowserApi.sendMessageWithResponse<{
+          result: boolean;
+          error: string;
+        }>(BiometricsCommands.CanEnableBiometricUnlock)
+      ).result
+    );
+  }
 }
--- a/apps/browser/src/popup/services/services.module.ts
+++ b/apps/browser/src/popup/services/services.module.ts
@@ -316,10 +316,8 @@ const safeProviders: SafeProvider[] = [
  }),
  safeProvider({
    provide: BiometricsService,
-    useFactory: () => {
-      return new ForegroundBrowserBiometricsService();
-    },
-    deps: [],
+    useClass: ForegroundBrowserBiometricsService,
+    deps: [PlatformUtilsService],
  }),
  safeProvider({
    provide: SyncService,
--- a/apps/browser/src/safari/safari/SafariWebExtensionHandler.swift
+++ b/apps/browser/src/safari/safari/SafariWebExtensionHandler.swift
@@ -152,7 +152,7 @@ class SafariWebExtensionHandler: NSObject, NSExtensionRequestHandling {
                response.userInfo = [
                    SFExtensionMessageKey: [
                        "message": [
-                            "command": "biometricUnlock",
+                            "command": "unlockWithBiometricsForUser",
                            "response": false,
                            "timestamp": Int64(NSDate().timeIntervalSince1970 * 1000),
                            "messageId": messageId,
@@ -177,7 +177,7 @@ class SafariWebExtensionHandler: NSObject, NSExtensionRequestHandling {
                response.userInfo = [
                    SFExtensionMessageKey: [
                        "message": [
-                            "command": "biometricUnlock",
+                            "command": "unlockWithBiometricsForUser",
                            "response": false,
                            "timestamp": Int64(NSDate().timeIntervalSince1970 * 1000),
                            "messageId": messageId,
@@ -209,7 +209,7 @@ class SafariWebExtensionHandler: NSObject, NSExtensionRequestHandling {

                        response.userInfo = [ SFExtensionMessageKey: [
                            "message": [
-                                "command": "biometricUnlock",
+                                "command": "unlockWithBiometricsForUser",
                                "response": true,
                                "timestamp": Int64(NSDate().timeIntervalSince1970 * 1000),
                                "userKeyB64": result!.replacingOccurrences(of: "\"", with: ""),
@@ -220,7 +220,7 @@ class SafariWebExtensionHandler: NSObject, NSExtensionRequestHandling {
                        response.userInfo = [
                            SFExtensionMessageKey: [
                                "message": [
-                                    "command": "biometricUnlock",
+                                    "command": "unlockWithBiometricsForUser",
                                    "response": true,
                                    "timestamp": Int64(NSDate().timeIntervalSince1970 * 1000),
                                    "messageId": messageId,
--- a/apps/cli/src/key-management/cli-biometrics-service.ts
+++ b/apps/cli/src/key-management/cli-biometrics-service.ts
@@ -24,4 +24,7 @@ export class CliBiometricsService extends BiometricsService {
  }

  async setShouldAutopromptNow(value: boolean): Promise<void> {}
+  async canEnableBiometricUnlock(): Promise<boolean> {
+    return false;
+  }
 }
--- a/apps/desktop/src/app/accounts/settings.component.spec.ts
+++ b/apps/desktop/src/app/accounts/settings.component.spec.ts
@@ -248,6 +248,7 @@ describe("SettingsComponent", () => {
  describe("biometrics enabled", () => {
    beforeEach(() => {
      desktopBiometricsService.getBiometricsStatus.mockResolvedValue(BiometricsStatus.Available);
+      desktopBiometricsService.canEnableBiometricUnlock.mockResolvedValue(true);
      vaultTimeoutSettingsService.isBiometricLockSet.mockResolvedValue(true);
    });

--- a/apps/desktop/src/app/accounts/settings.component.ts
+++ b/apps/desktop/src/app/accounts/settings.component.ts
@@ -388,24 +388,12 @@ export class SettingsComponent implements OnInit, OnDestroy {
        }
      });

-    this.supportsBiometric = this.shouldAllowBiometricSetup(
-      await this.biometricsService.getBiometricsStatus(),
-    );
+    this.supportsBiometric = await this.biometricsService.canEnableBiometricUnlock();
    this.timerId = setInterval(async () => {
-      this.supportsBiometric = this.shouldAllowBiometricSetup(
-        await this.biometricsService.getBiometricsStatus(),
-      );
+      this.supportsBiometric = await this.biometricsService.canEnableBiometricUnlock();
    }, 1000);
  }

-  private shouldAllowBiometricSetup(biometricStatus: BiometricsStatus): boolean {
-    return [
-      BiometricsStatus.Available,
-      BiometricsStatus.AutoSetupNeeded,
-      BiometricsStatus.ManualSetupNeeded,
-    ].includes(biometricStatus);
-  }
-
  async saveVaultTimeout(newValue: VaultTimeout) {
    if (newValue === VaultTimeoutStringType.Never) {
      const confirmed = await this.dialogService.openSimpleDialog({
--- a/apps/desktop/src/key-management/biometrics/main-biometrics.service.ts
+++ b/apps/desktop/src/key-management/biometrics/main-biometrics.service.ts
@@ -163,4 +163,8 @@ export class MainBiometricsService extends DesktopBiometricsService {
  async getShouldAutopromptNow(): Promise<boolean> {
    return this.shouldAutoPrompt;
  }
+
+  async canEnableBiometricUnlock(): Promise<boolean> {
+    return true;
+  }
 }
--- a/apps/desktop/src/key-management/biometrics/renderer-biometrics.service.spec.ts
+++ b/apps/desktop/src/key-management/biometrics/renderer-biometrics.service.spec.ts
@@ -0,0 +1,44 @@
+import { BiometricsStatus } from "@bitwarden/key-management";
+
+import { RendererBiometricsService } from "./renderer-biometrics.service";
+
+describe("renderer biometrics service tests", function () {
+  beforeEach(() => {
+    (global as any).ipc = {
+      keyManagement: {
+        biometric: {
+          authenticateWithBiometrics: jest.fn(),
+          getBiometricsStatus: jest.fn(),
+          unlockWithBiometricsForUser: jest.fn(),
+          getBiometricsStatusForUser: jest.fn(),
+          deleteBiometricUnlockKeyForUser: jest.fn(),
+          setupBiometrics: jest.fn(),
+          setClientKeyHalfForUser: jest.fn(),
+          getShouldAutoprompt: jest.fn(),
+          setShouldAutoprompt: jest.fn(),
+        },
+      },
+    };
+  });
+
+  describe("canEnableBiometricUnlock", () => {
+    const table: [BiometricsStatus, boolean][] = [
+      [BiometricsStatus.Available, true],
+      [BiometricsStatus.AutoSetupNeeded, true],
+      [BiometricsStatus.ManualSetupNeeded, true],
+
+      [BiometricsStatus.UnlockNeeded, false],
+      [BiometricsStatus.HardwareUnavailable, false],
+      [BiometricsStatus.PlatformUnsupported, false],
+      [BiometricsStatus.NotEnabledLocally, false],
+    ];
+    test.each(table)("canEnableBiometricUnlock(%s) === %s", async (status, expected) => {
+      const service = new RendererBiometricsService();
+      (global as any).ipc.keyManagement.biometric.getBiometricsStatus.mockResolvedValue(status);
+
+      const result = await service.canEnableBiometricUnlock();
+
+      expect(result).toBe(expected);
+    });
+  });
+});
--- a/apps/desktop/src/key-management/biometrics/renderer-biometrics.service.ts
+++ b/apps/desktop/src/key-management/biometrics/renderer-biometrics.service.ts
@@ -51,4 +51,13 @@ export class RendererBiometricsService extends DesktopBiometricsService {
  async setShouldAutopromptNow(value: boolean): Promise<void> {
    return await ipc.keyManagement.biometric.setShouldAutoprompt(value);
  }
+
+  async canEnableBiometricUnlock(): Promise<boolean> {
+    const biometricStatus = await this.getBiometricsStatus();
+    return [
+      BiometricsStatus.Available,
+      BiometricsStatus.AutoSetupNeeded,
+      BiometricsStatus.ManualSetupNeeded,
+    ].includes(biometricStatus);
+  }
 }
--- a/apps/web/src/app/key-management/web-biometric.service.ts
+++ b/apps/web/src/app/key-management/web-biometric.service.ts
@@ -24,4 +24,8 @@ export class WebBiometricsService extends BiometricsService {
  }

  async setShouldAutopromptNow(value: boolean): Promise<void> {}
+
+  async canEnableBiometricUnlock(): Promise<boolean> {
+    return false;
+  }
 }
--- a/libs/key-management/src/biometrics/biometric.service.ts
+++ b/libs/key-management/src/biometrics/biometric.service.ts
@@ -39,4 +39,5 @@ export abstract class BiometricsService {

  abstract getShouldAutopromptNow(): Promise<boolean>;
  abstract setShouldAutopromptNow(value: boolean): Promise<void>;
+  abstract canEnableBiometricUnlock(): Promise<boolean>;
 }
--- a/libs/key-management/src/biometrics/biometrics-commands.ts
+++ b/libs/key-management/src/biometrics/biometrics-commands.ts
@@ -8,6 +8,9 @@ export enum BiometricsCommands {
  /** Get biometric status for a specific user account. This includes both information about availability of cryptographic material (is the user configured for biometric unlock? is a masterpassword unlock needed? But also information about the biometric system's availability in a single status) */
  GetBiometricsStatusForUser = "getBiometricsStatusForUser",

+  /** Checks whether the biometric unlock can be enabled. */
+  CanEnableBiometricUnlock = "canEnableBiometricUnlock",
+
  // legacy
  Unlock = "biometricUnlock",
  IsAvailable = "biometricUnlockAvailable",