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

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

Open
vileanco opened this issue Jun 10, 2022 · 5 comments

Comments

@vileanco
Copy link

vileanco commented Jun 10, 2022

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:

vileanco@1963bd6

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
Copy link
Author

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
Copy link

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
Copy link

Same issue here

@willtpwise
Copy link

Same here

@patricktyndall
Copy link

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

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

No branches or pull requests

5 participants