From 6e39f39c6f4d47bfd742d2f0f3ae37aa711878bf Mon Sep 17 00:00:00 2001 From: James Date: Sun, 19 Apr 2020 14:18:34 -0400 Subject: [PATCH] adds validations to backend --- nodemon.json | 4 +- package.json | 3 +- src/collections/operations/create.js | 19 ++++-- src/errors/ValidationError.js | 10 +++ src/errors/index.js | 2 + src/express/responses/formatError.js | 9 ++- src/fields/sanitize.js | 2 +- src/fields/validate.js | 40 ++++++++++-- src/fields/validations.js | 98 ++++++++++++++++++++++------ 9 files changed, 148 insertions(+), 39 deletions(-) create mode 100644 src/errors/ValidationError.js diff --git a/nodemon.json b/nodemon.json index c3dbd9680..8044708cc 100644 --- a/nodemon.json +++ b/nodemon.json @@ -1,11 +1,13 @@ { "ignore": [ ".git", + "node_modules", "node_modules/**/node_modules", "src/client" ], "watch": [ - "src/" + "src/", + "demo/" ], "ext": "js,json" } diff --git a/package.json b/package.json index b2d830d8d..531ff3983 100644 --- a/package.json +++ b/package.json @@ -8,9 +8,8 @@ "test:int": "cross-env NODE_ENV=test jest --forceExit", "cov": "npm run core:build && node ./node_modules/jest/bin/jest.js src/tests --coverage", "dev": "nodemon demo/server.js", - "server": "node demo/server.js", "lint": "eslint **/*.js", - "debug:test:int": "node --inspect-brk node_modules/.bin/jest --runInBand" + "debug:test:int": "node --inspect-brk node_modules/.bin/jest" }, "bin": { "payload": "./src/bin/index.js" diff --git a/src/collections/operations/create.js b/src/collections/operations/create.js index 935b6bd3b..04891e2f9 100644 --- a/src/collections/operations/create.js +++ b/src/collections/operations/create.js @@ -1,5 +1,6 @@ const executePolicy = require('../../users/executePolicy'); const executeFieldHooks = require('../../fields/executeHooks'); +const validate = require('../../fields/validate'); const create = async (args) => { try { @@ -9,8 +10,6 @@ const create = async (args) => { await executePolicy(args, args.config.policies.create); - // Await validation here - let options = { Model: args.Model, config: args.config, @@ -22,13 +21,19 @@ const create = async (args) => { }; // ///////////////////////////////////// - // 2. Execute before create field-level hooks + // 2. Validate incoming data + // ///////////////////////////////////// + + await validate(args.config.fields, args.data); + + // ///////////////////////////////////// + // 3. Execute before create field-level hooks // ///////////////////////////////////// options.data = await executeFieldHooks(args.config.fields, args.data, 'beforeCreate'); // ///////////////////////////////////// - // 3. Execute before collection hook + // 4. Execute before collection hook // ///////////////////////////////////// const { beforeCreate } = args.config.hooks; @@ -38,7 +43,7 @@ const create = async (args) => { } // ///////////////////////////////////// - // 4. Perform database operation + // 5. Perform database operation // ///////////////////////////////////// const { @@ -60,7 +65,7 @@ const create = async (args) => { result = result.toJSON({ virtuals: true }); // ///////////////////////////////////// - // 5. Execute after collection hook + // 6. Execute after collection hook // ///////////////////////////////////// const { afterCreate } = args.config.hooks.afterCreate; @@ -70,7 +75,7 @@ const create = async (args) => { } // ///////////////////////////////////// - // 6. Return results + // 7. Return results // ///////////////////////////////////// return result; diff --git a/src/errors/ValidationError.js b/src/errors/ValidationError.js new file mode 100644 index 000000000..a5b7f1db5 --- /dev/null +++ b/src/errors/ValidationError.js @@ -0,0 +1,10 @@ +const httpStatus = require('http-status'); +const APIError = require('./APIError'); + +class ValidationError extends APIError { + constructor(results) { + super(results, httpStatus.BAD_REQUEST); + } +} + +module.exports = ValidationError; diff --git a/src/errors/index.js b/src/errors/index.js index ae5d53af2..9d803e9d1 100644 --- a/src/errors/index.js +++ b/src/errors/index.js @@ -5,6 +5,7 @@ const MissingCollectionLabel = require('./MissingCollectionLabel'); const MissingGlobalLabel = require('./MissingGlobalLabel'); const NotFound = require('./NotFound'); const Forbidden = require('./Forbidden'); +const ValidationError = require('./ValidationError'); module.exports = { APIError, @@ -14,4 +15,5 @@ module.exports = { MissingGlobalLabel, NotFound, Forbidden, + ValidationError, }; diff --git a/src/express/responses/formatError.js b/src/express/responses/formatError.js index d47fb3abf..b8a9b63dd 100644 --- a/src/express/responses/formatError.js +++ b/src/express/responses/formatError.js @@ -11,6 +11,12 @@ const formatErrorResponse = (incoming) => { }; } + if (Array.isArray(incoming.message)) { + return { + errors: incoming.message, + }; + } + if (incoming.name) { return { errors: [ @@ -21,9 +27,6 @@ const formatErrorResponse = (incoming) => { }; } } - // If the Mongoose error does not get returned with incoming && incoming.errors, - // it's of a type that we really don't know how to handle. Sometimes this means a TypeError, - // which we might be able to manipulate to get the error message itself and send that back. return { errors: [ diff --git a/src/fields/sanitize.js b/src/fields/sanitize.js index c9cb13764..05fdd3f1d 100644 --- a/src/fields/sanitize.js +++ b/src/fields/sanitize.js @@ -8,7 +8,7 @@ const sanitizeFields = (fields) => { if (!field.type) throw new MissingFieldType(field); if (typeof field.validation === 'undefined') { - field.validation = validations[field.type]; + field.validate = validations[field.type]; } if (!field.hooks) field.hooks = {}; diff --git a/src/fields/validate.js b/src/fields/validate.js index b328a87d9..97c856e53 100644 --- a/src/fields/validate.js +++ b/src/fields/validate.js @@ -1,9 +1,39 @@ -const executeFieldHooks = async (fields, data, hook) => { - for (const field of fields) { - if (typeof field.hooks[hook] === 'function') { - const result = await field.hooks[hook](data); +/* eslint-disable no-await-in-loop */ +/* eslint-disable no-restricted-syntax */ + +const { ValidationError } = require('../errors'); + +const iterateFields = async (fields, data, errors, path = '') => { + if (Array.isArray(data)) { + await Promise.all(data.map(async (row, i) => { + await iterateFields(fields, row, errors, `${path}.${i}.`); + })); + } else { + for (const field of fields) { + if (field.required) { + const validationResult = await field.validate(data[field.name], field); + + if (validationResult !== true) { + errors.push({ + field: field.name, + message: validationResult, + }); + } + } + + if (field.fields) { + await iterateFields(field.fields, data[field.name], errors, `${path}${field.name}.`); + } } } }; -module.exports = executeFieldHooks; +const validate = async (fields, data) => { + const errors = []; + await iterateFields(fields, data, errors); + if (errors.length > 0) { + throw new ValidationError(errors); + } +}; + +module.exports = validate; diff --git a/src/fields/validations.js b/src/fields/validations.js index 105243aab..324b5d4b4 100644 --- a/src/fields/validations.js +++ b/src/fields/validations.js @@ -1,35 +1,93 @@ const fieldToValidatorMap = { - number: (field, value) => { + number: (value, field) => { const parsedValue = parseInt(value, 10); - if (typeof parsedValue !== 'number') return false; - if (field.max && parsedValue > field.max) return false; - if (field.min && parsedValue < field.min) return false; + if (typeof parsedValue !== 'number' || Number.isNaN(parsedValue)) { + return `${field.label} value is not a valid number.`; + } + + if (field.max && parsedValue > field.max) { + return `${field.label} value is greater than the max allowed value of ${field.max}.`; + } + + if (field.min && parsedValue < field.min) { + return `${field.label} value is less than the min allowed value of ${field.min}.`; + } return true; }, - text: (field, value) => { - if (field.maxLength && value.length > field.maxLength) return false; - if (field.minLength && value.length < field.minLength) return false; + text: (value, field) => { + if (field.maxLength && value.length > field.maxLength) { + return `${field.label} length is greater than the max allowed length of ${field.maxLength}.`; + } + + if (field.minLength && value.length < field.minLength) { + return `${field.label} length is less than the minimum allowed length of ${field.minLength}.`; + } return true; }, - email: value => /\S+@\S+\.\S+/.test(value), - textarea: (field, value) => { - if (field.maxLength && value.length > field.maxLength) return false; - if (field.minLength && value.length < field.minLength) return false; + email: (value, field) => { + if (/\S+@\S+\.\S+/.test(value)) { + return true; + } + return `${field.label} is not a valid email address.`; + }, + textarea: (value, field) => { + if (field.maxLength && value.length > field.maxLength) { + return `${field.label} length is greater than the max allowed length of ${field.maxLength}.`; + } + + if (field.minLength && value.length < field.minLength) { + return `${field.label} length is less than the minimum allowed length of ${field.minLength}.`; + } return true; }, - wysiwyg: value => (!!value), - code: value => (!!value), - checkbox: value => Boolean(value), - date: value => value instanceof Date, - upload: value => (!!value), - relationship: value => (!!value), - repeater: value => (!!value), - select: value => (!!value), - flexible: value => (!!value), + wysiwyg: (value, field) => { + if (value) return true; + + return `${field.label} is required.`; + }, + code: (value, field) => { + if (value) return true; + + return `${field.label} is required.`; + }, + checkbox: (value, field) => { + if (value) { + return true; + } + + return `${field.label} can only be equal to true or false.`; + }, + date: (value, field) => { + if (value instanceof Date) { + return true; + } + + return `${field.label} is not a valid date.`; + }, + upload: (value, field) => { + if (value) return true; + return `${field.label} is required.`; + }, + relationship: (value, field) => { + if (value) return true; + return `${field.label} is required.`; + }, + repeater: (value, field) => { + if (value) return true; + return `${field.label} is required.`; + }, + select: (value, field) => { + if (value) return true; + return `${field.label} is required.`; + }, + flexible: (value, field) => { + if (value) return true; + return `${field.label} is required.`; + }, }; module.exports = fieldToValidatorMap;