Merge branch '351306-rate-limits-don-t-consider-relative-url-root-if-set' into 'master'

Consider relative URL root in Rack::Attack::Request

See merge request gitlab-org/gitlab!79738

ee/lib/ee/gitlab/rack_attack.rb

@@ -13,9 +13,7 @@ module EE
           super
 
           throttle_or_track(rack_attack, 'throttle_incident_management_notification_web', EE::Gitlab::Throttle.incident_management_options) do |req|
-            if req.web_request? &&
-               req.path.include?('alerts/notify') &&
-               EE::Gitlab::Throttle.settings.throttle_incident_management_notification_enabled
+            if req.alerts_notify? && EE::Gitlab::Throttle.settings.throttle_incident_management_notification_enabled
               req.path
             end
           end

ee/lib/ee/gitlab/rack_attack/request.rb

@@ -18,6 +18,10 @@ module EE
             false
           end
         end
+
+        def alerts_notify?
+          web_request? && logical_path.include?('alerts/notify')
+        end
       end
     end
   end

lib/gitlab/rack_attack/request.rb

@@ -28,23 +28,31 @@ module Gitlab
       end
 
       def api_request?
-        path.start_with?('/api')
+        logical_path.start_with?('/api')
+      end
+
+      def logical_path
+        @logical_path ||= path.delete_prefix(Gitlab.config.gitlab.relative_url_root)
+      end
+
+      def matches?(regex)
+        logical_path.match?(regex)
       end
 
       def api_internal_request?
-        path.match?(%r{^/api/v\d+/internal/})
+        matches?(%r{^/api/v\d+/internal/})
       end
 
       def health_check_request?
-        path.match?(%r{^/-/(health|liveness|readiness|metrics)})
+        matches?(%r{^/-/(health|liveness|readiness|metrics)})
       end
 
       def container_registry_event?
-        path.match?(%r{^/api/v\d+/container_registry_event/})
+        matches?(%r{^/api/v\d+/container_registry_event/})
       end
 
       def product_analytics_collector_request?
-        path.start_with?('/-/collector/i')
+        logical_path.start_with?('/-/collector/i')
       end
 
       def should_be_skipped?
@@ -56,7 +64,7 @@ module Gitlab
       end
 
       def protected_path?
-        path.match?(protected_paths_regex)
+        matches?(protected_paths_regex)
       end
 
       def throttle?(throttle, authenticated:)
@@ -178,15 +186,15 @@ module Gitlab
       end
 
       def packages_api_path?
-        path.match?(::Gitlab::Regex::Packages::API_PATH_REGEX)
+        matches?(::Gitlab::Regex::Packages::API_PATH_REGEX)
       end
 
       def git_lfs_path?
-        path.match?(::Gitlab::PathRegex.repository_git_lfs_route_regex)
+        matches?(::Gitlab::PathRegex.repository_git_lfs_route_regex)
       end
 
       def files_api_path?
-        path.match?(FILES_PATH_REGEX)
+        matches?(FILES_PATH_REGEX)
       end
 
       def frontend_request?
@@ -206,7 +214,7 @@ module Gitlab
         with_projects = params['with_projects']
         with_projects = true if with_projects.blank?
 
-        path.match?(GROUP_PATH_REGEX) && Gitlab::Utils.to_boolean(with_projects)
+        matches?(GROUP_PATH_REGEX) && Gitlab::Utils.to_boolean(with_projects)
       end
     end
   end

spec/lib/gitlab/rack_attack/request_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe Gitlab::RackAttack::Request do
     ::Rack::Attack::Request.new(
       env.reverse_merge(
         'REQUEST_METHOD' => 'GET',
-        'PATH_INFO' => path,
+        'PATH_INFO' => Gitlab.config.gitlab.relative_url_root + path,
         'rack.input' => StringIO.new,
         'rack.session' => session
       )
@@ -44,6 +44,14 @@ RSpec.describe Gitlab::RackAttack::Request do
 
     with_them do
       it { is_expected.to eq(expected) }
+
+      context 'when the application is mounted at a relative URL' do
+        before do
+          stub_config_setting(relative_url_root: '/gitlab/root')
+        end
+
+        it { is_expected.to eq(expected) }
+      end
     end
   end
 
@@ -65,6 +73,14 @@ RSpec.describe Gitlab::RackAttack::Request do
 
     with_them do
       it { is_expected.to eq(expected) }
+
+      context 'when the application is mounted at a relative URL' do
+        before do
+          stub_config_setting(relative_url_root: '/gitlab/root')
+        end
+
+        it { is_expected.to eq(expected) }
+      end
     end
   end
 
@@ -88,6 +104,14 @@ RSpec.describe Gitlab::RackAttack::Request do
 
     with_them do
       it { is_expected.to eq(expected) }
+
+      context 'when the application is mounted at a relative URL' do
+        before do
+          stub_config_setting(relative_url_root: '/gitlab/root')
+        end
+
+        it { is_expected.to eq(expected) }
+      end
     end
   end
 
@@ -107,6 +131,14 @@ RSpec.describe Gitlab::RackAttack::Request do
 
     with_them do
       it { is_expected.to eq(expected) }
+
+      context 'when the application is mounted at a relative URL' do
+        before do
+          stub_config_setting(relative_url_root: '/gitlab/root')
+        end
+
+        it { is_expected.to eq(expected) }
+      end
     end
   end
 
@@ -127,6 +159,14 @@ RSpec.describe Gitlab::RackAttack::Request do
 
     with_them do
       it { is_expected.to eq(expected) }
+
+      context 'when the application is mounted at a relative URL' do
+        before do
+          stub_config_setting(relative_url_root: '/gitlab/root')
+        end
+
+        it { is_expected.to eq(expected) }
+      end
     end
   end
 
@@ -162,6 +202,14 @@ RSpec.describe Gitlab::RackAttack::Request do
 
     with_them do
       it { is_expected.to eq(expected) }
+
+      context 'when the application is mounted at a relative URL' do
+        before do
+          stub_config_setting(relative_url_root: '/gitlab/root')
+        end
+
+        it { is_expected.to eq(expected) }
+      end
     end
   end
 
@@ -189,6 +237,14 @@ RSpec.describe Gitlab::RackAttack::Request do
 
     with_them do
       it { is_expected.to eq(expected) }
+
+      context 'when the application is mounted at a relative URL' do
+        before do
+          stub_config_setting(relative_url_root: '/gitlab/root')
+        end
+
+        it { is_expected.to eq(expected) }
+      end
     end
   end
 
@@ -255,6 +311,14 @@ RSpec.describe Gitlab::RackAttack::Request do
 
     with_them do
       it { is_expected.to eq(expected) }
+
+      context 'when the application is mounted at a relative URL' do
+        before do
+          stub_config_setting(relative_url_root: '/gitlab/root')
+        end
+
+        it { is_expected.to eq(expected) }
+      end
     end
   end
 end