Submit new issues created via the WebUI by non project members to Akismet for spam check.

6 files changed, 64 insertions, 13 deletions

diff --git a/CHANGELOG b/CHANGELOG
index dca9b209582..2efacc9e05b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -93,6 +93,7 @@ v 8.10.0
   - Fix viewing notification settings when a project is pending deletion
   - 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
   - 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 fa663c9bda4..10de472aad5 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -2,6 +2,7 @@ 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,
@@ -79,6 +80,21 @@ 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 = @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|
@@ -89,7 +105,7 @@ class Projects::IssuesController < Projects::ApplicationController
           render :new
         end
       end
-      format.js do |format|
+      format.js do
         @link = @issue.attachment.url.to_js
       end
     end
diff --git a/doc/integration/akismet.md b/doc/integration/akismet.md
index 5cc09bd536d..52c1b0a75d8 100644
--- a/doc/integration/akismet.md
+++ b/doc/integration/akismet.md
@@ -5,6 +5,9 @@ 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.
 
+> *Note:* As of 8.10 GitLab also submits issues created via the WebUI by non
+project members to Akismet to prevent spam.
+
 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
 from happening.
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index c588103e517..9adbde04884 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -21,17 +21,6 @@ module API
       def filter_issues_milestone(issues, milestone)
         issues.includes(:milestone).where('milestones.title' => milestone)
       end
-
-      def create_spam_log(project, current_user, attrs)
-        params = attrs.merge({
-          source_ip: client_ip(env),
-          user_agent: user_agent(env),
-          noteable_type: 'Issue',
-          via_api: true
-        })
-
-        ::CreateSpamLogService.new(project, current_user, params).execute
-      end
     end
 
     resource :issues do
@@ -171,7 +160,7 @@ module API
         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)
+          create_spam_log(project, current_user, attrs, env)
           render_api_error!({ error: 'Spam detected' }, 400)
         end
 
diff --git a/lib/gitlab/akismet_helper.rb b/lib/gitlab/akismet_helper.rb
index 04676fdb748..fa8a79207f6 100644
--- a/lib/gitlab/akismet_helper.rb
+++ b/lib/gitlab/akismet_helper.rb
@@ -43,5 +43,16 @@ module Gitlab
         false
       end
     end
+
+    def create_spam_log(project, current_user, attrs, env, api: true)
+      params = attrs.merge({
+                             source_ip: client_ip(env),
+                             user_agent: user_agent(env),
+                             noteable_type: 'Issue',
+                             via_api: api
+                           })
+
+      ::CreateSpamLogService.new(project, current_user, params).execute
+    end
   end
 end
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 7cf09fa4a4a..77f65057f71 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -243,6 +243,37 @@ describe Projects::IssuesController do
     end
   end
 
+  describe 'POST #create' do
+    context 'Akismet is enabled' do
+      before do
+        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
+
+      def post_spam_issue
+        sign_in(user)
+        spam_project = create(:empty_project, :public)
+        post :create, {
+          namespace_id: spam_project.namespace.to_param,
+          project_id: spam_project.to_param,
+          issue: { title: 'Spam Title', description: 'Spam lives here' }
+        }
+      end
+
+      it 'rejects an issue recognized as spam' do
+        expect{ post_spam_issue }.not_to change(Issue, :count)
+        expect(response).to render_template(:new)
+      end
+
+      it 'creates a spam log' do
+        post_spam_issue
+        spam_logs = SpamLog.all
+        expect(spam_logs.count).to eq(1)
+        expect(spam_logs[0].title).to eq('Spam Title')
+      end
+    end
+  end
+
   describe "DELETE #destroy" do
     context "when the user is a developer" do
       before { sign_in(user) }