From 4c4f653296e104566d2dd9a330b460c7ddc8cfc5 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 29 Apr 2022 08:20:38 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@14-10-stable-ee --- lib/gitlab/auth/request_authenticator.rb | 7 +- lib/gitlab/metrics/subscribers/rack_attack.rb | 12 +- lib/gitlab/rack_attack.rb | 10 +- lib/gitlab/rack_attack/request.rb | 28 +++- spec/lib/gitlab/auth/request_authenticator_spec.rb | 43 +++++- .../gitlab/metrics/subscribers/rack_attack_spec.rb | 152 +++++++++++++-------- spec/requests/rack_attack_global_spec.rb | 52 ++++--- spec/support/helpers/rack_attack_spec_helpers.rb | 8 +- .../requests/rack_attack_shared_examples.rb | 65 ++++++--- 9 files changed, 257 insertions(+), 120 deletions(-) diff --git a/lib/gitlab/auth/request_authenticator.rb b/lib/gitlab/auth/request_authenticator.rb index 0948663b4b3..0d376cdbd2e 100644 --- a/lib/gitlab/auth/request_authenticator.rb +++ b/lib/gitlab/auth/request_authenticator.rb @@ -13,6 +13,10 @@ module Gitlab @request = request end + def find_authenticated_requester(request_formats) + user(request_formats) || deploy_token_from_request + end + def user(request_formats) request_formats.each do |format| user = find_sessionless_user(format) @@ -84,7 +88,8 @@ module Gitlab def route_authentication_setting @route_authentication_setting ||= { job_token_allowed: api_request?, - basic_auth_personal_access_token: api_request? || git_request? + basic_auth_personal_access_token: api_request? || git_request?, + deploy_token_allowed: api_request? || git_request? } end diff --git a/lib/gitlab/metrics/subscribers/rack_attack.rb b/lib/gitlab/metrics/subscribers/rack_attack.rb index d86c0f83c6c..70dcc6fad90 100644 --- a/lib/gitlab/metrics/subscribers/rack_attack.rb +++ b/lib/gitlab/metrics/subscribers/rack_attack.rb @@ -73,12 +73,16 @@ module Gitlab matched: req.env['rack.attack.matched'] } - 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) # rubocop:disable CodeReuse/ActiveRecord + discriminator = req.env['rack.attack.match_discriminator'].to_s + discriminator_id = discriminator.split(':').last - rack_attack_info[:user_id] = user_id + if discriminator.starts_with?('user:') + user = User.find_by(id: discriminator_id) # rubocop:disable CodeReuse/ActiveRecord + + rack_attack_info[:user_id] = discriminator_id.to_i rack_attack_info['meta.user'] = user.username unless user.nil? + elsif discriminator.starts_with?('deploy_token:') + rack_attack_info[:deploy_token_id] = discriminator_id.to_i end Gitlab::InstrumentationHelper.add_instrumentation_data(rack_attack_info) diff --git a/lib/gitlab/rack_attack.rb b/lib/gitlab/rack_attack.rb index 3f4c0fa45aa..b2664f87306 100644 --- a/lib/gitlab/rack_attack.rb +++ b/lib/gitlab/rack_attack.rb @@ -95,7 +95,7 @@ module Gitlab authenticated_options = Gitlab::Throttle.options(throttle, authenticated: true) throttle_or_track(rack_attack, "throttle_authenticated_#{throttle}", authenticated_options) do |req| if req.throttle?(throttle, authenticated: true) - req.throttled_user_id([:api]) + req.throttled_identifer([:api]) end end end @@ -117,7 +117,7 @@ module Gitlab throttle_or_track(rack_attack, 'throttle_authenticated_web', Gitlab::Throttle.authenticated_web_options) do |req| if req.throttle_authenticated_web? - req.throttled_user_id([:api, :rss, :ics]) + req.throttled_identifer([:api, :rss, :ics]) end end @@ -129,19 +129,19 @@ module Gitlab throttle_or_track(rack_attack, 'throttle_authenticated_protected_paths_api', Gitlab::Throttle.protected_paths_options) do |req| if req.throttle_authenticated_protected_paths_api? - req.throttled_user_id([:api]) + req.throttled_identifer([:api]) end end throttle_or_track(rack_attack, 'throttle_authenticated_protected_paths_web', Gitlab::Throttle.protected_paths_options) do |req| if req.throttle_authenticated_protected_paths_web? - req.throttled_user_id([:api, :rss, :ics]) + req.throttled_identifer([:api, :rss, :ics]) end end throttle_or_track(rack_attack, 'throttle_authenticated_git_lfs', Gitlab::Throttle.throttle_authenticated_git_lfs_options) do |req| if req.throttle_authenticated_git_lfs? - req.throttled_user_id([:api]) + req.throttled_identifer([:api]) end end diff --git a/lib/gitlab/rack_attack/request.rb b/lib/gitlab/rack_attack/request.rb index b24afd28dd7..08a5ddb6ad1 100644 --- a/lib/gitlab/rack_attack/request.rb +++ b/lib/gitlab/rack_attack/request.rb @@ -9,18 +9,22 @@ module Gitlab GROUP_PATH_REGEX = %r{^/api/v\d+/groups/[^/]+/?$}.freeze def unauthenticated? - !(authenticated_user_id([:api, :rss, :ics]) || authenticated_runner_id) + !(authenticated_identifier([:api, :rss, :ics]) || authenticated_runner_id) end - def throttled_user_id(request_formats) - user_id = authenticated_user_id(request_formats) + def throttled_identifer(request_formats) + identifier = authenticated_identifier(request_formats) + return unless identifier - if Gitlab::RackAttack.user_allowlist.include?(user_id) + identifier_type = identifier[:identifier_type] + identifier_id = identifier[:identifier_id] + + if identifier_type == :user && Gitlab::RackAttack.user_allowlist.include?(identifier_id) Gitlab::Instrumentation::Throttle.safelist = 'throttle_user_allowlist' return end - user_id + "#{identifier_type}:#{identifier_id}" end def authenticated_runner_id @@ -169,8 +173,18 @@ module Gitlab private - def authenticated_user_id(request_formats) - request_authenticator.user(request_formats)&.id + def authenticated_identifier(request_formats) + requester = request_authenticator.find_authenticated_requester(request_formats) + + return unless requester + + identifier_type = if requester.is_a?(DeployToken) + :deploy_token + else + :user + end + + { identifier_type: identifier_type, identifier_id: requester.id } end def request_authenticator diff --git a/spec/lib/gitlab/auth/request_authenticator_spec.rb b/spec/lib/gitlab/auth/request_authenticator_spec.rb index 2bc80edb98c..0ce5e6a7f5c 100644 --- a/spec/lib/gitlab/auth/request_authenticator_spec.rb +++ b/spec/lib/gitlab/auth/request_authenticator_spec.rb @@ -76,6 +76,38 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do end end + describe '#find_authenticated_requester' do + let_it_be(:api_user) { create(:user) } + let_it_be(:deploy_token_user) { create(:user) } + + it 'returns the deploy token if it exists' do + allow_next_instance_of(described_class) do |authenticator| + expect(authenticator).to receive(:deploy_token_from_request).and_return(deploy_token_user) + allow(authenticator).to receive(:user).and_return(nil) + end + + expect(subject.find_authenticated_requester([:api])).to eq deploy_token_user + end + + it 'returns the user id if it exists' do + allow_next_instance_of(described_class) do |authenticator| + allow(authenticator).to receive(:deploy_token_from_request).and_return(deploy_token_user) + expect(authenticator).to receive(:user).and_return(api_user) + end + + expect(subject.find_authenticated_requester([:api])).to eq api_user + end + + it 'rerturns nil if no match is found' do + allow_next_instance_of(described_class) do |authenticator| + expect(authenticator).to receive(:deploy_token_from_request).and_return(nil) + expect(authenticator).to receive(:user).and_return(nil) + end + + expect(subject.find_authenticated_requester([:api])).to eq nil + end + end + describe '#find_sessionless_user' do let_it_be(:dependency_proxy_user) { build(:user) } let_it_be(:access_token_user) { build(:user) } @@ -380,10 +412,10 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do describe '#route_authentication_setting' do using RSpec::Parameterized::TableSyntax - where(:script_name, :expected_job_token_allowed, :expected_basic_auth_personal_access_token) do - '/api/endpoint' | true | true - '/namespace/project.git' | false | true - '/web/endpoint' | false | false + where(:script_name, :expected_job_token_allowed, :expected_basic_auth_personal_access_token, :expected_deploy_token_allowed) do + '/api/endpoint' | true | true | true + '/namespace/project.git' | false | true | true + '/web/endpoint' | false | false | false end with_them do @@ -394,7 +426,8 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do it 'returns correct settings' do expect(subject.send(:route_authentication_setting)).to eql({ job_token_allowed: expected_job_token_allowed, - basic_auth_personal_access_token: expected_basic_auth_personal_access_token + basic_auth_personal_access_token: expected_basic_auth_personal_access_token, + deploy_token_allowed: expected_deploy_token_allowed }) end end diff --git a/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb b/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb index 2d595632772..fda4b94bd78 100644 --- a/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb @@ -91,72 +91,110 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do end end - context 'when matched throttle requires user information' do - context 'when user not found' do - let(:event) do - ActiveSupport::Notifications::Event.new( - event_name, Time.current, Time.current + 2.seconds, '1', request: double( - :request, - ip: '1.2.3.4', - request_method: 'GET', - fullpath: '/api/v4/internal/authorized_keys', - env: { - 'rack.attack.match_type' => match_type, - 'rack.attack.matched' => 'throttle_authenticated_api', - 'rack.attack.match_discriminator' => 'not_exist_user_id' - } + context 'matching user or deploy token authenticated information' do + context 'when matching for user' do + context 'when user not found' do + let(:event) do + ActiveSupport::Notifications::Event.new( + event_name, Time.current, Time.current + 2.seconds, '1', request: double( + :request, + ip: '1.2.3.4', + request_method: 'GET', + fullpath: '/api/v4/internal/authorized_keys', + env: { + 'rack.attack.match_type' => match_type, + 'rack.attack.matched' => 'throttle_authenticated_api', + 'rack.attack.match_discriminator' => "user:#{non_existing_record_id}" + } + ) ) - ) + end + + it 'logs request information and user id' do + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Rack_Attack', + env: match_type, + remote_ip: '1.2.3.4', + request_method: 'GET', + path: '/api/v4/internal/authorized_keys', + matched: 'throttle_authenticated_api', + user_id: non_existing_record_id + ) + ) + subscriber.send(match_type, event) + end end - it 'logs request information and user id' do - expect(Gitlab::AuthLogger).to receive(:error).with( - include( - message: 'Rack_Attack', - env: match_type, - remote_ip: '1.2.3.4', - request_method: 'GET', - path: '/api/v4/internal/authorized_keys', - matched: 'throttle_authenticated_api', - user_id: 'not_exist_user_id' + context 'when user found' do + let(:user) { create(:user) } + let(:event) do + ActiveSupport::Notifications::Event.new( + event_name, Time.current, Time.current + 2.seconds, '1', request: double( + :request, + ip: '1.2.3.4', + request_method: 'GET', + fullpath: '/api/v4/internal/authorized_keys', + env: { + 'rack.attack.match_type' => match_type, + 'rack.attack.matched' => 'throttle_authenticated_api', + 'rack.attack.match_discriminator' => "user:#{user.id}" + } + ) ) - ) - subscriber.send(match_type, event) + end + + it 'logs request information and user meta' do + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Rack_Attack', + env: match_type, + remote_ip: '1.2.3.4', + request_method: 'GET', + path: '/api/v4/internal/authorized_keys', + matched: 'throttle_authenticated_api', + user_id: user.id, + 'meta.user' => user.username + ) + ) + subscriber.send(match_type, event) + end end end - context 'when user found' do - let(:user) { create(:user) } - let(:event) do - ActiveSupport::Notifications::Event.new( - event_name, Time.current, Time.current + 2.seconds, '1', request: double( - :request, - ip: '1.2.3.4', - request_method: 'GET', - fullpath: '/api/v4/internal/authorized_keys', - env: { - 'rack.attack.match_type' => match_type, - 'rack.attack.matched' => 'throttle_authenticated_api', - 'rack.attack.match_discriminator' => user.id - } + context 'when matching for deploy token' do + context 'when deploy token found' do + let(:deploy_token) { create(:deploy_token) } + let(:event) do + ActiveSupport::Notifications::Event.new( + event_name, Time.current, Time.current + 2.seconds, '1', request: double( + :request, + ip: '1.2.3.4', + request_method: 'GET', + fullpath: '/api/v4/internal/authorized_keys', + env: { + 'rack.attack.match_type' => match_type, + 'rack.attack.matched' => 'throttle_authenticated_api', + 'rack.attack.match_discriminator' => "deploy_token:#{deploy_token.id}" + } + ) ) - ) - end - - it 'logs request information and user meta' do - expect(Gitlab::AuthLogger).to receive(:error).with( - include( - message: 'Rack_Attack', - env: match_type, - remote_ip: '1.2.3.4', - request_method: 'GET', - path: '/api/v4/internal/authorized_keys', - matched: 'throttle_authenticated_api', - user_id: user.id, - 'meta.user' => user.username + end + + it 'logs request information and user meta' do + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Rack_Attack', + env: match_type, + remote_ip: '1.2.3.4', + request_method: 'GET', + path: '/api/v4/internal/authorized_keys', + matched: 'throttle_authenticated_api', + deploy_token_id: deploy_token.id + ) ) - ) - subscriber.send(match_type, event) + subscriber.send(match_type, event) + end end end end diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index f2126e3cf9c..115f78a5600 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -93,28 +93,28 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } let(:other_user_request_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' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the OAuth headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in basic auth' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, basic_auth_headers(user, token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, basic_auth_headers(other_user, other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with a read_api scope' do @@ -127,14 +127,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } let(:other_user_request_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' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the OAuth headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end end end @@ -155,14 +155,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(api_partial_url, oauth_access_token: token), {}] } let(:other_user_request_args) { [api(api_partial_url, oauth_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with a read_api scope' do @@ -171,7 +171,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(read_token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_read_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end end @@ -184,7 +184,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [rss_url(user), params: nil] } let(:other_user_request_args) { [rss_url(other_user), params: nil] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end end @@ -288,14 +288,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } let(:other_user_request_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' + it_behaves_like 'rate-limited user based token-authenticated requests' end end @@ -444,14 +444,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } let(:other_user_request_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' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'precedence over authenticated api throttle' do @@ -512,6 +512,16 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac end end end + + context 'authenticated via deploy token headers' do + let(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true, projects: [project]) } + let(:other_deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } + + let(:request_args) { [api(api_partial_url), { headers: deploy_token_headers(deploy_token) }] } + let(:other_user_request_args) { [api(api_partial_url), { headers: deploy_token_headers(other_deploy_token) }] } + + it_behaves_like 'rate-limited deploy-token-authenticated requests' + end end end @@ -558,7 +568,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac end end - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'getting a blob' do @@ -568,7 +578,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:path) { "/v2/#{blob.group.path}/dependency_proxy/containers/alpine/blobs/sha256:a0d0a0d46f8b52473982a3c466318f479767577551a53ffc9074c9fa7035982e" } let(:other_path) { "/v2/#{other_blob.group.path}/dependency_proxy/containers/alpine/blobs/sha256:a0d0a0d46f8b52473982a3c466318f479767577551a53ffc9074c9fa7035982e" } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end end @@ -598,7 +608,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [git_lfs_url, { headers: basic_auth_headers(user, token) }] } let(:other_user_request_args) { [git_lfs_url, { headers: basic_auth_headers(other_user, other_user_token) }] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'precedence over authenticated web throttle' do @@ -786,14 +796,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } let(:other_user_request_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' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'precedence over authenticated api throttle' do @@ -993,14 +1003,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(path, personal_access_token: token), {}] } let(:other_user_request_args) { [api(path, personal_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(path, personal_access_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(path, personal_access_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'precedence over authenticated api throttle' do diff --git a/spec/support/helpers/rack_attack_spec_helpers.rb b/spec/support/helpers/rack_attack_spec_helpers.rb index c82a87dc58e..6c06781df03 100644 --- a/spec/support/helpers/rack_attack_spec_helpers.rb +++ b/spec/support/helpers/rack_attack_spec_helpers.rb @@ -10,11 +10,11 @@ module RackAttackSpecHelpers end def private_token_headers(user) - { 'HTTP_PRIVATE_TOKEN' => user.private_token } + { Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => user.private_token } end def personal_access_token_headers(personal_access_token) - { 'HTTP_PRIVATE_TOKEN' => personal_access_token.token } + { Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => personal_access_token.token } end def oauth_token_headers(oauth_access_token) @@ -26,6 +26,10 @@ module RackAttackSpecHelpers { 'AUTHORIZATION' => "Basic #{encoded_login}" } end + def deploy_token_headers(deploy_token) + basic_auth_headers(deploy_token, deploy_token) + end + def expect_rejection(name = nil, &block) yield 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 c6c6c44dce8..68cb91d7414 100644 --- a/spec/support/shared_examples/requests/rack_attack_shared_examples.rb +++ b/spec/support/shared_examples/requests/rack_attack_shared_examples.rb @@ -8,7 +8,50 @@ # * requests_per_period # * period_in_seconds # * period -RSpec.shared_examples 'rate-limited token-authenticated requests' do +RSpec.shared_examples 'rate-limited user based token-authenticated requests' do + context 'when the throttle is enabled' do + before do + settings_to_set[:"#{throttle_setting_prefix}_enabled"] = true + stub_application_setting(settings_to_set) + end + + it 'does not reject requests if the user is in the allowlist' do + stub_env('GITLAB_THROTTLE_USER_ALLOWLIST', user.id.to_s) + Gitlab::RackAttack.configure_user_allowlist + + expect(Gitlab::Instrumentation::Throttle).to receive(:safelist=).with('throttle_user_allowlist').at_least(:once) + + (requests_per_period + 1).times do + make_request(request_args) + expect(response).not_to have_gitlab_http_status(:too_many_requests) + end + + stub_env('GITLAB_THROTTLE_USER_ALLOWLIST', nil) + Gitlab::RackAttack.configure_user_allowlist + end + end + + include_examples 'rate-limited token requests' do + let(:log_data) do + { + user_id: user.id, + 'meta.user' => user.username + } + end + end +end + +RSpec.shared_examples 'rate-limited deploy-token-authenticated requests' do + include_examples 'rate-limited token requests' do + let(:log_data) do + { + deploy_token_id: deploy_token.id + } + end + end +end + +RSpec.shared_examples 'rate-limited token requests' do let(:throttle_types) do { "throttle_protected_paths" => "throttle_authenticated_protected_paths_api", @@ -51,18 +94,6 @@ RSpec.shared_examples 'rate-limited token-authenticated requests' do expect_rejection { make_request(request_args) } end - it 'does not reject requests if the user is in the allowlist' do - stub_env('GITLAB_THROTTLE_USER_ALLOWLIST', user.id.to_s) - Gitlab::RackAttack.configure_user_allowlist - - expect(Gitlab::Instrumentation::Throttle).to receive(:safelist=).with('throttle_user_allowlist').at_least(:once) - - (requests_per_period + 1).times do - make_request(request_args) - expect(response).not_to have_gitlab_http_status(:too_many_requests) - end - end - it 'allows requests after throttling and then waiting for the next period' do requests_per_period.times do make_request(request_args) @@ -81,7 +112,7 @@ RSpec.shared_examples 'rate-limited token-authenticated requests' do end end - it 'counts requests from different users separately, even from the same IP' do + it 'counts requests from different requesters separately, even from the same IP' do requests_per_period.times do make_request(request_args) expect(response).not_to have_gitlab_http_status(:too_many_requests) @@ -92,7 +123,7 @@ RSpec.shared_examples 'rate-limited token-authenticated requests' do expect(response).not_to have_gitlab_http_status(:too_many_requests) end - it 'counts all requests from the same user, even via different IPs' do + it 'counts all requests from the same requesters, even via different IPs' do requests_per_period.times do make_request(request_args) expect(response).not_to have_gitlab_http_status(:too_many_requests) @@ -122,10 +153,8 @@ RSpec.shared_examples 'rate-limited token-authenticated requests' do remote_ip: '127.0.0.1', request_method: request_method, path: request_args.first, - user_id: user.id, - 'meta.user' => user.username, matched: throttle_types[throttle_setting_prefix] - }) + }.merge(log_data)) expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once -- cgit v1.2.3