UniStore: Evaluate Folder DTO attributes (#93968)

* UniStore: Evaluate Folder DTO attributes * Handle AccessControl * Reduce the number of parameters to newToFolderDto * Detach Metadata helpers from HTTPServer * Add tests --------- Signed-off-by: Maicon Costa <maiconscosta@gmail.com>
2025-07-28 14:52:36 +08:00 · 2024-10-07 07:08:16 -03:00
parent 5a7d6c104e
commit 261be0facd
8 changed files with 271 additions and 30 deletions
--- a/pkg/api/accesscontrol.go
+++ b/pkg/api/accesscontrol.go
@ -656,15 +656,15 @@ func (hs *HTTPServer) declareFixedRoles() error {

 // Metadata helpers
 // getAccessControlMetadata returns the accesscontrol metadata associated with a given resource
-func (hs *HTTPServer) getAccessControlMetadata(c *contextmodel.ReqContext,
+func getAccessControlMetadata(c *contextmodel.ReqContext,
 	prefix string, resourceID string) ac.Metadata {
 	ids := map[string]bool{resourceID: true}
-	return hs.getMultiAccessControlMetadata(c, prefix, ids)[resourceID]
+	return getMultiAccessControlMetadata(c, prefix, ids)[resourceID]
 }

 // getMultiAccessControlMetadata returns the accesscontrol metadata associated with a given set of resources
 // Context must contain permissions in the given org (see LoadPermissionsMiddleware or AuthorizeInOrgMiddleware)
-func (hs *HTTPServer) getMultiAccessControlMetadata(c *contextmodel.ReqContext,
+func getMultiAccessControlMetadata(c *contextmodel.ReqContext,
 	prefix string, resourceIDs map[string]bool) map[string]ac.Metadata {
 	if !c.QueryBool("accesscontrol") {
 		return map[string]ac.Metadata{}
--- a/pkg/api/apikey.go
+++ b/pkg/api/apikey.go
@ -56,7 +56,7 @@ func (hs *HTTPServer) GetAPIKeys(c *contextmodel.ReqContext) response.Response {
 		}
 	}

-	metadata := hs.getMultiAccessControlMetadata(c, "apikeys:id", ids)
+	metadata := getMultiAccessControlMetadata(c, "apikeys:id", ids)
 	if len(metadata) > 0 {
 		for _, key := range result {
 			key.AccessControl = metadata[strconv.FormatInt(key.ID, 10)]
--- a/pkg/api/datasources.go
+++ b/pkg/api/datasources.go
@ -136,7 +136,7 @@ func (hs *HTTPServer) GetDataSourceById(c *contextmodel.ReqContext) response.Res
 	dto := hs.convertModelToDtos(c.Req.Context(), dataSource)

 	// Add accesscontrol metadata
-	dto.AccessControl = hs.getAccessControlMetadata(c, datasources.ScopePrefix, dto.UID)
+	dto.AccessControl = getAccessControlMetadata(c, datasources.ScopePrefix, dto.UID)

 	return response.JSON(http.StatusOK, &dto)
 }
@ -222,7 +222,7 @@ func (hs *HTTPServer) GetDataSourceByUID(c *contextmodel.ReqContext) response.Re
 	dto := hs.convertModelToDtos(c.Req.Context(), ds)

 	// Add accesscontrol metadata
-	dto.AccessControl = hs.getAccessControlMetadata(c, datasources.ScopePrefix, dto.UID)
+	dto.AccessControl = getAccessControlMetadata(c, datasources.ScopePrefix, dto.UID)

 	return response.JSON(http.StatusOK, &dto)
 }
--- a/pkg/api/folder.go
+++ b/pkg/api/folder.go
@ -1,12 +1,14 @@
 package api

 import (
+	"context"
 	"errors"
 	"net/http"
 	"strconv"

 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic"

@ -28,6 +30,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/guardian"
 	"github.com/grafana/grafana/pkg/services/libraryelements/model"
 	"github.com/grafana/grafana/pkg/services/search"
+	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/util"
 	"github.com/grafana/grafana/pkg/util/errhttp"
 	"github.com/grafana/grafana/pkg/web"
@ -448,7 +451,7 @@ func (hs *HTTPServer) getFolderACMetadata(c *contextmodel.ReqContext, f *folder.
 		folderIDs[p.UID] = true
 	}

-	allMetadata := hs.getMultiAccessControlMetadata(c, dashboards.ScopeFoldersPrefix, folderIDs)
+	allMetadata := getMultiAccessControlMetadata(c, dashboards.ScopeFoldersPrefix, folderIDs)
 	metadata := map[string]bool{}
 	// Flatten metadata - if any parent has a permission, the child folder inherits it
 	for _, md := range allMetadata {
@ -629,6 +632,7 @@ type folderK8sHandler struct {
 	clientConfigProvider grafanaapiserver.DirectRestConfigProvider
 	// #TODO check if it makes more sense to move this to FolderAPIBuilder
 	accesscontrolService accesscontrol.Service
+	userService          user.Service
 }

 //-----------------------------------------------------------------------------------------
@ -641,6 +645,7 @@ func newFolderK8sHandler(hs *HTTPServer) *folderK8sHandler {
 		namespacer:           request.GetNamespaceMapper(hs.Cfg),
 		clientConfigProvider: hs.clientConfigProvider,
 		accesscontrolService: hs.accesscontrolService,
+		userService:          hs.userService,
 	}
 }

@ -692,13 +697,13 @@ func (fk8s *folderK8sHandler) createFolder(c *contextmodel.ReqContext) {
 		return
 	}

-	fk8s.accesscontrolService.ClearUserPermissionCache(c.SignedInUser)
-	f, err := internalfolders.UnstructuredToLegacyFolderDTO(*out)
+	folderDTO, err := fk8s.newToFolderDto(c, *out)
 	if err != nil {
 		fk8s.writeError(c, err)
 		return
 	}
-	c.JSON(http.StatusOK, f)
+
+	c.JSON(http.StatusOK, folderDTO)
 }

 // func (fk8s *folderK8sHandler) getFolder(c *contextmodel.ReqContext) {
@ -713,13 +718,13 @@ func (fk8s *folderK8sHandler) createFolder(c *contextmodel.ReqContext) {
 // 		return
 // 	}

-// 	f, err := internalfolders.UnstructuredToLegacyFolderDTO(*out)
+// folderDTO, err := fk8s.newToFolderDto(c, *out)
 // if err != nil {
 // 	fk8s.writeError(c, err)
 // 	return
 // }

-// 	c.JSON(http.StatusOK, f)
+// 	c.JSON(http.StatusOK, folderDTO)
 // }

 // func (fk8s *folderK8sHandler) deleteFolder(c *contextmodel.ReqContext) {
@ -755,13 +760,13 @@ func (fk8s *folderK8sHandler) createFolder(c *contextmodel.ReqContext) {
 // 		return
 // 	}

-// 	f, err := internalfolders.UnstructuredToLegacyFolderDTO(*out)
+// folderDTO, err := fk8s.newToFolderDto(c, *out)
 // if err != nil {
 // 	fk8s.writeError(c, err)
 // 	return
 // }

-// 	c.JSON(http.StatusOK, f)
+// 	c.JSON(http.StatusOK, folderDTO)
 // }

 //-----------------------------------------------------------------------------------------
@ -786,3 +791,128 @@ func (fk8s *folderK8sHandler) writeError(c *contextmodel.ReqContext, err error)
 	}
 	errhttp.Write(c.Req.Context(), err, c.Resp)
 }
+
+func (fk8s *folderK8sHandler) newToFolderDto(c *contextmodel.ReqContext, item unstructured.Unstructured) (dtos.Folder, error) {
+	ctx := c.Req.Context()
+
+	f := internalfolders.UnstructuredToLegacyFolder(item)
+
+	fDTO, err := internalfolders.UnstructuredToLegacyFolderDTO(item)
+	if err != nil {
+		return dtos.Folder{}, err
+	}
+
+	toDTO := func(f *folder.Folder, checkCanView bool) (dtos.Folder, error) {
+		g, err := guardian.NewByFolder(c.Req.Context(), f, c.SignedInUser.GetOrgID(), c.SignedInUser)
+		if err != nil {
+			return dtos.Folder{}, err
+		}
+
+		canEdit, _ := g.CanEdit()
+		canSave, _ := g.CanSave()
+		canAdmin, _ := g.CanAdmin()
+		canDelete, _ := g.CanDelete()
+
+		// Finding creator and last updater of the folder
+		updater, creator := anonString, anonString
+		if f.CreatedBy > 0 {
+			creator = fk8s.getUserLogin(ctx, f.CreatedBy)
+		}
+		if f.UpdatedBy > 0 {
+			updater = fk8s.getUserLogin(ctx, f.UpdatedBy)
+		}
+
+		acMetadata, _ := fk8s.getFolderACMetadata(c, f)
+
+		if checkCanView {
+			canView, _ := g.CanView()
+			if !canView {
+				return dtos.Folder{
+					UID:   REDACTED,
+					Title: REDACTED,
+				}, nil
+			}
+		}
+		metrics.MFolderIDsAPICount.WithLabelValues(metrics.NewToFolderDTO).Inc()
+
+		fDTO.CanSave = canSave
+		fDTO.CanEdit = canEdit
+		fDTO.CanAdmin = canAdmin
+		fDTO.CanDelete = canDelete
+		fDTO.CreatedBy = creator
+		fDTO.UpdatedBy = updater
+		fDTO.AccessControl = acMetadata
+
+		return *fDTO, nil
+	}
+
+	// no need to check view permission for the starting folder since it's already checked by the callers
+	folderDTO, err := toDTO(f, false)
+	if err != nil {
+		return dtos.Folder{}, err
+	}
+
+	// TODO: handle parents
+	/*
+		parents, err := fk8s.folderService.GetParents(ctx, folder.GetParentsQuery{UID: f.UID, OrgID: f.OrgID})
+		if err != nil {
+			// log the error instead of failing
+			fk8s.log.Error("failed to fetch folder parents", "folder", f.UID, "org", f.OrgID, "error", err)
+		}
+
+		folderDTO.Parents = make([]dtos.Folder, 0, len(parents))
+		for _, f := range parents {
+			DTO, err := toDTO(f, true)
+			if err != nil {
+				// fk8s.log.Error("failed to convert folder to DTO", "folder", f.UID, "org", f.OrgID, "error", err)
+				continue
+			}
+			folderDTO.Parents = append(folderDTO.Parents, DTO)
+		}
+	*/
+
+	return folderDTO, nil
+}
+
+func (fk8s *folderK8sHandler) getUserLogin(ctx context.Context, userID int64) string {
+	ctx, span := tracer.Start(ctx, "api.getUserLogin")
+	defer span.End()
+
+	query := user.GetUserByIDQuery{ID: userID}
+	user, err := fk8s.userService.GetByID(ctx, &query)
+	if err != nil {
+		return anonString
+	}
+	return user.Login
+}
+
+func (fk8s *folderK8sHandler) getFolderACMetadata(c *contextmodel.ReqContext, f *folder.Folder) (accesscontrol.Metadata, error) {
+	if !c.QueryBool("accesscontrol") {
+		return nil, nil
+	}
+
+	folderIDs := map[string]bool{f.UID: true}
+
+	// TODO: handle parents
+	/*
+		parents, err := fk8s.folderService.GetParents(c.Req.Context(), folder.GetParentsQuery{UID: f.UID, OrgID: c.SignedInUser.GetOrgID()})
+		if err != nil {
+			return nil, err
+		}
+
+		folderIDs := map[string]bool{f.UID: true}
+		for _, p := range parents {
+			folderIDs[p.UID] = true
+		}
+	*/
+
+	allMetadata := getMultiAccessControlMetadata(c, dashboards.ScopeFoldersPrefix, folderIDs)
+	metadata := map[string]bool{}
+	// Flatten metadata - if any parent has a permission, the child folder inherits it
+	for _, md := range allMetadata {
+		for action := range md {
+			metadata[action] = true
+		}
+	}
+	return metadata, nil
+}
--- a/pkg/api/folder_test.go
+++ b/pkg/api/folder_test.go
@ -5,17 +5,20 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"net/http/httptest"
 	"strings"
 	"testing"

 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
+	clientrest "k8s.io/client-go/rest"

 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
 	acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
+	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/folder"
@ -523,3 +526,105 @@ func TestFolderGetAPIEndpoint(t *testing.T) {
 		})
 	}
 }
+
+type mockClientConfigProvider struct {
+	host string
+}
+
+func (m mockClientConfigProvider) GetDirectRestConfig(c *contextmodel.ReqContext) *clientrest.Config {
+	return &clientrest.Config{
+		Host: m.host,
+	}
+}
+
+func (m mockClientConfigProvider) DirectlyServeHTTP(w http.ResponseWriter, r *http.Request) {}
+
+func TestHTTPServer_FolderMetadataK8s(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		//nolint:errcheck
+		fmt.Fprintln(w,
+			`{
+				"kind": "Folder",
+				"apiVersion": "folder.grafana.app/v0alpha1",
+				"metadata": {
+					"name": "ady4yobv315a8e",
+					"namespace": "default",
+					"uid": "28f306ee-ada1-40f4-8011-b2d1df462aad",
+					"creationTimestamp": "2024-09-17T04:16:35Z",
+					"annotations": {
+						"grafana.app/createdBy": "user:fdxsqt7t5ryf4a",
+						"grafana.app/originName": "SQL",
+						"grafana.app/originPath": "3"
+					}
+				},
+				"spec": {
+					"title": "Example folder 226"
+				}
+			}`)
+	}))
+	defer ts.Close()
+
+	mockClientConfigProvider := mockClientConfigProvider{
+		host: ts.URL,
+	}
+
+	setUpRBACGuardian(t)
+	folderService := &foldertest.FakeService{}
+	features := featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagKubernetesFolders)
+	server := SetupAPITestServer(t, func(hs *HTTPServer) {
+		hs.Cfg = setting.NewCfg()
+		hs.folderService = folderService
+		hs.QuotaService = quotatest.New(false, nil)
+		hs.SearchService = &mockSearchService{
+			ExpectedResult: model.HitList{},
+		}
+		hs.Features = features
+		hs.clientConfigProvider = mockClientConfigProvider
+	})
+
+	t.Run("Should attach access control metadata to folder response", func(t *testing.T) {
+		folderService.ExpectedFolder = &folder.Folder{UID: "ady4yobv315a8e"}
+
+		req := server.NewGetRequest("/api/folders/ady4yobv315a8e?accesscontrol=true")
+		webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{
+			1: accesscontrol.GroupScopesByActionContext(context.Background(), []accesscontrol.Permission{
+				{Action: dashboards.ActionFoldersRead, Scope: dashboards.ScopeFoldersAll},
+				{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID("ady4yobv315a8e")},
+			}),
+		}})
+
+		res, err := server.Send(req)
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusOK, res.StatusCode)
+		defer func() { require.NoError(t, res.Body.Close()) }()
+
+		body := dtos.Folder{}
+		require.NoError(t, json.NewDecoder(res.Body).Decode(&body))
+
+		assert.True(t, body.AccessControl[dashboards.ActionFoldersRead])
+		assert.True(t, body.AccessControl[dashboards.ActionFoldersWrite])
+	})
+
+	t.Run("Should not attach access control metadata to folder response", func(t *testing.T) {
+		folderService.ExpectedFolder = &folder.Folder{UID: "ady4yobv315a8e"}
+
+		req := server.NewGetRequest("/api/folders/ady4yobv315a8e")
+		webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{
+			1: accesscontrol.GroupScopesByActionContext(context.Background(), []accesscontrol.Permission{
+				{Action: dashboards.ActionFoldersRead, Scope: dashboards.ScopeFoldersAll},
+				{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID("ady4yobv315a8e")},
+			}),
+		}})
+
+		res, err := server.Send(req)
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusOK, res.StatusCode)
+		defer func() { require.NoError(t, res.Body.Close()) }()
+
+		body := dtos.Folder{}
+		require.NoError(t, json.NewDecoder(res.Body).Decode(&body))
+
+		assert.False(t, body.AccessControl[dashboards.ActionFoldersRead])
+		assert.False(t, body.AccessControl[dashboards.ActionFoldersWrite])
+	})
+}
--- a/pkg/api/plugins.go
+++ b/pkg/api/plugins.go
@ -123,7 +123,7 @@ func (hs *HTTPServer) GetPluginList(c *contextmodel.ReqContext) response.Respons
 	}

 	// Compute metadata
-	pluginsMetadata := hs.getMultiAccessControlMetadata(c, pluginaccesscontrol.ScopeProvider.GetResourceScope(""), filteredPluginIDs)
+	pluginsMetadata := getMultiAccessControlMetadata(c, pluginaccesscontrol.ScopeProvider.GetResourceScope(""), filteredPluginIDs)

 	// Prepare DTO
 	result := make(dtos.PluginList, 0)
--- a/pkg/api/user.go
+++ b/pkg/api/user.go
@ -91,7 +91,7 @@ func (hs *HTTPServer) getUserUserProfile(c *contextmodel.ReqContext, userID int6
 		userProfile.IsGrafanaAdminExternallySynced = login.IsGrafanaAdminExternallySynced(hs.Cfg, oauthInfo, authInfo.AuthModule)
 	}

-	userProfile.AccessControl = hs.getAccessControlMetadata(c, "global.users:id:", strconv.FormatInt(userID, 10))
+	userProfile.AccessControl = getAccessControlMetadata(c, "global.users:id:", strconv.FormatInt(userID, 10))
 	userProfile.AvatarURL = dtos.GetGravatarUrl(hs.Cfg, userProfile.Email)

 	return response.JSON(http.StatusOK, userProfile)
--- a/pkg/registry/apis/folders/conversions.go
+++ b/pkg/registry/apis/folders/conversions.go
@ -79,6 +79,12 @@ func UnstructuredToLegacyFolderDTO(item unstructured.Unstructured) (*dtos.Folder
 		return nil, err
 	}

+	// avoid panic
+	var createdTime time.Time
+	if created != nil {
+		createdTime = *created
+	}
+
 	dto := &dtos.Folder{
 		UID:       uid,
 		Title:     title,
@ -91,15 +97,15 @@ func UnstructuredToLegacyFolderDTO(item unstructured.Unstructured) (*dtos.Folder
 		// UpdatedBy: meta.GetCreatedBy(),
 		URL: getURL(meta, title),
 		// #TODO get Created in format "2024-09-12T15:37:41.09466+02:00"
-		Created: *created,
+		Created: createdTime,
 		// #TODO figure out whether we want to set "updated" and "updated by". Could replace with
 		// meta.GetUpdatedTimestamp() but it currently gets overwritten in prepareObjectForStorage().
-		Updated: *created,
+		Updated: createdTime,
 		// #TODO figure out how to set these properly
-		CanSave:   true,
-		CanEdit:   true,
-		CanAdmin:  true,
-		CanDelete: true,
+		CanSave:   false,
+		CanEdit:   false,
+		CanAdmin:  false,
+		CanDelete: false,
 		HasACL:    false,

 		// #TODO figure out about adding version, parents, orgID fields