1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Fix default SameSite for session cookies

Follow-up to #45501.

The Rack base class that `CookieStore` inherits from [always sets
`:same_site`][1].  Thus, `options.key?(:same_site)` always returns true
for session cookies, preventing a default value from being set.

It would be possible to change Rack to conditionally set `:same_site`,
but, from Rack's perspective, it has no reason to not set `:same_site`,
because it treats a `nil` value the same as no value.

Therefore, this commit specifies a default `:same_site` in `CookieStore`,
which simply defers to `request.cookies_same_site_protection` as
`CookieJar` does.

Fixes #45681.

[1]: https://github.com/rack/rack/blob/2.2.4/lib/rack/session/abstract/id.rb#L398-L402
This commit is contained in:
Jonathan Hefner 2022-07-28 16:00:01 -05:00
parent 74a9929ce1
commit c95780d7c6
4 changed files with 36 additions and 5 deletions

View file

@ -70,7 +70,7 @@ module ActionDispatch
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 @@ module ActionDispatch
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"

View file

@ -56,8 +56,12 @@ module ActionDispatch
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)

View file

@ -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 @@ class CookieStoreTest < ActionDispatch::IntegrationTest
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 @@ class CookieStoreTest < ActionDispatch::IntegrationTest
config["action_dispatch.key_generator"] ||= Generator
config["action_dispatch.cookies_rotations"] ||= Rotations
config["action_dispatch.cookies_same_site_protection"] ||= SameSite
end
super

View file

@ -1670,7 +1670,9 @@ module ApplicationTests
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