FEATURE: limit assets less that non asset paths

By default assets can be requested up to 200 times per 10 seconds
from the app, this includes CSS and avatars

app/controllers/application_controller.rb

@@ -659,6 +659,10 @@ class ApplicationController < ActionController::Base
       render_to_string status: status, layout: layout, formats: [:html], template: '/exceptions/not_found'
     end
 
+    def is_asset_path
+      request.env['DISCOURSE_IS_ASSET_PATH'] = 1
+    end
+
   protected
 
     def render_post_json(post, add_raw = true)

app/controllers/static_controller.rb

@@ -103,6 +103,8 @@ class StaticController < ApplicationController
   # instead we cache the favicon in redis and serve it out real quick with
   # a huge expiry, we also cache these assets in nginx so it bypassed if needed
   def favicon
+    is_asset_path
+
     hijack do
       data = DistributedMemoizer.memoize(FAVICON + SiteSetting.favicon_url, 60 * 30) do
         begin
@@ -138,16 +140,22 @@ class StaticController < ApplicationController
   end
 
   def brotli_asset
+    is_asset_path
+
     serve_asset(".br") do
       response.headers["Content-Encoding"] = 'br'
     end
   end
 
   def cdn_asset
+    is_asset_path
+
     serve_asset
   end
 
   def service_worker_asset
+    is_asset_path
+
     respond_to do |format|
       format.js do
         # https://github.com/w3c/ServiceWorker/blob/master/explainer.md#updating-a-service-worker

app/controllers/stylesheets_controller.rb

@@ -6,6 +6,8 @@ class StylesheetsController < ApplicationController
   end
 
   def show
+    is_asset_path
+
     show_resource
   end
 

app/controllers/user_avatars_controller.rb

@@ -33,6 +33,8 @@ class UserAvatarsController < ApplicationController
   end
 
   def show_proxy_letter
+    is_asset_path
+
     if SiteSetting.external_system_avatars_url !~ /^\/letter_avatar_proxy/
       raise Discourse::NotFound
     end
@@ -56,6 +58,8 @@ class UserAvatarsController < ApplicationController
   end
 
   def show_letter
+    is_asset_path
+
     params.require(:username)
     params.require(:version)
     params.require(:size)
@@ -75,6 +79,8 @@ class UserAvatarsController < ApplicationController
   end
 
   def show
+    is_asset_path
+
     # we need multisite support to keep a single origin pull for CDNs
     RailsMultisite::ConnectionManagement.with_hostname(params[:hostname]) do
       hijack do

config/discourse_defaults.conf

@@ -178,6 +178,10 @@ max_admin_api_reqs_per_key_per_minute = 60
 
 max_reqs_per_ip_per_minute = 200
 max_reqs_per_ip_per_10_seconds = 50
+
+# applies to asset type routes (avatars/css and so on)
+max_asset_reqs_per_ip_per_10_seconds = 200
+
 # global rate limiter will simply warn if the limit is exceeded, can be warn+block, warn, block or none
 max_reqs_per_ip_mode = none
 

lib/middleware/request_tracker.rb

@@ -171,6 +171,16 @@ class Middleware::RequestTracker
     end
     result
   ensure
+    if (limiters = env['DISCOURSE_RATE_LIMITERS']) && env['DISCOURSE_IS_ASSET_PATH']
+      limiters.each(&:rollback!)
+      env['DISCOURSE_ASSET_RATE_LIMITERS'].each do |limiter|
+        begin
+          limiter.performed!
+        rescue RateLimiter::LimitExceeded
+          # skip
+        end
+      end
+    end
     log_request_info(env, result, info) unless env["discourse.request_tracker.skip"]
   end
 
@@ -213,16 +223,35 @@ class Middleware::RequestTracker
         global: true
       )
 
+      limiter_assets10 = RateLimiter.new(
+        nil,
+        "global_ip_limit_10_assets_#{ip}",
+        GlobalSetting.max_asset_reqs_per_ip_per_10_seconds,
+        10,
+        global: true
+      )
+
+      env['DISCOURSE_RATE_LIMITERS'] = [limiter10, limiter60]
+      env['DISCOURSE_ASSET_RATE_LIMITERS'] = [limiter_assets10]
+
+      warn = GlobalSetting.max_reqs_per_ip_mode == "warn" ||
+        GlobalSetting.max_reqs_per_ip_mode == "warn+block"
+
+      if !limiter_assets10.can_perform?
+        if warn
+          Rails.logger.warn("Global asset IP rate limit exceeded for #{ip}: 10 second rate limit, uri: #{env["REQUEST_URI"]}")
+        end
+
+        return !(GlobalSetting.max_reqs_per_ip_mode == "warn")
+      end
+
       type = 10
       begin
         limiter10.performed!
         type = 60
         limiter60.performed!
       rescue RateLimiter::LimitExceeded
-        if (
-          GlobalSetting.max_reqs_per_ip_mode == "warn" ||
-          GlobalSetting.max_reqs_per_ip_mode == "warn+block"
-        )
+        if warn
           Rails.logger.warn("Global IP rate limit exceeded for #{ip}: #{type} second rate limit, uri: #{env["REQUEST_URI"]}")
           !(GlobalSetting.max_reqs_per_ip_mode == "warn")
         else

spec/components/middleware/request_tracker_spec.rb

@@ -191,6 +191,28 @@ describe Middleware::RequestTracker do
       expect(status).to eq(200)
     end
 
+    it "allows assets for more requests" do
+      global_setting :max_reqs_per_ip_per_10_seconds, 1
+      global_setting :max_reqs_per_ip_mode, 'block'
+      global_setting :max_asset_reqs_per_ip_per_10_seconds, 3
+
+      env1 = env("REMOTE_ADDR" => "1.1.1.1", "DISCOURSE_IS_ASSET_PATH" => 1)
+
+      status, _ = middleware.call(env1)
+      expect(status).to eq(200)
+      status, _ = middleware.call(env1)
+      expect(status).to eq(200)
+      status, _ = middleware.call(env1)
+      expect(status).to eq(200)
+      status, _ = middleware.call(env1)
+      expect(status).to eq(429)
+
+      env2 = env("REMOTE_ADDR" => "1.1.1.1")
+
+      status, _ = middleware.call(env2)
+      expect(status).to eq(429)
+    end
+
     it "does block if rate limiter is enabled" do
       global_setting :max_reqs_per_ip_per_10_seconds, 1
       global_setting :max_reqs_per_ip_mode, 'block'
@@ -199,13 +221,13 @@ describe Middleware::RequestTracker do
       env2 = env("REMOTE_ADDR" => "1.1.1.2")
 
       status, _ = middleware.call(env1)
-      status, _ = middleware.call(env1)
+      expect(status).to eq(200)
 
+      status, _ = middleware.call(env1)
       expect(status).to eq(429)
 
       status, _ = middleware.call(env2)
       expect(status).to eq(200)
-
     end
   end