From 9f13953879b90611c37547e9c7237286a6f09eaf Mon Sep 17 00:00:00 2001
From: Drew Blessing <drew@blessing.io>
Date: Fri, 11 Sep 2020 12:11:49 -0500
Subject: [PATCH] Report auth events in manage stage usage ping
Provide aggregate auth event details in usage ping
to help identify how often particular authentication
methods are being used. This will help inform decision
making about improvements and fixes.
---
app/models/authentication_event.rb | 9 +++++
.../dblessing-auth-events-usage-ping.yml | 5 +++
...d_result_index_to_authentication_events.rb | 18 ++++++++++
db/schema_migrations/20200916151442 | 1 +
db/structure.sql | 2 ++
lib/gitlab/usage_data.rb | 35 +++++++++++++++++++
spec/factories/authentication_event.rb | 11 ++++++
spec/lib/gitlab/usage_data_spec.rb | 21 ++++++++---
spec/models/authentication_event_spec.rb | 31 ++++++++++++++++
9 files changed, 128 insertions(+), 5 deletions(-)
create mode 100644 changelogs/unreleased/dblessing-auth-events-usage-ping.yml
create mode 100644 db/migrate/20200916151442_add_result_index_to_authentication_events.rb
create mode 100644 db/schema_migrations/20200916151442
create mode 100644 spec/factories/authentication_event.rb
diff --git a/app/models/authentication_event.rb b/app/models/authentication_event.rb
index 1ac3c5fbd9ce10d1..16b132c7ed5534ad 100644
--- a/app/models/authentication_event.rb
+++ b/app/models/authentication_event.rb
@@ -1,6 +1,8 @@
# frozen_string_literal: true
class AuthenticationEvent < ApplicationRecord
+ include UsageStatistics
+
belongs_to :user, optional: true
validates :provider, :user_name, :result, presence: true
@@ -9,4 +11,11 @@ class AuthenticationEvent < ApplicationRecord
failed: 0,
success: 1
}
+
+ scope :for_provider, ->(provider) { where(provider: provider) }
+ scope :ldap, -> { where('provider LIKE ?', 'ldap%')}
+
+ def self.providers
+ distinct.pluck(:provider)
+ end
end
diff --git a/changelogs/unreleased/dblessing-auth-events-usage-ping.yml b/changelogs/unreleased/dblessing-auth-events-usage-ping.yml
new file mode 100644
index 0000000000000000..26117006f1c38419
--- /dev/null
+++ b/changelogs/unreleased/dblessing-auth-events-usage-ping.yml
@@ -0,0 +1,5 @@
+---
+title: Report auth events in manage stage usage ping
+merge_request: 39747
+author:
+type: added
diff --git a/db/migrate/20200916151442_add_result_index_to_authentication_events.rb b/db/migrate/20200916151442_add_result_index_to_authentication_events.rb
new file mode 100644
index 0000000000000000..13b0521038ed3d3b
--- /dev/null
+++ b/db/migrate/20200916151442_add_result_index_to_authentication_events.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+class AddResultIndexToAuthenticationEvents < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+ INDEX_NAME = 'index_authentication_events_on_provider_user_id_created_at'
+
+ disable_ddl_transaction!
+
+ def up
+ add_concurrent_index :authentication_events, [:provider, :user_id, :created_at], where: 'result = 1', name: INDEX_NAME
+ end
+
+ def down
+ remove_concurrent_index_by_name :authentication_events, INDEX_NAME
+ end
+end
diff --git a/db/schema_migrations/20200916151442 b/db/schema_migrations/20200916151442
new file mode 100644
index 0000000000000000..36b2a4e9962490fa
--- /dev/null
+++ b/db/schema_migrations/20200916151442
@@ -0,0 +1 @@
+aef52404e6ce83d5d4b3de65ad00b665334f5ff2e5b7b6c3c622f79313657f26
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 753f44a018070557..67738c8623ed0847 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -19478,6 +19478,8 @@ CREATE UNIQUE INDEX index_atlassian_identities_on_extern_uid ON atlassian_identi
CREATE INDEX index_authentication_events_on_provider ON authentication_events USING btree (provider);
+CREATE INDEX index_authentication_events_on_provider_user_id_created_at ON authentication_events USING btree (provider, user_id, created_at) WHERE (result = 1);
+
CREATE INDEX index_authentication_events_on_user_id ON authentication_events USING btree (user_id);
CREATE INDEX index_award_emoji_on_awardable_type_and_awardable_id ON award_emoji USING btree (awardable_type, awardable_id);
diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb
index 6bb651bbae5cfd09..ea3e2ea064befe9e 100644
--- a/lib/gitlab/usage_data.rb
+++ b/lib/gitlab/usage_data.rb
@@ -542,6 +542,7 @@ def usage_activity_by_stage_manage(time_period)
groups: distinct_count(::GroupMember.where(time_period), :user_id),
users_created: count(::User.where(time_period), start: user_minimum_id, finish: user_maximum_id),
omniauth_providers: filtered_omniauth_provider_names.reject { |name| name == 'group_saml' },
+ user_auth_by_provider: distinct_count_user_auth_by_provider(time_period),
projects_imported: {
gitlab_project: projects_imported_count('gitlab_project', time_period),
gitlab: projects_imported_count('gitlab', time_period),
@@ -813,6 +814,7 @@ def clear_memoized
clear_memoization(:approval_merge_request_rule_maximum_id)
clear_memoization(:project_minimum_id)
clear_memoization(:project_maximum_id)
+ clear_memoization(:auth_providers)
end
# rubocop: disable CodeReuse/ActiveRecord
@@ -844,6 +846,39 @@ def deployment_count(relation)
def projects_imported_count(from, time_period)
distinct_count(::Project.imported_from(from).where(time_period), :creator_id) # rubocop: disable CodeReuse/ActiveRecord
end
+
+ # rubocop:disable CodeReuse/ActiveRecord
+ def distinct_count_user_auth_by_provider(time_period)
+ counts = auth_providers_except_ldap.each_with_object({}) do |provider, hash|
+ hash[provider] = distinct_count(
+ ::AuthenticationEvent.success.for_provider(provider).where(time_period), :user_id)
+ end
+
+ if any_ldap_auth_providers?
+ counts['ldap'] = distinct_count(
+ ::AuthenticationEvent.success.ldap.where(time_period), :user_id
+ )
+ end
+
+ counts
+ end
+ # rubocop:enable CodeReuse/ActiveRecord
+
+ # rubocop:disable UsageData/LargeTable
+ def auth_providers
+ strong_memoize(:auth_providers) do
+ ::AuthenticationEvent.providers
+ end
+ end
+ # rubocop:enable UsageData/LargeTable
+
+ def auth_providers_except_ldap
+ auth_providers.reject { |provider| provider.starts_with?('ldap') }
+ end
+
+ def any_ldap_auth_providers?
+ auth_providers.any? { |provider| provider.starts_with?('ldap') }
+ end
end
end
end
diff --git a/spec/factories/authentication_event.rb b/spec/factories/authentication_event.rb
new file mode 100644
index 0000000000000000..ff539c6f5c4aea71
--- /dev/null
+++ b/spec/factories/authentication_event.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+ factory :authentication_event do
+ user
+ provider { :standard }
+ user_name { 'Jane Doe' }
+ ip_address { '127.0.0.1' }
+ result { :failed }
+ end
+end
diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb
index 7c413f3bccab79c9..22c6ef7346b33838 100644
--- a/spec/lib/gitlab/usage_data_spec.rb
+++ b/spec/lib/gitlab/usage_data_spec.rb
@@ -29,7 +29,8 @@
user_minimum_id user_maximum_id unique_visit_service
deployment_minimum_id deployment_maximum_id
approval_merge_request_rule_minimum_id
- approval_merge_request_rule_maximum_id)
+ approval_merge_request_rule_maximum_id
+ auth_providers)
values.each do |key|
expect(described_class).to receive(:clear_memoization).with(key)
end
@@ -167,6 +168,8 @@
describe 'usage_activity_by_stage_manage' do
it 'includes accurate usage_activity_by_stage data' do
+ described_class.clear_memoization(:auth_providers)
+
stub_config(
omniauth:
{ providers: omniauth_providers }
@@ -174,21 +177,29 @@
for_defined_days_back do
user = create(:user)
+ user2 = create(:user)
create(:event, author: user)
create(:group_member, user: user)
+ create(:authentication_event, user: user, provider: :ldapmain, result: :success)
+ create(:authentication_event, user: user2, provider: :ldapsecondary, result: :success)
+ create(:authentication_event, user: user2, provider: :group_saml, result: :success)
+ create(:authentication_event, user: user2, provider: :group_saml, result: :success)
+ create(:authentication_event, user: user, provider: :group_saml, result: :failed)
end
expect(described_class.usage_activity_by_stage_manage({})).to include(
events: 2,
groups: 2,
- users_created: 4,
- omniauth_providers: ['google_oauth2']
+ users_created: 6,
+ omniauth_providers: ['google_oauth2'],
+ user_auth_by_provider: { 'group_saml' => 2, 'ldap' => 4 }
)
expect(described_class.usage_activity_by_stage_manage(described_class.last_28_days_time_period)).to include(
events: 1,
groups: 1,
- users_created: 2,
- omniauth_providers: ['google_oauth2']
+ users_created: 3,
+ omniauth_providers: ['google_oauth2'],
+ user_auth_by_provider: { 'group_saml' => 1, 'ldap' => 2 }
)
end
diff --git a/spec/models/authentication_event_spec.rb b/spec/models/authentication_event_spec.rb
index 56b0111f2c7c3d44..7390fde74a66ced4 100644
--- a/spec/models/authentication_event_spec.rb
+++ b/spec/models/authentication_event_spec.rb
@@ -12,4 +12,35 @@
it { is_expected.to validate_presence_of(:user_name) }
it { is_expected.to validate_presence_of(:result) }
end
+
+ describe 'scopes' do
+ let_it_be(:ldap_event) { create(:authentication_event, provider: :ldapmain, result: :failed) }
+ let_it_be(:google_oauth2) { create(:authentication_event, provider: :google_oauth2, result: :success) }
+
+ describe '.for_provider' do
+ it 'returns events only for the specified provider' do
+ expect(described_class.for_provider(:ldapmain)).to match_array ldap_event
+ end
+ end
+
+ describe '.ldap' do
+ it 'returns all events for an LDAP provider' do
+ expect(described_class.ldap).to match_array ldap_event
+ end
+ end
+ end
+
+ 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)
+ end
+
+ it 'returns an array of distinct providers' do
+ expect(described_class.providers).to match_array %w(ldapmain google_oauth2 standard)
+ end
+ end
end
--
GitLab