Authz: Refactor folder tree (#99554)

* Refactor folder tree to its own structure

* Make it possible to json encode the tree

* Use iterations for Ancestors and Children

---------

Co-authored-by: IevaVasiljeva <ieva.vasiljeva@grafana.com>
This commit is contained in:
Karl Persson 2025-02-11 12:36:11 +01:00 committed by GitHub
parent a1dacc24b9
commit bfa4fa3c68
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 299 additions and 183 deletions

View File

@ -23,9 +23,3 @@ type ListRequest struct {
Verb string
Action string
}
type FolderNode struct {
UID string
ParentUID *string
ChildrenUIDs []string
}

View File

@ -26,7 +26,6 @@ import (
"github.com/grafana/grafana/pkg/services/accesscontrol"
authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1"
"github.com/grafana/grafana/pkg/services/authz/rbac/store"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/storage/legacysql"
)
@ -59,7 +58,7 @@ type Service struct {
permCache *cacheWrap[map[string]bool]
teamCache *cacheWrap[[]int64]
basicRoleCache *cacheWrap[store.BasicRole]
folderCache *cacheWrap[map[string]FolderNode]
folderCache *cacheWrap[folderTree]
}
func NewService(
@ -83,7 +82,7 @@ func NewService(
permCache: newCacheWrap[map[string]bool](cache, logger, shortCacheTTL),
teamCache: newCacheWrap[[]int64](cache, logger, shortCacheTTL),
basicRoleCache: newCacheWrap[store.BasicRole](cache, logger, shortCacheTTL),
folderCache: newCacheWrap[map[string]FolderNode](cache, logger, shortCacheTTL),
folderCache: newCacheWrap[folderTree](cache, logger, shortCacheTTL),
sf: new(singleflight.Group),
}
}
@ -517,31 +516,26 @@ func (s *Service) checkInheritedPermissions(ctx context.Context, scopeMap map[st
defer span.End()
ctxLogger := s.logger.FromContext(ctx)
folderMap, err := s.buildFolderTree(ctx, req.Namespace)
tree, err := s.buildFolderTree(ctx, req.Namespace)
if err != nil {
ctxLogger.Error("could not build folder and dashboard tree", "error", err)
return false, err
}
currentUID := req.ParentFolder
for {
if node, has := folderMap[currentUID]; has {
scope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(node.UID)
if scopeMap[scope] {
return true, nil
}
if node.ParentUID == nil {
break
}
currentUID = *node.ParentUID
} else {
break
if scopeMap["folders:uid:"+req.ParentFolder] {
return true, nil
}
for n := range tree.Ancestors(req.ParentFolder) {
if scopeMap["folders:uid:"+n.UID] {
return true, nil
}
}
return false, nil
}
func (s *Service) buildFolderTree(ctx context.Context, ns claims.NamespaceInfo) (map[string]FolderNode, error) {
func (s *Service) buildFolderTree(ctx context.Context, ns claims.NamespaceInfo) (folderTree, error) {
ctx, span := s.tracer.Start(ctx, "authz_direct_db.service.buildFolderTree")
defer span.End()
@ -557,41 +551,16 @@ func (s *Service) buildFolderTree(ctx context.Context, ns claims.NamespaceInfo)
}
span.SetAttributes(attribute.Int("num_folders", len(folders)))
folderMap := make(map[string]FolderNode, len(folders))
for _, folder := range folders {
if node, has := folderMap[folder.UID]; !has {
folderMap[folder.UID] = FolderNode{
UID: folder.UID,
ParentUID: folder.ParentUID,
}
} else {
node.ParentUID = folder.ParentUID
folderMap[folder.UID] = node
}
// Register that the parent has this child node
if folder.ParentUID == nil {
continue
}
if parent, has := folderMap[*folder.ParentUID]; has {
parent.ChildrenUIDs = append(parent.ChildrenUIDs, folder.UID)
folderMap[*folder.ParentUID] = parent
} else {
folderMap[*folder.ParentUID] = FolderNode{
UID: *folder.ParentUID,
ChildrenUIDs: []string{folder.UID},
}
}
}
s.folderCache.Set(ctx, key, folderMap)
return folderMap, nil
tree := newFolderTree(folders)
s.folderCache.Set(ctx, key, tree)
return tree, nil
})
if err != nil {
return nil, err
return folderTree{}, err
}
return res.(map[string]FolderNode), nil
return res.(folderTree), nil
}
func (s *Service) listPermission(ctx context.Context, scopeMap map[string]bool, req *ListRequest) (*authzv1.ListResponse, error) {
@ -609,10 +578,10 @@ func (s *Service) listPermission(ctx context.Context, scopeMap map[string]bool,
return nil, status.Error(codes.NotFound, "unsupported resource")
}
var folderMap map[string]FolderNode
var tree folderTree
if t.folderSupport {
var err error
folderMap, err = s.buildFolderTree(ctx, req.Namespace)
tree, err = s.buildFolderTree(ctx, req.Namespace)
if err != nil {
ctxLogger.Error("could not build folder and dashboard tree", "error", err)
return nil, err
@ -621,16 +590,16 @@ func (s *Service) listPermission(ctx context.Context, scopeMap map[string]bool,
var res *authzv1.ListResponse
if strings.HasPrefix(req.Action, "folders:") {
res = buildFolderList(scopeMap, folderMap)
res = buildFolderList(scopeMap, tree)
} else {
res = buildItemList(scopeMap, folderMap, t.prefix())
res = buildItemList(scopeMap, tree, t.prefix())
}
span.SetAttributes(attribute.Int("num_folders", len(res.Folders)), attribute.Int("num_items", len(res.Items)))
return res, nil
}
func buildFolderList(scopes map[string]bool, tree map[string]FolderNode) *authzv1.ListResponse {
func buildFolderList(scopes map[string]bool, tree folderTree) *authzv1.ListResponse {
itemSet := make(map[string]struct{}, len(scopes))
for scope := range scopes {
@ -640,7 +609,9 @@ func buildFolderList(scopes map[string]bool, tree map[string]FolderNode) *authzv
}
itemSet[identifier] = struct{}{}
getChildren(tree, identifier, itemSet)
for n := range tree.Children(identifier) {
itemSet[n.UID] = struct{}{}
}
}
itemList := make([]string, 0, len(itemSet))
@ -651,7 +622,7 @@ func buildFolderList(scopes map[string]bool, tree map[string]FolderNode) *authzv
return &authzv1.ListResponse{Items: itemList}
}
func buildItemList(scopes map[string]bool, tree map[string]FolderNode, prefix string) *authzv1.ListResponse {
func buildItemList(scopes map[string]bool, tree folderTree, prefix string) *authzv1.ListResponse {
folderSet := make(map[string]struct{}, len(scopes))
itemSet := make(map[string]struct{}, len(scopes))
@ -661,7 +632,9 @@ func buildItemList(scopes map[string]bool, tree map[string]FolderNode, prefix st
continue
}
folderSet[identifier] = struct{}{}
getChildren(tree, identifier, folderSet)
for n := range tree.Children(identifier) {
folderSet[n.UID] = struct{}{}
}
} else {
identifier := strings.TrimPrefix(scope, prefix)
itemSet[identifier] = struct{}{}
@ -678,18 +651,3 @@ func buildItemList(scopes map[string]bool, tree map[string]FolderNode, prefix st
return &authzv1.ListResponse{Folders: folderList, Items: itemList}
}
func getChildren(folderMap map[string]FolderNode, folderUID string, folderSet map[string]struct{}) {
folder, has := folderMap[folderUID]
if !has {
return
}
for _, child := range folder.ChildrenUIDs {
// We have already processed all the children of this folder
if _, ok := folderSet[child]; ok {
return
}
folderSet[child] = struct{}{}
getChildren(folderMap, child, folderSet)
}
}

View File

@ -25,6 +25,7 @@ func TestService_checkPermission(t *testing.T) {
name string
permissions []accesscontrol.Permission
check CheckRequest
folders []store.Folder
expected bool
}
@ -146,11 +147,37 @@ func TestService_checkPermission(t *testing.T) {
},
expected: false,
},
{
name: "should return true if user has permissions on folder",
permissions: []accesscontrol.Permission{
{
Scope: "folders:uid:parent",
Kind: "folders",
Attribute: "uid",
Identifier: "parent",
},
},
folders: []store.Folder{
{UID: "parent"},
{UID: "child", ParentUID: strPtr("parent")},
},
check: CheckRequest{
Action: "dashboards:read",
Group: "dashboard.grafana.app",
Resource: "dashboards",
Name: "some_dashboard",
ParentFolder: "child",
},
expected: true,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
s := setupService()
s.folderCache.Set(context.Background(), folderCacheKey("default"), newFolderTree(tc.folders))
tc.check.Namespace = claims.NamespaceInfo{Value: "default", OrgID: 1}
got, err := s.checkPermission(context.Background(), getScopeMap(tc.permissions), &tc.check)
require.NoError(t, err)
assert.Equal(t, tc.expected, got)
@ -371,90 +398,11 @@ func TestService_getUserPermissions(t *testing.T) {
}
}
func TestService_buildFolderTree(t *testing.T) {
type testCase struct {
name string
folders []store.Folder
cacheHit bool
expectedTree map[string]FolderNode
}
testCases := []testCase{
{
name: "should return folder tree from cache if available",
folders: []store.Folder{
{UID: "folder1", ParentUID: nil},
{UID: "folder2", ParentUID: strPtr("folder1")},
},
cacheHit: true,
expectedTree: map[string]FolderNode{
"folder1": {UID: "folder1", ChildrenUIDs: []string{"folder2"}},
"folder2": {UID: "folder2", ParentUID: strPtr("folder1")},
},
},
{
name: "should return folder tree from store if not in cache",
folders: []store.Folder{
{UID: "folder1", ParentUID: nil},
{UID: "folder2", ParentUID: strPtr("folder1")},
},
cacheHit: false,
expectedTree: map[string]FolderNode{
"folder1": {UID: "folder1", ChildrenUIDs: []string{"folder2"}},
"folder2": {UID: "folder2", ParentUID: strPtr("folder1")},
},
},
}
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}
if tc.cacheHit {
s.folderCache.Set(ctx, folderCacheKey(ns.Value), tc.expectedTree)
}
store := &fakeStore{folders: tc.folders}
s.store = store
s.permissionStore = store
tree, err := s.buildFolderTree(ctx, ns)
require.NoError(t, err)
require.Len(t, tree, len(tc.expectedTree))
for _, folder := range tc.folders {
node, ok := tree[folder.UID]
require.True(t, ok)
// Check parent
if folder.ParentUID != nil {
require.NotNil(t, node.ParentUID)
require.Equal(t, *folder.ParentUID, *node.ParentUID)
} else {
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 tc.cacheHit {
require.Zero(t, store.calls)
} else {
require.Equal(t, 1, store.calls)
}
})
}
}
func TestService_listPermission(t *testing.T) {
type testCase struct {
name string
permissions []accesscontrol.Permission
folderTree map[string]FolderNode
folders []store.Folder
list ListRequest
expectedItems []string
expectedFolders []string
@ -503,9 +451,9 @@ func TestService_listPermission(t *testing.T) {
Identifier: "some_folder_2",
},
},
folderTree: map[string]FolderNode{
"some_folder_1": {UID: "some_folder_1"},
"some_folder_2": {UID: "some_folder_2"},
folders: []store.Folder{
{UID: "some_folder_1"},
{UID: "some_folder_2"},
},
list: ListRequest{
Action: "dashboards:read",
@ -526,13 +474,13 @@ func TestService_listPermission(t *testing.T) {
Identifier: "some_folder_1",
},
},
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")},
folders: []store.Folder{
{UID: "some_folder_parent"},
{UID: "some_folder_child", ParentUID: strPtr("some_folder_parent")},
{UID: "some_folder_subchild1", ParentUID: strPtr("some_folder_child")},
{UID: "some_folder_subchild2", ParentUID: strPtr("some_folder_child")},
{UID: "some_folder_subsubchild", ParentUID: strPtr("some_folder_subchild2")},
{UID: "some_folder_1", ParentUID: strPtr("some_other_folder")},
},
list: ListRequest{
Action: "dashboards:read",
@ -559,9 +507,9 @@ func TestService_listPermission(t *testing.T) {
Identifier: "some_folder_parent",
},
},
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")},
folders: []store.Folder{
{UID: "some_folder_parent"},
{UID: "some_folder_child", ParentUID: strPtr("some_folder_parent")},
},
list: ListRequest{
Action: "dashboards:read",
@ -589,23 +537,25 @@ func TestService_listPermission(t *testing.T) {
Identifier: "some_folder_parent",
},
},
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")},
folders: []store.Folder{
{UID: "some_folder_parent"},
{UID: "some_folder_child", ParentUID: strPtr("some_folder_parent")},
{UID: "some_folder_subchild", ParentUID: strPtr("some_folder_child")},
{UID: "some_folder_child2", ParentUID: strPtr("some_folder_parent")},
},
list: ListRequest{
Action: "dashboards:read",
Group: "dashboard.grafana.app",
Resource: "dashboards",
},
expectedFolders: []string{"some_folder_parent", "some_folder_child", "some_folder_subchild"},
expectedFolders: []string{"some_folder_parent", "some_folder_child", "some_folder_child2", "some_folder_subchild"},
},
{
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"},
folders: []store.Folder{
{UID: "some_folder_1"},
},
list: ListRequest{
Action: "dashboards:read",
@ -624,9 +574,9 @@ func TestService_listPermission(t *testing.T) {
Identifier: "some_folder_parent",
},
},
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")},
folders: []store.Folder{
{UID: "some_folder_parent"},
{UID: "some_folder_child", ParentUID: strPtr("some_folder_parent")},
},
list: ListRequest{
Action: "folders:read",
@ -640,8 +590,8 @@ func TestService_listPermission(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
s := setupService()
if tc.folderTree != nil {
s.folderCache.Set(context.Background(), folderCacheKey("default"), tc.folderTree)
if tc.folders != nil {
s.folderCache.Set(context.Background(), folderCacheKey("default"), newFolderTree(tc.folders))
}
tc.list.Namespace = claims.NamespaceInfo{Value: "default", OrgID: 1}
@ -667,7 +617,7 @@ func setupService() *Service {
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),
folderCache: newCacheWrap[folderTree](cache, logger, shortCacheTTL),
store: fStore,
permissionStore: fStore,
identityStore: &fakeIdentityStore{},

View File

@ -0,0 +1,108 @@
package rbac
import (
"iter"
"github.com/grafana/grafana/pkg/services/authz/rbac/store"
)
func newFolderTree(folders []store.Folder) folderTree {
t := folderTree{
Index: make(map[string]int, len(folders)),
Nodes: make([]folderNode, 0, len(folders)),
}
for _, f := range folders {
t.Insert(f.UID, f.ParentUID)
}
return t
}
type folderTree struct {
// All nodes for the folderTree.
Nodes []folderNode
// Index is a map of folderNode UID to its positons in Nodes.
Index map[string]int
}
type folderNode struct {
// UID is the uniqiue identifier for folderNode
UID string
// Parent is the position into folderTree nodes for parent, we store -1 for nodes that don't have a parent.
Parent int
// Children is positons into folderTree nodes for all children.
Children []int
}
func (t *folderTree) Insert(uid string, parentUID *string) int {
parent := -1
if parentUID != nil {
// find parent
i, ok := t.Index[*parentUID]
if !ok {
// insert parent if it don't exists yet
i = t.Insert(*parentUID, nil)
}
parent = i
}
i, ok := t.Index[uid]
if !ok {
// this node does not exist yet so we add it to the index and append the new node
i = len(t.Nodes)
t.Index[uid] = i
t.Nodes = append(t.Nodes, folderNode{
UID: uid,
Parent: parent,
})
} else {
// if a node is added as a parent node first, its parent will not be set, so we make sure to do it now
t.Nodes[i].Parent = parent
}
if parent != -1 {
// update parent to include the index of new child node
t.Nodes[parent].Children = append(t.Nodes[parent].Children, i)
}
return i
}
// Ancestors returns an iterator that yields ancestors for uid
func (t *folderTree) Ancestors(uid string) iter.Seq[folderNode] {
current, ok := t.Index[uid]
if !ok {
return func(yield func(folderNode) bool) {}
}
current = t.Nodes[current].Parent
return func(yield func(folderNode) bool) {
for {
if current == -1 || !yield(t.Nodes[current]) {
return
}
current = t.Nodes[current].Parent
}
}
}
// Children returns an iterator that yields all children for uid
func (t *folderTree) Children(uid string) iter.Seq[folderNode] {
current, ok := t.Index[uid]
if !ok {
return func(yield func(folderNode) bool) {}
}
queue := t.Nodes[current].Children
return func(yield func(folderNode) bool) {
for len(queue) > 0 {
current, queue = queue[0], queue[1:]
if !yield(t.Nodes[current]) {
return
}
queue = append(queue, t.Nodes[current].Children...)
}
}
}

View File

@ -0,0 +1,106 @@
package rbac
import (
"fmt"
"testing"
"github.com/grafana/grafana/pkg/services/authz/rbac/store"
"github.com/stretchr/testify/assert"
)
func Test_Tree(t *testing.T) {
tree := newFolderTree([]store.Folder{
{UID: "1"},
{UID: "11", ParentUID: strPtr("1")},
{UID: "12", ParentUID: strPtr("1")},
{UID: "111", ParentUID: strPtr("11")},
{UID: "1111", ParentUID: strPtr("111")},
{UID: "121", ParentUID: strPtr("12")},
// not ordered insert to make sure patching works correctly
{UID: "22", ParentUID: strPtr("2")},
{UID: "222", ParentUID: strPtr("22")},
{UID: "21", ParentUID: strPtr("2")},
{UID: "2"},
})
verify := func(t *testing.T, expected []string, visited map[string]bool) {
assert.Len(t, visited, len(expected))
for _, e := range expected {
assert.True(t, visited[e], fmt.Sprintf("did not visit node: %s", e))
}
}
t.Run("should iterate all children of folder 1", func(t *testing.T) {
visited := map[string]bool{}
for n := range tree.Children("1") {
visited[n.UID] = true
}
expected := []string{"11", "111", "1111", "12", "121"}
verify(t, expected, visited)
})
t.Run("should iterate all children of folder 2", func(t *testing.T) {
visited := map[string]bool{}
for n := range tree.Children("2") {
visited[n.UID] = true
}
expected := []string{"21", "22", "222"}
verify(t, expected, visited)
})
t.Run("should iterate all children of folder 111", func(t *testing.T) {
visited := map[string]bool{}
for n := range tree.Children("111") {
visited[n.UID] = true
}
expected := []string{"1111"}
verify(t, expected, visited)
})
t.Run("should iterate all children of folder 1111", func(t *testing.T) {
visited := map[string]bool{}
for n := range tree.Children("1111") {
visited[n.UID] = true
}
expected := []string{}
verify(t, expected, visited)
})
t.Run("should iterate all acestors of folder 1111", func(t *testing.T) {
visited := map[string]bool{}
for n := range tree.Ancestors("1111") {
visited[n.UID] = true
}
expected := []string{"1", "11", "111"}
verify(t, expected, visited)
})
t.Run("should iterate all acestors of folder 11", func(t *testing.T) {
visited := map[string]bool{}
for n := range tree.Ancestors("11") {
visited[n.UID] = true
}
expected := []string{"1"}
verify(t, expected, visited)
})
t.Run("should iterate all acestors of folder 222", func(t *testing.T) {
visited := map[string]bool{}
for n := range tree.Ancestors("222") {
visited[n.UID] = true
}
expected := []string{"2", "22"}
verify(t, expected, visited)
})
}