Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[v10.0.x] Nested folders: Fix folder hierarchy in folder responses #74581

Merged
merged 1 commit into from Sep 8, 2023
Merged

[v10.0.x] Nested folders: Fix folder hierarchy in folder responses #74581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
60 changes: 43 additions & 17 deletions pkg/api/folder.go
View file
Open in desktop
Expand Up @@ -22,6 +22,8 @@ import (
"github.com/grafana/grafana/pkg/web"
)

const REDACTED = "redacted"

// swagger:route GET /folders folders getFolders
//
// Get all folders.
Expand Down Expand Up @@ -86,12 +88,12 @@ func (hs *HTTPServer) GetFolderByUID(c *contextmodel.ReqContext) response.Respon
return apierrors.ToFolderErrorResponse(err)
}

g, err := guardian.NewByUID(c.Req.Context(), folder.UID, 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
Expand Down Expand Up @@ -119,11 +121,11 @@ func (hs *HTTPServer) GetFolderByID(c *contextmodel.ReqContext) response.Respons
return apierrors.ToFolderErrorResponse(err)
}

g, err := guardian.NewByUID(c.Req.Context(), folder.UID, 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
Expand Down Expand Up @@ -162,13 +164,13 @@ func (hs *HTTPServer) CreateFolder(c *contextmodel.ReqContext) response.Response
hs.accesscontrolService.ClearUserPermissionCache(c.SignedInUser)
}

g, err := guardian.NewByUID(c.Req.Context(), folder.UID, 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 {
Expand Down Expand Up @@ -223,11 +225,11 @@ func (hs *HTTPServer) MoveFolder(c *contextmodel.ReqContext) response.Response {
return response.Error(http.StatusInternalServerError, "move folder failed", err)
}

g, err := guardian.NewByUID(c.Req.Context(), cmd.UID, 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."
Expand Down Expand Up @@ -259,12 +261,12 @@ func (hs *HTTPServer) UpdateFolder(c *contextmodel.ReqContext) response.Response
if err != nil {
return apierrors.ToFolderErrorResponse(err)
}
g, err := guardian.NewByUID(c.Req.Context(), result.UID, 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
Expand Down Expand Up @@ -318,9 +320,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.NewByUID(c.Req.Context(), f.UID, c.OrgID, c.SignedInUser)
if err != nil {
return dtos.Folder{}, err
}

canEdit, _ := g.CanEdit()
canSave, _ := g.CanSave()
canAdmin, _ := g.CanAdmin()
Expand All @@ -337,6 +344,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,
Expand All @@ -354,13 +371,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})
Expand All @@ -371,10 +392,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) {
Expand Down
24 changes: 23 additions & 1 deletion pkg/api/folder_test.go
View file
Open in desktop
Expand Up @@ -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"
Expand Down Expand Up @@ -441,7 +442,6 @@ func TestFolderGetAPIEndpoint(t *testing.T) {
},
},
}
setUpRBACGuardian(t)

type testCase struct {
description string
Expand All @@ -451,6 +451,7 @@ func TestFolderGetAPIEndpoint(t *testing.T) {
expectedParentUIDs []string
expectedParentTitles []string
permissions []accesscontrol.Permission
g *guardian.FakeDashboardGuardian
}
tcs := []testCase{
{
Expand All @@ -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",
Expand All @@ -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},
},
}

Expand All @@ -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)
Expand Down