Start the migration of passwords from the Keychain.

chrome/browser/password_manager/password_store_proxy_mac_unittest.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/password_manager/password_store_proxy_mac.h"
 
 #include "base/files/scoped_temp_dir.h"
 #include "base/scoped_observer.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/password_manager/password_store_mac_internal.h"
+#include "chrome/browser/prefs/browser_prefs.h"
+#include "chrome/test/base/testing_pref_service_syncable.h"
 #include "components/os_crypt/os_crypt.h"
 #include "components/password_manager/core/browser/login_database.h"
 #include "components/password_manager/core/browser/password_manager_test_utils.h"
 #include "components/password_manager/core/browser/password_store_consumer.h"
+#include "components/password_manager/core/common/password_manager_pref_names.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "crypto/mock_apple_keychain.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
 using autofill::PasswordForm;
 using content::BrowserThread;
+using password_manager::PasswordStoreChange;
+using password_manager::PasswordStoreChangeList;
 using testing::_;
 using testing::ElementsAre;
 using testing::IsEmpty;
 using testing::Pointee;
 
 ACTION(QuitUIMessageLoop) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   base::MessageLoop::current()->Quit();
 }
 
+PasswordStoreChangeList AddChangeForForm(const PasswordForm& form) {
+  return PasswordStoreChangeList(
+      1, PasswordStoreChange(PasswordStoreChange::ADD, form));
+}
+
 class MockPasswordStoreConsumer
     : public password_manager::PasswordStoreConsumer {
  public:
   MOCK_METHOD1(OnGetPasswordStoreResultsConstRef,
                void(const std::vector<PasswordForm*>&));
 
   // GMock cannot mock methods with move-only args.
   void OnGetPasswordStoreResults(ScopedVector<PasswordForm> results) override {
     OnGetPasswordStoreResultsConstRef(results.get());
   }
@@ skipping 19 matching lines @@
   BadLoginDatabase() : password_manager::LoginDatabase(base::FilePath()) {}
   ~BadLoginDatabase() override {}
 
   // LoginDatabase:
   bool Init() override { return false; }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(BadLoginDatabase);
 };
 
-class PasswordStoreProxyMacTest : public testing::Test {
+class PasswordStoreProxyMacTest
+    : public testing::TestWithParam<PasswordStoreProxyMac::MigrationStatus> {
  public:
+  PasswordStoreProxyMacTest();
+  ~PasswordStoreProxyMacTest() override;
+
   void SetUp() override;
   void TearDown() override;
 
   void CreateAndInitPasswordStore(
       scoped_ptr<password_manager::LoginDatabase> login_db);
 
   void ClosePasswordStore();
 
   // Do a store-level query to wait for all the previously enqueued operations
   // to finish.
@@ skipping 14 matching lines @@
     return store_->login_metadata_db();
   }
 
   PasswordStoreProxyMac* store() { return store_.get(); }
 
  protected:
   content::TestBrowserThreadBundle ui_thread_;
 
   base::ScopedTempDir db_dir_;
   scoped_refptr<PasswordStoreProxyMac> store_;
+  TestingPrefServiceSyncable testing_prefs_;
 };
 
-void PasswordStoreProxyMacTest::SetUp() {
-  ASSERT_TRUE(db_dir_.CreateUniqueTempDir());
-
+PasswordStoreProxyMacTest::PasswordStoreProxyMacTest() {
+  EXPECT_TRUE(db_dir_.CreateUniqueTempDir());
+  chrome::RegisterUserProfilePrefs(testing_prefs_.registry());
+  testing_prefs_.SetInteger(password_manager::prefs::kKeychainMigrationStatus,
+                            GetParam());
   // Ensure that LoginDatabase will use the mock keychain if it needs to
   // encrypt/decrypt a password.
   OSCrypt::UseMockKeychain(true);
+}
+
+PasswordStoreProxyMacTest::~PasswordStoreProxyMacTest() {
+}
+
+void PasswordStoreProxyMacTest::SetUp() {
   scoped_ptr<password_manager::LoginDatabase> login_db(
       new password_manager::LoginDatabase(test_login_db_file_path()));
   CreateAndInitPasswordStore(login_db.Pass());
   // Make sure deferred initialization is performed before some tests start
   // accessing the |login_db| directly.
   FinishAsyncProcessing();
 }
 
 void PasswordStoreProxyMacTest::TearDown() {
   ClosePasswordStore();
 }
 
 void PasswordStoreProxyMacTest::CreateAndInitPasswordStore(
     scoped_ptr<password_manager::LoginDatabase> login_db) {
   store_ = new PasswordStoreProxyMac(
       BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI),
-      make_scoped_ptr(new crypto::MockAppleKeychain), login_db.Pass());
+      make_scoped_ptr(new crypto::MockAppleKeychain), login_db.Pass(),
+      &testing_prefs_);
   ASSERT_TRUE(store_->Init(syncer::SyncableService::StartSyncFlare()));
 }
 
 void PasswordStoreProxyMacTest::ClosePasswordStore() {
   if (!store_)
     return;
   store_->Shutdown();
   EXPECT_FALSE(store_->GetBackgroundTaskRunner());
   base::MessageLoop::current()->RunUntilIdle();
   store_ = nullptr;
@@ skipping 76 matching lines @@
   list.push_back(password_manager::PasswordStoreChange(
       password_manager::PasswordStoreChange::REMOVE, old_form));
   EXPECT_CALL(mock_observer, OnLoginsChanged(list));
   if (check_created)
     store()->RemoveLoginsCreatedBetween(base::Time(), next_day);
   else
     store()->RemoveLoginsSyncedBetween(base::Time(), next_day);
   FinishAsyncProcessing();
 }
 
-TEST_F(PasswordStoreProxyMacTest, FormLifeCycle) {
+// ----------- Tests -------------
+
+TEST_P(PasswordStoreProxyMacTest, FormLifeCycle) {
   PasswordForm password_form;
   password_form.origin = GURL("http://example.com");
   password_form.username_value = base::ASCIIToUTF16("test1@gmail.com");
   password_form.password_value = base::ASCIIToUTF16("12345");
   password_form.signon_realm = "http://example.com/";
 
   AddForm(password_form);
   password_form.password_value = base::ASCIIToUTF16("password");
   UpdateForm(password_form);
   RemoveForm(password_form);
+
+  int status = testing_prefs_.GetInteger(
+      password_manager::prefs::kKeychainMigrationStatus);
+  if (GetParam() == PasswordStoreProxyMac::NOT_STARTED ||
+      GetParam() == PasswordStoreProxyMac::FAILED_ONCE) {
+    EXPECT_EQ(PasswordStoreProxyMac::MIGRATED, status);
+  } else {
+    EXPECT_EQ(GetParam(), status);
+  }
 }
 
-TEST_F(PasswordStoreProxyMacTest, TestRemoveLoginsCreatedBetween) {
+TEST_P(PasswordStoreProxyMacTest, TestRemoveLoginsCreatedBetween) {
   CheckRemoveLoginsBetween(true);
 }
 
-TEST_F(PasswordStoreProxyMacTest, TestRemoveLoginsSyncedBetween) {
+TEST_P(PasswordStoreProxyMacTest, TestRemoveLoginsSyncedBetween) {
   CheckRemoveLoginsBetween(false);
 }
 
-TEST_F(PasswordStoreProxyMacTest, FillLogins) {
+TEST_P(PasswordStoreProxyMacTest, FillLogins) {
   PasswordForm password_form;
   password_form.origin = GURL("http://example.com");
   password_form.signon_realm = "http://example.com/";
   password_form.username_value = base::ASCIIToUTF16("test1@gmail.com");
   password_form.password_value = base::ASCIIToUTF16("12345");
   AddForm(password_form);
 
   PasswordForm blacklisted_form;
   blacklisted_form.origin = GURL("http://example2.com");
   blacklisted_form.signon_realm = "http://example2.com/";
@@ skipping 14 matching lines @@
       .WillOnce(QuitUIMessageLoop());
   base::MessageLoop::current()->Run();
 
   store()->GetAutofillableLogins(&mock_consumer);
   EXPECT_CALL(mock_consumer, OnGetPasswordStoreResultsConstRef(
                                  ElementsAre(Pointee(password_form))))
       .WillOnce(QuitUIMessageLoop());
   base::MessageLoop::current()->Run();
 }
 
-TEST_F(PasswordStoreProxyMacTest, OperationsOnABadDatabaseSilentlyFail) {
+TEST_P(PasswordStoreProxyMacTest, OperationsOnABadDatabaseSilentlyFail) {
   // Verify that operations on a PasswordStore with a bad database cause no
   // explosions, but fail without side effect, return no data and trigger no
   // notifications.
   ClosePasswordStore();
   CreateAndInitPasswordStore(make_scoped_ptr(new BadLoginDatabase));
   FinishAsyncProcessing();
   EXPECT_FALSE(login_db());
 
   // The store should outlive the observer.
   scoped_refptr<PasswordStoreProxyMac> store_refptr = store();
@@ skipping 44 matching lines @@
 
   // Delete one login; a range of logins.
   store()->RemoveLogin(*form);
   store()->RemoveLoginsCreatedBetween(base::Time(), base::Time::Max());
   store()->RemoveLoginsSyncedBetween(base::Time(), base::Time::Max());
   FinishAsyncProcessing();
 
   // Verify no notifications are fired during shutdown either.
   ClosePasswordStore();
 }
+
+INSTANTIATE_TEST_CASE_P(,
+                        PasswordStoreProxyMacTest,
+                        testing::Values(PasswordStoreProxyMac::NOT_STARTED,
+                                        PasswordStoreProxyMac::MIGRATED,
+                                        PasswordStoreProxyMac::FAILED_ONCE,
+                                        PasswordStoreProxyMac::FAILED_TWICE));
+
+// Test the migration process.
+class PasswordStoreProxyMacMigrationTest : public PasswordStoreProxyMacTest {
+ public:
+  void SetUp() override;
+
+  void TestMigration(bool lock_keychain);
+
+ protected:
+  scoped_ptr<password_manager::LoginDatabase> login_db_;
+  scoped_ptr<crypto::MockAppleKeychain> keychain_;
+};
+
+void PasswordStoreProxyMacMigrationTest::SetUp() {
+  login_db_.reset(
+      new password_manager::LoginDatabase(test_login_db_file_path()));
+  keychain_.reset(new crypto::MockAppleKeychain);
+}
+
+void PasswordStoreProxyMacMigrationTest::TestMigration(bool lock_keychain) {
+  PasswordForm form;
+  form.origin = GURL("http://accounts.google.com/LoginAuth");
+  form.signon_realm = "http://accounts.google.com/";
+  form.username_value = base::ASCIIToUTF16("my_username");
+  form.password_value = base::ASCIIToUTF16("12345");
+
+  login_db_->set_clear_password_values(true);
+  EXPECT_TRUE(login_db_->Init());
+  EXPECT_EQ(AddChangeForForm(form), login_db_->AddLogin(form));
+  login_db_.reset(

[vabr (Chromium), 2015/07/03 08:40:48]

Is this just to undo lines 420 and 421? Perhaps a code comment would help to
understand the purpose of this line in any case.

[vasilii, 2015/07/03 12:59:11]

I needed another instance because LoginDatabase doesn't tolerate double init.
The second init happened inside PasswordStoreProxyMac. The content on the other
hand is the same.

[vabr (Chromium), 2015/07/06 09:50:19]

Acknowledged.

+      new password_manager::LoginDatabase(test_login_db_file_path()));
+  MacKeychainPasswordFormAdapter adapter(keychain_.get());
+  EXPECT_TRUE(adapter.AddPassword(form));
+
+  // Force migration.
+  if (lock_keychain)
+    keychain_->set_locked(true);
+  crypto::MockAppleKeychain* weak_keychain = keychain_.get();
+  store_ = new PasswordStoreProxyMac(
+      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI),
+      keychain_.Pass(), login_db_.Pass(), &testing_prefs_);
+  ASSERT_TRUE(store_->Init(syncer::SyncableService::StartSyncFlare()));
+  FinishAsyncProcessing();
+
+  // Check the password is still there.
+  if (lock_keychain)
+    weak_keychain->set_locked(false);
+  MockPasswordStoreConsumer mock_consumer;
+  store()->GetLogins(form, PasswordStoreProxyMac::ALLOW_PROMPT, &mock_consumer);
+  EXPECT_CALL(mock_consumer,
+              OnGetPasswordStoreResultsConstRef(ElementsAre(Pointee(form))))
+      .WillOnce(QuitUIMessageLoop());
+  base::MessageLoop::current()->Run();
+
+  int status = testing_prefs_.GetInteger(
+      password_manager::prefs::kKeychainMigrationStatus);
+  if (lock_keychain) {
+    EXPECT_EQ(GetParam() == PasswordStoreProxyMac::NOT_STARTED
+                  ? PasswordStoreProxyMac::FAILED_ONCE
+                  : PasswordStoreProxyMac::FAILED_TWICE,
+              status);
+  } else {
+    EXPECT_EQ(PasswordStoreProxyMac::MIGRATED, status);
+  }
+}
+
+TEST_P(PasswordStoreProxyMacMigrationTest, TestSuccessfullMigration) {
+  TestMigration(false);
+}
+
+TEST_P(PasswordStoreProxyMacMigrationTest, TestFailedMigration) {
+  TestMigration(true);
+}
+
+INSTANTIATE_TEST_CASE_P(,
+                        PasswordStoreProxyMacMigrationTest,
+                        testing::Values(PasswordStoreProxyMac::NOT_STARTED,

[vabr (Chromium), 2015/07/03 08:40:48]

Would it make sense to run this test also for the other migration statuses (with
appropriate changes to lines 451-453)?
While I understand that no migration should happen in those other cases, that
probably also needs to be tested (in the sense that whatever happens, the logins
are available from the proxy after it is created with a non-empty database and
keychain).

[vasilii, 2015/07/03 12:59:11]

Done.

+                                        PasswordStoreProxyMac::FAILED_ONCE));
+
 }  // namespace