From 92203d754f535c01c5ec3175627425d20e3d2839 Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Thu, 18 Feb 2016 17:38:19 +0530 Subject: Fixed passing of delete method on button_to tag, creating wrong form csrf token Fixes #23524 --- .../controller/request_forgery_protection_test.rb | 44 ++++++++++++++++++++++ 1 file changed, 44 insertions(+) (limited to 'actionpack') diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 1984ad8825..6dc4f3fe51 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -136,6 +136,10 @@ class PerFormTokensController < ActionController::Base render inline: "<%= form_tag (params[:form_path] || '/per_form_tokens/post_one'), method: (params[:form_method] || :post) %>" end + def button_to + render inline: "<%= button_to 'Button', (params[:form_path] || '/per_form_tokens/post_one'), method: (params[:form_method] || :post) %>" + end + def post_one render plain: '' end @@ -710,6 +714,46 @@ class PerFormTokensControllerTest < ActionController::TestCase end end + def test_rejects_token_for_incorrect_method_button_to + get :button_to, params: { form_method: 'delete' } + + form_token = nil + assert_select 'input[name=custom_authenticity_token]' do |elts| + form_token = elts.first['value'] + assert_not_nil form_token + end + + actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) + expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'delete') + assert_equal expected, actual + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one' + assert_raises(ActionController::InvalidAuthenticityToken) do + patch :post_one, params: { custom_authenticity_token: form_token } + end + end + + def test_accepts_proper_token_for_delete_method_button_to + get :button_to, params: { form_method: 'delete' } + + form_token = nil + assert_select 'input[name=custom_authenticity_token]' do |elts| + form_token = elts.first['value'] + assert_not_nil form_token + end + + actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) + expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'delete') + assert_equal expected, actual + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one' + assert_nothing_raised do + delete :post_one, params: { custom_authenticity_token: form_token } + end + end + def test_accepts_global_csrf_token get :index -- cgit v1.2.3 From 2b4c0ae144768d72f042b2c2ec1bca4df386fb6f Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Mon, 22 Feb 2016 00:21:46 +0530 Subject: Refactored Request Forgery CSRF PerFormTokensController tests and DRY'ed them up. --- .../controller/request_forgery_protection_test.rb | 108 ++++++++------------- 1 file changed, 38 insertions(+), 70 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 6dc4f3fe51..c645af88d7 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -656,15 +656,9 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_accepts_token_for_correct_path_and_method get :index - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token - actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) - expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') - assert_equal expected, actual + assert_matches_session_token_on_server form_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one' @@ -677,15 +671,9 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_rejects_token_for_incorrect_path get :index - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token - actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) - expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') - assert_equal expected, actual + assert_matches_session_token_on_server form_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_two' @@ -697,15 +685,9 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_rejects_token_for_incorrect_method get :index - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token - actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) - expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') - assert_equal expected, actual + assert_matches_session_token_on_server form_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one' @@ -717,15 +699,9 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_rejects_token_for_incorrect_method_button_to get :button_to, params: { form_method: 'delete' } - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token - actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) - expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'delete') - assert_equal expected, actual + assert_matches_session_token_on_server form_token, 'delete' # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one' @@ -734,23 +710,19 @@ class PerFormTokensControllerTest < ActionController::TestCase end end - def test_accepts_proper_token_for_delete_method_button_to - get :button_to, params: { form_method: 'delete' } + %w{delete post patch}.each do |verb| + test "Accepts proper token for #{verb} method on button_to tag" do + get :button_to, params: { form_method: verb } - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token - actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) - expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'delete') - assert_equal expected, actual + assert_matches_session_token_on_server form_token, verb - # This is required because PATH_INFO isn't reset between requests. - @request.env['PATH_INFO'] = '/per_form_tokens/post_one' - assert_nothing_raised do - delete :post_one, params: { custom_authenticity_token: form_token } + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one' + assert_nothing_raised do + send verb, :post_one, params: { custom_authenticity_token: form_token } + end end end @@ -770,15 +742,9 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_ignores_params get :index, params: {form_path: '/per_form_tokens/post_one?foo=bar'} - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token - actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) - expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') - assert_equal expected, actual + assert_matches_session_token_on_server form_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one?foo=baz' @@ -791,11 +757,7 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_ignores_trailing_slash_during_generation get :index, params: {form_path: '/per_form_tokens/post_one/'} - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one' @@ -808,11 +770,7 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_ignores_trailing_slash_during_validation get :index - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one/' @@ -825,12 +783,7 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_method_is_case_insensitive get :index, params: {form_method: "POST"} - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end - + form_token = assert_presence_and_fetch_form_csrf_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one/' assert_nothing_raised do @@ -838,4 +791,19 @@ class PerFormTokensControllerTest < ActionController::TestCase end assert_response :success end + + private + def assert_presence_and_fetch_form_csrf_token + assert_select 'input[name="custom_authenticity_token"]' do |input| + form_csrf_token = input.first['value'] + assert_not_nil form_csrf_token + return form_csrf_token + end + end + + def assert_matches_session_token_on_server(form_token, method = 'post') + actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) + expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', method) + assert_equal expected, actual + end end -- cgit v1.2.3