From 8ead45f5344d13ae5667d9b97d529675b9037f07 Mon Sep 17 00:00:00 2001 From: Miles Blackwood Date: Thu, 1 May 2025 14:37:24 -0400 Subject: [PATCH] PM-17391 (#14323) * Establish patterns needing addressing. * Prevent non-password field misidentification (TOTP as username) * Allow 'tel' type for TOTP fill identification * Resolve todo and document. * Remove duplicated line. * Use account creation field type = totp to determine if focused field receives the inline menu for totp. Handles cases where totp can appear alongside login forms and also ensures general cipher and totp cipher inline menus render distinctly when focus changes between the field types necessitating. * Prevent type edge cases. --- .../autofill/background/overlay.background.ts | 26 ++-- .../autofill/enums/autofill-overlay.enum.ts | 1 + .../autofill-overlay-content.service.ts | 5 + .../src/autofill/services/autofill.service.ts | 119 +++++++++--------- 4 files changed, 84 insertions(+), 67 deletions(-) diff --git a/apps/browser/src/autofill/background/overlay.background.ts b/apps/browser/src/autofill/background/overlay.background.ts index 4e2e773a0c7..ab5dd4abb8f 100644 --- a/apps/browser/src/autofill/background/overlay.background.ts +++ b/apps/browser/src/autofill/background/overlay.background.ts @@ -661,20 +661,23 @@ export class OverlayBackground implements OverlayBackgroundInterface { return this.inlineMenuFido2Credentials.has(credentialId); } + /** + * When focused field data contains account creation field type of totp + * and there are totp fields in the current frame for page details return true + * + * @returns boolean + */ private isTotpFieldForCurrentField(): boolean { if (!this.focusedFieldData) { return false; } - const { tabId, frameId } = this.focusedFieldData; - const pageDetailsMap = this.pageDetailsForTab[tabId]; - if (!pageDetailsMap || !pageDetailsMap.has(frameId)) { + const totpFields = this.getTotpFields(); + if (!totpFields) { return false; } - const pageDetail = pageDetailsMap.get(frameId); return ( - pageDetail?.details?.fields?.every((field) => - this.inlineMenuFieldQualificationService.isTotpField(field), - ) || false + totpFields.length > 0 && + this.focusedFieldData?.accountCreationFieldType === InlineMenuAccountCreationFieldType.Totp ); } @@ -1399,7 +1402,7 @@ export class OverlayBackground implements OverlayBackgroundInterface { const pageDetailsMap = this.pageDetailsForTab[currentTabId]; const pageDetails = pageDetailsMap?.get(currentFrameId); - const fields = pageDetails.details.fields; + const fields = pageDetails?.details?.fields || []; const totpFields = fields.filter((f) => this.inlineMenuFieldQualificationService.isTotpField(f), ); @@ -1679,7 +1682,12 @@ export class OverlayBackground implements OverlayBackgroundInterface { !this.focusedFieldMatchesFillType( focusedFieldData?.inlineMenuFillType, previousFocusedFieldData, - ) + ) || + // a TOTP field was just focused to - or unfocused from — a non-TOTP field + // may want to generalize this logic if cipher inline menu types exceed [general cipher, TOTP] + [focusedFieldData, previousFocusedFieldData].filter( + (fd) => fd?.accountCreationFieldType === InlineMenuAccountCreationFieldType.Totp, + ).length === 1 ) { const updateAllCipherTypes = !this.focusedFieldMatchesFillType( CipherType.Login, diff --git a/apps/browser/src/autofill/enums/autofill-overlay.enum.ts b/apps/browser/src/autofill/enums/autofill-overlay.enum.ts index 66ad0da546d..9cc457f3c1a 100644 --- a/apps/browser/src/autofill/enums/autofill-overlay.enum.ts +++ b/apps/browser/src/autofill/enums/autofill-overlay.enum.ts @@ -32,6 +32,7 @@ export const InlineMenuAccountCreationFieldType = { Text: "text", Email: "email", Password: "password", + Totp: "totp", } as const; export type InlineMenuAccountCreationFieldTypes = diff --git a/apps/browser/src/autofill/services/autofill-overlay-content.service.ts b/apps/browser/src/autofill/services/autofill-overlay-content.service.ts index 55260cc1149..dc8f45d104b 100644 --- a/apps/browser/src/autofill/services/autofill-overlay-content.service.ts +++ b/apps/browser/src/autofill/services/autofill-overlay-content.service.ts @@ -1128,6 +1128,11 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ * @param autofillFieldData - Autofill field data captured from the form field element. */ private qualifyAccountCreationFieldType(autofillFieldData: AutofillField) { + if (this.inlineMenuFieldQualificationService.isTotpField(autofillFieldData)) { + autofillFieldData.accountCreationFieldType = InlineMenuAccountCreationFieldType.Totp; + return; + } + if (!this.inlineMenuFieldQualificationService.isUsernameField(autofillFieldData)) { autofillFieldData.accountCreationFieldType = InlineMenuAccountCreationFieldType.Password; return; diff --git a/apps/browser/src/autofill/services/autofill.service.ts b/apps/browser/src/autofill/services/autofill.service.ts index 0423078fd1c..da46ceb0864 100644 --- a/apps/browser/src/autofill/services/autofill.service.ts +++ b/apps/browser/src/autofill/services/autofill.service.ts @@ -931,28 +931,37 @@ export default class AutofillService implements AutofillServiceInterface { } if (!passwordFields.length) { - // No password fields on this page. Let's try to just fuzzy fill the username. - pageDetails.fields.forEach((f) => { - if ( - !options.skipUsernameOnlyFill && - f.viewable && - (f.type === "text" || f.type === "email" || f.type === "tel") && - AutofillService.fieldIsFuzzyMatch(f, AutoFillConstants.UsernameFieldNames) - ) { - usernames.push(f); + // If there are no passwords, username or TOTP fields may be present. + // username and TOTP fields are mutually exclusive + pageDetails.fields.forEach((field) => { + if (!field.viewable) { + return; } - if ( + const isFillableTotpField = options.allowTotpAutofill && - f.viewable && - (f.type === "text" || f.type === "number") && - (AutofillService.fieldIsFuzzyMatch(f, [ + ["number", "tel", "text"].some((t) => t === field.type) && + (AutofillService.fieldIsFuzzyMatch(field, [ ...AutoFillConstants.TotpFieldNames, ...AutoFillConstants.AmbiguousTotpFieldNames, ]) || - f.autoCompleteType === "one-time-code") - ) { - totps.push(f); + field.autoCompleteType === "one-time-code"); + + const isFillableUsernameField = + !options.skipUsernameOnlyFill && + ["email", "tel", "text"].some((t) => t === field.type) && + AutofillService.fieldIsFuzzyMatch(field, AutoFillConstants.UsernameFieldNames); + + // Prefer more uniquely keyworded fields first. + switch (true) { + case isFillableTotpField: + totps.push(field); + return; + case isFillableUsernameField: + usernames.push(field); + return; + default: + return; } }); } @@ -2903,52 +2912,46 @@ export default class AutofillService implements AutofillServiceInterface { /** * Accepts a field and returns true if the field contains a * value that matches any of the names in the provided list. + * + * Returns boolean and attr of value that was matched as a tuple if showMatch is set to true. + * * @param {AutofillField} field * @param {string[]} names - * @returns {boolean} + * @param {boolean} showMatch + * @returns {boolean | [boolean, { attr: string; value: string }?]} */ - static fieldIsFuzzyMatch(field: AutofillField, names: string[]): boolean { - if (AutofillService.hasValue(field.htmlID) && this.fuzzyMatch(names, field.htmlID)) { - return true; - } - if (AutofillService.hasValue(field.htmlName) && this.fuzzyMatch(names, field.htmlName)) { - return true; - } - if ( - AutofillService.hasValue(field["label-tag"]) && - this.fuzzyMatch(names, field["label-tag"]) - ) { - return true; - } - if (AutofillService.hasValue(field.placeholder) && this.fuzzyMatch(names, field.placeholder)) { - return true; - } - if ( - AutofillService.hasValue(field["label-left"]) && - this.fuzzyMatch(names, field["label-left"]) - ) { - return true; - } - if ( - AutofillService.hasValue(field["label-top"]) && - this.fuzzyMatch(names, field["label-top"]) - ) { - return true; - } - if ( - AutofillService.hasValue(field["label-aria"]) && - this.fuzzyMatch(names, field["label-aria"]) - ) { - return true; - } - if ( - AutofillService.hasValue(field.dataSetValues) && - this.fuzzyMatch(names, field.dataSetValues) - ) { - return true; - } + static fieldIsFuzzyMatch( + field: AutofillField, + names: string[], + showMatch: true, + ): [boolean, { attr: string; value: string }?]; + static fieldIsFuzzyMatch(field: AutofillField, names: string[]): boolean; + static fieldIsFuzzyMatch( + field: AutofillField, + names: string[], + showMatch: boolean = false, + ): boolean | [boolean, { attr: string; value: string }?] { + const attrs = [ + "htmlID", + "htmlName", + "label-tag", + "placeholder", + "label-left", + "label-top", + "label-aria", + "dataSetValues", + ]; - return false; + for (const attr of attrs) { + const value = field[attr]; + if (!AutofillService.hasValue(value)) { + continue; + } + if (this.fuzzyMatch(names, value)) { + return showMatch ? [true, { attr, value }] : true; + } + } + return showMatch ? [false] : false; } /**