[Password Manager] Add pref to store date when undecryptable logins were deleted on Mac

This CL includes:
- Add a util class which sets the preference on the main thread.
- Use utility class in LoginDatabase to set the date on successul deletion of undecryptable passwords.
- Expand DB tests to make sure that the pref is stored.

Bug: 791541
Change-Id: I32c2afc70c6091175cae09a7120de7622b568448
Reviewed-on: https://chromium-review.googlesource.com/1178052
Commit-Queue: Tonko Sabolčec <tsabolcec@google.com>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584060}

chrome/browser/password_manager/password_store_factory.cc

@@ -12,6 +12,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/rand_util.h"
 #include "build/build_config.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
@@ -165,6 +166,13 @@ PasswordStoreFactory::BuildServiceInstanceFor(
 
   std::unique_ptr<password_manager::LoginDatabase> login_db(
       password_manager::CreateLoginDatabase(profile->GetPath()));
+#if defined(OS_MACOSX)
+  PrefService* local_state = g_browser_process->local_state();
+  DCHECK(local_state);
+  login_db->InitPasswordRecoveryUtil(
+      std::make_unique<password_manager::PasswordRecoveryUtilMac>(
+          local_state, base::ThreadTaskRunnerHandle::Get()));
+#endif
 
   scoped_refptr<PasswordStore> ps;
 #if defined(OS_WIN)

chrome/browser/prefs/browser_prefs.cc

@@ -390,6 +390,7 @@ void RegisterLocalState(PrefRegistrySimple* registry) {
   IOThread::RegisterPrefs(registry);
   network_time::NetworkTimeTracker::RegisterPrefs(registry);
   OriginTrialPrefs::RegisterPrefs(registry);
+  password_manager::PasswordManager::RegisterLocalPrefs(registry);
   PrefProxyConfigTrackerImpl::RegisterPrefs(registry);
   ProfileInfoCache::RegisterPrefs(registry);
   profiles::RegisterPrefs(registry);
@@ -498,7 +499,6 @@ void RegisterLocalState(PrefRegistrySimple* registry) {
   app_metro_launch::RegisterPrefs(registry);
   component_updater::RegisterPrefsForSwReporter(registry);
   desktop_ios_promotion::RegisterLocalPrefs(registry);
-  password_manager::PasswordManager::RegisterLocalPrefs(registry);
 #if defined(GOOGLE_CHROME_BUILD)
   IncompatibleApplicationsUpdater::RegisterLocalStatePrefs(registry);
   ModuleDatabase::RegisterLocalStatePrefs(registry);

components/password_manager/core/browser/BUILD.gn

@@ -229,6 +229,13 @@ jumbo_static_library("browser") {
     sources += [ "login_database_posix.cc" ]
   }
 
+  if (is_mac && !is_ios) {
+    sources += [
+      "password_recovery_util_mac.cc",
+      "password_recovery_util_mac.h",
+    ]
+  }
+
   # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
   configs += [ "//build/config/compiler:no_size_t_to_int_warning" ]
 }

components/password_manager/core/browser/login_database.cc

@@ -631,6 +631,13 @@ bool LoginDatabase::Init() {
   return true;
 }
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+void LoginDatabase::InitPasswordRecoveryUtil(
+    std::unique_ptr<PasswordRecoveryUtilMac> password_recovery_util) {
+  password_recovery_util_ = std::move(password_recovery_util);
+}
+#endif
+
 void LoginDatabase::ReportMetrics(const std::string& sync_username,
                                   bool custom_passphrase_sync_enabled) {
   sql::Statement s(db_.GetCachedStatement(
@@ -1350,6 +1357,8 @@ DatabaseCleanupResult LoginDatabase::DeleteUndecryptableLogins() {
         metrics_util::DeleteUndecryptableLoginsReturnValue::
             kSuccessNoDeletions);
   } else {
+    DCHECK(password_recovery_util_);
+    password_recovery_util_->RecordPasswordRecovery();
     metrics_util::LogDeleteUndecryptableLoginsReturnValue(
         metrics_util::DeleteUndecryptableLoginsReturnValue::
             kSuccessLoginsDeleted);

components/password_manager/core/browser/login_database.h

@@ -26,6 +26,10 @@
 #include "base/gtest_prod_util.h"
 #endif
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+#include "components/password_manager/core/browser/password_recovery_util_mac.h"
+#endif
+
 namespace password_manager {
 
 class SQLTableBuilder;
@@ -45,6 +49,12 @@ class LoginDatabase {
   // should be called.
   virtual bool Init();
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  // Registers utility which is used to save password recovery status on MacOS.
+  void InitPasswordRecoveryUtil(
+      std::unique_ptr<PasswordRecoveryUtilMac> password_recovery_util);
+#endif
+
   // Reports usage metrics to UMA.
   void ReportMetrics(const std::string& sync_username,
                      bool custom_passphrase_sync_enabled);
@@ -255,6 +265,10 @@ class LoginDatabase {
   std::string blacklisted_statement_;
   std::string encrypted_statement_;
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  std::unique_ptr<PasswordRecoveryUtilMac> password_recovery_util_;
+#endif
+
 #if defined(OS_POSIX) && !defined(OS_MACOSX)
   // Whether password values should be encrypted.
   // TODO(crbug.com/571003) Only linux doesn't use encryption. Remove this once

components/password_manager/core/browser/login_database_unittest.cc

@@ -25,6 +25,9 @@
 #include "components/os_crypt/os_crypt_mocker.h"
 #include "components/password_manager/core/browser/password_manager_test_utils.h"
 #include "components/password_manager/core/browser/psl_matching_helper.h"
+#include "components/password_manager/core/common/password_manager_pref_names.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/testing_pref_service.h"
 #include "sql/database.h"
 #include "sql/statement.h"
 #include "sql/test/test_helpers.h"
@@ -2054,9 +2057,17 @@ class LoginDatabaseUndecryptableLoginsTest : public testing::Test {
 
   base::FilePath database_path() const { return database_path_; }
 
+  TestingPrefServiceSimple& testing_local_state() {
+    return testing_local_state_;
+  }
+
+  void RunUntilIdle() { scoped_task_environment_.RunUntilIdle(); }
+
  private:
   base::FilePath database_path_;
   base::ScopedTempDir temp_dir_;
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+  TestingPrefServiceSimple testing_local_state_;
 
   DISALLOW_COPY_AND_ASSIGN(LoginDatabaseUndecryptableLoginsTest);
 };
@@ -2119,6 +2130,11 @@ TEST_F(LoginDatabaseUndecryptableLoginsTest, DeleteUndecryptableLoginsTest) {
   ASSERT_TRUE(db.Init());
 
 #if defined(OS_MACOSX) && !defined(OS_IOS)
+  testing_local_state().registry()->RegisterTimePref(
+      prefs::kSyncUsersPasswordRecovery, base::Time());
+  db.InitPasswordRecoveryUtil(std::make_unique<PasswordRecoveryUtilMac>(
+      &testing_local_state(), base::ThreadTaskRunnerHandle::Get()));
+
   // Make sure that we can't get any logins when database is corrupted.
   std::vector<std::unique_ptr<PasswordForm>> result;
   EXPECT_FALSE(db.GetAutofillableLogins(&result));
@@ -2128,6 +2144,12 @@ TEST_F(LoginDatabaseUndecryptableLoginsTest, DeleteUndecryptableLoginsTest) {
   EXPECT_EQ(DatabaseCleanupResult::kSuccess, db.DeleteUndecryptableLogins());
   EXPECT_TRUE(db.GetAutofillableLogins(&result));
   EXPECT_THAT(result, UnorderedElementsAre(Pointee(form1), Pointee(form3)));
+
+  RunUntilIdle();
+
+  // Make sure that password recovery pref is set.
+  ASSERT_TRUE(
+      testing_local_state().HasPrefPath(prefs::kSyncUsersPasswordRecovery));
 #else
   EXPECT_EQ(DatabaseCleanupResult::kSuccess, db.DeleteUndecryptableLogins());
 #endif

components/password_manager/core/browser/password_manager.cc

@@ -303,14 +303,18 @@ void PasswordManager::RegisterProfilePrefs(
                              PrefRegistry::NO_REGISTRATION_FLAGS);
 }
 
-#if defined(OS_WIN)
 // static
 void PasswordManager::RegisterLocalPrefs(PrefRegistrySimple* registry) {
+#if defined(OS_WIN)
   registry->RegisterInt64Pref(prefs::kOsPasswordLastChanged, 0);
   registry->RegisterBooleanPref(prefs::kOsPasswordBlank, false);
-}
 #endif
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  registry->RegisterTimePref(prefs::kSyncUsersPasswordRecovery, base::Time());
+#endif
+}
+
 PasswordManager::PasswordManager(PasswordManagerClient* client)
     : client_(client) {
   DCHECK(client_);

components/password_manager/core/browser/password_manager.h

@@ -53,9 +53,9 @@ class PasswordManager : public LoginModel, public FormSubmissionObserver {
   enum class NavigationEntryToCheck { LAST_COMMITTED, VISIBLE };
 
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
-#if defined(OS_WIN)
+
   static void RegisterLocalPrefs(PrefRegistrySimple* registry);
-#endif
+
   explicit PasswordManager(PasswordManagerClient* client);
   ~PasswordManager() override;
 

components/password_manager/core/browser/password_recovery_util_mac.cc

+// Copyright 2018 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 "components/password_manager/core/browser/password_recovery_util_mac.h"
+
+#include "base/bind.h"
+#include "base/single_thread_task_runner.h"
+#include "base/time/time.h"
+#include "components/password_manager/core/common/password_manager_pref_names.h"
+#include "components/prefs/pref_service.h"
+
+namespace password_manager {
+
+PasswordRecoveryUtilMac::PasswordRecoveryUtilMac(
+    PrefService* local_state,
+    scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner)
+    : local_state_(local_state),
+      main_thread_task_runner_(main_thread_task_runner) {}
+
+PasswordRecoveryUtilMac::~PasswordRecoveryUtilMac() {}
+
+void PasswordRecoveryUtilMac::RecordPasswordRecovery() {
+  main_thread_task_runner_->PostTask(
+      FROM_HERE, base::BindOnce(
+                     [](PrefService* local_state) {
+                       local_state->SetTime(prefs::kSyncUsersPasswordRecovery,
+                                            base::Time::Now());
+                     },
+                     local_state_));
+}
+
+}  // namespace password_manager

components/password_manager/core/browser/password_recovery_util_mac.h

+// Copyright (c) 2018 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.
+
+#ifndef COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_RECOVERY_UTIL_MAC_H_
+#define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_RECOVERY_UTIL_MAC_H_
+
+#include "base/memory/scoped_refptr.h"
+
+class PrefService;
+
+namespace base {
+class SingleThreadTaskRunner;
+}  // namespace base
+
+namespace password_manager {
+
+// A utility class which is used by login database to store the date when some
+// undecryptable logins were deleted.
+class PasswordRecoveryUtilMac {
+ public:
+  PasswordRecoveryUtilMac(
+      PrefService* local_state,
+      scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner);
+
+  ~PasswordRecoveryUtilMac();
+
+  // Posts tasks on main thread to store the current time.
+  void RecordPasswordRecovery();
+
+ private:
+  PrefService* local_state_;
+  scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
+};
+
+}  // namespace password_manager
+
+#endif  // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_RECOVERY_UTIL_MAC_H_

components/password_manager/core/common/password_manager_pref_names.cc

@@ -24,6 +24,8 @@ const char kOsPasswordLastChanged[] =
 
 #if defined(OS_MACOSX)
 const char kKeychainMigrationStatus[] = "password_manager.keychain_migration";
+const char kSyncUsersPasswordRecovery[] =
+    "password_manager.sync_users_password_recovery";
 #endif
 
 const char kWasAutoSignInFirstRunExperienceShown[] =

components/password_manager/core/common/password_manager_pref_names.h

@@ -45,6 +45,11 @@ extern const char kOsPasswordLastChanged[];
 // The current status of migrating the passwords from the Keychain to the
 // database. Stores a value from MigrationStatus.
 extern const char kKeychainMigrationStatus[];
+
+// The date of when passwords were recovered for MacOS Sync users who
+// previously lost access to their password because of encryption key
+// modification in Keychain.
+extern const char kSyncUsersPasswordRecovery[];
 #endif
 
 // Boolean that indicated whether first run experience for the auto sign-in