Merge pull request #10924 from 18F/stages/rc-2024-07-09

Deploy RC 395 to Production

app/controllers/concerns/idv_session_concern.rb

@@ -61,6 +61,6 @@ def idv_session_user
 
   def user_needs_biometric_comparison?
     resolved_authn_context_result.biometric_comparison? &&
-      !current_user.identity_verified_with_biometric_comparison?
+      !idv_session_user.identity_verified_with_biometric_comparison?
   end
 end

app/controllers/concerns/two_factor_authenticatable_methods.rb

@@ -11,6 +11,24 @@ def auth_methods_session
     @auth_methods_session ||= AuthMethodsSession.new(user_session:)
   end
 
+  def handle_verification_for_authentication_context(result:, auth_method:, extra_analytics: nil)
+    analytics.multi_factor_auth(
+      **result.to_h,
+      multi_factor_auth_method: auth_method,
+      enabled_mfa_methods_count: mfa_context.enabled_mfa_methods_count,
+      new_device: new_device?,
+      **extra_analytics.to_h,
+    )
+
+    if result.success?
+      handle_valid_verification_for_authentication_context(auth_method:)
+    else
+      handle_invalid_verification_for_authentication_context
+    end
+  end
+
+  private
+
   def handle_valid_verification_for_authentication_context(auth_method:)
     mark_user_session_authenticated(auth_method:, authentication_type: :valid_2fa)
     disavowal_event, disavowal_token = create_user_event_with_disavowal(:sign_in_after_2fa)
@@ -38,8 +56,6 @@ def handle_valid_verification_for_authentication_context(auth_method:)
     reset_second_factor_attempts_count
   end
 
-  private
-
   def authenticate_user
     authenticate_user!(force: true)
   end
@@ -111,11 +127,7 @@ def handle_remember_device_preference(remember_device_preference)
   # Method will be renamed in the next refactor.
   # You can pass in any "type" with a corresponding I18n key in
   # two_factor_authentication.invalid_#{type}
-  def handle_invalid_otp(type:, context: nil)
-    if context == UserSessionContext::AUTHENTICATION_CONTEXT
-      handle_invalid_verification_for_authentication_context
-    end
-
+  def handle_invalid_otp(type:)
     update_invalid_user
 
     flash.now[:error] = invalid_otp_error(type)

app/controllers/two_factor_authentication/backup_code_verification_controller.rb

@@ -22,7 +22,6 @@ 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?))
       handle_result(result)
     end
 
@@ -48,7 +47,6 @@ def presenter_for_two_factor_authentication_method
     end
 
     def handle_invalid_backup_code
-      handle_invalid_verification_for_authentication_context
       update_invalid_user
 
       flash.now[:error] = t('two_factor_authentication.invalid_backup_code')
@@ -61,11 +59,13 @@ def handle_invalid_backup_code
     end
 
     def handle_result(result)
+      handle_verification_for_authentication_context(
+        result:,
+        auth_method: TwoFactorAuthenticatable::AuthMethod::BACKUP_CODE,
+      )
+
       if result.success?
         handle_remember_device_preference(backup_code_params[:remember_device])
-        handle_valid_verification_for_authentication_context(
-          auth_method: TwoFactorAuthenticatable::AuthMethod::BACKUP_CODE,
-        )
         return handle_last_code if all_codes_used?
         handle_valid_backup_code
       else

app/controllers/two_factor_authentication/otp_verification_controller.rb

@@ -23,21 +23,27 @@ def show
     def create
       result = otp_verification_form.submit
       post_analytics(result)
+
+      if UserSessionContext.authentication_or_reauthentication_context?(context)
+        handle_verification_for_authentication_context(
+          result:,
+          auth_method: params[:otp_delivery_preference],
+          extra_analytics: analytics_properties,
+        )
+      end
+
       if result.success?
         handle_remember_device_preference(params[:remember_device])
 
         if UserSessionContext.confirmation_context?(context)
           handle_valid_confirmation_otp
         else
-          handle_valid_verification_for_authentication_context(
-            auth_method: params[:otp_delivery_preference],
-          )
           redirect_to after_sign_in_path_for(current_user)
         end
 
         reset_otp_session_data
       else
-        handle_invalid_otp(context: context, type: 'otp')
+        handle_invalid_otp(type: 'otp')
       end
     end
 
@@ -133,10 +139,8 @@ def form_params
     end
 
     def post_analytics(result)
-      properties = result.to_h.merge(analytics_properties, new_device: new_device?)
+      properties = result.to_h.merge(analytics_properties)
       analytics.multi_factor_auth_setup(**properties) if context == 'confirmation'
-
-      analytics.multi_factor_auth(**properties)
     end
 
     def analytics_properties

app/controllers/two_factor_authentication/personal_key_verification_controller.rb

@@ -17,25 +17,18 @@ def show
     def create
       @personal_key_form = PersonalKeyForm.new(current_user, personal_key_param)
       result = @personal_key_form.submit
-
-      track_analytics(result)
       handle_result(result)
     end
 
     private
 
-    def track_analytics(result)
+    def analytics_properties
       mfa_created_at = current_user.encrypted_recovery_code_digest_generated_at
-      analytics_hash = result.to_h.merge(
+      {
         multi_factor_auth_method: 'personal-key',
         multi_factor_auth_method_created_at: mfa_created_at&.strftime('%s%L'),
-        new_device: new_device?,
-      )
-
-      analytics.multi_factor_auth(
-        **analytics_hash,
         pii_like_keypaths: [[:errors, :personal_key], [:error_details, :personal_key]],
-      )
+      }
     end
 
     def check_personal_key_enabled
@@ -49,14 +42,20 @@ def presenter_for_two_factor_authentication_method
     end
 
     def handle_result(result)
+      handle_verification_for_authentication_context(
+        result:,
+        auth_method: TwoFactorAuthenticatable::AuthMethod::PERSONAL_KEY,
+        extra_analytics: analytics_properties,
+      )
+
       if result.success?
         _event, disavowal_token = create_user_event_with_disavowal(:personal_key_used)
         alert_user_about_personal_key_sign_in(disavowal_token)
         remove_personal_key
 
         handle_valid_otp
       else
-        handle_invalid_otp(context: context, type: 'personal_key')
+        handle_invalid_otp(type: 'personal_key')
       end
     end
 
@@ -77,9 +76,6 @@ def personal_key_param
     end
 
     def handle_valid_otp
-      handle_valid_verification_for_authentication_context(
-        auth_method: TwoFactorAuthenticatable::AuthMethod::PERSONAL_KEY,
-      )
       if current_user.identity_verified? || current_user.password_reset_profile.present?
         redirect_to manage_personal_key_url
       elsif MfaPolicy.new(current_user).two_factor_enabled? &&

app/controllers/two_factor_authentication/piv_cac_verification_controller.rb

@@ -10,10 +10,10 @@ class PivCacVerificationController < ApplicationController
     before_action :reset_attempt_count_if_user_no_longer_locked_out, only: :show
 
     def show
-      analytics.multi_factor_auth_enter_piv_cac(**analytics_properties)
       if params[:token]
         process_token
       else
+        analytics.multi_factor_auth_enter_piv_cac(**analytics_properties)
         @presenter = presenter_for_two_factor_authentication_method
       end
     end
@@ -30,8 +30,14 @@ 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))
       session[:sign_in_flow] = :sign_in
+
+      handle_verification_for_authentication_context(
+        result:,
+        auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC,
+        extra_analytics: analytics_properties,
+      )
+
       if result.success?
         handle_valid_piv_cac
       else
@@ -47,15 +53,12 @@ def handle_valid_piv_cac
         presented: true,
       )
 
-      handle_valid_verification_for_authentication_context(
-        auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC,
-      )
       redirect_to after_sign_in_path_for(current_user)
     end
 
     def handle_invalid_piv_cac
       clear_piv_cac_information
-      handle_invalid_otp(context: context, type: 'piv_cac')
+      handle_invalid_otp(type: 'piv_cac')
     end
 
     # This overrides the method in TwoFactorAuthenticatable so that we

app/controllers/two_factor_authentication/totp_verification_controller.rb

@@ -21,16 +21,17 @@ 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?))
+
+      handle_verification_for_authentication_context(
+        result:,
+        auth_method: TwoFactorAuthenticatable::AuthMethod::TOTP,
+      )
 
       if result.success?
-        handle_valid_verification_for_authentication_context(
-          auth_method: TwoFactorAuthenticatable::AuthMethod::TOTP,
-        )
         handle_remember_device_preference(params[:remember_device])
         redirect_to after_sign_in_path_for(current_user)
       else
-        handle_invalid_otp(context: context, type: 'totp')
+        handle_invalid_otp(type: 'totp')
       end
     end
 

app/controllers/two_factor_authentication/webauthn_verification_controller.rb

@@ -18,14 +18,6 @@ def show
 
     def confirm
       result = form.submit
-      analytics.multi_factor_auth(
-        **result.to_h,
-        **analytics_properties,
-        multi_factor_auth_method_created_at:
-          webauthn_configuration_or_latest.created_at.strftime('%s%L'),
-        new_device: new_device?,
-      )
-
       handle_webauthn_result(result)
     end
 
@@ -43,6 +35,16 @@ def device_supports_webauthn_platform?
     end
 
     def handle_webauthn_result(result)
+      handle_verification_for_authentication_context(
+        result:,
+        auth_method:,
+        extra_analytics: {
+          **analytics_properties,
+          multi_factor_auth_method_created_at:
+            webauthn_configuration_or_latest.created_at.strftime('%s%L'),
+        },
+      )
+
       if result.success?
         handle_valid_webauthn
       else
@@ -51,21 +53,11 @@ def handle_webauthn_result(result)
     end
 
     def handle_valid_webauthn
-      if form.webauthn_configuration.platform_authenticator
-        handle_valid_verification_for_authentication_context(
-          auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN_PLATFORM,
-        )
-      else
-        handle_valid_verification_for_authentication_context(
-          auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN,
-        )
-      end
       handle_remember_device_preference(params[:remember_device])
       redirect_to after_sign_in_path_for(current_user)
     end
 
     def handle_invalid_webauthn(result)
-      handle_invalid_verification_for_authentication_context
       flash[:error] = result.first_error_message
 
       if platform_authenticator?
@@ -75,6 +67,14 @@ def handle_invalid_webauthn(result)
       end
     end
 
+    def auth_method
+      if platform_authenticator?
+        TwoFactorAuthenticatable::AuthMethod::WEBAUTHN_PLATFORM
+      else
+        TwoFactorAuthenticatable::AuthMethod::WEBAUTHN
+      end
+    end
+
     def confirm_webauthn_enabled
       return if TwoFactorAuthentication::WebauthnPolicy.new(current_user).enabled?
 
@@ -123,7 +123,7 @@ def analytics_properties
     def form
       @form ||= WebauthnVerificationForm.new(
         user: current_user,
-        platform_authenticator: platform_authenticator?,
+        platform_authenticator: platform_authenticator_param?,
         url_options:,
         challenge: user_session[:webauthn_challenge],
         protocol: request.protocol,
@@ -140,10 +140,18 @@ def check_sp_required_mfa
       check_sp_required_mfa_bypass(auth_method: 'webauthn')
     end
 
-    def platform_authenticator?
+    def platform_authenticator_param?
       params[:platform].to_s == 'true'
     end
 
+    def platform_authenticator?
+      if form.webauthn_configuration
+        form.webauthn_configuration.platform_authenticator
+      else
+        platform_authenticator_param?
+      end
+    end
+
     def webauthn_configuration_or_latest
       form.webauthn_configuration || webauthn_configurations.first
     end

app/controllers/users/piv_cac_login_controller.rb

@@ -50,7 +50,7 @@ def process_piv_cac_login
       clear_piv_cac_information
       clear_piv_cac_nonce
       if result.success?
-        process_valid_submission
+        process_valid_submission(result)
       else
         process_invalid_submission
       end
@@ -65,7 +65,7 @@ def piv_cac_login_form
       )
     end
 
-    def process_valid_submission
+    def process_valid_submission(result)
       user = piv_cac_login_form.user
       sign_in(:user, user)
 
@@ -76,7 +76,8 @@ def process_valid_submission
       )
 
       set_new_device_session(nil)
-      handle_valid_verification_for_authentication_context(
+      handle_verification_for_authentication_context(
+        result:,
         auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC,
       )
       redirect_to next_step
@@ -93,7 +94,6 @@ def next_step
     end
 
     def process_invalid_submission
-      handle_invalid_verification_for_authentication_context
       session[:needs_to_setup_piv_cac_after_sign_in] = true if piv_cac_login_form.valid_token?
 
       process_token_with_error