Moderators can only manage users

client/src/app/+admin/users/user-edit/user-create.component.ts

@@ -1,6 +1,6 @@
 import { Component, OnInit } from '@angular/core'
 import { Router } from '@angular/router'
-import { Notifier, ServerService } from '@app/core'
+import { AuthService, Notifier, ServerService } from '@app/core'
 import { UserCreate, UserRole } from '../../../../../../shared'
 import { UserEdit } from './user-edit'
 import { I18n } from '@ngx-translate/i18n-polyfill'
@@ -8,7 +8,6 @@ import { FormValidatorService } from '@app/shared/forms/form-validators/form-val
 import { UserValidatorsService } from '@app/shared/forms/form-validators/user-validators.service'
 import { ConfigService } from '@app/+admin/config/shared/config.service'
 import { UserService } from '@app/shared'
-import { UserAdminFlag } from '@shared/models/users/user-flag.model'
 
 @Component({
   selector: 'my-user-create',
@@ -22,6 +21,7 @@ export class UserCreateComponent extends UserEdit implements OnInit {
     protected serverService: ServerService,
     protected formValidatorService: FormValidatorService,
     protected configService: ConfigService,
+    protected auth: AuthService,
     private userValidatorsService: UserValidatorsService,
     private router: Router,
     private notifier: Notifier,

client/src/app/+admin/users/user-edit/user-edit.component.html

@@ -41,7 +41,7 @@
     <label i18n for="role">Role</label>
     <div class="peertube-select-container">
       <select id="role" formControlName="role">
-        <option *ngFor="let role of roles" [value]="role.value">
+        <option *ngFor="let role of getRoles()" [value]="role.value">
           {{ role.label }}
         </option>
       </select>

client/src/app/+admin/users/user-edit/user-edit.ts

@@ -1,22 +1,34 @@
-import { ServerService } from '../../../core'
+import { AuthService, ServerService } from '../../../core'
 import { FormReactive } from '../../../shared'
-import { USER_ROLE_LABELS, VideoResolution } from '../../../../../../shared'
+import { USER_ROLE_LABELS, UserRole, VideoResolution } from '../../../../../../shared'
 import { ConfigService } from '@app/+admin/config/shared/config.service'
 import { UserAdminFlag } from '@shared/models/users/user-flag.model'
 
 export abstract class UserEdit extends FormReactive {
   videoQuotaOptions: { value: string, label: string }[] = []
   videoQuotaDailyOptions: { value: string, label: string }[] = []
-  roles = Object.keys(USER_ROLE_LABELS)
-                .map(key => ({ value: key.toString(), label: USER_ROLE_LABELS[key] }))
   username: string
   userId: number
 
   protected abstract serverService: ServerService
   protected abstract configService: ConfigService
+  protected abstract auth: AuthService
   abstract isCreation (): boolean
   abstract getFormButtonTitle (): string
 
+  getRoles () {
+    const authUser = this.auth.getUser()
+
+    if (authUser.role === UserRole.ADMINISTRATOR) {
+      return Object.keys(USER_ROLE_LABELS)
+            .map(key => ({ value: key.toString(), label: USER_ROLE_LABELS[key] }))
+    }
+
+    return [
+      { value: UserRole.USER.toString(), label: USER_ROLE_LABELS[UserRole.USER] }
+    ]
+  }
+
   isTranscodingInformationDisplayed () {
     const formVideoQuota = parseInt(this.form.value['videoQuota'], 10)
 

client/src/app/+admin/users/user-edit/user-update.component.ts

@@ -1,7 +1,7 @@
 import { Component, OnDestroy, OnInit } from '@angular/core'
 import { ActivatedRoute, Router } from '@angular/router'
 import { Subscription } from 'rxjs'
-import { Notifier } from '@app/core'
+import { AuthService, Notifier } from '@app/core'
 import { ServerService } from '../../../core'
 import { UserEdit } from './user-edit'
 import { User, UserUpdate } from '../../../../../../shared'
@@ -29,6 +29,7 @@ export class UserUpdateComponent extends UserEdit implements OnInit, OnDestroy {
     protected formValidatorService: FormValidatorService,
     protected serverService: ServerService,
     protected configService: ConfigService,
+    protected auth: AuthService,
     private userValidatorsService: UserValidatorsService,
     private route: ActivatedRoute,
     private router: Router,

client/src/app/+admin/users/user-list/user-list.component.ts

@@ -1,5 +1,5 @@
 import { Component, OnInit, ViewChild } from '@angular/core'
-import { Notifier } from '@app/core'
+import { AuthService, Notifier } from '@app/core'
 import { SortMeta } from 'primeng/components/common/sortmeta'
 import { ConfirmService, ServerService } from '../../../core'
 import { RestPagination, RestTable, UserService } from '../../../shared'
@@ -30,11 +30,16 @@ export class UserListComponent extends RestTable implements OnInit {
     private confirmService: ConfirmService,
     private serverService: ServerService,
     private userService: UserService,
+    private auth: AuthService,
     private i18n: I18n
   ) {
     super()
   }
 
+  get authUser () {
+    return this.auth.getUser()
+  }
+
   get requiresEmailVerification () {
     return this.serverService.getConfig().signup.requiresEmailVerification
   }
@@ -45,22 +50,26 @@ export class UserListComponent extends RestTable implements OnInit {
     this.bulkUserActions = [
       {
         label: this.i18n('Delete'),
-        handler: users => this.removeUsers(users)
+        handler: users => this.removeUsers(users),
+        isDisplayed: users => users.every(u => this.authUser.canManage(u))
       },
       {
         label: this.i18n('Ban'),
         handler: users => this.openBanUserModal(users),
-        isDisplayed: users => users.every(u => u.blocked === false)
+        isDisplayed: users => users.every(u => this.authUser.canManage(u) && u.blocked === false)
       },
       {
         label: this.i18n('Unban'),
         handler: users => this.unbanUsers(users),
-        isDisplayed: users => users.every(u => u.blocked === true)
+        isDisplayed: users => users.every(u => this.authUser.canManage(u) && u.blocked === true)
       },
       {
         label: this.i18n('Set Email as Verified'),
         handler: users => this.setEmailsAsVerified(users),
-        isDisplayed: users => this.requiresEmailVerification && users.every(u => !u.blocked && u.emailVerified === false)
+        isDisplayed: users => {
+          return this.requiresEmailVerification &&
+            users.every(u => this.authUser.canManage(u) && !u.blocked && u.emailVerified === false)
+        }
       }
     ]
   }

client/src/app/core/auth/auth-user.model.ts

@@ -139,6 +139,15 @@ export class AuthUser extends User {
     return hasUserRight(this.role, right)
   }
 
+  canManage (user: ServerUserModel) {
+    const myRole = this.role
+
+    if (myRole === UserRole.ADMINISTRATOR) return true
+
+    // I'm a moderator: I can only manage users
+    return user.role === UserRole.USER
+  }
+
   save () {
     peertubeLocalStorage.setItem(AuthUser.KEYS.ID, this.id.toString())
     peertubeLocalStorage.setItem(AuthUser.KEYS.USERNAME, this.username)

client/src/app/shared/moderation/user-moderation-dropdown.component.ts

@@ -33,6 +33,7 @@ export class UserModerationDropdownComponent implements OnChanges {
     private serverService: ServerService,
     private userService: UserService,
     private blocklistService: BlocklistService,
+    private auth: AuthService,
     private i18n: I18n
   ) { }
 
@@ -230,7 +231,7 @@ export class UserModerationDropdownComponent implements OnChanges {
 
       if (this.user && authUser.id === this.user.id) return
 
-      if (this.user && authUser.hasRight(UserRight.MANAGE_USERS)) {
+      if (this.user && authUser.hasRight(UserRight.MANAGE_USERS) && authUser.canManage(this.user)) {
         this.userActions.push([
           {
             label: this.i18n('Edit'),

server/controllers/api/users/index.ts

@@ -31,7 +31,8 @@ import {
   usersAskSendVerifyEmailValidator,
   usersBlockingValidator,
   usersResetPasswordValidator,
-  usersVerifyEmailValidator
+  usersVerifyEmailValidator,
+  ensureCanManageUser
 } from '../../../middlewares/validators'
 import { UserModel } from '../../../models/account/user'
 import { auditLoggerFactory, getAuditIdFromRes, UserAuditView } from '../../../helpers/audit-logger'
@@ -97,12 +98,14 @@ usersRouter.post('/:id/block',
   authenticate,
   ensureUserHasRight(UserRight.MANAGE_USERS),
   asyncMiddleware(usersBlockingValidator),
+  ensureCanManageUser,
   asyncMiddleware(blockUser)
 )
 usersRouter.post('/:id/unblock',
   authenticate,
   ensureUserHasRight(UserRight.MANAGE_USERS),
   asyncMiddleware(usersBlockingValidator),
+  ensureCanManageUser,
   asyncMiddleware(unblockUser)
 )
 
@@ -132,6 +135,7 @@ usersRouter.put('/:id',
   authenticate,
   ensureUserHasRight(UserRight.MANAGE_USERS),
   asyncMiddleware(usersUpdateValidator),
+  ensureCanManageUser,
   asyncMiddleware(updateUser)
 )
 
@@ -139,6 +143,7 @@ usersRouter.delete('/:id',
   authenticate,
   ensureUserHasRight(UserRight.MANAGE_USERS),
   asyncMiddleware(usersRemoveValidator),
+  ensureCanManageUser,
   asyncMiddleware(removeUser)
 )
 

server/middlewares/validators/users.ts

@@ -30,6 +30,7 @@ import { UserRegister } from '../../../shared/models/users/user-register.model'
 import { isThemeNameValid } from '../../helpers/custom-validators/plugins'
 import { isThemeRegistered } from '../../lib/plugins/theme-utils'
 import { doesVideoExist } from '../../helpers/middlewares'
+import { UserRole } from '../../../shared/models/users'
 
 const usersAddValidator = [
   body('username').custom(isUserUsernameValid).withMessage('Should have a valid username (lowercase alphanumeric characters)'),
@@ -46,6 +47,12 @@ const usersAddValidator = [
     if (areValidationErrors(req, res)) return
     if (!await checkUserNameOrEmailDoesNotAlreadyExist(req.body.username, req.body.email, res)) return
 
+    const authUser = res.locals.oauth.token.User
+    if (authUser.role !== UserRole.ADMINISTRATOR && req.body.role !== UserRole.USER) {
+      return res.status(403)
+        .json({ error: 'You can only create users (and not administrators or moderators' })
+    }
+
     return next()
   }
 ]
@@ -75,21 +82,18 @@ const usersRegisterValidator = [
     if (body.channel) {
       if (!body.channel.name || !body.channel.displayName) {
         return res.status(400)
-          .send({ error: 'Channel is optional but if you specify it, channel.name and channel.displayName are required.' })
+          .json({ error: 'Channel is optional but if you specify it, channel.name and channel.displayName are required.' })
-          .end()
       }
 
       if (body.channel.name === body.username) {
         return res.status(400)
-                  .send({ error: 'Channel name cannot be the same than user username.' })
+                  .json({ error: 'Channel name cannot be the same than user username.' })
-                  .end()
       }
 
       const existing = await ActorModel.loadLocalByName(body.channel.name)
       if (existing) {
         return res.status(409)
-                  .send({ error: `Channel with name ${body.channel.name} already exists.` })
+                  .json({ error: `Channel with name ${body.channel.name} already exists.` })
-                  .end()
       }
     }
 
@@ -109,8 +113,7 @@ const usersRemoveValidator = [
     const user = res.locals.user
     if (user.username === 'root') {
       return res.status(400)
-                .send({ error: 'Cannot remove the root user' })
+                .json({ error: 'Cannot remove the root user' })
-                .end()
     }
 
     return next()
@@ -130,8 +133,7 @@ const usersBlockingValidator = [
     const user = res.locals.user
     if (user.username === 'root') {
       return res.status(400)
-                .send({ error: 'Cannot block the root user' })
+                .json({ error: 'Cannot block the root user' })
-                .end()
     }
 
     return next()
@@ -143,7 +145,7 @@ const deleteMeValidator = [
     const user = res.locals.oauth.token.User
     if (user.username === 'root') {
       return res.status(400)
-                .send({ error: 'You cannot delete your root account.' })
+                .json({ error: 'You cannot delete your root account.' })
                 .end()
     }
 
@@ -170,8 +172,7 @@ const usersUpdateValidator = [
     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.' })
+        .json({ error: 'Cannot change root role.' })
-        .end()
     }
 
     return next()
@@ -216,15 +217,14 @@ const usersUpdateMeValidator = [
     if (req.body.password || req.body.email) {
       if (!req.body.currentPassword) {
         return res.status(400)
-                  .send({ error: 'currentPassword parameter is missing.' })
+                  .json({ error: 'currentPassword parameter is missing.' })
                   .end()
       }
 
       const user = res.locals.oauth.token.User
       if (await user.isPasswordMatch(req.body.currentPassword) !== true) {
         return res.status(401)
-                  .send({ error: 'currentPassword is invalid.' })
+                  .json({ error: 'currentPassword is invalid.' })
-                  .end()
       }
     }
 
@@ -265,8 +265,7 @@ const ensureUserRegistrationAllowed = [
     const allowed = await isSignupAllowed()
     if (allowed === false) {
       return res.status(403)
-                .send({ error: 'User registration is not enabled or user limit is reached.' })
+                .json({ error: 'User registration is not enabled or user limit is reached.' })
-                .end()
     }
 
     return next()
@@ -279,8 +278,7 @@ const ensureUserRegistrationAllowedForIP = [
 
     if (allowed === false) {
       return res.status(403)
-                .send({ error: 'You are not on a network authorized for registration.' })
+                .json({ error: 'You are not on a network authorized for registration.' })
-                .end()
     }
 
     return next()
@@ -323,8 +321,7 @@ const usersResetPasswordValidator = [
     if (redisVerificationString !== req.body.verificationString) {
       return res
         .status(403)
-        .send({ error: 'Invalid verification string.' })
+        .json({ error: 'Invalid verification string.' })
-        .end()
     }
 
     return next()
@@ -371,8 +368,7 @@ const usersVerifyEmailValidator = [
     if (redisVerificationString !== req.body.verificationString) {
       return res
         .status(403)
-        .send({ error: 'Invalid verification string.' })
+        .json({ error: 'Invalid verification string.' })
-        .end()
     }
 
     return next()
@@ -389,14 +385,26 @@ const ensureAuthUserOwnsAccountValidator = [
 
     if (res.locals.account.id !== user.Account.id) {
       return res.status(403)
-                .send({ error: 'Only owner can access ratings list.' })
+                .json({ error: 'Only owner can access ratings list.' })
-                .end()
     }
 
     return next()
   }
 ]
 
+const ensureCanManageUser = [
+  (req: express.Request, res: express.Response, next: express.NextFunction) => {
+    const authUser = res.locals.oauth.token.User
+    const onUser = res.locals.user
+
+    if (authUser.role === UserRole.ADMINISTRATOR) return next()
+    if (authUser.role === UserRole.MODERATOR && onUser.role === UserRole.USER) return next()
+
+    return res.status(403)
+      .json({ error: 'A moderator can only manager users.' })
+  }
+]
+
 // ---------------------------------------------------------------------------
 
 export {
@@ -416,7 +424,8 @@ export {
   usersAskSendVerifyEmailValidator,
   usersVerifyEmailValidator,
   userAutocompleteValidator,
-  ensureAuthUserOwnsAccountValidator
+  ensureAuthUserOwnsAccountValidator,
+  ensureCanManageUser
 }
 
 // ---------------------------------------------------------------------------
@@ -434,16 +443,14 @@ async function checkUserNameOrEmailDoesNotAlreadyExist (username: string, email:
 
   if (user) {
     res.status(409)
-              .send({ error: 'User with this username or email already exists.' })
+              .json({ error: 'User with this username or email already exists.' })
-              .end()
     return false
   }
 
   const actor = await ActorModel.loadLocalByName(username)
   if (actor) {
     res.status(409)
-       .send({ error: 'Another actor (account/channel) with this name on this instance already exists or has already existed.' })
+       .json({ error: 'Another actor (account/channel) with this name on this instance already exists or has already existed.' })
-       .end()
     return false
   }
 
@@ -456,8 +463,7 @@ async function checkUserExist (finder: () => Bluebird<UserModel>, res: express.R
   if (!user) {
     if (abortResponse === true) {
       res.status(404)
-        .send({ error: 'User not found' })
+        .json({ error: 'User not found' })
-        .end()
     }
 
     return false

server/tests/api/check-params/users.ts

@@ -3,7 +3,7 @@
 import { omit } from 'lodash'
 import 'mocha'
 import { join } from 'path'
-import { UserRole, VideoImport, VideoImportState } from '../../../../shared'
+import { User, UserRole, VideoImport, VideoImportState } from '../../../../shared'
 
 import {
   addVideoChannel,
@@ -44,35 +44,79 @@ describe('Test users API validators', function () {
   const path = '/api/v1/users/'
   let userId: number
   let rootId: number
+  let moderatorId: number
   let videoId: number
   let server: ServerInfo
   let serverWithRegistrationDisabled: ServerInfo
   let userAccessToken = ''
+  let moderatorAccessToken = ''
   let channelId: number
-  const user = {
-    username: 'user1',
-    password: 'my super password'
-  }
 
   // ---------------------------------------------------------------
 
   before(async function () {
     this.timeout(30000)
 
-    server = await flushAndRunServer(1)
+    {
-    serverWithRegistrationDisabled = await flushAndRunServer(2)
+      const res = await Promise.all([
+        flushAndRunServer(1, { signup: { limit: 7 } }),
+        flushAndRunServer(2)
+      ])
 
-    await setAccessTokensToServers([ server ])
+      server = res[0]
+      serverWithRegistrationDisabled = res[1]
 
-    const videoQuota = 42000000
+      await setAccessTokensToServers([ server ])
-    await createUser({
+    }
-      url: server.url,
+
-      accessToken: server.accessToken,
+    {
-      username: user.username,
+      const user = {
-      password: user.password,
+        username: 'user1',
-      videoQuota: videoQuota
+        password: 'my super password'
-    })
+      }
-    userAccessToken = await userLogin(server, user)
+
+      const videoQuota = 42000000
+      await createUser({
+        url: server.url,
+        accessToken: server.accessToken,
+        username: user.username,
+        password: user.password,
+        videoQuota: videoQuota
+      })
+      userAccessToken = await userLogin(server, user)
+    }
+
+    {
+      const moderator = {
+        username: 'moderator1',
+        password: 'super password'
+      }
+
+      await createUser({
+        url: server.url,
+        accessToken: server.accessToken,
+        username: moderator.username,
+        password: moderator.password,
+        role: UserRole.MODERATOR
+      })
+
+      moderatorAccessToken = await userLogin(server, moderator)
+    }
+
+    {
+      const moderator = {
+        username: 'moderator2',
+        password: 'super password'
+      }
+
+      await createUser({
+        url: server.url,
+        accessToken: server.accessToken,
+        username: moderator.username,
+        password: moderator.password,
+        role: UserRole.MODERATOR
+      })
+    }
 
     {
       const res = await getMyUserInformation(server.url, server.accessToken)
@@ -83,6 +127,15 @@ describe('Test users API validators', function () {
       const res = await uploadVideo(server.url, server.accessToken, {})
       videoId = res.body.video.id
     }
+
+    {
+      const res = await getUsersList(server.url, server.accessToken)
+      const users: User[] = res.body.data
+
+      userId = users.find(u => u.username === 'user1').id
+      rootId = users.find(u => u.username === 'root').id
+      moderatorId = users.find(u => u.username === 'moderator2').id
+    }
   })
 
   describe('When listing users', function () {
@@ -251,6 +304,32 @@ describe('Test users API validators', function () {
       })
     })
 
+    it('Should fail to create a moderator or an admin with a moderator', async function () {
+      for (const role of [ UserRole.MODERATOR, UserRole.ADMINISTRATOR ]) {
+        const fields = immutableAssign(baseCorrectParams, { role })
+
+        await makePostBodyRequest({
+          url: server.url,
+          path,
+          token: moderatorAccessToken,
+          fields,
+          statusCodeExpected: 403
+        })
+      }
+    })
+
+    it('Should succeed to create a user with a moderator', async function () {
+      const fields = immutableAssign(baseCorrectParams, { username: 'a4656', email: 'a4656@example.com', role: UserRole.USER })
+
+      await makePostBodyRequest({
+        url: server.url,
+        path,
+        token: moderatorAccessToken,
+        fields,
+        statusCodeExpected: 200
+      })
+    })
+
     it('Should succeed with the correct params', async function () {
       await makePostBodyRequest({
         url: server.url,
@@ -468,11 +547,6 @@ describe('Test users API validators', function () {
   })
 
   describe('When getting a user', function () {
-    before(async function () {
-      const res = await getUsersList(server.url, server.accessToken)
-
-      userId = res.body.data[1].id
-    })
 
     it('Should fail with an non authenticated user', async function () {
       await makeGetRequest({ url: server.url, path: path + userId, token: 'super token', statusCodeExpected: 401 })
@@ -489,13 +563,6 @@ describe('Test users API validators', function () {
 
   describe('When updating a user', function () {
 
-    before(async function () {
-      const res = await getUsersList(server.url, server.accessToken)
-
-      userId = res.body.data[1].id
-      rootId = res.body.data[2].id
-    })
-
     it('Should fail with an invalid email attribute', async function () {
       const fields = {
         email: 'blabla'
@@ -565,7 +632,35 @@ describe('Test users API validators', function () {
     it('Should fail with invalid admin flags', async function () {
       const fields = { adminFlags: 'toto' }
 
-      await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields })
+      await makePutBodyRequest({ url: server.url, path, token: server.accessToken, fields })
+    })
+
+    it('Should fail to update an admin with a moderator', async function () {
+      const fields = {
+        videoQuota: 42
+      }
+
+      await makePutBodyRequest({
+        url: server.url,
+        path: path + moderatorId,
+        token: moderatorAccessToken,
+        fields,
+        statusCodeExpected: 403
+      })
+    })
+
+    it('Should succeed to update a user with a moderator', async function () {
+      const fields = {
+        videoQuota: 42
+      }
+
+      await makePutBodyRequest({
+        url: server.url,
+        path: path + userId,
+        token: moderatorAccessToken,
+        fields,
+        statusCodeExpected: 204
+      })
     })
 
     it('Should succeed with the correct params', async function () {
@@ -664,6 +759,17 @@ describe('Test users API validators', function () {
       await blockUser(server.url, userId, userAccessToken, 403)
       await unblockUser(server.url, userId, userAccessToken, 403)
     })
+
+    it('Should fail on a moderator with a moderator', async function () {
+      await removeUser(server.url, moderatorId, moderatorAccessToken, 403)
+      await blockUser(server.url, moderatorId, moderatorAccessToken, 403)
+      await unblockUser(server.url, moderatorId, moderatorAccessToken, 403)
+    })
+
+    it('Should succeed on a user with a moderator', async function () {
+      await blockUser(server.url, userId, moderatorAccessToken)
+      await unblockUser(server.url, userId, moderatorAccessToken)
+    })
   })
 
   describe('When deleting our account', function () {