From 9c34fafb8b728358a516a25120aa5f28567eae48 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Sat, 16 Jul 2016 11:42:44 -0500 Subject: [PATCH] Submit new issues created via the WebUI by non project members to Akismet for spam check. --- CHANGELOG | 1 + app/controllers/projects/issues_controller.rb | 18 ++++++++++- doc/integration/akismet.md | 3 ++ lib/api/issues.rb | 13 +------- lib/gitlab/akismet_helper.rb | 11 +++++++ .../projects/issues_controller_spec.rb | 31 +++++++++++++++++++ 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) }