From b32b010aee180f97679f8da139fe27796a909d83 Mon Sep 17 00:00:00 2001 From: idanto Date: Sat, 10 Mar 2018 11:21:36 +0200 Subject: [PATCH] Add content type validation header (#27) - Add content type validation header (optional check) - Support for multiple types (by swagger spec) - Support check only if body exists. - Refactor Error - Fix NSP badge --- README.md | 4 +- package-lock.json | 38 +++++--- src/customKeywords/contentTypeValidation.js | 31 ++++++ src/customKeywords/files.js | 4 +- src/inputValidationError.js | 74 ++++++++++++++ src/middleware.js | 103 ++++++-------------- src/{ => utils}/validators.js | 0 test/middleware-test.js | 84 ++++++++++++++++ test/pet-store-swagger-inheritance.yaml | 2 + test/pet-store-swagger-with-base-path.yaml | 12 ++- test/pet-store-swagger.yaml | 71 ++++++++++++-- test/test-server-with-options.js | 9 +- test/test-simple-server-with-base-path.js | 6 +- 13 files changed, 331 insertions(+), 107 deletions(-) create mode 100644 src/customKeywords/contentTypeValidation.js create mode 100644 src/inputValidationError.js rename src/{ => utils}/validators.js (100%) diff --git a/README.md b/README.md index 1697f41..c1f31c5 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ [![NPM Downloads][downloads-image]][downloads-url] [![Build Status][travis-image]][travis-url] [![Test Coverage][coveralls-image]][coveralls-url] -[![NSP Status](https://nodesecurity.io/orgs/zooz/projects/91986a79-6151-44df-a6f4-b12982a8858a/badge)](https://nodesecurity.io/orgs/zooz/projects/91986a79-6151-44df-a6f4-b12982a8858a) +[![NSP Status](https://nodesecurity.io/orgs/zooz/projects/3244db73-7215-4526-8cb0-b5b1e640fc6e/badge)](https://nodesecurity.io/orgs/zooz/projects/3244db73-7215-4526-8cb0-b5b1e640fc6e) [![Apache 2.0 License][license-image]][license-url] This package is used to perform input validation to express app using a [Swagger (Open API)](https://swagger.io/specification/) definition and [ajv](https://www.npmjs.com/package/ajv) @@ -74,9 +74,9 @@ Options currently supports: - `firstError` - Boolean that indicates if to return only the first error. - `makeOptionalAttributesNullable` - Boolean that forces preprocessing of Swagger schema to include 'null' as possible type for all non-required properties. Main use-case for this is to ensure correct handling of null values when Ajv type coercion is enabled - - `ajvConfigBody` - Object that will be passed as config to new Ajv instance which will be used for validating request body. Can be useful to e. g. enable type coercion (to automatically convert strings to numbers etc). See Ajv documentation for supported values. - `ajvConfigParams` - Object that will be passed as config to new Ajv instance which will be used for validating request body. See Ajv documentation for supported values. +- `contentTypeValidation` - Boolean that indicates if to perform content type validation in case `consume` field is specified and the request body is not empty. ```js formats: [ diff --git a/package-lock.json b/package-lock.json index a742307..defecc3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -54,12 +54,6 @@ "json-schema-traverse": "0.3.1" } }, - "ajv-keywords": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/ajv-keywords/-/ajv-keywords-2.1.0.tgz", - "integrity": "sha1-opbhf3v658HOT34N5T0pyzIWLfA=", - "dev": true - }, "align-text": { "version": "0.1.4", "resolved": "https://registry.npmjs.org/align-text/-/align-text-0.1.4.tgz", @@ -2276,12 +2270,6 @@ "integrity": "sha1-VSmk1nZUE07cxSZmVoNbD4Ua/O4=", "dev": true }, - "mime": { - "version": "1.4.1", - "resolved": "https://registry.npmjs.org/mime/-/mime-1.4.1.tgz", - "integrity": "sha512-KI1+qOZu5DcW6wayYHSzR/tXKCDC5Om4s1z2QJjDULzLcmf3DvzS7oluY4HCTrc+9FiKmWUgeNLg7W3uIQvxtQ==", - "dev": true - }, "mime-db": { "version": "1.30.0", "resolved": "https://registry.npmjs.org/mime-db/-/mime-db-1.30.0.tgz", @@ -3301,6 +3289,12 @@ "ms": "2.0.0" } }, + "mime": { + "version": "1.4.1", + "resolved": "https://registry.npmjs.org/mime/-/mime-1.4.1.tgz", + "integrity": "sha512-KI1+qOZu5DcW6wayYHSzR/tXKCDC5Om4s1z2QJjDULzLcmf3DvzS7oluY4HCTrc+9FiKmWUgeNLg7W3uIQvxtQ==", + "dev": true + }, "statuses": { "version": "1.3.1", "resolved": "https://registry.npmjs.org/statuses/-/statuses-1.3.1.tgz", @@ -3540,9 +3534,17 @@ "form-data": "2.3.1", "formidable": "1.1.1", "methods": "1.1.2", - "mime": "1.4.1", + "mime": "1.6.0", "qs": "6.5.1", "readable-stream": "2.3.3" + }, + "dependencies": { + "mime": { + "version": "1.6.0", + "resolved": "https://registry.npmjs.org/mime/-/mime-1.6.0.tgz", + "integrity": "sha512-x0Vn8spI+wuJ1O6S7gnbaQg8Pxh4NNHb7KSINmEWKiPE4RKOplvijn+NkmYmmRgP68mc70j2EbeTFRsrswaQeg==", + "dev": true + } } }, "supertest": { @@ -3592,11 +3594,19 @@ "dev": true, "requires": { "ajv": "5.5.2", - "ajv-keywords": "2.1.0", + "ajv-keywords": "2.1.1", "chalk": "2.2.0", "lodash": "4.17.4", "slice-ansi": "1.0.0", "string-width": "2.1.1" + }, + "dependencies": { + "ajv-keywords": { + "version": "2.1.1", + "resolved": "https://registry.npmjs.org/ajv-keywords/-/ajv-keywords-2.1.1.tgz", + "integrity": "sha1-YXmX/F9gV2iUxDX5QNgZ4TW4B2I=", + "dev": true + } } }, "tcomb": { diff --git a/src/customKeywords/contentTypeValidation.js b/src/customKeywords/contentTypeValidation.js new file mode 100644 index 0000000..aaf600b --- /dev/null +++ b/src/customKeywords/contentTypeValidation.js @@ -0,0 +1,31 @@ +const Ajv = require('ajv'); + +module.exports = { + compile: function contentTypeValidation(schema) { + const regex = buildContentTypeRegex(schema.types); + return function contentValidation(data) { + contentValidation.errors = []; + if (Number(data['content-length']) > 0) { + if (!regex.test(data['content-type'])) { + contentValidation.errors.push(new Ajv.ValidationError({ + keyword: 'content-type', + message: 'content-type must be one of ' + schema.types, + params: { pattern: schema.pattern, types: schema.types, 'content-type': data['content-type'], 'content-length': data['content-length'] } + })); + return false; + } + } + return true; + }; + }, + errors: true +}; + +function buildContentTypeRegex(contentTypes) { + let pattern = ''; + contentTypes.forEach(type => { + pattern += `(${type.replace(/\//g, '\\/')}.*\\s*\\S*)|`; + }); + + return new RegExp(pattern.substring(0, pattern.length - 1)); +} \ No newline at end of file diff --git a/src/customKeywords/files.js b/src/customKeywords/files.js index f777c11..02509d4 100644 --- a/src/customKeywords/files.js +++ b/src/customKeywords/files.js @@ -21,7 +21,7 @@ module.exports = { if (missingFiles.length > 0) { filesValidation.errors.push(new Ajv.ValidationError({ keyword: 'files', - message: 'Missing required files: ' + missingFiles.toString(), + message: `Missing required files: ${missingFiles.toString()}`, params: { requiredFiles: schema.required, missingFiles: missingFiles } })); return false; @@ -33,7 +33,7 @@ module.exports = { if (extraFiles.length > 0) { filesValidation.errors.push(new Ajv.ValidationError({ keyword: 'files', - message: 'Extra files are not allowed. Not allowed files: ' + extraFiles, + message: `Extra files are not allowed. Not allowed files: ${extraFiles}`, params: { allowedFiles: allFiles, extraFiles: extraFiles } })); return false; diff --git a/src/inputValidationError.js b/src/inputValidationError.js new file mode 100644 index 0000000..be3bdaf --- /dev/null +++ b/src/inputValidationError.js @@ -0,0 +1,74 @@ +'use strict'; + +/** + * Represent an input validation error + * errors field will include the `ajv` error + * @class InputValidationError + * @extends {Error} + */ +class InputValidationError extends Error { + constructor(errors, path, method, options) { + super('Input validation error'); + + if (options.beautifyErrors && options.firstError) { + this.errors = this.parseAjvError(errors[0], path, method); + } else if (options.beautifyErrors) { + this.errors = this.parseAjvErrors(errors, path, method); + } else { + this.errors = errors; + } + } + + parseAjvErrors(errors, path, method) { + var parsedError = []; + errors.forEach(function (error) { + parsedError.push(this.parseAjvError(error, path, method)); + }, this); + + return parsedError; + } + + parseAjvError(error, path, method) { + if (error.dataPath.startsWith('.header')) { + error.dataPath = error.dataPath.replace('.', ''); + error.dataPath = error.dataPath.replace('[', '/'); + error.dataPath = error.dataPath.replace(']', ''); + error.dataPath = error.dataPath.replace('\'', ''); + error.dataPath = error.dataPath.replace('\'', ''); + } + + if (error.dataPath.startsWith('.path')) { + error.dataPath = error.dataPath.replace('.', ''); + error.dataPath = error.dataPath.replace('.', '/'); + } + + if (error.dataPath.startsWith('.query')) { + error.dataPath = error.dataPath.replace('.', ''); + error.dataPath = error.dataPath.replace('.', '/'); + } + + if (error.dataPath.startsWith('.')) { + error.dataPath = error.dataPath.replace('.', 'body/'); + } + + if (error.dataPath.startsWith('[')) { + error.dataPath = `body/${error.dataPath}`; + } + + if (error.dataPath === '') { + error.dataPath = 'body'; + } + + if (error.keyword === 'enum') { + error.message += ` [${error.params.allowedValues.toString()}]`; + } + + if (error.validation) { + error.message = error.errors.message; + } + + return `${error.dataPath} ${error.message}`; + } +} + +module.exports = InputValidationError; \ No newline at end of file diff --git a/src/middleware.js b/src/middleware.js index 2d0f437..059395f 100644 --- a/src/middleware.js +++ b/src/middleware.js @@ -2,8 +2,10 @@ var SwaggerParser = require('swagger-parser'), Ajv = require('ajv'), - Validators = require('./validators'), + Validators = require('./utils/validators'), filesKeyword = require('./customKeywords/files'), + contentKeyword = require('./customKeywords/contentTypeValidation'), + InputValidationError = require('./inputValidationError'), schemaPreprocessor = require('./utils/schema-preprocessor'); var schemas = {}; @@ -47,8 +49,14 @@ function init(swaggerPath, options) { let localParameters = parameters.filter(function (parameter) { return parameter.in !== 'body'; }).concat(pathParameters); - if (localParameters.length > 0) { - schemas[parsedPath][currentMethod].parameters = buildParametersValidation(localParameters); + + if (bodySchema.length > 0) { + schemas[parsedPath][currentMethod].body = buildBodyValidation(bodySchema[0].schema, dereferenced.definitions, swaggers[1], currentPath, currentMethod, parsedPath); + } + + if (localParameters.length > 0 || middlewareOptions.contentTypeValidation) { + schemas[parsedPath][currentMethod].parameters = buildParametersValidation(localParameters, + dereferenced.paths[currentPath][currentMethod].consumes || dereferenced.paths[currentPath].consumes || dereferenced.consumes); } }); }); @@ -67,6 +75,7 @@ function init(swaggerPath, options) { */ function validate(req, res, next) { let path = extractPath(req); + return Promise.all([ _validateParams(req.headers, req.params, req.query, req.files, path, req.method.toLowerCase()).catch(e => e), _validateBody(req.body, path, req.method.toLowerCase()).catch(e => e) @@ -76,13 +85,10 @@ function validate(req, res, next) { } return next(); }).catch(function (errors) { - if (middlewareOptions.beautifyErrors && middlewareOptions.firstError) { - errors = parseAjvError(errors[0], path, req.method.toLowerCase()); - } else if (middlewareOptions.beautifyErrors) { - errors = parseAjvErrors(errors, path, req.method.toLowerCase()); - } - - return next(new InputValidationError(errors)); + const error = new InputValidationError(errors, path, req.method.toLowerCase(), + { beautifyErrors: middlewareOptions.beautifyErrors, + firstError: middlewareOptions.firstError }); + return next(error); }); }; @@ -105,57 +111,6 @@ function _validateParams(headers, pathParams, query, files, path, method) { }); } -function parseAjvErrors(errors, path, method) { - var parsedError = []; - errors.forEach(function (error) { - parsedError.push(parseAjvError(error, path, method)); - }, this); - - return parsedError; -} - -function parseAjvError(error, path, method) { - if (error.dataPath.startsWith('.header')) { - error.dataPath = error.dataPath.replace('.', ''); - error.dataPath = error.dataPath.replace('[', '/'); - error.dataPath = error.dataPath.replace(']', ''); - error.dataPath = error.dataPath.replace('\'', ''); - error.dataPath = error.dataPath.replace('\'', ''); - } - - if (error.dataPath.startsWith('.path')) { - error.dataPath = error.dataPath.replace('.', ''); - error.dataPath = error.dataPath.replace('.', '/'); - } - - if (error.dataPath.startsWith('.query')) { - error.dataPath = error.dataPath.replace('.', ''); - error.dataPath = error.dataPath.replace('.', '/'); - } - - if (error.dataPath.startsWith('.')) { - error.dataPath = error.dataPath.replace('.', 'body/'); - } - - if (error.dataPath.startsWith('[')) { - error.dataPath = 'body/' + error.dataPath; - } - - if (error.dataPath === '') { - error.dataPath = 'body'; - } - - if (error.keyword === 'enum') { - error.message += ' [' + error.params.allowedValues.toString() + ']'; - } - - if (error.validation) { - error.message = error.errors.message; - } - - return error.dataPath + ' ' + error.message; -} - function addCustomKeyword(ajv, formats) { if (formats) { formats.forEach(function (format) { @@ -164,6 +119,7 @@ function addCustomKeyword(ajv, formats) { } ajv.addKeyword('files', filesKeyword); + ajv.addKeyword('content', contentKeyword); } function extractPath(req) { @@ -209,7 +165,15 @@ function buildInheritance(discriminator, dereferencedDefinitions, swagger, curre return new Validators.OneOfValidator(inheritsObject); } -function buildParametersValidation(parameters) { +function createContentTypeHeaders(validate, contentTypes) { + if (!validate || !contentTypes) return; + + return { + types: contentTypes + }; +} + +function buildParametersValidation(parameters, contentTypes) { const defaultAjvOptions = { allErrors: true, coerceTypes: 'array' @@ -278,20 +242,9 @@ function buildParametersValidation(parameters) { } }, this); - return ajv.compile(ajvParametersSchema); -} + ajvParametersSchema.properties.headers.content = createContentTypeHeaders(middlewareOptions.contentTypeValidation, contentTypes); -/** - * Represent an input validation error - * errors field will include the `ajv` error - * @class InputValidationError - * @extends {Error} - */ -class InputValidationError extends Error { - constructor(errors, place, message) { - super(message); - this.errors = errors; - } + return ajv.compile(ajvParametersSchema); } module.exports = { diff --git a/src/validators.js b/src/utils/validators.js similarity index 100% rename from src/validators.js rename to src/utils/validators.js diff --git a/test/middleware-test.js b/test/middleware-test.js index 65c7825..7924233 100644 --- a/test/middleware-test.js +++ b/test/middleware-test.js @@ -573,6 +573,25 @@ describe('input-validation middleware tests', function () { done(); }); }); + it('bad request - wrong content-type (should be application/json)', function (done) { + request(app) + .put('/v1/pets') + .set('content-type', 'application/x-www-form-urlencoded') + .send([{ + name: 'name', + tag: 'tag', + test: { + field1: 'enum1' + } + }]) + .expect(400, function (err, res) { + if (err) { + throw err; + } + expect(res.body.more_info).to.includes('content-type must be one of application/json'); + done(); + }); + }); it('headers are in capital letters - should pass validation', function (done) { request(app) .get('/v1/capital') @@ -1607,6 +1626,71 @@ describe('input-validation middleware tests', function () { done(); }); }); + it('bad request - wrong content-type (should be application/json)', function (done) { + request(app) + .put('/pets') + .set('content-type', 'application/x-www-form-urlencoded') + .send([{ + name: 'name', + tag: 'tag', + test: { + field1: 'enum1' + } + }]) + .expect(400, function (err, res) { + if (err) { + throw err; + } + expect(res.body.more_info).to.be.a('string'); + expect(res.body.more_info).to.includes('headers content-type must be one of application/json,form-data'); + done(); + }); + }); + it('valid content-type when multiple content-types defind - should pass validation', function (done) { + request(app) + .put('/text') + .set('content-type', 'text/plain') + .send('text') + .expect(200, function (err, res) { + if (err) { + throw err; + } + expect(res.body.result).to.equal('OK'); + done(); + }); + }); + it('more detailed content-type - should pass validation', function (done) { + request(app) + .put('/pets') + .set('content-type', 'application/json; charset=utf-8') + .send([{ + name: 'name', + tag: 'tag', + test: { + field1: 'enum1' + } + }]) + .expect(200, function (err, res) { + if (err) { + throw err; + } + expect(res.body.result).to.equal('OK'); + done(); + }); + }); + it('valid empty request - should pass validation', function (done) { + request(app) + .put('/pets/1234') + .set('request-id', '1234') + .set('api-version', '1.0') + .expect(200, function (err, res) { + if (err) { + throw err; + } + expect(res.body.result).to.equal('OK'); + done(); + }); + }); }); describe('Server with options - Only beautify errors', function () { var app; diff --git a/test/pet-store-swagger-inheritance.yaml b/test/pet-store-swagger-inheritance.yaml index e0b5e17..536981e 100644 --- a/test/pet-store-swagger-inheritance.yaml +++ b/test/pet-store-swagger-inheritance.yaml @@ -14,6 +14,8 @@ produces: - application/json paths: /pets: + consumes: + - application/json get: summary: List all pets operationId: listPets diff --git a/test/pet-store-swagger-with-base-path.yaml b/test/pet-store-swagger-with-base-path.yaml index dae4130..67ae3f5 100644 --- a/test/pet-store-swagger-with-base-path.yaml +++ b/test/pet-store-swagger-with-base-path.yaml @@ -9,12 +9,16 @@ basePath: /v1 schemes: - http consumes: - - application/json + - application/json produces: - - application/json + - application/json paths: /pets: get: + consumes: + - application/json + produces: + - application/json summary: List all pets operationId: listPets tags: @@ -53,6 +57,10 @@ paths: post: summary: Create a pet operationId: createPets + consumes: + - application/json + produces: + - application/json parameters: - $ref: '#/parameters/ApiRequestId' - name: body diff --git a/test/pet-store-swagger.yaml b/test/pet-store-swagger.yaml index b214705..2fb550c 100644 --- a/test/pet-store-swagger.yaml +++ b/test/pet-store-swagger.yaml @@ -7,10 +7,6 @@ info: host: petstore.swagger.io schemes: - http -consumes: - - application/json -produces: - - application/json paths: /pets: get: @@ -88,17 +84,16 @@ paths: operationId: updatePats tags: - pets + consumes: + - application/json + - form-data parameters: - name: body in: body - required: true schema: type: array items: type: object - required: - - name - - test properties: name: type: string @@ -130,7 +125,6 @@ paths: enum: ['enum1', 'enum2'] field2: type: integer - responses: "200": description: Expected response to a valid request @@ -143,6 +137,10 @@ paths: patch: summary: Info for a specific pet operationId: incrementallyUpdatePet + consumes: + - application/json + produces: + - application/json tags: - pets parameters: @@ -185,6 +183,43 @@ paths: description: unexpected error schema: $ref: '#/definitions/Error' + put: + summary: Info for a specific pet + operationId: showPetById + tags: + - pets + consumes: + - application/json + parameters: + - $ref: '#/parameters/ApiVersion' + - $ref: '#/parameters/ApiRequestId' + - name: petId + in: path + required: true + description: The id of the pet to retrieve + type: string + minLength: 3 + maxLength: 10 + - name: body + in: body + schema: + type: object + properties: + name: + type: string + age: + type: integer + tag: + type: string + responses: + "200": + description: Expected response to a valid request + schema: + $ref: '#/definitions/Pets' + default: + description: unexpected error + schema: + $ref: '#/definitions/Error' /heartbeat: get: summary: Info for current system status @@ -198,6 +233,24 @@ paths: description: unexpected error schema: $ref: '#/definitions/Error' + /text: + put: + consumes: + - text/html + - text/plain + parameters: + - name: body + in: body + schema: + type: string + required: true + responses: + "200": + description: Expected response to a valid request + default: + description: unexpected error + schema: + $ref: '#/definitions/Error' definitions: Pet: required: diff --git a/test/test-server-with-options.js b/test/test-server-with-options.js index 0e1c12f..0030f54 100644 --- a/test/test-server-with-options.js +++ b/test/test-server-with-options.js @@ -11,7 +11,8 @@ var inputValidationOptions = { { name: 'int32', pattern: /^\d{1,10}$/ } ], beautifyErrors: true, - firstError: true + firstError: true, + contentTypeValidation: true }; module.exports = inputValidation.init('test/pet-store-swagger.yaml', inputValidationOptions) @@ -27,9 +28,15 @@ module.exports = inputValidation.init('test/pet-store-swagger.yaml', inputValida app.get('/pets/:petId', inputValidation.validate, function (req, res, next) { res.json({ result: 'OK' }); }); + app.put('/pets/:petId', inputValidation.validate, function (req, res, next) { + res.json({ result: 'OK' }); + }); app.put('/pets', inputValidation.validate, function (req, res, next) { res.json({ result: 'OK' }); }); + app.put('/text', bodyParser.text(), inputValidation.validate, function (req, res, next) { + res.json({ result: 'OK' }); + }); app.use(function (err, req, res, next) { if (err instanceof inputValidation.InputValidationError) { res.status(400).json({ more_info: err.errors }); diff --git a/test/test-simple-server-with-base-path.js b/test/test-simple-server-with-base-path.js index e9104a5..6a77330 100644 --- a/test/test-simple-server-with-base-path.js +++ b/test/test-simple-server-with-base-path.js @@ -5,7 +5,7 @@ var bodyParser = require('body-parser'); var inputValidation = require('../src/middleware'); var router = require('./router'); -module.exports = inputValidation.init('test/pet-store-swagger-with-base-path.yaml') +module.exports = inputValidation.init('test/pet-store-swagger-with-base-path.yaml', { contentTypeValidation: true }) .then(function () { var app = express(); app.use(bodyParser.json()); @@ -19,7 +19,9 @@ module.exports = inputValidation.init('test/pet-store-swagger-with-base-path.yam app.get('/v1/capital', inputValidation.validate, function (req, res, next) { res.json({ result: 'OK' }); }); - + app.put('/v1/pets', inputValidation.validate, function (req, res, next) { + res.json({ result: 'OK' }); + }); app.use('/v1', router); app.use(function (err, req, res, next) {