RBAC: Expand action sets when fetching permissions (#87967)

* logic to expand action set to the underlying actions when permissions are fetched from the DB

* updates needed for dependency injection

* clean up some code, also deduplicate scopes when grouping scopes and actions

* expand on a comment

* rename a method

pkg/api/folder_bench_test.go

@@ -448,8 +448,6 @@ func setupServer(b testing.TB, sc benchScenario, features featuremgmt.FeatureTog
 	license := licensingtest.NewFakeLicensing()
 	license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe()
 
-	acSvc := acimpl.ProvideOSSService(sc.cfg, acdb.ProvideService(sc.db), localcache.ProvideService(), features, tracing.InitializeTracerForTest())
-
 	quotaSrv := quotatest.New(false, nil)
 
 	dashStore, err := database.ProvideDashboardStore(sc.db, sc.cfg, features, tagimpl.ProvideService(sc.db), quotaSrv)
@@ -462,6 +460,8 @@ func setupServer(b testing.TB, sc benchScenario, features featuremgmt.FeatureTog
 
 	cfg := setting.NewCfg()
 	actionSets := resourcepermissions.NewActionSetService(ac)
+	acSvc := acimpl.ProvideOSSService(sc.cfg, acdb.ProvideService(sc.db), actionSets, localcache.ProvideService(), features, tracing.InitializeTracerForTest())
+
 	folderPermissions, err := ossaccesscontrol.ProvideFolderPermissions(
 		cfg, features, routing.NewRouteRegister(), sc.db, ac, license, &dashboards.FakeDashboardStore{}, folderServiceWithFlagOn, acSvc, sc.teamSvc, sc.userSvc, actionSets)
 	require.NoError(b, err)

pkg/cmd/grafana-cli/commands/conflict_user_command.go

@@ -89,7 +89,7 @@ func initializeConflictResolver(cmd *utils.ContextCommandLine, f Formatter, ctx
 	if err != nil {
 		return nil, fmt.Errorf("%v: %w", "failed to initialize tracer service", err)
 	}
-	acService, err := acimpl.ProvideService(cfg, s, routing, nil, nil, features, tracer)
+	acService, err := acimpl.ProvideService(cfg, s, routing, nil, nil, nil, features, tracer)
 	if err != nil {
 		return nil, fmt.Errorf("%v: %w", "failed to get access control", err)
 	}

pkg/server/wire.go

@@ -351,6 +351,9 @@ var wireBasicSet = wire.NewSet(
 	secretsMigrations.ProvideMigrateFromPluginService,
 	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)),
 	acimpl.ProvideAccessControl,
 	navtreeimpl.ProvideService,
 	wire.Bind(new(accesscontrol.AccessControl), new(*acimpl.AccessControl)),
@@ -382,7 +385,6 @@ var wireBasicSet = wire.NewSet(
 	// Kubernetes API server
 	grafanaapiserver.WireSet,
 	apiregistry.WireSet,
-	resourcepermissions.NewActionSetService,
 )
 
 var wireSet = wire.NewSet(

pkg/services/accesscontrol/accesscontrol.go

@@ -232,11 +232,29 @@ func BuildPermissionsMap(permissions []Permission) map[string]bool {
 
 // GroupScopesByAction will group scopes on action
 func GroupScopesByAction(permissions []Permission) map[string][]string {
-	m := make(map[string][]string)
+	// Use a map to deduplicate scopes.
+	// User can have the same permission from multiple sources (e.g. team, basic role, directly assigned etc).
+	// User will also have duplicate permissions if action sets are used, as we will be double writing permissions for a while.
+	m := make(map[string]map[string]struct{})
 	for i := range permissions {
-		m[permissions[i].Action] = append(m[permissions[i].Action], permissions[i].Scope)
+		if _, ok := m[permissions[i].Action]; !ok {
+			m[permissions[i].Action] = make(map[string]struct{})
+		}
+		m[permissions[i].Action][permissions[i].Scope] = struct{}{}
 	}
-	return m
+
+	res := make(map[string][]string, len(m))
+	for action, scopes := range m {
+		scopeList := make([]string, len(scopes))
+		i := 0
+		for scope := range scopes {
+			scopeList[i] = scope
+			i++
+		}
+		res[action] = scopeList
+	}
+
+	return res
 }
 
 // Reduce will reduce a list of permissions to its minimal form, grouping scopes by action

pkg/services/accesscontrol/acimpl/service.go

@@ -46,8 +46,8 @@ var SharedWithMeFolderPermission = accesscontrol.Permission{
 var OSSRolesPrefixes = []string{accesscontrol.ManagedRolePrefix, accesscontrol.ExternalServiceRolePrefix}
 
 func ProvideService(cfg *setting.Cfg, db db.DB, routeRegister routing.RouteRegister, cache *localcache.CacheService,
-	accessControl accesscontrol.AccessControl, features featuremgmt.FeatureToggles, tracer tracing.Tracer) (*Service, error) {
-	service := ProvideOSSService(cfg, database.ProvideService(db), cache, features, tracer)
+	accessControl accesscontrol.AccessControl, actionResolver accesscontrol.ActionResolver, features featuremgmt.FeatureToggles, tracer tracing.Tracer) (*Service, error) {
+	service := ProvideOSSService(cfg, database.ProvideService(db), actionResolver, cache, features, tracer)
 
 	api.NewAccessControlAPI(routeRegister, accessControl, service, features).RegisterAPIEndpoints()
 	if err := accesscontrol.DeclareFixedRoles(service, cfg); err != nil {
@@ -65,15 +65,16 @@ func ProvideService(cfg *setting.Cfg, db db.DB, routeRegister routing.RouteRegis
 	return service, nil
 }
 
-func ProvideOSSService(cfg *setting.Cfg, store accesscontrol.Store, cache *localcache.CacheService, features featuremgmt.FeatureToggles, tracer tracing.Tracer) *Service {
+func ProvideOSSService(cfg *setting.Cfg, store accesscontrol.Store, actionResolver accesscontrol.ActionResolver, cache *localcache.CacheService, features featuremgmt.FeatureToggles, tracer tracing.Tracer) *Service {
 	s := &Service{
-		cache:    cache,
-		cfg:      cfg,
-		features: features,
-		log:      log.New("accesscontrol.service"),
-		roles:    accesscontrol.BuildBasicRoleDefinitions(),
-		store:    store,
-		tracer:   tracer,
+		actionResolver: actionResolver,
+		cache:          cache,
+		cfg:            cfg,
+		features:       features,
+		log:            log.New("accesscontrol.service"),
+		roles:          accesscontrol.BuildBasicRoleDefinitions(),
+		store:          store,
+		tracer:         tracer,
 	}
 
 	return s
@@ -81,14 +82,15 @@ func ProvideOSSService(cfg *setting.Cfg, store accesscontrol.Store, cache *local
 
 // Service is the service implementing role based access control.
 type Service struct {
-	cache         *localcache.CacheService
-	cfg           *setting.Cfg
-	features      featuremgmt.FeatureToggles
-	log           log.Logger
-	registrations accesscontrol.RegistrationList
-	roles         map[string]*accesscontrol.RoleDTO
-	store         accesscontrol.Store
-	tracer        tracing.Tracer
+	actionResolver accesscontrol.ActionResolver
+	cache          *localcache.CacheService
+	cfg            *setting.Cfg
+	features       featuremgmt.FeatureToggles
+	log            log.Logger
+	registrations  accesscontrol.RegistrationList
+	roles          map[string]*accesscontrol.RoleDTO
+	store          accesscontrol.Store
+	tracer         tracing.Tracer
 }
 
 func (s *Service) GetUsageStats(_ context.Context) map[string]any {
@@ -138,6 +140,9 @@ func (s *Service) getUserPermissions(ctx context.Context, user identity.Requeste
 	if err != nil {
 		return nil, err
 	}
+	if s.features.IsEnabled(ctx, featuremgmt.FlagAccessActionSets) {
+		dbPermissions = s.actionResolver.ExpandActionSets(dbPermissions)
+	}
 
 	return append(permissions, dbPermissions...), nil
 }
@@ -157,8 +162,11 @@ func (s *Service) getBasicRolePermissions(ctx context.Context, role string, orgI
 		OrgID:        orgID,
 		RolePrefixes: OSSRolesPrefixes,
 	})
-	permissions = append(permissions, dbPermissions...)
-	return permissions, err
+	if s.features.IsEnabled(ctx, featuremgmt.FlagAccessActionSets) {
+		dbPermissions = s.actionResolver.ExpandActionSets(dbPermissions)
+	}
+
+	return append(permissions, dbPermissions...), err
 }
 
 func (s *Service) getTeamsPermissions(ctx context.Context, teamIDs []int64, orgID int64) (map[int64][]accesscontrol.Permission, error) {
@@ -170,6 +178,13 @@ func (s *Service) getTeamsPermissions(ctx context.Context, teamIDs []int64, orgI
 		OrgID:        orgID,
 		RolePrefixes: OSSRolesPrefixes,
 	})
+
+	if s.features.IsEnabled(ctx, featuremgmt.FlagAccessActionSets) {
+		for teamID, permissions := range teamPermissions {
+			teamPermissions[teamID] = s.actionResolver.ExpandActionSets(permissions)
+		}
+	}
+
 	return teamPermissions, err
 }
 
@@ -199,6 +214,9 @@ func (s *Service) getUserDirectPermissions(ctx context.Context, user identity.Re
 		return nil, err
 	}
 
+	if s.features.IsEnabled(ctx, featuremgmt.FlagAccessActionSets) {
+		permissions = s.actionResolver.ExpandActionSets(permissions)
+	}
 	if s.features.IsEnabled(ctx, featuremgmt.FlagNestedFolders) {
 		permissions = append(permissions, SharedWithMeFolderPermission)
 	}
@@ -404,6 +422,7 @@ func (s *Service) DeclarePluginRoles(ctx context.Context, ID, name string, regs
 	return nil
 }
 
+// TODO potential changes needed here?
 // SearchUsersPermissions returns all users' permissions filtered by action prefixes
 func (s *Service) SearchUsersPermissions(ctx context.Context, usr identity.Requester,
 	options accesscontrol.SearchOptions) (map[int64][]accesscontrol.Permission, error) {
@@ -418,6 +437,7 @@ func (s *Service) SearchUsersPermissions(ctx context.Context, usr identity.Reque
 
 		// Reroute to the user specific implementation of search permissions
 		// because it leverages the user permission cache.
+		// TODO
 		userPerms, err := s.SearchUserPermissions(ctx, usr.GetOrgID(), options)
 		if err != nil {
 			return nil, err

pkg/services/accesscontrol/acimpl/service_test.go

@@ -64,6 +64,7 @@ func TestUsageMetrics(t *testing.T) {
 			s := ProvideOSSService(
 				cfg,
 				database.ProvideService(db.InitTestDB(t)),
+				&actest.FakeActionResolver{},
 				localcache.ProvideService(),
 				featuremgmt.WithFeatures(),
 				tracing.InitializeTracerForTest(),

pkg/services/accesscontrol/actest/fake.go

@@ -156,3 +156,22 @@ 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
+}

pkg/services/accesscontrol/resolvers.go

@@ -16,7 +16,9 @@ type ScopeAttributeResolver interface {
 }
 
 type ActionResolver interface {
-	Resolve(action string) []string
+	ResolveAction(action string) []string
+	ResolveActionSet(actionSet string) []string
+	ExpandActionSets(permissions []Permission) []Permission
 }
 
 // ScopeAttributeResolverFunc is an adapter to allow functions to implement ScopeAttributeResolver interface
@@ -94,7 +96,7 @@ func (s *Resolvers) GetActionSetResolver() ActionSetResolver {
 		if s.actionResolver == nil {
 			return []string{action}
 		}
-		actionSetActions := s.actionResolver.Resolve(action)
+		actionSetActions := s.actionResolver.ResolveAction(action)
 		actions := append(actionSetActions, action)
 		s.log.Debug("Resolved action", "action", action, "resolved_actions", actions)
 		return actions

pkg/services/accesscontrol/resourcepermissions/store.go

@@ -10,6 +10,7 @@ import (
 	"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"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
@@ -756,7 +757,7 @@ type InMemoryActionSets struct {
 }
 
 // NewActionSetService returns a new instance of InMemoryActionSetService.
-func NewActionSetService(a *acimpl.AccessControl) ActionSetService {
+func NewActionSetService(a *acimpl.AccessControl) *InMemoryActionSets {
 	actionSets := &InMemoryActionSets{
 		log:                log.New("resourcepermissions.actionsets"),
 		actionSetToActions: make(map[string][]string),
@@ -766,20 +767,14 @@ func NewActionSetService(a *acimpl.AccessControl) ActionSetService {
 	return actionSets
 }
 
-func (s *InMemoryActionSets) Resolve(action string) []string {
+func (s *InMemoryActionSets) ResolveAction(action string) []string {
 	actionSets := s.actionToActionSets[action]
 	sets := make([]string, 0, len(actionSets))
 
 	for _, actionSet := range actionSets {
-		setParts := strings.Split(actionSet, ":")
-		if len(setParts) != 2 {
-			s.log.Debug("skipping resolution for action set with invalid name", "action set", actionSet)
-			continue
-		}
-		prefix := setParts[0]
 		// Only use action sets for folders and dashboards for now
-		// We need to verify that action sets for other resources do not share names with actions (eg, `datasources:query`)
-		if prefix != "folders" && prefix != "dashboards" {
+		// We need to verify that action sets for other resources do not share names with actions (eg, `datasources:read`)
+		if !isFolderOrDashboardAction(actionSet) {
 			continue
 		}
 		sets = append(sets, actionSet)
@@ -788,6 +783,35 @@ func (s *InMemoryActionSets) Resolve(action string) []string {
 	return sets
 }
 
+func (s *InMemoryActionSets) ResolveActionSet(actionSet string) []string {
+	// Only use action sets for folders and dashboards for now
+	// We need to verify that action sets for other resources do not share names with actions (eg, `datasources:read`)
+	if !isFolderOrDashboardAction(actionSet) {
+		return nil
+	}
+	return s.actionSetToActions[actionSet]
+}
+
+func isFolderOrDashboardAction(action string) bool {
+	return strings.HasPrefix(action, dashboards.ScopeDashboardsRoot) || strings.HasPrefix(action, dashboards.ScopeFoldersRoot)
+}
+
+func (s *InMemoryActionSets) ExpandActionSets(permissions []accesscontrol.Permission) []accesscontrol.Permission {
+	var expandedPermissions []accesscontrol.Permission
+	for _, permission := range permissions {
+		resolvedActions := s.ResolveActionSet(permission.Action)
+		if len(resolvedActions) == 0 {
+			expandedPermissions = append(expandedPermissions, permission)
+			continue
+		}
+		for _, action := range resolvedActions {
+			permission.Action = action
+			expandedPermissions = append(expandedPermissions, permission)
+		}
+	}
+	return expandedPermissions
+}
+
 // GetActionSet returns the action set for the given action.
 func (s *InMemoryActionSets) GetActionSet(actionName string) []string {
 	actionSet, ok := s.actionSetToActions[actionName]

pkg/services/accesscontrol/resourcepermissions/store_test.go

@@ -796,10 +796,6 @@ func TestStore_StoreActionSet(t *testing.T) {
 }
 
 func TestStore_ResolveActionSet(t *testing.T) {
-	if testing.Short() {
-		t.Skip("skipping integration test")
-	}
-
 	actionSetService := NewActionSetService(acimpl.ProvideAccessControl(featuremgmt.WithFeatures()))
 	actionSetService.StoreActionSet("folders", "edit", []string{"folders:read", "folders:write", "dashboards:read", "dashboards:write"})
 	actionSetService.StoreActionSet("folders", "view", []string{"folders:read", "dashboards:read"})
@@ -836,8 +832,105 @@ func TestStore_ResolveActionSet(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			actionSets := actionSetService.Resolve(tt.action)
+			actionSets := actionSetService.ResolveAction(tt.action)
 			require.ElementsMatch(t, tt.expectedActionSets, actionSets)
 		})
 	}
 }
+
+func TestStore_ExpandActions(t *testing.T) {
+	actionSetService := NewActionSetService(acimpl.ProvideAccessControl(featuremgmt.WithFeatures()))
+	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
+		permissions         []accesscontrol.Permission
+		expectedPermissions []accesscontrol.Permission
+	}
+
+	tests := []actionSetTest{
+		{
+			desc:                "should return empty list if no permissions are passed in",
+			permissions:         []accesscontrol.Permission{},
+			expectedPermissions: []accesscontrol.Permission{},
+		},
+		{
+			desc: "should return unchanged permissions if none of actions are part of any action sets",
+			permissions: []accesscontrol.Permission{
+				{
+					Action: "datasources:create",
+				},
+				{
+					Action: "users:read",
+					Scope:  "users:*",
+				},
+			},
+			expectedPermissions: []accesscontrol.Permission{
+				{
+					Action: "datasources:create",
+				},
+				{
+					Action: "users:read",
+					Scope:  "users:*",
+				},
+			},
+		},
+		{
+			desc: "should return unchanged permissions if none of actions are part of any action sets",
+			permissions: []accesscontrol.Permission{
+				{
+					Action: "datasources:create",
+				},
+				{
+					Action: "users:read",
+					Scope:  "users:*",
+				},
+			},
+			expectedPermissions: []accesscontrol.Permission{
+				{
+					Action: "datasources:create",
+				},
+				{
+					Action: "users:read",
+					Scope:  "users:*",
+				},
+			},
+		},
+		{
+			desc: "should be able to expand one permission and leave others unchanged",
+			permissions: []accesscontrol.Permission{
+				{
+					Action: "folders:view",
+					Scope:  "folders:uid:1",
+				},
+				{
+					Action: "users:read",
+					Scope:  "users:*",
+				},
+			},
+			expectedPermissions: []accesscontrol.Permission{
+				{
+					Action: "folders:read",
+					Scope:  "folders:uid:1",
+				},
+				{
+					Action: "dashboards:read",
+					Scope:  "folders:uid:1",
+				},
+				{
+					Action: "users:read",
+					Scope:  "users:*",
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			permissions := actionSetService.ExpandActionSets(tt.permissions)
+			require.ElementsMatch(t, tt.expectedPermissions, permissions)
+		})
+	}
+}

pkg/services/serviceaccounts/extsvcaccounts/service_test.go

@@ -43,7 +43,7 @@ func setupTestEnv(t *testing.T) *TestEnv {
 	}
 	logger := log.New("extsvcaccounts.test")
 	env.S = &ExtSvcAccountsService{
-		acSvc:    acimpl.ProvideOSSService(cfg, env.AcStore, localcache.New(0, 0), fmgt, tracing.InitializeTracerForTest()),
+		acSvc:    acimpl.ProvideOSSService(cfg, env.AcStore, &actest.FakeActionResolver{}, localcache.New(0, 0), fmgt, tracing.InitializeTracerForTest()),
 		features: fmgt,
 		logger:   logger,
 		metrics:  newMetrics(nil, env.SaSvc, logger),