From 1c7f89c41b56731123e5b3e8a234252eb7e4b06a Mon Sep 17 00:00:00 2001 From: linoman <2051016+linoman@users.noreply.github.com> Date: Wed, 16 Aug 2023 10:56:47 +0200 Subject: [PATCH] Auth: Add empty role usage metrics for service and user accounts (#73108) * Add tests for service accounts metrics usage * Add service account store implementation * Add service account service implementation * Add tests for org metrics usage * Add org implementation * Add service implementation --- .../serviceaccounts/database/stats.go | 12 +++-- .../serviceaccounts/database/stats_test.go | 9 ++++ pkg/services/serviceaccounts/manager/stats.go | 14 +++++- .../serviceaccounts/manager/stats_test.go | 10 ++-- pkg/services/serviceaccounts/models.go | 7 +-- pkg/services/user/userimpl/store.go | 22 +++++++++ pkg/services/user/userimpl/store_test.go | 47 +++++++++++++++++++ pkg/services/user/userimpl/user.go | 8 ++++ pkg/services/user/userimpl/user_test.go | 43 ++++++++++++++--- 9 files changed, 155 insertions(+), 17 deletions(-) diff --git a/pkg/services/serviceaccounts/database/stats.go b/pkg/services/serviceaccounts/database/stats.go index 7ade76f29ac..020419a1181 100644 --- a/pkg/services/serviceaccounts/database/stats.go +++ b/pkg/services/serviceaccounts/database/stats.go @@ -12,10 +12,16 @@ func (s *ServiceAccountsStoreImpl) GetUsageMetrics(ctx context.Context) (*servic sb := &db.SQLBuilder{} sb.Write("SELECT ") - sb.Write(`(SELECT COUNT(*) FROM ` + dialect.Quote("user") + - ` WHERE is_service_account = ` + dialect.BooleanStr(true) + `) AS serviceaccounts,`) + sb.Write(`(SELECT COUNT(*) FROM ` + dialect.Quote("user") + ` ` + + `WHERE is_service_account = ` + dialect.BooleanStr(true) + `) AS serviceaccounts,`) sb.Write(`(SELECT COUNT(*) FROM ` + dialect.Quote("api_key") + - ` WHERE service_account_id IS NOT NULL ) AS serviceaccount_tokens`) + `WHERE service_account_id IS NOT NULL ) AS serviceaccount_tokens,`) + sb.Write(`(SELECT COUNT(*) FROM ` + dialect.Quote("org_user") + ` AS ou ` + + `JOIN ` + dialect.Quote("user") + ` AS u ON u.id = ou.user_id ` + + `WHERE u.is_disabled = ` + dialect.BooleanStr(false) + ` ` + + `AND u.is_service_account = ` + dialect.BooleanStr(true) + ` ` + + `AND ou.role=?) AS serviceaccounts_with_no_role`) + sb.AddParams("None") var sqlStats serviceaccounts.Stats if err := s.sqlStore.WithDbSession(ctx, func(sess *db.Session) error { diff --git a/pkg/services/serviceaccounts/database/stats_test.go b/pkg/services/serviceaccounts/database/stats_test.go index eb45febc609..0050a8c03b4 100644 --- a/pkg/services/serviceaccounts/database/stats_test.go +++ b/pkg/services/serviceaccounts/database/stats_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/components/satokengen" + "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/serviceaccounts/tests" ) @@ -33,10 +34,18 @@ func TestIntegrationStore_UsageStats(t *testing.T) { _, err = store.AddServiceAccountToken(context.Background(), sa.ID, &cmd) require.NoError(t, err) + role := org.RoleNone + form := serviceaccounts.UpdateServiceAccountForm{ + Role: &role, + } + _, err = store.UpdateServiceAccount(context.Background(), sa.OrgID, sa.ID, &form) + require.NoError(t, err) + stats, err := store.GetUsageMetrics(context.Background()) require.NoError(t, err) assert.Equal(t, int64(1), stats.ServiceAccounts) + assert.Equal(t, int64(1), stats.ServiceAccountsWithNoRole) assert.Equal(t, int64(1), stats.Tokens) assert.Equal(t, true, stats.ForcedExpiryEnabled) } diff --git a/pkg/services/serviceaccounts/manager/stats.go b/pkg/services/serviceaccounts/manager/stats.go index 3d5adc779e7..f0c4f32d510 100644 --- a/pkg/services/serviceaccounts/manager/stats.go +++ b/pkg/services/serviceaccounts/manager/stats.go @@ -14,6 +14,9 @@ var ( // MStatTotalServiceAccounts is a metric gauge for total number of service accounts MStatTotalServiceAccounts prometheus.Gauge + // MStatTotalServiceAccountsNoRole is a metric gauge for total number of user accounts with no role + MStatTotalServiceAccountsNoRole prometheus.Gauge + // MStatTotalServiceAccountTokens is a metric gauge for total number of service account tokens MStatTotalServiceAccountTokens prometheus.Gauge @@ -27,6 +30,12 @@ func init() { Namespace: ExporterName, }) + MStatTotalServiceAccountsNoRole = prometheus.NewGauge(prometheus.GaugeOpts{ + Name: "stat_total_service_accounts_role_none", + Help: "total amount of service accounts with no role", + Namespace: ExporterName, + }) + MStatTotalServiceAccountTokens = prometheus.NewGauge(prometheus.GaugeOpts{ Name: "stat_total_service_account_tokens", Help: "total amount of service account tokens", @@ -36,6 +45,7 @@ func init() { prometheus.MustRegister( MStatTotalServiceAccounts, MStatTotalServiceAccountTokens, + MStatTotalServiceAccountsNoRole, ) } @@ -48,6 +58,7 @@ func (sa *ServiceAccountsService) getUsageMetrics(ctx context.Context) (map[stri } stats["stats.serviceaccounts.count"] = storeStats.ServiceAccounts + stats["stats.serviceaccounts.role_none.count"] = storeStats.ServiceAccountsWithNoRole stats["stats.serviceaccounts.tokens.count"] = storeStats.Tokens var forcedExpiryEnabled int64 = 0 @@ -64,8 +75,9 @@ func (sa *ServiceAccountsService) getUsageMetrics(ctx context.Context) (map[stri stats["stats.serviceaccounts.secret_scan.enabled.count"] = secretScanEnabled - MStatTotalServiceAccountTokens.Set(float64(storeStats.Tokens)) MStatTotalServiceAccounts.Set(float64(storeStats.ServiceAccounts)) + MStatTotalServiceAccountsNoRole.Set(float64(storeStats.ServiceAccountsWithNoRole)) + MStatTotalServiceAccountTokens.Set(float64(storeStats.Tokens)) return stats, nil } diff --git a/pkg/services/serviceaccounts/manager/stats_test.go b/pkg/services/serviceaccounts/manager/stats_test.go index 84e878d76ba..3f9099a6f46 100644 --- a/pkg/services/serviceaccounts/manager/stats_test.go +++ b/pkg/services/serviceaccounts/manager/stats_test.go @@ -18,15 +18,17 @@ func Test_UsageStats(t *testing.T) { require.NoError(t, err) storeMock.ExpectedStats = &serviceaccounts.Stats{ - ServiceAccounts: 1, - Tokens: 1, - ForcedExpiryEnabled: false, + ServiceAccounts: 1, + ServiceAccountsWithNoRole: 1, + Tokens: 1, + ForcedExpiryEnabled: false, } stats, err := svc.getUsageMetrics(context.Background()) require.NoError(t, err) - assert.Len(t, stats, 4, stats) + assert.Len(t, stats, 5, stats) assert.Equal(t, int64(1), stats["stats.serviceaccounts.count"].(int64)) + assert.Equal(t, int64(1), stats["stats.serviceaccounts.role_none.count"].(int64)) assert.Equal(t, int64(1), stats["stats.serviceaccounts.tokens.count"].(int64)) assert.Equal(t, int64(1), stats["stats.serviceaccounts.secret_scan.enabled.count"].(int64)) assert.Equal(t, int64(0), stats["stats.serviceaccounts.forced_expiry_enabled.count"].(int64)) diff --git a/pkg/services/serviceaccounts/models.go b/pkg/services/serviceaccounts/models.go index 3418d8e41fd..fb6af518cba 100644 --- a/pkg/services/serviceaccounts/models.go +++ b/pkg/services/serviceaccounts/models.go @@ -160,9 +160,10 @@ const ( ) type Stats struct { - ServiceAccounts int64 `xorm:"serviceaccounts"` - Tokens int64 `xorm:"serviceaccount_tokens"` - ForcedExpiryEnabled bool `xorm:"-"` + ServiceAccounts int64 `xorm:"serviceaccounts"` + ServiceAccountsWithNoRole int64 `xorm:"serviceaccounts_with_no_role"` + Tokens int64 `xorm:"serviceaccount_tokens"` + ForcedExpiryEnabled bool `xorm:"-"` } // AccessEvaluator is used to protect the "Configuration > Service accounts" page access diff --git a/pkg/services/user/userimpl/store.go b/pkg/services/user/userimpl/store.go index 54680dc6a9a..d9da5a44f9d 100644 --- a/pkg/services/user/userimpl/store.go +++ b/pkg/services/user/userimpl/store.go @@ -40,6 +40,7 @@ type store interface { Search(context.Context, *user.SearchUsersQuery) (*user.SearchUserQueryResult, error) Count(ctx context.Context) (int64, error) + CountUserAccountsWithEmptyRole(ctx context.Context) (int64, error) } type sqlStore struct { @@ -532,6 +533,27 @@ func (ss *sqlStore) Count(ctx context.Context) (int64, error) { return r.Count, err } +func (ss *sqlStore) CountUserAccountsWithEmptyRole(ctx context.Context) (int64, error) { + sb := &db.SQLBuilder{} + sb.Write("SELECT ") + sb.Write(`(SELECT COUNT (*) from ` + ss.dialect.Quote("org_user") + ` AS ou ` + + `LEFT JOIN ` + ss.dialect.Quote("user") + ` AS u ON u.id = ou.user_id ` + + `WHERE ou.role =? ` + + `AND u.is_service_account = ` + ss.dialect.BooleanStr(false) + ` ` + + `AND u.is_disabled = ` + ss.dialect.BooleanStr(false) + `) AS user_accounts_with_no_role`) + sb.AddParams("None") + + var countStats int64 + if err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { + _, err := sess.SQL(sb.GetSQLString(), sb.GetParams()...).Get(&countStats) + return err + }); err != nil { + return -1, err + } + + return countStats, nil +} + // validateOneAdminLeft validate that there is an admin user left func validateOneAdminLeft(ctx context.Context, sess *db.Session) error { count, err := sess.Where("is_admin=?", true).Count(&user.User{}) diff --git a/pkg/services/user/userimpl/store_test.go b/pkg/services/user/userimpl/store_test.go index 1291d1d4cbc..0d785b9cc7d 100644 --- a/pkg/services/user/userimpl/store_test.go +++ b/pkg/services/user/userimpl/store_test.go @@ -956,6 +956,53 @@ func updateDashboardACL(t *testing.T, sqlStore db.DB, dashboardID int64, items . return err } +func TestMetricsUsage(t *testing.T) { + ss := db.InitTestDB(t) + userStore := ProvideStore(ss, setting.NewCfg()) + quotaService := quotaimpl.ProvideService(ss, ss.Cfg) + orgService, err := orgimpl.ProvideService(ss, ss.Cfg, quotaService) + require.NoError(t, err) + + _, usrSvc := createOrgAndUserSvc(t, ss, ss.Cfg) + + t.Run("", func(t *testing.T) { + orgId := int64(1) + + // create first user + createFirtUserCmd := &user.CreateUserCommand{ + Login: "admin", + Email: "admin@admin.com", + Name: "admin", + OrgID: orgId, + } + _, err := usrSvc.Create(context.Background(), createFirtUserCmd) + require.NoError(t, err) + + // create second user + createSecondUserCmd := &user.CreateUserCommand{ + Login: "userWithoutRole", + Email: "userWithoutRole@userWithoutRole.com", + Name: "userWithoutRole", + } + secondUser, err := usrSvc.Create(context.Background(), createSecondUserCmd) + require.NoError(t, err) + + // assign the user to the org + cmd := org.AddOrgUserCommand{ + OrgID: secondUser.OrgID, + UserID: orgId, + Role: org.RoleNone, + } + err = orgService.AddOrgUser(context.Background(), &cmd) + require.NoError(t, err) + + // get metric usage + stats, err := userStore.CountUserAccountsWithEmptyRole(context.Background()) + require.NoError(t, err) + assert.Equal(t, int64(1), stats) + }) +} + // This function was copied from pkg/services/dashboards/database to circumvent // import cycles. When this org-related code is refactored into a service the // tests can the real GetDashboardACLInfoList functions diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index 43751d3cb87..a556ddf19dd 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -72,6 +72,14 @@ func (s *Service) GetUsageStats(ctx context.Context) map[string]interface{} { } stats["stats.case_insensitive_login.count"] = caseInsensitiveLoginVal + + count, err := s.store.CountUserAccountsWithEmptyRole(ctx) + if err != nil { + return nil + } + + stats["stats.user.role_none.count"] = count + return stats } diff --git a/pkg/services/user/userimpl/user_test.go b/pkg/services/user/userimpl/user_test.go index b66c8996dd6..43c9c5d3b5d 100644 --- a/pkg/services/user/userimpl/user_test.go +++ b/pkg/services/user/userimpl/user_test.go @@ -193,13 +193,40 @@ func TestUserService(t *testing.T) { }) } +func TestMetrics(t *testing.T) { + userStore := newUserStoreFake() + orgService := orgtest.NewOrgServiceFake() + + userService := Service{ + store: userStore, + orgService: orgService, + cacheService: localcache.ProvideService(), + teamService: &teamtest.FakeService{}, + } + + t.Run("update user with role None", func(t *testing.T) { + userStore.ExpectedCountUserAccountsWithEmptyRoles = int64(1) + + userService.cfg = setting.NewCfg() + userService.cfg.CaseInsensitiveLogin = true + + stats := userService.GetUsageStats(context.Background()) + assert.NotEmpty(t, stats) + + assert.Len(t, stats, 2, stats) + assert.Equal(t, 1, stats["stats.case_insensitive_login.count"]) + assert.Equal(t, int64(1), stats["stats.user.role_none.count"]) + }) +} + type FakeUserStore struct { - ExpectedUser *user.User - ExpectedSignedInUser *user.SignedInUser - ExpectedUserProfile *user.UserProfileDTO - ExpectedSearchUserQueryResult *user.SearchUserQueryResult - ExpectedError error - ExpectedDeleteUserError error + ExpectedUser *user.User + ExpectedSignedInUser *user.SignedInUser + ExpectedUserProfile *user.UserProfileDTO + ExpectedSearchUserQueryResult *user.SearchUserQueryResult + ExpectedError error + ExpectedDeleteUserError error + ExpectedCountUserAccountsWithEmptyRoles int64 } func newUserStoreFake() *FakeUserStore { @@ -290,6 +317,10 @@ func (f *FakeUserStore) Count(ctx context.Context) (int64, error) { return 0, nil } +func (f *FakeUserStore) CountUserAccountsWithEmptyRole(ctx context.Context) (int64, error) { + return f.ExpectedCountUserAccountsWithEmptyRoles, nil +} + func TestUpdateLastSeenAt(t *testing.T) { userStore := newUserStoreFake() orgService := orgtest.NewOrgServiceFake()