Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/controllers/concerns/two_factor_authenticatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion app/mailers/user_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions app/models/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions app/views/user_mailer/phone_added.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,5 @@ table.hr
| &nbsp;

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)))
1 change: 1 addition & 0 deletions config/locales/event_types/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions config/locales/event_types/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions config/locales/event_types/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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é
Expand Down
3 changes: 2 additions & 1 deletion config/locales/user_mailer/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion config/locales/user_mailer/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 3 additions & 2 deletions config/locales/user_mailer/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions spec/features/event_disavowal_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions spec/features/two_factor_authentication/change_factor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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
Expand Down
18 changes: 16 additions & 2 deletions spec/features/users/add_phone_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
3 changes: 2 additions & 1 deletion spec/mailers/user_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down