root
/
grafana
mirror of https://github.com/grafana/grafana.git
1
0
Fork 0
Code Issues Actions 196 Packages Projects Releases Wiki Activity

AuthZ: code and error cleanup (#111037) 

* use API errors where possible

* fix test
This commit is contained in:
Ieva 2025-09-18 11:38:02 +01:00 committed by GitHub
parent 51d3624bf9
commit 7bd20b98ba
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 18 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
pkg/registry/apis/iam/resourcepermission/sql.go
View File

@ -8,6 +8,8 @@ import (
"strings"
"time"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"github.com/grafana/authlib/types"
"github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1"
"github.com/grafana/grafana/pkg/registry/apis/iam/common"
@ -121,7 +123,6 @@ func (s *ResourcePermSqlBackend) latestUpdate(ctx context.Context, dbHelper *leg
return maxUpdated.UnixMilli()
}
// Get
// getRbacAssignmentsWithTx queries resource permissions based on the provided ListResourcePermissionsQuery and groups them by resource (e.g. {folder.grafana.app, folders, fold1})
func (s *ResourcePermSqlBackend) getRbacAssignmentsWithTx(ctx context.Context, sql *legacysql.LegacyDatabaseHelper, tx *session.SessionTx, query *ListResourcePermissionsQuery) ([]rbacAssignment, error) {
rawQuery, args, err := buildListResourcePermissionsQueryFromTemplate(sql, query)
@ -159,7 +160,7 @@ func (s *ResourcePermSqlBackend) getRbacAssignmentsWithTx(ctx context.Context, s
func (s *ResourcePermSqlBackend) getResourcePermission(ctx context.Context, sql *legacysql.LegacyDatabaseHelper, tx *session.SessionTx, ns types.NamespaceInfo, name string) (*v0alpha1.ResourcePermission, error) {
mapper, grn, err := s.splitResourceName(name)
if err != nil {
return nil, err
return nil, apierrors.NewInternalError(err)
}
resourceQuery := &ListResourcePermissionsQuery{
@ -170,19 +171,19 @@ func (s *ResourcePermSqlBackend) getResourcePermission(ctx context.Context, sql
assignments, err := s.getRbacAssignmentsWithTx(ctx, sql, tx, resourceQuery)
if err != nil {
return nil, err
return nil, apierrors.NewInternalError(err)
}
if len(assignments) == 0 {
return nil, fmt.Errorf("resource permission %q: %w", resourceQuery.Scopes, errNotFound)
return nil, apierrors.NewNotFound(v0alpha1.ResourcePermissionInfo.GroupResource(), name)
}
resourcePermission, err := s.toV0ResourcePermissions(assignments, ns.Value)
if err != nil {
return nil, err
return nil, apierrors.NewInternalError(err)
}
if resourcePermission == nil {
return nil, fmt.Errorf("resource permission %q: %w", resourceQuery.Scopes, errNotFound)
return nil, apierrors.NewNotFound(v0alpha1.ResourcePermissionInfo.GroupResource(), name)
}
return &resourcePermission[0], nil
@ -402,8 +403,6 @@ func (s *ResourcePermSqlBackend) createResourcePermission(
return timeNow().UnixMilli(), nil
}
// Update
func (s *ResourcePermSqlBackend) updateResourcePermission(ctx context.Context, dbHelper *legacysql.LegacyDatabaseHelper, ns types.NamespaceInfo, mapper Mapper, grn *groupResourceName, v0ResourcePerm *v0alpha1.ResourcePermission) (int64, error) {
if err := validateCreateAndUpdateInput(v0ResourcePerm, grn); err != nil {
return 0, err
@ -412,8 +411,8 @@ func (s *ResourcePermSqlBackend) updateResourcePermission(ctx context.Context, d
err := dbHelper.DB.GetSqlxSession().WithTransaction(ctx, func(tx *session.SessionTx) error {
currentPerms, err := s.getResourcePermission(ctx, dbHelper, tx, ns, grn.string())
if err != nil {
if errors.Is(err, errNotFound) {
return fmt.Errorf("resource permissions not found: %w", errNotFound)
if apierrors.IsNotFound(err) {
return apierrors.NewNotFound(v0alpha1.ResourcePermissionInfo.GroupResource(), grn.string())
}
s.logger.Error("could not get resource permissions", "orgID", ns.OrgID, "scope", grn.Name, "error", err.Error())
return fmt.Errorf("could not get the existing resource permissions for resource %s", grn.Name)
@ -514,8 +513,6 @@ func validateCreateAndUpdateInput(v0ResourcePerm *v0alpha1.ResourcePermission, g
return nil
}
// Delete
// deleteResourcePermission deletes resource permissions for a single ResourcePermission resource referenced by its name in the format <group>-<resource>-<name> (e.g. dashboard.grafana.app-dashboards-ad5rwqs)
func (s *ResourcePermSqlBackend) deleteResourcePermission(ctx context.Context, sql *legacysql.LegacyDatabaseHelper, ns types.NamespaceInfo, name string) error {
mapper, grn, err := s.splitResourceName(name)
@ -534,7 +531,6 @@ func (s *ResourcePermSqlBackend) deleteResourcePermission(ctx context.Context, s
return err
}
// run delete query
_, err = sql.DB.GetSqlxSession().Exec(ctx, rawQuery, args...)
if err != nil {
s.logger.Error("could not delete resource permissions", "scope", scope, "orgID", ns.OrgID, err.Error())

3
pkg/registry/apis/iam/resourcepermission/sql_test.go
View File

@ -306,7 +306,7 @@ func TestIntegration_ResourcePermSqlBackend_getResourcePermission(t *testing.T)
if tt.err != nil {
require.Error(t, err)
require.ErrorIs(t, err, tt.err)
require.ErrorAs(t, err, &tt.err)
return
}
require.NoError(t, err)
@ -533,7 +533,6 @@ func TestIntegration_ResourcePermSqlBackend_UpdateResourcePermission(t *testing.
_, err = backend.updateResourcePermission(ctx, sql, types.NamespaceInfo{Value: "default", OrgID: 1}, mapper, grn, resourcePerm)
require.Error(t, err)
require.ErrorIs(t, err, errNotFound)
})
t.Run("should update resource permission", func(t *testing.T) {

23
pkg/registry/apis/iam/resourcepermission/storage_backend.go
View File

@ -104,7 +104,7 @@ func (s *ResourcePermSqlBackend) ListIterator(ctx context.Context, req *resource
if err != nil {
logger := s.logger.FromContext(ctx)
logger.Error("Failed to get database helper", "error", err)
return 0, errDatabaseHelper
return 0, apierrors.NewInternalError(errDatabaseHelper)
}
iterator, err := s.newRoleIterator(ctx, dbHelper, ns, pagination)
@ -114,12 +114,12 @@ func (s *ResourcePermSqlBackend) ListIterator(ctx context.Context, req *resource
}()
}
if err != nil {
return 0, err
return 0, apierrors.NewInternalError(err)
}
err = callback(iterator)
if err != nil {
return 0, err
return 0, apierrors.NewInternalError(err)
}
return s.latestUpdate(ctx, dbHelper, ns), nil
@ -136,7 +136,7 @@ func (s *ResourcePermSqlBackend) ReadResource(ctx context.Context, req *resource
ns, err := types.ParseNamespace(req.Key.Namespace)
if err != nil {
rsp.Error = resource.AsErrorResult(err)
rsp.Error = resource.AsErrorResult(apierrors.NewBadRequest(err.Error()))
return rsp
}
if ns.OrgID <= 0 {
@ -155,7 +155,6 @@ func (s *ResourcePermSqlBackend) ReadResource(ctx context.Context, req *resource
// Hide the error from the user, but log it
logger := s.logger.FromContext(ctx)
logger.Error("Failed to get database helper", "error", err)
rsp.Error = resource.AsErrorResult(errDatabaseHelper)
return rsp
}
@ -166,15 +165,7 @@ func (s *ResourcePermSqlBackend) ReadResource(ctx context.Context, req *resource
})
if err != nil {
if errors.Is(err, errNotFound) {
rsp.Error = resource.AsErrorResult(
apierrors.NewNotFound(v0alpha1.ResourcePermissionInfo.GroupResource(), req.Key.Name),
)
} else if errors.Is(err, errUnknownGroupResource) || errors.Is(err, errInvalidName) {
rsp.Error = resource.AsErrorResult(apierrors.NewBadRequest(err.Error()))
} else {
rsp.Error = resource.AsErrorResult(err)
}
rsp.Error = resource.AsErrorResult(err)
return rsp
}
@ -260,7 +251,7 @@ func (s *ResourcePermSqlBackend) WriteEvent(ctx context.Context, event resource.
var v0resourceperm *v0alpha1.ResourcePermission
v0resourceperm, err = getResourcePermissionFromEvent(event)
if err != nil {
return 0, err
return 0, apierrors.NewBadRequest(fmt.Sprintf("invalid resource permission in event: %v", err))
}
if v0resourceperm.Name != event.Key.Name {
@ -294,7 +285,7 @@ func (s *ResourcePermSqlBackend) WriteEvent(ctx context.Context, event resource.
}
}
default:
return 0, fmt.Errorf("unsupported event type: %v", event.Type)
return 0, apierrors.NewMethodNotSupported(v0alpha1.ResourcePermissionInfo.GroupResource(), event.Type.String())
}
return rv, err

2
pkg/registry/apis/iam/resourcepermission/storage_backend_test.go
View File

@ -551,7 +551,7 @@ func TestIntegration_ResourcePermSqlBackend_ListIterator(t *testing.T) {
})
require.Error(t, err)
require.Equal(t, expectedErr, err)
require.ErrorAs(t, err, &expectedErr)
require.Zero(t, listRV)
})

8
pkg/registry/apis/iam/resourcepermission/templates.go
View File

@ -35,8 +35,6 @@ func mustTemplate(filename string) *template.Template {
panic(fmt.Sprintf("template file not found: %s", filename))
}
// List
type pageQueryTemplate struct {
sqltemplate.SQLTemplate
Query *PageQuery
@ -140,8 +138,6 @@ func buildListResourcePermissionsQueryFromTemplate(dbHelper *legacysql.LegacyDat
return rawQuery, req.GetArgs(), nil
}
// Create
type insertRoleTemplate struct {
sqltemplate.SQLTemplate
RoleTable string
@ -235,8 +231,6 @@ func buildInsertPermissionQuery(dbHelper *legacysql.LegacyDatabaseHelper, roleID
return rawQuery, req.GetArgs(), nil
}
// Update
type removePermissionTemplate struct {
sqltemplate.SQLTemplate
PermissionTable string
@ -268,8 +262,6 @@ func buildRemovePermissionQuery(dbHelper *legacysql.LegacyDatabaseHelper, scope,
return rawQuery, req.GetArgs(), nil
}
// Delete
type deleteResourcePermissionsQueryTemplate struct {
sqltemplate.SQLTemplate
Query *DeleteResourcePermissionsQuery