LG-802 Unify MFA setup events

app/controllers/two_factor_authentication/otp_verification_controller.rb

@@ -12,9 +12,10 @@ def show
     end
 
     def create
-      result = OtpVerificationForm.new(current_user, form_params[:code].strip).submit
+      result = OtpVerificationForm.new(current_user, sanitized_otp_code).submit
+      properties = result.to_h.merge(analytics_properties)
 
-      analytics.track_event(Analytics::MULTI_FACTOR_AUTH, result.to_h.merge(analytics_properties))
+      analytics.track_event(mfa_event_name, properties)
 
       if result.success?
         handle_valid_otp
@@ -62,10 +63,20 @@ def phone
         user_session[:unconfirmed_phone]
     end
 
+    def sanitized_otp_code
+      form_params[:code].strip
+    end
+
     def form_params
       params.permit(:code)
     end
 
+    def mfa_event_name
+      return Analytics::MULTI_FACTOR_AUTH_SETUP if context == 'confirmation'
+
+      Analytics::MULTI_FACTOR_AUTH
+    end
+
     def analytics_properties
       {
         context: context,

app/controllers/users/piv_cac_authentication_setup_controller.rb

@@ -53,7 +53,7 @@ def two_factor_enabled?
 
     def process_piv_cac_setup
       result = user_piv_cac_form.submit
-      analytics.track_event(Analytics::USER_REGISTRATION_PIV_CAC_ENABLED, result.to_h)
+      analytics.track_event(Analytics::MULTI_FACTOR_AUTH_SETUP, result.to_h)
       if result.success?
         process_valid_submission
       else

app/controllers/users/totp_setup_controller.rb

@@ -16,7 +16,7 @@ def new
     def confirm
       result = TotpSetupForm.new(current_user, new_totp_secret, params[:code].strip).submit
 
-      analytics.track_event(Analytics::TOTP_SETUP, result.to_h)
+      analytics.track_event(Analytics::MULTI_FACTOR_AUTH_SETUP, result.to_h)
 
       if result.success?
         process_valid_code

app/controllers/users/webauthn_setup_controller.rb

@@ -14,7 +14,7 @@ def new
     def confirm
       form = WebauthnSetupForm.new(current_user, user_session)
       result = form.submit(request.protocol, params)
-      analytics.track_event(Analytics::WEBAUTHN_SETUP_SUBMITTED, result.to_h)
+      analytics.track_event(Analytics::MULTI_FACTOR_AUTH_SETUP, result.to_h)
       if result.success?
         process_valid_webauthn
       else

app/forms/totp_setup_form.rb

@@ -29,6 +29,9 @@ def process_valid_submission
   end
 
   def extra_analytics_attributes
-    { totp_secret_present: secret.present? }
+    {
+      totp_secret_present: secret.present?,
+      multi_factor_auth_method: 'totp',
+    }
   end
 end

app/forms/user_piv_cac_setup_form.rb

@@ -10,7 +10,11 @@ class UserPivCacSetupForm
   def submit
     success = valid? && valid_token?
 
-    FormResponse.new(success: success && process_valid_submission, errors: {})
+    FormResponse.new(
+      success: success && process_valid_submission,
+      errors: {},
+      extra: extra_analytics_attributes
+    )
   end
 
   private
@@ -76,4 +80,10 @@ def user_has_no_piv_cac
       true
     end
   end
+
+  def extra_analytics_attributes
+    {
+      multi_factor_auth_method: 'piv_cac',
+    }
+  end
 end

app/forms/webauthn_setup_form.rb

@@ -82,6 +82,9 @@ def create_user_event
   end
 
   def extra_analytics_attributes
-    { mfa_method_counts: MfaContext.new(user).enabled_two_factor_configuration_counts_hash }
+    {
+      mfa_method_counts: MfaContext.new(user).enabled_two_factor_configuration_counts_hash,
+      multi_factor_auth_method: 'webauthn',
+    }
   end
 end

app/services/analytics.rb

@@ -100,6 +100,7 @@ def browser
   MULTI_FACTOR_AUTH_OPTION_LIST_VISIT = 'Multi-Factor Authentication: option list visited'.freeze
   MULTI_FACTOR_AUTH_PHONE_SETUP = 'Multi-Factor Authentication: phone setup'.freeze
   MULTI_FACTOR_AUTH_MAX_SENDS = 'Multi-Factor Authentication: max otp sends reached'.freeze
+  MULTI_FACTOR_AUTH_SETUP = 'Multi-Factor Authentication Setup'.freeze
   OPENID_CONNECT_BEARER_TOKEN = 'OpenID Connect: bearer token authentication'.freeze
   OPENID_CONNECT_REQUEST_AUTHORIZATION = 'OpenID Connect: authorization request'.freeze
   OPENID_CONNECT_TOKEN = 'OpenID Connect: token'.freeze
@@ -121,7 +122,6 @@ def browser
   SAML_AUTH = 'SAML Auth'.freeze
   SESSION_TIMED_OUT = 'Session Timed Out'.freeze
   SIGN_IN_PAGE_VISIT = 'Sign in page visited'.freeze
-  TOTP_SETUP = 'TOTP Setup'.freeze
   TOTP_SETUP_VISIT = 'TOTP Setup Visited'.freeze
   TOTP_USER_DISABLED = 'TOTP: User Disabled TOTP'.freeze
   TWILIO_PHONE_VALIDATION_FAILED = 'Twilio Phone Validation Failed'.freeze
@@ -139,10 +139,8 @@ def browser
   USER_REGISTRATION_PHONE_SETUP_VISIT = 'User Registration: phone setup visited'.freeze
   USER_REGISTRATION_PERSONAL_KEY_VISIT = 'User Registration: personal key visited'.freeze
   USER_REGISTRATION_PIV_CAC_DISABLED = 'User Registration: piv cac disabled'.freeze
-  USER_REGISTRATION_PIV_CAC_ENABLED = 'User Registration: piv cac enabled'.freeze
   USER_REGISTRATION_PIV_CAC_SETUP_VISIT = 'User Registration: piv cac setup visited'.freeze
   WEBAUTHN_DELETED = 'WebAuthn Deleted'.freeze
   WEBAUTHN_SETUP_VISIT = 'WebAuthn Setup Visited'.freeze
-  WEBAUTHN_SETUP_SUBMITTED = 'WebAuthn Setup Submitted'.freeze
   # rubocop:enable Metrics/LineLength
 end

spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb

@@ -307,7 +307,7 @@
             }
 
             expect(@analytics).to have_received(:track_event).
-              with(Analytics::MULTI_FACTOR_AUTH, properties)
+              with(Analytics::MULTI_FACTOR_AUTH_SETUP, properties)
             expect(subject).to have_received(:create_user_event).with(:phone_changed)
             expect(subject).to have_received(:create_user_event).exactly(:once)
             subject.current_user.email_addresses.each do |email_address|
@@ -352,7 +352,7 @@
             }
 
             expect(@analytics).to have_received(:track_event).
-              with(Analytics::MULTI_FACTOR_AUTH, properties)
+              with(Analytics::MULTI_FACTOR_AUTH_SETUP, properties)
           end
         end
       end
@@ -388,7 +388,7 @@
             }
 
             expect(@analytics).to have_received(:track_event).
-              with(Analytics::MULTI_FACTOR_AUTH, properties)
+              with(Analytics::MULTI_FACTOR_AUTH_SETUP, properties)
 
             expect(subject).to have_received(:create_user_event).with(:phone_confirmed)
             expect(subject).to have_received(:create_user_event).exactly(:once)

spec/controllers/users/totp_setup_controller_spec.rb

@@ -94,8 +94,8 @@
           stub_sign_in(user)
           stub_analytics
           allow(@analytics).to receive(:track_event)
+          subject.user_session[:new_totp_secret] = 'abcdehij'
 
-          get :new
           patch :confirm, params: { code: 123 }
         end
 
@@ -108,30 +108,24 @@
             success: false,
             errors: {},
             totp_secret_present: true,
+            multi_factor_auth_method: 'totp',
           }
-          expect(@analytics).to have_received(:track_event).with(Analytics::TOTP_SETUP, result)
+
+          expect(@analytics).to have_received(:track_event).
+            with(Analytics::MULTI_FACTOR_AUTH_SETUP, result)
         end
       end
 
       context 'when user presents correct code' do
         before do
           user = build(:user, personal_key: 'ABCD-DEFG-HIJK-LMNO')
+          secret = ROTP::Base32.random_base32
           stub_sign_in(user)
           stub_analytics
           allow(@analytics).to receive(:track_event)
+          subject.user_session[:new_totp_secret] = secret
 
-          code = '123455'
-          totp_secret = 'abdef'
-          subject.user_session[:new_totp_secret] = totp_secret
-          form = instance_double(TotpSetupForm)
-
-          allow(TotpSetupForm).to receive(:new).
-            with(subject.current_user, totp_secret, code).and_return(form)
-          response = FormResponse.new(success: true, errors: {})
-          allow(form).to receive(:submit).and_return(response)
-
-          get :new
-          patch :confirm, params: { code: code }
+          patch :confirm, params: { code: generate_totp_code(secret) }
         end
 
         it 'redirects to account_path with a success message' do
@@ -142,8 +136,12 @@
           result = {
             success: true,
             errors: {},
+            totp_secret_present: true,
+            multi_factor_auth_method: 'totp',
           }
-          expect(@analytics).to have_received(:track_event).with(Analytics::TOTP_SETUP, result)
+
+          expect(@analytics).to have_received(:track_event).
+            with(Analytics::MULTI_FACTOR_AUTH_SETUP, result)
         end
       end
     end
@@ -154,8 +152,8 @@
           stub_sign_in_before_2fa
           stub_analytics
           allow(@analytics).to receive(:track_event)
+          subject.user_session[:new_totp_secret] = 'abcdehij'
 
-          get :new
           patch :confirm, params: { code: 123 }
         end
 
@@ -168,29 +166,22 @@
             success: false,
             errors: {},
             totp_secret_present: true,
+            multi_factor_auth_method: 'totp',
           }
-          expect(@analytics).to have_received(:track_event).with(Analytics::TOTP_SETUP, result)
+          expect(@analytics).to have_received(:track_event).
+            with(Analytics::MULTI_FACTOR_AUTH_SETUP, result)
         end
       end
 
       context 'when user presents correct code' do
         before do
+          secret = ROTP::Base32.random_base32
           stub_sign_in_before_2fa
           stub_analytics
           allow(@analytics).to receive(:track_event)
+          subject.user_session[:new_totp_secret] = secret
 
-          code = '123455'
-          totp_secret = 'abdef'
-          subject.user_session[:new_totp_secret] = totp_secret
-          form = instance_double(TotpSetupForm)
-
-          allow(TotpSetupForm).to receive(:new).
-            with(subject.current_user, totp_secret, code).and_return(form)
-          response = FormResponse.new(success: true, errors: {})
-          allow(form).to receive(:submit).and_return(response)
-
-          get :new
-          patch :confirm, params: { code: code }
+          patch :confirm, params: { code: generate_totp_code(secret) }
         end
 
         it 'redirects to personal key page with a success message' do
@@ -201,8 +192,12 @@
           result = {
             success: true,
             errors: {},
+            totp_secret_present: true,
+            multi_factor_auth_method: 'totp',
           }
-          expect(@analytics).to have_received(:track_event).with(Analytics::TOTP_SETUP, result)
+
+          expect(@analytics).to have_received(:track_event).
+            with(Analytics::MULTI_FACTOR_AUTH_SETUP, result)
         end
       end
 
@@ -224,8 +219,11 @@
             success: false,
             errors: {},
             totp_secret_present: false,
+            multi_factor_auth_method: 'totp',
           }
-          expect(@analytics).to have_received(:track_event).with(Analytics::TOTP_SETUP, result)
+
+          expect(@analytics).to have_received(:track_event).
+            with(Analytics::MULTI_FACTOR_AUTH_SETUP, result)
         end
       end
     end

spec/controllers/users/webauthn_setup_controller_spec.rb

@@ -88,9 +88,14 @@
       end
 
       it 'tracks the submission' do
-        result = { success: true, errors: {}, mfa_method_counts: { auth_app: 1, phone: 1 } }
+        result = {
+          success: true,
+          errors: {},
+          mfa_method_counts: { auth_app: 1, phone: 1 },
+          multi_factor_auth_method: 'webauthn',
+        }
         expect(@analytics).to receive(:track_event).
-          with(Analytics::WEBAUTHN_SETUP_SUBMITTED, result)
+          with(Analytics::MULTI_FACTOR_AUTH_SETUP, result)
 
         patch :confirm, params: params
       end

spec/forms/totp_setup_form_spec.rb

@@ -10,9 +10,13 @@
       it 'returns FormResponse with success: true' do
         form = TotpSetupForm.new(user, secret, code)
         result = instance_double(FormResponse)
+        extra = {
+          totp_secret_present: true,
+          multi_factor_auth_method: 'totp',
+        }
 
         expect(FormResponse).to receive(:new).
-          with(success: true, errors: {}, extra: { totp_secret_present: true }).and_return(result)
+          with(success: true, errors: {}, extra: extra).and_return(result)
         expect(Event).to receive(:create).
           with(user_id: user.id, event_type: :authenticator_enabled)
         expect(form.submit).to eq result
@@ -24,9 +28,13 @@
       it 'returns FormResponse with success: false' do
         form = TotpSetupForm.new(user, secret, 'kode')
         result = instance_double(FormResponse)
+        extra = {
+          totp_secret_present: true,
+          multi_factor_auth_method: 'totp',
+        }
 
         expect(FormResponse).to receive(:new).
-          with(success: false, errors: {}, extra: { totp_secret_present: true }).and_return(result)
+          with(success: false, errors: {}, extra: extra).and_return(result)
         expect(Event).to_not receive(:create)
         expect(form.submit).to eq result
         expect(user.reload.totp_enabled?).to eq false
@@ -39,9 +47,13 @@
       it 'returns FormResponse with success: false' do
         form = TotpSetupForm.new(user, nil, 'kode')
         result = instance_double(FormResponse)
+        extra = {
+          totp_secret_present: false,
+          multi_factor_auth_method: 'totp',
+        }
 
         expect(FormResponse).to receive(:new).
-          with(success: false, errors: {}, extra: { totp_secret_present: false }).and_return(result)
+          with(success: false, errors: {}, extra: extra).and_return(result)
         expect(Event).to_not receive(:create)
         expect(form.submit).to eq result
         expect(user.reload.totp_enabled?).to eq false