From 68a3b9f2aacb0225ae8b883b561b144bac339cbd Mon Sep 17 00:00:00 2001
From: Chocobozzz <florian.bigard@gmail.com>
Date: Tue, 9 Aug 2016 21:44:45 +0200
Subject: [PATCH] Server: delete user with the id and not the username

---
 server/controllers/api/v1/users.js     | 4 ++--
 server/middlewares/validators/users.js | 5 +++--
 server/models/user.js                  | 5 +++++
 server/tests/api/checkParams.js        | 8 ++++----
 server/tests/api/users.js              | 2 +-
 server/tests/utils/users.js            | 6 +++---
 6 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/server/controllers/api/v1/users.js b/server/controllers/api/v1/users.js
index 057dcaf8d..704df770c 100644
--- a/server/controllers/api/v1/users.js
+++ b/server/controllers/api/v1/users.js
@@ -34,7 +34,7 @@ router.put('/:id',
   updateUser
 )
 
-router.delete('/:username',
+router.delete('/:id',
   oAuth.authenticate,
   admin.ensureIsAdmin,
   validatorsUsers.usersRemove,
@@ -83,7 +83,7 @@ function listUsers (req, res, next) {
 function removeUser (req, res, next) {
   waterfall([
     function getUser (callback) {
-      User.loadByUsername(req.params.username, callback)
+      User.loadById(req.params.id, callback)
     },
 
     function getVideos (user, callback) {
diff --git a/server/middlewares/validators/users.js b/server/middlewares/validators/users.js
index 175d90bcb..e540ab0d1 100644
--- a/server/middlewares/validators/users.js
+++ b/server/middlewares/validators/users.js
@@ -25,12 +25,12 @@ function usersAdd (req, res, next) {
 }
 
 function usersRemove (req, res, next) {
-  req.checkParams('username', 'Should have a valid username').isUserUsernameValid()
+  req.checkParams('id', 'Should have a valid id').notEmpty().isMongoId()
 
   logger.debug('Checking usersRemove parameters', { parameters: req.params })
 
   checkErrors(req, res, function () {
-    User.loadByUsername(req.params.username, function (err, user) {
+    User.loadById(req.params.id, function (err, user) {
       if (err) {
         logger.error('Error in usersRemove request validator.', { error: err })
         return res.sendStatus(500)
@@ -44,6 +44,7 @@ function usersRemove (req, res, next) {
 }
 
 function usersUpdate (req, res, next) {
+  req.checkParams('id', 'Should have a valid id').notEmpty().isMongoId()
   // Add old password verification
   req.checkBody('password', 'Should have a valid password').isUserPasswordValid()
 
diff --git a/server/models/user.js b/server/models/user.js
index 0bbd638d4..351ffef86 100644
--- a/server/models/user.js
+++ b/server/models/user.js
@@ -21,6 +21,7 @@ UserSchema.methods = {
 UserSchema.statics = {
   getByUsernameAndPassword: getByUsernameAndPassword,
   list: list,
+  loadById: loadById,
   loadByUsername: loadByUsername
 }
 
@@ -36,6 +37,10 @@ function list (callback) {
   return this.find(callback)
 }
 
+function loadById (id, callback) {
+  return this.findById(id, callback)
+}
+
 function loadByUsername (username, callback) {
   return this.findOne({ username: username }, callback)
 }
diff --git a/server/tests/api/checkParams.js b/server/tests/api/checkParams.js
index 128b07c4a..882948fac 100644
--- a/server/tests/api/checkParams.js
+++ b/server/tests/api/checkParams.js
@@ -610,23 +610,23 @@ describe('Test parameters validator', function () {
     })
 
     describe('When removing an user', function () {
-      it('Should fail with an incorrect username', function (done) {
+      it('Should fail with an incorrect id', function (done) {
         request(server.url)
           .delete(path + 'bla-bla')
           .set('Authorization', 'Bearer ' + server.accessToken)
           .expect(400, done)
       })
 
-      it('Should return 404 with a non existing username', function (done) {
+      it('Should return 404 with a non existing id', function (done) {
         request(server.url)
-          .delete(path + 'qzzerg')
+          .delete(path + '579f982228c99c221d8092b8')
           .set('Authorization', 'Bearer ' + server.accessToken)
           .expect(404, done)
       })
 
       it('Should success with the correct parameters', function (done) {
         request(server.url)
-          .delete(path + 'user1')
+          .delete(path + userId)
           .set('Authorization', 'Bearer ' + server.accessToken)
           .expect(204, done)
       })
diff --git a/server/tests/api/users.js b/server/tests/api/users.js
index 6f9eef181..a2557d2ab 100644
--- a/server/tests/api/users.js
+++ b/server/tests/api/users.js
@@ -235,7 +235,7 @@ describe('Test users', function () {
   })
 
   it('Should be able to remove this user', function (done) {
-    usersUtils.removeUser(server.url, accessToken, 'user_1', done)
+    usersUtils.removeUser(server.url, userId, accessToken, done)
   })
 
   it('Should not be able to login with this user', function (done) {
diff --git a/server/tests/utils/users.js b/server/tests/utils/users.js
index ed7a9d672..3b560e409 100644
--- a/server/tests/utils/users.js
+++ b/server/tests/utils/users.js
@@ -52,7 +52,7 @@ function getUsersList (url, end) {
     .end(end)
 }
 
-function removeUser (url, token, username, expectedStatus, end) {
+function removeUser (url, userId, accessToken, expectedStatus, end) {
   if (!end) {
     end = expectedStatus
     expectedStatus = 204
@@ -61,9 +61,9 @@ function removeUser (url, token, username, expectedStatus, end) {
   const path = '/api/v1/users'
 
   request(url)
-    .delete(path + '/' + username)
+    .delete(path + '/' + userId)
     .set('Accept', 'application/json')
-    .set('Authorization', 'Bearer ' + token)
+    .set('Authorization', 'Bearer ' + accessToken)
     .expect(expectedStatus)
     .end(end)
 }