diff --git a/app/services/spam/spam_action_service.rb b/app/services/spam/spam_action_service.rb
index b933b5dbb30e9b23bf139185e04a56afd85c44bf..1e4ca55b08d0b5d44699fc68a2be843e2128ee12 100644
--- a/app/services/spam/spam_action_service.rb
+++ b/app/services/spam/spam_action_service.rb
@@ -132,8 +132,10 @@ def create_spam_abuse_event(result)
verdict: result
}
+ base_class = Feature.enabled?(:rename_abuse_workers, user, type: :worker) ? AntiAbuse : Abuse
+
target.run_after_commit_or_now do
- Abuse::SpamAbuseEventsWorker.perform_async(params)
+ base_class::SpamAbuseEventsWorker.perform_async(params)
end
end
diff --git a/app/services/spam/spam_verdict_service.rb b/app/services/spam/spam_verdict_service.rb
index f69ee255e01ae2195a6006773da2c1ec7facab36..a8381589cc937ef82ff4120692b843588eaa6ba0 100644
--- a/app/services/spam/spam_verdict_service.rb
+++ b/app/services/spam/spam_verdict_service.rb
@@ -71,7 +71,8 @@ def get_spamcheck_verdict
if result.evaluated?
correlation_id = Labkit::Correlation::CorrelationId.current_id || ''
- Abuse::TrustScoreWorker.perform_async(user.id, :spamcheck, result.score, correlation_id)
+ base_class = Feature.enabled?(:rename_abuse_workers, user, type: :worker) ? AntiAbuse : Abuse
+ base_class::TrustScoreWorker.perform_async(user.id, :spamcheck, result.score, correlation_id)
end
result.verdict
diff --git a/app/workers/abuse/spam_abuse_events_worker.rb b/app/workers/abuse/spam_abuse_events_worker.rb
index 65c669abfdba8db955f13325369ccdb36c6a7d63..6afda83d60467d58e09fe7377e14bb49b309a2c8 100644
--- a/app/workers/abuse/spam_abuse_events_worker.rb
+++ b/app/workers/abuse/spam_abuse_events_worker.rb
@@ -11,50 +11,7 @@ class SpamAbuseEventsWorker
urgency :low
def perform(params)
- params = params.with_indifferent_access
-
- @user = User.find_by_id(params[:user_id])
- unless @user
- logger.info(structured_payload(message: "User not found.", user_id: params[:user_id]))
- return
- end
-
- report_user(params)
- end
-
- private
-
- attr_reader :user
-
- def report_user(params)
- category = 'spam'
- reporter = Users::Internal.security_bot
- report_params = { user_id: params[:user_id],
- reporter: reporter,
- category: category,
- message: 'User reported for abuse based on spam verdict' }
-
- abuse_report = AbuseReport.by_category(category).by_reporter_id(reporter.id).by_user_id(params[:user_id]).first
-
- abuse_report = AbuseReport.create!(report_params) if abuse_report.nil?
-
- create_abuse_event(abuse_report.id, params)
- end
-
- # Associate the abuse report with an abuse event
- def create_abuse_event(abuse_report_id, params)
- AntiAbuse::Event.create!(
- abuse_report_id: abuse_report_id,
- category: :spam,
- metadata: { noteable_type: params[:noteable_type],
- title: params[:title],
- description: params[:description],
- source_ip: params[:source_ip],
- user_agent: params[:user_agent],
- verdict: params[:verdict] },
- source: :spamcheck,
- user: user
- )
+ AntiAbuse::SpamAbuseEventsWorker.new.perform(params)
end
end
end
diff --git a/app/workers/abuse/trust_score_worker.rb b/app/workers/abuse/trust_score_worker.rb
index 2bb09ddcbe9f3c0ed138df3550b6025c1f24ea29..d7b713964fb15c9108ff7e02221597df4f38469c 100644
--- a/app/workers/abuse/trust_score_worker.rb
+++ b/app/workers/abuse/trust_score_worker.rb
@@ -11,14 +11,7 @@ class TrustScoreWorker
urgency :low
def perform(user_id, source, score, correlation_id = '')
- user = User.find_by_id(user_id)
- unless user
- logger.info(structured_payload(message: "User not found.", user_id: user_id))
- return
- end
-
- AntiAbuse::TrustScore.create!(user: user, source: source, score: score.to_f, correlation_id_value: correlation_id)
- AntiAbuse::TrustScoreCleanupWorker.perform_async(user.id, source)
+ AntiAbuse::TrustScoreWorker.new.perform(user_id, source, score, correlation_id)
end
end
end
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index 5e6c0803dc2a899ecd0e20643bf2df7667d1fac9..1709693ce367eb5c77a99124b519967ef2dfe486 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -2415,6 +2415,24 @@
:weight: 1
:idempotent: true
:tags: []
+- :name: anti_abuse_spam_abuse_events
+ :worker_name: AntiAbuse::SpamAbuseEventsWorker
+ :feature_category: :instance_resiliency
+ :has_external_dependencies: false
+ :urgency: :low
+ :resource_boundary: :unknown
+ :weight: 1
+ :idempotent: true
+ :tags: []
+- :name: anti_abuse_trust_score
+ :worker_name: AntiAbuse::TrustScoreWorker
+ :feature_category: :instance_resiliency
+ :has_external_dependencies: false
+ :urgency: :low
+ :resource_boundary: :unknown
+ :weight: 1
+ :idempotent: true
+ :tags: []
- :name: anti_abuse_trust_score_cleanup
:worker_name: AntiAbuse::TrustScoreCleanupWorker
:feature_category: :instance_resiliency
diff --git a/app/workers/anti_abuse/spam_abuse_events_worker.rb b/app/workers/anti_abuse/spam_abuse_events_worker.rb
new file mode 100644
index 0000000000000000000000000000000000000000..53dfaae1b422676c1262d7e179983522187c76ec
--- /dev/null
+++ b/app/workers/anti_abuse/spam_abuse_events_worker.rb
@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+module AntiAbuse
+ class SpamAbuseEventsWorker
+ include ApplicationWorker
+
+ data_consistency :delayed
+
+ idempotent!
+ feature_category :instance_resiliency
+ urgency :low
+
+ def perform(params)
+ params = params.with_indifferent_access
+
+ @user = User.find_by_id(params[:user_id])
+ unless @user
+ logger.info(structured_payload(message: "User not found.", user_id: params[:user_id]))
+ return
+ end
+
+ report_user(params)
+ end
+
+ private
+
+ attr_reader :user
+
+ def report_user(params)
+ category = 'spam'
+ reporter = Users::Internal.security_bot
+ report_params = { user_id: params[:user_id],
+ reporter: reporter,
+ category: category,
+ message: 'User reported for abuse based on spam verdict' }
+
+ abuse_report = AbuseReport.by_category(category).by_reporter_id(reporter.id).by_user_id(params[:user_id]).first
+
+ abuse_report = AbuseReport.create!(report_params) if abuse_report.nil?
+
+ create_abuse_event(abuse_report.id, params)
+ end
+
+ # Associate the abuse report with an abuse event
+ def create_abuse_event(abuse_report_id, params)
+ AntiAbuse::Event.create!(
+ abuse_report_id: abuse_report_id,
+ category: :spam,
+ metadata: { noteable_type: params[:noteable_type],
+ title: params[:title],
+ description: params[:description],
+ source_ip: params[:source_ip],
+ user_agent: params[:user_agent],
+ verdict: params[:verdict] },
+ source: :spamcheck,
+ user: user
+ )
+ end
+ end
+end
diff --git a/app/workers/anti_abuse/trust_score_worker.rb b/app/workers/anti_abuse/trust_score_worker.rb
new file mode 100644
index 0000000000000000000000000000000000000000..fc72b617e92032f89d850e96663573a56605d1fc
--- /dev/null
+++ b/app/workers/anti_abuse/trust_score_worker.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module AntiAbuse
+ class TrustScoreWorker
+ include ApplicationWorker
+
+ data_consistency :delayed
+
+ idempotent!
+ feature_category :instance_resiliency
+ urgency :low
+
+ def perform(user_id, source, score, correlation_id = '')
+ user = User.find_by_id(user_id)
+ unless user
+ logger.info(structured_payload(message: "User not found.", user_id: user_id))
+ return
+ end
+
+ AntiAbuse::TrustScore.create!(user: user, source: source, score: score.to_f, correlation_id_value: correlation_id)
+ AntiAbuse::TrustScoreCleanupWorker.perform_async(user.id, source)
+ end
+ end
+end
diff --git a/config/feature_flags/worker/rename_abuse_workers.yml b/config/feature_flags/worker/rename_abuse_workers.yml
new file mode 100644
index 0000000000000000000000000000000000000000..85b33aca447efbb4a53377af542844915783eb28
--- /dev/null
+++ b/config/feature_flags/worker/rename_abuse_workers.yml
@@ -0,0 +1,9 @@
+---
+name: rename_abuse_workers
+feature_issue_url: https://gitlab.com/gitlab-org/modelops/anti-abuse/team-tasks/-/issues/850
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/163792
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/480273
+milestone: '17.4'
+group: group::anti-abuse
+type: worker
+default_enabled: false
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index a62d4b789be2b93c04315c18af55fa9130e0ed5b..0ba0b02d36c76e36e6da2d22a822c4bc7e806c3c 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -43,6 +43,12 @@
- 1
- - analytics_usage_trends_counter_job
- 1
+- - anti_abuse_new_abuse_report
+ - 1
+- - anti_abuse_spam_abuse_events
+ - 1
+- - anti_abuse_trust_score
+ - 1
- - anti_abuse_trust_score_cleanup
- 1
- - app_sec_container_scanning_scan_image
diff --git a/ee/app/models/ee/abuse_report.rb b/ee/app/models/ee/abuse_report.rb
index 3e018c3ca6793afc3350ec95922c86b14b4ca3cc..0675e2cbe6660e84da71cd1a38f37d3d12c36b75 100644
--- a/ee/app/models/ee/abuse_report.rb
+++ b/ee/app/models/ee/abuse_report.rb
@@ -32,8 +32,10 @@ def reported_content
private
def run_abuse_report_worker
+ base_class = ::Feature.enabled?(:rename_abuse_workers, user, type: :worker) ? AntiAbuse : Abuse
+
run_after_commit_or_now do
- Abuse::NewAbuseReportWorker.perform_async(id)
+ base_class::NewAbuseReportWorker.perform_async(id)
end
end
end
diff --git a/ee/app/services/arkose/record_user_data_service.rb b/ee/app/services/arkose/record_user_data_service.rb
index 59c599325402dcc9d180c82244daabdfa916f5a0..43bd2fa2d2831bacaa77f665ab904ba0b98f436a 100644
--- a/ee/app/services/arkose/record_user_data_service.rb
+++ b/ee/app/services/arkose/record_user_data_service.rb
@@ -44,8 +44,10 @@ def custom_attributes
end
def store_risk_scores
- Abuse::TrustScoreWorker.perform_async(user.id, :arkose_global_score, response.global_score.to_f)
- Abuse::TrustScoreWorker.perform_async(user.id, :arkose_custom_score, response.custom_score.to_f)
+ base_class = Feature.enabled?(:rename_abuse_workers, user, type: :worker) ? AntiAbuse : Abuse
+
+ base_class::TrustScoreWorker.perform_async(user.id, :arkose_global_score, response.global_score.to_f)
+ base_class::TrustScoreWorker.perform_async(user.id, :arkose_custom_score, response.custom_score.to_f)
end
def logger
diff --git a/ee/app/services/phone_verification/users/record_user_data_service.rb b/ee/app/services/phone_verification/users/record_user_data_service.rb
index ae202d6fc107a6db0c12bb22be854ba23d839a34..1d8d7deb292b68e7372685e21df06068539b3bc4 100644
--- a/ee/app/services/phone_verification/users/record_user_data_service.rb
+++ b/ee/app/services/phone_verification/users/record_user_data_service.rb
@@ -31,7 +31,8 @@ def error_related_to_high_risk_user
end
def store_risk_score(risk_score)
- Abuse::TrustScoreWorker.perform_async(user.id, :telesign, risk_score.to_f)
+ base_class = Feature.enabled?(:rename_abuse_workers, user, type: :worker) ? AntiAbuse : Abuse
+ base_class::TrustScoreWorker.perform_async(user.id, :telesign, risk_score.to_f)
end
def assume_high_risk?
diff --git a/ee/app/workers/abuse/new_abuse_report_worker.rb b/ee/app/workers/abuse/new_abuse_report_worker.rb
index 007bfb1e585cf55a69b01f968814de6904304ff8..179ec2bba655139cc60b787802813e0adf0c92f0 100644
--- a/ee/app/workers/abuse/new_abuse_report_worker.rb
+++ b/ee/app/workers/abuse/new_abuse_report_worker.rb
@@ -11,49 +11,8 @@ class NewAbuseReportWorker
idempotent!
- attr_reader :user, :reporter, :abuse_report
-
def perform(abuse_report_id)
- @abuse_report = AbuseReport.find_by_id(abuse_report_id)
- return unless abuse_report&.category == 'spam'
-
- @reporter = abuse_report.reporter
- @user = abuse_report.user
-
- return unless user && reporter
- return unless reporter.gitlab_employee?
- return unless bannable_user?
-
- ApplicationRecord.transaction do
- Users::AutoBanService.new(user: user, reason: abuse_report.category).execute!
- UserCustomAttribute.set_banned_by_abuse_report(abuse_report)
- end
-
- log_event
- end
-
- private
-
- def bannable_user?
- return false unless user.active? && user.human?
- return false if user.gitlab_employee? || user.account_age_in_days > 7
- return false if user.belongs_to_paid_namespace?(exclude_trials: true) || user_owns_populated_namespaces?
-
- true
- end
-
- def user_owns_populated_namespaces?
- user.owned_groups.find { |group| group.users_count > 5 }
- end
-
- def log_event
- Gitlab::AppLogger.info(
- message: "User ban",
- user_id: user.id,
- username: user.username,
- abuse_report_id: abuse_report.id,
- reason: "Automatic ban triggered by abuse report for #{abuse_report.category}."
- )
+ AntiAbuse::NewAbuseReportWorker.new.perform(abuse_report_id)
end
end
end
diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml
index 28d6d825338f6e9a9b91963f06783f888b97668c..7fe38a1134a7cf7fbd3b5bb9a1c798cbdaa05d05 100644
--- a/ee/app/workers/all_queues.yml
+++ b/ee/app/workers/all_queues.yml
@@ -1029,6 +1029,15 @@
:weight: 1
:idempotent: true
:tags: []
+- :name: anti_abuse_new_abuse_report
+ :worker_name: AntiAbuse::NewAbuseReportWorker
+ :feature_category: :instance_resiliency
+ :has_external_dependencies: false
+ :urgency: :low
+ :resource_boundary: :unknown
+ :weight: 1
+ :idempotent: true
+ :tags: []
- :name: app_sec_container_scanning_scan_image
:worker_name: AppSec::ContainerScanning::ScanImageWorker
:feature_category: :software_composition_analysis
diff --git a/ee/app/workers/anti_abuse/new_abuse_report_worker.rb b/ee/app/workers/anti_abuse/new_abuse_report_worker.rb
new file mode 100644
index 0000000000000000000000000000000000000000..0994084d6a9dee811ec8ecd42d8a3b28595a6f15
--- /dev/null
+++ b/ee/app/workers/anti_abuse/new_abuse_report_worker.rb
@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+module AntiAbuse
+ class NewAbuseReportWorker
+ include ApplicationWorker
+
+ feature_category :instance_resiliency
+
+ data_consistency :delayed
+ urgency :low
+
+ idempotent!
+
+ attr_reader :user, :reporter, :abuse_report
+
+ def perform(abuse_report_id)
+ @abuse_report = AbuseReport.find_by_id(abuse_report_id)
+ return unless abuse_report&.category == 'spam'
+
+ @reporter = abuse_report.reporter
+ @user = abuse_report.user
+
+ return unless user && reporter
+ return unless reporter.gitlab_employee?
+ return unless bannable_user?
+
+ ApplicationRecord.transaction do
+ Users::AutoBanService.new(user: user, reason: abuse_report.category).execute!
+ UserCustomAttribute.set_banned_by_abuse_report(abuse_report)
+ end
+
+ log_event
+ end
+
+ private
+
+ def bannable_user?
+ return false unless user.active? && user.human?
+ return false if user.gitlab_employee? || user.account_age_in_days > 7
+ return false if user.belongs_to_paid_namespace?(exclude_trials: true) || user_owns_populated_namespaces?
+
+ true
+ end
+
+ def user_owns_populated_namespaces?
+ user.owned_groups.find { |group| group.users_count > 5 } # rubocop: disable Gitlab/NoFindInWorkers -- not ActiveRecordFind
+ end
+
+ def log_event
+ Gitlab::AppLogger.info(
+ message: "User ban",
+ user_id: user.id,
+ username: user.username,
+ abuse_report_id: abuse_report.id,
+ reason: "Automatic ban triggered by abuse report for #{abuse_report.category}."
+ )
+ end
+ end
+end
diff --git a/ee/spec/models/abuse_report_spec.rb b/ee/spec/models/abuse_report_spec.rb
index d529f6242b2e0990440cc045b78cf13784ae7811..32a393838b1ec612c0618fb81d746735f57e6f4d 100644
--- a/ee/spec/models/abuse_report_spec.rb
+++ b/ee/spec/models/abuse_report_spec.rb
@@ -4,9 +4,22 @@
RSpec.describe AbuseReport, feature_category: :insider_threat do
describe '.create' do
- it 'calls the new abuse report worker' do
- expect(Abuse::NewAbuseReportWorker).to receive(:perform_async)
- create(:abuse_report)
+ context 'when the rename_abuse_workers feature is enabled' do
+ it 'calls the new abuse report worker' do
+ expect(AntiAbuse::NewAbuseReportWorker).to receive(:perform_async)
+ create(:abuse_report)
+ end
+ end
+
+ context 'when the rename_abuse_workers feature is not enabled' do
+ before do
+ stub_feature_flags(rename_abuse_workers: false)
+ end
+
+ it 'calls the new abuse report worker' do
+ expect(Abuse::NewAbuseReportWorker).to receive(:perform_async)
+ create(:abuse_report)
+ end
end
end
diff --git a/ee/spec/services/arkose/record_user_data_service_spec.rb b/ee/spec/services/arkose/record_user_data_service_spec.rb
index 0e2280b0079fa155f91cca37d29bcca47fb4a1f0..09f6fd98128d2b86907786ea307f4038f95769a9 100644
--- a/ee/spec/services/arkose/record_user_data_service_spec.rb
+++ b/ee/spec/services/arkose/record_user_data_service_spec.rb
@@ -30,15 +30,34 @@
expect(user.custom_attributes.find_by(key: 'arkose_custom_score').value).to eq('0')
end
- it 'executes abuse trust score workers' do
- expect(Abuse::TrustScoreWorker).to receive(:perform_async).once.ordered.with(
- user.id, :arkose_global_score, 0.0
- )
- expect(Abuse::TrustScoreWorker).to receive(:perform_async).once.ordered.with(
- user.id, :arkose_custom_score, 0.0
- )
+ context 'when the rename_abuse_workers feature is enabled' do
+ it 'executes abuse trust score workers' do
+ expect(AntiAbuse::TrustScoreWorker).to receive(:perform_async).once.ordered.with(
+ user.id, :arkose_global_score, 0.0
+ )
+ expect(AntiAbuse::TrustScoreWorker).to receive(:perform_async).once.ordered.with(
+ user.id, :arkose_custom_score, 0.0
+ )
- service.execute
+ service.execute
+ end
+ end
+
+ context 'when the rename_abuse_workers feature is not enabled' do
+ before do
+ stub_feature_flags(rename_abuse_workers: false)
+ end
+
+ it 'executes abuse trust score workers' do
+ expect(Abuse::TrustScoreWorker).to receive(:perform_async).once.ordered.with(
+ user.id, :arkose_global_score, 0.0
+ )
+ expect(Abuse::TrustScoreWorker).to receive(:perform_async).once.ordered.with(
+ user.id, :arkose_custom_score, 0.0
+ )
+
+ service.execute
+ end
end
it 'logs user risk band assignment event' do
diff --git a/ee/spec/services/phone_verification/users/record_user_data_service_spec.rb b/ee/spec/services/phone_verification/users/record_user_data_service_spec.rb
index 52c5719bb864a75b00c5ea6b6ef782e7062e85e6..6ba4f1576fd1eb8885996f97bda407af2f8c9396 100644
--- a/ee/spec/services/phone_verification/users/record_user_data_service_spec.rb
+++ b/ee/spec/services/phone_verification/users/record_user_data_service_spec.rb
@@ -35,10 +35,25 @@
expect(phone_verification_record).not_to be_persisted
end
- it 'executes the abuse trust score worker' do
- expect(Abuse::TrustScoreWorker).to receive(:perform_async).once.with(user.id, :telesign, low_risk_score.to_f)
+ context 'when the rename_abuse_workers feature is enabled' do
+ it 'executes the abuse trust score worker' do
+ expect(AntiAbuse::TrustScoreWorker).to receive(:perform_async).once.with(user.id, :telesign,
+ low_risk_score.to_f)
- service.execute
+ service.execute
+ end
+
+ context 'when the rename_abuse_workers feature is not enabled' do
+ before do
+ stub_feature_flags(rename_abuse_workers: false)
+ end
+
+ it 'executes the abuse trust score worker' do
+ expect(Abuse::TrustScoreWorker).to receive(:perform_async).once.with(user.id, :telesign, low_risk_score.to_f)
+
+ service.execute
+ end
+ end
end
context 'when the risk score is 0' do
@@ -48,10 +63,24 @@
expect { service.execute }.to change { phone_verification_record.risk_score }.from(0).to(1)
end
- it 'executes the abuse trust score worker with a risk score of 1.0' do
- expect(Abuse::TrustScoreWorker).to receive(:perform_async).once.with(user.id, :telesign, 1.0)
+ context 'when the rename_abuse_workers feature is enabled' do
+ it 'executes the abuse trust score worker with a risk score of 1.0' do
+ expect(AntiAbuse::TrustScoreWorker).to receive(:perform_async).once.with(user.id, :telesign, 1.0)
- service.execute
+ service.execute
+ end
+ end
+
+ context 'when the rename_abuse_workers feature is not enabled' do
+ before do
+ stub_feature_flags(rename_abuse_workers: false)
+ end
+
+ it 'executes the abuse trust score worker with a risk score of 1.0' do
+ expect(Abuse::TrustScoreWorker).to receive(:perform_async).once.with(user.id, :telesign, 1.0)
+
+ service.execute
+ end
end
end
diff --git a/ee/spec/services/phone_verification/users/send_verification_code_service_spec.rb b/ee/spec/services/phone_verification/users/send_verification_code_service_spec.rb
index f386bc1ab79f66290bffd529c92c0164888e5e6e..b752bbfbacc9c801abcff6d3ba54c82171a81cfc 100644
--- a/ee/spec/services/phone_verification/users/send_verification_code_service_spec.rb
+++ b/ee/spec/services/phone_verification/users/send_verification_code_service_spec.rb
@@ -422,12 +422,6 @@
expect(record.telesign_reference_xid).to eq(telesign_reference_xid)
end
- it 'executes the abuse trust score worker' do
- expect(Abuse::TrustScoreWorker).to receive(:perform_async).once.with(user.id, :telesign, instance_of(Float))
-
- service.execute
- end
-
it 'updates sms_send_count and sms_sent_at', :freeze_time do
service.execute
record = user.phone_number_validation
diff --git a/ee/spec/workers/anti_abuse/new_abuse_report_worker_spec.rb b/ee/spec/workers/anti_abuse/new_abuse_report_worker_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..626c8d2b49f550117289cb236560a81fb7532308
--- /dev/null
+++ b/ee/spec/workers/anti_abuse/new_abuse_report_worker_spec.rb
@@ -0,0 +1,153 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe AntiAbuse::NewAbuseReportWorker, :saas, feature_category: :instance_resiliency do
+ let_it_be(:user, reload: true) { create(:user) }
+ let_it_be(:reporter) { create(:user) }
+ let_it_be(:abuse_report) { create(:abuse_report, user: user, reporter: reporter) }
+ let_it_be(:job_args) { [abuse_report.id] }
+
+ shared_examples 'bans user' do
+ it 'bans the user' do
+ expect(user).not_to be_banned
+
+ allow(Gitlab::AppLogger).to receive(:info)
+ expect(Gitlab::AppLogger).to receive(:info).with(
+ message: "User ban",
+ user_id: user.id,
+ username: user.username,
+ abuse_report_id: abuse_report.id,
+ reason: "Automatic ban triggered by abuse report for #{abuse_report.category}."
+ )
+
+ worker.perform(*job_args)
+
+ expect(user).to be_banned
+ expect(
+ user.custom_attributes.by_key(UserCustomAttribute::AUTO_BANNED_BY_ABUSE_REPORT_ID).first.value
+ ).to eq(abuse_report.id.to_s)
+ end
+ end
+
+ shared_examples 'does not ban user' do
+ it 'does not ban the user' do
+ expect(user).not_to receive(:ban!)
+ expect(UserCustomAttribute).not_to receive(:upsert_custom_attributes)
+ worker.perform(*job_args)
+
+ expect(user).not_to be_banned
+ end
+ end
+
+ it_behaves_like 'an idempotent worker' do
+ subject(:worker) { described_class.new }
+
+ context 'when reporter is a gitlab employee' do
+ before do
+ allow(reporter).to receive(:gitlab_employee?).and_return(true)
+ allow(user).to receive(:human?).and_return(true)
+ allow(user).to receive(:active?).and_return(true)
+ allow(AbuseReport).to receive(:find_by_id).and_return(abuse_report)
+ end
+
+ context 'when the user is a member of a namespace with a paid plan trial subscription' do
+ let_it_be(:trial_group) do
+ create(:group_with_plan, plan: :ultimate_plan, trial_ends_on: 1.day.from_now, reporters: user)
+ end
+
+ it_behaves_like 'bans user'
+ end
+
+ context 'when the user is a member of a namespace with a paid plan subscription' do
+ let_it_be(:paid_group) { create(:group_with_plan, plan: :ultimate_plan, reporters: user) }
+
+ it_behaves_like 'does not ban user'
+ end
+
+ context 'when the user is on a free plan' do
+ context 'when the user is human' do
+ it_behaves_like 'bans user'
+ end
+
+ context 'when the user is not human' do
+ before do
+ allow(user).to receive(:human?).and_return(false)
+ end
+
+ it_behaves_like 'does not ban user'
+ end
+ end
+
+ context 'when the user is not active' do
+ before do
+ allow(user).to receive(:active?).and_return(false)
+ end
+
+ it_behaves_like 'does not ban user'
+ end
+
+ context 'when the user is not found' do
+ before do
+ allow(abuse_report).to receive(:user).and_return(nil)
+ end
+
+ it 'does not start a transaction' do
+ expect(described_class).not_to receive(:bannable_user?)
+
+ worker.perform(*job_args)
+ end
+ end
+
+ context 'when the user is a gitlab employee' do
+ before do
+ allow(user).to receive(:gitlab_employee?).and_return(true)
+ end
+
+ it_behaves_like 'does not ban user'
+ end
+
+ context 'when the user age is greater than 7 days' do
+ before do
+ user.update_attribute(:created_at, 8.days.ago)
+ end
+
+ it_behaves_like 'does not ban user'
+ end
+
+ context 'when user owns namespaces with more than five members' do
+ let_it_be(:group) { create(:group, owners: user, guests: create_list(:user, 5)) }
+
+ it_behaves_like 'does not ban user'
+ end
+ end
+
+ context 'when reporter is not a gitlab employee' do
+ it_behaves_like 'does not ban user'
+ end
+
+ context 'when the abuse report is not found' do
+ before do
+ allow(AbuseReport).to receive(:find_by_id).and_return(nil)
+ end
+
+ it 'returns early' do
+ expect(abuse_report).not_to receive(:reporter)
+
+ worker.perform(*job_args)
+ end
+ end
+
+ context 'when there is an error executing ban' do
+ before do
+ allow(user).to receive(:ban!).and_raise(StateMachines::InvalidTransition)
+ end
+
+ it 'does not emit an application log' do
+ expect(Gitlab::AppLogger).not_to receive(:info)
+
+ worker.perform(*job_args)
+ end
+ end
+ end
+end
diff --git a/spec/services/spam/spam_action_service_spec.rb b/spec/services/spam/spam_action_service_spec.rb
index 315d4714516e03934b6c7660df693a2442f2c8cc..151b3ed43fa93dd2abfe3179e7fcb431bd69b37f 100644
--- a/spec/services/spam/spam_action_service_spec.rb
+++ b/spec/services/spam/spam_action_service_spec.rb
@@ -98,8 +98,45 @@
}
end
- it do
- expect(::Abuse::SpamAbuseEventsWorker).to receive(:perform_async).with(params)
+ context 'when the rename_abuse_workers feature is enabled' do
+ it 'executes the ::AntiAbuse::SpamAbuseEventsWorker' do
+ expect(::AntiAbuse::SpamAbuseEventsWorker).to receive(:perform_async).with(params)
+
+ subject
+ end
+ end
+
+ context 'when the rename_abuse_workers feature is not enabled' do
+ before do
+ stub_feature_flags(rename_abuse_workers: false)
+ end
+
+ it 'executes the ::Abuse::SpamAbuseEventsWorker' do
+ expect(::Abuse::SpamAbuseEventsWorker).to receive(:perform_async).with(params)
+
+ subject
+ end
+ end
+ end
+
+ shared_examples 'does not execute the SpamAbuseEventsWorker' do
+ specify do
+ expect(::Abuse::SpamAbuseEventsWorker).not_to receive(:perform_async)
+ expect(::AntiAbuse::SpamAbuseEventsWorker).not_to receive(:perform_async)
+
+ subject
+ end
+ end
+
+ shared_examples 'allows the spammable' do
+ it 'does not create a spam log' do
+ expect { subject }.not_to change(SpamLog, :count)
+ end
+
+ it_behaves_like 'does not execute the SpamAbuseEventsWorker'
+
+ it 'clears spam flags' do
+ expect(target).to receive(:clear_spam_flags!)
subject
end
@@ -182,11 +219,7 @@
expect { subject }.not_to change(SpamLog, :count)
end
- it 'does not call SpamAbuseEventsWorker' do
- expect(::Abuse::SpamAbuseEventsWorker).not_to receive(:perform_async)
-
- subject
- end
+ it_behaves_like 'does not execute the SpamAbuseEventsWorker'
end
context 'when spammable attributes have changed' do
@@ -317,12 +350,7 @@
end
it_behaves_like 'creates a spam log', target_type
-
- it 'does not call SpamAbuseEventsWorker' do
- expect(::Abuse::SpamAbuseEventsWorker).not_to receive(:perform_async)
-
- subject
- end
+ it_behaves_like 'does not execute the SpamAbuseEventsWorker'
it 'does not mark as spam' do
response = subject
@@ -344,21 +372,7 @@
allow(fake_verdict_service).to receive(:execute).and_return(ALLOW)
end
- it 'does not create a spam log' do
- expect { subject }.not_to change(SpamLog, :count)
- end
-
- it 'does not call SpamAbuseEventsWorker' do
- expect(::Abuse::SpamAbuseEventsWorker).not_to receive(:perform_async)
-
- subject
- end
-
- it 'clears spam flags' do
- expect(target).to receive(:clear_spam_flags!)
-
- subject
- end
+ it_behaves_like 'allows the spammable'
end
context 'when spam verdict service returns noop' do
@@ -366,21 +380,7 @@
allow(fake_verdict_service).to receive(:execute).and_return(NOOP)
end
- it 'does not create a spam log' do
- expect { subject }.not_to change(SpamLog, :count)
- end
-
- it 'does not call SpamAbuseEventsWorker' do
- expect(::Abuse::SpamAbuseEventsWorker).not_to receive(:perform_async)
-
- subject
- end
-
- it 'clears spam flags' do
- expect(target).to receive(:clear_spam_flags!)
-
- subject
- end
+ it_behaves_like 'allows the spammable'
end
context 'with spam verdict service options' do
diff --git a/spec/services/spam/spam_verdict_service_spec.rb b/spec/services/spam/spam_verdict_service_spec.rb
index 8669bca90bd8b462686fcbb752b4ead67f461344..f07b3d0d27f1caac712b1ccf562e39ca2ce51f6c 100644
--- a/spec/services/spam/spam_verdict_service_spec.rb
+++ b/spec/services/spam/spam_verdict_service_spec.rb
@@ -275,6 +275,7 @@
it 'returns the verdict' do
expect(Abuse::TrustScoreWorker).not_to receive(:perform_async)
+ expect(AntiAbuse::TrustScoreWorker).not_to receive(:perform_async)
is_expected.to eq(NOOP)
end
end
@@ -284,9 +285,22 @@
let(:verdict_value) { ::Spamcheck::SpamVerdict::Verdict::ALLOW }
context 'the result was evaluated' do
- it 'returns the verdict and updates the spam score' do
- expect(Abuse::TrustScoreWorker).to receive(:perform_async).once.with(user.id, :spamcheck, instance_of(Float), 'cid')
- is_expected.to eq(ALLOW)
+ context 'when the rename_abuse_workers feature is enabled' do
+ it 'returns the verdict and updates the spam score' do
+ expect(AntiAbuse::TrustScoreWorker).to receive(:perform_async).once.with(user.id, :spamcheck, instance_of(Float), 'cid')
+ is_expected.to eq(ALLOW)
+ end
+ end
+
+ context 'when the rename_abuse_workers feature is not enabled' do
+ before do
+ stub_feature_flags(rename_abuse_workers: false)
+ end
+
+ it 'returns the verdict and updates the spam score' do
+ expect(Abuse::TrustScoreWorker).to receive(:perform_async).once.with(user.id, :spamcheck, instance_of(Float), 'cid')
+ is_expected.to eq(ALLOW)
+ end
end
end
@@ -295,6 +309,7 @@
it 'returns the verdict and does not update the spam score' do
expect(Abuse::TrustScoreWorker).not_to receive(:perform_async)
+ expect(AntiAbuse::TrustScoreWorker).not_to receive(:perform_async)
expect(subject).to eq(ALLOW)
end
end
@@ -315,9 +330,22 @@
end
with_them do
- it "returns expected spam constant and updates the spam score" do
- expect(Abuse::TrustScoreWorker).to receive(:perform_async).once.with(user.id, :spamcheck, instance_of(Float), 'cid')
- is_expected.to eq(expected)
+ context 'when the rename_abuse_workers feature is enabled' do
+ it "returns expected spam constant and updates the spam score" do
+ expect(AntiAbuse::TrustScoreWorker).to receive(:perform_async).once.with(user.id, :spamcheck, instance_of(Float), 'cid')
+ is_expected.to eq(expected)
+ end
+ end
+
+ context 'when the rename_abuse_workers feature is not enabled' do
+ before do
+ stub_feature_flags(rename_abuse_workers: false)
+ end
+
+ it "returns expected spam constant and updates the spam score" do
+ expect(Abuse::TrustScoreWorker).to receive(:perform_async).once.with(user.id, :spamcheck, instance_of(Float), 'cid')
+ is_expected.to eq(expected)
+ end
end
end
end
diff --git a/spec/workers/anti_abuse/spam_abuse_events_worker_spec.rb b/spec/workers/anti_abuse/spam_abuse_events_worker_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..adaa1b88656c9f49d23401ca984cb1d55ee2c414
--- /dev/null
+++ b/spec/workers/anti_abuse/spam_abuse_events_worker_spec.rb
@@ -0,0 +1,85 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe AntiAbuse::SpamAbuseEventsWorker, :clean_gitlab_redis_shared_state, feature_category: :instance_resiliency do
+ let(:worker) { described_class.new }
+ let_it_be(:user) { create(:user) }
+
+ let(:params) do
+ {
+ user_id: user.id,
+ title: 'Test title',
+ description: 'Test description',
+ source_ip: '1.2.3.4',
+ user_agent: 'fake-user-agent',
+ noteable_type: 'Issue',
+ verdict: 'BLOCK_USER'
+ }
+ end
+
+ shared_examples 'creates an abuse event with the correct data' do
+ it do
+ expect { worker.perform(params) }.to change { AntiAbuse::Event.count }.from(0).to(1)
+ expect(AntiAbuse::Event.last.attributes).to include({
+ abuse_report_id: report_id,
+ category: "spam",
+ metadata: params.except(:user_id),
+ source: "spamcheck",
+ user_id: params[:user_id]
+ }.deep_stringify_keys)
+ end
+ end
+
+ it_behaves_like 'an idempotent worker' do
+ let(:job_args) { [params] }
+ end
+
+ context "when the user does not exist" do
+ let(:log_payload) { { 'message' => 'User not found.', 'user_id' => user.id } }
+
+ before do
+ allow(User).to receive(:find_by_id).with(user.id).and_return(nil)
+ end
+
+ it 'logs an error' do
+ expect(Sidekiq.logger).to receive(:info).with(hash_including(log_payload))
+
+ expect { worker.perform(params) }.not_to raise_exception
+ end
+
+ it 'does not report the user' do
+ expect(described_class).not_to receive(:report_user).with(user.id)
+
+ worker.perform(params)
+ end
+ end
+
+ context "when the user exists" do
+ context 'and there is an existing abuse report' do
+ let_it_be(:abuse_report) do
+ create(:abuse_report, user: user, reporter: Users::Internal.security_bot, message: 'Test report')
+ end
+
+ it_behaves_like 'creates an abuse event with the correct data' do
+ let(:report_id) { abuse_report.id }
+ end
+ end
+
+ context 'and there is no existing abuse report' do
+ it 'creates an abuse report with the correct data' do
+ expect { worker.perform(params) }.to change { AbuseReport.count }.from(0).to(1)
+ expect(AbuseReport.last.attributes).to include({
+ reporter_id: Users::Internal.security_bot.id,
+ user_id: user.id,
+ category: "spam",
+ message: "User reported for abuse based on spam verdict"
+ }.stringify_keys)
+ end
+
+ it_behaves_like 'creates an abuse event with the correct data' do
+ let(:report_id) { AbuseReport.last.attributes["id"] }
+ end
+ end
+ end
+end
diff --git a/spec/workers/anti_abuse/trust_score_worker_spec.rb b/spec/workers/anti_abuse/trust_score_worker_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..d0cb6618d21fb6d1f4f3b4dd7017ad77bd1a914f
--- /dev/null
+++ b/spec/workers/anti_abuse/trust_score_worker_spec.rb
@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe AntiAbuse::TrustScoreWorker, :clean_gitlab_redis_shared_state, feature_category: :instance_resiliency do
+ let(:worker) { described_class.new }
+ let_it_be(:user) { create(:user) }
+
+ subject(:perform) { worker.perform(user.id, :telesign, 0.85, 'foo') }
+
+ it_behaves_like 'an idempotent worker' do
+ let(:job_args) { [user.id, :telesign, 0.5] }
+ end
+
+ context "when the user does not exist" do
+ let(:log_payload) { { 'message' => 'User not found.', 'user_id' => user.id } }
+
+ before do
+ allow(User).to receive(:find_by_id).with(user.id).and_return(nil)
+ end
+
+ it 'logs an error' do
+ expect(Sidekiq.logger).to receive(:info).with(hash_including(log_payload))
+
+ expect { perform }.not_to raise_exception
+ end
+
+ it 'does not attempt to create the trust score' do
+ expect(AntiAbuse::TrustScore).not_to receive(:create!)
+
+ perform
+ end
+ end
+
+ context "when the user exists" do
+ it 'creates an abuse trust score with the correct data' do
+ expect { perform }.to change { AntiAbuse::TrustScore.count }.from(0).to(1)
+ expect(AntiAbuse::TrustScore.last.attributes).to include({
+ user_id: user.id,
+ source: "telesign",
+ score: 0.85,
+ correlation_id_value: 'foo'
+ }.stringify_keys)
+ end
+ end
+end