AuthZ: Recover from an outdated cached folder tree (#110293)

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
+			}
 		}
 	}
 

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