Commit d6089b70 authored by Mohamed Amir Yosef's avatar Mohamed Amir Yosef Committed by Commit Bot

[Passwords] FindBestMatches should respect credentials store

Before this patch:
FindBestMatches deduplicate credentials using only the username. As a
result, if two credentials with the same username exist in both stores,
only one is returned.

After this patch:
FindBestMatches does the deduplication within account and profile store
separately.

Bug: 1057647
Change-Id: I0e4d321c907ee370ad6d3b42933b8aa7efd1158d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2084992Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Reviewed-by: default avatarVadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746723}
parent f5492ab1
...@@ -349,12 +349,14 @@ void FindBestMatches( ...@@ -349,12 +349,14 @@ void FindBestMatches(
std::sort(non_federated_same_scheme->begin(), std::sort(non_federated_same_scheme->begin(),
non_federated_same_scheme->end(), IsBetterMatch); non_federated_same_scheme->end(), IsBetterMatch);
std::set<base::string16> usernames; std::set<std::pair<PasswordForm::Store, base::string16>> store_usernames;
for (const auto* match : *non_federated_same_scheme) { for (const auto* match : *non_federated_same_scheme) {
const base::string16& username = match->username_value; auto store_username =
// The first match for |username| in the sorted array is best match. std::make_pair(match->in_store, match->username_value);
if (!base::Contains(usernames, username)) { // The first match for |store_username| in the sorted array is best
usernames.insert(username); // match.
if (!base::Contains(store_usernames, store_username)) {
store_usernames.insert(store_username);
best_matches->push_back(match); best_matches->push_back(match);
} }
} }
......
...@@ -228,6 +228,59 @@ TEST(PasswordManagerUtil, FindBestMatches) { ...@@ -228,6 +228,59 @@ TEST(PasswordManagerUtil, FindBestMatches) {
} }
} }
TEST(PasswordManagerUtil, FindBestMatchesInProfileAndAccountStores) {
const base::string16 kUsername1 = base::ASCIIToUTF16("Username1");
const base::string16 kPassword1 = base::ASCIIToUTF16("Password1");
const base::string16 kUsername2 = base::ASCIIToUTF16("Username2");
const base::string16 kPassword2 = base::ASCIIToUTF16("Password2");
PasswordForm form;
form.is_public_suffix_match = false;
form.date_last_used = base::Time::Now();
// Add the same credentials in account and profile stores.
PasswordForm account_form1(form);
account_form1.username_value = kUsername1;
account_form1.password_value = kPassword1;
account_form1.in_store = PasswordForm::Store::kAccountStore;
PasswordForm profile_form1(account_form1);
profile_form1.in_store = PasswordForm::Store::kProfileStore;
// Add the credentials for the same username in account and profile stores but
// with different passwords.
PasswordForm account_form2(form);
account_form2.username_value = kUsername2;
account_form2.password_value = kPassword1;
account_form2.in_store = PasswordForm::Store::kAccountStore;
PasswordForm profile_form2(account_form2);
profile_form2.password_value = kPassword2;
profile_form2.in_store = PasswordForm::Store::kProfileStore;
std::vector<const PasswordForm*> matches;
matches.push_back(&account_form1);
matches.push_back(&profile_form1);
matches.push_back(&account_form2);
matches.push_back(&profile_form2);
std::vector<const PasswordForm*> best_matches;
const PasswordForm* preferred_match = nullptr;
std::vector<const PasswordForm*> same_scheme_matches;
FindBestMatches(matches, PasswordForm::Scheme::kHtml, &same_scheme_matches,
&best_matches, &preferred_match);
// All 4 matches should be returned in best matches.
EXPECT_EQ(best_matches.size(), 4U);
EXPECT_NE(std::find(best_matches.begin(), best_matches.end(), &account_form1),
best_matches.end());
EXPECT_NE(std::find(best_matches.begin(), best_matches.end(), &account_form2),
best_matches.end());
EXPECT_NE(std::find(best_matches.begin(), best_matches.end(), &profile_form1),
best_matches.end());
EXPECT_NE(std::find(best_matches.begin(), best_matches.end(), &profile_form2),
best_matches.end());
}
TEST(PasswordManagerUtil, GetMatchForUpdating_MatchUsername) { TEST(PasswordManagerUtil, GetMatchForUpdating_MatchUsername) {
autofill::PasswordForm stored = GetTestCredential(); autofill::PasswordForm stored = GetTestCredential();
autofill::PasswordForm parsed = GetTestCredential(); autofill::PasswordForm parsed = GetTestCredential();
......
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