Skip to content

Commit

Permalink
Update UI around passcode delivery
Browse files Browse the repository at this point in the history
**Why**: a bit easier to use, and a more common pattern for
selecting passcode delivery methods
  • Loading branch information
Brendan Sudol committed Sep 6, 2016
1 parent 7943857 commit 4f8a5f8
Show file tree
Hide file tree
Showing 14 changed files with 75 additions and 51 deletions.
2 changes: 1 addition & 1 deletion app/forms/two_factor_setup_form.rb
Expand Up @@ -12,7 +12,7 @@ def submit(params)
self.phone = params[:phone].phony_formatted(
format: :international, normalize: :US, spaces: ' '
)
self.otp_method = params[:voice].present? ? :voice : :sms
self.otp_method = params[:otp_method] == 'voice' ? :voice : :sms

valid?
end
Expand Down
26 changes: 17 additions & 9 deletions app/views/devise/two_factor_authentication/show.html.slim
@@ -1,12 +1,20 @@
- title t('titles.two_factor_verification')

h1.heading = t('headings.choose_otp_delivery')

p#2fa-description = t('devise.two_factor_authentication.choose_otp_delivery', phone: @phone_number)

= button_to t('devise.two_factor_authentication.buttons.confirm_with_sms'),
otp_send_path, class: 'btn btn-primary mb2', method: :get, params: { otp_method: :sms }
= button_to t('devise.two_factor_authentication.buttons.confirm_with_voice'),
otp_send_path, class: 'btn btn-primary mb2', method: :get, params: { otp_method: :voice }

p.mt2.mb0.italic = t('devise.two_factor_authentication.otp_sms_disclaimer')
p#2fa-description
= t('devise.two_factor_authentication.choose_otp_delivery', phone: @phone_number)
= form_tag(otp_send_path, method: :get, role: 'form', class: 'mt3') do
.mb3
= label_tag 'otp_method',
t('devise.two_factor_authentication.otp_method.title'),
class: 'hide block caps ls-05 bold'
label.radio.mr2
= radio_button_tag :otp_method, :sms, checked: true
span.indicator
= t('devise.two_factor_authentication.otp_method.sms')
label.radio
= radio_button_tag :otp_method, :voice
span.indicator
= t('devise.two_factor_authentication.otp_method.voice')
= submit_tag t('forms.buttons.submit'), class: 'btn btn-primary btn-wide mb2'
p.mb1.italic = t('devise.two_factor_authentication.otp_sms_disclaimer')
20 changes: 14 additions & 6 deletions app/views/devise/two_factor_authentication_setup/index.html.slim
Expand Up @@ -10,10 +10,18 @@ h1.heading = t('devise.two_factor_authentication.two_factor_setup')
p#2fa-description = t('devise.two_factor_authentication.otp_setup')
= f.input :phone, as: :tel, label: 'Phone Number', required: true,
input_html: { 'aria-describedby' => '2fa-description' }
= f.button :submit, t('devise.two_factor_authentication.buttons.' \
'confirm_with_sms'), class: 'mb2',
name: 'two_factor_setup_form[sms]'
= f.button :submit, t('devise.two_factor_authentication.buttons.' \
'confirm_with_voice'), class: 'mb2',
name: 'two_factor_setup_form[voice]'
.mb2
= label_tag 'two_factor_setup_form[otp_method]',
t('devise.two_factor_authentication.otp_method.title'),
class: 'block caps ls-05 bold'
label.radio.mr2
= radio_button_tag 'two_factor_setup_form[otp_method]', :sms,
checked: true
span.indicator
= t('devise.two_factor_authentication.otp_method.sms')
label.radio
= radio_button_tag 'two_factor_setup_form[otp_method]', :voice
span.indicator
= t('devise.two_factor_authentication.otp_method.voice')
= f.button :submit, t('forms.buttons.submit'), class: 'mt2'
p.mt2.mb0.italic = t('devise.two_factor_authentication.otp_sms_disclaimer')
29 changes: 17 additions & 12 deletions app/views/shared/choose_delivery_method.html.slim
@@ -1,15 +1,20 @@
- title t('titles.edit_info.confirm_phone')

h1.heading = t('forms.phone_confirmation.header_text')

p#2fa-description = t('devise.two_factor_authentication.choose_delivery_confirmation',
phone: @phone_number)

= button_to t('devise.two_factor_authentication.buttons.' \
'confirm_with_sms'), phone_confirmation_send_path,
class: 'btn btn-primary mb2', method: :get, params: { otp_method: :sms }
= button_to t('devise.two_factor_authentication.buttons.' \
'confirm_with_voice'), phone_confirmation_send_path,
class: 'btn btn-primary mb2', method: :get, params: { otp_method: :voice }

p.mt2.mb0.italic = t('devise.two_factor_authentication.otp_sms_disclaimer')
p#2fa-description
= t('devise.two_factor_authentication.choose_delivery_confirmation', phone: @phone_number)
= form_tag(phone_confirmation_send_path, method: :get, role: 'form', class: 'mt3') do
.mb3
= label_tag 'otp_method',
t('devise.two_factor_authentication.otp_method.title'),
class: 'hide block caps ls-05 bold'
label.radio.mr2
= radio_button_tag :otp_method, :sms, checked: true
span.indicator
= t('devise.two_factor_authentication.otp_method.sms')
label.radio
= radio_button_tag :otp_method, :voice
span.indicator
= t('devise.two_factor_authentication.otp_method.voice')
= submit_tag t('forms.buttons.submit'), class: 'btn btn-primary btn-wide mb2'
p.mb1.italic = t('devise.two_factor_authentication.otp_sms_disclaimer')
4 changes: 4 additions & 0 deletions config/locales/devise.two_factor_authentication.en.yml
Expand Up @@ -35,3 +35,7 @@ en:
buttons:
confirm_with_sms: Confirm with text message
confirm_with_voice: Confirm with voice call
otp_method:
title: Delivery method
sms: Text message (SMS)
voice: Phone call
1 change: 1 addition & 0 deletions config/locales/en.yml
Expand Up @@ -22,6 +22,7 @@ en:
resend_confirmation: Resend confirmation instructions
reset_password: Send reset password instructions
setup_totp: Set up authentication app
submit: Submit
required_field: Indicates a required field.
phone_confirmation:
header_text: Confirm your phone number
Expand Down
Expand Up @@ -36,7 +36,7 @@
patch(
:set,
two_factor_setup_form: { phone: '703-555-0100',
voice: 'Confirm with voice call' }
otp_method: 'voice' }
)

expect(response).to redirect_to(
Expand Down
4 changes: 2 additions & 2 deletions spec/features/saml/saml_spec.rb
Expand Up @@ -35,7 +35,7 @@

it 'prompts the user to confirm phone after setting up 2FA' do
fill_in 'Phone', with: '202-555-1212'
click_button t('devise.two_factor_authentication.buttons.confirm_with_sms')
click_button t('forms.buttons.submit')

expect(current_path).to eq phone_confirmation_path
end
Expand All @@ -53,7 +53,7 @@

it 'prompts the user to confirm phone after setting up 2FA' do
fill_in 'Phone', with: '202-555-1212'
click_button t('devise.two_factor_authentication.buttons.confirm_with_sms')
click_button t('forms.buttons.submit')

expect(current_path).to eq phone_confirmation_path
end
Expand Down
16 changes: 8 additions & 8 deletions spec/features/two_factor_authentication/sign_in_spec.rb
Expand Up @@ -17,7 +17,7 @@

scenario 'user does not fill out a phone number when signing up' do
sign_up_and_set_password
click_button t('devise.two_factor_authentication.buttons.confirm_with_sms')
click_button t('forms.buttons.submit')

expect(current_path).to eq phone_setup_path
end
Expand All @@ -34,23 +34,23 @@
scenario 'user leaves phone blank' do
sign_in_before_2fa
fill_in 'Phone', with: ''
click_button t('devise.two_factor_authentication.buttons.confirm_with_sms')
click_button t('forms.buttons.submit')

expect(page).to have_content invalid_phone_message
end

scenario 'user enters an invalid number with no digits' do
sign_in_before_2fa
fill_in 'Phone', with: 'five one zero five five five four three two one'
click_button t('devise.two_factor_authentication.buttons.confirm_with_sms')
click_button t('forms.buttons.submit')

expect(page).to have_content invalid_phone_message
end

scenario 'user enters a valid number' do
user = sign_in_before_2fa
fill_in 'Phone', with: '555-555-1212'
click_button t('devise.two_factor_authentication.buttons.confirm_with_sms')
click_button t('forms.buttons.submit')

expect(page).to_not have_content invalid_phone_message
expect(current_path).to eq phone_confirmation_path
Expand All @@ -73,7 +73,7 @@
@user = create(:user, :signed_up)
reset_email
sign_in_before_2fa(@user)
click_button t('devise.two_factor_authentication.buttons.confirm_with_sms')
click_button t('forms.buttons.submit')
end

it 'lets the user know they are signed in' do
Expand Down Expand Up @@ -107,7 +107,7 @@
scenario 'user can resend one-time password (OTP)' do
user = create(:user, :signed_up)
sign_in_before_2fa(user)
click_button t('devise.two_factor_authentication.buttons.confirm_with_sms')
click_button t('forms.buttons.submit')
click_link 'Resend'

expect(page).to have_content t('devise.two_factor_authentication.user.new_otp_sent')
Expand All @@ -116,7 +116,7 @@
scenario 'user who enters OTP incorrectly 3 times is locked out for OTP validity period' do
user = create(:user, :signed_up)
sign_in_before_2fa(user)
click_button t('devise.two_factor_authentication.buttons.confirm_with_sms')
click_button t('forms.buttons.submit')

3.times do
fill_in('code', with: 'bad-code')
Expand All @@ -129,7 +129,7 @@
user.update(second_factor_locked_at: Time.zone.now - (Devise.direct_otp_valid_for + 1.second))

sign_in_before_2fa(user)
click_button t('devise.two_factor_authentication.buttons.confirm_with_sms')
click_button t('forms.buttons.submit')

expect(page).to have_content t('devise.two_factor_authentication.header_text')
end
Expand Down
2 changes: 1 addition & 1 deletion spec/features/visitors/password_recovery_spec.rb
Expand Up @@ -161,7 +161,7 @@ def reset_password_and_sign_back_in(user)

it 'redirects user to profile after signing back in' do
reset_password_and_sign_back_in(@user)
click_button t('devise.two_factor_authentication.buttons.confirm_with_sms')
click_button t('forms.buttons.submit')
fill_in 'code', with: @user.reload.direct_otp
click_button 'Submit'

Expand Down
4 changes: 2 additions & 2 deletions spec/features/visitors/sign_up_spec.rb
Expand Up @@ -57,7 +57,7 @@
allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true)
@user = sign_in_before_2fa
fill_in 'Phone', with: '555-555-5555'
click_button t('devise.two_factor_authentication.buttons.confirm_with_sms')
click_button t('forms.buttons.submit')
end

it 'updates phone_confirmed_at and redirects to profile after confirmation' do
Expand Down Expand Up @@ -100,7 +100,7 @@
@existing_user = create(:user, :signed_up)
@user = sign_in_before_2fa
fill_in 'Phone', with: @existing_user.phone
click_button t('devise.two_factor_authentication.buttons.confirm_with_sms')
click_button t('forms.buttons.submit')
end

it 'pretends the phone is valid and prompts to confirm the number' do
Expand Down
4 changes: 2 additions & 2 deletions spec/forms/two_factor_setup_form_spec.rb
Expand Up @@ -25,7 +25,7 @@
context 'when voice is selected' do
before do
subject.submit(phone: valid_phone,
voice: 'Confirm with voice message')
otp_method: 'voice')
end

it 'sets otp_method to "voice"' do
Expand All @@ -36,7 +36,7 @@
context 'when SMS is selected' do
before do
subject.submit(phone: valid_phone,
sms: 'Confirm with text message')
otp_method: 'sms')
end

it 'sets otp_method to "sms"' do
Expand Down
6 changes: 3 additions & 3 deletions spec/support/features/session_helper.rb
Expand Up @@ -21,7 +21,7 @@ def sign_up_and_set_password
user = create(:user, :unconfirmed)
confirm_last_user
fill_in 'password_form_password', with: VALID_PASSWORD
click_button 'Submit'
click_button t('forms.buttons.submit')
user
end

Expand Down Expand Up @@ -69,8 +69,8 @@ def sign_up_and_2fa
allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true)
user = sign_up_and_set_password
fill_in 'Phone', with: '202-555-1212'
click_button t('devise.two_factor_authentication.buttons.confirm_with_sms')
click_button 'Submit'
click_button t('forms.buttons.submit')
click_button t('forms.buttons.submit')
user
end
end
Expand Down
Expand Up @@ -14,10 +14,8 @@
allow(view).to receive(:current_user).and_return(user)
render

expect(rendered).to have_button(t('devise.two_factor_authentication.' \
'buttons.confirm_with_sms'))
expect(rendered).to have_button(t('devise.two_factor_authentication.' \
'buttons.confirm_with_voice'))
expect(rendered).to have_content t('devise.two_factor_authentication.otp_method.sms')
expect(rendered).to have_content t('devise.two_factor_authentication.otp_method.voice')
end

it 'informs the user that an OTP will be sent to their number' do
Expand Down

0 comments on commit 4f8a5f8

Please sign in to comment.