diff --git a/CHANGELOG.md b/CHANGELOG.md index cd27344dd34..4547443943c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Unreleased ### Improvements/Changes - Layout: Improve layout margins and typographical consistency across several content pages. (#5880, #5884, #5887, #5888, #5908) - Typography: Updated monospace font to Roboto Mono for consistency across login.gov sites. (#5891) +- Multi-factor authentication: Add ability to opt numbers back in to receiving SMS that have replied "STOP" (#5894) - Icons: Replaced custom button icons using U.S. Web Design system icons. (#5904) ### Accessibility diff --git a/Gemfile b/Gemfile index 6cb8edc19b4..7ce472ece20 100644 --- a/Gemfile +++ b/Gemfile @@ -11,6 +11,7 @@ gem 'aws-sdk-kms', '~> 1.4' gem 'aws-sdk-pinpoint' gem 'aws-sdk-pinpointsmsvoice' gem 'aws-sdk-ses', '~> 1.6' +gem 'aws-sdk-sns' gem 'base32-crockford' gem 'blueprinter', '~> 0.25.3' gem 'bootsnap', '~> 1.9.0', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 946af3625aa..46e9627df58 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -136,6 +136,9 @@ GEM aws-sdk-ses (1.44.0) aws-sdk-core (~> 3, >= 3.122.0) aws-sigv4 (~> 1.1) + aws-sdk-sns (1.49.0) + aws-sdk-core (~> 3, >= 3.122.0) + aws-sigv4 (~> 1.1) aws-sigv4 (1.4.0) aws-eventstream (~> 1, >= 1.0.2) axe-core-api (4.3.2) @@ -689,6 +692,7 @@ DEPENDENCIES aws-sdk-pinpoint aws-sdk-pinpointsmsvoice aws-sdk-ses (~> 1.6) + aws-sdk-sns axe-core-rspec (~> 4.2) base32-crockford better_errors (>= 2.5.1) diff --git a/app/controllers/two_factor_authentication/sms_opt_in_controller.rb b/app/controllers/two_factor_authentication/sms_opt_in_controller.rb new file mode 100644 index 00000000000..37c160f6275 --- /dev/null +++ b/app/controllers/two_factor_authentication/sms_opt_in_controller.rb @@ -0,0 +1,97 @@ +module TwoFactorAuthentication + class SmsOptInController < ApplicationController + before_action :load_phone_configuration + + def new + @other_mfa_options_url = other_options_mfa_url + @cancel_url = cancel_url + + analytics.track_event( + Analytics::SMS_OPT_IN_VISIT, + new_user: new_user?, + has_other_auth_methods: has_other_auth_methods?, + phone_configuration_id: @phone_configuration.id, + ) + end + + def create + response = opt_out_manager.opt_in_phone_number(@phone_configuration.formatted_phone) + + analytics.track_event( + Analytics::SMS_OPT_IN_SUBMITTED, + response.to_h.merge( + new_user: new_user?, + has_other_auth_methods: has_other_auth_methods?, + phone_configuration_id: @phone_configuration.id, + ), + ) + + if response.success? + redirect_to otp_send_url(otp_delivery_selection_form: { otp_delivery_preference: :sms }) + else + @other_mfa_options_url = other_options_mfa_url + @cancel_url = cancel_url + + if !response.error + # unsuccessful, but didn't throw an exception: already opted in last 30 days + render :error + else + # one-off error, show form so users can try again + flash[:error] = t('two_factor_authentication.opt_in.error_retry') + render :new + end + end + end + + private + + def opt_out_manager + Telephony::Pinpoint::OptOutManager.new + end + + def mfa_context + @mfa_context ||= MfaContext.new(current_user) + end + + def load_phone_configuration + if user_session.present? && (unconfirmed_phone = user_session[:unconfirmed_phone]).present? + @phone_configuration = PhoneConfiguration.new(phone: unconfirmed_phone) + elsif user_session.present? && (phone_id = user_session[:phone_id]).present? + @phone_configuration = mfa_context.phone_configuration(phone_id) + else + render_not_found + end + end + + def other_options_mfa_url + if new_user? + two_factor_options_path + elsif has_other_auth_methods? && !user_fully_authenticated? + login_two_factor_options_path + end + end + + def cancel_url + if user_fully_authenticated? + account_path + elsif decorated_session.sp_name + return_to_sp_cancel_path + else + sign_out_path + end + end + + def has_other_auth_methods? + two_factor_configurations. + any? { |config| config.mfa_enabled? && config != @phone_configuration } + end + + def new_user? + two_factor_configurations.none? + end + + def two_factor_configurations + @two_factor_configurations ||= mfa_context.two_factor_configurations + end + end +end diff --git a/app/controllers/users/two_factor_authentication_controller.rb b/app/controllers/users/two_factor_authentication_controller.rb index 57da6dd9e0f..96a34c77d63 100644 --- a/app/controllers/users/two_factor_authentication_controller.rb +++ b/app/controllers/users/two_factor_authentication_controller.rb @@ -177,6 +177,12 @@ def handle_telephony_result(method:, default:) otp_make_default_number: default, reauthn: reauthn?, ) + elsif @telephony_result.error.is_a?(Telephony::OptOutError) && + IdentityConfig.store.sms_resubscribe_enabled + # clear message from https://github.com/18F/identity-idp/blob/7ad3feab24f6f9e0e45224d9e9be9458c0a6a648/app/controllers/users/phones_controller.rb#L40 + flash.delete(:info) + user_session[:phone_id] = phone_configuration.id if phone_configuration&.phone + redirect_to login_two_factor_sms_opt_in_path else invalid_phone_number(@telephony_result.error, action: action_name) end diff --git a/app/services/analytics.rb b/app/services/analytics.rb index f0f37e76a48..553470f076a 100644 --- a/app/services/analytics.rb +++ b/app/services/analytics.rb @@ -266,6 +266,8 @@ def browser_attributes SESSION_KEPT_ALIVE = 'Session Kept Alive' SESSION_TOTAL_DURATION_TIMEOUT = 'User Maximum Session Length Exceeded' SIGN_IN_PAGE_VISIT = 'Sign in page visited' + SMS_OPT_IN_SUBMITTED = 'SMS Opt-In: Submitted' + SMS_OPT_IN_VISIT = 'SMS Opt-In: Visited' SP_REDIRECT_INITIATED = 'SP redirect initiated' TELEPHONY_OTP_SENT = 'Telephony: OTP sent' THROTTLER_RATE_LIMIT_TRIGGERED = 'Throttler Rate Limit Triggered' diff --git a/app/views/two_factor_authentication/sms_opt_in/error.html.erb b/app/views/two_factor_authentication/sms_opt_in/error.html.erb new file mode 100644 index 00000000000..8a2a648f89f --- /dev/null +++ b/app/views/two_factor_authentication/sms_opt_in/error.html.erb @@ -0,0 +1,37 @@ +<%= image_tag asset_url('alert/fail-x.svg'), + alt: t('errors.alt.error'), + width: 54, + class: 'margin-bottom-2' %> + +<%= render PageHeadingComponent.new.with_content(t('two_factor_authentication.opt_in.title')) %> + +

+ <%= t( + 'two_factor_authentication.opt_in.opted_out_last_30d_html', + phone_number: content_tag(:strong, @phone_configuration.masked_phone), + ) %> +

+ +

<%= t('two_factor_authentication.opt_in.wait_30d_opt_in') %>

+ +<%= render( + 'shared/troubleshooting_options', + heading_tag: :h3, + heading: t('components.troubleshooting_options.default_heading'), + options: [ + @other_mfa_options_url && { + url: @other_mfa_options_url, + text: t('two_factor_authentication.login_options_link_text'), + }, + decorated_session.sp_name && { + url: return_to_sp_cancel_path(location: 'sms_resubscribe_error'), + text: t('idv.troubleshooting.options.get_help_at_sp', sp_name: decorated_session.sp_name), + }, + { + url: MarketingSite.contact_url, + text: t('links.contact_support', app_name: APP_NAME), + }, + ].select(&:present?), + ) %> + +<%= render 'shared/cancel', link: sign_out_path %> diff --git a/app/views/two_factor_authentication/sms_opt_in/new.html.erb b/app/views/two_factor_authentication/sms_opt_in/new.html.erb new file mode 100644 index 00000000000..c821501a25f --- /dev/null +++ b/app/views/two_factor_authentication/sms_opt_in/new.html.erb @@ -0,0 +1,31 @@ +<%= image_tag asset_url('alert/warning-lg.svg'), + alt: t('errors.alt.warning'), + width: 54, + class: 'margin-bottom-2' %> + +<%= render PageHeadingComponent.new.with_content(t('two_factor_authentication.opt_in.title')) %> + +

+ <%= t( + 'two_factor_authentication.opt_in.opted_out_html', + phone_number: content_tag(:strong, @phone_configuration.masked_phone), + ) %> +

+ +<%= link_to t('two_factor_authentication.mobile_terms_of_service'), + MarketingSite.messaging_practices_url, + class: 'margin-top-2' %> + +<%= button_to t('forms.buttons.send_security_code'), + login_two_factor_sms_opt_in_path, + class: 'usa-button usa-button--wide usa-button--big', + form_class: 'margin-y-5' %> + +<% if @other_mfa_options_url %> +

<%= t('two_factor_authentication.opt_in.cant_use_phone') %>

+ + <%= link_to t('two_factor_authentication.login_options_link_text'), + @other_mfa_options_url %> +<% end %> + +<%= render 'shared/cancel', link: @cancel_url %> diff --git a/config/application.yml.default b/config/application.yml.default index 868f1e9af31..92b693d8969 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -215,6 +215,7 @@ show_select_account_on_repeat_sp_visits: 'false' sp_context_needed_environment: 'prod' sp_handoff_bounce_max_seconds: '2' show_user_attribute_deprecation_warnings: 'false' +sms_resubscribe_enabled: 'false' test_ssn_allowed_list: '' unauthorized_scope_enabled: 'false' usps_upload_enabled: 'false' diff --git a/config/locales/links/en.yml b/config/locales/links/en.yml index 76df3eb9f40..680144f3f8c 100644 --- a/config/locales/links/en.yml +++ b/config/locales/links/en.yml @@ -9,6 +9,7 @@ en: cancel: Cancel cancel_account_creation: '‹ Cancel account creation' contact: Contact + contact_support: 'Contact %{app_name} Support' continue_sign_in: Continue sign in copy: Copy create_account: Create an account diff --git a/config/locales/links/es.yml b/config/locales/links/es.yml index 70c16875252..b75f6c0c3ed 100644 --- a/config/locales/links/es.yml +++ b/config/locales/links/es.yml @@ -9,6 +9,7 @@ es: cancel: Cancelar cancel_account_creation: '‹ Cancelar la creación de cuenta' contact: Contactar + contact_support: 'Póngase en contacto con el soporte técnico de %{app_name}' continue_sign_in: Continuar el inicio de sesión copy: Copiar create_account: Crear cuenta diff --git a/config/locales/links/fr.yml b/config/locales/links/fr.yml index 2ba4c0594a9..3d16d8cc41d 100644 --- a/config/locales/links/fr.yml +++ b/config/locales/links/fr.yml @@ -9,6 +9,7 @@ fr: cancel: Annuler cancel_account_creation: '‹ Annuler la création du compte' contact: Contact + contact_support: 'Contactez %{app_name} le support' continue_sign_in: Continuer la connexion copy: Copier create_account: Créer un compte diff --git a/config/locales/two_factor_authentication/en.yml b/config/locales/two_factor_authentication/en.yml index 26ae2f846b2..e8d45bb3440 100644 --- a/config/locales/two_factor_authentication/en.yml +++ b/config/locales/two_factor_authentication/en.yml @@ -71,6 +71,17 @@ en: incorrectly too many times. mobile_terms_of_service: Mobile terms of service no_auth_option: No authentication option could be found for you to sign in. + opt_in: + cant_use_phone: 'Can’t use your phone?' + error_retry: 'Sorry, we are having trouble opting you in. Please try again.' + opted_out_html: 'You’ve opted out of receiving text messages at %{phone_number}. + You can opt in and receive a security code again to that phone number.' + opted_out_last_30d_html: 'You’ve opted out of receiving text messages at + %{phone_number} within the last 30 days. We can only opt in a phone + number once every 30 days.' + title: We could not send a security code to your phone number + wait_30d_opt_in: 'After 30 days, you can opt in and receive a security code to + that phone number.' otp_delivery_preference: instruction: You can change this selection the next time you sign in. If you entered a landline, please select “Phone call” below. diff --git a/config/locales/two_factor_authentication/es.yml b/config/locales/two_factor_authentication/es.yml index fb07c89e8d9..dbd5968a00c 100644 --- a/config/locales/two_factor_authentication/es.yml +++ b/config/locales/two_factor_authentication/es.yml @@ -77,6 +77,19 @@ es: forma incorrecta demasiadas veces. mobile_terms_of_service: Condiciones de servicio móvil no_auth_option: No se pudo encontrar ninguna opción de autenticación para iniciar sesión + opt_in: + cant_use_phone: '¿No puede utilizar su teléfono?' + error_retry: 'Lo sentimos, estamos teniendo problemas para aceptarlo. Por favor, + inténtelo de nuevo.' + opted_out_html: 'Ha optado por no recibir mensajes de texto en el + %{phone_number}. Puede optar por recibir un código de seguridad de nuevo + a ese número de teléfono.' + opted_out_last_30d_html: 'Canceló su suscripción para recibir mensajes de texto + al %{phone_number} en los últimos 30 días. Solo podemos suscribir un + número telefónico una vez cada 30 días.' + title: 'No hemos podido enviar un código de seguridad a su número de teléfono' + wait_30d_opt_in: 'Después de 30 días, podrá inscribirse y recibir un código de + seguridad para ese número de teléfono.' otp_delivery_preference: instruction: Puede cambiar esta selección la próxima vez que inicie sesión. no_supported_options: No podemos verificar los números de teléfono de %{location} diff --git a/config/locales/two_factor_authentication/fr.yml b/config/locales/two_factor_authentication/fr.yml index 6f2e3cc814a..2ef837b33df 100644 --- a/config/locales/two_factor_authentication/fr.yml +++ b/config/locales/two_factor_authentication/fr.yml @@ -78,6 +78,20 @@ fr: PIV/CAC à de trop nombreuses reprises. mobile_terms_of_service: Conditions de service mobile no_auth_option: Aucune option d’authentification n’a été trouvée pour vous connecter + opt_in: + cant_use_phone: 'Vous ne pouvez pas utiliser votre téléphone?' + error_retry: 'Désolé, nous avons des difficultés à vous connecter. Veuillez + réessayer.' + opted_out_html: 'Vous avez choisi de ne plus recevoir de SMS à %{phone_number}. + Vous pouvez vous inscrire et recevoir à nouveau un code de sécurité à ce + numéro de téléphone.' + opted_out_last_30d_html: 'Vous avez choisi de ne plus recevoir de SMS au + %{phone_number} au cours des 30 derniers jours. Nous ne pouvons opter + pour un numéro de téléphone qu’une fois tous les 30 jours.' + title: 'Nous n’avons pas pu envoyer un code de sécurité à votre numéro de + téléphone' + wait_30d_opt_in: 'Après 30 jours, vous pouvez vous inscrire et recevoir un code + de sécurité à ce numéro de téléphone.' otp_delivery_preference: instruction: Vous pouvez modifier cette sélection lors de votre prochaine connexion. no_supported_options: Nous ne sommes pas en mesure de vérifier les numéros de diff --git a/config/routes.rb b/config/routes.rb index 9a0074c332b..70aa275aeba 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -102,7 +102,9 @@ get '/login/two_factor/:otp_delivery_preference' => 'two_factor_authentication/otp_verification#show', as: :login_two_factor, constraints: { otp_delivery_preference: /sms|voice/ } post '/login/two_factor/:otp_delivery_preference' => 'two_factor_authentication/otp_verification#create', - as: :login_otp + as: :login_otp, constraints: { otp_delivery_preference: /sms|voice/ } + get '/login/two_factor/sms/opt_in' => 'two_factor_authentication/sms_opt_in#new' + post '/login/two_factor/sms/opt_in' => 'two_factor_authentication/sms_opt_in#create' get 'login/add_piv_cac/prompt' => 'users/piv_cac_setup_from_sign_in#prompt' post 'login/add_piv_cac/prompt' => 'users/piv_cac_setup_from_sign_in#decline' diff --git a/lib/identity_config.rb b/lib/identity_config.rb index b5c29e2cdfd..7d3ccd10ab3 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -304,6 +304,7 @@ def self.build_store(config_map) config.add(:show_user_attribute_deprecation_warnings, type: :boolean) config.add(:skip_encryption_allowed_list, type: :json) config.add(:show_select_account_on_repeat_sp_visits, type: :boolean) + config.add(:sms_resubscribe_enabled, type: :boolean) config.add(:sp_context_needed_environment, type: :string) config.add(:sp_handoff_bounce_max_seconds, type: :integer) config.add(:sps_over_quota_limit_notify_email_list, type: :json) diff --git a/lib/telephony.rb b/lib/telephony.rb index 8e6678e9367..dcfa2b1a9d2 100644 --- a/lib/telephony.rb +++ b/lib/telephony.rb @@ -15,6 +15,8 @@ require 'telephony/test/sms_sender' require 'telephony/test/voice_sender' require 'telephony/pinpoint/aws_credential_builder' +require 'telephony/pinpoint/pinpoint_helper' +require 'telephony/pinpoint/opt_out_manager' require 'telephony/pinpoint/sms_sender' require 'telephony/pinpoint/voice_sender' diff --git a/lib/telephony/pinpoint/opt_out_manager.rb b/lib/telephony/pinpoint/opt_out_manager.rb new file mode 100644 index 00000000000..2a5300820b1 --- /dev/null +++ b/lib/telephony/pinpoint/opt_out_manager.rb @@ -0,0 +1,43 @@ +module Telephony + module Pinpoint + class OptOutManager + # @return [Response] + def opt_in_phone_number(phone_number) + Telephony.config.pinpoint.sms_configs.each do |config| + client = build_client(config) + next if client.nil? + + opt_in_response = client.opt_in_phone_number(phone_number: phone_number) + + return Response.new(success: opt_in_response.successful?) + rescue Aws::SNS::Errors::InvalidParameter + # This is thrown when the number has been opted in too recently + return Response.new(success: false) + rescue Seahorse::Client::NetworkingError, + Aws::SNS::Errors::ServiceError => error + PinpointHelper.notify_pinpoint_failover( + error: error, + region: config.region, + channel: :notification_service, + extra: {}, + ) + end + + PinpointHelper.handle_config_failure(:notification_service) + end + + # @api private + # @param [PinpointSmsConfig] config + # @return [nil, Aws::SNS::Client] + def build_client(config) + credentials = AwsCredentialBuilder.new(config).call + return if credentials.nil? + Aws::SNS::Client.new( + region: config.region, + retry_limit: 0, + credentials: credentials, + ) + end + end + end +end diff --git a/lib/telephony/pinpoint/pinpoint_helper.rb b/lib/telephony/pinpoint/pinpoint_helper.rb new file mode 100644 index 00000000000..829b64757b7 --- /dev/null +++ b/lib/telephony/pinpoint/pinpoint_helper.rb @@ -0,0 +1,33 @@ +module Telephony + module Pinpoint + module PinpointHelper + def self.notify_pinpoint_failover(error:, region:, channel:, extra:) + response = Response.new( + success: false, + error: error, + extra: extra.merge( + failover: true, + region: region, + channel: channel, + ), + ) + Telephony.config.logger.warn(response.to_h.to_json) + end + + # @return [Response] + def self.handle_config_failure(channel) + response = Response.new( + success: false, + error: UnknownFailureError.new('Failed to load AWS config'), + extra: { + channel: channel, + }, + ) + + Telephony.config.logger.warn(response.to_h.to_json) + + response + end + end + end +end diff --git a/lib/telephony/pinpoint/sms_sender.rb b/lib/telephony/pinpoint/sms_sender.rb index 2c6a0ad583a..f604f075bf3 100644 --- a/lib/telephony/pinpoint/sms_sender.rb +++ b/lib/telephony/pinpoint/sms_sender.rb @@ -16,7 +16,9 @@ class SmsSender # rubocop:disable Metrics/BlockLength # @return [Response] def send(message:, to:, country_code:, otp: nil) - return handle_config_failure if Telephony.config.pinpoint.sms_configs.empty? + if Telephony.config.pinpoint.sms_configs.empty? + return PinpointHelper.handle_config_failure(:sms) + end response = nil Telephony.config.pinpoint.sms_configs.each do |sms_config| @@ -44,9 +46,10 @@ def send(message:, to:, country_code:, otp: nil) finish = Time.zone.now response = build_response(pinpoint_response, start: start, finish: finish) return response if response.success? - notify_pinpoint_failover( + PinpointHelper.notify_pinpoint_failover( error: response.error, region: sms_config.region, + channel: :sms, extra: response.extra, ) rescue Aws::Pinpoint::Errors::InternalServerErrorException, @@ -54,20 +57,23 @@ def send(message:, to:, country_code:, otp: nil) Seahorse::Client::NetworkingError => e finish = Time.zone.now response = handle_pinpoint_error(e) - notify_pinpoint_failover( + PinpointHelper.notify_pinpoint_failover( error: e, region: sms_config.region, + channel: :sms, extra: { duration_ms: Util.duration_ms(start: start, finish: finish), }, ) end - response || handle_config_failure + response || PinpointHelper.handle_config_failure(:sms) end # rubocop:enable Metrics/BlockLength def phone_info(phone_number) - return handle_config_failure if Telephony.config.pinpoint.sms_configs.empty? + if Telephony.config.pinpoint.sms_configs.empty? + return PinpointHelper.handle_config_failure(:sms) + end response = nil error = nil @@ -82,9 +88,10 @@ def phone_info(phone_number) break if response rescue Seahorse::Client::NetworkingError, Aws::Pinpoint::Errors::InternalServerErrorException => error - notify_pinpoint_failover( + PinpointHelper.notify_pinpoint_failover( error: error, region: sms_config.region, + channel: :sms, extra: {}, ) end @@ -159,35 +166,20 @@ def error(message_response_result) status_code = message_response_result.status_code delivery_status = message_response_result.delivery_status exception_message = "Pinpoint Error: #{delivery_status} - #{status_code}" - exception_class = ERROR_HASH[delivery_status] || TelephonyError + exception_class = if permanent_failure_opt_out?(message_response_result) + OptOutError + else + ERROR_HASH[delivery_status] || TelephonyError + end exception_class.new(exception_message) end - def notify_pinpoint_failover(error:, region:, extra:) - response = Response.new( - success: false, - error: error, - extra: extra.merge( - failover: true, - region: region, - channel: 'sms', - ), - ) - Telephony.config.logger.warn(response.to_h.to_json) - end - - def handle_config_failure - response = Response.new( - success: false, - error: unknown_failure_error, - extra: { - channel: 'sms', - }, - ) - - Telephony.config.logger.warn(response.to_h.to_json) - - response + # Sometimes AWS Pinpoint returns PERMANENT_FAILURE with an "opted out" message + # instead of an OPT_OUT error + # @param [Aws::Pinpoint::Types::MessageResult] message_response_result + def permanent_failure_opt_out?(message_response_result) + message_response_result.delivery_status == 'PERMANENT_FAILURE' && + message_response_result.status_message&.include?('opted out') end def unknown_failure_error diff --git a/lib/telephony/pinpoint/voice_sender.rb b/lib/telephony/pinpoint/voice_sender.rb index 4eda3e28604..7aa4e938d59 100644 --- a/lib/telephony/pinpoint/voice_sender.rb +++ b/lib/telephony/pinpoint/voice_sender.rb @@ -5,7 +5,9 @@ module Pinpoint class VoiceSender # rubocop:disable Metrics/BlockLength def send(message:, to:, country_code:, otp: nil) - return handle_config_failure if Telephony.config.pinpoint.voice_configs.empty? + if Telephony.config.pinpoint.voice_configs.empty? + return PinpointHelper.handle_config_failure(:voice) + end language_code, voice_id = language_code_and_voice_id @@ -38,9 +40,10 @@ def send(message:, to:, country_code:, otp: nil) Seahorse::Client::NetworkingError => e finish = Time.zone.now last_error = handle_pinpoint_error(e) - notify_pinpoint_failover( + PinpointHelper.notify_pinpoint_failover( error: e, region: voice_config.region, + channel: :voice, extra: { message_id: response&.message_id, duration_ms: Util.duration_ms(start: start, finish: finish), @@ -48,7 +51,7 @@ def send(message:, to:, country_code:, otp: nil) ) end - last_error || handle_config_failure + last_error || PinpointHelper.handle_config_failure(:voice) end # rubocop:enable Metrics/BlockLength @@ -85,19 +88,6 @@ def handle_pinpoint_error(err) ) end - def notify_pinpoint_failover(error:, region:, extra:) - response = Response.new( - success: false, - error: error, - extra: extra.merge( - failover: true, - region: region, - channel: 'voice', - ), - ) - Telephony.config.logger.warn(response.to_h.to_json) - end - def language_code_and_voice_id case I18n.locale.to_sym when :en @@ -110,20 +100,6 @@ def language_code_and_voice_id ['en-US', 'Joey'] end end - - def handle_config_failure - response = Response.new( - success: false, - error: UnknownFailureError.new('Failed to load AWS config'), - extra: { - channel: 'sms', - }, - ) - - Telephony.config.logger.warn(response.to_h.to_json) - - response - end end end end diff --git a/lib/telephony/test/error_simulator.rb b/lib/telephony/test/error_simulator.rb index bba052320df..9b4dd8bf1a3 100644 --- a/lib/telephony/test/error_simulator.rb +++ b/lib/telephony/test/error_simulator.rb @@ -1,6 +1,10 @@ +# frozen_string_literal: true + module Telephony module Test class ErrorSimulator + OPT_OUT_PHONE_NUMBER = '2255559999' + def error_for_number(number) cleaned_number = number.gsub(/^\+1/, '').gsub(/\D/, '') case cleaned_number @@ -10,6 +14,8 @@ def error_for_number(number) InvalidPhoneNumberError.new('Simulated phone number error') when '2255552000' InvalidCallingAreaError.new('Simulated calling area error') + when OPT_OUT_PHONE_NUMBER + OptOutError.new('Simulated opt out error') end end end diff --git a/lib/telephony/test/sms_sender.rb b/lib/telephony/test/sms_sender.rb index eeee57de9c5..55147c2eb6e 100644 --- a/lib/telephony/test/sms_sender.rb +++ b/lib/telephony/test/sms_sender.rb @@ -21,6 +21,13 @@ def phone_info(phone_number) type: :voip, carrier: 'Test VOIP Carrier', ) + # Mask opt out errors because we do a phone_info check before trying to send + # so it would prevent us from getting an opt out error where it would actually appaer + when OptOutError + PhoneNumberInfo.new( + type: :mobile, + carrier: 'Test Mobile Carrier', + ) when TelephonyError PhoneNumberInfo.new( type: :unknown, diff --git a/spec/controllers/two_factor_authentication/sms_opt_in_controller_spec.rb b/spec/controllers/two_factor_authentication/sms_opt_in_controller_spec.rb new file mode 100644 index 00000000000..36cd9162c83 --- /dev/null +++ b/spec/controllers/two_factor_authentication/sms_opt_in_controller_spec.rb @@ -0,0 +1,186 @@ +require 'rails_helper' + +RSpec.describe TwoFactorAuthentication::SmsOptInController do + describe '#new' do + subject(:action) { get :new } + + context 'when loaded while using an existing phone' do + let(:user) { create(:user, :with_phone) } + let(:sp_name) { nil } + before do + stub_sign_in_before_2fa(user) + stub_analytics + allow(controller).to receive(:user_session). + and_return(phone_id: user.phone_configurations.first.id) + allow(controller).to receive(:decorated_session). + and_return(instance_double('SessionDecorator', sp_name: sp_name)) + end + + it 'tracks a visit event' do + action + + expect(assigns[:phone_configuration]).to eq(user.phone_configurations.first) + + expect(@analytics).to have_logged_event( + Analytics::SMS_OPT_IN_VISIT, + has_other_auth_methods: false, + phone_configuration_id: user.phone_configurations.first.id, + ) + end + + context 'when the user has other auth methods' do + let(:user) { create(:user, :with_phone, :with_authentication_app) } + + it 'has an other mfa options url' do + action + + expect(assigns[:other_mfa_options_url]).to eq(login_two_factor_options_path) + end + end + + context 'when the user is signing in through an SP' do + let(:sp_name) { 'An Example SP' } + + it 'points the cancel link back to the SP' do + action + + expect(assigns[:cancel_url]).to eq(return_to_sp_cancel_path) + end + end + end + + context 'when loaded while adding a new phone' do + let(:user) { create(:user) } + let(:phone) { Faker::PhoneNumber.cell_phone } + let(:user_session) { { unconfirmed_phone: phone } } + before do + stub_sign_in_before_2fa(user) + allow(controller).to receive(:user_session). + and_return(user_session) + end + + it 'assigns an in-memory phone configuration' do + expect { action }.to_not change { user.reload.phone_configurations.count } + + expect(assigns[:phone_configuration].phone).to eq(phone) + end + + context 'when user_session has both an unconfirmed phone and a phone_id' do + let(:user_session) do + { + unconfirmed_phone: phone, + phone_id: create(:phone_configuration).id, + } + end + + it 'prefers the unconfirmed_phone' do + action + + expect(assigns[:phone_configuration].phone).to eq(phone) + end + end + end + + context 'when loaded without any phone context' do + it 'renders a 404' do + expect(action).to be_not_found + expect(response).to render_template('pages/page_not_found') + end + end + end + + describe '#create' do + subject(:action) { post :create } + + context 'when loaded while using an existing phone' do + let(:user) { create(:user, :with_phone) } + before do + stub_sign_in(user) + stub_analytics + allow(controller).to receive(:user_session). + and_return(phone_id: user.phone_configurations.first.id) + + Telephony.config.pinpoint.add_sms_config do |sms| + sms.region = 'sms-region' + sms.access_key_id = 'fake-pnpoint-access-key-id-sms' + sms.secret_access_key = 'fake-pinpoint-secret-access-key-sms' + sms.application_id = 'backup-sms-application-id' + end + end + + context 'when resubscribing is successful' do + before do + Aws.config[:sns] = { + stub_responses: { + opt_in_phone_number: {}, + }, + } + end + + it 'redirects to the otp send controller' do + expect(action).to redirect_to( + otp_send_url(otp_delivery_selection_form: { otp_delivery_preference: :sms }), + ) + + expect(@analytics).to have_logged_event( + Analytics::SMS_OPT_IN_SUBMITTED, + success: true, + ) + end + end + + context 'when resubscribing is not successful' do + before do + Aws.config[:sns] = { + stub_responses: { + opt_in_phone_number: [ + 'InvalidParameter', + 'Invalid parameter: Cannot opt in right now, latest opt in is too recent', + ], + }, + } + end + + it 'renders an error' do + action + + expect(response).to render_template(:error) + + expect(@analytics).to have_logged_event( + Analytics::SMS_OPT_IN_SUBMITTED, + success: false, + ) + end + end + + context 'when resubscribing throws an error' do + before do + Aws.config[:sns] = { + stub_responses: { + opt_in_phone_number: 'InternalServerErrorException', + }, + } + end + + it 'renders the form with a flash error' do + action + + expect(response).to render_template(:new) + expect(flash[:error]).to be_present + + expect(@analytics).to have_logged_event( + Analytics::SMS_OPT_IN_SUBMITTED, + success: false, + ) + end + end + end + + context 'when loaded without any phone context' do + it 'renders a 404' do + expect(action).to be_not_found + expect(response).to render_template('pages/page_not_found') + 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 cc02f0ef9dc..835aedcb913 100644 --- a/spec/controllers/users/two_factor_authentication_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_controller_spec.rb @@ -344,6 +344,43 @@ def index expect(@user.reload.second_factor_locked_at.to_f).to be_within(0.1).of(Time.zone.now.to_f) end + + context 'when Pinpoint throws an opt-out error' do + before do + @user.phone_configurations.first.tap do |phone_config| + phone_config.phone = Telephony::Test::ErrorSimulator::OPT_OUT_PHONE_NUMBER + phone_config.save! + end + + allow(IdentityConfig.store).to receive(:sms_resubscribe_enabled). + and_return(sms_resubscribe_enabled) + end + + context 'when SMS resubscribing is enabled' do + let(:sms_resubscribe_enabled) { true } + + it 'redirects to the opt in controller' do + get :send_code, params: { + otp_delivery_selection_form: { otp_delivery_preference: 'sms' }, + } + + expect(response).to redirect_to(login_two_factor_sms_opt_in_path) + expect(controller.user_session[:phone_id]).to eq(@user.phone_configurations.first.id) + end + end + + context 'when SMS resubscribing is disabled' do + let(:sms_resubscribe_enabled) { false } + + it 'shows an opt out error flash' do + get :send_code, params: { + otp_delivery_selection_form: { otp_delivery_preference: 'sms' }, + } + + expect(flash[:error]).to eq(I18n.t('telephony.error.friendly_message.opt_out')) + end + end + end end context 'when selecting voice OTP delivery' do diff --git a/spec/lib/telephony/alert_sender_spec.rb b/spec/lib/telephony/alert_sender_spec.rb index bbfc8aae538..d35aa0b40b8 100644 --- a/spec/lib/telephony/alert_sender_spec.rb +++ b/spec/lib/telephony/alert_sender_spec.rb @@ -1,4 +1,6 @@ -describe Telephony::AlertSender do +require 'rails_helper' + +RSpec.describe Telephony::AlertSender do let(:configured_adapter) { :test } let(:recipient) { '+1 (202) 555-5000' } diff --git a/spec/lib/telephony/pinpoint/opt_out_manager_spec.rb b/spec/lib/telephony/pinpoint/opt_out_manager_spec.rb new file mode 100644 index 00000000000..7e7ce8dd4f4 --- /dev/null +++ b/spec/lib/telephony/pinpoint/opt_out_manager_spec.rb @@ -0,0 +1,81 @@ +require 'rails_helper' + +RSpec.describe Telephony::Pinpoint::OptOutManager do + include_context 'telephony' + + subject(:opt_out_manager) { Telephony::Pinpoint::OptOutManager.new } + + let(:phone_number) { Faker::PhoneNumber.cell_phone } + + before do + allow(opt_out_manager).to receive(:build_client). + and_return(first_client, second_client) + end + + let(:first_client) { Aws::SNS::Client.new(stub_responses: true) } + let(:second_client) { Aws::SNS::Client.new(stub_responses: true) } + + describe '#opt_in_phone_number' do + subject(:opt_in) { opt_out_manager.opt_in_phone_number(phone_number) } + + context 'when opting in is successful' do + before do + first_client.stub_responses(:opt_in_phone_number, {}) + end + + it 'has a successful response' do + expect(opt_in.success?).to eq(true) + expect(opt_in.error).to be_nil + end + end + + context 'when opting in is not successful (already opted in w/in last 30 days)' do + before do + first_client.stub_responses( + :opt_in_phone_number, + 'InvalidParameter', + 'Invalid parameter: Cannot opt in right now, latest opt in is too recent', + ) + end + + it 'returns a response that is unsuccessful, but has no error' do + expect(opt_in.success?).to eq(false) + expect(opt_in.error).to be_nil + end + end + + context 'when there is a network error' do + before do + first_client.stub_responses( + :opt_in_phone_number, + 'InternalServerErrorException', + ) + end + + it 'is an unsuccessful response with an error' do + expect(opt_in.success?).to eq(false) + expect(opt_in.error).to be_present + end + + context 'success in the backup region' do + before do + Telephony.config.pinpoint.add_sms_config do |sms| + sms.region = 'backup-sms-region' + sms.access_key_id = 'fake-pnpoint-access-key-id-sms' + sms.secret_access_key = 'fake-pinpoint-secret-access-key-sms' + sms.application_id = 'backup-sms-application-id' + end + + second_client.stub_responses(:opt_in_phone_number, {}) + end + + it 'fails over to the backup region and succeeds' do + expect(Telephony::Pinpoint::PinpointHelper).to receive(:notify_pinpoint_failover) + + expect(opt_in.success?).to eq(true) + expect(opt_in.error).to be_nil + end + end + end + end +end diff --git a/spec/lib/telephony/pinpoint/sms_sender_spec.rb b/spec/lib/telephony/pinpoint/sms_sender_spec.rb index ef91a1117ff..9decf608076 100644 --- a/spec/lib/telephony/pinpoint/sms_sender_spec.rb +++ b/spec/lib/telephony/pinpoint/sms_sender_spec.rb @@ -21,12 +21,14 @@ def ==(other) let(:status_code) { 400 } let(:delivery_status) { 'DUPLICATE' } let(:raised_error_message) { "Pinpoint Error: #{delivery_status} - #{status_code}" } + let(:status_message) { 'some status message' } before do mock_build_client Pinpoint::MockClient.message_response_result_status_code = status_code Pinpoint::MockClient.message_response_result_delivery_status = delivery_status + Pinpoint::MockClient.message_response_result_status_message = status_message end context 'when endpoint is a duplicate' do @@ -66,6 +68,17 @@ def ==(other) expect(response.extra[:delivery_status]).to eq('PERMANENT_FAILURE') expect(response.extra[:request_id]).to eq('fake-message-request-id') end + + context 'when the message indicates opt out' do + let(:status_message) { '+11234567890 is opted out' } + + it 'raises an OptOutError instead' do + response = subject.send(message: 'hello!', to: '+11234567890', country_code: 'US') + + expect(response.success?).to eq(false) + expect(response.error).to eq(Telephony::OptOutError.new(raised_error_message)) + end + end end context 'when a temporary failure occurs' do diff --git a/spec/views/two_factor_authentication/sms_opt_in/error.html.erb_spec.rb b/spec/views/two_factor_authentication/sms_opt_in/error.html.erb_spec.rb new file mode 100644 index 00000000000..0a644d9932c --- /dev/null +++ b/spec/views/two_factor_authentication/sms_opt_in/error.html.erb_spec.rb @@ -0,0 +1,78 @@ +require 'rails_helper' + +RSpec.describe 'two_factor_authentication/sms_opt_in/error.html.erb' do + let(:phone_configuration) { build(:phone_configuration, phone: '1 888-867-5309') } + let(:other_mfa_options_url) { nil } + let(:cancel_url) { nil } + + let(:sp_name) { nil } + let(:decorated_session) { instance_double('SessionDecorator', sp_name: sp_name) } + + before do + assign(:phone_configuration, phone_configuration) + assign(:other_mfa_options_url, other_mfa_options_url) + assign(:cancel_url, cancel_url) + allow(view).to receive(:decorated_session).and_return(decorated_session) + allow(view).to receive(:user_signing_up?).and_return(false) + end + + it 'renders the masked phone number' do + render + + expect(rendered).to have_content('(***) ***-5309') + end + + context 'troubleshooting links' do + context 'without an other_mfa_options_url' do + let(:other_mfa_options_url) { nil } + + it 'omits the other auth methods section' do + render + + expect(rendered).to_not have_content(t('two_factor_authentication.opt_in.cant_use_phone')) + expect(rendered).to_not have_content(t('two_factor_authentication.login_options_link_text')) + end + end + + context 'with an other_mfa_options_url' do + let(:other_mfa_options_url) { '/other' } + + it 'links to other options' do + render + + expect(rendered).to have_link( + t('two_factor_authentication.login_options_link_text'), + href: other_mfa_options_url, + ) + end + end + + context 'with an sp' do + let(:sp_name) { 'An Example SP' } + + it 'links to the sp' do + render + + expect(rendered).to have_link( + t('idv.troubleshooting.options.get_help_at_sp', sp_name: sp_name), + ) + end + end + + context 'without an sp' do + let(:sp_name) { nil } + + it 'has a contact support but not a "Get Help At" link' do + render + + expect(rendered).to have_link( + t('links.contact_support', app_name: APP_NAME), + href: MarketingSite.contact_url, + ) + + doc = Nokogiri::HTML(rendered) + expect(doc.css('.troubleshooting-options__options li').length).to eq(1) + end + end + end +end diff --git a/spec/views/two_factor_authentication/sms_opt_in/new.html.erb_spec.rb b/spec/views/two_factor_authentication/sms_opt_in/new.html.erb_spec.rb new file mode 100644 index 00000000000..26a8d9e7638 --- /dev/null +++ b/spec/views/two_factor_authentication/sms_opt_in/new.html.erb_spec.rb @@ -0,0 +1,47 @@ +require 'rails_helper' + +RSpec.describe 'two_factor_authentication/sms_opt_in/new.html.erb' do + let(:phone_configuration) { build(:phone_configuration, phone: '1-888-867-5309') } + let(:other_mfa_options_url) { nil } + let(:cancel_url) { nil } + + before do + assign(:phone_configuration, phone_configuration) + assign(:other_mfa_options_url, other_mfa_options_url) + assign(:cancel_url, cancel_url) + allow(view).to receive(:user_signing_up?).and_return(false) + end + + it 'renders the masked phone number' do + render + + expect(rendered).to have_content('(***) ***-5309') + end + + context 'other authentication methods' do + context 'without an other_mfa_options_url' do + let(:other_mfa_options_url) { nil } + + it 'omits the other auth methods section' do + render + + expect(rendered).to_not have_content(t('two_factor_authentication.opt_in.cant_use_phone')) + expect(rendered).to_not have_content(t('two_factor_authentication.login_options_link_text')) + end + end + + context 'with an other_mfa_options_url' do + let(:other_mfa_options_url) { '/other' } + + it 'links to other options' do + render + + expect(rendered).to have_content(t('two_factor_authentication.opt_in.cant_use_phone')) + expect(rendered).to have_link( + t('two_factor_authentication.login_options_link_text'), + href: other_mfa_options_url, + ) + end + end + end +end