diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index 9fefe7383e7..56cc6e4a78b 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/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 } diff --git a/pkg/services/folder/folderimpl/folder_test.go b/pkg/services/folder/folderimpl/folder_test.go index 06e23d061e8..ea55e054ae9 100644 --- a/pkg/services/folder/folderimpl/folder_test.go +++ b/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"), diff --git a/pkg/services/folder/folderimpl/sqlstore.go b/pkg/services/folder/folderimpl/sqlstore.go index d66c35894a0..2b3e334a025 100644 --- a/pkg/services/folder/folderimpl/sqlstore.go +++ b/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 }