From 76bc288d674ac98bf1f09a9c2a953861ad25881d Mon Sep 17 00:00:00 2001 From: Eric Leijonmarck Date: Fri, 3 Mar 2023 09:03:06 +0000 Subject: [PATCH] Service accounts: Remove revertapikeys endpoint (#64020) * remove revertapikeys endpoints * remove unused method --- pkg/services/serviceaccounts/api/api.go | 20 ---- .../serviceaccounts/database/store.go | 54 ---------- .../serviceaccounts/database/store_test.go | 99 ------------------- .../serviceaccounts/database/token_store.go | 22 ----- .../serviceaccounts/manager/service.go | 9 -- pkg/services/serviceaccounts/manager/store.go | 1 - 6 files changed, 205 deletions(-) diff --git a/pkg/services/serviceaccounts/api/api.go b/pkg/services/serviceaccounts/api/api.go index 83c9f5dbdef..dba8f532d09 100644 --- a/pkg/services/serviceaccounts/api/api.go +++ b/pkg/services/serviceaccounts/api/api.go @@ -43,7 +43,6 @@ type service interface { HideApiKeysTab(ctx context.Context, orgID int64) error 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 // Service account tokens AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) (*apikey.APIKey, error) DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error @@ -93,8 +92,6 @@ func (api *ServiceAccountsAPI) RegisterAPIEndpoints() { accesscontrol.EvalPermission(serviceaccounts.ActionCreate)), routing.Wrap(api.MigrateApiKeysToServiceAccounts)) serviceAccountsRoute.Post("/migrate/:keyId", auth(middleware.ReqOrgAdmin, accesscontrol.EvalPermission(serviceaccounts.ActionCreate)), routing.Wrap(api.ConvertToServiceAccount)) - serviceAccountsRoute.Post("/:serviceAccountId/revert/:keyId", auth(middleware.ReqOrgAdmin, - accesscontrol.EvalPermission(serviceaccounts.ActionDelete, serviceaccounts.ScopeID)), routing.Wrap(api.RevertApiKey)) }) } @@ -391,23 +388,6 @@ func (api *ServiceAccountsAPI) ConvertToServiceAccount(ctx *contextmodel.ReqCont return response.Success("Service accounts migrated") } -// POST /api/serviceaccounts/revert/:keyId -func (api *ServiceAccountsAPI) RevertApiKey(ctx *contextmodel.ReqContext) response.Response { - keyId, err := strconv.ParseInt(web.Params(ctx.Req)[":keyId"], 10, 64) - if err != nil { - return response.Error(http.StatusBadRequest, "key ID is invalid", err) - } - serviceAccountId, err := strconv.ParseInt(web.Params(ctx.Req)[":serviceAccountId"], 10, 64) - if err != nil { - return response.Error(http.StatusBadRequest, "service account ID is invalid", err) - } - - if err := api.service.RevertApiKey(ctx.Req.Context(), serviceAccountId, keyId); err != nil { - return response.Error(http.StatusInternalServerError, "error reverting to API key", err) - } - return response.Success("reverted service account to API key") -} - func (api *ServiceAccountsAPI) getAccessControlMetadata(c *contextmodel.ReqContext, saIDs map[string]bool) map[string]accesscontrol.Metadata { if api.accesscontrol.IsDisabled() || !c.QueryBool("accesscontrol") { return map[string]accesscontrol.Metadata{} diff --git a/pkg/services/serviceaccounts/database/store.go b/pkg/services/serviceaccounts/database/store.go index 599fda381ac..4f5d74203bd 100644 --- a/pkg/services/serviceaccounts/database/store.go +++ b/pkg/services/serviceaccounts/database/store.go @@ -436,60 +436,6 @@ func (s *ServiceAccountsStoreImpl) CreateServiceAccountFromApikey(ctx context.Co }) } -// RevertApiKey converts service account token to old API key -func (s *ServiceAccountsStoreImpl) RevertApiKey(ctx context.Context, saId int64, keyId int64) error { - query := apikey.GetByIDQuery{ApiKeyID: keyId} - if err := s.apiKeyService.GetApiKeyById(ctx, &query); err != nil { - return err - } - key := query.Result - - if key.ServiceAccountId == nil { - return fmt.Errorf("API key is not service account token") - } - - if *key.ServiceAccountId != saId { - return serviceaccounts.ErrServiceAccountAndTokenMismatch - } - - tokens, err := s.ListTokens(ctx, &serviceaccounts.GetSATokensQuery{ - OrgID: &key.OrgID, - ServiceAccountID: key.ServiceAccountId, - }) - if err != nil { - return fmt.Errorf("cannot revert token: %w", err) - } - if len(tokens) > 1 { - return fmt.Errorf("cannot revert token: service account contains more than one token") - } - - err = s.sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error { - user := user.User{} - has, err := sess.Where(`org_id = ? and id = ? and is_service_account = ?`, - key.OrgID, *key.ServiceAccountId, s.sqlStore.GetDialect().BooleanStr(true)).Get(&user) - if err != nil { - return err - } - if !has { - return serviceaccounts.ErrServiceAccountNotFound - } - // Detach API key from service account - if err := s.detachApiKeyFromServiceAccount(sess, key.ID); err != nil { - return err - } - // Delete service account - if err := s.deleteServiceAccount(sess, key.OrgID, *key.ServiceAccountId); err != nil { - return err - } - return nil - }) - - if err != nil { - return fmt.Errorf("cannot revert token to API key: %w", err) - } - return nil -} - func serviceAccountDeletions(dialect migrator.Dialect) []string { deletes := []string{ "DELETE FROM star WHERE user_id = ?", diff --git a/pkg/services/serviceaccounts/database/store_test.go b/pkg/services/serviceaccounts/database/store_test.go index 220f6d0cfd4..031bf7b1c19 100644 --- a/pkg/services/serviceaccounts/database/store_test.go +++ b/pkg/services/serviceaccounts/database/store_test.go @@ -2,7 +2,6 @@ package database import ( "context" - "math/rand" "testing" "github.com/stretchr/testify/assert" @@ -311,101 +310,3 @@ func TestStore_MigrateAllApiKeys(t *testing.T) { }) } } - -func TestStore_RevertApiKey(t *testing.T) { - cases := []struct { - desc string - key tests.TestApiKey - forceMismatchServiceAccount bool - expectedErr error - }{ - { - desc: "service account token should be reverted to api key", - key: tests.TestApiKey{Name: "Test1", Role: org.RoleEditor, OrgId: 1}, - expectedErr: nil, - }, - { - desc: "should fail reverting to api key when the token is assigned to a different service account", - key: tests.TestApiKey{Name: "Test1", Role: org.RoleEditor, OrgId: 1}, - forceMismatchServiceAccount: true, - expectedErr: serviceaccounts.ErrServiceAccountAndTokenMismatch, - }, - } - - for _, c := range cases { - t.Run(c.desc, func(t *testing.T) { - db, store := setupTestDatabase(t) - store.cfg.AutoAssignOrg = true - store.cfg.AutoAssignOrgId = 1 - store.cfg.AutoAssignOrgRole = "Viewer" - _, err := store.orgService.CreateWithMember(context.Background(), &org.CreateOrgCommand{Name: "main"}) - require.NoError(t, err) - - key := tests.SetupApiKey(t, db, c.key) - err = store.CreateServiceAccountFromApikey(context.Background(), key) - require.NoError(t, err) - - var saId int64 - if c.forceMismatchServiceAccount { - saId = rand.Int63() - } else { - q := serviceaccounts.SearchOrgServiceAccountsQuery{ - OrgID: key.OrgID, - Query: "", - Page: 1, - Limit: 50, - SignedInUser: &user.SignedInUser{ - UserID: 1, - OrgID: 1, - Permissions: map[int64]map[string][]string{ - key.OrgID: { - "serviceaccounts:read": {"serviceaccounts:id:*"}, - }, - }, - }, - } - serviceAccounts, err := store.SearchOrgServiceAccounts(context.Background(), &q) - require.NoError(t, err) - saId = serviceAccounts.ServiceAccounts[0].Id - } - - err = store.RevertApiKey(context.Background(), saId, key.ID) - - if c.expectedErr != nil { - require.ErrorIs(t, err, c.expectedErr) - } else { - require.NoError(t, err) - q := serviceaccounts.SearchOrgServiceAccountsQuery{ - OrgID: key.OrgID, - Query: "", - Page: 1, - Limit: 50, - SignedInUser: &user.SignedInUser{ - UserID: 1, - OrgID: 1, - Permissions: map[int64]map[string][]string{ - key.OrgID: { - "serviceaccounts:read": {"serviceaccounts:id:*"}, - }, - }, - }, - } - serviceAccounts, err := store.SearchOrgServiceAccounts(context.Background(), &q) - require.NoError(t, err) - // Service account should be deleted - require.Equal(t, int64(0), serviceAccounts.TotalCount) - - apiKeys, err := store.apiKeyService.GetAllAPIKeys(context.Background(), 1) - require.NoError(t, err) - require.Len(t, apiKeys, 1) - apiKey := apiKeys[0] - require.Equal(t, c.key.Name, apiKey.Name) - require.Equal(t, c.key.OrgId, apiKey.OrgID) - require.Equal(t, c.key.Role, apiKey.Role) - require.Equal(t, key.Key, apiKey.Key) - // Api key should not be linked to service account - require.Nil(t, apiKey.ServiceAccountId) - } - }) - } -} diff --git a/pkg/services/serviceaccounts/database/token_store.go b/pkg/services/serviceaccounts/database/token_store.go index abfc08ed24b..850d6a90b3e 100644 --- a/pkg/services/serviceaccounts/database/token_store.go +++ b/pkg/services/serviceaccounts/database/token_store.go @@ -129,25 +129,3 @@ func (s *ServiceAccountsStoreImpl) assignApiKeyToServiceAccount(sess *db.Session return nil } - -// detachApiKeyFromServiceAccount converts service account token to old API key -func (s *ServiceAccountsStoreImpl) detachApiKeyFromServiceAccount(sess *db.Session, apiKeyId int64) error { - key := apikey.APIKey{ID: apiKeyId} - exists, err := sess.Get(&key) - if err != nil { - s.log.Warn("Cannot get API key", "err", err) - return err - } - if !exists { - s.log.Warn("API key not found", "err", err) - return apikey.ErrNotFound - } - key.ServiceAccountId = nil - - if _, err := sess.ID(key.ID).AllCols().Update(&key); err != nil { - s.log.Error("Could not update api key", "err", err) - return err - } - - return nil -} diff --git a/pkg/services/serviceaccounts/manager/service.go b/pkg/services/serviceaccounts/manager/service.go index 031b4c7bcde..cd152e7cc81 100644 --- a/pkg/services/serviceaccounts/manager/service.go +++ b/pkg/services/serviceaccounts/manager/service.go @@ -248,15 +248,6 @@ func (sa *ServiceAccountsService) MigrateApiKeysToServiceAccounts(ctx context.Co } return sa.store.MigrateApiKeysToServiceAccounts(ctx, orgID) } -func (sa *ServiceAccountsService) RevertApiKey(ctx context.Context, orgID, keyID int64) error { - if err := validOrgID(orgID); err != nil { - return err - } - if err := validAPIKeyID(keyID); err != nil { - return err - } - return sa.store.RevertApiKey(ctx, orgID, keyID) -} func validOrgID(orgID int64) error { if orgID == 0 { diff --git a/pkg/services/serviceaccounts/manager/store.go b/pkg/services/serviceaccounts/manager/store.go index 0168d334366..46893811e11 100644 --- a/pkg/services/serviceaccounts/manager/store.go +++ b/pkg/services/serviceaccounts/manager/store.go @@ -29,7 +29,6 @@ type store interface { HideApiKeysTab(ctx context.Context, orgID int64) error 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, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) RevokeServiceAccountToken(ctx context.Context, orgId, serviceAccountId, tokenId int64) error AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) (*apikey.APIKey, error)