From a6536c2e0c34585beae14ee87e71be514ec7790f Mon Sep 17 00:00:00 2001 From: brave-builds <45370463+brave-builds@users.noreply.github.com> Date: Tue, 30 Jan 2024 21:26:34 +0100 Subject: [PATCH] Fix third-party cookies settings page. (uplift to 1.62.x) (#21766) Uplift of #21745 (squashed) to release --- .../api/settings_private/brave_prefs_util.cc | 24 +++++++++++++++++++ .../api/settings_private/brave_prefs_util.h | 14 +++++++++++ .../settings/default_brave_shields_handler.cc | 10 ++++++++ .../settings/default_brave_shields_handler.h | 14 +++++++++-- .../browser/brave_shields_util.cc | 18 ++++++++++++++ .../browser/brave_shields_util_unittest.cc | 15 ++++++++++++ 6 files changed, 93 insertions(+), 2 deletions(-) diff --git a/browser/extensions/api/settings_private/brave_prefs_util.cc b/browser/extensions/api/settings_private/brave_prefs_util.cc index 82016a10c0f8..f7811eb6afd4 100644 --- a/browser/extensions/api/settings_private/brave_prefs_util.cc +++ b/browser/extensions/api/settings_private/brave_prefs_util.cc @@ -12,6 +12,7 @@ #include "brave/components/brave_ads/core/public/prefs/pref_names.h" #include "brave/components/brave_news/common/pref_names.h" #include "brave/components/brave_rewards/common/pref_names.h" +#include "brave/components/brave_shields/browser/brave_shields_util.h" #include "brave/components/brave_shields/common/pref_names.h" #include "brave/components/brave_vpn/common/buildflags/buildflags.h" #include "brave/components/brave_wallet/browser/pref_names.h" @@ -27,14 +28,20 @@ #include "brave/components/request_otr/common/pref_names.h" #include "brave/components/speedreader/common/buildflags/buildflags.h" #include "brave/components/tor/buildflags/buildflags.h" +#include "chrome/browser/content_settings/cookie_settings_factory.h" +#include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/extensions/api/settings_private/prefs_util.h" +#include "chrome/browser/profiles/profile.h" #include "chrome/common/extensions/api/settings_private.h" #include "chrome/common/pref_names.h" #include "components/browsing_data/core/pref_names.h" +#include "components/content_settings/core/browser/cookie_settings.h" +#include "components/content_settings/core/common/pref_names.h" #include "components/gcm_driver/gcm_buildflags.h" #include "components/omnibox/browser/omnibox_prefs.h" #include "components/search_engines/search_engines_pref_names.h" #include "extensions/buildflags/buildflags.h" +#include "url/gurl.h" #if BUILDFLAG(ENABLE_BRAVE_WAYBACK_MACHINE) #include "brave/components/brave_wayback_machine/pref_names.h" @@ -332,4 +339,21 @@ const PrefsUtil::TypedPrefMap& BravePrefsUtil::GetAllowlistedKeys() { return *s_brave_allowlist; } +absl::optional BravePrefsUtil::GetPref( + const std::string& name) { + auto pref = PrefsUtil::GetPref(name); + // Simulate "Enforced" mode for kCookieControlsMode pref when Cookies are + // fully blocked via Shields. This will effectively disable the "Third-party + // cookies" mode selector on Settings page. + if (pref && name == prefs::kCookieControlsMode && + pref->enforcement == api::settings_private::Enforcement::kNone && + brave_shields::GetCookieControlType( + HostContentSettingsMapFactory::GetForProfile(profile()), + CookieSettingsFactory::GetForProfile(profile()).get(), + GURL()) == brave_shields::ControlType::BLOCK) { + pref->enforcement = api::settings_private::Enforcement::kEnforced; + } + return pref; +} + } // namespace extensions diff --git a/browser/extensions/api/settings_private/brave_prefs_util.h b/browser/extensions/api/settings_private/brave_prefs_util.h index 43319d27bbda..379f0a8ea78f 100644 --- a/browser/extensions/api/settings_private/brave_prefs_util.h +++ b/browser/extensions/api/settings_private/brave_prefs_util.h @@ -6,8 +6,19 @@ #ifndef BRAVE_BROWSER_EXTENSIONS_API_SETTINGS_PRIVATE_BRAVE_PREFS_UTIL_H_ #define BRAVE_BROWSER_EXTENSIONS_API_SETTINGS_PRIVATE_BRAVE_PREFS_UTIL_H_ +#include + +#define IsPrefTypeURL(...) \ + IsPrefTypeURL_NotUsed(); \ + Profile* profile() const { \ + return profile_.get(); \ + } \ + bool IsPrefTypeURL(__VA_ARGS__) + #include "chrome/browser/extensions/api/settings_private/prefs_util.h" +#undef IsPrefTypeURL + namespace extensions { class BravePrefsUtil : public PrefsUtil { @@ -17,6 +28,9 @@ class BravePrefsUtil : public PrefsUtil { // to prefs that clients of the settingsPrivate API may retrieve and // manipulate. const PrefsUtil::TypedPrefMap& GetAllowlistedKeys() override; + + absl::optional GetPref( + const std::string& name) override; }; } // namespace extensions diff --git a/browser/ui/webui/settings/default_brave_shields_handler.cc b/browser/ui/webui/settings/default_brave_shields_handler.cc index 0456be89f59c..2e6833b2cc55 100644 --- a/browser/ui/webui/settings/default_brave_shields_handler.cc +++ b/browser/ui/webui/settings/default_brave_shields_handler.cc @@ -101,6 +101,8 @@ void DefaultBraveShieldsHandler::RegisterMessages() { content_settings_observation_.Observe( HostContentSettingsMapFactory::GetForProfile(profile_)); + cookie_settings_observation_.Observe( + CookieSettingsFactory::GetForProfile(profile_).get()); } void DefaultBraveShieldsHandler::OnContentSettingChanged( @@ -133,6 +135,14 @@ void DefaultBraveShieldsHandler::OnContentSettingChanged( FireWebUIListener("brave-shields-settings-changed"); } +void DefaultBraveShieldsHandler::OnThirdPartyCookieBlockingChanged( + bool block_third_party_cookies) { + if (!IsJavascriptAllowed()) { + return; + } + FireWebUIListener("brave-shields-settings-changed"); +} + void DefaultBraveShieldsHandler::IsAdControlEnabled( const base::Value::List& args) { CHECK_EQ(args.size(), 1U); diff --git a/browser/ui/webui/settings/default_brave_shields_handler.h b/browser/ui/webui/settings/default_brave_shields_handler.h index d4894cb84a5f..b58d64a7a944 100644 --- a/browser/ui/webui/settings/default_brave_shields_handler.h +++ b/browser/ui/webui/settings/default_brave_shields_handler.h @@ -10,12 +10,15 @@ #include "base/scoped_observation.h" #include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h" #include "components/content_settings/core/browser/content_settings_observer.h" +#include "components/content_settings/core/browser/cookie_settings.h" #include "components/content_settings/core/browser/host_content_settings_map.h" class Profile; -class DefaultBraveShieldsHandler : public settings::SettingsPageUIHandler, - public content_settings::Observer { +class DefaultBraveShieldsHandler + : public settings::SettingsPageUIHandler, + public content_settings::Observer, + public content_settings::CookieSettings::Observer { public: DefaultBraveShieldsHandler(); DefaultBraveShieldsHandler(const DefaultBraveShieldsHandler&) = delete; @@ -35,6 +38,10 @@ class DefaultBraveShieldsHandler : public settings::SettingsPageUIHandler, const ContentSettingsPattern& secondary_pattern, ContentSettingsTypeSet content_type_set) override; + // content_settings::CookieSettings::Observer overrides: + void OnThirdPartyCookieBlockingChanged( + bool block_third_party_cookies) override; + void SetAdControlType(const base::Value::List& args); void IsAdControlEnabled(const base::Value::List& args); void SetCosmeticFilteringControlType(const base::Value::List& args); @@ -56,6 +63,9 @@ class DefaultBraveShieldsHandler : public settings::SettingsPageUIHandler, base::ScopedObservation content_settings_observation_{this}; + base::ScopedObservation + cookie_settings_observation_{this}; }; #endif // BRAVE_BROWSER_UI_WEBUI_SETTINGS_DEFAULT_BRAVE_SHIELDS_HANDLER_H_ diff --git a/components/brave_shields/browser/brave_shields_util.cc b/components/brave_shields/browser/brave_shields_util.cc index 83016b83b222..1a5b7a457e1d 100644 --- a/components/brave_shields/browser/brave_shields_util.cc +++ b/components/brave_shields/browser/brave_shields_util.cc @@ -443,10 +443,28 @@ void SetCookieControlType(HostContentSettingsMap* map, case ControlType::BLOCK: map->SetDefaultContentSetting(ContentSettingsType::COOKIES, CONTENT_SETTING_BLOCK); + // Toggle the state off->on to enforce the pref update event on switch + // between BLOCK_THIRD_PARTY->BLOCK so the upstream Third-party cookies + // Settings page can be updated correctly. This is a temporary measure + // until we figure out a better UI for Cookies Settings page. + profile_state->SetInteger( + ::prefs::kCookieControlsMode, + static_cast(content_settings::CookieControlsMode::kOff)); + profile_state->SetInteger( + ::prefs::kCookieControlsMode, + static_cast( + content_settings::CookieControlsMode::kBlockThirdParty)); break; case ControlType::BLOCK_THIRD_PARTY: map->SetDefaultContentSetting(ContentSettingsType::COOKIES, CONTENT_SETTING_ALLOW); + // Toggle the state off->on to enforce the pref update event on switch + // between BLOCK->BLOCK_THIRD_PARTY so the upstream Third-party cookies + // Settings page can be updated correctly. This is a temporary measure + // until we figure out a better UI for Cookies Settings page. + profile_state->SetInteger( + ::prefs::kCookieControlsMode, + static_cast(content_settings::CookieControlsMode::kOff)); profile_state->SetInteger( ::prefs::kCookieControlsMode, static_cast( diff --git a/components/brave_shields/browser/brave_shields_util_unittest.cc b/components/brave_shields/browser/brave_shields_util_unittest.cc index 648fb05662fe..fc023e17d091 100644 --- a/components/brave_shields/browser/brave_shields_util_unittest.cc +++ b/components/brave_shields/browser/brave_shields_util_unittest.cc @@ -456,6 +456,7 @@ TEST_F(BraveShieldsUtilTest, SetCookieControlType_Default) { setting = map->GetContentSetting(GURL("http://brave.com"), GURL(), ContentSettingsType::COOKIES); EXPECT_EQ(CONTENT_SETTING_ALLOW, setting); + EXPECT_FALSE(cookies->ShouldBlockThirdPartyCookies()); setting = map->GetContentSetting(GURL("http://brave.com"), GURL("http://brave.com"), ContentSettingsType::COOKIES); @@ -470,6 +471,20 @@ TEST_F(BraveShieldsUtilTest, SetCookieControlType_Default) { setting = map->GetContentSetting(GURL(), GURL("https://firstParty"), ContentSettingsType::COOKIES); EXPECT_EQ(CONTENT_SETTING_BLOCK, setting); + EXPECT_TRUE(cookies->ShouldBlockThirdPartyCookies()); + // setting should apply to all urls + setting = map->GetContentSetting(GURL("http://brave.com"), GURL(), + ContentSettingsType::COOKIES); + EXPECT_EQ(CONTENT_SETTING_BLOCK, setting); + setting = map->GetContentSetting(GURL("http://brave.com"), + GURL("https://firstParty"), + ContentSettingsType::COOKIES); + EXPECT_EQ(CONTENT_SETTING_BLOCK, setting); + + // Ensure BLOCK with kCookieControlsMode == kOff still blocks all cookies. + profile()->GetPrefs()->SetInteger( + ::prefs::kCookieControlsMode, + static_cast(content_settings::CookieControlsMode::kOff)); EXPECT_FALSE(cookies->ShouldBlockThirdPartyCookies()); // setting should apply to all urls setting = map->GetContentSetting(GURL("http://brave.com"), GURL(),