Merge branch 'jh-increase-project-rate-limit' into 'master'

Increase export rate limit to match import rate limit

See merge request gitlab-org/gitlab!33026

app/controllers/groups_controller.rb

@@ -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
 

app/controllers/projects_controller.rb

@@ -484,7 +484,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

doc/user/group/settings/import_export.md

@@ -93,6 +93,6 @@ 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  |
+| ---------------- | ---------------------------------------- |
+| Export           | 30 groups every 5 minutes                |
+| Download export  | 10 downloads per group every 10 minutes  |

doc/user/project/settings/import_export.md

@@ -163,7 +163,7 @@ 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  |
+| ---------------- | ----------------------------------------- |
+| Export           | 30 projects per 5 minutes                 |
+| Download export  | 10 downloads per project every 10 minutes |
 | Import           | 30 projects per 5 minutes                 |

lib/api/group_export.rb

@@ -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

lib/api/project_export.rb

@@ -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) || {}

lib/gitlab/application_rate_limiter.rb

@@ -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

spec/controllers/groups_controller_spec.rb

@@ -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

spec/controllers/projects_controller_spec.rb

@@ -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

spec/requests/api/group_export_spec.rb

@@ -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

spec/requests/api/project_export_spec.rb

@@ -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)