From 683b9627b3ad51f14457b580d0d988715b202f96 Mon Sep 17 00:00:00 2001
From: Matthew Caruana Galizia <mattcg@gmail.com>
Date: Fri, 20 May 2016 16:25:02 +0200
Subject: Respect `log_warning_on_csrf_failure` setting for all CSRF failures

CSRF verification for non-XHR GET requests (cross-origin `<script>`
tags) didn't check this flag before logging failures.

Setting `config.action_controller.log_warning_on_csrf_failure = false`
now disables logging for these CSRF failures as well.

Closes #25086.

Signed-off-by: Jeremy Daer <jeremydaer@gmail.com>
---
 .../metal/request_forgery_protection.rb            |  4 ++-
 .../controller/request_forgery_protection_test.rb  | 31 ++++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb
index f7e8d06f10..0559fbc6ce 100644
--- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb
+++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb
@@ -235,7 +235,9 @@ module ActionController #:nodoc:
       # we aren't serving an unauthorized cross-origin response.
       def verify_same_origin_request
         if marked_for_same_origin_verification? && non_xhr_javascript_response?
-          logger.warn CROSS_ORIGIN_JAVASCRIPT_WARNING if logger
+          if logger && log_warning_on_csrf_failure
+            logger.warn CROSS_ORIGIN_JAVASCRIPT_WARNING
+          end
           raise ActionController::InvalidCrossOriginRequest, CROSS_ORIGIN_JAVASCRIPT_WARNING
         end
       end
diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb
index d56241f9cd..d3f2ec6aa1 100644
--- a/actionpack/test/controller/request_forgery_protection_test.rb
+++ b/actionpack/test/controller/request_forgery_protection_test.rb
@@ -407,6 +407,37 @@ module RequestForgeryProtectionTests
     end
   end
 
+  def test_should_warn_on_not_same_origin_js
+    old_logger = ActionController::Base.logger
+    logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new
+    ActionController::Base.logger = logger
+
+    begin
+      assert_cross_origin_blocked { get :same_origin_js }
+
+      assert_equal 1, logger.logged(:warn).size
+      assert_match(/<script> tag on another site requested protected JavaScript/, logger.logged(:warn).last)
+    ensure
+      ActionController::Base.logger = old_logger
+    end
+  end
+
+  def test_should_not_warn_if_csrf_logging_disabled_and_not_same_origin_js
+    old_logger = ActionController::Base.logger
+    logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new
+    ActionController::Base.logger = logger
+    ActionController::Base.log_warning_on_csrf_failure = false
+
+    begin
+      assert_cross_origin_blocked { get :same_origin_js }
+
+      assert_equal 0, logger.logged(:warn).size
+    ensure
+      ActionController::Base.logger = old_logger
+      ActionController::Base.log_warning_on_csrf_failure = true
+    end
+  end
+
   # Allow non-GET requests since GET is all a remote <script> tag can muster.
   def test_should_allow_non_get_js_without_xhr_header
     session[:_csrf_token] = @token
-- 
cgit v1.2.3