Merge branch 'georgekoltsov/import-export-rate-limits-as-application-settings' into 'master'

Convert Import/Export rate limits to application settings

See merge request !35728

app/helpers/application_settings_helper.rb

@@ -321,7 +321,13 @@ def visible_attributes
       :email_restrictions_enabled,
       :email_restrictions,
       :issues_create_limit,
-      :raw_blob_request_limit
+      :raw_blob_request_limit,
+      :project_import_limit,
+      :project_export_limit,
+      :project_download_export_limit,
+      :group_import_limit,
+      :group_export_limit,
+      :group_download_export_limit
     ]
   end
 

app/models/application_setting_implementation.rb

@@ -158,7 +158,13 @@ def defaults
         snowplow_iglu_registry_url: nil,
         custom_http_clone_url_root: nil,
         productivity_analytics_start_date: Time.current,
-        snippet_size_limit: 50.megabytes
+        snippet_size_limit: 50.megabytes,
+        project_import_limit: 6,
+        project_export_limit: 6,
+        project_download_export_limit: 1,
+        group_import_limit: 6,
+        group_export_limit: 6,
+        group_download_export_limit: 1
       }
     end
 

app/views/admin/application_settings/_import_export_limits.html.haml

+= form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-import-export-limits-settings'), html: { class: 'fieldset-form' } do |f|
+  = form_errors(@application_setting)
+
+  %fieldset
+    .form-group
+      = f.label :project_import_limit, _('Max Project Import requests per minute per user'), class: 'label-bold'
+      = f.number_field :project_import_limit, class: 'form-control'
+
+  %fieldset
+    .form-group
+      = f.label :project_export_limit, _('Max Project Export requests per minute per user'), class: 'label-bold'
+      = f.number_field :project_export_limit, class: 'form-control'
+
+  %fieldset
+    .form-group
+      = f.label :project_download_export_limit, _('Max Project Export Download requests per minute per user'), class: 'label-bold'
+      = f.number_field :project_download_export_limit, class: 'form-control'
+
+  %fieldset
+    .form-group
+      = f.label :group_import_limit, _('Max Group Import requests per minute per user'), class: 'label-bold'
+      = f.number_field :group_import_limit, class: 'form-control'
+
+  %fieldset
+    .form-group
+      = f.label :group_export_limit, _('Max Group Export requests per minute per user'), class: 'label-bold'
+      = f.number_field :group_export_limit, class: 'form-control'
+
+  %fieldset
+    .form-group
+      = f.label :group_download_export_limit, _('Max Group Export Download requests per minute per user'), class: 'label-bold'
+      = f.number_field :group_download_export_limit, class: 'form-control'
+
+  = f.submit 'Save changes', class: "btn btn-success", data: { qa_selector: 'save_changes_button' }

app/views/admin/application_settings/network.html.haml

@@ -57,4 +57,15 @@
   .settings-content
     = render 'issue_limits'
 
+%section.settings.as-import-export-limits.no-animate#js-import-export-limits-settings{ class: ('expanded' if expanded_by_default?) }
+  .settings-header
+    %h4
+      = _('Import/Export Rate Limits')
+    %button.btn.btn-default.js-settings-toggle{ type: 'button' }
+      = expanded_by_default? ? _('Collapse') : _('Expand')
+    %p
+      = _('Configure limits for Project/Group Import/Export.')
+  .settings-content
+    = render 'import_export_limits'
+
 = render_if_exists 'admin/application_settings/ee_network_settings'

changelogs/unreleased/georgekoltsov-import-export-rate-limits-as-application-settings.yml

+---
+title: Convert Import/Export rate limits to configurable application settings
+merge_request: 35728
+author:
+type: added

db/migrate/20200701093859_add_import_export_limits_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddImportExportLimitsToApplicationSettings < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    add_column :application_settings, :project_import_limit, :integer, default: 6, null: false
+    add_column :application_settings, :project_export_limit, :integer, default: 6, null: false
+    add_column :application_settings, :project_download_export_limit, :integer, default: 1, null: false
+
+    add_column :application_settings, :group_import_limit, :integer, default: 6, null: false
+    add_column :application_settings, :group_export_limit, :integer, default: 6, null: false
+    add_column :application_settings, :group_download_export_limit, :integer, default: 1, null: false
+  end
+end

db/structure.sql

@@ -9145,6 +9145,12 @@ CREATE TABLE public.application_settings (
     compliance_frameworks smallint[] DEFAULT '{}'::smallint[] NOT NULL,
     notify_on_unknown_sign_in boolean DEFAULT true NOT NULL,
     default_branch_name text,
+    project_import_limit integer DEFAULT 6 NOT NULL,
+    project_export_limit integer DEFAULT 6 NOT NULL,
+    project_download_export_limit integer DEFAULT 1 NOT NULL,
+    group_import_limit integer DEFAULT 6 NOT NULL,
+    group_export_limit integer DEFAULT 6 NOT NULL,
+    group_download_export_limit integer DEFAULT 1 NOT NULL,
     CONSTRAINT check_51700b31b5 CHECK ((char_length(default_branch_name) <= 255)),
     CONSTRAINT check_d03919528d CHECK ((char_length(container_registry_vendor) <= 255)),
     CONSTRAINT check_d820146492 CHECK ((char_length(spam_check_endpoint_url) <= 255)),
@@ -23577,6 +23583,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200626060151
 20200626130220
 20200630110826
+20200701093859
 20200702123805
 20200703154822
 20200704143633

doc/security/rate_limits.md

@@ -26,6 +26,7 @@ similarly mitigated by a rate limit.
 - [User and IP rate limits](../user/admin_area/settings/user_and_ip_rate_limits.md).
 - [Raw endpoints rate limits](../user/admin_area/settings/rate_limits_on_raw_endpoints.md).
 - [Protected paths](../user/admin_area/settings/protected_paths.md).
+- [Import/Export rate limits](../user/admin_area/settings/import_export_rate_limits.md).
 
 ## Rack Attack initializer
 

doc/user/admin_area/settings/import_export_rate_limits.md

+---
+type: reference
+stage: Manage
+group: Import
+info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers
+---
+
+# Project/Group Import/Export rate limits
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/35728) in GitLab 13.2.
+
+The following table includes configurable rate limits. The following table includes limits on a
+per minute per user basis:
+
+| Limit                    | Default (per minute per user) |
+|--------------------------|-------------------------------|
+| Project Import           | 6                             |
+| Project Export           | 6                             |
+| Project Export Download  | 1                             |
+| Group Import             | 6                             |
+| Group Export             | 6                             |
+| Group Export Download    | 1                             |
+
+All rate limits are:
+
+- Configurable at **(admin)** **Admin Area > Settings > Network > Import/Export Rate Limits**
+- Applied per minute per user
+- Not applied per IP address
+- Active by default. To disable, set the option to `0`
+- Logged to `auth.log` file if exceed rate limit
+
+![Import/Export rate limits](img/import_export_rate_limits_v13_2.png)

doc/user/admin_area/settings/rate_limit_on_issues_creation.md

@@ -11,7 +11,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 
 This setting allows you to rate limit the requests to the issue creation endpoint.
 It defaults to 300 requests per minute.
-You can change it in **Admin Area > Settings > Network > Performance Optimization**.
+You can change it in **Admin Area > Settings > Network > Issues Rate Limits**.
 
 For example, requests using the
 [Projects::IssuesController#create](https://gitlab.com/gitlab-org/gitlab/raw/master/app/controllers/projects/issues_controller.rb)

lib/gitlab/application_rate_limiter.rb

@@ -19,17 +19,17 @@ class << self
       # and only do that when it's needed.
       def rate_limits
         {
-          issues_create:                { threshold: -> { Gitlab::CurrentSettings.current_application_settings.issues_create_limit }, interval: 1.minute },
-          project_export:               { threshold: 30, interval: 5.minutes },
-          project_download_export:      { threshold: 10, interval: 10.minutes },
+          issues_create:                { threshold: -> { application_settings.issues_create_limit }, interval: 1.minute },
+          project_export:               { threshold: -> { application_settings.project_export_limit }, interval: 1.minute },
+          project_download_export:      { threshold: -> { application_settings.project_download_export_limit }, interval: 1.minute },
           project_repositories_archive: { threshold: 5, interval: 1.minute },
-          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: 30, interval: 5.minutes },
-          group_download_export:       { threshold: 10, interval: 10.minutes },
-          group_import:                { threshold: 30, interval: 5.minutes }
+          project_generate_new_export:  { threshold: -> { application_settings.project_export_limit }, interval: 1.minute },
+          project_import:               { threshold: -> { application_settings.project_import_limit }, interval: 1.minute },
+          play_pipeline_schedule:       { threshold: 1, interval: 1.minute },
+          show_raw_controller:          { threshold: -> { application_settings.raw_blob_request_limit }, interval: 1.minute },
+          group_export:                 { threshold: -> { application_settings.group_export_limit }, interval: 1.minute },
+          group_download_export:        { threshold: -> { application_settings.group_download_export_limit }, interval: 1.minute },
+          group_import:                 { threshold: -> { application_settings.group_import_limit }, interval: 1.minute }
         }.freeze
       end
 
@@ -130,6 +130,10 @@ def action_key(key, scope)
 
         "application_rate_limiter:#{serialized}"
       end
+
+      def application_settings
+        Gitlab::CurrentSettings.current_application_settings
+      end
     end
   end
 end

locale/gitlab.pot

@@ -6082,6 +6082,9 @@ msgstr ""
 msgid "Configure limit for issues created per minute by web and API requests."
 msgstr ""
+msgid "Configure limits for Project/Group Import/Export."
+msgstr ""
+
 msgid "Configure limits for web and API requests."
 msgstr ""
@@ -12226,6 +12229,9 @@ msgstr ""
 msgid "Import timed out. Import took longer than %{import_jobs_expiration} seconds"
 msgstr ""
+msgid "Import/Export Rate Limits"
+msgstr ""
+
 msgid "Import/Export illustration"
 msgstr ""
@@ -13989,6 +13995,24 @@ msgstr ""
 msgid "Maven Metadata"
 msgstr ""
+msgid "Max Group Export Download requests per minute per user"
+msgstr ""
+
+msgid "Max Group Export requests per minute per user"
+msgstr ""
+
+msgid "Max Group Import requests per minute per user"
+msgstr ""
+
+msgid "Max Project Export Download requests per minute per user"
+msgstr ""
+
+msgid "Max Project Export requests per minute per user"
+msgstr ""
+
+msgid "Max Project Import requests per minute per user"
+msgstr ""
+
 msgid "Max access level"
 msgstr ""

spec/controllers/groups_controller_spec.rb

@@ -941,7 +941,7 @@ def group_moved_message(redirect_route, group)
 
         allow(Gitlab::ApplicationRateLimiter)
           .to receive(:increment)
-          .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:group_export][:threshold] + 1)
+          .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:group_export][:threshold].call + 1)
       end
 
       it 'throttles the endpoint' do
@@ -1015,7 +1015,7 @@ def group_moved_message(redirect_route, group)
 
         allow(Gitlab::ApplicationRateLimiter)
           .to receive(:increment)
-          .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:group_download_export][:threshold] + 1)
+          .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:group_download_export][:threshold].call + 1)
       end
 
       it 'throttles the endpoint' do

spec/controllers/projects_controller_spec.rb

@@ -1197,7 +1197,7 @@ def update_project(**parameters)
       before do
         allow(Gitlab::ApplicationRateLimiter)
           .to receive(:increment)
-          .and_return(Gitlab::ApplicationRateLimiter.rate_limits["project_#{action}".to_sym][:threshold] + 1)
+          .and_return(Gitlab::ApplicationRateLimiter.rate_limits["project_#{action}".to_sym][:threshold].call + 1)
       end
 
       it 'prevents requesting project export' do
@@ -1264,7 +1264,7 @@ def update_project(**parameters)
           before do
             allow(Gitlab::ApplicationRateLimiter)
               .to receive(:increment)
-              .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:project_download_export][:threshold] + 1)
+              .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:project_download_export][:threshold].call + 1)
           end
 
           it 'prevents requesting project export' do

spec/requests/api/group_export_spec.rb

@@ -33,6 +33,10 @@
     context 'group_import_export feature flag enabled' do
       before do
         stub_feature_flags(group_import_export: true)
+
+        allow(Gitlab::ApplicationRateLimiter)
+          .to receive(:increment)
+          .and_return(0)
       end
 
       context 'when export file exists' do
@@ -87,7 +91,7 @@
       before do
         allow(Gitlab::ApplicationRateLimiter)
           .to receive(:increment)
-          .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:group_download_export][:threshold] + 1)
+          .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:group_download_export][:threshold].call + 1)
       end
 
       it 'throttles the endpoint' do
@@ -162,7 +166,7 @@
 
         allow(Gitlab::ApplicationRateLimiter)
           .to receive(:increment)
-          .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:group_export][:threshold] + 1)
+          .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:group_export][:threshold].call + 1)
       end
 
       it 'throttles the endpoint' do

spec/requests/api/project_export_spec.rb

@@ -237,7 +237,7 @@
           before do
             allow(Gitlab::ApplicationRateLimiter)
               .to receive(:increment)
-              .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:project_download_export][:threshold] + 1)
+              .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:project_download_export][:threshold].call + 1)
           end
 
           it 'prevents requesting project export' do
@@ -362,7 +362,7 @@
           before do
             allow(Gitlab::ApplicationRateLimiter)
               .to receive(:increment)
-              .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:project_export][:threshold] + 1)
+              .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:project_export][:threshold].call + 1)
           end
 
           it 'prevents requesting project export' do

spec/requests/import/gitlab_groups_controller_spec.rb

@@ -16,6 +16,8 @@
       expect(import_export).to receive(:storage_path).and_return(import_path)
     end
 
+    allow(Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(false)
+
     stub_uploads_object_storage(ImportExportUploader)
   end