[iOS][Credential-Provider] Observe changes in the password store

Updates the credential store with changes from the password store.
Renames SyncAllCredentials to RequestSyncAllCredentials to make clearer
the async nature of it.
Creates entry functions for adding, updating, and removing passwords
which will be used to also keep up to date the OS metadata in a follow
up CL.
Injects the credential store for easier testing.

Bug: 1066803
Change-Id: Id79c3b276517c7bb223b3988db4d1a8c25e00025
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2152814
Commit-Queue: Javier Ernesto Flores Robles <javierrobles@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760483}

ios/chrome/browser/credential_provider/BUILD.gn

@@ -56,6 +56,7 @@ source_set("unit_tests") {
     "//components/password_manager/core/browser:test_support",
     "//ios/chrome/common/app_group",
     "//ios/chrome/common/credential_provider",
+    "//ios/chrome/common/credential_provider:ui",
     "//testing/gtest",
     "//url",
   ]

ios/chrome/browser/credential_provider/archivable_credential+password_form.h

@@ -15,7 +15,7 @@ struct PasswordForm;
 @interface ArchivableCredential (PasswordForm)
 
 // Convenience initializer from a PasswordForm. Will return nil for forms
-// with an empty origin or Android forms.
+// blacklisted by the user, with an empty origin or Android forms.
 - (instancetype)initWithPasswordForm:(const autofill::PasswordForm&)passwordForm
                              favicon:(NSString*)favicon
                 validationIdentifier:(NSString*)validationIdentifier;

ios/chrome/browser/credential_provider/archivable_credential+password_form.mm

@@ -38,7 +38,7 @@ NSString* recordIdentifierForPasswordForm(const autofill::PasswordForm& form) {
 - (instancetype)initWithPasswordForm:(const autofill::PasswordForm&)passwordForm
                              favicon:(NSString*)favicon
                 validationIdentifier:(NSString*)validationIdentifier {
-  if (passwordForm.origin.is_empty() ||
+  if (passwordForm.origin.is_empty() || passwordForm.blacklisted_by_user ||
       password_manager::IsValidAndroidFacetURI(passwordForm.signon_realm)) {
     return nil;
   }

ios/chrome/browser/credential_provider/credential_provider_service.h

@@ -16,34 +16,43 @@
 // Extension data up to date.
 class CredentialProviderService
     : public KeyedService,
-      public password_manager::PasswordStoreConsumer {
+      public password_manager::PasswordStoreConsumer,
+      public password_manager::PasswordStore::Observer {
  public:
   // Initializes the service.
   CredentialProviderService(
       scoped_refptr<password_manager::PasswordStore> password_store,
-      NSURL* file_url);
+      ArchivableCredentialStore* credential_store);
   ~CredentialProviderService() override;
 
   // KeyedService:
   void Shutdown() override;
 
  private:
-  friend class CredentialProviderServiceTest;
-
-  // Request all the credentials to sync them. B efore adding the fresh ones,
+  // Request all the credentials to sync them. Before adding the fresh ones,
   // the old ones are deleted.
-  void SyncAllCredentials();
+  void RequestSyncAllCredentials();
 
-  // Adds a credential with the passed args to the store.
+  // Adds a credential with |form| to the store.
   void SaveCredential(const autofill::PasswordForm& form) const;
 
-  // Syncs the store to disk.
-  void SyncStore() const;
+  // Updates a credential with |form| in the store.
+  void UpdateCredential(const autofill::PasswordForm& form) const;
+
+  // Removes a credential with |form| from the store.
+  void RemoveCredential(const autofill::PasswordForm& form) const;
+
+  // Syncs the credential store to disk.
+  void SyncStore(void (^completion)(NSError*)) const;
 
   // PasswordStoreConsumer:
   void OnGetPasswordStoreResults(
       std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
 
+  // PasswordStore::Observer:
+  void OnLoginsChanged(
+      const password_manager::PasswordStoreChangeList& changes) override;
+
   // The interface for getting and manipulating a user's saved passwords.
   scoped_refptr<password_manager::PasswordStore> password_store_;
 

ios/chrome/browser/credential_provider/credential_provider_service.mm

@@ -8,6 +8,7 @@
 #include "base/scoped_observer.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "build/build_config.h"
+#include "components/password_manager/core/browser/password_store_change.h"
 #include "ios/chrome/browser/credential_provider/archivable_credential+password_form.h"
 #include "ios/chrome/common/app_group/app_group_constants.h"
 #import "ios/chrome/common/credential_provider/archivable_credential.h"
@@ -18,10 +19,16 @@
 #include "components/password_manager/core/browser/android_affiliation/affiliation_utils.h"
 #endif  // !defined(NDEBUG)
 
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
 namespace {
 
 using autofill::PasswordForm;
 using password_manager::PasswordStore;
+using password_manager::PasswordStoreChange;
+using password_manager::PasswordStoreChangeList;
 
 BOOL ShouldSyncAllCredentials() {
   NSUserDefaults* shared_defaults = app_group::GetGroupUserDefaults();
@@ -30,19 +37,34 @@ BOOL ShouldSyncAllCredentials() {
       boolForKey:kUserDefaultsCredentialProviderFirstTimeSyncCompleted];
 }
 
+ArchivableCredential* CredentialFromForm(const PasswordForm& form) {
+  DCHECK(!form.blacklisted_by_user);
+  ArchivableCredential* credential =
+      [[ArchivableCredential alloc] initWithPasswordForm:form
+                                                 favicon:nil
+                                    validationIdentifier:nil];
+  if (!credential) {
+    // Verify that the credential is nil because it's an Android one or
+    // blacklisted.
+    DCHECK(password_manager::IsValidAndroidFacetURI(form.signon_realm) ||
+           form.blacklisted_by_user);
+  }
+  return credential;
+}
+
 }  // namespace
 
 CredentialProviderService::CredentialProviderService(
     scoped_refptr<PasswordStore> password_store,
-    NSURL* file_url)
-    : password_store_(password_store) {
+    ArchivableCredentialStore* credential_store)
+    : password_store_(password_store),
+      archivable_credential_store_(credential_store) {
   DCHECK(password_store_);
-  archivable_credential_store_ =
-      [[ArchivableCredentialStore alloc] initWithFileURL:file_url];
+  password_store_->AddObserver(this);
   // TODO(crbug.com/1066803): Wait for things to settle down before sync, and
   // sync credentials after Sync finishes or some seconds in the future.
   if (ShouldSyncAllCredentials()) {
-    SyncAllCredentials();
+    RequestSyncAllCredentials();
   }
 }
 
@@ -50,7 +72,7 @@ CredentialProviderService::~CredentialProviderService() {}
 
 void CredentialProviderService::Shutdown() {}
 
-void CredentialProviderService::SyncAllCredentials() {
+void CredentialProviderService::RequestSyncAllCredentials() {
   password_store_->GetAutofillableLogins(this);
 }
 
@@ -60,32 +82,65 @@ void CredentialProviderService::OnGetPasswordStoreResults(
   for (const auto& form : results) {
     SaveCredential(*form);
   }
-  SyncStore();
+  SyncStore(^(NSError* error) {
+    if (!error) {
+      [app_group::GetGroupUserDefaults()
+          setBool:YES
+           forKey:kUserDefaultsCredentialProviderFirstTimeSyncCompleted];
+    }
+  });
 }
 
 void CredentialProviderService::SaveCredential(const PasswordForm& form) const {
-  ArchivableCredential* credential =
-      [[ArchivableCredential alloc] initWithPasswordForm:form
-                                                 favicon:nil
-                                    validationIdentifier:nil];
+  ArchivableCredential* credential = CredentialFromForm(form);
   if (credential) {
     [archivable_credential_store_ addCredential:credential];
   }
-#if !defined(NDEBUG)
-  // Double check that the credential is nil because it is an Android one.
-  else {
-    DCHECK(password_manager::IsValidAndroidFacetURI(form.signon_realm));
+}
+
+void CredentialProviderService::UpdateCredential(
+    const PasswordForm& form) const {
+  ArchivableCredential* credential = CredentialFromForm(form);
+  if (credential) {
+    [archivable_credential_store_ updateCredential:credential];
   }
-#endif  // !defined(NDEBUG)
 }
 
-void CredentialProviderService::SyncStore() const {
+void CredentialProviderService::RemoveCredential(
+    const PasswordForm& form) const {
+  ArchivableCredential* credential = CredentialFromForm(form);
+  if (credential) {
+    [archivable_credential_store_ removeCredential:credential];
+  }
+}
+
+void CredentialProviderService::SyncStore(void (^completion)(NSError*)) const {
   [archivable_credential_store_ saveDataWithCompletion:^(NSError* error) {
     DCHECK(!error) << "An error occurred while saving to disk";
-    if (!error) {
-      [app_group::GetGroupUserDefaults()
-          setBool:YES
-           forKey:kUserDefaultsCredentialProviderFirstTimeSyncCompleted];
+    if (completion) {
+      completion(error);
     }
   }];
 }
+
+void CredentialProviderService::OnLoginsChanged(
+    const PasswordStoreChangeList& changes) {
+  for (const PasswordStoreChange& change : changes) {
+    switch (change.type()) {
+      case PasswordStoreChange::ADD:
+        SaveCredential(change.form());
+        break;
+      case PasswordStoreChange::UPDATE:
+        UpdateCredential(change.form());
+        break;
+      case PasswordStoreChange::REMOVE:
+        RemoveCredential(change.form());
+        break;
+
+      default:
+        NOTREACHED();
+        break;
+    }
+  }
+  SyncStore(nil);
+}

ios/chrome/browser/credential_provider/credential_provider_service_factory.mm

@@ -9,6 +9,7 @@
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/credential_provider/credential_provider_service.h"
 #include "ios/chrome/browser/passwords/ios_chrome_password_store_factory.h"
+#import "ios/chrome/common/credential_provider/archivable_credential_store.h"
 #import "ios/chrome/common/credential_provider/constants.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -46,7 +47,9 @@ CredentialProviderServiceFactory::BuildServiceInstanceFor(
   scoped_refptr<password_manager::PasswordStore> password_store =
       IOSChromePasswordStoreFactory::GetForBrowserState(
           browser_state, ServiceAccessType::IMPLICIT_ACCESS);
-
-  return std::make_unique<CredentialProviderService>(
-      password_store, CredentialProviderSharedArchivableStoreURL());
+  ArchivableCredentialStore* credential_store =
+      [[ArchivableCredentialStore alloc]
+          initWithFileURL:CredentialProviderSharedArchivableStoreURL()];
+  return std::make_unique<CredentialProviderService>(password_store,
+                                                     credential_store);
 }

ios/chrome/browser/credential_provider/credential_provider_service_unittest.mm

@@ -5,11 +5,16 @@
 #include "ios/chrome/browser/credential_provider/credential_provider_service.h"
 
 #include "base/files/scoped_temp_dir.h"
+#include "base/strings/utf_string_conversions.h"
 #import "base/test/ios/wait_util.h"
 #include "base/test/task_environment.h"
+#include "components/autofill/core/common/password_form.h"
 #include "components/password_manager/core/browser/password_store_default.h"
 #include "ios/chrome/common/app_group/app_group_constants.h"
+#import "ios/chrome/common/credential_provider/archivable_credential_store.h"
 #import "ios/chrome/common/credential_provider/constants.h"
+#import "ios/chrome/common/credential_provider/credential.h"
+#import "testing/gtest_mac.h"
 #include "testing/platform_test.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -18,6 +23,7 @@
 
 namespace {
 
+using autofill::PasswordForm;
 using base::test::ios::WaitUntilConditionOrTimeout;
 using base::test::ios::kWaitForFileOperationTimeout;
 using password_manager::PasswordStoreDefault;
@@ -36,8 +42,9 @@ class CredentialProviderServiceTest : public PlatformTest {
     NSUserDefaults* shared_defaults = app_group::GetGroupUserDefaults();
     EXPECT_FALSE([shared_defaults
         boolForKey:kUserDefaultsCredentialProviderFirstTimeSyncCompleted]);
-    credential_provider_service_ =
-        std::make_unique<CredentialProviderService>(password_store_, nil);
+    credential_store_ = [[ArchivableCredentialStore alloc] initWithFileURL:nil];
+    credential_provider_service_ = std::make_unique<CredentialProviderService>(
+        password_store_, credential_store_);
   }
 
   void TearDown() override {
@@ -65,6 +72,7 @@ class CredentialProviderServiceTest : public PlatformTest {
   base::test::TaskEnvironment task_environment_;
   std::unique_ptr<CredentialProviderService> credential_provider_service_;
   scoped_refptr<PasswordStoreDefault> password_store_;
+  ArchivableCredentialStore* credential_store_;
 
   DISALLOW_COPY_AND_ASSIGN(CredentialProviderServiceTest);
 };
@@ -84,4 +92,41 @@ TEST_F(CredentialProviderServiceTest, FirstSync) {
   }));
 }
 
+// Test that CredentialProviderService observes changes in the password store.
+TEST_F(CredentialProviderServiceTest, PasswordChanges) {
+  EXPECT_EQ(0u, credential_store_.credentials.count);
+
+  PasswordForm form;
+  form.origin = GURL("http://0.com");
+  form.signon_realm = "http://www.example.com/";
+  form.action = GURL("http://www.example.com/action");
+  form.password_element = base::ASCIIToUTF16("pwd");
+  form.password_value = base::ASCIIToUTF16("example");
+
+  password_store_->AddLogin(form);
+  task_environment_.RunUntilIdle();
+
+  // Expect the store to be populated with 1 credential.
+  ASSERT_EQ(1u, credential_store_.credentials.count);
+
+  NSString* keychainIdentifier =
+      credential_store_.credentials.firstObject.keychainIdentifier;
+  form.password_value = base::ASCIIToUTF16("secret");
+
+  password_store_->UpdateLogin(form);
+  task_environment_.RunUntilIdle();
+
+  // Expect that the credential in the store now has a different keychain
+  // identifier.
+  id<Credential> credential = credential_store_.credentials.firstObject;
+  EXPECT_NSNE(keychainIdentifier, credential.keychainIdentifier);
+  ASSERT_EQ(1u, credential_store_.credentials.count);
+
+  password_store_->RemoveLogin(form);
+  task_environment_.RunUntilIdle();
+
+  // Expect the store to be empty.
+  ASSERT_EQ(0u, credential_store_.credentials.count);
+}
+
 }  // namespace