From 9fa7f9e01119dbd92e9d16ebe1ddeb67ddfbe6b0 Mon Sep 17 00:00:00 2001 From: danielhumgon Date: Wed, 29 Jan 2020 03:12:10 -0400 Subject: [PATCH] refactor(user): Back-ported changes in user controller --- src/modules/users/controller.js | 251 ++++++++++++++++++++------------ src/modules/users/router.js | 27 +++- test/a02-users.spec.js | 186 ++++++++++++++++++++++- 3 files changed, 362 insertions(+), 102 deletions(-) diff --git a/src/modules/users/controller.js b/src/modules/users/controller.js index 78f6c69..848080e 100644 --- a/src/modules/users/controller.js +++ b/src/modules/users/controller.js @@ -1,16 +1,23 @@ const User = require('../../models/users') -/** +let _this +class UserController { + constructor () { + _this = this + this.User = User + } + + /** * @api {post} /users Create a new user - * @apiPermission + * @apiPermission user * @apiName CreateUser * @apiGroup Users * * @apiExample Example usage: - * curl -H "Content-Type: application/json" -X POST -d '{ "user": { "username": "johndoe", "password": "secretpasas" } }' localhost:5000/users + * curl -H "Content-Type: application/json" -X POST -d '{ "user": { "email": "email@format.com", "password": "secretpasas" } }' localhost:5001/users * * @apiParam {Object} user User object (required) - * @apiParam {String} user.username Username. + * @apiParam {String} user.email Email. * @apiParam {String} user.password Password. * * @apiSuccess {Object} users User object @@ -18,6 +25,7 @@ const User = require('../../models/users') * @apiSuccess {String} user.type User type (admin or user) * @apiSuccess {String} users.name User name * @apiSuccess {String} users.username User username + * @apiSuccess {String} users.email User email * * @apiSuccessExample {json} Success-Response: * HTTP/1.1 200 OK @@ -25,7 +33,7 @@ const User = require('../../models/users') * "user": { * "_id": "56bd1da600a526986cf65c80" * "name": "John Doe" - * "username": "johndoe" + * "email": "email@format.com" * } * } * @@ -38,30 +46,52 @@ const User = require('../../models/users') * "error": "Unprocessable Entity" * } */ -async function createUser (ctx) { - const user = new User(ctx.request.body.user) + async createUser (ctx) { + const user = new _this.User(ctx.request.body.user) - // Enforce default value of 'user' - user.type = 'user' + try { + /* + * ERROR HANDLERS + * + */ + // Required property + if (!user.email || typeof user.email !== 'string') { + throw new Error(`Property 'email' must be a string!`) + } - try { - await user.save() - } catch (err) { - ctx.throw(422, err.message) + const isEmail = await _this.validateEmail(user.email) + if (!isEmail) { + throw new Error(`Property 'email' must be email format!`) + } + + if (!user.password || typeof user.password !== 'string') { + throw new Error(`Property 'password' must be a string!`) + } + + if (user.name && typeof user.name !== 'string') { + throw new Error(`Property 'name' must be a string!`) + } + + // Enforce default value of 'user' + user.type = 'user' + + await user.save() + + const token = user.generateToken() + const response = user.toJSON() + + delete response.password + + ctx.body = { + user: response, + token + } + } catch (err) { + ctx.throw(422, err.message) + } } - const token = user.generateToken() - const response = user.toJSON() - - delete response.password - - ctx.body = { - user: response, - token - } -} - -/** + /** * @api {get} /users Get all users * @apiPermission user * @apiName GetUsers @@ -75,6 +105,7 @@ async function createUser (ctx) { * @apiSuccess {String} user.type User type (admin or user) * @apiSuccess {String} users.name User name * @apiSuccess {String} users.username User username + * @apiSuccess {String} users.email User email * * @apiSuccessExample {json} Success-Response: * HTTP/1.1 200 OK @@ -82,18 +113,22 @@ async function createUser (ctx) { * "users": [{ * "_id": "56bd1da600a526986cf65c80" * "name": "John Doe" - * "username": "johndoe" + * "email": "email@format.com" * }] * } * * @apiUse TokenError */ -async function getUsers (ctx) { - const users = await User.find({}, '-password') - ctx.body = { users } -} + async getUsers (ctx) { + try { + const users = await _this.User.find({}, '-password') + ctx.body = { users } + } catch (error) { + ctx.throw(404) + } + } -/** + /** * @api {get} /users/:id Get user by id * @apiPermission user * @apiName GetUser @@ -107,6 +142,7 @@ async function getUsers (ctx) { * @apiSuccess {String} user.type User type (admin or user) * @apiSuccess {String} users.name User name * @apiSuccess {String} users.username User username + * @apiSuccess {String} users.email User email * * @apiSuccessExample {json} Success-Response: * HTTP/1.1 200 OK @@ -114,38 +150,36 @@ async function getUsers (ctx) { * "user": { * "_id": "56bd1da600a526986cf65c80" * "name": "John Doe" - * "username": "johndoe" + * "email": "email@format.com" * } * } * * @apiUse TokenError */ -async function getUser (ctx, next) { - try { - const user = await User.findById(ctx.params.id, '-password') - if (!user) { - ctx.throw(404) - } + async getUser (ctx, next) { + try { + const user = await _this.User.findById(ctx.params.id, '-password') + if (!user) { + ctx.throw(404) + } - ctx.body = { - user - } - } catch (err) { - if (err === 404 || err.name === 'CastError') { - ctx.throw(404) - } + ctx.body = { + user + } + } catch (err) { + if (err === 404 || err.name === 'CastError') { + ctx.throw(404) + } - ctx.throw(500) + ctx.throw(500) + } + if (next) { + return next() + } } - - if (next) { - return next() - } -} - -/** + /** * @api {put} /users/:id Update a user - * @apiPermission + * @apiPermission user * @apiName UpdateUser * @apiGroup Users * @@ -154,13 +188,14 @@ async function getUser (ctx, next) { * * @apiParam {Object} user User object (required) * @apiParam {String} user.name Name. - * @apiParam {String} user.username Username. + * @apiParam {String} user.email Email. * * @apiSuccess {Object} users User object * @apiSuccess {ObjectId} users._id User id * @apiSuccess {String} user.type User type (admin or user) * @apiSuccess {String} users.name Updated name * @apiSuccess {String} users.username Updated username + * @apiSuccess {String} users.email Updated email * * @apiSuccessExample {json} Success-Response: * HTTP/1.1 200 OK @@ -168,7 +203,7 @@ async function getUser (ctx, next) { * "user": { * "_id": "56bd1da600a526986cf65c80" * "name": "Cool new name" - * "username": "johndoe" + * "email": "email@format.com" * } * } * @@ -183,29 +218,65 @@ async function getUser (ctx, next) { * * @apiUse TokenError */ -async function updateUser (ctx) { - const user = ctx.body.user + async updateUser (ctx) { + // Values obtain from user request. + // This variable is intended to validate the properties + // sent by the client + const userObj = ctx.request.body.user - // Save a copy of the original user type. - const userType = user.type + const user = ctx.body.user + try { + /* + * ERROR HANDLERS + * + */ + // Required property + if (userObj.email && typeof userObj.email !== 'string') { + throw new Error(`Property 'email' must be a string!`) + } + const isEmail = await _this.validateEmail(user.email) + if (userObj.email && !isEmail) { + throw new Error(`Property 'email' must be email format!`) + } + if (userObj.password && typeof userObj.password !== 'string') { + throw new Error(`Property 'password' must be a string!`) + } + if (userObj.name && typeof userObj.name !== 'string') { + throw new Error(`Property 'name' must be a string!`) + } + if (userObj.projects && !Array.isArray(userObj.projects)) { + throw new Error(`Property 'projects' must be a Array!`) + } + // Save a copy of the original user type. + const userType = user.type - Object.assign(user, ctx.request.body.user) + // If user type property is sent by the client + if (userObj.type) { + if (typeof userObj.type !== 'string') { + throw new Error(`Property 'type' must be a string!`) + } + // TODO: Here we can validate the user types allowed - // Unless the calling user is an admin, they can not change the user type. - if (userType !== 'admin') { - user.type = userType + // Unless the calling user is an admin, they can not change the user type. + if (userType !== 'admin') { + throw new Error(`Property 'type' just can change for Admin user`) + } + } + + Object.assign(user, ctx.request.body.user) + + await user.save() + + ctx.body = { + user + } + } catch (error) { + ctx.throw(422, error.message) + } } - - await user.save() - - ctx.body = { - user - } -} - -/** + /** * @api {delete} /users/:id Delete a user - * @apiPermission + * @apiPermission user * @apiName DeleteUser * @apiGroup Users * @@ -222,22 +293,22 @@ async function updateUser (ctx) { * * @apiUse TokenError */ - -async function deleteUser (ctx) { - const user = ctx.body.user - - await user.remove() - - ctx.status = 200 - ctx.body = { - success: true + async deleteUser (ctx) { + const user = ctx.body.user + await user.remove() + ctx.status = 200 + ctx.body = { + success: true + } + } + // Validate Email Format + async validateEmail (email) { + // eslint-disable-next-line no-useless-escape + if (/^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,3})+$/.test(email)) { + return true + } + return false } } -module.exports = { - createUser, - getUsers, - getUser, - updateUser, - deleteUser -} +module.exports = UserController diff --git a/src/modules/users/router.js b/src/modules/users/router.js index a8840aa..f79b6f2 100644 --- a/src/modules/users/router.js +++ b/src/modules/users/router.js @@ -1,5 +1,6 @@ const validator = require('../../middleware/validators') -const user = require('./controller') +const CONTROLLER = require('./controller') +const controller = new CONTROLLER() // export const baseUrl = '/users' module.exports.baseUrl = '/users' @@ -8,26 +9,40 @@ module.exports.routes = [ { method: 'POST', route: '/', - handlers: [user.createUser] + handlers: [controller.createUser] }, { method: 'GET', route: '/', - handlers: [validator.ensureUser, user.getUsers] + handlers: [ + validator.ensureUser, + controller.getUsers + ] }, { method: 'GET', route: '/:id', - handlers: [validator.ensureUser, user.getUser] + handlers: [ + validator.ensureUser, + controller.getUser + ] }, { method: 'PUT', route: '/:id', - handlers: [validator.ensureTargetUserOrAdmin, user.getUser, user.updateUser] + handlers: [ + validator.ensureTargetUserOrAdmin, + controller.getUser, + controller.updateUser + ] }, { method: 'DELETE', route: '/:id', - handlers: [validator.ensureTargetUserOrAdmin, user.getUser, user.deleteUser] + handlers: [ + validator.ensureTargetUserOrAdmin, + controller.getUser, + controller.deleteUser + ] } ] diff --git a/test/a02-users.spec.js b/test/a02-users.spec.js index 5c6ce9e..403f9c0 100644 --- a/test/a02-users.spec.js +++ b/test/a02-users.spec.js @@ -67,8 +67,77 @@ describe('Users', () => { } } }) + it('should reject signup if no email property is provided', async () => { + try { + const options = { + method: 'POST', + uri: `${LOCALHOST}/users`, + resolveWithFullResponse: true, + json: true, + body: { + user: { + password: 'supersecretpassword' + } + } + } - it('should sign up', async () => { + await rp(options) + } catch (err) { + assert.equal(err.statusCode, 422) + assert.include( + err.message, + `Property 'email' must be a string` + ) + } + }) + it('should reject signup if email property provided is wrong format', async () => { + try { + const options = { + method: 'POST', + uri: `${LOCALHOST}/users`, + resolveWithFullResponse: true, + json: true, + body: { + user: { + email: 'badEmailFormat', + password: 'test' + } + } + } + + await rp(options) + } catch (err) { + assert.equal(err.statusCode, 422) + assert.include( + err.message, + `Property 'email' must be email format` + ) + } + }) + it('should reject signup if no password property is provided', async () => { + try { + const options = { + method: 'POST', + uri: `${LOCALHOST}/users`, + resolveWithFullResponse: true, + json: true, + body: { + user: { + email: 'test2@test.com' + } + } + } + + await rp(options) + } catch (err) { + assert.equal(err.statusCode, 422) + assert.include( + err.message, + `Property 'password' must be a string` + ) + } + }) + it('should signup of type user by default', async () => { try { const options = { method: 'POST', @@ -99,6 +168,7 @@ describe('Users', () => { 'Password expected to be omited' ) assert.property(result.body, 'token', 'Token property exists.') + assert.equal(result.body.user.type, 'user') } catch (err) { console.log( 'Error authenticating test user: ' + JSON.stringify(err, null, 2) @@ -330,7 +400,7 @@ describe('Users', () => { } }) - it('should update user', async () => { + it('should update user with minimum inputs', async () => { const { user: { _id }, token @@ -363,6 +433,45 @@ describe('Users', () => { ) assert.equal(user.email, 'testToUpdate@test.com') }) + it('should update user with all inputs', async () => { + const { + user: { _id }, + token + } = context + + const options = { + method: 'PUT', + uri: `${LOCALHOST}/users/${_id}`, + resolveWithFullResponse: true, + json: true, + headers: { + Accept: 'application/json', + Authorization: `Bearer ${token}` + }, + body: { + user: { + email: 'testToUpdate@test.com', + name: 'my name', + username: 'myUsername' + } + } + } + + const result = await rp(options) + const user = result.body.user + // console.log(`user: ${util.inspect(user)}`) + + assert.hasAnyKeys(user, ['type', '_id', 'email', 'name']) + assert.equal(user._id, _id) + assert.notProperty( + user, + 'password', + 'Password property should not be returned' + ) + assert.equal(user.name, 'my name') + assert.equal(user.email, 'testToUpdate@test.com') + assert.equal(user.username, 'myUsername') + }) it('should not be able to update user type', async () => { try { @@ -382,16 +491,18 @@ describe('Users', () => { } } - let result = await rp(options) + const result = await rp(options) // console.log(`Users: ${JSON.stringify(result, null, 2)}`) assert(result.statusCode === 200, 'Status Code 200 expected.') assert(result.body.user.type === 'user', 'Type should be unchanged.') } catch (err) { - console.error('Error: ', err) - console.log('Error stringified: ' + JSON.stringify(err, null, 2)) - throw err + assert.equal(err.statusCode, 422) + assert.include( + err.message, + "Property 'type' just can change for Admin user" + ) } }) @@ -451,6 +562,69 @@ describe('Users', () => { const userName = result.body.user.name assert.equal(userName, 'This should work') }) + it('should not be able to update if name property is wrong', async () => { + const { + user: { _id }, + token + } = context + + const options = { + method: 'PUT', + uri: `${LOCALHOST}/users/${_id}`, + resolveWithFullResponse: true, + json: true, + headers: { + Accept: 'application/json', + Authorization: `Bearer ${token}` + }, + body: { + user: { + email: 'testToUpdate@test.com', + name: {} + } + } + } + try { + const result = await rp(options) + const user = result.body.user + // console.log(`user: ${util.inspect(user)}`) + + assert.hasAnyKeys(user, ['type', '_id', 'username']) + assert.equal(user._id, _id) + assert.notProperty( + user, + 'password', + 'Password property should not be returned' + ) + assert.notEqual(user.username, 'updatedcoolname31') + } catch (error) { + assert.equal(error.statusCode, 422) + assert.include(error.message, "Property 'name' must be a string!") + } + }) + it('should not be able to update if email property provided is wrong format', async () => { + try { + const options = { + method: 'POST', + uri: `${LOCALHOST}/users`, + resolveWithFullResponse: true, + json: true, + body: { + user: { + email: 'badEmailFormat' + } + } + } + + await rp(options) + } catch (err) { + assert.equal(err.statusCode, 422) + assert.include( + err.message, + `Property 'email' must be email format` + ) + } + }) }) describe('DELETE /users/:id', () => {