IAM - Fix error messages for resource permissions endpoints (#85773)

* IAM: fix many error messages in access-related code to provide more information

* Remove debug statement

* Refactor resourcepermissions package to use errutil

* Replace a few more errors with errutil and wrap errors found in users and teams services

* Apply diff of openAPI spec
This commit is contained in:
Aaron Godin 2024-04-17 08:53:28 -05:00 committed by GitHub
parent 60abf01526
commit d409d8e860
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 140 additions and 26 deletions

View File

@ -112,7 +112,7 @@ func TestService_DeclareFixedRoles(t *testing.T) {
},
},
wantErr: true,
err: accesscontrol.ErrInvalidBuiltinRole,
err: accesscontrol.ErrInvalidBuiltinRole.Build(accesscontrol.ErrInvalidBuiltinRoleData("WrongAdmin")),
},
{
name: "should add multiple registrations at once",
@ -224,7 +224,7 @@ func TestService_DeclarePluginRoles(t *testing.T) {
},
},
wantErr: true,
err: accesscontrol.ErrInvalidBuiltinRole,
err: accesscontrol.ErrInvalidBuiltinRole.Build(accesscontrol.ErrInvalidBuiltinRoleData("WrongAdmin")),
},
{
name: "should add multiple registrations at once",

View File

@ -3,18 +3,48 @@ package accesscontrol
import (
"errors"
"fmt"
"github.com/grafana/grafana/pkg/util/errutil"
)
const (
invalidBuiltInRoleMessage = `built-in role [{{ .Public.builtInRole }}] is not valid`
assignmentEntityNotFoundMessage = `{{ .Public.assignment }} not found`
)
var (
ErrInvalidBuiltinRole = errutil.BadRequest("accesscontrol.invalidBuiltInRole").
MustTemplate(invalidBuiltInRoleMessage, errutil.WithPublic(invalidBuiltInRoleMessage))
ErrNoneRoleAssignment = errutil.BadRequest("accesscontrol.noneRoleAssignment", errutil.WithPublicMessage("none role cannot receive permissions"))
ErrAssignmentEntityNotFound = errutil.BadRequest("accesscontrol.assignmentEntityNotFound").
MustTemplate(assignmentEntityNotFoundMessage, errutil.WithPublic(assignmentEntityNotFoundMessage))
// Note: these are intended to be replaced by equivalent errutil implementations.
// Avoid creating new errors with errors.New and prefer errutil
ErrInvalidRequestBody = errors.New("invalid request body")
ErrFixedRolePrefixMissing = errors.New("fixed role should be prefixed with '" + FixedRolePrefix + "'")
ErrInvalidBuiltinRole = errors.New("built-in role is not valid")
ErrNoneRoleAssignment = errors.New("none role cannot receive permissions")
ErrInvalidScope = errors.New("invalid scope")
ErrResolverNotFound = errors.New("no resolver found")
ErrPluginIDRequired = errors.New("plugin ID is required")
ErrRoleNotFound = errors.New("role not found")
)
func ErrInvalidBuiltinRoleData(builtInRole string) errutil.TemplateData {
return errutil.TemplateData{
Public: map[string]any{
"builtInRole": builtInRole,
},
}
}
func ErrAssignmentEntityNotFoundData(assignment string) errutil.TemplateData {
return errutil.TemplateData{
Public: map[string]any{
"assignment": assignment,
},
}
}
type ErrorInvalidRole struct{}
func (e *ErrorInvalidRole) Error() string {

View File

@ -14,6 +14,7 @@ import (
"text/template"
"time"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/middleware/cookies"
"github.com/grafana/grafana/pkg/models/usertoken"
"github.com/grafana/grafana/pkg/services/auth/identity"
@ -180,6 +181,13 @@ func AuthorizeInOrgMiddleware(ac AccessControl, authnService authn.Service) func
return func(c *contextmodel.ReqContext) {
targetOrgID, err := getTargetOrg(c)
if err != nil {
if errors.Is(err, ErrInvalidRequestBody) {
c.JSON(http.StatusBadRequest, map[string]string{
"message": err.Error(),
"traceID": tracing.TraceIDFromContext(c.Req.Context(), false),
})
return
}
deny(c, nil, fmt.Errorf("failed to get target org: %w", err))
return
}
@ -254,6 +262,9 @@ func UseGlobalOrgFromRequestData(cfg *setting.Cfg) OrgIDGetter {
return func(c *contextmodel.ReqContext) (int64, error) {
query, err := getOrgQueryFromRequest(c)
if err != nil {
if errors.Is(err, ErrInvalidRequestBody) {
return NoOrgID, err
}
// Special case of macaron handling invalid params
return NoOrgID, org.ErrOrgNotFound.Errorf("failed to get organization from context: %w", err)
}
@ -290,7 +301,9 @@ func getOrgQueryFromRequest(c *contextmodel.ReqContext) (*QueryWithOrg, error) {
}
if err := web.Bind(req, query); err != nil {
// Special case of macaron handling invalid params
if err.Error() == "unexpected EOF" {
return nil, fmt.Errorf("%w: unexpected end of JSON input", ErrInvalidRequestBody)
}
return nil, err
}

View File

@ -144,7 +144,7 @@ func (a *api) getPermissions(c *contextmodel.ReqContext) response.Response {
permissions, err := a.service.GetPermissions(c.Req.Context(), c.SignedInUser, resourceID)
if err != nil {
return response.ErrOrFallback(http.StatusInternalServerError, "failed to get permissions", err)
return response.ErrOrFallback(http.StatusInternalServerError, "Failed to get permissions", err)
}
if a.service.options.Assignments.BuiltInRoles && !a.service.license.FeatureEnabled("accesscontrol.enforcement") {
@ -229,7 +229,7 @@ type SetResourcePermissionsForUserParams struct {
func (a *api) setUserPermission(c *contextmodel.ReqContext) response.Response {
userID, err := strconv.ParseInt(web.Params(c.Req)[":userID"], 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "userID is invalid", err)
return response.Err(ErrInvalidParam.Build(ErrInvalidParamData("userID", err)))
}
resourceID := web.Params(c.Req)[":resourceID"]
@ -240,7 +240,7 @@ func (a *api) setUserPermission(c *contextmodel.ReqContext) response.Response {
_, err = a.service.SetUserPermission(c.Req.Context(), c.SignedInUser.GetOrgID(), accesscontrol.User{ID: userID}, resourceID, cmd.Permission)
if err != nil {
return response.ErrOrFallback(http.StatusBadRequest, "failed to set user permission", err)
return response.Err(err)
}
return permissionSetResponse(cmd)
@ -282,7 +282,7 @@ type SetResourcePermissionsForTeamParams struct {
func (a *api) setTeamPermission(c *contextmodel.ReqContext) response.Response {
teamID, err := strconv.ParseInt(web.Params(c.Req)[":teamID"], 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "teamID is invalid", err)
return response.Err(ErrInvalidParam.Build(ErrInvalidParamData("teamID", err)))
}
resourceID := web.Params(c.Req)[":resourceID"]
@ -293,7 +293,7 @@ func (a *api) setTeamPermission(c *contextmodel.ReqContext) response.Response {
_, err = a.service.SetTeamPermission(c.Req.Context(), c.SignedInUser.GetOrgID(), teamID, resourceID, cmd.Permission)
if err != nil {
return response.ErrOrFallback(http.StatusBadRequest, "failed to set team permission", err)
return response.Err(err)
}
return permissionSetResponse(cmd)
@ -343,7 +343,7 @@ func (a *api) setBuiltinRolePermission(c *contextmodel.ReqContext) response.Resp
_, err := a.service.SetBuiltInRolePermission(c.Req.Context(), c.SignedInUser.GetOrgID(), builtInRole, resourceID, cmd.Permission)
if err != nil {
return response.ErrOrFallback(http.StatusBadRequest, "failed to set role permission", err)
return response.Err(err)
}
return permissionSetResponse(cmd)
@ -383,12 +383,12 @@ func (a *api) setPermissions(c *contextmodel.ReqContext) response.Response {
cmd := setPermissionsCommand{}
if err := web.Bind(c.Req, &cmd); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
return response.Error(http.StatusBadRequest, "Bad request data: "+err.Error(), err)
}
_, err := a.service.SetPermissions(c.Req.Context(), c.SignedInUser.GetOrgID(), resourceID, cmd.Permissions...)
if err != nil {
return response.ErrOrFallback(http.StatusBadRequest, "failed to set permission", err)
return response.Err(err)
}
return response.Success("Permissions updated")

View File

@ -1,8 +1,56 @@
package resourcepermissions
import "errors"
import (
"github.com/grafana/grafana/pkg/util/errutil"
)
const (
invalidPermissionMessage = `Permission [{{ .Public.permission }}] is invalid for this resource type`
invalidAssignmentMessage = `Assignment [{{ .Public.assignment }}] is invalid for this resource type`
invalidParamMessage = `Param [{{ .Public.param }}] is invalid`
invalidRequestBody = `Request body is invalid: {{ .Public.reason }}`
)
var (
ErrInvalidPermission = errors.New("invalid permission")
ErrInvalidAssignment = errors.New("invalid assignment")
ErrInvalidParam = errutil.BadRequest("resourcePermissions.invalidParam").
MustTemplate(invalidParamMessage, errutil.WithPublic(invalidParamMessage))
ErrInvalidRequestBody = errutil.BadRequest("resourcePermissions.invalidRequestBody").
MustTemplate(invalidRequestBody, errutil.WithPublic(invalidRequestBody))
ErrInvalidPermission = errutil.BadRequest("resourcePermissions.invalidPermission").
MustTemplate(invalidPermissionMessage, errutil.WithPublic(invalidPermissionMessage))
ErrInvalidAssignment = errutil.BadRequest("resourcePermissions.invalidAssignment").
MustTemplate(invalidAssignmentMessage, errutil.WithPublic(invalidAssignmentMessage))
)
func ErrInvalidParamData(param string, err error) errutil.TemplateData {
return errutil.TemplateData{
Public: map[string]any{
"param": param,
},
Error: err,
}
}
func ErrInvalidRequestBodyData(reason string) errutil.TemplateData {
return errutil.TemplateData{
Public: map[string]any{
"reason": reason,
},
}
}
func ErrInvalidPermissionData(permission string) errutil.TemplateData {
return errutil.TemplateData{
Public: map[string]any{
"permission": permission,
},
}
}
func ErrInvalidAssignmentData(assignment string) errutil.TemplateData {
return errutil.TemplateData{
Public: map[string]any{
"assignment": assignment,
},
}
}

View File

@ -2,6 +2,7 @@ package resourcepermissions
import (
"context"
"errors"
"fmt"
"sort"
@ -285,7 +286,7 @@ func (s *Service) mapPermission(permission string) ([]string, error) {
return v, nil
}
}
return nil, ErrInvalidPermission
return nil, ErrInvalidPermission.Build(ErrInvalidPermissionData(permission))
}
func (s *Service) validateResource(ctx context.Context, orgID int64, resourceID string) error {
@ -297,27 +298,37 @@ func (s *Service) validateResource(ctx context.Context, orgID int64, resourceID
func (s *Service) validateUser(ctx context.Context, orgID, userID int64) error {
if !s.options.Assignments.Users {
return ErrInvalidAssignment
return ErrInvalidAssignment.Build(ErrInvalidAssignmentData("users"))
}
_, err := s.userService.GetSignedInUser(ctx, &user.GetSignedInUserQuery{OrgID: orgID, UserID: userID})
return err
switch {
case errors.Is(err, user.ErrUserNotFound):
return accesscontrol.ErrAssignmentEntityNotFound.Build(accesscontrol.ErrAssignmentEntityNotFoundData("user"))
default:
return err
}
}
func (s *Service) validateTeam(ctx context.Context, orgID, teamID int64) error {
if !s.options.Assignments.Teams {
return ErrInvalidAssignment
return ErrInvalidAssignment.Build(ErrInvalidAssignmentData("teams"))
}
if _, err := s.teamService.GetTeamByID(ctx, &team.GetTeamByIDQuery{OrgID: orgID, ID: teamID}); err != nil {
return err
switch {
case errors.Is(err, team.ErrTeamNotFound):
return accesscontrol.ErrAssignmentEntityNotFound.Build(accesscontrol.ErrAssignmentEntityNotFoundData("team"))
default:
return err
}
}
return nil
}
func (s *Service) validateBuiltinRole(ctx context.Context, builtinRole string) error {
func (s *Service) validateBuiltinRole(_ context.Context, builtinRole string) error {
if !s.options.Assignments.BuiltInRoles {
return ErrInvalidAssignment
return ErrInvalidAssignment.Build(ErrInvalidAssignmentData("builtInRoles"))
}
if err := accesscontrol.ValidateBuiltInRoles([]string{builtinRole}); err != nil {

View File

@ -388,7 +388,7 @@ func ValidateBuiltInRoles(builtInRoles []string) error {
return ErrNoneRoleAssignment
}
if !org.RoleType(br).IsValid() && br != RoleGrafanaAdmin {
return fmt.Errorf("'%s' %w", br, ErrInvalidBuiltinRole)
return ErrInvalidBuiltinRole.Build(ErrInvalidBuiltinRoleData(br))
}
}
return nil

View File

@ -48,7 +48,7 @@
},
"/access-control/roles": {
"get": {
"description": "Gets all existing roles. The response contains all global and organization local roles, for the organization which user is signed in.\n\nYou need to have a permission with action `roles:read` and scope `roles:*`.",
"description": "Gets all existing roles. The response contains all global and organization local roles, for the organization which user is signed in.\n\nYou need to have a permission with action `roles:read` and scope `roles:*`.\n\nThe `delegatable` flag reduces the set of roles to only those for which the signed-in user has permissions to assign.",
"tags": [
"access_control",
"enterprise"
@ -60,6 +60,11 @@
"type": "boolean",
"name": "delegatable",
"in": "query"
},
{
"type": "boolean",
"name": "includeHidden",
"in": "query"
}
],
"responses": {

View File

@ -12374,7 +12374,7 @@
},
"/access-control/roles": {
"get": {
"description": "Gets all existing roles. The response contains all global and organization local roles, for the organization which user is signed in.\n\nYou need to have a permission with action `roles:read` and scope `roles:*`.",
"description": "Gets all existing roles. The response contains all global and organization local roles, for the organization which user is signed in.\n\nYou need to have a permission with action `roles:read` and scope `roles:*`.\n\nThe `delegatable` flag reduces the set of roles to only those for which the signed-in user has permissions to assign.",
"operationId": "listRoles",
"parameters": [
{
@ -12383,6 +12383,13 @@
"schema": {
"type": "boolean"
}
},
{
"in": "query",
"name": "includeHidden",
"schema": {
"type": "boolean"
}
}
],
"responses": {