diff options
Diffstat (limited to 'app/services/spam/spam_action_service.rb')
| -rw-r--r-- | app/services/spam/spam_action_service.rb | 93 |
1 files changed, 72 insertions, 21 deletions
diff --git a/app/services/spam/spam_action_service.rb b/app/services/spam/spam_action_service.rb index b3d617256ff..ff32bc32d93 100644 --- a/app/services/spam/spam_action_service.rb +++ b/app/services/spam/spam_action_service.rb @@ -4,37 +4,69 @@ module Spam class SpamActionService include SpamConstants + ## + # Utility method to filter SpamParams from parameters, which will later be passed to #execute + # after the spammable is created/updated based on the remaining parameters. + # + # Takes a hash of parameters from an incoming request to modify a model (via a controller, + # service, or GraphQL mutation). + # + # Deletes the parameters which are related to spam and captcha processing, and returns + # them in a SpamParams parameters object. See: + # https://refactoring.com/catalog/introduceParameterObject.html + def self.filter_spam_params!(params) + # NOTE: The 'captcha_response' field can be expanded to multiple fields when we move to future + # alternative captcha implementations such as FriendlyCaptcha. See + # https://gitlab.com/gitlab-org/gitlab/-/issues/273480 + captcha_response = params.delete(:captcha_response) + + SpamParams.new( + api: params.delete(:api), + captcha_response: captcha_response, + spam_log_id: params.delete(:spam_log_id) + ) + end + attr_accessor :target, :request, :options attr_reader :spam_log - def initialize(spammable:, request:, user:, context: {}) + def initialize(spammable:, request:, user:, action:) @target = spammable @request = request @user = user - @context = context + @action = action @options = {} + end - if @request - @options[:ip_address] = @request.env['action_dispatch.remote_ip'].to_s - @options[:user_agent] = @request.env['HTTP_USER_AGENT'] - @options[:referrer] = @request.env['HTTP_REFERRER'] + def execute(spam_params:) + if request + options[:ip_address] = request.env['action_dispatch.remote_ip'].to_s + options[:user_agent] = request.env['HTTP_USER_AGENT'] + options[:referrer] = request.env['HTTP_REFERRER'] else - @options[:ip_address] = @target.ip_address - @options[:user_agent] = @target.user_agent + # TODO: This code is never used, because we do not perform a verification if there is not a + # request. Why? Should it be deleted? Or should we check even if there is no request? + options[:ip_address] = target.ip_address + options[:user_agent] = target.user_agent end - end - def execute(api: false, recaptcha_verified:, spam_log_id:) + recaptcha_verified = Captcha::CaptchaVerificationService.new.execute( + captcha_response: spam_params.captcha_response, + request: request + ) + if recaptcha_verified - # If it's a request which is already verified through reCAPTCHA, + # If it's a request which is already verified through captcha, # update the spam log accordingly. - SpamLog.verify_recaptcha!(user_id: user.id, id: spam_log_id) + SpamLog.verify_recaptcha!(user_id: user.id, id: spam_params.spam_log_id) + ServiceResponse.success(message: "Captcha was successfully verified") else - return if allowlisted?(user) - return unless request - return unless check_for_spam? + return ServiceResponse.success(message: 'Skipped spam check because user was allowlisted') if allowlisted?(user) + return ServiceResponse.success(message: 'Skipped spam check because request was not present') unless request + return ServiceResponse.success(message: 'Skipped spam check because it was not required') unless check_for_spam? - perform_spam_service_check(api) + perform_spam_service_check(spam_params.api) + ServiceResponse.success(message: "Spam check performed, check #{target.class.name} spammable model for any errors or captcha requirement") end end @@ -42,13 +74,27 @@ module Spam private - attr_reader :user, :context + attr_reader :user, :action + + ## + # In order to be proceed to the spam check process, the target must be + # a dirty instance, which means it should be already assigned with the new + # attribute values. + def ensure_target_is_dirty + msg = "Target instance of #{target.class.name} must be dirty (must have changes to save)" + raise(msg) unless target.has_changes_to_save? + end def allowlisted?(user) user.try(:gitlab_employee?) || user.try(:gitlab_bot?) || user.try(:gitlab_service_user?) end + ## + # Performs the spam check using the spam verdict service, and modifies the target model + # accordingly based on the result. def perform_spam_service_check(api) + ensure_target_is_dirty + # since we can check for spam, and recaptcha is not verified, # ask the SpamVerdictService what to do with the target. spam_verdict_service.execute.tap do |result| @@ -79,7 +125,7 @@ module Spam description: target.spam_description, source_ip: options[:ip_address], user_agent: options[:user_agent], - noteable_type: notable_type, + noteable_type: noteable_type, via_api: api } ) @@ -88,14 +134,19 @@ module Spam end def spam_verdict_service + context = { + action: action, + target_type: noteable_type + } + SpamVerdictService.new(target: target, user: user, - request: @request, + request: request, options: options, - context: context.merge(target_type: notable_type)) + context: context) end - def notable_type + def noteable_type @notable_type ||= target.class.to_s end end |