Experimental CookieStore API: some additional browser-side validation

Bug: 984646

Change-Id: Icb9721646d0c93ee06b049656b777e86acd05299
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1704356
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678467}

content/browser/cookie_store/cookie_store_manager.cc

@@ -15,6 +15,7 @@
 #include "content/browser/service_worker/service_worker_metrics.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_version.h"
+#include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/browser/browser_context.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "third_party/blink/public/common/service_worker/service_worker_status_code.h"
@@ -206,6 +207,15 @@ void CookieStoreManager::AppendSubscriptions(
     return;
   }
 
+  for (const auto& subscription : mojo_subscriptions) {
+    if (!ServiceWorkerUtils::ScopeMatches(service_worker_registration->scope(),
+                                          subscription->url)) {
+      // Another spot where BadMessage would be appropriate.
+      std::move(callback).Run(false);
+      return;
+    }
+  }
+
   if (mojo_subscriptions.empty()) {
     // Empty subscriptions are special-cased so we never have to serialize an
     // empty array of subscriptions. This is advantageous because the protobuf

content/browser/cookie_store/cookie_store_manager_unittest.cc

@@ -130,9 +130,13 @@ class CookieStoreWorkerTestHelper : public EmbeddedWorkerTestHelper {
            worker_helper_->install_subscription_batches_) {
         worker_helper_->cookie_store_service_->AppendSubscriptions(
             worker_helper_->service_worker_registration_id_,
-            std::move(subscriptions), base::BindOnce([](bool success) {
-              CHECK(success) << "AppendSubscriptions failed";
-            }));
+            std::move(subscriptions),
+            base::BindOnce(
+                [](bool expect_success, bool success) {
+                  EXPECT_EQ(expect_success, success)
+                      << "AppendSubscriptions wrong result";
+                },
+                worker_helper_->expect_subscription_success_));
       }
       worker_helper_->install_subscription_batches_.clear();
 
@@ -177,9 +181,11 @@ class CookieStoreWorkerTestHelper : public EmbeddedWorkerTestHelper {
   // Sets the cookie change subscriptions requested in the next install event.
   void SetOnInstallSubscriptions(
       std::vector<CookieStoreSync::Subscriptions> subscription_batches,
-      blink::mojom::CookieStore* cookie_store_service) {
+      blink::mojom::CookieStore* cookie_store_service,
+      bool expect_subscription_success = true) {
     install_subscription_batches_ = std::move(subscription_batches);
     cookie_store_service_ = cookie_store_service;
+    expect_subscription_success_ = expect_subscription_success;
   }
 
   // Spins inside a run loop until a service worker activate event is received.
@@ -200,6 +206,7 @@ class CookieStoreWorkerTestHelper : public EmbeddedWorkerTestHelper {
   // Used to add cookie change subscriptions during OnInstallEvent().
   blink::mojom::CookieStore* cookie_store_service_ = nullptr;
   std::vector<CookieStoreSync::Subscriptions> install_subscription_batches_;
+  bool expect_subscription_success_ = true;
   int64_t service_worker_registration_id_;
 
   // Set by WaitForActivateEvent(), used in OnActivateEvent().
@@ -437,6 +444,31 @@ TEST_P(CookieStoreManagerTest, OneSubscription) {
   EXPECT_EQ(GURL(kExampleScope), all_subscriptions[0]->url);
 }
 
+TEST_P(CookieStoreManagerTest, WrongDomainSubscription) {
+  std::vector<CookieStoreSync::Subscriptions> batches;
+  batches.emplace_back();
+
+  CookieStoreSync::Subscriptions& subscriptions = batches.back();
+  subscriptions.emplace_back(blink::mojom::CookieChangeSubscription::New());
+  subscriptions.back()->name = "cookie";
+  subscriptions.back()->match_type =
+      ::network::mojom::CookieMatchType::STARTS_WITH;
+  subscriptions.back()->url = GURL(kGoogleScope);
+
+  worker_test_helper_->SetOnInstallSubscriptions(std::move(batches),
+                                                 example_service_ptr_.get(),
+                                                 false /* expecting failure */);
+  int64_t registration_id =
+      RegisterServiceWorker(kExampleScope, kExampleWorkerScript);
+  ASSERT_NE(registration_id, kInvalidRegistrationId);
+
+  ASSERT_TRUE(
+      SetSessionCookie("cookie-name", "cookie-value", "google.com", "/"));
+  thread_bundle_.RunUntilIdle();
+
+  ASSERT_EQ(0u, worker_test_helper_->changes().size());
+}
+
 TEST_P(CookieStoreManagerTest, AppendSubscriptionsAfterEmptyInstall) {
   worker_test_helper_->SetOnInstallSubscriptions(
       std::vector<CookieStoreSync::Subscriptions>(),

third_party/blink/web_tests/external/wpt/cookie-store/serviceworker_cookieStore_subscriptions.js

@@ -11,7 +11,7 @@ self.addEventListener('install', (event) => {
     // cookie change logic that this test aims to cover.
     try {
       await cookieStore.subscribeToChanges([
-        { name: 'cookie-name1', matchType: 'equals', url: '/scope/path1' }]);
+        { name: 'cookie-name1', matchType: 'equals', url: '/cookie-store/path1' }]);
       await cookieStore.subscribeToChanges([
         { },  // Test the default values for subscription properties.
         { name: 'cookie-prefix', matchType: 'starts-with' },

third_party/blink/web_tests/external/wpt/cookie-store/serviceworker_cookieStore_subscriptions.tentative.https.html

@@ -9,13 +9,14 @@
 'use strict';
 
 (async () => {
-  const scope = 'scope';
+  // Not using an explicit scope here in order for script URL to be in scope,
+  // to cover implicit subscription URL construction.
 
-  let registration = await navigator.serviceWorker.getRegistration(scope);
+  let registration = await navigator.serviceWorker.getRegistration();
   if (registration)
     await registration.unregister();
   registration = await navigator.serviceWorker.register(
-      'serviceworker_cookieStore_subscriptions.js', {scope});
+      'serviceworker_cookieStore_subscriptions.js');
 
   fetch_tests_from_worker(registration.installing);
 })();

third_party/blink/web_tests/external/wpt/cookie-store/serviceworker_cookieStore_subscriptions_basic.js

@@ -8,7 +8,8 @@ self.addEventListener('install', (event) => {
   event.waitUntil((async () => {
     try {
       await cookieStore.subscribeToChanges([
-        { name: 'cookie-name', matchType: 'equals', url: '/scope/path' }]);
+        { name: 'cookie-name', matchType: 'equals',
+          url: '/cookie-store/scope/path' }]);
 
       // If the worker enters the "redundant" state, the UA may terminate it
       // before all tests have been reported to the client. Stifle errors in

third_party/blink/web_tests/external/wpt/cookie-store/serviceworker_cookieStore_subscriptions_eventhandler_attribute.js

@@ -8,7 +8,8 @@ self.addEventListener('install', (event) => {
   event.waitUntil((async () => {
     try {
       await cookieStore.subscribeToChanges([
-        { name: 'cookie-name', matchType: 'equals', url: '/scope/path' }]);
+        { name: 'cookie-name', matchType: 'equals',
+          url: '/cookie-store/scope/path' }]);
 
       // If the worker enters the "redundant" state, the UA may terminate it
       // before all tests have been reported to the client. Stifle errors in

third_party/blink/web_tests/external/wpt/cookie-store/serviceworker_cookieStore_subscriptions_mismatch.js

@@ -8,7 +8,8 @@ self.addEventListener('install', (event) => {
   event.waitUntil((async () => {
     try {
       await cookieStore.subscribeToChanges([
-        { name: 'cookie-name', matchType: 'equals', url: '/scope/path' }]);
+        { name: 'cookie-name', matchType: 'equals',
+          url: '/cookie-store/scope/path' }]);
 
       // If the worker enters the "redundant" state, the UA may terminate it
       // before all tests have been reported to the client. Stifle errors in