From 7d9f2265b9888f3176f1c248dee91c3c92093761 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 18 Apr 2016 22:58:30 +0200 Subject: [PATCH] FIX: improve support for handling emails coming from screened email addresses --- .../admin/templates/email-rejected.hbs | 6 +- .../admin/email_templates_controller.rb | 1 + app/jobs/scheduled/poll_mailbox.rb | 1 + app/models/user.rb | 1 + config/locales/server.en.yml | 8 +++ lib/email/receiver.rb | 57 +++++++++++-------- spec/components/email/receiver_spec.rb | 7 ++- spec/fixtures/emails/screened_email.eml | 9 +++ 8 files changed, 63 insertions(+), 27 deletions(-) create mode 100644 spec/fixtures/emails/screened_email.eml diff --git a/app/assets/javascripts/admin/templates/email-rejected.hbs b/app/assets/javascripts/admin/templates/email-rejected.hbs index 664eb7286..001a99f14 100644 --- a/app/assets/javascripts/admin/templates/email-rejected.hbs +++ b/app/assets/javascripts/admin/templates/email-rejected.hbs @@ -29,7 +29,11 @@ {{email.from_address}} {{/link-to}} {{else}} - — + {{#if email.from_address}} + {{email.from_address}} + {{else}} + — + {{/if}} {{/if}} diff --git a/app/controllers/admin/email_templates_controller.rb b/app/controllers/admin/email_templates_controller.rb index e7d632580..4622e9da2 100644 --- a/app/controllers/admin/email_templates_controller.rb +++ b/app/controllers/admin/email_templates_controller.rb @@ -14,6 +14,7 @@ class Admin::EmailTemplatesController < Admin::AdminController "system_messages.email_reject_post_error_specified", "system_messages.email_reject_user_not_found", "system_messages.email_reject_reply_key", "system_messages.email_reject_topic_closed", "system_messages.email_reject_topic_not_found", "system_messages.email_reject_trust_level", + "system_messages.email_reject_screened_email", "system_messages.pending_users_reminder", "system_messages.post_hidden", "system_messages.restore_failed", "system_messages.restore_succeeded", "system_messages.spam_post_blocked", "system_messages.too_many_spam_flags", diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index af2064930..44dce2adb 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/app/jobs/scheduled/poll_mailbox.rb @@ -55,6 +55,7 @@ module Jobs when Email::Receiver::EmptyEmailError then :email_reject_empty when Email::Receiver::NoBodyDetectedError then :email_reject_empty when Email::Receiver::UserNotFoundError then :email_reject_user_not_found + when Email::Receiver::ScreenedEmailError then :email_reject_screened_email when Email::Receiver::AutoGeneratedEmailError then :email_reject_auto_generated when Email::Receiver::InactiveUserError then :email_reject_inactive_user when Email::Receiver::BlockedUserError then :email_reject_blocked_user diff --git a/app/models/user.rb b/app/models/user.rb index 99b67475c..0909d6176 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -25,6 +25,7 @@ class User < ActiveRecord::Base has_many :user_badges, -> { where('user_badges.badge_id IN (SELECT id FROM badges WHERE enabled)') }, dependent: :destroy has_many :badges, through: :user_badges has_many :email_logs, dependent: :delete_all + has_many :incoming_emails, dependent: :delete_all has_many :post_timings has_many :topic_allowed_users, dependent: :destroy has_many :topics_allowed, through: :topic_allowed_users, source: :topic diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index b9cbcf59a..91eb6ac9f 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -66,6 +66,7 @@ en: 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." + screened_email_error: "Happens when the sender's email address was already screened." errors: &errors format: ! '%{attribute} %{message}' @@ -1857,6 +1858,13 @@ en: Your reply was sent from an unknown email address. Try sending from another email address, or contact a staff member. + email_reject_screened_email: + subject_template: "[%{site_name}] Email issue -- Blocked Email" + text_body_template: | + We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work. + + Your reply was sent from a blocked email address. Try sending from another email address, or contact a staff member. + email_reject_inactive_user: subject_template: "[%{site_name}] Email issue -- Inactive User" text_body_template: | diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 42c862ab0..9af176b1e 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -8,23 +8,24 @@ module Email class Receiver include ActionView::Helpers::NumberHelper - class ProcessingError < StandardError; end - class EmptyEmailError < ProcessingError; end - class UserNotFoundError < ProcessingError; end - class AutoGeneratedEmailError < ProcessingError; end + class ProcessingError < StandardError; end + class EmptyEmailError < ProcessingError; end + class ScreenedEmailError < ProcessingError; end + class UserNotFoundError < ProcessingError; end + class AutoGeneratedEmailError < ProcessingError; end class AutoGeneratedEmailReplyError < ProcessingError; end - class BouncedEmailError < ProcessingError; end - class NoBodyDetectedError < ProcessingError; end - class InactiveUserError < ProcessingError; end - class BlockedUserError < ProcessingError; end - class BadDestinationAddress < ProcessingError; end - class StrangersNotAllowedError < ProcessingError; end - class InsufficientTrustLevelError < ProcessingError; end - class ReplyUserNotMatchingError < ProcessingError; end - class TopicNotFoundError < ProcessingError; end - class TopicClosedError < ProcessingError; end - class InvalidPost < ProcessingError; end - class InvalidPostAction < ProcessingError; end + class BouncedEmailError < ProcessingError; end + class NoBodyDetectedError < ProcessingError; end + class InactiveUserError < ProcessingError; end + class BlockedUserError < ProcessingError; end + class BadDestinationAddress < ProcessingError; end + class StrangersNotAllowedError < ProcessingError; end + class InsufficientTrustLevelError < ProcessingError; end + class ReplyUserNotMatchingError < ProcessingError; end + class TopicNotFoundError < ProcessingError; end + class TopicClosedError < ProcessingError; end + class InvalidPost < ProcessingError; end + class InvalidPostAction < ProcessingError; end attr_reader :incoming_email @@ -55,6 +56,8 @@ module Email end def process_internal + raise ScreenedEmailError if ScreenedEmail.should_block?(@from_email) + user = find_or_create_user(@from_email, @from_display_name) raise UserNotFoundError if user.nil? @@ -209,16 +212,20 @@ module Email user = nil User.transaction do - user = User.find_by_email(email) + begin + user = User.find_by_email(email) - if user.nil? && SiteSetting.enable_staged_users - username = UserNameSuggester.sanitize_username(display_name) if display_name.present? - user = User.create( - email: email, - username: UserNameSuggester.suggest(username.presence || email), - name: display_name.presence || User.suggest_name(email), - staged: true - ) + if user.nil? && SiteSetting.enable_staged_users + username = UserNameSuggester.sanitize_username(display_name) if display_name.present? + user = User.create!( + email: email, + username: UserNameSuggester.suggest(username.presence || email), + name: display_name.presence || User.suggest_name(email), + staged: true + ) + end + rescue + user = nil end end diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 1dfb26f33..1903255f5 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -17,7 +17,12 @@ describe Email::Receiver do expect { Email::Receiver.new("") }.to raise_error(Email::Receiver::EmptyEmailError) end - it "raises and UserNotFoundError when staged users are disabled" do + it "raises a ScreenedEmailError when email address is screened" do + ScreenedEmail.expects(:should_block?).with("screened@mail.com").returns(true) + expect { process(:screened_email) }.to raise_error(Email::Receiver::ScreenedEmailError) + end + + it "raises an UserNotFoundError when staged users are disabled" do SiteSetting.enable_staged_users = false expect { process(:user_not_found) }.to raise_error(Email::Receiver::UserNotFoundError) end diff --git a/spec/fixtures/emails/screened_email.eml b/spec/fixtures/emails/screened_email.eml new file mode 100644 index 000000000..b8423cad5 --- /dev/null +++ b/spec/fixtures/emails/screened_email.eml @@ -0,0 +1,9 @@ +Return-Path: +From: Not Found +Date: Fri, 15 Jan 2016 00:12:43 +0100 +Message-ID: <51@foo.bar.mail> +Mime-Version: 1.0 +Content-Type: text/plain +Content-Transfer-Encoding: 7bit + +Email from a screened email address.