Skip to content

Commit

Permalink
Merge pull request #2039 from SAP-Oxygen/3-1-stable-session-id-patch-…
Browse files Browse the repository at this point in the history
…with-test

Fix for session ID fixation issue in ActiveRecord::SessionStore (squashed commits)
  • Loading branch information
spastorino committed Jul 12, 2011
2 parents 5c591e5 + 63fdd2c commit 926ef07
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 2 deletions.
31 changes: 31 additions & 0 deletions actionpack/test/activerecord/active_record_store_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,36 @@ def test_allows_session_fixation
assert_equal session_id, cookies['_session_id']
end
end

def test_incoming_invalid_session_id_via_cookie_should_be_ignored
with_test_route_set do
open_session do |sess|
sess.cookies['_session_id'] = 'INVALID'

sess.get '/set_session_value'
new_session_id = sess.cookies['_session_id']
assert_not_equal 'INVALID', new_session_id

sess.get '/get_session_value'
new_session_id_2 = sess.cookies['_session_id']
assert_equal new_session_id, new_session_id_2
end
end
end

def test_incoming_invalid_session_id_via_parameter_should_be_ignored
with_test_route_set(:cookie_only => false) do
open_session do |sess|
sess.get '/set_session_value', :_session_id => 'INVALID'
new_session_id = sess.cookies['_session_id']
assert_not_equal 'INVALID', new_session_id

sess.get '/get_session_value'
new_session_id_2 = sess.cookies['_session_id']
assert_equal new_session_id, new_session_id_2
end
end
end

private

Expand All @@ -247,6 +277,7 @@ def with_store(class_name)
session_class, ActiveRecord::SessionStore.session_class =
ActiveRecord::SessionStore.session_class, "ActiveRecord::SessionStore::#{class_name.camelize}".constantize
yield
ensure
ActiveRecord::SessionStore.session_class = session_class
end
end
8 changes: 6 additions & 2 deletions activerecord/lib/active_record/session_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,12 @@ def destroy
private
def get_session(env, sid)
Base.silence do
sid ||= generate_sid
session = find_session(sid)
unless sid and session = @@session_class.find_by_session_id(sid)
# If the sid was nil or if there is no pre-existing session under the sid,
# force the generation of a new sid and associate a new session associated with the new sid
sid = generate_sid
session = @@session_class.new(:session_id => sid, :data => {})
end
env[SESSION_RECORD_KEY] = session
[sid, session.data]
end
Expand Down

0 comments on commit 926ef07

Please sign in to comment.