SQL Injection (ORDER BY / sort parameter) via unsanitized sort field in team search API

diff --git a/packages/grafana-api-clients/src/clients/rtkq/iam/v0alpha1/endpoints.gen.ts b/packages/grafana-api-clients/src/clients/rtkq/iam/v0alpha1/endpoints.gen.ts index 2e5633e27f4f4..626d654503ac0 100644 --- a/packages/grafana-api-clients/src/clients/rtkq/iam/v0alpha1/endpoints.gen.ts +++ b/packages/grafana-api-clients/src/clients/rtkq/iam/v0alpha1/endpoints.gen.ts @@ -178,6 +178,7 @@ const injectedRtkApi = api offset: queryArg.offset, page: queryArg.page, accesscontrol: queryArg.accesscontrol, + sort: queryArg.sort, }, }), providesTags: ['Search'], @@ -919,6 +920,8 @@ export type GetSearchTeamsApiArg = { page?: number; /** when true, includes access control metadata in the response */ accesscontrol?: boolean; + /** sortable field */ + sort?: string; }; export type GetSearchUsersApiResponse = unknown; export type GetSearchUsersApiArg = { diff --git a/packages/grafana-openapi/src/apis/iam.grafana.app-v0alpha1.json b/packages/grafana-openapi/src/apis/iam.grafana.app-v0alpha1.json index 014c97bea6c1e..17a3322094eea 100644 --- a/packages/grafana-openapi/src/apis/iam.grafana.app-v0alpha1.json +++ b/packages/grafana-openapi/src/apis/iam.grafana.app-v0alpha1.json @@ -922,6 +922,35 @@ "schema": { "type": "boolean" } + }, + { + "name": "sort", + "in": "query", + "description": "sortable field", + "schema": { + "type": "string" + }, + "examples": { + "-email": { + "summary": "email descending", + "value": "-email" + }, + "-title": { + "summary": "title descending", + "value": "-title" + }, + "default": { + "summary": "default sorting" + }, + "email": { + "summary": "email ascending", + "value": "email" + }, + "title": { + "summary": "title ascending", + "value": "title" + } + } } ], "responses": { diff --git a/pkg/registry/apis/iam/legacysort/sort.go b/pkg/registry/apis/iam/legacysort/sort.go new file mode 100644 index 0000000000000..e2dcdd90061af --- /dev/null +++ b/pkg/registry/apis/iam/legacysort/sort.go @@ -0,0 +1,40 @@ +package legacysort + +import ( + "fmt" + "sort" + + "github.com/grafana/grafana/pkg/services/search/model" + "github.com/grafana/grafana/pkg/storage/unified/resourcepb" +) + +// ConvertToSortOptions translates unified search sort fields into legacy SQL sort options. +// fieldMapping maps unified field names (e.g. "title", "fields.email") to legacy sort key names (e.g. "name", "email"). +// sortOptions is the resource-specific SortOptionsByQueryParam map. +func ConvertToSortOptions( + sortBy []*resourcepb.ResourceSearchRequest_Sort, + fieldMapping map[string]string, + sortOptions map[string]model.SortOption, +) []model.SortOption { + opts := []model.SortOption{} + for _, s := range sortBy { + field := s.Field + if mapped, ok := fieldMapping[field]; ok { + field = mapped + } + + suffix := "asc" + if s.Desc { + suffix = "desc" + } + key := fmt.Sprintf("%s-%s", field, suffix) + + if opt, ok := sortOptions[key]; ok { + opts = append(opts, opt) + } + } + sort.Slice(opts, func(i, j int) bool { + return opts[i].Index < opts[j].Index || (opts[i].Index == opts[j].Index && opts[i].Name < opts[j].Name) + }) + return opts +} diff --git a/pkg/registry/apis/iam/legacysort/sort_test.go b/pkg/registry/apis/iam/legacysort/sort_test.go new file mode 100644 index 0000000000000..1898bca3df389 --- /dev/null +++ b/pkg/registry/apis/iam/legacysort/sort_test.go @@ -0,0 +1,79 @@ +package legacysort + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/services/search/model" + "github.com/grafana/grafana/pkg/storage/unified/resourcepb" +) + +func newSortOption(name string, index int) model.SortOption { + return model.SortOption{Name: name, Index: index} +} + +func TestConvertToSortOptions(t *testing.T) { + fieldMapping := map[string]string{ + "title": "name", + "fields.email": "email", + } + + sortOptions := map[string]model.SortOption{ + "name-asc": newSortOption("name-asc", 0), + "name-desc": newSortOption("name-desc", 0), + "email-asc": newSortOption("email-asc", 1), + "email-desc": newSortOption("email-desc", 1), + } + + t.Run("maps title to name ascending", func(t *testing.T) { + sortBy := []*resourcepb.ResourceSearchRequest_Sort{ + {Field: "title", Desc: false}, + } + opts := ConvertToSortOptions(sortBy, fieldMapping, sortOptions) + require.Len(t, opts, 1) + require.Equal(t, "name-asc", opts[0].Name) + }) + + t.Run("maps title to name descending", func(t *testing.T) { + sortBy := []*resourcepb.ResourceSearchRequest_Sort{ + {Field: "title", Desc: true}, + } + opts := ConvertToSortOptions(sortBy, fieldMapping, sortOptions) + require.Len(t, opts, 1) + require.Equal(t, "name-desc", opts[0].Name) + }) + + t.Run("maps prefixed field to email ascending", func(t *testing.T) { + sortBy := []*resourcepb.ResourceSearchRequest_Sort{ + {Field: "fields.email", Desc: false}, + } + opts := ConvertToSortOptions(sortBy, fieldMapping, sortOptions) + require.Len(t, opts, 1) + require.Equal(t, "email-asc", opts[0].Name) + }) + + t.Run("ignores unknown sort fields", func(t *testing.T) { + sortBy := []*resourcepb.ResourceSearchRequest_Sort{ + {Field: "unknown", Desc: false}, + } + opts := ConvertToSortOptions(sortBy, fieldMapping, sortOptions) + require.Len(t, opts, 0) + }) + + t.Run("orders multiple sort fields by index", func(t *testing.T) { + sortBy := []*resourcepb.ResourceSearchRequest_Sort{ + {Field: "fields.email", Desc: false}, + {Field: "title", Desc: true}, + } + opts := ConvertToSortOptions(sortBy, fieldMapping, sortOptions) + require.Len(t, opts, 2) + require.Equal(t, "name-desc", opts[0].Name) + require.Equal(t, "email-asc", opts[1].Name) + }) + + t.Run("returns empty for nil input", func(t *testing.T) { + opts := ConvertToSortOptions(nil, fieldMapping, sortOptions) + require.Len(t, opts, 0) + }) +} diff --git a/pkg/registry/apis/iam/team/legacy_search.go b/pkg/registry/apis/iam/team/legacy_search.go index 8e0f1e28fc747..263924260b2c1 100644 --- a/pkg/registry/apis/iam/team/legacy_search.go +++ b/pkg/registry/apis/iam/team/legacy_search.go @@ -13,7 +13,9 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/registry/apis/iam/legacysort" "github.com/grafana/grafana/pkg/services/team" + teamsortopts "github.com/grafana/grafana/pkg/services/team/sortopts" res "github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/grafana/grafana/pkg/storage/unified/resourcepb" "github.com/grafana/grafana/pkg/storage/unified/search/builders" @@ -24,6 +26,11 @@ const ( TeamResourceGroup = "iam.grafana.com" ) +var teamSortFieldMapping = map[string]string{ + res.SEARCH_FIELD_TITLE: "name", + fmt.Sprintf("%s%s", res.SEARCH_FIELD_PREFIX, builders.TEAM_SEARCH_EMAIL): "email", +} + // LegacyTeamSearchClient is a client for searching for teams in the legacy search engine. type LegacyTeamSearchClient struct { resourcepb.ResourceIndexClient @@ -68,6 +75,7 @@ func (c *LegacyTeamSearchClient) Search(ctx context.Context, req *resourcepb.Res Page: int(req.Page), Query: req.Query, OrgID: signedInUser.GetOrgID(), + SortOpts: legacysort.ConvertToSortOptions(req.SortBy, teamSortFieldMapping, teamsortopts.SortOptionsByQueryParam), } res, err := c.teamService.SearchTeams(ctx, query) diff --git a/pkg/registry/apis/iam/team_search.go b/pkg/registry/apis/iam/team_search.go index 02f97ac18b3c6..3b3bfbcfb1029 100644 --- a/pkg/registry/apis/iam/team_search.go +++ b/pkg/registry/apis/iam/team_search.go @@ -6,7 +6,9 @@ import ( "fmt" "net/http" "net/url" + "slices" "strconv" + "strings" "github.com/grafana/authlib/authz" authlib "github.com/grafana/authlib/types" @@ -137,6 +139,47 @@ func (s *TeamSearchHandler) GetAPIRoutes(defs map[string]common.OpenAPIDefinitio Schema: spec.BoolProperty(), }, }, + { + ParameterProps: spec3.ParameterProps{ + Name: "sort", + In: "query", + Description: "sortable field", + Examples: map[string]*spec3.Example{ + "default": { + ExampleProps: spec3.ExampleProps{ + Summary: "default sorting", + Value: "", + }, + }, + "title": { + ExampleProps: spec3.ExampleProps{ + Summary: "title ascending", + Value: "title", + }, + }, + "-title": { + ExampleProps: spec3.ExampleProps{ + Summary: "title descending", + Value: "-title", + }, + }, + "email": { + ExampleProps: spec3.ExampleProps{ + Summary: "email ascending", + Value: "email", + }, + }, + "-email": { + ExampleProps: spec3.ExampleProps{ + Summary: "email descending", + Value: "-email", + }, + }, + }, + Required: false, + Schema: spec.StringProperty(), + }, + }, }, Responses: &spec3.Responses{ ResponsesProps: spec3.ResponsesProps{ @@ -217,6 +260,33 @@ func (s *TeamSearchHandler) DoTeamSearch(w http.ResponseWriter, r *http.Request) }, } + if queryParams.Has("sort") { + for _, sortParam := range queryParams["sort"] { + currField := sortParam + desc := false + if strings.HasPrefix(sortParam, "-") { + currField = sortParam[1:] + desc = true + } + + if currField != resource.SEARCH_FIELD_TITLE && !slices.Contains(builders.TeamSortableExtraFields, currField) { + http.Error(w, fmt.Sprintf("invalid sort field: %s", currField), http.StatusBadRequest) + return + } + + sortField := currField + if slices.Contains(builders.TeamSortableExtraFields, currField) { + sortField = resource.SEARCH_FIELD_PREFIX + currField + } + + s := &resourcepb.ResourceSearchRequest_Sort{ + Field: sortField, + Desc: desc, + } + searchRequest.SortBy = append(searchRequest.SortBy, s) + } + } + result, err := s.client.Search(ctx, searchRequest) if err != nil { errhttp.Write(ctx, err, w) diff --git a/pkg/registry/apis/iam/team_search_test.go b/pkg/registry/apis/iam/team_search_test.go index 4d0e8d04f5a35..4b9176219acab 100644 --- a/pkg/registry/apis/iam/team_search_test.go +++ b/pkg/registry/apis/iam/team_search_test.go @@ -214,6 +214,53 @@ func TestTeamSearchHandler(t *testing.T) { require.Equal(t, tt.expectedPage, int(mockClient.LastSearchRequest.Page), fmt.Sprintf("mismatch page in test %d", i)) } }) + + t.Run("returns 400 for invalid sort field", func(t *testing.T) { + mockClient := &MockClient{} + + searchHandler := &TeamSearchHandler{ + log: log.New("grafana-apiserver.teams.search"), + client: mockClient, + tracer: tracing.NewNoopTracerService(), + features: featuremgmt.WithFeatures(), + } + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/teams/search?sort=invalid", nil) + req.Header.Add("content-type", "application/json") + req = req.WithContext(identity.WithRequester(req.Context(), &user.SignedInUser{Namespace: "test"})) + + searchHandler.DoTeamSearch(rr, req) + + assert.Equal(t, http.StatusBadRequest, rr.Code) + assert.Nil(t, mockClient.LastSearchRequest, "Search should not be called for invalid sort field") + }) + + t.Run("accepts valid sort fields", func(t *testing.T) { + for _, sortParam := range []string{"title", "-title", "email", "-email"} { + t.Run(sortParam, func(t *testing.T) { + mockClient := &MockClient{} + + searchHandler := &TeamSearchHandler{ + log: log.New("grafana-apiserver.teams.search"), + client: mockClient, + tracer: tracing.NewNoopTracerService(), + features: featuremgmt.WithFeatures(), + } + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/teams/search?sort="+sortParam, nil) + req.Header.Add("content-type", "application/json") + req = req.WithContext(identity.WithRequester(req.Context(), &user.SignedInUser{Namespace: "test"})) + + searchHandler.DoTeamSearch(rr, req) + + assert.NotEqual(t, http.StatusBadRequest, rr.Code) + require.NotNil(t, mockClient.LastSearchRequest) + require.Len(t, mockClient.LastSearchRequest.SortBy, 1) + }) + } + }) } type MockClient struct { diff --git a/pkg/registry/apis/iam/user/legacy_search.go b/pkg/registry/apis/iam/user/legacy_search.go index f35e2577559bb..54040cd7a2502 100644 --- a/pkg/registry/apis/iam/user/legacy_search.go +++ b/pkg/registry/apis/iam/user/legacy_search.go @@ -6,7 +6,6 @@ import ( "fmt" "math" "regexp" - "sort" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" @@ -14,8 +13,8 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/registry/apis/iam/legacysort" "github.com/grafana/grafana/pkg/services/org" - "github.com/grafana/grafana/pkg/services/search/model" "github.com/grafana/grafana/pkg/services/searchusers/sortopts" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/storage/unified/resource" @@ -35,6 +34,13 @@ var ( fieldLastSeenAt = fmt.Sprintf("%s%s", resource.SEARCH_FIELD_PREFIX, builders.USER_LAST_SEEN_AT) fieldRole = fmt.Sprintf("%s%s", resource.SEARCH_FIELD_PREFIX, builders.USER_ROLE) wildcardsMatcher = regexp.MustCompile(`[\*\?\\]`) + + userSortFieldMapping = map[string]string{ + fieldLastSeenAt: "lastSeenAtAge", + resource.SEARCH_FIELD_TITLE: "name", + fieldLogin: "login", + fieldEmail: "email", + } ) // UserLegacySearchClient is a client for searching for users in the legacy search engine. @@ -84,7 +90,7 @@ func (c *UserLegacySearchClient) Search(ctx context.Context, req *resourcepb.Res req.Page = 1 } - legacySortOptions := convertToSortOptions(req.SortBy) + legacySortOptions := legacysort.ConvertToSortOptions(req.SortBy, userSortFieldMapping, sortopts.SortOptionsByQueryParam) query := &org.SearchOrgUsersQuery{ OrgID: signedInUser.GetOrgID(), @@ -223,35 +229,3 @@ func createCells(u *org.OrgUserDTO, fields []string) [][]byte { } return cells } - -func convertToSortOptions(sortBy []*res ... [truncated]