diff --git a/ee/app/services/users/abuse/excessive_projects_download_ban_service.rb b/ee/app/services/users/abuse/excessive_projects_download_ban_service.rb
index cdf6d60b20498ea39add22fbf3152591ef180145..5f54a03dd22a3725ca93e36d39a10625a7da5017 100644
--- a/ee/app/services/users/abuse/excessive_projects_download_ban_service.rb
+++ b/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
diff --git a/ee/spec/services/users/abuse/excessive_projects_download_ban_service_spec.rb b/ee/spec/services/users/abuse/excessive_projects_download_ban_service_spec.rb
index 3e33cd85e09ac80d334b408a13cc5415e6a46fc8..ccfc218178c89b6d8ceb84ded3a9eb6cee904c92 100644
--- a/ee/spec/services/users/abuse/excessive_projects_download_ban_service_spec.rb
+++ b/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