FIX: Do not add mailing list headers to group SMTP emails (#13431)

When we are emailing people from a group inbox, we are having
a PM conversation with them, as a support account would. In this
case mailing list headers do not make sense. It is not like a forum
topic where you may have tens or hundreds of participants -- it is a
conversation between the group and a small handful of people
directly contacting the group, often just one person.

The only header left in tact was List-Unsubsribe which is important
for letting people opt out to notifications.
This commit is contained in:
Martin Brennan 2021-06-18 14:36:17 +10:00 committed by GitHub
parent 7f916ad06d
commit ff6114d83f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 32 additions and 12 deletions

View File

@ -98,6 +98,9 @@ module Email
topic_id = header_value('X-Discourse-Topic-Id')
reply_key = set_reply_key(post_id, user_id)
from_address = @message.from&.first
smtp_group_id = from_address.blank? ? nil : Group.where(
email_username: from_address, smtp_enabled: true
).pluck_first(:id)
# always set a default Message ID from the host
@message.header['Message-ID'] = "<#{SecureRandom.uuid}@#{host}>"
@ -161,20 +164,29 @@ module Email
list_id = "#{SiteSetting.title} <#{host}>"
end
# https://www.ietf.org/rfc/rfc3834.txt
@message.header['Precedence'] = 'list'
@message.header['List-ID'] = list_id
# When we are emailing people from a group inbox, we are having a PM
# conversation with them, as a support account would. In this case
# mailing list headers do not make sense. It is not like a forum topic
# where you may have tens or hundreds of participants -- it is a
# conversation between the group and a small handful of people
# directly contacting the group, often just one person.
if !smtp_group_id
if topic
if SiteSetting.private_email?
@message.header['List-Archive'] = "#{Discourse.base_url}#{topic.slugless_url}"
else
@message.header['List-Archive'] = topic.url
# https://www.ietf.org/rfc/rfc3834.txt
@message.header['Precedence'] = 'list'
@message.header['List-ID'] = list_id
if topic
if SiteSetting.private_email?
@message.header['List-Archive'] = "#{Discourse.base_url}#{topic.slugless_url}"
else
@message.header['List-Archive'] = topic.url
end
end
end
end
if reply_key.present? && @message.header['Reply-To'].to_s =~ /\<([^\>]+)\>/
if reply_key.present? && @message.header['Reply-To'].to_s =~ /\<([^\>]+)\>/ && !smtp_group_id
email = Regexp.last_match[1]
@message.header['List-Post'] = "<mailto:#{email}>"
end
@ -231,9 +243,7 @@ module Email
# Log when a message is being sent from a group SMTP address, so we
# can debug deliverability issues.
if from_address && smtp_group_id = Group.where(email_username: from_address, smtp_enabled: true).pluck_first(:id)
email_log.smtp_group_id = smtp_group_id
end
email_log.smtp_group_id = smtp_group_id
DiscourseEvent.trigger(:before_email_send, @message, @email_type)

View File

@ -366,6 +366,16 @@ describe Email::Sender do
expect(email_log.user_id).to be_blank
expect(email_log.smtp_group_id).to eq(group.id)
end
it "does not add any of the mailing list headers" do
TopicAllowedGroup.create(topic: post.topic, group: group)
email_sender.send
expect(message.header['List-ID']).to eq(nil)
expect(message.header['List-Post']).to eq(nil)
expect(message.header['List-Archive']).to eq(nil)
expect(message.header['Precedence']).to eq(nil)
end
end
end