From ec474567789b98645dccc1d46313df465b3f3ba9 Mon Sep 17 00:00:00 2001
From: Thong Kuah <tkuah@gitlab.com>
Date: Fri, 21 Apr 2023 14:52:51 +1200
Subject: [PATCH] Rate limit API deletion of member

- Rate limit project member deletion
- Rate limit group member deletion

Excessive calls leads to too many ToDosDestroyer::EntityLeaveWorker jobs
being scheduled.

Changelog: changed
---
 lib/api/members.rb                     |  2 ++
 lib/gitlab/application_rate_limiter.rb |  1 +
 spec/requests/api/members_spec.rb      | 24 ++++++++++++++++++++++++
 3 files changed, 27 insertions(+)

diff --git a/lib/api/members.rb b/lib/api/members.rb
index 1e640a6542a325..9321b7ad8d5b67 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 a336b3fefe17be..a8e74cbd7e6b36 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 4eff5e96e9cfc6..85ccfa3cf513be 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
-- 
GitLab