From 234f29f42bcae0225c35830de1854f1545e1d3cb Mon Sep 17 00:00:00 2001 From: Eric Leijonmarck Date: Fri, 22 Jul 2022 12:06:02 +0100 Subject: [PATCH] [v9.0.x] Auth: Add prometheus metrics for case insensitive ids (#52634) * merge backport * remove sa background service --- .../backgroundsvcs/background_services.go | 3 + .../authinfoservice/database/database.go | 1 + .../login/authinfoservice/database/stats.go | 149 ++++++++++++++++++ .../authinfoservice/database/usagestats.go | 55 ------- pkg/services/login/authinfoservice/service.go | 5 + .../login/authinfoservice/user_auth_test.go | 5 +- pkg/services/login/userprotection.go | 1 + 7 files changed, 163 insertions(+), 56 deletions(-) create mode 100644 pkg/services/login/authinfoservice/database/stats.go delete mode 100644 pkg/services/login/authinfoservice/database/usagestats.go diff --git a/pkg/server/backgroundsvcs/background_services.go b/pkg/server/backgroundsvcs/background_services.go index d891a17c4ca..e6e10ab266a 100644 --- a/pkg/server/backgroundsvcs/background_services.go +++ b/pkg/server/backgroundsvcs/background_services.go @@ -16,6 +16,7 @@ import ( "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/live" "github.com/grafana/grafana/pkg/services/live/pushhttp" + "github.com/grafana/grafana/pkg/services/login/authinfoservice" "github.com/grafana/grafana/pkg/services/ngalert" "github.com/grafana/grafana/pkg/services/notifications" plugindashboardsservice "github.com/grafana/grafana/pkg/services/plugindashboards/service" @@ -38,6 +39,7 @@ func ProvideBackgroundServiceRegistry( pluginsUpdateChecker *updatechecker.PluginsService, metrics *metrics.InternalMetricsService, secretsService *secretsManager.SecretsService, remoteCache *remotecache.RemoteCache, thumbnailsService thumbs.Service, StorageService store.StorageService, searchService searchV2.SearchService, entityEventsService store.EntityEventsService, + authInfoService *authinfoservice.Implementation, // Need to make sure these are initialized, is there a better place to put them? _ *dashboardsnapshots.Service, _ *alerting.AlertNotificationService, _ serviceaccounts.Service, _ *guardian.Provider, @@ -67,6 +69,7 @@ func ProvideBackgroundServiceRegistry( thumbnailsService, searchService, entityEventsService, + authInfoService, ) } diff --git a/pkg/services/login/authinfoservice/database/database.go b/pkg/services/login/authinfoservice/database/database.go index 434479d70a3..206ac3f9aeb 100644 --- a/pkg/services/login/authinfoservice/database/database.go +++ b/pkg/services/login/authinfoservice/database/database.go @@ -25,6 +25,7 @@ func ProvideAuthInfoStore(sqlStore sqlstore.Store, secretsService secrets.Servic secretsService: secretsService, logger: log.New("login.authinfo.store"), } + InitMetrics() return store } diff --git a/pkg/services/login/authinfoservice/database/stats.go b/pkg/services/login/authinfoservice/database/stats.go new file mode 100644 index 00000000000..d0bf4d81563 --- /dev/null +++ b/pkg/services/login/authinfoservice/database/stats.go @@ -0,0 +1,149 @@ +package database + +import ( + "context" + "sync" + "time" + + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/prometheus/client_golang/prometheus" +) + +type LoginStats struct { + DuplicateUserEntries int `xorm:"duplicate_user_entries"` + MixedCasedUsers int `xorm:"mixed_cased_users"` +} + +const ( + ExporterName = "grafana" + metricsCollectionInterval = time.Second * 60 * 4 // every 4 hours, indication of duplicate users +) + +var ( + // MStatDuplicateUserEntries is a indication metric gauge for number of users with duplicate emails or logins + MStatDuplicateUserEntries prometheus.Gauge + + // MStatHasDuplicateEntries is a metric for if there is duplicate users + MStatHasDuplicateEntries prometheus.Gauge + + // MStatMixedCasedUsers is a metric for if there is duplicate users + MStatMixedCasedUsers prometheus.Gauge + + once sync.Once + Initialised bool = false +) + +func InitMetrics() { + once.Do(func() { + MStatDuplicateUserEntries = prometheus.NewGauge(prometheus.GaugeOpts{ + Name: "stat_users_total_duplicate_user_entries", + Help: "total number of duplicate user entries by email or login", + Namespace: ExporterName, + }) + + MStatHasDuplicateEntries = prometheus.NewGauge(prometheus.GaugeOpts{ + Name: "stat_users_has_duplicate_user_entries", + Help: "instance has duplicate user entries by email or login", + Namespace: ExporterName, + }) + + MStatMixedCasedUsers = prometheus.NewGauge(prometheus.GaugeOpts{ + Name: "stat_users_total_mixed_cased_users", + Help: "total number of users with upper and lower case logins or emails", + Namespace: ExporterName, + }) + + prometheus.MustRegister( + MStatDuplicateUserEntries, + MStatHasDuplicateEntries, + MStatMixedCasedUsers, + ) + }) +} + +func (s *AuthInfoStore) RunMetricsCollection(ctx context.Context) error { + if _, err := s.GetLoginStats(ctx); err != nil { + s.logger.Warn("Failed to get authinfo metrics", "error", err.Error()) + } + updateStatsTicker := time.NewTicker(metricsCollectionInterval) + defer updateStatsTicker.Stop() + + for { + select { + case <-updateStatsTicker.C: + if _, err := s.GetLoginStats(ctx); err != nil { + s.logger.Warn("Failed to get authinfo metrics", "error", err.Error()) + } + case <-ctx.Done(): + return ctx.Err() + } + } +} + +func (s *AuthInfoStore) GetLoginStats(ctx context.Context) (LoginStats, error) { + var stats LoginStats + outerErr := s.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error { + rawSQL := `SELECT + (SELECT COUNT(*) FROM (` + s.duplicateUserEntriesSQL(ctx) + `) AS d WHERE (d.dup_login IS NOT NULL OR d.dup_email IS NOT NULL)) as duplicate_user_entries, + (SELECT COUNT(*) FROM (` + s.mixedCasedUsers(ctx) + `) AS mcu) AS mixed_cased_users + ` + _, err := dbSession.SQL(rawSQL).Get(&stats) + return err + }) + if outerErr != nil { + return stats, outerErr + } + + // set prometheus metrics stats + MStatDuplicateUserEntries.Set(float64(stats.DuplicateUserEntries)) + if stats.DuplicateUserEntries == 0 { + MStatHasDuplicateEntries.Set(float64(0)) + } else { + MStatHasDuplicateEntries.Set(float64(1)) + } + + MStatMixedCasedUsers.Set(float64(stats.MixedCasedUsers)) + return stats, nil +} + +func (s *AuthInfoStore) CollectLoginStats(ctx context.Context) (map[string]interface{}, error) { + m := map[string]interface{}{} + + loginStats, err := s.GetLoginStats(ctx) + if err != nil { + s.logger.Error("Failed to get login stats", "error", err) + return nil, err + } + + m["stats.users.duplicate_user_entries"] = loginStats.DuplicateUserEntries + if loginStats.DuplicateUserEntries > 0 { + m["stats.users.has_duplicate_user_entries"] = 1 + } else { + m["stats.users.has_duplicate_user_entries"] = 0 + } + + m["stats.users.mixed_cased_users"] = loginStats.MixedCasedUsers + + return m, nil +} + +func (s *AuthInfoStore) duplicateUserEntriesSQL(ctx context.Context) string { + userDialect := s.sqlStore.GetDialect().Quote("user") + // this query counts how many users have the same login or email. + // which might be confusing, but gives a good indication + // we want this query to not require too much cpu + sqlQuery := `SELECT + (SELECT login from ` + userDialect + ` WHERE (LOWER(login) = LOWER(u.login)) AND (login != u.login)) AS dup_login, + (SELECT email from ` + userDialect + ` WHERE (LOWER(email) = LOWER(u.email)) AND (email != u.email)) AS dup_email + FROM ` + userDialect + ` AS u` + return sqlQuery +} + +func (s *AuthInfoStore) mixedCasedUsers(ctx context.Context) string { + userDialect := s.sqlStore.GetDialect().Quote("user") + // this query counts how many users have upper case and lower case login or emails. + // why + // users login via IDP or service providers get upper cased domains at times :shrug: + sqlQuery := `SELECT login, email FROM ` + userDialect + ` WHERE (LOWER(login) != login OR lower(email) != email)` + return sqlQuery +} diff --git a/pkg/services/login/authinfoservice/database/usagestats.go b/pkg/services/login/authinfoservice/database/usagestats.go deleted file mode 100644 index 0bcdfd1319b..00000000000 --- a/pkg/services/login/authinfoservice/database/usagestats.go +++ /dev/null @@ -1,55 +0,0 @@ -package database - -import ( - "context" - - "github.com/grafana/grafana/pkg/services/sqlstore" -) - -type LoginStats struct { - DuplicateUserEntries int `xorm:"duplicate_user_entries"` -} - -func (s *AuthInfoStore) GetLoginStats(ctx context.Context) (LoginStats, error) { - var stats LoginStats - outerErr := s.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error { - rawSQL := `SELECT COUNT(*) as duplicate_user_entries FROM (` + s.duplicateUserEntriesSQL(ctx) + `) AS d - WHERE (d.dup_login IS NOT NULL OR d.dup_email IS NOT NULL)` - _, err := dbSession.SQL(rawSQL).Get(&stats) - return err - }) - if outerErr != nil { - return stats, outerErr - } - return stats, nil -} - -func (s *AuthInfoStore) CollectLoginStats(ctx context.Context) (map[string]interface{}, error) { - m := map[string]interface{}{} - - loginStats, err := s.GetLoginStats(ctx) - if err != nil { - s.logger.Error("Failed to get login stats", "error", err) - return nil, err - } - - m["stats.users.duplicate_user_entries"] = loginStats.DuplicateUserEntries - if loginStats.DuplicateUserEntries > 0 { - m["stats.users.has_duplicate_user_entries"] = 1 - } else { - m["stats.users.has_duplicate_user_entries"] = 0 - } - return m, nil -} - -func (s *AuthInfoStore) duplicateUserEntriesSQL(ctx context.Context) string { - userDialect := s.sqlStore.GetDialect().Quote("user") - // this query counts how many users have the same login or email. - // which might be confusing, but gives a good indication - // we want this query to not require too much cpu - sqlQuery := `SELECT - (SELECT login from ` + userDialect + ` WHERE (LOWER(login) = LOWER(u.login)) AND (login != u.login)) AS dup_login, - (SELECT email from ` + userDialect + ` WHERE (LOWER(email) = LOWER(u.email)) AND (email != u.email)) AS dup_email - FROM ` + userDialect + ` AS u` - return sqlQuery -} diff --git a/pkg/services/login/authinfoservice/service.go b/pkg/services/login/authinfoservice/service.go index e73832f1083..fbf3e0f9a2b 100644 --- a/pkg/services/login/authinfoservice/service.go +++ b/pkg/services/login/authinfoservice/service.go @@ -195,3 +195,8 @@ func (s *Implementation) SetAuthInfo(ctx context.Context, cmd *models.SetAuthInf func (s *Implementation) GetExternalUserInfoByLogin(ctx context.Context, query *models.GetExternalUserInfoByLoginQuery) error { return s.authInfoStore.GetExternalUserInfoByLogin(ctx, query) } + +func (s *Implementation) Run(ctx context.Context) error { + s.logger.Debug("Started AuthInfo Metrics collection service") + return s.authInfoStore.RunMetricsCollection(ctx) +} diff --git a/pkg/services/login/authinfoservice/user_auth_test.go b/pkg/services/login/authinfoservice/user_auth_test.go index 483791eb650..aaf4034d7c7 100644 --- a/pkg/services/login/authinfoservice/user_auth_test.go +++ b/pkg/services/login/authinfoservice/user_auth_test.go @@ -421,11 +421,14 @@ func TestUserAuth(t *testing.T) { } _, err = sqlStore.CreateUser(context.Background(), dupUserLogincmd) require.NoError(t, err) - // require metrics and statistics to be 2 + + // require stats to populate m, err := srv.authInfoStore.CollectLoginStats(context.Background()) require.NoError(t, err) require.Equal(t, 2, m["stats.users.duplicate_user_entries"]) require.Equal(t, 1, m["stats.users.has_duplicate_user_entries"]) + + require.Equal(t, 1, m["stats.users.mixed_cased_users"]) } }) }) diff --git a/pkg/services/login/userprotection.go b/pkg/services/login/userprotection.go index e400091b229..7cedf2e3be1 100644 --- a/pkg/services/login/userprotection.go +++ b/pkg/services/login/userprotection.go @@ -22,5 +22,6 @@ type Store interface { GetUserByLogin(ctx context.Context, login string) (*models.User, error) GetUserByEmail(ctx context.Context, email string) (*models.User, error) CollectLoginStats(ctx context.Context) (map[string]interface{}, error) + RunMetricsCollection(ctx context.Context) error GetLoginStats(ctx context.Context) (database.LoginStats, error) }