diff --git a/pkg/api/user_test.go b/pkg/api/user_test.go index 4e59ac05d8c..0879f79dd54 100644 --- a/pkg/api/user_test.go +++ b/pkg/api/user_test.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/models" acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/login/authinfoservice" @@ -48,7 +49,11 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) { fakeNow := time.Date(2019, 2, 11, 17, 30, 40, 0, time.UTC) secretsService := secretsManager.SetupTestService(t, database.ProvideSecretsStore(sqlStore)) authInfoStore := authinfostore.ProvideAuthInfoStore(sqlStore, secretsService) - srv := authinfoservice.ProvideAuthInfoService(&authinfoservice.OSSUserProtectionImpl{}, authInfoStore) + srv := authinfoservice.ProvideAuthInfoService( + &authinfoservice.OSSUserProtectionImpl{}, + authInfoStore, + &usagestats.UsageStatsMock{}, + ) hs.authInfoService = srv createUserCmd := models.CreateUserCommand{ diff --git a/pkg/services/login/authinfoservice/database/usagestats.go b/pkg/services/login/authinfoservice/database/usagestats.go new file mode 100644 index 00000000000..ed949400864 --- /dev/null +++ b/pkg/services/login/authinfoservice/database/usagestats.go @@ -0,0 +1,57 @@ +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 +} diff --git a/pkg/services/login/authinfoservice/service.go b/pkg/services/login/authinfoservice/service.go index 8fc26ab7b10..b14b9dfd965 100644 --- a/pkg/services/login/authinfoservice/service.go +++ b/pkg/services/login/authinfoservice/service.go @@ -5,6 +5,7 @@ import ( "errors" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/login" ) @@ -17,13 +18,13 @@ type Implementation struct { logger log.Logger } -func ProvideAuthInfoService(userProtectionService login.UserProtectionService, authInfoStore login.Store) *Implementation { +func ProvideAuthInfoService(userProtectionService login.UserProtectionService, authInfoStore login.Store, usageStats usagestats.Service) *Implementation { s := &Implementation{ UserProtectionService: userProtectionService, authInfoStore: authInfoStore, logger: log.New("login.authinfo"), } - + usageStats.RegisterMetricsFunc(authInfoStore.CollectLoginStats) return s } diff --git a/pkg/services/login/authinfoservice/user_auth_test.go b/pkg/services/login/authinfoservice/user_auth_test.go index e03640be847..5189a7d415e 100644 --- a/pkg/services/login/authinfoservice/user_auth_test.go +++ b/pkg/services/login/authinfoservice/user_auth_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/login/authinfoservice/database" secretstore "github.com/grafana/grafana/pkg/services/secrets/database" @@ -20,7 +21,11 @@ func TestUserAuth(t *testing.T) { sqlStore := sqlstore.InitTestDB(t) secretsService := secretsManager.SetupTestService(t, secretstore.ProvideSecretsStore(sqlStore)) authInfoStore := database.ProvideAuthInfoStore(sqlStore, secretsService) - srv := ProvideAuthInfoService(&OSSUserProtectionImpl{}, authInfoStore) + srv := ProvideAuthInfoService( + &OSSUserProtectionImpl{}, + authInfoStore, + &usagestats.UsageStatsMock{}, + ) t.Run("Given 5 users", func(t *testing.T) { for i := 0; i < 5; i++ { @@ -343,5 +348,46 @@ func TestUserAuth(t *testing.T) { require.NotNil(t, err) require.Nil(t, user) }) + + t.Run("calculate metrics on duplicate userstats", func(t *testing.T) { + // Restore after destructive operation + sqlStore = sqlstore.InitTestDB(t) + + for i := 0; i < 5; i++ { + cmd := models.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) + } + + // "Skipping duplicate users test for mysql as it does make unique constraint case insensitive by default + if sqlStore.GetDialect().DriverName() != "mysql" { + dupUserEmailcmd := models.CreateUserCommand{ + Email: "USERDUPLICATETEST1@TEST.COM", + Name: "user name 1", + Login: "USER_DUPLICATE_TEST_1_LOGIN", + } + _, err := sqlStore.CreateUser(context.Background(), dupUserEmailcmd) + require.NoError(t, err) + + // add additional user with duplicate login where DOMAIN is upper case + dupUserLogincmd := models.CreateUserCommand{ + Email: "userduplicatetest1@test.com", + Name: "user name 1", + Login: "user_duplicate_test_1_login", + } + _, err = sqlStore.CreateUser(context.Background(), dupUserLogincmd) + require.NoError(t, err) + // require metrics and statistics to be 2 + 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"]) + } + }) }) } diff --git a/pkg/services/login/loginservice/loginservice.go b/pkg/services/login/loginservice/loginservice.go index ea9338602b7..3879b50c257 100644 --- a/pkg/services/login/loginservice/loginservice.go +++ b/pkg/services/login/loginservice/loginservice.go @@ -15,7 +15,11 @@ var ( logger = log.New("login.ext_user") ) -func ProvideService(sqlStore sqlstore.Store, quotaService *quota.QuotaService, authInfoService login.AuthInfoService) *Implementation { +func ProvideService( + sqlStore sqlstore.Store, + quotaService *quota.QuotaService, + authInfoService login.AuthInfoService, +) *Implementation { s := &Implementation{ SQLStore: sqlStore, QuotaService: quotaService, diff --git a/pkg/services/login/userprotection.go b/pkg/services/login/userprotection.go index bdd8c23e255..0a36fd3b900 100644 --- a/pkg/services/login/userprotection.go +++ b/pkg/services/login/userprotection.go @@ -20,4 +20,5 @@ type Store interface { GetUserById(ctx context.Context, id int64) (*models.User, error) 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) } diff --git a/pkg/services/sqlstore/mockstore/mockstore.go b/pkg/services/sqlstore/mockstore/mockstore.go index e29509e5af2..5ea09a35652 100644 --- a/pkg/services/sqlstore/mockstore/mockstore.go +++ b/pkg/services/sqlstore/mockstore/mockstore.go @@ -5,6 +5,7 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/services/sqlstore/migrator" ) type OrgListResponse []struct { @@ -72,6 +73,10 @@ func (m *SQLStoreMock) GetSystemStats(ctx context.Context, query *models.GetSyst return m.ExpectedError } +func (m *SQLStoreMock) GetDialect() migrator.Dialect { + return nil +} + func (m *SQLStoreMock) HasEditPermissionInFolders(ctx context.Context, query *models.HasEditPermissionInFoldersQuery) error { return m.ExpectedError } diff --git a/pkg/services/sqlstore/store.go b/pkg/services/sqlstore/store.go index 422b30c7917..e9d2f6ce75e 100644 --- a/pkg/services/sqlstore/store.go +++ b/pkg/services/sqlstore/store.go @@ -4,6 +4,7 @@ import ( "context" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/sqlstore/migrator" ) type Store interface { @@ -11,6 +12,7 @@ type Store interface { GetAlertNotifiersUsageStats(ctx context.Context, query *models.GetAlertNotifierUsageStatsQuery) error GetDataSourceStats(ctx context.Context, query *models.GetDataSourceStatsQuery) error GetDataSourceAccessStats(ctx context.Context, query *models.GetDataSourceAccessStatsQuery) error + GetDialect() migrator.Dialect GetSystemStats(ctx context.Context, query *models.GetSystemStatsQuery) error GetOrgByName(name string) (*models.Org, error) CreateOrg(ctx context.Context, cmd *models.CreateOrgCommand) error