RBAC: protect folder creation and moving (#64636)

* protect moving folders to a subfolder and creating folders in a subfolder

* folder update endpoint isn't used for folder parent update

* lint

* move permission check logic to services, fix tests

* linting

pkg/api/folder.go

@@ -186,7 +186,7 @@ func (hs *HTTPServer) setDefaultFolderPermissions(ctx context.Context, orgID int
 			})
 		}
 
-		if !isNested {
+		if !isNested || !hs.Features.IsEnabled(featuremgmt.FlagNestedFolders) {
 			permissions = append(permissions, []accesscontrol.SetResourcePermissionCommand{
 				{BuiltinRole: string(org.RoleEditor), Permission: dashboards.PERMISSION_EDIT.String()},
 				{BuiltinRole: string(org.RoleViewer), Permission: dashboards.PERMISSION_VIEW.String()},
@@ -209,9 +209,11 @@ func (hs *HTTPServer) MoveFolder(c *contextmodel.ReqContext) response.Response {
 		}
 		var theFolder *folder.Folder
 		var err error
+
 		if cmd.NewParentUID != "" {
 			cmd.OrgID = c.OrgID
 			cmd.UID = web.Params(c.Req)[":uid"]
+			cmd.SignedInUser = c.SignedInUser
 			theFolder, err = hs.folderService.Move(c.Req.Context(), &cmd)
 			if err != nil {
 				return response.Error(http.StatusInternalServerError, "update folder uid failed", err)
@@ -228,9 +230,6 @@ func (hs *HTTPServer) MoveFolder(c *contextmodel.ReqContext) response.Response {
 //
 // Update folder.
 //
-// If nested folders are enabled then it optionally expects a new parent folder UID that moves the folder and
-// includes it into the response.
-//
 // Responses:
 // 200: folderResponse
 // 400: badRequestError

pkg/api/folder_test.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -11,142 +12,270 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/api/dtos"
-	"github.com/grafana/grafana/pkg/api/response"
-	"github.com/grafana/grafana/pkg/api/routing"
-	"github.com/grafana/grafana/pkg/infra/db/dbtest"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
 	acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
-	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/services/dashboards"
 	"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/quota/quotatest"
 	"github.com/grafana/grafana/pkg/services/search/model"
-	"github.com/grafana/grafana/pkg/services/team/teamtest"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/web/webtest"
 )
 
-func TestFoldersAPIEndpoint(t *testing.T) {
+func TestFoldersCreateAPIEndpoint(t *testing.T) {
 	folderService := &foldertest.FakeService{}
+	setUpRBACGuardian(t)
 
-	t.Run("Given a correct request for creating a folder", func(t *testing.T) {
+	folderWithoutParentInput := "{ \"uid\": \"uid\", \"title\": \"Folder\"}"
-		cmd := folder.CreateFolderCommand{
-			UID:   "uid",
-			Title: "Folder",
-		}
 
-		folderService.ExpectedFolder = &folder.Folder{ID: 1, UID: "uid", Title: "Folder"}
+	type testCase struct {
+		description            string
+		expectedCode           int
+		expectedFolder         *folder.Folder
+		expectedFolderSvcError error
+		permissions            []accesscontrol.Permission
+		withNestedFolders      bool
+		input                  string
+	}
+	tcs := []testCase{
+		{
+			description:    "folder creation succeeds given the correct request for creating a folder",
+			input:          folderWithoutParentInput,
+			expectedCode:   http.StatusOK,
+			expectedFolder: &folder.Folder{ID: 1, UID: "uid", Title: "Folder"},
+			permissions:    []accesscontrol.Permission{{Action: dashboards.ActionFoldersCreate}},
+		},
+		{
+			description:  "folder creation fails without permissions to create a folder",
+			input:        folderWithoutParentInput,
+			expectedCode: http.StatusForbidden,
+			permissions:  []accesscontrol.Permission{},
+		},
+		{
+			description:            "folder creation fails given folder service error %s",
+			input:                  folderWithoutParentInput,
+			expectedCode:           http.StatusConflict,
+			expectedFolderSvcError: dashboards.ErrFolderWithSameUIDExists,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersCreate}},
+		},
+		{
+			description:            "folder creation fails given folder service error %s",
+			input:                  folderWithoutParentInput,
+			expectedCode:           http.StatusBadRequest,
+			expectedFolderSvcError: dashboards.ErrFolderTitleEmpty,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersCreate}},
+		},
+		{
+			description:            "folder creation fails given folder service error %s",
+			input:                  folderWithoutParentInput,
+			expectedCode:           http.StatusBadRequest,
+			expectedFolderSvcError: dashboards.ErrDashboardInvalidUid,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersCreate}},
+		},
+		{
+			description:            "folder creation fails given folder service error %s",
+			input:                  folderWithoutParentInput,
+			expectedCode:           http.StatusBadRequest,
+			expectedFolderSvcError: dashboards.ErrDashboardUidTooLong,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersCreate}},
+		},
+		{
+			description:            "folder creation fails given folder service error %s",
+			input:                  folderWithoutParentInput,
+			expectedCode:           http.StatusConflict,
+			expectedFolderSvcError: dashboards.ErrFolderSameNameExists,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersCreate}},
+		},
+		{
+			description:            "folder creation fails given folder service error %s",
+			input:                  folderWithoutParentInput,
+			expectedCode:           http.StatusForbidden,
+			expectedFolderSvcError: dashboards.ErrFolderAccessDenied,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersCreate}},
+		},
+		{
+			description:            "folder creation fails given folder service error %s",
+			input:                  folderWithoutParentInput,
+			expectedCode:           http.StatusNotFound,
+			expectedFolderSvcError: dashboards.ErrFolderNotFound,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersCreate}},
+		},
+		{
+			description:            "folder creation fails given folder service error %s",
+			input:                  folderWithoutParentInput,
+			expectedCode:           http.StatusPreconditionFailed,
+			expectedFolderSvcError: dashboards.ErrFolderVersionMismatch,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersCreate}},
+		},
+	}
 
-		createFolderScenario(t, "When calling POST on", "/api/folders", "/api/folders", folderService, cmd,
+	for _, tc := range tcs {
-			func(sc *scenarioContext) {
+		folderService.ExpectedFolder = tc.expectedFolder
-				callCreateFolder(sc)
+		folderService.ExpectedError = tc.expectedFolderSvcError
+		folderPermService := acmock.NewMockedPermissionsService()
+		folderPermService.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil)
 
-				folder := dtos.Folder{}
+		srv := SetupAPITestServer(t, func(hs *HTTPServer) {
-				err := json.NewDecoder(sc.resp.Body).Decode(&folder)
+			hs.Cfg = &setting.Cfg{
-				require.NoError(t, err)
+				RBACEnabled: true,
+			}
+
+			if tc.withNestedFolders {
+				hs.Features = featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)
+			}
+			hs.folderService = folderService
+			hs.folderPermissionsService = folderPermService
+			hs.accesscontrolService = actest.FakeService{}
+		})
+
+		t.Run(testDescription(tc.description, tc.expectedFolderSvcError), func(t *testing.T) {
+			input := strings.NewReader(tc.input)
+			req := srv.NewPostRequest("/api/folders", input)
+			req = webtest.RequestWithSignedInUser(req, userWithPermissions(1, tc.permissions))
+			resp, err := srv.SendJSON(req)
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedCode, resp.StatusCode)
+
+			folder := dtos.Folder{}
+			err = json.NewDecoder(resp.Body).Decode(&folder)
+			require.NoError(t, err)
+			require.NoError(t, resp.Body.Close())
+
+			if tc.expectedCode == http.StatusOK {
 				assert.Equal(t, int64(1), folder.Id)
 				assert.Equal(t, "uid", folder.Uid)
 				assert.Equal(t, "Folder", folder.Title)
-			})
+			}
-	})
-
-	t.Run("Given incorrect requests for creating a folder", func(t *testing.T) {
-		t.Cleanup(func() {
-			folderService.ExpectedError = nil
 		})
-		testCases := []struct {
+	}
-			Error              error
+}
-			ExpectedStatusCode int
-		}{
-			{Error: dashboards.ErrFolderWithSameUIDExists, ExpectedStatusCode: 409},
-			{Error: dashboards.ErrFolderTitleEmpty, ExpectedStatusCode: 400},
-			{Error: dashboards.ErrFolderSameNameExists, ExpectedStatusCode: 409},
-			{Error: dashboards.ErrDashboardInvalidUid, ExpectedStatusCode: 400},
-			{Error: dashboards.ErrDashboardUidTooLong, ExpectedStatusCode: 400},
-			{Error: dashboards.ErrFolderAccessDenied, ExpectedStatusCode: 403},
-			{Error: dashboards.ErrFolderNotFound, ExpectedStatusCode: 404},
-			{Error: dashboards.ErrFolderVersionMismatch, ExpectedStatusCode: 412},
-		}
 
-		cmd := folder.CreateFolderCommand{
+func TestFoldersUpdateAPIEndpoint(t *testing.T) {
-			UID:   "uid",
+	folderService := &foldertest.FakeService{}
-			Title: "Folder",
+	setUpRBACGuardian(t)
-		}
 
-		for _, tc := range testCases {
+	type testCase struct {
-			folderService.ExpectedError = tc.Error
+		description            string
+		expectedCode           int
+		expectedFolder         *folder.Folder
+		expectedFolderSvcError error
+		permissions            []accesscontrol.Permission
+	}
+	tcs := []testCase{
+		{
+			description:    "folder updating succeeds given the correct request and permissions to update a folder",
+			expectedCode:   http.StatusOK,
+			expectedFolder: &folder.Folder{ID: 1, UID: "uid", Title: "Folder upd"},
+			permissions:    []accesscontrol.Permission{{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll}},
+		},
+		{
+			description:  "folder updating fails without permissions to update a folder",
+			expectedCode: http.StatusForbidden,
+			permissions:  []accesscontrol.Permission{},
+		},
+		{
+			description:            "folder updating fails given folder service error %s",
+			expectedCode:           http.StatusConflict,
+			expectedFolderSvcError: dashboards.ErrFolderWithSameUIDExists,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll}},
+		},
+		{
+			description:            "folder updating fails given folder service error %s",
+			expectedCode:           http.StatusBadRequest,
+			expectedFolderSvcError: dashboards.ErrFolderTitleEmpty,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll}},
+		},
+		{
+			description:            "folder updating fails given folder service error %s",
+			expectedCode:           http.StatusBadRequest,
+			expectedFolderSvcError: dashboards.ErrDashboardInvalidUid,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll}},
+		},
+		{
+			description:            "folder updating fails given folder service error %s",
+			expectedCode:           http.StatusBadRequest,
+			expectedFolderSvcError: dashboards.ErrDashboardUidTooLong,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll}},
+		},
+		{
+			description:            "folder updating fails given folder service error %s",
+			expectedCode:           http.StatusConflict,
+			expectedFolderSvcError: dashboards.ErrFolderSameNameExists,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll}},
+		},
+		{
+			description:            "folder updating fails given folder service error %s",
+			expectedCode:           http.StatusForbidden,
+			expectedFolderSvcError: dashboards.ErrFolderAccessDenied,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll}},
+		},
+		{
+			description:            "folder updating fails given folder service error %s",
+			expectedCode:           http.StatusNotFound,
+			expectedFolderSvcError: dashboards.ErrFolderNotFound,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll}},
+		},
+		{
+			description:            "folder updating fails given folder service error %s",
+			expectedCode:           http.StatusPreconditionFailed,
+			expectedFolderSvcError: dashboards.ErrFolderVersionMismatch,
+			permissions:            []accesscontrol.Permission{{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll}},
+		},
+	}
 
-			createFolderScenario(t, fmt.Sprintf("Expect '%s' error when calling POST on", tc.Error.Error()),
+	for _, tc := range tcs {
-				"/api/folders", "/api/folders", folderService, cmd, func(sc *scenarioContext) {
+		folderService.ExpectedFolder = tc.expectedFolder
-					callCreateFolder(sc)
+		folderService.ExpectedError = tc.expectedFolderSvcError
-					assert.Equalf(t, tc.ExpectedStatusCode, sc.resp.Code, "Wrong status code for error %s", tc.Error)
-				})
-		}
-	})
 
-	t.Run("Given a correct request for updating a folder", func(t *testing.T) {
+		srv := SetupAPITestServer(t, func(hs *HTTPServer) {
-		title := "Folder upd"
+			hs.Cfg = &setting.Cfg{
-		cmd := folder.UpdateFolderCommand{
+				RBACEnabled: true,
-			NewTitle: &title,
+			}
-		}
+			hs.folderService = folderService
+		})
 
-		folderService.ExpectedFolder = &folder.Folder{ID: 1, UID: "uid", Title: "Folder upd"}
+		t.Run(testDescription(tc.description, tc.expectedFolderSvcError), func(t *testing.T) {
+			input := strings.NewReader("{ \"uid\": \"uid\", \"title\": \"Folder upd\" }")
+			req := srv.NewRequest(http.MethodPut, "/api/folders/uid", input)
+			req = webtest.RequestWithSignedInUser(req, userWithPermissions(1, tc.permissions))
+			resp, err := srv.SendJSON(req)
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedCode, resp.StatusCode)
 
-		updateFolderScenario(t, "When calling PUT on", "/api/folders/uid", "/api/folders/:uid", folderService, cmd,
+			folder := dtos.Folder{}
-			func(sc *scenarioContext) {
+			err = json.NewDecoder(resp.Body).Decode(&folder)
-				callUpdateFolder(sc)
+			require.NoError(t, err)
+			require.NoError(t, resp.Body.Close())
 
-				folder := dtos.Folder{}
+			if tc.expectedCode == http.StatusOK {
-				err := json.NewDecoder(sc.resp.Body).Decode(&folder)
-				require.NoError(t, err)
 				assert.Equal(t, int64(1), folder.Id)
 				assert.Equal(t, "uid", folder.Uid)
 				assert.Equal(t, "Folder upd", folder.Title)
-			})
+			}
-	})
+		})
+	}
+}
 
-	t.Run("Given incorrect requests for updating a folder", func(t *testing.T) {
+func testDescription(description string, expectedErr error) string {
-		testCases := []struct {
+	if expectedErr != nil {
-			Error              error
+		return fmt.Sprintf(description, expectedErr.Error())
-			ExpectedStatusCode int
+	} else {
-		}{
+		return description
-			{Error: dashboards.ErrFolderWithSameUIDExists, ExpectedStatusCode: 409},
+	}
-			{Error: dashboards.ErrFolderTitleEmpty, ExpectedStatusCode: 400},
-			{Error: dashboards.ErrFolderSameNameExists, ExpectedStatusCode: 409},
-			{Error: dashboards.ErrDashboardInvalidUid, ExpectedStatusCode: 400},
-			{Error: dashboards.ErrDashboardUidTooLong, ExpectedStatusCode: 400},
-			{Error: dashboards.ErrFolderAccessDenied, ExpectedStatusCode: 403},
-			{Error: dashboards.ErrFolderNotFound, ExpectedStatusCode: 404},
-			{Error: dashboards.ErrFolderVersionMismatch, ExpectedStatusCode: 412},
-		}
-
-		title := "Folder upd"
-		cmd := folder.UpdateFolderCommand{
-			NewTitle: &title,
-		}
-
-		for _, tc := range testCases {
-			folderService.ExpectedError = tc.Error
-			updateFolderScenario(t, fmt.Sprintf("Expect '%s' error when calling PUT on", tc.Error.Error()),
-				"/api/folders/uid", "/api/folders/:uid", folderService, cmd, func(sc *scenarioContext) {
-					callUpdateFolder(sc)
-					assert.Equalf(t, tc.ExpectedStatusCode, sc.resp.Code, "Wrong status code for %s", tc.Error)
-				})
-		}
-	})
 }
 
 func TestHTTPServer_FolderMetadata(t *testing.T) {
 	setUpRBACGuardian(t)
 	folderService := &foldertest.FakeService{}
 	server := SetupAPITestServer(t, func(hs *HTTPServer) {
+		hs.Cfg = &setting.Cfg{
+			RBACEnabled: true,
+		}
 		hs.folderService = folderService
-		hs.AccessControl = acmock.New()
 		hs.QuotaService = quotatest.New(false, nil)
 		hs.SearchService = &mockSearchService{
 			ExpectedResult: model.HitList{},
@@ -229,76 +358,53 @@ func TestHTTPServer_FolderMetadata(t *testing.T) {
 	})
 }
 
-func callCreateFolder(sc *scenarioContext) {
+func TestFolderMoveAPIEndpoint(t *testing.T) {
-	sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
+	folderService := &foldertest.FakeService{}
-}
-
-func createFolderScenario(t *testing.T, desc string, url string, routePattern string, folderService folder.Service,
-	cmd folder.CreateFolderCommand, fn scenarioFunc) {
 	setUpRBACGuardian(t)
-	t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
-		aclMockResp := []*dashboards.DashboardACLInfoDTO{}
-		teamSvc := &teamtest.FakeService{}
-		dashSvc := &dashboards.FakeDashboardService{}
-		qResult1 := aclMockResp
-		dashSvc.On("GetDashboardACLInfoList", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardACLInfoListQuery")).Return(qResult1, nil)
-		qResult := &dashboards.Dashboard{}
-		dashSvc.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(qResult, nil)
-		store := dbtest.NewFakeDB()
-		guardian.InitLegacyGuardian(setting.NewCfg(), store, dashSvc, teamSvc)
-		folderPermissions := acmock.NewMockedPermissionsService()
-		folderPermissions.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil)
-		hs := HTTPServer{
-			AccessControl:            acmock.New(),
-			folderService:            folderService,
-			Cfg:                      setting.NewCfg(),
-			Features:                 featuremgmt.WithFeatures(),
-			accesscontrolService:     actest.FakeService{},
-			folderPermissionsService: folderPermissions,
-		}
 
-		sc := setupScenarioContext(t, url)
+	type testCase struct {
-		sc.defaultHandler = routing.Wrap(func(c *contextmodel.ReqContext) response.Response {
+		description  string
-			c.Req.Body = mockRequestBody(cmd)
+		expectedCode int
-			c.Req.Header.Add("Content-Type", "application/json")
+		permissions  []accesscontrol.Permission
-			sc.context = c
+		newParentUid string
-			sc.context.SignedInUser = &user.SignedInUser{OrgID: testOrgID, UserID: testUserID}
+	}
+	tcs := []testCase{
+		{
+			description:  "can move folder to another folder with specific permissions",
+			newParentUid: "newParentUid",
+			expectedCode: http.StatusOK,
+			permissions: []accesscontrol.Permission{
+				{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID("uid")},
+				{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID("newParentUid")},
+			},
+		},
+		{
+			description:  "forbidden to move folder to another folder without the write access on the folder being moved",
+			newParentUid: "newParentUid",
+			expectedCode: http.StatusForbidden,
+			permissions: []accesscontrol.Permission{
+				{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID("newParentUid")},
+			},
+		},
+	}
 
-			return hs.CreateFolder(c)
+	for _, tc := range tcs {
+		srv := SetupAPITestServer(t, func(hs *HTTPServer) {
+			hs.Cfg = &setting.Cfg{
+				RBACEnabled: true,
+			}
+			hs.Features = featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)
+			hs.folderService = folderService
 		})
 
-		sc.m.Post(routePattern, sc.defaultHandler)
+		t.Run(tc.description, func(t *testing.T) {
-
+			input := strings.NewReader(fmt.Sprintf("{ \"parentUid\": \"%s\"}", tc.newParentUid))
-		fn(sc)
+			req := srv.NewRequest(http.MethodPost, "/api/folders/uid/move", input)
-	})
+			req = webtest.RequestWithSignedInUser(req, userWithPermissions(1, tc.permissions))
-}
+			resp, err := srv.SendJSON(req)
-
+			require.NoError(t, err)
-func callUpdateFolder(sc *scenarioContext) {
+			require.Equal(t, tc.expectedCode, resp.StatusCode)
-	sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec()
+			require.NoError(t, resp.Body.Close())
-}
-
-func updateFolderScenario(t *testing.T, desc string, url string, routePattern string, folderService folder.Service,
-	cmd folder.UpdateFolderCommand, fn scenarioFunc) {
-	setUpRBACGuardian(t)
-	t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
-		hs := HTTPServer{
-			Cfg:           setting.NewCfg(),
-			AccessControl: acmock.New(),
-			folderService: folderService,
-		}
-
-		sc := setupScenarioContext(t, url)
-		sc.defaultHandler = routing.Wrap(func(c *contextmodel.ReqContext) response.Response {
-			c.Req.Body = mockRequestBody(cmd)
-			c.Req.Header.Add("Content-Type", "application/json")
-			sc.context = c
-			sc.context.SignedInUser = &user.SignedInUser{OrgID: testOrgID, UserID: testUserID}
-
-			return hs.UpdateFolder(c)
 		})
-
+	}
-		sc.m.Put(routePattern, sc.defaultHandler)
-
-		fn(sc)
-	})
 }

pkg/services/folder/folderimpl/folder.go

@@ -213,6 +213,22 @@ func (s *Service) getFolderByTitle(ctx context.Context, orgID int64, title strin
 func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (*folder.Folder, error) {
 	logger := s.log.FromContext(ctx)
 
+	if cmd.SignedInUser == nil {
+		return nil, folder.ErrBadRequest.Errorf("missing signed in user")
+	}
+
+	if !s.accessControl.IsDisabled() && s.features.IsEnabled(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))
+		hasAccess, evalErr := s.accessControl.Evaluate(ctx, cmd.SignedInUser, evaluator)
+		if evalErr != nil {
+			return nil, evalErr
+		}
+		if !hasAccess {
+			return nil, dashboards.ErrFolderAccessDenied
+		}
+	}
+
 	dashFolder := dashboards.NewDashboardFolder(cmd.Title)
 	dashFolder.OrgID = cmd.OrgID
 
@@ -223,9 +239,6 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
 
 	dashFolder.SetUID(trimmedUID)
 
-	if cmd.SignedInUser == nil {
-		return nil, folder.ErrBadRequest.Errorf("missing signed in user")
-	}
 	user := cmd.SignedInUser
 	userID := user.UserID
 	if userID == 0 {
@@ -478,15 +491,33 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol
 		return nil, folder.ErrBadRequest.Errorf("missing signed in user")
 	}
 
-	g, err := guardian.NewByUID(ctx, cmd.UID, cmd.OrgID, cmd.SignedInUser)
+	// Check that the user is allowed to move the folder to the destination folder
-	if err != nil {
+	if !s.accessControl.IsDisabled() {
-		return nil, err
+		var evaluator accesscontrol.Evaluator
-	}
+		if cmd.NewParentUID != "" {
-	if canSave, err := g.CanSave(); err != nil || !canSave {
+			evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(cmd.NewParentUID))
-		if err != nil {
+		} else {
-			return nil, toFolderError(err)
+			// Evaluate folder creation permission when moving folder to the root level
+			evaluator = accesscontrol.EvalPermission(dashboards.ActionFoldersCreate)
+		}
+		hasAccess, evalErr := s.accessControl.Evaluate(ctx, cmd.SignedInUser, evaluator)
+		if evalErr != nil {
+			return nil, evalErr
+		}
+		if !hasAccess {
+			return nil, dashboards.ErrFolderAccessDenied
+		}
+	} else {
+		g, err := guardian.NewByUID(ctx, cmd.UID, cmd.OrgID, cmd.SignedInUser)
+		if err != nil {
+			return nil, err
+		}
+		if canSave, err := g.CanSave(); err != nil || !canSave {
+			if err != nil {
+				return nil, toFolderError(err)
+			}
+			return nil, dashboards.ErrFolderAccessDenied
 		}
-		return nil, dashboards.ErrFolderAccessDenied
 	}
 
 	// here we get the folder, we need to get the height of current folder
@@ -634,25 +665,11 @@ func (s *Service) BuildSaveDashboardCommand(ctx context.Context, dto *dashboards
 		return nil, dashboards.ErrDashboardUidTooLong
 	}
 
-	isParentFolderChanged, err := s.dashboardStore.ValidateDashboardBeforeSave(ctx, dash, dto.Overwrite)
+	_, err := s.dashboardStore.ValidateDashboardBeforeSave(ctx, dash, dto.Overwrite)
 	if err != nil {
 		return nil, err
 	}
 
-	if isParentFolderChanged {
-		// Check that the user is allowed to add a dashboard to the folder
-		guardian, err := guardian.NewByDashboard(ctx, dash, dto.OrgID, dto.User)
-		if err != nil {
-			return nil, err
-		}
-		if canSave, err := guardian.CanCreate(dash.FolderID, dash.IsFolder); err != nil || !canSave {
-			if err != nil {
-				return nil, err
-			}
-			return nil, dashboards.ErrDashboardUpdateAccessDenied
-		}
-	}
-
 	guard, err := getGuardianForSavePermissionCheck(ctx, dash, dto.User)
 	if err != nil {
 		return nil, err

pkg/services/folder/folderimpl/folder_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/infra/tracing"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
 	acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
 	"github.com/grafana/grafana/pkg/services/dashboards"
@@ -61,7 +62,7 @@ func TestIntegrationFolderService(t *testing.T) {
 		folderStore := foldertest.NewFakeFolderStore(t)
 
 		cfg := setting.NewCfg()
-		cfg.RBACEnabled = false
+		cfg.RBACEnabled = true
 		features := featuremgmt.WithFeatures()
 
 		service := &Service{
@@ -73,6 +74,7 @@ func TestIntegrationFolderService(t *testing.T) {
 			features:             features,
 			bus:                  bus.ProvideBus(tracing.InitializeTracerForTest()),
 			db:                   db,
+			accessControl:        acimpl.ProvideAccessControl(cfg),
 		}
 
 		t.Run("Given user has no permissions", func(t *testing.T) {
@@ -168,6 +170,7 @@ func TestIntegrationFolderService(t *testing.T) {
 		t.Run("Given user has permission to save", func(t *testing.T) {
 			origNewGuardian := guardian.New
 			guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: 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")
@@ -342,9 +345,12 @@ func TestIntegrationDeleteNestedFolders(t *testing.T) {
 		features:             featuresFlagOn,
 		bus:                  bus.ProvideBus(tracing.InitializeTracerForTest()),
 		db:                   db,
+		accessControl:        acimpl.ProvideAccessControl(cfg),
 	}
 
-	signedInUser := user.SignedInUser{UserID: 1, OrgID: orgID}
+	signedInUser := user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{
+		orgID: {dashboards.ActionFoldersCreate: {}, dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersAll}},
+	}}
 	createCmd := folder.CreateFolderCommand{
 		OrgID:        orgID,
 		ParentUID:    "",
@@ -355,6 +361,7 @@ func TestIntegrationDeleteNestedFolders(t *testing.T) {
 		origNewGuardian := guardian.New
 		guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true, CanViewValue: true})
 
+		serviceWithFlagOn.store = nestedFolderStore
 		ancestorUIDs := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, 3, "", createCmd)
 
 		deleteCmd := folder.DeleteFolderCommand{
@@ -450,6 +457,7 @@ func TestNestedFolderServiceFeatureToggle(t *testing.T) {
 		dashboardFolderStore: dashboardFolderStore,
 		features:             featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
 		log:                  log.New("test-folder-service"),
+		accessControl:        acimpl.ProvideAccessControl(cfg),
 	}
 	t.Run("create folder", func(t *testing.T) {
 		nestedFolderStore.ExpectedFolder = &folder.Folder{ParentUID: util.GenerateShortUID()}
@@ -479,7 +487,7 @@ func TestNestedFolderService(t *testing.T) {
 
 			nestedFolderStore := NewFakeStore()
 
-			folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures(), nil, dbtest.NewFakeDB())
+			folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures(), acimpl.ProvideAccessControl(setting.NewCfg()), dbtest.NewFakeDB())
 			_, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
 				OrgID:        orgID,
 				Title:        "myFolder",
@@ -493,7 +501,7 @@ func TestNestedFolderService(t *testing.T) {
 	})
 
 	t.Run("with nested folder feature flag on", func(t *testing.T) {
-		t.Run("create, no error", func(t *testing.T) {
+		t.Run("Should be able to create a nested folder under the root", func(t *testing.T) {
 			g := guardian.New
 			guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true})
 			t.Cleanup(func() {
@@ -510,9 +518,7 @@ func TestNestedFolderService(t *testing.T) {
 
 			nestedFolderStore := NewFakeStore()
 
-			folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{
+			folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), acimpl.ProvideAccessControl(setting.NewCfg()), dbtest.NewFakeDB())
-				ExpectedEvaluate: true,
-			}, dbtest.NewFakeDB())
 			_, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
 				OrgID:        orgID,
 				Title:        "myFolder",
@@ -524,6 +530,71 @@ func TestNestedFolderService(t *testing.T) {
 			require.True(t, nestedFolderStore.CreateCalled)
 		})
 
+		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"
+
+			tempUser := &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}}
+			tempUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("wrong_uid")}}
+
+			// 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)
+			dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(&dashboards.Dashboard{}, nil)
+
+			folderSvc := setup(t, dashStore, nil, nil, featuremgmt.WithFeatures("nestedFolders"), acimpl.ProvideAccessControl(setting.NewCfg()), dbtest.NewFakeDB())
+			_, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
+				OrgID:        orgID,
+				Title:        dash.Title,
+				UID:          dash.UID,
+				SignedInUser: tempUser,
+				ParentUID:    "some_parent",
+			})
+			require.ErrorIs(t, err, dashboards.ErrFolderAccessDenied)
+		})
+
+		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"
+
+			// 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)
+			dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(&dashboards.Dashboard{}, nil)
+
+			dashboardFolderStore := foldertest.NewFakeFolderStore(t)
+			dashboardFolderStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).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")}}
+
+			nestedFolderStore := NewFakeStore()
+			folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), acimpl.ProvideAccessControl(setting.NewCfg()), dbtest.NewFakeDB())
+			_, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
+				OrgID:        orgID,
+				Title:        dash.Title,
+				UID:          dash.UID,
+				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})
@@ -646,56 +717,22 @@ func TestNestedFolderService(t *testing.T) {
 			require.NotNil(t, actualCmd)
 		})
 
-		t.Run("move, no view permission should fail", func(t *testing.T) {
+		t.Run("move without the right permissions should fail", func(t *testing.T) {
-			// This test creates and deletes the dashboard, so needs some extra setup.
-			g := guardian.New
-			guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanViewValue: false})
-			t.Cleanup(func() {
-				guardian.New = g
-			})
-
 			dashStore := &dashboards.FakeDashboardStore{}
 			dashboardFolderStore := foldertest.NewFakeFolderStore(t)
 
 			nestedFolderStore := NewFakeStore()
 			nestedFolderStore.ExpectedFolder = &folder.Folder{UID: "myFolder", ParentUID: "newFolder"}
 
-			folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{
+			nestedFolderUser := &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}}
-				ExpectedEvaluate: true,
+			nestedFolderUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("wrong_uid")}}
-			}, dbtest.NewFakeDB())
+
-			_, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: usr})
+			folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), acimpl.ProvideAccessControl(setting.NewCfg()), dbtest.NewFakeDB())
-			require.Error(t, err, dashboards.ErrFolderAccessDenied)
+			_, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: nestedFolderUser})
+			require.ErrorIs(t, err, dashboards.ErrFolderAccessDenied)
 		})
 
-		t.Run("move, no save permission should fail", func(t *testing.T) {
+		t.Run("move with the right permissions succeeds", func(t *testing.T) {
-			// This test creates and deletes the dashboard, so needs some extra setup.
-			g := guardian.New
-			guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: false, CanViewValue: true})
-			t.Cleanup(func() {
-				guardian.New = g
-			})
-
-			dashStore := &dashboards.FakeDashboardStore{}
-			dashboardFolderStore := foldertest.NewFakeFolderStore(t)
-
-			nestedFolderStore := NewFakeStore()
-			nestedFolderStore.ExpectedFolder = &folder.Folder{UID: "myFolder", ParentUID: "newFolder"}
-
-			folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{
-				ExpectedEvaluate: true,
-			}, dbtest.NewFakeDB())
-			_, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: usr})
-			require.Error(t, err, dashboards.ErrFolderAccessDenied)
-		})
-
-		t.Run("move, no error", 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, CanViewValue: true})
-			t.Cleanup(func() {
-				guardian.New = g
-			})
-
 			dashStore := &dashboards.FakeDashboardStore{}
 			dashboardFolderStore := foldertest.NewFakeFolderStore(t)
 
@@ -707,10 +744,47 @@ func TestNestedFolderService(t *testing.T) {
 				{UID: "newFolder3", ParentUID: "newFolder3"},
 			}
 
-			folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{
+			nestedFolderUser := &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}}
-				ExpectedEvaluate: true,
+			nestedFolderUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("newFolder")}}
-			}, dbtest.NewFakeDB())
+
-			f, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: usr})
+			folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), acimpl.ProvideAccessControl(setting.NewCfg()), dbtest.NewFakeDB())
+			f, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: nestedFolderUser})
+			require.NoError(t, err)
+			require.NotNil(t, f)
+		})
+
+		t.Run("move to the root folder without folder creation permissions fails", func(t *testing.T) {
+			dashStore := &dashboards.FakeDashboardStore{}
+			dashboardFolderStore := foldertest.NewFakeFolderStore(t)
+
+			nestedFolderStore := NewFakeStore()
+			nestedFolderStore.ExpectedFolder = &folder.Folder{UID: "myFolder", ParentUID: "newFolder"}
+
+			nestedFolderUser := &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}}
+			nestedFolderUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("")}}
+
+			folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), acimpl.ProvideAccessControl(setting.NewCfg()), dbtest.NewFakeDB())
+			_, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "", OrgID: orgID, SignedInUser: nestedFolderUser})
+			require.Error(t, err, dashboards.ErrFolderAccessDenied)
+		})
+
+		t.Run("move to the root folder with folder creation permissions succeeds", func(t *testing.T) {
+			dashStore := &dashboards.FakeDashboardStore{}
+			dashboardFolderStore := foldertest.NewFakeFolderStore(t)
+
+			nestedFolderStore := NewFakeStore()
+			nestedFolderStore.ExpectedFolder = &folder.Folder{UID: "myFolder", ParentUID: "newFolder"}
+			nestedFolderStore.ExpectedParentFolders = []*folder.Folder{
+				{UID: "newFolder", ParentUID: "newFolder"},
+				{UID: "newFolder2", ParentUID: "newFolder2"},
+				{UID: "newFolder3", ParentUID: "newFolder3"},
+			}
+
+			nestedFolderUser := &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}}
+			nestedFolderUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersCreate: {}}
+
+			folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), acimpl.ProvideAccessControl(setting.NewCfg()), dbtest.NewFakeDB())
+			f, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "", OrgID: orgID, SignedInUser: nestedFolderUser})
 			require.NoError(t, err)
 			require.NotNil(t, f)
 		})