Skip to content

Commit

Permalink
[Merge-112][Passwords] Additional metric for OTP regexes
Browse files Browse the repository at this point in the history
The metric measures how often an OTP regexs detects anything. It will be
used to compare coverage between new and old regexes.

(cherry picked from commit 51d4c63)

Bug: 1382805
Change-Id: I86a3474754d7a5bc22a79205a60d8d7d5d3710f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4290992
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1109635}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4293107
Cr-Commit-Position: refs/branch-heads/5615@{#33}
Cr-Branched-From: 9c6408e-refs/heads/main@{#1109224}
  • Loading branch information
Maxim Kolosovskiy authored and Chromium LUCI CQ committed Feb 27, 2023
1 parent f7ca6e1 commit c326a04
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -124,15 +124,22 @@ bool StringMatchesHiddenValue(const std::u16string& str) {
// The suspicion is based on server-side provided hints and on checking the
// field's id and name for hinting towards a CVC code, Social Security
// Number or one-time password.
bool IsNotPasswordField(const ProcessedField& field) {
return field.server_hints_not_password ||
field.autocomplete_flag == AutocompleteFlag::kNonPassword ||
StringMatchesCVC(field.field->name_attribute) ||
StringMatchesCVC(field.field->id_attribute) ||
StringMatchesSSN(field.field->name_attribute) ||
StringMatchesSSN(field.field->id_attribute) ||
StringMatchesOTP(field.field->name_attribute) ||
StringMatchesOTP(field.field->id_attribute);
bool IsNotPasswordField(const ProcessedField& field,
bool* otp_field_detected_with_regex = nullptr) {
if (field.server_hints_not_password ||
field.autocomplete_flag == AutocompleteFlag::kNonPassword ||
StringMatchesCVC(field.field->name_attribute) ||
StringMatchesCVC(field.field->id_attribute) ||
StringMatchesSSN(field.field->name_attribute) ||
StringMatchesSSN(field.field->id_attribute)) {
return true;
}
bool is_otp_field = StringMatchesOTP(field.field->name_attribute) ||
StringMatchesOTP(field.field->id_attribute);
if (otp_field_detected_with_regex) {
*otp_field_detected_with_regex |= is_otp_field;
}
return is_otp_field;
}

// Returns true if the |field| is suspected to be not the username field.
Expand Down Expand Up @@ -769,10 +776,18 @@ void ParseUsingBaseHeuristics(
if (!found_fields->HasPasswords()) {
// What is the best interactability among passwords?
Interactability password_max = Interactability::kUnlikely;
// TODO(crbug.com/1382805): The variable is used only for metrics for the
// new OTP regex launch. Remove the variable after the launch.
bool otp_field_detected_with_regex = false;
for (const ProcessedField& processed_field : processed_fields) {
if (processed_field.is_password && !IsNotPasswordField(processed_field))
if (processed_field.is_password &&
!IsNotPasswordField(processed_field,
&otp_field_detected_with_regex)) {
password_max = std::max(password_max, processed_field.interactability);
}
}
base::UmaHistogramBoolean("PasswordManager.ParserDetectedOtpFieldWithRegex",
otp_field_detected_with_regex);

// Try to find password elements (current, new, confirmation) among those
// with best interactability.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1970,6 +1970,24 @@ TEST(FormParserTest, SSN_and_OTP) {
}
}

TEST(FormParserTest, OtpRegexMetric) {
base::HistogramTester histogram_tester;
CheckTestData({{
.fields =
{
{.role = ElementRole::USERNAME, .form_control_type = "text"},
{.name = u"OneTimePassword", .form_control_type = "password"},
{.role = ElementRole::CURRENT_PASSWORD,
.form_control_type = "password"},
},
.fallback_only = false,
}});
// Two samples because |CheckTestData| parses the form in two modes: filling
// and saving.
histogram_tester.ExpectUniqueSample(
"PasswordManager.ParserDetectedOtpFieldWithRegex", true, 2);
}

// The parser should avoid identifying NOT_PASSWORD fields as passwords.
TEST(FormParserTest, NotPasswordField) {
CheckTestData({
Expand Down
11 changes: 11 additions & 0 deletions tools/metrics/histograms/metadata/password/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2186,6 +2186,17 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>

<histogram name="PasswordManager.ParserDetectedOtpFieldWithRegex"
enum="Boolean" expires_after="2023-07-01">
<owner>kolos@chromium.org</owner>
<owner>shaikhitdin@google.com</owner>
<summary>
Reports whether the form parser has detected an OTP field with regex.
Reported any time when the parser processes a form. Used to compare how
often the new and old OTP regexes detect any OTP field.
</summary>
</histogram>

<histogram
name="PasswordManager.PasswordChangeFlowDuration{EntryPoint}{StartEvent}{EndEvent}"
units="ms" expires_after="2023-05-09">
Expand Down

0 comments on commit c326a04

Please sign in to comment.