Add Finch feature to gate Enterprise Password Protection

Also added finch experiment to fieldtrial_testing_config.json

Bug: 804490,818559
Change-Id: I70b0bfdaf7d59ca91f27d634363221b5f3cdb403
Reviewed-on: https://chromium-review.googlesource.com/942308
Commit-Queue: Jialiu Lin <jialiul@chromium.org>
Reviewed-by: Luke Z <lpz@chromium.org>
Reviewed-by: Gayane Petrosyan <gayane@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542502}

chrome/browser/safe_browsing/chrome_password_protection_service.cc

@@ -832,6 +832,9 @@ MaybeCreateNavigationThrottle(content::NavigationHandle* navigation_handle) {
 PasswordProtectionTrigger
 ChromePasswordProtectionService::GetPasswordProtectionTriggerPref(
     const std::string& pref_name) const {
+  if (!base::FeatureList::IsEnabled(kEnterprisePasswordProtectionV1))
+    return PHISHING_REUSE;
+
   const PrefService::Preference* warning_trigger =
       profile_->GetPrefs()->FindPreference(pref_name);
   bool is_policy_managed =

chrome/browser/safe_browsing/chrome_password_protection_service_unittest.cc

@@ -305,6 +305,9 @@ TEST_F(ChromePasswordProtectionServiceTest,
 
 TEST_F(ChromePasswordProtectionServiceTest,
        VerifyPingingIsSkippedIfMatchEnterpriseWhitelist) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(
+      safe_browsing::kEnterprisePasswordProtectionV1);
   PasswordProtectionService::RequestOutcome reason =
       PasswordProtectionService::UNKNOWN;
   ASSERT_FALSE(

chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc

@@ -1657,6 +1657,8 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
 // enterprise safe browsing whitelist domains.
 IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
                        VerifyEnterpriseWhitelist) {
+  base::test::ScopedFeatureList scoped_features;
+  scoped_features.InitAndEnableFeature(kEnterprisePasswordProtectionV1);
   GURL url = embedded_test_server()->GetURL(kEmptyPage);
   // Add test server domain into the enterprise whitelist.
   base::ListValue whitelist;

components/password_manager/sync/browser/BUILD.gn

@@ -61,6 +61,9 @@ source_set("unit_tests") {
   ]
 
   if (!is_ios) {
-    deps += [ "//components/safe_browsing/common:safe_browsing_prefs" ]
+    deps += [
+      "//components/safe_browsing:features",
+      "//components/safe_browsing/common:safe_browsing_prefs",
+    ]
   }
 }

components/password_manager/sync/browser/DEPS

@@ -2,6 +2,7 @@ include_rules = [
   "+components/keyed_service/core",
   "+components/pref_registry",
   "+components/safe_browsing/common",
+  "+components/safe_browsing/features.h",
   "+components/signin/core/browser",
   "+components/sync/base",
   "+components/sync/driver",

components/password_manager/sync/browser/password_sync_util_unittest.cc

@@ -15,7 +15,9 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if !defined(OS_IOS)
+#include "base/test/scoped_feature_list.h"
 #include "components/safe_browsing/common/safe_browsing_prefs.h"
+#include "components/safe_browsing/features.h"
 #endif  // !OS_IOS
 
 using autofill::PasswordForm;
@@ -102,10 +104,13 @@ class PasswordSyncUtilEnterpriseTest : public SyncUsernameTestBase {
     prefs_.registry()->RegisterListPref(prefs::kPasswordProtectionLoginURLs);
     prefs_.registry()->RegisterStringPref(
         prefs::kPasswordProtectionChangePasswordURL, "");
+    feature_list_.InitAndEnableFeature(
+        safe_browsing::kEnterprisePasswordProtectionV1);
   }
 
  protected:
   TestingPrefServiceSimple prefs_;
+  base::test::ScopedFeatureList feature_list_;
 };
 
 TEST_F(PasswordSyncUtilEnterpriseTest,

components/safe_browsing/common/BUILD.gn

@@ -37,6 +37,7 @@ static_library("safe_browsing_prefs") {
     "//base:base",
     "//components/pref_registry:pref_registry",
     "//components/prefs",
+    "//components/safe_browsing:features",
     "//content/public/browser:browser",
     "//net:net",
   ]
@@ -52,6 +53,7 @@ source_set("unit_tests") {
     "//base:base",
     "//base/test:test_support",
     "//components/prefs:test_support",
+    "//components/safe_browsing:features",
     "//content/test:test_support",
     "//testing/gtest",
     "//url:url",

components/safe_browsing/common/safe_browsing_prefs.cc

@@ -10,6 +10,7 @@
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
+#include "components/safe_browsing/features.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/base/url_util.h"
 #include "url/gurl.h"
@@ -503,9 +504,11 @@ base::ListValue GetSafeBrowsingPreferencesList(PrefService* prefs) {
 void GetSafeBrowsingWhitelistDomainsPref(
     const PrefService& prefs,
     std::vector<std::string>* out_canonicalized_domain_list) {
-  const base::ListValue* pref_value =
-      prefs.GetList(prefs::kSafeBrowsingWhitelistDomains);
-  CanonicalizeDomainList(*pref_value, out_canonicalized_domain_list);
+  if (base::FeatureList::IsEnabled(kEnterprisePasswordProtectionV1)) {
+    const base::ListValue* pref_value =
+        prefs.GetList(prefs::kSafeBrowsingWhitelistDomains);
+    CanonicalizeDomainList(*pref_value, out_canonicalized_domain_list);
+  }
 }
 
 void CanonicalizeDomainList(
@@ -526,8 +529,10 @@ void CanonicalizeDomainList(
 bool IsURLWhitelistedByPolicy(const GURL& url,
                               StringListPrefMember* pref_member) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-  if (!pref_member)
+  if (!pref_member ||
+      !base::FeatureList::IsEnabled(kEnterprisePasswordProtectionV1)) {
     return false;
+  }
   std::vector<std::string> sb_whitelist_domains = pref_member->GetValue();
   return std::find_if(sb_whitelist_domains.begin(), sb_whitelist_domains.end(),
                       [&url](const std::string& domain) {
@@ -537,6 +542,9 @@ bool IsURLWhitelistedByPolicy(const GURL& url,
 
 bool IsURLWhitelistedByPolicy(const GURL& url, const PrefService& pref) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (!base::FeatureList::IsEnabled(kEnterprisePasswordProtectionV1))
+    return false;
+
   if (!pref.HasPrefPath(prefs::kSafeBrowsingWhitelistDomains))
     return false;
   const base::ListValue* whitelist =
@@ -563,8 +571,10 @@ void GetPasswordProtectionLoginURLsPref(const PrefService& prefs,
 
 bool MatchesPasswordProtectionLoginURL(const GURL& url,
                                        const PrefService& prefs) {
-  if (!url.is_valid())
+  if (!base::FeatureList::IsEnabled(kEnterprisePasswordProtectionV1) ||
+      !url.is_valid()) {
     return false;
+  }
 
   std::vector<GURL> login_urls;
   GetPasswordProtectionLoginURLsPref(prefs, &login_urls);
@@ -596,8 +606,10 @@ GURL GetPasswordProtectionChangePasswordURLPref(const PrefService& prefs) {
 
 bool MatchesPasswordProtectionChangePasswordURL(const GURL& url,
                                                 const PrefService& prefs) {
-  if (!url.is_valid())
+  if (!base::FeatureList::IsEnabled(kEnterprisePasswordProtectionV1) ||
+      !url.is_valid()) {
     return false;
+  }
 
   GURL change_password_url = GetPasswordProtectionChangePasswordURLPref(prefs);
   if (change_password_url.is_empty())

components/safe_browsing/common/safe_browsing_prefs_unittest.cc

@@ -12,6 +12,7 @@
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/testing_pref_service.h"
 #include "components/safe_browsing/common/safe_browsing_prefs.h"
+#include "components/safe_browsing/features.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
@@ -63,6 +64,11 @@ class SafeBrowsingPrefsTest : public ::testing::Test {
         base::JoinString(disabled_features, ","));
   }
 
+  void EnableEnterprisePasswordProtectionFeature() {
+    feature_list_.reset(new base::test::ScopedFeatureList);
+    feature_list_->InitAndEnableFeature(kEnterprisePasswordProtectionV1);
+  }
+
   std::string GetActivePref() { return GetExtendedReportingPrefName(prefs_); }
 
   // Convenience method for explicitly setting up all combinations of prefs and
@@ -359,7 +365,9 @@ TEST_F(SafeBrowsingPrefsTest, GetSafeBrowsingExtendedReportingLevel) {
   EXPECT_EQ(SBER_LEVEL_SCOUT, GetExtendedReportingLevel(prefs_));
 }
 
-TEST_F(SafeBrowsingPrefsTest, VerifyIsPasswordProtectionLoginURL) {
+TEST_F(SafeBrowsingPrefsTest, VerifyMatchesPasswordProtectionLoginURL) {
+  EnableEnterprisePasswordProtectionFeature();
+
   GURL url("https://mydomain.com/login.html#ref?username=alice");
   EXPECT_FALSE(prefs_.HasPrefPath(prefs::kPasswordProtectionLoginURLs));
   EXPECT_FALSE(MatchesPasswordProtectionLoginURL(url, prefs_));
@@ -376,7 +384,10 @@ TEST_F(SafeBrowsingPrefsTest, VerifyIsPasswordProtectionLoginURL) {
   EXPECT_TRUE(MatchesPasswordProtectionLoginURL(url, prefs_));
 }
 
-TEST_F(SafeBrowsingPrefsTest, VerifyIsPasswordProtectionChangePasswordURL) {
+TEST_F(SafeBrowsingPrefsTest,
+       VerifyMatchesPasswordProtectionChangePasswordURL) {
+  EnableEnterprisePasswordProtectionFeature();
+
   GURL url("https://mydomain.com/change_password.html#ref?username=alice");
   EXPECT_FALSE(prefs_.HasPrefPath(prefs::kPasswordProtectionChangePasswordURL));
   EXPECT_FALSE(MatchesPasswordProtectionChangePasswordURL(url, prefs_));
@@ -427,6 +438,8 @@ TEST_F(SafeBrowsingPrefsTest, IsExtendedReportingPolicyManaged) {
 }
 
 TEST_F(SafeBrowsingPrefsTest, VerifyIsURLWhitelistedByPolicy) {
+  EnableEnterprisePasswordProtectionFeature();
+
   GURL target_url("https://www.foo.com");
   // When PrefMember is null, URL is not whitelisted.
   EXPECT_FALSE(IsURLWhitelistedByPolicy(target_url, nullptr));

components/safe_browsing/features.cc

@@ -65,6 +65,9 @@ const base::Feature kAppendRecentNavigationEvents{
 const base::Feature kInspectDownloadedRarFiles{
     "InspectDownloadedRarFiles", base::FEATURE_DISABLED_BY_DEFAULT};
 
+const base::Feature kEnterprisePasswordProtectionV1{
+    "EnterprisePasswordProtectionV1", base::FEATURE_DISABLED_BY_DEFAULT};
+
 namespace {
 // List of experimental features. Boolean value for each list member should be
 // set to true if the experiment is currently running at a probability other
@@ -79,6 +82,7 @@ constexpr struct {
     {&kAppendRecentNavigationEvents, true},
     {&kCheckByURLLoaderThrottle, true},
     {&kDispatchSafetyNetCheckOffThread, false},
+    {&kEnterprisePasswordProtectionV1, true},
     {&kGaiaPasswordReuseReporting, true},
     {&kGoogleBrandedPhishingWarning, true},
     {&kInspectDownloadedRarFiles, true},

components/safe_browsing/features.h

@@ -52,6 +52,9 @@ extern const base::Feature kAppendRecentNavigationEvents;
 // unsafe.
 extern const base::Feature kInspectDownloadedRarFiles;
 
+// Control the Password Protection for Enterprise V1 feature;
+extern const base::Feature kEnterprisePasswordProtectionV1;
+
 base::ListValue GetFeatureStatusList();
 
 }  // namespace safe_browsing

testing/variations/fieldtrial_testing_config.json

@@ -2496,6 +2496,24 @@
             ]
         }
     ],
+    "PasswordProtectionForEnterprise": [
+        {
+            "platforms": [
+                "chromeos",
+                "linux",
+                "mac",
+                "win"
+            ],
+            "experiments": [
+                {
+                    "name": "V1Enabled",
+                    "enable_features": [
+                        "EnterprisePasswordProtectionV1"
+                    ]
+                }
+            ]
+        }
+    ],
     "PasswordSeparatedSigninFlow": [
         {
             "platforms": [