diff --git a/pkg/services/apikey/apikeyimpl/store.go b/pkg/services/apikey/apikeyimpl/store.go index 3f4dd84802d..c1bddd4f6e4 100644 --- a/pkg/services/apikey/apikeyimpl/store.go +++ b/pkg/services/apikey/apikeyimpl/store.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore/db" "github.com/grafana/grafana/pkg/setting" + "github.com/pkg/errors" "xorm.io/xorm" ) @@ -111,6 +112,7 @@ func (ss *sqlStore) AddAPIKey(ctx context.Context, cmd *apikey.AddCommand) error return apikey.ErrInvalidExpiration } + isRevoked := false t := apikey.APIKey{ OrgId: cmd.OrgId, Name: cmd.Name, @@ -119,12 +121,14 @@ func (ss *sqlStore) AddAPIKey(ctx context.Context, cmd *apikey.AddCommand) error Created: updated, Updated: updated, Expires: expires, - ServiceAccountId: nil, + ServiceAccountId: cmd.ServiceAccountID, + IsRevoked: &isRevoked, } if _, err := sess.Insert(&t); err != nil { - return err + return errors.Wrap(err, "failed to insert token") } + cmd.Result = &t return nil }) diff --git a/pkg/services/apikey/model.go b/pkg/services/apikey/model.go index 22fbc8ad2a4..8e8e13ee407 100644 --- a/pkg/services/apikey/model.go +++ b/pkg/services/apikey/model.go @@ -26,18 +26,21 @@ type APIKey struct { LastUsedAt *time.Time `xorm:"last_used_at"` Expires *int64 ServiceAccountId *int64 + IsRevoked *bool `xorm:"is_revoked"` } func (k APIKey) TableName() string { return "api_key" } // swagger:model type AddCommand struct { - Name string `json:"name" binding:"Required"` - Role org.RoleType `json:"role" binding:"Required"` - OrgId int64 `json:"-"` - Key string `json:"-"` - SecondsToLive int64 `json:"secondsToLive"` - Result *APIKey `json:"-"` + Name string `json:"name" binding:"Required"` + Role org.RoleType `json:"role" binding:"Required"` + OrgId int64 `json:"-"` + Key string `json:"-"` + SecondsToLive int64 `json:"secondsToLive"` + ServiceAccountID *int64 `json:"-"` + + Result *APIKey `json:"-"` } type DeleteCommand struct { diff --git a/pkg/services/contexthandler/contexthandler.go b/pkg/services/contexthandler/contexthandler.go index f255118523d..7f29e0fee1e 100644 --- a/pkg/services/contexthandler/contexthandler.go +++ b/pkg/services/contexthandler/contexthandler.go @@ -281,6 +281,12 @@ func (h *ContextHandler) initContextWithAPIKey(reqContext *models.ReqContext) bo return true } + if apikey.IsRevoked != nil && *apikey.IsRevoked { + reqContext.JsonApiErr(http.StatusUnauthorized, "Revoked token", nil) + + return true + } + // update api_key last used date if err := h.apiKeyService.UpdateAPIKeyLastUsedDate(reqContext.Req.Context(), apikey.Id); err != nil { reqContext.JsonApiErr(http.StatusInternalServerError, InvalidAPIKey, errKey) diff --git a/pkg/services/serviceaccounts/api/api.go b/pkg/services/serviceaccounts/api/api.go index dccd7e44813..745209c09ed 100644 --- a/pkg/services/serviceaccounts/api/api.go +++ b/pkg/services/serviceaccounts/api/api.go @@ -83,7 +83,7 @@ func (api *ServiceAccountsAPI) RegisterAPIEndpoints() { // swagger:route POST /serviceaccounts service_accounts createServiceAccount // -// Create service account +// # Create service account // // Required permissions (See note in the [introduction](https://grafana.com/docs/grafana/latest/developers/http_api/serviceaccount/#service-account-api) for an explanation): // action: `serviceaccounts:write` scope: `serviceaccounts:*` @@ -134,7 +134,7 @@ func (api *ServiceAccountsAPI) CreateServiceAccount(c *models.ReqContext) respon // swagger:route GET /serviceaccounts/{serviceAccountId} service_accounts retrieveServiceAccount // -// Get single serviceaccount by Id +// # Get single serviceaccount by Id // // Required permissions (See note in the [introduction](https://grafana.com/docs/grafana/latest/developers/http_api/serviceaccount/#service-account-api) for an explanation): // action: `serviceaccounts:read` scope: `serviceaccounts:id:1` (single service account) @@ -167,7 +167,10 @@ func (api *ServiceAccountsAPI) RetrieveServiceAccount(ctx *models.ReqContext) re serviceAccount.AvatarUrl = dtos.GetGravatarUrlWithDefault("", serviceAccount.Name) serviceAccount.AccessControl = metadata[saIDString] - tokens, err := api.store.ListTokens(ctx.Req.Context(), serviceAccount.OrgId, serviceAccount.Id) + tokens, err := api.store.ListTokens(ctx.Req.Context(), &serviceaccounts.GetSATokensQuery{ + OrgID: &serviceAccount.OrgId, + ServiceAccountID: &serviceAccount.Id, + }) if err != nil { api.log.Warn("Failed to list tokens for service account", "serviceAccount", serviceAccount.Id) } @@ -178,7 +181,7 @@ func (api *ServiceAccountsAPI) RetrieveServiceAccount(ctx *models.ReqContext) re // swagger:route PATCH /serviceaccounts/{serviceAccountId} service_accounts updateServiceAccount // -// Update service account +// # Update service account // // Required permissions (See note in the [introduction](https://grafana.com/docs/grafana/latest/developers/http_api/serviceaccount/#service-account-api) for an explanation): // action: `serviceaccounts:write` scope: `serviceaccounts:id:1` (single service account) @@ -247,7 +250,7 @@ func (api *ServiceAccountsAPI) validateRole(r *org.RoleType, orgRole *org.RoleTy // swagger:route DELETE /serviceaccounts/{serviceAccountId} service_accounts deleteServiceAccount // -// Delete service account +// # Delete service account // // Required permissions (See note in the [introduction](https://grafana.com/docs/grafana/latest/developers/http_api/serviceaccount/#service-account-api) for an explanation): // action: `serviceaccounts:delete` scope: `serviceaccounts:id:1` (single service account) @@ -272,7 +275,7 @@ func (api *ServiceAccountsAPI) DeleteServiceAccount(ctx *models.ReqContext) resp // swagger:route GET /serviceaccounts/search service_accounts searchOrgServiceAccountsWithPaging // -// Search service accounts with paging +// # Search service accounts with paging // // Required permissions (See note in the [introduction](https://grafana.com/docs/grafana/latest/developers/http_api/serviceaccount/#service-account-api) for an explanation): // action: `serviceaccounts:read` scope: `serviceaccounts:*` @@ -316,7 +319,9 @@ func (api *ServiceAccountsAPI) SearchOrgServiceAccountsWithPaging(c *models.ReqC saIDs[saIDString] = true metadata := api.getAccessControlMetadata(c, map[string]bool{saIDString: true}) sa.AccessControl = metadata[strconv.FormatInt(sa.Id, 10)] - tokens, err := api.store.ListTokens(ctx, sa.OrgId, sa.Id) + tokens, err := api.store.ListTokens(ctx, &serviceaccounts.GetSATokensQuery{ + OrgID: &sa.OrgId, ServiceAccountID: &sa.Id, + }) if err != nil { api.log.Warn("Failed to list tokens for service account", "serviceAccount", sa.Id) } diff --git a/pkg/services/serviceaccounts/api/token.go b/pkg/services/serviceaccounts/api/token.go index 236633e1870..c1f31191dbd 100644 --- a/pkg/services/serviceaccounts/api/token.go +++ b/pkg/services/serviceaccounts/api/token.go @@ -37,6 +37,8 @@ type TokenDTO struct { SecondsUntilExpiration *float64 `json:"secondsUntilExpiration"` // example: false HasExpired bool `json:"hasExpired"` + // example: false + IsRevoked *bool `json:"isRevoked"` } func hasExpired(expiration *int64) bool { @@ -51,7 +53,7 @@ const sevenDaysAhead = 7 * 24 * time.Hour // swagger:route GET /serviceaccounts/{serviceAccountId}/tokens service_accounts listTokens // -// Get service account tokens +// # Get service account tokens // // Required permissions (See note in the [introduction](https://grafana.com/docs/grafana/latest/developers/http_api/serviceaccount/#service-account-api) for an explanation): // action: `serviceaccounts:read` scope: `global:serviceaccounts:id:1` (single service account) @@ -70,15 +72,21 @@ func (api *ServiceAccountsAPI) ListTokens(ctx *models.ReqContext) response.Respo return response.Error(http.StatusBadRequest, "Service Account ID is invalid", err) } - saTokens, err := api.store.ListTokens(ctx.Req.Context(), ctx.OrgID, saID) + saTokens, err := api.store.ListTokens(ctx.Req.Context(), &serviceaccounts.GetSATokensQuery{ + OrgID: &ctx.OrgID, + ServiceAccountID: &saID, + }) if err != nil { return response.Error(http.StatusInternalServerError, "Internal server error", err) } - result := make([]*TokenDTO, len(saTokens)) + result := make([]TokenDTO, len(saTokens)) for i, t := range saTokens { - var expiration *time.Time = nil - var secondsUntilExpiration float64 = 0 + var ( + token = t // pin pointer + expiration *time.Time = nil + secondsUntilExpiration float64 = 0 + ) isExpired := hasExpired(t.Expires) if t.Expires != nil { @@ -89,14 +97,15 @@ func (api *ServiceAccountsAPI) ListTokens(ctx *models.ReqContext) response.Respo } } - result[i] = &TokenDTO{ - Id: t.Id, - Name: t.Name, - Created: &t.Created, + result[i] = TokenDTO{ + Id: token.Id, + Name: token.Name, + Created: &token.Created, Expiration: expiration, SecondsUntilExpiration: &secondsUntilExpiration, HasExpired: isExpired, - LastUsedAt: t.LastUsedAt, + LastUsedAt: token.LastUsedAt, + IsRevoked: token.IsRevoked, } } @@ -105,7 +114,7 @@ func (api *ServiceAccountsAPI) ListTokens(ctx *models.ReqContext) response.Respo // swagger:route POST /serviceaccounts/{serviceAccountId}/tokens service_accounts createToken // -// CreateNewToken adds a token to a service account +// # CreateNewToken adds a token to a service account // // Required permissions (See note in the [introduction](https://grafana.com/docs/grafana/latest/developers/http_api/serviceaccount/#service-account-api) for an explanation): // action: `serviceaccounts:write` scope: `serviceaccounts:id:1` (single service account) @@ -179,7 +188,7 @@ func (api *ServiceAccountsAPI) CreateToken(c *models.ReqContext) response.Respon // swagger:route DELETE /serviceaccounts/{serviceAccountId}/tokens/{tokenId} service_accounts deleteToken // -// DeleteToken deletes service account tokens +// # DeleteToken deletes service account tokens // // Required permissions (See note in the [introduction](https://grafana.com/docs/grafana/latest/developers/http_api/serviceaccount/#service-account-api) for an explanation): // action: `serviceaccounts:write` scope: `serviceaccounts:id:1` (single service account) diff --git a/pkg/services/serviceaccounts/api/token_test.go b/pkg/services/serviceaccounts/api/token_test.go index 7545e437413..b820b9660f1 100644 --- a/pkg/services/serviceaccounts/api/token_test.go +++ b/pkg/services/serviceaccounts/api/token_test.go @@ -259,10 +259,10 @@ func TestServiceAccountsAPI_DeleteToken(t *testing.T) { type saStoreMockTokens struct { serviceaccounts.Store - saAPIKeys []*apikey.APIKey + saAPIKeys []apikey.APIKey } -func (s *saStoreMockTokens) ListTokens(ctx context.Context, orgID, saID int64) ([]*apikey.APIKey, error) { +func (s *saStoreMockTokens) ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) { return s.saAPIKeys, nil } @@ -273,7 +273,7 @@ func TestServiceAccountsAPI_ListTokens(t *testing.T) { type testCreateSAToken struct { desc string - tokens []*apikey.APIKey + tokens []apikey.APIKey expectedHasExpired bool expectedResponseBodyField string expectedCode int @@ -287,7 +287,7 @@ func TestServiceAccountsAPI_ListTokens(t *testing.T) { testCases := []testCreateSAToken{ { desc: "should be able to list serviceaccount with no expiration date", - tokens: []*apikey.APIKey{{ + tokens: []apikey.APIKey{{ Id: 1, OrgId: 1, ServiceAccountId: &saId, @@ -307,7 +307,7 @@ func TestServiceAccountsAPI_ListTokens(t *testing.T) { }, { desc: "should be able to list serviceaccount with secondsUntilExpiration", - tokens: []*apikey.APIKey{{ + tokens: []apikey.APIKey{{ Id: 1, OrgId: 1, ServiceAccountId: &saId, @@ -327,7 +327,7 @@ func TestServiceAccountsAPI_ListTokens(t *testing.T) { }, { desc: "should be able to list serviceaccount with expired token", - tokens: []*apikey.APIKey{{ + tokens: []apikey.APIKey{{ Id: 1, OrgId: 1, ServiceAccountId: &saId, diff --git a/pkg/services/serviceaccounts/database/database.go b/pkg/services/serviceaccounts/database/database.go index 30950e4d922..2525223dedf 100644 --- a/pkg/services/serviceaccounts/database/database.go +++ b/pkg/services/serviceaccounts/database/database.go @@ -483,7 +483,10 @@ func (s *ServiceAccountsStoreImpl) RevertApiKey(ctx context.Context, saId int64, return ErrServiceAccountAndTokenMismatch } - tokens, err := s.ListTokens(ctx, key.OrgId, *key.ServiceAccountId) + tokens, err := s.ListTokens(ctx, &serviceaccounts.GetSATokensQuery{ + OrgID: &key.OrgId, + ServiceAccountID: key.ServiceAccountId, + }) if err != nil { return fmt.Errorf("cannot revert token: %w", err) } diff --git a/pkg/services/serviceaccounts/database/database_test.go b/pkg/services/serviceaccounts/database/database_test.go index f2f90ce5020..4bd7327bf2a 100644 --- a/pkg/services/serviceaccounts/database/database_test.go +++ b/pkg/services/serviceaccounts/database/database_test.go @@ -185,7 +185,10 @@ func TestStore_MigrateApiKeys(t *testing.T) { saMigrated := serviceAccounts.ServiceAccounts[0] require.Equal(t, string(key.Role), saMigrated.Role) - tokens, err := store.ListTokens(context.Background(), key.OrgId, saMigrated.Id) + tokens, err := store.ListTokens(context.Background(), &serviceaccounts.GetSATokensQuery{ + OrgID: &key.OrgId, + ServiceAccountID: &saMigrated.Id, + }) require.NoError(t, err) require.Len(t, tokens, 1) } @@ -264,7 +267,10 @@ func TestStore_MigrateAllApiKeys(t *testing.T) { saMigrated := serviceAccounts.ServiceAccounts[0] require.Equal(t, string(c.keys[0].Role), saMigrated.Role) - tokens, err := store.ListTokens(context.Background(), c.orgId, saMigrated.Id) + tokens, err := store.ListTokens(context.Background(), &serviceaccounts.GetSATokensQuery{ + OrgID: &c.orgId, + ServiceAccountID: &saMigrated.Id, + }) require.NoError(t, err) require.Len(t, tokens, 1) } diff --git a/pkg/services/serviceaccounts/database/stats.go b/pkg/services/serviceaccounts/database/stats.go index 2d47333c9c5..a279e46805a 100644 --- a/pkg/services/serviceaccounts/database/stats.go +++ b/pkg/services/serviceaccounts/database/stats.go @@ -3,15 +3,13 @@ package database import ( "context" "sync" - "time" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/prometheus/client_golang/prometheus" ) const ( - ExporterName = "grafana" - metricsCollectionInterval = time.Minute * 30 + ExporterName = "grafana" ) var ( @@ -46,25 +44,6 @@ func InitMetrics() { }) } -func (s *ServiceAccountsStoreImpl) RunMetricsCollection(ctx context.Context) error { - if _, err := s.GetUsageMetrics(ctx); err != nil { - s.log.Warn("Failed to get usage metrics", "error", err.Error()) - } - updateStatsTicker := time.NewTicker(metricsCollectionInterval) - defer updateStatsTicker.Stop() - - for { - select { - case <-updateStatsTicker.C: - if _, err := s.GetUsageMetrics(ctx); err != nil { - s.log.Warn("Failed to get usage metrics", "error", err.Error()) - } - case <-ctx.Done(): - return ctx.Err() - } - } -} - func (s *ServiceAccountsStoreImpl) GetUsageMetrics(ctx context.Context) (map[string]interface{}, error) { stats := map[string]interface{}{} diff --git a/pkg/services/serviceaccounts/database/token_store.go b/pkg/services/serviceaccounts/database/token_store.go index 0cb704607f6..8e6f6d62e94 100644 --- a/pkg/services/serviceaccounts/database/token_store.go +++ b/pkg/services/serviceaccounts/database/token_store.go @@ -2,27 +2,37 @@ package database import ( "context" - "time" "github.com/grafana/grafana/pkg/services/apikey" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/sqlstore" - "xorm.io/xorm" + "github.com/pkg/errors" ) -func (s *ServiceAccountsStoreImpl) ListTokens(ctx context.Context, orgId int64, serviceAccountId int64) ([]*apikey.APIKey, error) { - result := make([]*apikey.APIKey, 0) - err := s.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error { - var sess *xorm.Session +const maxRetrievedTokens = 300 +func (s *ServiceAccountsStoreImpl) ListTokens( + ctx context.Context, query *serviceaccounts.GetSATokensQuery, +) ([]apikey.APIKey, error) { + result := make([]apikey.APIKey, 0) + err := s.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error { quotedUser := s.sqlStore.Dialect.Quote("user") - sess = dbSession. - Join("inner", quotedUser, quotedUser+".id = api_key.service_account_id"). - Where(quotedUser+".org_id=? AND "+quotedUser+".id=?", orgId, serviceAccountId). + sess := dbSession.Limit(maxRetrievedTokens, 0).Where("api_key.service_account_id IS NOT NULL") + + if query.OrgID != nil { + sess = sess.Where(quotedUser+".org_id=?", *query.OrgID) + sess = sess.Where("api_key.org_id=?", *query.OrgID) + } + + if query.ServiceAccountID != nil { + sess = sess.Where("api_key.service_account_id=?", *query.ServiceAccountID) + } + + sess = sess.Join("inner", quotedUser, quotedUser+".id = api_key.service_account_id"). Asc("api_key.name") - return sess.Find(&result) + return errors.Wrapf(sess.Find(&result), "list token error") }) return result, err } @@ -33,37 +43,27 @@ func (s *ServiceAccountsStoreImpl) AddServiceAccountToken(ctx context.Context, s return err } - key := apikey.APIKey{OrgId: cmd.OrgId, Name: cmd.Name} - exists, _ := sess.Get(&key) - if exists { - return ErrDuplicateToken - } - - updated := time.Now() - var expires *int64 = nil - if cmd.SecondsToLive > 0 { - v := updated.Add(time.Second * time.Duration(cmd.SecondsToLive)).Unix() - expires = &v - } else if cmd.SecondsToLive < 0 { - return ErrInvalidTokenExpiration - } - - token := apikey.APIKey{ - OrgId: cmd.OrgId, + addKeyCmd := &apikey.AddCommand{ Name: cmd.Name, Role: org.RoleViewer, + OrgId: cmd.OrgId, Key: cmd.Key, - Created: updated, - Updated: updated, - Expires: expires, - LastUsedAt: nil, - ServiceAccountId: &serviceAccountId, + SecondsToLive: cmd.SecondsToLive, + ServiceAccountID: &serviceAccountId, } - if _, err := sess.Insert(&token); err != nil { + if err := s.apiKeyService.AddAPIKey(ctx, addKeyCmd); err != nil { + switch { + case errors.Is(err, apikey.ErrDuplicate): + return ErrDuplicateToken + case errors.Is(err, apikey.ErrInvalidExpiration): + return ErrInvalidTokenExpiration + } + return err } - cmd.Result = &token + + cmd.Result = addKeyCmd.Result return nil }) } @@ -85,6 +85,23 @@ func (s *ServiceAccountsStoreImpl) DeleteServiceAccountToken(ctx context.Context }) } +func (s *ServiceAccountsStoreImpl) RevokeServiceAccountToken(ctx context.Context, orgId, serviceAccountId, tokenId int64) error { + rawSQL := "UPDATE api_key SET is_revoked = ? WHERE id=? and org_id=? and service_account_id=?" + + return s.sqlStore.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { + result, err := sess.Exec(rawSQL, s.sqlStore.Dialect.BooleanStr(true), tokenId, orgId, serviceAccountId) + if err != nil { + return err + } + affected, err := result.RowsAffected() + if affected == 0 { + return ErrServiceAccountTokenNotFound + } + + return err + }) +} + // assignApiKeyToServiceAccount sets the API key service account ID func (s *ServiceAccountsStoreImpl) assignApiKeyToServiceAccount(sess *sqlstore.DBSession, apiKeyId int64, serviceAccountId int64) error { key := apikey.APIKey{Id: apiKeyId} diff --git a/pkg/services/serviceaccounts/database/token_store_test.go b/pkg/services/serviceaccounts/database/token_store_test.go index 47eb4ab1be8..d06011139fb 100644 --- a/pkg/services/serviceaccounts/database/token_store_test.go +++ b/pkg/services/serviceaccounts/database/token_store_test.go @@ -48,7 +48,10 @@ func TestStore_AddServiceAccountToken(t *testing.T) { require.Equal(t, t.Name(), newKey.Name) // Verify against DB - keys, errT := store.ListTokens(context.Background(), user.OrgID, user.ID) + keys, errT := store.ListTokens(context.Background(), &serviceaccounts.GetSATokensQuery{ + OrgID: &user.OrgID, + ServiceAccountID: &user.ID, + }) require.NoError(t, errT) @@ -57,6 +60,8 @@ func TestStore_AddServiceAccountToken(t *testing.T) { if k.Name == keyName { found = true require.Equal(t, key.HashedKey, newKey.Key) + require.False(t, *k.IsRevoked) + if tc.secondsToLive == 0 { require.Nil(t, k.Expires) } else { @@ -91,6 +96,48 @@ func TestStore_AddServiceAccountToken_WrongServiceAccount(t *testing.T) { require.Error(t, err, "It should not be possible to add token to non-existing service account") } +func TestStore_RevokeServiceAccountToken(t *testing.T) { + userToCreate := tests.TestUser{Login: "servicetestwithTeam@admin", IsServiceAccount: true} + db, store := setupTestDatabase(t) + sa := tests.SetupUserServiceAccount(t, db, userToCreate) + + keyName := t.Name() + key, err := apikeygen.New(sa.OrgID, keyName) + require.NoError(t, err) + + cmd := serviceaccounts.AddServiceAccountTokenCommand{ + Name: keyName, + OrgId: sa.OrgID, + Key: key.HashedKey, + SecondsToLive: 0, + Result: &apikey.APIKey{}, + } + + err = store.AddServiceAccountToken(context.Background(), sa.ID, &cmd) + require.NoError(t, err) + newKey := cmd.Result + + // Revoke SAT + err = store.RevokeServiceAccountToken(context.Background(), sa.OrgID, sa.ID, newKey.Id) + require.NoError(t, err) + + // Verify against DB + keys, errT := store.ListTokens(context.Background(), &serviceaccounts.GetSATokensQuery{ + OrgID: &sa.OrgID, + ServiceAccountID: &sa.ID, + }) + require.NoError(t, errT) + + for _, k := range keys { + if k.Name == keyName { + require.True(t, *k.IsRevoked) + return + } + } + + require.Fail(t, "Key not found") +} + func TestStore_DeleteServiceAccountToken(t *testing.T) { userToCreate := tests.TestUser{Login: "servicetestwithTeam@admin", IsServiceAccount: true} db, store := setupTestDatabase(t) @@ -124,7 +171,10 @@ func TestStore_DeleteServiceAccountToken(t *testing.T) { require.NoError(t, err) // Verify against DB - keys, errT := store.ListTokens(context.Background(), sa.OrgID, sa.ID) + keys, errT := store.ListTokens(context.Background(), &serviceaccounts.GetSATokensQuery{ + OrgID: &sa.OrgID, + ServiceAccountID: &sa.ID, + }) require.NoError(t, errT) for _, k := range keys { diff --git a/pkg/services/serviceaccounts/manager/service.go b/pkg/services/serviceaccounts/manager/service.go index fd546c4fd00..03808e1713c 100644 --- a/pkg/services/serviceaccounts/manager/service.go +++ b/pkg/services/serviceaccounts/manager/service.go @@ -2,6 +2,8 @@ package manager import ( "context" + "fmt" + "time" "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/infra/log" @@ -13,9 +15,14 @@ import ( "github.com/grafana/grafana/pkg/setting" ) +const ( + metricsCollectionInterval = time.Minute * 30 +) + type ServiceAccountsService struct { - store serviceaccounts.Store - log log.Logger + store serviceaccounts.Store + log log.Logger + backgroundLog log.Logger } func ProvideServiceAccountsService( @@ -28,8 +35,9 @@ func ProvideServiceAccountsService( ) (*ServiceAccountsService, error) { database.InitMetrics() s := &ServiceAccountsService{ - store: serviceAccountsStore, - log: log.New("serviceaccounts"), + store: serviceAccountsStore, + log: log.New("serviceaccounts"), + backgroundLog: log.New("serviceaccounts.background"), } if err := RegisterRoles(ac); err != nil { @@ -45,8 +53,33 @@ func ProvideServiceAccountsService( } func (sa *ServiceAccountsService) Run(ctx context.Context) error { - sa.log.Debug("Started Service Account Metrics collection service") - return sa.store.RunMetricsCollection(ctx) + sa.backgroundLog.Debug("service initialized") + + if _, err := sa.store.GetUsageMetrics(ctx); err != nil { + sa.log.Warn("Failed to get usage metrics", "error", err.Error()) + } + + updateStatsTicker := time.NewTicker(metricsCollectionInterval) + defer updateStatsTicker.Stop() + + for { + select { + case <-ctx.Done(): + if err := ctx.Err(); err != nil { + return fmt.Errorf("context error in service account background service: %w", ctx.Err()) + } + + sa.backgroundLog.Debug("stopped service account background service") + + return nil + case <-updateStatsTicker.C: + sa.backgroundLog.Debug("updating usage metrics") + + if _, err := sa.store.GetUsageMetrics(ctx); err != nil { + sa.backgroundLog.Warn("Failed to get usage metrics", "error", err.Error()) + } + } + } } func (sa *ServiceAccountsService) CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) { diff --git a/pkg/services/serviceaccounts/models.go b/pkg/services/serviceaccounts/models.go index a4a0feaace4..59048ba8d00 100644 --- a/pkg/services/serviceaccounts/models.go +++ b/pkg/services/serviceaccounts/models.go @@ -64,6 +64,11 @@ type ServiceAccountDTO struct { AccessControl map[string]bool `json:"accessControl,omitempty"` } +type GetSATokensQuery struct { + OrgID *int64 // optional filtering by org ID + ServiceAccountID *int64 // optional filtering by service account ID +} + type AddServiceAccountTokenCommand struct { Name string `json:"name" binding:"Required"` OrgId int64 `json:"-"` diff --git a/pkg/services/serviceaccounts/serviceaccounts.go b/pkg/services/serviceaccounts/serviceaccounts.go index 83dc48fbb11..4e825095288 100644 --- a/pkg/services/serviceaccounts/serviceaccounts.go +++ b/pkg/services/serviceaccounts/serviceaccounts.go @@ -28,9 +28,9 @@ type Store interface { MigrateApiKeysToServiceAccounts(ctx context.Context, orgID int64) error MigrateApiKey(ctx context.Context, orgID int64, keyId int64) error RevertApiKey(ctx context.Context, saId int64, keyId int64) error - ListTokens(ctx context.Context, orgID int64, serviceAccount int64) ([]*apikey.APIKey, error) + ListTokens(ctx context.Context, query *GetSATokensQuery) ([]apikey.APIKey, error) DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error + RevokeServiceAccountToken(ctx context.Context, orgId, serviceAccountId, tokenId int64) error AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *AddServiceAccountTokenCommand) error GetUsageMetrics(ctx context.Context) (map[string]interface{}, error) - RunMetricsCollection(ctx context.Context) error } diff --git a/pkg/services/serviceaccounts/tests/common.go b/pkg/services/serviceaccounts/tests/common.go index 9cda7250956..174e34605ba 100644 --- a/pkg/services/serviceaccounts/tests/common.go +++ b/pkg/services/serviceaccounts/tests/common.go @@ -183,8 +183,8 @@ func (s *ServiceAccountsStoreMock) RevertApiKey(ctx context.Context, saId int64, return nil } -func (s *ServiceAccountsStoreMock) ListTokens(ctx context.Context, orgID int64, serviceAccount int64) ([]*apikey.APIKey, error) { - s.Calls.ListTokens = append(s.Calls.ListTokens, []interface{}{ctx, orgID, serviceAccount}) +func (s *ServiceAccountsStoreMock) ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) { + s.Calls.ListTokens = append(s.Calls.ListTokens, []interface{}{ctx, query.OrgID, query.ServiceAccountID}) return nil, nil } diff --git a/pkg/services/sqlstore/migrations/apikey_mig.go b/pkg/services/sqlstore/migrations/apikey_mig.go index a681256de24..f175ea06fa1 100644 --- a/pkg/services/sqlstore/migrations/apikey_mig.go +++ b/pkg/services/sqlstore/migrations/apikey_mig.go @@ -95,4 +95,9 @@ func addApiKeyMigrations(mg *Migrator) { mg.AddMigration("Add last_used_at to api_key table", NewAddColumnMigration(apiKeyV2, &Column{ Name: "last_used_at", Type: DB_DateTime, Nullable: true, })) + + // is_revoked indicates whether key is revoked or not. Revoked keys should be kept in the table, but invalid. + mg.AddMigration("Add is_revoked column to api_key table", NewAddColumnMigration(apiKeyV2, &Column{ + Name: "is_revoked", Type: DB_Bool, Nullable: true, Default: "0", + })) } diff --git a/public/app/features/serviceaccounts/components/ServiceAccountTokensTable.tsx b/public/app/features/serviceaccounts/components/ServiceAccountTokensTable.tsx index e219a843e04..c4136a90281 100644 --- a/public/app/features/serviceaccounts/components/ServiceAccountTokensTable.tsx +++ b/public/app/features/serviceaccounts/components/ServiceAccountTokensTable.tsx @@ -25,18 +25,22 @@ export const ServiceAccountTokensTable = ({ tokens, timeZone, tokenActionsDisabl Created Last used at + {tokens.map((key) => { return ( - + {key.name} {formatDate(timeZone, key.created)} {formatLastUsedAtDate(timeZone, key.lastUsedAt)} + + {key.isRevoked && Revoked} +