bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2025-12-15 15:53:27 +00:00
Code Issues Releases Wiki Activity

PM-25471 Remove legacy hidden field autofill behavior (#16319)

Browse Source 
* PM-25471 WIP removed onlyVisible logic in order to always default to visible fields only and not fill hidden fields

* collect page details on autofill for inline menu
This commit is contained in:
Daniel Riera
2025-09-15 14:33:47 -04:00
committed by GitHub
parent 806111c94f
commit 4449cf45d8
6 changed files with 18 additions and 189 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
apps/browser/src/autofill/background/overlay.background.spec.ts
View File

@@ -3389,6 +3389,7 @@ describe("OverlayBackground", () => {
usePasskey: true,
portKey,
});
await flushPromises();
triggerWebRequestOnCompletedEvent(
mock<chrome.webRequest.WebResponseCacheDetails>({
statusCode: 200,

7
apps/browser/src/autofill/background/overlay.background.ts
View File

@@ -1127,11 +1127,16 @@ export class OverlayBackground implements OverlayBackgroundInterface {
{ inlineMenuCipherId, usePasskey }: OverlayPortMessage,
{ sender }: chrome.runtime.Port,
) {
await BrowserApi.tabSendMessage(
sender.tab,
{ command: "collectPageDetails" },
{ frameId: this.focusedFieldData?.frameId },
);
const pageDetailsForTab = this.pageDetailsForTab[sender.tab.id];
if (!inlineMenuCipherId || !pageDetailsForTab?.size) {
return;
}
const cipher = this.inlineMenuCiphers.get(inlineMenuCipherId);
if (usePasskey && cipher.login?.hasFido2Credentials) {
await this.authenticatePasskeyCredential(

2
apps/browser/src/autofill/services/abstractions/autofill.service.ts
View File

@@ -25,7 +25,6 @@ export interface AutoFillOptions {
tab: chrome.tabs.Tab;
skipUsernameOnlyFill?: boolean;
onlyEmptyFields?: boolean;
onlyVisibleFields?: boolean;
fillNewPassword?: boolean;
skipLastUsed?: boolean;
allowUntrustedIframe?: boolean;
@@ -43,7 +42,6 @@ export interface FormData {
export interface GenerateFillScriptOptions {
skipUsernameOnlyFill: boolean;
onlyEmptyFields: boolean;
onlyVisibleFields: boolean;
fillNewPassword: boolean;
allowTotpAutofill: boolean;
autoSubmitLogin: boolean;

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

@@ -741,7 +741,6 @@ describe("AutofillService", () => {
{
skipUsernameOnlyFill: autofillOptions.skipUsernameOnlyFill || false,
onlyEmptyFields: autofillOptions.onlyEmptyFields || false,
onlyVisibleFields: autofillOptions.onlyVisibleFields || false,
fillNewPassword: autofillOptions.fillNewPassword || false,
allowTotpAutofill: autofillOptions.allowTotpAutofill || false,
autoSubmitLogin: autofillOptions.allowTotpAutofill || false,
@@ -1070,7 +1069,6 @@ describe("AutofillService", () => {
skipLastUsed: !fromCommand,
skipUsernameOnlyFill: !fromCommand,
onlyEmptyFields: !fromCommand,
onlyVisibleFields: !fromCommand,
fillNewPassword: fromCommand,
allowUntrustedIframe: fromCommand,
allowTotpAutofill: fromCommand,
@@ -1100,7 +1098,6 @@ describe("AutofillService", () => {
skipLastUsed: !fromCommand,
skipUsernameOnlyFill: !fromCommand,
onlyEmptyFields: !fromCommand,
onlyVisibleFields: !fromCommand,
fillNewPassword: fromCommand,
allowUntrustedIframe: fromCommand,
allowTotpAutofill: fromCommand,
@@ -1127,7 +1124,6 @@ describe("AutofillService", () => {
skipLastUsed: !fromCommand,
skipUsernameOnlyFill: !fromCommand,
onlyEmptyFields: !fromCommand,
onlyVisibleFields: !fromCommand,
fillNewPassword: fromCommand,
allowUntrustedIframe: fromCommand,
allowTotpAutofill: fromCommand,
@@ -1306,7 +1302,6 @@ describe("AutofillService", () => {
skipLastUsed: false,
skipUsernameOnlyFill: false,
onlyEmptyFields: false,
onlyVisibleFields: false,
fillNewPassword: false,
allowUntrustedIframe: true,
allowTotpAutofill: false,
@@ -1353,7 +1348,6 @@ describe("AutofillService", () => {
skipLastUsed: false,
skipUsernameOnlyFill: false,
onlyEmptyFields: false,
onlyVisibleFields: false,
fillNewPassword: false,
allowUntrustedIframe: true,
allowTotpAutofill: false,
@@ -1903,23 +1897,14 @@ describe("AutofillService", () => {
options,
);
expect(AutofillService.loadPasswordFields).toHaveBeenCalledTimes(2);
expect(AutofillService.loadPasswordFields).toHaveBeenNthCalledWith(
1,
expect(AutofillService.loadPasswordFields).toHaveBeenCalledTimes(1);
expect(AutofillService.loadPasswordFields).toHaveBeenCalledWith(
pageDetails,
false,
false,
options.onlyEmptyFields,
options.fillNewPassword,
);
expect(AutofillService.loadPasswordFields).toHaveBeenNthCalledWith(
2,
pageDetails,
true,
true,
options.onlyEmptyFields,
options.fillNewPassword,
);
});
describe("given a valid list of forms within the passed page details", () => {
@@ -1932,36 +1917,7 @@ describe("AutofillService", () => {
jest.spyOn(autofillService as any, "findTotpField");
});
it("will attempt to find a username field from hidden fields if no visible username fields are found", async () => {
await autofillService["generateLoginFillScript"](
fillScript,
pageDetails,
filledFields,
options,
);
expect(autofillService["findUsernameField"]).toHaveBeenCalledTimes(2);
expect(autofillService["findUsernameField"]).toHaveBeenNthCalledWith(
1,
pageDetails,
passwordField,
false,
false,
false,
);
expect(autofillService["findUsernameField"]).toHaveBeenNthCalledWith(
2,
pageDetails,
passwordField,
true,
true,
false,
);
});
it("will not attempt to find a username field from hidden fields if the passed options indicate only visible fields should be referenced", async () => {
options.onlyVisibleFields = true;
it("will attempt to find a username field from visible fields", async () => {
await autofillService["generateLoginFillScript"](
fillScript,
pageDetails,
@@ -1970,25 +1926,16 @@ describe("AutofillService", () => {
);
expect(autofillService["findUsernameField"]).toHaveBeenCalledTimes(1);
expect(autofillService["findUsernameField"]).toHaveBeenNthCalledWith(
1,
expect(autofillService["findUsernameField"]).toHaveBeenCalledWith(
pageDetails,
passwordField,
false,
false,
false,
);
expect(autofillService["findUsernameField"]).not.toHaveBeenNthCalledWith(
2,
pageDetails,
passwordField,
true,
true,
false,
);
});
it("will attempt to find a totp field from hidden fields if no visible totp fields are found", async () => {
it("will attempt to find a totp field from visible fields", async () => {
options.allowTotpAutofill = true;
await autofillService["generateLoginFillScript"](
fillScript,
@@ -1997,53 +1944,14 @@ describe("AutofillService", () => {
options,
);
expect(autofillService["findTotpField"]).toHaveBeenCalledTimes(2);
expect(autofillService["findTotpField"]).toHaveBeenNthCalledWith(
1,
pageDetails,
passwordField,
false,
false,
false,
);
expect(autofillService["findTotpField"]).toHaveBeenNthCalledWith(
2,
pageDetails,
passwordField,
true,
true,
false,
);
});
it("will not attempt to find a totp field from hidden fields if the passed options indicate only visible fields should be referenced", async () => {
options.allowTotpAutofill = true;
options.onlyVisibleFields = true;
await autofillService["generateLoginFillScript"](
fillScript,
pageDetails,
filledFields,
options,
);
expect(autofillService["findTotpField"]).toHaveBeenCalledTimes(1);
expect(autofillService["findTotpField"]).toHaveBeenNthCalledWith(
1,
expect(autofillService["findTotpField"]).toHaveBeenCalledWith(
pageDetails,
passwordField,
false,
false,
false,
);
expect(autofillService["findTotpField"]).not.toHaveBeenNthCalledWith(
2,
pageDetails,
passwordField,
true,
true,
false,
);
});
it("will not attempt to find a totp field from hidden fields if the passed options do not allow for TOTP values to be filled", async () => {
@@ -2085,7 +1993,7 @@ describe("AutofillService", () => {
);
});
it("will attempt to match a password field that does not contain a form to a username field that is not visible", async () => {
it("will not attempt to match a password field that does not contain a form to a username field that is not visible", async () => {
usernameField.viewable = false;
usernameField.readonly = true;
@@ -2096,54 +2004,14 @@ describe("AutofillService", () => {
options,
);
expect(autofillService["findUsernameField"]).toHaveBeenCalledTimes(2);
expect(autofillService["findUsernameField"]).toHaveBeenNthCalledWith(
1,
pageDetails,
passwordField,
false,
false,
true,
);
expect(autofillService["findUsernameField"]).toHaveBeenNthCalledWith(
2,
pageDetails,
passwordField,
true,
true,
true,
);
});
it("will not attempt to match a password field that does not contain a form to a username field that is not visible if the passed options indicate only visible fields", async () => {
usernameField.viewable = false;
usernameField.readonly = true;
options.onlyVisibleFields = true;
await autofillService["generateLoginFillScript"](
fillScript,
pageDetails,
filledFields,
options,
);
expect(autofillService["findUsernameField"]).toHaveBeenCalledTimes(1);
expect(autofillService["findUsernameField"]).toHaveBeenNthCalledWith(
1,
expect(autofillService["findUsernameField"]).toHaveBeenCalledWith(
pageDetails,
passwordField,
false,
false,
true,
);
expect(autofillService["findUsernameField"]).not.toHaveBeenNthCalledWith(
2,
pageDetails,
passwordField,
true,
true,
true,
);
});
it("will attempt to match a password field that does not contain a form to a TOTP field", async () => {
@@ -2166,8 +2034,7 @@ describe("AutofillService", () => {
);
});
it("will attempt to match a password field that does not contain a form to a TOTP field that is not visible", async () => {
options.onlyVisibleFields = false;
it("will not attempt to match a password field that does not contain a form to a TOTP field that is not visible", async () => {
options.allowTotpAutofill = true;
totpField.viewable = false;
totpField.readonly = true;
@@ -2179,7 +2046,7 @@ describe("AutofillService", () => {
options,
);
expect(autofillService["findTotpField"]).toHaveBeenCalledTimes(2);
expect(autofillService["findTotpField"]).toHaveBeenCalledTimes(1);
expect(autofillService["findTotpField"]).toHaveBeenNthCalledWith(
1,
pageDetails,
@@ -2188,14 +2055,6 @@ describe("AutofillService", () => {
false,
true,
);
expect(autofillService["findTotpField"]).toHaveBeenNthCalledWith(
2,
pageDetails,
passwordField,
true,
true,
true,
);
});
});

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

@@ -437,7 +437,6 @@ export default class AutofillService implements AutofillServiceInterface {
const fillScript = await this.generateFillScript(pd.details, {
skipUsernameOnlyFill: options.skipUsernameOnlyFill || false,
onlyEmptyFields: options.onlyEmptyFields || false,
onlyVisibleFields: options.onlyVisibleFields || false,
fillNewPassword: options.fillNewPassword || false,
allowTotpAutofill: options.allowTotpAutofill || false,
autoSubmitLogin: options.autoSubmitLogin || false,
@@ -571,7 +570,6 @@ export default class AutofillService implements AutofillServiceInterface {
skipLastUsed: !fromCommand,
skipUsernameOnlyFill: !fromCommand,
onlyEmptyFields: !fromCommand,
onlyVisibleFields: !fromCommand,
fillNewPassword: fromCommand,
allowUntrustedIframe: fromCommand,
allowTotpAutofill: fromCommand,
@@ -676,7 +674,6 @@ export default class AutofillService implements AutofillServiceInterface {
skipLastUsed: !fromCommand,
skipUsernameOnlyFill: !fromCommand,
onlyEmptyFields: !fromCommand,
onlyVisibleFields: !fromCommand,
fillNewPassword: false,
allowUntrustedIframe: fromCommand,
allowTotpAutofill: false,
@@ -843,23 +840,13 @@ export default class AutofillService implements AutofillServiceInterface {
fillScript.untrustedIframe = await this.inUntrustedIframe(pageDetails.url, options);
let passwordFields = AutofillService.loadPasswordFields(
const passwordFields = AutofillService.loadPasswordFields(
pageDetails,
false,
false,
options.onlyEmptyFields,
options.fillNewPassword,
);
if (!passwordFields.length && !options.onlyVisibleFields) {
// not able to find any viewable password fields. maybe there are some "hidden" ones?
passwordFields = AutofillService.loadPasswordFields(
pageDetails,
true,
true,
options.onlyEmptyFields,
options.fillNewPassword,
);
}
for (const formKey in pageDetails.forms) {
// eslint-disable-next-line
@@ -874,11 +861,6 @@ export default class AutofillService implements AutofillServiceInterface {
if (login.username) {
username = this.findUsernameField(pageDetails, pf, false, false, false);
if (!username && !options.onlyVisibleFields) {
// not able to find any viewable username fields. maybe there are some "hidden" ones?
username = this.findUsernameField(pageDetails, pf, true, true, false);
}
if (username) {
usernames.push(username);
}
@@ -887,11 +869,6 @@ export default class AutofillService implements AutofillServiceInterface {
if (options.allowTotpAutofill && login.totp) {
totp = this.findTotpField(pageDetails, pf, false, false, false);
if (!totp && !options.onlyVisibleFields) {
// not able to find any viewable totp fields. maybe there are some "hidden" ones?
totp = this.findTotpField(pageDetails, pf, true, true, false);
}
if (totp) {
totps.push(totp);
}
@@ -909,11 +886,6 @@ export default class AutofillService implements AutofillServiceInterface {
if (login.username && pf.elementNumber > 0) {
username = this.findUsernameField(pageDetails, pf, false, false, true);
if (!username && !options.onlyVisibleFields) {
// not able to find any viewable username fields. maybe there are some "hidden" ones?
username = this.findUsernameField(pageDetails, pf, true, true, true);
}
if (username) {
usernames.push(username);
}
@@ -922,11 +894,6 @@ export default class AutofillService implements AutofillServiceInterface {
if (options.allowTotpAutofill && login.totp && pf.elementNumber > 0) {
totp = this.findTotpField(pageDetails, pf, false, false, true);
if (!totp && !options.onlyVisibleFields) {
// not able to find any viewable username fields. maybe there are some "hidden" ones?
totp = this.findTotpField(pageDetails, pf, true, true, true);
}
if (totp) {
totps.push(totp);
}

1
apps/browser/src/autofill/spec/autofill-mocks.ts
View File

@@ -114,7 +114,6 @@ export function createGenerateFillScriptOptionsMock(customFields = {}): Generate
return {
skipUsernameOnlyFill: false,
onlyEmptyFields: false,
onlyVisibleFields: false,
fillNewPassword: false,
allowTotpAutofill: false,
autoSubmitLogin: false,