diff --git a/pkg/api/folder.go b/pkg/api/folder.go index 46fc794395b..e566625b431 100644 --- a/pkg/api/folder.go +++ b/pkg/api/folder.go @@ -22,6 +22,8 @@ import ( "github.com/grafana/grafana/pkg/web" ) +const REDACTED = "redacted" + // swagger:route GET /folders folders getFolders // // Get all folders. @@ -86,12 +88,12 @@ func (hs *HTTPServer) GetFolderByUID(c *contextmodel.ReqContext) response.Respon return apierrors.ToFolderErrorResponse(err) } - g, err := guardian.NewByFolder(c.Req.Context(), folder, c.OrgID, c.SignedInUser) + folderDTO, err := hs.newToFolderDto(c, folder) if err != nil { return response.Err(err) } - return response.JSON(http.StatusOK, hs.newToFolderDto(c, g, folder)) + return response.JSON(http.StatusOK, folderDTO) } // swagger:route GET /folders/id/{folder_id} folders getFolderByID @@ -119,11 +121,11 @@ func (hs *HTTPServer) GetFolderByID(c *contextmodel.ReqContext) response.Respons return apierrors.ToFolderErrorResponse(err) } - g, err := guardian.NewByFolder(c.Req.Context(), folder, c.OrgID, c.SignedInUser) + folderDTO, err := hs.newToFolderDto(c, folder) if err != nil { return response.Err(err) } - return response.JSON(http.StatusOK, hs.newToFolderDto(c, g, folder)) + return response.JSON(http.StatusOK, folderDTO) } // swagger:route POST /folders folders createFolder @@ -160,13 +162,13 @@ func (hs *HTTPServer) CreateFolder(c *contextmodel.ReqContext) response.Response // Required for cases when caller wants to immediately interact with the newly created object hs.accesscontrolService.ClearUserPermissionCache(c.SignedInUser) - g, err := guardian.NewByFolder(c.Req.Context(), folder, c.OrgID, c.SignedInUser) + folderDTO, err := hs.newToFolderDto(c, folder) if err != nil { return response.Err(err) } // TODO set ParentUID if nested folders are enabled - return response.JSON(http.StatusOK, hs.newToFolderDto(c, g, folder)) + return response.JSON(http.StatusOK, folderDTO) } func (hs *HTTPServer) setDefaultFolderPermissions(ctx context.Context, orgID int64, user *user.SignedInUser, folder *folder.Folder) error { @@ -215,11 +217,11 @@ func (hs *HTTPServer) MoveFolder(c *contextmodel.ReqContext) response.Response { return response.Error(http.StatusInternalServerError, "move folder failed", err) } - g, err := guardian.NewByFolder(c.Req.Context(), theFolder, c.OrgID, c.SignedInUser) + folderDTO, err := hs.newToFolderDto(c, theFolder) if err != nil { return response.Err(err) } - return response.JSON(http.StatusOK, hs.newToFolderDto(c, g, theFolder)) + return response.JSON(http.StatusOK, folderDTO) } result := map[string]string{} result["message"] = "To use this service, you need to activate nested folder feature." @@ -251,12 +253,12 @@ func (hs *HTTPServer) UpdateFolder(c *contextmodel.ReqContext) response.Response if err != nil { return apierrors.ToFolderErrorResponse(err) } - g, err := guardian.NewByFolder(c.Req.Context(), result, c.OrgID, c.SignedInUser) + folderDTO, err := hs.newToFolderDto(c, result) if err != nil { return response.Err(err) } - return response.JSON(http.StatusOK, hs.newToFolderDto(c, g, result)) + return response.JSON(http.StatusOK, folderDTO) } // swagger:route DELETE /folders/{folder_uid} folders deleteFolder @@ -316,9 +318,14 @@ func (hs *HTTPServer) GetFolderDescendantCounts(c *contextmodel.ReqContext) resp return response.JSON(http.StatusOK, counts) } -func (hs *HTTPServer) newToFolderDto(c *contextmodel.ReqContext, g guardian.DashboardGuardian, f *folder.Folder) dtos.Folder { +func (hs *HTTPServer) newToFolderDto(c *contextmodel.ReqContext, f *folder.Folder) (dtos.Folder, error) { ctx := c.Req.Context() - toDTO := func(f *folder.Folder) dtos.Folder { + toDTO := func(f *folder.Folder, checkCanView bool) (dtos.Folder, error) { + g, err := guardian.NewByFolder(c.Req.Context(), f, c.OrgID, c.SignedInUser) + if err != nil { + return dtos.Folder{}, err + } + canEdit, _ := g.CanEdit() canSave, _ := g.CanSave() canAdmin, _ := g.CanAdmin() @@ -335,6 +342,16 @@ func (hs *HTTPServer) newToFolderDto(c *contextmodel.ReqContext, g guardian.Dash acMetadata, _ := hs.getFolderACMetadata(c, f) + if checkCanView { + canView, _ := g.CanView() + if !canView { + return dtos.Folder{ + Uid: REDACTED, + Title: REDACTED, + }, nil + } + } + return dtos.Folder{ Id: f.ID, Uid: f.UID, @@ -352,13 +369,17 @@ func (hs *HTTPServer) newToFolderDto(c *contextmodel.ReqContext, g guardian.Dash Version: f.Version, AccessControl: acMetadata, ParentUID: f.ParentUID, - } + }, nil } - folderDTO := toDTO(f) + // 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 + } if !hs.Features.IsEnabled(featuremgmt.FlagNestedFolders) { - return folderDTO + return folderDTO, nil } parents, err := hs.folderService.GetParents(ctx, folder.GetParentsQuery{UID: f.UID, OrgID: f.OrgID}) @@ -369,10 +390,15 @@ func (hs *HTTPServer) newToFolderDto(c *contextmodel.ReqContext, g guardian.Dash folderDTO.Parents = make([]dtos.Folder, 0, len(parents)) for _, f := range parents { - folderDTO.Parents = append(folderDTO.Parents, toDTO(f)) + DTO, err := toDTO(f, true) + if err != nil { + hs.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 + return folderDTO, nil } func (hs *HTTPServer) getFolderACMetadata(c *contextmodel.ReqContext, f *folder.Folder) (accesscontrol.Metadata, error) { diff --git a/pkg/api/folder_test.go b/pkg/api/folder_test.go index 7d3b12f8808..f982e717e7b 100644 --- a/pkg/api/folder_test.go +++ b/pkg/api/folder_test.go @@ -19,6 +19,7 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/folder/foldertest" + "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/quota/quotatest" "github.com/grafana/grafana/pkg/services/search/model" "github.com/grafana/grafana/pkg/services/user" @@ -441,7 +442,6 @@ func TestFolderGetAPIEndpoint(t *testing.T) { }, }, } - setUpRBACGuardian(t) type testCase struct { description string @@ -451,6 +451,7 @@ func TestFolderGetAPIEndpoint(t *testing.T) { expectedParentUIDs []string expectedParentTitles []string permissions []accesscontrol.Permission + g *guardian.FakeDashboardGuardian } tcs := []testCase{ { @@ -463,6 +464,19 @@ func TestFolderGetAPIEndpoint(t *testing.T) { permissions: []accesscontrol.Permission{ {Action: dashboards.ActionFoldersRead, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID("uid")}, }, + g: &guardian.FakeDashboardGuardian{CanViewValue: 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", + URL: "/api/folders/uid", + expectedCode: http.StatusOK, + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + expectedParentUIDs: []string{REDACTED, REDACTED}, + expectedParentTitles: []string{REDACTED, REDACTED}, + permissions: []accesscontrol.Permission{ + {Action: dashboards.ActionFoldersRead, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID("uid")}, + }, + g: &guardian.FakeDashboardGuardian{CanViewValue: false}, }, { description: "get folder by UID should not return parent folders if nested folder are disabled", @@ -474,6 +488,7 @@ func TestFolderGetAPIEndpoint(t *testing.T) { permissions: []accesscontrol.Permission{ {Action: dashboards.ActionFoldersRead, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID("uid")}, }, + g: &guardian.FakeDashboardGuardian{CanViewValue: true}, }, } @@ -487,6 +502,13 @@ func TestFolderGetAPIEndpoint(t *testing.T) { }) t.Run(tc.description, func(t *testing.T) { + origNewGuardian := guardian.New + t.Cleanup(func() { + guardian.New = origNewGuardian + }) + + guardian.MockDashboardGuardian(tc.g) + req := srv.NewGetRequest(tc.URL) req = webtest.RequestWithSignedInUser(req, userWithPermissions(1, tc.permissions)) resp, err := srv.Send(req)