IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2024-11-25 18:30:41 -06:00
Code Issues Packages Projects Releases Wiki Activity

Prevent moving a k6 folder (#88884)

Browse Source 
* iam-716 - prevent a folder move operation when the folder's uid or any of its parents uids begin with k6-app

* fox folder move check and only list non-k6 folders to users

* adding tests for moving

* add a test for listing folders

* fix the other tests

* use method that adds folder parent

---------

Co-authored-by: IevaVasiljeva <ieva.vasiljeva@grafana.com>
This commit is contained in:
Aaron Godin 2024-06-10 09:17:51 -05:00 committed by GitHub
parent 89a0bec208
commit 59a6a6513f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 119 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
pkg/api/folder.go
View File

@ -64,7 +64,13 @@ func (hs *HTTPServer) GetFolders(c *contextmodel.ReqContext) response.Response {
}
hits := make([]dtos.FolderSearchHit, 0)
requesterIsSvcAccount := c.SignedInUser.GetID().Namespace() == identity.NamespaceServiceAccount
for _, f := range folders {
// only list k6 folders when requested by a service account - prevents showing k6 folders in the UI for users
if (f.UID == accesscontrol.K6FolderUID || f.ParentUID == accesscontrol.K6FolderUID) && !requesterIsSvcAccount {
continue
}
hits = append(hits, dtos.FolderSearchHit{
ID: f.ID, // nolint:staticcheck
UID: f.UID,

1
pkg/services/accesscontrol/models.go
View File

@ -332,6 +332,7 @@ const (
GlobalOrgID = 0
NoOrgID = int64(-1)
GeneralFolderUID = "general"
K6FolderUID = "k6-app"
RoleGrafanaAdmin = "Grafana Admin"
// Permission actions

7
pkg/services/dashboards/service/dashboard_service.go
View File

@ -700,9 +700,16 @@ func makeQueryResult(query *dashboards.FindPersistedDashboardsQuery, res []dashb
hitList := make([]*model.Hit, 0)
hits := make(map[int64]*model.Hit)
requesterIsSvcAccount := query.SignedInUser.GetID().Namespace() == identity.NamespaceServiceAccount
for _, item := range res {
hit, exists := hits[item.ID]
if !exists {
// Don't list k6 items for users, we don't want users to interact with k6 folders directly through folder UI
if (item.UID == accesscontrol.K6FolderUID || item.FolderUID == accesscontrol.K6FolderUID) && !requesterIsSvcAccount {
continue
}
metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Dashboard).Inc()
hit = &model.Hit{
ID: item.ID,

33
pkg/services/folder/folderimpl/folder.go
View File

@ -180,7 +180,17 @@ func (s *Service) GetFolders(ctx context.Context, q folder.GetFoldersQuery) ([]*
}
}
return dashFolders, nil
// only list k6 folders when requested by a service account - prevents showing k6 folders in the UI for users
result := make([]*folder.Folder, 0, len(dashFolders))
requesterIsSvcAccount := qry.SignedInUser.GetID().Namespace() == identity.NamespaceServiceAccount
for _, folder := range dashFolders {
if (folder.UID == accesscontrol.K6FolderUID || folder.ParentUID == accesscontrol.K6FolderUID) && !requesterIsSvcAccount {
continue
}
result = append(result, folder)
}
return result, nil
}
func (s *Service) Get(ctx context.Context, q *folder.GetFolderQuery) (*folder.Folder, error) {
@ -870,6 +880,16 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol
return nil, folder.ErrBadRequest.Errorf("missing signed in user")
}
// k6-specific check to prevent folder move for a k6-app folder and its children
if cmd.UID == accesscontrol.K6FolderUID {
return nil, folder.ErrBadRequest.Errorf("k6 project may not be moved")
}
if f, err := s.store.Get(ctx, folder.GetFolderQuery{UID: &cmd.UID, OrgID: cmd.OrgID}); err != nil {
return nil, err
} else if f != nil && f.ParentUID == accesscontrol.K6FolderUID {
return nil, folder.ErrBadRequest.Errorf("k6 project may not be moved")
}
// Check that the user is allowed to move the folder to the destination folder
var evaluator accesscontrol.Evaluator
if cmd.NewParentUID != "" {
@ -902,24 +922,19 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol
return nil, folder.ErrMaximumDepthReached.Errorf("failed to move folder")
}
// if the current folder is already a parent of newparent, we should return error
for _, parent := range parents {
// if the current folder is already a parent of newparent, we should return error
if parent.UID == cmd.UID {
return nil, folder.ErrCircularReference.Errorf("failed to move folder")
}
}
newParentUID := ""
if cmd.NewParentUID != "" {
newParentUID = cmd.NewParentUID
}
var f *folder.Folder
if err := s.db.InTransaction(ctx, func(ctx context.Context) error {
if f, err = s.store.Update(ctx, folder.UpdateFolderCommand{
UID: cmd.UID,
OrgID: cmd.OrgID,
NewParentUID: &newParentUID,
NewParentUID: &cmd.NewParentUID,
SignedInUser: cmd.SignedInUser,
}); err != nil {
if s.db.GetDialect().IsUniqueConstraintViolation(err) {
@ -931,7 +946,7 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol
if _, err := s.legacyUpdate(ctx, &folder.UpdateFolderCommand{
UID: cmd.UID,
OrgID: cmd.OrgID,
NewParentUID: &newParentUID,
NewParentUID: &cmd.NewParentUID,
SignedInUser: cmd.SignedInUser,
// bypass optimistic locking used for dashboards
Overwrite: true,

81
pkg/services/folder/folderimpl/folder_test.go
View File

@ -1117,6 +1117,7 @@ func TestNestedFolderService(t *testing.T) {
t.Run("move without the right permissions should fail", func(t *testing.T) {
dashStore := &dashboards.FakeDashboardStore{}
dashboardFolderStore := foldertest.NewFakeFolderStore(t)
//dashboardFolderStore.On("GetFolderByUID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).Return(&folder.Folder{}, nil)
nestedFolderStore := NewFakeStore()
nestedFolderStore.ExpectedFolder = &folder.Folder{UID: "myFolder", ParentUID: "newFolder"}
@ -1153,6 +1154,34 @@ func TestNestedFolderService(t *testing.T) {
// require.NotNil(t, f)
})
t.Run("cannot move the k6 folder even when has permissions to move folders", func(t *testing.T) {
nestedFolderUser := &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}}
nestedFolderUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceAllScope()}}
features := featuremgmt.WithFeatures("nestedFolders")
folderSvc := setup(t, &dashboards.FakeDashboardStore{}, foldertest.NewFakeFolderStore(t), NewFakeStore(), features, acimpl.ProvideAccessControl(features), dbtest.NewFakeDB())
_, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: accesscontrol.K6FolderUID, NewParentUID: "newFolder", OrgID: orgID, SignedInUser: nestedFolderUser})
require.Error(t, err, folder.ErrBadRequest)
})
t.Run("cannot move a k6 subfolder even when has permissions to move folders", func(t *testing.T) {
nestedFolderUser := &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}}
nestedFolderUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceAllScope()}}
childUID := "k6-app-child"
nestedFolderStore := NewFakeStore()
nestedFolderStore.ExpectedFolder = &folder.Folder{
OrgID: orgID,
UID: childUID,
ParentUID: accesscontrol.K6FolderUID,
}
features := featuremgmt.WithFeatures("nestedFolders")
folderSvc := setup(t, &dashboards.FakeDashboardStore{}, foldertest.NewFakeFolderStore(t), nestedFolderStore, features, acimpl.ProvideAccessControl(features), dbtest.NewFakeDB())
_, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: childUID, NewParentUID: "newFolder", OrgID: orgID, SignedInUser: nestedFolderUser})
require.Error(t, err, folder.ErrBadRequest)
})
t.Run("move to the root folder without folder creation permissions fails", func(t *testing.T) {
dashStore := &dashboards.FakeDashboardStore{}
dashboardFolderStore := foldertest.NewFakeFolderStore(t)
@ -1461,6 +1490,58 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) {
})
})
t.Run("Should not list k6 folders or subfolders", func(t *testing.T) {
_, err = nestedFolderStore.Create(context.Background(), folder.CreateFolderCommand{
UID: accesscontrol.K6FolderUID,
OrgID: orgID,
SignedInUser: &signedInAdminUser,
})
require.NoError(t, err)
k6ChildFolder, err := nestedFolderStore.Create(context.Background(), folder.CreateFolderCommand{
UID: "k6-app-child",
ParentUID: accesscontrol.K6FolderUID,
OrgID: orgID,
SignedInUser: &signedInAdminUser,
})
require.NoError(t, err)
unrelatedFolder, err := nestedFolderStore.Create(context.Background(), folder.CreateFolderCommand{
UID: "another-folder",
OrgID: orgID,
SignedInUser: &signedInAdminUser,
})
require.NoError(t, err)
folders, err := serviceWithFlagOn.GetFolders(context.Background(), folder.GetFoldersQuery{
OrgID: orgID,
SignedInUser: &signedInAdminUser,
})
require.NoError(t, err)
assert.Equal(t, 1, len(folders), "should not return k6 folders or subfolders")
assert.Equal(t, unrelatedFolder.UID, folders[0].UID)
// Service accounts should be able to list k6 folders
svcAccountUser := user.SignedInUser{UserID: 2, IsServiceAccount: true, OrgID: orgID, Permissions: map[int64]map[string][]string{
orgID: {
dashboards.ActionFoldersRead: {dashboards.ScopeFoldersAll},
},
}}
folders, err = serviceWithFlagOn.GetFolders(context.Background(), folder.GetFoldersQuery{
OrgID: orgID,
SignedInUser: &svcAccountUser,
})
require.NoError(t, err)
assert.Equal(t, 3, len(folders), "service accounts should be able to list k6 folders")
t.Cleanup(func() {
//guardian.New = origNewGuardian
toDelete := []string{k6ChildFolder.UID, accesscontrol.K6FolderUID, unrelatedFolder.UID}
err := serviceWithFlagOn.store.Delete(context.Background(), toDelete, orgID)
assert.NoError(t, err)
})
})
t.Run("Should get org folders visible", func(t *testing.T) {
depth := 3