Folders: Allow folder editors and admins to create subfolders without any additional permissions (#91215)

* separate permissions for root level folder creation and subfolder creation

* fix tests

* fix tests

* fix tests

* frontend fix

* Update pkg/api/accesscontrol.go

Co-authored-by: Eric Leijonmarck <eric.leijonmarck@gmail.com>

* fix frontend when action sets are disabled

---------

Co-authored-by: Eric Leijonmarck <eric.leijonmarck@gmail.com>
This commit is contained in:
Ieva
2024-08-01 16:20:38 +01:00
committed by GitHub
parent 85e2ea2488
commit 2e2ddc5c42
19 changed files with 178 additions and 66 deletions

View File

@@ -554,17 +554,31 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
if s.features.IsEnabled(ctx, featuremgmt.FlagNestedFolders) && cmd.ParentUID != "" {
// Check that the user is allowed to create a subfolder in this folder
evaluator := accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(cmd.ParentUID))
parentUIDScope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(cmd.ParentUID)
legacyEvaluator := accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, parentUIDScope)
newEvaluator := accesscontrol.EvalPermission(dashboards.ActionFoldersCreate, parentUIDScope)
evaluator := accesscontrol.EvalAny(legacyEvaluator, newEvaluator)
hasAccess, evalErr := s.accessControl.Evaluate(ctx, cmd.SignedInUser, evaluator)
if evalErr != nil {
return nil, evalErr
}
if !hasAccess {
return nil, dashboards.ErrFolderAccessDenied
return nil, dashboards.ErrFolderCreationAccessDenied.Errorf("user is missing the permission with action either folders:create or folders:write and scope %s or any of the parent folder scopes", parentUIDScope)
}
dashFolder.FolderUID = cmd.ParentUID
}
if cmd.ParentUID == "" {
evaluator := accesscontrol.EvalPermission(dashboards.ActionFoldersCreate, dashboards.ScopeFoldersProvider.GetResourceScopeUID(folder.GeneralFolderUID))
hasAccess, evalErr := s.accessControl.Evaluate(ctx, cmd.SignedInUser, evaluator)
if evalErr != nil {
return nil, evalErr
}
if !hasAccess {
return nil, dashboards.ErrFolderCreationAccessDenied.Errorf("user is missing the permission with action folders:create and scope folders:uid:general, which is required to create a folder under the root level")
}
}
if s.features.IsEnabled(ctx, featuremgmt.FlagNestedFolders) && cmd.UID == folder.SharedWithMeFolderUID {
return nil, folder.ErrBadRequest.Errorf("cannot create folder with UID %s", folder.SharedWithMeFolderUID)
}
@@ -953,10 +967,12 @@ func (s *Service) canMove(ctx context.Context, cmd *folder.MoveFolderCommand) (b
var evaluator accesscontrol.Evaluator
parentUID := cmd.NewParentUID
if parentUID != "" {
evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(parentUID))
legacyEvaluator := accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(cmd.NewParentUID))
newEvaluator := accesscontrol.EvalPermission(dashboards.ActionFoldersCreate, dashboards.ScopeFoldersProvider.GetResourceScopeUID(cmd.NewParentUID))
evaluator = accesscontrol.EvalAny(legacyEvaluator, newEvaluator)
} else {
// Evaluate folder creation permission when moving folder to the root level
evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersCreate)
evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersCreate, dashboards.ScopeFoldersProvider.GetResourceScopeUID(folder.GeneralFolderUID))
parentUID = folder.GeneralFolderUID
}
if hasAccess, err := s.accessControl.Evaluate(ctx, cmd.SignedInUser, evaluator); err != nil {

View File

@@ -53,7 +53,8 @@ import (
)
var orgID = int64(1)
var usr = &user.SignedInUser{UserID: 1, OrgID: orgID}
var usr = &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{orgID: {dashboards.ActionFoldersCreate: {dashboards.ScopeFoldersProvider.GetResourceScopeUID(folder.GeneralFolderUID)}}}}
var noPermUsr = &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}}
func TestIntegrationProvideFolderService(t *testing.T) {
if testing.Short() {
@@ -81,14 +82,11 @@ func TestIntegrationFolderService(t *testing.T) {
features := featuremgmt.WithFeatures()
ac := acmock.New().WithPermissions([]accesscontrol.Permission{
{Action: accesscontrol.ActionAlertingRuleDelete, Scope: dashboards.ScopeFoldersAll},
})
alertingStore := ngstore.DBstore{
SQLStore: db,
Cfg: cfg.UnifiedAlerting,
Logger: log.New("test-alerting-store"),
AccessControl: ac,
AccessControl: actest.FakeAccessControl{ExpectedEvaluate: true},
}
service := &Service{
@@ -121,7 +119,7 @@ func TestIntegrationFolderService(t *testing.T) {
_, err := service.Get(context.Background(), &folder.GetFolderQuery{
UID: &folderUID,
OrgID: orgID,
SignedInUser: usr,
SignedInUser: noPermUsr,
})
require.Equal(t, err, dashboards.ErrFolderAccessDenied)
})
@@ -130,7 +128,7 @@ func TestIntegrationFolderService(t *testing.T) {
_, err := service.Get(context.Background(), &folder.GetFolderQuery{
UID: &folderUID,
OrgID: orgID,
SignedInUser: usr,
SignedInUser: noPermUsr,
})
require.Equal(t, err, dashboards.ErrFolderAccessDenied)
})
@@ -141,9 +139,9 @@ func TestIntegrationFolderService(t *testing.T) {
OrgID: orgID,
Title: f.Title,
UID: folderUID,
SignedInUser: usr,
SignedInUser: noPermUsr,
})
require.Equal(t, err, dashboards.ErrFolderAccessDenied)
require.Error(t, err)
})
title := "Folder-TEST"
@@ -155,7 +153,7 @@ func TestIntegrationFolderService(t *testing.T) {
UID: folderUID,
OrgID: orgID,
NewTitle: &title,
SignedInUser: usr,
SignedInUser: noPermUsr,
})
require.Equal(t, err, dashboards.ErrFolderAccessDenied)
})
@@ -170,7 +168,7 @@ func TestIntegrationFolderService(t *testing.T) {
UID: folderUID,
OrgID: orgID,
ForceDeleteRules: false,
SignedInUser: usr,
SignedInUser: noPermUsr,
})
require.Error(t, err)
require.Equal(t, err, dashboards.ErrFolderAccessDenied)
@@ -906,11 +904,15 @@ func TestNestedFolderService(t *testing.T) {
db, _ := sqlstore.InitTestDB(t)
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, features, acimpl.ProvideAccessControl(features, zanzana.NewNoopClient()), db)
tempUser := &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}}
tempUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersCreate: {dashboards.ScopeFoldersProvider.GetResourceScopeUID(folder.GeneralFolderUID)}}
_, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
Title: dash.Title,
UID: dash.UID,
SignedInUser: usr,
SignedInUser: tempUser,
})
require.NoError(t, err)
require.True(t, nestedFolderStore.CreateCalled)
@@ -918,7 +920,7 @@ func TestNestedFolderService(t *testing.T) {
})
t.Run("with nested folder feature flag on", func(t *testing.T) {
t.Run("Should be able to create a nested folder under the root", func(t *testing.T) {
t.Run("Should be able to create a nested folder under the root with the right permissions", func(t *testing.T) {
g := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true})
t.Cleanup(func() {
@@ -938,19 +940,50 @@ func TestNestedFolderService(t *testing.T) {
nestedFolderStore := NewFakeStore()
features := featuremgmt.WithFeatures("nestedFolders")
tempUser := &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}}
tempUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersCreate: {dashboards.ScopeFoldersProvider.GetResourceScopeUID(folder.GeneralFolderUID)}}
db, _ := sqlstore.InitTestDB(t)
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, features, acimpl.ProvideAccessControl(features, zanzana.NewNoopClient()), db)
_, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
Title: dash.Title,
UID: dash.UID,
SignedInUser: usr,
SignedInUser: tempUser,
})
require.NoError(t, err)
// CreateFolder should also call the folder store's create method.
require.True(t, nestedFolderStore.CreateCalled)
})
t.Run("Should not be able to create a folder under the root with subfolder creation permissions", func(t *testing.T) {
g := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true})
t.Cleanup(func() {
guardian.New = g
})
// dashboard store commands that should be called.
dashStore := &dashboards.FakeDashboardStore{}
dashboardFolderStore := foldertest.NewFakeFolderStore(t)
nestedFolderStore := NewFakeStore()
features := featuremgmt.WithFeatures("nestedFolders")
tempUser := &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}}
tempUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("subfolder_uid")}}
db, _ := sqlstore.InitTestDB(t)
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, features, acimpl.ProvideAccessControl(features, zanzana.NewNoopClient()), db)
_, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
Title: "some_folder",
UID: "some_uid",
SignedInUser: tempUser,
})
require.ErrorIs(t, err, dashboards.ErrFolderCreationAccessDenied)
})
t.Run("Should not be able to create new folder under another folder without the right permissions", func(t *testing.T) {
g := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true})
@@ -979,7 +1012,7 @@ func TestNestedFolderService(t *testing.T) {
SignedInUser: tempUser,
ParentUID: "some_parent",
})
require.ErrorIs(t, err, dashboards.ErrFolderAccessDenied)
require.ErrorIs(t, err, dashboards.ErrFolderCreationAccessDenied)
})
t.Run("Should be able to create new folder under another folder with the right permissions", func(t *testing.T) {
@@ -1017,6 +1050,19 @@ func TestNestedFolderService(t *testing.T) {
})
require.NoError(t, err)
require.True(t, nestedFolderStore.CreateCalled)
// Parent write access check will eventually be replaced with scoped folder creation check
nestedFolderUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersCreate: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("some_parent")}}
_, err = folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
Title: dash.Title + "2",
UID: dash.UID + "2",
SignedInUser: nestedFolderUser,
ParentUID: "some_parent",
})
require.NoError(t, err)
require.True(t, nestedFolderStore.CreateCalled)
})
t.Run("create without UID, no error", func(t *testing.T) {
@@ -1169,6 +1215,13 @@ func TestNestedFolderService(t *testing.T) {
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)
// Parent write access check will eventually be replaced with scoped folder creation check
nestedFolderUser.Permissions[orgID] = map[string][]string{
dashboards.ActionFoldersCreate: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("myFolder"), dashboards.ScopeFoldersProvider.GetResourceScopeUID("newFolder2")},
}
_, err = folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder2", OrgID: orgID, SignedInUser: nestedFolderUser})
require.NoError(t, err)
})
t.Run("cannot move the k6 folder even when has permissions to move folders", func(t *testing.T) {
@@ -1215,7 +1268,7 @@ func TestNestedFolderService(t *testing.T) {
require.Error(t, err, dashboards.ErrFolderAccessDenied)
})
t.Run("move to the root folder with folder creation permissions succeeds", func(t *testing.T) {
t.Run("move to the root folder with root folder creation permissions succeeds", func(t *testing.T) {
dashStore := &dashboards.FakeDashboardStore{}
dashboardFolderStore := foldertest.NewFakeFolderStore(t)
@@ -1228,7 +1281,12 @@ 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.ActionFoldersCreate: {}}
nestedFolderUser.Permissions[orgID] = map[string][]string{
dashboards.ActionFoldersCreate: {
dashboards.ScopeFoldersProvider.GetResourceScopeUID(folder.GeneralFolderUID),
dashboards.ScopeFoldersProvider.GetResourceScopeUID("myFolder"),
},
}
features := featuremgmt.WithFeatures("nestedFolders")
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, features, acimpl.ProvideAccessControl(features, zanzana.NewNoopClient()), dbtest.NewFakeDB())
@@ -1238,6 +1296,21 @@ func TestNestedFolderService(t *testing.T) {
// require.NotNil(t, f)
})
t.Run("move to the root folder with only subfolder creation permissions fails", func(t *testing.T) {
dashStore := &dashboards.FakeDashboardStore{}
dashboardFolderStore := foldertest.NewFakeFolderStore(t)
nestedFolderStore := NewFakeStore()
nestedFolderUser := &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}}
nestedFolderUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersCreate: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("some_subfolder")}}
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: "", OrgID: orgID, SignedInUser: nestedFolderUser})
require.Error(t, err)
})
t.Run("move when parentUID in the current subtree returns error from nested folder service", func(t *testing.T) {
g := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true, CanViewValue: true})
@@ -1960,7 +2033,7 @@ func TestGetChildrenFilterByPermission(t *testing.T) {
signedInAdminUser := user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{
orgID: {
dashboards.ActionFoldersCreate: {},
dashboards.ActionFoldersCreate: {dashboards.ScopeFoldersAll},
dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersAll},
dashboards.ActionFoldersRead: {dashboards.ScopeFoldersAll},
},
@@ -2285,7 +2358,10 @@ func TestIntegration_canMove(t *testing.T) {
description: "can move a folder to the root with folder create permissions",
destinationFolder: "",
permissions: map[string][]string{
dashboards.ActionFoldersCreate: {},
dashboards.ActionFoldersCreate: {
dashboards.ScopeFoldersProvider.GetResourceScopeUID(folder.GeneralFolderUID),
dashboards.ScopeFoldersProvider.GetResourceScopeUID(sourceFolder.UID),
},
},
},
{
@@ -2418,6 +2494,8 @@ func CreateSubtreeInStore(t *testing.T, store store, service *Service, depth int
title := fmt.Sprintf("%sfolder-%d", prefix, i)
cmd.Title = title
cmd.UID = util.GenerateShortUID()
cmd.OrgID = orgID
cmd.SignedInUser = &user.SignedInUser{OrgID: orgID, Permissions: map[int64]map[string][]string{orgID: {dashboards.ActionFoldersCreate: {dashboards.ScopeFoldersAll}}}}
f, err := service.Create(context.Background(), &cmd)
require.NoError(t, err)