From 3cf37fcf0d62cbc4d5e9a857c2f31aa9664d5ffb Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Sat, 7 Apr 2018 23:04:34 +0300 Subject: [PATCH 1/6] Add failing test --- .gitignore | 3 ++- test/form-data-swagger.yaml | 23 ++++++++++++++++++++++- test/middleware-test.js | 16 ++++++++++++++++ test/test-server-formdata.js | 5 ++++- 4 files changed, 44 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 355cc83..431298e 100644 --- a/.gitignore +++ b/.gitignore @@ -102,6 +102,7 @@ out/ # mpeltonen/sbt-idea plugin .idea_modules/ +.idea # JIRA plugin atlassian-ide-plugin.xml @@ -116,4 +117,4 @@ crashlytics-build.properties fabric.properties # Mocha Awsome -mochawesome-report/* \ No newline at end of file +mochawesome-report/* diff --git a/test/form-data-swagger.yaml b/test/form-data-swagger.yaml index c455f65..713f553 100644 --- a/test/form-data-swagger.yaml +++ b/test/form-data-swagger.yaml @@ -29,6 +29,27 @@ paths: responses: '200': description: Import result + /login: + post: + description: Login operation + produces: + - application/json + consumes: + - multipart/form-data + parameters: + - name: username + in: formData + required: true + type: string + description: Authentication username. + - name: password + in: formData + required: true + type: string + description: Authentication password. + responses: + '200': + description: Auth result definitions: parameters: @@ -47,4 +68,4 @@ parameters: description: 'global request id through the system.' type: string minLength: 1 - x-example: '123456' \ No newline at end of file + x-example: '123456' diff --git a/test/middleware-test.js b/test/middleware-test.js index 7924233..84cb65f 100644 --- a/test/middleware-test.js +++ b/test/middleware-test.js @@ -2275,5 +2275,21 @@ describe('input-validation middleware tests', function () { done(); }); }); + it('supports string formData', function (done) { + request(app) + .post('/login') + .set('api-version', '1.0') + .send({ + username: 'user', + password: 'pass' + }) + .expect(200, function (err, res) { + if (err) { + throw err; + } + expect(res.body.result).to.equal('OK'); + done(); + }); + }); }); }); diff --git a/test/test-server-formdata.js b/test/test-server-formdata.js index 6066342..4a4edaf 100644 --- a/test/test-server-formdata.js +++ b/test/test-server-formdata.js @@ -24,6 +24,9 @@ module.exports = inputValidation.init('test/form-data-swagger.yaml', inputValida app.post('/pets/import', upload.any(), inputValidation.validate, function (req, res, next) { res.json({ result: 'OK' }); }); + app.post('/login', upload.any(), 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 }); @@ -33,4 +36,4 @@ module.exports = inputValidation.init('test/form-data-swagger.yaml', inputValida module.exports = app; return Promise.resolve(app); - }); \ No newline at end of file + }); From f16645f7793d2d1a4523bcb9cf8aeffdb57b71be Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Sun, 8 Apr 2018 00:12:27 +0300 Subject: [PATCH 2/6] Fix text form field support refs: #30 --- README.md | 1 + src/middleware.js | 49 +++++++++++++++++++++++------------- src/utils/sourceResolver.js | 22 ++++++++++++++++ test/middleware-test.js | 19 +++++++++++--- test/test-server-formdata.js | 9 ++++--- 5 files changed, 75 insertions(+), 25 deletions(-) create mode 100644 src/utils/sourceResolver.js diff --git a/README.md b/README.md index d564dec..61229f7 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,7 @@ Options currently supports: - `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. +- `expectFormFieldsInBody` - Boolean that indicates whether fields of non-file type that are specified in the schema should be validated against request body (e. g. Multer is copying text fields to body) ```js formats: [ diff --git a/src/middleware.js b/src/middleware.js index 059395f..d205511 100644 --- a/src/middleware.js +++ b/src/middleware.js @@ -6,7 +6,8 @@ var SwaggerParser = require('swagger-parser'), filesKeyword = require('./customKeywords/files'), contentKeyword = require('./customKeywords/contentTypeValidation'), InputValidationError = require('./inputValidationError'), - schemaPreprocessor = require('./utils/schema-preprocessor'); + schemaPreprocessor = require('./utils/schema-preprocessor'), + sourceResolver = require('./utils/sourceResolver'); var schemas = {}; var middlewareOptions; @@ -16,7 +17,7 @@ var ajvConfigParams; /** * Initialize the input validation middleware * @param {string} swaggerPath - the path for the swagger file - * @param {Object} options - options.formats to add formats to ajv, options.beautifyErrors, options.firstError, options.fileNameField (default is 'fieldname' - multer package), options.ajvConfigBody and options.ajvConfigParams for config object that will be passed for creation of Ajv instance used for validation of body and parameters appropriately + * @param {Object} options - options.formats to add formats to ajv, options.beautifyErrors, options.firstError, options.expectFormFieldsInBody, options.fileNameField (default is 'fieldname' - multer package), options.ajvConfigBody and options.ajvConfigParams for config object that will be passed for creation of Ajv instance used for validation of body and parameters appropriately */ function init(swaggerPath, options) { middlewareOptions = options || {}; @@ -38,22 +39,41 @@ function init(swaggerPath, options) { schemas[parsedPath][currentMethod.toLowerCase()] = {}; const parameters = dereferenced.paths[currentPath][currentMethod].parameters || []; - let bodySchema = parameters.filter(function (parameter) { return parameter.in === 'body' }); + + let bodySchema = middlewareOptions.expectFormFieldsInBody + ? parameters.filter(function (parameter) { return (parameter.in === 'body' || (parameter.in === 'formData' && parameter.type !== 'file')) }) + : parameters.filter(function (parameter) { return parameter.in === 'body' }); + if (makeOptionalAttributesNullable) { schemaPreprocessor.makeOptionalAttributesNullable(bodySchema); } if (bodySchema.length > 0) { - schemas[parsedPath][currentMethod].body = buildBodyValidation(bodySchema[0].schema, dereferenced.definitions, swaggers[1], currentPath, currentMethod, parsedPath); + let validatedBodySchema; + if (bodySchema[0].in === 'body') { + validatedBodySchema = bodySchema[0].schema; + } else if (bodySchema[0].in === 'formData') { + validatedBodySchema = { + required: [], + properties: {} + }; + bodySchema.forEach((formField) => { + if (formField.type !== 'file') { + validatedBodySchema.properties[formField.name] = { + type: formField.type + }; + if (formField.required) { + validatedBodySchema.required.push(formField.name); + } + } + }); + } + schemas[parsedPath][currentMethod].body = buildBodyValidation(validatedBodySchema, dereferenced.definitions, swaggers[1], currentPath, currentMethod, parsedPath); } let localParameters = parameters.filter(function (parameter) { return parameter.in !== 'body'; }).concat(pathParameters); - 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); @@ -90,7 +110,7 @@ function validate(req, res, next) { firstError: middlewareOptions.firstError }); return next(error); }); -}; +} function _validateBody(body, path, method) { return new Promise(function (resolve, reject) { @@ -209,7 +229,7 @@ function buildParametersValidation(parameters, contentTypes) { additionalProperties: false }, files: { - title: 'HTTP form', + title: 'HTTP form files', files: { required: [], optional: [] @@ -222,7 +242,7 @@ function buildParametersValidation(parameters, contentTypes) { var data = Object.assign({}, parameter); const required = parameter.required; - const source = typeNameConversion[parameter.in] || parameter.in; + const source = sourceResolver.resolveParameterSource(parameter); const key = parameter.in === 'header' ? parameter.name.toLowerCase() : parameter.name; var destination = ajvParametersSchema.properties[source]; @@ -233,7 +253,7 @@ function buildParametersValidation(parameters, contentTypes) { if (data.type === 'file') { required ? destination.files.required.push(key) : destination.files.optional.push(key); - } else { + } else if (source !== 'fields') { if (required) { destination.required = destination.required || []; destination.required.push(key); @@ -252,8 +272,3 @@ module.exports = { validate: validate, InputValidationError: InputValidationError }; - -var typeNameConversion = { - header: 'headers', - formData: 'files' -}; diff --git a/src/utils/sourceResolver.js b/src/utils/sourceResolver.js new file mode 100644 index 0000000..225c1a6 --- /dev/null +++ b/src/utils/sourceResolver.js @@ -0,0 +1,22 @@ +/** + * Resolve value source for a given schema parameter + * @param {Object} parameter from Swagger schema + * @returns {string} + */ +function resolveParameterSource(parameter) { + if (parameter.in === 'formData') { + if (parameter.type === 'file') { + return 'files'; + } else { + return 'fields'; + } + } else if (parameter.in === 'header') { + return 'headers'; + } + + return parameter.in; +} + +module.exports = { + resolveParameterSource +}; diff --git a/test/middleware-test.js b/test/middleware-test.js index 84cb65f..738cec9 100644 --- a/test/middleware-test.js +++ b/test/middleware-test.js @@ -2279,10 +2279,8 @@ describe('input-validation middleware tests', function () { request(app) .post('/login') .set('api-version', '1.0') - .send({ - username: 'user', - password: 'pass' - }) + .field('username', 'user') + .field('password', 'pass') .expect(200, function (err, res) { if (err) { throw err; @@ -2291,5 +2289,18 @@ describe('input-validation middleware tests', function () { done(); }); }); + it('validates string formData', function (done) { + request(app) + .post('/login') + .set('api-version', '1.0') + .field('username', 'user') + .expect(400, function (err, res) { + if (err) { + throw err; + } + expect(res.body.more_info).to.includes('body should have required property \'password\''); + done(); + }); + }); }); }); diff --git a/test/test-server-formdata.js b/test/test-server-formdata.js index 4a4edaf..4e3ba3d 100644 --- a/test/test-server-formdata.js +++ b/test/test-server-formdata.js @@ -14,7 +14,8 @@ var inputValidationOptions = { { name: 'file', validate: () => { return true } } ], beautifyErrors: true, - firstError: true + firstError: true, + expectFormFieldsInBody: true }; module.exports = inputValidation.init('test/form-data-swagger.yaml', inputValidationOptions) @@ -24,9 +25,9 @@ module.exports = inputValidation.init('test/form-data-swagger.yaml', inputValida app.post('/pets/import', upload.any(), inputValidation.validate, function (req, res, next) { res.json({ result: 'OK' }); }); - app.post('/login', upload.any(), inputValidation.validate, function (req, res, next) { - res.json({ result: 'OK' }); - }); + app.post('/login', upload.any(), 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 }); From b46f8a4e677b81880f677a0e113d5b523b3783dd Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Sun, 8 Apr 2018 00:13:40 +0300 Subject: [PATCH 3/6] Clarify readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 61229f7..d62de8d 100644 --- a/README.md +++ b/README.md @@ -78,7 +78,7 @@ Options currently supports: - `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. -- `expectFormFieldsInBody` - Boolean that indicates whether fields of non-file type that are specified in the schema should be validated against request body (e. g. Multer is copying text fields to body) +- `expectFormFieldsInBody` - Boolean that indicates whether form fields of non-file type that are specified in the schema should be validated against request body (e. g. Multer is copying text form fields to body) ```js formats: [ From 46c1372d63c756702778e05e2ebaa0532b1dc604 Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Sun, 8 Apr 2018 00:22:23 +0300 Subject: [PATCH 4/6] Test for a combination of files and fields --- test/form-data-swagger.yaml | 21 +++++++++++++++++++++ test/middleware-test.js | 14 ++++++++++++++ test/test-server-formdata.js | 3 +++ 3 files changed, 38 insertions(+) diff --git a/test/form-data-swagger.yaml b/test/form-data-swagger.yaml index 713f553..117899d 100644 --- a/test/form-data-swagger.yaml +++ b/test/form-data-swagger.yaml @@ -50,6 +50,27 @@ paths: responses: '200': description: Auth result + /kennels/import: + post: + description: Login operation + produces: + - application/json + consumes: + - multipart/form-data + parameters: + - name: name + in: formData + required: true + type: string + description: Kennel name. + - name: blueprintFile + in: formData + required: true + type: file + description: File to import from. + responses: + '200': + description: Import result definitions: parameters: diff --git a/test/middleware-test.js b/test/middleware-test.js index 738cec9..49eb13c 100644 --- a/test/middleware-test.js +++ b/test/middleware-test.js @@ -2289,6 +2289,20 @@ describe('input-validation middleware tests', function () { done(); }); }); + it('supports mix of files and fields', function (done) { + request(app) + .post('/kennels/import') + .set('api-version', '1.0') + .field('name', 'kennel 1 ') + .attach('blueprintFile', 'LICENSE') + .expect(200, function (err, res) { + if (err) { + throw err; + } + expect(res.body.result).to.equal('OK'); + done(); + }); + }); it('validates string formData', function (done) { request(app) .post('/login') diff --git a/test/test-server-formdata.js b/test/test-server-formdata.js index 4e3ba3d..3b2357b 100644 --- a/test/test-server-formdata.js +++ b/test/test-server-formdata.js @@ -25,6 +25,9 @@ module.exports = inputValidation.init('test/form-data-swagger.yaml', inputValida app.post('/pets/import', upload.any(), inputValidation.validate, function (req, res, next) { res.json({ result: 'OK' }); }); + app.post('/kennels/import', upload.any(), inputValidation.validate, function (req, res, next) { + res.json({ result: 'OK' }); + }); app.post('/login', upload.any(), inputValidation.validate, function (req, res, next) { res.json({ result: 'OK' }); }); From 3f056bdd19fb8552b2b094367d8d80d53e3b1900 Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Sun, 8 Apr 2018 00:26:49 +0300 Subject: [PATCH 5/6] Add optional param for test --- test/form-data-swagger.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/form-data-swagger.yaml b/test/form-data-swagger.yaml index 117899d..8ded3ba 100644 --- a/test/form-data-swagger.yaml +++ b/test/form-data-swagger.yaml @@ -47,6 +47,10 @@ paths: required: true type: string description: Authentication password. + - name: params + in: formData + type: string + description: Optional params. responses: '200': description: Auth result From d7365c06bd41f771c9d2245e04c20dfe436a5bcc Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Tue, 24 Apr 2018 00:55:35 +0300 Subject: [PATCH 6/6] Extract logic into an internal function. Add comments --- src/middleware.js | 47 +++++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/src/middleware.js b/src/middleware.js index d205511..7539975 100644 --- a/src/middleware.js +++ b/src/middleware.js @@ -29,7 +29,7 @@ function init(swaggerPath, options) { SwaggerParser.dereference(swaggerPath), SwaggerParser.parse(swaggerPath) ]).then(function (swaggers) { - var dereferenced = swaggers[0]; + const dereferenced = swaggers[0]; Object.keys(dereferenced.paths).forEach(function (currentPath) { let pathParameters = dereferenced.paths[currentPath].parameters || []; let parsedPath = dereferenced.basePath && dereferenced.basePath !== '/' ? dereferenced.basePath.concat(currentPath.replace(/{/g, ':').replace(/}/g, '')) : currentPath.replace(/{/g, ':').replace(/}/g, ''); @@ -48,25 +48,7 @@ function init(swaggerPath, options) { schemaPreprocessor.makeOptionalAttributesNullable(bodySchema); } if (bodySchema.length > 0) { - let validatedBodySchema; - if (bodySchema[0].in === 'body') { - validatedBodySchema = bodySchema[0].schema; - } else if (bodySchema[0].in === 'formData') { - validatedBodySchema = { - required: [], - properties: {} - }; - bodySchema.forEach((formField) => { - if (formField.type !== 'file') { - validatedBodySchema.properties[formField.name] = { - type: formField.type - }; - if (formField.required) { - validatedBodySchema.required.push(formField.name); - } - } - }); - } + const validatedBodySchema = _getValidatedBodySchema(bodySchema); schemas[parsedPath][currentMethod].body = buildBodyValidation(validatedBodySchema, dereferenced.definitions, swaggers[1], currentPath, currentMethod, parsedPath); } @@ -86,6 +68,31 @@ function init(swaggerPath, options) { }); } +function _getValidatedBodySchema(bodySchema) { + let validatedBodySchema; + if (bodySchema[0].in === 'body') { + // if we are processing schema for a simple JSON payload, no additional processing needed + validatedBodySchema = bodySchema[0].schema; + } else if (bodySchema[0].in === 'formData') { + // if we are processing multipart form, assemble body schema from form field schemas + validatedBodySchema = { + required: [], + properties: {} + }; + bodySchema.forEach((formField) => { + if (formField.type !== 'file') { + validatedBodySchema.properties[formField.name] = { + type: formField.type + }; + if (formField.required) { + validatedBodySchema.required.push(formField.name); + } + } + }); + } + return validatedBodySchema; +} + /** * The middleware - should be called for each express route * @param {any} req