diff --git a/app/controllers/sign_up/registrations_controller.rb b/app/controllers/sign_up/registrations_controller.rb index ceaf77e4b93..2e082c1a820 100644 --- a/app/controllers/sign_up/registrations_controller.rb +++ b/app/controllers/sign_up/registrations_controller.rb @@ -41,9 +41,7 @@ def permitted_params def process_successful_creation user = @register_user_email_form.user - unless @register_user_email_form.email_taken? - create_user_event(:account_created, user) - end + create_user_event(:account_created, user) unless @register_user_email_form.email_taken? resend_confirmation = params[:user][:resend] session[:email] = @register_user_email_form.email diff --git a/app/controllers/users/phone_setup_controller.rb b/app/controllers/users/phone_setup_controller.rb index 6e26e60fb95..789ec49b7a2 100644 --- a/app/controllers/users/phone_setup_controller.rb +++ b/app/controllers/users/phone_setup_controller.rb @@ -9,17 +9,17 @@ class PhoneSetupController < ApplicationController before_action :set_setup_presenter def index - @user_phone_form = UserPhoneForm.new(current_user, nil) + @new_phone_form = NewPhoneForm.new(current_user) analytics.track_event(Analytics::USER_REGISTRATION_PHONE_SETUP_VISIT) end def create - @user_phone_form = UserPhoneForm.new(current_user, nil) - result = @user_phone_form.submit(user_phone_form_params) + @new_phone_form = NewPhoneForm.new(current_user) + result = @new_phone_form.submit(new_phone_form_params) analytics.track_event(Analytics::MULTI_FACTOR_AUTH_PHONE_SETUP, result.to_h) if result.success? - handle_create_success(@user_phone_form.phone) + handle_create_success(@new_phone_form.phone) else render :index end @@ -34,8 +34,8 @@ def set_setup_presenter def handle_create_success(phone) if MfaContext.new(current_user).phone_configurations.map(&:phone).index(phone).nil? prompt_to_confirm_phone(id: nil, - phone: @user_phone_form.phone, - selected_delivery_method: @user_phone_form.otp_delivery_preference) + phone: @new_phone_form.phone, + selected_delivery_method: @new_phone_form.otp_delivery_preference) else flash[:error] = t('errors.messages.phone_duplicate') redirect_to phone_setup_url @@ -46,10 +46,11 @@ def delivery_preference current_user.otp_delivery_preference end - def user_phone_form_params - params.require(:user_phone_form).permit(:phone, :international_code, - :otp_delivery_preference, - :otp_make_default_number) + def new_phone_form_params + params.require(:new_phone_form).permit(:phone, + :international_code, + :otp_delivery_preference, + :otp_make_default_number) end end end diff --git a/app/controllers/users/phones_controller.rb b/app/controllers/users/phones_controller.rb index b70b138274b..e3957c83bc4 100644 --- a/app/controllers/users/phones_controller.rb +++ b/app/controllers/users/phones_controller.rb @@ -6,12 +6,12 @@ class PhonesController < ReauthnRequiredController def add user_session[:phone_id] = nil - @user_phone_form = UserPhoneForm.new(current_user, nil) + @new_phone_form = NewPhoneForm.new(current_user) end def create - @user_phone_form = UserPhoneForm.new(current_user, nil) - if @user_phone_form.submit(user_params).success? + @new_phone_form = NewPhoneForm.new(current_user) + if @new_phone_form.submit(user_params).success? confirm_phone bypass_sign_in current_user else @@ -22,15 +22,16 @@ def create def edit set_phone_id # memoized for view - user_phone_form + @edit_phone_form = EditPhoneForm.new(current_user, phone_configuration) end def update - if user_phone_form.submit(user_params).success? && !already_has_phone? + @edit_phone_form = EditPhoneForm.new(current_user, phone_configuration) + if @edit_phone_form.submit(edit_params).success? process_updates bypass_sign_in current_user else - render_edit + render :edit end end @@ -50,10 +51,6 @@ def delete private - def user_phone_form - @user_phone_form ||= UserPhoneForm.new(current_user, phone_configuration) - end - def render_edit flash.now[:error] = t('errors.messages.phone_duplicate') if already_has_phone? render :edit @@ -67,13 +64,18 @@ def phone_configuration end def user_params - params.require(:user_phone_form).permit(:phone, :international_code, - :otp_delivery_preference, + params.require(:new_phone_form).permit(:phone, :international_code, + :otp_delivery_preference, + :otp_make_default_number) + end + + def edit_params + params.require(:edit_phone_form).permit(:otp_delivery_preference, :otp_make_default_number) end def already_has_phone? - @user_has_phone ||= @user_phone_form.already_has_phone? + @user_has_phone ||= @new_phone_form.already_has_phone? end def delivery_preference @@ -81,25 +83,24 @@ def delivery_preference end def process_updates - form = @user_phone_form - if form.phone_config_changed? + if @edit_phone_form.phone_config_changed? analytics.track_event(Analytics::PHONE_CHANGE_REQUESTED) OtpPreferenceUpdater.new( user: current_user, - preference: form.otp_delivery_preference, - default: form.otp_make_default_number, + preference: @edit_phone_form.otp_delivery_preference, + default: @edit_phone_form.otp_make_default_number, phone_id: user_session[:phone_id], - ).call + ).call end redirect_to account_url end def confirm_phone flash[:notice] = t('devise.registrations.phone_update_needs_confirmation') - prompt_to_confirm_phone(id: user_session[:phone_id], phone: @user_phone_form.phone, - selected_delivery_method: @user_phone_form.otp_delivery_preference, - selected_default_number: @user_phone_form.otp_make_default_number) + prompt_to_confirm_phone(id: user_session[:phone_id], phone: @new_phone_form.phone, + selected_delivery_method: @new_phone_form.otp_delivery_preference, + selected_default_number: @new_phone_form.otp_make_default_number) end def handle_successful_delete diff --git a/app/controllers/users/totp_setup_controller.rb b/app/controllers/users/totp_setup_controller.rb index 486433925c1..e95f4ee45ba 100644 --- a/app/controllers/users/totp_setup_controller.rb +++ b/app/controllers/users/totp_setup_controller.rb @@ -87,7 +87,7 @@ def process_successful_disable def revoke_otp_secret_key UpdateUser.new( user: current_user, - attributes: { otp_secret_key: nil}, + attributes: { otp_secret_key: nil }, ).call end diff --git a/app/controllers/users/two_factor_authentication_setup_controller.rb b/app/controllers/users/two_factor_authentication_setup_controller.rb index f21954bb5e6..7f493a0a6a7 100644 --- a/app/controllers/users/two_factor_authentication_setup_controller.rb +++ b/app/controllers/users/two_factor_authentication_setup_controller.rb @@ -35,7 +35,7 @@ def success def backup_code_only_processing if user_session[:signing_up] && - @two_factor_options_form.selection == 'backup_code_only' + @two_factor_options_form.selection == 'backup_code_only' user_session[:signing_up] = false end end diff --git a/app/forms/edit_phone_form.rb b/app/forms/edit_phone_form.rb new file mode 100644 index 00000000000..b359c86e12f --- /dev/null +++ b/app/forms/edit_phone_form.rb @@ -0,0 +1,66 @@ +class EditPhoneForm + include ActiveModel::Model + include RememberDeviceConcern + + validates :otp_delivery_preference, inclusion: { in: %w[voice sms] } + + attr_accessor :phone, :otp_delivery_preference, :otp_make_default_number, :phone_configuration + + def initialize(user, phone_configuration) + self.user = user + self.phone_configuration = phone_configuration + self.otp_make_default_number = true if default_phone_configuration? + end + + def submit(params) + ingest_submitted_params(params) + success = valid? + + self.phone = submitted_phone unless success + revoke_remember_device(user) if success + FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes) + end + + def delivery_preference_sms? + phone_configuration&.delivery_preference == 'sms' + end + + def delivery_preference_voice? + phone_configuration&.delivery_preference == 'voice' + end + + def phone_config_changed? + return true if phone_configuration&.delivery_preference != otp_delivery_preference + return true if otp_make_default_number && !default_phone_configuration? + false + end + + # :reek:FeatureEnvy + def masked_number + phone_number = phone_configuration.phone + return '' if !phone_number || phone_number.blank? + "***-***-#{phone_number[-4..-1]}" + end + + private + + attr_accessor :user, :submitted_phone + + def extra_analytics_attributes + { + otp_delivery_preference: otp_delivery_preference, + } + end + + def ingest_submitted_params(params) + delivery_prefs = params[:otp_delivery_preference] + default_prefs = params[:otp_make_default_number] + + self.otp_delivery_preference = delivery_prefs if delivery_prefs + self.otp_make_default_number = true if default_prefs + end + + def default_phone_configuration? + phone_configuration == user.default_phone_configuration + end +end diff --git a/app/forms/user_phone_form.rb b/app/forms/new_phone_form.rb similarity index 61% rename from app/forms/user_phone_form.rb rename to app/forms/new_phone_form.rb index 01dd16477ca..9cbb38eb043 100644 --- a/app/forms/user_phone_form.rb +++ b/app/forms/new_phone_form.rb @@ -1,4 +1,4 @@ -class UserPhoneForm +class NewPhoneForm include ActiveModel::Model include FormPhoneValidator include OtpDeliveryPreferenceValidator @@ -7,17 +7,12 @@ class UserPhoneForm validates :otp_delivery_preference, inclusion: { in: %w[voice sms] } attr_accessor :phone, :international_code, :otp_delivery_preference, - :otp_make_default_number, :phone_configuration + :otp_make_default_number - def initialize(user, phone_configuration) + def initialize(user) self.user = user - self.phone_configuration = phone_configuration - if phone_configuration.nil? - self.otp_delivery_preference = user.otp_delivery_preference - else - prefill_phone_number(phone_configuration) - end - self.otp_make_default_number = true if default_phone_configuration? + self.otp_delivery_preference = user.otp_delivery_preference + self.otp_make_default_number = false end def submit(params) @@ -32,28 +27,21 @@ def submit(params) end def delivery_preference_sms? - return true if phone_configuration.blank? - phone_configuration&.delivery_preference == 'sms' + true end def delivery_preference_voice? - phone_configuration&.delivery_preference == 'voice' + false end def already_has_phone? - formatted_user_phone != phone && user.phone_configurations.map(&:phone).include?(phone) - end - - def phone_config_changed? - return true if formatted_user_phone != phone - return true if phone_configuration&.delivery_preference != otp_delivery_preference - return true if otp_make_default_number && !default_phone_configuration? - false + user.phone_configurations.map(&:phone).include?(phone) end # :reek:FeatureEnvy def masked_number - phone_number = phone_configuration == nil ? nil : phone_configuration.phone + phone_number = nil + phone_number = phone_configuration.phone unless phone_configuration.nil? return '' if !phone_number || phone_number.blank? "***-***-#{phone_number[-4..-1]}" end @@ -92,12 +80,4 @@ def ingest_submitted_params(params) self.otp_delivery_preference = delivery_prefs if delivery_prefs self.otp_make_default_number = true if default_prefs end - - def default_phone_configuration? - phone_configuration == user.default_phone_configuration - end - - def formatted_user_phone - phone_configuration&.formatted_phone - end end diff --git a/app/javascript/packs/intl-tel-input.js b/app/javascript/packs/intl-tel-input.js index d2b46d778f9..210c6d20e6f 100644 --- a/app/javascript/packs/intl-tel-input.js +++ b/app/javascript/packs/intl-tel-input.js @@ -4,9 +4,9 @@ import 'intl-tel-input/build/js/utils.js'; import 'intl-tel-input'; // Setting variables that jQuery is using with a $ at the start of the const name -const $telInput = $('#user_phone_form_phone'); -const telInput = document.querySelector('#user_phone_form_phone'); -const $intlCode = $('#user_phone_form_international_code'); +const $telInput = $('#new_phone_form_phone'); +const telInput = document.querySelector('#new_phone_form_phone'); +const $intlCode = $('#new_phone_form_international_code'); // initialise plugin $telInput.intlTelInput({ diff --git a/app/models/null_service_provider.rb b/app/models/null_service_provider.rb index f33f494a615..c1d65cd5141 100644 --- a/app/models/null_service_provider.rb +++ b/app/models/null_service_provider.rb @@ -2,7 +2,7 @@ class NullServiceProvider attr_accessor :issuer, :friendly_name attr_accessor :ial - def initialize(issuer:, friendly_name: "Null ServiceProvider") + def initialize(issuer:, friendly_name: 'Null ServiceProvider') @issuer = issuer @friendly_name = friendly_name end diff --git a/app/services/idv/steps/doc_auth_base_step.rb b/app/services/idv/steps/doc_auth_base_step.rb index 32473cd34e8..dc110b51b89 100644 --- a/app/services/idv/steps/doc_auth_base_step.rb +++ b/app/services/idv/steps/doc_auth_base_step.rb @@ -1,6 +1,5 @@ # :reek:TooManyMethods # :reek:RepeatedConditional -# rubocop:disable Metrics/ClassLength module Idv module Steps class DocAuthBaseStep < Flow::BaseStep @@ -128,4 +127,3 @@ def rescue_network_errors end end end -# rubocop:enable Metrics/ClassLength diff --git a/app/services/idv/steps/verify_step.rb b/app/services/idv/steps/verify_step.rb index a6ed65128ed..4991528649a 100644 --- a/app/services/idv/steps/verify_step.rb +++ b/app/services/idv/steps/verify_step.rb @@ -37,7 +37,7 @@ def skip_legacy_steps end def perform_resolution(pii_from_doc) - stages = aamva_state?(pii_from_doc) ? [:resolution, :state_id] : [:resolution] + stages = aamva_state?(pii_from_doc) ? %i[resolution state_id] : [:resolution] idv_result = Idv::Agent.new(pii_from_doc).proof(*stages) FormResponse.new( success: idv_success(idv_result), diff --git a/app/services/service_provider_seeder.rb b/app/services/service_provider_seeder.rb index 5e940ca1a53..182e7f00868 100644 --- a/app/services/service_provider_seeder.rb +++ b/app/services/service_provider_seeder.rb @@ -10,12 +10,12 @@ def run next unless write_service_provider?(config) ServiceProvider.find_or_create_by!(issuer: issuer) do |sp| - sp.update({ + sp.update( approved: true, active: true, native: true, - friendly_name: config["friendly_name"] - }) + friendly_name: config['friendly_name'], + ) end.update!(config.except('restrict_to_deploy_env', 'uuid_priority')) end end diff --git a/app/views/users/phone_setup/index.html.erb b/app/views/users/phone_setup/index.html.erb index 02b14be4515..e05a3510acf 100644 --- a/app/views/users/phone_setup/index.html.erb +++ b/app/views/users/phone_setup/index.html.erb @@ -12,10 +12,22 @@ <%= t('two_factor_authentication.phone_fee_disclosure') %>

-<%= render 'users/shared/phone_form', - http_method: :patch, - target_url: phone_setup_path, - button: t('forms.buttons.send_security_code') %> +<%= simple_form_for(@new_phone_form, + html: { autocomplete: 'off', method: :patch, role: 'form' }, + data: { international_phone_form: true }, + url: phone_setup_path) do |f| %> + + <%= render 'users/shared/phone_number_edit', f: f %> + + <%= render 'users/shared/otp_delivery_preference_selection', + form_obj: @new_phone_form%> + <% if TwoFactorAuthentication::PhonePolicy.new(current_user).enabled? %> + <%= render 'users/shared/otp_make_default_number', + form_obj: @new_phone_form%> + <% end %> + <%= f.button :submit, t('forms.buttons.send_security_code'), class: 'no-auto-enable btn-wide' %> +<% end %> +
<%= link_to t('two_factor_authentication.choose_another_option'), two_factor_options_path %> diff --git a/app/views/users/phones/add.html.erb b/app/views/users/phones/add.html.erb index b27b8d8e3f8..60785c3e71a 100644 --- a/app/views/users/phones/add.html.erb +++ b/app/views/users/phones/add.html.erb @@ -10,10 +10,21 @@ <%= t('two_factor_authentication.phone_fee_disclosure') %>

-<%= render 'users/shared/phone_form', - http_method: :post, - target_url: add_phone_path, - button: t('forms.buttons.continue') %> +<%= simple_form_for(@new_phone_form, + html: {autocomplete: 'off', method: :post, role: 'form'}, + data: {international_phone_form: true}, + url: add_phone_path) do |f| %> + + <%= render 'users/shared/phone_number_edit', f: f %> + + <%= render 'users/shared/otp_delivery_preference_selection', + form_obj: @new_phone_form %> + <% if TwoFactorAuthentication::PhonePolicy.new(current_user).enabled? %> + <%= render 'users/shared/otp_make_default_number', + form_obj: @new_phone_form %> + <% end %> + <%= f.button :submit, t('forms.buttons.continue'), class: 'no-auto-enable btn-wide' %> +<% end %> <%= render 'shared/cancel', link: account_path %> diff --git a/app/views/users/phones/edit.html.erb b/app/views/users/phones/edit.html.erb index 000524725cf..331c101cce0 100644 --- a/app/views/users/phones/edit.html.erb +++ b/app/views/users/phones/edit.html.erb @@ -1,21 +1,36 @@ -<% title t('titles.edit_info.phone') %>

+<% title t('titles.edit_info.phone') %> +

<%= t('headings.edit_info.phone') %> +

-<%= render 'users/shared/phone_form', - http_method: :put, - target_url: manage_phone_path, - button: t('forms.buttons.submit.confirm_change') %> +<%= simple_form_for(@edit_phone_form, + html: {autocomplete: 'off', method: :put, role: 'form'}, + data: {international_phone_form: true}, + url: manage_phone_path) do |f| %> -<% if MfaPolicy.new(current_user).more_than_two_factors_enabled? && @user_phone_form.phone.present? %> -
+
+ <%= t('two_factor_authentication.phone_label') %>:  + <%= @edit_phone_form.masked_number %> +

+ + <%= render 'users/shared/otp_delivery_preference_selection', + form_obj: @edit_phone_form %> + <% if TwoFactorAuthentication::PhonePolicy.new(current_user).enabled? %> + <%= render 'users/shared/otp_make_default_number', + form_obj: @edit_phone_form %> + <% end %> + <%= f.button :submit, t('forms.buttons.submit.confirm_change'), class: 'no-auto-enable btn-wide' %> +<% end %> + + +<% if MfaPolicy.new(current_user).more_than_two_factors_enabled? %> +
- <%= button_to t('forms.phone.buttons.delete'), manage_phone_url(id: params[:id]), - class: 'btn btn-danger btn-wide', - method: :delete %> + <%= button_to t('forms.phone.buttons.delete'), manage_phone_path(id: params[:id]), + class: 'btn btn-danger btn-wide', + method: :delete %>
<% end %> <%= 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/shared/_otp_delivery_preference_selection.html.erb b/app/views/users/shared/_otp_delivery_preference_selection.html.erb index a293049a6dd..5f47e9d7603 100644 --- a/app/views/users/shared/_otp_delivery_preference_selection.html.erb +++ b/app/views/users/shared/_otp_delivery_preference_selection.html.erb @@ -1,3 +1,10 @@ +<% + form_name = form_obj.class.name.underscore.to_s + form_name_label = form_name + "[otp_delivery_preference]" + form_name_tag_sms = form_name + "_otp_delivery_preference_sms" + form_name_tag_voice = form_name + "_otp_delivery_preference_voice" +%> +
@@ -8,11 +15,11 @@