Merge pull request #23752 from vipulnsward/23524-fix-button_to_delete

Fixed passing of delete method on button_to tag, creating wrong form csrf token

2 files changed, 61 insertions, 49 deletions

diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb
index 1984ad8825..c645af88d7 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
@@ -652,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'
@@ -673,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'
@@ -693,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'
@@ -710,6 +696,36 @@ class PerFormTokensControllerTest < ActionController::TestCase
     end
   end
 
+  def test_rejects_token_for_incorrect_method_button_to
+    get :button_to, params: { form_method: 'delete' }
+
+    form_token = assert_presence_and_fetch_form_csrf_token
+
+    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'
+    assert_raises(ActionController::InvalidAuthenticityToken) do
+      patch :post_one, params: { custom_authenticity_token: form_token }
+    end
+  end
+
+  %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 = assert_presence_and_fetch_form_csrf_token
+
+      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
+        send verb, :post_one, params: { custom_authenticity_token: form_token }
+      end
+    end
+  end
+
   def test_accepts_global_csrf_token
     get :index
 
@@ -726,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'
@@ -747,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'
@@ -764,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/'
@@ -781,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
@@ -794,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
diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb
index 87218821ed..234e489115 100644
--- a/actionview/lib/action_view/helpers/url_helper.rb
+++ b/actionview/lib/action_view/helpers/url_helper.rb
@@ -312,7 +312,7 @@ module ActionView
         form_options[:'data-remote'] = true if remote
 
         request_token_tag = if form_method == 'post'
-          token_tag(nil, form_options: form_options)
+          token_tag(nil, form_options: form_options.merge(method: method))
         else
           ''
         end