diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 1f41ae6dee3..330388e7f38 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -2,7 +2,8 @@ class UserMailer < ActionMailer::Base include Mailable include LocaleHelper before_action :attach_images - default from: email_with_name(Figaro.env.email_from, Figaro.env.email_from) + default from: email_with_name(Figaro.env.email_from, Figaro.env.email_from), + reply_to: email_with_name(Figaro.env.email_from, Figaro.env.email_from) def email_changed(old_email) mail(to: old_email, subject: t('mailer.email_change_notice.subject')) diff --git a/config/initializers/devise_mailer_helpers.rb b/config/initializers/devise_mailer_helpers.rb deleted file mode 100644 index aa1cb85a475..00000000000 --- a/config/initializers/devise_mailer_helpers.rb +++ /dev/null @@ -1,27 +0,0 @@ -# This overrides the Devise mailer headers so that the recipient -# is determined based on whether or not an unconfirmed_email is present, -# as opposed to passing in the email as an argument to the job, which -# might expose it in some logs. -module Devise - module Mailers - module Helpers - def headers_for(action, opts) - headers = { - subject: subject_for(action), - to: recipient, - template_path: template_paths, - template_name: action, - }.merge(opts) - - @email = headers[:to] - headers - end - - private - - def recipient - resource.unconfirmed_email.presence || resource.email - end - end - end -end diff --git a/spec/config/initializers/active_job_logger_patch_spec.rb b/spec/config/initializers/active_job_logger_patch_spec.rb new file mode 100644 index 00000000000..ba9c12866ed --- /dev/null +++ b/spec/config/initializers/active_job_logger_patch_spec.rb @@ -0,0 +1,36 @@ +require 'rails_helper' + +# Covers config/initializers/active_job_logger_patch.rb, which overrides +# ActiveJob::Logging::LogSubscriber to standardize output and prevent sensitive +# user data from being logged. +describe ActiveJob::Logging::LogSubscriber do + it 'overrides the default job logger to output only specified parameters in JSON format' do + class FakeJob < ActiveJob::Base + def perform(sensitive_param:); end + end + + # This list corresponds to the initializer's output + permitted_attributes = %w( + timestamp + event_type + job_class + job_queue + job_id + duration + ) + + # In this case, we need to assert before the action which logs, block-style to + # match the initializer + expect(Rails.logger).to receive(:info) do |&blk| + output = JSON.parse(blk.call) + + # [Sidenote: The nested assertions don't seem to be reflected in the spec + # count--perhaps because of the uncommon block format?--but reversing them + # will show them failing as expected.] + output.keys.each { |k| expect(permitted_attributes).to include(k) } + expect(output.keys).to_not include('sensitive_param') + end + + FakeJob.perform_later(sensitive_param: '111-22-3333') + end +end diff --git a/spec/features/users/user_edit_spec.rb b/spec/features/users/user_edit_spec.rb index cc162c14cd1..9e80b4fa87d 100644 --- a/spec/features/users/user_edit_spec.rb +++ b/spec/features/users/user_edit_spec.rb @@ -4,6 +4,8 @@ let(:user) { create(:user, :signed_up) } context 'editing email' do + let(:new_email) { 'new_email@test.com' } + before do sign_in_and_2fa_user(user) visit manage_email_path @@ -15,6 +17,16 @@ expect(page).to have_current_path manage_email_path end + + scenario 'user receives confirmation message at new address' do + fill_in 'Email', with: new_email + click_button 'Update' + + open_last_email + click_email_link_matching(/confirmation_token/) + + expect(page).to have_content(new_email) + end end context 'editing 2FA phone number' do diff --git a/spec/features/visitors/password_recovery_spec.rb b/spec/features/visitors/password_recovery_spec.rb index 4a0db3975b1..bb7d8bae458 100644 --- a/spec/features/visitors/password_recovery_spec.rb +++ b/spec/features/visitors/password_recovery_spec.rb @@ -253,4 +253,29 @@ to(include('style-src \'self\' \'unsafe-inline\'')) end end + + context 'user resets password with unconfirmed email address edit' do + let(:original_email) { 'original_email@test.com' } + let(:new_email) { 'new_email@test.com' } + let(:user) { create(:user, :signed_up, email: original_email) } + + before do + sign_in_and_2fa_user(user) + visit manage_email_path + end + + it 'receives password reset message at original address' do + fill_in 'Email', with: new_email + click_button 'Update' + + expect(page).to have_content t('devise.registrations.email_update_needs_confirmation') + + visit sign_out_url + click_link t('links.passwords.forgot') + fill_in 'password_reset_email_form_email', with: original_email + click_button t('forms.buttons.continue') + + expect(open_last_email).to be_delivered_to(original_email) + end + end end