FIX: respect email domain whitelist/blacklist when creating staged users

config/locales/server.en.yml

@@ -80,6 +80,7 @@ en:
         bounced_email_error: "Email is a bounced email report."
         screened_email_error: "Happens when the sender's email address was already screened."
         unsubscribe_not_allowed: "Happens when unsubscribing via email is not allowed for this user."
+        email_not_allowed: "Happens when the email address is not on the whitelist or is on the blacklist."
       unrecognized_error: "Unrecognized Error"
 
   errors: &errors
@@ -2151,6 +2152,14 @@ en:
 
         Your reply was sent from a blocked email address. Try sending from another email address, or [contact a staff member](%{base_url}/about).
 
+    email_reject_not_allowed_email:
+      title: "Email Reject Not Allowed Email"
+      subject_template: "[%{email_prefix}] Email issue -- Blocked Email"
+      text_body_template: |
+        We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work.
+
+        Your reply was sent from a blocked email address. Try sending from another email address, or [contact a staff member](%{base_url}/about).
+
     email_reject_inactive_user:
       title: "Email Reject Inactive User"
       subject_template: "[%{email_prefix}] Email issue -- Inactive User"

lib/email/processor.rb

@@ -40,6 +40,7 @@ module Email
                          when Email::Receiver::NoBodyDetectedError         then :email_reject_empty
                          when Email::Receiver::UserNotFoundError           then :email_reject_user_not_found
                          when Email::Receiver::ScreenedEmailError          then :email_reject_screened_email
+                         when Email::Receiver::EmailNotAllowed             then :email_reject_not_allowed_email
                          when Email::Receiver::AutoGeneratedEmailError     then :email_reject_auto_generated
                          when Email::Receiver::InactiveUserError           then :email_reject_inactive_user
                          when Email::Receiver::BlockedUserError            then :email_reject_blocked_user

lib/email/receiver.rb

@@ -31,6 +31,7 @@ module Email
     class InvalidPost                  < ProcessingError; end
     class InvalidPostAction            < ProcessingError; end
     class UnsubscribeNotAllowed        < ProcessingError; end
+    class EmailNotAllowed              < ProcessingError; end
 
     attr_reader :incoming_email
     attr_reader :raw_email
@@ -86,7 +87,7 @@ module Email
       user = find_user(@from_email)
 
       if user.present?
-        process_user(user)
+        log_and_validate_user(user)
       else
         raise UserNotFoundError unless SiteSetting.enable_staged_users
       end
@@ -109,8 +110,10 @@ module Email
 
       # Lets create a staged user if there isn't one yet. We will try to
       # delete staged users in process!() if something bad happens.
-      user = find_or_create_user(@from_email, @from_display_name) if user.nil?
-      process_user(user)
+      if user.nil?
+        user = find_or_create_user(@from_email, @from_display_name)
+        log_and_validate_user(user)
+      end
 
       if post = find_related_post
         create_reply(user: user,
@@ -136,11 +139,11 @@ module Email
       end
     end
 
-    def process_user(user)
+    def log_and_validate_user(user)
       @incoming_email.update_columns(user_id: user.id)
 
       raise InactiveUserError if !user.active && !user.staged
-      raise BlockedUserError  if user.blocked
+      raise BlockedUserError if user.blocked
     end
 
     def is_bounce?
@@ -333,10 +336,12 @@ module Email
       user = nil
 
       User.transaction do
-        begin
-          user = User.find_by_email(email)
+        user = User.find_by_email(email)
 
-          if user.nil? && SiteSetting.enable_staged_users
+        if user.nil? && SiteSetting.enable_staged_users
+          raise EmailNotAllowed unless EmailValidator.allowed?(email)
+
+          begin
             username = UserNameSuggester.sanitize_username(display_name) if display_name.present?
             user = User.create!(
               email: email,
@@ -345,9 +350,9 @@ module Email
               staged: true
             )
             @staged_users << user
+          rescue
+            user = nil
           end
-        rescue
-          user = nil
         end
       end
 
@@ -717,8 +722,8 @@ module Email
                   return
                 end
               end
-            rescue ActiveRecord::RecordInvalid
-              # don't care if user already allowed
+            rescue ActiveRecord::RecordInvalid, EmailNotAllowed
+              # don't care if user already allowed or the user's email address is not allowed
             end
           end
         end

lib/validators/email_validator.rb

@@ -1,27 +1,32 @@
 class EmailValidator < ActiveModel::EachValidator
 
   def validate_each(record, attribute, value)
-    if (setting = SiteSetting.email_domains_whitelist).present?
-      unless email_in_restriction_setting?(setting, value) || is_developer?(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) && !is_developer?(value)
-        record.errors.add(attribute, I18n.t(:'user.email.not_allowed'))
-      end
+    unless EmailValidator.allowed?(value)
+      record.errors.add(attribute, I18n.t(:'user.email.not_allowed'))
     end
+
     if record.errors[attribute].blank? && value && ScreenedEmail.should_block?(value)
       record.errors.add(attribute, I18n.t(:'user.email.blocked'))
     end
   end
 
-  def email_in_restriction_setting?(setting, value)
+  def self.allowed?(email)
+    if (setting = SiteSetting.email_domains_whitelist).present?
+      return email_in_restriction_setting?(setting, email) || is_developer?(email)
+    elsif (setting = SiteSetting.email_domains_blacklist).present?
+      return !(email_in_restriction_setting?(setting, email) && !is_developer?(email))
+    end
+
+    true
+  end
+
+  def self.email_in_restriction_setting?(setting, value)
     domains = setting.gsub('.', '\.')
     regexp = Regexp.new("@(.+\\.)?(#{domains})", true)
     value =~ regexp
   end
 
-  def is_developer?(value)
+  def self.is_developer?(value)
     Rails.configuration.respond_to?(:developer_emails) && Rails.configuration.developer_emails.include?(value)
   end
 

spec/components/email/receiver_spec.rb

@@ -23,6 +23,16 @@ describe Email::Receiver do
     expect { process(:screened_email) }.to raise_error(Email::Receiver::ScreenedEmailError)
   end
 
+  it "raises EmailNotAllowed when email address is not on whitelist" do
+    SiteSetting.email_domains_whitelist = "example.com|bar.com"
+    expect { process(:blacklist_whitelist_email) }.to raise_error(Email::Receiver::EmailNotAllowed)
+  end
+
+  it "raises EmailNotAllowed when email address is on blacklist" do
+    SiteSetting.email_domains_blacklist = "email.com|mail.com"
+    expect { process(:blacklist_whitelist_email) }.to raise_error(Email::Receiver::EmailNotAllowed)
+  end
+
   it "raises an UserNotFoundError when staged users are disabled" do
     SiteSetting.enable_staged_users = false
     expect { process(:user_not_found) }.to raise_error(Email::Receiver::UserNotFoundError)
@@ -665,6 +675,23 @@ describe Email::Receiver do
     context "when unsubscribe via email is not allowed" do
       include_examples "no staged users", :unsubscribe_new_user
     end
+
+    context "when email address is not on whitelist" do
+      before do
+        SiteSetting.email_domains_whitelist = "example.com|bar.com"
+      end
+
+      include_examples "no staged users", :blacklist_whitelist_email
+    end
+
+    context "when email address is on blacklist" do
+      before do
+        SiteSetting.email_domains_blacklist = "email.com|mail.com"
+      end
+
+      include_examples "no staged users", :blacklist_whitelist_email
+    end
+
   end
 
 end

spec/components/validators/email_validator_spec.rb

@@ -30,6 +30,14 @@ describe EmailValidator do
       expect(blocks?('sam@e-mail.com')).to eq(true)
       expect(blocks?('sam@googlemail.com')).to eq(false)
     end
+
+    it "blocks based on email_domains_whitelist" do
+      SiteSetting.email_domains_whitelist = "googlemail.com|email.com"
+      expect(blocks?('sam@email.com')).to eq(false)
+      expect(blocks?('sam@bob.email.com')).to eq(false)
+      expect(blocks?('sam@e-mail.com')).to eq(true)
+      expect(blocks?('sam@googlemail.com')).to eq(false)
+    end
   end
 
   context '.email_regex' do

spec/fixtures/emails/blacklist_whitelist_email.eml

@@ -0,0 +1,9 @@
+Return-Path: <foo@mail.com>
+From: Foo <foo@mail.com>
+Date: Fri, 15 Jan 2016 00:12:43 +0100
+Message-ID: <51@foo.bar.mail>
+Mime-Version: 1.0
+Content-Type: text/plain
+Content-Transfer-Encoding: 7bit
+
+Email from a domain on blacklist or whitelist.