diff --git a/apps/browser/src/autofill/services/autofill.service.spec.ts b/apps/browser/src/autofill/services/autofill.service.spec.ts index d1fbf79bfaa..158fde6a567 100644 --- a/apps/browser/src/autofill/services/autofill.service.spec.ts +++ b/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(); const userVerificationService = mock(); const billingAccountProfileStateService = mock(); + const platformUtilsService = mock(); beforeEach(() => { - scriptInjectorService = new BrowserScriptInjectorService(); + scriptInjectorService = new BrowserScriptInjectorService(platformUtilsService, logService); autofillService = new AutofillService( cipherService, autofillSettingsService, diff --git a/apps/browser/src/autofill/services/autofill.service.ts b/apps/browser/src/autofill/services/autofill.service.ts index 10e2d84361b..dd875054414 100644 --- a/apps/browser/src/autofill/services/autofill.service.ts +++ b/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 { - // Autofill settings loaded from state can await the active account state indefinitely if - // not guarded by an active account check (e.g. the user is logged in) + // Autofill user settings loaded from state can await the active account state indefinitely + // 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. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.injectAutofillScripts(tab, 0, false); + void this.injectAutofillScripts(tab, 0, false); } } } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index f86a44bc8f9..067fae1de8f 100644 --- a/apps/browser/src/background/main.background.ts +++ b/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, diff --git a/apps/browser/src/manifest.v3.json b/apps/browser/src/manifest.v3.json index b7aaba2e0e0..93d798490f7 100644 --- a/apps/browser/src/manifest.v3.json +++ b/apps/browser/src/manifest.v3.json @@ -51,7 +51,7 @@ "default_popup": "popup/index.html" }, "permissions": [ - "", + "activeTab", "tabs", "contextMenus", "storage", @@ -65,7 +65,7 @@ "webRequestAuthProvider" ], "optional_permissions": ["nativeMessaging", "privacy"], - "host_permissions": [""], + "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'" diff --git a/apps/browser/src/platform/background/service-factories/browser-script-injector-service.factory.ts b/apps/browser/src/platform/background/service-factories/browser-script-injector-service.factory.ts index e3bc687f280..e9a8ee379a8 100644 --- a/apps/browser/src/platform/background/service-factories/browser-script-injector-service.factory.ts +++ b/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), + ), ); } diff --git a/apps/browser/src/platform/services/browser-script-injector.service.spec.ts b/apps/browser/src/platform/services/browser-script-injector.service.spec.ts index 6ae84c64646..d6ec3dfde96 100644 --- a/apps/browser/src/platform/services/browser-script-injector.service.spec.ts +++ b/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(); + const logService = mock(); beforeEach(() => { - scriptInjectorService = new BrowserScriptInjectorService(); + scriptInjectorService = new BrowserScriptInjectorService(platformUtilsService, logService); }); describe("inject", () => { diff --git a/apps/browser/src/platform/services/browser-script-injector.service.ts b/apps/browser/src/platform/services/browser-script-injector.service.ts index 54513188d58..5b3a10ef2bb 100644 --- a/apps/browser/src/platform/services/browser-script-injector.service.ts +++ b/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, { - world: mv3Details?.world ?? "ISOLATED", - }); + try { + 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; } diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 268ee9fa81b..36cbdf6292f 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/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, diff --git a/apps/browser/src/tools/background/fileless-importer.background.spec.ts b/apps/browser/src/tools/background/fileless-importer.background.spec.ts index 7d226fcd9d4..7b356b18fd5 100644 --- a/apps/browser/src/tools/background/fileless-importer.background.spec.ts +++ b/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(); const importService = mock(); const syncService = mock(); + const platformUtilsService = mock(); + const logService = mock(); let scriptInjectorService: BrowserScriptInjectorService; beforeEach(() => { - scriptInjectorService = new BrowserScriptInjectorService(); + scriptInjectorService = new BrowserScriptInjectorService(platformUtilsService, logService); filelessImporterBackground = new FilelessImporterBackground( configService, authService, diff --git a/apps/browser/src/vault/fido2/background/fido2.background.ts b/apps/browser/src/vault/fido2/background/fido2.background.ts index 856874cee3d..5e51e05d776 100644 --- a/apps/browser/src/vault/fido2/background/fido2.background.ts +++ b/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); + } } } diff --git a/apps/browser/src/vault/fido2/content/content-script.spec.ts b/apps/browser/src/vault/fido2/content/content-script.spec.ts index 29d3e9c257a..0c2a52ed101 100644 --- a/apps/browser/src/vault/fido2/content/content-script.spec.ts +++ b/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) => { - const windowOrigin = window.location.origin; + .mockImplementation((context) => { + const windowOrigin = context.location.origin; messenger = new Messenger({ - postMessage: (message, port) => window.postMessage(message, windowOrigin, [port]), - addEventListener: (listener) => window.addEventListener("message", listener), - removeEventListener: (listener) => window.removeEventListener("message", listener), + postMessage: (message, port) => context.postMessage(message, windowOrigin, [port]), + addEventListener: (listener) => context.addEventListener("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 = 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'", () => { - Object.defineProperty(document, "contentType", { - value: "application/json", - writable: true, - }); + it("skips initializing if the document content type is not 'text/html'", () => { + jest.clearAllMocks(); + + (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"); diff --git a/apps/browser/src/vault/fido2/content/content-script.ts b/apps/browser/src/vault/fido2/content/content-script.ts index 809db115533..fe3aafe9fb8 100644 --- a/apps/browser/src/vault/fido2/content/content-script.ts +++ b/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; } diff --git a/apps/browser/src/vault/fido2/content/page-script.ts b/apps/browser/src/vault/fido2/content/page-script.ts index 1de0f3258a4..5b04f7c1dd8 100644 --- a/apps/browser/src/vault/fido2/content/page-script.ts +++ b/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; diff --git a/apps/browser/src/vault/fido2/content/page-script.webauthn-supported.spec.ts b/apps/browser/src/vault/fido2/content/page-script.webauthn-supported.spec.ts index 211959c4663..c235d53cb05 100644 --- a/apps/browser/src/vault/fido2/content/page-script.webauthn-supported.spec.ts +++ b/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) => { - const windowOrigin = window.location.origin; + static forDOMCommunication: any = jest.fn((context) => { + const windowOrigin = context.location.origin; messenger = new Messenger({ - postMessage: (message, port) => window.postMessage(message, windowOrigin, [port]), - addEventListener: (listener) => window.addEventListener("message", listener), - removeEventListener: (listener) => window.removeEventListener("message", listener), + postMessage: (message, port) => context.postMessage(message, windowOrigin, [port]), + addEventListener: (listener) => context.addEventListener("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(); + }); + }); }); diff --git a/apps/browser/src/vault/fido2/content/page-script.webauthn-unsupported.spec.ts b/apps/browser/src/vault/fido2/content/page-script.webauthn-unsupported.spec.ts index f3aee685e1c..4b1f839a1d0 100644 --- a/apps/browser/src/vault/fido2/content/page-script.webauthn-unsupported.spec.ts +++ b/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) => { - const windowOrigin = window.location.origin; + static forDOMCommunication: any = jest.fn((context) => { + const windowOrigin = context.location.origin; messenger = new Messenger({ - postMessage: (message, port) => window.postMessage(message, windowOrigin, [port]), - addEventListener: (listener) => window.addEventListener("message", listener), - removeEventListener: (listener) => window.removeEventListener("message", listener), + postMessage: (message, port) => context.postMessage(message, windowOrigin, [port]), + addEventListener: (listener) => context.addEventListener("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", () => {