Change the forgery token implementation to just be a simple random string.

This deprecates the use of :secret and :digest which were only needed when we were hashing session ids.

3 files changed, 15 insertions, 127 deletions

diff --git a/actionpack/lib/action_controller/request_forgery_protection.rb b/actionpack/lib/action_controller/request_forgery_protection.rb
index 3e0e94a06b..f3e6288c26 100644
--- a/actionpack/lib/action_controller/request_forgery_protection.rb
+++ b/actionpack/lib/action_controller/request_forgery_protection.rb
@@ -5,8 +5,6 @@ module ActionController #:nodoc:
   module RequestForgeryProtection
     def self.included(base)
       base.class_eval do
-        class_inheritable_accessor :request_forgery_protection_options
-        self.request_forgery_protection_options = {}
         helper_method :form_authenticity_token
         helper_method :protect_against_forgery?
       end
@@ -14,7 +12,7 @@ module ActionController #:nodoc:
     end
     
     # Protecting controller actions from CSRF attacks by ensuring that all forms are coming from the current web application, not a
-    # forged link from another site, is done by embedding a token based on the session (which an attacker wouldn't know) in all
+    # forged link from another site, is done by embedding a token based on a random string stored in the session (which an attacker wouldn't know) in all
     # forms and Ajax requests generated by Rails and then verifying the authenticity of that token in the controller.  Only
     # HTML/JavaScript requests are checked, so this will not protect your XML API (presumably you'll have a different authentication
     # scheme there anyway).  Also, GET requests are not protected as these should be idempotent anyway.
@@ -57,12 +55,8 @@ module ActionController #:nodoc:
       # Example:
       #
       #   class FooController < ApplicationController
-      #     # uses the cookie session store (then you don't need a separate :secret)
       #     protect_from_forgery :except => :index
       #
-      #     # uses one of the other session stores that uses a session_id value.
-      #     protect_from_forgery :secret => 'my-little-pony', :except => :index
-      #
       #     # you can disable csrf protection on controller-by-controller basis:
       #     skip_before_filter :verify_authenticity_token
       #   end
@@ -70,13 +64,12 @@ module ActionController #:nodoc:
       # Valid Options:
       #
       # * <tt>:only/:except</tt> - Passed to the <tt>before_filter</tt> call.  Set which actions are verified.
-      # * <tt>:secret</tt> - Custom salt used to generate the <tt>form_authenticity_token</tt>.
-      #   Leave this off if you are using the cookie session store.
-      # * <tt>:digest</tt> - Message digest used for hashing.  Defaults to 'SHA1'.
       def protect_from_forgery(options = {})
         self.request_forgery_protection_token ||= :authenticity_token
         before_filter :verify_authenticity_token, :only => options.delete(:only), :except => options.delete(:except)
-        request_forgery_protection_options.update(options)
+        if options[:secret] || options[:digest]
+          ActiveSupport::Deprecation.warn("protect_from_forgery only takes :only and :except options now. :digest and :secret have no effect", caller)
+        end
       end
     end
 
@@ -90,7 +83,7 @@ module ActionController #:nodoc:
       #
       # * is the format restricted?  By default, only HTML and AJAX requests are checked.
       # * is it a GET request?  Gets should be safe and idempotent
-      # * Does the form_authenticity_token match the given _token value from the params?
+      # * Does the form_authenticity_token match the given token value from the params?
       def verified_request?
         !protect_against_forgery?     ||
           request.method == :get      ||
@@ -105,34 +98,9 @@ module ActionController #:nodoc:
       # Sets the token value for the current session.  Pass a <tt>:secret</tt> option
       # in +protect_from_forgery+ to add a custom salt to the hash.
       def form_authenticity_token
-        @form_authenticity_token ||= if !session.respond_to?(:session_id)
-          raise InvalidAuthenticityToken, "Request Forgery Protection requires a valid session.  Use #allow_forgery_protection to disable it, or use a valid session."
-        elsif request_forgery_protection_options[:secret]
-          authenticity_token_from_session_id
-        elsif session.respond_to?(:dbman) && session.dbman.respond_to?(:generate_digest)
-          authenticity_token_from_cookie_session
-        else
-          raise InvalidAuthenticityToken, "No :secret given to the #protect_from_forgery call.  Set that or use a session store capable of generating its own keys (Cookie Session Store)."
-        end
-      end
-      
-      # Generates a unique digest using the session_id and the CSRF secret.
-      def authenticity_token_from_session_id
-        key = if request_forgery_protection_options[:secret].respond_to?(:call)
-          request_forgery_protection_options[:secret].call(@session)
-        else
-          request_forgery_protection_options[:secret]
-        end
-        digest = request_forgery_protection_options[:digest] ||= 'SHA1'
-        OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new(digest), key.to_s, session.session_id.to_s)
-      end
-      
-      # No secret was given, so assume this is a cookie session store.
-      def authenticity_token_from_cookie_session
-        session[:csrf_id] ||= CGI::Session.generate_unique_id
-        session.dbman.generate_digest(session[:csrf_id])
+        session[:_csrf_token] ||= ActiveSupport::SecureRandom.base64(32)
       end
-      
+
       def protect_against_forgery?
         allow_forgery_protection && request_forgery_protection_token
       end
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index d5320596d5..972e425e35 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -883,12 +883,13 @@ class RenderTest < ActionController::TestCase
   end
 
   def test_enum_rjs_test
+    ActiveSupport::SecureRandom.stubs(:base64).returns("asdf")
     get :enum_rjs_test
     body = %{
       $$(".product").each(function(value, index) {
       new Effect.Highlight(element,{});
       new Effect.Highlight(value,{});
-      Sortable.create(value, {onUpdate:function(){new Ajax.Request('/test/order', {asynchronous:true, evalScripts:true, parameters:Sortable.serialize(value)})}});
+      Sortable.create(value, {onUpdate:function(){new Ajax.Request('/test/order', {asynchronous:true, evalScripts:true, parameters:Sortable.serialize(value) + '&authenticity_token=' + encodeURIComponent('asdf')})}});
       new Draggable(value, {});
       });
     }.gsub(/^      /, '').strip
diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb
index 00c6bc0ab0..ef0bf5fd08 100644
--- a/actionpack/test/controller/request_forgery_protection_test.rb
+++ b/actionpack/test/controller/request_forgery_protection_test.rb
@@ -5,13 +5,6 @@ ActionController::Routing::Routes.draw do |map|
   map.connect ':controller/:action/:id'
 end
 
-# simulates cookie session store
-class FakeSessionDbMan
-  def self.generate_digest(data)
-    Digest::SHA1.hexdigest("secure")
-  end
-end
-
 # common controller actions
 module RequestForgeryProtectionActions
   def index
@@ -36,29 +29,10 @@ end
 # sample controllers
 class RequestForgeryProtectionController < ActionController::Base
   include RequestForgeryProtectionActions
-  protect_from_forgery :only => :index, :secret => 'abc'
-end
-
-class RequestForgeryProtectionWithoutSecretController < ActionController::Base
-  include RequestForgeryProtectionActions
-  protect_from_forgery
-end
-
-# no token is given, assume the cookie store is used
-class CsrfCookieMonsterController < ActionController::Base
-  include RequestForgeryProtectionActions
   protect_from_forgery :only => :index
 end
 
-# sessions are turned off
-class SessionOffController < ActionController::Base
-  protect_from_forgery :secret => 'foobar'
-  session :off
-  def rescue_action(e) raise e end
-  include RequestForgeryProtectionActions
-end
-
-class FreeCookieController < CsrfCookieMonsterController
+class FreeCookieController < RequestForgeryProtectionController
   self.allow_forgery_protection = false
   
   def index
@@ -237,45 +211,9 @@ class RequestForgeryProtectionControllerTest < ActionController::TestCase
     @request    = ActionController::TestRequest.new
     @request.format = :html
     @response   = ActionController::TestResponse.new
-    class << @request.session
-      def session_id() '123' end
-    end
-    @token = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new('SHA1'), 'abc', '123')
-    ActionController::Base.request_forgery_protection_token = :authenticity_token
-  end
-end
+    @token      = "cf50faa3fe97702ca1ae"
 
-class RequestForgeryProtectionWithoutSecretControllerTest < ActionController::TestCase
-  def setup
-    @controller = RequestForgeryProtectionWithoutSecretController.new
-    @request    = ActionController::TestRequest.new
-    @response   = ActionController::TestResponse.new
-    class << @request.session
-      def session_id() '123' end
-    end
-    @token = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new('SHA1'), 'abc', '123')
-    ActionController::Base.request_forgery_protection_token = :authenticity_token
-  end
-  
-  # def test_should_raise_error_without_secret
-  #   assert_raises ActionController::InvalidAuthenticityToken do
-  #     get :index
-  #   end
-  # end
-end
-
-class CsrfCookieMonsterControllerTest < ActionController::TestCase
-  include RequestForgeryProtectionTests
-  def setup
-    @controller = CsrfCookieMonsterController.new
-    @request    = ActionController::TestRequest.new
-    @response   = ActionController::TestResponse.new
-    class << @request.session
-      attr_accessor :dbman
-    end
-    # simulate a cookie session store
-    @request.session.dbman = FakeSessionDbMan
-    @token = Digest::SHA1.hexdigest("secure")
+    ActiveSupport::SecureRandom.stubs(:base64).returns(@token)
     ActionController::Base.request_forgery_protection_token = :authenticity_token
   end
 end
@@ -285,7 +223,9 @@ class FreeCookieControllerTest < ActionController::TestCase
     @controller = FreeCookieController.new
     @request    = ActionController::TestRequest.new
     @response   = ActionController::TestResponse.new
-    @token      = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new('SHA1'), 'abc', '123')
+    @token      = "cf50faa3fe97702ca1ae"
+
+    ActiveSupport::SecureRandom.stubs(:base64).returns(@token)
   end
   
   def test_should_not_render_form_with_token_tag
@@ -304,24 +244,3 @@ class FreeCookieControllerTest < ActionController::TestCase
     end
   end
 end
-
-class SessionOffControllerTest < ActionController::TestCase
-  def setup
-    @controller = SessionOffController.new
-    @request    = ActionController::TestRequest.new
-    @response   = ActionController::TestResponse.new
-    @token      = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new('SHA1'), 'abc', '123')
-  end
-
-  # TODO: Rewrite this test.
-  # This test was passing but for the wrong reason.
-  # Sessions aren't really being turned off, so an exception was raised
-  # because sessions weren't on - not because the token didn't match.
-  #
-  # def test_should_raise_correct_exception
-  #   @request.session = {} # session(:off) doesn't appear to work with controller tests
-  #   assert_raises(ActionController::InvalidAuthenticityToken) do
-  #     post :index, :authenticity_token => @token, :format => :html
-  #   end
-  # end
-end