This commit is contained in:
Jonathan Prusik 2024-04-25 18:45:07 -04:00 committed by GitHub
commit e5e57626bf
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 229 additions and 61 deletions

View File

@ -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,

View File

@ -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
// 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);
}
}
}

View File

@ -806,7 +806,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,

View File

@ -51,7 +51,6 @@
"default_popup": "popup/index.html"
},
"permissions": [
"<all_urls>",
"tabs",
"contextMenus",
"storage",
@ -63,7 +62,7 @@
"offscreen"
],
"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'"

View File

@ -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),
),
);
}

View File

@ -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", () => {

View File

@ -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,24 @@ 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 ({ message }) {
// 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 (
message ===
"Cannot access contents of the page. Extension manifest must request permission to access the respective host."
) {
if (this.platformUtilsService.isDev()) {
this.logService.warning(
`BrowserApi.executeScriptInTab exception for ${injectDetails.file} in tab ${tabId}: ${message}`,
);
}
}
}
return;
}

View File

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

View File

@ -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,

View File

@ -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);
}
}
}

View File

@ -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<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'", () => {
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");

View File

@ -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;
}

View File

@ -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;

View File

@ -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();
});
});
});

View File

@ -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", () => {