Destroy user token when changing its role

6 files changed, 59 insertions, 23 deletions

diff --git a/server/controllers/api/users.ts b/server/controllers/api/users.ts
index aced4639e..79bb2665d 100644
--- a/server/controllers/api/users.ts
+++ b/server/controllers/api/users.ts
@@ -19,6 +19,7 @@ import {
 import { usersUpdateMyAvatarValidator, videosSortValidator } from '../../middlewares/validators'
 import { AccountVideoRateModel } from '../../models/account/account-video-rate'
 import { UserModel } from '../../models/account/user'
+import { OAuthTokenModel } from '../../models/oauth/oauth-token'
 import { VideoModel } from '../../models/video/video'
 
 const reqAvatarFile = createReqFiles('avatarfile', CONFIG.STORAGE.AVATARS_DIR, AVATAR_MIMETYPE_EXT)
@@ -288,6 +289,7 @@ async function updateMyAvatar (req: express.Request, res: express.Response, next
 async function updateUser (req: express.Request, res: express.Response, next: express.NextFunction) {
   const body: UserUpdate = req.body
   const user = res.locals.user as UserModel
+  const roleChanged = body.role !== undefined && body.role !== user.role
 
   if (body.email !== undefined) user.email = body.email
   if (body.videoQuota !== undefined) user.videoQuota = body.videoQuota
@@ -295,6 +297,11 @@ async function updateUser (req: express.Request, res: express.Response, next: ex
 
   await user.save()
 
+  // Destroy user token to refresh rights
+  if (roleChanged) {
+    await OAuthTokenModel.deleteUserToken(user.id)
+  }
+
   // Don't need to send this update to followers, these attributes are not propagated
 
   return res.sendStatus(204)
diff --git a/server/middlewares/validators/users.ts b/server/middlewares/validators/users.ts
index d22a745b4..990311d6f 100644
--- a/server/middlewares/validators/users.ts
+++ b/server/middlewares/validators/users.ts
@@ -77,6 +77,13 @@ const usersUpdateValidator = [
     if (areValidationErrors(req, res)) return
     if (!await checkUserIdExist(req.params.id, res)) return
 
+    const user = res.locals.user
+    if (user.username === 'root' && req.body.role !== undefined && user.role !== req.body.role) {
+      return res.status(400)
+        .send({ error: 'Cannot change root role.' })
+        .end()
+    }
+
     return next()
   }
 ]
diff --git a/server/models/oauth/oauth-token.ts b/server/models/oauth/oauth-token.ts
index 9d1b63813..528bb9587 100644
--- a/server/models/oauth/oauth-token.ts
+++ b/server/models/oauth/oauth-token.ts
@@ -159,4 +159,14 @@ export class OAuthTokenModel extends Model<OAuthTokenModel> {
         return token
       })
   }
+
+  static deleteUserToken (userId: number) {
+    const query = {
+      where: {
+        userId
+      }
+    }
+
+    return OAuthTokenModel.destroy(query)
+  }
 }
diff --git a/server/tests/api/check-params/users.ts b/server/tests/api/check-params/users.ts
index b0f35b9f7..9938fe3a2 100644
--- a/server/tests/api/check-params/users.ts
+++ b/server/tests/api/check-params/users.ts
@@ -20,6 +20,10 @@ describe('Test users API validators', function () {
   let server: ServerInfo
   let serverWithRegistrationDisabled: ServerInfo
   let userAccessToken = ''
+  const user = {
+    username: 'user1',
+    password: 'my super password'
+  }
 
   // ---------------------------------------------------------------
 
@@ -33,10 +37,6 @@ describe('Test users API validators', function () {
 
     await setAccessTokensToServers([ server ])
 
-    const user = {
-      username: 'user1',
-      password: 'my super password'
-    }
     const videoQuota = 42000000
     await createUser(server.url, server.accessToken, user.username, user.password, videoQuota)
     userAccessToken = await userLogin(server, user)
@@ -341,6 +341,14 @@ describe('Test users API validators', function () {
       await makePutBodyRequest({ url: server.url, path: path + userId, token: 'super token', fields, statusCodeExpected: 401 })
     })
 
+    it('Should fail when updating root role', async function () {
+      const fields = {
+        role: UserRole.MODERATOR
+      }
+
+      await makePutBodyRequest({ url: server.url, path: path + rootId, token: server.accessToken, fields })
+    })
+
     it('Should succeed with the correct params', async function () {
       const fields = {
         email: 'email@example.com',
@@ -349,6 +357,7 @@ describe('Test users API validators', function () {
       }
 
       await makePutBodyRequest({ url: server.url, path: path + userId, token: server.accessToken, fields, statusCodeExpected: 204 })
+      userAccessToken = await userLogin(server, user)
     })
   })
 
diff --git a/server/tests/api/users/users.ts b/server/tests/api/users/users.ts
index 6bb5fd698..c23b58089 100644
--- a/server/tests/api/users/users.ts
+++ b/server/tests/api/users/users.ts
@@ -4,10 +4,9 @@ import * as chai from 'chai'
 import 'mocha'
 import { UserRole } from '../../../../shared/index'
 import {
-  createUser, flushTests, getBlacklistedVideosList, getMyUserInformation, getMyUserVideoQuotaUsed, getMyUserVideoRating, getUserInformation,
-  getUsersList,
-  getUsersListPaginationAndSort, getVideosList, killallServers, login, makePutBodyRequest, rateVideo, registerUser, removeUser, removeVideo,
-  runServer, ServerInfo, serverLogin, testImage, updateMyAvatar, updateMyUser, updateUser, uploadVideo
+  createUser, flushTests, getBlacklistedVideosList, getMyUserInformation, getMyUserVideoQuotaUsed, getMyUserVideoRating,
+  getUserInformation, getUsersList, getUsersListPaginationAndSort, getVideosList, killallServers, login, makePutBodyRequest, rateVideo,
+  registerUser, removeUser, removeVideo, runServer, ServerInfo, testImage, updateMyAvatar, updateMyUser, updateUser, uploadVideo, userLogin
 } from '../../utils/index'
 import { follow } from '../../utils/server/follows'
 import { setAccessTokensToServers } from '../../utils/users/login'
@@ -21,6 +20,10 @@ describe('Test users', function () {
   let accessTokenUser: string
   let videoId: number
   let userId: number
+  const user = {
+    username: 'user_1',
+    password: 'super password'
+  }
 
   before(async function () {
     this.timeout(30000)
@@ -152,16 +155,11 @@ describe('Test users', function () {
   it('Should be able to upload a video again')
 
   it('Should be able to create a new user', async function () {
-    await createUser(server.url, accessToken, 'user_1', 'super password', 2 * 1024 * 1024)
+    await createUser(server.url, accessToken, user.username,user.password, 2 * 1024 * 1024)
   })
 
   it('Should be able to login with this user', async function () {
-    server.user = {
-      username: 'user_1',
-      password: 'super password'
-    }
-
-    accessTokenUser = await serverLogin(server)
+    accessTokenUser = await userLogin(server, user)
   })
 
   it('Should be able to get the user information', async function () {
@@ -297,9 +295,9 @@ describe('Test users', function () {
       accessToken: accessTokenUser,
       newPassword: 'new password'
     })
-    server.user.password = 'new password'
+    user.password = 'new password'
 
-    await login(server.url, server.client, server.user, 200)
+    await userLogin(server, user, 200)
   })
 
   it('Should be able to change the NSFW display attribute', async function () {
@@ -386,6 +384,12 @@ describe('Test users', function () {
     expect(user.id).to.be.a('number')
   })
 
+  it('Should have removed the user token', async function () {
+    await getMyUserVideoQuotaUsed(server.url, accessTokenUser, 401)
+
+    accessTokenUser = await userLogin(server, user)
+  })
+
   it('Should not be able to delete a user by a moderator', async function () {
     await removeUser(server.url, 2, accessTokenUser, 403)
   })
@@ -399,8 +403,7 @@ describe('Test users', function () {
   })
 
   it('Should not be able to login with this user', async function () {
-    // server.user is already set to user 1
-    await login(server.url, server.client, server.user, 400)
+    await userLogin(server, user, 400)
   })
 
   it('Should not have videos of this user', async function () {
@@ -417,12 +420,12 @@ describe('Test users', function () {
   })
 
   it('Should be able to login with this registered user', async function () {
-    server.user = {
+    const user15 = {
       username: 'user_15',
       password: 'my super password'
     }
 
-    accessToken = await serverLogin(server)
+    accessToken = await userLogin(server, user15)
   })
 
   it('Should have the correct video quota', async function () {
diff --git a/server/tests/utils/users/login.ts b/server/tests/utils/users/login.ts
index 04444e2f1..338ae1c00 100644
--- a/server/tests/utils/users/login.ts
+++ b/server/tests/utils/users/login.ts
@@ -32,8 +32,8 @@ async function serverLogin (server: Server) {
   return res.body.access_token as string
 }
 
-async function userLogin (server: Server, user: User) {
-  const res = await login(server.url, server.client, user, 200)
+async function userLogin (server: Server, user: User, expectedStatus = 200) {
+  const res = await login(server.url, server.client, user, expectedStatus)
 
   return res.body.access_token as string
 }