[PM-5796] Improve desktop biometric browser integration error handling (#7727)

* Re-register native messaging host integrations on startup

* Check for errors when generating the manifests

* Add log to component

* Switch to Promise.all

* Add injectable service

apps/browser/src/background/nativeMessaging.background.ts

@@ -204,6 +204,8 @@ export class NativeMessagingBackground {
         this.privateKey = null;
         this.connected = false;
 
+        this.logService.error("NativeMessaging port disconnected because of error: " + error);
+
         const reason = error != null ? "desktopIntegrationDisabled" : null;
         reject(new Error(reason));
       });

apps/desktop/src/app/accounts/settings.component.ts

@@ -14,6 +14,7 @@ import { DeviceType } from "@bitwarden/common/enums";
 import { VaultTimeoutAction } from "@bitwarden/common/enums/vault-timeout-action.enum";
 import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.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 { StateService } from "@bitwarden/common/platform/abstractions/state.service";
@@ -27,6 +28,7 @@ import { DialogService } from "@bitwarden/components";
 import { SetPinComponent } from "../../auth/components/set-pin.component";
 import { DesktopAutofillSettingsService } from "../../autofill/services/desktop-autofill-settings.service";
 import { DesktopSettingsService } from "../../platform/services/desktop-settings.service";
+import { NativeMessagingManifestService } from "../services/native-messaging-manifest.service";
 
 @Component({
   selector: "app-settings",
@@ -126,6 +128,8 @@ export class SettingsComponent implements OnInit {
     private biometricStateService: BiometricStateService,
     private desktopAutofillSettingsService: DesktopAutofillSettingsService,
     private authRequestService: AuthRequestServiceAbstraction,
+    private logService: LogService,
+    private nativeMessagingManifestService: NativeMessagingManifestService,
   ) {
     const isMac = this.platformUtilsService.getDevice() === DeviceType.MacOsDesktop;
 
@@ -628,11 +632,20 @@ export class SettingsComponent implements OnInit {
     }
 
     await this.stateService.setEnableBrowserIntegration(this.form.value.enableBrowserIntegration);
-    this.messagingService.send(
+
-      this.form.value.enableBrowserIntegration
+    const errorResult = await this.nativeMessagingManifestService.generate(
-        ? "enableBrowserIntegration"
+      this.form.value.enableBrowserIntegration,
-        : "disableBrowserIntegration",
     );
+    if (errorResult !== null) {
+      this.logService.error("Error in browser integration: " + errorResult);
+      await this.dialogService.openSimpleDialog({
+        title: { key: "browserIntegrationErrorTitle" },
+        content: { key: "browserIntegrationErrorDesc" },
+        acceptButtonText: { key: "ok" },
+        cancelButtonText: null,
+        type: "danger",
+      });
+    }
 
     if (!this.form.value.enableBrowserIntegration) {
       this.form.controls.enableBrowserIntegrationFingerprint.setValue(false);
@@ -651,11 +664,19 @@ export class SettingsComponent implements OnInit {
       await this.stateService.setDuckDuckGoSharedKey(null);
     }
 
-    this.messagingService.send(
+    const errorResult = await this.nativeMessagingManifestService.generateDuckDuckGo(
-      this.form.value.enableDuckDuckGoBrowserIntegration
+      this.form.value.enableDuckDuckGoBrowserIntegration,
-        ? "enableDuckDuckGoBrowserIntegration"
-        : "disableDuckDuckGoBrowserIntegration",
     );
+    if (errorResult !== null) {
+      this.logService.error("Error in DDG browser integration: " + errorResult);
+      await this.dialogService.openSimpleDialog({
+        title: { key: "browserIntegrationUnsupportedTitle" },
+        content: errorResult.message,
+        acceptButtonText: { key: "ok" },
+        cancelButtonText: null,
+        type: "warning",
+      });
+    }
   }
 
   async saveBrowserIntegrationFingerprint() {

apps/desktop/src/app/services/native-messaging-manifest.service.ts

@@ -0,0 +1,13 @@
+import { Injectable } from "@angular/core";
+
+@Injectable()
+export class NativeMessagingManifestService {
+  constructor() {}
+
+  async generate(create: boolean): Promise<Error | null> {
+    return ipc.platform.nativeMessaging.manifests.generate(create);
+  }
+  async generateDuckDuckGo(create: boolean): Promise<Error | null> {
+    return ipc.platform.nativeMessaging.manifests.generateDuckDuckGo(create);
+  }
+}

apps/desktop/src/app/services/services.module.ts

@@ -76,6 +76,7 @@ import { SearchBarService } from "../layout/search/search-bar.service";
 
 import { DesktopFileDownloadService } from "./desktop-file-download.service";
 import { InitService } from "./init.service";
+import { NativeMessagingManifestService } from "./native-messaging-manifest.service";
 import { RendererCryptoFunctionService } from "./renderer-crypto-function.service";
 
 const RELOAD_CALLBACK = new SafeInjectionToken<() => any>("RELOAD_CALLBACK");
@@ -249,6 +250,11 @@ const safeProviders: SafeProvider[] = [
     provide: DesktopAutofillSettingsService,
     deps: [StateProvider],
   }),
+  safeProvider({
+    provide: NativeMessagingManifestService,
+    useClass: NativeMessagingManifestService,
+    deps: [],
+  }),
 ];
 
 @NgModule({

apps/desktop/src/locales/en/messages.json

@@ -1632,6 +1632,12 @@
   "browserIntegrationUnsupportedTitle": {
     "message": "Browser integration not supported"
   },
+  "browserIntegrationErrorTitle": {
+    "message": "Error enabling browser integration"
+  },
+  "browserIntegrationErrorDesc": {
+    "message": "An error has occurred while enabling browser integration."
+  },
   "browserIntegrationMasOnlyDesc": {
     "message": "Unfortunately browser integration is only supported in the Mac App Store version for now."
   },

apps/desktop/src/main.ts

@@ -291,12 +291,20 @@ export class Main {
         this.powerMonitorMain.init();
         await this.updaterMain.init();
 
-        if (
+        const [browserIntegrationEnabled, ddgIntegrationEnabled] = await Promise.all([
-          (await this.stateService.getEnableBrowserIntegration()) ||
+          this.stateService.getEnableBrowserIntegration(),
-          (await firstValueFrom(
+          firstValueFrom(this.desktopAutofillSettingsService.enableDuckDuckGoBrowserIntegration$),
-            this.desktopAutofillSettingsService.enableDuckDuckGoBrowserIntegration$,
+        ]);
-          ))
+
-        ) {
+        if (browserIntegrationEnabled || ddgIntegrationEnabled) {
+          // Re-register the native messaging host integrations on startup, in case they are not present
+          if (browserIntegrationEnabled) {
+            this.nativeMessagingMain.generateManifests().catch(this.logService.error);
+          }
+          if (ddgIntegrationEnabled) {
+            this.nativeMessagingMain.generateDdgManifests().catch(this.logService.error);
+          }
+
           this.nativeMessagingMain.listen();
         }
 

apps/desktop/src/main/messaging.main.ts

@@ -75,22 +75,6 @@ export class MessagingMain {
       case "getWindowIsFocused":
         this.windowIsFocused();
         break;
-      case "enableBrowserIntegration":
-        this.main.nativeMessagingMain.generateManifests();
-        this.main.nativeMessagingMain.listen();
-        break;
-      case "enableDuckDuckGoBrowserIntegration":
-        this.main.nativeMessagingMain.generateDdgManifests();
-        this.main.nativeMessagingMain.listen();
-        break;
-      case "disableBrowserIntegration":
-        this.main.nativeMessagingMain.removeManifests();
-        this.main.nativeMessagingMain.stop();
-        break;
-      case "disableDuckDuckGoBrowserIntegration":
-        this.main.nativeMessagingMain.removeDdgManifests();
-        this.main.nativeMessagingMain.stop();
-        break;
       default:
         break;
     }

apps/desktop/src/main/native-messaging.main.ts

@@ -22,7 +22,55 @@ export class NativeMessagingMain {
     private windowMain: WindowMain,
     private userPath: string,
     private exePath: string,
-  ) {}
+  ) {
+    ipcMain.handle(
+      "nativeMessaging.manifests",
+      async (_event: any, options: { create: boolean }) => {
+        if (options.create) {
+          this.listen();
+          try {
+            await this.generateManifests();
+          } catch (e) {
+            this.logService.error("Error generating manifests: " + e);
+            return e;
+          }
+        } else {
+          this.stop();
+          try {
+            await this.removeManifests();
+          } catch (e) {
+            this.logService.error("Error removing manifests: " + e);
+            return e;
+          }
+        }
+        return null;
+      },
+    );
+
+    ipcMain.handle(
+      "nativeMessaging.ddgManifests",
+      async (_event: any, options: { create: boolean }) => {
+        if (options.create) {
+          this.listen();
+          try {
+            await this.generateDdgManifests();
+          } catch (e) {
+            this.logService.error("Error generating duckduckgo manifests: " + e);
+            return e;
+          }
+        } else {
+          this.stop();
+          try {
+            await this.removeDdgManifests();
+          } catch (e) {
+            this.logService.error("Error removing duckduckgo manifests: " + e);
+            return e;
+          }
+        }
+        return null;
+      },
+    );
+  }
 
   listen() {
     ipc.config.id = "bitwarden";
@@ -76,7 +124,7 @@ export class NativeMessagingMain {
     ipc.server.emit(socket, "message", message);
   }
 
-  generateManifests() {
+  async generateManifests() {
     const baseJson = {
       name: "com.8bit.bitwarden",
       description: "Bitwarden desktop <-> browser bridge",
@@ -84,6 +132,10 @@ export class NativeMessagingMain {
       type: "stdio",
     };
 
+    if (!existsSync(baseJson.path)) {
+      throw new Error(`Unable to find binary: ${baseJson.path}`);
+    }
+
     const firefoxJson = {
       ...baseJson,
       ...{ allowed_extensions: ["{446900e4-71c2-419f-a6a7-df9c091e268b}"] },
@@ -92,8 +144,11 @@ export class NativeMessagingMain {
       ...baseJson,
       ...{
         allowed_origins: [
+          // Chrome extension
           "chrome-extension://nngceckbapebfimnlniiiahkandclblb/",
+          // Edge extension
           "chrome-extension://jbkfoedolllekgbhcbcoahefnbanhhlh/",
+          // Opera extension
           "chrome-extension://ccnckbpmaceehanjmeomladnmlffdjgn/",
         ],
       },
@@ -102,27 +157,17 @@ export class NativeMessagingMain {
     switch (process.platform) {
       case "win32": {
         const destination = path.join(this.userPath, "browsers");
-        // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
+        await this.writeManifest(path.join(destination, "firefox.json"), firefoxJson);
-        // eslint-disable-next-line @typescript-eslint/no-floating-promises
+        await this.writeManifest(path.join(destination, "chrome.json"), chromeJson);
-        this.writeManifest(path.join(destination, "firefox.json"), firefoxJson);
-        // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-        // eslint-disable-next-line @typescript-eslint/no-floating-promises
-        this.writeManifest(path.join(destination, "chrome.json"), chromeJson);
 
-        // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
+        const nmhs = this.getWindowsNMHS();
-        // eslint-disable-next-line @typescript-eslint/no-floating-promises
+        for (const [key, value] of Object.entries(nmhs)) {
-        this.createWindowsRegistry(
+          let manifestPath = path.join(destination, "chrome.json");
-          "HKLM\\SOFTWARE\\Mozilla\\Firefox",
+          if (key === "Firefox") {
-          "HKCU\\SOFTWARE\\Mozilla\\NativeMessagingHosts\\com.8bit.bitwarden",
+            manifestPath = path.join(destination, "firefox.json");
-          path.join(destination, "firefox.json"),
+          }
-        );
+          await this.createWindowsRegistry(value, manifestPath);
-        // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
+        }
-        // eslint-disable-next-line @typescript-eslint/no-floating-promises
-        this.createWindowsRegistry(
-          "HKCU\\SOFTWARE\\Google\\Chrome",
-          "HKCU\\SOFTWARE\\Google\\Chrome\\NativeMessagingHosts\\com.8bit.bitwarden",
-          path.join(destination, "chrome.json"),
-        );
         break;
       }
       case "darwin": {
@@ -136,38 +181,30 @@ export class NativeMessagingMain {
               manifest = firefoxJson;
             }
 
-            this.writeManifest(p, manifest).catch((e) =>
+            await this.writeManifest(p, manifest);
-              this.logService.error(`Error writing manifest for ${key}. ${e}`),
-            );
           } else {
-            this.logService.warning(`${key} not found skipping.`);
+            this.logService.warning(`${key} not found, skipping.`);
           }
         }
         break;
       }
       case "linux":
         if (existsSync(`${this.homedir()}/.mozilla/`)) {
-          // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
+          await this.writeManifest(
-          // eslint-disable-next-line @typescript-eslint/no-floating-promises
-          this.writeManifest(
             `${this.homedir()}/.mozilla/native-messaging-hosts/com.8bit.bitwarden.json`,
             firefoxJson,
           );
         }
 
         if (existsSync(`${this.homedir()}/.config/google-chrome/`)) {
-          // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
+          await this.writeManifest(
-          // eslint-disable-next-line @typescript-eslint/no-floating-promises
-          this.writeManifest(
             `${this.homedir()}/.config/google-chrome/NativeMessagingHosts/com.8bit.bitwarden.json`,
             chromeJson,
           );
         }
 
         if (existsSync(`${this.homedir()}/.config/microsoft-edge/`)) {
-          // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
+          await this.writeManifest(
-          // eslint-disable-next-line @typescript-eslint/no-floating-promises
-          this.writeManifest(
             `${this.homedir()}/.config/microsoft-edge/NativeMessagingHosts/com.8bit.bitwarden.json`,
             chromeJson,
           );
@@ -178,20 +215,23 @@ export class NativeMessagingMain {
     }
   }
 
-  generateDdgManifests() {
+  async generateDdgManifests() {
     const manifest = {
       name: "com.8bit.bitwarden",
       description: "Bitwarden desktop <-> DuckDuckGo bridge",
       path: this.binaryPath(),
       type: "stdio",
     };
+
+    if (!existsSync(manifest.path)) {
+      throw new Error(`Unable to find binary: ${manifest.path}`);
+    }
+
     switch (process.platform) {
       case "darwin": {
         /* eslint-disable-next-line no-useless-escape */
         const path = `${this.homedir()}/Library/Containers/com.duckduckgo.macos.browser/Data/Library/Application\ Support/NativeMessagingHosts/com.8bit.bitwarden.json`;
-        this.writeManifest(path, manifest).catch((e) =>
+        await this.writeManifest(path, manifest);
-          this.logService.error(`Error writing manifest for DuckDuckGo. ${e}`),
-        );
         break;
       }
       default:
@@ -199,86 +239,50 @@ export class NativeMessagingMain {
     }
   }
 
-  removeManifests() {
+  async removeManifests() {
     switch (process.platform) {
-      case "win32":
+      case "win32": {
-        // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
+        await this.removeIfExists(path.join(this.userPath, "browsers", "firefox.json"));
-        // eslint-disable-next-line @typescript-eslint/no-floating-promises
+        await this.removeIfExists(path.join(this.userPath, "browsers", "chrome.json"));
-        fs.unlink(path.join(this.userPath, "browsers", "firefox.json"));
+
-        // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
+        const nmhs = this.getWindowsNMHS();
-        // eslint-disable-next-line @typescript-eslint/no-floating-promises
+        for (const [, value] of Object.entries(nmhs)) {
-        fs.unlink(path.join(this.userPath, "browsers", "chrome.json"));
+          await this.deleteWindowsRegistry(value);
-        // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
+        }
-        // eslint-disable-next-line @typescript-eslint/no-floating-promises
-        this.deleteWindowsRegistry(
-          "HKCU\\SOFTWARE\\Mozilla\\NativeMessagingHosts\\com.8bit.bitwarden",
-        );
-        // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-        // eslint-disable-next-line @typescript-eslint/no-floating-promises
-        this.deleteWindowsRegistry(
-          "HKCU\\SOFTWARE\\Google\\Chrome\\NativeMessagingHosts\\com.8bit.bitwarden",
-        );
         break;
+      }
       case "darwin": {
         const nmhs = this.getDarwinNMHS();
         for (const [, value] of Object.entries(nmhs)) {
-          const p = path.join(value, "NativeMessagingHosts", "com.8bit.bitwarden.json");
+          await this.removeIfExists(
-          if (existsSync(p)) {
+            path.join(value, "NativeMessagingHosts", "com.8bit.bitwarden.json"),
-            // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-            // eslint-disable-next-line @typescript-eslint/no-floating-promises
-            fs.unlink(p);
-          }
-        }
-        break;
-      }
-      case "linux":
-        if (
-          existsSync(`${this.homedir()}/.mozilla/native-messaging-hosts/com.8bit.bitwarden.json`)
-        ) {
-          // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-          // eslint-disable-next-line @typescript-eslint/no-floating-promises
-          fs.unlink(`${this.homedir()}/.mozilla/native-messaging-hosts/com.8bit.bitwarden.json`);
-        }
-
-        if (
-          existsSync(
-            `${this.homedir()}/.config/google-chrome/NativeMessagingHosts/com.8bit.bitwarden.json`,
-          )
-        ) {
-          // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-          // eslint-disable-next-line @typescript-eslint/no-floating-promises
-          fs.unlink(
-            `${this.homedir()}/.config/google-chrome/NativeMessagingHosts/com.8bit.bitwarden.json`,
-          );
-        }
-
-        if (
-          existsSync(
-            `${this.homedir()}/.config/microsoft-edge/NativeMessagingHosts/com.8bit.bitwarden.json`,
-          )
-        ) {
-          // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-          // eslint-disable-next-line @typescript-eslint/no-floating-promises
-          fs.unlink(
-            `${this.homedir()}/.config/microsoft-edge/NativeMessagingHosts/com.8bit.bitwarden.json`,
           );
         }
         break;
+      }
+      case "linux": {
+        await this.removeIfExists(
+          `${this.homedir()}/.mozilla/native-messaging-hosts/com.8bit.bitwarden.json`,
+        );
+        await this.removeIfExists(
+          `${this.homedir()}/.config/google-chrome/NativeMessagingHosts/com.8bit.bitwarden.json`,
+        );
+        await this.removeIfExists(
+          `${this.homedir()}/.config/microsoft-edge/NativeMessagingHosts/com.8bit.bitwarden.json`,
+        );
+        break;
+      }
       default:
         break;
     }
   }
 
-  removeDdgManifests() {
+  async removeDdgManifests() {
     switch (process.platform) {
       case "darwin": {
         /* eslint-disable-next-line no-useless-escape */
         const path = `${this.homedir()}/Library/Containers/com.duckduckgo.macos.browser/Data/Library/Application\ Support/NativeMessagingHosts/com.8bit.bitwarden.json`;
-        if (existsSync(path)) {
+        await this.removeIfExists(path);
-          // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-          // eslint-disable-next-line @typescript-eslint/no-floating-promises
-          fs.unlink(path);
-        }
         break;
       }
       default:
@@ -286,6 +290,16 @@ export class NativeMessagingMain {
     }
   }
 
+  private getWindowsNMHS() {
+    return {
+      Firefox: "HKCU\\SOFTWARE\\Mozilla\\NativeMessagingHosts\\com.8bit.bitwarden",
+      Chrome: "HKCU\\SOFTWARE\\Google\\Chrome\\NativeMessagingHosts\\com.8bit.bitwarden",
+      Chromium: "HKCU\\SOFTWARE\\Chromium\\NativeMessagingHosts\\com.8bit.bitwarden",
+      // Edge uses the same registry key as Chrome as a fallback, but it's has its own separate key as well.
+      "Microsoft Edge": "HKCU\\SOFTWARE\\Microsoft\\Edge\\NativeMessagingHosts\\com.8bit.bitwarden",
+    };
+  }
+
   private getDarwinNMHS() {
     /* eslint-disable no-useless-escape */
     return {
@@ -305,10 +319,13 @@ export class NativeMessagingMain {
   }
 
   private async writeManifest(destination: string, manifest: object) {
+    this.logService.debug(`Writing manifest: ${destination}`);
+
     if (!existsSync(path.dirname(destination))) {
       await fs.mkdir(path.dirname(destination));
     }
-    fs.writeFile(destination, JSON.stringify(manifest, null, 2)).catch(this.logService.error);
+
+    await fs.writeFile(destination, JSON.stringify(manifest, null, 2));
   }
 
   private binaryPath() {
@@ -327,24 +344,14 @@ export class NativeMessagingMain {
     return regedit;
   }
 
-  private async createWindowsRegistry(check: string, location: string, jsonFile: string) {
+  private async createWindowsRegistry(location: string, jsonFile: string) {
     const regedit = this.getRegeditInstance();
 
-    const list = util.promisify(regedit.list);
     const createKey = util.promisify(regedit.createKey);
     const putValue = util.promisify(regedit.putValue);
 
     this.logService.debug(`Adding registry: ${location}`);
 
-    // Check installed
-    try {
-      await list(check);
-    } catch {
-      this.logService.warning(`Not finding registry ${check} skipping.`);
-      return;
-    }
-
-    try {
     await createKey(location);
 
     // Insert path to manifest
@@ -357,9 +364,6 @@ export class NativeMessagingMain {
     };
 
     return putValue(obj);
-    } catch (error) {
-      this.logService.error(error);
-    }
   }
 
   private async deleteWindowsRegistry(key: string) {
@@ -385,4 +389,10 @@ export class NativeMessagingMain {
       return homedir();
     }
   }
+
+  private async removeIfExists(path: string) {
+    if (existsSync(path)) {
+      await fs.unlink(path);
+    }
+  }
 }

apps/desktop/src/platform/preload.ts

@@ -74,6 +74,13 @@ const nativeMessaging = {
   onMessage: (callback: (message: LegacyMessageWrapper | Message) => void) => {
     ipcRenderer.on("nativeMessaging", (_event, message) => callback(message));
   },
+
+  manifests: {
+    generate: (create: boolean): Promise<Error | null> =>
+      ipcRenderer.invoke("nativeMessaging.manifests", { create }),
+    generateDuckDuckGo: (create: boolean): Promise<Error | null> =>
+      ipcRenderer.invoke("nativeMessaging.ddgManifests", { create }),
+  },
 };
 
 const crypto = {