Service accounts: fix usage of errutil errors and convert more errors to errutil (#64299)

* fix usage of errutil errors and convert more errors to errutil

* fix tests
This commit is contained in:
Ieva 2023-03-08 11:32:09 +00:00 committed by GitHub
parent 312117bdfe
commit 1d1f58f0ed
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 39 additions and 100 deletions

View File

@ -2,7 +2,6 @@ package api
import (
"context"
"errors"
"net/http"
"strconv"
@ -114,22 +113,12 @@ func (api *ServiceAccountsAPI) CreateServiceAccount(c *contextmodel.ReqContext)
}
if err := api.validateRole(cmd.Role, &c.OrgRole); err != nil {
switch {
case errors.Is(err, serviceaccounts.ErrServiceAccountInvalidRole):
return response.Error(http.StatusBadRequest, err.Error(), err)
case errors.Is(err, serviceaccounts.ErrServiceAccountRolePrivilegeDenied):
return response.Error(http.StatusForbidden, err.Error(), err)
default:
return response.Error(http.StatusInternalServerError, "failed to create service account", err)
}
return response.ErrOrFallback(http.StatusInternalServerError, "failed to create service account", err)
}
serviceAccount, err := api.service.CreateServiceAccount(c.Req.Context(), c.OrgID, &cmd)
switch {
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)
if err != nil {
return response.ErrOrFallback(http.StatusInternalServerError, "Failed to create service account", err)
}
if !api.accesscontrol.IsDisabled() {
@ -169,12 +158,7 @@ func (api *ServiceAccountsAPI) RetrieveServiceAccount(ctx *contextmodel.ReqConte
serviceAccount, err := api.service.RetrieveServiceAccount(ctx.Req.Context(), ctx.OrgID, scopeID)
if err != nil {
switch {
case errors.Is(err, serviceaccounts.ErrServiceAccountNotFound):
return response.Error(http.StatusNotFound, "Failed to retrieve service account", err)
default:
return response.Error(http.StatusInternalServerError, "Failed to retrieve service account", err)
}
return response.ErrOrFallback(http.StatusInternalServerError, "Failed to retrieve service account", err)
}
saIDString := strconv.FormatInt(serviceAccount.Id, 10)
@ -220,24 +204,12 @@ func (api *ServiceAccountsAPI) UpdateServiceAccount(c *contextmodel.ReqContext)
}
if err := api.validateRole(cmd.Role, &c.OrgRole); err != nil {
switch {
case errors.Is(err, serviceaccounts.ErrServiceAccountInvalidRole):
return response.Error(http.StatusBadRequest, err.Error(), err)
case errors.Is(err, serviceaccounts.ErrServiceAccountRolePrivilegeDenied):
return response.Error(http.StatusForbidden, err.Error(), err)
default:
return response.Error(http.StatusInternalServerError, "failed to update service account", err)
}
return response.ErrOrFallback(http.StatusInternalServerError, "failed to update service account", err)
}
resp, err := api.service.UpdateServiceAccount(c.Req.Context(), c.OrgID, scopeID, &cmd)
if err != nil {
switch {
case errors.Is(err, serviceaccounts.ErrServiceAccountNotFound):
return response.Error(http.StatusNotFound, "Failed to retrieve service account", err)
default:
return response.Error(http.StatusInternalServerError, "Failed update service account", err)
}
return response.ErrOrFallback(http.StatusInternalServerError, "Failed update service account", err)
}
saIDString := strconv.FormatInt(resp.Id, 10)
@ -255,10 +227,10 @@ func (api *ServiceAccountsAPI) UpdateServiceAccount(c *contextmodel.ReqContext)
func (api *ServiceAccountsAPI) validateRole(r *org.RoleType, orgRole *org.RoleType) error {
if r != nil && !r.IsValid() {
return serviceaccounts.ErrServiceAccountInvalidRole
return serviceaccounts.ErrServiceAccountInvalidRole.Errorf("invalid role specified")
}
if r != nil && !orgRole.Includes(*r) {
return serviceaccounts.ErrServiceAccountRolePrivilegeDenied
return serviceaccounts.ErrServiceAccountRolePrivilegeDenied.Errorf("can not assign a role higher than user's role")
}
return nil
}

View File

@ -1,7 +1,6 @@
package api
import (
"errors"
"net/http"
"strconv"
"time"
@ -9,7 +8,6 @@ import (
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
apikeygenprefix "github.com/grafana/grafana/pkg/components/apikeygenprefixed"
"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/web"
@ -133,17 +131,12 @@ func (api *ServiceAccountsAPI) CreateToken(c *contextmodel.ReqContext) response.
}
// confirm service account exists
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)
default:
return response.Error(http.StatusInternalServerError, "Failed to retrieve service account", err)
}
if _, err = api.service.RetrieveServiceAccount(c.Req.Context(), c.OrgID, saID); err != nil {
return response.ErrOrFallback(http.StatusInternalServerError, "Failed to retrieve service account", err)
}
cmd := serviceaccounts.AddServiceAccountTokenCommand{}
if err := web.Bind(c.Req, &cmd); err != nil {
if err = web.Bind(c.Req, &cmd); err != nil {
return response.Error(http.StatusBadRequest, "Bad request data", err)
}
@ -176,13 +169,7 @@ 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, serviceaccounts.ErrInvalidTokenExpiration) {
return response.Error(http.StatusBadRequest, err.Error(), nil)
}
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)
return response.ErrOrFallback(http.StatusInternalServerError, "failed to add service account token", err)
}
result := &dtos.NewApiKeyResult{
@ -218,12 +205,7 @@ func (api *ServiceAccountsAPI) DeleteToken(c *contextmodel.ReqContext) response.
// confirm service account exists
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)
default:
return response.Error(http.StatusInternalServerError, "Failed to retrieve service account", err)
}
return response.ErrOrFallback(http.StatusInternalServerError, "Failed to retrieve service account", err)
}
tokenID, err := strconv.ParseInt(web.Params(c.Req)[":tokenId"], 10, 64)
@ -232,14 +214,7 @@ func (api *ServiceAccountsAPI) DeleteToken(c *contextmodel.ReqContext) response.
}
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
} else {
err = apikey.ErrNotFound
}
return response.Error(status, failedToDeleteMsg, err)
return response.ErrOrFallback(http.StatusInternalServerError, failedToDeleteMsg, err)
}
return response.Success("Service account token deleted")

View File

@ -92,7 +92,7 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) {
body: `{"name": "test"}`,
tokenTTL: -1,
permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}},
expectedErr: serviceaccounts.ErrServiceAccountNotFound,
expectedErr: serviceaccounts.ErrServiceAccountNotFound.Errorf(""),
expectedCode: http.StatusNotFound,
},
{
@ -155,7 +155,7 @@ func TestServiceAccountsAPI_DeleteToken(t *testing.T) {
saID: 1,
apikeyID: 1,
permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}},
expectedErr: serviceaccounts.ErrServiceAccountNotFound,
expectedErr: serviceaccounts.ErrServiceAccountNotFound.Errorf(""),
expectedCode: http.StatusNotFound,
},
}

View File

@ -160,7 +160,7 @@ func (s *ServiceAccountsStoreImpl) deleteServiceAccount(sess *db.Session, orgId,
return err
}
if !has {
return serviceaccounts.ErrServiceAccountNotFound
return serviceaccounts.ErrServiceAccountNotFound.Errorf("service account with id %d not found", serviceAccountId)
}
for _, sql := range ServiceAccountDeletions(s.sqlStore.GetDialect()) {
_, err := sess.Exec(sql, user.ID)
@ -211,7 +211,7 @@ func (s *ServiceAccountsStoreImpl) RetrieveServiceAccount(ctx context.Context, o
if ok, err := sess.Get(serviceAccount); err != nil {
return err
} else if !ok {
return serviceaccounts.ErrServiceAccountNotFound
return serviceaccounts.ErrServiceAccountNotFound.Errorf("service account with id %d not found", serviceAccountId)
}
return nil
@ -248,7 +248,7 @@ func (s *ServiceAccountsStoreImpl) RetrieveServiceAccountIdByName(ctx context.Co
if ok, err := sess.Get(serviceAccount); err != nil {
return err
} else if !ok {
return serviceaccounts.ErrServiceAccountNotFound
return serviceaccounts.ErrServiceAccountNotFound.Errorf("service account with name %s not found", name)
}
return nil

View File

@ -61,9 +61,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 serviceaccounts.ErrDuplicateToken
return serviceaccounts.ErrDuplicateToken.Errorf("service account token with name %s already exists in the organization", cmd.Name)
case errors.Is(err, apikey.ErrInvalidExpiration):
return serviceaccounts.ErrInvalidTokenExpiration
return serviceaccounts.ErrInvalidTokenExpiration.Errorf("invalid service account token expiration value %d", cmd.SecondsToLive)
}
return err
@ -84,7 +84,7 @@ func (s *ServiceAccountsStoreImpl) DeleteServiceAccountToken(ctx context.Context
}
affected, err := result.RowsAffected()
if affected == 0 {
return serviceaccounts.ErrServiceAccountTokenNotFound
return serviceaccounts.ErrServiceAccountTokenNotFound.Errorf("service account token with id %d not found", tokenId)
}
return err
@ -101,7 +101,7 @@ func (s *ServiceAccountsStoreImpl) RevokeServiceAccountToken(ctx context.Context
}
affected, err := result.RowsAffected()
if affected == 0 {
return serviceaccounts.ErrServiceAccountTokenNotFound
return serviceaccounts.ErrServiceAccountTokenNotFound.Errorf("service account token with id %d not found for service account with id %d", tokenId, serviceAccountId)
}
return err

View File

@ -1,14 +0,0 @@
package serviceaccounts
import "errors"
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")
)

View File

@ -244,25 +244,25 @@ func (sa *ServiceAccountsService) MigrateApiKeysToServiceAccounts(ctx context.Co
func validOrgID(orgID int64) error {
if orgID == 0 {
return serviceaccounts.ErrServiceAccountInvalidOrgID
return serviceaccounts.ErrServiceAccountInvalidOrgID.Errorf("invalid org ID 0 has been specified")
}
return nil
}
func validServiceAccountID(serviceaccountID int64) error {
if serviceaccountID == 0 {
return serviceaccounts.ErrServiceAccountInvalidID
return serviceaccounts.ErrServiceAccountInvalidID.Errorf("invalid service account ID 0 has been specified")
}
return nil
}
func validServiceAccountTokenID(tokenID int64) error {
if tokenID == 0 {
return serviceaccounts.ErrServiceAccountInvalidTokenID
return serviceaccounts.ErrServiceAccountInvalidTokenID.Errorf("invalid service account token ID 0 has been specified")
}
return nil
}
func validAPIKeyID(apiKeyID int64) error {
if apiKeyID == 0 {
return serviceaccounts.ErrServiceAccountInvalidAPIKeyID
return serviceaccounts.ErrServiceAccountInvalidAPIKeyID.Errorf("invalid API key ID 0 has been specified")
}
return nil
}

View File

@ -24,11 +24,17 @@ const (
)
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"))
ErrServiceAccountNotFound = errutil.NewBase(errutil.StatusNotFound, "serviceaccounts.ErrNotFound", errutil.WithPublicMessage("service account not found"))
ErrServiceAccountInvalidRole = errutil.NewBase(errutil.StatusBadRequest, "serviceaccounts.ErrInvalidRoleSpecified", errutil.WithPublicMessage("invalid role specified"))
ErrServiceAccountRolePrivilegeDenied = errutil.NewBase(errutil.StatusForbidden, "serviceaccounts.ErrRoleForbidden", errutil.WithPublicMessage("can not assign a role higher than user's role"))
ErrServiceAccountInvalidOrgID = errutil.NewBase(errutil.StatusBadRequest, "serviceaccounts.ErrInvalidOrgId", errutil.WithPublicMessage("invalid org id specified"))
ErrServiceAccountInvalidID = errutil.NewBase(errutil.StatusBadRequest, "serviceaccounts.ErrInvalidId", errutil.WithPublicMessage("invalid service account id specified"))
ErrServiceAccountInvalidAPIKeyID = errutil.NewBase(errutil.StatusBadRequest, "serviceaccounts.ErrInvalidAPIKeyId", errutil.WithPublicMessage("invalid api key id specified"))
ErrServiceAccountInvalidTokenID = errutil.NewBase(errutil.StatusBadRequest, "serviceaccounts.ErrInvalidTokenId", errutil.WithPublicMessage("invalid service account token id specified"))
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"))
)
type ServiceAccount struct {

View File

@ -366,7 +366,7 @@ func (s *Service) CreateServiceAccount(ctx context.Context, cmd *user.CreateUser
cmd.Email = cmd.Login
err := s.store.LoginConflict(ctx, cmd.Login, cmd.Email, s.cfg.CaseInsensitiveLogin)
if err != nil {
return nil, serviceaccounts.ErrServiceAccountAlreadyExists
return nil, serviceaccounts.ErrServiceAccountAlreadyExists.Errorf("service account with login %s already exists", cmd.Login)
}
// create user