diff --git a/pkg/api/folder.go b/pkg/api/folder.go index 5c9447a8692..ba95523964f 100644 --- a/pkg/api/folder.go +++ b/pkg/api/folder.go @@ -49,7 +49,6 @@ func (hs *HTTPServer) registerFolderAPI(apiRoute routing.RouteRegister, authoriz folderRoute.Get("/id/:id", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersRead, idScope)), routing.Wrap(hs.GetFolderByID)) folderRoute.Group("/:uid", func(folderUidRoute routing.RouteRegister) { - folderUidRoute.Get("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersRead, uidScope)), routing.Wrap(hs.GetFolderByUID)) folderUidRoute.Put("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, uidScope)), routing.Wrap(hs.UpdateFolder)) folderUidRoute.Post("/move", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, uidScope)), routing.Wrap(hs.MoveFolder)) folderUidRoute.Get("/counts", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersRead, uidScope)), routing.Wrap(hs.GetFolderDescendantCounts)) @@ -66,19 +65,20 @@ func (hs *HTTPServer) registerFolderAPI(apiRoute routing.RouteRegister, authoriz folderRoute.Get("/", handler.getFolders) folderRoute.Group("/:uid", func(folderUidRoute routing.RouteRegister) { folderUidRoute.Delete("/", handler.deleteFolder) + folderUidRoute.Get("/", handler.getFolder) }) } else { folderRoute.Post("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersCreate)), routing.Wrap(hs.CreateFolder)) folderRoute.Get("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersRead)), routing.Wrap(hs.GetFolders)) folderRoute.Group("/:uid", func(folderUidRoute routing.RouteRegister) { folderUidRoute.Delete("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersDelete, uidScope)), routing.Wrap(hs.DeleteFolder)) + folderUidRoute.Get("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersRead, uidScope)), routing.Wrap(hs.GetFolderByUID)) }) } // Only adding support for some routes with the k8s handler for now. Include the rest here. if false { handler := newFolderK8sHandler(hs) folderRoute.Group("/:uid", func(folderUidRoute routing.RouteRegister) { - folderUidRoute.Get("/", handler.getFolder) folderUidRoute.Put("/:uid", handler.updateFolder) }) } @@ -994,19 +994,9 @@ func (fk8s *folderK8sHandler) getFolderACMetadata(c *contextmodel.ReqContext, f return nil, nil } - if len(f.FullpathUIDs) == 0 { - return map[string]bool{}, nil - } - - parentsFullPathUIDs := strings.Split(f.FullpathUIDs, "/") - // The first part of the path is the newly created folder which we don't need to check here - if len(parentsFullPathUIDs) < 2 { - return map[string]bool{}, nil - } - - folderIDs := map[string]bool{f.UID: true} - for _, uid := range parentsFullPathUIDs[1:] { - folderIDs[uid] = true + folderIDs, err := fk8s.getParents(f) + if err != nil { + return nil, err } allMetadata := getMultiAccessControlMetadata(c, dashboards.ScopeFoldersPrefix, folderIDs) @@ -1019,3 +1009,22 @@ func (fk8s *folderK8sHandler) getFolderACMetadata(c *contextmodel.ReqContext, f } return metadata, nil } + +func (fk8s *folderK8sHandler) getParents(f *folder.Folder) (map[string]bool, error) { + folderIDs := map[string]bool{f.UID: true} + if (f.UID == accesscontrol.GeneralFolderUID) || (f.UID == folder.SharedWithMeFolderUID) { + return folderIDs, nil + } + + parentsFullPathUIDs := strings.Split(f.FullpathUIDs, "/") + // The first part of the path is the newly created folder which we don't need to check here + if len(parentsFullPathUIDs) < 2 { + return folderIDs, nil + } + + for _, uid := range parentsFullPathUIDs[1:] { + folderIDs[uid] = true + } + + return folderIDs, nil +} diff --git a/pkg/api/folder_test.go b/pkg/api/folder_test.go index f0c6999f96f..6a45068ea23 100644 --- a/pkg/api/folder_test.go +++ b/pkg/api/folder_test.go @@ -5,20 +5,17 @@ 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" @@ -513,105 +510,3 @@ 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/repoName": "SQL", - "grafana.app/repoPath": "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/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index 89466e7576c..586d0d3d3bc 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -286,9 +286,47 @@ func (s *Service) Get(ctx context.Context, q *folder.GetFolderQuery) (*folder.Fo f.FullpathUIDs = f.UID // set full path to the folder UID } + if s.features.IsEnabled(ctx, featuremgmt.FlagKubernetesFolders) { + f, err = s.setFullpath(ctx, f, q.SignedInUser) + } + return f, err } +func (s *Service) setFullpath(ctx context.Context, f *folder.Folder, user identity.Requester) (*folder.Folder, error) { + // #TODO is some kind of intermediate conversion required as is the case with user id where + // it gets parsed using UserIdentifier(). Also is there some kind of validation taking place as + // part of the parsing? + f.CreatedByUID = user.GetUID() + f.UpdatedByUID = user.GetUID() + + if f.ParentUID == "" { + return f, nil + } + + // Fetch the parent since the permissions for fetching the newly created folder + // are not yet present for the user--this requires a call to ClearUserPermissionCache + parents, err := s.GetParents(ctx, folder.GetParentsQuery{ + UID: f.UID, + OrgID: f.OrgID, + }) + if err != nil { + return nil, err + } + // #TODO revisit setting permissions so that we can centralise the logic for escaping slashes in titles + // Escape forward slashes in the title + escapedSlash := "\\/" + title := strings.Replace(f.Title, "/", escapedSlash, -1) + f.Fullpath = title + f.FullpathUIDs = f.UID + for _, p := range parents { + pt := strings.Replace(p.Title, "/", escapedSlash, -1) + f.Fullpath = f.Fullpath + "/" + pt + f.FullpathUIDs = f.FullpathUIDs + "/" + p.UID + } + return f, nil +} + func (s *Service) GetChildren(ctx context.Context, q *folder.GetChildrenQuery) ([]*folder.Folder, error) { defer func(t time.Time) { parent := q.UID @@ -673,33 +711,7 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) ( } if s.features.IsEnabled(ctx, featuremgmt.FlagKubernetesFolders) { - // #TODO is some kind of intermediate conversion required as is the case with user id where - // it gets parsed using UserIdentifier(). Also is there some kind of validation taking place as - // part of the parsing? - f.CreatedByUID = user.GetUID() - f.UpdatedByUID = user.GetUID() - - if f.ParentUID == "" { - return f, nil - } - - // Fetch the parent since the permissions for fetching the newly created folder - // are not yet present for the user--this requires a call to ClearUserPermissionCache - parent, err := s.Get(ctx, &folder.GetFolderQuery{ - UID: &f.ParentUID, - OrgID: f.OrgID, - WithFullpath: true, - WithFullpathUIDs: true, - SignedInUser: user, - }) - if err != nil { - return nil, err - } - // #TODO revisit setting permissions so that we can centralise the logic for escaping slashes in titles - // Escape forward slashes in the title - title := strings.Replace(f.Title, "/", "\\/", -1) - f.Fullpath = title + "/" + parent.Fullpath - f.FullpathUIDs = f.UID + "/" + parent.FullpathUIDs + f, err = s.setFullpath(ctx, f, user) } return f, nil diff --git a/pkg/tests/apis/folder/folders_test.go b/pkg/tests/apis/folder/folders_test.go index dfad1e0f505..d81058f0f3a 100644 --- a/pkg/tests/apis/folder/folders_test.go +++ b/pkg/tests/apis/folder/folders_test.go @@ -16,6 +16,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + "github.com/grafana/grafana/pkg/api" "github.com/grafana/grafana/pkg/api/dtos" folderv0alpha1 "github.com/grafana/grafana/pkg/apis/folder/v0alpha1" grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" @@ -767,6 +768,156 @@ func TestIntegrationFolderCreatePermissions(t *testing.T) { } } +func TestIntegrationFolderGetPermissions(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + type testCase struct { + description string + permissions []resourcepermissions.SetResourcePermissionCommand + expectedCode int + expectedParentUIDs []string + expectedParentTitles []string + checkAccessControl bool + } + tcs := []testCase{ + { + description: "get folder by UID should return parent folders if nested folder are enabled", + expectedCode: http.StatusOK, + expectedParentUIDs: []string{"parentuid"}, + expectedParentTitles: []string{"testparent"}, + permissions: []resourcepermissions.SetResourcePermissionCommand{ + { + Actions: []string{dashboards.ActionFoldersRead}, + Resource: "folders", + ResourceAttribute: "uid", + ResourceID: "*", + }, + }, + checkAccessControl: true, + }, + { + description: "get folder by UID should return parent folders redacted if nested folder are enabled and user does not have read access to parent folders", + expectedCode: http.StatusOK, + expectedParentUIDs: []string{api.REDACTED}, + expectedParentTitles: []string{api.REDACTED}, + permissions: []resourcepermissions.SetResourcePermissionCommand{ + { + Actions: []string{dashboards.ActionFoldersRead}, + Resource: "folders", + ResourceAttribute: "uid", + ResourceID: "descuid", + }, + }, + }, + { + description: "get folder by UID should not succeed if user doesn't have permissions for the folder", + expectedCode: http.StatusForbidden, + expectedParentUIDs: []string{}, + expectedParentTitles: []string{}, + permissions: []resourcepermissions.SetResourcePermissionCommand{}, + }, + } + + for _, tc := range tcs { + t.Run(tc.description, func(t *testing.T) { + helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{ + AppModeProduction: true, + DisableAnonymous: true, + APIServerStorageType: "unified", + UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{ + folderv0alpha1.RESOURCEGROUP: { + DualWriterMode: grafanarest.Mode1, + }, + }, + EnableFeatureToggles: []string{ + featuremgmt.FlagGrafanaAPIServerTestingWithExperimentalAPIs, + featuremgmt.FlagNestedFolders, + featuremgmt.FlagKubernetesFolders, + }, + }) + + // Create parent folder + parentPayload := `{ + "title": "testparent", + "uid": "parentuid" + }` + parentCreate := apis.DoRequest(helper, apis.RequestParams{ + User: helper.Org1.Admin, + Method: http.MethodPost, + Path: "/api/folders", + Body: []byte(parentPayload), + }, &folder.Folder{}) + require.NotNil(t, parentCreate.Result) + parentUID := parentCreate.Result.UID + require.NotEmpty(t, parentUID) + + // Create descendant folder + payload := "{ \"uid\": \"descuid\", \"title\": \"Folder\", \"parentUid\": \"parentuid\"}" + resp := apis.DoRequest(helper, apis.RequestParams{ + User: helper.Org1.Admin, + Method: http.MethodPost, + Path: "/api/folders", + Body: []byte(payload), + }, &dtos.Folder{}) + require.Equal(t, http.StatusOK, resp.Response.StatusCode) + + user := helper.CreateUser("user", apis.Org1, org.RoleNone, tc.permissions) + + // Get with accesscontrol disabled + getResp := apis.DoRequest(helper, apis.RequestParams{ + User: user, + Method: http.MethodGet, + Path: "/api/folders/descuid", + }, &dtos.Folder{}) + require.Equal(t, tc.expectedCode, getResp.Response.StatusCode) + require.NotNil(t, getResp.Result) + + require.False(t, getResp.Result.AccessControl[dashboards.ActionFoldersRead]) + require.False(t, getResp.Result.AccessControl[dashboards.ActionFoldersWrite]) + + parents := getResp.Result.Parents + require.Equal(t, len(tc.expectedParentUIDs), len(parents)) + require.Equal(t, len(tc.expectedParentTitles), len(parents)) + for i := 0; i < len(tc.expectedParentUIDs); i++ { + require.Equal(t, tc.expectedParentUIDs[i], parents[i].UID) + require.Equal(t, tc.expectedParentTitles[i], parents[i].Title) + } + + // Get with accesscontrol enabled + if tc.checkAccessControl { + acPerms := []resourcepermissions.SetResourcePermissionCommand{ + { + Actions: []string{dashboards.ActionFoldersRead}, + Resource: "folders", + ResourceAttribute: "uid", + ResourceID: "*", + }, + { + Actions: []string{dashboards.ActionFoldersWrite}, + Resource: "folders", + ResourceAttribute: "uid", + ResourceID: "parentuid", + }, + } + acUser := helper.CreateUser("acuser", apis.Org1, org.RoleNone, acPerms) + + getWithAC := apis.DoRequest(helper, apis.RequestParams{ + User: acUser, + Method: http.MethodGet, + Path: "/api/folders/descuid?accesscontrol=true", + }, &dtos.Folder{}) + require.Equal(t, tc.expectedCode, getWithAC.Response.StatusCode) + require.NotNil(t, getWithAC.Result) + + require.True(t, getWithAC.Result.AccessControl[dashboards.ActionFoldersRead]) + require.True(t, getWithAC.Result.AccessControl[dashboards.ActionFoldersWrite]) + } + }) + } +} + // TestFoldersCreateAPIEndpointK8S is the counterpart of pkg/api/folder_test.go TestFoldersCreateAPIEndpoint func TestFoldersCreateAPIEndpointK8S(t *testing.T) { if testing.Short() {