From 248af65f9c23960fed11eed92072da820fc79cf9 Mon Sep 17 00:00:00 2001 From: Eric Leijonmarck Date: Fri, 19 Jul 2024 16:16:23 +0100 Subject: [PATCH] Actionsets: Add ability for plugins to add actions for core actionsets (i.e. `folders:edit`) (#88776) * initial commit * Action sets stored remove the dependancy for actionsets got the actionsets registered storing the permissions * fix golanglinting * remove unused struct field * wip * actionset registry for a plugin from the actionsetservice * update to make declareactionset the primary way of plugin registration and modification * declare actually extends actionsets * tests fixed * tests skipped * skip tests * skip tests * skip tests * skip tests * change to warning instead * remove step from pipeline to see if it fails due to plugin not registering * reintroduce step but remove features dependancy * add back the tests that were failing * remove comments and another skip test * fix a comment and remove unneeded changes * fix and clean up, put the behaviour behind a feature toggle * clean up * fixing tests * hard-code allowed action sets for plugins * Apply suggestions from code review Co-authored-by: Gabriel MABILLE * small cleanup --------- Co-authored-by: IevaVasiljeva Co-authored-by: Gabriel MABILLE --- pkg/api/folder_bench_test.go | 2 +- pkg/plugins/ifaces.go | 5 + pkg/plugins/manager/fakes/fakes.go | 12 ++ pkg/plugins/models.go | 8 +- pkg/plugins/plugins.go | 3 +- pkg/server/wire.go | 2 + .../accesscontrol/acimpl/service_test.go | 5 +- pkg/services/accesscontrol/errors.go | 2 + .../accesscontrol/pluginutils/utils.go | 37 +++- .../accesscontrol/resourcepermissions/fake.go | 2 +- .../resourcepermissions/service.go | 25 ++- .../resourcepermissions/service_test.go | 9 +- .../resourcepermissions/store.go | 31 +-- .../resourcepermissions/store_test.go | 189 +++++++++++++++++- .../pluginsintegration/loader/loader_test.go | 4 +- .../pluginsintegration/pipeline/pipeline.go | 6 +- .../pluginsintegration/pipeline/steps.go | 27 +++ .../pluginsintegration/test_helper.go | 4 +- 18 files changed, 326 insertions(+), 47 deletions(-) diff --git a/pkg/api/folder_bench_test.go b/pkg/api/folder_bench_test.go index 223973eb85a..c9f0f300601 100644 --- a/pkg/api/folder_bench_test.go +++ b/pkg/api/folder_bench_test.go @@ -460,7 +460,7 @@ func setupServer(b testing.TB, sc benchScenario, features featuremgmt.FeatureTog folderServiceWithFlagOn := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashStore, folderStore, sc.db.DB(), features, supportbundlestest.NewFakeBundleService(), nil) cfg := setting.NewCfg() - actionSets := resourcepermissions.NewActionSetService() + actionSets := resourcepermissions.NewActionSetService(features) acSvc := acimpl.ProvideOSSService( sc.cfg, acdb.ProvideService(sc.db), actionSets, localcache.ProvideService(), features, tracing.InitializeTracerForTest(), zanzana.NewNoopClient(), sc.db.DB(), diff --git a/pkg/plugins/ifaces.go b/pkg/plugins/ifaces.go index a129d707b0f..c98cacd192b 100644 --- a/pkg/plugins/ifaces.go +++ b/pkg/plugins/ifaces.go @@ -134,6 +134,11 @@ type RoleRegistry interface { DeclarePluginRoles(ctx context.Context, ID, name string, registrations []RoleRegistration) error } +// ActionSetRegistry handles the plugin RBAC actionsets +type ActionSetRegistry interface { + RegisterActionSets(ctx context.Context, ID string, registrations []ActionSet) error +} + // ClientMiddleware is an interface representing the ability to create a middleware // that implements the Client interface. type ClientMiddleware interface { diff --git a/pkg/plugins/manager/fakes/fakes.go b/pkg/plugins/manager/fakes/fakes.go index 6e19df7bb35..67ac1b3e4ee 100644 --- a/pkg/plugins/manager/fakes/fakes.go +++ b/pkg/plugins/manager/fakes/fakes.go @@ -391,6 +391,18 @@ func (f *FakeRoleRegistry) DeclarePluginRoles(_ context.Context, _ string, _ str return f.ExpectedErr } +type FakeActionSetRegistry struct { + ExpectedErr error +} + +func NewFakeActionSetRegistry() *FakeActionSetRegistry { + return &FakeActionSetRegistry{} +} + +func (f *FakeActionSetRegistry) RegisterActionSets(_ context.Context, _ string, _ []plugins.ActionSet) error { + return f.ExpectedErr +} + type FakePluginFiles struct { OpenFunc func(name string) (fs.File, error) RemoveFunc func() error diff --git a/pkg/plugins/models.go b/pkg/plugins/models.go index 101dd6185a7..da6bb672ea4 100644 --- a/pkg/plugins/models.go +++ b/pkg/plugins/models.go @@ -314,8 +314,6 @@ func (e Error) PublicMessage() string { return "Plugin failed to load" } -// Access-Control related definitions - // RoleRegistration stores a role and its assignments to basic roles // (Viewer, Editor, Admin, Grafana Admin) type RoleRegistration struct { @@ -335,6 +333,12 @@ type Permission struct { Scope string `json:"scope"` } +// ActionSet is the model for ActionSet in RBAC. +type ActionSet struct { + Action string `json:"action"` + Actions []string `json:"actions"` +} + type QueryCachingConfig struct { Enabled bool `json:"enabled"` TTLMS int64 `json:"TTLMs"` diff --git a/pkg/plugins/plugins.go b/pkg/plugins/plugins.go index 33c001ca626..34f15fbe8d1 100644 --- a/pkg/plugins/plugins.go +++ b/pkg/plugins/plugins.go @@ -101,7 +101,8 @@ type JSONData struct { Routes []*Route `json:"routes"` // AccessControl settings - Roles []RoleRegistration `json:"roles,omitempty"` + Roles []RoleRegistration `json:"roles,omitempty"` + ActionSets []ActionSet `json:"actionSets,omitempty"` // Panel settings SkipDataQuery bool `json:"skipDataQuery"` diff --git a/pkg/server/wire.go b/pkg/server/wire.go index 8ca6910b160..7c511b14fb4 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -35,6 +35,7 @@ import ( "github.com/grafana/grafana/pkg/login/social/socialimpl" "github.com/grafana/grafana/pkg/middleware/csrf" "github.com/grafana/grafana/pkg/middleware/loggermw" + "github.com/grafana/grafana/pkg/plugins" apiregistry "github.com/grafana/grafana/pkg/registry/apis" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" @@ -351,6 +352,7 @@ var wireBasicSet = wire.NewSet( wire.Bind(new(secretsMigrations.SecretMigrationProvider), new(*secretsMigrations.SecretMigrationProviderImpl)), resourcepermissions.NewActionSetService, wire.Bind(new(accesscontrol.ActionResolver), new(resourcepermissions.ActionSetService)), + wire.Bind(new(plugins.ActionSetRegistry), new(resourcepermissions.ActionSetService)), acimpl.ProvideAccessControl, navtreeimpl.ProvideService, wire.Bind(new(accesscontrol.AccessControl), new(*acimpl.AccessControl)), diff --git a/pkg/services/accesscontrol/acimpl/service_test.go b/pkg/services/accesscontrol/acimpl/service_test.go index 74382c44a28..ae11d5b3b84 100644 --- a/pkg/services/accesscontrol/acimpl/service_test.go +++ b/pkg/services/accesscontrol/acimpl/service_test.go @@ -805,9 +805,10 @@ func TestService_SearchUserPermissions(t *testing.T) { ac := setupTestEnv(t) if tt.withActionSets { ac.features = featuremgmt.WithFeatures(featuremgmt.FlagAccessActionSets) - actionSetSvc := resourcepermissions.NewActionSetService() + actionSetSvc := resourcepermissions.NewActionSetService(ac.features) for set, actions := range tt.actionSets { - actionSetSvc.StoreActionSet(strings.Split(set, ":")[0], strings.Split(set, ":")[1], actions) + actionSetName := resourcepermissions.GetActionSetName(strings.Split(set, ":")[0], strings.Split(set, ":")[1]) + actionSetSvc.StoreActionSet(actionSetName, actions) } ac.actionResolver = actionSetSvc } diff --git a/pkg/services/accesscontrol/errors.go b/pkg/services/accesscontrol/errors.go index 34534bda777..e4580158b3e 100644 --- a/pkg/services/accesscontrol/errors.go +++ b/pkg/services/accesscontrol/errors.go @@ -27,6 +27,8 @@ var ( ErrResolverNotFound = errors.New("no resolver found") ErrPluginIDRequired = errors.New("plugin ID is required") ErrRoleNotFound = errors.New("role not found") + + ErrActionSetValidationFailed = errutil.ValidationFailed("accesscontrol.actionSetInvalid") ) func ErrInvalidBuiltinRoleData(builtInRole string) errutil.TemplateData { diff --git a/pkg/services/accesscontrol/pluginutils/utils.go b/pkg/services/accesscontrol/pluginutils/utils.go index 7db8cf635a0..0fe7a46b199 100644 --- a/pkg/services/accesscontrol/pluginutils/utils.go +++ b/pkg/services/accesscontrol/pluginutils/utils.go @@ -2,6 +2,7 @@ package pluginutils import ( "fmt" + "slices" "strings" "github.com/grafana/grafana/pkg/plugins" @@ -19,6 +20,8 @@ var ( "folders.permissions:read": "folders:uid:", "folders.permissions:write": "folders:uid:", } + + allowedActionSets = []string{"folders:view", "folders:edit", "folders:admin"} ) // ValidatePluginPermissions errors when a permission does not match expected pattern for plugins @@ -34,10 +37,36 @@ func ValidatePluginPermissions(pluginID string, permissions []ac.Permission) err permissions[i].Scope = scopePrefix + pluginID continue } - if !strings.HasPrefix(permissions[i].Action, pluginID+":") && - !strings.HasPrefix(permissions[i].Action, pluginID+".") { - return &ac.ErrorActionPrefixMissing{Action: permissions[i].Action, - Prefixes: []string{pluginaccesscontrol.ActionAppAccess, pluginID + ":", pluginID + "."}} + if err := ValidatePluginAction(pluginID, permissions[i].Action); err != nil { + return err + } + } + + return nil +} + +func ValidatePluginAction(pluginID, action string) error { + if !strings.HasPrefix(action, pluginID+":") && + !strings.HasPrefix(action, pluginID+".") { + return &ac.ErrorActionPrefixMissing{Action: action, + Prefixes: []string{pluginaccesscontrol.ActionAppAccess, pluginID + ":", pluginID + "."}} + } + + return nil +} + +// ValidatePluginActionSet errors when a actionset does not match expected pattern for plugins +// - action set should be one of the allow-listed action sets (currently only folder action sets are supported for plugins) +// - actions should have the pluginID prefix +func ValidatePluginActionSet(pluginID string, actionSet plugins.ActionSet) error { + if !slices.Contains(allowedActionSets, actionSet.Action) { + return ac.ErrActionSetValidationFailed.Errorf("currently only folder and dashboard action sets are supported, provided action set %s is not a folder or dashboard action set", actionSet.Action) + } + + // verify that actions have the pluginID prefix, plugins are only allowed to register actions for the plugin + for _, action := range actionSet.Actions { + if err := ValidatePluginAction(pluginID, action); err != nil { + return err } } diff --git a/pkg/services/accesscontrol/resourcepermissions/fake.go b/pkg/services/accesscontrol/resourcepermissions/fake.go index 7e0665a6d37..78473253cd6 100644 --- a/pkg/services/accesscontrol/resourcepermissions/fake.go +++ b/pkg/services/accesscontrol/resourcepermissions/fake.go @@ -29,4 +29,4 @@ func (f *FakeActionSetSvc) ExpandActionSetsWithFilter(permissions []accesscontro return f.ExpectedPermissions } -func (f *FakeActionSetSvc) StoreActionSet(resource, permission string, actions []string) {} +func (f *FakeActionSetSvc) StoreActionSet(name string, actions []string) {} diff --git a/pkg/services/accesscontrol/resourcepermissions/service.go b/pkg/services/accesscontrol/resourcepermissions/service.go index 923735d44d5..cf9e3a0d915 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service.go +++ b/pkg/services/accesscontrol/resourcepermissions/service.go @@ -12,7 +12,9 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/licensing" "github.com/grafana/grafana/pkg/services/org" @@ -21,6 +23,8 @@ import ( "github.com/grafana/grafana/pkg/setting" ) +var _ plugins.ActionSetRegistry = (*InMemoryActionSets)(nil) + type Store interface { // SetUserResourcePermission sets permission for managed user role on a resource SetUserResourcePermission( @@ -70,7 +74,7 @@ func New(cfg *setting.Cfg, actionSet[a] = struct{}{} } if features.IsEnabled(context.Background(), featuremgmt.FlagAccessActionSets) { - actionSetService.StoreActionSet(options.Resource, permission, actions) + actionSetService.StoreActionSet(GetActionSetName(options.Resource, permission), actions) } } @@ -90,6 +94,7 @@ func New(cfg *setting.Cfg, store: NewStore(cfg, sqlStore, features), options: options, license: license, + log: log.New("resourcepermissions"), permissions: permissions, actions: actions, sqlStore: sqlStore, @@ -119,6 +124,7 @@ type Service struct { api *api license licensing.Licensing + log log.Logger options Options permissions []string actions []string @@ -172,9 +178,14 @@ func (s *Service) GetPermissions(ctx context.Context, user identity.Requester, r if isFolderOrDashboardAction(action) { actionSetActions := s.actionSetSvc.ResolveActionSet(action) if len(actionSetActions) > 0 { + // Add all actions for folder + if s.options.Resource == dashboards.ScopeFoldersRoot { + expandedActions = append(expandedActions, actionSetActions...) + continue + } + // This check is needed for resolving inherited permissions - we don't want to include + // actions that are not related to dashboards when expanding dashboard action sets for _, actionSetAction := range actionSetActions { - // This check is needed for resolving inherited permissions - we don't want to include - // actions that are not related to dashboards when expanding folder action sets if slices.Contains(s.actions, actionSetAction) { expandedActions = append(expandedActions, actionSetAction) } @@ -427,7 +438,9 @@ type ActionSetService interface { // ResolveActionSet resolves an action set to a list of corresponding actions. ResolveActionSet(actionSet string) []string - StoreActionSet(resource, permission string, actions []string) + StoreActionSet(name string, actions []string) + + plugins.ActionSetRegistry } // ActionSet is a struct that represents a set of actions that can be performed on a resource. @@ -439,14 +452,16 @@ type ActionSet struct { // InMemoryActionSets is an in-memory implementation of the ActionSetService. type InMemoryActionSets struct { + features featuremgmt.FeatureToggles log log.Logger actionSetToActions map[string][]string actionToActionSets map[string][]string } // NewActionSetService returns a new instance of InMemoryActionSetService. -func NewActionSetService() ActionSetService { +func NewActionSetService(features featuremgmt.FeatureToggles) ActionSetService { actionSets := &InMemoryActionSets{ + features: features, log: log.New("resourcepermissions.actionsets"), actionSetToActions: make(map[string][]string), actionToActionSets: make(map[string][]string), diff --git a/pkg/services/accesscontrol/resourcepermissions/service_test.go b/pkg/services/accesscontrol/resourcepermissions/service_test.go index 039530fe1bf..2abccafbb1c 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/service_test.go @@ -291,7 +291,7 @@ func TestService_RegisterActionSets(t *testing.T) { features = featuremgmt.WithFeatures(featuremgmt.FlagAccessActionSets) } ac := acimpl.ProvideAccessControl(features, zanzana.NewNoopClient()) - actionSets := NewActionSetService() + actionSets := NewActionSetService(features) _, err := New( setting.NewCfg(), tt.options, features, routing.NewRouteRegister(), licensingtest.NewFakeLicensing(), ac, &actest.FakeService{}, db.InitTestDB(t), nil, nil, actionSets, @@ -336,10 +336,11 @@ func setupTestEnvironment(t *testing.T, ops Options) (*Service, user.Service, te license := licensingtest.NewFakeLicensing() license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe() acService := &actest.FakeService{} - ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient()) + features := featuremgmt.WithFeatures() + ac := acimpl.ProvideAccessControl(features, zanzana.NewNoopClient()) service, err := New( - cfg, ops, featuremgmt.WithFeatures(), routing.NewRouteRegister(), license, - ac, acService, sql, teamSvc, userSvc, NewActionSetService(), + cfg, ops, features, routing.NewRouteRegister(), license, + ac, acService, sql, teamSvc, userSvc, NewActionSetService(features), ) require.NoError(t, err) diff --git a/pkg/services/accesscontrol/resourcepermissions/store.go b/pkg/services/accesscontrol/resourcepermissions/store.go index 6e45d06afd2..ef19434c420 100644 --- a/pkg/services/accesscontrol/resourcepermissions/store.go +++ b/pkg/services/accesscontrol/resourcepermissions/store.go @@ -7,7 +7,9 @@ import ( "time" "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/accesscontrol/pluginutils" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" @@ -824,22 +826,12 @@ func (s *InMemoryActionSets) ExpandActionSetsWithFilter(permissions []accesscont return expandedPermissions } -// GetActionSet returns the action set for the given action. -func (s *InMemoryActionSets) GetActionSet(actionName string) []string { - actionSet, ok := s.actionSetToActions[actionName] - if !ok { - return nil - } - return actionSet -} - -func (s *InMemoryActionSets) StoreActionSet(resource, permission string, actions []string) { - name := GetActionSetName(resource, permission) +func (s *InMemoryActionSets) StoreActionSet(name string, actions []string) { actionSet := &ActionSet{ Action: name, Actions: actions, } - s.actionSetToActions[actionSet.Action] = actions + s.actionSetToActions[actionSet.Action] = append(s.actionSetToActions[actionSet.Action], actions...) for _, action := range actions { if _, ok := s.actionToActionSets[action]; !ok { @@ -850,6 +842,21 @@ func (s *InMemoryActionSets) StoreActionSet(resource, permission string, actions s.log.Debug("stored action set", "action set name", actionSet.Action) } +// RegisterActionSets allow the caller to expand the existing action sets with additional permissions +// This is intended to be used by plugins, and currently supports extending folder and dashboard action sets +func (s *InMemoryActionSets) RegisterActionSets(ctx context.Context, pluginID string, registrations []plugins.ActionSet) error { + if !s.features.IsEnabled(ctx, featuremgmt.FlagAccessActionSets) || !s.features.IsEnabled(ctx, featuremgmt.FlagAccessControlOnCall) { + return nil + } + for _, reg := range registrations { + if err := pluginutils.ValidatePluginActionSet(pluginID, reg); err != nil { + return err + } + s.StoreActionSet(reg.Action, reg.Actions) + } + return nil +} + // GetActionSetName function creates an action set from a list of actions and stores it inmemory. func GetActionSetName(resource, permission string) string { // lower cased diff --git a/pkg/services/accesscontrol/resourcepermissions/store_test.go b/pkg/services/accesscontrol/resourcepermissions/store_test.go index 18878f52ab2..ec22e388c4a 100644 --- a/pkg/services/accesscontrol/resourcepermissions/store_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/store_test.go @@ -11,6 +11,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -781,8 +782,8 @@ func TestStore_StoreActionSet(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - asService := NewActionSetService() - asService.StoreActionSet(tt.resource, tt.action, tt.actions) + asService := NewActionSetService(featuremgmt.WithFeatures(featuremgmt.FlagAccessActionSets)) + asService.StoreActionSet(GetActionSetName(tt.resource, tt.action), tt.actions) actionSetName := GetActionSetName(tt.resource, tt.action) actionSet := asService.ResolveActionSet(actionSetName) @@ -791,11 +792,179 @@ func TestStore_StoreActionSet(t *testing.T) { } } +func TestStore_RegisterActionSet(t *testing.T) { + type actionSetTest struct { + desc string + features featuremgmt.FeatureToggles + pluginID string + pluginActions []plugins.ActionSet + coreActionSets []ActionSet + expectedErr bool + expectedActionSets []ActionSet + } + + tests := []actionSetTest{ + { + desc: "should be able to register a plugin action set if the right feature toggles are enabled", + features: featuremgmt.WithFeatures(featuremgmt.FlagAccessActionSets, featuremgmt.FlagAccessControlOnCall), + pluginID: "test-app", + pluginActions: []plugins.ActionSet{ + { + Action: "folders:view", + Actions: []string{"test-app.resource:read"}, + }, + }, + expectedActionSets: []ActionSet{ + { + Action: "folders:view", + Actions: []string{"test-app.resource:read"}, + }, + }, + }, + { + desc: "should not register plugin action set if feature toggles are missing", + features: featuremgmt.WithFeatures(featuremgmt.FlagAccessControlOnCall), + pluginID: "test-app", + pluginActions: []plugins.ActionSet{ + { + Action: "folders:view", + Actions: []string{"test-app.resource:read"}, + }, + }, + expectedActionSets: []ActionSet{}, + }, + { + desc: "should be able to register multiple plugin action sets", + features: featuremgmt.WithFeatures(featuremgmt.FlagAccessActionSets, featuremgmt.FlagAccessControlOnCall), + pluginID: "test-app", + pluginActions: []plugins.ActionSet{ + { + Action: "folders:view", + Actions: []string{"test-app.resource:read"}, + }, + { + Action: "folders:edit", + Actions: []string{"test-app.resource:write", "test-app.resource:delete"}, + }, + }, + expectedActionSets: []ActionSet{ + { + Action: "folders:view", + Actions: []string{"test-app.resource:read"}, + }, + { + Action: "folders:edit", + Actions: []string{"test-app.resource:write", "test-app.resource:delete"}, + }, + }, + }, + { + desc: "action set actions should be added not replaced", + features: featuremgmt.WithFeatures(featuremgmt.FlagAccessActionSets, featuremgmt.FlagAccessControlOnCall), + pluginID: "test-app", + pluginActions: []plugins.ActionSet{ + { + Action: "folders:view", + Actions: []string{"test-app.resource:read"}, + }, + { + Action: "folders:edit", + Actions: []string{"test-app.resource:write", "test-app.resource:delete"}, + }, + }, + coreActionSets: []ActionSet{ + { + Action: "folders:view", + Actions: []string{"folders:read"}, + }, + { + Action: "folders:edit", + Actions: []string{"folders:write", "folders:delete"}, + }, + { + Action: "folders:admin", + Actions: []string{"folders.permissions:read"}, + }, + }, + expectedActionSets: []ActionSet{ + { + Action: "folders:view", + Actions: []string{"folders:read", "test-app.resource:read"}, + }, + { + Action: "folders:edit", + Actions: []string{"folders:write", "test-app.resource:write", "folders:delete", "test-app.resource:delete"}, + }, + { + Action: "folders:admin", + Actions: []string{"folders.permissions:read"}, + }, + }, + }, + { + desc: "should not be able to register an action that doesn't have a plugin prefix", + features: featuremgmt.WithFeatures(featuremgmt.FlagAccessActionSets, featuremgmt.FlagAccessControlOnCall), + pluginID: "test-app", + pluginActions: []plugins.ActionSet{ + { + Action: "folders:view", + Actions: []string{"test-app.resource:read"}, + }, + { + Action: "folders:edit", + Actions: []string{"users:read", "test-app.resource:delete"}, + }, + }, + expectedErr: true, + }, + { + desc: "should not be able to register action set that is not in the allow list", + features: featuremgmt.WithFeatures(featuremgmt.FlagAccessActionSets, featuremgmt.FlagAccessControlOnCall), + pluginID: "test-app", + pluginActions: []plugins.ActionSet{ + { + Action: "folders:super-admin", + Actions: []string{"test-app.resource:read"}, + }, + }, + expectedErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + asService := NewActionSetService(tt.features) + + err := asService.RegisterActionSets(context.Background(), tt.pluginID, tt.pluginActions) + if tt.expectedErr { + require.Error(t, err) + return + } + require.NoError(t, err) + + for _, set := range tt.coreActionSets { + asService.StoreActionSet(set.Action, set.Actions) + } + + for _, expected := range tt.expectedActionSets { + actions := asService.ResolveActionSet(expected.Action) + assert.ElementsMatch(t, expected.Actions, actions) + } + + if len(tt.expectedActionSets) == 0 { + for _, set := range tt.pluginActions { + registeredActions := asService.ResolveActionSet(set.Action) + assert.Empty(t, registeredActions, "no actions from plugin action sets should have been registered") + } + } + }) + } +} + func TestStore_ResolveActionSet(t *testing.T) { - 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"}) + actionSetService := NewActionSetService(featuremgmt.WithFeatures(featuremgmt.FlagAccessActionSets)) + 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"}) type actionSetTest struct { desc string @@ -835,10 +1004,10 @@ func TestStore_ResolveActionSet(t *testing.T) { } func TestStore_ExpandActions(t *testing.T) { - 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"}) + actionSetService := NewActionSetService(featuremgmt.WithFeatures(featuremgmt.FlagAccessActionSets)) + 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"}) type actionSetTest struct { desc string diff --git a/pkg/services/pluginsintegration/loader/loader_test.go b/pkg/services/pluginsintegration/loader/loader_test.go index 056e61df1e3..7b25522dfed 100644 --- a/pkg/services/pluginsintegration/loader/loader_test.go +++ b/pkg/services/pluginsintegration/loader/loader_test.go @@ -1476,7 +1476,7 @@ func newLoader(t *testing.T, cfg *config.PluginManagementCfg, reg registry.Servi finder.NewLocalFinder(false), reg), pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets), pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector), - pipeline.ProvideInitializationStage(cfg, reg, backendFactory, proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), fakes.NewFakePluginEnvProvider(), tracing.InitializeTracerForTest()), + pipeline.ProvideInitializationStage(cfg, reg, backendFactory, proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), fakes.NewFakeActionSetRegistry(), fakes.NewFakePluginEnvProvider(), tracing.InitializeTracerForTest()), terminate, errTracker) } @@ -1508,7 +1508,7 @@ func newLoaderWithOpts(t *testing.T, cfg *config.PluginManagementCfg, opts loade finder.NewLocalFinder(false), reg), pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets), pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector), - pipeline.ProvideInitializationStage(cfg, reg, backendFactoryProvider, proc, authServiceRegistry, fakes.NewFakeRoleRegistry(), fakes.NewFakePluginEnvProvider(), tracing.InitializeTracerForTest()), + pipeline.ProvideInitializationStage(cfg, reg, backendFactoryProvider, proc, authServiceRegistry, fakes.NewFakeRoleRegistry(), fakes.NewFakeActionSetRegistry(), fakes.NewFakePluginEnvProvider(), tracing.InitializeTracerForTest()), terminate, errTracker) } diff --git a/pkg/services/pluginsintegration/pipeline/pipeline.go b/pkg/services/pluginsintegration/pipeline/pipeline.go index f45cb3bfd76..7b11d0ad160 100644 --- a/pkg/services/pluginsintegration/pipeline/pipeline.go +++ b/pkg/services/pluginsintegration/pipeline/pipeline.go @@ -60,13 +60,17 @@ func ProvideValidationStage(cfg *config.PluginManagementCfg, sv signature.Valida func ProvideInitializationStage(cfg *config.PluginManagementCfg, pr registry.Service, bp plugins.BackendFactoryProvider, pm process.Manager, externalServiceRegistry auth.ExternalServiceRegistry, - roleRegistry plugins.RoleRegistry, pluginEnvProvider envvars.Provider, tracer tracing.Tracer) *initialization.Initialize { + roleRegistry plugins.RoleRegistry, + actionSetRegistry plugins.ActionSetRegistry, + pluginEnvProvider envvars.Provider, + tracer tracing.Tracer) *initialization.Initialize { return initialization.New(cfg, initialization.Opts{ InitializeFuncs: []initialization.InitializeFunc{ ExternalServiceRegistrationStep(cfg, externalServiceRegistry, tracer), initialization.BackendClientInitStep(pluginEnvProvider, bp), initialization.BackendProcessStartStep(pm), RegisterPluginRolesStep(roleRegistry), + RegisterActionSetsStep(actionSetRegistry), ReportBuildMetrics, initialization.PluginRegistrationStep(pr), }, diff --git a/pkg/services/pluginsintegration/pipeline/steps.go b/pkg/services/pluginsintegration/pipeline/steps.go index 5401323d41b..ebd811e0846 100644 --- a/pkg/services/pluginsintegration/pipeline/steps.go +++ b/pkg/services/pluginsintegration/pipeline/steps.go @@ -94,6 +94,33 @@ func (r *RegisterPluginRoles) Register(ctx context.Context, p *plugins.Plugin) ( return p, nil } +// RegisterActionSets implements an InitializeFunc for registering plugin action sets. +type RegisterActionSets struct { + log log.Logger + actionSetRegistry plugins.ActionSetRegistry +} + +// RegisterActionSetsStep returns a new InitializeFunc for registering plugin action sets. +func RegisterActionSetsStep(actionRegistry plugins.ActionSetRegistry) initialization.InitializeFunc { + return newRegisterActionSets(actionRegistry).Register +} + +func newRegisterActionSets(registry plugins.ActionSetRegistry) *RegisterActionSets { + return &RegisterActionSets{ + log: log.New("plugins.actionsets.registration"), + actionSetRegistry: registry, + } +} + +// Register registers the plugin action sets. +func (r *RegisterActionSets) Register(ctx context.Context, p *plugins.Plugin) (*plugins.Plugin, error) { + if err := r.actionSetRegistry.RegisterActionSets(ctx, p.ID, p.ActionSets); err != nil { + r.log.Warn("Plugin action set registration failed", "pluginId", p.ID, "error", err) + return nil, err + } + return p, nil +} + // ReportBuildMetrics reports build information for all plugins, except core and bundled plugins. func ReportBuildMetrics(_ context.Context, p *plugins.Plugin) (*plugins.Plugin, error) { if !p.IsCorePlugin() && !p.IsBundledPlugin() { diff --git a/pkg/services/pluginsintegration/test_helper.go b/pkg/services/pluginsintegration/test_helper.go index 737cf84d47c..d74e18a4bcf 100644 --- a/pkg/services/pluginsintegration/test_helper.go +++ b/pkg/services/pluginsintegration/test_helper.go @@ -54,7 +54,7 @@ func CreateIntegrationTestCtx(t *testing.T, cfg *setting.Cfg, coreRegistry *core disc := pipeline.ProvideDiscoveryStage(pCfg, finder.NewLocalFinder(true), reg) boot := pipeline.ProvideBootstrapStage(pCfg, signature.ProvideService(pCfg, statickey.New()), assetpath.ProvideService(pCfg, cdn)) valid := pipeline.ProvideValidationStage(pCfg, signature.NewValidator(signature.NewUnsignedAuthorizer(pCfg)), angularInspector) - init := pipeline.ProvideInitializationStage(pCfg, reg, provider.ProvideService(coreRegistry), proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), nil, tracing.InitializeTracerForTest()) + init := pipeline.ProvideInitializationStage(pCfg, reg, provider.ProvideService(coreRegistry), proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), fakes.NewFakeActionSetRegistry(), nil, tracing.InitializeTracerForTest()) term, err := pipeline.ProvideTerminationStage(pCfg, reg, proc) require.NoError(t, err) @@ -100,7 +100,7 @@ func CreateTestLoader(t *testing.T, cfg *pluginsCfg.PluginManagementCfg, opts Lo if opts.Initializer == nil { reg := registry.ProvideService() coreRegistry := coreplugin.NewRegistry(make(map[string]backendplugin.PluginFactoryFunc)) - opts.Initializer = pipeline.ProvideInitializationStage(cfg, reg, provider.ProvideService(coreRegistry), process.ProvideService(), &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), nil, tracing.InitializeTracerForTest()) + opts.Initializer = pipeline.ProvideInitializationStage(cfg, reg, provider.ProvideService(coreRegistry), process.ProvideService(), &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), fakes.NewFakeActionSetRegistry(), nil, tracing.InitializeTracerForTest()) } if opts.Terminator == nil {