diff --git a/app/logical/spam_detector.rb b/app/logical/spam_detector.rb index 21f0cdbbf..3f47623b1 100644 --- a/app/logical/spam_detector.rb +++ b/app/logical/spam_detector.rb @@ -4,6 +4,12 @@ class SpamDetector include Rakismet::Model + # if a person receives more than 10 automatic spam reports within a 1 hour + # window, automatically ban them forever. + AUTOBAN_THRESHOLD = 10 + AUTOBAN_WINDOW = 1.hours + AUTOBAN_DURATION = 999999 + attr_accessor :record, :user, :user_ip, :content, :comment_type rakismet_attrs author: proc { user.name }, author_email: proc { user.email }, @@ -24,6 +30,23 @@ class SpamDetector false end + def self.is_spammer?(user) + return false if user.is_gold? + + automatic_reports = ModerationReport.where("created_at > ?", AUTOBAN_WINDOW.ago).where(creator: User.system) + + dmail_reports = automatic_reports.where(model: Dmail.sent_by(user)) + comment_reports = automatic_reports.where(model: user.comments) + forum_post_reports = automatic_reports.where(model: user.forum_posts) + + report_count = dmail_reports.or(comment_reports).or(forum_post_reports).count + report_count >= AUTOBAN_THRESHOLD + end + + def self.ban_spammer!(spammer) + spammer.bans.create!(banner: User.system, reason: "Spambot.", duration: AUTOBAN_DURATION) + end + def initialize(record, user_ip: nil) case record when Dmail diff --git a/app/models/comment.rb b/app/models/comment.rb index 106248b16..92240d757 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -2,13 +2,14 @@ class Comment < ApplicationRecord include Mentionable validate :validate_creator_is_not_limited, :on => :create - validate :validate_comment_is_not_spam, on: :create validates_presence_of :body, :message => "has no content" belongs_to :post belongs_to :creator, class_name: "User" belongs_to_updater has_many :moderation_reports, as: :model has_many :votes, :class_name => "CommentVote", :dependent => :destroy + + before_create :autoreport_spam after_create :update_last_commented_at_on_create after_update(:if => ->(rec) {(!rec.is_deleted? || !rec.saved_change_to_is_deleted?) && CurrentUser.id != rec.creator_id}) do |rec| ModAction.log("comment ##{rec.id} updated by #{CurrentUser.name}", :comment_update) @@ -97,8 +98,10 @@ class Comment < ApplicationRecord end end - def validate_comment_is_not_spam - errors[:base] << "Failed to create comment" if SpamDetector.new(self).spam? + def autoreport_spam + if SpamDetector.new(self).spam? + moderation_reports << ModerationReport.new(creator: User.system, reason: "Spam.") + end end def update_last_commented_at_on_create diff --git a/app/models/dmail.rb b/app/models/dmail.rb index da2944f30..93400c715 100644 --- a/app/models/dmail.rb +++ b/app/models/dmail.rb @@ -1,10 +1,6 @@ require 'digest/sha1' class Dmail < ApplicationRecord - # if a person sends spam to more than 10 users within a 24 hour window, automatically ban them for 3 days. - AUTOBAN_THRESHOLD = 10 - AUTOBAN_WINDOW = 24.hours - AUTOBAN_DURATION = 3 validates_presence_of :title, :body, on: :create validate :validate_sender_is_not_banned, on: :create @@ -12,8 +8,10 @@ class Dmail < ApplicationRecord belongs_to :owner, :class_name => "User" belongs_to :to, :class_name => "User" belongs_to :from, :class_name => "User" + has_many :moderation_reports, as: :model after_initialize :initialize_attributes, if: :new_record? + before_create :autoreport_spam after_save :update_unread_dmail_count after_commit :send_email, on: :create @@ -27,29 +25,6 @@ class Dmail < ApplicationRecord scope :sent, -> { where("dmails.owner_id = dmails.from_id") } scope :received, -> { where("dmails.owner_id = dmails.to_id") } - concerning :SpamMethods do - class_methods do - def is_spammer?(user) - return false if user.is_gold? - - spammed_users = sent_by(user).where(is_spam: true).where("created_at > ?", AUTOBAN_WINDOW.ago).distinct.count(:to_id) - spammed_users >= AUTOBAN_THRESHOLD - end - - def ban_spammer(spammer) - spammer.bans.create! do |ban| - ban.banner = User.system - ban.reason = "Spambot." - ban.duration = AUTOBAN_DURATION - end - end - end - - def spam? - SpamDetector.new(self).spam? - end - end - module AddressMethods def to_name=(name) self.to = User.find_by_name(name) @@ -72,7 +47,6 @@ class Dmail < ApplicationRecord # recipient's copy copy = Dmail.new(params) copy.owner_id = copy.to_id - copy.is_spam = copy.spam? copy.save unless copy.to_id == copy.from_id # sender's copy @@ -80,8 +54,6 @@ class Dmail < ApplicationRecord copy.owner_id = copy.from_id copy.is_read = true copy.save - - Dmail.ban_spammer(copy.from) if Dmail.is_spammer?(copy.from) end copy @@ -200,6 +172,13 @@ class Dmail < ApplicationRecord owner == to end + def autoreport_spam + if is_recipient? && SpamDetector.new(self).spam? + self.is_deleted = true + moderation_reports << ModerationReport.new(creator: User.system, reason: "Spam.") + end + end + def update_unread_dmail_count return unless saved_change_to_id? || saved_change_to_is_read? || saved_change_to_is_deleted? diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb index b6eeb0d79..9fd5aa48c 100644 --- a/app/models/forum_post.rb +++ b/app/models/forum_post.rb @@ -14,12 +14,12 @@ class ForumPost < ApplicationRecord before_validation :initialize_is_deleted, :on => :create before_save :update_dtext_links, if: :dtext_links_changed? + before_create :autoreport_spam after_create :update_topic_updated_at_on_create after_update :update_topic_updated_at_on_update_for_original_posts after_destroy :update_topic_updated_at_on_destroy validates_presence_of :body validate :validate_topic_is_unlocked - validate :validate_post_is_not_spam, on: :create validate :topic_is_not_restricted, :on => :create before_destroy :validate_topic_is_unlocked after_save :delete_topic_if_original_post @@ -108,8 +108,10 @@ class ForumPost < ApplicationRecord votes.where(creator_id: user.id, score: score).exists? end - def validate_post_is_not_spam - errors[:base] << "Failed to create forum post" if SpamDetector.new(self, user_ip: CurrentUser.ip_addr).spam? + def autoreport_spam + if SpamDetector.new(self, user_ip: CurrentUser.ip_addr).spam? + moderation_reports << ModerationReport.new(creator: User.system, reason: "Spam.") + end end def validate_topic_is_unlocked diff --git a/app/models/moderation_report.rb b/app/models/moderation_report.rb index 66da90265..1e9fe471c 100644 --- a/app/models/moderation_report.rb +++ b/app/models/moderation_report.rb @@ -7,6 +7,7 @@ class ModerationReport < ApplicationRecord validates :creator, uniqueness: { scope: [:model_type, :model_id], message: "have already reported this message." } after_create :create_forum_post! + after_create :autoban_reported_user scope :user, -> { where(model_type: "User") } scope :dmail, -> { where(model_type: "Dmail") } @@ -54,6 +55,23 @@ class ModerationReport < ApplicationRecord updater.update(forum_post_message) end + def autoban_reported_user + if SpamDetector.is_spammer?(reported_user) + SpamDetector.ban_spammer!(reported_user) + end + end + + def reported_user + case model + when Comment, ForumPost + model.creator + when Dmail + model.from + else + raise NotImplementedError + end + end + def self.visible(user = CurrentUser.user) user.is_moderator? ? all : none end diff --git a/test/unit/dmail_test.rb b/test/unit/dmail_test.rb index afbf9d1b9..9a85b8509 100644 --- a/test/unit/dmail_test.rb +++ b/test/unit/dmail_test.rb @@ -15,31 +15,16 @@ class DmailTest < ActiveSupport::TestCase CurrentUser.user = nil end - context "spam" do - setup do - Dmail.any_instance.stubs(:spam?).returns(true) - @spammer = create(:user) + context "that is spam" do + should "be automatically reported and deleted" do @recipient = create(:user) - end + @spammer = create(:user, created_at: 2.weeks.ago, email: "akismet-guaranteed-spam@example.com") - should "not validate" do - assert_difference("Dmail.count", 2) do - Dmail.create_split(from: @spammer, to: @recipient, title: "spam", body: "wonderful spam") - assert(@recipient.dmails.last.is_spam?) - end - end + SpamDetector.stubs(:enabled?).returns(true) + dmail = create(:dmail, owner: @recipient, from: @spammer, to: @recipient, creator_ip_addr: "127.0.0.1") - should "autoban spammers after sending spam to N distinct users" do - users = create_list(:user, Dmail::AUTOBAN_THRESHOLD) - users.each do |user| - Dmail.create_split(from: @spammer, to: user, title: "spam", body: "wonderful spam") - end - - assert_equal(true, Dmail.is_spammer?(@spammer)) - assert_equal(true, @spammer.reload.is_banned) - assert_equal(1, @spammer.bans.count) - assert_match(/Spambot./, @spammer.bans.last.reason) - assert_match(/Spambot./, @spammer.feedback.last.body) + assert_equal(1, dmail.moderation_reports.count) + assert_equal(true, dmail.reload.is_deleted?) end end diff --git a/test/unit/spam_detector.rb b/test/unit/spam_detector.rb index 8a83e9b11..abaffde8e 100644 --- a/test/unit/spam_detector.rb +++ b/test/unit/spam_detector.rb @@ -16,7 +16,6 @@ class SpamDetectorTest < ActiveSupport::TestCase dmail = @user.dmails.last assert(SpamDetector.new(dmail).spam?) - assert(dmail.is_spam?) end should "not detect gold users as spammers" do @@ -24,7 +23,6 @@ class SpamDetectorTest < ActiveSupport::TestCase dmail = @spammer.dmails.last refute(SpamDetector.new(dmail).spam?) - refute(dmail.is_spam?) end should "not detect old users as spammers" do @@ -33,20 +31,27 @@ class SpamDetectorTest < ActiveSupport::TestCase dmail = @spammer.dmails.last refute(SpamDetector.new(dmail).spam?) - refute(dmail.is_spam?) end - should "log a message when spam is detected" do - Rails.logger.expects(:info) + should "generate a moderation report when spam is detected" do Dmail.create_split(from: @spammer, to: @user, title: "spam", body: "wonderful spam", creator_ip_addr: "127.0.0.1") + assert_equal(1, @user.dmails.last.moderation_reports.count) end should "pass messages through if akismet is down" do - Rakismet.expects(:akismet_call).raises(StandardError) + Rakismet.stubs(:akismet_call).raises(StandardError) dmail = create(:dmail, from: @spammer, to: @user, owner: @user, title: "spam", body: "wonderful spam", creator_ip_addr: "127.0.0.1") refute(SpamDetector.new(dmail).spam?) end + + should "autoban the user if they send too many spam dmails" do + count = SpamDetector::AUTOBAN_THRESHOLD + dmails = create_list(:dmail, count, from: @spammer, to: @user, owner: @user, creator_ip_addr: "127.0.0.1") + + assert_equal(count, ModerationReport.where(model: Dmail.sent_by(@spammer)).count) + assert_equal(true, @spammer.reload.is_banned?) + end end context "for forum posts" do @@ -54,45 +59,41 @@ class SpamDetectorTest < ActiveSupport::TestCase @forum_topic = as(@user) { create(:forum_topic) } end - should "detect spam" do + should "generate a moderation report when spam is detected" do as(@spammer) do - forum_post = build(:forum_post, topic: @forum_topic) - forum_post.validate + forum_post = create(:forum_post, creator: @spammer, topic: @forum_topic) assert(SpamDetector.new(forum_post, user_ip: "127.0.0.1").spam?) - assert(forum_post.invalid?) - assert_equal(["Failed to create forum post"], forum_post.errors.full_messages) + assert_equal(1, forum_post.moderation_reports.count) end end should "not detect gold users as spammers" do as(@user) do - forum_post = create(:forum_post, topic: @forum_topic) + forum_post = create(:forum_post, creator: @user, topic: @forum_topic) refute(SpamDetector.new(forum_post).spam?) - assert(forum_post.valid?) + assert_equal(0, forum_post.moderation_reports.count) end end end context "for comments" do - should "detect spam" do + should "generate a moderation report when spam is detected" do as(@spammer) do - comment = build(:comment) - comment.validate + comment = create(:comment, creator: @spammer) assert(SpamDetector.new(comment).spam?) - assert(comment.invalid?) - assert_equal(["Failed to create comment"], comment.errors.full_messages) + assert_equal(1, comment.moderation_reports.count) end end should "not detect gold users as spammers" do as(@user) do - comment = create(:comment) + comment = create(:comment, creator: @user) refute(SpamDetector.new(comment).spam?) - assert(comment.valid?) + assert_equal(0, comment.moderation_reports.count) end end end