public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

RBAC: Only write action sets (#112429)

Browse Source 
* implementation + broken tests

* finish tests and cleanup

* fix a bug in logic where we'd return too early for non dash and folder resources
This commit is contained in:
Ieva
2025-10-20 16:02:56 +01:00
committed by GitHub
parent c95b057cdc
commit 0a0311a2b2
9 changed files with 253 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/grafana-data/src/types/featureToggles.gen.ts
+5
View File
@@ -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;
}
pkg/services/accesscontrol/resourcepermissions/service.go
+14 -1
View File
@@ -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))
pkg/services/accesscontrol/resourcepermissions/store.go
+7 -34
View File
@@ -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
pkg/services/accesscontrol/resourcepermissions/store_test.go
+194
View File
@@ -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
pkg/services/featuremgmt/registry.go
+10
View File
@@ -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",
},
}
)
pkg/services/featuremgmt/toggles_gen.csv
+1
View File
@@ -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
1 Name Stage Owner requiresDevMode RequiresRestart FrontendOnly
272 cdnPluginsUrls experimental @grafana/plugins-platform-backend false false false
273 preventPanelChromeOverflow preview @grafana/grafana-frontend-platform false false true
274 pluginStoreServiceLoading experimental @grafana/plugins-platform-backend false false false
275 onlyStoreActionSets GA @grafana/identity-access-team false false false
pkg/services/featuremgmt/toggles_gen.go
+4
View File
@@ -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"
)
pkg/services/featuremgmt/toggles_gen.json
+18
View File
@@ -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",
pkg/setting/settings_rbac.go
-3
View File
@@ -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"))