Commit 536e7108 authored by Eugenia Grieff's avatar Eugenia Grieff

Add rate limit to issue creation

- Add check for rate limit in
Project::IssuesController
- Add issues_create_limit field to application
settings
- Add specs for issues controller and application
settings

Update PO files
parent 4e95c330
...@@ -219,6 +219,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController ...@@ -219,6 +219,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
:domain_blacklist_file, :domain_blacklist_file,
:raw_blob_request_limit, :raw_blob_request_limit,
:namespace_storage_size_limit, :namespace_storage_size_limit,
:issues_create_limit,
disabled_oauth_sign_in_sources: [], disabled_oauth_sign_in_sources: [],
import_sources: [], import_sources: [],
repository_storages: [], repository_storages: [],
......
...@@ -42,6 +42,9 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -42,6 +42,9 @@ class Projects::IssuesController < Projects::ApplicationController
before_action :authorize_import_issues!, only: [:import_csv] before_action :authorize_import_issues!, only: [:import_csv]
before_action :authorize_download_code!, only: [:related_branches] 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 before_action do
push_frontend_feature_flag(:vue_issuable_sidebar, project.group) push_frontend_feature_flag(:vue_issuable_sidebar, project.group)
push_frontend_feature_flag(:save_issuable_health_status, project.group, default_enabled: true) push_frontend_feature_flag(:save_issuable_health_status, project.group, default_enabled: true)
...@@ -296,6 +299,22 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -296,6 +299,22 @@ class Projects::IssuesController < Projects::ApplicationController
# 3. https://gitlab.com/gitlab-org/gitlab-foss/issues/42426 # 3. https://gitlab.com/gitlab-org/gitlab-foss/issues/42426
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42422') Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42422')
end 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 end
Projects::IssuesController.prepend_if_ee('EE::Projects::IssuesController') Projects::IssuesController.prepend_if_ee('EE::Projects::IssuesController')
...@@ -79,6 +79,7 @@ module ApplicationSettingImplementation ...@@ -79,6 +79,7 @@ module ApplicationSettingImplementation
housekeeping_gc_period: 200, housekeeping_gc_period: 200,
housekeeping_incremental_repack_period: 10, housekeeping_incremental_repack_period: 10,
import_sources: Settings.gitlab['import_sources'], import_sources: Settings.gitlab['import_sources'],
issues_create_limit: 300,
local_markdown_version: 0, local_markdown_version: 0,
max_artifacts_size: Settings.artifacts['max_size'], max_artifacts_size: Settings.artifacts['max_size'],
max_attachment_size: Settings.gitlab['max_attachment_size'], max_attachment_size: Settings.gitlab['max_attachment_size'],
......
= 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' }
...@@ -46,4 +46,15 @@ ...@@ -46,4 +46,15 @@
.settings-content .settings-content
= render 'protected_paths' = 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' = render_if_exists 'admin/application_settings/ee_network_settings'
---
title: Introduce rate limit for creating issues via web UI
merge_request: 28129
author:
type: performance
# 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
...@@ -397,6 +397,7 @@ CREATE TABLE public.application_settings ( ...@@ -397,6 +397,7 @@ CREATE TABLE public.application_settings (
email_restrictions text, email_restrictions text,
npm_package_requests_forwarding boolean DEFAULT true NOT NULL, npm_package_requests_forwarding boolean DEFAULT true NOT NULL,
namespace_storage_size_limit bigint DEFAULT 0 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, seat_link_enabled boolean DEFAULT true NOT NULL,
container_expiration_policies_enable_historic_entries boolean DEFAULT false NOT NULL container_expiration_policies_enable_historic_entries boolean DEFAULT false NOT NULL
); );
...@@ -13064,6 +13065,7 @@ COPY "schema_migrations" (version) FROM STDIN; ...@@ -13064,6 +13065,7 @@ COPY "schema_migrations" (version) FROM STDIN;
20200323134519 20200323134519
20200324093258 20200324093258
20200324115359 20200324115359
20200325111432
20200325152327 20200325152327
20200325160952 20200325160952
20200325183636 20200325183636
......
...@@ -19,8 +19,9 @@ module Gitlab ...@@ -19,8 +19,9 @@ module Gitlab
# and only do that when it's needed. # and only do that when it's needed.
def rate_limits def rate_limits
{ {
project_export: { threshold: 1, interval: 5.minutes }, issues_create: { threshold: -> { Gitlab::CurrentSettings.current_application_settings.issues_create_limit }, interval: 1.minute },
project_download_export: { threshold: 10, interval: 10.minutes }, project_export: { threshold: 1, interval: 5.minutes },
project_download_export: { threshold: 10, interval: 10.minutes },
project_repositories_archive: { threshold: 5, interval: 1.minute }, project_repositories_archive: { threshold: 5, interval: 1.minute },
project_generate_new_export: { threshold: 1, interval: 5.minutes }, project_generate_new_export: { threshold: 1, interval: 5.minutes },
project_import: { threshold: 30, interval: 5.minutes }, project_import: { threshold: 30, interval: 5.minutes },
......
...@@ -5331,6 +5331,9 @@ msgstr "" ...@@ -5331,6 +5331,9 @@ msgstr ""
msgid "Configure existing installation" msgid "Configure existing installation"
msgstr "" msgstr ""
msgid "Configure limit for issues created per minute by web and API requests."
msgstr ""
msgid "Configure limits for web and API requests." msgid "Configure limits for web and API requests."
msgstr "" msgstr ""
...@@ -11382,6 +11385,9 @@ msgstr "" ...@@ -11382,6 +11385,9 @@ msgstr ""
msgid "Issues Analytics" msgid "Issues Analytics"
msgstr "" msgstr ""
msgid "Issues Rate Limits"
msgstr ""
msgid "Issues can be bugs, tasks or ideas to be discussed. Also, issues are searchable and filterable." msgid "Issues can be bugs, tasks or ideas to be discussed. Also, issues are searchable and filterable."
msgstr "" msgstr ""
......
...@@ -1085,6 +1085,48 @@ describe Projects::IssuesController do ...@@ -1085,6 +1085,48 @@ describe Projects::IssuesController do
expect { subject }.to change(SentryIssue, :count) expect { subject }.to change(SentryIssue, :count)
end end
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 end
describe 'POST #mark_as_spam' do describe 'POST #mark_as_spam' do
......
...@@ -334,4 +334,20 @@ describe ApplicationSettings::UpdateService do ...@@ -334,4 +334,20 @@ describe ApplicationSettings::UpdateService do
expect(application_settings.protected_paths).to eq(['/users/password', '/users/sign_in']) expect(application_settings.protected_paths).to eq(['/users/password', '/users/sign_in'])
end end
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 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