Handle Twilio errors more gracefully

.reek

@@ -6,6 +6,7 @@ ControlParameter:
     - OpenidConnectRedirector#initialize
     - NoRetryJobs#call
     - PhoneFormatter#self.format
+    - Users::TwoFactorAuthenticationController#invalid_phone_number
 DuplicateMethodCall:
   exclude:
     - ApplicationController#disable_caching
@@ -47,6 +48,7 @@ FeatureEnvy:
     - Utf8Sanitizer#remote_ip
     - Idv::Proofer#validate_vendors
     - PersonalKeyGenerator#create_legacy_recovery_code
+    - TwoFactorAuthenticationController#capture_analytics_for_exception
 InstanceVariableAssumption:
   exclude:
     - User
@@ -105,6 +107,7 @@ TooManyStatements:
     - Upaya::RandomTools#self.random_weighted_sample
     - UserFlowFormatter#stop
     - Upaya::QueueConfig#self.choose_queue_adapter
+    - Users::TwoFactorAuthenticationController#send_code
 TooManyMethods:
   exclude:
     - Users::ConfirmationsController

.rubocop.yml

@@ -62,7 +62,7 @@ Metrics/ClassLength:
   - app/controllers/openid_connect/authorization_controller.rb
   - app/controllers/users/confirmations_controller.rb
   - app/controllers/users/sessions_controller.rb
-  - app/controllers/devise/two_factor_authentication_controller.rb
+  - app/controllers/users/two_factor_authentication_controller.rb
   - app/decorators/service_provider_session_decorator.rb
   - app/decorators/user_decorator.rb
   - app/services/analytics.rb

app/controllers/users/two_factor_authentication_controller.rb

@@ -4,6 +4,7 @@ class TwoFactorAuthenticationController < ApplicationController
 
     before_action :check_remember_device_preference
 
+    # rubocop:disable Metrics/MethodLength
     def show
       if current_user.piv_cac_enabled?
         redirect_to login_two_factor_piv_cac_url
@@ -14,19 +15,23 @@ def show
       else
         redirect_to two_factor_options_url
       end
+    rescue Twilio::REST::RestError, PhoneVerification::VerifyError => exception
+      invalid_phone_number(exception, action: 'show')
     end
+    # rubocop:enable Metrics/MethodLength
 
     def send_code
       result = otp_delivery_selection_form.submit(delivery_params)
       analytics.track_event(Analytics::OTP_DELIVERY_SELECTION, result.to_h)
 
       if result.success?
         handle_valid_otp_delivery_preference(user_selected_otp_delivery_preference)
+        update_otp_delivery_preference_if_needed
       else
         handle_invalid_otp_delivery_preference(result)
       end
     rescue Twilio::REST::RestError, PhoneVerification::VerifyError => exception
-      invalid_phone_number(exception)
+      invalid_phone_number(exception, action: 'send_code')
     end
 
     private
@@ -44,19 +49,56 @@ def validate_otp_delivery_preference_and_send_code
       end
     end
 
+    def update_otp_delivery_preference_if_needed
+      OtpDeliveryPreferenceUpdater.new(
+        user: current_user,
+        preference: delivery_params[:otp_delivery_preference],
+        context: otp_delivery_selection_form.context
+      ).call
+    end
+
     def handle_invalid_otp_delivery_preference(result)
       flash[:error] = result.errors[:phone].first
       preference = current_user.otp_delivery_preference
       redirect_to login_two_factor_url(otp_delivery_preference: preference)
     end
 
-    def invalid_phone_number(exception)
-      code = exception.code
-      analytics.track_event(
-        Analytics::TWILIO_PHONE_VALIDATION_FAILED, error: exception.message, code: code
+    def invalid_phone_number(exception, action:)
+      capture_analytics_for_exception(exception)
+
+      if action == 'show'
+        redirect_to_otp_verification_with_error
+      else
+        flash[:error] = error_message(exception.code)
+        redirect_back(fallback_location: account_url)
+      end
+    end
+
+    def redirect_to_otp_verification_with_error
+      flash[:error] = t('errors.messages.phone_unsupported')
+      redirect_to login_two_factor_url(
+        otp_delivery_preference: current_user.otp_delivery_preference, reauthn: reauthn?
       )
-      flash[:error] = error_message(code)
-      redirect_back(fallback_location: account_url)
+    end
+
+    # rubocop:disable Metrics/MethodLength
+    def capture_analytics_for_exception(exception)
+      attributes = {
+        error: exception.message,
+        code: exception.code,
+        context: context,
+        country: parsed_phone.country,
+      }
+      if exception.is_a?(PhoneVerification::VerifyError)
+        attributes[:status] = exception.status
+        attributes[:response] = exception.response
+      end
+      analytics.track_event(Analytics::TWILIO_PHONE_VALIDATION_FAILED, attributes)
+    end
+    # rubocop:enable Metrics/MethodLength
+
+    def parsed_phone
+      @parsed_phone ||= Phonelib.parse(phone_to_deliver_to)
     end
 
     def error_message(code)
@@ -68,7 +110,9 @@ def twilio_errors
     end
 
     def otp_delivery_selection_form
-      OtpDeliverySelectionForm.new(current_user, phone_to_deliver_to, context)
+      @otp_delivery_selection_form ||= OtpDeliverySelectionForm.new(
+        current_user, phone_to_deliver_to, context
+      )
     end
 
     def reauthn_param

app/forms/otp_delivery_selection_form.rb

@@ -2,7 +2,7 @@ class OtpDeliverySelectionForm
   include ActiveModel::Model
   include OtpDeliveryPreferenceValidator
 
-  attr_reader :otp_delivery_preference, :phone
+  attr_reader :otp_delivery_preference, :phone, :context
 
   validates :otp_delivery_preference, inclusion: { in: %w[sms voice] }
   validates :phone, presence: true
@@ -20,7 +20,6 @@ def submit(params)
     @success = valid?
 
     change_otp_delivery_preference_to_sms if unsupported_phone?
-    update_otp_delivery_preference if should_update_user?
 
     FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
   end
@@ -29,7 +28,7 @@ def submit(params)
 
   attr_writer :otp_delivery_preference
   attr_accessor :resend
-  attr_reader :success, :user, :context
+  attr_reader :success, :user
 
   def change_otp_delivery_preference_to_sms
     user_attributes = { otp_delivery_preference: 'sms' }
@@ -43,24 +42,6 @@ def unsupported_phone?
     error_messages[:phone].first != I18n.t('errors.messages.missing_field')
   end
 
-  def update_otp_delivery_preference
-    user_attributes = { otp_delivery_preference: otp_delivery_preference }
-    UpdateUser.new(user: user, attributes: user_attributes).call
-  end
-
-  def idv_context?
-    context == 'idv'
-  end
-
-  def otp_delivery_preference_changed?
-    otp_delivery_preference != user.otp_delivery_preference
-  end
-
-  def should_update_user?
-    return false if unsupported_phone?
-    success && otp_delivery_preference_changed? && !idv_context?
-  end
-
   def extra_analytics_attributes
     {
       otp_delivery_preference: otp_delivery_preference,

app/models/anonymous_user.rb

@@ -6,4 +6,8 @@ def uuid
   def second_factor_locked_at
     nil
   end
+
+  def phone
+    nil
+  end
 end

app/services/otp_delivery_preference_updater.rb

@@ -0,0 +1,29 @@
+class OtpDeliveryPreferenceUpdater
+  def initialize(user:, preference:, context:)
+    @user = user
+    @preference = preference
+    @context = context
+  end
+
+  def call
+    user_attributes = { otp_delivery_preference: preference }
+    UpdateUser.new(user: user, attributes: user_attributes).call if should_update_user?
+  end
+
+  private
+
+  attr_reader :user, :preference, :context
+
+  def should_update_user?
+    return false unless user
+    otp_delivery_preference_changed? && !idv_context?
+  end
+
+  def otp_delivery_preference_changed?
+    preference != user.otp_delivery_preference
+  end
+
+  def idv_context?
+    context == 'idv'
+  end
+end

app/services/phone_verification.rb

@@ -18,9 +18,18 @@ def initialize(phone:, code:, locale: nil)
     @locale = locale
   end
 
+  # rubocop:disable Style/GuardClause
   def send_sms
-    raise VerifyError.new(code: error_code, message: error_message) unless start_request.success?
+    unless start_request.success?
+      raise VerifyError.new(
+        code: error_code,
+        message: error_message,
+        status: start_request.response_code,
+        response: start_request.response_body
+      )
+    end
   end
+  # rubocop:enable Style/GuardClause
 
   private
 
@@ -36,6 +45,8 @@ def error_message
 
   def response_body
     @response_body ||= JSON.parse(start_request.response_body)
+  rescue JSON::ParserError
+    {}
   end
 
   def start_request
@@ -73,11 +84,13 @@ def country_code
   end
 
   class VerifyError < StandardError
-    attr_reader :code, :message
+    attr_reader :code, :message, :status, :response
 
-    def initialize(code:, message:)
+    def initialize(code:, message:, status:, response:)
       @code = code
       @message = message
+      @status = status
+      @response = response
     end
   end
 end

app/views/exception_notifier/_session.text.erb

@@ -16,6 +16,8 @@ Referer: <%= @request.referer %>
 <% session['user_return_to'] = session['user_return_to']&.split('?')&.first %>
 Session: <%= session %>
 
-User UUID: <%= @kontroller.analytics_user.uuid %>
+<% user = @kontroller.analytics_user %>
+User UUID: <%= user.uuid %>
+User's Country (based on phone): <%= Phonelib.parse(user.phone).country %>
 
 Visitor ID: <%= @request.cookies['ahoy_visitor'] %>

config/application.yml.example

@@ -155,6 +155,7 @@ development:
   piv_cac_service_url: 'https://localhost:8443/'
   piv_cac_verify_token_url: 'https://localhost:8443/'
   pkcs11_lib: '/opt/cloudhsm/lib/libcloudhsm_pkcs11.so'
+  programmable_sms_countries: 'US,CA,MX'
   proofer_mock_fallback: 'true'
   rack_mini_profiler: 'off'
   reauthn_window: '120'
@@ -277,7 +278,7 @@ production:
   piv_cac_agencies: '["DOD"]'
   piv_cac_enabled: 'false'
   pkcs11_lib: '/opt/cloudhsm/lib/libcloudhsm_pkcs11.so'
-  programmable_sms_countries: 'US,CA'
+  programmable_sms_countries: 'US,CA,MX'
   proofer_mock_fallback: 'true'
   reauthn_window: '120'
   recaptcha_enabled_percent: '0'
@@ -401,6 +402,7 @@ test:
   piv_cac_verify_token_secret: '3ac13bfa23e22adae321194c083e783faf89469f6f85dcc0802b27475c94b5c3891b5657bd87d0c1ad65de459166440512f2311018db90d57b15d8ab6660748f'
   piv_cac_verify_token_url: 'https://localhost:8443/'
   pkcs11_lib: '/opt/cloudhsm/lib/libcloudhsm_pkcs11.so'
+  programmable_sms_countries: 'US,CA,MX'
   proofer_mock_fallback: 'true'
   programmable_sms_countries: 'US,CA'
   reauthn_window: '120'

config/initializers/figaro.rb

@@ -30,6 +30,7 @@
   'password_max_attempts',
   'password_pepper',
   'password_strength_enabled',
+  'programmable_sms_countries',
   'queue_health_check_dead_interval_seconds',
   'reauthn_window',
   'recovery_code_length',

config/locales/errors/en.yml

@@ -40,6 +40,8 @@ en:
       otp_incorrect: Incorrect code. Did you type it correctly?
       password_incorrect: Incorrect password
       personal_key_incorrect: Incorrect personal key
+      phone_unsupported: Sorry, we are unable to send SMS at this time. Please try
+        the phone call option below, or use your personal key.
       requires_phone: requires you to enter your phone number.
       twilio_inbound_sms_invalid: The inbound Twilio SMS message failed validation.
       unauthorized_authn_context: Unauthorized authentication context

config/locales/errors/es.yml

@@ -38,6 +38,8 @@ es:
       otp_incorrect: El código es incorrecto. ¿Lo escribió correctamente?
       password_incorrect: La contraseña es incorrecta
       personal_key_incorrect: La clave personal es incorrecta
+      phone_unsupported: Lo sentimos, no podemos enviar SMS en este momento. Pruebe
+        la opción de llamada telefónica a continuación o use su clave personal.
       requires_phone: requiere que ingrese su número de teléfono.
       twilio_inbound_sms_invalid: El mensaje de Twilio SMS de entrada falló la validación.
       unauthorized_authn_context: Contexto de autenticación no autorizado

config/locales/errors/fr.yml

@@ -7,15 +7,15 @@ fr:
     invalid_totp: Code non valide. Veuillez essayer de nouveau.
     max_password_attempts_reached: Vous avez inscrit des mots de passe incorrects
       un trop grand nombre de fois. Vous pouvez réinitialiser votre mot de passe en
-      utilisant le lien « Vous avez oublié votre mot de passe? ».
+      utilisant le lien « Vous avez oublié votre mot de passe? ».
     messages:
       already_confirmed: a déjà été confirmé, veuillez essayer de vous connecter
       blank: Veuillez remplir ce champ.
       confirmation_code_incorrect: Code non valide. L'avez-vous inscrit correctement?
       confirmation_invalid_token: Lien de confirmation non valide. Le lien est expiré
         ou vous avez déjà confirmé votre compte.
       confirmation_period_expired: Lien de confirmation expiré. Vous pouvez cliquer
-        sur « Envoyer les instructions de confirmation de nouveau » pour en obtenir
+        sur « Envoyer les instructions de confirmation de nouveau » pour en obtenir
         un autre.
       expired: est expiré, veuillez en demander un nouveau
       format_mismatch: Veuillez vous assurer de respecter le format requis.
@@ -33,12 +33,16 @@ fr:
       not_found: introuvable
       not_locked: n'a pas été verrouillé
       not_saved:
-        one: '1 erreur a interdit la sauvegarde de cette %{resource} :'
-        other: "%{count} des erreurs ont empêché la sauvegarde de cette %{resource} :"
+        one: '1 erreur a interdit la sauvegarde de cette %{resource} :'
+        other: "%{count} des erreurs ont empêché la sauvegarde de cette %{resource}
+          :"
       otp_failed: NOT TRANSLATED YET
       otp_incorrect: Code non valide. L'avez-vous inscrit correctement?
       password_incorrect: Mot de passe incorrect
       personal_key_incorrect: Clé personnelle incorrecte
+      phone_unsupported: Désolé, nous ne sommes pas en mesure d'envoyer des SMS pour
+        le moment. S'il vous plaît essayez l'option d'appel téléphonique ci-dessous,
+        ou utilisez votre clé personnelle.
       requires_phone: vous demande d'entrer votre numéro de téléphone.
       twilio_inbound_sms_invalid: Le message SMS Twilio entrant a échoué à la validation.
       unauthorized_authn_context: Contexte d'authentification non autorisé