summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMayra Cabrera <mcabrera@gitlab.com>2019-07-29 11:42:35 -0500
committerMayra Cabrera <mcabrera@gitlab.com>2019-09-12 08:37:18 -0500
commited622fcc66a521e01eeda45b46872ba016dca0a6 (patch)
tree9358accc94d28d01baf6a0f65f3eac9618ffe5b5
parent0844ec02f2a6dde9e7fdcf53f4f02cc3547ac960 (diff)
downloadgitlab-ce-mc-moves-protected-path-throttle-to-gitlab-rails.tar.gz
Moves protected path to gitlab-railsmc-moves-protected-path-throttle-to-gitlab-rails
- Adds 4 columns to application_settings - 3 to mimic the configuration of existing throttles - 1 to store the protected paths on database - Set default protected paths (taken from Omnibus) - Add new section on admin panel to personalize protected paths configuration - Includes additional protected paths throttles. - Throttles were renamed to 'rack_attack_gitlab_rails'.rb, otherwise the omnibus file will overwrite this file. - If the settings are enabled, they will take precedence over the Omnibus settings
-rw-r--r--app/helpers/application_settings_helper.rb4
-rw-r--r--app/models/application_setting.rb17
-rw-r--r--app/models/application_setting_implementation.rb39
-rw-r--r--app/views/admin/application_settings/_protected_paths.html.haml25
-rw-r--r--app/views/admin/application_settings/network.html.haml11
-rw-r--r--changelogs/unreleased/mc-moves-protected-path-throttle-to-gitlab-rails.yml5
-rw-r--r--config/initializers/rack_attack.rb.example29
-rw-r--r--config/initializers/rack_attack_gitlab_rails.rb (renamed from config/initializers/rack_attack_global.rb)47
-rw-r--r--config/initializers/rack_attack_logging.rb10
-rw-r--r--db/migrate/20190801142441_add_throttle_protected_path_columns.rb25
-rw-r--r--db/schema.rb4
-rw-r--r--lib/gitlab/auth/ip_rate_limiter.rb1
-rw-r--r--locale/gitlab.pot18
-rw-r--r--spec/models/application_setting_spec.rb4
-rw-r--r--spec/requests/rack_attack_global_spec.rb104
-rw-r--r--spec/services/application_settings/update_service_spec.rb22
-rw-r--r--spec/support/shared_examples/requests/rack_attack_shared_examples.rb21
17 files changed, 346 insertions, 40 deletions
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 93e282e44be..3e82bd21402 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -265,6 +265,10 @@ module ApplicationSettingsHelper
:throttle_unauthenticated_enabled,
:throttle_unauthenticated_period_in_seconds,
:throttle_unauthenticated_requests_per_period,
+ :throttle_protected_paths_enabled,
+ :throttle_protected_paths_period_in_seconds,
+ :throttle_protected_paths_requests_per_period,
+ :protected_paths_raw,
:time_tracking_limit_to_hours,
:two_factor_grace_period,
:unique_ips_limit_enabled,
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index c9cd0140ed8..8a9e97188b7 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -210,6 +210,11 @@ class ApplicationSetting < ApplicationRecord
presence: true,
if: :static_objects_external_storage_url?
+ validates :protected_paths,
+ length: { maximum: 100, message: N_('is too long (maximum is 100 entries)') },
+ allow_nil: false,
+ if: :protected_paths_exists?
+
SUPPORTED_KEY_TYPES.each do |type|
validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
end
@@ -314,4 +319,16 @@ class ApplicationSetting < ApplicationRecord
def recaptcha_or_login_protection_enabled
recaptcha_enabled || login_recaptcha_protection_enabled
end
+
+ private
+
+ # On GitLab 12.3, a migration that checks validations of this
+ # model was added. Because 'protected_paths' column is not present
+ # at that point, the validation of protected path fails.
+ # We added this conditional to only execute validation of
+ # 'protected_paths' if the column is present.
+ # https://gitlab.com/gitlab-org/gitlab-ce/issues/66685
+ def protected_paths_exists?
+ ::Gitlab::Database.cached_column_exists?(self.class.table_name, 'protected_paths')
+ end
end
diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb
index 8d9597aa5a4..d486347bb1d 100644
--- a/app/models/application_setting_implementation.rb
+++ b/app/models/application_setting_implementation.rb
@@ -4,7 +4,7 @@ module ApplicationSettingImplementation
extend ActiveSupport::Concern
include Gitlab::Utils::StrongMemoize
- DOMAIN_LIST_SEPARATOR = %r{\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace
+ STRING_LIST_SEPARATOR = %r{\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace
| # or
\s # any whitespace character
| # or
@@ -16,6 +16,19 @@ module ApplicationSettingImplementation
FORBIDDEN_KEY_VALUE = KeyRestrictionValidator::FORBIDDEN
SUPPORTED_KEY_TYPES = %i[rsa dsa ecdsa ed25519].freeze
+ DEFAULT_PROTECTED_PATHS = [
+ '/users/password',
+ '/users/sign_in',
+ '/api/v3/session.json',
+ '/api/v3/session',
+ '/api/v4/session.json',
+ '/api/v4/session',
+ '/users',
+ '/users/confirmation',
+ '/unsubscribes/',
+ '/import/github/personal_access_token'
+ ].freeze
+
class_methods do
def defaults
{
@@ -92,6 +105,10 @@ module ApplicationSettingImplementation
throttle_unauthenticated_enabled: false,
throttle_unauthenticated_period_in_seconds: 3600,
throttle_unauthenticated_requests_per_period: 3600,
+ throttle_protected_paths_enabled: false,
+ throttle_protected_paths_in_seconds: 10,
+ throttle_protected_paths_per_period: 60,
+ protected_paths: DEFAULT_PROTECTED_PATHS,
time_tracking_limit_to_hours: false,
two_factor_grace_period: 48,
unique_ips_limit_enabled: false,
@@ -149,11 +166,11 @@ module ApplicationSettingImplementation
end
def domain_whitelist_raw=(values)
- self.domain_whitelist = domain_strings_to_array(values)
+ self.domain_whitelist = strings_to_array(values)
end
def domain_blacklist_raw=(values)
- self.domain_blacklist = domain_strings_to_array(values)
+ self.domain_blacklist = strings_to_array(values)
end
def domain_blacklist_file=(file)
@@ -167,7 +184,7 @@ module ApplicationSettingImplementation
def outbound_local_requests_whitelist_raw=(values)
clear_memoization(:outbound_local_requests_whitelist_arrays)
- self.outbound_local_requests_whitelist = domain_strings_to_array(values)
+ self.outbound_local_requests_whitelist = strings_to_array(values)
end
def add_to_outbound_local_requests_whitelist(values_array)
@@ -200,8 +217,16 @@ module ApplicationSettingImplementation
end
end
+ def protected_paths_raw
+ array_to_string(self.protected_paths)
+ end
+
+ def protected_paths_raw=(values)
+ self.protected_paths = strings_to_array(values)
+ end
+
def asset_proxy_whitelist=(values)
- values = domain_strings_to_array(values) if values.is_a?(String)
+ values = strings_to_array(values) if values.is_a?(String)
# make sure we always whitelist the running host
values << Gitlab.config.gitlab.host unless values.include?(Gitlab.config.gitlab.host)
@@ -316,11 +341,11 @@ module ApplicationSettingImplementation
arr&.join("\n")
end
- def domain_strings_to_array(values)
+ def strings_to_array(values)
return [] unless values
values
- .split(DOMAIN_LIST_SEPARATOR)
+ .split(STRING_LIST_SEPARATOR)
.map(&:strip)
.reject(&:empty?)
.uniq
diff --git a/app/views/admin/application_settings/_protected_paths.html.haml b/app/views/admin/application_settings/_protected_paths.html.haml
new file mode 100644
index 00000000000..ee4ba9e4a73
--- /dev/null
+++ b/app/views/admin/application_settings/_protected_paths.html.haml
@@ -0,0 +1,25 @@
+= form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-protected-paths-settings'), html: { class: 'fieldset-form' } do |f|
+ = form_errors(@application_setting)
+
+ %fieldset
+ .form-group
+ .form-check
+ = f.check_box :throttle_protected_paths_enabled, class: 'form-check-input'
+ = f.label :throttle_protected_paths_enabled, class: 'form-check-label' do
+ = _('Enable protected paths rate limit')
+ %span.form-text.text-muted
+ = _('Helps reduce request volume for protected paths')
+ .form-group
+ = f.label :throttle_protected_paths_requests_per_period, 'Max requests per period per user', class: 'label-bold'
+ = f.number_field :throttle_protected_paths_requests_per_period, class: 'form-control'
+ .form-group
+ = f.label :throttle_protected_paths_period_in_seconds, 'Rate limit period in seconds', class: 'label-bold'
+ = f.number_field :throttle_protected_paths_period_in_seconds, class: 'form-control'
+ .form-group
+ = f.label :protected_paths, class: 'label-bold' do
+ - relative_url_link = 'https://docs.gitlab.com/omnibus/settings/configuration.html#configuring-a-relative-url-for-gitlab'
+ - relative_url_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: relative_url_link }
+ = _('All paths are relative to the GitLab URL. Do not include %{relative_url_link_start}relative URL%{relative_url_link_end}.').html_safe % { relative_url_link_start: relative_url_link_start, relative_url_link_end: '</a>'.html_safe }
+ = f.text_area :protected_paths_raw, placeholder: '/users/sign_in,/users/password', class: 'form-control', rows: 10
+
+ = f.submit 'Save changes', class: 'btn btn-success'
diff --git a/app/views/admin/application_settings/network.html.haml b/app/views/admin/application_settings/network.html.haml
index 3a4d901ca1d..d9271e60db7 100644
--- a/app/views/admin/application_settings/network.html.haml
+++ b/app/views/admin/application_settings/network.html.haml
@@ -34,3 +34,14 @@
= _('Allow requests to the local network from hooks and services.')
.settings-content
= render 'outbound'
+
+%section.settings.as-protected-paths.no-animate#js-protected-paths-settings{ class: ('expanded' if expanded_by_default?) }
+ .settings-header
+ %h4
+ = _('Protected Paths')
+ %button.btn.btn-default.js-settings-toggle{ type: 'button' }
+ = expanded_by_default? ? _('Collapse') : _('Expand')
+ %p
+ = _('Configure paths to be protected by Rack Attack')
+ .settings-content
+ = render 'protected_paths'
diff --git a/changelogs/unreleased/mc-moves-protected-path-throttle-to-gitlab-rails.yml b/changelogs/unreleased/mc-moves-protected-path-throttle-to-gitlab-rails.yml
new file mode 100644
index 00000000000..47c6c926b42
--- /dev/null
+++ b/changelogs/unreleased/mc-moves-protected-path-throttle-to-gitlab-rails.yml
@@ -0,0 +1,5 @@
+---
+title: Allow users to configure protected paths from Admin panel
+merge_request: 31246
+author:
+type: added
diff --git a/config/initializers/rack_attack.rb.example b/config/initializers/rack_attack.rb.example
deleted file mode 100644
index 69052c029f2..00000000000
--- a/config/initializers/rack_attack.rb.example
+++ /dev/null
@@ -1,29 +0,0 @@
-# 1. Rename this file to rack_attack.rb
-# 2. Review the paths_to_be_protected and add any other path you need protecting
-#
-# If you change this file in a Merge Request, please also create a Merge Request on https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests
-
-paths_to_be_protected = [
- "#{Rails.application.config.relative_url_root}/users/password",
- "#{Rails.application.config.relative_url_root}/users/sign_in",
- "#{Rails.application.config.relative_url_root}/api/#{API::API.version}/session.json",
- "#{Rails.application.config.relative_url_root}/api/#{API::API.version}/session",
- "#{Rails.application.config.relative_url_root}/users",
- "#{Rails.application.config.relative_url_root}/users/confirmation",
- "#{Rails.application.config.relative_url_root}/unsubscribes/",
- "#{Rails.application.config.relative_url_root}/import/github/personal_access_token"
-
-]
-
-# Create one big regular expression that matches strings starting with any of
-# the paths_to_be_protected.
-paths_regex = Regexp.union(paths_to_be_protected.map { |path| /\A#{Regexp.escape(path)}/ })
-rack_attack_enabled = Gitlab.config.rack_attack.git_basic_auth['enabled']
-
-unless Rails.env.test? || !rack_attack_enabled
- Rack::Attack.throttle('protected paths', limit: 10, period: 60.seconds) do |req|
- if req.post? && req.path =~ paths_regex
- req.ip
- end
- end
-end
diff --git a/config/initializers/rack_attack_global.rb b/config/initializers/rack_attack_gitlab_rails.rb
index 86cb930eca9..f1f4f006b59 100644
--- a/config/initializers/rack_attack_global.rb
+++ b/config/initializers/rack_attack_gitlab_rails.rb
@@ -20,6 +20,13 @@ module Gitlab::Throttle
period_proc = proc { |req| settings.throttle_authenticated_web_period_in_seconds.seconds }
{ limit: limit_proc, period: period_proc }
end
+
+ def self.protected_paths_options
+ limit_proc = proc { |req| settings.throttle_protected_paths_requests_per_period }
+ period_proc = proc { |req| settings.throttle_protected_paths_period_in_seconds.seconds }
+
+ { limit: limit_proc, period: period_proc }
+ end
end
class Rack::Attack
@@ -42,6 +49,28 @@ class Rack::Attack
req.authenticated_user_id([:api, :rss, :ics])
end
+ throttle('throttle_unauthenticated_protected_paths', Gitlab::Throttle.protected_paths_options) do |req|
+ Gitlab::Throttle.settings.throttle_protected_paths_enabled &&
+ req.unauthenticated? &&
+ !req.should_be_skipped? &&
+ req.protected_path? &&
+ req.ip
+ end
+
+ throttle('throttle_authenticated_protected_paths_api', Gitlab::Throttle.protected_paths_options) do |req|
+ Gitlab::Throttle.settings.throttle_protected_paths_enabled &&
+ req.api_request? &&
+ req.protected_path? &&
+ req.authenticated_user_id([:api])
+ end
+
+ throttle('throttle_authenticated_protected_paths_web', Gitlab::Throttle.protected_paths_options) do |req|
+ Gitlab::Throttle.settings.throttle_protected_paths_enabled &&
+ req.web_request? &&
+ req.protected_path? &&
+ req.authenticated_user_id([:api, :rss, :ics])
+ end
+
class Request
def unauthenticated?
!authenticated_user_id([:api, :rss, :ics])
@@ -66,5 +95,23 @@ class Rack::Attack
def web_request?
!api_request?
end
+
+ def protected_path?
+ !protected_path_regex.nil?
+ end
+
+ def protected_path_regex
+ path =~ protected_paths_regex
+ end
+
+ private
+
+ def protected_paths
+ Gitlab::CurrentSettings.current_application_settings.protected_paths
+ end
+
+ def protected_paths_regex
+ Regexp.union(protected_paths.map { |path| /\A#{Regexp.escape(path)}/ })
+ end
end
end
diff --git a/config/initializers/rack_attack_logging.rb b/config/initializers/rack_attack_logging.rb
index b43fff24bb0..be7c2175cb2 100644
--- a/config/initializers/rack_attack_logging.rb
+++ b/config/initializers/rack_attack_logging.rb
@@ -12,10 +12,18 @@ ActiveSupport::Notifications.subscribe('rack.attack') do |name, start, finish, r
path: req.fullpath
}
- if %w(throttle_authenticated_api throttle_authenticated_web).include? req.env['rack.attack.matched']
+ throttles_with_user_information = [
+ :throttle_authenticated_api,
+ :throttle_authenticated_web,
+ :throttle_authenticated_protected_paths_api,
+ :throttle_authenticated_protected_paths_web
+ ]
+
+ if throttles_with_user_information.include? req.env['rack.attack.matched'].to_sym
user_id = req.env['rack.attack.match_discriminator']
user = User.find_by(id: user_id)
+ rack_attack_info[:throttle_type] = req.env['rack.attack.matched']
rack_attack_info[:user_id] = user_id
rack_attack_info[:username] = user.username unless user.nil?
end
diff --git a/db/migrate/20190801142441_add_throttle_protected_path_columns.rb b/db/migrate/20190801142441_add_throttle_protected_path_columns.rb
new file mode 100644
index 00000000000..bb6d54f3b7b
--- /dev/null
+++ b/db/migrate/20190801142441_add_throttle_protected_path_columns.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class AddThrottleProtectedPathColumns < ActiveRecord::Migration[5.2]
+ DOWNTIME = false
+
+ DEFAULT_PROTECTED_PATHS = [
+ '/users/password',
+ '/users/sign_in',
+ '/api/v3/session.json',
+ '/api/v3/session',
+ '/api/v4/session.json',
+ '/api/v4/session',
+ '/users',
+ '/users/confirmation',
+ '/unsubscribes/',
+ '/import/github/personal_access_token'
+ ]
+
+ def change
+ add_column :application_settings, :throttle_protected_paths_enabled, :boolean, default: true, null: false
+ add_column :application_settings, :throttle_protected_paths_requests_per_period, :integer, default: 10, null: false
+ add_column :application_settings, :throttle_protected_paths_period_in_seconds, :integer, default: 60, null: false
+ add_column :application_settings, :protected_paths, :string, array: true, limit: 255, default: DEFAULT_PROTECTED_PATHS
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 39faf1e651e..2d9c8ba880a 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -286,6 +286,10 @@ ActiveRecord::Schema.define(version: 2019_09_12_061145) do
t.string "encrypted_asset_proxy_secret_key_iv"
t.string "static_objects_external_storage_url", limit: 255
t.string "static_objects_external_storage_auth_token", limit: 255
+ t.boolean "throttle_protected_paths_enabled", default: true, null: false
+ t.integer "throttle_protected_paths_requests_per_period", default: 10, null: false
+ t.integer "throttle_protected_paths_period_in_seconds", default: 60, null: false
+ t.string "protected_paths", limit: 255, default: ["/users/password", "/users/sign_in", "/api/v3/session.json", "/api/v3/session", "/api/v4/session.json", "/api/v4/session", "/users", "/users/confirmation", "/unsubscribes/", "/import/github/personal_access_token"], array: true
t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id"
t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id"
t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id"
diff --git a/lib/gitlab/auth/ip_rate_limiter.rb b/lib/gitlab/auth/ip_rate_limiter.rb
index 0b7055b3256..74d359bcd28 100644
--- a/lib/gitlab/auth/ip_rate_limiter.rb
+++ b/lib/gitlab/auth/ip_rate_limiter.rb
@@ -24,6 +24,7 @@ module Gitlab
# Allow2Ban.filter will return false if this IP has not failed too often yet
@banned = Rack::Attack::Allow2Ban.filter(ip, config) do
# If we return false here, the failure for this IP is ignored by Allow2Ban
+ # If we return true here, the count for the IP is incremented.
ip_can_be_banned?
end
end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 81ff65a0c5e..3c972f56d12 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -984,6 +984,9 @@ msgstr ""
msgid "All merge conflicts were resolved. The merge request can now be merged."
msgstr ""
+msgid "All paths are relative to the GitLab URL. Do not include %{relative_url_link_start}relative URL%{relative_url_link_end}."
+msgstr ""
+
msgid "All projects"
msgstr ""
@@ -3204,6 +3207,9 @@ msgstr ""
msgid "Configure limits for web and API requests."
msgstr ""
+msgid "Configure paths to be protected by Rack Attack"
+msgstr ""
+
msgid "Configure push mirrors."
msgstr ""
@@ -4399,6 +4405,9 @@ msgstr ""
msgid "Enable or disable version check and usage ping."
msgstr ""
+msgid "Enable protected paths rate limit"
+msgstr ""
+
msgid "Enable reCAPTCHA or Akismet and set IP limits. For reCAPTCHA, we currently only support %{recaptcha_v2_link_start}v2%{recaptcha_v2_link_end}"
msgstr ""
@@ -5793,6 +5802,9 @@ msgstr ""
msgid "Helps prevent bots from creating accounts."
msgstr ""
+msgid "Helps reduce request volume for protected paths"
+msgstr ""
+
msgid "Hide archived projects"
msgstr ""
@@ -9355,6 +9367,9 @@ msgstr ""
msgid "Protected Branch"
msgstr ""
+msgid "Protected Paths"
+msgstr ""
+
msgid "Protected Tag"
msgstr ""
@@ -13910,6 +13925,9 @@ msgstr ""
msgid "is not an email you own"
msgstr ""
+msgid "is too long (maximum is 100 entries)"
+msgstr ""
+
msgid "is too long (maximum is 1000 entries)"
msgstr ""
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index d12f9b9100a..84c25b93fc6 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -51,6 +51,10 @@ describe ApplicationSetting do
it { is_expected.to allow_value(nil).for(:static_objects_external_storage_url) }
it { is_expected.to allow_value(http).for(:static_objects_external_storage_url) }
it { is_expected.to allow_value(https).for(:static_objects_external_storage_url) }
+ it { is_expected.to allow_value(['/example'] * 100).for(:protected_paths) }
+ it { is_expected.not_to allow_value(['/example'] * 101).for(:protected_paths) }
+ it { is_expected.not_to allow_value(nil).for(:protected_paths) }
+ it { is_expected.to allow_value([]).for(:protected_paths) }
context "when user accepted let's encrypt terms of service" do
before do
diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb
index cf459ba99c1..c87300ae48c 100644
--- a/spec/requests/rack_attack_global_spec.rb
+++ b/spec/requests/rack_attack_global_spec.rb
@@ -12,7 +12,9 @@ describe 'Rack Attack global throttles' do
throttle_authenticated_api_requests_per_period: 100,
throttle_authenticated_api_period_in_seconds: 1,
throttle_authenticated_web_requests_per_period: 100,
- throttle_authenticated_web_period_in_seconds: 1
+ throttle_authenticated_web_period_in_seconds: 1,
+ throttle_authenticated_protected_paths_request_per_period: 100,
+ throttle_authenticated_protected_paths_in_seconds: 1
}
end
@@ -35,6 +37,10 @@ describe 'Rack Attack global throttles' do
let(:url_api_internal) { '/api/v4/internal/check' }
before do
+ # Disabling protected paths throttle, otherwise requests to
+ # '/users/sign_in' are caught by this throttle.
+ settings_to_set[:throttle_protected_paths_enabled] = false
+
# Set low limits
settings_to_set[:throttle_unauthenticated_requests_per_period] = requests_per_period
settings_to_set[:throttle_unauthenticated_period_in_seconds] = period_in_seconds
@@ -203,6 +209,102 @@ describe 'Rack Attack global throttles' do
it_behaves_like 'rate-limited web authenticated requests'
end
+ describe 'protected paths' do
+ context 'unauthenticated requests' do
+ let(:protected_path_that_does_not_require_authentication) do
+ '/users/confirmation'
+ end
+
+ before do
+ settings_to_set[:throttle_protected_paths_requests_per_period] = requests_per_period # 1
+ settings_to_set[:throttle_protected_paths_period_in_seconds] = period_in_seconds # 10_000
+ end
+
+ context 'when protected paths throttle is disabled' do
+ before do
+ settings_to_set[:throttle_protected_paths_enabled] = false
+ stub_application_setting(settings_to_set)
+ end
+
+ it 'allows requests over the rate limit' do
+ (1 + requests_per_period).times do
+ get protected_path_that_does_not_require_authentication
+ expect(response).to have_http_status 200
+ end
+ end
+ end
+
+ context 'when protected paths throttle is enabled' do
+ before do
+ settings_to_set[:throttle_protected_paths_enabled] = true
+ stub_application_setting(settings_to_set)
+ end
+
+ it 'rejects requests over the rate limit' do
+ requests_per_period.times do
+ get protected_path_that_does_not_require_authentication
+ expect(response).to have_http_status 200
+ end
+
+ expect_rejection { get protected_path_that_does_not_require_authentication }
+ end
+ end
+ end
+
+ context 'API requests authenticated with personal access token', :api do
+ let(:user) { create(:user) }
+ let(:token) { create(:personal_access_token, user: user) }
+ let(:other_user) { create(:user) }
+ let(:other_user_token) { create(:personal_access_token, user: other_user) }
+ let(:throttle_setting_prefix) { 'throttle_protected_paths' }
+ let(:api_partial_url) { '/users' }
+
+ let(:protected_paths) do
+ [
+ '/api/v4/users'
+ ]
+ end
+
+ before do
+ settings_to_set[:protected_paths] = protected_paths
+ stub_application_setting(settings_to_set)
+ end
+
+ context 'with the token in the query string' do
+ let(:get_args) { [api(api_partial_url, personal_access_token: token)] }
+ let(:other_user_get_args) { [api(api_partial_url, personal_access_token: other_user_token)] }
+
+ it_behaves_like 'rate-limited token-authenticated requests'
+ end
+
+ context 'with the token in the headers' do
+ let(:get_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) }
+ let(:other_user_get_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) }
+
+ it_behaves_like 'rate-limited token-authenticated requests'
+ end
+ end
+
+ describe 'web requests authenticated with regular login' do
+ let(:throttle_setting_prefix) { 'throttle_protected_paths' }
+ let(:user) { create(:user) }
+ let(:url_that_requires_authentication) { '/dashboard/snippets' }
+
+ let(:protected_paths) do
+ [
+ url_that_requires_authentication
+ ]
+ end
+
+ before do
+ settings_to_set[:protected_paths] = protected_paths
+ stub_application_setting(settings_to_set)
+ end
+
+ it_behaves_like 'rate-limited web authenticated requests'
+ end
+ end
+
def api_get_args_with_token_headers(partial_url, token_headers)
["/api/#{API::API.version}#{partial_url}", params: nil, headers: token_headers]
end
diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb
index 51fb43907a6..ea7ea02cee3 100644
--- a/spec/services/application_settings/update_service_spec.rb
+++ b/spec/services/application_settings/update_service_spec.rb
@@ -295,4 +295,26 @@ describe ApplicationSettings::UpdateService do
expect(application_settings.raw_blob_request_limit).to eq(600)
end
end
+
+ context 'when protected path settings are passed' do
+ let(:params) do
+ {
+ throttle_protected_paths_enabled: 1,
+ throttle_protected_paths_period_in_seconds: 600,
+ throttle_protected_paths_requests_per_period: 100,
+ protected_paths_raw: "/users/password\r\n/users/sign_in\r\n"
+ }
+ end
+
+ it 'updates protected path settings' do
+ subject.execute
+
+ application_settings.reload
+
+ expect(application_settings.throttle_protected_paths_enabled).to be_truthy
+ expect(application_settings.throttle_protected_paths_period_in_seconds).to eq(600)
+ expect(application_settings.throttle_protected_paths_requests_per_period).to eq(100)
+ expect(application_settings.protected_paths).to eq(['/users/password', '/users/sign_in'])
+ end
+ end
end
diff --git a/spec/support/shared_examples/requests/rack_attack_shared_examples.rb b/spec/support/shared_examples/requests/rack_attack_shared_examples.rb
index afc6f59b773..a2e38cfc60b 100644
--- a/spec/support/shared_examples/requests/rack_attack_shared_examples.rb
+++ b/spec/support/shared_examples/requests/rack_attack_shared_examples.rb
@@ -8,6 +8,14 @@
# * period_in_seconds
# * period
shared_examples_for 'rate-limited token-authenticated requests' do
+ let(:throttle_types) do
+ {
+ "throttle_protected_paths" => "throttle_authenticated_protected_paths_api",
+ "throttle_authenticated_api" => "throttle_authenticated_api",
+ "throttle_authenticated_web" => "throttle_authenticated_web"
+ }
+ end
+
before do
# Set low limits
settings_to_set[:"#{throttle_setting_prefix}_requests_per_period"] = requests_per_period
@@ -84,7 +92,8 @@ shared_examples_for 'rate-limited token-authenticated requests' do
request_method: 'GET',
path: get_args.first,
user_id: user.id,
- username: user.username
+ username: user.username,
+ throttle_type: throttle_types[throttle_setting_prefix]
}
expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once
@@ -116,6 +125,13 @@ end
# * period_in_seconds
# * period
shared_examples_for 'rate-limited web authenticated requests' do
+ let(:throttle_types) do
+ {
+ "throttle_protected_paths" => "throttle_authenticated_protected_paths_web",
+ "throttle_authenticated_web" => "throttle_authenticated_web"
+ }
+ end
+
before do
login_as(user)
@@ -196,7 +212,8 @@ shared_examples_for 'rate-limited web authenticated requests' do
request_method: 'GET',
path: '/dashboard/snippets',
user_id: user.id,
- username: user.username
+ username: user.username,
+ throttle_type: throttle_types[throttle_setting_prefix]
}
expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once