Commit e031efef authored by Dominic Battre's avatar Dominic Battre Committed by Commit Bot

Fix PasswordManager.FillingAssistance for blacklisted sites

If a user has blacklisted a site for saving credentials, the login database
stores this as an entry with an empty password. This CL recognizes this and
reports that the password was typed while the site was blacklisted instead of
reporting that a password was typed even though a password exits (the empty
password).

There are still two special cases left:

1) If a PSL matching domain is blacklisted, it is still considered part of the
current site even though the user will see a save prompt.

2) The use user has ignored a password save prompt 3 times, we don't ask again.
In this case we will not report the site as blacklisted but report that no
password was stored.

Bug: 918846
Change-Id: I5cb0396940ec6be55b9e0715b3a0a74aa52815b9
Reviewed-on: https://chromium-review.googlesource.com/c/1448213Reviewed-by: default avatarVadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627953}
parent 14fa34f0
...@@ -401,6 +401,26 @@ void PasswordFormMetricsRecorder::CalculateFillingAssistanceMetric( ...@@ -401,6 +401,26 @@ void PasswordFormMetricsRecorder::CalculateFillingAssistanceMetric(
const FormData& submitted_form, const FormData& submitted_form,
const std::set<base::string16>& saved_usernames, const std::set<base::string16>& saved_usernames,
const std::set<base::string16>& saved_passwords) { const std::set<base::string16>& saved_passwords) {
// If the user asked to never save credentials on a domain, an entry with
// empty password exists for that domain.
bool blacklisted =
saved_passwords.find(base::string16()) != saved_passwords.end();
if (blacklisted && saved_passwords.size() == 1u) {
// Note that we miss two nuances here:
//
// 1) It is possible that the user logs in to a.example.com but
// b.example.com is blacklisted. We would still report
// kNoSavedCredentialsAndBlacklisted even though the user has not
// blacklisted a.example.com and is asked whether they want to save the
// credentials.
//
// 2) It is possible that the user ignored the save bubble a number of times
// (default threshold is 3). In this case, no credential is stored and we
// report kNoSavedCredentials and not kNoSavedCredentialsAndBlacklisted.
filling_assistance_ = FillingAssistance::kNoSavedCredentialsAndBlacklisted;
return;
}
if (saved_passwords.empty()) { if (saved_passwords.empty()) {
filling_assistance_ = FillingAssistance::kNoSavedCredentials; filling_assistance_ = FillingAssistance::kNoSavedCredentials;
return; return;
......
...@@ -266,7 +266,9 @@ class PasswordFormMetricsRecorder ...@@ -266,7 +266,9 @@ class PasswordFormMetricsRecorder
kNoSavedCredentials = 5, kNoSavedCredentials = 5,
// Neither user input nor filling. // Neither user input nor filling.
kNoUserInputNoFillingInPasswordFields = 6, kNoUserInputNoFillingInPasswordFields = 6,
kMaxValue = kNoUserInputNoFillingInPasswordFields, // Domain is blacklisted and no other credentials exist.
kNoSavedCredentialsAndBlacklisted = 7,
kMaxValue = kNoSavedCredentialsAndBlacklisted,
}; };
// The maximum number of combinations of the ManagerAction, UserAction and // The maximum number of combinations of the ManagerAction, UserAction and
......
...@@ -988,4 +988,38 @@ TEST(PasswordFormMetricsRecorder, ...@@ -988,4 +988,38 @@ TEST(PasswordFormMetricsRecorder,
.expectation = PasswordFormMetricsRecorder::FillingAssistance::kManual}); .expectation = PasswordFormMetricsRecorder::FillingAssistance::kManual});
} }
TEST(PasswordFormMetricsRecorder, FillingAssistanceBlacklistedDomain) {
CheckFillingAssistanceTestCase(
{.description_for_logging = "Submission while domain is blacklisted",
.fields = {{.value = "user1"},
{.value = "password1", .is_password = true}},
// A blacklisted domain is represented as empty username and password
// but empty username elements are stripped before
// PasswordFormMetricsRecorder::CalculateFillingAssistanceMetric is
// called.
.saved_usernames = {},
.saved_passwords = {""},
.expectation = PasswordFormMetricsRecorder::FillingAssistance::
kNoSavedCredentialsAndBlacklisted});
}
TEST(PasswordFormMetricsRecorder,
FillingAssistanceBlacklistedDomainWithCredential) {
CheckFillingAssistanceTestCase(
{.description_for_logging =
"Submission while domain is blacklisted but a credential is stored",
.fields = {{.value = "user1", .automatically_filled = true},
{.value = "password1",
.is_password = true,
.automatically_filled = true}},
// A blacklisted domain is represented as empty username and password
// but empty username elements are stripped before
// PasswordFormMetricsRecorder::CalculateFillingAssistanceMetric is
// called.
.saved_usernames = {"user1"},
.saved_passwords = {"", "password1"},
.expectation =
PasswordFormMetricsRecorder::FillingAssistance::kAutomatic});
}
} // namespace password_manager } // namespace password_manager
...@@ -41320,6 +41320,9 @@ Called by update_net_trust_anchors.py.--> ...@@ -41320,6 +41320,9 @@ Called by update_net_trust_anchors.py.-->
<int value="6" label="%-) Neither filling nor user input"> <int value="6" label="%-) Neither filling nor user input">
For example, it might be because of 3rd party password managers. For example, it might be because of 3rd party password managers.
</int> </int>
<int value="7"
label=":-X Unknown password typed, no saved credentials existed and
site was explicitly blacklisted"/>
</enum> </enum>
<enum name="PasswordManagerFirstRendererFillingResult"> <enum name="PasswordManagerFirstRendererFillingResult">
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