FIX: Use reply-to address for incoming emails if present (#13896)

When the Reply-To header is present for incoming emails we
want to use it instead of the from address. This is usually the
case when forwarding an email via a mailing list into Discourse.

For now we are only using the Reply-To header if the email has
been forwarded via Google Groups, which is why we are checking the
X-Original-From header too. In future we may want to use the Reply-To
header in more cases.

lib/email/receiver.rb

@@ -570,11 +570,28 @@ module Email
 
       return unless mail[:from]
 
+      # For now we are only using the Reply-To header if the email has
+      # been forwarded via Google Groups, which is why we are checking the
+      # X-Original-From header too. In future we may want to use the Reply-To
+      # header in more cases.
+      if mail['X-Original-From'].present?
+        if mail[:reply_to] && mail[:reply_to].errors.blank?
+          mail[:reply_to].each do |address_field|
+            from_address = address_field.address
+            from_display_name = address_field.display_name&.to_s
+            next if address_field.to_s != mail['X-Original-From'].to_s
+            next if !from_address&.include?("@")
+            return [from_address&.downcase, from_display_name&.strip]
+          end
+        end
+      end
+
       if mail[:from].errors.blank?
         mail[:from].each do |address_field|
           from_address = address_field.address
-          from_display_name = address_field.display_name.try(:to_s)
+          from_display_name = address_field.display_name&.to_s
-          return [from_address&.downcase, from_display_name&.strip] if from_address["@"]
+          next if !from_address&.include?("@")
+          return [from_address&.downcase, from_display_name&.strip]
         end
       end
 

spec/components/email/receiver_spec.rb

@@ -854,6 +854,38 @@ describe Email::Receiver do
       expect(Topic.last.ordered_posts[-1].post_type).to eq(Post.types[:moderator_action])
     end
 
+    describe "reply-to header" do
+      it "handles emails where there is a reply-to address, using that instead of the from address" do
+        SiteSetting.block_auto_generated_emails = false
+        expect { process(:reply_to_different_to_from) }.to change(Topic, :count)
+        user = User.last
+        incoming = IncomingEmail.find_by(message_id: "3848c3m98r439c348mc349@test.mailinglist.com")
+        topic = incoming.topic
+        expect(incoming.from_address).to eq("arthurmorgan@reddeadtest.com")
+        expect(user.email).to eq("arthurmorgan@reddeadtest.com")
+      end
+
+      it "does not use the reply-to address if an X-Original-From header is not present" do
+        SiteSetting.block_auto_generated_emails = false
+        expect { process(:reply_to_different_to_from_no_x_original) }.to change(Topic, :count)
+        user = User.last
+        incoming = IncomingEmail.find_by(message_id: "3848c3m98r439c348mc349@test.mailinglist.com")
+        topic = incoming.topic
+        expect(incoming.from_address).to eq("westernsupport@test.mailinglist.com")
+        expect(user.email).to eq("westernsupport@test.mailinglist.com")
+      end
+
+      it "does not use the reply-to address if the X-Original-From header is different from the reply-to address" do
+        SiteSetting.block_auto_generated_emails = false
+        expect { process(:reply_to_different_to_from_x_original_different) }.to change(Topic, :count)
+        user = User.last
+        incoming = IncomingEmail.find_by(message_id: "3848c3m98r439c348mc349@test.mailinglist.com")
+        topic = incoming.topic
+        expect(incoming.from_address).to eq("westernsupport@test.mailinglist.com")
+        expect(user.email).to eq("westernsupport@test.mailinglist.com")
+      end
+    end
+
     describe "when 'find_related_post_with_key' is disabled" do
       before do
         SiteSetting.find_related_post_with_key = false

spec/fixtures/emails/reply_to_different_to_from.eml

@@ -0,0 +1,16 @@
+From: 'Arthur Morgan' via Western Support <westernsupport@test.mailinglist.com>
+Reply-To: Arthur Morgan <arthurmorgan@reddeadtest.com>
+To: team@bar.com
+Subject: I need some support
+Date: Fri, 15 Jan 2016 00:12:43 +0100
+Message-ID: <3848c3m98r439c348mc349@test.mailinglist.com>
+Mime-Version: 1.0
+Content-Type: text/plain
+Content-Transfer-Encoding: 7bit
+X-Original-From: Arthur Morgan <arthurmorgan@reddeadtest.com>
+Precedence: list
+Mailing-list: list westernsupport@test.mailinglist.com; contact
+ westernsupport+owners@test.mailinglist.com
+List-ID: <westernsupport@test.mailinglist.com>
+
+You sir, are a fish.

spec/fixtures/emails/reply_to_different_to_from_no_x_original.eml

@@ -0,0 +1,11 @@
+From: 'Arthur Morgan' via Western Support <westernsupport@test.mailinglist.com>
+Reply-To: Arthur Morgan <arthurmorgan@reddeadtest.com>
+To: team@bar.com
+Subject: I need some support
+Date: Fri, 15 Jan 2016 00:12:43 +0100
+Message-ID: <3848c3m98r439c348mc349@test.mailinglist.com>
+Mime-Version: 1.0
+Content-Type: text/plain
+Content-Transfer-Encoding: 7bit
+
+You sir, are a fish.

spec/fixtures/emails/reply_to_different_to_from_x_original_different.eml

@@ -0,0 +1,16 @@
+From: 'Arthur Morgan' via Western Support <westernsupport@test.mailinglist.com>
+Reply-To: Arthur Morgan <arthurmorgan@reddeadtest.com>
+To: team@bar.com
+Subject: I need some support
+Date: Fri, 15 Jan 2016 00:12:43 +0100
+Message-ID: <3848c3m98r439c348mc349@test.mailinglist.com>
+Mime-Version: 1.0
+Content-Type: text/plain
+Content-Transfer-Encoding: 7bit
+X-Original-From: Dutch Van der Linde <dutch@reddeadtest.com>
+Precedence: list
+Mailing-list: list westernsupport@test.mailinglist.com; contact
+ westernsupport+owners@test.mailinglist.com
+List-ID: <westernsupport@test.mailinglist.com>
+
+We need more money. I have a plan.