Gi60s / openapi-enforcer

Apache License 2.0
94 stars 22 forks source link

Swagger 2.0 additionalProperties behavior #131

Closed cyberflohr closed 2 years ago

cyberflohr commented 2 years ago

Parsing a swagger 2.0 with the openapi-enforcer will set automatically additionalProperties = true on all defined schema objects. During payload validation unknown properties are not reported, since the schema is open for extension.

If I understand the text in this issue https://github.com/byu-oit/openapi-enforcer/issues/2 correct, the default for Swagger 2.0 specs should be false.

How can I solve this issue - thank you!

Openapi-enforcer: 1.16.1

Gi60s commented 2 years ago

Hello @cyberflohr.

Thank you for using the library and for asking the question.

The current behavior is correct. If you care to know why please read the section below titled Why Does additionalProperties Default to true

As for #2, I don't know if we ever implemented any code for this issue. I believe the issue was simply marked as resolved by the issue submitter. We could probably add the functionality to identify additional properties or you could also set additionalProperties to false for your schemas. A final solution (and the one I use most often) is to ignore any additional properties.

Let me know if this answers your question or resolves your issue.

Why Does additionalProperties Default to true

In regards to the Schema Object's additionalProperties property, the OpenAPI spec 2.0 defers to the JSON Schema. You can see this in the docs here https://spec.openapis.org/oas/v2.0#schema-object. I've made some edits for clarity, but it says:

The [additionalProperties is] taken from the JSON Schema definition...[The additionalProperties] definition is the same as the one from JSON Schema...

Looking at the JSON schema specification here http://json-schema.org/understanding-json-schema/reference/object.html#additional-properties, it says:

By default any additional properties are allowed.

The default to allow is important when it comes to using an allOf schema. Without additionalProperties equal to true, the evaluated object would fail on one or more of the schemas under the allOf. For example:

allOf:
  - type: object
    properties:
      name:
        type: string
  - type: object
    properties:
      age:
        type: integer

If additionalProperties defaulted to false and you tested a value { name: 'Bob', age: 25 } against this allOf schema it would fail the first schema because it included the property age and it would fail the second schema because it failed the property name.

Validating the value against each schema comes from the OpenAPI spec itself. At https://spec.openapis.org/oas/v2.0#composition-and-inheritance-polymorphism it says:

allOf takes in an array of object definitions that are validated independently

cyberflohr commented 2 years ago

Hi @Gi60s, I really appreciate the time and effort you spend into openapi-enforcer development and answering questions - thank you!

In the meantime (with the information you provided, reading Specs and other sources on the web) I agree to you - the default for additionalProperties must be true.

Really funny, because I use OpenAPI for several years now, but was not aware about this fact.

FYI: For validation purposes it could be of interest to detect non explicitly defined properties. The solution which was implemented in the swagger-request-validator is maybe something which can be added to openapi-enforcer too.

https://bitbucket.org/atlassian/swagger-request-validator/src/master/docs/FAQ.md

Gi60s commented 2 years ago

Thanks for sharing that link.

I'm working on version 2 of this library and you can see the progress in the v2 branch of this repo. It will have the ability to modify error levels for some errors, although seeing this, it makes me consider that there may be some cases for being able to modify all error levels.

There will be 4 possible error levels: error, warning, info, ignore. Any error that can have its level modified can be modified globally or on a per-instance basis.

Hopefully, we'll have an alpha release of version 2 soonish.