don't send more than 1 reply per day to auto-generated emails
This commit is contained in:
parent
1411eedad3
commit
214f5bff5c
|
@ -73,7 +73,6 @@ en:
|
||||||
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_error: "Email is a bounced email report."
|
bounced_email_error: "Email is a bounced email report."
|
||||||
auto_generated_email_reply: "Email contains a reply to an auto generated email."
|
|
||||||
screened_email_error: "Happens when the sender's email address was already screened."
|
screened_email_error: "Happens when the sender's email address was already screened."
|
||||||
|
|
||||||
errors: &errors
|
errors: &errors
|
||||||
|
|
|
@ -17,9 +17,6 @@ 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 || {}
|
||||||
|
@ -138,10 +135,6 @@ module Email
|
||||||
result['List-Unsubscribe'] = "<#{template_args[:user_preferences_url]}>"
|
result['List-Unsubscribe'] = "<#{template_args[:user_preferences_url]}>"
|
||||||
end
|
end
|
||||||
|
|
||||||
if @opts[:mark_as_reply_to_auto_generated]
|
|
||||||
result[REPLY_TO_AUTO_GENERATED_HEADER_KEY] = REPLY_TO_AUTO_GENERATED_HEADER_VALUE
|
|
||||||
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]
|
||||||
result['X-Discourse-Topic-Id'] = @opts[:topic_id].to_s if @opts[:topic_id]
|
result['X-Discourse-Topic-Id'] = @opts[:topic_id].to_s if @opts[:topic_id]
|
||||||
|
|
||||||
|
|
|
@ -15,16 +15,14 @@ module Email
|
||||||
receiver = Email::Receiver.new(@mail)
|
receiver = Email::Receiver.new(@mail)
|
||||||
receiver.process!
|
receiver.process!
|
||||||
rescue Email::Receiver::BouncedEmailError => e
|
rescue Email::Receiver::BouncedEmailError => e
|
||||||
|
# never reply to bounced emails
|
||||||
log_email_process_failure(@mail, e)
|
log_email_process_failure(@mail, e)
|
||||||
set_incoming_email_rejection_message(receiver.incoming_email, I18n.t("emails.incoming.errors.bounced_email_error"))
|
set_incoming_email_rejection_message(receiver.incoming_email, I18n.t("emails.incoming.errors.bounced_email_error"))
|
||||||
rescue Email::Receiver::AutoGeneratedEmailReplyError => e
|
|
||||||
log_email_process_failure(@mail, e)
|
|
||||||
set_incoming_email_rejection_message(receiver.incoming_email, I18n.t("emails.incoming.errors.auto_generated_email_reply"))
|
|
||||||
rescue => e
|
rescue => e
|
||||||
log_email_process_failure(@mail, e)
|
log_email_process_failure(@mail, e)
|
||||||
|
incoming_email = receiver.try(:incoming_email)
|
||||||
rejection_message = handle_failure(@mail, e)
|
rejection_message = handle_failure(@mail, e, incoming_email)
|
||||||
if rejection_message.present? && receiver && (incoming_email = receiver.incoming_email)
|
if rejection_message.present?
|
||||||
set_incoming_email_rejection_message(incoming_email, rejection_message.body.to_s)
|
set_incoming_email_rejection_message(incoming_email, rejection_message.body.to_s)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -32,7 +30,7 @@ module Email
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
def handle_failure(mail_string, e)
|
def handle_failure(mail_string, e, incoming_email)
|
||||||
message_template = case e
|
message_template = case e
|
||||||
when Email::Receiver::EmptyEmailError then :email_reject_empty
|
when Email::Receiver::EmptyEmailError then :email_reject_empty
|
||||||
when Email::Receiver::NoBodyDetectedError then :email_reject_empty
|
when Email::Receiver::NoBodyDetectedError then :email_reject_empty
|
||||||
|
@ -67,10 +65,6 @@ module Email
|
||||||
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)
|
||||||
|
@ -79,7 +73,11 @@ module Email
|
||||||
template_args[:site_name] = SiteSetting.title
|
template_args[:site_name] = SiteSetting.title
|
||||||
|
|
||||||
client_message = RejectionMailer.send_rejection(message_template, message.from, template_args)
|
client_message = RejectionMailer.send_rejection(message_template, message.from, template_args)
|
||||||
Email::Sender.new(client_message, message_template).send
|
|
||||||
|
# don't send more than 1 reply per day to auto-generated emails
|
||||||
|
if !incoming_email.try(:is_auto_generated) || can_reply_to_auto_generated?(message.from)
|
||||||
|
Email::Sender.new(client_message, message_template).send
|
||||||
|
end
|
||||||
else
|
else
|
||||||
Rails.logger.error("Unrecognized error type (#{e}) when processing incoming email\n\nMail:\n#{mail_string}")
|
Rails.logger.error("Unrecognized error type (#{e}) when processing incoming email\n\nMail:\n#{mail_string}")
|
||||||
end
|
end
|
||||||
|
@ -87,6 +85,17 @@ module Email
|
||||||
client_message
|
client_message
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def can_reply_to_auto_generated?(email)
|
||||||
|
key = "auto_generated_reply:#{email}:#{Date.today}"
|
||||||
|
|
||||||
|
if $redis.setnx(key, "1")
|
||||||
|
$redis.expire(key, 25.hours)
|
||||||
|
true
|
||||||
|
else
|
||||||
|
false
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def set_incoming_email_rejection_message(incoming_email, message)
|
def set_incoming_email_rejection_message(incoming_email, message)
|
||||||
incoming_email.update_attributes!(rejection_message: message)
|
incoming_email.update_attributes!(rejection_message: message)
|
||||||
end
|
end
|
||||||
|
|
|
@ -13,7 +13,6 @@ module Email
|
||||||
class ScreenedEmailError < ProcessingError; end
|
class ScreenedEmailError < 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 BouncedEmailError < ProcessingError; end
|
||||||
class NoBodyDetectedError < ProcessingError; end
|
class NoBodyDetectedError < ProcessingError; end
|
||||||
class InactiveUserError < ProcessingError; end
|
class InactiveUserError < ProcessingError; end
|
||||||
|
@ -82,8 +81,7 @@ module Email
|
||||||
|
|
||||||
if is_auto_generated?
|
if is_auto_generated?
|
||||||
@incoming_email.update_columns(is_auto_generated: true)
|
@incoming_email.update_columns(is_auto_generated: true)
|
||||||
raise AutoGeneratedEmailReplyError if check_reply_to_auto_generated_header
|
raise AutoGeneratedEmailError if SiteSetting.block_auto_generated_emails?
|
||||||
raise AutoGeneratedEmailError if SiteSetting.block_auto_generated_emails?
|
|
||||||
end
|
end
|
||||||
|
|
||||||
if action = subscription_action_for(body, subject)
|
if action = subscription_action_for(body, subject)
|
||||||
|
@ -151,20 +149,20 @@ module Email
|
||||||
if bounce_key && (email_log = EmailLog.find_by(bounce_key: bounce_key))
|
if bounce_key && (email_log = EmailLog.find_by(bounce_key: bounce_key))
|
||||||
email_log.update_columns(bounced: true)
|
email_log.update_columns(bounced: true)
|
||||||
email = email_log.user.try(:email) || @from_email
|
email = email_log.user.try(:email) || @from_email
|
||||||
if email.present?
|
if email.present? && @mail.error_status.present?
|
||||||
if @mail.error_status.present?
|
if @mail.error_status.start_with?("4.")
|
||||||
if @mail.error_status.start_with?("4.")
|
Email::Receiver.update_bounce_score(email, SOFT_BOUNCE_SCORE)
|
||||||
Email::Receiver.update_bounce_score(email, SOFT_BOUNCE_SCORE)
|
else @mail.error_status.start_with?("5.")
|
||||||
elsif @mail.error_status.start_with?("5.")
|
|
||||||
Email::Receiver.update_bounce_score(email, HARD_BOUNCE_SCORE)
|
|
||||||
end
|
|
||||||
elsif is_auto_generated?
|
|
||||||
Email::Receiver.update_bounce_score(email, HARD_BOUNCE_SCORE)
|
Email::Receiver.update_bounce_score(email, HARD_BOUNCE_SCORE)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
if is_auto_generated?
|
||||||
|
Email::Receiver.update_bounce_score(@from_email, SOFT_BOUNCE_SCORE)
|
||||||
|
end
|
||||||
|
|
||||||
true
|
true
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -534,20 +532,6 @@ 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
|
||||||
|
|
|
@ -142,7 +142,6 @@ describe Email::MessageBuilder do
|
||||||
body: 'hello world',
|
body: 'hello world',
|
||||||
topic_id: 1234,
|
topic_id: 1234,
|
||||||
post_id: 4567,
|
post_id: 4567,
|
||||||
mark_as_reply_to_auto_generated: true
|
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -154,12 +153,6 @@ 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
|
||||||
|
|
|
@ -61,10 +61,6 @@ describe Email::Receiver do
|
||||||
expect(IncomingEmail.last.is_bounce).to eq(true)
|
expect(IncomingEmail.last.is_bounce).to eq(true)
|
||||||
end
|
end
|
||||||
|
|
||||||
it "raises an AutoGeneratedEmailReplyError when email contains a marked reply" do
|
|
||||||
expect { process(:bounced_email_2) }.to raise_error(Email::Receiver::AutoGeneratedEmailReplyError)
|
|
||||||
end
|
|
||||||
|
|
||||||
context "bounces to VERP" do
|
context "bounces to VERP" do
|
||||||
|
|
||||||
let(:bounce_key) { "14b08c855160d67f2e0c2f8ef36e251e" }
|
let(:bounce_key) { "14b08c855160d67f2e0c2f8ef36e251e" }
|
||||||
|
|
|
@ -1,29 +0,0 @@
|
||||||
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--
|
|
|
@ -90,15 +90,6 @@ describe Jobs::PollMailbox do
|
||||||
)
|
)
|
||||||
end
|
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("emails.incoming.errors.auto_generated_email_reply")
|
|
||||||
)
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue