Commit c0c17db9 authored by Jialiu Lin's avatar Jialiu Lin Committed by Commit Bot

Revert "Add Finch feature to gate Enterprise Password Protection"

This reverts commit 063960e4.

Reason for revert: Breaking waterfall builds

Original change's description:
> 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}

TBR=gayane@chromium.org,dvadym@chromium.org,jialiul@chromium.org,lpz@chromium.org

Change-Id: Iaf5149ca999a951775bdd4b90282e31ee8507469
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 804490, 818559
Reviewed-on: https://chromium-review.googlesource.com/960201Reviewed-by: default avatarJialiu Lin <jialiul@chromium.org>
Commit-Queue: Jialiu Lin <jialiul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542760}
parent 90293972
...@@ -832,9 +832,6 @@ MaybeCreateNavigationThrottle(content::NavigationHandle* navigation_handle) { ...@@ -832,9 +832,6 @@ MaybeCreateNavigationThrottle(content::NavigationHandle* navigation_handle) {
PasswordProtectionTrigger PasswordProtectionTrigger
ChromePasswordProtectionService::GetPasswordProtectionTriggerPref( ChromePasswordProtectionService::GetPasswordProtectionTriggerPref(
const std::string& pref_name) const { const std::string& pref_name) const {
if (!base::FeatureList::IsEnabled(kEnterprisePasswordProtectionV1))
return PHISHING_REUSE;
const PrefService::Preference* warning_trigger = const PrefService::Preference* warning_trigger =
profile_->GetPrefs()->FindPreference(pref_name); profile_->GetPrefs()->FindPreference(pref_name);
bool is_policy_managed = bool is_policy_managed =
......
...@@ -305,9 +305,6 @@ TEST_F(ChromePasswordProtectionServiceTest, ...@@ -305,9 +305,6 @@ TEST_F(ChromePasswordProtectionServiceTest,
TEST_F(ChromePasswordProtectionServiceTest, TEST_F(ChromePasswordProtectionServiceTest,
VerifyPingingIsSkippedIfMatchEnterpriseWhitelist) { VerifyPingingIsSkippedIfMatchEnterpriseWhitelist) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
safe_browsing::kEnterprisePasswordProtectionV1);
PasswordProtectionService::RequestOutcome reason = PasswordProtectionService::RequestOutcome reason =
PasswordProtectionService::UNKNOWN; PasswordProtectionService::UNKNOWN;
ASSERT_FALSE( ASSERT_FALSE(
......
...@@ -1657,8 +1657,6 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest, ...@@ -1657,8 +1657,6 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
// enterprise safe browsing whitelist domains. // enterprise safe browsing whitelist domains.
IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest, IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
VerifyEnterpriseWhitelist) { VerifyEnterpriseWhitelist) {
base::test::ScopedFeatureList scoped_features;
scoped_features.InitAndEnableFeature(kEnterprisePasswordProtectionV1);
GURL url = embedded_test_server()->GetURL(kEmptyPage); GURL url = embedded_test_server()->GetURL(kEmptyPage);
// Add test server domain into the enterprise whitelist. // Add test server domain into the enterprise whitelist.
base::ListValue whitelist; base::ListValue whitelist;
......
...@@ -61,9 +61,6 @@ source_set("unit_tests") { ...@@ -61,9 +61,6 @@ source_set("unit_tests") {
] ]
if (!is_ios) { if (!is_ios) {
deps += [ deps += [ "//components/safe_browsing/common:safe_browsing_prefs" ]
"//components/safe_browsing:features",
"//components/safe_browsing/common:safe_browsing_prefs",
]
} }
} }
...@@ -2,7 +2,6 @@ include_rules = [ ...@@ -2,7 +2,6 @@ include_rules = [
"+components/keyed_service/core", "+components/keyed_service/core",
"+components/pref_registry", "+components/pref_registry",
"+components/safe_browsing/common", "+components/safe_browsing/common",
"+components/safe_browsing/features.h",
"+components/signin/core/browser", "+components/signin/core/browser",
"+components/sync/base", "+components/sync/base",
"+components/sync/driver", "+components/sync/driver",
......
...@@ -15,9 +15,7 @@ ...@@ -15,9 +15,7 @@
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#if !defined(OS_IOS) #if !defined(OS_IOS)
#include "base/test/scoped_feature_list.h"
#include "components/safe_browsing/common/safe_browsing_prefs.h" #include "components/safe_browsing/common/safe_browsing_prefs.h"
#include "components/safe_browsing/features.h"
#endif // !OS_IOS #endif // !OS_IOS
using autofill::PasswordForm; using autofill::PasswordForm;
...@@ -104,13 +102,10 @@ class PasswordSyncUtilEnterpriseTest : public SyncUsernameTestBase { ...@@ -104,13 +102,10 @@ class PasswordSyncUtilEnterpriseTest : public SyncUsernameTestBase {
prefs_.registry()->RegisterListPref(prefs::kPasswordProtectionLoginURLs); prefs_.registry()->RegisterListPref(prefs::kPasswordProtectionLoginURLs);
prefs_.registry()->RegisterStringPref( prefs_.registry()->RegisterStringPref(
prefs::kPasswordProtectionChangePasswordURL, ""); prefs::kPasswordProtectionChangePasswordURL, "");
feature_list_.InitAndEnableFeature(
safe_browsing::kEnterprisePasswordProtectionV1);
} }
protected: protected:
TestingPrefServiceSimple prefs_; TestingPrefServiceSimple prefs_;
base::test::ScopedFeatureList feature_list_;
}; };
TEST_F(PasswordSyncUtilEnterpriseTest, TEST_F(PasswordSyncUtilEnterpriseTest,
......
...@@ -37,7 +37,6 @@ static_library("safe_browsing_prefs") { ...@@ -37,7 +37,6 @@ static_library("safe_browsing_prefs") {
"//base:base", "//base:base",
"//components/pref_registry:pref_registry", "//components/pref_registry:pref_registry",
"//components/prefs", "//components/prefs",
"//components/safe_browsing:features",
"//content/public/browser:browser", "//content/public/browser:browser",
"//net:net", "//net:net",
] ]
...@@ -53,7 +52,6 @@ source_set("unit_tests") { ...@@ -53,7 +52,6 @@ source_set("unit_tests") {
"//base:base", "//base:base",
"//base/test:test_support", "//base/test:test_support",
"//components/prefs:test_support", "//components/prefs:test_support",
"//components/safe_browsing:features",
"//content/test:test_support", "//content/test:test_support",
"//testing/gtest", "//testing/gtest",
"//url:url", "//url:url",
......
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
#include "components/pref_registry/pref_registry_syncable.h" #include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_registry_simple.h" #include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/safe_browsing/features.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "net/base/url_util.h" #include "net/base/url_util.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -504,11 +503,9 @@ base::ListValue GetSafeBrowsingPreferencesList(PrefService* prefs) { ...@@ -504,11 +503,9 @@ base::ListValue GetSafeBrowsingPreferencesList(PrefService* prefs) {
void GetSafeBrowsingWhitelistDomainsPref( void GetSafeBrowsingWhitelistDomainsPref(
const PrefService& prefs, const PrefService& prefs,
std::vector<std::string>* out_canonicalized_domain_list) { std::vector<std::string>* out_canonicalized_domain_list) {
if (base::FeatureList::IsEnabled(kEnterprisePasswordProtectionV1)) { const base::ListValue* pref_value =
const base::ListValue* pref_value = prefs.GetList(prefs::kSafeBrowsingWhitelistDomains);
prefs.GetList(prefs::kSafeBrowsingWhitelistDomains); CanonicalizeDomainList(*pref_value, out_canonicalized_domain_list);
CanonicalizeDomainList(*pref_value, out_canonicalized_domain_list);
}
} }
void CanonicalizeDomainList( void CanonicalizeDomainList(
...@@ -529,10 +526,8 @@ void CanonicalizeDomainList( ...@@ -529,10 +526,8 @@ void CanonicalizeDomainList(
bool IsURLWhitelistedByPolicy(const GURL& url, bool IsURLWhitelistedByPolicy(const GURL& url,
StringListPrefMember* pref_member) { StringListPrefMember* pref_member) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO); DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
if (!pref_member || if (!pref_member)
!base::FeatureList::IsEnabled(kEnterprisePasswordProtectionV1)) {
return false; return false;
}
std::vector<std::string> sb_whitelist_domains = pref_member->GetValue(); std::vector<std::string> sb_whitelist_domains = pref_member->GetValue();
return std::find_if(sb_whitelist_domains.begin(), sb_whitelist_domains.end(), return std::find_if(sb_whitelist_domains.begin(), sb_whitelist_domains.end(),
[&url](const std::string& domain) { [&url](const std::string& domain) {
...@@ -542,9 +537,6 @@ bool IsURLWhitelistedByPolicy(const GURL& url, ...@@ -542,9 +537,6 @@ bool IsURLWhitelistedByPolicy(const GURL& url,
bool IsURLWhitelistedByPolicy(const GURL& url, const PrefService& pref) { bool IsURLWhitelistedByPolicy(const GURL& url, const PrefService& pref) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (!base::FeatureList::IsEnabled(kEnterprisePasswordProtectionV1))
return false;
if (!pref.HasPrefPath(prefs::kSafeBrowsingWhitelistDomains)) if (!pref.HasPrefPath(prefs::kSafeBrowsingWhitelistDomains))
return false; return false;
const base::ListValue* whitelist = const base::ListValue* whitelist =
...@@ -571,10 +563,8 @@ void GetPasswordProtectionLoginURLsPref(const PrefService& prefs, ...@@ -571,10 +563,8 @@ void GetPasswordProtectionLoginURLsPref(const PrefService& prefs,
bool MatchesPasswordProtectionLoginURL(const GURL& url, bool MatchesPasswordProtectionLoginURL(const GURL& url,
const PrefService& prefs) { const PrefService& prefs) {
if (!base::FeatureList::IsEnabled(kEnterprisePasswordProtectionV1) || if (!url.is_valid())
!url.is_valid()) {
return false; return false;
}
std::vector<GURL> login_urls; std::vector<GURL> login_urls;
GetPasswordProtectionLoginURLsPref(prefs, &login_urls); GetPasswordProtectionLoginURLsPref(prefs, &login_urls);
...@@ -606,10 +596,8 @@ GURL GetPasswordProtectionChangePasswordURLPref(const PrefService& prefs) { ...@@ -606,10 +596,8 @@ GURL GetPasswordProtectionChangePasswordURLPref(const PrefService& prefs) {
bool MatchesPasswordProtectionChangePasswordURL(const GURL& url, bool MatchesPasswordProtectionChangePasswordURL(const GURL& url,
const PrefService& prefs) { const PrefService& prefs) {
if (!base::FeatureList::IsEnabled(kEnterprisePasswordProtectionV1) || if (!url.is_valid())
!url.is_valid()) {
return false; return false;
}
GURL change_password_url = GetPasswordProtectionChangePasswordURLPref(prefs); GURL change_password_url = GetPasswordProtectionChangePasswordURLPref(prefs);
if (change_password_url.is_empty()) if (change_password_url.is_empty())
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "components/prefs/pref_registry_simple.h" #include "components/prefs/pref_registry_simple.h"
#include "components/prefs/testing_pref_service.h" #include "components/prefs/testing_pref_service.h"
#include "components/safe_browsing/common/safe_browsing_prefs.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 "content/public/test/test_browser_thread_bundle.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -64,11 +63,6 @@ class SafeBrowsingPrefsTest : public ::testing::Test { ...@@ -64,11 +63,6 @@ class SafeBrowsingPrefsTest : public ::testing::Test {
base::JoinString(disabled_features, ",")); base::JoinString(disabled_features, ","));
} }
void EnableEnterprisePasswordProtectionFeature() {
feature_list_.reset(new base::test::ScopedFeatureList);
feature_list_->InitAndEnableFeature(kEnterprisePasswordProtectionV1);
}
std::string GetActivePref() { return GetExtendedReportingPrefName(prefs_); } std::string GetActivePref() { return GetExtendedReportingPrefName(prefs_); }
// Convenience method for explicitly setting up all combinations of prefs and // Convenience method for explicitly setting up all combinations of prefs and
...@@ -365,9 +359,7 @@ TEST_F(SafeBrowsingPrefsTest, GetSafeBrowsingExtendedReportingLevel) { ...@@ -365,9 +359,7 @@ TEST_F(SafeBrowsingPrefsTest, GetSafeBrowsingExtendedReportingLevel) {
EXPECT_EQ(SBER_LEVEL_SCOUT, GetExtendedReportingLevel(prefs_)); EXPECT_EQ(SBER_LEVEL_SCOUT, GetExtendedReportingLevel(prefs_));
} }
TEST_F(SafeBrowsingPrefsTest, VerifyMatchesPasswordProtectionLoginURL) { TEST_F(SafeBrowsingPrefsTest, VerifyIsPasswordProtectionLoginURL) {
EnableEnterprisePasswordProtectionFeature();
GURL url("https://mydomain.com/login.html#ref?username=alice"); GURL url("https://mydomain.com/login.html#ref?username=alice");
EXPECT_FALSE(prefs_.HasPrefPath(prefs::kPasswordProtectionLoginURLs)); EXPECT_FALSE(prefs_.HasPrefPath(prefs::kPasswordProtectionLoginURLs));
EXPECT_FALSE(MatchesPasswordProtectionLoginURL(url, prefs_)); EXPECT_FALSE(MatchesPasswordProtectionLoginURL(url, prefs_));
...@@ -384,10 +376,7 @@ TEST_F(SafeBrowsingPrefsTest, VerifyMatchesPasswordProtectionLoginURL) { ...@@ -384,10 +376,7 @@ TEST_F(SafeBrowsingPrefsTest, VerifyMatchesPasswordProtectionLoginURL) {
EXPECT_TRUE(MatchesPasswordProtectionLoginURL(url, prefs_)); EXPECT_TRUE(MatchesPasswordProtectionLoginURL(url, prefs_));
} }
TEST_F(SafeBrowsingPrefsTest, TEST_F(SafeBrowsingPrefsTest, VerifyIsPasswordProtectionChangePasswordURL) {
VerifyMatchesPasswordProtectionChangePasswordURL) {
EnableEnterprisePasswordProtectionFeature();
GURL url("https://mydomain.com/change_password.html#ref?username=alice"); GURL url("https://mydomain.com/change_password.html#ref?username=alice");
EXPECT_FALSE(prefs_.HasPrefPath(prefs::kPasswordProtectionChangePasswordURL)); EXPECT_FALSE(prefs_.HasPrefPath(prefs::kPasswordProtectionChangePasswordURL));
EXPECT_FALSE(MatchesPasswordProtectionChangePasswordURL(url, prefs_)); EXPECT_FALSE(MatchesPasswordProtectionChangePasswordURL(url, prefs_));
...@@ -438,8 +427,6 @@ TEST_F(SafeBrowsingPrefsTest, IsExtendedReportingPolicyManaged) { ...@@ -438,8 +427,6 @@ TEST_F(SafeBrowsingPrefsTest, IsExtendedReportingPolicyManaged) {
} }
TEST_F(SafeBrowsingPrefsTest, VerifyIsURLWhitelistedByPolicy) { TEST_F(SafeBrowsingPrefsTest, VerifyIsURLWhitelistedByPolicy) {
EnableEnterprisePasswordProtectionFeature();
GURL target_url("https://www.foo.com"); GURL target_url("https://www.foo.com");
// When PrefMember is null, URL is not whitelisted. // When PrefMember is null, URL is not whitelisted.
EXPECT_FALSE(IsURLWhitelistedByPolicy(target_url, nullptr)); EXPECT_FALSE(IsURLWhitelistedByPolicy(target_url, nullptr));
......
...@@ -65,9 +65,6 @@ const base::Feature kAppendRecentNavigationEvents{ ...@@ -65,9 +65,6 @@ const base::Feature kAppendRecentNavigationEvents{
const base::Feature kInspectDownloadedRarFiles{ const base::Feature kInspectDownloadedRarFiles{
"InspectDownloadedRarFiles", base::FEATURE_DISABLED_BY_DEFAULT}; "InspectDownloadedRarFiles", base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kEnterprisePasswordProtectionV1{
"EnterprisePasswordProtectionV1", base::FEATURE_DISABLED_BY_DEFAULT};
namespace { namespace {
// List of experimental features. Boolean value for each list member should be // 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 // set to true if the experiment is currently running at a probability other
...@@ -82,7 +79,6 @@ constexpr struct { ...@@ -82,7 +79,6 @@ constexpr struct {
{&kAppendRecentNavigationEvents, true}, {&kAppendRecentNavigationEvents, true},
{&kCheckByURLLoaderThrottle, true}, {&kCheckByURLLoaderThrottle, true},
{&kDispatchSafetyNetCheckOffThread, false}, {&kDispatchSafetyNetCheckOffThread, false},
{&kEnterprisePasswordProtectionV1, true},
{&kGaiaPasswordReuseReporting, true}, {&kGaiaPasswordReuseReporting, true},
{&kGoogleBrandedPhishingWarning, true}, {&kGoogleBrandedPhishingWarning, true},
{&kInspectDownloadedRarFiles, true}, {&kInspectDownloadedRarFiles, true},
......
...@@ -52,9 +52,6 @@ extern const base::Feature kAppendRecentNavigationEvents; ...@@ -52,9 +52,6 @@ extern const base::Feature kAppendRecentNavigationEvents;
// unsafe. // unsafe.
extern const base::Feature kInspectDownloadedRarFiles; extern const base::Feature kInspectDownloadedRarFiles;
// Control the Password Protection for Enterprise V1 feature;
extern const base::Feature kEnterprisePasswordProtectionV1;
base::ListValue GetFeatureStatusList(); base::ListValue GetFeatureStatusList();
} // namespace safe_browsing } // namespace safe_browsing
......
...@@ -2497,24 +2497,6 @@ ...@@ -2497,24 +2497,6 @@
] ]
} }
], ],
"PasswordProtectionForEnterprise": [
{
"platforms": [
"chromeos",
"linux",
"mac",
"win"
],
"experiments": [
{
"name": "V1Enabled",
"enable_features": [
"EnterprisePasswordProtectionV1"
]
}
]
}
],
"PasswordSeparatedSigninFlow": [ "PasswordSeparatedSigninFlow": [
{ {
"platforms": [ "platforms": [
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment