From d0e714e64b13c808f26e3c824a9f9005c4da51c9 Mon Sep 17 00:00:00 2001 From: Irshad Ansari Date: Thu, 5 Mar 2020 20:59:43 +0530 Subject: [PATCH 1/2] Added validators and error handling on backend --- back-end/package-lock.json | 19 +++ back-end/package.json | 1 + back-end/server.js | 21 +++ back-end/src/models/User.js | 38 ++++- back-end/src/routes/api/books.js | 31 ++-- back-end/src/routes/api/courses.js | 13 +- back-end/src/routes/api/users.js | 167 +++++++++++----------- back-end/src/routes/createController.js | 180 ++++++++++++++++++++++++ back-end/src/utils/jwt.js | 29 ++++ back-end/src/validators/auth.js | 44 ++++++ 10 files changed, 438 insertions(+), 105 deletions(-) create mode 100644 back-end/src/routes/createController.js create mode 100644 back-end/src/utils/jwt.js create mode 100644 back-end/src/validators/auth.js diff --git a/back-end/package-lock.json b/back-end/package-lock.json index 2419e76..72a9ab3 100644 --- a/back-end/package-lock.json +++ b/back-end/package-lock.json @@ -303,6 +303,15 @@ "vary": "~1.1.2" } }, + "express-validator": { + "version": "6.4.0", + "resolved": "https://registry.npmjs.org/express-validator/-/express-validator-6.4.0.tgz", + "integrity": "sha512-Fs+x0yDOSiUV+o5jIRloMyBxqpSzJiMM8KQW1IRVv2l49F6ATU0F9uPa+3K6vXNlLlhUjauv2FCGLFPMaNr24w==", + "requires": { + "lodash": "^4.17.15", + "validator": "^12.1.0" + } + }, "extend": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/extend/-/extend-3.0.2.tgz", @@ -536,6 +545,11 @@ "resolved": "https://registry.npmjs.org/kareem/-/kareem-2.3.1.tgz", "integrity": "sha512-l3hLhffs9zqoDe8zjmb/mAN4B8VT3L56EUvKNqLFVs9YlFA+zx7ke1DO8STAdDyYNkeSo1nKmjuvQeI12So8Xw==" }, + "lodash": { + "version": "4.17.15", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.15.tgz", + "integrity": "sha512-8xOcRHvCjnocdS5cpwXQXVzmmh5e5+saE2QGoeQmbKmRS6J3VQppPOIt0MnmE+4xlZoumy0GPG0D0MVIQbNA1A==" + }, "lodash.foreach": { "version": "4.5.0", "resolved": "https://registry.npmjs.org/lodash.foreach/-/lodash.foreach-4.5.0.tgz", @@ -1050,6 +1064,11 @@ "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.4.0.tgz", "integrity": "sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A==" }, + "validator": { + "version": "12.2.0", + "resolved": "https://registry.npmjs.org/validator/-/validator-12.2.0.tgz", + "integrity": "sha512-jJfE/DW6tIK1Ek8nCfNFqt8Wb3nzMoAbocBF6/Icgg1ZFSBpObdnwVY2jQj6qUqzhx5jc71fpvBWyLGO7Xl+nQ==" + }, "vary": { "version": "1.1.2", "resolved": "https://registry.npmjs.org/vary/-/vary-1.1.2.tgz", diff --git a/back-end/package.json b/back-end/package.json index f59917a..65587b6 100644 --- a/back-end/package.json +++ b/back-end/package.json @@ -16,6 +16,7 @@ "cors": "^2.8.5", "dotenv": "^8.1.0", "express": "^4.17.1", + "express-validator": "^6.4.0", "jsonwebtoken": "^8.5.1", "mongoose": "^5.7.5", "mongoose-unique-validator": "^2.0.3", diff --git a/back-end/server.js b/back-end/server.js index c9c8215..d3ed26b 100644 --- a/back-end/server.js +++ b/back-end/server.js @@ -3,6 +3,7 @@ const app = express(); const bodyParser = require("body-parser"); const cors = require("cors"); const errorHandler = require("error-handler"); +const createError = require("http-errors"); const mongoose = require("mongoose"); require("./src/models/User"); @@ -40,6 +41,26 @@ mongoose.set("debug", true); app.use(require("./src/routes")); +app.use("*", (req, res, next) => { + next(new createError(404, "Page not found.")); +}); + +app.use((error, req, res, next) => { + if (error instanceof createError.HttpError) { + const obj = { + status: error.status, + message: error.message + }; + if (error.errors) { + obj.errors = error.errors; + } + res.status(error.status).json(obj); + } else { + console.log(error); + res.status(500).json({ status: 500, message: 'Server error.' }); + } +}); + app.listen(app.get("port"), () => { console.log(`Listening on port ${app.get("port")}`); }); diff --git a/back-end/src/models/User.js b/back-end/src/models/User.js index 1d7339c..1111cd0 100644 --- a/back-end/src/models/User.js +++ b/back-end/src/models/User.js @@ -1,6 +1,7 @@ const mongoose = require('mongoose') -const { Schema } = mongoose +const bcrypt = require('bcryptjs') const uniqueValidator = require('mongoose-unique-validator') +const { Schema } = mongoose const UserSchema = new Schema({ username: { @@ -71,6 +72,41 @@ UserSchema.methods.toJSON = function() { } } +const hashPassword = function(next) { + var user = this; + if (this.isModified('hash') || this.isNew) { + bcrypt.genSalt(10, function(err, salt) { + if (err) { + return next(err); + } + bcrypt.hash(user.hash, salt, function(err, hash) { + if (err) { + return next(err); + } + user.hash = hash; + next(); + }); + }); + } else { + return next(); + } +} + +const checkPassword = async function(pass) { + return new Promise((resolve, reject) => { + bcrypt.compare(pass, this.hash, function(err, isMatch) { + if (err) { + reject(err); + } + resolve(isMatch); + }); + }); +} + UserSchema.plugin(uniqueValidator, { message: 'is already taken.'}) +UserSchema.pre('save', hashPassword); + +UserSchema.methods.checkPassword = checkPassword; + module.exports = mongoose.model('User', UserSchema) \ No newline at end of file diff --git a/back-end/src/routes/api/books.js b/back-end/src/routes/api/books.js index b2dec69..a1ec2b3 100644 --- a/back-end/src/routes/api/books.js +++ b/back-end/src/routes/api/books.js @@ -1,14 +1,23 @@ - -const router = require("express").Router(); +const router = require('express').Router(); const request = require('request'); +const createController = require('../createController'); + +router.post( + '/getBooks', + createController(async (req, res) => { + const query = req.body.bookQuery.split(/\s*\s/).join('+'); -router.post("/getBooks", async (req, res) => { - const query = req.body.bookQuery.split(/\s*\s/).join("+"); - - request("https://www.googleapis.com/books/v1/volumes?q=" + query, {json: true}, (err, response, body) => { - if(err) { return console.log(err); } - return res.json(response); - }); -}); + request( + 'https://www.googleapis.com/books/v1/volumes?q=' + query, + { json: true }, + (err, response, body) => { + if (err) { + return console.log(err); + } + return res.json(response); + }, + ); + }), +); -module.exports = router; \ No newline at end of file +module.exports = router; diff --git a/back-end/src/routes/api/courses.js b/back-end/src/routes/api/courses.js index e40bb8f..ce769fb 100644 --- a/back-end/src/routes/api/courses.js +++ b/back-end/src/routes/api/courses.js @@ -2,6 +2,7 @@ const router = require('express').Router(); const axios = require('axios'); const querystring = require('querystring'); +const createController = require('../createController'); const udemyClientId = process.env.UDEMY_CLIENT_ID || ''; const udemyClientSecret = process.env.UDEMY_CLIENT_SECRET || ''; const udemyAuthKey = Buffer.from( @@ -14,8 +15,9 @@ const http = axios.create({ }, }); -router.get('/udemy', async (req, res, next) => { - try { +router.get( + '/udemy', + createController(async (req, res, next) => { const fields = [ 'search', 'price', @@ -39,10 +41,7 @@ router.get('/udemy', async (req, res, next) => { if (!data) throw new Error('No data present'); res.json({ courses: data.results }); - } catch (error) { - console.log(error); - res.status(500).send(); - } -}); + }), +); module.exports = router; diff --git a/back-end/src/routes/api/users.js b/back-end/src/routes/api/users.js index 6e00dcb..291faa8 100644 --- a/back-end/src/routes/api/users.js +++ b/back-end/src/routes/api/users.js @@ -1,98 +1,93 @@ -const router = require("express").Router(); -const User = require("../../models/User"); -const jwt = require("jsonwebtoken"); -const bcrypt = require("bcryptjs"); -const { - STATUS_OK, - USER_NOT_FOUND, - TOKEN_ERROR, - INCORRECT_PASSWORD -} = require("../../utils/constants"); +const router = require('express').Router(); +const createError = require('http-errors'); +const createController = require('../createController'); +const User = require('../../models/User'); +const authValidator = require('../../validators/auth'); +const jwt = require('../../utils/jwt'); +const { USER_NOT_FOUND, INCORRECT_PASSWORD } = require('../../utils/constants'); -router.post("/login", async (req, res, next) => { - let errors = {}; - const { username, password } = req.body; - await User.findOne({ username }).then(user => { - if (!user) { - errors.user = USER_NOT_FOUND; - return res.status(404).json(errors); - } - bcrypt.compare(password, user.hash).then(isMatch => { - if (isMatch) { - const payload = { - id: user._id, - username: user.username - }; - jwt.sign( - payload, - process.env.SECRET, - { - expiresIn: 3600 +router.post( + '/login', + createController( + async (req, res) => { + const { username, password } = res.locals.inputBody; + + const user = await User.findOne({ username }); + if (!user) + throw new createError(404, USER_NOT_FOUND, { + errors: { + username: USER_NOT_FOUND, }, - (err, token) => { - if (err) console.error(TOKEN_ERROR, err); - else { - res.json({ - success: true, - token - }); - } - } - ); + }); + const isMatch = await user.checkPassword(password); + if (isMatch) { + const token = await jwt.sign(user); + res.status(200).json({ + token, + }); } else { - errors.password = INCORRECT_PASSWORD; - return res.json(400).json(errors); + throw new createError(401, INCORRECT_PASSWORD, { + errors: { password: INCORRECT_PASSWORD }, + }); } - }); - }); -}); - -router.post("/register", async (req, res) => { - const { username, password, email, name, birth } = req.body; - if (await User.findOne({ username })) { - return res.json({ error: 'Username "' + username + '" is already taken' }); - } + }, + { + validation: { + throwError: true, + asObject: true, + validators: [authValidator.login], + }, + inputs: ['username', 'password'], + }, + ), +); - if (await User.findOne({ email })) { - return res.json({ error: 'Email "' + email + '" is already taken' }); - } +router.post( + '/register', + createController( + async (req, res) => { + const body = res.locals.inputBody; - const newUser = new User({ username, email, name, birth: new Date(birth) }); - - if (password) { - newUser.hash = bcrypt.hashSync(password, 10); - } + if (body.birth) { + body.birth = new Date(body.birth); + } - await newUser.save().then((user, err) => { - bcrypt.compare(password, user.hash).then(isMatch => { - if (isMatch) { - const payload = { - id: user._id, - username: user.username - }; - jwt.sign( - payload, - process.env.SECRET, - { - expiresIn: 3600 + if (await User.findOne({ username: body.username })) { + throw new createError(409, 'This username aleady exists.', { + errors: { + username: `Username '${body.username}' is already taken.'`, }, - (err, token) => { - if (err) console.error(TOKEN_ERROR, err); + }); + } - return res.json({ - status: STATUS_OK, - newUser: user, - success: true, - token - }); - } - ); - } else { - errors.password = INCORRECT_PASSWORD; - return res.json(400).json(errors); + if (await User.findOne({ email: body.email })) { + throw new createError(409, 'This email aleady exists.', { + errors: { + email: `Email '${body.email}' is already taken.'`, + }, + }); } - }); - }); -}); + + const newUser = new User(body); + + await newUser.save(); + + const token = await jwt.sign(newUser); + + res.status(201).json({ + user: newUser, + token, + }); + }, + { + validation: { + throwError: true, + asObject: true, + validators: [authValidator.register], + }, + inputs: ['name', 'username', ['password', 'hash'], 'email', 'birth'], + }, + ), +); module.exports = router; diff --git a/back-end/src/routes/createController.js b/back-end/src/routes/createController.js new file mode 100644 index 0000000..d5abed6 --- /dev/null +++ b/back-end/src/routes/createController.js @@ -0,0 +1,180 @@ +/** + * This will export a helper function which will abstract + * some functionalities of the controller. + */ + +const { validationResult } = require('express-validator'); +const createError = require('http-errors'); + +/** + * Convert errors array to object. + * @param {Array} errors + */ +const convertErrorToObject = (errors) => { + const fieldErrors = {}; + for (const error of errors) { + if (error.param && !fieldErrors[error.param]) + fieldErrors[error.param] = error.msg; + } + return fieldErrors; +}; + +/** + * Extract inputs from the request object. + * + * It will return all the fields if `fields` parameter is not specified + * or is empty. + * + * And if `fields` parameter is specified it will return the fields mentioned + * in it. + * + * @param {Object} req The request object from express + * @param {Array} fields The fields which needs to be extracted + */ +const getInputs = (req, fields) => { + const inputsObj = {}; + if (fields && fields.length > 0) { + for (const field of fields) { + if (Array.isArray(field)) { + if (field.length < 2) + throw new Error(`Invalid option in 'inputs' ` + field.join(' ')); + inputsObj[field[1]] = req.body[field[0]]; + } else { + inputsObj[field] = req.body[field]; + } + } + } else + for (const key of Object.keys(req.body)) inputsObj[key] = req.body[key]; + return inputsObj; +}; + +/** + * It returns the validation errors. + * + * @param {Object} req The request object from express + * @param {Boolean} asObject If it's value is true an Object type error + * is returned otherwise an array type error is returned + */ +const getValidationError = (req, asObject) => { + const errors = validationResult(req).array(); + if (asObject) { + return convertErrorToObject(errors); + } + return errors; +}; + +/** + * Create a middleware for validation check with the options specified + * @param {Object} options Options to be specified + * @param {String} options.errorMsg The error message that should be returned. + * @param {Boolean} options.throwError Specifies if the middleware should throw + * error or not. If set to false it will store the errors in req.validationError. + * @param {Boolean} options.asObject If the error returned should be an object + */ +const createValidationMiddleware = ({ + errorMsg, + throwError, + asObject, + renderPage, + renderData, +}) => { + return async function(req, res, next) { + try { + const errors = getValidationError(req, asObject); + if ( + (!asObject && errors.length > 0) || + (asObject && Object.keys(errors).length > 0) + ) { + if (throwError) { + next( + createError(400, { errors, code: 400, isCustom: true }, errorMsg), + ); + return; + } else res.locals.errors = errors; + + if (renderPage) { + let dataObj = {}; + if (renderData) { + if (typeof renderData == 'function') dataObj = await renderData(); + else if (typeof renderData == 'object') dataObj = renderData; + } + res.render(renderPage, dataObj); + return; + } + } + } catch (error) { + next(error); + } + + next(); + }; +}; + +/** + * This will create a series of middleware functions to execute common tasks + * based on the options provided. + * + * @param {*} cb A middleware which is to be executed + * @param {Object} options Options + * @param {Object} options.validation Validation Object + * @param {Array} options.validation.validators Validators array + * @param {String} options.validation.errorMsg Error message if validation failed + * @param {Boolean} options.validation.throwError If true throws error if validation fails + * @param {Boolean} options.validation.asObject Create error as object + * @param {Boolean|Array} options.inputs If true returns inputs in `res.locals.inputBody`. One can also provide an array with required fields + */ +const createController = ( + controller, + options = { + validation: { + validators: [], + errorMsg: 'Validation error.', + throwError: false, + asObject: false, + }, + inputs: false, + }, +) => { + const middlewareArray = []; + + const customController = async function(req, res, next) { + try { + await controller(req, res, next); + } catch (error) { + next(error); + } + }; + + if (options.inputs) { + let fields = []; + if (Array.isArray(options.inputs)) fields = options.inputs; + + middlewareArray.push((req, res, next) => { + const inputs = getInputs(req, fields); + if (res.locals.inputBody) { + res.locals.inputBody = { ...res.locals.inputBody, ...inputs }; + } else { + res.locals.inputBody = inputs; + } + next(); + }); + } + + if (options.validation) { + middlewareArray.push(options.validation.validators); + middlewareArray.push( + createValidationMiddleware({ + throwError: options.validation.throwError, + errorMsg: options.validation.errorMsg || 'Validation error.', + asObject: options.validation.asObject, + renderData: options.validation.renderData, + renderPage: options.validation.renderPage, + }), + ); + } + + middlewareArray.push(customController); + return middlewareArray; +}; + +module.exports = createController; diff --git a/back-end/src/utils/jwt.js b/back-end/src/utils/jwt.js new file mode 100644 index 0000000..62b592e --- /dev/null +++ b/back-end/src/utils/jwt.js @@ -0,0 +1,29 @@ +const jwt = require('jsonwebtoken'); + +module.exports.sign = async (user) => { + return new Promise(async (resolve, reject) => { + try { + const payload = { + id: user._id, + username: user.username, + }; + + jwt.sign( + payload, + process.env.SECRET, + { + expiresIn: 3600, + }, + (err, token) => { + if (err) { + reject(err); + } + + resolve(token); + }, + ); + } catch (error) { + reject(error); + } + }); +}; diff --git a/back-end/src/validators/auth.js b/back-end/src/validators/auth.js new file mode 100644 index 0000000..71b25e9 --- /dev/null +++ b/back-end/src/validators/auth.js @@ -0,0 +1,44 @@ +const { body } = require('express-validator'); + +module.exports = { + register: [ + body('name') + .trim() + .isString() + .notEmpty() + .withMessage('Name is required.'), + body('username') + .trim() + .isString() + .notEmpty({ ignore_whitespace: true }) + .withMessage('Username is required.') + .isAlphanumeric() + .withMessage('Username should be alphanumeric.'), + body('birth') + .trim() + .isString() + .optional(), + body('email') + .trim() + .isEmail() + .normalizeEmail() + .withMessage('Email is invalid.'), + body('password') + .trim() + .isString() + .isLength({ min: 8 }) + .withMessage('Password should be minimun of 8 charecters.'), + ], + + login: [ + body('username') + .trim() + .isString() + .withMessage('Username is required.'), + body('password') + .trim() + .isString() + .isLength({ min: 8 }) + .withMessage('Password should be minimun of 8 charecters.'), + ], +}; From 09e144babce9852129736ab129ed651dd5569589 Mon Sep 17 00:00:00 2001 From: Irshad Ansari Date: Thu, 5 Mar 2020 21:17:33 +0530 Subject: [PATCH 2/2] Changed frontend according to new API error structure --- front-end/src/components/Login/index.jsx | 23 +++++++++---- front-end/src/components/Register/index.jsx | 37 +++++++++++++-------- front-end/src/services/http.js | 31 +++++++++++++++++ 3 files changed, 70 insertions(+), 21 deletions(-) diff --git a/front-end/src/components/Login/index.jsx b/front-end/src/components/Login/index.jsx index 42ee111..edfe2f4 100644 --- a/front-end/src/components/Login/index.jsx +++ b/front-end/src/components/Login/index.jsx @@ -17,7 +17,8 @@ class Login extends Component { password: "", isAuthenticated: false, exists: false, - hidden: true + errors: {}, + commonError: null }; this.handleSubmit = this.handleSubmit.bind(this); @@ -38,7 +39,10 @@ class Login extends Component { username, password }; - + this.setState({ + errors: {}, + commonError: null + }) userLogin(user) .then(res => { @@ -56,10 +60,11 @@ class Login extends Component { this.props.history.push("/", this.state); } }).catch(error => { + const len = Object.keys(error.data).length; this.setState({ - hidden: false + errors: len > 0 ? error.data : {}, + commonError: len <= 0 ? error.message : null }); - this.props.history.push("/sign-in"); }) } @@ -70,7 +75,7 @@ class Login extends Component { } render() { - const { username, password, exists, hidden } = this.state; + const { username, password, exists, commonError, errors } = this.state; const isEnabled = username && password; return ( @@ -80,9 +85,9 @@ class Login extends Component {

Sign In

- {!exists ? ( + {!exists && commonError ? ( - + { commonError } ) : null}