cdimascio / express-openapi-validator

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

response validation allows schema to pass that has some differences #89

Open cdimascio opened 5 years ago

cdimascio commented 5 years ago

The following was posted by @LadIQe from #88. Copying details here

when I change response schema in OpenAPI documentation to test, if response validation works, and I execute it in swagger ui, I get no error. I looked into console, if something is there, but console/terminal is clear

As you can see on image below, my response schema is little bit different to test if validation works. But there is no error

Here is code in my app.js

const express = require('express')
const swaggerUi = require('swagger-ui-express')
const cookieParser = require('cookie-parser')
const bodyParser = require('body-parser')
const OpenApiValidator = require('express-openapi-validator').OpenApiValidator
const documentation = require('./components/documentation')
const app = express()

app.use(bodyParser.json())
app.use(bodyParser.text())
app.use(bodyParser.urlencoded())
app.use(express.json());
app.use(express.urlencoded({ extended: false }));
app.use(cookieParser())

app.use('/', swaggerUi.serve, swaggerUi.setup(documentation))

new OpenApiValidator({
  apiSpec: documentation,
  validateResponses: true,
  securityHandlers: {
    bearerAuth: (req, scopes, schema) => {
      console.log(req, scopes, schema)
    }
  }
}).install(app)

app.use((err, req, res, next) => {
  // format error
  console.error(err, err.stack);
  if (!err.status && !err.errors) {
    res.status(500).json({
      errors: [
        {
          message: err.message,
        },
      ],
    })
  } else {
    res.status(err.status).json({
      message: err.message,
      errors: err.errors,
    })
  }
})

module.exports = app
cdimascio commented 5 years ago

@LadIQe Ajv does not support a default of additionalProperties: false since it diverges from JSON schema. OpenAPI, I believe does use this as the default (ticket reference here. In any case, this is likely the root of the issue.

A possible solution would be for express-openapi-validator to recursively traverse the spec and add additionalProperties: false, if additionalProperties is not specified. This requires some work , but is likely worth doing to ensure behavior is consistent with OpenAPI 3.

In any case, as a workaround (for now), you should be able to manually add additionalProperties: false to your response models. This should cause a validation failure to trigger when additional properties are present.

Please give this a try and let me know if it solves (err works around) this issue you are experiencing.

LadIQe commented 5 years ago

Hi, I tried your workaround, but it doesnt work

Here is original response for logged user

LoggedUser: {
    type: 'object',
    properties: {
      token_type: {
        type: 'string'
      },
      expires_in: {
        $ref: '#/components/schemas/number'
      },
      access_token: {
        type: 'string'
      },
      refresh_token: {
        type: 'string'
      },
      user: {
        type: 'object',
        required: ['id'],
        properties: {
          id: {
            $ref: '#/components/schemas/number'
          }
        }
      }
    }
  }

I have tried these versions

LoggedUser: {
    type: 'object',
    properties: {
      token_type: {
        type: 'string'
      },
      expires_in: {
        $ref: '#/components/schemas/number'
      },
      access_token: {
        type: 'string'
      },
      refresh_token: {
        type: 'string'
      },
      user: {
        type: 'object',
        required: ['id'],
        properties: {
          id: {
            $ref: '#/components/schemas/number'
          }
        }
      }
    },
    additionalProperties: false
  }

LoggedUser: {
    type: 'object',
    properties: {
      token_type: {
        type: 'string'
      },
      expires_in: {
        $ref: '#/components/schemas/number'
      },
      access_token: {
        type: 'string'
      },
      refresh_token: {
        type: 'string'
      },
      user: {
        type: 'object',
        required: ['id'],
        properties: {
          id: {
            $ref: '#/components/schemas/number'
          }
        },
        additionalProperties: false
      }
    }
  }

Even these versions

LoggedUser: {
  type: 'object',
  additionalProperties: {
    type: 'object',
    properties: {
      token_type: {
        type: 'string'
      },
      expires_in: {
        $ref: '#/components/schemas/number'
      },
      access_token: {
        type: 'string'
      },
      refresh_token: {
        type: 'string'
      },
      user: {
        type: 'object',
        required: ['id'],
        properties: {
          id: {
            $ref: '#/components/schemas/number'
          }
        }
      }
    }
  }
}

LoggedUser: {
    type: 'object',
    properties: {
      token_type: {
        type: 'string'
      },
      expires_in: {
        $ref: '#/components/schemas/number'
      },
      access_token: {
        type: 'string'
      },
      refresh_token: {
        type: 'string'
      },
      user: {
        type: 'object',
        required: ['id'],
        additionalProperties: {
          type: 'object',
          properties: {
            id: {
              $ref: '#/components/schemas/number'
            }
          }
        }
      }
    }
  }

But none of them worked. Maybe Im doing something wrong, so feel free to alert me

cdimascio commented 5 years ago

@LadIQe i've made some modifications to the response validator to add support for additionalProperties. additionalProperties: false still must be explicitly set

Here is an example test case for reference:

https://github.com/cdimascio/express-openapi-validator/blob/master/test/resources/response.validation.yaml#L140

https://github.com/cdimascio/express-openapi-validator/blob/master/test/resources/response.validation.yaml#L82

top level additional prop: https://github.com/cdimascio/express-openapi-validator/blob/master/test/response.validation.spec.ts#L71

nested additional prop: https://github.com/cdimascio/express-openapi-validator/blob/master/test/response.validation.spec.ts#L90

This is available in v.2.13.0

LadIQe commented 5 years ago

@cdimascio thank you for your reply. Unfortunately, it still doesnt work. Validator can validate my documentation (when I make mistake), but still it doesnt validate swagger-ui-express

Idk, maybe problem is somewhere here:

const express = require('express')
const swaggerUi = require('swagger-ui-express')
const cookieParser = require('cookie-parser')
const bodyParser = require('body-parser')
const OpenApiValidator = require('express-openapi-validator').OpenApiValidator
const documentation = require('./components/documentation')
const app = express()

app.use(bodyParser.json())
app.use(bodyParser.text())
app.use(bodyParser.urlencoded())
app.use(express.json());
app.use(express.urlencoded({ extended: false }));
app.use(cookieParser())

app.use('/', swaggerUi.serve, swaggerUi.setup(documentation))  //  <----- here maybe?

new OpenApiValidator({  //  <----------- or here?
  apiSpec: documentation,
  validateResponses: true,
  securityHandlers: {
    bearerAuth: (req, scopes, schema) => {
      console.log(req, scopes, schema)
    }
  }
}).install(app)

app.use((err, req, res, next) => {
  // format error
  console.error(err, err.stack);
  if (!err.status && !err.errors) {
    res.status(500).json({
      errors: [
        {
          message: err.message,
        },
      ],
    })
  } else {
    res.status(err.status).json({
      message: err.message,
      errors: err.errors,
    })
  }
})

module.exports = app
LadIQe commented 5 years ago

@cdimascio ok again I changed order of swagger-ui and validator, now I got this:

starter-node-documentation       | { stack:
starter-node-documentation       |    'Error: not found\n    at Object.validationError (/usr/app/node_modules/express-openapi-validator/dist/middlewares/util.js:23:25)\n    at /usr/app/node_modules/express-openapi-validator/dist/middlewares/openapi.security.js:16:32\n    at Layer.handle [as handle_request] (/usr/app/node_modules/express/lib/router/layer.js:95:5)\n    at trim_prefix (/usr/app/node_modules/express/lib/router/index.js:317:13)\n    at /usr/app/node_modules/express/lib/router/index.js:284:7\n    at Function.process_params (/usr/app/node_modules/express/lib/router/index.js:335:12)\n    at next (/usr/app/node_modules/express/lib/router/index.js:275:10)\n    at /usr/app/node_modules/express-openapi-validator/dist/middlewares/openapi.multipart.js:37:13\n    at Layer.handle [as handle_request] (/usr/app/node_modules/express/lib/router/layer.js:95:5)',
starter-node-documentation       |   message: 'not found',
starter-node-documentation       |   toJSON: [Function: toJSON],
starter-node-documentation       |   status: 404,
starter-node-documentation       |   errors: [ { path: '/', message: 'not found', errors: undefined } ],
starter-node-documentation       |   name: 'Error',
starter-node-documentation       |   toString: [Function: toString] }