0

Refactoring password exporting related code to use CredentialUIEntry

This change replaces PasswordForm in password exporting code with CredentialUIEntry. This is done by removing dependency on
PasswordManagerPresenter and using SavedPasswordsPresenter.

Bug: 1330906
Change-Id: I1e075f6a1b0c26190b839ba9eae69b5bb8e73de3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688111
Commit-Queue: Viktor Semeniuk <vsemeniuk@google.com>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1012597}
This commit is contained in:
Viktor Semeniuk
2022-06-09 17:51:20 +00:00
committed by Chromium LUCI CQ
parent 0e812046ab
commit c18ea3e117
15 changed files with 193 additions and 261 deletions

@ -153,7 +153,7 @@ PasswordsPrivateDelegateImpl::PasswordsPrivateDelegateImpl(Profile* profile)
profile,
ServiceAccessType::EXPLICIT_ACCESS)),
password_manager_porter_(std::make_unique<PasswordManagerPorter>(
password_manager_presenter_.get(),
&saved_passwords_presenter_,
base::BindRepeating(
&PasswordsPrivateDelegateImpl::OnPasswordsExportProgress,
base::Unretained(this)))),

@ -32,6 +32,7 @@
#include "components/password_manager/core/browser/password_form.h"
#include "components/password_manager/core/browser/password_ui_utils.h"
#include "components/password_manager/core/browser/ui/credential_provider_interface.h"
#include "components/password_manager/core/browser/ui/credential_ui_entry.h"
#include "content/public/browser/browser_thread.h"
#include "ui/base/l10n/l10n_util.h"
#include "url/gurl.h"
@ -276,18 +277,14 @@ static jlong JNI_PasswordUIView_Init(JNIEnv* env,
PasswordUIViewAndroid::SerializationResult
PasswordUIViewAndroid::ObtainAndSerializePasswords(
const base::FilePath& target_directory) {
// This is run on a backend task runner. Do not access any member variables
// except for |credential_provider_for_testing_| and
// |password_manager_presenter_|.
password_manager::CredentialProviderInterface* const provider =
credential_provider_for_testing_ ? credential_provider_for_testing_.get()
: &password_manager_presenter_;
std::vector<std::unique_ptr<password_manager::PasswordForm>> passwords =
provider->GetAllPasswords();
std::vector<password_manager::CredentialUIEntry> credentials =
saved_passwords_presenter_.GetSavedCredentials();
base::EraseIf(credentials, [](const auto& credential) {
return credential.blocked_by_user;
});
// The UI should not trigger serialization if there are not passwords.
DCHECK(!passwords.empty());
DCHECK(!credentials.empty());
// Creating a file will block the execution on I/O.
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
@ -310,7 +307,7 @@ PasswordUIViewAndroid::ObtainAndSerializePasswords(
// Write the serialized data in CSV.
std::string data =
password_manager::PasswordCSVWriter::SerializePasswords(passwords);
password_manager::PasswordCSVWriter::SerializePasswords(credentials);
int bytes_written = base::WriteFile(export_file, data.data(), data.size());
if (bytes_written != base::checked_cast<int>(data.size())) {
return {
@ -318,7 +315,7 @@ PasswordUIViewAndroid::ObtainAndSerializePasswords(
logging::SystemErrorCodeToString(logging::GetLastSystemErrorCode())};
}
return {static_cast<int>(passwords.size()), export_file.value(),
return {static_cast<int>(credentials.size()), export_file.value(),
std::string()};
}

@ -25,7 +25,7 @@
#include "components/password_manager/core/browser/export/password_csv_writer.h"
#include "components/password_manager/core/browser/password_form.h"
#include "components/password_manager/core/browser/test_password_store.h"
#include "components/password_manager/core/browser/ui/credential_provider_interface.h"
#include "components/password_manager/core/browser/ui/credential_ui_entry.h"
#include "content/public/test/browser_task_environment.h"
#include "content/public/test/test_utils.h"
#include "testing/gtest/include/gtest/gtest.h"
@ -54,49 +54,6 @@ struct PasswordUIViewAndroidDestroyDeleter {
}
};
class FakeCredentialProvider
: public password_manager::CredentialProviderInterface {
public:
FakeCredentialProvider() = default;
FakeCredentialProvider(const FakeCredentialProvider&) = delete;
FakeCredentialProvider& operator=(const FakeCredentialProvider&) = delete;
~FakeCredentialProvider() override = default;
// password_manager::CredentialProviderInterface
std::vector<std::unique_ptr<PasswordForm>> GetAllPasswords() override;
// Adds a PasswordForm specified by the arguments to the list returned by
// GetAllPasswords.
void AddPasswordEntry(const std::string& origin,
const std::string& username,
const std::string& password);
private:
std::vector<std::unique_ptr<PasswordForm>> passwords_;
};
std::vector<std::unique_ptr<PasswordForm>>
FakeCredentialProvider::GetAllPasswords() {
std::vector<std::unique_ptr<PasswordForm>> clone;
for (const auto& password : passwords_) {
clone.push_back(std::make_unique<PasswordForm>(*password));
}
return clone;
}
void FakeCredentialProvider::AddPasswordEntry(const std::string& origin,
const std::string& username,
const std::string& password) {
auto form = std::make_unique<PasswordForm>();
form->url = GURL(origin);
form->signon_realm = origin;
form->username_value = base::UTF8ToUTF16(username);
form->password_value = base::UTF8ToUTF16(password);
passwords_.push_back(std::move(form));
}
} // namespace
class PasswordUIViewAndroidTest : public ::testing::Test {
@ -112,9 +69,34 @@ class PasswordUIViewAndroidTest : public ::testing::Test {
profiles::SetLastUsedProfile(testing_profile_->GetBaseName());
store_ = CreateAndUseTestPasswordStore(testing_profile_);
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
store_->Init(/*prefs=*/nullptr, /*affiliated_match_helper=*/nullptr);
ASSERT_TRUE(temp_dir().CreateUniqueTempDir());
}
void TearDown() override {
store_->ShutdownOnUIThread();
RunUntilIdle();
}
PasswordForm AddPasswordEntry(const std::string& origin,
const std::string& username,
const std::string& password) {
PasswordForm form;
form.url = GURL(origin);
form.signon_realm = origin;
form.username_value = base::UTF8ToUTF16(username);
form.password_value = base::UTF8ToUTF16(password);
store_->AddLogin(form);
RunUntilIdle();
return form;
}
void RunUntilIdle() { task_environment_.RunUntilIdle(); }
raw_ptr<JNIEnv> env() { return env_; }
base::ScopedTempDir& temp_dir() { return temp_dir_; }
private:
content::BrowserTaskEnvironment task_environment_;
TestingProfileManager testing_profile_manager_;
raw_ptr<TestingProfile> testing_profile_;
@ -127,26 +109,28 @@ class PasswordUIViewAndroidTest : public ::testing::Test {
// PasswordUIViewAndroid arrives at the same result as synchronous way to
// serialize the passwords.
TEST_F(PasswordUIViewAndroidTest, GetSerializedPasswords) {
FakeCredentialProvider provider;
provider.AddPasswordEntry("https://example.com", "username", "password");
PasswordForm form =
AddPasswordEntry("https://example.com", "username", "password");
// Let the PasswordCSVWriter compute the result instead of hard-coding it,
// because this test focuses on PasswordUIView and not on detecting changes in
// PasswordCSVWriter.
const std::string expected_result =
password_manager::PasswordCSVWriter::SerializePasswords(
provider.GetAllPasswords());
{password_manager::CredentialUIEntry(form)});
std::unique_ptr<PasswordUIViewAndroid, PasswordUIViewAndroidDestroyDeleter>
password_ui_view(
new PasswordUIViewAndroid(env_, JavaParamRef<jobject>(nullptr)));
new PasswordUIViewAndroid(env(), JavaParamRef<jobject>(nullptr)));
// SavedPasswordsPresenter needs time to initialize and fetch passwords.
RunUntilIdle();
PasswordUIViewAndroid::SerializationResult serialized_passwords;
password_ui_view->set_export_target_for_testing(&serialized_passwords);
password_ui_view->set_credential_provider_for_testing(&provider);
password_ui_view->HandleSerializePasswords(
env_, nullptr,
env(), nullptr,
base::android::ConvertUTF8ToJavaString(
env_, temp_dir_.GetPath().AsUTF8Unsafe()),
env(), temp_dir().GetPath().AsUTF8Unsafe()),
nullptr, nullptr);
content::RunAllTasksUntilIdle();
@ -168,23 +152,24 @@ TEST_F(PasswordUIViewAndroidTest, GetSerializedPasswords) {
// Test that destroying the PasswordUIView when tasks are pending does not lead
// to crashes.
TEST_F(PasswordUIViewAndroidTest, GetSerializedPasswords_Cancelled) {
FakeCredentialProvider provider;
provider.AddPasswordEntry("https://example.com", "username", "password");
AddPasswordEntry("https://example.com", "username", "password");
std::unique_ptr<PasswordUIViewAndroid, PasswordUIViewAndroidDestroyDeleter>
password_ui_view(
new PasswordUIViewAndroid(env_, JavaParamRef<jobject>(nullptr)));
new PasswordUIViewAndroid(env(), JavaParamRef<jobject>(nullptr)));
// SavedPasswordsPresenter needs time to initialize and fetch passwords.
RunUntilIdle();
PasswordUIViewAndroid::SerializationResult serialized_passwords;
serialized_passwords.entries_count = 123;
serialized_passwords.exported_file_path = "somepath";
password_ui_view->set_export_target_for_testing(&serialized_passwords);
password_ui_view->set_credential_provider_for_testing(&provider);
base::android::ScopedJavaLocalRef<jstring> java_target_dir =
base::android::ConvertUTF8ToJavaString(
env_, temp_dir_.GetPath().AsUTF8Unsafe());
env(), temp_dir().GetPath().AsUTF8Unsafe());
password_ui_view->HandleSerializePasswords(
env_, nullptr,
base::android::JavaParamRef<jstring>(env_, java_target_dir.obj()),
env(), nullptr,
base::android::JavaParamRef<jstring>(env(), java_target_dir.obj()),
nullptr, nullptr);
// Register the PasswordUIView for deletion. It should not destruct itself
// before the background tasks are run. The results of the background tasks
@ -200,22 +185,23 @@ TEST_F(PasswordUIViewAndroidTest, GetSerializedPasswords_Cancelled) {
// Test that an I/O error is reported.
TEST_F(PasswordUIViewAndroidTest, GetSerializedPasswords_WriteFailed) {
FakeCredentialProvider provider;
provider.AddPasswordEntry("https://example.com", "username", "password");
AddPasswordEntry("https://example.com", "username", "password");
std::unique_ptr<PasswordUIViewAndroid, PasswordUIViewAndroidDestroyDeleter>
password_ui_view(
new PasswordUIViewAndroid(env_, JavaParamRef<jobject>(nullptr)));
new PasswordUIViewAndroid(env(), JavaParamRef<jobject>(nullptr)));
// SavedPasswordsPresenter needs time to initialize and fetch passwords.
RunUntilIdle();
PasswordUIViewAndroid::SerializationResult serialized_passwords;
password_ui_view->set_export_target_for_testing(&serialized_passwords);
password_ui_view->set_credential_provider_for_testing(&provider);
base::android::ScopedJavaLocalRef<jstring> java_temp_file =
base::android::ConvertUTF8ToJavaString(
env_, "/This directory cannot be created");
env(), "/This directory cannot be created");
password_ui_view->HandleSerializePasswords(
env_, nullptr,
base::android::JavaParamRef<jstring>(env_, java_temp_file.obj()), nullptr,
nullptr);
env(), nullptr,
base::android::JavaParamRef<jstring>(env(), java_temp_file.obj()),
nullptr, nullptr);
content::RunAllTasksUntilIdle();
EXPECT_EQ(0, serialized_passwords.entries_count);
EXPECT_FALSE(serialized_passwords.error.empty());

@ -113,10 +113,9 @@ void PasswordImportConsumer::ConsumePassword(
} // namespace
PasswordManagerPorter::PasswordManagerPorter(
password_manager::CredentialProviderInterface*
credential_provider_interface,
password_manager::SavedPasswordsPresenter* presenter,
ProgressCallback on_export_progress_callback)
: credential_provider_interface_(credential_provider_interface),
: presenter_(presenter),
on_export_progress_callback_(on_export_progress_callback) {}
PasswordManagerPorter::~PasswordManagerPorter() = default;
@ -132,11 +131,10 @@ bool PasswordManagerPorter::Store() {
}
// Set a new exporter for this request.
exporter_ =
exporter_for_testing_
? std::move(exporter_for_testing_)
: std::make_unique<password_manager::PasswordManagerExporter>(
credential_provider_interface_, on_export_progress_callback_);
exporter_ = exporter_for_testing_
? std::move(exporter_for_testing_)
: std::make_unique<password_manager::PasswordManagerExporter>(
presenter_, on_export_progress_callback_);
// Start serialising while the user selects a file.
exporter_->PreparePasswordsForExport();

@ -20,7 +20,7 @@ class WebContents;
}
namespace password_manager {
class CredentialProviderInterface;
class SavedPasswordsPresenter;
class PasswordManagerExporter;
} // namespace password_manager
@ -36,11 +36,10 @@ class PasswordManagerPorter : public ui::SelectFileDialog::Listener,
base::RepeatingCallback<void(password_manager::ExportProgressStatus,
const std::string&)>;
// |credential_provider_interface| provides the credentials which can be
// exported. |on_export_progress_callback| will be called with updates to
// the progress of exporting.
PasswordManagerPorter(password_manager::CredentialProviderInterface*
credential_provider_interface,
// |presenter| provides the credentials which can be exported.
// |on_export_progress_callback| will be called with updates to the progress
// of exporting.
PasswordManagerPorter(password_manager::SavedPasswordsPresenter* presenter,
ProgressCallback on_export_progress_callback);
PasswordManagerPorter(const PasswordManagerPorter&) = delete;
@ -96,11 +95,10 @@ class PasswordManagerPorter : public ui::SelectFileDialog::Listener,
scoped_refptr<ui::SelectFileDialog> select_file_dialog_;
Profile* profile_ = nullptr;
// We store |credential_provider_interface_| and
// We store |presenter_| and
// |on_export_progress_callback_| to use them to create a new
// PasswordManagerExporter instance for each export.
raw_ptr<password_manager::CredentialProviderInterface>
credential_provider_interface_;
raw_ptr<password_manager::SavedPasswordsPresenter> presenter_;
ProgressCallback on_export_progress_callback_;
// If |exporter_for_testing_| is set, the next export will make it the current
// exporter, instead of creating a new instance.

@ -6,7 +6,7 @@
#include "base/strings/utf_string_conversions.h"
#include "components/password_manager/core/browser/export/csv_writer.h"
#include "components/password_manager/core/browser/password_form.h"
#include "components/password_manager/core/browser/ui/credential_ui_entry.h"
namespace password_manager {
@ -21,7 +21,7 @@ const char kPasswordColumnName[] = "password";
// static
std::string PasswordCSVWriter::SerializePasswords(
const std::vector<std::unique_ptr<PasswordForm>>& passwords) {
const std::vector<CredentialUIEntry>& credentials) {
std::vector<std::string> header(4);
header[0] = kTitleColumnName;
header[1] = kUrlColumnName;
@ -29,9 +29,9 @@ std::string PasswordCSVWriter::SerializePasswords(
header[3] = kPasswordColumnName;
std::vector<std::map<std::string, std::string>> records;
records.reserve(passwords.size());
for (const auto& password : passwords) {
records.push_back(PasswordFormToRecord(*password));
records.reserve(credentials.size());
for (const auto& credential : credentials) {
records.push_back(PasswordFormToRecord(credential));
}
std::string result;
@ -40,12 +40,12 @@ std::string PasswordCSVWriter::SerializePasswords(
}
std::map<std::string, std::string> PasswordCSVWriter::PasswordFormToRecord(
const PasswordForm& form) {
const CredentialUIEntry& credential) {
std::map<std::string, std::string> record;
record[kUrlColumnName] = form.url.spec();
record[kUsernameColumnName] = base::UTF16ToUTF8(form.username_value);
record[kPasswordColumnName] = base::UTF16ToUTF8(form.password_value);
record[kTitleColumnName] = form.url.host();
record[kUrlColumnName] = credential.url.spec();
record[kUsernameColumnName] = base::UTF16ToUTF8(credential.username);
record[kPasswordColumnName] = base::UTF16ToUTF8(credential.password);
record[kTitleColumnName] = credential.url.host();
return record;
}

@ -12,7 +12,7 @@
namespace password_manager {
struct PasswordForm;
struct CredentialUIEntry;
// Static-only class bundling together the API for serializing passwords into
// CSV format.
@ -22,16 +22,17 @@ class PasswordCSVWriter {
PasswordCSVWriter(const PasswordCSVWriter&) = delete;
PasswordCSVWriter& operator=(const PasswordCSVWriter&) = delete;
// Creates a CSV representation of the forms stored in |password|. Note that
// this loses all the metadata except for the origin, username and password.
// Creates a CSV representation of the credential stored in |credentials|.
// Note that this loses all the metadata except for the origin, username and
// password.
static std::string SerializePasswords(
const std::vector<std::unique_ptr<PasswordForm>>& passwords);
const std::vector<CredentialUIEntry>& credentials);
private:
// Converts |form| into a single line in the CSV format. Metadata are lost,
// see SerializePasswords.
// Converts |credential| into a single line in the CSV format. Metadata are
// lost, see SerializePasswords.
static std::map<std::string, std::string> PasswordFormToRecord(
const PasswordForm& form);
const CredentialUIEntry& credential);
};
} // namespace password_manager

@ -10,7 +10,7 @@
#include "base/strings/utf_string_conversions.h"
#include "components/password_manager/core/browser/import/csv_password.h"
#include "components/password_manager/core/browser/import/csv_password_sequence.h"
#include "components/password_manager/core/browser/password_form.h"
#include "components/password_manager/core/browser/ui/credential_ui_entry.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
@ -23,58 +23,58 @@ namespace {
MATCHER_P3(FormHasOriginUsernamePassword, origin, username, password, "") {
return arg.signon_realm == origin && arg.url == GURL(origin) &&
arg.username_value == base::UTF8ToUTF16(username) &&
arg.password_value == base::UTF8ToUTF16(password);
arg.username == base::UTF8ToUTF16(username) &&
arg.password == base::UTF8ToUTF16(password);
}
} // namespace
TEST(PasswordCSVWriterTest, SerializePasswords_ZeroPasswords) {
std::vector<std::unique_ptr<PasswordForm>> passwords;
std::vector<CredentialUIEntry> credentials;
CSVPasswordSequence seq(PasswordCSVWriter::SerializePasswords(passwords));
CSVPasswordSequence seq(PasswordCSVWriter::SerializePasswords(credentials));
ASSERT_EQ(CSVPassword::Status::kOK, seq.result());
EXPECT_EQ(seq.begin(), seq.end());
}
TEST(PasswordCSVWriterTest, SerializePasswords_SinglePassword) {
std::vector<std::unique_ptr<PasswordForm>> passwords;
std::vector<CredentialUIEntry> credentials;
PasswordForm form;
form.url = GURL("http://example.com");
form.username_value = u"Someone";
form.password_value = u"Secret";
passwords.push_back(std::make_unique<PasswordForm>(form));
credentials.emplace_back(form);
CSVPasswordSequence seq(PasswordCSVWriter::SerializePasswords(passwords));
CSVPasswordSequence seq(PasswordCSVWriter::SerializePasswords(credentials));
ASSERT_EQ(CSVPassword::Status::kOK, seq.result());
std::vector<PasswordForm> pwds;
std::vector<CredentialUIEntry> pwds;
for (const auto& pwd : seq) {
pwds.push_back(pwd.ToPasswordForm());
pwds.emplace_back(pwd.ToPasswordForm());
}
EXPECT_THAT(pwds, ElementsAre(FormHasOriginUsernamePassword(
"http://example.com/", "Someone", "Secret")));
}
TEST(PasswordCSVWriterTest, SerializePasswords_TwoPasswords) {
std::vector<std::unique_ptr<PasswordForm>> passwords;
std::vector<CredentialUIEntry> credentials;
PasswordForm form;
form.url = GURL("http://example.com");
form.username_value = u"Someone";
form.password_value = u"Secret";
passwords.push_back(std::make_unique<PasswordForm>(form));
credentials.emplace_back(form);
form.url = GURL("http://other.org");
form.username_value = u"Anyone";
form.password_value = u"None";
passwords.push_back(std::make_unique<PasswordForm>(form));
credentials.emplace_back(form);
CSVPasswordSequence seq(PasswordCSVWriter::SerializePasswords(passwords));
CSVPasswordSequence seq(PasswordCSVWriter::SerializePasswords(credentials));
ASSERT_EQ(CSVPassword::Status::kOK, seq.result());
std::vector<PasswordForm> pwds;
std::vector<CredentialUIEntry> pwds;
for (const auto& pwd : seq) {
pwds.push_back(pwd.ToPasswordForm());
pwds.emplace_back(pwd.ToPasswordForm());
}
EXPECT_THAT(pwds, ElementsAre(FormHasOriginUsernamePassword(
"http://example.com/", "Someone", "Secret"),

@ -15,10 +15,9 @@
#include "base/task/task_runner_util.h"
#include "build/build_config.h"
#include "components/password_manager/core/browser/export/password_csv_writer.h"
#include "components/password_manager/core/browser/password_form.h"
#include "components/password_manager/core/browser/password_list_sorter.h"
#include "components/password_manager/core/browser/password_manager_metrics_util.h"
#include "components/password_manager/core/browser/ui/credential_provider_interface.h"
#include "components/password_manager/core/browser/ui/credential_ui_entry.h"
#include "components/password_manager/core/browser/ui/saved_passwords_presenter.h"
namespace password_manager {
@ -58,25 +57,12 @@ bool DefaultDeleteFunction(const base::FilePath& file) {
return base::DeleteFile(file);
}
std::vector<std::unique_ptr<PasswordForm>> DeduplicatePasswordsAcrossStores(
std::vector<std::unique_ptr<PasswordForm>> passwords) {
auto get_sort_key = [](const auto& password) {
return CreateSortKey(*password, IgnoreStore(true));
};
auto cmp = [&](const auto& lhs, const auto& rhs) {
return get_sort_key(lhs) < get_sort_key(rhs);
};
base::flat_set<std::unique_ptr<PasswordForm>, decltype(cmp)> unique_passwords(
std::move(passwords), cmp);
return std::move(unique_passwords).extract();
}
} // namespace
PasswordManagerExporter::PasswordManagerExporter(
CredentialProviderInterface* credential_provider_interface,
SavedPasswordsPresenter* presenter,
ProgressCallback on_progress)
: credential_provider_interface_(credential_provider_interface),
: presenter_(presenter),
on_progress_(std::move(on_progress)),
last_progress_status_(ExportProgressStatus::NOT_STARTED),
write_function_(base::BindRepeating(&DefaultWriteFunction)),
@ -96,22 +82,18 @@ PasswordManagerExporter::~PasswordManagerExporter() = default;
void PasswordManagerExporter::PreparePasswordsForExport() {
DCHECK_EQ(GetProgressStatus(), ExportProgressStatus::NOT_STARTED);
std::vector<std::unique_ptr<PasswordForm>> password_list =
credential_provider_interface_->GetAllPasswords();
std::vector<CredentialUIEntry> credentials =
presenter_->GetSavedCredentials();
// Clear blocked credentials.
base::EraseIf(credentials, [](const auto& credential) {
return credential.blocked_by_user;
});
// Deduplicate passwords that are present in multiple stores, so the output
// file doesn't contain repeated data.
std::vector<std::unique_ptr<PasswordForm>> deduplicated_password_list =
DeduplicatePasswordsAcrossStores(std::move(password_list));
size_t deduplicated_password_list_size = deduplicated_password_list.size();
base::PostTaskAndReplyWithResult(
task_runner_.get(), FROM_HERE,
base::BindOnce(&PasswordCSVWriter::SerializePasswords,
std::move(deduplicated_password_list)),
base::BindOnce(&PasswordCSVWriter::SerializePasswords, credentials),
base::BindOnce(&PasswordManagerExporter::SetSerialisedPasswordList,
weak_factory_.GetWeakPtr(),
deduplicated_password_list_size));
weak_factory_.GetWeakPtr(), credentials.size()));
}
void PasswordManagerExporter::SetDestination(

@ -17,7 +17,7 @@
namespace password_manager {
class CredentialProviderInterface;
class SavedPasswordsPresenter;
// Controls the exporting of passwords. One instance per export flow.
// PasswordManagerExporter will perform the export asynchronously as soon as all
@ -33,9 +33,8 @@ class PasswordManagerExporter {
using SetPosixFilePermissionsCallback =
base::RepeatingCallback<bool(const base::FilePath&, int)>;
explicit PasswordManagerExporter(
CredentialProviderInterface* credential_provider_interface,
ProgressCallback on_progress);
explicit PasswordManagerExporter(SavedPasswordsPresenter* presenter,
ProgressCallback on_progress);
PasswordManagerExporter(const PasswordManagerExporter&) = delete;
PasswordManagerExporter& operator=(const PasswordManagerExporter&) = delete;
@ -96,7 +95,7 @@ class PasswordManagerExporter {
void Cleanup();
// The source of the password list which will be exported.
const raw_ptr<CredentialProviderInterface> credential_provider_interface_;
const raw_ptr<SavedPasswordsPresenter> presenter_;
// Callback to the UI.
ProgressCallback on_progress_;

@ -15,10 +15,11 @@
#include "base/test/task_environment.h"
#include "build/build_config.h"
#include "components/password_manager/core/browser/export/password_csv_writer.h"
#include "components/password_manager/core/browser/password_form.h"
#include "components/password_manager/core/browser/password_manager_metrics_util.h"
#include "components/password_manager/core/browser/ui/credential_provider_interface.h"
#include "components/password_manager/core/browser/test_password_store.h"
#include "components/password_manager/core/browser/ui/credential_ui_entry.h"
#include "components/password_manager/core/browser/ui/export_progress_status.h"
#include "components/password_manager/core/browser/ui/saved_passwords_presenter.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
@ -50,68 +51,50 @@ const base::FilePath::CharType kNullFileName[] = FILE_PATH_LITERAL("/nul");
const base::FilePath::CharType kNullFileName[] = FILE_PATH_LITERAL("/dev/null");
#endif
// Provides a predetermined set of credentials
class FakeCredentialProvider : public CredentialProviderInterface {
public:
FakeCredentialProvider() = default;
FakeCredentialProvider(const FakeCredentialProvider&) = delete;
FakeCredentialProvider& operator=(const FakeCredentialProvider&) = delete;
void SetPasswordList(
const std::vector<std::unique_ptr<PasswordForm>>& password_list) {
password_list_.clear();
for (const auto& form : password_list) {
password_list_.push_back(std::make_unique<PasswordForm>(*form));
}
}
// CredentialProviderInterface:
std::vector<std::unique_ptr<PasswordForm>> GetAllPasswords() override {
std::vector<std::unique_ptr<PasswordForm>> ret_val;
for (const auto& form : password_list_) {
ret_val.push_back(std::make_unique<PasswordForm>(*form));
}
return ret_val;
}
private:
std::vector<std::unique_ptr<PasswordForm>> password_list_;
};
// Creates a hardcoded set of credentials for tests.
std::vector<std::unique_ptr<PasswordForm>> CreatePasswordList() {
auto password_form = std::make_unique<PasswordForm>();
password_form->url = GURL("http://accounts.google.com/a/LoginAuth");
password_form->username_value = u"test@gmail.com";
password_form->password_value = u"test1";
std::vector<std::unique_ptr<PasswordForm>> password_forms;
password_forms.push_back(std::move(password_form));
return password_forms;
PasswordForm CreateTestPassword() {
PasswordForm password_form;
password_form.url = GURL("http://accounts.google.com/a/LoginAuth");
password_form.username_value = u"test@gmail.com";
password_form.password_value = u"test1";
password_form.in_store = PasswordForm::Store::kProfileStore;
return password_form;
}
class PasswordManagerExporterTest : public testing::Test {
public:
PasswordManagerExporterTest()
: task_environment_(base::test::TaskEnvironment::MainThreadType::UI),
exporter_(&fake_credential_provider_, mock_on_progress_.Get()),
exporter_(&presenter_, mock_on_progress_.Get()),
destination_path_(kNullFileName) {
exporter_.SetWriteForTesting(mock_write_file_.Get());
exporter_.SetDeleteForTesting(mock_delete_file_.Get());
exporter_.SetSetPosixFilePermissionsForTesting(
mock_set_posix_file_permissions_.Get());
store_->Init(/*prefs=*/nullptr, /*affiliated_match_helper=*/nullptr);
}
PasswordManagerExporterTest(const PasswordManagerExporterTest&) = delete;
PasswordManagerExporterTest& operator=(const PasswordManagerExporterTest&) =
delete;
~PasswordManagerExporterTest() override = default;
~PasswordManagerExporterTest() override {
store_->ShutdownOnUIThread();
task_environment_.RunUntilIdle();
}
void SetPasswordList(const std::vector<PasswordForm>& forms) {
for (const auto& form : forms) {
store_->AddLogin(form);
}
task_environment_.RunUntilIdle();
}
protected:
base::test::TaskEnvironment task_environment_;
FakeCredentialProvider fake_credential_provider_;
scoped_refptr<TestPasswordStore> store_ =
base::MakeRefCounted<TestPasswordStore>();
SavedPasswordsPresenter presenter_{store_};
base::MockCallback<
base::RepeatingCallback<void(ExportProgressStatus, const std::string&)>>
mock_on_progress_;
@ -125,11 +108,10 @@ class PasswordManagerExporterTest : public testing::Test {
};
TEST_F(PasswordManagerExporterTest, PasswordExportSetPasswordListFirst) {
std::vector<std::unique_ptr<PasswordForm>> password_list =
CreatePasswordList();
fake_credential_provider_.SetPasswordList(password_list);
PasswordForm form = CreateTestPassword();
SetPasswordList({form});
const std::string serialised(
PasswordCSVWriter::SerializePasswords(password_list));
PasswordCSVWriter::SerializePasswords({CredentialUIEntry(form)}));
EXPECT_CALL(mock_write_file_, Run(destination_path_, StrEq(serialised)))
.WillOnce(Return(true));
@ -147,7 +129,7 @@ TEST_F(PasswordManagerExporterTest, PasswordExportSetPasswordListFirst) {
// When writing fails, we should notify the UI of the failure and try to cleanup
// a possibly partial passwords file.
TEST_F(PasswordManagerExporterTest, WriteFileFailed) {
fake_credential_provider_.SetPasswordList(CreatePasswordList());
SetPasswordList({CreateTestPassword()});
const std::string destination_folder_name(
destination_path_.DirName().BaseName().AsUTF8Unsafe());
@ -167,11 +149,10 @@ TEST_F(PasswordManagerExporterTest, WriteFileFailed) {
// Test that GetProgressStatus() returns the last ExportProgressStatus sent
// to the callback.
TEST_F(PasswordManagerExporterTest, GetProgressReturnsLastCallbackStatus) {
std::vector<std::unique_ptr<PasswordForm>> password_list =
CreatePasswordList();
fake_credential_provider_.SetPasswordList(password_list);
PasswordForm form = CreateTestPassword();
SetPasswordList({form});
const std::string serialised(
PasswordCSVWriter::SerializePasswords(password_list));
PasswordCSVWriter::SerializePasswords({CredentialUIEntry(form)}));
const std::string destination_folder_name(
destination_path_.DirName().BaseName().AsUTF8Unsafe());
@ -191,7 +172,7 @@ TEST_F(PasswordManagerExporterTest, GetProgressReturnsLastCallbackStatus) {
}
TEST_F(PasswordManagerExporterTest, DontExportWithOnlyDestination) {
fake_credential_provider_.SetPasswordList(CreatePasswordList());
SetPasswordList({CreateTestPassword()});
EXPECT_CALL(mock_write_file_, Run(_, _)).Times(0);
EXPECT_CALL(mock_on_progress_,
@ -203,7 +184,7 @@ TEST_F(PasswordManagerExporterTest, DontExportWithOnlyDestination) {
}
TEST_F(PasswordManagerExporterTest, CancelAfterPasswords) {
fake_credential_provider_.SetPasswordList(CreatePasswordList());
SetPasswordList({CreateTestPassword()});
EXPECT_CALL(mock_write_file_, Run(_, _)).Times(0);
EXPECT_CALL(mock_on_progress_,
@ -216,7 +197,7 @@ TEST_F(PasswordManagerExporterTest, CancelAfterPasswords) {
}
TEST_F(PasswordManagerExporterTest, CancelWhileExporting) {
fake_credential_provider_.SetPasswordList(CreatePasswordList());
SetPasswordList({CreateTestPassword()});
EXPECT_CALL(mock_write_file_, Run(_, _)).Times(0);
EXPECT_CALL(mock_delete_file_, Run(destination_path_));
@ -235,7 +216,7 @@ TEST_F(PasswordManagerExporterTest, CancelWhileExporting) {
// The "Cancel" button may still be visible on the UI after we've completed
// exporting. If they choose to cancel, we should clear the file.
TEST_F(PasswordManagerExporterTest, CancelAfterExporting) {
fake_credential_provider_.SetPasswordList(CreatePasswordList());
SetPasswordList({CreateTestPassword()});
EXPECT_CALL(mock_write_file_, Run(_, _)).WillOnce(Return(true));
EXPECT_CALL(mock_delete_file_, Run(destination_path_));
@ -259,7 +240,7 @@ TEST_F(PasswordManagerExporterTest, CancelAfterExporting) {
// Chrome creates files using the broadest permissions allowed. Passwords are
// sensitive and should be explicitly limited to the owner.
TEST_F(PasswordManagerExporterTest, OutputHasRestrictedPermissions) {
fake_credential_provider_.SetPasswordList(CreatePasswordList());
SetPasswordList({CreateTestPassword()});
EXPECT_CALL(mock_write_file_, Run(_, _)).WillOnce(Return(true));
EXPECT_CALL(mock_set_posix_file_permissions_, Run(destination_path_, 0600))
@ -274,21 +255,18 @@ TEST_F(PasswordManagerExporterTest, OutputHasRestrictedPermissions) {
#endif
TEST_F(PasswordManagerExporterTest, DeduplicatesAcrossPasswordStores) {
auto password = std::make_unique<PasswordForm>();
password->in_store = PasswordForm::Store::kProfileStore;
password->url = GURL("http://g.com/auth");
password->username_value = u"user";
password->password_value = u"password";
PasswordForm password;
password.in_store = PasswordForm::Store::kProfileStore;
password.url = GURL("http://g.com/auth");
password.username_value = u"user";
password.password_value = u"password";
auto password_duplicate = std::make_unique<PasswordForm>(*password);
password_duplicate->in_store = PasswordForm::Store::kAccountStore;
PasswordForm password_duplicate = password;
password_duplicate.in_store = PasswordForm::Store::kAccountStore;
std::vector<std::unique_ptr<PasswordForm>> password_list;
password_list.push_back(std::move(password));
const std::string single_password_serialised(
PasswordCSVWriter::SerializePasswords(password_list));
password_list.push_back(std::move(password_duplicate));
fake_credential_provider_.SetPasswordList(password_list);
PasswordCSVWriter::SerializePasswords({CredentialUIEntry(password)}));
SetPasswordList({password, password_duplicate});
// The content written to the file should be the same as what would be
// computed before the duplicated password was added.

@ -12,7 +12,7 @@
#include <vector>
namespace password_manager {
struct PasswordForm;
struct CredentialUIEntry;
} // namespace password_manager
enum class WriteToURLStatus {
@ -44,8 +44,7 @@ enum class ExportState {
// Posts task to serialize passwords and calls `serializedPasswordsHandler`
// when serialization is finished.
- (void)serializePasswords:
(std::vector<std::unique_ptr<password_manager::PasswordForm>>)
passwords
(const std::vector<password_manager::CredentialUIEntry>&)passwords
handler:(void (^)(std::string))serializedPasswordsHandler;
@end
@ -108,7 +107,7 @@ enum class ExportState {
// Method to be called in order to start the export flow. This initiates
// the reauthentication procedure and asks for password serialization.
- (void)startExportFlow:
(std::vector<std::unique_ptr<password_manager::PasswordForm>>)passwords;
(const std::vector<password_manager::CredentialUIEntry>&)passwords;
// Called when the user cancels the export operation.
- (void)cancelExport;

@ -14,8 +14,8 @@
#include "base/task/thread_pool.h"
#include "base/threading/scoped_blocking_call.h"
#include "components/password_manager/core/browser/export/password_csv_writer.h"
#include "components/password_manager/core/browser/password_form.h"
#include "components/password_manager/core/browser/password_manager_metrics_util.h"
#include "components/password_manager/core/browser/ui/credential_ui_entry.h"
#include "components/password_manager/core/common/passwords_directory_util_ios.h"
#include "components/strings/grit/components_strings.h"
#import "ios/chrome/common/ui/reauthentication/reauthentication_module.h"
@ -45,8 +45,7 @@ enum class ReauthenticationStatus {
@implementation PasswordSerializerBridge
- (void)serializePasswords:
(std::vector<std::unique_ptr<password_manager::PasswordForm>>)
passwords
(const std::vector<password_manager::CredentialUIEntry>&)passwords
handler:(void (^)(std::string))serializedPasswordsHandler {
base::ThreadPool::PostTaskAndReplyWithResult(
FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_BLOCKING},
@ -166,7 +165,7 @@ enum class ReauthenticationStatus {
}
- (void)startExportFlow:
(std::vector<std::unique_ptr<password_manager::PasswordForm>>)passwords {
(const std::vector<password_manager::CredentialUIEntry>&)passwords {
DCHECK(!passwords.empty());
DCHECK(self.exportState == ExportState::IDLE);
if ([_weakReauthenticationModule canAttemptReauth]) {
@ -190,7 +189,7 @@ enum class ReauthenticationStatus {
}
- (void)serializePasswords:
(std::vector<std::unique_ptr<password_manager::PasswordForm>>)passwords {
(const std::vector<password_manager::CredentialUIEntry>&)passwords {
self.passwordCount = passwords.size();
__weak PasswordExporter* weakSelf = self;

@ -9,6 +9,7 @@
#include "base/test/task_environment.h"
#include "components/password_manager/core/browser/password_form.h"
#include "components/password_manager/core/browser/password_manager_metrics_util.h"
#include "components/password_manager/core/browser/ui/credential_ui_entry.h"
#include "components/strings/grit/components_strings.h"
#include "ios/chrome/grit/ios_strings.h"
#import "ios/chrome/test/app/password_test_util.h"
@ -35,8 +36,7 @@
}
- (void)serializePasswords:
(std::vector<std::unique_ptr<password_manager::PasswordForm>>)
passwords
(const std::vector<password_manager::CredentialUIEntry>&)passwords
handler:(void (^)(std::string))serializedPasswordsHandler {
_serializedPasswordsHandler = serializedPasswordsHandler;
}
@ -107,16 +107,13 @@ class PasswordExporterTest : public PlatformTest {
delegate:password_exporter_delegate_];
}
std::vector<std::unique_ptr<password_manager::PasswordForm>>
CreatePasswordList() {
auto password_form = std::make_unique<password_manager::PasswordForm>();
password_form->url = GURL("http://accounts.google.com/a/LoginAuth");
password_form->username_value = u"test@testmail.com";
password_form->password_value = u"test1";
std::vector<password_manager::CredentialUIEntry> CreatePasswordList() {
password_manager::PasswordForm password_form;
password_form.url = GURL("http://accounts.google.com/a/LoginAuth");
password_form.username_value = u"test@testmail.com";
password_form.password_value = u"test1";
std::vector<std::unique_ptr<password_manager::PasswordForm>> password_forms;
password_forms.push_back(std::move(password_form));
return password_forms;
return {password_manager::CredentialUIEntry(password_form)};
}
id password_exporter_delegate_;

@ -13,11 +13,11 @@
#include "base/strings/sys_string_conversions.h"
#include "components/google/core/common/google_util.h"
#include "components/keyed_service/core/service_access_type.h"
#include "components/password_manager/core/browser/password_form.h"
#include "components/password_manager/core/browser/password_list_sorter.h"
#include "components/password_manager/core/browser/password_manager_constants.h"
#include "components/password_manager/core/browser/password_manager_metrics_util.h"
#include "components/password_manager/core/browser/password_ui_utils.h"
#include "components/password_manager/core/browser/ui/credential_ui_entry.h"
#include "components/password_manager/core/browser/ui/password_check_referrer.h"
#include "components/password_manager/core/common/password_manager_features.h"
#include "components/password_manager/core/common/password_manager_pref_names.h"
@ -110,15 +110,13 @@ typedef NS_ENUM(NSInteger, ItemType) {
ItemTypeOnDeviceEncryptionOptedInLearnMore,
};
std::vector<std::unique_ptr<password_manager::PasswordForm>> CopyOf(
std::vector<password_manager::CredentialUIEntry> CopyOf(
const std::vector<password_manager::PasswordForm>& password_list) {
std::vector<std::unique_ptr<password_manager::PasswordForm>>
password_list_copy;
std::vector<password_manager::CredentialUIEntry> credentials;
for (const auto& form : password_list) {
password_list_copy.push_back(
std::make_unique<password_manager::PasswordForm>(form));
credentials.push_back(password_manager::CredentialUIEntry(form));
}
return password_list_copy;
return credentials;
}
bool ArePasswordsListsEqual(