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

[Passwords] Dedup federated credentials across stores in CredManAPI

Bug: 1093286
Change-Id: I13798515a76c1a1b5b15be27d03bc61a9cd0e4c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2341129Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798599}
parent 7a1e03d0
......@@ -56,7 +56,11 @@ auto MakeFlatSet(KeyGetter key_getter) {
// Remove duplicates in |forms| before displaying them in the account chooser.
void FilterDuplicates(
std::vector<std::unique_ptr<autofill::PasswordForm>>* forms) {
std::vector<std::unique_ptr<autofill::PasswordForm>> federated_forms;
auto federated_forms_with_unique_username =
MakeFlatSet(/*key_getter=*/[](const auto& form) {
return std::make_pair(form->username_value, form->federation_origin);
});
// The key is [username, signon_realm, store]. signon_realm is used only for
// PSL matches because those entries have it in the UI.
auto credentials = MakeFlatSet(/*key_getter=*/[](const auto& form) {
......@@ -67,7 +71,13 @@ void FilterDuplicates(
});
for (auto& form : *forms) {
if (!form->federation_origin.opaque()) {
federated_forms.push_back(std::move(form));
// |forms| contains credentials from both the profile and account stores.
// Therefore, it could potentially contains duplicate federated
// credentials. In case of duplicates, favor the account store version.
auto result =
federated_forms_with_unique_username.insert(std::move(form));
if (!result.second && form->IsUsingAccountStore())
*result.first = std::move(form);
} else {
auto result = credentials.insert(std::move(form));
if (!result.second && IsBetterMatch(*form, **result.first))
......@@ -93,6 +103,9 @@ void FilterDuplicates(
*result.first = std::move(form);
}
*forms = std::move(credentials_with_unique_passwords).extract();
std::vector<std::unique_ptr<autofill::PasswordForm>> federated_forms =
std::move(federated_forms_with_unique_username).extract();
std::move(federated_forms.begin(), federated_forms.end(),
std::back_inserter(*forms));
}
......
......@@ -202,4 +202,36 @@ TEST_F(CredentialManagerPendingRequestTaskTest,
EXPECT_TRUE(client()->forms_passed_to_ui()[0]->IsUsingAccountStore());
}
TEST_F(CredentialManagerPendingRequestTaskTest,
SameFederatedCredentialsInBothStores) {
// This is testing that when two federated credentials have the same username
// for the same origin, the account store version is passed to the UI.
GURL federation_url("https://google.com/");
CredentialManagerPendingRequestTask task(
&delegate_mock_, /*callback=*/base::DoNothing(),
CredentialMediationRequirement::kOptional, /*include_passwords=*/false,
{federation_url}, StoresToQuery::kProfileAndAccountStores);
form_.federation_origin = url::Origin::Create(federation_url);
form_.password_value = base::string16();
form_.signon_realm = "federation://example.com/google.com";
autofill::PasswordForm profile_form = form_;
profile_form.in_store = autofill::PasswordForm::Store::kProfileStore;
std::vector<std::unique_ptr<autofill::PasswordForm>> profile_forms;
profile_forms.push_back(
std::make_unique<autofill::PasswordForm>(profile_form));
autofill::PasswordForm account_form = form_;
account_form.in_store = autofill::PasswordForm::Store::kAccountStore;
std::vector<std::unique_ptr<autofill::PasswordForm>> account_forms;
account_forms.push_back(
std::make_unique<autofill::PasswordForm>(account_form));
task.OnGetPasswordStoreResultsFrom(profile_store_, std::move(profile_forms));
task.OnGetPasswordStoreResultsFrom(account_store_, std::move(account_forms));
ASSERT_EQ(1U, client()->forms_passed_to_ui().size());
EXPECT_TRUE(client()->forms_passed_to_ui()[0]->IsUsingAccountStore());
}
} // namespace password_manager
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