diff --git a/app/assets/images/2FA-sms.svg b/app/assets/images/2FA-sms.svg new file mode 100644 index 00000000000..07817c9c190 --- /dev/null +++ b/app/assets/images/2FA-sms.svg @@ -0,0 +1 @@ +2FA-text-message \ No newline at end of file diff --git a/app/assets/images/2FA-voice.svg b/app/assets/images/2FA-voice.svg new file mode 100644 index 00000000000..951120fb53a --- /dev/null +++ b/app/assets/images/2FA-voice.svg @@ -0,0 +1 @@ +2FA-phone-call \ No newline at end of file diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 27a4cd2ea96..aa469a2e397 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -164,7 +164,7 @@ def confirm_two_factor_authenticated end def prompt_to_set_up_2fa - redirect_to phone_setup_url + redirect_to two_factor_options_url end def prompt_to_enter_otp diff --git a/app/controllers/concerns/unconfirmed_user_concern.rb b/app/controllers/concerns/unconfirmed_user_concern.rb index 544dd5b4a47..432c1ee418e 100644 --- a/app/controllers/concerns/unconfirmed_user_concern.rb +++ b/app/controllers/concerns/unconfirmed_user_concern.rb @@ -43,7 +43,7 @@ def after_confirmation_url_for(user) elsif user.two_factor_enabled? account_url else - phone_setup_url + two_factor_options_url end end diff --git a/app/controllers/users/phone_setup_controller.rb b/app/controllers/users/phone_setup_controller.rb new file mode 100644 index 00000000000..4fd8c903220 --- /dev/null +++ b/app/controllers/users/phone_setup_controller.rb @@ -0,0 +1,45 @@ +module Users + class PhoneSetupController < ApplicationController + include UserAuthenticator + include PhoneConfirmation + + before_action :authenticate_user + before_action :authorize_phone_setup + + def index + @user_phone_form = UserPhoneForm.new(current_user) + @presenter = PhoneSetupPresenter.new(current_user.otp_delivery_preference) + analytics.track_event(Analytics::USER_REGISTRATION_PHONE_SETUP_VISIT) + end + + def create + @user_phone_form = UserPhoneForm.new(current_user) + @presenter = PhoneSetupPresenter.new(current_user.otp_delivery_preference) + result = @user_phone_form.submit(user_phone_form_params) + analytics.track_event(Analytics::MULTI_FACTOR_AUTH_PHONE_SETUP, result.to_h) + + if result.success? + prompt_to_confirm_phone(phone: @user_phone_form.phone) + else + render :index + end + end + + private + + def authorize_phone_setup + if user_fully_authenticated? + redirect_to account_url + elsif current_user.two_factor_enabled? + redirect_to user_two_factor_authentication_url + end + end + + def user_phone_form_params + params.require(:user_phone_form).permit( + :international_code, + :phone + ) + end + end +end diff --git a/app/controllers/users/phones_controller.rb b/app/controllers/users/phones_controller.rb index d6723c4a15d..5db18b67791 100644 --- a/app/controllers/users/phones_controller.rb +++ b/app/controllers/users/phones_controller.rb @@ -6,11 +6,12 @@ class PhonesController < ReauthnRequiredController def edit @user_phone_form = UserPhoneForm.new(current_user) + @presenter = PhoneSetupPresenter.new(current_user.otp_delivery_preference) end def update @user_phone_form = UserPhoneForm.new(current_user) - + @presenter = PhoneSetupPresenter.new(current_user) if @user_phone_form.submit(user_params).success? process_updates bypass_sign_in current_user diff --git a/app/controllers/users/two_factor_authentication_controller.rb b/app/controllers/users/two_factor_authentication_controller.rb index be08ca09c88..1947ab7941c 100644 --- a/app/controllers/users/two_factor_authentication_controller.rb +++ b/app/controllers/users/two_factor_authentication_controller.rb @@ -10,7 +10,7 @@ def show elsif current_user.two_factor_enabled? validate_otp_delivery_preference_and_send_code else - redirect_to phone_setup_url + redirect_to two_factor_options_url end end diff --git a/app/controllers/users/two_factor_authentication_setup_controller.rb b/app/controllers/users/two_factor_authentication_setup_controller.rb index a6e1d294e3b..9501da62289 100644 --- a/app/controllers/users/two_factor_authentication_setup_controller.rb +++ b/app/controllers/users/two_factor_authentication_setup_controller.rb @@ -1,21 +1,19 @@ module Users class TwoFactorAuthenticationSetupController < ApplicationController include UserAuthenticator - include PhoneConfirmation - before_action :authorize_otp_setup before_action :authenticate_user + before_action :authorize_2fa_setup def index - @user_phone_form = UserPhoneForm.new(current_user) - analytics.track_event(Analytics::USER_REGISTRATION_PHONE_SETUP_VISIT) + @two_factor_options_form = TwoFactorOptionsForm.new(current_user) + analytics.track_event(Analytics::USER_REGISTRATION_2FA_SETUP_VISIT) end - def set - @user_phone_form = UserPhoneForm.new(current_user) - result = @user_phone_form.submit(params[:user_phone_form]) - - analytics.track_event(Analytics::MULTI_FACTOR_AUTH_PHONE_SETUP, result.to_h) + def create + @two_factor_options_form = TwoFactorOptionsForm.new(current_user) + result = @two_factor_options_form.submit(two_factor_options_form_params) + analytics.track_event(Analytics::USER_REGISTRATION_2FA_SETUP, result.to_h) if result.success? process_valid_form @@ -26,16 +24,25 @@ def set private - def authorize_otp_setup + def authorize_2fa_setup if user_fully_authenticated? - redirect_to(request.referer || root_url) - elsif current_user&.two_factor_enabled? + redirect_to account_url + elsif current_user.two_factor_enabled? redirect_to user_two_factor_authentication_url end end def process_valid_form - prompt_to_confirm_phone(phone: @user_phone_form.phone) + case @two_factor_options_form.selection + when 'sms', 'voice' + redirect_to phone_setup_url + when 'auth_app' + redirect_to authenticator_setup_url + end + end + + def two_factor_options_form_params + params.require(:two_factor_options_form).permit(:selection) end end end diff --git a/app/forms/two_factor_options_form.rb b/app/forms/two_factor_options_form.rb new file mode 100644 index 00000000000..5473549adb1 --- /dev/null +++ b/app/forms/two_factor_options_form.rb @@ -0,0 +1,42 @@ +class TwoFactorOptionsForm + include ActiveModel::Model + + attr_reader :selection + + validates :selection, inclusion: { in: %w[voice sms auth_app piv_cac] } + + def initialize(user) + self.user = user + end + + def submit(params) + self.selection = params[:selection] + + success = valid? + + update_otp_delivery_preference_for_user if success && user_needs_updating? + + FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes) + end + + private + + attr_accessor :user + attr_writer :selection + + def extra_analytics_attributes + { + selection: selection, + } + end + + def user_needs_updating? + return false unless %w[voice sms].include?(selection) + selection != user.otp_delivery_preference + end + + def update_otp_delivery_preference_for_user + user_attributes = { otp_delivery_preference: selection } + UpdateUser.new(user: user, attributes: user_attributes).call + end +end diff --git a/app/forms/user_phone_form.rb b/app/forms/user_phone_form.rb index 8a3c0632fd3..fca174eebc3 100644 --- a/app/forms/user_phone_form.rb +++ b/app/forms/user_phone_form.rb @@ -3,6 +3,8 @@ class UserPhoneForm include FormPhoneValidator include OtpDeliveryPreferenceValidator + validates :otp_delivery_preference, inclusion: { in: %w[voice sms] } + attr_accessor :phone, :international_code, :otp_delivery_preference def initialize(user) @@ -16,9 +18,10 @@ def submit(params) ingest_submitted_params(params) success = valid? - self.phone = submitted_phone unless success - update_otp_delivery_preference_for_user if otp_delivery_preference_changed? && success + + update_otp_delivery_preference_for_user if + success && otp_delivery_preference.present? && otp_delivery_preference_changed? FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes) end @@ -44,7 +47,10 @@ def ingest_submitted_params(params) submitted_phone, country_code: international_code ) - self.otp_delivery_preference = params[:otp_delivery_preference] + + tfa_prefs = params[:otp_delivery_preference] + + self.otp_delivery_preference = tfa_prefs if tfa_prefs end def otp_delivery_preference_changed? diff --git a/app/javascript/app/form-field-format.js b/app/javascript/app/form-field-format.js index 8f13ce3e031..b28edc7f89e 100644 --- a/app/javascript/app/form-field-format.js +++ b/app/javascript/app/form-field-format.js @@ -1,6 +1,5 @@ import { SocialSecurityNumberFormatter, TextField } from 'field-kit'; import DateFormatter from './modules/date-formatter'; -import InternationalPhoneFormatter from './modules/international-phone-formatter'; import NumericFormatter from './modules/numeric-formatter'; import PersonalKeyFormatter from './modules/personal-key-formatter'; import USPhoneFormatter from './modules/us-phone-formatter'; @@ -11,7 +10,6 @@ function formatForm() { const formats = [ ['.dob', new DateFormatter()], ['.mfa', new NumericFormatter()], - ['.phone', new InternationalPhoneFormatter()], ['.us-phone', new USPhoneFormatter()], ['.personal-key', new PersonalKeyFormatter()], ['.ssn', new SocialSecurityNumberFormatter()], diff --git a/app/javascript/app/modules/international-phone-formatter.js b/app/javascript/app/modules/international-phone-formatter.js deleted file mode 100644 index 6f4376a3a46..00000000000 --- a/app/javascript/app/modules/international-phone-formatter.js +++ /dev/null @@ -1,77 +0,0 @@ -import { Formatter } from 'field-kit'; -import { asYouType as AsYouType } from 'libphonenumber-js'; - -const INTERNATIONAL_CODE_REGEX = /^\+\d{1,3} /; - -const fixCountryCodeSpacing = (text, countryCode) => { - // If the text is `+123456`, make it `+123 456` - if (text[countryCode.length + 1] !== ' ') { - return text.replace(`+${countryCode}`, `+${countryCode} `); - } - return text; -}; - -const getFormattedTextData = (text) => { - if (text === '1') { - text = '+1'; - } - - const asYouType = new AsYouType('US'); - let formattedText = asYouType.input(text); - const countryCode = asYouType.country_phone_code; - - if (asYouType.country_phone_code) { - formattedText = fixCountryCodeSpacing(formattedText, countryCode); - } - - return { - text: formattedText, - template: asYouType.template, - countryCode, - }; -}; - -const changeRemovesInternationalCode = (current, previous) => { - if (previous.text.match(INTERNATIONAL_CODE_REGEX) && - !current.text.match(INTERNATIONAL_CODE_REGEX) - ) { - return true; - } - return false; -}; - -const cursorPosition = (formattedTextData) => { - // If the text is `(23 )` the cursor goes after the 3 - const match = formattedTextData.text.match(/\d[^\d]*$/); - if (match) { - return match.index + 1; - } - return formattedTextData.text.length + 1; -}; - -class InternationalPhoneFormatter extends Formatter { - format(text) { - const formattedTextData = getFormattedTextData(text); - return super.format(formattedTextData.text); - } - - // eslint-disable-next-line class-methods-use-this - parse(text) { - return text.replace(/[^\d+]/g, ''); - } - - isChangeValid(change, error) { - const formattedTextData = getFormattedTextData(change.proposed.text); - const previousFormattedTextData = getFormattedTextData(change.current.text); - - if (changeRemovesInternationalCode(formattedTextData, previousFormattedTextData)) { - return false; - } - - change.proposed.text = formattedTextData.text; - change.proposed.selectedRange.start = cursorPosition(formattedTextData); - return super.isChangeValid(change, error); - } -} - -export default InternationalPhoneFormatter; diff --git a/app/javascript/app/phone-internationalization.js b/app/javascript/app/phone-internationalization.js index 167b2c3c618..5aa75d8ab21 100644 --- a/app/javascript/app/phone-internationalization.js +++ b/app/javascript/app/phone-internationalization.js @@ -76,7 +76,7 @@ const updateOTPDeliveryMethods = () => { return; } - const phoneInput = document.querySelector('[data-international-phone-form] .phone') || document.querySelector('[data-international-phone-form] .new-phone'); + const phoneInput = document.querySelector('[data-international-phone-form] .phone'); const phoneLabel = phoneRadio.parentNode.parentNode; const deliveryMethodHint = document.querySelector('#otp_delivery_preference_instruction'); const optPhoneLabelInfo = document.querySelector('#otp_phone_label_info'); @@ -111,7 +111,7 @@ const updateInternationalCodeInPhone = (phone, newCode) => { }; const updateInternationalCodeInput = () => { - const phoneInput = document.querySelector('[data-international-phone-form] .phone') || document.querySelector('[data-international-phone-form] .new-phone'); + const phoneInput = document.querySelector('[data-international-phone-form] .phone'); const phone = phoneInput.value; const inputInternationalCode = internationalCodeFromPhone(phone); const selectedInternationalCode = selectedInternationCodeOption().dataset.countryCode; @@ -122,7 +122,7 @@ const updateInternationalCodeInput = () => { }; document.addEventListener('DOMContentLoaded', () => { - const phoneInput = document.querySelector('[data-international-phone-form] .phone') || document.querySelector('[data-international-phone-form] .new-phone'); + const phoneInput = document.querySelector('[data-international-phone-form] .phone'); const codeInput = document.querySelector('[data-international-phone-form] .international-code'); if (phoneInput) { phoneInput.addEventListener('countryChange', updateOTPDeliveryMethods); diff --git a/app/presenters/phone_setup_presenter.rb b/app/presenters/phone_setup_presenter.rb new file mode 100644 index 00000000000..f6b55f8db5b --- /dev/null +++ b/app/presenters/phone_setup_presenter.rb @@ -0,0 +1,25 @@ +class PhoneSetupPresenter + include ActionView::Helpers::TranslationHelper + + attr_reader :otp_delivery_preference + + def initialize(otp_delivery_preference) + @otp_delivery_preference = otp_delivery_preference + end + + def heading + t("titles.phone_setup.#{otp_delivery_preference}") + end + + def label + t("devise.two_factor_authentication.phone_#{otp_delivery_preference}_label") + end + + def info + t("devise.two_factor_authentication.phone_#{otp_delivery_preference}_info_html") + end + + def image + "2FA-#{otp_delivery_preference}.svg" + end +end diff --git a/app/services/analytics.rb b/app/services/analytics.rb index 273416409d4..4bbd34ba5fe 100644 --- a/app/services/analytics.rb +++ b/app/services/analytics.rb @@ -109,6 +109,8 @@ def browser USER_REGISTRATION_EMAIL_CONFIRMATION_RESEND = 'User Registration: Email Confirmation requested due to invalid token'.freeze USER_REGISTRATION_ENTER_EMAIL_VISIT = 'User Registration: enter email visited'.freeze USER_REGISTRATION_INTRO_VISIT = 'User Registration: intro visited'.freeze + USER_REGISTRATION_2FA_SETUP = 'User Registration: 2FA Setup'.freeze + USER_REGISTRATION_2FA_SETUP_VISIT = 'User Registration: 2FA Setup visited'.freeze USER_REGISTRATION_PHONE_SETUP_VISIT = 'User Registration: phone setup visited'.freeze USER_REGISTRATION_PERSONAL_KEY_VISIT = 'User Registration: personal key visited'.freeze USER_REGISTRATION_PIV_CAC_DISABLED = 'User Registration: piv cac disabled'.freeze diff --git a/app/validators/otp_delivery_preference_validator.rb b/app/validators/otp_delivery_preference_validator.rb index b6f8ae643e2..42a22dc404f 100644 --- a/app/validators/otp_delivery_preference_validator.rb +++ b/app/validators/otp_delivery_preference_validator.rb @@ -5,16 +5,26 @@ module OtpDeliveryPreferenceValidator validate :otp_delivery_preference_supported end + def otp_delivery_preference_supported? + return true unless otp_delivery_preference == 'voice' + !phone_number_capabilities.sms_only? + end + def otp_delivery_preference_supported - capabilities = PhoneNumberCapabilities.new(phone) - return unless otp_delivery_preference == 'voice' && capabilities.sms_only? + return if otp_delivery_preference_supported? errors.add( :phone, I18n.t( 'devise.two_factor_authentication.otp_delivery_preference.phone_unsupported', - location: capabilities.unsupported_location + location: phone_number_capabilities.unsupported_location ) ) end + + private + + def phone_number_capabilities + @phone_number_capabilities ||= PhoneNumberCapabilities.new(phone) + end end diff --git a/app/views/shared/_cancel_or_back_to_options.html.slim b/app/views/shared/_cancel_or_back_to_options.html.slim new file mode 100644 index 00000000000..82652316b46 --- /dev/null +++ b/app/views/shared/_cancel_or_back_to_options.html.slim @@ -0,0 +1,5 @@ +.mt2.pt1.border-top +- if user_fully_authenticated? + = link_to cancel_link_text, account_path, class: 'h5' +- else + = link_to t('devise.two_factor_authentication.two_factor_choice_cancel'), two_factor_options_path diff --git a/app/views/users/phone_setup/index.html.slim b/app/views/users/phone_setup/index.html.slim new file mode 100644 index 00000000000..f6ddb4f5c43 --- /dev/null +++ b/app/views/users/phone_setup/index.html.slim @@ -0,0 +1,27 @@ +- title @presenter.heading += image_tag asset_url(@presenter.image), width: 200, class: 'mb2' + +h1.h3.my0 = @presenter.heading +p.mt-tiny.mb0 = @presenter.info += simple_form_for(@user_phone_form, + html: { autocomplete: 'off', role: 'form' }, + data: { unsupported_area_codes: unsupported_area_codes, + international_phone_form: true }, + method: :patch, + url: phone_setup_path) do |f| + .sm-col-8.js-intl-tel-code-select + = f.input :international_code, + collection: international_phone_codes, + include_blank: false, + input_html: { class: 'international-code' } + .sm-col-8.mb3 + = f.label :phone + strong.left = @presenter.label + = f.input :phone, as: :tel, label: false, required: true, + input_html: { class: 'phone col-8 mb4' } + = f.button :submit, t('forms.buttons.send_security_code') +.mt2.pt1.border-top + = link_to t('devise.two_factor_authentication.two_factor_choice_cancel'), two_factor_options_path + + = stylesheet_link_tag 'intl-tel-number/intlTelInput' + = javascript_pack_tag 'intl-tel-input' diff --git a/app/views/users/phones/edit.html.slim b/app/views/users/phones/edit.html.slim index 9099011ff55..bd922912786 100644 --- a/app/views/users/phones/edit.html.slim +++ b/app/views/users/phones/edit.html.slim @@ -6,6 +6,19 @@ h1.h3.my0 = t('headings.edit_info.phone') data: { unsupported_area_codes: unsupported_area_codes, international_phone_form: true }, url: manage_phone_path) do |f| - = render 'users/shared/phone_input', f: f + .sm-col-8.js-intl-tel-code-select + = f.input :international_code, + collection: international_phone_codes, + include_blank: false, + input_html: { class: 'international-code' } + .sm-col-8.mb3 + = f.label :phone + strong.left = @presenter.label + = f.input :phone, as: :tel, label: false, required: true, + input_html: { class: 'phone col-8 mb4' } + = render 'users/shared/otp_delivery_preference_selection' = f.button :submit, t('forms.buttons.submit.confirm_change') = render 'shared/cancel', link: account_path + += stylesheet_link_tag 'intl-tel-number/intlTelInput' += javascript_pack_tag 'intl-tel-input' diff --git a/app/views/users/piv_cac_authentication_setup/new.html.slim b/app/views/users/piv_cac_authentication_setup/new.html.slim index c62a521abdf..51f1aba2252 100644 --- a/app/views/users/piv_cac_authentication_setup/new.html.slim +++ b/app/views/users/piv_cac_authentication_setup/new.html.slim @@ -6,6 +6,6 @@ p.mt-tiny.mb3 = @presenter.description = link_to @presenter.piv_cac_capture_text, @presenter.piv_cac_service_link, class: 'btn btn-primary' -= render 'shared/cancel', link: account_path += render 'shared/cancel_or_back_to_options' == javascript_pack_tag 'clipboard' diff --git a/app/views/users/shared/_phone_input.html.slim b/app/views/users/shared/_phone_input.html.slim deleted file mode 100644 index 7e6cea23aa5..00000000000 --- a/app/views/users/shared/_phone_input.html.slim +++ /dev/null @@ -1,16 +0,0 @@ -.sm-col-8.js-intl-tel-code-select - = f.input :international_code, - collection: international_phone_codes, - include_blank: false, - input_html: { class: 'international-code' } -.sm-col-8.mb3 - = f.label :phone - strong.left = t('devise.two_factor_authentication.otp_phone_label') - span#otp_phone_label_info.ml1.italic - = t('devise.two_factor_authentication.otp_phone_label_info') - = f.input :phone, as: :tel, label: false, required: true, - input_html: { class: 'new-phone col-8 mb4' } -= render 'users/shared/otp_delivery_preference_selection' - -= stylesheet_link_tag 'intl-tel-number/intlTelInput' -= javascript_pack_tag 'intl-tel-input' diff --git a/app/views/users/totp_setup/new.html.slim b/app/views/users/totp_setup/new.html.slim index 0cf0000d630..b282b17db51 100644 --- a/app/views/users/totp_setup/new.html.slim +++ b/app/views/users/totp_setup/new.html.slim @@ -42,6 +42,7 @@ ul.list-reset .col.col-6.sm-col-5.px1 = submit_tag t('forms.buttons.submit.default'), class: 'col-12 btn btn-primary align-top' -= render 'shared/cancel', link: account_path + += render 'shared/cancel_or_back_to_options' == javascript_pack_tag 'clipboard' diff --git a/app/views/users/two_factor_authentication_setup/index.html.slim b/app/views/users/two_factor_authentication_setup/index.html.slim index fef9c4a4c6e..c490b3e1f8f 100644 --- a/app/views/users/two_factor_authentication_setup/index.html.slim +++ b/app/views/users/two_factor_authentication_setup/index.html.slim @@ -1,14 +1,40 @@ - title t('titles.two_factor_setup') -h1.h3.my0 = t('devise.two_factor_authentication.two_factor_setup') -p.mt-tiny.mb0 - = t('devise.two_factor_authentication.otp_setup_html') -= simple_form_for(@user_phone_form, - html: { autocomplete: 'off', role: 'form' }, - data: { unsupported_area_codes: unsupported_area_codes, - international_phone_form: true }, - method: :patch, - url: phone_setup_path) do |f| - = render 'users/shared/phone_input', f: f - = f.button :submit, t('forms.buttons.send_security_code') +h1.h3.my0 = t('devise.two_factor_authentication.two_factor_choice') +p.mt-tiny.mb3 + = t('devise.two_factor_authentication.two_factor_choice_intro') + += simple_form_for(@two_factor_options_form, + html: { autocomplete: 'off', role: 'form' }, + method: :patch, + url: two_factor_options_path) do |f| + .mb3 + fieldset.m0.p0.border-none. + legend.mb1.h4.serif.bold = t('forms.two_factor_choice.legend') + ':' + label.btn-border.col-12.mb1 for="two_factor_options_form_selection_sms" + .radio + = radio_button_tag 'two_factor_options_form[selection]', :sms, true + span.indicator.mt-tiny + span.blue.bold.fs-20p + = t('devise.two_factor_authentication.two_factor_choice_options.sms') + .regular.gray-dark.fs-10p.mb-tiny + = t('devise.two_factor_authentication.two_factor_choice_options.sms_info') + label.btn-border.col-12.mb1 for="two_factor_options_form_selection_voice" + .radio + = radio_button_tag 'two_factor_options_form[selection]', :voice, false + span.indicator.mt-tiny + span.blue.bold.fs-20p + = t('devise.two_factor_authentication.two_factor_choice_options.voice') + .regular.gray-dark.fs-10p.mb-tiny + = t('devise.two_factor_authentication.two_factor_choice_options.voice_info') + label.btn-border.col-12.mb1 for="two_factor_options_form_selection_auth_app" + .radio + = radio_button_tag 'two_factor_options_form[selection]', :auth_app, false + span.indicator.mt-tiny + span.blue.bold.fs-20p + = t('devise.two_factor_authentication.two_factor_choice_options.auth_app') + .regular.gray-dark.fs-10p.mb-tiny + = t('devise.two_factor_authentication.two_factor_choice_options.auth_app_info') + = f.button :submit, t('forms.buttons.continue') + = render 'shared/cancel', link: destroy_user_session_path diff --git a/config/locales/devise/en.yml b/config/locales/devise/en.yml index 911015e3a45..4e0f88be735 100644 --- a/config/locales/devise/en.yml +++ b/config/locales/devise/en.yml @@ -118,7 +118,6 @@ en: sms: Text message (SMS) title: How should we send you a code? voice: Phone call - otp_phone_label: Phone number otp_phone_label_info: Mobile phone or landline otp_phone_label_info_mobile_only: Mobile phone otp_setup_html: "Every time you log in, we will send you a @@ -131,6 +130,11 @@ en: personal_key_header_text: Enter your personal key personal_key_prompt: You can use this personal key once. If you still need a code after signing in, go to your account settings page to get a new one. + phone_sms_label: Mobile phone number + phone_sms_info_html: We'll text a security code each time you sign in. + phone_voice_label: Phone number + phone_voice_info_html: We'll call you with a security code each time + you sign in. piv_cac_fallback: link: Use your PIV/CAC instead text_html: Do you have your PIV/CAC? %{link} @@ -145,6 +149,20 @@ en: voice_link_text: Receive a code via phone call totp_header_text: Enter your authentication app code totp_info: Use any authenticator app to scan the QR code below. + two_factor_choice: Secure your account + two_factor_choice_options: + voice: Phone call + voice_info: Get your security code via phone call. + sms: Text message / SMS + sms_info: Get your security code via text message / SMS. + auth_app: Authentication application + auth_app_info: Set up an authentication application to get your security code + without providing a phone number. + piv_cac: Government employees + piv_cac_info: Use your PIV/CAC card to secure your account. + two_factor_choice_intro: login.gov makes sure you can access your account by + adding a second layer of security. + two_factor_choice_cancel: "‹ Choose another option" two_factor_setup: Add a phone number user: new_otp_sent: We sent you a new one-time security code. diff --git a/config/locales/devise/es.yml b/config/locales/devise/es.yml index d282144cc3a..98e82ea11d7 100644 --- a/config/locales/devise/es.yml +++ b/config/locales/devise/es.yml @@ -122,7 +122,6 @@ es: sms: Mensaje de texto (SMS, sigla en inglés) title: "¿Cómo deberíamos enviarle un código?" voice: Llamada telefónica - otp_phone_label: Número de teléfono otp_phone_label_info: El móvil o teléfono fijo. Si tiene un teléfono fijo, seleccione "Llamada telefónica" en la siguiente pregunta. otp_phone_label_info_mobile_only: NOT TRANSLATED YET @@ -137,6 +136,12 @@ es: personal_key_prompt: Puede usar esta clave personal una vez. Si todavía necesita un código después de iniciar una sesión, vaya a la página de configuración de su cuenta para obtener una clave nueva. + phone_sms_label: Número de teléfono móvil + phone_sms_info_html: Le enviaremos un mensaje de texto con un código de seguridad + cada vez que inicie sesión. + phone_voice_label: Número de teléfono + phone_voice_info_html: Te llamaremos con un código de seguridad cada + vez que inicies sesión. piv_cac_fallback: link: Use su PIV/CAC en su lugar text_html: "¿Tiene usted PIV/CAC? %{link}" @@ -152,6 +157,20 @@ es: totp_header_text: Ingrese su código de la app de autenticación totp_info: Use cualquier app de autenticación para escanear el código QR que aparece a continuación. + two_factor_choice: Asegure su cuenta + two_factor_choice_options: + voice: Llamada telefónica + voice_info: Obtenga su código de seguridad a través de una llamada telefónica. + sms: Mensaje de texto / SMS + sms_info: Obtenga su código de seguridad a través de mensajes de texto / SMS. + auth_app: Aplicación de autenticación + auth_app_info: Configure una aplicación de autenticación para obtener su código + de seguridad sin proporcionar un número de teléfono. + piv_cac: Empleados del Gobierno + piv_cac_info: Use su tarjeta PIV / CAC para asegurar su cuenta. + two_factor_choice_intro: login.gov se asegura de que pueda acceder a su cuenta + agregando una segunda capa de seguridad. + two_factor_choice_cancel: "‹ Elige otra opción" two_factor_setup: Añada un número de teléfono user: new_otp_sent: Le enviamos un nuevo código de sólo un uso diff --git a/config/locales/devise/fr.yml b/config/locales/devise/fr.yml index 36afb68776c..2fbb9fe4b2d 100644 --- a/config/locales/devise/fr.yml +++ b/config/locales/devise/fr.yml @@ -130,7 +130,6 @@ fr: sms: Message texte (SMS) title: Comment devrions-nous vous envoyer un code? voice: Appel téléphonique - otp_phone_label: Numéro de téléphone otp_phone_label_info: Cellulaire ou ligne fixe. Si vous entrez une ligne fixe, veuillez choisir l'option "Appel téléphonique" ci-dessous. otp_phone_label_info_mobile_only: NOT TRANSLATED YET @@ -145,6 +144,12 @@ fr: personal_key_prompt: Vous pouvez utiliser cette clé personnelle une fois seulement. Si vous avez toujours besoin d'un code après votre connexion, allez à la page des réglages de votre compte pour en obtenir un nouveau. + phone_sms_label: Numéro de téléphone portable + phone_sms_info_html: Nous vous enverrons un code de sécurité chaque + fois que vous vous connectez. + phone_voice_label: Numéro de téléphone + phone_voice_info_html: Nous vous appellerons avec un code de sécurité chaque + fois que vous vous connectez. piv_cac_fallback: link: Utilisez plutôt votre PIV/CAC text_html: Avez-vous votre PIV/CAC? %{link} @@ -160,6 +165,20 @@ fr: totp_header_text: Entrez votre code d'application d'authentification totp_info: Utilisez n'importe quelle application d'authentification pour balayer le code QR ci-dessous. + two_factor_choice: Sécurise ton compte + two_factor_choice_options: + voice: Appel téléphonique + voice_info: Obtenez votre code de sécurité par appel téléphonique. + sms: SMS + sms_info: Obtenez votre code de sécurité par SMS + auth_app: Application d'authentification + auth_app_info: Configurez une application d'authentification pour obtenir + votre code de sécurité sans fournir de numéro de téléphone. + piv_cac: Employés du gouvernement + piv_cac_info: Utilisez votre carte PIV / CAC pour sécuriser votre compte. + two_factor_choice_intro: login.gov s'assure que vous pouvez accéder à votre + compte en ajoutant une deuxième couche de sécurité. + two_factor_choice_cancel: "‹ Choisissez une autre option" two_factor_setup: Ajoutez un numéro de téléphone user: new_otp_sent: Nous vous avons envoyé un code de sécurité à utilisation unique. diff --git a/config/locales/forms/en.yml b/config/locales/forms/en.yml index c633a271cef..7b0286d8c45 100644 --- a/config/locales/forms/en.yml +++ b/config/locales/forms/en.yml @@ -72,6 +72,8 @@ en: code: One-time security code personal_key: Personal key try_again: Use another phone number + two_factor_choice: + legend: Select an option to secure your account verify_profile: instructions: Enter the ten-character code in the letter we sent you. name: Confirmation code diff --git a/config/locales/forms/es.yml b/config/locales/forms/es.yml index 7eb41249384..5f45b02309d 100644 --- a/config/locales/forms/es.yml +++ b/config/locales/forms/es.yml @@ -72,6 +72,8 @@ es: code: Código de seguridad de sólo un uso personal_key: Clave personal try_again: Use otro número de teléfono. + two_factor_choice: + legend: Seleccione una opción para proteger su cuenta verify_profile: instructions: Ingrese el código de 10 caracteres que le enviamos en la carta. name: Código de confirmación diff --git a/config/locales/forms/fr.yml b/config/locales/forms/fr.yml index 16930194f9b..463332376ff 100644 --- a/config/locales/forms/fr.yml +++ b/config/locales/forms/fr.yml @@ -75,6 +75,8 @@ fr: code: Code de sécurité personal_key: Clé personnelle try_again: Utilisez un autre numéro de téléphone + two_factor_choice: + legend: Sélectionnez une option pour sécuriser votre compte verify_profile: instructions: Entrez le code à dix caractères qui se trouve dans la lettre que nous vous avons envoyée. diff --git a/config/locales/titles/en.yml b/config/locales/titles/en.yml index cca277124b0..0170ca9833c 100644 --- a/config/locales/titles/en.yml +++ b/config/locales/titles/en.yml @@ -16,6 +16,9 @@ en: confirm: Confirm the password for your account forgot: Reset the password for your account personal_key: Just in case + phone_setup: + voice: Send your security code via phone call + sms: Send your security code via text message piv_cac_setup: new: Use your PIV/CAC card to secure your account certificate: diff --git a/config/locales/titles/es.yml b/config/locales/titles/es.yml index 619a8351add..2bd2bedf56f 100644 --- a/config/locales/titles/es.yml +++ b/config/locales/titles/es.yml @@ -16,6 +16,9 @@ es: confirm: Confirme la contraseña de su cuenta forgot: Restablezca la contraseña de su cuenta personal_key: Por si acaso + phone_setup: + voice: Envíe su código de seguridad a través de una llamada telefónica + sms: Envíe su código de seguridad a través de un mensaje de texto piv_cac_setup: new: Use su tarjeta PIV/CAC para asegurar su cuenta certificate: diff --git a/config/locales/titles/fr.yml b/config/locales/titles/fr.yml index bc4e3dfaaeb..bec817df8ea 100644 --- a/config/locales/titles/fr.yml +++ b/config/locales/titles/fr.yml @@ -16,6 +16,9 @@ fr: confirm: Confirmez le mot de passe de votre compte forgot: Réinitialisez le mot de passe de votre compte personal_key: Juste au cas + phone_setup: + voice: Envoyez votre code de sécurité par appel téléphonique + sms: Envoyer votre code de sécurité par SMS piv_cac_setup: new: Utilisez votre carte PIV/CAC pour sécuriser votre compte certificate: diff --git a/config/routes.rb b/config/routes.rb index dc8e7c96299..cea7e3dee93 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -123,8 +123,10 @@ post '/manage/personal_key' => 'users/personal_keys#update' get '/otp/send' => 'users/two_factor_authentication#send_code' - get '/phone_setup' => 'users/two_factor_authentication_setup#index' - patch '/phone_setup' => 'users/two_factor_authentication_setup#set' + get '/two_factor_options' => 'users/two_factor_authentication_setup#index' + patch '/two_factor_options' => 'users/two_factor_authentication_setup#create' + get '/phone_setup' => 'users/phone_setup#index' + patch '/phone_setup' => 'users/phone_setup#create' get '/users/two_factor_authentication' => 'users/two_factor_authentication#show', as: :user_two_factor_authentication # route name is used by two_factor_authentication gem diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index f6284e64ebf..13f5d1437e7 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -113,7 +113,7 @@ def index get :index - expect(response).to redirect_to phone_setup_url + expect(response).to redirect_to two_factor_options_url end end diff --git a/spec/controllers/users/phone_setup_controller_spec.rb b/spec/controllers/users/phone_setup_controller_spec.rb new file mode 100644 index 00000000000..f09ee2c3aef --- /dev/null +++ b/spec/controllers/users/phone_setup_controller_spec.rb @@ -0,0 +1,194 @@ +require 'rails_helper' + +describe Users::PhoneSetupController do + describe 'GET index' do + context 'when signed out' do + it 'redirects to sign in page' do + expect(PhoneSetupPresenter).to_not receive(:new) + + get :index + + expect(response).to redirect_to(new_user_session_url) + end + end + + context 'when signed in' do + it 'renders the index view' do + stub_analytics + user = build(:user, otp_delivery_preference: 'voice') + stub_sign_in_before_2fa(user) + + expect(@analytics).to receive(:track_event). + with(Analytics::USER_REGISTRATION_PHONE_SETUP_VISIT) + expect(PhoneSetupPresenter).to receive(:new).with(user.otp_delivery_preference) + expect(UserPhoneForm).to receive(:new).with(user) + + get :index + + expect(response).to render_template(:index) + end + end + end + + describe 'PATCH create' do + let(:user) { create(:user) } + + it 'tracks an event when the number is invalid' do + sign_in(user) + + stub_analytics + result = { + success: false, + errors: { phone: [t('errors.messages.improbable_phone')] }, + otp_delivery_preference: 'sms', + } + + expect(@analytics).to receive(:track_event). + with(Analytics::MULTI_FACTOR_AUTH_PHONE_SETUP, result) + + patch :create, params: { + user_phone_form: { + phone: '703-555-010', + international_code: 'US', + }, + } + + expect(response).to render_template(:index) + end + + context 'with voice' do + let(:user) { create(:user, otp_delivery_preference: 'voice') } + + it 'prompts to confirm the number' do + sign_in(user) + + stub_analytics + result = { + success: true, + errors: {}, + otp_delivery_preference: 'voice', + } + + expect(@analytics).to receive(:track_event). + with(Analytics::MULTI_FACTOR_AUTH_PHONE_SETUP, result) + + patch( + :create, + params: { + user_phone_form: { phone: '703-555-0100', + # otp_delivery_preference: 'voice', + international_code: 'US' }, + } + ) + + expect(response).to redirect_to( + otp_send_path( + otp_delivery_selection_form: { otp_delivery_preference: 'voice' } + ) + ) + + expect(subject.user_session[:context]).to eq 'confirmation' + end + end + + context 'with SMS' do + it 'prompts to confirm the number' do + sign_in(user) + + stub_analytics + + result = { + success: true, + errors: {}, + otp_delivery_preference: 'sms', + } + + expect(@analytics).to receive(:track_event). + with(Analytics::MULTI_FACTOR_AUTH_PHONE_SETUP, result) + + patch( + :create, + params: { + user_phone_form: { phone: '703-555-0100', + # otp_delivery_preference: :sms, + international_code: 'US' }, + } + ) + + expect(response).to redirect_to( + otp_send_path( + otp_delivery_selection_form: { otp_delivery_preference: 'sms' } + ) + ) + + expect(subject.user_session[:context]).to eq 'confirmation' + end + end + + context 'without selection' do + it 'prompts to confirm via SMS by default' do + sign_in(user) + + stub_analytics + result = { + success: true, + errors: {}, + otp_delivery_preference: 'sms', + } + + expect(@analytics).to receive(:track_event). + with(Analytics::MULTI_FACTOR_AUTH_PHONE_SETUP, result) + + patch( + :create, + params: { + user_phone_form: { phone: '703-555-0100', + # otp_delivery_preference: :sms, + international_code: 'US' }, + } + ) + + expect(response).to redirect_to( + otp_send_path( + otp_delivery_selection_form: { otp_delivery_preference: 'sms' } + ) + ) + + expect(subject.user_session[:context]).to eq 'confirmation' + end + end + end + + describe 'before_actions' do + it 'includes the appropriate before_actions' do + expect(subject).to have_actions( + :before, + :authenticate_user, + :authorize_phone_setup + ) + end + end + + describe '#authorize_otp_setup' do + context 'when the user is fully authenticated' do + it 'redirects to account url' do + stub_sign_in + + get :index + + expect(response).to redirect_to(account_url) + end + end + + context 'when the user is two_factor_enabled but not fully authenticated' do + it 'prompts to enter OTP' do + user = build(:user, :signed_up) + stub_sign_in_before_2fa(user) + + get :index + + expect(response).to redirect_to(user_two_factor_authentication_url) + end + end + end +end diff --git a/spec/controllers/users/two_factor_authentication_controller_spec.rb b/spec/controllers/users/two_factor_authentication_controller_spec.rb index a6cf11b3872..efb4febbae0 100644 --- a/spec/controllers/users/two_factor_authentication_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_controller_spec.rb @@ -105,7 +105,7 @@ def index stub_sign_in_before_2fa(build(:user)) get :show - expect(response).to redirect_to phone_setup_url + expect(response).to redirect_to two_factor_options_url end end end diff --git a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb index 3ac54eedb50..5f7323249a2 100644 --- a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb @@ -2,6 +2,16 @@ describe Users::TwoFactorAuthenticationSetupController do describe 'GET index' do + it 'tracks the visit in analytics' do + stub_sign_in_before_2fa + stub_analytics + + expect(@analytics).to receive(:track_event). + with(Analytics::USER_REGISTRATION_2FA_SETUP_VISIT) + + get :index + end + context 'when signed out' do it 'redirects to sign in page' do get :index @@ -9,165 +19,125 @@ expect(response).to redirect_to(new_user_session_url) end end + + context 'when fully authenticated' do + it 'redirects to account page' do + stub_sign_in + + get :index + + expect(response).to redirect_to(account_url) + end + end + + context 'already two factor enabled but not fully authenticated' do + it 'prompts for 2FA' do + user = build(:user, :signed_up) + stub_sign_in_before_2fa(user) + + get :index + + expect(response).to redirect_to(user_two_factor_authentication_url) + end + end end - describe 'PATCH set' do - let(:user) { create(:user) } + describe 'PATCH create' do + it 'submits the TwoFactorOptionsForm' do + user = build(:user) + stub_sign_in_before_2fa(user) + + voice_params = { + two_factor_options_form: { + selection: 'voice', + } + } + params = ActionController::Parameters.new(voice_params) + response = FormResponse.new(success: true, errors: {}, extra: { selection: 'voice' }) - it 'tracks an event when the number is invalid' do - sign_in(user) + form = instance_double(TwoFactorOptionsForm) + allow(TwoFactorOptionsForm).to receive(:new).with(user).and_return(form) + expect(form).to receive(:submit). + with(params.require(:two_factor_options_form).permit(:selection)). + and_return(response) + expect(form).to receive(:selection).and_return('voice') + patch :create, params: voice_params + end + + it 'tracks analytics event' do + stub_sign_in_before_2fa stub_analytics + result = { - success: false, - errors: { phone: [t('errors.messages.improbable_phone')] }, - otp_delivery_preference: 'sms', + selection: 'voice', + success: true, + errors: {}, } expect(@analytics).to receive(:track_event). - with(Analytics::MULTI_FACTOR_AUTH_PHONE_SETUP, result) + with(Analytics::USER_REGISTRATION_2FA_SETUP, result) - patch :set, params: { - user_phone_form: { - phone: '703-555-010', - otp_delivery_preference: :sms, - international_code: 'US', + patch :create, params: { + two_factor_options_form: { + selection: 'voice', }, } - - expect(response).to render_template(:index) end - context 'with voice' do - it 'prompts to confirm the number' do - sign_in(user) + context 'when the selection is sms' do + it 'redirects to phone setup page' do + stub_sign_in_before_2fa - stub_analytics - result = { - success: true, - errors: {}, - otp_delivery_preference: 'voice', + patch :create, params: { + two_factor_options_form: { + selection: 'sms', + }, } - expect(@analytics).to receive(:track_event). - with(Analytics::MULTI_FACTOR_AUTH_PHONE_SETUP, result) - - patch( - :set, - params: { - user_phone_form: { phone: '703-555-0100', - otp_delivery_preference: 'voice', - international_code: 'US' }, - } - ) - - expect(response).to redirect_to( - otp_send_path( - otp_delivery_selection_form: { otp_delivery_preference: 'voice' } - ) - ) - - expect(subject.user_session[:context]).to eq 'confirmation' + expect(response).to redirect_to phone_setup_url end end - context 'with SMS' do - it 'prompts to confirm the number' do - sign_in(user) - - stub_analytics + context 'when the selection is voice' do + it 'redirects to phone setup page' do + stub_sign_in_before_2fa - result = { - success: true, - errors: {}, - otp_delivery_preference: 'sms', + patch :create, params: { + two_factor_options_form: { + selection: 'voice', + }, } - expect(@analytics).to receive(:track_event). - with(Analytics::MULTI_FACTOR_AUTH_PHONE_SETUP, result) - - patch( - :set, - params: { - user_phone_form: { phone: '703-555-0100', - otp_delivery_preference: :sms, - international_code: 'US' }, - } - ) - - expect(response).to redirect_to( - otp_send_path( - otp_delivery_selection_form: { otp_delivery_preference: 'sms' } - ) - ) - - expect(subject.user_session[:context]).to eq 'confirmation' + expect(response).to redirect_to phone_setup_url end end - context 'without selection' do - it 'prompts to confirm via SMS by default' do - sign_in(user) + context 'when the selection is auth_app' do + it 'redirects to authentication app setup page' do + stub_sign_in_before_2fa - stub_analytics - result = { - success: true, - errors: {}, - otp_delivery_preference: 'sms', + patch :create, params: { + two_factor_options_form: { + selection: 'auth_app', + }, } - expect(@analytics).to receive(:track_event). - with(Analytics::MULTI_FACTOR_AUTH_PHONE_SETUP, result) - - patch( - :set, - params: { - user_phone_form: { phone: '703-555-0100', - otp_delivery_preference: :sms, - international_code: 'US' }, - } - ) - - expect(response).to redirect_to( - otp_send_path( - otp_delivery_selection_form: { otp_delivery_preference: 'sms' } - ) - ) - - expect(subject.user_session[:context]).to eq 'confirmation' + expect(response).to redirect_to authenticator_setup_url end end - end - describe 'before_actions' do - it 'includes the appropriate before_actions' do - expect(subject).to have_actions( - :before, - :authenticate_user, - :authorize_otp_setup - ) - end - end + context 'when the selection is not valid' do + it 'renders index page' do + stub_sign_in_before_2fa - describe '#authorize_otp_setup' do - context 'when the user is fully authenticated' do - it 'redirects to root url' do - user = create(:user, :signed_up) - sign_in(user) - - get :index - - expect(response).to redirect_to(root_url) - end - end - - context 'when the user is two_factor_enabled but not fully authenticated' do - it 'prompts to enter OTP' do - sign_in_before_2fa - - get :index + patch :create, params: { + two_factor_options_form: { + selection: 'foo', + }, + } - expect(response).to redirect_to(user_two_factor_authentication_path) + expect(response).to render_template(:index) end end end diff --git a/spec/features/accessibility/user_pages_spec.rb b/spec/features/accessibility/user_pages_spec.rb index 38367466aff..9db6fc614b6 100644 --- a/spec/features/accessibility/user_pages_spec.rb +++ b/spec/features/accessibility/user_pages_spec.rb @@ -28,8 +28,16 @@ end describe '2FA pages' do + scenario 'two factor options page' do + sign_up_and_set_password + + expect(current_path).to eq(two_factor_options_path) + expect(page).to be_accessible + end + scenario 'phone setup page' do sign_up_and_set_password + click_button t('forms.buttons.continue') expect(current_path).to eq(phone_setup_path) expect(page).to be_accessible diff --git a/spec/features/saml/loa1_sso_spec.rb b/spec/features/saml/loa1_sso_spec.rb index dc593198337..4e0b80685e7 100644 --- a/spec/features/saml/loa1_sso_spec.rb +++ b/spec/features/saml/loa1_sso_spec.rb @@ -182,7 +182,7 @@ saml_authn_request = auth_request.create(saml_settings) visit saml_authn_request - expect(current_path).to eq phone_setup_path + expect(current_path).to eq two_factor_options_path end end diff --git a/spec/features/saml/saml_spec.rb b/spec/features/saml/saml_spec.rb index 08df6bab357..48e95366c7c 100644 --- a/spec/features/saml/saml_spec.rb +++ b/spec/features/saml/saml_spec.rb @@ -35,11 +35,12 @@ class MockSession; end end it 'prompts the user to set up 2FA' do - expect(current_path).to eq phone_setup_path + expect(current_path).to eq two_factor_options_path end it 'prompts the user to confirm phone after setting up 2FA' do - fill_in 'Phone', with: '202-555-1212' + select_2fa_option('sms') + fill_in 'user_phone_form_phone', with: '202-555-1212' click_send_security_code expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'sms') diff --git a/spec/features/two_factor_authentication/remember_device_spec.rb b/spec/features/two_factor_authentication/remember_device_spec.rb index 92f5812d935..0e965c28f49 100644 --- a/spec/features/two_factor_authentication/remember_device_spec.rb +++ b/spec/features/two_factor_authentication/remember_device_spec.rb @@ -28,6 +28,7 @@ def remember_device_and_sign_out_user def remember_device_and_sign_out_user user = sign_up_and_set_password user.password = Features::SessionHelper::VALID_PASSWORD + select_2fa_option('sms') fill_in :user_phone_form_phone, with: '5551231234' click_send_security_code check :remember_device diff --git a/spec/features/two_factor_authentication/sign_in_spec.rb b/spec/features/two_factor_authentication/sign_in_spec.rb index 74ef66e1742..3b2b58adef3 100644 --- a/spec/features/two_factor_authentication/sign_in_spec.rb +++ b/spec/features/two_factor_authentication/sign_in_spec.rb @@ -9,9 +9,14 @@ attempt_to_bypass_2fa_setup - expect(current_path).to eq phone_setup_path + expect(current_path).to eq two_factor_options_path + + select_2fa_option('sms') + + click_continue + expect(page). - to have_content t('devise.two_factor_authentication.two_factor_setup') + to have_content t('titles.phone_setup.sms') send_security_code_without_entering_phone_number @@ -25,19 +30,20 @@ expect(page).to have_content invalid_phone_message - submit_2fa_setup_form_with_valid_phone_and_choose_phone_call_delivery + submit_2fa_setup_form_with_valid_phone expect(page).to_not have_content invalid_phone_message - expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'voice') + expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'sms') expect(user.reload.phone).to_not eq '+1 (555) 555-1212' - expect(user.voice?).to eq true + expect(user.sms?).to eq true end context 'user enters OTP incorrectly 3 times' do it 'locks the user out' do sign_in_before_2fa - submit_2fa_setup_form_with_valid_phone_and_choose_phone_call_delivery + select_2fa_option('sms') + submit_2fa_setup_form_with_valid_phone 3.times do fill_in('code', with: 'bad-code') click_button t('forms.buttons.submit.default') @@ -47,89 +53,52 @@ end end - context 'with U.S. phone that does not support phone delivery method' do + context 'with U.S. phone that does not support voice delivery method' do let(:unsupported_phone) { '242-555-5555' } - scenario 'renders an error if a user submits with phone selected' do + scenario 'renders an error if a user submits with voice selected' do sign_in_before_2fa + select_2fa_option('voice') fill_in 'Phone', with: unsupported_phone - choose 'Phone call' click_send_security_code - expect(current_path).to eq(phone_setup_path) - expect(page).to have_content t( - 'devise.two_factor_authentication.otp_delivery_preference.phone_unsupported', - location: 'Bahamas' - ) - end - - scenario 'disables the phone option and displays a warning with js', :js do - sign_in_before_2fa - select_country_and_type_phone_number(country: 'bs', number: '7035551212') - phone_radio_button = page.find( - '#user_phone_form_otp_delivery_preference_voice', - visible: :all - ) + expect(current_path).to eq phone_setup_path expect(page).to have_content t( 'devise.two_factor_authentication.otp_delivery_preference.phone_unsupported', location: 'Bahamas' ) - expect(phone_radio_button).to be_disabled - select_country_and_type_phone_number(country: 'us', number: '7035551212') - - expect(page).not_to have_content t( - 'devise.two_factor_authentication.otp_delivery_preference.phone_unsupported', - location: 'Bahamas' - ) - expect(phone_radio_button).to_not be_disabled + click_on t('devise.two_factor_authentication.two_factor_choice_cancel') + + expect(current_path).to eq two_factor_options_path end end - context 'with international phone that does not support phone delivery' do - scenario 'renders an error if a user submits with phone selected' do + context 'with international phone that does not support voice delivery' do + scenario 'updates international code as user types', :js do sign_in_before_2fa + select_2fa_option('voice') + fill_in 'Phone', with: '+81 54 354 3643' - select 'Turkey +90', from: 'International code' - fill_in 'Phone', with: '555-555-5000' - choose 'Phone call' - click_send_security_code + expect(page.find('#user_phone_form_international_code', visible: false).value).to eq 'JP' - expect(current_path).to eq(phone_setup_path) - expect(page).to have_content t( - 'devise.two_factor_authentication.otp_delivery_preference.phone_unsupported', - location: 'Turkey' - ) - end + fill_in 'Phone', with: '' + fill_in 'Phone', with: '+212 5376' - scenario 'disables the phone option and displays a warning with js', :js do - sign_in_before_2fa - select_country_and_type_phone_number(country: 'tr', number: '3122132965') + expect(page.find('#user_phone_form_international_code', visible: false).value).to eq 'MA' - phone_radio_button = page.find( - '#user_phone_form_otp_delivery_preference_voice', - visible: :all - ) + fill_in 'Phone', with: '' + fill_in 'Phone', with: '+81 54354' - expect(page).to have_content t( - 'devise.two_factor_authentication.otp_delivery_preference.phone_unsupported', - location: 'Turkey' - ) - expect(phone_radio_button).to be_disabled - - select_country_and_type_phone_number(country: 'ca', number: '3122132965') - - expect(page).not_to have_content t( - 'devise.two_factor_authentication.otp_delivery_preference.phone_unsupported', - location: 'Turkey' - ) - expect(phone_radio_button).to_not be_disabled + expect(page.find('#user_phone_form_international_code', visible: false).value).to eq 'JP' end scenario 'allows a user to continue typing even if a number is invalid', :js do sign_in_before_2fa + select_2fa_option('voice') + select_country_and_type_phone_number(country: 'us', number: '12345678901234567890') expect(phone_field.value).to eq('12345678901234567890') @@ -156,18 +125,17 @@ def send_security_code_without_entering_phone_number end def submit_2fa_setup_form_with_empty_string_phone - fill_in 'Phone', with: '' + fill_in 'user_phone_form_phone', with: '' click_send_security_code end def submit_2fa_setup_form_with_invalid_phone - fill_in 'Phone', with: 'five one zero five five five four three two one' + fill_in 'user_phone_form_phone', with: 'five one zero five five five four three two one' click_send_security_code end - def submit_2fa_setup_form_with_valid_phone_and_choose_phone_call_delivery - fill_in 'Phone', with: '555-555-1212' - choose 'Phone call' + def submit_2fa_setup_form_with_valid_phone + fill_in 'user_phone_form_phone', with: '555-555-1212' click_send_security_code end @@ -406,15 +374,16 @@ def submit_prefilled_otp_code context 'When setting up 2FA for the first time' do it 'enforces rate limiting only for current phone' do - second_user = create(:user, :signed_up, phone: '+1 202-555-1212') + second_user = create(:user, :signed_up, phone: '202-555-1212') sign_in_before_2fa max_attempts = Figaro.env.otp_delivery_blocklist_maxretry.to_i - submit_2fa_setup_form_with_valid_phone_and_choose_phone_call_delivery + select_2fa_option('sms') + submit_2fa_setup_form_with_valid_phone max_attempts.times do - click_link t('links.two_factor_authentication.resend_code.voice') + click_link t('links.two_factor_authentication.resend_code.sms') end expect(page).to have_content t('titles.account_locked') diff --git a/spec/features/users/regenerate_personal_key_spec.rb b/spec/features/users/regenerate_personal_key_spec.rb index 0503a0367a1..cc66086bda4 100644 --- a/spec/features/users/regenerate_personal_key_spec.rb +++ b/spec/features/users/regenerate_personal_key_spec.rb @@ -145,7 +145,8 @@ def sign_up_and_view_personal_key allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) sign_up_and_set_password - fill_in 'Phone', with: '202-555-1212' + select_2fa_option('sms') + fill_in 'user_phone_form_phone', with: '202-555-1212' click_send_security_code click_submit_default end diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index 0103e135e9a..a2495ab7652 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -55,7 +55,8 @@ allow(SmsOtpSenderJob).to receive(:perform_now).and_raise(twilio_error) sign_up_and_set_password - fill_in 'Phone', with: '202-555-1212' + select_2fa_option('sms') + fill_in 'user_phone_form_phone', with: '202-555-1212' click_send_security_code expect(current_path).to eq(phone_setup_path) @@ -155,8 +156,7 @@ it_behaves_like 'creating an account using authenticator app for 2FA', :oidc it 'allows a user to choose TOTP as 2FA method during sign up' do - user = create(:user) - sign_in_user(user) + sign_in_user set_up_2fa_with_authenticator_app click_acknowledge_personal_key diff --git a/spec/features/users/user_edit_spec.rb b/spec/features/users/user_edit_spec.rb index cc162c14cd1..bb726d6ab2f 100644 --- a/spec/features/users/user_edit_spec.rb +++ b/spec/features/users/user_edit_spec.rb @@ -31,7 +31,7 @@ end scenario 'user is able to submit with a Puerto Rico phone number as a US number', js: true do - fill_in 'Phone', with: '787 555-1234' + fill_in 'user_phone_form_phone', with: '787 555-1234' expect(page.find('#user_phone_form_international_code', visible: false).value).to eq 'PR' expect(page).to have_button(t('forms.buttons.submit.confirm_change'), disabled: false) @@ -41,7 +41,7 @@ allow(SmsOtpSenderJob).to receive(:perform_later) allow(VoiceOtpSenderJob).to receive(:perform_now) - fill_in 'Phone', with: '555-555-5000' + fill_in 'user_phone_form_phone', with: '555-555-5000' choose 'Phone call' click_button t('forms.buttons.submit.confirm_change') diff --git a/spec/features/visitors/email_confirmation_spec.rb b/spec/features/visitors/email_confirmation_spec.rb index add20a4df54..5084aabc1a4 100644 --- a/spec/features/visitors/email_confirmation_spec.rb +++ b/spec/features/visitors/email_confirmation_spec.rb @@ -17,7 +17,7 @@ fill_in 'password_form_password', with: Features::SessionHelper::VALID_PASSWORD click_button t('forms.buttons.continue') - expect(current_url).to eq phone_setup_url + expect(current_url).to eq two_factor_options_url expect(page).to_not have_content t('devise.confirmations.confirmed_but_must_set_password') end diff --git a/spec/features/visitors/password_recovery_spec.rb b/spec/features/visitors/password_recovery_spec.rb index 4a0db3975b1..a654d2bae7f 100644 --- a/spec/features/visitors/password_recovery_spec.rb +++ b/spec/features/visitors/password_recovery_spec.rb @@ -50,7 +50,7 @@ fill_in_credentials_and_submit(user.email, 'NewVal!dPassw0rd') - expect(current_path).to eq phone_setup_path + expect(current_path).to eq two_factor_options_path end end @@ -87,7 +87,7 @@ it 'prompts user to set up their 2FA options after signing back in' do reset_password_and_sign_back_in(@user) - expect(current_path).to eq phone_setup_path + expect(current_path).to eq two_factor_options_path end end diff --git a/spec/features/visitors/phone_confirmation_spec.rb b/spec/features/visitors/phone_confirmation_spec.rb index 6be62f7c9cd..c9e67734309 100644 --- a/spec/features/visitors/phone_confirmation_spec.rb +++ b/spec/features/visitors/phone_confirmation_spec.rb @@ -6,7 +6,8 @@ allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) allow(SmsOtpSenderJob).to receive(:perform_now) @user = sign_in_before_2fa - fill_in 'Phone', with: '555-555-5555' + select_2fa_option('sms') + fill_in 'user_phone_form_phone', with: '555-555-5555' click_send_security_code expect(SmsOtpSenderJob).to have_received(:perform_now).with( @@ -58,7 +59,8 @@ before do @existing_user = create(:user, :signed_up) @user = sign_in_before_2fa - fill_in 'Phone', with: @existing_user.phone + select_2fa_option('sms') + fill_in 'user_phone_form_phone', with: @existing_user.phone click_send_security_code end diff --git a/spec/forms/two_factor_options_form_spec.rb b/spec/forms/two_factor_options_form_spec.rb new file mode 100644 index 00000000000..d353ca0a48b --- /dev/null +++ b/spec/forms/two_factor_options_form_spec.rb @@ -0,0 +1,55 @@ +require 'rails_helper' + +describe TwoFactorOptionsForm do + let(:user) { build(:user) } + subject { described_class.new(user) } + + describe '#submit' do + it 'is successful if the selection is valid' do + %w[voice sms auth_app piv_cac].each do |selection| + result = subject.submit(selection: selection) + + expect(result.success?).to eq true + end + end + + it 'is unsuccessful if the selection is invalid' do + result = subject.submit(selection: '!!!!') + + expect(result.success?).to eq false + expect(result.errors).to include :selection + end + + context "when the selection is different from the user's otp_delivery_preference" do + it "updates the user's otp_delivery_preference" do + user_updater = instance_double(UpdateUser) + allow(UpdateUser). + to receive(:new). + with( + user: user, + attributes: { otp_delivery_preference: 'voice' } + ). + and_return(user_updater) + expect(user_updater).to receive(:call) + + result = subject.submit(selection: 'voice') + end + end + + context "when the selection is the same as the user's otp_delivery_preference" do + it "does not update the user's otp_delivery_preference" do + expect(UpdateUser).to_not receive(:new) + + result = subject.submit(selection: 'sms') + end + end + + context 'when the selection is not voice or sms' do + it "does not update the user's otp_delivery_preference" do + expect(UpdateUser).to_not receive(:new) + + result = subject.submit(selection: 'auth_app') + end + end + end +end diff --git a/spec/forms/user_phone_form_spec.rb b/spec/forms/user_phone_form_spec.rb index 25139596550..721fdfc48ff 100644 --- a/spec/forms/user_phone_form_spec.rb +++ b/spec/forms/user_phone_form_spec.rb @@ -43,7 +43,7 @@ expect(result.errors).to be_empty end - it 'include otp preference in the form response extra' do + it 'includes otp preference in the form response extra' do result = subject.submit(params) expect(result.extra).to eq( @@ -90,6 +90,87 @@ end end + context 'when otp_delivery_preference is not voice or sms' do + let(:params) do + { + phone: '703-555-1212', + international_code: 'US', + otp_delivery_preference: 'foo', + } + end + + it 'is invalid' do + result = subject.submit(params) + + expect(result.success?).to eq(false) + expect(result.errors[:otp_delivery_preference].first). + to eq 'is not included in the list' + end + end + + context 'when otp_delivery_preference is empty' do + let(:params) do + { + phone: '703-555-1212', + international_code: 'US', + otp_delivery_preference: '', + } + end + + it 'is invalid' do + result = subject.submit(params) + + expect(result.success?).to eq(false) + expect(result.errors[:otp_delivery_preference].first). + to eq 'is not included in the list' + end + end + + context 'when otp_delivery_preference param is not present' do + let(:params) do + { + phone: '703-555-1212', + international_code: 'US', + } + end + + it 'is valid' do + result = subject.submit(params) + + expect(result.success?).to eq(true) + end + end + + context "when the submitted otp_delivery_preference is different from the user's" do + it "updates the user's otp_delivery_preference" do + user_updater = instance_double(UpdateUser) + allow(UpdateUser). + to receive(:new). + with( + user: user, + attributes: { otp_delivery_preference: 'voice' } + ). + and_return(user_updater) + expect(user_updater).to receive(:call) + + params = { + phone: '555-555-5000', + international_code: 'US', + otp_delivery_preference: 'voice', + } + + result = subject.submit(params) + end + end + + context "when the submitted otp_delivery_preference is the same as the user's" do + it "does not update the user's otp_delivery_preference" do + expect(UpdateUser).to_not receive(:new) + + result = subject.submit(params) + end + end + it 'does not raise inclusion errors for Norwegian phone numbers' do # ref: https://github.com/18F/identity-private/issues/2392 params[:phone] = '21 11 11 11' diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index 84a0b2c871f..f492c2678d0 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -10,10 +10,16 @@ def sign_up_with(email) click_button t('forms.buttons.submit.default') end + def select_2fa_option(option) + find("label[for='two_factor_options_form_selection_#{option}']").click + click_on t('forms.buttons.continue') + end + def sign_up_and_2fa_loa1_user allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) user = sign_up_and_set_password - fill_in 'Phone', with: '202-555-1212' + select_2fa_option('sms') + fill_in 'user_phone_form_phone', with: '202-555-1212' click_send_security_code click_submit_default click_acknowledge_personal_key @@ -367,6 +373,7 @@ def submit_form_with_valid_password(password = VALID_PASSWORD) end def set_up_2fa_with_valid_phone + select_2fa_option('sms') fill_in 'user_phone_form[phone]', with: '202-555-1212' click_send_security_code end @@ -392,7 +399,7 @@ def register_user_with_authenticator_app(email = 'test@test.com') end def set_up_2fa_with_authenticator_app - click_link t('links.two_factor_authentication.app_option') + select_2fa_option('auth_app') expect(page).to have_current_path authenticator_setup_path diff --git a/spec/views/two_factor_authentication_setup/index.html.slim_spec.rb b/spec/views/phone_setup/index.html.slim_spec.rb similarity index 50% rename from spec/views/two_factor_authentication_setup/index.html.slim_spec.rb rename to spec/views/phone_setup/index.html.slim_spec.rb index 6cd6180aae2..94c1c5ebd31 100644 --- a/spec/views/two_factor_authentication_setup/index.html.slim_spec.rb +++ b/spec/views/phone_setup/index.html.slim_spec.rb @@ -1,17 +1,24 @@ require 'rails_helper' -describe 'users/two_factor_authentication_setup/index.html.slim' do +describe 'users/phone_setup/index.html.slim' do before do user = build_stubbed(:user) allow(view).to receive(:current_user).and_return(user) @user_phone_form = UserPhoneForm.new(user) - + @presenter = PhoneSetupPresenter.new('voice') render end it 'sets form autocomplete to off' do expect(rendered).to have_xpath("//form[@autocomplete='off']") end + + it 'renders a link to choose a different option' do + expect(rendered).to have_link( + t('devise.two_factor_authentication.two_factor_choice_cancel'), + href: two_factor_options_path + ) + end end diff --git a/spec/views/users/phones/edit.html.slim_spec.rb b/spec/views/users/phones/edit.html.slim_spec.rb index 25e42529823..a18ee38b7ab 100644 --- a/spec/views/users/phones/edit.html.slim_spec.rb +++ b/spec/views/users/phones/edit.html.slim_spec.rb @@ -6,6 +6,7 @@ user = build_stubbed(:user, :signed_up) allow(view).to receive(:current_user).and_return(user) @user_phone_form = UserPhoneForm.new(user) + @presenter = PhoneSetupPresenter.new('voice') end it 'has a localized title' do diff --git a/spec/views/users/piv_cac_authentication_setup/new.html.slim_spec.rb b/spec/views/users/piv_cac_authentication_setup/new.html.slim_spec.rb new file mode 100644 index 00000000000..0077477a2d3 --- /dev/null +++ b/spec/views/users/piv_cac_authentication_setup/new.html.slim_spec.rb @@ -0,0 +1,32 @@ +require 'rails_helper' + +describe 'users/piv_cac_authentication_setup/new.html.slim' do + before { @presenter = OpenStruct.new(title: 'foo', heading: 'bar', description: 'foobar') } + + context 'user is fully authenticated' do + it 'renders a link to cancel and go back to the account page' do + user = build_stubbed(:user, :signed_up) + allow(view).to receive(:current_user).and_return(user) + allow(view).to receive(:user_fully_authenticated?).and_return(true) + + render + + expect(rendered).to have_link(t('links.cancel'), href: account_path) + end + end + + context 'user is setting up 2FA' do + it 'renders a link to choose a different option' do + user = build_stubbed(:user) + allow(view).to receive(:current_user).and_return(user) + allow(view).to receive(:user_fully_authenticated?).and_return(false) + + render + + expect(rendered).to have_link( + t('devise.two_factor_authentication.two_factor_choice_cancel'), + href: two_factor_options_path + ) + end + end +end diff --git a/spec/views/users/totp_setup/new.html.slim_spec.rb b/spec/views/users/totp_setup/new.html.slim_spec.rb index 91b2437fd8e..84fe4756f0e 100644 --- a/spec/views/users/totp_setup/new.html.slim_spec.rb +++ b/spec/views/users/totp_setup/new.html.slim_spec.rb @@ -3,21 +3,47 @@ describe 'users/totp_setup/new.html.slim' do let(:user) { build_stubbed(:user, :signed_up) } - before do - allow(view).to receive(:current_user).and_return(user) - @code = 'D4C2L47CVZ3JJHD7' - @qrcode = 'qrcode.png' - end + context 'user is fully authenticated' do + before do + allow(view).to receive(:current_user).and_return(user) + allow(view).to receive(:user_fully_authenticated?).and_return(true) + @code = 'D4C2L47CVZ3JJHD7' + @qrcode = 'qrcode.png' + end + + it 'renders the QR code' do + render + + expect(rendered).to have_css('#qr-code', text: 'D4C2L47CVZ3JJHD7') + end - it 'renders the QR code' do - render + it 'renders the QR code image' do + render - expect(rendered).to have_css('#qr-code', text: 'D4C2L47CVZ3JJHD7') + expect(rendered).to have_css('img[src^="/images/qrcode.png"]') + end + + it 'renders a link to cancel and go back to the account page' do + render + + expect(rendered).to have_link(t('links.cancel'), href: account_path) + end end - it 'renders the QR code image' do - render + context 'user is setting up 2FA' do + it 'renders a link to choose a different option' do + user = build_stubbed(:user) + allow(view).to receive(:current_user).and_return(user) + allow(view).to receive(:user_fully_authenticated?).and_return(false) + @code = 'D4C2L47CVZ3JJHD7' + @qrcode = 'qrcode.png' + + render - expect(rendered).to have_css('img[src^="/images/qrcode.png"]') + expect(rendered).to have_link( + t('devise.two_factor_authentication.two_factor_choice_cancel'), + href: two_factor_options_path + ) + end end end