Simplify cookie_store by simply relying on cookies.signed.

4 files changed, 85 insertions, 48 deletions

diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb
index 479e62b23d..d2e5d2e965 100644
--- a/actionpack/test/abstract_unit.rb
+++ b/actionpack/test/abstract_unit.rb
@@ -162,6 +162,7 @@ class ActionController::IntegrationTest < ActiveSupport::TestCase
       middleware.use "ActionDispatch::Cookies"
       middleware.use "ActionDispatch::Flash"
       middleware.use "ActionDispatch::Head"
+      yield(middleware) if block_given?
     end
   end
 
diff --git a/actionpack/test/controller/cookie_test.rb b/actionpack/test/controller/cookie_test.rb
index 4971866e7c..f65eda5c69 100644
--- a/actionpack/test/controller/cookie_test.rb
+++ b/actionpack/test/controller/cookie_test.rb
@@ -58,6 +58,17 @@ class CookieTest < ActionController::TestCase
       head :ok
     end
 
+    def raise_data_overflow
+      cookies.signed[:foo] = 'bye!' * 1024
+      head :ok
+    end
+
+    def tampered_cookies
+      cookies[:tampered] = "BAh7BjoIZm9vIghiYXI%3D--123456780"
+      cookies.signed[:tampered]
+      head :ok
+    end
+
     def set_permanent_signed_cookie
       cookies.permanent.signed[:remember_me] = 100
       head :ok
@@ -74,7 +85,7 @@ class CookieTest < ActionController::TestCase
 
   def setup
     super
-    @request.env["action_dispatch.secret_token"] = "thisISverySECRET123"
+    @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"
     @request.host = "www.nextangle.com"
   end
 
@@ -163,6 +174,48 @@ class CookieTest < ActionController::TestCase
     assert_equal({"user_name" => "david"}, @response.cookies)
   end
 
+  def test_raise_data_overflow
+    assert_raise(ActionDispatch::Cookies::CookieOverflow) do
+      get :raise_data_overflow
+    end
+  end
+
+  def test_tampered_cookies
+    assert_nothing_raised do
+      get :tampered_cookies
+      assert_response :success
+    end
+  end
+
+  def test_raises_argument_error_if_missing_secret
+    assert_raise(ArgumentError, nil.inspect) {
+      @request.env["action_dispatch.secret_token"] = nil
+      get :set_signed_cookie
+    }
+
+    assert_raise(ArgumentError, ''.inspect) {
+      @request.env["action_dispatch.secret_token"] = ""
+      get :set_signed_cookie
+    }
+  end
+
+  def test_raises_argument_error_if_secret_is_probably_insecure
+    assert_raise(ArgumentError, "password".inspect) {
+      @request.env["action_dispatch.secret_token"] = "password"
+      get :set_signed_cookie
+    }
+
+    assert_raise(ArgumentError, "secret".inspect) {
+      @request.env["action_dispatch.secret_token"] = "secret"
+      get :set_signed_cookie
+    }
+
+    assert_raise(ArgumentError, "12345678901234567890123456789".inspect) {
+      @request.env["action_dispatch.secret_token"] = "12345678901234567890123456789"
+      get :set_signed_cookie
+    }
+  end
+
   private
     def assert_cookie_header(expected)
       header = @response.headers["Set-Cookie"]
diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb
index c662ce264b..01c8fd90a5 100644
--- a/actionpack/test/controller/flash_test.rb
+++ b/actionpack/test/controller/flash_test.rb
@@ -237,10 +237,19 @@ class FlashIntegrationTest < ActionController::IntegrationTest
   end
 
   private
+
+    # Overwrite get to send SessionSecret in env hash
+    def get(path, parameters = nil, env = {})
+      env["action_dispatch.secret_token"] ||= SessionSecret
+      super
+    end
+
     def with_test_route_set
       with_routing do |set|
         set.draw do |map|
-          match ':action', :to => ActionDispatch::Session::CookieStore.new(FlashIntegrationTest::TestController, :key => FlashIntegrationTest::SessionKey, :secret => FlashIntegrationTest::SessionSecret)
+          match ':action', :to => ActionDispatch::Session::CookieStore.new(
+            FlashIntegrationTest::TestController, :key => SessionKey, :secret => SessionSecret
+          )
         end
         yield
       end
diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb
index d2c1758af1..a6cdbf8032 100644
--- a/actionpack/test/dispatch/session/cookie_store_test.rb
+++ b/actionpack/test/dispatch/session/cookie_store_test.rb
@@ -55,42 +55,13 @@ class CookieStoreTest < ActionController::IntegrationTest
     }
   end
 
-  def test_raises_argument_error_if_missing_secret
-    assert_raise(ArgumentError, nil.inspect) {
-      ActionDispatch::Session::CookieStore.new(nil,
-       :key => SessionKey, :secret => nil)
-    }
-
-    assert_raise(ArgumentError, ''.inspect) {
-      ActionDispatch::Session::CookieStore.new(nil,
-       :key => SessionKey, :secret => '')
-    }
-  end
-
-  def test_raises_argument_error_if_secret_is_probably_insecure
-    assert_raise(ArgumentError, "password".inspect) {
-      ActionDispatch::Session::CookieStore.new(nil,
-       :key => SessionKey, :secret => "password")
-    }
-
-    assert_raise(ArgumentError, "secret".inspect) {
-      ActionDispatch::Session::CookieStore.new(nil,
-       :key => SessionKey, :secret => "secret")
-    }
-
-    assert_raise(ArgumentError, "12345678901234567890123456789".inspect) {
-      ActionDispatch::Session::CookieStore.new(nil,
-       :key => SessionKey, :secret => "12345678901234567890123456789")
-    }
-  end
-
   def test_setting_session_value
     with_test_route_set do
       get '/set_session_value'
       assert_response :success
       assert_equal "_myapp_session=#{response.body}; path=/; HttpOnly",
         headers['Set-Cookie']
-   end
+    end
   end
 
   def test_getting_session_value
@@ -99,7 +70,7 @@ class CookieStoreTest < ActionController::IntegrationTest
       get '/get_session_value'
       assert_response :success
       assert_equal 'foo: "bar"', response.body
-   end
+    end
   end
 
   def test_getting_session_id
@@ -127,7 +98,7 @@ class CookieStoreTest < ActionController::IntegrationTest
 
   def test_close_raises_when_data_overflows
     with_test_route_set do
-      assert_raise(ActionDispatch::Session::CookieStore::CookieOverflow) {
+      assert_raise(ActionDispatch::Cookies::CookieOverflow) {
         get '/raise_data_overflow'
       }
     end
@@ -209,30 +180,33 @@ class CookieStoreTest < ActionController::IntegrationTest
       get '/no_session_access'
       assert_response :success
 
-      # Mystery bug that came up in 2.3 as well. What is this trying to test?!
-      # assert_equal "_myapp_session=#{cookie_body}; path=/; expires=#{expected_expiry}; HttpOnly",
-      #   headers['Set-Cookie']
+      assert_equal "_myapp_session=#{cookie_body}; path=/; expires=#{expected_expiry}; HttpOnly",
+        headers['Set-Cookie']
     end
   end
 
   private
+
+    # Overwrite get to send SessionSecret in env hash
+    def get(path, parameters = nil, env = {})
+      env["action_dispatch.secret_token"] ||= SessionSecret
+      super
+    end
+
     def with_test_route_set(options = {})
       with_routing do |set|
         set.draw do |map|
           match ':action', :to => ::CookieStoreTest::TestController
         end
-        options = {:key => SessionKey, :secret => SessionSecret}.merge(options)
-        @app = ActionDispatch::Session::CookieStore.new(set, options)
+
+        options = { :key => SessionKey, :secret => SessionSecret }.merge!(options)
+
+        @app = self.class.build_app(set) do |middleware|
+          middleware.use ActionDispatch::Session::CookieStore, options
+          middleware.delete "ActionDispatch::ShowExceptions"
+        end
+
         yield
       end
     end
-
-    def unmarshal_session(cookie_string)
-      session = Rack::Utils.parse_query(cookie_string, ';,').inject({}) {|h,(k,v)|
-        h[k] = Array === v ? v.first : v
-        h
-      }[SessionKey]
-      verifier = ActiveSupport::MessageVerifier.new(SessionSecret, 'SHA1')
-      verifier.verify(session)
-    end
 end