Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix third-party cookies settings page. (uplift to 1.62.x) #21766

Merged
merged 1 commit into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions browser/extensions/api/settings_private/brave_prefs_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -332,4 +339,21 @@ const PrefsUtil::TypedPrefMap& BravePrefsUtil::GetAllowlistedKeys() {
return *s_brave_allowlist;
}

absl::optional<api::settings_private::PrefObject> 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
14 changes: 14 additions & 0 deletions browser/extensions/api/settings_private/brave_prefs_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <string>

#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 {
Expand All @@ -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<api::settings_private::PrefObject> GetPref(
const std::string& name) override;
};

} // namespace extensions
Expand Down
10 changes: 10 additions & 0 deletions browser/ui/webui/settings/default_brave_shields_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 12 additions & 2 deletions browser/ui/webui/settings/default_brave_shields_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -56,6 +63,9 @@ class DefaultBraveShieldsHandler : public settings::SettingsPageUIHandler,

base::ScopedObservation<HostContentSettingsMap, content_settings::Observer>
content_settings_observation_{this};
base::ScopedObservation<content_settings::CookieSettings,
content_settings::CookieSettings::Observer>
cookie_settings_observation_{this};
};

#endif // BRAVE_BROWSER_UI_WEBUI_SETTINGS_DEFAULT_BRAVE_SHIELDS_HANDLER_H_
18 changes: 18 additions & 0 deletions components/brave_shields/browser/brave_shields_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<int>(content_settings::CookieControlsMode::kOff));
profile_state->SetInteger(
::prefs::kCookieControlsMode,
static_cast<int>(
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<int>(content_settings::CookieControlsMode::kOff));
profile_state->SetInteger(
::prefs::kCookieControlsMode,
static_cast<int>(
Expand Down
15 changes: 15 additions & 0 deletions components/brave_shields/browser/brave_shields_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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<int>(content_settings::CookieControlsMode::kOff));
EXPECT_FALSE(cookies->ShouldBlockThirdPartyCookies());
// setting should apply to all urls
setting = map->GetContentSetting(GURL("http://brave.com"), GURL(),
Expand Down
Loading