From f82264c2b12b10c9e543fdce85d7a1a1df6c360a Mon Sep 17 00:00:00 2001 From: Alexander Zobnin Date: Wed, 15 Jun 2022 15:59:40 +0300 Subject: [PATCH] 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/api/http_server.go | 5 +- pkg/api/index.go | 3 +- pkg/services/serviceaccounts/api/api.go | 152 ++++--- pkg/services/serviceaccounts/api/api_test.go | 19 +- pkg/services/serviceaccounts/api/token.go | 3 + .../serviceaccounts/api/token_test.go | 8 +- .../serviceaccounts/database/database.go | 387 ++++++++++-------- .../serviceaccounts/database/database_test.go | 176 +++++++- .../serviceaccounts/database/token_store.go | 93 +++-- .../database/token_store_test.go | 41 +- .../serviceaccounts/manager/service.go | 4 +- pkg/services/serviceaccounts/models.go | 4 + .../serviceaccounts/serviceaccounts.go | 7 +- pkg/services/serviceaccounts/tests/common.go | 77 +++- pkg/services/sqlstore/apikey.go | 11 +- pkg/services/sqlstore/mockstore/mockstore.go | 2 +- pkg/services/sqlstore/store.go | 2 +- .../features/api-keys/APIKeysMigratedCard.tsx | 41 ++ .../features/api-keys/ApiKeysPage.test.tsx | 9 + public/app/features/api-keys/ApiKeysPage.tsx | 57 ++- public/app/features/api-keys/ApiKeysTable.tsx | 25 +- .../api-keys/MigrateToServiceAccountsCard.tsx | 45 ++ public/app/features/api-keys/state/actions.ts | 48 ++- .../app/features/api-keys/state/reducers.ts | 7 +- .../features/api-keys/state/selectors.test.ts | 10 + .../ServiceAccountsListPage.test.tsx | 3 + .../ServiceAccountsListPage.tsx | 21 +- .../features/serviceaccounts/state/actions.ts | 8 + .../serviceaccounts/state/reducers.ts | 5 + public/app/types/apiKeys.ts | 1 + public/app/types/serviceaccount.ts | 5 + 31 files changed, 961 insertions(+), 318 deletions(-) create mode 100644 public/app/features/api-keys/APIKeysMigratedCard.tsx create mode 100644 public/app/features/api-keys/MigrateToServiceAccountsCard.tsx diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 21415cf10ea..17f4095681f 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -24,6 +24,7 @@ import ( httpstatic "github.com/grafana/grafana/pkg/api/static" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/framework/coremodel" + "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/remotecache" @@ -162,6 +163,7 @@ type HTTPServer struct { dashboardVersionService dashver.Service starService star.Service CoremodelRegistry *coremodel.Registry + kvStore kvstore.KVStore } type ServerOptions struct { @@ -195,7 +197,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi avatarCacheServer *avatar.AvatarCacheServer, preferenceService pref.Service, entityEventsService store.EntityEventsService, teamsPermissionsService accesscontrol.TeamPermissionsService, folderPermissionsService accesscontrol.FolderPermissionsService, dashboardPermissionsService accesscontrol.DashboardPermissionsService, dashboardVersionService dashver.Service, - starService star.Service, coremodelRegistry *coremodel.Registry, csrfService csrf.Service, + starService star.Service, coremodelRegistry *coremodel.Registry, csrfService csrf.Service, kvStore kvstore.KVStore, ) (*HTTPServer, error) { web.Env = cfg.Env m := web.New() @@ -277,6 +279,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi dashboardVersionService: dashboardVersionService, starService: starService, CoremodelRegistry: coremodelRegistry, + kvStore: kvStore, } if hs.Listener != nil { hs.log.Debug("Using provided listener") diff --git a/pkg/api/index.go b/pkg/api/index.go index 945840a2936..bdcef87cf94 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -307,7 +307,8 @@ func (hs *HTTPServer) getNavTree(c *models.ReqContext, hasEditPerm bool, prefs * }) } - if hasAccess(ac.ReqOrgAdmin, apiKeyAccessEvaluator) { + apiKeysHidden, _, _ := hs.kvStore.Get(c.Req.Context(), c.OrgId, "serviceaccounts", "hideApiKeys") + if hasAccess(ac.ReqOrgAdmin, apiKeyAccessEvaluator) && apiKeysHidden != "1" { configNodes = append(configNodes, &dtos.NavLink{ Text: "API keys", Id: "apikeys", diff --git a/pkg/services/serviceaccounts/api/api.go b/pkg/services/serviceaccounts/api/api.go index bb995432274..3b5f8401d9c 100644 --- a/pkg/services/serviceaccounts/api/api.go +++ b/pkg/services/serviceaccounts/api/api.go @@ -62,21 +62,25 @@ func (api *ServiceAccountsAPI) RegisterAPIEndpoints( serviceAccountsRoute.Get("/:serviceAccountId", auth(middleware.ReqOrgAdmin, accesscontrol.EvalPermission(serviceaccounts.ActionRead, serviceaccounts.ScopeID)), routing.Wrap(api.RetrieveServiceAccount)) serviceAccountsRoute.Patch("/:serviceAccountId", auth(middleware.ReqOrgAdmin, - accesscontrol.EvalPermission(serviceaccounts.ActionWrite, serviceaccounts.ScopeID)), routing.Wrap(api.updateServiceAccount)) + accesscontrol.EvalPermission(serviceaccounts.ActionWrite, serviceaccounts.ScopeID)), routing.Wrap(api.UpdateServiceAccount)) serviceAccountsRoute.Delete("/:serviceAccountId", auth(middleware.ReqOrgAdmin, accesscontrol.EvalPermission(serviceaccounts.ActionDelete, serviceaccounts.ScopeID)), routing.Wrap(api.DeleteServiceAccount)) - // TODO: - // for 9.0 please reenable this with issue https://github.com/grafana/grafana-enterprise/issues/2969 - // serviceAccountsRoute.Post("/upgradeall", auth(middleware.ReqOrgAdmin, - // accesscontrol.EvalPermission(serviceaccounts.ActionCreate)), routing.Wrap(api.UpgradeServiceAccounts)) - // serviceAccountsRoute.Post("/convert/:keyId", auth(middleware.ReqOrgAdmin, - // accesscontrol.EvalPermission(serviceaccounts.ActionCreate, serviceaccounts.ScopeID)), routing.Wrap(api.ConvertToServiceAccount)) serviceAccountsRoute.Get("/:serviceAccountId/tokens", auth(middleware.ReqOrgAdmin, accesscontrol.EvalPermission(serviceaccounts.ActionRead, serviceaccounts.ScopeID)), routing.Wrap(api.ListTokens)) serviceAccountsRoute.Post("/:serviceAccountId/tokens", auth(middleware.ReqOrgAdmin, 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, + 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("/revert/:keyId", auth(middleware.ReqOrgAdmin, + accesscontrol.EvalPermission(serviceaccounts.ActionDelete)), routing.Wrap(api.RevertApiKey)) }) } @@ -101,55 +105,7 @@ func (api *ServiceAccountsAPI) CreateServiceAccount(c *models.ReqContext) respon return response.JSON(http.StatusCreated, serviceAccount) } -func (api *ServiceAccountsAPI) DeleteServiceAccount(ctx *models.ReqContext) response.Response { - scopeID, err := strconv.ParseInt(web.Params(ctx.Req)[":serviceAccountId"], 10, 64) - if err != nil { - return response.Error(http.StatusBadRequest, "serviceAccountId is invalid", err) - } - err = api.service.DeleteServiceAccount(ctx.Req.Context(), ctx.OrgId, scopeID) - if err != nil { - return response.Error(http.StatusInternalServerError, "Service account deletion error", err) - } - return response.Success("Service account deleted") -} - -func (api *ServiceAccountsAPI) UpgradeServiceAccounts(ctx *models.ReqContext) response.Response { - if err := api.store.UpgradeServiceAccounts(ctx.Req.Context()); err == nil { - return response.Success("Service accounts upgraded") - } else { - return response.Error(http.StatusInternalServerError, "Internal server error", err) - } -} - -func (api *ServiceAccountsAPI) ConvertToServiceAccount(ctx *models.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) - } - if err := api.store.ConvertToServiceAccounts(ctx.Req.Context(), []int64{keyId}); err == nil { - return response.Success("Service accounts converted") - } else { - return response.Error(500, "Internal server error", err) - } -} - -func (api *ServiceAccountsAPI) getAccessControlMetadata(c *models.ReqContext, saIDs map[string]bool) map[string]accesscontrol.Metadata { - if api.accesscontrol.IsDisabled() || !c.QueryBool("accesscontrol") { - return map[string]accesscontrol.Metadata{} - } - - if c.SignedInUser.Permissions == nil { - return map[string]accesscontrol.Metadata{} - } - - permissions, ok := c.SignedInUser.Permissions[c.OrgId] - if !ok { - return map[string]accesscontrol.Metadata{} - } - - return accesscontrol.GetResourcesMetadata(c.Req.Context(), permissions, "serviceaccounts:id:", saIDs) -} - +// GET /api/serviceaccounts/:serviceAccountId func (api *ServiceAccountsAPI) RetrieveServiceAccount(ctx *models.ReqContext) response.Response { scopeID, err := strconv.ParseInt(web.Params(ctx.Req)[":serviceAccountId"], 10, 64) if err != nil { @@ -180,7 +136,8 @@ func (api *ServiceAccountsAPI) RetrieveServiceAccount(ctx *models.ReqContext) re return response.JSON(http.StatusOK, serviceAccount) } -func (api *ServiceAccountsAPI) updateServiceAccount(c *models.ReqContext) response.Response { +// PATCH /api/serviceaccounts/:serviceAccountId +func (api *ServiceAccountsAPI) UpdateServiceAccount(c *models.ReqContext) response.Response { scopeID, err := strconv.ParseInt(web.Params(c.Req)[":serviceAccountId"], 10, 64) if err != nil { return response.Error(http.StatusBadRequest, "Service Account ID is invalid", err) @@ -221,6 +178,19 @@ func (api *ServiceAccountsAPI) updateServiceAccount(c *models.ReqContext) respon }) } +// DELETE /api/serviceaccounts/:serviceAccountId +func (api *ServiceAccountsAPI) DeleteServiceAccount(ctx *models.ReqContext) response.Response { + scopeID, err := strconv.ParseInt(web.Params(ctx.Req)[":serviceAccountId"], 10, 64) + if err != nil { + return response.Error(http.StatusBadRequest, "Service account ID is invalid", err) + } + err = api.service.DeleteServiceAccount(ctx.Req.Context(), ctx.OrgId, scopeID) + if err != nil { + return response.Error(http.StatusInternalServerError, "Service account deletion error", err) + } + return response.Success("Service account deleted") +} + // SearchOrgServiceAccountsWithPaging is an HTTP handler to search for org users with paging. // GET /api/serviceaccounts/search func (api *ServiceAccountsAPI) SearchOrgServiceAccountsWithPaging(c *models.ReqContext) response.Response { @@ -266,3 +236,71 @@ func (api *ServiceAccountsAPI) SearchOrgServiceAccountsWithPaging(c *models.ReqC return response.JSON(http.StatusOK, serviceAccountSearch) } + +// GET /api/serviceaccounts/migrationstatus +func (api *ServiceAccountsAPI) GetAPIKeysMigrationStatus(ctx *models.ReqContext) response.Response { + upgradeStatus, err := api.store.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 *models.ReqContext) response.Response { + if err := api.store.HideApiKeysTab(ctx.Req.Context(), ctx.OrgId); err != nil { + return response.Error(http.StatusInternalServerError, "Internal server error", err) + } + return response.Success("API keys hidden") +} + +// POST /api/serviceaccounts/migrate +func (api *ServiceAccountsAPI) MigrateApiKeysToServiceAccounts(ctx *models.ReqContext) response.Response { + if err := api.store.MigrateApiKeysToServiceAccounts(ctx.Req.Context(), ctx.OrgId); err == nil { + return response.Success("API keys migrated to service accounts") + } else { + return response.Error(http.StatusInternalServerError, "Internal server error", err) + } +} + +// POST /api/serviceaccounts/migrate/:keyId +func (api *ServiceAccountsAPI) ConvertToServiceAccount(ctx *models.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) + } + if err := api.store.MigrateApiKey(ctx.Req.Context(), ctx.OrgId, keyId); err == nil { + return response.Success("Service accounts converted") + } else { + return response.Error(http.StatusInternalServerError, "Error converting API key", err) + } +} + +// POST /api/serviceaccounts/revert/:keyId +func (api *ServiceAccountsAPI) RevertApiKey(ctx *models.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) + } + if err := api.store.RevertApiKey(ctx.Req.Context(), keyId); err != nil { + return response.Error(http.StatusInternalServerError, "Error reverting API key", err) + } + return response.Success("API key reverted") +} + +func (api *ServiceAccountsAPI) getAccessControlMetadata(c *models.ReqContext, saIDs map[string]bool) map[string]accesscontrol.Metadata { + if api.accesscontrol.IsDisabled() || !c.QueryBool("accesscontrol") { + return map[string]accesscontrol.Metadata{} + } + + if c.SignedInUser.Permissions == nil { + return map[string]accesscontrol.Metadata{} + } + + permissions, ok := c.SignedInUser.Permissions[c.OrgId] + if !ok { + return map[string]accesscontrol.Metadata{} + } + + return accesscontrol.GetResourcesMetadata(c.Req.Context(), permissions, "serviceaccounts:id:", saIDs) +} diff --git a/pkg/services/serviceaccounts/api/api_test.go b/pkg/services/serviceaccounts/api/api_test.go index 01f3f75a39d..e82ef65faa3 100644 --- a/pkg/services/serviceaccounts/api/api_test.go +++ b/pkg/services/serviceaccounts/api/api_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/grafana/grafana/pkg/api/routing" + "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" @@ -34,6 +35,8 @@ var ( func TestServiceAccountsAPI_CreateServiceAccount(t *testing.T) { store := sqlstore.InitTestDB(t) + kvStore := kvstore.ProvideService(store) + saStore := database.NewServiceAccountsStore(store, kvStore) svcmock := tests.ServiceAccountMock{} autoAssignOrg := store.Cfg.AutoAssignOrg @@ -121,7 +124,7 @@ func TestServiceAccountsAPI_CreateServiceAccount(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { serviceAccountRequestScenario(t, http.MethodPost, serviceAccountPath, testUser, func(httpmethod string, endpoint string, user *tests.TestUser) { - server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, database.NewServiceAccountsStore(store)) + server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, saStore) marshalled, err := json.Marshal(tc.body) require.NoError(t, err) @@ -152,6 +155,8 @@ func TestServiceAccountsAPI_CreateServiceAccount(t *testing.T) { // with permissions and without permissions func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) { store := sqlstore.InitTestDB(t) + kvStore := kvstore.ProvideService(store) + saStore := database.NewServiceAccountsStore(store, kvStore) svcmock := tests.ServiceAccountMock{} var requestResponse = func(server *web.Mux, httpMethod, requestpath string) *httptest.ResponseRecorder { @@ -180,7 +185,7 @@ func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) { } serviceAccountRequestScenario(t, http.MethodDelete, serviceAccountIDPath, &testcase.user, func(httpmethod string, endpoint string, user *tests.TestUser) { createduser := tests.SetupUserServiceAccount(t, store, testcase.user) - server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), testcase.acmock, store, database.NewServiceAccountsStore(store)) + server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), testcase.acmock, store, saStore) actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, fmt.Sprint(createduser.Id))).Code require.Equal(t, testcase.expectedCode, actual) }) @@ -204,7 +209,7 @@ func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) { } serviceAccountRequestScenario(t, http.MethodDelete, serviceAccountIDPath, &testcase.user, func(httpmethod string, endpoint string, user *tests.TestUser) { createduser := tests.SetupUserServiceAccount(t, store, testcase.user) - server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), testcase.acmock, store, database.NewServiceAccountsStore(store)) + server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), testcase.acmock, store, saStore) actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, createduser.Id)).Code require.Equal(t, testcase.expectedCode, actual) }) @@ -246,6 +251,8 @@ func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock, func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) { store := sqlstore.InitTestDB(t) + kvStore := kvstore.ProvideService(store) + saStore := database.NewServiceAccountsStore(store, kvStore) svcmock := tests.ServiceAccountMock{} type testRetrieveSATestCase struct { desc string @@ -310,7 +317,7 @@ func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) { createdUser := tests.SetupUserServiceAccount(t, store, *tc.user) scopeID = int(createdUser.Id) } - server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, database.NewServiceAccountsStore(store)) + server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, saStore) actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, scopeID)) @@ -335,6 +342,8 @@ func newString(s string) *string { func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) { store := sqlstore.InitTestDB(t) + kvStore := kvstore.ProvideService(store) + saStore := database.NewServiceAccountsStore(store, kvStore) svcmock := tests.ServiceAccountMock{} type testUpdateSATestCase struct { desc string @@ -428,7 +437,7 @@ func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - server, saAPI := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, database.NewServiceAccountsStore(store)) + server, saAPI := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, saStore) scopeID := tc.Id if tc.user != nil { createdUser := tests.SetupUserServiceAccount(t, store, *tc.user) diff --git a/pkg/services/serviceaccounts/api/token.go b/pkg/services/serviceaccounts/api/token.go index ecce0763a31..6ea860c1a14 100644 --- a/pkg/services/serviceaccounts/api/token.go +++ b/pkg/services/serviceaccounts/api/token.go @@ -38,6 +38,7 @@ func hasExpired(expiration *int64) bool { const sevenDaysAhead = 7 * 24 * time.Hour +// GET /api/serviceaccounts/:serviceAccountId/tokens func (api *ServiceAccountsAPI) ListTokens(ctx *models.ReqContext) response.Response { saID, err := strconv.ParseInt(web.Params(ctx.Req)[":serviceAccountId"], 10, 64) if err != nil { @@ -76,6 +77,7 @@ func (api *ServiceAccountsAPI) ListTokens(ctx *models.ReqContext) response.Respo } // CreateNewToken adds an API key to a service account +// POST /api/serviceaccounts/:serviceAccountId/tokens func (api *ServiceAccountsAPI) CreateToken(c *models.ReqContext) response.Response { saID, err := strconv.ParseInt(web.Params(c.Req)[":serviceAccountId"], 10, 64) if err != nil { @@ -136,6 +138,7 @@ func (api *ServiceAccountsAPI) CreateToken(c *models.ReqContext) response.Respon } // DeleteToken deletes service account tokens +// DELETE /api/serviceaccounts/:serviceAccountId/tokens/:tokenId func (api *ServiceAccountsAPI) DeleteToken(c *models.ReqContext) response.Response { saID, err := strconv.ParseInt(web.Params(c.Req)[":serviceAccountId"], 10, 64) if err != nil { diff --git a/pkg/services/serviceaccounts/api/token_test.go b/pkg/services/serviceaccounts/api/token_test.go index 6fb2b4fe66a..9f4e925b7a4 100644 --- a/pkg/services/serviceaccounts/api/token_test.go +++ b/pkg/services/serviceaccounts/api/token_test.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/components/apikeygen" apikeygenprefix "github.com/grafana/grafana/pkg/components/apikeygenprefixed" + "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" @@ -50,6 +51,8 @@ func createTokenforSA(t *testing.T, store serviceaccounts.Store, keyName string, func TestServiceAccountsAPI_CreateToken(t *testing.T) { store := sqlstore.InitTestDB(t) + kvStore := kvstore.ProvideService(store) + saStore := database.NewServiceAccountsStore(store, kvStore) svcmock := tests.ServiceAccountMock{} sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true}) @@ -130,7 +133,7 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) { bodyString = string(b) } - server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, database.NewServiceAccountsStore(store)) + server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, saStore) actual := requestResponse(server, http.MethodPost, endpoint, strings.NewReader(bodyString)) actualCode := actual.Code @@ -164,8 +167,9 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) { func TestServiceAccountsAPI_DeleteToken(t *testing.T) { store := sqlstore.InitTestDB(t) + kvStore := kvstore.ProvideService(store) svcMock := &tests.ServiceAccountMock{} - saStore := database.NewServiceAccountsStore(store) + saStore := database.NewServiceAccountsStore(store, kvStore) sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true}) type testCreateSAToken struct { diff --git a/pkg/services/serviceaccounts/database/database.go b/pkg/services/serviceaccounts/database/database.go index c85c2ccd0f0..90f0398924c 100644 --- a/pkg/services/serviceaccounts/database/database.go +++ b/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 } diff --git a/pkg/services/serviceaccounts/database/database_test.go b/pkg/services/serviceaccounts/database/database_test.go index 97a40680d01..d92e0521582 100644 --- a/pkg/services/serviceaccounts/database/database_test.go +++ b/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) + } + }) + } +} diff --git a/pkg/services/serviceaccounts/database/token_store.go b/pkg/services/serviceaccounts/database/token_store.go index 4a4b04f9585..ce4e860d34f 100644 --- a/pkg/services/serviceaccounts/database/token_store.go +++ b/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 } diff --git a/pkg/services/serviceaccounts/database/token_store_test.go b/pkg/services/serviceaccounts/database/token_store_test.go index 1560e19ffcd..85f9a316f6c 100644 --- a/pkg/services/serviceaccounts/database/token_store_test.go +++ b/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 { diff --git a/pkg/services/serviceaccounts/manager/service.go b/pkg/services/serviceaccounts/manager/service.go index aebed6a1b5c..583040c52f8 100644 --- a/pkg/services/serviceaccounts/manager/service.go +++ b/pkg/services/serviceaccounts/manager/service.go @@ -4,6 +4,7 @@ import ( "context" "github.com/grafana/grafana/pkg/api/routing" + "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/services/accesscontrol" @@ -29,13 +30,14 @@ func ProvideServiceAccountsService( cfg *setting.Cfg, features featuremgmt.FeatureToggles, store *sqlstore.SQLStore, + kvStore kvstore.KVStore, ac accesscontrol.AccessControl, routeRegister routing.RouteRegister, usageStats usagestats.Service, ) (*ServiceAccountsService, error) { s := &ServiceAccountsService{ features: features, - store: database.NewServiceAccountsStore(store), + store: database.NewServiceAccountsStore(store, kvStore), log: log.New("serviceaccounts"), } diff --git a/pkg/services/serviceaccounts/models.go b/pkg/services/serviceaccounts/models.go index c617a280424..5acb745df6e 100644 --- a/pkg/services/serviceaccounts/models.go +++ b/pkg/services/serviceaccounts/models.go @@ -73,6 +73,10 @@ type ServiceAccountProfileDTO struct { type ServiceAccountFilter string // used for filtering +type APIKeysMigrationStatus struct { + Migrated bool `json:"migrated"` +} + const ( FilterOnlyExpiredTokens ServiceAccountFilter = "expiredTokens" FilterOnlyDisabled ServiceAccountFilter = "disabled" diff --git a/pkg/services/serviceaccounts/serviceaccounts.go b/pkg/services/serviceaccounts/serviceaccounts.go index 632aa42a928..71490383fb6 100644 --- a/pkg/services/serviceaccounts/serviceaccounts.go +++ b/pkg/services/serviceaccounts/serviceaccounts.go @@ -22,8 +22,11 @@ type Store interface { RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*ServiceAccountProfileDTO, error) RetrieveServiceAccountIdByName(ctx context.Context, orgID int64, name string) (int64, error) DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error - UpgradeServiceAccounts(ctx context.Context) error - ConvertToServiceAccounts(ctx context.Context, keys []int64) error + GetAPIKeysMigrationStatus(ctx context.Context, orgID int64) (*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 + RevertApiKey(ctx context.Context, keyId int64) error ListTokens(ctx context.Context, orgID int64, serviceAccount int64) ([]*models.ApiKey, error) DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *AddServiceAccountTokenCommand) error diff --git a/pkg/services/serviceaccounts/tests/common.go b/pkg/services/serviceaccounts/tests/common.go index 97ec2fef759..79526c10435 100644 --- a/pkg/services/serviceaccounts/tests/common.go +++ b/pkg/services/serviceaccounts/tests/common.go @@ -19,6 +19,13 @@ type TestUser struct { IsServiceAccount bool } +type TestApiKey struct { + Name string + Role models.RoleType + OrgId int64 + Key string +} + func SetupUserServiceAccount(t *testing.T, sqlStore *sqlstore.SQLStore, testUser TestUser) *models.User { role := string(models.ROLE_VIEWER) if testUser.Role != "" { @@ -35,6 +42,28 @@ func SetupUserServiceAccount(t *testing.T, sqlStore *sqlstore.SQLStore, testUser return u1 } +func SetupApiKey(t *testing.T, sqlStore *sqlstore.SQLStore, testKey TestApiKey) *models.ApiKey { + role := models.ROLE_VIEWER + if testKey.Role != "" { + role = testKey.Role + } + + addKeyCmd := &models.AddApiKeyCommand{ + Name: testKey.Name, + Role: role, + OrgId: testKey.OrgId, + } + + if testKey.Key != "" { + addKeyCmd.Key = testKey.Key + } else { + addKeyCmd.Key = "secret" + } + err := sqlStore.AddAPIKey(context.Background(), addKeyCmd) + require.NoError(t, err) + return addKeyCmd.Result +} + // create mock for serviceaccountservice type ServiceAccountMock struct{} @@ -72,17 +101,20 @@ var _ serviceaccounts.Store = new(ServiceAccountsStoreMock) var _ serviceaccounts.Service = new(ServiceAccountMock) type Calls struct { - CreateServiceAccount []interface{} - RetrieveServiceAccount []interface{} - DeleteServiceAccount []interface{} - UpgradeServiceAccounts []interface{} - ConvertServiceAccounts []interface{} - ListTokens []interface{} - DeleteServiceAccountToken []interface{} - UpdateServiceAccount []interface{} - AddServiceAccountToken []interface{} - SearchOrgServiceAccounts []interface{} - RetrieveServiceAccountIdByName []interface{} + CreateServiceAccount []interface{} + RetrieveServiceAccount []interface{} + DeleteServiceAccount []interface{} + GetAPIKeysMigrationStatus []interface{} + HideApiKeysTab []interface{} + MigrateApiKeysToServiceAccounts []interface{} + MigrateApiKey []interface{} + RevertApiKey []interface{} + ListTokens []interface{} + DeleteServiceAccountToken []interface{} + UpdateServiceAccount []interface{} + AddServiceAccountToken []interface{} + SearchOrgServiceAccounts []interface{} + RetrieveServiceAccountIdByName []interface{} } type ServiceAccountsStoreMock struct { @@ -106,13 +138,28 @@ func (s *ServiceAccountsStoreMock) DeleteServiceAccount(ctx context.Context, org return nil } -func (s *ServiceAccountsStoreMock) UpgradeServiceAccounts(ctx context.Context) error { - s.Calls.UpgradeServiceAccounts = append(s.Calls.UpgradeServiceAccounts, []interface{}{ctx}) +func (s *ServiceAccountsStoreMock) HideApiKeysTab(ctx context.Context, orgID int64) error { + s.Calls.HideApiKeysTab = append(s.Calls.HideApiKeysTab, []interface{}{ctx}) return nil } -func (s *ServiceAccountsStoreMock) ConvertToServiceAccounts(ctx context.Context, keys []int64) error { - s.Calls.ConvertServiceAccounts = append(s.Calls.ConvertServiceAccounts, []interface{}{ctx}) +func (s *ServiceAccountsStoreMock) GetAPIKeysMigrationStatus(ctx context.Context, orgID int64) (*serviceaccounts.APIKeysMigrationStatus, error) { + s.Calls.GetAPIKeysMigrationStatus = append(s.Calls.GetAPIKeysMigrationStatus, []interface{}{ctx}) + return nil, nil +} + +func (s *ServiceAccountsStoreMock) MigrateApiKeysToServiceAccounts(ctx context.Context, orgID int64) error { + s.Calls.MigrateApiKeysToServiceAccounts = append(s.Calls.MigrateApiKeysToServiceAccounts, []interface{}{ctx}) + return nil +} + +func (s *ServiceAccountsStoreMock) MigrateApiKey(ctx context.Context, orgID int64, keyId int64) error { + s.Calls.MigrateApiKey = append(s.Calls.MigrateApiKey, []interface{}{ctx}) + return nil +} + +func (s *ServiceAccountsStoreMock) RevertApiKey(ctx context.Context, keyId int64) error { + s.Calls.RevertApiKey = append(s.Calls.RevertApiKey, []interface{}{ctx}) return nil } diff --git a/pkg/services/sqlstore/apikey.go b/pkg/services/sqlstore/apikey.go index 74d75356641..ab06c912e13 100644 --- a/pkg/services/sqlstore/apikey.go +++ b/pkg/services/sqlstore/apikey.go @@ -42,12 +42,15 @@ func (ss *SQLStore) GetAPIKeys(ctx context.Context, query *models.GetApiKeysQuer }) } -// GetAllOrgsAPIKeys queries the database for valid non SA APIKeys across all orgs -func (ss *SQLStore) GetAllOrgsAPIKeys(ctx context.Context) []*models.ApiKey { +// GetAllAPIKeys queries the database for valid non SA APIKeys across all orgs +func (ss *SQLStore) GetAllAPIKeys(ctx context.Context, orgID int64) []*models.ApiKey { result := make([]*models.ApiKey, 0) err := ss.WithDbSession(ctx, func(dbSession *DBSession) error { - sess := dbSession. //CHECK how many API keys do our clients have? Can we load them all? - Where("(expires IS NULL OR expires >= ?) AND service_account_id IS NULL", timeNow().Unix()).Asc("name") + sess := dbSession. + Where("(expires IS NULL OR expires >= ?) AND service_account_id IS NULL", timeNow().Unix()).Asc("name") + if orgID != -1 { + sess = sess.Where("org_id=?", orgID) + } return sess.Find(&result) }) if err != nil { diff --git a/pkg/services/sqlstore/mockstore/mockstore.go b/pkg/services/sqlstore/mockstore/mockstore.go index 77b73c6948c..e68bc681ff5 100644 --- a/pkg/services/sqlstore/mockstore/mockstore.go +++ b/pkg/services/sqlstore/mockstore/mockstore.go @@ -537,7 +537,7 @@ func (m *SQLStoreMock) GetAPIKeys(ctx context.Context, query *models.GetApiKeysQ return m.ExpectedError } -func (m *SQLStoreMock) GetAllOrgsAPIKeys(ctx context.Context) []*models.ApiKey { +func (m *SQLStoreMock) GetAllAPIKeys(ctx context.Context, orgID int64) []*models.ApiKey { return nil } diff --git a/pkg/services/sqlstore/store.go b/pkg/services/sqlstore/store.go index 1e3124f148a..422b30c7917 100644 --- a/pkg/services/sqlstore/store.go +++ b/pkg/services/sqlstore/store.go @@ -111,7 +111,7 @@ type Store interface { SetAlertNotificationStateToPendingCommand(ctx context.Context, cmd *models.SetAlertNotificationStateToPendingCommand) error GetOrCreateAlertNotificationState(ctx context.Context, cmd *models.GetOrCreateNotificationStateQuery) error GetAPIKeys(ctx context.Context, query *models.GetApiKeysQuery) error - GetAllOrgsAPIKeys(ctx context.Context) []*models.ApiKey + GetAllAPIKeys(ctx context.Context, orgID int64) []*models.ApiKey DeleteApiKey(ctx context.Context, cmd *models.DeleteApiKeyCommand) error AddAPIKey(ctx context.Context, cmd *models.AddApiKeyCommand) error GetApiKeyById(ctx context.Context, query *models.GetApiKeyByIdQuery) error diff --git a/public/app/features/api-keys/APIKeysMigratedCard.tsx b/public/app/features/api-keys/APIKeysMigratedCard.tsx new file mode 100644 index 00000000000..8198154a64f --- /dev/null +++ b/public/app/features/api-keys/APIKeysMigratedCard.tsx @@ -0,0 +1,41 @@ +import { css } from '@emotion/css'; +import React from 'react'; + +import { GrafanaTheme2 } from '@grafana/data'; +import { Alert, LinkButton, useStyles2 } from '@grafana/ui'; + +interface Props { + onHideApiKeys: () => void; +} + +export const APIKeysMigratedCard = ({ onHideApiKeys }: Props): JSX.Element => { + const styles = useStyles2(getStyles); + + return ( + +
+ We have upgraded your API keys into more powerful Service accounts and tokens. All your keys are safe and + working - you will find them inside respective service accounts. Keys are now called tokens. +
+
+ + Go to service accounts tab and never show API keys tab again + + Go to service accounts tab +
+ + ); +}; + +export const getStyles = (theme: GrafanaTheme2) => ({ + text: css` + margin-bottom: ${theme.spacing(2)}; + `, + actionRow: css` + display: flex; + align-items: center; + `, + actionButton: css` + margin-right: ${theme.spacing(2)}; + `, +}); diff --git a/public/app/features/api-keys/ApiKeysPage.test.tsx b/public/app/features/api-keys/ApiKeysPage.test.tsx index 33e2142283d..a2d1a772fc7 100644 --- a/public/app/features/api-keys/ApiKeysPage.test.tsx +++ b/public/app/features/api-keys/ApiKeysPage.test.tsx @@ -25,9 +25,13 @@ jest.mock('app/core/core', () => { const setup = (propOverrides: Partial) => { const loadApiKeysMock = jest.fn(); const deleteApiKeyMock = jest.fn(); + const migrateApiKeyMock = jest.fn(); const addApiKeyMock = jest.fn(); + const migrateAllMock = jest.fn(); const toggleIncludeExpiredMock = jest.fn(); const setSearchQueryMock = mockToolkitActionCreator(setSearchQuery); + const getApiKeysMigrationStatusMock = jest.fn(); + const hideApiKeysMock = jest.fn(); const props: Props = { navModel: { main: { @@ -44,12 +48,17 @@ const setup = (propOverrides: Partial) => { deleteApiKey: deleteApiKeyMock, setSearchQuery: setSearchQueryMock, addApiKey: addApiKeyMock, + getApiKeysMigrationStatus: getApiKeysMigrationStatusMock, + migrateApiKey: migrateApiKeyMock, + migrateAll: migrateAllMock, + hideApiKeys: hideApiKeysMock, apiKeysCount: 0, timeZone: 'utc', includeExpired: false, includeExpiredDisabled: false, toggleIncludeExpired: toggleIncludeExpiredMock, canCreate: true, + apiKeysMigrated: false, }; Object.assign(props, propOverrides); diff --git a/public/app/features/api-keys/ApiKeysPage.tsx b/public/app/features/api-keys/ApiKeysPage.tsx index 2febce819d4..9df08efd22e 100644 --- a/public/app/features/api-keys/ApiKeysPage.tsx +++ b/public/app/features/api-keys/ApiKeysPage.tsx @@ -14,12 +14,23 @@ import { getTimeZone } from 'app/features/profile/state/selectors'; import { AccessControlAction, ApiKey, NewApiKey, StoreState } from 'app/types'; import { ShowModalReactEvent } from 'app/types/events'; +import { APIKeysMigratedCard } from './APIKeysMigratedCard'; import { ApiKeysActionBar } from './ApiKeysActionBar'; import { ApiKeysAddedModal } from './ApiKeysAddedModal'; import { ApiKeysController } from './ApiKeysController'; import { ApiKeysForm } from './ApiKeysForm'; import { ApiKeysTable } from './ApiKeysTable'; -import { addApiKey, deleteApiKey, loadApiKeys, toggleIncludeExpired } from './state/actions'; +import { MigrateToServiceAccountsCard } from './MigrateToServiceAccountsCard'; +import { + addApiKey, + deleteApiKey, + migrateApiKey, + migrateAll, + loadApiKeys, + toggleIncludeExpired, + getApiKeysMigrationStatus, + hideApiKeys, +} from './state/actions'; import { setSearchQuery } from './state/reducers'; import { getApiKeys, getApiKeysCount, getIncludeExpired, getIncludeExpiredDisabled } from './state/selectors'; @@ -36,15 +47,20 @@ function mapStateToProps(state: StoreState) { includeExpired: getIncludeExpired(state.apiKeys), includeExpiredDisabled: getIncludeExpiredDisabled(state.apiKeys), canCreate: canCreate, + apiKeysMigrated: state.apiKeys.apiKeysMigrated, }; } const mapDispatchToProps = { loadApiKeys, deleteApiKey, + migrateApiKey, + migrateAll, setSearchQuery, toggleIncludeExpired, addApiKey, + getApiKeysMigrationStatus, + hideApiKeys, }; const connector = connect(mapStateToProps, mapDispatchToProps); @@ -64,6 +80,7 @@ export class ApiKeysPageUnconnected extends PureComponent { componentDidMount() { this.fetchApiKeys(); + this.props.getApiKeysMigrationStatus(); } async fetchApiKeys() { @@ -74,6 +91,14 @@ export class ApiKeysPageUnconnected extends PureComponent { this.props.deleteApiKey(key.id!); }; + onMigrateAll = () => { + this.props.migrateAll(); + }; + + onMigrateApiKey = (key: ApiKey) => { + this.props.migrateApiKey(key.id!); + }; + onSearchQueryChange = (value: string) => { this.props.setSearchQuery(value); }; @@ -116,6 +141,11 @@ export class ApiKeysPageUnconnected extends PureComponent { } }; + onHideApiKeys = async () => { + await this.props.hideApiKeys(); + window.location.reload(); + }; + render() { const { hasFetched, @@ -127,6 +157,7 @@ export class ApiKeysPageUnconnected extends PureComponent { includeExpired, includeExpiredDisabled, canCreate, + apiKeysMigrated, } = this.props; if (!hasFetched) { @@ -142,18 +173,17 @@ export class ApiKeysPageUnconnected extends PureComponent { {({ isAdding, toggleIsAdding }) => { - const showCTA = !isAdding && apiKeysCount === 0; + const showCTA = !isAdding && apiKeysCount === 0 && !apiKeysMigrated; const showTable = apiKeysCount > 0; return ( <> - {/* TODO: enable when API keys to service accounts migration is ready - {config.featureToggles.serviceAccounts && ( - - Service accounts give you more control. API keys will be automatically migrated into tokens inside - respective service accounts. The current API keys will still work, but will be called tokens and - you will find them in the detail view of a respective service account. - - )} */} + {/* TODO: remove feature flag check before GA */} + {config.featureToggles.serviceAccounts && !apiKeysMigrated && ( + + )} + {config.featureToggles.serviceAccounts && apiKeysMigrated && ( + + )} {showCTA ? ( { - + ) : null} diff --git a/public/app/features/api-keys/ApiKeysTable.tsx b/public/app/features/api-keys/ApiKeysTable.tsx index 15502aea0a6..f15391ae5f9 100644 --- a/public/app/features/api-keys/ApiKeysTable.tsx +++ b/public/app/features/api-keys/ApiKeysTable.tsx @@ -2,7 +2,8 @@ import { css } from '@emotion/css'; import React, { FC } from 'react'; import { dateTimeFormat, GrafanaTheme2, TimeZone } from '@grafana/data'; -import { DeleteButton, Icon, IconName, Tooltip, useTheme2 } from '@grafana/ui'; +import { config } from '@grafana/runtime'; +import { Button, DeleteButton, HorizontalGroup, Icon, IconName, Tooltip, useTheme2 } from '@grafana/ui'; import { contextSrv } from 'app/core/core'; import { AccessControlAction } from 'app/types'; @@ -12,9 +13,10 @@ interface Props { apiKeys: ApiKey[]; timeZone: TimeZone; onDelete: (apiKey: ApiKey) => void; + onMigrate: (apiKey: ApiKey) => void; } -export const ApiKeysTable: FC = ({ apiKeys, timeZone, onDelete }) => { +export const ApiKeysTable: FC = ({ apiKeys, timeZone, onDelete, onMigrate }) => { const theme = useTheme2(); const styles = getStyles(theme); @@ -47,12 +49,19 @@ export const ApiKeysTable: FC = ({ apiKeys, timeZone, onDelete }) => { )} - onDelete(key)} - disabled={!contextSrv.hasPermissionInMetadata(AccessControlAction.ActionAPIKeysDelete, key)} - /> + + {config.featureToggles.serviceAccounts && ( + + )} + onDelete(key)} + disabled={!contextSrv.hasPermissionInMetadata(AccessControlAction.ActionAPIKeysDelete, key)} + /> + ); diff --git a/public/app/features/api-keys/MigrateToServiceAccountsCard.tsx b/public/app/features/api-keys/MigrateToServiceAccountsCard.tsx new file mode 100644 index 00000000000..002e78f76e7 --- /dev/null +++ b/public/app/features/api-keys/MigrateToServiceAccountsCard.tsx @@ -0,0 +1,45 @@ +import { css } from '@emotion/css'; +import React from 'react'; + +import { GrafanaTheme2 } from '@grafana/data'; +import { Alert, Button, useStyles2 } from '@grafana/ui'; + +interface Props { + onMigrate: () => void; + disabled?: boolean; +} + +export const MigrateToServiceAccountsCard = ({ onMigrate, disabled }: Props): JSX.Element => { + const styles = useStyles2(getStyles); + + return ( + +
+ Service accounts give you more control. API keys will be automatically migrated into tokens inside respective + service accounts. The current API keys will still work, but will be called tokens and you will find them in the + detail view of a respective service account. +
+
+ {!disabled && ( + + )} + Read more about Service accounts and how to turn them on +
+ + ); +}; + +export const getStyles = (theme: GrafanaTheme2) => ({ + text: css` + margin-bottom: ${theme.spacing(2)}; + `, + actionRow: css` + display: flex; + align-items: center; + `, + actionButton: css` + margin-right: ${theme.spacing(2)}; + `, +}); diff --git a/public/app/features/api-keys/state/actions.ts b/public/app/features/api-keys/state/actions.ts index 1abb5355ab4..ca9da51bee7 100644 --- a/public/app/features/api-keys/state/actions.ts +++ b/public/app/features/api-keys/state/actions.ts @@ -1,7 +1,14 @@ -import { getBackendSrv } from 'app/core/services/backend_srv'; +import { config } from '@grafana/runtime'; +import { getBackendSrv } from 'app/core/services/backend_srv'; import { ApiKey, ThunkResult } from 'app/types'; -import { apiKeysLoaded, includeExpiredToggled, isFetching, setSearchQuery } from './reducers'; +import { + apiKeysLoaded, + includeExpiredToggled, + isFetching, + apiKeysMigrationStatusLoaded, + setSearchQuery, +} from './reducers'; export function addApiKey(apiKey: ApiKey, openModal: (key: string) => void): ThunkResult { return async (dispatch) => { @@ -31,6 +38,43 @@ export function deleteApiKey(id: number): ThunkResult { }; } +export function migrateApiKey(id: number): ThunkResult { + return async (dispatch) => { + try { + await getBackendSrv().post(`/api/serviceaccounts/migrate/${id}`); + } finally { + dispatch(loadApiKeys()); + } + }; +} + +export function migrateAll(): ThunkResult { + return async (dispatch) => { + try { + await getBackendSrv().post('/api/serviceaccounts/migrate'); + } finally { + dispatch(getApiKeysMigrationStatus()); + dispatch(loadApiKeys()); + } + }; +} + +export function getApiKeysMigrationStatus(): ThunkResult { + return async (dispatch) => { + // TODO: remove when service account enabled by default (or use another way to detect if it's enabled) + if (config.featureToggles.serviceAccounts) { + const result = await getBackendSrv().get('/api/serviceaccounts/migrationstatus'); + dispatch(apiKeysMigrationStatusLoaded(!!result?.migrated)); + } + }; +} + +export function hideApiKeys(): ThunkResult { + return async (dispatch) => { + await getBackendSrv().post('/api/serviceaccounts/hideApiKeys'); + }; +} + export function toggleIncludeExpired(): ThunkResult { return (dispatch) => { dispatch(includeExpiredToggled()); diff --git a/public/app/features/api-keys/state/reducers.ts b/public/app/features/api-keys/state/reducers.ts index 327d18d3abc..4e51b74ba56 100644 --- a/public/app/features/api-keys/state/reducers.ts +++ b/public/app/features/api-keys/state/reducers.ts @@ -8,6 +8,7 @@ export const initialApiKeysState: ApiKeysState = { keys: [], keysIncludingExpired: [], searchQuery: '', + apiKeysMigrated: false, }; const apiKeysSlice = createSlice({ @@ -22,6 +23,9 @@ const apiKeysSlice = createSlice({ : state.includeExpired; return { ...state, hasFetched: true, keys, keysIncludingExpired, includeExpired }; }, + apiKeysMigrationStatusLoaded: (state, action): ApiKeysState => { + return { ...state, apiKeysMigrated: action.payload }; + }, setSearchQuery: (state, action): ApiKeysState => { return { ...state, searchQuery: action.payload }; }, @@ -34,7 +38,8 @@ const apiKeysSlice = createSlice({ }, }); -export const { apiKeysLoaded, includeExpiredToggled, isFetching, setSearchQuery } = apiKeysSlice.actions; +export const { apiKeysLoaded, includeExpiredToggled, isFetching, setSearchQuery, apiKeysMigrationStatusLoaded } = + apiKeysSlice.actions; export const apiKeysReducer = apiKeysSlice.reducer; diff --git a/public/app/features/api-keys/state/selectors.test.ts b/public/app/features/api-keys/state/selectors.test.ts index e1ba2f4d0f9..d96c98f083c 100644 --- a/public/app/features/api-keys/state/selectors.test.ts +++ b/public/app/features/api-keys/state/selectors.test.ts @@ -16,6 +16,7 @@ describe('API Keys selectors', () => { searchQuery: '', hasFetched: true, includeExpired: false, + apiKeysMigrated: false, }; const keyCount = getApiKeysCount(mockState); expect(keyCount).toBe(5); @@ -28,6 +29,7 @@ describe('API Keys selectors', () => { searchQuery: '', hasFetched: true, includeExpired: true, + apiKeysMigrated: false, }; const keyCount = getApiKeysCount(mockState); expect(keyCount).toBe(8); @@ -43,6 +45,7 @@ describe('API Keys selectors', () => { searchQuery: '', hasFetched: true, includeExpired: false, + apiKeysMigrated: false, }; const keys = getApiKeys(mockState); expect(keys).toEqual(mockKeys); @@ -55,6 +58,7 @@ describe('API Keys selectors', () => { searchQuery: '5', hasFetched: true, includeExpired: false, + apiKeysMigrated: false, }; const keys = getApiKeys(mockState); expect(keys.length).toEqual(1); @@ -69,6 +73,7 @@ describe('API Keys selectors', () => { searchQuery: '', hasFetched: true, includeExpired: true, + apiKeysMigrated: false, }; const keys = getApiKeys(mockState); expect(keys).toEqual(mockKeysIncludingExpired); @@ -81,6 +86,7 @@ describe('API Keys selectors', () => { searchQuery: '5', hasFetched: true, includeExpired: true, + apiKeysMigrated: false, }; const keys = getApiKeys(mockState); expect(keys.length).toEqual(1); @@ -96,6 +102,7 @@ describe('API Keys selectors', () => { searchQuery: '', hasFetched: true, includeExpired: true, + apiKeysMigrated: false, }; const includeExpired = getIncludeExpired(mockState); expect(includeExpired).toBe(true); @@ -108,6 +115,7 @@ describe('API Keys selectors', () => { searchQuery: '', hasFetched: true, includeExpired: false, + apiKeysMigrated: false, }; const includeExpired = getIncludeExpired(mockState); expect(includeExpired).toBe(false); @@ -122,6 +130,7 @@ describe('API Keys selectors', () => { searchQuery: '', hasFetched: true, includeExpired: true, + apiKeysMigrated: false, }; const includeExpiredDisabled = getIncludeExpiredDisabled(mockState); expect(includeExpiredDisabled).toBe(true); @@ -134,6 +143,7 @@ describe('API Keys selectors', () => { searchQuery: '', hasFetched: true, includeExpired: false, + apiKeysMigrated: false, }; const includeExpiredDisabled = getIncludeExpired(mockState); expect(includeExpiredDisabled).toBe(false); diff --git a/public/app/features/serviceaccounts/ServiceAccountsListPage.test.tsx b/public/app/features/serviceaccounts/ServiceAccountsListPage.test.tsx index 48dedee0a4b..d40a59b4071 100644 --- a/public/app/features/serviceaccounts/ServiceAccountsListPage.test.tsx +++ b/public/app/features/serviceaccounts/ServiceAccountsListPage.test.tsx @@ -22,6 +22,7 @@ const setup = (propOverrides: Partial) => { const updateServiceAccountMock = jest.fn(); const changeStateFilterMock = jest.fn(); const createServiceAccountTokenMock = jest.fn(); + const getApiKeysMigrationStatusMock = jest.fn(); const props: Props = { navModel: { main: { @@ -41,6 +42,7 @@ const setup = (propOverrides: Partial) => { showPaging: false, totalPages: 1, serviceAccounts: [], + apiKeysMigrated: false, changeQuery: changeQueryMock, fetchACOptions: fetchACOptionsMock, fetchServiceAccounts: fetchServiceAccountsMock, @@ -48,6 +50,7 @@ const setup = (propOverrides: Partial) => { updateServiceAccount: updateServiceAccountMock, changeStateFilter: changeStateFilterMock, createServiceAccountToken: createServiceAccountTokenMock, + getApiKeysMigrationStatus: getApiKeysMigrationStatusMock, }; Object.assign(props, propOverrides); diff --git a/public/app/features/serviceaccounts/ServiceAccountsListPage.tsx b/public/app/features/serviceaccounts/ServiceAccountsListPage.tsx index e760f7016b3..070dc7a9fd8 100644 --- a/public/app/features/serviceaccounts/ServiceAccountsListPage.tsx +++ b/public/app/features/serviceaccounts/ServiceAccountsListPage.tsx @@ -4,7 +4,7 @@ import React, { useEffect, useState } from 'react'; import { connect, ConnectedProps } from 'react-redux'; import { GrafanaTheme2, OrgRole } from '@grafana/data'; -import { ConfirmModal, FilterInput, Icon, LinkButton, RadioButtonGroup, Tooltip, useStyles2 } from '@grafana/ui'; +import { Alert, ConfirmModal, FilterInput, Icon, LinkButton, RadioButtonGroup, Tooltip, useStyles2 } from '@grafana/ui'; import EmptyListCTA from 'app/core/components/EmptyListCTA/EmptyListCTA'; import Page from 'app/core/components/Page/Page'; import PageLoader from 'app/core/components/PageLoader/PageLoader'; @@ -22,6 +22,7 @@ import { updateServiceAccount, changeStateFilter, createServiceAccountToken, + getApiKeysMigrationStatus, } from './state/actions'; interface OwnProps {} @@ -43,6 +44,7 @@ const mapDispatchToProps = { updateServiceAccount, changeStateFilter, createServiceAccountToken, + getApiKeysMigrationStatus, }; const connector = connect(mapStateToProps, mapDispatchToProps); @@ -55,6 +57,7 @@ export const ServiceAccountsListPageUnconnected = ({ builtInRoles, query, serviceAccountStateFilter, + apiKeysMigrated, changeQuery, fetchACOptions, fetchServiceAccounts, @@ -62,6 +65,7 @@ export const ServiceAccountsListPageUnconnected = ({ updateServiceAccount, changeStateFilter, createServiceAccountToken, + getApiKeysMigrationStatus, }: Props): JSX.Element => { const styles = useStyles2(getStyles); const [isAddModalOpen, setIsAddModalOpen] = useState(false); @@ -72,10 +76,11 @@ export const ServiceAccountsListPageUnconnected = ({ useEffect(() => { fetchServiceAccounts({ withLoadingIndicator: true }); + getApiKeysMigrationStatus(); if (contextSrv.licensedAccessControlEnabled()) { fetchACOptions(); } - }, [fetchACOptions, fetchServiceAccounts]); + }, [fetchACOptions, fetchServiceAccounts, getApiKeysMigrationStatus]); const noServiceAccountsCreated = serviceAccounts.length === 0 && serviceAccountStateFilter === ServiceAccountStateFilter.All && !query; @@ -151,9 +156,21 @@ export const ServiceAccountsListPageUnconnected = ({ setCurrentServiceAccount(null); }; + const onMigrationInfoClose = () => { + // TODO: dismiss banner permanently + }; + return ( + {apiKeysMigrated && ( + + )}

Service accounts

diff --git a/public/app/features/serviceaccounts/state/actions.ts b/public/app/features/serviceaccounts/state/actions.ts index 2448de4b901..aeebfa4e6ac 100644 --- a/public/app/features/serviceaccounts/state/actions.ts +++ b/public/app/features/serviceaccounts/state/actions.ts @@ -15,6 +15,7 @@ import { serviceAccountsFetchBegin, serviceAccountsFetched, serviceAccountsFetchEnd, + apiKeysMigrationStatusLoaded, stateFilterChanged, } from './reducers'; @@ -41,6 +42,13 @@ export function fetchACOptions(): ThunkResult { }; } +export function getApiKeysMigrationStatus(): ThunkResult { + return async (dispatch) => { + const result = await getBackendSrv().get('/api/serviceaccounts/migrationstatus'); + dispatch(apiKeysMigrationStatusLoaded(!!result?.migrated)); + }; +} + interface FetchServiceAccountsParams { withLoadingIndicator: boolean; } diff --git a/public/app/features/serviceaccounts/state/reducers.ts b/public/app/features/serviceaccounts/state/reducers.ts index fc672060286..571ec2b40d7 100644 --- a/public/app/features/serviceaccounts/state/reducers.ts +++ b/public/app/features/serviceaccounts/state/reducers.ts @@ -51,6 +51,7 @@ export const initialStateList: ServiceAccountsState = { totalPages: 1, showPaging: false, serviceAccountStateFilter: ServiceAccountStateFilter.All, + apiKeysMigrated: false, }; interface ServiceAccountsFetched { @@ -89,6 +90,9 @@ const serviceAccountsSlice = createSlice({ builtInRolesLoaded: (state, action: PayloadAction>): ServiceAccountsState => { return { ...state, builtInRoles: action.payload }; }, + apiKeysMigrationStatusLoaded: (state, action): ServiceAccountsState => { + return { ...state, apiKeysMigrated: action.payload }; + }, queryChanged: (state, action: PayloadAction) => { return { ...state, @@ -114,6 +118,7 @@ export const { serviceAccountsFetched, acOptionsLoaded, builtInRolesLoaded, + apiKeysMigrationStatusLoaded, pageChanged, stateFilterChanged, queryChanged, diff --git a/public/app/types/apiKeys.ts b/public/app/types/apiKeys.ts index e024de1da1e..727d42783dc 100644 --- a/public/app/types/apiKeys.ts +++ b/public/app/types/apiKeys.ts @@ -25,4 +25,5 @@ export interface ApiKeysState { keysIncludingExpired: ApiKey[]; searchQuery: string; hasFetched: boolean; + apiKeysMigrated: boolean; } diff --git a/public/app/types/serviceaccount.ts b/public/app/types/serviceaccount.ts index ba0989251fa..836bdaaf8f1 100644 --- a/public/app/types/serviceaccount.ts +++ b/public/app/types/serviceaccount.ts @@ -66,6 +66,7 @@ export interface ServiceAccountsState { isLoading: boolean; roleOptions: Role[]; builtInRoles: Record; + apiKeysMigrated: boolean; // search / filtering query: string; @@ -75,3 +76,7 @@ export interface ServiceAccountsState { showPaging: boolean; serviceAccountStateFilter: ServiceAccountStateFilter; } + +export interface ServiceAccountsUpgradeStatus { + upgraded: boolean; +}