Use reCaptcha when an issue identified as spam

11 files changed, 86 insertions, 14 deletions

diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss
index 8734a3b1598..1e605337f09 100644
--- a/app/assets/stylesheets/pages/issues.scss
+++ b/app/assets/stylesheets/pages/issues.scss
@@ -148,3 +148,7 @@ ul.related-merge-requests > li {
     border: 1px solid $border-gray-normal;
   }
 }
+
+.recaptcha {
+  margin-bottom: 30px;
+}
diff --git a/app/controllers/concerns/spammable_actions.rb b/app/controllers/concerns/spammable_actions.rb
index 562f92bd83c..a6891149bfa 100644
--- a/app/controllers/concerns/spammable_actions.rb
+++ b/app/controllers/concerns/spammable_actions.rb
@@ -1,6 +1,8 @@
 module SpammableActions
   extend ActiveSupport::Concern
 
+  include Recaptcha::Verify
+
   included do
     before_action :authorize_submit_spammable!, only: :mark_as_spam
   end
@@ -15,6 +17,15 @@ module SpammableActions
 
   private
 
+  def recaptcha_params
+    return {} unless params[:recaptcha_verification] && Gitlab::Recaptcha.load_configurations! && verify_recaptcha
+
+    {
+      recaptcha_verified: true,
+      spam_log_id: params[:spam_log_id]
+    }
+  end
+
   def spammable
     raise NotImplementedError, "#{self.class} does not implement #{__method__}"
   end
@@ -22,4 +33,11 @@ module SpammableActions
   def authorize_submit_spammable!
     access_denied! unless current_user.admin?
   end
+
+  def render_recaptcha?
+    return false if spammable.errors.count > 1 # re-render "new" template in case there are other errors
+    return false unless Gitlab::Recaptcha.enabled?
+
+    spammable.spam
+  end
 end
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 8472ceca329..c75b8987a4b 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -93,15 +93,13 @@ class Projects::IssuesController < Projects::ApplicationController
   def create
     extra_params = { request: request,
                      merge_request_for_resolving_discussions: merge_request_for_resolving_discussions }
+    extra_params.merge!(recaptcha_params)
+
     @issue = Issues::CreateService.new(project, current_user, issue_params.merge(extra_params)).execute
 
     respond_to do |format|
       format.html do
-        if @issue.valid?
-          redirect_to issue_path(@issue)
-        else
-          render :new
-        end
+        html_response_create
       end
       format.js do
         @link = @issue.attachment.url.to_js
@@ -178,6 +176,20 @@ class Projects::IssuesController < Projects::ApplicationController
 
   protected
 
+  def html_response_create
+    if @issue.valid?
+      redirect_to issue_path(@issue)
+    elsif render_recaptcha?
+      if params[:recaptcha_verification]
+        flash[:alert] = 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'
+      end
+
+      render :verify
+    else
+      render :new
+    end
+  end
+
   def issue
     # The Sortable default scope causes performance issues when used with find_by
     @noteable = @issue ||= @project.issues.where(iid: params[:id]).reorder(nil).take || redirect_old
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index bf27f3d4d51..68bf01ba08d 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -17,7 +17,7 @@ class RegistrationsController < Devise::RegistrationsController
     if !Gitlab::Recaptcha.load_configurations! || verify_recaptcha
       super
     else
-      flash[:alert] = 'There was an error with the reCAPTCHA. Please re-solve the reCAPTCHA.'
+      flash[:alert] = 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'
       flash.delete :recaptcha_error
       render action: 'new'
     end
@@ -30,7 +30,7 @@ class RegistrationsController < Devise::RegistrationsController
       format.html do
         session.try(:destroy)
         redirect_to new_user_session_path, notice: "Account successfully removed."
-      end 
+      end
     end
   end
 
diff --git a/app/models/concerns/spammable.rb b/app/models/concerns/spammable.rb
index 1acff093aa1..423ae98a60e 100644
--- a/app/models/concerns/spammable.rb
+++ b/app/models/concerns/spammable.rb
@@ -11,6 +11,7 @@ module Spammable
     has_one :user_agent_detail, as: :subject, dependent: :destroy
 
     attr_accessor :spam
+    attr_accessor :spam_log
 
     after_validation :check_for_spam, on: :create
 
@@ -34,9 +35,14 @@ module Spammable
   end
 
   def check_for_spam
-    if spam?
-      self.errors.add(:base, "Your #{spammable_entity_type} has been recognized as spam and has been discarded.")
-    end
+    error_msg = if Gitlab::Recaptcha.enabled?
+                  "Your #{spammable_entity_type} has been recognized as spam. "\
+                  "You can still submit it by solving Captcha."
+                else
+                  "Your #{spammable_entity_type} has been recognized as spam and has been discarded."
+                end
+
+    self.errors.add(:base, error_msg) if spam?
   end
 
   def spammable_entity_type
diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb
index d2eb46ac41b..c9168f74249 100644
--- a/app/services/issues/create_service.rb
+++ b/app/services/issues/create_service.rb
@@ -3,6 +3,8 @@ module Issues
     def execute
       @request = params.delete(:request)
       @api = params.delete(:api)
+      @recaptcha_verified = params.delete(:recaptcha_verified)
+      @spam_log_id = params.delete(:spam_log_id)
 
       issue_attributes = params.merge(merge_request_for_resolving_discussions: merge_request_for_resolving_discussions)
       @issue = BuildService.new(project, current_user, issue_attributes).execute
@@ -11,7 +13,13 @@ module Issues
     end
 
     def before_create(issuable)
-      issuable.spam = spam_service.check(@api)
+      if @recaptcha_verified
+        spam_log = current_user.spam_logs.find_by(id: @spam_log_id, title: issuable.title)
+        spam_log.update!(recaptcha_verified: true) if spam_log
+      else
+        issuable.spam = spam_service.check(@api)
+        issuable.spam_log = spam_service.spam_log
+      end
     end
 
     def after_create(issuable)
@@ -35,7 +43,7 @@ module Issues
     private
 
     def spam_service
-      SpamService.new(@issue, @request)
+      @spam_service ||= SpamService.new(@issue, @request)
     end
 
     def user_agent_detail_service
diff --git a/app/services/spam_service.rb b/app/services/spam_service.rb
index 48903291799..024a7c19d33 100644
--- a/app/services/spam_service.rb
+++ b/app/services/spam_service.rb
@@ -1,5 +1,6 @@
 class SpamService
   attr_accessor :spammable, :request, :options
+  attr_reader :spam_log
 
   def initialize(spammable, request = nil)
     @spammable = spammable
@@ -63,7 +64,7 @@ class SpamService
   end
 
   def create_spam_log(api)
-    SpamLog.create(
+    @spam_log = SpamLog.create!(
       {
         user_id: spammable_owner_id,
         title: spammable.spam_title,
diff --git a/app/views/admin/spam_logs/_spam_log.html.haml b/app/views/admin/spam_logs/_spam_log.html.haml
index 4ce4eab8753..33f6d847782 100644
--- a/app/views/admin/spam_logs/_spam_log.html.haml
+++ b/app/views/admin/spam_logs/_spam_log.html.haml
@@ -14,6 +14,8 @@
   %td
     = spam_log.via_api? ? 'Y' : 'N'
   %td
+    = spam_log.recaptcha_verified ? 'Y' : 'N'
+  %td
     = spam_log.noteable_type
   %td
     = spam_log.title
diff --git a/app/views/admin/spam_logs/index.html.haml b/app/views/admin/spam_logs/index.html.haml
index 0fdd5bd9960..8aaa6379730 100644
--- a/app/views/admin/spam_logs/index.html.haml
+++ b/app/views/admin/spam_logs/index.html.haml
@@ -10,6 +10,7 @@
           %th User
           %th Source IP
           %th API?
+          %th Recaptcha verified?
           %th Type
           %th Title
           %th Description
diff --git a/app/views/devise/shared/_signup_box.html.haml b/app/views/devise/shared/_signup_box.html.haml
index 01ecf237925..5a44ec45b7b 100644
--- a/app/views/devise/shared/_signup_box.html.haml
+++ b/app/views/devise/shared/_signup_box.html.haml
@@ -23,7 +23,7 @@
         = f.password_field :password, class: "form-control bottom", required: true, pattern: ".{#{@minimum_password_length},}", title: "Minimum length is #{@minimum_password_length} characters."
         %p.gl-field-hint Minimum length is #{@minimum_password_length} characters
       %div
-      - if current_application_settings.recaptcha_enabled
+      - if Gitlab::Recaptcha.enabled?
         = recaptcha_tags
       %div
         = f.submit "Register", class: "btn-register btn"
diff --git a/app/views/projects/issues/verify.html.haml b/app/views/projects/issues/verify.html.haml
new file mode 100644
index 00000000000..1934b18c086
--- /dev/null
+++ b/app/views/projects/issues/verify.html.haml
@@ -0,0 +1,20 @@
+- page_title "Anti-spam verification"
+
+%h3.page-title
+  Anti-spam verification
+%hr
+
+%p
+  We detected potential spam in the issue description. Please verify that you are not a robot to submit the issue.
+
+= form_for [@project.namespace.becomes(Namespace), @project, @issue] do |f|
+  .recaptcha
+    - params[:issue].each do |field, value|
+      = hidden_field(:issue, field, value: value)
+    = hidden_field_tag(:merge_request_for_resolving_discussions, params[:merge_request_for_resolving_discussions])
+    = hidden_field_tag(:spam_log_id, @issue.spam_log.id)
+    = hidden_field_tag(:recaptcha_verification, true)
+    = recaptcha_tags
+
+  .row-content-block.footer-block
+    = f.submit "Submit #{@issue.class.model_name.human.downcase}", class: 'btn btn-create'