From d77e3c3352a49898b06f4fc4c532308c4bf0e1ea Mon Sep 17 00:00:00 2001 From: Jonathan Prusik Date: Fri, 19 Jan 2024 11:47:25 -0500 Subject: [PATCH] [PM-4437] Bug - Exclude non-standard search inputs from autofill (#7449) * exclude non-standard search inputs from identity autofill * exclude non-standard search inputs from all autofill * check against excluded login field types when loading password fields * clean up logic and add tests * add SearchFieldNames values --- .../autofill/services/autofill-constants.ts | 12 +++- .../services/autofill.service.spec.ts | 64 +++++++++++++------ .../src/autofill/services/autofill.service.ts | 45 +++++++++---- 3 files changed, 86 insertions(+), 35 deletions(-) diff --git a/apps/browser/src/autofill/services/autofill-constants.ts b/apps/browser/src/autofill/services/autofill-constants.ts index cefecc5012..332edf69cf 100644 --- a/apps/browser/src/autofill/services/autofill-constants.ts +++ b/apps/browser/src/autofill/services/autofill-constants.ts @@ -42,6 +42,8 @@ export class AutoFillConstants { "verificationCode", ]; + static readonly SearchFieldNames: string[] = ["search", "query", "find", "go"]; + static readonly PasswordFieldIgnoreList: string[] = [ "onetimepassword", "captcha", @@ -49,9 +51,7 @@ export class AutoFillConstants { "forgot", ]; - static readonly ExcludedAutofillTypes: string[] = [ - "radio", - "checkbox", + static readonly ExcludedAutofillLoginTypes: string[] = [ "hidden", "file", "button", @@ -59,6 +59,12 @@ export class AutoFillConstants { "reset", "search", ]; + + static readonly ExcludedAutofillTypes: string[] = [ + "radio", + "checkbox", + ...AutoFillConstants.ExcludedAutofillLoginTypes, + ]; } export class CreditCardAutoFillConstants { diff --git a/apps/browser/src/autofill/services/autofill.service.spec.ts b/apps/browser/src/autofill/services/autofill.service.spec.ts index 7901fe63b7..cd02e8fcbd 100644 --- a/apps/browser/src/autofill/services/autofill.service.spec.ts +++ b/apps/browser/src/autofill/services/autofill.service.spec.ts @@ -2260,8 +2260,7 @@ describe("AutofillService", () => { tagName: "span", }); pageDetails.fields = [spanField]; - jest.spyOn(AutofillService, "forCustomFieldsOnly"); - jest.spyOn(autofillService as any, "isExcludedType"); + jest.spyOn(AutofillService, "isExcludedField"); const value = autofillService["generateCardFillScript"]( fillScript, @@ -2270,8 +2269,7 @@ describe("AutofillService", () => { options, ); - expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(spanField); - expect(autofillService["isExcludedType"]).not.toHaveBeenCalled(); + expect(AutofillService["isExcludedField"]).toHaveBeenCalled(); expect(value).toStrictEqual(unmodifiedFillScriptValues); }); @@ -2285,8 +2283,7 @@ describe("AutofillService", () => { type: excludedType, }); pageDetails.fields = [invalidField]; - jest.spyOn(AutofillService, "forCustomFieldsOnly"); - jest.spyOn(autofillService as any, "isExcludedType"); + jest.spyOn(AutofillService, "isExcludedField"); const value = autofillService["generateCardFillScript"]( fillScript, @@ -2295,9 +2292,8 @@ describe("AutofillService", () => { options, ); - expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(invalidField); - expect(autofillService["isExcludedType"]).toHaveBeenCalledWith( - invalidField.type, + expect(AutofillService["isExcludedField"]).toHaveBeenCalledWith( + invalidField, AutoFillConstants.ExcludedAutofillTypes, ); expect(value).toStrictEqual(unmodifiedFillScriptValues); @@ -2315,7 +2311,7 @@ describe("AutofillService", () => { }); pageDetails.fields = [notViewableField]; jest.spyOn(AutofillService, "forCustomFieldsOnly"); - jest.spyOn(autofillService as any, "isExcludedType"); + jest.spyOn(AutofillService, "isExcludedField"); const value = autofillService["generateCardFillScript"]( fillScript, @@ -2325,7 +2321,7 @@ describe("AutofillService", () => { ); expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(notViewableField); - expect(autofillService["isExcludedType"]).toHaveBeenCalled(); + expect(AutofillService["isExcludedField"]).toHaveBeenCalled(); expect(value).toStrictEqual(unmodifiedFillScriptValues); }); }); @@ -2410,7 +2406,7 @@ describe("AutofillService", () => { options.cipher.card.code = "testCode"; options.cipher.card.brand = "testBrand"; jest.spyOn(AutofillService, "forCustomFieldsOnly"); - jest.spyOn(autofillService as any, "isExcludedType"); + jest.spyOn(AutofillService, "isExcludedField"); jest.spyOn(AutofillService as any, "isFieldMatch"); jest.spyOn(autofillService as any, "makeScriptAction"); jest.spyOn(AutofillService, "hasValue"); @@ -2428,7 +2424,7 @@ describe("AutofillService", () => { ); expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledTimes(6); - expect(autofillService["isExcludedType"]).toHaveBeenCalledTimes(6); + expect(AutofillService["isExcludedField"]).toHaveBeenCalledTimes(6); expect(AutofillService["isFieldMatch"]).toHaveBeenCalled(); expect(autofillService["makeScriptAction"]).toHaveBeenCalledTimes(4); expect(AutofillService["hasValue"]).toHaveBeenCalledTimes(6); @@ -2877,7 +2873,7 @@ describe("AutofillService", () => { beforeEach(() => { pageDetails.fields = []; jest.spyOn(AutofillService, "forCustomFieldsOnly"); - jest.spyOn(autofillService as any, "isExcludedType"); + jest.spyOn(AutofillService, "isExcludedField"); jest.spyOn(AutofillService as any, "isFieldMatch"); jest.spyOn(autofillService as any, "makeScriptAction"); jest.spyOn(autofillService as any, "makeScriptActionWithValue"); @@ -2895,7 +2891,7 @@ describe("AutofillService", () => { ); expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(customField); - expect(autofillService["isExcludedType"]).not.toHaveBeenCalled(); + expect(AutofillService["isExcludedField"]).toHaveBeenCalled(); expect(AutofillService["isFieldMatch"]).not.toHaveBeenCalled(); expect(value.script).toStrictEqual([]); }); @@ -2912,8 +2908,8 @@ describe("AutofillService", () => { ); expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(excludedField); - expect(autofillService["isExcludedType"]).toHaveBeenCalledWith( - excludedField.type, + expect(AutofillService["isExcludedField"]).toHaveBeenCalledWith( + excludedField, AutoFillConstants.ExcludedAutofillTypes, ); expect(AutofillService["isFieldMatch"]).not.toHaveBeenCalled(); @@ -2932,7 +2928,7 @@ describe("AutofillService", () => { ); expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(viewableField); - expect(autofillService["isExcludedType"]).toHaveBeenCalled(); + expect(AutofillService["isExcludedField"]).toHaveBeenCalled(); expect(AutofillService["isFieldMatch"]).not.toHaveBeenCalled(); expect(value.script).toStrictEqual([]); }); @@ -3314,7 +3310,7 @@ describe("AutofillService", () => { describe("isExcludedType", () => { it("returns true if the passed type is within the excluded type list", () => { - const value = autofillService["isExcludedType"]( + const value = AutofillService["isExcludedType"]( "hidden", AutoFillConstants.ExcludedAutofillTypes, ); @@ -3323,7 +3319,7 @@ describe("AutofillService", () => { }); it("returns true if the passed type is within the excluded type list", () => { - const value = autofillService["isExcludedType"]( + const value = AutofillService["isExcludedType"]( "text", AutoFillConstants.ExcludedAutofillTypes, ); @@ -3332,6 +3328,34 @@ describe("AutofillService", () => { }); }); + describe("isSearchField", () => { + it("returns true if the passed field type is 'search'", () => { + const typedSearchField = createAutofillFieldMock({ type: "search" }); + const value = AutofillService["isSearchField"](typedSearchField); + + expect(value).toBe(true); + }); + + it("returns true if the passed field type is missing and another checked attribute value contains a reference to search", () => { + const untypedSearchField = createAutofillFieldMock({ + htmlID: "aSearchInput", + placeholder: null, + type: null, + value: null, + }); + const value = AutofillService["isSearchField"](untypedSearchField); + + expect(value).toBe(true); + }); + + it("returns false if the passed field is not a search field", () => { + const typedSearchField = createAutofillFieldMock(); + const value = AutofillService["isSearchField"](typedSearchField); + + expect(value).toBe(false); + }); + }); + describe("isFieldMatch", () => { it("returns true if the passed value is equal to one of the values in the passed options list", () => { const passedAttribute = "cc-name"; diff --git a/apps/browser/src/autofill/services/autofill.service.ts b/apps/browser/src/autofill/services/autofill.service.ts index 11c3bafad8..f14dd0c14c 100644 --- a/apps/browser/src/autofill/services/autofill.service.ts +++ b/apps/browser/src/autofill/services/autofill.service.ts @@ -480,6 +480,11 @@ export default class AutofillService implements AutofillServiceInterface { return; } + // Check if the input is an untyped/mistyped search input + if (AutofillService.isSearchField(field)) { + return; + } + const matchingIndex = this.findMatchingFieldIndex(field, fieldNames); if (matchingIndex > -1) { const matchingField: FieldView = fields[matchingIndex]; @@ -732,11 +737,7 @@ export default class AutofillService implements AutofillServiceInterface { const fillFields: { [id: string]: AutofillField } = {}; pageDetails.fields.forEach((f) => { - if (AutofillService.forCustomFieldsOnly(f)) { - return; - } - - if (this.isExcludedType(f.type, AutoFillConstants.ExcludedAutofillTypes)) { + if (AutofillService.isExcludedField(f, AutoFillConstants.ExcludedAutofillTypes)) { return; } @@ -1114,11 +1115,7 @@ export default class AutofillService implements AutofillServiceInterface { const fillFields: { [id: string]: AutofillField } = {}; pageDetails.fields.forEach((f) => { - if (AutofillService.forCustomFieldsOnly(f)) { - return; - } - - if (this.isExcludedType(f.type, AutoFillConstants.ExcludedAutofillTypes)) { + if (AutofillService.isExcludedField(f, AutoFillConstants.ExcludedAutofillTypes)) { return; } @@ -1343,10 +1340,34 @@ export default class AutofillService implements AutofillServiceInterface { * @returns {boolean} * @private */ - private isExcludedType(type: string, excludedTypes: string[]) { + private static isExcludedType(type: string, excludedTypes: string[]) { return excludedTypes.indexOf(type) > -1; } + private static isSearchField(field: AutofillField) { + const matchFieldAttributeValues = [field.type, field.htmlName, field.htmlID, field.placeholder]; + const matchPattern = new RegExp(AutoFillConstants.SearchFieldNames.join("|"), "gi"); + + return Boolean(matchFieldAttributeValues.join(" ").match(matchPattern)); + } + + static isExcludedField(field: AutofillField, excludedTypes: string[]) { + if (AutofillService.forCustomFieldsOnly(field)) { + return true; + } + + if (this.isExcludedType(field.type, excludedTypes)) { + return true; + } + + // Check if the input is an untyped/mistyped search input + if (this.isSearchField(field)) { + return true; + } + + return false; + } + /** * Accepts the value of a field, a list of possible options that define if * a field can be matched to a vault cipher, and a secondary optional list @@ -1476,7 +1497,7 @@ export default class AutofillService implements AutofillServiceInterface { ) { const arr: AutofillField[] = []; pageDetails.fields.forEach((f) => { - if (AutofillService.forCustomFieldsOnly(f)) { + if (AutofillService.isExcludedField(f, AutoFillConstants.ExcludedAutofillLoginTypes)) { return; }