From e594059865d169d0ef5121f1a6ea26d03c4d556a Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Wed, 4 Sep 2024 15:42:37 -0500 Subject: [PATCH] [PM-11474] Rework injection of delayed passkey script to address identified bugs (#10830) * [PM-11474] Delay cleanup of injected passkey script * [PM-11474] Incorporating changes to ensure passkeys function within Safari script effectively * [PM-11474] Fixing jest tests * [PM-11474] Fixing jest tests --- .../fido2-page-script-append.mv2.spec.ts | 22 ++++++++----- .../content/fido2-page-script-append.mv2.ts | 12 ++++--- .../fido2-page-script-delay-append.mv2.ts | 31 ++++++++++++------- 3 files changed, 41 insertions(+), 24 deletions(-) diff --git a/apps/browser/src/autofill/fido2/content/fido2-page-script-append.mv2.spec.ts b/apps/browser/src/autofill/fido2/content/fido2-page-script-append.mv2.spec.ts index d53d9e685e..f5f8dd770c 100644 --- a/apps/browser/src/autofill/fido2/content/fido2-page-script-append.mv2.spec.ts +++ b/apps/browser/src/autofill/fido2/content/fido2-page-script-append.mv2.spec.ts @@ -4,12 +4,21 @@ describe("FIDO2 page-script for manifest v2", () => { let createdScriptElement: HTMLScriptElement; jest.spyOn(window.document, "createElement"); + beforeEach(() => { + jest.useFakeTimers(); + }); + afterEach(() => { Object.defineProperty(window.document, "contentType", { value: "text/html", writable: true }); jest.clearAllMocks(); + jest.clearAllTimers(); jest.resetModules(); }); + afterAll(() => { + jest.useRealTimers(); + }); + it("skips appending the `page-script.js` file if the document contentType is not `text/html`", () => { Object.defineProperty(window.document, "contentType", { value: "text/plain", writable: true }); @@ -19,7 +28,7 @@ describe("FIDO2 page-script for manifest v2", () => { }); it("appends the `page-script.js` file to the document head when the contentType is `text/html`", () => { - jest.spyOn(window.document.head, "insertBefore").mockImplementation((node) => { + jest.spyOn(window.document.head, "prepend").mockImplementation((node) => { createdScriptElement = node as HTMLScriptElement; return node; }); @@ -28,16 +37,13 @@ describe("FIDO2 page-script for manifest v2", () => { expect(window.document.createElement).toHaveBeenCalledWith("script"); expect(chrome.runtime.getURL).toHaveBeenCalledWith(Fido2ContentScript.PageScript); - expect(window.document.head.insertBefore).toHaveBeenCalledWith( - expect.any(HTMLScriptElement), - window.document.head.firstChild, - ); + expect(window.document.head.prepend).toHaveBeenCalledWith(expect.any(HTMLScriptElement)); expect(createdScriptElement.src).toBe(`chrome-extension://id/${Fido2ContentScript.PageScript}`); }); it("appends the `page-script.js` file to the document element if the head is not available", () => { window.document.documentElement.removeChild(window.document.head); - jest.spyOn(window.document.documentElement, "insertBefore").mockImplementation((node) => { + jest.spyOn(window.document.documentElement, "prepend").mockImplementation((node) => { createdScriptElement = node as HTMLScriptElement; return node; }); @@ -46,9 +52,8 @@ describe("FIDO2 page-script for manifest v2", () => { expect(window.document.createElement).toHaveBeenCalledWith("script"); expect(chrome.runtime.getURL).toHaveBeenCalledWith(Fido2ContentScript.PageScript); - expect(window.document.documentElement.insertBefore).toHaveBeenCalledWith( + expect(window.document.documentElement.prepend).toHaveBeenCalledWith( expect.any(HTMLScriptElement), - window.document.documentElement.firstChild, ); expect(createdScriptElement.src).toBe(`chrome-extension://id/${Fido2ContentScript.PageScript}`); }); @@ -63,6 +68,7 @@ describe("FIDO2 page-script for manifest v2", () => { jest.spyOn(createdScriptElement, "remove"); createdScriptElement.dispatchEvent(new Event("load")); + jest.runAllTimers(); expect(createdScriptElement.remove).toHaveBeenCalled(); }); diff --git a/apps/browser/src/autofill/fido2/content/fido2-page-script-append.mv2.ts b/apps/browser/src/autofill/fido2/content/fido2-page-script-append.mv2.ts index 4e806d2990..e5280c088b 100644 --- a/apps/browser/src/autofill/fido2/content/fido2-page-script-append.mv2.ts +++ b/apps/browser/src/autofill/fido2/content/fido2-page-script-append.mv2.ts @@ -2,18 +2,20 @@ * This script handles injection of the FIDO2 override page script into the document. * This is required for manifest v2, but will be removed when we migrate fully to manifest v3. */ -import { Fido2ContentScript } from "../enums/fido2-content-script.enum"; - (function (globalContext) { if (globalContext.document.contentType !== "text/html") { return; } const script = globalContext.document.createElement("script"); - script.src = chrome.runtime.getURL(Fido2ContentScript.PageScript); - script.addEventListener("load", () => script.remove()); + script.src = chrome.runtime.getURL("content/fido2-page-script.js"); + script.addEventListener("load", removeScriptOnLoad); const scriptInsertionPoint = globalContext.document.head || globalContext.document.documentElement; - scriptInsertionPoint.insertBefore(script, scriptInsertionPoint.firstChild); + scriptInsertionPoint.prepend(script); + + function removeScriptOnLoad() { + globalThis.setTimeout(() => script?.remove(), 5000); + } })(globalThis); diff --git a/apps/browser/src/autofill/fido2/content/fido2-page-script-delay-append.mv2.ts b/apps/browser/src/autofill/fido2/content/fido2-page-script-delay-append.mv2.ts index 4afeb76a0d..c75a37c1b6 100644 --- a/apps/browser/src/autofill/fido2/content/fido2-page-script-delay-append.mv2.ts +++ b/apps/browser/src/autofill/fido2/content/fido2-page-script-delay-append.mv2.ts @@ -2,26 +2,35 @@ * This script handles injection of the FIDO2 override page script into the document. * This is required for manifest v2, but will be removed when we migrate fully to manifest v3. */ -import { Fido2ContentScript } from "../enums/fido2-content-script.enum"; - (function (globalContext) { if (globalContext.document.contentType !== "text/html") { return; } - if (globalContext.document.readyState === "complete") { - loadScript(); + const script = globalContext.document.createElement("script"); + script.src = chrome.runtime.getURL("content/fido2-page-script.js"); + script.addEventListener("load", removeScriptOnLoad); + + // We are ensuring that the script injection is delayed in the event that we are loading + // within an iframe element. This prevents an issue with web mail clients that load content + // using ajax within iframes. In particular, Zimbra web mail client was observed to have this issue. + // @see https://github.com/bitwarden/clients/issues/9618 + const delayScriptInjection = + globalContext.window.top !== globalContext.window && + globalContext.document.readyState !== "complete"; + if (delayScriptInjection) { + globalContext.document.addEventListener("DOMContentLoaded", injectScript); } else { - globalContext.addEventListener("DOMContentLoaded", loadScript); + injectScript(); } - function loadScript() { - const script = globalContext.document.createElement("script"); - script.src = chrome.runtime.getURL(Fido2ContentScript.PageScript); - script.addEventListener("load", () => script.remove()); - + function injectScript() { const scriptInsertionPoint = globalContext.document.head || globalContext.document.documentElement; - scriptInsertionPoint.insertBefore(script, scriptInsertionPoint.firstChild); + scriptInsertionPoint.prepend(script); + } + + function removeScriptOnLoad() { + globalThis.setTimeout(() => script?.remove(), 5000); } })(globalThis);