[PM-7627] [MV3] Do not run fido2 content scripts on browser settings or extension pages (#8863)

* do no run fido2 content scripts on browser settings or extension background pages

* remove unneeded overlay visibility setting state guard

* only filter content script and page script and update test

* handle content script host permission errors

* add activeTab to mv3 permissions

* allow other browser inject errors to throw

apps/browser/src/autofill/services/autofill.service.spec.ts

@@ -12,6 +12,7 @@ import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abs
 import { EventType } from "@bitwarden/common/enums";
 import { UriMatchStrategy } from "@bitwarden/common/models/domain/domain-service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { EventCollectionService } from "@bitwarden/common/services/event/event-collection.service";
 import {
@@ -74,9 +75,10 @@ describe("AutofillService", () => {
   const logService = mock<LogService>();
   const userVerificationService = mock<UserVerificationService>();
   const billingAccountProfileStateService = mock<BillingAccountProfileStateService>();
+  const platformUtilsService = mock<PlatformUtilsService>();
 
   beforeEach(() => {
-    scriptInjectorService = new BrowserScriptInjectorService();
+    scriptInjectorService = new BrowserScriptInjectorService(platformUtilsService, logService);
     autofillService = new AutofillService(
       cipherService,
       autofillSettingsService,

apps/browser/src/autofill/services/autofill.service.ts

@@ -3,7 +3,6 @@ import { firstValueFrom } from "rxjs";
 import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
-import { AutofillOverlayVisibility } from "@bitwarden/common/autofill/constants";
 import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service";
 import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service";
 import { InlineMenuVisibilitySetting } from "@bitwarden/common/autofill/types";
@@ -107,17 +106,13 @@ export default class AutofillService implements AutofillServiceInterface {
     frameId = 0,
     triggeringOnPageLoad = true,
   ): Promise<void> {
-    // Autofill settings loaded from state can await the active account state indefinitely if
+    // Autofill user settings loaded from state can await the active account state indefinitely
-    // not guarded by an active account check (e.g. the user is logged in)
+    // if not guarded by an active account check (e.g. the user is logged in)
     const activeAccount = await firstValueFrom(this.accountService.activeAccount$);
 
-    // These settings are not available until the user logs in
-    let overlayVisibility: InlineMenuVisibilitySetting = AutofillOverlayVisibility.Off;
     let autoFillOnPageLoadIsEnabled = false;
+    const overlayVisibility = await this.getOverlayVisibility();
 
-    if (activeAccount) {
-      overlayVisibility = await this.getOverlayVisibility();
-    }
     const mainAutofillScript = overlayVisibility
       ? "bootstrap-autofill-overlay.js"
       : "bootstrap-autofill.js";
@@ -2087,9 +2082,7 @@ export default class AutofillService implements AutofillServiceInterface {
     for (let index = 0; index < tabs.length; index++) {
       const tab = tabs[index];
       if (tab.url?.startsWith("http")) {
-        // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
+        void this.injectAutofillScripts(tab, 0, false);
-        // eslint-disable-next-line @typescript-eslint/no-floating-promises
-        this.injectAutofillScripts(tab, 0, false);
       }
     }
   }

apps/browser/src/background/main.background.ts

@@ -813,7 +813,10 @@ export default class MainBackground {
     );
     this.totpService = new TotpService(this.cryptoFunctionService, this.logService);
 
-    this.scriptInjectorService = new BrowserScriptInjectorService();
+    this.scriptInjectorService = new BrowserScriptInjectorService(
+      this.platformUtilsService,
+      this.logService,
+    );
     this.autofillService = new AutofillService(
       this.cipherService,
       this.autofillSettingsService,

apps/browser/src/manifest.v3.json

@@ -51,7 +51,7 @@
     "default_popup": "popup/index.html"
   },
   "permissions": [
-    "<all_urls>",
+    "activeTab",
     "tabs",
     "contextMenus",
     "storage",
@@ -65,7 +65,7 @@
     "webRequestAuthProvider"
   ],
   "optional_permissions": ["nativeMessaging", "privacy"],
-  "host_permissions": ["<all_urls>"],
+  "host_permissions": ["https://*/*", "http://*/*"],
   "content_security_policy": {
     "extension_pages": "script-src 'self' 'wasm-unsafe-eval'; object-src 'self'",
     "sandbox": "sandbox allow-scripts; script-src 'self'"

apps/browser/src/platform/background/service-factories/browser-script-injector-service.factory.ts

@@ -1,10 +1,20 @@
+import {
+  LogServiceInitOptions,
+  logServiceFactory,
+} from "../../background/service-factories/log-service.factory";
 import { BrowserScriptInjectorService } from "../../services/browser-script-injector.service";
 
 import { CachedServices, FactoryOptions, factory } from "./factory-options";
+import {
+  PlatformUtilsServiceInitOptions,
+  platformUtilsServiceFactory,
+} from "./platform-utils-service.factory";
 
 type BrowserScriptInjectorServiceOptions = FactoryOptions;
 
-export type BrowserScriptInjectorServiceInitOptions = BrowserScriptInjectorServiceOptions;
+export type BrowserScriptInjectorServiceInitOptions = BrowserScriptInjectorServiceOptions &
+  PlatformUtilsServiceInitOptions &
+  LogServiceInitOptions;
 
 export function browserScriptInjectorServiceFactory(
   cache: { browserScriptInjectorService?: BrowserScriptInjectorService } & CachedServices,
@@ -14,6 +24,10 @@ export function browserScriptInjectorServiceFactory(
     cache,
     "browserScriptInjectorService",
     opts,
-    async () => new BrowserScriptInjectorService(),
+    async () =>
+      new BrowserScriptInjectorService(
+        await platformUtilsServiceFactory(cache, opts),
+        await logServiceFactory(cache, opts),
+      ),
   );
 }

apps/browser/src/platform/services/browser-script-injector.service.spec.ts

@@ -1,3 +1,8 @@
+import { mock } from "jest-mock-extended";
+
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
+
 import { BrowserApi } from "../browser/browser-api";
 
 import {
@@ -20,9 +25,11 @@ describe("ScriptInjectorService", () => {
   let scriptInjectorService: BrowserScriptInjectorService;
   jest.spyOn(BrowserApi, "executeScriptInTab").mockImplementation();
   jest.spyOn(BrowserApi, "isManifestVersion");
+  const platformUtilsService = mock<PlatformUtilsService>();
+  const logService = mock<LogService>();
 
   beforeEach(() => {
-    scriptInjectorService = new BrowserScriptInjectorService();
+    scriptInjectorService = new BrowserScriptInjectorService(platformUtilsService, logService);
   });
 
   describe("inject", () => {

apps/browser/src/platform/services/browser-script-injector.service.ts

@@ -1,3 +1,6 @@
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
+
 import { BrowserApi } from "../browser/browser-api";
 
 import {
@@ -7,6 +10,13 @@ import {
 } from "./abstractions/script-injector.service";
 
 export class BrowserScriptInjectorService extends ScriptInjectorService {
+  constructor(
+    private readonly platformUtilsService: PlatformUtilsService,
+    private readonly logService: LogService,
+  ) {
+    super();
+  }
+
   /**
    * Facilitates the injection of a script into a tab context. Will adjust
    * behavior between manifest v2 and v3 based on the passed configuration.
@@ -23,9 +33,26 @@ export class BrowserScriptInjectorService extends ScriptInjectorService {
     const injectionDetails = this.buildInjectionDetails(injectDetails, file);
 
     if (BrowserApi.isManifestVersion(3)) {
-      await BrowserApi.executeScriptInTab(tabId, injectionDetails, {
+      try {
-        world: mv3Details?.world ?? "ISOLATED",
+        await BrowserApi.executeScriptInTab(tabId, injectionDetails, {
-      });
+          world: mv3Details?.world ?? "ISOLATED",
+        });
+      } catch (error) {
+        // Swallow errors for host permissions, since this is believed to be a Manifest V3 Chrome bug
+        // @TODO remove when the bugged behaviour is resolved
+        if (
+          error.message !==
+          "Cannot access contents of the page. Extension manifest must request permission to access the respective host."
+        ) {
+          throw error;
+        }
+
+        if (this.platformUtilsService.isDev()) {
+          this.logService.warning(
+            `BrowserApi.executeScriptInTab exception for ${injectDetails.file} in tab ${tabId}: ${error.message}`,
+          );
+        }
+      }
 
       return;
     }

apps/browser/src/popup/services/services.module.ts

@@ -353,7 +353,7 @@ const safeProviders: SafeProvider[] = [
   safeProvider({
     provide: ScriptInjectorService,
     useClass: BrowserScriptInjectorService,
-    deps: [],
+    deps: [PlatformUtilsService, LogService],
   }),
   safeProvider({
     provide: KeyConnectorService,

apps/browser/src/tools/background/fileless-importer.background.spec.ts

@@ -5,6 +5,8 @@ import { PolicyService } from "@bitwarden/common/admin-console/services/policy/p
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
 import { AuthService } from "@bitwarden/common/auth/services/auth.service";
 import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction";
 import { Importer, ImportResult, ImportServiceAbstraction } from "@bitwarden/importer/core";
 
@@ -38,10 +40,12 @@ describe("FilelessImporterBackground ", () => {
   const notificationBackground = mock<NotificationBackground>();
   const importService = mock<ImportServiceAbstraction>();
   const syncService = mock<SyncService>();
+  const platformUtilsService = mock<PlatformUtilsService>();
+  const logService = mock<LogService>();
   let scriptInjectorService: BrowserScriptInjectorService;
 
   beforeEach(() => {
-    scriptInjectorService = new BrowserScriptInjectorService();
+    scriptInjectorService = new BrowserScriptInjectorService(platformUtilsService, logService);
     filelessImporterBackground = new FilelessImporterBackground(
       configService,
       authService,

apps/browser/src/vault/fido2/background/fido2.background.ts

@@ -70,13 +70,13 @@ export class Fido2Background implements Fido2BackgroundInterface {
    */
   async injectFido2ContentScriptsInAllTabs() {
     const tabs = await BrowserApi.tabsQuery({});
+
     for (let index = 0; index < tabs.length; index++) {
       const tab = tabs[index];
-      if (!tab.url?.startsWith("https")) {
-        continue;
-      }
 
-      void this.injectFido2ContentScripts(tab);
+      if (tab.url?.startsWith("https")) {
+        void this.injectFido2ContentScripts(tab);
+      }
     }
   }
 

apps/browser/src/vault/fido2/content/content-script.spec.ts

@@ -15,17 +15,43 @@ jest.mock("../../../autofill/utils", () => ({
   }),
 }));
 
+const originalGlobalThis = globalThis;
+const mockGlobalThisDocument = {
+  ...originalGlobalThis.document,
+  contentType: "text/html",
+  location: {
+    ...originalGlobalThis.document.location,
+    href: "https://localhost",
+    origin: "https://localhost",
+    protocol: "https:",
+  },
+};
+
 describe("Fido2 Content Script", () => {
+  beforeAll(() => {
+    (jest.spyOn(globalThis, "document", "get") as jest.Mock).mockImplementation(
+      () => mockGlobalThisDocument,
+    );
+  });
+
+  afterEach(() => {
+    jest.resetModules();
+  });
+
+  afterAll(() => {
+    jest.clearAllMocks();
+  });
+
   let messenger: Messenger;
   const messengerForDOMCommunicationSpy = jest
     .spyOn(Messenger, "forDOMCommunication")
-    .mockImplementation((window) => {
+    .mockImplementation((context) => {
-      const windowOrigin = window.location.origin;
+      const windowOrigin = context.location.origin;
 
       messenger = new Messenger({
-        postMessage: (message, port) => window.postMessage(message, windowOrigin, [port]),
+        postMessage: (message, port) => context.postMessage(message, windowOrigin, [port]),
-        addEventListener: (listener) => window.addEventListener("message", listener),
+        addEventListener: (listener) => context.addEventListener("message", listener),
-        removeEventListener: (listener) => window.removeEventListener("message", listener),
+        removeEventListener: (listener) => context.removeEventListener("message", listener),
       });
       messenger.destroy = jest.fn();
       return messenger;
@@ -33,16 +59,6 @@ describe("Fido2 Content Script", () => {
   const portSpy: MockProxy<chrome.runtime.Port> = createPortSpyMock(Fido2PortName.InjectedScript);
   chrome.runtime.connect = jest.fn(() => portSpy);
 
-  afterEach(() => {
-    Object.defineProperty(document, "contentType", {
-      value: "text/html",
-      writable: true,
-    });
-
-    jest.clearAllMocks();
-    jest.resetModules();
-  });
-
   it("destroys the messenger when the port is disconnected", () => {
     require("./content-script");
 
@@ -151,11 +167,31 @@ describe("Fido2 Content Script", () => {
     await expect(result).rejects.toEqual(errorMessage);
   });
 
-  it("skips initializing the content script if the document content type is not 'text/html'", () => {
+  it("skips initializing if the document content type is not 'text/html'", () => {
-    Object.defineProperty(document, "contentType", {
+    jest.clearAllMocks();
-      value: "application/json",
+
-      writable: true,
+    (jest.spyOn(globalThis, "document", "get") as jest.Mock).mockImplementation(() => ({
-    });
+      ...mockGlobalThisDocument,
+      contentType: "application/json",
+    }));
+
+    require("./content-script");
+
+    expect(messengerForDOMCommunicationSpy).not.toHaveBeenCalled();
+  });
+
+  it("skips initializing if the document location protocol is not 'https'", () => {
+    jest.clearAllMocks();
+
+    (jest.spyOn(globalThis, "document", "get") as jest.Mock).mockImplementation(() => ({
+      ...mockGlobalThisDocument,
+      location: {
+        ...mockGlobalThisDocument.location,
+        href: "http://localhost",
+        origin: "http://localhost",
+        protocol: "http:",
+      },
+    }));
 
     require("./content-script");
 

apps/browser/src/vault/fido2/content/content-script.ts

@@ -15,7 +15,11 @@ import {
 import { MessageWithMetadata, Messenger } from "./messaging/messenger";
 
 (function (globalContext) {
-  if (globalContext.document.contentType !== "text/html") {
+  const shouldExecuteContentScript =
+    globalContext.document.contentType === "text/html" &&
+    globalContext.document.location.protocol === "https:";
+
+  if (!shouldExecuteContentScript) {
     return;
   }
 

apps/browser/src/vault/fido2/content/page-script.ts

@@ -6,9 +6,14 @@ import { MessageType } from "./messaging/message";
 import { Messenger } from "./messaging/messenger";
 
 (function (globalContext) {
-  if (globalContext.document.contentType !== "text/html") {
+  const shouldExecuteContentScript =
+    globalContext.document.contentType === "text/html" &&
+    globalContext.document.location.protocol === "https:";
+
+  if (!shouldExecuteContentScript) {
     return;
   }
+
   const BrowserPublicKeyCredential = globalContext.PublicKeyCredential;
   const BrowserNavigatorCredentials = navigator.credentials;
   const BrowserAuthenticatorAttestationResponse = globalContext.AuthenticatorAttestationResponse;

apps/browser/src/vault/fido2/content/page-script.webauthn-supported.spec.ts

@@ -10,17 +10,29 @@ import { WebauthnUtils } from "../webauthn-utils";
 import { MessageType } from "./messaging/message";
 import { Messenger } from "./messaging/messenger";
 
+const originalGlobalThis = globalThis;
+const mockGlobalThisDocument = {
+  ...originalGlobalThis.document,
+  contentType: "text/html",
+  location: {
+    ...originalGlobalThis.document.location,
+    href: "https://localhost",
+    origin: "https://localhost",
+    protocol: "https:",
+  },
+};
+
 let messenger: Messenger;
 jest.mock("./messaging/messenger", () => {
   return {
     Messenger: class extends jest.requireActual("./messaging/messenger").Messenger {
-      static forDOMCommunication: any = jest.fn((window) => {
+      static forDOMCommunication: any = jest.fn((context) => {
-        const windowOrigin = window.location.origin;
+        const windowOrigin = context.location.origin;
 
         messenger = new Messenger({
-          postMessage: (message, port) => window.postMessage(message, windowOrigin, [port]),
+          postMessage: (message, port) => context.postMessage(message, windowOrigin, [port]),
-          addEventListener: (listener) => window.addEventListener("message", listener),
+          addEventListener: (listener) => context.addEventListener("message", listener),
-          removeEventListener: (listener) => window.removeEventListener("message", listener),
+          removeEventListener: (listener) => context.removeEventListener("message", listener),
         });
         messenger.destroy = jest.fn();
         return messenger;
@@ -31,6 +43,10 @@ jest.mock("./messaging/messenger", () => {
 jest.mock("../webauthn-utils");
 
 describe("Fido2 page script with native WebAuthn support", () => {
+  (jest.spyOn(globalThis, "document", "get") as jest.Mock).mockImplementation(
+    () => mockGlobalThisDocument,
+  );
+
   const mockCredentialCreationOptions = createCredentialCreationOptionsMock();
   const mockCreateCredentialsResult = createCreateCredentialResultMock();
   const mockCredentialRequestOptions = createCredentialRequestOptionsMock();
@@ -39,9 +55,12 @@ describe("Fido2 page script with native WebAuthn support", () => {
 
   require("./page-script");
 
+  afterEach(() => {
+    jest.resetModules();
+  });
+
   afterAll(() => {
     jest.clearAllMocks();
-    jest.resetModules();
   });
 
   describe("creating WebAuthn credentials", () => {
@@ -118,4 +137,42 @@ describe("Fido2 page script with native WebAuthn support", () => {
       expect(messenger.destroy).toHaveBeenCalled();
     });
   });
+
+  describe("content script execution", () => {
+    beforeEach(() => {
+      jest.clearAllMocks();
+      jest.resetModules();
+    });
+
+    it("skips initializing if the document content type is not 'text/html'", () => {
+      jest.spyOn(Messenger, "forDOMCommunication");
+
+      (jest.spyOn(globalThis, "document", "get") as jest.Mock).mockImplementation(() => ({
+        ...mockGlobalThisDocument,
+        contentType: "json/application",
+      }));
+
+      require("./content-script");
+
+      expect(Messenger.forDOMCommunication).not.toHaveBeenCalled();
+    });
+
+    it("skips initializing if the document location protocol is not 'https'", () => {
+      jest.spyOn(Messenger, "forDOMCommunication");
+
+      (jest.spyOn(globalThis, "document", "get") as jest.Mock).mockImplementation(() => ({
+        ...mockGlobalThisDocument,
+        location: {
+          ...mockGlobalThisDocument.location,
+          href: "http://localhost",
+          origin: "http://localhost",
+          protocol: "http:",
+        },
+      }));
+
+      require("./content-script");
+
+      expect(Messenger.forDOMCommunication).not.toHaveBeenCalled();
+    });
+  });
 });

apps/browser/src/vault/fido2/content/page-script.webauthn-unsupported.spec.ts

@@ -9,17 +9,29 @@ import { WebauthnUtils } from "../webauthn-utils";
 import { MessageType } from "./messaging/message";
 import { Messenger } from "./messaging/messenger";
 
+const originalGlobalThis = globalThis;
+const mockGlobalThisDocument = {
+  ...originalGlobalThis.document,
+  contentType: "text/html",
+  location: {
+    ...originalGlobalThis.document.location,
+    href: "https://localhost",
+    origin: "https://localhost",
+    protocol: "https:",
+  },
+};
+
 let messenger: Messenger;
 jest.mock("./messaging/messenger", () => {
   return {
     Messenger: class extends jest.requireActual("./messaging/messenger").Messenger {
-      static forDOMCommunication: any = jest.fn((window) => {
+      static forDOMCommunication: any = jest.fn((context) => {
-        const windowOrigin = window.location.origin;
+        const windowOrigin = context.location.origin;
 
         messenger = new Messenger({
-          postMessage: (message, port) => window.postMessage(message, windowOrigin, [port]),
+          postMessage: (message, port) => context.postMessage(message, windowOrigin, [port]),
-          addEventListener: (listener) => window.addEventListener("message", listener),
+          addEventListener: (listener) => context.addEventListener("message", listener),
-          removeEventListener: (listener) => window.removeEventListener("message", listener),
+          removeEventListener: (listener) => context.removeEventListener("message", listener),
         });
         messenger.destroy = jest.fn();
         return messenger;
@@ -30,15 +42,22 @@ jest.mock("./messaging/messenger", () => {
 jest.mock("../webauthn-utils");
 
 describe("Fido2 page script without native WebAuthn support", () => {
+  (jest.spyOn(globalThis, "document", "get") as jest.Mock).mockImplementation(
+    () => mockGlobalThisDocument,
+  );
+
   const mockCredentialCreationOptions = createCredentialCreationOptionsMock();
   const mockCreateCredentialsResult = createCreateCredentialResultMock();
   const mockCredentialRequestOptions = createCredentialRequestOptionsMock();
   const mockCredentialAssertResult = createAssertCredentialResultMock();
   require("./page-script");
 
+  afterEach(() => {
+    jest.resetModules();
+  });
+
   afterAll(() => {
     jest.clearAllMocks();
-    jest.resetModules();
   });
 
   describe("creating WebAuthn credentials", () => {