Add console warning message for SameSite=Lax-allow-unsafe intervention

Display a warning message in the console when SameSiteByDefaultCookies
is enabled and a cookie without a SameSite attribute (which is defaulted
into Lax mode) gets sent on a request with an unsafe method. (This will
happen if the cookie is less than 2 minutes old, as a temporary
intervention to avoid breaking POST-based auth flows.)

Bug: 990439
Change-Id: I0776acb18c829c06cfc2eab7d5a69da05a6c44d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1779278Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693018}

content/browser/frame_host/render_frame_host_impl.cc

@@ -182,6 +182,7 @@
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "mojo/public/cpp/system/data_pipe.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
+#include "net/cookies/cookie_constants.h"
 #include "services/device/public/cpp/device_features.h"
 #include "services/device/public/mojom/sensor_provider.mojom.h"
 #include "services/device/public/mojom/wake_lock.mojom.h"
@@ -7353,6 +7354,23 @@ void RenderFrameHostImpl::AddSameSiteCookieDeprecationMessage(
         "can review cookies in developer tools under "
         "Application>Storage>Cookies and see more details at "
         "https://www.chromestatus.com/feature/5633521622188032.";
+  } else if (warning ==
+             net::CanonicalCookie::CookieInclusionStatus::WarningReason::
+                 WARN_SAMESITE_UNSPECIFIED_LAX_ALLOW_UNSAFE) {
+    if (!ShouldAddCookieSameSiteDeprecationMessage(
+            cookie_url, &cookie_lax_allow_unsafe_deprecation_url_hashes_)) {
+      return;
+    }
+    deprecation_message =
+        "A cookie associated with a resource at " + cookie_url +
+        " set without a `SameSite` attribute was sent with a non-idempotent "
+        "top-level cross-site request because it was less than " +
+        base::NumberToString(net::kLaxAllowUnsafeMaxAge.InMinutes()) +
+        " minutes old. A future release of Chrome will treat such cookies as "
+        "if they were set with `SameSite=Lax` and will only allow them to be "
+        "sent with top-level cross-site requests if the HTTP method is safe. "
+        "See more details at "
+        "https://www.chromestatus.com/feature/5088147346030592.";
   }
 
   if (deprecation_message.empty())

content/browser/frame_host/render_frame_host_impl.h

@@ -2339,6 +2339,7 @@ class CONTENT_EXPORT RenderFrameHostImpl
   base::circular_deque<size_t> cookie_no_samesite_deprecation_url_hashes_;
   base::circular_deque<size_t>
       cookie_samesite_none_insecure_deprecation_url_hashes_;
+  base::circular_deque<size_t> cookie_lax_allow_unsafe_deprecation_url_hashes_;
 
   // The lifecycle state of the frame.
   blink::mojom::FrameLifecycleState frame_lifecycle_state_ =

content/browser/frame_host/render_frame_host_impl_browsertest.cc

@@ -46,7 +46,9 @@
 #include "content/test/did_commit_navigation_interceptor.h"
 #include "content/test/frame_host_test_interface.mojom.h"
 #include "content/test/test_content_browser_client.h"
+#include "net/base/features.h"
 #include "net/base/net_errors.h"
+#include "net/cookies/cookie_constants.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/controllable_http_response.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
@@ -2474,7 +2476,7 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
 // Test deduplication of SameSite cookie deprecation messages.
 // TODO(crbug.com/976475): This test is flaky.
 IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
-                       DISABLED_SameSiteCookieDeprecationMessages) {
+                       DISABLED_DeduplicateSameSiteCookieDeprecationMessages) {
 #if defined(OS_ANDROID)
   // TODO(crbug.com/974701): This test is broken on Android that is
   // Marshmallow or older.
@@ -2519,6 +2521,72 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
   EXPECT_EQ(console_observer.messages()[1], console_observer.messages()[2]);
 }
 
+// Enable SameSiteByDefaultCookies to test deprecation messages for
+// Lax-allow-unsafe.
+class RenderFrameHostImplSameSiteByDefaultCookiesBrowserTest
+    : public RenderFrameHostImplBrowserTest {
+ public:
+  void SetUp() override {
+    feature_list_.InitWithFeatures({features::kCookieDeprecationMessages,
+                                    net::features::kSameSiteByDefaultCookies},
+                                   {});
+    RenderFrameHostImplBrowserTest::SetUp();
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(RenderFrameHostImplSameSiteByDefaultCookiesBrowserTest,
+                       DisplaySameSiteCookieDeprecationMessages) {
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  ConsoleObserverDelegate console_observer(web_contents, "*");
+  web_contents->SetDelegate(&console_observer);
+
+  // Test deprecation messages for SameSiteByDefault.
+  // Set a cookie without SameSite on b.com, then access it in a cross-site
+  // context.
+  base::Time set_cookie_time = base::Time::Now();
+  GURL url =
+      embedded_test_server()->GetURL("x.com", "/set-cookie?nosamesite=1");
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+  // Message does not appear in same-site context (main frame is x).
+  ASSERT_EQ(0u, console_observer.messages().size());
+  url = embedded_test_server()->GetURL(
+      "a.com", "/cross_site_iframe_factory.html?a(x())");
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+  // Message appears in cross-site context (a framing x).
+  EXPECT_EQ(1u, console_observer.messages().size());
+
+  // Test deprecation messages for CookiesWithoutSameSiteMustBeSecure.
+  // Set a cookie with SameSite=None but without Secure.
+  url = embedded_test_server()->GetURL(
+      "c.com", "/set-cookie?samesitenoneinsecure=1;SameSite=None");
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+  // The 1 message from before, plus the (different) message for setting the
+  // SameSite=None insecure cookie.
+  EXPECT_EQ(2u, console_observer.messages().size());
+
+  // Test deprecation messages for Lax-allow-unsafe.
+  url = embedded_test_server()->GetURL("a.com",
+                                       "/form_that_posts_cross_site.html");
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+  // Submit the form to make a cross-site POST request to x.com.
+  TestNavigationObserver form_post_observer(shell()->web_contents(), 1);
+  EXPECT_TRUE(ExecJs(shell(), "document.getElementById('text-form').submit()"));
+  form_post_observer.Wait();
+
+  // The test should not take more than 2 minutes.
+  ASSERT_LT(base::Time::Now() - set_cookie_time, net::kLaxAllowUnsafeMaxAge);
+  EXPECT_EQ(3u, console_observer.messages().size());
+
+  // Check that the messages were all distinct.
+  EXPECT_NE(console_observer.messages()[0], console_observer.messages()[1]);
+  EXPECT_NE(console_observer.messages()[0], console_observer.messages()[2]);
+  EXPECT_NE(console_observer.messages()[1], console_observer.messages()[2]);
+}
+
 IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
                        SchedulerTrackedFeatures) {
   EXPECT_TRUE(

content/browser/storage_partition_impl.cc

@@ -359,13 +359,14 @@ void DeprecateSameSiteCookies(int process_id,
     switch (warning) {
       case net::CanonicalCookie::CookieInclusionStatus::
           WARN_SAMESITE_UNSPECIFIED_CROSS_SITE_CONTEXT:
+      case net::CanonicalCookie::CookieInclusionStatus::
+          WARN_SAMESITE_UNSPECIFIED_LAX_ALLOW_UNSAFE:
         samesite_treated_as_lax_cookies = true;
         break;
       case net::CanonicalCookie::CookieInclusionStatus::
           WARN_SAMESITE_NONE_INSECURE:
         samesite_none_insecure_cookies = true;
         break;
-      // TODO(crbug.com/990439): Add messages for Lax-Allow-Unsafe intervention.
       default:
         break;
     }
@@ -375,6 +376,8 @@ void DeprecateSameSiteCookies(int process_id,
     }
   }
 
+  // TODO(crbug.com/990439): Do we need separate UseCounter metrics for
+  // Lax-allow-unsafe? We already have histograms in CanonicalCookie.
   if (samesite_treated_as_lax_cookies) {
     GetContentClient()->browser()->LogWebFeatureForCurrentPage(
         frame, blink::mojom::WebFeature::kCookieNoSameSite);