[PM-12775] Password autofill should not occur within 2FA fields (#11303)

This commit is contained in:
Cesar Gonzalez 2024-10-03 16:53:45 -05:00 committed by GitHub
parent f2d3311cb4
commit bf38f2dfee
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 57 additions and 32 deletions

View File

@ -74,7 +74,7 @@ describe("AutofillInit", () => {
Object.defineProperty(document, "readyState", { value: "complete", writable: true });
autofillInit.init();
jest.advanceTimersByTime(250);
jest.advanceTimersByTime(750);
expect(sendExtensionMessageSpy).toHaveBeenCalledWith("bgCollectPageDetails", {
sender: "autofillInit",

View File

@ -78,7 +78,7 @@ class AutofillInit implements AutofillInitInterface {
this.clearCollectPageDetailsOnLoadTimeout();
this.collectPageDetailsOnLoadTimeout = setTimeout(
() => this.sendExtensionMessage("bgCollectPageDetails", { sender: "autofillInit" }),
250,
750,
);
};

View File

@ -34,28 +34,30 @@ export class AutoFillConstants {
"totpcode",
"2facode",
"approvals_code",
"code",
"mfacode",
"otc",
"otc-code",
"otp",
"onetimecode",
"otp-code",
"otpcode",
"pin",
"onetimepassword",
"security_code",
"twofactor",
"twofa",
"twofactorcode",
"verificationCode",
"verification code",
];
static readonly AmbiguousTotpFieldNames: string[] = ["code", "pin", "otc", "otp"];
static readonly SearchFieldNames: string[] = ["search", "query", "find", "go"];
static readonly FieldIgnoreList: string[] = ["captcha", "findanything", "forgot"];
static readonly PasswordFieldExcludeList: string[] = [
"hint",
...AutoFillConstants.FieldIgnoreList,
"onetimepassword",
...AutoFillConstants.TotpFieldNames,
];
static readonly ExcludedAutofillLoginTypes: string[] = [

View File

@ -2260,29 +2260,23 @@ describe("AutofillService", () => {
options,
);
expect(AutofillService.fieldIsFuzzyMatch).toHaveBeenCalledTimes(4);
expect(AutofillService.fieldIsFuzzyMatch).toHaveBeenNthCalledWith(
1,
expect(AutofillService.fieldIsFuzzyMatch).toHaveBeenCalledWith(
usernameField,
AutoFillConstants.UsernameFieldNames,
);
expect(AutofillService.fieldIsFuzzyMatch).toHaveBeenNthCalledWith(
2,
expect(AutofillService.fieldIsFuzzyMatch).toHaveBeenCalledWith(
emailField,
AutoFillConstants.UsernameFieldNames,
);
expect(AutofillService.fieldIsFuzzyMatch).toHaveBeenNthCalledWith(
3,
expect(AutofillService.fieldIsFuzzyMatch).toHaveBeenCalledWith(
telephoneField,
AutoFillConstants.UsernameFieldNames,
);
expect(AutofillService.fieldIsFuzzyMatch).toHaveBeenNthCalledWith(
4,
expect(AutofillService.fieldIsFuzzyMatch).toHaveBeenCalledWith(
totpField,
AutoFillConstants.UsernameFieldNames,
);
expect(AutofillService.fieldIsFuzzyMatch).not.toHaveBeenNthCalledWith(
5,
expect(AutofillService.fieldIsFuzzyMatch).not.toHaveBeenCalledWith(
nonViewableField,
AutoFillConstants.UsernameFieldNames,
);
@ -2328,6 +2322,7 @@ describe("AutofillService", () => {
it("will not attempt to fuzzy match a totp field if totp autofill is not allowed", async () => {
options.allowTotpAutofill = false;
jest.spyOn(autofillService as any, "findMatchingFieldIndex");
await autofillService["generateLoginFillScript"](
fillScript,
@ -2336,7 +2331,7 @@ describe("AutofillService", () => {
options,
);
expect(AutofillService.fieldIsFuzzyMatch).not.toHaveBeenCalledWith(
expect(autofillService["findMatchingFieldIndex"]).not.toHaveBeenCalledWith(
expect.anything(),
AutoFillConstants.TotpFieldNames,
);
@ -2386,7 +2381,6 @@ describe("AutofillService", () => {
false,
false,
);
expect(AutofillService.fieldIsFuzzyMatch).not.toHaveBeenCalled();
expect(AutofillService.fillByOpid).toHaveBeenCalledTimes(2);
expect(AutofillService.fillByOpid).toHaveBeenNthCalledWith(
1,

View File

@ -887,7 +887,10 @@ export default class AutofillService implements AutofillServiceInterface {
options.allowTotpAutofill &&
f.viewable &&
(f.type === "text" || f.type === "number") &&
(AutofillService.fieldIsFuzzyMatch(f, AutoFillConstants.TotpFieldNames) ||
(AutofillService.fieldIsFuzzyMatch(f, [
...AutoFillConstants.TotpFieldNames,
...AutoFillConstants.AmbiguousTotpFieldNames,
]) ||
f.autoCompleteType === "one-time-code")
) {
totps.push(f);
@ -2558,6 +2561,11 @@ export default class AutofillService implements AutofillServiceInterface {
return;
}
// We want to avoid treating TOTP fields as password fields
if (AutofillService.fieldIsFuzzyMatch(f, AutoFillConstants.TotpFieldNames)) {
return;
}
const isLikePassword = () => {
if (f.type !== "text") {
return false;
@ -2670,12 +2678,18 @@ export default class AutofillService implements AutofillServiceInterface {
(withoutForm || f.form === passwordField.form) &&
(canBeHidden || f.viewable) &&
(f.type === "text" || f.type === "number") &&
AutofillService.fieldIsFuzzyMatch(f, AutoFillConstants.TotpFieldNames)
AutofillService.fieldIsFuzzyMatch(f, [
...AutoFillConstants.TotpFieldNames,
...AutoFillConstants.AmbiguousTotpFieldNames,
])
) {
totpField = f;
if (
this.findMatchingFieldIndex(f, AutoFillConstants.TotpFieldNames) > -1 ||
this.findMatchingFieldIndex(f, [
...AutoFillConstants.TotpFieldNames,
...AutoFillConstants.AmbiguousTotpFieldNames,
]) > -1 ||
f.autoCompleteType === "one-time-code"
) {
// We found an exact match. No need to keep looking.

View File

@ -30,7 +30,6 @@ export class InlineMenuFieldQualificationService
this.webAuthnAutocompleteValue,
]);
private fieldIgnoreListString = AutoFillConstants.FieldIgnoreList.join(",");
private passwordFieldExcludeListString = AutoFillConstants.PasswordFieldExcludeList.join(",");
private currentPasswordAutocompleteValue = "current-password";
private newPasswordAutoCompleteValue = "new-password";
private autofillFieldKeywordsMap: AutofillKeywordsMap = new WeakMap();
@ -927,7 +926,7 @@ export class InlineMenuFieldQualificationService
return false;
}
return !(this.passwordFieldExcludeListString.indexOf(cleanedValue) > -1);
return !AutoFillConstants.PasswordFieldExcludeList.some((i) => cleanedValue.indexOf(i) > -1);
}
/**
@ -1094,13 +1093,29 @@ export class InlineMenuFieldQualificationService
];
const keywordsSet = new Set<string>();
for (let i = 0; i < keywords.length; i++) {
if (typeof keywords[i] === "string") {
keywords[i]
.toLowerCase()
.replace(/-/g, "")
.replace(/[^a-zA-Z0-9]+/g, "|")
.split("|")
.forEach((keyword) => keywordsSet.add(keyword));
if (keywords[i] && typeof keywords[i] === "string") {
let keywordEl = keywords[i].toLowerCase();
keywordsSet.add(keywordEl);
// Remove hyphens from all potential keywords, we want to treat these as a single word.
keywordEl = keywordEl.replace(/-/g, "");
// Split the keyword by non-alphanumeric characters to get the keywords without treating a space as a separator.
keywordEl.split(/[^\p{L}\d]+/gu).forEach((keyword) => {
if (keyword) {
keywordsSet.add(keyword);
}
});
// Collapse all spaces and split by non-alphanumeric characters to get the keywords
keywordEl
.replace(/\s/g, "")
.split(/[^\p{L}\d]+/gu)
.forEach((keyword) => {
if (keyword) {
keywordsSet.add(keyword);
}
});
}
}