Skip to content

Commit

Permalink
Make fetching insecure session fallback configurable
Browse files Browse the repository at this point in the history
  • Loading branch information
tmandke authored and stevenharman committed Nov 3, 2023
1 parent 734c38f commit 7743696
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 1 deletion.
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ been updated in the last 30 days. The 30 days cutoff can be changed using the
Configuration
--------------

Disable fallback to use insecure session by providing the option `secure_session_only`
when setting up the session store.
```ruby
Rails.application.config.session_store :active_record_store, :key => '_my_app_session', :secure_session_only => true
```

The default assumes a `sessions` table with columns:

* `id` (numeric primary key),
Expand Down
7 changes: 6 additions & 1 deletion lib/action_dispatch/session/active_record_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ class ActiveRecordStore < ActionDispatch::Session::AbstractSecureStore
SESSION_RECORD_KEY = 'rack.session.record'
ENV_SESSION_OPTIONS_KEY = Rack::RACK_SESSION_OPTIONS

def initialize(app, options = {})
@secure_session_only = options.delete(:secure_session_only) { false }
super(app, options)
end

private
def get_session(request, sid)
logger.silence do
Expand Down Expand Up @@ -136,7 +141,7 @@ def get_session_with_fallback(sid)
if sid && !self.class.private_session_id?(sid.public_id)
if (secure_session = session_class.find_by_session_id(sid.private_id))
secure_session
elsif (insecure_session = session_class.find_by_session_id(sid.public_id))
elsif !@secure_session_only && (insecure_session = session_class.find_by_session_id(sid.public_id))
insecure_session.session_id = sid.private_id # this causes the session to be secured
insecure_session
end
Expand Down
23 changes: 23 additions & 0 deletions test/action_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,29 @@ def test_session_store_with_all_domains
end
end

define_method :"test_unsecured_sessions_are_ignored_when_insecure_fallback_is_disabled_#{class_name}" do
with_store(class_name) do
with_test_route_set(secure_session_only: true) do
get '/set_session_value', params: { foo: 'baz' }
assert_response :success
public_session_id = cookies['_session_id']

session = ActiveRecord::SessionStore::Session.last
session.data # otherwise we cannot save
session.session_id = public_session_id
session.save!

get '/get_session_value'
assert_response :success

session.reload
new_session = ActiveRecord::SessionStore::Session.last
assert_not_equal public_session_id, new_session.session_id
assert_not_equal session.session_id, new_session.session_id
end
end
end

# to avoid a different kind of timing attack
define_method :"test_sessions_cannot_be_retrieved_by_their_private_session_id_for_#{class_name}" do
with_store(class_name) do
Expand Down

0 comments on commit 7743696

Please sign in to comment.