Skip to content
Browse files

Only send secure cookies over SSL.

  • Loading branch information...
1 parent 8c049c6 commit 17f2fb44c0b0a836a961f02fc292ae90f06a67dc @loe loe committed with tenderlove Sep 13, 2010
View
6 actionpack/lib/action_controller/session/abstract_store.rb
@@ -180,6 +180,10 @@ def call(env)
options = env[ENV_SESSION_OPTIONS_KEY]
if !session_data.is_a?(AbstractStore::SessionHash) || session_data.loaded? || options[:expire_after]
+ request = ActionController::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
@@ -198,7 +202,7 @@ def call(env)
expiry = Time.now + options[:expire_after]
cookie << "; expires=#{expiry.httpdate}"
end
- cookie << "; Secure" if options[:secure]
+ cookie << "; secure" if options[:secure]
cookie << "; HttpOnly" if options[:httponly]
headers = response[1]
View
5 actionpack/lib/action_controller/session/cookie_store.rb
@@ -101,8 +101,9 @@ def call(env)
session_data = env[ENV_SESSION_KEY]
options = env[ENV_SESSION_OPTIONS_KEY]
-
- if !session_data.is_a?(AbstractStore::SessionHash) || session_data.loaded? || options[:expire_after]
+ request = ActionController::Request.new(env)
+
+ if !(options[:secure] && !request.ssl?) && (!session_data.is_a?(AbstractStore::SessionHash) || session_data.loaded? || options[:expire_after])
session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.loaded?
persistent_session_id!(session_data)
View
10 actionpack/test/controller/cookie_test.rb
@@ -42,6 +42,10 @@ def authenticate_with_http_only
cookies["user_name"] = { :value => "david", :httponly => true }
end
+ def authenticate_with_secure
+ cookies["user_name"] = { :value => "david", :secure => true }
+ end
+
def set_permanent_cookie
cookies.permanent[:user_name] = "Jamie"
end
@@ -94,6 +98,12 @@ def test_setting_cookie_with_http_only
assert_equal ["user_name=david; path=/; HttpOnly"], @response.headers["Set-Cookie"]
assert_equal({"user_name" => "david"}, @response.cookies)
end
+
+ def test_setting_cookie_with_secure
+ get :authenticate_with_secure
+ assert_equal ["user_name=david; path=/; secure"], @response.headers["Set-Cookie"]
+ assert_equal({"user_name" => "david"}, @response.cookies)
+ end
def test_multiple_cookies
get :set_multiple_cookies
View
28 actionpack/test/controller/session/cookie_store_test.rb
@@ -6,7 +6,6 @@ class CookieStoreTest < ActionController::IntegrationTest
SessionSecret = 'b3c631c314c0bbca50c1b2843150fe33'
DispatcherApp = ActionController::Dispatcher.new
- CookieStoreApp = ActionController::Session::CookieStore.new(DispatcherApp, :key => SessionKey, :secret => SessionSecret)
Verifier = ActiveSupport::MessageVerifier.new(SessionSecret, 'SHA1')
@@ -62,10 +61,6 @@ def set_session_value_and_cookie
def rescue_action(e) raise end
end
- def setup
- @integration_session = open_session(CookieStoreApp)
- end
-
def test_raises_argument_error_if_missing_session_key
assert_raise(ArgumentError, nil.inspect) {
ActionController::Session::CookieStore.new(nil,
@@ -152,6 +147,23 @@ def test_disregards_tampered_sessions
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
+
def test_close_raises_when_data_overflows
with_test_route_set do
assert_raise(ActionController::Session::CookieStore::CookieOverflow) {
@@ -272,13 +284,17 @@ def test_setting_session_value_and_cookie
end
private
- def with_test_route_set
+ def with_test_route_set(options = {})
with_routing do |set|
set.draw do |map|
map.with_options :controller => "cookie_store_test/test" do |c|
c.connect "/:action"
end
end
+
+ options = { :key => SessionKey, :secret => SessionSecret }.merge!(options)
+ @integration_session = open_session(ActionController::Session::CookieStore.new(DispatcherApp, options))
+
yield
end
end
View
15 actionpack/test/controller/session/mem_cache_store_test.rb
@@ -37,13 +37,6 @@ def rescue_action(e) raise end
begin
DispatcherApp = ActionController::Dispatcher.new
- MemCacheStoreApp = ActionController::Session::MemCacheStore.new(
- DispatcherApp, :key => '_session_id')
-
-
- def setup
- @integration_session = open_session(MemCacheStoreApp)
- end
def test_setting_and_getting_session_value
with_test_route_set do
@@ -177,14 +170,18 @@ def test_prevents_session_fixation
end
private
- def with_test_route_set
+ def with_test_route_set(options = {})
with_routing do |set|
set.draw do |map|
map.with_options :controller => "mem_cache_store_test/test" do |c|
c.connect "/:action"
end
end
+
+ options = { :key => '_session_id' }.merge!(options)
+ @integration_session = open_session(ActionController::Session::MemCacheStore.new(DispatcherApp, options))
+
yield
end
end
-end
+end

0 comments on commit 17f2fb4

Please sign in to comment.
Something went wrong with that request. Please try again.