FIX: don't create staged users when incoming email is rejected

FIX: don't send subscription mail to new users

config/locales/server.en.yml

@@ -79,6 +79,7 @@ en:
         topic_closed_error: "Happens when a reply came in but the related topic has been closed."
         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."
       unrecognized_error: "Unrecognized Error"
 
   errors: &errors

lib/email/processor.rb

@@ -49,7 +49,8 @@ module Email
                          when Email::Receiver::ReplyUserNotMatchingError   then :email_reject_reply_user_not_matching
                          when Email::Receiver::TopicNotFoundError          then :email_reject_topic_not_found
                          when Email::Receiver::TopicClosedError            then :email_reject_topic_closed
-                         when Email::Receiver::InvalidPost                 then :email_reject_invalid_post
+                         when Email::Receiver::InvalidPost                 then :email_reject_invalid_pos
+                         when Email::Receiver::UnsubscribeNotAllowed       then :email_reject_invalid_post
                          when ActiveRecord::Rollback                       then :email_reject_invalid_post
                          when Email::Receiver::InvalidPostAction           then :email_reject_invalid_post_action
                          when Discourse::InvalidAccess                     then :email_reject_invalid_access

lib/email/receiver.rb

@@ -30,6 +30,7 @@ module Email
     class TopicClosedError             < ProcessingError; end
     class InvalidPost                  < ProcessingError; end
     class InvalidPostAction            < ProcessingError; end
+    class UnsubscribeNotAllowed        < ProcessingError; end
 
     attr_reader :incoming_email
     attr_reader :raw_email
@@ -38,7 +39,7 @@ module Email
 
     def initialize(mail_string)
       raise EmptyEmailError if mail_string.blank?
-      @staged_users_created = 0
+      @staged_users = []
       @raw_email = try_to_encode(mail_string, "UTF-8") || try_to_encode(mail_string, "ISO-8859-1") || mail_string
       @mail = Mail.new(@raw_email)
       @message_id = @mail.message_id.presence || Digest::MD5.hexdigest(mail_string)
@@ -82,14 +83,13 @@ module Email
       raise NoSenderDetectedError if @from_email.blank?
       raise ScreenedEmailError if ScreenedEmail.should_block?(@from_email)
 
-      user = find_or_create_user(@from_email, @from_display_name)
+      user = find_user(@from_email)
 
-      raise UserNotFoundError if user.nil?
+      if user.present?
-
+        process_user(user)
-      @incoming_email.update_columns(user_id: user.id)
+      else
-
+        raise UserNotFoundError unless SiteSetting.enable_staged_users
-      raise InactiveUserError if !user.active && !user.staged
+      end
-      raise BlockedUserError  if user.blocked
 
       body, elided = select_body
       body ||= ""
@@ -102,9 +102,17 @@ module Email
       end
 
       if action = subscription_action_for(body, subject)
-        message = SubscriptionMailer.send(action, user)
+        raise UnsubscribeNotAllowed if user.nil?
-        Email::Sender.new(message, :subscription).send
+        send_subscription_mail(action, user)
-      elsif post = find_related_post
+        return
+      end
+
+      # 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 post = find_related_post
         create_reply(user: user,
                      raw: body,
                      elided: elided,
@@ -128,6 +136,13 @@ module Email
       end
     end
 
+    def process_user(user)
+      @incoming_email.update_columns(user_id: user.id)
+
+      raise InactiveUserError if !user.active && !user.staged
+      raise BlockedUserError  if user.blocked
+    end
+
     def is_bounce?
       return false unless @mail.bounced? || verp
 
@@ -310,6 +325,10 @@ module Email
       @suject ||= @mail.subject.presence || I18n.t("emails.incoming.default_subject", email: @from_email)
     end
 
+    def find_user(email)
+      User.find_by_email(email)
+    end
+
     def find_or_create_user(email, display_name)
       user = nil
 
@@ -325,7 +344,7 @@ module Email
               name: display_name.presence || User.suggest_name(email),
               staged: true
             )
-            @staged_users_created += 1
+            @staged_users << user
           end
         rescue
           user = nil
@@ -693,7 +712,7 @@ module Email
                   topic.add_small_action(sender, "invited_user", user.username)
                 end
                 # cap number of staged users created per email
-                if @staged_users_created > SiteSetting.maximum_staged_users_per_email
+                if @staged_users.count > SiteSetting.maximum_staged_users_per_email
                   topic.add_moderator_post(sender, I18n.t("emails.incoming.maximum_staged_user_per_email_reached"))
                   return
                 end
@@ -717,6 +736,10 @@ module Email
       !topic.topic_allowed_groups.where("group_id IN (SELECT group_id FROM group_users WHERE user_id = ?)", user.id).exists?
     end
 
+    def send_subscription_mail(action, user)
+      message = SubscriptionMailer.send(action, user)
+      Email::Sender.new(message, :subscription).send
+    end
   end
 
 end

spec/components/email/receiver_spec.rb

@@ -300,6 +300,12 @@ describe Email::Receiver do
           expect(before_deliveries).to eq ActionMailer::Base.deliveries.count
         end
       end
+
+      it "raises an UnsubscribeNotAllowed and does not send an unsubscribe email" do
+        before_deliveries = ActionMailer::Base.deliveries.count
+        expect { process(:unsubscribe_new_user) }.to raise_error { Email::Receiver::UnsubscribeNotAllowed }
+        expect(before_deliveries).to eq ActionMailer::Base.deliveries.count
+      end
     end
 
     it "handles inline reply" do
@@ -623,4 +629,42 @@ describe Email::Receiver do
     end
   end
 
+  context "no staged users on error" do
+    before do
+      SiteSetting.enable_staged_users = true
+    end
+
+    shared_examples "no staged users" do |email_name|
+      it "does not create staged users" do
+        staged_user_count = User.where(staged: true).count
+        process(email_name) rescue nil
+        expect(User.where(staged: true).count).to eq(staged_user_count)
+      end
+    end
+
+    context "when email address is screened" do
+      before do
+        ScreenedEmail.expects(:should_block?).with("screened@mail.com").returns(true)
+      end
+
+      include_examples "no staged users", :screened_email
+    end
+
+    context "when the mail is auto generated" do
+      include_examples "no staged users", :auto_generated_header
+    end
+
+    context "when email is a bounced email" do
+      include_examples "no staged users", :bounced_email
+    end
+
+    context "when the body is blank" do
+      include_examples "no staged users", :no_body
+    end
+
+    context "when unsubscribe via email is not allowed" do
+      include_examples "no staged users", :unsubscribe_new_user
+    end
+  end
+
 end