Merge branch 'km/move-clientkeyhalf-to-platform-impl' into km/simplify-linux-biometrics

2026-02-07 04:03:29 +00:00 · 2025-06-11 13:55:45 +02:00
parent 291eb95c1a eabffa24b4
commit 38341c3cf6
4 changed files with 180 additions and 107 deletions
--- a/apps/desktop/src/key-management/biometrics/main-biometrics.service.spec.ts
+++ b/apps/desktop/src/key-management/biometrics/main-biometrics.service.spec.ts
@@ -184,12 +184,23 @@ describe("MainBiometricsService", function () {
        biometricStateService.getRequirePasswordOnStart.mockResolvedValue(
          requirePasswordOnStart as boolean,
        );
-        (sut as any).clientKeyHalves = new Map();
-        const userId = "test" as UserId;
-        if (hasKeyHalf) {
-          (sut as any).clientKeyHalves.set(userId, "test");
+        if (!requirePasswordOnStart) {
+          (sut as any).osBiometricsService.getBiometricsFirstUnlockStatusForUser = jest
+            .fn()
+            .mockResolvedValue(BiometricsStatus.Available);
+        } else {
+          if (hasKeyHalf) {
+            (sut as any).osBiometricsService.getBiometricsFirstUnlockStatusForUser = jest
+              .fn()
+              .mockResolvedValue(BiometricsStatus.Available);
+          } else {
+            (sut as any).osBiometricsService.getBiometricsFirstUnlockStatusForUser = jest
+              .fn()
+              .mockResolvedValue(BiometricsStatus.UnlockNeeded);
+          }
        }

+        const userId = "test" as UserId;
        const actual = await sut.getBiometricsStatusForUser(userId);
        expect(actual).toBe(expected);
      }
@@ -256,34 +267,24 @@ describe("MainBiometricsService", function () {

    it("should return null if no biometric key is returned ", async () => {
      const userId = "test" as UserId;
-      (sut as any).clientKeyHalves.set(userId, "testKeyHalf");
-
+      osBiometricsService.getBiometricKey.mockResolvedValue(null);
      const userKey = await sut.unlockWithBiometricsForUser(userId);

      expect(userKey).toBeNull();
-      expect(osBiometricsService.getBiometricKey).toHaveBeenCalledWith(
-        "Bitwarden_biometric",
-        `${userId}_user_biometric`,
-        "testKeyHalf",
-      );
+      expect(osBiometricsService.getBiometricKey).toHaveBeenCalledWith(userId);
    });

    it("should return the biometric key if a valid key is returned", async () => {
      const userId = "test" as UserId;
-      (sut as any).clientKeyHalves.set(userId, "testKeyHalf");
      const biometricKey = new SymmetricCryptoKey(new Uint8Array(64));
      osBiometricsService.getBiometricKey.mockResolvedValue(biometricKey);

      const userKey = await sut.unlockWithBiometricsForUser(userId);

      expect(userKey).not.toBeNull();
-      expect(userKey!.keyB64).toBe(biometricKey);
+      expect(userKey!.keyB64).toBe(biometricKey.toBase64());
      expect(userKey!.inner().type).toBe(EncryptionType.AesCbc256_HmacSha256_B64);
-      expect(osBiometricsService.getBiometricKey).toHaveBeenCalledWith(
-        "Bitwarden_biometric",
-        `${userId}_user_biometric`,
-        "testKeyHalf",
-      );
+      expect(osBiometricsService.getBiometricKey).toHaveBeenCalledWith(userId);
    });
  });

@@ -305,25 +306,12 @@ describe("MainBiometricsService", function () {
      (sut as any).osBiometricsService = osBiometricsService;
    });

-    it("should throw an error if no client key half is provided", async () => {
-      const userId = "test" as UserId;
-      await expect(sut.setBiometricProtectedUnlockKeyForUser(userId, unlockKey)).rejects.toThrow(
-        "No client key half provided for user",
-      );
-    });
-
    it("should call the platform specific setBiometricKey method", async () => {
      const userId = "test" as UserId;
-      (sut as any).clientKeyHalves.set(userId, "testKeyHalf");

      await sut.setBiometricProtectedUnlockKeyForUser(userId, unlockKey);

-      expect(osBiometricsService.setBiometricKey).toHaveBeenCalledWith(
-        "Bitwarden_biometric",
-        `${userId}_user_biometric`,
-        unlockKey,
-        "testKeyHalf",
-      );
+      expect(osBiometricsService.setBiometricKey).toHaveBeenCalledWith(userId, unlockKey);
    });
  });

@@ -345,10 +333,7 @@ describe("MainBiometricsService", function () {

      await sut.deleteBiometricUnlockKeyForUser(userId);

-      expect(osBiometricsService.deleteBiometricKey).toHaveBeenCalledWith(
-        "Bitwarden_biometric",
-        `${userId}_user_biometric`,
-      );
+      expect(osBiometricsService.deleteBiometricKey).toHaveBeenCalledWith(userId);
    });
  });

--- a/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.spec.ts
+++ b/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.spec.ts
@@ -0,0 +1,143 @@
+import { mock } from "jest-mock-extended";
+
+import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service";
+import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
+import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
+import { UserId } from "@bitwarden/common/types/guid";
+import { BiometricsStatus, BiometricStateService } from "@bitwarden/key-management";
+
+import OsBiometricsServiceWindows from "./os-biometrics-windows.service";
+
+jest.mock("@bitwarden/desktop-napi", () => ({
+  biometrics: {
+    available: jest.fn(),
+    setBiometricSecret: jest.fn(),
+    getBiometricSecret: jest.fn(),
+    deriveKeyMaterial: jest.fn(),
+    prompt: jest.fn(),
+  },
+  passwords: {
+    getPassword: jest.fn(),
+    deletePassword: jest.fn(),
+  },
+}));
+
+describe("OsBiometricsServiceWindows", () => {
+  let service: OsBiometricsServiceWindows;
+  let biometricStateService: BiometricStateService;
+
+  beforeEach(() => {
+    const i18nService = mock<I18nService>();
+    const logService = mock<LogService>();
+    biometricStateService = mock<BiometricStateService>();
+    const encryptionService = mock<EncryptService>();
+    const cryptoFunctionService = mock<CryptoFunctionService>();
+    service = new OsBiometricsServiceWindows(
+      i18nService,
+      null,
+      logService,
+      biometricStateService,
+      encryptionService,
+      cryptoFunctionService,
+    );
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  describe("getBiometricsFirstUnlockStatusForUser", () => {
+    const userId = "test-user-id" as UserId;
+    it("should return Available when requirePasswordOnRestart is false", async () => {
+      biometricStateService.getRequirePasswordOnStart = jest.fn().mockResolvedValue(false);
+      const result = await service.getBiometricsFirstUnlockStatusForUser(userId);
+      expect(result).toBe(BiometricsStatus.Available);
+    });
+    it("should return Available when requirePasswordOnRestart is true and client key half is set", async () => {
+      biometricStateService.getRequirePasswordOnStart = jest.fn().mockResolvedValue(true);
+      (service as any).clientKeyHalves = new Map<string, Uint8Array>();
+      (service as any).clientKeyHalves.set(userId, new Uint8Array([1, 2, 3, 4]));
+      const result = await service.getBiometricsFirstUnlockStatusForUser(userId);
+      expect(result).toBe(BiometricsStatus.Available);
+    });
+    it("should return UnlockNeeded when requirePasswordOnRestart is true and client key half is not set", async () => {
+      biometricStateService.getRequirePasswordOnStart = jest.fn().mockResolvedValue(true);
+      (service as any).clientKeyHalves = new Map<string, Uint8Array>();
+      const result = await service.getBiometricsFirstUnlockStatusForUser(userId);
+      expect(result).toBe(BiometricsStatus.UnlockNeeded);
+    });
+  });
+
+  describe("getOrCreateBiometricEncryptionClientKeyHalf", () => {
+    const userId = "test-user-id" as UserId;
+    const key = new SymmetricCryptoKey(new Uint8Array(64));
+    let encryptionService: EncryptService;
+    let cryptoFunctionService: CryptoFunctionService;
+
+    beforeEach(() => {
+      encryptionService = mock<EncryptService>();
+      cryptoFunctionService = mock<CryptoFunctionService>();
+      service = new OsBiometricsServiceWindows(
+        mock<I18nService>(),
+        null,
+        mock<LogService>(),
+        biometricStateService,
+        encryptionService,
+        cryptoFunctionService,
+      );
+    });
+
+    it("should return null if getRequirePasswordOnRestart is false", async () => {
+      biometricStateService.getRequirePasswordOnStart = jest.fn().mockResolvedValue(false);
+      const result = await service.getOrCreateBiometricEncryptionClientKeyHalf(userId, key);
+      expect(result).toBeNull();
+    });
+
+    it("should return cached key half if already present", async () => {
+      biometricStateService.getRequirePasswordOnStart = jest.fn().mockResolvedValue(true);
+      const cachedKeyHalf = new Uint8Array([10, 20, 30]);
+      (service as any).clientKeyHalves.set(userId.toString(), cachedKeyHalf);
+      const result = await service.getOrCreateBiometricEncryptionClientKeyHalf(userId, key);
+      expect(result).toBe(cachedKeyHalf);
+    });
+
+    it("should decrypt and return existing encrypted client key half", async () => {
+      biometricStateService.getRequirePasswordOnStart = jest.fn().mockResolvedValue(true);
+      biometricStateService.getEncryptedClientKeyHalf = jest
+        .fn()
+        .mockResolvedValue(new Uint8Array([1, 2, 3]));
+      const decrypted = new Uint8Array([4, 5, 6]);
+      encryptionService.decryptBytes = jest.fn().mockResolvedValue(decrypted);
+
+      const result = await service.getOrCreateBiometricEncryptionClientKeyHalf(userId, key);
+
+      expect(biometricStateService.getEncryptedClientKeyHalf).toHaveBeenCalledWith(userId);
+      expect(encryptionService.decryptBytes).toHaveBeenCalledWith(new Uint8Array([1, 2, 3]), key);
+      expect(result).toEqual(decrypted);
+      expect((service as any).clientKeyHalves.get(userId.toString())).toEqual(decrypted);
+    });
+
+    it("should generate, encrypt, store, and cache a new key half if none exists", async () => {
+      biometricStateService.getRequirePasswordOnStart = jest.fn().mockResolvedValue(true);
+      biometricStateService.getEncryptedClientKeyHalf = jest.fn().mockResolvedValue(null);
+      const randomBytes = new Uint8Array([7, 8, 9]);
+      cryptoFunctionService.randomBytes = jest.fn().mockResolvedValue(randomBytes);
+      const encrypted = new Uint8Array([10, 11, 12]);
+      encryptionService.encryptBytes = jest.fn().mockResolvedValue(encrypted);
+      biometricStateService.setEncryptedClientKeyHalf = jest.fn().mockResolvedValue(undefined);
+
+      const result = await service.getOrCreateBiometricEncryptionClientKeyHalf(userId, key);
+
+      expect(cryptoFunctionService.randomBytes).toHaveBeenCalledWith(32);
+      expect(encryptionService.encryptBytes).toHaveBeenCalledWith(randomBytes, key);
+      expect(biometricStateService.setEncryptedClientKeyHalf).toHaveBeenCalledWith(
+        encrypted,
+        userId,
+      );
+      expect(result).toBeNull();
+      expect((service as any).clientKeyHalves.get(userId.toString())).toBeNull();
+    });
+  });
+});
--- a/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.ts
+++ b/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.ts
@@ -27,7 +27,7 @@ export default class OsBiometricsServiceWindows implements OsBiometricService {
  private _iv: string | null = null;
  // Use getKeyMaterial helper instead of direct access
  private _osKeyHalf: string | null = null;
-  private clientKeyHalves = new Map<string, Uint8Array | null>();
+  private clientKeyHalves = new Map<UserId, Uint8Array>();

  constructor(
    private i18nService: I18nService,
@@ -45,8 +45,8 @@ export default class OsBiometricsServiceWindows implements OsBiometricService {
  async getBiometricKey(userId: UserId): Promise<SymmetricCryptoKey | null> {
    const value = await passwords.getPassword(SERVICE, getLookupKeyForUser(userId));
    let clientKeyHalfB64: string | null = null;
-    if (this.clientKeyHalves.has(userId.toString())) {
-      clientKeyHalfB64 = Utils.fromBufferToB64(this.clientKeyHalves.get(userId.toString()));
+    if (this.clientKeyHalves.has(userId)) {
+      clientKeyHalfB64 = Utils.fromBufferToB64(this.clientKeyHalves.get(userId));
    }

    if (value == null || value == "") {
@@ -274,7 +274,7 @@ export default class OsBiometricsServiceWindows implements OsBiometricService {

  async runSetup(): Promise<void> {}

-  private async getOrCreateBiometricEncryptionClientKeyHalf(
+  async getOrCreateBiometricEncryptionClientKeyHalf(
    userId: UserId,
    key: SymmetricCryptoKey,
  ): Promise<Uint8Array | null> {
@@ -283,8 +283,8 @@ export default class OsBiometricsServiceWindows implements OsBiometricService {
      return null;
    }

-    if (this.clientKeyHalves.has(userId.toString())) {
-      return this.clientKeyHalves.get(userId.toString());
+    if (this.clientKeyHalves.has(userId)) {
+      return this.clientKeyHalves.get(userId);
    }

    // Retrieve existing key half if it exists
@@ -301,18 +301,21 @@ export default class OsBiometricsServiceWindows implements OsBiometricService {
      await this.biometricStateService.setEncryptedClientKeyHalf(encKey, userId);
    }

-    this.clientKeyHalves.set(userId.toString(), clientKeyHalf);
+    this.clientKeyHalves.set(userId, clientKeyHalf);

    return clientKeyHalf;
  }

  async getBiometricsFirstUnlockStatusForUser(userId: UserId): Promise<BiometricsStatus> {
    const requireClientKeyHalf = await this.biometricStateService.getRequirePasswordOnStart(userId);
-    const clientKeyHalfB64 = this.clientKeyHalves.get(userId);
-    const clientKeyHalfSatisfied = !requireClientKeyHalf || !!clientKeyHalfB64;
-    if (!clientKeyHalfSatisfied) {
+    if (!requireClientKeyHalf) {
+      return BiometricsStatus.Available;
+    }
+
+    if (this.clientKeyHalves.has(userId)) {
+      return BiometricsStatus.Available;
+    } else {
      return BiometricsStatus.UnlockNeeded;
    }
-    return BiometricsStatus.Available;
  }
 }
--- a/apps/desktop/src/key-management/electron-key.service.spec.ts
+++ b/apps/desktop/src/key-management/electron-key.service.spec.ts
@@ -9,14 +9,11 @@ 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 { Utils } from "@bitwarden/common/platform/misc/utils";
-import { CsprngArray } from "@bitwarden/common/types/csprng";
 import { UserId } from "@bitwarden/common/types/guid";
 import { UserKey } from "@bitwarden/common/types/key";
 import { BiometricStateService, KdfConfigService } from "@bitwarden/key-management";

 import {
-  makeEncString,
-  makeStaticByteArray,
  makeSymmetricCryptoKey,
  FakeAccountService,
  mockAccountServiceWith,
@@ -97,11 +94,10 @@ describe("ElectronKeyService", () => {

          expect(biometricService.setBiometricProtectedUnlockKeyForUser).toHaveBeenCalledWith(
            mockUserId,
-            userKey.keyB64,
+            userKey,
          );
          expect(biometricStateService.setEncryptedClientKeyHalf).not.toHaveBeenCalled();
          expect(biometricStateService.getBiometricUnlockEnabled).toHaveBeenCalledWith(mockUserId);
-          expect(biometricStateService.getRequirePasswordOnStart).toHaveBeenCalledWith(mockUserId);
        });

        describe("require password on start enabled", () => {
@@ -109,65 +105,11 @@ describe("ElectronKeyService", () => {
            biometricStateService.getRequirePasswordOnStart.mockResolvedValue(true);
          });

-          it("sets new biometric client key half and biometric unlock key when no biometric client key half stored", async () => {
-            const clientKeyHalfBytes = makeStaticByteArray(32);
-            const clientKeyHalf = Utils.fromBufferToUtf8(clientKeyHalfBytes);
-            const encryptedClientKeyHalf = makeEncString();
-            biometricStateService.getEncryptedClientKeyHalf.mockResolvedValue(null);
-            cryptoFunctionService.randomBytes.mockResolvedValue(
-              clientKeyHalfBytes.buffer as CsprngArray,
-            );
-            encryptService.encryptString.mockResolvedValue(encryptedClientKeyHalf);
-
+          it("sets biometric key", async () => {
            await keyService.setUserKey(userKey, mockUserId);

            expect(biometricService.setBiometricProtectedUnlockKeyForUser).toHaveBeenCalledWith(
              mockUserId,
-              userKey.keyB64,
-            );
-            expect(biometricStateService.setEncryptedClientKeyHalf).toHaveBeenCalledWith(
-              encryptedClientKeyHalf,
-              mockUserId,
-            );
-            expect(biometricStateService.getBiometricUnlockEnabled).toHaveBeenCalledWith(
-              mockUserId,
-            );
-            expect(biometricStateService.getRequirePasswordOnStart).toHaveBeenCalledWith(
-              mockUserId,
-            );
-            expect(biometricStateService.getEncryptedClientKeyHalf).toHaveBeenCalledWith(
-              mockUserId,
-            );
-            expect(cryptoFunctionService.randomBytes).toHaveBeenCalledWith(32);
-            expect(encryptService.encryptString).toHaveBeenCalledWith(clientKeyHalf, userKey);
-          });
-
-          it("sets decrypted biometric client key half and biometric unlock key when existing biometric client key half stored", async () => {
-            const encryptedClientKeyHalf = makeEncString();
-            const clientKeyHalf = Utils.fromBufferToUtf8(makeStaticByteArray(32));
-            biometricStateService.getEncryptedClientKeyHalf.mockResolvedValue(
-              encryptedClientKeyHalf,
-            );
-            encryptService.decryptString.mockResolvedValue(clientKeyHalf);
-
-            await keyService.setUserKey(userKey, mockUserId);
-
-            expect(biometricService.setBiometricProtectedUnlockKeyForUser).toHaveBeenCalledWith(
-              mockUserId,
-              userKey.keyB64,
-            );
-            expect(biometricStateService.setEncryptedClientKeyHalf).not.toHaveBeenCalled();
-            expect(biometricStateService.getBiometricUnlockEnabled).toHaveBeenCalledWith(
-              mockUserId,
-            );
-            expect(biometricStateService.getRequirePasswordOnStart).toHaveBeenCalledWith(
-              mockUserId,
-            );
-            expect(biometricStateService.getEncryptedClientKeyHalf).toHaveBeenCalledWith(
-              mockUserId,
-            );
-            expect(encryptService.decryptString).toHaveBeenCalledWith(
-              encryptedClientKeyHalf,
              userKey,
            );
          });