From d6cf740417f507648e9d98f3615dea54264c08fe Mon Sep 17 00:00:00 2001 From: Artem Kukharenko Date: Sat, 9 Jun 2018 22:32:11 +0300 Subject: [PATCH] [*] Move validator to middleware --- .gitignore | 1 + package.json | 2 +- src/helpers/joi.adapter.js | 47 ++++++++++ src/helpers/joi.adapter.spec.js | 28 ++++++ src/helpers/validator.js | 40 +++++++++ src/helpers/validator.spec.js | 72 ++++++++++++++++ src/middlewares/validate.js | 36 ++++++++ src/middlewares/validate.spec.js | 60 +++++++++++++ src/resources/account/account.controller.js | 29 ++----- src/resources/account/public.js | 12 +-- .../validators/forgotPassword.validator.js | 32 ++++--- .../validators/resetPassword.validator.js | 32 ++++--- .../account/validators/signin.validator.js | 69 +++++++++------ .../account/validators/signup.validator.js | 32 ++++--- .../validators/verifyEmail.validator.js | 34 +++++--- src/resources/base.validator.js | 85 ------------------- src/resources/user/index.js | 4 +- src/resources/user/user.controller.js | 17 ++-- .../user/validators/update.validator.js | 36 +++++--- 19 files changed, 455 insertions(+), 213 deletions(-) create mode 100644 src/helpers/joi.adapter.js create mode 100644 src/helpers/joi.adapter.spec.js create mode 100644 src/helpers/validator.js create mode 100644 src/helpers/validator.spec.js create mode 100644 src/middlewares/validate.js create mode 100644 src/middlewares/validate.spec.js delete mode 100644 src/resources/base.validator.js diff --git a/.gitignore b/.gitignore index 3c3629e..7a1537b 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ +.idea node_modules diff --git a/package.json b/package.json index afbe55c..8b66912 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "start": "node src/app.js", "test": "run-s test:**", "test:eslint": "eslint ./src", - "test:mocha": "NODE_ENV=test mocha --timeout 20000 --recursive --exit -c -R spec src/**/*.spec.js", + "test:mocha": "NODE_ENV=test mocha --timeout 20000 --recursive --exit -c -R spec 'src/**/*.spec.js'", "development": "NODE_ENV=development nodemon --watch src src/app.js", "format": "prettier-eslint --write \"src/**/*.js\"", "add-contributor": "all-contributors add", diff --git a/src/helpers/joi.adapter.js b/src/helpers/joi.adapter.js new file mode 100644 index 0000000..d28e978 --- /dev/null +++ b/src/helpers/joi.adapter.js @@ -0,0 +1,47 @@ +const Joi = require('joi'); +const _ = require('lodash'); + +const joiOptions = { + abortEarly: false, + allowUnknown: true, + stripUnknown: { + objects: true, + }, +}; + +/** + * Parse and return list of errors + * @param {object} joiError + * @return {object[]} + */ +const parseJoiErrors = (joiError) => { + let resultErrors = []; + if (joiError && _.isArray(joiError.details)) { + resultErrors = joiError.details.map((error) => { + const pathLastPart = error.path.slice(error.path.length - error.context.key.length); + + if (pathLastPart === error.context.key) { + return { [error.path]: error.message }; + } + + return { [error.context.key]: error.message }; + }); + } + + return resultErrors; +}; + +const validate = _.curry((schema, payload) => { + const { error, value } = Joi.validate(payload, schema, joiOptions); + + return { + errors: parseJoiErrors(error), + value, + }; +}); + +module.exports = { + ...Joi, + __validate: Joi.validate, + validate, +}; diff --git a/src/helpers/joi.adapter.spec.js b/src/helpers/joi.adapter.spec.js new file mode 100644 index 0000000..6e533c4 --- /dev/null +++ b/src/helpers/joi.adapter.spec.js @@ -0,0 +1,28 @@ +const Joi = require('./joi.adapter'); +const chai = require('chai'); + +chai.should(); + +describe('joi validator', () => { + it('should apply args partially', () => { + const schema = { + email: Joi.string(), + }; + const validationResult = Joi.validate(schema, { + email: 'test@test.com', + }); + + const validationResultPartial = Joi.validate(schema)({ + email: 'test@test.com', + }); + + validationResult.should.be.deep.equal(validationResultPartial); + + validationResult.should.be.deep.equal({ + errors: [], + value: { + email: 'test@test.com', + }, + }); + }); +}); diff --git a/src/helpers/validator.js b/src/helpers/validator.js new file mode 100644 index 0000000..766b7ef --- /dev/null +++ b/src/helpers/validator.js @@ -0,0 +1,40 @@ +const _ = require('lodash'); + +const Symbols = { + PERSISTENT: Symbol('persistent'), +}; + +module.exports.Symbols = Symbols; + +const getValidators = (validators = []) => { + if (_.isFunction(validators)) { + return [validators]; + } + + if (!Array.isArray(validators) || !validators.every(_.isFunction)) { + throw Error('Validators must be a function or array of functions'); + } + + return validators; +}; + +module.exports.validate = (payload, validators = []) => { + const persistentData = payload[Symbols.PERSISTENT]; + return getValidators(validators).reduce(async (result, validator) => { + const data = await result; + + if (data.errors.length) { + return data; + } + + const validationResult = await validator(data.value, persistentData); + + return { + errors: validationResult.errors || [], + value: validationResult.value, + }; + }, { + errors: [], + value: payload, + }); +}; diff --git a/src/helpers/validator.spec.js b/src/helpers/validator.spec.js new file mode 100644 index 0000000..c0158a4 --- /dev/null +++ b/src/helpers/validator.spec.js @@ -0,0 +1,72 @@ +const { validate, Symbols } = require('./validator'); +const chai = require('chai'); + +chai.should(); + +describe('validator', () => { + it("should return '{ errors: [], payload }' with empty validators array", async () => { + const validationResult = await validate({}, []); + validationResult.should.be.deep.equal({ + errors: [], + value: {}, + }); + }); + + it('should return the value from the last validator', async () => { + const validators = [ + () => ({ value: 1 }), + () => ({ value: 2 }), + ]; + + const validationResult = await validate({}, validators); + validationResult.should.be.deep.equal({ + errors: [], + value: 2, + }); + }); + + it('should skip validators after errors appeared', async () => { + const validators = [ + () => ({ value: 1 }), + () => ({ value: 2, errors: ['Errors was appear'] }), + () => ({ value: 3 }), + ]; + + const validationResult = await validate({}, validators); + validationResult.should.be.deep.equal({ + errors: ['Errors was appear'], + value: 2, + }); + }); + + it('should apply persistent data to each validator', async () => { + const persistentData = { + persistant: 'Wow! persistent!', + }; + + const payload = { + [Symbols.PERSISTENT]: persistentData, + }; + + const validators = [ + (data, persistent) => { + persistent.should.be.equal(persistentData); + return { + value: 1, + }; + }, + (data, persist) => { + persist.should.be.equal(persistentData); + return { + value: 2, + }; + }, + ]; + + const validationResult = await validate(payload, validators); + validationResult.should.be.deep.equal({ + errors: [], + value: 2, + }); + }); +}); diff --git a/src/middlewares/validate.js b/src/middlewares/validate.js new file mode 100644 index 0000000..8bf16cb --- /dev/null +++ b/src/middlewares/validate.js @@ -0,0 +1,36 @@ +const { validate, Symbols } = require('helpers/validator'); + +const defaultOptions = { + throwOnInvalid: true, +}; + +const validateMiddleware = (validators = [], options = defaultOptions) => async (ctx, next) => { + const { + throwOnInvalid, + } = { + ...defaultOptions, + ...options, + }; + + const payload = { + ...ctx.request.body, + ...ctx.query, + ...ctx.params, + [Symbols.PERSISTENT]: ctx, + }; + + const result = await validate(payload, validators); + + if (throwOnInvalid && result.errors.length) { + ctx.body = { + errors: result.errors, + }; + + ctx.throw(400); + } + + ctx.validatedRequest = result; + await next(); +}; + +module.exports = validateMiddleware; diff --git a/src/middlewares/validate.spec.js b/src/middlewares/validate.spec.js new file mode 100644 index 0000000..f385727 --- /dev/null +++ b/src/middlewares/validate.spec.js @@ -0,0 +1,60 @@ +const path = require('path'); +const _ = require('lodash'); +require('app-module-path').addPath(path.resolve(__dirname, '../')); + +const validate = require('./validate'); +const Joi = require('helpers/joi.adapter'); +const chai = require('chai'); + +chai.should(); + +describe('validator', () => { + const ctx = { + request: { + body: { + test: 'test', + }, + }, + query: {}, + params: {}, + throw: (status) => { + throw new Error(status); + }, + }; + + const noop = () => { }; + + it('should add validatedRequest to ctx', async () => { + const schema = { + test: Joi.string(), + }; + const ctxMock = _.cloneDeep(ctx); + + await validate(Joi.validate(schema))(ctxMock, noop); + ctxMock.validatedRequest.should.deep.equal({ errors: [], value: { test: 'test' } }); + }); + + it('should throw error for wrong validation', async () => { + const schema = { + test: Joi.string().email(), + }; + const ctxMock = _.cloneDeep(ctx); + + try { + await validate(Joi.validate(schema))(ctxMock, noop); + } catch (err) { + err.message.should.be.equal('400'); + } + }); + + + it('should throw error if validators is not a an function', async () => { + const ctxMock = _.cloneDeep(ctx); + + try { + await validate('wrong validate func')(ctxMock, noop); + } catch (err) { + err.message.should.be.equal('Validators must be a function or array of functions'); + } + }); +}); diff --git a/src/resources/account/account.controller.js b/src/resources/account/account.controller.js index 4bddc92..3383d77 100644 --- a/src/resources/account/account.controller.js +++ b/src/resources/account/account.controller.js @@ -1,5 +1,3 @@ -const validators = require('./validators'); - const userService = require('resources/user/user.service'); const authService = require('auth.service'); const emailService = require('email.service'); @@ -34,11 +32,8 @@ const createUserAccount = async (userData) => { * Create user, company, default app, send signup confirmation email and * create auth token for user to login */ -exports.signup = async (ctx, next) => { - const result = await validators.signup.validate(ctx); - ctx.assert(!result.errors, 400); - - const { value: userData } = result; +exports.signup = async (ctx) => { + const userData = ctx.validatedRequest.value; const user = await createUserAccount(userData); const response = {}; @@ -53,10 +48,7 @@ exports.signup = async (ctx, next) => { * sets `emailVerified` to true if token is valid */ exports.verifyEmail = async (ctx, next) => { - const result = await validators.verifyEmail.validate(ctx); - ctx.assert(!result.errors, 400); - - const { value: data } = result; + const data = ctx.validatedRequest.value; const user = await userService.markEmailAsVerified(data.userId); const token = authService.createAuthToken({ @@ -72,10 +64,7 @@ exports.verifyEmail = async (ctx, next) => { * Loads user by email and compare password hashes */ exports.signin = async (ctx, next) => { - const result = await validators.signin.validate(ctx); - ctx.assert(!result.errors, 400); - - const { value: signinData } = result; + const signinData = ctx.validatedRequest.value; const token = authService.createAuthToken({ userId: signinData.userId }); @@ -90,10 +79,7 @@ exports.signin = async (ctx, next) => { * `forgotPasswordToken` field. If user not found, returns validator's error */ exports.forgotPassword = async (ctx, next) => { - const result = await validators.forgotPassword.validate(ctx); - ctx.assert(!result.errors, 400); - - const { value: data } = result; + const data = ctx.validatedRequest.value; const user = await userService.findOne({ email: data.email }); let { resetPasswordToken } = user; @@ -111,10 +97,7 @@ exports.forgotPassword = async (ctx, next) => { * Updates user password, used in combination with forgotPassword */ exports.resetPassword = async (ctx, next) => { - const result = await validators.resetPassword.validate(ctx); - ctx.assert(!result.errors, 400); - - const { userId, password } = result.value; + const { userId, password } = ctx.validatedRequest.value; await userService.updatePassword(userId, password); await userService.updateResetPasswordToken(userId, ''); diff --git a/src/resources/account/public.js b/src/resources/account/public.js index 031c6d4..e23788d 100644 --- a/src/resources/account/public.js +++ b/src/resources/account/public.js @@ -1,13 +1,15 @@ const Router = require('koa-router'); +const validate = require('middlewares/validate'); +const validators = require('./validators'); const router = new Router(); const controller = require('./account.controller'); -router.post('/signup', controller.signup); -router.get('/verifyEmail/:token', controller.verifyEmail); -router.post('/signin', controller.signin); -router.post('/forgotPassword', controller.forgotPassword); -router.put('/resetPassword', controller.resetPassword); +router.post('/signup', validate(validators.signup), controller.signup); +router.get('/verifyEmail/:token', validate(validators.verifyEmail), controller.verifyEmail); +router.post('/signin', validate(validators.signin), controller.signin); +router.post('/forgotPassword', validate(validators.forgotPassword), controller.forgotPassword); +router.put('/resetPassword', validate(validators.resetPassword), controller.resetPassword); router.post('/resend', controller.resendVerification); module.exports = router.routes(); diff --git a/src/resources/account/validators/forgotPassword.validator.js b/src/resources/account/validators/forgotPassword.validator.js index 43a7ee4..cb8ebc6 100644 --- a/src/resources/account/validators/forgotPassword.validator.js +++ b/src/resources/account/validators/forgotPassword.validator.js @@ -1,5 +1,4 @@ -const Joi = require('joi'); -const baseValidator = require('resources/base.validator'); +const Joi = require('helpers/joi.adapter'); const userService = require('resources/user/user.service'); const schema = { @@ -15,16 +14,23 @@ const schema = { }), }; -exports.validate = ctx => - baseValidator(ctx, schema, async (data) => { - const userExists = await userService.exists({ email: data.email }); +const validateFunc = async (data) => { + const userExists = await userService.exists({ email: data.email }); + const errors = []; + if (!userExists) { + errors.push({ + email: `Couldn't find account associated with ${data.email}. Please try again`, + }); + return { errors }; + } - if (!userExists) { - ctx.errors.push({ - email: `Couldn't find account associated with ${data.email}. Please try again`, - }); - return false; - } + return { + value: data, + errors, + }; +}; - return data; - }); +module.exports = [ + Joi.validate(schema), + validateFunc, +]; diff --git a/src/resources/account/validators/resetPassword.validator.js b/src/resources/account/validators/resetPassword.validator.js index 686dab0..b042f17 100644 --- a/src/resources/account/validators/resetPassword.validator.js +++ b/src/resources/account/validators/resetPassword.validator.js @@ -1,6 +1,5 @@ -const Joi = require('joi'); +const Joi = require('helpers/joi.adapter'); -const baseValidator = require('resources/base.validator'); const userService = require('resources/user/user.service'); const schema = { @@ -19,17 +18,26 @@ const schema = { }, }), }; +const validateFunc = async (data) => { + const user = await userService.findOne({ resetPasswordToken: data.token }); + const errors = []; -exports.validate = ctx => - baseValidator(ctx, schema, async (data) => { - const user = await userService.findOne({ resetPasswordToken: data.token }); - if (!user) { - ctx.errors.push({ token: 'Password reset link has expired or invalid' }); - return false; - } + if (!user) { + errors.push({ token: 'Password reset link has expired or invalid' }); + return { errors }; + } - return { + + return { + value: { userId: user._id, password: data.password, - }; - }); + }, + errors, + }; +}; + +module.exports = [ + Joi.validate(schema), + validateFunc, +]; diff --git a/src/resources/account/validators/signin.validator.js b/src/resources/account/validators/signin.validator.js index 6e39d76..a4614d4 100644 --- a/src/resources/account/validators/signin.validator.js +++ b/src/resources/account/validators/signin.validator.js @@ -1,6 +1,4 @@ -const Joi = require('joi'); - -const baseValidator = require('resources/base.validator'); +const Joi = require('helpers/joi.adapter'); const userService = require('resources/user/user.service'); const securityUtil = require('security.util'); @@ -33,31 +31,46 @@ const schema = { }), }; -exports.validate = ctx => - baseValidator(ctx, schema, async (signinData) => { - const user = await userService.findOne({ email: signinData.email }); - if (!user) { - ctx.errors.push({ credentials: incorrectCredentials }); - return false; - } - - const isPasswordMatch = await securityUtil.compareTextWithHash( - signinData.password, - user.passwordHash, - user.passwordSalt, - ); - - if (!isPasswordMatch) { - ctx.errors.push({ credentials: incorrectCredentials }); - return false; - } - - if (!user.isEmailVerified) { - ctx.errors.push({ email: 'Please verify your email to sign in' }); - return false; - } +const validateFunc = async (signinData) => { + const user = await userService.findOne({ email: signinData.email }); + const errors = []; + if (!user) { + errors.push({ credentials: incorrectCredentials }); + return { + errors, + }; + } + + const isPasswordMatch = await securityUtil.compareTextWithHash( + signinData.password, + user.passwordHash, + user.passwordSalt, + ); + if (!isPasswordMatch) { + errors.push({ credentials: incorrectCredentials }); return { - userId: user._id, + errors, }; - }); + } + + if (!user.isEmailVerified) { + errors.push({ email: 'Please verify your email to sign in' }); + return { + errors, + }; + } + + return { + value: { + userId: user._id, + }, + errors, + }; +}; + +module.exports = [ + Joi.validate(schema), + validateFunc, +]; + diff --git a/src/resources/account/validators/signup.validator.js b/src/resources/account/validators/signup.validator.js index d107178..db63f3c 100644 --- a/src/resources/account/validators/signup.validator.js +++ b/src/resources/account/validators/signup.validator.js @@ -1,7 +1,6 @@ -const Joi = require('joi'); +const Joi = require('helpers/joi.adapter'); const userService = require('resources/user/user.service'); -const baseValidator = require('resources/base.validator'); const schema = { firstName: Joi.string() @@ -43,13 +42,24 @@ const schema = { }), }; -exports.validate = ctx => - baseValidator(ctx, schema, async (data) => { - const userExists = await userService.exists({ email: data.email }); - if (userExists) { - ctx.errors.push({ email: 'User with this email is already registered.' }); - return false; - } +const validateFunc = async (data) => { + const userExists = await userService.exists({ email: data.email }); + const errors = []; + if (userExists) { + errors.push({ email: 'User with this email is already registered.' }); + return { + errors, + }; + } - return data; - }); + + return { + value: data, + errors, + }; +}; + +module.exports = [ + Joi.validate(schema), + validateFunc, +]; diff --git a/src/resources/account/validators/verifyEmail.validator.js b/src/resources/account/validators/verifyEmail.validator.js index 7f6cc54..8321e3d 100644 --- a/src/resources/account/validators/verifyEmail.validator.js +++ b/src/resources/account/validators/verifyEmail.validator.js @@ -1,7 +1,6 @@ -const Joi = require('joi'); +const Joi = require('helpers/joi.adapter'); const userService = require('resources/user/user.service'); -const baseValidator = require('resources/base.validator'); const schema = { token: Joi.string().options({ @@ -11,16 +10,25 @@ const schema = { }), }; -exports.validate = ctx => - baseValidator(ctx, schema, async (data) => { - const user = await userService.findOne({ signupToken: ctx.params.token }); - - if (!user) { - ctx.errors.push({ token: 'Token is invalid' }); - return false; - } - +const validateFunc = async (data) => { + const user = await userService.findOne({ signupToken: data.token }); + const errors = []; + if (!user) { + errors.push({ token: 'Token is invalid' }); return { - userId: user._id, + errors, }; - }); + } + + return { + value: { + userId: user._id, + }, + errors, + }; +}; + +module.exports = [ + Joi.validate(schema), + validateFunc, +]; diff --git a/src/resources/base.validator.js b/src/resources/base.validator.js deleted file mode 100644 index ef19d7f..0000000 --- a/src/resources/base.validator.js +++ /dev/null @@ -1,85 +0,0 @@ -const Joi = require('joi'); - -const joiOptions = { - abortEarly: false, - allowUnknown: true, - stripUnknown: { - objects: true, - }, -}; - -/** - * Parse and return list of errors - * @param {object} joiError - * @return {object[]} - */ -const parseJoiErrors = (joiError) => { - let resultErrors = []; - if (joiError && joiError.details instanceof Array) { - resultErrors = joiError.details.map((error) => { - const pathLastPart = error.path.slice(error.path.length - error.context.key.length); - - if (pathLastPart === error.context.key) { - return { [error.path]: error.message }; - } - - return { [error.context.key]: error.message }; - }); - } - - return resultErrors; -}; - -/** - * Validate request and send 400(bad request), when request is not valid - * @param {object} ctx - * @param {object|Function} schema - Joi validation schema or validation function - * @param {Function} validateFn - * @return {object} - */ -module.exports = async (ctx, schema, validateFn) => { - let validateFunc = validateFn; - let joiSchema = schema; - - if (typeof schema === 'function') { - validateFunc = schema; - joiSchema = null; - } - - let joiResult; - if (joiSchema) { - let body; - if (ctx.method === 'GET') { - body = ctx.params; - } else { - body = ctx.request.body; - } - - joiResult = Joi.validate(body, joiSchema, joiOptions); - if (joiResult.error) { - ctx.errors = parseJoiErrors(joiResult.error); - - ctx.status = 400; - ctx.body = { errors: ctx.errors }; - return { errors: ctx.errors }; - } - } - - ctx.errors = []; - const data = await validateFunc(joiResult.value); - const result = { - errors: null, - value: data, - }; - - if (ctx.errors.length) { - result.errors = ctx.errors; - } - - if (result.errors) { - ctx.status = 400; - ctx.body = { errors: ctx.errors }; - } - - return result; -}; diff --git a/src/resources/user/index.js b/src/resources/user/index.js index 663c239..6ddb8fe 100644 --- a/src/resources/user/index.js +++ b/src/resources/user/index.js @@ -1,9 +1,11 @@ const Router = require('koa-router'); +const validate = require('middlewares/validate'); +const validators = require('./validators'); const router = new Router(); const controller = require('./user.controller'); router.get('/current', controller.getCurrent); -router.put('/current', controller.updateCurrent); +router.put('/current', validate(validators.update, { throwOnInvalid: false }), controller.updateCurrent); module.exports = router.routes(); diff --git a/src/resources/user/user.controller.js b/src/resources/user/user.controller.js index bef5c80..d5a7346 100644 --- a/src/resources/user/user.controller.js +++ b/src/resources/user/user.controller.js @@ -1,20 +1,23 @@ const _ = require('lodash'); - const userService = require('./user.service'); -const validators = require('./validators'); const userOmitFelds = ['passwordHash', 'passwordSalt', 'signupToken', 'resetPasswordToken']; -exports.getCurrent = async (ctx, next) => { +exports.getCurrent = async (ctx) => { const user = await userService.findById(ctx.state.user._id); ctx.body = _.omit(user, userOmitFelds); }; -exports.updateCurrent = async (ctx, next) => { - const result = await validators.update.validate(ctx); - ctx.assert(!result.errors, 400); +exports.updateCurrent = async (ctx) => { + if (ctx.validatedRequest.errors.length) { + ctx.body = { + errors: ctx.validatedRequest.errors, + }; + + ctx.throw(400); + } - const { value: userData } = result; + const userData = ctx.validatedRequest.value; const user = await userService.updateInfo(ctx.state.user._id, userData); ctx.body = _.omit(user, userOmitFelds); diff --git a/src/resources/user/validators/update.validator.js b/src/resources/user/validators/update.validator.js index e2e9214..b0364ec 100644 --- a/src/resources/user/validators/update.validator.js +++ b/src/resources/user/validators/update.validator.js @@ -1,6 +1,5 @@ -const Joi = require('joi'); +const Joi = require('helpers/joi.adapter'); -const baseValidator = require('resources/base.validator'); const userService = require('resources/user/user.service'); const schema = { @@ -30,16 +29,25 @@ const schema = { }), }; -exports.validate = ctx => - baseValidator(ctx, schema, async (data) => { - const userExist = await userService.exists({ - _id: { $ne: ctx.state.user._id }, - email: data.email, - }); - if (userExist) { - ctx.errors.push({ email: 'This email is already in use.' }); - return false; - } - - return data; +const validateFunc = async (data, pesistentData) => { + const userExist = await userService.exists({ + _id: { $ne: pesistentData.state.user._id }, + email: data.email, }); + const errors = []; + + if (userExist) { + errors.push({ email: 'This email is already in use.' }); + return { errors }; + } + + return { + value: data, + errors, + }; +}; + +module.exports = [ + Joi.validate(schema), + validateFunc, +];