Skip to content

Default to request's cookies_same_site_protection option #222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
# Change Log

All notable changes to this project will be documented in this file.

## Unreleased

* Default to the request's `cookies_same_site_protection` setting, brining
`ActiveRecordStore` in line with the default behavior of `CookieStore`.
[@sharman [#222](https://github.com/rails/activerecord-session_store/pull/222)]
* Drop Rails 7.0 support.
[@sharman [#221](https://github.com/rails/activerecord-session_store/pull/221)]

## 2.2.0

Expand Down
9 changes: 5 additions & 4 deletions lib/action_dispatch/session/active_record_store.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
require "active_support/core_ext/module/attribute_accessors"
require 'action_dispatch/middleware/session/abstract_store'
require "action_dispatch/middleware/session/abstract_store"

module ActionDispatch
module Session
Expand Down Expand Up @@ -57,12 +57,14 @@ class ActiveRecordStore < ActionDispatch::Session::AbstractSecureStore
# ActiveRecord::SessionStore::Session
class_attribute :session_class

SESSION_RECORD_KEY = 'rack.session.record'
DEFAULT_SAME_SITE = proc { |request| request.cookies_same_site_protection } # :nodoc:
ENV_SESSION_OPTIONS_KEY = Rack::RACK_SESSION_OPTIONS
SESSION_RECORD_KEY = "rack.session.record"

def initialize(app, options = {})
@secure_session_only = options.delete(:secure_session_only) { false }
super(app, options)
options[:same_site] = DEFAULT_SAME_SITE unless options.key?(:same_site)
super
end

private
Expand Down Expand Up @@ -167,7 +169,6 @@ def self.private_session_id?(session_id)
# user tried to retrieve a session by a private key?
session_id =~ /\A\d+::/
end

end
end
end
25 changes: 25 additions & 0 deletions test/action_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,31 @@ def test_getting_nil_session_value
end
end

def test_default_same_site_derives_SameSite_from_env
with_test_route_set do
get "/set_session_value"
assert_match %r{SameSite=Lax}i, headers["Set-Cookie"]
end
end

def test_explicit_same_site_sets_SameSite
session_options(same_site: :strict)

with_test_route_set do
get "/set_session_value"
assert_match %r{SameSite=Strict}i, headers["Set-Cookie"]
end
end

def test_explicit_nil_same_site_omits_SameSite
session_options(same_site: nil)

with_test_route_set do
get "/set_session_value"
assert_no_match %r{SameSite=}i, headers["Set-Cookie"]
end
end

def test_calling_reset_session_twice_does_not_raise_errors
with_test_route_set do
get '/call_reset_session', :params => { :twice => "true" }
Expand Down
10 changes: 10 additions & 0 deletions test/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,16 @@ def self.build_app(routes = nil)

private

# Overwrite `get` to set env hash
def get(path, **options)
options[:headers] ||= {}
options[:headers].tap do |config|
config["action_dispatch.cookies_same_site_protection"] ||= ->(_) { :lax }
end

super
end

def session_options(options = {})
(@session_options ||= {key: "_session_id"}).merge!(options)
end
Expand Down