diff --git a/pkg/services/serviceaccounts/api/api.go b/pkg/services/serviceaccounts/api/api.go index 706a61eb8be..83c9f5dbdef 100644 --- a/pkg/services/serviceaccounts/api/api.go +++ b/pkg/services/serviceaccounts/api/api.go @@ -16,7 +16,6 @@ import ( contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "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/setting" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/web" @@ -133,7 +132,7 @@ func (api *ServiceAccountsAPI) CreateServiceAccount(c *contextmodel.ReqContext) serviceAccount, err := api.service.CreateServiceAccount(c.Req.Context(), c.OrgID, &cmd) switch { - case errors.Is(err, database.ErrServiceAccountAlreadyExists): + case errors.Is(err, serviceaccounts.ErrServiceAccountAlreadyExists): return response.Error(http.StatusBadRequest, "Failed to create service account", err) case err != nil: return response.Error(http.StatusInternalServerError, "Failed to create service account", err) diff --git a/pkg/services/serviceaccounts/api/token.go b/pkg/services/serviceaccounts/api/token.go index 516ea8e6049..9c4bc2918c7 100644 --- a/pkg/services/serviceaccounts/api/token.go +++ b/pkg/services/serviceaccounts/api/token.go @@ -12,7 +12,6 @@ import ( "github.com/grafana/grafana/pkg/services/apikey" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/serviceaccounts" - "github.com/grafana/grafana/pkg/services/serviceaccounts/database" "github.com/grafana/grafana/pkg/web" ) @@ -177,10 +176,10 @@ func (api *ServiceAccountsAPI) CreateToken(c *contextmodel.ReqContext) response. apiKey, err := api.service.AddServiceAccountToken(c.Req.Context(), saID, &cmd) if err != nil { - if errors.Is(err, database.ErrInvalidTokenExpiration) { + if errors.Is(err, serviceaccounts.ErrInvalidTokenExpiration) { return response.Error(http.StatusBadRequest, err.Error(), nil) } - if errors.Is(err, database.ErrDuplicateToken) { + if errors.Is(err, serviceaccounts.ErrDuplicateToken) { return response.Error(http.StatusConflict, err.Error(), nil) } return response.Error(http.StatusInternalServerError, "Failed to add service account token", err) diff --git a/pkg/services/serviceaccounts/database/errors.go b/pkg/services/serviceaccounts/database/errors.go deleted file mode 100644 index 76560c4bc71..00000000000 --- a/pkg/services/serviceaccounts/database/errors.go +++ /dev/null @@ -1,13 +0,0 @@ -package database - -import ( - "errors" -) - -var ( - ErrServiceAccountAlreadyExists = errors.New("service account already exists") - ErrServiceAccountTokenNotFound = errors.New("service account token not found") - ErrInvalidTokenExpiration = errors.New("invalid SecondsToLive value") - ErrDuplicateToken = errors.New("service account token with given name already exists in the organization") - ErrServiceAccountAndTokenMismatch = errors.New("API token does not belong to the given service account") -) diff --git a/pkg/services/serviceaccounts/database/store.go b/pkg/services/serviceaccounts/database/store.go index c2a16aa436a..599fda381ac 100644 --- a/pkg/services/serviceaccounts/database/store.go +++ b/pkg/services/serviceaccounts/database/store.go @@ -3,7 +3,6 @@ package database //nolint:goimports import ( "context" - "errors" "fmt" "strings" "time" @@ -55,40 +54,17 @@ func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, org if saForm.Role != nil { role = *saForm.Role } - var newSA *user.User - createErr := s.sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) (err error) { - var errUser error - newSA, errUser = s.userService.CreateServiceAccount(ctx, &user.CreateUserCommand{ - Login: generatedLogin, - OrgID: orgId, - Name: saForm.Name, - IsDisabled: isDisabled, - IsServiceAccount: true, - SkipOrgSetup: true, - }) - if errUser != nil { - return errUser - } - errAddOrgUser := s.orgService.AddOrgUser(ctx, &org.AddOrgUserCommand{ - Role: role, - OrgID: orgId, - UserID: newSA.ID, - AllowAddingServiceAccount: true, - }) - if errAddOrgUser != nil { - return errAddOrgUser - } - - return nil + newSA, err := s.userService.CreateServiceAccount(ctx, &user.CreateUserCommand{ + Login: generatedLogin, + OrgID: orgId, + Name: saForm.Name, + IsDisabled: isDisabled, + IsServiceAccount: true, + DefaultOrgRole: string(role), }) - - if createErr != nil { - if errors.Is(createErr, user.ErrUserAlreadyExists) { - return nil, ErrServiceAccountAlreadyExists - } - - return nil, fmt.Errorf("failed to create service account: %w", createErr) + if err != nil { + return nil, fmt.Errorf("failed to create service account: %w", err) } return &serviceaccounts.ServiceAccountDTO{ @@ -473,7 +449,7 @@ func (s *ServiceAccountsStoreImpl) RevertApiKey(ctx context.Context, saId int64, } if *key.ServiceAccountId != saId { - return ErrServiceAccountAndTokenMismatch + return serviceaccounts.ErrServiceAccountAndTokenMismatch } tokens, err := s.ListTokens(ctx, &serviceaccounts.GetSATokensQuery{ diff --git a/pkg/services/serviceaccounts/database/store_test.go b/pkg/services/serviceaccounts/database/store_test.go index 0f0b11ae843..220f6d0cfd4 100644 --- a/pkg/services/serviceaccounts/database/store_test.go +++ b/pkg/services/serviceaccounts/database/store_test.go @@ -328,7 +328,7 @@ func TestStore_RevertApiKey(t *testing.T) { desc: "should fail reverting to api key when the token is assigned to a different service account", key: tests.TestApiKey{Name: "Test1", Role: org.RoleEditor, OrgId: 1}, forceMismatchServiceAccount: true, - expectedErr: ErrServiceAccountAndTokenMismatch, + expectedErr: serviceaccounts.ErrServiceAccountAndTokenMismatch, }, } diff --git a/pkg/services/serviceaccounts/database/token_store.go b/pkg/services/serviceaccounts/database/token_store.go index 24a00c7b34f..2965ed337a4 100644 --- a/pkg/services/serviceaccounts/database/token_store.go +++ b/pkg/services/serviceaccounts/database/token_store.go @@ -58,9 +58,9 @@ func (s *ServiceAccountsStoreImpl) AddServiceAccountToken(ctx context.Context, s if err := s.apiKeyService.AddAPIKey(ctx, addKeyCmd); err != nil { switch { case errors.Is(err, apikey.ErrDuplicate): - return ErrDuplicateToken + return serviceaccounts.ErrDuplicateToken case errors.Is(err, apikey.ErrInvalidExpiration): - return ErrInvalidTokenExpiration + return serviceaccounts.ErrInvalidTokenExpiration } return err @@ -81,7 +81,7 @@ func (s *ServiceAccountsStoreImpl) DeleteServiceAccountToken(ctx context.Context } affected, err := result.RowsAffected() if affected == 0 { - return ErrServiceAccountTokenNotFound + return serviceaccounts.ErrServiceAccountTokenNotFound } return err @@ -98,7 +98,7 @@ func (s *ServiceAccountsStoreImpl) RevokeServiceAccountToken(ctx context.Context } affected, err := result.RowsAffected() if affected == 0 { - return ErrServiceAccountTokenNotFound + return serviceaccounts.ErrServiceAccountTokenNotFound } return err diff --git a/pkg/services/serviceaccounts/models.go b/pkg/services/serviceaccounts/models.go index 1239cb72650..37713d2a7cd 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/org" "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/util/errutil" ) var ( @@ -22,6 +23,14 @@ const ( ActionPermissionsWrite = "serviceaccounts.permissions:write" ) +var ( + ErrServiceAccountAlreadyExists = errutil.NewBase(errutil.StatusBadRequest, "serviceaccounts.ErrAlreadyExists", errutil.WithPublicMessage("service account already exists")) + ErrServiceAccountTokenNotFound = errutil.NewBase(errutil.StatusNotFound, "serviceaccounts.ErrTokenNotFound", errutil.WithPublicMessage("service account token not found")) + ErrInvalidTokenExpiration = errutil.NewBase(errutil.StatusValidationFailed, "serviceaccounts.ErrInvalidInput", errutil.WithPublicMessage("invalid SecondsToLive value")) + ErrDuplicateToken = errutil.NewBase(errutil.StatusBadRequest, "serviceaccounts.ErrTokenAlreadyExists", errutil.WithPublicMessage("service account token with given name already exists in the organization")) + ErrServiceAccountAndTokenMismatch = errutil.NewBase(errutil.StatusBadRequest, "serviceaccounts.ErrToeknMismatch", errutil.WithPublicMessage("API token does not belong to the given service account")) +) + type ServiceAccount struct { Id int64 } diff --git a/pkg/services/thumbs/crawler_auth.go b/pkg/services/thumbs/crawler_auth.go index 93d604d454b..d1fd757b902 100644 --- a/pkg/services/thumbs/crawler_auth.go +++ b/pkg/services/thumbs/crawler_auth.go @@ -9,7 +9,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/serviceaccounts" - "github.com/grafana/grafana/pkg/services/serviceaccounts/database" ) type CrawlerAuthSetupService interface { @@ -98,7 +97,7 @@ func (o *OSSCrawlerAuthSetupService) Setup(ctx context.Context) (CrawlerAuth, er } serviceAccount, err := o.serviceAccounts.CreateServiceAccount(ctx, orgId, &saForm) - accountAlreadyExists := errors.Is(err, database.ErrServiceAccountAlreadyExists) + accountAlreadyExists := errors.Is(err, serviceaccounts.ErrServiceAccountAlreadyExists) if !accountAlreadyExists && err != nil { o.log.Error("Failed to create the service account", "err", err, "accountName", serviceAccountNameOrg, "orgId", orgId) diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index 354374bb536..fc7912a5b3b 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -14,6 +14,7 @@ import ( ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/quota" + "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/supportbundles" "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/services/user" @@ -467,26 +468,12 @@ func (s *Service) getOrgIDForNewUser(ctx context.Context, cmd *user.CreateUserCo return orgID, err } -// CreateServiceAccount is a copy of Create with a single difference; it will create the OrgUser service account. +// CreateServiceAccount creates a service account in the user table and adds service account to an organisation in the org_user table func (s *Service) CreateServiceAccount(ctx context.Context, cmd *user.CreateUserCommand) (*user.User, error) { - cmdOrg := org.GetOrgIDForNewUserCommand{ - Email: cmd.Email, - Login: cmd.Login, - OrgID: cmd.OrgID, - OrgName: cmd.OrgName, - SkipOrgSetup: cmd.SkipOrgSetup, - } - orgID, err := s.orgService.GetIDForNewUser(ctx, cmdOrg) + cmd.Email = cmd.Login + err := s.store.LoginConflict(ctx, cmd.Login, cmd.Email, s.cfg.CaseInsensitiveLogin) if err != nil { - return nil, err - } - if cmd.Email == "" { - cmd.Email = cmd.Login - } - - err = s.store.LoginConflict(ctx, cmd.Login, cmd.Email, s.cfg.CaseInsensitiveLogin) - if err != nil { - return nil, err + return nil, serviceaccounts.ErrServiceAccountAlreadyExists } // create user @@ -494,15 +481,12 @@ func (s *Service) CreateServiceAccount(ctx context.Context, cmd *user.CreateUser Email: cmd.Email, Name: cmd.Name, Login: cmd.Login, - Company: cmd.Company, - IsAdmin: cmd.IsAdmin, IsDisabled: cmd.IsDisabled, OrgID: cmd.OrgID, - EmailVerified: cmd.EmailVerified, Created: time.Now(), Updated: time.Now(), LastSeenAt: time.Now().AddDate(-10, 0, 0), - IsServiceAccount: cmd.IsServiceAccount, + IsServiceAccount: true, } salt, err := util.GetRandomString(10) @@ -516,40 +500,22 @@ func (s *Service) CreateServiceAccount(ctx context.Context, cmd *user.CreateUser } usr.Rands = rands - if len(cmd.Password) > 0 { - encodedPassword, err := util.EncodePassword(cmd.Password, usr.Salt) - if err != nil { - return nil, err - } - usr.Password = encodedPassword - } - _, err = s.store.Insert(ctx, usr) if err != nil { return nil, err } // create org user link - if !cmd.SkipOrgSetup { - orgCmd := &org.AddOrgUserCommand{ - OrgID: orgID, - UserID: usr.ID, - Role: org.RoleAdmin, - AllowAddingServiceAccount: true, - } - - if s.cfg.AutoAssignOrg && !usr.IsAdmin { - if len(cmd.DefaultOrgRole) > 0 { - orgCmd.Role = org.RoleType(cmd.DefaultOrgRole) - } else { - orgCmd.Role = org.RoleType(s.cfg.AutoAssignOrgRole) - } - } - - if err = s.orgService.AddOrgUser(ctx, orgCmd); err != nil { - return nil, err - } + orgCmd := &org.AddOrgUserCommand{ + OrgID: cmd.OrgID, + UserID: usr.ID, + Role: org.RoleType(cmd.DefaultOrgRole), + AllowAddingServiceAccount: true, } + if err = s.orgService.AddOrgUser(ctx, orgCmd); err != nil { + return nil, err + } + return usr, nil }