1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-10 13:23:34 +00:00

[PM-26916] inline menu not autofilling email field for oatsovernight.com (#17182)

* PM-26916 utilize opid on focused fields as first validation in order to avoid erroneously filling other similar fields

* extract logic to helper and take totp and multiple forms into account

* run prettier

* avoid filling with opid if already filled

* clean up comments and avoid early return so all fields are scanned

* add tests
This commit is contained in:
Daniel Riera
2025-11-13 10:26:32 -05:00
committed by GitHub
parent c32dee13ca
commit 42a79e65cf
6 changed files with 254 additions and 27 deletions

View File

@@ -47,6 +47,7 @@ export type FocusedFieldData = {
accountCreationFieldType?: string; accountCreationFieldType?: string;
showPasskeys?: boolean; showPasskeys?: boolean;
focusedFieldForm?: string; focusedFieldForm?: string;
focusedFieldOpid?: string;
}; };
export type InlineMenuElementPosition = { export type InlineMenuElementPosition = {

View File

@@ -1176,6 +1176,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
fillNewPassword: true, fillNewPassword: true,
allowTotpAutofill: true, allowTotpAutofill: true,
focusedFieldForm: this.focusedFieldData?.focusedFieldForm, focusedFieldForm: this.focusedFieldData?.focusedFieldForm,
focusedFieldOpid: this.focusedFieldData?.focusedFieldOpid,
}); });
if (totpCode) { if (totpCode) {
@@ -1861,6 +1862,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
fillNewPassword: true, fillNewPassword: true,
allowTotpAutofill: false, allowTotpAutofill: false,
focusedFieldForm: this.focusedFieldData?.focusedFieldForm, focusedFieldForm: this.focusedFieldData?.focusedFieldForm,
focusedFieldOpid: this.focusedFieldData?.focusedFieldOpid,
}); });
globalThis.setTimeout(async () => { globalThis.setTimeout(async () => {

View File

@@ -29,6 +29,7 @@ export interface AutoFillOptions {
allowTotpAutofill?: boolean; allowTotpAutofill?: boolean;
autoSubmitLogin?: boolean; autoSubmitLogin?: boolean;
focusedFieldForm?: string; focusedFieldForm?: string;
focusedFieldOpid?: string;
} }
export interface FormData { export interface FormData {
@@ -47,6 +48,7 @@ export interface GenerateFillScriptOptions {
cipher: CipherView; cipher: CipherView;
tabUrl: string; tabUrl: string;
defaultUriMatch: UriMatchStrategySetting; defaultUriMatch: UriMatchStrategySetting;
focusedFieldOpid?: string;
} }
export type CollectPageDetailsResponseMessage = { export type CollectPageDetailsResponseMessage = {

View File

@@ -975,6 +975,7 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
showPasskeys: !!autofillFieldData?.showPasskeys, showPasskeys: !!autofillFieldData?.showPasskeys,
accountCreationFieldType: autofillFieldData?.accountCreationFieldType, accountCreationFieldType: autofillFieldData?.accountCreationFieldType,
focusedFieldForm: autofillFieldData?.form, focusedFieldForm: autofillFieldData?.form,
focusedFieldOpid: autofillFieldData?.opid,
}; };
const allFields = this.formFieldElements; const allFields = this.formFieldElements;

View File

@@ -50,6 +50,7 @@ import AutofillPageDetails from "../models/autofill-page-details";
import AutofillScript from "../models/autofill-script"; import AutofillScript from "../models/autofill-script";
import { import {
createAutofillFieldMock, createAutofillFieldMock,
createAutofillFormMock,
createAutofillPageDetailsMock, createAutofillPageDetailsMock,
createAutofillScriptMock, createAutofillScriptMock,
createChromeTabMock, createChromeTabMock,
@@ -2309,6 +2310,147 @@ describe("AutofillService", () => {
untrustedIframe: false, untrustedIframe: false,
}); });
}); });
describe("given a focused username field", () => {
let focusedField: AutofillField;
let passwordField: AutofillField;
beforeEach(() => {
focusedField = createAutofillFieldMock({
opid: "focused-username",
type: "text",
form: "form1",
elementNumber: 1,
});
passwordField = createAutofillFieldMock({
opid: "password",
type: "password",
form: "form1",
elementNumber: 2,
});
pageDetails.forms = {
form1: createAutofillFormMock({ opid: "form1" }),
};
options.focusedFieldOpid = "focused-username";
jest.spyOn(autofillService as any, "inUntrustedIframe").mockResolvedValue(false);
jest.spyOn(AutofillService, "fillByOpid");
});
it("will return early when no matching password is found and set autosubmit if enabled", async () => {
pageDetails.fields = [focusedField];
options.autoSubmitLogin = true;
const value = await autofillService["generateLoginFillScript"](
fillScript,
pageDetails,
filledFields,
options,
);
expect(AutofillService.fillByOpid).toHaveBeenCalledTimes(1);
expect(AutofillService.fillByOpid).toHaveBeenCalledWith(
fillScript,
focusedField,
options.cipher.login.username,
);
expect(value.autosubmit).toEqual(["form1"]);
});
it("will prioritize focused field and skip passwords in different forms", async () => {
const otherUsername = createAutofillFieldMock({
opid: "other-username",
type: "text",
form: "form1",
elementNumber: 2,
});
const passwordDifferentForm = createAutofillFieldMock({
opid: "password-different",
type: "password",
form: "form2",
elementNumber: 1,
});
pageDetails.fields = [focusedField, otherUsername, passwordField, passwordDifferentForm];
pageDetails.forms.form2 = createAutofillFormMock({ opid: "form2" });
await autofillService["generateLoginFillScript"](
fillScript,
pageDetails,
filledFields,
options,
);
expect(AutofillService.fillByOpid).toHaveBeenCalledWith(
fillScript,
focusedField,
options.cipher.login.username,
);
expect(AutofillService.fillByOpid).toHaveBeenCalledWith(
fillScript,
passwordField,
options.cipher.login.password,
);
expect(AutofillService.fillByOpid).not.toHaveBeenCalledWith(
fillScript,
otherUsername,
expect.anything(),
);
expect(AutofillService.fillByOpid).not.toHaveBeenCalledWith(
fillScript,
passwordDifferentForm,
expect.anything(),
);
});
it("will not fill focused field if already in filledFields", async () => {
pageDetails.fields = [focusedField, passwordField];
filledFields[focusedField.opid] = focusedField;
await autofillService["generateLoginFillScript"](
fillScript,
pageDetails,
filledFields,
options,
);
expect(AutofillService.fillByOpid).not.toHaveBeenCalledWith(
fillScript,
focusedField,
expect.anything(),
);
expect(AutofillService.fillByOpid).toHaveBeenCalledWith(
fillScript,
passwordField,
options.cipher.login.password,
);
});
it.each([
["email", "email"],
["tel", "tel"],
])("will treat focused %s field as username field", async (type, opid) => {
const focusedTypedField = createAutofillFieldMock({
opid: `focused-${opid}`,
type: type as "email" | "tel",
form: "form1",
elementNumber: 1,
});
pageDetails.fields = [focusedTypedField, passwordField];
options.focusedFieldOpid = `focused-${opid}`;
await autofillService["generateLoginFillScript"](
fillScript,
pageDetails,
filledFields,
options,
);
expect(AutofillService.fillByOpid).toHaveBeenCalledWith(
fillScript,
focusedTypedField,
options.cipher.login.username,
);
});
});
}); });
}); });

View File

@@ -451,6 +451,7 @@ export default class AutofillService implements AutofillServiceInterface {
cipher: options.cipher, cipher: options.cipher,
tabUrl: tab.url, tabUrl: tab.url,
defaultUriMatch: defaultUriMatch, defaultUriMatch: defaultUriMatch,
focusedFieldOpid: options.focusedFieldOpid,
}); });
if (!fillScript || !fillScript.script || !fillScript.script.length) { if (!fillScript || !fillScript.script || !fillScript.script.length) {
@@ -837,7 +838,7 @@ export default class AutofillService implements AutofillServiceInterface {
} }
const passwords: AutofillField[] = []; const passwords: AutofillField[] = [];
const usernames: AutofillField[] = []; const usernames = new Map<string, AutofillField>();
const totps: AutofillField[] = []; const totps: AutofillField[] = [];
let pf: AutofillField = null; let pf: AutofillField = null;
let username: AutofillField = null; let username: AutofillField = null;
@@ -871,6 +872,70 @@ export default class AutofillService implements AutofillServiceInterface {
const prioritizedPasswordFields = const prioritizedPasswordFields =
loginPasswordFields.length > 0 ? loginPasswordFields : registrationPasswordFields; loginPasswordFields.length > 0 ? loginPasswordFields : registrationPasswordFields;
const focusedField =
options.focusedFieldOpid &&
pageDetails.fields.find((f) => f.opid === options.focusedFieldOpid);
const focusedForm = focusedField?.form;
const isFocusedTotpField =
focusedField &&
options.allowTotpAutofill &&
(focusedField.type === "text" ||
focusedField.type === "number" ||
focusedField.type === "tel") &&
(AutofillService.fieldIsFuzzyMatch(focusedField, [
...AutoFillConstants.TotpFieldNames,
...AutoFillConstants.AmbiguousTotpFieldNames,
]) ||
focusedField.autoCompleteType === "one-time-code") &&
!AutofillService.fieldIsFuzzyMatch(focusedField, [
...AutoFillConstants.RecoveryCodeFieldNames,
]);
const focusedUsernameField =
focusedField &&
!isFocusedTotpField &&
login.username &&
(focusedField.type === "text" ||
focusedField.type === "email" ||
focusedField.type === "tel") &&
focusedField;
const passwordMatchesFocused = (pf: AutofillField): boolean =>
!focusedField
? true
: focusedForm != null
? pf.form === focusedForm
: focusedUsernameField &&
pf.form == null &&
this.findUsernameField(pageDetails, pf, false, false, true)?.opid ===
focusedUsernameField.opid;
const getUsernameForPassword = (
pf: AutofillField,
withoutForm: boolean,
): AutofillField | null => {
// use focused username if it matches this password, otherwise fall back to finding username field before password
if (focusedUsernameField && passwordMatchesFocused(pf)) {
return focusedUsernameField;
}
return this.findUsernameField(pageDetails, pf, false, false, withoutForm);
};
if (focusedUsernameField && !prioritizedPasswordFields.some(passwordMatchesFocused)) {
if (!Object.prototype.hasOwnProperty.call(filledFields, focusedUsernameField.opid)) {
filledFields[focusedUsernameField.opid] = focusedUsernameField;
AutofillService.fillByOpid(fillScript, focusedUsernameField, login.username);
if (options.autoSubmitLogin && focusedUsernameField.form) {
fillScript.autosubmit = [focusedUsernameField.form];
}
return AutofillService.setFillScriptForFocus(
{ [focusedUsernameField.opid]: focusedUsernameField },
fillScript,
);
}
}
for (const formKey in pageDetails.forms) { for (const formKey in pageDetails.forms) {
// eslint-disable-next-line // eslint-disable-next-line
if (!pageDetails.forms.hasOwnProperty(formKey)) { if (!pageDetails.forms.hasOwnProperty(formKey)) {
@@ -878,20 +943,25 @@ export default class AutofillService implements AutofillServiceInterface {
} }
prioritizedPasswordFields.forEach((passField) => { prioritizedPasswordFields.forEach((passField) => {
if (focusedField && !passwordMatchesFocused(passField)) {
return;
}
pf = passField; pf = passField;
passwords.push(pf); passwords.push(pf);
if (login.username) { if (login.username) {
username = this.findUsernameField(pageDetails, pf, false, false, false); username = getUsernameForPassword(pf, false);
if (username) { if (username) {
usernames.push(username); usernames.set(username.opid, username);
} }
} }
if (options.allowTotpAutofill && login.totp) { if (options.allowTotpAutofill && login.totp) {
totp = this.findTotpField(pageDetails, pf, false, false, false); totp =
isFocusedTotpField && passwordMatchesFocused(passField)
? focusedField
: this.findTotpField(pageDetails, pf, false, false, false);
if (totp) { if (totp) {
totps.push(totp); totps.push(totp);
} }
@@ -900,27 +970,33 @@ export default class AutofillService implements AutofillServiceInterface {
} }
if (passwordFields.length && !passwords.length) { if (passwordFields.length && !passwords.length) {
// The page does not have any forms with password fields. Use the first password field on the page and the // in the event that password fields exist but weren't processed within form elements.
// input field just before it as the username. // select matching password if focused, otherwise first in prioritized list. for username, use focused field if it matches, otherwise find field before password.
pf = prioritizedPasswordFields[0]; const passwordFieldToUse = focusedField
passwords.push(pf); ? prioritizedPasswordFields.find(passwordMatchesFocused) || prioritizedPasswordFields[0]
: prioritizedPasswordFields[0];
if (login.username && pf.elementNumber > 0) { if (passwordFieldToUse) {
username = this.findUsernameField(pageDetails, pf, false, false, true); passwords.push(passwordFieldToUse);
if (login.username && passwordFieldToUse.elementNumber > 0) {
username = getUsernameForPassword(passwordFieldToUse, true);
if (username) { if (username) {
usernames.push(username); usernames.set(username.opid, username);
} }
} }
if (options.allowTotpAutofill && login.totp && pf.elementNumber > 0) { if (options.allowTotpAutofill && login.totp && passwordFieldToUse.elementNumber > 0) {
totp = this.findTotpField(pageDetails, pf, false, false, true); totp =
isFocusedTotpField && passwordMatchesFocused(passwordFieldToUse)
? focusedField
: this.findTotpField(pageDetails, passwordFieldToUse, false, false, true);
if (totp) { if (totp) {
totps.push(totp); totps.push(totp);
} }
} }
} }
}
if (!passwordFields.length) { if (!passwordFields.length) {
// If there are no passwords, username or TOTP fields may be present. // If there are no passwords, username or TOTP fields may be present.
@@ -951,7 +1027,7 @@ export default class AutofillService implements AutofillServiceInterface {
totps.push(field); totps.push(field);
return; return;
case isFillableUsernameField: case isFillableUsernameField:
usernames.push(field); usernames.set(field.opid, field);
return; return;
default: default:
return; return;
@@ -960,9 +1036,10 @@ export default class AutofillService implements AutofillServiceInterface {
} }
const formElementsSet = new Set<string>(); const formElementsSet = new Set<string>();
usernames.forEach((u) => { const usernamesToFill = focusedUsernameField ? [focusedUsernameField] : [...usernames.values()];
// eslint-disable-next-line
if (filledFields.hasOwnProperty(u.opid)) { usernamesToFill.forEach((u) => {
if (Object.prototype.hasOwnProperty.call(filledFields, u.opid)) {
return; return;
} }
@@ -2330,12 +2407,14 @@ export default class AutofillService implements AutofillServiceInterface {
const includesUsernameFieldName = const includesUsernameFieldName =
this.findMatchingFieldIndex(f, AutoFillConstants.UsernameFieldNames) > -1; this.findMatchingFieldIndex(f, AutoFillConstants.UsernameFieldNames) > -1;
const isInSameForm = f.form === passwordField.form; // only consider fields in same form if both have non-null form values
// null forms are treated as separate
const isInSameForm =
f.form != null && passwordField.form != null && f.form === passwordField.form;
// An email or tel field in the same form as the password field is likely a qualified // An email or tel field in the same form as the password field is likely a qualified
// candidate for autofill, even if visibility checks are unreliable // candidate for autofill, even if visibility checks are unreliable
const isQualifiedUsernameField = const isQualifiedUsernameField = isInSameForm && (f.type === "email" || f.type === "tel");
f.form === passwordField.form && (f.type === "email" || f.type === "tel");
if ( if (
!f.disabled && !f.disabled &&