Commit 0200b26b authored by Drew Blessing's avatar Drew Blessing Committed by Drew Blessing

Replace poorly performing auth event provider query in usage ping

Usage ping queried distinct auth event providers from the database
which caused query timeouts on GitLab.com. Although not as
dynamic/intelligent this now relies on active configuration
to determine which providers to report on.
parent 88cf7733
...@@ -300,13 +300,13 @@ class SessionsController < Devise::SessionsController ...@@ -300,13 +300,13 @@ class SessionsController < Devise::SessionsController
def authentication_method def authentication_method
if user_params[:otp_attempt] if user_params[:otp_attempt]
"two-factor" AuthenticationEvent::TWO_FACTOR
elsif user_params[:device_response] && Feature.enabled?(:webauthn) 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) elsif user_params[:device_response] && !Feature.enabled?(:webauthn)
"two-factor-via-u2f-device" AuthenticationEvent::TWO_FACTOR_U2F
else else
"standard" AuthenticationEvent::STANDARD
end end
end end
......
...@@ -3,6 +3,12 @@ ...@@ -3,6 +3,12 @@
class AuthenticationEvent < ApplicationRecord class AuthenticationEvent < ApplicationRecord
include UsageStatistics 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 belongs_to :user, optional: true
validates :provider, :user_name, :result, presence: true validates :provider, :user_name, :result, presence: true
...@@ -17,6 +23,6 @@ class AuthenticationEvent < ApplicationRecord ...@@ -17,6 +23,6 @@ class AuthenticationEvent < ApplicationRecord
scope :ldap, -> { where('provider LIKE ?', 'ldap%')} scope :ldap, -> { where('provider LIKE ?', 'ldap%')}
def self.providers def self.providers
distinct.pluck(:provider) STATIC_PROVIDERS | Devise.omniauth_providers.map(&:to_s)
end end
end end
---
title: Replace poorly performing auth event providers query in usage ping
merge_request: 47710
author:
type: fixed
...@@ -172,6 +172,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do ...@@ -172,6 +172,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
omniauth: omniauth:
{ providers: omniauth_providers } { providers: omniauth_providers }
) )
allow(Devise).to receive(:omniauth_providers).and_return(%w(ldapmain ldapsecondary group_saml))
for_defined_days_back do for_defined_days_back do
user = create(:user) user = create(:user)
...@@ -190,14 +191,14 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do ...@@ -190,14 +191,14 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
groups: 2, groups: 2,
users_created: 6, users_created: 6,
omniauth_providers: ['google_oauth2'], 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( expect(described_class.usage_activity_by_stage_manage(described_class.last_28_days_time_period)).to include(
events: 1, events: 1,
groups: 1, groups: 1,
users_created: 3, users_created: 3,
omniauth_providers: ['google_oauth2'], 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 end
......
...@@ -37,15 +37,11 @@ RSpec.describe AuthenticationEvent do ...@@ -37,15 +37,11 @@ RSpec.describe AuthenticationEvent do
describe '.providers' do describe '.providers' do
before do before do
create(:authentication_event, provider: :ldapmain) allow(Devise).to receive(:omniauth_providers).and_return(%w(ldapmain google_oauth2))
create(:authentication_event, provider: :google_oauth2)
create(:authentication_event, provider: :standard)
create(:authentication_event, provider: :standard)
create(:authentication_event, provider: :standard)
end end
it 'returns an array of distinct providers' do 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 end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment