From ee2f2e1fb13f3039bcc26b14e6a93f0ba0217686 Mon Sep 17 00:00:00 2001 From: Jonathan Prusik Date: Fri, 13 Oct 2023 13:38:48 -0400 Subject: [PATCH] [PM-4127] Bugfix - Check original target tab URL before executing deferred action due to reprompt (#6434) * remove solve for pm-3613 (will readdress in pm-4014) * check original target tab URL before executing deferred action due to reprompt * only check if target tab host+path changed during reprompt --- apps/browser/src/autofill/content/autofill.js | 5 --- .../insert-autofill-content.service.spec.ts | 34 ------------------- .../insert-autofill-content.service.ts | 13 +------ .../popup/components/vault/view.component.ts | 12 ++++++- 4 files changed, 12 insertions(+), 52 deletions(-) diff --git a/apps/browser/src/autofill/content/autofill.js b/apps/browser/src/autofill/content/autofill.js index 2f3857d3fa..ef0fb73408 100644 --- a/apps/browser/src/autofill/content/autofill.js +++ b/apps/browser/src/autofill/content/autofill.js @@ -993,11 +993,6 @@ 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 0ab74875fb..828d768ca2 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,7 +108,6 @@ 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); @@ -120,7 +119,6 @@ describe("InsertAutofillContentService", () => { expect( insertAutofillContentService["userCancelledUntrustedIframeAutofill"] ).not.toHaveBeenCalled(); - expect(insertAutofillContentService["tabURLChanged"]).not.toHaveBeenCalled(); expect(insertAutofillContentService["runFillScriptAction"]).not.toHaveBeenCalled(); }); @@ -130,7 +128,6 @@ 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); @@ -142,7 +139,6 @@ describe("InsertAutofillContentService", () => { expect( insertAutofillContentService["userCancelledUntrustedIframeAutofill"] ).not.toHaveBeenCalled(); - expect(insertAutofillContentService["tabURLChanged"]).not.toHaveBeenCalled(); expect(insertAutofillContentService["runFillScriptAction"]).not.toHaveBeenCalled(); }); @@ -154,7 +150,6 @@ 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); @@ -164,7 +159,6 @@ describe("InsertAutofillContentService", () => { expect( insertAutofillContentService["userCancelledUntrustedIframeAutofill"] ).not.toHaveBeenCalled(); - expect(insertAutofillContentService["tabURLChanged"]).not.toHaveBeenCalled(); expect(insertAutofillContentService["runFillScriptAction"]).not.toHaveBeenCalled(); }); @@ -178,7 +172,6 @@ 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); @@ -188,31 +181,6 @@ 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(); }); @@ -226,7 +194,6 @@ 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); @@ -236,7 +203,6 @@ 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 ad40b76fbc..46cb53d4f5 100644 --- a/apps/browser/src/autofill/services/insert-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/insert-autofill-content.service.ts @@ -38,8 +38,7 @@ class InsertAutofillContentService implements InsertAutofillContentServiceInterf !fillScript.script?.length || this.fillingWithinSandboxedIframe() || this.userCancelledInsecureUrlAutofill(fillScript.savedUrls) || - this.userCancelledUntrustedIframeAutofill(fillScript) || - this.tabURLChanged(fillScript.savedUrls) + this.userCancelledUntrustedIframeAutofill(fillScript) ) { return; } @@ -47,16 +46,6 @@ 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. diff --git a/apps/browser/src/vault/popup/components/vault/view.component.ts b/apps/browser/src/vault/popup/components/vault/view.component.ts index 6c9f3967d5..29027b3350 100644 --- a/apps/browser/src/vault/popup/components/vault/view.component.ts +++ b/apps/browser/src/vault/popup/components/vault/view.component.ts @@ -331,11 +331,21 @@ export class ViewComponent extends BaseViewComponent { } private async doAutofill() { + const originalTabURL = this.tab.url?.length && new URL(this.tab.url); + if (!(await this.promptPassword())) { return false; } - if (this.pageDetails == null || this.pageDetails.length === 0) { + const currentTabURL = this.tab.url?.length && new URL(this.tab.url); + + const originalTabHostPath = + originalTabURL && `${originalTabURL.origin}${originalTabURL.pathname}`; + const currentTabHostPath = currentTabURL && `${currentTabURL.origin}${currentTabURL.pathname}`; + + const tabUrlChanged = originalTabHostPath !== currentTabHostPath; + + if (this.pageDetails == null || this.pageDetails.length === 0 || tabUrlChanged) { this.platformUtilsService.showToast("error", null, this.i18nService.t("autofillError")); return false; }