Folders: Improve folder move permission checks (#90588)
* check that a user doesn't have higher plugin access on the destination folder than they have on the source folder when moving folders * Update pkg/services/folder/folderimpl/folder_test.go --------- Co-authored-by: Jo <joao.guerreiro@grafana.com>
This commit is contained in:
Ieva
Jo
@@ -882,14 +882,7 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol
|
||||
}
|
||||
|
||||
// Check that the user is allowed to move the folder to the destination folder
|
||||
var evaluator accesscontrol.Evaluator
|
||||
if cmd.NewParentUID != "" {
|
||||
evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(cmd.NewParentUID))
|
||||
} else {
|
||||
// Evaluate folder creation permission when moving folder to the root level
|
||||
evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersCreate)
|
||||
}
|
||||
hasAccess, evalErr := s.accessControl.Evaluate(ctx, cmd.SignedInUser, evaluator)
|
||||
hasAccess, evalErr := s.canMove(ctx, cmd)
|
||||
if evalErr != nil {
|
||||
return nil, evalErr
|
||||
}
|
||||
@@ -955,6 +948,68 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol
|
||||
return f, nil
|
||||
}
|
||||
|
||||
func (s *Service) canMove(ctx context.Context, cmd *folder.MoveFolderCommand) (bool, error) {
|
||||
// Check that the user is allowed to move the folder to the destination folder
|
||||
var evaluator accesscontrol.Evaluator
|
||||
parentUID := cmd.NewParentUID
|
||||
if parentUID != "" {
|
||||
evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(parentUID))
|
||||
} else {
|
||||
// Evaluate folder creation permission when moving folder to the root level
|
||||
evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersCreate)
|
||||
parentUID = folder.GeneralFolderUID
|
||||
}
|
||||
if hasAccess, err := s.accessControl.Evaluate(ctx, cmd.SignedInUser, evaluator); err != nil {
|
||||
return false, err
|
||||
} else if !hasAccess {
|
||||
return false, dashboards.ErrMoveAccessDenied.Errorf("user does not have permissions to move a folder to folder with UID %s", parentUID)
|
||||
}
|
||||
|
||||
// Check that the user would not be elevating their permissions by moving a folder to the destination folder
|
||||
// This is needed for plugins, as different folders can have different plugin configs
|
||||
// We do this by checking that there are no permissions that user has on the destination parent folder but not on the source folder
|
||||
// We also need to look at the folder tree for the destination folder, as folder permissions are inherited
|
||||
newFolderAndParentUIDs, err := s.getFolderAndParentUIDScopes(ctx, parentUID, cmd.OrgID)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
||||
permissions := cmd.SignedInUser.GetPermissions()
|
||||
var evaluators []accesscontrol.Evaluator
|
||||
currentFolderScope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(cmd.UID)
|
||||
for action, scopes := range permissions {
|
||||
for _, scope := range newFolderAndParentUIDs {
|
||||
if slices.Contains(scopes, scope) {
|
||||
evaluators = append(evaluators, accesscontrol.EvalPermission(action, currentFolderScope))
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if hasAccess, err := s.accessControl.Evaluate(ctx, cmd.SignedInUser, accesscontrol.EvalAll(evaluators...)); err != nil {
|
||||
return false, err
|
||||
} else if !hasAccess {
|
||||
return false, dashboards.ErrFolderAccessEscalation.Errorf("user cannot move a folder to another folder where they have higher permissions")
|
||||
}
|
||||
return true, nil
|
||||
}
|
||||
|
||||
func (s *Service) getFolderAndParentUIDScopes(ctx context.Context, folderUID string, orgID int64) ([]string, error) {
|
||||
folderAndParentUIDScopes := []string{dashboards.ScopeFoldersProvider.GetResourceScopeUID(folderUID)}
|
||||
if folderUID == folder.GeneralFolderUID {
|
||||
return folderAndParentUIDScopes, nil
|
||||
}
|
||||
folderParents, err := s.store.GetParents(ctx, folder.GetParentsQuery{UID: folderUID, OrgID: orgID})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
for _, newParent := range folderParents {
|
||||
scope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(newParent.UID)
|
||||
folderAndParentUIDScopes = append(folderAndParentUIDScopes, scope)
|
||||
}
|
||||
return folderAndParentUIDScopes, nil
|
||||
}
|
||||
|
||||
// nestedFolderDelete inspects the folder referenced by the cmd argument, deletes all the entries for
|
||||
// its descendant folders (folders which are nested within it either directly or indirectly) from
|
||||
// the folder store and returns the UIDs for all its descendants.
|
||||
|
||||
@@ -41,6 +41,7 @@ import (
|
||||
"github.com/grafana/grafana/pkg/services/librarypanels"
|
||||
"github.com/grafana/grafana/pkg/services/ngalert/models"
|
||||
ngstore "github.com/grafana/grafana/pkg/services/ngalert/store"
|
||||
"github.com/grafana/grafana/pkg/services/org"
|
||||
"github.com/grafana/grafana/pkg/services/quota/quotatest"
|
||||
"github.com/grafana/grafana/pkg/services/sqlstore"
|
||||
"github.com/grafana/grafana/pkg/services/store/entity"
|
||||
@@ -1144,7 +1145,7 @@ func TestNestedFolderService(t *testing.T) {
|
||||
features := featuremgmt.WithFeatures("nestedFolders")
|
||||
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, features, acimpl.ProvideAccessControl(features, zanzana.NewNoopClient()), dbtest.NewFakeDB())
|
||||
_, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: nestedFolderUser})
|
||||
require.ErrorIs(t, err, dashboards.ErrFolderAccessDenied)
|
||||
require.ErrorIs(t, err, dashboards.ErrMoveAccessDenied)
|
||||
})
|
||||
|
||||
t.Run("move with the right permissions succeeds", func(t *testing.T) {
|
||||
@@ -1160,14 +1161,14 @@ func TestNestedFolderService(t *testing.T) {
|
||||
}
|
||||
|
||||
nestedFolderUser := &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}}
|
||||
nestedFolderUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("newFolder")}}
|
||||
nestedFolderUser.Permissions[orgID] = map[string][]string{
|
||||
dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("myFolder"), dashboards.ScopeFoldersProvider.GetResourceScopeUID("newFolder")},
|
||||
}
|
||||
|
||||
features := featuremgmt.WithFeatures("nestedFolders")
|
||||
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, features, acimpl.ProvideAccessControl(features, zanzana.NewNoopClient()), dbtest.NewFakeDB())
|
||||
_, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: nestedFolderUser})
|
||||
require.NoError(t, err)
|
||||
// the folder is set inside InTransaction() but the fake one is called
|
||||
// require.NotNil(t, f)
|
||||
})
|
||||
|
||||
t.Run("cannot move the k6 folder even when has permissions to move folders", func(t *testing.T) {
|
||||
@@ -2190,6 +2191,161 @@ func TestGetChildrenFilterByPermission(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestIntegration_canMove(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("skipping integration test")
|
||||
}
|
||||
|
||||
dashStore := &dashboards.FakeDashboardStore{}
|
||||
dashboardFolderStore := foldertest.NewFakeFolderStore(t)
|
||||
|
||||
db, cfg := sqlstore.InitTestDB(t)
|
||||
folderStore := ProvideStore(db)
|
||||
orgID := CreateOrg(t, db, cfg)
|
||||
|
||||
adminUsr := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleAdmin}
|
||||
|
||||
// Set up source folder and a source folder parent
|
||||
sourceParent, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{
|
||||
UID: "source-parent",
|
||||
OrgID: orgID,
|
||||
Title: "Source parent",
|
||||
SignedInUser: adminUsr,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
sourceFolder, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{
|
||||
UID: "source",
|
||||
OrgID: orgID,
|
||||
Title: "Source",
|
||||
ParentUID: sourceParent.UID,
|
||||
SignedInUser: adminUsr,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Set up destination folder and destination folder parent
|
||||
destParent, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{
|
||||
UID: "destination-parent",
|
||||
OrgID: orgID,
|
||||
Title: "Destination parent",
|
||||
SignedInUser: adminUsr,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
destFolder, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{
|
||||
UID: "destination",
|
||||
OrgID: orgID,
|
||||
Title: "Destination",
|
||||
ParentUID: destParent.UID,
|
||||
SignedInUser: adminUsr,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
features := featuremgmt.WithFeatures("nestedFolders")
|
||||
folderSvc := setup(t, dashStore, dashboardFolderStore, folderStore, features, acimpl.ProvideAccessControl(features, zanzana.NewNoopClient()), dbtest.NewFakeDB())
|
||||
|
||||
testCases := []struct {
|
||||
description string
|
||||
destinationFolder string
|
||||
permissions map[string][]string
|
||||
expectedErr error
|
||||
}{
|
||||
{
|
||||
description: "can move a folder if has edit access to both folders",
|
||||
destinationFolder: destFolder.UID,
|
||||
permissions: map[string][]string{
|
||||
dashboards.ActionFoldersWrite: {
|
||||
dashboards.ScopeFoldersProvider.GetResourceScopeUID(sourceFolder.UID),
|
||||
dashboards.ScopeFoldersProvider.GetResourceScopeUID(destFolder.UID),
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
description: "can't move a folder if missing write access to the destination folder",
|
||||
destinationFolder: destFolder.UID,
|
||||
permissions: map[string][]string{
|
||||
dashboards.ActionFoldersWrite: {
|
||||
dashboards.ScopeFoldersProvider.GetResourceScopeUID(sourceFolder.UID),
|
||||
},
|
||||
dashboards.ActionFoldersRead: {
|
||||
dashboards.ScopeFoldersProvider.GetResourceScopeUID(destFolder.UID),
|
||||
},
|
||||
},
|
||||
expectedErr: dashboards.ErrMoveAccessDenied,
|
||||
},
|
||||
{
|
||||
description: "can't move a folder to the root if missing folder create permissions",
|
||||
destinationFolder: "",
|
||||
permissions: map[string][]string{
|
||||
dashboards.ActionFoldersWrite: {
|
||||
dashboards.ScopeFoldersProvider.GetResourceScopeUID(sourceFolder.UID),
|
||||
},
|
||||
},
|
||||
expectedErr: dashboards.ErrMoveAccessDenied,
|
||||
},
|
||||
{
|
||||
description: "can move a folder to the root with folder create permissions",
|
||||
destinationFolder: "",
|
||||
permissions: map[string][]string{
|
||||
dashboards.ActionFoldersCreate: {},
|
||||
},
|
||||
},
|
||||
{
|
||||
description: "can't move a folder to another folder where user has higher plugin permissions",
|
||||
destinationFolder: destFolder.UID,
|
||||
permissions: map[string][]string{
|
||||
dashboards.ActionFoldersWrite: {
|
||||
dashboards.ScopeFoldersProvider.GetResourceScopeUID(sourceFolder.UID),
|
||||
dashboards.ScopeFoldersProvider.GetResourceScopeUID(destFolder.UID),
|
||||
},
|
||||
"some_plugin:action": {
|
||||
dashboards.ScopeFoldersProvider.GetResourceScopeUID(destFolder.UID),
|
||||
},
|
||||
},
|
||||
expectedErr: dashboards.ErrFolderAccessEscalation,
|
||||
},
|
||||
{
|
||||
description: "can move a folder to another folder where user has lower permissions",
|
||||
destinationFolder: destFolder.UID,
|
||||
permissions: map[string][]string{
|
||||
dashboards.ActionFoldersWrite: {
|
||||
dashboards.ScopeFoldersProvider.GetResourceScopeUID(sourceFolder.UID),
|
||||
dashboards.ScopeFoldersProvider.GetResourceScopeUID(destFolder.UID),
|
||||
},
|
||||
"some_plugin:action": {
|
||||
dashboards.ScopeFoldersProvider.GetResourceScopeUID(sourceFolder.UID),
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
description: "can't move a folder to another folder where user has higher plugin permissions through inheritance",
|
||||
destinationFolder: destFolder.UID,
|
||||
permissions: map[string][]string{
|
||||
dashboards.ActionFoldersWrite: {
|
||||
dashboards.ScopeFoldersProvider.GetResourceScopeUID(sourceFolder.UID),
|
||||
dashboards.ScopeFoldersProvider.GetResourceScopeUID(destFolder.UID),
|
||||
},
|
||||
"some_plugin:action": {
|
||||
dashboards.ScopeFoldersProvider.GetResourceScopeUID(destParent.UID),
|
||||
},
|
||||
},
|
||||
expectedErr: dashboards.ErrFolderAccessEscalation,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
usr := &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}}
|
||||
usr.Permissions[orgID] = tc.permissions
|
||||
|
||||
t.Run(tc.description, func(t *testing.T) {
|
||||
_, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: sourceFolder.UID, NewParentUID: tc.destinationFolder, OrgID: orgID, SignedInUser: usr})
|
||||
if tc.expectedErr == nil {
|
||||
require.NoError(t, err)
|
||||
} else {
|
||||
require.ErrorIs(t, err, tc.expectedErr)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSupportBundle(t *testing.T) {
|
||||
f := func(uid, parent string) *folder.Folder { return &folder.Folder{UID: uid, ParentUID: parent} }
|
||||
for _, tc := range []struct {
|
||||
|
||||
Reference in New Issue
Block a user