Folders: Fix guardian to use folder service (#99339)

pkg/services/libraryelements/guard.go

@@ -6,6 +6,7 @@ import (
 	"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"
@@ -41,7 +42,7 @@ func (l *LibraryElementService) requireEditPermissionsOnFolderUID(ctx context.Co
 		return dashboards.ErrFolderAccessDenied
 	}
 
-	g, err := guardian.NewByUID(ctx, folderUID, user.GetOrgID(), user)
+	g, err := guardian.NewByFolderUID(ctx, folderUID, user.GetOrgID(), user)
 	if err != nil {
 		return err
 	}
@@ -67,7 +68,10 @@ func (l *LibraryElementService) requireEditPermissionsOnFolder(ctx context.Conte
 		return dashboards.ErrFolderAccessDenied
 	}
 
-	g, err := guardian.New(ctx, folderID, user.GetOrgID(), user)
+	g, err := guardian.NewByFolder(ctx, &folder.Folder{
+		ID:    folderID,
+		OrgID: user.GetOrgID(),
+	}, user.GetOrgID(), user)
 	if err != nil {
 		return err
 	}
@@ -88,7 +92,10 @@ func (l *LibraryElementService) requireViewPermissionsOnFolder(ctx context.Conte
 		return nil
 	}
 
-	g, err := guardian.New(ctx, folderID, user.GetOrgID(), user)
+	g, err := guardian.NewByFolder(ctx, &folder.Folder{
+		ID:    folderID,
+		OrgID: user.GetOrgID(),
+	}, user.GetOrgID(), user)
 	if err != nil {
 		return err
 	}

pkg/services/libraryelements/libraryelements_get_all_test.go

@@ -539,7 +539,7 @@ func TestGetAllLibraryElements(t *testing.T) {
 
 	scenarioWithPanel(t, "When an admin tries to get all library panels and two exist and folderFilterUIDs is set to existing folders, it should succeed and the result should be correct",
 		func(t *testing.T, sc scenarioContext) {
-			newFolder := createFolder(t, sc, "NewFolder")
+			newFolder := createFolder(t, sc, "NewFolder", nil)
 			// nolint:staticcheck
 			command := getCreatePanelCommand(newFolder.ID, newFolder.UID, "Text - Library Panel2")
 			sc.reqContext.Req.Body = mockRequestBody(command)
@@ -608,7 +608,7 @@ func TestGetAllLibraryElements(t *testing.T) {
 
 	scenarioWithPanel(t, "When an admin tries to get all library panels and two exist and folderFilter is set to a nonexistent folders, it should succeed and the result should be correct",
 		func(t *testing.T, sc scenarioContext) {
-			newFolder := createFolder(t, sc, "NewFolder")
+			newFolder := createFolder(t, sc, "NewFolder", nil)
 			// nolint:staticcheck
 			command := getCreatePanelCommand(newFolder.ID, sc.folder.UID, "Text - Library Panel2")
 			sc.reqContext.Req.Body = mockRequestBody(command)

pkg/services/libraryelements/libraryelements_patch_test.go

@@ -24,7 +24,7 @@ func TestPatchLibraryElement(t *testing.T) {
 
 	scenarioWithPanel(t, "When an admin tries to patch a library panel that exists, it should succeed",
 		func(t *testing.T, sc scenarioContext) {
-			newFolder := createFolder(t, sc, "NewFolder")
+			newFolder := createFolder(t, sc, "NewFolder", nil)
 			cmd := model.PatchLibraryElementCommand{
 				FolderID:  newFolder.ID, // nolint:staticcheck
 				FolderUID: &newFolder.UID,
@@ -91,7 +91,7 @@ func TestPatchLibraryElement(t *testing.T) {
 
 	scenarioWithPanel(t, "When an admin tries to patch a library panel with folder only, it should change folder successfully and return correct result",
 		func(t *testing.T, sc scenarioContext) {
-			newFolder := createFolder(t, sc, "NewFolder")
+			newFolder := createFolder(t, sc, "NewFolder", nil)
 			cmd := model.PatchLibraryElementCommand{
 				FolderID:  newFolder.ID, // nolint:staticcheck
 				FolderUID: &newFolder.UID,
@@ -335,7 +335,7 @@ func TestPatchLibraryElement(t *testing.T) {
 
 	scenarioWithPanel(t, "When an admin tries to patch a library panel with a folder where a library panel with the same name already exists, it should fail",
 		func(t *testing.T, sc scenarioContext) {
-			newFolder := createFolder(t, sc, "NewFolder")
+			newFolder := createFolder(t, sc, "NewFolder", nil)
 			// nolint:staticcheck
 			command := getCreatePanelCommand(newFolder.ID, newFolder.UID, "Text - Library Panel")
 			sc.ctx.Req.Body = mockRequestBody(command)

pkg/services/libraryelements/libraryelements_permissions_test.go

@@ -38,7 +38,7 @@ func TestLibraryElementPermissionsGeneralFolder(t *testing.T) {
 
 		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")
+				folder := createFolder(t, sc, "Folder", nil)
 				// nolint:staticcheck
 				command := getCreatePanelCommand(folder.ID, folder.UID, "Library Panel Name")
 				sc.reqContext.Req.Body = mockRequestBody(command)
@@ -56,7 +56,7 @@ func TestLibraryElementPermissionsGeneralFolder(t *testing.T) {
 
 		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")
+				folder := createFolder(t, sc, "Folder", nil)
 				command := getCreatePanelCommand(0, "", "Library Panel Name")
 				sc.reqContext.Req.Body = mockRequestBody(command)
 				resp := sc.service.createHandler(sc.reqContext)
@@ -178,7 +178,7 @@ func TestLibraryElementCreatePermissions(t *testing.T) {
 	for _, testCase := range accessCases {
 		testScenario(t, testCase.desc,
 			func(t *testing.T, sc scenarioContext) {
-				folder := createFolder(t, sc, "Folder")
+				folder := createFolder(t, sc, "Folder", nil)
 				sc.reqContext.SignedInUser.Permissions = map[int64]map[string][]string{
 					1: testCase.permissions,
 				}
@@ -235,14 +235,14 @@ func TestLibraryElementPatchPermissions(t *testing.T) {
 	for _, testCase := range accessCases {
 		testScenario(t, testCase.desc,
 			func(t *testing.T, sc scenarioContext) {
-				fromFolder := createFolder(t, sc, "FromFolder")
+				fromFolder := createFolder(t, sc, "FromFolder", nil)
 				// nolint:staticcheck
 				command := getCreatePanelCommand(fromFolder.ID, fromFolder.UID, "Library Panel Name")
 				sc.reqContext.Req.Body = mockRequestBody(command)
 				resp := sc.service.createHandler(sc.reqContext)
 				result := validateAndUnMarshalResponse(t, resp)
 
-				toFolder := createFolder(t, sc, "ToFolder")
+				toFolder := createFolder(t, sc, "ToFolder", nil)
 
 				sc.reqContext.SignedInUser.Permissions = map[int64]map[string][]string{
 					1: testCase.permissions,
@@ -268,6 +268,7 @@ func TestLibraryElementDeletePermissions(t *testing.T) {
 			desc: "can delete library elements when granted write access to the correct folder",
 			permissions: map[string][]string{
 				dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("Folder")},
+				dashboards.ActionFoldersRead:  {dashboards.ScopeFoldersProvider.GetResourceScopeUID("Folder")},
 			},
 			status: http.StatusOK,
 		},
@@ -275,6 +276,7 @@ func TestLibraryElementDeletePermissions(t *testing.T) {
 			desc: "can delete library elements when granted write access to all folders",
 			permissions: map[string][]string{
 				dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceAllScope()},
+				dashboards.ActionFoldersRead:  {dashboards.ScopeFoldersProvider.GetResourceAllScope()},
 			},
 			status: http.StatusOK,
 		},
@@ -282,6 +284,7 @@ func TestLibraryElementDeletePermissions(t *testing.T) {
 			desc: "can't delete library elements when granted write access to the wrong folder",
 			permissions: map[string][]string{
 				dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("Other_folder")},
+				dashboards.ActionFoldersRead:  {dashboards.ScopeFoldersProvider.GetResourceScopeUID("Other_folder")},
 			},
 			status: http.StatusForbidden,
 		},
@@ -297,7 +300,7 @@ func TestLibraryElementDeletePermissions(t *testing.T) {
 	for _, testCase := range accessCases {
 		testScenario(t, testCase.desc,
 			func(t *testing.T, sc scenarioContext) {
-				folder := createFolder(t, sc, "Folder")
+				folder := createFolder(t, sc, "Folder", sc.service.folderService)
 				// nolint:staticcheck
 				command := getCreatePanelCommand(folder.ID, folder.UID, "Library Panel Name")
 				sc.reqContext.Req.Body = mockRequestBody(command)
@@ -327,7 +330,7 @@ func TestLibraryElementsWithMissingFolders(t *testing.T) {
 
 	testScenario(t, "When a user tries to patch a library panel by moving it to a folder that doesn't exist, it should fail",
 		func(t *testing.T, sc scenarioContext) {
-			folder := createFolder(t, sc, "Folder")
+			folder := createFolder(t, sc, "Folder", nil)
 			// nolint:staticcheck
 			command := getCreatePanelCommand(folder.ID, folder.UID, "Library Panel Name")
 			sc.reqContext.Req.Body = mockRequestBody(command)
@@ -368,7 +371,7 @@ func TestLibraryElementsGetPermissions(t *testing.T) {
 	for _, testCase := range getCases {
 		testScenario(t, testCase.desc,
 			func(t *testing.T, sc scenarioContext) {
-				folder := createFolder(t, sc, "Folder")
+				folder := createFolder(t, sc, "Folder", nil)
 				// nolint:staticcheck
 				cmd := getCreatePanelCommand(folder.ID, folder.UID, "Library Panel")
 				sc.reqContext.Req.Body = mockRequestBody(cmd)
@@ -419,7 +422,7 @@ func TestLibraryElementsGetAllPermissions(t *testing.T) {
 		testScenario(t, testCase.desc,
 			func(t *testing.T, sc scenarioContext) {
 				for i := 1; i <= 2; i++ {
-					folder := createFolder(t, sc, fmt.Sprintf("Folder%d", i))
+					folder := createFolder(t, sc, fmt.Sprintf("Folder%d", i), nil)
 					// nolint:staticcheck
 					cmd := getCreatePanelCommand(folder.ID, folder.UID, fmt.Sprintf("Library Panel %d", i))
 					sc.reqContext.Req.Body = mockRequestBody(cmd)

pkg/services/libraryelements/libraryelements_test.go

@@ -32,7 +32,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"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"
 	"github.com/grafana/grafana/pkg/services/org"
@@ -99,7 +98,7 @@ func TestDeleteLibraryPanelsInFolder(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) {
 			err := sc.service.DeleteLibraryElementsInFolder(sc.reqContext.Req.Context(), sc.reqContext.SignedInUser, sc.folder.UID+"xxxx")
-			require.EqualError(t, err, dashboards.ErrFolderNotFound.Error())
+			require.EqualError(t, err, guardian.ErrGuardianFolderNotFound.Errorf("failed to get folder by UID: %w", dashboards.ErrFolderNotFound).Error())
 		})
 
 	scenarioWithPanel(t, "When an admin tries to delete a folder that contains disconnected elements, it should delete all disconnected elements too",
@@ -300,17 +299,19 @@ func createDashboard(t *testing.T, sqlStore db.DB, user user.SignedInUser, dash
 	require.NoError(t, err)
 	ac := actest.FakeAccessControl{ExpectedEvaluate: true}
 	folderPermissions := acmock.NewMockedPermissionsService()
+	folderPermissions.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil)
 	dashboardPermissions := acmock.NewMockedPermissionsService()
 	dashboardPermissions.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil)
 	folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore)
-	var expectedFolder *folder.Folder
-	if dash.FolderUID != "" || dash.FolderID != 0 { // nolint:staticcheck
-		expectedFolder = &folder.Folder{ID: folderID, UID: folderUID}
-	}
+	fStore := folderimpl.ProvideStore(sqlStore)
+	folderSvc := folderimpl.ProvideService(fStore, ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore,
+		folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), cfg, nil, tracing.InitializeTracerForTest())
+	_, err = folderSvc.Create(context.Background(), &folder.CreateFolderCommand{UID: folderUID, SignedInUser: &user, Title: folderUID + "-title"})
+	require.NoError(t, err)
 	service, err := dashboardservice.ProvideDashboardServiceImpl(
 		cfg, dashboardStore, folderStore,
 		features, folderPermissions, ac,
-		&foldertest.FakeService{ExpectedFolder: expectedFolder},
+		folderSvc,
 		folder.NewFakeStore(),
 		nil,
 		nil,
@@ -327,22 +328,24 @@ func createDashboard(t *testing.T, sqlStore db.DB, user user.SignedInUser, dash
 	return dashboard
 }
 
-func createFolder(t *testing.T, sc scenarioContext, title string) *folder.Folder {
+func createFolder(t *testing.T, sc scenarioContext, title string, folderSvc folder.Service) *folder.Folder {
 	t.Helper()
 
-	features := featuremgmt.WithFeatures()
-	cfg := setting.NewCfg()
-	ac := actest.FakeAccessControl{ExpectedEvaluate: true}
-	dashboardStore, err := database.ProvideDashboardStore(sc.sqlStore, cfg, features, tagimpl.ProvideService(sc.sqlStore))
-	require.NoError(t, err)
+	if folderSvc == nil {
+		features := featuremgmt.WithFeatures()
+		cfg := setting.NewCfg()
+		ac := actest.FakeAccessControl{ExpectedEvaluate: true}
+		dashboardStore, err := database.ProvideDashboardStore(sc.sqlStore, cfg, features, tagimpl.ProvideService(sc.sqlStore))
+		require.NoError(t, err)
 
-	folderStore := folderimpl.ProvideDashboardFolderStore(sc.sqlStore)
-	store := folderimpl.ProvideStore(sc.sqlStore)
-	s := folderimpl.ProvideService(store, ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sc.sqlStore,
-		features, supportbundlestest.NewFakeBundleService(), cfg, nil, tracing.InitializeTracerForTest())
-	t.Logf("Creating folder with title and UID %q", title)
+		folderStore := folderimpl.ProvideDashboardFolderStore(sc.sqlStore)
+		store := folderimpl.ProvideStore(sc.sqlStore)
+		folderSvc = folderimpl.ProvideService(store, ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sc.sqlStore,
+			features, supportbundlestest.NewFakeBundleService(), cfg, nil, tracing.InitializeTracerForTest())
+		t.Logf("Creating folder with title and UID %q", title)
+	}
 	ctx := identity.WithRequester(context.Background(), &sc.user)
-	folder, err := s.Create(ctx, &folder.CreateFolderCommand{
+	folder, err := folderSvc.Create(ctx, &folder.CreateFolderCommand{
 		OrgID: sc.user.OrgID, Title: title, UID: title, SignedInUser: &sc.user,
 	})
 	require.NoError(t, err)
@@ -399,15 +402,18 @@ func scenarioWithPanel(t *testing.T, desc string, fn func(t *testing.T, sc scena
 	folderPermissions := acmock.NewMockedPermissionsService()
 	dashboardPermissions := acmock.NewMockedPermissionsService()
 	folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore)
+	fStore := folderimpl.ProvideStore(sqlStore)
+	folderSvc := folderimpl.ProvideService(fStore, ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore,
+		folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), cfg, nil, tracing.InitializeTracerForTest())
 	dashboardService, svcErr := dashboardservice.ProvideDashboardServiceImpl(
 		cfg, dashboardStore, folderStore,
 		features, folderPermissions, ac,
-		foldertest.NewFakeService(), folder.NewFakeStore(),
+		folderSvc, fStore,
 		nil, nil, nil, nil, quotaService, nil,
 	)
 	require.NoError(t, svcErr)
 	dashboardService.RegisterDashboardPermissions(dashboardPermissions)
-	guardian.InitAccessControlGuardian(cfg, ac, dashboardService)
+	guardian.InitAccessControlGuardian(cfg, ac, dashboardService, folderSvc, log.NewNopLogger())
 
 	testScenario(t, desc, func(t *testing.T, sc scenarioContext) {
 		// nolint:staticcheck
@@ -462,23 +468,23 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo
 		folderPermissions.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil)
 		dashboardPermissions := acmock.NewMockedPermissionsService()
 		folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore)
+		fStore := folderimpl.ProvideStore(sqlStore)
+		folderSvc := folderimpl.ProvideService(fStore, ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore,
+			folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), cfg, nil, tracing.InitializeTracerForTest())
 		dashService, dashSvcErr := dashboardservice.ProvideDashboardServiceImpl(
 			cfg, dashboardStore, folderStore,
 			features, folderPermissions, ac,
-			foldertest.NewFakeService(), folder.NewFakeStore(),
+			folderSvc, fStore,
 			nil, nil, nil, nil, quotaService, nil,
 		)
 		require.NoError(t, dashSvcErr)
 		dashService.RegisterDashboardPermissions(dashboardPermissions)
-		guardian.InitAccessControlGuardian(cfg, ac, dashService)
-		fStore := folderimpl.ProvideStore(sqlStore)
-		folderSrv := folderimpl.ProvideService(fStore, ac, bus.ProvideBus(tracer), dashboardStore, folderStore, sqlStore,
-			features, supportbundlestest.NewFakeBundleService(), cfg, nil, tracing.InitializeTracerForTest())
+		guardian.InitAccessControlGuardian(cfg, ac, dashService, folderSvc, log.NewNopLogger())
 		service := LibraryElementService{
 			Cfg:           cfg,
 			features:      featuremgmt.WithFeatures(),
 			SQLStore:      sqlStore,
-			folderService: folderSrv,
+			folderService: folderSvc,
 		}
 
 		// deliberate difference between signed in user and user in db to make it crystal clear
@@ -510,7 +516,7 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo
 			},
 		}
 
-		sc.folder = createFolder(t, sc, "ScenarioFolder")
+		sc.folder = createFolder(t, sc, "ScenarioFolder", folderSvc)
 
 		fn(t, sc)
 	})