Access Control / Authorization

PoC: Demonstrates how a non-service account could be exposed to the k6 folder in legacy search path due to NotIn not being properly applied, potentially leaking restricted folders or causing pagination issues. Setup: Have a non-service account user and a folder hierarchy that includes a special k6-like folder (UID accesscontrol.K6FolderUID). Before fix (vulnerable behavior): A GetChildren call with a NotIn on the folder name (to exclude k6) is sent to the legacy search path. Because NotIn on SEARCH_FIELD_NAME is not supported in legacy search, the NotIn operator is ignored or treated incorrectly (as In), which can result in the k6 folder being included in results or pagination being affected. Example request (gRPC payload, simplified): POST /api/search?orgId=ORG1 Content: { "options": { "fields": [ {"Key": "SEARCH_FIELD_NAME", "Operator": "In", "Values": ["zzz", "yyy"]}, {"Key": "SEARCH_FIELD_NAME", "Operator": "NotIn", "Values": ["k6-app-uid"]} ] }, "limit": 50 } Expected (poisoned pre-fix) outcome: The backend misinterprets NotIn as In for legacy search, causing the k6 folder to appear in results for non-service accounts or cause pagination anomalies (e.g., fewer results than requested). After fix: The legacy path ignores NotIn on the name field and GetChildren applies a post-filter fallback to remove the k6 folder for non-service accounts, resulting in no exposure of k6 and correct pagination semantics (though a corner case may yield 49 results if k6 would have appeared on the page, which is acceptable during the transitional legacy period). Note: A service account will not have the k6 excluded (NotIn not applied), as seen in tests where NotIn is not added for service accounts.

diff --git a/pkg/registry/apis/dashboard/legacysearcher/search_client.go b/pkg/registry/apis/dashboard/legacysearcher/search_client.go index 2ad9c11582a8b..17dff4d44def6 100644 --- a/pkg/registry/apis/dashboard/legacysearcher/search_client.go +++ b/pkg/registry/apis/dashboard/legacysearcher/search_client.go @@ -192,6 +192,12 @@ func (c *DashboardSearchClient) Search(ctx context.Context, req *resourcepb.Reso case resource.SEARCH_FIELD_TAGS: query.Tags = field.GetValues() case resource.SEARCH_FIELD_NAME: + // NotIn is not supported by legacy search; ignore it and let the + // caller post-filter the results. This avoids misinterpreting + // NotIn as In, which would break the query entirely. + if field.Operator == string(selection.NotIn) { + continue + } query.DashboardUIDs = field.GetValues() query.DashboardIds = nil case resource.SEARCH_FIELD_FOLDER: diff --git a/pkg/services/folder/folderimpl/unifiedstore.go b/pkg/services/folder/folderimpl/unifiedstore.go index 7215a8eb9a63d..c19ee970e22f6 100644 --- a/pkg/services/folder/folderimpl/unifiedstore.go +++ b/pkg/services/folder/folderimpl/unifiedstore.go @@ -255,6 +255,22 @@ func (ss *FolderUnifiedStoreImpl) GetChildren(ctx context.Context, q folder.GetC }) } + // Exclude k6 folder from search results at the query level to avoid returning + // fewer results than the LIMIT, which breaks pagination. The unified/bleve + // search backend handles NotIn correctly. The legacy search backend ignores + // NotIn (it is not supported), so we also keep a post-filter as a fallback + // for legacy mode. The post-filter alone would break pagination (returning + // 49 instead of 50 results), but this is acceptable as a temporary state + // until legacy search is fully removed. + allowK6Folder := (q.SignedInUser != nil && q.SignedInUser.IsIdentityType(claims.TypeServiceAccount)) + if !allowK6Folder { + req.Options.Fields = append(req.Options.Fields, &resourcepb.Requirement{ + Key: resource.SEARCH_FIELD_NAME, + Operator: string(selection.NotIn), + Values: []string{accesscontrol.K6FolderUID}, + }) + } + // now, get children of the parent folder out, err := ss.k8sclient.Search(ctx, q.OrgID, req) if err != nil { @@ -266,14 +282,14 @@ func (ss *FolderUnifiedStoreImpl) GetChildren(ctx context.Context, q folder.GetC return nil, err } - allowK6Folder := (q.SignedInUser != nil && q.SignedInUser.IsIdentityType(claims.TypeServiceAccount)) - hits := make([]*folder.FolderReference, 0) + hits := make([]*folder.FolderReference, 0, len(res.Hits)) for _, item := range res.Hits { - // filter out k6 folders if request is not from a service account + // TODO: Remove this post-filter once legacy search is fully removed. + // Post-filter k6 folder as a fallback for the legacy search backend, + // which ignores the NotIn filter above. if item.Name == accesscontrol.K6FolderUID && !allowK6Folder { continue } - f := &folder.FolderReference{ ID: item.Field.GetNestedInt64(resource.SEARCH_FIELD_LEGACY_ID), UID: item.Name, diff --git a/pkg/services/folder/folderimpl/unifiedstore_test.go b/pkg/services/folder/folderimpl/unifiedstore_test.go index 030d2e6693d1b..53761e08a38dd 100644 --- a/pkg/services/folder/folderimpl/unifiedstore_test.go +++ b/pkg/services/folder/folderimpl/unifiedstore_test.go @@ -214,6 +214,11 @@ func TestGetChildren(t *testing.T) { Operator: string(selection.In), Values: []string{"folder1"}, }, + { + Key: resource.SEARCH_FIELD_NAME, + Operator: string(selection.NotIn), + Values: []string{accesscontrol.K6FolderUID}, + }, }, }, Limit: folderSearchLimit, // should default to folderSearchLimit @@ -273,6 +278,11 @@ func TestGetChildren(t *testing.T) { Operator: string(selection.In), Values: []string{"folder1"}, }, + { + Key: resource.SEARCH_FIELD_NAME, + Operator: string(selection.NotIn), + Values: []string{accesscontrol.K6FolderUID}, + }, }, }, Limit: folderSearchLimit, // should default to folderSearchLimit @@ -319,6 +329,11 @@ func TestGetChildren(t *testing.T) { Operator: string(selection.In), Values: []string{"folder2"}, }, + { + Key: resource.SEARCH_FIELD_NAME, + Operator: string(selection.NotIn), + Values: []string{accesscontrol.K6FolderUID}, + }, }, }, Limit: 10, @@ -356,31 +371,33 @@ func TestGetChildren(t *testing.T) { require.Equal(t, "folder2", result[0].UID) }) - t.Run("k6 folder should only be returned to service accounts", func(t *testing.T) { - mockCli.On("Search", mock.Anything, orgID, mock.Anything).Return(&resourcepb.ResourceSearchResponse{ + t.Run("k6 folder should be excluded via NotIn filter for non-service accounts", func(t *testing.T) { + // For non-service-account users, the search request should include a NotIn filter for k6-app + hasK6NotInFilter := func(req *resourcepb.ResourceSearchRequest) bool { + for _, f := range req.Options.Fields { + if f.Key == resource.SEARCH_FIELD_NAME && + f.Operator == string(selection.NotIn) && + len(f.Values) == 1 && f.Values[0] == accesscontrol.K6FolderUID { + return true + } + } + return false + } + + mockCli.On("Search", mock.Anything, orgID, mock.MatchedBy(hasK6NotInFilter)).Return(&resourcepb.ResourceSearchResponse{ Results: &resourcepb.ResourceTable{ Columns: []*resourcepb.ResourceTableColumnDefinition{ {Name: "folder", Type: resourcepb.ResourceTableColumnDefinition_STRING}, }, - Rows: []*resourcepb.ResourceTableRow{ - { - Key: &resourcepb.ResourceKey{Name: accesscontrol.K6FolderUID, Resource: "folder"}, - Cells: [][]byte{[]byte("folder1")}, - }, - }, + Rows: []*resourcepb.ResourceTableRow{}, }, - TotalHits: 1, - }, nil) + TotalHits: 0, + }, nil).Once() mockCli.On("Get", mock.Anything, "folder", orgID, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{ Object: map[string]interface{}{ "metadata": map[string]interface{}{"name": "folder"}, }, }, nil) - mockCli.On("Get", mock.Anything, accesscontrol.K6FolderUID, orgID, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{"name": accesscontrol.K6FolderUID}, - }, - }, nil) result, err := store.GetChildren(ctx, folder.GetChildrenQuery{ UID: "folder", @@ -388,14 +405,48 @@ func TestGetChildren(t *testing.T) { }) require.NoError(t, err) require.Len(t, result, 0) + }) + + t.Run("k6 folder should be returned for service accounts (no NotIn filter)", func(t *testing.T) { + // For service accounts, the search request should NOT include a NotIn filter for k6-app + hasNoK6NotInFilter := func(req *resourcepb.ResourceSearchRequest) bool { + for _, f := range req.Options.Fields { + if f.Key == resource.SEARCH_FIELD_NAME && + f.Operator == string(selection.NotIn) { + return false + } + } + return true + } + + mockCli.On("Search", mock.Anything, orgID, mock.MatchedBy(hasNoK6NotInFilter)).Return(&resourcepb.ResourceSearchResponse{ + Results: &resourcepb.ResourceTable{ + Columns: []*resourcepb.ResourceTableColumnDefinition{ + {Name: "folder", Type: resourcepb.ResourceTableColumnDefinition_STRING}, + }, + Rows: []*resourcepb.ResourceTableRow{ + { + Key: &resourcepb.ResourceKey{Name: accesscontrol.K6FolderUID, Resource: "folder"}, + Cells: [][]byte{[]byte("folder")}, + }, + }, + }, + TotalHits: 1, + }, nil).Once() + mockCli.On("Get", mock.Anything, accesscontrol.K6FolderUID, orgID, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{ + Object: map[string]interface{}{ + "metadata": map[string]interface{}{"name": accesscontrol.K6FolderUID}, + }, + }, nil) - result, err = store.GetChildren(ctx, folder.GetChildrenQuery{ + result, err := store.GetChildren(ctx, folder.GetChildrenQuery{ UID: "folder", OrgID: orgID, SignedInUser: &identity.StaticRequester{Type: claims.TypeServiceAccount}, }) require.NoError(t, err) require.Len(t, result, 1) + require.Equal(t, accesscontrol.K6FolderUID, result[0].UID) }) t.Run("should not do get requests for the children if RefOnly is true", func(t *testing.T) { @@ -407,6 +458,11 @@ func TestGetChildren(t *testing.T) { Operator: string(selection.In), Values: []string{"folder1"}, }, + { + Key: resource.SEARCH_FIELD_NAME, + Operator: string(selection.NotIn), + Values: []string{accesscontrol.K6FolderUID}, + }, }, }, Limit: folderSearchLimit, // should default to folderSearchLimit diff --git a/pkg/storage/unified/search/bleve_test.go b/pkg/storage/unified/search/bleve_test.go index 751751b351c9f..ed402c5026b18 100644 --- a/pkg/storage/unified/search/bleve_test.go +++ b/pkg/storage/unified/search/bleve_test.go @@ -21,8 +21,10 @@ import ( bolterrors "go.etcd.io/bbolt/errors" "go.uber.org/atomic" "go.uber.org/goleak" + "k8s.io/apimachinery/pkg/selection" authlib "github.com/grafana/authlib/types" + "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/infra/log" @@ -428,7 +430,8 @@ func testBleveBackend(t *testing.T, backend *bleveBackend) { { Action: resource.ActionIndex, Doc: &resource.IndexableDocument{ - RV: 1, + RV: 1, + Name: "zzz", Key: &resourcepb.ResourceKey{ Name: "zzz", Namespace: "ns", @@ -453,7 +456,8 @@ func testBleveBackend(t *testing.T, backend *bleveBackend) { { Action: resource.ActionIndex, Doc: &resource.IndexableDocument{ - RV: 2, + RV: 2, + Name: "yyy", Key: &resourcepb.ResourceKey{ Name: "yyy", Namespace: "ns", @@ -492,6 +496,57 @@ func testBleveBackend(t *testing.T, backend *bleveBackend) { resource.AssertTableSnapshot(t, filepath.Join("testdata", "manual-folder.json"), rsp.Results) }) + t.Run("folder NotIn field filter excludes matching names", func(t *testing.T) { + require.NotNil(t, foldersIndex) + key := folderKey + + // NotIn should exclude the named folder from results + rsp, err := foldersIndex.Search(ctx, NewStubAccessClient(map[string]bool{"folders": true}), &resourcepb.ResourceSearchRequest{ + Options: &resourcepb.ListOptions{ + Key: key, + Fields: []*resourcepb.Requirement{{ + Key: resource.SEARCH_FIELD_NAME, + Operator: string(selection.NotIn), + Values: []string{"zzz"}, + }}, + }, + Limit: 100000, + }, nil, nil) + require.NoError(t, err) + require.Equal(t, int64(1), rsp.TotalHits) + require.Equal(t, "yyy", rsp.Results.Rows[0].Key.Name) + }) + + t.Run("folder In and NotIn on same field compose correctly", func(t *testing.T) { + require.NotNil(t, foldersIndex) + key := folderKey + + // In selects both folders, NotIn excludes one — should return only the non-excluded one. + // This is the exact query pattern used by GetChildren to exclude the k6 folder + // while also filtering by allowed folder UIDs. + rsp, err := foldersIndex.Search(ctx, NewStubAccessClient(map[string]bool{"folders": true}), &resourcepb.ResourceSearchRequest{ + Options: &resourcepb.ListOptions{ + Key: key, + Fields: []*resourcepb.Requirement{ + { + Key: resource.SEARCH_FIELD_NAME, + Operator: string(selection.In), + Values: []string{"zzz", "yyy"}, + }, + { + Key: resource.SEARCH_FIELD_NAME, + Operator: string(selection.NotIn), + Values: []string{"zzz"}, + }, + }, + }, + Limit: 100000, + }, nil, nil) + require.NoError(t, err) + require.Equal(t, int64(1), rsp.TotalHits) + require.Equal(t, "yyy", rsp.Results.Rows[0].Key.Name) + }) + t.Run("simple federation", func(t *testing.T) { // The other tests must run first to build the indexes require.NotNil(t, dashboardsIndex)