Upstream/bitwarden-browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser.git synced 2024-06-20 09:35:22 +02:00
Code Issues Projects Releases Wiki Activity

[PM-5531] Improve captcha field autofill disqualification (#7581)

Browse Source 
* improve captcha field autofill disqualification

* add tests
This commit is contained in:
Jonathan Prusik 2024-01-19 12:38:23 -05:00 committed by GitHub
parent d85485e5cb
commit 487d17daed
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 114 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
apps/browser/src/autofill/services/autofill-constants.ts
View File

@ -44,11 +44,11 @@ export class AutoFillConstants {
static readonly SearchFieldNames: string[] = ["search", "query", "find", "go"];
static readonly PasswordFieldIgnoreList: string[] = [
static readonly FieldIgnoreList: string[] = ["captcha", "findanything", "forgot"];
static readonly PasswordFieldExcludeList: string[] = [
...AutoFillConstants.FieldIgnoreList,
"onetimepassword",
"captcha",
"findanything",
"forgot",
];
static readonly ExcludedAutofillLoginTypes: string[] = [

62
apps/browser/src/autofill/services/autofill.service.spec.ts
View File

@ -2282,7 +2282,7 @@ describe("AutofillService", () => {
tagName: "span",
});
pageDetails.fields = [spanField];
jest.spyOn(AutofillService, "isExcludedField");
jest.spyOn(AutofillService, "isExcludedFieldType");
const value = autofillService["generateCardFillScript"](
fillScript,
@ -2291,7 +2291,7 @@ describe("AutofillService", () => {
options,
);
expect(AutofillService["isExcludedField"]).toHaveBeenCalled();
expect(AutofillService["isExcludedFieldType"]).toHaveBeenCalled();
expect(value).toStrictEqual(unmodifiedFillScriptValues);
});
@ -2305,7 +2305,7 @@ describe("AutofillService", () => {
type: excludedType,
});
pageDetails.fields = [invalidField];
jest.spyOn(AutofillService, "isExcludedField");
jest.spyOn(AutofillService, "isExcludedFieldType");
const value = autofillService["generateCardFillScript"](
fillScript,
@ -2314,7 +2314,7 @@ describe("AutofillService", () => {
options,
);
expect(AutofillService["isExcludedField"]).toHaveBeenCalledWith(
expect(AutofillService["isExcludedFieldType"]).toHaveBeenCalledWith(
invalidField,
AutoFillConstants.ExcludedAutofillTypes,
);
@ -2333,7 +2333,7 @@ describe("AutofillService", () => {
});
pageDetails.fields = [notViewableField];
jest.spyOn(AutofillService, "forCustomFieldsOnly");
jest.spyOn(AutofillService, "isExcludedField");
jest.spyOn(AutofillService, "isExcludedFieldType");
const value = autofillService["generateCardFillScript"](
fillScript,
@ -2343,7 +2343,7 @@ describe("AutofillService", () => {
);
expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(notViewableField);
expect(AutofillService["isExcludedField"]).toHaveBeenCalled();
expect(AutofillService["isExcludedFieldType"]).toHaveBeenCalled();
expect(value).toStrictEqual(unmodifiedFillScriptValues);
});
});
@ -2428,7 +2428,7 @@ describe("AutofillService", () => {
options.cipher.card.code = "testCode";
options.cipher.card.brand = "testBrand";
jest.spyOn(AutofillService, "forCustomFieldsOnly");
jest.spyOn(AutofillService, "isExcludedField");
jest.spyOn(AutofillService, "isExcludedFieldType");
jest.spyOn(AutofillService as any, "isFieldMatch");
jest.spyOn(autofillService as any, "makeScriptAction");
jest.spyOn(AutofillService, "hasValue");
@ -2446,7 +2446,7 @@ describe("AutofillService", () => {
);
expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledTimes(6);
expect(AutofillService["isExcludedField"]).toHaveBeenCalledTimes(6);
expect(AutofillService["isExcludedFieldType"]).toHaveBeenCalledTimes(6);
expect(AutofillService["isFieldMatch"]).toHaveBeenCalled();
expect(autofillService["makeScriptAction"]).toHaveBeenCalledTimes(4);
expect(AutofillService["hasValue"]).toHaveBeenCalledTimes(6);
@ -2895,7 +2895,7 @@ describe("AutofillService", () => {
beforeEach(() => {
pageDetails.fields = [];
jest.spyOn(AutofillService, "forCustomFieldsOnly");
jest.spyOn(AutofillService, "isExcludedField");
jest.spyOn(AutofillService, "isExcludedFieldType");
jest.spyOn(AutofillService as any, "isFieldMatch");
jest.spyOn(autofillService as any, "makeScriptAction");
jest.spyOn(autofillService as any, "makeScriptActionWithValue");
@ -2913,7 +2913,7 @@ describe("AutofillService", () => {
);
expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(customField);
expect(AutofillService["isExcludedField"]).toHaveBeenCalled();
expect(AutofillService["isExcludedFieldType"]).toHaveBeenCalled();
expect(AutofillService["isFieldMatch"]).not.toHaveBeenCalled();
expect(value.script).toStrictEqual([]);
});
@ -2930,7 +2930,7 @@ describe("AutofillService", () => {
);
expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(excludedField);
expect(AutofillService["isExcludedField"]).toHaveBeenCalledWith(
expect(AutofillService["isExcludedFieldType"]).toHaveBeenCalledWith(
excludedField,
AutoFillConstants.ExcludedAutofillTypes,
);
@ -2950,7 +2950,7 @@ describe("AutofillService", () => {
);
expect(AutofillService.forCustomFieldsOnly).toHaveBeenCalledWith(viewableField);
expect(AutofillService["isExcludedField"]).toHaveBeenCalled();
expect(AutofillService["isExcludedFieldType"]).toHaveBeenCalled();
expect(AutofillService["isFieldMatch"]).not.toHaveBeenCalled();
expect(value.script).toStrictEqual([]);
});
@ -3690,6 +3690,15 @@ describe("AutofillService", () => {
expect(result).toStrictEqual([passwordField]);
});
it("returns the an empty array if the field's htmlID contains the words `password` and `captcha`", () => {
passwordField.htmlID = "inputPasswordCaptcha";
pageDetails.fields = [passwordField];
const result = AutofillService.loadPasswordFields(pageDetails, false, false, false, false);
expect(result).toStrictEqual([]);
});
it("returns the field in an array if the field's htmlName contains the word `password`", () => {
passwordField.htmlName = "password";
pageDetails.fields = [passwordField];
@ -3699,6 +3708,15 @@ describe("AutofillService", () => {
expect(result).toStrictEqual([passwordField]);
});
it("returns the an empty array if the field's htmlName contains the words `password` and `captcha`", () => {
passwordField.htmlName = "inputPasswordCaptcha";
pageDetails.fields = [passwordField];
const result = AutofillService.loadPasswordFields(pageDetails, false, false, false, false);
expect(result).toStrictEqual([]);
});
it("returns the field in an array if the field's placeholder contains the word `password`", () => {
passwordField.placeholder = "password";
pageDetails.fields = [passwordField];
@ -3707,6 +3725,26 @@ describe("AutofillService", () => {
expect(result).toStrictEqual([passwordField]);
});
it("returns the an empty array if the field's placeholder contains the words `password` and `captcha`", () => {
passwordField.placeholder = "inputPasswordCaptcha";
pageDetails.fields = [passwordField];
const result = AutofillService.loadPasswordFields(pageDetails, false, false, false, false);
expect(result).toStrictEqual([]);
});
it("returns the an empty array if any of the field's checked attributed contain the words `captcha` while any other attribute contains the word `password` and no excluded terms", () => {
passwordField.htmlID = "inputPasswordCaptcha";
passwordField.htmlName = "captcha";
passwordField.placeholder = "Enter password";
pageDetails.fields = [passwordField];
const result = AutofillService.loadPasswordFields(pageDetails, false, false, false, false);
expect(result).toStrictEqual([]);
});
});
describe("given a field that is not viewable", () => {

84
apps/browser/src/autofill/services/autofill.service.ts
View File

@ -736,7 +736,7 @@ export default class AutofillService implements AutofillServiceInterface {
const fillFields: { [id: string]: AutofillField } = {};
pageDetails.fields.forEach((f) => {
if (AutofillService.isExcludedField(f, AutoFillConstants.ExcludedAutofillTypes)) {
if (AutofillService.isExcludedFieldType(f, AutoFillConstants.ExcludedAutofillTypes)) {
return;
}
@ -1114,7 +1114,7 @@ export default class AutofillService implements AutofillServiceInterface {
const fillFields: { [id: string]: AutofillField } = {};
pageDetails.fields.forEach((f) => {
if (AutofillService.isExcludedField(f, AutoFillConstants.ExcludedAutofillTypes)) {
if (AutofillService.isExcludedFieldType(f, AutoFillConstants.ExcludedAutofillTypes)) {
return;
}
@ -1350,7 +1350,7 @@ export default class AutofillService implements AutofillServiceInterface {
return Boolean(matchFieldAttributeValues.join(" ").match(matchPattern));
}
static isExcludedField(field: AutofillField, excludedTypes: string[]) {
static isExcludedFieldType(field: AutofillField, excludedTypes: string[]) {
if (AutofillService.forCustomFieldsOnly(field)) {
return true;
}
@ -1477,6 +1477,44 @@ export default class AutofillService implements AutofillServiceInterface {
}
}
static valueIsLikePassword(value: string) {
if (value == null) {
return false;
}
// Removes all whitespace, _ and - characters
const cleanedValue = value.toLowerCase().replace(/[\s_-]/g, "");
if (cleanedValue.indexOf("password") < 0) {
return false;
}
if (AutoFillConstants.PasswordFieldExcludeList.some((i) => cleanedValue.indexOf(i) > -1)) {
return false;
}
return true;
}
static fieldHasDisqualifyingAttributeValue(field: AutofillField) {
const checkedAttributeValues = [field.htmlID, field.htmlName, field.placeholder];
let valueIsOnExclusionList = false;
for (let i = 0; i < checkedAttributeValues.length; i++) {
const checkedAttributeValue = checkedAttributeValues[i];
const cleanedValue = checkedAttributeValue?.toLowerCase().replace(/[\s_-]/g, "");
valueIsOnExclusionList = Boolean(
cleanedValue && AutoFillConstants.FieldIgnoreList.some((i) => cleanedValue.indexOf(i) > -1),
);
if (valueIsOnExclusionList) {
break;
}
}
return valueIsOnExclusionList;
}
/**
* Accepts a pageDetails object with a list of fields and returns a list of
* fields that are likely to be password fields.
@ -1495,45 +1533,39 @@ export default class AutofillService implements AutofillServiceInterface {
fillNewPassword: boolean,
) {
const arr: AutofillField[] = [];
pageDetails.fields.forEach((f) => {
if (AutofillService.isExcludedField(f, AutoFillConstants.ExcludedAutofillLoginTypes)) {
if (AutofillService.isExcludedFieldType(f, AutoFillConstants.ExcludedAutofillLoginTypes)) {
return;
}
// If any attribute values match disqualifying values, the entire field should not be used
if (AutofillService.fieldHasDisqualifyingAttributeValue(f)) {
return;
}
const isPassword = f.type === "password";
const valueIsLikePassword = (value: string) => {
if (value == null) {
return false;
}
// Removes all whitespace, _ and - characters
// eslint-disable-next-line
const cleanedValue = value.toLowerCase().replace(/[\s_\-]/g, "");
if (cleanedValue.indexOf("password") < 0) {
return false;
}
if (AutoFillConstants.PasswordFieldIgnoreList.some((i) => cleanedValue.indexOf(i) > -1)) {
return false;
}
return true;
};
const isLikePassword = () => {
if (f.type !== "text") {
return false;
}
if (valueIsLikePassword(f.htmlID)) {
if (AutofillService.valueIsLikePassword(f.htmlID)) {
return true;
}
if (valueIsLikePassword(f.htmlName)) {
if (AutofillService.valueIsLikePassword(f.htmlName)) {
return true;
}
if (valueIsLikePassword(f.placeholder)) {
if (AutofillService.valueIsLikePassword(f.placeholder)) {
return true;
}
return false;
};
if (
!f.disabled &&
(canBeReadOnly || !f.readonly) &&
@ -1545,6 +1577,7 @@ export default class AutofillService implements AutofillServiceInterface {
arr.push(f);
}
});
return arr;
}
@ -1621,7 +1654,10 @@ export default class AutofillService implements AutofillServiceInterface {
continue;
}
const fieldIsDisqualified = AutofillService.fieldHasDisqualifyingAttributeValue(f);
if (
!fieldIsDisqualified &&
!f.disabled &&
(canBeReadOnly || !f.readonly) &&
(withoutForm || f.form === passwordField.form) &&