From 01b93e625eaebf603b8bcb50f4d6ca64ff164dce Mon Sep 17 00:00:00 2001 From: mkarolin Date: Fri, 6 Mar 2020 19:44:00 -0500 Subject: [PATCH] Migrate shield settings. Migrates all shields settings patterns to use wildcard for protocol and port. This was needed for cookies due to Chromium change that uses SiteForCookies which doesn't contain port information to match settings patterns. However, using wildcards for protocol and port makes sense for all shields settings. Chromium change: https://chromium.googlesource.com/chromium/src/+/5d02d9e0ab081fb052eed0ab65581fb5b4622fa4 commit 5d02d9e0ab081fb052eed0ab65581fb5b4622fa4 Author: Maks Orlovich Date: Sun Dec 15 22:14:10 2019 +0000 Give ResourceRequest::site_for_cookies proper type. (With net::RedirectInfo matching it as well). It previously being a URL was highly misleading since: 1) There was a special meaning to empty URLs 2) The path bits weren't really guaranteed to exist. ... And also having a dedicated type will simplify computations by centralizing the use... in following CLs. Bug: 577565 --- ...browsing_data_remover_delegate_unittest.cc | 2 +- .../brave_site_settings_counter_unittest.cc | 2 +- common/pref_names.cc | 1 + common/pref_names.h | 1 + .../browser/brave_shields_util.cc | 18 +- .../browser/brave_shields_util.h | 3 +- .../browser/brave_shields_util_unittest.cc | 64 ++-- .../brave_content_settings_pref_provider.cc | 79 +++++ .../brave_content_settings_pref_provider.h | 9 + ...content_settings_pref_provider_unittest.cc | 330 ++++++++++++++++++ .../browser/brave_content_settings_utils.cc | 37 ++ .../browser/brave_content_settings_utils.h | 3 + .../brave_content_settings_utils_unittest.cc | 93 +++++ test/BUILD.gn | 3 + 14 files changed, 587 insertions(+), 58 deletions(-) create mode 100644 components/content_settings/core/browser/brave_content_settings_pref_provider_unittest.cc create mode 100644 components/content_settings/core/browser/brave_content_settings_utils_unittest.cc diff --git a/browser/browsing_data/brave_browsing_data_remover_delegate_unittest.cc b/browser/browsing_data/brave_browsing_data_remover_delegate_unittest.cc index b00b4495a14..4e22ba3b6a6 100644 --- a/browser/browsing_data/brave_browsing_data_remover_delegate_unittest.cc +++ b/browser/browsing_data/brave_browsing_data_remover_delegate_unittest.cc @@ -74,7 +74,7 @@ TEST_F(BraveBrowsingDataRemoverDelegateTest, ShieldsSettingsClearTest) { kBatURL, GURL(), ContentSettingsType::PLUGINS, brave_shields::kFingerprinting, CONTENT_SETTING_ALLOW); map()->SetContentSettingCustomScope( - brave_shields::GetPatternFromURL(kGoogleURL, true), + brave_shields::GetPatternFromURL(kGoogleURL), ContentSettingsPattern::Wildcard(), ContentSettingsType::JAVASCRIPT, "", CONTENT_SETTING_BLOCK); map()->SetContentSettingDefaultScope( diff --git a/browser/browsing_data/counters/brave_site_settings_counter_unittest.cc b/browser/browsing_data/counters/brave_site_settings_counter_unittest.cc index 42a2640d5b2..e78512733ee 100644 --- a/browser/browsing_data/counters/brave_site_settings_counter_unittest.cc +++ b/browser/browsing_data/counters/brave_site_settings_counter_unittest.cc @@ -100,7 +100,7 @@ TEST_F(BraveSiteSettingsCounterTest, Count) { kBatURL, GURL(), ContentSettingsType::PLUGINS, brave_shields::kFingerprinting, CONTENT_SETTING_ALLOW); map()->SetContentSettingCustomScope( - brave_shields::GetPatternFromURL(kGoogleURL, true), + brave_shields::GetPatternFromURL(kGoogleURL), ContentSettingsPattern::Wildcard(), ContentSettingsType::JAVASCRIPT, "", CONTENT_SETTING_BLOCK); map()->SetContentSettingDefaultScope( diff --git a/common/pref_names.cc b/common/pref_names.cc index 2e390af2158..38ae3cec545 100644 --- a/common/pref_names.cc +++ b/common/pref_names.cc @@ -78,6 +78,7 @@ const char kRemoteDebuggingEnabled[] = "brave.remote_debugging_enabled"; const char kAutocompleteEnabled[] = "brave.autocomplete_enabled"; const char kBraveDarkMode[] = "brave.dark_mode"; const char kOtherBookmarksMigrated[] = "brave.other_bookmarks_migrated"; +const char kBraveShieldsSettingsVersion[] = "brave.shields_settings_version"; #if !BUILDFLAG(USE_GCM_FROM_PLATFORM) const char kBraveGCMChannelStatus[] = "brave.gcm.channel_status"; #endif diff --git a/common/pref_names.h b/common/pref_names.h index 8b911a8d899..9adfda02193 100644 --- a/common/pref_names.h +++ b/common/pref_names.h @@ -69,6 +69,7 @@ extern const char kRemoteDebuggingEnabled[]; extern const char kAutocompleteEnabled[]; extern const char kBraveDarkMode[]; extern const char kOtherBookmarksMigrated[]; +extern const char kBraveShieldsSettingsVersion[]; #if !BUILDFLAG(USE_GCM_FROM_PLATFORM) extern const char kBraveGCMChannelStatus[]; #endif diff --git a/components/brave_shields/browser/brave_shields_util.cc b/components/brave_shields/browser/brave_shields_util.cc index 4d80adfbe7c..3b5a9a9d072 100644 --- a/components/brave_shields/browser/brave_shields_util.cc +++ b/components/brave_shields/browser/brave_shields_util.cc @@ -65,17 +65,11 @@ ContentSetting GetDefaultBlockFromControlType(ControlType type) { } // namespace -ContentSettingsPattern GetPatternFromURL(const GURL& url, - bool scheme_wildcard) { +ContentSettingsPattern GetPatternFromURL(const GURL& url) { DCHECK(url.is_empty() ? url.possibly_invalid_spec() == "" : url.is_valid()); if (url.is_empty() && url.possibly_invalid_spec() == "") return ContentSettingsPattern::Wildcard(); - auto origin = url.GetOrigin(); - return scheme_wildcard && !url.has_port() - ? ContentSettingsPattern::FromString("*://" + url.host() + "/*") - : ContentSettingsPattern::FromString( - origin.scheme() + "://" + origin.host() + ":" + - base::NumberToString(origin.EffectiveIntPort()) + "/*"); + return ContentSettingsPattern::FromString("*://" + url.host() + "/*"); } std::string ControlTypeToString(ControlType type) { @@ -117,7 +111,7 @@ void SetBraveShieldsEnabled(Profile* profile, DCHECK(!url.is_empty()) << "url for shields setting cannot be blank"; - auto primary_pattern = GetPatternFromURL(url, true); + auto primary_pattern = GetPatternFromURL(url); if (!primary_pattern.IsValid()) return; @@ -137,7 +131,7 @@ void ResetBraveShieldsEnabled(Profile* profile, if (url.is_valid() && !url.SchemeIsHTTPOrHTTPS()) return; - auto primary_pattern = GetPatternFromURL(url, true); + auto primary_pattern = GetPatternFromURL(url); if (!primary_pattern.IsValid()) return; @@ -311,7 +305,7 @@ ControlType GetFingerprintingControlType(Profile* profile, const GURL& url) { void SetHTTPSEverywhereEnabled(Profile* profile, bool enable, const GURL& url) { - auto primary_pattern = GetPatternFromURL(url, true); + auto primary_pattern = GetPatternFromURL(url); if (!primary_pattern.IsValid()) return; @@ -329,7 +323,7 @@ void SetHTTPSEverywhereEnabled(Profile* profile, void ResetHTTPSEverywhereEnabled(Profile* profile, bool enable, const GURL& url) { - auto primary_pattern = GetPatternFromURL(url, true); + auto primary_pattern = GetPatternFromURL(url); if (!primary_pattern.IsValid()) return; diff --git a/components/brave_shields/browser/brave_shields_util.h b/components/brave_shields/browser/brave_shields_util.h index 2721b247abf..206c949c9ac 100644 --- a/components/brave_shields/browser/brave_shields_util.h +++ b/components/brave_shields/browser/brave_shields_util.h @@ -25,8 +25,7 @@ namespace brave_shields { enum ControlType { ALLOW = 0, BLOCK, BLOCK_THIRD_PARTY, DEFAULT, INVALID }; -ContentSettingsPattern GetPatternFromURL(const GURL& url, - bool scheme_wildcard = false); +ContentSettingsPattern GetPatternFromURL(const GURL& url); std::string ControlTypeToString(ControlType type); ControlType ControlTypeFromString(const std::string& string); diff --git a/components/brave_shields/browser/brave_shields_util_unittest.cc b/components/brave_shields/browser/brave_shields_util_unittest.cc index 74689a17851..5d079d9ed70 100644 --- a/components/brave_shields/browser/brave_shields_util_unittest.cc +++ b/components/brave_shields/browser/brave_shields_util_unittest.cc @@ -42,64 +42,44 @@ TEST_F(BraveShieldsUtilTest, GetPatternFromURL) { auto pattern = GetPatternFromURL(GURL()); EXPECT_EQ(ContentSettingsPattern::Wildcard(), pattern); - // no scheme wildcard + // scheme is a wildcard, sould match any scheme pattern = GetPatternFromURL(GURL("http://brave.com")); EXPECT_TRUE(pattern.Matches(GURL("http://brave.com"))); EXPECT_TRUE(pattern.Matches(GURL("http://brave.com/path1"))); EXPECT_TRUE(pattern.Matches(GURL("http://brave.com/path2"))); - EXPECT_FALSE(pattern.Matches(GURL("https://brave.com"))); + EXPECT_TRUE(pattern.Matches(GURL("https://brave.com"))); + EXPECT_TRUE(pattern.Matches(GURL("ftp://brave.com"))); EXPECT_FALSE(pattern.Matches(GURL("http://subdomain.brave.com"))); EXPECT_FALSE(pattern.Matches(GURL("http://brave2.com"))); + // path is a wildcard pattern = GetPatternFromURL(GURL("http://brave.com/path1")); EXPECT_TRUE(pattern.Matches(GURL("http://brave.com"))); EXPECT_TRUE(pattern.Matches(GURL("http://brave.com/path1"))); EXPECT_TRUE(pattern.Matches(GURL("http://brave.com/path2"))); - EXPECT_FALSE(pattern.Matches(GURL("https://brave.com"))); EXPECT_FALSE(pattern.Matches(GURL("http://subdomain.brave.com"))); EXPECT_FALSE(pattern.Matches(GURL("http://brave2.com"))); - // with scheme wildcard - pattern = GetPatternFromURL(GURL("http://brave.com"), true); - EXPECT_TRUE(pattern.Matches(GURL("http://brave.com"))); - EXPECT_TRUE(pattern.Matches(GURL("http://brave.com/path1"))); - EXPECT_TRUE(pattern.Matches(GURL("http://brave.com/path2"))); - EXPECT_TRUE(pattern.Matches(GURL("https://brave.com"))); - EXPECT_FALSE(pattern.Matches(GURL("http://subdomain.brave.com"))); - EXPECT_FALSE(pattern.Matches(GURL("http://brave2.com"))); - - // with port + // port is a wildcard pattern = GetPatternFromURL(GURL("http://brave.com:8080")); + EXPECT_TRUE(pattern.Matches(GURL("http://brave.com"))); EXPECT_TRUE(pattern.Matches(GURL("http://brave.com:8080"))); EXPECT_TRUE(pattern.Matches(GURL("http://brave.com:8080/path1"))); EXPECT_TRUE(pattern.Matches(GURL("http://brave.com:8080/path2"))); - EXPECT_FALSE(pattern.Matches(GURL("https://brave.com:8080"))); - EXPECT_FALSE(pattern.Matches(GURL("http://brave.com"))); - EXPECT_FALSE(pattern.Matches(GURL("https://brave.com"))); + EXPECT_TRUE(pattern.Matches(GURL("http://brave.com:5555"))); + EXPECT_TRUE(pattern.Matches(GURL("https://brave.com"))); + EXPECT_TRUE(pattern.Matches(GURL("https://brave.com:8080"))); EXPECT_FALSE(pattern.Matches(GURL("http://subdomain.brave.com"))); EXPECT_FALSE(pattern.Matches(GURL("http://brave2.com"))); // with implied port pattern = GetPatternFromURL(GURL("https://brianbondy.com")); - EXPECT_EQ(pattern.ToString(), "https://brianbondy.com:443"); + EXPECT_EQ(pattern.ToString(), "brianbondy.com"); pattern = GetPatternFromURL(GURL("http://brianbondy.com")); - EXPECT_EQ(pattern.ToString(), "http://brianbondy.com:80"); + EXPECT_EQ(pattern.ToString(), "brianbondy.com"); // with specified port pattern = GetPatternFromURL(GURL("http://brianbondy.com:8080")); - EXPECT_EQ(pattern.ToString(), "http://brianbondy.com:8080"); - - // with port and scheme wildcard - // scheme wildcard with explicit port is not a valid pattern so this is - // identical to "with port" - pattern = GetPatternFromURL(GURL("http://brave.com:8080"), true); - EXPECT_TRUE(pattern.Matches(GURL("http://brave.com:8080"))); - EXPECT_TRUE(pattern.Matches(GURL("http://brave.com:8080/path1"))); - EXPECT_TRUE(pattern.Matches(GURL("http://brave.com:8080/path2"))); - EXPECT_FALSE(pattern.Matches(GURL("https://brave.com:8080"))); - EXPECT_FALSE(pattern.Matches(GURL("http://brave.com"))); - EXPECT_FALSE(pattern.Matches(GURL("https://brave.com"))); - EXPECT_FALSE(pattern.Matches(GURL("http://subdomain.brave.com"))); - EXPECT_FALSE(pattern.Matches(GURL("http://brave2.com:8080"))); + EXPECT_EQ(pattern.ToString(), "brianbondy.com"); } TEST_F(BraveShieldsUtilTest, ControlTypeToString) { @@ -248,11 +228,11 @@ TEST_F(BraveShieldsUtilTest, SetAdControlType_ForOrigin) { brave_shields::kAds); EXPECT_EQ(CONTENT_SETTING_ALLOW, setting); - // setting should not apply to different scheme + // setting should also apply to different scheme setting = map->GetContentSetting(GURL("https://brave.com"), GURL(), ContentSettingsType::PLUGINS, brave_shields::kAds); - EXPECT_EQ(CONTENT_SETTING_DEFAULT, setting); + EXPECT_EQ(CONTENT_SETTING_ALLOW, setting); // setting should not apply to default setting = map->GetContentSetting( @@ -437,15 +417,15 @@ TEST_F(BraveShieldsUtilTest, SetCookieControlType_ForOrigin) { ContentSettingsType::PLUGINS, brave_shields::kCookies); EXPECT_EQ(CONTENT_SETTING_ALLOW, setting); - // override should not apply to different scheme + // override should also apply to different scheme setting = map->GetContentSetting(GURL("https://brave.com"), GURL(), ContentSettingsType::PLUGINS, brave_shields::kCookies); - EXPECT_EQ(CONTENT_SETTING_DEFAULT, setting); + EXPECT_EQ(CONTENT_SETTING_ALLOW, setting); setting = map->GetContentSetting( GURL("https://brave.com"), GURL("https://firstParty"), ContentSettingsType::PLUGINS, brave_shields::kCookies); - EXPECT_EQ(CONTENT_SETTING_DEFAULT, setting); + EXPECT_EQ(CONTENT_SETTING_ALLOW, setting); // override should not apply to default setting = map->GetContentSetting( @@ -671,15 +651,15 @@ TEST_F(BraveShieldsUtilTest, SetFingerprintingControlType_ForOrigin) { ContentSettingsType::PLUGINS, brave_shields::kFingerprinting); EXPECT_EQ(CONTENT_SETTING_ALLOW, setting); - // override should not apply to different scheme + // override should also apply to different scheme setting = map->GetContentSetting(GURL("https://brave.com"), GURL(), ContentSettingsType::PLUGINS, brave_shields::kFingerprinting); - EXPECT_EQ(CONTENT_SETTING_DEFAULT, setting); + EXPECT_EQ(CONTENT_SETTING_ALLOW, setting); setting = map->GetContentSetting( GURL("https://brave.com"), GURL("https://firstParty"), ContentSettingsType::PLUGINS, brave_shields::kFingerprinting); - EXPECT_EQ(CONTENT_SETTING_DEFAULT, setting); + EXPECT_EQ(CONTENT_SETTING_ALLOW, setting); // override should not apply to default setting = @@ -999,10 +979,10 @@ TEST_F(BraveShieldsUtilTest, SetNoScriptControlType_ForOrigin) { ContentSettingsType::JAVASCRIPT, ""); EXPECT_EQ(CONTENT_SETTING_BLOCK, setting); - // setting should not apply to different scheme + // setting should also apply to different scheme setting = map->GetContentSetting(GURL("https://brave.com"), GURL(), ContentSettingsType::JAVASCRIPT, ""); - EXPECT_EQ(CONTENT_SETTING_ALLOW, setting); + EXPECT_EQ(CONTENT_SETTING_BLOCK, setting); // setting should not apply to default setting = map->GetContentSetting(GURL(), GURL(), diff --git a/components/content_settings/core/browser/brave_content_settings_pref_provider.cc b/components/content_settings/core/browser/brave_content_settings_pref_provider.cc index ab77fc86f7e..fd30aa5f101 100644 --- a/components/content_settings/core/browser/brave_content_settings_pref_provider.cc +++ b/components/content_settings/core/browser/brave_content_settings_pref_provider.cc @@ -9,13 +9,16 @@ #include #include "base/bind.h" +#include "base/optional.h" #include "base/task/post_task.h" #include "brave/common/network_constants.h" #include "brave/common/pref_names.h" #include "brave/components/brave_shields/common/brave_shield_constants.h" +#include "brave/components/content_settings/core/browser/brave_content_settings_utils.h" #include "components/content_settings/core/browser/content_settings_pref.h" #include "components/content_settings/core/browser/website_settings_registry.h" #include "components/content_settings/core/common/content_settings_utils.h" +#include "components/pref_registry/pref_registry_syncable.h" #include "components/prefs/pref_service.h" #include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_thread.h" @@ -119,6 +122,8 @@ BravePrefProvider::BravePrefProvider(PrefService* prefs, } } + MigrateShieldsSettings(off_the_record); + AddObserver(this); OnCookieSettingsChanged(ContentSettingsType::PLUGINS); } @@ -131,6 +136,80 @@ void BravePrefProvider::ShutdownOnUIThread() { PrefProvider::ShutdownOnUIThread(); } +// static +void BravePrefProvider::RegisterProfilePrefs( + user_prefs::PrefRegistrySyncable* registry) { + PrefProvider::RegisterProfilePrefs(registry); + // Register shields settings migration pref. + registry->RegisterIntegerPref(kBraveShieldsSettingsVersion, 1); +} + +void BravePrefProvider::MigrateShieldsSettings(bool incognito) { + // Incognito inherits from regular profile, so nothing to do. + if (incognito) + return; + MigrateShieldsSettingsV1ToV2(); +} + +void BravePrefProvider::MigrateShieldsSettingsV1ToV2() { + // Check if migration is needed. + if (prefs_->GetInteger(kBraveShieldsSettingsVersion) != 1) + return; + + // All sources in ContentSettingsType::PLUGINS we want to migrate. + for (const auto& resource_id : GetShieldsResourceIDs()) { + MigrateShieldsSettingsV1ToV2ForOneType(ContentSettingsType::PLUGINS, + resource_id); + } + + // ContentSettingsType::JAVASCRIPT. + MigrateShieldsSettingsV1ToV2ForOneType(ContentSettingsType::JAVASCRIPT, + std::string()); + + // Mark migration as done. + prefs_->SetInteger(kBraveShieldsSettingsVersion, 2); +} + +void BravePrefProvider::MigrateShieldsSettingsV1ToV2ForOneType( + ContentSettingsType content_type, + const std::string& resource_id) { + using OldRule = std::pair; + // Find rules that can be migrated and create replacement rules for them. + std::vector old_rules; + std::vector new_rules; + auto rule_iterator = PrefProvider::GetRuleIterator(content_type, resource_id, + /*off_the_record*/ false); + while (rule_iterator && rule_iterator->HasNext()) { + auto rule = rule_iterator->Next(); + auto new_primary_pattern = + ConvertPatternToWildcardSchemeAndPort(rule.primary_pattern); + auto new_secondary_pattern = + ConvertPatternToWildcardSchemeAndPort(rule.secondary_pattern); + if (new_primary_pattern || new_secondary_pattern) { + old_rules.emplace_back(rule.primary_pattern, rule.secondary_pattern); + new_rules.emplace_back( + new_primary_pattern.value_or(rule.primary_pattern), + new_secondary_pattern.value_or(rule.secondary_pattern), + rule.value.Clone()); + } + } + rule_iterator.reset(); + + // Migrate. + DCHECK(old_rules.size() == new_rules.size()); + for (size_t i = 0; i < old_rules.size(); i++) { + // Remove current setting. + PrefProvider::SetWebsiteSetting( + old_rules[i].first, old_rules[i].second, content_type, resource_id, + ContentSettingToValue(CONTENT_SETTING_DEFAULT)); + // Add new setting. + PrefProvider::SetWebsiteSetting( + new_rules[i].primary_pattern, new_rules[i].secondary_pattern, + content_type, resource_id, + ContentSettingToValue(ValueToContentSetting(&(new_rules[i].value)))); + } +} + bool BravePrefProvider::SetWebsiteSetting( const ContentSettingsPattern& primary_pattern, const ContentSettingsPattern& secondary_pattern, diff --git a/components/content_settings/core/browser/brave_content_settings_pref_provider.h b/components/content_settings/core/browser/brave_content_settings_pref_provider.h index 6b76b560fe8..97b5878cfb9 100644 --- a/components/content_settings/core/browser/brave_content_settings_pref_provider.h +++ b/components/content_settings/core/browser/brave_content_settings_pref_provider.h @@ -33,6 +33,8 @@ class BravePrefProvider : public PrefProvider, bool store_last_modified); ~BravePrefProvider() override; + static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry); + // content_settings::PrefProvider overrides: void ShutdownOnUIThread() override; bool SetWebsiteSetting( @@ -47,6 +49,13 @@ class BravePrefProvider : public PrefProvider, bool incognito) const override; private: + friend class BravePrefProviderTest; + FRIEND_TEST(BravePrefProviderTest, TestShieldsSettingsMigration); + FRIEND_TEST(BravePrefProviderTest, TestShieldsSettingsMigrationVersion); + void MigrateShieldsSettings(bool incognito); + void MigrateShieldsSettingsV1ToV2(); + void MigrateShieldsSettingsV1ToV2ForOneType(ContentSettingsType content_type, + const std::string& resource_id); void UpdateCookieRules(ContentSettingsType content_type, bool incognito); void OnCookieSettingsChanged(ContentSettingsType content_type); void NotifyChanges(const std::vector& rules, bool incognito); diff --git a/components/content_settings/core/browser/brave_content_settings_pref_provider_unittest.cc b/components/content_settings/core/browser/brave_content_settings_pref_provider_unittest.cc new file mode 100644 index 00000000000..7d95c1efb90 --- /dev/null +++ b/components/content_settings/core/browser/brave_content_settings_pref_provider_unittest.cc @@ -0,0 +1,330 @@ +/* Copyright (c) 2020 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include +#include + +#include "base/macros.h" +#include "base/optional.h" +#include "brave/common/pref_names.h" +#include "brave/components/brave_shields/common/brave_shield_constants.h" +#include "brave/components/content_settings/core/browser/brave_content_settings_pref_provider.h" +#include "chrome/test/base/testing_profile.h" +#include "components/content_settings/core/browser/content_settings_registry.h" +#include "components/content_settings/core/common/content_settings.h" +#include "components/content_settings/core/common/content_settings_pattern.h" +#include "components/content_settings/core/common/content_settings_utils.h" +#include "components/content_settings/core/test/content_settings_test_utils.h" +#include "components/pref_registry/pref_registry_syncable.h" +#include "components/prefs/pref_service.h" +#include "content/public/test/browser_task_environment.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "url/gurl.h" + +namespace content_settings { + +namespace { + +using GURLSourcePair = std::pair; + +ContentSettingsPattern SecondaryUrlToPattern(const GURL& gurl) { + CHECK(gurl == GURL() || gurl == GURL("https://firstParty/*")); + if (gurl == GURL()) + return ContentSettingsPattern::Wildcard(); + else + return ContentSettingsPattern::FromString("https://firstParty/*"); +} + +class ShieldsSetting { + public: + ShieldsSetting(BravePrefProvider* provider, + const std::vector urls) + : provider_(provider), urls_(urls) {} + virtual ~ShieldsSetting() = default; + + virtual void SetPreMigrationSettings(const ContentSettingsPattern& pattern, + ContentSetting setting) { + for (const auto& url_source : urls_) { + provider_->SetWebsiteSetting( + pattern, SecondaryUrlToPattern(url_source.first), + ContentSettingsType::PLUGINS, url_source.second, + ContentSettingToValue(setting)); + } + } + + void CheckSettingsAreDefault(const GURL& url) const { + CheckSettings(url, CONTENT_SETTING_DEFAULT); + } + + void CheckSettingsWouldBlock(const GURL& url) const { + CheckSettings(url, CONTENT_SETTING_BLOCK); + } + + void CheckSettingsWouldAllow(const GURL& url) const { + CheckSettings(url, CONTENT_SETTING_ALLOW); + } + + protected: + virtual void CheckSettings(const GURL& url, ContentSetting setting) const { + for (const auto& url_source : urls_) { + EXPECT_EQ(setting, + TestUtils::GetContentSetting(provider_, url, url_source.first, + ContentSettingsType::PLUGINS, + url_source.second, false)); + } + } + + BravePrefProvider* provider_; + const std::vector urls_; +}; + +class ShieldsCookieSetting : public ShieldsSetting { + public: + explicit ShieldsCookieSetting(BravePrefProvider* provider) + : ShieldsSetting(provider, + {{GURL(), brave_shields::kCookies}, + {GURL("https://firstParty/*"), brave_shields::kCookies}, + {GURL(), brave_shields::kReferrers}}) {} +}; + +class ShieldsFingerprintingSetting : public ShieldsSetting { + public: + explicit ShieldsFingerprintingSetting(BravePrefProvider* provider) + : ShieldsSetting( + provider, + {{GURL(), brave_shields::kFingerprinting}, + {GURL("https://firstParty/*"), brave_shields::kFingerprinting}}) {} +}; + +class ShieldsHTTPSESetting : public ShieldsSetting { + public: + explicit ShieldsHTTPSESetting(BravePrefProvider* provider) + : ShieldsSetting(provider, + {{GURL(), brave_shields::kHTTPUpgradableResources}}) {} +}; + +class ShieldsAdsSetting : public ShieldsSetting { + public: + explicit ShieldsAdsSetting(BravePrefProvider* provider) + : ShieldsSetting(provider, + {{GURL(), brave_shields::kAds}, + {GURL(), brave_shields::kTrackers}}) {} +}; + +class ShieldsEnabledSetting : public ShieldsSetting { + public: + explicit ShieldsEnabledSetting(BravePrefProvider* provider) + : ShieldsSetting(provider, {{GURL(), brave_shields::kBraveShields}}) {} +}; + +class ShieldsUnknownResourceIDSetting : public ShieldsSetting { + public: + explicit ShieldsUnknownResourceIDSetting(BravePrefProvider* provider) + : ShieldsSetting(provider, {{GURL(), "test_resource_id"}}) {} +}; + +class ShieldsScriptSetting : public ShieldsSetting { + public: + explicit ShieldsScriptSetting(BravePrefProvider* provider) + : ShieldsSetting(provider, {}) {} + + void SetPreMigrationSettings(const ContentSettingsPattern& pattern, + ContentSetting setting) override { + provider_->SetWebsiteSetting(pattern, ContentSettingsPattern::Wildcard(), + ContentSettingsType::JAVASCRIPT, "", + ContentSettingToValue(setting)); + } + + private: + void CheckSettings(const GURL& url, ContentSetting setting) const override { + EXPECT_EQ(setting, TestUtils::GetContentSetting( + provider_, url, GURL(), + ContentSettingsType::JAVASCRIPT, "", false)); + } +}; + +} // namespace + +class BravePrefProviderTest : public testing::Test { + public: + BravePrefProviderTest() { + // Ensure all content settings are initialized. + ContentSettingsRegistry::GetInstance(); + } + + private: + content::BrowserTaskEnvironment task_environment_; +}; + +TEST_F(BravePrefProviderTest, TestShieldsSettingsMigration) { + TestingProfile testing_profile; + BravePrefProvider provider(testing_profile.GetPrefs(), false /* incognito */, + true /* store_last_modified */); + + ShieldsCookieSetting cookie_settings(&provider); + ShieldsFingerprintingSetting fp_settings(&provider); + ShieldsHTTPSESetting httpse_settings(&provider); + ShieldsAdsSetting ads_settings(&provider); + ShieldsEnabledSetting enabled_settings(&provider); + ShieldsScriptSetting script_settings(&provider); + ShieldsUnknownResourceIDSetting unknown_resource_id_settings(&provider); + + GURL url("http://brave.com:8080/"); + GURL url2("http://allowed.brave.com:3030"); + // Check that the settings for the url are default values. + cookie_settings.CheckSettingsAreDefault(url); + cookie_settings.CheckSettingsAreDefault(url2); + fp_settings.CheckSettingsAreDefault(url); + httpse_settings.CheckSettingsAreDefault(url); + ads_settings.CheckSettingsAreDefault(url); + enabled_settings.CheckSettingsAreDefault(url); + script_settings.CheckSettingsAreDefault(url); + + // Set pre-migrtion patterns different from defaults. + // ------------------------------------------------------ + ContentSettingsPattern pattern = ContentSettingsPattern::FromURL(url); + ContentSettingsPattern pattern2 = ContentSettingsPattern::FromURL(url2); + // Cookies. + cookie_settings.SetPreMigrationSettings(pattern, CONTENT_SETTING_BLOCK); + cookie_settings.SetPreMigrationSettings(pattern2, CONTENT_SETTING_ALLOW); + // Pattern that doesn't need to be migrated. + cookie_settings.SetPreMigrationSettings( + ContentSettingsPattern::FromString("*://help.brave.com/*"), + CONTENT_SETTING_BLOCK); + // Check that settings would block brave.com:8080, but not brave.com:5555. + cookie_settings.CheckSettingsWouldBlock(url); + cookie_settings.CheckSettingsWouldAllow(url2); + cookie_settings.CheckSettingsAreDefault(GURL("http://brave.com:5555")); + + // Finterprinting. + fp_settings.SetPreMigrationSettings(pattern, CONTENT_SETTING_ALLOW); + // Check that settings would allow brave.com:8080, but not brave.com:5555. + fp_settings.CheckSettingsWouldAllow(url); + fp_settings.CheckSettingsAreDefault(GURL("http://brave.com:5555")); + + // HTTPSE. + httpse_settings.SetPreMigrationSettings(pattern, CONTENT_SETTING_BLOCK); + // Check that settings would block brave.com:8080, but not brave.com:5555. + httpse_settings.CheckSettingsWouldBlock(url); + httpse_settings.CheckSettingsAreDefault(GURL("http://brave.com:5555")); + + // Ads. + ads_settings.SetPreMigrationSettings(pattern, CONTENT_SETTING_ALLOW); + // Check that settings would allow brave.com:8080, but not brave.com:5555. + ads_settings.CheckSettingsWouldAllow(url); + ads_settings.CheckSettingsAreDefault(GURL("http://brave.com:5555")); + + // Enabled. + enabled_settings.SetPreMigrationSettings(pattern, CONTENT_SETTING_BLOCK); + // Check that settings would block brave.com:8080, but not brave.com:5555. + httpse_settings.CheckSettingsWouldBlock(url); + httpse_settings.CheckSettingsAreDefault(GURL("http://brave.com:5555")); + + // Scripts. + script_settings.SetPreMigrationSettings(pattern, CONTENT_SETTING_BLOCK); + // Check that settings would block brave.com:8080, but not brave.com:5555. + script_settings.CheckSettingsWouldBlock(url); + script_settings.CheckSettingsAreDefault(GURL("http://brave.com:5555")); + + // Unknown resource_id. + unknown_resource_id_settings.SetPreMigrationSettings(pattern, + CONTENT_SETTING_BLOCK); + // Check that settings would block brave.com:8080, but not brave.com:5555. + unknown_resource_id_settings.CheckSettingsWouldBlock(url); + unknown_resource_id_settings.CheckSettingsAreDefault( + GURL("http://brave.com:5555")); + + // Migrate settings. + // ------------------------------------------------------ + testing_profile.GetPrefs()->SetInteger(kBraveShieldsSettingsVersion, 1); + provider.MigrateShieldsSettings(/*incognito*/ false); + + // Check post-migration settings. + // ------------------------------------------------------ + // Cookies. + // Check that settings would block brave.com with any protocol and port. + cookie_settings.CheckSettingsWouldBlock(url); + cookie_settings.CheckSettingsWouldBlock(GURL("http://brave.com:5555")); + cookie_settings.CheckSettingsWouldBlock(GURL("https://brave.com")); + // Check that settings would allow allow.brave.com with any protocol and port. + cookie_settings.CheckSettingsWouldAllow(url2); + cookie_settings.CheckSettingsWouldAllow(GURL("https://allowed.brave.com")); + // Check the pattern that didn't need to be migrated. + cookie_settings.CheckSettingsWouldBlock( + GURL("https://help.brave.com/article1.html")); + // Would not block a different domain. + cookie_settings.CheckSettingsAreDefault(GURL("http://brave2.com")); + + // Fingerprinting. + // Check that settings would allow brave.com with any protocol and port. + fp_settings.CheckSettingsWouldAllow(url); + fp_settings.CheckSettingsWouldAllow(GURL("http://brave.com:5555")); + fp_settings.CheckSettingsWouldAllow(GURL("https://brave.com")); + // Would not allow a different domain. + fp_settings.CheckSettingsAreDefault(GURL("http://brave2.com")); + + // HTTPSE. + // Check that settings would block brave.com with any protocol and port. + httpse_settings.CheckSettingsWouldBlock(url); + httpse_settings.CheckSettingsWouldBlock(GURL("http://brave.com:5555")); + // Would not block a different domain. + httpse_settings.CheckSettingsAreDefault(GURL("http://brave2.com")); + + // Ads. + // Check that settings would allow brave.com with any protocol and port. + ads_settings.CheckSettingsWouldAllow(url); + ads_settings.CheckSettingsWouldAllow(GURL("http://brave.com:5555")); + ads_settings.CheckSettingsWouldAllow(GURL("https://brave.com")); + // Would not allow a different domain. + ads_settings.CheckSettingsAreDefault(GURL("http://brave2.com")); + + // Enabled. + // Check that settings would block brave.com with any protocol and port. + httpse_settings.CheckSettingsWouldBlock(url); + httpse_settings.CheckSettingsWouldBlock(GURL("http://brave.com:5555")); + httpse_settings.CheckSettingsWouldBlock(GURL("https://brave.com")); + // Would not block a different domain. + httpse_settings.CheckSettingsAreDefault(GURL("http://brave2.com")); + + // Scripts. + // Check that settings would block brave.com with any protocol and port. + script_settings.CheckSettingsWouldBlock(url); + script_settings.CheckSettingsWouldBlock(GURL("http://brave.com:5555")); + script_settings.CheckSettingsWouldBlock(GURL("https://brave.com")); + // Would not block a different domain. + script_settings.CheckSettingsAreDefault(GURL("http://brave2.com")); + + // Unknown resource_id - should not have been migrated. + // Check that settings would block brave.com:8080, but not brave.com:5555. + unknown_resource_id_settings.CheckSettingsWouldBlock(url); + unknown_resource_id_settings.CheckSettingsAreDefault( + GURL("http://brave.com:5555")); + + provider.ShutdownOnUIThread(); +} + +TEST_F(BravePrefProviderTest, TestShieldsSettingsMigrationVersion) { + TestingProfile testing_profile; + PrefService* prefs = testing_profile.GetPrefs(); + BravePrefProvider provider(prefs, false /* incognito */, + true /* store_last_modified */); + + // Should have migrated when constrcuted (with profile). + EXPECT_EQ(2, prefs->GetInteger(kBraveShieldsSettingsVersion)); + + // Reset and check that migration runs. + prefs->SetInteger(kBraveShieldsSettingsVersion, 1); + provider.MigrateShieldsSettings(/*incognito*/ false); + EXPECT_EQ(2, prefs->GetInteger(kBraveShieldsSettingsVersion)); + + // Test that migration doesn't run for another version. + prefs->SetInteger(kBraveShieldsSettingsVersion, 5); + provider.MigrateShieldsSettings(/*incognito*/ false); + EXPECT_EQ(5, prefs->GetInteger(kBraveShieldsSettingsVersion)); + + provider.ShutdownOnUIThread(); +} + +} // namespace content_settings diff --git a/components/content_settings/core/browser/brave_content_settings_utils.cc b/components/content_settings/core/browser/brave_content_settings_utils.cc index 1cdc24ee5f4..95f096642f2 100644 --- a/components/content_settings/core/browser/brave_content_settings_utils.cc +++ b/components/content_settings/core/browser/brave_content_settings_utils.cc @@ -7,7 +7,9 @@ #include +#include "base/optional.h" #include "brave/components/brave_shields/common/brave_shield_constants.h" +#include "url/gurl.h" namespace { @@ -21,6 +23,32 @@ const std::vector kShieldsResourceIDs { brave_shields::kReferrers, brave_shields::kCookies }; +bool CanPatternBeConvertedToWildcardSchemeAndPort( + const ContentSettingsPattern& pattern) { + // 1. Wildcard is alerady in the desired state. + // 2. Our firstParty placeholder shouldn't be converted. + // 3. Patterns that have file:// scheme. + // 4. We only want to convert patterns that have a specific host, so something + // like "http://*:80/*" should be left alone. + if (pattern == ContentSettingsPattern::Wildcard() || + pattern == ContentSettingsPattern::FromString("https://firstParty/*") || + pattern.GetScheme() == ContentSettingsPattern::SCHEME_FILE || + pattern.MatchesAllHosts()) + return false; + // Check for the case when the scheme is wildcard, but the port isn't. + if (pattern.GetScheme() == ContentSettingsPattern::SCHEME_WILDCARD) { + GURL check_for_port_url("http://" + pattern.ToString()); + return check_for_port_url.has_port(); + } + GURL url(pattern.ToString()); + if (url.is_empty()) + return false; + if (url.has_scheme()) + return !ContentSettingsPattern::IsNonWildcardDomainNonPortScheme( + url.scheme_piece()); + return url.has_port(); +} + } // namespace namespace content_settings { @@ -36,4 +64,13 @@ bool IsShieldsResourceID( resource_identifier) != kShieldsResourceIDs.end(); } +base::Optional ConvertPatternToWildcardSchemeAndPort( + const ContentSettingsPattern& pattern) { + if (!CanPatternBeConvertedToWildcardSchemeAndPort(pattern)) + return base::nullopt; + base::Optional new_pattern = + ContentSettingsPattern::FromString("*://" + pattern.GetHost() + "/*"); + return new_pattern; +} + } // namespace content_settings diff --git a/components/content_settings/core/browser/brave_content_settings_utils.h b/components/content_settings/core/browser/brave_content_settings_utils.h index ab9344887f4..9cd5f0e17e0 100644 --- a/components/content_settings/core/browser/brave_content_settings_utils.h +++ b/components/content_settings/core/browser/brave_content_settings_utils.h @@ -17,6 +17,9 @@ const std::vector& GetShieldsResourceIDs(); bool IsShieldsResourceID(const ResourceIdentifier& resource_identifier); +base::Optional ConvertPatternToWildcardSchemeAndPort( + const ContentSettingsPattern& pattern); + } // namespace content_settings #endif // BRAVE_COMPONENTS_CONTENT_SETTINGS_CORE_BROWSER_BRAVE_CONTENT_SETTINGS_UTILS_H_ diff --git a/components/content_settings/core/browser/brave_content_settings_utils_unittest.cc b/components/content_settings/core/browser/brave_content_settings_utils_unittest.cc new file mode 100644 index 00000000000..90989316d3a --- /dev/null +++ b/components/content_settings/core/browser/brave_content_settings_utils_unittest.cc @@ -0,0 +1,93 @@ +/* Copyright (c) 2020 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include + +#include "base/macros.h" +#include "base/optional.h" +#include "brave/components/content_settings/core/browser/brave_content_settings_utils.h" +#include "chrome/test/base/testing_profile.h" +#include "components/content_settings/core/common/content_settings_pattern.h" +#include "content/public/test/browser_task_environment.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "url/gurl.h" + +using content_settings::ConvertPatternToWildcardSchemeAndPort; + +class BraveContentSettingsUtilsTest : public testing::Test { + public: + BraveContentSettingsUtilsTest() = default; + ~BraveContentSettingsUtilsTest() override = default; + + void SetUp() override { profile_ = std::make_unique(); } + + TestingProfile* profile() { return profile_.get(); } + + private: + content::BrowserTaskEnvironment task_environment_; + std::unique_ptr profile_; + + DISALLOW_COPY_AND_ASSIGN(BraveContentSettingsUtilsTest); +}; + +TEST_F(BraveContentSettingsUtilsTest, + TestConvertPatternToWildcardSchemeAndPort) { + // Full wildcard pattern. + EXPECT_EQ(base::nullopt, ConvertPatternToWildcardSchemeAndPort( + ContentSettingsPattern::Wildcard())); + + // Brave first party placeholder pattern. + EXPECT_EQ(base::nullopt, + ConvertPatternToWildcardSchemeAndPort( + ContentSettingsPattern::FromString("https://firstParty/*"))); + + // file:// scheme pattern. + EXPECT_EQ( + base::nullopt, + ConvertPatternToWildcardSchemeAndPort( + ContentSettingsPattern::FromString("file:///a/b/c.zip"))); + + // Wildcard host pattern. + EXPECT_EQ(base::nullopt, + ConvertPatternToWildcardSchemeAndPort( + ContentSettingsPattern::FromString("http://*:8080/*"))); + + // Wildcard scheme, no port. + EXPECT_EQ(base::nullopt, + ConvertPatternToWildcardSchemeAndPort( + ContentSettingsPattern::FromString("*://brave.com/*"))); + EXPECT_EQ(base::nullopt, + ConvertPatternToWildcardSchemeAndPort( + ContentSettingsPattern::FromString("*://brave.com:*/"))); + + // Wildcard scheme, has port. + auto pattern = ConvertPatternToWildcardSchemeAndPort( + ContentSettingsPattern::FromString("*://brave.com:8080/*")); + EXPECT_NE(base::nullopt, pattern); + EXPECT_EQ(pattern->ToString(), "brave.com"); + EXPECT_TRUE(pattern->Matches(GURL("http://brave.com:80/path1"))); + EXPECT_TRUE(pattern->Matches(GURL("https://brave.com/path2"))); + EXPECT_FALSE(pattern->Matches(GURL("http://brave2.com:8080"))); + pattern.reset(); + + // Scheme, no port. + pattern = ConvertPatternToWildcardSchemeAndPort( + ContentSettingsPattern::FromString("http://brave.com/")); + EXPECT_NE(base::nullopt, pattern); + EXPECT_EQ(pattern->ToString(), "brave.com"); + EXPECT_TRUE(pattern->Matches(GURL("ftp://brave.com:80/path1"))); + EXPECT_TRUE(pattern->Matches(GURL("https://brave.com/path2"))); + EXPECT_FALSE(pattern->Matches(GURL("http://brave2.com:8080"))); + pattern.reset(); + + // Scheme and port. + pattern = ConvertPatternToWildcardSchemeAndPort( + ContentSettingsPattern::FromString("https://brave.com:56558/")); + EXPECT_NE(base::nullopt, pattern); + EXPECT_EQ(pattern->ToString(), "brave.com"); + EXPECT_TRUE(pattern->Matches(GURL("wss://brave.com:80/path1"))); + EXPECT_TRUE(pattern->Matches(GURL("https://brave.com/path2"))); + EXPECT_FALSE(pattern->Matches(GURL("http://brave2.com:8080"))); +} diff --git a/test/BUILD.gn b/test/BUILD.gn index 35d6ba6a497..b819e8f7fae 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -109,6 +109,8 @@ test("brave_unit_tests") { "//brave/components/brave_shields/browser/adblock_stub_response_unittest.cc", "//brave/components/brave_shields/browser/cosmetic_merge_unittest.cc", "//brave/components/brave_shields/browser/https_everywhere_recently_used_cache_unittest.cpp", + "//brave/components/content_settings/core/browser/brave_content_settings_pref_provider_unittest.cc", + "//brave/components/content_settings/core/browser/brave_content_settings_utils_unittest.cc", "//brave/components/ntp_sponsored_images/browser/ntp_sponsored_images_service_unittest.cc", "//brave/components/ntp_sponsored_images/browser/view_counter_model_unittest.cc", "//brave/components/ntp_sponsored_images/browser/view_counter_service_unittest.cc", @@ -130,6 +132,7 @@ test("brave_unit_tests") { "//chrome:browser_dependencies", "//chrome:child_dependencies", "//chrome/test:test_support", + "//components/content_settings/core/test:test_support", "//components/prefs", "//components/prefs:test_support", "//components/version_info",