FIX: Make sure CC addresses for forwarded emails are used

When forwarding emails into the group inbox we are currently
ignoring the original CC addresses on the forwarded email. This
commit makes sure we record those CC addresses on the incoming
email record for the topic, as well as inviting those CC addresses
as staged users on the topic when it is created.

lib/email/receiver.rb

@@ -143,13 +143,17 @@ module Email
     end
 
     def create_incoming_email
+      cc_addresses = Array.wrap(@mail.cc)
+      if embedded_email&.cc
+        cc_addresses.concat(embedded_email.cc)
+      end
       IncomingEmail.create(
         message_id: @message_id,
         raw: Email::Cleaner.new(@raw_email).execute,
         subject: subject,
         from_address: @from_email,
         to_addresses: @mail.to,
-        cc_addresses: @mail.cc,
+        cc_addresses: cc_addresses,
         imap_uid_validity: @opts[:imap_uid_validity],
         imap_uid: @opts[:imap_uid],
         imap_group_id: @opts[:imap_group_id],
@@ -589,11 +593,10 @@ module Email
       # which we can extract from embedded_email_raw.
       if has_been_forwarded?
         if mail[:from].to_s =~ group_incoming_emails_regex && embedded_email[:from].errors.blank?
-          embedded_email[:from].each do |address_field|
-            from_address = address_field.address
-            from_display_name = address_field.display_name&.to_s
-            next if !from_address&.include?("@")
-            return [from_address&.downcase, from_display_name&.strip]
+          from_address, from_display_name = extract_from_fields_from_header(embedded_email, :from)
+          if from_address
+            @forwarded_by_address, @forwarded_by_name = extract_from_fields_from_header(mail, :from)
+            return [from_address, from_display_name]
           end
         end
       end
@@ -604,23 +607,16 @@ module Email
       # 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
+          from_address, from_display_name = extract_from_fields_from_header(
+            mail, :reply_to, comparison_headers: ['X-Original-From']
+          )
+          return [from_address, from_display_name] if from_address
         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&.to_s
-          next if !from_address&.include?("@")
-          return [from_address&.downcase, from_display_name&.strip]
-        end
+        from_address, from_display_name = extract_from_fields_from_header(mail, :from)
+        return [from_address, from_display_name] if from_address
       end
 
       return extract_from_address_and_name(mail.from) if mail.from.is_a? String
@@ -637,6 +633,24 @@ module Email
       nil
     end
 
+    def extract_from_fields_from_header(mail_object, header, comparison_headers: [])
+      mail_object[header].each do |address_field|
+        from_address = address_field.address
+        from_display_name = address_field.display_name&.to_s
+
+        comparison_failed = false
+        comparison_headers.each do |comparison_header|
+          comparison_failed = true if address_field.to_s != mail_object[comparison_header].to_s
+        end
+
+        next if comparison_failed
+        next if !from_address&.include?("@")
+        return [from_address&.downcase, from_display_name&.strip]
+      end
+
+      [nil, nil]
+    end
+
     def extract_from_address_and_name(value)
       if value[";"]
         from_display_name, from_address = value.split(";")
@@ -948,6 +962,10 @@ module Email
       embedded = Mail.new(embedded_email_raw)
       email, display_name = parse_from_field(embedded)
 
+      if @forwarded_by_address && @forwarded_by_name
+        @forwarded_by_user = stage_sender_user(@forwarded_by_address, @forwarded_by_name)
+      end
+
       return false if email.blank? || !email["@"]
 
       post = forwarded_email_create_topic(destination: destination,
@@ -974,7 +992,10 @@ module Email
                        post_type: post_type,
                        skip_validations: user.staged?)
         else
-          post.topic.add_small_action(user, "forwarded")
+          if @forwarded_by_user
+            post.topic.topic_allowed_users.find_or_create_by!(user_id: @forwarded_by_user.id)
+          end
+          post.topic.add_small_action(@forwarded_by_user || user, "forwarded")
         end
       end
 
@@ -1298,7 +1319,11 @@ module Email
       if result.post
         @incoming_email.update_columns(topic_id: result.post.topic_id, post_id: result.post.id)
         if result.post.topic&.private_message? && !is_bounce?
-          add_other_addresses(result.post, user)
+          add_other_addresses(result.post, user, @mail)
+
+          if has_been_forwarded?
+            add_other_addresses(result.post, @forwarded_by_user || user, embedded_email)
+          end
         end
 
         # Alert the people involved in the topic now that the incoming email
@@ -1320,11 +1345,11 @@ module Email
       html
     end
 
-    def add_other_addresses(post, sender)
+    def add_other_addresses(post, sender, mail_object)
       %i(to cc bcc).each do |d|
-        next if @mail[d].blank?
+        next if mail_object[d].blank?
 
-        @mail[d].each do |address_field|
+        mail_object[d].each do |address_field|
           begin
             address_field.decoded
             email = address_field.address.downcase
@@ -1367,7 +1392,11 @@ module Email
     end
 
     def stage_from_user
-      @from_user ||= find_or_create_user!(@from_email, @from_display_name).tap do |u|
+      @from_user ||= stage_sender_user(@from_email, @from_display_name)
+    end
+
+    def stage_sender_user(email, display_name)
+      find_or_create_user!(email, display_name).tap do |u|
         log_and_validate_user(u)
       end
     end

spec/components/email/receiver_spec.rb

@@ -1033,7 +1033,7 @@ describe Email::Receiver do
     end
 
     context "when a group forwards an email to its inbox" do
-      let!(:topic) do
+      before do
         group.update!(
           email_username: "team@somesmtpaddress.com",
           incoming_email: "team@somesmtpaddress.com|support+team@bar.com",
@@ -1042,14 +1042,65 @@ describe Email::Receiver do
           smtp_ssl: true,
           smtp_enabled: true
         )
-        process(:forwarded_by_group_to_group)
-        Topic.last
       end
 
       it "does not use the team's address as the from_address; it uses the original sender address" do
+        process(:forwarded_by_group_to_group)
+        topic = Topic.last
         expect(topic.incoming_email.first.to_addresses).to include("support+team@bar.com")
         expect(topic.incoming_email.first.from_address).to eq("fred@bedrock.com")
       end
+
+      context "with forwarded emails behaviour set to create replies" do
+        before do
+          SiteSetting.forwarded_emails_behaviour = "create_replies"
+        end
+
+        it "does not use the team's address as the from_address; it uses the original sender address" do
+          process(:forwarded_by_group_to_group)
+          topic = Topic.last
+          expect(topic.incoming_email.first.to_addresses).to include("support+team@bar.com")
+          expect(topic.incoming_email.first.from_address).to eq("fred@bedrock.com")
+        end
+
+        it "does not say the email was forwarded by the original sender, it says the email is forwarded by the group" do
+          expect { process(:forwarded_by_group_to_group) }.to change { User.where(staged: true).count }.by(4)
+          topic = Topic.last
+          forwarded_small_post = topic.ordered_posts.last
+          expect(forwarded_small_post.action_code).to eq("forwarded")
+          expect(forwarded_small_post.user).to eq(User.find_by_email("team@somesmtpaddress.com"))
+        end
+
+        it "keeps the CC addresses from the forwarded email" do
+          expect { process(:forwarded_by_group_to_group) }.to change { User.where(staged: true).count }.by(4)
+          topic = Topic.last
+          cc_user1 = User.find_by_email("terry@ccland.com")
+          cc_user2 = User.find_by_email("don@mmtest.com")
+          expect(cc_user1).not_to eq(nil)
+          expect(cc_user2).not_to eq(nil)
+          expect(topic.topic_allowed_users.pluck(:user_id)).to include(cc_user1.id)
+          expect(topic.incoming_email.first.cc_addresses).to eq("terry@ccland.com;don@mmtest.com")
+        end
+
+        context "when staged user for the team email already exists" do
+          let!(:staged_team_user) do
+            User.create!(
+              email: "team@somesmtpaddress.com",
+              username: UserNameSuggester.suggest("team@somesmtpaddress.com"),
+              name: "team teamson",
+              staged: true
+            )
+          end
+
+          it "uses that and does not create another staged user" do
+            expect { process(:forwarded_by_group_to_group) }.to change { User.where(staged: true).count }.by(3)
+            topic = Topic.last
+            forwarded_small_post = topic.ordered_posts.last
+            expect(forwarded_small_post.action_code).to eq("forwarded")
+            expect(forwarded_small_post.user).to eq(staged_team_user)
+          end
+        end
+      end
     end
 
     context "emailing a group by email_username and following reply flow" do

spec/fixtures/emails/forwarded_by_group_to_group.eml

@@ -17,6 +17,7 @@ From: Fred Flintstone <fred@bedrock.com>
 Date: Mon, 1 Dec 2016 13:37:42 +0100
 Subject: Re: Login problems
 To: Discourse Team <team@somesmtpaddress.com>
+CC: Terry Jones <terry@ccland.com>, don@mmtest.com
 
 Hello I am having some issues with my forum.