Merge branch 'add-rate-limiting-to-incident-management-create' into 'master'

Add rack attack settings for prometheus alert endpoint

See merge request gitlab-org/gitlab!17859

app/models/application_setting_implementation.rb

@@ -109,6 +109,9 @@ module ApplicationSettingImplementation
         throttle_protected_paths_in_seconds: 10,
         throttle_protected_paths_per_period: 60,
         protected_paths: DEFAULT_PROTECTED_PATHS,
+        throttle_incident_management_notification_enabled: false,
+        throttle_incident_management_notification_period_in_seconds: 3600,
+        throttle_incident_management_notification_per_period: 3600,
         time_tracking_limit_to_hours: false,
         two_factor_grace_period: 48,
         unique_ips_limit_enabled: false,

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

@@ -45,3 +45,5 @@
       = _('Configure paths to be protected by Rack Attack. A web server restart is required after changing these settings.')
   .settings-content
     = render 'protected_paths'
+
+= render_if_exists 'admin/application_settings/ee_network_settings'

config/initializers/rack_attack_new.rb

@@ -125,4 +125,5 @@ class Rack::Attack
   end
 end
 
+::Rack::Attack.extend_if_ee('::EE::Gitlab::Rack::Attack') # rubocop: disable Cop/InjectEnterpriseEditionModule
 ::Rack::Attack::Request.prepend_if_ee('::EE::Gitlab::Rack::Attack::Request')

db/migrate/20190930025655_add_incident_management_throttle_columns_to_application_setting.rb

+# frozen_string_literal: true
+
+class AddIncidentManagementThrottleColumnsToApplicationSetting < ActiveRecord::Migration[5.2]
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def up
+    add_column(:application_settings,
+                            :throttle_incident_management_notification_enabled,
+                            :boolean,
+                            null: false,
+                            default: false)
+
+    add_column(:application_settings,
+                            :throttle_incident_management_notification_period_in_seconds,
+                            :integer,
+                            default: 3_600)
+
+    add_column(:application_settings,
+                            :throttle_incident_management_notification_per_period,
+                            :integer,
+                            default: 3_600)
+  end
+
+  def down
+    remove_column :application_settings, :throttle_incident_management_notification_enabled
+    remove_column :application_settings, :throttle_incident_management_notification_period_in_seconds
+    remove_column :application_settings, :throttle_incident_management_notification_per_period
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_09_29_180827) do
+ActiveRecord::Schema.define(version: 2019_09_30_025655) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -326,6 +326,9 @@ ActiveRecord::Schema.define(version: 2019_09_29_180827) do
     t.integer "throttle_protected_paths_requests_per_period", default: 10, null: false
     t.integer "throttle_protected_paths_period_in_seconds", default: 60, null: false
     t.string "protected_paths", limit: 255, default: ["/users/password", "/users/sign_in", "/api/v3/session.json", "/api/v3/session", "/api/v4/session.json", "/api/v4/session", "/users", "/users/confirmation", "/unsubscribes/", "/import/github/personal_access_token"], array: true
+    t.boolean "throttle_incident_management_notification_enabled", default: false, null: false
+    t.integer "throttle_incident_management_notification_period_in_seconds", default: 3600
+    t.integer "throttle_incident_management_notification_per_period", default: 3600
     t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id"
     t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id"
     t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id"

ee/app/helpers/ee/application_settings_helper.rb

@@ -43,7 +43,10 @@ module EE
         :slack_app_enabled,
         :slack_app_id,
         :slack_app_secret,
-        :slack_app_verification_token
+        :slack_app_verification_token,
+        :throttle_incident_management_notification_enabled,
+        :throttle_incident_management_notification_period_in_seconds,
+        :throttle_incident_management_notification_per_period
       ]
     end
 

ee/app/views/admin/application_settings/_ee_network_settings.haml

+- if License.feature_available?(:incident_management)
+  %section.settings.as-incident-management-alerts.no-animate#js-incident-management-settings{ class: ('expanded' if expanded_by_default?) }
+    .settings-header
+      %h4
+        = _('Incident Management Limits')
+      %button.btn.btn-default.js-settings-toggle{ type: 'button' }
+        = expanded_by_default? ? _('Collapse') : _('Expand')
+      %p
+        = _('Configure limits on the number of inbound alerts able to be sent to a project.')
+    .settings-content
+      = render 'network_incident_management'

ee/app/views/admin/application_settings/_network_incident_management.html.haml

+= form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-incident-management-settings'), html: { class: 'fieldset-form' } do |f|
+  = form_errors(@application_setting)
+
+  %fieldset
+    .form-group
+      .form-check
+        = f.check_box :throttle_incident_management_notification_enabled, class: 'form-check-input', data: { qa_selector: 'throttle_unauthenticated_checkbox' }
+        = f.label :throttle_incident_management_notification_enabled, class: 'form-check-label' do
+          = _('Enable Incident Management inbound alert limit')
+        %span.form-text.text-muted
+          = _('Helps reduce alert volume (e.g. if creating too many issues)')
+    .form-group
+      = f.label :throttle_incident_management_notification_period_in_seconds, 'Max requests per period per project', class: 'label-bold'
+      = f.number_field :throttle_incident_management_notification_period_in_seconds, class: 'form-control'
+    .form-group
+      = f.label :throttle_incident_management_notification_per_period, 'Rate limit period in seconds', class: 'label-bold'
+      = f.number_field :throttle_incident_management_notification_per_period, class: 'form-control'
+
+  = f.submit 'Save changes', class: "btn btn-success"

ee/changelogs/unreleased/add-rate-limiting-to-incident-management-create.yml

+---
+title: Add rack attack settings for prometheus and generic alert endpoint
+merge_request: 17859
+author:
+type: added

ee/lib/ee/gitlab/rack/attack.rb

+# frozen_string_literal: true
+
+module EE::Gitlab::Throttle
+  def self.settings
+    Gitlab::Throttle.settings
+  end
+
+  def self.incident_management_options
+    limit_proc = proc { |req| settings.throttle_incident_management_notification_per_period }
+    period_proc = proc { |req| settings.throttle_incident_management_notification_period_in_seconds.seconds }
+
+    { limit: limit_proc, period: period_proc }
+  end
+end
+
+module EE::Gitlab::Rack::Attack
+  Rack::Attack.throttle('throttle_incident_management_notification_web', EE::Gitlab::Throttle.incident_management_options) do |req|
+    EE::Gitlab::Throttle.settings.throttle_incident_management_notification_enabled &&
+      req.web_request? &&
+      req.path.include?('alerts/notify') &&
+      req.path
+  end
+end

ee/spec/requests/rack_attack_global_spec.rb

@@ -3,15 +3,7 @@
 require 'spec_helper'
 
 describe 'Rack Attack global throttles' do
-  around do |example|
-    # Instead of test environment's :null_store so the throttles can increment
-    Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
-
-    # Make time-dependent tests deterministic
-    Timecop.freeze { example.run }
-
-    Rack::Attack.cache.store = Rails.cache
-  end
+  include_context 'rack attack cache store'
 
   context 'when the request is from Geo secondary' do
     let(:project) { create(:project) }

ee/spec/requests/rack_attack_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Rack Attack EE throttles' do
+  include RackAttackSpecHelpers
+
+  let(:project) { create(:project) }
+
+  include_context 'rack attack cache store'
+
+  shared_examples_for 'incident management rate limiting' do
+    let(:settings) { Gitlab::CurrentSettings.current_application_settings }
+
+    let(:token) { double(token: '123456') }
+    let(:period) { period_in_seconds.seconds }
+
+    let(:settings_to_set) do
+      {
+        throttle_incident_management_notification_enabled: throttle_enabled,
+        throttle_incident_management_notification_per_period: requests_per_period,
+        throttle_incident_management_notification_period_in_seconds: period_in_seconds
+      }
+    end
+
+    let(:post_args) { post_args_with_token_headers(path, oauth_token_headers(token)) }
+
+    before do
+      stub_application_setting(settings_to_set)
+    end
+
+    context 'limits set' do
+      let(:requests_per_period) { 1 }
+      let(:period_in_seconds) { 10000 }
+
+      context 'when the throttle is enabled' do
+        let(:throttle_enabled) { true }
+
+        it 'rejects requests over the rate limit' do
+          # At first, allow requests under the rate limit.
+          requests_per_period.times do
+            post(*post_args)
+            expect(response).to have_http_status 200
+          end
+
+          # the last straw
+          expect_rejection { post(*post_args) }
+        end
+
+        it 'allows requests after throttling and then waiting for the next period' do
+          requests_per_period.times do
+            post(*post_args)
+            expect(response).to have_http_status 200
+          end
+
+          expect_rejection { post(*post_args) }
+
+          Timecop.travel(period.from_now) do
+            requests_per_period.times do
+              post(*post_args)
+              expect(response).to have_http_status 200
+            end
+
+            expect_rejection { post(*post_args) }
+          end
+        end
+      end
+
+      context 'when the throttle is disabled' do
+        let(:throttle_enabled) { false }
+
+        it 'allows requests over the rate limit' do
+          # At first, allow requests under the rate limit.
+          requests_per_period.times do
+            post(*post_args)
+            expect(response).to have_http_status 200
+          end
+
+          # requests still allowed
+          post(*post_args)
+          expect(response).to have_http_status 200
+        end
+      end
+    end
+  end
+
+  describe 'requests to prometheus alert notify endpoint with oauth token' do
+    before do
+      allow_any_instance_of(Projects::Prometheus::Alerts::NotifyService).to receive(:execute).and_return true
+    end
+
+    it_behaves_like 'incident management rate limiting' do
+      let(:path) { "/#{project.full_path}/prometheus/alerts/notify" }
+    end
+  end
+
+  describe 'requests to generic alert notify endpoint with oauth token' do
+    before do
+      allow_any_instance_of(Projects::Alerting::NotifyService).to receive(:execute).and_return double(success?: true)
+    end
+
+    it_behaves_like 'incident management rate limiting' do
+      let(:path) { "/#{project.full_path}/alerts/notify" }
+    end
+  end
+end

locale/gitlab.pot

@@ -4130,6 +4130,9 @@ msgstr ""
 msgid "Configure limits for web and API requests."
 msgstr ""
 
+msgid "Configure limits on the number of inbound alerts able to be sent to a project."
+msgstr ""
+
 msgid "Configure paths to be protected by Rack Attack. A web server restart is required after changing these settings."
 msgstr ""
 
@@ -5748,6 +5751,9 @@ msgstr ""
 msgid "Enable HTML emails"
 msgstr ""
 
+msgid "Enable Incident Management inbound alert limit"
+msgstr ""
+
 msgid "Enable Pseudonymizer data collection"
 msgstr ""
 
@@ -8252,6 +8258,9 @@ msgstr ""
 msgid "Helps prevent bots from creating accounts."
 msgstr ""
 
+msgid "Helps reduce alert volume (e.g. if creating too many issues)"
+msgstr ""
+
 msgid "Helps reduce request volume for protected paths"
 msgstr ""
 
@@ -8596,6 +8605,9 @@ msgstr ""
 msgid "In the next step, you'll be able to select the projects you want to import."
 msgstr ""
 
+msgid "Incident Management Limits"
+msgstr ""
+
 msgid "Incidents"
 msgstr ""
 

spec/requests/rack_attack_global_spec.rb

 require 'spec_helper'
 
 describe 'Rack Attack global throttles' do
+  include RackAttackSpecHelpers
+
   let(:settings) { Gitlab::CurrentSettings.current_application_settings }
 
   # Start with really high limits and override them with low limits to ensure
@@ -22,15 +24,7 @@ describe 'Rack Attack global throttles' do
   let(:period_in_seconds) { 10000 }
   let(:period) { period_in_seconds.seconds }
 
-  around do |example|
-    # Instead of test environment's :null_store so the throttles can increment
-    Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
-
-    # Make time-dependent tests deterministic
-    Timecop.freeze { example.run }
-
-    Rack::Attack.cache.store = Rails.cache
-  end
+  include_context 'rack attack cache store'
 
   describe 'unauthenticated requests' do
     let(:url_that_does_not_require_authentication) { '/users/sign_in' }
@@ -361,30 +355,4 @@ describe 'Rack Attack global throttles' do
       end
     end
   end
-
-  def api_get_args_with_token_headers(partial_url, token_headers)
-    ["/api/#{API::API.version}#{partial_url}", params: nil, headers: token_headers]
-  end
-
-  def rss_url(user)
-    "/dashboard/projects.atom?feed_token=#{user.feed_token}"
-  end
-
-  def private_token_headers(user)
-    { 'HTTP_PRIVATE_TOKEN' => user.private_token }
-  end
-
-  def personal_access_token_headers(personal_access_token)
-    { 'HTTP_PRIVATE_TOKEN' => personal_access_token.token }
-  end
-
-  def oauth_token_headers(oauth_access_token)
-    { 'AUTHORIZATION' => "Bearer #{oauth_access_token.token}" }
-  end
-
-  def expect_rejection(&block)
-    yield
-
-    expect(response).to have_http_status(429)
-  end
 end

spec/support/helpers/rack_attack_spec_helpers.rb

+# frozen_string_literal: true
+
+module RackAttackSpecHelpers
+  def post_args_with_token_headers(url, token_headers)
+    [url, params: nil, headers: token_headers]
+  end
+
+  def api_get_args_with_token_headers(partial_url, token_headers)
+    ["/api/#{API::API.version}#{partial_url}", params: nil, headers: token_headers]
+  end
+
+  def rss_url(user)
+    "/dashboard/projects.atom?feed_token=#{user.feed_token}"
+  end
+
+  def private_token_headers(user)
+    { 'HTTP_PRIVATE_TOKEN' => user.private_token }
+  end
+
+  def personal_access_token_headers(personal_access_token)
+    { 'HTTP_PRIVATE_TOKEN' => personal_access_token.token }
+  end
+
+  def oauth_token_headers(oauth_access_token)
+    { 'AUTHORIZATION' => "Bearer #{oauth_access_token.token}" }
+  end
+
+  def expect_rejection(&block)
+    yield
+
+    expect(response).to have_http_status(429)
+  end
+end

spec/support/shared_contexts/rack_attack_shared_context.rb

+# frozen_string_literal: true
+
+shared_context 'rack attack cache store' do
+  around do |example|
+    # Instead of test environment's :null_store so the throttles can increment
+    Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
+
+    # Make time-dependent tests deterministic
+    Timecop.freeze { example.run }
+
+    Rack::Attack.cache.store = Rails.cache
+  end
+end