[rSAFor] Require CORS for TopLevelStorageAccess grants eligibility
TopLevelStorageAccess grants should only be considered when the
request mode is 'cors'. To perform this check, add enum CookieSettingOverride::kTopLevelStorageAccessGrantEligible to
url_request.cookie_setting_overrides_ when the request mode is cors,
and only allow cross-site cookie granted by TopLevelStorageAccess
if this require enum exists.
Bug: 1401091
Change-Id: I41a74f530882e38c8051a53013e8a3bf149f1b02
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4144296
Reviewed-by: Maks Orlovich <[email protected]>
Reviewed-by: Chris Fredrickson <[email protected]>
Commit-Queue: Shuran Huang <[email protected]>
Reviewed-by: Takashi Toyoshima <[email protected]>
Reviewed-by: Balazs Engedy <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1096976}
diff --git a/chrome/browser/net/storage_test_utils.cc b/chrome/browser/net/storage_test_utils.cc
index 6561496..c36ddf9 100644
--- a/chrome/browser/net/storage_test_utils.cc
+++ b/chrome/browser/net/storage_test_utils.cc
@@ -170,4 +170,16 @@
.ExtractBool();
}
+std::string FetchWithCredentials(content::RenderFrameHost* frame,
+ const GURL& url,
+ const bool cors_enabled) {
+ constexpr char script[] = R"(
+ fetch($1, {mode: $2, credentials: 'include'})
+ .then((result) => result.text());
+ )";
+ const std::string mode = cors_enabled ? "cors" : "no-cors";
+ return content::EvalJs(frame, content::JsReplace(script, url, mode))
+ .ExtractString();
+}
+
} // namespace storage::test
diff --git a/chrome/browser/net/storage_test_utils.h b/chrome/browser/net/storage_test_utils.h
index b54023e..94b462e 100644
--- a/chrome/browser/net/storage_test_utils.h
+++ b/chrome/browser/net/storage_test_utils.h
@@ -7,6 +7,8 @@
#include <string>
+class GURL;
+
namespace content {
class RenderFrameHost;
} // namespace content
@@ -45,6 +47,13 @@
// value of true; false otherwise.
bool HasStorageAccessForFrame(content::RenderFrameHost* frame);
+// Helper to see if a credentialed fetch has cookies access via top-level
+// storage access grants. Returns the content of the response if the promise
+// resolves. `cors_enabled` sets fetch RequestMode to be "cors" or "no-cors".
+std::string FetchWithCredentials(content::RenderFrameHost* frame,
+ const GURL& url,
+ const bool cors_enabled);
+
} // namespace test
} // namespace storage
#endif // CHROME_BROWSER_NET_STORAGE_TEST_UTILS_H_
diff --git a/chrome/browser/top_level_storage_access_api/request_storage_access_for_origin_browsertest.cc b/chrome/browser/top_level_storage_access_api/request_storage_access_for_origin_browsertest.cc
index 8966346..973160b 100644
--- a/chrome/browser/top_level_storage_access_api/request_storage_access_for_origin_browsertest.cc
+++ b/chrome/browser/top_level_storage_access_api/request_storage_access_for_origin_browsertest.cc
@@ -188,6 +188,13 @@
return ChildFrameAt(GetFrame(), 0);
}
+ std::string CookiesFromFetchWithCredentials(content::RenderFrameHost* frame,
+ const std::string& host,
+ const bool cors_enabled) {
+ return storage::test::FetchWithCredentials(
+ frame, https_server_.GetURL(host, "/echoheader?cookie"), cors_enabled);
+ }
+
net::test_server::EmbeddedTestServer& https_server() { return https_server_; }
private:
@@ -204,14 +211,12 @@
SetBlockThirdPartyCookies(true);
// Set a cookie on `kHostB` and `kHostC`.
- content::SetCookie(browser()->profile(), GetURL(kHostB),
- "thirdparty=b;SameSite=None;Secure");
+ SetCrossSiteCookieOnHost(kHostB);
ASSERT_EQ(content::GetCookies(browser()->profile(), GetURL(kHostB)),
- "thirdparty=b");
- content::SetCookie(browser()->profile(), GetURL(kHostC),
- "thirdparty=c;SameSite=None;Secure");
+ "cross-site=b.test");
+ SetCrossSiteCookieOnHost(kHostC);
ASSERT_EQ(content::GetCookies(browser()->profile(), GetURL(kHostC)),
- "thirdparty=c");
+ "cross-site=c.test");
NavigateToPageWithFrame(kHostA);
NavigateFrameTo(kHostB, "/iframe.html");
@@ -253,15 +258,23 @@
->GetCookieManagerForBrowserProcess()
->SetTopLevelStorageAccessSettings(settings, base::DoNothing());
+ // document.hasStorageAccess() does not have cookie access with top-level
+ // storage access grant.
EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
- EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetNestedFrame()));
+ EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetNestedFrame()));
NavigateFrameTo(kHostB, "/iframe.html");
NavigateNestedFrameTo(kHostC, "/echoheader?cookie");
EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
- EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetNestedFrame()));
- EXPECT_EQ(GetNestedFrameContent(), "thirdparty=c");
- EXPECT_EQ(ReadCookiesViaJS(GetNestedFrame()), "thirdparty=c");
+ EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetNestedFrame()));
+ EXPECT_EQ(GetNestedFrameContent(), "None");
+ EXPECT_EQ(ReadCookiesViaJS(GetNestedFrame()), "");
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/true),
+ "None");
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetNestedFrame(), kHostC,
+ /*cors_enabled=*/true),
+ "cross-site=c.test");
}
IN_PROC_BROWSER_TEST_F(RequestStorageAccessForOriginBrowserTest,
@@ -336,6 +349,9 @@
EXPECT_EQ(GetFrameContent(), "None");
EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostASubdomain,
+ /*cors_enabled=*/true),
+ "None");
}
// Tests to validate First-Party Set use with `requestStorageAccessForOrigin`.
@@ -384,23 +400,29 @@
// the requestor, because it is a service domain.
EXPECT_TRUE(storage::test::RequestStorageAccessForOrigin(
GetPrimaryMainFrame(), GetURL(kHostB).spec()));
- EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/true),
+ "cross-site=b.test");
// Repeated calls should also return true.
EXPECT_TRUE(storage::test::RequestStorageAccessForOrigin(
GetPrimaryMainFrame(), GetURL(kHostB).spec()));
// Navigate iframe to a cross-site, cookie-reading endpoint, and verify that
- // the cookie is sent.
+ // the cookie is sent for the cors-enabled subresource request.
NavigateFrameTo(kHostB, "/echoheader?cookie");
- EXPECT_EQ(GetFrameContent(), "cross-site=b.test");
- EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "cross-site=b.test");
- EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
-
- // Also validate that an additional site C was not granted access.
- NavigateFrameTo(kHostC, "/echoheader?cookie");
EXPECT_EQ(GetFrameContent(), "None");
EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/true),
+ "cross-site=b.test");
+
+ // Also validate that an additional site C was not granted access.
+ NavigateFrameTo(kHostC, "/echoheader?cookie");
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostC,
+ /*cors_enabled=*/true),
+ "None");
content::FetchHistogramsFromChildProcesses();
@@ -426,6 +448,9 @@
EXPECT_EQ(GetFrameContent(), "None");
EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostA,
+ /*cors_enabled=*/true),
+ "None");
// The promise should be rejected; `khostB` is a service domain.
EXPECT_FALSE(storage::test::RequestStorageAccessForOrigin(
GetPrimaryMainFrame(), GetURL(kHostA).spec()));
@@ -437,6 +462,9 @@
EXPECT_EQ(GetFrameContent(), "None");
EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostA,
+ /*cors_enabled=*/true),
+ "None");
content::FetchHistogramsFromChildProcesses();
EXPECT_THAT(histogram_tester.GetBucketCount(
@@ -461,12 +489,18 @@
EXPECT_EQ(GetFrameContent(), "None");
EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/true),
+ "None");
// `kHostB` cannot be granted access via `RequestStorageAccessForOrigin`,
// because the call is not from the top-level page and because `kHostB` is a
// service domain.
EXPECT_FALSE(storage::test::RequestStorageAccessForOrigin(
GetFrame(), GetURL(kHostA).spec()));
EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/true),
+ "None");
// Navigate iframe to a cross-site, cookie-reading endpoint, and verify that
// the cookie is not sent.
@@ -474,6 +508,9 @@
EXPECT_EQ(GetFrameContent(), "None");
EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/true),
+ "None");
// However, a regular `requestStorageAccess` call should be granted;
// requesting on behalf of another domain is what is not acceptable.
@@ -511,6 +548,9 @@
EXPECT_FALSE(storage::test::RequestStorageAccessForOrigin(
GetPrimaryMainFrame(), GetURL(kHostD).spec()));
EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostD,
+ /*cors_enabled=*/true),
+ "None");
// Navigate iframe to a cross-site, cookie-reading endpoint, and verify that
// the cookie is not sent.
@@ -518,6 +558,9 @@
EXPECT_EQ(GetFrameContent(), "None");
EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostD,
+ /*cors_enabled=*/true),
+ "None");
content::FetchHistogramsFromChildProcesses();
EXPECT_THAT(histogram_tester.GetBucketCount(
@@ -542,24 +585,37 @@
EXPECT_EQ(GetFrameContent(), "None");
EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/true),
+ "None");
EXPECT_TRUE(storage::test::RequestStorageAccessForOrigin(
GetPrimaryMainFrame(), GetURL(kHostB).spec()));
- EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
// Navigate iframe to a cross-site, cookie-reading endpoint, and verify that
- // the cookie is sent:
+ // the cookie is sent for the cors-enabled subresource request.
NavigateFrameTo(kHostB, "/echoheader?cookie");
- EXPECT_EQ(GetFrameContent(), "cross-site=b.test");
- EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "cross-site=b.test");
- EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(GetFrameContent(), "None");
+ EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
+ EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/true),
+ "cross-site=b.test");
+ // Subresource request with cors disabled does not have cookie access.
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/false),
+ "None");
NavigateToPageWithFrame(kHostASubdomain);
NavigateFrameTo(kHostB, "/echoheader?cookie");
// Storage access grants are scoped to the embedded origin on the top-level
- // site. Accordingly, the access should be granted.
- EXPECT_EQ(GetFrameContent(), "cross-site=b.test");
- EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "cross-site=b.test");
- EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ // site. Accordingly, the access is be granted for subresource request.
+ EXPECT_EQ(GetFrameContent(), "None");
+ EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
+ EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/true),
+ "cross-site=b.test");
}
IN_PROC_BROWSER_TEST_F(
@@ -578,24 +634,37 @@
EXPECT_EQ(GetFrameContent(), "None");
EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/true),
+ "None");
EXPECT_TRUE(storage::test::RequestStorageAccessForOrigin(
GetPrimaryMainFrame(), GetURL(kHostB).spec()));
- EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
// Navigate iframe to a cross-site, cookie-reading endpoint, and verify that
- // the cookie is sent:
+ // the cookie is sent for the cors-enabled subresource request.
NavigateFrameTo(kHostB, "/echoheader?cookie");
- EXPECT_EQ(GetFrameContent(), "cross-site=b.test");
- EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "cross-site=b.test");
- EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(GetFrameContent(), "None");
+ EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
+ EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/true),
+ "cross-site=b.test");
+ // Subresource request with cors disabled does not have cookie access.
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/false),
+ "None");
NavigateToPageWithFrame(kHostA);
NavigateFrameTo(kHostB, "/echoheader?cookie");
// When top-level site scoping is enabled, the subdomain's grant counts for
// the less-specific domain; otherwise, it does not.
- EXPECT_EQ(GetFrameContent(), "cross-site=b.test");
- EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "cross-site=b.test");
- EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(GetFrameContent(), "None");
+ EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "");
+ EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/true),
+ "cross-site=b.test");
}
// Tests to validate `requestStorageAccessForOrigin` behavior with FPS disabled.
@@ -737,22 +806,22 @@
EXPECT_EQ(GetFrameContent(), "cross-site=b.test(partitioned)");
EXPECT_EQ(ReadCookiesViaJS(GetFrame()), "cross-site=b.test(partitioned)");
EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/true),
+ "cross-site=b.test(partitioned)");
// kHostA can request storage access on behalf of kHostB, and it is granted
// (by an implicit grant):
EXPECT_TRUE(storage::test::RequestStorageAccessForOrigin(
GetPrimaryMainFrame(), GetURL(kHostB).spec()));
- EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
-
- // When the frame subsequently navigates to an endpoint on kHostB, kHostB's
- // unpartitioned and partitioned cookies are sent, and the iframe retains
- // storage access.
+ // When the frame makes a subresource request to an endpoint on kHostB,
+ // kHostB's unpartitioned and partitioned cookies are sent, and the iframe
+ // retains storage access.
NavigateFrameTo(kHostB, "/echoheader?cookie");
- EXPECT_EQ(GetFrameContent(),
+ EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+ EXPECT_EQ(CookiesFromFetchWithCredentials(GetFrame(), kHostB,
+ /*cors_enabled=*/true),
"cross-site=b.test; cross-site=b.test(partitioned)");
- EXPECT_EQ(ReadCookiesViaJS(GetFrame()),
- "cross-site=b.test; cross-site=b.test(partitioned)");
- EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
}
} // namespace
diff --git a/components/content_settings/core/browser/cookie_settings.cc b/components/content_settings/core/browser/cookie_settings.cc
index ffc05b6..9bb7496 100644
--- a/components/content_settings/core/browser/cookie_settings.cc
+++ b/components/content_settings/core/browser/cookie_settings.cc
@@ -222,7 +222,10 @@
}
}
- if (block && ShouldConsiderTopLevelStorageAccessGrants(query_reason)) {
+ if (block &&
+ overrides.Has(
+ net::CookieSettingOverride::kTopLevelStorageAccessGrantEligible) &&
+ ShouldConsiderTopLevelStorageAccessGrants(query_reason)) {
ContentSetting host_setting = host_content_settings_map_->GetContentSetting(
url, first_party_url, ContentSettingsType::TOP_LEVEL_STORAGE_ACCESS);
diff --git a/components/content_settings/core/browser/cookie_settings_unittest.cc b/components/content_settings/core/browser/cookie_settings_unittest.cc
index 9df434b..6ac216c1 100644
--- a/components/content_settings/core/browser/cookie_settings_unittest.cc
+++ b/components/content_settings/core/browser/cookie_settings_unittest.cc
@@ -76,6 +76,7 @@
struct TestCase {
std::string test_name;
bool storage_access_api_enabled;
+ bool top_level_storage_access_grant_eligible;
bool force_allow_third_party_cookies;
};
@@ -146,12 +147,20 @@
return GetParam().storage_access_api_enabled;
}
+ bool IsTopLevelStorageAccessGrantEligible() const {
+ return GetParam().top_level_storage_access_grant_eligible;
+ }
+
bool IsForceAllowThirdPartyCookies() const {
return GetParam().force_allow_third_party_cookies;
}
net::CookieSettingOverrides GetCookieSettingOverrides() const {
net::CookieSettingOverrides overrides;
+ if (IsTopLevelStorageAccessGrantEligible()) {
+ overrides.Put(
+ net::CookieSettingOverride::kTopLevelStorageAccessGrantEligible);
+ }
if (IsForceAllowThirdPartyCookies()) {
overrides.Put(net::CookieSettingOverride::kForceThirdPartyByUser);
}
@@ -166,11 +175,25 @@
: CONTENT_SETTING_BLOCK;
}
+ // A version of above that considers Top-Level Storage Access API grant
+ // instead of Storage Access API grant, and user force allow.
+ ContentSetting SettingWithEitherOverrideForTopLevel() const {
+ // TODO(crbug.com/1385156): Check TopLevelStorageAccessAPI instead after
+ // separating the feature flag.
+ return (IsStorageAccessAPIEnabled() &&
+ IsTopLevelStorageAccessGrantEligible()) ||
+ IsForceAllowThirdPartyCookies()
+ ? CONTENT_SETTING_ALLOW
+ : CONTENT_SETTING_BLOCK;
+ }
+
ContentSetting SettingWithForceAllowThirdPartyCookies() const {
return IsForceAllowThirdPartyCookies() ? CONTENT_SETTING_ALLOW
: CONTENT_SETTING_BLOCK;
}
+ // The cookie access result would be blocked if not for a Storage Access API
+ // grant or force allow.
net::cookie_util::StorageAccessResult
BlockedStorageAccessResultWithEitherOverride() const {
if (IsStorageAccessAPIEnabled()) {
@@ -183,9 +206,16 @@
return net::cookie_util::StorageAccessResult::ACCESS_BLOCKED;
}
+ // A version of above that considers Top-Level Storage Access API grant
+ // instead of Storage Access API grant, and user force allow to allow cookie
+ // access.
net::cookie_util::StorageAccessResult
BlockedStorageAccessResultWithTopLevelOverride() const {
- if (IsStorageAccessAPIEnabled()) {
+ // TODO(crbug.com/1385156): Check TopLevelStorageAccessAPI instead after
+ // separating the feature flag.
+ if (IsStorageAccessAPIEnabled() && IsTopLevelStorageAccessGrantEligible()) {
+ // TODO(crbug.com/1385156): Separate metrics between StorageAccessAPI
+ // and the page-level variant.
return net::cookie_util::StorageAccessResult::
ACCESS_ALLOWED_TOP_LEVEL_STORAGE_ACCESS_GRANT;
}
@@ -766,9 +796,9 @@
// grants. TODO(crbug.com/1385156): as requirements for the two APIs solidify,
// this will likely not continue to be true.
TEST_P(CookieSettingsTest, GetCookieSettingTopLevelStorageAccess) {
- const GURL top_level_url = GURL(kFirstPartySite);
- const GURL url = GURL(kAllowedSite);
- const GURL third_url = GURL(kBlockedSite);
+ const GURL top_level_url(kFirstPartySite);
+ const GURL url(kAllowedSite);
+ const GURL third_url(kBlockedSite);
base::HistogramTester histogram_tester;
histogram_tester.ExpectTotalCount(kAllowedRequestsHistogram, 0);
@@ -784,7 +814,7 @@
EXPECT_EQ(cookie_settings_->GetCookieSetting(url, top_level_url,
GetCookieSettingOverrides(),
nullptr, QueryReason::kCookies),
- SettingWithEitherOverride());
+ SettingWithEitherOverrideForTopLevel());
histogram_tester.ExpectTotalCount(kAllowedRequestsHistogram, 1);
histogram_tester.ExpectBucketCount(
kAllowedRequestsHistogram,
@@ -1107,10 +1137,14 @@
/* no prefix */,
CookieSettingsTest,
testing::ValuesIn<TestCase>({
- {"disable_SAA", false, false},
- {"enable_SAA", true, false},
- {"disable_SAA_force_3PCs", false, true},
- {"enable_SAA_force_3PCs", true, true},
+ {"disable_all", false, false, false},
+ {"disable_SAA_disable_TopLevel_force_3PCs", false, false, true},
+ {"disable_SAA_enable_TopLevel", false, true, false},
+ {"disable_SAA_enable_TopLevel_force_3PCs", false, true, true},
+ {"enable_SAA_disable_TopLevel", true, false, false},
+ {"enable_SAA_disable_TopLevel_force_3PCs", true, false, true},
+ {"enable_SAA_enable_TopLevel", true, true, false},
+ {"enable_all", true, true, true},
}),
[](const testing::TestParamInfo<CookieSettingsTest::ParamType>& info) {
return info.param.test_name;
@@ -1121,10 +1155,14 @@
/* no prefix */,
CookieSettingsTestSandboxV4Enabled,
testing::ValuesIn<TestCase>({
- {"disable_SAA", false, false},
- {"enable_SAA", true, false},
- {"disable_SAA_force_3PCs", false, true},
- {"enable_SAA_force_3PCs", true, true},
+ {"disable_all", false, false, false},
+ {"disable_SAA_disable_TopLevel_force_3PCs", false, false, true},
+ {"disable_SAA_enable_TopLevel", false, true, false},
+ {"disable_SAA_enable_TopLevel_force_3PCs", false, true, true},
+ {"enable_SAA_disable_TopLevel", true, false, false},
+ {"enable_SAA_disable_TopLevel_force_3PCs", true, false, true},
+ {"enable_SAA_enable_TopLevel", true, true, false},
+ {"enable_all", true, true, true},
}),
[](const testing::TestParamInfo<CookieSettingsTest::ParamType>& info) {
return info.param.test_name;
diff --git a/net/cookies/cookie_setting_override.h b/net/cookies/cookie_setting_override.h
index a475727f5..24bba57 100644
--- a/net/cookies/cookie_setting_override.h
+++ b/net/cookies/cookie_setting_override.h
@@ -17,12 +17,15 @@
// When specified, the user has indicated to force allowing third-party
// cookies.
kForceThirdPartyByUser = 1,
+ // When specified, third-party cookies may be allowed based on existence of
+ // TopLevelStorageAccess grants.
+ kTopLevelStorageAccessGrantEligible = 2,
+ kMaxValue = kTopLevelStorageAccessGrantEligible,
};
-using CookieSettingOverrides =
- base::EnumSet<CookieSettingOverride,
- CookieSettingOverride::kNone,
- CookieSettingOverride::kForceThirdPartyByUser>;
+using CookieSettingOverrides = base::EnumSet<CookieSettingOverride,
+ CookieSettingOverride::kNone,
+ CookieSettingOverride::kMaxValue>;
} // namespace net
diff --git a/net/url_request/url_request_test_util.cc b/net/url_request/url_request_test_util.cc
index d37a409..23d6267 100644
--- a/net/url_request/url_request_test_util.cc
+++ b/net/url_request/url_request_test_util.cc
@@ -341,6 +341,7 @@
kStageBeforeRedirect | // a delegate can trigger a redirection
kStageCompletedError; // request canceled by delegate
created_requests_++;
+ cookie_setting_overrides_ = request->cookie_setting_overrides();
return OK;
}
diff --git a/net/url_request/url_request_test_util.h b/net/url_request/url_request_test_util.h
index bcdc183e..0531e8c5 100644
--- a/net/url_request/url_request_test_util.h
+++ b/net/url_request/url_request_test_util.h
@@ -293,6 +293,10 @@
fps_cache_filter_ = std::move(cache_filter);
}
+ CookieSettingOverrides cookie_setting_overrides() const {
+ return cookie_setting_overrides_;
+ }
+
protected:
// NetworkDelegate:
int OnBeforeURLRequest(URLRequest* request,
@@ -377,6 +381,8 @@
int next_request_id_ = 0;
FirstPartySetsCacheFilter fps_cache_filter_;
+
+ CookieSettingOverrides cookie_setting_overrides_;
};
// ----------------------------------------------------------------------------
diff --git a/services/network/cookie_settings.cc b/services/network/cookie_settings.cc
index 98df360..c40b9e6 100644
--- a/services/network/cookie_settings.cc
+++ b/services/network/cookie_settings.cc
@@ -247,7 +247,9 @@
IsAllowedByStorageAccessGrant(url, first_party_url)) {
storage_access_result = net::cookie_util::StorageAccessResult::
ACCESS_ALLOWED_STORAGE_ACCESS_GRANT;
- } else if (ShouldConsiderTopLevelStorageAccessGrants(query_reason) &&
+ } else if (overrides.Has(net::CookieSettingOverride::
+ kTopLevelStorageAccessGrantEligible) &&
+ ShouldConsiderTopLevelStorageAccessGrants(query_reason) &&
IsAllowedByTopLevelStorageAccessGrant(url, first_party_url)) {
storage_access_result = net::cookie_util::StorageAccessResult::
ACCESS_ALLOWED_TOP_LEVEL_STORAGE_ACCESS_GRANT;
diff --git a/services/network/cookie_settings_unittest.cc b/services/network/cookie_settings_unittest.cc
index e4b6359..c030bab 100644
--- a/services/network/cookie_settings_unittest.cc
+++ b/services/network/cookie_settings_unittest.cc
@@ -62,6 +62,7 @@
struct TestCase {
std::string test_name;
bool storage_access_api_enabled;
+ bool top_level_storage_access_grant_eligible;
bool force_allow_third_party_cookies;
};
@@ -93,12 +94,20 @@
return GetParam().storage_access_api_enabled;
}
+ bool IsTopLevelStorageAccessGrantEligible() const {
+ return GetParam().top_level_storage_access_grant_eligible;
+ }
+
bool IsForceAllowThirdPartyCookies() const {
return GetParam().force_allow_third_party_cookies;
}
net::CookieSettingOverrides GetCookieSettingOverrides() const {
net::CookieSettingOverrides overrides;
+ if (IsTopLevelStorageAccessGrantEligible()) {
+ overrides.Put(
+ net::CookieSettingOverride::kTopLevelStorageAccessGrantEligible);
+ }
if (IsForceAllowThirdPartyCookies()) {
overrides.Put(net::CookieSettingOverride::kForceThirdPartyByUser);
}
@@ -116,6 +125,18 @@
: CONTENT_SETTING_BLOCK;
}
+ // A version of above that considers Top-Level Storage Access API grant
+ // instead of Storage Access API grant, and user force allow.
+ ContentSetting SettingWithEitherOverrideForTopLevel() const {
+ // TODO(crbug.com/1385156): Check TopLevelStorageAccessAPI instead after
+ // separating the feature flag.
+ return (IsStorageAccessAPIEnabled() &&
+ IsTopLevelStorageAccessGrantEligible()) ||
+ IsForceAllowThirdPartyCookies()
+ ? CONTENT_SETTING_ALLOW
+ : CONTENT_SETTING_BLOCK;
+ }
+
ContentSetting SettingWithForceAllowThirdPartyCookies() const {
return IsForceAllowThirdPartyCookies() ? CONTENT_SETTING_ALLOW
: CONTENT_SETTING_BLOCK;
@@ -128,6 +149,8 @@
: net::cookie_util::StorageAccessResult::ACCESS_BLOCKED;
}
+ // The cookie access result would be blocked if not for a Storage Access API
+ // grant or force allow.
net::cookie_util::StorageAccessResult
BlockedStorageAccessResultWithEitherOverride() const {
if (IsStorageAccessAPIEnabled()) {
@@ -140,6 +163,23 @@
return net::cookie_util::StorageAccessResult::ACCESS_BLOCKED;
}
+ // A version of above that considers Top-Level Storage Access API grant
+ // instead of Storage Access API grant, and user force allow to allow cookie
+ // access.
+ net::cookie_util::StorageAccessResult
+ BlockedStorageAccessResultWithEitherOverrideForTopLevel() const {
+ // TODO(crbug.com/1385156): Check TopLevelStorageAccessAPI instead after
+ // separating the feature flag.
+ if (IsStorageAccessAPIEnabled() && IsTopLevelStorageAccessGrantEligible()) {
+ return net::cookie_util::StorageAccessResult::
+ ACCESS_ALLOWED_TOP_LEVEL_STORAGE_ACCESS_GRANT;
+ }
+ if (IsForceAllowThirdPartyCookies()) {
+ return net::cookie_util::StorageAccessResult::ACCESS_ALLOWED_FORCED;
+ }
+ return net::cookie_util::StorageAccessResult::ACCESS_BLOCKED;
+ }
+
private:
base::test::ScopedFeatureList features_;
base::test::TaskEnvironment task_environment_;
@@ -339,6 +379,109 @@
}
}
+// The Top-Level Storage Access API should unblock storage access that would
+// otherwise be blocked.
+TEST_P(CookieSettingsTest, GetCookieSettingTopLevelStorageAccessUnblocks) {
+ const GURL top_level_url(kURL);
+ const GURL url(kOtherURL);
+ const GURL third_url(kDomainURL);
+
+ base::HistogramTester histogram_tester;
+ histogram_tester.ExpectTotalCount(kAllowedRequestsHistogram, 0);
+
+ CookieSettings settings;
+ settings.set_content_settings(
+ {CreateSetting("*", "*", CONTENT_SETTING_ALLOW)});
+ settings.set_block_third_party_cookies(true);
+
+ // Only set the storage access granted by Top-Level Storage Access API.
+ settings.set_top_level_storage_access_grants(
+ {CreateSetting(url.host(), top_level_url.host(), CONTENT_SETTING_ALLOW)});
+
+ // When requesting our setting for the embedder/top-level combination our
+ // grant is for access should be allowed. For any other domain pairs access
+ // should still be blocked.
+ EXPECT_EQ(
+ settings.GetCookieSetting(url, top_level_url, GetCookieSettingOverrides(),
+ nullptr, QueryReason::kCookies),
+ SettingWithEitherOverrideForTopLevel());
+ histogram_tester.ExpectTotalCount(kAllowedRequestsHistogram, 1);
+ histogram_tester.ExpectBucketCount(
+ kAllowedRequestsHistogram,
+ static_cast<int>(
+ BlockedStorageAccessResultWithEitherOverrideForTopLevel()),
+ 1);
+
+ // Check the cookie setting that does not match the top-level storage access
+ // grant--the |top_level_url| granting access to |url| is now being loaded
+ // under |url| as the top level url.
+ EXPECT_EQ(
+ settings.GetCookieSetting(top_level_url, url, GetCookieSettingOverrides(),
+ nullptr, QueryReason::kCookies),
+ SettingWithForceAllowThirdPartyCookies());
+ histogram_tester.ExpectTotalCount(kAllowedRequestsHistogram, 2);
+ // TODO(crbug.com/1385156): Separate metrics between StorageAccessAPI
+ // and the page-level variant.
+ histogram_tester.ExpectBucketCount(
+ kAllowedRequestsHistogram,
+ static_cast<int>(net::cookie_util::StorageAccessResult::
+ ACCESS_ALLOWED_TOP_LEVEL_STORAGE_ACCESS_GRANT),
+ IsStorageAccessAPIEnabled() && IsTopLevelStorageAccessGrantEligible()
+ ? 1
+ : 0);
+ histogram_tester.ExpectBucketCount(
+ kAllowedRequestsHistogram,
+ static_cast<int>(
+ BlockedStorageAccessResultWithEitherOverrideForTopLevel()),
+ IsStorageAccessAPIEnabled() && IsTopLevelStorageAccessGrantEligible()
+ ? 1
+ : 2);
+
+ // Check the cookie setting that does not match the top-level storage access
+ // grant where a |third_url| is used.
+ EXPECT_EQ(
+ settings.GetCookieSetting(url, third_url, GetCookieSettingOverrides(),
+ nullptr, QueryReason::kCookies),
+ SettingWithForceAllowThirdPartyCookies());
+ EXPECT_EQ(settings.GetCookieSetting(third_url, top_level_url,
+ GetCookieSettingOverrides(), nullptr,
+ QueryReason::kCookies),
+ SettingWithForceAllowThirdPartyCookies());
+
+ // If third-party cookies are blocked, Top-Level Storage Access grant takes
+ // precedence over possible override to force allow third-party cookies.
+ {
+ base::HistogramTester histogram_tester_2;
+ EXPECT_EQ(settings.GetCookieSetting(url, top_level_url,
+ GetCookieSettingOverrides(), nullptr,
+ QueryReason::kCookies),
+ SettingWithEitherOverrideForTopLevel());
+ histogram_tester_2.ExpectTotalCount(kAllowedRequestsHistogram, 1);
+ histogram_tester_2.ExpectBucketCount(
+ kAllowedRequestsHistogram,
+ static_cast<int>(
+ BlockedStorageAccessResultWithEitherOverrideForTopLevel()),
+ 1);
+ }
+
+ // If cookies are globally blocked, Top-Level Storage Access grants and 3PC
+ // override should both be ignored.
+ {
+ settings.set_content_settings(
+ {CreateSetting("*", "*", CONTENT_SETTING_BLOCK)});
+ base::HistogramTester histogram_tester_2;
+ EXPECT_EQ(settings.GetCookieSetting(url, top_level_url,
+ GetCookieSettingOverrides(), nullptr,
+ QueryReason::kCookies),
+ CONTENT_SETTING_BLOCK);
+ histogram_tester_2.ExpectTotalCount(kAllowedRequestsHistogram, 1);
+ histogram_tester_2.ExpectBucketCount(
+ kAllowedRequestsHistogram,
+ static_cast<int>(net::cookie_util::StorageAccessResult::ACCESS_BLOCKED),
+ 1);
+ }
+}
+
// Subdomains of the granted embedding url should not gain access if a valid
// grant exists.
TEST_P(CookieSettingsTest, GetCookieSettingSAAResourceWildcards) {
@@ -1055,10 +1198,14 @@
/* no prefix */,
CookieSettingsTest,
testing::ValuesIn<TestCase>({
- {"disable_SAA", false, false},
- {"enable_SAA", true, false},
- {"disable_SAA_force_3PCs", false, true},
- {"enable_SAA_force_3PCs", true, true},
+ {"disable_all", false, false, false},
+ {"disable_SAA_disable_TopLevel_force_3PCs", false, false, true},
+ {"disable_SAA_enable_TopLevel", false, true, false},
+ {"disable_SAA_enable_TopLevel_force_3PCs", false, true, true},
+ {"enable_SAA_disable_TopLevel", true, false, false},
+ {"enable_SAA_disable_TopLevel_force_3PCs", true, false, true},
+ {"enable_SAA_enable_TopLevel", true, true, false},
+ {"enable_all", true, true, true},
}),
[](const testing::TestParamInfo<CookieSettingsTest::ParamType>& info) {
return info.param.test_name;
diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc
index dd848ae4..0f5ad68 100644
--- a/services/network/url_loader.cc
+++ b/services/network/url_loader.cc
@@ -46,6 +46,7 @@
#include "net/base/upload_file_element_reader.h"
#include "net/cookies/canonical_cookie.h"
#include "net/cookies/cookie_inclusion_status.h"
+#include "net/cookies/cookie_setting_override.h"
#include "net/cookies/cookie_store.h"
#include "net/cookies/cookie_util.h"
#include "net/cookies/site_for_cookies.h"
@@ -728,6 +729,11 @@
request.net_log_reference_info.value());
}
+ if (network::cors::IsCorsEnabledRequestMode(request_mode_)) {
+ url_request_->set_cookie_setting_overrides(net::CookieSettingOverrides(
+ net::CookieSettingOverride::kTopLevelStorageAccessGrantEligible));
+ }
+
#if BUILDFLAG(IS_ANDROID)
if (base::FeatureList::IsEnabled(net::features::kRecordRadioWakeupTrigger)) {
MaybeRecordURLLoaderCreationForWakeupTrigger(request, traffic_annotation);
diff --git a/services/network/url_loader_unittest.cc b/services/network/url_loader_unittest.cc
index 7c1d147..8ed2cc78 100644
--- a/services/network/url_loader_unittest.cc
+++ b/services/network/url_loader_unittest.cc
@@ -56,6 +56,7 @@
#include "net/cookies/cookie_access_result.h"
#include "net/cookies/cookie_change_dispatcher.h"
#include "net/cookies/cookie_inclusion_status.h"
+#include "net/cookies/cookie_setting_override.h"
#include "net/cookies/cookie_util.h"
#include "net/dns/mock_host_resolver.h"
#include "net/http/http_network_session.h"
@@ -4745,6 +4746,77 @@
EXPECT_TRUE(url_loader->AllowCookies(third_party_url, site_for_cookies));
}
+TEST_F(URLLoaderTest, CookieSettingOverrides_NoCors) {
+ GURL url("http://www.example.com.test/");
+ base::RunLoop delete_run_loop;
+ // No-cors request should not have the `kTopLevelStorageAccessGrantEligible`
+ // override.
+ ResourceRequest request = CreateResourceRequest("GET", url);
+ // Request mode is `no-cors` by default.
+ ASSERT_EQ(request.mode, network::mojom::RequestMode::kNoCors);
+ mojo::PendingRemote<mojom::URLLoader> loader;
+ std::unique_ptr<URLLoader> url_loader;
+ context().mutable_factory_params().process_id = mojom::kBrowserProcessId;
+ url_loader = URLLoaderOptions().MakeURLLoader(
+ context(), DeleteLoaderCallback(&delete_run_loop, &url_loader),
+ loader.InitWithNewPipeAndPassReceiver(), request,
+ client()->CreateRemote());
+
+ client()->RunUntilComplete();
+ delete_run_loop.Run();
+
+ EXPECT_FALSE(test_network_delegate()->cookie_setting_overrides().Has(
+ net::CookieSettingOverride::kTopLevelStorageAccessGrantEligible));
+}
+
+TEST_F(URLLoaderTest, CookieSettingOverrides_Cors) {
+ GURL url("http://www.example.com.test/");
+ base::RunLoop delete_run_loop;
+ // Cors request should have the `kTopLevelStorageAccessGrantEligible`
+ // override.
+ ResourceRequest request = CreateResourceRequest("GET", url);
+ // Set request mode to `cors`.
+ request.mode = network::mojom::RequestMode::kCors;
+ mojo::PendingRemote<mojom::URLLoader> loader;
+ std::unique_ptr<URLLoader> url_loader;
+ context().mutable_factory_params().process_id = mojom::kBrowserProcessId;
+ url_loader = URLLoaderOptions().MakeURLLoader(
+ context(), DeleteLoaderCallback(&delete_run_loop, &url_loader),
+ loader.InitWithNewPipeAndPassReceiver(), request,
+ client()->CreateRemote());
+
+ client()->RunUntilComplete();
+ delete_run_loop.Run();
+
+ EXPECT_TRUE(test_network_delegate()->cookie_setting_overrides().Has(
+ net::CookieSettingOverride::kTopLevelStorageAccessGrantEligible));
+}
+
+TEST_F(URLLoaderTest, CookieSettingOverrides_Cors_UnchangedOnRedirects) {
+ GURL dest_url("http://www.example.com.test/");
+ GURL redirecting_url =
+ test_server()->GetURL("/server-redirect?" + dest_url.spec());
+
+ base::RunLoop delete_run_loop;
+ // Cors request should have the `kTopLevelStorageAccessGrantEligible`
+ // override.
+ ResourceRequest request = CreateResourceRequest("GET", redirecting_url);
+ // Set request mode to `cors`.
+ request.mode = network::mojom::RequestMode::kCors;
+ mojo::Remote<mojom::URLLoader> loader;
+ std::unique_ptr<URLLoader> url_loader;
+ context().mutable_factory_params().process_id = mojom::kBrowserProcessId;
+ url_loader = URLLoaderOptions().MakeURLLoader(
+ context(), DeleteLoaderCallback(&delete_run_loop, &url_loader),
+ loader.BindNewPipeAndPassReceiver(), request, client()->CreateRemote());
+
+ client()->RunUntilRedirectReceived();
+ loader->FollowRedirect({}, {}, {}, absl::nullopt);
+
+ EXPECT_TRUE(test_network_delegate()->cookie_setting_overrides().Has(
+ net::CookieSettingOverride::kTopLevelStorageAccessGrantEligible));
+}
+
namespace {
enum class TestMode {
