diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index 245542700b7..4b943d22dd3 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -34,7 +34,6 @@ import ( "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" "github.com/grafana/grafana/pkg/services/publicdashboards" "github.com/grafana/grafana/pkg/services/search/model" "github.com/grafana/grafana/pkg/services/search/sort" @@ -326,15 +325,8 @@ func (s *Service) GetLegacy(ctx context.Context, q *folder.GetFolderQuery) (*fol return nil, err } - // do not get guardian by the folder ID because it differs from the nested folder ID - // and the legacy folder ID has been associated with the permissions: - // use the folde UID instead that is the same for both - g, err := guardian.NewByFolder(ctx, f, f.OrgID, q.SignedInUser) - if err != nil { - return nil, err - } - - if canView, err := g.CanView(); err != nil || !canView { + evaluator := accesscontrol.EvalPermission(dashboards.ActionFoldersRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(f.UID)) + if canView, err := s.accessControl.Evaluate(ctx, q.SignedInUser, evaluator); err != nil || !canView { if err != nil { return nil, toFolderError(err) } @@ -419,17 +411,13 @@ func (s *Service) GetChildrenLegacy(ctx context.Context, q *folder.GetChildrenQu // we only need to check access to the folder // if the parent is accessible then the subfolders are accessible as well (due to inheritance) - g, err := guardian.NewByFolderUID(ctx, q.UID, q.OrgID, q.SignedInUser) - if err != nil { - return nil, err - } - - guardianFunc := g.CanView + folderScope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(q.UID) + evaluator := accesscontrol.EvalPermission(dashboards.ActionFoldersRead, folderScope) if q.Permission == dashboardaccess.PERMISSION_EDIT { - guardianFunc = g.CanEdit + evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, folderScope) } - hasAccess, err := guardianFunc() + hasAccess, err := s.accessControl.Evaluate(ctx, q.SignedInUser, evaluator) if err != nil { return nil, err } @@ -981,12 +969,8 @@ func (s *Service) DeleteLegacy(ctx context.Context, cmd *folder.DeleteFolderComm return folder.ErrBadRequest.Errorf("invalid orgID") } - guard, err := guardian.NewByFolderUID(ctx, cmd.UID, cmd.OrgID, cmd.SignedInUser) - if err != nil { - return err - } - - if canSave, err := guard.CanDelete(); err != nil || !canSave { + evaluator := accesscontrol.EvalPermission(dashboards.ActionFoldersDelete, dashboards.ScopeFoldersProvider.GetResourceScopeUID(cmd.UID)) + if canDelete, err := s.accessControl.Evaluate(ctx, cmd.SignedInUser, evaluator); err != nil || !canDelete { if err != nil { return toFolderError(err) } @@ -994,7 +978,7 @@ func (s *Service) DeleteLegacy(ctx context.Context, cmd *folder.DeleteFolderComm } folders := []string{cmd.UID} - err = s.db.InTransaction(ctx, func(ctx context.Context) error { + err := s.db.InTransaction(ctx, func(ctx context.Context) error { descendants, err := s.nestedFolderDelete(ctx, cmd) if err != nil { @@ -1402,27 +1386,24 @@ func (s *Service) buildSaveDashboardCommand(ctx context.Context, dto *dashboards return nil, err } - guard, err := getGuardianForSavePermissionCheck(ctx, dash, dto.User) - if err != nil { - return nil, err - } - + var evaluator accesscontrol.Evaluator + // Check write permission for existing dashboards, create permission for new dashboards if dash.ID == 0 { metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Folder).Inc() - // nolint:staticcheck - if canCreate, err := guard.CanCreate(dash.FolderID, dash.IsFolder); err != nil || !canCreate { - if err != nil { - return nil, err - } - return nil, dashboards.ErrDashboardUpdateAccessDenied + parentUID := dash.FolderUID + if parentUID == "" { + parentUID = folder.GeneralFolderUID } + evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersCreate, dashboards.ScopeFoldersProvider.GetResourceScopeUID(parentUID)) } else { - if canSave, err := guard.CanSave(); err != nil || !canSave { - if err != nil { - return nil, err - } - return nil, dashboards.ErrDashboardUpdateAccessDenied + evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(dash.UID)) + } + + if hasAccess, err := s.accessControl.Evaluate(ctx, dto.User, evaluator); err != nil || !hasAccess { + if err != nil { + return nil, err } + return nil, dashboards.ErrDashboardUpdateAccessDenied } var userID int64 @@ -1482,34 +1463,6 @@ func SplitFullpath(s string) []string { return result } -// getGuardianForSavePermissionCheck returns the guardian to be used for checking permission of dashboard -// It replaces deleted Dashboard.GetDashboardIdForSavePermissionCheck() -func getGuardianForSavePermissionCheck(ctx context.Context, d *dashboards.Dashboard, user identity.Requester) (guardian.DashboardGuardian, error) { - newDashboard := d.ID == 0 - - if newDashboard { - // if it's a new dashboard/folder check the parent folder permissions - metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Folder).Inc() - // nolint:staticcheck - guard, err := guardian.NewByFolder(ctx, &folder.Folder{ - ID: d.FolderID, // nolint:staticcheck - OrgID: d.OrgID, - }, d.OrgID, user) - if err != nil { - return nil, err - } - return guard, nil - } - guard, err := guardian.NewByFolder(ctx, &folder.Folder{ - UID: d.UID, - OrgID: d.OrgID, - }, d.OrgID, user) - if err != nil { - return nil, err - } - return guard, nil -} - func (s *Service) nestedFolderCreate(ctx context.Context, cmd *folder.CreateFolderCommand) (*folder.Folder, error) { ctx, span := s.tracer.Start(ctx, "folder.nestedFolderCreate") defer span.End() diff --git a/pkg/services/folder/folderimpl/folder_test.go b/pkg/services/folder/folderimpl/folder_test.go index 8665b079bcc..4c7e33e52c9 100644 --- a/pkg/services/folder/folderimpl/folder_test.go +++ b/pkg/services/folder/folderimpl/folder_test.go @@ -38,7 +38,6 @@ 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/libraryelements" "github.com/grafana/grafana/pkg/services/libraryelements/model" "github.com/grafana/grafana/pkg/services/librarypanels" @@ -108,7 +107,7 @@ func TestIntegrationFolderService(t *testing.T) { features: features, bus: bus.ProvideBus(tracing.InitializeTracerForTest()), db: db, - accessControl: acimpl.ProvideAccessControl(features), + accessControl: actest.FakeAccessControl{ExpectedEvaluate: true}, metrics: newFoldersMetrics(nil), registry: make(map[string]folder.RegistryService), tracer: tracing.InitializeTracerForTest(), @@ -117,10 +116,8 @@ func TestIntegrationFolderService(t *testing.T) { require.NoError(t, service.RegisterService(alertingStore)) t.Run("Given user has no permissions", func(t *testing.T) { - origNewGuardian := guardian.New - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{}) - folderUID := util.GenerateShortUID() + service.accessControl = actest.FakeAccessControl{ExpectedEvaluate: false} f := folder.NewFolder("Folder", "") f.UID = folderUID @@ -191,14 +188,11 @@ func TestIntegrationFolderService(t *testing.T) { }) t.Cleanup(func() { - guardian.New = origNewGuardian + service.accessControl = actest.FakeAccessControl{ExpectedEvaluate: true} }) }) t.Run("Given user has permission to save", func(t *testing.T) { - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true, CanViewValue: true}) - service.features = featuremgmt.WithFeatures() - t.Run("When creating folder should not return access denied error", func(t *testing.T) { dash := dashboards.NewDashboardFolder("Test-Folder") dash.ID = rand.Int63() @@ -301,9 +295,6 @@ func TestIntegrationFolderService(t *testing.T) { }) t.Run("Given user has permission to view", func(t *testing.T) { - origNewGuardian := guardian.New - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanViewValue: true}) - t.Run("When get folder by uid should return folder", func(t *testing.T) { expected := folder.NewFolder(util.GenerateShortUID(), "") expected.UID = util.GenerateShortUID() @@ -325,10 +316,6 @@ func TestIntegrationFolderService(t *testing.T) { require.Equal(t, folder.RootFolder, actual) require.NoError(t, err) }) - - t.Cleanup(func() { - guardian.New = origNewGuardian - }) }) t.Run("Should map errors correct", func(t *testing.T) { @@ -384,7 +371,7 @@ func TestIntegrationNestedFolderService(t *testing.T) { publicDashboardFakeService := publicdashboards.NewFakePublicDashboardServiceWrapper(t) b := bus.ProvideBus(tracing.InitializeTracerForTest()) - ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures()) + ac := actest.FakeAccessControl{ExpectedEvaluate: true} serviceWithFlagOn := &Service{ log: slog.New(logtest.NewTestHandler(t)).With("logger", "test-folder-service"), @@ -434,13 +421,6 @@ func TestIntegrationNestedFolderService(t *testing.T) { t.Run("Should get descendant counts", func(t *testing.T) { depth := 5 t.Run("With nested folder feature flag on", func(t *testing.T) { - origNewGuardian := guardian.New - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{ - CanSaveValue: true, - CanViewValue: true, - // CanEditValue is required to create library elements - CanEditValue: true, - }) publicDashboardFakeService.On("DeleteByDashboardUIDs", mock.Anything, mock.Anything, mock.Anything).Return(nil) dashSrv, err := dashboardservice.ProvideDashboardServiceImpl(cfg, dashStore, folderStore, featuresFlagOn, folderPermissions, ac, actest.FakeService{}, serviceWithFlagOn, nil, @@ -495,7 +475,6 @@ func TestIntegrationNestedFolderService(t *testing.T) { require.Equal(t, int64(2), m[entity.StandardKindLibraryPanel]) t.Cleanup(func() { - guardian.New = origNewGuardian for _, ancestor := range ancestors { err := serviceWithFlagOn.store.Delete(context.Background(), []string{ancestor.UID}, orgID) assert.NoError(t, err) @@ -522,14 +501,6 @@ func TestIntegrationNestedFolderService(t *testing.T) { publicDashboardService: publicDashboardFakeService, } - origNewGuardian := guardian.New - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{ - CanSaveValue: true, - CanViewValue: true, - // CanEditValue is required to create library elements - CanEditValue: true, - }) - publicDashboardFakeService.On("DeleteByDashboardUIDs", mock.Anything, mock.Anything, mock.Anything).Return(nil) dashSrv, err := dashboardservice.ProvideDashboardServiceImpl(cfg, dashStore, folderStore, featuresFlagOff, @@ -583,7 +554,6 @@ func TestIntegrationNestedFolderService(t *testing.T) { require.Equal(t, int64(1), m[entity.StandardKindLibraryPanel]) t.Cleanup(func() { - guardian.New = origNewGuardian for _, ancestor := range ancestors { err := serviceWithFlagOn.store.Delete(context.Background(), []string{ancestor.UID}, orgID) assert.NoError(t, err) @@ -604,6 +574,7 @@ func TestIntegrationNestedFolderService(t *testing.T) { metrics: newFoldersMetrics(nil), tracer: tracing.InitializeTracerForTest(), publicDashboardService: publicDashboardFakeService, + accessControl: actest.FakeAccessControl{ExpectedEvaluate: true}, } testCases := []struct { @@ -664,14 +635,6 @@ func TestIntegrationNestedFolderService(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - origNewGuardian := guardian.New - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{ - CanSaveValue: true, - CanViewValue: true, - // CanEditValue is required to create library elements - CanEditValue: true, - }) - dashStore, err := database.ProvideDashboardStore(db, cfg, tc.featuresFlag, tagimpl.ProvideService(db)) require.NoError(t, err) nestedFolderStore := ProvideStore(db) @@ -754,21 +717,12 @@ func TestIntegrationNestedFolderService(t *testing.T) { }) require.ErrorIs(t, err, tc.libPanelSubErr) } - t.Cleanup(func() { - guardian.New = origNewGuardian - }) }) } }) } func TestNestedFolderServiceFeatureToggle(t *testing.T) { - g := guardian.New - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) - t.Cleanup(func() { - guardian.New = g - }) - nestedFolderStore := folder.NewFakeStore() dashStore := dashboards.FakeDashboardStore{} @@ -785,7 +739,7 @@ func TestNestedFolderServiceFeatureToggle(t *testing.T) { dashboardStore: &dashStore, dashboardFolderStore: dashboardFolderStore, features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), - accessControl: acimpl.ProvideAccessControl(featuremgmt.WithFeatures()), + accessControl: actest.FakeAccessControl{ExpectedEvaluate: true}, metrics: newFoldersMetrics(nil), tracer: tracing.InitializeTracerForTest(), } @@ -799,12 +753,6 @@ func TestNestedFolderServiceFeatureToggle(t *testing.T) { } func TestFolderServiceDualWrite(t *testing.T) { - g := guardian.New - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) - t.Cleanup(func() { - guardian.New = g - }) - db, _ := sqlstore.InitTestDB(t) cfg := setting.NewCfg() features := featuremgmt.WithFeatures() @@ -822,7 +770,7 @@ func TestFolderServiceDualWrite(t *testing.T) { dashboardStore: dashStore, dashboardFolderStore: dashboardFolderStore, features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), - accessControl: acimpl.ProvideAccessControl(featuremgmt.WithFeatures()), + accessControl: actest.FakeAccessControl{ExpectedEvaluate: true}, metrics: newFoldersMetrics(nil), tracer: tracing.InitializeTracerForTest(), bus: bus.ProvideBus(tracing.InitializeTracerForTest()), @@ -865,12 +813,6 @@ func TestFolderServiceDualWrite(t *testing.T) { func TestNestedFolderService(t *testing.T) { t.Run("with feature flag unset", func(t *testing.T) { t.Run("Should create a folder in both dashboard and folders tables", func(t *testing.T) { - g := guardian.New - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) - t.Cleanup(func() { - guardian.New = g - }) - // dash is needed here because folderSvc.Create expects SaveDashboard to return it dash := dashboards.NewDashboardFolder("myFolder") dash.ID = rand.Int63() @@ -887,7 +829,7 @@ func TestNestedFolderService(t *testing.T) { features := featuremgmt.WithFeatures() db, _ := sqlstore.InitTestDB(t) - folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, features, acimpl.ProvideAccessControl(features), db) + folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, features, actest.FakeAccessControl{ExpectedEvaluate: true}, 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)}} @@ -905,12 +847,6 @@ 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 with the right permissions", func(t *testing.T) { - g := guardian.New - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) - t.Cleanup(func() { - guardian.New = g - }) - dash := dashboards.NewDashboardFolder("myFolder") dash.ID = rand.Int63() dash.UID = "some_uid" @@ -941,12 +877,6 @@ func TestNestedFolderService(t *testing.T) { }) 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{} @@ -969,12 +899,6 @@ func TestNestedFolderService(t *testing.T) { }) 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}) - t.Cleanup(func() { - guardian.New = g - }) - dash := dashboards.NewDashboardFolder("Test-Folder") dash.ID = rand.Int63() dash.UID = "some_uid" @@ -1000,12 +924,6 @@ func TestNestedFolderService(t *testing.T) { }) t.Run("Should be able to create new folder under another folder with the right permissions", func(t *testing.T) { - g := guardian.New - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) - t.Cleanup(func() { - guardian.New = g - }) - dash := dashboards.NewDashboardFolder("Test-Folder") dash.ID = rand.Int63() dash.UID = "some_uid" @@ -1019,7 +937,7 @@ func TestNestedFolderService(t *testing.T) { dashboardFolderStore.On("GetFolderByUID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).Return(&folder.Folder{}, nil) nestedFolderUser := &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}} - nestedFolderUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("some_parent")}} + nestedFolderUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersCreate: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("some_parent")}} nestedFolderStore := folder.NewFakeStore() db, _ := sqlstore.InitTestDB(t) @@ -1034,28 +952,9 @@ 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) { - 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{} dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*dashboards.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil) @@ -1080,12 +979,6 @@ func TestNestedFolderService(t *testing.T) { }) t.Run("create failed because of circular reference", func(t *testing.T) { - g := guardian.New - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) - t.Cleanup(func() { - guardian.New = g - }) - dashboardFolder := dashboards.NewDashboardFolder("myFolder") dashboardFolder.ID = rand.Int63() dashboardFolder.UID = "myFolder" @@ -1126,13 +1019,6 @@ func TestNestedFolderService(t *testing.T) { }) t.Run("create returns error from nested folder service", func(t *testing.T) { - // This test creates and deletes the dashboard, so needs some extra setup. - 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{} dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*dashboards.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil) @@ -1296,12 +1182,6 @@ func TestNestedFolderService(t *testing.T) { }) 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}) - t.Cleanup(func() { - guardian.New = g - }) - dashStore := &dashboards.FakeDashboardStore{} dashboardFolderStore := foldertest.NewFakeFolderStore(t) @@ -1318,12 +1198,6 @@ func TestNestedFolderService(t *testing.T) { }) t.Run("move when new parentUID depth + subTree height bypassed maximum depth returns error", func(t *testing.T) { - g := guardian.New - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true, CanViewValue: true}) - t.Cleanup(func() { - guardian.New = g - }) - dashStore := &dashboards.FakeDashboardStore{} dashboardFolderStore := foldertest.NewFakeFolderStore(t) @@ -1344,12 +1218,6 @@ func TestNestedFolderService(t *testing.T) { }) 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}) - t.Cleanup(func() { - guardian.New = g - }) - dashStore := &dashboards.FakeDashboardStore{} dashboardFolderStore := foldertest.NewFakeFolderStore(t) @@ -1366,13 +1234,6 @@ func TestNestedFolderService(t *testing.T) { }) t.Run("create returns error if maximum depth reached", func(t *testing.T) { - // This test creates and deletes the dashboard, so needs some extra setup. - 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{} dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*dashboards.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil).Times(2) @@ -1405,12 +1266,6 @@ func TestNestedFolderService(t *testing.T) { }) t.Run("get default folder, no error", 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{} nestedFolderStore := folder.NewFakeStore() @@ -1501,11 +1356,6 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) { SignedInUser: &signedInAdminUser, } - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{ - CanSaveValue: true, - CanViewValue: true, - }) - t.Run("Should get folders shared with given user", func(t *testing.T) { depth := 3 @@ -1551,7 +1401,6 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) { require.NotContains(t, sharedFoldersUIDs, ancestorFoldersWithPermissions[1].UID) t.Cleanup(func() { - //guardian.New = origNewGuardian toDelete := make([]string, 0, len(ancestorFoldersWithPermissions)+len(ancestorFoldersWithoutPermissions)) for _, ancestor := range append(ancestorFoldersWithPermissions, ancestorFoldersWithoutPermissions...) { toDelete = append(toDelete, ancestor.UID) @@ -1606,7 +1455,6 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) { assert.Equal(t, 3, len(folders), "service accounts should be able to list k6 folders") t.Cleanup(func() { - //guardian.New = origNewGuardian toDelete := []string{k6ChildFolder.UID, accesscontrol.K6FolderUID, unrelatedFolder.UID} err := serviceWithFlagOn.store.Delete(context.Background(), toDelete, orgID) assert.NoError(t, err) @@ -1846,11 +1694,6 @@ func TestFolderServiceGetFolder(t *testing.T) { }, }} - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{ - CanSaveValue: true, - CanViewValue: true, - }) - getSvc := func(features featuremgmt.FeatureToggles) Service { folderStore := ProvideDashboardFolderStore(db) @@ -1990,11 +1833,6 @@ func TestFolderServiceGetFolders(t *testing.T) { SignedInUser: &signedInAdminUser, } - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{ - CanSaveValue: true, - CanViewValue: true, - }) - prefix := "getfolders/ff/off" folders := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOff, 5, prefix, createCmd, true) f := folders[rand.Intn(len(folders))] @@ -2062,17 +1900,6 @@ func TestGetChildrenFilterByPermission(t *testing.T) { tracer: tracing.InitializeTracerForTest(), } - 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: {}, @@ -2103,7 +1930,6 @@ func TestGetChildrenFilterByPermission(t *testing.T) { 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{ @@ -2114,9 +1940,7 @@ func TestGetChildrenFilterByPermission(t *testing.T) { }) 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, @@ -2126,9 +1950,7 @@ func TestGetChildrenFilterByPermission(t *testing.T) { }) 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, @@ -2146,7 +1968,6 @@ func TestGetChildrenFilterByPermission(t *testing.T) { }) 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, @@ -2164,7 +1985,6 @@ func TestGetChildrenFilterByPermission(t *testing.T) { }) 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 diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go index 58ebe2aa878..2d6914f8dda 100644 --- a/pkg/services/guardian/guardian.go +++ b/pkg/services/guardian/guardian.go @@ -33,12 +33,6 @@ var New = func(ctx context.Context, dashId int64, orgId int64, user identity.Req panic("no guardian factory implementation provided") } -// NewByFolderUID factory for creating a new folder guardian instance -// When using access control this function is replaced on startup and the AccessControlDashboardGuardian is returned -var NewByFolderUID = func(ctx context.Context, folderUID string, orgId int64, user identity.Requester) (DashboardGuardian, error) { - panic("no guardian factory implementation provided") -} - // NewByFolder factory for creating a new folder guardian instance // When using access control this function is replaced on startup and the AccessControlDashboardGuardian is returned var NewByFolder = func(ctx context.Context, f *folder.Folder, orgId int64, user identity.Requester) (DashboardGuardian, error) { @@ -102,13 +96,6 @@ func MockDashboardGuardian(mock *FakeDashboardGuardian) { return mock, nil } - NewByFolderUID = func(_ context.Context, folderUID string, orgId int64, user identity.Requester) (DashboardGuardian, error) { - mock.OrgID = orgId - mock.DashUID = folderUID - mock.User = user - return mock, nil - } - NewByFolder = func(_ context.Context, f *folder.Folder, orgId int64, user identity.Requester) (DashboardGuardian, error) { mock.OrgID = orgId mock.DashUID = f.UID diff --git a/pkg/services/guardian/provider.go b/pkg/services/guardian/provider.go index 7be96481b51..2f904a2b72d 100644 --- a/pkg/services/guardian/provider.go +++ b/pkg/services/guardian/provider.go @@ -31,10 +31,6 @@ func InitAccessControlGuardian( return NewAccessControlDashboardGuardian(ctx, cfg, dashId, user, ac, dashboardService, folderService, logger) } - NewByFolderUID = func(ctx context.Context, folderUID string, orgId int64, user identity.Requester) (DashboardGuardian, error) { - return NewAccessControlFolderGuardianByUID(ctx, cfg, folderUID, user, ac, dashboardService, folderService) - } - NewByFolder = func(ctx context.Context, f *folder.Folder, orgId int64, user identity.Requester) (DashboardGuardian, error) { return NewAccessControlFolderGuardian(ctx, cfg, f, user, ac, orgId, dashboardService, folderService) } diff --git a/pkg/services/libraryelements/guard.go b/pkg/services/libraryelements/guard.go index 737d961f8c8..c7c7d5d973a 100644 --- a/pkg/services/libraryelements/guard.go +++ b/pkg/services/libraryelements/guard.go @@ -2,12 +2,11 @@ package libraryelements import ( "context" + "strconv" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" - "github.com/grafana/grafana/pkg/services/folder" - "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/libraryelements/model" "github.com/grafana/grafana/pkg/services/org" ) @@ -42,12 +41,8 @@ func (l *LibraryElementService) requireEditPermissionsOnFolderUID(ctx context.Co return dashboards.ErrFolderAccessDenied } - g, err := guardian.NewByFolderUID(ctx, folderUID, user.GetOrgID(), user) - if err != nil { - return err - } - - canEdit, err := g.CanEdit() + evaluator := accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(folderUID)) + canEdit, err := l.AccessControl.Evaluate(ctx, user, evaluator) if err != nil { return err } @@ -68,15 +63,11 @@ func (l *LibraryElementService) requireEditPermissionsOnFolder(ctx context.Conte return dashboards.ErrFolderAccessDenied } - g, err := guardian.NewByFolder(ctx, &folder.Folder{ - ID: folderID, - OrgID: user.GetOrgID(), - }, user.GetOrgID(), user) - if err != nil { - return err + evaluator := accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScope(strconv.FormatInt(folderID, 10))) + if isGeneralFolder(folderID) { + evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(accesscontrol.GeneralFolderUID)) } - - canEdit, err := g.CanEdit() + canEdit, err := l.AccessControl.Evaluate(ctx, user, evaluator) if err != nil { return err } @@ -88,19 +79,11 @@ func (l *LibraryElementService) requireEditPermissionsOnFolder(ctx context.Conte } func (l *LibraryElementService) requireViewPermissionsOnFolder(ctx context.Context, user identity.Requester, folderID int64) error { + evaluator := accesscontrol.EvalPermission(dashboards.ActionFoldersRead, dashboards.ScopeFoldersProvider.GetResourceScope(strconv.FormatInt(folderID, 10))) if isGeneralFolder(folderID) { - return nil + evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(accesscontrol.GeneralFolderUID)) } - - g, err := guardian.NewByFolder(ctx, &folder.Folder{ - ID: folderID, - OrgID: user.GetOrgID(), - }, user.GetOrgID(), user) - if err != nil { - return err - } - - canView, err := g.CanView() + canView, err := l.AccessControl.Evaluate(ctx, user, evaluator) if err != nil { return err } diff --git a/pkg/services/libraryelements/libraryelements_patch_test.go b/pkg/services/libraryelements/libraryelements_patch_test.go index 3a7c962722f..2ba9c7f82e1 100644 --- a/pkg/services/libraryelements/libraryelements_patch_test.go +++ b/pkg/services/libraryelements/libraryelements_patch_test.go @@ -324,9 +324,10 @@ func TestIntegration_PatchLibraryElement(t *testing.T) { resp := sc.service.createHandler(sc.reqContext) var result = validateAndUnMarshalResponse(t, resp) cmd := model.PatchLibraryElementCommand{ - Name: "Text - Library Panel", - Version: 1, - Kind: int64(model.PanelElement), + Name: "Text - Library Panel", + Version: 1, + Kind: int64(model.PanelElement), + FolderUID: &sc.folder.UID, } sc.ctx.Req = web.SetURLParams(sc.ctx.Req, map[string]string{":uid": result.Result.UID}) sc.ctx.Req.Body = mockRequestBody(cmd) diff --git a/pkg/services/libraryelements/libraryelements_permissions_test.go b/pkg/services/libraryelements/libraryelements_permissions_test.go index 095eebc4965..fd682d70235 100644 --- a/pkg/services/libraryelements/libraryelements_permissions_test.go +++ b/pkg/services/libraryelements/libraryelements_permissions_test.go @@ -9,131 +9,153 @@ import ( "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" + "github.com/grafana/grafana/pkg/services/accesscontrol/actest" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/folder/folderimpl" "github.com/grafana/grafana/pkg/services/libraryelements/model" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/web" ) func TestLibraryElementPermissionsGeneralFolder(t *testing.T) { - var generalFolderCases = []struct { - role org.RoleType - status int - }{ - {org.RoleAdmin, 200}, - {org.RoleEditor, 200}, - {org.RoleViewer, 403}, - } + testScenario(t, "When user with tries to create a library panel in the General folder, it should return correct status", + func(t *testing.T, sc scenarioContext) { + sc.reqContext.OrgRole = org.RoleViewer + command := getCreatePanelCommand(0, "", "Library Panel Name") + sc.reqContext.Req.Body = mockRequestBody(command) + resp := sc.service.createHandler(sc.reqContext) + require.Equal(t, http.StatusForbidden, resp.Status()) - for _, testCase := range generalFolderCases { - testScenario(t, fmt.Sprintf("When %s tries to create a library panel in the General folder, it should return correct status", testCase.role), - func(t *testing.T, sc scenarioContext) { - sc.reqContext.OrgRole = testCase.role + sc.reqContext.OrgRole = org.RoleEditor + sc.reqContext.Req.Body = mockRequestBody(command) + resp = sc.service.createHandler(sc.reqContext) + require.Equal(t, http.StatusOK, resp.Status()) + }) - command := getCreatePanelCommand(0, "", "Library Panel Name") - sc.reqContext.Req.Body = mockRequestBody(command) - resp := sc.service.createHandler(sc.reqContext) - require.Equal(t, testCase.status, resp.Status()) - }) + testScenario(t, "When user tries to patch a library panel by moving it to the General folder, it should return correct status", + func(t *testing.T, sc scenarioContext) { + folder := createFolder(t, sc, "Folder", nil) + // nolint:staticcheck + command := getCreatePanelCommand(folder.ID, folder.UID, "Library Panel Name") + sc.reqContext.Req.Body = mockRequestBody(command) + resp := sc.service.createHandler(sc.reqContext) + result := validateAndUnMarshalResponse(t, resp) - testScenario(t, fmt.Sprintf("When %s tries to patch a library panel by moving it to the General folder, it should return correct status", testCase.role), - func(t *testing.T, sc scenarioContext) { - folder := createFolder(t, sc, "Folder", nil) - // nolint:staticcheck - command := getCreatePanelCommand(folder.ID, folder.UID, "Library Panel Name") - sc.reqContext.Req.Body = mockRequestBody(command) - resp := sc.service.createHandler(sc.reqContext) - result := validateAndUnMarshalResponse(t, resp) - sc.reqContext.OrgRole = testCase.role + // nolint:staticcheck + sc.reqContext.OrgRole = org.RoleViewer + cmd := model.PatchLibraryElementCommand{FolderID: 0, Version: 1, Kind: int64(model.PanelElement)} + sc.ctx.Req = web.SetURLParams(sc.ctx.Req, map[string]string{":uid": result.Result.UID}) + sc.ctx.Req.Body = mockRequestBody(cmd) + resp = sc.service.patchHandler(sc.reqContext) + require.Equal(t, http.StatusForbidden, resp.Status()) - // nolint:staticcheck - cmd := model.PatchLibraryElementCommand{FolderID: 0, Version: 1, Kind: int64(model.PanelElement)} - sc.ctx.Req = web.SetURLParams(sc.ctx.Req, map[string]string{":uid": result.Result.UID}) - sc.ctx.Req.Body = mockRequestBody(cmd) - resp = sc.service.patchHandler(sc.reqContext) - require.Equal(t, testCase.status, resp.Status()) - }) + sc.reqContext.OrgRole = org.RoleEditor + sc.ctx.Req.Body = mockRequestBody(cmd) + resp = sc.service.patchHandler(sc.reqContext) + require.Equal(t, http.StatusOK, resp.Status()) + }) - testScenario(t, fmt.Sprintf("When %s tries to patch a library panel by moving it from the General folder, it should return correct status", testCase.role), - func(t *testing.T, sc scenarioContext) { - folder := createFolder(t, sc, "Folder", nil) - command := getCreatePanelCommand(0, "", "Library Panel Name") - sc.reqContext.Req.Body = mockRequestBody(command) - resp := sc.service.createHandler(sc.reqContext) - result := validateAndUnMarshalResponse(t, resp) - sc.reqContext.OrgRole = testCase.role + testScenario(t, "When user tries to patch a library panel by moving it from the General folder, it should return correct status", + func(t *testing.T, sc scenarioContext) { + folder := createFolder(t, sc, "Folder", nil) + command := getCreatePanelCommand(0, "", "Library Panel Name") + sc.reqContext.Req.Body = mockRequestBody(command) + sc.service.AccessControl = actest.FakeAccessControl{ExpectedEvaluate: true} + resp := sc.service.createHandler(sc.reqContext) + result := validateAndUnMarshalResponse(t, resp) - // nolint:staticcheck - cmd := model.PatchLibraryElementCommand{FolderID: folder.ID, Version: 1, Kind: int64(model.PanelElement)} - sc.ctx.Req = web.SetURLParams(sc.ctx.Req, map[string]string{":uid": result.Result.UID}) - sc.ctx.Req.Body = mockRequestBody(cmd) - resp = sc.service.patchHandler(sc.reqContext) - require.Equal(t, testCase.status, resp.Status()) - }) + sc.reqContext.OrgRole = org.RoleViewer + cmd := model.PatchLibraryElementCommand{FolderUID: &folder.UID, Version: 1, Kind: int64(model.PanelElement)} + sc.service.AccessControl = acimpl.ProvideAccessControl(featuremgmt.WithFeatures()) + sc.service.AccessControl.RegisterScopeAttributeResolver(dashboards.NewFolderIDScopeResolver(folderimpl.ProvideDashboardFolderStore(sc.sqlStore), sc.service.folderService)) + sc.ctx.Req = web.SetURLParams(sc.ctx.Req, map[string]string{":uid": result.Result.UID}) + sc.ctx.Req.Body = mockRequestBody(cmd) + resp = sc.service.patchHandler(sc.reqContext) + require.Equal(t, http.StatusForbidden, resp.Status()) - testScenario(t, fmt.Sprintf("When %s tries to delete a library panel in the General folder, it should return correct status", testCase.role), - func(t *testing.T, sc scenarioContext) { - cmd := getCreatePanelCommand(0, "", "Library Panel Name") - sc.reqContext.Req.Body = mockRequestBody(cmd) - resp := sc.service.createHandler(sc.reqContext) - result := validateAndUnMarshalResponse(t, resp) - sc.reqContext.OrgRole = testCase.role + sc.reqContext.OrgRole = org.RoleEditor + sc.reqContext.Permissions[sc.user.OrgID][dashboards.ActionFoldersWrite] = append(sc.reqContext.Permissions[sc.user.OrgID][dashboards.ActionFoldersWrite], dashboards.ScopeFoldersProvider.GetResourceScopeUID(folder.UID)) + sc.reqContext.Permissions[sc.user.OrgID][dashboards.ActionFoldersWrite] = append(sc.reqContext.Permissions[sc.user.OrgID][dashboards.ActionFoldersWrite], dashboards.ScopeFoldersProvider.GetResourceScopeUID(accesscontrol.GeneralFolderUID)) + sc.ctx.Req.Body = mockRequestBody(cmd) + resp = sc.service.patchHandler(sc.reqContext) + require.Equal(t, http.StatusOK, resp.Status()) + }) - sc.ctx.Req = web.SetURLParams(sc.ctx.Req, map[string]string{":uid": result.Result.UID}) - resp = sc.service.deleteHandler(sc.reqContext) - require.Equal(t, testCase.status, resp.Status()) - }) + testScenario(t, "When user tries to delete a library panel in the General folder, it should return correct status", + func(t *testing.T, sc scenarioContext) { + cmd := getCreatePanelCommand(0, "", "Library Panel Name") + sc.reqContext.Req.Body = mockRequestBody(cmd) + sc.service.AccessControl = actest.FakeAccessControl{ExpectedEvaluate: true} + resp := sc.service.createHandler(sc.reqContext) + result := validateAndUnMarshalResponse(t, resp) - testScenario(t, fmt.Sprintf("When %s tries to get a library panel from General folder, it should return correct response", testCase.role), - func(t *testing.T, sc scenarioContext) { - cmd := getCreatePanelCommand(0, "", "Library Panel in General Folder") - sc.reqContext.Req.Body = mockRequestBody(cmd) - resp := sc.service.createHandler(sc.reqContext) - result := validateAndUnMarshalResponse(t, resp) - result.Result.Meta.CreatedBy.Name = userInDbName - result.Result.Meta.CreatedBy.AvatarUrl = userInDbAvatar - result.Result.Meta.UpdatedBy.Name = userInDbName - result.Result.Meta.UpdatedBy.AvatarUrl = userInDbAvatar - result.Result.Meta.FolderName = "General" - result.Result.Meta.FolderUID = "general" - result.Result.FolderUID = "general" - sc.reqContext.OrgRole = testCase.role + sc.reqContext.OrgRole = org.RoleViewer + sc.service.AccessControl = acimpl.ProvideAccessControl(featuremgmt.WithFeatures()) + sc.ctx.Req = web.SetURLParams(sc.ctx.Req, map[string]string{":uid": result.Result.UID}) + resp = sc.service.deleteHandler(sc.reqContext) + require.Equal(t, http.StatusForbidden, resp.Status()) - sc.ctx.Req = web.SetURLParams(sc.ctx.Req, map[string]string{":uid": result.Result.UID}) - resp = sc.service.getHandler(sc.reqContext) - require.Equal(t, 200, resp.Status()) - var actual libraryElementResult - err := json.Unmarshal(resp.Body(), &actual) - require.NoError(t, err) - if diff := cmp.Diff(result.Result, actual.Result, getCompareOptions()...); diff != "" { - t.Fatalf("Result mismatch (-want +got):\n%s", diff) - } - }) + sc.reqContext.OrgRole = org.RoleEditor + resp = sc.service.deleteHandler(sc.reqContext) + require.Equal(t, http.StatusOK, resp.Status()) + }) - testScenario(t, fmt.Sprintf("When %s tries to get all library panels from General folder, it should return correct response", testCase.role), - func(t *testing.T, sc scenarioContext) { - cmd := getCreatePanelCommand(0, "", "Library Panel in General Folder") - sc.reqContext.Req.Body = mockRequestBody(cmd) - resp := sc.service.createHandler(sc.reqContext) - result := validateAndUnMarshalResponse(t, resp) - result.Result.Meta.CreatedBy.Name = userInDbName - result.Result.Meta.CreatedBy.AvatarUrl = userInDbAvatar - result.Result.Meta.UpdatedBy.Name = userInDbName - result.Result.Meta.UpdatedBy.AvatarUrl = userInDbAvatar - result.Result.Meta.FolderName = "General" - sc.reqContext.OrgRole = testCase.role + testScenario(t, "When user tries to get a library panel from General folder, it should return correct response", + func(t *testing.T, sc scenarioContext) { + sc.service.AccessControl = actest.FakeAccessControl{ExpectedEvaluate: true} + cmd := getCreatePanelCommand(0, "", "Library Panel in General Folder") + sc.reqContext.Req.Body = mockRequestBody(cmd) + resp := sc.service.createHandler(sc.reqContext) + result := validateAndUnMarshalResponse(t, resp) + result.Result.Meta.CreatedBy.Name = userInDbName + result.Result.Meta.CreatedBy.AvatarUrl = userInDbAvatar + result.Result.Meta.UpdatedBy.Name = userInDbName + result.Result.Meta.UpdatedBy.AvatarUrl = userInDbAvatar + result.Result.Meta.FolderName = "General" + result.Result.Meta.FolderUID = "general" + result.Result.FolderUID = "general" - resp = sc.service.getAllHandler(sc.reqContext) - require.Equal(t, 200, resp.Status()) - var actual libraryElementsSearch - err := json.Unmarshal(resp.Body(), &actual) - require.NoError(t, err) - require.Equal(t, 1, len(actual.Result.Elements)) - if diff := cmp.Diff(result.Result, actual.Result.Elements[0], getCompareOptions()...); diff != "" { - t.Fatalf("Result mismatch (-want +got):\n%s", diff) - } - }) - } + sc.service.AccessControl = acimpl.ProvideAccessControl(featuremgmt.WithFeatures()) + sc.reqContext.Permissions[sc.user.OrgID][dashboards.ActionFoldersRead] = append(sc.reqContext.Permissions[sc.user.OrgID][dashboards.ActionFoldersRead], dashboards.ScopeFoldersProvider.GetResourceScopeUID(accesscontrol.GeneralFolderUID)) + sc.ctx.Req = web.SetURLParams(sc.ctx.Req, map[string]string{":uid": result.Result.UID}) + resp = sc.service.getHandler(sc.reqContext) + require.Equal(t, 200, resp.Status()) + var actual libraryElementResult + err := json.Unmarshal(resp.Body(), &actual) + require.NoError(t, err) + if diff := cmp.Diff(result.Result, actual.Result, getCompareOptions()...); diff != "" { + t.Fatalf("Result mismatch (-want +got):\n%s", diff) + } + }) + + testScenario(t, "When user tries to get all library panels from General folder, it should return correct response", + func(t *testing.T, sc scenarioContext) { + sc.service.AccessControl = actest.FakeAccessControl{ExpectedEvaluate: true} + cmd := getCreatePanelCommand(0, "", "Library Panel in General Folder") + sc.reqContext.Req.Body = mockRequestBody(cmd) + resp := sc.service.createHandler(sc.reqContext) + result := validateAndUnMarshalResponse(t, resp) + result.Result.Meta.CreatedBy.Name = userInDbName + result.Result.Meta.CreatedBy.AvatarUrl = userInDbAvatar + result.Result.Meta.UpdatedBy.Name = userInDbName + result.Result.Meta.UpdatedBy.AvatarUrl = userInDbAvatar + result.Result.Meta.FolderName = "General" + + sc.service.AccessControl = acimpl.ProvideAccessControl(featuremgmt.WithFeatures()) + sc.reqContext.Permissions[sc.user.OrgID][dashboards.ActionFoldersRead] = append(sc.reqContext.Permissions[sc.user.OrgID][dashboards.ActionFoldersRead], dashboards.ScopeFoldersProvider.GetResourceScopeUID(accesscontrol.GeneralFolderUID)) + resp = sc.service.getAllHandler(sc.reqContext) + require.Equal(t, 200, resp.Status()) + var actual libraryElementsSearch + err := json.Unmarshal(resp.Body(), &actual) + require.NoError(t, err) + require.Equal(t, 1, len(actual.Result.Elements)) + if diff := cmp.Diff(result.Result, actual.Result.Elements[0], getCompareOptions()...); diff != "" { + t.Fatalf("Result mismatch (-want +got):\n%s", diff) + } + }) } func TestLibraryElementCreatePermissions(t *testing.T) { diff --git a/pkg/services/libraryelements/libraryelements_test.go b/pkg/services/libraryelements/libraryelements_test.go index faffe25225e..6f28616ec2e 100644 --- a/pkg/services/libraryelements/libraryelements_test.go +++ b/pkg/services/libraryelements/libraryelements_test.go @@ -37,7 +37,6 @@ import ( "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/folder/folderimpl" "github.com/grafana/grafana/pkg/services/folder/foldertest" - "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/libraryelements/model" ngstore "github.com/grafana/grafana/pkg/services/ngalert/store" "github.com/grafana/grafana/pkg/services/org" @@ -106,8 +105,10 @@ func TestIntegration_DeleteLibraryPanelsInFolder(t *testing.T) { scenarioWithPanel(t, "When an admin tries to delete a folder uid that doesn't exist, it should fail", func(t *testing.T, sc scenarioContext) { + sc.service.AccessControl = acimpl.ProvideAccessControl(featuremgmt.WithFeatures()) + sc.service.AccessControl.RegisterScopeAttributeResolver(dashboards.NewFolderUIDScopeResolver(sc.service.folderService)) err := sc.service.DeleteLibraryElementsInFolder(sc.reqContext.Req.Context(), sc.reqContext.SignedInUser, sc.folder.UID+"xxxx") - require.EqualError(t, err, guardian.ErrGuardianFolderNotFound.Errorf("failed to get folder by UID: %w", dashboards.ErrFolderNotFound).Error()) + require.ErrorIs(t, err, dashboards.ErrFolderAccessDenied) }) scenarioWithPanel(t, "When an admin tries to delete a folder that contains disconnected elements, it should delete all disconnected elements too", @@ -468,7 +469,6 @@ func scenarioWithPanel(t *testing.T, desc string, fn func(t *testing.T, sc scena ) require.NoError(t, svcErr) dashboardService.RegisterDashboardPermissions(dashboardPermissions) - guardian.InitAccessControlGuardian(cfg, ac, dashboardService, folderSvc, log.NewNopLogger()) testScenario(t, desc, func(t *testing.T, sc scenarioContext) { // nolint:staticcheck @@ -542,13 +542,13 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo ) require.NoError(t, dashSvcErr) dashService.RegisterDashboardPermissions(dashboardPermissions) - guardian.InitAccessControlGuardian(cfg, ac, dashService, folderSvc, log.NewNopLogger()) service := LibraryElementService{ Cfg: cfg, features: featuremgmt.WithFeatures(), SQLStore: sqlStore, folderService: folderSvc, dashboardsService: dashService, + AccessControl: ac, log: log.NewNopLogger(), } diff --git a/pkg/services/ngalert/tests/util.go b/pkg/services/ngalert/tests/util.go index 0d356fabd7b..0c0bc387d66 100644 --- a/pkg/services/ngalert/tests/util.go +++ b/pkg/services/ngalert/tests/util.go @@ -122,13 +122,15 @@ func CreateTestAlertRuleWithLabels(t testing.TB, ctx context.Context, dbstore *s OrgRole: org.RoleAdmin, IsGrafanaAdmin: true, Permissions: map[int64]map[string][]string{ - orgID: {dashboards.ActionFoldersCreate: {dashboards.ScopeFoldersAll}}, + orgID: { + dashboards.ActionFoldersCreate: {dashboards.ScopeFoldersAll}, + dashboards.ActionFoldersRead: {dashboards.ScopeFoldersAll}, + }, }, } ctx = identity.WithRequester(ctx, user) _, err := dbstore.FolderService.Create(ctx, &folder.CreateFolderCommand{OrgID: orgID, Title: "FOLDER-" + util.GenerateShortUID(), UID: folderUID, SignedInUser: user}) - // var foldr *folder.Folder if errors.Is(err, dashboards.ErrFolderWithSameUIDExists) || errors.Is(err, dashboards.ErrFolderVersionMismatch) { _, err = dbstore.FolderService.Get(ctx, &folder.GetFolderQuery{OrgID: orgID, UID: &folderUID, SignedInUser: user}) }