Merge branch 'anti-abuse/44-add-allowlisted-users-to-git-abuse-service' into 'master'

Add allowlisted users to git anti-abuse service

See merge request gitlab-org/gitlab!91181

ee/app/services/users/abuse/excessive_projects_download_ban_service.rb

@@ -35,7 +35,8 @@ def rate_limited?(peek: false)
           :unique_project_downloads_for_application,
           scope: current_user,
           resource: project,
-          peek: peek
+          peek: peek,
+          users_allowlist: users_allowlist
         )
       end
 
@@ -92,6 +93,10 @@ def time_period
         @time_period ||= settings.max_number_of_repository_downloads_within_time_period
       end
 
+      def users_allowlist
+        @git_rate_limit_users_allowlist ||= settings.git_rate_limit_users_allowlist
+      end
+
       def settings
         @settings ||= Gitlab::CurrentSettings.current_application_settings
       end

ee/spec/services/users/abuse/excessive_projects_download_ban_service_spec.rb

@@ -2,47 +2,51 @@
 
 require 'spec_helper'
 
-RSpec.describe Users::Abuse::ExcessiveProjectsDownloadBanService, :clean_gitlab_redis_shared_state do
+RSpec.describe Users::Abuse::ExcessiveProjectsDownloadBanService, :clean_gitlab_redis_rate_limiting do
   describe '.execute' do
     let_it_be(:admin) { create(:user, :admin) }
 
     let(:user) { create(:user) }
+    let(:project) { create(:project) }
     let(:limit) { 3 }
     let(:time_period_in_seconds) { 60 }
 
-    subject(:execute) { described_class.execute(user, create(:project)) }
+    subject(:execute) { described_class.execute(user, project) }
 
     before do
       stub_application_setting(max_number_of_repository_downloads: limit)
       stub_application_setting(max_number_of_repository_downloads_within_time_period: time_period_in_seconds)
     end
 
-    it 'counts repeated downloads of a project only once' do
-      expect(user).not_to receive(:ban!)
+    shared_examples 'sends email to admins' do
+      it 'sends email to admins', :aggregate_failures do
+        double = instance_double(ActionMailer::MessageDelivery, deliver_later: nil)
+        expect(Notify).to receive(:user_auto_banned_email) { double }
+          .with(admin.id, user.id, max_project_downloads: limit, within_seconds: time_period_in_seconds)
+          .once
+        expect(double).to receive(:deliver_later).once
 
-      project = create(:project)
-      (limit + 1).times { described_class.execute(user, project) }
+        execute
+      end
     end
 
-    it 'returns { banned: false } when user does not exceed download limit' do
-      expect(execute).to include(banned: false)
-    end
+    context 'when user downloads the same project multiple times within the set time period' do
+      before do
+        (limit + 1).times { described_class.execute(user, project) }
+      end
 
-    context 'when user exceeds the download limit within the set time period' do
-      shared_examples 'sends email to admins' do
-        it 'sends email to admins', :aggregate_failures do
-          double = instance_double(ActionMailer::MessageDelivery, deliver_later: nil)
-          expect(Notify).to receive(:user_auto_banned_email) { double }
-            .with(admin.id, user.id, max_project_downloads: limit, within_seconds: time_period_in_seconds)
-            .once
-          expect(double).to receive(:deliver_later).once
+      it 'counts repeated downloads of a project only once' do
+        expect(user).not_to receive(:ban!)
+      end
 
-          execute
-        end
+      it 'returns { banned: false } when user does not exceed download limit' do
+        expect(execute).to include(banned: false)
       end
+    end
 
+    context 'when user exceeds the download limit within the set time period' do
       before do
-        limit.times { described_class.execute(user, create(:project)) }
+        limit.times { described_class.execute(user, build_stubbed(:project)) }
       end
 
       it { is_expected.to include(banned: true) }
@@ -73,60 +77,88 @@
 
       it_behaves_like 'sends email to admins'
 
-      context 'when auto_ban_user_on_excessive_projects_download feature flag is disabled' do
-        before do
-          stub_feature_flags(auto_ban_user_on_excessive_projects_download: false)
-        end
+      it 'sends email to admins only once' do
+        (limit + 1).times { described_class.execute(user, build_stubbed(:project)) }
 
-        it { is_expected.to include(banned: false) }
+        expect(Notify).not_to receive(:user_auto_banned_email)
 
-        it 'does not ban the user' do
-          expect(user).not_to receive(:ban!)
+        execute
+      end
+    end
 
-          execute
-        end
+    context 'when auto_ban_user_on_excessive_projects_download feature flag is disabled' do
+      before do
+        stub_feature_flags(auto_ban_user_on_excessive_projects_download: false)
+        limit.times { described_class.execute(user, build_stubbed(:project)) }
+      end
+
+      it { is_expected.to include(banned: false) }
 
-        it 'does not log a ban event' do
-          expect(Gitlab::AppLogger).not_to receive(:info).with(
-            message: "User ban",
-            user: user.username,
-            email: user.email,
-            ban_by: described_class.name
-          )
+      it 'does not ban the user' do
+        expect(user).not_to receive(:ban!)
 
-          execute
-        end
+        execute
+      end
 
-        it_behaves_like 'sends email to admins'
+      it 'does not log a ban event' do
+        expect(Gitlab::AppLogger).not_to receive(:info).with(
+          message: "User ban",
+          user: user.username,
+          email: user.email,
+          ban_by: described_class.name
+        )
+
+        execute
       end
 
-      context 'when user is already banned' do
-        before do
-          user.ban!
-        end
+      it_behaves_like 'sends email to admins'
+    end
+
+    context 'when user is already banned' do
+      before do
+        user.ban!
+        limit.times { described_class.execute(user, build_stubbed(:project)) }
+      end
 
-        it { is_expected.to include(banned: true) }
+      it { is_expected.to include(banned: true) }
 
-        it 'logs the event' do
-          expect(Gitlab::AppLogger).not_to receive(:info).with(
-            message: "Invalid transition when banning: \
-              Cannot transition state via :ban from :banned (Reason(s): State cannot transition via \"ban\"",
-            user: user.username,
-            email: user.email,
-            ban_by: described_class.name
-          )
+      it 'logs the event' do
+        expect(Gitlab::AppLogger).not_to receive(:info).with(
+          message: "Invalid transition when banning: \
+            Cannot transition state via :ban from :banned (Reason(s): State cannot transition via \"ban\"",
+          user: user.username,
+          email: user.email,
+          ban_by: described_class.name
+        )
 
-          execute
-        end
+        execute
       end
     end
 
-    it 'sends email to admins only once' do
-      (limit + 1).times { described_class.execute(user, create(:project)) }
+    context 'when allowlisted user exceeds the download limit within the set time period' do
+      before do
+        stub_application_setting(git_rate_limit_users_allowlist: [user.username])
+        limit.times { described_class.execute(user, build_stubbed(:project)) }
+      end
 
-      expect(Notify).not_to receive(:user_auto_banned_email)
+      it { is_expected.to include(banned: false) }
 
-      execute
+      it 'does not ban the user' do
+        expect(user).not_to receive(:ban!)
+
+        execute
+      end
+
+      it 'does not log a ban event' do
+        expect(Gitlab::AppLogger).not_to receive(:info).with(
+          message: "User ban",
+          user: user.username,
+          email: user.email,
+          ban_by: described_class.name
+        )
+
+        execute
+      end
     end
   end
 end