Use NewDeviceConcern consistently for session value

app/controllers/two_factor_authentication/backup_code_verification_controller.rb

@@ -3,6 +3,7 @@
 module TwoFactorAuthentication
   class BackupCodeVerificationController < ApplicationController
     include TwoFactorAuthenticatable
+    include NewDeviceConcern
 
     prepend_before_action :authenticate_user
     before_action :check_sp_required_mfa
@@ -22,7 +23,7 @@ def create
       @backup_code_form = BackupCodeVerificationForm.new(current_user)
       result = @backup_code_form.submit(backup_code_params)
       analytics.track_mfa_submit_event(
-        result.to_h.merge(new_device: user_session[:new_device]),
+        result.to_h.merge(new_device: new_device?),
       )
       irs_attempts_api_tracker.mfa_login_backup_code(success: result.success?)
       handle_result(result)

app/controllers/two_factor_authentication/otp_verification_controller.rb

@@ -4,6 +4,7 @@ module TwoFactorAuthentication
   class OtpVerificationController < ApplicationController
     include TwoFactorAuthenticatable
     include MfaSetupConcern
+    include NewDeviceConcern
 
     before_action :check_sp_required_mfa
     before_action :confirm_multiple_factors_enabled
@@ -132,7 +133,7 @@ def form_params
     end
 
     def post_analytics(result)
-      properties = result.to_h.merge(analytics_properties, new_device: user_session[:new_device])
+      properties = result.to_h.merge(analytics_properties, new_device: new_device?)
       analytics.multi_factor_auth_setup(**properties) if context == 'confirmation'
 
       analytics.track_mfa_submit_event(properties)

app/controllers/two_factor_authentication/personal_key_verification_controller.rb

@@ -3,6 +3,7 @@
 module TwoFactorAuthentication
   class PersonalKeyVerificationController < ApplicationController
     include TwoFactorAuthenticatable
+    include NewDeviceConcern
 
     prepend_before_action :authenticate_user
     before_action :check_personal_key_enabled
@@ -28,7 +29,7 @@ def track_analytics(result)
       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: user_session[:new_device],
+        new_device: new_device?,
       )
 
       analytics.track_mfa_submit_event(analytics_hash)

app/controllers/two_factor_authentication/piv_cac_verification_controller.rb

@@ -4,6 +4,7 @@ module TwoFactorAuthentication
   class PivCacVerificationController < ApplicationController
     include TwoFactorAuthenticatable
     include PivCacConcern
+    include NewDeviceConcern
 
     before_action :confirm_piv_cac_enabled, only: :show
     before_action :reset_attempt_count_if_user_no_longer_locked_out, only: :show
@@ -105,7 +106,7 @@ def analytics_properties
         context: context,
         multi_factor_auth_method: 'piv_cac',
         piv_cac_configuration_id: piv_cac_verification_form&.piv_cac_configuration&.id,
-        new_device: user_session[:new_device],
+        new_device: new_device?,
       }
     end
   end

app/controllers/two_factor_authentication/totp_verification_controller.rb

@@ -3,6 +3,7 @@
 module TwoFactorAuthentication
   class TotpVerificationController < ApplicationController
     include TwoFactorAuthenticatable
+    include NewDeviceConcern
 
     before_action :check_sp_required_mfa
     before_action :confirm_totp_enabled
@@ -20,7 +21,7 @@ def show
 
     def create
       result = TotpVerificationForm.new(current_user, params.require(:code).strip).submit
-      analytics.track_mfa_submit_event(result.to_h.merge(new_device: user_session[: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?

app/controllers/two_factor_authentication/webauthn_verification_controller.rb

@@ -4,6 +4,7 @@ module TwoFactorAuthentication
   # The WebauthnVerificationController class is responsible webauthn verification at sign in
   class WebauthnVerificationController < ApplicationController
     include TwoFactorAuthenticatable
+    include NewDeviceConcern
 
     before_action :check_sp_required_mfa
     before_action :check_if_device_supports_platform_auth, only: :show
@@ -22,7 +23,7 @@ def confirm
         **analytics_properties,
         multi_factor_auth_method_created_at:
           webauthn_configuration_or_latest.created_at.strftime('%s%L'),
-        new_device: user_session[:new_device],
+        new_device: new_device?,
       )
 
       if analytics_properties[:multi_factor_auth_method] == 'webauthn_platform'

spec/controllers/two_factor_authentication/backup_code_verification_controller_spec.rb

@@ -32,7 +32,7 @@
             errors: {},
             multi_factor_auth_method: 'backup_code',
             multi_factor_auth_method_created_at: Time.zone.now.strftime('%s%L'),
-            new_device: nil,
+            new_device: true,
           }
 
           expect(@analytics).to receive(:track_mfa_submit_event).
@@ -99,7 +99,7 @@
               errors: {},
               multi_factor_auth_method: 'backup_code',
               multi_factor_auth_method_created_at: Time.zone.now.strftime('%s%L'),
-              new_device: nil,
+              new_device: true,
             })
 
           expect(@irs_attempts_api_tracker).to receive(:track_event).
@@ -113,37 +113,19 @@
       end
     end
 
-    context 'with new device session value' do
-      it 'tracks new device value' do
-        freeze_time do
-          sign_in_before_2fa(user)
-          subject.user_session[:new_device] = false
-          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: false,
-          }
-
-          expect(@analytics).to receive(:track_mfa_submit_event).
-            with(analytics_hash)
+    context 'with existing device' do
+      before do
+        allow(controller).to receive(:new_device?).and_return(false)
+      end
 
-          expect(@irs_attempts_api_tracker).to receive(:track_event).
-            with(:mfa_login_backup_code, success: true)
+      it 'tracks new device value' do
+        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(subject.user_session[:auth_events]).to eq(
-            [
-              auth_method: TwoFactorAuthenticatable::AuthMethod::BACKUP_CODE,
-              at: Time.zone.now,
-            ],
-          )
-          expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq false
-        end
+        post :create, params: payload
       end
     end
 
@@ -194,7 +176,7 @@
           errors: {},
           multi_factor_auth_method: 'backup_code',
           multi_factor_auth_method_created_at: nil,
-          new_device: nil,
+          new_device: true,
         }
 
         stub_analytics

spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb

@@ -140,7 +140,7 @@
           context: 'authentication',
           multi_factor_auth_method: 'sms',
           multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'),
-          new_device: nil,
+          new_device: true,
           phone_configuration_id: controller.current_user.default_phone_configuration.id,
           area_code: parsed_phone.area_code,
           country_code: parsed_phone.country,
@@ -220,7 +220,7 @@
           context: 'authentication',
           multi_factor_auth_method: 'sms',
           multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'),
-          new_device: nil,
+          new_device: true,
           phone_configuration_id: controller.current_user.default_phone_configuration.id,
           area_code: parsed_phone.area_code,
           country_code: parsed_phone.country,
@@ -287,7 +287,7 @@
           context: 'authentication',
           multi_factor_auth_method: 'sms',
           multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'),
-          new_device: nil,
+          new_device: true,
           phone_configuration_id: controller.current_user.default_phone_configuration.id,
           area_code: parsed_phone.area_code,
           country_code: parsed_phone.country,
@@ -329,37 +329,21 @@
         end
       end
 
-      context 'with new device session value' do
-        it 'tracks new device value' do
-          subject.user_session[:new_device] = false
-          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: false,
-            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,
-          }
+      context 'with existing device' do
+        before do
+          allow(controller).to receive(:new_device?).and_return(false)
+        end
 
+        it 'tracks new device value' do
           stub_analytics
 
           expect(@analytics).to receive(:track_mfa_submit_event).
-            with(properties)
+            with(hash_including(new_device: false))
 
-          freeze_time do
-            post :create, params: {
-              code: subject.current_user.reload.direct_otp,
-              otp_delivery_preference: 'sms',
-            }
-          end
+          post :create, params: {
+            code: subject.current_user.reload.direct_otp,
+            otp_delivery_preference: 'sms',
+          }
         end
       end
 
@@ -512,7 +496,7 @@
               context: 'confirmation',
               multi_factor_auth_method: 'sms',
               multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'),
-              new_device: nil,
+              new_device: true,
               phone_configuration_id: phone_id,
               area_code: parsed_phone.area_code,
               country_code: parsed_phone.country,
@@ -603,7 +587,7 @@
               multi_factor_auth_method: 'sms',
               phone_configuration_id: controller.current_user.default_phone_configuration.id,
               multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'),
-              new_device: nil,
+              new_device: true,
               area_code: parsed_phone.area_code,
               country_code: parsed_phone.country,
               phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164),
@@ -685,7 +669,7 @@
               context: 'confirmation',
               multi_factor_auth_method: 'sms',
               multi_factor_auth_method_created_at: nil,
-              new_device: nil,
+              new_device: true,
               confirmation_for_add_phone: false,
               phone_configuration_id: nil,
               area_code: parsed_phone.area_code,

spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb

@@ -56,7 +56,7 @@
           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: nil,
+          new_device: true,
         }
 
         expect(@analytics).to receive(:track_mfa_submit_event).
@@ -112,31 +112,19 @@
           expect(response).to redirect_to(account_path)
         end
       end
-    end
 
-    context 'with new device session value' do
-      let(:user) { create(:user, :with_phone) }
-      let(:personal_key) { { personal_key: PersonalKeyGenerator.new(user).create } }
-      let(:payload) { { personal_key_form: personal_key } }
+      context 'with existing device' do
+        before do
+          allow(controller).to receive(:new_device?).and_return(false)
+        end
 
-      it 'tracks new device value' do
-        personal_key
-        sign_in_before_2fa(user)
-        stub_analytics
-        subject.user_session[:new_device] = false
-        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: false,
-        }
+        it 'tracks new device value' do
+          stub_analytics
+          stub_sign_in_before_2fa(user)
 
-        expect(@analytics).to receive(:track_mfa_submit_event).
-          with(analytics_hash)
+          expect(@analytics).to receive(:track_mfa_submit_event).
+            with(hash_including(new_device: false))
 
-        freeze_time do
           post :create, params: payload
         end
       end
@@ -221,7 +209,7 @@
           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: nil,
+          new_device: true,
         }
 
         expect(@analytics).to receive(:track_mfa_submit_event).