From e15d11df1856b0bf64ff66b1280c034ca47903a2 Mon Sep 17 00:00:00 2001 From: "Ryan C. Gordon" Date: Wed, 5 Apr 2017 02:32:50 -0400 Subject: [PATCH 1/4] Added an API to ask if an incoming email should be dropped at the SMTP level. This lets an SMTP server optionally decide if it should reject a mail without passing it on to Discourse at all, possibly before even reading the email's payload, to prevent spam-induced backscatter and save resources. This just does the bare minimum sanity checking that could prevent obvious backscatter. For legit errors from legit users, Discourse will still send a much more pleasant reply email. --- app/controllers/admin/email_controller.rb | 13 +++++++++++++ config/routes.rb | 1 + 2 files changed, 14 insertions(+) diff --git a/app/controllers/admin/email_controller.rb b/app/controllers/admin/email_controller.rb index df400de93e5..ca7dd112c00 100644 --- a/app/controllers/admin/email_controller.rb +++ b/app/controllers/admin/email_controller.rb @@ -69,6 +69,19 @@ class Admin::EmailController < Admin::AdminController end end + def smtp_should_reject + params.require(:from) + params.require(:to) + # These strings aren't localized; they are sent to an anonymous SMTP user. + if User.find_by_email(params[:from]).nil? && !SiteSetting.enable_staged_users + render json: { reject: true, reason: "Mail from your address is not accepted. Do you have an account here?" } + elsif Email::Receiver.new(params[:from]).check_address(params[:to]).nil? + render json: { reject: true, reason: "Mail to this address is not accepted. Check the address and try to send again?" } + else + render json: { reject: false } + end + end + def handle_mail params.require(:email) Email::Processor.process!(params[:email]) diff --git a/config/routes.rb b/config/routes.rb index b8091c79397..e6c3d9ed719 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -155,6 +155,7 @@ Discourse::Application.routes.draw do get "/incoming_from_bounced/:id" => "email#incoming_from_bounced" get "preview-digest" => "email#preview_digest" get "send-digest" => "email#send_digest" + get "smtp_should_reject" post "handle_mail" end end From a51c191a665c592711232c9f2010522f0af2f55d Mon Sep 17 00:00:00 2001 From: "Ryan C. Gordon" Date: Wed, 5 Apr 2017 12:45:58 -0400 Subject: [PATCH 2/4] Make Email::Receiver.check_address() into a class method. --- app/controllers/admin/email_controller.rb | 2 +- lib/email/receiver.rb | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/email_controller.rb b/app/controllers/admin/email_controller.rb index ca7dd112c00..0d9d0aca269 100644 --- a/app/controllers/admin/email_controller.rb +++ b/app/controllers/admin/email_controller.rb @@ -75,7 +75,7 @@ class Admin::EmailController < Admin::AdminController # These strings aren't localized; they are sent to an anonymous SMTP user. if User.find_by_email(params[:from]).nil? && !SiteSetting.enable_staged_users render json: { reject: true, reason: "Mail from your address is not accepted. Do you have an account here?" } - elsif Email::Receiver.new(params[:from]).check_address(params[:to]).nil? + elsif Email::Receiver.check_address(Email.downcase(params[:to])).nil? render json: { reject: true, reason: "Mail to this address is not accepted. Check the address and try to send again?" } else render json: { reject: false } diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index ec113b504ac..2322970bae1 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -302,11 +302,11 @@ module Email def destinations all_destinations - .map { |d| check_address(d) } + .map { |d| Email::Receiver.check_address(d) } .drop_while(&:blank?) end - def check_address(address) + def self.check_address(address) # only check for a group/category when 'email_in' is enabled if SiteSetting.email_in group = Group.find_by_email(address) @@ -317,7 +317,7 @@ module Email end # reply - match = reply_by_email_address_regex.match(address) + match = Email::Receiver.reply_by_email_address_regex.match(address) if match && match.captures match.captures.each do |c| next if c.blank? @@ -443,7 +443,7 @@ module Email true end - def reply_by_email_address_regex + def self.reply_by_email_address_regex @reply_by_email_address_regex ||= begin reply_addresses = [ SiteSetting.reply_by_email_address, @@ -652,7 +652,7 @@ module Email end def should_invite?(email) - email !~ reply_by_email_address_regex && + email !~ Email::Receiver.reply_by_email_address_regex && email !~ group_incoming_emails_regex && email !~ category_email_in_regex end From c51af1333806b9d7e2de165b00b0dca5d8cd5dce Mon Sep 17 00:00:00 2001 From: "Ryan C. Gordon" Date: Wed, 5 Apr 2017 12:46:56 -0400 Subject: [PATCH 3/4] smtp_should_reject API: use better approach to find user email. --- app/controllers/admin/email_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/admin/email_controller.rb b/app/controllers/admin/email_controller.rb index 0d9d0aca269..f77c64d7784 100644 --- a/app/controllers/admin/email_controller.rb +++ b/app/controllers/admin/email_controller.rb @@ -73,7 +73,7 @@ class Admin::EmailController < Admin::AdminController params.require(:from) params.require(:to) # These strings aren't localized; they are sent to an anonymous SMTP user. - if User.find_by_email(params[:from]).nil? && !SiteSetting.enable_staged_users + if !User.exists?(email: Email.downcase(params[:from])) && !SiteSetting.enable_staged_users render json: { reject: true, reason: "Mail from your address is not accepted. Do you have an account here?" } elsif Email::Receiver.check_address(Email.downcase(params[:to])).nil? render json: { reject: true, reason: "Mail to this address is not accepted. Check the address and try to send again?" } From 888d1512ecfde37e62df008e7bb592e6c9ba8580 Mon Sep 17 00:00:00 2001 From: "Ryan C. Gordon" Date: Thu, 6 Apr 2017 01:49:34 -0400 Subject: [PATCH 4/4] Corrected indentation. --- app/controllers/admin/email_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/admin/email_controller.rb b/app/controllers/admin/email_controller.rb index f77c64d7784..416e0c1b940 100644 --- a/app/controllers/admin/email_controller.rb +++ b/app/controllers/admin/email_controller.rb @@ -74,11 +74,11 @@ class Admin::EmailController < Admin::AdminController params.require(:to) # These strings aren't localized; they are sent to an anonymous SMTP user. if !User.exists?(email: Email.downcase(params[:from])) && !SiteSetting.enable_staged_users - render json: { reject: true, reason: "Mail from your address is not accepted. Do you have an account here?" } + render json: { reject: true, reason: "Mail from your address is not accepted. Do you have an account here?" } elsif Email::Receiver.check_address(Email.downcase(params[:to])).nil? - render json: { reject: true, reason: "Mail to this address is not accepted. Check the address and try to send again?" } + render json: { reject: true, reason: "Mail to this address is not accepted. Check the address and try to send again?" } else - render json: { reject: false } + render json: { reject: false } end end