FEATURE: add setting to show content of forwarded emails in topics (#7935)

config/locales/server.en.yml

@@ -1811,7 +1811,7 @@ en:
     attachment_content_type_blacklist: "List of keywords used to blacklist attachments based on the content type."
     attachment_filename_blacklist: "List of keywords used to blacklist attachments based on the filename."
 
-    enable_forwarded_emails: "[BETA] Allow users to create a topic by forwarding an email in."
+    forwarded_emails_behaviour: "How to treat a forwarded email to Discourse"
     always_show_trimmed_content: "Always show trimmed part of incoming emails. WARNING: might reveal email addresses."
     private_email: "Don't include content from posts or topics in email title or email body. NOTE: also disables digest emails."
     email_total_attachment_size_limit_kb: "Max total size of files attached to outgoing emails. Set to 0 to disable sending of attachments."

config/site_settings.yml

@@ -1022,7 +1022,13 @@ email:
     type: list
     default: "smime.p7s|signature.asc|winmail.dat"
     list_type: compact
-  enable_forwarded_emails: false
+  forwarded_emails_behaviour:
+    type: enum
+    default: hide
+    choices:
+      - hide
+      - quote
+      - create_replies
   always_show_trimmed_content: false
   private_email: false
   email_custom_template:

db/migrate/20190724162522_migrate_forwarded_emails_behaviour_site_settings.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+class MigrateForwardedEmailsBehaviourSiteSettings < ActiveRecord::Migration[5.2]
+  def up
+    execute <<~SQL
+      UPDATE site_settings
+      SET name      = 'forwarded_emails_behaviour',
+          data_type = 7,
+          value     = 'create_replies'
+      WHERE name = 'enable_forwarded_emails' AND value = 't';
+    SQL
+
+    execute <<~SQL
+      DELETE
+      FROM site_settings
+      WHERE name = 'enable_forwarded_emails';
+    SQL
+  end
+
+  def down
+    execute <<~SQL
+      UPDATE site_settings
+      SET name      = 'enable_forwarded_emails',
+          data_type = 5,
+          value     = 't'
+      WHERE name = 'forwarded_emails_behaviour' AND value = 'create_replies';
+    SQL
+
+    execute <<~SQL
+      DELETE
+      FROM site_settings
+      WHERE name = 'forwarded_emails_behaviour';
+    SQL
+  end
+end

lib/email/receiver.rb

@@ -660,7 +660,7 @@ module Email
     end
 
     def process_destination(destination, user, body, elided, hidden_reason_id)
-      return if SiteSetting.enable_forwarded_emails &&
+      return if SiteSetting.forwarded_emails_behaviour != "hide" &&
                 has_been_forwarded? &&
                 process_forwarded_email(destination, user)
 
@@ -800,27 +800,30 @@ module Email
 
     def process_forwarded_email(destination, user)
       user ||= stage_from_user
-      embedded = Mail.new(embedded_email_raw)
+      case SiteSetting.forwarded_emails_behaviour
-      email, display_name = parse_from_field(embedded)
+      when "create_replies"
-
+        forwarded_email_create_replies(destination, user)
-      return false if email.blank? || !email["@"]
+      when "quote"
-
+        forwarded_email_quote_forwarded(destination, user)
-      raw = try_to_encode(embedded.decoded, "UTF-8").presence || embedded.to_s
+      else
-      title = embedded.subject.presence || subject
+        false
+      end
+    end
 
+    def forwarded_email_create_topic(destination: , user: , raw: , title: , date: nil, embedded_user: nil)
       case destination[:type]
       when :group
         group = destination[:obj]
-        embedded_user = find_or_create_user(email, display_name)
+        topic_user = embedded_user&.call || user
-        post = create_topic(user: embedded_user,
+        create_topic(user: topic_user,
-                            raw: raw,
+                     raw: raw,
-                            title: title,
+                     title: title,
-                            archetype: Archetype.private_message,
+                     archetype: Archetype.private_message,
-                            target_usernames: [user.username],
+                     target_usernames: [user.username],
-                            target_group_names: [group.name],
+                     target_group_names: [group.name],
-                            is_group_message: true,
+                     is_group_message: true,
-                            skip_validations: true,
+                     skip_validations: true,
-                            created_at: embedded.date)
+                     created_at: date)
 
       when :category
         category = destination[:obj]
@@ -828,16 +831,31 @@ module Email
         return false if user.staged? && !category.email_in_allow_strangers
         return false if !user.has_trust_level?(SiteSetting.email_in_min_trust)
 
-        embedded_user = find_or_create_user(email, display_name)
+        topic_user = embedded_user&.call || user
-        post = create_topic(user: embedded_user,
+        create_topic(user: topic_user,
-                            raw: raw,
+                     raw: raw,
-                            title: title,
+                     title: title,
-                            category: category.id,
+                     category: category.id,
-                            skip_validations: embedded_user.staged?,
+                     skip_validations: topic_user.staged?,
-                            created_at: embedded.date)
+                     created_at: date)
       else
-        return false
+        false
       end
+    end
+
+    def forwarded_email_create_replies(destination, user)
+      embedded = Mail.new(embedded_email_raw)
+      email, display_name = parse_from_field(embedded)
+
+      return false if email.blank? || !email["@"]
+
+      post = forwarded_email_create_topic(destination: destination,
+                                          user: user,
+                                          raw: try_to_encode(embedded.decoded, "UTF-8").presence || embedded.to_s,
+                                          title: embedded.subject.presence || subject,
+                                          date: embedded.date,
+                                          embedded_user: lambda { find_or_create_user(email, display_name) })
+      return false unless post
 
       if post&.topic
         # mark post as seen for the forwarder
@@ -846,7 +864,7 @@ module Email
         # create reply when available
         if @before_embedded.present?
           post_type = Post.types[:regular]
-          post_type = Post.types[:whisper] if post.topic.private_message? && group.usernames[user.username]
+          post_type = Post.types[:whisper] if post.topic.private_message? && destination[:obj].usernames[user.username]
 
           create_reply(user: user,
                        raw: @before_embedded,
@@ -860,6 +878,19 @@ module Email
       true
     end
 
+    def forwarded_email_quote_forwarded(destination, user)
+      embedded = embedded_email_raw
+      raw = <<~EOF
+        #{@before_embedded}
+
+        [quote]
+        #{PlainTextToMarkdown.new(embedded).to_markdown}
+        [/quote]
+      EOF
+
+      return true if forwarded_email_create_topic(destination: destination, user: user, raw: raw, title: subject)
+    end
+
     def self.reply_by_email_address_regex(extract_reply_key = true, include_verp = false)
       reply_addresses = [SiteSetting.reply_by_email_address]
       reply_addresses << (SiteSetting.alternative_reply_by_email_addresses.presence || "").split("|")

spec/components/email/receiver_spec.rb

@@ -708,6 +708,41 @@ describe Email::Receiver do
 
   end
 
+  shared_examples "creates topic with forwarded message as quote" do |destination, address|
+    it "creates topic with forwarded message as quote" do
+      expect { process(:forwarded_email_1) }.to change(Topic, :count)
+
+      topic = Topic.last
+      if destination == :category
+        expect(topic.category).to eq(Category.where(email_in: address).first)
+      else
+        expect(topic.archetype).to eq(Archetype.private_message)
+        expect(topic.allowed_groups).to eq(Group.where(incoming_email: address))
+      end
+
+      post = Post.last
+
+      expect(post.user.email).to eq("ba@bar.com")
+      expect(post.raw).to eq(<<~EOF.chomp
+        @team, can you have a look at this email below?
+
+        [quote]
+        From: Some One &lt;some@one\\.com&gt;
+        To: Ba Bar &lt;ba@bar\\.com&gt;
+        Date: Mon, 1 Dec 2016 00:13:37 \\+0100
+        Subject: Discoursing much?
+
+        Hello Ba Bar,
+
+        Discoursing much today?
+
+        XoXo
+        [/quote]
+      EOF
+      )
+    end
+  end
+
   context "new message to a group" do
 
     fab!(:group) { Fabricate(:group, incoming_email: "team@bar.com|meat@bar.com") }
@@ -818,10 +853,10 @@ describe Email::Receiver do
       expect(user.user_option.email_messages_level).to eq(UserOption.email_level_types[:always])
     end
 
-    context "with forwarded emails enabled" do
+    context "with forwarded emails behaviour set to create replies" do
       before do
         Fabricate(:group, incoming_email: "some_group@bar.com")
-        SiteSetting.enable_forwarded_emails = true
+        SiteSetting.forwarded_emails_behaviour = "create_replies"
       end
 
       it "handles forwarded emails" do
@@ -842,7 +877,7 @@ describe Email::Receiver do
         group.add(Fabricate(:user, email: "ba@bar.com"))
         group.save
 
-        SiteSetting.enable_forwarded_emails = true
+        SiteSetting.forwarded_emails_behaviour = "create_replies"
         expect { process(:forwarded_email_2) }.to change(Topic, :count)
 
         forwarded_post, last_post = *Post.last(2)
@@ -863,6 +898,14 @@ describe Email::Receiver do
 
     end
 
+    context "with forwarded emails behaviour set to quote" do
+      before do
+        SiteSetting.forwarded_emails_behaviour = "quote"
+      end
+
+      include_examples "creates topic with forwarded message as quote", :group, "team@bar.com|meat@bar.com"
+    end
+
     context "when message sent to a group has no key and find_related_post_with_key is enabled" do
       let!(:topic) do
         process(:email_reply_1)
@@ -1202,6 +1245,22 @@ describe Email::Receiver do
 
         include_examples "does not create staged users", :no_date, Email::Receiver::InvalidPost
       end
+
+      context "with forwarded emails behaviour set to quote" do
+        before do
+          SiteSetting.forwarded_emails_behaviour = "quote"
+        end
+
+        context "with a category which allows strangers" do
+          fab!(:category) { Fabricate(:category, email_in: "team@bar.com", email_in_allow_strangers: true) }
+          include_examples "creates topic with forwarded message as quote", :category, "team@bar.com"
+        end
+
+        context "with a category which doesn't allow strangers" do
+          fab!(:category) { Fabricate(:category, email_in: "team@bar.com", email_in_allow_strangers: false) }
+          include_examples "cleans up staged users", :forwarded_email_1, Email::Receiver::StrangersNotAllowedError
+        end
+      end
     end
 
     context "email is a reply" do
@@ -1220,9 +1279,9 @@ describe Email::Receiver do
         include_examples "does not create staged users", :reply_user_not_matching, Email::Receiver::ReplyUserNotMatchingError
       end
 
-      context "when forwarded emails are enabled" do
+      context "with forwarded emails behaviour set to create replies" do
         before do
-          SiteSetting.enable_forwarded_emails = true
+          SiteSetting.forwarded_emails_behaviour = "create_replies"
         end
 
         context "when a reply contains a forwareded email" do