From a9f0e157786f188448bc68491b2b31bdb2f5a22b Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Fri, 24 Jan 2025 09:51:39 +0100 Subject: [PATCH] AuthZ: Change cache interface (#99058) * Authz: Switch to remotecache * Todos * lint * lint test * test readibility * Remove ttls * implement a cache wrap * Rm unused func * Comment * Update workspace: * Use cache * Fix comment --- pkg/services/authz/client.go | 3 + pkg/services/authz/rbac/cache.go | 69 +++++++++-- pkg/services/authz/rbac/models.go | 6 +- pkg/services/authz/rbac/service.go | 93 +++++++------- pkg/services/authz/rbac/service_test.go | 153 +++++++++++------------- pkg/services/authz/server.go | 8 +- 6 files changed, 190 insertions(+), 142 deletions(-) diff --git a/pkg/services/authz/client.go b/pkg/services/authz/client.go index 90a1dca4de8..9d7dcc6fee0 100644 --- a/pkg/services/authz/client.go +++ b/pkg/services/authz/client.go @@ -3,6 +3,7 @@ package authz import ( "context" "errors" + "time" "github.com/fullstorydev/grpchan" "github.com/fullstorydev/grpchan/inprocgrpc" @@ -13,6 +14,7 @@ import ( authnlib "github.com/grafana/authlib/authn" authzlib "github.com/grafana/authlib/authz" authzv1 "github.com/grafana/authlib/authz/proto/v1" + "github.com/grafana/authlib/cache" authlib "github.com/grafana/authlib/types" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" @@ -67,6 +69,7 @@ func ProvideAuthZClient( ), log.New("authz-grpc-server"), tracer, + cache.NewLocalCache(cache.Config{Expiry: 5 * time.Minute, CleanupInterval: 10 * time.Minute}), ) return newInProcLegacyClient(server, tracer) } diff --git a/pkg/services/authz/rbac/cache.go b/pkg/services/authz/rbac/cache.go index 49ce54278c0..197b4c9ae23 100644 --- a/pkg/services/authz/rbac/cache.go +++ b/pkg/services/authz/rbac/cache.go @@ -1,29 +1,84 @@ package rbac +import ( + "context" + "encoding/json" + "errors" + "time" + + "github.com/grafana/authlib/cache" + + "github.com/grafana/grafana/pkg/infra/log" +) + func userIdentifierCacheKey(namespace, userUID string) string { - return "UID_" + namespace + "_" + userUID + return namespace + ".uid_" + userUID } func userIdentifierCacheKeyById(namespace, ID string) string { - return "ID_" + namespace + "_" + ID + return namespace + ".id_" + ID } func anonymousPermCacheKey(namespace, action string) string { - return namespace + "_anonymous_" + action + return namespace + ".perm_anonymous_" + action } func userPermCacheKey(namespace, userUID, action string) string { - return namespace + "_" + userUID + "_" + action + return namespace + ".perm_" + userUID + "_" + action } func userBasicRoleCacheKey(namespace, userUID string) string { - return namespace + "_" + userUID + return namespace + ".basic_role_" + userUID } func userTeamCacheKey(namespace, userUID string) string { - return namespace + "_" + userUID + return namespace + ".team_" + userUID } func folderCacheKey(namespace string) string { - return namespace + return namespace + ".folders" +} + +type cacheWrap[T any] struct { + cache cache.Cache + logger log.Logger + ttl time.Duration +} + +// cacheWrap is a wrapper around the authlib Cache that provides typed Get and Set methods +// it handles encoding/decoding for a specific type. +func newCacheWrap[T any](cache cache.Cache, logger log.Logger, ttl time.Duration) *cacheWrap[T] { + return &cacheWrap[T]{cache: cache, logger: logger, ttl: ttl} +} + +func (c *cacheWrap[T]) Get(ctx context.Context, key string) (T, bool) { + var value T + data, err := c.cache.Get(ctx, key) + if err != nil { + if !errors.Is(err, cache.ErrNotFound) { + c.logger.Warn("failed to get from cache", "key", key, "error", err) + } + return value, false + } + + err = json.Unmarshal(data, &value) + if err != nil { + c.logger.Warn("failed to unmarshal from cache", "key", key, "error", err) + return value, false + } + + return value, true +} + +func (c *cacheWrap[T]) Set(ctx context.Context, key string, value T) { + data, err := json.Marshal(value) + if err != nil { + c.logger.Warn("failed to marshal to cache", "key", key, "error", err) + return + } + + err = c.cache.Set(ctx, key, data, c.ttl) + if err != nil { + c.logger.Warn("failed to set to cache", "key", key, "error", err) + } } diff --git a/pkg/services/authz/rbac/models.go b/pkg/services/authz/rbac/models.go index 67f5374d154..a94311b46d5 100644 --- a/pkg/services/authz/rbac/models.go +++ b/pkg/services/authz/rbac/models.go @@ -25,7 +25,7 @@ type ListRequest struct { } type FolderNode struct { - uid string - parentUID *string - childrenUIDs []string + UID string + ParentUID *string + ChildrenUIDs []string } diff --git a/pkg/services/authz/rbac/service.go b/pkg/services/authz/rbac/service.go index 9ccd1427035..a0d2c3b2408 100644 --- a/pkg/services/authz/rbac/service.go +++ b/pkg/services/authz/rbac/service.go @@ -15,9 +15,9 @@ import ( "k8s.io/apiserver/pkg/endpoints/request" authzv1 "github.com/grafana/authlib/authz/proto/v1" + "github.com/grafana/authlib/cache" claims "github.com/grafana/authlib/types" - "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/registry/apis/iam/common" @@ -49,15 +49,15 @@ type Service struct { logger log.Logger tracer tracing.Tracer - // Cache for user permissions, user team memberships and user basic roles - idCache *localcache.CacheService - permCache *localcache.CacheService - teamCache *localcache.CacheService - basicRoleCache *localcache.CacheService - folderCache *localcache.CacheService - // Deduplication of concurrent requests sf *singleflight.Group + + // Cache for user permissions, user team memberships and user basic roles + idCache *cacheWrap[store.UserIdentifiers] + permCache *cacheWrap[map[string]bool] + teamCache *cacheWrap[[]int64] + basicRoleCache *cacheWrap[store.BasicRole] + folderCache *cacheWrap[map[string]FolderNode] } func NewService( @@ -66,7 +66,7 @@ func NewService( permissionStore store.PermissionStore, logger log.Logger, tracer tracing.Tracer, - + cache cache.Cache, ) *Service { return &Service{ store: store.NewStore(sql, tracer), @@ -75,11 +75,11 @@ func NewService( logger: logger, tracer: tracer, mapper: newMapper(), - idCache: localcache.New(longCacheTTL, longCleanupInterval), - permCache: localcache.New(shortCacheTTL, shortCleanupInterval), - teamCache: localcache.New(shortCacheTTL, shortCleanupInterval), - basicRoleCache: localcache.New(longCacheTTL, longCleanupInterval), - folderCache: localcache.New(shortCacheTTL, shortCleanupInterval), + idCache: newCacheWrap[store.UserIdentifiers](cache, logger, longCacheTTL), + permCache: newCacheWrap[map[string]bool](cache, logger, shortCacheTTL), + teamCache: newCacheWrap[[]int64](cache, logger, shortCacheTTL), + basicRoleCache: newCacheWrap[store.BasicRole](cache, logger, longCacheTTL), + folderCache: newCacheWrap[map[string]FolderNode](cache, logger, shortCacheTTL), sf: new(singleflight.Group), } } @@ -285,8 +285,8 @@ func (s *Service) getUserPermissions(ctx context.Context, ns claims.NamespaceInf } userPermKey := userPermCacheKey(ns.Value, userIdentifiers.UID, action) - if cached, ok := s.permCache.Get(userPermKey); ok { - return cached.(map[string]bool), nil + if cached, ok := s.permCache.Get(ctx, userPermKey); ok { + return cached, nil } res, err, _ := s.sf.Do(userPermKey+"_getUserPermissions", func() (interface{}, error) { @@ -315,7 +315,7 @@ func (s *Service) getUserPermissions(ctx context.Context, ns claims.NamespaceInf } scopeMap := getScopeMap(permissions) - s.permCache.Set(userPermKey, scopeMap, 0) + s.permCache.Set(ctx, userPermKey, scopeMap) span.SetAttributes(attribute.Int("num_permissions_fetched", len(permissions))) return scopeMap, nil @@ -333,17 +333,16 @@ func (s *Service) getAnonymousPermissions(ctx context.Context, ns claims.Namespa defer span.End() anonPermKey := anonymousPermCacheKey(ns.Value, action) - if cached, ok := s.permCache.Get(anonPermKey); ok { - return cached.(map[string]bool), nil + if cached, ok := s.permCache.Get(ctx, anonPermKey); ok { + return cached, nil } - res, err, _ := s.sf.Do(anonPermKey+"_getAnonymousPermissions", func() (interface{}, error) { permissions, err := s.permissionStore.GetUserPermissions(ctx, ns, store.PermissionsQuery{Action: action, ActionSets: actionSets, Role: "Viewer"}) if err != nil { return nil, err } scopeMap := getScopeMap(permissions) - s.permCache.Set(anonPermKey, scopeMap, 0) + s.permCache.Set(ctx, anonPermKey, scopeMap) return scopeMap, nil }) @@ -367,13 +366,13 @@ func (s *Service) getRendererPermissions(ctx context.Context, action string) (ma func (s *Service) GetUserIdentifiers(ctx context.Context, ns claims.NamespaceInfo, userUID string) (*store.UserIdentifiers, error) { uidCacheKey := userIdentifierCacheKey(ns.Value, userUID) - if cached, ok := s.idCache.Get(uidCacheKey); ok { - return cached.(*store.UserIdentifiers), nil + if cached, ok := s.idCache.Get(ctx, uidCacheKey); ok { + return &cached, nil } idCacheKey := userIdentifierCacheKeyById(ns.Value, userUID) - if cached, ok := s.idCache.Get(idCacheKey); ok { - return cached.(*store.UserIdentifiers), nil + if cached, ok := s.idCache.Get(ctx, idCacheKey); ok { + return &cached, nil } var userIDQuery store.UserIdentifierQuery @@ -384,12 +383,12 @@ func (s *Service) GetUserIdentifiers(ctx context.Context, ns claims.NamespaceInf userIDQuery = store.UserIdentifierQuery{UserUID: userUID} } userIdentifiers, err := s.store.GetUserIdentifiers(ctx, userIDQuery) - if err != nil { + if err != nil || userIdentifiers == nil { return nil, fmt.Errorf("could not get user internal id: %w", err) } - s.idCache.Set(uidCacheKey, userIdentifiers, 0) - s.idCache.Set(idCacheKey, userIdentifiers, 0) + s.idCache.Set(ctx, uidCacheKey, *userIdentifiers) + s.idCache.Set(ctx, idCacheKey, *userIdentifiers) return userIdentifiers, nil } @@ -400,8 +399,8 @@ func (s *Service) getUserTeams(ctx context.Context, ns claims.NamespaceInfo, use teamIDs := make([]int64, 0, 50) teamsCacheKey := userTeamCacheKey(ns.Value, userIdentifiers.UID) - if cached, ok := s.teamCache.Get(teamsCacheKey); ok { - return cached.([]int64), nil + if cached, ok := s.teamCache.Get(ctx, teamsCacheKey); ok { + return cached, nil } teamQuery := legacy.ListUserTeamsQuery{ @@ -422,7 +421,7 @@ func (s *Service) getUserTeams(ctx context.Context, ns claims.NamespaceInfo, use break } } - s.teamCache.Set(teamsCacheKey, teamIDs, 0) + s.teamCache.Set(ctx, teamsCacheKey, teamIDs) span.SetAttributes(attribute.Int("num_user_teams", len(teamIDs))) return teamIDs, nil @@ -433,8 +432,8 @@ func (s *Service) getUserBasicRole(ctx context.Context, ns claims.NamespaceInfo, defer span.End() basicRoleKey := userBasicRoleCacheKey(ns.Value, userIdentifiers.UID) - if cached, ok := s.basicRoleCache.Get(basicRoleKey); ok { - return cached.(store.BasicRole), nil + if cached, ok := s.basicRoleCache.Get(ctx, basicRoleKey); ok { + return cached, nil } basicRole, err := s.store.GetBasicRoles(ctx, ns, store.BasicRoleQuery{UserID: userIdentifiers.ID}) @@ -444,7 +443,7 @@ func (s *Service) getUserBasicRole(ctx context.Context, ns claims.NamespaceInfo, if basicRole == nil { basicRole = &store.BasicRole{} } - s.basicRoleCache.Set(basicRoleKey, *basicRole, 0) + s.basicRoleCache.Set(ctx, basicRoleKey, *basicRole) return *basicRole, nil } @@ -512,14 +511,14 @@ func (s *Service) checkInheritedPermissions(ctx context.Context, scopeMap map[st currentUID := req.ParentFolder for { if node, has := folderMap[currentUID]; has { - scope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(node.uid) + scope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(node.UID) if scopeMap[scope] { return true, nil } - if node.parentUID == nil { + if node.ParentUID == nil { break } - currentUID = *node.parentUID + currentUID = *node.ParentUID } else { break } @@ -532,8 +531,8 @@ func (s *Service) buildFolderTree(ctx context.Context, ns claims.NamespaceInfo) defer span.End() key := folderCacheKey(ns.Value) - if cached, ok := s.folderCache.Get(key); ok { - return cached.(map[string]FolderNode), nil + if cached, ok := s.folderCache.Get(ctx, key); ok { + return cached, nil } res, err, _ := s.sf.Do(ns.Value+"_buildFolderTree", func() (interface{}, error) { @@ -547,11 +546,11 @@ func (s *Service) buildFolderTree(ctx context.Context, ns claims.NamespaceInfo) for _, folder := range folders { if node, has := folderMap[folder.UID]; !has { folderMap[folder.UID] = FolderNode{ - uid: folder.UID, - parentUID: folder.ParentUID, + UID: folder.UID, + ParentUID: folder.ParentUID, } } else { - node.parentUID = folder.ParentUID + node.ParentUID = folder.ParentUID folderMap[folder.UID] = node } // Register that the parent has this child node @@ -559,17 +558,17 @@ func (s *Service) buildFolderTree(ctx context.Context, ns claims.NamespaceInfo) continue } if parent, has := folderMap[*folder.ParentUID]; has { - parent.childrenUIDs = append(parent.childrenUIDs, folder.UID) + parent.ChildrenUIDs = append(parent.ChildrenUIDs, folder.UID) folderMap[*folder.ParentUID] = parent } else { folderMap[*folder.ParentUID] = FolderNode{ - uid: *folder.ParentUID, - childrenUIDs: []string{folder.UID}, + UID: *folder.ParentUID, + ChildrenUIDs: []string{folder.UID}, } } } - s.folderCache.Set(key, folderMap, 0) + s.folderCache.Set(ctx, key, folderMap) return folderMap, nil }) @@ -642,7 +641,7 @@ func getChildren(folderMap map[string]FolderNode, folderUID string, folderSet ma if !has { return } - for _, child := range folder.childrenUIDs { + for _, child := range folder.ChildrenUIDs { // We have already processed all the children of this folder if _, ok := folderSet[child]; ok { return diff --git a/pkg/services/authz/rbac/service_test.go b/pkg/services/authz/rbac/service_test.go index f4f301008e3..26b3cd02451 100644 --- a/pkg/services/authz/rbac/service_test.go +++ b/pkg/services/authz/rbac/service_test.go @@ -4,14 +4,15 @@ import ( "context" "fmt" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/sync/singleflight" + "github.com/grafana/authlib/cache" claims "github.com/grafana/authlib/types" - "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/registry/apis/iam/legacy" @@ -149,11 +150,7 @@ func TestService_checkPermission(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - s := &Service{ - logger: log.NewNopLogger(), - tracer: tracing.NewNoopTracerService(), - mapper: newMapper(), - } + s := setupService() got, err := s.checkPermission(context.Background(), getScopeMap(tc.permissions), &tc.check) require.NoError(t, err) assert.Equal(t, tc.expected, got) @@ -197,21 +194,16 @@ func TestService_getUserTeams(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { ctx := context.Background() + s := setupService() + ns := claims.NamespaceInfo{Value: "stacks-12", OrgID: 1, StackID: 12} userIdentifiers := &store.UserIdentifiers{UID: "test-uid"} identityStore := &fakeIdentityStore{teams: tc.teams, err: tc.expectedError} + s.identityStore = identityStore - cacheService := localcache.New(shortCacheTTL, shortCleanupInterval) if tc.cacheHit { - cacheService.Set(userTeamCacheKey(ns.Value, userIdentifiers.UID), tc.expectedTeams, 0) - } - - s := &Service{ - teamCache: cacheService, - identityStore: identityStore, - logger: log.New("test"), - tracer: tracing.NewNoopTracerService(), + s.teamCache.Set(ctx, userTeamCacheKey(ns.Value, userIdentifiers.UID), tc.expectedTeams) } teams, err := s.getUserTeams(ctx, ns, userIdentifiers) @@ -279,22 +271,16 @@ func TestService_getUserBasicRole(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { ctx := context.Background() + s := setupService() ns := claims.NamespaceInfo{Value: "stacks-12", OrgID: 1, StackID: 12} userIdentifiers := &store.UserIdentifiers{UID: "test-uid", ID: 1} store := &fakeStore{basicRole: &tc.basicRole, err: tc.expectedError} + s.store = store + s.permissionStore = store - cacheService := localcache.New(shortCacheTTL, shortCleanupInterval) if tc.cacheHit { - cacheService.Set(userBasicRoleCacheKey(ns.Value, userIdentifiers.UID), tc.expectedRole, 0) - } - - s := &Service{ - basicRoleCache: cacheService, - store: store, - permissionStore: store, - logger: log.New("test"), - tracer: tracing.NewNoopTracerService(), + s.basicRoleCache.Set(ctx, userBasicRoleCacheKey(ns.Value, userIdentifiers.UID), tc.expectedRole) } role, err := s.getUserBasicRole(ctx, ns, userIdentifiers) @@ -350,13 +336,14 @@ func TestService_getUserPermissions(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { ctx := context.Background() + s := setupService() + userID := &store.UserIdentifiers{UID: "test-uid", ID: 112} ns := claims.NamespaceInfo{Value: "stacks-12", OrgID: 1, StackID: 12} action := "dashboards:read" - cacheService := localcache.New(shortCacheTTL, shortCleanupInterval) if tc.cacheHit { - cacheService.Set(userPermCacheKey(ns.Value, userID.UID, action), tc.expectedPerms, 0) + s.permCache.Set(ctx, userPermCacheKey(ns.Value, userID.UID, action), tc.expectedPerms) } store := &fakeStore{ @@ -364,20 +351,9 @@ func TestService_getUserPermissions(t *testing.T) { basicRole: &store.BasicRole{Role: "viewer", IsAdmin: false}, userPermissions: tc.permissions, } - - s := &Service{ - store: store, - permissionStore: store, - identityStore: &fakeIdentityStore{teams: []int64{1, 2}}, - logger: log.NewNopLogger(), - tracer: tracing.NewNoopTracerService(), - mapper: newMapper(), - idCache: localcache.New(longCacheTTL, longCleanupInterval), - permCache: cacheService, - sf: new(singleflight.Group), - basicRoleCache: localcache.New(longCacheTTL, longCleanupInterval), - teamCache: localcache.New(shortCacheTTL, shortCleanupInterval), - } + s.store = store + s.permissionStore = store + s.identityStore = &fakeIdentityStore{teams: []int64{1, 2}} perms, err := s.getIdentityPermissions(ctx, ns, claims.TypeUser, userID.UID, action) require.NoError(t, err) @@ -412,8 +388,8 @@ func TestService_buildFolderTree(t *testing.T) { }, cacheHit: true, expectedTree: map[string]FolderNode{ - "folder1": {uid: "folder1", childrenUIDs: []string{"folder2"}}, - "folder2": {uid: "folder2", parentUID: strPtr("folder1")}, + "folder1": {UID: "folder1", ChildrenUIDs: []string{"folder2"}}, + "folder2": {UID: "folder2", ParentUID: strPtr("folder1")}, }, }, { @@ -424,8 +400,8 @@ func TestService_buildFolderTree(t *testing.T) { }, cacheHit: false, expectedTree: map[string]FolderNode{ - "folder1": {uid: "folder1", childrenUIDs: []string{"folder2"}}, - "folder2": {uid: "folder2", parentUID: strPtr("folder1")}, + "folder1": {UID: "folder1", ChildrenUIDs: []string{"folder2"}}, + "folder2": {UID: "folder2", ParentUID: strPtr("folder1")}, }, }, } @@ -433,23 +409,17 @@ func TestService_buildFolderTree(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { ctx := context.Background() + s := setupService() + ns := claims.NamespaceInfo{Value: "stacks-12", OrgID: 1, StackID: 12} - cacheService := localcache.New(shortCacheTTL, shortCleanupInterval) if tc.cacheHit { - cacheService.Set(folderCacheKey(ns.Value), tc.expectedTree, 0) + s.folderCache.Set(ctx, folderCacheKey(ns.Value), tc.expectedTree) } store := &fakeStore{folders: tc.folders} - - s := &Service{ - store: store, - permissionStore: store, - folderCache: cacheService, - logger: log.New("test"), - sf: new(singleflight.Group), - tracer: tracing.NewNoopTracerService(), - } + s.store = store + s.permissionStore = store tree, err := s.buildFolderTree(ctx, ns) @@ -460,15 +430,15 @@ func TestService_buildFolderTree(t *testing.T) { require.True(t, ok) // Check parent if folder.ParentUID != nil { - require.NotNil(t, node.parentUID) - require.Equal(t, *folder.ParentUID, *node.parentUID) + require.NotNil(t, node.ParentUID) + require.Equal(t, *folder.ParentUID, *node.ParentUID) } else { - require.Nil(t, node.parentUID) + require.Nil(t, node.ParentUID) } // Check children - if len(node.childrenUIDs) > 0 { - epectedChildren := tc.expectedTree[folder.UID].childrenUIDs - require.ElementsMatch(t, node.childrenUIDs, epectedChildren) + if len(node.ChildrenUIDs) > 0 { + epectedChildren := tc.expectedTree[folder.UID].ChildrenUIDs + require.ElementsMatch(t, node.ChildrenUIDs, epectedChildren) } } if tc.cacheHit { @@ -534,8 +504,8 @@ func TestService_listPermission(t *testing.T) { }, }, folderTree: map[string]FolderNode{ - "some_folder_1": {uid: "some_folder_1"}, - "some_folder_2": {uid: "some_folder_2"}, + "some_folder_1": {UID: "some_folder_1"}, + "some_folder_2": {UID: "some_folder_2"}, }, list: ListRequest{ Action: "dashboards:read", @@ -557,12 +527,12 @@ func TestService_listPermission(t *testing.T) { }, }, folderTree: map[string]FolderNode{ - "some_folder_parent": {uid: "some_folder_parent", childrenUIDs: []string{"some_folder_child"}}, - "some_folder_child": {uid: "some_folder_child", parentUID: strPtr("some_folder_parent"), childrenUIDs: []string{"some_folder_subchild1", "some_folder_subchild2"}}, - "some_folder_subchild1": {uid: "some_folder_subchild1", parentUID: strPtr("some_folder_child")}, - "some_folder_subchild2": {uid: "some_folder_subchild2", parentUID: strPtr("some_folder_child"), childrenUIDs: []string{"some_folder_subsubchild"}}, - "some_folder_subsubchild": {uid: "some_folder_subsubchild", parentUID: strPtr("some_folder_subchild2")}, - "some_folder_1": {uid: "some_folder_1", parentUID: strPtr("some_other_folder")}, + "some_folder_parent": {UID: "some_folder_parent", ChildrenUIDs: []string{"some_folder_child"}}, + "some_folder_child": {UID: "some_folder_child", ParentUID: strPtr("some_folder_parent"), ChildrenUIDs: []string{"some_folder_subchild1", "some_folder_subchild2"}}, + "some_folder_subchild1": {UID: "some_folder_subchild1", ParentUID: strPtr("some_folder_child")}, + "some_folder_subchild2": {UID: "some_folder_subchild2", ParentUID: strPtr("some_folder_child"), ChildrenUIDs: []string{"some_folder_subsubchild"}}, + "some_folder_subsubchild": {UID: "some_folder_subsubchild", ParentUID: strPtr("some_folder_subchild2")}, + "some_folder_1": {UID: "some_folder_1", ParentUID: strPtr("some_other_folder")}, }, list: ListRequest{ Action: "dashboards:read", @@ -590,8 +560,8 @@ func TestService_listPermission(t *testing.T) { }, }, folderTree: map[string]FolderNode{ - "some_folder_parent": {uid: "some_folder_parent", childrenUIDs: []string{"some_folder_child"}}, - "some_folder_child": {uid: "some_folder_child", parentUID: strPtr("some_folder_parent")}, + "some_folder_parent": {UID: "some_folder_parent", ChildrenUIDs: []string{"some_folder_child"}}, + "some_folder_child": {UID: "some_folder_child", ParentUID: strPtr("some_folder_parent")}, }, list: ListRequest{ Action: "dashboards:read", @@ -620,9 +590,9 @@ func TestService_listPermission(t *testing.T) { }, }, folderTree: map[string]FolderNode{ - "some_folder_parent": {uid: "some_folder_parent", childrenUIDs: []string{"some_folder_child"}}, - "some_folder_child": {uid: "some_folder_child", parentUID: strPtr("some_folder_parent"), childrenUIDs: []string{"some_folder_subchild"}}, - "some_folder_subchild": {uid: "some_folder_subchild", parentUID: strPtr("some_folder_child")}, + "some_folder_parent": {UID: "some_folder_parent", ChildrenUIDs: []string{"some_folder_child"}}, + "some_folder_child": {UID: "some_folder_child", ParentUID: strPtr("some_folder_parent"), ChildrenUIDs: []string{"some_folder_subchild"}}, + "some_folder_subchild": {UID: "some_folder_subchild", ParentUID: strPtr("some_folder_child")}, }, list: ListRequest{ Action: "dashboards:read", @@ -635,7 +605,7 @@ func TestService_listPermission(t *testing.T) { name: "return no dashboards and folders if the user doesn't have access to any resources", permissions: []accesscontrol.Permission{}, folderTree: map[string]FolderNode{ - "some_folder_1": {uid: "some_folder_1"}, + "some_folder_1": {UID: "some_folder_1"}, }, list: ListRequest{ Action: "dashboards:read", @@ -647,16 +617,11 @@ func TestService_listPermission(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - folderCache := localcache.New(shortCacheTTL, shortCleanupInterval) + s := setupService() if tc.folderTree != nil { - folderCache.Set(folderCacheKey("default"), tc.folderTree, 0) - } - s := &Service{ - logger: log.New("test"), - folderCache: folderCache, - mapper: newMapper(), - tracer: tracing.NewNoopTracerService(), + s.folderCache.Set(context.Background(), folderCacheKey("default"), tc.folderTree) } + tc.list.Namespace = claims.NamespaceInfo{Value: "default", OrgID: 1} got, err := s.listPermission(context.Background(), getScopeMap(tc.permissions), &tc.list) require.NoError(t, err) @@ -667,6 +632,26 @@ func TestService_listPermission(t *testing.T) { } } +func setupService() *Service { + cache := cache.NewLocalCache(cache.Config{Expiry: 5 * time.Minute, CleanupInterval: 5 * time.Minute}) + logger := log.New("authz-rbac-service") + fStore := &fakeStore{} + return &Service{ + logger: logger, + mapper: newMapper(), + tracer: tracing.NewNoopTracerService(), + idCache: newCacheWrap[store.UserIdentifiers](cache, logger, longCacheTTL), + permCache: newCacheWrap[map[string]bool](cache, logger, shortCacheTTL), + teamCache: newCacheWrap[[]int64](cache, logger, shortCacheTTL), + basicRoleCache: newCacheWrap[store.BasicRole](cache, logger, longCacheTTL), + folderCache: newCacheWrap[map[string]FolderNode](cache, logger, shortCacheTTL), + store: fStore, + permissionStore: fStore, + identityStore: &fakeIdentityStore{}, + sf: new(singleflight.Group), + } +} + func strPtr(s string) *string { return &s } diff --git a/pkg/services/authz/server.go b/pkg/services/authz/server.go index 49f583386fc..84a6b2dd9c3 100644 --- a/pkg/services/authz/server.go +++ b/pkg/services/authz/server.go @@ -2,6 +2,7 @@ package authz import ( authzv1 "github.com/grafana/authlib/authz/proto/v1" + cache "github.com/grafana/authlib/cache" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" @@ -13,13 +14,18 @@ import ( "github.com/grafana/grafana/pkg/storage/legacysql" ) -func RegisterRBACAuthZService(handler grpcserver.Provider, db legacysql.LegacyDatabaseProvider, tracer tracing.Tracer) { +func RegisterRBACAuthZService( + handler grpcserver.Provider, + db legacysql.LegacyDatabaseProvider, + tracer tracing.Tracer, + cache cache.Cache) { server := rbac.NewService( db, legacy.NewLegacySQLStores(db), store.NewSQLPermissionStore(db, tracer), log.New("authz-grpc-server"), tracer, + cache, ) srv := handler.GetServer()