diff --git a/pkg/cmd/grafana-cli/runner/wire.go b/pkg/cmd/grafana-cli/runner/wire.go index ba30f24548c..f4493320527 100644 --- a/pkg/cmd/grafana-cli/runner/wire.go +++ b/pkg/cmd/grafana-cli/runner/wire.go @@ -101,7 +101,6 @@ import ( secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" secretsMigrator "github.com/grafana/grafana/pkg/services/secrets/migrator" "github.com/grafana/grafana/pkg/services/serviceaccounts" - "github.com/grafana/grafana/pkg/services/serviceaccounts/database" serviceaccountsmanager "github.com/grafana/grafana/pkg/services/serviceaccounts/manager" "github.com/grafana/grafana/pkg/services/shorturls" "github.com/grafana/grafana/pkg/services/sqlstore" @@ -243,8 +242,6 @@ var wireSet = wire.NewSet( pluginSettings.ProvideService, wire.Bind(new(pluginsettings.Service), new(*pluginSettings.Service)), alerting.ProvideService, - database.ProvideServiceAccountsStore, - wire.Bind(new(serviceaccounts.Store), new(*database.ServiceAccountsStoreImpl)), ossaccesscontrol.ProvideServiceAccountPermissions, wire.Bind(new(accesscontrol.ServiceAccountPermissionsService), new(*ossaccesscontrol.ServiceAccountPermissionsService)), serviceaccountsmanager.ProvideServiceAccountsService, diff --git a/pkg/server/wire.go b/pkg/server/wire.go index 7d0d0faf912..193bd990e12 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -111,8 +111,8 @@ import ( secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" secretsMigrator "github.com/grafana/grafana/pkg/services/secrets/migrator" "github.com/grafana/grafana/pkg/services/serviceaccounts" - "github.com/grafana/grafana/pkg/services/serviceaccounts/database" serviceaccountsmanager "github.com/grafana/grafana/pkg/services/serviceaccounts/manager" + serviceaccountsretriever "github.com/grafana/grafana/pkg/services/serviceaccounts/retriever" "github.com/grafana/grafana/pkg/services/shorturls" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" @@ -271,8 +271,8 @@ var wireBasicSet = wire.NewSet( pluginSettings.ProvideService, wire.Bind(new(pluginsettings.Service), new(*pluginSettings.Service)), alerting.ProvideService, - database.ProvideServiceAccountsStore, - wire.Bind(new(serviceaccounts.Store), new(*database.ServiceAccountsStoreImpl)), + serviceaccountsretriever.ProvideService, + wire.Bind(new(serviceaccountsretriever.ServiceAccountRetriever), new(*serviceaccountsretriever.Service)), ossaccesscontrol.ProvideServiceAccountPermissions, wire.Bind(new(accesscontrol.ServiceAccountPermissionsService), new(*ossaccesscontrol.ServiceAccountPermissionsService)), serviceaccountsmanager.ProvideServiceAccountsService, diff --git a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go index d88f49c4779..20a4c94723b 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go @@ -13,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/serviceaccounts" + "github.com/grafana/grafana/pkg/services/serviceaccounts/retriever" "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/services/team/teamimpl" "github.com/grafana/grafana/pkg/services/user" @@ -283,7 +284,7 @@ type ServiceAccountPermissionsService struct { func ProvideServiceAccountPermissions( cfg *setting.Cfg, router routing.RouteRegister, sql db.DB, ac accesscontrol.AccessControl, - license models.Licensing, serviceAccountStore serviceaccounts.Store, service accesscontrol.Service, + license models.Licensing, serviceAccountRetrieverService *retriever.Service, service accesscontrol.Service, teamService team.Service, userService user.Service, ) (*ServiceAccountPermissionsService, error) { options := resourcepermissions.Options{ @@ -294,7 +295,7 @@ func ProvideServiceAccountPermissions( if err != nil { return err } - _, err = serviceAccountStore.RetrieveServiceAccount(ctx, orgID, id) + _, err = serviceAccountRetrieverService.RetrieveServiceAccount(ctx, orgID, id) return err }, Assignments: resourcepermissions.Assignments{ diff --git a/pkg/services/serviceaccounts/api/api.go b/pkg/services/serviceaccounts/api/api.go index 549e5001d6c..fc20c400a3a 100644 --- a/pkg/services/serviceaccounts/api/api.go +++ b/pkg/services/serviceaccounts/api/api.go @@ -1,6 +1,7 @@ package api import ( + "context" "errors" "net/http" "strconv" @@ -12,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/middleware" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/apikey" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/serviceaccounts/database" @@ -22,22 +24,39 @@ import ( type ServiceAccountsAPI struct { cfg *setting.Cfg - service serviceaccounts.Service + service service accesscontrol accesscontrol.AccessControl accesscontrolService accesscontrol.Service RouterRegister routing.RouteRegister - store serviceaccounts.Store log log.Logger permissionService accesscontrol.ServiceAccountPermissionsService } +// Service implements the API exposed methods for service accounts. +type service interface { + CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) + RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error) + UpdateServiceAccount(ctx context.Context, orgID, serviceAccountID int64, + saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) + 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 + RevertApiKey(ctx context.Context, saId int64, keyId int64) error + // Service account tokens + AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error + DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error +} + func NewServiceAccountsAPI( cfg *setting.Cfg, - service serviceaccounts.Service, + service service, accesscontrol accesscontrol.AccessControl, accesscontrolService accesscontrol.Service, routerRegister routing.RouteRegister, - store serviceaccounts.Store, permissionService accesscontrol.ServiceAccountPermissionsService, ) *ServiceAccountsAPI { return &ServiceAccountsAPI{ @@ -46,7 +65,6 @@ func NewServiceAccountsAPI( accesscontrol: accesscontrol, accesscontrolService: accesscontrolService, RouterRegister: routerRegister, - store: store, log: log.New("serviceaccounts.api"), permissionService: permissionService, } @@ -116,7 +134,7 @@ func (api *ServiceAccountsAPI) CreateServiceAccount(c *models.ReqContext) respon } } - serviceAccount, err := api.store.CreateServiceAccount(c.Req.Context(), c.OrgID, &cmd) + serviceAccount, err := api.service.CreateServiceAccount(c.Req.Context(), c.OrgID, &cmd) switch { case errors.Is(err, database.ErrServiceAccountAlreadyExists): return response.Error(http.StatusBadRequest, "Failed to create service account", err) @@ -159,7 +177,7 @@ func (api *ServiceAccountsAPI) RetrieveServiceAccount(ctx *models.ReqContext) re return response.Error(http.StatusBadRequest, "Service Account ID is invalid", err) } - serviceAccount, err := api.store.RetrieveServiceAccount(ctx.Req.Context(), ctx.OrgID, scopeID) + serviceAccount, err := api.service.RetrieveServiceAccount(ctx.Req.Context(), ctx.OrgID, scopeID) if err != nil { switch { case errors.Is(err, serviceaccounts.ErrServiceAccountNotFound): @@ -174,7 +192,7 @@ func (api *ServiceAccountsAPI) RetrieveServiceAccount(ctx *models.ReqContext) re serviceAccount.AvatarUrl = dtos.GetGravatarUrlWithDefault("", serviceAccount.Name) serviceAccount.AccessControl = metadata[saIDString] - tokens, err := api.store.ListTokens(ctx.Req.Context(), &serviceaccounts.GetSATokensQuery{ + tokens, err := api.service.ListTokens(ctx.Req.Context(), &serviceaccounts.GetSATokensQuery{ OrgID: &serviceAccount.OrgId, ServiceAccountID: &serviceAccount.Id, }) @@ -222,7 +240,7 @@ func (api *ServiceAccountsAPI) UpdateServiceAccount(c *models.ReqContext) respon } } - resp, err := api.store.UpdateServiceAccount(c.Req.Context(), c.OrgID, scopeID, &cmd) + resp, err := api.service.UpdateServiceAccount(c.Req.Context(), c.OrgID, scopeID, &cmd) if err != nil { switch { case errors.Is(err, serviceaccounts.ErrServiceAccountNotFound): @@ -312,7 +330,15 @@ func (api *ServiceAccountsAPI) SearchOrgServiceAccountsWithPaging(c *models.ReqC if onlyDisabled { filter = serviceaccounts.FilterOnlyDisabled } - serviceAccountSearch, err := api.store.SearchOrgServiceAccounts(ctx, c.OrgID, c.Query("query"), filter, page, perPage, c.SignedInUser) + q := serviceaccounts.SearchOrgServiceAccountsQuery{ + OrgID: c.OrgID, + Query: c.Query("query"), + Page: page, + Limit: perPage, + Filter: filter, + SignedInUser: c.SignedInUser, + } + serviceAccountSearch, err := api.service.SearchOrgServiceAccounts(ctx, &q) if err != nil { return response.Error(http.StatusInternalServerError, "Failed to get service accounts for current organization", err) } @@ -326,7 +352,7 @@ func (api *ServiceAccountsAPI) SearchOrgServiceAccountsWithPaging(c *models.ReqC saIDs[saIDString] = true metadata := api.getAccessControlMetadata(c, map[string]bool{saIDString: true}) sa.AccessControl = metadata[strconv.FormatInt(sa.Id, 10)] - tokens, err := api.store.ListTokens(ctx, &serviceaccounts.GetSATokensQuery{ + tokens, err := api.service.ListTokens(ctx, &serviceaccounts.GetSATokensQuery{ OrgID: &sa.OrgId, ServiceAccountID: &sa.Id, }) if err != nil { @@ -340,7 +366,7 @@ func (api *ServiceAccountsAPI) SearchOrgServiceAccountsWithPaging(c *models.ReqC // GET /api/serviceaccounts/migrationstatus func (api *ServiceAccountsAPI) GetAPIKeysMigrationStatus(ctx *models.ReqContext) response.Response { - upgradeStatus, err := api.store.GetAPIKeysMigrationStatus(ctx.Req.Context(), ctx.OrgID) + upgradeStatus, err := api.service.GetAPIKeysMigrationStatus(ctx.Req.Context(), ctx.OrgID) if err != nil { return response.Error(http.StatusInternalServerError, "Internal server error", err) } @@ -349,7 +375,7 @@ func (api *ServiceAccountsAPI) GetAPIKeysMigrationStatus(ctx *models.ReqContext) // 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 { + if err := api.service.HideApiKeysTab(ctx.Req.Context(), ctx.OrgID); err != nil { return response.Error(http.StatusInternalServerError, "Internal server error", err) } return response.Success("API keys hidden") @@ -357,7 +383,7 @@ func (api *ServiceAccountsAPI) HideApiKeysTab(ctx *models.ReqContext) response.R // 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 { + if err := api.service.MigrateApiKeysToServiceAccounts(ctx.Req.Context(), ctx.OrgID); err != nil { return response.Error(http.StatusInternalServerError, "Internal server error", err) } @@ -371,7 +397,7 @@ func (api *ServiceAccountsAPI) ConvertToServiceAccount(ctx *models.ReqContext) r return response.Error(http.StatusBadRequest, "Key ID is invalid", err) } - if err := api.store.MigrateApiKey(ctx.Req.Context(), ctx.OrgID, keyId); err != nil { + if err := api.service.MigrateApiKey(ctx.Req.Context(), ctx.OrgID, keyId); err != nil { return response.Error(http.StatusInternalServerError, "Error converting API key", err) } @@ -389,7 +415,7 @@ func (api *ServiceAccountsAPI) RevertApiKey(ctx *models.ReqContext) response.Res return response.Error(http.StatusBadRequest, "service account ID is invalid", err) } - if err := api.store.RevertApiKey(ctx.Req.Context(), serviceAccountId, keyId); err != nil { + if err := api.service.RevertApiKey(ctx.Req.Context(), serviceAccountId, keyId); err != nil { return response.Error(http.StatusInternalServerError, "error reverting to API key", err) } return response.Success("reverted service account to API key") @@ -464,7 +490,7 @@ type DeleteServiceAccountParams struct { // swagger:response searchOrgServiceAccountsWithPagingResponse type SearchOrgServiceAccountsWithPagingResponse struct { // in:body - Body *serviceaccounts.SearchServiceAccountsResult + Body *serviceaccounts.SearchOrgServiceAccountsResult } // swagger:response createServiceAccountResponse diff --git a/pkg/services/serviceaccounts/api/api_test.go b/pkg/services/serviceaccounts/api/api_test.go index 54f8ac7a0ef..2595c81594f 100644 --- a/pkg/services/serviceaccounts/api/api_test.go +++ b/pkg/services/serviceaccounts/api/api_test.go @@ -32,6 +32,7 @@ import ( "github.com/grafana/grafana/pkg/services/quota/quotatest" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/serviceaccounts/database" + "github.com/grafana/grafana/pkg/services/serviceaccounts/retriever" "github.com/grafana/grafana/pkg/services/serviceaccounts/tests" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/team/teamimpl" @@ -46,6 +47,13 @@ var ( serviceAccountIDPath = serviceAccountPath + "%v" ) +// TODO: +// refactor this set of tests to make use of fakes for the ServiceAccountService +// all of the API tests are calling with all of the db store injections +// which is not ideal +// this is a bit of a hack to get the tests to pass until we refactor the tests +// to use fakes as in the user service tests + func TestServiceAccountsAPI_CreateServiceAccount(t *testing.T) { store := db.InitTestDB(t) services := setupTestServices(t, store) @@ -162,7 +170,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, usr *tests.TestUser) { - server, api := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store, services.SAStore) + server, api := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store) marshalled, err := json.Marshal(tc.body) require.NoError(t, err) @@ -239,7 +247,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, &services.SAService, routing.NewRouteRegister(), testcase.acmock, store, services.SAStore) + server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), testcase.acmock, store) actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, fmt.Sprint(createduser.ID))).Code require.Equal(t, testcase.expectedCode, actual) }) @@ -263,7 +271,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, &services.SAService, routing.NewRouteRegister(), testcase.acmock, store, services.SAStore) + server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), testcase.acmock, store) actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, createduser.ID)).Code require.Equal(t, testcase.expectedCode, actual) }) @@ -275,21 +283,29 @@ func serviceAccountRequestScenario(t *testing.T, httpMethod string, endpoint str fn(httpMethod, endpoint, user) } -func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock, +func setupTestServer( + t *testing.T, + svc *tests.ServiceAccountMock, routerRegister routing.RouteRegister, acmock *accesscontrolmock.Mock, - sqlStore db.DB, saStore serviceaccounts.Store) (*web.Mux, *ServiceAccountsAPI) { + sqlStore db.DB, +) (*web.Mux, *ServiceAccountsAPI) { cfg := setting.NewCfg() teamSvc := teamimpl.ProvideService(sqlStore, cfg) - - userSvc, err := userimpl.ProvideService(sqlStore, nil, cfg, teamimpl.ProvideService(sqlStore, cfg), nil, quotatest.New(false, nil)) + orgSvc, err := orgimpl.ProvideService(sqlStore, cfg, quotatest.New(false, nil)) require.NoError(t, err) + + userSvc, err := userimpl.ProvideService(sqlStore, orgSvc, cfg, teamimpl.ProvideService(sqlStore, cfg), nil, quotatest.New(false, nil)) + require.NoError(t, err) + + // TODO: create fake for retriever to pass into the permissionservice + retrieverSvc := retriever.ProvideService(sqlStore, nil, nil, nil, nil) saPermissionService, err := ossaccesscontrol.ProvideServiceAccountPermissions( - cfg, routing.NewRouteRegister(), sqlStore, acmock, &licensing.OSSLicensingService{}, saStore, acmock, teamSvc, userSvc) + cfg, routing.NewRouteRegister(), sqlStore, acmock, &licensing.OSSLicensingService{}, retrieverSvc, acmock, teamSvc, userSvc) require.NoError(t, err) acService := actest.FakeService{} - a := NewServiceAccountsAPI(cfg, svc, acmock, acService, routerRegister, saStore, saPermissionService) + a := NewServiceAccountsAPI(cfg, svc, acmock, acService, routerRegister, saPermissionService) a.RegisterAPIEndpoints() a.cfg.ApiKeyMaxSecondsToLive = -1 // disable api key expiration @@ -317,6 +333,7 @@ func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock, func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) { store := db.InitTestDB(t) services := setupTestServices(t, store) + type testRetrieveSATestCase struct { desc string user *tests.TestUser @@ -380,7 +397,7 @@ func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) { createdUser := tests.SetupUserServiceAccount(t, store, *tc.user) scopeID = int(createdUser.ID) } - server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store, services.SAStore) + server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store) actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, scopeID)) @@ -406,6 +423,7 @@ func newString(s string) *string { func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) { store := db.InitTestDB(t) services := setupTestServices(t, store) + type testUpdateSATestCase struct { desc string user *tests.TestUser @@ -498,7 +516,7 @@ func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - server, saAPI := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store, services.SAStore) + server, saAPI := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store) scopeID := tc.Id if tc.user != nil { createdUser := tests.SetupUserServiceAccount(t, store, *tc.user) @@ -528,7 +546,7 @@ func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) { assert.Equal(t, tc.user.Login, serviceAccountData["login"].(string)) // Ensure the user was updated in DB - sa, err := saAPI.store.RetrieveServiceAccount(context.Background(), 1, int64(scopeID)) + sa, err := saAPI.service.RetrieveServiceAccount(context.Background(), 1, int64(scopeID)) require.NoError(t, err) require.Equal(t, *tc.body.Name, sa.Name) require.Equal(t, string(*tc.body.Role), sa.Role) @@ -540,7 +558,6 @@ func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) { type services struct { OrgService org.Service UserService user.Service - SAStore serviceaccounts.Store SAService tests.ServiceAccountMock APIKeyService apikey.Service } @@ -555,13 +572,13 @@ func setupTestServices(t *testing.T, db *sqlstore.SQLStore) services { require.NoError(t, err) userSvc, err := userimpl.ProvideService(db, orgService, db.Cfg, nil, nil, quotaService) require.NoError(t, err) - saStore := database.ProvideServiceAccountsStore(db, apiKeyService, kvStore, userSvc, orgService) - svcmock := tests.ServiceAccountMock{} + + saStore := database.ProvideServiceAccountsStore(nil, db, apiKeyService, kvStore, userSvc, orgService) + svcmock := tests.ServiceAccountMock{Store: saStore, Calls: tests.Calls{}, Stats: nil, SecretScanEnabled: false} return services{ OrgService: orgService, UserService: userSvc, - SAStore: saStore, SAService: svcmock, APIKeyService: apiKeyService, } diff --git a/pkg/services/serviceaccounts/api/token.go b/pkg/services/serviceaccounts/api/token.go index 2327422c7eb..6be829ccad5 100644 --- a/pkg/services/serviceaccounts/api/token.go +++ b/pkg/services/serviceaccounts/api/token.go @@ -72,7 +72,7 @@ func (api *ServiceAccountsAPI) ListTokens(ctx *models.ReqContext) response.Respo return response.Error(http.StatusBadRequest, "Service Account ID is invalid", err) } - saTokens, err := api.store.ListTokens(ctx.Req.Context(), &serviceaccounts.GetSATokensQuery{ + saTokens, err := api.service.ListTokens(ctx.Req.Context(), &serviceaccounts.GetSATokensQuery{ OrgID: &ctx.OrgID, ServiceAccountID: &saID, }) @@ -134,7 +134,7 @@ func (api *ServiceAccountsAPI) CreateToken(c *models.ReqContext) response.Respon } // confirm service account exists - if _, err := api.store.RetrieveServiceAccount(c.Req.Context(), c.OrgID, saID); err != nil { + if _, err := api.service.RetrieveServiceAccount(c.Req.Context(), c.OrgID, saID); err != nil { switch { case errors.Is(err, serviceaccounts.ErrServiceAccountNotFound): return response.Error(http.StatusNotFound, "Failed to retrieve service account", err) @@ -175,7 +175,7 @@ func (api *ServiceAccountsAPI) CreateToken(c *models.ReqContext) response.Respon cmd.Key = newKeyInfo.HashedKey - if err := api.store.AddServiceAccountToken(c.Req.Context(), saID, &cmd); err != nil { + if err := api.service.AddServiceAccountToken(c.Req.Context(), saID, &cmd); err != nil { if errors.Is(err, database.ErrInvalidTokenExpiration) { return response.Error(http.StatusBadRequest, err.Error(), nil) } @@ -217,7 +217,7 @@ func (api *ServiceAccountsAPI) DeleteToken(c *models.ReqContext) response.Respon } // confirm service account exists - if _, err := api.store.RetrieveServiceAccount(c.Req.Context(), c.OrgID, saID); err != nil { + if _, err := api.service.RetrieveServiceAccount(c.Req.Context(), c.OrgID, saID); err != nil { switch { case errors.Is(err, serviceaccounts.ErrServiceAccountNotFound): return response.Error(http.StatusNotFound, "Failed to retrieve service account", err) @@ -231,7 +231,7 @@ func (api *ServiceAccountsAPI) DeleteToken(c *models.ReqContext) response.Respon return response.Error(http.StatusBadRequest, "Token ID is invalid", err) } - if err = api.store.DeleteServiceAccountToken(c.Req.Context(), c.OrgID, saID, tokenID); err != nil { + if err = api.service.DeleteServiceAccountToken(c.Req.Context(), c.OrgID, saID, tokenID); err != nil { status := http.StatusNotFound if err != nil && !errors.Is(err, apikey.ErrNotFound) { status = http.StatusInternalServerError diff --git a/pkg/services/serviceaccounts/api/token_test.go b/pkg/services/serviceaccounts/api/token_test.go index ca5fda55914..9e63c738856 100644 --- a/pkg/services/serviceaccounts/api/token_test.go +++ b/pkg/services/serviceaccounts/api/token_test.go @@ -32,7 +32,7 @@ const ( serviceaccountIDTokensDetailPath = "/api/serviceaccounts/%v/tokens/%v" // #nosec G101 ) -func createTokenforSA(t *testing.T, store serviceaccounts.Store, keyName string, orgID int64, saID int64, secondsToLive int64) *apikey.APIKey { +func createTokenforSA(t *testing.T, service serviceaccounts.Service, keyName string, orgID int64, saID int64, secondsToLive int64) *apikey.APIKey { key, err := apikeygen.New(orgID, keyName) require.NoError(t, err) @@ -44,7 +44,7 @@ func createTokenforSA(t *testing.T, store serviceaccounts.Store, keyName string, Result: &apikey.APIKey{}, } - err = store.AddServiceAccountToken(context.Background(), saID, &cmd) + err = service.AddServiceAccountToken(context.Background(), saID, &cmd) require.NoError(t, err) return cmd.Result } @@ -131,7 +131,7 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) { bodyString = string(b) } - server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store, services.SAStore) + server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store) actual := requestResponse(server, http.MethodPost, endpoint, strings.NewReader(bodyString)) actualCode := actual.Code @@ -166,6 +166,7 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) { func TestServiceAccountsAPI_DeleteToken(t *testing.T) { store := db.InitTestDB(t) services := setupTestServices(t, store) + sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true}) type testCreateSAToken struct { @@ -225,11 +226,11 @@ func TestServiceAccountsAPI_DeleteToken(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - token := createTokenforSA(t, services.SAStore, tc.keyName, sa.OrgID, sa.ID, 1) + token := createTokenforSA(t, &services.SAService, tc.keyName, sa.OrgID, sa.ID, 1) endpoint := fmt.Sprintf(serviceaccountIDTokensDetailPath, sa.ID, token.Id) bodyString := "" - server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store, services.SAStore) + server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store) actual := requestResponse(server, http.MethodDelete, endpoint, strings.NewReader(bodyString)) actualCode := actual.Code @@ -249,20 +250,11 @@ func TestServiceAccountsAPI_DeleteToken(t *testing.T) { } } -type saStoreMockTokens struct { - serviceaccounts.Store - saAPIKeys []apikey.APIKey -} - -func (s *saStoreMockTokens) ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) { - return s.saAPIKeys, nil -} - func TestServiceAccountsAPI_ListTokens(t *testing.T) { store := db.InitTestDB(t) - svcmock := tests.ServiceAccountMock{} - sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true}) + services := setupTestServices(t, store) + sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true}) type testCreateSAToken struct { desc string tokens []apikey.APIKey @@ -351,7 +343,8 @@ func TestServiceAccountsAPI_ListTokens(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { endpoint := fmt.Sprintf(serviceAccountIDPath+"/tokens", sa.ID) - server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, &saStoreMockTokens{saAPIKeys: tc.tokens}) + services.SAService.ExpectedTokens = tc.tokens + server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store) actual := requestResponse(server, http.MethodGet, endpoint, http.NoBody) actualCode := actual.Code diff --git a/pkg/services/serviceaccounts/database/database.go b/pkg/services/serviceaccounts/database/store.go similarity index 85% rename from pkg/services/serviceaccounts/database/database.go rename to pkg/services/serviceaccounts/database/store.go index 20982a29f42..63c417505f1 100644 --- a/pkg/services/serviceaccounts/database/database.go +++ b/pkg/services/serviceaccounts/database/store.go @@ -18,20 +18,23 @@ import ( "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/setting" ) type ServiceAccountsStoreImpl struct { - sqlStore *sqlstore.SQLStore + cfg *setting.Cfg + sqlStore db.DB apiKeyService apikey.Service kvStore kvstore.KVStore log log.Logger - userService user.Service orgService org.Service + userService user.Service } -func ProvideServiceAccountsStore(store *sqlstore.SQLStore, apiKeyService apikey.Service, +func ProvideServiceAccountsStore(cfg *setting.Cfg, store db.DB, apiKeyService apikey.Service, kvStore kvstore.KVStore, userService user.Service, orgService org.Service) *ServiceAccountsStoreImpl { return &ServiceAccountsStoreImpl{ + cfg: cfg, sqlStore: store, apiKeyService: apiKeyService, kvStore: kvStore, @@ -101,9 +104,11 @@ func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, org } // UpdateServiceAccount updates service account -func (s *ServiceAccountsStoreImpl) UpdateServiceAccount(ctx context.Context, +func (s *ServiceAccountsStoreImpl) UpdateServiceAccount( + ctx context.Context, orgId, serviceAccountId int64, - saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) { + saForm *serviceaccounts.UpdateServiceAccountForm, +) (*serviceaccounts.ServiceAccountProfileDTO, error) { updatedUser := &serviceaccounts.ServiceAccountProfileDTO{} err := s.sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error { @@ -175,7 +180,7 @@ func (s *ServiceAccountsStoreImpl) DeleteServiceAccount(ctx context.Context, org func (s *ServiceAccountsStoreImpl) deleteServiceAccount(sess *db.Session, orgId, serviceAccountId int64) error { user := user.User{} has, err := sess.Where(`org_id = ? and id = ? and is_service_account = ?`, - orgId, serviceAccountId, s.sqlStore.Dialect.BooleanStr(true)).Get(&user) + orgId, serviceAccountId, s.sqlStore.GetDialect().BooleanStr(true)).Get(&user) if err != nil { return err } @@ -197,8 +202,8 @@ func (s *ServiceAccountsStoreImpl) RetrieveServiceAccount(ctx context.Context, o err := s.sqlStore.WithDbSession(ctx, func(dbSession *db.Session) error { sess := dbSession.Table("org_user") - sess.Join("INNER", s.sqlStore.Dialect.Quote("user"), - fmt.Sprintf("org_user.user_id=%s.id", s.sqlStore.Dialect.Quote("user"))) + sess.Join("INNER", s.sqlStore.GetDialect().Quote("user"), + fmt.Sprintf("org_user.user_id=%s.id", s.sqlStore.GetDialect().Quote("user"))) whereConditions := make([]string, 0, 3) whereParams := make([]interface{}, 0) @@ -211,8 +216,8 @@ func (s *ServiceAccountsStoreImpl) RetrieveServiceAccount(ctx context.Context, o whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = %s", - s.sqlStore.Dialect.Quote("user"), - s.sqlStore.Dialect.BooleanStr(true))) + s.sqlStore.GetDialect().Quote("user"), + s.sqlStore.GetDialect().BooleanStr(true))) sess.Where(strings.Join(whereConditions, " AND "), whereParams...) @@ -250,12 +255,12 @@ func (s *ServiceAccountsStoreImpl) RetrieveServiceAccountIdByName(ctx context.Co whereConditions := []string{ fmt.Sprintf("%s.name = ?", - s.sqlStore.Dialect.Quote("user")), + s.sqlStore.GetDialect().Quote("user")), fmt.Sprintf("%s.org_id = ?", - s.sqlStore.Dialect.Quote("user")), + s.sqlStore.GetDialect().Quote("user")), fmt.Sprintf("%s.is_service_account = %s", - s.sqlStore.Dialect.Quote("user"), - s.sqlStore.Dialect.BooleanStr(true)), + s.sqlStore.GetDialect().Quote("user"), + s.sqlStore.GetDialect().BooleanStr(true)), } whereParams := []interface{}{name, orgId} @@ -281,34 +286,31 @@ func (s *ServiceAccountsStoreImpl) RetrieveServiceAccountIdByName(ctx context.Co return serviceAccount.Id, nil } -func (s *ServiceAccountsStoreImpl) SearchOrgServiceAccounts( - ctx context.Context, orgId int64, query string, filter serviceaccounts.ServiceAccountFilter, page int, limit int, - signedInUser *user.SignedInUser, -) (*serviceaccounts.SearchServiceAccountsResult, error) { - searchResult := &serviceaccounts.SearchServiceAccountsResult{ +func (s *ServiceAccountsStoreImpl) SearchOrgServiceAccounts(ctx context.Context, query *serviceaccounts.SearchOrgServiceAccountsQuery) (*serviceaccounts.SearchOrgServiceAccountsResult, error) { + searchResult := &serviceaccounts.SearchOrgServiceAccountsResult{ TotalCount: 0, ServiceAccounts: make([]*serviceaccounts.ServiceAccountDTO, 0), - Page: page, - PerPage: limit, + Page: query.Page, + PerPage: query.Limit, } err := s.sqlStore.WithDbSession(ctx, func(dbSession *db.Session) error { sess := dbSession.Table("org_user") - sess.Join("INNER", s.sqlStore.Dialect.Quote("user"), fmt.Sprintf("org_user.user_id=%s.id", s.sqlStore.Dialect.Quote("user"))) + sess.Join("INNER", s.sqlStore.GetDialect().Quote("user"), fmt.Sprintf("org_user.user_id=%s.id", s.sqlStore.GetDialect().Quote("user"))) whereConditions := make([]string, 0) whereParams := make([]interface{}, 0) whereConditions = append(whereConditions, "org_user.org_id = ?") - whereParams = append(whereParams, orgId) + whereParams = append(whereParams, query.OrgID) whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = %s", - s.sqlStore.Dialect.Quote("user"), - s.sqlStore.Dialect.BooleanStr(true))) + s.sqlStore.GetDialect().Quote("user"), + s.sqlStore.GetDialect().BooleanStr(true))) - if !accesscontrol.IsDisabled(s.sqlStore.Cfg) { - acFilter, err := accesscontrol.Filter(signedInUser, "org_user.user_id", "serviceaccounts:id:", serviceaccounts.ActionRead) + if !accesscontrol.IsDisabled(s.cfg) { + acFilter, err := accesscontrol.Filter(query.SignedInUser, "org_user.user_id", "serviceaccounts:id:", serviceaccounts.ActionRead) if err != nil { return err } @@ -316,13 +318,13 @@ func (s *ServiceAccountsStoreImpl) SearchOrgServiceAccounts( whereParams = append(whereParams, acFilter.Args...) } - if query != "" { - queryWithWildcards := "%" + query + "%" - whereConditions = append(whereConditions, "(email "+s.sqlStore.Dialect.LikeStr()+" ? OR name "+s.sqlStore.Dialect.LikeStr()+" ? OR login "+s.sqlStore.Dialect.LikeStr()+" ?)") + if query.Query != "" { + queryWithWildcards := "%" + query.Query + "%" + whereConditions = append(whereConditions, "(email "+s.sqlStore.GetDialect().LikeStr()+" ? OR name "+s.sqlStore.GetDialect().LikeStr()+" ? OR login "+s.sqlStore.GetDialect().LikeStr()+" ?)") whereParams = append(whereParams, queryWithWildcards, queryWithWildcards, queryWithWildcards) } - switch filter { + switch query.Filter { case serviceaccounts.FilterIncludeAll: // pass case serviceaccounts.FilterOnlyExpiredTokens: @@ -336,17 +338,17 @@ func (s *ServiceAccountsStoreImpl) SearchOrgServiceAccounts( whereConditions = append( whereConditions, "is_disabled = ?") - whereParams = append(whereParams, s.sqlStore.Dialect.BooleanStr(true)) + whereParams = append(whereParams, s.sqlStore.GetDialect().BooleanStr(true)) default: - s.log.Warn("invalid filter user for service account filtering", "service account search filtering", filter) + s.log.Warn("invalid filter user for service account filtering", "service account search filtering", query.Filter) } if len(whereConditions) > 0 { sess.Where(strings.Join(whereConditions, " AND "), whereParams...) } - if limit > 0 { - offset := limit * (page - 1) - sess.Limit(limit, offset) + if query.Limit > 0 { + offset := query.Limit * (query.Page - 1) + sess.Limit(query.Limit, offset) } sess.Cols( @@ -367,7 +369,7 @@ func (s *ServiceAccountsStoreImpl) SearchOrgServiceAccounts( // get total serviceaccount := serviceaccounts.ServiceAccountDTO{} countSess := dbSession.Table("org_user") - sess.Join("INNER", s.sqlStore.Dialect.Quote("user"), fmt.Sprintf("org_user.user_id=%s.id", s.sqlStore.Dialect.Quote("user"))) + sess.Join("INNER", s.sqlStore.GetDialect().Quote("user"), fmt.Sprintf("org_user.user_id=%s.id", s.sqlStore.GetDialect().Quote("user"))) if len(whereConditions) > 0 { countSess.Where(strings.Join(whereConditions, " AND "), whereParams...) @@ -468,9 +470,6 @@ func (s *ServiceAccountsStoreImpl) CreateServiceAccountFromApikey(ctx context.Co } if err := s.assignApiKeyToServiceAccount(sess, key.Id, newSA.ID); err != nil { - if err := s.userService.Delete(ctx, &user.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) } @@ -508,7 +507,7 @@ func (s *ServiceAccountsStoreImpl) RevertApiKey(ctx context.Context, saId int64, err = s.sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error { user := user.User{} has, err := sess.Where(`org_id = ? and id = ? and is_service_account = ?`, - key.OrgId, *key.ServiceAccountId, s.sqlStore.Dialect.BooleanStr(true)).Get(&user) + key.OrgId, *key.ServiceAccountId, s.sqlStore.GetDialect().BooleanStr(true)).Get(&user) if err != nil { return err } diff --git a/pkg/services/serviceaccounts/database/database_test.go b/pkg/services/serviceaccounts/database/store_test.go similarity index 85% rename from pkg/services/serviceaccounts/database/database_test.go rename to pkg/services/serviceaccounts/database/store_test.go index 41084066d4c..17aa3758deb 100644 --- a/pkg/services/serviceaccounts/database/database_test.go +++ b/pkg/services/serviceaccounts/database/store_test.go @@ -121,7 +121,7 @@ func setupTestDatabase(t *testing.T) (*sqlstore.SQLStore, *ServiceAccountsStoreI require.NoError(t, err) userSvc, err := userimpl.ProvideService(db, orgService, db.Cfg, nil, nil, quotaService) require.NoError(t, err) - return db, ProvideServiceAccountsStore(db, apiKeyService, kvStore, userSvc, orgService) + return db, ProvideServiceAccountsStore(db.Cfg, db, apiKeyService, kvStore, userSvc, orgService) } func TestStore_RetrieveServiceAccount(t *testing.T) { @@ -174,9 +174,9 @@ func TestStore_MigrateApiKeys(t *testing.T) { 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" + store.cfg.AutoAssignOrg = true + store.cfg.AutoAssignOrgId = 1 + store.cfg.AutoAssignOrgRole = "Viewer" _, err := store.orgService.CreateWithMember(context.Background(), &org.CreateOrgCommand{Name: "main"}) require.NoError(t, err) key := tests.SetupApiKey(t, db, c.key) @@ -186,11 +186,22 @@ func TestStore_MigrateApiKeys(t *testing.T) { } else { require.NoError(t, err) - serviceAccounts, err := store.SearchOrgServiceAccounts(context.Background(), key.OrgId, "", "all", 1, 50, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{ - key.OrgId: { - "serviceaccounts:read": {"serviceaccounts:id:*"}, + q := serviceaccounts.SearchOrgServiceAccountsQuery{ + OrgID: key.OrgId, + Query: "", + Page: 1, + Limit: 50, + SignedInUser: &user.SignedInUser{ + UserID: 1, + OrgID: 1, + Permissions: map[int64]map[string][]string{ + key.OrgId: { + "serviceaccounts:read": {"serviceaccounts:id:*"}, + }, + }, }, - }}) + } + serviceAccounts, err := store.SearchOrgServiceAccounts(context.Background(), &q) require.NoError(t, err) require.Equal(t, int64(1), serviceAccounts.TotalCount) saMigrated := serviceAccounts.ServiceAccounts[0] @@ -251,9 +262,9 @@ func TestStore_MigrateAllApiKeys(t *testing.T) { 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" + store.cfg.AutoAssignOrg = true + store.cfg.AutoAssignOrgId = 1 + store.cfg.AutoAssignOrgRole = "Viewer" _, err := store.orgService.CreateWithMember(context.Background(), &org.CreateOrgCommand{Name: "main"}) require.NoError(t, err) @@ -267,11 +278,22 @@ func TestStore_MigrateAllApiKeys(t *testing.T) { } else { require.NoError(t, err) - serviceAccounts, err := store.SearchOrgServiceAccounts(context.Background(), c.orgId, "", "all", 1, 50, &user.SignedInUser{UserID: 101, OrgID: c.orgId, Permissions: map[int64]map[string][]string{ - c.orgId: { - "serviceaccounts:read": {"serviceaccounts:id:*"}, + q := serviceaccounts.SearchOrgServiceAccountsQuery{ + OrgID: c.orgId, + Query: "", + Page: 1, + Limit: 50, + SignedInUser: &user.SignedInUser{ + UserID: 1, + OrgID: 1, + Permissions: map[int64]map[string][]string{ + c.orgId: { + "serviceaccounts:read": {"serviceaccounts:id:*"}, + }, + }, }, - }}) + } + serviceAccounts, err := store.SearchOrgServiceAccounts(context.Background(), &q) require.NoError(t, err) require.Equal(t, c.expectedServiceAccouts, serviceAccounts.TotalCount) if c.expectedServiceAccouts > 0 { @@ -313,9 +335,9 @@ func TestStore_RevertApiKey(t *testing.T) { 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" + store.cfg.AutoAssignOrg = true + store.cfg.AutoAssignOrgId = 1 + store.cfg.AutoAssignOrgRole = "Viewer" _, err := store.orgService.CreateWithMember(context.Background(), &org.CreateOrgCommand{Name: "main"}) require.NoError(t, err) @@ -327,11 +349,22 @@ func TestStore_RevertApiKey(t *testing.T) { if c.forceMismatchServiceAccount { saId = rand.Int63() } else { - serviceAccounts, err := store.SearchOrgServiceAccounts(context.Background(), key.OrgId, "", "all", 1, 50, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{ - key.OrgId: { - "serviceaccounts:read": {"serviceaccounts:id:*"}, + q := serviceaccounts.SearchOrgServiceAccountsQuery{ + OrgID: key.OrgId, + Query: "", + Page: 1, + Limit: 50, + SignedInUser: &user.SignedInUser{ + UserID: 1, + OrgID: 1, + Permissions: map[int64]map[string][]string{ + key.OrgId: { + "serviceaccounts:read": {"serviceaccounts:id:*"}, + }, + }, }, - }}) + } + serviceAccounts, err := store.SearchOrgServiceAccounts(context.Background(), &q) require.NoError(t, err) saId = serviceAccounts.ServiceAccounts[0].Id } @@ -342,12 +375,22 @@ func TestStore_RevertApiKey(t *testing.T) { require.ErrorIs(t, err, c.expectedErr) } else { require.NoError(t, err) - - serviceAccounts, err := store.SearchOrgServiceAccounts(context.Background(), key.OrgId, "", "all", 1, 50, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{ - key.OrgId: { - "serviceaccounts:read": {"serviceaccounts:id:*"}, + q := serviceaccounts.SearchOrgServiceAccountsQuery{ + OrgID: key.OrgId, + Query: "", + Page: 1, + Limit: 50, + SignedInUser: &user.SignedInUser{ + UserID: 1, + OrgID: 1, + Permissions: map[int64]map[string][]string{ + key.OrgId: { + "serviceaccounts:read": {"serviceaccounts:id:*"}, + }, + }, }, - }}) + } + serviceAccounts, err := store.SearchOrgServiceAccounts(context.Background(), &q) require.NoError(t, err) // Service account should be deleted require.Equal(t, int64(0), serviceAccounts.TotalCount) diff --git a/pkg/services/serviceaccounts/errors.go b/pkg/services/serviceaccounts/errors.go index 2ea8853d995..a2005fe0d16 100644 --- a/pkg/services/serviceaccounts/errors.go +++ b/pkg/services/serviceaccounts/errors.go @@ -6,4 +6,9 @@ var ( ErrServiceAccountNotFound = errors.New("service account not found") ErrServiceAccountInvalidRole = errors.New("invalid role specified") ErrServiceAccountRolePrivilegeDenied = errors.New("can not assign a role higher than user's role") + ErrServiceAccountInvalidOrgID = errors.New("invalid org id specified") + ErrServiceAccountInvalidID = errors.New("invalid service account id specified") + ErrServiceAccountInvalidAPIKeyID = errors.New("invalid api key id specified") + ErrServiceAccountInvalidTokenID = errors.New("invalid service account token id specified") + ErrServiceAccountUpdateForm = errors.New("invalid update form") ) diff --git a/pkg/services/serviceaccounts/manager/service.go b/pkg/services/serviceaccounts/manager/service.go index 635e467df8d..80c1fb96cb6 100644 --- a/pkg/services/serviceaccounts/manager/service.go +++ b/pkg/services/serviceaccounts/manager/service.go @@ -2,16 +2,23 @@ package manager import ( "context" + "errors" "fmt" "time" "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" + "github.com/grafana/grafana/pkg/services/apikey" + "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/serviceaccounts/api" + "github.com/grafana/grafana/pkg/services/serviceaccounts/database" "github.com/grafana/grafana/pkg/services/serviceaccounts/secretscan" + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" ) @@ -21,7 +28,7 @@ const ( ) type ServiceAccountsService struct { - store serviceaccounts.Store + store store log log.Logger backgroundLog log.Logger secretScanService secretscan.Checker @@ -35,13 +42,26 @@ func ProvideServiceAccountsService( ac accesscontrol.AccessControl, routeRegister routing.RouteRegister, usageStats usagestats.Service, - serviceAccountsStore serviceaccounts.Store, + store *sqlstore.SQLStore, + apiKeyService apikey.Service, + kvStore kvstore.KVStore, + userService user.Service, + orgService org.Service, permissionService accesscontrol.ServiceAccountPermissionsService, accesscontrolService accesscontrol.Service, ) (*ServiceAccountsService, error) { + serviceAccountsStore := database.ProvideServiceAccountsStore( + cfg, + store, + apiKeyService, + kvStore, + userService, + orgService, + ) + log := log.New("serviceaccounts") s := &ServiceAccountsService{ store: serviceAccountsStore, - log: log.New("serviceaccounts"), + log: log, backgroundLog: log.New("serviceaccounts.background"), } @@ -51,7 +71,7 @@ func ProvideServiceAccountsService( usageStats.RegisterMetricsFunc(s.getUsageMetrics) - serviceaccountsAPI := api.NewServiceAccountsAPI(cfg, s, ac, accesscontrolService, routeRegister, s.store, permissionService) + serviceaccountsAPI := api.NewServiceAccountsAPI(cfg, s, ac, accesscontrolService, routeRegister, permissionService) serviceaccountsAPI.RegisterAPIEndpoints() s.secretScanEnabled = cfg.SectionWithEnvOverrides("secretscan").Key("enabled").MustBool(false) @@ -122,13 +142,144 @@ func (sa *ServiceAccountsService) Run(ctx context.Context) error { } func (sa *ServiceAccountsService) CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) { + if err := validOrgID(orgID); err != nil { + return nil, err + } return sa.store.CreateServiceAccount(ctx, orgID, saForm) } -func (sa *ServiceAccountsService) DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error { - return sa.store.DeleteServiceAccount(ctx, orgID, serviceAccountID) +func (sa *ServiceAccountsService) RetrieveServiceAccount(ctx context.Context, orgID int64, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error) { + if err := validOrgID(orgID); err != nil { + return nil, err + } + if err := validServiceAccountID(serviceAccountID); err != nil { + return nil, err + } + return sa.store.RetrieveServiceAccount(ctx, orgID, serviceAccountID) } func (sa *ServiceAccountsService) RetrieveServiceAccountIdByName(ctx context.Context, orgID int64, name string) (int64, error) { + if err := validOrgID(orgID); err != nil { + return 0, err + } + if name == "" { + return 0, errors.New("name is required") + } return sa.store.RetrieveServiceAccountIdByName(ctx, orgID, name) } + +func (sa *ServiceAccountsService) DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error { + if err := validOrgID(orgID); err != nil { + return err + } + if err := validServiceAccountID(serviceAccountID); err != nil { + return err + } + return sa.store.DeleteServiceAccount(ctx, orgID, serviceAccountID) +} + +func (sa *ServiceAccountsService) UpdateServiceAccount(ctx context.Context, orgID int64, serviceAccountID int64, saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) { + if err := validOrgID(orgID); err != nil { + return nil, err + } + if err := validServiceAccountID(serviceAccountID); err != nil { + return nil, err + } + return sa.store.UpdateServiceAccount(ctx, orgID, serviceAccountID, saForm) +} + +func (sa *ServiceAccountsService) SearchOrgServiceAccounts(ctx context.Context, query *serviceaccounts.SearchOrgServiceAccountsQuery) (*serviceaccounts.SearchOrgServiceAccountsResult, error) { + if query.Page <= 0 || query.Limit <= 0 { + query.SetDefaults() + // optional: logging + } + return sa.store.SearchOrgServiceAccounts(ctx, query) +} + +func (sa *ServiceAccountsService) ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) { + return sa.store.ListTokens(ctx, query) +} + +func (sa *ServiceAccountsService) AddServiceAccountToken(ctx context.Context, serviceAccountID int64, query *serviceaccounts.AddServiceAccountTokenCommand) error { + if err := validServiceAccountID(serviceAccountID); err != nil { + return err + } + return sa.store.AddServiceAccountToken(ctx, serviceAccountID, query) +} + +func (sa *ServiceAccountsService) DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID int64, tokenID int64) error { + if err := validOrgID(orgID); err != nil { + return err + } + if err := validServiceAccountID(serviceAccountID); err != nil { + return err + } + if err := validServiceAccountTokenID(tokenID); err != nil { + return err + } + 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 + } + return sa.store.HideApiKeysTab(ctx, orgID) +} + +func (sa *ServiceAccountsService) MigrateApiKey(ctx context.Context, orgID, keyID int64) error { + if err := validOrgID(orgID); err != nil { + return err + } + if err := validAPIKeyID(keyID); err != nil { + return err + } + return sa.store.MigrateApiKey(ctx, orgID, keyID) +} +func (sa *ServiceAccountsService) MigrateApiKeysToServiceAccounts(ctx context.Context, orgID int64) error { + if err := validOrgID(orgID); err != nil { + return err + } + return sa.store.MigrateApiKeysToServiceAccounts(ctx, orgID) +} +func (sa *ServiceAccountsService) RevertApiKey(ctx context.Context, orgID, keyID int64) error { + if err := validOrgID(orgID); err != nil { + return err + } + if err := validAPIKeyID(keyID); err != nil { + return err + } + return sa.store.RevertApiKey(ctx, orgID, keyID) +} + +func validOrgID(orgID int64) error { + if orgID == 0 { + return serviceaccounts.ErrServiceAccountInvalidOrgID + } + return nil +} +func validServiceAccountID(serviceaccountID int64) error { + if serviceaccountID == 0 { + return serviceaccounts.ErrServiceAccountInvalidID + } + return nil +} +func validServiceAccountTokenID(tokenID int64) error { + if tokenID == 0 { + return serviceaccounts.ErrServiceAccountInvalidTokenID + } + return nil +} +func validAPIKeyID(apiKeyID int64) error { + if apiKeyID == 0 { + return serviceaccounts.ErrServiceAccountInvalidAPIKeyID + } + return nil +} diff --git a/pkg/services/serviceaccounts/manager/service_test.go b/pkg/services/serviceaccounts/manager/service_test.go index 7e80399d500..916e825ca67 100644 --- a/pkg/services/serviceaccounts/manager/service_test.go +++ b/pkg/services/serviceaccounts/manager/service_test.go @@ -4,17 +4,144 @@ import ( "context" "testing" - "github.com/grafana/grafana/pkg/services/serviceaccounts/tests" - "github.com/stretchr/testify/assert" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/apikey" + "github.com/grafana/grafana/pkg/services/org" + "github.com/grafana/grafana/pkg/services/serviceaccounts" + "github.com/stretchr/testify/require" ) +type FakeServiceAccountStore struct { + ExpectedServiceAccountID *serviceaccounts.ServiceAccount + ExpectedServiceAccountDTO *serviceaccounts.ServiceAccountDTO + ExpectedServiceAccountProfileDTO *serviceaccounts.ServiceAccountProfileDTO + ExpectedSearchServiceAccountQueryResult *serviceaccounts.SearchOrgServiceAccountsResult + ExpectedServiceAccountMigrationStatus *serviceaccounts.APIKeysMigrationStatus + ExpectedStats *serviceaccounts.Stats + ExpectedApiKeys []apikey.APIKey + ExpectedError error +} + +func newServiceAccountStoreFake() *FakeServiceAccountStore { + return &FakeServiceAccountStore{} +} + +// CreateServiceAccount is a fake creating a service account. +func (f *FakeServiceAccountStore) RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error) { + return f.ExpectedServiceAccountProfileDTO, f.ExpectedError +} + +// RetrieveServiceAccountIdByName is a fake retrieving a service account id by name. +func (f *FakeServiceAccountStore) RetrieveServiceAccountIdByName(ctx context.Context, orgID int64, name string) (int64, error) { + return f.ExpectedServiceAccountID.Id, f.ExpectedError +} + +// CreateServiceAccount is a fake creating a service account. +func (f *FakeServiceAccountStore) CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) { + return f.ExpectedServiceAccountDTO, f.ExpectedError +} + +// SearchOrgServiceAccounts is a fake searching for service accounts. +func (f *FakeServiceAccountStore) SearchOrgServiceAccounts(ctx context.Context, query *serviceaccounts.SearchOrgServiceAccountsQuery) (*serviceaccounts.SearchOrgServiceAccountsResult, error) { + return f.ExpectedSearchServiceAccountQueryResult, f.ExpectedError +} + +// UpdateServiceAccount is a fake updating a service account. +func (f *FakeServiceAccountStore) UpdateServiceAccount(ctx context.Context, orgID, serviceAccountID int64, + saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) { + return f.ExpectedServiceAccountProfileDTO, f.ExpectedError +} + +// DeleteServiceAccount is a fake deleting a service account. +func (f *FakeServiceAccountStore) DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error { + 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 +} + +// MigrateApiKeysToServiceAccounts is a fake migrating api keys to service accounts. +func (f *FakeServiceAccountStore) MigrateApiKeysToServiceAccounts(ctx context.Context, orgID int64) error { + return f.ExpectedError +} + +// MigrateApiKey is a fake migrating an api key to a service account. +func (f *FakeServiceAccountStore) MigrateApiKey(ctx context.Context, orgID int64, keyId int64) error { + return f.ExpectedError +} + +// RevertApiKey is a fake reverting an api key to a service account. +func (f *FakeServiceAccountStore) RevertApiKey(ctx context.Context, saId int64, keyId int64) error { + return f.ExpectedError +} + +// ListTokens is a fake listing tokens. +func (f *FakeServiceAccountStore) ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) { + return f.ExpectedApiKeys, f.ExpectedError +} + +// RevokeServiceAccountToken is a fake revoking a service account token. +func (f *FakeServiceAccountStore) RevokeServiceAccountToken(ctx context.Context, orgId, serviceAccountId, tokenId int64) error { + return f.ExpectedError +} + +// AddServiceAccountToken is a fake adding a service account token. +func (f *FakeServiceAccountStore) AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error { + return f.ExpectedError +} + +// DeleteServiceAccountToken is a fake deleting a service account token. +func (f *FakeServiceAccountStore) DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error { + return f.ExpectedError +} + +// GetUsageMetrics is a fake getting usage metrics. +func (f *FakeServiceAccountStore) GetUsageMetrics(ctx context.Context) (*serviceaccounts.Stats, error) { + return f.ExpectedStats, f.ExpectedError +} + +type SecretsCheckerFake struct { + ExpectedError error +} + +func (f *SecretsCheckerFake) CheckTokens(ctx context.Context) error { + return f.ExpectedError +} + func TestProvideServiceAccount_DeleteServiceAccount(t *testing.T) { - t.Run("should call store function", func(t *testing.T) { - storeMock := &tests.ServiceAccountsStoreMock{Calls: tests.Calls{}} - svc := ServiceAccountsService{store: storeMock} - err := svc.DeleteServiceAccount(context.Background(), 1, 1) + storeMock := newServiceAccountStoreFake() + svc := ServiceAccountsService{storeMock, log.New("test"), log.New("background.test"), &SecretsCheckerFake{}, false, 0} + testOrgId := 1 + + t.Run("should create service account", func(t *testing.T) { + serviceAccountName := "new Service Account" + serviceAccountRole := org.RoleAdmin + isDisabled := true + saForm := &serviceaccounts.CreateServiceAccountForm{ + Name: serviceAccountName, + Role: &serviceAccountRole, + IsDisabled: &isDisabled, + } + storeMock.ExpectedServiceAccountDTO = &serviceaccounts.ServiceAccountDTO{ + Id: 1, + Name: serviceAccountName, + Role: string(serviceAccountRole), + IsDisabled: isDisabled, + } + _, err := svc.CreateServiceAccount(context.Background(), int64(testOrgId), saForm) + require.NoError(t, err) + }) + + t.Run("should delete service account", func(t *testing.T) { + err := svc.DeleteServiceAccount(context.Background(), int64(testOrgId), 1) require.NoError(t, err) - assert.Len(t, storeMock.Calls.DeleteServiceAccount, 1) }) } diff --git a/pkg/services/serviceaccounts/manager/stats_test.go b/pkg/services/serviceaccounts/manager/stats_test.go index 78f0ab6c846..4b4b2dd6ff1 100644 --- a/pkg/services/serviceaccounts/manager/stats_test.go +++ b/pkg/services/serviceaccounts/manager/stats_test.go @@ -4,22 +4,22 @@ import ( "context" "testing" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/serviceaccounts" - "github.com/grafana/grafana/pkg/services/serviceaccounts/tests" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func Test_UsageStats(t *testing.T) { - storeMock := &tests.ServiceAccountsStoreMock{Calls: tests.Calls{}, Stats: &serviceaccounts.Stats{ - ServiceAccounts: 1, - Tokens: 1, - }} - svc := ServiceAccountsService{store: storeMock, secretScanEnabled: true} + storeMock := newServiceAccountStoreFake() + svc := ServiceAccountsService{storeMock, log.New("test"), log.New("background-test"), &SecretsCheckerFake{}, true, 5} err := svc.DeleteServiceAccount(context.Background(), 1, 1) require.NoError(t, err) - assert.Len(t, storeMock.Calls.DeleteServiceAccount, 1) + storeMock.ExpectedStats = &serviceaccounts.Stats{ + ServiceAccounts: 1, + Tokens: 1, + } stats, err := svc.getUsageMetrics(context.Background()) require.NoError(t, err) diff --git a/pkg/services/serviceaccounts/manager/store.go b/pkg/services/serviceaccounts/manager/store.go new file mode 100644 index 00000000000..448820059b7 --- /dev/null +++ b/pkg/services/serviceaccounts/manager/store.go @@ -0,0 +1,39 @@ +package manager + +import ( + "context" + + "github.com/grafana/grafana/pkg/services/apikey" + "github.com/grafana/grafana/pkg/services/serviceaccounts" +) + +/* +Store is the database store for service accounts. + +migration from apikeys to service accounts: +HideApiKeyTab is used to hide the api key tab in the UI. +MigrateApiKeysToServiceAccounts migrates all API keys to service accounts. +MigrateApiKey migrates a single API key to a service account. + +// only used for interal api calls +RevertApiKey reverts a single service account to an API key. +*/ +type store interface { + CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) + SearchOrgServiceAccounts(ctx context.Context, query *serviceaccounts.SearchOrgServiceAccountsQuery) (*serviceaccounts.SearchOrgServiceAccountsResult, error) + UpdateServiceAccount(ctx context.Context, orgID, serviceAccountID int64, + saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) + 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 + RevertApiKey(ctx context.Context, saId int64, keyId int64) error + ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) + RevokeServiceAccountToken(ctx context.Context, orgId, serviceAccountId, tokenId int64) error + AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error + DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error + GetUsageMetrics(ctx context.Context) (*serviceaccounts.Stats, error) +} diff --git a/pkg/services/serviceaccounts/models.go b/pkg/services/serviceaccounts/models.go index 2b1bc8a9c66..5c74cda8b6e 100644 --- a/pkg/services/serviceaccounts/models.go +++ b/pkg/services/serviceaccounts/models.go @@ -6,6 +6,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/apikey" "github.com/grafana/grafana/pkg/services/org" + "github.com/grafana/grafana/pkg/services/user" ) var ( @@ -38,9 +39,10 @@ type CreateServiceAccountForm struct { // swagger:model type UpdateServiceAccountForm struct { - Name *string `json:"name"` - Role *org.RoleType `json:"role"` - IsDisabled *bool `json:"isDisabled"` + Name *string `json:"name"` + ServiceAccountID int64 `json:"serviceAccountId"` + Role *org.RoleType `json:"role"` + IsDisabled *bool `json:"isDisabled"` } // swagger: model @@ -77,8 +79,22 @@ type AddServiceAccountTokenCommand struct { Result *apikey.APIKey `json:"-"` } +type SearchOrgServiceAccountsQuery struct { + OrgID int64 + Query string + Filter ServiceAccountFilter + Page int + Limit int + SignedInUser *user.SignedInUser +} + +func (q *SearchOrgServiceAccountsQuery) SetDefaults() { + q.Page = 1 + q.Limit = 100 +} + // swagger: model -type SearchServiceAccountsResult struct { +type SearchOrgServiceAccountsResult struct { // It can be used for pagination of the user list // E.g. if totalCount is equal to 100 users and // the perpage parameter is set to 10 then there are 10 pages of users. diff --git a/pkg/services/serviceaccounts/retriever/retriever.go b/pkg/services/serviceaccounts/retriever/retriever.go new file mode 100644 index 00000000000..f2d41622db4 --- /dev/null +++ b/pkg/services/serviceaccounts/retriever/retriever.go @@ -0,0 +1,52 @@ +package retriever + +import ( + "context" + + "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/infra/kvstore" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/apikey" + "github.com/grafana/grafana/pkg/services/org" + "github.com/grafana/grafana/pkg/services/serviceaccounts" + "github.com/grafana/grafana/pkg/services/serviceaccounts/database" + "github.com/grafana/grafana/pkg/services/user" +) + +// ServiceAccountRetriever is the service that retrieves service accounts. +// At the time of writing, this service is only used by the service accounts permissions service +// to avoid cyclic dependency between the ServiceAccountService and the ServiceAccountPermissionsService +type ServiceAccountRetriever interface { + RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error) +} + +// ServiceAccountRetriever is the service that manages service accounts. +type Service struct { + store *database.ServiceAccountsStoreImpl + logger log.Logger +} + +func ProvideService( + store db.DB, + apiKeyService apikey.Service, + kvStore kvstore.KVStore, + userService user.Service, + orgService org.Service, +) *Service { + serviceAccountsStore := database.ProvideServiceAccountsStore( + nil, + store, + apiKeyService, + kvStore, + userService, + orgService, + ) + return &Service{ + store: serviceAccountsStore, + logger: log.New("serviceaccountretriever"), + } +} + +func (s *Service) RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error) { + return s.store.RetrieveServiceAccount(ctx, orgID, serviceAccountID) +} diff --git a/pkg/services/serviceaccounts/serviceaccounts.go b/pkg/services/serviceaccounts/serviceaccounts.go index 940a9cb3ce7..1452aa10e9d 100644 --- a/pkg/services/serviceaccounts/serviceaccounts.go +++ b/pkg/services/serviceaccounts/serviceaccounts.go @@ -2,9 +2,6 @@ package serviceaccounts import ( "context" - - "github.com/grafana/grafana/pkg/services/apikey" - "github.com/grafana/grafana/pkg/services/user" ) /* @@ -16,37 +13,9 @@ do not have a password. type Service interface { CreateServiceAccount(ctx context.Context, orgID int64, saForm *CreateServiceAccountForm) (*ServiceAccountDTO, error) DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error - RetrieveServiceAccountIdByName(ctx context.Context, orgID int64, name string) (int64, error) -} - -/* -Store is the database store for service accounts. - -migration from apikeys to service accounts: -HideApiKeyTab is used to hide the api key tab in the UI. -MigrateApiKeysToServiceAccounts migrates all API keys to service accounts. -MigrateApiKey migrates a single API key to a service account. - -// only used for interal api calls -RevertApiKey reverts a single service account to an API key. -*/ -type Store interface { - CreateServiceAccount(ctx context.Context, orgID int64, saForm *CreateServiceAccountForm) (*ServiceAccountDTO, error) - SearchOrgServiceAccounts(ctx context.Context, orgID int64, query string, filter ServiceAccountFilter, page int, limit int, - signedInUser *user.SignedInUser) (*SearchServiceAccountsResult, error) - UpdateServiceAccount(ctx context.Context, orgID, serviceAccountID int64, - saForm *UpdateServiceAccountForm) (*ServiceAccountProfileDTO, error) 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 - 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, saId int64, keyId int64) error - ListTokens(ctx context.Context, query *GetSATokensQuery) ([]apikey.APIKey, error) - DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error - RevokeServiceAccountToken(ctx context.Context, orgId, serviceAccountId, tokenId int64) error + UpdateServiceAccount(ctx context.Context, orgID, serviceAccountID int64, + saForm *UpdateServiceAccountForm) (*ServiceAccountProfileDTO, error) AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *AddServiceAccountTokenCommand) error - GetUsageMetrics(ctx context.Context) (*Stats, error) } diff --git a/pkg/services/serviceaccounts/tests/common.go b/pkg/services/serviceaccounts/tests/common.go index 232cfc8da8b..1b5a16dd171 100644 --- a/pkg/services/serviceaccounts/tests/common.go +++ b/pkg/services/serviceaccounts/tests/common.go @@ -101,21 +101,59 @@ func SetupApiKey(t *testing.T, sqlStore *sqlstore.SQLStore, testKey TestApiKey) return addKeyCmd.Result } +// Service implements the API exposed methods for service accounts. +type serviceAccountStore interface { + CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) + RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error) + UpdateServiceAccount(ctx context.Context, orgID, serviceAccountID int64, + saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) + 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 + RevertApiKey(ctx context.Context, saId int64, keyId int64) error + // Service account tokens + AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error + DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error +} + // create mock for serviceaccountservice -type ServiceAccountMock struct{} +type ServiceAccountMock struct { + Store serviceAccountStore + Calls Calls + Stats *serviceaccounts.Stats + SecretScanEnabled bool + ExpectedTokens []apikey.APIKey + ExpectedError error +} + +func (s *ServiceAccountMock) CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) { + s.Calls.CreateServiceAccount = append(s.Calls.CreateServiceAccount, []interface{}{ctx, orgID, saForm}) + return s.Store.CreateServiceAccount(ctx, orgID, saForm) +} +func (s *ServiceAccountMock) DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error { + s.Calls.DeleteServiceAccount = append(s.Calls.DeleteServiceAccount, []interface{}{ctx, orgID, serviceAccountID}) + return s.Store.DeleteServiceAccount(ctx, orgID, serviceAccountID) +} + +func (s *ServiceAccountMock) RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error) { + s.Calls.RetrieveServiceAccount = append(s.Calls.RetrieveServiceAccount, []interface{}{ctx, orgID, serviceAccountID}) + return s.Store.RetrieveServiceAccount(ctx, orgID, serviceAccountID) +} + +func (s *ServiceAccountMock) UpdateServiceAccount(ctx context.Context, + orgID, serviceAccountID int64, + saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) { + s.Calls.UpdateServiceAccount = append(s.Calls.UpdateServiceAccount, []interface{}{ctx, orgID, serviceAccountID, saForm}) + return s.Store.UpdateServiceAccount(ctx, orgID, serviceAccountID, saForm) +} func (s *ServiceAccountMock) RetrieveServiceAccountIdByName(ctx context.Context, orgID int64, name string) (int64, error) { return 0, nil } - -func (s *ServiceAccountMock) CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) { - return nil, nil -} - -func (s *ServiceAccountMock) DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error { - return nil -} - func (s *ServiceAccountMock) Migrated(ctx context.Context, orgID int64) bool { return false } @@ -132,9 +170,6 @@ func SetupMockAccesscontrol(t *testing.T, return acmock } -// this is a way to see -// that the Mock implements the store interface -var _ serviceaccounts.Store = new(ServiceAccountsStoreMock) var _ serviceaccounts.Service = new(ServiceAccountMock) type Calls struct { @@ -154,95 +189,52 @@ type Calls struct { RetrieveServiceAccountIdByName []interface{} } -type ServiceAccountsStoreMock struct { - serviceaccounts.Store - Stats *serviceaccounts.Stats - Calls Calls -} - -func (s *ServiceAccountsStoreMock) RetrieveServiceAccountIdByName(ctx context.Context, orgID int64, name string) (int64, error) { - s.Calls.RetrieveServiceAccountIdByName = append(s.Calls.RetrieveServiceAccountIdByName, []interface{}{ctx, orgID, name}) - return 0, nil -} - -func (s *ServiceAccountsStoreMock) CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) { - // now we can test that the mock has these calls when we call the function - s.Calls.CreateServiceAccount = append(s.Calls.CreateServiceAccount, []interface{}{ctx, orgID, saForm}) - return nil, nil -} - -func (s *ServiceAccountsStoreMock) DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error { - // now we can test that the mock has these calls when we call the function - s.Calls.DeleteServiceAccount = append(s.Calls.DeleteServiceAccount, []interface{}{ctx, orgID, serviceAccountID}) - return nil -} - -func (s *ServiceAccountsStoreMock) HideApiKeysTab(ctx context.Context, orgID int64) error { +func (s *ServiceAccountMock) HideApiKeysTab(ctx context.Context, orgID int64) error { s.Calls.HideApiKeysTab = append(s.Calls.HideApiKeysTab, []interface{}{ctx}) return nil } -func (s *ServiceAccountsStoreMock) GetAPIKeysMigrationStatus(ctx context.Context, orgID int64) (*serviceaccounts.APIKeysMigrationStatus, error) { +func (s *ServiceAccountMock) 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 { +func (s *ServiceAccountMock) 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 { +func (s *ServiceAccountMock) 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, saId int64, keyId int64) error { +func (s *ServiceAccountMock) RevertApiKey(ctx context.Context, saId int64, keyId int64) error { s.Calls.RevertApiKey = append(s.Calls.RevertApiKey, []interface{}{ctx}) return nil } -func (s *ServiceAccountsStoreMock) ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) { +func (s *ServiceAccountMock) ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) { s.Calls.ListTokens = append(s.Calls.ListTokens, []interface{}{ctx, query.OrgID, query.ServiceAccountID}) + return s.ExpectedTokens, s.ExpectedError +} + +func (s *ServiceAccountMock) SearchOrgServiceAccounts(ctx context.Context, query *serviceaccounts.SearchOrgServiceAccountsQuery) (*serviceaccounts.SearchOrgServiceAccountsResult, error) { + s.Calls.SearchOrgServiceAccounts = append(s.Calls.SearchOrgServiceAccounts, []interface{}{ctx, query}) return nil, nil } -func (s *ServiceAccountsStoreMock) RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error) { - s.Calls.RetrieveServiceAccount = append(s.Calls.RetrieveServiceAccount, []interface{}{ctx, orgID, serviceAccountID}) - return nil, nil -} - -func (s *ServiceAccountsStoreMock) UpdateServiceAccount(ctx context.Context, - orgID, serviceAccountID int64, - saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) { - s.Calls.UpdateServiceAccount = append(s.Calls.UpdateServiceAccount, []interface{}{ctx, orgID, serviceAccountID, saForm}) - - return nil, nil -} - -func (s *ServiceAccountsStoreMock) SearchOrgServiceAccounts( - ctx context.Context, - orgID int64, - query string, - filter serviceaccounts.ServiceAccountFilter, - page int, - limit int, - user *user.SignedInUser) (*serviceaccounts.SearchServiceAccountsResult, error) { - s.Calls.SearchOrgServiceAccounts = append(s.Calls.SearchOrgServiceAccounts, []interface{}{ctx, orgID, query, page, limit, user}) - return nil, nil -} - -func (s *ServiceAccountsStoreMock) DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error { - s.Calls.DeleteServiceAccountToken = append(s.Calls.DeleteServiceAccountToken, []interface{}{ctx, orgID, serviceAccountID, tokenID}) - return nil -} - -func (s *ServiceAccountsStoreMock) AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error { +func (s *ServiceAccountMock) AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error { s.Calls.AddServiceAccountToken = append(s.Calls.AddServiceAccountToken, []interface{}{ctx, cmd}) - return nil + return s.Store.AddServiceAccountToken(ctx, serviceAccountID, cmd) } -func (s *ServiceAccountsStoreMock) GetUsageMetrics(ctx context.Context) (*serviceaccounts.Stats, error) { +func (s *ServiceAccountMock) DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error { + s.Calls.DeleteServiceAccountToken = append(s.Calls.DeleteServiceAccountToken, []interface{}{ctx, orgID, serviceAccountID, tokenID}) + return s.Store.DeleteServiceAccountToken(ctx, orgID, serviceAccountID, tokenID) +} + +func (s *ServiceAccountMock) GetUsageMetrics(ctx context.Context) (*serviceaccounts.Stats, error) { if s.Stats == nil { return &serviceaccounts.Stats{}, nil } diff --git a/pkg/services/thumbs/crawler_auth.go b/pkg/services/thumbs/crawler_auth.go index 30b74f48a83..93d604d454b 100644 --- a/pkg/services/thumbs/crawler_auth.go +++ b/pkg/services/thumbs/crawler_auth.go @@ -16,14 +16,13 @@ type CrawlerAuthSetupService interface { Setup(ctx context.Context) (CrawlerAuth, error) } -func ProvideCrawlerAuthSetupService(serviceAccounts serviceaccounts.Service, serviceAccountsStore serviceaccounts.Store, +func ProvideCrawlerAuthSetupService(serviceAccounts serviceaccounts.Service, sqlStore db.DB, orgService org.Service) *OSSCrawlerAuthSetupService { return &OSSCrawlerAuthSetupService{ serviceAccountNamePrefix: "dashboard-previews-crawler-org-", serviceAccounts: serviceAccounts, log: log.New("oss_crawler_account_setup_service"), sqlStore: sqlStore, - serviceAccountsStore: serviceAccountsStore, orgService: orgService, } } @@ -32,7 +31,6 @@ type OSSCrawlerAuthSetupService struct { log log.Logger serviceAccountNamePrefix string serviceAccounts serviceaccounts.Service - serviceAccountsStore serviceaccounts.Store sqlStore db.DB orgService org.Service } @@ -117,7 +115,7 @@ func (o *OSSCrawlerAuthSetupService) Setup(ctx context.Context) (CrawlerAuth, er } // update org_role to make sure everything works properly if someone has changed the role since SA's original creation - dto, err := o.serviceAccountsStore.UpdateServiceAccount(ctx, orgId, id, &serviceaccounts.UpdateServiceAccountForm{ + dto, err := o.serviceAccounts.UpdateServiceAccount(ctx, orgId, id, &serviceaccounts.UpdateServiceAccountForm{ Name: &serviceAccountNameOrg, Role: &orgRole, })