Commit 43099925 authored by GitLab Bot's avatar GitLab Bot

Add latest changes from gitlab-org/gitlab@master

parent e66d6781
...@@ -13,6 +13,7 @@ class ApplicationController < ActionController::Base ...@@ -13,6 +13,7 @@ class ApplicationController < ActionController::Base
include WithPerformanceBar include WithPerformanceBar
include SessionlessAuthentication include SessionlessAuthentication
include ConfirmEmailWarning include ConfirmEmailWarning
include Gitlab::Tracking::ControllerConcern
before_action :authenticate_user! before_action :authenticate_user!
before_action :enforce_terms!, if: :should_enforce_terms? before_action :enforce_terms!, if: :should_enforce_terms?
......
...@@ -265,6 +265,10 @@ module ApplicationSettingsHelper ...@@ -265,6 +265,10 @@ module ApplicationSettingsHelper
:throttle_unauthenticated_enabled, :throttle_unauthenticated_enabled,
:throttle_unauthenticated_period_in_seconds, :throttle_unauthenticated_period_in_seconds,
:throttle_unauthenticated_requests_per_period, :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, :time_tracking_limit_to_hours,
:two_factor_grace_period, :two_factor_grace_period,
:unique_ips_limit_enabled, :unique_ips_limit_enabled,
...@@ -308,6 +312,10 @@ module ApplicationSettingsHelper ...@@ -308,6 +312,10 @@ module ApplicationSettingsHelper
def instance_clusters_enabled? def instance_clusters_enabled?
can?(current_user, :read_cluster, Clusters::Instance.new) can?(current_user, :read_cluster, Clusters::Instance.new)
end end
def omnibus_protected_paths_throttle?
Rack::Attack.throttles.key?('protected paths')
end
end end
ApplicationSettingsHelper.prepend_if_ee('EE::ApplicationSettingsHelper') # rubocop: disable Cop/InjectEnterpriseEditionModule ApplicationSettingsHelper.prepend_if_ee('EE::ApplicationSettingsHelper') # rubocop: disable Cop/InjectEnterpriseEditionModule
......
...@@ -210,6 +210,10 @@ class ApplicationSetting < ApplicationRecord ...@@ -210,6 +210,10 @@ class ApplicationSetting < ApplicationRecord
presence: true, presence: true,
if: :static_objects_external_storage_url? 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| SUPPORTED_KEY_TYPES.each do |type|
validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type } validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
end end
......
...@@ -4,7 +4,7 @@ module ApplicationSettingImplementation ...@@ -4,7 +4,7 @@ module ApplicationSettingImplementation
extend ActiveSupport::Concern extend ActiveSupport::Concern
include Gitlab::Utils::StrongMemoize 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 | # or
\s # any whitespace character \s # any whitespace character
| # or | # or
...@@ -16,6 +16,19 @@ module ApplicationSettingImplementation ...@@ -16,6 +16,19 @@ module ApplicationSettingImplementation
FORBIDDEN_KEY_VALUE = KeyRestrictionValidator::FORBIDDEN FORBIDDEN_KEY_VALUE = KeyRestrictionValidator::FORBIDDEN
SUPPORTED_KEY_TYPES = %i[rsa dsa ecdsa ed25519].freeze 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 class_methods do
def defaults def defaults
{ {
...@@ -92,6 +105,10 @@ module ApplicationSettingImplementation ...@@ -92,6 +105,10 @@ module ApplicationSettingImplementation
throttle_unauthenticated_enabled: false, throttle_unauthenticated_enabled: false,
throttle_unauthenticated_period_in_seconds: 3600, throttle_unauthenticated_period_in_seconds: 3600,
throttle_unauthenticated_requests_per_period: 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, time_tracking_limit_to_hours: false,
two_factor_grace_period: 48, two_factor_grace_period: 48,
unique_ips_limit_enabled: false, unique_ips_limit_enabled: false,
...@@ -149,11 +166,11 @@ module ApplicationSettingImplementation ...@@ -149,11 +166,11 @@ module ApplicationSettingImplementation
end end
def domain_whitelist_raw=(values) def domain_whitelist_raw=(values)
self.domain_whitelist = domain_strings_to_array(values) self.domain_whitelist = strings_to_array(values)
end end
def domain_blacklist_raw=(values) def domain_blacklist_raw=(values)
self.domain_blacklist = domain_strings_to_array(values) self.domain_blacklist = strings_to_array(values)
end end
def domain_blacklist_file=(file) def domain_blacklist_file=(file)
...@@ -167,7 +184,7 @@ module ApplicationSettingImplementation ...@@ -167,7 +184,7 @@ module ApplicationSettingImplementation
def outbound_local_requests_whitelist_raw=(values) def outbound_local_requests_whitelist_raw=(values)
clear_memoization(:outbound_local_requests_whitelist_arrays) 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 end
def add_to_outbound_local_requests_whitelist(values_array) def add_to_outbound_local_requests_whitelist(values_array)
...@@ -200,8 +217,16 @@ module ApplicationSettingImplementation ...@@ -200,8 +217,16 @@ module ApplicationSettingImplementation
end end
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) 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 # make sure we always whitelist the running host
values << Gitlab.config.gitlab.host unless values.include?(Gitlab.config.gitlab.host) values << Gitlab.config.gitlab.host unless values.include?(Gitlab.config.gitlab.host)
...@@ -316,11 +341,11 @@ module ApplicationSettingImplementation ...@@ -316,11 +341,11 @@ module ApplicationSettingImplementation
arr&.join("\n") arr&.join("\n")
end end
def domain_strings_to_array(values) def strings_to_array(values)
return [] unless values return [] unless values
values values
.split(DOMAIN_LIST_SEPARATOR) .split(STRING_LIST_SEPARATOR)
.map(&:strip) .map(&:strip)
.reject(&:empty?) .reject(&:empty?)
.uniq .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 @@ ...@@ -34,3 +34,14 @@
= _('Allow requests to the local network from hooks and services.') = _('Allow requests to the local network from hooks and services.')
.settings-content .settings-content
= render 'outbound' = 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 ...@@ -12,10 +12,18 @@ ActiveSupport::Notifications.subscribe('rack.attack') do |name, start, finish, r
path: req.fullpath 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_id = req.env['rack.attack.match_discriminator']
user = User.find_by(id: user_id) 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[:user_id] = user_id
rack_attack_info[:username] = user.username unless user.nil? rack_attack_info[:username] = user.username unless user.nil?
end end
......
...@@ -3,6 +3,15 @@ module Gitlab::Throttle ...@@ -3,6 +3,15 @@ module Gitlab::Throttle
Gitlab::CurrentSettings.current_application_settings Gitlab::CurrentSettings.current_application_settings
end 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 def self.unauthenticated_options
limit_proc = proc { |req| settings.throttle_unauthenticated_requests_per_period } limit_proc = proc { |req| settings.throttle_unauthenticated_requests_per_period }
period_proc = proc { |req| settings.throttle_unauthenticated_period_in_seconds.seconds } period_proc = proc { |req| settings.throttle_unauthenticated_period_in_seconds.seconds }
...@@ -20,6 +29,13 @@ module Gitlab::Throttle ...@@ -20,6 +29,13 @@ module Gitlab::Throttle
period_proc = proc { |req| settings.throttle_authenticated_web_period_in_seconds.seconds } period_proc = proc { |req| settings.throttle_authenticated_web_period_in_seconds.seconds }
{ limit: limit_proc, period: period_proc } { limit: limit_proc, period: period_proc }
end 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 end
class Rack::Attack class Rack::Attack
...@@ -42,6 +58,28 @@ class Rack::Attack ...@@ -42,6 +58,28 @@ class Rack::Attack
req.authenticated_user_id([:api, :rss, :ics]) req.authenticated_user_id([:api, :rss, :ics])
end 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 class Request
def unauthenticated? def unauthenticated?
!authenticated_user_id([:api, :rss, :ics]) !authenticated_user_id([:api, :rss, :ics])
...@@ -66,6 +104,24 @@ class Rack::Attack ...@@ -66,6 +104,24 @@ class Rack::Attack
def web_request? def web_request?
!api_request? !api_request?
end 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
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 ...@@ -322,6 +322,10 @@ ActiveRecord::Schema.define(version: 2019_09_26_041216) do
t.string "encrypted_asset_proxy_secret_key_iv" t.string "encrypted_asset_proxy_secret_key_iv"
t.string "static_objects_external_storage_url", limit: 255 t.string "static_objects_external_storage_url", limit: 255
t.string "static_objects_external_storage_auth_token", 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 ["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 ["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" t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id"
......
...@@ -33,8 +33,13 @@ the `author` field. GitLab team members **should not**. ...@@ -33,8 +33,13 @@ the `author` field. GitLab team members **should not**.
## What warrants a changelog entry? ## What warrants a changelog entry?
- Any change that introduces a database migration **must** have a changelog entry.
- Any user-facing change **should** have a changelog entry. Example: "GitLab now - Any user-facing change **should** have a changelog entry. Example: "GitLab now
uses system fonts for all text." uses system fonts for all text."
- Performance improvements **should** have a changelog entry.
- _Any_ contribution from a community member, no matter how small, **may** have
a changelog entry regardless of these guidelines if the contributor wants one.
Example: "Fixed a typo on the search results page."
- Any docs-only changes **should not** have a changelog entry. - Any docs-only changes **should not** have a changelog entry.
- Any change behind a feature flag **should not** have a changelog entry. The entry should be added [in the merge request removing the feature flags](feature_flags/development.md). - Any change behind a feature flag **should not** have a changelog entry. The entry should be added [in the merge request removing the feature flags](feature_flags/development.md).
- A fix for a regression introduced and then fixed in the same release (i.e., - A fix for a regression introduced and then fixed in the same release (i.e.,
...@@ -43,12 +48,6 @@ the `author` field. GitLab team members **should not**. ...@@ -43,12 +48,6 @@ the `author` field. GitLab team members **should not**.
- Any developer-facing change (e.g., refactoring, technical debt remediation, - Any developer-facing change (e.g., refactoring, technical debt remediation,
test suite changes) **should not** have a changelog entry. Example: "Reduce test suite changes) **should not** have a changelog entry. Example: "Reduce
database records created during Cycle Analytics model spec." database records created during Cycle Analytics model spec."
- _Any_ contribution from a community member, no matter how small, **may** have
a changelog entry regardless of these guidelines if the contributor wants one.
Example: "Fixed a typo on the search results page."
- Performance improvements **should** have a changelog entry.
- Any change that introduces a database migration **must** have a
changelog entry.
## Writing good changelog entries ## Writing good changelog entries
......
...@@ -24,6 +24,7 @@ module Gitlab ...@@ -24,6 +24,7 @@ module Gitlab
# Allow2Ban.filter will return false if this IP has not failed too often yet # Allow2Ban.filter will return false if this IP has not failed too often yet
@banned = Rack::Attack::Allow2Ban.filter(ip, config) do @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 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? ip_can_be_banned?
end end
end end
......
...@@ -6,6 +6,17 @@ module Gitlab ...@@ -6,6 +6,17 @@ module Gitlab
module Tracking module Tracking
SNOWPLOW_NAMESPACE = 'gl' SNOWPLOW_NAMESPACE = 'gl'
module ControllerConcern
extend ActiveSupport::Concern
protected
def track_event(action = action_name, **args)
category = args.delete(:category) || self.class.name
Gitlab::Tracking.event(category, action.to_s, **args)
end
end
class << self class << self
def enabled? def enabled?
Gitlab::CurrentSettings.snowplow_enabled? Gitlab::CurrentSettings.snowplow_enabled?
......
...@@ -1270,6 +1270,9 @@ msgstr "" ...@@ -1270,6 +1270,9 @@ msgstr ""
msgid "All merge conflicts were resolved. The merge request can now be merged." msgid "All merge conflicts were resolved. The merge request can now be merged."
msgstr "" 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" msgid "All projects"
msgstr "" msgstr ""
...@@ -4038,6 +4041,9 @@ msgstr "" ...@@ -4038,6 +4041,9 @@ msgstr ""
msgid "Configure limits for web and API requests." msgid "Configure limits for web and API requests."
msgstr "" 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." msgid "Configure push mirrors."
msgstr "" msgstr ""
...@@ -5680,6 +5686,9 @@ msgstr "" ...@@ -5680,6 +5686,9 @@ msgstr ""
msgid "Enable or disable version check and usage ping." msgid "Enable or disable version check and usage ping."
msgstr "" msgstr ""
msgid "Enable protected paths rate limit"
msgstr ""
msgid "Enable proxy" msgid "Enable proxy"
msgstr "" msgstr ""
...@@ -8058,6 +8067,9 @@ msgstr "" ...@@ -8058,6 +8067,9 @@ msgstr ""
msgid "Helps prevent bots from creating accounts." msgid "Helps prevent bots from creating accounts."
msgstr "" msgstr ""
msgid "Helps reduce request volume for protected paths"
msgstr ""
msgid "Hide archived projects" msgid "Hide archived projects"
msgstr "" msgstr ""
...@@ -10706,6 +10718,9 @@ msgstr "" ...@@ -10706,6 +10718,9 @@ msgstr ""
msgid "OmniAuth" msgid "OmniAuth"
msgstr "" 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" msgid "Onboarding"
msgstr "" msgstr ""
...@@ -12571,6 +12586,9 @@ msgstr "" ...@@ -12571,6 +12586,9 @@ msgstr ""
msgid "Protected Environments" msgid "Protected Environments"
msgstr "" msgstr ""
msgid "Protected Paths"
msgstr ""
msgid "Protected Tag" msgid "Protected Tag"
msgstr "" msgstr ""
...@@ -18958,6 +18976,9 @@ msgstr "" ...@@ -18958,6 +18976,9 @@ msgstr ""
msgid "is not an email you own" msgid "is not an email you own"
msgstr "" msgstr ""
msgid "is too long (maximum is 100 entries)"
msgstr ""
msgid "is too long (maximum is 1000 entries)" msgid "is too long (maximum is 1000 entries)"
msgstr "" msgstr ""
......
...@@ -56,6 +56,8 @@ describe ApplicationController do ...@@ -56,6 +56,8 @@ describe ApplicationController do
end end
end end
it_behaves_like 'a Trackable Controller'
describe '#add_gon_variables' do describe '#add_gon_variables' do
before do before do
Gon.clear Gon.clear
......
...@@ -470,6 +470,8 @@ describe 'Issues' do ...@@ -470,6 +470,8 @@ describe 'Issues' do
expect(page).to have_content 'None' expect(page).to have_content 'None'
end end
wait_for_requests
expect(issue.reload.assignees).to be_empty expect(issue.reload.assignees).to be_empty
end end
......
...@@ -51,6 +51,10 @@ describe ApplicationSetting do ...@@ -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(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(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(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 context "when user accepted let's encrypt terms of service" do
before do before do
......
...@@ -12,7 +12,9 @@ describe 'Rack Attack global throttles' do ...@@ -12,7 +12,9 @@ describe 'Rack Attack global throttles' do
throttle_authenticated_api_requests_per_period: 100, throttle_authenticated_api_requests_per_period: 100,
throttle_authenticated_api_period_in_seconds: 1, throttle_authenticated_api_period_in_seconds: 1,
throttle_authenticated_web_requests_per_period: 100, 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 end
...@@ -35,6 +37,10 @@ describe 'Rack Attack global throttles' do ...@@ -35,6 +37,10 @@ describe 'Rack Attack global throttles' do
let(:url_api_internal) { '/api/v4/internal/check' } let(:url_api_internal) { '/api/v4/internal/check' }
before do 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 # Set low limits
settings_to_set[:throttle_unauthenticated_requests_per_period] = requests_per_period settings_to_set[:throttle_unauthenticated_requests_per_period] = requests_per_period
settings_to_set[:throttle_unauthenticated_period_in_seconds] = period_in_seconds settings_to_set[:throttle_unauthenticated_period_in_seconds] = period_in_seconds
...@@ -203,6 +209,159 @@ describe 'Rack Attack global throttles' do ...@@ -203,6 +209,159 @@ describe 'Rack Attack global throttles' do
it_behaves_like 'rate-limited web authenticated requests' it_behaves_like 'rate-limited web authenticated requests'
end 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) def api_get_args_with_token_headers(partial_url, token_headers)
["/api/#{API::API.version}#{partial_url}", params: nil, headers: token_headers] ["/api/#{API::API.version}#{partial_url}", params: nil, headers: token_headers]
end end
......
...@@ -295,4 +295,26 @@ describe ApplicationSettings::UpdateService do ...@@ -295,4 +295,26 @@ describe ApplicationSettings::UpdateService do
expect(application_settings.raw_blob_request_limit).to eq(600) expect(application_settings.raw_blob_request_limit).to eq(600)
end end
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 end
...@@ -8,6 +8,14 @@ ...@@ -8,6 +8,14 @@
# * period_in_seconds # * period_in_seconds
# * period # * period
shared_examples_for 'rate-limited token-authenticated requests' do 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 before do
# Set low limits # Set low limits
settings_to_set[:"#{throttle_setting_prefix}_requests_per_period"] = requests_per_period 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 ...@@ -84,7 +92,8 @@ shared_examples_for 'rate-limited token-authenticated requests' do
request_method: 'GET', request_method: 'GET',
path: get_args.first, path: get_args.first,
user_id: user.id, 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 expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once
...@@ -116,6 +125,13 @@ end ...@@ -116,6 +125,13 @@ end
# * period_in_seconds # * period_in_seconds
# * period # * period
shared_examples_for 'rate-limited web authenticated requests' do 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 before do
login_as(user) login_as(user)
...@@ -196,7 +212,8 @@ shared_examples_for 'rate-limited web authenticated requests' do ...@@ -196,7 +212,8 @@ shared_examples_for 'rate-limited web authenticated requests' do
request_method: 'GET', request_method: 'GET',
path: '/dashboard/snippets', path: '/dashboard/snippets',
user_id: user.id, 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 expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once
......
# frozen_string_literal: true
shared_examples 'a Trackable Controller' do
describe '#track_event' do
before do
sign_in user
end
context 'with no params' do
controller(described_class) do
def index
track_event
head :ok
end
end
it 'tracks the action name' do
expect(Gitlab::Tracking).to receive(:event).with('AnonymousController', 'index', {})
get :index
end
end
context 'with params' do
controller(described_class) do
def index
track_event('some_event', category: 'SomeCategory', label: 'errorlabel')
head :ok
end
end
it 'tracks with the specified param' do
expect(Gitlab::Tracking).to receive(:event).with('SomeCategory', 'some_event', label: 'errorlabel')
get :index
end
end
end
end
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