From 60b217962565c9c67f0a0c73e8378de55dede3fb Mon Sep 17 00:00:00 2001 From: Yash Sharma Date: Mon, 30 Jan 2017 16:34:25 -0600 Subject: [PATCH 1/6] Added deleteUserByEmail admin endpoint --- api/v1/controllers/EcosystemController.js | 2 +- api/v1/controllers/UserController.js | 19 +++++++++++++++++++ api/v1/services/UserService.js | 10 ++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/api/v1/controllers/EcosystemController.js b/api/v1/controllers/EcosystemController.js index 1a6d6f4..cb17723 100644 --- a/api/v1/controllers/EcosystemController.js +++ b/api/v1/controllers/EcosystemController.js @@ -50,7 +50,7 @@ function deleteEcosystem (req, res, next) { EcosystemService .deleteEcosystem(name) .then(function () { - res.body = {} + res.body = {}; next(); return null; diff --git a/api/v1/controllers/UserController.js b/api/v1/controllers/UserController.js index 5fb4a18..974b9f8 100644 --- a/api/v1/controllers/UserController.js +++ b/api/v1/controllers/UserController.js @@ -100,6 +100,24 @@ function requestPasswordReset (req, res, next) { }); } +function deleteUser (req, res , next) { + var userEmail = req.body.email; + + services.UserService + .findUserByEmail(userEmail) + .then(function (user) { + return services.UserService.deleteUser(user); + }) + .then(function () { + res.body = {}; + + return next(); + }) + .catch(function (error){ + return next(error); + }); +} + router.use(bodyParser.json()); router.use(middleware.auth); @@ -108,6 +126,7 @@ router.post('/accredited', middleware.request(requests.AccreditedUserCreationReq middleware.permission(roles.ORGANIZERS), createAccreditedUser); router.post('/reset', middleware.request(requests.ResetTokenRequest), requestPasswordReset); router.get('/:id', middleware.permission(roles.ORGANIZERS, isRequester), getUser); +router.delete('/', middleware.permission(roles.ORGANIZERS), deleteUser); router.use(middleware.response); router.use(middleware.errors); diff --git a/api/v1/services/UserService.js b/api/v1/services/UserService.js index 097e23c..02b377c 100644 --- a/api/v1/services/UserService.js +++ b/api/v1/services/UserService.js @@ -118,3 +118,13 @@ module.exports.resetPassword = function (user, password) { return updated.save(); }); }; + +module.exports.deleteUser = function (user) { + if(user.hasRole(utils.roles.ATTENDEE, false)) { + var message = "The provided user has already filled out a registration"; + var source = "AttendeeRole"; + throw new errors.InvalidParameterError(message, source); + } + + return user.destroy(); +}; \ No newline at end of file From 514da0bf5c32a27e157f36fb9f1402781de7c3da Mon Sep 17 00:00:00 2001 From: Yash Sharma Date: Mon, 30 Jan 2017 18:04:22 -0600 Subject: [PATCH 2/6] Added tests and addresseed PR review --- api/v1/controllers/UserController.js | 2 +- api/v1/services/UserService.js | 6 +-- test/user.js | 55 +++++++++++++++++++++++++++- 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/api/v1/controllers/UserController.js b/api/v1/controllers/UserController.js index 974b9f8..c8c6cdd 100644 --- a/api/v1/controllers/UserController.js +++ b/api/v1/controllers/UserController.js @@ -126,7 +126,7 @@ router.post('/accredited', middleware.request(requests.AccreditedUserCreationReq middleware.permission(roles.ORGANIZERS), createAccreditedUser); router.post('/reset', middleware.request(requests.ResetTokenRequest), requestPasswordReset); router.get('/:id', middleware.permission(roles.ORGANIZERS, isRequester), getUser); -router.delete('/', middleware.permission(roles.ORGANIZERS), deleteUser); +router.delete('/', middleware.permission(roles.SUPERUSER), deleteUser); router.use(middleware.response); router.use(middleware.errors); diff --git a/api/v1/services/UserService.js b/api/v1/services/UserService.js index 02b377c..b976c16 100644 --- a/api/v1/services/UserService.js +++ b/api/v1/services/UserService.js @@ -120,9 +120,9 @@ module.exports.resetPassword = function (user, password) { }; module.exports.deleteUser = function (user) { - if(user.hasRole(utils.roles.ATTENDEE, false)) { - var message = "The provided user has already filled out a registration"; - var source = "AttendeeRole"; + if(user.hasRoles(utils.roles.ALL, false)) { + var message = "The provided user has a role and can not be deleted"; + var source = "UserRole"; throw new errors.InvalidParameterError(message, source); } diff --git a/test/user.js b/test/user.js index f1de507..e4ca554 100644 --- a/test/user.js +++ b/test/user.js @@ -10,6 +10,7 @@ var User = require('../api/v1/models/User.js'); var assert = chai.assert; var expect = chai.expect; +var tracker = require('mock-knex').getTracker(); describe('UserService',function(){ describe('createUser', function () { @@ -157,5 +158,55 @@ describe('UserService',function(){ _save.restore(); done(); }); - }) -}); + }); + + describe('deleteUser', function() { + var toDeleteUser; + var invalidToDeleteUser; + var _destroy; + before(function(done) { + toDeleteUser = User.forge({ id: 1, email: 'to@delete.com'}); + invalidToDeleteUser = User.forge({ id: 2, email: 'cant@delete.com'}); + invalidToDeleteUser.setPassword('password123') + .then(function(updatedUser) { + updatedUser.related('roles').add({ role: utils.roles.ATTENDEE}); + + _destroy = sinon.spy(User.prototype,'destroy'); + + done(); + }); + }); + beforeEach(function (done) { + tracker.install(); + done(); + }); + it('deletes a user with no role', function(done) { + tracker.on('query', function (query) { + query.response([]); + }); + + var deleteValid = UserService.deleteUser(toDeleteUser); + deleteValid + .then(function () { + assert(_destroy.calledOnce, "User destroy not called"); + + done(); + }) + .catch(function (err) { + done(err); + }); + }); + it('throws an exception when deleting a user with a role', function(done) { + expect(() => UserService.deleteUser(invalidToDeleteUser)).to.throw(errors.InvalidParameterError); + done(); + }); + afterEach(function (done) { + tracker.uninstall(); + done(); + }); + after(function(done) { + _destroy.restore(); + done(); + }); + }); +}); \ No newline at end of file From 6d7bc222a249a39a6e76f8c94a549c329f573ec1 Mon Sep 17 00:00:00 2001 From: Yash Sharma Date: Mon, 30 Jan 2017 20:40:48 -0600 Subject: [PATCH 3/6] Code review v2 --- api/v1/services/UserService.js | 2 +- test/user.js | 12 ++++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/api/v1/services/UserService.js b/api/v1/services/UserService.js index b976c16..4734f83 100644 --- a/api/v1/services/UserService.js +++ b/api/v1/services/UserService.js @@ -122,7 +122,7 @@ module.exports.resetPassword = function (user, password) { module.exports.deleteUser = function (user) { if(user.hasRoles(utils.roles.ALL, false)) { var message = "The provided user has a role and can not be deleted"; - var source = "UserRole"; + var source = null; throw new errors.InvalidParameterError(message, source); } diff --git a/test/user.js b/test/user.js index e4ca554..646355f 100644 --- a/test/user.js +++ b/test/user.js @@ -167,14 +167,11 @@ describe('UserService',function(){ before(function(done) { toDeleteUser = User.forge({ id: 1, email: 'to@delete.com'}); invalidToDeleteUser = User.forge({ id: 2, email: 'cant@delete.com'}); - invalidToDeleteUser.setPassword('password123') - .then(function(updatedUser) { - updatedUser.related('roles').add({ role: utils.roles.ATTENDEE}); + invalidToDeleteUser.related('roles').add({ role: utils.roles.ATTENDEE}); - _destroy = sinon.spy(User.prototype,'destroy'); + _destroy = sinon.spy(User.prototype,'destroy'); - done(); - }); + done(); }); beforeEach(function (done) { tracker.install(); @@ -186,8 +183,7 @@ describe('UserService',function(){ }); var deleteValid = UserService.deleteUser(toDeleteUser); - deleteValid - .then(function () { + deleteValid.then(function () { assert(_destroy.calledOnce, "User destroy not called"); done(); From 0c2e8b461df2b980b800659fb48c0eaa71b8391a Mon Sep 17 00:00:00 2001 From: Yash Sharma Date: Mon, 30 Jan 2017 20:44:34 -0600 Subject: [PATCH 4/6] Review 2.1 --- api/v1/services/UserService.js | 2 +- test/user.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/v1/services/UserService.js b/api/v1/services/UserService.js index 4734f83..9119bfa 100644 --- a/api/v1/services/UserService.js +++ b/api/v1/services/UserService.js @@ -123,7 +123,7 @@ module.exports.deleteUser = function (user) { if(user.hasRoles(utils.roles.ALL, false)) { var message = "The provided user has a role and can not be deleted"; var source = null; - throw new errors.InvalidParameterError(message, source); + return _Promise.reject(new errors.InvalidParameterError(message, source)); } return user.destroy(); diff --git a/test/user.js b/test/user.js index 646355f..51d6ed7 100644 --- a/test/user.js +++ b/test/user.js @@ -193,8 +193,8 @@ describe('UserService',function(){ }); }); it('throws an exception when deleting a user with a role', function(done) { - expect(() => UserService.deleteUser(invalidToDeleteUser)).to.throw(errors.InvalidParameterError); - done(); + var invalidDeletion = UserService.deleteUser(invalidToDeleteUser); + expect(invalidDeletion).to.eventually.be.rejectedWith(errors.InvalidParameterError).and.notify(done) }); afterEach(function (done) { tracker.uninstall(); From e90afada20924ec1dcced0d146e2a8251be7bfde Mon Sep 17 00:00:00 2001 From: Nick Magerko Date: Tue, 31 Jan 2017 17:46:55 -0600 Subject: [PATCH 5/6] Revert "Delete User (For Admins)" --- api/v1/controllers/EcosystemController.js | 2 +- api/v1/controllers/UserController.js | 19 --------- api/v1/services/UserService.js | 10 ----- test/user.js | 51 +---------------------- 4 files changed, 3 insertions(+), 79 deletions(-) diff --git a/api/v1/controllers/EcosystemController.js b/api/v1/controllers/EcosystemController.js index cb17723..1a6d6f4 100644 --- a/api/v1/controllers/EcosystemController.js +++ b/api/v1/controllers/EcosystemController.js @@ -50,7 +50,7 @@ function deleteEcosystem (req, res, next) { EcosystemService .deleteEcosystem(name) .then(function () { - res.body = {}; + res.body = {} next(); return null; diff --git a/api/v1/controllers/UserController.js b/api/v1/controllers/UserController.js index c8c6cdd..5fb4a18 100644 --- a/api/v1/controllers/UserController.js +++ b/api/v1/controllers/UserController.js @@ -100,24 +100,6 @@ function requestPasswordReset (req, res, next) { }); } -function deleteUser (req, res , next) { - var userEmail = req.body.email; - - services.UserService - .findUserByEmail(userEmail) - .then(function (user) { - return services.UserService.deleteUser(user); - }) - .then(function () { - res.body = {}; - - return next(); - }) - .catch(function (error){ - return next(error); - }); -} - router.use(bodyParser.json()); router.use(middleware.auth); @@ -126,7 +108,6 @@ router.post('/accredited', middleware.request(requests.AccreditedUserCreationReq middleware.permission(roles.ORGANIZERS), createAccreditedUser); router.post('/reset', middleware.request(requests.ResetTokenRequest), requestPasswordReset); router.get('/:id', middleware.permission(roles.ORGANIZERS, isRequester), getUser); -router.delete('/', middleware.permission(roles.SUPERUSER), deleteUser); router.use(middleware.response); router.use(middleware.errors); diff --git a/api/v1/services/UserService.js b/api/v1/services/UserService.js index 9119bfa..097e23c 100644 --- a/api/v1/services/UserService.js +++ b/api/v1/services/UserService.js @@ -118,13 +118,3 @@ module.exports.resetPassword = function (user, password) { return updated.save(); }); }; - -module.exports.deleteUser = function (user) { - if(user.hasRoles(utils.roles.ALL, false)) { - var message = "The provided user has a role and can not be deleted"; - var source = null; - return _Promise.reject(new errors.InvalidParameterError(message, source)); - } - - return user.destroy(); -}; \ No newline at end of file diff --git a/test/user.js b/test/user.js index 51d6ed7..f1de507 100644 --- a/test/user.js +++ b/test/user.js @@ -10,7 +10,6 @@ var User = require('../api/v1/models/User.js'); var assert = chai.assert; var expect = chai.expect; -var tracker = require('mock-knex').getTracker(); describe('UserService',function(){ describe('createUser', function () { @@ -158,51 +157,5 @@ describe('UserService',function(){ _save.restore(); done(); }); - }); - - describe('deleteUser', function() { - var toDeleteUser; - var invalidToDeleteUser; - var _destroy; - before(function(done) { - toDeleteUser = User.forge({ id: 1, email: 'to@delete.com'}); - invalidToDeleteUser = User.forge({ id: 2, email: 'cant@delete.com'}); - invalidToDeleteUser.related('roles').add({ role: utils.roles.ATTENDEE}); - - _destroy = sinon.spy(User.prototype,'destroy'); - - done(); - }); - beforeEach(function (done) { - tracker.install(); - done(); - }); - it('deletes a user with no role', function(done) { - tracker.on('query', function (query) { - query.response([]); - }); - - var deleteValid = UserService.deleteUser(toDeleteUser); - deleteValid.then(function () { - assert(_destroy.calledOnce, "User destroy not called"); - - done(); - }) - .catch(function (err) { - done(err); - }); - }); - it('throws an exception when deleting a user with a role', function(done) { - var invalidDeletion = UserService.deleteUser(invalidToDeleteUser); - expect(invalidDeletion).to.eventually.be.rejectedWith(errors.InvalidParameterError).and.notify(done) - }); - afterEach(function (done) { - tracker.uninstall(); - done(); - }); - after(function(done) { - _destroy.restore(); - done(); - }); - }); -}); \ No newline at end of file + }) +}); From a409afb110256127ce1f2e9d62638de50af9e517 Mon Sep 17 00:00:00 2001 From: Nick Magerko Date: Wed, 1 Feb 2017 16:41:33 -0600 Subject: [PATCH 6/6] fix: bad key in response --- api/v1/controllers/RegistrationController.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/v1/controllers/RegistrationController.js b/api/v1/controllers/RegistrationController.js index c8093df..7163524 100644 --- a/api/v1/controllers/RegistrationController.js +++ b/api/v1/controllers/RegistrationController.js @@ -167,7 +167,7 @@ function fetchAttendeeByUser(req, res, next) { .findAttendeeByUser(req.user, true) .then(function(attendee){ res.body = attendee.toJSON(); - delete res.body.data.reviewer; + delete res.body.reviewer; next(); return null; @@ -203,6 +203,7 @@ function updateAttendeeByUser(req, res, next) { }) .then(function(attendee){ res.body = attendee.toJSON(); + delete res.body.reviewer; next(); return null;