mirror of
https://github.com/bitwarden/clients.git
synced 2026-07-07 21:10:55 +08:00
[PM-31646] prioritize usernames ahead of ambiguous TOTPs (#19453)
Split isFillableTotpField into isFillableTotpField (TotpFieldNames / one-time-code autocomplete) and maybeFillableTotpField (AmbiguousTotpFieldNames only) and reorder the classification switch so that a reliable TOTP signal still wins unconditionally, but a username match beats a purely ambiguous TOTP match. This fixes Yahoo's login-username field being misclassified as TOTP because 'code' appears in AmbiguousTotpFieldNames.
This commit is contained in:
parent
852e5015a7
commit
877598ec9c
@ -2438,6 +2438,139 @@ describe("AutofillService", () => {
|
||||
AutoFillConstants.TotpFieldNames,
|
||||
);
|
||||
});
|
||||
|
||||
it("classifies a field matching TotpFieldNames as TOTP", async () => {
|
||||
const totpCode = "123456";
|
||||
options.allowTotpAutofill = true;
|
||||
totpService.getCode$.mockReturnValue(of({ code: totpCode, period: 30 }));
|
||||
const totpField = createAutofillFieldMock({
|
||||
opid: "reliable-totp",
|
||||
type: "text",
|
||||
form: "validFormId",
|
||||
htmlName: AutoFillConstants.TotpFieldNames[0],
|
||||
elementNumber: 5,
|
||||
});
|
||||
pageDetails.fields = [totpField];
|
||||
|
||||
await autofillService["generateLoginFillScript"](
|
||||
fillScript,
|
||||
pageDetails,
|
||||
filledFields,
|
||||
options,
|
||||
);
|
||||
|
||||
expect(AutofillService.fillByOpid).toHaveBeenCalledWith(fillScript, totpField, totpCode);
|
||||
});
|
||||
|
||||
it("classifies a field matching AmbiguousTotpFieldNames as username when it also matches username signals", async () => {
|
||||
options.allowTotpAutofill = true;
|
||||
// Simulates a username field whose label-left was contaminated with an
|
||||
// ambiguous TOTP keyword.
|
||||
const ambiguousField = createAutofillFieldMock({
|
||||
opid: "ambiguous-totp",
|
||||
type: "text",
|
||||
form: "validFormId",
|
||||
htmlName: "username",
|
||||
htmlID: AutoFillConstants.AmbiguousTotpFieldNames[0],
|
||||
"label-left": AutoFillConstants.AmbiguousTotpFieldNames[0],
|
||||
elementNumber: 5,
|
||||
});
|
||||
pageDetails.fields = [ambiguousField];
|
||||
|
||||
await autofillService["generateLoginFillScript"](
|
||||
fillScript,
|
||||
pageDetails,
|
||||
filledFields,
|
||||
options,
|
||||
);
|
||||
|
||||
expect(AutofillService.fillByOpid).toHaveBeenCalledTimes(1);
|
||||
expect(AutofillService.fillByOpid).toHaveBeenCalledWith(
|
||||
fillScript,
|
||||
ambiguousField,
|
||||
options.cipher.login.username,
|
||||
);
|
||||
});
|
||||
|
||||
it("classifies a field matching only AmbiguousTotpFieldNames as TOTP when there are no username signals", async () => {
|
||||
const totpCode = "123456";
|
||||
options.allowTotpAutofill = true;
|
||||
totpService.getCode$.mockReturnValue(of({ code: totpCode, period: 30 }));
|
||||
const ambiguousField = createAutofillFieldMock({
|
||||
opid: "ambiguous-totp-only",
|
||||
type: "text",
|
||||
form: "validFormId",
|
||||
htmlName: AutoFillConstants.AmbiguousTotpFieldNames[0],
|
||||
htmlID: AutoFillConstants.AmbiguousTotpFieldNames[0],
|
||||
elementNumber: 5,
|
||||
});
|
||||
pageDetails.fields = [ambiguousField];
|
||||
|
||||
await autofillService["generateLoginFillScript"](
|
||||
fillScript,
|
||||
pageDetails,
|
||||
filledFields,
|
||||
options,
|
||||
);
|
||||
|
||||
expect(AutofillService.fillByOpid).toHaveBeenCalledWith(
|
||||
fillScript,
|
||||
ambiguousField,
|
||||
totpCode,
|
||||
);
|
||||
});
|
||||
|
||||
it("classifies a field matching TotpFieldNames as TOTP even when it also matches username signals", async () => {
|
||||
const totpCode = "123456";
|
||||
options.allowTotpAutofill = true;
|
||||
totpService.getCode$.mockReturnValue(of({ code: totpCode, period: 30 }));
|
||||
const field = createAutofillFieldMock({
|
||||
opid: "reliable-totp-with-username-signal",
|
||||
type: "text",
|
||||
form: "validFormId",
|
||||
htmlName: AutoFillConstants.TotpFieldNames[0],
|
||||
"label-left": "username",
|
||||
elementNumber: 5,
|
||||
});
|
||||
pageDetails.fields = [field];
|
||||
|
||||
await autofillService["generateLoginFillScript"](
|
||||
fillScript,
|
||||
pageDetails,
|
||||
filledFields,
|
||||
options,
|
||||
);
|
||||
|
||||
expect(AutofillService.fillByOpid).toHaveBeenCalledWith(fillScript, field, totpCode);
|
||||
expect(AutofillService.fillByOpid).not.toHaveBeenCalledWith(
|
||||
fillScript,
|
||||
field,
|
||||
options.cipher.login.username,
|
||||
);
|
||||
});
|
||||
|
||||
it("classifies a field with autocomplete=one-time-code as TOTP", async () => {
|
||||
const totpCode = "123456";
|
||||
options.allowTotpAutofill = true;
|
||||
totpService.getCode$.mockReturnValue(of({ code: totpCode, period: 30 }));
|
||||
const field = createAutofillFieldMock({
|
||||
opid: "otp-autocomplete",
|
||||
type: "text",
|
||||
form: "validFormId",
|
||||
autoCompleteType: "one-time-code",
|
||||
elementNumber: 5,
|
||||
});
|
||||
pageDetails.fields = [field];
|
||||
|
||||
await autofillService["generateLoginFillScript"](
|
||||
fillScript,
|
||||
pageDetails,
|
||||
filledFields,
|
||||
options,
|
||||
);
|
||||
|
||||
expect(AutofillService.fillByOpid).toHaveBeenCalledWith(fillScript, field, totpCode);
|
||||
});
|
||||
});
|
||||
|
||||
it("returns a value indicating if the page url is in an untrusted iframe", async () => {
|
||||
|
||||
@ -1067,29 +1067,36 @@ export default class AutofillService implements AutofillServiceInterface {
|
||||
return;
|
||||
}
|
||||
|
||||
const isFillableTotpField =
|
||||
const isTotpCandidate =
|
||||
options.allowTotpAutofill &&
|
||||
["number", "tel", "text"].some((t) => t === field.type) &&
|
||||
(AutofillService.fieldIsFuzzyMatch(field, [
|
||||
...AutoFillConstants.TotpFieldNames,
|
||||
...AutoFillConstants.AmbiguousTotpFieldNames,
|
||||
]) ||
|
||||
field.autoCompleteType === "one-time-code") &&
|
||||
!AutofillService.fieldIsFuzzyMatch(field, [...AutoFillConstants.RecoveryCodeFieldNames]);
|
||||
|
||||
const isFillableUsernameField =
|
||||
const isTotpField =
|
||||
isTotpCandidate &&
|
||||
(AutofillService.fieldIsFuzzyMatch(field, AutoFillConstants.TotpFieldNames) ||
|
||||
field.autoCompleteType === "one-time-code");
|
||||
|
||||
const maybeTotpField =
|
||||
isTotpCandidate &&
|
||||
AutofillService.fieldIsFuzzyMatch(field, AutoFillConstants.AmbiguousTotpFieldNames);
|
||||
|
||||
const isUsernameField =
|
||||
!options.skipUsernameOnlyFill &&
|
||||
["email", "tel", "text"].some((t) => t === field.type) &&
|
||||
AutofillService.fieldIsFuzzyMatch(field, AutoFillConstants.UsernameFieldNames);
|
||||
|
||||
// Prefer more uniquely keyworded fields first.
|
||||
// Reliable TOTP signals win unconditionally; username wins over ambiguous TOTP signals.
|
||||
switch (true) {
|
||||
case isFillableTotpField:
|
||||
case isTotpField:
|
||||
totps.push(field);
|
||||
return;
|
||||
case isFillableUsernameField:
|
||||
case isUsernameField:
|
||||
usernames.set(field.opid, field);
|
||||
return;
|
||||
case maybeTotpField:
|
||||
totps.push(field);
|
||||
return;
|
||||
default:
|
||||
return;
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user