diff --git a/app/controllers/users/reset_passwords_controller.rb b/app/controllers/users/reset_passwords_controller.rb index dfff6ab03ec..47b11696dc5 100644 --- a/app/controllers/users/reset_passwords_controller.rb +++ b/app/controllers/users/reset_passwords_controller.rb @@ -90,7 +90,12 @@ def request_id end def handle_valid_email - create_account_if_email_not_found + RequestPasswordReset.new( + email: email, + request_id: request_id, + analytics: analytics, + irs_attempts_api_tracker: irs_attempts_api_tracker, + ).perform session[:email] = email resend_confirmation = email_params[:resend] @@ -103,24 +108,6 @@ def store_token_in_session session[:reset_password_token] = params[:reset_password_token] end - def create_account_if_email_not_found - user, result = RequestPasswordReset.new( - email: email, - request_id: request_id, - analytics: analytics, - irs_attempts_api_tracker: irs_attempts_api_tracker, - ).perform - - return unless result - - analytics.user_registration_email(**result.to_h) - irs_attempts_api_tracker.user_registration_email_submitted( - email: email, - success: result.success?, - ) - create_user_event(:account_created, user) - end - def handle_invalid_or_expired_token(result) flash[:error] = t("devise.passwords.#{result.errors[:user].first}") session.delete(:reset_password_token) diff --git a/app/forms/register_user_email_form.rb b/app/forms/register_user_email_form.rb index f712535b26c..e01c85f04b5 100644 --- a/app/forms/register_user_email_form.rb +++ b/app/forms/register_user_email_form.rb @@ -10,15 +10,13 @@ class RegisterUserEmailForm attr_reader :email_address, :terms_accepted attr_accessor :email_language - attr_accessor :password_reset_requested def self.model_name ActiveModel::Name.new(self, nil, 'User') end - def initialize(analytics:, attempts_tracker:, password_reset_requested: false) + def initialize(analytics:, attempts_tracker:) @rate_limited = false - @password_reset_requested = password_reset_requested @analytics = analytics @attempts_tracker = attempts_tracker end @@ -53,7 +51,7 @@ def validate_terms_accepted errors.add(:terms_accepted, t('errors.registration.terms'), type: :terms) end - def submit(params, instructions = nil) + def submit(params) @terms_accepted = !!ActiveModel::Type::Boolean.new.cast(params[:terms_accepted]) build_user_and_email_address_with_email( email: params[:email], @@ -62,7 +60,7 @@ def submit(params, instructions = nil) self.request_id = params[:request_id] self.success = valid? - process_successful_submission(request_id, instructions) if success + process_successful_submission(request_id) if success FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes) end @@ -72,10 +70,6 @@ def email_taken? @email_taken = lookup_email_taken end - def password_reset_requested? - @password_reset_requested - end - private attr_writer :email, :email_address @@ -98,7 +92,7 @@ def lookup_email_taken true end - def process_successful_submission(request_id, instructions) + def process_successful_submission(request_id) # To prevent discovery of existing emails, we check to see if the email is # already taken and if so, we act as if the user registration was successful. if email_address_record&.user&.suspended? @@ -111,7 +105,7 @@ def process_successful_submission(request_id, instructions) elsif email_taken? send_sign_up_confirmed_email else - send_sign_up_email(request_id, instructions) + send_sign_up_email(request_id) end end @@ -140,7 +134,7 @@ def rate_limit!(rate_limit_type) @rate_limited = rate_limiter.limited? end - def send_sign_up_email(request_id, instructions) + def send_sign_up_email(request_id) rate_limit!(:reg_unconfirmed_email) if rate_limited? @@ -154,11 +148,7 @@ def send_sign_up_email(request_id, instructions) user.accepted_terms_at = Time.zone.now user.save! - SendSignUpEmailConfirmation.new(user).call( - request_id: email_request_id(request_id), - instructions: instructions, - password_reset_requested: password_reset_requested?, - ) + SendSignUpEmailConfirmation.new(user).call(request_id: email_request_id(request_id)) end end diff --git a/app/mailers/anonymous_mailer.rb b/app/mailers/anonymous_mailer.rb new file mode 100644 index 00000000000..2e13570519f --- /dev/null +++ b/app/mailers/anonymous_mailer.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +# AnonymousMailer handles all email sending not associated with a user. It expects to be called +# using `with` that receives an `email` string value. +# +# You MUST deliver these messages using `deliver_now`. Anonymous messages rely on a plaintext email +# address, which is personally-identifiable information (PII). All method arguments are stored in +# the database when the email is being sent asynchronously by ActiveJob and we must not put PII in +# the database in plaintext. +# +# Example: +# +# AnonymousMailer.with(email:).password_reset_missing_user(request_id:).deliver_now +# +class AnonymousMailer < ActionMailer::Base + include Mailable + include LocaleHelper + + before_action :attach_images + + after_action :add_metadata + + default( + from: email_with_name( + IdentityConfig.store.email_from, + IdentityConfig.store.email_from_display_name, + ), + reply_to: email_with_name( + IdentityConfig.store.email_from, + IdentityConfig.store.email_from_display_name, + ), + ) + + layout 'mailer' + + def password_reset_missing_user(request_id:) + @request_id = request_id + + mail( + to: email, + subject: t('anonymous_mailer.password_reset_missing_user.subject'), + ) + end + + private + + def email + params.fetch(:email) + end + + def add_metadata + message.instance_variable_set(:@_metadata, action: action_name) + end +end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index d51013d7531..e0e6aa2fa5f 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -6,7 +6,7 @@ # # 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 +# email is being sent asynchronously by ActiveJob and we must not put PII in the database in # plaintext. # # Example: @@ -38,6 +38,8 @@ class UserEmailAddressMismatchError < StandardError; end ), ) + layout 'mailer' + def validate_user_and_email_address @user = params.fetch(:user) @email_address = params.fetch(:email_address) @@ -56,10 +58,10 @@ def add_metadata ) end - def email_confirmation_instructions(token, request_id:, instructions:) + def email_confirmation_instructions(token, request_id:) with_user_locale(user) do presenter = ConfirmationEmailPresenter.new(user, view_context) - @first_sentence = instructions || presenter.first_sentence + @first_sentence = presenter.first_sentence @confirmation_period = presenter.confirmation_period @request_id = request_id @locale = locale_url_param @@ -71,21 +73,6 @@ def email_confirmation_instructions(token, request_id:, instructions:) end end - 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 - @confirmation_period = presenter.confirmation_period - @request_id = request_id - @locale = locale_url_param - @token = token - mail( - to: email_address.email, - subject: t('user_mailer.email_confirmation_instructions.email_not_found'), - ) - end - end - def signup_with_your_email with_user_locale(user) do @root_url = root_url(locale: locale_url_param) diff --git a/app/services/request_password_reset.rb b/app/services/request_password_reset.rb index c5b9505f049..5dddff998c9 100644 --- a/app/services/request_password_reset.rb +++ b/app/services/request_password_reset.rb @@ -6,28 +6,13 @@ allowed_members: [:request_id] ) do def perform - if user_should_receive_registration_email? - form = RegisterUserEmailForm.new( - password_reset_requested: true, - analytics: analytics, - attempts_tracker: irs_attempts_api_tracker, - ) - result = form.submit({ email: email, terms_accepted: '1' }, instructions) - [form.user, result] - else - send_reset_password_instructions - nil - end - end - - private - - def send_reset_password_instructions - rate_limiter = RateLimiter.new(user: user, rate_limit_type: :reset_password_email) + rate_limiter = RateLimiter.new(target: email, rate_limit_type: :reset_password_email) rate_limiter.increment! if rate_limiter.limited? analytics.rate_limit_reached(limiter_type: :reset_password_email) irs_attempts_api_tracker.forgot_password_email_rate_limited(email: email) + elsif user.blank? + AnonymousMailer.with(email:).password_reset_missing_user(request_id:).deliver_now elsif user.suspended? UserMailer.with( user: user, @@ -47,42 +32,13 @@ def send_reset_password_instructions end end - def instructions - I18n.t( - 'user_mailer.email_confirmation_instructions.first_sentence.forgot_password', - app_name: APP_NAME, - ) - end - - ## - # If a user record does not exist for an email address, we send a registration - # email instead of a reset email so the user can go through the account - # creation process without having to receive another email - # - # If a user exists but does not have any confirmed email addresses, we send - # them a reset email so they can set the password on the account - # - # If a user exists and has a confirmed email addresses, but this email address - # is not confirmed we should not let them reset the password with this email - # address. Instead we send them an email to create an account with the - # unconfirmed email address - ## - def user_should_receive_registration_email? - return true if user.nil? - return false unless user.confirmed? - return false if email_address_record.confirmed? - true - end + private def user @user ||= email_address_record&.user end - # We want to find the EmailAddress with preferring to find the confirmed one first - # if both a confirmed and an unconfirmed row exist def email_address_record - @email_address_record ||= begin - EmailAddress.find_with_confirmed_or_unconfirmed_email(email) - end + @email_address_record ||= EmailAddress.confirmed.find_with_email(email) end end.freeze diff --git a/app/services/send_sign_up_email_confirmation.rb b/app/services/send_sign_up_email_confirmation.rb index 4768c1942bf..62af8f19ecf 100644 --- a/app/services/send_sign_up_email_confirmation.rb +++ b/app/services/send_sign_up_email_confirmation.rb @@ -9,14 +9,9 @@ def initialize(user) @user = user end - def call(request_id: nil, instructions: nil, password_reset_requested: false) + def call(request_id: nil) update_email_address_record - - if password_reset_requested && !user.confirmed? - send_pw_reset_request_unconfirmed_user_email(request_id, instructions) - else - send_confirmation_email(request_id, instructions) - end + send_confirmation_email(request_id) end private @@ -53,19 +48,10 @@ def update_email_address_record ) end - def send_confirmation_email(request_id, instructions) + def send_confirmation_email(request_id) UserMailer.with(user: user, email_address: email_address).email_confirmation_instructions( confirmation_token, request_id: request_id, - instructions: instructions, - ).deliver_now_or_later - end - - def send_pw_reset_request_unconfirmed_user_email(request_id, instructions) - UserMailer.with(user: user, email_address: email_address).unconfirmed_email_instructions( - confirmation_token, - request_id: request_id, - instructions: instructions, ).deliver_now_or_later end diff --git a/app/views/anonymous_mailer/password_reset_missing_user.html.erb b/app/views/anonymous_mailer/password_reset_missing_user.html.erb new file mode 100644 index 00000000000..250746c6bca --- /dev/null +++ b/app/views/anonymous_mailer/password_reset_missing_user.html.erb @@ -0,0 +1,42 @@ +
+ <%= t('anonymous_mailer.password_reset_missing_user.info_no_account', app_name: APP_NAME) %> +
++ <%= t('anonymous_mailer.password_reset_missing_user.info_request_different', app_name: APP_NAME) %> +
+ +
+
|
+ + |
+ <%= t( + 'anonymous_mailer.password_reset_missing_user.use_this_email_html', + create_account_link_html: link_to( + t('anonymous_mailer.password_reset_missing_user.create_new_account'), + sign_up_register_url(request_id: @request_id), + target: '_blank', + rel: 'noopener', + ), + ) %> +
diff --git a/app/views/layouts/user_mailer.html.erb b/app/views/layouts/mailer.html.erb similarity index 100% rename from app/views/layouts/user_mailer.html.erb rename to app/views/layouts/mailer.html.erb diff --git a/app/views/user_mailer/unconfirmed_email_instructions.html.erb b/app/views/user_mailer/unconfirmed_email_instructions.html.erb deleted file mode 100644 index 823ab278531..00000000000 --- a/app/views/user_mailer/unconfirmed_email_instructions.html.erb +++ /dev/null @@ -1,46 +0,0 @@ -- <%= @first_sentence %> -
-- <%= t('user_mailer.email_confirmation_instructions.request_with_diff_email', app_name: APP_NAME) %> -
- -
-
|
- - |
- <%= t('user_mailer.email_confirmation_instructions.use_this_email') %> - <%= link_to t('user_mailer.email_confirmation_instructions.create_new_account'), - sign_up_create_email_confirmation_url( - _request_id: @request_id, - confirmation_token: @token, - locale: @locale, - ), - target: '_blank', - rel: 'noopener' %> - <%= t( - 'user_mailer.email_confirmation_instructions.footer', - confirmation_period: @confirmation_period, - ) %> -
diff --git a/config/locales/anonymous_mailer/en.yml b/config/locales/anonymous_mailer/en.yml new file mode 100644 index 00000000000..f4ed4639c1d --- /dev/null +++ b/config/locales/anonymous_mailer/en.yml @@ -0,0 +1,12 @@ +--- +en: + anonymous_mailer: + password_reset_missing_user: + create_new_account: create a new account + info_no_account: You tried to reset your %{app_name} password but we don’t have + an account linked to this email address. + info_request_different: You can request your password using a different email + address that is connected to your %{app_name} account. + subject: Email not found + try_different_email: Try a different email address + use_this_email_html: Or use this email address and %{create_account_link_html}. diff --git a/config/locales/anonymous_mailer/es.yml b/config/locales/anonymous_mailer/es.yml new file mode 100644 index 00000000000..d45cd399adc --- /dev/null +++ b/config/locales/anonymous_mailer/es.yml @@ -0,0 +1,12 @@ +--- +es: + anonymous_mailer: + password_reset_missing_user: + create_new_account: crea una cuenta nueva + info_no_account: Intentó restablecer su contraseña de %{app_name} pero no + tenemos una cuenta vinculada a esta dirección de correo electrónico. + info_request_different: Puedes solicitar tu contraseña usando una dirección de + correo electrónico diferente que está conectada a su cuenta %{app_name}. + subject: El correo electrónico no encontrado + try_different_email: Pruebe con una dirección de correo electrónico diferente + use_this_email_html: O use este correo electrónico y %{create_account_link_html}. diff --git a/config/locales/anonymous_mailer/fr.yml b/config/locales/anonymous_mailer/fr.yml new file mode 100644 index 00000000000..04258a2ba3f --- /dev/null +++ b/config/locales/anonymous_mailer/fr.yml @@ -0,0 +1,13 @@ +--- +fr: + anonymous_mailer: + password_reset_missing_user: + create_new_account: créer un nouveau compte + info_no_account: Vous avez essayé de réinitialiser le mot de passe de votre + compte %{app_name}, mais nous ne possédons pas de compte associé à cette + adresse courriel. + info_request_different: Vous pouvez demander votre mot de passe en utilisant une + adresse e-mail différente qui est connectée à votre compte %{app_name}. + subject: Email non trouvé + try_different_email: Essayez une autre adresse e-mail + use_this_email_html: Ou utilisez cet e-mail et %{create_account_link_html}. diff --git a/config/locales/user_mailer/en.yml b/config/locales/user_mailer/en.yml index 0c3d1f9f205..9e3d3e66c92 100644 --- a/config/locales/user_mailer/en.yml +++ b/config/locales/user_mailer/en.yml @@ -71,22 +71,14 @@ en: email addresses. We recommend that you also change your password. subject: New email address added email_confirmation_instructions: - create_new_account: create a new account. - email_not_found: Email not found first_sentence: confirmed: Trying to change your email address? - forgot_password: You tried to reset your %{app_name} password but we don’t have - an account linked to this email address. unconfirmed: Thanks for submitting your email address. footer: This link will expire in %{confirmation_period}. header: '%{intro} Please click the link below or copy and paste the entire link into your browser.' link_text: Confirm email address - request_with_diff_email: You can request your password using a different email - address that is connected to your %{app_name} account. subject: Confirm your email - try_diff_email: Try a different email address - use_this_email: Or use this email address and email_deleted: header: An email address was deleted from your %{app_name} profile. help_html: If you did not want to delete this email address, please visit the diff --git a/config/locales/user_mailer/es.yml b/config/locales/user_mailer/es.yml index 12540d650c8..a44ed1340c0 100644 --- a/config/locales/user_mailer/es.yml +++ b/config/locales/user_mailer/es.yml @@ -76,22 +76,14 @@ es: contraseña. subject: Nueva dirección de correo electrónico añadida email_confirmation_instructions: - create_new_account: crea una cuenta nueva. - email_not_found: El correo electrónico no encontrado first_sentence: confirmed: '¿Desea cambiar su email?' - forgot_password: Intentó restablecer su contraseña de %{app_name} pero no - tenemos una cuenta vinculada a esta dirección de correo electrónico. unconfirmed: Gracias por enviar su dirección de correo electrónico. footer: Este enlace expira en %{confirmation_period}. header: '%{intro} Haga clic en el enlace de abajo o copie y pegue el enlace completo en su navegador.' link_text: Confirmar el correo - request_with_diff_email: Puedes solicitar tu contraseña usando una dirección de - correo electrónico diferente que está conectada a su cuenta %{app_name}. subject: Confirme su email - try_diff_email: Pruebe con una dirección de correo electrónico diferente - use_this_email: O use este correo electrónico y email_deleted: header: Se eliminó una dirección de correo electrónico de su perfil de %{app_name}. diff --git a/config/locales/user_mailer/fr.yml b/config/locales/user_mailer/fr.yml index b1e075c8cea..308db563801 100644 --- a/config/locales/user_mailer/fr.yml +++ b/config/locales/user_mailer/fr.yml @@ -79,24 +79,14 @@ fr: également votre mot de passe. subject: Nouvelle adresse email ajoutée email_confirmation_instructions: - create_new_account: créer un nouveau compte. - email_not_found: Email non trouvé first_sentence: confirmed: Vous tentez de changer votre adresse courriel? - forgot_password: Vous avez essayé de réinitialiser le mot de passe de votre - compte %{app_name}, mais nous ne possédons pas de compte associé à - cette adresse courriel. unconfirmed: Merci d’avoir envoyé votre adresse email. footer: Ce lien expirera dans %{confirmation_period}. header: '%{intro} Veuillez cliquer sur le lien ci-dessous ou copier et coller le lien complet dans votre navigateur.' link_text: Confirmez votre adresse email - request_with_diff_email: Vous pouvez demander votre mot de passe en utilisant - une adresse e-mail différente qui est connectée à votre compte - %{app_name}. subject: Confirmez votre adresse courriel - try_diff_email: Essayez une autre adresse e-mail - use_this_email: Ou utilisez cet e-mail et email_deleted: header: Une adresse email a été supprimée de votre profil %{app_name}. help_html: Si vous ne souhaitez pas supprimer cette adresse électronique diff --git a/spec/controllers/users/reset_passwords_controller_spec.rb b/spec/controllers/users/reset_passwords_controller_spec.rb index cd9e95afc9f..6e190c4a60b 100644 --- a/spec/controllers/users/reset_passwords_controller_spec.rb +++ b/spec/controllers/users/reset_passwords_controller_spec.rb @@ -538,12 +538,6 @@ it 'send an email to tell the user they do not have an account yet' do stub_analytics - stub_attempts_tracker - - expect(@irs_attempts_api_tracker).to receive(:user_registration_email_submitted).with( - email: email, - **success_properties, - ) expect do put :create, params: { @@ -551,6 +545,9 @@ } end.to(change { ActionMailer::Base.deliveries.count }.by(1)) + expect(ActionMailer::Base.deliveries.last.subject). + to eq t('anonymous_mailer.password_reset_missing_user.subject') + analytics_hash = { success: true, errors: {}, @@ -561,19 +558,6 @@ } expect(@analytics).to have_logged_event('Password Reset: Email Submitted', analytics_hash) - - analytics_hash = { - success: true, - rate_limited: false, - errors: {}, - email_already_exists: false, - user_id: User.find_with_email(email).uuid, - domain_name: 'example.com', - } - expect(@analytics).to have_logged_event( - 'User Registration: Email Submitted', - analytics_hash, - ) expect(response).to redirect_to forgot_password_path end end @@ -636,15 +620,10 @@ before do stub_analytics - stub_attempts_tracker allow(@analytics).to receive(:track_event) end - it 'sends password reset email to user and tracks event' do - expect(@irs_attempts_api_tracker).to receive(:forgot_password_email_sent).with( - email: user.email, - ) - + it 'sends missing user email and tracks event' do expect { put :create, params: params }. to change { ActionMailer::Base.deliveries.count }.by(1) @@ -652,7 +631,7 @@ with('Password Reset: Email Submitted', analytics_hash) expect(ActionMailer::Base.deliveries.last.subject). - to eq t('user_mailer.reset_password_instructions.subject') + to eq t('anonymous_mailer.password_reset_missing_user.subject') expect(response).to redirect_to forgot_password_path end end diff --git a/spec/features/multiple_emails/reset_password_spec.rb b/spec/features/multiple_emails/reset_password_spec.rb index 9199a201ca2..0a2d5d64a66 100644 --- a/spec/features/multiple_emails/reset_password_spec.rb +++ b/spec/features/multiple_emails/reset_password_spec.rb @@ -30,16 +30,11 @@ ) end - scenario 'it sends the unconfirmed address email if the requested email is not confirmed' do + scenario 'it sends the missing user email if the requested email is not confirmed' do user = create(:user, :with_multiple_emails) unconfirmed_email_address = user.reload.email_addresses.last unconfirmed_email_address.update!(confirmed_at: nil) - create_account_instructions_text = I18n.t( - 'user_mailer.email_confirmation_instructions.first_sentence.forgot_password', - app_name: APP_NAME, - ) - visit root_path click_link t('links.passwords.forgot') fill_in t('account.index.email'), with: unconfirmed_email_address.email @@ -48,8 +43,7 @@ expect_delivered_email_count(1) expect_delivered_email( to: [unconfirmed_email_address.email], - subject: t('user_mailer.email_confirmation_instructions.email_not_found'), - body: [create_account_instructions_text], + subject: t('anonymous_mailer.password_reset_missing_user.subject'), ) end end diff --git a/spec/features/visitors/password_recovery_spec.rb b/spec/features/visitors/password_recovery_spec.rb index efaa5e59b32..7ef9da95dd3 100644 --- a/spec/features/visitors/password_recovery_spec.rb +++ b/spec/features/visitors/password_recovery_spec.rb @@ -38,26 +38,20 @@ end context 'user has confirmed email, but not set a password yet', email: true do - it 'shows the reset password form and confirms user after setting a password' do + it 'sends the missing user email' do user = create(:user, :unconfirmed) confirm_last_user reset_email - trigger_reset_password_and_click_email_link(user.email) - - expect(current_path).to eq edit_user_password_path - - password = 'NewVal!dPassw0rd' - fill_in t('forms.passwords.edit.labels.password'), with: password - fill_in t('components.password_confirmation.confirm_label'), - with: password - click_button t('forms.passwords.edit.buttons.submit') - - expect(current_path).to eq new_user_session_path - - fill_in_credentials_and_submit(user.email, 'NewVal!dPassw0rd') + visit new_user_password_path + fill_in t('account.index.email'), with: user.email + click_button t('forms.buttons.continue') - expect(current_path).to eq authentication_methods_setup_path + expect_delivered_email_count(1) + expect_delivered_email( + to: [user.email], + subject: t('anonymous_mailer.password_reset_missing_user.subject'), + ) end end diff --git a/spec/mailers/anonymous_mailer_spec.rb b/spec/mailers/anonymous_mailer_spec.rb new file mode 100644 index 00000000000..51cea1b94ee --- /dev/null +++ b/spec/mailers/anonymous_mailer_spec.rb @@ -0,0 +1,12 @@ +require 'rails_helper' + +RSpec.describe AnonymousMailer, type: :mailer do + let(:email) { 'email@example.com' } + let(:request_id) { SecureRandom.uuid } + + describe '#password_reset_missing_user' do + subject(:mail) { AnonymousMailer.with(email:).password_reset_missing_user(request_id:) } + + it_behaves_like 'a system email', synchronous_only: true + end +end diff --git a/spec/mailers/previews/anonymous_mailer_preview.rb b/spec/mailers/previews/anonymous_mailer_preview.rb new file mode 100644 index 00000000000..0346475d8a9 --- /dev/null +++ b/spec/mailers/previews/anonymous_mailer_preview.rb @@ -0,0 +1,15 @@ +class AnonymousMailerPreview < ActionMailer::Preview + def password_reset_missing_user + AnonymousMailer.with(email:).password_reset_missing_user(request_id:) + end + + private + + def email + 'email@example.com' + end + + def request_id + SecureRandom.uuid + end +end diff --git a/spec/mailers/previews/anonymous_mailer_preview_spec.rb b/spec/mailers/previews/anonymous_mailer_preview_spec.rb new file mode 100644 index 00000000000..3aa10c568dd --- /dev/null +++ b/spec/mailers/previews/anonymous_mailer_preview_spec.rb @@ -0,0 +1,18 @@ +require 'rails_helper' +require_relative './anonymous_mailer_preview' + +RSpec.describe AnonymousMailerPreview do + AnonymousMailerPreview.instance_methods(false).each do |mailer_method| + describe "##{mailer_method}" do + it 'generates a preview without raising an error' do + expect { AnonymousMailerPreview.new.public_send(mailer_method).body }.to_not raise_error + end + end + end + + it 'has a preview method for each mailer method' do + mailer_methods = AnonymousMailer.instance_methods(false) + preview_methods = AnonymousMailerPreview.instance_methods(false) + expect(mailer_methods - preview_methods).to be_empty + end +end diff --git a/spec/mailers/previews/user_mailer_preview.rb b/spec/mailers/previews/user_mailer_preview.rb index 7c1ad4a4a2b..d1ac134c690 100644 --- a/spec/mailers/previews/user_mailer_preview.rb +++ b/spec/mailers/previews/user_mailer_preview.rb @@ -4,24 +4,9 @@ def email_confirmation_instructions 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.with(user: user, email_address: email_address_record).unconfirmed_email_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 signup_with_your_email UserMailer.with(user: user, email_address: email_address_record).signup_with_your_email end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 9c4063bd135..a199fb88830 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -144,7 +144,6 @@ end describe '#email_confirmation_instructions' do - let(:instructions) { 'do the things' } let(:request_id) { '1234-abcd' } let(:token) { 'asdf123' } @@ -153,7 +152,6 @@ email_confirmation_instructions( token, request_id: request_id, - instructions: instructions, ) end diff --git a/spec/services/request_password_reset_spec.rb b/spec/services/request_password_reset_spec.rb index aaf42754c11..3fdeca4a58d 100644 --- a/spec/services/request_password_reset_spec.rb +++ b/spec/services/request_password_reset_spec.rb @@ -3,7 +3,9 @@ RSpec.describe RequestPasswordReset do describe '#perform' do let(:user) { create(:user) } - let(:email) { user.email_addresses.first.email } + let(:request_id) { SecureRandom.uuid } + let(:email_address) { user.email_addresses.first } + let(:email) { email_address.email } let(:irs_attempts_api_tracker) do instance_double( IrsAttemptsApi::Tracker, @@ -17,32 +19,24 @@ end context 'when the user is not found' do - it 'sends the account registration email' do + it 'sends the user missing email' do email = 'nonexistent@example.com' - send_sign_up_email_confirmation = instance_double(SendSignUpEmailConfirmation) - expect(send_sign_up_email_confirmation).to receive(:call).with( - hash_including( - instructions: I18n.t( - 'user_mailer.email_confirmation_instructions.first_sentence.forgot_password', - app_name: APP_NAME, - ), - ), - ) - expect(SendSignUpEmailConfirmation).to receive(:new).and_return( - send_sign_up_email_confirmation, - ) + mailer = instance_double(AnonymousMailer) + mail = instance_double(ActionMailer::MessageDelivery) + expect(AnonymousMailer).to receive(:with).with(email:).and_return(mailer) + expect(mailer).to receive(:password_reset_missing_user).with(request_id:).and_return(mail) + expect(mail).to receive(:deliver_now) RequestPasswordReset.new( email: email, irs_attempts_api_tracker: irs_attempts_api_tracker, + request_id: request_id, ).perform - user = User.find_with_email(email) - expect(user).to be_present end end - context 'when the user is found and confirmed' do + context 'when the user is found' do subject(:perform) do described_class.new( email: email, @@ -91,7 +85,7 @@ end end - context 'when the user is found and confirmed, but is suspended' do + context 'when the user is found, but is suspended' do subject(:perform) do described_class.new( email: email, @@ -172,37 +166,23 @@ context 'when the user is found and confirmed, but the email address is not' do let(:user) { create(:user, :with_multiple_emails) } - - let(:unconfirmed_email_address) do + let(:email_address) do user.reload.email_addresses.last.tap do |email_address| email_address.update!(confirmed_at: nil) end end - it 'sends the account registration email' do - send_sign_up_email_confirmation = instance_double(SendSignUpEmailConfirmation) - expect(send_sign_up_email_confirmation).to receive(:call).with( - hash_including( - instructions: I18n.t( - 'user_mailer.email_confirmation_instructions.first_sentence.forgot_password', - app_name: APP_NAME, - ), - ), - ) - expect(SendSignUpEmailConfirmation).to receive(:new).and_return( - send_sign_up_email_confirmation, - ) - - RequestPasswordReset.new( - email: unconfirmed_email_address.email, - ).perform - end - - it 'does not send a recovery activated push event' do - expect(PushNotification::HttpPush).to_not receive(:deliver) + it 'sends the user missing email' do + mailer = instance_double(AnonymousMailer) + mail = instance_double(ActionMailer::MessageDelivery) + expect(AnonymousMailer).to receive(:with).with(email:).and_return(mailer) + expect(mailer).to receive(:password_reset_missing_user).with(request_id:).and_return(mail) + expect(mail).to receive(:deliver_now) RequestPasswordReset.new( - email: unconfirmed_email_address.email, + email:, + irs_attempts_api_tracker:, + request_id:, ).perform 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 6312885d45a..77190028a36 100644 --- a/spec/services/send_sign_up_email_confirmation_spec.rb +++ b/spec/services/send_sign_up_email_confirmation_spec.rb @@ -5,7 +5,6 @@ let(:user) { create(:user, confirmed_at: nil) } let(:email_address) { user.email_addresses.take } let(:request_id) { '1234-abcd' } - let(:instructions) { 'do the things' } let(:confirmation_token) { 'confirm-me' } before do @@ -22,31 +21,15 @@ it 'sends the user an email with a confirmation link and the request id' do email_address.update!(confirmed_at: Time.zone.now) - subject.call(request_id: request_id, instructions: instructions) + subject.call(request_id: request_id) expect_delivered_email_count(1) expect_delivered_email( to: [user.email_addresses.first.email], subject: t('user_mailer.email_confirmation_instructions.subject'), - body: [request_id, instructions], + body: [request_id], ) 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 - subject.call( - request_id: request_id, - instructions: instructions, - password_reset_requested: true, - ) - - expect_delivered_email_count(1) - expect_delivered_email( - to: [email_address.email], - subject: t('user_mailer.email_confirmation_instructions.email_not_found'), - ) - end - end - it 'updates the confirmation values on the email address for the user' do subject.call(request_id: request_id) diff --git a/spec/views/account_reset/user_mailer/unconfirmed_email_instructions.html.erb_spec.rb b/spec/views/account_reset/user_mailer/unconfirmed_email_instructions.html.erb_spec.rb deleted file mode 100644 index fe5dd97e245..00000000000 --- a/spec/views/account_reset/user_mailer/unconfirmed_email_instructions.html.erb_spec.rb +++ /dev/null @@ -1,66 +0,0 @@ -require 'rails_helper' - -RSpec.describe 'user_mailer/unconfirmed_email_instructions.html.erb' do - it 'states that the email is not associated with a user account' do - user = build_stubbed(:user, confirmed_at: nil) - assign(:resource, user) - presenter = ConfirmationEmailPresenter.new(user, self) - assign(:confirmation_period, presenter.confirmation_period) - render - - expect(rendered).to have_content( - t( - 'user_mailer.email_confirmation_instructions.request_with_diff_email', - app_name: APP_NAME || 'login.gov', - ), - ) - - expect(rendered).to have_content( - t( - 'user_mailer.email_confirmation_instructions.footer', - confirmation_period: presenter.confirmation_period, - ), - ) - end - - it 'mentions how long the user has to confirm' do - user = build_stubbed(:user, confirmed_at: nil) - assign(:resource, user) - presenter = ConfirmationEmailPresenter.new(user, self) - assign(:confirmation_period, presenter.confirmation_period) - render - - expect(rendered).to have_content( - t( - 'user_mailer.email_confirmation_instructions.footer', - confirmation_period: presenter.confirmation_period, - ), - ) - end - - it 'includes a link to create another account with the email address' do - assign(:resource, build_stubbed(:user, confirmed_at: nil)) - assign(:token, 'foo') - render - - expect(rendered).to have_link( - t('user_mailer.email_confirmation_instructions.create_new_account'), - href: 'http://test.host/sign_up/email/confirm?confirmation_token=foo', - ) - end - - context 'in a non-default locale' do - before { assign(:locale, 'fr') } - - it 'puts the locale in the account creation URL' do - assign(:resource, build_stubbed(:user, confirmed_at: nil)) - assign(:token, 'foo') - render - - expect(rendered).to have_link( - t('user_mailer.email_confirmation_instructions.create_new_account'), - href: 'http://test.host/fr/sign_up/email/confirm?confirmation_token=foo', - ) - end - end -end diff --git a/spec/views/anonymous_mailer/password_reset_missing_user.html.erb_spec.rb b/spec/views/anonymous_mailer/password_reset_missing_user.html.erb_spec.rb new file mode 100644 index 00000000000..a3eb126a4ef --- /dev/null +++ b/spec/views/anonymous_mailer/password_reset_missing_user.html.erb_spec.rb @@ -0,0 +1,25 @@ +require 'rails_helper' + +RSpec.describe 'anonymous_mailer/password_reset_missing_user.html.erb' do + let(:request_id) { SecureRandom.uuid } + + before do + assign(:request_id, request_id) + end + + subject(:rendered) { render } + + it 'links to trying another email, maintaining request id' do + expect(rendered).to have_link( + t('anonymous_mailer.password_reset_missing_user.try_different_email'), + href: new_user_password_url(request_id:), + ) + end + + it 'links to create an account, maintaining request id' do + expect(rendered).to have_link( + t('anonymous_mailer.password_reset_missing_user.create_new_account'), + href: sign_up_register_url(request_id:), + ) + end +end diff --git a/spec/views/layouts/user_mailer.html.erb_spec.rb b/spec/views/layouts/mailer.html.erb_spec.rb similarity index 97% rename from spec/views/layouts/user_mailer.html.erb_spec.rb rename to spec/views/layouts/mailer.html.erb_spec.rb index e4d26801abf..b87977b1ca2 100644 --- a/spec/views/layouts/user_mailer.html.erb_spec.rb +++ b/spec/views/layouts/mailer.html.erb_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -RSpec.describe 'layouts/user_mailer.html.erb' do +RSpec.describe 'layouts/mailer.html.erb' do let(:user) { build_stubbed(:user) } before do