RestrictedCookieManager: check prefs before reporting on listeners.
This is needed for a few operations that can bypass prefs, like users
clearing all the cookies.
Bug: 958923
Change-Id: Id5551698f5664f8f4356827ed0f4456c813c2e70
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1625045
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663086}
diff --git a/services/network/restricted_cookie_manager.cc b/services/network/restricted_cookie_manager.cc
index f5f8838c..bb891b1e 100644
--- a/services/network/restricted_cookie_manager.cc
+++ b/services/network/restricted_cookie_manager.cc
@@ -26,10 +26,16 @@
class RestrictedCookieManager::Listener : public base::LinkNode<Listener> {
public:
Listener(net::CookieStore* cookie_store,
+ const RestrictedCookieManager* restricted_cookie_manager,
const GURL& url,
+ const GURL& site_for_cookies,
net::CookieOptions options,
mojom::CookieChangeListenerPtr mojo_listener)
- : url_(url), options_(options), mojo_listener_(std::move(mojo_listener)) {
+ : restricted_cookie_manager_(restricted_cookie_manager),
+ url_(url),
+ site_for_cookies_(site_for_cookies),
+ options_(options),
+ mojo_listener_(std::move(mojo_listener)) {
// TODO(pwnall): add a constructor w/options to net::CookieChangeDispatcher.
cookie_store_subscription_ =
cookie_store->GetChangeDispatcher().AddCallbackForUrl(
@@ -58,14 +64,31 @@
if (cookie.IncludeForRequestURL(url_, options_) !=
net::CanonicalCookie::CookieInclusionStatus::INCLUDE)
return;
+
+ // When a user blocks a site's access to cookies, the existing cookies are
+ // not deleted. This check prevents the site from observing their cookies
+ // being deleted at a later time, which can happen due to eviction or due to
+ // the user explicitly deleting all cookies.
+ if (!restricted_cookie_manager_->cookie_settings()->IsCookieAccessAllowed(
+ url_, site_for_cookies_))
+ return;
+
mojo_listener_->OnCookieChange(cookie, ToCookieChangeCause(cause));
}
// The CookieChangeDispatcher subscription used by this listener.
std::unique_ptr<net::CookieChangeSubscription> cookie_store_subscription_;
+ // Raw pointer usage is safe because RestrictedCookieManager owns this
+ // instance and is guaranteed to outlive it.
+ const RestrictedCookieManager* const restricted_cookie_manager_;
+
// The URL whose cookies this listener is interested in.
const GURL url_;
+
+ // Site context in which we're used; used for permission-checking.
+ const GURL site_for_cookies_;
+
// CanonicalCookie::IncludeForRequestURL options for this listener's interest.
const net::CookieOptions options_;
@@ -217,8 +240,9 @@
net::cookie_util::ComputeSameSiteContextForScriptGet(
url, site_for_cookies, base::nullopt /*initiator*/));
- auto listener = std::make_unique<Listener>(cookie_store_, url, net_options,
- std::move(mojo_listener));
+ auto listener =
+ std::make_unique<Listener>(cookie_store_, this, url, site_for_cookies,
+ net_options, std::move(mojo_listener));
listener->mojo_listener().set_connection_error_handler(
base::BindOnce(&RestrictedCookieManager::RemoveChangeListener,
diff --git a/services/network/restricted_cookie_manager.h b/services/network/restricted_cookie_manager.h
index f0ec3f6..a26fcf92 100644
--- a/services/network/restricted_cookie_manager.h
+++ b/services/network/restricted_cookie_manager.h
@@ -42,6 +42,8 @@
const url::Origin& origin);
~RestrictedCookieManager() override;
+ const CookieSettings* cookie_settings() const { return cookie_settings_; }
+
void GetAllForUrl(const GURL& url,
const GURL& site_for_cookies,
mojom::CookieManagerGetOptionsPtr options,
diff --git a/services/network/restricted_cookie_manager_unittest.cc b/services/network/restricted_cookie_manager_unittest.cc
index 421cb85..e0d92bfb 100644
--- a/services/network/restricted_cookie_manager_unittest.cc
+++ b/services/network/restricted_cookie_manager_unittest.cc
@@ -436,6 +436,23 @@
EXPECT_EQ("cookie-value", listener.observed_changes()[0].cookie.Value());
}
+TEST_F(RestrictedCookieManagerTest, ChangeSettings) {
+ network::mojom::CookieChangeListenerPtr listener_ptr;
+ network::mojom::CookieChangeListenerRequest request =
+ mojo::MakeRequest(&listener_ptr);
+ sync_service_->AddChangeListener(GURL("http://example.com/test/"),
+ GURL("http://notexample.com"),
+ std::move(listener_ptr));
+ TestCookieChangeListener listener(std::move(request));
+
+ ASSERT_THAT(listener.observed_changes(), testing::SizeIs(0));
+
+ cookie_settings_.set_block_third_party_cookies(true);
+ SetSessionCookie("cookie-name", "cookie-value", "example.com", "/");
+ base::RunLoop().RunUntilIdle();
+ ASSERT_THAT(listener.observed_changes(), testing::SizeIs(0));
+}
+
TEST_F(RestrictedCookieManagerTest, AddChangeListenerFromWrongOrigin) {
network::mojom::CookieChangeListenerPtr bad_listener_ptr;
network::mojom::CookieChangeListenerRequest bad_request =
