Revert "SECURITY: BCC active user emails from group SMTP (#19724)"

This reverts commit 7bd83ef6b5.
This commit is contained in:
Alan Guo Xiang Tan 2023-01-05 09:34:51 +08:00
parent c83a7c91d1
commit 1cb5200450
8 changed files with 6 additions and 65 deletions

View File

@ -46,12 +46,6 @@ module Jobs
cc.match(EmailValidator.email_regex) ? cc : nil
end.compact
# 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,
@ -74,13 +68,7 @@ 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: cc_addresses,
bcc_addresses: bcc_addresses
)
message = GroupSmtpMailer.send_mail(group, email, post, cc_addresses)
Email::Sender.new(message, :group_smtp, recipient_user).send
# Create an incoming email record to avoid importing again from IMAP

View File

@ -5,7 +5,7 @@ require_dependency 'email/message_builder'
class GroupSmtpMailer < ActionMailer::Base
include Email::BuildEmailHelper
def send_mail(from_group, to_address, post, cc_addresses: nil, bcc_addresses: nil)
def send_mail(from_group, to_address, post, cc_addresses = nil)
raise 'SMTP is disabled' if !SiteSetting.enable_smtp
op_incoming_email = post.topic.first_post.incoming_email
@ -51,8 +51,7 @@ class GroupSmtpMailer < ActionMailer::Base
from: from_group.email_username,
from_alias: I18n.t('email_from_without_site', user_name: group_name),
html_override: html_override(post),
cc: cc_addresses,
bcc: bcc_addresses
cc: cc_addresses
)
end

View File

@ -132,7 +132,6 @@ end
# cc_user_ids :integer is an Array
# raw :text
# topic_id :integer
# bcc_addresses :text
#
# Indexes
#

View File

@ -1,7 +0,0 @@
# frozen_string_literal: true
class AddBccAddressesToEmailLog < ActiveRecord::Migration[7.0]
def change
add_column :email_logs, :bcc_addresses, :text, null: true
end
end

View File

@ -141,8 +141,7 @@ module Email
body: body,
charset: 'UTF-8',
from: from_value,
cc: @opts[:cc],
bcc: @opts[:bcc]
cc: @opts[:cc]
}
args[:delivery_method_options] = @opts[:delivery_method_options] if @opts[:delivery_method_options]

View File

@ -98,10 +98,6 @@ 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')
@ -327,12 +323,6 @@ 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?

View File

@ -40,16 +40,7 @@ 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,
cc_addresses: %w[otherguy@test.com cormac@lit.com],
bcc_addresses: [],
)
.returns(message)
GroupSmtpMailer.expects(:send_mail).with(group, "test@test.com", post, ["otherguy@test.com", "cormac@lit.com"]).returns(message)
subject.execute(args)
end
@ -185,27 +176,11 @@ 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)
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"])
expect(ActionMailer::Base.deliveries.last.subject).to eq("Re: Help I need support")
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

View File

@ -36,7 +36,6 @@ describe GroupSmtpMailer do
Message-ID: <a52f67a3d3560f2a35276cda8519b10b595623bcb66912bb92df6651ad5f75be@mail.gmail.com>
Subject: Hello from John
To: "bugs@gmail.com" <bugs@gmail.com>
Cc: someotherperson@test.com
Content-Type: text/plain; charset="UTF-8"
Hello,
@ -77,7 +76,6 @@ 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)