From a95a4cc89155f448e6f9ca0957170f3c72a9d964 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 30 Jul 2019 09:59:19 +0200 Subject: [PATCH] Moderators can only manage users --- .../users/user-edit/user-create.component.ts | 4 +- .../users/user-edit/user-edit.component.html | 2 +- .../app/+admin/users/user-edit/user-edit.ts | 20 ++- .../users/user-edit/user-update.component.ts | 3 +- .../users/user-list/user-list.component.ts | 19 +- client/src/app/core/auth/auth-user.model.ts | 9 + .../user-moderation-dropdown.component.ts | 3 +- server/controllers/api/users/index.ts | 7 +- server/middlewares/validators/users.ts | 72 ++++---- server/tests/api/check-params/users.ts | 166 ++++++++++++++---- 10 files changed, 227 insertions(+), 78 deletions(-) diff --git a/client/src/app/+admin/users/user-edit/user-create.component.ts b/client/src/app/+admin/users/user-edit/user-create.component.ts index 9a6801806..3b57a49c6 100644 --- a/client/src/app/+admin/users/user-edit/user-create.component.ts +++ b/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, diff --git a/client/src/app/+admin/users/user-edit/user-edit.component.html b/client/src/app/+admin/users/user-edit/user-edit.component.html index 400bac5d4..cb0f36f05 100644 --- a/client/src/app/+admin/users/user-edit/user-edit.component.html +++ b/client/src/app/+admin/users/user-edit/user-edit.component.html @@ -41,7 +41,7 @@
diff --git a/client/src/app/+admin/users/user-edit/user-edit.ts b/client/src/app/+admin/users/user-edit/user-edit.ts index ee6d2c489..6625d65d6 100644 --- a/client/src/app/+admin/users/user-edit/user-edit.ts +++ b/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) diff --git a/client/src/app/+admin/users/user-edit/user-update.component.ts b/client/src/app/+admin/users/user-edit/user-update.component.ts index 04b2935f4..c7052a925 100644 --- a/client/src/app/+admin/users/user-edit/user-update.component.ts +++ b/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, diff --git a/client/src/app/+admin/users/user-list/user-list.component.ts b/client/src/app/+admin/users/user-list/user-list.component.ts index c9c790689..ab82713b2 100644 --- a/client/src/app/+admin/users/user-list/user-list.component.ts +++ b/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) + } } ] } diff --git a/client/src/app/core/auth/auth-user.model.ts b/client/src/app/core/auth/auth-user.model.ts index abb11fdc2..334ede0cd 100644 --- a/client/src/app/core/auth/auth-user.model.ts +++ b/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) diff --git a/client/src/app/shared/moderation/user-moderation-dropdown.component.ts b/client/src/app/shared/moderation/user-moderation-dropdown.component.ts index 24f717821..e9d4c1437 100644 --- a/client/src/app/shared/moderation/user-moderation-dropdown.component.ts +++ b/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'), diff --git a/server/controllers/api/users/index.ts b/server/controllers/api/users/index.ts index 63747a0a9..ae40e86f8 100644 --- a/server/controllers/api/users/index.ts +++ b/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) ) diff --git a/server/middlewares/validators/users.ts b/server/middlewares/validators/users.ts index da92c715d..16d297047 100644 --- a/server/middlewares/validators/users.ts +++ b/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.' }) - .end() + .json({ error: 'Channel is optional but if you specify it, channel.name and channel.displayName are required.' }) } if (body.channel.name === body.username) { return res.status(400) - .send({ error: 'Channel name cannot be the same than user username.' }) - .end() + .json({ error: 'Channel name cannot be the same than user username.' }) } const existing = await ActorModel.loadLocalByName(body.channel.name) if (existing) { return res.status(409) - .send({ error: `Channel with name ${body.channel.name} already exists.` }) - .end() + .json({ error: `Channel with name ${body.channel.name} already exists.` }) } } @@ -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' }) - .end() + .json({ error: 'Cannot remove the root user' }) } 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' }) - .end() + .json({ error: 'Cannot block the root user' }) } 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.' }) - .end() + .json({ error: 'Cannot change root role.' }) } 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.' }) - .end() + .json({ error: 'currentPassword is invalid.' }) } } @@ -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.' }) - .end() + .json({ error: 'User registration is not enabled or user limit is reached.' }) } 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.' }) - .end() + .json({ error: 'You are not on a network authorized for registration.' }) } return next() @@ -323,8 +321,7 @@ const usersResetPasswordValidator = [ if (redisVerificationString !== req.body.verificationString) { return res .status(403) - .send({ error: 'Invalid verification string.' }) - .end() + .json({ error: 'Invalid verification string.' }) } return next() @@ -371,8 +368,7 @@ const usersVerifyEmailValidator = [ if (redisVerificationString !== req.body.verificationString) { return res .status(403) - .send({ error: 'Invalid verification string.' }) - .end() + .json({ error: 'Invalid verification string.' }) } 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.' }) - .end() + .json({ error: 'Only owner can access ratings list.' }) } 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.' }) - .end() + .json({ error: 'User with this username or email already exists.' }) 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.' }) - .end() + .json({ error: 'Another actor (account/channel) with this name on this instance already exists or has already existed.' }) return false } @@ -456,8 +463,7 @@ async function checkUserExist (finder: () => Bluebird, res: express.R if (!user) { if (abortResponse === true) { res.status(404) - .send({ error: 'User not found' }) - .end() + .json({ error: 'User not found' }) } return false diff --git a/server/tests/api/check-params/users.ts b/server/tests/api/check-params/users.ts index 5b788e328..939b919ed 100644 --- a/server/tests/api/check-params/users.ts +++ b/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 createUser({ - url: server.url, - accessToken: server.accessToken, - username: user.username, - password: user.password, - videoQuota: videoQuota - }) - userAccessToken = await userLogin(server, user) + await setAccessTokensToServers([ server ]) + } + + { + const user = { + username: 'user1', + password: 'my super password' + } + + 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 () {