Merge PR rails#42872

Author: rafaelfranca

actionpack/CHANGELOG.md

@@ -1,3 +1,12 @@
+*   Update `HostAuthorization` middleware to render debug info only
+    when `config.consider_all_requests_local` is set to true.
+
+    Also, blocked host info is always logged with level `error`.
+
+    Fixes #42813
+
+    *Nikita Vyrko*
+
 *  Add Server-Timing middleware
 
    Server-Timing specification defines how the server can communicate to browsers performance metrics

actionpack/lib/action_dispatch/middleware/host_authorization.rb

@@ -11,7 +11,10 @@ module ActionDispatch
   #
   # When a request comes to an unauthorized host, the +response_app+
   # application will be executed and rendered. If no +response_app+ is given, a
-  # default one will run, which responds with <tt>403 Forbidden</tt>.
+  # default one will run.
+  # The default response app logs blocked host info with level 'error' and
+  # responds with <tt>403 Forbidden</tt>. The body of the response contains debug info
+  # if +config.consider_all_requests_local+ is set to true, otherwise the body is empty.
   class HostAuthorization
     class Permissions # :nodoc:
       def initialize(hosts)
@@ -56,17 +59,43 @@ def sanitize_string(host)
         end
     end
 
-    DEFAULT_RESPONSE_APP = -> env do
-      request = Request.new(env)
+    class DefaultResponseApp # :nodoc:
+      RESPONSE_STATUS = 403
+
+      def call(env)
+        request = Request.new(env)
+        format = request.xhr? ? "text/plain" : "text/html"
+
+        log_error(request)
+        response(format, response_body(request))
+      end
+
+      private
+        def response_body(request)
+          return "" unless request.get_header("action_dispatch.show_detailed_exceptions")
+
+          template = DebugView.new(host: request.host)
+          template.render(template: "rescues/blocked_host", layout: "rescues/layout")
+        end
+
+        def response(format, body)
+          [RESPONSE_STATUS,
+           { "Content-Type" => "#{format}; charset=#{Response.default_charset}",
+             "Content-Length" => body.bytesize.to_s },
+           [body]]
+        end
+
+        def log_error(request)
+          logger = available_logger(request)
 
-      format = request.xhr? ? "text/plain" : "text/html"
-      template = DebugView.new(host: request.host)
-      body = template.render(template: "rescues/blocked_host", layout: "rescues/layout")
+          return unless logger
 
-      [403, {
-        "Content-Type" => "#{format}; charset=#{Response.default_charset}",
-        "Content-Length" => body.bytesize.to_s,
-      }, [body]]
+          logger.error("[#{self.class.name}] Blocked host: #{request.host}")
+        end
+
+        def available_logger(request)
+          request.logger || ActionView::Base.logger
+        end
     end
 
     def initialize(app, hosts, deprecated_response_app = nil, exclude: nil, response_app: nil)
@@ -83,7 +112,7 @@ def initialize(app, hosts, deprecated_response_app = nil, exclude: nil, response
         response_app ||= deprecated_response_app
       end
 
-      @response_app = response_app || DEFAULT_RESPONSE_APP
+      @response_app = response_app || DefaultResponseApp.new
     end
 
     def call(env)

actionpack/test/dispatch/host_authorization_test.rb

@@ -6,11 +6,20 @@
 class HostAuthorizationTest < ActionDispatch::IntegrationTest
   App = -> env { [200, {}, %w(Success)] }
 
-  test "blocks requests to unallowed host" do
+  test "blocks requests to unallowed host with empty body" do
     @app = ActionDispatch::HostAuthorization.new(App, %w(only.com))
 
     get "/"
 
+    assert_response :forbidden
+    assert_empty response.body
+  end
+
+  test "renders debug info when all requests considered as local" do
+    @app = ActionDispatch::HostAuthorization.new(App, %w(only.com))
+
+    get "/", env: { "action_dispatch.show_detailed_exceptions" => true }
+
     assert_response :forbidden
     assert_match "Blocked host: www.example.com", response.body
   end
@@ -80,6 +89,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
 
     get "/", env: {
       "HOST" => "www.example.local",
+      "action_dispatch.show_detailed_exceptions" => true
     }
 
     assert_response :forbidden
@@ -100,6 +110,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
 
     get "/", env: {
       "HOST" => ".example.com",
+      "action_dispatch.show_detailed_exceptions" => true
     }
 
     assert_response :forbidden
@@ -126,7 +137,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
   test "sanitizes regular expressions to prevent accidental matches" do
     @app = ActionDispatch::HostAuthorization.new(App, [/w.example.co/])
 
-    get "/"
+    get "/", env: { "action_dispatch.show_detailed_exceptions" => true }
 
     assert_response :forbidden
     assert_match "Blocked host: www.example.com", response.body
@@ -149,6 +160,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     get "/", env: {
       "HTTP_X_FORWARDED_HOST" => "127.0.0.1",
       "HOST" => "www.example.com",
+      "action_dispatch.show_detailed_exceptions" => true
     }
 
     assert_response :forbidden
@@ -173,6 +185,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     get "/", env: {
       "HTTP_X_FORWARDED_HOST" => "localhost",
       "HOST" => "www.example.com",
+      "action_dispatch.show_detailed_exceptions" => true
     }
 
     assert_response :forbidden
@@ -185,6 +198,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     get "/", env: {
       "HTTP_X_FORWARDED_HOST" => "sub.domain.com",
       "HOST" => "domain.com",
+      "action_dispatch.show_detailed_exceptions" => true
     }
 
     assert_response :forbidden
@@ -215,7 +229,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
   test "exclude misses block unallowed hosts" do
     @app = ActionDispatch::HostAuthorization.new(App, "only.com", exclude: ->(req) { req.path == "/bar" })
 
-    get "/foo"
+    get "/foo", env: { "action_dispatch.show_detailed_exceptions" => true }
 
     assert_response :forbidden
     assert_match "Blocked host: www.example.com", response.body
@@ -226,6 +240,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
 
     get "/", env: {
       "HOST" => "attacker.com#x.example.com",
+      "action_dispatch.show_detailed_exceptions" => true
     }
 
     assert_response :forbidden
@@ -237,6 +252,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
 
     get "/", env: {
       "HOST" => "sub-example.com",
+      "action_dispatch.show_detailed_exceptions" => true
     }
 
     assert_response :forbidden
@@ -248,4 +264,30 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
       ActionDispatch::HostAuthorization.new(App, "example.com", ->(env) { true })
     end
   end
+
+  test "uses logger from the env" do
+    @app = ActionDispatch::HostAuthorization.new(App, %w(only.com))
+    output = StringIO.new
+
+    get "/", env: { "action_dispatch.logger" => Logger.new(output) }
+
+    assert_response :forbidden
+    assert_match "Blocked host: www.example.com", output.rewind && output.read
+  end
+
+  test "uses ActionView::Base logger when no logger in the env" do
+    @app = ActionDispatch::HostAuthorization.new(App, %w(only.com))
+    output = StringIO.new
+    logger = Logger.new(output)
+
+    _old, ActionView::Base.logger = ActionView::Base.logger, logger
+    begin
+      get "/"
+    ensure
+      ActionView::Base.logger = _old
+    end
+
+    assert_response :forbidden
+    assert_match "Blocked host: www.example.com", output.rewind && output.read
+  end
 end