From 2488901da8a4d896003254869a378f0c1b4ed3f2 Mon Sep 17 00:00:00 2001 From: Jaap van der Plas Date: Wed, 13 Feb 2019 15:55:38 +0100 Subject: =?UTF-8?q?Don=E2=80=99t=20log=20recipients=20when=20sending=20mai?= =?UTF-8?q?l?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- actionmailer/lib/action_mailer/base.rb | 2 +- actionmailer/lib/action_mailer/log_subscriber.rb | 5 ++--- actionmailer/test/base_test.rb | 17 +++++++++++++++++ actionmailer/test/log_subscriber_test.rb | 8 ++++---- actionmailer/test/mailers/base_mailer.rb | 4 ++-- 5 files changed, 26 insertions(+), 10 deletions(-) (limited to 'actionmailer') diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 5610212fad..033a7f5b9e 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -593,6 +593,7 @@ module ActionMailer 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 @@ module ActionMailer 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 diff --git a/actionmailer/lib/action_mailer/log_subscriber.rb b/actionmailer/lib/action_mailer/log_subscriber.rb index 25c99342c2..26910f20f0 100644 --- a/actionmailer/lib/action_mailer/log_subscriber.rb +++ b/actionmailer/lib/action_mailer/log_subscriber.rb @@ -10,11 +10,10 @@ module ActionMailer 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 diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb index d0c4f189fd..15613d4dce 100644 --- a/actionmailer/test/base_test.rb +++ b/actionmailer/test/base_test.rb @@ -913,6 +913,23 @@ class BaseTest < ActiveSupport::TestCase 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 diff --git a/actionmailer/test/log_subscriber_test.rb b/actionmailer/test/log_subscriber_test.rb index fb569ce45f..f09f1997c0 100644 --- a/actionmailer/test/log_subscriber_test.rb +++ b/actionmailer/test/log_subscriber_test.rb @@ -24,11 +24,11 @@ class AMLogSubscriberTest < ActionMailer::TestCase 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 @@ class AMLogSubscriberTest < ActionMailer::TestCase 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) diff --git a/actionmailer/test/mailers/base_mailer.rb b/actionmailer/test/mailers/base_mailer.rb index c1bb48cc96..dbe1c4f0e6 100644 --- a/actionmailer/test/mailers/base_mailer.rb +++ b/actionmailer/test/mailers/base_mailer.rb @@ -21,8 +21,8 @@ class BaseMailer < ActionMailer::Base 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 -- cgit v1.2.3