Add BlockedEmail, to block signups based on email. Track stats of how many times each email address is blocked, and last time it was blocked. Move email validation out of User model and into EmailValidator. Signup form remembers which email addresses have failed and shows validation error on email field.

This commit is contained in:
Neil Lalonde 2013-07-25 13:01:27 -04:00
parent e25638dab0
commit 5f8a130277
10 changed files with 155 additions and 23 deletions

View File

@ -14,6 +14,7 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF
accountPasswordConfirm: 0,
accountChallenge: 0,
formSubmitted: false,
rejectedEmails: Em.A([]),
submitDisabled: function() {
if (this.get('formSubmitted')) return true;
@ -64,6 +65,14 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF
}
email = this.get("accountEmail");
if (this.get('rejectedEmails').contains(email)) {
return Discourse.InputValidation.create({
failed: true,
reason: I18n.t('user.email.invalid')
});
}
if ((this.get('authOptions.email') === email) && this.get('authOptions.email_valid')) {
return Discourse.InputValidation.create({
ok: true,
@ -84,7 +93,7 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF
failed: true,
reason: I18n.t('user.email.invalid')
});
}.property('accountEmail'),
}.property('accountEmail', 'rejectedEmails.@each'),
usernameMatch: function() {
if (this.usernameNeedsToBeValidatedWithEmail()) {
@ -262,6 +271,9 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF
createAccountController.set('complete', true);
} else {
createAccountController.flash(result.message || I18n.t('create_account.failed'), 'error');
if (result.errors && result.errors.email && result.values) {
createAccountController.get('rejectedEmails').pushObject(result.values.email);
}
createAccountController.set('formSubmitted', false);
}
if (result.active) {

View File

@ -194,7 +194,9 @@ class UsersController < ApplicationController
else
render json: {
success: false,
message: I18n.t("login.errors", errors: user.errors.full_messages.join("\n"))
message: I18n.t("login.errors", errors: user.errors.full_messages.join("\n")),
errors: user.errors.to_hash,
values: user.attributes.slice("name", "username", "email")
}
end
rescue ActiveRecord::StatementInvalid

View File

@ -0,0 +1,25 @@
class BlockedEmail < ActiveRecord::Base
before_validation :set_defaults
validates :email, presence: true, uniqueness: true
def self.actions
@actions ||= Enum.new(:block, :do_nothing)
end
def self.should_block?(email)
record = BlockedEmail.where(email: email).first
if record
record.match_count += 1
record.last_match_at = Time.zone.now
record.save
end
record && record.action_type == actions[:block]
end
def set_defaults
self.action_type ||= BlockedEmail.actions[:block]
end
end

View File

@ -42,10 +42,9 @@ class User < ActiveRecord::Base
has_one :user_search_data
validates_presence_of :username
validates_presence_of :email
validates_uniqueness_of :email
validate :username_validator
validate :email_validator, if: :email_changed?
validates :email, presence: true, uniqueness: true
validates :email, email: true, if: :email_changed?
validate :password_validator
before_save :cook
@ -565,24 +564,6 @@ class User < ActiveRecord::Base
end
end
def email_validator
if (setting = SiteSetting.email_domains_whitelist).present?
unless email_in_restriction_setting?(setting)
errors.add(:email, I18n.t(:'user.email.not_allowed'))
end
elsif (setting = SiteSetting.email_domains_blacklist).present?
if email_in_restriction_setting?(setting)
errors.add(:email, I18n.t(:'user.email.not_allowed'))
end
end
end
def email_in_restriction_setting?(setting)
domains = setting.gsub('.', '\.')
regexp = Regexp.new("@(#{domains})", true)
self.email =~ regexp
end
def password_validator
if (@raw_password && @raw_password.length < 6) || (@password_required && !@raw_password)
errors.add(:password, "must be 6 letters or longer")

View File

@ -731,6 +731,7 @@ en:
must_begin_with_alphanumeric: "must begin with a letter or number"
email:
not_allowed: "is not allowed from that email provider. Please use another email address."
blocked: "is not allowed."
invite_mailer:
subject_template: "[%{site_name}] %{invitee_name} invited you to join a discussion on %{site_name}"

View File

@ -0,0 +1,12 @@
class CreateBlockedEmails < ActiveRecord::Migration
def change
create_table :blocked_emails do |t|
t.string :email, null: false
t.integer :action_type, null: false
t.integer :match_count, null: false, default: 0
t.datetime :last_match_at
t.timestamps
end
add_index :blocked_emails, :email, unique: true
end
end

View File

@ -0,0 +1,24 @@
class EmailValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value)
if (setting = SiteSetting.email_domains_whitelist).present?
unless email_in_restriction_setting?(setting, value)
record.errors.add(attribute, I18n.t(:'user.email.not_allowed'))
end
elsif (setting = SiteSetting.email_domains_blacklist).present?
if email_in_restriction_setting?(setting, value)
record.errors.add(attribute, I18n.t(:'user.email.not_allowed'))
end
end
if record.errors[attribute].blank? and BlockedEmail.should_block?(value)
record.errors.add(attribute, I18n.t(:'user.email.blocked'))
end
end
def email_in_restriction_setting?(setting, value)
domains = setting.gsub('.', '\.')
regexp = Regexp.new("@(#{domains})", true)
value =~ regexp
end
end

View File

@ -0,0 +1,23 @@
require 'spec_helper'
describe EmailValidator do
let(:record) { Fabricate.build(:user, email: "bad@spamclub.com") }
let(:validator) { described_class.new({attributes: :email}) }
subject(:validate) { validator.validate_each(record,:email,record.email) }
context "blocked email" do
it "doesn't add an error when email doesn't match a blocked email" do
BlockedEmail.stubs(:should_block?).with(record.email).returns(false)
validate
record.errors[:email].should_not be_present
end
it "adds an error when email matches a blocked email" do
BlockedEmail.stubs(:should_block?).with(record.email).returns(true)
validate
record.errors[:email].should be_present
end
end
end

View File

@ -0,0 +1,4 @@
Fabricator(:blocked_email) do
email { sequence(:email) { |n| "bad#{n}@spammers.org" } }
action_type BlockedEmail.actions[:block]
end

View File

@ -0,0 +1,48 @@
require 'spec_helper'
describe BlockedEmail do
let(:email) { 'block@spamfromhome.org' }
describe "new record" do
it "sets a default action_type" do
BlockedEmail.create(email: email).action_type.should == BlockedEmail.actions[:block]
end
it "last_match_at is null" do
# If we manually load the table with some emails, we can see whether those emails
# have ever been blocked by looking at last_match_at.
BlockedEmail.create(email: email).last_match_at.should be_nil
end
end
describe "#should_block?" do
subject { BlockedEmail.should_block?(email) }
it "returns false if a record with the email doesn't exist" do
subject.should be_false
end
shared_examples "when a BlockedEmail record matches" do
it "updates statistics" do
Timecop.freeze(Time.zone.now) do
expect { subject }.to change { blocked_email.reload.match_count }.by(1)
blocked_email.last_match_at.should be_within_one_second_of(Time.zone.now)
end
end
end
context "action_type is :block" do
let!(:blocked_email) { Fabricate(:blocked_email, email: email, action_type: BlockedEmail.actions[:block]) }
it { should be_true }
include_examples "when a BlockedEmail record matches"
end
context "action_type is :do_nothing" do
let!(:blocked_email) { Fabricate(:blocked_email, email: email, action_type: BlockedEmail.actions[:do_nothing]) }
it { should be_false }
include_examples "when a BlockedEmail record matches"
end
end
end