[nested folder]Add circular reference detection in create nested folder (#60163)

* add circulic reference detection in create neste folder

* redeclare mock

* add log for getHeight when depassing limit

pkg/services/folder/folderimpl/folder.go

@@ -543,14 +543,14 @@ func (s *Service) MakeUserAdmin(ctx context.Context, orgID int64, userID, folder
 
 func (s *Service) nestedFolderCreate(ctx context.Context, cmd *folder.CreateFolderCommand) (*folder.Folder, error) {
 	if cmd.ParentUID != "" {
-		if err := s.validateParent(ctx, cmd.OrgID, cmd.ParentUID); err != nil {
+		if err := s.validateParent(ctx, cmd.OrgID, cmd.ParentUID, cmd.UID); err != nil {
 			return nil, err
 		}
 	}
 	return s.store.Create(ctx, *cmd)
 }
 
-func (s *Service) validateParent(ctx context.Context, orgID int64, parentUID string) error {
+func (s *Service) validateParent(ctx context.Context, orgID int64, parentUID string, UID string) error {
 	ancestors, err := s.store.GetParents(ctx, folder.GetParentsQuery{UID: parentUID, OrgID: orgID})
 	if err != nil {
 		return fmt.Errorf("failed to get parents: %w", err)
@@ -560,6 +560,18 @@ func (s *Service) validateParent(ctx context.Context, orgID int64, parentUID str
 		return folder.ErrMaximumDepthReached
 	}
 
+	// Create folder under itself is not allowed
+	if parentUID == UID {
+		return folder.ErrCircularReference
+	}
+
+	// check there is no circular reference
+	for _, ancestor := range ancestors {
+		if ancestor.UID == UID {
+			return folder.ErrCircularReference
+		}
+	}
+
 	return nil
 }
 

pkg/services/folder/folderimpl/folder_test.go

@@ -464,11 +464,71 @@ func TestNestedFolderService(t *testing.T) {
 			require.True(t, store.CreateCalled)
 		})
 
+		t.Run("create without UID, no error", func(t *testing.T) {
+			// dashboard store & service commands that should be called.
+			dashStore = &dashboards.FakeDashboardStore{}
+			foldersvc.dashboardStore = dashStore
+			dashboardsvc.On("BuildSaveDashboardCommand",
+				mock.Anything, mock.AnythingOfType("*dashboards.SaveDashboardDTO"),
+				mock.AnythingOfType("bool"), mock.AnythingOfType("bool")).Return(&models.SaveDashboardCommand{}, nil)
+			dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("models.SaveDashboardCommand")).Return(&models.Dashboard{Uid: "newUID"}, nil)
+			dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil)
+			f, err := foldersvc.Create(context.Background(), &folder.CreateFolderCommand{
+				OrgID:        orgID,
+				Title:        "myFolder",
+				SignedInUser: usr,
+			})
+			require.NoError(t, err)
+			// CreateFolder should also call the folder store's create method.
+			require.True(t, store.CreateCalled)
+			require.Equal(t, "newUID", f.UID)
+		})
+
+		t.Run("create failed because of circular reference", func(t *testing.T) {
+			// dashboard store & service commands that should be called.
+			dashboardFolder := models.NewDashboardFolder("myFolder")
+			dashboardFolder.Id = rand.Int63()
+			dashboardFolder.Uid = "myFolder"
+			f := folder.FromDashboard(dashboardFolder)
+
+			dashStore = &dashboards.FakeDashboardStore{}
+			foldersvc.dashboardStore = dashStore
+			dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*models.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil)
+			dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("models.SaveDashboardCommand")).Return(dashboardFolder, nil)
+			dashStore.On("GetFolderByID", mock.Anything, orgID, dashboardFolder.Id).Return(f, nil)
+			var actualCmd *models.DeleteDashboardCommand
+			dashStore.On("DeleteDashboard", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
+				actualCmd = args.Get(1).(*models.DeleteDashboardCommand)
+			}).Return(nil).Once()
+
+			store.ExpectedParentFolders = []*folder.Folder{
+				{UID: "newFolder", ParentUID: "newFolder"},
+				{UID: "newFolder2", ParentUID: "newFolder2"},
+				{UID: "newFolder3", ParentUID: "newFolder3"},
+				{UID: "myFolder", ParentUID: "newFolder"},
+			}
+
+			cmd := folder.CreateFolderCommand{
+				ParentUID:    "myFolder1",
+				OrgID:        orgID,
+				Title:        "myFolder",
+				UID:          "myFolder",
+				SignedInUser: usr,
+			}
+			_, err := foldersvc.Create(context.Background(), &cmd)
+			require.Error(t, err, folder.ErrCircularReference)
+			// CreateFolder should also call the folder store's create method.
+			require.True(t, store.CreateCalled)
+			require.NotNil(t, actualCmd)
+		})
+
 		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{})
 
+			dashStore = &dashboards.FakeDashboardStore{}
+			foldersvc.dashboardStore = dashStore
 			// dashboard store & service commands that should be called.
 			dashboardsvc.On("BuildSaveDashboardCommand",
 				mock.Anything, mock.AnythingOfType("*dashboards.SaveDashboardDTO"),
@@ -595,6 +655,8 @@ func TestNestedFolderService(t *testing.T) {
 				guardian.New = g
 			})
 
+			dashStore = &dashboards.FakeDashboardStore{}
+			foldersvc.dashboardStore = dashStore
 			// dashboard store & service commands that should be called.
 			dashboardsvc.On("BuildSaveDashboardCommand",
 				mock.Anything, mock.AnythingOfType("*dashboards.SaveDashboardDTO"),

pkg/services/folder/folderimpl/sqlstore.go

@@ -268,7 +268,7 @@ func (ss *sqlStore) getParentsMySQL(ctx context.Context, cmd folder.GetParentsQu
 func (ss *sqlStore) GetHeight(ctx context.Context, foldrUID string, orgID int64, parentUID *string) (int, error) {
 	height := -1
 	queue := []string{foldrUID}
-	for len(queue) > 0 {
+	for len(queue) > 0 && height <= folder.MaxNestedFolderDepth {
 		length := len(queue)
 		height++
 		for i := 0; i < length; i++ {
@@ -286,5 +286,8 @@ func (ss *sqlStore) GetHeight(ctx context.Context, foldrUID string, orgID int64,
 			}
 		}
 	}
+	if height > folder.MaxNestedFolderDepth {
+		ss.log.Warn("folder height exceeds the maximum allowed depth, You might have a circular reference", "uid", foldrUID, "orgId", orgID, "maxDepth", folder.MaxNestedFolderDepth)
+	}
 	return height, nil
 }