diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index edd0852e0cf..a113a35da95 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/app/jobs/scheduled/poll_mailbox.rb @@ -18,6 +18,26 @@ module Jobs end end + def handle_mail(mail) + begin + Email::Receiver.new(mail).process + rescue Email::Receiver::UserNotSufficientTrustLevelError => e + # inform the user about the rejection + @message = Mail::Message.new(mail) + clientMessage = RejectionMailer.send_trust_level(@message.from, @message.body) + email_sender = Email::Sender.new(clientMessage, :email_reject_trust_level) + email_sender.send + rescue Email::Receiver::ProcessingError + # all other ProcessingErrors are ok to be dropped + rescue StandardError => e + # Inform Admins about error + GroupMessage.create(Group[:admins].name, :email_error_notification, + {limit_once_per: false, message_params: {source: mail, error: e}}) + ensure + mail.delete + end + end + def poll_pop3s Net::POP3.enable_ssl(OpenSSL::SSL::VERIFY_NONE) Net::POP3.start(SiteSetting.pop3s_polling_host, @@ -26,15 +46,7 @@ module Jobs SiteSetting.pop3s_polling_password) do |pop| unless pop.mails.empty? pop.each do |mail| - if Email::Receiver.new(mail.pop).process == Email::Receiver.results[:processed] - mail.delete - else - @message = Mail::Message.new(mail.pop) - # One for you (mod), and one for me (sender) - GroupMessage.create(Group[:moderators].name, :email_reject_notification, {limit_once_per: false, message_params: {from: @message.from, body: @message.body}}) - clientMessage = RejectionMailer.send_rejection(@message.from, @message.body) - Email::Sender.new(clientMessage, :email_reject_notification).send - end + handle_mail mail.pop end end end diff --git a/app/mailers/rejection_mailer.rb b/app/mailers/rejection_mailer.rb index fa9957361af..1e060aa55a0 100644 --- a/app/mailers/rejection_mailer.rb +++ b/app/mailers/rejection_mailer.rb @@ -6,4 +6,8 @@ class RejectionMailer < ActionMailer::Base def send_rejection(from, body) build_email(from, template: 'email_reject_notification', from: from, body: body) end + + def send_trust_level(from, body, to) + build_email(from, template: 'email_reject_trust_level', to: to) + end end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 2361259da21..fa5c2f17c63 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1111,17 +1111,23 @@ en: subject_template: "Import completed successfully" text_body_template: "The import was successful." - email_reject_notification: - subject_template: "Message posting failed" + email_error_notification: + subject_template: "Error parsing email" text_body_template: | - This is an automated message to inform you that the user a message failed to meet topic criteria. + This is an automated message to inform you that parsing the following incoming email failed. Please review the following message. - From - %{from} - - Contents - %{body}. - + Error - %{error} + + %{source} + + email_reject_trust_level: + subject_template: "Message rejected" + text_body_template: | + The message you've send to %{to} was rejected by the system. + + You do not have the required trust to post new topics to this email address. too_many_spam_flags: subject_template: "New account blocked" diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index f1a6e279b84..7ce6102cab9 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -5,9 +5,12 @@ module Email class Receiver - def self.results - @results ||= Enum.new(:unprocessable, :missing, :processed, :error) - end + class ProcessingError < StandardError; end + class EmailUnparsableError < ProcessingError; end + class EmptyEmailError < ProcessingError; end + class UserNotFoundError < ProcessingError; end + class UserNotSufficientTrustLevelError < ProcessingError; end + class EmailLogNotFound < ProcessingError; end attr_reader :body, :reply_key, :email_log @@ -32,21 +35,21 @@ module Email end def process - return Email::Receiver.results[:unprocessable] if @raw.blank? + raise EmptyEmailError if @raw.blank? @message = Mail::Message.new(@raw) # First remove the known discourse stuff. parse_body - return Email::Receiver.results[:unprocessable] if @body.blank? + raise EmptyEmailError if @body.blank? # Then run the github EmailReplyParser on it in case we didn't catch it @body = EmailReplyParser.read(@body).visible_text.force_encoding('UTF-8') discourse_email_parser - return Email::Receiver.results[:unprocessable] if @body.blank? + raise EmailUnparsableError if @body.blank? if is_in_email? @user = User.find_by_email(@message.from.first) @@ -55,29 +58,25 @@ module Email @user = Discourse.system_user end - return Email::Receiver.results[:unprocessable] if @user.blank? or not @user.has_trust_level?(TrustLevel.levels[SiteSetting.email_in_min_trust.to_i]) + raise UserNotFoundError if @user.blank? + raise UserNotSufficientTrustLevelError.new @user if not @user.has_trust_level?(TrustLevel.levels[SiteSetting.email_in_min_trust.to_i]) create_new_topic - return Email::Receiver.results[:processed] + else + @reply_key = @message.to.first + + # Extract the `reply_key` from the format the site has specified + tokens = SiteSetting.reply_by_email_address.split("%{reply_key}") + tokens.each do |t| + @reply_key.gsub!(t, "") if t.present? + end + + # Look up the email log for the reply key + @email_log = EmailLog.for(reply_key) + raise EmailLogNotFound if @email_log.blank? + + create_reply end - - @reply_key = @message.to.first - - # Extract the `reply_key` from the format the site has specified - tokens = SiteSetting.reply_by_email_address.split("%{reply_key}") - tokens.each do |t| - @reply_key.gsub!(t, "") if t.present? - end - - # Look up the email log for the reply key - @email_log = EmailLog.for(reply_key) - return Email::Receiver.results[:missing] if @email_log.blank? - - create_reply - - Email::Receiver.results[:processed] - rescue - Email::Receiver.results[:error] end private diff --git a/spec/components/email/poll_mailbox_spec.rb b/spec/components/email/poll_mailbox_spec.rb new file mode 100644 index 00000000000..1465ca1c777 --- /dev/null +++ b/spec/components/email/poll_mailbox_spec.rb @@ -0,0 +1,107 @@ +# -*- encoding : utf-8 -*- + +require 'spec_helper' +require 'email/receiver' +require 'jobs/scheduled/poll_mailbox' +require 'email/message_builder' + +describe Jobs::PollMailbox do + + describe "processing email" do + + let!(:poller) { Jobs::PollMailbox.new } + let!(:receiver) { mock } + let!(:email) { mock } + + before do + Email::Receiver.expects(:new).with(email).returns(receiver) + end + + describe "all goes fine" do + + it "email gets deleted" do + receiver.expects(:process) + email.expects(:delete) + + poller.handle_mail(email) + end + end + + describe "raises Untrusted error" do + + before do + receiver.expects(:process).raises(Email::Receiver::UserNotSufficientTrustLevelError) + email.expects(:delete) + + Mail::Message.expects(:new).returns(email) + + email.expects(:from) + email.expects(:body) + + clientMessage = mock + senderMock = mock + RejectionMailer.expects(:send_trust_level).returns(clientMessage) + Email::Sender.expects(:new).with( + clientMessage, :email_reject_trust_level).returns(senderMock) + senderMock.expects(:send) + end + + it "sends a reply and deletes the email" do + poller.handle_mail(email) + end + end + + describe "raises error" do + + it "deletes email on ProcessingError" do + receiver.expects(:process).raises(Email::Receiver::ProcessingError) + email.expects(:delete) + + poller.handle_mail(email) + end + + it "deletes email on EmailUnparsableError" do + receiver.expects(:process).raises(Email::Receiver::EmailUnparsableError) + email.expects(:delete) + + poller.handle_mail(email) + end + + it "deletes email on EmptyEmailError" do + receiver.expects(:process).raises(Email::Receiver::EmptyEmailError) + email.expects(:delete) + + poller.handle_mail(email) + end + + it "deletes email on UserNotFoundError" do + receiver.expects(:process).raises(Email::Receiver::UserNotFoundError) + email.expects(:delete) + + poller.handle_mail(email) + end + + it "deletes email on EmailLogNotFound" do + receiver.expects(:process).raises(Email::Receiver::EmailLogNotFound) + email.expects(:delete) + + poller.handle_mail(email) + end + + + it "informs admins on any other error" do + receiver.expects(:process).raises(TypeError) + email.expects(:delete) + GroupMessage.expects(:create) do |args| + args[0].should eq "admins" + args[1].shouled eq :email_error_notification + args[2].message_params.source.should eq email + args[2].message_params.error.should_be instance_of(TypeError) + end + + poller.handle_mail(email) + end + end + end + +end \ No newline at end of file diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 9217020b3ae..ad0697111ba 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -10,23 +10,13 @@ describe Email::Receiver do SiteSetting.stubs(:email_in).returns(false) end - describe "exception raised" do - it "returns error if it encountered an error processing" do - receiver = Email::Receiver.new("some email") - def receiver.parse_body - raise "ERROR HAPPENED!" - end - expect(receiver.process).to eq(Email::Receiver.results[:error]) - end - end - describe 'invalid emails' do - it "returns unprocessable if the message is blank" do - expect(Email::Receiver.new("").process).to eq(Email::Receiver.results[:unprocessable]) + it "raises EmptyEmailError if the message is blank" do + expect { Email::Receiver.new("").process }.to raise_error(Email::Receiver::EmptyEmailError) end - it "returns unprocessable if the message is not an email" do - expect(Email::Receiver.new("asdf" * 30).process).to eq(Email::Receiver.results[:unprocessable]) + it "raises EmailUnparsableError if the message is not an email" do + expect { Email::Receiver.new("asdf" * 30).process}.to raise_error(Email::Receiver::EmptyEmailError) end end @@ -35,7 +25,7 @@ describe Email::Receiver do let(:receiver) { Email::Receiver.new(reply_below) } it "processes correctly" do - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq( "So presumably all the quoted garbage and my (proper) signature will get stripped from my reply?") @@ -47,7 +37,7 @@ stripped from my reply?") let(:receiver) { Email::Receiver.new(reply_below) } it "processes correctly" do - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq("The EC2 instance - I've seen that there tends to be odd and " + "unrecommended settings on the Bitnami installs that I've checked out.") end @@ -58,7 +48,7 @@ stripped from my reply?") let(:receiver) { Email::Receiver.new(attachment) } it "processes correctly" do - expect(receiver.process).to eq(Email::Receiver.results[:unprocessable]) + expect { receiver.process}.to raise_error(Email::Receiver::EmptyEmailError) expect(receiver.body).to be_blank end end @@ -68,7 +58,7 @@ stripped from my reply?") let(:receiver) { Email::Receiver.new(dutch) } it "processes correctly" do - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq("Dit is een antwoord in het Nederlands.") end end @@ -79,7 +69,7 @@ stripped from my reply?") it "processes correctly" do I18n.expects(:t).with('user_notifications.previous_discussion').returns('כלטוב') - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq("שלום") end end @@ -90,7 +80,7 @@ stripped from my reply?") it "processes correctly" do I18n.expects(:t).with('user_notifications.previous_discussion').returns('媽!我上電視了!') - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq("媽!我上電視了!") end end @@ -100,7 +90,7 @@ stripped from my reply?") let(:receiver) { Email::Receiver.new(wrote) } it "removes via lines if we know them" do - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq("Hello this email has content!") end end @@ -110,7 +100,7 @@ stripped from my reply?") let(:receiver) { Email::Receiver.new(wrote) } it "processes correctly" do - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq("Thanks!") end end @@ -120,7 +110,7 @@ stripped from my reply?") let(:receiver) { Email::Receiver.new(previous) } it "processes correctly" do - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq("This will not include the previous discussion that is present in this email.") end end @@ -130,7 +120,7 @@ stripped from my reply?") let(:receiver) { Email::Receiver.new(paragraphs) } it "processes correctly" do - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq( "Is there any reason the *old* candy can't be be kept in silos while the new candy is imported into *new* silos? @@ -161,10 +151,8 @@ greatest show ever created. Everyone should watch it. EmailLog.expects(:for).returns(nil) end - let!(:result) { receiver.process } - - it "returns missing" do - expect(result).to eq(Email::Receiver.results[:missing]) + it "raises EmailLogNotFoundError" do + expect{ receiver.process }.to raise_error(Email::Receiver::EmailLogNotFound) end end @@ -185,10 +173,6 @@ greatest show ever created. Everyone should watch it. let!(:result) { receiver.process } - it "returns a processed result" do - expect(result).to eq(Email::Receiver.results[:processed]) - end - it "extracts the body" do expect(receiver.body).to eq(reply_body) end @@ -229,10 +213,8 @@ Jakie" } User.expects(:find_by_email).returns(nil) end - let!(:result) { receiver.process } - - it "returns unprocessable" do - expect(result).to eq(Email::Receiver.results[:unprocessable]) + it "raises user not found error" do + expect { receiver.process }.to raise_error(Email::Receiver::UserNotFoundError) end end @@ -244,10 +226,8 @@ Jakie" } SiteSetting.stubs(:email_in_min_trust).returns(TrustLevel.levels[:elder].to_s) end - let!(:result) { receiver.process } - - it "returns unprocessable" do - expect(result).to eq(Email::Receiver.results[:unprocessable]) + it "raises untrusted user error" do + expect { receiver.process }.to raise_error(Email::Receiver::UserNotSufficientTrustLevelError) end end @@ -289,10 +269,6 @@ Jakie" } let!(:result) { receiver.process } - it "returns a processed result" do - expect(result).to eq(Email::Receiver.results[:processed]) - end - it "extracts the body" do expect(receiver.body).to eq(email_body) end @@ -327,10 +303,8 @@ Jakie" } Category.expects(:find_by_email).returns(nil) end - let!(:result) { receiver.process } - - it "returns missing" do - expect(result).to eq(Email::Receiver.results[:missing]) + it "raises EmailLogNotFoundError" do + expect{ receiver.process }.to raise_error(Email::Receiver::EmailLogNotFound) end end @@ -343,10 +317,8 @@ Jakie" } "discourse-in@appmail.adventuretime.ooo").returns(category) end - let!(:result) { receiver.process } - - it "returns unprocessable" do - expect(result).to eq(Email::Receiver.results[:unprocessable]) + it "raises UserNotFoundError" do + expect{ receiver.process }.to raise_error(Email::Receiver::UserNotFoundError) end end @@ -360,10 +332,8 @@ Jakie" } SiteSetting.stubs(:email_in_min_trust).returns(TrustLevel.levels[:elder].to_s) end - let!(:result) { receiver.process } - - it "returns unprocessable" do - expect(result).to eq(Email::Receiver.results[:unprocessable]) + it "raises untrusted user error" do + expect { receiver.process }.to raise_error(Email::Receiver::UserNotSufficientTrustLevelError) end end @@ -408,10 +378,6 @@ Jakie" } let!(:result) { receiver.process } - it "returns a processed result" do - expect(result).to eq(Email::Receiver.results[:processed]) - end - it "extracts the body" do expect(receiver.body).to eq(email_body) end @@ -449,10 +415,8 @@ Jakie "discourse-in@appmail.adventuretime.ooo").returns(non_inbox_email_category) end - let!(:result) { receiver.process } - - it "returns unprocessable" do - expect(result).to eq(Email::Receiver.results[:unprocessable]) + it "raises UserNotFoundError" do + expect{ receiver.process }.to raise_error(Email::Receiver::UserNotFoundError) end end @@ -495,10 +459,6 @@ Jakie let!(:result) { receiver.process } - it "returns a processed result" do - expect(result).to eq(Email::Receiver.results[:processed]) - end - it "extracts the body" do expect(receiver.body).to eq(email_body) end