[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
This commit is contained in:
Jonathan Prusik 2023-10-13 13:38:48 -04:00 committed by GitHub
parent 95d4406a7e
commit ee2f2e1fb1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 12 additions and 52 deletions

View File

@ -993,11 +993,6 @@
function fillTheElement(el, op) { function fillTheElement(el, op) {
var shouldCheck; var shouldCheck;
if (el && null !== op && void 0 !== op && !(el.disabled || el.a || el.readOnly)) { 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), switch (markTheFilling && el.form && !el.form.opfilled && (el.form.opfilled = true),
el.type ? el.type.toLowerCase() : null) { el.type ? el.type.toLowerCase() : null) {
case 'checkbox': case 'checkbox':

View File

@ -108,7 +108,6 @@ describe("InsertAutofillContentService", () => {
jest.spyOn(insertAutofillContentService as any, "fillingWithinSandboxedIframe"); jest.spyOn(insertAutofillContentService as any, "fillingWithinSandboxedIframe");
jest.spyOn(insertAutofillContentService as any, "userCancelledInsecureUrlAutofill"); jest.spyOn(insertAutofillContentService as any, "userCancelledInsecureUrlAutofill");
jest.spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill"); jest.spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill");
jest.spyOn(insertAutofillContentService as any, "tabURLChanged");
jest.spyOn(insertAutofillContentService as any, "runFillScriptAction"); jest.spyOn(insertAutofillContentService as any, "runFillScriptAction");
insertAutofillContentService.fillForm(fillScript); insertAutofillContentService.fillForm(fillScript);
@ -120,7 +119,6 @@ describe("InsertAutofillContentService", () => {
expect( expect(
insertAutofillContentService["userCancelledUntrustedIframeAutofill"] insertAutofillContentService["userCancelledUntrustedIframeAutofill"]
).not.toHaveBeenCalled(); ).not.toHaveBeenCalled();
expect(insertAutofillContentService["tabURLChanged"]).not.toHaveBeenCalled();
expect(insertAutofillContentService["runFillScriptAction"]).not.toHaveBeenCalled(); expect(insertAutofillContentService["runFillScriptAction"]).not.toHaveBeenCalled();
}); });
@ -130,7 +128,6 @@ describe("InsertAutofillContentService", () => {
.mockReturnValue(true); .mockReturnValue(true);
jest.spyOn(insertAutofillContentService as any, "userCancelledInsecureUrlAutofill"); jest.spyOn(insertAutofillContentService as any, "userCancelledInsecureUrlAutofill");
jest.spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill"); jest.spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill");
jest.spyOn(insertAutofillContentService as any, "tabURLChanged");
jest.spyOn(insertAutofillContentService as any, "runFillScriptAction"); jest.spyOn(insertAutofillContentService as any, "runFillScriptAction");
insertAutofillContentService.fillForm(fillScript); insertAutofillContentService.fillForm(fillScript);
@ -142,7 +139,6 @@ describe("InsertAutofillContentService", () => {
expect( expect(
insertAutofillContentService["userCancelledUntrustedIframeAutofill"] insertAutofillContentService["userCancelledUntrustedIframeAutofill"]
).not.toHaveBeenCalled(); ).not.toHaveBeenCalled();
expect(insertAutofillContentService["tabURLChanged"]).not.toHaveBeenCalled();
expect(insertAutofillContentService["runFillScriptAction"]).not.toHaveBeenCalled(); expect(insertAutofillContentService["runFillScriptAction"]).not.toHaveBeenCalled();
}); });
@ -154,7 +150,6 @@ describe("InsertAutofillContentService", () => {
.spyOn(insertAutofillContentService as any, "userCancelledInsecureUrlAutofill") .spyOn(insertAutofillContentService as any, "userCancelledInsecureUrlAutofill")
.mockReturnValue(true); .mockReturnValue(true);
jest.spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill"); jest.spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill");
jest.spyOn(insertAutofillContentService as any, "tabURLChanged");
jest.spyOn(insertAutofillContentService as any, "runFillScriptAction"); jest.spyOn(insertAutofillContentService as any, "runFillScriptAction");
insertAutofillContentService.fillForm(fillScript); insertAutofillContentService.fillForm(fillScript);
@ -164,7 +159,6 @@ describe("InsertAutofillContentService", () => {
expect( expect(
insertAutofillContentService["userCancelledUntrustedIframeAutofill"] insertAutofillContentService["userCancelledUntrustedIframeAutofill"]
).not.toHaveBeenCalled(); ).not.toHaveBeenCalled();
expect(insertAutofillContentService["tabURLChanged"]).not.toHaveBeenCalled();
expect(insertAutofillContentService["runFillScriptAction"]).not.toHaveBeenCalled(); expect(insertAutofillContentService["runFillScriptAction"]).not.toHaveBeenCalled();
}); });
@ -178,7 +172,6 @@ describe("InsertAutofillContentService", () => {
jest jest
.spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill") .spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill")
.mockReturnValue(true); .mockReturnValue(true);
jest.spyOn(insertAutofillContentService as any, "tabURLChanged").mockReturnValue(false);
jest.spyOn(insertAutofillContentService as any, "runFillScriptAction"); jest.spyOn(insertAutofillContentService as any, "runFillScriptAction");
insertAutofillContentService.fillForm(fillScript); insertAutofillContentService.fillForm(fillScript);
@ -188,31 +181,6 @@ describe("InsertAutofillContentService", () => {
expect( expect(
insertAutofillContentService["userCancelledUntrustedIframeAutofill"] insertAutofillContentService["userCancelledUntrustedIframeAutofill"]
).toHaveBeenCalled(); ).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(); expect(insertAutofillContentService["runFillScriptAction"]).not.toHaveBeenCalled();
}); });
@ -226,7 +194,6 @@ describe("InsertAutofillContentService", () => {
jest jest
.spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill") .spyOn(insertAutofillContentService as any, "userCancelledUntrustedIframeAutofill")
.mockReturnValue(false); .mockReturnValue(false);
jest.spyOn(insertAutofillContentService as any, "tabURLChanged").mockReturnValue(false);
jest.spyOn(insertAutofillContentService as any, "runFillScriptAction"); jest.spyOn(insertAutofillContentService as any, "runFillScriptAction");
insertAutofillContentService.fillForm(fillScript); insertAutofillContentService.fillForm(fillScript);
@ -236,7 +203,6 @@ describe("InsertAutofillContentService", () => {
expect( expect(
insertAutofillContentService["userCancelledUntrustedIframeAutofill"] insertAutofillContentService["userCancelledUntrustedIframeAutofill"]
).toHaveBeenCalled(); ).toHaveBeenCalled();
expect(insertAutofillContentService["tabURLChanged"]).toHaveBeenCalled();
expect(insertAutofillContentService["runFillScriptAction"]).toHaveBeenCalledTimes(3); expect(insertAutofillContentService["runFillScriptAction"]).toHaveBeenCalledTimes(3);
expect(insertAutofillContentService["runFillScriptAction"]).toHaveBeenNthCalledWith( expect(insertAutofillContentService["runFillScriptAction"]).toHaveBeenNthCalledWith(
1, 1,

View File

@ -38,8 +38,7 @@ class InsertAutofillContentService implements InsertAutofillContentServiceInterf
!fillScript.script?.length || !fillScript.script?.length ||
this.fillingWithinSandboxedIframe() || this.fillingWithinSandboxedIframe() ||
this.userCancelledInsecureUrlAutofill(fillScript.savedUrls) || this.userCancelledInsecureUrlAutofill(fillScript.savedUrls) ||
this.userCancelledUntrustedIframeAutofill(fillScript) || this.userCancelledUntrustedIframeAutofill(fillScript)
this.tabURLChanged(fillScript.savedUrls)
) { ) {
return; return;
} }
@ -47,16 +46,6 @@ class InsertAutofillContentService implements InsertAutofillContentServiceInterf
fillScript.script.forEach(this.runFillScriptAction); 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 * Identifies if the execution of this script is happening
* within a sandboxed iframe. * within a sandboxed iframe.

View File

@ -331,11 +331,21 @@ export class ViewComponent extends BaseViewComponent {
} }
private async doAutofill() { private async doAutofill() {
const originalTabURL = this.tab.url?.length && new URL(this.tab.url);
if (!(await this.promptPassword())) { if (!(await this.promptPassword())) {
return false; 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")); this.platformUtilsService.showToast("error", null, this.i18nService.t("autofillError"));
return false; return false;
} }