From 7d5a858e5ce54d449066ad0a00917248475fa7f0 Mon Sep 17 00:00:00 2001 From: Tom Kadwill Date: Fri, 2 May 2014 15:54:35 +0100 Subject: [PATCH] Moved 'params[request_forgery_protection_token]' into its own method and improved tests. --- actionpack/CHANGELOG.md | 7 +++++ .../metal/request_forgery_protection.rb | 2 +- .../request_forgery_protection_test.rb | 31 ++++++++++++++++--- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 7427fba5e2..d52ccd3d5e 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,10 @@ +* Moved `params[request_forgery_protection_token]` into its own method + and improved tests. + + Fixes #11316. + + *Tom Kadwill* + * Added verification of route constraints given as a Proc or an object responding to `:matches?`. Previously, when given an non-complying object, it would just silently fail to enforce the constraint. It will now raise an `ArgumentError` diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index e3b1f5ae7c..1355fe87d0 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -247,7 +247,7 @@ module ActionController #:nodoc: # * Does the X-CSRF-Token header match the form_authenticity_token def verified_request? !protect_against_forgery? || request.get? || request.head? || - form_authenticity_token == params[request_forgery_protection_token] || + form_authenticity_token == form_authenticity_param || form_authenticity_token == request.headers['X-CSRF-Token'] end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 5ab5141966..07c2115832 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -462,16 +462,37 @@ end class CustomAuthenticityParamControllerTest < ActionController::TestCase def setup super - ActionController::Base.request_forgery_protection_token = :custom_token_name + @old_logger = ActionController::Base.logger + @logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new + @token = "foobar" + ActionController::Base.request_forgery_protection_token = @token end def teardown - ActionController::Base.request_forgery_protection_token = :authenticity_token + ActionController::Base.request_forgery_protection_token = nil super end - def test_should_allow_custom_token - post :index, :custom_token_name => 'foobar' - assert_response :ok + def test_should_not_warn_if_form_authenticity_param_matches_form_authenticity_token + ActionController::Base.logger = @logger + SecureRandom.stubs(:base64).returns(@token) + + begin + post :index, :custom_token_name => 'foobar' + assert_equal 0, @logger.logged(:warn).size + ensure + ActionController::Base.logger = @old_logger + end + end + + def test_should_warn_if_form_authenticity_param_does_not_match_form_authenticity_token + ActionController::Base.logger = @logger + + begin + post :index, :custom_token_name => 'bazqux' + assert_equal 1, @logger.logged(:warn).size + ensure + ActionController::Base.logger = @old_logger + end end end