Skip to content

Commit

Permalink
[M109][Passwords] New regex for OTP fields
Browse files Browse the repository at this point in the history
To inspect field names and ids, I used dashboards/_7cd0859a_1ba0_40f0_bdaf_ca67d1d9814a?p=SUBSTR:otp

(cherry picked from commit a399097)

Bug: 1382805
Change-Id: Iafcc6600e7d68cd4e9530c0790bc5d21766b0a4f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4016377
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Reviewed-by: Christoph Schwering <schwering@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1070341}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4020522
Auto-Submit: Maxim Kolosovskiy <kolos@chromium.org>
Commit-Queue: Christoph Schwering <schwering@google.com>
Cr-Commit-Position: refs/branch-heads/5414@{#17}
Cr-Branched-From: 4417ee5-refs/heads/main@{#1070088}
  • Loading branch information
Maxim Kolosovskiy authored and Chromium LUCI CQ committed Nov 14, 2022
1 parent 621dc60 commit 65e2573
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 3 deletions.
22 changes: 22 additions & 0 deletions components/autofill/core/common/autofill_regex_constants.h
Expand Up @@ -625,8 +625,30 @@ inline constexpr char16_t kUrlSearchActionRe[] =
/////////////////////////////////////////////////////////////////////////////
inline constexpr char16_t kSocialSecurityRe[] =
u"ssn|social.?security.?(num(ber)?|#)*";
// TODO(crbug.com/1382805): Remove it once the new regex launched.
inline constexpr char16_t kOneTimePwdRe[] =
u"one.?time|sms.?(code|token|password|pwd|pass)";
inline constexpr char16_t kNewOneTimePwdRe[] =
// "One time" is good signal that it is an OTP field.
u"one.?time|"
// The main tokens are good signals, but they are short, require word
// boundaries around them.
u"(\\b|_)(otp|otc|totp|sms|2fa|mfa)(\\b|_)|"
// Alternatively, require companion tokens before or after the main tokens.
u"(otp|otc|totp|sms|2fa|mfa).?(code|token|input|value|pin|login|verif|pass|"
u"pwd|psw|auth|field)|"
u"(verif(y|ication)?|email|phone|text|login|input|txt|user).?(otp|otc|totp|"
u"sms|2fa|mfa)|"
// Sometimes the main tokens are combined with each other.
u"sms.?otp|mfa.?otp|"
// "code" is not so strong signal as the main tokens, but in combination
// with "verification" and its variations it is.
u"verif(y|ication)?.?code|(\\b|_)vcode|"
// 'Second factor' and its variations are good signals.
u"(second|two|2).?factor|"
// A couple of custom strings that are usually OTP fields.
u"wfls-token|email_code";

// Matches strings that consist of one repeated non alphanumeric symbol,
// that is likely a result of website modifying the value to hide it.
inline constexpr char16_t kHiddenValueRe[] = u"^(\\W)\\1+$";
Expand Down
Expand Up @@ -102,7 +102,12 @@ bool StringMatchesSSN(const std::u16string& str) {

// Returns true if the |str| contains words related to one time password fields.
bool StringMatchesOTP(const std::u16string& str) {
return autofill::MatchesRegex<autofill::kOneTimePwdRe>(str);
if (base::FeatureList::IsEnabled(
password_manager::features::kNewRegexForOtpFields)) {
return autofill::MatchesRegex<autofill::kNewOneTimePwdRe>(str);
} else {
return autofill::MatchesRegex<autofill::kOneTimePwdRe>(str);
}
}

// Returns true if the |str| consists of one repeated non alphanumeric symbol.
Expand Down
Expand Up @@ -1890,9 +1890,10 @@ TEST(FormParserTest, CCNumber) {
});
}

// TODO(crbug.com/1382805): Remove this test once the new regex launched.
// The parser should avoid identifying Social Security number and
// one time password fields as passwords.
TEST(FormParserTest, SSN_and_OTP) {
TEST(FormParserTest, SSN_and_OTP_Old_Regex) {
for (const char16_t* field_name :
{u"SocialSecurityNumber", u"OneTimePassword", u"SMS-token"}) {
CheckTestData({
Expand Down Expand Up @@ -1928,6 +1929,46 @@ TEST(FormParserTest, SSN_and_OTP) {
}
}

TEST(FormParserTest, SSN_and_OTP) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
password_manager::features::kNewRegexForOtpFields);
for (const char16_t* field_name :
{u"SocialSecurityNumber", u"OneTimePassword", u"SMS-token", u"otp-code",
u"input_SMS", u"second.factor"}) {
CheckTestData({
{
.description_for_logging = "Field name matches the SSN/OTP pattern,"
"Ignore that one.",
.fields =
{
{.role = ElementRole::USERNAME,
.form_control_type = "text"},
{.name = field_name, .form_control_type = "password"},
{.role = ElementRole::CURRENT_PASSWORD,
.form_control_type = "password"},
},
// The result should be trusted for more than just fallback, because
// there is an actual password field present.
.fallback_only = false,
},
{
.description_for_logging = "Create a fallback for the only password"
"field being an SSN/OTP field",
.fields =
{
{.role = ElementRole::USERNAME,
.form_control_type = "text"},
{.role = ElementRole::CURRENT_PASSWORD,
.name = field_name,
.form_control_type = "password"},
},
.fallback_only = true,
},
});
}
}

// The parser should avoid identifying NOT_PASSWORD fields as passwords.
TEST(FormParserTest, NotPasswordField) {
CheckTestData({
Expand Down
Expand Up @@ -130,6 +130,11 @@ BASE_FEATURE(kMuteCompromisedPasswords,
#endif
);

// Enables new regex for OTP fields.
BASE_FEATURE(kNewRegexForOtpFields,
"NewRegexForOtpFields",
base::FEATURE_DISABLED_BY_DEFAULT);

// Enables the new password viewing subpage.
BASE_FEATURE(kPasswordViewPageInSettings,
"PasswordViewPageInSettings",
Expand Down
Expand Up @@ -47,7 +47,7 @@ BASE_DECLARE_FEATURE(kIOSPasswordUISplit);
BASE_DECLARE_FEATURE(kIOSPasswordManagerCrossOriginIframeSupport);
#endif // IS_IOS
BASE_DECLARE_FEATURE(kMuteCompromisedPasswords);

BASE_DECLARE_FEATURE(kNewRegexForOtpFields);
BASE_DECLARE_FEATURE(kPasswordViewPageInSettings);
BASE_DECLARE_FEATURE(kSendPasswords);
BASE_DECLARE_FEATURE(kLeakDetectionUnauthenticated);
Expand Down

0 comments on commit 65e2573

Please sign in to comment.