Merge pull request #31 from Zooz/fix/30-text-field-validation

refs: #30
Clarify readme

* Test for a combination of files and fields

* Add optional param for test

* Extract logic into an internal function. Add comments

Author: idanto

.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/*
+mochawesome-report/*

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 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: [

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 || {};
@@ -28,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, '');
@@ -38,22 +39,23 @@ 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);
+                        const validatedBodySchema = _getValidatedBodySchema(bodySchema);
+                        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);
@@ -66,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
@@ -90,7 +117,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 +236,7 @@ function buildParametersValidation(parameters, contentTypes) {
                 additionalProperties: false
             },
             files: {
-                title: 'HTTP form',
+                title: 'HTTP form files',
                 files: {
                     required: [],
                     optional: []
@@ -222,7 +249,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 +260,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 +279,3 @@ module.exports = {
     validate: validate,
     InputValidationError: InputValidationError
 };
-
-var typeNameConversion = {
-    header: 'headers',
-    formData: 'files'
-};

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
+};

test/form-data-swagger.yaml

@@ -29,6 +29,52 @@ 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.
+        - name: params
+          in: formData
+          type: string
+          description: Optional params.
+      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:
@@ -47,4 +93,4 @@ parameters:
     description: 'global request id through the system.'
     type: string
     minLength: 1
-    x-example: '123456'
+    x-example: '123456'

test/middleware-test.js

@@ -2275,5 +2275,46 @@ describe('input-validation middleware tests', function () {
                     done();
                 });
         });
+        it('supports string formData', function (done) {
+            request(app)
+                .post('/login')
+                .set('api-version', '1.0')
+                .field('username', 'user')
+                .field('password', 'pass')
+                .expect(200, function (err, res) {
+                    if (err) {
+                        throw err;
+                    }
+                    expect(res.body.result).to.equal('OK');
+                    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')
+                .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();
+                });
+        });
     });
 });

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,6 +25,12 @@ 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' });
+        });
         app.use(function (err, req, res, next) {
             if (err instanceof inputValidation.InputValidationError) {
                 res.status(400).json({ more_info: err.errors });
@@ -33,4 +40,4 @@ module.exports = inputValidation.init('test/form-data-swagger.yaml', inputValida
         module.exports = app;
 
         return Promise.resolve(app);
-    });
+    });