Nested Folders: Support getting of nested folder in folder service wh… (#58597)

* Nested Folders: Support getting of nested folder in folder service when feature flag is set

* Fix lint

* Fix some tests

* Fix ngalert test

* ngalert fix

* Fix API tests

* Fix some tests and lint

* Fix lint 2

* Fix library elements and panels

* Add access control to get folder

* Cleanup and minor test change
This commit is contained in:
idafurjes
2022-11-11 14:28:24 +01:00
committed by GitHub
parent 88a829e103
commit 080ea88af7
39 changed files with 372 additions and 420 deletions

View File

@@ -34,6 +34,7 @@ type Service struct {
searchService *search.SearchService
features *featuremgmt.FeatureManager
permissions accesscontrol.FolderPermissionsService
accessControl accesscontrol.AccessControl
// bus is currently used to publish events that cause scheduler to update rules.
bus bus.Bus
@@ -62,10 +63,41 @@ func ProvideService(
searchService: searchService,
features: features,
permissions: folderPermissionsService,
accessControl: ac,
bus: bus,
}
}
func (s *Service) Get(ctx context.Context, cmd *folder.GetFolderQuery) (*folder.Folder, error) {
user, err := appcontext.User(ctx)
if err != nil {
return nil, err
}
if s.cfg.IsFeatureToggleEnabled(featuremgmt.FlagNestedFolders) {
if ok, err := s.accessControl.Evaluate(ctx, user, accesscontrol.EvalPermission(
dashboards.ActionFoldersRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(*cmd.UID),
)); !ok {
if err != nil {
return nil, toFolderError(err)
}
return nil, dashboards.ErrFolderAccessDenied
}
return s.store.Get(ctx, *cmd)
}
switch {
case cmd.UID != nil:
return s.getFolderByUID(ctx, user, cmd.OrgID, *cmd.UID)
case cmd.ID != nil:
return s.getFolderByID(ctx, user, *cmd.ID, cmd.OrgID)
case cmd.Title != nil:
return s.getFolderByTitle(ctx, user, cmd.OrgID, *cmd.Title)
default:
return nil, folder.ErrBadRequest.Errorf("either on of UID, ID, Title fields must be present")
}
}
func (s *Service) GetFolders(ctx context.Context, user *user.SignedInUser, orgID int64, limit int64, page int64) ([]*models.Folder, error) {
searchQuery := search.Query{
SignedInUser: user,
@@ -95,9 +127,9 @@ func (s *Service) GetFolders(ctx context.Context, user *user.SignedInUser, orgID
return folders, nil
}
func (s *Service) GetFolderByID(ctx context.Context, user *user.SignedInUser, id int64, orgID int64) (*models.Folder, error) {
func (s *Service) getFolderByID(ctx context.Context, user *user.SignedInUser, id int64, orgID int64) (*folder.Folder, error) {
if id == 0 {
return &models.Folder{Id: id, Title: "General"}, nil
return &folder.Folder{ID: id, Title: "General"}, nil
}
dashFolder, err := s.dashboardStore.GetFolderByID(ctx, orgID, id)
@@ -105,7 +137,7 @@ func (s *Service) GetFolderByID(ctx context.Context, user *user.SignedInUser, id
return nil, err
}
g := guardian.New(ctx, dashFolder.Id, orgID, user)
g := guardian.New(ctx, dashFolder.ID, orgID, user)
if canView, err := g.CanView(); err != nil || !canView {
if err != nil {
return nil, toFolderError(err)
@@ -116,13 +148,13 @@ func (s *Service) GetFolderByID(ctx context.Context, user *user.SignedInUser, id
return dashFolder, nil
}
func (s *Service) GetFolderByUID(ctx context.Context, user *user.SignedInUser, orgID int64, uid string) (*models.Folder, error) {
func (s *Service) getFolderByUID(ctx context.Context, user *user.SignedInUser, orgID int64, uid string) (*folder.Folder, error) {
dashFolder, err := s.dashboardStore.GetFolderByUID(ctx, orgID, uid)
if err != nil {
return nil, err
}
g := guardian.New(ctx, dashFolder.Id, orgID, user)
g := guardian.New(ctx, dashFolder.ID, orgID, user)
if canView, err := g.CanView(); err != nil || !canView {
if err != nil {
return nil, toFolderError(err)
@@ -133,13 +165,13 @@ func (s *Service) GetFolderByUID(ctx context.Context, user *user.SignedInUser, o
return dashFolder, nil
}
func (s *Service) GetFolderByTitle(ctx context.Context, user *user.SignedInUser, orgID int64, title string) (*models.Folder, error) {
func (s *Service) getFolderByTitle(ctx context.Context, user *user.SignedInUser, orgID int64, title string) (*folder.Folder, error) {
dashFolder, err := s.dashboardStore.GetFolderByTitle(ctx, orgID, title)
if err != nil {
return nil, err
}
g := guardian.New(ctx, dashFolder.Id, orgID, user)
g := guardian.New(ctx, dashFolder.ID, orgID, user)
if canView, err := g.CanView(); err != nil || !canView {
if err != nil {
return nil, toFolderError(err)
@@ -189,7 +221,7 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
return nil, toFolderError(err)
}
var createdFolder *models.Folder
var createdFolder *folder.Folder
createdFolder, err = s.dashboardStore.GetFolderByID(ctx, cmd.OrgID, dash.Id)
if err != nil {
return nil, err
@@ -209,9 +241,9 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
{BuiltinRole: string(org.RoleViewer), Permission: models.PERMISSION_VIEW.String()},
}...)
_, permissionErr = s.permissions.SetPermissions(ctx, cmd.OrgID, createdFolder.Uid, permissions...)
_, permissionErr = s.permissions.SetPermissions(ctx, cmd.OrgID, createdFolder.UID, permissions...)
} else if s.cfg.EditorsCanAdmin && user.IsRealUser() && !user.IsAnonymous {
permissionErr = s.MakeUserAdmin(ctx, cmd.OrgID, userID, createdFolder.Id, true)
permissionErr = s.MakeUserAdmin(ctx, cmd.OrgID, userID, createdFolder.ID, true)
}
if permissionErr != nil {
@@ -242,7 +274,7 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
// We'll log the error and also roll back the previously-created
// (legacy) folder.
s.log.Error("error saving folder to nested folder store", err)
err = s.DeleteFolder(ctx, &folder.DeleteFolderCommand{UID: createdFolder.Uid, OrgID: cmd.OrgID, ForceDeleteRules: true})
err = s.DeleteFolder(ctx, &folder.DeleteFolderCommand{UID: createdFolder.UID, OrgID: cmd.OrgID, ForceDeleteRules: true})
if err != nil {
s.log.Error("error deleting folder after failed save to nested folder store", err)
}
@@ -254,7 +286,7 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
return folder.FromDashboard(dash), nil
}
func (s *Service) Update(ctx context.Context, user *user.SignedInUser, orgID int64, existingUid string, cmd *models.UpdateFolderCommand) (*models.Folder, error) {
func (s *Service) Update(ctx context.Context, user *user.SignedInUser, orgID int64, existingUid string, cmd *models.UpdateFolderCommand) (*folder.Folder, error) {
foldr, err := s.legacyUpdate(ctx, user, orgID, existingUid, cmd)
if err != nil {
return nil, err
@@ -276,7 +308,7 @@ func (s *Service) Update(ctx context.Context, user *user.SignedInUser, orgID int
if err != nil {
return nil, err
}
_, err = s.store.Update(ctx, folder.UpdateFolderCommand{
foldr, err := s.store.Update(ctx, folder.UpdateFolderCommand{
Folder: getFolder,
NewUID: &cmd.Uid,
NewTitle: &cmd.Title,
@@ -285,11 +317,12 @@ func (s *Service) Update(ctx context.Context, user *user.SignedInUser, orgID int
if err != nil {
return nil, err
}
return foldr, nil
}
return foldr, nil
}
func (s *Service) legacyUpdate(ctx context.Context, user *user.SignedInUser, orgID int64, existingUid string, cmd *models.UpdateFolderCommand) (*models.Folder, error) {
func (s *Service) legacyUpdate(ctx context.Context, user *user.SignedInUser, orgID int64, existingUid string, cmd *models.UpdateFolderCommand) (*folder.Folder, error) {
query := models.GetDashboardQuery{OrgId: orgID, Uid: existingUid}
_, err := s.dashboardStore.GetDashboard(ctx, &query)
if err != nil {
@@ -322,7 +355,7 @@ func (s *Service) legacyUpdate(ctx context.Context, user *user.SignedInUser, org
return nil, toFolderError(err)
}
var foldr *models.Folder
var foldr *folder.Folder
foldr, err = s.dashboardStore.GetFolderByID(ctx, orgID, dash.Id)
if err != nil {
return nil, err
@@ -359,7 +392,7 @@ func (s *Service) DeleteFolder(ctx context.Context, cmd *folder.DeleteFolderComm
return err
}
guard := guardian.New(ctx, dashFolder.Id, cmd.OrgID, user)
guard := guardian.New(ctx, dashFolder.ID, cmd.OrgID, user)
if canSave, err := guard.CanDelete(); err != nil || !canSave {
if err != nil {
return toFolderError(err)
@@ -367,7 +400,7 @@ func (s *Service) DeleteFolder(ctx context.Context, cmd *folder.DeleteFolderComm
return dashboards.ErrFolderAccessDenied
}
deleteCmd := models.DeleteDashboardCommand{OrgId: cmd.OrgID, Id: dashFolder.Id, ForceDeleteFolderRules: cmd.ForceDeleteRules}
deleteCmd := models.DeleteDashboardCommand{OrgId: cmd.OrgID, Id: dashFolder.ID, ForceDeleteFolderRules: cmd.ForceDeleteRules}
if err := s.dashboardStore.DeleteDashboard(ctx, &deleteCmd); err != nil {
return toFolderError(err)
@@ -416,12 +449,6 @@ func (s *Service) Delete(ctx context.Context, cmd *folder.DeleteFolderCommand) e
return nil
}
func (s *Service) Get(ctx context.Context, cmd *folder.GetFolderQuery) (*folder.Folder, error) {
// check the flag, if old - do whatever did before
// for new only the store
return s.store.Get(ctx, *cmd)
}
func (s *Service) GetParents(ctx context.Context, cmd *folder.GetParentsQuery) ([]*folder.Folder, error) {
// check the flag, if old - do whatever did before
// for new only the store

View File

@@ -15,6 +15,7 @@ import (
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/dashboards"
dashboardsvc "github.com/grafana/grafana/pkg/services/dashboards/service"
@@ -79,26 +80,26 @@ func TestIntegrationFolderService(t *testing.T) {
folderId := rand.Int63()
folderUID := util.GenerateShortUID()
f := models.NewFolder("Folder")
f.Id = folderId
f.Uid = folderUID
f := folder.NewFolder("Folder", "")
f.ID = folderId
f.UID = folderUID
dashStore.On("GetFolderByID", mock.Anything, orgID, folderId).Return(f, nil)
dashStore.On("GetFolderByUID", mock.Anything, orgID, folderUID).Return(f, nil)
t.Run("When get folder by id should return access denied error", func(t *testing.T) {
_, err := service.GetFolderByID(context.Background(), usr, folderId, orgID)
_, err := service.getFolderByID(context.Background(), usr, folderId, orgID)
require.Equal(t, err, dashboards.ErrFolderAccessDenied)
})
t.Run("When get folder by id, with id = 0 should return default folder", func(t *testing.T) {
folder, err := service.GetFolderByID(context.Background(), usr, 0, orgID)
foldr, err := service.getFolderByID(context.Background(), usr, 0, orgID)
require.NoError(t, err)
require.Equal(t, folder, &models.Folder{Id: 0, Title: "General"})
require.Equal(t, foldr, &folder.Folder{ID: 0, Title: "General"})
})
t.Run("When get folder by uid should return access denied error", func(t *testing.T) {
_, err := service.GetFolderByUID(context.Background(), usr, orgID, folderUID)
_, err := service.getFolderByUID(context.Background(), usr, orgID, folderUID)
require.Equal(t, err, dashboards.ErrFolderAccessDenied)
})
@@ -157,7 +158,7 @@ func TestIntegrationFolderService(t *testing.T) {
t.Run("When creating folder should not return access denied error", func(t *testing.T) {
dash := models.NewDashboardFolder("Test-Folder")
dash.Id = rand.Int63()
f := models.DashboardToFolder(dash)
f := folder.FromDashboard(dash)
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(dash, nil).Once()
@@ -170,7 +171,7 @@ func TestIntegrationFolderService(t *testing.T) {
UID: "someuid",
})
require.NoError(t, err)
require.Equal(t, f, actualFolder.ToLegacyModel())
require.Equal(t, f, actualFolder)
})
t.Run("When creating folder should return error if uid is general", func(t *testing.T) {
@@ -190,7 +191,7 @@ func TestIntegrationFolderService(t *testing.T) {
dashboardFolder := models.NewDashboardFolder("Folder")
dashboardFolder.Id = rand.Int63()
dashboardFolder.Uid = util.GenerateShortUID()
f := models.DashboardToFolder(dashboardFolder)
f := folder.FromDashboard(dashboardFolder)
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)
@@ -207,10 +208,10 @@ func TestIntegrationFolderService(t *testing.T) {
})
t.Run("When deleting folder by uid should not return access denied error", func(t *testing.T) {
f := models.NewFolder(util.GenerateShortUID())
f.Id = rand.Int63()
f.Uid = util.GenerateShortUID()
dashStore.On("GetFolderByUID", mock.Anything, orgID, f.Uid).Return(f, nil)
f := folder.NewFolder(util.GenerateShortUID(), "")
f.ID = rand.Int63()
f.UID = util.GenerateShortUID()
dashStore.On("GetFolderByUID", mock.Anything, orgID, f.UID).Return(f, nil)
var actualCmd *models.DeleteDashboardCommand
dashStore.On("DeleteDashboard", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
@@ -221,13 +222,13 @@ func TestIntegrationFolderService(t *testing.T) {
ctx := context.Background()
ctx = appcontext.WithUser(ctx, usr)
err := service.DeleteFolder(ctx, &folder.DeleteFolderCommand{
UID: f.Uid,
UID: f.UID,
OrgID: orgID,
ForceDeleteRules: expectedForceDeleteRules,
})
require.NoError(t, err)
require.NotNil(t, actualCmd)
require.Equal(t, f.Id, actualCmd.Id)
require.Equal(t, f.ID, actualCmd.Id)
require.Equal(t, orgID, actualCmd.OrgId)
require.Equal(t, expectedForceDeleteRules, actualCmd.ForceDeleteFolderRules)
})
@@ -242,33 +243,33 @@ func TestIntegrationFolderService(t *testing.T) {
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanViewValue: true})
t.Run("When get folder by id should return folder", func(t *testing.T) {
expected := models.NewFolder(util.GenerateShortUID())
expected.Id = rand.Int63()
expected := folder.NewFolder(util.GenerateShortUID(), "")
expected.ID = rand.Int63()
dashStore.On("GetFolderByID", mock.Anything, orgID, expected.Id).Return(expected, nil)
dashStore.On("GetFolderByID", mock.Anything, orgID, expected.ID).Return(expected, nil)
actual, err := service.GetFolderByID(context.Background(), usr, expected.Id, orgID)
actual, err := service.getFolderByID(context.Background(), usr, expected.ID, orgID)
require.Equal(t, expected, actual)
require.NoError(t, err)
})
t.Run("When get folder by uid should return folder", func(t *testing.T) {
expected := models.NewFolder(util.GenerateShortUID())
expected.Uid = util.GenerateShortUID()
expected := folder.NewFolder(util.GenerateShortUID(), "")
expected.UID = util.GenerateShortUID()
dashStore.On("GetFolderByUID", mock.Anything, orgID, expected.Uid).Return(expected, nil)
dashStore.On("GetFolderByUID", mock.Anything, orgID, expected.UID).Return(expected, nil)
actual, err := service.GetFolderByUID(context.Background(), usr, orgID, expected.Uid)
actual, err := service.getFolderByUID(context.Background(), usr, orgID, expected.UID)
require.Equal(t, expected, actual)
require.NoError(t, err)
})
t.Run("When get folder by title should return folder", func(t *testing.T) {
expected := models.NewFolder("TEST-" + util.GenerateShortUID())
expected := folder.NewFolder("TEST-"+util.GenerateShortUID(), "")
dashStore.On("GetFolderByTitle", mock.Anything, orgID, expected.Title).Return(expected, nil)
actual, err := service.GetFolderByTitle(context.Background(), usr, orgID, expected.Title)
actual, err := service.getFolderByTitle(context.Background(), usr, orgID, expected.Title)
require.Equal(t, expected, actual)
require.NoError(t, err)
})
@@ -311,7 +312,7 @@ func TestNestedFolderServiceFeatureToggle(t *testing.T) {
mock.AnythingOfType("bool"), mock.AnythingOfType("bool")).Return(&models.SaveDashboardCommand{}, nil)
dashStore := dashboards.FakeDashboardStore{}
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("models.SaveDashboardCommand")).Return(&models.Dashboard{}, nil)
dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&models.Folder{}, nil)
dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil)
cfg := setting.NewCfg()
cfg.RBACEnabled = false
nestedFoldersEnabled := true
@@ -338,18 +339,6 @@ func TestNestedFolderServiceFeatureToggle(t *testing.T) {
require.NotNil(t, res.UID)
})
t.Run("delete folder", func(t *testing.T) {
folderStore.ExpectedFolder = &folder.Folder{}
err := folderService.Delete(context.Background(), &folder.DeleteFolderCommand{})
require.NoError(t, err)
})
t.Run("get folder", func(t *testing.T) {
folderStore.ExpectedFolder = &folder.Folder{}
_, err := folderService.Get(context.Background(), &folder.GetFolderQuery{})
require.NoError(t, err)
})
t.Run("get parents folder", func(t *testing.T) {
folderStore.ExpectedFolder = &folder.Folder{}
_, err := folderService.GetParents(context.Background(), &folder.GetParentsQuery{})
@@ -378,12 +367,6 @@ func TestNestedFolderServiceFeatureToggle(t *testing.T) {
require.NoError(t, err)
require.Equal(t, 4, len(res))
})
t.Run("move folder", func(t *testing.T) {
folderStore.ExpectedFolder = &folder.Folder{}
_, err := folderService.Move(context.Background(), &folder.MoveFolderCommand{})
require.NoError(t, err)
})
}
func TestNestedFolderService(t *testing.T) {
@@ -412,7 +395,7 @@ func TestNestedFolderService(t *testing.T) {
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{}, nil)
dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&models.Folder{}, nil)
dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil)
ctx = appcontext.WithUser(ctx, usr)
_, err := foldersvc.Create(ctx, &folder.CreateFolderCommand{
@@ -430,7 +413,7 @@ func TestNestedFolderService(t *testing.T) {
dashStore.On("DeleteDashboard", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
actualCmd = args.Get(1).(*models.DeleteDashboardCommand)
}).Return(nil).Once()
dashStore.On("GetFolderByUID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).Return(&models.Folder{}, nil)
dashStore.On("GetFolderByUID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).Return(&folder.Folder{}, nil)
g := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true})
@@ -463,6 +446,9 @@ func TestNestedFolderService(t *testing.T) {
dashboardStore: dashStore,
store: store,
features: features,
accessControl: actest.FakeAccessControl{
ExpectedEvaluate: true,
},
}
t.Run("create, no error", func(t *testing.T) {
@@ -471,7 +457,7 @@ func TestNestedFolderService(t *testing.T) {
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{}, nil)
dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&models.Folder{}, nil)
dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil)
ctx = appcontext.WithUser(ctx, usr)
_, err := foldersvc.Create(ctx, &folder.CreateFolderCommand{
OrgID: orgID,
@@ -493,8 +479,8 @@ func TestNestedFolderService(t *testing.T) {
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{}, nil)
dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&models.Folder{}, nil)
dashStore.On("GetFolderByUID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).Return(&models.Folder{}, nil)
dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil)
dashStore.On("GetFolderByUID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).Return(&folder.Folder{}, nil)
// return an error from the folder store
store.ExpectedError = errors.New("FAILED")