diff --git a/app/assets/stylesheets/_uswds.scss b/app/assets/stylesheets/_uswds.scss index 2742d09a988..94e4c5ca8be 100644 --- a/app/assets/stylesheets/_uswds.scss +++ b/app/assets/stylesheets/_uswds.scss @@ -14,3 +14,4 @@ @forward 'usa-skipnav'; @forward 'usa-tag'; @forward 'uswds-form-controls'; +@forward 'uswds-utilities'; diff --git a/app/assets/stylesheets/application.css.scss b/app/assets/stylesheets/application.css.scss index 6015355f074..f77d6eccc9f 100644 --- a/app/assets/stylesheets/application.css.scss +++ b/app/assets/stylesheets/application.css.scss @@ -3,5 +3,4 @@ @forward 'uswds'; @forward 'design-system-waiting-room'; @forward 'components'; -@forward 'uswds-utilities'; @forward 'utilities'; diff --git a/app/assets/stylesheets/components/_profile-section.scss b/app/assets/stylesheets/components/_profile-section.scss index 9604d5bedb7..497c10edf9e 100644 --- a/app/assets/stylesheets/components/_profile-section.scss +++ b/app/assets/stylesheets/components/_profile-section.scss @@ -1,5 +1,17 @@ @use 'uswds-core' as *; +@use '../variables/app' as *; .profile-info-box { + border: 0; + border-radius: 0; + margin-bottom: 0; + overflow: hidden; padding: units(4); } + +@include at-media('mobile') { + .profile-info-box { + border-radius: $border-radius-md; + margin-bottom: units(4); + } +} diff --git a/app/assets/stylesheets/variables/_app.scss b/app/assets/stylesheets/variables/_app.scss index 3249beb3be3..37d4c0909b5 100644 --- a/app/assets/stylesheets/variables/_app.scss +++ b/app/assets/stylesheets/variables/_app.scss @@ -14,6 +14,7 @@ $sm-h4: 1rem !default; $sm-h5: 0.875rem !default; $sm-h6: 0.75rem !default; +$border-radius-md: 6px !default; $border-radius-xl: 16px !default; $container-skinny-width: 620px !default; diff --git a/app/controllers/concerns/saml_idp_auth_concern.rb b/app/controllers/concerns/saml_idp_auth_concern.rb index c80a3022a2c..ed7b993576a 100644 --- a/app/controllers/concerns/saml_idp_auth_concern.rb +++ b/app/controllers/concerns/saml_idp_auth_concern.rb @@ -66,7 +66,7 @@ def result service_provider: saml_request_service_provider, authn_context: requested_authn_contexts, authn_context_comparison: saml_request.requested_authn_context_comparison, - nameid_format: saml_request.name_id_format, + nameid_format: name_id_format, ) end @@ -78,8 +78,8 @@ def validate_and_create_saml_request_object @saml_request_validator = SamlRequestValidator.new(blank_cert: true) end - def response_name_id_format - @response_name_id_format ||= specified_name_id_format || default_name_id_format + def name_id_format + @name_id_format ||= specified_name_id_format || default_name_id_format end def specified_name_id_format @@ -93,6 +93,9 @@ def recognized_name_id_format? end def default_name_id_format + if saml_request_service_provider&.email_nameid_format_allowed + return Saml::Idp::Constants::NAME_ID_FORMAT_EMAIL + end Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT end @@ -167,7 +170,7 @@ def attribute_asserter(principal) AttributeAsserter.new( user: principal, service_provider: saml_request_service_provider, - name_id_format: response_name_id_format, + name_id_format: name_id_format, authn_request: saml_request, decrypted_pii: decrypted_pii, user_session: user_session, @@ -187,7 +190,7 @@ def build_asserted_attributes(principal) def saml_response encode_response( current_user, - name_id_format: response_name_id_format, + name_id_format: name_id_format, authn_context_classref: response_authn_context, reference_id: active_identity.session_uuid, encryption: encryption_opts, diff --git a/app/controllers/idv/how_to_verify_controller.rb b/app/controllers/idv/how_to_verify_controller.rb index 2c05bdef304..a41f98da563 100644 --- a/app/controllers/idv/how_to_verify_controller.rb +++ b/app/controllers/idv/how_to_verify_controller.rb @@ -12,15 +12,19 @@ class HowToVerifyController < ApplicationController check_or_render_not_found -> { self.class.enabled? } def show + @selection = if idv_session.skip_doc_auth == false + Idv::HowToVerifyForm::REMOTE + elsif idv_session.skip_doc_auth == true + Idv::HowToVerifyForm::IPP + end + analytics.idv_doc_auth_how_to_verify_visited(**analytics_arguments) - @idv_how_to_verify_form = Idv::HowToVerifyForm.new + @idv_how_to_verify_form = Idv::HowToVerifyForm.new(selection: @selection) end def update clear_future_steps! - @idv_how_to_verify_form = Idv::HowToVerifyForm.new - result = @idv_how_to_verify_form.submit(how_to_verify_form_params) - + result = Idv::HowToVerifyForm.new.submit(how_to_verify_form_params) if how_to_verify_form_params[:selection] == [] sendable_form_params = {} else @@ -44,8 +48,10 @@ def update idv_session.skip_doc_auth_from_how_to_verify = true redirect_to idv_document_capture_url end + else - render :show, locals: { error: result.first_error_message } + flash[:error] = result.first_error_message + redirect_to idv_how_to_verify_url end end diff --git a/app/controllers/two_factor_authentication/backup_code_verification_controller.rb b/app/controllers/two_factor_authentication/backup_code_verification_controller.rb index 7c5831edf57..10bcc18e140 100644 --- a/app/controllers/two_factor_authentication/backup_code_verification_controller.rb +++ b/app/controllers/two_factor_authentication/backup_code_verification_controller.rb @@ -22,7 +22,9 @@ def show def create @backup_code_form = BackupCodeVerificationForm.new(current_user) result = @backup_code_form.submit(backup_code_params) - analytics.multi_factor_auth(**result.to_h.merge(new_device: new_device?)) + analytics.track_mfa_submit_event( + result.to_h.merge(new_device: new_device?), + ) irs_attempts_api_tracker.mfa_login_backup_code(success: result.success?) handle_result(result) end diff --git a/app/controllers/two_factor_authentication/otp_verification_controller.rb b/app/controllers/two_factor_authentication/otp_verification_controller.rb index f3ca89a228f..3bf40093f41 100644 --- a/app/controllers/two_factor_authentication/otp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/otp_verification_controller.rb @@ -136,7 +136,7 @@ def post_analytics(result) properties = result.to_h.merge(analytics_properties, new_device: new_device?) analytics.multi_factor_auth_setup(**properties) if context == 'confirmation' - analytics.multi_factor_auth(**properties) + analytics.track_mfa_submit_event(properties) if UserSessionContext.reauthentication_context?(context) irs_attempts_api_tracker.mfa_login_phone_otp_submitted( diff --git a/app/controllers/two_factor_authentication/personal_key_verification_controller.rb b/app/controllers/two_factor_authentication/personal_key_verification_controller.rb index af95dd27af2..85302e11991 100644 --- a/app/controllers/two_factor_authentication/personal_key_verification_controller.rb +++ b/app/controllers/two_factor_authentication/personal_key_verification_controller.rb @@ -32,10 +32,7 @@ def track_analytics(result) new_device: new_device?, ) - analytics.multi_factor_auth( - **analytics_hash, - pii_like_keypaths: [[:errors, :personal_key], [:error_details, :personal_key]], - ) + analytics.track_mfa_submit_event(analytics_hash) end def check_personal_key_enabled 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 ccd783d158e..56151472bfa 100644 --- a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb +++ b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb @@ -30,7 +30,9 @@ def redirect_to_piv_cac_service def process_token result = piv_cac_verification_form.submit - analytics.multi_factor_auth(**result.to_h.merge(analytics_properties)) + analytics.track_mfa_submit_event( + result.to_h.merge(analytics_properties), + ) irs_attempts_api_tracker.mfa_login_piv_cac( success: result.success?, subject_dn: piv_cac_verification_form.x509_dn, diff --git a/app/controllers/two_factor_authentication/totp_verification_controller.rb b/app/controllers/two_factor_authentication/totp_verification_controller.rb index 28c50c48444..111c8a52c1b 100644 --- a/app/controllers/two_factor_authentication/totp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/totp_verification_controller.rb @@ -21,7 +21,7 @@ def show def create result = TotpVerificationForm.new(current_user, params.require(:code).strip).submit - analytics.multi_factor_auth(**result.to_h.merge(new_device: new_device?)) + analytics.track_mfa_submit_event(result.to_h.merge(new_device: new_device?)) irs_attempts_api_tracker.mfa_login_totp(success: result.success?) if result.success? diff --git a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb index 75f8e6255a3..458ebf650a8 100644 --- a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb +++ b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb @@ -18,7 +18,7 @@ def show def confirm result = form.submit - analytics.multi_factor_auth( + analytics.track_mfa_submit_event( **result.to_h, **analytics_properties, multi_factor_auth_method_created_at: diff --git a/app/controllers/users/backup_code_setup_controller.rb b/app/controllers/users/backup_code_setup_controller.rb index 0de1097304b..03580ac54cc 100644 --- a/app/controllers/users/backup_code_setup_controller.rb +++ b/app/controllers/users/backup_code_setup_controller.rb @@ -80,7 +80,16 @@ def confirm_backup_codes; end private def validate_multi_mfa_selection - redirect_to backup_code_confirm_setup_url unless in_multi_mfa_selection_flow? + if IdentityConfig.store.backup_code_confirm_setup_screen_enabled + redirect_to backup_code_confirm_setup_url unless in_multi_mfa_selection_flow? + else + redirect_to root_url unless internal_referrer? + end + end + + def internal_referrer? + UserSessionContext.reauthentication_context?(context) || + session[:account_redirect_path] || in_multi_mfa_selection_flow? end def analytics_properties_for_visit diff --git a/app/controllers/users/two_factor_authentication_controller.rb b/app/controllers/users/two_factor_authentication_controller.rb index 79bcb6135ac..09d9c825126 100644 --- a/app/controllers/users/two_factor_authentication_controller.rb +++ b/app/controllers/users/two_factor_authentication_controller.rb @@ -321,14 +321,9 @@ def send_user_otp(method) end def otp_length - configured_length = TwoFactorAuthenticatable::DIRECT_OTP_LENGTH - if configured_length == 6 - I18n.t('telephony.format_length.six') - elsif configured_length == 10 - I18n.t('telephony.format_length.ten') - else - raise "Missing translation for OTP length: #{configured_length}" - end + bucket = AbTests::IDV_TEN_DIGIT_OTP.bucket(current_user.uuid) + length = bucket == :ten_digit_otp ? 'ten' : 'six' + I18n.t("telephony.format_length.#{length}") end def user_selected_default_number diff --git a/app/forms/idv/how_to_verify_form.rb b/app/forms/idv/how_to_verify_form.rb index 7178c44338e..bb4eed1adb8 100644 --- a/app/forms/idv/how_to_verify_form.rb +++ b/app/forms/idv/how_to_verify_form.rb @@ -9,13 +9,8 @@ class HowToVerifyForm attr_reader :selection - validates :selection, presence: { - message: proc { I18n.t('errors.doc_auth.how_to_verify_form') }, - } - validates :selection, inclusion: { - in: [REMOTE, IPP], - message: proc { I18n.t('errors.doc_auth.how_to_verify_form') }, - } + validates :selection, + presence: { message: proc { I18n.t('errors.doc_auth.how_to_verify_form') } } def initialize(selection: nil) @selection = selection diff --git a/app/javascript/packages/phone-input/package.json b/app/javascript/packages/phone-input/package.json index e3c49b40102..530f2283179 100644 --- a/app/javascript/packages/phone-input/package.json +++ b/app/javascript/packages/phone-input/package.json @@ -4,7 +4,7 @@ "version": "1.0.0", "dependencies": { "intl-tel-input": "^17.0.19", - "libphonenumber-js": "^1.11.2" + "libphonenumber-js": "^1.11.1" }, "sideEffects": [ "./index.ts" diff --git a/app/models/user.rb b/app/models/user.rb index 843d990fc2b..2274e8dd6d9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -428,6 +428,10 @@ def has_devices? !recent_devices.empty? end + def new_device?(cookie_uuid:) + !cookie_uuid || !devices.exists?(cookie_uuid:) + end + def authenticated_device?(cookie_uuid:) return false if cookie_uuid.blank? devices.joins(:events).exists?(cookie_uuid:, events: { event_type: :sign_in_after_2fa }) diff --git a/app/services/analytics.rb b/app/services/analytics.rb index 0f1ee156c45..f8f415cbe2a 100644 --- a/app/services/analytics.rb +++ b/app/services/analytics.rb @@ -52,6 +52,13 @@ def first_event_this_session? session[:first_event] end + def track_mfa_submit_event(attributes) + multi_factor_auth( + **attributes, + pii_like_keypaths: [[:errors, :personal_key], [:error_details, :personal_key]], + ) + end + def request_attributes attributes = { user_ip: request.remote_ip, diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index b6f0cf176b1..2317b33810f 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -3730,7 +3730,6 @@ def logout_initiated( # @param [Boolean] success Whether authentication was successful # @param [Hash] errors Authentication error reasons, if unsuccessful - # @param [Hash] error_details Details for error that occurred in unsuccessful submission # @param [String] context # @param [Boolean] new_device # @param [String] multi_factor_auth_method @@ -3749,7 +3748,6 @@ def logout_initiated( def multi_factor_auth( success:, errors: nil, - error_details: nil, context: nil, new_device: nil, multi_factor_auth_method: nil, @@ -3769,27 +3767,24 @@ def multi_factor_auth( ) track_event( 'Multi-Factor Authentication', - { - success: success, - errors: errors, - error_details: error_details, - context: context, - new_device: new_device, - multi_factor_auth_method: multi_factor_auth_method, - multi_factor_auth_method_created_at: multi_factor_auth_method_created_at, - auth_app_configuration_id: auth_app_configuration_id, - piv_cac_configuration_id: piv_cac_configuration_id, - key_id: key_id, - webauthn_configuration_id: webauthn_configuration_id, - confirmation_for_add_phone: confirmation_for_add_phone, - phone_configuration_id: phone_configuration_id, - pii_like_keypaths: pii_like_keypaths, - area_code: area_code, - country_code: country_code, - phone_fingerprint: phone_fingerprint, - frontend_error:, - **extra, - }.compact, + success: success, + errors: errors, + context: context, + new_device: new_device, + multi_factor_auth_method: multi_factor_auth_method, + multi_factor_auth_method_created_at: multi_factor_auth_method_created_at, + auth_app_configuration_id: auth_app_configuration_id, + piv_cac_configuration_id: piv_cac_configuration_id, + key_id: key_id, + webauthn_configuration_id: webauthn_configuration_id, + confirmation_for_add_phone: confirmation_for_add_phone, + phone_configuration_id: phone_configuration_id, + pii_like_keypaths: pii_like_keypaths, + area_code: area_code, + country_code: country_code, + phone_fingerprint: phone_fingerprint, + frontend_error:, + **extra, ) end diff --git a/app/services/reporting/total_user_count_report.rb b/app/services/reporting/total_user_count_report.rb index 7d372e282c1..d7a875ad60c 100644 --- a/app/services/reporting/total_user_count_report.rb +++ b/app/services/reporting/total_user_count_report.rb @@ -60,13 +60,13 @@ def total_user_count def verified_user_count Reports::BaseReport.transaction_with_timeout do - Profile.where(active: true).where('verified_at <= ?', end_date).count + Profile.where(active: true).where('activated_at <= ?', end_date).count end end def new_verified_user_count Reports::BaseReport.transaction_with_timeout do - Profile.where(active: true).where(verified_at: current_month).count + Profile.where(active: true).where(activated_at: current_month).count end end @@ -79,7 +79,7 @@ def annual_total_user_count def annual_verified_user_count Reports::BaseReport.transaction_with_timeout do Profile.where(active: true). - where(verified_at: annual_start_date..annual_end_date). + where(activated_at: annual_start_date..annual_end_date). count end end diff --git a/app/services/saml_request_validator.rb b/app/services/saml_request_validator.rb index cff2945dbf3..2b21aadc0bc 100644 --- a/app/services/saml_request_validator.rb +++ b/app/services/saml_request_validator.rb @@ -7,7 +7,7 @@ class SamlRequestValidator validate :authorized_service_provider validate :authorized_authn_context validate :parsable_vtr - validate :authorized_nameid_format + validate :authorized_email_nameid_format def initialize(blank_cert: false) @blank_cert = blank_cert @@ -43,7 +43,7 @@ def parsed_vectors_of_trust return @parsed_vectors_of_trust if defined?(@parsed_vectors_of_trust) @parsed_vectors_of_trust = begin - if vtr.present? + if vtr.is_a?(Array) && !vtr.empty? vtr.map { |vot| Vot::Parser.new(vector_of_trust: vot).parse } end rescue Vot::Parser::ParseException @@ -119,24 +119,17 @@ def ial_max_requested? Array(authn_context).include?(Saml::Idp::Constants::IALMAX_AUTHN_CONTEXT_CLASSREF) end - def authorized_nameid_format - return if satisfiable_nameid_format? - return if email_nameid_format? && service_provider&.email_nameid_format_allowed - return if legacy_name_id_behavior_needed? + def authorized_email_nameid_format + return unless email_nameid_format? + return if service_provider&.email_nameid_format_allowed errors.add(:nameid_format, :unauthorized_nameid_format, type: :unauthorized_nameid_format) end - def satisfiable_nameid_format? - nameid_format.nil? || [Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT, - Saml::Idp::Constants::NAME_ID_FORMAT_UNSPECIFIED].include?(nameid_format) - end - - def legacy_name_id_behavior_needed? - service_provider&.use_legacy_name_id_behavior && !email_nameid_format? - end - def email_nameid_format? - nameid_format == Saml::Idp::Constants::NAME_ID_FORMAT_EMAIL + [ + 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress', + 'urn:oasis:names:tc:SAML:2.0:nameid-format:emailAddress', + ].include?(nameid_format) end end diff --git a/app/services/vot/component_expander.rb b/app/services/vot/component_expander.rb deleted file mode 100644 index 12420b94d09..00000000000 --- a/app/services/vot/component_expander.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -module Vot - class ComponentExpander - attr_reader :initial_components, :component_map, :expanded_components - - def initialize(initial_components:, component_map:) - @initial_components = initial_components - @component_map = component_map - @expanded_components = [] - end - - def expand - initial_components.each do |component| - expand_and_add_component(component) - end - expanded_components.sort_by(&:name) - end - - private - - def expand_and_add_component(component) - # Do not add components if we have alread expanded and added them. - # This prevents infinite recursion. - return if expanded_components.include?(component) - - expanded_components.push(component) - component.implied_component_values.each do |implied_component_name| - implied_component = component_map[implied_component_name] - expand_and_add_component(implied_component) - end - end - end -end diff --git a/app/services/vot/parser.rb b/app/services/vot/parser.rb index 08586cb43f3..004d77c3a6f 100644 --- a/app/services/vot/parser.rb +++ b/app/services/vot/parser.rb @@ -44,53 +44,52 @@ def initialize(vector_of_trust: nil, acr_values: nil) end def parse - if initial_components.blank? + initial_components = + if vector_of_trust.present? + map_initial_vector_of_trust_components_to_component_values + elsif acr_values.present? + map_initial_acr_values_to_component_values + end + + if !initial_components raise ParseException.new('VoT parser called without VoT or ACR values') end - validate_component_uniqueness!(initial_components) - - expanded_components = Vot::ComponentExpander.new(initial_components:, component_map:).expand - requirement_list = expanded_components.flat_map(&:requirements) - Result.new( - component_values: expanded_components, - aal2?: requirement_list.include?(:aal2), - phishing_resistant?: requirement_list.include?(:phishing_resistant), - hspd12?: requirement_list.include?(:hspd12), - identity_proofing?: requirement_list.include?(:identity_proofing), - biometric_comparison?: requirement_list.include?(:biometric_comparison), - ialmax?: requirement_list.include?(:ialmax), - enhanced_ipp?: requirement_list.include?(:enhanced_ipp), - ) + expand_components_with_initial_components(initial_components) end private - def initial_components - return @initial_components if defined?(@initial_components) - - component_string = vector_of_trust.presence || acr_values || '' - @initial_components ||= component_string.split(component_separator).map do |component_name| - component_map.fetch(component_name) + def map_initial_vector_of_trust_components_to_component_values + vector_of_trust.split('.').map do |component_value_name| + SupportedComponentValues.by_name.fetch(component_value_name) rescue KeyError - raise_unsupported_component_exception(component_name) + raise_unsupported_component_exception(component_value_name) end end - def component_separator - if vector_of_trust.present? - '.' - else - ' ' + def map_initial_acr_values_to_component_values + acr_values.split(' ').map do |component_value_name| + LegacyComponentValues.by_name.fetch(component_value_name) + rescue KeyError + raise_unsupported_component_exception(component_value_name) end end - def component_map - if vector_of_trust.present? - SupportedComponentValues.by_name - else - LegacyComponentValues.by_name - end + def expand_components_with_initial_components(initial_components) + validate_component_uniqueness!(initial_components) + resulting_components = add_implied_components(initial_components).sort_by(&:name) + requirement_list = resulting_components.flat_map(&:requirements) + Result.new( + component_values: resulting_components, + aal2?: requirement_list.include?(:aal2), + phishing_resistant?: requirement_list.include?(:phishing_resistant), + hspd12?: requirement_list.include?(:hspd12), + identity_proofing?: requirement_list.include?(:identity_proofing), + biometric_comparison?: requirement_list.include?(:biometric_comparison), + ialmax?: requirement_list.include?(:ialmax), + enhanced_ipp?: requirement_list.include?(:enhanced_ipp), + ) end def validate_component_uniqueness!(component_values) @@ -99,6 +98,21 @@ def validate_component_uniqueness!(component_values) end end + def add_implied_components(component_values) + component_values.flat_map do |component_value| + component_with_implied_components(component_value) + end.uniq + end + + def component_with_implied_components(component_value) + [ + component_value, + *component_value.implied_component_values.map do |implied_component_value| + component_with_implied_components(implied_component_value) + end, + ].flatten + end + def raise_unsupported_component_exception(component_value_name) if vector_of_trust.present? raise ParseException, "#{vector_of_trust} contains unkown component #{component_value_name}" diff --git a/app/services/vot/supported_component_values.rb b/app/services/vot/supported_component_values.rb index af7392684b2..a49f23a7d45 100644 --- a/app/services/vot/supported_component_values.rb +++ b/app/services/vot/supported_component_values.rb @@ -11,37 +11,37 @@ module SupportedComponentValues C2 = ComponentValue.new( name: 'C2', description: 'AAL2 conformant features are engaged', - implied_component_values: ['C1'], + implied_component_values: [C1], requirements: [:aal2], ).freeze Ca = ComponentValue.new( name: 'Ca', description: 'A phishing resistant authenticator is required', - implied_component_values: ['C1'], + implied_component_values: [C1], requirements: [:phishing_resistant], ).freeze Cb = ComponentValue.new( name: 'Cb', description: 'A PIV/CAC card is required', - implied_component_values: ['C1'], + implied_component_values: [C1], requirements: [:hspd12], ).freeze P1 = ComponentValue.new( name: 'P1', description: 'Identity proofing is performed', - implied_component_values: ['C2'], + implied_component_values: [C2], requirements: [:identity_proofing], ).freeze Pb = ComponentValue.new( name: 'Pb', description: 'A biometric comparison is required as part of identity proofing', - implied_component_values: ['P1'], + implied_component_values: [P1], requirements: [:biometric_comparison], ).freeze Pe = ComponentValue.new( name: 'Pe', description: 'Enhanced In Person Proofing is required', - implied_component_values: ['P1'], + implied_component_values: [P1], requirements: [:enhanced_ipp], ).freeze diff --git a/app/views/accounts/history/show.html.erb b/app/views/accounts/history/show.html.erb index ff8a227df2c..943233bf3dc 100644 --- a/app/views/accounts/history/show.html.erb +++ b/app/views/accounts/history/show.html.erb @@ -13,7 +13,7 @@ -
+

<%= t('headings.account.activity') %> diff --git a/app/views/idv/how_to_verify/show.html.erb b/app/views/idv/how_to_verify/show.html.erb index 3b8b75993d5..ebda9307adf 100644 --- a/app/views/idv/how_to_verify/show.html.erb +++ b/app/views/idv/how_to_verify/show.html.erb @@ -1,16 +1,5 @@ <% self.title = t('doc_auth.headings.how_to_verify') %> <%= render PageHeadingComponent.new.with_content(t('doc_auth.headings.how_to_verify')) %> - -<% if defined?(error) %> - <%= render AlertComponent.new( - type: :error, - class: 'margin-bottom-4', - text_tag: 'div', - ) do %> - <%= error %> - <% end %> -<% end %> -

<%= @presenter.how_to_verify_info %>

diff --git a/app/views/users/backup_code_setup/edit.html.erb b/app/views/users/backup_code_setup/edit.html.erb index 043c45357b2..580c811f447 100644 --- a/app/views/users/backup_code_setup/edit.html.erb +++ b/app/views/users/backup_code_setup/edit.html.erb @@ -7,7 +7,7 @@ <%= render ButtonComponent.new( url: backup_code_setup_path, - method: :post, + method: IdentityConfig.store.backup_code_confirm_setup_screen_enabled ? :post : :get, big: true, wide: true, class: 'margin-top-3 margin-bottom-2', diff --git a/config/application.yml.default b/config/application.yml.default index 772c6068aee..bbed146510e 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -58,6 +58,7 @@ aws_kms_multi_region_key_id: alias/login-dot-gov-keymaker-multi-region aws_kms_session_key_id: alias/login-dot-gov-test-keymaker aws_logo_bucket: '' aws_region: 'us-west-2' +backup_code_confirm_setup_screen_enabled: true backup_code_cost: '2000$8$1$' broken_personal_key_window_start: '2021-07-29T00:00:00Z' broken_personal_key_window_finish: '2021-09-22T00:00:00Z' @@ -454,6 +455,7 @@ production: attribute_encryption_key_queue: '[]' available_locales: 'en,es,fr' aws_logo_bucket: '' + backup_code_confirm_setup_screen_enabled: false dashboard_api_token: '' dashboard_url: https://dashboard.demo.login.gov database_host: '' diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 87e472b033b..f2f2462fde3 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -74,6 +74,7 @@ def self.store config.add(:aws_kms_session_key_id, type: :string) config.add(:aws_logo_bucket, type: :string) config.add(:aws_region, type: :string) + config.add(:backup_code_confirm_setup_screen_enabled, type: :boolean) config.add(:backup_code_cost, type: :string) config.add(:broken_personal_key_window_finish, type: :timestamp) config.add(:broken_personal_key_window_start, type: :timestamp) diff --git a/lib/reporting/proofing_rate_report.rb b/lib/reporting/proofing_rate_report.rb index da635c9dd52..634cf11eb68 100644 --- a/lib/reporting/proofing_rate_report.rb +++ b/lib/reporting/proofing_rate_report.rb @@ -87,20 +87,12 @@ def to_csv def reports @reports ||= begin sub_reports = [0, *DATE_INTERVALS].each_cons(2).map do |slice_end, slice_start| - time_range = if slice_end.zero? - Range.new( - (end_date - slice_start.days).beginning_of_day, - (end_date - slice_end.days).end_of_day, - ) - else - Range.new( - (end_date - slice_start.days).beginning_of_day, - (end_date - slice_end.days).end_of_day - 1.day, - ) - end Reporting::IdentityVerificationReport.new( issuers: nil, # all issuers - time_range: time_range, + time_range: Range.new( + (end_date - slice_start.days).beginning_of_day, + (end_date - slice_end.days).beginning_of_day, + ), cloudwatch_client: cloudwatch_client, ) end diff --git a/lib/saml_idp_constants.rb b/lib/saml_idp_constants.rb index 46b37ae8bb4..b50d4e02454 100644 --- a/lib/saml_idp_constants.rb +++ b/lib/saml_idp_constants.rb @@ -30,7 +30,6 @@ module Constants NAME_ID_FORMAT_PERSISTENT = 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent' NAME_ID_FORMAT_EMAIL = 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress' - NAME_ID_FORMAT_UNSPECIFIED = 'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified' VALID_NAME_ID_FORMATS = [NAME_ID_FORMAT_PERSISTENT, NAME_ID_FORMAT_EMAIL].freeze REQUESTED_ATTRIBUTES_CLASSREF = 'http://idmanagement.gov/ns/requested_attributes?ReqAttr=' diff --git a/spec/controllers/idv/how_to_verify_controller_spec.rb b/spec/controllers/idv/how_to_verify_controller_spec.rb index 4fe227bb666..3b20836174d 100644 --- a/spec/controllers/idv/how_to_verify_controller_spec.rb +++ b/spec/controllers/idv/how_to_verify_controller_spec.rb @@ -155,29 +155,6 @@ end let(:analytics_name) { :idv_doc_auth_how_to_verify_submitted } - shared_examples_for 'invalid form submissions' do - it 'invalidates future steps' do - expect(subject).to receive(:clear_future_steps!) - - put :update - end - - it 'logs the invalid value and re-renders the page' do - put :update, params: params - - expect(@analytics).to have_received(:track_event).with(analytics_name, analytics_args) - expect(response).to render_template :show - end - - it 'redirects to how_to_verify' do - put :update, params: params - - expect(flash[:error]).not_to be_present - expect(subject.idv_session.skip_doc_auth).to be_nil - expect(subject.idv_session.opted_in_to_in_person_proofing).to be_nil - end - end - context 'no selection made' do let(:analytics_args) do { @@ -191,29 +168,17 @@ }.merge(ab_test_args) end - let(:params) { nil } + it 'invalidates future steps' do + expect(subject).to receive(:clear_future_steps!) - it_behaves_like 'invalid form submissions' - end + put :update + end - context 'an invalid selection is submitted' do - # (This should only be possible if someone alters the form) - let(:selection) { 'carrier_pigeon' } + it 'sends analytics_submitted event when nothing is selected' do + put :update - let(:analytics_args) do - { - step: 'how_to_verify', - analytics_id: 'Doc Auth', - skip_hybrid_handoff: nil, - 'selection' => selection, - irs_reproofing: false, - error_details: { selection: { inclusion: true } }, - errors: { selection: ['Select a way to verify your identity.'] }, - success: false, - }.merge(ab_test_args) + expect(@analytics).to have_received(:track_event).with(analytics_name, analytics_args) end - - it_behaves_like 'invalid form submissions' end context 'remote' do @@ -271,6 +236,34 @@ expect(@analytics).to have_received(:track_event).with(analytics_name, analytics_args) end end + + context 'undo/back' do + it 'sets skip_doc_auth to nil and does not redirect' do + put :update, params: { undo_step: true } + + expect(subject.idv_session.skip_doc_auth).to be_nil + expect(subject.idv_session.skip_doc_auth_from_how_to_verify).to be_nil + expect(subject.idv_session.opted_in_to_in_person_proofing).to be_nil + expect(response).to redirect_to(idv_how_to_verify_url) + end + end + end + + context 'form submission error' do + let(:invalid_params) do + { + idv_how_to_verify_form: { selection: '' }, + } + end + + it 'redirects to how to verify when a form submission error is encountered' do + put :update, params: invalid_params + + expect(flash[:error]).to be_present + expect(subject.idv_session.skip_doc_auth).to be_nil + expect(subject.idv_session.opted_in_to_in_person_proofing).to be_nil + expect(response).to redirect_to(idv_how_to_verify_url) + end end describe '#step_info' do diff --git a/spec/controllers/saml_idp_controller_spec.rb b/spec/controllers/saml_idp_controller_spec.rb index 71e7f0f8fd5..d877c33d893 100644 --- a/spec/controllers/saml_idp_controller_spec.rb +++ b/spec/controllers/saml_idp_controller_spec.rb @@ -133,7 +133,7 @@ # the RubySAML library won't let us pass an empty string in as the certificate # element, so this test substitutes a SAMLRequest that has that element blank let(:blank_cert_element_req) do - <<-XML.gsub(/^[\s]+|[\s]+\n/, '') + <<-XML.gsub(/^[\s\t]*|[\s\t]*\n/, '') http://localhost:3000 @@ -1451,6 +1451,52 @@ def name_id_version(format_urn) end end + context 'service provider uses email NameID format and is allowed to use email' do + let(:user) { create(:user, :fully_registered) } + + before do + settings = saml_settings( + overrides: { + issuer: sp1_issuer, + name_identifier_format: Saml::Idp::Constants::NAME_ID_FORMAT_EMAIL, + }, + ) + ServiceProvider. + find_by(issuer: settings.issuer). + update!(email_nameid_format_allowed: true) + generate_saml_response(user, settings) + end + + # Testing the element when the SP is configured to use a + # NameID format of emailAddress rather than the default persistent UUID. + context 'Subject' do + let(:subject) do + xmldoc.subject_nodeset[0] + end + + it 'has a saml:Subject element' do + expect(subject).to_not be_nil + end + + context 'NameID' do + let(:name_id) { subject.at('//ds:NameID', ds: Saml::XML::Namespaces::ASSERTION) } + + it 'has a saml:NameID element' do + expect(name_id).to_not be_nil + end + + it 'has a format attribute defining the NameID to be email' do + expect(name_id.attributes['Format'].value). + to eq(Saml::Idp::Constants::NAME_ID_FORMAT_EMAIL) + end + + it 'has NameID value of the email address of the user making the AuthN Request' do + expect(name_id.children.first.to_s).to eq(user.email) + end + end + end + end + context 'no matching cert from the SAML request' do let(:user) { create(:user, :fully_registered) } @@ -1513,7 +1559,7 @@ def name_id_version(format_urn) # the RubySAML library won't let us pass an empty string in as the certificate # element, so this test substitutes a SAMLRequest that has that element blank let(:blank_cert_element_req) do - <<-XML.gsub(/^[\s]+|[\s]+\n/, '') + <<-XML.gsub(/^[\s\t]*|[\s\t]*\n/, '') http://localhost:3000 @@ -1627,143 +1673,217 @@ def name_id_version(format_urn) end end - describe 'NameID format' do + context 'nameid_format is missing' do let(:user) { create(:user, :fully_registered) } - let(:subject_element) { xmldoc.subject_nodeset[0] } - let(:name_id) { subject_element.at('//ds:NameID', ds: Saml::XML::Namespaces::ASSERTION) } - let(:auth_settings) { saml_settings(overrides: { name_identifier_format: }) } - let(:name_identifier_format) { nil } - let(:email_allowed) { nil } - let(:use_legacy_name_id_behavior) { nil } before do stub_analytics - service_provider = ServiceProvider.find_by(issuer: auth_settings.issuer) - IdentityLinker.new(user, service_provider).link_identity - service_provider.update!( - use_legacy_name_id_behavior:, - email_nameid_format_allowed: email_allowed, - ) + allow(@analytics).to receive(:track_event) end - shared_examples_for 'sends the UUID' do - it 'sends the UUID' do - generate_saml_response(user, auth_settings) + it 'defaults to persistent' do + auth_settings = saml_settings(overrides: { name_identifier_format: nil }) + service_provider = build(:service_provider, issuer: auth_settings.issuer) + IdentityLinker.new(user, service_provider).link_identity + user.identities.last.update!(verified_attributes: ['email']) + generate_saml_response(user, auth_settings) - expect(response.status).to eq(200) + expect(response.status).to eq(200) - expect(name_id.attributes['Format'].value). - to eq(Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT) + analytics_hash = { + success: true, + errors: {}, + nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT, + authn_context: request_authn_contexts, + authn_context_comparison: 'exact', + requested_ial: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, + service_provider: 'http://localhost:3000', + endpoint: "/api/saml/auth#{path_year}", + idv: false, + finish_profile: false, + request_signed: true, + matching_cert_serial: saml_test_sp_cert_serial, + } - expect(name_id.children.first.to_s).to eq(user.last_identity.uuid) - end + expect(@analytics).to have_received(:track_event). + with('SAML Auth', analytics_hash) end - shared_examples_for 'sends the email' do - it 'sends the email' do - generate_saml_response(user, auth_settings) - - expect(response.status).to eq(200) - - expect(name_id.attributes['Format'].value). - to eq(Saml::Idp::Constants::NAME_ID_FORMAT_EMAIL) + it 'defaults to email when added to issuers_with_email_nameid_format' do + auth_settings = saml_settings( + overrides: { + issuer: sp1_issuer, + name_identifier_format: nil, + }, + ) + ServiceProvider. + find_by(issuer: auth_settings.issuer). + update!(email_nameid_format_allowed: true) + IdentityLinker.new(user, sp1).link_identity + user.identities.last.update!(verified_attributes: ['email']) + generate_saml_response(user, auth_settings) - expect(name_id.children.first.to_s).to eq(user.email) - end - end + expect(response.status).to eq(200) - shared_examples_for 'returns an unauthorized nameid error' do - it 'returns an error' do - generate_saml_response(user, auth_settings) + analytics_hash = { + success: true, + errors: {}, + nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_EMAIL, + authn_context: request_authn_contexts, + authn_context_comparison: 'exact', + requested_ial: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, + service_provider: auth_settings.issuer, + endpoint: "/api/saml/auth#{path_year}", + idv: false, + finish_profile: false, + request_signed: true, + matching_cert_serial: saml_test_sp_cert_serial, + } - expect(controller).to render_template('saml_idp/auth/error') - expect(response.status).to eq(400) - expect(response.body).to include(t('errors.messages.unauthorized_nameid_format')) - end + expect(@analytics).to have_received(:track_event). + with('SAML Auth', analytics_hash) end + end - context 'when the NameID format has the value "unspecified"' do - let(:name_identifier_format) { Saml::Idp::Constants::NAME_ID_FORMAT_UNSPECIFIED } - - context 'when the service provider is not configured with use_legacy_name_id_behavior' do - let(:use_legacy_name_id_behavior) { false } - - it_behaves_like 'sends the UUID' - end - context 'when the service provider is configured with use_legacy_name_id_behavior' do - let(:use_legacy_name_id_behavior) { true } - it 'sends the id, not the UUID' do - generate_saml_response(user, auth_settings) - - expect(response.status).to eq(200) - - expect(name_id.attributes['Format'].value). - to eq(Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT) + context 'service provider uses email NameID format but is not allowed to use email' do + it 'returns an error' do + stub_analytics + allow(@analytics).to receive(:track_event) - expect(name_id.children.first.to_s).to eq(user.id.to_s) - end - end - end + auth_settings = saml_settings( + overrides: { name_identifier_format: Saml::Idp::Constants::NAME_ID_FORMAT_EMAIL }, + ) + saml_get_auth(auth_settings) - context 'when the NameID format is missing' do - let(:name_identifier_format) { nil } + expect(controller).to render_template('saml_idp/auth/error') + expect(response.status).to eq(400) + expect(response.body).to include(t('errors.messages.unauthorized_nameid_format')) - context 'when the service provider is not configured with use_legacy_name_id_behavior' do - let(:use_legacy_name_id_behavior) { false } + analytics_hash = { + success: false, + errors: { nameid_format: [t('errors.messages.unauthorized_nameid_format')] }, + error_details: { nameid_format: { unauthorized_nameid_format: true } }, + nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_EMAIL, + authn_context: request_authn_contexts, + authn_context_comparison: 'exact', + service_provider: 'http://localhost:3000', + request_signed: true, + requested_ial: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, + endpoint: "/api/saml/auth#{path_year}", + idv: false, + finish_profile: false, + matching_cert_serial: saml_test_sp_cert_serial, + } - it_behaves_like 'sends the UUID' - end - context 'when the service provider is configured with use_legacy_name_id_behavior' do - let(:use_legacy_name_id_behavior) { true } - it_behaves_like 'sends the UUID' - end + expect(@analytics).to have_received(:track_event). + with('SAML Auth', analytics_hash) end + end - context 'when the NameID format is "persistent"' do - let(:name_identifier_format) { Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT } + context 'service provider sends unsupported NameID format' do + let(:user) { create(:user, :fully_registered) } + let(:xmldoc) { SamlResponseDoc.new('controller', 'response_assertion', response) } + let(:subject) { xmldoc.subject_nodeset[0] } + let(:name_id) { subject.at('//ds:NameID', ds: Saml::XML::Namespaces::ASSERTION) } - it_behaves_like 'sends the UUID' + before do + stub_analytics + allow(@analytics).to receive(:track_event) end - context 'when the NameID format is "email"' do - let(:name_identifier_format) { Saml::Idp::Constants::NAME_ID_FORMAT_EMAIL } + it 'sends the appropriate identifier for non-email NameID SPs' do + auth_settings = saml_settings(overrides: { name_identifier_format: nil }) + auth_settings.name_identifier_format = + 'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified' + service_provider = build(:service_provider, issuer: auth_settings.issuer) + IdentityLinker.new(user, service_provider).link_identity + user.identities.last.update!(verified_attributes: ['email']) + generate_saml_response(user, auth_settings) - context 'when the service provider is not allowed to use email' do - let(:email_allowed) { false } + expect(response.status).to eq(200) - it_behaves_like 'returns an unauthorized nameid error' - end + analytics_hash = { + success: true, + errors: {}, + nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT, + authn_context: request_authn_contexts, + authn_context_comparison: 'exact', + requested_ial: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, + service_provider: 'http://localhost:3000', + endpoint: "/api/saml/auth#{path_year}", + idv: false, + finish_profile: false, + request_signed: true, + matching_cert_serial: saml_test_sp_cert_serial, + } - context 'when the service provider is allowed to use email' do - let(:email_allowed) { true } - it_behaves_like 'sends the email' - end + expect(name_id.children.first.to_s).to eq(user.agency_identities.last.uuid) + expect(@analytics).to have_received(:track_event). + with('SAML Auth', analytics_hash) end - context 'when the NameID format is an unsupported value' do - let(:name_identifier_format) { 'urn:oasis:names:tc:SAML:1.1:nameid-format:transient' } - let(:use_legacy_name_id_behavior) { nil } + it 'sends the appropriate identifier for email NameID SPs' do + auth_settings = saml_settings(overrides: { name_identifier_format: nil }) + auth_settings.name_identifier_format = + 'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified' + service_provider = ServiceProvider.find_by(issuer: auth_settings.issuer) + service_provider.update!(email_nameid_format_allowed: true) + IdentityLinker.new(user, service_provider).link_identity + user.identities.last.update!(verified_attributes: ['email']) + generate_saml_response(user, auth_settings) - context 'when the service provider is not configured with use_legacy_name_id_behavior' do - let(:use_legacy_name_id_behavior) { false } + expect(response.status).to eq(200) - it_behaves_like 'returns an unauthorized nameid error' - end + analytics_hash = { + success: true, + errors: {}, + nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_EMAIL, + authn_context: request_authn_contexts, + authn_context_comparison: 'exact', + requested_ial: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, + service_provider: auth_settings.issuer, + endpoint: "/api/saml/auth#{path_year}", + idv: false, + finish_profile: false, + request_signed: true, + matching_cert_serial: saml_test_sp_cert_serial, + } - context 'when the service provider is configured with use_legacy_name_id_behavior' do - let(:use_legacy_name_id_behavior) { true } + expect(name_id.children.first.to_s).to eq(user.email_addresses.first.email) + expect(@analytics).to have_received(:track_event). + with('SAML Auth', analytics_hash) + end - it 'sends the id, not the UUID' do - generate_saml_response(user, auth_settings) + it 'sends the old user ID for legacy SPS' do + auth_settings = saml_settings(overrides: { name_identifier_format: nil }) + auth_settings.name_identifier_format = + 'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified' + service_provider = ServiceProvider.find_by(issuer: auth_settings.issuer) + service_provider.update!(use_legacy_name_id_behavior: true) + IdentityLinker.new(user, service_provider).link_identity + user.identities.last.update!(verified_attributes: ['email']) + generate_saml_response(user, auth_settings) - expect(response.status).to eq(200) + expect(response.status).to eq(200) - expect(name_id.attributes['Format'].value). - to eq(Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT) + analytics_hash = { + success: true, + errors: {}, + nameid_format: 'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified', + authn_context: request_authn_contexts, + authn_context_comparison: 'exact', + requested_ial: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, + service_provider: 'http://localhost:3000', + endpoint: "/api/saml/auth#{path_year}", + idv: false, + finish_profile: false, + request_signed: true, + matching_cert_serial: saml_test_sp_cert_serial, + } - expect(name_id.children.first.to_s).to eq(user.id.to_s) - end - end + expect(name_id.children.first.to_s).to eq(user.id.to_s) + expect(@analytics).to have_received(:track_event). + with('SAML Auth', analytics_hash) end end diff --git a/spec/controllers/two_factor_authentication/backup_code_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/backup_code_verification_controller_spec.rb index 65df3035474..8884908d96a 100644 --- a/spec/controllers/two_factor_authentication/backup_code_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/backup_code_verification_controller_spec.rb @@ -27,6 +27,16 @@ sign_in_before_2fa(user) stub_analytics stub_attempts_tracker + analytics_hash = { + success: true, + errors: {}, + multi_factor_auth_method: 'backup_code', + multi_factor_auth_method_created_at: Time.zone.now.strftime('%s%L'), + new_device: true, + } + + expect(@analytics).to receive(:track_mfa_submit_event). + with(analytics_hash) expect(@irs_attempts_api_tracker).to receive(:track_event). with(:mfa_login_backup_code, success: true) @@ -37,15 +47,6 @@ post :create, params: payload - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - success: true, - errors: {}, - multi_factor_auth_method: 'backup_code', - multi_factor_auth_method_created_at: Time.zone.now.strftime('%s%L'), - new_device: true, - ) - expect(subject.user_session[:auth_events]).to eq( [ auth_method: TwoFactorAuthenticatable::AuthMethod::BACKUP_CODE, @@ -92,23 +93,22 @@ stub_analytics stub_attempts_tracker + expect(@analytics).to receive(:track_mfa_submit_event). + with({ + success: true, + errors: {}, + multi_factor_auth_method: 'backup_code', + multi_factor_auth_method_created_at: Time.zone.now.strftime('%s%L'), + new_device: true, + }) + expect(@irs_attempts_api_tracker).to receive(:track_event). with(:mfa_login_backup_code, success: true) - post :create, params: payload + expect(@analytics).to receive(:track_event). + with('User marked authenticated', authentication_type: :valid_2fa) - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - success: true, - errors: {}, - multi_factor_auth_method: 'backup_code', - multi_factor_auth_method_created_at: Time.zone.now.strftime('%s%L'), - new_device: true, - ) - expect(@analytics).to have_logged_event( - 'User marked authenticated', - authentication_type: :valid_2fa, - ) + post :create, params: payload end end end @@ -122,12 +122,10 @@ stub_analytics stub_sign_in_before_2fa(user) - post :create, params: payload + expect(@analytics).to receive(:track_mfa_submit_event). + with(hash_including(new_device: false)) - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - hash_including(new_device: false), - ) + post :create, params: payload end end @@ -173,13 +171,26 @@ user.second_factor_attempts_count = IdentityConfig.store.login_otp_confirmation_max_attempts - 1 user.save + properties = { + success: false, + errors: {}, + multi_factor_auth_method: 'backup_code', + multi_factor_auth_method_created_at: nil, + new_device: true, + } stub_analytics stub_attempts_tracker + expect(@analytics).to receive(:track_mfa_submit_event). + with(properties) + expect(@irs_attempts_api_tracker).to receive(:track_event). with(:mfa_login_backup_code, success: false) + expect(@analytics).to receive(:track_event). + with('Multi-Factor Authentication: max attempts reached') + expect(@irs_attempts_api_tracker).to receive(:mfa_login_rate_limited). with(mfa_device_type: 'backup_code') @@ -187,15 +198,6 @@ with(PushNotification::MfaLimitAccountLockedEvent.new(user: subject.current_user)) post :create, params: payload - - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - success: false, - errors: {}, - multi_factor_auth_method: 'backup_code', - new_device: true, - ) - expect(@analytics).to have_logged_event('Multi-Factor Authentication: max attempts reached') end it 'records unsuccessful 2fa event' 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 0b2e8d4a6ab..2cc1a037dfa 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -122,43 +122,41 @@ end describe '#create' do - let(:user) { create(:user, :with_phone) } - let(:parsed_phone) { Phonelib.parse(user.default_phone_configuration.phone) } + let(:parsed_phone) { Phonelib.parse(controller.current_user.default_phone_configuration.phone) } context 'when the user enters an invalid OTP during authentication context' do subject(:response) { post :create, params: { code: '12345', otp_delivery_preference: 'sms' } } before do - sign_in_before_2fa(user) + sign_in_before_2fa controller.user_session[:mfa_selections] = ['sms'] expect(controller.current_user.reload.second_factor_attempts_count).to eq 0 + phone_configuration_created_at = controller.current_user. + default_phone_configuration.created_at - stub_analytics - stub_attempts_tracker - - expect(@irs_attempts_api_tracker).to receive(:mfa_login_phone_otp_submitted). - with({ reauthentication: false, success: false }) - end - - it 'logs analytics' do - response - - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', + properties = { success: false, error_details: { code: { wrong_length: true, incorrect: true } }, confirmation_for_add_phone: false, context: 'authentication', multi_factor_auth_method: 'sms', - multi_factor_auth_method_created_at: user.default_phone_configuration.created_at. - strftime('%s%L'), + multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'), new_device: true, - phone_configuration_id: user.default_phone_configuration.id, + phone_configuration_id: controller.current_user.default_phone_configuration.id, area_code: parsed_phone.area_code, country_code: parsed_phone.country, phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), enabled_mfa_methods_count: 1, in_account_creation_flow: false, - ) + } + + stub_analytics + stub_attempts_tracker + + expect(@analytics).to receive(:track_mfa_submit_event). + with(properties) + + expect(@irs_attempts_api_tracker).to receive(:mfa_login_phone_otp_submitted). + with({ reauthentication: false, success: false }) end it 'increments second_factor_attempts_count' do @@ -193,7 +191,7 @@ context 'when the user enters an invalid OTP during reauthentication context' do it 'increments second_factor_attempts_count' do - sign_in_before_2fa(user) + sign_in_before_2fa controller.user_session[:context] = 'reauthentication' post :create, params: { code: '12345', otp_delivery_preference: 'sms' } @@ -203,23 +201,42 @@ end context 'when the user has reached the max number of OTP attempts' do - let(:user) do - create( + it 'tracks the event' do + user = create( :user, :fully_registered, - :with_phone, second_factor_attempts_count: IdentityConfig.store.login_otp_confirmation_max_attempts - 1, ) - end - - it 'tracks the event' do sign_in_before_2fa(user) controller.user_session[:mfa_selections] = ['sms'] + phone_configuration_created_at = controller.current_user. + default_phone_configuration.created_at + + properties = { + success: false, + error_details: { code: { wrong_length: true, incorrect: true } }, + confirmation_for_add_phone: false, + context: 'authentication', + multi_factor_auth_method: 'sms', + multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'), + new_device: true, + phone_configuration_id: controller.current_user.default_phone_configuration.id, + area_code: parsed_phone.area_code, + country_code: parsed_phone.country, + phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), + enabled_mfa_methods_count: 1, + in_account_creation_flow: false, + } stub_analytics stub_attempts_tracker + expect(@analytics).to receive(:track_mfa_submit_event). + with(properties) + + expect(@analytics).to receive(:track_event). + with('Multi-Factor Authentication: max attempts reached') expect(PushNotification::HttpPush).to receive(:deliver). with(PushNotification::MfaLimitAccountLockedEvent.new(user: controller.current_user)) @@ -229,32 +246,15 @@ expect(@irs_attempts_api_tracker).to receive(:mfa_login_rate_limited). with(mfa_device_type: 'otp') - post :create, params: { code: '12345', otp_delivery_preference: 'sms' } - - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - success: false, - error_details: { code: { wrong_length: true, incorrect: true } }, - confirmation_for_add_phone: false, - context: 'authentication', - multi_factor_auth_method: 'sms', - multi_factor_auth_method_created_at: user.default_phone_configuration.created_at. - strftime('%s%L'), - new_device: true, - phone_configuration_id: user.default_phone_configuration.id, - area_code: parsed_phone.area_code, - country_code: parsed_phone.country, - phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), - enabled_mfa_methods_count: 1, - in_account_creation_flow: false, - ) - expect(@analytics).to have_logged_event('Multi-Factor Authentication: max attempts reached') + post :create, params: + { code: '12345', + otp_delivery_preference: 'sms' } end end context 'when the user enters a valid OTP' do before do - sign_in_before_2fa(user) + sign_in_before_2fa subject.user_session[:mfa_selections] = ['sms'] expect(subject.current_user.reload.second_factor_attempts_count).to eq 0 end @@ -279,9 +279,31 @@ end it 'tracks the valid authentication event' do + phone_configuration_created_at = controller.current_user. + default_phone_configuration.created_at + properties = { + success: true, + confirmation_for_add_phone: false, + context: 'authentication', + multi_factor_auth_method: 'sms', + multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'), + new_device: true, + phone_configuration_id: controller.current_user.default_phone_configuration.id, + area_code: parsed_phone.area_code, + country_code: parsed_phone.country, + phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), + enabled_mfa_methods_count: 1, + in_account_creation_flow: false, + } + stub_analytics stub_attempts_tracker + expect(@analytics).to receive(:track_mfa_submit_event). + with(properties) + expect(@analytics).to receive(:track_event). + with('User marked authenticated', authentication_type: :valid_2fa) + expect(@irs_attempts_api_tracker).to receive(:mfa_login_phone_otp_submitted). with(reauthentication: false, success: true) @@ -305,27 +327,6 @@ ) expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq false end - - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - success: true, - confirmation_for_add_phone: false, - context: 'authentication', - multi_factor_auth_method: 'sms', - multi_factor_auth_method_created_at: user.default_phone_configuration.created_at. - strftime('%s%L'), - new_device: true, - phone_configuration_id: user.default_phone_configuration.id, - area_code: parsed_phone.area_code, - country_code: parsed_phone.country, - phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), - enabled_mfa_methods_count: 1, - in_account_creation_flow: false, - ) - expect(@analytics).to have_logged_event( - 'User marked authenticated', - authentication_type: :valid_2fa, - ) end context 'with existing device' do @@ -336,15 +337,13 @@ it 'tracks new device value' do stub_analytics + expect(@analytics).to receive(:track_mfa_submit_event). + with(hash_including(new_device: false)) + post :create, params: { code: subject.current_user.reload.direct_otp, otp_delivery_preference: 'sms', } - - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - hash_including(new_device: false), - ) end end @@ -403,7 +402,7 @@ context 'when the user lockout period expires' do before do - sign_in_before_2fa(user) + sign_in_before_2fa lockout_period = IdentityConfig.store.lockout_period_in_minutes.minutes subject.current_user.update( second_factor_locked_at: Time.zone.now - lockout_period - 1.second, diff --git a/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb index a244e28ee70..2baef2c7369 100644 --- a/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb @@ -47,11 +47,29 @@ let(:payload) { { personal_key_form: personal_key } } it 'tracks the valid authentication event' do personal_key - multi_factor_auth_method_created_at = user.reload. - encrypted_recovery_code_digest_generated_at.strftime('%s%L') sign_in_before_2fa(user) stub_analytics + analytics_hash = { + success: true, + errors: {}, + multi_factor_auth_method: 'personal-key', + multi_factor_auth_method_created_at: user.reload. + encrypted_recovery_code_digest_generated_at.strftime('%s%L'), + new_device: true, + } + + expect(@analytics).to receive(:track_mfa_submit_event). + with(analytics_hash) + + expect(@analytics).to receive(:track_event).with( + 'Personal key: Alert user about sign in', + hash_including(emails: 1, sms_message_ids: ['fake-message-id']), + ) + + expect(@analytics).to receive(:track_event). + with('User marked authenticated', authentication_type: :valid_2fa) + expect(controller).to receive(:handle_valid_verification_for_authentication_context). with(auth_method: TwoFactorAuthenticatable::AuthMethod::PERSONAL_KEY). and_call_original @@ -67,23 +85,6 @@ ) expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq false end - - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - success: true, - errors: {}, - multi_factor_auth_method: 'personal-key', - multi_factor_auth_method_created_at:, - new_device: true, - ) - expect(@analytics).to have_logged_event( - 'Personal key: Alert user about sign in', - hash_including(emails: 1, sms_message_ids: ['fake-message-id']), - ) - expect(@analytics).to have_logged_event( - 'User marked authenticated', - authentication_type: :valid_2fa, - ) end context 'with enable_additional_mfa_redirect_for_personal_key_mfa? set to true' do @@ -121,12 +122,10 @@ stub_analytics stub_sign_in_before_2fa(user) - post :create, params: payload + expect(@analytics).to receive(:track_mfa_submit_event). + with(hash_including(new_device: false)) - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - hash_including(new_device: false), - ) + post :create, params: payload end end end @@ -204,6 +203,20 @@ stub_analytics stub_attempts_tracker + properties = { + success: false, + errors: { personal_key: [t('errors.messages.personal_key_incorrect')] }, + error_details: { personal_key: { personal_key_incorrect: true } }, + multi_factor_auth_method: 'personal-key', + multi_factor_auth_method_created_at: personal_key_generated_at.strftime('%s%L'), + new_device: true, + } + + expect(@analytics).to receive(:track_mfa_submit_event). + with(properties) + expect(@analytics).to receive(:track_event). + with('Multi-Factor Authentication: max attempts reached') + expect(@irs_attempts_api_tracker).to receive(:mfa_login_rate_limited). with(mfa_device_type: 'personal_key') @@ -211,17 +224,6 @@ with(PushNotification::MfaLimitAccountLockedEvent.new(user: subject.current_user)) post :create, params: payload - - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - success: false, - errors: { personal_key: [t('errors.messages.personal_key_incorrect')] }, - error_details: { personal_key: { personal_key_incorrect: true } }, - multi_factor_auth_method: 'personal-key', - multi_factor_auth_method_created_at: personal_key_generated_at.strftime('%s%L'), - new_device: true, - ) - expect(@analytics).to have_logged_event('Multi-Factor Authentication: max attempts reached') end it 'records unsuccessful 2fa event' do 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 4e0a56e92a2..e62ea8697cd 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 @@ -105,26 +105,17 @@ stub_attempts_tracker cfg = controller.current_user.piv_cac_configurations.first - expect(@irs_attempts_api_tracker).to receive(:mfa_login_piv_cac).with( - success: true, - subject_dn: x509_subject, - ) - - expect(controller).to receive(:handle_valid_verification_for_authentication_context). - with(auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC). - and_call_original - - get :show, params: { token: 'good-token' } - - expect(@analytics).to have_logged_event( - :multi_factor_auth_enter_piv_cac, + attributes = { context: 'authentication', multi_factor_auth_method: 'piv_cac', new_device: true, piv_cac_configuration_id: nil, - ) - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', + } + + expect(@analytics).to receive(:track_event). + with(:multi_factor_auth_enter_piv_cac, attributes) + + submit_attributes = { success: true, errors: {}, context: 'authentication', @@ -134,11 +125,23 @@ piv_cac_configuration_id: cfg.id, piv_cac_configuration_dn_uuid: cfg.x509_dn_uuid, key_id: 'foo', + } + expect(@analytics).to receive(:track_mfa_submit_event). + with(submit_attributes) + + expect(@irs_attempts_api_tracker).to receive(:mfa_login_piv_cac).with( + success: true, + subject_dn: x509_subject, ) - expect(@analytics).to have_logged_event( - 'User marked authenticated', - authentication_type: :valid_2fa, - ) + + expect(@analytics).to receive(:track_event). + with('User marked authenticated', authentication_type: :valid_2fa) + + expect(controller).to receive(:handle_valid_verification_for_authentication_context). + with(auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC). + and_call_original + + get :show, params: { token: 'good-token' } end context 'with existing device' do @@ -150,12 +153,10 @@ stub_analytics stub_sign_in_before_2fa(user) - get :show, params: { token: 'good-token' } + expect(@analytics).to receive(:track_mfa_submit_event). + with(hash_including(new_device: false)) - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - hash_including(new_device: false), - ) + get :show, params: { token: 'good-token' } end end end @@ -239,39 +240,46 @@ stub_analytics stub_attempts_tracker + attributes = { + context: 'authentication', + multi_factor_auth_method: 'piv_cac', + new_device: true, + piv_cac_configuration_id: nil, + } + + expect(@analytics).to receive(:track_event). + with(:multi_factor_auth_enter_piv_cac, attributes) + expect(@irs_attempts_api_tracker).to receive(:mfa_login_rate_limited). with(mfa_device_type: 'piv_cac') piv_cac_mismatch = { type: 'user.piv_cac_mismatch' } + submit_attributes = { + success: false, + errors: piv_cac_mismatch, + context: 'authentication', + multi_factor_auth_method: 'piv_cac', + multi_factor_auth_method_created_at: nil, + new_device: true, + key_id: 'foo', + piv_cac_configuration_dn_uuid: 'bad-uuid', + piv_cac_configuration_id: nil, + } + expect(@analytics).to receive(:track_mfa_submit_event). + with(submit_attributes) + expect(@irs_attempts_api_tracker).to receive(:mfa_login_piv_cac).with( success: false, subject_dn: bad_dn, ) + expect(@analytics).to receive(:track_event). + with('Multi-Factor Authentication: max attempts reached') expect(PushNotification::HttpPush).to receive(:deliver). with(PushNotification::MfaLimitAccountLockedEvent.new(user: subject.current_user)) get :show, params: { token: 'bad-token' } - - expect(@analytics).to have_logged_event( - :multi_factor_auth_enter_piv_cac, - context: 'authentication', - multi_factor_auth_method: 'piv_cac', - new_device: true, - piv_cac_configuration_id: nil, - ) - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - success: false, - errors: piv_cac_mismatch, - context: 'authentication', - multi_factor_auth_method: 'piv_cac', - new_device: true, - key_id: 'foo', - piv_cac_configuration_dn_uuid: 'bad-uuid', - ) - expect(@analytics).to have_logged_event('Multi-Factor Authentication: max attempts reached') end end diff --git a/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb index e2ec77add5c..5cc253b1c51 100644 --- a/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb @@ -45,6 +45,18 @@ it 'tracks the valid authentication event' do cfg = controller.current_user.auth_app_configurations.first + attributes = { + success: true, + errors: {}, + multi_factor_auth_method: 'totp', + multi_factor_auth_method_created_at: cfg.created_at.strftime('%s%L'), + new_device: true, + auth_app_configuration_id: controller.current_user.auth_app_configurations.first.id, + } + expect(@analytics).to receive(:track_mfa_submit_event). + with(attributes) + expect(@analytics).to receive(:track_event). + with('User marked authenticated', authentication_type: :valid_2fa) expect(@irs_attempts_api_tracker).to receive(:track_event). with(:mfa_login_totp, success: true) expect(controller).to receive(:handle_valid_verification_for_authentication_context). @@ -52,20 +64,6 @@ and_call_original post :create, params: { code: generate_totp_code(@secret) } - - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - success: true, - errors: {}, - multi_factor_auth_method: 'totp', - multi_factor_auth_method_created_at: cfg.created_at.strftime('%s%L'), - new_device: true, - auth_app_configuration_id: controller.current_user.auth_app_configurations.first.id, - ) - expect(@analytics).to have_logged_event( - 'User marked authenticated', - authentication_type: :valid_2fa, - ) end context 'with existing device' do @@ -76,12 +74,10 @@ it 'tracks new device value' do stub_analytics - post :create, params: { code: generate_totp_code(@secret) } + expect(@analytics).to receive(:track_mfa_submit_event). + with(hash_including(new_device: false)) - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - hash_including(new_device: false), - ) + post :create, params: { code: generate_totp_code(@secret) } end end end @@ -92,26 +88,18 @@ app1 = Db::AuthAppConfiguration.create(user, user.generate_totp_secret, nil, 'foo') app2 = Db::AuthAppConfiguration.create(user, user.generate_totp_secret, nil, 'bar') + expect(@analytics).to receive(:track_event). + with('User marked authenticated', authentication_type: :valid_2fa).twice + sign_in_as_user(user) post :create, params: { code: generate_totp_code(app1.otp_secret_key) } expect(response).to redirect_to account_url - expect(@analytics).to have_logged_event( - 'User marked authenticated', - authentication_type: :valid_2fa, - ) - @analytics.events.clear - sign_out(user) sign_in_as_user(user) post :create, params: { code: generate_totp_code(app2.otp_secret_key) } expect(response).to redirect_to account_url - - expect(@analytics).to have_logged_event( - 'User marked authenticated', - authentication_type: :valid_2fa, - ) end end @@ -167,6 +155,19 @@ @secret = user.generate_totp_secret Db::AuthAppConfiguration.create(user, @secret, nil, 'foo') + attributes = { + success: false, + errors: {}, + multi_factor_auth_method: 'totp', + multi_factor_auth_method_created_at: nil, + new_device: true, + auth_app_configuration_id: nil, + } + + expect(@analytics).to receive(:track_mfa_submit_event). + with(attributes) + expect(@analytics).to receive(:track_event). + with('Multi-Factor Authentication: max attempts reached') expect(@irs_attempts_api_tracker).to receive(:track_event). with(:mfa_login_totp, success: false) @@ -177,15 +178,6 @@ with(PushNotification::MfaLimitAccountLockedEvent.new(user: subject.current_user)) post :create, params: { code: '12345' } - - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - success: false, - errors: {}, - multi_factor_auth_method: 'totp', - new_device: true, - ) - expect(@analytics).to have_logged_event('Multi-Factor Authentication: max attempts reached') end end diff --git a/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb index 7d1ecf24292..802b619982b 100644 --- a/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb @@ -133,12 +133,27 @@ ) controller.current_user.webauthn_configurations.first end + let(:result) do + { + context: 'authentication', + multi_factor_auth_method: 'webauthn', + success: true, + webauthn_configuration_id: webauthn_configuration.id, + multi_factor_auth_method_created_at: webauthn_configuration.created_at.strftime('%s%L'), + new_device: true, + } + end before do allow(WebauthnVerificationForm).to receive(:domain_name).and_return('localhost:3000') end it 'tracks a valid submission' do + expect(@analytics).to receive(:track_mfa_submit_event). + with(result) + expect(@analytics).to receive(:track_event). + with('User marked authenticated', authentication_type: :valid_2fa) + expect(@irs_attempts_api_tracker).to receive(:track_event).with( :mfa_login_webauthn_roaming, success: true, @@ -158,20 +173,6 @@ ) expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq false end - - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - context: 'authentication', - multi_factor_auth_method: 'webauthn', - success: true, - webauthn_configuration_id: webauthn_configuration.id, - multi_factor_auth_method_created_at: webauthn_configuration.created_at.strftime('%s%L'), - new_device: true, - ) - expect(@analytics).to have_logged_event( - 'User marked authenticated', - authentication_type: :valid_2fa, - ) end context 'with existing device' do @@ -182,12 +183,10 @@ it 'tracks new device value' do stub_analytics - patch :confirm, params: params + expect(@analytics).to receive(:track_mfa_submit_event). + with(hash_including(new_device: false)) - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - hash_including(new_device: false), - ) + patch :confirm, params: params end end @@ -202,8 +201,14 @@ ) controller.current_user.webauthn_configurations.first end + let(:result) { super().merge(multi_factor_auth_method: 'webauthn_platform') } it 'tracks a valid submission' do + expect(@analytics).to receive(:track_mfa_submit_event). + with(result) + expect(@analytics).to receive(:track_event). + with('User marked authenticated', authentication_type: :valid_2fa) + expect(@irs_attempts_api_tracker).to receive(:track_event).with( :mfa_login_webauthn_platform, success: true, @@ -221,21 +226,6 @@ false, ) end - - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - context: 'authentication', - multi_factor_auth_method: 'webauthn_platform', - success: true, - webauthn_configuration_id: webauthn_configuration.id, - multi_factor_auth_method_created_at: webauthn_configuration.created_at. - strftime('%s%L'), - new_device: true, - ) - expect(@analytics).to have_logged_event( - 'User marked authenticated', - authentication_type: :valid_2fa, - ) end end end @@ -248,20 +238,19 @@ credential_public_key: credential_public_key, ) webauthn_configuration = controller.current_user.webauthn_configurations.first + result = { context: 'authentication', + multi_factor_auth_method: 'webauthn', + success: false, + error_details: { authenticator_data: { invalid_authenticator_data: true } }, + webauthn_configuration_id: webauthn_configuration.id, + multi_factor_auth_method_created_at: webauthn_configuration.created_at. + strftime('%s%L'), + new_device: true } + expect(@analytics).to receive(:track_mfa_submit_event). + with(result) expect(controller).to receive(:create_user_event).with(:sign_in_unsuccessful_2fa) patch :confirm, params: params - - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', - context: 'authentication', - multi_factor_auth_method: 'webauthn', - success: false, - error_details: { authenticator_data: { invalid_authenticator_data: true } }, - webauthn_configuration_id: webauthn_configuration.id, - multi_factor_auth_method_created_at: webauthn_configuration.created_at.strftime('%s%L'), - new_device: true, - ) end context 'webauthn_platform returns an error from frontend API' do @@ -308,10 +297,7 @@ end it 'logs an event with error details' do - patch :confirm, params: params - - expect(@analytics).to have_logged_event( - 'Multi-Factor Authentication', + expect(@analytics).to receive(:track_mfa_submit_event).with( success: false, error_details: { authenticator_data: { blank: true }, @@ -325,8 +311,11 @@ multi_factor_auth_method_created_at: second_webauthn_platform_configuration.created_at.strftime('%s%L'), new_device: true, + webauthn_configuration_id: nil, frontend_error: webauthn_error, ) + + patch :confirm, params: params end end end diff --git a/spec/controllers/users/backup_code_setup_controller_spec.rb b/spec/controllers/users/backup_code_setup_controller_spec.rb index 20934de2c76..f0c2e12ed12 100644 --- a/spec/controllers/users/backup_code_setup_controller_spec.rb +++ b/spec/controllers/users/backup_code_setup_controller_spec.rb @@ -70,6 +70,33 @@ it_behaves_like 'valid backup codes creation' end + + context 'backup code confirm setup feature disabled' do + before do + allow(IdentityConfig.store).to receive(:backup_code_confirm_setup_screen_enabled). + and_return(false) + end + + it 'redirects to root url' do + expect(response).to redirect_to(root_url) + end + + context 'in multi mfa setup flow' do + before do + allow(controller).to receive(:in_multi_mfa_selection_flow?).and_return(true) + end + + it_behaves_like 'valid backup codes creation' + end + + context 'adding backup codes from account dashboard' do + before do + controller.user_session[:account_redirect_path] = account_path + end + + it_behaves_like 'valid backup codes creation' + end + end end describe '#create' do diff --git a/spec/controllers/users/two_factor_authentication_controller_spec.rb b/spec/controllers/users/two_factor_authentication_controller_spec.rb index 067fcd2d695..5fc618b23fd 100644 --- a/spec/controllers/users/two_factor_authentication_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_controller_spec.rb @@ -595,38 +595,6 @@ def index ) end - it 'sends a 6-digit OTP when the idv_ten_digit_otp A/B test is in progress' do - stub_const( - 'AbTests::IDV_TEN_DIGIT_OTP', - FakeAbTestBucket.new.tap { |ab| ab.assign(@user.uuid => :ten_digit_otp) }, - ) - - sign_in_before_2fa(@user) - subject.user_session[:context] = 'confirmation' - subject.user_session[:unconfirmed_phone] = @unconfirmed_phone - parsed_phone = Phonelib.parse(@unconfirmed_phone) - - allow(Telephony).to receive(:send_confirmation_otp).and_call_original - - get :send_code, params: otp_delivery_form_sms - - expect(Telephony).to have_received(:send_confirmation_otp).with( - otp: subject.current_user.direct_otp, - to: @unconfirmed_phone, - expiration: 10, - channel: :sms, - otp_format: 'digit', - otp_length: '6', - domain: IdentityConfig.store.domain_name, - country_code: 'US', - extra_metadata: { - area_code: parsed_phone.area_code, - phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), - resend: nil, - }, - ) - end - it 'tracks the enrollment attempt event' do sign_in_before_2fa(@user) subject.user_session[:context] = 'confirmation' diff --git a/spec/features/account/backup_codes_spec.rb b/spec/features/account/backup_codes_spec.rb index 478645b6c66..b36e47f4c97 100644 --- a/spec/features/account/backup_codes_spec.rb +++ b/spec/features/account/backup_codes_spec.rb @@ -36,6 +36,29 @@ expect(page).to have_content(t('notices.backup_codes_deleted')) expect(page).to have_current_path(account_two_factor_authentication_path) end + + context 'backup code confirm setup feature disabled' do + before do + allow(IdentityConfig.store).to receive(:backup_code_confirm_setup_screen_enabled). + and_return(false) + end + + it 'allows user to regenerate backup codes' do + expect(page).to have_content(t('account.index.backup_codes_exist')) + old_backup_code = user.backup_code_configurations.sample + click_link t('forms.backup_code.regenerate'), href: backup_code_regenerate_path + click_on t('account.index.backup_code_confirm_regenerate') + + expect(page).to have_current_path(backup_code_setup_path) + expect(page).to have_content(t('forms.backup_code.title')) + expect(BackupCodeConfiguration.where(id: old_backup_code.id).any?).to eq(false) + + click_continue + + expect(page).to have_content(t('notices.backup_codes_configured')) + expect(page).to have_current_path(account_two_factor_authentication_path) + end + end end context 'without backup codes and having another mfa method' do @@ -82,6 +105,34 @@ expect(page).to have_current_path(account_two_factor_authentication_path) expect(page).to have_content(expected_message) end + + context 'backup code confirm setup feature disabled' do + before do + allow(IdentityConfig.store).to receive(:backup_code_confirm_setup_screen_enabled). + and_return(false) + end + + it 'allows user to create backup codes' do + click_on t('forms.backup_code.generate') + + expect(page).to have_current_path(backup_code_setup_path) + + generated_at = user.backup_code_configurations. + order(created_at: :asc).first.created_at. + in_time_zone('UTC') + formatted_generated_at = l(generated_at, format: t('time.formats.event_timestamp')) + + expected_message = "#{t('account.index.backup_codes_exist')} #{formatted_generated_at}" + + expect(page).to have_current_path(backup_code_setup_path) + expect(page).to have_content(t('forms.backup_code.title')) + click_continue + + expect(page).to have_content(t('notices.backup_codes_configured')) + expect(page).to have_current_path(account_two_factor_authentication_path) + expect(page).to have_content(expected_message) + end + end end context 'with only backup codes' do diff --git a/spec/lib/reporting/proofing_rate_report_spec.rb b/spec/lib/reporting/proofing_rate_report_spec.rb index 63e3521c800..0cdf1ed0aa5 100644 --- a/spec/lib/reporting/proofing_rate_report_spec.rb +++ b/spec/lib/reporting/proofing_rate_report_spec.rb @@ -2,7 +2,7 @@ require 'reporting/proofing_rate_report' RSpec.describe Reporting::ProofingRateReport do - let(:end_date) { Date.new(2022, 1, 1).in_time_zone('UTC').end_of_day } + let(:end_date) { Date.new(2022, 1, 1).in_time_zone('UTC').beginning_of_day } let(:parallel) { true } subject(:report) do @@ -25,7 +25,7 @@ successfully_verified_users: 1, idv_doc_auth_rejected: 1, idv_fraud_rejected: 0, - time_range: (end_date - 30.days).beginning_of_day..end_date, + time_range: (end_date - 30.days)..end_date, ), instance_double( 'Reporting::IdentityVerificationReport', @@ -39,7 +39,7 @@ successfully_verified_users: 2, idv_doc_auth_rejected: 1, idv_fraud_rejected: 1, - time_range: (end_date - 60.days).beginning_of_day..end_date, + time_range: (end_date - 60.days)..end_date, ), instance_double( 'Reporting::IdentityVerificationReport', @@ -53,7 +53,7 @@ successfully_verified_users: 3, idv_doc_auth_rejected: 1, idv_fraud_rejected: 2, - time_range: (end_date - 90.days).beginning_of_day..end_date, + time_range: (end_date - 90.days)..end_date, ), ], ) @@ -136,26 +136,26 @@ expect(report.reports.map(&:time_range)).to eq( [ - (end_date - 30.days).beginning_of_day..end_date, - (end_date - 60.days).beginning_of_day..end_date, - (end_date - 90.days).beginning_of_day..end_date, + (end_date - 30.days)..end_date, + (end_date - 60.days)..end_date, + (end_date - 90.days)..end_date, ], ) expect(Reporting::IdentityVerificationReport).to have_received(:new).with( - time_range: (end_date - 30.days).beginning_of_day..end_date, + time_range: (end_date - 30.days)..end_date, issuers: nil, cloudwatch_client: report.cloudwatch_client, ).once expect(Reporting::IdentityVerificationReport).to have_received(:new).with( - time_range: (end_date - 60.days).beginning_of_day..(end_date - 30.days).end_of_day, + time_range: (end_date - 60.days)..(end_date - 30.days), issuers: nil, cloudwatch_client: report.cloudwatch_client, ).once expect(Reporting::IdentityVerificationReport).to have_received(:new).with( - time_range: (end_date - 90.days).beginning_of_day..(end_date - 60.days).end_of_day, + time_range: (end_date - 90.days)..(end_date - 60.days), issuers: nil, cloudwatch_client: report.cloudwatch_client, ).once diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 88ddf55bdb2..67798ff36d4 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1540,6 +1540,55 @@ def it_should_not_send_survey end end + describe '#new_device?' do + let(:user_agent) { 'A computer on the internet' } + let(:ip_address) { '4.4.4.4' } + let(:existing_device_cookie) { 'existing_device_cookie' } + let(:cookie_jar) do + { + device: existing_device_cookie, + }.with_indifferent_access.tap do |cookie_jar| + allow(cookie_jar).to receive(:permanent).and_return({}) + end + end + let(:request) do + double( + remote_ip: ip_address, + user_agent: user_agent, + cookie_jar: cookie_jar, + ) + end + let(:user) { create(:user, :fully_registered) } + let(:device) { create(:device, user: user, cookie_uuid: existing_device_cookie) } + + context 'with existing device' do + before do + # Memoize user and device before specs run + user + device + end + it 'does not expect a device to be new' do + cookies = request.cookie_jar + device_present = user.new_device?(cookie_uuid: cookies[:device]) + expect(device_present).to eq(false) + end + end + + context 'with new device' do + let(:device) { create(:device, user: user, cookie_uuid: 'non_existing_device_cookie') } + before do + # Memoize user and device before specs run + user + device + end + it 'expects a new device' do + cookies = request.cookie_jar + device_present = user.new_device?(cookie_uuid: cookies[:device]) + expect(device_present).to eq(true) + end + end + end + describe '#authenticated_device?' do let(:user) { create(:user, :fully_registered) } let(:device) { create(:device, user:) } diff --git a/spec/services/reporting/total_user_count_report_spec.rb b/spec/services/reporting/total_user_count_report_spec.rb index 9135cd25e67..b67f7111bb6 100644 --- a/spec/services/reporting/total_user_count_report_spec.rb +++ b/spec/services/reporting/total_user_count_report_spec.rb @@ -77,13 +77,10 @@ context 'with one verified and one non-verified user' do before do - user1 = create(:user) + create(:user) user2 = create(:user) - create(:profile, :active, :verified, user: user1) # MW: The :verified trait doesn't set active: true. This feels confusing. - # user2 active profile but unverified - create(:profile, :active, :verified, user: user2) - user2.profiles.first.deactivate(:password_reset) + create(:profile, :active, user: user2) end let(:expected_total_count) { 2 } let(:expected_verified_count) { 1 } diff --git a/spec/services/vot/component_expander_spec.rb b/spec/services/vot/component_expander_spec.rb deleted file mode 100644 index f66cae695b4..00000000000 --- a/spec/services/vot/component_expander_spec.rb +++ /dev/null @@ -1,68 +0,0 @@ -require 'rails_helper' - -RSpec.describe Vot::ComponentExpander do - context 'with a component with no implied components' do - it 'returns the single component' do - component_a = Vot::ComponentValue.new( - name: 'A1', description: 'Test component', requirements: [], - implied_component_values: [] - ) - component_b = Vot::ComponentValue.new( - name: 'B1', description: 'Test component', requirements: [], - implied_component_values: [] - ) - component_map = { 'A1' => component_a, 'B1' => component_b } - initial_components = [component_a] - - result = described_class.new(component_map:, initial_components:).expand - - expect(result).to eq([component_a]) - end - end - - context 'with a component with several layers of implied components' do - it 'returns the components expanded into an array' do - component_a = Vot::ComponentValue.new( - name: 'A1', description: 'Test component', requirements: [], - implied_component_values: [] - ) - component_b = Vot::ComponentValue.new( - name: 'B1', description: 'Test component', requirements: [], - implied_component_values: ['A1'] - ) - component_c = Vot::ComponentValue.new( - name: 'C1', description: 'Test component', requirements: [], - implied_component_values: ['B1'] - ) - component_map = { 'A1' => component_a, 'B1' => component_b, 'C1' => component_c } - initial_components = [component_c] - - result = described_class.new(component_map:, initial_components:).expand - - expect(result).to eq([component_a, component_b, component_c]) - end - end - - context 'with a component with cyclic implied component relationships' do - it 'returns the components expanded into an array' do - component_a = Vot::ComponentValue.new( - name: 'A1', description: 'Test component', requirements: [], - implied_component_values: ['C1'] - ) - component_b = Vot::ComponentValue.new( - name: 'B1', description: 'Test component', requirements: [], - implied_component_values: ['A1'] - ) - component_c = Vot::ComponentValue.new( - name: 'C1', description: 'Test component', requirements: [], - implied_component_values: ['B1'] - ) - component_map = { 'A1' => component_a, 'B1' => component_b, 'C1' => component_c } - initial_components = [component_c] - - result = described_class.new(component_map:, initial_components:).expand - - expect(result).to eq([component_a, component_b, component_c]) - end - end -end diff --git a/spec/support/fake_analytics.rb b/spec/support/fake_analytics.rb index 63abdd58ca0..efb6adc9e05 100644 --- a/spec/support/fake_analytics.rb +++ b/spec/support/fake_analytics.rb @@ -168,6 +168,10 @@ def track_event(event, attributes = {}) nil end + def track_mfa_submit_event(_attributes) + # no-op + end + def browser_attributes {} end diff --git a/spec/views/users/backup_code_setup/edit.html.erb_spec.rb b/spec/views/users/backup_code_setup/edit.html.erb_spec.rb index 7db44566d88..cd2c972ec50 100644 --- a/spec/views/users/backup_code_setup/edit.html.erb_spec.rb +++ b/spec/views/users/backup_code_setup/edit.html.erb_spec.rb @@ -13,4 +13,18 @@ it 'has a link to cancel and return to account page' do expect(rendered).to have_link(t('links.cancel'), href: account_path) end + + context 'backup code confirm setup feature disabled' do + before do + allow(IdentityConfig.store).to receive(:backup_code_confirm_setup_screen_enabled). + and_return(false) + end + + it 'has a link to confirm and proceed to setup' do + expect(rendered).to have_link( + t('account.index.backup_code_confirm_regenerate'), + href: backup_code_setup_path, + ) + end + end end diff --git a/yarn.lock b/yarn.lock index 70ab8487714..a7b39fd89e8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4665,10 +4665,10 @@ levn@^0.4.1: prelude-ls "^1.2.1" type-check "~0.4.0" -libphonenumber-js@^1.11.2: - version "1.11.2" - resolved "https://registry.yarnpkg.com/libphonenumber-js/-/libphonenumber-js-1.11.2.tgz#9ddd7d1a1e1be0e7c596c7e09487c362b4f1210c" - integrity sha512-V9mGLlaXN1WETzqQvSu6qf6XVAr3nFuJvWsHcuzCCCo6xUKawwSxOPTpan5CGOSKTn5w/bQuCZcLPJkyysgC3w== +libphonenumber-js@^1.11.1: + version "1.11.1" + resolved "https://registry.yarnpkg.com/libphonenumber-js/-/libphonenumber-js-1.11.1.tgz#2596683e1876bfee74082bb49339fe0a85ae34f9" + integrity sha512-Wze1LPwcnzvcKGcRHFGFECTaLzxOtujwpf924difr5zniyYv1C2PiW0419qDR7m8lKDxsImu5mwxFuXhXpjmvw== lightningcss-darwin-arm64@1.23.0: version "1.23.0"