[Passwords] Implement observing Compromised Credentials in CCProvider

This change adds the DatabaseCompromisedCredentialsObserver and
CompromisedCredentialsConsumer interfaces to
CompromisedCredentialsProvider, allowing it to monitor changes to the
compromised credentials table in the database.

Bug: 1047726
Change-Id: Ia3a9b5216fb25e563e96c6f8b1cf6e7f8a028d01
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2060670
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742117}

components/password_manager/core/browser/ui/compromised_credentials_provider.cc

@@ -3,19 +3,30 @@
 // found in the LICENSE file.
 
 #include "components/password_manager/core/browser/ui/compromised_credentials_provider.h"
+#include <iterator>
+
 #include "base/logging.h"
 
 namespace password_manager {
 
-CompromisedCredentialsProvider::CompromisedCredentialsProvider() = default;
+CompromisedCredentialsProvider::CompromisedCredentialsProvider(
+    scoped_refptr<PasswordStore> store)
+    : store_(std::move(store)) {
+  DCHECK(store_);
+  store_->AddDatabaseCompromisedCredentialsObserver(this);
+}
 
-CompromisedCredentialsProvider::~CompromisedCredentialsProvider() = default;
+CompromisedCredentialsProvider::~CompromisedCredentialsProvider() {
+  store_->RemoveDatabaseCompromisedCredentialsObserver(this);
+}
+
+void CompromisedCredentialsProvider::Init() {
+  store_->GetAllCompromisedCredentials(this);
+}
 
 CompromisedCredentialsProvider::CredentialsView
 CompromisedCredentialsProvider::GetCompromisedCredentials() const {
-  // TODO(crbug.com/1047726): Implement.
-  NOTIMPLEMENTED();
-  return {};
+  return compromised_credentials_;
 }
 
 void CompromisedCredentialsProvider::AddObserver(Observer* observer) {
@@ -26,10 +37,25 @@ void CompromisedCredentialsProvider::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
-void CompromisedCredentialsProvider::NotifyCompromisedCredentialsChanged(
-    CredentialsView credentials) {
+void CompromisedCredentialsProvider::OnCompromisedCredentialsChanged() {
+  // Cancel ongoing requests to the password store and issue a new request.
+  cancelable_task_tracker()->TryCancelAll();
+  store_->GetAllCompromisedCredentials(this);
+}
+
+void CompromisedCredentialsProvider::OnGetCompromisedCredentials(
+    std::vector<CompromisedCredentials> compromised_credentials) {
+  // TODO(crbug.com/1047726): Implement setting the password for each
+  // credential.
+  compromised_credentials_.assign(
+      std::make_move_iterator(compromised_credentials.begin()),
+      std::make_move_iterator(compromised_credentials.end()));
+  NotifyCompromisedCredentialsChanged();
+}
+
+void CompromisedCredentialsProvider::NotifyCompromisedCredentialsChanged() {
   for (auto& observer : observers_)
-    observer.OnCompromisedCredentialsChanged(credentials);
+    observer.OnCompromisedCredentialsChanged(compromised_credentials_);
 }
 
 }  // namespace password_manager

components/password_manager/core/browser/ui/compromised_credentials_provider.h

@@ -6,9 +6,13 @@
 #define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_UI_COMPROMISED_CREDENTIALS_PROVIDER_H_
 
 #include "base/containers/span.h"
+#include "base/memory/scoped_refptr.h"
 #include "base/observer_list.h"
 #include "base/observer_list_types.h"
 #include "base/strings/string16.h"
+#include "components/password_manager/core/browser/compromised_credentials_consumer.h"
+#include "components/password_manager/core/browser/compromised_credentials_table.h"
+#include "components/password_manager/core/browser/password_store.h"
 #include "url/gurl.h"
 
 namespace password_manager {
@@ -16,19 +20,27 @@ namespace password_manager {
 // This class provides a read-only view over saved compromised credentials. It
 // supports an observer interface, and clients can register themselves to get
 // notified about changes to the list.
-class CompromisedCredentialsProvider {
+class CompromisedCredentialsProvider
+    : public PasswordStore::DatabaseCompromisedCredentialsObserver,
+      public CompromisedCredentialsConsumer {
  public:
-  // Simple credential. This is similar to the CompromisedCredentials struct,
-  // but it does not store information about creation time or why the credential
-  // is compromised, however it does know about the password of the associated
-  // credential.
-  struct Credential {
-    base::string16 username;
+  // Simple struct that augments the CompromisedCredentials with a password.
+  struct CredentialWithPassword : CompromisedCredentials {
+    // Enable explicit construction and assignment from the parent struct. These
+    // will leave |password| empty.
+    explicit CredentialWithPassword(CompromisedCredentials credential)
+        : CompromisedCredentials(std::move(credential)) {}
+
+    CredentialWithPassword& operator=(CompromisedCredentials credential) {
+      CompromisedCredentials::operator=(std::move(credential));
+      password.clear();
+      return *this;
+    }
+
     base::string16 password;
-    GURL url;
   };
 
-  using CredentialsView = base::span<const Credential>;
+  using CredentialsView = base::span<const CredentialWithPassword>;
 
   // Observer interface. Clients can implement this to get notified about
   // changes to the list of compromised credentials. Clients can register and
@@ -40,8 +52,10 @@ class CompromisedCredentialsProvider {
         CredentialsView credentials) = 0;
   };
 
-  CompromisedCredentialsProvider();
-  ~CompromisedCredentialsProvider();
+  explicit CompromisedCredentialsProvider(scoped_refptr<PasswordStore> store);
+  ~CompromisedCredentialsProvider() override;
+
+  void Init();
 
   // Returns a read-only view over the currently compromised credentials.
   CredentialsView GetCompromisedCredentials() const;
@@ -51,8 +65,21 @@ class CompromisedCredentialsProvider {
   void RemoveObserver(Observer* observer);
 
  private:
-  // Notify observers about changes in the compromised credentials.
-  void NotifyCompromisedCredentialsChanged(CredentialsView credentials);
+  // PasswordStore::DatabaseCompromisedCredentialsObserver:
+  void OnCompromisedCredentialsChanged() override;
+
+  // CompromisedCredentialsConsumer:
+  void OnGetCompromisedCredentials(
+      std::vector<CompromisedCredentials> compromised_credentials) override;
+
+  // Notify observers about changes to |compromised_credentials_|.
+  void NotifyCompromisedCredentialsChanged();
+
+  // The password store containing the compromised credentials.
+  scoped_refptr<PasswordStore> store_;
+
+  // Cache of the most recently obtained compromised credentials.
+  std::vector<CredentialWithPassword> compromised_credentials_;
 
   base::ObserverList<Observer, /*check_empty=*/true> observers_;
 };

components/password_manager/core/browser/ui/compromised_credentials_provider_unittest.cc

@@ -4,6 +4,11 @@
 
 #include "components/password_manager/core/browser/ui/compromised_credentials_provider.h"
 
+#include "base/memory/scoped_refptr.h"
+#include "base/test/task_environment.h"
+#include "components/password_manager/core/browser/compromised_credentials_table.h"
+#include "components/password_manager/core/browser/test_password_store.h"
+#include "components/sync/model/syncable_service.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -11,27 +16,87 @@ namespace password_manager {
 
 namespace {
 
+using ::testing::ElementsAreArray;
+using ::testing::IsEmpty;
+
 struct MockCompromisedCredentialsProviderObserver
-    : ::testing::StrictMock<CompromisedCredentialsProvider::Observer> {
-  MOCK_METHOD1(OnCompromisedCredentialsChanged,
-               void(CompromisedCredentialsProvider::CredentialsView));
+    : CompromisedCredentialsProvider::Observer {
+  MOCK_METHOD(void,
+              OnCompromisedCredentialsChanged,
+              (CompromisedCredentialsProvider::CredentialsView),
+              (override));
 };
 
+using StrictMockCompromisedCredentialsProviderObserver =
+    ::testing::StrictMock<MockCompromisedCredentialsProviderObserver>;
+
 class CompromisedCredentialsProviderTest : public ::testing::Test {
  protected:
+  CompromisedCredentialsProviderTest() {
+    store_->Init(syncer::SyncableService::StartSyncFlare(), /*prefs=*/nullptr);
+  }
+
+  ~CompromisedCredentialsProviderTest() override {
+    store_->ShutdownOnUIThread();
+    task_env_.RunUntilIdle();
+  }
+
+  TestPasswordStore& store() { return *store_; }
   CompromisedCredentialsProvider& provider() { return provider_; }
 
+  void RunUntilIdle() { task_env_.RunUntilIdle(); }
+
  private:
-  CompromisedCredentialsProvider provider_;
+  base::test::SingleThreadTaskEnvironment task_env_;
+  scoped_refptr<TestPasswordStore> store_ =
+      base::MakeRefCounted<TestPasswordStore>();
+  CompromisedCredentialsProvider provider_{store_};
 };
 
 }  // namespace
 
 // Tests whether adding and removing an observer works as expected.
-TEST_F(CompromisedCredentialsProviderTest, Observers) {
-  MockCompromisedCredentialsProviderObserver observer;
+TEST_F(CompromisedCredentialsProviderTest, NotifyObservers) {
+  std::vector<CompromisedCredentials> credentials = {CompromisedCredentials()};
+
+  StrictMockCompromisedCredentialsProviderObserver observer;
   provider().AddObserver(&observer);
+
+  // Adding a credential should notify observers. Furthermore, the credential
+  // should be present of the list that is passed along.
+  EXPECT_CALL(observer,
+              OnCompromisedCredentialsChanged(ElementsAreArray(credentials)));
+  store().AddCompromisedCredentials(credentials[0]);
+  RunUntilIdle();
+  EXPECT_THAT(store().compromised_credentials(), ElementsAreArray(credentials));
+
+  // Adding the exact same credential should not result in a notification, as
+  // the database is not actually modified.
+  EXPECT_CALL(observer, OnCompromisedCredentialsChanged).Times(0);
+  store().AddCompromisedCredentials(credentials[0]);
+  RunUntilIdle();
+
+  // Remove should notify, and observers should be passed an empty list.
+  EXPECT_CALL(observer, OnCompromisedCredentialsChanged(IsEmpty()));
+  store().RemoveCompromisedCredentials(
+      credentials[0].signon_realm, credentials[0].username,
+      RemoveCompromisedCredentialsReason::kRemove);
+  RunUntilIdle();
+  EXPECT_THAT(store().compromised_credentials(), IsEmpty());
+
+  // Similarly to repeated add, a repeated remove should not notify either.
+  EXPECT_CALL(observer, OnCompromisedCredentialsChanged).Times(0);
+  store().RemoveCompromisedCredentials(
+      credentials[0].signon_realm, credentials[0].username,
+      RemoveCompromisedCredentialsReason::kRemove);
+  RunUntilIdle();
+
+  // After an observer is removed it should no longer receive notifications.
   provider().RemoveObserver(&observer);
+  EXPECT_CALL(observer, OnCompromisedCredentialsChanged).Times(0);
+  store().AddCompromisedCredentials(credentials[0]);
+  RunUntilIdle();
+  EXPECT_THAT(store().compromised_credentials(), ElementsAreArray(credentials));
 }
 
 }  // namespace password_manager