diff --git a/pkg/api/accesscontrol.go b/pkg/api/accesscontrol.go index c2b4be1abee..eb4fadbd79b 100644 --- a/pkg/api/accesscontrol.go +++ b/pkg/api/accesscontrol.go @@ -9,6 +9,7 @@ import ( "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/libraryelements" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginaccesscontrol" @@ -390,7 +391,7 @@ func (hs *HTTPServer) declareFixedRoles() error { Role: ac.RoleDTO{ Name: "fixed:dashboards:creator", DisplayName: "Creator", - Description: "Create dashboard in general folder.", + Description: "Create dashboards under the root folder.", Group: "Dashboards", Permissions: []ac.Permission{ {Action: dashboards.ActionFoldersRead, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.GeneralFolderUID)}, @@ -434,10 +435,10 @@ func (hs *HTTPServer) declareFixedRoles() error { Role: ac.RoleDTO{ Name: "fixed:folders:creator", DisplayName: "Creator", - Description: "Create folders.", + Description: "Create folders under root level", Group: "Folders", Permissions: []ac.Permission{ - {Action: dashboards.ActionFoldersCreate}, + {Action: dashboards.ActionFoldersCreate, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID(folder.GeneralFolderUID)}, }, }, Grants: []string{"Editor"}, @@ -481,7 +482,7 @@ func (hs *HTTPServer) declareFixedRoles() error { Permissions: ac.ConcatPermissions( foldersReaderRole.Role.Permissions, []ac.Permission{ - {Action: dashboards.ActionFoldersCreate}, + {Action: dashboards.ActionFoldersCreate, Scope: dashboards.ScopeFoldersAll}, {Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll}, {Action: dashboards.ActionFoldersDelete, Scope: dashboards.ScopeFoldersAll}, {Action: dashboards.ActionDashboardsWrite, Scope: dashboards.ScopeFoldersAll}, @@ -498,7 +499,7 @@ func (hs *HTTPServer) declareFixedRoles() error { Role: ac.RoleDTO{ Name: "fixed:library.panels:creator", DisplayName: "Creator", - Description: "Create library panel in general folder.", + Description: "Create library panel under the root folder.", Group: "Library panels", Permissions: []ac.Permission{ {Action: dashboards.ActionFoldersRead, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.GeneralFolderUID)}, @@ -525,7 +526,7 @@ func (hs *HTTPServer) declareFixedRoles() error { Role: ac.RoleDTO{ Name: "fixed:library.panels:general.reader", DisplayName: "General reader", - Description: "Read all library panels in general folder.", + Description: "Read all library panels under the root folder.", Group: "Library panels", Permissions: []ac.Permission{ {Action: libraryelements.ActionLibraryPanelsRead, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.GeneralFolderUID)}, @@ -552,9 +553,9 @@ func (hs *HTTPServer) declareFixedRoles() error { libraryPanelsGeneralWriterRole := ac.RoleRegistration{ Role: ac.RoleDTO{ Name: "fixed:library.panels:general.writer", - DisplayName: "General writer", + DisplayName: "Root level writer", Group: "Library panels", - Description: "Create, read, write or delete all library panels and their permissions in the general folder.", + Description: "Create, read, write or delete all library panels and their permissions under the root folder.", Permissions: ac.ConcatPermissions(libraryPanelsGeneralReaderRole.Role.Permissions, []ac.Permission{ {Action: libraryelements.ActionLibraryPanelsWrite, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.GeneralFolderUID)}, {Action: libraryelements.ActionLibraryPanelsDelete, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID(ac.GeneralFolderUID)}, diff --git a/pkg/services/accesscontrol/resourcepermissions/service_test.go b/pkg/services/accesscontrol/resourcepermissions/service_test.go index 2abccafbb1c..1da32d62d3e 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/service_test.go @@ -251,7 +251,7 @@ func TestService_RegisterActionSets(t *testing.T) { }, { Action: "folders:edit", - Actions: []string{"folders:read", "dashboards:read", "folders:write", "dashboards:write"}, + Actions: []string{"folders:read", "dashboards:read", "folders:write", "dashboards:write", "folders:create"}, }, }, }, diff --git a/pkg/services/accesscontrol/resourcepermissions/store.go b/pkg/services/accesscontrol/resourcepermissions/store.go index 208aa5d5fbc..5e4b45b7442 100644 --- a/pkg/services/accesscontrol/resourcepermissions/store.go +++ b/pkg/services/accesscontrol/resourcepermissions/store.go @@ -3,6 +3,7 @@ package resourcepermissions import ( "context" "fmt" + "slices" "strings" "time" @@ -827,19 +828,24 @@ func (s *InMemoryActionSets) ExpandActionSetsWithFilter(permissions []accesscont } func (s *InMemoryActionSets) StoreActionSet(name string, actions []string) { - actionSet := &ActionSet{ - Action: name, - Actions: actions, + // To avoid backwards incompatible changes, we don't want to store these actions in the DB + // Once action sets are fully enabled, we can include dashboards.ActionFoldersCreate in the list of other folder edit/admin actions + // Tracked in https://github.com/grafana/identity-access-team/issues/794 + if name == "folders:edit" || name == "folders:admin" { + if !slices.Contains(s.actionSetToActions[name], dashboards.ActionFoldersCreate) { + actions = append(actions, dashboards.ActionFoldersCreate) + } } - s.actionSetToActions[actionSet.Action] = append(s.actionSetToActions[actionSet.Action], actions...) + + s.actionSetToActions[name] = append(s.actionSetToActions[name], actions...) for _, action := range actions { if _, ok := s.actionToActionSets[action]; !ok { s.actionToActionSets[action] = []string{} } - s.actionToActionSets[action] = append(s.actionToActionSets[action], actionSet.Action) + s.actionToActionSets[action] = append(s.actionToActionSets[action], name) } - s.log.Debug("stored action set", "action set name", actionSet.Action) + s.log.Debug("stored action set", "action set name", name) } // RegisterActionSets allow the caller to expand the existing action sets with additional permissions diff --git a/pkg/services/accesscontrol/resourcepermissions/store_test.go b/pkg/services/accesscontrol/resourcepermissions/store_test.go index ec22e388c4a..1c8cc91107d 100644 --- a/pkg/services/accesscontrol/resourcepermissions/store_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/store_test.go @@ -787,7 +787,7 @@ func TestStore_StoreActionSet(t *testing.T) { actionSetName := GetActionSetName(tt.resource, tt.action) actionSet := asService.ResolveActionSet(actionSetName) - require.Equal(t, tt.actions, actionSet) + require.Equal(t, append(tt.actions, "folders:create"), actionSet) }) } } @@ -947,6 +947,9 @@ func TestStore_RegisterActionSet(t *testing.T) { for _, expected := range tt.expectedActionSets { actions := asService.ResolveActionSet(expected.Action) + if expected.Action == "folders:edit" || expected.Action == "folders:admin" { + expected.Actions = append(expected.Actions, "folders:create") + } assert.ElementsMatch(t, expected.Actions, actions) } diff --git a/pkg/services/annotations/annotationsimpl/annotations_test.go b/pkg/services/annotations/annotationsimpl/annotations_test.go index 57c5e2135fb..8e8a7e92d06 100644 --- a/pkg/services/annotations/annotationsimpl/annotations_test.go +++ b/pkg/services/annotations/annotationsimpl/annotations_test.go @@ -188,8 +188,6 @@ func TestIntegrationAnnotationListingWithInheritedRBAC(t *testing.T) { permissions := []accesscontrol.Permission{ { Action: dashboards.ActionFoldersCreate, - }, { - Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll, }, } diff --git a/pkg/services/dashboards/database/database_folder_test.go b/pkg/services/dashboards/database/database_folder_test.go index 46496da30f0..ffbf6413c79 100644 --- a/pkg/services/dashboards/database/database_folder_test.go +++ b/pkg/services/dashboards/database/database_folder_test.go @@ -291,8 +291,6 @@ func TestIntegrationDashboardInheritedFolderRBAC(t *testing.T) { Permissions: map[int64]map[string][]string{u.OrgID: accesscontrol.GroupScopesByActionContext(context.Background(), []accesscontrol.Permission{ { Action: dashboards.ActionFoldersCreate, - }, { - Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll, }}), }, diff --git a/pkg/services/dashboards/database/database_test.go b/pkg/services/dashboards/database/database_test.go index 7cdf213fdb6..b36337db2d8 100644 --- a/pkg/services/dashboards/database/database_test.go +++ b/pkg/services/dashboards/database/database_test.go @@ -839,6 +839,7 @@ func TestIntegrationFindDashboardsByTitle(t *testing.T) { dashboards.ActionDashboardsRead: []string{dashboards.ScopeDashboardsAll}, dashboards.ActionFoldersRead: []string{dashboards.ScopeFoldersAll}, dashboards.ActionFoldersWrite: []string{dashboards.ScopeFoldersAll}, + dashboards.ActionFoldersCreate: []string{dashboards.ScopeFoldersAll}, }, }, } @@ -956,6 +957,7 @@ func TestIntegrationFindDashboardsByFolder(t *testing.T) { dashboards.ActionDashboardsRead: []string{dashboards.ScopeDashboardsAll}, dashboards.ActionFoldersRead: []string{dashboards.ScopeFoldersAll}, dashboards.ActionFoldersWrite: []string{dashboards.ScopeFoldersAll}, + dashboards.ActionFoldersCreate: []string{dashboards.ScopeFoldersAll}, }, }, } diff --git a/pkg/services/dashboards/errors.go b/pkg/services/dashboards/errors.go index dcabc17a9fb..6e1528124cf 100644 --- a/pkg/services/dashboards/errors.go +++ b/pkg/services/dashboards/errors.go @@ -123,15 +123,16 @@ var ( Status: "bad-request", } - ErrFolderNotFound = errors.New("folder not found") - ErrFolderVersionMismatch = errors.New("the folder has been changed by someone else") - ErrFolderTitleEmpty = errors.New("folder title cannot be empty") - ErrFolderWithSameUIDExists = errors.New("a folder/dashboard with the same uid already exists") - ErrFolderInvalidUID = errors.New("invalid uid for folder provided") - ErrFolderSameNameExists = errors.New("a folder with the same name already exists in the current location") - ErrFolderAccessDenied = errors.New("access denied to folder") - ErrMoveAccessDenied = errutil.Forbidden("folders.forbiddenMove", errutil.WithPublicMessage("Access denied to the destination folder")) - ErrFolderAccessEscalation = errutil.Forbidden("folders.accessEscalation", errutil.WithPublicMessage("Cannot move a folder to a folder where you have higher permissions")) + ErrFolderNotFound = errors.New("folder not found") + ErrFolderVersionMismatch = errors.New("the folder has been changed by someone else") + ErrFolderTitleEmpty = errors.New("folder title cannot be empty") + ErrFolderWithSameUIDExists = errors.New("a folder/dashboard with the same uid already exists") + ErrFolderInvalidUID = errors.New("invalid uid for folder provided") + ErrFolderSameNameExists = errors.New("a folder with the same name already exists in the current location") + ErrFolderAccessDenied = errors.New("access denied to folder") + ErrMoveAccessDenied = errutil.Forbidden("folders.forbiddenMove", errutil.WithPublicMessage("Access denied to the destination folder")) + ErrFolderAccessEscalation = errutil.Forbidden("folders.accessEscalation", errutil.WithPublicMessage("Cannot move a folder to a folder where you have higher permissions")) + ErrFolderCreationAccessDenied = errutil.Forbidden("folders.forbiddenCreation", errutil.WithPublicMessage("not enough permissions to create a folder in the selected location")) ) // DashboardErr represents a dashboard error. diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index d72b657d87c..773c3939dea 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -30,7 +30,7 @@ import ( var ( provisionerPermissions = []accesscontrol.Permission{ - {Action: dashboards.ActionFoldersCreate}, + {Action: dashboards.ActionFoldersCreate, Scope: dashboards.ScopeFoldersAll}, {Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll}, {Action: dashboards.ActionDashboardsCreate, Scope: dashboards.ScopeFoldersAll}, {Action: dashboards.ActionDashboardsWrite, Scope: dashboards.ScopeFoldersAll}, diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index 23e6ca82875..3ed5026d282 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -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 { diff --git a/pkg/services/folder/folderimpl/folder_test.go b/pkg/services/folder/folderimpl/folder_test.go index ffd792a2f6c..ee18314c129 100644 --- a/pkg/services/folder/folderimpl/folder_test.go +++ b/pkg/services/folder/folderimpl/folder_test.go @@ -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) diff --git a/pkg/services/libraryelements/libraryelements_test.go b/pkg/services/libraryelements/libraryelements_test.go index 9517e9adf94..3b3b3d77fd5 100644 --- a/pkg/services/libraryelements/libraryelements_test.go +++ b/pkg/services/libraryelements/libraryelements_test.go @@ -323,7 +323,7 @@ func createFolder(t *testing.T, sc scenarioContext, title string) *folder.Folder features := featuremgmt.WithFeatures() cfg := setting.NewCfg() - ac := actest.FakeAccessControl{} + ac := actest.FakeAccessControl{ExpectedEvaluate: true} quotaService := quotatest.New(false, nil) dashboardStore, err := database.ProvideDashboardStore(sc.replStore, cfg, features, tagimpl.ProvideService(sc.sqlStore), quotaService) require.NoError(t, err) @@ -428,7 +428,7 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo LastSeenAt: time.Now(), // Allow user to create folders Permissions: map[int64]map[string][]string{ - 1: {dashboards.ActionFoldersCreate: {}}, + 1: {dashboards.ActionFoldersCreate: {dashboards.ScopeFoldersAll}}, }, } req := &http.Request{ diff --git a/pkg/services/ngalert/api/api_provisioning_test.go b/pkg/services/ngalert/api/api_provisioning_test.go index 763d88e2204..5bc102cd90a 100644 --- a/pkg/services/ngalert/api/api_provisioning_test.go +++ b/pkg/services/ngalert/api/api_provisioning_test.go @@ -1819,7 +1819,7 @@ func createTestEnv(t *testing.T, testConfig string) testEnvironment { require.NoError(t, err) folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore) - folderService := folderimpl.ProvideService(actest.FakeAccessControl{}, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sqlStore, featuremgmt.WithFeatures(), supportbundlestest.NewFakeBundleService(), nil) + folderService := folderimpl.ProvideService(actest.FakeAccessControl{ExpectedEvaluate: true}, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sqlStore, featuremgmt.WithFeatures(), supportbundlestest.NewFakeBundleService(), nil) store := store.DBstore{ Logger: log, SQLStore: sqlStore, diff --git a/pkg/services/ngalert/provisioning/alert_rules_test.go b/pkg/services/ngalert/provisioning/alert_rules_test.go index 30b7a307723..88e95d24c37 100644 --- a/pkg/services/ngalert/provisioning/alert_rules_test.go +++ b/pkg/services/ngalert/provisioning/alert_rules_test.go @@ -1486,7 +1486,7 @@ func TestProvisiongWithFullpath(t *testing.T) { signedInUser := user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{ orgID: { - dashboards.ActionFoldersCreate: {}, + dashboards.ActionFoldersCreate: {dashboards.ScopeFoldersAll}, dashboards.ActionFoldersRead: {dashboards.ScopeFoldersAll}, dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersAll}}, }} diff --git a/pkg/services/ngalert/store/alert_rule_test.go b/pkg/services/ngalert/store/alert_rule_test.go index 186cb3e85f6..a6ac6aaf0b7 100644 --- a/pkg/services/ngalert/store/alert_rule_test.go +++ b/pkg/services/ngalert/store/alert_rule_test.go @@ -1243,5 +1243,5 @@ func setupFolderService(t *testing.T, sqlStore db.ReplDB, cfg *setting.Cfg, feat folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore.DB()) _, dashboardStore := testutil.SetupDashboardService(t, sqlStore, folderStore, cfg) - return testutil.SetupFolderService(t, cfg, sqlStore.DB(), dashboardStore, folderStore, inProcBus, features, &actest.FakeAccessControl{}) + return testutil.SetupFolderService(t, cfg, sqlStore.DB(), dashboardStore, folderStore, inProcBus, features, &actest.FakeAccessControl{ExpectedEvaluate: true}) } diff --git a/pkg/services/ngalert/tests/util.go b/pkg/services/ngalert/tests/util.go index 5080e2e3d5d..eb2f74befe4 100644 --- a/pkg/services/ngalert/tests/util.go +++ b/pkg/services/ngalert/tests/util.go @@ -98,6 +98,9 @@ func CreateTestAlertRuleWithLabels(t testing.TB, ctx context.Context, dbstore *s OrgID: orgID, OrgRole: org.RoleAdmin, IsGrafanaAdmin: true, + Permissions: map[int64]map[string][]string{ + orgID: {dashboards.ActionFoldersCreate: {dashboards.ScopeFoldersAll}}, + }, } ctx = identity.WithRequester(ctx, user) diff --git a/pkg/services/sqlstore/permissions/dashboard_test.go b/pkg/services/sqlstore/permissions/dashboard_test.go index f92c0d9ce39..a9c29d04e64 100644 --- a/pkg/services/sqlstore/permissions/dashboard_test.go +++ b/pkg/services/sqlstore/permissions/dashboard_test.go @@ -15,7 +15,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/accesscontrol" - "github.com/grafana/grafana/pkg/services/accesscontrol/mock" + "github.com/grafana/grafana/pkg/services/accesscontrol/actest" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" "github.com/grafana/grafana/pkg/services/dashboards/database" @@ -822,7 +822,7 @@ func setupNestedTest(t *testing.T, usr *user.SignedInUser, perms []accesscontrol dashStore, err := database.ProvideDashboardStore(db, cfg, features, tagimpl.ProvideService(db), quotatest.New(false, nil)) require.NoError(t, err) - folderSvc := folderimpl.ProvideService(mock.New(), bus.ProvideBus(tracing.InitializeTracerForTest()), dashStore, folderimpl.ProvideDashboardFolderStore(db), db, features, supportbundlestest.NewFakeBundleService(), nil) + folderSvc := folderimpl.ProvideService(actest.FakeAccessControl{ExpectedEvaluate: true}, bus.ProvideBus(tracing.InitializeTracerForTest()), dashStore, folderimpl.ProvideDashboardFolderStore(db), db, features, supportbundlestest.NewFakeBundleService(), nil) // create parent folder parent, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{ diff --git a/pkg/services/sqlstore/permissions/dashboards_bench_test.go b/pkg/services/sqlstore/permissions/dashboards_bench_test.go index 26ee1fa05fb..362905c447e 100644 --- a/pkg/services/sqlstore/permissions/dashboards_bench_test.go +++ b/pkg/services/sqlstore/permissions/dashboards_bench_test.go @@ -37,9 +37,6 @@ func benchmarkDashboardPermissionFilter(b *testing.B, numUsers, numDashboards, n 1: accesscontrol.GroupScopesByActionContext(context.Background(), []accesscontrol.Permission{ { Action: dashboards.ActionFoldersCreate, - }, - { - Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll, }, }), diff --git a/public/app/features/browse-dashboards/permissions.ts b/public/app/features/browse-dashboards/permissions.ts index f93beba255e..f107500de11 100644 --- a/public/app/features/browse-dashboards/permissions.ts +++ b/public/app/features/browse-dashboards/permissions.ts @@ -6,12 +6,21 @@ function checkFolderPermission(action: AccessControlAction, folderDTO?: FolderDT return folderDTO ? contextSrv.hasPermissionInMetadata(action, folderDTO) : contextSrv.hasPermission(action); } +function checkCanCreateFolders(folderDTO?: FolderDTO) { + if (folderDTO && !config.featureToggles.nestedFolders) { + return false; + } + + return config.featureToggles.accessActionSets + ? checkFolderPermission(AccessControlAction.FoldersCreate, folderDTO) + : checkFolderPermission(AccessControlAction.FoldersCreate) && + checkFolderPermission(AccessControlAction.FoldersWrite, folderDTO); +} + export function getFolderPermissions(folderDTO?: FolderDTO) { // Can only create a folder if we have permissions and either we're at root or nestedFolders is enabled const canCreateDashboards = checkFolderPermission(AccessControlAction.DashboardsCreate, folderDTO); - const canCreateFolders = Boolean( - (!folderDTO || config.featureToggles.nestedFolders) && checkFolderPermission(AccessControlAction.FoldersCreate) - ); + const canCreateFolders = checkCanCreateFolders(folderDTO); const canDeleteFolders = checkFolderPermission(AccessControlAction.FoldersDelete, folderDTO); const canEditDashboards = checkFolderPermission(AccessControlAction.DashboardsWrite, folderDTO); const canEditFolders = checkFolderPermission(AccessControlAction.FoldersWrite, folderDTO);