From bd50e632e4062a0781d20090253a3553fcd8e726 Mon Sep 17 00:00:00 2001 From: "Gene M. Angelo, Jr" Date: Thu, 13 Oct 2022 10:26:08 -0400 Subject: [PATCH 01/11] LG-7446 Create "Cancel" Links and Supporting Cancellation Code for Identity Proofing Process (1 of n) (#7124) * Create concern to render 404 if The IdentityConfig.store.inherited_proofing_enabled returns false in preparation for use in the Inherited Proofing (IP) cancellations controller that needs to be created. Eventually, this code may get thrown away once IP goes live; however, this makes for less lines of code that need to be removed once/if it does. changelog: Improvements, Upcoming Features, LG-7446 Create Inherited Proofing Cancellation Links and Process * Add skeleton InheritedProofingCancellationsController Specs to be added in subsequent PR when controller actions are fleshed out. * Add InheritedProofingCancellationsController views and i18n * Add routes for InheritedProofingCancellationsController actions * Satisfy Brakeman violations Specifically, the "Render path contains parameter value" violation. This commit whitelists the flow steps expected and raises an error if params[:step] is not found in the whitelist. Confidence: Weak Category: Dynamic Render Path Check: Render Message: Render path contains parameter value Code: render(action => ButtonComponent.new(:action => (lambda do button_to(idv_inherited_proofing_cancel_path(:step => params[:step]), { **tag_options }, &block) end), :method => :put, :big => true, :wide => true, :outline => true).with_content(t("inherited_proofing.cancel.actions.keep_going")), {}) File: app/views/idv/inherited_proofing_cancellations/new.html.erb Line: 23 Confidence: Weak Category: Dynamic Render Path Check: Render Message: Render path contains parameter value Code: render(action => SpinnerButtonComponent.new(:action => (lambda do button_to(idv_inherited_proofing_cancel_path(:step => params[:step], :location => "cancel"), { **tag_options }, &block) end), :method => :delete, :big => true, :wide => true, :outline => true, :form => ({ :data => ({ :form_steps_wait => "" }) })) .with_content(CancellationsPresenter.new(:sp_name => decorated_session.sp_name, :url_options => url_options).exit_action_text), {}) File: app/views/idv/inherited_proofing_cancellations/new.html.erb Line: 44 --- .../images/inherited_proofing/switch.png | Bin 0 -> 998 bytes .../allow_whitelisted_flow_step_concern.rb | 37 +++++++ .../inherited_proofing_404_concern.rb | 13 +++ ...rited_proofing_cancellations_controller.rb | 93 ++++++++++++++++++ .../idv/inherited_proofing_controller.rb | 9 +- .../destroy.html.erb | 8 ++ .../new.html.erb | 55 +++++++++++ config/locales/inherited_proofing/en.yml | 16 +++ config/locales/inherited_proofing/es.yml | 17 ++++ config/locales/inherited_proofing/fr.yml | 18 ++++ config/locales/titles/en.yml | 3 + config/locales/titles/es.yml | 3 + config/locales/titles/fr.yml | 3 + config/routes.rb | 3 + 14 files changed, 270 insertions(+), 8 deletions(-) create mode 100644 app/assets/images/inherited_proofing/switch.png create mode 100644 app/controllers/concerns/allow_whitelisted_flow_step_concern.rb create mode 100644 app/controllers/concerns/inherited_proofing_404_concern.rb create mode 100644 app/controllers/idv/inherited_proofing_cancellations_controller.rb create mode 100644 app/views/idv/inherited_proofing_cancellations/destroy.html.erb create mode 100644 app/views/idv/inherited_proofing_cancellations/new.html.erb diff --git a/app/assets/images/inherited_proofing/switch.png b/app/assets/images/inherited_proofing/switch.png new file mode 100644 index 0000000000000000000000000000000000000000..315b1e56868c7f2ea44be27162df9b18cb2292ae GIT binary patch literal 998 zcmV{rvp=`~3X*{rdI%_x1et^!)Yq_xJYp_x1Mn@A~oc^YrTY?eX#Q@bU5P@9^&L z@9pmI>FVp~>FVg{>gMO^&hOOW;^g7tmT`;Ha)y2iA&yRfsjud=qTv9_(Sw4<`VaMFZt(SvHwexs+WqNb~&rK+K& zs$tD{o}#B*%W|8aq?(?inVh1SoS}}Kr%%UcO2uQ3lbMc^m_x%{Lc&{xlAVi>mWq#+ zijI{%!B{-OR)de4iH(yxz*d5enTU*&IlohZhK+!QjDUoTeuIj6gpPiKiF|>Gdw_>} ze};H{g>`y?aeIYxcz|zod~S1lX>WIEZ+U2Lc4lmJW@~h1Yjb64b7E?9VP|k*W^Z3* zZeC<)cE7W7|htBg4Udi$uJ{Dt)F~%5UjLAjY(Z*NIwDF<- zJ(82PZ=s~NSn{y+3)H+WN)Gy==%nb|2u(lQL|qSn@s|&0-jnx8bcni30FISaTnDg1 z)Lj9-m38a{uril|Ph}n3QVY5oV`wtg=%Oj;96_p)PL_f?GSpcLhLPbjO93)jffY=@ z;8ZH6f^sS=umUUa6fBrSUa;oD?Fx6nVdoxKs0+H?L;`z3lE7Y&B(N7G3G4+)0$0J@ zt-B^&biWd)3eLDDNdi-WbEygJ1+K5cUf}u`vKP2j>;9l53?joIOF=I(^s*F`29Ro?#8M#b)!Nf0wQ4)fgcSdj znpnYqIP1tiZ(7xz`CTIFHbB$&g6OM5(Fw^x$1v2qDo7qyzk!llizFY#BQG@m!+Eqr zLQotIe6`rpxuL4Rf6^oURNb(^+1kcf3I}>LThIa%o1aaF=o1c0&HOg UKraE_f&c&j07*qoM6N<$f?InVTL1t6 literal 0 HcmV?d00001 diff --git a/app/controllers/concerns/allow_whitelisted_flow_step_concern.rb b/app/controllers/concerns/allow_whitelisted_flow_step_concern.rb new file mode 100644 index 00000000000..0eaf23922ae --- /dev/null +++ b/app/controllers/concerns/allow_whitelisted_flow_step_concern.rb @@ -0,0 +1,37 @@ +# This Concern satisfies the brakeman gem "Dynamic Render Path" violation +# that is raised when rendering dynamic content in views and partials +# that come directly from params. In the below example, idv_inherited_proofing_cancel_path +# would render "/verify/inherited_proofing/cancel?step=" where +# == the value of params[:step], which could potentially be dangerous: +# <%= render ButtonComponent.new(action: ->(...) do +# button_to(idv_inherited_proofing_cancel_path(step: params[:step]), ...) ... +# end +# %> +module AllowWhitelistedFlowStepConcern + extend ActiveSupport::Concern + + included do + before_action :flow_step! + end + + private + + def flow_step! + flow_step = flow_step_param + unless flow_step_whitelist.include? flow_step + Rails.logger.warn "Flow step param \"#{flow_step})\" was not whitelisted!" + render_not_found and return + end + + @flow_step = flow_step + end + + # Override this method for flow step params other than params[:step] + def flow_step_param + params[:step] + end + + def flow_step_whitelist + raise NotImplementedError, '#flow_step_whitelist must be overridden' + end +end diff --git a/app/controllers/concerns/inherited_proofing_404_concern.rb b/app/controllers/concerns/inherited_proofing_404_concern.rb new file mode 100644 index 00000000000..51abb1e6cb2 --- /dev/null +++ b/app/controllers/concerns/inherited_proofing_404_concern.rb @@ -0,0 +1,13 @@ +module InheritedProofing404Concern + extend ActiveSupport::Concern + + included do + before_action :render_404_if_disabled + end + + private + + def render_404_if_disabled + render_not_found unless IdentityConfig.store.inherited_proofing_enabled + end +end diff --git a/app/controllers/idv/inherited_proofing_cancellations_controller.rb b/app/controllers/idv/inherited_proofing_cancellations_controller.rb new file mode 100644 index 00000000000..c11272be7b2 --- /dev/null +++ b/app/controllers/idv/inherited_proofing_cancellations_controller.rb @@ -0,0 +1,93 @@ +module Idv + class InheritedProofingCancellationsController < ApplicationController + include IdvSession + include GoBackHelper + include InheritedProofing404Concern + include AllowWhitelistedFlowStepConcern + + before_action :confirm_idv_needed + + def new + # NOTE: Uncomment this when analytics are implemented. + # properties = ParseControllerFromReferer.new(request.referer).call + # analytics.idv_inherited_proofing_cancellation_visited(step: params[:step], **properties) + self.session_go_back_path = go_back_path || idv_inherited_proofing_path + @presenter = CancellationsPresenter.new( + sp_name: decorated_session.sp_name, + url_options: url_options, + ) + end + + def update + # NOTE: Uncomment this when analytics are implemented. + # analytics.idv_inherited_proofing_cancellation_go_back(step: params[:step]) + redirect_to session_go_back_path || idv_inherited_proofing_path + end + + def destroy + # analytics.idv_inherited_proofing_cancellation_confirmed(step: params[:step]) + cancel_session + render json: { redirect_url: cancelled_redirect_path } + end + + private + + def cancel_session + cancel_idv_session + cancel_user_session + end + + def cancel_idv_session + idv_session = user_session[:idv] + idv_session&.clear + end + + def cancel_user_session + user_session['idv'] = {} + end + + def cancelled_redirect_path + return return_to_sp_failure_to_proof_path(location_params) if decorated_session.sp_name + + account_path + end + + def session_go_back_path=(path) + idv_session.go_back_path = path + end + + def session_go_back_path + idv_session.go_back_path + end + + # AllowWhitelistedFlowStepConcern Concern overrides + + def flow_step_whitelist + Idv::Flows::InheritedProofingFlow::STEPS.keys + end + + # IdvSession Concern overrides + + def confirm_idv_vendor_session_started + return if flash[:allow_confirmations_continue] + + redirect_to idv_inherited_proofing_url unless idv_session.proofing_started? + end + + def hybrid_session? + false + end + + # NOTE: Override and use Inherited Proofing (IP)-specific :throttle_type + # if current IDV-specific :idv_resolution type is unacceptable! + # def idv_attempter_throttled? + # ... + # end + + # IdvSession Concern > EffectiveUser Concern overrides + + def effective_user_id + current_user&.id + end + end +end diff --git a/app/controllers/idv/inherited_proofing_controller.rb b/app/controllers/idv/inherited_proofing_controller.rb index 90e9e79c9b4..0d76f6d4728 100644 --- a/app/controllers/idv/inherited_proofing_controller.rb +++ b/app/controllers/idv/inherited_proofing_controller.rb @@ -2,10 +2,9 @@ module Idv class InheritedProofingController < ApplicationController include Flow::FlowStateMachine include IdvSession + include InheritedProofing404Concern include InheritedProofingConcern - before_action :render_404_if_disabled - FLOW_STATE_MACHINE_SETTINGS = { step_url: :idv_inherited_proofing_step_url, final_url: :idv_phone_url, @@ -16,11 +15,5 @@ class InheritedProofingController < ApplicationController def return_to_sp redirect_to return_to_sp_failure_to_proof_url(step: next_step, location: params[:location]) end - - private - - def render_404_if_disabled - render_not_found unless IdentityConfig.store.inherited_proofing_enabled - end end end diff --git a/app/views/idv/inherited_proofing_cancellations/destroy.html.erb b/app/views/idv/inherited_proofing_cancellations/destroy.html.erb new file mode 100644 index 00000000000..81444e35844 --- /dev/null +++ b/app/views/idv/inherited_proofing_cancellations/destroy.html.erb @@ -0,0 +1,8 @@ +<% title t('titles.inherited_proofing.cancelled') %> + +<%= render StatusPageComponent.new(status: :error) do |c| %> + <% c.header { t('inherited_proofing.cancel.headings.confirmation.hybrid') } %> + +

<%= t('inherited_proofing.cancel.instructions.switch_back') %>

+ <%= image_tag(asset_url('inherited_proofing/switch.png'), width: 193, alt: t('inherited_proofing.cancel.instructions.switch_back_image')) %> +<% end %> diff --git a/app/views/idv/inherited_proofing_cancellations/new.html.erb b/app/views/idv/inherited_proofing_cancellations/new.html.erb new file mode 100644 index 00000000000..957673fa5b1 --- /dev/null +++ b/app/views/idv/inherited_proofing_cancellations/new.html.erb @@ -0,0 +1,55 @@ +<% title t('titles.inherited_proofing.cancellation_prompt') %> + +<%= render StatusPageComponent.new(status: :warning) do |c| %> + <% c.header { t('inherited_proofing.cancel.headings.prompt.standard') } %> + +

<%= t('inherited_proofing.cancel.headings.start_over') %>

+ +

<%= t('inherited_proofing.cancel.description.start_over') %>

+ +
+ NOTE: Render "Start Over" button here, but first have to create an IP-specific SessionController. + <%#= render ButtonComponent.new( + action: ->(**tag_options, &block) do + button_to(idv_session_path(step: @flow_step), **tag_options, &block) + end, + method: :delete, + big: true, + wide: true, + ).with_content(t('inherited_proofing.cancel.actions.start_over')) %> +
+ <%= render ButtonComponent.new( + action: ->(**tag_options, &block) do + button_to(idv_inherited_proofing_cancel_path(step: @flow_step), **tag_options, &block) + end, + method: :put, + big: true, + wide: true, + outline: true, + ).with_content(t('inherited_proofing.cancel.actions.keep_going')) %> +
+
+ +
+ +

<%= @presenter.exit_heading %>

+ + <% @presenter.exit_description.each do |p_content| %> +

<%= p_content %>

+ <% end %> + +
+ <%= render SpinnerButtonComponent.new( + action: ->(**tag_options, &block) do + button_to(idv_inherited_proofing_cancel_path(step: @flow_step, location: 'cancel'), **tag_options, &block) + end, + method: :delete, + big: true, + wide: true, + outline: true, + form: { data: { form_steps_wait: '' } }, + ).with_content(@presenter.exit_action_text) %> +
+<% end %> + +<% javascript_packs_tag_once 'form-steps-wait' %> diff --git a/config/locales/inherited_proofing/en.yml b/config/locales/inherited_proofing/en.yml index 3d969e148a0..c483acac45b 100644 --- a/config/locales/inherited_proofing/en.yml +++ b/config/locales/inherited_proofing/en.yml @@ -3,6 +3,22 @@ en: inherited_proofing: buttons: continue: Continue + cancel: + actions: + keep_going: No, keep going + start_over: Start over + description: + start_over: If you start over, you will restart this process from the beginning + and the information you entered will not be saved. + headings: + confirmation: + hybrid: You have cancelled uploading photos of your ID on this phone + prompt: + standard: Cancel verifying your identity? + start_over: Start over verifying your identity + instructions: + switch_back: Switch back to your computer to finish verifying your identity. + switch_back_image: Arrow pointing from phone to computer headings: lets_go: How verifying your identity works retrieval: We are retrieving your information from My HealtheVet diff --git a/config/locales/inherited_proofing/es.yml b/config/locales/inherited_proofing/es.yml index e7c57b0a075..f3d54c86544 100644 --- a/config/locales/inherited_proofing/es.yml +++ b/config/locales/inherited_proofing/es.yml @@ -3,6 +3,23 @@ es: inherited_proofing: buttons: continue: Continuar + cancel: + actions: + keep_going: No, continuar + start_over: Empezar de nuevo + description: + start_over: Si empieza de nuevo, el proceso se reiniciará y la información que + haya ingresado se perderá. + headings: + confirmation: + hybrid: Ha cancelado la carga de fotos de su identificación en este teléfono + prompt: + standard: ¿Cancelar la verificación de su identidad? + start_over: Empezar de nuevo a verificar su identidad + instructions: + switch_back: Regrese a su computadora para continuar con la verificación de su + identidad. + switch_back_image: Flecha que apunta del teléfono a la computadora headings: lets_go: to be implemented retrieval: Estamos recuperando su información de My HealtheVet diff --git a/config/locales/inherited_proofing/fr.yml b/config/locales/inherited_proofing/fr.yml index 37396246a21..501322985c9 100644 --- a/config/locales/inherited_proofing/fr.yml +++ b/config/locales/inherited_proofing/fr.yml @@ -3,6 +3,24 @@ fr: inherited_proofing: buttons: continue: Continuer + cancel: + actions: + keep_going: Non, continuer + start_over: Recommencer + description: + start_over: Si vous recommencez, vous reprendrez ce processus depuis le début et + les informations que vous avez saisies ne seront pas enregistrées. + headings: + confirmation: + hybrid: Vous avez annulé le téléchargement de vos photos d’identité sur ce + téléphone + prompt: + standard: Annuler la vérification de votre identité? + start_over: Recommencez la vérification de votre identité + instructions: + switch_back: Retournez sur votre ordinateur pour continuer à vérifier votre + identité. + switch_back_image: Flèche pointant du téléphone vers l’ordinateur headings: lets_go: to be implemented retrieval: Nous récupérons vos informations depuis My HealtheVet. diff --git a/config/locales/titles/en.yml b/config/locales/titles/en.yml index eea957e3cd0..8f17ae048ff 100644 --- a/config/locales/titles/en.yml +++ b/config/locales/titles/en.yml @@ -40,6 +40,9 @@ en: reset_password: Reset Password review: Re-enter your password verify_info: Verify your information + inherited_proofing: + cancellation_prompt: Cancel identity verification + cancelled: Identity verification is cancelled mfa_setup: suggest_second_mfa: You’ve added your first authentication method! Add a second method as a backup. diff --git a/config/locales/titles/es.yml b/config/locales/titles/es.yml index 6d1525981ba..243749ccf40 100644 --- a/config/locales/titles/es.yml +++ b/config/locales/titles/es.yml @@ -40,6 +40,9 @@ es: reset_password: Restablecer la contraseña review: Vuelve a ingresar tu contraseña verify_info: Verifica tu información + inherited_proofing: + cancellation_prompt: Cancela la verificación de identidad + cancelled: Se canceló la verificación de identidad mfa_setup: suggest_second_mfa: ¡Has agregado tu primer método de autenticación! Agrega un segundo método como respaldo. diff --git a/config/locales/titles/fr.yml b/config/locales/titles/fr.yml index 45f0883a802..7ae9c25f414 100644 --- a/config/locales/titles/fr.yml +++ b/config/locales/titles/fr.yml @@ -40,6 +40,9 @@ fr: reset_password: Réinitialisez le mot de passe review: Saisissez à nouveau votre mot de passe verify_info: Vérifiez vos informations + inherited_proofing: + cancellation_prompt: Annulez la vérification d’identité + cancelled: La vérification d’identité est annulée mfa_setup: suggest_second_mfa: Vous avez ajouté votre première méthode d’authentification ! Ajoutez-en une deuxième en guise de sauvegarde. diff --git a/config/routes.rb b/config/routes.rb index df14f0c88ee..58484fe6ac0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -349,6 +349,9 @@ get '/inherited_proofing/:step' => 'inherited_proofing#show', as: :inherited_proofing_step put '/inherited_proofing/:step' => 'inherited_proofing#update' get '/inherited_proofing/return_to_sp' => 'inherited_proofing#return_to_sp' + get '/inherited_proofing/cancel/' => 'inherited_proofing_cancellations#new', as: :inherited_proofing_cancel + put '/inherited_proofing/cancel' => 'inherited_proofing_cancellations#update' + delete '/inherited_proofing/cancel' => 'inherited_proofing_cancellations#destroy' # deprecated routes get '/confirmations' => 'personal_key#show' From 64149e6bd587d59ef3d9a54aca73b54a5dbb48cb Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 13 Oct 2022 10:34:08 -0400 Subject: [PATCH 02/11] Remove unused analytics events (#7142) changelog: Internal, Analytics, Remove unused analytics events --- app/services/analytics_events.rb | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 1ec5126caea..af08a97bb6e 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -812,17 +812,6 @@ def idv_personal_key_downloaded track_event('IdV: personal key downloaded') end - # @identity.idp.previous_event_name IdV: show personal key modal - # User opened IDV personal key confirmation modal - def idv_personal_key_confirm_visited - track_event('IdV: personal key confirm visited') - end - - # User submitted IDV personal key confirmation modal - def idv_personal_key_confirm_submitted - track_event('IdV: personal key confirm submitted') - end - # @param [Boolean] success # @param [Hash] errors # The user submitted their phone on the phone confirmation page From 9ab4bd4acd88574578eb5a8d1616632b1171fba2 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 13 Oct 2022 11:00:03 -0400 Subject: [PATCH 03/11] Remove JavaScript optimization from asset pipeline (#7136) changelog: Internal, Build Tooling, Remove redundant JavaScript optimization step --- config/environments/production.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/config/environments/production.rb b/config/environments/production.rb index 7f4411de810..330ac83cb27 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -11,7 +11,6 @@ # and request is nil during asset precompilation (IdentityConfig.store.asset_host.presence || IdentityConfig.store.domain_name) if request end - config.assets.js_compressor = :uglifier config.assets.compile = false config.assets.digest = true config.assets.gzip = false From 8ecca523ffee094f82efcac51290918d4c35e6ca Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 13 Oct 2022 11:00:14 -0400 Subject: [PATCH 04/11] Remove unused support for proc methods in frontend logger (#7143) changelog: Internal, Analytics, Remove unused feature support in frontend logger Last usages removed in #7110 --- app/services/frontend_logger.rb | 9 +-------- spec/services/frontend_logger_spec.rb | 12 ------------ 2 files changed, 1 insertion(+), 20 deletions(-) diff --git a/app/services/frontend_logger.rb b/app/services/frontend_logger.rb index 316dfe549f6..8c41a9f21d8 100644 --- a/app/services/frontend_logger.rb +++ b/app/services/frontend_logger.rb @@ -8,14 +8,7 @@ def initialize(analytics:, event_map:) def track_event(name, attributes) if (analytics_method = event_map[name]) - if analytics_method.is_a?(Proc) - analytics_method.call(analytics, **attributes) - else - analytics_method.bind_call( - analytics, - **hash_from_method_kwargs(attributes, analytics_method), - ) - end + analytics_method.bind_call(analytics, **hash_from_method_kwargs(attributes, analytics_method)) else analytics.track_event("Frontend: #{name}", attributes) end diff --git a/spec/services/frontend_logger_spec.rb b/spec/services/frontend_logger_spec.rb index fa4f34a799a..0ee377968a6 100644 --- a/spec/services/frontend_logger_spec.rb +++ b/spec/services/frontend_logger_spec.rb @@ -12,10 +12,8 @@ class ExampleAnalytics < FakeAnalytics end let(:analytics) { ExampleAnalytics.new } - let(:proc_handler) { proc {} } let(:event_map) do { - 'proc' => proc_handler, 'method' => ExampleAnalyticsEvents.instance_method(:example_method_handler), } end @@ -37,16 +35,6 @@ class ExampleAnalytics < FakeAnalytics end end - context 'with proc event handler' do - let(:name) { 'proc' } - - it 'calls proc with analytics instance' do - expect(proc_handler).to receive(:call).with(analytics, attributes) - - call - end - end - context 'with method handler' do let(:name) { 'method' } From 6af52cdd6677a7d872fa63cc8d67fe58d4877924 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Thu, 13 Oct 2022 10:57:49 -0500 Subject: [PATCH 05/11] Ensure all UserMailer emails do not use plaintext emails as parameters (#7106) * Ensure all UserMailer emails have matching User and EmailAddress parameters and plaintext emails are not used as parameters changelog: Internal, Email, Ensure all UserMailer emails have matching User and EmailAddress parameters and plaintext emails are not used as parameters Co-authored-by: Zach Margolis * fix mailer previews Co-authored-by: Zach Margolis --- .../accounts/personal_keys_controller.rb | 3 +- .../two_factor_authenticatable_methods.rb | 4 +- app/controllers/idv/gpo_controller.rb | 3 +- .../users/email_confirmations_controller.rb | 4 +- app/controllers/users/emails_controller.rb | 5 +- app/forms/register_user_email_form.rb | 11 +- app/jobs/get_usps_proofing_results_job.rb | 12 +- app/mailers/user_mailer.rb | 127 +++++++----- app/services/account_reset/cancel.rb | 3 +- app/services/account_reset/create_request.rb | 3 +- app/services/account_reset/delete_account.rb | 3 +- .../grant_requests_and_send_emails.rb | 3 +- .../notify_user_of_request_cancellation.rb | 3 +- .../idv/in_person/completion_survey_sender.rb | 6 +- app/services/idv/steps/upload_step.rb | 6 +- app/services/request_password_reset.rb | 4 +- app/services/reset_user_password.rb | 3 +- app/services/send_add_email_confirmation.rb | 12 +- .../send_sign_up_email_confirmation.rb | 8 +- .../alert_user_about_account_verified.rb | 4 +- .../alert_user_about_new_device.rb | 4 +- .../alert_user_about_password_change.rb | 4 +- .../alert_user_about_personal_key_sign_in.rb | 4 +- .../enrollment_helper.rb | 4 +- .../controllers/idv/review_controller_spec.rb | 19 +- .../sign_up/registrations_controller_spec.rb | 2 - .../otp_verification_controller_spec.rb | 13 +- .../users/delete_controller_spec.rb | 2 - .../users/passwords_controller_spec.rb | 15 +- .../users/phones_controller_spec.rb | 2 - .../multiple_emails/add_email_spec.rb | 118 +++++++++-- .../multiple_emails/email_management_spec.rb | 20 +- .../multiple_emails/reset_password_spec.rb | 46 +++-- spec/features/new_device_tracking_spec.rb | 44 ++-- spec/features/phone/add_phone_spec.rb | 12 +- .../sign_in_via_personal_key_spec.rb | 11 +- .../users/regenerate_personal_key_spec.rb | 14 +- spec/forms/register_user_email_form_spec.rb | 13 +- .../get_usps_proofing_results_job_spec.rb | 94 ++++----- spec/mailers/previews/user_mailer_preview.rb | 97 ++++----- .../previews/user_mailer_preview_spec.rb | 3 +- spec/mailers/report_mailer_spec.rb | 4 - spec/mailers/user_mailer_spec.rb | 189 ++++++++++++------ spec/rails_helper.rb | 2 +- spec/services/account_reset/cancel_spec.rb | 18 +- ...otify_user_of_request_cancellation_spec.rb | 25 ++- .../completion_survey_sender_spec.rb | 49 +++-- .../send_sign_up_email_confirmation_spec.rb | 52 +++-- .../alert_user_about_account_verified_spec.rb | 35 ++-- .../alert_user_about_new_device_spec.rb | 30 +-- .../alert_user_about_password_change_spec.rb | 22 +- ...rt_user_about_personal_key_sign_in_spec.rb | 22 +- .../enrollment_helper_spec.rb | 19 +- spec/support/email_delivery.rb | 9 - spec/support/features/mailer_helper.rb | 17 -- spec/support/mailer_helper.rb | 31 +++ .../layouts/user_mailer.html.erb_spec.rb | 2 +- 57 files changed, 747 insertions(+), 547 deletions(-) delete mode 100644 spec/support/email_delivery.rb delete mode 100644 spec/support/features/mailer_helper.rb create mode 100644 spec/support/mailer_helper.rb diff --git a/app/controllers/accounts/personal_keys_controller.rb b/app/controllers/accounts/personal_keys_controller.rb index 6c2f3285ce5..4591ae6f868 100644 --- a/app/controllers/accounts/personal_keys_controller.rb +++ b/app/controllers/accounts/personal_keys_controller.rb @@ -36,7 +36,8 @@ def pii_locked? # @return [FormResponse] def send_new_personal_key_notifications emails = current_user.confirmed_email_addresses.map do |email_address| - UserMailer.personal_key_regenerated(current_user, email_address.email).deliver_now_or_later + UserMailer.with(user: current_user, email_address: email_address).personal_key_regenerated. + deliver_now_or_later end telephony_responses = MfaContext.new(current_user). diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index 4d8b8bb0cab..b80401b0fb7 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -225,8 +225,8 @@ def phone_confirmed def send_phone_added_email event = create_user_event_with_disavowal(:phone_added, current_user) current_user.confirmed_email_addresses.each do |email_address| - UserMailer.phone_added(current_user, email_address, disavowal_token: event.disavowal_token). - deliver_now_or_later + UserMailer.with(user: current_user, email_address: email_address). + phone_added(disavowal_token: event.disavowal_token).deliver_now_or_later end end diff --git a/app/controllers/idv/gpo_controller.rb b/app/controllers/idv/gpo_controller.rb index 3bd309ce65b..8c49bd98f31 100644 --- a/app/controllers/idv/gpo_controller.rb +++ b/app/controllers/idv/gpo_controller.rb @@ -97,7 +97,8 @@ def confirmation_maker_perform def send_reminder current_user.confirmed_email_addresses.each do |email_address| - UserMailer.letter_reminder(current_user, email_address.email).deliver_now_or_later + UserMailer.with(user: current_user, email_address: email_address). + letter_reminder.deliver_now_or_later end end diff --git a/app/controllers/users/email_confirmations_controller.rb b/app/controllers/users/email_confirmations_controller.rb index 483248bc92a..6f1ac0b81f2 100644 --- a/app/controllers/users/email_confirmations_controller.rb +++ b/app/controllers/users/email_confirmations_controller.rb @@ -50,8 +50,8 @@ def process_successful_confirmation(email_address) def confirm_and_notify(email_address) email_address.update!(confirmed_at: Time.zone.now) email_address.user.confirmed_email_addresses.each do |confirmed_email_address| - UserMailer.email_added(email_address.user, confirmed_email_address.email). - deliver_now_or_later + UserMailer.with(user: email_address.user, email_address: confirmed_email_address). + email_added.deliver_now_or_later end notify_subscribers(email_address) end diff --git a/app/controllers/users/emails_controller.rb b/app/controllers/users/emails_controller.rb index 3fa5803d757..f50a7fc41da 100644 --- a/app/controllers/users/emails_controller.rb +++ b/app/controllers/users/emails_controller.rb @@ -104,12 +104,13 @@ def check_max_emails_per_account end def retain_confirmed_emails - @current_confirmed_emails = current_user.confirmed_email_addresses.map(&:email) + @current_confirmed_emails = current_user.confirmed_email_addresses.to_a end def send_delete_email_notification @current_confirmed_emails.each do |confirmed_email| - UserMailer.email_deleted(current_user, confirmed_email).deliver_now_or_later + UserMailer.with(user: current_user, email_address: confirmed_email). + email_deleted.deliver_now_or_later end end end diff --git a/app/forms/register_user_email_form.rb b/app/forms/register_user_email_form.rb index a375438425c..41442d18ab3 100644 --- a/app/forms/register_user_email_form.rb +++ b/app/forms/register_user_email_form.rb @@ -153,7 +153,8 @@ def send_sign_up_confirmed_email email: email, email_already_registered: true, ) else - UserMailer.signup_with_your_email(existing_user, email).deliver_now_or_later + UserMailer.with(user: existing_user, email_address: email_address_record). + signup_with_your_email.deliver_now_or_later end end @@ -161,8 +162,14 @@ def user_unconfirmed? existing_user.email_addresses.none?(&:confirmed?) end + def email_address_record + return @email_address_record if defined?(@email_address_record) + @email_address_record = EmailAddress.find_with_email(email) + @email_address_record + end + def existing_user - @existing_user ||= User.find_with_email(email) || AnonymousUser.new + @existing_user ||= email_address_record&.user || AnonymousUser.new end def email_request_id(request_id) diff --git a/app/jobs/get_usps_proofing_results_job.rb b/app/jobs/get_usps_proofing_results_job.rb index 56835044cf1..a8742e91a8d 100644 --- a/app/jobs/get_usps_proofing_results_job.rb +++ b/app/jobs/get_usps_proofing_results_job.rb @@ -263,9 +263,7 @@ def process_enrollment_response(enrollment, response) def send_verified_email(user, enrollment) user.confirmed_email_addresses.each do |email_address| # rubocop:disable IdentityIdp/MailLaterLinter - UserMailer.in_person_verified( - user, - email_address, + UserMailer.with(user: user, email_address: email_address).in_person_verified( enrollment: enrollment, ).deliver_later(**mail_delivery_params) # rubocop:enable IdentityIdp/MailLaterLinter @@ -275,9 +273,7 @@ def send_verified_email(user, enrollment) def send_failed_email(user, enrollment) user.confirmed_email_addresses.each do |email_address| # rubocop:disable IdentityIdp/MailLaterLinter - UserMailer.in_person_failed( - user, - email_address, + UserMailer.with(user: user, email_address: email_address).in_person_failed( enrollment: enrollment, ).deliver_later(**mail_delivery_params) # rubocop:enable IdentityIdp/MailLaterLinter @@ -287,9 +283,7 @@ def send_failed_email(user, enrollment) def send_failed_fraud_email(user, enrollment) user.confirmed_email_addresses.each do |email_address| # rubocop:disable IdentityIdp/MailLaterLinter - UserMailer.in_person_failed_fraud( - user, - email_address, + UserMailer.with(user: user, email_address: email_address).in_person_failed_fraud( enrollment: enrollment, ).deliver_later(**mail_delivery_params) # rubocop:enable IdentityIdp/MailLaterLinter diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 4d533bfb4cc..4261efdf800 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -1,7 +1,26 @@ +# UserMailer handles all email sending to the User class. It expects to be called using `with` +# that receives a `user` and `email_address`. This pattern is preferred as the User and +# EmailAddress database records are needed across any email being sent. +# +# Arguments sent to UserMailer must not include personally-identifiable information (PII). +# This includes email addresses. All arguments to UserMailer are stored in the database when the +# email is being sent asynchronusly by ActiveJob and we must not put PII in the database in +# plaintext. +# +# Example: +# +# UserMailer.with(user: user, email_address: email_address). +# reset_password_instructions(token: token) +# class UserMailer < ActionMailer::Base include Mailable include LocaleHelper + class UserEmailAddressMismatchError < StandardError; end + + attr_reader :user, :email_address + + before_action :validate_user_and_email_address before_action :attach_images default( from: email_with_name( @@ -14,7 +33,17 @@ class UserMailer < ActionMailer::Base ), ) - def email_confirmation_instructions(user, email, token, request_id:, instructions:) + def validate_user_and_email_address + @user = params.fetch(:user) + @email_address = params.fetch(:email_address) + if @user.id != @email_address.user_id + raise UserEmailAddressMisMatchError.new( + "User ID #{@user.id} does not match EmailAddress ID #{@email_address.id}", + ) + end + end + + def email_confirmation_instructions(token, request_id:, instructions:) with_user_locale(user) do presenter = ConfirmationEmailPresenter.new(user, view_context) @first_sentence = instructions || presenter.first_sentence @@ -22,11 +51,14 @@ def email_confirmation_instructions(user, email, token, request_id:, instruction @request_id = request_id @locale = locale_url_param @token = token - mail(to: email, subject: t('user_mailer.email_confirmation_instructions.subject')) + mail( + to: email_address.email, + subject: t('user_mailer.email_confirmation_instructions.subject'), + ) end end - def unconfirmed_email_instructions(user, email, token, request_id:, instructions:) + def unconfirmed_email_instructions(token, request_id:, instructions:) with_user_locale(user) do presenter = ConfirmationEmailPresenter.new(user, view_context) @first_sentence = instructions || presenter.first_sentence @@ -34,28 +66,31 @@ def unconfirmed_email_instructions(user, email, token, request_id:, instructions @request_id = request_id @locale = locale_url_param @token = token - mail(to: email, subject: t('user_mailer.email_confirmation_instructions.email_not_found')) + mail( + to: email_address.email, + subject: t('user_mailer.email_confirmation_instructions.email_not_found'), + ) end end - def signup_with_your_email(user, email) + def signup_with_your_email with_user_locale(user) do @root_url = root_url(locale: locale_url_param) - mail(to: email, subject: t('mailer.email_reuse_notice.subject')) + mail(to: email_address.email, subject: t('mailer.email_reuse_notice.subject')) end end - def reset_password_instructions(user, email, token:) + def reset_password_instructions(token:) with_user_locale(user) do @locale = locale_url_param @token = token @pending_profile_requires_verification = user.decorate.pending_profile_requires_verification? @hide_title = @pending_profile_requires_verification - mail(to: email, subject: t('user_mailer.reset_password_instructions.subject')) + mail(to: email_address.email, subject: t('user_mailer.reset_password_instructions.subject')) end end - def password_changed(user, email_address, disavowal_token:) + def password_changed(disavowal_token:) return unless email_should_receive_nonessential_notifications?(email_address.email) with_user_locale(user) do @@ -64,7 +99,7 @@ def password_changed(user, email_address, disavowal_token:) end end - def phone_added(user, email_address, disavowal_token:) + def phone_added(disavowal_token:) return unless email_should_receive_nonessential_notifications?(email_address.email) with_user_locale(user) do @@ -73,16 +108,16 @@ def phone_added(user, email_address, disavowal_token:) end end - def personal_key_sign_in(user, email, disavowal_token:) - return unless email_should_receive_nonessential_notifications?(email) + def personal_key_sign_in(disavowal_token:) + return unless email_should_receive_nonessential_notifications?(email_address.email) with_user_locale(user) do @disavowal_token = disavowal_token - mail(to: email, subject: t('user_mailer.personal_key_sign_in.subject')) + mail(to: email_address.email, subject: t('user_mailer.personal_key_sign_in.subject')) end end - def new_device_sign_in(user:, email_address:, date:, location:, disavowal_token:) + def new_device_sign_in(date:, location:, disavowal_token:) return unless email_should_receive_nonessential_notifications?(email_address.email) with_user_locale(user) do @@ -96,15 +131,15 @@ def new_device_sign_in(user:, email_address:, date:, location:, disavowal_token: end end - def personal_key_regenerated(user, email) - return unless email_should_receive_nonessential_notifications?(email) + def personal_key_regenerated + return unless email_should_receive_nonessential_notifications?(email_address.email) with_user_locale(user) do - mail(to: email, subject: t('user_mailer.personal_key_regenerated.subject')) + mail(to: email_address.email, subject: t('user_mailer.personal_key_regenerated.subject')) end end - def account_reset_request(user, email_address, account_reset) + def account_reset_request(account_reset) with_user_locale(user) do @token = account_reset&.request_token @header = t('user_mailer.account_reset_request.header') @@ -115,7 +150,7 @@ def account_reset_request(user, email_address, account_reset) end end - def account_reset_granted(user, email_address, account_reset) + def account_reset_granted(account_reset) with_user_locale(user) do @token = account_reset&.request_token @granted_token = account_reset&.granted_token @@ -126,44 +161,44 @@ def account_reset_granted(user, email_address, account_reset) end end - def account_reset_complete(user, email_address) + def account_reset_complete with_user_locale(user) do mail(to: email_address.email, subject: t('user_mailer.account_reset_complete.subject')) end end - def account_reset_cancel(user, email_address) + def account_reset_cancel with_user_locale(user) do mail(to: email_address.email, subject: t('user_mailer.account_reset_cancel.subject')) end end - def please_reset_password(user, email_address) + def please_reset_password with_user_locale(user) do mail( - to: email_address, + to: email_address.email, subject: t('user_mailer.please_reset_password.subject', app_name: APP_NAME), ) end end - def doc_auth_desktop_link_to_sp(user, email_address, application, link) + def doc_auth_desktop_link_to_sp(application, link) with_user_locale(user) do @link = link @application = application - mail(to: email_address, subject: t('user_mailer.doc_auth_link.subject')) + mail(to: email_address.email, subject: t('user_mailer.doc_auth_link.subject')) end end - def letter_reminder(user, email) - return unless email_should_receive_nonessential_notifications?(email) + def letter_reminder + return unless email_should_receive_nonessential_notifications?(email_address.email) with_user_locale(user) do - mail(to: email, subject: t('user_mailer.letter_reminder.subject')) + mail(to: email_address.email, subject: t('user_mailer.letter_reminder.subject')) end end - def add_email(user, email, token) + def add_email(token) with_user_locale(user) do presenter = ConfirmationEmailPresenter.new(user, view_context) @first_sentence = presenter.first_sentence @@ -172,32 +207,34 @@ def add_email(user, email, token) confirmation_token: token, locale: locale_url_param, ) - mail(to: email, subject: t('user_mailer.add_email.subject')) + mail(to: email_address.email, subject: t('user_mailer.add_email.subject')) end end - def email_added(user, email) - return unless email_should_receive_nonessential_notifications?(email) + def email_added + return unless email_should_receive_nonessential_notifications?(email_address.email) with_user_locale(user) do - mail(to: email, subject: t('user_mailer.email_added.subject')) + mail(to: email_address.email, subject: t('user_mailer.email_added.subject')) end end - def email_deleted(user, email) - return unless email_should_receive_nonessential_notifications?(email) + def email_deleted + return unless email_should_receive_nonessential_notifications?(email_address.email) with_user_locale(user) do - mail(to: email, subject: t('user_mailer.email_deleted.subject')) + mail(to: email_address.email, subject: t('user_mailer.email_deleted.subject')) end end - def add_email_associated_with_another_account(email) - @root_url = root_url(locale: locale_url_param) - mail(to: email, subject: t('mailer.email_reuse_notice.subject')) + def add_email_associated_with_another_account + with_user_locale(user) do + @root_url = root_url(locale: locale_url_param) + mail(to: email_address.email, subject: t('mailer.email_reuse_notice.subject')) + end end - def account_verified(user, email_address, date_time:, sp_name:, disavowal_token:) + def account_verified(date_time:, sp_name:, disavowal_token:) return unless email_should_receive_nonessential_notifications?(email_address.email) with_user_locale(user) do @@ -211,7 +248,7 @@ def account_verified(user, email_address, date_time:, sp_name:, disavowal_token: end end - def in_person_completion_survey(user, email_address) + def in_person_completion_survey with_user_locale(user) do @header = t('user_mailer.in_person_completion_survey.header') @privacy_url = MarketingSite.security_and_privacy_practices_url @@ -223,7 +260,7 @@ def in_person_completion_survey(user, email_address) end end - def in_person_ready_to_verify(user, email_address, enrollment:) + def in_person_ready_to_verify(enrollment:) attachments.inline['barcode.png'] = BarcodeOutputter.new( code: enrollment.enrollment_code, ).image_data @@ -241,7 +278,7 @@ def in_person_ready_to_verify(user, email_address, enrollment:) end end - def in_person_verified(user, email_address, enrollment:) + def in_person_verified(enrollment:) with_user_locale(user) do @hide_title = true @presenter = Idv::InPerson::VerificationResultsEmailPresenter.new( @@ -255,7 +292,7 @@ def in_person_verified(user, email_address, enrollment:) end end - def in_person_failed(user, email_address, enrollment:) + def in_person_failed(enrollment:) with_user_locale(user) do @presenter = Idv::InPerson::VerificationResultsEmailPresenter.new( enrollment: enrollment, @@ -268,7 +305,7 @@ def in_person_failed(user, email_address, enrollment:) end end - def in_person_failed_fraud(user, email_address, enrollment:) + def in_person_failed_fraud(enrollment:) with_user_locale(user) do @presenter = Idv::InPerson::VerificationResultsEmailPresenter.new( enrollment: enrollment, diff --git a/app/services/account_reset/cancel.rb b/app/services/account_reset/cancel.rb index 111c7858e92..a43fabae485 100644 --- a/app/services/account_reset/cancel.rb +++ b/app/services/account_reset/cancel.rb @@ -25,7 +25,8 @@ def call def notify_user_via_email_of_account_reset_cancellation user.confirmed_email_addresses.each do |email_address| - UserMailer.account_reset_cancel(user, email_address).deliver_now_or_later + UserMailer.with(user: user, email_address: email_address).account_reset_cancel. + deliver_now_or_later end end diff --git a/app/services/account_reset/create_request.rb b/app/services/account_reset/create_request.rb index a7b8f50564f..d0c9611d496 100644 --- a/app/services/account_reset/create_request.rb +++ b/app/services/account_reset/create_request.rb @@ -33,7 +33,8 @@ def create_request def notify_user_by_email(request) user.confirmed_email_addresses.each do |email_address| - UserMailer.account_reset_request(user, email_address, request).deliver_now_or_later + UserMailer.with(user: user, email_address: email_address).account_reset_request(request). + deliver_now_or_later end end diff --git a/app/services/account_reset/delete_account.rb b/app/services/account_reset/delete_account.rb index 79ceae619c1..4208adf410a 100644 --- a/app/services/account_reset/delete_account.rb +++ b/app/services/account_reset/delete_account.rb @@ -55,7 +55,8 @@ def send_push_notifications def notify_user_via_email_of_deletion user.confirmed_email_addresses.each do |email_address| - UserMailer.account_reset_complete(user, email_address).deliver_now_or_later + UserMailer.with(user: user, email_address: email_address). + account_reset_complete.deliver_now_or_later end end diff --git a/app/services/account_reset/grant_requests_and_send_emails.rb b/app/services/account_reset/grant_requests_and_send_emails.rb index f328772059c..f6e1f119570 100644 --- a/app/services/account_reset/grant_requests_and_send_emails.rb +++ b/app/services/account_reset/grant_requests_and_send_emails.rb @@ -55,7 +55,8 @@ def grant_request_and_send_email(arr) arr = arr.reload user.confirmed_email_addresses.each do |email_address| - UserMailer.account_reset_granted(user, email_address, arr).deliver_now_or_later + UserMailer.with(user: user, email_address: email_address). + account_reset_granted(arr).deliver_now_or_later end true end diff --git a/app/services/account_reset/notify_user_of_request_cancellation.rb b/app/services/account_reset/notify_user_of_request_cancellation.rb index 1f7eccee099..d570fc9b44e 100644 --- a/app/services/account_reset/notify_user_of_request_cancellation.rb +++ b/app/services/account_reset/notify_user_of_request_cancellation.rb @@ -15,7 +15,8 @@ def call def notify_user_via_email_of_account_reset_cancellation user.confirmed_email_addresses.each do |email_address| - UserMailer.account_reset_cancel(user, email_address).deliver_now_or_later + UserMailer.with(user: user, email_address: email_address).account_reset_cancel. + deliver_now_or_later end end diff --git a/app/services/idv/in_person/completion_survey_sender.rb b/app/services/idv/in_person/completion_survey_sender.rb index b84a9c3da5d..1d8551cec0a 100644 --- a/app/services/idv/in_person/completion_survey_sender.rb +++ b/app/services/idv/in_person/completion_survey_sender.rb @@ -8,10 +8,8 @@ def self.send_completion_survey(user, issuer) return unless user.should_receive_in_person_completion_survey?(issuer) user.confirmed_email_addresses.each do |email_address| - UserMailer.in_person_completion_survey( - user, - email_address, - ).deliver_now_or_later + UserMailer.with(user: user, email_address: email_address).in_person_completion_survey. + deliver_now_or_later end user.mark_in_person_completion_survey_sent(issuer) diff --git a/app/services/idv/steps/upload_step.rb b/app/services/idv/steps/upload_step.rb index 088e88d3b56..039b91aebe8 100644 --- a/app/services/idv/steps/upload_step.rb +++ b/app/services/idv/steps/upload_step.rb @@ -50,9 +50,9 @@ def application def send_user_to_email_sent_step mark_step_complete(:send_link) mark_step_complete(:link_sent) - UserMailer.doc_auth_desktop_link_to_sp( - current_user, current_user.confirmed_email_addresses.first.email, application, link - ).deliver_now_or_later + UserMailer.with( + user: current_user, email_address: current_user.confirmed_email_addresses.first, + ).doc_auth_desktop_link_to_sp(application, link).deliver_now_or_later form_response(destination: :email_sent) end diff --git a/app/services/request_password_reset.rb b/app/services/request_password_reset.rb index b97790da680..17bba0d727d 100644 --- a/app/services/request_password_reset.rb +++ b/app/services/request_password_reset.rb @@ -26,7 +26,9 @@ def send_reset_password_instructions irs_attempts_api_tracker.forgot_password_email_rate_limited(email: email) else token = user.set_reset_password_token - UserMailer.reset_password_instructions(user, email, token: token).deliver_now_or_later + UserMailer.with(user: user, email_address: email_address_record).reset_password_instructions( + token: token, + ).deliver_now_or_later event = PushNotification::RecoveryActivatedEvent.new(user: user) PushNotification::HttpPush.deliver(event) diff --git a/app/services/reset_user_password.rb b/app/services/reset_user_password.rb index 55141c976c4..d4152b4836c 100644 --- a/app/services/reset_user_password.rb +++ b/app/services/reset_user_password.rb @@ -33,7 +33,8 @@ def log_event def notify_user user.email_addresses.each do |email_address| - UserMailer.please_reset_password(user, email_address.email).deliver_now_or_later + UserMailer.with(user: user, email_address: email_address).please_reset_password. + deliver_now_or_later end end end diff --git a/app/services/send_add_email_confirmation.rb b/app/services/send_add_email_confirmation.rb index d6d10d7c06f..a263c83efc4 100644 --- a/app/services/send_add_email_confirmation.rb +++ b/app/services/send_add_email_confirmation.rb @@ -47,15 +47,15 @@ def send_email end def send_email_associated_with_another_account_email - UserMailer.add_email_associated_with_another_account( - email_address.email, - ).deliver_now_or_later + UserMailer.with( + user: user, + email_address: email_address, + ).add_email_associated_with_another_account. + deliver_now_or_later end def send_confirmation_email - UserMailer.add_email( - user, - email_address.email, + UserMailer.with(user: user, email_address: email_address).add_email( confirmation_token, ).deliver_now_or_later end diff --git a/app/services/send_sign_up_email_confirmation.rb b/app/services/send_sign_up_email_confirmation.rb index bc3aa932868..6c983d4b1c4 100644 --- a/app/services/send_sign_up_email_confirmation.rb +++ b/app/services/send_sign_up_email_confirmation.rb @@ -52,9 +52,7 @@ def update_email_address_record end def send_confirmation_email(request_id, instructions) - UserMailer.email_confirmation_instructions( - user, - email_address.email, + UserMailer.with(user: user, email_address: email_address).email_confirmation_instructions( confirmation_token, request_id: request_id, instructions: instructions, @@ -62,9 +60,7 @@ def send_confirmation_email(request_id, instructions) end def send_pw_reset_request_unconfirmed_user_email(request_id, instructions) - UserMailer.unconfirmed_email_instructions( - user, - email_address.email, + UserMailer.with(user: user, email_address: email_address).unconfirmed_email_instructions( confirmation_token, request_id: request_id, instructions: instructions, 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 9849fff94bd..4823b813da4 100644 --- a/app/services/user_alerts/alert_user_about_account_verified.rb +++ b/app/services/user_alerts/alert_user_about_account_verified.rb @@ -3,9 +3,7 @@ class AlertUserAboutAccountVerified def self.call(user:, date_time:, sp_name:, disavowal_token:) sp_name ||= APP_NAME user.confirmed_email_addresses.each do |email_address| - UserMailer.account_verified( - user, - email_address, + UserMailer.with(user: user, email_address: email_address).account_verified( date_time: date_time, sp_name: sp_name, disavowal_token: disavowal_token, diff --git a/app/services/user_alerts/alert_user_about_new_device.rb b/app/services/user_alerts/alert_user_about_new_device.rb index c5cc1b977ba..4b48e7cec63 100644 --- a/app/services/user_alerts/alert_user_about_new_device.rb +++ b/app/services/user_alerts/alert_user_about_new_device.rb @@ -3,9 +3,7 @@ class AlertUserAboutNewDevice def self.call(user, device, disavowal_token) login_location = DeviceDecorator.new(device).last_sign_in_location_and_ip user.confirmed_email_addresses.each do |email_address| - UserMailer.new_device_sign_in( - user: user, - email_address: email_address, + UserMailer.with(user: user, email_address: email_address).new_device_sign_in( date: device.last_used_at.in_time_zone('Eastern Time (US & Canada)'). strftime('%B %-d, %Y %H:%M Eastern Time'), location: login_location, diff --git a/app/services/user_alerts/alert_user_about_password_change.rb b/app/services/user_alerts/alert_user_about_password_change.rb index c35fd5bf0f4..afbf087f2d2 100644 --- a/app/services/user_alerts/alert_user_about_password_change.rb +++ b/app/services/user_alerts/alert_user_about_password_change.rb @@ -2,8 +2,8 @@ module UserAlerts class AlertUserAboutPasswordChange def self.call(user, disavowal_token) user.confirmed_email_addresses.each do |email_address| - UserMailer.password_changed(user, email_address, disavowal_token: disavowal_token). - deliver_now_or_later + UserMailer.with(user: user, email_address: email_address). + password_changed(disavowal_token: disavowal_token).deliver_now_or_later end end end diff --git a/app/services/user_alerts/alert_user_about_personal_key_sign_in.rb b/app/services/user_alerts/alert_user_about_personal_key_sign_in.rb index ad8f91e403c..4dc8d13d6d7 100644 --- a/app/services/user_alerts/alert_user_about_personal_key_sign_in.rb +++ b/app/services/user_alerts/alert_user_about_personal_key_sign_in.rb @@ -3,8 +3,8 @@ class AlertUserAboutPersonalKeySignIn # @return [FormResponse] def self.call(user, disavowal_token) emails = user.confirmed_email_addresses.map do |email_address| - UserMailer.personal_key_sign_in( - user, email_address.email, disavowal_token: disavowal_token + UserMailer.with(user: user, email_address: email_address).personal_key_sign_in( + disavowal_token: disavowal_token, ).deliver_now_or_later end telephony_responses = MfaContext.new(user).phone_configurations.map do |phone_configuration| diff --git a/app/services/usps_in_person_proofing/enrollment_helper.rb b/app/services/usps_in_person_proofing/enrollment_helper.rb index d0458756215..1699fd8742d 100644 --- a/app/services/usps_in_person_proofing/enrollment_helper.rb +++ b/app/services/usps_in_person_proofing/enrollment_helper.rb @@ -22,9 +22,7 @@ def schedule_in_person_enrollment(user, pii) def send_ready_to_verify_email(user, enrollment) user.confirmed_email_addresses.each do |email_address| - UserMailer.in_person_ready_to_verify( - user, - email_address, + UserMailer.with(user: user, email_address: email_address).in_person_ready_to_verify( enrollment: enrollment, ).deliver_now_or_later end diff --git a/spec/controllers/idv/review_controller_spec.rb b/spec/controllers/idv/review_controller_spec.rb index 31a62d1d6f6..d9dcfdd2337 100644 --- a/spec/controllers/idv/review_controller_spec.rb +++ b/spec/controllers/idv/review_controller_spec.rb @@ -435,18 +435,15 @@ def show end it 'sends ready to verify email' do - mailer = instance_double(ActionMailer::MessageDelivery, deliver_now_or_later: true) - user.email_addresses.each do |email_address| - expect(UserMailer).to receive(:in_person_ready_to_verify). - with( - user, - email_address, - enrollment: instance_of(InPersonEnrollment), - ). - and_return(mailer) - end - put :create, params: { user: { password: ControllerHelper::VALID_PASSWORD } } + + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [user.email_addresses.first.email], + subject: t('user_mailer.in_person_ready_to_verify.subject', app_name: APP_NAME), + } + ) end context 'when there is a 4xx error' do diff --git a/spec/controllers/sign_up/registrations_controller_spec.rb b/spec/controllers/sign_up/registrations_controller_spec.rb index d24ee781dd7..9a814c18a0f 100644 --- a/spec/controllers/sign_up/registrations_controller_spec.rb +++ b/spec/controllers/sign_up/registrations_controller_spec.rb @@ -1,8 +1,6 @@ require 'rails_helper' describe SignUp::RegistrationsController, devise: true do - include Features::MailerHelper - describe '#new' do it 'allows user to visit the sign up page' do get :new diff --git a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb index ef449b5477f..d1adcfbb708 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -416,11 +416,14 @@ it 'tracks the update event and notifies via email about number change' do expect(subject).to have_received(:create_user_event).with(:phone_changed) expect(subject).to have_received(:create_user_event).exactly(:once) - subject.current_user.email_addresses.each do |email_address| - expect(UserMailer).to have_received(:phone_added). - with(subject.current_user, email_address, disavowal_token: instance_of(String)) - end - expect(@mailer).to have_received(:deliver_now_or_later) + + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [subject.current_user.email_addresses.first.email], + subject: t('user_mailer.phone_added.subject'), + } + ) end end diff --git a/spec/controllers/users/delete_controller_spec.rb b/spec/controllers/users/delete_controller_spec.rb index 2bd2c465769..276ded40fed 100644 --- a/spec/controllers/users/delete_controller_spec.rb +++ b/spec/controllers/users/delete_controller_spec.rb @@ -1,8 +1,6 @@ require 'rails_helper' describe Users::DeleteController do - include Features::MailerHelper - describe '#show' do it 'shows and logs a visit' do stub_analytics diff --git a/spec/controllers/users/passwords_controller_spec.rb b/spec/controllers/users/passwords_controller_spec.rb index 4dde04e5cfa..522106775b1 100644 --- a/spec/controllers/users/passwords_controller_spec.rb +++ b/spec/controllers/users/passwords_controller_spec.rb @@ -1,8 +1,6 @@ require 'rails_helper' describe Users::PasswordsController do - include Features::MailerHelper - describe '#update' do context 'form returns success' do it 'redirects to profile and sends a password change email' do @@ -63,16 +61,19 @@ it 'sends the user an email' do user = create(:user) - mail = double - expect(mail).to receive(:deliver_now_or_later) - expect(UserMailer).to receive(:password_changed). - with(user, user.email_addresses.first, hash_including(:disavowal_token)). - and_return(mail) stub_sign_in(user) params = { password: 'salty new password' } patch :update, params: { update_user_password_form: params } + + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [user.email_addresses.first.email], + subject: t('devise.mailer.password_updated.subject'), + } + ) end end diff --git a/spec/controllers/users/phones_controller_spec.rb b/spec/controllers/users/phones_controller_spec.rb index 06af2627eef..78568811a3c 100644 --- a/spec/controllers/users/phones_controller_spec.rb +++ b/spec/controllers/users/phones_controller_spec.rb @@ -1,8 +1,6 @@ require 'rails_helper' describe Users::PhonesController do - include Features::MailerHelper - let(:user) { create(:user, :signed_up, with: { phone: '+1 (202) 555-1234' }) } before do stub_sign_in(user) diff --git a/spec/features/multiple_emails/add_email_spec.rb b/spec/features/multiple_emails/add_email_spec.rb index 471555af26f..edf3a1395d4 100644 --- a/spec/features/multiple_emails/add_email_spec.rb +++ b/spec/features/multiple_emails/add_email_spec.rb @@ -4,7 +4,6 @@ let(:email) { 'test@test.com' } it 'allows the user to add an email and confirm with an active session' do - allow(UserMailer).to receive(:email_added).and_call_original user = create(:user, :signed_up) sign_in_user_and_add_email(user) unconfirmed_email_text = "#{email} #{t('email_addresses.unconfirmed')}" @@ -17,11 +16,29 @@ expect(page).to have_current_path(account_path) expect(page).to have_content(t('devise.confirmations.confirmed')) expect(page).to_not have_content(unconfirmed_email_text) - expect(UserMailer).to have_received(:email_added).twice + + expect_delivered_email_count(3) + expect_delivered_email( + 0, { + to: [user.confirmed_email_addresses[1].email], + subject: t('user_mailer.add_email.subject'), + } + ) + expect_delivered_email( + 1, { + to: [user.confirmed_email_addresses[0].email], + subject: t('user_mailer.email_added.subject'), + } + ) + expect_delivered_email( + 2, { + to: [user.confirmed_email_addresses[1].email], + subject: t('user_mailer.email_added.subject'), + } + ) end it 'allows the user to add an email and confirm without an active session' do - allow(UserMailer).to receive(:email_added).and_call_original user = create(:user, :signed_up) sign_in_user_and_add_email(user) @@ -30,7 +47,26 @@ click_on_link_in_confirmation_email expect(page).to have_current_path(root_path) expect(page).to have_content(t('devise.confirmations.confirmed_but_sign_in')) - expect(UserMailer).to have_received(:email_added).twice + + expect_delivered_email_count(3) + expect_delivered_email( + 0, { + to: [user.confirmed_email_addresses[1].email], + subject: t('user_mailer.add_email.subject'), + } + ) + expect_delivered_email( + 1, { + to: [user.confirmed_email_addresses[0].email], + subject: t('user_mailer.email_added.subject'), + } + ) + expect_delivered_email( + 2, { + to: [user.confirmed_email_addresses[1].email], + subject: t('user_mailer.email_added.subject'), + } + ) end it 'notifies user they are already confirmed without an active session' do @@ -46,6 +82,14 @@ expect(page).to have_current_path(root_path) action = t('devise.confirmations.sign_in') expect(page).to have_content(t('devise.confirmations.already_confirmed', action: action)) + + expect_delivered_email_count(3) + expect_delivered_email( + 0, { + to: [user.confirmed_email_addresses[1].email], + subject: t('user_mailer.add_email.subject'), + } + ) end it 'notifies user they are already confirmed with an active session' do @@ -58,6 +102,14 @@ expect(page).to have_current_path(account_path) expect(page).to have_content(t('devise.confirmations.already_confirmed', action: nil).strip) + + expect_delivered_email_count(3) + expect_delivered_email( + 0, { + to: [user.confirmed_email_addresses[1].email], + subject: t('user_mailer.add_email.subject'), + } + ) end it 'notifies user they are already confirmed on another account after clicking on link' do @@ -72,10 +124,18 @@ expect(page).to have_content( t('devise.confirmations.confirmed_but_remove_from_other_account', app_name: APP_NAME), ) + + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [user.reload.email_addresses[1].email], + subject: t('user_mailer.add_email.subject'), + } + ) end it 'notifies user they are already confirmed on another account via email' do - create(:user, :signed_up, email: email) + initial_user = create(:user, :signed_up, email: email) user = create(:user, :signed_up) sign_in_user_and_add_email(user, false) @@ -83,6 +143,20 @@ expect(last_email_sent.default_part_body.to_s).to have_content( t('user_mailer.add_email_associated_with_another_account.intro_html', app_name: APP_NAME), ) + + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [initial_user.email_addresses.first.email], + subject: t('mailer.email_reuse_notice.subject'), + } + ) + expect_delivered_email( + 0, { + to: [email], + subject: t('mailer.email_reuse_notice.subject'), + } + ) end it 'routes to root with a bad confirmation token' do @@ -163,9 +237,21 @@ user = create(:user, :signed_up) sign_in_user_and_add_email(user) - expect(UserMailer).to receive(:add_email). - with(user, anything, anything).and_call_original click_button t('links.resend') + + expect_delivered_email_count(2) + expect_delivered_email( + 0, { + to: [user.reload.email_addresses[1].email], + subject: t('user_mailer.add_email.subject'), + } + ) + expect_delivered_email( + 1, { + to: [user.email_addresses[1].email], + subject: t('user_mailer.add_email.subject'), + } + ) end it 'invalidates the confirmation email/token after 24 hours' do @@ -179,6 +265,14 @@ expect(page).to have_current_path(root_path) expect(page).to_not have_content(t('devise.confirmations.confirmed_but_sign_in')) end + + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [user.reload.email_addresses[1].email], + subject: t('user_mailer.add_email.subject'), + } + ) end it 'does not raise a 500 if user submits in rapid succession violating a db constraint' do @@ -203,7 +297,7 @@ expect(page).to have_content(t('email_addresses.add.duplicate')) end - def sign_in_user_and_add_email(user, add_email = true) + def sign_in_user_and_add_email(user, _add_email = true) sign_in_and_2fa_user(user) visit account_path @@ -215,14 +309,6 @@ def sign_in_user_and_add_email(user, add_email = true) expect(page).to have_current_path(add_email_path) - if add_email - expect(UserMailer).to receive(:add_email). - with(user, anything, anything).and_call_original - else - expect(UserMailer).to receive(:add_email_associated_with_another_account). - with(email).and_call_original - end - fill_in t('forms.registration.labels.email'), with: email click_button t('forms.buttons.submit.default') diff --git a/spec/features/multiple_emails/email_management_spec.rb b/spec/features/multiple_emails/email_management_spec.rb index 654415ad52c..d390bf7c8ba 100644 --- a/spec/features/multiple_emails/email_management_spec.rb +++ b/spec/features/multiple_emails/email_management_spec.rb @@ -73,17 +73,27 @@ end it 'sends notification to all confirmed emails when email address is deleted' do - allow(UserMailer).to receive(:email_deleted).and_call_original user = create(:user, :signed_up, email: 'test@example.com ') confirmed_email1 = user.confirmed_email_addresses.first - create(:email_address, user: user, confirmed_at: Time.zone.now) - user.email_addresses.reload + confirmed_email2 = create(:email_address, user: user, confirmed_at: Time.zone.now) sign_in_and_2fa_user(user) expect(page).to have_current_path(account_path) - delete_email_should_not_fail(confirmed_email1) - expect(UserMailer).to have_received(:email_deleted).twice + + expect_delivered_email_count(2) + expect_delivered_email( + 0, { + to: [confirmed_email1.email], + subject: t('user_mailer.email_deleted.subject'), + } + ) + expect_delivered_email( + 1, { + to: [confirmed_email2.email], + subject: t('user_mailer.email_deleted.subject'), + } + ) end it 'allows a user to create an account with the old email address' do diff --git a/spec/features/multiple_emails/reset_password_spec.rb b/spec/features/multiple_emails/reset_password_spec.rb index 3b2e176f3b6..86199d17f01 100644 --- a/spec/features/multiple_emails/reset_password_spec.rb +++ b/spec/features/multiple_emails/reset_password_spec.rb @@ -5,29 +5,33 @@ user = create(:user, :with_multiple_emails) email1, email2 = user.reload.email_addresses.map(&:email) - mail1 = double - expect(mail1).to receive(:deliver_now_or_later) - expect(UserMailer).to receive(:reset_password_instructions). - with(user, email1, hash_including(:token)). - and_return(mail1) - visit root_path click_link t('links.passwords.forgot') fill_in 'Email', with: email1 click_button t('forms.buttons.continue') - Capybara.reset_session! + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [email1], + subject: t('user_mailer.reset_password_instructions.subject'), + } + ) - mail2 = double - expect(mail2).to receive(:deliver_now_or_later) - expect(UserMailer).to receive(:reset_password_instructions). - with(user, email2, hash_including(:token)). - and_return(mail2) + Capybara.reset_session! visit root_path click_link t('links.passwords.forgot') fill_in 'Email', with: email2 click_button t('forms.buttons.continue') + + expect_delivered_email_count(2) + expect_delivered_email( + 1, { + to: [email2], + subject: t('user_mailer.reset_password_instructions.subject'), + } + ) end scenario 'it sends the unconfirmed address email if the requested email is not confirmed' do @@ -40,18 +44,18 @@ app_name: APP_NAME, ) - mail = double - expect(mail).to receive(:deliver_now_or_later) - expect(UserMailer).to receive(:unconfirmed_email_instructions).with( - instance_of(User), - unconfirmed_email_address.email, - instance_of(String), - hash_including(instructions: create_account_instructions_text), - ).and_return(mail) - visit root_path click_link t('links.passwords.forgot') fill_in 'Email', with: unconfirmed_email_address.email click_button t('forms.buttons.continue') + + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [unconfirmed_email_address.email], + subject: t('user_mailer.email_confirmation_instructions.email_not_found'), + body: [create_account_instructions_text], + } + ) end end diff --git a/spec/features/new_device_tracking_spec.rb b/spec/features/new_device_tracking_spec.rb index e9de84ce863..b9f63dc3299 100644 --- a/spec/features/new_device_tracking_spec.rb +++ b/spec/features/new_device_tracking_spec.rb @@ -9,23 +9,22 @@ end it 'sends a user notification on signin' do - allow(UserMailer).to receive(:new_device_sign_in).and_call_original - sign_in_user(user) expect(user.reload.devices.length).to eq 2 device = user.devices.order(created_at: :desc).first - expect(UserMailer).to have_received(:new_device_sign_in). - with( - user: user, - email_address: user.email_addresses.first, - date: device.last_used_at.in_time_zone('Eastern Time (US & Canada)'). - strftime('%B %-d, %Y %H:%M Eastern Time'), - location: 'From 127.0.0.1 (IP address potentially located in United States)', - disavowal_token: instance_of(String), - ) + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [user.email_addresses.first.email], + subject: t('user_mailer.new_device_sign_in.subject', app_name: APP_NAME), + body: [device.last_used_at.in_time_zone('Eastern Time (US & Canada)'). + strftime('%B %-d, %Y %H:%M Eastern Time'), + 'From 127.0.0.1 (IP address potentially located in United States)'], + } + ) end end @@ -48,23 +47,22 @@ end it 'does not send an SMS' do - allow(UserMailer).to receive(:new_device_sign_in).and_call_original - sign_in_user(user) expect(user.reload.devices.length).to eq 2 device = user.devices.order(created_at: :desc).first - - expect(UserMailer).to have_received(:new_device_sign_in). - with( - user: user, - email_address: user.email_addresses.first, - date: device.last_used_at.in_time_zone('Eastern Time (US & Canada)'). - strftime('%B %-d, %Y %H:%M Eastern Time'), - location: 'From 127.0.0.1 (IP address potentially located in United States)', - disavowal_token: instance_of(String), - ) + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [user.email_addresses.first.email], + subject: t('user_mailer.new_device_sign_in.subject', app_name: APP_NAME), + body: [device.last_used_at.in_time_zone('Eastern Time (US & Canada)'). + strftime('%B %-d, %Y %H:%M Eastern Time'), + 'From 127.0.0.1 (IP address potentially located in United States)'], + } + ) + expect(Telephony::Test::Message.messages.count).to eq 0 end end end diff --git a/spec/features/phone/add_phone_spec.rb b/spec/features/phone/add_phone_spec.rb index 316633cd648..112213fa8e8 100644 --- a/spec/features/phone/add_phone_spec.rb +++ b/spec/features/phone/add_phone_spec.rb @@ -25,10 +25,6 @@ user = create(:user, :signed_up) phone = '+1 (225) 278-1234' - expect(UserMailer).to receive(:phone_added). - with(user, user.email_addresses.first, hash_including(:disavowal_token)). - and_call_original - sign_in_and_2fa_user(user) within('.sidenav') do click_on t('account.navigation.add_phone_number') @@ -37,6 +33,14 @@ click_continue fill_in_code_with_last_phone_otp click_submit_default + + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [user.email_addresses.first.email], + subject: t('user_mailer.phone_added.subject'), + } + ) end scenario 'adding a new phone number validates number', js: true do diff --git a/spec/features/two_factor_authentication/sign_in_via_personal_key_spec.rb b/spec/features/two_factor_authentication/sign_in_via_personal_key_spec.rb index 2d72030c7c2..020d1150f03 100644 --- a/spec/features/two_factor_authentication/sign_in_via_personal_key_spec.rb +++ b/spec/features/two_factor_authentication/sign_in_via_personal_key_spec.rb @@ -9,9 +9,6 @@ raw_key = PersonalKeyGenerator.new(user).create old_key = user.reload.encrypted_recovery_code_digest - personal_key_sign_in_mail = double - expect(personal_key_sign_in_mail).to receive(:deliver_now_or_later) - expect(UserMailer).to receive(:personal_key_sign_in).and_return(personal_key_sign_in_mail) expect(Telephony).to receive(:send_personal_key_sign_in_notice). with(to: '+1 (202) 345-6789', country_code: 'US') @@ -21,6 +18,14 @@ click_submit_default expect(user.reload.encrypted_recovery_code_digest).to_not eq old_key expect(current_path).to eq account_path + + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [user.email_addresses.first.email], + subject: t('user_mailer.personal_key_sign_in.subject'), + } + ) end context 'user enters incorrect personal key' do diff --git a/spec/features/users/regenerate_personal_key_spec.rb b/spec/features/users/regenerate_personal_key_spec.rb index 5a595988db2..4ae3df12d96 100644 --- a/spec/features/users/regenerate_personal_key_spec.rb +++ b/spec/features/users/regenerate_personal_key_spec.rb @@ -13,12 +13,6 @@ sign_in_and_2fa_user(user) old_digest = user.encrypted_recovery_code_digest - # The user should receive an SMS and an email - personal_key_sign_in_mail = double - expect(personal_key_sign_in_mail).to receive(:deliver_now_or_later) - expect(UserMailer).to receive(:personal_key_regenerated). - with(user, user.email). - and_return(personal_key_sign_in_mail) expect(Telephony).to receive(:send_personal_key_regeneration_notice). with(to: user.phone_configurations.first.phone, country_code: 'US') @@ -27,6 +21,14 @@ click_continue expect(user.reload.encrypted_recovery_code_digest).to_not eq old_digest + + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [user.email_addresses.first.email], + subject: t('user_mailer.personal_key_regenerated.subject'), + } + ) end end diff --git a/spec/forms/register_user_email_form_spec.rb b/spec/forms/register_user_email_form_spec.rb index 90dd2a545c4..9fc40a7ae8c 100644 --- a/spec/forms/register_user_email_form_spec.rb +++ b/spec/forms/register_user_email_form_spec.rb @@ -12,11 +12,6 @@ it 'sets success to true to prevent revealing account existence' do existing_user = create(:user, :signed_up, email: 'taken@gmail.com') - mailer = instance_double(ActionMailer::MessageDelivery) - allow(UserMailer).to receive(:signup_with_your_email). - with(existing_user, existing_user.email).and_return(mailer) - allow(mailer).to receive(:deliver_now_or_later) - extra = { email_already_exists: true, throttled: false, @@ -30,7 +25,13 @@ **extra, ) expect(subject.email).to eq 'taken@gmail.com' - expect(mailer).to have_received(:deliver_now_or_later) + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [subject.email], + subject: t('mailer.email_reuse_notice.subject'), + } + ) end it 'creates throttle events after reaching throttle limit' do diff --git a/spec/jobs/get_usps_proofing_results_job_spec.rb b/spec/jobs/get_usps_proofing_results_job_spec.rb index 0cc4aa41af9..49a497732ee 100644 --- a/spec/jobs/get_usps_proofing_results_job_spec.rb +++ b/spec/jobs/get_usps_proofing_results_job_spec.rb @@ -116,6 +116,7 @@ let(:job_analytics) { FakeAnalytics.new } before do + ActiveJob::Base.queue_adapter = :test allow(job).to receive(:analytics).and_return(job_analytics) allow(IdentityConfig.store).to receive(:get_usps_proofing_results_job_reprocess_delay_minutes). and_return(reprocess_delay_minutes) @@ -293,61 +294,46 @@ it 'sends proofing failed email on response with failed status' do stub_request_failed_proofing_results - mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) user = pending_enrollment.user - user.email_addresses.each do |email_address| - # it sends with the default delay - expect(mailer).to receive(:deliver_later).with(wait: 1.hour) - expect(UserMailer).to receive(:in_person_failed). - with( - user, - email_address, - enrollment: instance_of(InPersonEnrollment), - ). - and_return(mailer) - end - job.perform(Time.zone.now) + freeze_time do + expect do + job.perform(Time.zone.now) + end.to have_enqueued_mail(UserMailer, :in_person_failed).with( + params: { user: user, email_address: user.email_addresses.first }, + args: [{ enrollment: pending_enrollment }], + ).at(Time.zone.now + 1.hour) + end end it 'sends failed email when fraudSuspected is true' do stub_request_failed_suspected_fraud_proofing_results - mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) user = pending_enrollment.user - user.email_addresses.each do |email_address| - # it sends with the default delay - expect(mailer).to receive(:deliver_later).with(wait: 1.hour) - expect(UserMailer).to receive(:in_person_failed_fraud). - with( - user, - email_address, - enrollment: instance_of(InPersonEnrollment), - ). - and_return(mailer) - end - job.perform(Time.zone.now) + freeze_time do + expect do + job.perform(Time.zone.now) + end.to have_enqueued_mail(UserMailer, :in_person_failed_fraud).with( + params: { user: user, email_address: user.email_addresses.first }, + args: [{ enrollment: pending_enrollment }], + ).at(Time.zone.now + 1.hour) + end end it 'sends proofing verifed email on 2xx responses with valid JSON' do stub_request_passed_proofing_results - mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) user = pending_enrollment.user - user.email_addresses.each do |email_address| - # it sends with the default delay - expect(mailer).to receive(:deliver_later).with(wait: 1.hour) - expect(UserMailer).to receive(:in_person_verified). - with( - user, - email_address, - enrollment: instance_of(InPersonEnrollment), - ). - and_return(mailer) - end - job.perform(Time.zone.now) + freeze_time do + expect do + job.perform(Time.zone.now) + end.to have_enqueued_mail(UserMailer, :in_person_verified).with( + params: { user: user, email_address: user.email_addresses.first }, + args: [{ enrollment: pending_enrollment }], + ).at(Time.zone.now + 1.hour) + end end context 'a custom delay greater than zero is set' do @@ -356,15 +342,16 @@ allow(IdentityConfig.store). to(receive(:in_person_results_delay_in_hours).and_return(5)) - - mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) user = pending_enrollment.user - user.email_addresses.each do |email_address| - expect(mailer).to receive(:deliver_later).with(wait: 5.hours) - expect(UserMailer).to receive(:in_person_verified).and_return(mailer) - end - job.perform(Time.zone.now) + freeze_time do + expect do + job.perform(Time.zone.now) + end.to have_enqueued_mail(UserMailer, :in_person_verified).with( + params: { user: user, email_address: user.email_addresses.first }, + args: [{ enrollment: pending_enrollment }], + ).at(Time.zone.now + 5.hours) + end end end @@ -374,15 +361,16 @@ allow(IdentityConfig.store). to(receive(:in_person_results_delay_in_hours).and_return(0)) - - mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) user = pending_enrollment.user - user.email_addresses.each do |email_address| - expect(mailer).to receive(:deliver_later).with(no_args) - expect(UserMailer).to receive(:in_person_verified).and_return(mailer) - end - job.perform(Time.zone.now) + freeze_time do + expect do + job.perform(Time.zone.now) + end.to have_enqueued_mail(UserMailer, :in_person_verified).with( + params: { user: user, email_address: user.email_addresses.first }, + args: [{ enrollment: pending_enrollment }], + ) + end end end end diff --git a/spec/mailers/previews/user_mailer_preview.rb b/spec/mailers/previews/user_mailer_preview.rb index 702974eb2a7..3fe82dbb0f6 100644 --- a/spec/mailers/previews/user_mailer_preview.rb +++ b/spec/mailers/previews/user_mailer_preview.rb @@ -1,21 +1,18 @@ class UserMailerPreview < ActionMailer::Preview def email_confirmation_instructions - UserMailer.email_confirmation_instructions( - user, - email_address, - SecureRandom.hex, - request_id: SecureRandom.uuid, - instructions: I18n.t( - 'user_mailer.email_confirmation_instructions.first_sentence.forgot_password', - app_name: APP_NAME, - ), - ) + UserMailer.with(user: user, email_address: email_address_record). + email_confirmation_instructions( + SecureRandom.hex, + request_id: SecureRandom.uuid, + instructions: I18n.t( + 'user_mailer.email_confirmation_instructions.first_sentence.forgot_password', + app_name: APP_NAME, + ), + ) end def unconfirmed_email_instructions - UserMailer.unconfirmed_email_instructions( - user, - email_address, + UserMailer.with(user: user, email_address: email_address_record).unconfirmed_email_instructions( SecureRandom.hex, request_id: SecureRandom.uuid, instructions: I18n.t( @@ -26,29 +23,32 @@ def unconfirmed_email_instructions end def signup_with_your_email - UserMailer.signup_with_your_email(user, email_address) + UserMailer.with(user: user, email_address: email_address_record).signup_with_your_email end def reset_password_instructions - UserMailer.reset_password_instructions(user, email_address, token: SecureRandom.hex) + UserMailer.with(user: user, email_address: email_address_record).reset_password_instructions( + token: SecureRandom.hex, + ) end def password_changed - UserMailer.password_changed(user, email_address_record, disavowal_token: SecureRandom.hex) + UserMailer.with(user: user, email_address: email_address_record). + password_changed(disavowal_token: SecureRandom.hex) end def phone_added - UserMailer.phone_added(user, email_address_record, disavowal_token: SecureRandom.hex) + UserMailer.with(user: user, email_address: email_address_record). + phone_added(disavowal_token: SecureRandom.hex) end def personal_key_sign_in - UserMailer.personal_key_sign_in(user, email_address, disavowal_token: SecureRandom.hex) + UserMailer.with(user: user, email_address: email_address_record). + personal_key_sign_in(disavowal_token: SecureRandom.hex) end def new_device_sign_in - UserMailer.new_device_sign_in( - user: user, - email_address: email_address_record, + UserMailer.with(user: user, email_address: email_address_record).new_device_sign_in( date: 'February 25, 2019 15:02', location: 'Washington, DC', disavowal_token: SecureRandom.hex, @@ -56,61 +56,61 @@ def new_device_sign_in end def personal_key_regenerated - UserMailer.personal_key_regenerated(user, email_address) + UserMailer.with(user: user, email_address: email_address_record).personal_key_regenerated end def account_reset_request - UserMailer.account_reset_request( - user, email_address_record, user.build_account_reset_request + UserMailer.with(user: user, email_address: email_address_record).account_reset_request( + user.build_account_reset_request, ) end def account_reset_granted - UserMailer.account_reset_granted( - user, email_address_record, user.build_account_reset_request + UserMailer.with(user: user, email_address: email_address_record).account_reset_granted( + user.build_account_reset_request, ) end def account_reset_complete - UserMailer.account_reset_complete(user, email_address_record) + UserMailer.with(user: user, email_address: email_address_record).account_reset_complete end def account_reset_cancel - UserMailer.account_reset_cancel(user, email_address_record) + UserMailer.with(user: user, email_address: email_address_record).account_reset_cancel end def please_reset_password - UserMailer.please_reset_password(user, email_address) + UserMailer.with(user: user, email_address: email_address_record).please_reset_password end def doc_auth_desktop_link_to_sp - UserMailer.doc_auth_desktop_link_to_sp(user, email_address, 'Example App', '/') + UserMailer.with(user: user, email_address: email_address_record). + doc_auth_desktop_link_to_sp('Example App', '/') end def letter_reminder - UserMailer.letter_reminder(user, email_address) + UserMailer.with(user: user, email_address: email_address_record).letter_reminder end def add_email - UserMailer.add_email(user, email_address, SecureRandom.hex) + UserMailer.with(user: user, email_address: email_address_record).add_email(SecureRandom.hex) end def email_added - UserMailer.email_added(user, email_address) + UserMailer.with(user: user, email_address: email_address_record).email_added end def email_deleted - UserMailer.email_deleted(user, email_address) + UserMailer.with(user: user, email_address: email_address_record).email_deleted end def add_email_associated_with_another_account - UserMailer.add_email_associated_with_another_account(email_address) + UserMailer.with(user: user, email_address: email_address_record). + add_email_associated_with_another_account end def account_verified - UserMailer.account_verified( - user, - email_address_record, + UserMailer.with(user: user, email_address: email_address_record).account_verified( date_time: DateTime.now, sp_name: 'Example App', disavowal_token: SecureRandom.hex, @@ -118,40 +118,29 @@ def account_verified end def in_person_completion_survey - UserMailer.in_person_completion_survey( - user, - email_address_record, - ) + UserMailer.with(user: user, email_address: email_address_record).in_person_completion_survey end def in_person_ready_to_verify - UserMailer.in_person_ready_to_verify( - user, - email_address_record, + UserMailer.with(user: user, email_address: email_address_record).in_person_ready_to_verify( enrollment: in_person_enrollment, ) end def in_person_verified - UserMailer.in_person_verified( - user, - email_address_record, + UserMailer.with(user: user, email_address: email_address_record).in_person_verified( enrollment: in_person_enrollment, ) end def in_person_failed - UserMailer.in_person_failed( - user, - email_address_record, + UserMailer.with(user: user, email_address: email_address_record).in_person_failed( enrollment: in_person_enrollment, ) end def in_person_failed_fraud - UserMailer.in_person_failed_fraud( - user, - email_address_record, + UserMailer.with(user: user, email_address: email_address_record).in_person_failed_fraud( enrollment: in_person_enrollment, ) end diff --git a/spec/mailers/previews/user_mailer_preview_spec.rb b/spec/mailers/previews/user_mailer_preview_spec.rb index cd7479b8612..5c6ac3ed4be 100644 --- a/spec/mailers/previews/user_mailer_preview_spec.rb +++ b/spec/mailers/previews/user_mailer_preview_spec.rb @@ -13,7 +13,8 @@ it 'has a preview method for each mailer method' do mailer_methods = UserMailer.instance_methods(false) preview_methods = UserMailerPreview.instance_methods(false) - expect(mailer_methods - preview_methods).to be_empty + mailer_helper_methods = [:email_address, :user, :validate_user_and_email_address] + expect(mailer_methods - mailer_helper_methods - preview_methods).to be_empty end it 'uses user and email records that cannot be saved' do diff --git a/spec/mailers/report_mailer_spec.rb b/spec/mailers/report_mailer_spec.rb index 7d437fd327f..4a74eec27db 100644 --- a/spec/mailers/report_mailer_spec.rb +++ b/spec/mailers/report_mailer_spec.rb @@ -49,8 +49,4 @@ expect(mail.html_part.body).to have_content('issuer2') end end - - def strip_tags(str) - ActionController::Base.helpers.strip_tags(str) - end end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 9db9a85694b..5cb93fbfa3c 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -4,10 +4,11 @@ let(:user) { build(:user) } let(:email_address) { user.email_addresses.first } let(:banned_email) { 'banned_email+123abc@gmail.com' } + let(:banned_email_address) { create(:email_address, email: banned_email, user: user) } describe '#add_email' do let(:token) { SecureRandom.hex } - let(:mail) { UserMailer.add_email(user, email_address, token) } + let(:mail) { UserMailer.with(user: user, email_address: email_address).add_email(token) } it_behaves_like 'a system email' it_behaves_like 'an email that respects user email locale preference' @@ -21,13 +22,15 @@ end describe '#email_deleted' do - let(:mail) { UserMailer.email_deleted(user, 'old@email.com') } + let(:mail) do + UserMailer.with(user: user, email_address: email_address).email_deleted + end it_behaves_like 'a system email' it_behaves_like 'an email that respects user email locale preference' it 'sends to the old email' do - expect(mail.to).to eq ['old@email.com'] + expect(mail.to).to eq [email_address.email] end it 'renders the subject' do @@ -40,15 +43,37 @@ ) expect_email_body_to_have_help_and_contact_links end + end - it 'does not send mail to emails in nonessential email banlist' do - mail = UserMailer.email_deleted(user, banned_email) - expect(mail.to).to eq(nil) + describe '#add_email_associated_with_another_account' do + let(:mail) do + UserMailer.with(user: user, email_address: email_address). + add_email_associated_with_another_account + end + + it_behaves_like 'a system email' + it_behaves_like 'an email that respects user email locale preference' + + it 'sends to the specified email' do + expect(mail.to).to eq [email_address.email] + end + + it 'renders the subject' do + expect(mail.subject).to eq t('mailer.email_reuse_notice.subject') + end + + it 'renders the body' do + expect_email_body_to_have_help_and_contact_links end end describe '#password_changed' do - let(:mail) { UserMailer.password_changed(user, email_address, disavowal_token: '123abc') } + let(:mail) do + UserMailer.with( + user: user, + email_address: email_address, + ).password_changed(disavowal_token: '123abc') + end it_behaves_like 'a system email' it_behaves_like 'an email that respects user email locale preference' @@ -72,14 +97,17 @@ end it 'does not send mail to emails in nonessential email banlist' do - email_address = EmailAddress.new(email: banned_email) - mail = UserMailer.password_changed(user, email_address, disavowal_token: '123abc') + mail = UserMailer.with(user: user, email_address: banned_email_address). + password_changed(disavowal_token: '123abc') expect(mail.to).to eq(nil) end end describe '#personal_key_sign_in' do - let(:mail) { UserMailer.personal_key_sign_in(user, user.email, disavowal_token: 'asdf1234') } + let(:mail) do + UserMailer.with(user: user, email_address: user.email_addresses.first). + personal_key_sign_in(disavowal_token: 'asdf1234') + end it_behaves_like 'a system email' it_behaves_like 'an email that respects user email locale preference' @@ -102,7 +130,9 @@ end it 'does not send mail to emails in nonessential email banlist' do - mail = UserMailer.personal_key_sign_in(user, banned_email, disavowal_token: 'asdf1234') + mail = UserMailer.with(user: user, email_address: banned_email_address). + personal_key_sign_in(disavowal_token: 'asdf1234') + expect(mail.to).to eq(nil) end end @@ -113,13 +143,12 @@ let(:token) { 'asdf123' } let(:mail) do - UserMailer.email_confirmation_instructions( - user, - user.email, - token, - request_id: request_id, - instructions: instructions, - ) + UserMailer.with(user: user, email_address: user.email_addresses.first). + email_confirmation_instructions( + token, + request_id: request_id, + instructions: instructions, + ) end it_behaves_like 'a system email' @@ -131,9 +160,7 @@ location = 'Washington, DC' disavowal_token = 'asdf1234' let(:mail) do - UserMailer.new_device_sign_in( - user: user, - email_address: email_address, + UserMailer.with(user: user, email_address: email_address).new_device_sign_in( date: date, location: location, disavowal_token: disavowal_token, @@ -168,9 +195,7 @@ it 'does not send mail to emails in nonessential email banlist' do email_address = EmailAddress.new(email: banned_email) - mail = UserMailer.new_device_sign_in( - user: user, - email_address: email_address, + mail = UserMailer.with(user: user, email_address: email_address).new_device_sign_in( date: date, location: location, disavowal_token: disavowal_token, @@ -180,7 +205,9 @@ end describe '#personal_key_regenerated' do - let(:mail) { UserMailer.personal_key_regenerated(user, user.email) } + let(:mail) do + UserMailer.with(user: user, email_address: email_address).personal_key_regenerated + end it_behaves_like 'a system email' it_behaves_like 'an email that respects user email locale preference' @@ -200,13 +227,16 @@ end it 'does not send mail to emails in nonessential email banlist' do - mail = UserMailer.personal_key_regenerated(user, banned_email) + mail = UserMailer.with(user: user, email_address: banned_email_address). + personal_key_regenerated expect(mail.to).to eq(nil) end end describe '#signup_with_your_email' do - let(:mail) { UserMailer.signup_with_your_email(user, user.email) } + let(:mail) do + UserMailer.with(user: user, email_address: user.email_addresses.first).signup_with_your_email + end it_behaves_like 'a system email' it_behaves_like 'an email that respects user email locale preference' @@ -240,7 +270,10 @@ describe '#phone_added' do disavowal_token = 'i_am_disavowal_token' - let(:mail) { UserMailer.phone_added(user, email_address, disavowal_token: disavowal_token) } + let(:mail) do + UserMailer.with(user: user, email_address: email_address). + phone_added(disavowal_token: disavowal_token) + end it_behaves_like 'a system email' it_behaves_like 'an email that respects user email locale preference' @@ -260,8 +293,8 @@ end it 'does not send mail to emails in nonessential email banlist' do - email_address = EmailAddress.new(email: banned_email) - mail = UserMailer.phone_added(user, email_address, disavowal_token: disavowal_token) + mail = UserMailer.with(user: user, email_address: banned_email_address). + phone_added(disavowal_token: disavowal_token) expect(mail.to).to eq(nil) end end @@ -276,7 +309,10 @@ def expect_email_body_to_have_help_and_contact_links end describe '#account_reset_request' do - let(:mail) { UserMailer.account_reset_request(user, email_address, account_reset) } + let(:mail) do + UserMailer.with(user: user, email_address: email_address).account_reset_request(account_reset) + end + let(:account_reset) { user.account_reset_request } it_behaves_like 'a system email' @@ -316,7 +352,10 @@ def expect_email_body_to_have_help_and_contact_links end describe '#account_reset_granted' do - let(:mail) { UserMailer.account_reset_granted(user, email_address, user.account_reset_request) } + let(:mail) do + UserMailer.with(user: user, email_address: email_address). + account_reset_granted(user.account_reset_request) + end it_behaves_like 'a system email' it_behaves_like 'an email that respects user email locale preference' @@ -338,7 +377,10 @@ def expect_email_body_to_have_help_and_contact_links end describe '#account_reset_complete' do - let(:mail) { UserMailer.account_reset_complete(user, email_address) } + let(:mail) do + UserMailer.with(user: user, email_address: email_address). + account_reset_complete + end it_behaves_like 'a system email' it_behaves_like 'an email that respects user email locale preference' @@ -359,8 +401,33 @@ def expect_email_body_to_have_help_and_contact_links end end + describe '#account_reset_cancel' do + let(:mail) do + UserMailer.with(user: user, email_address: email_address). + account_reset_cancel + end + + it_behaves_like 'a system email' + it_behaves_like 'an email that respects user email locale preference' + + it 'sends to the current email' do + expect(mail.to).to eq [email_address.email] + end + + it 'renders the subject' do + expect(mail.subject).to eq t('user_mailer.account_reset_cancel.subject') + end + + it 'renders the body' do + expect(mail.html_part.body). + to have_content( + strip_tags(t('user_mailer.account_reset_cancel.intro_html', app_name: APP_NAME)), + ) + end + end + describe '#please_reset_password' do - let(:mail) { UserMailer.please_reset_password(user, email_address.email) } + let(:mail) { UserMailer.with(user: user, email_address: email_address).please_reset_password } it_behaves_like 'a system email' it_behaves_like 'an email that respects user email locale preference' @@ -387,7 +454,10 @@ def expect_email_body_to_have_help_and_contact_links describe '#doc_auth_desktop_link_to_sp' do let(:app) { 'login.gov' } let(:link) { root_url } - let(:mail) { UserMailer.doc_auth_desktop_link_to_sp(user, email_address.email, app, link) } + let(:mail) do + UserMailer.with(user: user, email_address: email_address). + doc_auth_desktop_link_to_sp(app, link) + end it_behaves_like 'a system email' it_behaves_like 'an email that respects user email locale preference' @@ -409,7 +479,7 @@ def expect_email_body_to_have_help_and_contact_links end describe '#letter_reminder' do - let(:mail) { UserMailer.letter_reminder(user, email_address.email) } + let(:mail) { UserMailer.with(user: user, email_address: email_address).letter_reminder } it_behaves_like 'a system email' it_behaves_like 'an email that respects user email locale preference' @@ -428,7 +498,7 @@ def expect_email_body_to_have_help_and_contact_links end it 'does not send mail to emails in nonessential email banlist' do - mail = UserMailer.letter_reminder(user, banned_email) + mail = UserMailer.with(user: user, email_address: banned_email_address).letter_reminder expect(mail.to).to eq(nil) end end @@ -438,10 +508,11 @@ def expect_email_body_to_have_help_and_contact_links let(:sp_name) { '' } let(:date_time) { Time.zone.now } let(:mail) do - UserMailer.account_verified( - user, email_address, date_time: date_time, sp_name: sp_name, - disavowal_token: disavowal_token - ) + UserMailer.with(user: user, email_address: email_address). + account_verified( + date_time: date_time, sp_name: sp_name, + disavowal_token: disavowal_token + ) end it_behaves_like 'a system email' @@ -456,11 +527,12 @@ def expect_email_body_to_have_help_and_contact_links end it 'does not send mail to emails in nonessential email banlist' do - email_address = EmailAddress.new(email: banned_email) - mail = UserMailer.account_verified( - user, email_address, date_time: date_time, sp_name: sp_name, - disavowal_token: disavowal_token - ) + mail = UserMailer.with(user: user, email_address: banned_email_address). + account_verified( + date_time: date_time, + sp_name: sp_name, + disavowal_token: disavowal_token, + ) expect(mail.to).to eq(nil) end end @@ -476,9 +548,7 @@ def expect_email_body_to_have_help_and_contact_links end let(:mail) do - UserMailer.in_person_ready_to_verify( - user, - user.email_addresses.first, + UserMailer.with(user: user, email_address: email_address).in_person_ready_to_verify( enrollment: enrollment, ) end @@ -497,9 +567,7 @@ def expect_email_body_to_have_help_and_contact_links end let(:mail) do - UserMailer.in_person_verified( - user, - user.email_addresses.first, + UserMailer.with(user: user, email_address: email_address).in_person_verified( enrollment: enrollment, ) end @@ -518,9 +586,7 @@ def expect_email_body_to_have_help_and_contact_links end let(:mail) do - UserMailer.in_person_failed( - user, - user.email_addresses.first, + UserMailer.with(user: user, email_address: email_address).in_person_failed( enrollment: enrollment, ) end @@ -539,9 +605,7 @@ def expect_email_body_to_have_help_and_contact_links end let(:mail) do - UserMailer.in_person_failed_fraud( - user, - user.email_addresses.first, + UserMailer.with(user: user, email_address: email_address).in_person_failed_fraud( enrollment: enrollment, ) end @@ -552,10 +616,7 @@ def expect_email_body_to_have_help_and_contact_links describe '#in_person_completion_survey' do let(:mail) do - UserMailer.in_person_completion_survey( - user, - email_address, - ) + UserMailer.with(user: user, email_address: email_address).in_person_completion_survey end it_behaves_like 'a system email' @@ -590,8 +651,4 @@ def expect_email_body_to_have_help_and_contact_links ) end end - - def strip_tags(str) - ActionController::Base.helpers.strip_tags(str) - end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 9f8b72a9593..75d77ab663d 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -38,7 +38,7 @@ config.include EmailSpec::Helpers config.include EmailSpec::Matchers config.include AbstractController::Translation - config.include Features::MailerHelper, type: :feature + config.include MailerHelper config.include Features::SessionHelper, type: :feature config.include Features::StripTagsHelper, type: :feature config.include ViewComponent::TestHelpers, type: :component diff --git a/spec/services/account_reset/cancel_spec.rb b/spec/services/account_reset/cancel_spec.rb index 6ddc50ad227..c4f21aca4ad 100644 --- a/spec/services/account_reset/cancel_spec.rb +++ b/spec/services/account_reset/cancel_spec.rb @@ -56,14 +56,15 @@ it 'notifies the user via email of the account reset cancellation' do token = create_account_reset_request_for(user) - - @mailer = instance_double(ActionMailer::MessageDelivery, deliver_now_or_later: true) - user.email_addresses.each do |email_address| - expect(UserMailer).to receive(:account_reset_cancel).with(user, email_address). - and_return(@mailer) - end - AccountReset::Cancel.new(token).call + + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [user.email_addresses.first.email], + subject: t('user_mailer.account_reset_cancel.subject'), + } + ) end it 'updates the account_reset_request' do @@ -100,9 +101,8 @@ end it 'does not notify the user via email of the account reset cancellation' do - expect(UserMailer).to_not receive(:account_reset_cancel) - AccountReset::Cancel.new('foo').call + expect_delivered_email_count(0) end it 'does not update the account_reset_request' do diff --git a/spec/services/account_reset/notify_user_of_request_cancellation_spec.rb b/spec/services/account_reset/notify_user_of_request_cancellation_spec.rb index 9b4f4600975..249f673e6fa 100644 --- a/spec/services/account_reset/notify_user_of_request_cancellation_spec.rb +++ b/spec/services/account_reset/notify_user_of_request_cancellation_spec.rb @@ -10,18 +10,21 @@ email_address1 = user.email_addresses.first email_address2 = create(:email_address, user: user) - mail1 = double - mail2 = double - - expect(UserMailer).to receive(:account_reset_cancel). - with(user, email_address1).and_return(mail1) - expect(UserMailer).to receive(:account_reset_cancel). - with(user, email_address2).and_return(mail2) - - expect(mail1).to receive(:deliver_now_or_later) - expect(mail2).to receive(:deliver_now_or_later) - subject.call + + expect_delivered_email_count(2) + expect_delivered_email( + 0, { + to: [email_address1.email], + subject: t('user_mailer.account_reset_cancel.subject'), + } + ) + expect_delivered_email( + 1, { + to: [email_address2.email], + subject: t('user_mailer.account_reset_cancel.subject'), + } + ) end it 'sends a text to all of the user phone numbers' do diff --git a/spec/services/idv/in_person/completion_survey_sender_spec.rb b/spec/services/idv/in_person/completion_survey_sender_spec.rb index 323d8a42c43..6a45b37a33b 100644 --- a/spec/services/idv/in_person/completion_survey_sender_spec.rb +++ b/spec/services/idv/in_person/completion_survey_sender_spec.rb @@ -2,50 +2,47 @@ describe Idv::InPerson::CompletionSurveySender do describe '.send_completion_survey' do - let(:user) { instance_double(User) } + let(:user) { create(:user) } let(:issuer) { 'test_issuer' } it 'does nothing if the user should not receive a survey' do - expect(UserMailer).to_not receive(:in_person_completion_survey) allow(user).to receive(:should_receive_in_person_completion_survey?). with(issuer).and_return(false) described_class.send_completion_survey(user, issuer) + expect_delivered_email_count(0) end context 'user should receive a survey' do - let(:message) { instance_double(ActionMailer::MessageDelivery) } - let(:message2) { instance_double(ActionMailer::MessageDelivery) } - let(:email_address_one) { 'hello@world.com' } - let(:email_address_two) { 'hola@mundo.com' } before do allow(user).to receive(:should_receive_in_person_completion_survey?). with(issuer).and_return(true) - allow(user).to receive(:confirmed_email_addresses). - and_return([ - email_address_one, - email_address_two, - ]) - allow(UserMailer).to receive(:in_person_completion_survey). - with(user, email_address_one). - and_return(message) - allow(UserMailer).to receive(:in_person_completion_survey). - with(user, email_address_two). - and_return(message2) - allow(message).to receive(:deliver_now_or_later) - allow(message2).to receive(:deliver_now_or_later) - allow(user).to receive(:mark_in_person_completion_survey_sent). - with(issuer) - described_class.send_completion_survey(user, issuer) + create(:service_provider, issuer: issuer) + create(:in_person_enrollment, user: user, issuer: issuer, status: :passed) end + it 'sends a survey to the user\'s confirmed email addresses' do - expect(message).to have_received(:deliver_now_or_later) - expect(message2).to have_received(:deliver_now_or_later) + create(:email_address, user: user) + described_class.send_completion_survey(user, issuer) + + expect_delivered_email_count(2) + expect_delivered_email( + 0, { + to: [user.confirmed_email_addresses[0].email], + subject: t('user_mailer.in_person_completion_survey.subject', app_name: APP_NAME), + } + ) + expect_delivered_email( + 1, { + to: [user.confirmed_email_addresses[1].email], + subject: t('user_mailer.in_person_completion_survey.subject', app_name: APP_NAME), + } + ) end it 'marks the user as having received a survey' do - expect(user).to have_received(:mark_in_person_completion_survey_sent). - with(issuer) + described_class.send_completion_survey(user, issuer) + expect(user.in_person_enrollments.find_by(issuer: issuer).follow_up_survey_sent).to eq true end end end diff --git a/spec/services/send_sign_up_email_confirmation_spec.rb b/spec/services/send_sign_up_email_confirmation_spec.rb index 8639ac15dd0..37aa3bcfe9f 100644 --- a/spec/services/send_sign_up_email_confirmation_spec.rb +++ b/spec/services/send_sign_up_email_confirmation_spec.rb @@ -15,43 +15,39 @@ confirmation_token: nil, confirmation_sent_at: nil, ) - user.reload end subject { described_class.new(user) } it 'sends the user an email with a confirmation link and the request id' do email_address.update!(confirmed_at: Time.zone.now) - mail = double - expect(mail).to receive(:deliver_now_or_later) - expect(UserMailer).to receive(:email_confirmation_instructions).with( - user, - email_address.email, - confirmation_token, - request_id: request_id, - instructions: instructions, - ).and_return(mail) subject.call(request_id: request_id, instructions: instructions) + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [user.email_addresses.first.email], + subject: t('user_mailer.email_confirmation_instructions.subject'), + body: [request_id, instructions], + } + ) end context 'when resetting a password' do it 'sends an email with a link to try another email if the current email is unconfirmed' do - mail = double - expect(mail).to receive(:deliver_now_or_later) - expect(UserMailer).to receive(:unconfirmed_email_instructions).with( - user, - email_address.email, - confirmation_token, - request_id: request_id, - instructions: instructions, - ).and_return(mail) - subject.call( request_id: request_id, instructions: instructions, password_reset_requested: true, ) + + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [email_address.email], + subject: t('user_mailer.email_confirmation_instructions.email_not_found'), + } + ) end end @@ -81,17 +77,19 @@ let(:user) { email_address.user } it 'regenerates a token if the token is expired' do - mail = double - expect(mail).to receive(:deliver_now_or_later) - - expect(UserMailer).to receive(:email_confirmation_instructions).with( - user, email_address.email, confirmation_token, instance_of(Hash) - ).and_return(mail) - subject.call expect(email_address.reload.confirmation_token).to eq(confirmation_token) expect(email_address.confirmation_sent_at).to be_within(5.seconds).of(Time.zone.now) + + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [user.email_addresses.first.email], + subject: t('user_mailer.email_confirmation_instructions.subject'), + body: [confirmation_token], + } + ) end end end 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 aa13e30eaad..4f2a3659ddd 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 @@ -12,8 +12,6 @@ create(:email_address, user: user, confirmed_at: nil) confirmed_email_addresses = user.confirmed_email_addresses - allow(UserMailer).to receive(:account_verified).and_call_original - described_class.call( user: user, date_time: date_time, @@ -21,17 +19,28 @@ disavowal_token: disavowal_token, ) - expect(UserMailer).to have_received(:account_verified). - exactly(confirmed_email_addresses.count).times - - confirmed_email_addresses.each do |email| - expect(UserMailer).to have_received(:account_verified). - with(user, - email, - date_time: date_time, - sp_name: '', - disavowal_token: disavowal_token) - end + expect_delivered_email_count(3) + expect_delivered_email( + 0, { + to: [confirmed_email_addresses[0].email], + subject: t('user_mailer.account_verified.subject', sp_name: ''), + body: [disavowal_token], + } + ) + expect_delivered_email( + 1, { + to: [confirmed_email_addresses[1].email], + subject: t('user_mailer.account_verified.subject', sp_name: ''), + body: [disavowal_token], + } + ) + expect_delivered_email( + 2, { + to: [confirmed_email_addresses[2].email], + subject: t('user_mailer.account_verified.subject', sp_name: ''), + body: [disavowal_token], + } + ) end end end diff --git a/spec/services/user_alerts/alert_user_about_new_device_spec.rb b/spec/services/user_alerts/alert_user_about_new_device_spec.rb index 997839a19a3..468ecacf333 100644 --- a/spec/services/user_alerts/alert_user_about_new_device_spec.rb +++ b/spec/services/user_alerts/alert_user_about_new_device_spec.rb @@ -11,23 +11,23 @@ confirmed_email_addresses = create_list(:email_address, 2, user: user) create(:email_address, user: user, confirmed_at: nil) - allow(UserMailer).to receive(:new_device_sign_in).and_call_original - described_class.call(user, device, disavowal_token) - expect(UserMailer).to have_received(:new_device_sign_in).twice - expect(UserMailer).to have_received(:new_device_sign_in). - with(user: user, - email_address: confirmed_email_addresses[0], - date: instance_of(String), - location: instance_of(String), - disavowal_token: disavowal_token) - expect(UserMailer).to have_received(:new_device_sign_in). - with(user: user, - email_address: confirmed_email_addresses[1], - date: instance_of(String), - location: instance_of(String), - disavowal_token: disavowal_token) + expect_delivered_email_count(2) + expect_delivered_email( + 0, { + to: [confirmed_email_addresses[0].email], + subject: t('user_mailer.new_device_sign_in.subject', app_name: APP_NAME), + body: [disavowal_token], + } + ) + expect_delivered_email( + 1, { + to: [confirmed_email_addresses[1].email], + subject: t('user_mailer.new_device_sign_in.subject', app_name: APP_NAME), + body: [disavowal_token], + } + ) end end end diff --git a/spec/services/user_alerts/alert_user_about_password_change_spec.rb b/spec/services/user_alerts/alert_user_about_password_change_spec.rb index 72e24879633..0e4e4294f33 100644 --- a/spec/services/user_alerts/alert_user_about_password_change_spec.rb +++ b/spec/services/user_alerts/alert_user_about_password_change_spec.rb @@ -9,15 +9,23 @@ confirmed_email_addresses = create_list(:email_address, 2, user: user) create(:email_address, user: user, confirmed_at: nil) - allow(UserMailer).to receive(:password_changed).and_call_original - described_class.call(user, disavowal_token) - expect(UserMailer).to have_received(:password_changed).twice - expect(UserMailer).to have_received(:password_changed). - with(user, confirmed_email_addresses[0], disavowal_token: disavowal_token) - expect(UserMailer).to have_received(:password_changed). - with(user, confirmed_email_addresses[1], disavowal_token: disavowal_token) + expect_delivered_email_count(2) + expect_delivered_email( + 0, { + to: [confirmed_email_addresses[0].email], + subject: t('devise.mailer.password_updated.subject'), + body: [disavowal_token], + } + ) + expect_delivered_email( + 1, { + to: [confirmed_email_addresses[1].email], + subject: t('devise.mailer.password_updated.subject'), + body: [disavowal_token], + } + ) end end end diff --git a/spec/services/user_alerts/alert_user_about_personal_key_sign_in_spec.rb b/spec/services/user_alerts/alert_user_about_personal_key_sign_in_spec.rb index b3cdefc9928..a3aedcc998a 100644 --- a/spec/services/user_alerts/alert_user_about_personal_key_sign_in_spec.rb +++ b/spec/services/user_alerts/alert_user_about_personal_key_sign_in_spec.rb @@ -13,16 +13,10 @@ create(:phone_configuration, user: user, phone: '(202) 222-2222'), ] - allow(UserMailer).to receive(:personal_key_sign_in).and_call_original allow(Telephony).to receive(:send_personal_key_sign_in_notice) response = described_class.call(user, disavowal_token) - expect(UserMailer).to have_received(:personal_key_sign_in).twice - expect(UserMailer).to have_received(:personal_key_sign_in). - with(user, confirmed_email_addresses[0].email, disavowal_token: disavowal_token) - expect(UserMailer).to have_received(:personal_key_sign_in). - with(user, confirmed_email_addresses[1].email, disavowal_token: disavowal_token) expect(Telephony).to have_received(:send_personal_key_sign_in_notice). with(to: phone_configurations[0].phone, country_code: 'US') expect(Telephony).to have_received(:send_personal_key_sign_in_notice). @@ -30,6 +24,22 @@ expect(response.to_h[:emails]).to eq(2) expect(response.to_h[:sms_message_ids].size).to eq(2) + + expect_delivered_email_count(2) + expect_delivered_email( + 0, { + to: [confirmed_email_addresses[0].email], + subject: t('user_mailer.personal_key_sign_in.subject'), + body: [disavowal_token], + } + ) + expect_delivered_email( + 1, { + to: [confirmed_email_addresses[1].email], + subject: t('user_mailer.personal_key_sign_in.subject'), + body: [disavowal_token], + } + ) end end end diff --git a/spec/services/usps_in_person_proofing/enrollment_helper_spec.rb b/spec/services/usps_in_person_proofing/enrollment_helper_spec.rb index 8a822ee22fc..428d75b89ec 100644 --- a/spec/services/usps_in_person_proofing/enrollment_helper_spec.rb +++ b/spec/services/usps_in_person_proofing/enrollment_helper_spec.rb @@ -94,18 +94,15 @@ end it 'sends verification emails' do - mailer = instance_double(ActionMailer::MessageDelivery, deliver_now_or_later: true) - user.email_addresses.each do |email_address| - expect(UserMailer).to receive(:in_person_ready_to_verify). - with( - user, - email_address, - enrollment: instance_of(InPersonEnrollment), - ). - and_return(mailer) - end - subject.schedule_in_person_enrollment(user, pii) + + expect_delivered_email_count(1) + expect_delivered_email( + 0, { + to: [user.email_addresses.first.email], + subject: t('user_mailer.in_person_ready_to_verify.subject', app_name: APP_NAME), + } + ) end end end diff --git a/spec/support/email_delivery.rb b/spec/support/email_delivery.rb deleted file mode 100644 index 7085381bf01..00000000000 --- a/spec/support/email_delivery.rb +++ /dev/null @@ -1,9 +0,0 @@ -module ActionMailer - class MessageDelivery - def deliver_later(_opts = {}) - # rubocop:disable IdentityIdp/MailLaterLinter - deliver_now - # rubocop:enable IdentityIdp/MailLaterLinter - end - end -end diff --git a/spec/support/features/mailer_helper.rb b/spec/support/features/mailer_helper.rb deleted file mode 100644 index 41cd2d9134f..00000000000 --- a/spec/support/features/mailer_helper.rb +++ /dev/null @@ -1,17 +0,0 @@ -RSpec.configure do |config| - config.before(:each, email: true) do - ActionMailer::Base.deliveries = [] - end -end - -module Features - module MailerHelper - def last_email - ActionMailer::Base.deliveries.last - end - - def reset_email - ActionMailer::Base.deliveries = [] - end - end -end diff --git a/spec/support/mailer_helper.rb b/spec/support/mailer_helper.rb new file mode 100644 index 00000000000..c36e60de0f3 --- /dev/null +++ b/spec/support/mailer_helper.rb @@ -0,0 +1,31 @@ +module MailerHelper + def last_email + ActionMailer::Base.deliveries.last + end + + def reset_email + ActionMailer::Base.deliveries = [] + end + + def expect_delivered_email_count(count) + expect(ActionMailer::Base.deliveries.count).to eq count + end + + def strip_tags(str) + ActionController::Base.helpers.strip_tags(str) + end + + def expect_delivered_email(index, hash) + mail = ActionMailer::Base.deliveries[index] + expect(mail.to).to eq hash[:to] if hash[:to] + + expect(mail.subject).to eq hash[:subject] if hash[:subject] + + if hash[:body] + delivered_body = mail.text_part.decoded.gsub("\r\n", ' ') + hash[:body].to_a.each do |expected_body| + expect(delivered_body).to include(expected_body) + end + end + end +end diff --git a/spec/views/layouts/user_mailer.html.erb_spec.rb b/spec/views/layouts/user_mailer.html.erb_spec.rb index d2acbf298de..1b14de82e26 100644 --- a/spec/views/layouts/user_mailer.html.erb_spec.rb +++ b/spec/views/layouts/user_mailer.html.erb_spec.rb @@ -4,7 +4,7 @@ let(:user) { build_stubbed(:user) } before do - @mail = UserMailer.email_added(user, 'foo@example.com') + @mail = UserMailer.with(user: user, email_address: user.email_addresses.first).email_added allow(view).to receive(:message).and_return(@mail) allow(view).to receive(:attachments).and_return(@mail.attachments) From 3bdd9bcb4ff03bd5b0d30b31509a11f262b55243 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Thu, 13 Oct 2022 13:45:20 -0700 Subject: [PATCH 06/11] Prepare to drop unused registration_logs columns (LG-6317) (#7131) - Make registration_logs.submitted_at nullable, stop writing it - Ignore other columns changelog: Internal, Logging, Stop writing extra registration_logs timestamps --- app/models/registration_log.rb | 9 +++++++++ app/services/funnel/registration/add_mfa.rb | 2 +- ...2457_alter_registration_logs_nullable_submitted_at.rb | 7 +++++++ db/schema.rb | 4 ++-- 4 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 db/primary_migrate/20221012172457_alter_registration_logs_nullable_submitted_at.rb diff --git a/app/models/registration_log.rb b/app/models/registration_log.rb index 5f3121154a2..d3ba1b86206 100644 --- a/app/models/registration_log.rb +++ b/app/models/registration_log.rb @@ -1,3 +1,12 @@ class RegistrationLog < ApplicationRecord + self.ignored_columns = %w[ + submitted_at + confirmed_at + password_at + first_mfa + first_mfa_at + second_mfa + ] + belongs_to :user end diff --git a/app/services/funnel/registration/add_mfa.rb b/app/services/funnel/registration/add_mfa.rb index 8be49424caf..200da332e7c 100644 --- a/app/services/funnel/registration/add_mfa.rb +++ b/app/services/funnel/registration/add_mfa.rb @@ -3,7 +3,7 @@ module Registration class AddMfa def self.call(user_id, mfa_method, analytics) now = Time.zone.now - funnel = RegistrationLog.where(user_id: user_id).first_or_create(submitted_at: now) + funnel = RegistrationLog.create_or_find_by(user_id: user_id) return if funnel.registered_at.present? analytics.user_registration_user_fully_registered(mfa_method: mfa_method) diff --git a/db/primary_migrate/20221012172457_alter_registration_logs_nullable_submitted_at.rb b/db/primary_migrate/20221012172457_alter_registration_logs_nullable_submitted_at.rb new file mode 100644 index 00000000000..1d6a4445bd1 --- /dev/null +++ b/db/primary_migrate/20221012172457_alter_registration_logs_nullable_submitted_at.rb @@ -0,0 +1,7 @@ +class AlterRegistrationLogsNullableSubmittedAt < ActiveRecord::Migration[7.0] + def change + safety_assured do + change_column_null :registration_logs, :submitted_at, true + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 0c62309953f..21d440b9bf9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2022_09_21_233413) do +ActiveRecord::Schema[7.0].define(version: 2022_10_12_172457) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" enable_extension "pgcrypto" @@ -483,7 +483,7 @@ create_table "registration_logs", force: :cascade do |t| t.integer "user_id", null: false - t.datetime "submitted_at", precision: nil, null: false + t.datetime "submitted_at", precision: nil t.datetime "confirmed_at", precision: nil t.datetime "password_at", precision: nil t.string "first_mfa" From 2a6bc17e50bb3bc23097d9e1aa1d9a82bb85e00f Mon Sep 17 00:00:00 2001 From: Julia Allen <51330839+julialeague@users.noreply.github.com> Date: Thu, 13 Oct 2022 15:40:50 -0700 Subject: [PATCH 07/11] LG-7251 Update SAML SP request flow to POST internally instead of GET (#6894) * Update SAML SP request flow to POST internally instead of GET * Add route for internal SAML auth POST requests * changelog: Improvements, Service Provider Authentication, Update SAML Authentication Flow * Add feature flag for SAML internal POST update, tests for the flag --- app/controllers/application_controller.rb | 20 +++++-- app/controllers/saml_completion_controller.rb | 27 +++++++++ app/controllers/saml_post_controller.rb | 3 +- .../saml_post_form.html.erb} | 0 config/application.yml.default | 2 + config/routes.rb | 3 + lib/identity_config.rb | 1 + .../application_controller_spec.rb | 46 ++++++++++++++ .../saml_completion_controller_spec.rb | 60 +++++++++++++++++++ spec/features/ialmax/saml_sign_in_spec.rb | 16 ++++- .../multiple_emails/sp_sign_in_spec.rb | 6 +- .../remember_device/sp_expiration_spec.rb | 10 ++-- .../reports/authorization_count_spec.rb | 23 ++++++- .../saml/authorization_confirmation_spec.rb | 5 +- spec/features/saml/ial1_sso_spec.rb | 19 +++--- spec/features/saml/multiple_endpoints_spec.rb | 1 + .../saml/redirect_uri_validation_spec.rb | 4 +- spec/features/saml/saml_relay_state_spec.rb | 13 ++-- spec/features/saml/saml_spec.rb | 19 +++--- spec/features/sign_in/banned_users_spec.rb | 3 +- spec/features/users/sign_in_spec.rb | 7 ++- spec/support/features/session_helper.rb | 10 ++-- spec/support/idv_examples/fail_to_verify.rb | 1 + .../idv_examples/sp_requested_attributes.rb | 2 +- spec/support/saml_auth_helper.rb | 4 +- .../shared_examples/account_creation.rb | 12 ++-- spec/support/shared_examples/sign_in.rb | 13 ++-- 27 files changed, 267 insertions(+), 63 deletions(-) create mode 100644 app/controllers/saml_completion_controller.rb rename app/views/{saml_post/auth.html.erb => shared/saml_post_form.html.erb} (100%) create mode 100644 spec/controllers/saml_completion_controller_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6444780a8cc..27675689222 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -434,10 +434,22 @@ def sp_session end def sp_session_request_url_with_updated_params - # Login.gov redirects to the orginal request_url after a user authenticates - # replace prompt=login with prompt=select_account to prevent sign_out - # which should only every occur once when the user lands on Login.gov with prompt=login - url = sp_session[:request_url]&.gsub('prompt=login', 'prompt=select_account') + # Temporarily place SAML route update behind a feature flag + if IdentityConfig.store.saml_internal_post + return unless sp_session[:request_url].present? + request_url = URI(sp_session[:request_url]) + url = if request_url.path.match?('saml') + complete_saml_url + else + # Login.gov redirects to the orginal request_url after a user authenticates + # replace prompt=login with prompt=select_account to prevent sign_out + # which should only ever occur once when the user + # lands on Login.gov with prompt=login + sp_session[:request_url]&.gsub('prompt=login', 'prompt=select_account') + end + else + url = sp_session[:request_url]&.gsub('prompt=login', 'prompt=select_account') + end # If the user has changed the locale, we should preserve that as well if url && locale_url_param && UriService.params(url)[:locale] != locale_url_param diff --git a/app/controllers/saml_completion_controller.rb b/app/controllers/saml_completion_controller.rb new file mode 100644 index 00000000000..4a92c37968f --- /dev/null +++ b/app/controllers/saml_completion_controller.rb @@ -0,0 +1,27 @@ +# Handles the INTERNAL redirect which happens when a service provider authentication request +# is passed through the IDP's authentication flow (sign-in, MFA, etc.) +# The original request was saved to the sp_session object, so we retrieve it to pass on the +# original request url in the form of a POST request to the SamlIdpController#auth method +class SamlCompletionController < ApplicationController + # Pass the original service provider request to the main SamlIdpController#auth method + # via a POST with form parameters replacing the url query parameters + def index + if sp_session.present? + request_url = URI(sp_session[:request_url]) + path_year = request_url.path[-4..-1] + path_method = "api_saml_finalauthpost#{path_year}_url" + action_url = Rails.application.routes.url_helpers.send(path_method) + + # Takes the query params which were set internally in the + # sp_session (so they should always be valid). + # A bad request that originated outside of the IDP would have + # already responded with a 400 status before reaching this point. + form_params = UriService.params(request_url) + + render 'shared/saml_post_form', locals: { action_url: action_url, form_params: form_params }, + layout: false + else + render_not_found + end + end +end diff --git a/app/controllers/saml_post_controller.rb b/app/controllers/saml_post_controller.rb index a9c2d389e9e..386dcfe0afd 100644 --- a/app/controllers/saml_post_controller.rb +++ b/app/controllers/saml_post_controller.rb @@ -9,6 +9,7 @@ def auth form_params = params.permit(:SAMLRequest, :RelayState, :SigAlg, :Signature) - render locals: { action_url: action_url, form_params: form_params }, layout: false + render 'shared/saml_post_form', locals: { action_url: action_url, form_params: form_params }, + layout: false end end diff --git a/app/views/saml_post/auth.html.erb b/app/views/shared/saml_post_form.html.erb similarity index 100% rename from app/views/saml_post/auth.html.erb rename to app/views/shared/saml_post_form.html.erb diff --git a/config/application.yml.default b/config/application.yml.default index abb8196fde2..57d27020f33 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -255,6 +255,7 @@ rules_of_use_horizon_years: 6 rules_of_use_updated_at: '2022-01-19T00:00:00Z' # Production has a newer timestamp than this, update directly in S3 s3_public_reports_enabled: false s3_reports_enabled: false +saml_internal_post: false saml_secret_rotation_enabled: false seed_agreements_data: true select_multiple_mfa_options: false @@ -537,6 +538,7 @@ test: s3_report_bucket_prefix: '' s3_report_public_bucket_prefix: '' saml_endpoint_configs: '[{"suffix":"2022","secret_key_passphrase":"trust-but-verify"}]' + saml_internal_post: true scrypt_cost: 800$8$1$ select_multiple_mfa_options: false secret_key_base: test_secret_key_base diff --git a/config/routes.rb b/config/routes.rb index 58484fe6ac0..9b189c5cad0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -22,8 +22,11 @@ post "/api/saml/auth#{suffix}" => 'saml_post#auth' # actual SAML handling POST route post "/api/saml/authpost#{suffix}" => 'saml_idp#auth' + # The internal auth post which will not be logged as an external request + post "/api/saml/finalauthpost#{suffix}" => 'saml_idp#auth' get "/api/saml/auth#{suffix}" => 'saml_idp#auth' end + get '/api/saml/complete' => 'saml_completion#index', as: :complete_saml post '/api/service_provider' => 'service_provider#update' post '/api/verify/images' => 'idv/image_uploads#create' diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 8f7ec34b0ea..22d37d1ce81 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -353,6 +353,7 @@ def self.build_store(config_map) config.add(:seed_agreements_data, type: :boolean) config.add(:select_multiple_mfa_options, type: :boolean) config.add(:service_provider_request_ttl_hours, type: :integer) + config.add(:saml_internal_post, type: :boolean) config.add(:session_check_delay, type: :integer) config.add(:session_check_frequency, type: :integer) config.add(:session_encryption_key, type: :string) diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index c39d5da3144..1efca5c90a5 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -415,6 +415,20 @@ def index expect(url_with_updated_params).to eq(nil) end + context 'with a SAML request' do + let(:sp_session_request_url) { '/api/saml/auth2022' } + it 'returns the saml completion url' do + expect(url_with_updated_params).to eq complete_saml_url + end + end + + context 'with an OIDC request' do + let(:sp_session_request_url) { '/openid_connect/authorize' } + it 'returns the original request' do + expect(url_with_updated_params).to eq '/openid_connect/authorize' + end + end + context 'with a url that has prompt=login' do let(:sp_session_request_url) { '/authorize?prompt=login' } it 'changes it to prompt=select_account' do @@ -429,6 +443,38 @@ def index expect(url_with_updated_params).to eq('/authorize?locale=es') end end + + context 'with saml_internal_post feature flag set to false' do + before { allow(IdentityConfig.store).to receive(:saml_internal_post).and_return false } + context 'with a SAML request' do + let(:sp_session_request_url) { '/api/saml/auth2022?SAMLRequest=blah' } + it 'returns the original request url' do + expect(url_with_updated_params).to eq '/api/saml/auth2022?SAMLRequest=blah' + end + end + + context 'with an OIDC request' do + let(:sp_session_request_url) { '/openid_connect/authorize' } + it 'returns the original request' do + expect(url_with_updated_params).to eq '/openid_connect/authorize' + end + end + + context 'with a url that has prompt=login' do + let(:sp_session_request_url) { '/authorize?prompt=login' } + it 'changes it to prompt=select_account' do + expect(url_with_updated_params).to eq('/authorize?prompt=select_account') + end + end + + context 'when the locale has been changed' do + before { I18n.locale = :es } + let(:sp_session_request_url) { '/authorize' } + it 'adds the locale to the url' do + expect(url_with_updated_params).to eq('/authorize?locale=es') + end + end + end end def expect_user_event_to_have_been_created(user, event_type) diff --git a/spec/controllers/saml_completion_controller_spec.rb b/spec/controllers/saml_completion_controller_spec.rb new file mode 100644 index 00000000000..f1b748a6a71 --- /dev/null +++ b/spec/controllers/saml_completion_controller_spec.rb @@ -0,0 +1,60 @@ +require 'rails_helper' + +describe SamlCompletionController do + describe 'GET #index' do + render_views + include ActionView::Helpers::FormTagHelper + + let(:form_action_regex) { / Date: Fri, 14 Oct 2022 10:02:51 -0400 Subject: [PATCH 08/11] Allow longer wait delay for in-person feature specs (#7145) **Why**: Clicking "Continue" from the "prepare" step will wait for a client-side logging event before continuing to the "State ID" step, which often cannot complete before the 0.5 second tolerance allowed by default in local development environments. changelog: Internal, Automated Testing, Improve reliability of feature specs --- spec/support/features/in_person_helper.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/spec/support/features/in_person_helper.rb b/spec/support/features/in_person_helper.rb index cc6a56d8c8a..2c3552bb6db 100644 --- a/spec/support/features/in_person_helper.rb +++ b/spec/support/features/in_person_helper.rb @@ -63,9 +63,7 @@ def complete_prepare_step(_user = nil) def complete_state_id_step(_user = nil) # Wait for page to load before attempting to fill out form - expect(page).to have_current_path( - idv_in_person_step_path(step: :state_id), - ) + expect(page).to have_current_path(idv_in_person_step_path(step: :state_id), wait: 10) fill_out_state_id_form_ok click_idv_continue end From 26cd77059f49ecec5a9227bfd08338b0e2ccd587 Mon Sep 17 00:00:00 2001 From: Doug Price Date: Fri, 14 Oct 2022 11:18:08 -0400 Subject: [PATCH 09/11] LG-7702: record the issuer of the SP requesting idv in the profile. (#7125) * LG-7702: record the issuer of the SP requesting idv in the profile. changelog: Internal, Identity Verification, Track the agency requesting identity verification. * record the initiating sp as an association * use the issuer as the foreign key --- app/models/profile.rb | 7 +++++++ app/services/idv/profile_maker.rb | 6 ++++-- app/services/idv/session.rb | 1 + ..._initiating_service_provider_issuer_to_profile.rb | 5 +++++ db/schema.rb | 1 + spec/services/idv/profile_maker_spec.rb | 12 ++++++++++++ 6 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 db/primary_migrate/20221011151135_add_initiating_service_provider_issuer_to_profile.rb diff --git a/app/models/profile.rb b/app/models/profile.rb index 4a214ee25a1..10b496b228c 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -2,6 +2,13 @@ class Profile < ApplicationRecord self.ignored_columns = %w[phone_confirmed] belongs_to :user + # rubocop:disable Rails/InverseOf + belongs_to :initiating_service_provider, + class_name: 'ServiceProvider', + foreign_key: 'initiating_service_provider_issuer', + primary_key: 'issuer', + optional: true + # rubocop:enable Rails/InverseOf has_many :gpo_confirmation_codes, dependent: :destroy has_one :in_person_enrollment, dependent: :destroy diff --git a/app/services/idv/profile_maker.rb b/app/services/idv/profile_maker.rb index 2c9be2cc064..8ff4f0744fa 100644 --- a/app/services/idv/profile_maker.rb +++ b/app/services/idv/profile_maker.rb @@ -2,14 +2,16 @@ module Idv class ProfileMaker attr_reader :pii_attributes - def initialize(applicant:, user:, user_password:) + def initialize(applicant:, user:, user_password:, initiating_service_provider: nil) self.pii_attributes = Pii::Attributes.new_from_hash(applicant) self.user = user self.user_password = user_password + self.initiating_service_provider = initiating_service_provider end def save_profile(active:, deactivation_reason: nil) profile = Profile.new(user: user, active: false, deactivation_reason: deactivation_reason) + profile.initiating_service_provider = initiating_service_provider profile.encrypt_pii(pii_attributes, user_password) profile.proofing_components = current_proofing_components profile.save! @@ -23,7 +25,7 @@ def current_proofing_components user.proofing_component&.as_json || {} end - attr_accessor :user, :user_password, :phone_confirmed + attr_accessor :user, :user_password, :phone_confirmed, :initiating_service_provider attr_writer :pii_attributes end end diff --git a/app/services/idv/session.rb b/app/services/idv/session.rb index d7986843e0b..3e61523e5a0 100644 --- a/app/services/idv/session.rb +++ b/app/services/idv/session.rb @@ -162,6 +162,7 @@ def build_profile_maker(user_password) applicant: applicant, user: current_user, user_password: user_password, + initiating_service_provider: service_provider, ) end diff --git a/db/primary_migrate/20221011151135_add_initiating_service_provider_issuer_to_profile.rb b/db/primary_migrate/20221011151135_add_initiating_service_provider_issuer_to_profile.rb new file mode 100644 index 00000000000..ef55eab91e6 --- /dev/null +++ b/db/primary_migrate/20221011151135_add_initiating_service_provider_issuer_to_profile.rb @@ -0,0 +1,5 @@ +class AddInitiatingServiceProviderIssuerToProfile < ActiveRecord::Migration[7.0] + def change + add_column :profiles, :initiating_service_provider_issuer, :string, null: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 21d440b9bf9..326cde73108 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -437,6 +437,7 @@ t.jsonb "proofing_components" t.string "name_zip_birth_year_signature" t.date "reproof_at" + t.string "initiating_service_provider_issuer" t.index ["name_zip_birth_year_signature"], name: "index_profiles_on_name_zip_birth_year_signature" t.index ["reproof_at"], name: "index_profiles_on_reproof_at" t.index ["ssn_signature"], name: "index_profiles_on_ssn_signature" diff --git a/spec/services/idv/profile_maker_spec.rb b/spec/services/idv/profile_maker_spec.rb index eb9a6130418..881891735a2 100644 --- a/spec/services/idv/profile_maker_spec.rb +++ b/spec/services/idv/profile_maker_spec.rb @@ -5,12 +5,14 @@ let(:applicant) { { first_name: 'Some', last_name: 'One' } } let(:user) { create(:user, :signed_up) } let(:user_password) { user.password } + let(:initiating_service_provider) { nil } subject do described_class.new( applicant: applicant, user: user, user_password: user_password, + initiating_service_provider: initiating_service_provider, ) end @@ -52,5 +54,15 @@ expect(profile.deactivation_reason).to be_nil end end + + context 'with an initiating service provider' do + let(:initiating_service_provider) { create(:service_provider) } + + it 'creates a profile with the initiating sp recorded' do + profile = subject.save_profile(active: true, deactivation_reason: nil) + + expect(profile.initiating_service_provider).to eq initiating_service_provider + end + end end end From 5c5dbf18eac97331ffe8b25ec9cd096c2b859c55 Mon Sep 17 00:00:00 2001 From: Matt Gardner Date: Fri, 14 Oct 2022 12:58:57 -0400 Subject: [PATCH 10/11] changelog: Improvements, In-Person Proofing, updates translations in french and spanish (#7139) --- config/locales/components/es.yml | 2 +- config/locales/components/fr.yml | 2 +- config/locales/idv/es.yml | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/config/locales/components/es.yml b/config/locales/components/es.yml index 24dcb36389f..b8882712470 100644 --- a/config/locales/components/es.yml +++ b/config/locales/components/es.yml @@ -38,4 +38,4 @@ es: label: Imprima esta página troubleshooting_options: default_heading: '¿Tiene alguna dificultad? Esto es lo que puede hacer:' - new_feature: Nueva función + new_feature: Nuevo artículo diff --git a/config/locales/components/fr.yml b/config/locales/components/fr.yml index e1be7225778..01a3d53937f 100644 --- a/config/locales/components/fr.yml +++ b/config/locales/components/fr.yml @@ -38,4 +38,4 @@ fr: label: Imprimer cette page troubleshooting_options: default_heading: 'Des difficultés? Voici ce que vous pouvez faire:' - new_feature: Nouvelle fonction + new_feature: Nouvelle fonctionnalité diff --git a/config/locales/idv/es.yml b/config/locales/idv/es.yml index 5763e6561a6..f30c18bfc3b 100644 --- a/config/locales/idv/es.yml +++ b/config/locales/idv/es.yml @@ -211,7 +211,8 @@ es: supported_documents: Vea la lista de documentos de identidad emitidos por el estado que son aceptados verify_by_mail: Verifique su dirección por correo - verify_in_person: Verifique su cédula de identidad en persona en una oficina de correos + verify_in_person: Verifique su documento de identidad en persona en una oficina + de correos welcome: no_js_header: Debe habilitar JavaScript para verificar su identidad. no_js_intro: '%{sp_name} requiere que usted verifique su identidad. Debe From 8560940842f63352593a2881ebaeffdb591d4a53 Mon Sep 17 00:00:00 2001 From: "Gene M. Angelo, Jr" Date: Mon, 17 Oct 2022 09:00:29 -0400 Subject: [PATCH 11/11] LG-7446 Create "Cancel" Links and Supporting Cancellation Code for Identity Proofing Process (2 of n) (#7144) * Segregate Inherited Proofing routes changelog: Improvements, Upcoming Features, LG-7446 Create Inherited Proofing Cancellation Links and Process * Rename concern to avoid whitelist in naming - Flow step whitelist should be compared as strings so this was changed as well. - ...not Symbols, because they will be compared against params[:step] which will be a String value. - Remove unnecessary code * Add InheritedProofingCancellationsController specs --- ...rn.rb => allowlisted_flow_step_concern.rb} | 8 +- ...rited_proofing_cancellations_controller.rb | 25 +-- config/routes.rb | 21 +- ..._proofing_cancellations_controller_spec.rb | 191 ++++++++++++++++++ 4 files changed, 217 insertions(+), 28 deletions(-) rename app/controllers/concerns/{allow_whitelisted_flow_step_concern.rb => allowlisted_flow_step_concern.rb} (84%) create mode 100644 spec/controllers/idv/inherited_proofing_cancellations_controller_spec.rb diff --git a/app/controllers/concerns/allow_whitelisted_flow_step_concern.rb b/app/controllers/concerns/allowlisted_flow_step_concern.rb similarity index 84% rename from app/controllers/concerns/allow_whitelisted_flow_step_concern.rb rename to app/controllers/concerns/allowlisted_flow_step_concern.rb index 0eaf23922ae..7a2d4ba1657 100644 --- a/app/controllers/concerns/allow_whitelisted_flow_step_concern.rb +++ b/app/controllers/concerns/allowlisted_flow_step_concern.rb @@ -7,7 +7,7 @@ # button_to(idv_inherited_proofing_cancel_path(step: params[:step]), ...) ... # end # %> -module AllowWhitelistedFlowStepConcern +module AllowlistedFlowStepConcern extend ActiveSupport::Concern included do @@ -18,7 +18,7 @@ module AllowWhitelistedFlowStepConcern def flow_step! flow_step = flow_step_param - unless flow_step_whitelist.include? flow_step + unless flow_step_allowlist.include? flow_step Rails.logger.warn "Flow step param \"#{flow_step})\" was not whitelisted!" render_not_found and return end @@ -31,7 +31,7 @@ def flow_step_param params[:step] end - def flow_step_whitelist - raise NotImplementedError, '#flow_step_whitelist must be overridden' + def flow_step_allowlist + raise NotImplementedError, '#flow_step_allowlist must be overridden' end end diff --git a/app/controllers/idv/inherited_proofing_cancellations_controller.rb b/app/controllers/idv/inherited_proofing_cancellations_controller.rb index c11272be7b2..c277fd9c9d0 100644 --- a/app/controllers/idv/inherited_proofing_cancellations_controller.rb +++ b/app/controllers/idv/inherited_proofing_cancellations_controller.rb @@ -3,7 +3,7 @@ class InheritedProofingCancellationsController < ApplicationController include IdvSession include GoBackHelper include InheritedProofing404Concern - include AllowWhitelistedFlowStepConcern + include AllowlistedFlowStepConcern before_action :confirm_idv_needed @@ -25,6 +25,7 @@ def update end def destroy + # NOTE: Uncomment this when analytics are implemented. # analytics.idv_inherited_proofing_cancellation_confirmed(step: params[:step]) cancel_session render json: { redirect_url: cancelled_redirect_path } @@ -52,6 +53,10 @@ def cancelled_redirect_path account_path end + def location_params + params.permit(:step, :location).to_h.symbolize_keys + end + def session_go_back_path=(path) idv_session.go_back_path = path end @@ -60,22 +65,10 @@ def session_go_back_path idv_session.go_back_path end - # AllowWhitelistedFlowStepConcern Concern overrides - - def flow_step_whitelist - Idv::Flows::InheritedProofingFlow::STEPS.keys - end - - # IdvSession Concern overrides - - def confirm_idv_vendor_session_started - return if flash[:allow_confirmations_continue] - - redirect_to idv_inherited_proofing_url unless idv_session.proofing_started? - end + # AllowlistedFlowStepConcern Concern overrides - def hybrid_session? - false + def flow_step_allowlist + @flow_step_allowlist ||= Idv::Flows::InheritedProofingFlow::STEPS.keys.map(&:to_s) end # NOTE: Override and use Inherited Proofing (IP)-specific :throttle_type diff --git a/config/routes.rb b/config/routes.rb index 9b189c5cad0..e23547ed07a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -348,19 +348,24 @@ get '/in_person/:step' => 'in_person#show', as: :in_person_step put '/in_person/:step' => 'in_person#update' - get '/inherited_proofing' => 'inherited_proofing#index' - get '/inherited_proofing/:step' => 'inherited_proofing#show', as: :inherited_proofing_step - put '/inherited_proofing/:step' => 'inherited_proofing#update' - get '/inherited_proofing/return_to_sp' => 'inherited_proofing#return_to_sp' - get '/inherited_proofing/cancel/' => 'inherited_proofing_cancellations#new', as: :inherited_proofing_cancel - put '/inherited_proofing/cancel' => 'inherited_proofing_cancellations#update' - delete '/inherited_proofing/cancel' => 'inherited_proofing_cancellations#destroy' - # deprecated routes get '/confirmations' => 'personal_key#show' post '/confirmations' => 'personal_key#update' end + # Inherited Proofing (IP)-specific routes. + scope '/verify/inherited_proofing', module: 'idv', as: 'idv_inherited_proofing' do + # NOTE: cancellation routes need to be before any other IP + # routes in this scope. + get '/cancel' => 'inherited_proofing_cancellations#new', as: :cancel + put '/cancel' => 'inherited_proofing_cancellations#update' + delete '/cancel' => 'inherited_proofing_cancellations#destroy' + get '/' => 'inherited_proofing#index' + get '/:step' => 'inherited_proofing#show', as: :step + put '/:step' => 'inherited_proofing#update' + get '/return_to_sp' => 'inherited_proofing#return_to_sp' + end + namespace :api do post '/verify/v2/document_capture' => 'verify/document_capture#create' delete '/verify/v2/document_capture_errors' => 'verify/document_capture_errors#delete' diff --git a/spec/controllers/idv/inherited_proofing_cancellations_controller_spec.rb b/spec/controllers/idv/inherited_proofing_cancellations_controller_spec.rb new file mode 100644 index 00000000000..8289bacec5e --- /dev/null +++ b/spec/controllers/idv/inherited_proofing_cancellations_controller_spec.rb @@ -0,0 +1,191 @@ +require 'rails_helper' + +shared_examples 'an HTML 404 not found' do + it 'returns a 404 not found' do + expect(response).to have_http_status(:not_found) + end +end + +describe Idv::InheritedProofingCancellationsController do + let(:step) { Idv::Flows::InheritedProofingFlow::STEPS.keys.first } + + describe 'before_actions' do + it 'includes before_actions from IdvSession' do + expect(subject).to have_actions(:before, :redirect_if_sp_context_needed) + end + + it 'includes before_actions from InheritedProofing404Concern' do + expect(subject).to have_actions(:before, :render_404_if_disabled) + end + + it 'includes before_actions from AllowlistedFlowStepConcern' do + expect(subject).to have_actions(:before, :flow_step!) + end + end + + describe '#new' do + let(:go_back_path) { '/path/to/return' } + + before do + allow(controller).to receive(:go_back_path).and_return(go_back_path) + end + + context 'when the inherited proofing feature flipper is turned off' do + before do + allow(IdentityConfig.store).to receive(:inherited_proofing_enabled).and_return(false) + stub_sign_in + end + + describe '#new' do + before do + get :new, params: { step: step } + end + + it_behaves_like 'an HTML 404 not found' + end + + describe '#update' do + before do + get :update, params: { step: step } + end + + it_behaves_like 'an HTML 404 not found' + end + + describe '#destroy' do + before do + get :destroy, params: { step: step } + end + + it_behaves_like 'an HTML 404 not found' + end + end + + context 'when the flow step is not in the allowed list' do + before do + stub_sign_in + end + + let(:step) { :not_found_step } + let(:expected_logger_warning) { "Flow step param \"#{step})\" was not whitelisted!" } + + describe '#new' do + before do + expect(Rails.logger).to receive(:warn).with(expected_logger_warning) + get :new, params: { step: step } + end + + it_behaves_like 'an HTML 404 not found' + end + + describe '#update' do + before do + expect(Rails.logger).to receive(:warn).with(expected_logger_warning) + get :update, params: { step: step } + end + + it_behaves_like 'an HTML 404 not found' + end + + describe '#destroy' do + before do + expect(Rails.logger).to receive(:warn).with(expected_logger_warning) + get :destroy, params: { step: step } + end + + it_behaves_like 'an HTML 404 not found' + end + end + + context 'when there is no session' do + it 'redirects to root' do + get :new + + expect(response).to redirect_to(root_url) + end + end + + context 'when there is a session' do + before do + stub_sign_in + end + + it 'renders template' do + get :new, params: { step: step } + + expect(response).to render_template(:new) + end + + it 'stores go back path' do + get :new, params: { step: step } + + expect(controller.user_session[:idv][:go_back_path]).to eq(go_back_path) + end + end + end + + describe '#update' do + before do + stub_sign_in + end + + it 'redirects to idv_inherited_proofing_path' do + put :update, params: { step: step, cancel: 'true' } + + expect(response).to redirect_to idv_inherited_proofing_url + end + + context 'when a go back path is stored in session' do + let(:go_back_path) { '/path/to/return' } + + before do + allow(controller).to receive(:user_session).and_return( + idv: { go_back_path: go_back_path }, + ) + end + + it 'redirects to go back path' do + put :update, params: { step: step, cancel: 'true' } + + expect(response).to redirect_to go_back_path + end + end + end + + describe '#destroy' do + context 'when there is no session' do + it 'redirects to root' do + delete :destroy, params: { step: step } + + expect(response).to redirect_to(root_url) + end + end + + context 'when there is a session' do + let(:user) { create(:user) } + + before do + stub_sign_in user + end + + before do + allow(controller).to receive(:user_session). + and_return(idv: { go_back_path: '/path/to/return' }) + end + + it 'destroys session' do + expect(subject).to receive(:cancel_session).once + + delete :destroy, params: { step: step } + end + + it 'renders a json response with the redirect path set to account_path' do + delete :destroy, params: { step: step } + + parsed_body = JSON.parse(response.body, symbolize_names: true) + expect(response).not_to render_template(:destroy) + expect(parsed_body).to eq({ redirect_url: account_path }) + end + end + end +end