diff --git a/.reek b/.reek index 4883301e48b..54bff8852c2 100644 --- a/.reek +++ b/.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 diff --git a/.rubocop.yml b/.rubocop.yml index 9cb21bca7e5..9af4903ae03 100644 --- a/.rubocop.yml +++ b/.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 diff --git a/app/controllers/users/two_factor_authentication_controller.rb b/app/controllers/users/two_factor_authentication_controller.rb index 2c93a0fd861..b24e0eeaf25 100644 --- a/app/controllers/users/two_factor_authentication_controller.rb +++ b/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,7 +15,10 @@ 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) @@ -22,11 +26,12 @@ def send_code 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 diff --git a/app/forms/otp_delivery_selection_form.rb b/app/forms/otp_delivery_selection_form.rb index 4971439a99f..75b34f515f2 100644 --- a/app/forms/otp_delivery_selection_form.rb +++ b/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, diff --git a/app/models/anonymous_user.rb b/app/models/anonymous_user.rb index 95582e98bae..83eab07e127 100644 --- a/app/models/anonymous_user.rb +++ b/app/models/anonymous_user.rb @@ -6,4 +6,8 @@ def uuid def second_factor_locked_at nil end + + def phone + nil + end end diff --git a/app/services/otp_delivery_preference_updater.rb b/app/services/otp_delivery_preference_updater.rb new file mode 100644 index 00000000000..2713dbbb2a2 --- /dev/null +++ b/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 diff --git a/app/services/phone_verification.rb b/app/services/phone_verification.rb index 94abe87be27..1ecc430869a 100644 --- a/app/services/phone_verification.rb +++ b/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 diff --git a/app/views/exception_notifier/_session.text.erb b/app/views/exception_notifier/_session.text.erb index 65583037bfc..496e1aa9abc 100644 --- a/app/views/exception_notifier/_session.text.erb +++ b/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'] %> diff --git a/config/application.yml.example b/config/application.yml.example index c744ae5d7c3..bd67a6eb170 100644 --- a/config/application.yml.example +++ b/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' diff --git a/config/initializers/figaro.rb b/config/initializers/figaro.rb index ed1d5602f6d..fef45c0cbf7 100644 --- a/config/initializers/figaro.rb +++ b/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', diff --git a/config/locales/errors/en.yml b/config/locales/errors/en.yml index 0302c404593..5dfe8ea8114 100644 --- a/config/locales/errors/en.yml +++ b/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 diff --git a/config/locales/errors/es.yml b/config/locales/errors/es.yml index bb8fc94bd65..3f5cd91d7ed 100644 --- a/config/locales/errors/es.yml +++ b/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 diff --git a/config/locales/errors/fr.yml b/config/locales/errors/fr.yml index 4746589909b..7b14eebc229 100644 --- a/config/locales/errors/fr.yml +++ b/config/locales/errors/fr.yml @@ -7,7 +7,7 @@ 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. @@ -15,7 +15,7 @@ fr: 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é diff --git a/spec/controllers/users/two_factor_authentication_controller_spec.rb b/spec/controllers/users/two_factor_authentication_controller_spec.rb index c87b128c9da..85ef7036ebc 100644 --- a/spec/controllers/users/two_factor_authentication_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_controller_spec.rb @@ -332,11 +332,18 @@ def index } twilio_error = "[HTTP 400] : error message\n\n" + twilio_error_hash = { + error: twilio_error, + code: '', + context: 'confirmation', + country: 'US', + } + expect(@analytics).to receive(:track_event). with(Analytics::OTP_DELIVERY_SELECTION, analytics_hash) expect(@analytics).to receive(:track_event). - with(Analytics::TWILIO_PHONE_VALIDATION_FAILED, error: twilio_error, code: '') + with(Analytics::TWILIO_PHONE_VALIDATION_FAILED, twilio_error_hash) get :send_code, params: { otp_delivery_selection_form: { otp_delivery_preference: 'sms' } } end @@ -345,7 +352,11 @@ def index stub_analytics code = 60_033 error_message = 'error' - verify_error = PhoneVerification::VerifyError.new(code: code, message: error_message) + response = '{"error_code":"60004"}' + status = 400 + verify_error = PhoneVerification::VerifyError.new( + code: code, message: error_message, status: status, response: response + ) allow(SmsOtpSenderJob).to receive(:perform_now).and_raise(verify_error) analytics_hash = { @@ -357,12 +368,20 @@ def index country_code: '1', area_code: '202', } + twilio_error_hash = { + error: error_message, + code: code, + context: 'confirmation', + country: 'US', + status: status, + response: response, + } expect(@analytics).to receive(:track_event). with(Analytics::OTP_DELIVERY_SELECTION, analytics_hash) expect(@analytics).to receive(:track_event). - with(Analytics::TWILIO_PHONE_VALIDATION_FAILED, error: error_message, code: code) + with(Analytics::TWILIO_PHONE_VALIDATION_FAILED, twilio_error_hash) get :send_code, params: { otp_delivery_selection_form: { otp_delivery_preference: 'sms' } } end diff --git a/spec/features/two_factor_authentication/change_factor_spec.rb b/spec/features/two_factor_authentication/change_factor_spec.rb index c4ca885c936..e0125e2acca 100644 --- a/spec/features/two_factor_authentication/change_factor_spec.rb +++ b/spec/features/two_factor_authentication/change_factor_spec.rb @@ -175,6 +175,25 @@ end end + context 'with SMS and number that Verify does not think is valid' do + it 'rescues the VerifyError' do + allow(SmsOtpSenderJob).to receive(:perform_later) + PhoneVerification.adapter = FakeAdapter + allow(FakeAdapter).to receive(:post).and_return(FakeAdapter::ErrorResponse.new) + + user = create(:user, :signed_up, phone: '+17035551212') + visit new_user_session_path + sign_in_live_with_2fa(user) + visit manage_phone_path + select 'Morocco', from: 'user_phone_form_international_code' + fill_in 'user_phone_form_phone', with: '+212 661-289325' + click_button t('forms.buttons.submit.confirm_change') + + expect(current_path).to eq manage_phone_path + expect(page).to have_content t('errors.messages.invalid_phone_number') + end + end + def complete_2fa_confirmation complete_2fa_confirmation_without_entering_otp click_submit_default diff --git a/spec/features/two_factor_authentication/sign_in_spec.rb b/spec/features/two_factor_authentication/sign_in_spec.rb index 6d25a053a85..11ec4f488a9 100644 --- a/spec/features/two_factor_authentication/sign_in_spec.rb +++ b/spec/features/two_factor_authentication/sign_in_spec.rb @@ -562,6 +562,45 @@ def submit_prefilled_otp_code expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'sms') end end + + context 'with SMS and international number that Verify does not think is valid' do + it 'rescues the VerifyError' do + allow(SmsOtpSenderJob).to receive(:perform_later) do |*args| + SmsOtpSenderJob.perform_now(*args) + end + PhoneVerification.adapter = FakeAdapter + allow(FakeAdapter).to receive(:post).and_return(FakeAdapter::ErrorResponse.new) + + user = create(:user, :signed_up, phone: '+212 661-289324') + sign_in_user(user) + + expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'sms') + expect(page). + to have_content t('errors.messages.phone_unsupported') + end + end + + context 'user with Voice preference sends SMS, causing a Twilio error' do + it 'does not change their OTP delivery preference' do + allow(Figaro.env).to receive(:programmable_sms_countries).and_return('CA') + allow(VoiceOtpSenderJob).to receive(:perform_later) + allow(SmsOtpSenderJob).to receive(:perform_later) do |*args| + SmsOtpSenderJob.perform_now(*args) + end + PhoneVerification.adapter = FakeAdapter + allow(FakeAdapter).to receive(:post).and_return(FakeAdapter::ErrorResponse.new) + + user = create(:user, :signed_up, phone: '+17035551212', otp_delivery_preference: 'voice') + sign_in_user(user) + + expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'voice') + + click_link t('links.two_factor_authentication.sms') + + expect(page).to have_content t('errors.messages.invalid_phone_number') + expect(user.reload.otp_delivery_preference).to eq 'voice' + end + end end describe 'when the user is not piv/cac enabled' do @@ -621,6 +660,7 @@ def submit_prefilled_otp_code end end + # TODO: readd profile redirect, modal tests describe 'signing in when user does not already have personal key' do # For example, when migrating users from another DB it 'displays personal key and redirects to profile' do diff --git a/spec/forms/otp_delivery_selection_form_spec.rb b/spec/forms/otp_delivery_selection_form_spec.rb index d281a5dcda7..d1a50d84ae0 100644 --- a/spec/forms/otp_delivery_selection_form_spec.rb +++ b/spec/forms/otp_delivery_selection_form_spec.rb @@ -66,56 +66,38 @@ end end - context 'with authentication context' do - context 'when otp_delivery_preference is the same as the user otp_delivery_preference' do - it 'does not update the user' do - user = build_stubbed(:user, otp_delivery_preference: 'sms') - form = OtpDeliverySelectionForm.new(user, phone_to_deliver_to, 'authentication') - - expect(UpdateUser).to_not receive(:new) - - form.submit(otp_delivery_preference: 'sms') - end - end - - context 'when otp_delivery_preference is different from the user otp_delivery_preference' do - it 'updates the user' do - user = build_stubbed(:user, otp_delivery_preference: 'voice') - form = OtpDeliverySelectionForm.new(user, phone_to_deliver_to, 'authentication') - attributes = { otp_delivery_preference: 'sms' } + context 'with voice preference and unsupported phone' do + it 'changes the otp_delivery_preference to sms' do + user = build_stubbed(:user, otp_delivery_preference: 'voice') + form = OtpDeliverySelectionForm.new( + user, + '+12423270143', + 'authentication' + ) + attributes = { otp_delivery_preference: 'sms' } - updated_user = instance_double(UpdateUser) - allow(UpdateUser).to receive(:new). - with(user: user, attributes: attributes).and_return(updated_user) + updated_user = instance_double(UpdateUser) + allow(UpdateUser).to receive(:new). + with(user: user, attributes: attributes).and_return(updated_user) - expect(updated_user).to receive(:call) + expect(updated_user).to receive(:call) - form.submit(otp_delivery_preference: 'sms') - end + form.submit(otp_delivery_preference: 'voice') end end - context 'with idv context' do - context 'when otp_delivery_preference is the same as the user otp_delivery_preference' do - it 'does not update the user' do - user = build_stubbed(:user, otp_delivery_preference: 'sms') - form = OtpDeliverySelectionForm.new(user, phone_to_deliver_to, 'idv') - - expect(UpdateUser).to_not receive(:new) - - form.submit(otp_delivery_preference: 'sms') - end - end - - context 'when otp_delivery_preference is different from the user otp_delivery_preference' do - it 'does not update the user' do - user = build_stubbed(:user, otp_delivery_preference: 'voice') - form = OtpDeliverySelectionForm.new(user, phone_to_deliver_to, 'idv') + context 'with voice preference and supported phone' do + it 'does not change the otp_delivery_preference to sms' do + user = build_stubbed(:user, otp_delivery_preference: 'voice') + form = OtpDeliverySelectionForm.new( + user, + '+17035551212', + 'authentication' + ) - expect(UpdateUser).to_not receive(:new) + expect(UpdateUser).to_not receive(:new) - form.submit(otp_delivery_preference: 'sms') - end + form.submit(otp_delivery_preference: 'voice') end end end diff --git a/spec/services/otp_delivery_preference_updater_spec.rb b/spec/services/otp_delivery_preference_updater_spec.rb new file mode 100644 index 00000000000..d8b28dc88b4 --- /dev/null +++ b/spec/services/otp_delivery_preference_updater_spec.rb @@ -0,0 +1,89 @@ +require 'rails_helper' + +describe OtpDeliveryPreferenceUpdater do + subject do + OtpDeliveryPreferenceUpdater.new( + user: build_stubbed(:user, otp_delivery_preference: 'sms'), + preference: 'sms', + context: 'authentication' + ) + end + + describe '#call' do + context 'with authentication context' do + context 'when otp_delivery_preference is the same as the user otp_delivery_preference' do + it 'does not update the user' do + expect(UpdateUser).to_not receive(:new) + + subject.call + end + end + + context 'when otp_delivery_preference is different from the user otp_delivery_preference' do + it 'updates the user' do + user = build_stubbed(:user, otp_delivery_preference: 'voice') + updater = OtpDeliveryPreferenceUpdater.new( + user: user, + preference: 'sms', + context: 'authentication' + ) + attributes = { otp_delivery_preference: 'sms' } + + updated_user = instance_double(UpdateUser) + allow(UpdateUser).to receive(:new). + with(user: user, attributes: attributes).and_return(updated_user) + + expect(updated_user).to receive(:call) + + updater.call + end + end + end + + context 'with idv context' do + context 'when otp_delivery_preference is the same as the user otp_delivery_preference' do + it 'does not update the user' do + user = build_stubbed(:user, otp_delivery_preference: 'sms') + updater = OtpDeliveryPreferenceUpdater.new( + user: user, + preference: 'sms', + context: 'idv' + ) + + expect(UpdateUser).to_not receive(:new) + + updater.call + end + end + + context 'when otp_delivery_preference is different from the user otp_delivery_preference' do + it 'does not update the user' do + user = build_stubbed(:user, otp_delivery_preference: 'voice') + updater = OtpDeliveryPreferenceUpdater.new( + user: user, + preference: 'sms', + context: 'idv' + ) + + expect(UpdateUser).to_not receive(:new) + + updater.call + end + end + end + + context 'when user is nil' do + it 'does not update the user' do + updater = OtpDeliveryPreferenceUpdater.new( + user: nil, + preference: 'sms', + context: 'idv' + ) + + expect(UpdateUser).to_not receive(:new) + + updater.call + end + end + end +end diff --git a/spec/services/phone_verification_spec.rb b/spec/services/phone_verification_spec.rb index dbbc692fd1a..86db9f3fe67 100644 --- a/spec/services/phone_verification_spec.rb +++ b/spec/services/phone_verification_spec.rb @@ -45,5 +45,21 @@ expect(error).to be_a(PhoneVerification::VerifyError) end end + + it 'raises VerifyError when response body is not valid JSON' do + PhoneVerification.adapter = FakeAdapter + phone = '17035551212' + code = '123456' + + allow(FakeAdapter).to receive(:post).and_return(FakeAdapter::EmptyResponse.new) + + expect { PhoneVerification.new(phone: phone, code: code).send_sms }.to raise_error do |error| + expect(error.code).to eq 0 + expect(error.message).to eq '' + expect(error.status).to eq 400 + expect(error.response).to eq '' + expect(error).to be_a(PhoneVerification::VerifyError) + end + end end end diff --git a/spec/support/fake_adapter.rb b/spec/support/fake_adapter.rb index f721126e560..628551296c9 100644 --- a/spec/support/fake_adapter.rb +++ b/spec/support/fake_adapter.rb @@ -20,5 +20,23 @@ def response_body message: 'Invalid number', }.to_json end + + def response_code + 400 + end + end + + class EmptyResponse + def success? + false + end + + def response_body + '' + end + + def response_code + 400 + end end end