Merge branch 'cablett-spam-endpoint-params' into 'master'

Send params to external spam endpoint

See merge request gitlab-org/gitlab!33240

app/services/concerns/spam_check_methods.rb

@@ -22,15 +22,18 @@ module SpamCheckMethods
   # a dirty instance, which means it should be already assigned with the new
   # attribute values.
   # rubocop:disable Gitlab/ModuleWithInstanceVariables
-  def spam_check(spammable, user)
+  def spam_check(spammable, user, action:)
+    raise ArgumentError.new('Please provide an action, such as :create') unless action
+
     Spam::SpamActionService.new(
       spammable: spammable,
-      request: @request
+      request: @request,
+      user: user,
+      context: { action: action }
     ).execute(
       api: @api,
       recaptcha_verified: @recaptcha_verified,
-      spam_log_id: @spam_log_id,
-      user: user)
+      spam_log_id: @spam_log_id)
   end
   # rubocop:enable Gitlab/ModuleWithInstanceVariables
 end

app/services/issues/create_service.rb

@@ -15,7 +15,7 @@ module Issues
     end
 
     def before_create(issue)
-      spam_check(issue, current_user)
+      spam_check(issue, current_user, action: :create)
       issue.move_to_end
 
       # current_user (defined in BaseService) is not available within run_after_commit block

app/services/issues/update_service.rb

@@ -18,7 +18,7 @@ module Issues
     end
 
     def before_update(issue, skip_spam_check: false)
-      spam_check(issue, current_user) unless skip_spam_check
+      spam_check(issue, current_user, action: :update) unless skip_spam_check
     end
 
     def after_update(issue)

app/services/snippets/create_service.rb

@@ -13,7 +13,7 @@ module Snippets
 
       @snippet.author = current_user
 
-      spam_check(@snippet, current_user)
+      spam_check(@snippet, current_user, action: :create)
 
       if save_and_commit
         UserAgentDetailService.new(@snippet, @request).create

app/services/snippets/update_service.rb

@@ -14,7 +14,7 @@ module Snippets
       end
 
       update_snippet_attributes(snippet)
-      spam_check(snippet, current_user)
+      spam_check(snippet, current_user, action: :update)
 
       if save_and_commit(snippet)
         Gitlab::UsageDataCounters::SnippetCounter.count(:update)

app/services/spam/spam_action_service.rb

@@ -7,9 +7,11 @@ module Spam
     attr_accessor :target, :request, :options
     attr_reader :spam_log
 
-    def initialize(spammable:, request:)
+    def initialize(spammable:, request:, user:, context: {})
       @target = spammable
       @request = request
+      @user = user
+      @context = context
       @options = {}
 
       if @request
@@ -22,7 +24,7 @@ module Spam
       end
     end
 
-    def execute(api: false, recaptcha_verified:, spam_log_id:, user:)
+    def execute(api: false, recaptcha_verified:, spam_log_id:)
       if recaptcha_verified
         # If it's a request which is already verified through reCAPTCHA,
         # update the spam log accordingly.
@@ -40,6 +42,8 @@ module Spam
 
     private
 
+    attr_reader :user, :context
+
     def allowlisted?(user)
       user.respond_to?(:gitlab_employee) && user.gitlab_employee?
     end
@@ -75,7 +79,7 @@ module Spam
           description: target.spam_description,
           source_ip: options[:ip_address],
           user_agent: options[:user_agent],
-          noteable_type: target.class.to_s,
+          noteable_type: notable_type,
           via_api: api
         }
       )
@@ -85,8 +89,14 @@ module Spam
 
     def spam_verdict_service
       SpamVerdictService.new(target: target,
+                             user: user,
                              request: @request,
-                             options: options)
+                             options: options,
+                             context: context.merge(target_type: notable_type))
+    end
+
+    def notable_type
+      @notable_type ||= target.class.to_s
     end
   end
 end

app/services/spam/spam_verdict_service.rb

@@ -5,11 +5,12 @@ module Spam
     include AkismetMethods
     include SpamConstants
 
-    def initialize(target:, request:, options:, verdict_params: {})
+    def initialize(user:, target:, request:, options:, context: {})
       @target = target
       @request = request
+      @user = user
       @options = options
-      @verdict_params = assemble_verdict_params(verdict_params)
+      @verdict_params = assemble_verdict_params(context)
     end
 
     def execute
@@ -27,7 +28,7 @@ module Spam
 
     private
 
-    attr_reader :target, :request, :options, :verdict_params
+    attr_reader :user, :target, :request, :options, :verdict_params
 
     def akismet_verdict
       if akismet.spam?
@@ -66,11 +67,23 @@ module Spam
       end
     end
 
-    def assemble_verdict_params(params)
-      return {} unless endpoint_url
-
-      params.merge({
-        user_id: target.author_id
+    def assemble_verdict_params(context)
+      return {} unless endpoint_url.present?
+
+      project = target.try(:project)
+
+      context.merge({
+        target: {
+          title: target.spam_title,
+          description: target.spam_description,
+          type: target.class.to_s
+        },
+        user: {
+          created_at: user.created_at,
+          email: user.email,
+          username: user.username
+        },
+        user_in_project: user.authorized_project?(project)
       })
     end
 

spec/services/spam/spam_action_service_spec.rb

@@ -24,7 +24,7 @@ describe Spam::SpamActionService do
   end
 
   describe '#initialize' do
-    subject { described_class.new(spammable: issue, request: request) }
+    subject { described_class.new(spammable: issue, request: request, user: user) }
 
     context 'when the request is nil' do
       let(:request) { nil }
@@ -53,7 +53,7 @@ describe Spam::SpamActionService do
 
   shared_examples 'only checks for spam if a request is provided' do
     context 'when request is missing' do
-      subject { described_class.new(spammable: issue, request: nil) }
+      subject { described_class.new(spammable: issue, request: nil, user: user) }
 
       it "doesn't check as spam" do
         subject
@@ -78,9 +78,9 @@ describe Spam::SpamActionService do
     let_it_be(:existing_spam_log) { create(:spam_log, user: user, recaptcha_verified: false) }
 
     subject do
-      described_service = described_class.new(spammable: issue, request: request)
+      described_service = described_class.new(spammable: issue, request: request, user: user)
       allow(described_service).to receive(:allowlisted?).and_return(allowlisted)
-      described_service.execute(user: user, api: nil, recaptcha_verified: recaptcha_verified, spam_log_id: existing_spam_log.id)
+      described_service.execute(api: nil, recaptcha_verified: recaptcha_verified, spam_log_id: existing_spam_log.id)
     end
 
     before do

spec/services/spam/spam_verdict_service_spec.rb

@@ -16,9 +16,10 @@ describe Spam::SpamVerdictService do
   let(:request) { double(:request, env: env) }
 
   let(:check_for_spam) { true }
-  let(:issue) { build(:issue) }
+  let_it_be(:user) { create(:user) }
+  let(:issue) { build(:issue, author: user) }
   let(:service) do
-    described_class.new(target: issue, request: request, options: {})
+    described_class.new(user: user, target: issue, request: request, options: {})
   end
 
   describe '#execute' do