Syncing compromised credentials

This change syncs local Compromised credentials.

Bug: 1137775
Change-Id: Icb137b171d8caa3e8c2988b60a201b581140e067
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593618
Commit-Queue: Viktor Semeniuk <vsemeniuk@google.com>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843460}

components/password_manager/core/browser/sync/password_sync_bridge.cc

@@ -63,8 +63,43 @@ base::Time ConvertToBaseTime(uint64_t time) {
       base::TimeDelta::FromMicroseconds(time));
 }
 
+// Converts 'compromised_credentials' into PasswordIssues.
+// 'compromised_credentials' should contain only unique Compromise Types.
+sync_pb::PasswordSpecificsData::PasswordIssues
+PasswordIssuesFromCompromisedCredentials(
+    const std::vector<CompromisedCredentials>& compromised_credentials) {
+  sync_pb::PasswordSpecificsData::PasswordIssues issues;
+  for (const auto& compromised_credential : compromised_credentials) {
+    sync_pb::PasswordSpecificsData::PasswordIssues::PasswordIssue issue;
+    issue.set_date_first_detection_microseconds(
+        compromised_credential.create_time.ToDeltaSinceWindowsEpoch()
+            .InMicroseconds());
+    issue.set_is_muted(compromised_credential.is_muted.value());
+    switch (compromised_credential.compromise_type) {
+      case CompromiseType::kLeaked:
+        DCHECK(!issues.has_leaked_password_issue());
+        *issues.mutable_leaked_password_issue() = std::move(issue);
+        break;
+      case CompromiseType::kPhished:
+        DCHECK(!issues.has_phished_password_issue());
+        *issues.mutable_phished_password_issue() = std::move(issue);
+        break;
+      case CompromiseType::kWeak:
+        DCHECK(!issues.has_weak_password_issue());
+        *issues.mutable_weak_password_issue() = std::move(issue);
+        break;
+      case CompromiseType::kReused:
+        DCHECK(!issues.has_reused_password_issue());
+        *issues.mutable_reused_password_issue() = std::move(issue);
+        break;
+    }
+  }
+  return issues;
+}
+
 sync_pb::PasswordSpecifics SpecificsFromPassword(
-    const PasswordForm& password_form) {
+    const PasswordForm& password_form,
+    const std::vector<CompromisedCredentials>& compromised_credentials) {
   sync_pb::PasswordSpecifics specifics;
   sync_pb::PasswordSpecificsData* password_data =
       specifics.mutable_client_only_encrypted_data();
@@ -94,6 +129,8 @@ sync_pb::PasswordSpecifics SpecificsFromPassword(
       password_form.federation_origin.opaque()
           ? std::string()
           : password_form.federation_origin.Serialize());
+  *password_data->mutable_password_issues() =
+      PasswordIssuesFromCompromisedCredentials(compromised_credentials);
   return specifics;
 }
 
@@ -153,10 +190,10 @@ std::vector<CompromisedCredentials> CompromisedCredentialsFromEntityChange(
   const sync_pb::PasswordSpecificsData& password_data =
       entity_change.data().specifics.password().client_only_encrypted_data();
 
-  std::vector<CompromisedCredentials> issues;
+  std::vector<CompromisedCredentials> compromised_credentials;
 
   if (!password_data.has_password_issues())
-    return issues;
+    return compromised_credentials;
 
   const std::string& signon_realm = password_data.signon_realm();
   const base::string16& username =
@@ -164,31 +201,34 @@ std::vector<CompromisedCredentials> CompromisedCredentialsFromEntityChange(
 
   const auto& password_issues = password_data.password_issues();
   if (password_issues.has_leaked_password_issue()) {
-    issues.emplace_back(CreateCompromisedCredentials(
+    compromised_credentials.push_back(CreateCompromisedCredentials(
         signon_realm, username, CompromiseType::kLeaked,
         password_issues.leaked_password_issue()));
   }
   if (password_issues.has_reused_password_issue()) {
-    issues.emplace_back(CreateCompromisedCredentials(
+    compromised_credentials.push_back(CreateCompromisedCredentials(
         signon_realm, username, CompromiseType::kReused,
         password_issues.reused_password_issue()));
   }
   if (password_issues.has_weak_password_issue()) {
-    issues.emplace_back(CreateCompromisedCredentials(
+    compromised_credentials.push_back(CreateCompromisedCredentials(
         signon_realm, username, CompromiseType::kWeak,
         password_issues.weak_password_issue()));
   }
   if (password_issues.has_phished_password_issue()) {
-    issues.emplace_back(CreateCompromisedCredentials(
+    compromised_credentials.push_back(CreateCompromisedCredentials(
         signon_realm, username, CompromiseType::kPhished,
         password_issues.phished_password_issue()));
   }
-  return issues;
+  return compromised_credentials;
 }
 
-std::unique_ptr<syncer::EntityData> CreateEntityData(const PasswordForm& form) {
+std::unique_ptr<syncer::EntityData> CreateEntityData(
+    const PasswordForm& form,
+    const std::vector<CompromisedCredentials>& compromised_credentials) {
   auto entity_data = std::make_unique<syncer::EntityData>();
-  *entity_data->specifics.mutable_password() = SpecificsFromPassword(form);
+  *entity_data->specifics.mutable_password() =
+      SpecificsFromPassword(form, compromised_credentials);
   entity_data->name = form.signon_realm;
   return entity_data;
 }
@@ -343,7 +383,12 @@ void PasswordSyncBridge::ActOnPasswordStoreChanges(
     switch (change.type()) {
       case PasswordStoreChange::ADD:
       case PasswordStoreChange::UPDATE: {
-        change_processor()->Put(storage_key, CreateEntityData(change.form()),
+        const std::vector<CompromisedCredentials> compromised_credentials =
+            password_store_sync_->ReadSecurityIssues(
+                FormPrimaryKey(change.primary_key()));
+        change_processor()->Put(
+            storage_key,
+            CreateEntityData(change.form(), compromised_credentials),
             &metadata_change_list);
         break;
       }
@@ -437,8 +482,11 @@ base::Optional<syncer::ModelError> PasswordSyncBridge::MergeSyncData(
     for (const auto& pair : key_to_local_form_map) {
       const int primary_key = pair.first;
       const PasswordForm& local_password_form = *pair.second;
+      const std::vector<CompromisedCredentials> compromised_credentials =
+          password_store_sync_->ReadSecurityIssues(FormPrimaryKey(primary_key));
+
       std::unique_ptr<syncer::EntityData> local_form_entity_data =
-          CreateEntityData(local_password_form);
+          CreateEntityData(local_password_form, compromised_credentials);
       const std::string client_tag_of_local_password =
           GetClientTag(*local_form_entity_data);
       client_tags_of_local_passwords.insert(client_tag_of_local_password);
@@ -790,7 +838,10 @@ void PasswordSyncBridge::GetData(StorageKeyList storage_keys,
   for (const std::string& storage_key : storage_keys) {
     int primary_key = ParsePrimaryKey(storage_key);
     if (key_to_form_map.count(primary_key) != 0) {
-      batch->Put(storage_key, CreateEntityData(*key_to_form_map[primary_key]));
+      const std::vector<CompromisedCredentials> compromised_credentials =
+          password_store_sync_->ReadSecurityIssues(FormPrimaryKey(primary_key));
+      batch->Put(storage_key, CreateEntityData(*key_to_form_map[primary_key],
+                                               compromised_credentials));
     }
   }
   std::move(callback).Run(std::move(batch));
@@ -810,8 +861,11 @@ void PasswordSyncBridge::GetAllDataForDebugging(DataCallback callback) {
   auto batch = std::make_unique<syncer::MutableDataBatch>();
   for (const auto& pair : key_to_form_map) {
     PasswordForm form = *pair.second;
+    const std::vector<CompromisedCredentials> compromised_credentials =
+        password_store_sync_->ReadSecurityIssues(FormPrimaryKey(pair.first));
     form.password_value = base::UTF8ToUTF16("<redacted>");
-    batch->Put(base::NumberToString(pair.first), CreateEntityData(form));
+    batch->Put(base::NumberToString(pair.first),
+               CreateEntityData(form, compromised_credentials));
   }
   std::move(callback).Run(std::move(batch));
 }

components/password_manager/core/browser/sync/password_sync_bridge_unittest.cc

@@ -52,6 +52,30 @@ MATCHER_P(EntityDataHasSignonRealm, expected_signon_realm, "") {
              .signon_realm() == expected_signon_realm;
 }
 
+bool SpecificsHasExpectedCompromiseTypes(
+    const sync_pb::PasswordSpecificsData::PasswordIssues& specifics,
+    const std::vector<CompromiseType>& expected_types) {
+  return base::ranges::all_of(expected_types, [&specifics](auto type) {
+    switch (type) {
+      case CompromiseType::kLeaked:
+        return specifics.has_leaked_password_issue();
+      case CompromiseType::kPhished:
+        return specifics.has_phished_password_issue();
+      case CompromiseType::kWeak:
+        return specifics.has_weak_password_issue();
+      case CompromiseType::kReused:
+        return specifics.has_reused_password_issue();
+    }
+  });
+}
+
+MATCHER_P(EntityDataHasSecurityIssueTypes, expected_issue_types, "") {
+  const auto& password_issues_data =
+      arg->specifics.password().client_only_encrypted_data().password_issues();
+  return SpecificsHasExpectedCompromiseTypes(password_issues_data,
+                                             expected_issue_types);
+}
+
 // |*arg| must be of type PasswordForm.
 MATCHER_P(FormHasSignonRealm, expected_signon_realm, "") {
   return arg.signon_realm == expected_signon_realm;
@@ -179,6 +203,11 @@ class FakeDatabase {
     return FormRetrievalResult::kSuccess;
   }
 
+  std::vector<CompromisedCredentials> ReadSecurityIssues(
+      FormPrimaryKey parent_key) {
+    return security_issues_[parent_key.value()];
+  }
+
   PasswordStoreChangeList AddLogin(const PasswordForm& form,
                                    AddLoginError* error) {
     if (error) {
@@ -192,6 +221,18 @@ class FakeDatabase {
     return PasswordStoreChangeList();
   }
 
+  bool AddCompromisedCredentials(
+      base::span<const CompromisedCredentials> issues) {
+    if (issues.empty())
+      return true;
+    int primary_key = GetPrimaryKey(issues[0]);
+    DCHECK_NE(primary_key, -1);
+    DCHECK(!base::Contains(security_issues_, primary_key));
+    for (const auto& issue : issues)
+      security_issues_[primary_key].push_back(issue);
+    return true;
+  }
+
   PasswordStoreChangeList AddLoginForPrimaryKey(int primary_key,
                                                 const PasswordForm& form) {
     DCHECK_EQ(0U, data_.count(primary_key));
@@ -229,8 +270,19 @@ class FakeDatabase {
     return -1;
   }
 
+  int GetPrimaryKey(const CompromisedCredentials& issue) const {
+    for (const auto& pair : data_) {
+      if (pair.second->username_value == issue.username &&
+          pair.second->signon_realm == issue.signon_realm) {
+        return pair.first;
+      }
+    }
+    return -1;
+  }
+
   int primary_key_ = 1;
   PrimaryKeyToFormMap data_;
+  std::map<int, std::vector<CompromisedCredentials>> security_issues_;
   AddLoginError error_ = AddLoginError::kNone;
 
   DISALLOW_COPY_AND_ASSIGN(FakeDatabase);
@@ -325,8 +377,13 @@ class PasswordSyncBridgeTest : public testing::Test {
         .WillByDefault(testing::Return(&mock_sync_metadata_store_sync_));
     ON_CALL(mock_password_store_sync_, ReadAllLogins)
         .WillByDefault(Invoke(&fake_db_, &FakeDatabase::ReadAllLogins));
+    ON_CALL(mock_password_store_sync_, ReadSecurityIssues)
+        .WillByDefault(Invoke(&fake_db_, &FakeDatabase::ReadSecurityIssues));
     ON_CALL(mock_password_store_sync_, AddLoginSync)
         .WillByDefault(Invoke(&fake_db_, &FakeDatabase::AddLogin));
+    ON_CALL(mock_password_store_sync_, AddCompromisedCredentialsSync)
+        .WillByDefault(
+            Invoke(&fake_db_, &FakeDatabase::AddCompromisedCredentials));
     ON_CALL(mock_password_store_sync_, UpdateLoginSync)
         .WillByDefault(Invoke(&fake_db_, &FakeDatabase::UpdateLogin));
     ON_CALL(mock_password_store_sync_, RemoveLoginByPrimaryKeySync)
@@ -1201,4 +1258,70 @@ TEST_F(PasswordSyncBridgeTest,
   EXPECT_EQ(error, base::nullopt);
 }
 
+TEST_F(PasswordSyncBridgeTest, ShouldPutSecurityIssuesOnLoginChange) {
+  ON_CALL(mock_processor(), IsTrackingMetadata()).WillByDefault(Return(true));
+  // Since this remote creation is the first entry in the FakeDatabase, it will
+  // be assigned a primary key 1.
+  const int kPrimaryKey1 = 1;
+  const std::string kPrimaryKeyStr1 = "1";
+  const std::vector<CompromiseType> kIssuesTypes = {CompromiseType::kLeaked,
+                                                    CompromiseType::kReused};
+  const PasswordForm kForm = MakePasswordForm(kSignonRealm1);
+
+  fake_db()->AddLoginForPrimaryKey(kPrimaryKey1, kForm);
+  fake_db()->AddCompromisedCredentials(
+      MakeCompromisedCredentials(kForm, kIssuesTypes));
+
+  PasswordStoreChangeList changes;
+  changes.push_back(
+      PasswordStoreChange(PasswordStoreChange::UPDATE, kForm, kPrimaryKey1));
+  EXPECT_CALL(
+      mock_processor(),
+      Put(kPrimaryKeyStr1, EntityDataHasSecurityIssueTypes(kIssuesTypes), _));
+
+  bridge()->ActOnPasswordStoreChanges(changes);
+}
+
+TEST_F(PasswordSyncBridgeTest, ShouldAddLocalSecurityIssuesDuringInitialMerge) {
+  const int kPrimaryKey1 = 1000;
+  const std::string kPrimaryKeyStr1 = "1000";
+  const std::vector<CompromiseType> kIssuesTypes = {CompromiseType::kLeaked,
+                                                    CompromiseType::kReused};
+  const PasswordForm kForm = MakePasswordForm(kSignonRealm1);
+
+  sync_pb::PasswordSpecifics specifics1 =
+      CreateSpecificsWithSignonRealm(kSignonRealm1);
+
+  fake_db()->AddLoginForPrimaryKey(kPrimaryKey1, kForm);
+  fake_db()->AddCompromisedCredentials(
+      MakeCompromisedCredentials(kForm, kIssuesTypes));
+
+  EXPECT_CALL(
+      mock_processor(),
+      Put(kPrimaryKeyStr1, EntityDataHasSecurityIssueTypes(kIssuesTypes), _));
+
+  base::Optional<syncer::ModelError> error =
+      bridge()->MergeSyncData(bridge()->CreateMetadataChangeList(), {});
+  EXPECT_FALSE(error);
+}
+
+TEST_F(PasswordSyncBridgeTest, GetDataWithIssuesForStorageKey) {
+  const int kPrimaryKey1 = 1000;
+  const std::string kPrimaryKeyStr1 = "1000";
+  const std::vector<CompromiseType> kIssuesTypes = {CompromiseType::kLeaked,
+                                                    CompromiseType::kReused};
+  const PasswordForm kForm = MakePasswordForm(kSignonRealm1);
+
+  fake_db()->AddLoginForPrimaryKey(kPrimaryKey1, kForm);
+  fake_db()->AddCompromisedCredentials(
+      MakeCompromisedCredentials(kForm, kIssuesTypes));
+
+  base::Optional<sync_pb::PasswordSpecifics> optional_specifics =
+      GetDataFromBridge(/*storage_key=*/kPrimaryKeyStr1);
+  ASSERT_TRUE(optional_specifics.has_value());
+  ASSERT_TRUE(SpecificsHasExpectedCompromiseTypes(
+      optional_specifics.value().client_only_encrypted_data().password_issues(),
+      kIssuesTypes));
+}
+
 }  // namespace password_manager