From d16374d339eb400f7e2b7565aca0457a7e2559c0 Mon Sep 17 00:00:00 2001 From: Karl Persson <23356117+kalleep@users.noreply.github.com> Date: Mon, 3 Feb 2025 12:14:28 +0100 Subject: [PATCH] Authz: For list collect all folder permisions into items (#99955) * For list collect all folder permisions into items --------- Co-authored-by: Gabriel MABILLE --- pkg/services/authz/rbac/service.go | 70 +++++++++++++++++-------- pkg/services/authz/rbac/service_test.go | 46 +++++++++++----- 2 files changed, 83 insertions(+), 33 deletions(-) diff --git a/pkg/services/authz/rbac/service.go b/pkg/services/authz/rbac/service.go index 9c8a82a8ee1..26bd352903c 100644 --- a/pkg/services/authz/rbac/service.go +++ b/pkg/services/authz/rbac/service.go @@ -619,27 +619,28 @@ func (s *Service) listPermission(ctx context.Context, scopeMap map[string]bool, } } - folderSet := make(map[string]struct{}, len(scopeMap)) - - prefix := t.prefix() - itemSet := make(map[string]struct{}, len(scopeMap)) - for scope := range scopeMap { - if strings.HasPrefix(scope, "folders:uid:") { - identifier := strings.TrimPrefix(scope, "folders:uid:") - if _, ok := folderSet[identifier]; ok { - continue - } - folderSet[identifier] = struct{}{} - getChildren(folderMap, identifier, folderSet) - } else { - identifier := strings.TrimPrefix(scope, prefix) - itemSet[identifier] = struct{}{} - } + var res *authzv1.ListResponse + if strings.HasPrefix(req.Action, "folders:") { + res = buildFolderList(scopeMap, folderMap) + } else { + res = buildItemList(scopeMap, folderMap, t.prefix()) } - folderList := make([]string, 0, len(folderSet)) - for folder := range folderSet { - folderList = append(folderList, folder) + 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 { + itemSet := make(map[string]struct{}, len(scopes)) + + for scope := range scopes { + identifier := strings.TrimPrefix(scope, "folders:uid:") + if _, ok := itemSet[identifier]; ok { + continue + } + + itemSet[identifier] = struct{}{} + getChildren(tree, identifier, itemSet) } itemList := make([]string, 0, len(itemSet)) @@ -647,8 +648,35 @@ func (s *Service) listPermission(ctx context.Context, scopeMap map[string]bool, itemList = append(itemList, item) } - span.SetAttributes(attribute.Int("num_folders", len(folderList)), attribute.Int("num_items", len(itemList))) - return &authzv1.ListResponse{Folders: folderList, Items: itemList}, nil + return &authzv1.ListResponse{Items: itemList} +} + +func buildItemList(scopes map[string]bool, tree map[string]FolderNode, prefix string) *authzv1.ListResponse { + folderSet := make(map[string]struct{}, len(scopes)) + itemSet := make(map[string]struct{}, len(scopes)) + + for scope := range scopes { + if identifier, ok := strings.CutPrefix(scope, "folders:uid:"); ok { + if _, ok := folderSet[identifier]; ok { + continue + } + folderSet[identifier] = struct{}{} + getChildren(tree, identifier, folderSet) + } else { + identifier := strings.TrimPrefix(scope, prefix) + itemSet[identifier] = struct{}{} + } + } + folderList := make([]string, 0, len(folderSet)) + for folder := range folderSet { + folderList = append(folderList, folder) + } + itemList := make([]string, 0, len(itemSet)) + for item := range itemSet { + itemList = append(itemList, item) + } + + return &authzv1.ListResponse{Folders: folderList, Items: itemList} } func getChildren(folderMap map[string]FolderNode, folderUID string, folderSet map[string]struct{}) { diff --git a/pkg/services/authz/rbac/service_test.go b/pkg/services/authz/rbac/service_test.go index 586f528f863..a8ad76d61fe 100644 --- a/pkg/services/authz/rbac/service_test.go +++ b/pkg/services/authz/rbac/service_test.go @@ -452,13 +452,13 @@ func TestService_buildFolderTree(t *testing.T) { func TestService_listPermission(t *testing.T) { type testCase struct { - name string - permissions []accesscontrol.Permission - folderTree map[string]FolderNode - list ListRequest - expectedDashboards []string - expectedFolders []string - expectedAll bool + name string + permissions []accesscontrol.Permission + folderTree map[string]FolderNode + list ListRequest + expectedItems []string + expectedFolders []string + expectedAll bool } testCases := []testCase{ @@ -512,8 +512,8 @@ func TestService_listPermission(t *testing.T) { Group: "dashboard.grafana.app", Resource: "dashboards", }, - expectedDashboards: []string{"some_dashboard"}, - expectedFolders: []string{"some_folder_1", "some_folder_2"}, + expectedItems: []string{"some_dashboard"}, + expectedFolders: []string{"some_folder_1", "some_folder_2"}, }, { name: "should return folders that user has inherited access to", @@ -568,8 +568,8 @@ func TestService_listPermission(t *testing.T) { Group: "dashboard.grafana.app", Resource: "dashboards", }, - expectedDashboards: []string{"some_dashboard"}, - expectedFolders: []string{"some_folder_parent", "some_folder_child"}, + expectedItems: []string{"some_dashboard"}, + expectedFolders: []string{"some_folder_parent", "some_folder_child"}, }, { name: "should deduplicate folders that user has inherited as well as direct access to", @@ -613,6 +613,28 @@ func TestService_listPermission(t *testing.T) { Resource: "dashboards", }, }, + { + name: "should collect folder permissions into items", + permissions: []accesscontrol.Permission{ + { + Action: "folders:read", + Scope: "folders:uid:some_folder_parent", + Kind: "folders", + Attribute: "uid", + 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")}, + }, + list: ListRequest{ + Action: "folders:read", + Group: "folder.grafana.app", + Resource: "folders", + }, + expectedItems: []string{"some_folder_parent", "some_folder_child"}, + }, } for _, tc := range testCases { @@ -626,7 +648,7 @@ func TestService_listPermission(t *testing.T) { got, err := s.listPermission(context.Background(), getScopeMap(tc.permissions), &tc.list) require.NoError(t, err) assert.Equal(t, tc.expectedAll, got.All) - assert.ElementsMatch(t, tc.expectedDashboards, got.Items) + assert.ElementsMatch(t, tc.expectedItems, got.Items) assert.ElementsMatch(t, tc.expectedFolders, got.Folders) }) }