From 261be0facddad769e33ee9bc69452c62e36203ea Mon Sep 17 00:00:00 2001 From: maicon Date: Mon, 7 Oct 2024 07:08:16 -0300 Subject: [PATCH] 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 --- pkg/api/accesscontrol.go | 6 +- pkg/api/apikey.go | 2 +- pkg/api/datasources.go | 4 +- pkg/api/folder.go | 162 ++++++++++++++++++++--- pkg/api/folder_test.go | 105 +++++++++++++++ pkg/api/plugins.go | 2 +- pkg/api/user.go | 2 +- pkg/registry/apis/folders/conversions.go | 18 ++- 8 files changed, 271 insertions(+), 30 deletions(-) diff --git a/pkg/api/accesscontrol.go b/pkg/api/accesscontrol.go index 0b1cac3cc2b..8c23b559997 100644 --- 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{} diff --git a/pkg/api/apikey.go b/pkg/api/apikey.go index ea2fe9c1e34..067c3e8bcc3 100644 --- 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)] diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index 2a6b3bfd4b8..4a29dfa6c19 100644 --- 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) } diff --git a/pkg/api/folder.go b/pkg/api/folder.go index ca1cfe041cf..9f554da2c7e 100644 --- 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) -// if err != nil { -// fk8s.writeError(c, err) -// return -// } +// 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) -// if err != nil { -// fk8s.writeError(c, err) -// return -// } +// 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 +} diff --git a/pkg/api/folder_test.go b/pkg/api/folder_test.go index 752e649a030..c5086968880 100644 --- 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]) + }) +} diff --git a/pkg/api/plugins.go b/pkg/api/plugins.go index e947233e2fb..905feddef0f 100644 --- 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) diff --git a/pkg/api/user.go b/pkg/api/user.go index eb3b2c265ca..26096c78d38 100644 --- 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) diff --git a/pkg/registry/apis/folders/conversions.go b/pkg/registry/apis/folders/conversions.go index e70a5dff7d0..86389ca1434 100644 --- 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