Submit all issues on public projects to Akismet if enabled.

9 files changed, 54 insertions, 36 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 2efacc9e05b..27e68431488 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -94,6 +94,7 @@ v 8.10.0
   - Updated compare dropdown menus to use GL dropdown
   - Redirects back to issue after clicking login link
   - Submit issues created via the WebUI by non project members to Akismet !5333
+  - All created issues, API or WebUI, can be submitted to Akismet for spam check !5333
   - Eager load award emoji on notes
   - Allow to define manual actions/builds on Pipelines and Environments
   - Fix pagination when sorting by columns with lots of ties (like priority)
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 10de472aad5..b527dd0f4f2 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -2,7 +2,6 @@ class Projects::IssuesController < Projects::ApplicationController
   include ToggleSubscriptionAction
   include IssuableActions
   include ToggleAwardEmoji
-  include Gitlab::AkismetHelper
 
   before_action :module_enabled
   before_action :issue, only: [:edit, :update, :show, :referenced_merge_requests,
@@ -80,23 +79,14 @@ class Projects::IssuesController < Projects::ApplicationController
   end
 
   def create
-    text = [params[:issue][:title], params[:issue][:description]].reject(&:blank?).join("\n")
-
-    if check_for_spam?(project, current_user) && is_spam?(request.env, current_user, text)
-      attrs = {
-        user_id: current_user.id,
-        project_id: project.id,
-        title: params[:issue][:title],
-        description: params[:issue][:description]
-      }
-      create_spam_log(project, current_user, attrs, request.env, api: false)
+    @issue = Issues::CreateService.new(project, current_user, issue_params.merge({ request: request })).execute
+
+    if @issue.nil?
       @issue = @project.issues.new
       flash[:notice] = 'Your issue has been recognized as spam and has been discarded.'
       render :new and return
     end
 
-    @issue = Issues::CreateService.new(project, current_user, issue_params).execute
-
     respond_to do |format|
       format.html do
         if @issue.valid?
diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb
index e63e1af8766..496ea5a86a2 100644
--- a/app/services/issues/create_service.rb
+++ b/app/services/issues/create_service.rb
@@ -3,9 +3,13 @@ module Issues
     def execute
       filter_params
       label_params = params[:label_ids]
-      issue = project.issues.new(params.except(:label_ids))
+      issue = project.issues.new(params.except(:label_ids, :request))
       issue.author = params[:author] || current_user
 
+      if Issues::SpamCheckService.new(project, current_user, params).spam_detected?
+        return nil
+      end
+
       if issue.save
         issue.update_attributes(label_ids: label_params)
         notification_service.new_issue(issue, current_user)
diff --git a/app/services/issues/spam_check_service.rb b/app/services/issues/spam_check_service.rb
new file mode 100644
index 00000000000..b8d4e37faf5
--- /dev/null
+++ b/app/services/issues/spam_check_service.rb
@@ -0,0 +1,25 @@
+module Issues
+  class SpamCheckService < BaseService
+    include Gitlab::AkismetHelper
+
+    def spam_detected?
+      text = [params[:title], params[:description]].reject(&:blank?).join("\n")
+      request = params[:request]
+
+      if request
+        if check_for_spam?(project) && is_spam?(request.env, current_user, text)
+          attrs = {
+            user_id: current_user.id,
+            project_id: project.id,
+            title: params[:title],
+            description: params[:description]
+          }
+          create_spam_log(project, current_user, attrs, request.env, api: false)
+          return true
+        end
+      end
+
+      false
+    end
+  end
+end
diff --git a/doc/integration/akismet.md b/doc/integration/akismet.md
index 52c1b0a75d8..99a28b493c9 100644
--- a/doc/integration/akismet.md
+++ b/doc/integration/akismet.md
@@ -1,12 +1,14 @@
 # Akismet
 
+> *Note:* Before 8.10 only issues submitted via the API and for non-project
+members were submitted to Akismet.
+
 GitLab leverages [Akismet](http://akismet.com) to protect against spam. Currently
-GitLab uses Akismet to prevent users who are not members of a project from
-creating spam via the GitLab API. Detected spam will be rejected, and
-an entry in the "Spam Log" section in the Admin page will be created.
+GitLab uses Akismet to prevent the creation of spam issues on public projects. Issues
+created via the WebUI or the API can be submitted to Akismet for review.
 
-> *Note:* As of 8.10 GitLab also submits issues created via the WebUI by non
-project members to Akismet to prevent spam.
+Detected spam will be rejected, and an entry in the "Spam Log" section in the
+Admin page will be created.
 
 Privacy note: GitLab submits the user's IP and user agent to Akismet. Note that
 adding a user to a project will disable the Akismet check and prevent this
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index 9adbde04884..787d416b960 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -157,15 +157,13 @@ module API
         end
 
         project = user_project
-        text = [attrs[:title], attrs[:description]].reject(&:blank?).join("\n")
 
-        if check_for_spam?(project, current_user) && is_spam?(env, current_user, text)
-          create_spam_log(project, current_user, attrs, env)
+        issue = ::Issues::CreateService.new(project, current_user, attrs.merge({ request: request })).execute
+
+        if issue.nil?
           render_api_error!({ error: 'Spam detected' }, 400)
         end
 
-        issue = ::Issues::CreateService.new(project, current_user, attrs).execute
-
         if issue.valid?
           # Find or create labels and attach to issue. Labels are valid because
           # we already checked its name, so there can't be an error here
diff --git a/lib/gitlab/akismet_helper.rb b/lib/gitlab/akismet_helper.rb
index fa8a79207f6..fc1fbc5b600 100644
--- a/lib/gitlab/akismet_helper.rb
+++ b/lib/gitlab/akismet_helper.rb
@@ -17,8 +17,8 @@ module Gitlab
       env['HTTP_USER_AGENT']
     end
 
-    def check_for_spam?(project, user)
-      akismet_enabled? && !project.team.member?(user)
+    def check_for_spam?(project)
+      akismet_enabled? && project.public?
     end
 
     def is_spam?(environment, user, text)
diff --git a/spec/lib/gitlab/akismet_helper_spec.rb b/spec/lib/gitlab/akismet_helper_spec.rb
index 88a71528867..b08396da4d2 100644
--- a/spec/lib/gitlab/akismet_helper_spec.rb
+++ b/spec/lib/gitlab/akismet_helper_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::AkismetHelper, type: :helper do
-  let(:project) { create(:project) }
+  let(:project) { create(:project, :public) }
   let(:user) { create(:user) }
 
   before do
@@ -11,13 +11,13 @@ describe Gitlab::AkismetHelper, type: :helper do
   end
 
   describe '#check_for_spam?' do
-    it 'returns true for non-member' do
-      expect(helper.check_for_spam?(project, user)).to eq(true)
+    it 'returns true for public project' do
+      expect(helper.check_for_spam?(project)).to eq(true)
     end
 
-    it 'returns false for member' do
-      project.team << [user, :guest]
-      expect(helper.check_for_spam?(project, user)).to eq(false)
+    it 'returns false for private project' do
+      project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PRIVATE)
+      expect(helper.check_for_spam?(project)).to eq(false)
     end
   end
 
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 12f2cfa6942..9d3d28e0b91 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -531,10 +531,8 @@ describe API::API, api: true  do
 
   describe 'POST /projects/:id/issues with spam filtering' do
     before do
-      Grape::Endpoint.before_each do |endpoint|
-        allow(endpoint).to receive(:check_for_spam?).and_return(true)
-        allow(endpoint).to receive(:is_spam?).and_return(true)
-      end
+      allow_any_instance_of(Gitlab::AkismetHelper).to receive(:check_for_spam?).and_return(true)
+      allow_any_instance_of(Gitlab::AkismetHelper).to receive(:is_spam?).and_return(true)
     end
 
     let(:params) do