Merge branch 'allow-custom-rate-limiting-response' into 'master'

Allow custom rate limiting response text

See merge request gitlab-org/gitlab!50693

app/helpers/application_settings_helper.rb

@@ -335,7 +335,8 @@ module ApplicationSettingsHelper
       :group_export_limit,
       :group_download_export_limit,
       :wiki_page_max_content_bytes,
-      :container_registry_delete_tags_service_timeout
+      :container_registry_delete_tags_service_timeout,
+      :rate_limiting_response_text
     ]
   end
 

app/models/application_setting.rb

@@ -400,6 +400,10 @@ class ApplicationSetting < ApplicationRecord
   validates :ci_jwt_signing_key,
             rsa_key: true, allow_nil: true
 
+  validates :rate_limiting_response_text,
+            length: { maximum: 255, message: _('is too long (maximum is %{count} characters)') },
+            allow_blank: true
+
   attr_encrypted :asset_proxy_secret_key,
                  mode: :per_attribute_iv,
                  key: Settings.attr_encrypted_db_key_base_truncated,

app/models/application_setting_implementation.rb

@@ -172,7 +172,8 @@ module ApplicationSettingImplementation
         container_registry_delete_tags_service_timeout: 250,
         container_registry_expiration_policies_worker_capacity: 0,
         kroki_enabled: false,
-        kroki_url: nil
+        kroki_url: nil,
+        rate_limiting_response_text: nil
       }
     end
 

app/views/admin/application_settings/_ip_limits.html.haml

@@ -49,5 +49,12 @@
     .form-group
       = f.label :throttle_authenticated_web_period_in_seconds, 'Authenticated web rate limit period in seconds', class: 'label-bold'
       = f.number_field :throttle_authenticated_web_period_in_seconds, class: 'form-control'
+    %hr
+    %h5
+      = _('Response text')
+    .form-group
+      = f.label :rate_limiting_response_text, class: 'label-bold' do
+        = _('A plain-text response to show to clients that hit the rate limit.')
+      = f.text_area :rate_limiting_response_text, placeholder: ::Gitlab::Throttle::DEFAULT_RATE_LIMITING_RESPONSE_TEXT, class: 'form-control', rows: 5
 
   = f.submit 'Save changes', class: "gl-button btn btn-success", data: { qa_selector: 'save_changes_button' }

changelogs/unreleased/allow-custom-rate-limiting-response.yml

+---
+title: Allow custom response to be set when rate limits are exceeded
+merge_request: 50693
+author:
+type: added

db/migrate/20201230161206_add_rate_limiting_response_text_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddRateLimitingResponseTextToApplicationSettings < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  # rubocop:disable Migration/AddLimitToTextColumns
+  # limit is added in 20210101110640_set_limit_for_rate_limiting_response_text
+  def change
+    add_column :application_settings, :rate_limiting_response_text, :text
+  end
+  # rubocop:enable Migration/AddLimitToTextColumns
+end

db/migrate/20210101110640_set_limit_for_rate_limiting_response_text.rb

+# frozen_string_literal: true
+
+class SetLimitForRateLimitingResponseText < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_text_limit :application_settings, :rate_limiting_response_text, 255
+  end
+
+  def down
+    remove_text_limit :application_settings, :rate_limiting_response_text
+  end
+end

db/schema_migrations/20201230161206

+b3fcc73c6b61469d770e9eb9a14c88bb86398db4ab4b6dc5283718a147db0ac0
\ No newline at end of file

db/schema_migrations/20210101110640

+8aac4108b658a7a0646ec230dc2568cb51fea0535b13dfba8b8c9e6edb401d07
\ No newline at end of file

db/structure.sql

@@ -9376,12 +9376,14 @@ CREATE TABLE application_settings (
     cloud_license_enabled boolean DEFAULT false NOT NULL,
     disable_feed_token boolean DEFAULT false NOT NULL,
     personal_access_token_prefix text,
+    rate_limiting_response_text text,
     CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)),
     CONSTRAINT check_17d9558205 CHECK ((char_length((kroki_url)::text) <= 1024)),
     CONSTRAINT check_2dba05b802 CHECK ((char_length(gitpod_url) <= 255)),
     CONSTRAINT check_51700b31b5 CHECK ((char_length(default_branch_name) <= 255)),
     CONSTRAINT check_57123c9593 CHECK ((char_length(help_page_documentation_base_url) <= 255)),
     CONSTRAINT check_718b4458ae CHECK ((char_length(personal_access_token_prefix) <= 20)),
+    CONSTRAINT check_7227fad848 CHECK ((char_length(rate_limiting_response_text) <= 255)),
     CONSTRAINT check_85a39b68ff CHECK ((char_length(encrypted_ci_jwt_signing_key_iv) <= 255)),
     CONSTRAINT check_9a719834eb CHECK ((char_length(secret_detection_token_revocation_url) <= 255)),
     CONSTRAINT check_9c6c447a13 CHECK ((char_length(maintenance_mode_message) <= 255)),

doc/api/settings.md

@@ -83,7 +83,8 @@ Example response:
   "raw_blob_request_limit": 300,
   "wiki_page_max_content_bytes": 52428800,
   "require_admin_approval_after_user_signup": false,
-  "personal_access_token_prefix": "GL-"
+  "personal_access_token_prefix": "GL-",
+  "rate_limiting_response_text": null
 }
 ```
 
@@ -176,7 +177,8 @@ Example response:
   "raw_blob_request_limit": 300,
   "wiki_page_max_content_bytes": 52428800,
   "require_admin_approval_after_user_signup": false,
-  "personal_access_token_prefix": "GL-"
+  "personal_access_token_prefix": "GL-",
+  "rate_limiting_response_text": null
 }
 ```
 
@@ -330,6 +332,7 @@ listed in the descriptions of the relevant settings.
 | `pseudonymizer_enabled`                  | boolean          | no                                   | **(PREMIUM)** When enabled, GitLab runs a background job that produces pseudonymized CSVs of the GitLab database to upload to your configured object storage directory.
 | `push_event_activities_limit`            | integer          | no                                   | Number of changes (branches or tags) in a single push to determine whether individual push events or bulk push events are created. [Bulk push events are created](../user/admin_area/settings/push_event_activities_limit.md) if it surpasses that value. |
 | `push_event_hooks_limit`                 | integer          | no                                   | Number of changes (branches or tags) in a single push to determine whether webhooks and services fire or not. Webhooks and services aren't submitted if it surpasses that value. |
+| `rate_limiting_response_text`            | string           | no                                   | When rate limiting is enabled via the `throttle_*` settings, send this plain text response when a rate limit is exceeded. 'Retry later' is sent if this is blank. |
 | `raw_blob_request_limit`                 | integer          | no                                   | Max number of requests per minute for each raw path. Default: 300. To disable throttling set to 0.|
 | `recaptcha_enabled`                      | boolean          | no                                   | (**If enabled, requires:** `recaptcha_private_key` and `recaptcha_site_key`) Enable reCAPTCHA. |
 | `recaptcha_private_key`                  | string           | required by: `recaptcha_enabled`     | Private key for reCAPTCHA. |

doc/user/admin_area/settings/user_and_ip_rate_limits.md

@@ -25,6 +25,17 @@ By default, all Git operations are first tried unathenticated. Because of this,
 
 ![user-and-ip-rate-limits](img/user_and_ip_rate_limits.png)
 
+## Response text
+
+A request that exceeds a rate limit will get a 429 response code and a
+plain-text body, which by default is:
+
+```plaintext
+Retry later
+```
+
+It is possible to customize this response text in the admin area.
+
 ## Use an HTTP header to bypass rate limiting
 
 > [Introduced](https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/622) in GitLab 13.6.

ee/spec/lib/gitlab/rack_attack_spec.rb

@@ -11,7 +11,7 @@ RSpec.describe Gitlab::RackAttack, :aggregate_failures do
       stub_const("Rack::Attack", fake_rack_attack)
       stub_const("Rack::Attack::Request", fake_rack_attack_request)
 
-      expect(fake_rack_attack).to receive(:throttled_response_retry_after_header=).with(true)
+      allow(fake_rack_attack).to receive(:throttled_response=)
       allow(fake_rack_attack).to receive(:throttle)
       allow(fake_rack_attack).to receive(:track)
       allow(fake_rack_attack).to receive(:safelist)

lib/gitlab/rack_attack.rb

@@ -10,8 +10,17 @@ module Gitlab
     def self.configure(rack_attack)
       # This adds some methods used by our throttles to the `Rack::Request`
       rack_attack::Request.include(Gitlab::RackAttack::Request)
-      # Send the Retry-After header so clients (e.g. python-gitlab) can make good choices about delays
-      Rack::Attack.throttled_response_retry_after_header = true
+
+      # This is Rack::Attack::DEFAULT_THROTTLED_RESPONSE, modified to allow a custom response
+      Rack::Attack.throttled_response = lambda do |env|
+        # Send the Retry-After header so clients (e.g. python-gitlab) can make good choices about delays
+        match_data = env['rack.attack.match_data']
+        now = match_data[:epoch_time]
+        retry_after = match_data[:period] - (now % match_data[:period])
+
+        [429, { 'Content-Type' => 'text/plain', 'Retry-After' => retry_after.to_s }, [Gitlab::Throttle.rate_limiting_response_text]]
+      end
+
       # Configure the throttles
       configure_throttles(rack_attack)
 

lib/gitlab/throttle.rb

@@ -2,6 +2,8 @@
 
 module Gitlab
   class Throttle
+    DEFAULT_RATE_LIMITING_RESPONSE_TEXT = 'Retry later'
+
     def self.settings
       Gitlab::CurrentSettings.current_application_settings
     end
@@ -46,5 +48,9 @@ module Gitlab
 
       { limit: limit_proc, period: period_proc }
     end
+
+    def self.rate_limiting_response_text
+      (settings.rate_limiting_response_text.presence || DEFAULT_RATE_LIMITING_RESPONSE_TEXT) + "\n"
+    end
   end
 end

locale/gitlab.pot

@@ -1304,6 +1304,9 @@ msgstr ""
 msgid "A plain HTML site that uses Netlify for CI/CD instead of GitLab, but still with all the other great GitLab features"
 msgstr ""
 
+msgid "A plain-text response to show to clients that hit the rate limit."
+msgstr ""
+
 msgid "A platform value can be web, mob or app."
 msgstr ""
 
@@ -23928,6 +23931,9 @@ msgstr ""
 msgid "Response metrics (NGINX)"
 msgstr ""
 
+msgid "Response text"
+msgstr ""
+
 msgid "Restart Terminal"
 msgstr ""
 

spec/lib/gitlab/rack_attack_spec.rb

@@ -22,8 +22,7 @@ RSpec.describe Gitlab::RackAttack, :aggregate_failures do
       stub_const("Rack::Attack", fake_rack_attack)
       stub_const("Rack::Attack::Request", fake_rack_attack_request)
 
-      # Expect rather than just allow, because this is actually fairly important functionality
-      expect(fake_rack_attack).to receive(:throttled_response_retry_after_header=).with(true)
+      allow(fake_rack_attack).to receive(:throttled_response=)
       allow(fake_rack_attack).to receive(:throttle)
       allow(fake_rack_attack).to receive(:track)
       allow(fake_rack_attack).to receive(:safelist)
@@ -36,6 +35,12 @@ RSpec.describe Gitlab::RackAttack, :aggregate_failures do
       expect(fake_rack_attack_request).to include(described_class::Request)
     end
 
+    it 'configures the throttle response' do
+      described_class.configure(fake_rack_attack)
+
+      expect(fake_rack_attack).to have_received(:throttled_response=).with(an_instance_of(Proc))
+    end
+
     it 'configures the safelist' do
       described_class.configure(fake_rack_attack)
 

spec/lib/gitlab/throttle_spec.rb

@@ -30,4 +30,32 @@ RSpec.describe Gitlab::Throttle do
       end
     end
   end
+
+  describe '.rate_limiting_response_text' do
+    subject { described_class.rate_limiting_response_text }
+
+    context 'when the setting is not present' do
+      before do
+        stub_application_setting(rate_limiting_response_text: '')
+      end
+
+      it 'returns the default value with a trailing newline' do
+        expect(subject).to eq(described_class::DEFAULT_RATE_LIMITING_RESPONSE_TEXT + "\n")
+      end
+    end
+
+    context 'when the setting is present' do
+      let(:response_text) do
+        'Rate limit exceeded; see https://docs.gitlab.com/ee/user/gitlab_com/#gitlabcom-specific-rate-limits for more details'
+      end
+
+      before do
+        stub_application_setting(rate_limiting_response_text: response_text)
+      end
+
+      it 'returns the default value with a trailing newline' do
+        expect(subject).to eq(response_text + "\n")
+      end
+    end
+  end
 end

spec/requests/rack_attack_global_spec.rb

@@ -60,6 +60,24 @@ RSpec.describe 'Rack Attack global throttles' do
         expect_rejection { get url_that_does_not_require_authentication }
       end
 
+      context 'with custom response text' do
+        before do
+          stub_application_setting(rate_limiting_response_text: 'Custom response')
+        end
+
+        it 'rejects requests over the rate limit' do
+          # At first, allow requests under the rate limit.
+          requests_per_period.times do
+            get url_that_does_not_require_authentication
+            expect(response).to have_gitlab_http_status(:ok)
+          end
+
+          # the last straw
+          expect_rejection { get url_that_does_not_require_authentication }
+          expect(response.body).to eq("Custom response\n")
+        end
+      end
+
       it 'allows requests after throttling and then waiting for the next period' do
         requests_per_period.times do
           get url_that_does_not_require_authentication

spec/support/helpers/rack_attack_spec_helpers.rb

@@ -25,6 +25,7 @@ module RackAttackSpecHelpers
     yield
 
     expect(response).to have_gitlab_http_status(:too_many_requests)
+    expect(response).to have_header('Retry-After')
   end
 
   def expect_ok(&block)