Commit 484fe4b8 authored by Pavel Shutsin's avatar Pavel Shutsin

Merge branch 'fix-project-export-download-throttling' into 'master'

Fix scope of project export download throttling

See merge request gitlab-org/gitlab!82925
parents c8389f10 ec7b9c18
...@@ -541,9 +541,9 @@ class ProjectsController < Projects::ApplicationController ...@@ -541,9 +541,9 @@ class ProjectsController < Projects::ApplicationController
def check_export_rate_limit! def check_export_rate_limit!
prefixed_action = "project_#{params[:action]}".to_sym prefixed_action = "project_#{params[:action]}".to_sym
project_scope = params[:action] == 'download_export' ? @project : nil group_scope = params[:action] == 'download_export' ? @project.namespace : nil
check_rate_limit!(prefixed_action, scope: [current_user, project_scope].compact) check_rate_limit!(prefixed_action, scope: [current_user, group_scope].compact)
end end
def render_edit def render_edit
......
...@@ -1443,14 +1443,15 @@ RSpec.describe ProjectsController do ...@@ -1443,14 +1443,15 @@ RSpec.describe ProjectsController do
end end
describe '#download_export', :clean_gitlab_redis_rate_limiting do describe '#download_export', :clean_gitlab_redis_rate_limiting do
let(:project) { create(:project, :with_export, service_desk_enabled: false) }
let(:action) { :download_export } let(:action) { :download_export }
context 'object storage enabled' do context 'object storage enabled' do
context 'when project export is enabled' do context 'when project export is enabled' do
it 'returns 302' do it 'returns 200' do
get action, params: { namespace_id: project.namespace, id: project } get action, params: { namespace_id: project.namespace, id: project }
expect(response).to have_gitlab_http_status(:found) expect(response).to have_gitlab_http_status(:ok)
end end
end end
...@@ -1490,14 +1491,37 @@ RSpec.describe ProjectsController do ...@@ -1490,14 +1491,37 @@ RSpec.describe ProjectsController do
expect(response.body).to eq('This endpoint has been requested too many times. Try again later.') 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) expect(response).to have_gitlab_http_status(:too_many_requests)
end end
end
context 'applies correct scope when throttling', :clean_gitlab_redis_rate_limiting do
before do
stub_application_setting(project_download_export_limit: 1)
end
it 'applies correct scope when throttling' do it 'applies throttle per namespace' do
expect(Gitlab::ApplicationRateLimiter) expect(Gitlab::ApplicationRateLimiter)
.to receive(:throttled?) .to receive(:throttled?)
.with(:project_download_export, scope: [user, project]) .with(:project_download_export, scope: [user, project.namespace])
post action, params: { namespace_id: project.namespace, id: project } post action, params: { namespace_id: project.namespace, id: project }
end end
it 'throttles downloads within same namespaces' do
# simulate prior request to the same namespace, which increments the rate limit counter for that scope
Gitlab::ApplicationRateLimiter.throttled?(:project_download_export, scope: [user, project.namespace])
get action, params: { namespace_id: project.namespace, id: project }
expect(response).to have_gitlab_http_status(:too_many_requests)
end
it 'allows downloads from different namespaces' do
# simulate prior request to a different namespace, which increments the rate limit counter for that scope
Gitlab::ApplicationRateLimiter.throttled?(:project_download_export,
scope: [user, create(:project, :with_export).namespace])
get action, params: { namespace_id: project.namespace, id: project }
expect(response).to have_gitlab_http_status(:ok)
end
end end
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