diff --git a/Gemfile b/Gemfile index fa75df1..f4a30cb 100644 --- a/Gemfile +++ b/Gemfile @@ -1,3 +1,9 @@ source 'https://rubygems.org' +git "https://github.com/rails/rails.git" do + gem "activerecord" + gem "actionpack" + gem "railties" +end + gemspec diff --git a/README.md b/README.md index 1ed08c7..b16cc17 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,25 @@ been updated in the last 30 days. The 30 days cutoff can be changed using the Configuration -------------- -The default assumes a `sessions` table with columns: +A cookie is used to identify the appropriate ActiveRecord record. +By default this cookie is in cleartext. +You may optionally configure this cookie to be signed or encrypted. For example, at the end of `config/application.rb`: + +```ruby +ActiveRecord::SessionStore::Session.sign_cookie = true +ActiveRecord::SessionStore::Session.encrypt_cookie = true +``` + +Setting these configuration values has the following behaviors: + +| `sign_cookie` | `encrypt_cookie` | Behavior | +| ------------- | ---------------- | ---------------- | +| false | false | Cleartext cookie | +| false | true | Encrypted cookie | +| true | false | Signed cookie | +| true | true | Encrypted cookie | + +The default assumes a `sessions` tables with columns: * `id` (numeric primary key), * `session_id` (string, usually varchar; maximum length is 255), and diff --git a/activerecord-session_store.gemspec b/activerecord-session_store.gemspec index efe846d..88fc92f 100644 --- a/activerecord-session_store.gemspec +++ b/activerecord-session_store.gemspec @@ -27,3 +27,4 @@ Gem::Specification.new do |s| s.add_development_dependency('sqlite3') end + diff --git a/gemfiles/rails_edge.gemfile b/gemfiles/rails_edge.gemfile index b78671f..26eb457 100644 --- a/gemfiles/rails_edge.gemfile +++ b/gemfiles/rails_edge.gemfile @@ -6,7 +6,6 @@ git "https://github.com/rails/rails.git", :branch => "main" do gem "railties" end -gem "rack", :git => "https://github.com/rack/rack.git", :branch => "main" -gem "rack-session", :git => "https://github.com/rack/rack-session.git", :branch => "main" +gem "rack", "~> 2.0", ">= 2.2.4" gemspec :path => "../" diff --git a/lib/action_dispatch/session/active_record_store.rb b/lib/action_dispatch/session/active_record_store.rb index 99bd8e2..ab04ec1 100644 --- a/lib/action_dispatch/session/active_record_store.rb +++ b/lib/action_dispatch/session/active_record_store.rb @@ -9,6 +9,22 @@ module Session # provided, but any object duck-typing to an Active Record Session class # with text +session_id+ and +data+ attributes is sufficient. # + # A cookie is used to identify the appropriate ActiveRecord record. + # By default this cookie is in cleartext. + # You may optionally configure this cookie to be signed or encrypted. + # For example, at the end of `config/application.rb`: + # + # ActiveRecord::SessionStore::Session.sign_cookie = true + # ActiveRecord::SessionStore::Session.encrypt_cookie = true + # + # Setting these configuration values has the following behaviors: + # sign_cookie encrypt_cookie Behavior + # ------------- ---------------- ---------------- + # false false Cleartext cookie + # false true Encrypted cookie + # true false Signed cookie + # true true Encrypted cookie + # # The default assumes a +sessions+ tables with columns: # +id+ (numeric primary key), # +session_id+ (string, usually varchar; maximum length is 255), and @@ -74,6 +90,60 @@ def get_session(request, sid) end end + def cookie_is_signed_or_encrypted? + ActiveRecord::SessionStore::Session.sign_cookie || ActiveRecord::SessionStore::Session.encrypt_cookie + end + + # Inspired by ActionDispatch::Session::CookieStore + # https://github.com/rails/rails/blob/main/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb + # (ideally should by DRY in ActionPack by migrating to ActionDispatch::Session::AbstractSecureStore) + # (noting that ActionDispatch::Session::CookieStore does not currently respect :cookie_only) + def extract_session_id(req) + sid = stale_session_check! do + sid_str = unpacked_cookie_data(req) + sid_str && Rack::Session::SessionId.new(sid_str) + end + + # Inspired by Rack::Session::Abstract::Persisted + # https://github.com/rack/rack/blob/abca7d59c566320f1b60d1f5224beac9d201fa3b/lib/rack/session/abstract/id.rb + sid ||= Rack::Session::SessionId.new(req.params[@key]) unless @cookie_only || ! req.params[@key] + sid + end + + # Inspired by ActionDispatch::Session::CookieStore + # https://github.com/rails/rails/blob/main/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb + # (ideally should by DRY in ActionPack by migrating to ActionDispatch::Session::AbstractSecureStore) + def unpacked_cookie_data(req) + + req.fetch_header("action_dispatch.request.unsigned_session_cookie") do |k| + v = stale_session_check! do + get_cookie(req) || nil # not empty string + end + req.set_header k, v + end + end + + # Duplicated from ActionDispatch::Session::CookieStore + # (ideally should by DRY in ActionPack by migrating to ActionDispatch::Session::AbstractSecureStore) + def set_cookie(request, session_id, cookie) + cookie_jar(request)[@key] = cookie + end + + # Duplicated from ActionDispatch::Session::CookieStore + # (ideally should by DRY in ActionPack by migrating to ActionDispatch::Session::AbstractSecureStore) + def get_cookie(req) + cookie_jar(req)[@key] + end + + # Inspired from ActionDispatch::Session::CookieStore + def cookie_jar(request) + if cookie_is_signed_or_encrypted? + request.cookie_jar.signed_or_encrypted + else + request.cookie_jar + end + end + def write_session(request, sid, session_data, options) logger.silence do record, sid = get_session_model(request, sid) diff --git a/lib/active_record/session_store.rb b/lib/active_record/session_store.rb index 966bf4a..b164934 100644 --- a/lib/active_record/session_store.rb +++ b/lib/active_record/session_store.rb @@ -9,7 +9,9 @@ module SessionStore autoload :Session, 'active_record/session_store/session' module ClassMethods # :nodoc: + mattr_accessor :encrypt_cookie mattr_accessor :serializer + mattr_accessor :sign_cookie def serialize(data) serializer_class.dump(data) if data diff --git a/test/action_controller_test.rb b/test/action_controller_test.rb index 06badbd..92a352a 100644 --- a/test/action_controller_test.rb +++ b/test/action_controller_test.rb @@ -1,6 +1,23 @@ require 'helper' class ActionControllerTest < ActionDispatch::IntegrationTest + SessionKey = "_session_id" + SessionSecret = "b3c631c314c0bbca50c1b2843150fe33" + SessionSalt = "signed or encrypted cookie" + + Generator = ActiveSupport::KeyGenerator.new(SessionSecret, iterations: 1000) + Rotations = ActiveSupport::Messages::RotationConfiguration.new + + Verifier = ActiveSupport::MessageVerifier.new( + Generator.generate_key(SessionSalt), serializer: Marshal + ) + + Cipher = "aes-256-gcm" + + Encryptor = ActiveSupport::MessageEncryptor.new( + Generator.generate_key(SessionSalt, 32), cipher: Cipher, serializer: Marshal + ) + class TestController < ActionController::Base protect_from_forgery def no_session_access @@ -38,6 +55,9 @@ def renew def setup ActionDispatch::Session::ActiveRecordStore.session_class.drop_table! rescue nil ActionDispatch::Session::ActiveRecordStore.session_class.create_table! + + ActiveRecord::SessionStore::Session.sign_cookie = false + ActiveRecord::SessionStore::Session.encrypt_cookie = false end %w{ session sql_bypass }.each do |class_name| @@ -127,7 +147,7 @@ def test_getting_session_value_after_session_reset get '/set_session_value' assert_response :success assert cookies['_session_id'] - session_cookie = cookies.send(:hash_for)['_session_id'] + session_cookie = cookies.get_cookie("_session_id") get '/call_reset_session' assert_response :success @@ -163,6 +183,70 @@ def test_getting_session_id end end + def test_signed_cookie + ActiveRecord::SessionStore::Session.sign_cookie = true + with_test_route_set do + get '/set_session_value' + assert_response :success + assert cookies['_session_id'] + session_id_signed = cookies['_session_id'] + + session_id = Verifier.verified(session_id_signed) rescue nil + + get '/get_session_id' + assert_response :success + assert_equal session_id, response.body, "should be able to read signed session id" + end + end + + def test_encrypted_cookie + ActiveRecord::SessionStore::Session.encrypt_cookie = true + with_test_route_set do + get '/set_session_value' + assert_response :success + assert cookies['_session_id'] + session_id_encrypted = cookies['_session_id'] + session_id = Encryptor.decrypt_and_verify(session_id_encrypted) rescue nil + + get '/get_session_id' + assert_response :success + assert_equal session_id, response.body, "should be able to read encrypted session id" + end + end + + # From https://github.com/rails/rails/blob/main/actionpack/test/dispatch/session/cookie_store_test.rb + def test_signed_cookie_disregards_tampered_sessions + ActiveRecord::SessionStore::Session.sign_cookie = true + with_test_route_set do + bad_key = Generator.generate_key(SessionSalt).reverse + + verifier = ActiveSupport::MessageVerifier.new(bad_key, serializer: Marshal) + + cookies[SessionKey] = verifier.generate({ "foo" => "bar", "session_id" => "abc" }) + + get "/get_session_value" + + assert_response :success + assert_equal "foo: nil", response.body + end + end + + # From https://github.com/rails/rails/blob/main/actionpack/test/dispatch/session/cookie_store_test.rb + def test_encrypted_cookie_disregards_tampered_sessions + ActiveRecord::SessionStore::Session.encrypt_cookie = true + with_test_route_set do + + encryptor = ActiveSupport::MessageEncryptor.new("A" * 32, cipher: Cipher, serializer: Marshal) + + cookies[SessionKey] = encryptor.encrypt_and_sign({ "foo" => "bar", "session_id" => "abc" }) + + get "/get_session_value" + + assert_response :success + assert_equal "foo: nil", response.body + end + end + def test_doesnt_write_session_cookie_if_session_id_is_already_exists with_test_route_set do get '/set_session_value' @@ -318,4 +402,45 @@ def test_session_store_with_all_domains end end end + + private + + # Overwrite get to send SessionSecret in env hash + # Inspired by https://github.com/rails/rails/blob/main/actionpack/test/dispatch/session/cookie_store_test.rb + def get(path, **options) + options[:headers] ||= {} + options[:headers].tap do |config| + signed = ActiveRecord::SessionStore::Session.sign_cookie + encrypted = ActiveRecord::SessionStore::Session.encrypt_cookie + aead_mode = (Cipher == "aes-256-gcm") + + if signed || encrypted + config["action_dispatch.key_generator"] ||= Generator + config["action_dispatch.cookies_rotations"] ||= Rotations + end + + if signed && ! encrypted + config["action_dispatch.signed_cookie_salt"] = SessionSalt + + elsif encrypted + config["action_dispatch.secret_key_base"] = SessionSecret + + config["action_dispatch.encrypted_cookie_cipher"] = Cipher + + if aead_mode + config["action_dispatch.authenticated_encrypted_cookie_salt"] = SessionSalt + config["action_dispatch.use_authenticated_cookie_encryption"] = true + + else + config["action_dispatch.encrypted_cookie_salt"] = SessionSalt + config["action_dispatch.encrypted_signed_cookie_salt"] = SessionSalt + config["action_dispatch.use_authenticated_cookie_encryption"] = false + end + end + + end + + super + end + end diff --git a/test/destroy_session_test.rb b/test/destroy_session_test.rb index c8777b6..a97caa1 100644 --- a/test/destroy_session_test.rb +++ b/test/destroy_session_test.rb @@ -11,6 +11,9 @@ def setup @req = ActionDispatch::Request.empty ActionDispatch::Session::ActiveRecordStore.session_class.drop_table! rescue nil ActionDispatch::Session::ActiveRecordStore.session_class.create_table! + + ActiveRecord::SessionStore::Session.sign_cookie = false + ActiveRecord::SessionStore::Session.encrypt_cookie = false end def record_key @@ -50,4 +53,4 @@ def store end end end -end \ No newline at end of file +end diff --git a/test/helper.rb b/test/helper.rb index 20d8b0b..14b0961 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -6,6 +6,7 @@ require 'minitest/autorun' require 'active_record/session_store' +require 'active_support/messages/rotation_configuration' ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:') diff --git a/test/logger_silencer_test.rb b/test/logger_silencer_test.rb index 8462fbb..f19a81f 100644 --- a/test/logger_silencer_test.rb +++ b/test/logger_silencer_test.rb @@ -19,6 +19,9 @@ def setup session_class.drop_table! rescue nil session_class.create_table! ActionDispatch::Session::ActiveRecordStore.session_class = session_class + + ActiveRecord::SessionStore::Session.sign_cookie = false + ActiveRecord::SessionStore::Session.encrypt_cookie = false end %w{ session sql_bypass }.each do |class_name| diff --git a/test/session_test.rb b/test/session_test.rb index e49c76d..f2d35a8 100644 --- a/test/session_test.rb +++ b/test/session_test.rb @@ -14,6 +14,9 @@ def setup Session.drop_table! if Session.table_exists? @session_klass = Class.new(Session) ActiveRecord::SessionStore::Session.serializer = :json + + ActiveRecord::SessionStore::Session.sign_cookie = false + ActiveRecord::SessionStore::Session.encrypt_cookie = false end def test_data_column_name diff --git a/test/sql_bypass_test.rb b/test/sql_bypass_test.rb index 3fc1dc2..6ad3524 100644 --- a/test/sql_bypass_test.rb +++ b/test/sql_bypass_test.rb @@ -16,6 +16,9 @@ def test_create_table assert Session.table_exists? SqlBypass.drop_table! assert !Session.table_exists? + + ActiveRecord::SessionStore::Session.sign_cookie = false + ActiveRecord::SessionStore::Session.encrypt_cookie = false end def test_new_record?