RBAC: Add cache for oss rbac permissions (#55098)

* RBAC: Add cache for oss permissions

* RBAC: include service account actions

* RBAC: revert changes to fetch service account permissions

* Update comment for setting

* RBAC: Disable permission chache for tests

conf/defaults.ini

@@ -666,7 +666,7 @@ managed_identity_client_id =
 
 #################################### Role-based Access Control ###########
 [rbac]
-# If enabled, cache permissions in a in memory cache (Enterprise only)
+# If enabled, cache permissions in a in memory cache
 permission_cache = true
 
 #################################### SMTP / Emailing #####################

pkg/api/common_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/grafana/grafana/pkg/api/response"
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/infra/fs"
+	"github.com/grafana/grafana/pkg/infra/localcache"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/infra/remotecache"
 	"github.com/grafana/grafana/pkg/infra/tracing"
@@ -379,7 +380,7 @@ func setupHTTPServerWithCfgDb(
 		acService = acmock
 	} else {
 		var err error
-		acService, err = acimpl.ProvideService(cfg, database.ProvideService(db), routeRegister)
+		acService, err = acimpl.ProvideService(cfg, database.ProvideService(db), routeRegister, localcache.ProvideService())
 		require.NoError(t, err)
 		ac = acimpl.ProvideAccessControl(cfg)
 	}

pkg/services/accesscontrol/acimpl/service.go

@@ -2,8 +2,11 @@ package acimpl
 
 import (
 	"context"
+	"fmt"
+	"time"
 
 	"github.com/grafana/grafana/pkg/api/routing"
+	"github.com/grafana/grafana/pkg/infra/localcache"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/infra/metrics"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
@@ -14,8 +17,12 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 )
 
-func ProvideService(cfg *setting.Cfg, store accesscontrol.Store, routeRegister routing.RouteRegister) (*Service, error) {
+const (
-	service := ProvideOSSService(cfg, store)
+	cacheTTL = 10 * time.Second
+)
+
+func ProvideService(cfg *setting.Cfg, store accesscontrol.Store, routeRegister routing.RouteRegister, cache *localcache.CacheService) (*Service, error) {
+	service := ProvideOSSService(cfg, store, cache)
 
 	if !accesscontrol.IsDisabled(cfg) {
 		api.NewAccessControlAPI(routeRegister, service).RegisterAPIEndpoints()
@@ -27,11 +34,12 @@ func ProvideService(cfg *setting.Cfg, store accesscontrol.Store, routeRegister r
 	return service, nil
 }
 
-func ProvideOSSService(cfg *setting.Cfg, store accesscontrol.Store) *Service {
+func ProvideOSSService(cfg *setting.Cfg, store accesscontrol.Store, cache *localcache.CacheService) *Service {
 	s := &Service{
 		cfg:   cfg,
 		store: store,
 		log:   log.New("accesscontrol.service"),
+		cache: cache,
 		roles: accesscontrol.BuildBasicRoleDefinitions(),
 	}
 
@@ -43,6 +51,7 @@ type Service struct {
 	log           log.Logger
 	cfg           *setting.Cfg
 	store         accesscontrol.Store
+	cache         *localcache.CacheService
 	registrations accesscontrol.RegistrationList
 	roles         map[string]*accesscontrol.RoleDTO
 }
@@ -63,12 +72,19 @@ var actionsToFetch = append(
 )
 
 // GetUserPermissions returns user permissions based on built-in roles
-func (s *Service) GetUserPermissions(ctx context.Context, user *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
+func (s *Service) GetUserPermissions(ctx context.Context, user *user.SignedInUser, options accesscontrol.Options) ([]accesscontrol.Permission, error) {
 	timer := prometheus.NewTimer(metrics.MAccessPermissionsSummary)
 	defer timer.ObserveDuration()
 
-	permissions := make([]accesscontrol.Permission, 0)
+	if !s.cfg.RBACPermissionCache || !user.HasUniqueId() {
+		return s.getUserPermissions(ctx, user, options)
+	}
 
+	return s.getCachedUserPermissions(ctx, user, options)
+}
+
+func (s *Service) getUserPermissions(ctx context.Context, user *user.SignedInUser, options accesscontrol.Options) ([]accesscontrol.Permission, error) {
+	permissions := make([]accesscontrol.Permission, 0)
 	for _, builtin := range accesscontrol.GetOrgRoles(user) {
 		if basicRole, ok := s.roles[builtin]; ok {
 			permissions = append(permissions, basicRole.Permissions...)
@@ -89,6 +105,32 @@ func (s *Service) GetUserPermissions(ctx context.Context, user *user.SignedInUse
 	return append(permissions, dbPermissions...), nil
 }
 
+func (s *Service) getCachedUserPermissions(ctx context.Context, user *user.SignedInUser, options accesscontrol.Options) ([]accesscontrol.Permission, error) {
+	key, err := permissionCacheKey(user)
+	if err != nil {
+		return nil, err
+	}
+
+	if !options.ReloadCache {
+		permissions, ok := s.cache.Get(key)
+		if ok {
+			s.log.Debug("using cached permissions", "key", key)
+			return permissions.([]accesscontrol.Permission), nil
+		}
+	}
+
+	s.log.Debug("fetch permissions from store", "key", key)
+	permissions, err := s.getUserPermissions(ctx, user, options)
+	if err != nil {
+		return nil, err
+	}
+
+	s.log.Debug("cache permissions", "key", key)
+	s.cache.Set(key, permissions, cacheTTL)
+
+	return permissions, nil
+}
+
 func (s *Service) DeleteUserPermissions(ctx context.Context, orgID int64, userID int64) error {
 	return s.store.DeleteUserPermissions(ctx, orgID, userID)
 }
@@ -140,3 +182,11 @@ func (s *Service) RegisterFixedRoles(ctx context.Context) error {
 func (s *Service) IsDisabled() bool {
 	return accesscontrol.IsDisabled(s.cfg)
 }
+
+func permissionCacheKey(user *user.SignedInUser) (string, error) {
+	key, err := user.GetCacheKey()
+	if err != nil {
+		return "", err
+	}
+	return fmt.Sprintf("rbac-permissions-%s", key), nil
+}

pkg/services/accesscontrol/acimpl/service_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/api/routing"
+	"github.com/grafana/grafana/pkg/infra/localcache"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
@@ -59,6 +60,7 @@ func TestUsageMetrics(t *testing.T) {
 				cfg,
 				database.ProvideService(sqlstore.InitTestDB(t)),
 				routing.NewRouteRegister(),
+				localcache.ProvideService(),
 			)
 			require.NoError(t, errInitAc)
 			assert.Equal(t, tt.expectedValue, s.GetUsageStats(context.Background())["stats.oss.accesscontrol.enabled.count"])

pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go

@@ -258,6 +258,20 @@ func (e DatasourcePermissionsService) MapActions(permission accesscontrol.Resour
 	return ""
 }
 
+var (
+	ServiceAccountEditActions = []string{
+		serviceaccounts.ActionRead,
+		serviceaccounts.ActionWrite,
+	}
+	ServiceAccountAdminActions = []string{
+		serviceaccounts.ActionRead,
+		serviceaccounts.ActionWrite,
+		serviceaccounts.ActionDelete,
+		serviceaccounts.ActionPermissionsRead,
+		serviceaccounts.ActionPermissionsWrite,
+	}
+)
+
 type ServiceAccountPermissionsService struct {
 	*resourcepermissions.Service
 }
@@ -283,8 +297,8 @@ func ProvideServiceAccountPermissions(
 			BuiltInRoles: false,
 		},
 		PermissionsToActions: map[string][]string{
-			"Edit":  {serviceaccounts.ActionRead, serviceaccounts.ActionWrite},
+			"Edit":  ServiceAccountEditActions,
-			"Admin": {serviceaccounts.ActionRead, serviceaccounts.ActionWrite, serviceaccounts.ActionDelete, serviceaccounts.ActionPermissionsRead, serviceaccounts.ActionPermissionsWrite},
+			"Admin": ServiceAccountAdminActions,
 		},
 		ReaderRoleName: "Service account permission reader",
 		WriterRoleName: "Service account permission writer",

pkg/tests/testinfra/testinfra.go

@@ -202,6 +202,11 @@ func CreateGrafDir(t *testing.T, opts ...GrafanaOpts) (string, string) {
 	_, err = alertingSect.NewKey("max_attempts", "3")
 	require.NoError(t, err)
 
+	rbacSect, err := cfg.NewSection("rbac")
+	require.NoError(t, err)
+	_, err = rbacSect.NewKey("permission_cache", "false")
+	require.NoError(t, err)
+
 	getOrCreateSection := func(name string) (*ini.Section, error) {
 		section, err := cfg.GetSection(name)
 		if err != nil {