Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON request bodies are not strictly validated #837

Closed
lsorber opened this issue Jan 3, 2019 · 7 comments
Closed

JSON request bodies are not strictly validated #837

lsorber opened this issue Jan 3, 2019 · 7 comments

Comments

@lsorber
Copy link

lsorber commented Jan 3, 2019

Description

GitHub is starting to apply strict validation in their REST API [1].

Connexion already has a strict_validation flag [2], but it doesn't apply to the request body if its content type is JSON.

Expected behaviour

I would appreciate it if strict validation also validates JSON request bodies.

Related issue: #223
CC: @dsem

[1] https://developer.github.com/changes/2018-09-25-stricter-validation-coming-soon-in-the-rest-api/
[2] https://github.com/zalando/connexion/blob/master/docs/request.rst#parameter-validation

@dtkav
Copy link
Collaborator

dtkav commented Jan 8, 2019

Hey @lsorber - Can you say more about how the the GitHub change is related to this ticket?

AFAICT strict_validation in connexion was created error on extra form-parameters in OpenAPI 2.0.

In OpenAPI 3.0.x I believe you can use the additionalProperties flag.
If you set additionalProperties: false the jsonschema validator should complain if you try to include fields that aren't in the spec.

The OpenAPI folks decided that additionalProperties should be true by default. This was surprising to me also, but does match the behavior of many popular REST APIs.

We could potentially have strict_validation setdefault additionalProperties: false in the users' spec, but I think it might be better if the user sets this explicitly in their spec.

What do you think?

@kornicameister
Copy link
Contributor

kornicameister commented Jan 23, 2019

So what's the scope of strict_validation for Connexion and OAS3? What is strictly validated? Obviously, you're saying that request_body does not fall into that category and the only SOT is the schema. What about query_params or path_params and response bodies?

cc @dtkav

@dtkav
Copy link
Collaborator

dtkav commented Jan 24, 2019

AFAICT originally in swagger 2.0, strict_validation was created for form-parameters and query-parameters only. It raised an error if there were extra parameters.

It's unclear how strict_validation should be implemented in openapi3. I believe, per the spec, that allowing extra parameters should now be controlled by additionalProperties.

It has been a goal of mine to move the validation out of connexion and into the openapi-spec-validator as much as possible for separation of concerns.

@kornicameister
Copy link
Contributor

I think that that goal is actually correct. That means that once that goal is achieved strict_validation will become deprecated for connexion+OAS3?

Still, I am little puzzled as to what is currently still strictly validated under connexion+OAS3? I've been trying to follow the code to find on my own but I lost track quite fast :(. If you don't mind answering that question it'd be awesome and perhaps beneficial to others reading the thread. I mean, you wrote that it is unclear how strict_validation ought to be implemented for OAS3 and that's fine but it the same time that response does not answer if there is anything validated like that right now or just something or perhaps nothing.

Cheers 👍

@dtkav
Copy link
Collaborator

dtkav commented Jan 28, 2019

Sorry @kornicameister - this state of validation is pretty confusing (I have been trying to fix it with #760)

The strict_validation flag (regardless of spec version) checks for additional keys in two places:

  1. query parameters
  2. form data

The biggest problem with connexion validation at the moment comes from this line:
https://github.com/zalando/connexion/blob/9b76bbc8fc4d1742b78b663cdec8e2df3b41ec86/connexion/decorators/validation.py#L117

Which basically means that if your API endpoint doesn't consume only json data, the body won't be validated (!). The only exception to this is if the endpoint consumes[0] is form-data (!).

This issue has been around for a long time (before I started working on the project), and it means that connexion is significantly less useful for non-json (or even mixed content-type) apis.

I'm attempting to fix it with #760

@lsorber
Copy link
Author

lsorber commented Jan 29, 2019

@dtkav I agree that if strict validation can be achieved with OpenAPI, that would be preferable.

Unfortunately, it is not possible to combine additionalProperties: false with OpenAPI's allOf-style inheritance, see e.g., [1]. This is a pretty common use case I would imagine, so it appears that there is no good solution for strict validation of JSON request bodies at this time.

One possible solution might be to add support for x-oas-draft-alternateSchemas (#861), so that newer JSON Schema drafts might offer better support for merging schemas.

[1] https://stackoverflow.com/questions/22689900/json-schema-allof-with-additionalproperties

@dtaniwaki
Copy link

I still have an issue using oneOf in a formdata with v2.7.0. Are we going to merge #760 or something to fix it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants