cdimascio / express-openapi-validator

🦋 Auto-validates api requests, responses, and securities using ExpressJS and an OpenAPI 3.x specification
MIT License
906 stars 205 forks source link

oneOf with (additionalProperties: false) and (validateResponses: removeAdditional setting) does not work as expected #740

Open vileanco opened 2 years ago

vileanco commented 2 years ago

Describe the bug It looks like removeAdditional: "failing" response validation fails. I feel like the validator first tries to validate against type A and in progress removes property "extra", then it passes this to validate against B and it now also fails since "extra" field is missing. Note that the schema works if I change the order of TypeA and TypeB in the oneOf property.

To Reproduce I've added a test case that fails in my fork of this repo, link to commit:

https://github.com/vileanco/express-openapi-validator/commit/1963bd6d0f3dc101290df945d792bcc6fcc5c72f

Actual behavior Response validation fails with 500 "Internal server error". Errors returned

errors: [
    {
      path: '/response/type',
      message: 'must be equal to one of the allowed values: A',
      errorCode: 'enum.openapi.validation'
    },
    {
      path: '/response/extra',
      message: "must have required property 'extra'",
      errorCode: 'required.openapi.validation'
    },
    {
      path: '/response',
      message: 'must match exactly one schema in oneOf',
      errorCode: 'oneOf.openapi.validation'
    }
  ]

Expected behavior Response validation should pass.

Examples and context

Validator options:

{
        apiSpec,
        validateApiSpec: true,
        validateRequests: true,
        validateResponses: { removeAdditional: 'failing' },
}

Spec:

openapi: "3.0.2"
info:
  version: 1.0.0
  title: requestBodies $ref
  description: requestBodies $ref Test

servers:
  - url: /v1

paths:
  /one_of:
    post:
      requestBody:
        required: true
        content:
          application/json:
            schema:
              oneOf:
                - $ref: "#/components/schemas/TypeA"
                - $ref: "#/components/schemas/TypeB"
      responses:
        "200":
          description: OK
          content:
            application/json:
              schema:
                oneOf:
                  - $ref: "#/components/schemas/TypeA"
                  - $ref: "#/components/schemas/TypeB"

        "400":
          description: Bad Request

components:
  schemas:
    TypeA:
      type: object
      required:
        - id
        - type
      additionalProperties: false
      properties:
        id:
          type: string
        type:
          type: string
          enum:
            - A

    TypeB:
      type: object
      required:
        - id
        - type
        - extra
      additionalProperties: false
      properties:
        id:
          type: string
        type:
          type: string
          enum:
            - B
        extra:
          type: string
vileanco commented 2 years ago

Looking further into ajv docs https://ajv.js.org/guide/modifying-data.html#removing-additional-properties it looks like this is expected behavior. So I tried this also with the suggestion of adding discriminator property but still seeing same behavior.

Tianle-Leviathan commented 1 year ago

I am running into the same issue here and after some debugging, I find that express-openapi-validator does not pass distriminator: true to ajv and the generated validate function is incorrect. To generate the correct one, discriminator: true has to be set. But there is no way to explicitly config ajv as express-openapi-validator only extracts coerceTypes, removeAdditional.

PedramMarandi commented 3 months ago

Same issue here

willtpwise commented 2 months ago

Same here

patricktyndall commented 2 weeks ago

Seems like a lot of these problems are due to the OpenAPI spec itself simply does not support using additionalProperties: false this way with combinators like anyOf, allOf.

From other discussion it seems this is fixed by the unevaluatedProperties: false which is now supported by JSON Schema (and thus OAS 3.1), and AJV.

Seems like this PR will fix it