diff --git a/app/controllers/admin/email_controller.rb b/app/controllers/admin/email_controller.rb index bfb4e75e188..4a09d01ccc7 100644 --- a/app/controllers/admin/email_controller.rb +++ b/app/controllers/admin/email_controller.rb @@ -175,13 +175,20 @@ class Admin::EmailController < Admin::AdminController params.require(:id) begin - bounced = EmailLog.find_by(id: params[:id].to_i) - raise Discourse::InvalidParameters if bounced.nil? + email_log = EmailLog.find_by(id: params[:id].to_i, bounced: true) + raise Discourse::InvalidParameters if email_log&.bounce_key.blank? - email_local_part, email_domain = SiteSetting.notification_email.split('@') - bounced_to_address = "#{email_local_part}+verp-#{bounced.bounce_key}@#{email_domain}" + if Email::Sender.bounceable_reply_address? + bounced_to_address = Email::Sender.bounce_address(email_log.bounce_key) + incoming_email = IncomingEmail.find_by(to_addresses: bounced_to_address) + end + + if incoming_email.nil? + email_local_part, email_domain = SiteSetting.notification_email.split('@') + bounced_to_address = "#{email_local_part}+verp-#{email_log.bounce_key}@#{email_domain}" + incoming_email = IncomingEmail.find_by(to_addresses: bounced_to_address) + end - incoming_email = IncomingEmail.find_by(to_addresses: bounced_to_address) raise Discourse::NotFound if incoming_email.nil? serializer = IncomingEmailDetailsSerializer.new(incoming_email, root: false) diff --git a/app/serializers/incoming_email_details_serializer.rb b/app/serializers/incoming_email_details_serializer.rb index f67f9fcb1e8..68959ebb1fa 100644 --- a/app/serializers/incoming_email_details_serializer.rb +++ b/app/serializers/incoming_email_details_serializer.rb @@ -25,7 +25,7 @@ class IncomingEmailDetailsSerializer < ApplicationSerializer end def include_error_description? - @error_string[EMAIL_RECEIVER_ERROR_PREFIX] + @error_string && @error_string[EMAIL_RECEIVER_ERROR_PREFIX] end def headers diff --git a/lib/email/sender.rb b/lib/email/sender.rb index 7f23bddeeaf..0de51e5bf3e 100644 --- a/lib/email/sender.rb +++ b/lib/email/sender.rb @@ -165,13 +165,13 @@ module Email @message.header['List-Post'] = "" end - if SiteSetting.reply_by_email_address.present? && SiteSetting.reply_by_email_address["+"] + if Email::Sender.bounceable_reply_address? email_log.bounce_key = SecureRandom.hex # WARNING: RFC claims you can not set the Return Path header, this is 100% correct # however Rails has special handling for this header and ends up using this value # as the Envelope From address so stuff works as expected - @message.header[:return_path] = SiteSetting.reply_by_email_address.sub("%{reply_key}", "verp-#{email_log.bounce_key}") + @message.header[:return_path] = Email::Sender.bounce_address(email_log.bounce_key) end email_log.post_id = post_id if post_id.present? @@ -282,5 +282,12 @@ module Email header_value('Reply-To').gsub!("%{reply_key}", reply_key) end + def self.bounceable_reply_address? + SiteSetting.reply_by_email_address.present? && SiteSetting.reply_by_email_address["+"] + end + + def self.bounce_address(bounce_key) + SiteSetting.reply_by_email_address.sub("%{reply_key}", "verp-#{bounce_key}") + end end end diff --git a/spec/requests/admin/email_controller_spec.rb b/spec/requests/admin/email_controller_spec.rb index 99c7fd8a1b1..8f41da60433 100644 --- a/spec/requests/admin/email_controller_spec.rb +++ b/spec/requests/admin/email_controller_spec.rb @@ -227,6 +227,77 @@ describe Admin::EmailController do end end + describe '#incoming_from_bounced' do + it 'raises an error when the email log entry does not exist' do + get "/admin/email/incoming_from_bounced/12345.json" + expect(response.status).to eq(404) + + json = JSON.parse(response.body) + expect(json["errors"]).to include("Discourse::InvalidParameters") + end + + it 'raises an error when the email log entry is not marked as bounced' do + get "/admin/email/incoming_from_bounced/#{email_log.id}.json" + expect(response.status).to eq(404) + + json = JSON.parse(response.body) + expect(json["errors"]).to include("Discourse::InvalidParameters") + end + + context 'bounced email log entry exists' do + let(:email_log) { Fabricate(:email_log, bounced: true, bounce_key: SecureRandom.hex) } + let(:error_message) { "Email::Receiver::BouncedEmailError" } + + it 'returns an incoming email sent to the reply_by_email_address' do + SiteSetting.reply_by_email_address = "replies+%{reply_key}@example.com" + + Fabricate(:incoming_email, + is_bounce: true, + error: error_message, + to_addresses: Email::Sender.bounce_address(email_log.bounce_key) + ) + + get "/admin/email/incoming_from_bounced/#{email_log.id}.json" + expect(response.status).to eq(200) + + json = JSON.parse(response.body) + expect(json["error"]).to eq(error_message) + end + + it 'returns an incoming email sent to the notification_email address' do + Fabricate(:incoming_email, + is_bounce: true, + error: error_message, + to_addresses: SiteSetting.notification_email.sub("@", "+verp-#{email_log.bounce_key}@") + ) + + get "/admin/email/incoming_from_bounced/#{email_log.id}.json" + expect(response.status).to eq(200) + + json = JSON.parse(response.body) + expect(json["error"]).to eq(error_message) + end + + it 'raises an error if the bounce_key is blank' do + email_log.update(bounce_key: nil) + + get "/admin/email/incoming_from_bounced/#{email_log.id}.json" + expect(response.status).to eq(404) + + json = JSON.parse(response.body) + expect(json["errors"]).to include("Discourse::InvalidParameters") + end + + it 'raises an error if there is no incoming email' do + get "/admin/email/incoming_from_bounced/#{email_log.id}.json" + expect(response.status).to eq(404) + + json = JSON.parse(response.body) + expect(json["errors"]).to include("Discourse::NotFound") + end + end + end + describe '#advanced_test' do it 'should ...' do email = <<~EMAIL