diff --git a/src/lib/users.js b/src/lib/users.js index 111aeb1..aad7fa0 100644 --- a/src/lib/users.js +++ b/src/lib/users.js @@ -90,6 +90,51 @@ class UserLib { throw err } } + + async updateUser (existingUser, newData) { + try { + // Input Validation + if (!newData.email || typeof newData.email !== 'string') { + throw new Error("Property 'email' must be a string!") + } + if (!newData.password || typeof newData.password !== 'string') { + throw new Error("Property 'password' must be a string!") + } + if (!newData.name || typeof newData.name !== 'string') { + throw new Error("Property 'name' must be a string!") + } + + // Save a copy of the original user type. + const userType = existingUser.type + console.log('userType: ', userType) + + // If user 'type' property is sent by the client + if (newData.type) { + if (typeof newData.type !== 'string') { + throw new Error("Property 'type' must be a string!") + } + + // Unless the calling user is an admin, they can not change the user type. + if (userType !== 'admin') { + throw new Error("Property 'type' can only be changed by Admin user") + } + } + + // Overwrite any existing data with the new data. + Object.assign(existingUser, newData) + + // Save the changes to the database. + await existingUser.save() + + // Delete the password property. + delete existingUser.password + + return existingUser + } catch (err) { + wlogger.error('Error in lib/users.js/updateUser()') + throw err + } + } } module.exports = UserLib diff --git a/src/modules/users/controller.js b/src/modules/users/controller.js index 8c5b481..9ef4f79 100644 --- a/src/modules/users/controller.js +++ b/src/modules/users/controller.js @@ -200,54 +200,11 @@ class UserController { * @apiUse TokenError */ 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 - - 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 existingUser = ctx.body.user + const newData = ctx.request.body.user - const isEmail = await _this.validateEmail(userObj.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 - - // 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') { - throw new Error("Property 'type' can only be changed by Admin user") - } - } - - Object.assign(user, ctx.request.body.user) - - await user.save() + const user = await _this.userLib.updateUser(existingUser, newData) ctx.body = { user diff --git a/test/e2e/automated/a02-users.rest-e2e.js b/test/e2e/automated/a02-users.rest-e2e.js index d239866..96519f5 100644 --- a/test/e2e/automated/a02-users.rest-e2e.js +++ b/test/e2e/automated/a02-users.rest-e2e.js @@ -384,355 +384,314 @@ describe('Users', () => { ) }) }) - // - // describe('PUT /users/:id', () => { - // it('should not update user if token is invalid', async () => { - // try { - // const options = { - // method: 'PUT', - // url: `${LOCALHOST}/users/1`, - // headers: { - // Accept: 'application/json', - // Authorization: 'Bearer 1' - // } - // } - // await axios(options) - // - // assert.equal(true, false, 'Unexpected behavior') - // } catch (err) { - // assert.equal(err.response.status, 401) - // } - // }) - // - // it('should throw 401 if non-admin updating other user', async () => { - // const { token } = context - // - // try { - // const options = { - // method: 'PUT', - // url: `${LOCALHOST}/users/1`, - // headers: { - // Accept: 'application/json', - // Authorization: `Bearer ${token}` - // } - // } - // await axios(options) - // - // assert.equal(true, false, 'Unexpected behavior') - // } catch (err) { - // assert.equal(err.response.status, 401) - // } - // }) - // - // it('should not be able to update user type', async () => { - // try { - // const options = { - // method: 'PUT', - // url: `${LOCALHOST}/users/${context.user._id.toString()}`, - // headers: { - // Authorization: `Bearer ${context.token}` - // }, - // data: { - // user: { - // name: 'new name', - // type: 'test' - // } - // } - // } - // await axios(options) - // - // // console.log(`Users: ${JSON.stringify(result.data, null, 2)}`) - // - // // assert(result.status === 200, 'Status Code 200 expected.') - // // assert(result.data.user.type === 'user', 'Type should be unchanged.') - // assert.equal(true, false, 'Unexpected behavior') - // } catch (err) { - // assert.equal(err.response.status, 422) - // assert.include( - // err.response.data, - // "Property 'type' can only be changed by Admin user" - // ) - // } - // }) - // - // it('should not be able to update other user when not admin', async () => { - // try { - // const options = { - // method: 'PUT', - // url: `${LOCALHOST}/users/${context.user2._id.toString()}`, - // headers: { - // Authorization: `Bearer ${context.token}` - // }, - // data: { - // user: { - // name: 'This should not work' - // } - // } - // } - // await axios(options) - // - // // console.log(`result: ${JSON.stringify(result.data, null, 2)}`) - // - // assert(false, 'Unexpected result') - // } catch (err) { - // assert.equal(err.response.status, 401) - // } - // }) - // - // it('should not be able to update if name property is wrong', async () => { - // try { - // const _id = context.user._id - // const token = context.token - // - // const options = { - // method: 'PUT', - // url: `${LOCALHOST}/users/${_id}`, - // headers: { - // Accept: 'application/json', - // Authorization: `Bearer ${token}` - // }, - // data: { - // user: { - // email: 'testToUpdate@test.com', - // name: {} - // } - // } - // } - // await axios(options) - // } catch (error) { - // assert.equal(error.response.status, 422) - // assert.include(error.response.data, "Property 'name' must be a string!") - // } - // }) - // it('should not be able to update if password property is not string', async () => { - // const { token } = context - // const _id = context.user._id - // try { - // const options = { - // method: 'PUT', - // url: `${LOCALHOST}/users/${_id}`, - // headers: { - // Accept: 'application/json', - // Authorization: `Bearer ${token}` - // }, - // data: { - // user: { - // password: 1234 - // } - // } - // } - // await axios(options) - // - // assert.equal(true, false, 'Unexpected behavior') - // } catch (err) { - // assert.equal(err.response.status, 422) - // assert.include( - // err.response.data, - // "Property 'password' must be a string!" - // ) - // } - // }) - // it('should not be able to update if project property is not array', async () => { - // const { token } = context - // const _id = context.user._id - // try { - // const options = { - // method: 'PUT', - // url: `${LOCALHOST}/users/${_id}`, - // headers: { - // Accept: 'application/json', - // Authorization: `Bearer ${token}` - // }, - // data: { - // user: { - // projects: 'projects' - // } - // } - // } - // await axios(options) - // - // assert.equal(true, false, 'Unexpected behavior') - // } catch (err) { - // assert.equal(err.response.status, 422) - // assert.include( - // err.response.data, - // "Property 'projects' must be a Array!" - // ) - // } - // }) - // it('should not be able to update if email is not string', async () => { - // const { token } = context - // const _id = context.user._id - // try { - // const options = { - // method: 'PUT', - // url: `${LOCALHOST}/users/${_id}`, - // headers: { - // Accept: 'application/json', - // Authorization: `Bearer ${token}` - // }, - // data: { - // user: { - // email: 1234 - // } - // } - // } - // await axios(options) - // - // assert.equal(true, false, 'Unexpected behavior') - // } catch (err) { - // assert.equal(err.response.status, 422) - // assert.include(err.response.data, "Property 'email' must be a string!") - // } - // }) - // it('should not be able to update if email is wrong format', async () => { - // try { - // const _id = context.user._id - // const token = context.token - // - // const options = { - // method: 'PUT', - // url: `${LOCALHOST}/users/${_id}`, - // headers: { - // Accept: 'application/json', - // Authorization: `Bearer ${token}` - // }, - // data: { - // user: { - // email: 'badEmailFormat' - // } - // } - // } - // await axios(options) - // } catch (err) { - // assert.equal(err.response.status, 422) - // assert.include( - // err.response.data, - // "Property 'email' must be email format!" - // ) - // } - // }) - // it('should not be able to update type property if is not string', async () => { - // try { - // const _id = context.user._id - // const token = context.token - // - // const options = { - // method: 'PUT', - // url: `${LOCALHOST}/users/${_id}`, - // headers: { - // Accept: 'application/json', - // Authorization: `Bearer ${token}` - // }, - // data: { - // user: { - // type: 1 - // } - // } - // } - // await axios(options) - // } catch (err) { - // assert.equal(err.response.status, 422) - // assert.include(err.response.data, "Property 'type' must be a string!") - // } - // }) - // - // it('should be able to update other user when admin', async () => { - // const adminJWT = context.adminJWT - // - // const options = { - // method: 'PUT', - // url: `${LOCALHOST}/users/${context.user2._id.toString()}`, - // headers: { - // Authorization: `Bearer ${adminJWT}` - // }, - // data: { - // user: { - // name: 'This should work' - // } - // } - // } - // const result = await axios(options) - // // console.log(`result stringified: ${JSON.stringify(result, null, 2)}`) - // - // const userName = result.data.user.name - // assert.equal(userName, 'This should work') - // }) - // it('should update user with minimum inputs', async () => { - // const _id = context.user._id - // const token = context.token - // - // const options = { - // method: 'PUT', - // url: `${LOCALHOST}/users/${_id}`, - // headers: { - // Accept: 'application/json', - // Authorization: `Bearer ${token}` - // }, - // data: { - // user: { email: 'testToUpdate@test.com' } - // } - // } - // - // const result = await axios(options) - // const user = result.data.user - // // console.log(`user: ${util.inspect(user)}`) - // - // assert.property(user, 'type') - // assert.property(user, 'email') - // - // assert.property(user, '_id') - // assert.equal(user._id, _id) - // - // assert.notProperty( - // user, - // 'password', - // 'Password property should not be returned' - // ) - // assert.equal(user.email, 'testToUpdate@test.com') - // }) - // - // it('should update user with all inputs', async () => { - // const _id = context.user._id - // const token = context.token - // - // const options = { - // method: 'PUT', - // url: `${LOCALHOST}/users/${_id}`, - // headers: { - // Accept: 'application/json', - // Authorization: `Bearer ${token}` - // }, - // data: { - // user: { - // email: 'testToUpdate@test.com', - // name: 'my name', - // username: 'myUsername' - // } - // } - // } - // const result = await axios(options) - // - // const user = result.data.user - // // console.log(`user: ${util.inspect(user)}`) - // - // assert.property(user, 'type') - // assert.property(user, 'email') - // assert.property(user, 'name') - // - // assert.property(user, '_id') - // 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') - // }) - // }) - // + + describe('PUT /users/:id', () => { + it('should not update user if token is invalid', async () => { + try { + const options = { + method: 'PUT', + url: `${LOCALHOST}/users/1`, + headers: { + Accept: 'application/json', + Authorization: 'Bearer 1' + } + } + await axios(options) + + assert.equal(true, false, 'Unexpected behavior') + } catch (err) { + assert.equal(err.response.status, 401) + } + }) + + it('should throw 401 if non-admin updating other user', async () => { + const { token } = context + + try { + const options = { + method: 'PUT', + url: `${LOCALHOST}/users/1`, + headers: { + Accept: 'application/json', + Authorization: `Bearer ${token}` + } + } + await axios(options) + + assert.equal(true, false, 'Unexpected behavior') + } catch (err) { + assert.equal(err.response.status, 401) + } + }) + + it('should not be able to update user type', async () => { + try { + const options = { + method: 'PUT', + url: `${LOCALHOST}/users/${context.user._id.toString()}`, + headers: { + Authorization: `Bearer ${context.token}` + }, + data: { + user: { + email: 'test@test.com', + password: 'password', + name: 'new name', + type: 'test' + } + } + } + await axios(options) + + // console.log(`Users: ${JSON.stringify(result.data, null, 2)}`) + + // assert(result.status === 200, 'Status Code 200 expected.') + // assert(result.data.user.type === 'user', 'Type should be unchanged.') + assert.equal(true, false, 'Unexpected behavior') + } catch (err) { + assert.equal(err.response.status, 422) + assert.include( + err.response.data, + "Property 'type' can only be changed by Admin user" + ) + } + }) + + it('should not be able to update other user when not admin', async () => { + try { + const options = { + method: 'PUT', + url: `${LOCALHOST}/users/${context.user2._id.toString()}`, + headers: { + Authorization: `Bearer ${context.token}` + }, + data: { + user: { + name: 'This should not work' + } + } + } + await axios(options) + + // console.log(`result: ${JSON.stringify(result.data, null, 2)}`) + + assert(false, 'Unexpected result') + } catch (err) { + assert.equal(err.response.status, 401) + } + }) + + it('should not be able to update if name property is wrong', async () => { + try { + const _id = context.user._id + const token = context.token + + const options = { + method: 'PUT', + url: `${LOCALHOST}/users/${_id}`, + headers: { + Accept: 'application/json', + Authorization: `Bearer ${token}` + }, + data: { + user: { + email: 'testToUpdate@test.com', + name: {}, + password: 'password' + } + } + } + await axios(options) + } catch (error) { + assert.equal(error.response.status, 422) + assert.include(error.response.data, "Property 'name' must be a string!") + } + }) + + it('should not be able to update if password property is not string', async () => { + const { token } = context + const _id = context.user._id + try { + const options = { + method: 'PUT', + url: `${LOCALHOST}/users/${_id}`, + headers: { + Accept: 'application/json', + Authorization: `Bearer ${token}` + }, + data: { + user: { + password: 1234, + email: 'test@test.com', + name: 'test' + } + } + } + await axios(options) + + assert.equal(true, false, 'Unexpected behavior') + } catch (err) { + assert.equal(err.response.status, 422) + assert.include( + err.response.data, + "Property 'password' must be a string!" + ) + } + }) + + it('should not be able to update if email is not string', async () => { + const { token } = context + const _id = context.user._id + try { + const options = { + method: 'PUT', + url: `${LOCALHOST}/users/${_id}`, + headers: { + Accept: 'application/json', + Authorization: `Bearer ${token}` + }, + data: { + user: { + email: 1234 + } + } + } + await axios(options) + + assert.equal(true, false, 'Unexpected behavior') + } catch (err) { + assert.equal(err.response.status, 422) + assert.include(err.response.data, "Property 'email' must be a string!") + } + }) + + it('should not be able to update type property if is not string', async () => { + try { + const _id = context.user._id + const token = context.token + + const options = { + method: 'PUT', + url: `${LOCALHOST}/users/${_id}`, + headers: { + Accept: 'application/json', + Authorization: `Bearer ${token}` + }, + data: { + user: { + type: 1, + email: 'test@test.com', + name: 'test', + password: 'password' + } + } + } + await axios(options) + } catch (err) { + assert.equal(err.response.status, 422) + assert.include(err.response.data, "Property 'type' must be a string!") + } + }) + + it('should be able to update other user when admin', async () => { + const adminJWT = context.adminJWT + + const options = { + method: 'PUT', + url: `${LOCALHOST}/users/${context.user2._id.toString()}`, + headers: { + Authorization: `Bearer ${adminJWT}` + }, + data: { + user: { + name: 'This should work', + email: 'test@test.com' + // password: 'password' + } + } + } + const result = await axios(options) + // console.log(`result stringified: ${JSON.stringify(result, null, 2)}`) + + const userName = result.data.user.name + assert.equal(userName, 'This should work') + }) + + it('should update user with minimum inputs', async () => { + const _id = context.user._id + const token = context.token + + const options = { + method: 'PUT', + url: `${LOCALHOST}/users/${_id}`, + headers: { + Accept: 'application/json', + Authorization: `Bearer ${token}` + }, + data: { + user: { email: 'testToUpdate@test.com' } + } + } + + const result = await axios(options) + const user = result.data.user + // console.log(`user: ${util.inspect(user)}`) + + assert.property(user, 'type') + assert.property(user, 'email') + + assert.property(user, '_id') + assert.equal(user._id, _id) + + assert.notProperty( + user, + 'password', + 'Password property should not be returned' + ) + assert.equal(user.email, 'testToUpdate@test.com') + }) + + it('should update user with all inputs', async () => { + const _id = context.user._id + const token = context.token + + const options = { + method: 'PUT', + url: `${LOCALHOST}/users/${_id}`, + headers: { + Accept: 'application/json', + Authorization: `Bearer ${token}` + }, + data: { + user: { + email: 'testToUpdate@test.com', + name: 'my name', + username: 'myUsername' + } + } + } + const result = await axios(options) + + const user = result.data.user + // console.log(`user: ${util.inspect(user)}`) + + assert.property(user, 'type') + assert.property(user, 'email') + assert.property(user, 'name') + + assert.property(user, '_id') + 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') + }) + }) + // describe('DELETE /users/:id', () => { // it('should not delete user if token is invalid', async () => { // try { diff --git a/test/unit/biz-logic/a02-users.lib-unit.js b/test/unit/biz-logic/a02-users.lib-unit.js index c58aa96..0f70da4 100644 --- a/test/unit/biz-logic/a02-users.lib-unit.js +++ b/test/unit/biz-logic/a02-users.lib-unit.js @@ -1,5 +1,7 @@ /* Unit tests for the src/lib/users.js business logic library. + + TODO: verify that an admin can change the type of a user */ // Public npm libraries @@ -17,7 +19,7 @@ const UserLib = require('../../../src/lib/users') describe('#users', () => { let uut let sandbox - let testUserId = '' + let testUser = {} before(async () => { // Connect to the Mongo Database. @@ -127,7 +129,7 @@ describe('#users', () => { const { userData, token } = await uut.createUser(usrObj) - testUserId = userData._id + testUser = userData // Assert that the user model has the expected properties with expected values. assert.property(userData, 'type') @@ -204,10 +206,14 @@ describe('#users', () => { }) it('should return the user model', async () => { - const params = { id: testUserId } + const params = { id: testUser._id } const result = await uut.getUser(params) // console.log('result: ', result) + // Replace the JSON model with an actual Mongoos model. Used by later + // test cases. + testUser = result + // Assert that the expected properties for the user model exist. assert.property(result, 'type') assert.property(result, '_id') @@ -215,4 +221,114 @@ describe('#users', () => { assert.property(result, 'name') }) }) + + describe('#updateUser', () => { + it('should throw an error if no input is given', async () => { + try { + await uut.updateUser() + + assert.fail('Unexpected code path') + } catch (err) { + // console.log(err) + assert.include(err.message, 'Cannot read property') + } + }) + + it('should throw an error if no email given', async () => { + try { + await uut.updateUser(testUser, {}) + + assert.fail('Unexpected code path') + } catch (err) { + // console.log(err) + assert.include(err.message, "Property 'email' must be a string!") + } + }) + + it('should throw an error if no password given', async () => { + try { + const newData = { + email: 'test@test.com' + } + + await uut.updateUser(testUser, newData) + + assert.fail('Unexpected code path') + } catch (err) { + // console.log(err) + assert.include(err.message, "Property 'password' must be a string!") + } + }) + + it('should throw an error if no name given', async () => { + try { + const newData = { + email: 'test@test.com', + password: 'password' + } + + await uut.updateUser(testUser, newData) + + assert.fail('Unexpected code path') + } catch (err) { + // console.log(err) + assert.include(err.message, "Property 'name' must be a string!") + } + }) + + it('should throw an error for malformed type given', async () => { + try { + const newData = { + email: 'test@test.com', + password: 'password', + name: 'test', + type: 1234 + } + + await uut.updateUser(testUser, newData) + + assert.fail('Unexpected code path') + } catch (err) { + // console.log(err) + assert.include(err.message, "Property 'type' must be a string!") + } + }) + + it('should throw an error if normal user tries to change themselves into an admin', async () => { + try { + const newData = { + email: 'test@test.com', + password: 'password', + name: 'test', + type: 'admin' + } + + await uut.updateUser(testUser, newData) + + assert.fail('Unexpected code path') + } catch (err) { + // console.log(err) + assert.include(err.message, "Property 'type' can only be changed by Admin user") + } + }) + + it('should update the user model', async () => { + const newData = { + email: 'test@test.com', + password: 'password', + name: 'testy tester' + } + + const result = await uut.updateUser(testUser, newData) + + // Assert that expected properties and values exist. + assert.property(result, '_id') + assert.property(result, 'email') + assert.equal(result.email, 'test@test.com') + assert.property(result, 'name') + assert.equal(result.name, 'testy tester') + }) + + // TODO: verify that an admin can change the type of a user + }) }) diff --git a/test/unit/rest-api/a02-users.rest-unit.js b/test/unit/rest-api/a02-users.rest-unit.js index 985c2fe..4386874 100644 --- a/test/unit/rest-api/a02-users.rest-unit.js +++ b/test/unit/rest-api/a02-users.rest-unit.js @@ -10,6 +10,7 @@ const mongoose = require('mongoose') // Local support libraries const config = require('../../../config') const testUtils = require('../../utils/test-utils') +const User = require('../../../src/models/users') const UserController = require('../../../src/modules/users/controller') let uut @@ -19,6 +20,8 @@ let ctx const mockContext = require('../../unit/mocks/ctx-mock').context describe('Users', () => { + let testUser = {} + before(async () => { // Connect to the Mongo Database. mongoose.Promise = global.Promise @@ -102,6 +105,10 @@ describe('Users', () => { // Assert that expected properties exist in the returned data. assert.property(ctx.response.body, 'user') assert.property(ctx.response.body, 'token') + + // Used by downstream tests. + testUser = ctx.response.body.user + // console.log('testUser: ', testUser) }) }) @@ -177,4 +184,43 @@ describe('Users', () => { } }) }) + + describe('PUT /users/:id', () => { + it('should return 422 if no input data given', async () => { + try { + await uut.updateUser(ctx) + + assert.fail('Unexpected result') + } catch (err) { + // console.log(err) + assert.equal(err.status, 422) + assert.include(err.message, 'Cannot read property') + } + }) + + it('should return 200 on success', async () => { + // Prep the testUser data. + // console.log('testUser: ', testUser) + testUser.password = 'password' + delete testUser.type + + // Replace the testUser variable with an actual model from the DB. + const existingUser = await User.findById(testUser._id) + + ctx.body = { + user: existingUser + } + ctx.request.body = { + user: testUser + } + + await uut.updateUser(ctx) + + // Assert the expected HTTP response + assert.equal(ctx.status, 200) + + // Assert that expected properties exist in the returned data. + assert.property(ctx.response.body, 'user') + }) + }) })