From 7641d88224ec321baa789fd6cbfe731dd51fabf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 17 Nov 2014 12:04:29 +0100 Subject: [PATCH] FEATURE: new 'maximum new user accounts per registration IP' site setting --- app/controllers/users_controller.rb | 3 +-- app/services/spam_rules_enforcer.rb | 9 ++----- app/services/user_authenticator.rb | 7 +++-- config/locales/server.en.yml | 1 + config/site_settings.yml | 1 + lib/spam_handler.rb | 14 ++++++++++ .../allowed_ip_address_validator.rb | 11 +++++--- spec/components/spam_handler_spec.rb | 27 +++++++++++++++++++ .../allowed_ip_address_validator_spec.rb | 12 +++++++-- 9 files changed, 67 insertions(+), 18 deletions(-) create mode 100644 lib/spam_handler.rb create mode 100644 spec/components/spam_handler_spec.rb diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 00b2e618626..f74d5064126 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -227,8 +227,7 @@ class UsersController < ApplicationController authentication = UserAuthenticator.new(user, session) if !authentication.has_authenticator? && !SiteSetting.enable_local_logins - render nothing: true, status: 500 - return + return render nothing: true, status: 500 end authentication.start diff --git a/app/services/spam_rules_enforcer.rb b/app/services/spam_rules_enforcer.rb index 9de7c851fae..f306cfdc576 100644 --- a/app/services/spam_rules_enforcer.rb +++ b/app/services/spam_rules_enforcer.rb @@ -2,7 +2,6 @@ # receive, their trust level, etc. class SpamRulesEnforcer - # The exclamation point means that this method may make big changes to posts and users. def self.enforce!(arg) SpamRulesEnforcer.new(arg).enforce! end @@ -13,12 +12,8 @@ class SpamRulesEnforcer end def enforce! - if @user - SpamRule::AutoBlock.new(@user).perform - end - if @post - SpamRule::FlagSockpuppets.new(@post).perform - end + SpamRule::AutoBlock.new(@user).perform if @user + SpamRule::FlagSockpuppets.new(@post).perform if @post true end diff --git a/app/services/user_authenticator.rb b/app/services/user_authenticator.rb index 8c5adb9d2a3..dd27dbcd1ce 100644 --- a/app/services/user_authenticator.rb +++ b/app/services/user_authenticator.rb @@ -1,4 +1,5 @@ class UserAuthenticator + def initialize(user, session, authenticator_finder = Users::OmniauthCallbacksController) @user = user @session = session[:authentication] @@ -18,10 +19,7 @@ class UserAuthenticator end def finish - if authenticator - authenticator.after_create_account(@user, @session) - end - + authenticator.after_create_account(@user, @session) if authenticator @session = nil end @@ -40,4 +38,5 @@ class UserAuthenticator def authenticator_name @session && @session[:authenticator_name] end + end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 98151e618cc..a55817e789b 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -941,6 +941,7 @@ en: staff_like_weight: "How much extra weighting factor to give staff likes." levenshtein_distance_spammer_emails: "When matching spammer emails, number of characters difference that will still allow a fuzzy match." + max_new_accounts_per_registration_ip: "If there are already (n) trust level 0 accounts from this IP, stop accepting new signups from that IP." reply_by_email_enabled: "Enable replying to topics via email." reply_by_email_address: "Template for reply by email incoming email address, for example: %{reply_key}@reply.example.com or replies+%{reply_key}@example.com" diff --git a/config/site_settings.yml b/config/site_settings.yml index 24cb6906d59..284615fe092 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -576,6 +576,7 @@ spam: default: 2 min: 0 max: 3 + max_new_accounts_per_registration_ip: 3 rate_limits: unique_posts_mins: diff --git a/lib/spam_handler.rb b/lib/spam_handler.rb new file mode 100644 index 00000000000..a60a8cd5b5c --- /dev/null +++ b/lib/spam_handler.rb @@ -0,0 +1,14 @@ +class SpamHandler + + def self.should_prevent_registration_from_ip?(ip_address) + return false if SiteSetting.max_new_accounts_per_registration_ip <= 0 + + tl0_accounts_with_same_ip = User.unscoped + .where(trust_level: TrustLevel[0]) + .where("ip_address = ?", ip_address.to_s) + .count + + tl0_accounts_with_same_ip >= SiteSetting.max_new_accounts_per_registration_ip + end + +end diff --git a/lib/validators/allowed_ip_address_validator.rb b/lib/validators/allowed_ip_address_validator.rb index 61795e44e2f..5094a9bcb68 100644 --- a/lib/validators/allowed_ip_address_validator.rb +++ b/lib/validators/allowed_ip_address_validator.rb @@ -1,9 +1,14 @@ +require_dependency "spam_handler" + class AllowedIpAddressValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) - if record.ip_address and ScreenedIpAddress.should_block?(record.ip_address) - record.errors.add(attribute, options[:message] || I18n.t('user.ip_address.blocked')) + if record.ip_address + if ScreenedIpAddress.should_block?(record.ip_address) || + (record.trust_level == TrustLevel[0] && SpamHandler.should_prevent_registration_from_ip?(record.ip_address)) + record.errors.add(attribute, options[:message] || I18n.t('user.ip_address.blocked')) + end end end -end \ No newline at end of file +end diff --git a/spec/components/spam_handler_spec.rb b/spec/components/spam_handler_spec.rb new file mode 100644 index 00000000000..7154d45486b --- /dev/null +++ b/spec/components/spam_handler_spec.rb @@ -0,0 +1,27 @@ +require "spec_helper" +require "spam_handler" + +describe SpamHandler do + + describe "#should_prevent_registration_from_ip?" do + + it "works" do + # max_new_accounts_per_registration_ip = 0 disables the check + SiteSetting.stubs(:max_new_accounts_per_registration_ip).returns(0) + + Fabricate(:user, ip_address: "42.42.42.42", trust_level: TrustLevel[1]) + Fabricate(:user, ip_address: "42.42.42.42", trust_level: TrustLevel[0]) + + # only prevents registration for TL0 + SiteSetting.stubs(:max_new_accounts_per_registration_ip).returns(2) + + Fabricate(:user, ip_address: "42.42.42.42", trust_level: TrustLevel[1]) + Fabricate(:user, ip_address: "42.42.42.42", trust_level: TrustLevel[0]) + + Fabricate(:user, ip_address: "42.42.42.42", trust_level: TrustLevel[1]) + -> { Fabricate(:user, ip_address: "42.42.42.42", trust_level: TrustLevel[0]) }.should raise_error(ActiveRecord::RecordInvalid) + end + + end + +end diff --git a/spec/components/validators/allowed_ip_address_validator_spec.rb b/spec/components/validators/allowed_ip_address_validator_spec.rb index 3df87639a19..96bd162f504 100644 --- a/spec/components/validators/allowed_ip_address_validator_spec.rb +++ b/spec/components/validators/allowed_ip_address_validator_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe AllowedIpAddressValidator do - let(:record) { Fabricate.build(:user, ip_address: '99.232.23.123') } + let(:record) { Fabricate.build(:user, trust_level: TrustLevel[0], ip_address: '99.232.23.123') } let(:validator) { described_class.new({attributes: :ip_address}) } subject(:validate) { validator.validate_each(record, :ip_address, record.ip_address) } @@ -14,6 +14,14 @@ describe AllowedIpAddressValidator do end end + context "ip address isn't allowed for registration" do + it 'should add an error' do + SpamHandler.stubs(:should_prevent_registration_from_ip?).returns(true) + validate + record.errors[:ip_address].should be_present + end + end + context "ip address should not be blocked" do it "shouldn't add an error" do ScreenedIpAddress.stubs(:should_block?).returns(false) @@ -31,4 +39,4 @@ describe AllowedIpAddressValidator do end end -end \ No newline at end of file +end