diff --git a/pkg/api/folder.go b/pkg/api/folder.go index 109281b1990..d8709daf367 100644 --- a/pkg/api/folder.go +++ b/pkg/api/folder.go @@ -64,13 +64,7 @@ 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, diff --git a/pkg/services/dashboards/database/database.go b/pkg/services/dashboards/database/database.go index c3ae1f976b3..bad463c509d 100644 --- a/pkg/services/dashboards/database/database.go +++ b/pkg/services/dashboards/database/database.go @@ -7,6 +7,7 @@ import ( "strings" "time" + "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/metrics" @@ -876,6 +877,11 @@ func (d *dashboardStore) FindDashboards(ctx context.Context, query *dashboards.F }) } + // only list k6 folders when requested by a service account - prevents showing k6 folders in the UI for users + if query.SignedInUser == nil || query.SignedInUser.GetID().Namespace() != identity.NamespaceServiceAccount { + filters = append(filters, searchstore.K6FolderFilter{}) + } + filters = append(filters, permissions.NewAccessControlDashboardPermissionFilter(query.SignedInUser, query.Permission, query.Type, d.features, recursiveQueriesAreSupported)) filters = append(filters, searchstore.DeletedFilter{Deleted: query.IsDeleted}) diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index c94d8b4df7f..f9d2168b710 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -700,16 +700,9 @@ 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, diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index 073e6ca239f..8b70af72be3 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -180,17 +180,7 @@ func (s *Service) GetFolders(ctx context.Context, q folder.GetFoldersQuery) ([]* } } - // 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 + return dashFolders, nil } func (s *Service) Get(ctx context.Context, q *folder.GetFolderQuery) (*folder.Folder, error) { diff --git a/pkg/services/folder/folderimpl/sqlstore.go b/pkg/services/folder/folderimpl/sqlstore.go index a554c6470f2..41f732b8ccc 100644 --- a/pkg/services/folder/folderimpl/sqlstore.go +++ b/pkg/services/folder/folderimpl/sqlstore.go @@ -9,9 +9,11 @@ import ( "github.com/grafana/dskit/concurrency" + "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/metrics" + "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" @@ -319,6 +321,13 @@ func (ss *sqlStore) GetChildren(ctx context.Context, q folder.GetChildrenQuery) } sql.WriteString(")") } + + // only list k6 folders when requested by a service account - prevents showing k6 folders in the UI for users + if q.SignedInUser == nil || q.SignedInUser.GetID().Namespace() != identity.NamespaceServiceAccount { + sql.WriteString(" AND uid != ?") + args = append(args, accesscontrol.K6FolderUID) + } + sql.WriteString(" ORDER BY title ASC") if q.Limit != 0 { @@ -474,6 +483,12 @@ func (ss *sqlStore) GetFolders(ctx context.Context, q getFoldersQuery) ([]*folde } } + // only list k6 folders when requested by a service account - prevents showing k6 folders in the UI for users + if q.SignedInUser == nil || q.SignedInUser.GetID().Namespace() != identity.NamespaceServiceAccount { + s.WriteString(" AND f0.uid != ? AND (f0.parent_uid != ? OR f0.parent_uid IS NULL)") + args = append(args, accesscontrol.K6FolderUID, accesscontrol.K6FolderUID) + } + if len(q.ancestorUIDs) == 0 { if q.OrderByTitle { s.WriteString(` ORDER BY f0.title ASC`) diff --git a/pkg/services/folder/folderimpl/sqlstore_test.go b/pkg/services/folder/folderimpl/sqlstore_test.go index e346c91a960..c966bbf679b 100644 --- a/pkg/services/folder/folderimpl/sqlstore_test.go +++ b/pkg/services/folder/folderimpl/sqlstore_test.go @@ -14,11 +14,13 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/org/orgimpl" "github.com/grafana/grafana/pkg/services/quota/quotatest" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) @@ -731,6 +733,68 @@ func TestIntegrationGetChildren(t *testing.T) { t.Errorf("Result mismatch (-want +got):\n%s", diff) } }) + + t.Run("should hide k6-app folder for users but not for service accounts", func(t *testing.T) { + _, err = folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: "k6-app-folder", + OrgID: orgID, + UID: accesscontrol.K6FolderUID, + }) + require.NoError(t, err) + + children, err := folderStore.GetChildren(context.Background(), folder.GetChildrenQuery{ + OrgID: orgID, + SignedInUser: usr, + }) + require.NoError(t, err) + require.Equal(t, 1, len(children)) + assert.Equal(t, parent.UID, children[0].UID) + + // Service account should be able to list k6 folder + children, err = folderStore.GetChildren(context.Background(), folder.GetChildrenQuery{ + OrgID: orgID, + SignedInUser: &user.SignedInUser{UserID: 2, OrgID: orgID, IsServiceAccount: true}, + }) + require.NoError(t, err) + require.Equal(t, 2, len(children)) + childrenUIDs := make([]string, 0, len(children)) + for _, child := range children { + childrenUIDs = append(childrenUIDs, child.UID) + } + assert.EqualValues(t, []string{parent.UID, accesscontrol.K6FolderUID}, childrenUIDs) + }) + + t.Run("pagination works if k6-app folder is hidden", func(t *testing.T) { + for i := 0; i < 4; i++ { + _, err = folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: fmt.Sprintf("root-%d", i), + OrgID: orgID, + UID: fmt.Sprintf("root-%d", i), + }) + require.NoError(t, err) + } + + // Should skip k6-app folder but get parent folder and two more folders + children, err := folderStore.GetChildren(context.Background(), folder.GetChildrenQuery{ + OrgID: orgID, + SignedInUser: usr, + Limit: 3, + }) + require.NoError(t, err) + require.Equal(t, 3, len(children)) + assert.EqualValues(t, []string{parent.UID, "root-0", "root-1"}, []string{children[0].UID, children[1].UID, children[2].UID}) + + // Should get the two remaining folders + children, err = folderStore.GetChildren(context.Background(), folder.GetChildrenQuery{ + OrgID: orgID, + SignedInUser: usr, + Page: 2, + Limit: 3, + }) + require.NoError(t, err) + require.Equal(t, 2, len(children)) + assert.EqualValues(t, []string{"root-2", "root-3"}, []string{children[0].UID, children[1].UID}) + }) } func TestIntegrationGetHeight(t *testing.T) { diff --git a/pkg/services/sqlstore/searchstore/filters.go b/pkg/services/sqlstore/searchstore/filters.go index 825585641fb..432ac9e4a4d 100644 --- a/pkg/services/sqlstore/searchstore/filters.go +++ b/pkg/services/sqlstore/searchstore/filters.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/search/model" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" @@ -127,6 +128,14 @@ func (f DashboardFilter) Where() (string, []any) { return sqlUIDin("dashboard.uid", f.UIDs) } +type K6FolderFilter struct{} + +func (f K6FolderFilter) Where() (string, []any) { + filter := "dashboard.uid != ? AND (dashboard.folder_uid != ? OR dashboard.folder_uid IS NULL)" + params := []any{accesscontrol.K6FolderUID, accesscontrol.K6FolderUID} + return filter, params +} + type TagsFilter struct { Tags []string }