From d6e9c430903825b1dfad583daee465b3f72dc738 Mon Sep 17 00:00:00 2001 From: W. Andrew Loe III Date: Mon, 13 Sep 2010 14:29:25 -0700 Subject: [PATCH] Only send secure cookies over SSL. --- .../middleware/session/abstract_store.rb | 5 ++++- actionpack/test/dispatch/cookies_test.rb | 11 +++++++++++ .../test/dispatch/session/cookie_store_test.rb | 17 +++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index dd82294..348a2d1 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -152,6 +152,10 @@ module ActionDispatch options = env[ENV_SESSION_OPTIONS_KEY] if !session_data.is_a?(AbstractStore::SessionHash) || session_data.loaded? || options[:expire_after] + request = ActionDispatch::Request.new(env) + + return response if (options[:secure] && !request.ssl?) + session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.loaded? sid = options[:id] || generate_sid @@ -165,7 +169,6 @@ module ActionDispatch cookie[:expires] = Time.now + options.delete(:expire_after) end - request = ActionDispatch::Request.new(env) set_cookie(request, cookie.merge!(options)) end diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index cadae5d..360fb35 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -47,6 +47,11 @@ class CookiesTest < ActionController::TestCase cookies["user_name"] = { :value => "david", :httponly => true } head :ok end + + def authenticate_with_secure + cookies["user_name"] = { :value => "david", :secure => true } + head :ok + end def set_permanent_cookie cookies.permanent[:user_name] = "Jamie" @@ -128,6 +133,12 @@ class CookiesTest < ActionController::TestCase assert_cookie_header "user_name=david; path=/; HttpOnly" assert_equal({"user_name" => "david"}, @response.cookies) end + + def test_setting_cookie_with_secure + get :authenticate_with_secure + assert_cookie_header "user_name=david; path=/; secure" + assert_equal({"user_name" => "david"}, @response.cookies) + end def test_multiple_cookies get :set_multiple_cookies diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index 496fa69..dd580f0 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -105,6 +105,23 @@ class CookieStoreTest < ActionController::IntegrationTest assert_equal 'foo: nil', response.body end end + + def test_does_not_set_secure_cookies_over_http + with_test_route_set(:secure => true) do + get '/set_session_value' + assert_response :success + assert_equal nil, headers['Set-Cookie'] + end + end + + def test_does_set_secure_cookies_over_https + with_test_route_set(:secure => true) do + get '/set_session_value', nil, 'HTTPS' => 'on' + assert_response :success + assert_equal "_myapp_session=#{response.body}; path=/; secure; HttpOnly", + headers['Set-Cookie'] + end + end # {:foo=>#, :session_id=>"ce8b0752a6ab7c7af3cdb8a80e6b9e46"} SignedSerializedCookie = "BAh7BzoIZm9vbzodU2Vzc2lvbkF1dG9sb2FkVGVzdDo6Rm9vBjoJQGJhciIIYmF6Og9zZXNzaW9uX2lkIiVjZThiMDc1MmE2YWI3YzdhZjNjZGI4YTgwZTZiOWU0Ng==--2bf3af1ae8bd4e52b9ac2099258ace0c380e601c" -- 1.7.2.3