Commit 9feede8d authored by Sean McGivern's avatar Sean McGivern

Allow custom rate limiting response text

When a request exceeds a rate limit configured in the admin panel, we
default to returning a 429 response with a plain-text body of 'Retry
later'. This change allows the response body to be customised - although
it must still be plain text - which allows, for example, GitLab.com to
provide a link to GitLab.com-specific rate limiting documentation.
parent c71d8433
...@@ -335,7 +335,8 @@ module ApplicationSettingsHelper ...@@ -335,7 +335,8 @@ module ApplicationSettingsHelper
:group_export_limit, :group_export_limit,
:group_download_export_limit, :group_download_export_limit,
:wiki_page_max_content_bytes, :wiki_page_max_content_bytes,
:container_registry_delete_tags_service_timeout :container_registry_delete_tags_service_timeout,
:rate_limiting_response_text
] ]
end end
......
...@@ -400,6 +400,10 @@ class ApplicationSetting < ApplicationRecord ...@@ -400,6 +400,10 @@ class ApplicationSetting < ApplicationRecord
validates :ci_jwt_signing_key, validates :ci_jwt_signing_key,
rsa_key: true, allow_nil: true 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, attr_encrypted :asset_proxy_secret_key,
mode: :per_attribute_iv, mode: :per_attribute_iv,
key: Settings.attr_encrypted_db_key_base_truncated, key: Settings.attr_encrypted_db_key_base_truncated,
......
...@@ -172,7 +172,8 @@ module ApplicationSettingImplementation ...@@ -172,7 +172,8 @@ module ApplicationSettingImplementation
container_registry_delete_tags_service_timeout: 250, container_registry_delete_tags_service_timeout: 250,
container_registry_expiration_policies_worker_capacity: 0, container_registry_expiration_policies_worker_capacity: 0,
kroki_enabled: false, kroki_enabled: false,
kroki_url: nil kroki_url: nil,
rate_limiting_response_text: nil
} }
end end
......
...@@ -49,5 +49,12 @@ ...@@ -49,5 +49,12 @@
.form-group .form-group
= f.label :throttle_authenticated_web_period_in_seconds, 'Authenticated web rate limit period in seconds', class: 'label-bold' = 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' = 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' } = f.submit 'Save changes', class: "gl-button btn btn-success", data: { qa_selector: 'save_changes_button' }
---
title: Allow custom response to be set when rate limits are exceeded
merge_request: 50693
author:
type: added
# 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
# 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
b3fcc73c6b61469d770e9eb9a14c88bb86398db4ab4b6dc5283718a147db0ac0
\ No newline at end of file
8aac4108b658a7a0646ec230dc2568cb51fea0535b13dfba8b8c9e6edb401d07
\ No newline at end of file
...@@ -9376,12 +9376,14 @@ CREATE TABLE application_settings ( ...@@ -9376,12 +9376,14 @@ CREATE TABLE application_settings (
cloud_license_enabled boolean DEFAULT false NOT NULL, cloud_license_enabled boolean DEFAULT false NOT NULL,
disable_feed_token boolean DEFAULT false NOT NULL, disable_feed_token boolean DEFAULT false NOT NULL,
personal_access_token_prefix text, 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 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_17d9558205 CHECK ((char_length((kroki_url)::text) <= 1024)),
CONSTRAINT check_2dba05b802 CHECK ((char_length(gitpod_url) <= 255)), CONSTRAINT check_2dba05b802 CHECK ((char_length(gitpod_url) <= 255)),
CONSTRAINT check_51700b31b5 CHECK ((char_length(default_branch_name) <= 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_57123c9593 CHECK ((char_length(help_page_documentation_base_url) <= 255)),
CONSTRAINT check_718b4458ae CHECK ((char_length(personal_access_token_prefix) <= 20)), 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_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_9a719834eb CHECK ((char_length(secret_detection_token_revocation_url) <= 255)),
CONSTRAINT check_9c6c447a13 CHECK ((char_length(maintenance_mode_message) <= 255)), CONSTRAINT check_9c6c447a13 CHECK ((char_length(maintenance_mode_message) <= 255)),
......
...@@ -83,7 +83,8 @@ Example response: ...@@ -83,7 +83,8 @@ Example response:
"raw_blob_request_limit": 300, "raw_blob_request_limit": 300,
"wiki_page_max_content_bytes": 52428800, "wiki_page_max_content_bytes": 52428800,
"require_admin_approval_after_user_signup": false, "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: ...@@ -176,7 +177,8 @@ Example response:
"raw_blob_request_limit": 300, "raw_blob_request_limit": 300,
"wiki_page_max_content_bytes": 52428800, "wiki_page_max_content_bytes": 52428800,
"require_admin_approval_after_user_signup": false, "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. ...@@ -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. | `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_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. | | `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.| | `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_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. | | `recaptcha_private_key` | string | required by: `recaptcha_enabled` | Private key for reCAPTCHA. |
......
...@@ -25,6 +25,17 @@ By default, all Git operations are first tried unathenticated. Because of this, ...@@ -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) ![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 ## Use an HTTP header to bypass rate limiting
> [Introduced](https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/622) in GitLab 13.6. > [Introduced](https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/622) in GitLab 13.6.
......
...@@ -11,7 +11,7 @@ RSpec.describe Gitlab::RackAttack, :aggregate_failures do ...@@ -11,7 +11,7 @@ RSpec.describe Gitlab::RackAttack, :aggregate_failures do
stub_const("Rack::Attack", fake_rack_attack) stub_const("Rack::Attack", fake_rack_attack)
stub_const("Rack::Attack::Request", fake_rack_attack_request) 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(:throttle)
allow(fake_rack_attack).to receive(:track) allow(fake_rack_attack).to receive(:track)
allow(fake_rack_attack).to receive(:safelist) allow(fake_rack_attack).to receive(:safelist)
......
...@@ -10,8 +10,17 @@ module Gitlab ...@@ -10,8 +10,17 @@ module Gitlab
def self.configure(rack_attack) def self.configure(rack_attack)
# This adds some methods used by our throttles to the `Rack::Request` # This adds some methods used by our throttles to the `Rack::Request`
rack_attack::Request.include(Gitlab::RackAttack::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 the throttles
configure_throttles(rack_attack) configure_throttles(rack_attack)
......
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
module Gitlab module Gitlab
class Throttle class Throttle
DEFAULT_RATE_LIMITING_RESPONSE_TEXT = 'Retry later'
def self.settings def self.settings
Gitlab::CurrentSettings.current_application_settings Gitlab::CurrentSettings.current_application_settings
end end
...@@ -46,5 +48,9 @@ module Gitlab ...@@ -46,5 +48,9 @@ module Gitlab
{ limit: limit_proc, period: period_proc } { limit: limit_proc, period: period_proc }
end end
def self.rate_limiting_response_text
(settings.rate_limiting_response_text.presence || DEFAULT_RATE_LIMITING_RESPONSE_TEXT) + "\n"
end
end end
end end
...@@ -1304,6 +1304,9 @@ msgstr "" ...@@ -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" msgid "A plain HTML site that uses Netlify for CI/CD instead of GitLab, but still with all the other great GitLab features"
msgstr "" 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." msgid "A platform value can be web, mob or app."
msgstr "" msgstr ""
...@@ -23928,6 +23931,9 @@ msgstr "" ...@@ -23928,6 +23931,9 @@ msgstr ""
msgid "Response metrics (NGINX)" msgid "Response metrics (NGINX)"
msgstr "" msgstr ""
msgid "Response text"
msgstr ""
msgid "Restart Terminal" msgid "Restart Terminal"
msgstr "" msgstr ""
......
...@@ -22,8 +22,7 @@ RSpec.describe Gitlab::RackAttack, :aggregate_failures do ...@@ -22,8 +22,7 @@ RSpec.describe Gitlab::RackAttack, :aggregate_failures do
stub_const("Rack::Attack", fake_rack_attack) stub_const("Rack::Attack", fake_rack_attack)
stub_const("Rack::Attack::Request", fake_rack_attack_request) stub_const("Rack::Attack::Request", fake_rack_attack_request)
# Expect rather than just allow, because this is actually fairly important functionality allow(fake_rack_attack).to receive(:throttled_response=)
expect(fake_rack_attack).to receive(:throttled_response_retry_after_header=).with(true)
allow(fake_rack_attack).to receive(:throttle) allow(fake_rack_attack).to receive(:throttle)
allow(fake_rack_attack).to receive(:track) allow(fake_rack_attack).to receive(:track)
allow(fake_rack_attack).to receive(:safelist) allow(fake_rack_attack).to receive(:safelist)
...@@ -36,6 +35,12 @@ RSpec.describe Gitlab::RackAttack, :aggregate_failures do ...@@ -36,6 +35,12 @@ RSpec.describe Gitlab::RackAttack, :aggregate_failures do
expect(fake_rack_attack_request).to include(described_class::Request) expect(fake_rack_attack_request).to include(described_class::Request)
end 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 it 'configures the safelist' do
described_class.configure(fake_rack_attack) described_class.configure(fake_rack_attack)
......
...@@ -30,4 +30,32 @@ RSpec.describe Gitlab::Throttle do ...@@ -30,4 +30,32 @@ RSpec.describe Gitlab::Throttle do
end end
end 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 end
...@@ -60,6 +60,24 @@ RSpec.describe 'Rack Attack global throttles' do ...@@ -60,6 +60,24 @@ RSpec.describe 'Rack Attack global throttles' do
expect_rejection { get url_that_does_not_require_authentication } expect_rejection { get url_that_does_not_require_authentication }
end 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 it 'allows requests after throttling and then waiting for the next period' do
requests_per_period.times do requests_per_period.times do
get url_that_does_not_require_authentication get url_that_does_not_require_authentication
......
...@@ -25,6 +25,7 @@ module RackAttackSpecHelpers ...@@ -25,6 +25,7 @@ module RackAttackSpecHelpers
yield yield
expect(response).to have_gitlab_http_status(:too_many_requests) expect(response).to have_gitlab_http_status(:too_many_requests)
expect(response).to have_header('Retry-After')
end end
def expect_ok(&block) def expect_ok(&block)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment