diff --git a/lib/api/members.rb b/lib/api/members.rb index 1e640a6542a3250199e14a5a2e5dd3cecb9e922e..9321b7ad8d5b678da71d053c7759c5a3e008e70a 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -177,6 +177,8 @@ class Members < ::API::Base source = find_source(source_type, params[:id]) member = source_members(source).find_by!(user_id: params[:user_id]) + check_rate_limit!(:member_delete, scope: [source, current_user]) + destroy_conditionally!(member) do ::Members::DestroyService.new(current_user).execute(member, skip_subresources: params[:skip_subresources], unassign_issuables: params[:unassign_issuables]) end diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb index a336b3fefe17be94b9f6a6363e2dbc413e309c70..a8e74cbd7e6b36661dd5f23533c619cef262dece 100644 --- a/lib/gitlab/application_rate_limiter.rb +++ b/lib/gitlab/application_rate_limiter.rb @@ -30,6 +30,7 @@ def rate_limits # rubocop:disable Metrics/AbcSize group_download_export: { threshold: -> { application_settings.group_download_export_limit }, interval: 1.minute }, group_import: { threshold: -> { application_settings.group_import_limit }, interval: 1.minute }, group_testing_hook: { threshold: 5, interval: 1.minute }, + member_delete: { threshold: 60, interval: 1.minute }, profile_add_new_email: { threshold: 5, interval: 1.minute }, web_hook_calls: { interval: 1.minute }, web_hook_calls_mid: { interval: 1.minute }, diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 4eff5e96e9cfc6ad0ae353b3490467408bb2af53..85ccfa3cf513be09a1f104d41d889ec732ef8a69 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -739,6 +739,30 @@ end.to change { source.members.count }.by(-1) end + it_behaves_like 'rate limited endpoint', rate_limit_key: :member_delete do + let(:current_user) { maintainer } + + let(:another_member) { create(:user) } + + before do + source.add_developer(another_member) + end + + # We rate limit scoped by the group / project + let(:delete_paths) do + [ + api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer), + api("/#{source_type.pluralize}/#{source.id}/members/#{another_member.id}", maintainer) + ] + end + + def request + delete_member_path = delete_paths.shift + + delete delete_member_path + end + end + it_behaves_like '412 response' do let(:request) { api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer) } end