Merge pull request rails#45688 from jonathanhefner/session-cookies-default-same_site

Fix default `SameSite` for session cookies

Author: jonathanhefner

actionpack/lib/action_dispatch/middleware/cookies.rb

@@ -70,7 +70,7 @@ def cookies_serializer
     end
 
     def cookies_same_site_protection
-      get_header(Cookies::COOKIES_SAME_SITE_PROTECTION) || Proc.new { }
+      get_header(Cookies::COOKIES_SAME_SITE_PROTECTION)&.call(self)
     end
 
     def cookies_digest
@@ -454,7 +454,7 @@ def handle_options(options)
           options[:path]      ||= "/"
 
           unless options.key?(:same_site)
-            options[:same_site] = request.cookies_same_site_protection.call(request)
+            options[:same_site] = request.cookies_same_site_protection
           end
 
           if options[:domain] == :all || options[:domain] == "all"

actionpack/lib/action_dispatch/middleware/session/cookie_store.rb

@@ -56,8 +56,12 @@ def initialize(session_id, cookie_value = {})
         end
       end
 
+      DEFAULT_SAME_SITE = proc { |request| request.cookies_same_site_protection } # :nodoc:
+
       def initialize(app, options = {})
-        super(app, options.merge!(cookie_only: true))
+        options[:cookie_only] = true
+        options[:same_site] = DEFAULT_SAME_SITE if !options.key?(:same_site)
+        super
       end
 
       def delete_session(req, session_id, options)

actionpack/test/dispatch/session/cookie_store_test.rb

@@ -13,6 +13,8 @@ class CookieStoreTest < ActionDispatch::IntegrationTest
   Generator = ActiveSupport::KeyGenerator.new(SessionSecret, iterations: 1000)
   Rotations = ActiveSupport::Messages::RotationConfiguration.new
 
+  SameSite = proc { :lax }
+
   Encryptor = ActiveSupport::MessageEncryptor.new(
     Generator.generate_key(SessionSalt, 32), cipher: "aes-256-gcm", serializer: Marshal
   )
@@ -379,8 +381,29 @@ def test_session_store_with_all_domains
     end
   end
 
+  test "default same_site derives SameSite from env" do
+    with_test_route_set do
+      get "/set_session_value"
+      assert_match %r/SameSite=Lax/, headers["Set-Cookie"]
+    end
+  end
+
+  test "explicit same_site sets SameSite" do
+    with_test_route_set(same_site: :strict) do
+      get "/set_session_value"
+      assert_match %r/SameSite=Strict/, headers["Set-Cookie"]
+    end
+  end
+
+  test "explicit nil same_site omits SameSite" do
+    with_test_route_set(same_site: nil) do
+      get "/set_session_value"
+      assert_no_match %r/SameSite=/, headers["Set-Cookie"]
+    end
+  end
+
   private
-    # Overwrite get to send SessionSecret in env hash
+    # Overwrite `get` to set env hash
     def get(path, **options)
       options[:headers] ||= {}
       options[:headers].tap do |config|
@@ -390,6 +413,8 @@ def get(path, **options)
 
         config["action_dispatch.key_generator"] ||= Generator
         config["action_dispatch.cookies_rotations"] ||= Rotations
+
+        config["action_dispatch.cookies_same_site_protection"] ||= SameSite
       end
 
       super

railties/test/application/configuration_test.rb

@@ -1670,7 +1670,9 @@ def index
       make_basic_app
 
       assert_equal ActionDispatch::Session::CookieStore, app.config.session_store
-      assert_equal session_options, app.config.session_options
+      session_options.each do |key, value|
+        assert_equal value, app.config.session_options[key]
+      end
     end
 
     test "config.log_level defaults to debug in development" do