Merge branch 'jrochkind/gitlab-ce-fix_2839_send_abuse_report_notify' into 'master'

Send an email to admin email when a user is reported for spam

Replaces !1547.

Fixes #2839.

See merge request !1634

CHANGELOG

@@ -8,6 +8,7 @@ v 8.2.0 (unreleased)
   - Allow users to select the Files view as default project view (Cristian Bica)
 
 v 8.1.0 (unreleased)
+  - Send an email to admin email when a user is reported for spam (Jonathan Rochkind)
   - Fix bug preventing mentioned issued from being closed when MR is merged using fast-forward merge.
   - Fix nonatomic database update potentially causing project star counts to go negative (Stan Hu)
   - Fix error preventing displaying of commit data for a directory with a leading dot (Stan Hu)

app/controllers/abuse_reports_controller.rb

@@ -9,6 +9,10 @@ class AbuseReportsController < ApplicationController
     @abuse_report.reporter = current_user
 
     if @abuse_report.save
+      if current_application_settings.admin_notification_email.present?
+        AbuseReportMailer.delay.notify(@abuse_report.id)
+      end
+
       message = "Thank you for your report. A GitLab administrator will look into it shortly."
       redirect_to root_path, notice: message
     else

app/controllers/admin/application_settings_controller.rb

@@ -55,6 +55,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       :default_snippet_visibility,
       :restricted_signup_domains_raw,
       :version_check_enabled,
+      :admin_notification_email,
       :user_oauth_applications,
       restricted_visibility_levels: [],
       import_sources: []

app/mailers/abuse_report_mailer.rb

+class AbuseReportMailer < BaseMailer
+  include Gitlab::CurrentSettings
+
+  def notify(abuse_report_id)
+    @abuse_report = AbuseReport.find(abuse_report_id)
+
+    mail(
+      to:       current_application_settings.admin_notification_email, 
+      subject:  "#{@abuse_report.user.name} (#{@abuse_report.user.username}) was reported for abuse"
+    )
+  end
+end

app/models/application_setting.rb

@@ -44,6 +44,10 @@ class ApplicationSetting < ActiveRecord::Base
     allow_blank: true,
     format: { with: /\A#{URI.regexp(%w(http https))}\z/, message: "should be a valid url" }
 
+  validates :admin_notification_email,
+    allow_blank: true,
+    email: true
+
   validates_each :restricted_visibility_levels do |record, attr, value|
     unless value.nil?
       value.each do |level|

app/views/abuse_report_mailer/notify.html.haml

+%p
+  #{link_to @abuse_report.user.name, user_url(@abuse_report.user)} 
+  (@#{@abuse_report.user.username}) was reported for abuse by 
+  #{link_to @abuse_report.reporter.name, user_url(@abuse_report.reporter)} 
+  (@#{@abuse_report.reporter.username}).
+
+%blockquote
+  = @abuse_report.message
+
+%p
+  = link_to "View details", abuse_reports_url

app/views/abuse_report_mailer/notify.text.haml

+#{@abuse_report.user.name} (@#{@abuse_report.user.username}) was reported for abuse by #{@abuse_report.reporter.name} (@#{@abuse_report.reporter.username}).
+\
+> #{@abuse_report.message}
+\
+View details: #{admin_abuse_reports_url}

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

@@ -47,6 +47,12 @@
           = f.label :version_check_enabled do
             = f.check_box :version_check_enabled
             Version check enabled
+    .form-group
+      = f.label :admin_notification_email, class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.text_field :admin_notification_email, class: 'form-control'
+        .help-block
+          Abuse reports will be sent to this address if it is set. Abuse reports are always available in the admin area.
 
   %fieldset
     %legend Account and Limit Settings

db/migrate/20151008143519_add_admin_notification_email_setting.rb

+class AddAdminNotificationEmailSetting < ActiveRecord::Migration
+  def change
+    add_column :application_settings, :admin_notification_email, :string
+  end
+end

db/schema.rb

@@ -46,6 +46,7 @@ ActiveRecord::Schema.define(version: 20151016195706) do
     t.integer  "session_expire_delay",         default: 10080, null: false
     t.text     "import_sources"
     t.text     "help_page_text"
+    t.string   "admin_notification_email"
   end
 
   create_table "audit_events", force: true do |t|

spec/controllers/abuse_reports_controller_spec.rb

+require 'spec_helper'
+
+describe AbuseReportsController do
+  let(:reporter)    { create(:user) }
+  let(:user)        { create(:user) }
+  let(:message)     { "This user is a spammer" }
+
+  before do
+    sign_in(reporter)
+  end
+
+  describe "POST create" do
+    context "with admin notification email set" do
+      let(:admin_email) { "admin@example.com"}
+
+      before(:each) do
+        stub_application_setting(admin_notification_email: admin_email)
+      end
+
+      it "sends a notification email" do
+        post :create,
+          abuse_report: {
+            user_id: user.id,
+            message: message
+          }
+
+        email = ActionMailer::Base.deliveries.last
+
+        expect(email.to).to eq([admin_email])
+        expect(email.subject).to include(user.username)
+        expect(email.text_part.body).to include(message)
+      end
+
+      it "saves the abuse report" do
+        expect do
+          post :create,
+            abuse_report: {
+              user_id: user.id,
+              message: message
+            }
+        end.to change { AbuseReport.count }.by(1)
+      end
+    end
+
+    context "without admin notification email set" do
+      before(:each) do
+        stub_application_setting(admin_notification_email: nil)
+      end
+
+      it "does not send a notification email" do
+        expect do
+          post :create,
+            abuse_report: {
+              user_id: user.id,
+              message: message
+            }
+        end.not_to change { ActionMailer::Base.deliveries.count }
+      end
+
+      it "saves the abuse report" do
+        expect do
+          post :create,
+            abuse_report: {
+              user_id: user.id,
+              message: message
+            }
+        end.to change { AbuseReport.count }.by(1)
+      end
+    end
+  end
+
+end