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") + @issue = Issues::CreateService.new(project, current_user, issue_params.merge({ request: request })).execute - 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) + 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 -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. +> *Note:* Before 8.10 only issues submitted via the API and for non-project +members were submitted to Akismet. -> *Note:* As of 8.10 GitLab also submits issues created via the WebUI by non -project members to Akismet to prevent spam. +GitLab leverages [Akismet](http://akismet.com) to protect against spam. Currently +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. + +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