Service accounts: Remove Add API keys buttons and remove one state of migrating for API keys tab (#63411)

* add: hide apikeys tab on start

* make use of store method

* added hiding of apikeys tab for new org creation

* missing err check

* removed unused files

* implemennted fake to make tests run

* move check for globalHideApikeys from org to admin

* refactor to remove the fake

* removed unused method calls for interface

* Update pkg/services/serviceaccounts/manager/service.go

Co-authored-by: Alexander Zobnin <alexanderzobnin@gmail.com>

* Update pkg/services/serviceaccounts/manager/service.go

Co-authored-by: Alexander Zobnin <alexanderzobnin@gmail.com>

* remove the checkglobal method

* removed duplicate global set const

* add count of apikeys for performance

* remove apikeys adding in UI

* added back deleted file

* added comment on component

* changed wording and copy for hiding and migrating service accounts

* refactor: remove migrationstatus in front/backend

This removes the migrationstatus state from the UI in favor of only
looking at the number of API keys to determine what to show to the user.
This simplifies the logic and makes less calls to the backend with each
page load. This was called both on the API keys page and the Service
accounts page.

- removes the state of migrationstatus from the UI
- removes the backend call
- removes the backend endpoint for migrationstatus

* Update pkg/services/apikey/apikeyimpl/xorm_store.go

Co-authored-by: Karl Persson <kalle.persson@grafana.com>

* changes the contet to also be primary

* change id of version for footer component

---------

Co-authored-by: Alexander Zobnin <alexanderzobnin@gmail.com>
Co-authored-by: Karl Persson <kalle.persson@grafana.com>

pkg/services/apikey/apikey.go

@@ -7,6 +7,7 @@ import (
 type Service interface {
 	GetAPIKeys(ctx context.Context, query *GetApiKeysQuery) error
 	GetAllAPIKeys(ctx context.Context, orgID int64) ([]*APIKey, error)
+	CountAPIKeys(ctx context.Context, orgID int64) (int64, error)
 	DeleteApiKey(ctx context.Context, cmd *DeleteCommand) error
 	AddAPIKey(ctx context.Context, cmd *AddCommand) error
 	GetApiKeyById(ctx context.Context, query *GetByIDQuery) error

pkg/services/apikey/apikeyimpl/apikey.go

@@ -68,6 +68,9 @@ func (s *Service) AddAPIKey(ctx context.Context, cmd *apikey.AddCommand) error {
 func (s *Service) UpdateAPIKeyLastUsedDate(ctx context.Context, tokenID int64) error {
 	return s.store.UpdateAPIKeyLastUsedDate(ctx, tokenID)
 }
+func (s *Service) CountAPIKeys(ctx context.Context, orgID int64) (int64, error) {
+	return s.store.CountAPIKeys(ctx, orgID)
+}
 
 func readQuotaConfig(cfg *setting.Cfg) (*quota.Map, error) {
 	limits := &quota.Map{}

pkg/services/apikey/apikeyimpl/sqlx_store.go

@@ -63,6 +63,18 @@ func (ss *sqlxStore) GetAllAPIKeys(ctx context.Context, orgID int64) ([]*apikey.
 	return result, err
 }
 
+func (ss *sqlxStore) CountAPIKeys(ctx context.Context, orgID int64) (int64, error) {
+	type result struct {
+		Count int64
+	}
+	r := result{}
+	err := ss.sess.Get(ctx, &r, `SELECT COUNT(*) AS count FROM api_key WHERE service_account_id IS NULL and org_id = ?`, orgID)
+	if err != nil {
+		return 0, err
+	}
+	return r.Count, err
+}
+
 func (ss *sqlxStore) DeleteApiKey(ctx context.Context, cmd *apikey.DeleteCommand) error {
 	res, err := ss.sess.Exec(ctx, "DELETE FROM api_key WHERE id=? and org_id=? and service_account_id IS NULL", cmd.ID, cmd.OrgID)
 	if err != nil {

pkg/services/apikey/apikeyimpl/store.go

@@ -10,6 +10,7 @@ import (
 type store interface {
 	GetAPIKeys(ctx context.Context, query *apikey.GetApiKeysQuery) error
 	GetAllAPIKeys(ctx context.Context, orgID int64) ([]*apikey.APIKey, error)
+	CountAPIKeys(ctx context.Context, orgID int64) (int64, error)
 	DeleteApiKey(ctx context.Context, cmd *apikey.DeleteCommand) error
 	AddAPIKey(ctx context.Context, cmd *apikey.AddCommand) error
 	GetApiKeyById(ctx context.Context, query *apikey.GetByIDQuery) error

pkg/services/apikey/apikeyimpl/xorm_store.go

@@ -65,6 +65,25 @@ func (ss *sqlStore) GetAllAPIKeys(ctx context.Context, orgID int64) ([]*apikey.A
 	return result, err
 }
 
+func (ss *sqlStore) CountAPIKeys(ctx context.Context, orgID int64) (int64, error) {
+	type result struct {
+		Count int64
+	}
+
+	r := result{}
+	err := ss.db.WithDbSession(ctx, func(sess *sqlstore.DBSession) error {
+		rawSQL := "SELECT COUNT(*) AS count FROM api_key WHERE org_id = ? and service_account_id IS NULL"
+		if _, err := sess.SQL(rawSQL, orgID).Get(&r); err != nil {
+			return err
+		}
+		return nil
+	})
+	if err != nil {
+		return 0, err
+	}
+	return r.Count, err
+}
+
 func (ss *sqlStore) DeleteApiKey(ctx context.Context, cmd *apikey.DeleteCommand) error {
 	return ss.db.WithDbSession(ctx, func(sess *db.Session) error {
 		rawSQL := "DELETE FROM api_key WHERE id=? and org_id=? and service_account_id IS NULL"

pkg/services/apikey/apikeytest/fake.go

@@ -8,6 +8,7 @@ import (
 
 type Service struct {
 	ExpectedError   error
+	ExpectedCount   int64
 	ExpectedAPIKeys []*apikey.APIKey
 	ExpectedAPIKey  *apikey.APIKey
 }
@@ -19,6 +20,9 @@ func (s *Service) GetAPIKeys(ctx context.Context, query *apikey.GetApiKeysQuery)
 func (s *Service) GetAllAPIKeys(ctx context.Context, orgID int64) ([]*apikey.APIKey, error) {
 	return s.ExpectedAPIKeys, s.ExpectedError
 }
+func (s *Service) CountAPIKeys(ctx context.Context, orgID int64) (int64, error) {
+	return s.ExpectedCount, s.ExpectedError
+}
 func (s *Service) GetApiKeyById(ctx context.Context, query *apikey.GetByIDQuery) error {
 	query.Result = s.ExpectedAPIKey
 	return s.ExpectedError

pkg/services/navtree/navtreeimpl/admin.go

@@ -80,12 +80,13 @@ func (s *ServiceImpl) getOrgAdminNode(c *contextmodel.ReqContext) (*navtree.NavL
 	}
 
 	hideApiKeys, _, _ := s.kvStore.Get(c.Req.Context(), c.OrgID, "serviceaccounts", "hideApiKeys")
-	apiKeys, err := s.apiKeyService.GetAllAPIKeys(c.Req.Context(), c.OrgID)
+	apiKeys, err := s.apiKeyService.CountAPIKeys(c.Req.Context(), c.OrgID)
 	if err != nil {
 		return nil, err
 	}
 
-	apiKeysHidden := hideApiKeys == "1" && len(apiKeys) == 0
+	// Hide API keys if the global setting is set or if the org setting is set and there are no API keys
+	apiKeysHidden := hideApiKeys == "1" && apiKeys == 0
 	if hasAccess(ac.ReqOrgAdmin, ac.ApiKeyAccessEvaluator) && !apiKeysHidden {
 		configNodes = append(configNodes, &navtree.NavLink{
 			Text:     "API keys",

pkg/services/serviceaccounts/api/api.go

@@ -41,7 +41,6 @@ type service interface {
 	SearchOrgServiceAccounts(ctx context.Context, query *serviceaccounts.SearchOrgServiceAccountsQuery) (*serviceaccounts.SearchOrgServiceAccountsResult, error)
 	ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error)
 	DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error
-	GetAPIKeysMigrationStatus(ctx context.Context, orgID int64) (*serviceaccounts.APIKeysMigrationStatus, error)
 	HideApiKeysTab(ctx context.Context, orgID int64) error
 	MigrateApiKeysToServiceAccounts(ctx context.Context, orgID int64) error
 	MigrateApiKey(ctx context.Context, orgID int64, keyId int64) error
@@ -89,8 +88,6 @@ func (api *ServiceAccountsAPI) RegisterAPIEndpoints() {
 			accesscontrol.EvalPermission(serviceaccounts.ActionWrite, serviceaccounts.ScopeID)), routing.Wrap(api.CreateToken))
 		serviceAccountsRoute.Delete("/:serviceAccountId/tokens/:tokenId", auth(middleware.ReqOrgAdmin,
 			accesscontrol.EvalPermission(serviceaccounts.ActionWrite, serviceaccounts.ScopeID)), routing.Wrap(api.DeleteToken))
-		serviceAccountsRoute.Get("/migrationstatus", auth(middleware.ReqOrgAdmin,
-			accesscontrol.EvalPermission(serviceaccounts.ActionRead)), routing.Wrap(api.GetAPIKeysMigrationStatus))
 		serviceAccountsRoute.Post("/hideApiKeys", auth(middleware.ReqOrgAdmin,
 			accesscontrol.EvalPermission(serviceaccounts.ActionCreate)), routing.Wrap(api.HideApiKeysTab))
 		serviceAccountsRoute.Post("/migrate", auth(middleware.ReqOrgAdmin,
@@ -364,15 +361,6 @@ func (api *ServiceAccountsAPI) SearchOrgServiceAccountsWithPaging(c *contextmode
 	return response.JSON(http.StatusOK, serviceAccountSearch)
 }
 
-// GET /api/serviceaccounts/migrationstatus
-func (api *ServiceAccountsAPI) GetAPIKeysMigrationStatus(ctx *contextmodel.ReqContext) response.Response {
-	upgradeStatus, err := api.service.GetAPIKeysMigrationStatus(ctx.Req.Context(), ctx.OrgID)
-	if err != nil {
-		return response.Error(http.StatusInternalServerError, "Internal server error", err)
-	}
-	return response.JSON(http.StatusOK, upgradeStatus)
-}
-
 // POST /api/serviceaccounts/hideapikeys
 func (api *ServiceAccountsAPI) HideApiKeysTab(ctx *contextmodel.ReqContext) response.Response {
 	if err := api.service.HideApiKeysTab(ctx.Req.Context(), ctx.OrgID); err != nil {

pkg/services/serviceaccounts/api/api_test.go

@@ -81,7 +81,7 @@ func TestServiceAccountsAPI_CreateServiceAccount(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
 			server := setupTests(t, func(a *ServiceAccountsAPI) {
-				a.service = &fakeService{ExpectedServiceAccount: tt.expectedSA, ExpectedErr: tt.expectedErr}
+				a.service = &fakeServiceAccountService{ExpectedServiceAccount: tt.expectedSA, ExpectedErr: tt.expectedErr}
 			})
 			req := server.NewRequest(http.MethodPost, "/api/serviceaccounts/", strings.NewReader(tt.body))
 			webtest.RequestWithSignedInUser(req, &user.SignedInUser{OrgRole: tt.basicRole, OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}})
@@ -159,7 +159,7 @@ func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
 			server := setupTests(t, func(a *ServiceAccountsAPI) {
-				a.service = &fakeService{ExpectedServiceAccountProfile: tt.expectedSA}
+				a.service = &fakeServiceAccountService{ExpectedServiceAccountProfile: tt.expectedSA}
 			})
 			req := server.NewGetRequest(fmt.Sprintf("/api/serviceaccounts/%d", tt.id))
 			webtest.RequestWithSignedInUser(req, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}})
@@ -221,7 +221,7 @@ func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
 			server := setupTests(t, func(a *ServiceAccountsAPI) {
-				a.service = &fakeService{ExpectedServiceAccountProfile: tt.expectedSA}
+				a.service = &fakeServiceAccountService{ExpectedServiceAccountProfile: tt.expectedSA}
 			})
 
 			req := server.NewRequest(http.MethodPatch, fmt.Sprintf("/api/serviceaccounts/%d", tt.id), strings.NewReader(tt.body))
@@ -240,7 +240,7 @@ func setupTests(t *testing.T, opts ...func(a *ServiceAccountsAPI)) *webtest.Serv
 	cfg := setting.NewCfg()
 	api := &ServiceAccountsAPI{
 		cfg:                  cfg,
-		service:              &fakeService{},
+		service:              &fakeServiceAccountService{},
 		accesscontrolService: &actest.FakeService{},
 		accesscontrol:        acimpl.ProvideAccessControl(cfg),
 		RouterRegister:       routing.NewRouteRegister(),
@@ -255,9 +255,9 @@ func setupTests(t *testing.T, opts ...func(a *ServiceAccountsAPI)) *webtest.Serv
 	return webtest.NewServer(t, api.RouterRegister)
 }
 
-var _ service = new(fakeService)
+var _ service = new(fakeServiceAccountService)
 
-type fakeService struct {
+type fakeServiceAccountService struct {
 	service
 	ExpectedErr                   error
 	ExpectedAPIKey                *apikey.APIKey
@@ -266,30 +266,30 @@ type fakeService struct {
 	ExpectedServiceAccountProfile *serviceaccounts.ServiceAccountProfileDTO
 }
 
-func (f *fakeService) CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) {
+func (f *fakeServiceAccountService) CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) {
 	return f.ExpectedServiceAccount, f.ExpectedErr
 }
 
-func (f *fakeService) DeleteServiceAccount(ctx context.Context, orgID, id int64) error {
+func (f *fakeServiceAccountService) DeleteServiceAccount(ctx context.Context, orgID, id int64) error {
 	return f.ExpectedErr
 }
 
-func (f *fakeService) RetrieveServiceAccount(ctx context.Context, orgID, id int64) (*serviceaccounts.ServiceAccountProfileDTO, error) {
+func (f *fakeServiceAccountService) RetrieveServiceAccount(ctx context.Context, orgID, id int64) (*serviceaccounts.ServiceAccountProfileDTO, error) {
 	return f.ExpectedServiceAccountProfile, f.ExpectedErr
 }
 
-func (f *fakeService) ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) {
+func (f *fakeServiceAccountService) ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) {
 	return f.ExpectedServiceAccountTokens, f.ExpectedErr
 }
 
-func (f *fakeService) UpdateServiceAccount(ctx context.Context, orgID, id int64, cmd *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) {
+func (f *fakeServiceAccountService) UpdateServiceAccount(ctx context.Context, orgID, id int64, cmd *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) {
 	return f.ExpectedServiceAccountProfile, f.ExpectedErr
 }
 
-func (f *fakeService) AddServiceAccountToken(ctx context.Context, id int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) (*apikey.APIKey, error) {
+func (f *fakeServiceAccountService) AddServiceAccountToken(ctx context.Context, id int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) (*apikey.APIKey, error) {
 	return f.ExpectedAPIKey, f.ExpectedErr
 }
 
-func (f *fakeService) DeleteServiceAccountToken(ctx context.Context, orgID, id, tokenID int64) error {
+func (f *fakeServiceAccountService) DeleteServiceAccountToken(ctx context.Context, orgID, id, tokenID int64) error {
 	return f.ExpectedErr
 }

pkg/services/serviceaccounts/api/token_test.go

@@ -43,7 +43,7 @@ func TestServiceAccountsAPI_ListTokens(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
 			server := setupTests(t, func(a *ServiceAccountsAPI) {
-				a.service = &fakeService{}
+				a.service = &fakeServiceAccountService{}
 			})
 			req := server.NewGetRequest(fmt.Sprintf("/api/serviceaccounts/%d/tokens", tt.id))
 			webtest.RequestWithSignedInUser(req, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}})
@@ -109,7 +109,7 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) {
 		t.Run(tt.desc, func(t *testing.T) {
 			server := setupTests(t, func(a *ServiceAccountsAPI) {
 				a.cfg.ApiKeyMaxSecondsToLive = tt.tokenTTL
-				a.service = &fakeService{
+				a.service = &fakeServiceAccountService{
 					ExpectedErr:    tt.expectedErr,
 					ExpectedAPIKey: tt.expectedAPIKey,
 				}
@@ -163,7 +163,7 @@ func TestServiceAccountsAPI_DeleteToken(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
 			server := setupTests(t, func(a *ServiceAccountsAPI) {
-				a.service = &fakeService{ExpectedErr: tt.expectedErr}
+				a.service = &fakeServiceAccountService{ExpectedErr: tt.expectedErr}
 			})
 
 			req := server.NewRequest(http.MethodDelete, fmt.Sprintf("/api/serviceaccounts/%d/tokens/%d", tt.saID, tt.apikeyID), nil)

pkg/services/serviceaccounts/database/store.go

@@ -388,22 +388,6 @@ func (s *ServiceAccountsStoreImpl) SearchOrgServiceAccounts(ctx context.Context,
 	return searchResult, nil
 }
 
-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)

pkg/services/serviceaccounts/manager/service.go

@@ -226,13 +226,6 @@ func (sa *ServiceAccountsService) DeleteServiceAccountToken(ctx context.Context,
 	return sa.store.DeleteServiceAccountToken(ctx, orgID, serviceAccountID, tokenID)
 }
 
-func (sa *ServiceAccountsService) GetAPIKeysMigrationStatus(ctx context.Context, orgID int64) (status *serviceaccounts.APIKeysMigrationStatus, err error) {
-	if err := validOrgID(orgID); err != nil {
-		return nil, err
-	}
-	return sa.store.GetAPIKeysMigrationStatus(ctx, orgID)
-}
-
 func (sa *ServiceAccountsService) HideApiKeysTab(ctx context.Context, orgID int64) error {
 	if err := validOrgID(orgID); err != nil {
 		return err

pkg/services/serviceaccounts/manager/service_test.go

@@ -17,10 +17,10 @@ type FakeServiceAccountStore struct {
 	ExpectedServiceAccountDTO               *serviceaccounts.ServiceAccountDTO
 	ExpectedServiceAccountProfileDTO        *serviceaccounts.ServiceAccountProfileDTO
 	ExpectedSearchServiceAccountQueryResult *serviceaccounts.SearchOrgServiceAccountsResult
-	ExpectedServiceAccountMigrationStatus   *serviceaccounts.APIKeysMigrationStatus
 	ExpectedStats                           *serviceaccounts.Stats
 	ExpectedAPIKeys                         []apikey.APIKey
 	ExpectedAPIKey                          *apikey.APIKey
+	ExpectedBoolean                         bool
 	ExpectedError                           error
 }
 
@@ -59,11 +59,6 @@ func (f *FakeServiceAccountStore) DeleteServiceAccount(ctx context.Context, orgI
 	return f.ExpectedError
 }
 
-// GetAPIKeysMigrationStatus is a fake getting the api keys migration status.
-func (f *FakeServiceAccountStore) GetAPIKeysMigrationStatus(ctx context.Context, orgID int64) (*serviceaccounts.APIKeysMigrationStatus, error) {
-	return f.ExpectedServiceAccountMigrationStatus, f.ExpectedError
-}
-
 // HideApiKeysTab is a fake hiding the api keys tab.
 func (f *FakeServiceAccountStore) HideApiKeysTab(ctx context.Context, orgID int64) error {
 	return f.ExpectedError

pkg/services/serviceaccounts/manager/store.go

@@ -26,7 +26,6 @@ type store interface {
 	RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error)
 	RetrieveServiceAccountIdByName(ctx context.Context, orgID int64, name string) (int64, error)
 	DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error
-	GetAPIKeysMigrationStatus(ctx context.Context, orgID int64) (*serviceaccounts.APIKeysMigrationStatus, error)
 	HideApiKeysTab(ctx context.Context, orgID int64) error
 	MigrateApiKeysToServiceAccounts(ctx context.Context, orgID int64) error
 	MigrateApiKey(ctx context.Context, orgID int64, keyId int64) error

pkg/services/serviceaccounts/models.go

@@ -130,10 +130,6 @@ type ServiceAccountProfileDTO struct {
 
 type ServiceAccountFilter string // used for filtering
 
-type APIKeysMigrationStatus struct {
-	Migrated bool `json:"migrated"`
-}
-
 const (
 	FilterOnlyExpiredTokens ServiceAccountFilter = "expiredTokens"
 	FilterOnlyDisabled      ServiceAccountFilter = "disabled"