[ios][PhishGuard] Enable Gaia and enterprise hash behind flag

Design doc: go/bling-phishguard

This CL continues work to compile code in iOS that was previously
disabled behind PASSWORD_REUSE_DETECTION_ENABLED.

Runtime changes are put behind a new feature flag.

Bug: 1147962
Change-Id: I102c7c34630e7d3e848f2077b2703d223e4a788c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2536936Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Ali Juma <ajuma@chromium.org>
Commit-Queue: edchin <edchin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827270}

components/password_manager/core/browser/password_sync_util.cc

@@ -7,6 +7,8 @@
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "components/password_manager/core/browser/password_form.h"
+#include "components/password_manager/core/common/password_manager_features.h"
+#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 #include "components/sync/base/user_selectable_type.h"
 #include "components/sync/driver/sync_user_settings.h"
@@ -14,10 +16,6 @@
 #include "google_apis/gaia/gaia_urls.h"
 #include "url/origin.h"
 
-#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
-#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
-#endif  // PASSWORD_REUSE_DETECTION_ENABLED
-
 using url::Origin;
 
 namespace {
@@ -87,13 +85,12 @@ bool IsGaiaCredentialPage(const std::string& signon_realm) {
 
 bool ShouldSaveEnterprisePasswordHash(const PasswordForm& form,
                                       const PrefService& prefs) {
-#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
-  return safe_browsing::MatchesPasswordProtectionLoginURL(form.url, prefs) ||
-         safe_browsing::MatchesPasswordProtectionChangePasswordURL(form.url,
-                                                                   prefs);
-#else
+  if (base::FeatureList::IsEnabled(features::kPasswordReuseDetectionEnabled)) {
+    return safe_browsing::MatchesPasswordProtectionLoginURL(form.url, prefs) ||
+           safe_browsing::MatchesPasswordProtectionChangePasswordURL(form.url,
+                                                                     prefs);
+  }
   return false;
-#endif  // PASSWORD_REUSE_DETECTION_ENABLED
 }
 
 }  // namespace sync_util

components/password_manager/core/browser/sync_credentials_filter.cc

@@ -68,12 +68,11 @@ bool SyncCredentialsFilter::ShouldSave(const PasswordForm& form) const {
 
 bool SyncCredentialsFilter::ShouldSaveGaiaPasswordHash(
     const PasswordForm& form) const {
-#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
-  return !client_->IsIncognito() &&
-         sync_util::IsGaiaCredentialPage(form.signon_realm);
-#else
+  if (base::FeatureList::IsEnabled(features::kPasswordReuseDetectionEnabled)) {
+    return !client_->IsIncognito() &&
+           sync_util::IsGaiaCredentialPage(form.signon_realm);
+  }
   return false;
-#endif  // PASSWORD_REUSE_DETECTION_ENABLED
 }
 
 bool SyncCredentialsFilter::ShouldSaveEnterprisePasswordHash(

components/password_manager/core/browser/sync_credentials_filter_unittest.cc

@@ -30,28 +30,26 @@
 #include "components/password_manager/core/common/password_manager_features.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/testing_pref_service.h"
+#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
-#include "components/safe_browsing/core/common/safe_browsing_prefs.h"  // nogncheck
-#endif  // PASSWORD_REUSE_DETECTION_ENABLED
-
 namespace password_manager {
 
 namespace {
 
 const char kFilledAndLoginActionName[] =
     "PasswordManager_SyncCredentialFilledAndLoginSuccessfull";
-
-#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
 const char kEnterpriseURL[] = "https://enterprise.test/";
-#endif  // PASSWORD_REUSE_DETECTION_ENABLED
 
 class FakePasswordManagerClient : public StubPasswordManagerClient {
  public:
   explicit FakePasswordManagerClient(signin::IdentityManager* identity_manager)
       : identity_manager_(identity_manager) {
-#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
+    if (!base::FeatureList::IsEnabled(
+            features::kPasswordReuseDetectionEnabled)) {
+      return;
+    }
+
     // Initializes and configures prefs.
     prefs_ = std::make_unique<TestingPrefServiceSimple>();
     prefs_->registry()->RegisterStringPref(
@@ -59,7 +57,6 @@ class FakePasswordManagerClient : public StubPasswordManagerClient {
     prefs_->registry()->RegisterListPref(prefs::kPasswordProtectionLoginURLs);
     prefs_->SetString(prefs::kPasswordProtectionChangePasswordURL,
                       kEnterpriseURL);
-#endif  // PASSWORD_REUSE_DETECTION_ENABLED
   }
 
   ~FakePasswordManagerClient() override {
@@ -81,9 +78,7 @@ class FakePasswordManagerClient : public StubPasswordManagerClient {
     last_committed_origin_ = url::Origin::Create(GURL(url_spec));
   }
 
-#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
   PrefService* GetPrefs() const override { return prefs_.get(); }
-#endif
 
   bool IsIncognito() const override { return is_incognito_; }
 
@@ -95,9 +90,7 @@ class FakePasswordManagerClient : public StubPasswordManagerClient {
       new testing::NiceMock<MockPasswordStore>;
   bool is_incognito_ = false;
   signin::IdentityManager* identity_manager_;
-#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
   std::unique_ptr<TestingPrefServiceSimple> prefs_;
-#endif  // PASSWORD_REUSE_DETECTION_ENABLED
 
   DISALLOW_COPY_AND_ASSIGN(FakePasswordManagerClient);
 };
@@ -112,26 +105,28 @@ class CredentialsFilterTest : public SyncUsernameTestBase,
   // Flag for creating a PasswordFormManager, deciding its IsNewLogin() value.
   enum class LoginState { NEW, EXISTING };
 
-  CredentialsFilterTest()
-      : client_(identity_manager()),
-        pending_(SimpleGaiaForm("user@gmail.com")),
-        form_manager_(&client_,
-                      driver_.AsWeakPtr(),
-                      pending_.form_data,
-                      &fetcher_,
-                      std::make_unique<PasswordSaveManagerImpl>(
-                          std::make_unique<StubFormSaver>()),
-                      nullptr /* metrics_recorder */),
-        filter_(&client_,
-                base::BindRepeating(&SyncUsernameTestBase::sync_service,
-                                    base::Unretained(this))) {
+  CredentialsFilterTest() : pending_(SimpleGaiaForm("user@gmail.com")) {
     if (GetParam()) {
-      feature_list_.InitAndEnableFeature(
-          features::kEnablePasswordsAccountStorage);
+      feature_list_.InitWithFeatures(
+          /*enable_features=*/{features::kPasswordReuseDetectionEnabled,
+                               features::kEnablePasswordsAccountStorage},
+          /*disable_features=*/{});
     } else {
-      feature_list_.InitAndDisableFeature(
-          features::kEnablePasswordsAccountStorage);
+      feature_list_.InitWithFeatures(
+          /*enable_features=*/{features::kPasswordReuseDetectionEnabled},
+          /*disable_features=*/{features::kEnablePasswordsAccountStorage});
     }
+
+    client_ = std::make_unique<FakePasswordManagerClient>(identity_manager());
+    form_manager_ = std::make_unique<PasswordFormManager>(
+        client_.get(), driver_.AsWeakPtr(), pending_.form_data, &fetcher_,
+        std::make_unique<PasswordSaveManagerImpl>(
+            std::make_unique<StubFormSaver>()),
+        nullptr /* metrics_recorder */);
+    filter_ = std::make_unique<SyncCredentialsFilter>(
+        client_.get(), base::BindRepeating(&SyncUsernameTestBase::sync_service,
+                                           base::Unretained(this)));
+
     fetcher_.Fetch();
   }
 
@@ -146,19 +141,19 @@ class CredentialsFilterTest : public SyncUsernameTestBase,
     fetcher_.SetNonFederated(matches);
     fetcher_.NotifyFetchCompleted();
 
-    form_manager_.ProvisionallySave(pending_.form_data, &driver_, nullptr);
+    form_manager_->ProvisionallySave(pending_.form_data, &driver_, nullptr);
   }
 
  protected:
   base::test::ScopedFeatureList feature_list_;
 
-  FakePasswordManagerClient client_;
+  std::unique_ptr<FakePasswordManagerClient> client_;
   StubPasswordManagerDriver driver_;
   PasswordForm pending_;
   FakeFormFetcher fetcher_;
-  PasswordFormManager form_manager_;
+  std::unique_ptr<PasswordFormManager> form_manager_;
 
-  SyncCredentialsFilter filter_;
+  std::unique_ptr<SyncCredentialsFilter> filter_;
 };
 
 TEST_P(CredentialsFilterTest, ReportFormLoginSuccess_ExistingSyncCredentials) {
@@ -167,7 +162,7 @@ TEST_P(CredentialsFilterTest, ReportFormLoginSuccess_ExistingSyncCredentials) {
 
   base::UserActionTester tester;
   SavePending(LoginState::EXISTING);
-  filter_.ReportFormLoginSuccess(form_manager_);
+  filter_->ReportFormLoginSuccess(*form_manager_);
   EXPECT_EQ(1, tester.GetActionCount(kFilledAndLoginActionName));
 }
 
@@ -177,7 +172,7 @@ TEST_P(CredentialsFilterTest, ReportFormLoginSuccess_NewSyncCredentials) {
 
   base::UserActionTester tester;
   SavePending(LoginState::NEW);
-  filter_.ReportFormLoginSuccess(form_manager_);
+  filter_->ReportFormLoginSuccess(*form_manager_);
   EXPECT_EQ(0, tester.GetActionCount(kFilledAndLoginActionName));
 }
 
@@ -189,7 +184,7 @@ TEST_P(CredentialsFilterTest, ReportFormLoginSuccess_GAIANotSyncCredentials) {
 
   base::UserActionTester tester;
   SavePending(LoginState::EXISTING);
-  filter_.ReportFormLoginSuccess(form_manager_);
+  filter_->ReportFormLoginSuccess(*form_manager_);
   EXPECT_EQ(0, tester.GetActionCount(kFilledAndLoginActionName));
 }
 
@@ -200,7 +195,7 @@ TEST_P(CredentialsFilterTest, ReportFormLoginSuccess_NotGAIACredentials) {
 
   base::UserActionTester tester;
   SavePending(LoginState::EXISTING);
-  filter_.ReportFormLoginSuccess(form_manager_);
+  filter_->ReportFormLoginSuccess(*form_manager_);
   EXPECT_EQ(0, tester.GetActionCount(kFilledAndLoginActionName));
 }
 
@@ -210,7 +205,7 @@ TEST_P(CredentialsFilterTest, ReportFormLoginSuccess_NotSyncing) {
 
   base::UserActionTester tester;
   SavePending(LoginState::EXISTING);
-  filter_.ReportFormLoginSuccess(form_manager_);
+  filter_->ReportFormLoginSuccess(*form_manager_);
   EXPECT_EQ(0, tester.GetActionCount(kFilledAndLoginActionName));
 }
 
@@ -224,9 +219,9 @@ TEST_P(CredentialsFilterTest, ShouldSave_NotSignedIn) {
   // account yet, then the just-signed-in account will *become* the primary
   // account immediately, so it shouldn't be saved either.
   if (base::FeatureList::IsEnabled(features::kEnablePasswordsAccountStorage))
-    EXPECT_FALSE(filter_.ShouldSave(form));
+    EXPECT_FALSE(filter_->ShouldSave(form));
   else
-    EXPECT_TRUE(filter_.ShouldSave(form));
+    EXPECT_TRUE(filter_->ShouldSave(form));
 }
 
 TEST_P(CredentialsFilterTest, ShouldSave_NotSyncCredential) {
@@ -234,7 +229,7 @@ TEST_P(CredentialsFilterTest, ShouldSave_NotSyncCredential) {
 
   FakeSigninAs("different_user@example.org");
   SetSyncingPasswords(true);
-  EXPECT_TRUE(filter_.ShouldSave(form));
+  EXPECT_TRUE(filter_->ShouldSave(form));
 }
 
 TEST_P(CredentialsFilterTest, ShouldSave_SyncCredential) {
@@ -242,7 +237,7 @@ TEST_P(CredentialsFilterTest, ShouldSave_SyncCredential) {
 
   FakeSigninAs("user@example.org");
   SetSyncingPasswords(true);
-  EXPECT_FALSE(filter_.ShouldSave(form));
+  EXPECT_FALSE(filter_->ShouldSave(form));
 }
 
 TEST_P(CredentialsFilterTest, ShouldSave_SignIn_Form) {
@@ -250,7 +245,7 @@ TEST_P(CredentialsFilterTest, ShouldSave_SignIn_Form) {
   form.form_data.is_gaia_with_skip_save_password_form = true;
 
   SetSyncingPasswords(false);
-  EXPECT_FALSE(filter_.ShouldSave(form));
+  EXPECT_FALSE(filter_->ShouldSave(form));
 }
 
 TEST_P(CredentialsFilterTest, ShouldSave_SyncCredential_NotSyncingPasswords) {
@@ -262,75 +257,73 @@ TEST_P(CredentialsFilterTest, ShouldSave_SyncCredential_NotSyncingPasswords) {
   // to save the password for the primary account - doesn't matter if passwords
   // are being synced or not.
   if (base::FeatureList::IsEnabled(features::kEnablePasswordsAccountStorage))
-    EXPECT_FALSE(filter_.ShouldSave(form));
+    EXPECT_FALSE(filter_->ShouldSave(form));
   else
-    EXPECT_TRUE(filter_.ShouldSave(form));
+    EXPECT_TRUE(filter_->ShouldSave(form));
 }
 
-#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
 TEST_P(CredentialsFilterTest, ShouldSaveGaiaPasswordHash) {
   PasswordForm gaia_form = SimpleGaiaForm("user@gmail.org");
-  EXPECT_TRUE(filter_.ShouldSaveGaiaPasswordHash(gaia_form));
+  EXPECT_TRUE(filter_->ShouldSaveGaiaPasswordHash(gaia_form));
 
   PasswordForm other_form = SimpleNonGaiaForm("user@example.org");
-  EXPECT_FALSE(filter_.ShouldSaveGaiaPasswordHash(other_form));
+  EXPECT_FALSE(filter_->ShouldSaveGaiaPasswordHash(other_form));
 }
 
 TEST_P(CredentialsFilterTest, ShouldNotSaveGaiaPasswordHashIncognito) {
-  client_.SetIsIncognito(true);
+  client_->SetIsIncognito(true);
   PasswordForm gaia_form = SimpleGaiaForm("user@gmail.org");
-  EXPECT_FALSE(filter_.ShouldSaveGaiaPasswordHash(gaia_form));
+  EXPECT_FALSE(filter_->ShouldSaveGaiaPasswordHash(gaia_form));
 
   PasswordForm other_form = SimpleNonGaiaForm("user@example.org");
-  EXPECT_FALSE(filter_.ShouldSaveGaiaPasswordHash(other_form));
+  EXPECT_FALSE(filter_->ShouldSaveGaiaPasswordHash(other_form));
 }
 
 TEST_P(CredentialsFilterTest, ShouldSaveEnterprisePasswordHash) {
   PasswordForm gaia_form = SimpleGaiaForm("user@gmail.org");
-  EXPECT_FALSE(filter_.ShouldSaveEnterprisePasswordHash(gaia_form));
+  EXPECT_FALSE(filter_->ShouldSaveEnterprisePasswordHash(gaia_form));
 
   PasswordForm other_form = SimpleNonGaiaForm("user@example.org");
-  EXPECT_FALSE(filter_.ShouldSaveEnterprisePasswordHash(other_form));
+  EXPECT_FALSE(filter_->ShouldSaveEnterprisePasswordHash(other_form));
 
   PasswordForm enterprise_form =
       SimpleNonGaiaForm("user@enterprise.test", kEnterpriseURL);
-  EXPECT_TRUE(filter_.ShouldSaveEnterprisePasswordHash(enterprise_form));
+  EXPECT_TRUE(filter_->ShouldSaveEnterprisePasswordHash(enterprise_form));
 }
 
 TEST_P(CredentialsFilterTest, ShouldNotSaveEnterprisePasswordHashIncognito) {
-  client_.SetIsIncognito(true);
+  client_->SetIsIncognito(true);
   PasswordForm gaia_form = SimpleGaiaForm("user@gmail.org");
-  EXPECT_FALSE(filter_.ShouldSaveEnterprisePasswordHash(gaia_form));
+  EXPECT_FALSE(filter_->ShouldSaveEnterprisePasswordHash(gaia_form));
 
   PasswordForm other_form = SimpleNonGaiaForm("user@example.org");
-  EXPECT_FALSE(filter_.ShouldSaveEnterprisePasswordHash(other_form));
+  EXPECT_FALSE(filter_->ShouldSaveEnterprisePasswordHash(other_form));
 
   PasswordForm enterprise_form =
       SimpleNonGaiaForm("user@enterprise.test", kEnterpriseURL);
-  EXPECT_FALSE(filter_.ShouldSaveEnterprisePasswordHash(enterprise_form));
+  EXPECT_FALSE(filter_->ShouldSaveEnterprisePasswordHash(enterprise_form));
 }
 
 TEST_P(CredentialsFilterTest, IsSyncAccountEmail) {
   FakeSigninAs("user@gmail.com");
-  EXPECT_FALSE(filter_.IsSyncAccountEmail("user"));
-  EXPECT_FALSE(filter_.IsSyncAccountEmail("user2@gmail.com"));
-  EXPECT_FALSE(filter_.IsSyncAccountEmail("user2@example.com"));
-  EXPECT_TRUE(filter_.IsSyncAccountEmail("user@gmail.com"));
-  EXPECT_TRUE(filter_.IsSyncAccountEmail("us.er@gmail.com"));
-  EXPECT_TRUE(filter_.IsSyncAccountEmail("user@googlemail.com"));
+  EXPECT_FALSE(filter_->IsSyncAccountEmail("user"));
+  EXPECT_FALSE(filter_->IsSyncAccountEmail("user2@gmail.com"));
+  EXPECT_FALSE(filter_->IsSyncAccountEmail("user2@example.com"));
+  EXPECT_TRUE(filter_->IsSyncAccountEmail("user@gmail.com"));
+  EXPECT_TRUE(filter_->IsSyncAccountEmail("us.er@gmail.com"));
+  EXPECT_TRUE(filter_->IsSyncAccountEmail("user@googlemail.com"));
 }
 
 TEST_P(CredentialsFilterTest, IsSyncAccountEmailIncognito) {
-  client_.SetIsIncognito(true);
+  client_->SetIsIncognito(true);
   FakeSigninAs("user@gmail.com");
-  EXPECT_FALSE(filter_.IsSyncAccountEmail("user"));
-  EXPECT_FALSE(filter_.IsSyncAccountEmail("user2@gmail.com"));
-  EXPECT_FALSE(filter_.IsSyncAccountEmail("user2@example.com"));
-  EXPECT_TRUE(filter_.IsSyncAccountEmail("user@gmail.com"));
-  EXPECT_TRUE(filter_.IsSyncAccountEmail("us.er@gmail.com"));
-  EXPECT_TRUE(filter_.IsSyncAccountEmail("user@googlemail.com"));
+  EXPECT_FALSE(filter_->IsSyncAccountEmail("user"));
+  EXPECT_FALSE(filter_->IsSyncAccountEmail("user2@gmail.com"));
+  EXPECT_FALSE(filter_->IsSyncAccountEmail("user2@example.com"));
+  EXPECT_TRUE(filter_->IsSyncAccountEmail("user@gmail.com"));
+  EXPECT_TRUE(filter_->IsSyncAccountEmail("us.er@gmail.com"));
+  EXPECT_TRUE(filter_->IsSyncAccountEmail("user@googlemail.com"));
 }
-#endif  // PASSWORD_REUSE_DETECTION_ENABLED
 
 INSTANTIATE_TEST_SUITE_P(, CredentialsFilterTest, ::testing::Bool());
 

components/password_manager/core/common/password_manager_features.cc

@@ -71,6 +71,16 @@ const base::Feature kPasswordCheck = {"PasswordCheck",
 const base::Feature kPasswordImport = {"PasswordImport",
                                        base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Enables password reuse detection.
+const base::Feature kPasswordReuseDetectionEnabled = {
+    "PasswordReuseDetectionEnabled",
+#if defined(OS_IOS)
+    base::FEATURE_DISABLED_BY_DEFAULT
+#else
+    base::FEATURE_ENABLED_BY_DEFAULT
+#endif  // defined(OS_IOS)
+};
+
 // Enables password scripts fetching for the |PasswordChangeInSettings| feature.
 const base::Feature kPasswordScriptsFetching = {
     "PasswordScriptsFetching", base::FEATURE_DISABLED_BY_DEFAULT};

components/password_manager/core/common/password_manager_features.h

@@ -30,6 +30,7 @@ extern const base::Feature kPasswordChange;
 extern const base::Feature kPasswordChangeInSettings;
 extern const base::Feature kPasswordCheck;
 extern const base::Feature kPasswordImport;
+extern const base::Feature kPasswordReuseDetectionEnabled;
 extern const base::Feature kPasswordScriptsFetching;
 extern const base::Feature kPasswordsWeaknessCheck;
 extern const base::Feature kRecoverFromNeverSaveAndroid;