Refactored Request Forgery CSRF PerFormTokensController tests and DRY…

…'ed them up.
kabelak · Feb 22, 2016 · 2b4c0ae · 2b4c0ae
1 parent 92203d7
commit 2b4c0ae
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 71 deletions.
diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb
@@ -656,15 +656,9 @@ def test_per_form_token_is_same_size_as_global_token
   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 @@ def test_accepts_token_for_correct_path_and_method
   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 @@ def test_rejects_token_for_incorrect_path
   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 @@ def test_rejects_token_for_incorrect_method
   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 @@ def test_rejects_token_for_incorrect_method_button_to
     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 @@ def test_accepts_global_csrf_token
   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 @@ def test_ignores_params
   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 @@ def test_ignores_trailing_slash_during_generation
   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,17 +783,27 @@ def test_ignores_trailing_slash_during_validation
   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
       post :post_one, params: {custom_authenticity_token: form_token}
     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
@@ -311,7 +311,7 @@ def button_to(name = nil, options = nil, html_options = nil, &block)
         form_options[:action] = url
         form_options[:'data-remote'] = true if remote
 
-        request_token_tag = if (form_method == 'post' || method == 'delete')
+        request_token_tag = if form_method == 'post'
           token_tag(nil, form_options: form_options.merge(method: method))
         else
           ''