From e6a0605c74fc52e2047be72ef2535712d74f574d Mon Sep 17 00:00:00 2001 From: Ahmet Date: Mon, 13 May 2019 09:17:38 +0100 Subject: [PATCH] Add new option hide_pii that hides emails being logged to info in action_mailer hide_pii defaults to true --- lib/loga/configuration.rb | 6 +- lib/loga/log_subscribers/action_mailer.rb | 17 ++++- spec/integration/rails/action_mailer_spec.rb | 2 +- .../log_subscribers/action_mailer_spec.rb | 69 +++++++++++++++++-- 4 files changed, 83 insertions(+), 11 deletions(-) diff --git a/lib/loga/configuration.rb b/lib/loga/configuration.rb index 2923244..f0e22ec 100644 --- a/lib/loga/configuration.rb +++ b/lib/loga/configuration.rb @@ -15,9 +15,10 @@ class Configuration ].freeze attr_accessor :device, :filter_exceptions, :filter_parameters, - :host, :level, :service_version, :sync, :tags + :host, :level, :service_version, :sync, :tags, :hide_pii attr_reader :logger, :format, :service_name + # rubocop:disable Metrics/MethodLength def initialize(user_options = {}, framework_options = {}) options = default_options.merge(framework_options) .merge(environment_options) @@ -33,11 +34,13 @@ def initialize(user_options = {}, framework_options = {}) self.service_version = options[:service_version] || ServiceVersionStrategies.call self.sync = options[:sync] self.tags = options[:tags] + self.hide_pii = options[:hide_pii] validate @logger = initialize_logger end + # rubocop:enable Metrics/MethodLength def format=(name) @format = name.to_s.to_sym @@ -68,6 +71,7 @@ def default_options level: :info, sync: true, tags: [], + hide_pii: true, } end diff --git a/lib/loga/log_subscribers/action_mailer.rb b/lib/loga/log_subscribers/action_mailer.rb index 9a8918c..4924946 100644 --- a/lib/loga/log_subscribers/action_mailer.rb +++ b/lib/loga/log_subscribers/action_mailer.rb @@ -9,7 +9,11 @@ def deliver(event) recipients = event.payload[:to].join(',') unique_id = event.payload[:unique_id] duration = event.duration.round(1) - message = "#{mailer}: Sent mail to #{recipients} in (#{duration}ms)" + message = ''.tap do |string| + string << "#{mailer}: Sent mail" + string << " to #{recipients}" unless hide_pii? + string << " in (#{duration}ms)" + end loga_event = Event.new( data: { mailer: mailer, unique_id: unique_id }, @@ -41,10 +45,15 @@ def receive(event) from = event.payload[:from] mailer = event.payload[:mailer] unique_id = event.payload[:unique_id] + message = ''.tap do |string| + string << 'Received mail' + string << " from #{from}" unless hide_pii? + string << " in (#{event.duration.round(1)}ms)" + end loga_event = Event.new( data: { mailer: mailer, unique_id: unique_id }, - message: "Received mail #{from} in (#{event.duration.round(1)}ms)", + message: message, type: 'action_mailer', ) @@ -54,6 +63,10 @@ def receive(event) def logger Loga.logger end + + def hide_pii? + Loga.configuration.hide_pii + end end end end diff --git a/spec/integration/rails/action_mailer_spec.rb b/spec/integration/rails/action_mailer_spec.rb index e9a241f..47f1ceb 100644 --- a/spec/integration/rails/action_mailer_spec.rb +++ b/spec/integration/rails/action_mailer_spec.rb @@ -44,7 +44,7 @@ it 'has the proper payload for message delivery' do FakeMailer.send_email - message_pattern = /^FakeMailer: Sent mail to user@example.com in \(*/ + message_pattern = /^FakeMailer: Sent mail \(*/ expect(last_log_entry['short_message']).to match(message_pattern) end diff --git a/spec/unit/loga/log_subscribers/action_mailer_spec.rb b/spec/unit/loga/log_subscribers/action_mailer_spec.rb index a347f0b..3b71425 100644 --- a/spec/unit/loga/log_subscribers/action_mailer_spec.rb +++ b/spec/unit/loga/log_subscribers/action_mailer_spec.rb @@ -24,11 +24,34 @@ to: ['user@example.com'], } end + let(:config) { instance_double Loga::Configuration, hide_pii: hide_pii } - it 'logs an info message' do + before do allow(Loga.logger).to receive(:info) - mailer.deliver(event) - expect(Loga.logger).to have_received(:info).with(kind_of(Loga::Event)) + allow(Loga).to receive(:configuration).and_return(config) + end + + context 'when configuration hide_pii is true' do + let(:hide_pii) { true } + + it 'logs an info message' do + mailer.deliver(event) + expect(Loga.logger).to have_received(:info).with(Loga::Event) do |event| + expect(event.message).to include('FakeMailer: Sent mail') + expect(event.message).not_to include('user@example.com') + end + end + end + + context 'when configuration option hide_pii is false' do + let(:hide_pii) { false } + + it 'logs an info message' do + mailer.deliver(event) + expect(Loga.logger).to have_received(:info).with(Loga::Event) do |event| + expect(event.message).to include('FakeMailer: Sent mail to user@example.com') + end + end end end end @@ -45,7 +68,12 @@ it 'logs an info message' do allow(Loga.logger).to receive(:debug) mailer.process(event) - expect(Loga.logger).to have_received(:debug).with(kind_of(Loga::Event)) + expect(Loga.logger).to have_received(:debug) + .with(kind_of(Loga::Event)) do |event| + expect(event.message).to include( + 'FakeMailer#hello_world: Processed outbound mail', + ) + end end end end @@ -59,11 +87,38 @@ subject: 'Lorem ipsum', } end + let(:config) { instance_double Loga::Configuration, hide_pii: hide_pii } - it 'logs an info message' do + before do allow(Loga.logger).to receive(:info) - mailer.receive(event) - expect(Loga.logger).to have_received(:info).with(kind_of(Loga::Event)) + allow(Loga).to receive(:configuration).and_return(config) + end + + context 'when configuration hide_pii is true' do + let(:hide_pii) { true } + + it 'logs an info message without email' do + mailer.receive(event) + expect(Loga.logger).to have_received(:info) + .with(kind_of(Loga::Event)) do |event| + expect(event.message).to include('Received mail') + expect(event.message).not_to include('loremipsum@example.com') + end + end + end + + context 'when configuration option hide_pii is false' do + let(:hide_pii) { false } + + it 'logs an info message with email' do + mailer.receive(event) + expect(Loga.logger).to have_received(:info) + .with(kind_of(Loga::Event)) do |event| + expect(event.message).to include( + 'Received mail from loremipsum@example.com', + ) + end + end end end end