[Passwords] Implement {Start,Stop}BulkLeakCheck in BulkLeakCheckServiceAdapter

This change implements StartBulkLeakCheck() and StopBulkLeakCheck() in
the BulkLeakCheckServiceAdapter and makes it listen to the OnEdited()
event issued by the SavedPasswordsPresenter.

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

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

@@ -3,9 +3,50 @@
 // found in the LICENSE file.
 
 #include "components/password_manager/core/browser/ui/bulk_leak_check_service_adapter.h"
+
+#include <memory>
+#include <tuple>
+
 #include "base/logging.h"
+#include "components/autofill/core/common/password_form.h"
+#include "components/password_manager/core/browser/leak_detection/bulk_leak_check.h"
+#include "components/password_manager/core/browser/leak_detection/encryption_utils.h"
+#include "components/password_manager/core/browser/ui/saved_passwords_presenter.h"
 
 namespace password_manager {
+
+namespace {
+
+using autofill::PasswordForm;
+
+// Simple struct that stores a canonicalized credential.
+struct CanonicalizedCredential {
+  explicit CanonicalizedCredential(const PasswordForm& form)
+      : canonicalized_username(CanonicalizeUsername(form.username_value)),
+        password(form.password_value) {}
+
+  base::string16 canonicalized_username;
+  base::string16 password;
+};
+
+bool operator<(const CanonicalizedCredential& lhs,
+               const CanonicalizedCredential& rhs) {
+  return std::tie(lhs.canonicalized_username, lhs.password) <
+         std::tie(rhs.canonicalized_username, rhs.password);
+}
+
+}  // namespace
+
+const char kBulkLeakCheckDataKey[] = "bulk-leak-check-data";
+
+BulkLeakCheckData::BulkLeakCheckData(const PasswordForm& leaked_form)
+    : leaked_forms({leaked_form}) {}
+
+BulkLeakCheckData::BulkLeakCheckData(std::vector<PasswordForm> leaked_forms)
+    : leaked_forms(std::move(leaked_forms)) {}
+
+BulkLeakCheckData::~BulkLeakCheckData() = default;
+
 BulkLeakCheckServiceAdapter::BulkLeakCheckServiceAdapter(
     SavedPasswordsPresenter* presenter,
     BulkLeakCheckService* service)
@@ -19,14 +60,40 @@ BulkLeakCheckServiceAdapter::~BulkLeakCheckServiceAdapter() {
   presenter_->RemoveObserver(this);
 }
 
-void BulkLeakCheckServiceAdapter::StartBulkLeakCheck() {
-  // TODO(crbug.com/1047726): Implement.
-  NOTIMPLEMENTED();
+bool BulkLeakCheckServiceAdapter::StartBulkLeakCheck() {
+  if (service_->state() == BulkLeakCheckService::State::kRunning)
+    return false;
+
+  // Even though the BulkLeakCheckService performs canonicalization eventually
+  // we do it here to de-dupe credentials that have the same canonicalized form.
+  // Each canonicalized credential is mapped to a list of saved passwords that
+  // correspond to this credential.
+  std::map<CanonicalizedCredential, std::vector<PasswordForm>> canonicalized;
+  for (const PasswordForm& form : presenter_->GetSavedPasswords())
+    canonicalized[CanonicalizedCredential(form)].push_back(form);
+
+  // Build the list of LeakCheckCredentials and attach the corresponding saved
+  // passwords as UserData. Lastly,forward them to the service to start the
+  // check.
+  std::vector<LeakCheckCredential> credentials;
+  credentials.reserve(canonicalized.size());
+
+  for (auto& pair : canonicalized) {
+    const CanonicalizedCredential& credential = pair.first;
+    std::vector<PasswordForm>& forms = pair.second;
+    credentials.emplace_back(credential.canonicalized_username,
+                             credential.password);
+    credentials.back().SetUserData(
+        kBulkLeakCheckDataKey,
+        std::make_unique<BulkLeakCheckData>(std::move(forms)));
+  }
+
+  service_->CheckUsernamePasswordPairs(std::move(credentials));
+  return true;
 }
 
 void BulkLeakCheckServiceAdapter::StopBulkLeakCheck() {
-  // TODO(crbug.com/1047726): Implement.
-  NOTIMPLEMENTED();
+  service_->Cancel();
 }
 
 BulkLeakCheckService::State BulkLeakCheckServiceAdapter::GetBulkLeakCheckState()
@@ -38,9 +105,14 @@ size_t BulkLeakCheckServiceAdapter::GetPendingChecksCount() const {
   return service_->GetPendingChecksCount();
 }
 
-void BulkLeakCheckServiceAdapter::OnEdited(const autofill::PasswordForm& form) {
-  // TODO(crbug.com/1047726): Implement.
-  NOTIMPLEMENTED();
+void BulkLeakCheckServiceAdapter::OnEdited(const PasswordForm& form) {
+  // Here no extra canonicalization is needed, as there are no other forms we
+  // could de-dupe before we pass it on to the service.
+  std::vector<LeakCheckCredential> credentials;
+  credentials.emplace_back(form.username_value, form.password_value);
+  credentials.back().SetUserData(kBulkLeakCheckDataKey,
+                                 std::make_unique<BulkLeakCheckData>(form));
+  service_->CheckUsernamePasswordPairs(std::move(credentials));
 }
 
 }  // namespace password_manager

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

@@ -6,10 +6,30 @@
 #define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_UI_BULK_LEAK_CHECK_SERVICE_ADAPTER_H_
 
 #include "components/password_manager/core/browser/bulk_leak_check_service.h"
+#include "components/password_manager/core/browser/leak_detection/bulk_leak_check.h"
 #include "components/password_manager/core/browser/ui/saved_passwords_presenter.h"
 
+namespace autofill {
+struct PasswordForm;
+}
+
 namespace password_manager {
 
+// Key used to store an instance of BulkLeakCheckData in the user data map of a
+// LeakCheckCredential.
+extern const char kBulkLeakCheckDataKey[];
+
+// This struct bundles forms that correspond to the same LeakCheckCredential.
+// That is, all of the forms in |leaked_forms| correspond to the same pair of
+// canonicalized username and password.
+struct BulkLeakCheckData : LeakCheckCredential::Data {
+  explicit BulkLeakCheckData(const autofill::PasswordForm& leaked_form);
+  explicit BulkLeakCheckData(std::vector<autofill::PasswordForm> leaked_forms);
+  ~BulkLeakCheckData() override;
+
+  std::vector<autofill::PasswordForm> leaked_forms;
+};
+
 // This class serves as an apdater for the BulkLeakCheckService and exposes an
 // API that is intended to be consumed from the settings page.
 class BulkLeakCheckServiceAdapter : public SavedPasswordsPresenter::Observer {
@@ -18,10 +38,12 @@ class BulkLeakCheckServiceAdapter : public SavedPasswordsPresenter::Observer {
                               BulkLeakCheckService* service);
   ~BulkLeakCheckServiceAdapter() override;
 
-  // Instructs the adapter to start a check. This will obtain the list of saved
-  // passwords from |presenter_|, perform de-duplication of username and
-  // password pairs and then feed it to the |service_| for checking.
-  void StartBulkLeakCheck();
+  // Instructs the adapter to start a check. This is a no-op in case a check is
+  // already running. Otherwise, this will obtain the list of saved passwords
+  // from |presenter_|, perform de-duplication of username and password pairs
+  // and then feed it to the |service_| for checking.
+  // Returns whether new check was started.
+  bool StartBulkLeakCheck();
 
   // This asks |service_| to stop an ongoing check.
   void StopBulkLeakCheck();

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

@@ -4,10 +4,19 @@
 
 #include "components/password_manager/core/browser/ui/bulk_leak_check_service_adapter.h"
 
+#include <tuple>
+#include <vector>
+
+#include "base/containers/span.h"
+#include "base/memory/ptr_util.h"
 #include "base/strings/string_piece_forward.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/gmock_move_support.h"
 #include "base/test/task_environment.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/password_manager/core/browser/bulk_leak_check_service.h"
+#include "components/password_manager/core/browser/leak_detection/bulk_leak_check.h"
+#include "components/password_manager/core/browser/leak_detection/leak_detection_check_factory.h"
 #include "components/password_manager/core/browser/leak_detection/mock_leak_detection_check_factory.h"
 #include "components/password_manager/core/browser/test_password_store.h"
 #include "components/password_manager/core/browser/ui/saved_passwords_presenter.h"
@@ -20,13 +29,72 @@
 namespace password_manager {
 namespace {
 
+constexpr char kExampleCom[] = "https://example.com";
+constexpr char kExampleOrg[] = "https://example.org";
+
+constexpr char kUsername1[] = "alice";
+constexpr char kUsername2[] = "bob";
+
+constexpr char kPassword1[] = "f00b4r";
+constexpr char kPassword2[] = "s3cr3t";
+
+using autofill::PasswordForm;
+using ::testing::ByMove;
+using ::testing::NiceMock;
+using ::testing::Return;
+
+MATCHER_P(CredentialsAre, credentials, "") {
+  return std::equal(arg.begin(), arg.end(), credentials.get().begin(),
+                    credentials.get().end(),
+                    [](const auto& lhs, const auto& rhs) {
+                      return lhs.username() == rhs.username() &&
+                             lhs.password() == rhs.password();
+                    });
+}
+
+MATCHER_P(SavedPasswordsAre, passwords, "") {
+  return std::equal(arg.begin(), arg.end(), passwords.begin(), passwords.end(),
+                    [](const auto& lhs, const auto& rhs) {
+                      return lhs.signon_realm == rhs.signon_realm &&
+                             lhs.username_value == rhs.username_value &&
+                             lhs.password_value == rhs.password_value;
+                    });
+}
+
+PasswordForm MakeSavedPassword(base::StringPiece signon_realm,
+                               base::StringPiece username,
+                               base::StringPiece password) {
+  PasswordForm form;
+  form.signon_realm = std::string(signon_realm);
+  form.username_value = base::ASCIIToUTF16(username);
+  form.password_value = base::ASCIIToUTF16(password);
+  return form;
+}
+
+LeakCheckCredential MakeLeakCheckCredential(base::StringPiece username,
+                                            base::StringPiece password) {
+  return LeakCheckCredential(base::ASCIIToUTF16(username),
+                             base::ASCIIToUTF16(password));
+}
+
+struct MockBulkLeakCheck : BulkLeakCheck {
+  MOCK_METHOD(void,
+              CheckCredentials,
+              (std::vector<LeakCheckCredential> credentials),
+              (override));
+  MOCK_METHOD(size_t, GetPendingChecksCount, (), (const override));
+};
+
+using NiceMockBulkLeakCheck = ::testing::NiceMock<MockBulkLeakCheck>;
+
 class BulkLeakCheckServiceAdapterTest : public ::testing::Test {
  public:
   BulkLeakCheckServiceAdapterTest() {
-    service_.set_leak_factory(
-        std::make_unique<MockLeakDetectionCheckFactory>());
-
-    store_->Init(syncer::SyncableService::StartSyncFlare(), /*prefs=*/nullptr);
+    auto factory = std::make_unique<MockLeakDetectionCheckFactory>();
+    factory_ = factory.get();
+    service_.set_leak_factory(std::move(factory));
+    store_->Init(syncer::SyncableService::StartSyncFlare(),
+                 /*prefs=*/nullptr);
   }
 
   ~BulkLeakCheckServiceAdapterTest() override {
@@ -34,8 +102,13 @@ class BulkLeakCheckServiceAdapterTest : public ::testing::Test {
     task_env_.RunUntilIdle();
   }
 
+  TestPasswordStore& store() { return *store_; }
+  SavedPasswordsPresenter& presenter() { return presenter_; }
+  MockLeakDetectionCheckFactory& factory() { return *factory_; }
   BulkLeakCheckServiceAdapter& adapter() { return adapter_; }
 
+  void RunUntilIdle() { task_env_.RunUntilIdle(); }
+
  private:
   base::test::TaskEnvironment task_env_;
   signin::IdentityTestEnvironment identity_test_env_;
@@ -45,6 +118,7 @@ class BulkLeakCheckServiceAdapterTest : public ::testing::Test {
   BulkLeakCheckService service_{
       identity_test_env_.identity_manager(),
       base::MakeRefCounted<network::TestSharedURLLoaderFactory>()};
+  MockLeakDetectionCheckFactory* factory_ = nullptr;
   BulkLeakCheckServiceAdapter adapter_{&presenter_, &service_};
 };
 
@@ -56,4 +130,118 @@ TEST_F(BulkLeakCheckServiceAdapterTest, OnCreation) {
             adapter().GetBulkLeakCheckState());
 }
 
+// Checks that starting a leak check correctly transforms the list of saved
+// passwords into LeakCheckCredentials and attaches the underlying password
+// forms as user data.
+TEST_F(BulkLeakCheckServiceAdapterTest, StartBulkLeakCheck) {
+  std::vector<PasswordForm> passwords = {
+      MakeSavedPassword(kExampleCom, kUsername1, kPassword1),
+      MakeSavedPassword(kExampleOrg, kUsername2, kPassword2)};
+  store().AddLogin(passwords[0]);
+  store().AddLogin(passwords[1]);
+  RunUntilIdle();
+
+  auto leak_check = std::make_unique<NiceMockBulkLeakCheck>();
+  std::vector<LeakCheckCredential> credentials;
+  EXPECT_CALL(*leak_check, CheckCredentials).WillOnce(MoveArg(&credentials));
+  EXPECT_CALL(factory(), TryCreateBulkLeakCheck)
+      .WillOnce(Return(ByMove(std::move(leak_check))));
+  adapter().StartBulkLeakCheck();
+
+  std::vector<LeakCheckCredential> expected;
+  expected.push_back(MakeLeakCheckCredential(kUsername1, kPassword1));
+  expected.push_back(MakeLeakCheckCredential(kUsername2, kPassword2));
+
+  EXPECT_THAT(credentials, CredentialsAre(std::cref(expected)));
+
+  EXPECT_THAT(static_cast<BulkLeakCheckData*>(
+                  credentials[0].GetUserData(kBulkLeakCheckDataKey))
+                  ->leaked_forms,
+              SavedPasswordsAre(base::make_span(&passwords[0], 1)));
+
+  EXPECT_THAT(static_cast<BulkLeakCheckData*>(
+                  credentials[1].GetUserData(kBulkLeakCheckDataKey))
+                  ->leaked_forms,
+              SavedPasswordsAre(base::make_span(&passwords[1], 1)));
+}
+
+// Tests that multiple credentials with effectively the same username are
+// correctly deduped before starting the leak check.
+TEST_F(BulkLeakCheckServiceAdapterTest, StartBulkLeakCheckDedupes) {
+  std::vector<PasswordForm> passwords = {
+      MakeSavedPassword(kExampleCom, "alice", kPassword1),
+      MakeSavedPassword(kExampleCom, "ALICE", kPassword1),
+      MakeSavedPassword(kExampleCom, "Alice@example.com", kPassword1)};
+
+  store().AddLogin(passwords[0]);
+  store().AddLogin(passwords[1]);
+  store().AddLogin(passwords[2]);
+  RunUntilIdle();
+
+  auto leak_check = std::make_unique<NiceMockBulkLeakCheck>();
+  std::vector<LeakCheckCredential> credentials;
+  EXPECT_CALL(*leak_check, CheckCredentials).WillOnce(MoveArg(&credentials));
+  EXPECT_CALL(factory(), TryCreateBulkLeakCheck)
+      .WillOnce(Return(ByMove(std::move(leak_check))));
+  adapter().StartBulkLeakCheck();
+
+  std::vector<LeakCheckCredential> expected;
+  expected.push_back(MakeLeakCheckCredential("alice", kPassword1));
+  EXPECT_THAT(credentials, CredentialsAre(std::cref(expected)));
+
+  EXPECT_THAT(static_cast<BulkLeakCheckData*>(
+                  credentials[0].GetUserData(kBulkLeakCheckDataKey))
+                  ->leaked_forms,
+              SavedPasswordsAre(passwords));
+}
+
+// Checks that trying to start a leak check when another check is already
+// running does nothing and returns false to the caller.
+TEST_F(BulkLeakCheckServiceAdapterTest, MultipleStarts) {
+  auto leak_check = std::make_unique<NiceMockBulkLeakCheck>();
+  auto& leak_check_ref = *leak_check;
+  EXPECT_CALL(leak_check_ref, CheckCredentials);
+  EXPECT_CALL(factory(), TryCreateBulkLeakCheck)
+      .WillOnce(Return(ByMove(std::move(leak_check))));
+  EXPECT_TRUE(adapter().StartBulkLeakCheck());
+
+  EXPECT_CALL(leak_check_ref, CheckCredentials).Times(0);
+  EXPECT_FALSE(adapter().StartBulkLeakCheck());
+}
+
+// Checks that stopping the leak check correctly resets the state of the bulk
+// leak check.
+TEST_F(BulkLeakCheckServiceAdapterTest, StopBulkLeakCheck) {
+  auto leak_check = std::make_unique<NiceMockBulkLeakCheck>();
+  EXPECT_CALL(*leak_check, CheckCredentials);
+  EXPECT_CALL(factory(), TryCreateBulkLeakCheck)
+      .WillOnce(Return(ByMove(std::move(leak_check))));
+  adapter().StartBulkLeakCheck();
+  EXPECT_EQ(BulkLeakCheckService::State::kRunning,
+            adapter().GetBulkLeakCheckState());
+
+  adapter().StopBulkLeakCheck();
+  EXPECT_EQ(BulkLeakCheckService::State::kIdle,
+            adapter().GetBulkLeakCheckState());
+}
+
+// Tests that editing a password through the presenter will result in another
+// call to CheckCredentials with a corresponding change to the checked password.
+TEST_F(BulkLeakCheckServiceAdapterTest, OnEdited) {
+  PasswordForm password =
+      MakeSavedPassword(kExampleCom, kUsername1, kPassword1);
+  store().AddLogin(password);
+  RunUntilIdle();
+
+  std::vector<LeakCheckCredential> expected;
+  expected.push_back(MakeLeakCheckCredential(kUsername1, kPassword2));
+
+  auto leak_check = std::make_unique<NiceMockBulkLeakCheck>();
+  EXPECT_CALL(*leak_check,
+              CheckCredentials(CredentialsAre(std::cref(expected))));
+  EXPECT_CALL(factory(), TryCreateBulkLeakCheck)
+      .WillOnce(Return(ByMove(std::move(leak_check))));
+  presenter().EditPassword(password, base::ASCIIToUTF16(kPassword2));
+}
+
 }  // namespace password_manager