diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 2829497ad0b..817aafbccca 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -1223,4 +1223,9 @@ export interface FeatureToggles { * @default false */ pluginStoreServiceLoading?: boolean; + /** + * When storing dashboard and folder resource permissions, only store action sets and not the full list of underlying permission + * @default true + */ + onlyStoreActionSets?: boolean; } diff --git a/pkg/services/accesscontrol/resourcepermissions/service.go b/pkg/services/accesscontrol/resourcepermissions/service.go index da90da70b86..20892affd6e 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service.go +++ b/pkg/services/accesscontrol/resourcepermissions/service.go @@ -354,9 +354,22 @@ func (s *Service) mapPermission(permission string) ([]string, error) { return []string{}, nil } + var actions []string + + // Write action sets for folders and dashboards + if s.options.Resource == dashboards.ScopeFoldersRoot || s.options.Resource == dashboards.ScopeDashboardsRoot { + actions = append(actions, GetActionSetName(s.options.Resource, permission)) + + // If we only want to store action sets, return now + if s.features.IsEnabledGlobally(featuremgmt.FlagOnlyStoreActionSets) { + return actions, nil + } + } + for k, v := range s.options.PermissionsToActions { if permission == k { - return v, nil + actions = append(actions, v...) + return actions, nil } } return nil, ErrInvalidPermission.Build(ErrInvalidPermissionData(permission)) diff --git a/pkg/services/accesscontrol/resourcepermissions/store.go b/pkg/services/accesscontrol/resourcepermissions/store.go index a2bc9c45557..9b3587a553d 100644 --- a/pkg/services/accesscontrol/resourcepermissions/store.go +++ b/pkg/services/accesscontrol/resourcepermissions/store.go @@ -696,13 +696,13 @@ func (s *store) createPermissions(sess *db.Session, roleID int64, cmd SetResourc resource := cmd.Resource resourceID := cmd.ResourceID resourceAttribute := cmd.ResourceAttribute - permission := cmd.Permission - /* - Add ACTION SET of managed permissions to in-memory store - */ - if s.shouldStoreActionSet(resource, permission) { - actionSetName := GetActionSetName(resource, permission) - p := managedPermission(actionSetName, resource, resourceID, resourceAttribute) + + if len(missingActions) == 0 { + return nil + } + + for action := range missingActions { + p := managedPermission(action, resource, resourceID, resourceAttribute) p.RoleID = roleID p.Created = time.Now() p.Updated = time.Now() @@ -710,39 +710,12 @@ func (s *store) createPermissions(sess *db.Session, roleID int64, cmd SetResourc permissions = append(permissions, p) } - // If there are no missing actions for the resource (in case of access level downgrade or resource removal), we don't need to insert any actions - // we still want to add the action set (when permission != "") - if len(missingActions) == 0 && !s.shouldStoreActionSet(resource, permission) { - return nil - } - - // if we have actionset feature enabled and are only working with action sets - // skip adding the missing actions to the permissions table - if !s.shouldStoreActionSet(resource, permission) || !s.cfg.RBAC.OnlyStoreAccessActionSets { - for action := range missingActions { - p := managedPermission(action, resource, resourceID, resourceAttribute) - p.RoleID = roleID - p.Created = time.Now() - p.Updated = time.Now() - p.Kind, p.Attribute, p.Identifier = p.SplitScope() - permissions = append(permissions, p) - } - } - if _, err := sess.InsertMulti(&permissions); err != nil { return err } return nil } -func (s *store) shouldStoreActionSet(resource, permission string) bool { - if permission == "" { - return false - } - actionSetName := GetActionSetName(resource, permission) - return isFolderOrDashboardAction(actionSetName) -} - func deletePermissions(sess *db.Session, ids []int64) error { if len(ids) == 0 { return nil diff --git a/pkg/services/accesscontrol/resourcepermissions/store_test.go b/pkg/services/accesscontrol/resourcepermissions/store_test.go index 359002d0795..9706e1b93cd 100644 --- a/pkg/services/accesscontrol/resourcepermissions/store_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/store_test.go @@ -736,6 +736,184 @@ func TestIntegrationStore_DeleteResourcePermissions(t *testing.T) { } } +func TestIntegrationStore_setResourcePermission(t *testing.T) { + testutil.SkipIntegrationTestInShortMode(t) + + type setResourcePermissionTest struct { + desc string + orgID int64 + userID int64 + actions []string + permission string + resource string + resourceID string + resourceAttribute string + existingPermissions []accesscontrol.Permission + expectedPermissions []orgPermission + } + + tests := []setResourcePermissionTest{ + { + desc: "should set only action set permissions for folder when user doesn't have any resource permissions yet", + userID: 1, + orgID: 1, + actions: []string{"folders:edit"}, + resource: "folders", + resourceID: "1", + resourceAttribute: "uid", + expectedPermissions: []orgPermission{ + { + OrgID: 1, + Action: "folders:edit", + Scope: "folders:uid:1", + }, + }, + }, + { + desc: "should remove the existing action set and underlying resource permissions when action set is empty", + orgID: 1, + userID: 1, + actions: []string{}, + resource: "folders", + resourceID: "1", + resourceAttribute: "uid", + existingPermissions: []accesscontrol.Permission{ + { + Action: "folders:read", + Scope: "folders:uid:1", + }, + { + Action: "folders:view", + Scope: "folders:uid:1", + }, + }, + expectedPermissions: []orgPermission{}, + }, + { + desc: "when increasing access level, should replace the existing permissions with the higher action set", + orgID: 1, + userID: 1, + actions: []string{"folders:edit"}, + resource: "folders", + resourceID: "1", + resourceAttribute: "uid", + existingPermissions: []accesscontrol.Permission{ + { + Action: "folders:read", + Scope: "folders:uid:1", + }, + { + Action: "folders:view", + Scope: "folders:uid:1", + }, + }, + expectedPermissions: []orgPermission{ + { + OrgID: 1, + Action: "folders:edit", + Scope: "folders:uid:1", + }, + }, + }, + { + desc: "when decreasing access level, should replace the existing permissions with the lower action set", + orgID: 1, + userID: 1, + actions: []string{"folders:view"}, + resource: "folders", + resourceID: "1", + resourceAttribute: "uid", + existingPermissions: []accesscontrol.Permission{ + { + Action: "folders:admin", + Scope: "folders:uid:1", + }, + { + Action: "folders:delete", + Scope: "folders:uid:1", + }, + }, + expectedPermissions: []orgPermission{ + { + OrgID: 1, + Action: "folders:view", + Scope: "folders:uid:1", + }, + }, + }, + { + desc: "when updating access level, should not touch resource permissions on other resources", + orgID: 1, + userID: 1, + actions: []string{"folders:view"}, + resource: "folders", + resourceID: "1", + resourceAttribute: "uid", + existingPermissions: []accesscontrol.Permission{ + { + Action: "folders:admin", + Scope: "folders:uid:2", + }, + { + Action: "folders:delete", + Scope: "folders:uid:2", + }, + { + Action: "folders:edit", + Scope: "folders:uid:1", + }, + }, + expectedPermissions: []orgPermission{ + { + OrgID: 1, + Action: "folders:admin", + Scope: "folders:uid:2", + }, + { + OrgID: 1, + Action: "folders:delete", + Scope: "folders:uid:2", + }, + { + OrgID: 1, + Action: "folders:view", + Scope: "folders:uid:1", + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + store, _, _ := setupTestEnv(t) + + // Populate store with existing permissions + setPermissionsHelper(store, test.existingPermissions, t) + + // Set new permission + _, err := store.SetResourcePermissions(context.Background(), test.orgID, []SetResourcePermissionsCommand{ + { + User: accesscontrol.User{ID: test.userID}, + SetResourcePermissionCommand: SetResourcePermissionCommand{ + Actions: test.actions, + Resource: test.resource, + ResourceID: test.resourceID, + ResourceAttribute: test.resourceAttribute, + Permission: test.permission, + }, + }, + }, ResourceHooks{}) + require.NoError(t, err) + + // Get permissions directly from DB to verify + permissions := retrievePermissionsHelper(store, t) + fmt.Println(permissions) + + require.Equal(t, test.expectedPermissions, permissions) + }) + } +} + func retrievePermissionsHelper(store *store, t *testing.T) []orgPermission { permissions := []orgPermission{} err := store.sql.WithDbSession(context.Background(), func(sess *db.Session) error { @@ -752,6 +930,22 @@ func retrievePermissionsHelper(store *store, t *testing.T) []orgPermission { return permissions } +func setPermissionsHelper(store *store, permissions []accesscontrol.Permission, t *testing.T) { + err := store.sql.WithTransactionalDbSession(context.Background(), func(sess *db.Session) error { + sql := "INSERT INTO permission(role_id, action, scope, created, updated, kind, attribute, identifier) VALUES(?, ?, ?, ?, ?, ?, ?, ?)" + for _, p := range permissions { + kind, attribute, identifier := p.SplitScope() + // Hardcode role_id to 1 since the test only tests one managed role, would need to extend the logic if we test with several managed roles + _, err := sess.Exec(sql, 1, p.Action, p.Scope, time.Now(), time.Now(), kind, attribute, identifier) + if err != nil { + return err + } + } + return nil + }) + require.NoError(t, err) +} + func TestStore_StoreActionSet(t *testing.T) { type actionSetTest struct { desc string diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index cb7ed8020a5..738ea415852 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -2119,6 +2119,16 @@ var ( Owner: grafanaPluginsPlatformSquad, Expression: "false", }, + { + Name: "onlyStoreActionSets", + Description: "When storing dashboard and folder resource permissions, only store action sets and not the full list of underlying permission", + Stage: FeatureStageGeneralAvailability, + FrontendOnly: false, + HideFromDocs: true, + HideFromAdminPage: true, // this should not be a user facing change + Owner: identityAccessTeam, + Expression: "true", + }, } ) diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 744e7c10dfa..e3b2f70df20 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -272,3 +272,4 @@ cdnPluginsLoadFirst,experimental,@grafana/plugins-platform-backend,false,false,f cdnPluginsUrls,experimental,@grafana/plugins-platform-backend,false,false,false preventPanelChromeOverflow,preview,@grafana/grafana-frontend-platform,false,false,true pluginStoreServiceLoading,experimental,@grafana/plugins-platform-backend,false,false,false +onlyStoreActionSets,GA,@grafana/identity-access-team,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index c501ebb4e91..e49d4f7d05b 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -1097,4 +1097,8 @@ const ( // FlagPluginStoreServiceLoading // Load plugins during store service startup instead of wire provider FlagPluginStoreServiceLoading = "pluginStoreServiceLoading" + + // FlagOnlyStoreActionSets + // When storing dashboard and folder resource permissions, only store action sets and not the full list of underlying permission + FlagOnlyStoreActionSets = "onlyStoreActionSets" ) diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index cab882b702c..38a432381a0 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -2774,6 +2774,24 @@ "expression": "true" } }, + { + "metadata": { + "name": "onlyStoreActionSets", + "resourceVersion": "1759844046154", + "creationTimestamp": "2025-10-07T13:24:26Z", + "annotations": { + "grafana.app/updatedTimestamp": "2025-10-07 13:34:06.15476 +0000 UTC" + } + }, + "spec": { + "description": "When storing dashboard and folder resource permissions, only store action sets and not the full list of underlying permission", + "stage": "GA", + "codeowner": "@grafana/identity-access-team", + "hideFromAdminPage": true, + "hideFromDocs": true, + "expression": "true" + } + }, { "metadata": { "name": "otelLogsFormatting", diff --git a/pkg/setting/settings_rbac.go b/pkg/setting/settings_rbac.go index 5bfd91fedcc..ed598239a16 100644 --- a/pkg/setting/settings_rbac.go +++ b/pkg/setting/settings_rbac.go @@ -20,8 +20,6 @@ type RBACSettings struct { // run the zanzana reconciliation loop. ZanzanaReconciliationInterval time.Duration - OnlyStoreAccessActionSets bool - // set of resources that should generate managed permissions when created resourcesWithPermissionsOnCreation map[string]struct{} @@ -37,7 +35,6 @@ func (cfg *Cfg) readRBACSettings() { s.PermissionValidationEnabled = rbac.Key("permission_validation_enabled").MustBool(false) s.ResetBasicRoles = rbac.Key("reset_basic_roles").MustBool(false) s.SingleOrganization = rbac.Key("single_organization").MustBool(false) - s.OnlyStoreAccessActionSets = rbac.Key("only_store_access_action_sets").MustBool(false) // List of resources to generate managed permissions for upon resource creation (dashboard, folder, service-account, datasource) resources := util.SplitString(rbac.Key("resources_with_managed_permissions_on_creation").MustString("dashboard, folder, service-account, datasource"))