diff options
| -rw-r--r-- | app/models/concerns/akismet_submittable.rb | 15 | ||||
| -rw-r--r-- | app/models/concerns/spammable.rb | 58 | ||||
| -rw-r--r-- | app/models/issue.rb | 2 | ||||
| -rw-r--r-- | app/services/issues/create_service.rb | 2 | ||||
| -rw-r--r-- | app/services/spam_check_service.rb | 22 | ||||
| -rw-r--r-- | lib/api/issues.rb | 2 | ||||
| -rw-r--r-- | lib/gitlab/akismet_helper.rb | 34 | ||||
| -rw-r--r-- | spec/factories/user_agent_details.rb | 2 | ||||
| -rw-r--r-- | spec/models/concerns/spammable_spec.rb | 41 | ||||
| -rw-r--r-- | spec/models/user_agent_detail_spec.rb | 5 |
10 files changed, 145 insertions, 38 deletions
diff --git a/app/models/concerns/akismet_submittable.rb b/app/models/concerns/akismet_submittable.rb deleted file mode 100644 index 17821688941..00000000000 --- a/app/models/concerns/akismet_submittable.rb +++ /dev/null @@ -1,15 +0,0 @@ -module AkismetSubmittable - extend ActiveSupport::Concern - - included do - has_one :user_agent_detail, as: :subject - end - - def can_be_submitted? - if user_agent_detail - user_agent_detail.submittable? - else - false - end - end -end diff --git a/app/models/concerns/spammable.rb b/app/models/concerns/spammable.rb index 3b8e6df2da9..bbf6a3e0be3 100644 --- a/app/models/concerns/spammable.rb +++ b/app/models/concerns/spammable.rb @@ -1,16 +1,70 @@ module Spammable extend ActiveSupport::Concern + include Gitlab::AkismetHelper + + module ClassMethods + def attr_spammable(*attrs) + attrs.each do |attr| + spammable_attrs << attr.to_s + end + end + end included do + has_one :user_agent_detail, as: :subject, dependent: :destroy attr_accessor :spam after_validation :check_for_spam, on: :create + + cattr_accessor :spammable_attrs, instance_accessor: false do + [] + end + end + + def can_be_submitted? + if user_agent_detail + user_agent_detail.submittable? + else + false + end + end + + def submit_ham + return unless akismet_enabled? && can_be_submitted? + ham!(user_agent_detail, spammable_text, creator) + end + + def submit_spam + return unless akismet_enabled? && can_be_submitted? + spam!(user_agent_detail, spammable_text, creator) + end + + def spam?(env, user) + is_spam?(env, user, spammable_text) end - def spam? + def spam_detected? @spam end def check_for_spam - self.errors.add(:base, "Your #{self.class.name.underscore} has been recognized as spam and has been discarded.") if spam? + self.errors.add(:base, "Your #{self.class.name.underscore} has been recognized as spam and has been discarded.") if spam_detected? + end + + private + + def spammable_text + result = [] + self.class.spammable_attrs.each do |entry| + result << self.send(entry) + end + result.reject(&:blank?).join("\n") + end + + def creator + if self.author_id + User.find(self.author_id) + elsif self.creator_id + User.find(self.creator_id) + end end end diff --git a/app/models/issue.rb b/app/models/issue.rb index 6c2635498e5..5408e24b21c 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -37,6 +37,8 @@ class Issue < ActiveRecord::Base scope :order_due_date_asc, -> { reorder('issues.due_date IS NULL, issues.due_date ASC') } scope :order_due_date_desc, -> { reorder('issues.due_date IS NULL, issues.due_date DESC') } + attr_spammable :title, :description + state_machine :state, initial: :opened do event :close do transition [:reopened, :opened] => :closed diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 8e9d74103c7..d580834be83 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -8,7 +8,7 @@ module Issues issue = project.issues.new(params) issue.author = params[:author] || current_user - issue.spam = spam_check_service.execute(request, api) + issue.spam = spam_check_service.execute(request, api, issue) if issue.save issue.update_attributes(label_ids: label_params) diff --git a/app/services/spam_check_service.rb b/app/services/spam_check_service.rb index 7c3e692bde9..7d6754546a8 100644 --- a/app/services/spam_check_service.rb +++ b/app/services/spam_check_service.rb @@ -1,23 +1,17 @@ class SpamCheckService < BaseService - include Gitlab::AkismetHelper + attr_accessor :request, :api, :subject - attr_accessor :request, :api + def execute(request, api, subject) + @request, @api, @subject = request, api, subject + return false unless request || subject.check_for_spam?(project) + return false unless subject.spam?(request.env, current_user) - def execute(request, api) - @request, @api = request, api - return false unless request || check_for_spam?(project) - return false unless is_spam?(request.env, current_user, text) - create_spam_log true end private - - def text - [params[:title], params[:description]].reject(&:blank?).join("\n") - end def spam_log_attrs { @@ -25,9 +19,9 @@ class SpamCheckService < BaseService project_id: project.id, title: params[:title], description: params[:description], - source_ip: client_ip(request.env), - user_agent: user_agent(request.env), - noteable_type: 'Issue', + source_ip: subject.client_ip(request.env), + user_agent: subject.user_agent(request.env), + noteable_type: subject.class.to_s, via_api: api } end diff --git a/lib/api/issues.rb b/lib/api/issues.rb index c4d3134da6c..7bbfc137c2c 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -160,7 +160,7 @@ module API issue = ::Issues::CreateService.new(project, current_user, attrs.merge(request: request, api: true)).execute - if issue.spam? + if issue.spam_detected? render_api_error!({ error: 'Spam detected' }, 400) end diff --git a/lib/gitlab/akismet_helper.rb b/lib/gitlab/akismet_helper.rb index 207736b59db..19e73820321 100644 --- a/lib/gitlab/akismet_helper.rb +++ b/lib/gitlab/akismet_helper.rb @@ -43,5 +43,39 @@ module Gitlab false end end + + def ham!(details, text, user) + client = akismet_client + + params = { + type: 'comment', + text: text, + author: user.name, + author_email: user.email + } + + begin + client.submit_ham(details.ip_address, details.user_agent, params) + rescue => e + Rails.logger.error("Unable to connect to Akismet: #{e}, skipping!") + end + end + + def spam!(details, text, user) + client = akismet_client + + params = { + type: 'comment', + text: text, + author: user.name, + author_email: user.email + } + + begin + client.submit_spam(details.ip_address, details.user_agent, params) + rescue => e + Rails.logger.error("Unable to connect to Akismet: #{e}, skipping!") + end + end end end diff --git a/spec/factories/user_agent_details.rb b/spec/factories/user_agent_details.rb index 5fc40915911..10de5dcb329 100644 --- a/spec/factories/user_agent_details.rb +++ b/spec/factories/user_agent_details.rb @@ -2,6 +2,8 @@ FactoryGirl.define do factory :user_agent_detail do ip_address '127.0.0.1' user_agent 'AppleWebKit/537.36' + subject_id 1 + subject_type 'Issue' trait :on_issue do association :subject, factory: :issue diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb new file mode 100644 index 00000000000..e61a6dcb69d --- /dev/null +++ b/spec/models/concerns/spammable_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +describe Issue, 'Spammable' do + let(:issue) { create(:issue, description: 'Test Desc.') } + + describe 'Associations' do + it { is_expected.to have_one(:user_agent_detail).dependent(:destroy) } + end + + describe 'ClassMethods' do + it 'should return correct attr_spammable' do + expect(issue.send(:spammable_text)).to eq("#{issue.title}\n#{issue.description}") + end + end + + describe 'InstanceMethods' do + it 'should return the correct creator' do + expect(issue.send(:creator).id).to eq(issue.author_id) + end + + it 'should be invalid if spam' do + issue.spam = true + expect(issue.valid?).to be_truthy + end + + it 'should be submittable' do + create(:user_agent_detail, subject_id: issue.id, subject_type: issue.class.to_s) + expect(issue.can_be_submitted?).to be_truthy + end + end + + describe 'AkismetMethods' do + before do + allow_any_instance_of(Gitlab::AkismetHelper).to receive_messages(check_for_spam?: true, is_spam?: true, ham!: nil, spam!: nil) + end + + it { expect(issue.spam?(:mock_env, :mock_user)).to be_truthy } + it { expect(issue.submit_spam).to be_nil } + it { expect(issue.submit_ham).to be_nil } + end +end diff --git a/spec/models/user_agent_detail_spec.rb b/spec/models/user_agent_detail_spec.rb index 8debcbbeba6..ba21161fc7f 100644 --- a/spec/models/user_agent_detail_spec.rb +++ b/spec/models/user_agent_detail_spec.rb @@ -13,10 +13,5 @@ describe UserAgentDetail, type: :model do detail = create(:user_agent_detail, :on_issue) expect(detail).to be_valid end - - it 'should not be valid without a subject' do - detail = build(:user_agent_detail) - expect(detail).not_to be_valid - end end end |