diff --git a/pkg/services/accesscontrol/acimpl/service_test.go b/pkg/services/accesscontrol/acimpl/service_test.go index 3e9a1a0fc34..c222de708ca 100644 --- a/pkg/services/accesscontrol/acimpl/service_test.go +++ b/pkg/services/accesscontrol/acimpl/service_test.go @@ -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", diff --git a/pkg/services/accesscontrol/errors.go b/pkg/services/accesscontrol/errors.go index 175f5015329..1152c6f6fa0 100644 --- a/pkg/services/accesscontrol/errors.go +++ b/pkg/services/accesscontrol/errors.go @@ -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 { diff --git a/pkg/services/accesscontrol/middleware.go b/pkg/services/accesscontrol/middleware.go index f7fb8393e5f..714f37b9041 100644 --- a/pkg/services/accesscontrol/middleware.go +++ b/pkg/services/accesscontrol/middleware.go @@ -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 } diff --git a/pkg/services/accesscontrol/resourcepermissions/api.go b/pkg/services/accesscontrol/resourcepermissions/api.go index 95db0372e8a..b8c05dfc775 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api.go +++ b/pkg/services/accesscontrol/resourcepermissions/api.go @@ -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") diff --git a/pkg/services/accesscontrol/resourcepermissions/error.go b/pkg/services/accesscontrol/resourcepermissions/error.go index 53a8164a926..985fd51cf08 100644 --- a/pkg/services/accesscontrol/resourcepermissions/error.go +++ b/pkg/services/accesscontrol/resourcepermissions/error.go @@ -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, + }, + } +} diff --git a/pkg/services/accesscontrol/resourcepermissions/service.go b/pkg/services/accesscontrol/resourcepermissions/service.go index cf2e638275f..aaabb2ad9c2 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service.go +++ b/pkg/services/accesscontrol/resourcepermissions/service.go @@ -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 { diff --git a/pkg/services/accesscontrol/roles.go b/pkg/services/accesscontrol/roles.go index 72334fee499..a81feff00ab 100644 --- a/pkg/services/accesscontrol/roles.go +++ b/pkg/services/accesscontrol/roles.go @@ -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 diff --git a/public/api-merged.json b/public/api-merged.json index 2bea2e006eb..c1c20da0c81 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -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": { diff --git a/public/openapi3.json b/public/openapi3.json index 243e97eac9b..fa944a233dc 100644 --- a/public/openapi3.json +++ b/public/openapi3.json @@ -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": {