From bd2b248f0e3a0fe7724549af2880007c3af2da19 Mon Sep 17 00:00:00 2001 From: Ieva Date: Thu, 23 May 2024 12:14:01 +0100 Subject: [PATCH] RBAC: Clean up action set code (#88147) * 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 --- pkg/api/folder_bench_test.go | 2 +- pkg/server/wire.go | 3 +- .../accesscontrol/acimpl/accesscontrol.go | 8 -- .../acimpl/accesscontrol_test.go | 108 ------------------ .../accesscontrol/acimpl/service_test.go | 3 +- pkg/services/accesscontrol/actest/fake.go | 19 --- pkg/services/accesscontrol/api/api_test.go | 6 +- pkg/services/accesscontrol/evaluator.go | 34 ------ pkg/services/accesscontrol/resolvers.go | 21 ---- .../accesscontrol/resourcepermissions/fake.go | 24 ++++ .../resourcepermissions/service.go | 38 ++++++ .../resourcepermissions/service_test.go | 10 +- .../resourcepermissions/store.go | 40 ------- .../resourcepermissions/store_test.go | 12 +- .../extsvcaccounts/service_test.go | 3 +- 15 files changed, 82 insertions(+), 249 deletions(-) create mode 100644 pkg/services/accesscontrol/resourcepermissions/fake.go diff --git a/pkg/api/folder_bench_test.go b/pkg/api/folder_bench_test.go index c761827b221..891cb730234 100644 --- a/pkg/api/folder_bench_test.go +++ b/pkg/api/folder_bench_test.go @@ -459,7 +459,7 @@ func setupServer(b testing.TB, sc benchScenario, features featuremgmt.FeatureTog folderServiceWithFlagOn := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashStore, folderStore, sc.db, features, supportbundlestest.NewFakeBundleService(), nil) cfg := setting.NewCfg() - actionSets := resourcepermissions.NewActionSetService(ac) + actionSets := resourcepermissions.NewActionSetService() acSvc := acimpl.ProvideOSSService(sc.cfg, acdb.ProvideService(sc.db), actionSets, localcache.ProvideService(), features, tracing.InitializeTracerForTest()) folderPermissions, err := ossaccesscontrol.ProvideFolderPermissions( diff --git a/pkg/server/wire.go b/pkg/server/wire.go index 248091de127..9b950af203f 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -353,8 +353,7 @@ var wireBasicSet = wire.NewSet( secretsMigrations.ProvideSecretMigrationProvider, wire.Bind(new(secretsMigrations.SecretMigrationProvider), new(*secretsMigrations.SecretMigrationProviderImpl)), resourcepermissions.NewActionSetService, - wire.Bind(new(accesscontrol.ActionResolver), new(*resourcepermissions.InMemoryActionSets)), - wire.Bind(new(resourcepermissions.ActionSetService), new(*resourcepermissions.InMemoryActionSets)), + wire.Bind(new(accesscontrol.ActionResolver), new(resourcepermissions.ActionSetService)), acimpl.ProvideAccessControl, navtreeimpl.ProvideService, wire.Bind(new(accesscontrol.AccessControl), new(*acimpl.AccessControl)), diff --git a/pkg/services/accesscontrol/acimpl/accesscontrol.go b/pkg/services/accesscontrol/acimpl/accesscontrol.go index 3ca9142c71b..374c844c4c6 100644 --- a/pkg/services/accesscontrol/acimpl/accesscontrol.go +++ b/pkg/services/accesscontrol/acimpl/accesscontrol.go @@ -48,10 +48,6 @@ func (a *AccessControl) Evaluate(ctx context.Context, user identity.Requester, e return false, nil } - if a.features.IsEnabled(ctx, featuremgmt.FlagAccessActionSets) { - evaluator = evaluator.AppendActionSets(ctx, a.resolvers.GetActionSetResolver()) - } - a.debug(ctx, user, "Evaluating permissions", evaluator) // Test evaluation without scope resolver first, this will prevent 403 for wildcard scopes when resource does not exist if evaluator.Evaluate(permissions) { @@ -74,10 +70,6 @@ func (a *AccessControl) RegisterScopeAttributeResolver(prefix string, resolver a a.resolvers.AddScopeAttributeResolver(prefix, resolver) } -func (a *AccessControl) RegisterActionResolver(resolver accesscontrol.ActionResolver) { - a.resolvers.SetActionResolver(resolver) -} - func (a *AccessControl) debug(ctx context.Context, ident identity.Requester, msg string, eval accesscontrol.Evaluator) { namespace, id := ident.GetNamespacedID() a.log.FromContext(ctx).Debug(msg, "namespace", namespace, "id", id, "orgID", ident.GetOrgID(), "permissions", eval.GoString()) diff --git a/pkg/services/accesscontrol/acimpl/accesscontrol_test.go b/pkg/services/accesscontrol/acimpl/accesscontrol_test.go index 33b9b185e4e..9bc60141a14 100644 --- a/pkg/services/accesscontrol/acimpl/accesscontrol_test.go +++ b/pkg/services/accesscontrol/acimpl/accesscontrol_test.go @@ -2,15 +2,12 @@ package acimpl_test import ( "context" - "strings" "testing" "github.com/stretchr/testify/assert" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" - "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions" - "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/user" ) @@ -24,7 +21,6 @@ func TestAccessControl_Evaluate(t *testing.T) { expected bool expectedErr error scopeResolver accesscontrol.ScopeAttributeResolver - actionSets map[string][]string } tests := []testCase{ @@ -65,101 +61,6 @@ func TestAccessControl_Evaluate(t *testing.T) { }), expected: true, }, - { - desc: "expect user to have access when resolver translates actions to action sets", - user: user.SignedInUser{ - OrgID: 1, - Permissions: map[int64]map[string][]string{ - 1: {"folders:edit": {"folders:uid:test_folder"}}, - }, - }, - evaluator: accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, "folders:uid:test_folder"), - actionSets: map[string][]string{ - "folders:edit": {dashboards.ActionFoldersWrite, dashboards.ActionFoldersRead, dashboards.ActionDashboardsWrite, dashboards.ActionDashboardsRead}, - }, - expected: true, - }, - { - desc: "expect user to have access when resolver translates scopes, as well as expands actions to action sets", - user: user.SignedInUser{ - OrgID: 1, - Permissions: map[int64]map[string][]string{ - 1: {"folders:edit": {"folders:uid:test_folder"}}, - }, - }, - evaluator: accesscontrol.EvalPermission(dashboards.ActionDashboardsRead, "dashboards:uid:test_dashboard"), - actionSets: map[string][]string{ - "folders:edit": {dashboards.ActionFoldersWrite, dashboards.ActionFoldersRead, dashboards.ActionDashboardsWrite, dashboards.ActionDashboardsRead}, - }, - resolverPrefix: "dashboards:uid:", - scopeResolver: accesscontrol.ScopeAttributeResolverFunc(func(ctx context.Context, orgID int64, scope string) ([]string, error) { - return []string{"folders:uid:test_folder"}, nil - }), - expected: true, - }, - { - desc: "expect user to have access with eval all evaluator when resolver translates scopes, as well as expands actions to action sets", - user: user.SignedInUser{ - OrgID: 1, - Permissions: map[int64]map[string][]string{ - 1: {"folders:edit": {"folders:uid:test_folder"}}, - }, - }, - evaluator: accesscontrol.EvalAll( - accesscontrol.EvalPermission(dashboards.ActionFoldersRead, "folders:uid:test_folder"), - accesscontrol.EvalPermission(dashboards.ActionDashboardsWrite, "dashboards:uid:test_dashboard"), - ), - actionSets: map[string][]string{ - "folders:edit": {dashboards.ActionFoldersWrite, dashboards.ActionFoldersRead, dashboards.ActionDashboardsWrite, dashboards.ActionDashboardsRead}, - }, - resolverPrefix: "dashboards:uid:", - scopeResolver: accesscontrol.ScopeAttributeResolverFunc(func(ctx context.Context, orgID int64, scope string) ([]string, error) { - return []string{"folders:uid:test_folder"}, nil - }), - expected: true, - }, - { - desc: "expect user to not have access with eval all evaluator with resolvers when not all permissions resolve to permissions that the user has", - user: user.SignedInUser{ - OrgID: 1, - Permissions: map[int64]map[string][]string{ - 1: {"folders:edit": {"folders:uid:test_folder"}}, - }, - }, - evaluator: accesscontrol.EvalAll( - accesscontrol.EvalPermission(dashboards.ActionDashboardsWrite, "datasources:uid:test_ds"), - accesscontrol.EvalPermission(dashboards.ActionDashboardsWrite, "dashboards:uid:test_dashboard"), - ), - actionSets: map[string][]string{ - "folders:edit": {dashboards.ActionFoldersWrite, dashboards.ActionFoldersRead, dashboards.ActionDashboardsWrite, dashboards.ActionDashboardsRead}, - }, - resolverPrefix: "dashboards:uid:", - scopeResolver: accesscontrol.ScopeAttributeResolverFunc(func(ctx context.Context, orgID int64, scope string) ([]string, error) { - return []string{"folders:uid:test_folder"}, nil - }), - expected: false, - }, - { - desc: "expect user to have access with eval any evaluator when resolver translates scopes, as well as expands actions to action sets", - user: user.SignedInUser{ - OrgID: 1, - Permissions: map[int64]map[string][]string{ - 1: {"folders:edit": {"folders:uid:test_folder"}}, - }, - }, - evaluator: accesscontrol.EvalAny( - accesscontrol.EvalPermission(dashboards.ActionDashboardsDelete, "dashboards:uid:test_dashboard"), - accesscontrol.EvalPermission(dashboards.ActionDashboardsWrite, "dashboards:uid:test_dashboard"), - ), - actionSets: map[string][]string{ - "folders:edit": {dashboards.ActionFoldersWrite, dashboards.ActionFoldersRead, dashboards.ActionDashboardsWrite, dashboards.ActionDashboardsRead}, - }, - resolverPrefix: "dashboards:uid:", - scopeResolver: accesscontrol.ScopeAttributeResolverFunc(func(ctx context.Context, orgID int64, scope string) ([]string, error) { - return []string{"folders:uid:test_folder"}, nil - }), - expected: true, - }, } for _, tt := range tests { @@ -170,15 +71,6 @@ func TestAccessControl_Evaluate(t *testing.T) { ac.RegisterScopeAttributeResolver(tt.resolverPrefix, tt.scopeResolver) } - if tt.actionSets != nil { - actionSetResolver := resourcepermissions.NewActionSetService(ac) - for actionSet, actions := range tt.actionSets { - splitActionSet := strings.Split(actionSet, ":") - actionSetResolver.StoreActionSet(splitActionSet[0], splitActionSet[1], actions) - } - ac.RegisterActionResolver(actionSetResolver) - } - hasAccess, err := ac.Evaluate(context.Background(), &tt.user, tt.evaluator) assert.Equal(t, tt.expected, hasAccess) if tt.expectedErr != nil { diff --git a/pkg/services/accesscontrol/acimpl/service_test.go b/pkg/services/accesscontrol/acimpl/service_test.go index d5f45490d9e..ed3628f0ef0 100644 --- a/pkg/services/accesscontrol/acimpl/service_test.go +++ b/pkg/services/accesscontrol/acimpl/service_test.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" "github.com/grafana/grafana/pkg/services/accesscontrol/database" + "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions" "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/licensing" @@ -64,7 +65,7 @@ func TestUsageMetrics(t *testing.T) { s := ProvideOSSService( cfg, database.ProvideService(db.InitTestDB(t)), - &actest.FakeActionResolver{}, + &resourcepermissions.FakeActionSetSvc{}, localcache.ProvideService(), featuremgmt.WithFeatures(), tracing.InitializeTracerForTest(), diff --git a/pkg/services/accesscontrol/actest/fake.go b/pkg/services/accesscontrol/actest/fake.go index 025d3278e38..7b8059f23f1 100644 --- a/pkg/services/accesscontrol/actest/fake.go +++ b/pkg/services/accesscontrol/actest/fake.go @@ -156,22 +156,3 @@ func (f *FakePermissionsService) DeleteResourcePermissions(ctx context.Context, func (f *FakePermissionsService) MapActions(permission accesscontrol.ResourcePermission) string { return f.ExpectedMappedAction } - -type FakeActionResolver struct { - ExpectedErr error - ExpectedActionSets []string - ExpectedActions []string - ExpectedPermissions []accesscontrol.Permission -} - -func (f *FakeActionResolver) ResolveAction(action string) []string { - return f.ExpectedActionSets -} - -func (f *FakeActionResolver) ResolveActionSet(actionSet string) []string { - return f.ExpectedActions -} - -func (f *FakeActionResolver) ExpandActionSets(permissions []accesscontrol.Permission) []accesscontrol.Permission { - return f.ExpectedPermissions -} diff --git a/pkg/services/accesscontrol/api/api_test.go b/pkg/services/accesscontrol/api/api_test.go index 0502504a373..ed5c8bae082 100644 --- a/pkg/services/accesscontrol/api/api_test.go +++ b/pkg/services/accesscontrol/api/api_test.go @@ -113,7 +113,11 @@ func TestAPI_getUserPermissions(t *testing.T) { var output util.DynMap err := json.NewDecoder(res.Body).Decode(&output) require.NoError(t, err) - require.Equal(t, tt.expectedOutput, output) + for k, v := range output { + scopes, ok := tt.expectedOutput[k] + require.True(t, ok) + require.ElementsMatch(t, scopes, v) + } } }) } diff --git a/pkg/services/accesscontrol/evaluator.go b/pkg/services/accesscontrol/evaluator.go index 0fef6bb1b4f..4b082e267d4 100644 --- a/pkg/services/accesscontrol/evaluator.go +++ b/pkg/services/accesscontrol/evaluator.go @@ -16,9 +16,6 @@ type Evaluator interface { Evaluate(permissions map[string][]string) bool // MutateScopes executes a sequence of ScopeModifier functions on all embedded scopes of an evaluator and returns a new Evaluator MutateScopes(ctx context.Context, mutate ScopeAttributeMutator) (Evaluator, error) - // AppendActionSets extends the evaluator with relevant action sets - // (e.g. evaluator checking `folders:write` is extended to check for any of `folders:write`, `folders:edit`, `folders:admin`) - AppendActionSets(ctx context.Context, mutate ActionSetResolver) Evaluator // String returns a string representation of permission required by the evaluator fmt.Stringer fmt.GoStringer @@ -110,17 +107,6 @@ func (p permissionEvaluator) MutateScopes(ctx context.Context, mutate ScopeAttri return EvalPermission(p.Action, scopes...), nil } -func (p permissionEvaluator) AppendActionSets(ctx context.Context, resolve ActionSetResolver) Evaluator { - resolvedActions := resolve(ctx, p.Action) - - evals := make([]Evaluator, 0, len(resolvedActions)) - for _, action := range resolvedActions { - evals = append(evals, EvalPermission(action, p.Scopes...)) - } - - return EvalAny(evals...) -} - func (p permissionEvaluator) String() string { return p.Action } @@ -171,16 +157,6 @@ func (a allEvaluator) MutateScopes(ctx context.Context, mutate ScopeAttributeMut return EvalAll(modified...), nil } -func (a allEvaluator) AppendActionSets(ctx context.Context, resolve ActionSetResolver) Evaluator { - evals := make([]Evaluator, 0, len(a.allOf)) - for _, e := range a.allOf { - resolvedSets := e.AppendActionSets(ctx, resolve) - evals = append(evals, resolvedSets) - } - - return EvalAll(evals...) -} - func (a allEvaluator) String() string { permissions := make([]string, 0, len(a.allOf)) for _, e := range a.allOf { @@ -242,16 +218,6 @@ func (a anyEvaluator) MutateScopes(ctx context.Context, mutate ScopeAttributeMut return EvalAny(modified...), nil } -func (a anyEvaluator) AppendActionSets(ctx context.Context, resolve ActionSetResolver) Evaluator { - evals := make([]Evaluator, 0, len(a.anyOf)) - for _, e := range a.anyOf { - resolvedSets := e.AppendActionSets(ctx, resolve) - evals = append(evals, resolvedSets) - } - - return EvalAny(evals...) -} - func (a anyEvaluator) String() string { permissions := make([]string, 0, len(a.anyOf)) for _, e := range a.anyOf { diff --git a/pkg/services/accesscontrol/resolvers.go b/pkg/services/accesscontrol/resolvers.go index 43967268f8b..35942adee30 100644 --- a/pkg/services/accesscontrol/resolvers.go +++ b/pkg/services/accesscontrol/resolvers.go @@ -16,8 +16,6 @@ type ScopeAttributeResolver interface { } type ActionResolver interface { - ResolveAction(action string) []string - ResolveActionSet(actionSet string) []string ExpandActionSets(permissions []Permission) []Permission } @@ -30,8 +28,6 @@ func (f ScopeAttributeResolverFunc) Resolve(ctx context.Context, orgID int64, sc type ScopeAttributeMutator func(context.Context, string) ([]string, error) -type ActionSetResolver func(context.Context, string) []string - const ( ttl = 30 * time.Second cleanInterval = 2 * time.Minute @@ -49,7 +45,6 @@ type Resolvers struct { log log.Logger cache *localcache.CacheService attributeResolvers map[string]ScopeAttributeResolver - actionResolver ActionResolver } func (s *Resolvers) AddScopeAttributeResolver(prefix string, resolver ScopeAttributeResolver) { @@ -57,10 +52,6 @@ func (s *Resolvers) AddScopeAttributeResolver(prefix string, resolver ScopeAttri s.attributeResolvers[prefix] = resolver } -func (s *Resolvers) SetActionResolver(resolver ActionResolver) { - s.actionResolver = resolver -} - func (s *Resolvers) GetScopeAttributeMutator(orgID int64) ScopeAttributeMutator { return func(ctx context.Context, scope string) ([]string, error) { key := getScopeCacheKey(orgID, scope) @@ -90,15 +81,3 @@ func (s *Resolvers) GetScopeAttributeMutator(orgID int64) ScopeAttributeMutator func getScopeCacheKey(orgID int64, scope string) string { return fmt.Sprintf("%s-%v", scope, orgID) } - -func (s *Resolvers) GetActionSetResolver() ActionSetResolver { - return func(ctx context.Context, action string) []string { - if s.actionResolver == nil { - return []string{action} - } - actionSetActions := s.actionResolver.ResolveAction(action) - actions := append(actionSetActions, action) - s.log.Debug("Resolved action", "action", action, "resolved_actions", actions) - return actions - } -} diff --git a/pkg/services/accesscontrol/resourcepermissions/fake.go b/pkg/services/accesscontrol/resourcepermissions/fake.go new file mode 100644 index 00000000000..86b455baa80 --- /dev/null +++ b/pkg/services/accesscontrol/resourcepermissions/fake.go @@ -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) {} diff --git a/pkg/services/accesscontrol/resourcepermissions/service.go b/pkg/services/accesscontrol/resourcepermissions/service.go index c20ccea2873..0181fabf327 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service.go +++ b/pkg/services/accesscontrol/resourcepermissions/service.go @@ -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 +} diff --git a/pkg/services/accesscontrol/resourcepermissions/service_test.go b/pkg/services/accesscontrol/resourcepermissions/service_test.go index 67b350b4883..ec208395cb8 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/service_test.go @@ -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) diff --git a/pkg/services/accesscontrol/resourcepermissions/store.go b/pkg/services/accesscontrol/resourcepermissions/store.go index 814d7b53115..2f33bcc519f 100644 --- a/pkg/services/accesscontrol/resourcepermissions/store.go +++ b/pkg/services/accesscontrol/resourcepermissions/store.go @@ -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)) diff --git a/pkg/services/accesscontrol/resourcepermissions/store_test.go b/pkg/services/accesscontrol/resourcepermissions/store_test.go index 0399561474f..d80156fc01e 100644 --- a/pkg/services/accesscontrol/resourcepermissions/store_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/store_test.go @@ -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"}) diff --git a/pkg/services/serviceaccounts/extsvcaccounts/service_test.go b/pkg/services/serviceaccounts/extsvcaccounts/service_test.go index b86f8befab6..39a22eee791 100644 --- a/pkg/services/serviceaccounts/extsvcaccounts/service_test.go +++ b/pkg/services/serviceaccounts/extsvcaccounts/service_test.go @@ -14,6 +14,7 @@ import ( ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" + "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions" "github.com/grafana/grafana/pkg/services/apikey" "github.com/grafana/grafana/pkg/services/extsvcauth" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -43,7 +44,7 @@ func setupTestEnv(t *testing.T) *TestEnv { } logger := log.New("extsvcaccounts.test") env.S = &ExtSvcAccountsService{ - acSvc: acimpl.ProvideOSSService(cfg, env.AcStore, &actest.FakeActionResolver{}, localcache.New(0, 0), fmgt, tracing.InitializeTracerForTest()), + acSvc: acimpl.ProvideOSSService(cfg, env.AcStore, &resourcepermissions.FakeActionSetSvc{}, localcache.New(0, 0), fmgt, tracing.InitializeTracerForTest()), features: fmgt, logger: logger, metrics: newMetrics(nil, env.SaSvc, logger),