diff --git a/app/controllers/concerns/two_factor_authenticatable.rb b/app/controllers/concerns/two_factor_authenticatable.rb index 3b488f11160..0e35844b38b 100644 --- a/app/controllers/concerns/two_factor_authenticatable.rb +++ b/app/controllers/concerns/two_factor_authenticatable.rb @@ -159,8 +159,9 @@ def phone_confirmed end 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(email_address).deliver_later + UserMailer.phone_added(email_address, disavowal_token: event.disavowal_token).deliver_later end end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 5df4577cd1b..718d6b5866a 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -32,7 +32,8 @@ def password_changed(email_address, disavowal_token:) mail(to: email_address.email, subject: t('devise.mailer.password_updated.subject')) end - def phone_added(email_address) + def phone_added(email_address, disavowal_token:) + @disavowal_token = disavowal_token mail(to: email_address.email, subject: t('user_mailer.phone_added.subject')) end diff --git a/app/models/event.rb b/app/models/event.rb index bc8bb208a74..573c8fc50f3 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -25,6 +25,7 @@ class Event < ApplicationRecord sign_in_before_2fa: 18, sign_in_after_2fa: 19, email_deleted: 20, + phone_added: 21, } validates :event_type, presence: true diff --git a/app/views/user_mailer/phone_added.html.slim b/app/views/user_mailer/phone_added.html.slim index a427a86574f..c9ab448a426 100644 --- a/app/views/user_mailer/phone_added.html.slim +++ b/app/views/user_mailer/phone_added.html.slim @@ -11,6 +11,5 @@ table.hr |   p == t('.help', - app: link_to(APP_NAME, Figaro.env.mailer_domain_name, class: 'gray'), - help_link: link_to(t('user_mailer.help_link_text'), MarketingSite.help_url), - contact_link: link_to(t('user_mailer.contact_link_text'), MarketingSite.contact_url)) + disavowal_link: link_to(t('.disavowal_link'), + event_disavowal_url(disavowal_token: @disavowal_token))) diff --git a/config/locales/event_types/en.yml b/config/locales/event_types/en.yml index 3c3069494b9..20b665f734b 100644 --- a/config/locales/event_types/en.yml +++ b/config/locales/event_types/en.yml @@ -14,6 +14,7 @@ en: new_personal_key: Personal key changed password_changed: Password changed personal_key_used: Personal key used to sign in + phone_added: Phone number added phone_changed: Phone number changed phone_confirmed: Phone confirmed phone_removed: Phone number removed diff --git a/config/locales/event_types/es.yml b/config/locales/event_types/es.yml index 4d17bfa99f4..6e813d92f7f 100644 --- a/config/locales/event_types/es.yml +++ b/config/locales/event_types/es.yml @@ -14,6 +14,7 @@ es: new_personal_key: Clave personal cambiado password_changed: Contraseña cambiada personal_key_used: Clave personal utilizada para iniciar sesión + phone_added: Teléfono añadido phone_changed: Número de teléfono cambiado phone_confirmed: Teléfono confirmado phone_removed: Teléfono eliminado diff --git a/config/locales/event_types/fr.yml b/config/locales/event_types/fr.yml index a44492f2806..af0c0b4d00f 100644 --- a/config/locales/event_types/fr.yml +++ b/config/locales/event_types/fr.yml @@ -14,6 +14,7 @@ fr: new_personal_key: Clé personnelle modifié password_changed: Mot de passe modifié personal_key_used: Clé personnelle utilisée pour la connexion + phone_added: Numéro de téléphone ajouté phone_changed: Numéro de téléphone modifié phone_confirmed: Numéro de téléphone confirmé phone_removed: Numéro de téléphone supprimé diff --git a/config/locales/user_mailer/en.yml b/config/locales/user_mailer/en.yml index 39bcf23bd6b..db96e693439 100644 --- a/config/locales/user_mailer/en.yml +++ b/config/locales/user_mailer/en.yml @@ -131,8 +131,9 @@ en: intro: Personal key used to sign in subject: Account Security Alert phone_added: + disavowal_link: reset your password help: If you did not make this change, sign in to your profile and manage your - phone numbers. We recommend that you also change your password. + phone numbers. We also recommend that you %{disavowal_link}. intro: A new phone number was added to your %{app} profile. subject: New phone number added please_reset_password: diff --git a/config/locales/user_mailer/es.yml b/config/locales/user_mailer/es.yml index 9181dbdd636..c165d526df4 100644 --- a/config/locales/user_mailer/es.yml +++ b/config/locales/user_mailer/es.yml @@ -135,8 +135,9 @@ es: intro: Clave personal utilizada para iniciar sesión subject: Alerta de seguridad de cuenta phone_added: + disavowal_link: restablezca su contraseña help: Si no realizó este cambio, inicie sesión en su perfil y administre sus - números de teléfono. Le recomendamos que también cambie su contraseña. + números de teléfono. También le recomendamos que %{disavowal_link}. intro: Se agregó un nuevo número de teléfono a su perfil de %{app}. subject: Nuevo número de teléfono añadido please_reset_password: diff --git a/config/locales/user_mailer/fr.yml b/config/locales/user_mailer/fr.yml index 42add87ad3a..805b3e473bb 100644 --- a/config/locales/user_mailer/fr.yml +++ b/config/locales/user_mailer/fr.yml @@ -143,9 +143,10 @@ fr: intro: La clé personnelle utilisée pour vous connecter subject: Alerte de sécurité du compte phone_added: + disavowal_link: réinitialiser votre mot de passe help: Si vous n'avez pas apporté cette modification, connectez-vous à votre - profil et gérez vos numéros de téléphone. Nous vous recommandons de changer - également votre mot de passe. + profil et gérez vos numéros de téléphone. Nous vous recommandons également + de %{disavowal_link}. intro: Un nouveau numéro de téléphone a été ajouté à votre profil %{app}. subject: Nouveau numéro de téléphone ajouté please_reset_password: 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 8de8f0b267e..1bed2f06675 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -290,7 +290,8 @@ allow(subject).to receive(:create_user_event) @mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) subject.current_user.email_addresses.each do |email_address| - allow(UserMailer).to receive(:phone_added).with(email_address). + allow(UserMailer).to receive(:phone_added). + with(email_address, disavowal_token: instance_of(String)). and_return(@mailer) end @previous_phone = MfaContext.new(subject.current_user).phone_configurations.first&.phone @@ -329,7 +330,8 @@ 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(email_address) + expect(UserMailer).to have_received(:phone_added). + with(email_address, disavowal_token: instance_of(String)) end expect(@mailer).to have_received(:deliver_later) end diff --git a/spec/features/event_disavowal_spec.rb b/spec/features/event_disavowal_spec.rb index a227b692b09..06eb73ab8d0 100644 --- a/spec/features/event_disavowal_spec.rb +++ b/spec/features/event_disavowal_spec.rb @@ -39,6 +39,21 @@ disavow_last_action_and_reset_password end + scenario 'disavowing a phone being added' do + sign_in_and_2fa_user(user) + visit add_phone_path + + fill_in 'user_phone_form[phone]', with: '202-555-3434' + + choose 'user_phone_form_otp_delivery_preference_sms' + check 'user_phone_form_otp_make_default_number' + click_button t('forms.buttons.continue') + + submit_prefilled_otp_code(user, 'sms') + + disavow_last_action_and_reset_password + end + scenario 'attempting to disavow an event with an invalid disavowal token' do visit event_disavowal_path(disavowal_token: 'this is a totally fake token') @@ -112,6 +127,13 @@ expect(page.current_path).to eq(login_two_factor_path(otp_delivery_preference: :sms)) end + def submit_prefilled_otp_code(user, delivery_preference) + expect(current_path). + to eq login_two_factor_path(otp_delivery_preference: delivery_preference) + fill_in('code', with: user.reload.direct_otp) + click_button t('forms.buttons.submit.default') + end + def perform_disavowable_password_reset visit forgot_password_path fill_in :password_reset_email_form_email, with: user.email diff --git a/spec/features/two_factor_authentication/change_factor_spec.rb b/spec/features/two_factor_authentication/change_factor_spec.rb index e3ebeadb898..f4cadf90007 100644 --- a/spec/features/two_factor_authentication/change_factor_spec.rb +++ b/spec/features/two_factor_authentication/change_factor_spec.rb @@ -29,7 +29,9 @@ mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) user.email_addresses.each do |email_address| - allow(UserMailer).to receive(:phone_added).with(email_address).and_return(mailer) + allow(UserMailer).to receive(:phone_added). + with(email_address, hash_including(:disavowal_token)). + and_return(mailer) end @previous_phone_confirmed_at = @@ -59,7 +61,8 @@ expect(current_path).to eq account_path user.email_addresses.each do |email_address| - expect(UserMailer).to have_received(:phone_added).with(email_address) + expect(UserMailer).to have_received(:phone_added). + with(email_address, hash_including(:disavowal_token)) end expect(mailer).to have_received(:deliver_later) expect(page).to have_content new_phone diff --git a/spec/features/users/add_phone_spec.rb b/spec/features/users/add_phone_spec.rb index 86183e71cbd..0ab175ed028 100644 --- a/spec/features/users/add_phone_spec.rb +++ b/spec/features/users/add_phone_spec.rb @@ -5,8 +5,6 @@ user = create(:user, :signed_up) phone = '+1 (225) 278-1234' - expect(UserMailer).to receive(:phone_added).with(user.email_addresses.first).and_call_original - sign_in_and_2fa_user(user) click_on t('account.index.phone_add') fill_in :user_phone_form_phone, with: phone @@ -19,4 +17,20 @@ expect(user.phone_configurations[0].confirmed_at).to be_present expect(user.phone_configurations[1].confirmed_at).to be_present end + + scenario 'adding a new phone number sends the user an email with a disavowal link' do + user = create(:user, :signed_up) + phone = '+1 (225) 278-1234' + + expect(UserMailer).to receive(:phone_added). + with(user.email_addresses.first, hash_including(:disavowal_token)). + and_call_original + + sign_in_and_2fa_user(user) + click_on t('account.index.phone_add') + fill_in :user_phone_form_phone, with: phone + click_continue + fill_in_code_with_last_phone_otp + click_submit_default + end end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 6d6705c6b72..b4faa528091 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -191,7 +191,8 @@ end describe 'phone_added' do - let(:mail) { UserMailer.phone_added(email_address) } + disavowal_token = 'i_am_disavowal_token' + let(:mail) { UserMailer.phone_added(email_address, disavowal_token: disavowal_token) } it_behaves_like 'a system email'