Refactor cookie_only option to survive multiple requests and add regression tests. References #10048. [theflow]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@8176 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

2 files changed, 95 insertions, 3 deletions

diff --git a/actionpack/lib/action_controller/cgi_process.rb b/actionpack/lib/action_controller/cgi_process.rb
index b3739ce399..6a802aa8fa 100644
--- a/actionpack/lib/action_controller/cgi_process.rb
+++ b/actionpack/lib/action_controller/cgi_process.rb
@@ -33,13 +33,14 @@ module ActionController #:nodoc:
   end
 
   class CgiRequest < AbstractRequest #:nodoc:
-    attr_accessor :cgi, :session_options, :cookie_only
+    attr_accessor :cgi, :session_options
     class SessionFixationAttempt < StandardError; end #:nodoc:
 
     DEFAULT_SESSION_OPTIONS = {
       :database_manager => CGI::Session::CookieStore, # store data in cookie
       :prefix           => "ruby_sess.",    # prefix session file names
       :session_path     => "/",             # available to all paths in app
+      :session_key      => "_session_id",
       :cookie_only      => true
     } unless const_defined?(:DEFAULT_SESSION_OPTIONS)
 
@@ -47,7 +48,6 @@ module ActionController #:nodoc:
       @cgi = cgi
       @session_options = session_options
       @env = @cgi.send!(:env_table)
-      @cookie_only = session_options.delete :cookie_only
       super()
     end
 
@@ -112,7 +112,7 @@ module ActionController #:nodoc:
           @session = Hash.new
         else
           stale_session_check! do
-            if @cookie_only && request_parameters[session_options_with_string_keys['session_key']]
+            if cookie_only? && query_parameters[session_options_with_string_keys['session_key']]
               raise SessionFixationAttempt
             end
             case value = session_options_with_string_keys['new_session']
@@ -158,6 +158,10 @@ module ActionController #:nodoc:
         end
       end
 
+      def cookie_only?
+        session_options_with_string_keys['cookie_only']
+      end
+
       def stale_session_check!
         yield
       rescue ArgumentError => argument_error
diff --git a/actionpack/test/controller/session_fixation_test.rb b/actionpack/test/controller/session_fixation_test.rb
new file mode 100644
index 0000000000..0b0dce770e
--- /dev/null
+++ b/actionpack/test/controller/session_fixation_test.rb
@@ -0,0 +1,88 @@
+require File.dirname(__FILE__) + '/../abstract_unit'
+
+class SessionFixationTest < Test::Unit::TestCase
+  class MockCGI < CGI #:nodoc:
+    attr_accessor :stdoutput, :env_table
+
+    def initialize(env, data = '')
+      self.env_table = env
+      self.stdoutput = StringIO.new
+      super(nil, StringIO.new(data))
+    end
+  end
+
+  class TestController < ActionController::Base
+    session :session_key => '_myapp_session_id', :secret => 'secret', :except => :default_session_key
+    session :cookie_only => false, :only => :allow_session_fixation
+
+    def default_session_key
+      render :text => "default_session_key"
+    end
+
+    def custom_session_key
+      render :text => "custom_session_key: #{params[:id]}"
+    end
+
+    def allow_session_fixation
+      render :text => "allow_session_fixation"
+    end
+
+    def rescue_action(e) raise end
+  end
+
+  def setup
+    @controller = TestController.new
+  end
+
+  def test_should_be_able_to_make_a_successful_request
+    cgi = mock_cgi_for_request_to(:custom_session_key, :id => 1)
+
+    assert_nothing_raised do
+      @controller.send(:process, ActionController::CgiRequest.new(cgi, {}), ActionController::CgiResponse.new(cgi))
+    end
+    assert_equal 'custom_session_key: 1', @controller.response.body
+    assert_not_nil @controller.session
+  end
+
+  def test_should_catch_session_fixation_attempt
+    cgi = mock_cgi_for_request_to(:custom_session_key, :_myapp_session_id => 42)
+
+    assert_raises ActionController::CgiRequest::SessionFixationAttempt do
+      @controller.send(:process, ActionController::CgiRequest.new(cgi, {}), ActionController::CgiResponse.new(cgi))
+    end
+    assert_nil @controller.session
+  end
+
+  def test_should_not_catch_session_fixation_attempt_when_cookie_only_setting_is_disabled
+    cgi = mock_cgi_for_request_to(:allow_session_fixation, :_myapp_session_id => 42)
+
+    assert_nothing_raised do
+      @controller.send(:process, ActionController::CgiRequest.new(cgi, {}), ActionController::CgiResponse.new(cgi))
+    end
+    assert ! @controller.response.body.blank?
+    assert_not_nil @controller.session
+  end
+
+  def test_should_catch_session_fixation_attempt_with_default_session_key
+    ActionController::Base.session_store = :p_store # using the default session_key is not possible with cookie store
+    cgi = mock_cgi_for_request_to(:default_session_key, :_session_id => 42)
+
+    assert_raises ActionController::CgiRequest::SessionFixationAttempt do
+      @controller.send(:process, ActionController::CgiRequest.new(cgi, {}), ActionController::CgiResponse.new(cgi))
+    end
+    assert @controller.response.body.blank?
+    assert_nil @controller.session
+  end
+
+private
+
+  def mock_cgi_for_request_to(action, params = {})
+    MockCGI.new({
+      "REQUEST_METHOD" => "GET",
+      "QUERY_STRING"   => "action=#{action}&#{params.to_query}",
+      "REQUEST_URI"    => "/",
+      "SERVER_PORT"    => "80",
+      "HTTP_HOST"      => "testdomain.com" }, '')
+  end
+
+end