AuthZ: Deleting managed role permissions for a specified resource (#110617)

* basics for deleting managed role permissions for a specified resource

* fix the query

* fix query tests

* storage tests

* sql tests

* add missing import

* Update pkg/registry/apis/iam/resourcepermission/storage_backend.go

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

* PR feedback

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

---------

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
This commit is contained in:
Ieva 2025-09-05 16:22:09 +01:00 committed by GitHub
parent ba202ebab1
commit d692303e76
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 291 additions and 12 deletions

View File

@ -50,6 +50,11 @@ type ListResourcePermissionsQuery struct {
// TODO Pagination common.Pagination
}
type DeleteResourcePermissionsQuery struct {
Scope string
OrgID int64
}
type rbacAssignmentCreate struct {
Action string // e.g. "dashboards:edit"
Scope string // e.g. "folders:uid:1"

View File

@ -0,0 +1,8 @@
DELETE FROM {{ .Ident .PermissionTable }} as p
WHERE p.scope = {{ .Arg .Query.Scope }}
AND p.role_id IN (
SELECT r.id
FROM {{ .Ident .RoleTable }} as r
WHERE r.name LIKE {{ .Arg .ManagedRolePattern }}
AND r.org_id = {{ .Arg .Query.OrgID }}
)

View File

@ -8,7 +8,6 @@ import (
"strings"
"github.com/grafana/authlib/types"
"github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1"
"github.com/grafana/grafana/pkg/registry/apis/iam/legacy"
"github.com/grafana/grafana/pkg/services/accesscontrol"
@ -320,3 +319,31 @@ func (s *ResourcePermSqlBackend) createResourcePermission(
// Update
// 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)
if err != nil {
return err
}
scope := mapper.Scope(grn.Name)
resourceQuery := &DeleteResourcePermissionsQuery{
Scope: scope,
OrgID: ns.OrgID,
}
rawQuery, args, err := buildDeleteResourcePermissionsQueryFromTemplate(sql, resourceQuery)
if err != nil {
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())
return fmt.Errorf("could not delete resource permission")
}
return nil
}

View File

@ -43,7 +43,7 @@ func setupTestRoles(t *testing.T, store db.DB) {
sess := store.GetSqlxSession()
_, err := sess.Exec(context.Background(),
`INSERT INTO role (id, version, org_id, uid, name, display_name, description, group_name, hidden, created, updated)
`INSERT INTO role (id, version, org_id, uid, name, display_name, description, group_name, hidden, created, updated)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?), (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?), (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?),
(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?), (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
// Managed roles
@ -207,6 +207,68 @@ func TestIntegration_ResourcePermSqlBackend_getResourcePermission(t *testing.T)
}
}
func TestResourcePermSqlBackend_deleteResourcePermission(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
backend := setupBackend(t)
sql, err := backend.dbProvider(context.Background())
require.NoError(t, err)
setupTestRoles(t, sql.DB)
tests := []struct {
name string
resource string
orgID int64
want v0alpha1.ResourcePermission
err error
}{
{
name: "should return an error for unknown resource type",
orgID: 1,
resource: "unknown.grafana.app-unknown-u1",
err: errUnknownGroupResource,
},
{
name: "should return an error for invalid resource name",
orgID: 1,
resource: "invalid.grafana.app-invalid",
err: errInvalidName,
},
{
name: "should delete permissions in org1 for fold1",
resource: "folder.grafana.app-folders-fold1",
orgID: 1,
err: nil,
},
{
name: "should delete permissions in org2 for fold1",
resource: "folder.grafana.app-folders-fold1",
orgID: 2,
err: nil,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ns := types.NamespaceInfo{
OrgID: tt.orgID,
}
err := backend.deleteResourcePermission(context.Background(), sql, ns, tt.resource)
if tt.err != nil {
require.Error(t, err)
require.ErrorIs(t, err, tt.err)
return
}
require.NoError(t, err)
// check that the resource has been deleted
_, err = backend.getResourcePermission(context.Background(), sql, ns, tt.resource)
require.Error(t, err)
})
}
}
func TestIntegration_ResourcePermSqlBackend_CreateResourcePermission(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test")

View File

@ -170,6 +170,14 @@ func (s *ResourcePermSqlBackend) WriteEvent(ctx context.Context, event resource.
return 0, apierrors.NewBadRequest(fmt.Sprintf("invalid key %q: %v", event.Key, err.Error()))
}
dbHelper, err := s.dbProvider(ctx)
if err != nil {
// Hide the error from the user, but log it
logger := s.logger.FromContext(ctx)
logger.Error("Failed to get database helper", "error", err)
return 0, errDatabaseHelper
}
mapper, grn, err := s.splitResourceName(event.Key.Name)
if err != nil {
return 0, apierrors.NewBadRequest(fmt.Sprintf("invalid resource name %q: %v", event.Key.Name, err.Error()))
@ -180,6 +188,8 @@ func (s *ResourcePermSqlBackend) WriteEvent(ctx context.Context, event resource.
}
switch event.Type {
case resourcepb.WatchEvent_DELETED:
err = s.deleteResourcePermission(ctx, dbHelper, ns, event.Key.Name)
case resourcepb.WatchEvent_ADDED:
{
var v0resourceperm *v0alpha1.ResourcePermission
@ -199,11 +209,6 @@ func (s *ResourcePermSqlBackend) WriteEvent(ctx context.Context, event resource.
)
}
dbHelper, err := s.dbProvider(ctx)
if err != nil {
return 0, err
}
rv, err = s.createResourcePermission(ctx, dbHelper, ns, mapper, grn, v0resourceperm)
if err != nil {
if errors.Is(err, errInvalidSpec) || errors.Is(err, errInvalidName) {
@ -219,5 +224,5 @@ func (s *ResourcePermSqlBackend) WriteEvent(ctx context.Context, event resource.
return 0, fmt.Errorf("unsupported event type: %v", event.Type)
}
return rv, nil
return rv, err
}

View File

@ -329,3 +329,100 @@ func TestWriteEvent_Add(t *testing.T) {
})
})
}
func TestWriteEvent_Delete(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
backend := setupBackend(t)
sql, err := backend.dbProvider(context.Background())
require.NoError(t, err)
setupTestRoles(t, sql.DB)
updated1 := time.Date(2025, 9, 2, 0, 0, 0, 0, time.UTC)
updated2 := time.Date(2025, 9, 3, 0, 0, 0, 0, time.UTC) // managed role for team 1 has a later updated permission
gr := v0alpha1.ResourcePermissionInfo.GroupResource()
t.Run("Should error if namespace is invalid", func(t *testing.T) {
rv, err := backend.WriteEvent(context.Background(), resource.WriteEvent{
Key: &resourcepb.ResourceKey{Group: gr.Group, Resource: gr.Resource, Name: "folder.grafana.app-folders-fold1", Namespace: "invalid"},
Type: resourcepb.WatchEvent_DELETED,
})
require.Zero(t, rv)
require.NotNil(t, err)
require.Contains(t, err.Error(), "requires a valid namespace")
})
t.Run("Should fail to delete resource permissions if resource name is not specified", func(t *testing.T) {
_, err = backend.WriteEvent(context.Background(), resource.WriteEvent{
Key: &resourcepb.ResourceKey{Group: gr.Group, Resource: gr.Resource, Name: "", Namespace: "default"},
Type: resourcepb.WatchEvent_DELETED,
})
require.NotNil(t, err)
require.Contains(t, err.Error(), "invalid key")
})
t.Run("Should fail to delete resource permissions for unknown resource", func(t *testing.T) {
_, err = backend.WriteEvent(context.Background(), resource.WriteEvent{
Key: &resourcepb.ResourceKey{Group: gr.Group, Resource: gr.Resource, Name: "unknown.grafana.app-unknown-uid", Namespace: "default"},
Type: resourcepb.WatchEvent_DELETED,
})
require.NotNil(t, err)
require.Contains(t, err.Error(), "unknown group/resource")
})
t.Run("Should successfully delete fold1 permissions in org-1", func(t *testing.T) {
// Check that permissions exist
resp := backend.ReadResource(context.Background(), &resourcepb.ReadRequest{
Key: &resourcepb.ResourceKey{Name: "folder.grafana.app-folders-fold1", Namespace: "default"},
})
require.NotNil(t, resp)
require.Nil(t, resp.Error)
require.NotNil(t, resp.Value)
var permission v0alpha1.ResourcePermission
err := json.Unmarshal(resp.Value, &permission)
require.NoError(t, err)
require.Len(t, permission.Spec.Permissions, 1)
_, err = backend.WriteEvent(context.Background(), resource.WriteEvent{
Key: &resourcepb.ResourceKey{Group: gr.Group, Resource: gr.Resource, Name: "folder.grafana.app-folders-fold1", Namespace: "default"},
Type: resourcepb.WatchEvent_DELETED,
})
require.Nil(t, err)
// Check that permissions are deleted
resp = backend.ReadResource(context.Background(), &resourcepb.ReadRequest{
Key: &resourcepb.ResourceKey{Name: "folder.grafana.app-folders-fold1", Namespace: "default"},
})
require.NotNil(t, resp.Error)
// Check that org-2 permissions are unaffected
resp = backend.ReadResource(context.Background(), &resourcepb.ReadRequest{
Key: &resourcepb.ResourceKey{Name: "folder.grafana.app-folders-fold1", Namespace: "org-2"},
})
require.NotNil(t, resp)
require.Nil(t, resp.Error)
require.NotNil(t, resp.Value)
require.Equal(t, updated1.UnixMilli(), resp.ResourceVersion)
err = json.Unmarshal(resp.Value, &permission)
require.NoError(t, err)
require.Len(t, permission.Spec.Permissions, 1)
// Check that dash1 permissions in org 1 are unaffected
resp = backend.ReadResource(context.Background(), &resourcepb.ReadRequest{
Key: &resourcepb.ResourceKey{Name: "dashboard.grafana.app-dashboards-dash1", Namespace: "default"},
})
require.NotNil(t, resp)
require.Nil(t, resp.Error)
require.NotNil(t, resp.Value)
require.Equal(t, updated2.UnixMilli(), resp.ResourceVersion)
err = json.Unmarshal(resp.Value, &permission)
require.NoError(t, err)
require.Len(t, permission.Spec.Permissions, 4)
})
}

View File

@ -18,10 +18,11 @@ var (
sqlTemplates = template.Must(template.New("sql").ParseFS(sqlTemplatesFS, `queries/*.sql`))
roleInsertTplt = mustTemplate("role_insert.sql")
assignmentInsertTplt = mustTemplate("assignment_insert.sql")
permissionInsertTplt = mustTemplate("permission_insert.sql")
resourcePermissionsQueryTplt = mustTemplate("resource_permission_query.sql")
resourcePermissionsQueryTplt = mustTemplate("resource_permission_query.sql")
resourcePermissionDeletionQueryTplt = mustTemplate("resource_permission_deletion_query.sql")
roleInsertTplt = mustTemplate("role_insert.sql")
assignmentInsertTplt = mustTemplate("assignment_insert.sql")
permissionInsertTplt = mustTemplate("permission_insert.sql")
)
func mustTemplate(filename string) *template.Template {
@ -178,3 +179,32 @@ func buildInsertPermissionQuery(dbHelper *legacysql.LegacyDatabaseHelper, roleID
// Update
// Delete
type deleteResourcePermissionsQueryTemplate struct {
sqltemplate.SQLTemplate
Query *DeleteResourcePermissionsQuery
PermissionTable string
RoleTable string
ManagedRolePattern string
}
func (r deleteResourcePermissionsQueryTemplate) Validate() error {
return nil
}
func buildDeleteResourcePermissionsQueryFromTemplate(sql *legacysql.LegacyDatabaseHelper, query *DeleteResourcePermissionsQuery) (string, []interface{}, error) {
req := deleteResourcePermissionsQueryTemplate{
SQLTemplate: sqltemplate.New(sql.DialectForDriver()),
Query: query,
PermissionTable: sql.Table("permission"),
RoleTable: sql.Table("role"),
ManagedRolePattern: "managed:%",
}
rawQuery, err := sqltemplate.Execute(resourcePermissionDeletionQueryTplt, req)
if err != nil {
return "", nil, fmt.Errorf("execute template %q: %w", resourcePermissionDeletionQueryTplt.Name(), err)
}
return rawQuery, req.GetArgs(), nil
}

View File

@ -73,6 +73,18 @@ func TestTemplates(t *testing.T) {
return &v
}
getDeleteResourcePermissionsQuery := func(q *DeleteResourcePermissionsQuery) sqltemplate.SQLTemplate {
v := deleteResourcePermissionsQueryTemplate{
SQLTemplate: sqltemplate.New(nodb.DialectForDriver()),
Query: q,
PermissionTable: nodb.Table("permission"),
RoleTable: nodb.Table("role"),
ManagedRolePattern: "managed:%",
}
v.SQLTemplate = mocks.NewTestingSQLTemplate()
return &v
}
mocks.CheckQuerySnapshots(t, mocks.TemplateTestSetup{
RootDir: "testdata",
SQLTemplatesFS: sqlTemplatesFS,
@ -135,6 +147,15 @@ func TestTemplates(t *testing.T) {
}),
},
},
resourcePermissionDeletionQueryTplt: {
{
Name: "basic_delete_query",
Data: getDeleteResourcePermissionsQuery(&DeleteResourcePermissionsQuery{
Scope: "dash_123",
OrgID: 3,
}),
},
},
},
})
}

View File

@ -0,0 +1,8 @@
DELETE FROM `grafana`.`permission` as p
WHERE p.scope = 'dash_123'
AND p.role_id IN (
SELECT r.id
FROM `grafana`.`role` as r
WHERE r.name LIKE 'managed:%'
AND r.org_id = 3
)

View File

@ -0,0 +1,8 @@
DELETE FROM "grafana"."permission" as p
WHERE p.scope = 'dash_123'
AND p.role_id IN (
SELECT r.id
FROM "grafana"."role" as r
WHERE r.name LIKE 'managed:%'
AND r.org_id = 3
)

View File

@ -0,0 +1,8 @@
DELETE FROM "grafana"."permission" as p
WHERE p.scope = 'dash_123'
AND p.role_id IN (
SELECT r.id
FROM "grafana"."role" as r
WHERE r.name LIKE 'managed:%'
AND r.org_id = 3
)