Commit 4acc0443 authored by Igor Drozdov's avatar Igor Drozdov

Merge branch '335075_rate_limiting_for_files_api' into 'master'

Apply throttling settings to Files API

See merge request gitlab-org/gitlab!68561
parents 5735f5bd 2773f95d
......@@ -21,7 +21,8 @@ module Gitlab
:throttle_authenticated_protected_paths_api,
:throttle_authenticated_protected_paths_web,
:throttle_authenticated_packages_api,
:throttle_authenticated_git_lfs
:throttle_authenticated_git_lfs,
:throttle_authenticated_files_api
].freeze
PAYLOAD_KEYS = [
......
......@@ -145,6 +145,18 @@ module Gitlab
end
end
throttle_or_track(rack_attack, 'throttle_unauthenticated_files_api', Gitlab::Throttle.unauthenticated_files_api_options) do |req|
if req.throttle_unauthenticated_files_api?
req.ip
end
end
throttle_or_track(rack_attack, 'throttle_authenticated_files_api', Gitlab::Throttle.authenticated_files_api_options) do |req|
if req.throttle_authenticated_files_api?
req.throttled_user_id([:api])
end
end
rack_attack.safelist('throttle_bypass_header') do |req|
Gitlab::Throttle.bypass_header.present? &&
req.get_header(Gitlab::Throttle.bypass_header) == '1'
......
......@@ -3,6 +3,8 @@
module Gitlab
module RackAttack
module Request
FILES_PATH_REGEX = %r{^/api/v\d+/projects/[^/]+/repository/files/.+}.freeze
def unauthenticated?
!(authenticated_user_id([:api, :rss, :ics]) || authenticated_runner_id)
end
......@@ -61,6 +63,7 @@ module Gitlab
def throttle_unauthenticated?
!should_be_skipped? &&
!throttle_unauthenticated_packages_api? &&
!throttle_unauthenticated_files_api? &&
Gitlab::Throttle.settings.throttle_unauthenticated_enabled &&
unauthenticated?
end
......@@ -68,6 +71,7 @@ module Gitlab
def throttle_authenticated_api?
api_request? &&
!throttle_authenticated_packages_api? &&
!throttle_authenticated_files_api? &&
Gitlab::Throttle.settings.throttle_authenticated_api_enabled
end
......@@ -115,6 +119,19 @@ module Gitlab
Gitlab::Throttle.settings.throttle_authenticated_git_lfs_enabled
end
def throttle_unauthenticated_files_api?
files_api_path? &&
Feature.enabled?(:files_api_throttling, default_enabled: :yaml) &&
Gitlab::Throttle.settings.throttle_unauthenticated_files_api_enabled &&
unauthenticated?
end
def throttle_authenticated_files_api?
files_api_path? &&
Feature.enabled?(:files_api_throttling, default_enabled: :yaml) &&
Gitlab::Throttle.settings.throttle_authenticated_files_api_enabled
end
private
def authenticated_user_id(request_formats)
......@@ -140,6 +157,10 @@ module Gitlab
def git_lfs_path?
path =~ Gitlab::PathRegex.repository_git_lfs_route_regex
end
def files_api_path?
path =~ FILES_PATH_REGEX
end
end
end
end
......
......@@ -70,6 +70,20 @@ module Gitlab
{ limit: limit_proc, period: period_proc }
end
def self.unauthenticated_files_api_options
limit_proc = proc { |req| settings.throttle_unauthenticated_files_api_requests_per_period }
period_proc = proc { |req| settings.throttle_unauthenticated_files_api_period_in_seconds.seconds }
{ limit: limit_proc, period: period_proc }
end
def self.authenticated_files_api_options
limit_proc = proc { |req| settings.throttle_authenticated_files_api_requests_per_period }
period_proc = proc { |req| settings.throttle_authenticated_files_api_period_in_seconds.seconds }
{ limit: limit_proc, period: period_proc }
end
def self.rate_limiting_response_text
(settings.rate_limiting_response_text.presence || DEFAULT_RATE_LIMITING_RESPONSE_TEXT) + "\n"
end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::RackAttack::Request do
describe 'FILES_PATH_REGEX' do
subject { described_class::FILES_PATH_REGEX }
it { is_expected.to match('/api/v4/projects/1/repository/files/README') }
it { is_expected.to match('/api/v4/projects/1/repository/files/README?ref=master') }
it { is_expected.to match('/api/v4/projects/1/repository/files/README/blame') }
it { is_expected.to match('/api/v4/projects/1/repository/files/README/raw') }
it { is_expected.to match('/api/v4/projects/some%2Fnested%2Frepo/repository/files/README') }
it { is_expected.not_to match('/api/v4/projects/some/nested/repo/repository/files/README') }
end
end
......@@ -24,7 +24,11 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac
throttle_authenticated_packages_api_requests_per_period: 100,
throttle_authenticated_packages_api_period_in_seconds: 1,
throttle_authenticated_git_lfs_requests_per_period: 100,
throttle_authenticated_git_lfs_period_in_seconds: 1
throttle_authenticated_git_lfs_period_in_seconds: 1,
throttle_unauthenticated_files_api_requests_per_period: 100,
throttle_unauthenticated_files_api_period_in_seconds: 1,
throttle_authenticated_files_api_requests_per_period: 100,
throttle_authenticated_files_api_period_in_seconds: 1
}
end
......@@ -711,6 +715,212 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac
end
end
describe 'Files API' do
let(:request_method) { 'GET' }
context 'unauthenticated' do
let_it_be(:project) { create(:project, :public, :custom_repo, files: { 'README' => 'foo' }) }
let(:throttle_setting_prefix) { 'throttle_unauthenticated_files_api' }
let(:files_path_that_does_not_require_authentication) { "/api/v4/projects/#{project.id}/repository/files/README?ref=master" }
def do_request
get files_path_that_does_not_require_authentication
end
before do
settings_to_set[:throttle_unauthenticated_files_api_requests_per_period] = requests_per_period
settings_to_set[:throttle_unauthenticated_files_api_period_in_seconds] = period_in_seconds
end
context 'when unauthenticated files api throttle is disabled' do
before do
settings_to_set[:throttle_unauthenticated_files_api_enabled] = false
stub_application_setting(settings_to_set)
end
it 'allows requests over the rate limit' do
(1 + requests_per_period).times do
do_request
expect(response).to have_gitlab_http_status(:ok)
end
end
context 'when unauthenticated api throttle is enabled' do
before do
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_enabled] = true
stub_application_setting(settings_to_set)
end
it 'rejects requests over the unauthenticated api rate limit' do
requests_per_period.times do
do_request
expect(response).to have_gitlab_http_status(:ok)
end
expect_rejection { do_request }
end
end
end
context 'when unauthenticated files api throttle is enabled' do
before do
settings_to_set[:throttle_unauthenticated_files_api_requests_per_period] = requests_per_period # 1
settings_to_set[:throttle_unauthenticated_files_api_period_in_seconds] = period_in_seconds # 10_000
settings_to_set[:throttle_unauthenticated_files_api_enabled] = true
stub_application_setting(settings_to_set)
end
it 'rejects requests over the rate limit' do
requests_per_period.times do
do_request
expect(response).to have_gitlab_http_status(:ok)
end
expect_rejection { do_request }
end
context 'when feature flag is off' do
before do
stub_feature_flags(files_api_throttling: false)
end
it 'allows requests over the rate limit' do
(1 + requests_per_period).times do
do_request
expect(response).to have_gitlab_http_status(:ok)
end
end
end
context 'when unauthenticated api throttle is lower' do
before do
settings_to_set[:throttle_unauthenticated_requests_per_period] = 0
settings_to_set[:throttle_unauthenticated_period_in_seconds] = period_in_seconds
settings_to_set[:throttle_unauthenticated_enabled] = true
stub_application_setting(settings_to_set)
end
it 'ignores unauthenticated api throttle' do
requests_per_period.times do
do_request
expect(response).to have_gitlab_http_status(:ok)
end
expect_rejection { do_request }
end
end
it_behaves_like 'tracking when dry-run mode is set' do
let(:throttle_name) { 'throttle_unauthenticated_files_api' }
end
end
end
context 'authenticated', :api do
let_it_be(:project) { create(:project, :internal, :custom_repo, files: { 'README' => 'foo' }) }
let_it_be(:user) { create(:user) }
let_it_be(:token) { create(:personal_access_token, user: user) }
let_it_be(:other_user) { create(:user) }
let_it_be(:other_user_token) { create(:personal_access_token, user: other_user) }
let(:throttle_setting_prefix) { 'throttle_authenticated_files_api' }
let(:api_partial_url) { "/projects/#{project.id}/repository/files/README?ref=master" }
before do
stub_application_setting(settings_to_set)
end
context 'with the token in the query string' do
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'
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'
end
context 'precedence over authenticated api throttle' do
before do
settings_to_set[:throttle_authenticated_files_api_requests_per_period] = requests_per_period
settings_to_set[:throttle_authenticated_files_api_period_in_seconds] = period_in_seconds
end
def do_request
get api(api_partial_url, personal_access_token: token)
end
context 'when authenticated files api throttle is enabled' do
before do
settings_to_set[:throttle_authenticated_files_api_enabled] = true
end
context 'when authenticated api throttle is lower' do
before do
settings_to_set[:throttle_authenticated_api_requests_per_period] = 0
settings_to_set[:throttle_authenticated_api_period_in_seconds] = period_in_seconds
settings_to_set[:throttle_authenticated_api_enabled] = true
stub_application_setting(settings_to_set)
end
it 'ignores authenticated api throttle' do
requests_per_period.times do
do_request
expect(response).to have_gitlab_http_status(:ok)
end
expect_rejection { do_request }
end
end
context 'when feature flag is off' do
before do
stub_feature_flags(files_api_throttling: false)
end
it 'allows requests over the rate limit' do
(1 + requests_per_period).times do
do_request
expect(response).to have_gitlab_http_status(:ok)
end
end
end
end
context 'when authenticated files api throttle is disabled' do
before do
settings_to_set[:throttle_authenticated_files_api_enabled] = false
end
context 'when authenticated api throttle is enabled' do
before do
settings_to_set[:throttle_authenticated_api_requests_per_period] = requests_per_period
settings_to_set[:throttle_authenticated_api_period_in_seconds] = period_in_seconds
settings_to_set[:throttle_authenticated_api_enabled] = true
stub_application_setting(settings_to_set)
end
it 'rejects requests over the authenticated api rate limit' do
requests_per_period.times do
do_request
expect(response).to have_gitlab_http_status(:ok)
end
expect_rejection { do_request }
end
end
end
end
end
end
describe 'throttle bypass header' do
let(:headers) { {} }
let(:bypass_header) { 'gitlab-bypass-rate-limiting' }
......
# frozen_string_literal: true
#
# Requires let variables:
# * throttle_setting_prefix: "throttle_authenticated_api", "throttle_authenticated_web", "throttle_protected_paths", "throttle_authenticated_packages_api", "throttle_authenticated_git_lfs"
# * throttle_setting_prefix: "throttle_authenticated_api", "throttle_authenticated_web", "throttle_protected_paths", "throttle_authenticated_packages_api", "throttle_authenticated_git_lfs", "throttle_authenticated_files_api"
# * request_method
# * request_args
# * other_user_request_args
......@@ -15,7 +15,8 @@ RSpec.shared_examples 'rate-limited token-authenticated requests' do
"throttle_authenticated_api" => "throttle_authenticated_api",
"throttle_authenticated_web" => "throttle_authenticated_web",
"throttle_authenticated_packages_api" => "throttle_authenticated_packages_api",
"throttle_authenticated_git_lfs" => "throttle_authenticated_git_lfs"
"throttle_authenticated_git_lfs" => "throttle_authenticated_git_lfs",
"throttle_authenticated_files_api" => "throttle_authenticated_files_api"
}
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