diff --git a/.circleci/config.yml b/.circleci/config.yml index 05fadbdae3d..14536085ace 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -77,7 +77,6 @@ jobs: cp certs/saml2018.crt.example certs/saml2018.crt cp keys/saml.key.enc.example keys/saml.key.enc cp keys/saml2018.key.enc.example keys/saml2018.key.enc - bin/generate-example-keys bundle exec rake db:setup --trace bundle exec rake assets:precompile diff --git a/.gitignore b/.gitignore index 6085b076fbd..d2424706337 100644 --- a/.gitignore +++ b/.gitignore @@ -40,8 +40,8 @@ Vagrantfile /keys/*.key.enc !/keys/*.key.enc.example /keys/equifax_rsa -/keys/equifax_rsa.pub /keys/equifax_gpg.pub.bin +/keys/equifax_rsa.pub /coverage /db/*.sqlite3 /doc/search_stats.csv @@ -60,6 +60,7 @@ Vagrantfile /vendor/bundle saml_*.txt +saml_*.shr # This is a hack to keep the files that are added to the repo and to prevent git from worrying about # new (transient) files that may be created in those dirs. diff --git a/.reek b/.reek index 7a05f1eb0f0..165c5e1c031 100644 --- a/.reek +++ b/.reek @@ -18,7 +18,6 @@ DuplicateMethodCall: - UserFlowExporter#self.massage_assets - BasicAuthUrl#build - fallback_to_english - - Idv::Proofer#load_vendors! - Upaya::RandomTools#self.random_weighted_sample - SmsController#authenticate FeatureEnvy: @@ -46,7 +45,6 @@ FeatureEnvy: - UserEncryptedAttributeOverrides#find_with_email - Utf8Sanitizer#event_attributes - Utf8Sanitizer#remote_ip - - Idv::Proofer#validate_vendors - TwoFactorAuthenticationController#capture_analytics_for_exception - Users::SessionsController#configure_permitted_parameters - UspsConfirmationExporter#make_entry_row @@ -105,7 +103,6 @@ TooManyStatements: - UserFlowExporter#self.massage_html - UserFlowExporter#self.run - Idv::Agent#proof - - Idv::Proofer#configure_vendors - Idv::VendorResult#initialize - SamlIdpController#auth - Upaya::QueueConfig#self.choose_queue_adapter diff --git a/Gemfile b/Gemfile index 4fa076abaf3..acf84c96d1b 100644 --- a/Gemfile +++ b/Gemfile @@ -113,6 +113,5 @@ end group :production do gem 'aamva', git: 'git@github.com:18F/identity-aamva-api-client-gem', tag: 'v3.1.0' - gem 'equifax', git: 'git@github.com:18F/identity-equifax-api-client-gem.git', tag: 'v1.1.0' gem 'lexisnexis', git: 'git@github.com:18F/identity-lexisnexis-api-client-gem', tag: 'v1.1.0' end diff --git a/Gemfile.lock b/Gemfile.lock index 3e1572c5a97..3a7ee7e011b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -9,19 +9,6 @@ GIT httpi xmldsig -GIT - remote: git@github.com:18F/identity-equifax-api-client-gem.git - revision: de4258c7608997f72e119b16718eeead4d39db70 - tag: v1.1.0 - specs: - equifax (1.1.0) - activesupport - dotenv - gyoku - hashie - logger - savon - GIT remote: git@github.com:18F/identity-lexisnexis-api-client-gem revision: d17049ab1a03d50c0cc8a272d86cf2144192fab5 @@ -350,7 +337,6 @@ GEM rb-fsevent (~> 0.9, >= 0.9.4) rb-inotify (~> 0.9, >= 0.9.7) ruby_dep (~> 1.2) - logger (1.2.8) lograge (0.10.0) actionpack (>= 4) activesupport (>= 4) @@ -694,7 +680,6 @@ DEPENDENCIES devise (~> 4.1) dotiw email_spec - equifax! exception_notification factory_bot_rails fakefs diff --git a/README.md b/README.md index c40d38e8235..2b09568866a 100644 --- a/README.md +++ b/README.md @@ -174,7 +174,7 @@ it into the "Index pattern" field, then click the "Next step" button. 10. On `Step 2 of 2: Configure settings`, select `@timestamp` from the `Time Filter field name` dropdown, then click "Create index pattern". -11. Create some more events on the IdP app +11. Create some more events on the IdP app. 12. Refresh the Kibana website. You should now see new events show up in the Discover section. diff --git a/app/assets/images/carat-right.svg b/app/assets/images/carat-right.svg new file mode 100644 index 00000000000..3eb96ad96ab --- /dev/null +++ b/app/assets/images/carat-right.svg @@ -0,0 +1 @@ +carat-right \ No newline at end of file diff --git a/app/assets/images/sp-logos/sba.png b/app/assets/images/sp-logos/sba.png new file mode 100644 index 00000000000..3393c5fd6d7 Binary files /dev/null and b/app/assets/images/sp-logos/sba.png differ diff --git a/app/assets/stylesheets/components/_profile-section.scss b/app/assets/stylesheets/components/_profile-section.scss index dcfeea6a3e0..fb84149b984 100644 --- a/app/assets/stylesheets/components/_profile-section.scss +++ b/app/assets/stylesheets/components/_profile-section.scss @@ -3,6 +3,7 @@ border-bottom: $border-width solid $border-color; border-radius: 0; margin-bottom: 0; + overflow: hidden; .bg-lightest-blue img { margin-top: -2px; diff --git a/app/assets/stylesheets/components/_spinner.scss b/app/assets/stylesheets/components/_spinner.scss new file mode 100644 index 00000000000..d86fd6a0b2c --- /dev/null +++ b/app/assets/stylesheets/components/_spinner.scss @@ -0,0 +1,5 @@ +.spinner { + margin-left: auto; + margin-right: auto; + width: 144px; +} diff --git a/app/assets/stylesheets/components/_util.scss b/app/assets/stylesheets/components/_util.scss index e7eae1c18a6..c81c2666c03 100644 --- a/app/assets/stylesheets/components/_util.scss +++ b/app/assets/stylesheets/components/_util.scss @@ -6,6 +6,8 @@ .invisible { visibility: hidden; } +.hidden { display: none; } + .truncate-inline { max-width: 80%; overflow: hidden; diff --git a/app/assets/stylesheets/components/all.scss b/app/assets/stylesheets/components/all.scss index 1165aafc80f..bb7b20f5809 100644 --- a/app/assets/stylesheets/components/all.scss +++ b/app/assets/stylesheets/components/all.scss @@ -23,6 +23,7 @@ @import 'accordion'; @import 'util'; @import 'verification-badge'; +@import 'spinner'; @import 'space-addon'; @import 'space-misc'; diff --git a/app/assets/stylesheets/variables/_app.scss b/app/assets/stylesheets/variables/_app.scss index c0aeded5bc1..2125b2ec0fb 100644 --- a/app/assets/stylesheets/variables/_app.scss +++ b/app/assets/stylesheets/variables/_app.scss @@ -13,7 +13,7 @@ $line-height: 1.5 !default; $bold-font-weight: bold !default; $heading-font-family: $serif-font-family !default; $heading-font-weight: bold !default; -$heading-line-height: 1.3 !default; +$heading-line-height: 1.5 !default; $caps-letter-spacing: 1px !default; $line-height-0: .75 !default; // For when a tighter-than-normal leading is desired. diff --git a/app/controllers/account_reset/confirm_request_controller.rb b/app/controllers/account_reset/confirm_request_controller.rb index d6172b22c0b..f3c554062ae 100644 --- a/app/controllers/account_reset/confirm_request_controller.rb +++ b/app/controllers/account_reset/confirm_request_controller.rb @@ -5,7 +5,10 @@ def show if email.blank? redirect_to root_url else - render :show, locals: { email: email } + render :show, locals: { + email: email, sms_phone: SmsLoginOptionPolicy.new(current_user).configured? + } + sign_out end end end diff --git a/app/controllers/account_reset/request_controller.rb b/app/controllers/account_reset/request_controller.rb index 3b16b6d6f7f..b6698c03ca7 100644 --- a/app/controllers/account_reset/request_controller.rb +++ b/app/controllers/account_reset/request_controller.rb @@ -6,13 +6,14 @@ class RequestController < ApplicationController before_action :confirm_two_factor_enabled before_action :confirm_user_not_verified - def show; end + def show + analytics.track_event(Analytics::ACCOUNT_RESET_VISIT) + end def create - analytics.track_event(Analytics::ACCOUNT_RESET, event: :request) - create_request - send_notifications - reset_session_with_email + analytics.track_event(Analytics::ACCOUNT_RESET, analytics_attributes) + AccountReset::CreateRequest.new(current_user).call + flash[:email] = current_user.email redirect_to account_reset_confirm_request_url end @@ -22,36 +23,24 @@ def check_account_reset_enabled redirect_to root_url unless FeatureManagement.account_reset_enabled? end - def confirm_user_not_verified - # IAL2 users should not be able to reset account to comply with AAL2 reqs - redirect_to account_url if decorated_user.identity_verified? - end - - def reset_session_with_email - email = current_user.email - sign_out - flash[:email] = email - end + def confirm_two_factor_enabled + return if current_user.two_factor_enabled? - def send_notifications - phone = current_user.phone - if phone - SmsAccountResetNotifierJob.perform_now( - phone: phone, - cancel_token: current_user.account_reset_request.request_token - ) - end - UserMailer.account_reset_request(current_user).deliver_later + redirect_to two_factor_options_url end - def create_request - AccountResetService.new(current_user).create_request + def confirm_user_not_verified + # IAL2 users should not be able to reset account to comply with AAL2 reqs + redirect_to account_url if decorated_user.identity_verified? end - def confirm_two_factor_enabled - return if current_user.two_factor_enabled? - - redirect_to phone_setup_url + def analytics_attributes + { + event: 'request', + sms_phone: SmsLoginOptionPolicy.new(current_user).configured?, + totp: AuthAppLoginOptionPolicy.new(current_user).configured?, + piv_cac: PivCacLoginOptionPolicy.new(current_user).configured?, + } end end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d852a58b905..9943a413fc3 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -126,7 +126,7 @@ def service_provider_request end def after_sign_in_path_for(_user) - user_session[:stored_location] || sp_session[:request_url] || signed_in_url + user_session.delete(:stored_location) || sp_session[:request_url] || signed_in_url end def signed_in_url diff --git a/app/controllers/concerns/account_recoverable.rb b/app/controllers/concerns/account_recoverable.rb index 5d80977bfff..492401c6c75 100644 --- a/app/controllers/concerns/account_recoverable.rb +++ b/app/controllers/concerns/account_recoverable.rb @@ -1,5 +1,5 @@ module AccountRecoverable def piv_cac_enabled_but_not_phone_enabled? - current_user.piv_cac_enabled? && !current_user.phone_enabled? + current_user.piv_cac_enabled? && !current_user.phone_configuration&.mfa_enabled? end end diff --git a/app/controllers/concerns/authorizable.rb b/app/controllers/concerns/authorizable.rb index 524f3bff347..ced31305fc1 100644 --- a/app/controllers/concerns/authorizable.rb +++ b/app/controllers/concerns/authorizable.rb @@ -1,6 +1,6 @@ module Authorizable def authorize_user - return unless current_user.phone_enabled? + return unless current_user.phone_configuration&.mfa_enabled? if user_fully_authenticated? redirect_to account_url diff --git a/app/controllers/concerns/idv_session.rb b/app/controllers/concerns/idv_session.rb index c0d8abd1074..883f2c840bd 100644 --- a/app/controllers/concerns/idv_session.rb +++ b/app/controllers/concerns/idv_session.rb @@ -2,7 +2,7 @@ module IdvSession extend ActiveSupport::Concern def confirm_idv_session_started - return if current_user.decorate.needs_profile_usps_verification? + return if current_user.decorate.pending_profile_requires_verification? redirect_to idv_session_url if idv_session.params.blank? end diff --git a/app/controllers/concerns/phone_confirmation.rb b/app/controllers/concerns/phone_confirmation.rb index 01cdf412549..3327749e5bb 100644 --- a/app/controllers/concerns/phone_confirmation.rb +++ b/app/controllers/concerns/phone_confirmation.rb @@ -15,6 +15,6 @@ def prompt_to_confirm_phone(phone:, context: 'confirmation', selected_delivery_m def otp_delivery_method(phone, selected_delivery_method) return :sms if PhoneNumberCapabilities.new(phone).sms_only? return selected_delivery_method if selected_delivery_method.present? - current_user.otp_delivery_preference + current_user.phone_configuration&.delivery_preference || current_user.otp_delivery_preference end end diff --git a/app/controllers/concerns/two_factor_authenticatable.rb b/app/controllers/concerns/two_factor_authenticatable.rb index 28f1d05e06f..0ec2683f91a 100644 --- a/app/controllers/concerns/two_factor_authenticatable.rb +++ b/app/controllers/concerns/two_factor_authenticatable.rb @@ -54,7 +54,7 @@ def current_password_required? def check_already_authenticated return unless initial_authentication_context? - redirect_to account_url if user_fully_authenticated? + redirect_to after_otp_verification_confirmation_url if user_fully_authenticated? end def reset_attempt_count_if_user_no_longer_locked_out @@ -140,7 +140,7 @@ def assign_phone end def old_phone - current_user.phone + current_user.phone_configuration&.phone end def phone_changed @@ -260,7 +260,7 @@ def authenticator_view_data two_factor_authentication_method: two_factor_authentication_method, user_email: current_user.email, remember_device_available: false, - phone_enabled: current_user.phone_enabled?, + phone_enabled: current_user.phone_configuration&.mfa_enabled?, }.merge(generic_data) end @@ -282,7 +282,7 @@ def display_phone_to_deliver_to def voice_otp_delivery_unsupported? phone_number = if authentication_context? - current_user.phone + current_user.phone_configuration&.phone else user_session[:unconfirmed_phone] end @@ -297,7 +297,7 @@ def reenter_phone_number_path locale = LinkLocaleResolver.locale if idv_context? idv_phone_path(locale: locale) - elsif current_user.phone.present? + elsif current_user.phone_configuration.present? manage_phone_path(locale: locale) else phone_setup_path(locale: locale) @@ -305,7 +305,7 @@ def reenter_phone_number_path end def confirmation_for_phone_change? - confirmation_context? && current_user.phone.present? + confirmation_context? && current_user.phone_configuration.present? end def presenter_for_two_factor_authentication_method diff --git a/app/controllers/concerns/verify_profile_concern.rb b/app/controllers/concerns/verify_profile_concern.rb index 3b827929f97..709cf7603a7 100644 --- a/app/controllers/concerns/verify_profile_concern.rb +++ b/app/controllers/concerns/verify_profile_concern.rb @@ -11,17 +11,8 @@ def account_or_verify_profile_url def account_or_verify_profile_route return 'account' if idv_context? || profile_context? - return 'account' unless current_user.decorate.pending_profile_requires_verification? - verify_profile_route - end - - def verify_profile_route - decorated_user = current_user.decorate - if decorated_user.needs_profile_phone_verification? - flash[:notice] = t('account.index.verification.instructions') - return 'verify_profile_phone' - end - return 'verify_account' if decorated_user.needs_profile_usps_verification? + return 'account' unless profile_needs_verification? + 'verify_account' end def profile_needs_verification? diff --git a/app/controllers/idv/cancellations_controller.rb b/app/controllers/idv/cancellations_controller.rb index 079ef33c975..6b2fa99a852 100644 --- a/app/controllers/idv/cancellations_controller.rb +++ b/app/controllers/idv/cancellations_controller.rb @@ -6,7 +6,8 @@ class CancellationsController < ApplicationController before_action :confirm_idv_needed def new - analytics.track_event(Analytics::IDV_CANCELLATION) + properties = ParseControllerFromReferer.new(request.referer).call + analytics.track_event(Analytics::IDV_CANCELLATION, properties) @presenter = CancellationPresenter.new(view_context: view_context) end diff --git a/app/controllers/idv/come_back_later_controller.rb b/app/controllers/idv/come_back_later_controller.rb index eee0d92e1cb..cd46a611f74 100644 --- a/app/controllers/idv/come_back_later_controller.rb +++ b/app/controllers/idv/come_back_later_controller.rb @@ -4,12 +4,14 @@ class ComeBackLaterController < ApplicationController before_action :confirm_user_needs_usps_confirmation - def show; end + def show + analytics.track_event(Analytics::IDV_COME_BACK_LATER_VISIT) + end private def confirm_user_needs_usps_confirmation - redirect_to account_url unless current_user.decorate.needs_profile_usps_verification? + redirect_to account_url unless current_user.decorate.pending_profile_requires_verification? end end end diff --git a/app/controllers/idv/confirmations_controller.rb b/app/controllers/idv/confirmations_controller.rb index c1e13ae0357..4bb1ce61c71 100644 --- a/app/controllers/idv/confirmations_controller.rb +++ b/app/controllers/idv/confirmations_controller.rb @@ -35,7 +35,7 @@ def confirm_profile_has_been_created def track_final_idv_event result = { success: true, - new_phone_added: idv_session.params['phone'] != current_user.phone, + new_phone_added: idv_session.params['phone'] != current_user.phone_configuration&.phone, } analytics.track_event(Analytics::IDV_FINAL, result) end diff --git a/app/controllers/idv/otp_delivery_method_controller.rb b/app/controllers/idv/otp_delivery_method_controller.rb index 8978669f80c..e8e0a147542 100644 --- a/app/controllers/idv/otp_delivery_method_controller.rb +++ b/app/controllers/idv/otp_delivery_method_controller.rb @@ -5,19 +5,17 @@ class OtpDeliveryMethodController < ApplicationController before_action :confirm_phone_step_complete before_action :confirm_step_needed - before_action :set_otp_delivery_method_presenter - before_action :set_otp_delivery_selection_form + before_action :idv_phone # Memoize to use ivar in the view - def new; end + def new + analytics.track_event(Analytics::IDV_PHONE_OTP_DELIVERY_SELECTION_VISIT) + end def create - result = @otp_delivery_selection_form.submit(otp_delivery_selection_params) + result = otp_delivery_selection_form.submit(otp_delivery_selection_params) + analytics.track_event(Analytics::IDV_PHONE_OTP_DELIVERY_SELECTION_SUBMITTED, result.to_h) if result.success? - prompt_to_confirm_phone( - phone: @otp_delivery_selection_form.phone, - context: 'idv', - selected_delivery_method: @otp_delivery_selection_form.otp_delivery_preference - ) + prompt_to_confirm_idv_phone else render :new end @@ -26,7 +24,7 @@ def create private def confirm_phone_step_complete - redirect_to idv_review_url if idv_session.vendor_phone_confirmation != true + redirect_to idv_phone_url if idv_session.vendor_phone_confirmation != true end def confirm_step_needed @@ -34,24 +32,26 @@ def confirm_step_needed idv_session.user_phone_confirmation == true end - def otp_delivery_selection_params - params.require(:otp_delivery_selection_form).permit( - :otp_delivery_preference - ) + def idv_phone + @idv_phone ||= PhoneFormatter.format(idv_session.params[:phone]) end - def set_otp_delivery_method_presenter - @set_otp_delivery_method_presenter = Idv::OtpDeliveryMethodPresenter.new( - idv_session.params[:phone] + def prompt_to_confirm_idv_phone + prompt_to_confirm_phone( + phone: idv_phone, + context: 'idv', + selected_delivery_method: otp_delivery_selection_form.otp_delivery_preference ) end - def set_otp_delivery_selection_form - @otp_delivery_selection_form = OtpDeliverySelectionForm.new( - current_user, - idv_session.params[:phone], - 'idv' + def otp_delivery_selection_params + params.require(:otp_delivery_selection_form).permit( + :otp_delivery_preference ) end + + def otp_delivery_selection_form + @otp_delivery_selection_form ||= Idv::OtpDeliveryMethodForm.new + end end end diff --git a/app/controllers/idv/sessions_controller.rb b/app/controllers/idv/sessions_controller.rb index 660a261c37a..4c9ae33a16e 100644 --- a/app/controllers/idv/sessions_controller.rb +++ b/app/controllers/idv/sessions_controller.rb @@ -16,10 +16,10 @@ class SessionsController < ApplicationController delegate :attempts_exceeded?, to: :step, prefix: true def new + analytics.track_event(Analytics::IDV_BASIC_INFO_VISIT) user_session[:context] = 'idv' set_idv_form @selected_state = user_session[:idv_jurisdiction] - analytics.track_event(Analytics::IDV_BASIC_INFO_VISIT) end def create diff --git a/app/controllers/idv/usps_controller.rb b/app/controllers/idv/usps_controller.rb index f42b4fbbeea..362d474e45f 100644 --- a/app/controllers/idv/usps_controller.rb +++ b/app/controllers/idv/usps_controller.rb @@ -12,7 +12,7 @@ def create create_user_event(:usps_mail_sent, current_user) idv_session.address_verification_mechanism = :usps - if current_user.decorate.needs_profile_usps_verification? + if current_user.decorate.pending_profile_requires_verification? resend_letter redirect_to idv_come_back_later_url else diff --git a/app/controllers/two_factor_authentication/otp_verification_controller.rb b/app/controllers/two_factor_authentication/otp_verification_controller.rb index 8e661ac75f5..e86e697162e 100644 --- a/app/controllers/two_factor_authentication/otp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/otp_verification_controller.rb @@ -40,13 +40,12 @@ def confirming_phone? end def phone_enabled? - current_user.phone_enabled? + current_user.phone_configuration&.mfa_enabled? end def confirm_voice_capability return if two_factor_authentication_method == 'sms' - phone = current_user&.phone || user_session[:unconfirmed_phone] capabilities = PhoneNumberCapabilities.new(phone) return unless capabilities.sms_only? @@ -58,6 +57,10 @@ def confirm_voice_capability redirect_to login_two_factor_url(otp_delivery_preference: 'sms', reauthn: reauthn?) end + def phone + current_user&.phone_configuration&.phone || user_session[:unconfirmed_phone] + end + def form_params params.permit(:code) end diff --git a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb index 32ea41c4d2c..19380eee8e7 100644 --- a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb +++ b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb @@ -39,25 +39,32 @@ def handle_valid_piv_cac end def next_step - return account_recovery_setup_url unless current_user.phone_enabled? + return account_recovery_setup_url unless current_user.phone_configuration&.mfa_enabled? after_otp_verification_confirmation_url end def handle_invalid_piv_cac clear_piv_cac_information - # create new nonce for retry - create_piv_cac_nonce handle_invalid_otp(type: 'piv_cac') end + # This overrides the method in TwoFactorAuthenticatable so that we + # redirect back to ourselves rather than rendering the :show template. + # This removes the token from the address bar and preserves the error + # in the flash. + def render_show_after_invalid + flash[:error] = flash.now[:error] + redirect_to login_two_factor_piv_cac_url + end + def piv_cac_view_data { two_factor_authentication_method: two_factor_authentication_method, user_email: current_user.email, remember_device_available: false, totp_enabled: current_user.totp_enabled?, - phone_enabled: current_user.phone_enabled?, + phone_enabled: current_user.phone_configuration&.mfa_enabled?, piv_cac_nonce: piv_cac_nonce, }.merge(generic_data) end diff --git a/app/controllers/users/phone_setup_controller.rb b/app/controllers/users/phone_setup_controller.rb index ead14b0dc7c..adbcf2af193 100644 --- a/app/controllers/users/phone_setup_controller.rb +++ b/app/controllers/users/phone_setup_controller.rb @@ -10,13 +10,13 @@ class PhoneSetupController < ApplicationController def index @user_phone_form = UserPhoneForm.new(current_user) - @presenter = PhoneSetupPresenter.new(current_user.otp_delivery_preference) + @presenter = PhoneSetupPresenter.new(delivery_preference) analytics.track_event(Analytics::USER_REGISTRATION_PHONE_SETUP_VISIT) end def create @user_phone_form = UserPhoneForm.new(current_user) - @presenter = PhoneSetupPresenter.new(current_user.otp_delivery_preference) + @presenter = PhoneSetupPresenter.new(delivery_preference) result = @user_phone_form.submit(user_phone_form_params) analytics.track_event(Analytics::MULTI_FACTOR_AUTH_PHONE_SETUP, result.to_h) @@ -29,6 +29,10 @@ def create private + def delivery_preference + current_user.phone_configuration&.delivery_preference || current_user.otp_delivery_preference + end + def two_factor_enabled? current_user.two_factor_enabled? end diff --git a/app/controllers/users/phones_controller.rb b/app/controllers/users/phones_controller.rb index 32045dd9a28..86f655b7aea 100644 --- a/app/controllers/users/phones_controller.rb +++ b/app/controllers/users/phones_controller.rb @@ -6,12 +6,12 @@ class PhonesController < ReauthnRequiredController def edit @user_phone_form = UserPhoneForm.new(current_user) - @presenter = PhoneSetupPresenter.new(current_user.otp_delivery_preference) + @presenter = PhoneSetupPresenter.new(delivery_preference) end def update @user_phone_form = UserPhoneForm.new(current_user) - @presenter = PhoneSetupPresenter.new(current_user.otp_delivery_preference) + @presenter = PhoneSetupPresenter.new(delivery_preference) if @user_phone_form.submit(user_params).success? process_updates bypass_sign_in current_user @@ -26,6 +26,10 @@ def user_params params.require(:user_phone_form).permit(:phone, :international_code, :otp_delivery_preference) end + def delivery_preference + current_user.phone_configuration&.delivery_preference || current_user.otp_delivery_preference + end + def process_updates if @user_phone_form.phone_changed? analytics.track_event(Analytics::PHONE_CHANGE_REQUESTED) diff --git a/app/controllers/users/piv_cac_authentication_setup_controller.rb b/app/controllers/users/piv_cac_authentication_setup_controller.rb index 1d016e3fcbd..b7df4bbd715 100644 --- a/app/controllers/users/piv_cac_authentication_setup_controller.rb +++ b/app/controllers/users/piv_cac_authentication_setup_controller.rb @@ -13,10 +13,10 @@ class PivCacAuthenticationSetupController < ApplicationController def new if params.key?(:token) process_piv_cac_setup + elsif flash[:error_type].present? + render_error else - analytics.track_event(Analytics::USER_REGISTRATION_PIV_CAC_SETUP_VISIT) - @presenter = PivCacAuthenticationSetupPresenter.new(user_piv_cac_form) - render :new + render_prompt end end @@ -36,6 +36,17 @@ def redirect_to_piv_cac_service private + def render_prompt + analytics.track_event(Analytics::USER_REGISTRATION_PIV_CAC_SETUP_VISIT) + @presenter = PivCacAuthenticationSetupPresenter.new(user_piv_cac_form) + render :new + end + + def render_error + @presenter = PivCacAuthenticationSetupErrorPresenter.new(error: flash[:error_type]) + render :error + end + def two_factor_enabled? current_user.two_factor_enabled? end @@ -68,14 +79,14 @@ def process_valid_submission end def next_step - return account_url if current_user.phone_enabled? + return account_url if current_user.phone_configuration&.mfa_enabled? account_recovery_setup_url end def process_invalid_submission - @presenter = PivCacAuthenticationSetupErrorPresenter.new(user_piv_cac_form) clear_piv_cac_information - render :error + flash[:error_type] = user_piv_cac_form.error_type + redirect_to setup_piv_cac_url end def authorize_piv_cac_disable diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 11e7019ed04..b48093f87c0 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -7,6 +7,7 @@ class SessionsController < Devise::SessionsController skip_before_action :session_expires_at, only: [:active] skip_before_action :require_no_authentication, only: [:new] + before_action :store_sp_metadata_in_session, only: [:new] before_action :check_user_needs_redirect, only: [:new] before_action :apply_secure_headers_override, only: [:new] before_action :configure_permitted_parameters, only: [:new] @@ -88,7 +89,6 @@ def process_locked_out_user def handle_valid_authentication sign_in(resource_name, resource) cache_active_profile - store_sp_metadata_in_session unless request_id.empty? redirect_to user_two_factor_authentication_url end @@ -118,6 +118,7 @@ def track_authentication_attempt(email) user_id: user.uuid, user_locked_out: user_locked_out?(user), stored_location: session['user_return_to'], + sp_request_url_present: sp_session[:request_url].present?, } analytics.track_event(Analytics::EMAIL_AND_PASSWORD_AUTH, properties) @@ -144,12 +145,12 @@ def user_locked_out?(user) end def store_sp_metadata_in_session - return if sp_session[:issuer] + return if sp_session[:issuer] || request_id.empty? StoreSpMetadataInSession.new(session: session, request_id: request_id).call end def request_id - params[:user].fetch(:request_id, '') + params.fetch(:request_id, '') end end end diff --git a/app/controllers/users/two_factor_authentication_controller.rb b/app/controllers/users/two_factor_authentication_controller.rb index 09d2f847de8..e4f285aa902 100644 --- a/app/controllers/users/two_factor_authentication_controller.rb +++ b/app/controllers/users/two_factor_authentication_controller.rb @@ -10,7 +10,7 @@ def show redirect_to login_two_factor_piv_cac_url elsif current_user.totp_enabled? redirect_to login_two_factor_authenticator_url - elsif current_user.phone_enabled? + elsif phone_enabled? validate_otp_delivery_preference_and_send_code else redirect_to two_factor_options_url @@ -36,8 +36,16 @@ def send_code private + def phone_enabled? + phone_configuration&.mfa_enabled? + end + + def phone_configuration + current_user.phone_configuration + end + def validate_otp_delivery_preference_and_send_code - delivery_preference = current_user.otp_delivery_preference + delivery_preference = phone_configuration.delivery_preference result = otp_delivery_selection_form.submit(otp_delivery_preference: delivery_preference) analytics.track_event(Analytics::OTP_DELIVERY_SELECTION, result.to_h) @@ -59,7 +67,7 @@ def update_otp_delivery_preference_if_needed def handle_invalid_otp_delivery_preference(result) flash[:error] = result.errors[:phone].first - preference = current_user.otp_delivery_preference + preference = current_user.phone_configuration.delivery_preference redirect_to login_two_factor_url(otp_delivery_preference: preference) end @@ -77,7 +85,8 @@ def invalid_phone_number(exception, action:) 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? + otp_delivery_preference: current_user.phone_configuration.delivery_preference, + reauthn: reauthn? ) end @@ -170,7 +179,7 @@ def delivery_params end def phone_to_deliver_to - return current_user.phone if authentication_context? + return current_user.phone_configuration.phone if authentication_context? user_session[:unconfirmed_phone] end diff --git a/app/controllers/users/verify_profile_phone_controller.rb b/app/controllers/users/verify_profile_phone_controller.rb deleted file mode 100644 index 050c703b655..00000000000 --- a/app/controllers/users/verify_profile_phone_controller.rb +++ /dev/null @@ -1,44 +0,0 @@ -module Users - class VerifyProfilePhoneController < ApplicationController - include PhoneConfirmation - - before_action :confirm_two_factor_authenticated - before_action :confirm_phone_verification_needed - - def index - prompt_to_confirm_phone(phone: profile_phone, context: 'profile') - end - - private - - def confirm_phone_verification_needed - return if unverified_phone? - redirect_to account_url - end - - def pending_profile_requires_verification? - current_user.decorate.pending_profile_requires_verification? - end - - def unverified_phone? - pending_profile_requires_verification? && - pending_profile.phone_confirmed? && - current_user.phone != profile_phone - end - - def profile_phone - @_profile_phone ||= decrypted_pii.phone.to_s - end - - def pending_profile - @_pending_profile ||= current_user.decorate.pending_profile - end - - def decrypted_pii - @_decrypted_pii ||= begin - cacher = Pii::Cacher.new(current_user, user_session) - cacher.fetch - end - end - end -end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index afc0fec3c8b..dbc27e88ecb 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -10,10 +10,7 @@ def destroy private def track_account_deletion_event - controller_and_action_from_referer = ParseControllerFromReferer.new(request.referer).call - properties = { - request_came_from: controller_and_action_from_referer, - } + properties = ParseControllerFromReferer.new(request.referer).call analytics.track_event(Analytics::ACCOUNT_DELETION, properties) end diff --git a/app/decorators/service_provider_session_decorator.rb b/app/decorators/service_provider_session_decorator.rb index 8e91dce7f04..2d6e511ec95 100644 --- a/app/decorators/service_provider_session_decorator.rb +++ b/app/decorators/service_provider_session_decorator.rb @@ -103,6 +103,10 @@ def cancel_link_url view_context.sign_up_start_url(request_id: sp_session[:request_id]) end + def failure_to_proof_url + sp.failure_to_proof_url || sp_return_url + end + def sp_alert?(path) sp_alert.present? && !sp_alert[:exclude_paths]&.include?(path) end diff --git a/app/decorators/session_decorator.rb b/app/decorators/session_decorator.rb index bac80eb89ba..1a890710051 100644 --- a/app/decorators/session_decorator.rb +++ b/app/decorators/session_decorator.rb @@ -31,6 +31,8 @@ def cancel_link_url view_context.root_url end + def failure_to_proof_url; end + def sp_name; end def sp_agency; end diff --git a/app/decorators/user_decorator.rb b/app/decorators/user_decorator.rb index edda951c676..919595205e3 100644 --- a/app/decorators/user_decorator.rb +++ b/app/decorators/user_decorator.rb @@ -36,7 +36,7 @@ def confirmation_period end def masked_two_factor_phone_number - masked_number(user.phone) + masked_number(user.phone_configuration&.phone) end def active_identity_for(service_provider) @@ -74,14 +74,6 @@ def active_profile_newer_than_pending_profile? user.active_profile.activated_at >= pending_profile.created_at end - def needs_profile_phone_verification? - pending_profile_requires_verification? && pending_profile.phone_confirmed? - end - - def needs_profile_usps_verification? - pending_profile_requires_verification? && !pending_profile.phone_confirmed? - end - # This user's most recently activated profile that has also been deactivated # due to a password reset, or nil if there is no such profile def password_reset_profile diff --git a/app/forms/idv/otp_delivery_method_form.rb b/app/forms/idv/otp_delivery_method_form.rb new file mode 100644 index 00000000000..af9b20a1f62 --- /dev/null +++ b/app/forms/idv/otp_delivery_method_form.rb @@ -0,0 +1,24 @@ +module Idv + class OtpDeliveryMethodForm + include ActiveModel::Model + + attr_reader :otp_delivery_preference + + validates :otp_delivery_preference, inclusion: { in: %w[sms voice] } + + def submit(params) + self.otp_delivery_preference = params[:otp_delivery_preference] + FormResponse.new(success: valid?, errors: errors.messages, extra: extra_analytics_attributes) + end + + private + + attr_writer :otp_delivery_preference + + def extra_analytics_attributes + { + otp_delivery_preference: otp_delivery_preference, + } + end + end +end diff --git a/app/forms/idv/phone_form.rb b/app/forms/idv/phone_form.rb index 67678c29f25..30f4a1420ce 100644 --- a/app/forms/idv/phone_form.rb +++ b/app/forms/idv/phone_form.rb @@ -10,7 +10,7 @@ class PhoneForm def initialize(idv_params, user) @idv_params = idv_params @user = user - self.phone = initial_phone_value(idv_params[:phone] || user.phone) + self.phone = initial_phone_value(idv_params[:phone] || user.phone_configuration&.phone) self.international_code = PhoneFormatter::DEFAULT_COUNTRY end @@ -20,7 +20,7 @@ def submit(params) success = valid? update_idv_params(formatted_phone) if success - FormResponse.new(success: success, errors: errors.messages) + FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes) end private @@ -45,11 +45,22 @@ def update_idv_params(phone) idv_params[:phone] = normalized_phone return idv_params[:phone_confirmed_at] = nil unless phone == formatted_user_phone - idv_params[:phone_confirmed_at] = user.phone_confirmed_at + idv_params[:phone_confirmed_at] = user.phone_configuration&.confirmed_at end def formatted_user_phone - Phonelib.parse(user.phone).international + Phonelib.parse(user.phone_configuration&.phone).international + end + + def parsed_phone + @parsed_phone ||= Phonelib.parse(phone) + end + + def extra_analytics_attributes + { + country_code: parsed_phone.country, + area_code: parsed_phone.area_code, + } end end end diff --git a/app/forms/otp_delivery_selection_form.rb b/app/forms/otp_delivery_selection_form.rb index 75b34f515f2..19053e1b4b1 100644 --- a/app/forms/otp_delivery_selection_form.rb +++ b/app/forms/otp_delivery_selection_form.rb @@ -46,7 +46,7 @@ def extra_analytics_attributes { otp_delivery_preference: otp_delivery_preference, resend: resend, - country_code: parsed_phone.country_code, + country_code: parsed_phone.country, area_code: parsed_phone.area_code, context: context, } diff --git a/app/forms/two_factor_options_form.rb b/app/forms/two_factor_options_form.rb index 6ca9a0b8bec..adb16f78cc1 100644 --- a/app/forms/two_factor_options_form.rb +++ b/app/forms/two_factor_options_form.rb @@ -36,6 +36,7 @@ def extra_analytics_attributes def user_needs_updating? return false unless %w[voice sms].include?(selection) + return false if selection == user.phone_configuration&.delivery_preference selection != user.otp_delivery_preference end diff --git a/app/forms/user_phone_form.rb b/app/forms/user_phone_form.rb index 2d8c18f36e2..3f7edba07af 100644 --- a/app/forms/user_phone_form.rb +++ b/app/forms/user_phone_form.rb @@ -9,9 +9,14 @@ class UserPhoneForm def initialize(user) self.user = user - self.phone = user.phone - self.international_code = Phonelib.parse(phone).country || PhoneFormatter::DEFAULT_COUNTRY - self.otp_delivery_preference = user.otp_delivery_preference + phone_configuration = user.phone_configuration + if phone_configuration.nil? + self.otp_delivery_preference = user.otp_delivery_preference + else + self.phone = phone_configuration.phone + self.international_code = Phonelib.parse(phone).country || PhoneFormatter::DEFAULT_COUNTRY + self.otp_delivery_preference = phone_configuration.delivery_preference + end end def submit(params) @@ -54,7 +59,7 @@ def ingest_submitted_params(params) end def otp_delivery_preference_changed? - otp_delivery_preference != user.otp_delivery_preference + otp_delivery_preference != user.phone_configuration&.delivery_preference end def update_otp_delivery_preference_for_user @@ -63,6 +68,6 @@ def update_otp_delivery_preference_for_user end def formatted_user_phone - Phonelib.parse(user.phone).international + Phonelib.parse(user.phone_configuration.phone).international end end diff --git a/app/helpers/form_helper.rb b/app/helpers/form_helper.rb index cb2e03da771..3c4dfbf0035 100644 --- a/app/helpers/form_helper.rb +++ b/app/helpers/form_helper.rb @@ -83,10 +83,6 @@ def international_phone_codes end end - def unsupported_area_codes - PhoneNumberCapabilities::VOICE_UNSUPPORTED_US_AREA_CODES - end - def supported_jurisdictions Idv::FormJurisdictionValidator::SUPPORTED_JURISDICTIONS end diff --git a/app/javascript/app/phone-internationalization.js b/app/javascript/app/phone-internationalization.js index dd63a714de6..f7054dcbba8 100644 --- a/app/javascript/app/phone-internationalization.js +++ b/app/javascript/app/phone-internationalization.js @@ -1,39 +1,12 @@ -import { PhoneFormatter } from 'field-kit'; - const INTERNATIONAL_CODE_REGEX = /^\+(\d+) |^1 /; const I18n = window.LoginGov.I18n; -const phoneFormatter = new PhoneFormatter(); - -const getPhoneUnsupportedAreaCodeCountry = (areaCode) => { - const form = document.querySelector('[data-international-phone-form]'); - const phoneUnsupportedAreaCodes = JSON.parse(form.dataset.unsupportedAreaCodes); - return phoneUnsupportedAreaCodes[areaCode]; -}; - -const areaCodeFromUSPhone = (phone) => { - const digits = phoneFormatter.digitsWithoutCountryCode(phone); - if (digits.length >= 10) { - return digits.slice(0, 3); - } - return null; -}; const selectedInternationCodeOption = () => { const dropdown = document.querySelector('[data-international-phone-form] .international-code'); return dropdown.item(dropdown.selectedIndex); }; -const unsupportedUSPhoneOTPDeliveryWarningMessage = (phone) => { - const areaCode = areaCodeFromUSPhone(phone); - const country = getPhoneUnsupportedAreaCodeCountry(areaCode); - if (country) { - const messageTemplate = I18n.t('devise.two_factor_authentication.otp_delivery_preference.phone_unsupported'); - return messageTemplate.replace('%{location}', country); - } - return null; -}; - const unsupportedInternationalPhoneOTPDeliveryWarningMessage = () => { const selectedOption = selectedInternationCodeOption(); if (selectedOption.dataset.smsOnly === 'true') { @@ -57,14 +30,6 @@ const enablePhoneState = (phoneRadio, phoneLabel, deliveryMethodHint) => { deliveryMethodHint.innerText = I18n.t('devise.two_factor_authentication.otp_delivery_preference.instruction'); }; -const unsupportedPhoneOTPDeliveryWarningMessage = (phone) => { - const internationCodeOption = selectedInternationCodeOption(); - if (internationCodeOption.dataset.countryCode === '1') { - return unsupportedUSPhoneOTPDeliveryWarningMessage(phone); - } - return unsupportedInternationalPhoneOTPDeliveryWarningMessage(); -}; - const updateOTPDeliveryMethods = () => { const phoneRadio = document.querySelector('[data-international-phone-form] .otp_delivery_preference_voice'); const smsRadio = document.querySelector('[data-international-phone-form] .otp_delivery_preference_sms'); @@ -73,13 +38,10 @@ const updateOTPDeliveryMethods = () => { return; } - const phoneInput = document.querySelector('[data-international-phone-form] .phone'); const phoneLabel = phoneRadio.parentNode.parentNode; const deliveryMethodHint = document.querySelector('#otp_delivery_preference_instruction'); - const phone = phoneInput.value; - - const warningMessage = unsupportedPhoneOTPDeliveryWarningMessage(phone); + const warningMessage = unsupportedInternationalPhoneOTPDeliveryWarningMessage(); if (warningMessage) { disablePhoneState(phoneRadio, phoneLabel, smsRadio, deliveryMethodHint, warningMessage); diff --git a/app/javascript/packs/personal-key-page-controller.js b/app/javascript/packs/personal-key-page-controller.js index 3e0f67e42b5..fd2255d8b21 100644 --- a/app/javascript/packs/personal-key-page-controller.js +++ b/app/javascript/packs/personal-key-page-controller.js @@ -1,3 +1,5 @@ +import base32Crockford from 'base32-crockford-browser'; + const modalSelector = '#personal-key-confirm'; const modal = new window.LoginGov.Modal({ el: modalSelector }); @@ -43,12 +45,31 @@ function resetForm() { unsetInvalidHTML(); } +function formatInput(value) { + // Coerce mistaken user input from 'problem' letters: + // https://en.wikipedia.org/wiki/Base32#Crockford.27s_Base32 + value = base32Crockford.decode(value); + value = base32Crockford.encode(value); + + // Add back the dashes + value = value.toString().match(/.{4}/g).join('-'); + + // And uppercase + return value.toUpperCase(); +} + function handleSubmit(event) { event.preventDefault(); - const value = input.value; + // As above, in case browser lacks HTML5 validation (e.g., IE < 11) + if (input.value.length < 19) { + setInvalidHTML(); + return; + } + + const value = formatInput(input.value); - if (value.toUpperCase() === personalKey) { + if (value === personalKey) { unsetInvalidHTML(); // Recovery code page, without js enabled, has a form submission that posts // to the server with no body. diff --git a/app/jobs/sms_account_reset_notifier_job.rb b/app/jobs/sms_account_reset_notifier_job.rb index 43d5045c07e..12c6cb4d0a7 100644 --- a/app/jobs/sms_account_reset_notifier_job.rb +++ b/app/jobs/sms_account_reset_notifier_job.rb @@ -2,13 +2,13 @@ class SmsAccountResetNotifierJob < ApplicationJob queue_as :sms include Rails.application.routes.url_helpers - def perform(phone:, cancel_token:) + def perform(phone:, token:) TwilioService::Utils.new.send_sms( to: phone, body: I18n.t( 'jobs.sms_account_reset_notifier_job.message', app: APP_NAME, - cancel_link: account_reset_cancel_url(token: cancel_token) + cancel_link: account_reset_cancel_url(token: token) ) ) end diff --git a/app/models/identity.rb b/app/models/identity.rb index 6c5fb9928de..b2c9bb1a9c7 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -29,6 +29,6 @@ def decorate end def piv_cac_available? - PivCacService.piv_cac_available_for_agency?(sp_metadata[:agency]) + PivCacService.piv_cac_available_for_agency?(sp_metadata[:agency], user.email) end end diff --git a/app/models/profile.rb b/app/models/profile.rb index 98aeff5be39..5551d5c8eb4 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -1,4 +1,6 @@ class Profile < ApplicationRecord + self.ignored_columns = %w[phone_confirmed] + belongs_to :user has_many :usps_confirmation_codes, dependent: :destroy diff --git a/app/models/service_provider.rb b/app/models/service_provider.rb index c62d65aa661..240ba646986 100644 --- a/app/models/service_provider.rb +++ b/app/models/service_provider.rb @@ -42,8 +42,8 @@ def live? active? && approved? end - def piv_cac_available? - PivCacService.piv_cac_available_for_agency?(agency) + def piv_cac_available?(user = nil) + PivCacService.piv_cac_available_for_agency?(agency, user&.email) end private diff --git a/app/models/user.rb b/app/models/user.rb index 75cfdfd3a2b..b9357ebe893 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,6 +1,9 @@ # rubocop:disable Rails/HasManyOrHasOneDependent class User < ApplicationRecord - self.ignored_columns = %w[encrypted_password password_salt password_cost] + self.ignored_columns = %w[ + encrypted_password password_salt password_cost encryption_key + recovery_code recovery_cost recovery_salt + ] include NonNullUuid @@ -53,23 +56,19 @@ def confirm_piv_cac?(proposed_uuid) end def piv_cac_enabled? - FeatureManagement.piv_cac_enabled? && x509_dn_uuid.present? + PivCacLoginOptionPolicy.new(self).enabled? end def piv_cac_available? - piv_cac_enabled? || identities.any?(&:piv_cac_available?) + PivCacLoginOptionPolicy.new(self).available? end def need_two_factor_authentication?(_request) two_factor_enabled? end - def phone_enabled? - phone.present? - end - def two_factor_enabled? - phone_enabled? || totp_enabled? || piv_cac_enabled? + phone_configuration&.mfa_enabled? || totp_enabled? || piv_cac_enabled? end def send_two_factor_authentication_code(_code) diff --git a/app/policies/piv_cac_login_option_policy.rb b/app/policies/piv_cac_login_option_policy.rb index aca26fe7c5d..d6e06ca2d57 100644 --- a/app/policies/piv_cac_login_option_policy.rb +++ b/app/policies/piv_cac_login_option_policy.rb @@ -7,6 +7,14 @@ def configured? FeatureManagement.piv_cac_enabled? && user.x509_dn_uuid.present? end + def enabled? + configured? + end + + def available? + enabled? || user.identities.any?(&:piv_cac_available?) + end + private attr_reader :user diff --git a/app/policies/sms_login_option_policy.rb b/app/policies/sms_login_option_policy.rb index 53657192910..f038277f191 100644 --- a/app/policies/sms_login_option_policy.rb +++ b/app/policies/sms_login_option_policy.rb @@ -4,7 +4,7 @@ def initialize(user) end def configured? - user.phone.present? + user.phone_configuration.present? end private diff --git a/app/policies/voice_login_option_policy.rb b/app/policies/voice_login_option_policy.rb index 23ca5d4b8c4..55913a29704 100644 --- a/app/policies/voice_login_option_policy.rb +++ b/app/policies/voice_login_option_policy.rb @@ -12,7 +12,7 @@ def configured? attr_reader :user def user_has_a_phone_number_that_we_can_call? - phone = user.phone + phone = user.phone_configuration&.phone phone.present? && !PhoneNumberCapabilities.new(phone).sms_only? end end diff --git a/app/presenters/idv/idv_failure_presenter.rb b/app/presenters/idv/idv_failure_presenter.rb index bae0cdc260d..4bbaf6d8a6f 100644 --- a/app/presenters/idv/idv_failure_presenter.rb +++ b/app/presenters/idv/idv_failure_presenter.rb @@ -30,24 +30,7 @@ def message end def next_steps - [help_step, sp_step, profile_step].compact - end - - private - - def help_step - link_to t('idv.messages.help_center_html'), MarketingSite.help_url - end - - def sp_step - return unless (sp_name = decorated_session.sp_name) - link = link_to(sp_name, decorated_session.sp_return_url) - t('idv.messages.jurisdiction.sp_support', link: link) - end - - def profile_step - link = link_to(t('idv.messages.jurisdiction.profile_link'), account_path) - t('idv.messages.jurisdiction.profile', link: link) + [] end end end diff --git a/app/presenters/idv/jurisdiction_failure_presenter.rb b/app/presenters/idv/jurisdiction_failure_presenter.rb index 29e18bc0964..b3be3840237 100644 --- a/app/presenters/idv/jurisdiction_failure_presenter.rb +++ b/app/presenters/idv/jurisdiction_failure_presenter.rb @@ -34,7 +34,7 @@ def message end def next_steps - [try_again_step, sp_step, profile_step].compact + [] end private @@ -42,21 +42,5 @@ def next_steps def i18n_args jurisdiction ? { state: state_name_for_abbrev(jurisdiction) } : {} end - - def try_again_step - link = link_to(t('idv.messages.jurisdiction.try_again_link'), idv_jurisdiction_path) - t('idv.messages.jurisdiction.try_again', link: link) - end - - def sp_step - return unless (sp_name = decorated_session.sp_name) - link = link_to(sp_name, decorated_session.sp_return_url) - t('idv.messages.jurisdiction.sp_support', link: link) - end - - def profile_step - link = link_to(t('idv.messages.jurisdiction.profile_link'), account_path) - t('idv.messages.jurisdiction.profile', link: link) - end end end diff --git a/app/presenters/idv/max_attempts_failure_presenter.rb b/app/presenters/idv/max_attempts_failure_presenter.rb index 33df16ab249..d3ff836993c 100644 --- a/app/presenters/idv/max_attempts_failure_presenter.rb +++ b/app/presenters/idv/max_attempts_failure_presenter.rb @@ -32,28 +32,7 @@ def message end def next_steps - [sp_step, help_step, profile_step].compact - end - - private - - def sp_step - return unless (sp_name = decorated_session.sp_name) - link = link_to(sp_name, decorated_session.sp_return_url) - t('idv.messages.jurisdiction.sp_support', link: link) - end - - def help_step - link = link_to( - t('idv.messages.read_about_security_and_privacy.link'), - MarketingSite.help_privacy_and_security_url - ) - t('idv.messages.read_about_security_and_privacy.text', link: link) - end - - def profile_step - link = link_to(t('idv.messages.jurisdiction.profile_link'), account_path) - t('idv.messages.jurisdiction.profile', link: link) + [] end end end diff --git a/app/presenters/idv/otp_delivery_method_presenter.rb b/app/presenters/idv/otp_delivery_method_presenter.rb deleted file mode 100644 index 7cdf0f8802a..00000000000 --- a/app/presenters/idv/otp_delivery_method_presenter.rb +++ /dev/null @@ -1,24 +0,0 @@ -module Idv - class OtpDeliveryMethodPresenter - attr_reader :phone - - delegate :sms_only?, to: :phone_number_capabilites - - def initialize(phone) - @phone = PhoneFormatter.format(phone) - end - - def phone_unsupported_message - I18n.t( - 'devise.two_factor_authentication.otp_delivery_preference.phone_unsupported', - location: phone_number_capabilites.unsupported_location - ) - end - - private - - def phone_number_capabilites - @phone_number_capabilites ||= PhoneNumberCapabilities.new(phone) - end - end -end diff --git a/app/presenters/idv/ssn_failure_presenter.rb b/app/presenters/idv/ssn_failure_presenter.rb index 7250f3c9866..644d128bbd8 100644 --- a/app/presenters/idv/ssn_failure_presenter.rb +++ b/app/presenters/idv/ssn_failure_presenter.rb @@ -31,7 +31,7 @@ def message end def next_steps - [try_again_step, sign_out_step, profile_step] + [] end private diff --git a/app/presenters/piv_cac_authentication_setup_error_presenter.rb b/app/presenters/piv_cac_authentication_setup_error_presenter.rb index 0b00fbdbb84..54e9771a5ea 100644 --- a/app/presenters/piv_cac_authentication_setup_error_presenter.rb +++ b/app/presenters/piv_cac_authentication_setup_error_presenter.rb @@ -1,6 +1,8 @@ class PivCacAuthenticationSetupErrorPresenter < PivCacAuthenticationSetupBasePresenter - def error - form.error_type + attr_accessor :error + + def initialize(error:) + @error = error end def may_select_another_certificate? diff --git a/app/presenters/two_factor_options_presenter.rb b/app/presenters/two_factor_options_presenter.rb index 5ef4a5e360a..60aa9f2e35c 100644 --- a/app/presenters/two_factor_options_presenter.rb +++ b/app/presenters/two_factor_options_presenter.rb @@ -43,7 +43,8 @@ def available_2fa_types def piv_cac_if_available return [] if current_user.piv_cac_enabled? - return [] unless current_user.piv_cac_available? || service_provider&.piv_cac_available? + return [] unless current_user.piv_cac_available? || + service_provider&.piv_cac_available?(current_user) %w[piv_cac] end end diff --git a/app/services/account_reset/cancel.rb b/app/services/account_reset/cancel.rb index 77bba914934..4171cd34e12 100644 --- a/app/services/account_reset/cancel.rb +++ b/app/services/account_reset/cancel.rb @@ -54,7 +54,7 @@ def user end def phone - user.phone + user.phone_configuration&.phone end def extra_analytics_attributes diff --git a/app/services/account_reset/create_request.rb b/app/services/account_reset/create_request.rb new file mode 100644 index 00000000000..ac0ea23b662 --- /dev/null +++ b/app/services/account_reset/create_request.rb @@ -0,0 +1,41 @@ +module AccountReset + class CreateRequest + def initialize(user) + @user = user + end + + def call + create_request + notify_user_by_email + notify_user_by_sms_if_applicable + end + + private + + attr_reader :user + + def create_request + request = AccountResetRequest.find_or_create_by(user: user) + request.update!( + request_token: SecureRandom.uuid, + requested_at: Time.zone.now, + cancelled_at: nil, + granted_at: nil, + granted_token: nil + ) + end + + def notify_user_by_email + UserMailer.account_reset_request(user).deliver_later + end + + def notify_user_by_sms_if_applicable + phone = user.phone_configuration&.phone + return unless phone + SmsAccountResetNotifierJob.perform_now( + phone: phone, + token: user.account_reset_request.request_token + ) + end + end +end diff --git a/app/services/account_reset_service.rb b/app/services/account_reset_service.rb index 43e5fad94cb..b59d8d57bc3 100644 --- a/app/services/account_reset_service.rb +++ b/app/services/account_reset_service.rb @@ -3,15 +3,6 @@ def initialize(user) @user_id = user.id end - def create_request - account_reset = account_reset_request - account_reset.update(request_token: SecureRandom.uuid, - requested_at: Time.zone.now, - cancelled_at: nil, - granted_at: nil, - granted_token: nil) - end - def self.report_fraud(token) account_reset = token.blank? ? nil : AccountResetRequest.find_by(request_token: token) return false unless account_reset diff --git a/app/services/analytics.rb b/app/services/analytics.rb index de4444d0039..788fa6f9edd 100644 --- a/app/services/analytics.rb +++ b/app/services/analytics.rb @@ -56,6 +56,7 @@ def browser # rubocop:disable Metrics/LineLength ACCOUNT_RESET = 'Account Reset'.freeze ACCOUNT_DELETION = 'Account Deletion Requested'.freeze + ACCOUNT_RESET_VISIT = 'Account deletion and reset visited'.freeze ACCOUNT_VISIT = 'Account Page Visited'.freeze EMAIL_AND_PASSWORD_AUTH = 'Email and Password Authentication'.freeze EMAIL_CHANGE_REQUEST = 'Email Change Request'.freeze @@ -64,6 +65,7 @@ def browser IDV_BASIC_INFO_SUBMITTED_VENDOR = 'IdV: basic info vendor submitted'.freeze IDV_CANCELLATION = 'IdV: cancellation visited'.freeze IDV_CANCELLATION_CONFIRMED = 'IdV: cancellation confirmed'.freeze + IDV_COME_BACK_LATER_VISIT = 'IdV: come back later visited'.freeze IDV_MAX_ATTEMPTS_EXCEEDED = 'IdV: max attempts exceeded'.freeze IDV_FINAL = 'IdV: final resolution'.freeze IDV_INTRO_VISIT = 'IdV: intro visited'.freeze @@ -71,6 +73,8 @@ def browser IDV_JURISDICTION_FORM = 'IdV: jurisdiction form submitted'.freeze IDV_PHONE_CONFIRMATION_FORM = 'IdV: phone confirmation form'.freeze IDV_PHONE_CONFIRMATION_VENDOR = 'IdV: phone confirmation vendor'.freeze + IDV_PHONE_OTP_DELIVERY_SELECTION_SUBMITTED = 'IdV: Phone OTP Delivery Selection Submitted'.freeze + IDV_PHONE_OTP_DELIVERY_SELECTION_VISIT = 'IdV: Phone OTP delivery Selection Visited'.freeze IDV_PHONE_RECORD_VISIT = 'IdV: phone of record visited'.freeze IDV_REVIEW_COMPLETE = 'IdV: review complete'.freeze IDV_REVIEW_VISIT = 'IdV: review info visited'.freeze diff --git a/app/services/idv/profile_maker.rb b/app/services/idv/profile_maker.rb index 6a9a51362bf..a5f0fd0f16f 100644 --- a/app/services/idv/profile_maker.rb +++ b/app/services/idv/profile_maker.rb @@ -2,17 +2,15 @@ module Idv class ProfileMaker attr_reader :pii_attributes - def initialize(applicant:, user:, phone_confirmed:, user_password:) + def initialize(applicant:, user:, user_password:) self.pii_attributes = Pii::Attributes.new_from_hash(applicant) self.user = user self.user_password = user_password - self.phone_confirmed = phone_confirmed end def save_profile profile = Profile.new( deactivation_reason: :verification_pending, - phone_confirmed: phone_confirmed, user: user ) profile.encrypt_pii(pii_attributes, user_password) diff --git a/app/services/idv/proofer.rb b/app/services/idv/proofer.rb index 4f9ac4cf409..50eb6902904 100644 --- a/app/services/idv/proofer.rb +++ b/app/services/idv/proofer.rb @@ -18,73 +18,61 @@ def attribute?(key) ATTRIBUTES.include?(key&.to_sym) end - def init - @vendors = configure_vendors(STAGES, configuration) - end - def get_vendor(stage) - @vendors[stage] + stage = stage.to_sym + vendor = vendors[stage] + return vendor if vendor.present? + return unless mock_fallback_enabled? + mock_vendors[stage] end - def configure - yield(configuration) + def validate_vendors! + return if mock_fallback_enabled? + missing_stages = STAGES - vendors.keys + return if missing_stages.empty? + raise "No proofer vendor configured for stage(s): #{missing_stages.join(', ')}" end - def configuration - @configuration ||= Configuration.new - end + private - class Configuration - attr_accessor :mock_fallback, :raise_on_missing_proofers, :vendors - def initialize - @mock_fallback = false - @raise_on_missing_proofers = true - @vendors = [] + def vendors + @vendors ||= begin + require_mock_vendors_if_enabled + available_vendors.each_with_object({}) do |vendor, result| + vendor_stage = vendor.stage&.downcase&.to_sym + next unless STAGES.include?(vendor_stage) + result[vendor_stage] = vendor + end end end - def configure_vendors(stages, config) - external_vendors = loaded_vendors - available_external_vendors = available_vendors(config.vendors, external_vendors) - require_mock_vendors if config.mock_fallback - mock_vendors = loaded_vendors - external_vendors - - vendors = assign_vendors(stages, available_external_vendors, mock_vendors) - - validate_vendors(stages, vendors) if config.raise_on_missing_proofers - - vendors + def available_vendors + external_vendors = ::Proofer::Base.descendants - mock_vendors.values + external_vendors.select do |vendor| + configured_vendor_names.include?(vendor.vendor_name) + end end - private - - def loaded_vendors - ::Proofer::Base.descendants + def configured_vendor_names + JSON.parse(Figaro.env.proofer_vendors || '[]') end - def available_vendors(configured_vendors, vendors) - vendors.select { |vendor| configured_vendors.include?(vendor.vendor_name) } + def mock_vendors + return {} unless mock_fallback_enabled? + { + resolution: ResolutionMock, + state_id: StateIdMock, + address: AddressMock, + } end - def require_mock_vendors + def require_mock_vendors_if_enabled + return unless mock_fallback_enabled? Dir[Rails.root.join('lib', 'proofer_mocks', '*')].each { |file| require file } end - def assign_vendors(stages, external_vendors, mock_vendors) - stages.each_with_object({}) do |stage, vendors| - vendor = stage_vendor(stage, external_vendors) || stage_vendor(stage, mock_vendors) - vendors[stage] = vendor if vendor - end - end - - def stage_vendor(stage, vendors) - vendors.find { |vendor| stage == vendor.stage&.to_sym } - end - - def validate_vendors(stages, vendors) - missing_stages = stages - vendors.keys - return if missing_stages.empty? - raise "No proofer vendor configured for stage(s): #{missing_stages.join(', ')}" + def mock_fallback_enabled? + Figaro.env.proofer_mock_fallback == 'true' end end end diff --git a/app/services/idv/session.rb b/app/services/idv/session.rb index b8182f9f20f..bdc4c12ecf9 100644 --- a/app/services/idv/session.rb +++ b/app/services/idv/session.rb @@ -129,7 +129,6 @@ def applicant_params def build_profile_maker(user_password) Idv::ProfileMaker.new( applicant: applicant_params, - phone_confirmed: vendor_phone_confirmation || false, user: current_user, user_password: user_password ) diff --git a/app/services/otp_delivery_preference_updater.rb b/app/services/otp_delivery_preference_updater.rb index 2713dbbb2a2..15c74563a4a 100644 --- a/app/services/otp_delivery_preference_updater.rb +++ b/app/services/otp_delivery_preference_updater.rb @@ -20,7 +20,9 @@ def should_update_user? end def otp_delivery_preference_changed? - preference != user.otp_delivery_preference + return true if preference != user.otp_delivery_preference + phone_configuration = user.phone_configuration + phone_configuration.present? && preference != phone_configuration.delivery_preference end def idv_context? diff --git a/app/services/parse_controller_from_referer.rb b/app/services/parse_controller_from_referer.rb index 3b4dc6b9a45..2558d5b3544 100644 --- a/app/services/parse_controller_from_referer.rb +++ b/app/services/parse_controller_from_referer.rb @@ -4,9 +4,7 @@ def initialize(referer) end def call - return 'no referer' if referer.nil? - - controller_and_action_from_referer + { request_came_from: controller_and_action_from_referer } end private @@ -14,6 +12,7 @@ def call attr_reader :referer def controller_and_action_from_referer + return 'no referer' if referer.nil? "#{controller_that_made_the_request}##{controller_action}" end diff --git a/app/services/phone_number_capabilities.rb b/app/services/phone_number_capabilities.rb index 09fc756b66b..7c1e1f7458d 100644 --- a/app/services/phone_number_capabilities.rb +++ b/app/services/phone_number_capabilities.rb @@ -1,26 +1,4 @@ class PhoneNumberCapabilities - VOICE_UNSUPPORTED_US_AREA_CODES = { - '264' => 'Anguilla', - '268' => 'Antigua and Barbuda', - '242' => 'Bahamas', - '246' => 'Barbados', - '441' => 'Bermuda', - '284' => 'British Virgin Islands', - '345' => 'Cayman Islands', - '767' => 'Dominica', - '809' => 'Dominican Republic', - '829' => 'Dominican Republic', - '849' => 'Dominican Republic', - '473' => 'Grenada', - '876' => 'Jamaica', - '664' => 'Montserrat', - '869' => 'Saint Kitts and Nevis', - '758' => 'Saint Lucia', - '784' => 'Saint Vincent Grenadines', - '868' => 'Trinidad and Tobago', - '649' => 'Turks and Caicos Islands', - }.freeze - INTERNATIONAL_CODES = YAML.load_file( Rails.root.join('config', 'country_dialing_codes.yml') ).freeze @@ -32,38 +10,27 @@ def initialize(phone) end def sms_only? - if international_code == '1' - VOICE_UNSUPPORTED_US_AREA_CODES[area_code].present? - elsif country_code_data - country_code_data['sms_only'] - end + return true if country_code_data.nil? + country_code_data['sms_only'] end def unsupported_location - if international_code == '1' - VOICE_UNSUPPORTED_US_AREA_CODES[area_code] - elsif country_code_data - country_code_data['name'] - end + country_code_data['name'] if country_code_data end private - def area_code - @area_code ||= parsed_phone.area_code - end - def country_code_data - @country_code_data ||= INTERNATIONAL_CODES.select do |_, value| - value['country_code'] == international_code + @country_code_data ||= INTERNATIONAL_CODES.select do |key, _| + key == two_letter_country_code end.values.first end - def international_code - @international_code ||= parsed_phone.country_code + def two_letter_country_code + parsed_phone.country end def parsed_phone - @parsed_phone ||= Phonelib.parse(phone) + Phonelib.parse(phone) end end diff --git a/app/services/piv_cac_service.rb b/app/services/piv_cac_service.rb index 6de250dc710..20464254015 100644 --- a/app/services/piv_cac_service.rb +++ b/app/services/piv_cac_service.rb @@ -28,19 +28,46 @@ def piv_cac_verify_token_link Figaro.env.piv_cac_verify_token_url end - def piv_cac_available_for_agency?(agency) - return if agency.blank? + def piv_cac_available_for_agency?(agency, email = nil) return unless FeatureManagement.piv_cac_enabled? - @piv_cac_agencies ||= begin - piv_cac_agencies = Figaro.env.piv_cac_agencies || '[]' - JSON.parse(piv_cac_agencies) - end - - @piv_cac_agencies.include?(agency) + available_for_agency?(agency) || available_for_email?(agency, email) end private + def available_for_agency?(agency) + return if agency.blank? + piv_cac_agencies = JSON.parse(Figaro.env.piv_cac_agencies || '[]') + piv_cac_agencies.include?(agency) + end + + def available_for_email?(agency, email) + return unless email.present? && agency_scoped_by_email?(agency) + + piv_cac_email_domains = Figaro.env.piv_cac_email_domains || '[]' + + (_, email_domain) = email.split(/@/, 2) + domains = JSON.parse(piv_cac_email_domains) + domains.any? { |supported_domain| domain_match?(email_domain, supported_domain) } + end + + def agency_scoped_by_email?(agency) + return if agency.blank? + + piv_cac_agencies_email_scope = + JSON.parse(Figaro.env.piv_cac_agencies_scoped_by_email || '[]') + + piv_cac_agencies_email_scope.include?(agency) + end + + def domain_match?(given, matcher) + if matcher[0] == '.' + given.end_with?(matcher) + else + given == matcher + end + end + def randomize_uri(uri) # we only support {random}, so we're going for performance here uri.gsub('{random}') { |_| SecureRandom.hex(RANDOM_HOSTNAME_BYTES) } @@ -49,6 +76,7 @@ def randomize_uri(uri) # Only used in tests def reset_piv_cac_avaialable_agencies @piv_cac_agencies = nil + @piv_cac_agencies_email_scope = nil end def token_present(token) diff --git a/app/services/populate_phone_configurations_table.rb b/app/services/populate_phone_configurations_table.rb index 672e7aa85d2..a1c123da5ec 100644 --- a/app/services/populate_phone_configurations_table.rb +++ b/app/services/populate_phone_configurations_table.rb @@ -1,9 +1,20 @@ class PopulatePhoneConfigurationsTable + def initialize + @count = 0 + @total = 0 + end + + # :reek:DuplicateMethodCall def call # we don't have a uniqueness constraint in the database to let us blindly insert # everything in a single SQL statement. So we have to load by batches and copy # over. Much slower, but doesn't duplicate information. - User.in_batches(of: 1000) { |relation| process_batch(relation) } + User.in_batches(of: 1000) do |relation| + sleep(1) + process_batch(relation) + Rails.logger.info "#{@count} / #{@total}" + end + Rails.logger.info "Processed #{@count} user phone configurations" end private @@ -12,15 +23,17 @@ def call def process_batch(relation) User.transaction do relation.each do |user| - next if user.phone_configuration.present? || user.phone.blank? + @total += 1 + next if user.phone_configuration.present? || user.encrypted_phone.blank? user.create_phone_configuration(phone_info_for_user(user)) + @count += 1 end end end def phone_info_for_user(user) { - phone: user.phone, + encrypted_phone: user.encrypted_phone, confirmed_at: user.phone_confirmed_at, delivery_preference: user.otp_delivery_preference, } diff --git a/app/services/remember_device_cookie.rb b/app/services/remember_device_cookie.rb index 93c0ff1f08d..28a916d39d3 100644 --- a/app/services/remember_device_cookie.rb +++ b/app/services/remember_device_cookie.rb @@ -46,6 +46,6 @@ def expired? end def user_has_changed_phone?(user) - user.phone_confirmed_at.to_i > created_at.to_i + user.phone_configuration&.confirmed_at.to_i > created_at.to_i end end diff --git a/app/services/request_key_manager.rb b/app/services/request_key_manager.rb index 4fda5fbbe44..6144e7f39db 100644 --- a/app/services/request_key_manager.rb +++ b/app/services/request_key_manager.rb @@ -19,9 +19,4 @@ def self.read_key_file(key_file, passphrase) key_file = Rails.root.join('keys', 'saml.key.enc') read_key_file(key_file, Figaro.env.saml_passphrase) end - - cattr_accessor :equifax_ssh_key do - key_file = Rails.root.join('keys', 'equifax_rsa') - read_key_file(key_file, Figaro.env.equifax_ssh_passphrase) - end end diff --git a/app/view_models/account_show.rb b/app/view_models/account_show.rb index 2a4c3b4ee98..45fb06b4db7 100644 --- a/app/view_models/account_show.rb +++ b/app/view_models/account_show.rb @@ -33,10 +33,8 @@ def password_reset_partial end def pending_profile_partial - if decorated_user.needs_profile_usps_verification? + if decorated_user.pending_profile_requires_verification? 'accounts/pending_profile_usps' - elsif decorated_user.needs_profile_phone_verification? - 'accounts/pending_profile_phone' else 'shared/null' end diff --git a/app/views/account_reset/confirm_request/show.html.slim b/app/views/account_reset/confirm_request/show.html.slim index 7ba5b9833b4..54660aea114 100644 --- a/app/views/account_reset/confirm_request/show.html.slim +++ b/app/views/account_reset/confirm_request/show.html.slim @@ -6,3 +6,8 @@ h1.mt1.mb-12p.h3 = t('headings.verify_email') p == t('account_reset.confirm_request.instructions', email: email) + - if sms_phone + p + == t('account_reset.confirm_request.security_note') + p + == t('account_reset.confirm_request.close_window') diff --git a/app/views/accounts/_pending_profile_phone.html.slim b/app/views/accounts/_pending_profile_phone.html.slim deleted file mode 100644 index 14dc02731c8..00000000000 --- a/app/views/accounts/_pending_profile_phone.html.slim +++ /dev/null @@ -1,3 +0,0 @@ -.mb4.alert.alert-warning - p = t('account.index.verification.instructions') - p.mb0 = link_to t('account.index.verification.with_phone_button'), verify_profile_phone_path diff --git a/app/views/accounts/show.html.slim b/app/views/accounts/show.html.slim index f704a423588..acd5c7d55b2 100644 --- a/app/views/accounts/show.html.slim +++ b/app/views/accounts/show.html.slim @@ -36,7 +36,7 @@ h1.hide = t('titles.account') = render 'account_item', name: t('account.index.phone'), - content: current_user.phone, + content: current_user.phone_configuration&.phone, path: manage_phone_path, action: @view_model.edit_action_partial diff --git a/app/views/exception_notifier/_session.text.erb b/app/views/exception_notifier/_session.text.erb index 3446c186bdd..082443ef447 100644 --- a/app/views/exception_notifier/_session.text.erb +++ b/app/views/exception_notifier/_session.text.erb @@ -18,6 +18,6 @@ Session: <%= session %> <% user = @kontroller.analytics_user || AnonymousUser.new %> User UUID: <%= user.uuid %> -User's Country (based on phone): <%= Phonelib.parse(user.phone).country %> +User's Country (based on phone): <%= Phonelib.parse(user.phone_configuration.phone).country %> Visitor ID: <%= @request.cookies['ahoy_visitor'] %> diff --git a/app/views/idv/otp_delivery_method/new.html.slim b/app/views/idv/otp_delivery_method/new.html.slim index ddcd37b2aab..92eac66e68c 100644 --- a/app/views/idv/otp_delivery_method/new.html.slim +++ b/app/views/idv/otp_delivery_method/new.html.slim @@ -1,8 +1,8 @@ h1.h3.my0 = t('idv.titles.otp_delivery_method') p.mt1 = t('idv.messages.otp_delivery_method.phone_number_html', - phone: @set_otp_delivery_method_presenter.phone) -= simple_form_for(@otp_delivery_selection_form, url: idv_otp_delivery_method_url, - html: { autocomplete: 'off', method: 'put', role: 'form', class: 'mt3' }) do |f| + phone: @idv_phone) += form_tag(idv_otp_delivery_method_url, method: 'put', autocomplete: 'off', + role: 'form', class: 'mt3') fieldset.mb3.p0.border-none label.btn-border.col-12.mb1 .radio @@ -13,30 +13,16 @@ p.mt1 = t('idv.messages.otp_delivery_method.phone_number_html', = t('devise.two_factor_authentication.otp_delivery_preference.sms') .regular.gray-dark.fs-10p.mb-tiny = t('devise.two_factor_authentication.two_factor_choice_options.sms_info') - - if @set_otp_delivery_method_presenter.sms_only? - label.btn-border.col-12.mb0.btn-disabled - .radio - = radio_button_tag 'otp_delivery_selection_form[otp_delivery_preference]', - :voice, false, - disabled: true, - class: :otp_delivery_preference_voice - span.indicator.mt-tiny - span.blue.bold.fs-20p - = t('devise.two_factor_authentication.otp_delivery_preference.voice') - .regular.gray-dark.fs-10p.mb-tiny - = t('devise.two_factor_authentication.two_factor_choice_options.voice_info') - p.mt2.mb0 = @set_otp_delivery_method_presenter.phone_unsupported_message - - else - label.btn-border.col-12.mb0 - .radio - = radio_button_tag 'otp_delivery_selection_form[otp_delivery_preference]', - :voice, false, - class: :otp_delivery_preference_voice - span.indicator.mt-tiny - span.blue.bold.fs-20p - = t('devise.two_factor_authentication.otp_delivery_preference.voice') - .regular.gray-dark.fs-10p.mb-tiny - = t('devise.two_factor_authentication.two_factor_choice_options.voice_info') + label.btn-border.col-12.mb0 + .radio + = radio_button_tag 'otp_delivery_selection_form[otp_delivery_preference]', + :voice, false, + class: :otp_delivery_preference_voice + span.indicator.mt-tiny + span.blue.bold.fs-20p + = t('devise.two_factor_authentication.otp_delivery_preference.voice') + .regular.gray-dark.fs-10p.mb-tiny + = t('devise.two_factor_authentication.two_factor_choice_options.voice_info') - if FeatureManagement.enable_usps_verification? .mt3 = t('idv.form.no_alternate_phone_html', @@ -45,8 +31,7 @@ p.mt1 = t('idv.messages.otp_delivery_method.phone_number_html', = t('instructions.mfa.wrong_number_html', link: link_to(t('forms.two_factor.try_again'), idv_phone_path)) .mt3 - = f.submit t('idv.buttons.send_confirmation_code'), - type: :submit, + = submit_tag t('idv.buttons.send_confirmation_code'), class: 'sm-col-6 col-12 btn btn-primary' .mt3.border-top .mt1 diff --git a/app/views/idv/phone/new.html.slim b/app/views/idv/phone/new.html.slim index fa6131991e3..2308854d491 100644 --- a/app/views/idv/phone/new.html.slim +++ b/app/views/idv/phone/new.html.slim @@ -1,6 +1,6 @@ - title t('idv.titles.phone') -h1.h2.my0 = t('idv.titles.session.phone') +h1.h3.my0 = t('idv.titles.session.phone') .mt2 == t('idv.messages.phone.alert') @@ -12,7 +12,7 @@ ul.py1.m0 = simple_form_for(@idv_form, url: idv_phone_path, html: { autocomplete: 'off', method: :put, role: 'form', class: 'mt2' }) do |f| = f.label :phone, label: t('idv.form.phone'), class: 'bold' - = f.input :phone, required: true, input_html: { class: 'us-phone' }, label: false, + = f.input :phone, required: true, input_html: { class: 'us-phone sm-col-8' }, label: false, wrapper_html: { class: 'mr2' } - if FeatureManagement.enable_usps_verification? diff --git a/app/views/idv/shared/_failure_to_proof_url.html.slim b/app/views/idv/shared/_failure_to_proof_url.html.slim new file mode 100644 index 00000000000..20691c38430 --- /dev/null +++ b/app/views/idv/shared/_failure_to_proof_url.html.slim @@ -0,0 +1,9 @@ +- if decorated_session.sp_name + hr + .mb2.mt2 + .right = link_to image_tag(asset_url('carat-right.svg'), size: '10'), + decorated_session.failure_to_proof_url, class: 'bold block btn-link text-decoration-none' + = link_to t('idv.failure.help.get_help_html', sp_name: decorated_session.sp_name), + decorated_session.failure_to_proof_url, + class: 'block btn-link text-decoration-none' + hr diff --git a/app/views/idv/shared/verification_failure.html.slim b/app/views/idv/shared/verification_failure.html.slim index 34cac002f37..72430f36e3f 100644 --- a/app/views/idv/shared/verification_failure.html.slim +++ b/app/views/idv/shared/verification_failure.html.slim @@ -1,5 +1,7 @@ = render 'shared/failure', presenter: presenter p == presenter.warning_message -.mt4 - = link_to presenter.button_text, presenter.button_path, class: 'btn btn-primary' += render 'idv/shared/failure_to_proof_url', presenter: presenter + +.mt3 + = link_to presenter.button_text, presenter.button_path, class: 'btn btn-primary btn-link' diff --git a/app/views/layouts/base.html.slim b/app/views/layouts/base.html.slim index acfe5b7cca3..c33a20e9a55 100644 --- a/app/views/layouts/base.html.slim +++ b/app/views/layouts/base.html.slim @@ -51,8 +51,10 @@ html lang="#{I18n.locale}" class='no-js' body class="#{Rails.env}-env site #{yield(:background_cls)}" .site-wrap = render 'shared/i18n_mode' if FeatureManagement.enable_i18n_mode? - = render 'shared/no_pii_banner' if FeatureManagement.no_pii_mode? - = render 'shared/usa_banner' + - if FeatureManagement.fake_banner_mode? + = render 'shared/fake_banner' + - else + = render 'shared/usa_banner' - if content_for?(:nav) = yield(:nav) - else diff --git a/app/views/shared/_failure.html.slim b/app/views/shared/_failure.html.slim index bdf4d8ac5f3..fec4ea47aa5 100644 --- a/app/views/shared/_failure.html.slim +++ b/app/views/shared/_failure.html.slim @@ -12,6 +12,7 @@ p == presenter.description - if presenter.message.present? h2.h4.mb2.mt3.my0 = presenter.message + = render 'idv/shared/failure_to_proof_url', presenter: presenter - presenter.next_steps.each do |step| p == step diff --git a/app/views/shared/_fake_banner.html.slim b/app/views/shared/_fake_banner.html.slim new file mode 100644 index 00000000000..0a95f398dc3 --- /dev/null +++ b/app/views/shared/_fake_banner.html.slim @@ -0,0 +1,6 @@ +.py1.bg-maroon.white.fs-12p.line-height-1.center + = t('idv.messages.sessions.no_pii') +.py1.bg-navy.white.fs-12p.line-height-1.center + = image_tag(asset_url('us-flag.png'), size: '18x12', + alt: 'US flag', class: 'mr1 align-bottom') + = t('.fake_site') diff --git a/app/views/shared/_spinner.html.slim b/app/views/shared/_spinner.html.slim new file mode 100644 index 00000000000..1a0ae2c6689 --- /dev/null +++ b/app/views/shared/_spinner.html.slim @@ -0,0 +1,18 @@ +.spinner.hidden + div + = image_tag(asset_url('spinner.gif'), + srcset: asset_url('spinner@2x.gif'), + height: 144, + width: 144, + alt: '') +- nonce = content_security_policy_script_nonce += nonced_javascript_tag do + | var nonce="#{ nonce }"; + | document.addEventListener('DOMContentLoaded', () => { + | const button = document.querySelector('.no-spinner'); + | const info = document.querySelector('.spinner'); + | button.addEventListener('click', () => { + | button.classList.add('hidden'); + | info.classList.remove('hidden'); + | }); + | }); diff --git a/app/views/two_factor_authentication/piv_cac_verification/show.html.slim b/app/views/two_factor_authentication/piv_cac_verification/show.html.slim index eb7143187c1..544211b0c88 100644 --- a/app/views/two_factor_authentication/piv_cac_verification/show.html.slim +++ b/app/views/two_factor_authentication/piv_cac_verification/show.html.slim @@ -1,11 +1,12 @@ - title t('titles.present_piv_cac') h1.h3.my0 = @presenter.header -p.mt-tiny.mb3 = @presenter.help_text - -= link_to @presenter.piv_cac_capture_text, - @presenter.piv_cac_service_link, - class: 'btn btn-primary' +.no-spinner + p.mt-tiny.mb3 = @presenter.help_text + = link_to @presenter.piv_cac_capture_text, + @presenter.piv_cac_service_link, + class: 'btn btn-primary activate-spinner' += render 'shared/spinner' = render 'shared/fallback_links', presenter: @presenter = render 'shared/cancel', link: @presenter.cancel_link diff --git a/app/views/users/phone_setup/index.html.slim b/app/views/users/phone_setup/index.html.slim index 8712ea5855c..6bb605abeb4 100644 --- a/app/views/users/phone_setup/index.html.slim +++ b/app/views/users/phone_setup/index.html.slim @@ -5,8 +5,7 @@ h1.h3.my0 = @presenter.heading p.mt-tiny.mb0 = @presenter.info = simple_form_for(@user_phone_form, html: { autocomplete: 'off', role: 'form' }, - data: { unsupported_area_codes: unsupported_area_codes, - international_phone_form: true }, + data: { international_phone_form: true }, method: :patch, url: phone_setup_path) do |f| .sm-col-8.js-intl-tel-code-select diff --git a/app/views/users/phones/edit.html.slim b/app/views/users/phones/edit.html.slim index bd922912786..e72bfa54792 100644 --- a/app/views/users/phones/edit.html.slim +++ b/app/views/users/phones/edit.html.slim @@ -3,8 +3,7 @@ h1.h3.my0 = t('headings.edit_info.phone') = simple_form_for(@user_phone_form, html: { autocomplete: 'off', method: :put, role: 'form' }, - data: { unsupported_area_codes: unsupported_area_codes, - international_phone_form: true }, + data: { international_phone_form: true }, url: manage_phone_path) do |f| .sm-col-8.js-intl-tel-code-select = f.input :international_code, diff --git a/app/views/users/piv_cac_authentication_setup/error.html.slim b/app/views/users/piv_cac_authentication_setup/error.html.slim index a0de89f5daa..62180f583e5 100644 --- a/app/views/users/piv_cac_authentication_setup/error.html.slim +++ b/app/views/users/piv_cac_authentication_setup/error.html.slim @@ -1,5 +1,3 @@ -- title @presenter.title - div class='alert alert-error' role='alert' = @presenter.title @@ -11,10 +9,9 @@ p.mt-tiny.mb3 = @presenter.description - cancel = sign_up_or_idv_no_js_link || link .mt2.pt1.border-top - - if @presenter.may_select_another_certificate? - = link_to t('forms.piv_cac_setup.choose_different_certificate'), - setup_piv_cac_url, class: 'h5' - br + = link_to t('forms.piv_cac_setup.choose_different_certificate'), + setup_piv_cac_url, class: 'h5' + br - if user_signing_up? || user_verifying_identity? - method = user_signing_up? ? :delete : :get @@ -25,5 +22,3 @@ p.mt-tiny.mb3 = @presenter.description user_signing_up: user_signing_up? - else = link_to cancel_link_text, cancel, class: 'h5' - -== javascript_pack_tag 'clipboard' diff --git a/app/views/users/piv_cac_authentication_setup/new.html.slim b/app/views/users/piv_cac_authentication_setup/new.html.slim index 51f1aba2252..dff8a0ead84 100644 --- a/app/views/users/piv_cac_authentication_setup/new.html.slim +++ b/app/views/users/piv_cac_authentication_setup/new.html.slim @@ -1,11 +1,12 @@ - title @presenter.title h1.h3.my0 = @presenter.heading -p.mt-tiny.mb3 = @presenter.description +.no-spinner + p.mt-tiny.mb3 = @presenter.description -= link_to @presenter.piv_cac_capture_text, - @presenter.piv_cac_service_link, - class: 'btn btn-primary' -= render 'shared/cancel_or_back_to_options' + = link_to @presenter.piv_cac_capture_text, + @presenter.piv_cac_service_link, + class: 'btn btn-primary activate-spinner' -== javascript_pack_tag 'clipboard' += render 'shared/spinner' += render 'shared/cancel_or_back_to_options' diff --git a/bin/generate-example-keys b/bin/generate-example-keys deleted file mode 100755 index 12d9cf7ab95..00000000000 --- a/bin/generate-example-keys +++ /dev/null @@ -1,46 +0,0 @@ -#!/usr/bin/env ruby - -def run(command) - abort "command failed (#{$?}): #{command}" unless system command -end - -def equifax_gpg_private_exists? - list_keys_output = `gpg --list-secret-keys` - list_keys_output.include? 'login dot gov (development only) ' -end - -def generate_equifax_gpg_private_key - if equifax_gpg_private_exists? - puts 'Equifax GPG private key exists. Skipping.' - return - end - parameters = ' - Key-Type: 1 - Subkey-Type: 1 - Name-Real: login dot gov - Name-Comment: development only - Name-Email: logs@login.gov - Expire-Date: 0 - Passphrase: sekret - # Do a commit here, so that we can later print "done" - %commit - %echo done - ' - run "echo '#{parameters}' | gpg --batch --pinentry-mode loopback --gen-key" - run 'gpg --export --output keys/equifax_gpg.pub.bin logs@login.gov' -end - -def generate_equifax_rsa_private_key - if File.exists? 'keys/equifax_rsa' - puts 'Equifax RSA private key exists. Skipping.' - return - end - run 'ssh-keygen -t rsa -b 4096 -C "logs@login.gov" -N "sekret" -f "keys/equifax_rsa"' -end - -puts "Note: This script is meant for local development use only." -puts " Under no circumstances should this be used to generate keys" -puts " for a production system." - -generate_equifax_gpg_private_key -generate_equifax_rsa_private_key diff --git a/bin/setup b/bin/setup index c659ff8f96d..a02271c8925 100755 --- a/bin/setup +++ b/bin/setup @@ -37,7 +37,6 @@ Dir.chdir APP_ROOT do if ARGV.shift == "--docker" then run 'docker-compose build' - run 'docker-compose run --rm web bin/generate-example-keys' run 'docker-compose run --rm web yarn install' run 'docker-compose run --rm web rake db:create' run 'docker-compose run --rm web rake db:environment:set' diff --git a/certs/sp/rrb_bos_prod.crt b/certs/sp/rrb_bos_prod.crt new file mode 100644 index 00000000000..32741041f15 --- /dev/null +++ b/certs/sp/rrb_bos_prod.crt @@ -0,0 +1,24 @@ +-----BEGIN CERTIFICATE----- +MIID8DCCAtigAwIBAgIJAKQ2emVpMMv9MA0GCSqGSIb3DQEBCwUAMIGMMQswCQYD +VQQGEwJVUzELMAkGA1UECAwCSUwxEDAOBgNVBAcMB0NoaWNhZ28xIjAgBgNVBAoM +GVJhaWxyb2FkIFJldGlyZW1lbnQgQm9hcmQxDDAKBgNVBAsMA0JPUzEMMAoGA1UE +AwwDQk9TMR4wHAYJKoZIhvcNAQkBFg9TdXBwb3J0QHJyYi5nb3YwHhcNMTcwNTE5 +MTgwNjUwWhcNMjAwNTE4MTgwNjUwWjCBjDELMAkGA1UEBhMCVVMxCzAJBgNVBAgM +AklMMRAwDgYDVQQHDAdDaGljYWdvMSIwIAYDVQQKDBlSYWlscm9hZCBSZXRpcmVt +ZW50IEJvYXJkMQwwCgYDVQQLDANCT1MxDDAKBgNVBAMMA0JPUzEeMBwGCSqGSIb3 +DQEJARYPU3VwcG9ydEBycmIuZ292MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB +CgKCAQEAzg23+AQySx0lBTNi3xpjC9RLYutqKi3kPyU55X9L2yBCXcScEF76mlVy +Ji5a8345nKkcgnT3zIjdatlOm4dO4TEj4qspTuwDX7jDUDmph/1R41QXuBD4jP3i +AvhyJcNg+WoUCYuILaTCmLdj2pjPx/utNOGifWcIfRGj1QlFsJdQtoPF4OctKiMB +f4ktWz+EwKAsWqoxVI3qlFSnSU6JdQZnsY7XqkknCvU3eHSPyj9Mt1OrgMfb7bMc +rzPjNT6YB6E3tEUVhslOvUj3EiScCDbmy4gq6m2cEoOttp0El9jnHudGSSiTEUqB +l1Mep59aXg5kd4aH4hBzLS1qRfOhqwIDAQABo1MwUTAdBgNVHQ4EFgQUWw7q7bbh +1VwBL8DsAIAGeHnGeoQwHwYDVR0jBBgwFoAUWw7q7bbh1VwBL8DsAIAGeHnGeoQw +DwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAPK9QAloXAeiRvv01 +HlhTQjf2YJ4F8lWJN5lkrWwm75+R0q8PEpizkldF0OvifwTRljhIh5d8UGy1Yqnp +sgGpUrQp1XQGhsM02s94DlTDGXDkQkgDhUkiFku1Ve6cCO8zlslxxFbh65nhJjtc +O1rTdiFeMVMe2aSAUXTCgZeppHdPAg3xQNABwoUcvDZVAB9zw9nUPW2C4wqzcPZV +XRmcJpMRGleUol7zms16kY8sx9cK8e+If8xwJmo2ROYWEEc7QAexWmHZChbbmZ/H +25QMoa1jPcuOpaKHYBbfYVKIG9XOd3wG0w/CbRmRuHRlBiKIbyfWW/oXb0NlkHZt +6ElMJQ== +-----END CERTIFICATE----- diff --git a/config/application.yml.example b/config/application.yml.example index 8f508c03e07..77a0f3c89d9 100644 --- a/config/application.yml.example +++ b/config/application.yml.example @@ -111,17 +111,6 @@ development: enable_rate_limiting: 'false' enable_test_routes: 'true' enable_usps_verification: 'true' - equifax_avs_username: 'sekret' - equifax_development_example_gpg_passphrase: 'sekret' - equifax_eid_username: 'sekret' - equifax_endpoint: 'sekret' - equifax_gpg_email: 'logs@login.gov' - equifax_password: 'sekret' - equifax_phone_username: 'sekret' - equifax_sftp_directory: '/directory' - equifax_sftp_host: 'example.com' - equifax_sftp_username: 'user' - equifax_ssh_passphrase: 'sekret' exception_recipients: 'test1@test.com' hmac_fingerprinter_key: 'a2c813d4dca919340866ba58063e4072adc459b767a74cf2666d5c1eef3861db26708e7437abde1755eb24f4034386b0fea1850a1cb7e56bff8fae3cc6ade96c' hmac_fingerprinter_key_queue: '["11111111111111111111111111111111", "22222222222222222222222222222222"]' @@ -150,6 +139,7 @@ development: password_pepper: 'f22d4b2cafac9066fe2f4416f5b7a32c' password_strength_enabled: 'true' piv_cac_agencies: '["Test Government Agency"]' + piv_cac_email_domains: '[".mil"]' piv_cac_enabled: 'true' piv_cac_verify_token_secret: 'ee7f20f44cdc2ba0c6830f70470d1d1d059e1279cdb58134db92b35947b1528ef5525ece5910cf4f2321ab989a618feea12ef95711dbc62b9601e8520a34ee12' piv_cac_service_url: 'https://localhost:8443/' @@ -235,17 +225,6 @@ production: enable_rate_limiting: 'true' enable_test_routes: 'false' enable_usps_verification: 'false' - equifax_avs_username: - equifax_development_example_gpg_passphrase: - equifax_eid_username: - equifax_endpoint: - equifax_gpg_email: - equifax_password: - equifax_phone_username: - equifax_sftp_directory: # '/directory' - equifax_sftp_host: # 'example.com' - equifax_sftp_username: - equifax_ssh_passphrase: exception_recipients: 'user1@example.com,user2@example.com' google_analytics_key: # 'UA-XXXXXXXXX-YY' hmac_fingerprinter_key: # generate via `rake secret` @@ -276,6 +255,8 @@ production: password_pepper: # generate via `rake secret` password_strength_enabled: 'true' piv_cac_agencies: '["DOD","NGA","EOP"]' + piv_cac_agencies_scoped_by_email: '["GSA"]' + piv_cac_email_domains: '[".mil"]' piv_cac_enabled: 'false' pkcs11_lib: '/opt/cloudhsm/lib/libcloudhsm_pkcs11.so' programmable_sms_countries: 'US,CA,MX' @@ -360,17 +341,6 @@ test: enable_rate_limiting: 'true' enable_test_routes: 'true' enable_usps_verification: 'true' - equifax_avs_username: 'sekret' - equifax_development_example_gpg_passphrase: 'sekret' - equifax_eid_username: 'sekret' - equifax_endpoint: 'sekret' - equifax_gpg_email: 'logs@login.gov' - equifax_password: 'sekret' - equifax_phone_username: 'sekret' - equifax_sftp_directory: '/directory' - equifax_sftp_host: 'example.com' - equifax_sftp_username: 'user' - equifax_ssh_passphrase: 'sekret' exception_recipients: 'test1@test.com' hmac_fingerprinter_key: 'a2c813d4dca919340866ba58063e4072adc459b767a74cf2666d5c1eef3861db26708e7437abde1755eb24f4034386b0fea1850a1cb7e56bff8fae3cc6ade96c' hmac_fingerprinter_key_queue: '["old-key-one", "old-key-two"]' @@ -397,6 +367,7 @@ test: password_pepper: 'f22d4b2cafac9066fe2f4416f5b7a32c' password_strength_enabled: 'false' piv_cac_agencies: '["Test Government Agency"]' + piv_cac_email_domains: '[".mil"]' piv_cac_enabled: 'true' piv_cac_service_url: 'https://localhost:8443/' piv_cac_verify_token_secret: '3ac13bfa23e22adae321194c083e783faf89469f6f85dcc0802b27475c94b5c3891b5657bd87d0c1ad65de459166440512f2311018db90d57b15d8ab6660748f' diff --git a/config/environments/test.rb b/config/environments/test.rb index 2e3f157a11b..9d3896121e2 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -34,6 +34,9 @@ Bullet.enable = true Bullet.bullet_logger = true Bullet.raise = true + Bullet.add_whitelist( + type: :n_plus_one_query, class_name: 'User', association: :phone_configuration + ) end config.active_support.test_order = :random diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml index 02a59f0e42d..0f56c8f2fa3 100644 --- a/config/i18n-tasks.yml +++ b/config/i18n-tasks.yml @@ -103,6 +103,7 @@ ignore_unused: - 'devise.two_factor_authentication.max_otp_login_attempts_reached' - 'devise.two_factor_authentication.max_otp_requests_reached' - 'devise.two_factor_authentication.max_personal_key_login_attempts_reached' + - 'devise.two_factor_authentication.max_piv_cac_login_attempts_reached' - 'devise.two_factor_authentication.phone_sms_info_html' - 'devise.two_factor_authentication.phone_sms_label' - 'devise.two_factor_authentication.phone_voice_info_html' diff --git a/config/initializers/figaro.rb b/config/initializers/figaro.rb index fef45c0cbf7..268bd73444f 100644 --- a/config/initializers/figaro.rb +++ b/config/initializers/figaro.rb @@ -10,7 +10,6 @@ 'enable_rate_limiting', 'enable_test_routes', 'enable_usps_verification', - 'equifax_ssh_passphrase', 'exception_recipients', 'hmac_fingerprinter_key', 'issuers_with_email_nameid_format', diff --git a/config/initializers/idv_proofer.rb b/config/initializers/idv_proofer.rb new file mode 100644 index 00000000000..25a1ecd1dac --- /dev/null +++ b/config/initializers/idv_proofer.rb @@ -0,0 +1,2 @@ +Dir[Rails.root.join('lib', 'proofer_mocks', '*')].each { |file| require file } +Idv::Proofer.validate_vendors! diff --git a/config/initializers/proofer.rb b/config/initializers/proofer.rb deleted file mode 100644 index 324ba43f535..00000000000 --- a/config/initializers/proofer.rb +++ /dev/null @@ -1,18 +0,0 @@ -# rubocop:disable Metrics/LineLength -if FeatureManagement.enable_identity_verification? - Idv::Proofer.configure do |config| - config.mock_fallback = Figaro.env.proofer_mock_fallback == 'true' - config.raise_on_missing_proofers = false if Figaro.env.proofer_raise_on_missing_proofers == 'false' - config.vendors = JSON.parse(Figaro.env.proofer_vendors || '[]') - end - - Idv::Proofer.init - - # Until equifax is removed, ensure env variables are available - [/^equifax_/].each do |pattern| - ENV.keys.grep(pattern).each do |env_var_name| - ENV[env_var_name.upcase] = ENV[env_var_name] - end - end -end -# rubocop:enable Metrics/LineLength diff --git a/config/locales/account/en.yml b/config/locales/account/en.yml index 813ff2cce61..96ddbee1cbb 100644 --- a/config/locales/account/en.yml +++ b/config/locales/account/en.yml @@ -25,7 +25,6 @@ en: instructions: Your account requires a secret code to be verified. reactivate_button: Enter the code you received via US mail success: Your account has been verified. - with_phone_button: Verify with your phone items: delete_your_account: Delete your account personal_key: Personal key diff --git a/config/locales/account/es.yml b/config/locales/account/es.yml index 430d6eef899..239124a01dc 100644 --- a/config/locales/account/es.yml +++ b/config/locales/account/es.yml @@ -25,7 +25,6 @@ es: instructions: Su cuenta requiere que un código secreto sea verificado. reactivate_button: Ingrese el código que recibió por correo postal. success: Su cuenta ha sido verificada. - with_phone_button: Verifique con su teléfono. items: delete_your_account: Eliminar su cuenta personal_key: Clave personal diff --git a/config/locales/account/fr.yml b/config/locales/account/fr.yml index ba5b9de01bd..5df241818f7 100644 --- a/config/locales/account/fr.yml +++ b/config/locales/account/fr.yml @@ -27,7 +27,6 @@ fr: instructions: Votre compte requiert la vérification d'un code secret. reactivate_button: Entrez le code que vous avez reçu par la poste success: Votre compte a été vérifié. - with_phone_button: Verifiez avec votre téléphone items: delete_your_account: Supprimer votre compte personal_key: Clé personnelle diff --git a/config/locales/account_reset/en.yml b/config/locales/account_reset/en.yml index 97286aff899..c0af8a61f3c 100644 --- a/config/locales/account_reset/en.yml +++ b/config/locales/account_reset/en.yml @@ -9,10 +9,11 @@ en: title: You have deleted your account confirm_request: check_your_email: Check your email + close_window: You can close this window if you're done. instructions: We sent an email to %{email} to begin the account delete process. Follow the instructions in your email to complete the process. -

As a security measure, we also sent a text to your registered phone - number.

You can close this window if you are done. + security_note: As a security measure, we also sent a text to your registered + phone number. delete_account: are_you_sure: Are you sure you want to delete your account? cancel: Cancel diff --git a/config/locales/account_reset/es.yml b/config/locales/account_reset/es.yml index 0ec383a9f6f..e4344e0de0c 100644 --- a/config/locales/account_reset/es.yml +++ b/config/locales/account_reset/es.yml @@ -9,11 +9,12 @@ es: title: Has eliminado tu cuenta confirm_request: check_your_email: Consultar su correo electrónico + close_window: Puede cerrar esta ventana si ha terminado. instructions: Enviamos un correo electrónico a %{email} para comenzar el proceso de eliminación de cuenta. Siga las instrucciones en su - correo electrónico para completar el proceso.

Como medida de seguridad, - también enviamos un mensaje de texto a su registro número de teléfono.

- Puede cerrar esta ventana si ha terminado. + correo electrónico para completar el proceso. + security_note: Como medida de seguridad, también enviamos un mensaje de texto + a su registro número de teléfono. delete_account: are_you_sure: "¿Seguro que quieres eliminar tu cuenta?" cancel: Cancelar diff --git a/config/locales/account_reset/fr.yml b/config/locales/account_reset/fr.yml index f151c7c61e4..089b9e588df 100644 --- a/config/locales/account_reset/fr.yml +++ b/config/locales/account_reset/fr.yml @@ -9,11 +9,12 @@ fr: title: Vous avez supprimé votre compte confirm_request: check_your_email: Vérifiez votre email + close_window: Vous pouvez fermer cette fenêtre si vous avez terminé. instructions: Nous avons envoyé un e-mail à %{email} pour commencer le compte. Supprimer le processus. Suivez les instructions dans votre e-mail - pour terminer le processus.

Par mesure de sécurité, nous avons également - envoyé un SMS sur votre téléphone enregistré nombre.

Vous pouvez - fermer cette fenêtre si vous avez terminé. + pour terminer le processus. + security_note: Par mesure de sécurité, nous avons également envoyé un SMS à + votre numéro de téléphone enregistré. delete_account: are_you_sure: Êtes-vous sûr de vouloir supprimer votre compte? cancel: Annuler diff --git a/config/locales/devise/en.yml b/config/locales/devise/en.yml index bea36b559c3..2f25bd393b2 100644 --- a/config/locales/devise/en.yml +++ b/config/locales/devise/en.yml @@ -97,6 +97,9 @@ en: max_personal_key_login_attempts_reached: For your security, your account is temporarily locked because you have entered the personal key incorrectly too many times. + max_piv_cac_login_attempts_reached: For your security, your account is temporarily + locked because you have presented your piv/cac credential incorrectly too + many times. otp_delivery_preference: instruction: You can change this selection the next time you log in. If you entered a landline, please select "Phone call" below. diff --git a/config/locales/devise/es.yml b/config/locales/devise/es.yml index a6e6b540d1e..ab6d32c6982 100644 --- a/config/locales/devise/es.yml +++ b/config/locales/devise/es.yml @@ -99,6 +99,7 @@ es: max_personal_key_login_attempts_reached: Para su seguridad, su cuenta ha sido bloqueada temporalmente porque ha ingresado incorrectamente la clave personal demasiadas veces. + max_piv_cac_login_attempts_reached: NOT TRANSLATED YET otp_delivery_preference: instruction: Puede cambiar esta selección la próxima vez que inicie sesión. phone_unsupported: NOT TRANSLATED YET diff --git a/config/locales/devise/fr.yml b/config/locales/devise/fr.yml index 5c9e2ffea5f..07d87cb707b 100644 --- a/config/locales/devise/fr.yml +++ b/config/locales/devise/fr.yml @@ -106,6 +106,7 @@ fr: max_personal_key_login_attempts_reached: Pour votre sécurité, votre compte est temporairement verrouillé, car vous avez entré le code de sécurité à utilisation unique de façon erronée à de trop nombreuses reprises. + max_piv_cac_login_attempts_reached: NOT TRANSLATED YET otp_delivery_preference: instruction: Vous pouvez changer cette sélection la prochaine fois que vous vous connectez. diff --git a/config/locales/idv/en.yml b/config/locales/idv/en.yml index 3fb4d7a0aff..688c82967ae 100644 --- a/config/locales/idv/en.yml +++ b/config/locales/idv/en.yml @@ -44,6 +44,8 @@ en: errors: link: please contact us text: If you keep getting these errors, %{link}, or try again tomorrow. + help: + get_help_html: Get help at %{sp_name} phone: fail: For your security, identity verification for your account is locked for 24 hours. @@ -108,14 +110,11 @@ en: dupe_ssn2_link: sign out now and sign back in hardfail: We can't log you in right now, but you can try verifying your identity again in %{hours} hours. - help_center_html: Visit our Help Center to learn more about - verifying your account. jurisdiction: no_id: I don't have a state-issued ID no_id_failure: We're working hard to add more ways to verify your identity. profile: To access your account in the future, you can %{link}. profile_link: view your account here - sp_support: Visit %{link} for more information. try_again: Make a mistake? You can %{link}. try_again_link: try again unsupported_jurisdiction_failure: We're working hard to add more states and @@ -135,11 +134,7 @@ en: rules: - in your name, or a family member's name - not a virtual phone (such as Google Voice or Skype) - - not a pre-paid phone number - a U.S. number - read_about_security_and_privacy: - link: read about how login.gov keeps your information safe - text: You can %{link} on our help page. return_to_profile: "‹ Return to your login.gov profile" return_to_sp_html: You can now log into %{sp}. review: @@ -152,7 +147,7 @@ en: sessions: id_information_message: as it appears on your state-issued ID id_information_subtitle: ID Information - no_pii: Do not use real personal information (demo purposes only) + no_pii: FAKE Do not use real personal information (demo purposes only) FAKE review_message: When you enter your password, login.gov will secure your personal information. We do this to make sure no one else can access your information. success: Next, we'll need a phone number. diff --git a/config/locales/idv/es.yml b/config/locales/idv/es.yml index f502fae2d03..2647bec0619 100644 --- a/config/locales/idv/es.yml +++ b/config/locales/idv/es.yml @@ -44,6 +44,8 @@ es: errors: link: contáctanos text: Si sigues recibiendo estos errores, %{link} o vuelve a intentarlo mañana. + help: + get_help_html: Obtener ayuda en %{sp_name} phone: fail: Para su seguridad, la verificación de identidad de su cuenta está bloqueada durante 24 horas. @@ -105,15 +107,12 @@ es: dupe_ssn2_html: Por favor %{link} con el email que utilizó originalmente. dupe_ssn2_link: Cerrar ahora y volver a iniciar sesión hardfail: NOT TRANSLATED YET - help_center_html: Visite nuestro Centro de Ayuda para obtener - más información sobre la verificación de su cuenta. jurisdiction: no_id: No tengo una identificación emitida por el estado no_id_failure: Estamos trabajando arduamente para agregar más formas de verificar su identidad. profile: Para acceder a su cuenta en el futuro, puede %{link}. profile_link: mira tu cuenta aquí - sp_support: Visita %{link} para obtener más información. try_again: "¿Cometer un error? Puedes %{link}." try_again_link: intentarlo de nuevo unsupported_jurisdiction_failure: Estamos trabajando duro para agregar más @@ -136,9 +135,6 @@ es: - no es un teléfono virtual (como Google Voice o Skype) - no es un número de teléfono prepago - un número de EE. UU. - read_about_security_and_privacy: - link: leer sobre cómo login.gov mantiene su información segura - text: Puede %{link} en nuestra página de ayuda. return_to_profile: NOT TRANSLATED YET return_to_sp_html: NOT TRANSLATED YET review: @@ -152,7 +148,8 @@ es: sessions: id_information_message: como aparece en su identificación emitida por el estado id_information_subtitle: Información de identificación - no_pii: No utilice información personal real (sólo para propósitos de demostración) + no_pii: FALSO No utilice información personal real (sólo para propósitos de + demostración) FALSO review_message: Cuando ingrese su contraseña, login.gov protegerá su información personal. Hacemos esto para asegurarnos de que nadie más pueda acceder a su información. diff --git a/config/locales/idv/fr.yml b/config/locales/idv/fr.yml index aaca0125152..3e3aa266df2 100644 --- a/config/locales/idv/fr.yml +++ b/config/locales/idv/fr.yml @@ -48,6 +48,8 @@ fr: errors: link: contactez-nous text: Si vous continuez à recevoir ces erreurs, %{link} ou réessayez demain. + help: + get_help_html: Obtenir de l'aide à %{sp_name} phone: fail: Pour votre sécurité, la vérification d'identité de votre compte est verrouillée pendant 24 heures. @@ -111,15 +113,12 @@ fr: originalement. dupe_ssn2_link: déconnectez-vous puis connectez-vous à nouveau hardfail: NOT TRANSLATED YET - help_center_html: Visitez notre Centre d'aide pour en apprendre - davantage sur la façon dont nous vérifions votre compte. jurisdiction: no_id: Je n'ai pas de carte d'identité officielle no_id_failure: Nous travaillons dur pour ajouter plus de moyens de vérifier votre identité. profile: Pour accéder à votre compte dans le futur, vous pouvez %{link}. profile_link: voir votre compte ici - sp_support: Visitez %{link} pour plus d'informations. try_again: Faire une erreur? Vous pouvez %{link}. try_again_link: réessayer unsupported_jurisdiction_failure: Nous travaillons dur pour ajouter plus d'états @@ -143,9 +142,6 @@ fr: - pas un téléphone virtuel (comme Google Voice ou Skype) - pas un numéro de téléphone prépayé - un numéro américain - read_about_security_and_privacy: - link: lire comment login.gov protège vos informations - text: Vous pouvez %{link} sur notre page d'aide. return_to_profile: NOT TRANSLATED YET return_to_sp_html: NOT TRANSLATED YET review: @@ -161,8 +157,8 @@ fr: sessions: id_information_message: tel qu'il apparaît sur votre carte d'identité officielle id_information_subtitle: Informations d'identification - no_pii: N'utilisez pas de véritables données personnelles (il s'agit d'une - démonstration seulement) + no_pii: TRUQUÉ N'utilisez pas de véritables données personnelles (il s'agit + d'une démonstration seulement) TRUQUÉ review_message: Lorsque vous entrez votre mot de passe, login.gov sécurise vos informations personnelles. Nous faisons cela pour nous assurer que personne d'autre ne puisse accéder à vos informations. diff --git a/config/locales/shared/en.yml b/config/locales/shared/en.yml index 5760ad608b8..2ebc490e558 100644 --- a/config/locales/shared/en.yml +++ b/config/locales/shared/en.yml @@ -1,6 +1,8 @@ --- en: shared: + fake_banner: + fake_site: A FAKE website of the United States government footer_lite: gsa: U.S. General Services Administration usa_banner: diff --git a/config/locales/shared/es.yml b/config/locales/shared/es.yml index dbbb99d678f..92851b1c293 100644 --- a/config/locales/shared/es.yml +++ b/config/locales/shared/es.yml @@ -1,6 +1,8 @@ --- es: shared: + fake_banner: + fake_site: Un FALSO sitio web del gobierno de Estados Unidos footer_lite: gsa: Administración General de Servicios de EE. UU. usa_banner: diff --git a/config/locales/shared/fr.yml b/config/locales/shared/fr.yml index d54177fd046..9b1a71fea4f 100644 --- a/config/locales/shared/fr.yml +++ b/config/locales/shared/fr.yml @@ -1,6 +1,8 @@ --- fr: shared: + fake_banner: + fake_site: Un site TRUQUÉ du gouvernement des États-Unis footer_lite: gsa: Administration des services généraux des États-Unis usa_banner: diff --git a/config/routes.rb b/config/routes.rb index 0317f73f01d..d4e78b4bad5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -115,8 +115,6 @@ as: :verify_personal_key post '/account/reactivate/verify_personal_key' => 'users/verify_personal_key#create', as: :create_verify_personal_key - get '/account/verify_phone' => 'users/verify_profile_phone#index', as: :verify_profile_phone - post '/account/verify_phone' => 'users/verify_profile_phone#create' get '/account_recovery_setup' => 'account_recovery_setup#index' if FeatureManagement.piv_cac_enabled? diff --git a/config/service_providers.yml b/config/service_providers.yml index d9d1a4da731..915a326fa9b 100644 --- a/config/service_providers.yml +++ b/config/service_providers.yml @@ -153,6 +153,7 @@ development: agency_id: 1 redirect_uris: - 'http://localhost:9292/auth/result' + - 'http://localhost:9292/' cert: 'sp_sinatra_demo' friendly_name: 'Example Sinatra App' @@ -328,7 +329,7 @@ production: # RRB Online Retirement Application 'urn:gov:gsa:SAML:2.0.profiles:sp:sso:RRB:BOS-Pre-Prod': agency_id: 4 - friendly_name: 'Railroad Retirement Board' + friendly_name: 'Railroad Retirement Board Preprod' agency: 'RRB' logo: 'rrb.svg' acs_url: 'https://onlinetest.rrb.gov/AA1/login/login/callback' @@ -396,6 +397,7 @@ production: logo: 'cbp-ttp.png' redirect_uris: - 'https://ttp.cbp.dhs.gov' + - 'https://ttp.cbp.dhs.gov/login' return_to_sp_url: https://ttp.cbp.dhs.gov/ # CBP ROAM (formerly OARS) @@ -407,6 +409,7 @@ production: logo: 'cbp.png' redirect_uris: - 'gov.dhs.cbp.pspd.oars.user.prod://result' + - 'gov.dhs.cbp.pspd.oars.user.prod://result/logout' restrict_to_deploy_env: 'prod' # CBP I'm Ready @@ -633,7 +636,7 @@ production: attribute_bundle: - x509_subject - x509_presented - + # My Move.mil 'urn:gov:gsa:openidconnect.profiles:sp:sso:dod:mymovemilprod': agency_id: 8 @@ -749,3 +752,30 @@ production: attribute_bundle: - email restrict_to_deploy_env: 'prod' + + # RRB Benefit Online Services (BOS) + 'urn:gov:gsa:openidconnect.profiles:sp:sso:RRB:BOS_AA1_Prod': + agency_id: 4 + friendly_name: 'Railroad Retirement Board' + agency: 'RRB' + logo: 'rrb.svg' + failure_to_proof_url: 'https://online.rrb.gov/AA1/login/login/FailureProof' + return_to_sp_url: 'https://online.rrb.gov/AA1' + redirect_uris: + - 'https://online.rrb.gov/AA1/login/login/RRBHome' + - 'https://online.rrb.gov/AA1/login/login/SignInCallback' + cert: 'rrb_bos_prod' + attribute_bundle: + - email + - first_name + - middle_name + - last_name + - address1 + - address2 + - city + - state + - zipcode + - dob + - ssn + - phone + restrict_to_deploy_env: 'prod' diff --git a/lib/feature_management.rb b/lib/feature_management.rb index b0df4bc0544..022665287d8 100644 --- a/lib/feature_management.rb +++ b/lib/feature_management.rb @@ -81,8 +81,8 @@ def self.current_env_allowed_to_see_usps_code? ENVS_WHERE_PREFILLING_USPS_CODE_ALLOWED.include?(Figaro.env.domain_name) end - def self.no_pii_mode? - enable_identity_verification? && Figaro.env.profile_proofing_vendor == :mock + def self.fake_banner_mode? + Rails.env.production? && Figaro.env.domain_name != 'secure.login.gov' end def self.enable_saml_cert_rotation? diff --git a/package.json b/package.json index 3f4e0c5d137..e3dee44ddd2 100644 --- a/package.json +++ b/package.json @@ -12,6 +12,7 @@ }, "dependencies": { "@rails/webpacker": "^3.5.5", + "base32-crockford-browser": "^1.0.0", "basscss-sass": "^3.0.0", "classlist.js": "^1.1.20150312", "clipboard": "^1.6.1", diff --git a/spec/controllers/account_reset/confirm_request_controller_spec.rb b/spec/controllers/account_reset/confirm_request_controller_spec.rb index fc582f00a8e..4db995aa123 100644 --- a/spec/controllers/account_reset/confirm_request_controller_spec.rb +++ b/spec/controllers/account_reset/confirm_request_controller_spec.rb @@ -2,17 +2,7 @@ RSpec.describe AccountReset::ConfirmRequestController do describe '#show' do - context 'email in session' do - it 'renders the page and deletes the email from the session' do - allow(controller).to receive(:flash).and_return(email: 'test@example.com') - - get :show - - expect(response).to render_template(:show) - end - end - - context 'no email in session' do + context 'no email in flash' do it 'redirects to the new user registration path' do get :show diff --git a/spec/controllers/account_reset/delete_account_controller_spec.rb b/spec/controllers/account_reset/delete_account_controller_spec.rb index 14b4f241440..d219b7ad611 100644 --- a/spec/controllers/account_reset/delete_account_controller_spec.rb +++ b/spec/controllers/account_reset/delete_account_controller_spec.rb @@ -1,10 +1,12 @@ require 'rails_helper' describe AccountReset::DeleteAccountController do + include AccountResetHelper + describe '#delete' do it 'logs a good token to the analytics' do user = create(:user) - AccountResetService.new(user).create_request + create_account_reset_request_for(user) AccountResetService.new(user).grant_request session[:granted_token] = AccountResetRequest.all[0].granted_token @@ -33,7 +35,7 @@ describe '#show' do it 'prevents parameter leak' do user = create(:user) - AccountResetService.new(user).create_request + create_account_reset_request_for(user) AccountResetService.new(user).grant_request get :show, params: { token: AccountResetRequest.all[0].granted_token } @@ -49,7 +51,7 @@ it 'renders the page' do user = create(:user) - AccountResetService.new(user).create_request + create_account_reset_request_for(user) AccountResetService.new(user).grant_request session[:granted_token] = AccountResetRequest.all[0].granted_token @@ -60,7 +62,7 @@ it 'displays a flash and redirects to root if the token is expired' do user = create(:user) - AccountResetService.new(user).create_request + create_account_reset_request_for(user) AccountResetService.new(user).grant_request stub_analytics diff --git a/spec/controllers/account_reset/report_fraud_controller_spec.rb b/spec/controllers/account_reset/report_fraud_controller_spec.rb index 383378f1fae..56114cf0104 100644 --- a/spec/controllers/account_reset/report_fraud_controller_spec.rb +++ b/spec/controllers/account_reset/report_fraud_controller_spec.rb @@ -1,10 +1,12 @@ require 'rails_helper' describe AccountReset::ReportFraudController do + include AccountResetHelper + describe '#update' do it 'logs a good token to the analytics' do user = create(:user) - AccountResetService.new(user).create_request + create_account_reset_request_for(user) stub_analytics expect(@analytics).to receive(:track_event). diff --git a/spec/controllers/account_reset/request_controller_spec.rb b/spec/controllers/account_reset/request_controller_spec.rb index 9fb3678f3a7..9c85c8bd757 100644 --- a/spec/controllers/account_reset/request_controller_spec.rb +++ b/spec/controllers/account_reset/request_controller_spec.rb @@ -3,52 +3,122 @@ describe AccountReset::RequestController do describe '#show' do it 'renders the page' do - sign_in_before_2fa + user = build(:user, :with_authentication_app) + stub_sign_in_before_2fa(user) get :show expect(response).to render_template(:show) end - it 'redirects to root without 2fa' do + it 'redirects to root if user not signed in' do get :show expect(response).to redirect_to root_url end - it 'redirects to phone setup url if 2fa not setup' do - user = create(:user) - sign_in_before_2fa(user) + it 'redirects to root if feature is not enabled' do + allow(FeatureManagement).to receive(:account_reset_enabled?).and_return(false) + user = build(:user, :with_authentication_app) + stub_sign_in_before_2fa(user) + + get :show + + expect(response).to redirect_to root_url + end + + it 'redirects to 2FA setup url if 2FA not set up' do + stub_sign_in_before_2fa get :show - expect(response).to redirect_to phone_setup_url + expect(response).to redirect_to two_factor_options_url + end + + it 'logs the visit to analytics' do + user = build(:user, :with_authentication_app) + stub_sign_in_before_2fa(user) + stub_analytics + + expect(@analytics).to receive(:track_event).with(Analytics::ACCOUNT_RESET_VISIT) + + get :show end end describe '#create' do - it 'logs the request in the analytics' do + it 'logs totp user in the analytics' do + user = build(:user, :with_authentication_app) + stub_sign_in_before_2fa(user) + + stub_analytics + attributes = { + event: 'request', + sms_phone: false, + totp: true, + piv_cac: false, + } + expect(@analytics).to receive(:track_event). + with(Analytics::ACCOUNT_RESET, attributes) + + post :create + end + + it 'logs sms user in the analytics' do TwilioService::Utils.telephony_service = FakeSms - sign_in_before_2fa + user = build(:user, :signed_up) + stub_sign_in_before_2fa(user) + + stub_analytics + attributes = { + event: 'request', + sms_phone: true, + totp: false, + piv_cac: false, + } + expect(@analytics).to receive(:track_event). + with(Analytics::ACCOUNT_RESET, attributes) + + post :create + end + + it 'logs PIV/CAC user in the analytics' do + user = build(:user, :with_piv_or_cac) + stub_sign_in_before_2fa(user) stub_analytics + attributes = { + event: 'request', + sms_phone: false, + totp: false, + piv_cac: true, + } expect(@analytics).to receive(:track_event). - with(Analytics::ACCOUNT_RESET, event: :request) + with(Analytics::ACCOUNT_RESET, attributes) + + post :create + end + it 'redirects to root if user not signed in' do post :create + + expect(response).to redirect_to root_url end - it 'redirects to root without 2fa' do + it 'redirects to root if feature is not enabled' do + allow(FeatureManagement).to receive(:account_reset_enabled?).and_return(false) + user = build(:user, :with_authentication_app) + stub_sign_in_before_2fa(user) + post :create expect(response).to redirect_to root_url end - it 'redirects to phone setup url if 2fa not setup' do - user = create(:user) - sign_in_before_2fa(user) + it 'redirects to 2FA setup url if 2FA not set up' do + stub_sign_in_before_2fa post :create - expect(response).to redirect_to phone_setup_url + expect(response).to redirect_to two_factor_options_url end end end diff --git a/spec/controllers/idv/cancellations_controller_spec.rb b/spec/controllers/idv/cancellations_controller_spec.rb index bce4eb261f2..63b46b4693e 100644 --- a/spec/controllers/idv/cancellations_controller_spec.rb +++ b/spec/controllers/idv/cancellations_controller_spec.rb @@ -2,11 +2,23 @@ describe Idv::CancellationsController do describe '#new' do - it 'tracks an analytics event' do + it 'tracks the event in analytics when referer is nil' do + stub_sign_in + stub_analytics + properties = { request_came_from: 'no referer' } + + expect(@analytics).to receive(:track_event).with(Analytics::IDV_CANCELLATION, properties) + + get :new + end + + it 'tracks the event in analytics when referer is present' do stub_sign_in stub_analytics + request.env['HTTP_REFERER'] = 'http://example.com/' + properties = { request_came_from: 'users/sessions#new' } - expect(@analytics).to receive(:track_event).with(Analytics::IDV_CANCELLATION) + expect(@analytics).to receive(:track_event).with(Analytics::IDV_CANCELLATION, properties) get :new end diff --git a/spec/controllers/idv/come_back_later_controller_spec.rb b/spec/controllers/idv/come_back_later_controller_spec.rb index c689378560d..c41961ec024 100644 --- a/spec/controllers/idv/come_back_later_controller_spec.rb +++ b/spec/controllers/idv/come_back_later_controller_spec.rb @@ -2,18 +2,22 @@ describe Idv::ComeBackLaterController do let(:user) { build_stubbed(:user, :signed_up) } - let(:needs_profile_usps_verification) { true } + let(:pending_profile_requires_verification) { true } before do user_decorator = instance_double(UserDecorator) - allow(user_decorator).to receive(:needs_profile_usps_verification?). - and_return(needs_profile_usps_verification) + allow(user_decorator).to receive(:pending_profile_requires_verification?). + and_return(pending_profile_requires_verification) allow(user).to receive(:decorate).and_return(user_decorator) allow(subject).to receive(:current_user).and_return(user) end context 'user needs USPS address verification' do it 'renders the show template' do + stub_analytics + + expect(@analytics).to receive(:track_event).with(Analytics::IDV_COME_BACK_LATER_VISIT) + get :show expect(response).to render_template :show @@ -21,7 +25,7 @@ end context 'user does not need USPS address verification' do - let(:needs_profile_usps_verification) { false } + let(:pending_profile_requires_verification) { false } it 'redirects to the account path' do get :show diff --git a/spec/controllers/idv/confirmations_controller_spec.rb b/spec/controllers/idv/confirmations_controller_spec.rb index adb891e083c..c7afa4e695c 100644 --- a/spec/controllers/idv/confirmations_controller_spec.rb +++ b/spec/controllers/idv/confirmations_controller_spec.rb @@ -15,7 +15,6 @@ def stub_idv_session profile_maker = Idv::ProfileMaker.new( applicant: applicant, user: user, - phone_confirmed: true, user_password: password ) profile = profile_maker.save_profile @@ -112,7 +111,7 @@ def index context 'user used 2FA phone as phone of record' do before do - subject.idv_session.params['phone'] = user.phone + subject.idv_session.params['phone'] = user.phone_configuration.phone end it 'tracks final IdV event' do diff --git a/spec/controllers/idv/otp_delivery_method_controller_spec.rb b/spec/controllers/idv/otp_delivery_method_controller_spec.rb index 234c7d4218e..a5f3a3671c5 100644 --- a/spec/controllers/idv/otp_delivery_method_controller_spec.rb +++ b/spec/controllers/idv/otp_delivery_method_controller_spec.rb @@ -39,9 +39,9 @@ subject.idv_session.vendor_phone_confirmation = false end - it 'redirects to the review controller' do + it 'redirects to the phone controller' do get :new - expect(response).to redirect_to idv_review_path + expect(response).to redirect_to idv_phone_path end end @@ -51,6 +51,16 @@ expect(response).to render_template :new end end + + it 'tracks an analytics event' do + stub_analytics + allow(@analytics).to receive(:track_event) + + get :new + + expect(@analytics).to have_received(:track_event). + with(Analytics::IDV_PHONE_OTP_DELIVERY_SELECTION_VISIT) + end end describe '#create' do @@ -89,9 +99,9 @@ subject.idv_session.vendor_phone_confirmation = false end - it 'redirects to the review controller' do + it 'redirects to the phone controller' do post :create, params: params - expect(response).to redirect_to idv_review_path + expect(response).to redirect_to idv_phone_path end end @@ -100,6 +110,22 @@ post :create, params: params expect(response).to redirect_to otp_send_path(params) end + + it 'tracks an analytics event' do + stub_analytics + allow(@analytics).to receive(:track_event) + + post :create, params: params + + result = { + success: true, + errors: {}, + otp_delivery_preference: 'sms', + } + + expect(@analytics).to have_received(:track_event). + with(Analytics::IDV_PHONE_OTP_DELIVERY_SELECTION_SUBMITTED, result) + end end context 'user has selected voice' do @@ -115,6 +141,22 @@ post :create, params: params expect(response).to redirect_to otp_send_path(params) end + + it 'tracks an analytics event' do + stub_analytics + allow(@analytics).to receive(:track_event) + + post :create, params: params + + result = { + success: true, + errors: {}, + otp_delivery_preference: 'voice', + } + + expect(@analytics).to have_received(:track_event). + with(Analytics::IDV_PHONE_OTP_DELIVERY_SELECTION_SUBMITTED, result) + end end context 'form is invalid' do @@ -130,6 +172,22 @@ post :create, params: params expect(response).to render_template :new end + + it 'tracks an analytics event' do + stub_analytics + allow(@analytics).to receive(:track_event) + + post :create, params: params + + result = { + success: false, + errors: { otp_delivery_preference: ['is not included in the list'] }, + otp_delivery_preference: '🎷', + } + + expect(@analytics).to have_received(:track_event). + with(Analytics::IDV_PHONE_OTP_DELIVERY_SELECTION_SUBMITTED, result) + end end end end diff --git a/spec/controllers/idv/phone_controller_spec.rb b/spec/controllers/idv/phone_controller_spec.rb index 0484ad4fd50..c7ded6be30b 100644 --- a/spec/controllers/idv/phone_controller_spec.rb +++ b/spec/controllers/idv/phone_controller_spec.rb @@ -71,7 +71,7 @@ end it 'renders #new' do - put :create, params: { idv_phone_form: { phone: '703', international_code: 'US' } } + put :create, params: { idv_phone_form: { phone: '703' } } expect(flash[:warning]).to be_nil expect(subject.idv_session.params).to be_empty @@ -87,6 +87,8 @@ errors: { phone: [t('errors.messages.must_have_us_country_code')], }, + country_code: nil, + area_code: nil, } expect(@analytics).to have_received(:track_event).with( @@ -106,9 +108,14 @@ user = build(:user, phone: good_phone, phone_confirmed_at: Time.zone.now) stub_verify_steps_one_and_two(user) - put :create, params: { idv_phone_form: { phone: good_phone, international_code: 'US' } } + put :create, params: { idv_phone_form: { phone: good_phone } } - result = { success: true, errors: {} } + result = { + success: true, + errors: {}, + area_code: '703', + country_code: 'US', + } expect(@analytics).to have_received(:track_event).with( Analytics::IDV_PHONE_CONFIRMATION_FORM, result @@ -120,13 +127,13 @@ user = build(:user, phone: good_phone, phone_confirmed_at: Time.zone.now) stub_verify_steps_one_and_two(user) - put :create, params: { idv_phone_form: { phone: good_phone, international_code: 'US' } } + put :create, params: { idv_phone_form: { phone: good_phone } } expect(response).to redirect_to idv_phone_result_path expected_params = { phone: normalized_phone, - phone_confirmed_at: user.phone_confirmed_at, + phone_confirmed_at: user.phone_configuration.confirmed_at, } expect(subject.idv_session.params).to eq expected_params end @@ -137,7 +144,7 @@ user = build(:user, phone: '+1 (415) 555-0130', phone_confirmed_at: Time.zone.now) stub_verify_steps_one_and_two(user) - put :create, params: { idv_phone_form: { phone: good_phone, international_code: 'US' } } + put :create, params: { idv_phone_form: { phone: good_phone } } expect(response).to redirect_to idv_phone_result_path diff --git a/spec/controllers/idv/review_controller_spec.rb b/spec/controllers/idv/review_controller_spec.rb index f0a6f8c9e0b..bfa4d713cc9 100644 --- a/spec/controllers/idv/review_controller_spec.rb +++ b/spec/controllers/idv/review_controller_spec.rb @@ -20,7 +20,7 @@ city: 'Somewhere', state: 'KS', zipcode: zipcode, - phone: user.phone, + phone: user.phone_configuration&.phone, ssn: '12345678', } end diff --git a/spec/controllers/idv/usps_controller_spec.rb b/spec/controllers/idv/usps_controller_spec.rb index 0214d4be532..69e6b954bd2 100644 --- a/spec/controllers/idv/usps_controller_spec.rb +++ b/spec/controllers/idv/usps_controller_spec.rb @@ -60,12 +60,12 @@ context 'resending a letter' do let(:has_pending_profile) { true } - let(:pending_profile) { create(:profile, phone_confirmed: false) } + let(:pending_profile) { create(:profile) } before do stub_sign_in(user) stub_decorated_user_with_pending_profile(user) - allow(user.decorate).to receive(:needs_profile_usps_verification?).and_return(true) + allow(user.decorate).to receive(:pending_profile_requires_verification?).and_return(true) end it 'calls the UspsConfirmationMaker to send another letter and redirects' do diff --git a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb index cae8cb32be7..2ce79386881 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -264,7 +264,7 @@ sign_in_as_user subject.user_session[:unconfirmed_phone] = '+1 (703) 555-5555' subject.user_session[:context] = 'confirmation' - @previous_phone_confirmed_at = subject.current_user.phone_confirmed_at + @previous_phone_confirmed_at = subject.current_user.phone_configuration&.confirmed_at subject.current_user.create_direct_otp stub_analytics allow(@analytics).to receive(:track_event) @@ -272,7 +272,7 @@ @mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) allow(UserMailer).to receive(:phone_changed).with(subject.current_user). and_return(@mailer) - @previous_phone = subject.current_user.phone + @previous_phone = subject.current_user.phone_configuration&.phone end context 'user has an existing phone number' do @@ -442,7 +442,7 @@ idv_session.params = { 'phone' => '+1 (703) 555-5555' } subject.user_session[:unconfirmed_phone] = '+1 (703) 555-5555' subject.user_session[:context] = 'idv' - @previous_phone_confirmed_at = subject.current_user.phone_confirmed_at + @previous_phone_confirmed_at = subject.current_user.phone_configuration&.confirmed_at allow(subject).to receive(:idv_session).and_return(idv_session) stub_analytics allow(@analytics).to receive(:track_event) diff --git a/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb index 6d86cd32d07..bc0f98abbfc 100644 --- a/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb @@ -102,8 +102,8 @@ expect(subject.current_user.reload.second_factor_attempts_count).to eq 1 end - it 're-renders the piv/cac entry screen' do - expect(response).to render_template(:show) + it 'redirects to the piv/cac entry screen' do + expect(response).to redirect_to login_two_factor_piv_cac_path end it 'displays flash error message' do @@ -126,8 +126,8 @@ expect(subject.current_user.reload.second_factor_attempts_count).to eq 1 end - it 're-renders the piv/cac entry screen' do - expect(response).to render_template(:show) + it 'redirects to the piv/cac entry screen' do + expect(response).to redirect_to login_two_factor_piv_cac_path end it 'displays flash error message' do diff --git a/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb b/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb index 03d9a4afbb2..bd282961ad3 100644 --- a/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb +++ b/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb @@ -111,7 +111,7 @@ context 'when redirected with an error token' do it 'renders the error template' do get :new, params: { token: bad_token } - expect(response).to render_template(:error) + expect(response).to redirect_to setup_piv_cac_path end it 'resets the piv/cac session information' do diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index e7c3161b716..e17882442ca 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -161,6 +161,7 @@ user_id: user.uuid, user_locked_out: false, stored_location: 'http://example.com', + sp_request_url_present: false, } expect(@analytics).to receive(:track_event). @@ -178,6 +179,7 @@ user_id: user.uuid, user_locked_out: false, stored_location: nil, + sp_request_url_present: false, } expect(@analytics).to receive(:track_event). @@ -193,6 +195,7 @@ user_id: 'anonymous-uuid', user_locked_out: false, stored_location: nil, + sp_request_url_present: false, } expect(@analytics).to receive(:track_event). @@ -214,6 +217,7 @@ user_id: user.uuid, user_locked_out: true, stored_location: nil, + sp_request_url_present: false, } expect(@analytics).to receive(:track_event). @@ -222,6 +226,23 @@ post :create, params: { user: { email: user.email.upcase, password: user.password } } end + it 'tracks the presence of SP request_url in session' do + subject.session[:sp] = { request_url: 'http://example.com' } + stub_analytics + analytics_hash = { + success: false, + user_id: 'anonymous-uuid', + user_locked_out: false, + stored_location: nil, + sp_request_url_present: true, + } + + expect(@analytics).to receive(:track_event). + with(Analytics::EMAIL_AND_PASSWORD_AUTH, analytics_hash) + + post :create, params: { user: { email: 'foo@example.com', password: 'password' } } + end + context 'LOA1 user' do it 'computes one SCrypt hash for the user password' do user = create(:user, :signed_up) @@ -281,6 +302,7 @@ user_id: user.uuid, user_locked_out: false, stored_location: nil, + sp_request_url_present: false, } expect(@analytics).to receive(:track_event). @@ -382,7 +404,6 @@ profile = create( :profile, deactivation_reason: :verification_pending, - phone_confirmed: false, pii: { ssn: '6666', dob: '1920-01-01' } ) user = profile.user diff --git a/spec/controllers/users/two_factor_authentication_controller_spec.rb b/spec/controllers/users/two_factor_authentication_controller_spec.rb index 0f7173ce72b..522182857bb 100644 --- a/spec/controllers/users/two_factor_authentication_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_controller_spec.rb @@ -134,7 +134,7 @@ def index expect(SmsOtpSenderJob).to have_received(:perform_later).with( code: subject.current_user.direct_otp, - phone: subject.current_user.phone, + phone: subject.current_user.phone_configuration.phone, otp_created_at: subject.current_user.direct_otp_sent_at.to_s, message: 'jobs.sms_otp_sender_job.login_message', locale: nil @@ -151,7 +151,7 @@ def index expect(SmsOtpSenderJob).to have_received(:perform_later).with( code: subject.current_user.direct_otp, - phone: subject.current_user.phone, + phone: subject.current_user.phone_configuration.phone, otp_created_at: subject.current_user.direct_otp_sent_at.to_s, message: 'jobs.sms_otp_sender_job.login_message', locale: nil @@ -167,7 +167,7 @@ def index otp_delivery_preference: 'sms', resend: nil, context: 'authentication', - country_code: '1', + country_code: 'US', area_code: '202', } @@ -179,8 +179,10 @@ def index it 'calls OtpRateLimiter#exceeded_otp_send_limit? and #increment' do otp_rate_limiter = instance_double(OtpRateLimiter) - allow(OtpRateLimiter).to receive(:new).with(phone: @user.phone, user: @user). - and_return(otp_rate_limiter) + allow(OtpRateLimiter).to receive(:new).with( + phone: @user.phone_configuration.phone, + user: @user + ).and_return(otp_rate_limiter) expect(otp_rate_limiter).to receive(:exceeded_otp_send_limit?).twice expect(otp_rate_limiter).to receive(:increment) @@ -216,7 +218,7 @@ def index expect(VoiceOtpSenderJob).to have_received(:perform_later).with( code: subject.current_user.direct_otp, - phone: subject.current_user.phone, + phone: subject.current_user.phone_configuration.phone, otp_created_at: subject.current_user.direct_otp_sent_at.to_s, locale: nil ) @@ -236,7 +238,7 @@ def index otp_delivery_preference: 'voice', resend: nil, context: 'authentication', - country_code: '1', + country_code: 'US', area_code: '202', } @@ -341,7 +343,7 @@ def index otp_delivery_preference: 'sms', resend: nil, context: 'confirmation', - country_code: '1', + country_code: 'US', area_code: '202', } twilio_error = "[HTTP 400] : error message\n\n" @@ -379,7 +381,7 @@ def index otp_delivery_preference: 'sms', resend: nil, context: 'confirmation', - country_code: '1', + country_code: 'US', area_code: '202', } twilio_error_hash = { diff --git a/spec/controllers/users/verify_account_controller_spec.rb b/spec/controllers/users/verify_account_controller_spec.rb index 16fb4af70ee..c982177f313 100644 --- a/spec/controllers/users/verify_account_controller_spec.rb +++ b/spec/controllers/users/verify_account_controller_spec.rb @@ -17,8 +17,7 @@ profile: pending_profile, otp_fingerprint: Pii::Fingerprinter.fingerprint(otp) ) - allow(decorated_user).to receive(:needs_profile_phone_verification?).and_return(false) - allow(decorated_user).to receive(:needs_profile_usps_verification?). + allow(decorated_user).to receive(:pending_profile_requires_verification?). and_return(has_pending_profile) end diff --git a/spec/controllers/users/verify_profile_phone_controller_spec.rb b/spec/controllers/users/verify_profile_phone_controller_spec.rb deleted file mode 100644 index 980d2b084f6..00000000000 --- a/spec/controllers/users/verify_profile_phone_controller_spec.rb +++ /dev/null @@ -1,56 +0,0 @@ -require 'rails_helper' - -RSpec.describe Users::VerifyProfilePhoneController do - include Features::LocalizationHelper - - let(:has_pending_profile) { true } - let(:user) { create(:user) } - let(:profile_phone) { user.phone } - let(:phone_confirmed) { false } - let(:pii_attributes) { Pii::Attributes.new_from_hash(phone: profile_phone) } - let(:pending_profile) { build(:profile, phone_confirmed: phone_confirmed) } - - before do - stub_sign_in(user) - decorated_user = stub_decorated_user_with_pending_profile(user) - allow(decorated_user).to receive(:needs_profile_phone_verification?). - and_return(has_pending_profile) - allow(decorated_user).to receive(:needs_profile_usps_verification?).and_return(false) - allow(controller).to receive(:decrypted_pii).and_return(pii_attributes) - end - - describe '#index' do - context 'user has pending profile' do - context 'phone is not confirmed' do - it 'redirects to profile page' do - get :index - - expect(response).to redirect_to(account_url) - end - end - - context 'phone is confirmed and different than 2FA' do - let(:profile_phone) { '703-555-9999' } - let(:phone_confirmed) { true } - - it 'redirects to OTP confirmation flow' do - get :index - - expect(response).to redirect_to( - otp_send_path(otp_delivery_selection_form: { otp_delivery_preference: 'sms' }) - ) - end - end - end - - context 'user does not have pending profile' do - let(:has_pending_profile) { false } - - it 'redirects to profile page' do - get :index - - expect(response).to redirect_to(account_url) - end - end - end -end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 828f8076c20..70a196645da 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -58,7 +58,7 @@ parser = instance_double(ParseControllerFromReferer) expect(ParseControllerFromReferer).to receive(:new).and_return(parser) - expect(parser).to receive(:call) + expect(parser).to receive(:call).and_return({}) delete :destroy end diff --git a/spec/decorators/service_provider_session_decorator_spec.rb b/spec/decorators/service_provider_session_decorator_spec.rb index 6336bd8da51..b8b76baeaa6 100644 --- a/spec/decorators/service_provider_session_decorator_spec.rb +++ b/spec/decorators/service_provider_session_decorator_spec.rb @@ -189,4 +189,19 @@ to eq 'https://www.example.com/sign_up/start' end end + + describe '#failure_to_proof_url' do + it 'returns the failure_to_proof_url if present on the sp' do + url = 'https://www.example.com/fail' + allow_any_instance_of(ServiceProvider).to receive(:failure_to_proof_url).and_return(url) + expect(subject.failure_to_proof_url).to eq url + end + + it 'returns the return_to_sp_url if the failure_to_proof_url is not present on the sp' do + url = 'https://www.example.com/' + allow_any_instance_of(ServiceProvider).to receive(:failure_to_proof_url).and_return(nil) + allow_any_instance_of(ServiceProvider).to receive(:return_to_sp_url).and_return(url) + expect(subject.failure_to_proof_url).to eq url + end + end end diff --git a/spec/decorators/user_decorator_spec.rb b/spec/decorators/user_decorator_spec.rb index 937a77e2814..6f5b5e08236 100644 --- a/spec/decorators/user_decorator_spec.rb +++ b/spec/decorators/user_decorator_spec.rb @@ -204,84 +204,6 @@ end end - describe '#needs_profile_phone_verification?' do - context 'pending profile does not require verification' do - it 'returns false' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user_decorator).to receive(:pending_profile_requires_verification?). - and_return(false) - - expect(user_decorator.needs_profile_phone_verification?).to eq false - end - end - - context 'pending profile requires verification and phone is confirmed' do - it 'returns true' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user_decorator).to receive(:pending_profile_requires_verification?). - and_return(true) - allow(user_decorator).to receive(:pending_profile). - and_return(Profile.new(phone_confirmed: true)) - - expect(user_decorator.needs_profile_phone_verification?).to eq true - end - end - - context 'pending profile requires verification and phone is not confirmed' do - it 'returns false' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user_decorator).to receive(:pending_profile_requires_verification?). - and_return(true) - allow(user_decorator).to receive(:pending_profile). - and_return(Profile.new(phone_confirmed: false)) - - expect(user_decorator.needs_profile_phone_verification?).to eq false - end - end - end - - describe '#needs_profile_usps_verification?' do - context 'pending profile does not require verification' do - it 'returns false' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user_decorator).to receive(:pending_profile_requires_verification?). - and_return(false) - - expect(user_decorator.needs_profile_usps_verification?).to eq false - end - end - - context 'pending profile requires verification and phone is not confirmed' do - it 'returns true' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user_decorator).to receive(:pending_profile_requires_verification?). - and_return(true) - allow(user_decorator).to receive(:pending_profile). - and_return(Profile.new(phone_confirmed: false)) - - expect(user_decorator.needs_profile_usps_verification?).to eq true - end - end - - context 'pending profile requires verification and phone is confirmed' do - it 'returns false' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user_decorator).to receive(:pending_profile_requires_verification?). - and_return(true) - allow(user_decorator).to receive(:pending_profile). - and_return(Profile.new(phone_confirmed: true)) - - expect(user_decorator.needs_profile_usps_verification?).to eq false - end - end - end - describe '#should_acknowledge_personal_key?' do context 'user has no personal key' do context 'service provider with loa1' do diff --git a/spec/factories/users.rb b/spec/factories/users.rb index f4f685f34c2..10a64ff94f1 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -16,6 +16,16 @@ end end + after :stub do |user| + if user.phone + user.phone_configuration = build_stubbed(:phone_configuration, + user: user, + phone: user.phone, + confirmed_at: user.phone_confirmed_at, + delivery_preference: user.otp_delivery_preference) + end + end + trait :with_phone do phone '+1 202-555-1212' phone_confirmed_at Time.zone.now diff --git a/spec/features/account_reset/delete_account_spec.rb b/spec/features/account_reset/delete_account_spec.rb index e9635fdf13c..7daa30108fe 100644 --- a/spec/features/account_reset/delete_account_spec.rb +++ b/spec/features/account_reset/delete_account_spec.rb @@ -13,6 +13,14 @@ click_link t('two_factor_authentication.login_options_link_text') click_link t('devise.two_factor_authentication.account_reset.link') click_button t('account_reset.request.yes_continue') + + expect(page). + to have_content strip_tags( + t('account_reset.confirm_request.instructions', email: user.email) + ) + expect(page).to have_content t('account_reset.confirm_request.security_note') + expect(page).to have_content t('account_reset.confirm_request.close_window') + reset_email Timecop.travel(Time.zone.now + 2.days) do @@ -40,6 +48,28 @@ end end + context 'as an LOA1 user without a phone' do + it 'does not tell the user that an SMS was sent to their registered phone' do + user = create(:user, :with_authentication_app) + signin(user.email, user.password) + click_link t('two_factor_authentication.login_options_link_text') + click_link t('devise.two_factor_authentication.account_reset.link') + click_button t('account_reset.request.yes_continue') + + expect(page). + to have_content strip_tags( + t('account_reset.confirm_request.instructions', email: user.email) + ) + expect(page).to_not have_content t('account_reset.confirm_request.security_note') + expect(page).to have_content t('account_reset.confirm_request.close_window') + + # user should now be signed out + visit account_path + + expect(page).to have_current_path(new_user_session_path) + end + end + context 'as an LOA3 user' do let(:user) do create( diff --git a/spec/features/idv/steps/phone_step_spec.rb b/spec/features/idv/steps/phone_step_spec.rb index a4b775e522d..bbb61e1eb74 100644 --- a/spec/features/idv/steps/phone_step_spec.rb +++ b/spec/features/idv/steps/phone_step_spec.rb @@ -19,7 +19,7 @@ user = user_with_2fa start_idv_from_sp complete_idv_steps_before_phone_step(user) - fill_out_phone_form_ok(user.phone) + fill_out_phone_form_ok(user.phone_configuration.phone) click_idv_continue expect(page).to have_content(t('idv.titles.session.review')) diff --git a/spec/features/idv/steps/review_step_spec.rb b/spec/features/idv/steps/review_step_spec.rb index 76926e73023..cb42d043f6c 100644 --- a/spec/features/idv/steps/review_step_spec.rb +++ b/spec/features/idv/steps/review_step_spec.rb @@ -46,7 +46,6 @@ profile = user.profiles.first expect(profile.active?).to eq true - expect(profile.phone_confirmed).to eq true expect(UspsConfirmation.count).to eq(0) end end @@ -72,7 +71,6 @@ profile = user.profiles.first expect(profile.active?).to eq false - expect(profile.phone_confirmed).to eq false end context 'with an sp' do diff --git a/spec/features/idv/steps/usps_step_spec.rb b/spec/features/idv/steps/usps_step_spec.rb index f0f6b82d810..e8e1686fa0a 100644 --- a/spec/features/idv/steps/usps_step_spec.rb +++ b/spec/features/idv/steps/usps_step_spec.rb @@ -56,7 +56,6 @@ def expect_user_to_be_unverified(user) expect(profile.active?).to eq false expect(profile.deactivation_reason).to eq 'verification_pending' - expect(profile.phone_confirmed).to eq false end end diff --git a/spec/features/saml/loa1_sso_spec.rb b/spec/features/saml/loa1_sso_spec.rb index e700001bc12..4aa7596082e 100644 --- a/spec/features/saml/loa1_sso_spec.rb +++ b/spec/features/saml/loa1_sso_spec.rb @@ -81,7 +81,7 @@ it 'user can view and confirm personal key during sign up', :js do allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) user = create(:user, :with_phone) - code = 'ABC1-DEF2-GHI3-JKL4' + code = 'ABC1-DEF2-GH13-JK14' stub_personal_key(user: user, code: code) loa1_sp_session @@ -95,6 +95,25 @@ expect(current_path).to eq sign_up_completed_path end + it 'coerces invalid characters into their Crockford Base32 equivalents', :js do + displayed_personal_key = '0000-1111-1111-1234' + misread_personal_key = 'ooOO-iiII-llLL-1234' + + allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) + user = create(:user, :with_phone) + stub_personal_key(user: user, code: displayed_personal_key) + + loa1_sp_session + sign_in_and_require_viewing_personal_key(user) + expect(current_path).to eq sign_up_personal_key_path + + click_on(t('forms.buttons.continue')) + enter_personal_key_words_on_modal(misread_personal_key) + click_on t('forms.buttons.continue'), class: 'personal-key-confirm' + + expect(current_path).to eq sign_up_completed_path + end + it 'redirects user to SP after asking for new personal key during sign up' do allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) diff --git a/spec/features/sign_in/two_factor_options_spec.rb b/spec/features/sign_in/two_factor_options_spec.rb index 65e95df3ed1..c1b49981453 100644 --- a/spec/features/sign_in/two_factor_options_spec.rb +++ b/spec/features/sign_in/two_factor_options_spec.rb @@ -61,6 +61,26 @@ end end + context "the user's otp_delivery_preference is voice but number is unsupported" do + it 'only displays SMS and Personal key' do + user = create(:user, :signed_up, otp_delivery_preference: 'voice', phone: '+12423270143') + sign_in_user(user) + + click_link t('two_factor_authentication.login_options_link_text') + + expect(page). + to have_content t('two_factor_authentication.login_options.sms') + expect(page). + to_not have_content t('two_factor_authentication.login_options.voice') + expect(page). + to have_content t('two_factor_authentication.login_options.personal_key') + expect(page). + to_not have_content t('two_factor_authentication.login_options.piv_cac') + expect(page). + to_not have_content t('two_factor_authentication.login_options.auth_app') + end + end + context 'when the user only has TOTP configured' do it 'only displays TOTP and Personal key' do user = create(:user, :with_authentication_app) diff --git a/spec/features/two_factor_authentication/change_factor_spec.rb b/spec/features/two_factor_authentication/change_factor_spec.rb index 5550497fbff..06a53905f73 100644 --- a/spec/features/two_factor_authentication/change_factor_spec.rb +++ b/spec/features/two_factor_authentication/change_factor_spec.rb @@ -60,6 +60,7 @@ scenario 'editing phone number with no voice otp support only allows sms delivery' do user.update(otp_delivery_preference: 'voice') + user.phone_configuration.update(delivery_preference: 'voice') unsupported_phone = '242-327-0143' visit manage_phone_path @@ -82,7 +83,7 @@ allow(SmsOtpSenderJob).to receive(:perform_later) user = sign_in_and_2fa_user - old_phone = user.phone + old_phone = user.phone_configuration.phone visit manage_phone_path update_phone_number @@ -109,7 +110,7 @@ allow(SmsOtpSenderJob).to receive(:perform_later) user = sign_in_and_2fa_user - old_phone = user.phone + old_phone = user.phone_configuration.phone Timecop.travel(Figaro.env.reauthn_window.to_i + 1) do visit manage_phone_path diff --git a/spec/features/two_factor_authentication/multiple_tabs_spec.rb b/spec/features/two_factor_authentication/multiple_tabs_spec.rb new file mode 100644 index 00000000000..0593d891557 --- /dev/null +++ b/spec/features/two_factor_authentication/multiple_tabs_spec.rb @@ -0,0 +1,9 @@ +require 'rails_helper' + +feature 'user interacts with 2FA across multiple browser tabs' do + include SpAuthHelper + include SamlAuthHelper + + it_behaves_like 'visiting 2fa when fully authenticated', :oidc + it_behaves_like 'visiting 2fa when fully authenticated', :saml +end diff --git a/spec/features/two_factor_authentication/remember_device_spec.rb b/spec/features/two_factor_authentication/remember_device_spec.rb index 2f31bb951a0..297348d158a 100644 --- a/spec/features/two_factor_authentication/remember_device_spec.rb +++ b/spec/features/two_factor_authentication/remember_device_spec.rb @@ -2,6 +2,7 @@ feature 'Remembering a 2FA device' do include IdvHelper + include SamlAuthHelper before do allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) @@ -22,6 +23,7 @@ def remember_device_and_sign_out_user end it_behaves_like 'remember device' + it_behaves_like 'remember device after being idle on sign in page' end context 'sign up' do diff --git a/spec/features/two_factor_authentication/sign_in_spec.rb b/spec/features/two_factor_authentication/sign_in_spec.rb index 6dbc01bf4f3..a6eaaa5b6a8 100644 --- a/spec/features/two_factor_authentication/sign_in_spec.rb +++ b/spec/features/two_factor_authentication/sign_in_spec.rb @@ -350,7 +350,7 @@ def submit_prefilled_otp_code expect(current_path).to eq account_path - phone_fingerprint = Pii::Fingerprinter.fingerprint(user.phone) + phone_fingerprint = Pii::Fingerprinter.fingerprint(user.phone_configuration.phone) rate_limited_phone = OtpRequestsTracker.find_by(phone_fingerprint: phone_fingerprint) # let findtime period expire @@ -387,7 +387,7 @@ def submit_prefilled_otp_code sign_in_before_2fa(second_user) click_link t('links.two_factor_authentication.get_another_code') - phone_fingerprint = Pii::Fingerprinter.fingerprint(first_user.phone) + phone_fingerprint = Pii::Fingerprinter.fingerprint(first_user.phone_configuration.phone) rate_limited_phone = OtpRequestsTracker.find_by(phone_fingerprint: phone_fingerprint) expect(current_path).to eq otp_send_path diff --git a/spec/features/users/piv_cac_management_spec.rb b/spec/features/users/piv_cac_management_spec.rb index 093b26add42..1d39546ab11 100644 --- a/spec/features/users/piv_cac_management_spec.rb +++ b/spec/features/users/piv_cac_management_spec.rb @@ -58,6 +58,40 @@ def find_form(page, attributes) expect(user.x509_dn_uuid).to eq uuid end + scenario 'displays error for a bad piv/cac' do + stub_piv_cac_service + + sign_in_and_2fa_user(user) + visit account_path + click_link t('forms.buttons.enable'), href: setup_piv_cac_url + + expect(page).to have_link(t('forms.piv_cac_setup.submit')) + + nonce = get_piv_cac_nonce_from_link(find_link(t('forms.piv_cac_setup.submit'))) + visit_piv_cac_service(setup_piv_cac_url, + nonce: nonce, + error: 'certificate.bad') + expect(current_path).to eq setup_piv_cac_path + expect(page).to have_content(t('headings.piv_cac_setup.certificate.bad')) + end + + scenario 'displays error for an expired piv/cac' do + stub_piv_cac_service + + sign_in_and_2fa_user(user) + visit account_path + click_link t('forms.buttons.enable'), href: setup_piv_cac_url + + expect(page).to have_link(t('forms.piv_cac_setup.submit')) + + nonce = get_piv_cac_nonce_from_link(find_link(t('forms.piv_cac_setup.submit'))) + visit_piv_cac_service(setup_piv_cac_url, + nonce: nonce, + error: 'certificate.expired') + expect(current_path).to eq setup_piv_cac_path + expect(page).to have_content(t('headings.piv_cac_setup.certificate.expired')) + end + scenario "doesn't allow unassociation of a piv/cac" do stub_piv_cac_service @@ -111,7 +145,7 @@ def find_form(page, attributes) PivCacService.send(:reset_piv_cac_avaialable_agencies) end - scenario 'does not allow association of a piv/cac with an account' do + scenario "doesn't advertise association of a piv/cac with an account" do stub_piv_cac_service sign_in_and_2fa_user(user) diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index 7952f44d5b6..04f72777622 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -320,7 +320,7 @@ it 'falls back to SMS with an error message' do allow(SmsOtpSenderJob).to receive(:perform_later) allow(VoiceOtpSenderJob).to receive(:perform_later) - user = create(:user, :signed_up, phone: '+91 1234567890', otp_delivery_preference: 'voice') + user = create(:user, :signed_up, phone: '+1 441-295-9644', otp_delivery_preference: 'voice') signin(user.email, user.password) expect(VoiceOtpSenderJob).to_not have_received(:perform_later) @@ -329,7 +329,7 @@ to have_current_path(login_two_factor_path(otp_delivery_preference: 'sms', reauthn: false)) expect(page).to have_content t( 'devise.two_factor_authentication.otp_delivery_preference.phone_unsupported', - location: 'India' + location: 'Bermuda' ) expect(user.reload.otp_delivery_preference).to eq 'sms' end diff --git a/spec/features/users/verify_profile_spec.rb b/spec/features/users/verify_profile_spec.rb index f02f6149ca2..5f361ec6366 100644 --- a/spec/features/users/verify_profile_spec.rb +++ b/spec/features/users/verify_profile_spec.rb @@ -8,8 +8,7 @@ profile = create( :profile, deactivation_reason: :verification_pending, - pii: { ssn: '666-66-1234', dob: '1920-01-01', phone: '703-555-9999' }, - phone_confirmed: phone_confirmed, + pii: { ssn: '666-66-1234', dob: '1920-01-01', phone: '+1 703-555-9999' }, user: user ) otp_fingerprint = Pii::Fingerprinter.fingerprint(otp) @@ -17,11 +16,13 @@ end context 'USPS letter' do - let(:phone_confirmed) { false } - - scenario 'profile phone not confirmed' do + scenario 'valid OTP' do sign_in_live_with_2fa(user) - expect(page).to have_link(t('idv.buttons.cancel'), href: account_path) + fill_in t('forms.verify_profile.name'), with: otp + click_button t('forms.verify_profile.submit') + + expect(page).to have_content(t('account.index.verification.success')) + expect(page).to have_current_path(account_path) end scenario 'OTP has expired' do @@ -45,20 +46,4 @@ expect(page.body).to_not match('the wrong code') end end - - context 'profile phone confirmed' do - let(:phone_confirmed) { true } - - before do - allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) - end - - scenario 'not yet verified with user' do - sign_in_live_with_2fa(user) - click_submit_default - - expect(current_path).to eq account_path - expect(page).to_not have_content(t('account.index.verification.with_phone_button')) - end - end end diff --git a/spec/features/visitors/phone_confirmation_spec.rb b/spec/features/visitors/phone_confirmation_spec.rb index d8f0969def7..de472a7d4c0 100644 --- a/spec/features/visitors/phone_confirmation_spec.rb +++ b/spec/features/visitors/phone_confirmation_spec.rb @@ -61,7 +61,7 @@ @existing_user = create(:user, :signed_up) @user = sign_in_before_2fa select_2fa_option('sms') - fill_in 'user_phone_form_phone', with: @existing_user.phone + fill_in 'user_phone_form_phone', with: @existing_user.phone_configuration.phone click_send_security_code end diff --git a/spec/forms/idv/otp_delivery_method_form_spec.rb b/spec/forms/idv/otp_delivery_method_form_spec.rb new file mode 100644 index 00000000000..5c08f6404bc --- /dev/null +++ b/spec/forms/idv/otp_delivery_method_form_spec.rb @@ -0,0 +1,38 @@ +require 'rails_helper' + +describe Idv::OtpDeliveryMethodForm do + let(:otp_delivery_preference) { 'sms' } + let(:params) { { otp_delivery_preference: otp_delivery_preference } } + + describe '#submit' do + context 'with sms as the delivery method' do + it 'is successful' do + result = subject.submit(params) + + expect(result.success?).to eq(true) + expect(subject.otp_delivery_preference).to eq('sms') + end + end + + context 'with voice as the delivery method' do + let(:otp_delivery_preference) { 'voice' } + + it 'is successful' do + result = subject.submit(params) + + expect(result.success?).to eq(true) + expect(subject.otp_delivery_preference).to eq('voice') + end + end + + context 'with an unsupported value as the delivery method' do + let(:otp_delivery_preference) { '☎️' } + + it 'is unsuccessful' do + result = subject.submit(params) + + expect(result.success?).to eq(false) + end + end + end +end diff --git a/spec/forms/idv/phone_form_spec.rb b/spec/forms/idv/phone_form_spec.rb index 257b0df8bbd..00412d34256 100644 --- a/spec/forms/idv/phone_form_spec.rb +++ b/spec/forms/idv/phone_form_spec.rb @@ -45,7 +45,7 @@ expected_params = { phone: '2025551212', - phone_confirmed_at: user.phone_confirmed_at, + phone_confirmed_at: user.phone_configuration.confirmed_at, } expect(subject.idv_params).to eq expected_params diff --git a/spec/forms/otp_delivery_selection_form_spec.rb b/spec/forms/otp_delivery_selection_form_spec.rb index d1a50d84ae0..2ab054ae758 100644 --- a/spec/forms/otp_delivery_selection_form_spec.rb +++ b/spec/forms/otp_delivery_selection_form_spec.rb @@ -25,7 +25,7 @@ extra = { otp_delivery_preference: 'sms', resend: true, - country_code: '1', + country_code: 'US', area_code: '202', context: 'authentication', } diff --git a/spec/forms/user_phone_form_spec.rb b/spec/forms/user_phone_form_spec.rb index cdd712b4753..b64bd59bd11 100644 --- a/spec/forms/user_phone_form_spec.rb +++ b/spec/forms/user_phone_form_spec.rb @@ -23,7 +23,7 @@ ) subject = UserPhoneForm.new(user) - expect(subject.phone).to eq(user.phone) + expect(subject.phone).to eq(user.phone_configuration.phone) expect(subject.international_code).to eq('US') expect(subject.otp_delivery_preference).to eq(user.otp_delivery_preference) end @@ -212,7 +212,7 @@ end it 'returns false if the user phone has not changed' do - params[:phone] = user.phone + params[:phone] = user.phone_configuration.phone subject.submit(params) expect(subject.phone_changed?).to eq(false) diff --git a/spec/jobs/sms_account_reset_notifier_job_spec.rb b/spec/jobs/sms_account_reset_notifier_job_spec.rb index d80cdfe9ee6..822ce2935fa 100644 --- a/spec/jobs/sms_account_reset_notifier_job_spec.rb +++ b/spec/jobs/sms_account_reset_notifier_job_spec.rb @@ -14,7 +14,7 @@ subject(:perform) do SmsAccountResetNotifierJob.perform_now( phone: '+1 (888) 555-5555', - cancel_token: 'UUID1' + token: 'UUID1' ) end diff --git a/spec/lib/feature_management_spec.rb b/spec/lib/feature_management_spec.rb index 59d208348d7..cd2b68e5120 100644 --- a/spec/lib/feature_management_spec.rb +++ b/spec/lib/feature_management_spec.rb @@ -181,48 +181,34 @@ end end - describe '.no_pii_mode?' do - let(:proofing_vendor) { :mock } - let(:enable_identity_verification) { false } - - before do - allow_any_instance_of(Figaro.env).to receive(:profile_proofing_vendor). - and_return(proofing_vendor) - allow(Figaro.env).to receive(:enable_identity_verification). - and_return(enable_identity_verification.to_json) - end - - subject(:no_pii_mode?) { FeatureManagement.no_pii_mode? } - - context 'with mock ID-proofing vendors' do - let(:proofing_vendor) { :mock } - - context 'with identity verification enabled' do - let(:enable_identity_verification) { true } - - it { expect(no_pii_mode?).to eq(true) } - end - - context 'with identity verification disabled' do - let(:enable_identity_verification) { false } - - it { expect(no_pii_mode?).to eq(false) } + describe '.fake_banner_mode?' do + context 'when on secure.login.gov' do + it 'does not display the fake banner' do + allow(Figaro.env).to receive(:domain_name). + and_return('secure.login.gov') + allow(Rails.env).to receive(:production?). + and_return(true) + expect(FeatureManagement.fake_banner_mode?).to eq(false) end end - context 'with real ID-proofing vendors' do - let(:proofing_vendor) { :not_mock } - - context 'with identity verification enabled' do - let(:enable_identity_verification) { true } - - it { expect(no_pii_mode?).to eq(false) } + context 'when the host is not secure.login.gov and the Rails env is production' do + it 'displays the fake banner' do + allow(Figaro.env).to receive(:domain_name). + and_return('test.login.gov') + allow(Rails.env).to receive(:production?). + and_return(true) + expect(FeatureManagement.fake_banner_mode?).to eq(true) end + end - context 'with identity verification disabled' do - let(:enable_identity_verification) { false } - - it { expect(no_pii_mode?).to eq(false) } + context 'when the host is not secure.login.gov and the Rails env is not in production' do + it 'does not display the fake banner' do + allow(Figaro.env).to receive(:domain_name). + and_return('test.login.gov') + allow(Rails.env).to receive(:production?). + and_return(false) + expect(FeatureManagement.fake_banner_mode?).to eq(false) end end end diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb index fa3d4aba0eb..d327c4fb468 100644 --- a/spec/models/identity_spec.rb +++ b/spec/models/identity_spec.rb @@ -133,10 +133,9 @@ describe '#piv_cac_available?' do context 'when agency configured to support piv/cac' do before(:each) do - allow(Figaro.env).to receive(:piv_cac_agencies).and_return( - [service_provider.agency].to_json - ) - PivCacService.send(:reset_piv_cac_avaialable_agencies) + allow(PivCacService).to receive(:piv_cac_available_for_agency?).with( + service_provider.agency, identity_with_sp.user.email + ).and_return(true) end it 'returns truthy' do @@ -146,10 +145,9 @@ context 'when agency is not configured to support piv/cac' do before(:each) do - allow(Figaro.env).to receive(:piv_cac_agencies).and_return( - [service_provider.agency + 'X'].to_json - ) - PivCacService.send(:reset_piv_cac_avaialable_agencies) + allow(PivCacService).to receive(:piv_cac_available_for_agency?).with( + service_provider.agency, identity_with_sp.user.email + ).and_return(false) end it 'returns falsey' do diff --git a/spec/models/service_provider_spec.rb b/spec/models/service_provider_spec.rb index 6fb3c2def22..663e401de83 100644 --- a/spec/models/service_provider_spec.rb +++ b/spec/models/service_provider_spec.rb @@ -70,25 +70,30 @@ describe 'piv_cac_available?' do context 'when the service provider is with an enabled agency' do it 'is truthy' do - allow(Figaro.env).to receive(:piv_cac_agencies).and_return( - [service_provider.agency].to_json - ) - PivCacService.send(:reset_piv_cac_avaialable_agencies) - + allow(PivCacService).to receive(:piv_cac_available_for_agency?).and_return(true) expect(service_provider.piv_cac_available?).to be_truthy end end context 'when the service provider agency is not enabled' do it 'is falsey' do - allow(Figaro.env).to receive(:piv_cac_agencies).and_return( - [service_provider.agency + 'X'].to_json - ) - PivCacService.send(:reset_piv_cac_avaialable_agencies) + allow(PivCacService).to receive(:piv_cac_available_for_agency?).and_return(false) expect(service_provider.piv_cac_available?).to be_falsey end end + + context 'when the service provider setting depends on the user email' do + let(:user) { build(:user) } + + it 'calls with the user email' do + expect(PivCacService).to receive( + :piv_cac_available_for_agency? + ).with(service_provider.agency, user.email) + + service_provider.piv_cac_available?(user) + end + end end describe '#encryption_opts' do diff --git a/spec/policies/piv_cac_login_option_policy_spec.rb b/spec/policies/piv_cac_login_option_policy_spec.rb new file mode 100644 index 00000000000..b17cefbb3c9 --- /dev/null +++ b/spec/policies/piv_cac_login_option_policy_spec.rb @@ -0,0 +1,66 @@ +require 'rails_helper' + +describe PivCacLoginOptionPolicy do + let(:subject) { described_class.new(user) } + + describe '#configured?' do + context 'without a piv configured' do + let(:user) { build(:user) } + + it { expect(subject.configured?).to be_falsey } + end + + context 'with a piv configured' do + let(:user) { build(:user, :with_piv_or_cac) } + + it { expect(subject.configured?).to be_truthy } + end + end + + describe '#enabled?' do + context 'without a piv configured' do + let(:user) { build(:user) } + + it { expect(subject.configured?).to be_falsey } + end + + context 'with a piv configured' do + let(:user) { build(:user, :with_piv_or_cac) } + + it { expect(subject.configured?).to be_truthy } + end + end + + describe '#available?' do + let(:user) { build(:user) } + + context 'when enabled' do + before(:each) do + allow(subject).to receive(:enabled?).and_return(true) + end + + it { expect(subject.available?).to be_truthy } + end + + context 'when associated with a supported identity' do + before(:each) do + identity = double + allow(identity).to receive(:piv_cac_available?).and_return(true) + allow(user).to receive(:identities).and_return([identity]) + end + + it { expect(subject.available?).to be_truthy } + end + + context 'when not enabled and not a supported identity' do + before(:each) do + identity = double + allow(identity).to receive(:piv_cac_available?).and_return(false) + allow(user).to receive(:identities).and_return([identity]) + allow(subject).to receive(:enabled?).and_return(false) + end + + it { expect(subject.available?).to be_falsey } + end + end +end diff --git a/spec/presenters/idv/idv_failure_presenter_spec.rb b/spec/presenters/idv/idv_failure_presenter_spec.rb index fad49064627..bdb860215a2 100644 --- a/spec/presenters/idv/idv_failure_presenter_spec.rb +++ b/spec/presenters/idv/idv_failure_presenter_spec.rb @@ -48,38 +48,8 @@ describe '#next_steps' do subject { presenter.next_steps } - it 'includes `help_step`, `sp_step`, and `profile_step`' do - expect(subject).to eq( - [ - presenter.send(:help_step), - presenter.send(:sp_step), - presenter.send(:profile_step), - ] - ) - end - end - - describe '#help_step' do - subject { presenter.send(:help_step) } - - it 'includes help url' do - expect(subject).to include(MarketingSite.help_url) - end - end - - describe '#sp_step' do - subject { presenter.send(:sp_step) } - - it 'includes sp url' do - expect(subject).to include(decorated_session.sp_return_url) - end - end - - describe '#profile_step' do - subject { presenter.send(:profile_step) } - - it 'includes profile url' do - expect(subject).to include(view_context.account_path) + it 'is empty' do + expect(subject).to eq([]) end end diff --git a/spec/presenters/idv/otp_delivery_method_presenter_spec.rb b/spec/presenters/idv/otp_delivery_method_presenter_spec.rb deleted file mode 100644 index e547547e1a1..00000000000 --- a/spec/presenters/idv/otp_delivery_method_presenter_spec.rb +++ /dev/null @@ -1,31 +0,0 @@ -require 'rails_helper' - -describe Idv::OtpDeliveryMethodPresenter do - let(:phone) { '(703) 555-0000' } - let(:formatted_phone) { '+1 703-555-0000' } - let(:phone_number_capabilities) { PhoneNumberCapabilities.new(formatted_phone) } - - subject { Idv::OtpDeliveryMethodPresenter.new(phone) } - - before do - allow(PhoneNumberCapabilities).to receive(:new). - with(formatted_phone). - and_return(phone_number_capabilities) - end - - describe '#phone_unsupported_message' do - it 'returns a message saying the phone is unsupported in the location' do - unsupported_location = '🌃🌇🏙🌇🌃' - allow(phone_number_capabilities).to receive(:sms_only?).and_return(true) - allow(phone_number_capabilities).to receive(:unsupported_location). - and_return(unsupported_location) - - expect(subject.phone_unsupported_message).to eq( - t( - 'devise.two_factor_authentication.otp_delivery_preference.phone_unsupported', - location: unsupported_location - ) - ) - end - end -end diff --git a/spec/presenters/idv/ssn_failure_presenter_spec.rb b/spec/presenters/idv/ssn_failure_presenter_spec.rb index 476a10cf170..c57127fce0e 100644 --- a/spec/presenters/idv/ssn_failure_presenter_spec.rb +++ b/spec/presenters/idv/ssn_failure_presenter_spec.rb @@ -23,14 +23,8 @@ describe '#next_steps' do subject { presenter.next_steps } - it 'includes `try_again_step`, `sign_out_step`, and `profile_step`' do - expect(subject).to eq( - [ - presenter.send(:try_again_step), - presenter.send(:sign_out_step), - presenter.send(:profile_step), - ] - ) + it 'is empty' do + expect(subject).to eq([]) end end diff --git a/spec/presenters/piv_cac_authentication_setup_error_presenter_spec.rb b/spec/presenters/piv_cac_authentication_setup_error_presenter_spec.rb index b952156ed1b..30d5095ea2a 100644 --- a/spec/presenters/piv_cac_authentication_setup_error_presenter_spec.rb +++ b/spec/presenters/piv_cac_authentication_setup_error_presenter_spec.rb @@ -1,17 +1,12 @@ require 'rails_helper' describe PivCacAuthenticationSetupErrorPresenter do - let(:presenter) { described_class.new(form) } - let(:form) do - OpenStruct.new( - error_type: error - ) - end + let(:presenter) { described_class.new(error: error) } let(:error) { 'certificate.none' } describe '#error' do it 'reflects the form' do - expect(presenter.error).to eq form.error_type + expect(presenter.error).to eq error end end diff --git a/spec/services/account_reset/cancel_spec.rb b/spec/services/account_reset/cancel_spec.rb index f59132af926..377acedfc91 100644 --- a/spec/services/account_reset/cancel_spec.rb +++ b/spec/services/account_reset/cancel_spec.rb @@ -28,7 +28,7 @@ AccountReset::Cancel.new(token).call expect(SmsAccountResetCancellationNotifierJob). - to have_received(:perform_now).with(phone: user.phone) + to have_received(:perform_now).with(phone: user.phone_configuration.phone) end end @@ -37,6 +37,8 @@ token = create_account_reset_request_for(user) allow(SmsAccountResetCancellationNotifierJob).to receive(:perform_now) user.update!(phone: nil) + user.phone_configuration.destroy! + user.reload AccountReset::Cancel.new(token).call diff --git a/spec/services/account_reset_service_spec.rb b/spec/services/account_reset_service_spec.rb index 55f63f306d2..a7b50bb112d 100644 --- a/spec/services/account_reset_service_spec.rb +++ b/spec/services/account_reset_service_spec.rb @@ -4,35 +4,11 @@ include AccountResetHelper let(:user) { create(:user) } - let(:subject) { AccountResetService.new(user) } let(:user2) { create(:user) } - let(:subject2) { AccountResetService.new(user2) } - - describe '#create_request' do - it 'creates a new account reset request on the user' do - subject.create_request - arr = user.account_reset_request - expect(arr.request_token).to be_present - expect(arr.requested_at).to be_present - expect(arr.cancelled_at).to be_nil - expect(arr.granted_at).to be_nil - expect(arr.granted_token).to be_nil - end - - it 'creates a new account reset request in the db' do - subject.create_request - arr = AccountResetRequest.find_by(user_id: user.id) - expect(arr.request_token).to be_present - expect(arr.requested_at).to be_present - expect(arr.cancelled_at).to be_nil - expect(arr.granted_at).to be_nil - expect(arr.granted_token).to be_nil - end - end describe '#report_fraud' do it 'removes tokens from the request' do - subject.create_request + create_account_reset_request_for(user) AccountResetService.report_fraud(user.account_reset_request.request_token) arr = AccountResetRequest.find_by(user_id: user.id) expect(arr.request_token).to_not be_present @@ -60,9 +36,8 @@ describe '#grant_request' do it 'adds a notified at timestamp and granted token to the user' do - rd = subject - rd.create_request - rd.grant_request + create_account_reset_request_for(user) + AccountResetService.new(user).grant_request arr = AccountResetRequest.find_by(user_id: user.id) expect(arr.granted_at).to be_present expect(arr.granted_token).to be_present @@ -72,7 +47,7 @@ describe '.grant_tokens_and_send_notifications' do context 'after waiting the full wait period' do it 'does not send notifications when the notifications were already sent' do - subject.create_request + create_account_reset_request_for(user) after_waiting_the_full_wait_period do AccountResetService.grant_tokens_and_send_notifications @@ -82,7 +57,7 @@ end it 'does not send notifications when the request was cancelled' do - subject.create_request + create_account_reset_request_for(user) cancel_request_for(user) after_waiting_the_full_wait_period do @@ -92,7 +67,7 @@ end it 'sends notifications after a request is granted' do - subject.create_request + create_account_reset_request_for(user) after_waiting_the_full_wait_period do notifications_sent = AccountResetService.grant_tokens_and_send_notifications @@ -102,8 +77,8 @@ end it 'sends 2 notifications after 2 requests are granted' do - subject.create_request - subject2.create_request + create_account_reset_request_for(user) + create_account_reset_request_for(user2) after_waiting_the_full_wait_period do notifications_sent = AccountResetService.grant_tokens_and_send_notifications @@ -115,14 +90,14 @@ context 'after not waiting the full wait period' do it 'does not send notifications after a request' do - subject.create_request + create_account_reset_request_for(user) notifications_sent = AccountResetService.grant_tokens_and_send_notifications expect(notifications_sent).to eq(0) end it 'does not send notifications when the request was cancelled' do - subject.create_request + create_account_reset_request_for(user) cancel_request_for(user) notifications_sent = AccountResetService.grant_tokens_and_send_notifications diff --git a/spec/services/idv/profile_maker_spec.rb b/spec/services/idv/profile_maker_spec.rb index 8c40bfd19d9..af800348125 100644 --- a/spec/services/idv/profile_maker_spec.rb +++ b/spec/services/idv/profile_maker_spec.rb @@ -5,14 +5,12 @@ let(:applicant) { { first_name: 'Some', last_name: 'One' } } let(:user) { create(:user, :signed_up) } let(:user_password) { user.password } - let(:phone_confirmed) { false } subject do described_class.new( applicant: applicant, user: user, - user_password: user_password, - phone_confirmed: phone_confirmed + user_password: user_password ) end @@ -28,15 +26,5 @@ expect(pii).to be_a Pii::Attributes expect(pii.first_name).to eq 'Some' end - - context 'when phone_confirmed is true' do - let(:phone_confirmed) { true } - it { expect(subject.save_profile.phone_confirmed).to eq(true) } - end - - context 'when phone_confirmed is false' do - let(:phone_confirmed) { false } - it { expect(subject.save_profile.phone_confirmed).to eq(false) } - end end end diff --git a/spec/services/idv/proofer_spec.rb b/spec/services/idv/proofer_spec.rb index 93c0d841666..64c9d979a10 100644 --- a/spec/services/idv/proofer_spec.rb +++ b/spec/services/idv/proofer_spec.rb @@ -1,20 +1,54 @@ require 'rails_helper' describe Idv::Proofer do - describe '.attribute?' do - subject { described_class.attribute?(attribute) } + let(:resolution_dummy) do + class_double('Proofer::Base', vendor_name: 'dummy:resolution', stage: :resolution) + end + let(:state_id_dummy) do + class_double('Proofer::Base', vendor_name: 'dummy:state_id', stage: :state_id) + end + let(:address_dummy) do + class_double('Proofer::Base', vendor_name: 'dummy:address', stage: :address) + end + let(:dummy_vendors) { [resolution_dummy, state_id_dummy, address_dummy] } + + let(:proofer_vendors) { '["dummy:resolution", "dummy:state_id", "dummy:address"]' } + let(:proofer_mock_fallback) { 'false' } + + before do + allow(Figaro.env).to receive(:proofer_vendors). + and_return(proofer_vendors) + allow(Figaro.env).to receive(:proofer_mock_fallback). + and_return(proofer_mock_fallback) + + original_descendants = Proofer::Base.descendants + allow(Proofer::Base).to receive(:descendants).and_return( + original_descendants + dummy_vendors + ) + + subject.instance_variable_set(:@vendors, nil) + end + + after do + # This is necessary to prevent mocks created in these examples from leaking + # out with the memoized vendors value + subject.instance_variable_set(:@vendors, nil) + end + + subject { described_class } + describe '.attribute?' do context 'when the attribute exists' do context 'and is passed as a string' do let(:attribute) { 'last_name' } - it { is_expected.to eq(true) } + it { expect(subject.attribute?(attribute)).to eq(true) } end context 'and is passed as a symbol' do let(:attribute) { :last_name } - it { is_expected.to eq(true) } + it { expect(subject.attribute?(attribute)).to eq(true) } end end @@ -22,241 +56,108 @@ context 'and is passed as a string' do let(:attribute) { 'fooobar' } - it { is_expected.to eq(false) } + it { expect(subject.attribute?(attribute)).to eq(false) } end context 'and is passed as a symbol' do let(:attribute) { :fooobar } - it { is_expected.to eq(false) } + it { expect(subject.attribute?(attribute)).to eq(false) } end end end - describe '.loaded_vendors' do - subject { described_class.send(:loaded_vendors) } - - it 'returns all of the subclasses of Proofer::Base' do - subclasses = ['foo'] - expect(::Proofer::Base).to receive(:descendants).and_return(subclasses) - expect(subject).to eq(subclasses) - end - end - - describe '.available_vendors' do - subject { described_class.send(:available_vendors, configured_vendors, vendors) } - - let(:vendors) do - [ - class_double('Proofer::Base', vendor_name: 'foo'), - class_double('Proofer::Base', vendor_name: 'baz'), - ] - end - - let(:configured_vendors) { %w[foo bar] } - - it 'selects only the vendors that have been configured' do - available_vendors = [vendors.first] - expect(subject).to eq(available_vendors) - end - end - - describe '.require_mock_vendors' do - subject { described_class.send(:require_mock_vendors) } + describe '.get_vendor' do + context 'with mock proofers enabled' do + let(:proofer_mock_fallback) { 'true' } - it 'requires all of the mock vendors' do - Dir[Rails.root.join('lib', 'proofer_mocks', '*')].each do |file| - expect_any_instance_of(Object).to receive(:require).with(file) + context 'with a vendor configured for the state' do + it 'returns the vendor' do + expect(subject.get_vendor(:resolution)).to eq(resolution_dummy) + expect(subject.get_vendor(:state_id)).to eq(state_id_dummy) + expect(subject.get_vendor(:address)).to eq(address_dummy) + end end - subject - end - end - - describe '.assign_vendors' do - subject { described_class.send(:assign_vendors, stages, external_vendors, mock_vendors) } - - let(:stages) { %i[resolution state_id address] } - - let(:external_vendors) do - [ - class_double('Proofer::Base', stage: :resolution), - class_double('Proofer::Base', stage: :foo), - ] - end - - let(:mock_vendors) do - [ - class_double('Proofer::Base', stage: :resolution), - class_double('Proofer::Base', stage: 'state_id'), - class_double('Proofer::Base', stage: :baz), - ] - end - - it 'maps stages to vendors, falling back to mock vendors' do - assigned_vendors = { - resolution: external_vendors.first, - state_id: mock_vendors.second, - } - expect(subject).to eq(assigned_vendors) - end - end - - describe '.stage_vendor' do - subject { described_class.send(:stage_vendor, stage, vendors) } - - let(:stage) { :foo } - - context 'when stage is a string' do - let(:vendors) do - [ - class_double('Proofer::Base', stage: :resolution), - class_double('Proofer::Base', stage: 'foo'), - ] - end + context 'without a vendor configured for the state' do + let(:proofer_vendors) { '["dummy:state_id"]' } - it 'selects the vendor for the stage' do - expect(subject).to eq(vendors.second) + it 'returns a mock vendor' do + expect(subject.get_vendor(:resolution)).to eq(ResolutionMock) + expect(subject.get_vendor(:state_id)).to eq(state_id_dummy) + expect(subject.get_vendor(:address)).to eq(AddressMock) + end end - end - context 'when stage is a symbol' do - let(:vendors) do - [ - class_double('Proofer::Base', stage: :resolution), - class_double('Proofer::Base', stage: :foo), - ] - end + context 'without a proofer vendor configuration' do + let(:proofer_vendors) { nil } - it 'selects the vendor for the stage' do - expect(subject).to eq(vendors.second) + it 'returns all mock proofers' do + expect(subject.get_vendor(:resolution)).to eq(ResolutionMock) + expect(subject.get_vendor(:state_id)).to eq(StateIdMock) + expect(subject.get_vendor(:address)).to eq(AddressMock) + end end end - context 'when no vendor exists' do - let(:vendors) do - [ - class_double('Proofer::Base', stage: :resolution), - ] - end + context 'without mock proofers enabled' do + let(:proofer_mock_fallback) { 'false' } - it 'is nil' do - expect(subject).to be_nil + context 'with a vendor configured for the state' do + it 'returns the vendor' do + expect(subject.get_vendor(:resolution)).to eq(resolution_dummy) + expect(subject.get_vendor(:state_id)).to eq(state_id_dummy) + expect(subject.get_vendor(:address)).to eq(address_dummy) + end end - end - end - - describe '.validate_vendors' do - subject { described_class.send(:validate_vendors, stages, vendors) } - - let(:stages) { %i[foo] } - context 'when there are vendors for all stages' do - let(:vendors) { { foo: class_double('Proofer::Base') } } + context 'without a vendor configured for the state' do + let(:proofer_vendors) { '["dummy:state_id"]' } - it 'does not raise an error' do - expect { subject }.not_to raise_error + it 'returns nil' do + expect(subject.get_vendor(:resolution)).to eq(nil) + expect(subject.get_vendor(:state_id)).to eq(state_id_dummy) + expect(subject.get_vendor(:address)).to eq(nil) + end end - end - context 'when there are stages without vendors' do - let(:vendors) { { bar: class_double('Proofer::Base') } } + context 'without a proofer vendor configuration' do + let(:proofer_vendors) { nil } - it 'does raises an error' do - expect { subject }.to raise_error('No proofer vendor configured for stage(s): foo') + it 'returns nil' do + expect(subject.get_vendor(:resolution)).to eq(nil) + expect(subject.get_vendor(:state_id)).to eq(nil) + expect(subject.get_vendor(:address)).to eq(nil) + end end end end - describe '.configure_vendors' do - subject { described_class.configure_vendors(stages, config) } - - let(:stages) { %i[foo] } - - let(:config) { double } - - let(:configured_vendors) { %w[vendor1 vendor2] } - - let(:loaded_vendors) do - [ - class_double('Proofer::Base', stage: :foo, vendor_name: 'vendor3'), - class_double('Proofer::Base', stage: :foo, vendor_name: 'vendor1'), - class_double('Proofer::Base', stage: :bar, vendor_name: 'vendor2'), - ] - end - - let(:mock_vendors) do - [ - class_double('Proofer::Base', stage: :foo), - class_double('Proofer::Base', stage: :baz), - ] - end + describe '.validate_vendors!' do + let(:proofer_mock_fallback) { 'false' } - before do - expect(config).to receive(:vendors).and_return(configured_vendors) - end - - context 'default configuration' do - before do - expect(config).to receive(:mock_fallback).and_return(false) - expect(config).to receive(:raise_on_missing_proofers).and_return(true) - expect(described_class). - to receive(:loaded_vendors).and_return(loaded_vendors, loaded_vendors) - end - - context 'when a stage is missing an external vendor' do - let(:stages) { %i[foo baz] } - - it 'raises' do - expect { subject }.to raise_error('No proofer vendor configured for stage(s): baz') - end - end - - context 'when all stages have vendors' do - it 'maps the vendors, ignoring non-configured ones' do - expect(subject).to eq(foo: loaded_vendors.second) - end + context 'with vendors configured for each stage' do + it 'does not raise' do + expect { described_class.validate_vendors! }.to_not raise_error end end - context 'when mock_fallback is enabled' do - before do - expect(config).to receive(:mock_fallback).and_return(true) - expect(config).to receive(:raise_on_missing_proofers).and_return(true) - expect(described_class).to receive(:loaded_vendors).and_return(loaded_vendors, mock_vendors) - end + context 'without vendors configured for each stage' do + let(:proofer_vendors) { '["dummy:state_id"]' } - context 'when a stage is missing an external vendor' do - let(:stages) { %i[foo baz] } - - it 'does not raise' do - expect { subject }.not_to raise_error - end - - it 'returns the mapped vendors with the mock fallback' do - expect(subject).to eq(foo: loaded_vendors.second, baz: mock_vendors.second) - end + it 'does raise' do + expect { described_class.validate_vendors! }.to raise_error( + RuntimeError, 'No proofer vendor configured for stage(s): resolution, address' + ) end end - context 'when raise_on_missing_proofers is disabled' do - before do - expect(config).to receive(:mock_fallback).and_return(false) - expect(config).to receive(:raise_on_missing_proofers).and_return(false) - expect(described_class). - to receive(:loaded_vendors).and_return(loaded_vendors, loaded_vendors) - end + context 'without vendors configured but with mock vendors enabled' do + let(:proofer_vendors) { '["dummy:state_id"]' } + let(:proofer_mock_fallback) { 'true' } - context 'when a stage is missing an external vendor' do - let(:stages) { %i[foo baz] } - - it 'does not raise' do - expect { subject }.not_to raise_error - end - - it 'returns the mapped vendors missing the stage' do - expect(subject).to eq(foo: loaded_vendors.second) - end + it 'does not raise' do + expect { described_class.validate_vendors! }.to_not raise_error end end end diff --git a/spec/services/otp_rate_limiter_spec.rb b/spec/services/otp_rate_limiter_spec.rb index 481448a53ac..0a3c37268e4 100644 --- a/spec/services/otp_rate_limiter_spec.rb +++ b/spec/services/otp_rate_limiter_spec.rb @@ -2,8 +2,11 @@ RSpec.describe OtpRateLimiter do let(:current_user) { build(:user, :with_phone) } - subject(:otp_rate_limiter) { OtpRateLimiter.new(phone: current_user.phone, user: current_user) } - let(:phone_fingerprint) { Pii::Fingerprinter.fingerprint(current_user.phone) } + subject(:otp_rate_limiter) do + OtpRateLimiter.new(phone: current_user.phone_configuration.phone, user: current_user) + end + + let(:phone_fingerprint) { Pii::Fingerprinter.fingerprint(current_user.phone_configuration.phone) } let(:rate_limited_phone) { OtpRequestsTracker.find_by(phone_fingerprint: phone_fingerprint) } describe '#exceeded_otp_send_limit?' do @@ -24,7 +27,7 @@ describe '#increment' do it 'updates otp_last_sent_at' do - tracker = OtpRequestsTracker.find_or_create_with_phone(current_user.phone) + tracker = OtpRequestsTracker.find_or_create_with_phone(current_user.phone_configuration.phone) old_otp_last_sent_at = tracker.reload.otp_last_sent_at otp_rate_limiter.increment new_otp_last_sent_at = tracker.reload.otp_last_sent_at @@ -52,7 +55,7 @@ otp_rate_limiter.lock_out_user - expect(current_user.second_factor_locked_at.to_i).to eq(Time.zone.now.to_i) + expect(current_user.second_factor_locked_at).to be_within(1.second).of(Time.zone.now) end end end diff --git a/spec/services/parse_controller_from_referer_spec.rb b/spec/services/parse_controller_from_referer_spec.rb index 680825808b4..e39533bfc85 100644 --- a/spec/services/parse_controller_from_referer_spec.rb +++ b/spec/services/parse_controller_from_referer_spec.rb @@ -5,16 +5,18 @@ context 'when the referer is nil' do it 'returns "no referer" string' do parser = ParseControllerFromReferer.new(nil) + result = { request_came_from: 'no referer' } - expect(parser.call).to eq 'no referer' + expect(parser.call).to eq result end end context 'when the referer is present' do it 'returns the corresponding controller and action' do parser = ParseControllerFromReferer.new('http://example.com/') + result = { request_came_from: 'users/sessions#new' } - expect(parser.call).to eq 'users/sessions#new' + expect(parser.call).to eq result end end end diff --git a/spec/services/phone_number_capabilities_spec.rb b/spec/services/phone_number_capabilities_spec.rb index 4b9afdc5887..10475041cef 100644 --- a/spec/services/phone_number_capabilities_spec.rb +++ b/spec/services/phone_number_capabilities_spec.rb @@ -9,59 +9,49 @@ it { expect(subject.sms_only?).to eq(false) } end - context 'voice is not supported for the area code' do + context 'Bahamas number' do let(:phone) { '+1 (242) 327-0143' } it { expect(subject.sms_only?).to eq(true) } end + context 'Bermuda number' do + let(:phone) { '+1 (441) 295-9644' } + it { expect(subject.sms_only?).to eq(true) } + end + context 'voice is supported for the international code' do let(:phone) { '+55 (555) 555-5000' } # pending while international voice is disabled for all international codes xit { expect(subject.sms_only?).to eq(false) } end - context 'voice is not supported for the international code' do - let(:phone) { '+212 1234 12345' } + context 'Morocco number' do + let(:phone) { '+212 661-289325' } + it { expect(subject.sms_only?).to eq(true) } + end + + context "phonelib returns nil or a 2-letter country code that doesn't match our YAML" do + let(:phone) { '703-555-1212' } it { expect(subject.sms_only?).to eq(true) } end end describe '#unsupported_location' do - it 'returns the name of the unsupported area code location' do + it 'returns the name of the unsupported country (Bahamas)' do locality = PhoneNumberCapabilities.new('+1 (242) 327-0143').unsupported_location expect(locality).to eq('Bahamas') end - it 'returns the name of the unsupported international code location' do - locality = PhoneNumberCapabilities.new('+355 1234 12345').unsupported_location - expect(locality).to eq('Albania') + it 'returns the name of the unsupported country (Bermuda)' do + locality = PhoneNumberCapabilities.new('+1 (441) 295-9644').unsupported_location + expect(locality).to eq('Bermuda') end - end - describe 'list of unsupported area codes' do - it 'is up to date' do - unsupported_area_codes = { - '264' => 'Anguilla', - '268' => 'Antigua and Barbuda', - '242' => 'Bahamas', - '246' => 'Barbados', - '441' => 'Bermuda', - '284' => 'British Virgin Islands', - '345' => 'Cayman Islands', - '767' => 'Dominica', - '809' => 'Dominican Republic', - '829' => 'Dominican Republic', - '849' => 'Dominican Republic', - '473' => 'Grenada', - '876' => 'Jamaica', - '664' => 'Montserrat', - '869' => 'Saint Kitts and Nevis', - '758' => 'Saint Lucia', - '784' => 'Saint Vincent Grenadines', - '868' => 'Trinidad and Tobago', - '649' => 'Turks and Caicos Islands', - } - expect(PhoneNumberCapabilities::VOICE_UNSUPPORTED_US_AREA_CODES).to eq unsupported_area_codes + context 'phonelib returns nil' do + it 'returns nil' do + locality = PhoneNumberCapabilities.new('703-555-1212').unsupported_location + expect(locality).to be_nil + end end end end diff --git a/spec/services/piv_cac_service_spec.rb b/spec/services/piv_cac_service_spec.rb index b9127b7599b..5222cb9afc4 100644 --- a/spec/services/piv_cac_service_spec.rb +++ b/spec/services/piv_cac_service_spec.rb @@ -161,4 +161,103 @@ end end end + + describe '#piv_cac_available_for_agency?' do + let(:subject) { PivCacService.piv_cac_available_for_agency?('foo', 'foo@example.com') } + + context 'with an agency not encouraged to use piv/cac for anyone' do + before(:each) do + allow(PivCacService).to receive(:available_for_agency?).and_return(false) + allow(PivCacService).to receive(:available_for_email?).and_return(false) + end + + it { expect(subject).to be_falsey } + end + + context 'with an agency encouraged to use piv/cac for everyone' do + before(:each) do + allow(PivCacService).to receive(:available_for_agency?).and_return(true) + allow(PivCacService).to receive(:available_for_email?).and_return(false) + end + + it { expect(subject).to eq true } + end + + context 'with an agency encouraged to use piv/cac for certain email domains' do + before(:each) do + allow(PivCacService).to receive(:available_for_agency?).and_return(false) + allow(PivCacService).to receive(:available_for_email?).and_return(true) + end + + it { expect(subject).to eq true } + end + end + + describe '#available_for_agency?' do + let(:subject) { PivCacService.send(:available_for_agency?, 'foo') } + + context 'with the agency not configured to be available' do + before(:each) do + allow(FeatureManagement).to receive(:piv_cac_enabled?).and_return(true) + allow(Figaro.env).to receive(:piv_cac_agencies).and_return('["bar"]') + end + + it { expect(subject).to be_falsey } + end + + context 'with the agency configured to be available' do + before(:each) do + allow(FeatureManagement).to receive(:piv_cac_enabled?).and_return(true) + allow(Figaro.env).to receive(:piv_cac_agencies).and_return('["bar","foo"]') + end + + it { expect(subject).to eq true } + end + end + + describe '#available_for_email?' do + let(:subject) { PivCacService.send(:available_for_email?, 'foo', 'foo@bar.example.com') } + + context 'with the agency not configured to be available' do + before(:each) do + allow(FeatureManagement).to receive(:piv_cac_enabled?).and_return(true) + allow(Figaro.env).to receive(:piv_cac_agencies_scoped_by_email).and_return('["bar"]') + end + + it { expect(subject).to be_falsey } + end + + context 'with the agency configured to be available' do + before(:each) do + allow(FeatureManagement).to receive(:piv_cac_enabled?).and_return(true) + allow(Figaro.env).to receive(:piv_cac_agencies_scoped_by_email).and_return('["bar","foo"]') + end + + context 'but not in the right email domain' do + before(:each) do + allow(Figaro.env).to receive(:piv_cac_email_domains).and_return( + '["example.com", "baz.example.com"]' + ) + end + + it { expect(subject).to be_falsey } + end + + context 'in the right full domain' do + before(:each) do + allow(Figaro.env).to receive(:piv_cac_email_domains).and_return('["bar.example.com"]') + end + + it { expect(subject).to eq true } + end + + context 'in the right subdomain' do + before(:each) do + allow(Figaro.env).to receive(:piv_cac_email_domains).and_return('[".example.com"]') + end + + it { expect(subject).to eq true } + end + end + end end diff --git a/spec/services/populate_phone_configurations_table_spec.rb b/spec/services/populate_phone_configurations_table_spec.rb index da4f3e9c384..19bd60bbffd 100644 --- a/spec/services/populate_phone_configurations_table_spec.rb +++ b/spec/services/populate_phone_configurations_table_spec.rb @@ -22,6 +22,11 @@ user.reload end + it 'migrates without decrypting and re-encrypting' do + expect(EncryptedAttribute).to_not receive(:new) + subject.call + end + it 'migrates the phone' do subject.call configuration = user.reload.phone_configuration diff --git a/spec/services/remember_device_cookie_spec.rb b/spec/services/remember_device_cookie_spec.rb index 26f7617e519..98bc69a4c02 100644 --- a/spec/services/remember_device_cookie_spec.rb +++ b/spec/services/remember_device_cookie_spec.rb @@ -2,7 +2,7 @@ describe RememberDeviceCookie do let(:phone_confirmed_at) { 90.days.ago } - let(:user) { create(:user, phone_confirmed_at: phone_confirmed_at) } + let(:user) { create(:user, :with_phone, phone_confirmed_at: phone_confirmed_at) } let(:created_at) { Time.zone.now } subject { described_class.new(user_id: user.id, created_at: created_at) } diff --git a/spec/services/request_key_manager_spec.rb b/spec/services/request_key_manager_spec.rb index a06380eec8b..0f2f64e2cdc 100644 --- a/spec/services/request_key_manager_spec.rb +++ b/spec/services/request_key_manager_spec.rb @@ -1,14 +1,6 @@ require 'rails_helper' describe RequestKeyManager do - describe '.equifax_ssh_key' do - it 'initializes' do - ssh_key = described_class.equifax_ssh_key - - expect(ssh_key).to be_a OpenSSL::PKey::RSA - end - end - describe '.private_key' do it 'initializes' do ssh_key = described_class.private_key diff --git a/spec/support/account_reset_helper.rb b/spec/support/account_reset_helper.rb index ff32b8695ce..9176d3078c1 100644 --- a/spec/support/account_reset_helper.rb +++ b/spec/support/account_reset_helper.rb @@ -1,6 +1,6 @@ module AccountResetHelper def create_account_reset_request_for(user) - AccountResetService.new(user).create_request + AccountReset::CreateRequest.new(user).call account_reset_request = AccountResetRequest.find_by(user_id: user.id) account_reset_request.request_token end diff --git a/spec/support/features/idv_step_helper.rb b/spec/support/features/idv_step_helper.rb index c1cf1bd96b6..be7ad433e6d 100644 --- a/spec/support/features/idv_step_helper.rb +++ b/spec/support/features/idv_step_helper.rb @@ -63,7 +63,7 @@ def complete_idv_steps_before_phone_otp_verification_step(user = user_with_2fa) def complete_idv_steps_with_phone_before_review_step(user = user_with_2fa) complete_idv_steps_before_phone_step(user) - fill_out_phone_form_ok(user.phone) + fill_out_phone_form_ok(user.phone_configuration.phone) click_idv_continue end diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index a7482c619dc..6decb223a87 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -81,7 +81,7 @@ def sign_in_before_2fa(user = create(:user)) allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) login_as(user, scope: :user, run_callbacks: false) - if user.phone.present? + if user.phone_configuration.present? Warden.on_next_request do |proxy| session = proxy.env['rack.session'] session['warden.user.user.session'] = {} diff --git a/spec/support/idv_examples/usps_otp_verification_step.rb b/spec/support/idv_examples/usps_otp_verification_step.rb index a59e498948b..0b48d6affcb 100644 --- a/spec/support/idv_examples/usps_otp_verification_step.rb +++ b/spec/support/idv_examples/usps_otp_verification_step.rb @@ -4,7 +4,6 @@ create( :profile, deactivation_reason: :verification_pending, - phone_confirmed: false, pii: { ssn: '123-45-6789', dob: '1970-01-01' } ) end diff --git a/spec/support/shared_examples/remember_device.rb b/spec/support/shared_examples/remember_device.rb index 24c78a0b7c1..dbfa29aafd4 100644 --- a/spec/support/shared_examples/remember_device.rb +++ b/spec/support/shared_examples/remember_device.rb @@ -83,3 +83,36 @@ expect(current_url).to start_with('http://localhost:7654/auth/result') end end + +shared_examples 'remember device after being idle on sign in page' do + it 'redirects to the OIDC SP even though session is deleted' do + # We want to simulate a user that has already visited an OIDC SP and that + # has checked "remember me for 30 days", such that the next URL the app will + # redirect to after signing in with email and password is the SP redirect + # URI. + user = remember_device_and_sign_out_user + IdentityLinker.new( + user, 'urn:gov:gsa:openidconnect:sp:server' + ).link_identity(verified_attributes: %w[email]) + + visit_idp_from_sp_with_loa1(:oidc) + request_id = ServiceProviderRequest.last.uuid + click_link t('links.sign_in') + + Timecop.travel(Devise.timeout_in + 1.minute) do + # Simulate being idle on the sign in page long enough for the session to + # be deleted from Redis, but since Redis doesn't respect Timecop, we need + # to expire the session manually. + session_store.send(:destroy_session_from_sid, session_cookie.value) + # Simulate refreshing the page with JS to avoid a CSRF error + visit new_user_session_url(request_id: request_id) + + expect(page.response_headers['Content-Security-Policy']). + to(include('form-action \'self\' http://localhost:7654/auth/result')) + + fill_in_credentials_and_submit(user.email, user.password) + + expect(current_url).to start_with('http://localhost:7654/auth/result') + end + end +end diff --git a/spec/support/shared_examples/sign_in.rb b/spec/support/shared_examples/sign_in.rb index 0dd1cacce63..9c0353c20af 100644 --- a/spec/support/shared_examples/sign_in.rb +++ b/spec/support/shared_examples/sign_in.rb @@ -30,22 +30,15 @@ shared_examples 'signing in as LOA1 with personal key' do |sp| it 'redirects to the SP after acknowledging new personal key', email: true do - user = create_loa1_account_go_back_to_sp_and_sign_out(sp) - old_personal_key = PersonalKeyGenerator.new(user).create - visit_idp_from_sp_with_loa1(sp) - click_link t('links.sign_in') - fill_in_credentials_and_submit(user.email, 'Val!d Pass w0rd') - choose_another_security_option('personal_key') - enter_personal_key(personal_key: old_personal_key) - click_submit_default + loa1_sign_in_with_personal_key_goes_to_sp(sp) + end +end - expect(page).to have_current_path(manage_personal_key_path) - if sp == :oidc - expect(page.response_headers['Content-Security-Policy']). - to(include('form-action \'self\' http://localhost:7654')) - end +shared_examples 'visiting 2fa when fully authenticated' do |sp| + it 'redirects to SP after visiting a 2fa screen when fully authenticated', email: true do + loa1_sign_in_with_personal_key_goes_to_sp(sp) - click_acknowledge_personal_key + visit login_two_factor_options_path expect(current_url).to eq @saml_authn_request if sp == :saml @@ -207,3 +200,30 @@ def personal_key_for_loa3_user(user, pii) personal_key end + +def loa1_sign_in_with_personal_key_goes_to_sp(sp) + user = create_loa1_account_go_back_to_sp_and_sign_out(sp) + old_personal_key = PersonalKeyGenerator.new(user).create + visit_idp_from_sp_with_loa1(sp) + click_link t('links.sign_in') + fill_in_credentials_and_submit(user.email, 'Val!d Pass w0rd') + choose_another_security_option('personal_key') + enter_personal_key(personal_key: old_personal_key) + click_submit_default + + expect(page).to have_current_path(manage_personal_key_path) + if sp == :oidc + expect(page.response_headers['Content-Security-Policy']). + to(include('form-action \'self\' http://localhost:7654')) + end + + click_acknowledge_personal_key + + expect(current_url).to eq @saml_authn_request if sp == :saml + + return unless sp == :oidc + + redirect_uri = URI(current_url) + + expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') +end diff --git a/spec/support/shared_examples_for_phone_validation.rb b/spec/support/shared_examples_for_phone_validation.rb index f77c04d4402..cf2c732b093 100644 --- a/spec/support/shared_examples_for_phone_validation.rb +++ b/spec/support/shared_examples_for_phone_validation.rb @@ -15,9 +15,11 @@ it 'is valid' do second_user = build_stubbed(:user, :signed_up, phone: '+1 (202) 555-1213') allow(User).to receive(:exists?).with(email: 'new@gmail.com').and_return(false) - allow(User).to receive(:exists?).with(phone: second_user.phone).and_return(true) + allow(User).to receive(:exists?).with( + phone: second_user.phone_configuration.phone + ).and_return(true) - params[:phone] = second_user.phone + params[:phone] = second_user.phone_configuration.phone result = subject.submit(params) expect(result).to be_kind_of(FormResponse) @@ -36,7 +38,8 @@ context 'when phone is same as current user' do it 'is valid' do user.phone = '+1 (703) 500-5000' - params[:phone] = user.phone + user.phone_configuration.phone = '+1 (703) 500-5000' + params[:phone] = user.phone_configuration.phone result = subject.submit(params) expect(result).to be_kind_of(FormResponse) diff --git a/spec/support/sp_auth_helper.rb b/spec/support/sp_auth_helper.rb index aba47c4c6ea..ed88e19f435 100644 --- a/spec/support/sp_auth_helper.rb +++ b/spec/support/sp_auth_helper.rb @@ -27,7 +27,7 @@ def create_loa3_account_go_back_to_sp_and_sign_out(sp) fill_out_idv_form_ok click_idv_continue click_idv_continue - fill_out_phone_form_ok(user.phone) + fill_out_phone_form_ok(user.phone_configuration.phone) click_idv_continue fill_in :user_password, with: user.password click_continue diff --git a/spec/view_models/account_show_spec.rb b/spec/view_models/account_show_spec.rb index 9c2ac0e388a..cca373d355a 100644 --- a/spec/view_models/account_show_spec.rb +++ b/spec/view_models/account_show_spec.rb @@ -77,8 +77,7 @@ context 'user needs profile usps verification' do it 'returns the accounts/pending_profile_usps partial' do user = User.new - allow(user).to receive(:needs_profile_usps_verification?).and_return(true) - allow(user).to receive(:needs_profile_phone_verification?).and_return(false) + allow(user).to receive(:pending_profile_requires_verification?).and_return(true) profile_index = AccountShow.new( decrypted_pii: {}, personal_key: 'foo', decorated_user: user ) @@ -87,22 +86,10 @@ end end - context 'user needs profile phone verification' do - it 'returns the accounts/pending_profile_phone partial' do - user = User.new - allow(user).to receive(:needs_profile_usps_verification?).and_return(false) - allow(user).to receive(:needs_profile_phone_verification?).and_return(true) - profile_index = AccountShow.new(decrypted_pii: {}, personal_key: '', decorated_user: user) - - expect(profile_index.pending_profile_partial).to eq 'accounts/pending_profile_phone' - end - end - context 'user does not need profile verification' do it 'returns the shared/null partial' do user = User.new - allow(user).to receive(:needs_profile_phone_verification?).and_return(false) - allow(user).to receive(:needs_profile_usps_verification?).and_return(false) + allow(user).to receive(:pending_profile_requires_verification?).and_return(false) profile_index = AccountShow.new(decrypted_pii: {}, personal_key: '', decorated_user: user) expect(profile_index.pending_profile_partial).to eq 'shared/null' diff --git a/spec/views/account_reset/confirm_request/show.html.slim_spec.rb b/spec/views/account_reset/confirm_request/show.html.slim_spec.rb index 471a851cd0d..74321b6fa50 100644 --- a/spec/views/account_reset/confirm_request/show.html.slim_spec.rb +++ b/spec/views/account_reset/confirm_request/show.html.slim_spec.rb @@ -3,6 +3,7 @@ describe 'account_reset/confirm_request/show.html.slim' do before do allow(view).to receive(:email).and_return('foo@bar.com') + allow(view).to receive(:sms_phone).and_return(true) end it 'has a localized title' do @@ -10,13 +11,4 @@ render end - - it 'contains the user email' do - email = 'foo@bar.com' - session[:email] = email - - render - - expect(rendered).to have_content(email) - end end diff --git a/spec/views/layouts/application.html.slim_spec.rb b/spec/views/layouts/application.html.slim_spec.rb index 8fbb81d9206..f09d58e4403 100644 --- a/spec/views/layouts/application.html.slim_spec.rb +++ b/spec/views/layouts/application.html.slim_spec.rb @@ -29,6 +29,24 @@ end end + context 'when FeatureManagement.fake_banner_mode? is true' do + it 'displays the fake banner' do + allow(FeatureManagement).to receive(:fake_banner_mode?).and_return(true) + render + + expect(rendered).to have_content('FAKE') + end + end + + context 'when FeatureManagement.fake_banner_mode? is false' do + it 'does not display the fake banner' do + allow(FeatureManagement).to receive(:fake_banner_mode?).and_return(false) + render + + expect(rendered).to_not have_content('FAKE') + end + end + context 'when i18n mode enabled' do before do allow(FeatureManagement).to receive(:enable_i18n_mode?).and_return(true) diff --git a/spec/views/two_factor_authentication/totp_verification/show.html.slim_spec.rb b/spec/views/two_factor_authentication/totp_verification/show.html.slim_spec.rb index ec5e6509535..04d24969a7d 100644 --- a/spec/views/two_factor_authentication/totp_verification/show.html.slim_spec.rb +++ b/spec/views/two_factor_authentication/totp_verification/show.html.slim_spec.rb @@ -6,7 +6,7 @@ attributes_for(:generic_otp_presenter).merge( two_factor_authentication_method: 'authenticator', user_email: view.current_user.email, - phone_enabled: user.phone_enabled? + phone_enabled: user.phone_configuration&.mfa_enabled? ) end diff --git a/yarn.lock b/yarn.lock index b42d18b1ff2..5cdcc9f5abb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -876,6 +876,12 @@ balanced-match@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/balanced-match/-/balanced-match-1.0.0.tgz#89b4d199ab2bee49de164ea02b89ce462d71b767" +base32-crockford-browser@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/base32-crockford-browser/-/base32-crockford-browser-1.0.0.tgz#3684970a5826ba1430f01e719cf923b00d773dd8" + dependencies: + optimist ">=0.1.0" + base64-js@^1.0.2: version "1.2.1" resolved "https://registry.yarnpkg.com/base64-js/-/base64-js-1.2.1.tgz#a91947da1f4a516ea38e5b4ec0ec3773675e0886" @@ -4099,6 +4105,10 @@ minimist@^1.1.3, minimist@^1.2.0: version "1.2.0" resolved "https://registry.yarnpkg.com/minimist/-/minimist-1.2.0.tgz#a35008b20f41383eec1fb914f4cd5df79a264284" +minimist@~0.0.1: + version "0.0.10" + resolved "https://registry.yarnpkg.com/minimist/-/minimist-0.0.10.tgz#de3f98543dbf96082be48ad1a0c7cda836301dcf" + mississippi@^1.3.0: version "1.3.1" resolved "https://registry.yarnpkg.com/mississippi/-/mississippi-1.3.1.tgz#2a8bb465e86550ac8b36a7b6f45599171d78671e" @@ -4465,6 +4475,13 @@ opn@^5.1.0: dependencies: is-wsl "^1.1.0" +optimist@>=0.1.0: + version "0.6.1" + resolved "https://registry.yarnpkg.com/optimist/-/optimist-0.6.1.tgz#da3ea74686fa21a19a111c326e90eb15a0196686" + dependencies: + minimist "~0.0.1" + wordwrap "~0.0.2" + optimize-css-assets-webpack-plugin@^3.2.0: version "3.2.0" resolved "https://registry.yarnpkg.com/optimize-css-assets-webpack-plugin/-/optimize-css-assets-webpack-plugin-3.2.0.tgz#09a40c4cefde1dd0142444a873c56aa29eb18e6f" @@ -6851,6 +6868,10 @@ wordwrap@0.0.2: version "0.0.2" resolved "https://registry.yarnpkg.com/wordwrap/-/wordwrap-0.0.2.tgz#b79669bb42ecb409f83d583cad52ca17eaa1643f" +wordwrap@~0.0.2: + version "0.0.3" + resolved "https://registry.yarnpkg.com/wordwrap/-/wordwrap-0.0.3.tgz#a3d5da6cd5c0bc0008d37234bbaf1bed63059107" + wordwrap@~1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/wordwrap/-/wordwrap-1.0.0.tgz#27584810891456a4171c8d0226441ade90cbcaeb"