mirror of
https://github.com/codeninjasllc/discourse.git
synced 2024-11-23 15:48:43 -05:00
FIX: Don't send rejection mailer to bounced emails.
This commit is contained in:
parent
2ecff60af2
commit
5734c7f3f3
10 changed files with 188 additions and 13 deletions
|
@ -25,17 +25,31 @@ module Jobs
|
||||||
mail_string = popmail.pop
|
mail_string = popmail.pop
|
||||||
receiver = Email::Receiver.new(mail_string)
|
receiver = Email::Receiver.new(mail_string)
|
||||||
receiver.process!
|
receiver.process!
|
||||||
|
rescue Email::Receiver::BouncedEmailError => e
|
||||||
|
log_email_process_failure(mail_string, e)
|
||||||
|
|
||||||
|
set_incoming_email_rejection_message(
|
||||||
|
receiver.incoming_email, I18n.t("email.incoming.errors.bounced_email_report")
|
||||||
|
)
|
||||||
|
rescue Email::Receiver::AutoGeneratedEmailReplyError => e
|
||||||
|
log_email_process_failure(mail_string, e)
|
||||||
|
|
||||||
|
set_incoming_email_rejection_message(
|
||||||
|
receiver.incoming_email,
|
||||||
|
I18n.t("email.incoming.errors.auto_generated_email_reply")
|
||||||
|
)
|
||||||
rescue => e
|
rescue => e
|
||||||
rejection_message = handle_failure(mail_string, e)
|
rejection_message = handle_failure(mail_string, e)
|
||||||
if rejection_message.present? && receiver && receiver.incoming_email
|
if rejection_message.present? && receiver && (incoming_email = receiver.incoming_email)
|
||||||
receiver.incoming_email.rejection_message = rejection_message.body.to_s
|
set_incoming_email_rejection_message(
|
||||||
receiver.incoming_email.save
|
incoming_email, rejection_message.body.to_s
|
||||||
|
)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def handle_failure(mail_string, e)
|
def handle_failure(mail_string, e)
|
||||||
Rails.logger.warn("Email can not be processed: #{e}\n\n#{mail_string}") if SiteSetting.log_mail_processing_failures
|
log_email_process_failure(mail_string, e)
|
||||||
|
|
||||||
message_template = case e
|
message_template = case e
|
||||||
when Email::Receiver::EmptyEmailError then :email_reject_empty
|
when Email::Receiver::EmptyEmailError then :email_reject_empty
|
||||||
|
@ -70,6 +84,10 @@ module Jobs
|
||||||
template_args[:rate_limit_description] = e.description
|
template_args[:rate_limit_description] = e.description
|
||||||
end
|
end
|
||||||
|
|
||||||
|
if message_template == :email_reject_auto_generated
|
||||||
|
template_args[:mark_as_reply_to_auto_generated] = true
|
||||||
|
end
|
||||||
|
|
||||||
if message_template
|
if message_template
|
||||||
# inform the user about the rejection
|
# inform the user about the rejection
|
||||||
message = Mail::Message.new(mail_string)
|
message = Mail::Message.new(mail_string)
|
||||||
|
@ -116,5 +134,17 @@ module Jobs
|
||||||
$redis.zadd(POLL_MAILBOX_ERRORS_KEY, now, now.to_s)
|
$redis.zadd(POLL_MAILBOX_ERRORS_KEY, now, now.to_s)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def set_incoming_email_rejection_message(incoming_email, message)
|
||||||
|
incoming_email.update_attributes!(rejection_message: message)
|
||||||
|
end
|
||||||
|
|
||||||
|
def log_email_process_failure(mail_string, exception)
|
||||||
|
if SiteSetting.log_mail_processing_failures
|
||||||
|
Rails.logger.warn("Email can not be processed: #{exception}\n\n#{mail_string}")
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -64,6 +64,8 @@ en:
|
||||||
reply_user_not_matching_error: "Happens when a reply came in from a different email address the notification was sent to."
|
reply_user_not_matching_error: "Happens when a reply came in from a different email address the notification was sent to."
|
||||||
topic_not_found_error: "Happens when a reply came in but the related topic has been deleted."
|
topic_not_found_error: "Happens when a reply came in but the related topic has been deleted."
|
||||||
topic_closed_error: "Happens when a reply came in but the related topic has been closed."
|
topic_closed_error: "Happens when a reply came in but the related topic has been closed."
|
||||||
|
bounced_email_report: "Email is a bounced email report."
|
||||||
|
auto_generated_email_reply: "Email contains a reply to an auto generated email."
|
||||||
|
|
||||||
errors: &errors
|
errors: &errors
|
||||||
format: ! '%{attribute} %{message}'
|
format: ! '%{attribute} %{message}'
|
||||||
|
|
|
@ -17,6 +17,9 @@ module Email
|
||||||
class MessageBuilder
|
class MessageBuilder
|
||||||
attr_reader :template_args
|
attr_reader :template_args
|
||||||
|
|
||||||
|
REPLY_TO_AUTO_GENERATED_HEADER_KEY = "X-Discourse-Reply-to-Auto-Generated".freeze
|
||||||
|
REPLY_TO_AUTO_GENERATED_HEADER_VALUE = "marked".freeze
|
||||||
|
|
||||||
def initialize(to, opts=nil)
|
def initialize(to, opts=nil)
|
||||||
@to = to
|
@to = to
|
||||||
@opts = opts || {}
|
@opts = opts || {}
|
||||||
|
@ -132,7 +135,11 @@ module Email
|
||||||
def header_args
|
def header_args
|
||||||
result = {}
|
result = {}
|
||||||
if @opts[:add_unsubscribe_link]
|
if @opts[:add_unsubscribe_link]
|
||||||
result['List-Unsubscribe'] = "<#{template_args[:user_preferences_url]}>" if @opts[:add_unsubscribe_link]
|
result['List-Unsubscribe'] = "<#{template_args[:user_preferences_url]}>"
|
||||||
|
end
|
||||||
|
|
||||||
|
if @opts[:mark_as_reply_to_auto_generated]
|
||||||
|
result[REPLY_TO_AUTO_GENERATED_HEADER_KEY] = REPLY_TO_AUTO_GENERATED_HEADER_VALUE
|
||||||
end
|
end
|
||||||
|
|
||||||
result['X-Discourse-Post-Id'] = @opts[:post_id].to_s if @opts[:post_id]
|
result['X-Discourse-Post-Id'] = @opts[:post_id].to_s if @opts[:post_id]
|
||||||
|
|
|
@ -12,6 +12,8 @@ module Email
|
||||||
class EmptyEmailError < ProcessingError; end
|
class EmptyEmailError < ProcessingError; end
|
||||||
class UserNotFoundError < ProcessingError; end
|
class UserNotFoundError < ProcessingError; end
|
||||||
class AutoGeneratedEmailError < ProcessingError; end
|
class AutoGeneratedEmailError < ProcessingError; end
|
||||||
|
class AutoGeneratedEmailReplyError < ProcessingError; end
|
||||||
|
class BouncedEmailError < ProcessingError; end
|
||||||
class NoBodyDetectedError < ProcessingError; end
|
class NoBodyDetectedError < ProcessingError; end
|
||||||
class InactiveUserError < ProcessingError; end
|
class InactiveUserError < ProcessingError; end
|
||||||
class BlockedUserError < ProcessingError; end
|
class BlockedUserError < ProcessingError; end
|
||||||
|
@ -62,6 +64,8 @@ module Email
|
||||||
body, @elided = select_body
|
body, @elided = select_body
|
||||||
body ||= ""
|
body ||= ""
|
||||||
|
|
||||||
|
raise BouncedEmailError if (@mail.bounced? && !@mail.retryable?)
|
||||||
|
raise AutoGeneratedEmailReplyError if check_reply_to_auto_generated_header
|
||||||
raise AutoGeneratedEmailError if is_auto_generated?
|
raise AutoGeneratedEmailError if is_auto_generated?
|
||||||
raise NoBodyDetectedError if body.blank? && !@mail.has_attachments?
|
raise NoBodyDetectedError if body.blank? && !@mail.has_attachments?
|
||||||
raise InactiveUserError if !user.active && !user.staged
|
raise InactiveUserError if !user.active && !user.staged
|
||||||
|
@ -422,6 +426,20 @@ module Email
|
||||||
!topic.topic_allowed_groups.where("group_id IN (SELECT group_id FROM group_users WHERE user_id = ?)", user.id).exists?
|
!topic.topic_allowed_groups.where("group_id IN (SELECT group_id FROM group_users WHERE user_id = ?)", user.id).exists?
|
||||||
end
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def check_reply_to_auto_generated_header
|
||||||
|
headers = Mail::Header.new(@mail.body.to_s.gsub("\r\n\r\n", "\r\n")).to_a
|
||||||
|
|
||||||
|
index = headers.find_index do |f|
|
||||||
|
f.name == Email::MessageBuilder::REPLY_TO_AUTO_GENERATED_HEADER_KEY
|
||||||
|
end
|
||||||
|
|
||||||
|
if index
|
||||||
|
headers[index].value == Email::MessageBuilder::REPLY_TO_AUTO_GENERATED_HEADER_VALUE
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
|
|
|
@ -136,10 +136,15 @@ describe Email::MessageBuilder do
|
||||||
|
|
||||||
context "header args" do
|
context "header args" do
|
||||||
|
|
||||||
let(:message_with_header_args) { Email::MessageBuilder.new(to_address,
|
let(:message_with_header_args) do
|
||||||
body: 'hello world',
|
Email::MessageBuilder.new(
|
||||||
topic_id: 1234,
|
to_address,
|
||||||
post_id: 4567) }
|
body: 'hello world',
|
||||||
|
topic_id: 1234,
|
||||||
|
post_id: 4567,
|
||||||
|
mark_as_reply_to_auto_generated: true
|
||||||
|
)
|
||||||
|
end
|
||||||
|
|
||||||
it "passes through a post_id" do
|
it "passes through a post_id" do
|
||||||
expect(message_with_header_args.header_args['X-Discourse-Post-Id']).to eq('4567')
|
expect(message_with_header_args.header_args['X-Discourse-Post-Id']).to eq('4567')
|
||||||
|
@ -149,6 +154,12 @@ describe Email::MessageBuilder do
|
||||||
expect(message_with_header_args.header_args['X-Discourse-Topic-Id']).to eq('1234')
|
expect(message_with_header_args.header_args['X-Discourse-Topic-Id']).to eq('1234')
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "marks the email as replying to an auto generated email" do
|
||||||
|
expect(message_with_header_args.header_args[
|
||||||
|
Email::MessageBuilder::REPLY_TO_AUTO_GENERATED_HEADER_KEY
|
||||||
|
]).to eq(Email::MessageBuilder::REPLY_TO_AUTO_GENERATED_HEADER_VALUE)
|
||||||
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
|
|
||||||
context "unsubscribe link" do
|
context "unsubscribe link" do
|
||||||
|
|
|
@ -8,10 +8,6 @@ describe Email::Receiver do
|
||||||
SiteSetting.reply_by_email_address = "reply+%{reply_key}@bar.com"
|
SiteSetting.reply_by_email_address = "reply+%{reply_key}@bar.com"
|
||||||
end
|
end
|
||||||
|
|
||||||
def email(email_name)
|
|
||||||
fixture_file("emails/#{email_name}.eml")
|
|
||||||
end
|
|
||||||
|
|
||||||
def process(email_name)
|
def process(email_name)
|
||||||
Email::Receiver.new(email(email_name)).process!
|
Email::Receiver.new(email(email_name)).process!
|
||||||
end
|
end
|
||||||
|
@ -54,6 +50,17 @@ describe Email::Receiver do
|
||||||
expect { process(:bad_destinations) }.to raise_error(Email::Receiver::BadDestinationAddress)
|
expect { process(:bad_destinations) }.to raise_error(Email::Receiver::BadDestinationAddress)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "raises an BouncerEmailError when email is a bounced email" do
|
||||||
|
expect { process(:bounced_email) }.to raise_error(Email::Receiver::BouncedEmailError)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "raises an AutoGeneratedEmailReplyError when email contains a reply marked
|
||||||
|
as reply to an auto generated email".squish do
|
||||||
|
|
||||||
|
expect { process(:bounced_email_2) }
|
||||||
|
.to raise_error(Email::Receiver::AutoGeneratedEmailReplyError)
|
||||||
|
end
|
||||||
|
|
||||||
context "reply" do
|
context "reply" do
|
||||||
|
|
||||||
let(:reply_key) { "4f97315cc828096c9cb34c6f1a0d6fe8" }
|
let(:reply_key) { "4f97315cc828096c9cb34c6f1a0d6fe8" }
|
||||||
|
|
39
spec/fixtures/emails/bounced_email.eml
vendored
Normal file
39
spec/fixtures/emails/bounced_email.eml
vendored
Normal file
|
@ -0,0 +1,39 @@
|
||||||
|
Delivered-To: someguy@discourse.org
|
||||||
|
Date: Thu, 7 Apr 2016 19:04:30 +0900 (JST)
|
||||||
|
From: MAILER-DAEMON@b-s-c.co.jp (Mail Delivery System)
|
||||||
|
Subject: Undelivered Mail Returned to Sender
|
||||||
|
To: someguy@discourse.org
|
||||||
|
MIME-Version: 1.0
|
||||||
|
Content-Type: multipart/report; report-type=delivery-status;
|
||||||
|
boundary="18F5D18A0075.1460023470/some@daemon.com"
|
||||||
|
|
||||||
|
This is a MIME-encapsulated message.
|
||||||
|
|
||||||
|
--18F5D18A0075.1460023470/some@daemon.com
|
||||||
|
Content-Description: Notification
|
||||||
|
Content-Type: text/plain; charset=us-ascii
|
||||||
|
|
||||||
|
Your email bounced
|
||||||
|
|
||||||
|
--18F5D18A0075.1460023470/some@daemon.com
|
||||||
|
Content-Description: Delivery report
|
||||||
|
Content-Type: message/delivery-status
|
||||||
|
|
||||||
|
Final-Recipient: rfc822; linux-admin@b-s-c.co.jp
|
||||||
|
Original-Recipient: rfc822;linux-admin@b-s-c.co.jp
|
||||||
|
Action: failed
|
||||||
|
Status: 5.1.1
|
||||||
|
Diagnostic-Code: X-Postfix; unknown user: "linux-admin"
|
||||||
|
|
||||||
|
--18F5D18A0075.1460023470/some@daemon.com
|
||||||
|
Content-Description: Undelivered Message
|
||||||
|
Content-Type: message/rfc822
|
||||||
|
|
||||||
|
Return-Path: <someguy@discourse.org>
|
||||||
|
Date: Thu, 07 Apr 2016 03:04:28 -0700 (PDT)
|
||||||
|
From: someguy@discourse.org
|
||||||
|
X-Discourse-Auto-Generated: marked
|
||||||
|
|
||||||
|
This is the body
|
||||||
|
|
||||||
|
--18F5D18A0075.1460023470/some@daemon.com--
|
29
spec/fixtures/emails/bounced_email_2.eml
vendored
Normal file
29
spec/fixtures/emails/bounced_email_2.eml
vendored
Normal file
|
@ -0,0 +1,29 @@
|
||||||
|
Delivered-To: someguy@discourse.org
|
||||||
|
Date: Thu, 7 Apr 2016 19:04:30 +0900 (JST)
|
||||||
|
From: MAILER-DAEMON@b-s-c.co.jp (Mail Delivery System)
|
||||||
|
Subject: Undelivered Mail Returned to Sender
|
||||||
|
To: someguy@discourse.org
|
||||||
|
MIME-Version: 1.0
|
||||||
|
Content-Type: multipart/report; report-type=delivery-status;
|
||||||
|
boundary="18F5D18A0075.1460023470/some@daemon.com"
|
||||||
|
|
||||||
|
This is a MIME-encapsulated message.
|
||||||
|
|
||||||
|
--18F5D18A0075.1460023470/some@daemon.com
|
||||||
|
Content-Description: Notification
|
||||||
|
Content-Type: text/plain; charset=us-ascii
|
||||||
|
|
||||||
|
Your email bounced
|
||||||
|
|
||||||
|
--18F5D18A0075.1460023470/some@daemon.com
|
||||||
|
Content-Description: Undelivered Message
|
||||||
|
Content-Type: message/rfc822
|
||||||
|
|
||||||
|
Return-Path: <someguy@discourse.org>
|
||||||
|
Date: Thu, 07 Apr 2016 03:04:28 -0700 (PDT)
|
||||||
|
From: someguy@discourse.org
|
||||||
|
X-Discourse-Reply-to-Auto-Generated: marked
|
||||||
|
|
||||||
|
This is the body
|
||||||
|
|
||||||
|
--18F5D18A0075.1460023470/some@daemon.com--
|
|
@ -44,4 +44,32 @@ describe Jobs::PollMailbox do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe "#process_popmail" do
|
||||||
|
def process_popmail(email_name)
|
||||||
|
pop_mail = stub("pop mail")
|
||||||
|
pop_mail.expects(:pop).returns(email(email_name))
|
||||||
|
Jobs::PollMailbox.new.process_popmail(pop_mail)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not reply to a bounced email" do
|
||||||
|
expect { process_popmail(:bounced_email) }.to_not change { ActionMailer::Base.deliveries.count }
|
||||||
|
|
||||||
|
incoming_email = IncomingEmail.last
|
||||||
|
|
||||||
|
expect(incoming_email.rejection_message).to eq(
|
||||||
|
I18n.t("email.incoming.errors.bounced_email_report")
|
||||||
|
)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not reply to an email containing a reply to an auto generated email" do
|
||||||
|
expect { process_popmail(:bounced_email_2) }.to_not change { ActionMailer::Base.deliveries.count }
|
||||||
|
|
||||||
|
incoming_email = IncomingEmail.last
|
||||||
|
|
||||||
|
expect(incoming_email.rejection_message).to eq(
|
||||||
|
I18n.t("email.incoming.errors.auto_generated_email_reply")
|
||||||
|
)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
|
|
|
@ -79,4 +79,8 @@ module Helpers
|
||||||
result
|
result
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def email(email_name)
|
||||||
|
fixture_file("emails/#{email_name}.eml")
|
||||||
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue