Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion app/jobs/reports/deleted_user_accounts_report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ def perform(_date)
emails = report_hash['emails']
issuers = report_hash['issuers']
report = deleted_user_accounts_data_for_issuers(issuers)
# rubocop:disable IdentityIdp/MailLaterLinter
emails.each do |email|
ReportMailer.deleted_user_accounts_report(
email: email,
name: name,
issuers: issuers,
data: report,
).deliver_now_or_later
).deliver_now
# rubocop:enable IdentityIdp/MailLaterLinter
end
end
end
Expand Down
5 changes: 5 additions & 0 deletions app/mailers/user_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class UserEmailAddressMismatchError < StandardError; end

before_action :validate_user_and_email_address
before_action :attach_images
after_action :add_metadata
default(
from: email_with_name(
IdentityConfig.store.email_from,
Expand All @@ -43,6 +44,10 @@ def validate_user_and_email_address
end
end

def add_metadata
message.instance_variable_set(:@_metadata, { user: user, action: action_name })
end

def email_confirmation_instructions(token, request_id:, instructions:)
with_user_locale(user) do
presenter = ConfirmationEmailPresenter.new(user, view_context)
Expand Down
12 changes: 12 additions & 0 deletions app/services/analytics_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2978,5 +2978,17 @@ def idv_personal_key_acknowledgment_toggled(checked:, proofing_components:, **ex
**extra,
)
end

# Logs after an email is sent
# @param [String] action type of email being sent
# @param [String, nil] ses_message_id AWS SES Message ID
def email_sent(action:, ses_message_id:, **extra)
track_event(
'Email Sent',
action: action,
ses_message_id: ses_message_id,
**extra,
)
end
end
# rubocop:enable Metrics/ModuleLength
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ def self.call
return unless Db::ServiceProviderQuotaLimit::AnySpOverQuotaLimit.call
email_list = IdentityConfig.store.sps_over_quota_limit_notify_email_list
email_list.each do |email|
ReportMailer.sps_over_quota_limit(email).deliver_now_or_later
# rubocop:disable IdentityIdp/MailLaterLinter
ReportMailer.sps_over_quota_limit(email).deliver_now
# rubocop:enable IdentityIdp/MailLaterLinter
Comment on lines 8 to 10
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 what if we only applied the lint to UserMailer and its subclasses instead of ApplicationMailer?

Copy link
Copy Markdown
Contributor Author

@mitchellhenke mitchellhenke Oct 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that'd make sense, I don't know how to do that off-hand from a brief look at the current linter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok! I'll poke around at it for a follow-up

end
end
end
Expand Down
4 changes: 4 additions & 0 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@
require_relative '../lib/identity_config'
require_relative '../lib/fingerprinter'
require_relative '../lib/identity_job_log_subscriber'
require_relative '../lib/email_delivery_observer'

Bundler.require(*Rails.groups)

require_relative '../lib/mailer_sensitive_information_checker'

APP_NAME = 'Login.gov'.freeze

module Identity
Expand Down Expand Up @@ -93,6 +96,7 @@ class Application < Rails::Application
mail.display_name = IdentityConfig.store.email_from_display_name
end.to_s,
}
config.action_mailer.observers = %w[EmailDeliveryObserver]

require 'headers_filter'
config.middleware.insert_before 0, HeadersFilter
Expand Down
10 changes: 10 additions & 0 deletions config/initializers/ext_action_mailer.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
# Monkeypatches the MessageDelivery to add deliver_now_or_later that
# can route between #deliver_now and #deliver_later
module DeliverLaterArgumentChecker
def deliver_later(...)
MailerSensitiveInformationChecker.check_for_sensitive_pii!(@params, @args, @action)
super(...)
end
end

module ActionMailer
class MessageDelivery
prepend DeliverLaterArgumentChecker

def deliver_now_or_later(opts = {})
MailerSensitiveInformationChecker.check_for_sensitive_pii!(@params, @args, @action)
# rubocop:disable IdentityIdp/MailLaterLinter
if IdentityConfig.store.deliver_mail_async
deliver_later(opts)
Expand Down
6 changes: 3 additions & 3 deletions lib/aws/ses.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ class Base
def initialize(*); end

def deliver(mail)
response = ses_client.send_raw_email(raw_message: { data: mail.to_s })
mail.message_id = "#{response.message_id}@email.amazonses.com"
response
ses_client.send_raw_email(raw_message: { data: mail.to_s }).tap do |response|
mail.header[:ses_message_id] = response.message_id
end
end

alias deliver! deliver
Expand Down
9 changes: 9 additions & 0 deletions lib/email_delivery_observer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class EmailDeliveryObserver
def self.delivered_email(mail)
metadata = mail.instance_variable_get(:@_metadata) || {}
user = metadata[:user] || AnonymousUser.new
action = metadata[:action]
Analytics.new(user: user, request: nil, sp: nil, session: {}).
email_sent(action: action, ses_message_id: mail.header[:ses_message_id]&.value)
end
end
54 changes: 54 additions & 0 deletions lib/mailer_sensitive_information_checker.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
class MailerSensitiveInformationChecker
class SensitiveKeyError < StandardError; end

class SensitiveValueError < StandardError; end

def self.check_for_sensitive_pii!(params, args_array, action)
args = ActiveJob::Arguments.serialize(args_array)
serialized_params = ActiveJob::Arguments.serialize(params)
serialized_args_string = args.to_s
serialized_params_string = serialized_params.to_s

if params[:email_address].is_a?(EmailAddress)
check_hash(params.except(:email_address), action)
else
check_hash(params, action)
end
args.each do |arg|
next unless arg.is_a?(Hash)
check_hash(arg, action)
end

if SessionEncryptor::SENSITIVE_REGEX.match?(serialized_args_string)
self.alert(SensitiveValueError.new)
end

if SessionEncryptor::SENSITIVE_REGEX.match?(serialized_params_string)
self.alert(SensitiveValueError.new)
end
end

def self.check_hash(hash, action)
hash.deep_transform_keys do |key|
if SessionEncryptor::SENSITIVE_KEYS.include?(key.to_s)
exception = SensitiveKeyError.new(
"#{key} unexpectedly appeared in #{action} Mailer args",
)
self.alert(exception)
end
end
end

def self.alert(exception)
if IdentityConfig.store.session_encryptor_alert_enabled
NewRelic::Agent.notice_error(exception)
else
raise exception
end
end

class << self
include ::NewRelic::Agent::MethodTracer
add_method_tracer :check_for_sensitive_pii!, "Custom/#{name}/check_for_sensitive_pii!"
end
end
2 changes: 1 addition & 1 deletion spec/lib/aws/ses_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@

it 'sets the message id on the mail argument' do
subject.deliver!(mail)
expect(mail.message_id).to eq('123abc@email.amazonses.com')
expect(mail.header['ses-message-id'].value).to eq('123abc')
end

it 'retries timed out requests' do
Expand Down
2 changes: 1 addition & 1 deletion spec/mailers/previews/user_mailer_preview_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
it 'has a preview method for each mailer method' do
mailer_methods = UserMailer.instance_methods(false)
preview_methods = UserMailerPreview.instance_methods(false)
mailer_helper_methods = [:email_address, :user, :validate_user_and_email_address]
mailer_helper_methods = [:email_address, :user, :validate_user_and_email_address, :add_metadata]
expect(mailer_methods - mailer_helper_methods - preview_methods).to be_empty
end

Expand Down
27 changes: 27 additions & 0 deletions spec/mailers/user_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -670,4 +670,31 @@ def expect_email_body_to_have_help_and_contact_links
)
end
end

# rubocop:disable IdentityIdp/MailLaterLinter
describe '#deliver_later' do
it 'does not queue email if it potentially contains sensitive value' do
user = create(:user)
mailer = UserMailer.with(
user: user,
email_address: user.email_addresses.first,
).add_email(Idp::Constants::MOCK_IDV_APPLICANT[:last_name])
expect { mailer.deliver_later }.to raise_error(
MailerSensitiveInformationChecker::SensitiveValueError,
)
end

it 'does not queue email if it potentially contains sensitive keys' do
user = create(:user)
mailer = UserMailer.with(user: user, email_address: user.email_addresses.first).add_email(
{
first_name: nil,
},
)
expect { mailer.deliver_later }.to raise_error(
MailerSensitiveInformationChecker::SensitiveKeyError,
)
end
end
# rubocop:enable IdentityIdp/MailLaterLinter
end