Commit 1033618c authored by GitLab Bot's avatar GitLab Bot

Automatic merge of gitlab-org/gitlab master

parents 995d0988 91ce0ccf
......@@ -243,6 +243,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
:domain_denylist_file,
:raw_blob_request_limit,
:issues_create_limit,
:notes_create_limit,
:default_branch_name,
disabled_oauth_sign_in_sources: [],
import_sources: [],
......
......@@ -10,6 +10,7 @@ class Projects::NotesController < Projects::ApplicationController
before_action :authorize_read_note!
before_action :authorize_create_note!, only: [:create]
before_action :authorize_resolve_note!, only: [:resolve, :unresolve]
before_action :create_rate_limit, only: [:create]
feature_category :issue_tracking
......@@ -90,4 +91,17 @@ class Projects::NotesController < Projects::ApplicationController
def whitelist_query_limiting
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42383')
end
def create_rate_limit
key = :notes_create
return unless rate_limiter.throttled?(key, scope: [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
def rate_limiter
::Gitlab::ApplicationRateLimiter
end
end
......@@ -25,6 +25,7 @@ module Mutations
def resolve(args)
noteable = authorized_find!(id: args[:noteable_id])
verify_rate_limit!(current_user)
note = ::Notes::CreateService.new(
noteable.project,
......@@ -54,6 +55,14 @@ module Mutations
confidential: args[:confidential]
}
end
def verify_rate_limit!(current_user)
rate_limiter, key = ::Gitlab::ApplicationRateLimiter, :notes_create
return unless rate_limiter.throttled?(key, scope: [current_user])
raise Gitlab::Graphql::Errors::ResourceNotAvailable,
'This endpoint has been requested too many times. Try again later.'
end
end
end
end
......
......@@ -328,6 +328,7 @@ module ApplicationSettingsHelper
:email_restrictions_enabled,
:email_restrictions,
:issues_create_limit,
:notes_create_limit,
:raw_blob_request_limit,
:project_import_limit,
:project_export_limit,
......
......@@ -444,6 +444,9 @@ class ApplicationSetting < ApplicationRecord
presence: true,
numericality: { only_integer: true, greater_than: 0 }
validates :notes_create_limit,
numericality: { only_integer: true, greater_than_or_equal_to: 0 }
attr_encrypted :asset_proxy_secret_key,
mode: :per_attribute_iv,
key: Settings.attr_encrypted_db_key_base_truncated,
......
......@@ -93,6 +93,7 @@ module ApplicationSettingImplementation
import_sources: Settings.gitlab['import_sources'],
invisible_captcha_enabled: false,
issues_create_limit: 300,
notes_create_limit: 300,
local_markdown_version: 0,
login_recaptcha_protection_enabled: false,
max_artifacts_size: Settings.artifacts['max_size'],
......
......@@ -125,3 +125,5 @@ module AlertManagement
end
end
end
AlertManagement::AlertProcessing.prepend_ee_mod
= form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-note-limits-settings'), html: { class: 'fieldset-form' } do |f|
= form_errors(@application_setting)
%fieldset
.form-group
= f.label :notes_create_limit, _('Max requests per minute per user'), class: 'label-bold'
= f.number_field :notes_create_limit, class: 'form-control gl-form-input'
= f.submit _('Save changes'), class: "gl-button btn btn-success", data: { qa_selector: 'save_changes_button' }
......@@ -61,6 +61,17 @@
.settings-content
= render 'issue_limits'
%section.settings.as-note-limits.no-animate#js-note-limits-settings{ class: ('expanded' if expanded_by_default?) }
.settings-header
%h4
= _('Notes Rate Limits')
%button.btn.gl-button.btn-default.js-settings-toggle{ type: 'button' }
= expanded_by_default? ? _('Collapse') : _('Expand')
%p
= _('Configure limit for notes created per minute by web and API requests.')
.settings-content
= render 'note_limits'
%section.settings.as-import-export-limits.no-animate#js-import-export-limits-settings{ class: ('expanded' if expanded_by_default?) }
.settings-header
%h4
......
......@@ -2,7 +2,7 @@
= form_tag oauth_application_path(application) do
%input{ :name => "_method", :type => "hidden", :value => "delete" }/
- if defined? small
= button_tag type: "submit", class: "gl-button btn btn-transparent", data: { confirm: _("Are you sure?") } do
= button_tag type: "submit", class: "gl-button btn btn-default", data: { confirm: _("Are you sure?") } do
%span.sr-only
= _('Destroy')
= sprite_icon('remove')
......
......@@ -40,8 +40,8 @@
- application.redirect_uri.split.each do |uri|
%div= uri
%td= application.access_tokens.count
%td
= link_to edit_oauth_application_path(application), class: "gl-button btn btn-transparent gl-mr-2" do
%td.gl-display-flex
= link_to edit_oauth_application_path(application), class: "gl-button btn btn-default gl-mr-2" do
%span.sr-only
= _('Edit')
= sprite_icon('pencil')
......
---
title: Add application rate limit for Notes creation
merge_request: 53637
author:
type: added
---
title: Fix action button alignment for application inside the table in oauth/applications
merge_request: 52465
author: Yogi (@yo)
type: fixed
# frozen_string_literal: true
class AddNotesCreateLimitToApplicationSettings < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column :application_settings, :notes_create_limit, :integer, default: 300, null: false
end
end
818fcf0f0fec9d2833b091ef380005a2d485486522fb63e2a7b2fd01dbf1ff79
\ No newline at end of file
......@@ -9413,6 +9413,7 @@ CREATE TABLE application_settings (
git_two_factor_session_expiry integer DEFAULT 15 NOT NULL,
asset_proxy_allowlist text,
keep_latest_artifact boolean DEFAULT true NOT NULL,
notes_create_limit integer DEFAULT 300 NOT NULL,
CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)),
CONSTRAINT check_17d9558205 CHECK ((char_length((kroki_url)::text) <= 1024)),
......
......@@ -36,6 +36,11 @@ are paginated.
Read more on [pagination](README.md#pagination).
## Rate limits
To help avoid abuse, you can limit your users to a specific number of `Create` request per minute.
See [Notes rate limits](../user/admin_area/settings/rate_limit_on_notes_creation.md).
## Issues
### List project issue notes
......
......@@ -25,11 +25,14 @@ similarly mitigated by a rate limit.
## Admin Area settings
- [Issues rate limits](../user/admin_area/settings/rate_limit_on_issues_creation.md).
- [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).
These are rate limits you can set in the Admin Area of your instance:
- [Import/Export rate limits](../user/admin_area/settings/import_export_rate_limits.md)
- [Issues rate limits](../user/admin_area/settings/rate_limit_on_issues_creation.md)
- [Notes rate limits](../user/admin_area/settings/rate_limit_on_notes_creation.md)
- [Protected paths](../user/admin_area/settings/protected_paths.md)
- [Raw endpoints rate limits](../user/admin_area/settings/rate_limits_on_raw_endpoints.md)
- [User and IP rate limits](../user/admin_area/settings/user_and_ip_rate_limits.md)
## Non-configurable limits
......
......@@ -94,6 +94,7 @@ Access the default page for admin area settings by navigating to **Admin Area >
| [Outbound requests](../../../security/webhooks.md) | Allow requests to the local network from hooks and services. |
| [Protected Paths](protected_paths.md) | Configure paths to be protected by Rack Attack. |
| [Incident Management](../../../operations/incident_management/index.md) Limits | Configure limits on the number of inbound alerts able to be sent to a project. |
| [Notes creation limit](rate_limit_on_notes_creation.md)| Set a rate limit on the note creation requests. |
## Geo
......
---
type: reference
stage: Plan
group: Project Management
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/#assignments
---
# Rate limits on note creation
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53637) in GitLab 13.9.
This setting allows you to rate limit the requests to the note creation endpoint.
To change the note creation rate limit:
1. Go to **Admin Area > Settings > Network**.
1. Expand the **Notes Rate Limits** section.
1. Enter the new value.
1. Select **Save changes**.
This limit is:
- Applied independently per user.
- Not applied per IP address.
The default value is `300`.
Requests over the rate limit are logged into the `auth.log` file.
For example, if you set a limit of 300, requests using the
[Projects::NotesController#create](https://gitlab.com/gitlab-org/gitlab/blob/master/app/controllers/projects/notes_controller.rb)
action exceeding a rate of 300 per minute are blocked. Access to the endpoint is allowed after one minute.
# frozen_string_literal: true
module IncidentManagement
# Returns users who are oncall by reading shifts from the DB.
# The DB is the historical record, so we should adhere to it
# when it is available. If rotations are missing persited
# shifts for some reason, we'll fallback to a generated shift.
# It may also be possible that no one is on call for a rotation.
class OncallUsersFinder
include Gitlab::Utils::StrongMemoize
# @param project [Project]
# @option oncall_at [ActiveSupport::TimeWithZone]
# Limits users to only those
# on-call at the specified time.
def initialize(project, oncall_at: Time.current)
@project = project
@oncall_at = oncall_at
end
# @return [User::ActiveRecord_Relation]
def execute
return User.none unless Gitlab::IncidentManagement.oncall_schedules_available?(project)
return User.none unless user_ids.present?
User.id_in(user_ids)
end
private
attr_reader :project, :oncall_at
def user_ids
strong_memoize(:user_ids) do
user_ids_for_persisted_shifts.concat(user_ids_for_predicted_shifts).uniq
end
end
def user_ids_for_persisted_shifts
ids_for_persisted_shifts.flat_map(&:last)
end
def rotation_ids_for_persisted_shifts
ids_for_persisted_shifts.flat_map(&:first)
end
# @return [Array<[rotation_id, user_id]>]
# @example - [ [1, 16], [2, 200] ]
def ids_for_persisted_shifts
strong_memoize(:ids_for_persisted_shifts) do
project.incident_management_oncall_rotations
.merge(IncidentManagement::OncallShift.for_timestamp(oncall_at))
.pluck_id_and_user_id
end
end
def user_ids_for_predicted_shifts
rotations_without_persisted_shifts.map do |rotation|
next unless shift = IncidentManagement::OncallShiftGenerator.new(rotation).for_timestamp(oncall_at)
shift.participant.user_id
end
end
def rotations_without_persisted_shifts
project.incident_management_oncall_rotations
.except_ids(rotation_ids_for_persisted_shifts)
.with_shift_generation_associations
end
end
end
......@@ -103,6 +103,7 @@ module EE
has_many :sourced_pipelines, class_name: 'Ci::Sources::Project', foreign_key: :source_project_id
has_many :incident_management_oncall_schedules, class_name: 'IncidentManagement::OncallSchedule', inverse_of: :project
has_many :incident_management_oncall_rotations, class_name: 'IncidentManagement::OncallRotation', through: :incident_management_oncall_schedules, source: :rotations
elastic_index_dependant_association :issues, on_change: :visibility_level
......
......@@ -11,8 +11,6 @@ module IncidentManagement
belongs_to :user, class_name: 'User', foreign_key: :user_id
has_many :shifts, class_name: 'OncallShift', inverse_of: :participant, foreign_key: :participant_id
scope :ordered_asc, -> { order(id: :asc) }
# Uniqueness validations added here should be duplicated
# in IncidentManagement::OncallRotation::CreateService
# as bulk insertion skips validations
......
......@@ -13,7 +13,8 @@ module IncidentManagement
NAME_LENGTH = 200
belongs_to :schedule, class_name: 'OncallSchedule', inverse_of: 'rotations', foreign_key: 'oncall_schedule_id'
has_many :participants, class_name: 'OncallParticipant', inverse_of: :rotation
# Note! If changing the order of participants, also change the :with_shift_generation_associations scope.
has_many :participants, -> { order(id: :asc) }, class_name: 'OncallParticipant', inverse_of: :rotation
has_many :users, through: :participants
has_many :shifts, class_name: 'OncallShift', inverse_of: :rotation, foreign_key: :rotation_id
......@@ -23,9 +24,20 @@ module IncidentManagement
validates :length_unit, presence: true
scope :started, -> { where('starts_at < ?', Time.current) }
scope :except_ids, -> (ids) { where.not(id: ids) }
scope :with_shift_generation_associations, -> do
joins(:participants, :schedule)
.distinct
.includes(:participants, :schedule)
.order(:id, 'incident_management_oncall_participants.id ASC')
end
delegate :project, to: :schedule
def self.pluck_id_and_user_id
joins(shifts: { participant: :user }).pluck(:id, 'users.id')
end
def shift_duration
# As length_unit is an enum, input is guaranteed to be appropriate
length.public_send(length_unit) # rubocop:disable GitlabSecurity/PublicSend
......
......@@ -11,7 +11,7 @@ module IncidentManagement
DESCRIPTION_LENGTH = 1000
belongs_to :project, inverse_of: :incident_management_oncall_schedules
has_many :rotations, class_name: 'OncallRotation'
has_many :rotations, class_name: 'OncallRotation', inverse_of: :schedule
has_many :participants, class_name: 'OncallParticipant', through: :rotations
has_internal_id :iid, scope: :project
......
......@@ -21,6 +21,10 @@ module IncidentManagement
where("tstzrange(starts_at, ends_at, '[)') && tstzrange(?, ?, '[)')", starts_at, ends_at)
end
scope :for_timestamp, -> (timestamp) do
where('incident_management_oncall_shifts.starts_at <= :timestamp AND '\
'incident_management_oncall_shifts.ends_at > :timestamp', timestamp: timestamp)
end
private
......
# frozen_string_literal: true
module EE
module AlertManagement
module AlertProcessing
extend ::Gitlab::Utils::Override
private
override :complete_post_processing_tasks
def complete_post_processing_tasks
super
notify_oncall if oncall_notification_recipients.present?
end
def notify_oncall
notification_service
.async
.notify_oncall_users_of_alert(oncall_notification_recipients.to_a, alert)
end
def oncall_notification_recipients
strong_memoize(:oncall_notification_recipients) do
::IncidentManagement::OncallUsersFinder.new(project).execute
end
end
end
end
end
......@@ -66,6 +66,12 @@ module EE
mailer.provisioned_member_access_granted_email(group_member.id).deliver_later
end
def notify_oncall_users_of_alert(users, alert)
users.each do |user|
mailer.prometheus_alert_fired_email(alert.project, user, alert).deliver_later
end
end
private
def add_mr_approvers_email(merge_request, approvers, current_user)
......
......@@ -124,7 +124,7 @@ module IncidentManagement
end
def participants
@participants ||= rotation.participants.ordered_asc
@participants ||= rotation.participants
end
def rotation_starts_at
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe IncidentManagement::OncallUsersFinder do
let_it_be_with_refind(:project) { create(:project) }
# Project 1 Shift 1 Shift 2 Shift 3
# s1 -> r1 [p1] | user 1; saved | user 1; unsaved | user 1; unsaved |
# -> r2 [p1, p2] | user 2; saved | user 3; saved | user 2; unsaved |
# s2 -> r1 [p1] | user 4; saved | user 4; unsaved | user 4; unsaved |
# -> r2 [p1] | user 1; saved | user 1; unsaved | user 1; unsaved |
# -> r3 | N/A | N/A | N/A |
#
# Project 2
# s1 -> r1 [p1, p2] | user 5; saved | user 2; saved | user 5; unsaved |
# Schedule 1 / Rotation 1
let_it_be(:s1) { create(:incident_management_oncall_schedule, project: project) }
let_it_be(:s1_r1) { create(:incident_management_oncall_rotation, :with_participant, schedule: s1) }
let_it_be(:s1_r1_p1) { s1_r1.participants.first }
let_it_be(:user_1) { s1_r1_p1.user }
let_it_be(:s1_r1_shift1) { create(:incident_management_oncall_shift, participant: s1_r1_p1) }
# Schedule 1 / Rotation 2
let_it_be(:s1_r2) { create(:incident_management_oncall_rotation, :with_participant, schedule: s1) }
let_it_be(:s1_r2_p1) { s1_r2.participants.first }
let_it_be(:user_2) { s1_r2_p1.user }
let_it_be(:s1_r2_shift1) { create(:incident_management_oncall_shift, participant: s1_r2_p1) }
let_it_be(:s1_r2_p2) { create(:incident_management_oncall_participant, rotation: s1_r2) }
let_it_be(:user_3) { s1_r2_p2.user }
let_it_be(:s1_r2_shift2) { create(:incident_management_oncall_shift, participant: s1_r2_p2, starts_at: s1_r2_shift1.ends_at) }
# Schedule 2 / Rotation 1
let_it_be(:s2) { create(:incident_management_oncall_schedule, project: project) }
let_it_be(:s2_r1) { create(:incident_management_oncall_rotation, :with_participant, schedule: s2) }
let_it_be(:s2_r1_p1) { s2_r1.participants.first }
let_it_be(:user_4) { s2_r1_p1.user }
let_it_be(:s2_r1_shift1) { create(:incident_management_oncall_shift, participant: s2_r1_p1) }
# Schedule 2 / Rotation 2 - has same user as s1_r1_p1
let_it_be(:s2_r2) { create(:incident_management_oncall_rotation, schedule: s2) }
let_it_be(:s2_r2_p1) { create(:incident_management_oncall_participant, user: user_1, rotation: s2_r2) }
let_it_be(:s2_r2_shift1) { create(:incident_management_oncall_shift, participant: s2_r2_p1) }
# Schedule 2 / Rotation 3 - has no participants
let_it_be(:s2_r3) { create(:incident_management_oncall_rotation, schedule: s2) }
# Other Project - has same user as s1_r2_p1
let_it_be(:proj2_s1_r1_p1) { create(:incident_management_oncall_participant) } # user_5
let_it_be(:proj2_s1_r1_shift1) { create(:incident_management_oncall_shift, participant: proj2_s1_r1_p1) }
let_it_be(:proj2_s1_r1) { proj2_s1_r1_p1.rotation }
let_it_be(:proj2_s1_r1_p2) { create(:incident_management_oncall_participant, :with_developer_access, user: user_2, rotation: proj2_s1_r1) }
let_it_be(:proj2_s1_r1_shift2) { create(:incident_management_oncall_shift, participant: proj2_s1_r1_p2, starts_at: proj2_s1_r1_shift1.ends_at) }
let(:oncall_at) { Time.current }
describe '#execute' do
subject(:execute) { described_class.new(project, oncall_at: oncall_at).execute }
context 'when feature is available' do
before do
stub_licensed_features(oncall_schedules: true)
end
context 'without parameters uses current time' do
subject(:execute) { described_class.new(project).execute }
it { is_expected.to contain_exactly(user_1, user_2, user_4) }
end
context 'with :oncall_at parameter specified' do
let(:during_first_shift) { Time.current }
let(:during_second_shift) { s1_r2_shift2.starts_at + 5.minutes }
let(:after_second_shift) { s1_r2_shift2.ends_at + 5.minutes }
let(:before_shifts) { s1_r1.starts_at - 15.minutes }
context 'with all persisted shifts for oncall_at time' do
let(:oncall_at) { during_first_shift }
it { is_expected.to contain_exactly(user_1, user_2, user_4) }
it 'does not attempt to generate shifts' do
expect(IncidentManagement::OncallShiftGenerator).not_to receive(:new)
execute
end
end
context 'with some persisted shifts for oncall_at time' do
let(:oncall_at) { during_second_shift }
it { is_expected.to contain_exactly(user_1, user_3, user_4) }
it 'does not run additional queries for each persisted shift' do
query_count = ActiveRecord::QueryRecorder.new { execute }
create(:incident_management_oncall_shift, participant: s1_r1_p1, starts_at: s1_r1_shift1.ends_at)
expect { described_class.new(project, oncall_at: oncall_at).execute }.not_to exceed_query_limit(query_count)
end
end
context 'with no persisted shifts for oncall_at time' do
let(:oncall_at) { after_second_shift }
it { is_expected.to contain_exactly(user_1, user_2, user_4) }
end
context 'before rotations have started' do
let(:oncall_at) { before_shifts }
it { is_expected.to be_empty }
end
it 'does not require additional queries to generate shifts' do
query_count = ActiveRecord::QueryRecorder.new { described_class.new(project, oncall_at: during_first_shift).execute }
expect { described_class.new(project, oncall_at: after_second_shift).execute }
.not_to exceed_query_limit(query_count)
end
end
end
context 'when feature is not avaiable' do
before do
stub_licensed_features(oncall_schedules: false)
end
it { is_expected.to eq(IncidentManagement::OncallParticipant.none) }
end
end
end
......@@ -4,7 +4,7 @@ require 'spec_helper'
RSpec.describe IncidentManagement::OncallShiftGenerator do
let_it_be(:rotation_start_time) { Time.parse('2020-12-08 00:00:00 UTC').utc }
let_it_be(:rotation) { create(:incident_management_oncall_rotation, starts_at: rotation_start_time, length: 5, length_unit: :days) }
let_it_be_with_reload(:rotation) { create(:incident_management_oncall_rotation, starts_at: rotation_start_time, length: 5, length_unit: :days) }
let(:current_time) { Time.parse('2020-12-08 15:00:00 UTC').utc }
let(:shift_length) { rotation.shift_duration }
......
......@@ -8,9 +8,8 @@ RSpec.describe UpdateUndefinedConfidenceFromOccurrences, :migration do
let(:identifiers) { table(:vulnerability_identifiers) }
let(:scanners) { table(:vulnerability_scanners) }
let(:projects) { table(:projects) }
let(:vul1) { attributes_for(:vulnerabilities_finding, id: 1, report_type: 2, confidence: 5) } # rubocop: disable RSpec/FactoriesInMigrationSpecs
let(:vul2) { attributes_for(:vulnerabilities_finding, id: 2, report_type: 2, confidence: 5) } # rubocop: disable RSpec/FactoriesInMigrationSpecs
let(:vul1) { attributes_for_vulnerability_finding(id: 1) }
let(:vul2) { attributes_for_vulnerability_finding(id: 2) }
before do
stub_const("#{described_class}::BATCH_SIZE", 2)
......@@ -106,4 +105,40 @@ RSpec.describe UpdateUndefinedConfidenceFromOccurrences, :migration do
expect(vulnerabilities.exists?(confidence: 0)).to be_truthy
end
private
def attributes_for_vulnerability_finding(id:, report_type: 2, confidence: 5)
uuid = SecureRandom.uuid
{
id: id,
confidence: confidence,
report_type: report_type,
project_fingerprint: SecureRandom.hex(20),
location_fingerprint: Digest::SHA1.hexdigest(SecureRandom.hex(10)),
uuid: uuid,
name: "Vulnerability Finding #{uuid}",
raw_metadata: raw_metadata
}
end
def raw_metadata
{ "description" => "The cipher does not provide data integrity update 1",
"message" => "The cipher does not provide data integrity",
"cve" => "818bf5dacb291e15d9e6dc3c5ac32178:CIPHER",
"solution" => "GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.",
"location" => { "file" => "maven/src/main/java/com/gitlab/security_products/tests/App.java", "start_line" => 29, "end_line" => 29, "class" => "com.gitlab.security_products.tests.App", "method" => "insecureCypher" },
"links" => [{ "name" => "Cipher does not check for integrity first?", "url" => "https://crypto.stackexchange.com/questions/31428/pbewithmd5anddes-cipher-does-not-check-for-integrity-first" }],
"assets" => [{ "type" => "postman", "name" => "Test Postman Collection", "url" => "http://localhost/test.collection" }],
"evidence" =>
{ "summary" => "Credit card detected",
"request" => { "headers" => [{ "name" => "Accept", "value" => "*/*" }], "method" => "GET", "url" => "http://goat:8080/WebGoat/logout", "body" => nil },
"response" => { "headers" => [{ "name" => "Content-Length", "value" => "0" }], "reason_phrase" => "OK", "status_code" => 200, "body" => nil },
"source" => { "id" => "assert:Response Body Analysis", "name" => "Response Body Analysis", "url" => "htpp://hostname/documentation" },
"supporting_messages" =>
[{ "name" => "Origional", "request" => { "headers" => [{ "name" => "Accept", "value" => "*/*" }], "method" => "GET", "url" => "http://goat:8080/WebGoat/logout", "body" => "" } },
{ "name" => "Recorded",
"request" => { "headers" => [{ "name" => "Accept", "value" => "*/*" }], "method" => "GET", "url" => "http://goat:8080/WebGoat/logout", "body" => "" },
"response" => { "headers" => [{ "name" => "Content-Length", "value" => "0" }], "reason_phrase" => "OK", "status_code" => 200, "body" => "" } }] } }
end
end
......@@ -38,18 +38,6 @@ RSpec.describe IncidentManagement::OncallParticipant do
end
end
describe 'scopes' do
describe '.ordered_asc' do
let_it_be(:participant1) { create(:incident_management_oncall_participant, :with_developer_access, rotation: rotation) }
let_it_be(:participant2) { create(:incident_management_oncall_participant, :with_developer_access, rotation: rotation) }
let_it_be(:participant3) { create(:incident_management_oncall_participant, :with_developer_access, rotation: rotation) }
subject { described_class.ordered_asc }
it { is_expected.to eq([participant1, participant2, participant3]) }
end
end
private
def remove_user_from_project(user, project)
......
......@@ -6,10 +6,10 @@ RSpec.describe IncidentManagement::OncallRotation do
let_it_be(:schedule) { create(:incident_management_oncall_schedule) }
describe '.associations' do
it { is_expected.to belong_to(:schedule) }
it { is_expected.to have_many(:participants) }
it { is_expected.to belong_to(:schedule).class_name('OncallSchedule').inverse_of(:rotations) }
it { is_expected.to have_many(:participants).order(id: :asc).class_name('OncallParticipant').inverse_of(:rotation) }
it { is_expected.to have_many(:users).through(:participants) }
it { is_expected.to have_many(:shifts) }
it { is_expected.to have_many(:shifts).class_name('OncallShift').inverse_of(:rotation) }
end
describe '.validations' do
......
......@@ -7,7 +7,7 @@ RSpec.describe IncidentManagement::OncallSchedule do
describe '.associations' do
it { is_expected.to belong_to(:project) }
it { is_expected.to have_many(:rotations) }
it { is_expected.to have_many(:rotations).inverse_of(:schedule) }
it { is_expected.to have_many(:participants).through(:rotations) }
end
......
......@@ -45,36 +45,36 @@ RSpec.describe IncidentManagement::OncallShift do
end
describe 'scopes' do
describe '.for_timeframe' do
let_it_be(:monday) { Time.current.next_week(:monday) }
let_it_be(:tuesday) { monday + 1.day }
let_it_be(:wednesday) { tuesday + 1.day }
let_it_be(:thursday) { wednesday + 1.day }
let_it_be(:friday) { thursday + 1.day }
let_it_be(:saturday) { friday + 1.day }
let_it_be(:sunday) { saturday + 1.day }
# Using multiple participants in different rotations
# to be able to simultaneously save shifts which would
# conflict if they were part of the same rotation
let_it_be(:participant2) { create(:incident_management_oncall_participant, :with_developer_access) }
let_it_be(:participant3) { create(:incident_management_oncall_participant, :with_developer_access) }
# First rotation
let_it_be(:mon_to_tue) { create_shift(monday, tuesday, participant) }
let_it_be(:tue_to_wed) { create_shift(tuesday, wednesday, participant) }
let_it_be(:wed_to_thu) { create_shift(wednesday, thursday, participant) }
let_it_be(:thu_to_fri) { create_shift(thursday, friday, participant) }
let_it_be(:fri_to_sat) { create_shift(friday, saturday, participant) }
let_it_be(:sat_to_sun) { create_shift(saturday, sunday, participant) }
# Second rotation
let_it_be(:mon_to_thu) { create_shift(monday, thursday, participant2) }
let_it_be(:fri_to_sun) { create_shift(friday, sunday, participant2) }
# Third rotation
let_it_be(:tue_to_sun) { create_shift(wednesday, sunday, participant3) }
let_it_be(:monday) { Time.current.next_week(:monday) }
let_it_be(:tuesday) { monday + 1.day }
let_it_be(:wednesday) { tuesday + 1.day }
let_it_be(:thursday) { wednesday + 1.day }
let_it_be(:friday) { thursday + 1.day }
let_it_be(:saturday) { friday + 1.day }
let_it_be(:sunday) { saturday + 1.day }
# Using multiple participants in different rotations
# to be able to simultaneously save shifts which would
# conflict if they were part of the same rotation
let_it_be(:participant2) { create(:incident_management_oncall_participant, :with_developer_access) }
let_it_be(:participant3) { create(:incident_management_oncall_participant, :with_developer_access) }
# First rotation
let_it_be(:mon_to_tue) { create_shift(monday, tuesday, participant) }
let_it_be(:tue_to_wed) { create_shift(tuesday, wednesday, participant) }
let_it_be(:wed_to_thu) { create_shift(wednesday, thursday, participant) }
let_it_be(:thu_to_fri) { create_shift(thursday, friday, participant) }
let_it_be(:fri_to_sat) { create_shift(friday, saturday, participant) }
let_it_be(:sat_to_sun) { create_shift(saturday, sunday, participant) }
# Second rotation
let_it_be(:mon_to_thu) { create_shift(monday, thursday, participant2) }
let_it_be(:fri_to_sun) { create_shift(friday, sunday, participant2) }
# Third rotation
let_it_be(:tue_to_sun) { create_shift(tuesday, sunday, participant3) }
describe '.for_timeframe' do
subject(:shifts) { described_class.for_timeframe(wednesday, saturday) }
it 'includes shifts which cover the timeframe' do
......@@ -102,10 +102,32 @@ RSpec.describe IncidentManagement::OncallShift do
describe '.order_starts_at_desc' do
subject { described_class.order_starts_at_desc }
let_it_be(:shift1) { create_shift(Time.current, Time.current + 1.hour, participant) }
let_it_be(:shift2) { create_shift(Time.current + 2.hours, Time.current + 3.hours, participant) }
it do
is_expected.to eq [
sat_to_sun,
fri_to_sun,
fri_to_sat,
thu_to_fri,
wed_to_thu,
tue_to_sun,
tue_to_wed,
mon_to_thu,
mon_to_tue
]
end
end
describe '.for_timestamp' do
subject(:shifts) { described_class.for_timestamp(wednesday) }
it { is_expected.to eq [shift2, shift1]}
it 'includes shifts which cover the timestamp' do
expect(shifts).to contain_exactly(
mon_to_thu, # Covers timestamp
wed_to_thu, # Starts at timestamp
tue_to_sun # Covers timestamp
)
# Notable excluded shift: tue_to_wed (Ends at timestamp)
end
end
end
......
......@@ -55,6 +55,7 @@ RSpec.describe Project do
it { is_expected.to have_many(:approval_rules) }
it { is_expected.to have_many(:incident_management_oncall_schedules).class_name('IncidentManagement::OncallSchedule') }
it { is_expected.to have_many(:incident_management_oncall_rotations).through(:incident_management_oncall_schedules).source(:rotations) }
describe '#jira_issue_association_required_to_merge_enabled?' do
using RSpec::Parameterized::TableSyntax
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe AlertManagement::ProcessPrometheusAlertService do
let_it_be(:project, refind: true) { create(:project) }
before do
allow(ProjectServiceWorker).to receive(:perform_async)
end
describe '#execute' do
let(:service) { described_class.new(project, payload) }
subject(:execute) { service.execute }
context 'when alert payload is valid' do
let(:parsed_payload) { Gitlab::AlertManagement::Payload.parse(project, payload, monitoring_tool: 'Prometheus') }
let(:fingerprint) { parsed_payload.gitlab_fingerprint }
let(:payload) do
{
'status' => 'firing',
'labels' => { 'alertname' => 'GitalyFileServerDown' },
'annotations' => { 'title' => 'Alert title' },
'startsAt' => '2020-04-27T10:10:22.265949279Z',
'generatorURL' => 'http://8d467bd4607a:9090/graph?g0.expr=vector%281%29&g0.tab=1'
}
end
context 'with on-call schedule' do
let_it_be(:schedule) { create(:incident_management_oncall_schedule, project: project) }
let_it_be(:rotation) { create(:incident_management_oncall_rotation, schedule: schedule) }
let_it_be(:participant) { create(:incident_management_oncall_participant, :with_developer_access, rotation: rotation) }
let(:notification_args) do
[
[participant.user],
having_attributes(class: AlertManagement::Alert, fingerprint: fingerprint)
]
end
it_behaves_like 'Alert Notification Service sends notification email to on-call users'
end
end
end
end
......@@ -862,4 +862,18 @@ RSpec.describe EE::NotificationService, :mailer do
end
end
end
context 'IncidentManagement::Oncall' do
describe '#notify_oncall_users_of_alert' do
let_it_be(:user) { create(:user) }
let_it_be(:alert) { create(:alert_management_alert) }
let_it_be(:project) { alert.project }
it 'sends an email to the specified users' do
expect(Notify).to receive(:prometheus_alert_fired_email).with(project, user, alert).and_call_original
subject.notify_oncall_users_of_alert([user], alert)
end
end
end
end
......@@ -6,6 +6,7 @@ RSpec.describe Projects::Alerting::NotifyService do
let_it_be(:project, refind: true) { create(:project) }
describe '#execute' do
let_it_be(:integration) { create(:alert_management_http_integration, project: project) }
let(:service) { described_class.new(project, payload) }
let(:token) { integration.token }
let(:payload) do
......@@ -14,8 +15,6 @@ RSpec.describe Projects::Alerting::NotifyService do
}
end
let(:integration) { create(:alert_management_http_integration, project: project) }
subject { service.execute(token, integration) }
context 'existing alert with same payload fingerprint' do
......@@ -64,5 +63,19 @@ RSpec.describe Projects::Alerting::NotifyService do
end
end
end
context 'with on-call schedules' do
let_it_be(:schedule) { create(:incident_management_oncall_schedule, project: project) }
let_it_be(:rotation) { create(:incident_management_oncall_rotation, schedule: schedule) }
let_it_be(:participant) { create(:incident_management_oncall_participant, :with_developer_access, rotation: rotation) }
let(:notification_args) do
[
[participant.user],
having_attributes(class: AlertManagement::Alert, title: payload['title'])
]
end
it_behaves_like 'Alert Notification Service sends notification email to on-call users'
end
end
end
# frozen_string_literal: true
RSpec.shared_examples 'Alert Notification Service sends notification email to on-call users' do
let(:notification_service) { instance_double(NotificationService) }
context 'with oncall schedules enabled' do
before do
stub_licensed_features(oncall_schedules: project)
end
it 'sends a notification' do
expect(NotificationService).to receive(:new).and_return(notification_service)
expect(notification_service)
.to receive_message_chain(:async, :notify_oncall_users_of_alert)
.with(*notification_args)
expect(subject).to be_success
end
end
context 'with oncall schedules disabled' do
it 'does not notify the on-call users' do
expect(NotificationService).not_to receive(:new)
expect(subject).to be_success
end
end
end
......@@ -231,7 +231,7 @@ module API
post ':id/issues' do
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42320')
check_rate_limit! :issues_create, [current_user, :issues_create]
check_rate_limit! :issues_create, [current_user]
authorize! :create_issue, user_project
......
......@@ -4,6 +4,7 @@ module API
class Notes < ::API::Base
include PaginationParams
helpers ::API::Helpers::NotesHelpers
helpers Helpers::RateLimiter
before { authenticate! }
......@@ -72,6 +73,7 @@ module API
optional :created_at, type: String, desc: 'The creation date of the note'
end
post ":id/#{noteables_str}/:noteable_id/notes", feature_category: feature_category do
check_rate_limit! :notes_create, [current_user]
noteable = find_noteable(noteable_type, params[:noteable_id])
opts = {
......
......@@ -20,6 +20,7 @@ module Gitlab
def rate_limits
{
issues_create: { threshold: -> { application_settings.issues_create_limit }, interval: 1.minute },
notes_create: { threshold: -> { application_settings.notes_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 },
......
......@@ -7504,6 +7504,9 @@ msgstr ""
msgid "Configure limit for issues created per minute by web and API requests."
msgstr ""
msgid "Configure limit for notes created per minute by web and API requests."
msgstr ""
msgid "Configure limits for Project/Group Import/Export."
msgstr ""
......@@ -17964,6 +17967,9 @@ msgstr ""
msgid "Max file size is 200 KB."
msgstr ""
msgid "Max requests per minute per user"
msgstr ""
msgid "Max role"
msgstr ""
......@@ -20143,6 +20149,9 @@ msgstr ""
msgid "NoteForm|Note"
msgstr ""
msgid "Notes Rate Limits"
msgstr ""
msgid "Notes|Are you sure you want to cancel creating this comment?"
msgstr ""
......
......@@ -727,6 +727,42 @@ RSpec.describe Projects::NotesController do
end
end
end
context 'when the endpoint receives requests above the limit' do
before do
stub_application_setting(notes_create_limit: 5)
end
it 'prevents from creating more notes', :request_store do
5.times { create! }
expect { create! }
.to change { Gitlab::GitalyClient.get_request_count }.by(0)
create!
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 in auth.log' do
attributes = {
message: 'Application_Rate_Limiter_Request',
env: :notes_create_request_limit,
remote_ip: '0.0.0.0',
request_method: 'POST',
path: "/#{project.full_path}/notes",
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 { create! }
end
end
end
describe 'PUT update' do
......
......@@ -561,6 +561,7 @@ project:
- alert_management_http_integrations
- exported_protected_branches
- incident_management_oncall_schedules
- incident_management_oncall_rotations
- debian_distributions
- merge_request_metrics
award_emoji:
......
......@@ -114,6 +114,12 @@ RSpec.describe ApplicationSetting do
it { is_expected.to allow_value(nil).for(:repository_storages_weighted_default) }
it { is_expected.not_to allow_value({ default: 100, shouldntexist: 50 }).for(:repository_storages_weighted) }
it { is_expected.to allow_value(400).for(:notes_create_limit) }
it { is_expected.not_to allow_value('two').for(:notes_create_limit) }
it { is_expected.not_to allow_value(nil).for(:notes_create_limit) }
it { is_expected.not_to allow_value(5.5).for(:notes_create_limit) }
it { is_expected.not_to allow_value(-2).for(:notes_create_limit) }
context 'help_page_documentation_base_url validations' do
it { is_expected.to allow_value(nil).for(:help_page_documentation_base_url) }
it { is_expected.to allow_value('https://docs.gitlab.com').for(:help_page_documentation_base_url) }
......
......@@ -43,6 +43,8 @@ RSpec.describe 'Adding a DiffNote' do
it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote
it_behaves_like 'a Note mutation when there are rate limit validation errors'
context do
let(:diff_refs) { build(:commit).diff_refs } # Allow fake diff refs so arguments are valid
......
......@@ -46,6 +46,8 @@ RSpec.describe 'Adding an image DiffNote' do
it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote
it_behaves_like 'a Note mutation when there are rate limit validation errors'
context do
let(:diff_refs) { build(:commit).diff_refs } # Allow fake diff refs so arguments are valid
......
......@@ -37,6 +37,8 @@ RSpec.describe 'Adding a Note' do
it_behaves_like 'a Note mutation when the given resource id is not for a Noteable'
it_behaves_like 'a Note mutation when there are rate limit validation errors'
it 'returns the note' do
post_graphql_mutation(mutation, current_user: current_user)
......
......@@ -64,3 +64,14 @@ RSpec.shared_examples 'a Note mutation when the given resource id is not for a N
let(:match_errors) { include(/does not represent an instance of Note/) }
end
end
RSpec.shared_examples 'a Note mutation when there are rate limit validation errors' do
before do
stub_application_setting(notes_create_limit: 3)
3.times { post_graphql_mutation(mutation, current_user: current_user) }
end
it_behaves_like 'a Note mutation that does not create a Note'
it_behaves_like 'a mutation that returns top-level errors',
errors: ['This endpoint has been requested too many times. Try again later.']
end
......@@ -274,6 +274,19 @@ RSpec.shared_examples 'noteable API' do |parent_type, noteable_type, id_name|
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when request exceeds the rate limit' do
before do
allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
end
it 'prevents users from creating more notes' do
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), params: { body: 'hi!' }
expect(response).to have_gitlab_http_status(:too_many_requests)
expect(json_response['message']['error']).to eq('This endpoint has been requested too many times. Try again later.')
end
end
end
describe "PUT /#{parent_type}/:id/#{noteable_type}/:noteable_id/notes/:note_id" do
......
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