1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-11-21 11:35:34 +01:00

[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
This commit is contained in:
Jonathan Prusik 2024-01-19 11:47:25 -05:00 committed by GitHub
parent 07af08b893
commit d77e3c3352
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 86 additions and 35 deletions

View File

@ -42,6 +42,8 @@ export class AutoFillConstants {
"verificationCode", "verificationCode",
]; ];
static readonly SearchFieldNames: string[] = ["search", "query", "find", "go"];
static readonly PasswordFieldIgnoreList: string[] = [ static readonly PasswordFieldIgnoreList: string[] = [
"onetimepassword", "onetimepassword",
"captcha", "captcha",
@ -49,9 +51,7 @@ export class AutoFillConstants {
"forgot", "forgot",
]; ];
static readonly ExcludedAutofillTypes: string[] = [ static readonly ExcludedAutofillLoginTypes: string[] = [
"radio",
"checkbox",
"hidden", "hidden",
"file", "file",
"button", "button",
@ -59,6 +59,12 @@ export class AutoFillConstants {
"reset", "reset",
"search", "search",
]; ];
static readonly ExcludedAutofillTypes: string[] = [
"radio",
"checkbox",
...AutoFillConstants.ExcludedAutofillLoginTypes,
];
} }
export class CreditCardAutoFillConstants { export class CreditCardAutoFillConstants {

View File

@ -2260,8 +2260,7 @@ describe("AutofillService", () => {
tagName: "span", tagName: "span",
}); });
pageDetails.fields = [spanField]; pageDetails.fields = [spanField];
jest.spyOn(AutofillService, "forCustomFieldsOnly"); jest.spyOn(AutofillService, "isExcludedField");
jest.spyOn(autofillService as any, "isExcludedType");
const value = autofillService["generateCardFillScript"]( const value = autofillService["generateCardFillScript"](
fillScript, fillScript,
@ -2270,8 +2269,7 @@ describe("AutofillService", () => {
options, options,
); );
expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(spanField); expect(AutofillService["isExcludedField"]).toHaveBeenCalled();
expect(autofillService["isExcludedType"]).not.toHaveBeenCalled();
expect(value).toStrictEqual(unmodifiedFillScriptValues); expect(value).toStrictEqual(unmodifiedFillScriptValues);
}); });
@ -2285,8 +2283,7 @@ describe("AutofillService", () => {
type: excludedType, type: excludedType,
}); });
pageDetails.fields = [invalidField]; pageDetails.fields = [invalidField];
jest.spyOn(AutofillService, "forCustomFieldsOnly"); jest.spyOn(AutofillService, "isExcludedField");
jest.spyOn(autofillService as any, "isExcludedType");
const value = autofillService["generateCardFillScript"]( const value = autofillService["generateCardFillScript"](
fillScript, fillScript,
@ -2295,9 +2292,8 @@ describe("AutofillService", () => {
options, options,
); );
expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(invalidField); expect(AutofillService["isExcludedField"]).toHaveBeenCalledWith(
expect(autofillService["isExcludedType"]).toHaveBeenCalledWith( invalidField,
invalidField.type,
AutoFillConstants.ExcludedAutofillTypes, AutoFillConstants.ExcludedAutofillTypes,
); );
expect(value).toStrictEqual(unmodifiedFillScriptValues); expect(value).toStrictEqual(unmodifiedFillScriptValues);
@ -2315,7 +2311,7 @@ describe("AutofillService", () => {
}); });
pageDetails.fields = [notViewableField]; pageDetails.fields = [notViewableField];
jest.spyOn(AutofillService, "forCustomFieldsOnly"); jest.spyOn(AutofillService, "forCustomFieldsOnly");
jest.spyOn(autofillService as any, "isExcludedType"); jest.spyOn(AutofillService, "isExcludedField");
const value = autofillService["generateCardFillScript"]( const value = autofillService["generateCardFillScript"](
fillScript, fillScript,
@ -2325,7 +2321,7 @@ describe("AutofillService", () => {
); );
expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(notViewableField); expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(notViewableField);
expect(autofillService["isExcludedType"]).toHaveBeenCalled(); expect(AutofillService["isExcludedField"]).toHaveBeenCalled();
expect(value).toStrictEqual(unmodifiedFillScriptValues); expect(value).toStrictEqual(unmodifiedFillScriptValues);
}); });
}); });
@ -2410,7 +2406,7 @@ describe("AutofillService", () => {
options.cipher.card.code = "testCode"; options.cipher.card.code = "testCode";
options.cipher.card.brand = "testBrand"; options.cipher.card.brand = "testBrand";
jest.spyOn(AutofillService, "forCustomFieldsOnly"); 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, "isFieldMatch");
jest.spyOn(autofillService as any, "makeScriptAction"); jest.spyOn(autofillService as any, "makeScriptAction");
jest.spyOn(AutofillService, "hasValue"); jest.spyOn(AutofillService, "hasValue");
@ -2428,7 +2424,7 @@ describe("AutofillService", () => {
); );
expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledTimes(6); expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledTimes(6);
expect(autofillService["isExcludedType"]).toHaveBeenCalledTimes(6); expect(AutofillService["isExcludedField"]).toHaveBeenCalledTimes(6);
expect(AutofillService["isFieldMatch"]).toHaveBeenCalled(); expect(AutofillService["isFieldMatch"]).toHaveBeenCalled();
expect(autofillService["makeScriptAction"]).toHaveBeenCalledTimes(4); expect(autofillService["makeScriptAction"]).toHaveBeenCalledTimes(4);
expect(AutofillService["hasValue"]).toHaveBeenCalledTimes(6); expect(AutofillService["hasValue"]).toHaveBeenCalledTimes(6);
@ -2877,7 +2873,7 @@ describe("AutofillService", () => {
beforeEach(() => { beforeEach(() => {
pageDetails.fields = []; pageDetails.fields = [];
jest.spyOn(AutofillService, "forCustomFieldsOnly"); 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, "isFieldMatch");
jest.spyOn(autofillService as any, "makeScriptAction"); jest.spyOn(autofillService as any, "makeScriptAction");
jest.spyOn(autofillService as any, "makeScriptActionWithValue"); jest.spyOn(autofillService as any, "makeScriptActionWithValue");
@ -2895,7 +2891,7 @@ describe("AutofillService", () => {
); );
expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(customField); expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(customField);
expect(autofillService["isExcludedType"]).not.toHaveBeenCalled(); expect(AutofillService["isExcludedField"]).toHaveBeenCalled();
expect(AutofillService["isFieldMatch"]).not.toHaveBeenCalled(); expect(AutofillService["isFieldMatch"]).not.toHaveBeenCalled();
expect(value.script).toStrictEqual([]); expect(value.script).toStrictEqual([]);
}); });
@ -2912,8 +2908,8 @@ describe("AutofillService", () => {
); );
expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(excludedField); expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(excludedField);
expect(autofillService["isExcludedType"]).toHaveBeenCalledWith( expect(AutofillService["isExcludedField"]).toHaveBeenCalledWith(
excludedField.type, excludedField,
AutoFillConstants.ExcludedAutofillTypes, AutoFillConstants.ExcludedAutofillTypes,
); );
expect(AutofillService["isFieldMatch"]).not.toHaveBeenCalled(); expect(AutofillService["isFieldMatch"]).not.toHaveBeenCalled();
@ -2932,7 +2928,7 @@ describe("AutofillService", () => {
); );
expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(viewableField); expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(viewableField);
expect(autofillService["isExcludedType"]).toHaveBeenCalled(); expect(AutofillService["isExcludedField"]).toHaveBeenCalled();
expect(AutofillService["isFieldMatch"]).not.toHaveBeenCalled(); expect(AutofillService["isFieldMatch"]).not.toHaveBeenCalled();
expect(value.script).toStrictEqual([]); expect(value.script).toStrictEqual([]);
}); });
@ -3314,7 +3310,7 @@ describe("AutofillService", () => {
describe("isExcludedType", () => { describe("isExcludedType", () => {
it("returns true if the passed type is within the excluded type list", () => { it("returns true if the passed type is within the excluded type list", () => {
const value = autofillService["isExcludedType"]( const value = AutofillService["isExcludedType"](
"hidden", "hidden",
AutoFillConstants.ExcludedAutofillTypes, AutoFillConstants.ExcludedAutofillTypes,
); );
@ -3323,7 +3319,7 @@ describe("AutofillService", () => {
}); });
it("returns true if the passed type is within the excluded type list", () => { it("returns true if the passed type is within the excluded type list", () => {
const value = autofillService["isExcludedType"]( const value = AutofillService["isExcludedType"](
"text", "text",
AutoFillConstants.ExcludedAutofillTypes, 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", () => { describe("isFieldMatch", () => {
it("returns true if the passed value is equal to one of the values in the passed options list", () => { it("returns true if the passed value is equal to one of the values in the passed options list", () => {
const passedAttribute = "cc-name"; const passedAttribute = "cc-name";

View File

@ -480,6 +480,11 @@ export default class AutofillService implements AutofillServiceInterface {
return; return;
} }
// Check if the input is an untyped/mistyped search input
if (AutofillService.isSearchField(field)) {
return;
}
const matchingIndex = this.findMatchingFieldIndex(field, fieldNames); const matchingIndex = this.findMatchingFieldIndex(field, fieldNames);
if (matchingIndex > -1) { if (matchingIndex > -1) {
const matchingField: FieldView = fields[matchingIndex]; const matchingField: FieldView = fields[matchingIndex];
@ -732,11 +737,7 @@ export default class AutofillService implements AutofillServiceInterface {
const fillFields: { [id: string]: AutofillField } = {}; const fillFields: { [id: string]: AutofillField } = {};
pageDetails.fields.forEach((f) => { pageDetails.fields.forEach((f) => {
if (AutofillService.forCustomFieldsOnly(f)) { if (AutofillService.isExcludedField(f, AutoFillConstants.ExcludedAutofillTypes)) {
return;
}
if (this.isExcludedType(f.type, AutoFillConstants.ExcludedAutofillTypes)) {
return; return;
} }
@ -1114,11 +1115,7 @@ export default class AutofillService implements AutofillServiceInterface {
const fillFields: { [id: string]: AutofillField } = {}; const fillFields: { [id: string]: AutofillField } = {};
pageDetails.fields.forEach((f) => { pageDetails.fields.forEach((f) => {
if (AutofillService.forCustomFieldsOnly(f)) { if (AutofillService.isExcludedField(f, AutoFillConstants.ExcludedAutofillTypes)) {
return;
}
if (this.isExcludedType(f.type, AutoFillConstants.ExcludedAutofillTypes)) {
return; return;
} }
@ -1343,10 +1340,34 @@ export default class AutofillService implements AutofillServiceInterface {
* @returns {boolean} * @returns {boolean}
* @private * @private
*/ */
private isExcludedType(type: string, excludedTypes: string[]) { private static isExcludedType(type: string, excludedTypes: string[]) {
return excludedTypes.indexOf(type) > -1; 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 * 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 * 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[] = []; const arr: AutofillField[] = [];
pageDetails.fields.forEach((f) => { pageDetails.fields.forEach((f) => {
if (AutofillService.forCustomFieldsOnly(f)) { if (AutofillService.isExcludedField(f, AutoFillConstants.ExcludedAutofillLoginTypes)) {
return; return;
} }