From 9f36d8ad433aaa0eec051b49938fff7d3d8e06b6 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Tue, 7 Sep 2021 08:46:28 +1000 Subject: [PATCH] FIX: Capture CC addresses for forwarded emails (#14254) When forwarding emails into the group inbox, we now use the original sender email as the from_address since 2ac9fd9dff5a2a9210e2b1b765e1a324bbb4f421. However, we have not been saving the original CC addresses of the forwarded email, which are needed to include those recipients in on the conversation when replying via the group inbox. This commit captures the CC addresses on the incoming email, and makes sure the emails are created as staged users and added to the list of topic allowed users so they are included on CC's sent by the GroupSmtpEmail and other jobs. --- lib/email/receiver.rb | 26 +++++++++----- spec/components/email/receiver_spec.rb | 31 ++++++++++++++-- .../emails/forwarded_by_group_to_inbox.eml | 1 + .../forwarded_by_group_to_inbox_double_cc.eml | 36 +++++++++++++++++++ 4 files changed, 83 insertions(+), 11 deletions(-) create mode 100644 spec/fixtures/emails/forwarded_by_group_to_inbox_double_cc.eml diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 2a0ce1920de..2b2cd363950 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -143,13 +143,17 @@ module Email end def create_incoming_email + cc_addresses = Array.wrap(@mail.cc) + if has_been_forwarded? && 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], @@ -956,7 +960,7 @@ module Email 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) + @forwarded_by_user = stage_sender_user(forwarded_by_address, forwarded_by_name) end return false if email.blank? || !email["@"] @@ -985,10 +989,10 @@ module Email post_type: post_type, skip_validations: user.staged?) else - if forwarded_by_user - post.topic.topic_allowed_users.find_or_create_by!(user_id: forwarded_by_user.id) + 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") + post.topic.add_small_action(@forwarded_by_user || user, "forwarded") end end @@ -1324,7 +1328,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 @@ -1346,11 +1354,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 diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index e8be9d1148f..3f6e103adc2 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -1064,13 +1064,40 @@ describe Email::Receiver do 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_inbox) }.to change { User.where(staged: true).count }.by(2) + expect { process(:forwarded_by_group_to_inbox) }.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 track of the cc addresses of the forwarded email and creates staged users for them" do + expect { process(:forwarded_by_group_to_inbox) }.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@ccland.com") + fred_user = User.find_by_email("fred@bedrock.com") + team_user = User.find_by_email("team@somesmtpaddress.com") + expect(topic.incoming_email.first.cc_addresses).to eq("terry@ccland.com;don@ccland.com") + expect(topic.topic_allowed_users.pluck(:user_id)).to match_array([ + fred_user.id, team_user.id, cc_user1.id, cc_user2.id + ]) + end + + it "keeps track of the cc addresses of the final forwarded email as well" do + expect { process(:forwarded_by_group_to_inbox_double_cc) }.to change { User.where(staged: true).count }.by(5) + topic = Topic.last + cc_user1 = User.find_by_email("terry@ccland.com") + cc_user2 = User.find_by_email("don@ccland.com") + fred_user = User.find_by_email("fred@bedrock.com") + team_user = User.find_by_email("team@somesmtpaddress.com") + someother_user = User.find_by_email("someotherparty@test.com") + expect(topic.incoming_email.first.cc_addresses).to eq("someotherparty@test.com;terry@ccland.com;don@ccland.com") + expect(topic.topic_allowed_users.pluck(:user_id)).to match_array([ + fred_user.id, team_user.id, someother_user.id, cc_user1.id, cc_user2.id + ]) + end + context "when staged user for the team email already exists" do let!(:staged_team_user) do User.create!( @@ -1082,7 +1109,7 @@ describe Email::Receiver do end it "uses that and does not create another staged user" do - expect { process(:forwarded_by_group_to_inbox) }.to change { User.where(staged: true).count }.by(1) + expect { process(:forwarded_by_group_to_inbox) }.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") diff --git a/spec/fixtures/emails/forwarded_by_group_to_inbox.eml b/spec/fixtures/emails/forwarded_by_group_to_inbox.eml index fe170e5d490..c20a1dc486e 100644 --- a/spec/fixtures/emails/forwarded_by_group_to_inbox.eml +++ b/spec/fixtures/emails/forwarded_by_group_to_inbox.eml @@ -17,6 +17,7 @@ From: Fred Flintstone Date: Mon, 1 Dec 2016 13:37:42 +0100 Subject: Re: Login problems To: Discourse Team +CC: Terry Jones , don@ccland.com Hello I am having some issues with my forum. diff --git a/spec/fixtures/emails/forwarded_by_group_to_inbox_double_cc.eml b/spec/fixtures/emails/forwarded_by_group_to_inbox_double_cc.eml new file mode 100644 index 00000000000..1b4f8558b0f --- /dev/null +++ b/spec/fixtures/emails/forwarded_by_group_to_inbox_double_cc.eml @@ -0,0 +1,36 @@ +Message-ID: <58@somesmtpaddress.mail> +From: Discourse Team +To: support+team@bar.com +CC: someotherparty@test.com +Date: Mon, 1 Dec 2016 13:37:42 +0100 +Subject: Fwd: Login problems +Content-Type: multipart/related; boundary="00000000000072702105c89858de" + +--00000000000072702105c89858de +Content-Type: multipart/alternative; boundary="00000000000072702005c89858dd" + +--00000000000072702005c89858dd +Content-Type: text/plain; charset="UTF-8" +Content-Transfer-Encoding: quoted-printable + +---------- Forwarded message --------- +From: Fred Flintstone +Date: Mon, 1 Dec 2016 13:37:42 +0100 +Subject: Re: Login problems +To: Discourse Team +CC: Terry Jones , don@ccland.com + +Hello I am having some issues with my forum. + +Fred + +--00000000000072702005c89858dd +Content-Type: text/html; charset="UTF-8" +Content-Transfer-Encoding: quoted-printable + +

Hello I am having some issues with my forum.

+ +
Fred
+ +--00000000000072702005c89858dd-- +--00000000000072702105c89858de