CookieStore: Return EffectiveSameSite from CookieStore APIs

This updates CookieStoreAPI to return EffectiveSameSite across all
of its endpoints. If EffectiveSameSite is not available it will
fall back to using SameSite.

Bug: 1065010, 1092695
Change-Id: I26b0dd8e9ccbf3a57c3eef1a828d94fb4ce14568
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2295927
Commit-Queue: Ayu Ishii <ayui@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792337}

chrome/browser/net/cookie_store_samesite_browsertest.cc

+// Copyright (c) 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/path_service.h"
+#include "base/strings/stringprintf.h"
+#include "base/test/scoped_feature_list.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "components/content_settings/core/common/features.h"
+#include "content/public/common/content_paths.h"
+#include "content/public/common/content_switches.h"
+#include "content/public/test/browser_test.h"
+#include "content/public/test/browser_test_utils.h"
+#include "net/base/features.h"
+#include "net/dns/mock_host_resolver.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+
+namespace {
+
+class CookieStoreSameSiteTest : public InProcessBrowserTest,
+                                public ::testing::WithParamInterface<bool> {
+ protected:
+  CookieStoreSameSiteTest()
+      : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {
+    if (SameSiteFeatureEnabled()) {
+      feature_list_.InitAndEnableFeature(
+          net::features::kSameSiteByDefaultCookies);
+    } else {
+      feature_list_.InitAndDisableFeature(
+          net::features::kSameSiteByDefaultCookies);
+    }
+  }
+
+  void SetUpOnMainThread() override {
+    host_resolver()->AddRule("*", "127.0.0.1");
+    base::FilePath path;
+    base::PathService::Get(content::DIR_TEST_DATA, &path);
+    https_server_.ServeFilesFromDirectory(path);
+    https_server_.AddDefaultHandlers(GetChromeTestDataDir());
+    https_server_.SetSSLConfig(net::EmbeddedTestServer::CERT_TEST_NAMES);
+    ASSERT_TRUE(https_server_.Start());
+  }
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitchASCII(switches::kEnableBlinkFeatures,
+                                    "CookieStoreDocument");
+  }
+
+  void NavigateToPageWithFrame(const std::string& host) {
+    GURL main_url(https_server_.GetURL(host, "/iframe.html"));
+    ui_test_utils::NavigateToURL(browser(), main_url);
+  }
+
+  content::RenderFrameHost* GetFrame() {
+    content::WebContents* web_contents =
+        browser()->tab_strip_model()->GetActiveWebContents();
+    return ChildFrameAt(web_contents->GetMainFrame(), 0);
+  }
+
+  std::string GetCookieSameSite(const std::string& cookie_name) {
+    std::string script = base::StringPrintf(R"(
+      (async () => {
+          const cookie = await cookieStore.get('%s');
+          return cookie ? cookie.sameSite : '';
+      }) ();
+    )",
+                                            cookie_name.c_str());
+    return content::EvalJs(GetFrame(), script).ExtractString();
+  }
+
+ protected:
+  bool SameSiteFeatureEnabled() { return GetParam(); }
+
+  net::test_server::EmbeddedTestServer https_server_;
+  base::test::ScopedFeatureList feature_list_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CookieStoreSameSiteTest);
+};
+
+IN_PROC_BROWSER_TEST_P(CookieStoreSameSiteTest,
+                       CookieStoreUsesEffectiveSameSiteForUnspecifiedCookies) {
+  NavigateToPageWithFrame("a.test");
+
+  // Create unspecified sameSite cookie with document.cookie because CookieStore
+  // doesn't allow unspecified sameSite.
+  EXPECT_TRUE(content::ExecJs(GetFrame(),
+                              "document.cookie='unspecified-cookie=value'"));
+
+  std::string sameSite = GetCookieSameSite("unspecified-cookie");
+  if (SameSiteFeatureEnabled()) {
+    EXPECT_EQ("lax", sameSite);
+  } else {
+    EXPECT_EQ("none", sameSite);
+  }
+}
+
+IN_PROC_BROWSER_TEST_P(CookieStoreSameSiteTest,
+                       CookieStoreSameSiteDefaultsToStrict) {
+  NavigateToPageWithFrame("a.test");
+
+  // Create cookie with CookieStore with no sameSite.
+  EXPECT_TRUE(content::ExecJs(GetFrame(),
+                              "cookieStore.set('default-cookie', 'value')"));
+
+  EXPECT_EQ("strict", GetCookieSameSite("default-cookie"));
+}
+
+IN_PROC_BROWSER_TEST_P(CookieStoreSameSiteTest,
+                       CookieStoreUsesSpecifiedSameSite) {
+  NavigateToPageWithFrame("a.test");
+
+  // Create cookie with CookieStore with none sameSite. CookieStore defaults to
+  // Secure when written from a secure web origin, therefore allows for sameSite
+  // none.
+  EXPECT_TRUE(content::ExecJs(GetFrame(),
+                              "cookieStore.set({name: 'none-cookie', value: "
+                              "'value', sameSite: 'none'})"));
+
+  EXPECT_EQ("none", GetCookieSameSite("none-cookie"));
+}
+
+INSTANTIATE_TEST_SUITE_P(All, CookieStoreSameSiteTest, ::testing::Bool());
+}  // namespace

chrome/test/BUILD.gn

@@ -1007,6 +1007,7 @@ if (!is_android) {
       "../browser/net/chrome_network_service_browsertest.cc",
       "../browser/net/chrome_network_service_restart_browsertest.cc",
       "../browser/net/cookie_policy_browsertest.cc",
+      "../browser/net/cookie_store_samesite_browsertest.cc",
       "../browser/net/dns_over_https_browsertest.cc",
       "../browser/net/dns_probe_browsertest.cc",
       "../browser/net/errorpage_browsertest.cc",

third_party/blink/renderer/modules/cookie_store/cookie_change_event.cc

@@ -64,20 +64,37 @@ String ToCookieListItemSameSite(network::mojom::CookieSameSite same_site) {
   NOTREACHED();
 }
 
+String ToCookieListItemEffectiveSameSite(
+    network::mojom::CookieEffectiveSameSite effective_same_site) {
+  switch (effective_same_site) {
+    case network::mojom::CookieEffectiveSameSite::kStrictMode:
+      return "strict";
+    case network::mojom::CookieEffectiveSameSite::kLaxMode:
+    case network::mojom::CookieEffectiveSameSite::kLaxModeAllowUnsafe:
+      return "lax";
+    case network::mojom::CookieEffectiveSameSite::kNoRestriction:
+      return "none";
+    case network::mojom::CookieEffectiveSameSite::kUndefined:
+      return String();
+  }
+}
+
 }  // namespace
 
 // static
-// TODO(crbug.com/1092695): Update to take in CookieWithAccessResult so
-// CookieListItem can use EffectiveSameSite for SameSite.
 CookieListItem* CookieChangeEvent::ToCookieListItem(
     const CanonicalCookie& canonical_cookie,
+    const network::mojom::blink::CookieEffectiveSameSite& effective_same_site,
     bool is_deleted) {
   CookieListItem* list_item = CookieListItem::Create();
 
   list_item->setName(canonical_cookie.Name());
   list_item->setPath(canonical_cookie.Path());
   list_item->setSecure(canonical_cookie.IsSecure());
-  auto&& same_site = ToCookieListItemSameSite(canonical_cookie.SameSite());
+  // Use effective same site if available, otherwise use same site.
+  auto&& same_site = ToCookieListItemEffectiveSameSite(effective_same_site);
+  if (same_site.IsNull())
+    same_site = ToCookieListItemSameSite(canonical_cookie.SameSite());
   if (!same_site.IsNull())
     list_item->setSameSite(same_site);
 
@@ -104,14 +121,14 @@ CookieListItem* CookieChangeEvent::ToCookieListItem(
 
 // static
 void CookieChangeEvent::ToEventInfo(
-    const CanonicalCookie& backend_cookie,
-    ::network::mojom::CookieChangeCause change_cause,
+    const network::mojom::blink::CookieChangeInfoPtr& change_info,
     HeapVector<Member<CookieListItem>>& changed,
     HeapVector<Member<CookieListItem>>& deleted) {
-  switch (change_cause) {
+  switch (change_info->cause) {
     case ::network::mojom::CookieChangeCause::INSERTED: {
-      CookieListItem* cookie =
-          ToCookieListItem(backend_cookie, false /* is_deleted */);
+      CookieListItem* cookie = ToCookieListItem(
+          change_info->cookie, change_info->access_result->effective_same_site,
+          false /* is_deleted */);
       changed.push_back(cookie);
       break;
     }
@@ -120,8 +137,9 @@ void CookieChangeEvent::ToEventInfo(
     case ::network::mojom::CookieChangeCause::EXPIRED:
     case ::network::mojom::CookieChangeCause::EVICTED:
     case ::network::mojom::CookieChangeCause::EXPIRED_OVERWRITE: {
-      CookieListItem* cookie =
-          ToCookieListItem(backend_cookie, true /* is_deleted */);
+      CookieListItem* cookie = ToCookieListItem(
+          change_info->cookie, change_info->access_result->effective_same_site,
+          true /* is_deleted */);
       deleted.push_back(cookie);
       break;
     }

third_party/blink/renderer/modules/cookie_store/cookie_change_event.h

@@ -63,13 +63,14 @@ class CookieChangeEvent final : public Event {
 
   static CookieListItem* ToCookieListItem(
       const CanonicalCookie& canonical_cookie,
+      const network::mojom::blink::CookieEffectiveSameSite&,
       bool is_deleted);  // True for information from a cookie deletion event.
 
   // Helper for converting backend event information into a CookieChangeEvent.
-  static void ToEventInfo(const CanonicalCookie& cookie,
-                          ::network::mojom::CookieChangeCause cause,
-                          HeapVector<Member<CookieListItem>>& changed,
-                          HeapVector<Member<CookieListItem>>& deleted);
+  static void ToEventInfo(
+      const network::mojom::blink::CookieChangeInfoPtr& change_info,
+      HeapVector<Member<CookieListItem>>& changed,
+      HeapVector<Member<CookieListItem>>& deleted);
 
  private:
   HeapVector<Member<CookieListItem>> changed_;

third_party/blink/renderer/modules/cookie_store/cookie_store.cc

@@ -357,8 +357,7 @@ void CookieStore::RemoveAllEventListeners() {
 void CookieStore::OnCookieChange(
     network::mojom::blink::CookieChangeInfoPtr change) {
   HeapVector<Member<CookieListItem>> changed, deleted;
-  CookieChangeEvent::ToEventInfo(change->cookie, change->cause, changed,
-                                 deleted);
+  CookieChangeEvent::ToEventInfo(change, changed, deleted);
   if (changed.IsEmpty() && deleted.IsEmpty()) {
     // The backend only reported OVERWRITE events, which are dropped.
     return;
@@ -433,7 +432,9 @@ void CookieStore::GetAllForUrlToGetAllResult(
   cookies.ReserveInitialCapacity(backend_cookies.size());
   for (const auto& backend_cookie : backend_cookies) {
     cookies.push_back(CookieChangeEvent::ToCookieListItem(
-        backend_cookie->cookie, false /* is_deleted */));
+        backend_cookie->cookie,
+        backend_cookie->access_result->effective_same_site,
+        false /* is_deleted */));
   }
 
   resolver->Resolve(std::move(cookies));
@@ -456,7 +457,9 @@ void CookieStore::GetAllForUrlToGetResult(
 
   const auto& backend_cookie = backend_cookies.front();
   CookieListItem* cookie = CookieChangeEvent::ToCookieListItem(
-      backend_cookie->cookie, false /* is_deleted */);
+      backend_cookie->cookie,
+      backend_cookie->access_result->effective_same_site,
+      false /* is_deleted */);
   resolver->Resolve(cookie);
 }
 

third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc

@@ -2336,8 +2336,7 @@ void ServiceWorkerGlobalScope::StartCookieChangeEvent(
 
   HeapVector<Member<CookieListItem>> changed;
   HeapVector<Member<CookieListItem>> deleted;
-  CookieChangeEvent::ToEventInfo(change->cookie, change->cause, changed,
-                                 deleted);
+  CookieChangeEvent::ToEventInfo(change, changed, deleted);
   Event* event = ExtendableCookieChangeEvent::Create(
       event_type_names::kCookiechange, std::move(changed), std::move(deleted),
       observer);