Dredd: additionalProperties and required fields ignored when using a schema described with allOf

[gac3k]

Describe the bug
A similar problem has already been reported here in past, and was related to fact, that when Swagger 2 schema is not de-referenced, the more strict validation (additionalProperties and required properties, are not being validated). That can be solved by well... de-referencing the schema. But then, there is another problem. De-referencing is not merging the allOf references (which is an expected behavior) and it seems that Dredd can't handle such behavior properly. Meaning it can interpret the schema and execute the request, but additionalProperties and required part of schema seems to be ignored completely.

To Reproduce

1. Create a de-referenced schema containing allOf parts
2. Run dredd tests against that schema

Expected behavior
additionalProperties and require flags should be considered during test.

What is in your dredd.yml?

color: true
dry-run: false
hookfiles: 'test/hooks.js'
language: nodejs
require: null
server: 'npm start'
server-wait: 3
init: false
custom: {}
names: false
only: []
reporter: html
output: []
header: []
sorted: false
user: null
inline-errors: true
details: true
method: []
loglevel: error
path: []
hooks-worker-timeout: 5000
hooks-worker-connect-timeout: 1500
hooks-worker-connect-retry: 500
hooks-worker-after-connect-wait: 100
hooks-worker-term-timeout: 5000
hooks-worker-term-retry: 500
hooks-worker-handler-host: 127.0.0.1
hooks-worker-handler-port: 61321
config: ./dredd.yml
blueprint: docs/swagger2.yml
endpoint: 'http://localhost:61355'

What's your dredd --version output?

dredd v12.2.0 (Linux 5.0.0-37-generic; x64)

[gac3k]

To give you a bit more of a details about the example schema, here it is:

      "VehicleResponse": {
        "allOf": [
          {
            "properties": {
              "id": {
                "format": "uuid",
                "type": "string"
              }
            },
            "required": [
              "id",
              "make",
              "body",
            ],
            "type": "object"
          },
          {
            "additionalProperties": false,
            "properties": {
              "additional_details": {
                "type": "object",
                "x-nullable": true
              },
              "additional_equipment": {
                "type": "array",
                "x-nullable": true
              },
              "body": {
                "type": "string"
              },
              "make": {
                "maxLength": 100,
                "type": "string"
              },
     }
}

The following schema should except no additional properties in the response, but while the schema is properly merged while testing, that flag is completely ignored.

[gac3k]

I've also found a workaround that seems to work, which is to specify at the hooks level, a snippet like this.

const { merge } = require('lodash');
const expectedBody = JSON.parse(transaction.expected.bodySchema);

if(expectedBody.hasOwnProperty('allOf')) {
    const mergedSchema = expectedBody.allOf.reduce((a, b) => {
    return merge(a, b);
});

transaction.expected.bodySchema = JSON.stringify(mergedSchema);

Which is basically flatting up expected body schema if the allOf property is existing.
That seems to work, but I am not a big fan of it, so hopefully we can figure out something better :)

[artem-zakharchenko]

Hi, @gac3k. Thanks for reporting this.

Currently de-referenced parts of a schema placed in allOf are ignored. The reason for that is the JSON Schema validator Gavel (a Dredd's dependency that does actual validation) is using. We are currently working on updating the validator there and I can already confirm that allOf, additionalProperties, and required are working as expected with the updated validator. We are going to finalize the work and propagate the update to Dredd soon. Thanks for your patience.

On the other hand, even with the updated JSON Schema validator, I don't think this is a valid JSON Schema:

{
  allOf: [
    {
      properties: {
        id: {
          format: 'uuid',
          type: 'string'
        }
      },
      required: ['id', 'make', 'body'],
      type: 'object'
    },
    {
      additionalProperties: false,
      properties: {
        additional_details: {
          type: 'object',
          'x-nullable': true
        },
        additional_equipment: {
          type: 'array',
          'x-nullable': true
        },
        body: {
          type: 'string'
        },
        make: {
          maxLength: 100,
          type: 'string'
        }
      }
    }
  ]
}

Two parts of the allOf declare a contradictory behavior:

• id is required in the first member,
• id is forbidden in the second member (not listed among known properties with additionalProperties: false

Merging the parts of allOf as you suggested does solve the issue, but I don't think Dredd, Gavel or even a JSON Schema validator should be doing that.

I don't think that's a correct declaration per JSON Schema spec (see the section that mentions additionalProperties). When such schema is provided to the validator it never resolves the id property.

What I think may be happening is that due to the JSON Schema validator's limitations Dredd users were suggested to de-reference the schema in the past, while with the validator's update it won't be necessary anymore. If you happen to stumble upon the issue that suggested de-referencing, please, post it here so I could verify that in a regression test.

[gac3k]

@artem-zakharchenko

I don't think this is a valid JSON Schema

You're right. I was writing that by heart, so it was just to more or less show where the problem is. Anyway thanks for the information and for the great work by far!

My specific use case is kinda untypical, because I am using the schema automatically generated from AWS Api Gateway, then I am transferring it and adding some information to the schema, as also I am modifying the whole thing using hooks, so not sure if the raw schema with references will help you with regression testing, but I'll attach it here later :)

For now will go with workaround provided above, and once update is here, I am happy to pull the changes ;)

Once again
Thanks!

[kylef]

@gac3k I think one of the underlying problems is https://github.com/apiaryio/api-elements.js/issues/399, which is that Dredd (Gavel specifically) treats the particular schema as Draft 3 which does not support allOf. Another work around could be adding the missing $schema property to identify the JSON Schema as Draft 4:

const schema = JSON.parse(transaction.expected.bodySchema);
schema.$schema = 'http://json-schema.org/draft-04/schema#';
transaction.expected.bodySchema = JSON.stringify(schema);

Although solving https://github.com/apiaryio/api-elements.js/issues/399 wouldn't be too hard but I am not sure when we will get to fixing it; hopefully soon.

[Relequestual]

Hey all.
OpenAPI 3.0 and prior has a subset superset of JSON Schema, meaning you can't simply use an existing JSON Schema library for validation, unless they specifically have an OpenAPI mode.

OpenAPI 3.1 will support full JSON Schema draft 2019-09 (formally draft-08). I know it's going to take time for tooling to catch up, but this is a heads up, having stumbled onto this issue.

[kylef]

@gac3k We've resolved this issue in Dredd 13.0.1, the work around should no longer be necessary.