From 075f2c327b8535863f83271a3ca267e77fe50aad Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Mon, 9 Sep 2024 15:24:33 -0400 Subject: [PATCH 1/2] Use `Profile#initiating_service_provider` to determine SP named in `account_verified` email MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a user completes verification we send them an `account_verified` email which contains the following information: - The date the verification occurred - The service provider for which the verification was performed This applies to all verifications except for verifications through in-person which have their own email. Prior to this commit the service provider was determined using the SP in the session. This means that when the user is verifying in an out-of-band flow (verify-by-mail or fraud review in this case) their email would likely not contain the name of the SP that triggered the verification. For verify-by-mail we tell the user to enter their code by going to “secure.login.gov”. In this case there is no SP in the session. For fraud review we made no effort to send an email with a named SP. This commit modifies the logic to use `Profile#initiating_service_provider` to determine which SP to name in the email. As a result the email should contain the name of the service provider for which the user was originally proofing. This will be true for the inline flow and the out-of-band flows. The `UserAlerts::AlertUserAboutAccountVerified` service is used to send these `account_verified` notifications. This commit changes this service to take a profile as its only argument since all of the information necessary for this email is accessible via the profile model. [skip changelog] --- .../idv/by_mail/enter_code_controller.rb | 6 +-- .../idv/enter_password_controller.rb | 6 +-- .../alert_user_about_account_verified.rb | 7 ++-- lib/action_account.rb | 13 +------ .../idv/by_mail/enter_code_controller_spec.rb | 4 +- .../idv/enter_password_controller_spec.rb | 6 ++- spec/lib/action_account_spec.rb | 4 ++ .../alert_user_about_account_verified_spec.rb | 37 +++++++++++++------ 8 files changed, 48 insertions(+), 35 deletions(-) diff --git a/app/controllers/idv/by_mail/enter_code_controller.rb b/app/controllers/idv/by_mail/enter_code_controller.rb index 772bac0757a..519f5105c8e 100644 --- a/app/controllers/idv/by_mail/enter_code_controller.rb +++ b/app/controllers/idv/by_mail/enter_code_controller.rb @@ -101,12 +101,10 @@ def note_if_user_did_not_receive_letter def prepare_for_personal_key unless account_not_ready_to_be_activated? - event, _disavowal_token = create_user_event(:account_verified) + create_user_event(:account_verified) UserAlerts::AlertUserAboutAccountVerified.call( - user: current_user, - date_time: event.created_at, - sp_name: decorated_sp_session.sp_name, + profile: current_user.active_profile, ) flash[:success] = t('account.index.verification.success') end diff --git a/app/controllers/idv/enter_password_controller.rb b/app/controllers/idv/enter_password_controller.rb index ee8efbb2ddc..da49e8d3e84 100644 --- a/app/controllers/idv/enter_password_controller.rb +++ b/app/controllers/idv/enter_password_controller.rb @@ -134,11 +134,9 @@ def init_profile end if idv_session.profile.active? - event, _disavowal_token = create_user_event(:account_verified) + create_user_event(:account_verified) UserAlerts::AlertUserAboutAccountVerified.call( - user: current_user, - date_time: event.created_at, - sp_name: decorated_sp_session.sp_name, + profile: idv_session.profile, ) end end diff --git a/app/services/user_alerts/alert_user_about_account_verified.rb b/app/services/user_alerts/alert_user_about_account_verified.rb index 4ad165888d4..a03a27d942c 100644 --- a/app/services/user_alerts/alert_user_about_account_verified.rb +++ b/app/services/user_alerts/alert_user_about_account_verified.rb @@ -2,11 +2,12 @@ module UserAlerts class AlertUserAboutAccountVerified - def self.call(user:, date_time:, sp_name:) - sp_name ||= APP_NAME + def self.call(profile:) + user = profile.user + sp_name ||= profile.initiating_service_provider&.friendly_name || APP_NAME user.confirmed_email_addresses.each do |email_address| UserMailer.with(user: user, email_address: email_address).account_verified( - date_time: date_time, + date_time: profile.verified_at, sp_name: sp_name, ).deliver_now_or_later end diff --git a/lib/action_account.rb b/lib/action_account.rb index 2ef444c7d9b..cc6c1982f87 100644 --- a/lib/action_account.rb +++ b/lib/action_account.rb @@ -254,14 +254,6 @@ def run(args:, config:) class ReviewPass include LogBase - def alert_verified(user:, date_time:) - UserAlerts::AlertUserAboutAccountVerified.call( - user: user, - date_time: date_time, - sp_name: nil, - ) - end - def run(args:, config:) uuids = args @@ -286,10 +278,9 @@ def run(args:, config:) success = true if profile.active? - event, _disavowal_token = UserEventCreator.new(current_user: user). + UserEventCreator.new(current_user: user). create_out_of_band_user_event(:account_verified) - - alert_verified(user: user, date_time: event.created_at) + UserAlerts::AlertUserAboutAccountVerified.call(profile: profile) log_texts << log_text[:profile_activated] else diff --git a/spec/controllers/idv/by_mail/enter_code_controller_spec.rb b/spec/controllers/idv/by_mail/enter_code_controller_spec.rb index d3d74ceeae8..de0cdf59936 100644 --- a/spec/controllers/idv/by_mail/enter_code_controller_spec.rb +++ b/spec/controllers/idv/by_mail/enter_code_controller_spec.rb @@ -215,7 +215,9 @@ it 'dispatches account verified alert' do action - expect(UserAlerts::AlertUserAboutAccountVerified).to have_received(:call) + expect(UserAlerts::AlertUserAboutAccountVerified).to have_received(:call).with( + profile: user.active_profile, + ) end context 'with establishing in person enrollment' do diff --git a/spec/controllers/idv/enter_password_controller_spec.rb b/spec/controllers/idv/enter_password_controller_spec.rb index 0a17501585d..d5c63384fec 100644 --- a/spec/controllers/idv/enter_password_controller_spec.rb +++ b/spec/controllers/idv/enter_password_controller_spec.rb @@ -394,9 +394,13 @@ def show end it 'dispatches account verified alert' do - expect(UserAlerts::AlertUserAboutAccountVerified).to receive(:call) + allow(UserAlerts::AlertUserAboutAccountVerified).to receive(:call) put :create, params: { user: { password: ControllerHelper::VALID_PASSWORD } } + + expect(UserAlerts::AlertUserAboutAccountVerified).to have_received(:call).with( + profile: user.reload.active_profile, + ) end it 'creates an `account_verified` event once per confirmation' do diff --git a/spec/lib/action_account_spec.rb b/spec/lib/action_account_spec.rb index 2ff93a4206d..e9bacd37a78 100644 --- a/spec/lib/action_account_spec.rb +++ b/spec/lib/action_account_spec.rb @@ -204,6 +204,10 @@ subject(:result) { subtask.run(args:, config:) } it 'Pass a user that has a pending review', aggregate_failures: true do + expect(UserAlerts::AlertUserAboutAccountVerified).to receive(:call).with( + profile: user.pending_profile, + ) + profile_fraud_review_pending_at = user.pending_profile.fraud_review_pending_at # rubocop:disable Layout/LineLength diff --git a/spec/services/user_alerts/alert_user_about_account_verified_spec.rb b/spec/services/user_alerts/alert_user_about_account_verified_spec.rb index 44445fd6b4e..41ab2c4f0a6 100644 --- a/spec/services/user_alerts/alert_user_about_account_verified_spec.rb +++ b/spec/services/user_alerts/alert_user_about_account_verified_spec.rb @@ -2,34 +2,49 @@ RSpec.describe UserAlerts::AlertUserAboutAccountVerified do describe '#call' do - let(:user) { create(:user, :fully_registered) } - let(:device) { create(:device, user: user) } - let(:date_time) { Time.zone.now } + let(:user) { profile.user } + let(:profile) do + create( + :profile, + :active, + initiating_service_provider: service_provider, + ) + end + let(:service_provider) { create(:service_provider) } it 'sends an email to all confirmed email addresses' do create_list(:email_address, 2, user: user) create(:email_address, user: user, confirmed_at: nil) confirmed_email_addresses = user.confirmed_email_addresses - described_class.call( - user: user, - date_time: date_time, - sp_name: '', - ) + described_class.call(profile: profile) expect_delivered_email_count(3) expect_delivered_email( to: [confirmed_email_addresses[0].email], - subject: t('user_mailer.account_verified.subject', sp_name: ''), + subject: t('user_mailer.account_verified.subject', sp_name: service_provider.friendly_name), ) expect_delivered_email( to: [confirmed_email_addresses[1].email], - subject: t('user_mailer.account_verified.subject', sp_name: ''), + subject: t('user_mailer.account_verified.subject', sp_name: service_provider.friendly_name), ) expect_delivered_email( to: [confirmed_email_addresses[2].email], - subject: t('user_mailer.account_verified.subject', sp_name: ''), + subject: t('user_mailer.account_verified.subject', sp_name: service_provider.friendly_name), ) end + + context 'when no service provider initiated the proofing event' do + let(:service_provider) { nil } + + it 'sends the email with Login.gov as the initiating service provider' do + described_class.call(profile: profile) + + expect_delivered_email( + to: [user.confirmed_email_addresses.first.email], + subject: t('user_mailer.account_verified.subject', sp_name: APP_NAME), + ) + end + end end end From 1864e0daf2628370ab4a8169d0116dc438056ec6 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Tue, 10 Sep 2024 08:33:55 -0400 Subject: [PATCH 2/2] Update app/services/user_alerts/alert_user_about_account_verified.rb Co-authored-by: Zach Margolis --- app/services/user_alerts/alert_user_about_account_verified.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/user_alerts/alert_user_about_account_verified.rb b/app/services/user_alerts/alert_user_about_account_verified.rb index a03a27d942c..cc6e5c62ca6 100644 --- a/app/services/user_alerts/alert_user_about_account_verified.rb +++ b/app/services/user_alerts/alert_user_about_account_verified.rb @@ -4,7 +4,7 @@ module UserAlerts class AlertUserAboutAccountVerified def self.call(profile:) user = profile.user - sp_name ||= profile.initiating_service_provider&.friendly_name || APP_NAME + sp_name = profile.initiating_service_provider&.friendly_name || APP_NAME user.confirmed_email_addresses.each do |email_address| UserMailer.with(user: user, email_address: email_address).account_verified( date_time: profile.verified_at,