Merge branch '55241-rate-limit-issue-creation' into 'master'

Add rate limit  to issue creation

Closes #55241

See merge request gitlab-org/gitlab!28129

app/controllers/admin/application_settings_controller.rb

@@ -219,6 +219,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       :domain_blacklist_file,
       :raw_blob_request_limit,
       :namespace_storage_size_limit,
+      :issues_create_limit,
       disabled_oauth_sign_in_sources: [],
       import_sources: [],
       repository_storages: [],

app/controllers/projects/issues_controller.rb

@@ -42,6 +42,9 @@ class Projects::IssuesController < Projects::ApplicationController
   before_action :authorize_import_issues!, only: [:import_csv]
   before_action :authorize_download_code!, only: [:related_branches]
 
+  # Limit the amount of issues created per minute
+  before_action :create_rate_limit, only: [:create]
+
   before_action do
     push_frontend_feature_flag(:vue_issuable_sidebar, project.group)
     push_frontend_feature_flag(:save_issuable_health_status, project.group, default_enabled: true)
@@ -296,6 +299,22 @@ class Projects::IssuesController < Projects::ApplicationController
     # 3. https://gitlab.com/gitlab-org/gitlab-foss/issues/42426
     Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42422')
   end
+
+  private
+
+  def create_rate_limit
+    key = :issues_create
+
+    if rate_limiter.throttled?(key, scope: [@project, @current_user])
+      rate_limiter.log_request(request, "#{key}_request_limit".to_sym, current_user)
+
+      render plain: _('This endpoint has been requested too many times. Try again later.'), status: :too_many_requests
+    end
+  end
+
+  def rate_limiter
+    ::Gitlab::ApplicationRateLimiter
+  end
 end
 
 Projects::IssuesController.prepend_if_ee('EE::Projects::IssuesController')

app/models/application_setting_implementation.rb

@@ -79,6 +79,7 @@ module ApplicationSettingImplementation
         housekeeping_gc_period: 200,
         housekeeping_incremental_repack_period: 10,
         import_sources: Settings.gitlab['import_sources'],
+        issues_create_limit: 300,
         local_markdown_version: 0,
         max_artifacts_size: Settings.artifacts['max_size'],
         max_attachment_size: Settings.gitlab['max_attachment_size'],

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

+= form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-issue-limits-settings'), html: { class: 'fieldset-form' } do |f|
+  = form_errors(@application_setting)
+
+  %fieldset
+    .form-group
+      = f.label :issues_create_limit, 'Max requests per second per user', class: 'label-bold'
+      = f.number_field :issues_create_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

@@ -46,4 +46,15 @@
   .settings-content
     = render 'protected_paths'
 
+%section.settings.as-issue-limits.no-animate#js-issue-limits-settings{ class: ('expanded' if expanded_by_default?) }
+  .settings-header
+    %h4
+      = _('Issues Rate Limits')
+    %button.btn.btn-default.js-settings-toggle{ type: 'button' }
+      = expanded_by_default? ? _('Collapse') : _('Expand')
+    %p
+      = _('Configure limit for issues created per minute by web and API requests.')
+  .settings-content
+    = render 'issue_limits'
+
 = render_if_exists 'admin/application_settings/ee_network_settings'

changelogs/unreleased/55241-rate-limit-issue-creation.yml

+---
+title: Introduce rate limit for creating issues via web UI
+merge_request: 28129
+author:
+type: performance

db/migrate/20200325111432_add_issues_create_limit_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddIssuesCreateLimitToApplicationSettings < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    add_column :application_settings, :issues_create_limit, :integer, default: 300, null: false
+  end
+end

db/structure.sql

@@ -397,6 +397,7 @@ CREATE TABLE public.application_settings (
     email_restrictions text,
     npm_package_requests_forwarding boolean DEFAULT true NOT NULL,
     namespace_storage_size_limit bigint DEFAULT 0 NOT NULL,
+    issues_create_limit integer DEFAULT 300 NOT NULL,
     seat_link_enabled boolean DEFAULT true NOT NULL,
     container_expiration_policies_enable_historic_entries boolean DEFAULT false NOT NULL
 );
@@ -13098,6 +13099,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200323134519
 20200324093258
 20200324115359
+20200325111432
 20200325152327
 20200325160952
 20200325183636

lib/gitlab/application_rate_limiter.rb

@@ -19,8 +19,9 @@ module Gitlab
       # and only do that when it's needed.
       def rate_limits
         {
-          project_export:              { threshold: 1, interval: 5.minutes },
-          project_download_export:     { threshold: 10, interval: 10.minutes },
+          issues_create:                { threshold: -> { Gitlab::CurrentSettings.current_application_settings.issues_create_limit }, interval: 1.minute },
+          project_export:               { threshold: 1, 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_import:              { threshold: 30, interval: 5.minutes },

locale/gitlab.pot

@@ -5334,6 +5334,9 @@ msgstr ""
 msgid "Configure existing installation"
 msgstr ""
 
+msgid "Configure limit for issues created per minute by web and API requests."
+msgstr ""
+
 msgid "Configure limits for web and API requests."
 msgstr ""
 
@@ -11385,6 +11388,9 @@ msgstr ""
 msgid "Issues Analytics"
 msgstr ""
 
+msgid "Issues Rate Limits"
+msgstr ""
+
 msgid "Issues can be bugs, tasks or ideas to be discussed. Also, issues are searchable and filterable."
 msgstr ""
 

spec/controllers/projects/issues_controller_spec.rb

@@ -1085,6 +1085,48 @@ describe Projects::IssuesController do
         expect { subject }.to change(SentryIssue, :count)
       end
     end
+
+    context 'when the endpoint receives requests above the limit' do
+      before do
+        stub_application_setting(issues_create_limit: 5)
+      end
+
+      it 'prevents from creating more issues', :request_store do
+        5.times { post_new_issue }
+
+        expect { post_new_issue }
+          .to change { Gitlab::GitalyClient.get_request_count }.by(1) # creates 1 projects and 0 issues
+
+        post_new_issue
+        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
+
+      it 'logs the event on auth.log' do
+        attributes = {
+          message: 'Application_Rate_Limiter_Request',
+          env: :issues_create_request_limit,
+          remote_ip: '0.0.0.0',
+          request_method: 'POST',
+          path: "/#{project.full_path}/-/issues",
+          user_id: user.id,
+          username: user.username
+        }
+
+        expect(Gitlab::AuthLogger).to receive(:error).with(attributes).once
+
+        project.add_developer(user)
+        sign_in(user)
+
+        6.times do
+          post :create, params: {
+            namespace_id: project.namespace.to_param,
+            project_id: project,
+            issue: { title: 'Title', description: 'Description' }
+          }
+        end
+      end
+    end
   end
 
   describe 'POST #mark_as_spam' do

spec/services/application_settings/update_service_spec.rb

@@ -334,4 +334,20 @@ describe ApplicationSettings::UpdateService do
       expect(application_settings.protected_paths).to eq(['/users/password', '/users/sign_in'])
     end
   end
+
+  context 'when issues_create_limit is passsed' do
+    let(:params) do
+      {
+        issues_create_limit: 600
+      }
+    end
+
+    it 'updates issues_create_limit value' do
+      subject.execute
+
+      application_settings.reload
+
+      expect(application_settings.issues_create_limit).to eq(600)
+    end
+  end
 end