Merge branch 'dblessing_auth_events_usage_ping_fix' into 'master'

Replace poorly performing auth event provider query in usage ping

See merge request gitlab-org/gitlab!47710

app/controllers/sessions_controller.rb

@@ -300,13 +300,13 @@ class SessionsController < Devise::SessionsController
 
   def authentication_method
     if user_params[:otp_attempt]
-      "two-factor"
+      AuthenticationEvent::TWO_FACTOR
     elsif user_params[:device_response] && Feature.enabled?(:webauthn)
-      "two-factor-via-webauthn-device"
+      AuthenticationEvent::TWO_FACTOR_WEBAUTHN
     elsif user_params[:device_response] && !Feature.enabled?(:webauthn)
-      "two-factor-via-u2f-device"
+      AuthenticationEvent::TWO_FACTOR_U2F
     else
-      "standard"
+      AuthenticationEvent::STANDARD
     end
   end
 

app/models/authentication_event.rb

@@ -3,6 +3,12 @@
 class AuthenticationEvent < ApplicationRecord
   include UsageStatistics
 
+  TWO_FACTOR = 'two-factor'.freeze
+  TWO_FACTOR_U2F = 'two-factor-via-u2f-device'.freeze
+  TWO_FACTOR_WEBAUTHN = 'two-factor-via-webauthn-device'.freeze
+  STANDARD = 'standard'.freeze
+  STATIC_PROVIDERS = [TWO_FACTOR, TWO_FACTOR_U2F, TWO_FACTOR_WEBAUTHN, STANDARD].freeze
+
   belongs_to :user, optional: true
 
   validates :provider, :user_name, :result, presence: true
@@ -17,6 +23,6 @@ class AuthenticationEvent < ApplicationRecord
   scope :ldap, -> { where('provider LIKE ?', 'ldap%')}
 
   def self.providers
-    distinct.pluck(:provider)
+    STATIC_PROVIDERS | Devise.omniauth_providers.map(&:to_s)
   end
 end

changelogs/unreleased/dblessing_auth_events_usage_ping_fix.yml

+---
+title: Replace poorly performing auth event providers query in usage ping
+merge_request: 47710
+author:
+type: fixed

spec/lib/gitlab/usage_data_spec.rb

@@ -172,6 +172,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
         omniauth:
           { providers: omniauth_providers }
       )
+      allow(Devise).to receive(:omniauth_providers).and_return(%w(ldapmain ldapsecondary group_saml))
 
       for_defined_days_back do
         user = create(:user)
@@ -190,14 +191,14 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
         groups: 2,
         users_created: 6,
         omniauth_providers: ['google_oauth2'],
-        user_auth_by_provider: { 'group_saml' => 2, 'ldap' => 4 }
+        user_auth_by_provider: { 'group_saml' => 2, 'ldap' => 4, 'standard' => 0, 'two-factor' => 0, 'two-factor-via-u2f-device' => 0, "two-factor-via-webauthn-device" => 0 }
       )
       expect(described_class.usage_activity_by_stage_manage(described_class.last_28_days_time_period)).to include(
         events: 1,
         groups: 1,
         users_created: 3,
         omniauth_providers: ['google_oauth2'],
-        user_auth_by_provider: { 'group_saml' => 1, 'ldap' => 2 }
+        user_auth_by_provider: { 'group_saml' => 1, 'ldap' => 2, 'standard' => 0, 'two-factor' => 0, 'two-factor-via-u2f-device' => 0, "two-factor-via-webauthn-device" => 0 }
       )
     end
 

spec/models/authentication_event_spec.rb

@@ -37,15 +37,11 @@ RSpec.describe AuthenticationEvent do
 
   describe '.providers' do
     before do
-      create(:authentication_event, provider: :ldapmain)
-      create(:authentication_event, provider: :google_oauth2)
-      create(:authentication_event, provider: :standard)
-      create(:authentication_event, provider: :standard)
-      create(:authentication_event, provider: :standard)
+      allow(Devise).to receive(:omniauth_providers).and_return(%w(ldapmain google_oauth2))
     end
 
     it 'returns an array of distinct providers' do
-      expect(described_class.providers).to match_array %w(ldapmain google_oauth2 standard)
+      expect(described_class.providers).to match_array %w(ldapmain google_oauth2 standard two-factor two-factor-via-u2f-device two-factor-via-webauthn-device)
     end
   end
 end