diff --git a/pkg/services/serviceaccounts/api/api.go b/pkg/services/serviceaccounts/api/api.go index 322fc34d9ad..e5e5a2c4022 100644 --- a/pkg/services/serviceaccounts/api/api.go +++ b/pkg/services/serviceaccounts/api/api.go @@ -101,17 +101,6 @@ func (api *ServiceAccountsAPI) CreateServiceAccount(c *contextmodel.ReqContext) if api.cfg.RBAC.PermissionsOnCreation("service-account") { if c.SignedInUser.IsIdentityType(claims.TypeUser) { - userID, err := c.SignedInUser.GetInternalID() - if err != nil { - return response.Error(http.StatusInternalServerError, "Failed to parse user id", err) - } - - if _, err := api.permissionService.SetUserPermission(c.Req.Context(), - c.SignedInUser.GetOrgID(), accesscontrol.User{ID: userID}, - strconv.FormatInt(serviceAccount.Id, 10), "Admin"); err != nil { - return response.Error(http.StatusInternalServerError, "Failed to set permissions for service account creator", err) - } - // Clear permission cache for the user who's created the service account, so that new permissions are fetched for their next call // Required for cases when caller wants to immediately interact with the newly created object api.accesscontrolService.ClearUserPermissionCache(c.SignedInUser) diff --git a/pkg/services/serviceaccounts/manager/service.go b/pkg/services/serviceaccounts/manager/service.go index 5fbae6c88ef..0a0a74bc2c6 100644 --- a/pkg/services/serviceaccounts/manager/service.go +++ b/pkg/services/serviceaccounts/manager/service.go @@ -4,8 +4,12 @@ import ( "context" "errors" "fmt" + "strconv" "time" + "github.com/grafana/authlib/claims" + + "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/infra/log" @@ -29,6 +33,8 @@ type ServiceAccountsService struct { acService accesscontrol.Service permissions accesscontrol.ServiceAccountPermissionsService + cfg *setting.Cfg + db db.DB store store log log.Logger backgroundLog log.Logger @@ -58,6 +64,8 @@ func ProvideServiceAccountsService( orgService, ) s := &ServiceAccountsService{ + cfg: cfg, + db: store, acService: acService, permissions: permissions, store: serviceAccountsStore, @@ -150,7 +158,39 @@ func (sa *ServiceAccountsService) CreateServiceAccount(ctx context.Context, orgI if err := validOrgID(orgID); err != nil { return nil, err } - return sa.store.CreateServiceAccount(ctx, orgID, saForm) + + var serviceAccount *serviceaccounts.ServiceAccountDTO + err := sa.db.InTransaction(ctx, func(ctx context.Context) error { + var err error + serviceAccount, err = sa.store.CreateServiceAccount(ctx, orgID, saForm) + if err != nil { + return err + } + + user, err := identity.GetRequester(ctx) + if err == nil && sa.cfg.RBAC.PermissionsOnCreation("service-account") { + if user.IsIdentityType(claims.TypeUser) { + userID, err := user.GetInternalID() + if err != nil { + return err + } + + if _, err := sa.permissions.SetUserPermission(ctx, + orgID, accesscontrol.User{ID: userID}, + strconv.FormatInt(serviceAccount.Id, 10), "Admin"); err != nil { + return err + } + } + } + + return nil + }) + + if err != nil { + return nil, err + } + + return serviceAccount, nil } func (sa *ServiceAccountsService) RetrieveServiceAccount(ctx context.Context, orgID int64, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error) { diff --git a/pkg/services/serviceaccounts/manager/service_test.go b/pkg/services/serviceaccounts/manager/service_test.go index 579ee7ff598..4115b9b3e25 100644 --- a/pkg/services/serviceaccounts/manager/service_test.go +++ b/pkg/services/serviceaccounts/manager/service_test.go @@ -4,13 +4,15 @@ import ( "context" "testing" - "github.com/grafana/grafana/pkg/services/accesscontrol/actest" "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/accesscontrol/actest" "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/tests/testsuite" ) type FakeServiceAccountStore struct { @@ -116,11 +118,22 @@ func (f *SecretsCheckerFake) CheckTokens(ctx context.Context) error { return f.ExpectedError } +func TestMain(m *testing.M) { + testsuite.Run(m) +} + func TestProvideServiceAccount_DeleteServiceAccount(t *testing.T) { storeMock := newServiceAccountStoreFake() acSvc := actest.FakeService{} pSvc := &actest.FakePermissionsService{} - svc := ServiceAccountsService{acSvc, pSvc, storeMock, log.NewNopLogger(), log.NewNopLogger(), &SecretsCheckerFake{}, false, 0} + svc := ServiceAccountsService{ + acService: acSvc, + permissions: pSvc, + store: storeMock, + db: db.InitTestDB(t), + log: log.NewNopLogger(), + secretScanEnabled: false, + } testOrgId := 1 t.Run("should create service account", func(t *testing.T) { diff --git a/pkg/services/serviceaccounts/manager/stats_test.go b/pkg/services/serviceaccounts/manager/stats_test.go index 43698a18494..034b75bc429 100644 --- a/pkg/services/serviceaccounts/manager/stats_test.go +++ b/pkg/services/serviceaccounts/manager/stats_test.go @@ -16,7 +16,15 @@ func Test_UsageStats(t *testing.T) { acSvc := actest.FakeService{} pSvc := actest.FakePermissionsService{} storeMock := newServiceAccountStoreFake() - svc := ServiceAccountsService{acSvc, &pSvc, storeMock, log.NewNopLogger(), log.NewNopLogger(), &SecretsCheckerFake{}, true, 5} + svc := ServiceAccountsService{ + acService: acSvc, + permissions: &pSvc, + store: storeMock, + log: log.NewNopLogger(), + secretScanService: &SecretsCheckerFake{}, + secretScanEnabled: true, + secretScanInterval: 5, + } err := svc.DeleteServiceAccount(context.Background(), 1, 1) require.NoError(t, err)