ServiceAccounts: API keys migration (#50002)

* ServiceAccounts: able to get upgrade status

* Banner with API keys migration info

* Show API keys migration info on Service accounts page

* Migrate individual API keys

* Use transaction for key migration

* Migrate all api keys to service accounts

* Hide api keys after migration

* Migrate API keys separately for each org

* Revert API key

* Revert key API method

* Rename migration actions and reducers

* Fix linter errors

* Tests for migrating single API key

* Tests for migrating all api keys

* More tests

* Fix reverting tokens

* API: rename convert to migrate

* Add api route descriptions to methods

* rearrange methods in api.go

* Refactor: rename and move some methods

* Prevent assigning tokens to non-existing service accounts

* Refactor: ID TO Id

* Refactor: fix error message

* Delete service account if migration failed

* Fix linter errors

pkg/services/serviceaccounts/database/database.go

@@ -8,31 +8,35 @@ import (
 	"strings"
 	"time"
 
+	"github.com/grafana/grafana/pkg/infra/kvstore"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
-	"xorm.io/xorm"
 )
 
 type ServiceAccountsStoreImpl struct {
 	sqlStore *sqlstore.SQLStore
+	kvStore  kvstore.KVStore
 	log      log.Logger
 }
 
-func NewServiceAccountsStore(store *sqlstore.SQLStore) *ServiceAccountsStoreImpl {
+func NewServiceAccountsStore(store *sqlstore.SQLStore, kvStore kvstore.KVStore) *ServiceAccountsStoreImpl {
 	return &ServiceAccountsStoreImpl{
 		sqlStore: store,
+		kvStore:  kvStore,
+		log:      log.New("serviceaccounts.store"),
 	}
 }
 
-func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, orgID int64, name string) (saDTO *serviceaccounts.ServiceAccountDTO, err error) {
+// CreateServiceAccount creates service account
+func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, orgId int64, name string) (saDTO *serviceaccounts.ServiceAccountDTO, err error) {
 	generatedLogin := "sa-" + strings.ToLower(name)
 	generatedLogin = strings.ReplaceAll(generatedLogin, " ", "-")
 	cmd := models.CreateUserCommand{
 		Login:            generatedLogin,
-		OrgId:            orgID,
+		OrgId:            orgId,
 		Name:             name,
 		IsServiceAccount: true,
 	}
@@ -53,6 +57,64 @@ func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, org
 		Tokens: 0,
 	}, nil
 }
+
+// UpdateServiceAccount updates service account
+func (s *ServiceAccountsStoreImpl) UpdateServiceAccount(ctx context.Context,
+	orgId, serviceAccountId int64,
+	saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) {
+	updatedUser := &serviceaccounts.ServiceAccountProfileDTO{}
+
+	err := s.sqlStore.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
+		var err error
+		updatedUser, err = s.RetrieveServiceAccount(ctx, orgId, serviceAccountId)
+		if err != nil {
+			return err
+		}
+
+		if saForm.Name == nil && saForm.Role == nil && saForm.IsDisabled == nil {
+			return nil
+		}
+
+		updateTime := time.Now()
+		if saForm.Role != nil {
+			var orgUser models.OrgUser
+			orgUser.Role = *saForm.Role
+			orgUser.Updated = updateTime
+
+			if _, err := sess.Where("org_id = ? AND user_id = ?", orgId, serviceAccountId).Update(&orgUser); err != nil {
+				return err
+			}
+
+			updatedUser.Role = string(*saForm.Role)
+		}
+
+		if saForm.Name != nil || saForm.IsDisabled != nil {
+			user := models.User{
+				Updated: updateTime,
+			}
+
+			if saForm.IsDisabled != nil {
+				user.IsDisabled = *saForm.IsDisabled
+				updatedUser.IsDisabled = *saForm.IsDisabled
+				sess.UseBool("is_disabled")
+			}
+
+			if saForm.Name != nil {
+				user.Name = *saForm.Name
+				updatedUser.Name = *saForm.Name
+			}
+
+			if _, err := sess.ID(serviceAccountId).Update(&user); err != nil {
+				return err
+			}
+		}
+
+		return nil
+	})
+
+	return updatedUser, err
+}
+
 func ServiceAccountDeletions() []string {
 	deletes := []string{
 		"DELETE FROM api_key WHERE service_account_id = ?",
@@ -61,110 +123,34 @@ func ServiceAccountDeletions() []string {
 	return deletes
 }
 
-func (s *ServiceAccountsStoreImpl) DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error {
+// DeleteServiceAccount deletes service account and all associated tokens
+func (s *ServiceAccountsStoreImpl) DeleteServiceAccount(ctx context.Context, orgId, serviceAccountId int64) error {
 	return s.sqlStore.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
-		user := models.User{}
-		has, err := sess.Where(`org_id = ? and id = ? and is_service_account = ?`,
-			orgID, serviceAccountID, s.sqlStore.Dialect.BooleanStr(true)).Get(&user)
+		return s.deleteServiceAccount(sess, orgId, serviceAccountId)
+	})
+}
+
+func (s *ServiceAccountsStoreImpl) deleteServiceAccount(sess *sqlstore.DBSession, orgId, serviceAccountId int64) error {
+	user := models.User{}
+	has, err := sess.Where(`org_id = ? and id = ? and is_service_account = ?`,
+		orgId, serviceAccountId, s.sqlStore.Dialect.BooleanStr(true)).Get(&user)
+	if err != nil {
+		return err
+	}
+	if !has {
+		return serviceaccounts.ErrServiceAccountNotFound
+	}
+	for _, sql := range ServiceAccountDeletions() {
+		_, err := sess.Exec(sql, user.Id)
 		if err != nil {
 			return err
 		}
-		if !has {
-			return serviceaccounts.ErrServiceAccountNotFound
-		}
-		for _, sql := range ServiceAccountDeletions() {
-			_, err := sess.Exec(sql, user.Id)
-			if err != nil {
-				return err
-			}
-		}
-		return nil
-	})
-}
-
-func (s *ServiceAccountsStoreImpl) UpgradeServiceAccounts(ctx context.Context) error {
-	basicKeys := s.sqlStore.GetAllOrgsAPIKeys(ctx)
-	if len(basicKeys) > 0 {
-		s.log.Info("Launching background thread to upgrade API keys to service accounts", "numberKeys", len(basicKeys))
-		go func() {
-			for _, key := range basicKeys {
-				err := s.CreateServiceAccountFromApikey(ctx, key)
-				if err != nil {
-					s.log.Error("migating to service accounts failed with error", err)
-				}
-			}
-		}()
 	}
 	return nil
 }
 
-func (s *ServiceAccountsStoreImpl) ConvertToServiceAccounts(ctx context.Context, keys []int64) error {
-	basicKeys := s.sqlStore.GetAllOrgsAPIKeys(ctx)
-	if len(basicKeys) == 0 {
-		return nil
-	}
-	if len(basicKeys) != len(keys) {
-		return fmt.Errorf("one of the keys already has a serviceaccount")
-	}
-	for _, key := range basicKeys {
-		if !contains(keys, key.Id) {
-			s.log.Error("convert service accounts stopped for keyId %d as it is not part of the query to convert or already has a service account", key.Id)
-			continue
-		}
-		err := s.CreateServiceAccountFromApikey(ctx, key)
-		if err != nil {
-			s.log.Error("converting to service accounts failed with error", err)
-		}
-	}
-	return nil
-}
-
-func (s *ServiceAccountsStoreImpl) CreateServiceAccountFromApikey(ctx context.Context, key *models.ApiKey) error {
-	prefix := "sa-autogen-"
-	cmd := models.CreateUserCommand{
-		Login:            fmt.Sprintf("%v-%v-%v", prefix, key.OrgId, key.Name),
-		Name:             prefix + key.Name,
-		OrgId:            key.OrgId,
-		DefaultOrgRole:   string(key.Role),
-		IsServiceAccount: true,
-	}
-
-	newSA, errCreateSA := s.sqlStore.CreateUser(ctx, cmd)
-	if errCreateSA != nil {
-		return fmt.Errorf("failed to create service account: %w", errCreateSA)
-	}
-
-	if errUpdateKey := s.assignApiKeyToServiceAccount(ctx, key.Id, newSA.Id); errUpdateKey != nil {
-		return fmt.Errorf(
-			"failed to attach new service account to API key for keyId: %d and newServiceAccountId: %d with error: %w",
-			key.Id, newSA.Id, errUpdateKey,
-		)
-	}
-
-	s.log.Debug("Updated basic api key", "keyId", key.Id, "newServiceAccountId", newSA.Id)
-
-	return nil
-}
-
-//nolint:gosimple
-func (s *ServiceAccountsStoreImpl) ListTokens(ctx context.Context, orgID int64, serviceAccountID int64) ([]*models.ApiKey, error) {
-	result := make([]*models.ApiKey, 0)
-	err := s.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error {
-		var sess *xorm.Session
-
-		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).
-			Asc("api_key.name")
-
-		return sess.Find(&result)
-	})
-	return result, err
-}
-
-// RetrieveServiceAccountByID returns a service account by its ID
-func (s *ServiceAccountsStoreImpl) RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error) {
+// RetrieveServiceAccount returns a service account by its ID
+func (s *ServiceAccountsStoreImpl) RetrieveServiceAccount(ctx context.Context, orgId, serviceAccountId int64) (*serviceaccounts.ServiceAccountProfileDTO, error) {
 	serviceAccount := &serviceaccounts.ServiceAccountProfileDTO{}
 
 	err := s.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error {
@@ -176,10 +162,10 @@ func (s *ServiceAccountsStoreImpl) RetrieveServiceAccount(ctx context.Context, o
 		whereParams := make([]interface{}, 0)
 
 		whereConditions = append(whereConditions, "org_user.org_id = ?")
-		whereParams = append(whereParams, orgID)
+		whereParams = append(whereParams, orgId)
 
 		whereConditions = append(whereConditions, "org_user.user_id = ?")
-		whereParams = append(whereParams, serviceAccountID)
+		whereParams = append(whereParams, serviceAccountId)
 
 		whereConditions = append(whereConditions,
 			fmt.Sprintf("%s.is_service_account = %s",
@@ -212,7 +198,7 @@ func (s *ServiceAccountsStoreImpl) RetrieveServiceAccount(ctx context.Context, o
 	return serviceAccount, err
 }
 
-func (s *ServiceAccountsStoreImpl) RetrieveServiceAccountIdByName(ctx context.Context, orgID int64, name string) (int64, error) {
+func (s *ServiceAccountsStoreImpl) RetrieveServiceAccountIdByName(ctx context.Context, orgId int64, name string) (int64, error) {
 	serviceAccount := &struct {
 		Id int64
 	}{}
@@ -229,7 +215,7 @@ func (s *ServiceAccountsStoreImpl) RetrieveServiceAccountIdByName(ctx context.Co
 				s.sqlStore.Dialect.Quote("user"),
 				s.sqlStore.Dialect.BooleanStr(true)),
 		}
-		whereParams := []interface{}{name, orgID}
+		whereParams := []interface{}{name, orgId}
 
 		sess.Where(strings.Join(whereConditions, " AND "), whereParams...)
 
@@ -253,64 +239,8 @@ func (s *ServiceAccountsStoreImpl) RetrieveServiceAccountIdByName(ctx context.Co
 	return serviceAccount.Id, nil
 }
 
-func (s *ServiceAccountsStoreImpl) UpdateServiceAccount(ctx context.Context,
-	orgID, serviceAccountID int64,
-	saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) {
-	updatedUser := &serviceaccounts.ServiceAccountProfileDTO{}
-
-	err := s.sqlStore.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
-		var err error
-		updatedUser, err = s.RetrieveServiceAccount(ctx, orgID, serviceAccountID)
-		if err != nil {
-			return err
-		}
-
-		if saForm.Name == nil && saForm.Role == nil && saForm.IsDisabled == nil {
-			return nil
-		}
-
-		updateTime := time.Now()
-		if saForm.Role != nil {
-			var orgUser models.OrgUser
-			orgUser.Role = *saForm.Role
-			orgUser.Updated = updateTime
-
-			if _, err := sess.Where("org_id = ? AND user_id = ?", orgID, serviceAccountID).Update(&orgUser); err != nil {
-				return err
-			}
-
-			updatedUser.Role = string(*saForm.Role)
-		}
-
-		if saForm.Name != nil || saForm.IsDisabled != nil {
-			user := models.User{
-				Updated: updateTime,
-			}
-
-			if saForm.IsDisabled != nil {
-				user.IsDisabled = *saForm.IsDisabled
-				updatedUser.IsDisabled = *saForm.IsDisabled
-				sess.UseBool("is_disabled")
-			}
-
-			if saForm.Name != nil {
-				user.Name = *saForm.Name
-				updatedUser.Name = *saForm.Name
-			}
-
-			if _, err := sess.ID(serviceAccountID).Update(&user); err != nil {
-				return err
-			}
-		}
-
-		return nil
-	})
-
-	return updatedUser, err
-}
-
 func (s *ServiceAccountsStoreImpl) SearchOrgServiceAccounts(
-	ctx context.Context, orgID int64, query string, filter serviceaccounts.ServiceAccountFilter, page int, limit int,
+	ctx context.Context, orgId int64, query string, filter serviceaccounts.ServiceAccountFilter, page int, limit int,
 	signedInUser *models.SignedInUser,
 ) (*serviceaccounts.SearchServiceAccountsResult, error) {
 	searchResult := &serviceaccounts.SearchServiceAccountsResult{
@@ -328,7 +258,7 @@ func (s *ServiceAccountsStoreImpl) SearchOrgServiceAccounts(
 		whereParams := make([]interface{}, 0)
 
 		whereConditions = append(whereConditions, "org_user.org_id = ?")
-		whereParams = append(whereParams, orgID)
+		whereParams = append(whereParams, orgId)
 
 		whereConditions = append(whereConditions,
 			fmt.Sprintf("%s.is_service_account = %s",
@@ -415,11 +345,134 @@ func (s *ServiceAccountsStoreImpl) SearchOrgServiceAccounts(
 	return searchResult, nil
 }
 
-func contains(s []int64, e int64) bool {
-	for _, a := range s {
-		if a == e {
-			return true
+func (s *ServiceAccountsStoreImpl) GetAPIKeysMigrationStatus(ctx context.Context, orgId int64) (status *serviceaccounts.APIKeysMigrationStatus, err error) {
+	migrationStatus, exists, err := s.kvStore.Get(ctx, orgId, "serviceaccounts", "migrationStatus")
+	if err != nil {
+		return nil, err
+	}
+	if exists && migrationStatus == "1" {
+		return &serviceaccounts.APIKeysMigrationStatus{
+			Migrated: true,
+		}, nil
+	} else {
+		return &serviceaccounts.APIKeysMigrationStatus{
+			Migrated: false,
+		}, nil
+	}
+}
+
+func (s *ServiceAccountsStoreImpl) HideApiKeysTab(ctx context.Context, orgId int64) error {
+	if err := s.kvStore.Set(ctx, orgId, "serviceaccounts", "hideApiKeys", "1"); err != nil {
+		s.log.Error("Failed to hide API keys tab", err)
+	}
+	return nil
+}
+
+func (s *ServiceAccountsStoreImpl) MigrateApiKeysToServiceAccounts(ctx context.Context, orgId int64) error {
+	basicKeys := s.sqlStore.GetAllAPIKeys(ctx, orgId)
+	if len(basicKeys) > 0 {
+		for _, key := range basicKeys {
+			err := s.CreateServiceAccountFromApikey(ctx, key)
+			if err != nil {
+				s.log.Error("migating to service accounts failed with error", err)
+				return err
+			}
+			s.log.Debug("API key converted to service account token", "keyId", key.Id)
 		}
 	}
-	return false
+	if err := s.kvStore.Set(ctx, orgId, "serviceaccounts", "migrationStatus", "1"); err != nil {
+		s.log.Error("Failed to write API keys migration status", err)
+	}
+	return nil
+}
+
+func (s *ServiceAccountsStoreImpl) MigrateApiKey(ctx context.Context, orgId int64, keyId int64) error {
+	basicKeys := s.sqlStore.GetAllAPIKeys(ctx, orgId)
+	if len(basicKeys) == 0 {
+		return fmt.Errorf("no API keys to convert found")
+	}
+	for _, key := range basicKeys {
+		if keyId == key.Id {
+			err := s.CreateServiceAccountFromApikey(ctx, key)
+			if err != nil {
+				s.log.Error("converting to service account failed with error", "keyId", keyId, "error", err)
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+func (s *ServiceAccountsStoreImpl) CreateServiceAccountFromApikey(ctx context.Context, key *models.ApiKey) error {
+	prefix := "sa-autogen"
+	cmd := models.CreateUserCommand{
+		Login:            fmt.Sprintf("%v-%v-%v", prefix, key.OrgId, key.Name),
+		Name:             fmt.Sprintf("%v-%v", prefix, key.Name),
+		OrgId:            key.OrgId,
+		DefaultOrgRole:   string(key.Role),
+		IsServiceAccount: true,
+	}
+
+	return s.sqlStore.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
+		newSA, errCreateSA := s.sqlStore.CreateUser(ctx, cmd)
+		if errCreateSA != nil {
+			return fmt.Errorf("failed to create service account: %w", errCreateSA)
+		}
+
+		if err := s.assignApiKeyToServiceAccount(sess, key.Id, newSA.Id); err != nil {
+			if err := s.sqlStore.DeleteUser(ctx, &models.DeleteUserCommand{UserId: newSA.Id}); err != nil {
+				s.log.Error("Error deleting service account", "error", err)
+			}
+			return fmt.Errorf("failed to migrate API key to service account token: %w", err)
+		}
+
+		return nil
+	})
+}
+
+// RevertApiKey converts service account token to old API key
+func (s *ServiceAccountsStoreImpl) RevertApiKey(ctx context.Context, keyId int64) error {
+	query := models.GetApiKeyByIdQuery{ApiKeyId: keyId}
+	if err := s.sqlStore.GetApiKeyById(ctx, &query); err != nil {
+		return err
+	}
+	key := query.Result
+
+	if key.ServiceAccountId == nil {
+		// TODO: better error message
+		return fmt.Errorf("API key is not linked to service account")
+	}
+
+	tokens, err := s.ListTokens(ctx, key.OrgId, *key.ServiceAccountId)
+	if err != nil {
+		return fmt.Errorf("cannot revert API key: %w", err)
+	}
+	if len(tokens) > 1 {
+		return fmt.Errorf("cannot revert API key: service account contains more than one token")
+	}
+
+	err = s.sqlStore.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
+		user := models.User{}
+		has, err := sess.Where(`org_id = ? and id = ? and is_service_account = ?`,
+			key.OrgId, *key.ServiceAccountId, s.sqlStore.Dialect.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 API key: %w", err)
+	}
+	return nil
 }

pkg/services/serviceaccounts/database/database_test.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"testing"
 
+	"github.com/grafana/grafana/pkg/infra/kvstore"
+	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
@@ -70,7 +72,8 @@ func TestStore_DeleteServiceAccount(t *testing.T) {
 func setupTestDatabase(t *testing.T) (*sqlstore.SQLStore, *ServiceAccountsStoreImpl) {
 	t.Helper()
 	db := sqlstore.InitTestDB(t)
-	return db, NewServiceAccountsStore(db)
+	kvStore := kvstore.ProvideService(db)
+	return db, NewServiceAccountsStore(db, kvStore)
 }
 
 func TestStore_RetrieveServiceAccount(t *testing.T) {
@@ -106,3 +109,174 @@ func TestStore_RetrieveServiceAccount(t *testing.T) {
 		})
 	}
 }
+
+func TestStore_MigrateApiKeys(t *testing.T) {
+	cases := []struct {
+		desc        string
+		key         tests.TestApiKey
+		expectedErr error
+	}{
+		{
+			desc:        "api key should be migrated to service account token",
+			key:         tests.TestApiKey{Name: "Test1", Role: models.ROLE_EDITOR, OrgId: 1},
+			expectedErr: nil,
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.desc, func(t *testing.T) {
+			db, store := setupTestDatabase(t)
+			store.sqlStore.Cfg.AutoAssignOrg = true
+			store.sqlStore.Cfg.AutoAssignOrgId = 1
+			store.sqlStore.Cfg.AutoAssignOrgRole = "Viewer"
+			err := store.sqlStore.CreateOrg(context.Background(), &models.CreateOrgCommand{Name: "main"})
+			require.NoError(t, err)
+			key := tests.SetupApiKey(t, db, c.key)
+			err = store.MigrateApiKey(context.Background(), key.OrgId, key.Id)
+			if c.expectedErr != nil {
+				require.ErrorIs(t, err, c.expectedErr)
+			} else {
+				require.NoError(t, err)
+
+				serviceAccounts, err := store.SearchOrgServiceAccounts(context.Background(), key.OrgId, "", "all", 1, 50, &models.SignedInUser{UserId: 1, OrgId: 1, Permissions: map[int64]map[string][]string{
+					key.OrgId: {
+						"serviceaccounts:read": {"serviceaccounts:id:*"},
+					},
+				}})
+				require.NoError(t, err)
+				require.Equal(t, int64(1), serviceAccounts.TotalCount)
+				saMigrated := serviceAccounts.ServiceAccounts[0]
+				require.Equal(t, string(key.Role), saMigrated.Role)
+
+				tokens, err := store.ListTokens(context.Background(), key.OrgId, saMigrated.Id)
+				require.NoError(t, err)
+				require.Len(t, tokens, 1)
+			}
+		})
+	}
+}
+
+func TestStore_MigrateAllApiKeys(t *testing.T) {
+	cases := []struct {
+		desc                   string
+		keys                   []tests.TestApiKey
+		orgId                  int64
+		expectedServiceAccouts int64
+		expectedErr            error
+	}{
+		{
+			desc: "api keys should be migrated to service account tokens within provided org",
+			keys: []tests.TestApiKey{
+				{Name: "test1", Role: models.ROLE_EDITOR, Key: "secret1", OrgId: 1},
+				{Name: "test2", Role: models.ROLE_EDITOR, Key: "secret2", OrgId: 1},
+				{Name: "test3", Role: models.ROLE_EDITOR, Key: "secret3", OrgId: 2},
+			},
+			orgId:                  1,
+			expectedServiceAccouts: 2,
+			expectedErr:            nil,
+		},
+		{
+			desc: "api keys from another orgs shouldn't be migrated",
+			keys: []tests.TestApiKey{
+				{Name: "test1", Role: models.ROLE_EDITOR, Key: "secret1", OrgId: 2},
+				{Name: "test2", Role: models.ROLE_EDITOR, Key: "secret2", OrgId: 2},
+			},
+			orgId:                  1,
+			expectedServiceAccouts: 0,
+			expectedErr:            nil,
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.desc, func(t *testing.T) {
+			db, store := setupTestDatabase(t)
+			store.sqlStore.Cfg.AutoAssignOrg = true
+			store.sqlStore.Cfg.AutoAssignOrgId = 1
+			store.sqlStore.Cfg.AutoAssignOrgRole = "Viewer"
+			err := store.sqlStore.CreateOrg(context.Background(), &models.CreateOrgCommand{Name: "main"})
+			require.NoError(t, err)
+
+			for _, key := range c.keys {
+				tests.SetupApiKey(t, db, key)
+			}
+
+			err = store.MigrateApiKeysToServiceAccounts(context.Background(), c.orgId)
+			if c.expectedErr != nil {
+				require.ErrorIs(t, err, c.expectedErr)
+			} else {
+				require.NoError(t, err)
+
+				serviceAccounts, err := store.SearchOrgServiceAccounts(context.Background(), c.orgId, "", "all", 1, 50, &models.SignedInUser{UserId: 101, OrgId: c.orgId, Permissions: map[int64]map[string][]string{
+					c.orgId: {
+						"serviceaccounts:read": {"serviceaccounts:id:*"},
+					},
+				}})
+				require.NoError(t, err)
+				require.Equal(t, c.expectedServiceAccouts, serviceAccounts.TotalCount)
+				if c.expectedServiceAccouts > 0 {
+					saMigrated := serviceAccounts.ServiceAccounts[0]
+					require.Equal(t, string(c.keys[0].Role), saMigrated.Role)
+
+					tokens, err := store.ListTokens(context.Background(), c.orgId, saMigrated.Id)
+					require.NoError(t, err)
+					require.Len(t, tokens, 1)
+				}
+			}
+		})
+	}
+}
+
+func TestStore_RevertApiKey(t *testing.T) {
+	cases := []struct {
+		desc        string
+		key         tests.TestApiKey
+		expectedErr error
+	}{
+		{
+			desc:        "service account token should be reverted to api key",
+			key:         tests.TestApiKey{Name: "Test1", Role: models.ROLE_EDITOR, OrgId: 1},
+			expectedErr: nil,
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.desc, func(t *testing.T) {
+			db, store := setupTestDatabase(t)
+			store.sqlStore.Cfg.AutoAssignOrg = true
+			store.sqlStore.Cfg.AutoAssignOrgId = 1
+			store.sqlStore.Cfg.AutoAssignOrgRole = "Viewer"
+			err := store.sqlStore.CreateOrg(context.Background(), &models.CreateOrgCommand{Name: "main"})
+			require.NoError(t, err)
+
+			key := tests.SetupApiKey(t, db, c.key)
+			err = store.MigrateApiKey(context.Background(), key.OrgId, key.Id)
+			require.NoError(t, err)
+			err = store.RevertApiKey(context.Background(), key.Id)
+
+			if c.expectedErr != nil {
+				require.ErrorIs(t, err, c.expectedErr)
+			} else {
+				require.NoError(t, err)
+
+				serviceAccounts, err := store.SearchOrgServiceAccounts(context.Background(), key.OrgId, "", "all", 1, 50, &models.SignedInUser{UserId: 1, OrgId: 1, Permissions: map[int64]map[string][]string{
+					key.OrgId: {
+						"serviceaccounts:read": {"serviceaccounts:id:*"},
+					},
+				}})
+				require.NoError(t, err)
+				// Service account should be deleted
+				require.Equal(t, int64(0), serviceAccounts.TotalCount)
+
+				apiKeys := store.sqlStore.GetAllAPIKeys(context.Background(), 1)
+				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)
+			}
+		})
+	}
+}

pkg/services/serviceaccounts/database/token_store.go

@@ -7,10 +7,31 @@ import (
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
+	"xorm.io/xorm"
 )
 
-func (s *ServiceAccountsStoreImpl) AddServiceAccountToken(ctx context.Context, saID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error {
+func (s *ServiceAccountsStoreImpl) ListTokens(ctx context.Context, orgId int64, serviceAccountId int64) ([]*models.ApiKey, error) {
+	result := make([]*models.ApiKey, 0)
+	err := s.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error {
+		var sess *xorm.Session
+
+		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).
+			Asc("api_key.name")
+
+		return sess.Find(&result)
+	})
+	return result, err
+}
+
+func (s *ServiceAccountsStoreImpl) AddServiceAccountToken(ctx context.Context, serviceAccountId int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error {
 	return s.sqlStore.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
+		if _, err := s.RetrieveServiceAccount(ctx, cmd.OrgId, serviceAccountId); err != nil {
+			return err
+		}
+
 		key := models.ApiKey{OrgId: cmd.OrgId, Name: cmd.Name}
 		exists, _ := sess.Get(&key)
 		if exists {
@@ -26,7 +47,7 @@ func (s *ServiceAccountsStoreImpl) AddServiceAccountToken(ctx context.Context, s
 			return &ErrInvalidExpirationSAToken{}
 		}
 
-		t := models.ApiKey{
+		token := models.ApiKey{
 			OrgId:            cmd.OrgId,
 			Name:             cmd.Name,
 			Role:             models.ROLE_VIEWER,
@@ -34,22 +55,22 @@ func (s *ServiceAccountsStoreImpl) AddServiceAccountToken(ctx context.Context, s
 			Created:          updated,
 			Updated:          updated,
 			Expires:          expires,
-			ServiceAccountId: &saID,
+			ServiceAccountId: &serviceAccountId,
 		}
 
-		if _, err := sess.Insert(&t); err != nil {
+		if _, err := sess.Insert(&token); err != nil {
 			return err
 		}
-		cmd.Result = &t
+		cmd.Result = &token
 		return nil
 	})
 }
 
-func (s *ServiceAccountsStoreImpl) DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error {
+func (s *ServiceAccountsStoreImpl) DeleteServiceAccountToken(ctx context.Context, orgId, serviceAccountId, tokenId int64) error {
 	rawSQL := "DELETE FROM api_key WHERE id=? and org_id=? and service_account_id=?"
 
 	return s.sqlStore.WithDbSession(ctx, func(sess *sqlstore.DBSession) error {
-		result, err := sess.Exec(rawSQL, tokenID, orgID, serviceAccountID)
+		result, err := sess.Exec(rawSQL, tokenId, orgId, serviceAccountId)
 		if err != nil {
 			return err
 		}
@@ -64,25 +85,45 @@ func (s *ServiceAccountsStoreImpl) DeleteServiceAccountToken(ctx context.Context
 }
 
 // assignApiKeyToServiceAccount sets the API key service account ID
-func (s *ServiceAccountsStoreImpl) assignApiKeyToServiceAccount(ctx context.Context, apikeyId int64, saccountId int64) error {
-	return s.sqlStore.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
-		key := models.ApiKey{Id: apikeyId}
-		exists, err := sess.Get(&key)
-		if err != nil {
-			s.log.Warn("API key not loaded", "err", err)
-			return err
-		}
-		if !exists {
-			s.log.Warn("API key not found", "err", err)
-			return models.ErrApiKeyNotFound
-		}
-		key.ServiceAccountId = &saccountId
+func (s *ServiceAccountsStoreImpl) assignApiKeyToServiceAccount(sess *sqlstore.DBSession, apiKeyId int64, serviceAccountId int64) error {
+	key := models.ApiKey{Id: apiKeyId}
+	exists, err := sess.Get(&key)
+	if err != nil {
+		s.log.Warn("API key not loaded", "err", err)
+		return err
+	}
+	if !exists {
+		s.log.Warn("API key not found", "err", err)
+		return models.ErrApiKeyNotFound
+	}
+	key.ServiceAccountId = &serviceAccountId
 
-		if _, err := sess.ID(key.Id).Update(&key); err != nil {
-			s.log.Warn("Could not update api key", "err", err)
-			return err
-		}
+	if _, err := sess.ID(key.Id).Update(&key); err != nil {
+		s.log.Warn("Could not update api key", "err", err)
+		return err
+	}
 
-		return nil
-	})
+	return nil
+}
+
+// detachApiKeyFromServiceAccount converts service account token to old API key
+func (s *ServiceAccountsStoreImpl) detachApiKeyFromServiceAccount(sess *sqlstore.DBSession, apiKeyId int64) error {
+	key := models.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 models.ErrApiKeyNotFound
+	}
+	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
 }

pkg/services/serviceaccounts/database/token_store_test.go

@@ -70,40 +70,61 @@ func TestStore_AddServiceAccountToken(t *testing.T) {
 	}
 }
 
-func TestStore_DeleteServiceAccountToken(t *testing.T) {
-	userToCreate := tests.TestUser{Login: "servicetestwithTeam@admin", IsServiceAccount: true}
+func TestStore_AddServiceAccountToken_WrongServiceAccount(t *testing.T) {
+	saToCreate := tests.TestUser{Login: "servicetestwithTeam@admin", IsServiceAccount: true}
 	db, store := setupTestDatabase(t)
-	user := tests.SetupUserServiceAccount(t, db, userToCreate)
+	sa := tests.SetupUserServiceAccount(t, db, saToCreate)
 
 	keyName := t.Name()
-	key, err := apikeygen.New(user.OrgId, keyName)
+	key, err := apikeygen.New(sa.OrgId, keyName)
 	require.NoError(t, err)
 
 	cmd := serviceaccounts.AddServiceAccountTokenCommand{
 		Name:          keyName,
-		OrgId:         user.OrgId,
+		OrgId:         sa.OrgId,
 		Key:           key.HashedKey,
 		SecondsToLive: 0,
 		Result:        &models.ApiKey{},
 	}
 
-	err = store.AddServiceAccountToken(context.Background(), user.Id, &cmd)
+	err = store.AddServiceAccountToken(context.Background(), sa.Id+1, &cmd)
+	require.Error(t, err, "It should not be possible to add token to non-existing service account")
+}
+
+func TestStore_DeleteServiceAccountToken(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:        &models.ApiKey{},
+	}
+
+	err = store.AddServiceAccountToken(context.Background(), sa.Id, &cmd)
 	require.NoError(t, err)
 	newKey := cmd.Result
 
 	// Delete key from wrong service account
-	err = store.DeleteServiceAccountToken(context.Background(), user.OrgId, user.Id+2, newKey.Id)
+	err = store.DeleteServiceAccountToken(context.Background(), sa.OrgId, sa.Id+2, newKey.Id)
 	require.Error(t, err)
 
 	// Delete key from wrong org
-	err = store.DeleteServiceAccountToken(context.Background(), user.OrgId+2, user.Id, newKey.Id)
+	err = store.DeleteServiceAccountToken(context.Background(), sa.OrgId+2, sa.Id, newKey.Id)
 	require.Error(t, err)
 
-	err = store.DeleteServiceAccountToken(context.Background(), user.OrgId, user.Id, newKey.Id)
+	err = store.DeleteServiceAccountToken(context.Background(), sa.OrgId, sa.Id, newKey.Id)
 	require.NoError(t, err)
 
 	// Verify against DB
-	keys, errT := store.ListTokens(context.Background(), user.OrgId, user.Id)
+	keys, errT := store.ListTokens(context.Background(), sa.OrgId, sa.Id)
 	require.NoError(t, errT)
 
 	for _, k := range keys {