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

RBAC: Clean up action set code (#88147)

Browse Source 
* remove unused action set code, refactor the existing code

* fix import ordering

* use a separate interface for permission expansion after all, to avoid circular dependencies

* add comments, fix a test
This commit is contained in:
Ieva
2024-05-23 12:14:01 +01:00
committed by GitHub
parent 84ef99c1dc
commit bd2b248f0e
15 changed files with 82 additions and 249 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/accesscontrol/resourcepermissions/fake.go
+24
View File
@@ -0,0 +1,24 @@
package resourcepermissions
import "github.com/grafana/grafana/pkg/services/accesscontrol"
type FakeActionSetSvc struct {
ExpectedErr error
ExpectedActionSets []string
ExpectedActions []string
ExpectedPermissions []accesscontrol.Permission
}
func (f *FakeActionSetSvc) ResolveAction(action string) []string {
return f.ExpectedActionSets
}
func (f *FakeActionSetSvc) ResolveActionSet(actionSet string) []string {
return f.ExpectedActions
}
func (f *FakeActionSetSvc) ExpandActionSets(permissions []accesscontrol.Permission) []accesscontrol.Permission {
return f.ExpectedPermissions
}
func (f *FakeActionSetSvc) StoreActionSet(resource, permission string, actions []string) {}
pkg/services/accesscontrol/resourcepermissions/service.go
+38
View File
@@ -8,6 +8,7 @@ import (
"github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/featuremgmt"
@@ -368,3 +369,40 @@ func (s *Service) declareFixedRoles() error {
return s.service.DeclareFixedRoles(readerRole, writerRole)
}
type ActionSetService interface {
// ActionResolver defines method for expanding permissions from permissions with action sets to fine-grained permissions.
// We use an ActionResolver interface to avoid circular dependencies
accesscontrol.ActionResolver
// ResolveAction returns all the action sets that the action belongs to.
ResolveAction(action string) []string
// ResolveActionSet resolves an action set to a list of corresponding actions.
ResolveActionSet(actionSet string) []string
StoreActionSet(resource, permission string, actions []string)
}
// ActionSet is a struct that represents a set of actions that can be performed on a resource.
// An example of an action set is "folders:edit" which represents the set of RBAC actions that are granted by edit access to a folder.
type ActionSet struct {
Action string `json:"action"`
Actions []string `json:"actions"`
}
// InMemoryActionSets is an in-memory implementation of the ActionSetService.
type InMemoryActionSets struct {
log log.Logger
actionSetToActions map[string][]string
actionToActionSets map[string][]string
}
// NewActionSetService returns a new instance of InMemoryActionSetService.
func NewActionSetService() ActionSetService {
actionSets := &InMemoryActionSets{
log: log.New("resourcepermissions.actionsets"),
actionSetToActions: make(map[string][]string),
actionToActionSets: make(map[string][]string),
}
return actionSets
}
pkg/services/accesscontrol/resourcepermissions/service_test.go
+5 -5
View File
@@ -290,7 +290,7 @@ func TestService_RegisterActionSets(t *testing.T) {
features = featuremgmt.WithFeatures(featuremgmt.FlagAccessActionSets)
}
ac := acimpl.ProvideAccessControl(features)
actionSets := NewActionSetService(ac)
actionSets := NewActionSetService()
_, err := New(
setting.NewCfg(), tt.options, features, routing.NewRouteRegister(), licensingtest.NewFakeLicensing(),
ac, &actest.FakeService{}, db.InitTestDB(t), nil, nil, actionSets,
@@ -299,14 +299,14 @@ func TestService_RegisterActionSets(t *testing.T) {
if len(tt.expectedActionSets) > 0 {
for _, expectedActionSet := range tt.expectedActionSets {
actionSet := actionSets.GetActionSet(expectedActionSet.Action)
actionSet := actionSets.ResolveActionSet(expectedActionSet.Action)
assert.ElementsMatch(t, expectedActionSet.Actions, actionSet)
}
} else {
// Check that action sets have not been registered
for permission := range tt.options.PermissionsToActions {
actionSetName := GetActionSetName(tt.options.Resource, permission)
assert.Nil(t, actionSets.GetActionSet(actionSetName))
assert.Nil(t, actionSets.ResolveActionSet(actionSetName))
}
}
})
@@ -334,11 +334,11 @@ func setupTestEnvironment(t *testing.T, ops Options) (*Service, user.Service, te
license := licensingtest.NewFakeLicensing()
license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe()
ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures())
acService := &actest.FakeService{}
ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures())
service, err := New(
cfg, ops, featuremgmt.WithFeatures(), routing.NewRouteRegister(), license,
ac, acService, sql, teamSvc, userSvc, NewActionSetService(ac),
ac, acService, sql, teamSvc, userSvc, NewActionSetService(),
)
require.NoError(t, err)
pkg/services/accesscontrol/resourcepermissions/store.go
-40
View File
@@ -7,9 +7,7 @@ import (
"time"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/org"
@@ -729,44 +727,6 @@ func managedPermission(action, resource string, resourceID, resourceAttribute st
}
}
/*
ACTION SETS
Stores actionsets IN MEMORY
*/
// ActionSet is a struct that represents a set of actions that can be performed on a resource.
// An example of an action set is "folders:edit" which represents the set of RBAC actions that are granted by edit access to a folder.
type ActionSetService interface {
accesscontrol.ActionResolver
GetActionSet(actionName string) []string
//GetActionSetName(resource, permission string) string
StoreActionSet(resource, permission string, actions []string)
}
type ActionSet struct {
Action string `json:"action"`
Actions []string `json:"actions"`
}
// InMemoryActionSets is an in-memory implementation of the ActionSetService.
type InMemoryActionSets struct {
log log.Logger
actionSetToActions map[string][]string
actionToActionSets map[string][]string
}
// NewActionSetService returns a new instance of InMemoryActionSetService.
func NewActionSetService(a *acimpl.AccessControl) *InMemoryActionSets {
actionSets := &InMemoryActionSets{
log: log.New("resourcepermissions.actionsets"),
actionSetToActions: make(map[string][]string),
actionToActionSets: make(map[string][]string),
}
a.RegisterActionResolver(actionSets)
return actionSets
}
func (s *InMemoryActionSets) ResolveAction(action string) []string {
actionSets := s.actionToActionSets[action]
sets := make([]string, 0, len(actionSets))
pkg/services/accesscontrol/resourcepermissions/store_test.go
+4 -8
View File
@@ -12,7 +12,6 @@ import (
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/org"
@@ -782,21 +781,18 @@ func TestStore_StoreActionSet(t *testing.T) {
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
store, _, _ := setupTestEnv(t)
store.features = featuremgmt.WithFeatures(featuremgmt.FlagAccessActionSets)
ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures())
asService := NewActionSetService(ac)
asService := NewActionSetService()
asService.StoreActionSet(tt.resource, tt.action, tt.actions)
actionSetName := GetActionSetName(tt.resource, tt.action)
actionSet := asService.GetActionSet(actionSetName)
actionSet := asService.ResolveActionSet(actionSetName)
require.Equal(t, tt.actions, actionSet)
})
}
}
func TestStore_ResolveActionSet(t *testing.T) {
actionSetService := NewActionSetService(acimpl.ProvideAccessControl(featuremgmt.WithFeatures()))
actionSetService := NewActionSetService()
actionSetService.StoreActionSet("folders", "edit", []string{"folders:read", "folders:write", "dashboards:read", "dashboards:write"})
actionSetService.StoreActionSet("folders", "view", []string{"folders:read", "dashboards:read"})
actionSetService.StoreActionSet("dashboards", "view", []string{"dashboards:read"})
@@ -839,7 +835,7 @@ func TestStore_ResolveActionSet(t *testing.T) {
}
func TestStore_ExpandActions(t *testing.T) {
actionSetService := NewActionSetService(acimpl.ProvideAccessControl(featuremgmt.WithFeatures()))
actionSetService := NewActionSetService()
actionSetService.StoreActionSet("folders", "edit", []string{"folders:read", "folders:write", "dashboards:read", "dashboards:write"})
actionSetService.StoreActionSet("folders", "view", []string{"folders:read", "dashboards:read"})
actionSetService.StoreActionSet("dashboards", "view", []string{"dashboards:read"})