FEATURE: handle bounced emails

This commit is contained in:
Régis Hanol 2016-05-02 23:15:32 +02:00
parent b7b5caa50e
commit 8e611ec7a1
27 changed files with 354 additions and 23 deletions

View file

@ -0,0 +1,9 @@
import AdminEmailLogsController from 'admin/controllers/admin-email-logs';
import debounce from 'discourse/lib/debounce';
import EmailLog from 'admin/models/email-log';
export default AdminEmailLogsController.extend({
filterEmailLogs: debounce(function() {
EmailLog.findAll(this.get("filter")).then(logs => this.set("model", logs));
}, 250).observes("filter.{user,address,type,skipped_reason}")
});

View file

@ -0,0 +1,2 @@
import AdminEmailLogs from 'admin/routes/admin-email-logs';
export default AdminEmailLogs.extend({ status: "bounced" });

View file

@ -10,6 +10,7 @@ export default {
this.resource('adminEmail', { path: '/email'}, function() {
this.route('sent');
this.route('skipped');
this.route('bounced');
this.route('received');
this.route('rejected');
this.route('previewDigest', { path: '/preview-digest' });

View file

@ -0,0 +1,49 @@
{{#load-more selector=".email-list tr" action="loadMore"}}
<table class='table email-list'>
<thead>
<tr>
<th>{{i18n 'admin.email.time'}}</th>
<th>{{i18n 'admin.email.user'}}</th>
<th>{{i18n 'admin.email.to_address'}}</th>
<th>{{i18n 'admin.email.email_type'}}</th>
<th>{{i18n 'admin.email.skipped_reason'}}</th>
</tr>
</thead>
<tr class="filters">
<td>{{i18n 'admin.email.logs.filters.title'}}</td>
<td>{{text-field value=filter.user placeholderKey="admin.email.logs.filters.user_placeholder"}}</td>
<td>{{text-field value=filter.address placeholderKey="admin.email.logs.filters.address_placeholder"}}</td>
<td>{{text-field value=filter.type placeholderKey="admin.email.logs.filters.type_placeholder"}}</td>
<td>{{text-field value=filter.skipped_reason placeholderKey="admin.email.logs.filters.skipped_reason_placeholder"}}</td>
</tr>
{{#each l in model}}
<tr>
<td>{{format-date l.created_at}}</td>
<td>
{{#if l.user}}
{{#link-to 'adminUser' l.user}}{{avatar l.user imageSize="tiny"}}{{/link-to}}
{{#link-to 'adminUser' l.user}}{{l.user.username}}{{/link-to}}
{{else}}
&mdash;
{{/if}}
</td>
<td><a href='mailto:{{unbound l.to_address}}'>{{l.to_address}}</a></td>
<td>{{l.email_type}}</td>
<td>
{{#if l.post_url}}
<a href="{{l.post_url}}">{{l.skipped_reason}}</a>
{{else}}
{{l.skipped_reason}}
{{/if}}
</td>
</tr>
{{else}}
<tr><td colspan="5">{{i18n 'admin.email.logs.none'}}</td></tr>
{{/each}}
</table>
{{/load-more}}
{{conditional-loading-spinner condition=loading}}

View file

@ -4,6 +4,7 @@
{{nav-item route='adminCustomizeEmailTemplates' label='admin.email.templates'}}
{{nav-item route='adminEmail.sent' label='admin.email.sent'}}
{{nav-item route='adminEmail.skipped' label='admin.email.skipped'}}
{{nav-item route='adminEmail.bounced' label='admin.email.bounced'}}
{{nav-item route='adminEmail.received' label='admin.email.received'}}
{{nav-item route='adminEmail.rejected' label='admin.email.rejected'}}
{{/admin-nav}}

View file

@ -6,8 +6,6 @@
//= require admin/models/tl3-requirements
//= require admin/models/admin-user
//= require_tree ./admin/models
//= require admin/routes/admin-email-logs
//= require admin/controllers/admin-email-skipped
//= require discourse/lib/export-result
//= require_tree ./admin

View file

@ -27,6 +27,11 @@ class Admin::EmailController < Admin::AdminController
render_serialized(email_logs, EmailLogSerializer)
end
def bounced
email_logs = filter_email_logs(EmailLog.bounced, params)
render_serialized(email_logs, EmailLogSerializer)
end
def received
incoming_emails = filter_incoming_emails(IncomingEmail, params)
render_serialized(incoming_emails, IncomingEmailSerializer)

View file

@ -114,7 +114,11 @@ module Jobs
end
if EmailLog.reached_max_emails?(user)
return skip_message(I18n.t('email_log.exceeded_limit'))
return skip_message(I18n.t('email_log.exceeded_emails_limit'))
end
if (user.user_stat.try(:bounce_score) || 0) >= SiteSetting.bounce_score_threshold
return skip_message(I18n.t('email_log.exceeded_bounces_limit'))
end
message = EmailLog.unique_email_per_post(post, user) do

View file

@ -10,7 +10,7 @@ module Jobs
UserAction.ensure_consistency!
TopicFeaturedUsers.ensure_consistency!
PostRevision.ensure_consistency!
UserStat.update_view_counts(13.hours.ago)
UserStat.ensure_consistency!(13.hours.ago)
Topic.ensure_consistency!
Badge.ensure_consistency!
CategoryUser.ensure_consistency!

View file

@ -29,7 +29,8 @@ module Jobs
log_email_process_failure(mail_string, e)
set_incoming_email_rejection_message(
receiver.incoming_email, I18n.t("email.incoming.errors.bounced_email_report")
receiver.incoming_email,
I18n.t("email.incoming.errors.bounced_email_report")
)
rescue Email::Receiver::AutoGeneratedEmailReplyError => e
log_email_process_failure(mail_string, e)
@ -41,9 +42,7 @@ module Jobs
rescue => e
rejection_message = handle_failure(mail_string, e)
if rejection_message.present? && receiver && (incoming_email = receiver.incoming_email)
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

View file

@ -9,6 +9,7 @@ class EmailLog < ActiveRecord::Base
scope :sent, -> { where(skipped: false) }
scope :skipped, -> { where(skipped: true) }
scope :bounced, -> { sent.where(bounced: true) }
after_create do
# Update last_emailed_at if the user_id is present and email was sent

View file

@ -10,7 +10,9 @@ class EmailToken < ActiveRecord::Base
after_create do
# Expire the previous tokens
EmailToken.where(['user_id = ? and id != ?', self.user_id, self.id]).update_all 'expired = true'
EmailToken.where(user_id: self.user_id)
.where("id != ?", self.id)
.update_all(expired: true)
end
def self.token_length
@ -38,7 +40,7 @@ class EmailToken < ActiveRecord::Base
end
def self.valid_token_format?(token)
return token.present? && token =~ /[a-f0-9]{#{token.length/2}}/i
token.present? && token =~ /\h{#{token.length/2}}/i
end
def self.atomic_confirm(token)
@ -51,11 +53,12 @@ class EmailToken < ActiveRecord::Base
user = email_token.user
failure[:user] = user
row_count = EmailToken.where(id: email_token.id, expired: false).update_all 'confirmed = true'
if row_count == 1
return { success: true, user: user, email_token: email_token }
end
return failure
if row_count == 1
{ success: true, user: user, email_token: email_token }
else
failure
end
end
def self.confirm(token)
@ -81,7 +84,11 @@ class EmailToken < ActiveRecord::Base
end
def self.confirmable(token)
EmailToken.where("token = ? and expired = FALSE AND ((NOT confirmed AND created_at >= ?) OR (confirmed AND created_at >= ?))", token, EmailToken.valid_after, EmailToken.confirm_valid_after).includes(:user).first
EmailToken.where(token: token)
.where(expired: false)
.where("(NOT confirmed AND created_at >= ?) OR (confirmed AND created_at >= ?)", EmailToken.valid_after, EmailToken.confirm_valid_after)
.includes(:user)
.first
end
end

View file

@ -52,7 +52,8 @@ class UserHistory < ActiveRecord::Base
grant_moderation: 34,
revoke_moderation: 35,
backup_operation: 36,
rate_limited_like: 37 # not used anymore
rate_limited_like: 37, # not used anymore
revoke_email: 38
)
end

View file

@ -3,6 +3,17 @@ class UserStat < ActiveRecord::Base
belongs_to :user
after_save :trigger_badges
def self.ensure_consistency!(last_seen = 1.hour.ago)
reset_bounce_scores
update_view_counts(last_seen)
end
def self.reset_bounce_scores
UserStat.where("reset_bounce_score_after < now()")
.where("bounce_score > 0")
.update_all(bounce_score: 0)
end
# Updates the denormalized view counts for all users
def self.update_view_counts(last_seen = 1.hour.ago)

View file

@ -9,7 +9,8 @@ class EmailLogSerializer < ApplicationSerializer
:skipped,
:skipped_reason,
:post_url,
:post_description
:post_description,
:bounced
has_one :user, serializer: BasicUserSerializer, embed: :objects

View file

@ -334,6 +334,14 @@ class StaffActionLogger
}))
end
def log_revoke_email(user, opts={})
UserHistory.create(params(opts).merge({
action: UserHistory.actions[:revoke_email],
target_user_id: user.id,
details: user.email
}))
end
private
def params(opts=nil)

View file

@ -2303,6 +2303,7 @@ en:
test_error: "There was a problem sending the test email. Please double-check your mail settings, verify that your host is not blocking mail connections, and try again."
sent: "Sent"
skipped: "Skipped"
bounced: "Bounced"
received: "Received"
rejected: "Rejected"
sent_at: "Sent At"

View file

@ -1175,6 +1175,7 @@ en:
enable_staged_users: "Automatically create staged users when processing incoming emails."
auto_generated_whitelist: "List of email addresses that won't be checked for auto-generated content."
block_auto_generated_emails: "Block incoming emails identified as being auto generated."
bounce_score_threshold: "The maximum user bounce score before the they are deactivated. A soft bounce adds 1, a hard bounce adds 2."
manual_polling_enabled: "Push emails using the API for email replies."
pop3_polling_enabled: "Poll via POP3 for email replies."
@ -2420,7 +2421,8 @@ en:
post_deleted: "post was deleted by the author"
user_suspended: "user was suspended"
already_read: "user has already read this post"
exceeded_limit: "Exceeded max_emails_per_day_per_user"
exceeded_emails_limit: "Exceeded max_emails_per_day_per_user"
exceeded_bounces_limit: "Exceeded bounce_score_threshold"
message_blank: "message is blank"
message_to_blank: "message.to is blank"
text_part_body_blank: "text_part.body is blank"

View file

@ -124,6 +124,7 @@ Discourse::Application.routes.draw do
post "test"
get "sent"
get "skipped"
get "bounced"
get "received"
get "rejected"
get "/incoming/:id/raw" => "email#raw_email"

View file

@ -571,6 +571,9 @@ email:
default: ''
type: list
block_auto_generated_emails: true
bounce_score_threshold:
default: 4
min: 1
files:

View file

@ -0,0 +1,8 @@
class AddSupportForBouncedEmails < ActiveRecord::Migration
def change
add_column :email_logs, :bounced, :boolean, null: false, default: false
add_column :incoming_emails, :is_bounce, :boolean, null: false, default: false
add_column :user_stats, :bounce_score, :integer, null: false, default: 0
add_column :user_stats, :reset_bounce_score_after, :datetime
end
end

View file

@ -56,10 +56,6 @@ module Email
end
def process_internal
# temporarily disable processing automated replies to VERP
return if @mail.destinations.any? { |to| to[/\+verp-\h{32}@/i] }
raise BouncedEmailError if @mail.bounced? && !@mail.retryable?
raise ScreenedEmailError if ScreenedEmail.should_block?(@from_email)
user = find_or_create_user(@from_email, @from_display_name)
@ -68,6 +64,7 @@ module Email
@incoming_email.update_columns(user_id: user.id)
raise BouncedEmailError if is_bounce?
raise InactiveUserError if !user.active && !user.staged
raise BlockedUserError if user.blocked
@ -132,6 +129,61 @@ module Email
end
end
SOFT_BOUNCE_SCORE ||= 1
HARD_BOUNCE_SCORE ||= 2
def is_bounce?
return false unless @mail.bounced? || verp
@incoming_email.update_columns(is_bounce: true)
if verp
bounce_key = verp[/\+verp-(\h{32})@/, 1]
if bounce_key && (email_log = EmailLog.find_by(bounce_key: bounce_key))
email_log.update_columns(bounced: true)
if @mail.error_status.present?
if @mail.error_status.start_with?("4.")
update_bounce_score(email_log.user.email, SOFT_BOUNCE_SCORE)
elsif @mail.error_status.start_with?("5.")
update_bounce_score(email_log.user.email, HARD_BOUNCE_SCORE)
end
end
end
end
true
end
def verp
@verp ||= @mail.destinations.select { |to| to[/\+verp-\h{32}@/] }.first
end
def update_bounce_score(email, score)
# only update bounce score once per day
key = "bounce_score:#{email}:#{Date.today}"
if $redis.setnx(key, "1")
$redis.expire(key, 25.hours)
if user = User.find_by(email: email)
user.user_stat.bounce_score += score
user.user_stat.reset_bounce_score_after = 30.days.from_now
user.user_stat.save
if user.active && user.user_stat.bounce_score >= SiteSetting.bounce_score_threshold
user.deactivate
StaffActionLogger.new(Discourse.system_user).log_revoke_email(user)
EmailToken.where(email: user.email, confirmed: true).update_all(confirmed: false)
end
end
true
else
false
end
end
def is_auto_generated?
return false if SiteSetting.auto_generated_whitelist.split('|').include?(@from_email)
@mail[:precedence].to_s[/list|junk|bulk|auto_reply/i] ||

View file

@ -55,14 +55,57 @@ describe Email::Receiver do
expect { process(:bad_destinations) }.to raise_error(Email::Receiver::BadDestinationAddress)
end
it "raises an BouncerEmailError when email is a bounced email" do
it "raises a BouncerEmailError when email is a bounced email" do
expect { process(:bounced_email) }.to raise_error(Email::Receiver::BouncedEmailError)
expect(IncomingEmail.last.is_bounce).to eq(true)
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
let(:bounce_key) { "14b08c855160d67f2e0c2f8ef36e251e" }
let(:bounce_key_2) { "b542fb5a9bacda6d28cc061d18e4eb83" }
let!(:user) { Fabricate(:user, email: "foo@bar.com", active: true) }
let!(:email_log) { Fabricate(:email_log, user: user, bounce_key: bounce_key) }
let!(:email_log_2) { Fabricate(:email_log, user: user, bounce_key: bounce_key_2) }
before do
$redis.del("bounce_score:#{user.email}:#{Date.today}")
$redis.del("bounce_score:#{user.email}:#{2.days.from_now.to_date}")
end
it "deals with soft bounces" do
expect { process(:soft_bounce_via_verp) }.to raise_error(Email::Receiver::BouncedEmailError)
email_log.reload
expect(email_log.bounced).to eq(true)
expect(email_log.user.active).to eq(true)
expect(email_log.user.user_stat.bounce_score).to eq(1)
end
it "deals with hard bounces" do
expect { process(:hard_bounce_via_verp) }.to raise_error(Email::Receiver::BouncedEmailError)
email_log.reload
expect(email_log.bounced).to eq(true)
expect(email_log.user.active).to eq(true)
expect(email_log.user.user_stat.bounce_score).to eq(2)
Timecop.freeze(2.days.from_now) do
expect { process(:hard_bounce_via_verp_2) }.to raise_error(Email::Receiver::BouncedEmailError)
email_log_2.reload
expect(email_log_2.bounced).to eq(true)
expect(email_log_2.user.active).to eq(false)
expect(email_log_2.user.user_stat.bounce_score).to eq(4)
end
end
end
context "reply" do
let(:reply_key) { "4f97315cc828096c9cb34c6f1a0d6fe8" }

View file

@ -0,0 +1,39 @@
Delivered-To: foo+verp-14b08c855160d67f2e0c2f8ef36e251e@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: foo+verp-14b08c855160d67f2e0c2f8ef36e251e@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: <foo+verp-14b08c855160d67f2e0c2f8ef36e251e@discourse.org>
Date: Thu, 07 Apr 2016 03:04:28 -0700 (PDT)
From: foo+verp-14b08c855160d67f2e0c2f8ef36e251e@discourse.org
X-Discourse-Auto-Generated: marked
This is the body
--18F5D18A0075.1460023470/some@daemon.com--

View file

@ -0,0 +1,40 @@
Delivered-To: foo+verp-b542fb5a9bacda6d28cc061d18e4eb83@discourse.org
Date: Thu, 7 Apr 2016 19:04:30 +0900 (JST)
From: MAILER-DAEMON@b-s-c.co.jp (Mail Delivery System)
Message-ID: <hard-bounced@foo.bar.mail>
Subject: Undelivered Mail Returned to Sender
To: foo+verp-b542fb5a9bacda6d28cc061d18e4eb83@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: <foo+verp-b542fb5a9bacda6d28cc061d18e4eb83@discourse.org>
Date: Thu, 07 Apr 2016 03:04:28 -0700 (PDT)
From: foo+verp-b542fb5a9bacda6d28cc061d18e4eb83@discourse.org
X-Discourse-Auto-Generated: marked
This is the body
--18F5D18A0075.1460023470/some@daemon.com--

View file

@ -0,0 +1,39 @@
Delivered-To: foo+verp-14b08c855160d67f2e0c2f8ef36e251e@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: foo+verp-14b08c855160d67f2e0c2f8ef36e251e@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: 4.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: <foo+verp-14b08c855160d67f2e0c2f8ef36e251e@discourse.org>
Date: Thu, 07 Apr 2016 03:04:28 -0700 (PDT)
From: foo+verp-14b08c855160d67f2e0c2f8ef36e251e@discourse.org
X-Discourse-Auto-Generated: marked
This is the body
--18F5D18A0075.1460023470/some@daemon.com--

View file

@ -204,6 +204,12 @@ describe Jobs::UserEmail do
expect(EmailLog.where(user_id: user.id, skipped: true).count).to eq(1)
end
it "does not send notification if bounce threshold is reached" do
user.user_stat.update(bounce_score: SiteSetting.bounce_score_threshold)
Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id, post_id: post.id)
expect(EmailLog.where(user_id: user.id, skipped: true).count).to eq(1)
end
it "doesn't send the mail if the user is using mailing list mode" do
Email::Sender.any_instance.expects(:send).never
user.user_option.update_column(:mailing_list_mode, true)