diff --git a/identity/handler.go b/identity/handler.go index 7560724899db..98b73665d553 100644 --- a/identity/handler.go +++ b/identity/handler.go @@ -119,10 +119,7 @@ func (h *Handler) RegisterAdminRoutes(admin *x.RouterAdmin) { // Paginated Identity List Response // // swagger:response listIdentities -// -//nolint:deadcode,unused -//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions -type listIdentitiesResponse struct { +type _ struct { migrationpagination.ResponseHeaderAnnotation // List of identities @@ -133,11 +130,10 @@ type listIdentitiesResponse struct { // Paginated List Identity Parameters // -// swagger:parameters listIdentities +// Note: Filters cannot be combined. // -//nolint:deadcode,unused -//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions -type listIdentitiesParameters struct { +// swagger:parameters listIdentities +type _ struct { migrationpagination.RequestParameters // List of ids used to filter identities. @@ -183,11 +179,73 @@ type listIdentitiesParameters struct { crdbx.ConsistencyRequestParameters } +func parseListIdentitiesParameters(r *http.Request) (params ListIdentityParameters, err error) { + query := r.URL.Query() + var requestedFilters int + + params.Expand = ExpandDefault + + if ids := query["ids"]; len(ids) > 0 { + requestedFilters++ + for _, v := range ids { + id, err := uuid.FromString(v) + if err != nil { + return params, errors.WithStack(herodot.ErrBadRequest.WithReasonf("Invalid UUID value `%s` for parameter `ids`.", v)) + } + params.IdsFilter = append(params.IdsFilter, id) + } + } + if len(params.IdsFilter) > 500 { + return params, errors.WithStack(herodot.ErrBadRequest.WithReason("The number of ids to filter must not exceed 500.")) + } + + if orgID := query.Get("organization_id"); orgID != "" { + requestedFilters++ + params.OrganizationID, err = uuid.FromString(orgID) + if err != nil { + return params, errors.WithStack(herodot.ErrBadRequest.WithReasonf("Invalid UUID value `%s` for parameter `organization_id`.", orgID)) + } + } + + if identifier := query.Get("credentials_identifier"); identifier != "" { + requestedFilters++ + params.Expand = ExpandEverything + params.CredentialsIdentifier = identifier + } + + if identifier := query.Get("credentials_identifier_similar"); identifier != "" { + requestedFilters++ + params.Expand = ExpandEverything + params.CredentialsIdentifierSimilar = identifier + } + + for _, v := range query["include_credential"] { + params.Expand = ExpandEverything + tc, ok := ParseCredentialsType(v) + if !ok { + return params, errors.WithStack(herodot.ErrBadRequest.WithReasonf("Invalid value `%s` for parameter `include_credential`.", v)) + } + params.DeclassifyCredentials = append(params.DeclassifyCredentials, tc) + } + + if requestedFilters > 1 { + return params, errors.WithStack(herodot.ErrBadRequest.WithReason("You cannot combine multiple filters in this API")) + } + + params.KeySetPagination, params.PagePagination, err = x.ParseKeysetOrPagePagination(r) + if err != nil { + return params, err + } + params.ConsistencyLevel = crdbx.ConsistencyLevelFromRequest(r) + + return params, nil +} + // swagger:route GET /admin/identities identity listIdentities // // # List Identities // -// Lists all [identities](https://www.ory.sh/docs/kratos/concepts/identity-user-model) in the system. +// Lists all [identities](https://www.ory.sh/docs/kratos/concepts/identity-user-model) in the system. Note: filters cannot be combined. // // Produces: // - application/json @@ -201,54 +259,7 @@ type listIdentitiesParameters struct { // 200: listIdentities // default: errorGeneric func (h *Handler) list(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { - includeCredentials := r.URL.Query()["include_credential"] - var err error - var declassify []CredentialsType - for _, v := range includeCredentials { - tc, ok := ParseCredentialsType(v) - if ok { - declassify = append(declassify, tc) - } else { - h.r.Writer().WriteError(w, r, errors.WithStack(herodot.ErrBadRequest.WithReasonf("Invalid value `%s` for parameter `include_credential`.", declassify))) - return - } - } - - var orgId uuid.UUID - if orgIdStr := r.URL.Query().Get("organization_id"); orgIdStr != "" { - orgId, err = uuid.FromString(r.URL.Query().Get("organization_id")) - if err != nil { - h.r.Writer().WriteError(w, r, errors.WithStack(herodot.ErrBadRequest.WithReasonf("Invalid UUID value `%s` for parameter `organization_id`.", r.URL.Query().Get("organization_id")))) - return - } - } - var idsFilter []uuid.UUID - for _, v := range r.URL.Query()["ids"] { - id, err := uuid.FromString(v) - if err != nil { - h.r.Writer().WriteError(w, r, errors.WithStack(herodot.ErrBadRequest.WithReasonf("Invalid UUID value `%s` for parameter `ids`.", v))) - return - } - idsFilter = append(idsFilter, id) - } - - params := ListIdentityParameters{ - Expand: ExpandDefault, - IdsFilter: idsFilter, - CredentialsIdentifier: r.URL.Query().Get("credentials_identifier"), - CredentialsIdentifierSimilar: r.URL.Query().Get("preview_credentials_identifier_similar"), - OrganizationID: orgId, - ConsistencyLevel: crdbx.ConsistencyLevelFromRequest(r), - DeclassifyCredentials: declassify, - } - if params.CredentialsIdentifier != "" && params.CredentialsIdentifierSimilar != "" { - h.r.Writer().WriteError(w, r, herodot.ErrBadRequest.WithReason("Cannot pass both credentials_identifier and preview_credentials_identifier_similar.")) - return - } - if params.CredentialsIdentifier != "" || params.CredentialsIdentifierSimilar != "" || len(params.DeclassifyCredentials) > 0 { - params.Expand = ExpandEverything - } - params.KeySetPagination, params.PagePagination, err = x.ParseKeysetOrPagePagination(r) + params, err := parseListIdentitiesParameters(r) if err != nil { h.r.Writer().WriteError(w, r, err) return @@ -271,7 +282,7 @@ func (h *Handler) list(w http.ResponseWriter, r *http.Request, _ httprouter.Para } u := *r.URL pagepagination.PaginationHeader(w, &u, total, params.PagePagination.Page, params.PagePagination.ItemsPerPage) - } else { + } else if nextPage != nil { u := *r.URL keysetpagination.Header(w, &u, nextPage) } diff --git a/identity/handler_test.go b/identity/handler_test.go index e3362a6ecf94..d55448cdce3b 100644 --- a/identity/handler_test.go +++ b/identity/handler_test.go @@ -369,21 +369,50 @@ func TestHandler(t *testing.T) { id := x.ParseUUID(res.Get("id").String()) ids = append(ids, id) } - require.Equal(t, len(ids), identitiesAmount) + require.Len(t, ids, identitiesAmount) }) t.Run("case=list few identities", func(t *testing.T) { - url := "/identities?ids=" + ids[0].String() + url := "/identities?ids=" + ids[0].String() + "&ids=" + ids[0].String() // duplicate ID is deduplicated in result for i := 1; i < listAmount; i++ { url += "&ids=" + ids[i].String() } res := get(t, adminTS, url, http.StatusOK) identities := res.Array() - require.Equal(t, len(identities), listAmount) + require.Len(t, identities, listAmount) }) }) + t.Run("case=list identities by ID is capped at 500", func(t *testing.T) { + url := "/identities?ids=" + x.NewUUID().String() + for i := 0; i < 501; i++ { + url += "&ids=" + x.NewUUID().String() + } + res := get(t, adminTS, url, http.StatusBadRequest) + assert.Contains(t, res.Get("error.reason").String(), "must not exceed 500") + }) + + t.Run("case=list identities cannot combine filters", func(t *testing.T) { + filters := []string{ + "ids=" + x.NewUUID().String(), + "credentials_identifier=foo@bar.com", + "credentials_identifier_similar=bar.com", + "organization_id=" + x.NewUUID().String(), + } + for i := range filters { + for j := range filters { + if i == j { + continue // OK to use the same filter multiple times. Behavior varies by filter, though. + } + + url := "/identities?" + filters[i] + "&" + filters[j] + res := get(t, adminTS, url, http.StatusBadRequest) + assert.Contains(t, res.Get("error.reason").String(), "cannot combine multiple filters") + } + } + }) + t.Run("case=malformed ids should return an error", func(t *testing.T) { res := get(t, adminTS, "/identities?ids=not-a-uuid", http.StatusBadRequest) assert.Contains(t, res.Get("error.reason").String(), "Invalid UUID value `not-a-uuid` for parameter `ids`.", "%s", res.Raw) diff --git a/internal/client-go/api_identity.go b/internal/client-go/api_identity.go index 9e4aec1b6c58..b7dd012adf83 100644 --- a/internal/client-go/api_identity.go +++ b/internal/client-go/api_identity.go @@ -227,7 +227,7 @@ type IdentityAPI interface { /* * ListIdentities List Identities - * Lists all [identities](https://www.ory.sh/docs/kratos/concepts/identity-user-model) in the system. + * Lists all [identities](https://www.ory.sh/docs/kratos/concepts/identity-user-model) in the system. Note: filters cannot be combined. * @param ctx context.Context - for authentication, logging, cancellation, deadlines, tracing, etc. Passed from http.Request or context.Background(). * @return IdentityAPIApiListIdentitiesRequest */ @@ -2137,7 +2137,7 @@ func (r IdentityAPIApiListIdentitiesRequest) Execute() ([]Identity, *http.Respon /* * ListIdentities List Identities - * Lists all [identities](https://www.ory.sh/docs/kratos/concepts/identity-user-model) in the system. + * Lists all [identities](https://www.ory.sh/docs/kratos/concepts/identity-user-model) in the system. Note: filters cannot be combined. * @param ctx context.Context - for authentication, logging, cancellation, deadlines, tracing, etc. Passed from http.Request or context.Background(). * @return IdentityAPIApiListIdentitiesRequest */ diff --git a/internal/httpclient/api_identity.go b/internal/httpclient/api_identity.go index 9e4aec1b6c58..b7dd012adf83 100644 --- a/internal/httpclient/api_identity.go +++ b/internal/httpclient/api_identity.go @@ -227,7 +227,7 @@ type IdentityAPI interface { /* * ListIdentities List Identities - * Lists all [identities](https://www.ory.sh/docs/kratos/concepts/identity-user-model) in the system. + * Lists all [identities](https://www.ory.sh/docs/kratos/concepts/identity-user-model) in the system. Note: filters cannot be combined. * @param ctx context.Context - for authentication, logging, cancellation, deadlines, tracing, etc. Passed from http.Request or context.Background(). * @return IdentityAPIApiListIdentitiesRequest */ @@ -2137,7 +2137,7 @@ func (r IdentityAPIApiListIdentitiesRequest) Execute() ([]Identity, *http.Respon /* * ListIdentities List Identities - * Lists all [identities](https://www.ory.sh/docs/kratos/concepts/identity-user-model) in the system. + * Lists all [identities](https://www.ory.sh/docs/kratos/concepts/identity-user-model) in the system. Note: filters cannot be combined. * @param ctx context.Context - for authentication, logging, cancellation, deadlines, tracing, etc. Passed from http.Request or context.Background(). * @return IdentityAPIApiListIdentitiesRequest */ diff --git a/spec/api.json b/spec/api.json index 907845a46f7c..87650d834bb7 100644 --- a/spec/api.json +++ b/spec/api.json @@ -3930,7 +3930,7 @@ }, "/admin/identities": { "get": { - "description": "Lists all [identities](https://www.ory.sh/docs/kratos/concepts/identity-user-model) in the system.", + "description": "Lists all [identities](https://www.ory.sh/docs/kratos/concepts/identity-user-model) in the system. Note: filters cannot be combined.", "operationId": "listIdentities", "parameters": [ { diff --git a/spec/swagger.json b/spec/swagger.json index 031ad06841ba..9071e0d298bf 100755 --- a/spec/swagger.json +++ b/spec/swagger.json @@ -171,7 +171,7 @@ "oryAccessToken": [] } ], - "description": "Lists all [identities](https://www.ory.sh/docs/kratos/concepts/identity-user-model) in the system.", + "description": "Lists all [identities](https://www.ory.sh/docs/kratos/concepts/identity-user-model) in the system. Note: filters cannot be combined.", "produces": [ "application/json" ],