Support Akismet spam checking for creation of issues via API

Currently any spam detected by Akismet by non-members via API will be logged
in a separate table in the admin page.

Closes #5612

CHANGELOG

@@ -74,6 +74,7 @@ v 8.4.0
   - Don't notify users twice if they are both project watchers and subscribers (Stan Hu)
   - Remove gray background from layout in UI
   - Fix signup for OAuth providers that don't provide a name
+  - Support Akismet spam checking for creation of issues via API (Stan Hu)
   - Implement new UI for group page
   - Implement search inside emoji picker
   - Let the CI runner know about builds that this build depends on

Gemfile

@@ -36,8 +36,9 @@ gem 'omniauth-twitter',       '~> 1.2.0'
 gem 'omniauth_crowd',         '~> 2.2.0'
 gem 'rack-oauth2',            '~> 1.2.1'
 
-# reCAPTCHA protection
+# Spam and anti-bot protection
 gem 'recaptcha', require: 'recaptcha/rails'
+gem 'akismet', '~> 2.0'
 
 # Two-factor authentication
 gem 'devise-two-factor', '~> 2.0.0'

Gemfile.lock

@@ -49,6 +49,7 @@ GEM
     addressable (2.3.8)
     after_commit_queue (1.3.0)
       activerecord (>= 3.0)
+    akismet (2.0.0)
     allocations (1.0.3)
     annotate (2.6.10)
       activerecord (>= 3.2, <= 4.3)
@@ -884,6 +885,7 @@ DEPENDENCIES
   acts-as-taggable-on (~> 3.4)
   addressable (~> 2.3.8)
   after_commit_queue
+  akismet (~> 2.0)
   allocations (~> 1.0)
   annotate (~> 2.6.0)
   asana (~> 0.4.0)

app/controllers/admin/application_settings_controller.rb

@@ -79,6 +79,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       :recaptcha_private_key,
       :sentry_enabled,
       :sentry_dsn,
+      :akismet_enabled,
+      :akismet_api_key,
       restricted_visibility_levels: [],
       import_sources: []
     )

app/controllers/admin/spam_logs_controller.rb

+class Admin::SpamLogsController < Admin::ApplicationController
+  before_action :set_spam_log, only: [:destroy]
+
+  def index
+    @spam_logs = SpamLog.order(created_at: :desc).page(params[:page])
+  end
+
+  def destroy
+    @spam_log.destroy
+    message = 'Spam log was successfully destroyed.'
+
+    if params[:remove_user]
+      username = @spam_log.user.username
+      @spam_log.user.destroy
+      message = "User #{username} was successfully destroyed."
+    end
+
+    respond_to do |format|
+      format.json { render json: '{}' }
+      format.html { redirect_to admin_spam_logs_path, notice: message }
+    end
+  end
+
+  private
+
+  def set_spam_log
+    @spam_log = SpamLog.find(params[:id])
+  end
+
+  def spam_log_params
+    params[:spam_log]
+  end
+end

app/models/application_setting.rb

@@ -88,6 +88,10 @@ class ApplicationSetting < ActiveRecord::Base
             presence: true,
             if: :sentry_enabled
 
+  validates :akismet_api_key,
+            presence: true,
+            if: :akismet_enabled
+
   validates_each :restricted_visibility_levels do |record, attr, value|
     unless value.nil?
       value.each do |level|
@@ -143,7 +147,9 @@ class ApplicationSetting < ActiveRecord::Base
       shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'],
       max_artifacts_size: Settings.artifacts['max_size'],
       require_two_factor_authentication: false,
-      two_factor_grace_period: 48
+      two_factor_grace_period: 48,
+      recaptcha_enabled: false,
+      akismet_enabled: false
     )
   end
 

app/models/spam_log.rb

+class SpamLog < ActiveRecord::Base
+  belongs_to :user
+
+  validates :user, presence: true
+
+  def truncated_description
+    if description.present? && description.length > 100
+      return description[0..100] + "..."
+    end
+
+    description
+  end
+end

app/models/spam_report.rb

+class SpamReport < ActiveRecord::Base
+  belongs_to :user
+
+  validates :user, presence: true
+end

app/models/user.rb

@@ -138,6 +138,7 @@ class User < ActiveRecord::Base
   has_many :assigned_merge_requests,  dependent: :destroy, foreign_key: :assignee_id, class_name: "MergeRequest"
   has_many :oauth_applications, class_name: 'Doorkeeper::Application', as: :owner, dependent: :destroy
   has_one  :abuse_report,             dependent: :destroy
+  has_many :spam_log,                 dependent: :destroy
   has_many :builds,                   dependent: :nullify, class_name: 'Ci::Build'
 
 

app/services/create_spam_log_service.rb

+class CreateSpamLogService < BaseService
+  def initialize(project, user, params)
+    super(project, user, params)
+  end
+
+  def execute
+    spam_params = params.merge({ user_id: @current_user.id,
+                                 project_id: @project.id } )
+    spam_log = SpamLog.new(spam_params)
+    spam_log.save
+    spam_log
+  end
+end

app/views/admin/application_settings/_form.html.haml

@@ -218,20 +218,37 @@
           = f.label :recaptcha_enabled do
             = f.check_box :recaptcha_enabled
             Enable reCAPTCHA
-          %span.help-block#recaptcha_help_block Helps preventing bots from creating accounts
+          %span.help-block#recaptcha_help_block Helps prevent bots from creating accounts
 
     .form-group
       = f.label :recaptcha_site_key, 'reCAPTCHA Site Key', class: 'control-label col-sm-2'
       .col-sm-10
         = f.text_field :recaptcha_site_key, class: 'form-control'
         .help-block
-          Generate site and private keys here:
-          %a{ href: 'http://www.google.com/recaptcha', target: '_blank'} http://www.google.com/recaptcha
+          Generate site and private keys at
+          %a{ href: 'http://www.google.com/recaptcha', target: 'blank'} http://www.google.com/recaptcha
+
     .form-group
       = f.label :recaptcha_private_key, 'reCAPTCHA Private Key', class: 'control-label col-sm-2'
       .col-sm-10
         = f.text_field :recaptcha_private_key, class: 'form-control'
 
+    .form-group
+      .col-sm-offset-2.col-sm-10
+        .checkbox
+          = f.label :akismet_enabled do
+            = f.check_box :akismet_enabled
+            Enable Akismet
+          %span.help-block#akismet_help_block Helps prevent bots from creating issues
+
+    .form-group
+      = f.label :akismet_api_key, 'Akismet API Key', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.text_field :akismet_api_key, class: 'form-control'
+        .help-block
+          Generate API key at
+          %a{ href: 'http://www.akismet.com', target: 'blank'} http://www.akismet.com
+
   %fieldset
     %legend Error Reporting and Logging
     %p

app/views/admin/spam_logs/_spam_log.html.haml

+- user = spam_log.user
+%tr
+  %td
+    = spam_log.created_at.to_s(:short)
+  %td
+    - if user
+      = link_to user.name, user
+    - else
+      (removed)
+  %td
+    = spam_log.source_ip
+  %td
+    = spam_log.via_api ? 'Y' : 'N'
+  %td
+    = spam_log.noteable_type
+  %td
+    = spam_log.title
+  %td
+    = spam_log.truncated_description
+  %td
+    - if user
+      = link_to 'Remove user', admin_spam_log_path(spam_log, remove_user: true),
+        data: { confirm: "USER #{user.name} WILL BE REMOVED! Are you sure?" }, method: :delete, class: "btn btn-xs btn-remove"
+  %td
+    - if user && !user.blocked?
+      = link_to 'Block user', block_admin_user_path(user), data: {confirm: 'USER WILL BE BLOCKED! Are you sure?'}, method: :put, class: "btn btn-xs"
+    - else
+      .btn.btn-xs.disabled
+        Already Blocked
+    = link_to 'Remove log', [:admin, spam_log, format: :json], remote: true, method: :delete, class: "btn btn-xs btn-close js-remove-tr"

app/views/admin/spam_logs/index.html.haml

+- page_title "Spam Logs"
+%h3.page-title Spam Logs
+%hr
+- if @spam_logs.present?
+  .table-holder
+    %table.table
+      %thead
+        %tr
+          %th Date
+          %th User
+          %th Source IP
+          %th Via API?
+          %th Type
+          %th Title
+          %th Description
+          %th Primary Action
+          %th
+      = render @spam_logs
+  = paginate @spam_logs
+- else
+  %h4 There are no Spam Logs

app/views/layouts/nav/_admin.html.haml

@@ -82,6 +82,13 @@
         Abuse Reports
         %span.count= number_with_delimiter(AbuseReport.count(:all))
 
+  = nav_link(controller: :spam_logs) do
+    = link_to admin_spam_logs_path, title: "Spam Logs" do
+      = icon('exclamation-triangle fw')
+      %span
+        Spam Logs
+        %span.count= number_with_delimiter(SpamLog.count(:all))
+
   = nav_link(controller: :application_settings, html_options: { class: 'separate-item'}) do
     = link_to admin_application_settings_path, title: 'Settings' do
       = icon('cogs fw')

config/routes.rb

@@ -211,6 +211,8 @@ Rails.application.routes.draw do
     end
 
     resources :abuse_reports, only: [:index, :destroy]
+    resources :spam_logs, only: [:index, :destroy]
+
     resources :applications
 
     resources :groups, constraints: { id: /[^\/]+/ } do

db/migrate/20151231152326_add_akismet_to_application_settings.rb

+class AddAkismetToApplicationSettings < ActiveRecord::Migration
+  def change
+    change_table :application_settings do |t|
+      t.boolean :akismet_enabled, default: false
+      t.string :akismet_api_key
+    end
+  end
+end

db/migrate/20160109054846_create_spam_logs.rb

+class CreateSpamLogs < ActiveRecord::Migration
+  def change
+    create_table :spam_logs do |t|
+      t.integer :user_id
+      t.string :source_ip
+      t.string :user_agent
+      t.boolean :via_api
+      t.integer :project_id
+      t.string :noteable_type
+      t.string :title
+      t.text :description
+
+      t.timestamps null: false
+    end
+  end
+end

db/schema.rb

@@ -64,6 +64,8 @@ ActiveRecord::Schema.define(version: 20160128233227) do
     t.integer  "metrics_sample_interval",           default: 15
     t.boolean  "sentry_enabled",                    default: false
     t.string   "sentry_dsn"
+    t.boolean  "akismet_enabled",                   default: false
+    t.string   "akismet_api_key"
   end
 
   create_table "audit_events", force: :cascade do |t|
@@ -770,6 +772,19 @@ ActiveRecord::Schema.define(version: 20160128233227) do
   add_index "snippets", ["project_id"], name: "index_snippets_on_project_id", using: :btree
   add_index "snippets", ["visibility_level"], name: "index_snippets_on_visibility_level", using: :btree
 
+  create_table "spam_logs", force: :cascade do |t|
+    t.integer  "user_id"
+    t.string   "source_ip"
+    t.string   "user_agent"
+    t.boolean  "via_api"
+    t.integer  "project_id"
+    t.string   "noteable_type"
+    t.string   "title"
+    t.text     "description"
+    t.datetime "created_at",    null: false
+    t.datetime "updated_at",    null: false
+  end
+
   create_table "subscriptions", force: :cascade do |t|
     t.integer  "user_id"
     t.integer  "subscribable_id"

doc/integration/README.md

@@ -15,6 +15,7 @@ See the documentation below for details on how to configure these services.
 - [OAuth2 provider](oauth_provider.md) OAuth2 application creation
 - [Gmail actions buttons](gmail_action_buttons_for_gitlab.md) Adds GitLab actions to messages
 - [reCAPTCHA](recaptcha.md) Configure GitLab to use Google reCAPTCHA for new users
+- [Akismet](akismet.md) Configure Akismet to stop spam
 
 GitLab Enterprise Edition contains [advanced Jenkins support][jenkins].
 

doc/integration/akismet.md

+# 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.
+
+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.
+
+## Configuration
+
+To use Akismet:
+
+1. Go to the URL: https://akismet.com/account/
+
+2. Sign-in or create a new account.
+
+3. Click on "Show" to reveal the API key.
+
+4. Go to Applications Settings on Admin Area (`admin/application_settings`)
+
+5. Check the `Enable Akismet` checkbox
+
+6. Fill in the API key from step 3.
+
+7. Save the configuration.
+
+![Screenshot of Akismet settings](img/akismet_settings.png)

features/admin/spam_logs.feature

+Feature: Admin spam logs
+  Background:
+    Given I sign in as an admin
+    And spam logs exist
+
+  Scenario: Browse spam logs
+    When I visit spam logs page
+    Then I should see list of spam logs

features/steps/admin/spam_logs.rb

+class Spinach::Features::AdminSpamLogs < Spinach::FeatureSteps
+  include SharedAuthentication
+  include SharedPaths
+  include SharedAdmin
+
+  step 'I should see list of spam logs' do
+    page.should have_content("Spam Logs")
+    spam_log = SpamLog.first
+    page.should have_content spam_log.title
+    page.should have_content spam_log.description
+    page.should have_link("Remove user")
+    page.should have_link("Block user")
+  end
+
+  step 'spam logs exist' do
+    create(:spam_log)
+  end
+end

features/steps/shared/paths.rb

@@ -191,6 +191,10 @@ module SharedPaths
     visit admin_application_settings_path
   end
 
+  step 'I visit spam logs page' do
+    visit admin_spam_logs_path
+  end
+
   step 'I visit applications page' do
     visit admin_applications_path
   end

lib/api/issues.rb

@@ -3,6 +3,8 @@ module API
   class Issues < Grape::API
     before { authenticate! }
 
+    helpers ::Gitlab::AkismetHelper
+
     helpers do
       def filter_issues_state(issues, state)
         case state
@@ -19,6 +21,15 @@ 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.dup
+        params[:source_ip] = env['REMOTE_ADDR']
+        params[:user_agent] = env['HTTP_USER_AGENT']
+        params[:noteable_type] = 'Issue'
+        params[:via_api] = true
+        ::CreateSpamLogService.new(project, current_user, params).execute
+      end
     end
 
     resource :issues do
@@ -114,7 +125,16 @@ module API
           render_api_error!({ labels: errors }, 400)
         end
 
-        issue = ::Issues::CreateService.new(user_project, current_user, attrs).execute
+        project = user_project
+        text = attrs[:title]
+        text += "\n#{attrs[:description]}" if attrs[:description].present?
+
+        if check_for_spam?(project, current_user) && is_spam?(env, current_user, text)
+          create_spam_log(project, current_user, attrs)
+          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

lib/gitlab/akismet_helper.rb

+module Gitlab
+  module AkismetHelper
+    def akismet_enabled?
+      current_application_settings.akismet_enabled
+    end
+
+    def akismet_client
+      ::Akismet::Client.new(current_application_settings.akismet_api_key,
+                            Gitlab.config.gitlab.url)
+    end
+
+    def check_for_spam?(project, user)
+      akismet_enabled? && !project.team.member?(user)
+    end
+
+    def is_spam?(environment, user, text)
+      client = akismet_client
+      ip_address = environment['REMOTE_ADDR']
+      user_agent = environment['HTTP_USER_AGENT']
+
+      params = {
+        type: 'comment',
+        text: text,
+        created_at: DateTime.now,
+        author: user.name,
+        author_email: user.email,
+        referrer: environment['HTTP_REFERER'],
+      }
+
+      begin
+        is_spam, is_blatant = client.check(ip_address, user_agent, params)
+        is_spam || is_blatant
+      rescue => e
+        Rails.logger.error("Unable to connect to Akismet: #{e}, skipping check")
+        false
+      end
+    end
+  end
+end

lib/gitlab/current_settings.rb

@@ -34,7 +34,8 @@ module Gitlab
         shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'],
         max_artifacts_size: Settings.artifacts['max_size'],
         require_two_factor_authentication: false,
-        two_factor_grace_period: 48
+        two_factor_grace_period: 48,
+        akismet_enabled: false
       )
     end
 

spec/controllers/admin/spam_logs_controller_spec.rb

+require 'spec_helper'
+
+describe Admin::SpamLogsController do
+  let(:admin)    { create(:admin) }
+  let(:spam_log) { create(:spam_log, user: admin) }
+
+  before do
+    sign_in(admin)
+  end
+
+  describe '#index' do
+    it 'lists all spam logs' do
+      get :index
+      expect(response.status).to eq(200)
+    end
+  end
+
+  describe '#destroy' do
+    it 'destroys just spam log' do
+      user = spam_log.user
+      delete :destroy, id: spam_log.id
+
+      expect(SpamLog.all.count).to eq(0)
+      expect(User.find(user.id)).to be_truthy
+      expect(response.status).to eq(302)
+    end
+
+    it 'destroys user and his spam logs' do
+      user = spam_log.user
+      delete :destroy, id: spam_log.id, remove_user: true
+
+      expect(SpamLog.all.count).to eq(0)
+      expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound)
+      expect(response.status).to eq(302)
+    end
+
+    it 'destroys user and his spam logs with JSON format' do
+      user = spam_log.user
+      delete :destroy, id: spam_log.id, remove_user: true, format: :json
+
+      expect(SpamLog.all.count).to eq(0)
+      expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound)
+      expect(JSON.parse(response.body)).to eq({})
+      expect(response.status).to eq(200)
+    end
+  end
+end

spec/factories/spam_logs.rb

+# Read about factories at https://github.com/thoughtbot/factory_girl
+
+FactoryGirl.define do
+  factory :spam_log do
+    user
+  end
+end

spec/lib/gitlab/akismet_helper_spec.rb

+require 'spec_helper'
+
+describe Gitlab::AkismetHelper, type: :helper do
+  let(:project) { create(:project) }
+  let(:user) { create(:user) }
+
+  before do
+    allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url))
+    current_application_settings.akismet_enabled = true
+    current_application_settings.akismet_api_key = '12345'
+  end
+
+  describe '#check_for_spam?' do
+    it 'returns true for non-member' do
+      expect(helper.check_for_spam?(project, user)).to eq(true)
+    end
+
+    it 'returns false for member' do
+      project.team << [user, :guest]
+      expect(helper.check_for_spam?(project, user)).to eq(false)
+    end
+  end
+
+  describe '#is_spam?' do
+    it 'returns true for spam' do
+      environment = {
+        'REMOTE_ADDR' => '127.0.0.1',
+        'HTTP_USER_AGENT' => 'Test User Agent'
+      }
+
+      allow_any_instance_of(::Akismet::Client).to receive(:check).and_return([true, true])
+      expect(helper.is_spam?(environment, user, 'Is this spam?')).to eq(true)
+    end
+  end
+end

spec/requests/api/issues_spec.rb

@@ -241,6 +241,28 @@ describe API::API, api: true  do
     end
   end
 
+  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
+    end
+
+    it "should create a new project issue" do
+      post api("/projects/#{project.id}/issues", user),
+           title: 'new issue', labels: 'label, label2'
+      expect(response.status).to eq(400)
+      expect(json_response['message']).to eq({ "error" => "Spam detected" })
+      spam_logs = SpamLog.all
+      expect(spam_logs.count).to eq(1)
+      expect(spam_logs[0].title).to eq('new issue')
+      expect(spam_logs[0].user).to eq(user)
+      expect(spam_logs[0].noteable_type).to eq('Issue')
+      expect(spam_logs[0].project_id).to eq(project.id)
+    end
+  end
+
   describe "PUT /projects/:id/issues/:issue_id to update only title" do
     it "should update a project issue" do
       put api("/projects/#{project.id}/issues/#{issue.id}", user),