From 621ffa01aad57356050559d4b7f8b34cdd786c80 Mon Sep 17 00:00:00 2001 From: Jonathan Prusik Date: Tue, 19 Sep 2023 16:37:21 -0400 Subject: [PATCH] [PM-3613] Check for page change before delayed auto-fill action execution (#6280) * check for page change before delayed auto-fill action execution * update test --- apps/browser/src/autofill/content/autofill.js | 7 +++- .../insert-autofill-content.service.spec.ts | 34 +++++++++++++++++++ .../insert-autofill-content.service.ts | 13 ++++++- 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/apps/browser/src/autofill/content/autofill.js b/apps/browser/src/autofill/content/autofill.js index 1833c09e15..2f3857d3fa 100644 --- a/apps/browser/src/autofill/content/autofill.js +++ b/apps/browser/src/autofill/content/autofill.js @@ -986,13 +986,18 @@ styleTimeout = 200; /** - * Fll an element `el` using the value `op` from the fill script + * Fill an element `el` using the value `op` from the fill script * @param {HTMLElement} el * @param {string} op */ function fillTheElement(el, op) { var shouldCheck; if (el && null !== op && void 0 !== op && !(el.disabled || el.a || el.readOnly)) { + const tabURLChanged = !fillScript.savedUrls?.some(url => url.startsWith(window.location.origin)) + // Check to make sure the page location didn't change + if (tabURLChanged) { + return; + } switch (markTheFilling && el.form && !el.form.opfilled && (el.form.opfilled = true), el.type ? el.type.toLowerCase() : null) { case 'checkbox': diff --git a/apps/browser/src/autofill/services/insert-autofill-content.service.spec.ts b/apps/browser/src/autofill/services/insert-autofill-content.service.spec.ts index 828d768ca2..0ab74875fb 100644 --- a/apps/browser/src/autofill/services/insert-autofill-content.service.spec.ts +++ b/apps/browser/src/autofill/services/insert-autofill-content.service.spec.ts @@ -108,6 +108,7 @@ describe("InsertAutofillContentService", () => { jest.spyOn(insertAutofillContentService as any, "fillingWithinSandboxedIframe"); jest.spyOn(insertAutofillContentService as any, "userCancelledInsecureUrlAutofill"); jest.spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill"); + jest.spyOn(insertAutofillContentService as any, "tabURLChanged"); jest.spyOn(insertAutofillContentService as any, "runFillScriptAction"); insertAutofillContentService.fillForm(fillScript); @@ -119,6 +120,7 @@ describe("InsertAutofillContentService", () => { expect( insertAutofillContentService["userCancelledUntrustedIframeAutofill"] ).not.toHaveBeenCalled(); + expect(insertAutofillContentService["tabURLChanged"]).not.toHaveBeenCalled(); expect(insertAutofillContentService["runFillScriptAction"]).not.toHaveBeenCalled(); }); @@ -128,6 +130,7 @@ describe("InsertAutofillContentService", () => { .mockReturnValue(true); jest.spyOn(insertAutofillContentService as any, "userCancelledInsecureUrlAutofill"); jest.spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill"); + jest.spyOn(insertAutofillContentService as any, "tabURLChanged"); jest.spyOn(insertAutofillContentService as any, "runFillScriptAction"); insertAutofillContentService.fillForm(fillScript); @@ -139,6 +142,7 @@ describe("InsertAutofillContentService", () => { expect( insertAutofillContentService["userCancelledUntrustedIframeAutofill"] ).not.toHaveBeenCalled(); + expect(insertAutofillContentService["tabURLChanged"]).not.toHaveBeenCalled(); expect(insertAutofillContentService["runFillScriptAction"]).not.toHaveBeenCalled(); }); @@ -150,6 +154,7 @@ describe("InsertAutofillContentService", () => { .spyOn(insertAutofillContentService as any, "userCancelledInsecureUrlAutofill") .mockReturnValue(true); jest.spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill"); + jest.spyOn(insertAutofillContentService as any, "tabURLChanged"); jest.spyOn(insertAutofillContentService as any, "runFillScriptAction"); insertAutofillContentService.fillForm(fillScript); @@ -159,6 +164,7 @@ describe("InsertAutofillContentService", () => { expect( insertAutofillContentService["userCancelledUntrustedIframeAutofill"] ).not.toHaveBeenCalled(); + expect(insertAutofillContentService["tabURLChanged"]).not.toHaveBeenCalled(); expect(insertAutofillContentService["runFillScriptAction"]).not.toHaveBeenCalled(); }); @@ -172,6 +178,7 @@ describe("InsertAutofillContentService", () => { jest .spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill") .mockReturnValue(true); + jest.spyOn(insertAutofillContentService as any, "tabURLChanged").mockReturnValue(false); jest.spyOn(insertAutofillContentService as any, "runFillScriptAction"); insertAutofillContentService.fillForm(fillScript); @@ -181,6 +188,31 @@ describe("InsertAutofillContentService", () => { expect( insertAutofillContentService["userCancelledUntrustedIframeAutofill"] ).toHaveBeenCalled(); + expect(insertAutofillContentService["tabURLChanged"]).not.toHaveBeenCalled(); + expect(insertAutofillContentService["runFillScriptAction"]).not.toHaveBeenCalled(); + }); + + it("returns early if the page location origin does not match against any of the cipher saved URLs", () => { + jest + .spyOn(insertAutofillContentService as any, "fillingWithinSandboxedIframe") + .mockReturnValue(false); + jest + .spyOn(insertAutofillContentService as any, "userCancelledInsecureUrlAutofill") + .mockReturnValue(false); + jest + .spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill") + .mockReturnValue(false); + jest.spyOn(insertAutofillContentService as any, "tabURLChanged").mockReturnValue(true); + jest.spyOn(insertAutofillContentService as any, "runFillScriptAction"); + + insertAutofillContentService.fillForm(fillScript); + + expect(insertAutofillContentService["fillingWithinSandboxedIframe"]).toHaveBeenCalled(); + expect(insertAutofillContentService["userCancelledInsecureUrlAutofill"]).toHaveBeenCalled(); + expect( + insertAutofillContentService["userCancelledUntrustedIframeAutofill"] + ).toHaveBeenCalled(); + expect(insertAutofillContentService["tabURLChanged"]).toHaveBeenCalled(); expect(insertAutofillContentService["runFillScriptAction"]).not.toHaveBeenCalled(); }); @@ -194,6 +226,7 @@ describe("InsertAutofillContentService", () => { jest .spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill") .mockReturnValue(false); + jest.spyOn(insertAutofillContentService as any, "tabURLChanged").mockReturnValue(false); jest.spyOn(insertAutofillContentService as any, "runFillScriptAction"); insertAutofillContentService.fillForm(fillScript); @@ -203,6 +236,7 @@ describe("InsertAutofillContentService", () => { expect( insertAutofillContentService["userCancelledUntrustedIframeAutofill"] ).toHaveBeenCalled(); + expect(insertAutofillContentService["tabURLChanged"]).toHaveBeenCalled(); expect(insertAutofillContentService["runFillScriptAction"]).toHaveBeenCalledTimes(3); expect(insertAutofillContentService["runFillScriptAction"]).toHaveBeenNthCalledWith( 1, diff --git a/apps/browser/src/autofill/services/insert-autofill-content.service.ts b/apps/browser/src/autofill/services/insert-autofill-content.service.ts index 89f644ba6b..4e47e73704 100644 --- a/apps/browser/src/autofill/services/insert-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/insert-autofill-content.service.ts @@ -38,7 +38,8 @@ class InsertAutofillContentService implements InsertAutofillContentServiceInterf !fillScript.script?.length || this.fillingWithinSandboxedIframe() || this.userCancelledInsecureUrlAutofill(fillScript.savedUrls) || - this.userCancelledUntrustedIframeAutofill(fillScript) + this.userCancelledUntrustedIframeAutofill(fillScript) || + this.tabURLChanged(fillScript.savedUrls) ) { return; } @@ -46,6 +47,16 @@ class InsertAutofillContentService implements InsertAutofillContentServiceInterf fillScript.script.forEach(this.runFillScriptAction); } + /** + * Determines if the page URL no longer matches one of the cipher's savedURL domains + * @param {string[] | null} savedUrls + * @returns {boolean} + * @private + */ + private tabURLChanged(savedUrls?: AutofillScript["savedUrls"]): boolean { + return savedUrls && !savedUrls.some((url) => url.startsWith(window.location.origin)); + } + /** * Identifies if the execution of this script is happening * within a sandboxed iframe.