diff --git a/app/jobs/regular/group_smtp_email.rb b/app/jobs/regular/group_smtp_email.rb index b06801be188..b0fc4a890e9 100644 --- a/app/jobs/regular/group_smtp_email.rb +++ b/app/jobs/regular/group_smtp_email.rb @@ -44,6 +44,12 @@ module Jobs EmailAddressValidator.valid_value?(address) end + # Mask the email addresses of non-staged users so + # they are not revealed unnecessarily when we are sending + # the email notification out. + bcc_addresses = User.not_staged.with_email(cc_addresses).pluck(:email) + cc_addresses = cc_addresses - bcc_addresses + # There is a rare race condition causing the Imap::Sync class to create # an incoming email and associated post/topic, which then kicks off # the PostAlerter to notify others in the PM about a reply in the topic, @@ -66,7 +72,13 @@ module Jobs # The EmailLog record created by the sender will have the raw email # stored, the group smtp ID, and any cc addresses recorded for later # cross referencing. - message = GroupSmtpMailer.send_mail(group, email, post, cc_addresses) + message = GroupSmtpMailer.send_mail( + group, + email, + post, + cc_addresses: cc_addresses, + bcc_addresses: bcc_addresses + ) Email::Sender.new(message, :group_smtp, recipient_user).send # Create an incoming email record to avoid importing again from IMAP diff --git a/app/mailers/group_smtp_mailer.rb b/app/mailers/group_smtp_mailer.rb index 5a72c1c92c7..c394439026a 100644 --- a/app/mailers/group_smtp_mailer.rb +++ b/app/mailers/group_smtp_mailer.rb @@ -3,7 +3,7 @@ class GroupSmtpMailer < ActionMailer::Base include Email::BuildEmailHelper - def send_mail(from_group, to_address, post, cc_addresses = nil) + def send_mail(from_group, to_address, post, cc_addresses: nil, bcc_addresses: nil) raise 'SMTP is disabled' if !SiteSetting.enable_smtp op_incoming_email = post.topic.first_post.incoming_email @@ -46,7 +46,8 @@ class GroupSmtpMailer < ActionMailer::Base from: from_group.smtp_from_address, from_alias: I18n.t('email_from_without_site', group_name: group_name), html_override: html_override(post), - cc: cc_addresses + cc: cc_addresses, + bcc: bcc_addresses ) end diff --git a/app/models/email_log.rb b/app/models/email_log.rb index 8e3c7b163d3..b5ab6d6402d 100644 --- a/app/models/email_log.rb +++ b/app/models/email_log.rb @@ -144,6 +144,7 @@ end # topic_id :integer # bounce_error_code :string # smtp_transaction_response :string(500) +# bcc_addresses :text # # Indexes # diff --git a/db/migrate/20221125001635_add_bcc_addresses_to_email_log.rb b/db/migrate/20221125001635_add_bcc_addresses_to_email_log.rb new file mode 100644 index 00000000000..a54c55312df --- /dev/null +++ b/db/migrate/20221125001635_add_bcc_addresses_to_email_log.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddBccAddressesToEmailLog < ActiveRecord::Migration[7.0] + def change + add_column :email_logs, :bcc_addresses, :text, null: true + end +end diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb index 62236252e0d..9308705cd64 100644 --- a/lib/email/message_builder.rb +++ b/lib/email/message_builder.rb @@ -141,7 +141,8 @@ module Email body: body, charset: 'UTF-8', from: from_value, - cc: @opts[:cc] + cc: @opts[:cc], + bcc: @opts[:bcc] } args[:delivery_method_options] = @opts[:delivery_method_options] if @opts[:delivery_method_options] diff --git a/lib/email/sender.rb b/lib/email/sender.rb index fd6148cdc65..c31df8b76ac 100644 --- a/lib/email/sender.rb +++ b/lib/email/sender.rb @@ -98,6 +98,10 @@ module Email email_log.cc_user_ids = User.with_email(cc_addresses).pluck(:id) end + if bcc_addresses.any? + email_log.bcc_addresses = bcc_addresses.join(";") + end + host = Email::Sender.host_for(Discourse.base_url) post_id = header_value('X-Discourse-Post-Id') @@ -300,6 +304,12 @@ module Email end end + def bcc_addresses + @bcc_addresses ||= begin + @message.try(:bcc) || [] + end + end + def self.host_for(base_url) host = "localhost" if base_url.present? diff --git a/spec/jobs/regular/group_smtp_email_spec.rb b/spec/jobs/regular/group_smtp_email_spec.rb index 898506f4f4c..d01ce2c02d8 100644 --- a/spec/jobs/regular/group_smtp_email_spec.rb +++ b/spec/jobs/regular/group_smtp_email_spec.rb @@ -36,7 +36,16 @@ RSpec.describe Jobs::GroupSmtpEmail do it "sends an email using the GroupSmtpMailer and Email::Sender" do message = Mail::Message.new(body: "hello", to: "myemail@example.invalid") - GroupSmtpMailer.expects(:send_mail).with(group, "test@test.com", post, ["otherguy@test.com", "cormac@lit.com"]).returns(message) + GroupSmtpMailer + .expects(:send_mail) + .with( + group, + "test@test.com", + post, + cc_addresses: %w[otherguy@test.com cormac@lit.com], + bcc_addresses: [], + ) + .returns(message) subject.execute(args) end @@ -172,11 +181,27 @@ RSpec.describe Jobs::GroupSmtpEmail do it "has the cc_addresses and cc_user_ids filled in correctly" do subject.execute(args) expect(ActionMailer::Base.deliveries.count).to eq(1) - expect(ActionMailer::Base.deliveries.last.subject).to eq("Re: Help I need support") + sent_mail = ActionMailer::Base.deliveries.last + expect(sent_mail.subject).to eq("Re: Help I need support") + expect(sent_mail.cc).to eq(["otherguy@test.com", "cormac@lit.com"]) email_log = EmailLog.find_by(post_id: post.id, topic_id: post.topic_id, user_id: recipient_user.id) expect(email_log.cc_addresses).to eq("otherguy@test.com;cormac@lit.com") expect(email_log.cc_user_ids).to match_array([staged1.id, staged2.id]) end + + it "where cc_addresses match non-staged users, convert to bcc_addresses" do + staged2.update!(staged: false, active: true) + subject.execute(args) + expect(ActionMailer::Base.deliveries.count).to eq(1) + sent_mail = ActionMailer::Base.deliveries.last + expect(sent_mail.subject).to eq("Re: Help I need support") + expect(sent_mail.cc).to eq(["otherguy@test.com"]) + expect(sent_mail.bcc).to eq(["cormac@lit.com"]) + email_log = EmailLog.find_by(post_id: post.id, topic_id: post.topic_id, user_id: recipient_user.id) + expect(email_log.cc_addresses).to eq("otherguy@test.com") + expect(email_log.bcc_addresses).to eq("cormac@lit.com") + expect(email_log.cc_user_ids).to match_array([staged1.id]) + end end context "when the post in the argument is the OP" do diff --git a/spec/mailers/group_smtp_mailer_spec.rb b/spec/mailers/group_smtp_mailer_spec.rb index 9bf091f697e..7677cf455c4 100644 --- a/spec/mailers/group_smtp_mailer_spec.rb +++ b/spec/mailers/group_smtp_mailer_spec.rb @@ -35,6 +35,7 @@ RSpec.describe GroupSmtpMailer do Message-ID: Subject: Hello from John To: "bugs@gmail.com" + Cc: someotherperson@test.com Content-Type: text/plain; charset="UTF-8" Hello, @@ -75,6 +76,7 @@ RSpec.describe GroupSmtpMailer do sent_mail = ActionMailer::Base.deliveries[0] expect(sent_mail.to).to contain_exactly('john@doe.com') + expect(sent_mail.cc).to contain_exactly('someotherperson@test.com') expect(sent_mail.reply_to).to eq(nil) expect(sent_mail.subject).to eq('Re: Hello from John') expect(sent_mail.to_s).to include(raw)