From c4904aacc0634280518ea6c471bfe43cfcfd5e67 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Fri, 31 May 2013 11:41:40 -0400 Subject: [PATCH] Automatically flag someone as a spammer if their posts get at least X spam flags from N users while their trust level is 'new user'. Staff can clear and set this status from the user record in admin. --- .../javascripts/admin/models/admin_user.js | 22 ++ .../admin/templates/user.js.handlebars | 18 ++ app/controllers/admin/users_controller.rb | 39 +-- app/models/post.rb | 2 +- app/models/post_action.rb | 5 + app/models/site_setting.rb | 3 + app/serializers/admin_user_serializer.rb | 3 +- app/services/spam_rules_enforcer.rb | 64 +++++ config/locales/client.en.yml | 7 + config/locales/server.en.yml | 23 ++ config/routes.rb | 2 + .../20130603192412_add_blocked_to_users.rb | 5 + lib/guardian.rb | 18 +- spec/components/discourse_spec.rb | 2 +- spec/components/guardian_spec.rb | 2 +- .../admin/users_controller_spec.rb | 47 +++- spec/fabricators/user_fabricator.rb | 17 +- spec/integration/.gitkeep | 0 spec/integration/spam_rules_spec.rb | 103 ++++++++ spec/services/spam_rules_enforcer_spec.rb | 235 ++++++++++++++++++ 20 files changed, 585 insertions(+), 32 deletions(-) create mode 100644 app/services/spam_rules_enforcer.rb create mode 100644 db/migrate/20130603192412_add_blocked_to_users.rb delete mode 100644 spec/integration/.gitkeep create mode 100644 spec/integration/spam_rules_spec.rb create mode 100644 spec/services/spam_rules_enforcer_spec.rb diff --git a/app/assets/javascripts/admin/models/admin_user.js b/app/assets/javascripts/admin/models/admin_user.js index 5a63786c2..c1377849a 100644 --- a/app/assets/javascripts/admin/models/admin_user.js +++ b/app/assets/javascripts/admin/models/admin_user.js @@ -150,6 +150,28 @@ Discourse.AdminUser = Discourse.User.extend({ }); }, + unblock: function() { + Discourse.ajax('/admin/users/' + this.id + '/unblock', {type: 'PUT'}).then(function() { + // succeeded + window.location.reload(); + }, function(e) { + // failed + var error = Em.String.i18n('admin.user.unblock_failed', { error: "http: " + e.status + " - " + e.body }); + bootbox.alert(error); + }); + }, + + block: function() { + Discourse.ajax('/admin/users/' + this.id + '/block', {type: 'PUT'}).then(function() { + // succeeded + window.location.reload(); + }, function(e) { + // failed + var error = Em.String.i18n('admin.user.block_failed', { error: "http: " + e.status + " - " + e.body }); + bootbox.alert(error); + }); + }, + sendActivationEmail: function() { Discourse.ajax('/users/' + this.get('username') + '/send_activation_email').then(function() { // succeeded diff --git a/app/assets/javascripts/admin/templates/user.js.handlebars b/app/assets/javascripts/admin/templates/user.js.handlebars index ea2c2c106..ac283ddbe 100644 --- a/app/assets/javascripts/admin/templates/user.js.handlebars +++ b/app/assets/javascripts/admin/templates/user.js.handlebars @@ -161,6 +161,24 @@ {{/if}} +
+
{{i18n admin.user.blocked}}
+
{{blocked}}
+
+ {{#if blocked}} + + {{else}} + + {{/if}} + {{i18n admin.user.block_explanation}} +
+
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 77ea2653a..d35685cd0 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -2,6 +2,8 @@ require_dependency 'user_destroyer' class Admin::UsersController < Admin::AdminController + before_filter :fetch_user, only: [:ban, :unban, :refresh_browsers, :revoke_admin, :grant_admin, :revoke_moderation, :grant_moderation, :approve, :activate, :deactivate, :block, :unblock] + def index # Sort order if params[:query] == "active" @@ -35,7 +37,6 @@ class Admin::UsersController < Admin::AdminController end def ban - @user = User.where(id: params[:user_id]).first guardian.ensure_can_ban!(@user) @user.banned_till = params[:duration].to_i.days.from_now @user.banned_at = DateTime.now @@ -45,7 +46,6 @@ class Admin::UsersController < Admin::AdminController end def unban - @user = User.where(id: params[:user_id]).first guardian.ensure_can_ban!(@user) @user.banned_till = nil @user.banned_at = nil @@ -55,41 +55,35 @@ class Admin::UsersController < Admin::AdminController end def refresh_browsers - @user = User.where(id: params[:user_id]).first MessageBus.publish "/file-change", ["refresh"], user_ids: [@user.id] render nothing: true end def revoke_admin - @admin = User.where(id: params[:user_id]).first - guardian.ensure_can_revoke_admin!(@admin) - @admin.revoke_admin! + guardian.ensure_can_revoke_admin!(@user) + @user.revoke_admin! render nothing: true end def grant_admin - @user = User.where(id: params[:user_id]).first guardian.ensure_can_grant_admin!(@user) @user.grant_admin! render_serialized(@user, AdminUserSerializer) end def revoke_moderation - @moderator = User.where(id: params[:user_id]).first - guardian.ensure_can_revoke_moderation!(@moderator) - @moderator.revoke_moderation! + guardian.ensure_can_revoke_moderation!(@user) + @user.revoke_moderation! render nothing: true end def grant_moderation - @user = User.where(id: params[:user_id]).first guardian.ensure_can_grant_moderation!(@user) @user.grant_moderation! render_serialized(@user, AdminUserSerializer) end def approve - @user = User.where(id: params[:user_id]).first guardian.ensure_can_approve!(@user) @user.approve(current_user) render nothing: true @@ -103,19 +97,29 @@ class Admin::UsersController < Admin::AdminController end def activate - @user = User.where(id: params[:user_id]).first guardian.ensure_can_activate!(@user) @user.activate render nothing: true end def deactivate - @user = User.where(id: params[:user_id]).first guardian.ensure_can_deactivate!(@user) @user.deactivate render nothing: true end + def block + guardian.ensure_can_block_user! @user + SpamRulesEnforcer.punish! @user + render nothing: true + end + + def unblock + guardian.ensure_can_unblock_user! @user + SpamRulesEnforcer.clear @user + render nothing: true + end + def destroy user = User.where(id: params[:id]).first guardian.ensure_can_delete_user!(user) @@ -126,4 +130,11 @@ class Admin::UsersController < Admin::AdminController end end + + private + + def fetch_user + @user = User.where(id: params[:user_id]).first + end + end diff --git a/app/models/post.rb b/app/models/post.rb index bd20e8b96..992ae4d2b 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -49,7 +49,7 @@ class Post < ActiveRecord::Base scope :with_topic_subtype, ->(subtype) { joins(:topic).where('topics.subtype = ?', subtype) } def self.hidden_reasons - @hidden_reasons ||= Enum.new(:flag_threshold_reached, :flag_threshold_reached_again) + @hidden_reasons ||= Enum.new(:flag_threshold_reached, :flag_threshold_reached_again, :new_user_spam_threshold_reached) end def self.types diff --git a/app/models/post_action.rb b/app/models/post_action.rb index ca65b67ad..46991c3c5 100644 --- a/app/models/post_action.rb +++ b/app/models/post_action.rb @@ -19,6 +19,8 @@ class PostAction < ActiveRecord::Base validate :message_quality + scope :spam_flags, -> { where(post_action_type_id: PostActionType.types[:spam]) } + def self.update_flagged_posts_count posts_flagged_count = PostAction.joins(post: :topic) .where('post_actions.post_action_type_id' => PostActionType.notify_flag_type_ids, @@ -68,6 +70,7 @@ class PostAction < ActiveRecord::Base f = actions.map{|t| ["#{PostActionType.types[t]}_count", 0]} Post.with_deleted.update_all(Hash[*f.flatten], id: post.id) update_flagged_posts_count + # TODO: SpamRulesEnforcer.enforce!(post.user) end def self.act(user, post, post_action_type_id, opts={}) @@ -247,6 +250,8 @@ class PostAction < ActiveRecord::Base end end end + + SpamRulesEnforcer.enforce!(post.user) if post_action_type == :spam end def self.flagged_posts_report(filter) diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 65dcfaf88..c47782f04 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -58,6 +58,9 @@ class SiteSetting < ActiveRecord::Base setting(:flags_required_to_hide_post, 3) setting(:cooldown_minutes_after_hiding_posts, 10) + setting(:num_flags_to_block_new_user, 3) + setting(:num_users_to_block_new_user, 3) + # used mainly for dev, force hostname for Discourse.base_url # You would usually use multisite for this setting(:force_hostname, '') diff --git a/app/serializers/admin_user_serializer.rb b/app/serializers/admin_user_serializer.rb index b9d23a474..8858f2bbd 100644 --- a/app/serializers/admin_user_serializer.rb +++ b/app/serializers/admin_user_serializer.rb @@ -24,7 +24,8 @@ class AdminUserSerializer < BasicUserSerializer :ip_address, :can_send_activation_email, :can_activate, - :can_deactivate + :can_deactivate, + :blocked def is_banned object.is_banned? diff --git a/app/services/spam_rules_enforcer.rb b/app/services/spam_rules_enforcer.rb new file mode 100644 index 000000000..d1fa66bcc --- /dev/null +++ b/app/services/spam_rules_enforcer.rb @@ -0,0 +1,64 @@ +# The SpamRulesEnforcer class takes action against users based on flags that their posts +# receive, their trust level, etc. +class SpamRulesEnforcer + + # The exclamation point means that this method may make big changes to posts and the user. + def self.enforce!(user) + SpamRulesEnforcer.new(user).enforce! + end + + def self.block?(user) + SpamRulesEnforcer.new(user).block? + end + + def self.punish!(user) + SpamRulesEnforcer.new(user).punish_user + end + + def self.clear(user) + SpamRulesEnforcer.new(user).clear_user + end + + + def initialize(user) + @user = user + end + + def enforce! + punish_user if block? + true + end + + def block? + @user.blocked? or + (!@user.has_trust_level?(:basic) and + SiteSetting.num_flags_to_block_new_user > 0 and + SiteSetting.num_users_to_block_new_user > 0 and + num_spam_flags_against_user >= SiteSetting.num_flags_to_block_new_user and + num_users_who_flagged_spam_against_user >= SiteSetting.num_users_to_block_new_user) + end + + def num_spam_flags_against_user + Post.where(user_id: @user.id).sum(:spam_count) + end + + def num_users_who_flagged_spam_against_user + post_ids = Post.where('user_id = ? and spam_count > 0', @user.id).pluck(:id) + return 0 if post_ids.empty? + PostAction.spam_flags.where(post_id: post_ids).uniq.pluck(:user_id).size + end + + def punish_user + Post.update_all(["hidden = true, hidden_reason_id = COALESCE(hidden_reason_id, ?)", Post.hidden_reasons[:new_user_spam_threshold_reached]], user_id: @user.id) + SystemMessage.create(@user, :too_many_spam_flags) + @user.blocked = true + @user.save + end + + def clear_user + SystemMessage.create(@user, :unblocked) + @user.blocked = false + @user.save + end + +end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 9c608f702..fee4f6b93 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1048,6 +1048,7 @@ en: banned: "Banned?" moderator: "Moderator?" admin: "Admin?" + blocked: "Blocked?" show_admin_profile: "Admin" refresh_browsers: "Force browser refresh" show_public_profile: "Show Public Profile" @@ -1056,6 +1057,8 @@ en: grant_admin: 'Grant Admin' revoke_moderation: 'Revoke Moderation' grant_moderation: 'Grant Moderation' + unblock: 'Unblock' + block: 'Block' reputation: Reputation permissions: Permissions activity: Activity @@ -1081,6 +1084,10 @@ en: activate_failed: "There was a problem activating the user." deactivate_account: "Deactivate Account" deactivate_failed: "There was a problem deactivating the user." + unblock_failed: 'There was a problem unblocking the user.' + block_failed: 'There was a problem blocking the user.' + block_explanation: "A blocked user can't post or start topics." + site_content: none: "Choose a type of content to begin editing." diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 8b83e7906..2bcc530ec 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -470,6 +470,9 @@ en: flags_required_to_hide_post: "Number of flags that cause a post to be automatically hidden and PM sent to the user (0 for never)" cooldown_minutes_after_hiding_posts: "Number of minutes a user must wait before they can edit the post that was hidden due to flagging" + num_flags_to_block_new_user: "If a new user's posts get this many spam flags from (n) different users, hide all their posts and prevent future posting. 0 disables this feature." + num_users_to_block_new_user: "If a new user's posts get (x) spam flags from this many different users, hide all their posts and prevent future posting. 0 disables this feature." + traditional_markdown_linebreaks: "Use traditional linebreaks in Markdown, which require two trailing spaces for a linebreak" post_undo_action_window_mins: "Number of seconds users are allowed to reverse actions on a post (like, flag, etc)" @@ -836,6 +839,26 @@ en: subject_template: "Import completed successfully" text_body_template: "The import was successful." + too_many_spam_flags: + subject_template: "Posts hidden due to community flagging" + text_body_template: | + Hello, + + This is an automated message from %{site_name} to inform you that your posts have been hidden as spam due to community flagging. + + You will not be able to create any more posts unless a staff member determines that your posts are not spam. + + For additional guidance, please refer to our [FAQ](%{base_url}/faq). + + unblocked: + subject_template: "Account unblocked" + text_body_template: | + Hello, + + This is an automated message from %{site_name} to inform you that we have removed the blocked status of your account after the community flagged your post(s) as spam. + + You can start posting again. + unsubscribe_link: "If you'd like to unsubscribe from these emails, visit your [user preferences](%{user_preferences_url})." user_notifications: diff --git a/config/routes.rb b/config/routes.rb index c26799d47..f974c5c9e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -52,6 +52,8 @@ Discourse::Application.routes.draw do post 'refresh_browsers', constraints: AdminConstraint.new put 'activate' put 'deactivate' + put 'block' + put 'unblock' end resources :impersonate, constraints: AdminConstraint.new diff --git a/db/migrate/20130603192412_add_blocked_to_users.rb b/db/migrate/20130603192412_add_blocked_to_users.rb new file mode 100644 index 000000000..cf52fc74c --- /dev/null +++ b/db/migrate/20130603192412_add_blocked_to_users.rb @@ -0,0 +1,5 @@ +class AddBlockedToUsers < ActiveRecord::Migration + def change + add_column :users, :blocked, :boolean, default: false + end +end diff --git a/lib/guardian.rb b/lib/guardian.rb index d21bf0a95..a88402324 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -133,6 +133,14 @@ class Guardian can_administer?(user) && not(user.moderator?) end + def can_block_user?(user) + user && is_staff? && not(user.staff?) + end + + def can_unblock_user?(user) + user && is_staff? + end + def can_delete_user?(user_to_delete) can_administer?(user_to_delete) && user_to_delete.post_count <= 0 end @@ -211,8 +219,16 @@ class Guardian is_staff? end + def can_create_topic?(parent) + can_create_post?(parent) + end + + def can_create_post?(parent) + !SpamRulesEnforcer.block?(@user) + end + def can_create_post_on_topic?(topic) - is_staff? || not(topic.closed? || topic.archived?) + is_staff? || (not(topic.closed? || topic.archived?) && can_create_post?(topic)) end # Editing Methods diff --git a/spec/components/discourse_spec.rb b/spec/components/discourse_spec.rb index dad0b8e17..6426bdb20 100644 --- a/spec/components/discourse_spec.rb +++ b/spec/components/discourse_spec.rb @@ -51,7 +51,7 @@ describe Discourse do context '#system_user' do let!(:admin) { Fabricate(:admin) } - let!(:another_admin) { Fabricate(:another_admin) } + let!(:another_admin) { Fabricate(:admin) } it 'returns the user specified by the site setting system_username' do SiteSetting.stubs(:system_username).returns(another_admin.username) diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index ea75df262..9c042f2bc 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -7,7 +7,7 @@ describe Guardian do let(:user) { build(:user) } let(:moderator) { build(:moderator) } let(:admin) { build(:admin) } - let(:another_admin) { build(:another_admin) } + let(:another_admin) { build(:admin) } let(:coding_horror) { build(:coding_horror) } let(:topic) { build(:topic, user: user) } diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 0893caae6..5e1f142ab 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -81,7 +81,7 @@ describe Admin::UsersController do context '.revoke_admin' do before do - @another_admin = Fabricate(:another_admin) + @another_admin = Fabricate(:admin) end it 'raises an error unless the user can revoke access' do @@ -189,6 +189,51 @@ describe Admin::UsersController do end end + context 'block' do + before do + @user = Fabricate(:user) + end + + it "raises an error when the user doesn't have permission" do + Guardian.any_instance.expects(:can_block_user?).with(@user).returns(false) + SpamRulesEnforcer.expects(:punish!).never + xhr :put, :block, user_id: @user.id + response.should be_forbidden + end + + it "returns a 403 if the user doesn't exist" do + xhr :put, :block, user_id: 123123 + response.should be_forbidden + end + + it "punishes the user for spamming" do + SpamRulesEnforcer.expects(:punish!).with(@user) + xhr :put, :block, user_id: @user.id + end + end + + context 'unblock' do + before do + @user = Fabricate(:user) + end + + it "raises an error when the user doesn't have permission" do + Guardian.any_instance.expects(:can_unblock_user?).with(@user).returns(false) + xhr :put, :unblock, user_id: @user.id + response.should be_forbidden + end + + it "returns a 403 if the user doesn't exist" do + xhr :put, :unblock, user_id: 123123 + response.should be_forbidden + end + + it "punishes the user for spamming" do + SpamRulesEnforcer.expects(:clear).with(@user) + xhr :put, :unblock, user_id: @user.id + end + end + end end diff --git a/spec/fabricators/user_fabricator.rb b/spec/fabricators/user_fabricator.rb index 49a1d120e..149ece720 100644 --- a/spec/fabricators/user_fabricator.rb +++ b/spec/fabricators/user_fabricator.rb @@ -29,23 +29,16 @@ Fabricator(:walter_white, from: :user) do end Fabricator(:moderator, from: :user) do - name 'A. Moderator' - username 'moderator' - email 'moderator@discourse.org' + name { sequence(:name) {|i| "A#{i} Moderator"} } + username { sequence(:username) {|i| "moderator#{i}"} } + email { sequence(:email) {|i| "moderator#{i}@discourse.org"} } moderator true end Fabricator(:admin, from: :user) do name 'Anne Admin' - username 'anne' - email 'anne@discourse.org' - admin true -end - -Fabricator(:another_admin, from: :user) do - name 'Anne Admin the 2nd' - username 'anne2' - email 'anne2@discourse.org' + username { sequence(:username) {|i| "anne#{i}"} } + email { sequence(:email) {|i| "anne#{i}@discourse.org"} } admin true end diff --git a/spec/integration/.gitkeep b/spec/integration/.gitkeep deleted file mode 100644 index e69de29bb..000000000 diff --git a/spec/integration/spam_rules_spec.rb b/spec/integration/spam_rules_spec.rb new file mode 100644 index 000000000..283509310 --- /dev/null +++ b/spec/integration/spam_rules_spec.rb @@ -0,0 +1,103 @@ +# encoding: UTF-8 + +require 'spec_helper' + +describe PostAction do + + before do + SiteSetting.flags_required_to_hide_post = 0 # never + SiteSetting.num_flags_to_block_new_user = 2 + SiteSetting.num_users_to_block_new_user = 2 + end + + Given!(:admin) { Fabricate(:admin) } # needed to send a system message + Given(:user1) { Fabricate(:user) } + Given(:user2) { Fabricate(:user) } + + context 'spammer is a new user' do + Given(:spammer) { Fabricate(:user, trust_level: TrustLevel.levels[:newuser]) } + + context 'spammer post is not flagged enough times' do + Given!(:spam_post) { Fabricate(:post, user: spammer) } + Given!(:spam_post2) { Fabricate(:post, user: spammer) } + When { PostAction.act(user1, spam_post, PostActionType.types[:spam]) } + Then { expect(spam_post.reload).to_not be_hidden } + + context 'spam posts are flagged enough times, but not by enough users' do + When { PostAction.act(user1, spam_post2, PostActionType.types[:spam]) } + Then { expect(spam_post.reload).to_not be_hidden } + And { expect(spam_post2.reload).to_not be_hidden } + And { expect(spammer.reload).to_not be_blocked } + end + + context 'one spam post is flagged enough times by enough users' do + Given!(:another_topic) { Fabricate(:topic) } + Given!(:private_messages_count) { spammer.private_topics_count } + + When { PostAction.act(user2, spam_post, PostActionType.types[:spam]) } + + Invariant { expect(Guardian.new(spammer).can_create_topic?(nil)).to be_false } + Invariant { expect{PostCreator.create(spammer, {title: 'limited time offer for you', raw: 'better buy this stuff ok', archetype_id: 1})}.to raise_error(Discourse::InvalidAccess) } + Invariant { expect{PostCreator.create(spammer, {topic_id: another_topic.id, raw: 'my reply is spam in your topic', archetype_id: 1})}.to raise_error(Discourse::InvalidAccess) } + + Then { expect(spammer.reload).to be_blocked } + And { expect(spam_post.reload).to be_hidden } + And { expect(spam_post2.reload).to be_hidden } + And { expect(spammer.reload.private_topics_count).to eq(private_messages_count + 1) } + + + # The following cases describe when a staff user takes some action, but the user + # still won't be able to make posts. + # A staff user needs to clear the spam flag from the user record. + + context "a post's flags are cleared" do + When { PostAction.clear_flags!(spam_post, admin); spammer.reload } + Then { expect(spammer.reload).to be_blocked } + end + + context "a post is deleted" do + When { spam_post.trash!; spammer.reload } + Then { expect(spammer.reload).to be_blocked } + end + + context "spammer becomes a basic user" do + When { spammer.change_trust_level!(:basic); spammer.reload } + Then { expect(spammer.reload).to be_blocked } + end + end + + context 'flags_required_to_hide_post takes effect too' do + Given { SiteSetting.stubs(:flags_required_to_hide_post).returns(2) } + When { PostAction.act(user2, spam_post, PostActionType.types[:spam]) } + Then { expect(spammer.reload).to be_blocked } + And { expect(Guardian.new(spammer).can_create_topic?(nil)).to be_false } + end + end + end + + context "spammer has trust level basic" do + Given(:spammer) { Fabricate(:user, trust_level: TrustLevel.levels[:basic]) } + + context 'one spam post is flagged enough times by enough users' do + Given!(:spam_post) { Fabricate(:post, user: spammer) } + Given!(:private_messages_count) { spammer.private_topics_count } + When { PostAction.act(user1, spam_post, PostActionType.types[:spam]) } + When { PostAction.act(user2, spam_post, PostActionType.types[:spam]) } + Then { expect(spam_post.reload).to_not be_hidden } + And { expect(Guardian.new(spammer).can_create_topic?(nil)).to be_true } + And { expect{PostCreator.create(spammer, {title: 'limited time offer for you', raw: 'better buy this stuff ok', archetype_id: 1})}.to_not raise_error } + And { expect(spammer.reload.private_topics_count).to eq(private_messages_count) } + end + end + + [[:user, trust_level: TrustLevel.levels[:regular]], [:admin], [:moderator]].each do |spammer_args| + context "spammer is trusted #{spammer_args[0]}" do + Given!(:spammer) { Fabricate(*spammer_args) } + Given!(:spam_post) { Fabricate(:post, user: spammer) } + Given!(:private_messages_count) { spammer.private_topics_count } + When { PostAction.act(user1, spam_post, PostActionType.types[:spam]) } + When { PostAction.act(user2, spam_post, PostActionType.types[:spam]) } + Then { expect(spam_post.reload).to_not be_hidden } + end + end +end diff --git a/spec/services/spam_rules_enforcer_spec.rb b/spec/services/spam_rules_enforcer_spec.rb new file mode 100644 index 000000000..29f691990 --- /dev/null +++ b/spec/services/spam_rules_enforcer_spec.rb @@ -0,0 +1,235 @@ +require 'spec_helper' + +describe SpamRulesEnforcer do + + before do + SiteSetting.stubs(:flags_required_to_hide_post).returns(0) # never + SiteSetting.stubs(:num_flags_to_block_new_user).returns(2) + SiteSetting.stubs(:num_users_to_block_new_user).returns(2) + end + + describe 'enforce!' do + let(:post) { Fabricate.build(:post, user: Fabricate.build(:user, trust_level: TrustLevel.levels[:newuser])) } + subject { SpamRulesEnforcer.new(post.user) } + + it "does nothing if the user's trust level is higher than 'new user'" do + basic_user = Fabricate.build(:user, trust_level: TrustLevel.levels[:basic]) + enforcer = SpamRulesEnforcer.new(basic_user) + enforcer.expects(:num_spam_flags_against_user).never + enforcer.expects(:num_users_who_flagged_spam_against_user).never + enforcer.expects(:punish_user).never + enforcer.enforce! + end + + it 'takes no action if not enough flags by enough users have been submitted' do + subject.stubs(:block?).returns(false) + subject.expects(:punish_user).never + subject.enforce! + end + + it 'delivers punishment when there are enough flags from enough users' do + subject.stubs(:block?).returns(true) + subject.expects(:punish_user) + subject.enforce! + end + end + + describe 'num_spam_flags_against_user' do + before { SpamRulesEnforcer.any_instance.stubs(:punish_user) } + let(:post) { Fabricate(:post) } + let(:enforcer) { SpamRulesEnforcer.new(post.user) } + subject { enforcer.num_spam_flags_against_user } + + it 'returns 0 when there are no flags' do + expect(subject).to eq(0) + end + + it 'returns 0 when there is one flag that has a reason other than spam' do + Fabricate(:flag, post: post, post_action_type_id: PostActionType.types[:off_topic]) + expect(subject).to eq(0) + end + + it 'returns 2 when there are two flags with spam as the reason' do + 2.times { Fabricate(:flag, post: post, post_action_type_id: PostActionType.types[:spam]) } + expect(subject).to eq(2) + end + + it 'returns 2 when there are two spam flags, each on a different post' do + Fabricate(:flag, post: post, post_action_type_id: PostActionType.types[:spam]) + Fabricate(:flag, post: Fabricate(:post, user: post.user), post_action_type_id: PostActionType.types[:spam]) + expect(subject).to eq(2) + end + end + + describe 'num_users_who_flagged_spam_against_user' do + before { SpamRulesEnforcer.any_instance.stubs(:punish_user) } + let(:post) { Fabricate(:post) } + let(:enforcer) { SpamRulesEnforcer.new(post.user) } + subject { enforcer.num_users_who_flagged_spam_against_user } + + it 'returns 0 when there are no flags' do + expect(subject).to eq(0) + end + + it 'returns 0 when there is one flag that has a reason other than spam' do + Fabricate(:flag, post: post, post_action_type_id: PostActionType.types[:off_topic]) + expect(subject).to eq(0) + end + + it 'returns 1 when there is one spam flag' do + Fabricate(:flag, post: post, post_action_type_id: PostActionType.types[:spam]) + expect(subject).to eq(1) + end + + it 'returns 2 when there are two spam flags from 2 users' do + Fabricate(:flag, post: post, post_action_type_id: PostActionType.types[:spam]) + Fabricate(:flag, post: post, post_action_type_id: PostActionType.types[:spam]) + expect(subject).to eq(2) + end + + it 'returns 1 when there are two spam flags on two different posts from 1 user' do + flagger = Fabricate(:user) + Fabricate(:flag, post: post, user: flagger, post_action_type_id: PostActionType.types[:spam]) + Fabricate(:flag, post: Fabricate(:post, user: post.user), user: flagger, post_action_type_id: PostActionType.types[:spam]) + expect(subject).to eq(1) + end + end + + describe 'punish_user' do + let!(:admin) { Fabricate(:admin) } # needed for SystemMessage + let(:user) { Fabricate(:user) } + let!(:post) { Fabricate(:post, user: user) } + subject { SpamRulesEnforcer.new(user) } + + before do + SpamRulesEnforcer.stubs(:block?).with {|u| u.id != user.id }.returns(false) + SpamRulesEnforcer.stubs(:block?).with {|u| u.id == user.id }.returns(true) + subject.stubs(:block?).returns(true) + end + + it "hides all the user's posts" do + subject.punish_user + expect(post.reload).to be_hidden + end + + it 'prevents the user from making new posts' do + subject.punish_user + expect(Guardian.new(user).can_create_post?(nil)).to be_false + end + + it 'sends a system message to the user' do + SystemMessage.expects(:create).with(user, anything, anything) + subject.punish_user + end + + it 'sets the blocked flag' do + subject.punish_user + expect(user.reload.blocked).to be_true + end + end + + describe 'block?' do + + context 'never been blocked' do + shared_examples "can't be blocked" do + it "returns false" do + enforcer = SpamRulesEnforcer.new(user) + enforcer.expects(:num_spam_flags_against_user).never + enforcer.expects(:num_users_who_flagged_spam_against_user).never + expect(enforcer.block?).to be_false + end + end + + [:basic, :regular, :leader, :elder].each do |trust_level| + context "user has trust level #{trust_level}" do + let(:user) { Fabricate(:user, trust_level: TrustLevel.levels[trust_level]) } + include_examples "can't be blocked" + end + end + + context "user is an admin" do + let(:user) { Fabricate(:admin) } + include_examples "can't be blocked" + end + + context "user is a moderator" do + let(:user) { Fabricate(:moderator) } + include_examples "can't be blocked" + end + end + + context 'new user' do + let(:user) { Fabricate(:user, trust_level: TrustLevel.levels[:newuser]) } + subject { SpamRulesEnforcer.new(user) } + + it 'returns false if there are no spam flags' do + subject.stubs(:num_spam_flags_against_user).returns(0) + subject.stubs(:num_users_who_flagged_spam_against_user).returns(0) + expect(subject.block?).to be_false + end + + it 'returns false if there are not received enough flags' do + subject.stubs(:num_spam_flags_against_user).returns(1) + subject.stubs(:num_users_who_flagged_spam_against_user).returns(2) + expect(subject.block?).to be_false + end + + it 'returns false if there have not been enough users' do + subject.stubs(:num_spam_flags_against_user).returns(2) + subject.stubs(:num_users_who_flagged_spam_against_user).returns(1) + expect(subject.block?).to be_false + end + + it 'returns false if num_flags_to_block_new_user is 0' do + SiteSetting.stubs(:num_flags_to_block_new_user).returns(0) + subject.stubs(:num_spam_flags_against_user).returns(100) + subject.stubs(:num_users_who_flagged_spam_against_user).returns(100) + expect(subject.block?).to be_false + end + + it 'returns false if num_users_to_block_new_user is 0' do + SiteSetting.stubs(:num_users_to_block_new_user).returns(0) + subject.stubs(:num_spam_flags_against_user).returns(100) + subject.stubs(:num_users_who_flagged_spam_against_user).returns(100) + expect(subject.block?).to be_false + end + + it 'returns true when there are enough flags from enough users' do + subject.stubs(:num_spam_flags_against_user).returns(2) + subject.stubs(:num_users_who_flagged_spam_against_user).returns(2) + expect(subject.block?).to be_true + end + end + + context "blocked, but has higher trust level now" do + let(:user) { Fabricate(:user, blocked: true, trust_level: TrustLevel.levels[:basic]) } + subject { SpamRulesEnforcer.new(user) } + + it 'returns false' do + expect(subject.block?).to be_true + end + end + end + + describe "clear_user" do + let!(:admin) { Fabricate(:admin) } # needed for SystemMessage + let(:user) { Fabricate(:user) } + subject { SpamRulesEnforcer.new(user) } + + it 'sets blocked flag to false' do + subject.clear_user + expect(user.reload).to_not be_blocked + end + + it 'sends a system message' do + SystemMessage.expects(:create).with(user, anything, anything) + subject.clear_user + end + + it 'allows user to make new posts' do + subject.clear_user + expect(Guardian.new(user).can_create_post?(nil)).to be_true + end + end + +end