From 4e46d91b8dafcb340b47d73b84e04e035b466a47 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Fri, 25 Oct 2013 13:25:02 -0400 Subject: [PATCH] Refactor SpamRulesEnforcer so that each spam rule is in its own class --- app/services/spam_rule/auto_block.rb | 45 +++ app/services/spam_rule/flag_sockpuppets.rb | 37 +++ app/services/spam_rules_enforcer.rb | 66 +---- lib/guardian.rb | 2 +- spec/services/auto_block_spec.rb | 221 +++++++++++++++ spec/services/flag_sockpuppets_spec.rb | 145 ++++++++++ spec/services/spam_rules_enforcer_spec.rb | 312 +-------------------- 7 files changed, 462 insertions(+), 366 deletions(-) create mode 100644 app/services/spam_rule/auto_block.rb create mode 100644 app/services/spam_rule/flag_sockpuppets.rb create mode 100644 spec/services/auto_block_spec.rb create mode 100644 spec/services/flag_sockpuppets_spec.rb diff --git a/app/services/spam_rule/auto_block.rb b/app/services/spam_rule/auto_block.rb new file mode 100644 index 00000000000..9cc67a6efd2 --- /dev/null +++ b/app/services/spam_rule/auto_block.rb @@ -0,0 +1,45 @@ +class SpamRule::AutoBlock + + def initialize(user) + @user = user + end + + def self.block?(user) + self.new(user).block? + end + + def self.punish!(user) + self.new(user).block_user + end + + def perform + block_user if block? + 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 block_user + Post.transaction do + if UserBlocker.block(@user, nil, {message: :too_many_spam_flags}) and SiteSetting.notify_mods_when_user_blocked + GroupMessage.create(Group[:moderators].name, :user_automatically_blocked, {user: @user, limit_once_per: false}) + end + end + end +end \ No newline at end of file diff --git a/app/services/spam_rule/flag_sockpuppets.rb b/app/services/spam_rule/flag_sockpuppets.rb new file mode 100644 index 00000000000..813ab1d8b82 --- /dev/null +++ b/app/services/spam_rule/flag_sockpuppets.rb @@ -0,0 +1,37 @@ +class SpamRule::FlagSockpuppets + + def initialize(post) + @post = post + end + + def perform + if SiteSetting.flag_sockpuppets and reply_is_from_sockpuppet? + flag_sockpuppet_users + true + else + false + end + end + + def reply_is_from_sockpuppet? + return false if @post.post_number and @post.post_number == 1 + + first_post = @post.topic.posts.by_post_number.first + return false if first_post.user.nil? + + !first_post.user.staff? and !@post.user.staff? and + @post.user != first_post.user and + @post.user.ip_address == first_post.user.ip_address and + @post.user.new_user? and + !ScreenedIpAddress.is_whitelisted?(@post.user.ip_address) + end + + def flag_sockpuppet_users + system_user = Discourse.system_user + PostAction.act(system_user, @post, PostActionType.types[:spam], message: I18n.t('flag_reason.sockpuppet')) rescue PostAction::AlreadyActed + if (first_post = @post.topic.posts.by_post_number.first).try(:user).try(:new_user?) + PostAction.act(system_user, first_post, PostActionType.types[:spam], message: I18n.t('flag_reason.sockpuppet')) rescue PostAction::AlreadyActed + end + end + +end \ No newline at end of file diff --git a/app/services/spam_rules_enforcer.rb b/app/services/spam_rules_enforcer.rb index 16475b267e5..9de7c851fae 100644 --- a/app/services/spam_rules_enforcer.rb +++ b/app/services/spam_rules_enforcer.rb @@ -2,8 +2,6 @@ # receive, their trust level, etc. class SpamRulesEnforcer - include Rails.application.routes.url_helpers - # The exclamation point means that this method may make big changes to posts and users. def self.enforce!(arg) SpamRulesEnforcer.new(arg).enforce! @@ -15,73 +13,13 @@ class SpamRulesEnforcer end def enforce! - # TODO: once rules are in their own classes, invoke them from here in priority order if @user - block_user if block? + SpamRule::AutoBlock.new(@user).perform end if @post - flag_sockpuppet_users if SiteSetting.flag_sockpuppets and reply_is_from_sockpuppet? + SpamRule::FlagSockpuppets.new(@post).perform end true end - # TODO: move this sockpuppet code to its own class. We should be able to add more rules, like ActiveModel validators. - def reply_is_from_sockpuppet? - return false if @post.post_number and @post.post_number == 1 - - first_post = @post.topic.posts.by_post_number.first - return false if first_post.user.nil? - - !first_post.user.staff? and !@post.user.staff? and - @post.user != first_post.user and - @post.user.ip_address == first_post.user.ip_address and - @post.user.new_user? and - !ScreenedIpAddress.is_whitelisted?(@post.user.ip_address) - end - - def flag_sockpuppet_users - system_user = Discourse.system_user - PostAction.act(system_user, @post, PostActionType.types[:spam], message: I18n.t('flag_reason.sockpuppet')) rescue PostAction::AlreadyActed - if (first_post = @post.topic.posts.by_post_number.first).try(:user).try(:new_user?) - PostAction.act(system_user, first_post, PostActionType.types[:spam], message: I18n.t('flag_reason.sockpuppet')) rescue PostAction::AlreadyActed - end - end - - # TODO: move all this auto-block code to another class: - def self.block?(user) - SpamRulesEnforcer.new(user).block? - end - - def self.punish!(user) - SpamRulesEnforcer.new(user).block_user - 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 block_user - Post.transaction do - if UserBlocker.block(@user, nil, {message: :too_many_spam_flags}) and SiteSetting.notify_mods_when_user_blocked - GroupMessage.create(Group[:moderators].name, :user_automatically_blocked, {user: @user, limit_once_per: false}) - end - end - end - - end diff --git a/lib/guardian.rb b/lib/guardian.rb index 060895c9475..0c44e075fb8 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -252,7 +252,7 @@ class Guardian end def can_create_post?(parent) - !SpamRulesEnforcer.block?(@user) && ( + !SpamRule::AutoBlock.block?(@user) && ( !parent || !parent.category || Category.post_create_allowed(self).where(:id => parent.category.id).count == 1 diff --git a/spec/services/auto_block_spec.rb b/spec/services/auto_block_spec.rb new file mode 100644 index 00000000000..ca4309a395f --- /dev/null +++ b/spec/services/auto_block_spec.rb @@ -0,0 +1,221 @@ +require 'spec_helper' + +describe SpamRule::AutoBlock 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 'perform' do + let(:post) { Fabricate.build(:post, user: Fabricate.build(:user, trust_level: TrustLevel.levels[:newuser])) } + subject { described_class.new(post.user) } + + it 'takes no action if user should not be blocked' do + subject.stubs(:block?).returns(false) + subject.expects(:block_user).never + subject.perform + end + + it 'delivers punishment when user should be blocked' do + subject.stubs(:block?).returns(true) + subject.expects(:block_user) + subject.perform + end + end + + describe 'num_spam_flags_against_user' do + before { described_class.any_instance.stubs(:block_user) } + let(:post) { Fabricate(:post) } + let(:enforcer) { described_class.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 { described_class.any_instance.stubs(:block_user) } + let(:post) { Fabricate(:post) } + let(:enforcer) { described_class.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 'block_user' do + let!(:admin) { Fabricate(:admin) } # needed for SystemMessage + let(:user) { Fabricate(:user) } + let!(:post) { Fabricate(:post, user: user) } + subject { described_class.new(user) } + + before do + described_class.stubs(:block?).with {|u| u.id != user.id }.returns(false) + described_class.stubs(:block?).with {|u| u.id == user.id }.returns(true) + subject.stubs(:block?).returns(true) + end + + context 'user is not blocked' do + before do + UserBlocker.expects(:block).with(user, nil, has_entries(message: :too_many_spam_flags)).returns(true) + end + + it 'prevents the user from making new posts' do + subject.block_user + expect(Guardian.new(user).can_create_post?(nil)).to be_false + end + + it 'sends private message to moderators' do + SiteSetting.stubs(:notify_mods_when_user_blocked).returns(true) + moderator = Fabricate(:moderator) + GroupMessage.expects(:create).with do |group, msg_type, params| + group == Group[:moderators].name and msg_type == :user_automatically_blocked and params[:user].id == user.id + end + subject.block_user + end + + it "doesn't send a pm to moderators if notify_mods_when_user_blocked is false" do + SiteSetting.stubs(:notify_mods_when_user_blocked).returns(false) + GroupMessage.expects(:create).never + subject.block_user + end + end + + context 'user is already blocked' do + before do + UserBlocker.expects(:block).with(user, nil, has_entries(message: :too_many_spam_flags)).returns(false) + end + + it "doesn't send a pm to moderators if the user is already blocked" do + GroupMessage.expects(:create).never + subject.block_user + end + end + end + + describe 'block?' do + + context 'never been blocked' do + shared_examples "can't be blocked" do + it "returns false" do + enforcer = described_class.new(user) + enforcer.expects(:num_spam_flags_against_user).never + enforcer.expects(:num_users_who_flagged_spam_against_user).never + enforcer.block?.should eq(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 { described_class.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 { described_class.new(user) } + + it 'returns false' do + expect(subject.block?).to be_true + end + end + end +end diff --git a/spec/services/flag_sockpuppets_spec.rb b/spec/services/flag_sockpuppets_spec.rb new file mode 100644 index 00000000000..e50254adb0f --- /dev/null +++ b/spec/services/flag_sockpuppets_spec.rb @@ -0,0 +1,145 @@ +require 'spec_helper' + +describe SpamRule::FlagSockpuppets do + + let(:user1) { Fabricate(:user, ip_address: '182.189.119.174') } + let(:post1) { Fabricate(:post, user: user1, topic: Fabricate(:topic, user: user1)) } + + describe 'perform' do + let(:rule) { described_class.new(post1) } + subject(:perform) { rule.perform } + + it 'does nothing if flag_sockpuppets is disabled' do + SiteSetting.stubs(:flag_sockpuppets).returns(false) + rule.expects(:reply_is_from_sockpuppet?).never + rule.expects(:flag_sockpuppet_users).never + perform.should eq(false) + end + + context 'flag_sockpuppets is enabled' do + before { SiteSetting.stubs(:flag_sockpuppets).returns(true) } + + it 'flags posts when it should' do + rule.expects(:reply_is_from_sockpuppet?).returns(:true) + rule.expects(:flag_sockpuppet_users).once + perform.should eq(true) + end + + it "doesn't flag posts when it shouldn't" do + rule.expects(:reply_is_from_sockpuppet?).returns(false) + rule.expects(:flag_sockpuppet_users).never + perform.should eq(false) + end + end + end + + describe 'reply_is_from_sockpuppet?' do + it 'is false for the first post in a topic' do + described_class.new(post1).reply_is_from_sockpuppet?.should eq(false) + end + + it 'is false if users have different IP addresses' do + post2 = Fabricate(:post, user: Fabricate(:user, ip_address: '182.189.199.199'), topic: post1.topic) + described_class.new(post2).reply_is_from_sockpuppet?.should eq(false) + end + + it 'is true if users have the same IP address and are new' do + post2 = Fabricate(:post, user: Fabricate(:user, ip_address: user1.ip_address), topic: post1.topic) + described_class.new(post2).reply_is_from_sockpuppet?.should eq(true) + end + + it 'is false if the ip address is whitelisted' do + ScreenedIpAddress.stubs(:is_whitelisted?).with(user1.ip_address).returns(true) + post2 = Fabricate(:post, user: Fabricate(:user, ip_address: user1.ip_address), topic: post1.topic) + described_class.new(post2).reply_is_from_sockpuppet?.should eq(false) + end + + it 'is false if reply and first post are from the same user' do + post2 = Fabricate(:post, user: user1, topic: post1.topic) + described_class.new(post2).reply_is_from_sockpuppet?.should eq(false) + end + + it 'is false if first post user is staff' do + staff1 = Fabricate(:admin, ip_address: '182.189.119.174') + staff_post1 = Fabricate(:post, user: staff1, topic: Fabricate(:topic, user: staff1)) + post2 = Fabricate(:post, user: Fabricate(:user, ip_address: staff1.ip_address), topic: staff_post1.topic) + described_class.new(post2).reply_is_from_sockpuppet?.should eq(false) + end + + it 'is false if second post user is staff' do + post2 = Fabricate(:post, user: Fabricate(:moderator, ip_address: user1.ip_address), topic: post1.topic) + described_class.new(post2).reply_is_from_sockpuppet?.should eq(false) + end + + it 'is false if both users are staff' do + staff1 = Fabricate(:moderator, ip_address: '182.189.119.174') + staff_post1 = Fabricate(:post, user: staff1, topic: Fabricate(:topic, user: staff1)) + post2 = Fabricate(:post, user: Fabricate(:admin, ip_address: staff1.ip_address), topic: staff_post1.topic) + described_class.new(post2).reply_is_from_sockpuppet?.should eq(false) + end + + it 'is true if first post user was created over 24 hours ago and has trust level higher than 0' do + old_user = Fabricate(:user, ip_address: '182.189.119.174', created_at: 25.hours.ago, trust_level: TrustLevel.levels[:basic]) + first_post = Fabricate(:post, user: old_user, topic: Fabricate(:topic, user: old_user)) + post2 = Fabricate(:post, user: Fabricate(:user, ip_address: old_user.ip_address), topic: first_post.topic) + described_class.new(post2).reply_is_from_sockpuppet?.should eq(true) + end + + it 'is false if second post user was created over 24 hours ago and has trust level higher than 0' do + post2 = Fabricate(:post, user: Fabricate(:user, ip_address: user1.ip_address, created_at: 25.hours.ago, trust_level: TrustLevel.levels[:basic]), topic: post1.topic) + described_class.new(post2).reply_is_from_sockpuppet?.should eq(false) + end + + it 'is true if first post user was created less that 24 hours ago and has trust level higher than 0' do + new_user = Fabricate(:user, ip_address: '182.189.119.174', created_at: 1.hour.ago, trust_level: TrustLevel.levels[:basic]) + first_post = Fabricate(:post, user: new_user, topic: Fabricate(:topic, user: new_user)) + post2 = Fabricate(:post, user: Fabricate(:user, ip_address: new_user.ip_address), topic: first_post.topic) + described_class.new(post2).reply_is_from_sockpuppet?.should eq(true) + end + + it 'is true if second user was created less that 24 hours ago and has trust level higher than 0' do + post2 = Fabricate(:post, user: Fabricate(:user, ip_address: user1.ip_address, created_at: 23.hours.ago, trust_level: TrustLevel.levels[:basic]), topic: post1.topic) + described_class.new(post2).reply_is_from_sockpuppet?.should eq(true) + end + + # A weird case + it 'is false when user is nil on first post' do + post1.user = nil; post1.save! + post2 = Fabricate(:post, user: Fabricate(:user), topic: post1.topic) + described_class.new(post2).reply_is_from_sockpuppet?.should eq(false) + end + end + + describe 'flag_sockpuppet_users' do + let(:post2) { Fabricate(:post, user: Fabricate(:user, ip_address: user1.ip_address), topic: post1.topic) } + + it 'flags post and first post if both users are new' do + PostAction.expects(:act).with(Discourse.system_user, post1, PostActionType.types[:spam], anything).once + PostAction.expects(:act).with(Discourse.system_user, post2, PostActionType.types[:spam], anything).once + described_class.new(post2).flag_sockpuppet_users + end + + it "doesn't flag the first post more than once" do + PostAction.expects(:act).with(Discourse.system_user, post2, PostActionType.types[:spam], anything).once + PostAction.stubs(:act).with(Discourse.system_user, post1, PostActionType.types[:spam], anything).raises(PostAction::AlreadyActed) + described_class.new(post2).flag_sockpuppet_users + end + + it "doesn't flag the first post if the user is not new" do + old_user = Fabricate(:user, ip_address: '182.189.119.174', created_at: 25.hours.ago, trust_level: TrustLevel.levels[:basic]) + first_post = Fabricate(:post, user: old_user, topic: Fabricate(:topic, user: old_user)) + post2 = Fabricate(:post, user: Fabricate(:user, ip_address: old_user.ip_address), topic: first_post.topic) + PostAction.expects(:act).with(anything, post2, anything, anything).once + PostAction.expects(:act).with(anything, first_post, anything, anything).never + described_class.new(post2).flag_sockpuppet_users + end + + it "doesn't create a flag if user is nil on first post" do + post1.user_id = nil + post1.save + PostAction.expects(:act).with(anything, post2, anything, anything).once + PostAction.expects(:act).with(anything, post1, anything, anything).never + described_class.new(post2).flag_sockpuppet_users + end + end +end diff --git a/spec/services/spam_rules_enforcer_spec.rb b/spec/services/spam_rules_enforcer_spec.rb index f68ca3cd587..0d60c465471 100644 --- a/spec/services/spam_rules_enforcer_spec.rb +++ b/spec/services/spam_rules_enforcer_spec.rb @@ -6,312 +6,22 @@ describe SpamRulesEnforcer do SystemMessage.stubs(:create) end - context 'flagging posts based on IP address of users' do - describe 'reply_is_from_sockpuppet?' do - let(:user1) { Fabricate(:user, ip_address: '182.189.119.174') } - let(:post1) { Fabricate(:post, user: user1, topic: Fabricate(:topic, user: user1)) } + describe 'enforce!' do + context 'post argument' do + subject(:enforce) { described_class.enforce!(Fabricate.build(:post)) } - it 'is false for the first post in a topic' do - SpamRulesEnforcer.new(post1).reply_is_from_sockpuppet?.should eq(false) - end - - it 'is false if users have different IP addresses' do - post2 = Fabricate(:post, user: Fabricate(:user, ip_address: '182.189.199.199'), topic: post1.topic) - SpamRulesEnforcer.new(post2).reply_is_from_sockpuppet?.should eq(false) - end - - it 'is true if users have the same IP address and are new' do - post2 = Fabricate(:post, user: Fabricate(:user, ip_address: user1.ip_address), topic: post1.topic) - SpamRulesEnforcer.new(post2).reply_is_from_sockpuppet?.should eq(true) - end - - it 'is false if the ip address is whitelisted' do - ScreenedIpAddress.stubs(:is_whitelisted?).with(user1.ip_address).returns(true) - post2 = Fabricate(:post, user: Fabricate(:user, ip_address: user1.ip_address), topic: post1.topic) - SpamRulesEnforcer.new(post2).reply_is_from_sockpuppet?.should eq(false) - end - - it 'is false if reply and first post are from the same user' do - post2 = Fabricate(:post, user: user1, topic: post1.topic) - SpamRulesEnforcer.new(post2).reply_is_from_sockpuppet?.should eq(false) - end - - it 'is false if first post user is staff' do - staff1 = Fabricate(:admin, ip_address: '182.189.119.174') - staff_post1 = Fabricate(:post, user: staff1, topic: Fabricate(:topic, user: staff1)) - post2 = Fabricate(:post, user: Fabricate(:user, ip_address: staff1.ip_address), topic: staff_post1.topic) - SpamRulesEnforcer.new(post2).reply_is_from_sockpuppet?.should eq(false) - end - - it 'is false if second post user is staff' do - post2 = Fabricate(:post, user: Fabricate(:moderator, ip_address: user1.ip_address), topic: post1.topic) - SpamRulesEnforcer.new(post2).reply_is_from_sockpuppet?.should eq(false) - end - - it 'is false if both users are staff' do - staff1 = Fabricate(:moderator, ip_address: '182.189.119.174') - staff_post1 = Fabricate(:post, user: staff1, topic: Fabricate(:topic, user: staff1)) - post2 = Fabricate(:post, user: Fabricate(:admin, ip_address: staff1.ip_address), topic: staff_post1.topic) - SpamRulesEnforcer.new(post2).reply_is_from_sockpuppet?.should eq(false) - end - - it 'is true if first post user was created over 24 hours ago and has trust level higher than 0' do - old_user = Fabricate(:user, ip_address: '182.189.119.174', created_at: 25.hours.ago, trust_level: TrustLevel.levels[:basic]) - first_post = Fabricate(:post, user: old_user, topic: Fabricate(:topic, user: old_user)) - post2 = Fabricate(:post, user: Fabricate(:user, ip_address: old_user.ip_address), topic: first_post.topic) - SpamRulesEnforcer.new(post2).reply_is_from_sockpuppet?.should eq(true) - end - - it 'is false if second post user was created over 24 hours ago and has trust level higher than 0' do - post2 = Fabricate(:post, user: Fabricate(:user, ip_address: user1.ip_address, created_at: 25.hours.ago, trust_level: TrustLevel.levels[:basic]), topic: post1.topic) - SpamRulesEnforcer.new(post2).reply_is_from_sockpuppet?.should eq(false) - end - - it 'is true if first post user was created less that 24 hours ago and has trust level higher than 0' do - new_user = Fabricate(:user, ip_address: '182.189.119.174', created_at: 1.hour.ago, trust_level: TrustLevel.levels[:basic]) - first_post = Fabricate(:post, user: new_user, topic: Fabricate(:topic, user: new_user)) - post2 = Fabricate(:post, user: Fabricate(:user, ip_address: new_user.ip_address), topic: first_post.topic) - SpamRulesEnforcer.new(post2).reply_is_from_sockpuppet?.should eq(true) - end - - it 'is true if second user was created less that 24 hours ago and has trust level higher than 0' do - post2 = Fabricate(:post, user: Fabricate(:user, ip_address: user1.ip_address, created_at: 23.hours.ago, trust_level: TrustLevel.levels[:basic]), topic: post1.topic) - SpamRulesEnforcer.new(post2).reply_is_from_sockpuppet?.should eq(true) - end - - # A weird case - it 'is false when user is nil on first post' do - post1.user = nil; post1.save! - post2 = Fabricate(:post, user: Fabricate(:user), topic: post1.topic) - SpamRulesEnforcer.new(post2).reply_is_from_sockpuppet?.should eq(false) - end - end - end - - context 'auto-blocking users based on flags' 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(:block_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(:block_user).never - subject.enforce! - end - - it 'delivers punishment when there are enough flags from enough users' do - subject.stubs(:block?).returns(true) - subject.expects(:block_user) - subject.enforce! + it 'performs the FlagSockpuppetRule' do + SpamRule::FlagSockpuppets.any_instance.expects(:perform).once + enforce end end - describe 'num_spam_flags_against_user' do - before { SpamRulesEnforcer.any_instance.stubs(:block_user) } - let(:post) { Fabricate(:post) } - let(:enforcer) { SpamRulesEnforcer.new(post.user) } - subject { enforcer.num_spam_flags_against_user } + context 'user argument' do + subject(:enforce) { described_class.enforce!(Fabricate.build(: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(:block_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 'block_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 - - context 'user is not blocked' do - before do - UserBlocker.expects(:block).with(user, nil, has_entries(message: :too_many_spam_flags)).returns(true) - end - - it 'prevents the user from making new posts' do - subject.block_user - expect(Guardian.new(user).can_create_post?(nil)).to be_false - end - - it 'sends private message to moderators' do - SiteSetting.stubs(:notify_mods_when_user_blocked).returns(true) - moderator = Fabricate(:moderator) - GroupMessage.expects(:create).with do |group, msg_type, params| - group == Group[:moderators].name and msg_type == :user_automatically_blocked and params[:user].id == user.id - end - subject.block_user - end - - it "doesn't send a pm to moderators if notify_mods_when_user_blocked is false" do - SiteSetting.stubs(:notify_mods_when_user_blocked).returns(false) - GroupMessage.expects(:create).never - subject.block_user - end - end - - context 'user is already blocked' do - before do - UserBlocker.expects(:block).with(user, nil, has_entries(message: :too_many_spam_flags)).returns(false) - end - - it "doesn't send a pm to moderators if the user is already blocked" do - GroupMessage.expects(:create).never - subject.block_user - end - 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 + it 'performs the AutoBlock' do + SpamRule::AutoBlock.any_instance.expects(:perform).once + enforce end end end