Skip to content

Commit

Permalink
use domain registry to get the correct parent domain for 3rd-party co…
Browse files Browse the repository at this point in the history 
…okies

fix brave/brave-browser#9489
  • Loading branch information
@bridiver
bridiver committed Apr 28, 2020
1 parent 8e96e22 commit f7449a0
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 45 deletions.
80 changes: 69 additions & 11 deletions browser/net/brave_network_delegate_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,18 @@ class BraveNetworkDelegateBrowserTest : public InProcessBrowserTest {

third_party_cookie_url_ =
embedded_test_server()->GetURL("b.com", "/set-cookie?name=Good");
first_party_cookie_url_ =
embedded_test_server()->GetURL("a.com",
"/set-cookie?name=Good");
subdomain_first_party_cookie_url_ =
embedded_test_server()->GetURL("subdomain.a.com",
"/set-cookie?name=Good");

domain_registry_url_ = embedded_test_server()->GetURL("mobile.twitter.com",
"/cookie_iframe.html");
iframe_domain_registry_url_ = embedded_test_server()->GetURL("blah.twitter.com",
"/set-cookie?name=Good;domain=twitter.com");

google_oauth_cookie_url_ =
https_server_.GetURL("accounts.google.com", "/set-cookie?oauth=true");

Expand Down Expand Up @@ -139,7 +148,7 @@ class BraveNetworkDelegateBrowserTest : public InProcessBrowserTest {
void ExpectCookiesOnHost(const GURL url,
const std::string& expected) {
EXPECT_EQ(expected, content::GetCookies(browser()->profile(),
url));
url)) << url.spec();
}

void NavigateFrameTo(const GURL url) {
Expand All @@ -161,7 +170,10 @@ class BraveNetworkDelegateBrowserTest : public InProcessBrowserTest {
GURL cookie_iframe_url_;
GURL https_cookie_iframe_url_;
GURL third_party_cookie_url_;
GURL first_party_cookie_url_;
GURL subdomain_first_party_cookie_url_;
GURL domain_registry_url_;
GURL iframe_domain_registry_url_;
GURL google_oauth_cookie_url_;
GURL wordpress_top_url_;
GURL wordpress_frame_url_;
Expand Down Expand Up @@ -260,11 +272,6 @@ IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest,

ExpectCookiesOnHost(top_level_page_url_, "name=Good");
ExpectCookiesOnHost(third_party_cookie_url_, "");

NavigateFrameTo(subdomain_first_party_cookie_url_);

ExpectCookiesOnHost(top_level_page_url_, "name=Good");
ExpectCookiesOnHost(subdomain_first_party_cookie_url_, "name=Good");
}

IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest,
Expand Down Expand Up @@ -337,11 +344,6 @@ IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest,

ExpectCookiesOnHost(top_level_page_url_, "name=Good");
ExpectCookiesOnHost(GURL("http://b.com"), "");

NavigateFrameTo(subdomain_first_party_cookie_url_);

ExpectCookiesOnHost(top_level_page_url_, "name=Good");
ExpectCookiesOnHost(subdomain_first_party_cookie_url_, "name=Good");
}

IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest,
Expand Down Expand Up @@ -482,6 +484,62 @@ IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest,
ExpectCookiesOnHost(GURL("https://accounts.google.com"), "");
}

IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest,
ShieldsToggleBlockThirdPartyWithDefaultAllowAll) {
DefaultAllowAllCookies();

BlockThirdPartyCookies(cookie_iframe_url_);
NavigateToPageWithFrame(cookie_iframe_url_);
NavigateFrameTo(third_party_cookie_url_);

ExpectCookiesOnHost(cookie_iframe_url_, "name=Good");
ExpectCookiesOnHost(third_party_cookie_url_, "");

NavigateFrameTo(first_party_cookie_url_);
ExpectCookiesOnHost(cookie_iframe_url_, "name=Good");
ExpectCookiesOnHost(first_party_cookie_url_, "name=Good");
}

IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest,
ShieldsToggleBlockThirdPartyWithDefaultBlockAll) {
DefaultBlockAllCookies();

BlockThirdPartyCookies(cookie_iframe_url_);
NavigateToPageWithFrame(cookie_iframe_url_);
NavigateFrameTo(third_party_cookie_url_);

ExpectCookiesOnHost(cookie_iframe_url_, "name=Good");
ExpectCookiesOnHost(third_party_cookie_url_, "");

NavigateFrameTo(first_party_cookie_url_);
ExpectCookiesOnHost(cookie_iframe_url_, "name=Good");
ExpectCookiesOnHost(first_party_cookie_url_, "name=Good");
}

IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest,
ShieldsToggleBlockThirdPartyAllowSubdomain) {
DefaultBlockAllCookies();

BlockThirdPartyCookies(cookie_iframe_url_);
NavigateToPageWithFrame(cookie_iframe_url_);
NavigateFrameTo(subdomain_first_party_cookie_url_);

ExpectCookiesOnHost(top_level_page_url_, "name=Good");
ExpectCookiesOnHost(subdomain_first_party_cookie_url_, "name=Good");
}

IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest,
ShieldsToggleBlockThirdPartyAllowDomainRegistry) {
DefaultBlockAllCookies();

BlockThirdPartyCookies(domain_registry_url_);
NavigateToPageWithFrame(domain_registry_url_);
NavigateFrameTo(iframe_domain_registry_url_);

ExpectCookiesOnHost(domain_registry_url_, "name=Good");
ExpectCookiesOnHost(iframe_domain_registry_url_, "name=Good");
}

// Test to ensure that we treat wp.com and wordpress.com as equal first parties
// for the purposes of ability to set / send storage.
// The following tests check each of the following.
Expand Down
51 changes: 17 additions & 34 deletions components/content_settings/core/browser/brave_content_settings_pref_provider.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,30 +22,34 @@
#include "components/prefs/pref_service.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"

namespace content_settings {

namespace {

Rule CloneRule(const Rule& rule, bool reverse_patterns = false) {
auto secondary_pattern = rule.secondary_pattern;
if (secondary_pattern ==
// brave plugin rules incorrectly use first party url as primary
auto primary_pattern = reverse_patterns ? rule.secondary_pattern
: rule.primary_pattern;
auto secondary_pattern = reverse_patterns ? rule.primary_pattern
: rule.secondary_pattern;

if (primary_pattern ==
ContentSettingsPattern::FromString("https://firstParty/*")) {
if (!rule.primary_pattern.MatchesAllHosts()) {
secondary_pattern = ContentSettingsPattern::FromString(
"*://[*.]" + rule.primary_pattern.GetHost() + "/*");
DCHECK(reverse_patterns); // we should only hit this for brave plugin rules
if (!secondary_pattern.MatchesAllHosts()) {
primary_pattern = ContentSettingsPattern::FromString(
"*://[*.]" +
net::registry_controlled_domains::GetDomainAndRegistry(
secondary_pattern.GetHost(),
net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES) + "/*");
} else {
secondary_pattern = rule.primary_pattern;
primary_pattern = secondary_pattern;
}
}

// brave plugin rules incorrectly use the embedded url as the primary
if (reverse_patterns)
return Rule(secondary_pattern,
rule.primary_pattern,
rule.value.Clone());

return Rule(rule.primary_pattern,
return Rule(primary_pattern,
secondary_pattern,
rule.value.Clone());
}
Expand Down Expand Up @@ -343,27 +347,6 @@ void BravePrefProvider::UpdateCookieRules(ContentSettingsType content_type,
rules.push_back(CloneRule(wp_com_rule));
brave_cookie_rules_[incognito].push_back(CloneRule(wp_com_rule));

// Add ability for google properties to send cookies to each other,
// especially needed for google drive playback.
//
// Partial fix for https://github.com/brave/brave-browser/issues/1122
const auto googleapis_com_pattern = ContentSettingsPattern::FromString(
"https://[*.]googleapis.com/*");
const auto google_com_pattern = ContentSettingsPattern::FromString(
"https://[*.]google.com/*");
const std::vector<ContentSettingsPattern> patterns = {
googleapis_com_pattern, google_com_pattern};
for (const auto &outer : patterns) {
for (const auto &inner : patterns) {
const auto a_rule = Rule(
outer,
inner,
ContentSettingToValue(CONTENT_SETTING_ALLOW)->Clone());
rules.push_back(CloneRule(a_rule));
brave_cookie_rules_[incognito].push_back(CloneRule(a_rule));
}
}

// add chromium cookies
auto chromium_cookies_iterator = PrefProvider::GetRuleIterator(
ContentSettingsType::COOKIES,
Expand Down

0 comments on commit f7449a0

Please sign in to comment.