diff --git a/Gemfile b/Gemfile index fe5eb36727d..79060145dee 100644 --- a/Gemfile +++ b/Gemfile @@ -98,7 +98,6 @@ group :development, :test do gem 'i18n-tasks', '~> 1.0' gem 'knapsack' gem 'nokogiri', '~> 1.14.0' - gem 'parallel_tests', '~> 3.8.0' gem 'pg_query', require: false gem 'pry-byebug' gem 'pry-doc' diff --git a/Gemfile.lock b/Gemfile.lock index d6f072de731..108a13e8f7d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -435,8 +435,6 @@ GEM openssl (> 2.0, < 3.1) orm_adapter (0.5.0) parallel (1.22.1) - parallel_tests (3.8.1) - parallel parser (3.2.0.0) ast (~> 2.4.1) pg (1.4.5) @@ -776,7 +774,6 @@ DEPENDENCIES newrelic_rpm (~> 8.0) nokogiri (~> 1.14.0) octokit (>= 4.25.0) - parallel_tests (~> 3.8.0) pg pg_query phonelib diff --git a/Makefile b/Makefile index bce9ab58f6c..6cfceba4535 100644 --- a/Makefile +++ b/Makefile @@ -133,8 +133,8 @@ browsers.json: yarn.lock .browserslistrc ## Generates browsers.json browser supp yarn generate-browsers-json test: export RAILS_ENV := test -test: $(CONFIG) ## Runs RSpec and yarn tests in parallel - bundle exec rake parallel:spec && yarn test +test: $(CONFIG) ## Runs RSpec and yarn tests + bundle exec rspec && yarn test test_serial: export RAILS_ENV := test test_serial: $(CONFIG) ## Runs RSpec and yarn tests serially diff --git a/app/assets/stylesheets/components/_alert.scss b/app/assets/stylesheets/components/_alert.scss index 89cc10bb464..9fe7191669d 100644 --- a/app/assets/stylesheets/components/_alert.scss +++ b/app/assets/stylesheets/components/_alert.scss @@ -23,3 +23,7 @@ transform: translateY(-50%); } } + +.usa-alert__text > p:last-child { + margin-bottom: 0; +} diff --git a/app/components/vendor_outage_alert_component.rb b/app/components/vendor_outage_alert_component.rb index dbf7a21d104..822ff103efa 100644 --- a/app/components/vendor_outage_alert_component.rb +++ b/app/components/vendor_outage_alert_component.rb @@ -37,6 +37,6 @@ def outages end def vendor_status - @vendor_status ||= VendorStatus.new + @vendor_status ||= OutageStatus.new end end diff --git a/app/controllers/concerns/api/csrf_token_concern.rb b/app/controllers/concerns/api/csrf_token_concern.rb new file mode 100644 index 00000000000..593a24043f2 --- /dev/null +++ b/app/controllers/concerns/api/csrf_token_concern.rb @@ -0,0 +1,7 @@ +module Api + module CsrfTokenConcern + def add_csrf_token_header_to_response + response.set_header('X-CSRF-Token', form_authenticity_token) + end + end +end diff --git a/app/controllers/concerns/idv/document_capture_concern.rb b/app/controllers/concerns/idv/document_capture_concern.rb index 1e43757e7a4..1a873998793 100644 --- a/app/controllers/concerns/idv/document_capture_concern.rb +++ b/app/controllers/concerns/idv/document_capture_concern.rb @@ -1,7 +1,9 @@ module Idv module DocumentCaptureConcern def override_document_capture_step_csp - return if params[:step] != 'document_capture' + if !IdentityConfig.store.doc_auth_document_capture_controller_enabled + return if params[:step] != 'document_capture' + end policy = current_content_security_policy policy.connect_src(*policy.connect_src, 'us.acas.acuant.net') diff --git a/app/controllers/concerns/idv/step_utilities_concern.rb b/app/controllers/concerns/idv/step_utilities_concern.rb index abe4a2c268c..626abf69c62 100644 --- a/app/controllers/concerns/idv/step_utilities_concern.rb +++ b/app/controllers/concerns/idv/step_utilities_concern.rb @@ -43,5 +43,15 @@ def irs_reproofing? service_provider: current_sp, ).present? end + + def document_capture_session + @document_capture_session ||= DocumentCaptureSession.find_by( + uuid: flow_session[document_capture_session_uuid_key], + ) + end + + def document_capture_session_uuid_key + :document_capture_session_uuid + end end end diff --git a/app/controllers/concerns/idv/verify_info_concern.rb b/app/controllers/concerns/idv/verify_info_concern.rb index 9e1acd7dce2..4f3dd6402a7 100644 --- a/app/controllers/concerns/idv/verify_info_concern.rb +++ b/app/controllers/concerns/idv/verify_info_concern.rb @@ -114,7 +114,7 @@ def async_state_done(current_async_state) move_applicant_to_idv_session idv_session.mark_verify_info_step_complete! idv_session.invalidate_steps_after_verify_info! - redirect_to idv_phone_url + redirect_to next_step_url else idv_session.invalidate_verify_info_step! end @@ -122,6 +122,11 @@ def async_state_done(current_async_state) analytics.idv_doc_auth_verify_proofing_results(**form_response.to_h) end + def next_step_url + return idv_gpo_url if OutageStatus.new.gpo_only? + idv_phone_url + end + def summarize_result_and_throttle_failures(summary_result) if summary_result.success? add_proofing_components diff --git a/app/controllers/concerns/reauthentication_required_concern.rb b/app/controllers/concerns/reauthentication_required_concern.rb index c8e2845ce9a..5bafdbcf1d8 100644 --- a/app/controllers/concerns/reauthentication_required_concern.rb +++ b/app/controllers/concerns/reauthentication_required_concern.rb @@ -1,10 +1,32 @@ module ReauthenticationRequiredConcern + include MfaSetupConcern + def confirm_recently_authenticated + if IdentityConfig.store.reauthentication_for_second_factor_management_enabled + confirm_recently_authenticated_2fa + else + @reauthn = reauthn? + return unless user_signed_in? + return if recently_authenticated? + + prompt_for_current_password + end + end + + def confirm_recently_authenticated_2fa @reauthn = reauthn? - return unless user_signed_in? - return if recently_authenticated? + return unless user_fully_authenticated? + non_remembered_device_authentication = user_session[:auth_method].present? && + user_session[:auth_method] != 'remember_device' + return if recently_authenticated? && non_remembered_device_authentication + return if in_multi_mfa_selection_flow? + + analytics.user_2fa_reauthentication_required( + auth_method: user_session[:auth_method], + authenticated_at: user_session[:authn_at], + ) - prompt_for_current_password + prompt_for_second_factor end private @@ -24,6 +46,13 @@ def prompt_for_current_password redirect_to user_password_confirm_url end + def prompt_for_second_factor + store_location(request.url) + user_session[:context] = 'reauthentication' + + redirect_to login_two_factor_options_path(reauthn: true) + end + def factor_from_controller_name { # see LG-5701, translate these diff --git a/app/controllers/concerns/remember_device_concern.rb b/app/controllers/concerns/remember_device_concern.rb index db2a2b15193..64b44c144f7 100644 --- a/app/controllers/concerns/remember_device_concern.rb +++ b/app/controllers/concerns/remember_device_concern.rb @@ -14,7 +14,11 @@ def save_remember_device_preference end def check_remember_device_preference - return unless UserSessionContext.authentication_or_reauthentication_context?(context) + if IdentityConfig.store.reauthentication_for_second_factor_management_enabled + return unless UserSessionContext.authentication_context?(context) + else + return unless UserSessionContext.authentication_or_reauthentication_context?(context) + end return if remember_device_cookie.nil? return unless remember_device_cookie.valid_for_user?( user: current_user, diff --git a/app/controllers/idv/doc_auth_controller.rb b/app/controllers/idv/doc_auth_controller.rb index 2861a53eb7f..b2e55231a30 100644 --- a/app/controllers/idv/doc_auth_controller.rb +++ b/app/controllers/idv/doc_auth_controller.rb @@ -70,11 +70,30 @@ def flow_session end def check_for_outage - if VendorStatus.new.any_ial2_vendor_outage? - session[:vendor_outage_redirect] = current_step - session[:vendor_outage_redirect_from_idv] = true - redirect_to vendor_outage_url - end + return if flow_session[:skip_vendor_outage] + + return redirect_for_proofing_vendor_outage if OutageStatus.new.any_idv_vendor_outage? + return redirect_for_gpo_only if FeatureManagement.idv_gpo_only? + end + + def redirect_for_proofing_vendor_outage + session[:vendor_outage_redirect] = current_step + session[:vendor_outage_redirect_from_idv] = true + + redirect_to vendor_outage_url + end + + def redirect_for_gpo_only + return redirect_to vendor_outage_url unless FeatureManagement.gpo_verification_enabled? + + # During a phone outage, skip the hybrid handoff + # step and go straight to document upload + flow_session[:skip_upload_step] = true unless FeatureManagement.idv_allow_hybrid_flow? + + session[:vendor_outage_redirect] = current_step + session[:vendor_outage_redirect_from_idv] = true + + redirect_to idv_mail_only_warning_url end end end diff --git a/app/controllers/idv/document_capture_controller.rb b/app/controllers/idv/document_capture_controller.rb index 5a75c89d5d9..9505554cba6 100644 --- a/app/controllers/idv/document_capture_controller.rb +++ b/app/controllers/idv/document_capture_controller.rb @@ -3,18 +3,35 @@ class DocumentCaptureController < ApplicationController include IdvSession include StepIndicatorConcern include StepUtilitiesConcern + include DocumentCaptureConcern before_action :render_404_if_document_capture_controller_disabled before_action :confirm_two_factor_authenticated + before_action :confirm_agreement_step_complete + before_action :override_document_capture_step_csp def show increment_step_counts analytics.idv_doc_auth_document_capture_visited(**analytics_arguments) + Funnel::DocAuth::RegisterStep.new(current_user.id, sp_session[:issuer]). + call('document_capture', :view, true) + render :show, locals: extra_view_variables end + def update + handle_stored_result + + analytics.idv_doc_auth_document_capture_submitted(**analytics_arguments) + + Funnel::DocAuth::RegisterStep.new(current_user.id, sp_session[:issuer]). + call('document_capture', :update, true) + + redirect_to idv_ssn_url + end + def extra_view_variables url_builder = ImageUploadPresignedUrlGenerator.new @@ -33,7 +50,6 @@ def extra_view_variables transaction_id: flow_session[:document_capture_session_uuid], ), }.merge( - native_camera_ab_testing_variables, acuant_sdk_upgrade_a_b_testing_variables, in_person_cta_variant_testing_variables, ) @@ -45,6 +61,12 @@ def render_404_if_document_capture_controller_disabled render_not_found unless IdentityConfig.store.doc_auth_document_capture_controller_enabled end + def confirm_agreement_step_complete + return if flow_session['Idv::Steps::AgreementStep'] + + redirect_to idv_doc_auth_url + end + def analytics_arguments { flow_path: flow_path, @@ -65,13 +87,6 @@ def increment_step_counts current_flow_step_counts['Idv::Steps::DocumentCaptureStep'] += 1 end - def native_camera_ab_testing_variables - { - acuant_sdk_upgrade_ab_test_bucket: - AbTests::ACUANT_SDK.bucket(flow_session[:document_capture_session_uuid]), - } - end - def acuant_sdk_upgrade_a_b_testing_variables bucket = AbTests::ACUANT_SDK.bucket(flow_session[:document_capture_session_uuid]) testing_enabled = IdentityConfig.store.idv_acuant_sdk_upgrade_a_b_testing_enabled @@ -97,5 +112,79 @@ def in_person_cta_variant_testing_variables in_person_cta_variant_active: bucket, } end + + def handle_stored_result + if stored_result&.success? + save_proofing_components + extract_pii_from_doc(stored_result, store_in_session: !hybrid_flow_mobile?) + else + extra = { stored_result_present: stored_result.present? } + failure(I18n.t('doc_auth.errors.general.network_error'), extra) + end + end + + def stored_result + return @stored_result if defined?(@stored_result) + @stored_result = document_capture_session&.load_result + end + + def save_proofing_components + return unless current_user + + doc_auth_vendor = DocAuthRouter.doc_auth_vendor( + discriminator: flow_session[document_capture_session_uuid_key], + analytics: analytics, + ) + + component_attributes = { + document_check: doc_auth_vendor, + document_type: 'state_id', + } + ProofingComponent.create_or_find_by(user: current_user).update(component_attributes) + end + + def hybrid_flow_mobile? + user_id_from_token.present? + end + + def user_id_from_token + flow_session[:doc_capture_user_id] + end + + # copied from doc_auth_base_step.rb + # @param [DocAuth::Response, + # DocumentCaptureSessionAsyncResult, + # DocumentCaptureSessionResult] response + def extract_pii_from_doc(response, store_in_session: false) + pii_from_doc = response.pii_from_doc.merge( + uuid: effective_user.uuid, + phone: effective_user.phone_configurations.take&.phone, + uuid_prefix: ServiceProvider.find_by(issuer: sp_session[:issuer])&.app_id, + ) + + flow_session[:had_barcode_read_failure] = response.attention_with_barcode? + if store_in_session + flow_session[:pii_from_doc] ||= {} + flow_session[:pii_from_doc].merge!(pii_from_doc) + idv_session.clear_applicant! + end + track_document_state(pii_from_doc[:state]) + end + + def track_document_state(state) + return unless IdentityConfig.store.state_tracking_enabled && state + doc_auth_log = DocAuthLog.find_by(user_id: current_user.id) + return unless doc_auth_log + doc_auth_log.state = state + doc_auth_log.save! + end + + # copied from Flow::Failure module + def failure(message, extra = nil) + flow_session[:error_message] = message + form_response_params = { success: false, errors: { message: message } } + form_response_params[:extra] = extra unless extra.nil? + FormResponse.new(**form_response_params) + end end end diff --git a/app/controllers/idv/gpo_only_warning_controller.rb b/app/controllers/idv/gpo_only_warning_controller.rb new file mode 100644 index 00000000000..8735f8fa13f --- /dev/null +++ b/app/controllers/idv/gpo_only_warning_controller.rb @@ -0,0 +1,17 @@ +module Idv + class GpoOnlyWarningController < ApplicationController + include IdvSession + include StepIndicatorConcern + + before_action :confirm_two_factor_authenticated + + def show + user_session['idv/doc_auth'][:skip_vendor_outage] = true + render :show, locals: { current_sp:, exit_url: } + end + + def exit_url + current_sp&.return_to_sp_url || account_path + end + end +end diff --git a/app/controllers/idv/phone_controller.rb b/app/controllers/idv/phone_controller.rb index b16f92a9126..6afe9035396 100644 --- a/app/controllers/idv/phone_controller.rb +++ b/app/controllers/idv/phone_controller.rb @@ -14,9 +14,14 @@ class PhoneController < ApplicationController def new analytics.idv_phone_use_different(step: params[:step]) if params[:step] + async_state = step.async_state + + # It's possible that create redirected here after a success and left the + # throttle maxed out. Check for success before checking throttle. + return async_state_done(async_state) if async_state.done? + redirect_to failure_url(:fail) and return if throttle.throttled? - async_state = step.async_state if async_state.none? Funnel::DocAuth::RegisterStep.new(current_user.id, current_sp&.issuer). call(:verify_phone, :view, true) @@ -29,8 +34,6 @@ def new analytics.proofing_address_result_missing flash.now[:error] = I18n.t('idv.failure.timeout') render :new, locals: { gpo_letter_available: gpo_letter_available } - elsif async_state.done? - async_state_done(async_state) end end @@ -57,16 +60,9 @@ def throttle @throttle ||= Throttle.new(user: current_user, throttle_type: :proof_address) end - def max_attempts_reached - analytics.throttler_rate_limit_triggered( - throttle_type: :proof_address, - step_name: step_name, - ) - end - def redirect_to_next_step if phone_confirmation_required? - if VendorStatus.new.all_phone_vendor_outage? + if OutageStatus.new.all_phone_vendor_outage? redirect_to vendor_outage_path(from: :idv_phone) else send_phone_confirmation_otp_and_handle_result @@ -113,7 +109,6 @@ def handle_send_phone_confirmation_otp_failure(result) end def handle_proofing_failure - max_attempts_reached if step.failure_reason == :fail redirect_to failure_url(step.failure_reason) end @@ -125,6 +120,7 @@ def step @step ||= Idv::PhoneStep.new( idv_session: idv_session, trace_id: amzn_trace_id, + analytics: analytics, attempts_tracker: irs_attempts_api_tracker, ) end @@ -186,7 +182,7 @@ def new_phone_added? def gpo_letter_available return @gpo_letter_available if defined?(@gpo_letter_available) - @gpo_letter_available ||= FeatureManagement.enable_gpo_verification? && + @gpo_letter_available ||= FeatureManagement.gpo_verification_enabled? && !Idv::GpoMail.new(current_user).mail_spammed? end diff --git a/app/controllers/idv/phone_errors_controller.rb b/app/controllers/idv/phone_errors_controller.rb index 31cfab5e2ec..fed541d4730 100644 --- a/app/controllers/idv/phone_errors_controller.rb +++ b/app/controllers/idv/phone_errors_controller.rb @@ -56,7 +56,7 @@ def track_event(type:) # rubocop:disable Naming/MemoizedInstanceVariableName def set_gpo_letter_available return @gpo_letter_available if defined?(@gpo_letter_available) - @gpo_letter_available ||= FeatureManagement.enable_gpo_verification? && + @gpo_letter_available ||= FeatureManagement.gpo_verification_enabled? && !Idv::GpoMail.new(current_user).mail_spammed? end # rubocop:enable Naming/MemoizedInstanceVariableName diff --git a/app/controllers/sign_up/registrations_controller.rb b/app/controllers/sign_up/registrations_controller.rb index 19204b5c997..51a9ddb7e71 100644 --- a/app/controllers/sign_up/registrations_controller.rb +++ b/app/controllers/sign_up/registrations_controller.rb @@ -72,7 +72,7 @@ def sp_request_id end def redirect_if_ial2_and_vendor_outage - return unless ial2_requested? && VendorStatus.new.any_ial2_vendor_outage? + return unless ial2_requested? && OutageStatus.new.any_idv_vendor_outage? session[:vendor_outage_redirect] = CREATE_ACCOUNT return redirect_to vendor_outage_url diff --git a/app/controllers/users/backup_code_setup_controller.rb b/app/controllers/users/backup_code_setup_controller.rb index d1dc40098e2..191ca00855d 100644 --- a/app/controllers/users/backup_code_setup_controller.rb +++ b/app/controllers/users/backup_code_setup_controller.rb @@ -3,6 +3,7 @@ class BackupCodeSetupController < ApplicationController include MfaSetupConcern include RememberDeviceConcern include SecureHeadersConcern + include ReauthenticationRequiredConcern before_action :authenticate_user! before_action :confirm_user_authenticated_for_2fa_setup @@ -10,6 +11,9 @@ class BackupCodeSetupController < ApplicationController before_action :set_backup_code_setup_presenter before_action :apply_secure_headers_override before_action :authorize_backup_code_disable, only: [:delete] + before_action :confirm_recently_authenticated_2fa, if: -> do + IdentityConfig.store.reauthentication_for_second_factor_management_enabled + end helper_method :in_multi_mfa_selection_flow? diff --git a/app/controllers/users/phones_controller.rb b/app/controllers/users/phones_controller.rb index 22c3210acd9..ca4f3988773 100644 --- a/app/controllers/users/phones_controller.rb +++ b/app/controllers/users/phones_controller.rb @@ -31,7 +31,7 @@ def create private def redirect_if_phone_vendor_outage - return unless VendorStatus.new.all_phone_vendor_outage? + return unless OutageStatus.new.all_phone_vendor_outage? redirect_to vendor_outage_path(from: :users_phones) end diff --git a/app/controllers/users/piv_cac_authentication_setup_controller.rb b/app/controllers/users/piv_cac_authentication_setup_controller.rb index ff366d4490c..dbc37096e9c 100644 --- a/app/controllers/users/piv_cac_authentication_setup_controller.rb +++ b/app/controllers/users/piv_cac_authentication_setup_controller.rb @@ -5,12 +5,16 @@ class PivCacAuthenticationSetupController < ApplicationController include MfaSetupConcern include RememberDeviceConcern include SecureHeadersConcern + include ReauthenticationRequiredConcern before_action :authenticate_user! before_action :confirm_user_authenticated_for_2fa_setup before_action :authorize_piv_cac_disable, only: :delete before_action :set_piv_cac_setup_csp_form_action_uris, only: :new before_action :cap_piv_cac_count, only: %i[new submit_new_piv_cac] + before_action :confirm_recently_authenticated_2fa, if: -> do + IdentityConfig.store.reauthentication_for_second_factor_management_enabled + end helper_method :in_multi_mfa_selection_flow? diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 14d112334ca..f10861b521c 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -6,6 +6,7 @@ class SessionsController < Devise::SessionsController include SecureHeadersConcern include RememberDeviceConcern include Ial2ProfileConcern + include Api::CsrfTokenConcern rescue_from ActionController::InvalidAuthenticityToken, with: :redirect_to_signin @@ -15,6 +16,7 @@ class SessionsController < Devise::SessionsController before_action :check_user_needs_redirect, only: [:new] before_action :apply_secure_headers_override, only: [:new, :create] before_action :clear_session_bad_password_count_if_window_expired, only: [:create] + after_action :add_csrf_token_header_to_response, only: [:keepalive] def new analytics.sign_in_page_visit( @@ -51,14 +53,12 @@ def destroy end def active - response.headers['Etag'] = '' # clear etags to prevent caching session[:pinged_at] = now Rails.logger.debug(alive?: alive?, expires_at: expires_at) render json: { live: alive?, timeout: expires_at, remaining: remaining_session_time } end def keepalive - response.headers['Etag'] = '' # clear etags to prevent caching session[:session_expires_at] = now + Devise.timeout_in if alive? analytics.session_kept_alive if alive? diff --git a/app/controllers/users/totp_setup_controller.rb b/app/controllers/users/totp_setup_controller.rb index 9ef3bc836f4..542b0b0a303 100644 --- a/app/controllers/users/totp_setup_controller.rb +++ b/app/controllers/users/totp_setup_controller.rb @@ -4,12 +4,16 @@ class TotpSetupController < ApplicationController include MfaSetupConcern include RememberDeviceConcern include SecureHeadersConcern + include ReauthenticationRequiredConcern before_action :authenticate_user! before_action :confirm_user_authenticated_for_2fa_setup before_action :set_totp_setup_presenter before_action :apply_secure_headers_override before_action :cap_auth_app_count, only: %i[new confirm] + before_action :confirm_recently_authenticated_2fa, if: -> do + IdentityConfig.store.reauthentication_for_second_factor_management_enabled + end helper_method :in_multi_mfa_selection_flow? diff --git a/app/controllers/users/two_factor_authentication_controller.rb b/app/controllers/users/two_factor_authentication_controller.rb index 7b152845a77..b413a5bc881 100644 --- a/app/controllers/users/two_factor_authentication_controller.rb +++ b/app/controllers/users/two_factor_authentication_controller.rb @@ -43,7 +43,7 @@ def non_phone_redirect end def phone_redirect - return unless phone_enabled? && !VendorStatus.new.any_phone_vendor_outage? + return unless phone_enabled? && !OutageStatus.new.any_phone_vendor_outage? validate_otp_delivery_preference_and_send_code true end @@ -145,7 +145,7 @@ def redirect_if_blank_phone end def redirect_to_vendor_outage_if_phone_only - return unless VendorStatus.new.all_phone_vendor_outage? && + return unless OutageStatus.new.all_phone_vendor_outage? && phone_enabled? && !MfaPolicy.new(current_user).multiple_factors_enabled? redirect_to vendor_outage_path(from: :two_factor_authentication) diff --git a/app/controllers/users/webauthn_setup_controller.rb b/app/controllers/users/webauthn_setup_controller.rb index 2b04aa2dd38..adc3ea3d46d 100644 --- a/app/controllers/users/webauthn_setup_controller.rb +++ b/app/controllers/users/webauthn_setup_controller.rb @@ -3,11 +3,15 @@ class WebauthnSetupController < ApplicationController include MfaSetupConcern include RememberDeviceConcern include SecureHeadersConcern + include ReauthenticationRequiredConcern before_action :authenticate_user! before_action :confirm_user_authenticated_for_2fa_setup before_action :apply_secure_headers_override before_action :set_webauthn_setup_presenter + before_action :confirm_recently_authenticated_2fa, if: -> do + IdentityConfig.store.reauthentication_for_second_factor_management_enabled + end helper_method :in_multi_mfa_selection_flow? diff --git a/app/controllers/vendor_outage_controller.rb b/app/controllers/vendor_outage_controller.rb index ab47989c85e..fdc023b203f 100644 --- a/app/controllers/vendor_outage_controller.rb +++ b/app/controllers/vendor_outage_controller.rb @@ -1,6 +1,6 @@ class VendorOutageController < ApplicationController def show - vendor_status = VendorStatus.new( + vendor_status = OutageStatus.new( sp: current_sp, from: session.delete(:vendor_outage_redirect), from_idv: session.delete(:vendor_outage_redirect_from_idv), @@ -17,7 +17,7 @@ def from_idv_phone? end def gpo_letter_available? - FeatureManagement.enable_gpo_verification? && + FeatureManagement.gpo_verification_enabled? && current_user && !Idv::GpoMail.new(current_user).mail_spammed? end diff --git a/app/forms/idv/in_person/address_form.rb b/app/forms/idv/in_person/address_form.rb index a9bf9764bdf..eb6ce10935f 100644 --- a/app/forms/idv/in_person/address_form.rb +++ b/app/forms/idv/in_person/address_form.rb @@ -8,6 +8,10 @@ class AddressForm attr_accessor(*ATTRIBUTES) + def initialize(capture_secondary_id_enabled:) + @capture_secondary_id_enabled = capture_secondary_id_enabled + end + def self.model_name ActiveModel::Name.new(self, nil, 'InPersonAddress') end @@ -28,6 +32,9 @@ def submit(params) private + attr_reader :capture_secondary_id_enabled + alias_method :capture_secondary_id_enabled?, :capture_secondary_id_enabled + def consume_params(params) params.each do |key, value| raise_invalid_address_parameter_error(key) unless ATTRIBUTES.include?(key.to_sym) diff --git a/app/forms/new_phone_form.rb b/app/forms/new_phone_form.rb index af4f76885e0..068310eafea 100644 --- a/app/forms/new_phone_form.rb +++ b/app/forms/new_phone_form.rb @@ -46,11 +46,11 @@ def submit(params) end def delivery_preference_sms? - !VendorStatus.new.vendor_outage?(:sms) + !OutageStatus.new.vendor_outage?(:sms) end def delivery_preference_voice? - VendorStatus.new.vendor_outage?(:sms) || setup_voice_preference? + OutageStatus.new.vendor_outage?(:sms) || setup_voice_preference? end # @return [Telephony::PhoneNumberInfo, nil] diff --git a/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.spec.ts b/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.spec.ts index e961ba9d37b..634aeb4ebfd 100644 --- a/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.spec.ts +++ b/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.spec.ts @@ -1,6 +1,6 @@ import type { SinonStub } from 'sinon'; import userEvent from '@testing-library/user-event'; -import { screen, waitFor } from '@testing-library/dom'; +import { screen, waitFor, fireEvent } from '@testing-library/dom'; import { useSandbox, useDefineProperty } from '@18f/identity-test-helpers'; import '@18f/identity-spinner-button/spinner-button-element'; import { CAPTCHA_EVENT_NAME } from './captcha-submit-button-element'; @@ -35,58 +35,30 @@ describe('CaptchaSubmitButtonElement', () => { `; }); - it('submits the form', async () => { + it('does not prevent default form submission', async () => { const button = screen.getByRole('button', { name: 'Submit' }); const form = document.querySelector('form')!; - sandbox.stub(form, 'submit'); + let didSubmit = false; + form.addEventListener('submit', (event) => { + expect(event.defaultPrevented).to.equal(false); + event.preventDefault(); + didSubmit = true; + }); await userEvent.click(button); - await waitFor(() => expect((form.submit as SinonStub).called).to.be.true()); + await waitFor(() => expect(didSubmit).to.be.true()); }); - context('with form validation errors', () => { - beforeEach(() => { - document.body.innerHTML = ` -
- - - - - - -
- `; - }); - - it('does not submit the form and reports validity', async () => { - const button = screen.getByRole('button', { name: 'Submit' }); - const form = document.querySelector('form')!; - const input = document.querySelector('input')!; - - let didSubmit = false; - form.addEventListener('submit', (event) => { - event.preventDefault(); - didSubmit = true; - }); - - let didReportInvalid = false; - input.addEventListener('invalid', () => { - didReportInvalid = true; - }); + it('unbinds form events when disconnected', () => { + const submitButton = document.querySelector('lg-captcha-submit-button')!; + const form = submitButton.form!; + form.removeChild(submitButton); - await userEvent.click(button); + sandbox.spy(submitButton, 'shouldInvokeChallenge'); + fireEvent.submit(form); - expect(didSubmit).to.be.false(); - expect(didReportInvalid).to.be.true(); - }); - - it('stops or otherwise prevents the spinner button from spinning', async () => { - const button = screen.getByRole('button', { name: 'Submit' }); - await userEvent.click(button); - - expect(document.querySelector('.spinner-button--spinner-active')).to.not.exist(); - }); + expect(submitButton.shouldInvokeChallenge).not.to.have.been.called(); }); context('with configured recaptcha', () => { @@ -130,7 +102,7 @@ describe('CaptchaSubmitButtonElement', () => { expect(grecaptcha.execute).to.have.been.calledWith(RECAPTCHA_SITE_KEY, { action: RECAPTCHA_ACTION_NAME, }); - expect(Object.fromEntries(new FormData(form))).to.deep.equal({ + expect(Object.fromEntries(new window.FormData(form))).to.deep.equal({ recaptcha_token: RECAPTCHA_TOKEN_VALUE, }); }); @@ -145,10 +117,14 @@ describe('CaptchaSubmitButtonElement', () => { const button = screen.getByRole('button', { name: 'Submit' }); const form = document.querySelector('form')!; - sandbox.stub(form, 'submit'); + let didSubmit = false; + form.addEventListener('submit', (event) => { + event.preventDefault(); + didSubmit = true; + }); await userEvent.click(button); - await waitFor(() => expect((form.submit as SinonStub).called).to.be.true()); + await waitFor(() => expect(didSubmit).to.be.true()); expect(grecaptcha.ready).not.to.have.been.called(); }); diff --git a/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.ts b/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.ts index 3828d484d39..7f5eecde1c1 100644 --- a/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.ts +++ b/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.ts @@ -1,8 +1,16 @@ export const CAPTCHA_EVENT_NAME = 'lg:captcha-submit-button:challenge'; class CaptchaSubmitButtonElement extends HTMLElement { + form: HTMLFormElement | null; + connectedCallback() { - this.button.addEventListener('click', (event) => this.handleButtonClick(event)); + this.form = this.closest('form'); + + this.form?.addEventListener('submit', this.handleFormSubmit); + } + + disconnectedCallback() { + this.form?.removeEventListener('submit', this.handleFormSubmit); } get button(): HTMLButtonElement { @@ -13,10 +21,6 @@ class CaptchaSubmitButtonElement extends HTMLElement { return this.querySelector('[type=hidden]')!; } - get form(): HTMLFormElement | null { - return this.closest('form'); - } - get recaptchaSiteKey(): string | null { return this.getAttribute('recaptcha-site-key'); } @@ -48,21 +52,12 @@ class CaptchaSubmitButtonElement extends HTMLElement { return !event.defaultPrevented; } - handleButtonClick(event: MouseEvent) { - event.preventDefault(); - - if (this.form && !this.form.reportValidity()) { - // Prevent any associated custom click handling, e.g. spinner button spinning - event.stopImmediatePropagation(); - return; - } - + handleFormSubmit = (event: SubmitEvent) => { if (this.shouldInvokeChallenge()) { + event.preventDefault(); this.invokeChallenge(); - } else { - this.submit(); } - } + }; } declare global { diff --git a/app/javascript/packages/request/index.spec.ts b/app/javascript/packages/request/index.spec.ts index e18e0a5a414..7daf28d5500 100644 --- a/app/javascript/packages/request/index.spec.ts +++ b/app/javascript/packages/request/index.spec.ts @@ -1,3 +1,5 @@ +import sinon from 'sinon'; +import type { SinonStub } from 'sinon'; import { useSandbox } from '@18f/identity-test-helpers'; import { request } from '.'; @@ -25,6 +27,7 @@ describe('request', () => { expect(window.fetch).to.have.been.calledOnce(); }); + it('works even if the CSRF token is not found on the page', async () => { sandbox.stub(window, 'fetch').callsFake(() => Promise.resolve( @@ -38,6 +41,7 @@ describe('request', () => { csrf: () => undefined, }); }); + it('does not try to send a csrf when csrf is false', async () => { sandbox.stub(window, 'fetch').callsFake((url, init = {}) => { const headers = init.headers as Headers; @@ -54,6 +58,7 @@ describe('request', () => { csrf: false, }); }); + it('prefers the json prop if both json and body props are provided', async () => { const preferredData = { prefered: 'data' }; sandbox.stub(window, 'fetch').callsFake((url, init = {}) => { @@ -71,6 +76,7 @@ describe('request', () => { body: JSON.stringify({ bad: 'data' }), }); }); + it('works with the native body prop', async () => { const preferredData = { this: 'works' }; sandbox.stub(window, 'fetch').callsFake((url, init = {}) => { @@ -87,6 +93,7 @@ describe('request', () => { body: JSON.stringify(preferredData), }); }); + it('includes additional headers supplied in options', async () => { sandbox.stub(window, 'fetch').callsFake((url, init = {}) => { const headers = init.headers as Headers; @@ -105,6 +112,7 @@ describe('request', () => { }, }); }); + it('skips json serialization when json is a boolean', async () => { const preferredData = { this: 'works' }; sandbox.stub(window, 'fetch').callsFake((url, init = {}) => { @@ -122,6 +130,7 @@ describe('request', () => { body: JSON.stringify(preferredData), }); }); + it('converts a POJO to a JSON string with supplied via the json property', async () => { const preferredData = { this: 'works' }; sandbox.stub(window, 'fetch').callsFake((url, init = {}) => { @@ -138,4 +147,67 @@ describe('request', () => { json: preferredData, }); }); + + context('with response including csrf token', () => { + beforeEach(() => { + sandbox.stub(window, 'fetch').callsFake(() => + Promise.resolve( + new Response(JSON.stringify({}), { + status: 200, + headers: [['X-CSRF-Token', 'new-token']], + }), + ), + ); + }); + + it('does nothing, gracefully', async () => { + await request('https://example.com', {}); + }); + + context('with global csrf token', () => { + beforeEach(() => { + document.head.innerHTML += ` + + + `; + }); + + it('replaces global csrf token with the response token', async () => { + await request('https://example.com', {}); + + const metaToken = document.querySelector('meta[name="csrf-token"]')!; + expect(metaToken.content).to.equal('new-token'); + }); + + it('uses response token for next request', async () => { + await request('https://example.com', {}); + (window.fetch as SinonStub).resetHistory(); + await request('https://example.com', {}); + expect(window.fetch).to.have.been.calledWith( + sinon.match.string, + sinon.match((init) => init!.headers!.get('x-csrf-token') === 'new-token'), + ); + }); + + context('with form csrf token', () => { + beforeEach(() => { + document.body.innerHTML += ` +
+
+ `; + }); + + it('replaces form tokens with the response token', async () => { + await request('https://example.com', {}); + + const inputs = document.querySelectorAll('input'); + expect(inputs).to.have.lengthOf(2); + expect(Array.from(inputs).map((input) => input.value)).to.deep.equal([ + 'new-token', + 'new-token', + ]); + }); + }); + }); + }); }); diff --git a/app/javascript/packages/request/index.ts b/app/javascript/packages/request/index.ts index 927a71def66..c7b518152e1 100644 --- a/app/javascript/packages/request/index.ts +++ b/app/javascript/packages/request/index.ts @@ -12,10 +12,43 @@ interface RequestOptions extends RequestInit { csrf?: boolean | CSRFGetter; } -const getCSRFToken = () => - document.querySelector('meta[name="csrf-token"]')?.content; +class CSRF { + static get token(): string | null { + return this.#tokenMetaElement?.content || null; + } + + static set token(value: string | null) { + if (!value) { + return; + } + + if (this.#tokenMetaElement) { + this.#tokenMetaElement.content = value; + } + + this.#paramInputElements.forEach((input) => { + input.value = value; + }); + } + + static get param(): string | undefined { + return this.#paramMetaElement?.content; + } -export async function request( + static get #tokenMetaElement(): HTMLMetaElement | null { + return document.querySelector('meta[name="csrf-token"]'); + } + + static get #paramMetaElement(): HTMLMetaElement | null { + return document.querySelector('meta[name="csrf-param"]'); + } + + static get #paramInputElements(): NodeListOf { + return document.querySelectorAll(`input[name="${this.param}"]`); + } +} + +export async function request( url: string, options: Partial = {}, ): Promise { @@ -24,7 +57,7 @@ export async function request( headers = new Headers(headers); if (csrf) { - const csrfToken = typeof csrf === 'boolean' ? getCSRFToken() : csrf(); + const csrfToken = typeof csrf === 'boolean' ? CSRF.token : csrf(); if (csrfToken) { headers.set('X-CSRF-Token', csrfToken); @@ -41,9 +74,11 @@ export async function request( } const response = await window.fetch(url, { ...fetchOptions, headers, body }); - if (response.ok) { - return json ? response.json() : response.text(); + CSRF.token = response.headers.get('X-CSRF-Token'); + + if (!response.ok) { + throw new Error(); } - throw new Error(await response.json()); + return json ? response.json() : response.text(); } diff --git a/app/javascript/packages/spinner-button/spinner-button-element.spec.ts b/app/javascript/packages/spinner-button/spinner-button-element.spec.ts index ed2678340bc..12e132e0d1b 100644 --- a/app/javascript/packages/spinner-button/spinner-button-element.spec.ts +++ b/app/javascript/packages/spinner-button/spinner-button-element.spec.ts @@ -3,7 +3,6 @@ import { getByRole, fireEvent, screen } from '@testing-library/dom'; import type { SinonStub } from 'sinon'; import { useSandbox } from '@18f/identity-test-helpers'; import './spinner-button-element'; -import type { SpinnerButtonElement } from './spinner-button-element'; describe('SpinnerButtonElement', () => { const sandbox = useSandbox({ useFakeTimers: true }); @@ -14,20 +13,37 @@ describe('SpinnerButtonElement', () => { interface WrapperOptions { actionMessage?: string; - tagName?: string; - spinOnClick?: boolean; + inForm?: boolean; + isButtonTo?: boolean; } - function createWrapper({ actionMessage, tagName = 'a', spinOnClick }: WrapperOptions = {}) { - document.body.innerHTML = ` + function createWrapper({ + actionMessage, + tagName = 'a', + spinOnClick, + inForm, + isButtonTo, + }: WrapperOptions = {}) { + let tag; + if (tagName === 'a') { + tag = 'Click Me'; + } else { + tag = ''; + } + + if (isButtonTo) { + tag = `
${tag}
`; + } + + let html = ` <% end %> -<% if FeatureManagement.enable_gpo_verification? && !@mail_spammed %> +<% if FeatureManagement.gpo_verification_enabled? && !@mail_spammed %> <%= link_to t('idv.messages.gpo.resend'), idv_gpo_path, class: 'display-block margin-bottom-2' %> <% end %> diff --git a/app/views/idv/in_person/address.html.erb b/app/views/idv/in_person/address.html.erb index c7a9640b682..ec88ca63365 100644 --- a/app/views/idv/in_person/address.html.erb +++ b/app/views/idv/in_person/address.html.erb @@ -6,21 +6,35 @@ <%= render PageHeadingComponent.new.with_content(t('in_person_proofing.headings.address')) %> <% end %> -

- <%= t('in_person_proofing.body.address.info') %> - <%= new_window_link_to( - t('in_person_proofing.body.address.learn_more'), - MarketingSite.help_center_article_url( - category: 'verify-your-identity', - article: 'verify-your-identity-in-person', - ), - ) %> -

+<% unless capture_secondary_id_enabled %> +

+ <%= t('in_person_proofing.body.address.info') %> + <%= new_window_link_to( + t('in_person_proofing.body.address.learn_more'), + MarketingSite.help_center_article_url( + category: 'verify-your-identity', + article: 'verify-your-identity-in-person', + ), + ) %> +

+<% end %> <%= simple_form_for( form, url: url_for, method: 'PUT', html: { autocomplete: 'off' } ) do |f| %> + <% if capture_secondary_id_enabled %> + <%= render ValidatedFieldComponent.new( + collection: us_states_territories, + form: f, + label: t('idv.form.state'), + label_html: { class: 'usa-label' }, + name: :state, + prompt: t('in_person_proofing.form.address.state_prompt'), + required: true, + selected: pii[:state], + ) %> + <% end %> <%= render ValidatedFieldComponent.new( form: f, input_html: { value: pii[:address1] }, @@ -33,7 +47,7 @@ <%= render ValidatedFieldComponent.new( form: f, input_html: { value: pii[:address2] }, - label: t('idv.form.address2_optional'), + label: capture_secondary_id_enabled ? t('idv.form.address2') : t('idv.form.address2_optional'), label_html: { class: 'usa-label' }, maxlength: 255, name: :address2, @@ -48,16 +62,19 @@ name: :city, required: true, ) %> - <%= render ValidatedFieldComponent.new( - collection: us_states_territories, - form: f, - label: t('idv.form.state'), - label_html: { class: 'usa-label' }, - name: :state, - prompt: t('in_person_proofing.form.address.state_prompt'), - required: true, - selected: pii[:state], - ) %> + + <% unless capture_secondary_id_enabled %> + <%= render ValidatedFieldComponent.new( + collection: us_states_territories, + form: f, + label: t('idv.form.state'), + label_html: { class: 'usa-label' }, + name: :state, + prompt: t('in_person_proofing.form.address.state_prompt'), + required: true, + selected: pii[:state], + ) %> + <% end %>
<%# using :tel for mobile numeric keypad %> @@ -74,19 +91,21 @@ ) %>
- <%= render ValidatedFieldComponent.new( - as: :radio_buttons, - checked: pii[:same_address_as_id], - collection: [ - [t('in_person_proofing.form.address.same_address_choice_yes'), true], - [t('in_person_proofing.form.address.same_address_choice_no'), false], - ], - form: f, - label: t('in_person_proofing.form.address.same_address'), - name: :same_address_as_id, - required: true, - wrapper: :uswds_radio_buttons, - ) %> + <% unless capture_secondary_id_enabled %> + <%= render ValidatedFieldComponent.new( + as: :radio_buttons, + checked: pii[:same_address_as_id], + collection: [ + [t('in_person_proofing.form.address.same_address_choice_yes'), true], + [t('in_person_proofing.form.address.same_address_choice_no'), false], + ], + form: f, + label: t('in_person_proofing.form.address.same_address'), + name: :same_address_as_id, + required: true, + wrapper: :uswds_radio_buttons, + ) %> + <% end %> <%= f.submit class: 'margin-top-1' do %> <% if updating_address %> diff --git a/app/views/idv/phone/new.html.erb b/app/views/idv/phone/new.html.erb index e0a34dec849..2c6e5f38dfc 100644 --- a/app/views/idv/phone/new.html.erb +++ b/app/views/idv/phone/new.html.erb @@ -71,8 +71,8 @@ <%= f.radio_button( :otp_delivery_preference, :sms, - checked: !VendorStatus.new.vendor_outage?(:sms), # We want SMS to be default checked - disabled: VendorStatus.new.vendor_outage?(:sms), + checked: !OutageStatus.new.vendor_outage?(:sms), # We want SMS to be default checked + disabled: OutageStatus.new.vendor_outage?(:sms), class: 'usa-radio__input usa-radio__input--bordered', ) %> <%= f.label :otp_delivery_preference_sms, t('two_factor_authentication.otp_delivery_preference.sms'), class: 'usa-radio__label width-full' %> @@ -81,7 +81,7 @@ <%= f.radio_button( :otp_delivery_preference, :voice, - disabled: VendorStatus.new.vendor_outage?(:voice), + disabled: OutageStatus.new.vendor_outage?(:voice), class: 'usa-radio__input usa-radio__input--bordered', ) %> <%= f.label :otp_delivery_preference_voice, t('two_factor_authentication.otp_delivery_preference.voice'), class: 'usa-radio__label width-full' %> diff --git a/app/views/shared/_sp_alert.html.erb b/app/views/shared/_sp_alert.html.erb index fdc0ce00877..211785df9b2 100644 --- a/app/views/shared/_sp_alert.html.erb +++ b/app/views/shared/_sp_alert.html.erb @@ -1,6 +1,6 @@ <% alert = decorated_session.sp_alert(section) %> <% if alert %> - <%= render AlertComponent.new(text_tag: 'div') do %> + <%= render AlertComponent.new(text_tag: 'div', class: 'margin-bottom-4') do %> <%= raw sanitize(alert, tags: %w[a b strong em br p ol ul li], attributes: %w[href target]) %> <% end %> <% end %> diff --git a/app/views/users/shared/_otp_delivery_preference_selection.html.erb b/app/views/users/shared/_otp_delivery_preference_selection.html.erb index cc3bb484104..3727d029215 100644 --- a/app/views/users/shared/_otp_delivery_preference_selection.html.erb +++ b/app/views/users/shared/_otp_delivery_preference_selection.html.erb @@ -14,7 +14,7 @@ form_name_label, :sms, form_obj.delivery_preference_sms?, - disabled: VendorStatus.new.vendor_outage?(:sms), + disabled: OutageStatus.new.vendor_outage?(:sms), class: 'js-otp-delivery-preference usa-radio__input usa-radio__input--bordered', ) %>