Don’t log recipients when sending mail

Since production applications typically run with log level info and
email adresses should be considered as sensitive data we want to prevent 
them from ending up in the logs. In development mode (with log level
debug) they are still logged as part of the Mail::Message object.
This commit is contained in:
Jaap van der Plas 2019-02-13 15:55:38 +01:00 committed by George Claghorn
parent 47e3bbeb90
commit 2488901da8
5 changed files with 26 additions and 10 deletions

@ -593,6 +593,7 @@ def deliver_mail(mail) #:nodoc:
private
def set_payload_for_mail(payload, mail)
payload[:mail] = mail.encoded
payload[:mailer] = name
payload[:message_id] = mail.message_id
payload[:subject] = mail.subject
@ -601,7 +602,6 @@ def set_payload_for_mail(payload, mail)
payload[:bcc] = mail.bcc if mail.bcc.present?
payload[:cc] = mail.cc if mail.cc.present?
payload[:date] = mail.date
payload[:mail] = mail.encoded
payload[:perform_deliveries] = mail.perform_deliveries
end

@ -10,11 +10,10 @@ class LogSubscriber < ActiveSupport::LogSubscriber
def deliver(event)
info do
perform_deliveries = event.payload[:perform_deliveries]
recipients = Array(event.payload[:to]).join(", ")
if perform_deliveries
"Sent mail to #{recipients} (#{event.duration.round(1)}ms)"
"Delivered mail #{event.payload[:message_id]} (#{event.duration.round(1)}ms)"
else
"Skipped sending mail to #{recipients} as `perform_deliveries` is false"
"Skipped delivery of mail #{event.payload[:message_id]} as `perform_deliveries` is false"
end
end

@ -913,6 +913,23 @@ def a_callback
ActiveSupport::Notifications.unsubscribe "process.action_mailer"
end
test "notification for deliver" do
begin
events = []
ActiveSupport::Notifications.subscribe("deliver.action_mailer") do |*args|
events << ActiveSupport::Notifications::Event.new(*args)
end
BaseMailer.welcome(body: "Hello there").deliver_now
assert_equal 1, events.length
assert_equal "deliver.action_mailer", events[0].name
assert_not_nil events[0].payload[:message_id]
ensure
ActiveSupport::Notifications.unsubscribe "deliver.action_mailer"
end
end
private
# Execute the block setting the given values and restoring old values after

@ -24,11 +24,11 @@ def set_logger(logger)
end
def test_deliver_is_notified
BaseMailer.welcome.deliver_now
BaseMailer.welcome(message_id: "123@abc").deliver_now
wait
assert_equal(1, @logger.logged(:info).size)
assert_match(/Sent mail to system@test\.lindsaar\.net/, @logger.logged(:info).first)
assert_match(/Delivered mail 123@abc/, @logger.logged(:info).first)
assert_equal(2, @logger.logged(:debug).size)
assert_match(/BaseMailer#welcome: processed outbound mail in [\d.]+ms/, @logger.logged(:debug).first)
@ -38,11 +38,11 @@ def test_deliver_is_notified
end
def test_deliver_message_when_perform_deliveries_is_false
BaseMailer.welcome_without_deliveries.deliver_now
BaseMailer.welcome_without_deliveries(message_id: "123@abc").deliver_now
wait
assert_equal(1, @logger.logged(:info).size)
assert_match("Skipped sending mail to system@test.lindsaar.net as `perform_deliveries` is false", @logger.logged(:info).first)
assert_match("Skipped delivery of mail 123@abc as `perform_deliveries` is false", @logger.logged(:info).first)
assert_equal(2, @logger.logged(:debug).size)
assert_match(/BaseMailer#welcome_without_deliveries: processed outbound mail in [\d.]+ms/, @logger.logged(:debug).first)

@ -21,8 +21,8 @@ def welcome_from_another_path(path)
mail(template_name: "welcome", template_path: path)
end
def welcome_without_deliveries
mail(template_name: "welcome")
def welcome_without_deliveries(hash = {})
mail({ template_name: "welcome" }.merge!(hash))
mail.perform_deliveries = false
end