Commit b3926b03 authored by Josianne Hyson's avatar Josianne Hyson

Increase update export rate limits to match import

We want to have consistent rate limits across project/group import
and exports. Update all these endpoints to use matching rate keys and
scopes.

Also increase export rate limit to be consistent with the import rate
limit as currently the export limit is to restrictive.

MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/33026

Original issue: https://gitlab.com/gitlab-org/gitlab/-/issues/212707
parent 051c4be4
......@@ -264,11 +264,12 @@ class GroupsController < Groups::ApplicationController
def export_rate_limit
prefixed_action = "group_#{params[:action]}".to_sym
if Gitlab::ApplicationRateLimiter.throttled?(prefixed_action, scope: [current_user, prefixed_action, @group])
scope = params[:action] == :download_export ? @group : nil
if Gitlab::ApplicationRateLimiter.throttled?(prefixed_action, scope: [current_user, scope].compact)
Gitlab::ApplicationRateLimiter.log_request(request, "#{prefixed_action}_request_limit".to_sym, current_user)
flash[:alert] = _('This endpoint has been requested too many times. Try again later.')
redirect_to edit_group_path(@group)
render plain: _('This endpoint has been requested too many times. Try again later.'), status: :too_many_requests
end
end
......
......@@ -485,7 +485,7 @@ class ProjectsController < Projects::ApplicationController
project_scope = params[:action] == :download_export ? @project : nil
if rate_limiter.throttled?(prefixed_action, scope: [current_user, prefixed_action, project_scope].compact)
if rate_limiter.throttled?(prefixed_action, scope: [current_user, project_scope].compact)
rate_limiter.log_request(request, "#{prefixed_action}_request_limit".to_sym, current_user)
render plain: _('This endpoint has been requested too many times. Try again later.'), status: :too_many_requests
......
......@@ -92,7 +92,7 @@ For example:
To help avoid abuse, users are rate limited to:
| Request Type | Limit |
| ---------------- | ------------------------------ |
| Export | 1 group every 5 minutes |
| Download export | 10 downloads every 10 minutes |
| Request Type | Limit |
| ---------------- | ---------------------------------------- |
| Export | 30 groups every 5 minutes |
| Download export | 10 downloads per group every 10 minutes |
......@@ -162,8 +162,8 @@ all imported projects are given the visibility of `Private`.
To help avoid abuse, users are rate limited to:
| Request Type | Limit |
| ---------------- | --------------------------- |
| Export | 1 project per 5 minutes |
| Download export | 10 projects per 10 minutes |
| Import | 30 projects per 5 minutes |
| Request Type | Limit |
| ---------------- | ----------------------------------------- |
| Export | 30 projects per 5 minutes |
| Download export | 10 downloads per project every 10 minutes |
| Import | 30 projects per 5 minutes |
......@@ -2,6 +2,8 @@
module API
class GroupExport < Grape::API
helpers Helpers::RateLimiter
before do
not_found! unless Feature.enabled?(:group_import_export, user_group, default_enabled: true)
......@@ -16,6 +18,8 @@ module API
detail 'This feature was introduced in GitLab 12.5.'
end
get ':id/export/download' do
check_rate_limit! :group_download_export, [current_user, user_group]
if user_group.export_file_exists?
present_carrierwave_file!(user_group.export_file)
else
......@@ -27,6 +31,8 @@ module API
detail 'This feature was introduced in GitLab 12.5.'
end
post ':id/export' do
check_rate_limit! :group_export, [current_user]
export_service = ::Groups::ImportExport::ExportService.new(group: user_group, user: current_user)
if export_service.async_execute
......
......@@ -25,7 +25,7 @@ module API
detail 'This feature was introduced in GitLab 10.6.'
end
get ':id/export/download' do
check_rate_limit! :project_download_export, [current_user, :project_download_export, user_project]
check_rate_limit! :project_download_export, [current_user, user_project]
if user_project.export_file_exists?
present_carrierwave_file!(user_project.export_file)
......@@ -45,7 +45,7 @@ module API
end
end
post ':id/export' do
check_rate_limit! :project_export, [current_user, :project_export]
check_rate_limit! :project_export, [current_user]
project_export_params = declared_params(include_missing: false)
after_export_params = project_export_params.delete(:upload) || {}
......
......@@ -20,14 +20,14 @@ module Gitlab
def rate_limits
{
issues_create: { threshold: -> { Gitlab::CurrentSettings.current_application_settings.issues_create_limit }, interval: 1.minute },
project_export: { threshold: 1, interval: 5.minutes },
project_export: { threshold: 30, interval: 5.minutes },
project_download_export: { threshold: 10, interval: 10.minutes },
project_repositories_archive: { threshold: 5, interval: 1.minute },
project_generate_new_export: { threshold: 1, interval: 5.minutes },
project_generate_new_export: { threshold: 30, interval: 5.minutes },
project_import: { threshold: 30, interval: 5.minutes },
play_pipeline_schedule: { threshold: 1, interval: 1.minute },
show_raw_controller: { threshold: -> { Gitlab::CurrentSettings.current_application_settings.raw_blob_request_limit }, interval: 1.minute },
group_export: { threshold: 1, interval: 5.minutes },
group_export: { threshold: 30, interval: 5.minutes },
group_download_export: { threshold: 10, interval: 10.minutes },
group_import: { threshold: 30, interval: 5.minutes }
}.freeze
......
......@@ -862,14 +862,17 @@ describe GroupsController do
context 'when the endpoint receives requests above the rate limit' do
before do
sign_in(admin)
allow(Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
allow(Gitlab::ApplicationRateLimiter)
.to receive(:increment)
.and_return(Gitlab::ApplicationRateLimiter.rate_limits[:group_export][:threshold] + 1)
end
it 'throttles the endpoint' do
post :export, params: { id: group.to_param }
expect(flash[:alert]).to eq('This endpoint has been requested too many times. Try again later.')
expect(response).to have_gitlab_http_status(:found)
expect(response.body).to eq('This endpoint has been requested too many times. Try again later.')
expect(response).to have_gitlab_http_status :too_many_requests
end
end
end
......@@ -933,14 +936,17 @@ describe GroupsController do
context 'when the endpoint receives requests above the rate limit' do
before do
sign_in(admin)
allow(Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
allow(Gitlab::ApplicationRateLimiter)
.to receive(:increment)
.and_return(Gitlab::ApplicationRateLimiter.rate_limits[:group_download_export][:threshold] + 1)
end
it 'throttles the endpoint' do
get :download_export, params: { id: group.to_param }
expect(flash[:alert]).to eq('This endpoint has been requested too many times. Try again later.')
expect(response).to have_gitlab_http_status(:found)
expect(response.body).to eq('This endpoint has been requested too many times. Try again later.')
expect(response).to have_gitlab_http_status :too_many_requests
end
end
end
......
......@@ -1159,14 +1159,13 @@ describe ProjectsController do
end
shared_examples 'rate limits project export endpoint' do
it 'prevents requesting project export' do
exportable_project = create(:project)
exportable_project.add_maintainer(user)
post action, params: { namespace_id: exportable_project.namespace, id: exportable_project }
expect(response).to have_gitlab_http_status(:found)
before do
allow(Gitlab::ApplicationRateLimiter)
.to receive(:increment)
.and_return(Gitlab::ApplicationRateLimiter.rate_limits["project_#{action}".to_sym][:threshold] + 1)
end
it 'prevents requesting project export' do
post action, params: { namespace_id: project.namespace, id: project }
expect(response.body).to eq('This endpoint has been requested too many times. Try again later.')
......@@ -1228,9 +1227,9 @@ describe ProjectsController do
context 'when the endpoint receives requests above the limit', :clean_gitlab_redis_cache do
before do
allow(::Gitlab::ApplicationRateLimiter)
.to receive(:throttled?)
.and_return(true)
allow(Gitlab::ApplicationRateLimiter)
.to receive(:increment)
.and_return(Gitlab::ApplicationRateLimiter.rate_limits[:project_download_export][:threshold] + 1)
end
it 'prevents requesting project export' do
......
......@@ -82,6 +82,22 @@ describe API::GroupExport do
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when the requests have exceeded the rate limit' do
before do
allow(Gitlab::ApplicationRateLimiter)
.to receive(:increment)
.and_return(Gitlab::ApplicationRateLimiter.rate_limits[:group_download_export][:threshold] + 1)
end
it 'throttles the endpoint' do
get api(download_path, user)
expect(json_response["message"])
.to include('error' => 'This endpoint has been requested too many times. Try again later.')
expect(response).to have_gitlab_http_status :too_many_requests
end
end
end
describe 'POST /groups/:group_id/export' do
......@@ -139,5 +155,23 @@ describe API::GroupExport do
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when the requests have exceeded the rate limit' do
before do
group.add_owner(user)
allow(Gitlab::ApplicationRateLimiter)
.to receive(:increment)
.and_return(Gitlab::ApplicationRateLimiter.rate_limits[:group_export][:threshold] + 1)
end
it 'throttles the endpoint' do
post api(path, user)
expect(json_response["message"])
.to include('error' => 'This endpoint has been requested too many times. Try again later.')
expect(response).to have_gitlab_http_status :too_many_requests
end
end
end
end
......@@ -235,7 +235,9 @@ describe API::ProjectExport, :clean_gitlab_redis_cache do
let(:request) { get api(download_path, admin) }
before do
allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
allow(Gitlab::ApplicationRateLimiter)
.to receive(:increment)
.and_return(Gitlab::ApplicationRateLimiter.rate_limits[:project_download_export][:threshold] + 1)
end
it 'prevents requesting project export' do
......@@ -357,11 +359,13 @@ describe API::ProjectExport, :clean_gitlab_redis_cache do
it_behaves_like 'post project export start'
context 'when rate limit is exceeded across projects' do
it 'prevents requesting project export' do
post api(path_none, admin)
expect(response).not_to have_gitlab_http_status(:too_many_requests)
before do
allow(Gitlab::ApplicationRateLimiter)
.to receive(:increment)
.and_return(Gitlab::ApplicationRateLimiter.rate_limits[:project_export][:threshold] + 1)
end
it 'prevents requesting project export' do
post api(path, admin)
expect(response).to have_gitlab_http_status(:too_many_requests)
......
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