From 9c8fc8097103ceb02db13810732bdf4b9019f1e7 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com> Date: Mon, 5 May 2025 16:10:04 +0200 Subject: [PATCH] Browser integration verification always re-prompts after desktop app is locked (#14370) --- .../biometric-message-handler.service.spec.ts | 130 +++++++++++++++++- .../biometric-message-handler.service.ts | 21 ++- 2 files changed, 146 insertions(+), 5 deletions(-) diff --git a/apps/desktop/src/services/biometric-message-handler.service.spec.ts b/apps/desktop/src/services/biometric-message-handler.service.spec.ts index 0e92e3839fe..9ddc3da8ed4 100644 --- a/apps/desktop/src/services/biometric-message-handler.service.spec.ts +++ b/apps/desktop/src/services/biometric-message-handler.service.spec.ts @@ -1,6 +1,6 @@ import { NgZone } from "@angular/core"; import { mock, MockProxy } from "jest-mock-extended"; -import { of } from "rxjs"; +import { BehaviorSubject, filter, firstValueFrom, of, take, timeout, timer } from "rxjs"; import { AccountInfo, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; @@ -73,10 +73,13 @@ describe("BiometricMessageHandlerService", () => { ngZone = mock(); i18nService = mock(); + desktopSettingsService.browserIntegrationEnabled$ = of(false); + desktopSettingsService.browserIntegrationFingerprintEnabled$ = of(false); + (global as any).ipc = { platform: { ephemeralStore: { - listEphemeralValueKeys: jest.fn(), + listEphemeralValueKeys: jest.fn(() => Promise.resolve([])), getEphemeralValue: jest.fn(), removeEphemeralValue: jest.fn(), setEphemeralValue: jest.fn(), @@ -107,6 +110,129 @@ describe("BiometricMessageHandlerService", () => { ); }); + describe("constructor", () => { + let browserIntegrationEnabled = new BehaviorSubject(true); + let browserIntegrationFingerprintEnabled = new BehaviorSubject(true); + + beforeEach(async () => { + (global as any).ipc = { + platform: { + ephemeralStore: { + listEphemeralValueKeys: jest.fn(() => + Promise.resolve(["connectedApp_appId1", "connectedApp_appId2"]), + ), + getEphemeralValue: jest.fn((key) => { + if (key === "connectedApp_appId1") { + return Promise.resolve( + JSON.stringify({ + publicKey: Utils.fromUtf8ToB64("publicKeyApp1"), + sessionSecret: Utils.fromUtf8ToB64("sessionSecretApp1"), + trusted: true, + }), + ); + } else if (key === "connectedApp_appId2") { + return Promise.resolve( + JSON.stringify({ + publicKey: Utils.fromUtf8ToB64("publicKeyApp2"), + sessionSecret: Utils.fromUtf8ToB64("sessionSecretApp2"), + trusted: false, + }), + ); + } + return Promise.resolve(null); + }), + removeEphemeralValue: jest.fn(), + setEphemeralValue: jest.fn(), + }, + }, + }; + + desktopSettingsService.browserIntegrationEnabled$ = browserIntegrationEnabled.asObservable(); + desktopSettingsService.browserIntegrationFingerprintEnabled$ = + browserIntegrationFingerprintEnabled.asObservable(); + + service = new BiometricMessageHandlerService( + cryptoFunctionService, + keyService, + encryptService, + logService, + messagingService, + desktopSettingsService, + biometricStateService, + biometricsService, + dialogService, + accountService, + authService, + ngZone, + i18nService, + ); + }); + + afterEach(() => { + browserIntegrationEnabled = new BehaviorSubject(true); + browserIntegrationFingerprintEnabled = new BehaviorSubject(true); + + desktopSettingsService.browserIntegrationEnabled$ = browserIntegrationEnabled.asObservable(); + desktopSettingsService.browserIntegrationFingerprintEnabled$ = + browserIntegrationFingerprintEnabled.asObservable(); + }); + + it("should clear connected apps when browser integration disabled", async () => { + browserIntegrationEnabled.next(false); + + await firstValueFrom( + timer(0, 100).pipe( + filter( + () => + (global as any).ipc.platform.ephemeralStore.removeEphemeralValue.mock.calls.length == + 2, + ), + take(1), + timeout(1000), + ), + ); + + expect((global as any).ipc.platform.ephemeralStore.removeEphemeralValue).toHaveBeenCalledWith( + "connectedApp_appId1", + ); + expect((global as any).ipc.platform.ephemeralStore.removeEphemeralValue).toHaveBeenCalledWith( + "connectedApp_appId2", + ); + }); + + it("should un-trust connected apps when browser integration verification fingerprint disabled", async () => { + browserIntegrationFingerprintEnabled.next(false); + + await firstValueFrom( + timer(0, 100).pipe( + filter( + () => + (global as any).ipc.platform.ephemeralStore.setEphemeralValue.mock.calls.length == 2, + ), + take(1), + timeout(1000), + ), + ); + + expect((global as any).ipc.platform.ephemeralStore.setEphemeralValue).toHaveBeenCalledWith( + "connectedApp_appId1", + JSON.stringify({ + publicKey: Utils.fromUtf8ToB64("publicKeyApp1"), + sessionSecret: Utils.fromUtf8ToB64("sessionSecretApp1"), + trusted: false, + }), + ); + expect((global as any).ipc.platform.ephemeralStore.setEphemeralValue).toHaveBeenCalledWith( + "connectedApp_appId2", + JSON.stringify({ + publicKey: Utils.fromUtf8ToB64("publicKeyApp2"), + sessionSecret: Utils.fromUtf8ToB64("sessionSecretApp2"), + trusted: false, + }), + ); + }); + }); + describe("setup encryption", () => { it("should ignore when public key missing in message", async () => { await service.handleMessage({ diff --git a/apps/desktop/src/services/biometric-message-handler.service.ts b/apps/desktop/src/services/biometric-message-handler.service.ts index 48026bca388..3851f40505b 100644 --- a/apps/desktop/src/services/biometric-message-handler.service.ts +++ b/apps/desktop/src/services/biometric-message-handler.service.ts @@ -91,12 +91,27 @@ export class BiometricMessageHandlerService { private i18nService: I18nService, ) { combineLatest([ - this.desktopSettingService.browserIntegrationFingerprintEnabled$, this.desktopSettingService.browserIntegrationEnabled$, + this.desktopSettingService.browserIntegrationFingerprintEnabled$, ]) .pipe( - concatMap(async () => { - await this.connectedApps.clear(); + concatMap(async ([browserIntegrationEnabled, browserIntegrationFingerprintEnabled]) => { + if (!browserIntegrationEnabled) { + this.logService.info("[Native Messaging IPC] Clearing connected apps"); + await this.connectedApps.clear(); + } else if (!browserIntegrationFingerprintEnabled) { + this.logService.info( + "[Native Messaging IPC] Browser integration fingerprint validation is disabled, untrusting all connected apps", + ); + const connected = await this.connectedApps.list(); + for (const appId of connected) { + const connectedApp = await this.connectedApps.get(appId); + if (connectedApp != null) { + connectedApp.trusted = false; + await this.connectedApps.set(appId, connectedApp); + } + } + } }), ) .subscribe();