belgif / rest-guide

REST Guidelines of Belgian government institutions
https://www.belgif.be/specification/rest/api-guide/
Apache License 2.0
24 stars 4 forks source link

Handling undefined request parameters #110

Closed pvdbosch closed 3 months ago

pvdbosch commented 1 year ago

Formulate guidelines how an API should handle request parameters (in JSON body, query param or other) that aren't defined in OpenAPI.

For response elements, the guideline is for clients to ignore unknown properties to be able to evolve an API:

For request parameters, this makes less sense however:

A (very specific) counter-example is an Angular paging table view component which may send additional query params to an API that can be safely ignored.

OpenAPI (JSON Schema) allows additional JSON properties by default, but for query parameters the situation is less clear.

Some interesting reads on this topic:

pvdbosch commented 1 year ago

eHealth encountered a problem where a typo in a client did go unnoticed because it wasn't validated, so seems useful to them.

Default Axway APIGW behavior is to strip unknown query params which is also quite dangerous (but this behavior can be changed to pass through any unknown query param).

Other use case: support for complex search queries on a lot of parameters, e.g. "?age=...", "?age_lt=...", "?age_gt=..."

For JSON payloads, strictly speaking additional properties are by default allowed, but this behavior can probably be configured in most validators (I'll verify this). Using a different schema for request (disallowing additional props) and response (allowing additional props) has too much impact however.

WG decided that:

We could consider standardizing upon an OpenAPI extension keyword to declare support of undefined parameters (e.g. x-additional-query-params: like OpenAPI's additionalProperties/patternProperties), but this doesn't seem worthwhile for now.

I'll create a PR with a rule.

We should still discuss if another type of input validation issue than "schemaViolation" should be returned within the bad request problem response.

pvdbosch commented 1 year ago

I couldn't find any option to configure commonly used openapi validators to fail on unknown JSON properties during request processing.

One feature request was refused, saying that the schema should be changed instead to additionalProperties: false (e.g. with some pre-processing mechanism). This could be a hassle, cause this makes it difficult to use the same schema in request and response. For query parameters, there's no way to specify in OpenAPI that unknown params should be (dis)allowed: https://github.com/OAI/OpenAPI-Specification/issues/2511

The Jackson library can be tuned to fail or ignore unknown JSON properties:

pvdbosch commented 1 year ago

Proposal for issue type: urn:problem-type:belgif:input-validation:unknownRequestParameter (within badRequest problem response)

jpraet commented 1 year ago

For consistency with https://github.com/belgif/rest-guide/issues/113, where issue types like urn:problem-type:belgif:input-validation:mutuallyExclusiveParameters and urn:problem-type:belgif:input-validation:incompleteParameterGroup are proposed:

maybe urn:problem-type:belgif:input-validation:unknownParameter?

Or should we add the Request to the other issue types? urn:problem-type:belgif:input-validation:mutuallyExclusiveRequestParameters, urn:problem-type:belgif:input-validation:incompleteRequestParameterGroup? That becomes quite verbose.

pvdbosch commented 1 year ago

I agree to drop "request", it's redundant with "input-validation"

pvdbosch commented 1 year ago

TODO: check if those using GCloud's APIGW can work together for implementation Agreed to standardize on 'urn:problem-type:belgif:input-validation:unknownParameter'

(only in combination with in: query or in: body ; in : path => ResourceNotFound , in: header => unknown is allowed for http headers)

jpraet commented 1 year ago

Maybe an explicit mention should be made in the guidelines that this rule does not apply for API's that consume JSON events / notifications / webhooks via an HTTP POST? A new property added by the publisher of the events should not cause an error at the consumer side. Just like unknown properties should be ignored by the client when consuming data retrieved from a server.

pvdbosch commented 1 year ago

Unknown parameters should indeed be accepted in any webhook/callback request. Webhooks/callbacks aren't documented in the guide however, and I don't yet know of any implementations following the guide using them; so mentioning them could cause more confusion than clarity.

We'd also have to clearly define semantics around callbacks (client, service, API, ...) to avoid confusion because the API (application programming interface) is defined by the sender of the events in such a case.

jpraet commented 3 months ago

Can this be closed? https://www.belgif.be/specification/rest/api-guide/#rule-req-valid