Skip to content

Commit

Permalink
[Password Manager] Implement AddCredentials in saved_password_presenter.
Browse files Browse the repository at this point in the history
Bug: 1325290
Change-Id: I8bb1f3478309ee55ec822d4e58ea0cb2cd362b67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3804567
Reviewed-by: Bruno Braga <brunobraga@google.com>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: David Jean <djean@chromium.org>
Commit-Queue: Elias Khsheibun <eliaskh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1032943}
  • Loading branch information
Elias authored and Chromium LUCI CQ committed Aug 9, 2022
1 parent 1bc174a commit 9269cd3
Show file tree
Hide file tree
Showing 18 changed files with 307 additions and 96 deletions.
Expand Up @@ -867,7 +867,7 @@ TEST_F(PasswordAccessoryControllerTest, SavePasswordsDisabledUpdatesStore) {
expected_form.signon_realm = kExampleSignonRealm;
expected_form.url = GURL(kExampleSite);
expected_form.date_created = base::Time::Now();
EXPECT_CALL(*mock_password_store_, AddLogin(Eq(expected_form)));
EXPECT_CALL(*mock_password_store_, AddLogin(Eq(expected_form), _));
controller()->OnToggleChanged(
autofill::AccessoryAction::TOGGLE_SAVE_PASSWORDS, false);
}
Expand Down
Expand Up @@ -28,6 +28,7 @@
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"

using ::testing::_;
using ::testing::Return;
using ::testing::ReturnRef;

Expand Down Expand Up @@ -172,7 +173,7 @@ TEST_F(ItemsBubbleControllerTest, OnPasswordActionAddPassword) {
form.username_value = u"User";
form.password_value = u"123456";

EXPECT_CALL(*GetStore(), AddLogin(form));
EXPECT_CALL(*GetStore(), AddLogin(form, _));

controller()->OnPasswordAction(
form, PasswordBubbleControllerBase::PasswordAction::kAddPassword);
Expand Down
Expand Up @@ -1481,11 +1481,13 @@ TEST_F(ManagePasswordsUIControllerTest, SaveUnsyncedCredentialsInProfileStore) {
EXPECT_CALL(*profile_store,
AddLogin(MatchesLoginAndURL(credentials[0].username_value,
credentials[0].password_value,
credentials[0].url)));
credentials[0].url),
_));
EXPECT_CALL(*profile_store,
AddLogin(MatchesLoginAndURL(credentials[1].username_value,
credentials[1].password_value,
credentials[1].url)));
credentials[1].url),
_));

// Save.
EXPECT_CALL(*controller(), OnUpdateBubbleAndIconVisibility());
Expand Down
Expand Up @@ -240,8 +240,9 @@ TEST_F(WebsiteLoginManagerImplTest, SaveGeneratedPassword) {
password_manager::PasswordForm::Scheme::kHtml, kFakeUrl, GURL(kFakeUrl));
// Presave generated password. Form with empty username is presaved.
EXPECT_CALL(*store(), GetLogins(form_digest, _));
EXPECT_CALL(*store(),
AddLogin(FormMatches(MakeSimplePasswordFormWithoutUsername())));
EXPECT_CALL(
*store(),
AddLogin(FormMatches(MakeSimplePasswordFormWithoutUsername()), _));
manager_->PresaveGeneratedPassword(
{GURL(kFakeUrl), kFakeUsername}, kFakeNewPassword,
MakeFormDataWithPasswordField(), base::OnceClosure());
Expand Down Expand Up @@ -413,7 +414,7 @@ TEST_F(WebsiteLoginManagerImplTest, SaveSubmittedPasswordNewLogin) {
EXPECT_FALSE(manager_->SubmittedPasswordIsSame());

// Expect the password to get saved.
EXPECT_CALL(*store(), AddLogin(FormMatches(form)));
EXPECT_CALL(*store(), AddLogin(FormMatches(form), _));
EXPECT_TRUE(manager_->SaveSubmittedPassword());
}

Expand Down
Expand Up @@ -638,7 +638,7 @@ TEST_P(FormFetcherImplTest, DoNotTryToMigrateHTTPPasswordsOnHTTPSites) {

Fetch();
EXPECT_CALL(*profile_mock_store_, GetLogins(_, _)).Times(0);
EXPECT_CALL(*profile_mock_store_, AddLogin(_)).Times(0);
EXPECT_CALL(*profile_mock_store_, AddLogin).Times(0);
EXPECT_CALL(consumer_, OnFetchCompleted);
DeliverPasswordStoreResults(
/*profile_store_results=*/MakeResults(empty_forms),
Expand Down Expand Up @@ -751,7 +751,7 @@ TEST_P(FormFetcherImplTest, TryToMigrateHTTPPasswordsOnHTTPSSites) {
ASSERT_TRUE(account_store_migrator);
}
// Now perform the actual migration.
EXPECT_CALL(*profile_mock_store_, AddLogin(https_form));
EXPECT_CALL(*profile_mock_store_, AddLogin(https_form, _));
EXPECT_CALL(consumer_, OnFetchCompleted);
profile_store_migrator->OnGetPasswordStoreResultsFrom(
profile_mock_store_.get(), MakeResults({http_form}));
Expand All @@ -767,7 +767,7 @@ TEST_P(FormFetcherImplTest, TryToMigrateHTTPPasswordsOnHTTPSSites) {
// No migration should happen when results are present.
Fetch();
EXPECT_CALL(*profile_mock_store_, GetLogins(_, _)).Times(0);
EXPECT_CALL(*profile_mock_store_, AddLogin(_)).Times(0);
EXPECT_CALL(*profile_mock_store_, AddLogin).Times(0);
EXPECT_CALL(consumer_, OnFetchCompleted);
DeliverPasswordStoreResults(
/*profile_store_results=*/MakeResults({https_form}),
Expand Down Expand Up @@ -859,7 +859,7 @@ TEST_P(FormFetcherImplTest, StateIsWaitingDuringMigration) {
EXPECT_EQ(FormFetcher::State::WAITING, form_fetcher_->GetState());

// Now perform the actual migration.
EXPECT_CALL(*profile_mock_store_, AddLogin(https_form));
EXPECT_CALL(*profile_mock_store_, AddLogin(https_form, _));
profile_store_migrator->OnGetPasswordStoreResultsFrom(
profile_mock_store_.get(), MakeResults({http_form}));
if (account_mock_store_) {
Expand Down
Expand Up @@ -101,7 +101,7 @@ void FormSaverImplSaveTest::SaveCredential(
switch (GetParam()) {
case SaveOperation::kSave:
expected.date_password_modified = base::Time::Now();
EXPECT_CALL(*mock_store_, AddLogin(expected));
EXPECT_CALL(*mock_store_, AddLogin(expected, _));
return form_saver_.Save(std::move(pending), matches, old_password);
case SaveOperation::kUpdate:
if (old_password != pending.password_value)
Expand Down Expand Up @@ -291,7 +291,7 @@ TEST_P(FormSaverImplSaveTest, FormDataSanitized) {
PasswordForm saved;
switch (GetParam()) {
case SaveOperation::kSave:
EXPECT_CALL(*mock_store_, AddLogin(_)).WillOnce(SaveArg<0>(&saved));
EXPECT_CALL(*mock_store_, AddLogin).WillOnce(SaveArg<0>(&saved));
return form_saver_.Save(std::move(pending), {}, u"");
case SaveOperation::kUpdate:
EXPECT_CALL(*mock_store_, UpdateLogin(_)).WillOnce(SaveArg<0>(&saved));
Expand Down Expand Up @@ -338,7 +338,7 @@ TEST_F(FormSaverImplTest, Blocklist) {
password_manager_util::MakeNormalizedBlocklistedForm(
PasswordFormDigest(observed));

EXPECT_CALL(*mock_store_, AddLogin(FormWithSomeDate(blocklisted)));
EXPECT_CALL(*mock_store_, AddLogin(FormWithSomeDate(blocklisted), _));
PasswordForm result = form_saver_.Blocklist(PasswordFormDigest(observed));
EXPECT_THAT(result, FormWithSomeDate(blocklisted));
}
Expand Down
Expand Up @@ -191,8 +191,8 @@ void HttpPasswordStoreMigratorTest::TestFullStore(bool is_hsts) {
expected_federated_form.url = GURL("https://localhost");
expected_federated_form.action = GURL("https://localhost");

EXPECT_CALL(store(), AddLogin(expected_form));
EXPECT_CALL(store(), AddLogin(expected_federated_form));
EXPECT_CALL(store(), AddLogin(expected_form, _));
EXPECT_CALL(store(), AddLogin(expected_federated_form, _));
EXPECT_CALL(store(), RemoveLogin(form)).Times(is_hsts);
EXPECT_CALL(store(), RemoveLogin(federated_form)).Times(is_hsts);
EXPECT_CALL(consumer(),
Expand Down
Expand Up @@ -95,10 +95,18 @@ void PasswordImporter::ConsumePasswords(
if (!seq)
return;

for (const auto& pwd : seq->csv_passwords) {
presenter_->AddCredential(password_manager::CredentialUIEntry(pwd),
password_manager::PasswordForm::Type::kImported);
}
std::vector<password_manager::CredentialUIEntry> credentials;
credentials.reserve(seq->csv_passwords.size());

base::ranges::transform(
seq->csv_passwords, std::back_inserter(credentials),
[](const password_manager::CSVPassword& csv_password) {
return password_manager::CredentialUIEntry(csv_password);
});

presenter_->AddCredentials(credentials,
password_manager::PasswordForm::Type::kImported,
base::DoNothing());

UMA_HISTOGRAM_COUNTS_1M("PasswordManager.ImportedPasswordsPerUserInCSV",
seq->csv_passwords.size());
Expand Down
Expand Up @@ -16,7 +16,10 @@ class MockPasswordStoreInterface : public PasswordStoreInterface {
MockPasswordStoreInterface();

MOCK_METHOD(bool, IsAbleToSavePasswords, (), (const, override));
MOCK_METHOD(void, AddLogin, (const PasswordForm&), (override));
MOCK_METHOD(void,
AddLogin,
(const PasswordForm&, base::OnceClosure),
(override));
MOCK_METHOD(void, UpdateLogin, (const PasswordForm&), (override));
MOCK_METHOD(void,
UpdateLoginWithPrimaryKey,
Expand Down
Expand Up @@ -267,7 +267,7 @@ TEST_F(PasswordGenerationManagerTest, PresaveGeneratedPassword_New) {
generated_with_date.date_created = base::Time::Now();
generated_with_date.date_password_modified = base::Time::Now();

EXPECT_CALL(store(), AddLogin(generated_with_date));
EXPECT_CALL(store(), AddLogin(generated_with_date, _));
manager().PresaveGeneratedPassword(generated, {}, &form_saver());
EXPECT_TRUE(manager().HasGeneratedPassword());
}
Expand All @@ -279,7 +279,7 @@ TEST_F(PasswordGenerationManagerTest, PresaveGeneratedPassword_Replace) {
generated_with_date.date_created = base::Time::Now();
generated_with_date.date_password_modified = base::Time::Now();

EXPECT_CALL(store(), AddLogin(generated_with_date));
EXPECT_CALL(store(), AddLogin(generated_with_date, _));
manager().PresaveGeneratedPassword(generated, {}, &form_saver());

ForwardByMinute();
Expand All @@ -301,7 +301,7 @@ TEST_F(PasswordGenerationManagerTest, PresaveGeneratedPassword_ReplaceTwice) {
generated_with_date.date_created = base::Time::Now();
generated_with_date.date_password_modified = base::Time::Now();

EXPECT_CALL(store(), AddLogin(generated_with_date));
EXPECT_CALL(store(), AddLogin(generated_with_date, _));
manager().PresaveGeneratedPassword(generated, {}, &form_saver());

ForwardByMinute();
Expand Down Expand Up @@ -340,7 +340,7 @@ TEST_F(PasswordGenerationManagerTest, PresaveGeneratedPassword_WithConflict) {
generated_with_date.date_password_modified = base::Time::Now();
generated_with_date.username_value.clear();

EXPECT_CALL(store(), AddLogin(generated_with_date));
EXPECT_CALL(store(), AddLogin(generated_with_date, _));
manager().PresaveGeneratedPassword(generated, {&saved}, &form_saver());
EXPECT_TRUE(manager().HasGeneratedPassword());
}
Expand All @@ -354,7 +354,7 @@ TEST_F(PasswordGenerationManagerTest,
generated_with_date.date_password_modified = base::Time::Now();

const PasswordForm saved = CreateSaved();
EXPECT_CALL(store(), AddLogin(generated_with_date));
EXPECT_CALL(store(), AddLogin(generated_with_date, _));
manager().PresaveGeneratedPassword(generated, {&saved}, &form_saver());
EXPECT_TRUE(manager().HasGeneratedPassword());
}
Expand All @@ -365,7 +365,7 @@ TEST_F(PasswordGenerationManagerTest,
TEST_F(PasswordGenerationManagerTest, PresaveGeneratedPassword_ThenSaveAsNew) {
const PasswordForm generated = CreateGenerated();

EXPECT_CALL(store(), AddLogin(_));
EXPECT_CALL(store(), AddLogin);
manager().PresaveGeneratedPassword(generated, {}, &form_saver());

// User edits after submission.
Expand Down Expand Up @@ -407,7 +407,7 @@ TEST_F(PasswordGenerationManagerTest, PresaveGeneratedPassword_ThenUpdate) {
unrelated_psl_password.username_value = u"another username";
unrelated_psl_password.password_value = u"some password";

EXPECT_CALL(store(), AddLogin(_));
EXPECT_CALL(store(), AddLogin);
const std::vector<const PasswordForm*> matches = {
&related_password, &related_psl_password, &unrelated_password,
&unrelated_psl_password};
Expand Down Expand Up @@ -442,7 +442,7 @@ TEST_F(PasswordGenerationManagerTest, PresaveGeneratedPassword_ThenUpdate) {
TEST_F(PasswordGenerationManagerTest, PasswordNoLongerGenerated) {
const PasswordForm generated = CreateGenerated();

EXPECT_CALL(store(), AddLogin(_));
EXPECT_CALL(store(), AddLogin);
manager().PresaveGeneratedPassword(generated, {}, &form_saver());

EXPECT_CALL(store(), RemoveLogin(FormHasUniqueKey(generated)));
Expand All @@ -459,7 +459,7 @@ TEST_F(PasswordGenerationManagerTest,
generated_with_date.date_created = base::Time::Now();
generated_with_date.date_password_modified = base::Time::Now();

EXPECT_CALL(store(), AddLogin(generated_with_date));
EXPECT_CALL(store(), AddLogin(generated_with_date, _));
manager().PresaveGeneratedPassword(generated, {}, &form_saver());

EXPECT_CALL(store(), RemoveLogin(FormHasUniqueKey(generated_with_date)));
Expand All @@ -471,7 +471,7 @@ TEST_F(PasswordGenerationManagerTest,
generated_with_date = generated;
generated_with_date.date_created = base::Time::Now();
generated_with_date.date_password_modified = base::Time::Now();
EXPECT_CALL(store(), AddLogin(generated_with_date));
EXPECT_CALL(store(), AddLogin(generated_with_date, _));
manager().PresaveGeneratedPassword(generated, {}, &form_saver());
EXPECT_TRUE(manager().HasGeneratedPassword());
}
Expand All @@ -484,7 +484,7 @@ TEST_F(PasswordGenerationManagerTest, PresaveGeneratedPassword_CloneUpdates) {
generated_with_date.date_created = base::Time::Now();
generated_with_date.date_password_modified = base::Time::Now();

EXPECT_CALL(store(), AddLogin(generated_with_date));
EXPECT_CALL(store(), AddLogin(generated_with_date, _));
manager().PresaveGeneratedPassword(generated, {}, &form_saver());

std::unique_ptr<PasswordGenerationManager> cloned_state = manager().Clone();
Expand All @@ -507,7 +507,7 @@ TEST_F(PasswordGenerationManagerTest, PresaveGeneratedPassword_CloneSurvives) {
auto original = std::make_unique<PasswordGenerationManager>(&client());
const PasswordForm generated = CreateGenerated();

EXPECT_CALL(store(), AddLogin(_));
EXPECT_CALL(store(), AddLogin);
original->PresaveGeneratedPassword(generated, {}, &form_saver());

std::unique_ptr<PasswordGenerationManager> cloned_manager = original->Clone();
Expand Down

0 comments on commit 9269cd3

Please sign in to comment.