diff --git a/pkg/registry/apis/iam/resourcepermission/queries/permission_insert.sql b/pkg/registry/apis/iam/resourcepermission/queries/permission_insert.sql index 1ba966e21eb..1dc0ac83e3b 100644 --- a/pkg/registry/apis/iam/resourcepermission/queries/permission_insert.sql +++ b/pkg/registry/apis/iam/resourcepermission/queries/permission_insert.sql @@ -1,11 +1,11 @@ INSERT INTO {{ .Ident .PermissionTable }} (role_id, action, scope, created, updated, kind, attribute, identifier) VALUES ( - {{ .Arg $.RoleID }}, - {{ .Arg $.Permission.Action }}, - {{ .Arg $.Permission.Scope }}, - {{ .Arg $.Now }}, - {{ .Arg $.Now }}, - {{ .Arg $.Permission.Kind }}, - {{ .Arg $.Permission.Attribute }}, - {{ .Arg $.Permission.Identifier }} + {{ .Arg .RoleID }}, + {{ .Arg .Permission.Action }}, + {{ .Arg .Permission.Scope }}, + {{ .Arg .Now }}, + {{ .Arg .Now }}, + {{ .Arg .Permission.Kind }}, + {{ .Arg .Permission.Attribute }}, + {{ .Arg .Permission.Identifier }} ) diff --git a/pkg/registry/apis/iam/resourcepermission/queries/permission_remove.sql b/pkg/registry/apis/iam/resourcepermission/queries/permission_remove.sql new file mode 100644 index 00000000000..56ffa56dab0 --- /dev/null +++ b/pkg/registry/apis/iam/resourcepermission/queries/permission_remove.sql @@ -0,0 +1,9 @@ +DELETE FROM {{ .Ident .PermissionTable }} AS p +WHERE p.scope = {{ .Arg .Scope }} AND p.action = {{ .Arg .Action }} +AND p.role_id = ( + SELECT r.id + FROM {{ .Ident .RoleTable }} AS r + WHERE r.org_id = {{ .Arg .OrgID }} + AND r.name = {{ .Arg .RoleName }} + LIMIT 1 +) diff --git a/pkg/registry/apis/iam/resourcepermission/sql.go b/pkg/registry/apis/iam/resourcepermission/sql.go index ba8c0d93ea1..e66542f2e76 100644 --- a/pkg/registry/apis/iam/resourcepermission/sql.go +++ b/pkg/registry/apis/iam/resourcepermission/sql.go @@ -156,7 +156,7 @@ func (s *ResourcePermSqlBackend) getRbacAssignmentsWithTx(ctx context.Context, s } // getResourcePermission retrieves a single ResourcePermission by its name in the format -- (e.g. dashboard.grafana.app-dashboards-ad5rwqs) -func (s *ResourcePermSqlBackend) getResourcePermission(ctx context.Context, sql *legacysql.LegacyDatabaseHelper, ns types.NamespaceInfo, name string) (*v0alpha1.ResourcePermission, error) { +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 @@ -168,11 +168,10 @@ func (s *ResourcePermSqlBackend) getResourcePermission(ctx context.Context, sql ActionSets: mapper.ActionSets(), } - var assignments []rbacAssignment - err = sql.DB.GetSqlxSession().WithTransaction(ctx, func(tx *session.SessionTx) error { - assignments, err = s.getRbacAssignmentsWithTx(ctx, sql, tx, resourceQuery) - return err - }) + assignments, err := s.getRbacAssignmentsWithTx(ctx, sql, tx, resourceQuery) + if err != nil { + return nil, err + } if len(assignments) == 0 { return nil, fmt.Errorf("resource permission %q: %w", resourceQuery.Scopes, errNotFound) @@ -263,10 +262,10 @@ func (s *ResourcePermSqlBackend) storeRbacAssignment(ctx context.Context, dbHelp // buildRbacAssignments builds the list of assignments (role assignments and permissions) for a given ResourcePermission spec // It resolves user/team/service account UIDs to internal IDs for the role name and assignee subjectID -func (s *ResourcePermSqlBackend) buildRbacAssignments(ctx context.Context, ns types.NamespaceInfo, mapper Mapper, v0ResourcePerm *v0alpha1.ResourcePermission, rbacScope string) ([]rbacAssignmentCreate, error) { - assignments := make([]rbacAssignmentCreate, 0, len(v0ResourcePerm.Spec.Permissions)) +func (s *ResourcePermSqlBackend) buildRbacAssignments(ctx context.Context, ns types.NamespaceInfo, mapper Mapper, v0ResourcePerm []v0alpha1.ResourcePermissionspecPermission, rbacScope string) ([]rbacAssignmentCreate, error) { + assignments := make([]rbacAssignmentCreate, 0, len(v0ResourcePerm)) - for _, perm := range v0ResourcePerm.Spec.Permissions { + for _, perm := range v0ResourcePerm { rbacActionSet, err := mapper.ActionSet(perm.Verb) if err != nil { return nil, err @@ -371,22 +370,11 @@ func (s *ResourcePermSqlBackend) existsResourcePermission(ctx context.Context, t func (s *ResourcePermSqlBackend) createResourcePermission( ctx context.Context, dbHelper *legacysql.LegacyDatabaseHelper, ns types.NamespaceInfo, mapper Mapper, grn *groupResourceName, v0ResourcePerm *v0alpha1.ResourcePermission, ) (int64, error) { - if v0ResourcePerm == nil { - return 0, fmt.Errorf("resource permission cannot be nil") + if err := validateCreateAndUpdateInput(v0ResourcePerm, grn); err != nil { + return 0, err } - if len(v0ResourcePerm.Spec.Permissions) == 0 { - return 0, fmt.Errorf("resource permission must have at least one permission: %w", errInvalidSpec) - } - - // Validate that the group/resource/name in the name matches the spec - if grn.Group != v0ResourcePerm.Spec.Resource.ApiGroup || - grn.Resource != v0ResourcePerm.Spec.Resource.Resource || - grn.Name != v0ResourcePerm.Spec.Resource.Name { - return 0, fmt.Errorf("resource permission name does not match spec: %w", errInvalidSpec) - } - - assignments, err := s.buildRbacAssignments(ctx, ns, mapper, v0ResourcePerm, mapper.Scope(grn.Name)) + assignments, err := s.buildRbacAssignments(ctx, ns, mapper, v0ResourcePerm.Spec.Permissions, mapper.Scope(grn.Name)) if err != nil { return 0, err } @@ -416,6 +404,116 @@ func (s *ResourcePermSqlBackend) createResourcePermission( // 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 + } + + 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) + } + 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) + } + + permissionsToAdd, permissionsToRemove := diffPermissions(currentPerms.Spec.Permissions, v0ResourcePerm.Spec.Permissions) + + if len(permissionsToRemove) > 0 { + permsToRemove, err := s.buildRbacAssignments(ctx, ns, mapper, permissionsToRemove, mapper.Scope(grn.Name)) + if err != nil { + return err + } + + for _, perm := range permsToRemove { + removePermQuery, args, err := buildRemovePermissionQuery(dbHelper, perm.Scope, perm.Action, perm.RoleName, ns.OrgID) + if err != nil { + return err + } + _, err = tx.Exec(ctx, removePermQuery, args...) + if err != nil { + s.logger.Error("could not remove role permission", "scope", perm.Scope, "role", perm.RoleName, "error", err.Error()) + return fmt.Errorf("could not remove role permission") + } + } + } + + if len(permissionsToAdd) > 0 { + permsToAdd, err := s.buildRbacAssignments(ctx, ns, mapper, permissionsToAdd, mapper.Scope(grn.Name)) + if err != nil { + return err + } + + for _, assignment := range permsToAdd { + if err := s.storeRbacAssignment(ctx, dbHelper, tx, ns.OrgID, assignment); err != nil { + return err + } + } + } + + return nil + }) + + if err != nil { + return 0, err + } + + // Return a timestamp as resource version + return timeNow().UnixMilli(), nil +} + +func diffPermissions(currentPermissions, desiredPermissions []v0alpha1.ResourcePermissionspecPermission) (permissionsToAdd, permissionsToRemove []v0alpha1.ResourcePermissionspecPermission) { + for _, desired := range desiredPermissions { + found := false + for _, existing := range currentPermissions { + if desired.Name == existing.Name && desired.Kind == existing.Kind && desired.Verb == existing.Verb { + found = true + break + } + } + if !found { + permissionsToAdd = append(permissionsToAdd, desired) + } + } + + // Compile a list of permissions to remove + for _, existing := range currentPermissions { + found := false + for _, desired := range desiredPermissions { + if desired.Name == existing.Name && desired.Kind == existing.Kind && desired.Verb == existing.Verb { + found = true + break + } + } + if !found { + permissionsToRemove = append(permissionsToRemove, existing) + } + } + + return permissionsToAdd, permissionsToRemove +} + +func validateCreateAndUpdateInput(v0ResourcePerm *v0alpha1.ResourcePermission, grn *groupResourceName) error { + if v0ResourcePerm == nil { + return fmt.Errorf("resource permission cannot be nil") + } + + if len(v0ResourcePerm.Spec.Permissions) == 0 { + return fmt.Errorf("resource permission must have at least one permission: %w", errInvalidSpec) + } + + // Validate that the group/resource/name in the name matches the spec + if grn.Group != v0ResourcePerm.Spec.Resource.ApiGroup || + grn.Resource != v0ResourcePerm.Spec.Resource.Resource || + grn.Name != v0ResourcePerm.Spec.Resource.Name { + return fmt.Errorf("resource permission name does not match spec: %w", errInvalidSpec) + } + + return nil +} + // Delete // deleteResourcePermission deletes resource permissions for a single ResourcePermission resource referenced by its name in the format -- (e.g. dashboard.grafana.app-dashboards-ad5rwqs) diff --git a/pkg/registry/apis/iam/resourcepermission/sql_test.go b/pkg/registry/apis/iam/resourcepermission/sql_test.go index 977ac4b9981..7d6b492c590 100644 --- a/pkg/registry/apis/iam/resourcepermission/sql_test.go +++ b/pkg/registry/apis/iam/resourcepermission/sql_test.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/iam/common" "github.com/grafana/grafana/pkg/registry/apis/iam/legacy" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/sqlstore/session" "github.com/grafana/grafana/pkg/storage/legacysql" "github.com/grafana/grafana/pkg/tests/testsuite" "github.com/grafana/grafana/pkg/util/testutil" @@ -297,7 +298,12 @@ func TestIntegration_ResourcePermSqlBackend_getResourcePermission(t *testing.T) ns := types.NamespaceInfo{ OrgID: tt.orgID, } - got, err := backend.getResourcePermission(context.Background(), sql, ns, tt.resource) + var got *v0alpha1.ResourcePermission + err = sql.DB.GetSqlxSession().WithTransaction(context.Background(), func(tx *session.SessionTx) error { + got, err = backend.getResourcePermission(context.Background(), sql, tx, ns, tt.resource) + return err + }) + if tt.err != nil { require.Error(t, err) require.ErrorIs(t, err, tt.err) @@ -369,7 +375,10 @@ func TestIntegration_ResourcePermSqlBackend_deleteResourcePermission(t *testing. require.NoError(t, err) // check that the resource has been deleted - _, err = backend.getResourcePermission(context.Background(), sql, ns, tt.resource) + err = sql.DB.GetSqlxSession().WithTransaction(context.Background(), func(tx *session.SessionTx) error { + _, err = backend.getResourcePermission(context.Background(), sql, tx, ns, tt.resource) + return err + }) require.Error(t, err) }) } @@ -487,6 +496,113 @@ func TestIntegration_ResourcePermSqlBackend_CreateResourcePermission(t *testing. }) } +func TestIntegration_ResourcePermSqlBackend_UpdateResourcePermission(t *testing.T) { + testutil.SkipIntegrationTestInShortMode(t) + + backend := setupBackend(t) + backend.identityStore = NewFakeIdentityStore(t) + ctx := context.Background() + sql, err := backend.dbProvider(ctx) + require.NoError(t, err) + setupTestRoles(t, sql.DB) + + t.Run("should fail to update resource permission for a resource that doesn't have any permissions yet", func(t *testing.T) { + resourcePerm := &v0alpha1.ResourcePermission{ + ObjectMeta: metav1.ObjectMeta{ + Name: "folder.grafana.app-folders-newfold", + Namespace: "default", + }, + Spec: v0alpha1.ResourcePermissionSpec{ + Resource: v0alpha1.ResourcePermissionspecResource{ + ApiGroup: "folder.grafana.app", + Resource: "folders", + Name: "newfold", + }, + Permissions: []v0alpha1.ResourcePermissionspecPermission{ + { + Kind: v0alpha1.ResourcePermissionSpecPermissionKindBasicRole, + Name: "Viewer", + Verb: "view", + }, + }, + }, + } + + mapper, grn, err := backend.splitResourceName(resourcePerm.Name) + require.NoError(t, err) + + _, 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) { + resourcePerm := &v0alpha1.ResourcePermission{ + ObjectMeta: metav1.ObjectMeta{ + Name: "folder.grafana.app-folders-fold1", + Namespace: "default", + }, + Spec: v0alpha1.ResourcePermissionSpec{ + Resource: v0alpha1.ResourcePermissionspecResource{ + ApiGroup: "folder.grafana.app", + Resource: "folders", + Name: "fold1", + }, + Permissions: []v0alpha1.ResourcePermissionspecPermission{ + { + Kind: v0alpha1.ResourcePermissionSpecPermissionKindBasicRole, + Name: "Editor", + Verb: "view", + }, + { + Kind: v0alpha1.ResourcePermissionSpecPermissionKindUser, + Name: "user-1", + Verb: "view", + }, + { + Kind: v0alpha1.ResourcePermissionSpecPermissionKindServiceAccount, + Name: "sa-1", + Verb: "admin", + }, + }, + }, + } + + mapper, grn, err := backend.splitResourceName(resourcePerm.Name) + require.NoError(t, err) + + rv, err := backend.updateResourcePermission(ctx, sql, types.NamespaceInfo{Value: "default", OrgID: 1}, mapper, grn, resourcePerm) + require.NoError(t, err) + require.Equal(t, timeNow().UnixMilli(), rv) + + var permission accesscontrol.Permission + sess := sql.DB.GetSqlxSession() + + // Check that the right permissions exist and that the old ones have been removed + // User-1 should still have view access to fold1 + // User-2 should no longer have edit on fold1 + // Service account should now have admin on fold1 + // Builtin Editor should now have view access to fold1 + + err = sess.Get(ctx, &permission, "SELECT action FROM permission WHERE scope = ? AND role_id = (SELECT role_id FROM user_role WHERE org_id = ? AND user_id = ?)", "folders:uid:fold1", 1, "1") + require.NoError(t, err) + require.Equal(t, "folders:view", permission.Action) + + count := 0 + err = sess.Get(ctx, &count, "SELECT COUNT(*) FROM permission WHERE role_id = (SELECT role_id FROM user_role WHERE org_id = ? AND user_id = ?)", 1, "2") + require.NoError(t, err) + require.Equal(t, 0, count) + + err = sess.Get(ctx, &permission, "SELECT action FROM permission WHERE scope = ? AND role_id = (SELECT role_id FROM user_role WHERE org_id = ? AND user_id = ?)", "folders:uid:fold1", 1, "3") + require.NoError(t, err) + require.Equal(t, "folders:admin", permission.Action) + + err = sess.Get(ctx, &permission, "SELECT action FROM permission WHERE scope = ? AND role_id = (SELECT role_id FROM builtin_role WHERE org_id = ? AND role = ?)", "folders:uid:fold1", 1, "Editor") + require.NoError(t, err) + require.Equal(t, "folders:view", permission.Action) + }) +} + type fakeIdentityStore struct { t *testing.T @@ -499,8 +615,8 @@ type fakeIdentityStore struct { func NewFakeIdentityStore(t *testing.T) *fakeIdentityStore { return &fakeIdentityStore{ t: t, - users: map[string]int64{"captain": 101}, - serviceAccounts: map[string]int64{"robot": 201}, + users: map[string]int64{"captain": 101, "user-1": 1, "user-2": 2}, + serviceAccounts: map[string]int64{"robot": 201, "sa-1": 3}, teams: map[string]int64{"devs": 301}, expectedNs: types.NamespaceInfo{Value: "default"}, } diff --git a/pkg/registry/apis/iam/resourcepermission/storage_backend.go b/pkg/registry/apis/iam/resourcepermission/storage_backend.go index 01d3434c1ff..72449ce4bca 100644 --- a/pkg/registry/apis/iam/resourcepermission/storage_backend.go +++ b/pkg/registry/apis/iam/resourcepermission/storage_backend.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/registry/apis/iam/common" idStore "github.com/grafana/grafana/pkg/registry/apis/iam/legacy" + "github.com/grafana/grafana/pkg/services/sqlstore/session" "github.com/grafana/grafana/pkg/storage/legacysql" "github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/grafana/grafana/pkg/storage/unified/resourcepb" @@ -156,7 +157,12 @@ func (s *ResourcePermSqlBackend) ReadResource(ctx context.Context, req *resource return rsp } - resourcePermission, err := s.getResourcePermission(ctx, dbHelper, ns, req.Key.Name) + var resourcePermission *v0alpha1.ResourcePermission + err = dbHelper.DB.GetSqlxSession().WithTransaction(ctx, func(tx *session.SessionTx) error { + resourcePermission, err = s.getResourcePermission(ctx, dbHelper, tx, ns, req.Key.Name) + return err + }) + if err != nil { if errors.Is(err, errNotFound) { rsp.Error = resource.AsErrorResult( @@ -171,6 +177,7 @@ func (s *ResourcePermSqlBackend) ReadResource(ctx context.Context, req *resource } rsp.ResourceVersion = resourcePermission.GetUpdateTimestamp().UnixMilli() + resourcePermission.Namespace = ns.Value // ensure namespace is set, this is required when existing and new resources are compared for updates rsp.Value, err = json.Marshal(resourcePermission) if err != nil { rsp.Error = resource.AsErrorResult(err) @@ -245,7 +252,7 @@ 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: + case resourcepb.WatchEvent_ADDED, resourcepb.WatchEvent_MODIFIED: { var v0resourceperm *v0alpha1.ResourcePermission v0resourceperm, err = getResourcePermissionFromEvent(event) @@ -264,14 +271,22 @@ func (s *ResourcePermSqlBackend) WriteEvent(ctx context.Context, event resource. ) } - rv, err = s.createResourcePermission(ctx, dbHelper, ns, mapper, grn, v0resourceperm) + if event.Type == resourcepb.WatchEvent_ADDED { + rv, err = s.createResourcePermission(ctx, dbHelper, ns, mapper, grn, v0resourceperm) + if err != nil && errors.Is(err, errConflict) { + return 0, apierrors.NewConflict(v0alpha1.ResourcePermissionInfo.GroupResource(), event.Key.Name, err) + } + } else { + rv, err = s.updateResourcePermission(ctx, dbHelper, ns, mapper, grn, v0resourceperm) + if errors.Is(err, errNotFound) { + return 0, apierrors.NewNotFound(v0alpha1.ResourcePermissionInfo.GroupResource(), event.Key.Name) + } + } + if err != nil { if errors.Is(err, errInvalidSpec) || errors.Is(err, errInvalidName) { return 0, apierrors.NewBadRequest(err.Error()) } - if errors.Is(err, errConflict) { - return 0, apierrors.NewConflict(v0alpha1.ResourcePermissionInfo.GroupResource(), event.Key.Name, err) - } return 0, err } } diff --git a/pkg/registry/apis/iam/resourcepermission/storage_backend_test.go b/pkg/registry/apis/iam/resourcepermission/storage_backend_test.go index f8ace252491..0192251cf5c 100644 --- a/pkg/registry/apis/iam/resourcepermission/storage_backend_test.go +++ b/pkg/registry/apis/iam/resourcepermission/storage_backend_test.go @@ -708,3 +708,240 @@ func TestIntegration_WriteEvent_Delete(t *testing.T) { require.Len(t, permission.Spec.Permissions, 4) }) } + +func TestWriteEvent_Modify(t *testing.T) { + store := db.InitTestDB(t) + + timeNow = func() time.Time { + return time.Date(2025, 8, 28, 17, 13, 0, 0, time.UTC) + } + + sqlHelper := &legacysql.LegacyDatabaseHelper{ + DB: store, + Table: func(name string) string { return name }, + } + + dbProvider := func(ctx context.Context) (*legacysql.LegacyDatabaseHelper, error) { + return sqlHelper, nil + } + + t.Run("should error with invalid namespace", func(t *testing.T) { + backend := ProvideStorageBackend(dbProvider) + + rv, err := backend.WriteEvent(context.Background(), resource.WriteEvent{ + Type: resourcepb.WatchEvent_MODIFIED, + Key: &resourcepb.ResourceKey{Name: "folder.grafana.app-folders-fold1", Namespace: "invalid"}, + }) + + require.Zero(t, rv) + require.NotNil(t, err) + require.Contains(t, err.Error(), "requires a valid namespace") + }) + + t.Run("should error if there are no permission specified in the body", func(t *testing.T) { + backend := ProvideStorageBackend(dbProvider) + + resourcePerm, err := utils.MetaAccessor(&v0alpha1.ResourcePermission{ + ObjectMeta: metav1.ObjectMeta{ + Name: "folder.grafana.app-folders-fold1", + Namespace: "default", + }, + Spec: v0alpha1.ResourcePermissionSpec{ + Resource: v0alpha1.ResourcePermissionspecResource{ + ApiGroup: "folder.grafana.app", + Resource: "folders", + Name: "fold1", + }, + }, + }) + require.NoError(t, err) + + gr := v0alpha1.ResourcePermissionInfo.GroupResource() + rv, err := backend.WriteEvent(context.Background(), resource.WriteEvent{ + Type: resourcepb.WatchEvent_MODIFIED, + Key: &resourcepb.ResourceKey{Group: gr.Group, Resource: gr.Resource, Name: "folder.grafana.app-folders-fold1", Namespace: "default"}, + Object: resourcePerm, + }) + require.Zero(t, rv) + require.NotNil(t, err) + require.Contains(t, err.Error(), errInvalidSpec.Error()) + }) + + t.Run("should error if name and spec do not match", func(t *testing.T) { + backend := ProvideStorageBackend(dbProvider) + + resourcePerm, err := utils.MetaAccessor(&v0alpha1.ResourcePermission{ + ObjectMeta: metav1.ObjectMeta{ + Name: "folder.grafana.app-folders-fold1", + Namespace: "default", + }, + Spec: v0alpha1.ResourcePermissionSpec{ + Resource: v0alpha1.ResourcePermissionspecResource{ + ApiGroup: "folder.grafana.app", + Resource: "folders", + Name: "fold2", + }, + Permissions: []v0alpha1.ResourcePermissionspecPermission{ + { + Kind: v0alpha1.ResourcePermissionSpecPermissionKindBasicRole, + Name: "Viewer", + Verb: "Admin", + }, + }, + }, + }) + require.NoError(t, err) + + gr := v0alpha1.ResourcePermissionInfo.GroupResource() + rv, err := backend.WriteEvent(context.Background(), resource.WriteEvent{ + Type: resourcepb.WatchEvent_MODIFIED, + Key: &resourcepb.ResourceKey{Group: gr.Group, Resource: gr.Resource, Name: "folder.grafana.app-folders-fold1", Namespace: "default"}, + Object: resourcePerm, + }) + require.Zero(t, rv) + require.NotNil(t, err) + require.Contains(t, err.Error(), errInvalidSpec.Error()) + }) + + t.Run("should error if resource name is empty", func(t *testing.T) { + backend := ProvideStorageBackend(dbProvider) + + resourcePerm, err := utils.MetaAccessor(&v0alpha1.ResourcePermission{ + ObjectMeta: metav1.ObjectMeta{ + Name: "folder.grafana.app-folders-", + Namespace: "default", + }, + Spec: v0alpha1.ResourcePermissionSpec{ + Resource: v0alpha1.ResourcePermissionspecResource{ + ApiGroup: "folder.grafana.app", + Resource: "folders", + Name: "", + }, + Permissions: []v0alpha1.ResourcePermissionspecPermission{ + { + Kind: v0alpha1.ResourcePermissionSpecPermissionKindBasicRole, + Name: "Viewer", + Verb: "Admin", + }, + }, + }, + }) + require.NoError(t, err) + + gr := v0alpha1.ResourcePermissionInfo.GroupResource() + rv, err := backend.WriteEvent(context.Background(), resource.WriteEvent{ + Type: resourcepb.WatchEvent_MODIFIED, + Key: &resourcepb.ResourceKey{Group: gr.Group, Resource: gr.Resource, Name: "folder.grafana.app-folders-", Namespace: "default"}, + Object: resourcePerm, + }) + require.Zero(t, rv) + require.NotNil(t, err) + require.Contains(t, err.Error(), errInvalidName.Error()) + }) + + t.Run("should error if the resource is unknown", func(t *testing.T) { + backend := ProvideStorageBackend(dbProvider) + + resourcePerm, err := utils.MetaAccessor(&v0alpha1.ResourcePermission{ + ObjectMeta: metav1.ObjectMeta{ + Name: "unknown.grafana.app-unknown-ukn1", + Namespace: "default", + }, + Spec: v0alpha1.ResourcePermissionSpec{ + Resource: v0alpha1.ResourcePermissionspecResource{ + ApiGroup: "unknown.grafana.app", + Resource: "unknown", + Name: "ukn1", + }, + Permissions: []v0alpha1.ResourcePermissionspecPermission{ + { + Kind: v0alpha1.ResourcePermissionSpecPermissionKindBasicRole, + Name: "Viewer", + Verb: "Admin", + }, + }, + }, + }) + require.NoError(t, err) + + gr := v0alpha1.ResourcePermissionInfo.GroupResource() + rv, err := backend.WriteEvent(context.Background(), resource.WriteEvent{ + Type: resourcepb.WatchEvent_MODIFIED, + Key: &resourcepb.ResourceKey{Group: gr.Group, Resource: gr.Resource, Name: "unknown.grafana.app-unknown-ukn1", Namespace: "default"}, + Object: resourcePerm, + }) + require.Zero(t, rv) + require.NotNil(t, err) + require.Contains(t, err.Error(), errUnknownGroupResource.Error()) + }) + + t.Run("should work with valid resource permission", func(t *testing.T) { + backend := ProvideStorageBackend(dbProvider) + backend.identityStore = NewFakeIdentityStore(t) + + resourcePerm, err := utils.MetaAccessor(&v0alpha1.ResourcePermission{ + ObjectMeta: metav1.ObjectMeta{ + Name: "folder.grafana.app-folders-fold1", + Namespace: "default", + }, + Spec: v0alpha1.ResourcePermissionSpec{ + Resource: v0alpha1.ResourcePermissionspecResource{ + ApiGroup: "folder.grafana.app", + Resource: "folders", + Name: "fold1", + }, + Permissions: []v0alpha1.ResourcePermissionspecPermission{ + { + Kind: v0alpha1.ResourcePermissionSpecPermissionKindBasicRole, + Name: "Viewer", + Verb: "Admin", + }, + }, + }, + }) + require.NoError(t, err) + + // Create resource first + gr := v0alpha1.ResourcePermissionInfo.GroupResource() + rv, err := backend.WriteEvent(context.Background(), resource.WriteEvent{ + Type: resourcepb.WatchEvent_ADDED, + Key: &resourcepb.ResourceKey{Group: gr.Group, Resource: gr.Resource, Name: "folder.grafana.app-folders-fold1", Namespace: "default"}, + Object: resourcePerm, + }) + + require.NoError(t, err) + require.Equal(t, timeNow().UnixMilli(), rv) + + // Modify resource + resourcePerm, err = utils.MetaAccessor(&v0alpha1.ResourcePermission{ + ObjectMeta: metav1.ObjectMeta{ + Name: "folder.grafana.app-folders-fold1", + Namespace: "default", + }, + Spec: v0alpha1.ResourcePermissionSpec{ + Resource: v0alpha1.ResourcePermissionspecResource{ + ApiGroup: "folder.grafana.app", + Resource: "folders", + Name: "fold1", + }, + Permissions: []v0alpha1.ResourcePermissionspecPermission{ + { + Kind: v0alpha1.ResourcePermissionSpecPermissionKindBasicRole, + Name: "Viewer", + Verb: "Edit", + }, + }, + }, + }) + require.NoError(t, err) + + rv, err = backend.WriteEvent(context.Background(), resource.WriteEvent{ + Type: resourcepb.WatchEvent_MODIFIED, + Key: &resourcepb.ResourceKey{Group: gr.Group, Resource: gr.Resource, Name: "folder.grafana.app-folders-fold1", Namespace: "default"}, + Object: resourcePerm, + }) + + require.NoError(t, err) + require.Equal(t, timeNow().UnixMilli(), rv) + }) +} diff --git a/pkg/registry/apis/iam/resourcepermission/templates.go b/pkg/registry/apis/iam/resourcepermission/templates.go index 234a840d561..79a30fe70e8 100644 --- a/pkg/registry/apis/iam/resourcepermission/templates.go +++ b/pkg/registry/apis/iam/resourcepermission/templates.go @@ -23,6 +23,7 @@ var ( roleInsertTplt = mustTemplate("role_insert.sql") assignmentInsertTplt = mustTemplate("assignment_insert.sql") permissionInsertTplt = mustTemplate("permission_insert.sql") + permissionRemoveTplt = mustTemplate("permission_remove.sql") pageQueryTplt = mustTemplate("page_query.sql") latestUpdateTplt = mustTemplate("latest_update_query.sql") ) @@ -236,6 +237,37 @@ func buildInsertPermissionQuery(dbHelper *legacysql.LegacyDatabaseHelper, roleID // Update +type removePermissionTemplate struct { + sqltemplate.SQLTemplate + PermissionTable string + RoleTable string + Scope string + Action string + OrgID int64 + RoleName string +} + +func (t removePermissionTemplate) Validate() error { + return nil +} + +func buildRemovePermissionQuery(dbHelper *legacysql.LegacyDatabaseHelper, scope, action, roleName string, orgID int64) (string, []any, error) { + req := removePermissionTemplate{ + SQLTemplate: sqltemplate.New(dbHelper.DialectForDriver()), + PermissionTable: dbHelper.Table("permission"), + RoleTable: dbHelper.Table("role"), + Scope: scope, + Action: action, + OrgID: orgID, + RoleName: roleName, + } + rawQuery, err := sqltemplate.Execute(permissionRemoveTplt, req) + if err != nil { + return "", nil, fmt.Errorf("rendering sql template: %w", err) + } + return rawQuery, req.GetArgs(), nil +} + // Delete type deleteResourcePermissionsQueryTemplate struct { diff --git a/pkg/registry/apis/iam/resourcepermission/templates_test.go b/pkg/registry/apis/iam/resourcepermission/templates_test.go index 6bc492587b7..6229add4140 100644 --- a/pkg/registry/apis/iam/resourcepermission/templates_test.go +++ b/pkg/registry/apis/iam/resourcepermission/templates_test.go @@ -43,6 +43,20 @@ func TestTemplates(t *testing.T) { return &v } + getRemovePermission := func(scope, action, roleName string) sqltemplate.SQLTemplate { + v := removePermissionTemplate{ + SQLTemplate: sqltemplate.New(nodb.DialectForDriver()), + PermissionTable: nodb.Table("permission"), + RoleTable: nodb.Table("role"), + Scope: scope, + Action: action, + OrgID: 55, + RoleName: roleName, + } + v.SQLTemplate = mocks.NewTestingSQLTemplate() + return &v + } + getInsertAssignment := func(orgID int64, roleID int64, assignment rbacAssignmentCreate) sqltemplate.SQLTemplate { v := insertAssignmentTemplate{ SQLTemplate: sqltemplate.New(nodb.DialectForDriver()), @@ -137,6 +151,12 @@ func TestTemplates(t *testing.T) { }), }, }, + permissionRemoveTplt: { + { + Name: "remove_permission", + Data: getRemovePermission("folders:uid:folder1", "folders:edit", "managed:users:1:permissions"), + }, + }, assignmentInsertTplt: { { Name: "insert user assignment", diff --git a/pkg/registry/apis/iam/resourcepermission/testdata/mysql--permission_remove-remove_permission.sql b/pkg/registry/apis/iam/resourcepermission/testdata/mysql--permission_remove-remove_permission.sql new file mode 100755 index 00000000000..2ad8648da08 --- /dev/null +++ b/pkg/registry/apis/iam/resourcepermission/testdata/mysql--permission_remove-remove_permission.sql @@ -0,0 +1,9 @@ +DELETE FROM `grafana`.`permission` AS p +WHERE p.scope = 'folders:uid:folder1' AND p.action = 'folders:edit' +AND p.role_id = ( + SELECT r.id + FROM `grafana`.`role` AS r + WHERE r.org_id = 55 + AND r.name = 'managed:users:1:permissions' + LIMIT 1 +) diff --git a/pkg/registry/apis/iam/resourcepermission/testdata/postgres--permission_remove-remove_permission.sql b/pkg/registry/apis/iam/resourcepermission/testdata/postgres--permission_remove-remove_permission.sql new file mode 100755 index 00000000000..1a5e5326586 --- /dev/null +++ b/pkg/registry/apis/iam/resourcepermission/testdata/postgres--permission_remove-remove_permission.sql @@ -0,0 +1,9 @@ +DELETE FROM "grafana"."permission" AS p +WHERE p.scope = 'folders:uid:folder1' AND p.action = 'folders:edit' +AND p.role_id = ( + SELECT r.id + FROM "grafana"."role" AS r + WHERE r.org_id = 55 + AND r.name = 'managed:users:1:permissions' + LIMIT 1 +) diff --git a/pkg/registry/apis/iam/resourcepermission/testdata/sqlite--permission_remove-remove_permission.sql b/pkg/registry/apis/iam/resourcepermission/testdata/sqlite--permission_remove-remove_permission.sql new file mode 100755 index 00000000000..1a5e5326586 --- /dev/null +++ b/pkg/registry/apis/iam/resourcepermission/testdata/sqlite--permission_remove-remove_permission.sql @@ -0,0 +1,9 @@ +DELETE FROM "grafana"."permission" AS p +WHERE p.scope = 'folders:uid:folder1' AND p.action = 'folders:edit' +AND p.role_id = ( + SELECT r.id + FROM "grafana"."role" AS r + WHERE r.org_id = 55 + AND r.name = 'managed:users:1:permissions' + LIMIT 1 +)