Folders: Allow listing folders with write permission (#83527)

* Folders: Allow listing folders with write permission

* Check for subfolder access if parent does not have

* Add test

* GetFolders: fix ordering

* Apply suggestion from code review
This commit is contained in:
Sofia Papagiannaki 2024-03-15 14:05:27 +02:00 committed by GitHub
parent 39232a0776
commit 1208888bb6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 347 additions and 16 deletions

View File

@ -43,12 +43,18 @@ const REDACTED = "redacted"
// 403: forbiddenError
// 500: internalServerError
func (hs *HTTPServer) GetFolders(c *contextmodel.ReqContext) response.Response {
permission := dashboardaccess.PERMISSION_VIEW
if c.Query("permission") == "Edit" {
permission = dashboardaccess.PERMISSION_EDIT
}
if hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagNestedFolders) {
q := &folder.GetChildrenQuery{
OrgID: c.SignedInUser.GetOrgID(),
Limit: c.QueryInt64("limit"),
Page: c.QueryInt64("page"),
UID: c.Query("parentUid"),
Permission: permission,
SignedInUser: c.SignedInUser,
}
@ -71,7 +77,7 @@ func (hs *HTTPServer) GetFolders(c *contextmodel.ReqContext) response.Response {
return response.JSON(http.StatusOK, hits)
}
hits, err := hs.searchFolders(c)
hits, err := hs.searchFolders(c, permission)
if err != nil {
return apierrors.ToFolderErrorResponse(err)
}
@ -448,7 +454,7 @@ func (hs *HTTPServer) getFolderACMetadata(c *contextmodel.ReqContext, f *folder.
return metadata, nil
}
func (hs *HTTPServer) searchFolders(c *contextmodel.ReqContext) ([]dtos.FolderSearchHit, error) {
func (hs *HTTPServer) searchFolders(c *contextmodel.ReqContext, permission dashboardaccess.PermissionType) ([]dtos.FolderSearchHit, error) {
searchQuery := search.Query{
SignedInUser: c.SignedInUser,
DashboardIds: make([]int64, 0),
@ -456,7 +462,7 @@ func (hs *HTTPServer) searchFolders(c *contextmodel.ReqContext) ([]dtos.FolderSe
Limit: c.QueryInt64("limit"),
OrgId: c.SignedInUser.GetOrgID(),
Type: "dash-folder",
Permission: dashboardaccess.PERMISSION_VIEW,
Permission: permission,
Page: c.QueryInt64("page"),
}
@ -494,6 +500,12 @@ type GetFoldersParams struct {
// in:query
// required:false
ParentUID string `json:"parentUid"`
// Set to `Edit` to return folders that the user can edit
// in:query
// required: false
// default:View
// Enum: Edit,View
Permission string `json:"permission"`
}
// swagger:parameters getFolderByUID

View File

@ -553,6 +553,7 @@ func (dr *DashboardServiceImpl) filterUserSharedDashboards(ctx context.Context,
userDashFolders, err := dr.folderService.GetFolders(ctx, folder.GetFoldersQuery{
UIDs: folderUIDs,
OrgID: user.GetOrgID(),
OrderByTitle: true,
SignedInUser: user,
})
if err != nil {

View File

@ -22,6 +22,7 @@ import (
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/guardian"
@ -298,12 +299,16 @@ func (s *Service) GetChildren(ctx context.Context, q *folder.GetChildrenQuery) (
return nil, err
}
canView, err := g.CanView()
guardianFunc := g.CanView
if q.Permission == dashboardaccess.PERMISSION_EDIT {
guardianFunc = g.CanEdit
}
hasAccess, err := guardianFunc()
if err != nil {
return nil, err
}
if !canView {
if !hasAccess {
return nil, dashboards.ErrFolderAccessDenied
}
@ -341,8 +346,19 @@ func (s *Service) GetChildren(ctx context.Context, q *folder.GetChildrenQuery) (
func (s *Service) getRootFolders(ctx context.Context, q *folder.GetChildrenQuery) ([]*folder.Folder, error) {
permissions := q.SignedInUser.GetPermissions()
folderPermissions := permissions[dashboards.ActionFoldersRead]
folderPermissions = append(folderPermissions, permissions[dashboards.ActionDashboardsRead]...)
var folderPermissions []string
if q.Permission == dashboardaccess.PERMISSION_EDIT {
folderPermissions = permissions[dashboards.ActionFoldersWrite]
folderPermissions = append(folderPermissions, permissions[dashboards.ActionDashboardsWrite]...)
} else {
folderPermissions = permissions[dashboards.ActionFoldersRead]
folderPermissions = append(folderPermissions, permissions[dashboards.ActionDashboardsRead]...)
}
if len(folderPermissions) == 0 && !q.SignedInUser.GetIsGrafanaAdmin() {
return nil, nil
}
q.FolderUIDs = make([]string, 0, len(folderPermissions))
for _, p := range folderPermissions {
if p == dashboards.ScopeFoldersAll {
@ -401,12 +417,12 @@ func (s *Service) getRootFolders(ctx context.Context, q *folder.GetChildrenQuery
// GetSharedWithMe returns folders available to user, which cannot be accessed from the root folders
func (s *Service) GetSharedWithMe(ctx context.Context, q *folder.GetChildrenQuery) ([]*folder.Folder, error) {
start := time.Now()
availableNonRootFolders, err := s.getAvailableNonRootFolders(ctx, q.OrgID, q.SignedInUser)
availableNonRootFolders, err := s.getAvailableNonRootFolders(ctx, q)
if err != nil {
s.metrics.sharedWithMeFetchFoldersRequestsDuration.WithLabelValues("failure").Observe(time.Since(start).Seconds())
return nil, folder.ErrInternal.Errorf("failed to fetch subfolders to which the user has explicit access: %w", err)
}
rootFolders, err := s.GetChildren(ctx, &folder.GetChildrenQuery{UID: "", OrgID: q.OrgID, SignedInUser: q.SignedInUser})
rootFolders, err := s.GetChildren(ctx, &folder.GetChildrenQuery{UID: "", OrgID: q.OrgID, SignedInUser: q.SignedInUser, Permission: q.Permission})
if err != nil {
s.metrics.sharedWithMeFetchFoldersRequestsDuration.WithLabelValues("failure").Observe(time.Since(start).Seconds())
return nil, folder.ErrInternal.Errorf("failed to fetch root folders to which the user has access: %w", err)
@ -417,10 +433,21 @@ func (s *Service) GetSharedWithMe(ctx context.Context, q *folder.GetChildrenQuer
return availableNonRootFolders, nil
}
func (s *Service) getAvailableNonRootFolders(ctx context.Context, orgID int64, user identity.Requester) ([]*folder.Folder, error) {
permissions := user.GetPermissions()
folderPermissions := permissions[dashboards.ActionFoldersRead]
folderPermissions = append(folderPermissions, permissions[dashboards.ActionDashboardsRead]...)
func (s *Service) getAvailableNonRootFolders(ctx context.Context, q *folder.GetChildrenQuery) ([]*folder.Folder, error) {
permissions := q.SignedInUser.GetPermissions()
var folderPermissions []string
if q.Permission == dashboardaccess.PERMISSION_EDIT {
folderPermissions = permissions[dashboards.ActionFoldersWrite]
folderPermissions = append(folderPermissions, permissions[dashboards.ActionDashboardsWrite]...)
} else {
folderPermissions = permissions[dashboards.ActionFoldersRead]
folderPermissions = append(folderPermissions, permissions[dashboards.ActionDashboardsRead]...)
}
if len(folderPermissions) == 0 {
return nil, nil
}
nonRootFolders := make([]*folder.Folder, 0)
folderUids := make([]string, 0, len(folderPermissions))
for _, p := range folderPermissions {
@ -437,8 +464,9 @@ func (s *Service) getAvailableNonRootFolders(ctx context.Context, orgID int64, u
dashFolders, err := s.GetFolders(ctx, folder.GetFoldersQuery{
UIDs: folderUids,
OrgID: orgID,
SignedInUser: user,
OrgID: q.OrgID,
SignedInUser: q.SignedInUser,
OrderByTitle: true,
WithFullpathUIDs: true,
})
if err != nil {

View File

@ -10,6 +10,7 @@ import (
"testing"
"time"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
@ -25,6 +26,7 @@ import (
"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
"github.com/grafana/grafana/pkg/services/dashboards/database"
dashboardservice "github.com/grafana/grafana/pkg/services/dashboards/service"
"github.com/grafana/grafana/pkg/services/featuremgmt"
@ -1798,6 +1800,247 @@ func TestFolderServiceGetFolders(t *testing.T) {
})
}
// TODO replace it with an API test under /pkg/tests/api/folders
// whenever the golang client with get updated to allow filtering child folders by permission
func TestGetChildrenFilterByPermission(t *testing.T) {
db := sqlstore.InitTestDB(t)
signedInAdminUser := user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{
orgID: {
dashboards.ActionFoldersCreate: {},
dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersAll},
dashboards.ActionFoldersRead: {dashboards.ScopeFoldersAll},
},
}}
quotaService := quotatest.New(false, nil)
folderStore := ProvideDashboardFolderStore(db)
cfg := setting.NewCfg()
featuresFlagOff := featuremgmt.WithFeatures()
dashStore, err := database.ProvideDashboardStore(db, db.Cfg, featuresFlagOff, tagimpl.ProvideService(db), quotaService)
require.NoError(t, err)
nestedFolderStore := ProvideStore(db, db.Cfg)
b := bus.ProvideBus(tracing.InitializeTracerForTest())
ac := acimpl.ProvideAccessControl(cfg)
features := featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)
folderSvcOn := &Service{
cfg: cfg,
log: log.New("test-folder-service"),
dashboardStore: dashStore,
dashboardFolderStore: folderStore,
store: nestedFolderStore,
features: features,
bus: b,
db: db,
accessControl: ac,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
}
origGuardian := guardian.New
fakeGuardian := &guardian.FakeDashboardGuardian{
CanSaveValue: true,
CanEditUIDs: []string{},
CanViewUIDs: []string{},
}
guardian.MockDashboardGuardian(fakeGuardian)
t.Cleanup(func() {
guardian.New = origGuardian
})
viewer := user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{
orgID: {
dashboards.ActionFoldersRead: {},
dashboards.ActionFoldersWrite: {},
},
}}
// no view permission
// |_ subfolder under no view permission with view permission
// |_ subfolder under no view permission with view permissionn and with edit permission
// with edit permission
// |_ subfolder under with edit permission
// no edit permission
// |_ subfolder under no edit permission
// |_ subfolder under no edit permission with edit permission
noViewPermission, err := folderSvcOn.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
ParentUID: "",
Title: "no view permission",
SignedInUser: &signedInAdminUser,
})
require.NoError(t, err)
f, err := folderSvcOn.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
ParentUID: noViewPermission.UID,
Title: "subfolder under no view permission with view permission",
SignedInUser: &signedInAdminUser,
})
viewer.Permissions[orgID][dashboards.ActionFoldersRead] = append(viewer.Permissions[orgID][dashboards.ActionFoldersRead], dashboards.ScopeFoldersProvider.GetResourceScopeUID(f.UID))
fakeGuardian.CanViewUIDs = append(fakeGuardian.CanViewUIDs, f.UID)
require.NoError(t, err)
f, err = folderSvcOn.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
ParentUID: noViewPermission.UID,
Title: "subfolder under no view permission with view permission and with edit permission",
SignedInUser: &signedInAdminUser,
})
require.NoError(t, err)
viewer.Permissions[orgID][dashboards.ActionFoldersRead] = append(viewer.Permissions[orgID][dashboards.ActionFoldersRead], dashboards.ScopeFoldersProvider.GetResourceScopeUID(f.UID))
fakeGuardian.CanViewUIDs = append(fakeGuardian.CanViewUIDs, f.UID)
viewer.Permissions[orgID][dashboards.ActionFoldersWrite] = append(viewer.Permissions[orgID][dashboards.ActionFoldersWrite], dashboards.ScopeFoldersProvider.GetResourceScopeUID(f.UID))
fakeGuardian.CanEditUIDs = append(fakeGuardian.CanEditUIDs, f.UID)
withEditPermission, err := folderSvcOn.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
ParentUID: "",
Title: "with edit permission",
SignedInUser: &signedInAdminUser,
})
require.NoError(t, err)
viewer.Permissions[orgID][dashboards.ActionFoldersRead] = append(viewer.Permissions[orgID][dashboards.ActionFoldersRead], dashboards.ScopeFoldersProvider.GetResourceScopeUID(withEditPermission.UID))
fakeGuardian.CanViewUIDs = append(fakeGuardian.CanViewUIDs, withEditPermission.UID)
viewer.Permissions[orgID][dashboards.ActionFoldersWrite] = append(viewer.Permissions[orgID][dashboards.ActionFoldersWrite], dashboards.ScopeFoldersProvider.GetResourceScopeUID(withEditPermission.UID))
fakeGuardian.CanEditUIDs = append(fakeGuardian.CanEditUIDs, withEditPermission.UID)
_, err = folderSvcOn.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
ParentUID: withEditPermission.UID,
Title: "subfolder under with edit permission",
SignedInUser: &signedInAdminUser,
})
require.NoError(t, err)
noEditPermission, err := folderSvcOn.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
ParentUID: "",
Title: "no edit permission",
SignedInUser: &signedInAdminUser,
})
require.NoError(t, err)
viewer.Permissions[orgID][dashboards.ActionFoldersRead] = append(viewer.Permissions[orgID][dashboards.ActionFoldersRead], dashboards.ScopeFoldersProvider.GetResourceScopeUID(noEditPermission.UID))
fakeGuardian.CanViewUIDs = append(fakeGuardian.CanViewUIDs, noEditPermission.UID)
_, err = folderSvcOn.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
ParentUID: noEditPermission.UID,
Title: "subfolder under no edit permission",
SignedInUser: &signedInAdminUser,
})
require.NoError(t, err)
f, err = folderSvcOn.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
ParentUID: noEditPermission.UID,
Title: "subfolder under no edit permission with edit permission",
SignedInUser: &signedInAdminUser,
})
require.NoError(t, err)
viewer.Permissions[orgID][dashboards.ActionFoldersWrite] = append(viewer.Permissions[orgID][dashboards.ActionFoldersWrite], dashboards.ScopeFoldersProvider.GetResourceScopeUID(f.UID))
fakeGuardian.CanEditUIDs = append(fakeGuardian.CanEditUIDs, f.UID)
testCases := []struct {
name string
q folder.GetChildrenQuery
expectedErr error
expectedFolders []string
}{
{
name: "should return root folders with view permission",
q: folder.GetChildrenQuery{
OrgID: orgID,
SignedInUser: &viewer,
},
expectedFolders: []string{
"Shared with me",
"no edit permission",
"with edit permission"},
},
{
name: "should return subfolders with view permission",
q: folder.GetChildrenQuery{
OrgID: orgID,
SignedInUser: &viewer,
UID: noEditPermission.UID,
},
expectedFolders: []string{
"subfolder under no edit permission",
"subfolder under no edit permission with edit permission"},
},
{
name: "should return shared with me folders with view permission",
q: folder.GetChildrenQuery{
OrgID: orgID,
SignedInUser: &viewer,
UID: folder.SharedWithMeFolderUID,
},
expectedFolders: []string{
"subfolder under no view permission with view permission",
"subfolder under no view permission with view permission and with edit permission"},
},
{
name: "should return root folders with edit permission",
q: folder.GetChildrenQuery{
OrgID: orgID,
SignedInUser: &viewer,
Permission: dashboardaccess.PERMISSION_EDIT,
},
expectedFolders: []string{
"Shared with me",
"with edit permission"},
},
{
name: "should fail returning subfolders with edit permission when parent folder has no edit permission",
q: folder.GetChildrenQuery{
OrgID: orgID,
SignedInUser: &viewer,
Permission: dashboardaccess.PERMISSION_EDIT,
UID: noEditPermission.UID,
},
expectedErr: dashboards.ErrFolderAccessDenied,
},
{
name: "should return shared with me folders with edit permission",
q: folder.GetChildrenQuery{
OrgID: orgID,
SignedInUser: &viewer,
Permission: dashboardaccess.PERMISSION_EDIT,
UID: folder.SharedWithMeFolderUID,
},
expectedFolders: []string{
"subfolder under no edit permission with edit permission",
"subfolder under no view permission with view permission and with edit permission",
},
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
folders, err := folderSvcOn.GetChildren(context.Background(), &tc.q)
if tc.expectedErr != nil {
require.Error(t, err)
require.Equal(t, tc.expectedErr, err)
} else {
require.NoError(t, err)
actual := make([]string, 0, len(folders))
for _, f := range folders {
actual = append(actual, f.Title)
}
if cmp.Diff(tc.expectedFolders, actual) != "" {
t.Fatalf("unexpected folders: %s", cmp.Diff(tc.expectedFolders, actual))
}
}
})
}
}
func TestSupportBundle(t *testing.T) {
f := func(uid, parent string) *folder.Folder { return &folder.Folder{UID: uid, ParentUID: parent} }
for _, tc := range []struct {

View File

@ -477,6 +477,10 @@ func (ss *sqlStore) GetFolders(ctx context.Context, q getFoldersQuery) ([]*folde
}
if len(q.ancestorUIDs) == 0 {
if q.OrderByTitle {
s.WriteString(` ORDER BY f0.title ASC`)
}
err := sess.SQL(s.String(), args...).Find(&partialFolders)
if err != nil {
return err
@ -488,6 +492,9 @@ func (ss *sqlStore) GetFolders(ctx context.Context, q getFoldersQuery) ([]*folde
// filter out folders if they are not in the subtree of the given ancestor folders
if err := batch(len(q.ancestorUIDs), int(q.BatchSize), func(start2, end2 int) error {
s2, args2 := getAncestorsSQL(ss.db.GetDialect(), q.ancestorUIDs, start2, end2, s.String(), args)
if q.OrderByTitle {
s2 += " ORDER BY f0.title ASC"
}
err := sess.SQL(s2, args2...).Find(&partialFolders)
if err != nil {
return err

View File

@ -7,6 +7,7 @@ import (
"github.com/grafana/grafana/pkg/infra/metrics"
"github.com/grafana/grafana/pkg/infra/slugify"
"github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util/errutil"
)
@ -163,6 +164,10 @@ type GetFoldersQuery struct {
WithFullpathUIDs bool
BatchSize uint64
// OrderByTitle is used to sort the folders by title
// Set to true when ordering is meaningful (used for listing folders)
// otherwise better to keep it false since ordering can have a performance impact
OrderByTitle bool
SignedInUser identity.Requester `json:"-"`
}
@ -185,6 +190,9 @@ type GetChildrenQuery struct {
Limit int64
Page int64
// Permission to filter by
Permission dashboardaccess.PermissionType
SignedInUser identity.Requester `json:"-"`
// array of folder uids to filter by

View File

@ -62,13 +62,21 @@ type FakeDashboardGuardian struct {
CanViewValue bool
CanAdminValue bool
CanViewUIDs []string
CanEditUIDs []string
CanSaveUIDs []string
}
func (g *FakeDashboardGuardian) CanSave() (bool, error) {
if g.CanSaveUIDs != nil {
return slices.Contains(g.CanSaveUIDs, g.DashUID), nil
}
return g.CanSaveValue, nil
}
func (g *FakeDashboardGuardian) CanEdit() (bool, error) {
if g.CanEditUIDs != nil {
return slices.Contains(g.CanEditUIDs, g.DashUID), nil
}
return g.CanEditValue, nil
}

View File

@ -4316,6 +4316,17 @@
"description": "The parent folder UID",
"name": "parentUid",
"in": "query"
},
{
"enum": [
"Edit",
"View"
],
"type": "string",
"default": "View",
"description": "Set to `Edit` to return folders that the user can edit",
"name": "permission",
"in": "query"
}
],
"responses": {

View File

@ -16848,6 +16848,19 @@
"schema": {
"type": "string"
}
},
{
"description": "Set to `Edit` to return folders that the user can edit",
"in": "query",
"name": "permission",
"schema": {
"default": "View",
"enum": [
"Edit",
"View"
],
"type": "string"
}
}
],
"responses": {