From a95a4cc89155f448e6f9ca0957170f3c72a9d964 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 30 Jul 2019 09:59:19 +0200 Subject: Moderators can only manage users --- server/controllers/api/users/index.ts | 7 +- server/middlewares/validators/users.ts | 72 +++++++------- server/tests/api/check-params/users.ts | 166 +++++++++++++++++++++++++++------ 3 files changed, 181 insertions(+), 64 deletions(-) (limited to 'server') 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 () { -- cgit v1.2.3