From 885812f694d2d89a143771ff13604d775fd00fcb Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Mon, 1 Sep 2025 11:16:01 +0200 Subject: [PATCH] AuthZ: Recover from an outdated cached folder tree (#110293) --- pkg/services/authz/rbac/service.go | 58 +++++++++++++++++-------- pkg/services/authz/rbac/service_test.go | 39 ++++++++++++++++- 2 files changed, 78 insertions(+), 19 deletions(-) diff --git a/pkg/services/authz/rbac/service.go b/pkg/services/authz/rbac/service.go index 36ee02e2790..9933775c79b 100644 --- a/pkg/services/authz/rbac/service.go +++ b/pkg/services/authz/rbac/service.go @@ -30,10 +30,8 @@ import ( ) const ( - shortCacheTTL = 30 * time.Second - shortCleanupInterval = 2 * time.Minute - longCacheTTL = 2 * time.Minute - longCleanupInterval = 4 * time.Minute + shortCacheTTL = 30 * time.Second + longCacheTTL = 2 * time.Minute ) type Service struct { @@ -630,10 +628,16 @@ func (s *Service) checkInheritedPermissions(ctx context.Context, scopeMap map[st defer span.End() ctxLogger := s.logger.FromContext(ctx) - tree, err := s.buildFolderTree(ctx, req.Namespace) - if err != nil { - ctxLogger.Error("could not build folder and dashboard tree", "error", err) - return false, err + tree, ok := s.getCachedFolderTree(ctx, req.Namespace) + + // Check cached tree is up to date + if !ok || !s.isFolderInTree(tree, req.ParentFolder) { + var err error + tree, err = s.buildFolderTree(ctx, req.Namespace) + if err != nil { + ctxLogger.Error("could not build folder and dashboard tree", "error", err) + return false, err + } } if scopeMap["folders:uid:"+req.ParentFolder] { @@ -649,15 +653,29 @@ func (s *Service) checkInheritedPermissions(ctx context.Context, scopeMap map[st return false, nil } +// getCachedFolderTree returns the cached folder tree for the given namespace. +func (s *Service) getCachedFolderTree(ctx context.Context, ns types.NamespaceInfo) (folderTree, bool) { + ctx, span := s.tracer.Start(ctx, "authz_direct_db.service.getCachedFolderTree") + defer span.End() + key := folderCacheKey(ns.Value) + return s.folderCache.Get(ctx, key) +} + +// isFolderInTree checks if the given parent folder exists in the folder tree. +func (s *Service) isFolderInTree(tree folderTree, folder string) bool { + // Special case for general folder, which is technically not in the tree + if folder == accesscontrol.GeneralFolderUID { + return true + } + _, exists := tree.Index[folder] + return exists +} + +// buildFolderTree builds the folder tree for the given namespace and caches it. func (s *Service) buildFolderTree(ctx context.Context, ns types.NamespaceInfo) (folderTree, error) { ctx, span := s.tracer.Start(ctx, "authz_direct_db.service.buildFolderTree") defer span.End() - key := folderCacheKey(ns.Value) - if cached, ok := s.folderCache.Get(ctx, key); ok { - return cached, nil - } - res, err, _ := s.sf.Do(ns.Value+"_buildFolderTree", func() (interface{}, error) { folders, err := s.folderStore.ListFolders(ctx, ns) if err != nil { @@ -666,7 +684,8 @@ func (s *Service) buildFolderTree(ctx context.Context, ns types.NamespaceInfo) ( span.SetAttributes(attribute.Int("num_folders", len(folders))) tree := newFolderTree(folders) - s.folderCache.Set(ctx, key, tree) + + s.folderCache.Set(ctx, folderCacheKey(ns.Value), tree) return tree, nil }) @@ -695,10 +714,13 @@ func (s *Service) listPermission(ctx context.Context, scopeMap map[string]bool, var tree folderTree if t.HasFolderSupport() { var err error - tree, err = s.buildFolderTree(ctx, req.Namespace) - if err != nil { - ctxLogger.Error("could not build folder and dashboard tree", "error", err) - return nil, err + tree, ok = s.getCachedFolderTree(ctx, req.Namespace) + if !ok { + tree, err = s.buildFolderTree(ctx, req.Namespace) + if err != nil { + ctxLogger.Error("could not build folder and dashboard tree", "error", err) + return nil, err + } } } diff --git a/pkg/services/authz/rbac/service_test.go b/pkg/services/authz/rbac/service_test.go index 6ae33e9636f..cee36a63d59 100644 --- a/pkg/services/authz/rbac/service_test.go +++ b/pkg/services/authz/rbac/service_test.go @@ -229,7 +229,7 @@ func TestService_checkPermission(t *testing.T) { Identifier: "parent", }, }, - folders: []store.Folder{{UID: "parent"}}, + folders: []store.Folder{{UID: "parent"}, {UID: "other_parent"}}, check: CheckRequest{ Action: "dashboards:create", Group: "dashboard.grafana.app", @@ -297,6 +297,43 @@ func TestService_checkPermission(t *testing.T) { } } +func TestService_checkPermission_folderCacheMissRecovery(t *testing.T) { + s := setupService() + ctx := context.Background() + + // User has root folder access + userPermissions := map[string]bool{ + "folders:uid:root": true, + } + + // Populate store with folders + folderStore := &fakeStore{ + folders: []store.Folder{{UID: "root"}, {UID: "sub", ParentUID: strPtr("root")}}, + disableNsCheck: true, + } + s.folderStore = folderStore + + // Sub folder is missing from the cache + s.folderCache.Set(ctx, folderCacheKey("default"), newFolderTree([]store.Folder{{UID: "root"}})) + + // Perform check on sub folder + check := CheckRequest{ + Action: "dashboards:read", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Name: "dash1", + ParentFolder: "sub", + Namespace: types.NamespaceInfo{Value: "default", OrgID: 1}, + } + + got, err := s.checkPermission(ctx, userPermissions, &check) + require.NoError(t, err) + assert.True(t, got) + + // Check that folder store was queried despite the initial cache hit + assert.Equal(t, 1, folderStore.calls) +} + func TestService_getUserTeams(t *testing.T) { type testCase struct { name string