Auth: Add prometheus metrics for case insensitive ids (#52162)

* rename file

* add promstats for authinfostore every 4h

* add mixed cased users

* cherry pick and cleanup

* fix: query login,email instead of count()

* FIX: mysql missing alias to subquery

* fix: query working on mysql, sqllite3, and pg
This commit is contained in:
Eric Leijonmarck 2022-07-21 18:11:35 +01:00 committed by GitHub
parent 7251b8524f
commit fb24ff3081
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 188 additions and 59 deletions

View File

@ -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"
@ -40,7 +41,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,
saService *samanager.ServiceAccountsService,
saService *samanager.ServiceAccountsService, 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,
@ -71,6 +72,7 @@ func ProvideBackgroundServiceRegistry(
searchService,
entityEventsService,
saService,
authInfoService,
)
}

View File

@ -26,6 +26,7 @@ func ProvideAuthInfoStore(sqlStore sqlstore.Store, secretsService secrets.Servic
secretsService: secretsService,
logger: log.New("login.authinfo.store"),
}
InitMetrics()
return store
}

View File

@ -0,0 +1,150 @@
package database
import (
"context"
"sync"
"time"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/db"
"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 := db.DB.GetDialect(s.sqlStore).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
}

View File

@ -1,57 +0,0 @@
package database
import (
"context"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/db"
)
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) + `)`
_, 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 := db.DB.GetDialect(s.sqlStore).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
WHERE (dup_login IS NOT NULL OR dup_email IS NOT NULL)
`
return sqlQuery
}

View File

@ -196,3 +196,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)
}

View File

@ -371,6 +371,28 @@ func TestUserAuth(t *testing.T) {
require.Nil(t, user)
})
t.Run("should be able to run loginstats query in all dbs", func(t *testing.T) {
// we need to see that we can run queries for all db
// as it is only a concern for postgres/sqllite3
// where we have duplicate users
// Restore after destructive operation
sqlStore = sqlstore.InitTestDB(t)
for i := 0; i < 5; i++ {
cmd := user.CreateUserCommand{
Email: fmt.Sprint("user", i, "@test.com"),
Name: fmt.Sprint("user", i),
Login: fmt.Sprint("loginuser", i),
OrgID: 1,
}
_, err := sqlStore.CreateUser(context.Background(), cmd)
require.Nil(t, err)
}
_, err := srv.authInfoStore.GetLoginStats(context.Background())
require.Nil(t, err)
})
t.Run("calculate metrics on duplicate userstats", func(t *testing.T) {
// Restore after destructive operation
sqlStore = sqlstore.InitTestDB(t)
@ -404,11 +426,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"])
}
})
})

View File

@ -4,6 +4,7 @@ import (
"context"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/login/authinfoservice/database"
"github.com/grafana/grafana/pkg/services/user"
)
@ -22,4 +23,6 @@ type Store interface {
GetUserByLogin(ctx context.Context, login string) (*user.User, error)
GetUserByEmail(ctx context.Context, email string) (*user.User, error)
CollectLoginStats(ctx context.Context) (map[string]interface{}, error)
RunMetricsCollection(ctx context.Context) error
GetLoginStats(ctx context.Context) (database.LoginStats, error)
}