Hilzu / express-openapi-validate

Express middleware to validate requests based on an OpenAPI 3 document
Apache License 2.0
75 stars 12 forks source link

Schema composition breaks with additionalProperties: false #51

Open tariq-tb opened 5 years ago

tariq-tb commented 5 years ago

Schema composition doesn't appear to work with additionalProperties: false.

openapi.yaml

openapi: 3.0.1
components:
  schemas:
    Pet:
      type: object
      properties: 
        name:
          type: string
        age:
          type: integer
      required: 
        - name
        - age
      additionalProperties: false
    Dog:
      allOf: 
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            bark:
              type: boolean
          required:
            - bark
      additionalProperties: false

paths:
  /simple:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Pet'
      responses:
        '200':
          content:
            application/json:
              schema:
                type: object
  /complex:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Dog'
      responses:
        '200':
          content:
            application/json:
              schema:
                type: object

Simple schema

  1. Hitting /simple with the following works as expected.
{
    "name": "hunter",
    "age": 12,
}
  1. Including an extra property foo in the request...
{
    "name": "hunter",
    "age": 12,
    "foo": true
}

...returns an error as expected because additionalProperties: false is set on the Pet schema.

{
    "error": {
        "name": "ValidationError",
        "message": "Error while validating request: request.body should NOT have additional properties",
        "data": [
            {
                "keyword": "additionalProperties",
                "dataPath": ".body",
                "schemaPath": "#/properties/body/additionalProperties",
                "params": {
                    "additionalProperty": "foo"
                },
                "message": "should NOT have additional properties"
            }
        ]
    }
}

Complex schema with composition

On the other hand, hitting /complex with the following...

{
    "name": "hunter",
    "age": 12,
    "bark": true
}

...returns an unexpected error.

{
    "error": {
        "name": "ValidationError",
        "message": "Error while validating request: request.body should NOT have additional properties",
        "data": [
            {
                "keyword": "additionalProperties",
                "dataPath": ".body",
                "schemaPath": "#/properties/body/additionalProperties",
                "params": {
                    "additionalProperty": "name"
                },
                "message": "should NOT have additional properties"
            }
        ]
    }
}
keepersmith commented 4 years ago

I discovered this as well, and it is more of an issue with OAS than express-openapi-validate. To get it to work, you need to put in stubs for properties of Pet, and use additionalProperties: false in the object explicity defined in Dog; like so:

Dog:
  allOf: 
    - $ref: '#/components/schemas/Pet'
    - type: object
      properties:
        bark:
          type: boolean
        name: {}
        age: {}
      required:
        - bark
        - name
        - age
      additionalProperties: false
tariq-tb commented 4 years ago

I agree this might not be an issue with express-openapi-validate, just opening up the discussion here, happy to move it elsewhere. Tried your solution, but it did not resolve the issue.

Using your suggestions:

openapi: 3.0.1
components:
  schemas:
    Pet:
      type: object
      properties: 
        name:
          type: string
        age:
          type: integer
      required: 
        - name
        - age
      additionalProperties: false
    Dog:
      allOf: 
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            bark:
              type: boolean
            name: {}
            age: {}
          required:
            - bark
            - name
            - age
          additionalProperties: false
      # additionalProperties: false

...now results in:

{
    "error": {
        "name": "ValidationError",
        "message": "Error while validating request: request.body should NOT have additional properties",
        "data": [
            {
                "keyword": "additionalProperties",
                "dataPath": ".body",
                "schemaPath": "#/properties/body/allOf/0/additionalProperties",
                "params": {
                    "additionalProperty": "bark"
                },
                "message": "should NOT have additional properties"
            }
        ]
    }
}
thetumper commented 4 years ago

If you remove the additionalProperties in the Pet schema, it will no longer complain about bark. Specifying that in Pet conflicts with the set of properties in the Dog schema.

tariq-tb commented 4 years ago

That's correct, but then /simple allows extra properties which isn't ideal.

thetumper commented 4 years ago

True. Workaround would be to have Pet remain "generic" -- only used as ref by other types, which specify additionalProperties: false, and are the ones actually used.

I.e., specify this:


PetConcrete:
      allOf: 
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            name: {}
            age: {}
          additionalProperties: false

and then use PetConcrete for the /simple request body type, not Pet.

tariq-tb commented 4 years ago

Confirmed that your workaround along with @keepersmith's change does work, for better or worse. Updated openapi.yaml below.

components:
  schemas:
    Pet:
      type: object
      properties: 
        name:
          type: string
        age:
          type: integer
      required: 
        - name
        - age
      # additionalProperties: false
    Dog:
      allOf: 
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            bark:
              type: boolean
            name: {}
            age: {}
          required:
            - bark
          additionalProperties: false
    PetConcrete:
      allOf: 
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            name: {}
            age: {}
          additionalProperties: false
paths:
  /simple:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/PetConcrete'
      responses:
        '200':
          content:
            application/json:
              schema:
                type: object
  /complex:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Dog'
      responses:
        '200':
          content:
            application/json:
              schema:
                type: object