Commit ec79631b authored by Jan Wilken Dörrie's avatar Jan Wilken Dörrie Committed by Commit Bot

[Passwords] Add feature to treat new_password heuristics as reliable

This change adds a feature flag that allows treating heuristics to find
new_password fields as reliable. This increases the number of password
forms on which password generation can be offered, but might lead to
false positives.

Bug: 1140493
Change-Id: Ie7083a14664b811d2bac87c4dac302e35575a79b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2490116Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819891}
parent d24d15d7
...@@ -305,8 +305,9 @@ struct PasswordForm { ...@@ -305,8 +305,9 @@ struct PasswordForm {
bool only_for_fallback = false; bool only_for_fallback = false;
// True iff the new password field was found with server hints or autocomplete // True iff the new password field was found with server hints or autocomplete
// attributes. Only set on form parsing for filling, and not persisted. Used // attributes or the kTreatNewPasswordHeuristicsAsReliable feature is enabled.
// as signal for password generation eligibility. // Only set on form parsing for filling, and not persisted. Used as signal for
// password generation eligibility.
bool is_new_password_reliable = false; bool is_new_password_reliable = false;
// Serialized to prefs, so don't change numeric values! // Serialized to prefs, so don't change numeric values!
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include <utility> #include <utility>
#include <vector> #include <vector>
#include "base/feature_list.h"
#include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_functions.h"
#include "base/no_destructor.h" #include "base/no_destructor.h"
#include "base/ranges/algorithm.h" #include "base/ranges/algorithm.h"
...@@ -200,7 +201,7 @@ struct SignificantFields { ...@@ -200,7 +201,7 @@ struct SignificantFields {
bool is_fallback = false; bool is_fallback = false;
// True iff the new password field was found with server hints or autocomplete // True iff the new password field was found with server hints or autocomplete
// attributes. // attributes or the kTreatNewPasswordHeuristicsAsReliable feature is enabled.
bool is_new_password_reliable = false; bool is_new_password_reliable = false;
// True if the current form has only username, but no passwords. // True if the current form has only username, but no passwords.
...@@ -974,16 +975,10 @@ std::unique_ptr<PasswordForm> FormDataParser::Parse(const FormData& form_data, ...@@ -974,16 +975,10 @@ std::unique_ptr<PasswordForm> FormDataParser::Parse(const FormData& form_data,
} }
} }
// Pass the "reliability" information to mark the new-password fields as
// eligible for automatic password generation. This only makes sense when
// forms are analysed for filling, because no passwords are generated when the
// user saves the already entered one.
if (mode == Mode::kFilling && significant_fields.new_password) {
significant_fields.is_new_password_reliable = true;
}
// (3) Now try to fill the gaps. // (3) Now try to fill the gaps.
const bool username_found_before_heuristic = significant_fields.username; const bool username_found_before_heuristic = significant_fields.username;
const bool new_password_found_before_heuristic =
significant_fields.new_password;
// Try to parse with base heuristic. // Try to parse with base heuristic.
if (!significant_fields.is_single_username) { if (!significant_fields.is_single_username) {
...@@ -1015,6 +1010,16 @@ std::unique_ptr<PasswordForm> FormDataParser::Parse(const FormData& form_data, ...@@ -1015,6 +1010,16 @@ std::unique_ptr<PasswordForm> FormDataParser::Parse(const FormData& form_data,
} }
} }
// Pass the "reliability" information to mark the new-password fields as
// eligible for automatic password generation. This only makes sense when
// forms are analysed for filling, because no passwords are generated when the
// user saves the already entered one.
significant_fields.is_new_password_reliable =
mode == Mode::kFilling && significant_fields.new_password &&
(new_password_found_before_heuristic ||
base::FeatureList::IsEnabled(
features::kTreatNewPasswordHeuristicsAsReliable));
base::UmaHistogramEnumeration("PasswordManager.UsernameDetectionMethod", base::UmaHistogramEnumeration("PasswordManager.UsernameDetectionMethod",
method); method);
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include <set> #include <set>
#include <utility> #include <utility>
#include "base/feature_list.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
...@@ -449,6 +450,9 @@ TEST(FormParserTest, SkipNotTextFields) { ...@@ -449,6 +450,9 @@ TEST(FormParserTest, SkipNotTextFields) {
} }
TEST(FormParserTest, OnlyPasswordFields) { TEST(FormParserTest, OnlyPasswordFields) {
const bool kTreatNewPasswordHeuristicsAsReliable =
base::FeatureList::IsEnabled(
features::kTreatNewPasswordHeuristicsAsReliable);
CheckTestData({ CheckTestData({
{ {
.description_for_logging = "1 password field", .description_for_logging = "1 password field",
...@@ -472,7 +476,7 @@ TEST(FormParserTest, OnlyPasswordFields) { ...@@ -472,7 +476,7 @@ TEST(FormParserTest, OnlyPasswordFields) {
.value = "pw", .value = "pw",
.form_control_type = "password"}, .form_control_type = "password"},
}, },
.is_new_password_reliable = false, .is_new_password_reliable = kTreatNewPasswordHeuristicsAsReliable,
}, },
{ {
.description_for_logging = .description_for_logging =
...@@ -486,7 +490,7 @@ TEST(FormParserTest, OnlyPasswordFields) { ...@@ -486,7 +490,7 @@ TEST(FormParserTest, OnlyPasswordFields) {
.value = "pw2", .value = "pw2",
.form_control_type = "password"}, .form_control_type = "password"},
}, },
.is_new_password_reliable = false, .is_new_password_reliable = kTreatNewPasswordHeuristicsAsReliable,
}, },
{ {
.description_for_logging = .description_for_logging =
...@@ -503,7 +507,7 @@ TEST(FormParserTest, OnlyPasswordFields) { ...@@ -503,7 +507,7 @@ TEST(FormParserTest, OnlyPasswordFields) {
.value = "pw2", .value = "pw2",
.form_control_type = "password"}, .form_control_type = "password"},
}, },
.is_new_password_reliable = false, .is_new_password_reliable = kTreatNewPasswordHeuristicsAsReliable,
}, },
{ {
.description_for_logging = "3 password fields with different values", .description_for_logging = "3 password fields with different values",
......
...@@ -90,6 +90,11 @@ const base::Feature kPasswordsWeaknessCheck = { ...@@ -90,6 +90,11 @@ const base::Feature kPasswordsWeaknessCheck = {
const base::Feature kRecoverFromNeverSaveAndroid = { const base::Feature kRecoverFromNeverSaveAndroid = {
"RecoverFromNeverSaveAndroid", base::FEATURE_DISABLED_BY_DEFAULT}; "RecoverFromNeverSaveAndroid", base::FEATURE_DISABLED_BY_DEFAULT};
// Treat heuritistics to find new password fields as reliable. This enables
// password generation on more forms, but could lead to false positives.
const base::Feature kTreatNewPasswordHeuristicsAsReliable = {
"TreatNewPasswordHeuristicsAsReliable", base::FEATURE_DISABLED_BY_DEFAULT};
// Enables support of filling and saving on username first flow. // Enables support of filling and saving on username first flow.
const base::Feature kUsernameFirstFlow = {"UsernameFirstFlow", const base::Feature kUsernameFirstFlow = {"UsernameFirstFlow",
base::FEATURE_DISABLED_BY_DEFAULT}; base::FEATURE_DISABLED_BY_DEFAULT};
......
...@@ -33,6 +33,7 @@ extern const base::Feature kPasswordImport; ...@@ -33,6 +33,7 @@ extern const base::Feature kPasswordImport;
extern const base::Feature kPasswordScriptsFetching; extern const base::Feature kPasswordScriptsFetching;
extern const base::Feature kPasswordsWeaknessCheck; extern const base::Feature kPasswordsWeaknessCheck;
extern const base::Feature kRecoverFromNeverSaveAndroid; extern const base::Feature kRecoverFromNeverSaveAndroid;
extern const base::Feature kTreatNewPasswordHeuristicsAsReliable;
extern const base::Feature kUsernameFirstFlow; extern const base::Feature kUsernameFirstFlow;
extern const base::Feature kWellKnownChangePassword; extern const base::Feature kWellKnownChangePassword;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment