Commit 61e19429 authored by Stan Hu's avatar Stan Hu

Merge branch 'ee-mc-moves-protected-path-throttle-to-gitlab-rails' into 'master'

Moves Protected paths throttling from Omnibus to GitLab Rails

See merge request gitlab-org/gitlab!16463
parents 6f942a42 bc73fc40
......@@ -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,
......@@ -308,6 +312,10 @@ module ApplicationSettingsHelper
def instance_clusters_enabled?
can?(current_user, :read_cluster, Clusters::Instance.new)
end
def omnibus_protected_paths_throttle?
Rack::Attack.throttles.key?('protected paths')
end
end
ApplicationSettingsHelper.prepend_if_ee('EE::ApplicationSettingsHelper') # rubocop: disable Cop/InjectEnterpriseEditionModule
......
......@@ -210,6 +210,10 @@ 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
SUPPORTED_KEY_TYPES.each do |type|
validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
end
......
......@@ -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
......
= 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
- if omnibus_protected_paths_throttle?
.bs-callout.bs-callout-danger
- relative_url_link = 'https://docs.gitlab.com/ee/user/admin_area/settings/protected_paths.html#migrating-from-omnibus'
- relative_url_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: relative_url_link }
= _("Omnibus Protected Paths throttle is active. From 12.4, Omnibus throttle is deprecated and will be removed in a future release. Please read the %{relative_url_link_start}Migrating Protected Paths documentation%{relative_url_link_end}.").html_safe % { relative_url_link_start: relative_url_link_start, relative_url_link_end: '</a>'.html_safe }
.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'
......@@ -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. A web server restart is required after changing these settings.')
.settings-content
= render 'protected_paths'
---
title: Allow users to configure protected paths from Admin panel
merge_request: 31246
author:
type: added
......@@ -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
......
......@@ -3,6 +3,15 @@ module Gitlab::Throttle
Gitlab::CurrentSettings.current_application_settings
end
def self.protected_paths_enabled?
!self.omnibus_protected_paths_present? &&
self.settings.throttle_protected_paths_enabled?
end
def self.omnibus_protected_paths_present?
Rack::Attack.throttles.key?('protected paths')
end
def self.unauthenticated_options
limit_proc = proc { |req| settings.throttle_unauthenticated_requests_per_period }
period_proc = proc { |req| settings.throttle_unauthenticated_period_in_seconds.seconds }
......@@ -20,6 +29,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 +58,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.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.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.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,6 +104,24 @@ 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
......
# 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
......@@ -322,6 +322,10 @@ ActiveRecord::Schema.define(version: 2019_09_26_041216) 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"
......
......@@ -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
......
......@@ -1270,6 +1270,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 ""
......@@ -4038,6 +4041,9 @@ msgstr ""
msgid "Configure limits for web and API requests."
msgstr ""
msgid "Configure paths to be protected by Rack Attack. A web server restart is required after changing these settings."
msgstr ""
msgid "Configure push mirrors."
msgstr ""
......@@ -5680,6 +5686,9 @@ msgstr ""
msgid "Enable or disable version check and usage ping."
msgstr ""
msgid "Enable protected paths rate limit"
msgstr ""
msgid "Enable proxy"
msgstr ""
......@@ -8058,6 +8067,9 @@ msgstr ""
msgid "Helps prevent bots from creating accounts."
msgstr ""
msgid "Helps reduce request volume for protected paths"
msgstr ""
msgid "Hide archived projects"
msgstr ""
......@@ -10706,6 +10718,9 @@ msgstr ""
msgid "OmniAuth"
msgstr ""
msgid "Omnibus Protected Paths throttle is active. From 12.4, Omnibus throttle is deprecated and will be removed in a future release. Please read the %{relative_url_link_start}Migrating Protected Paths documentation%{relative_url_link_end}."
msgstr ""
msgid "Onboarding"
msgstr ""
......@@ -12571,6 +12586,9 @@ msgstr ""
msgid "Protected Environments"
msgstr ""
msgid "Protected Paths"
msgstr ""
msgid "Protected Tag"
msgstr ""
......@@ -18958,6 +18976,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 ""
......
......@@ -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
......
......@@ -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,159 @@ 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
context 'when Omnibus throttle is present' do
before do
allow(Gitlab::Throttle)
.to receive(:omnibus_protected_paths_present?).and_return(true)
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
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
context 'when Omnibus throttle is present' 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)] }
before do
settings_to_set[:"#{throttle_setting_prefix}_requests_per_period"] = requests_per_period
settings_to_set[:"#{throttle_setting_prefix}_period_in_seconds"] = period_in_seconds
settings_to_set[:"#{throttle_setting_prefix}_enabled"] = true
stub_application_setting(settings_to_set)
allow(Gitlab::Throttle)
.to receive(:omnibus_protected_paths_present?).and_return(true)
end
it 'allows requests over the rate limit' do
(1 + requests_per_period).times do
get(*get_args)
expect(response).to have_http_status 200
end
end
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'
context 'when Omnibus throttle is present' do
before do
settings_to_set[:"#{throttle_setting_prefix}_requests_per_period"] = requests_per_period
settings_to_set[:"#{throttle_setting_prefix}_period_in_seconds"] = period_in_seconds
settings_to_set[:"#{throttle_setting_prefix}_enabled"] = true
stub_application_setting(settings_to_set)
allow(Gitlab::Throttle)
.to receive(:omnibus_protected_paths_present?).and_return(true)
login_as(user)
end
it 'allows requests over the rate limit' do
(1 + requests_per_period).times do
get url_that_requires_authentication
expect(response).to have_http_status 200
end
end
end
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
......
......@@ -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
......@@ -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
......
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