From e69219184b1a3262ec5e617d30337b6431c9840c Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 8 Aug 2018 14:58:21 +0200 Subject: Implement user blocking on server side --- server/controllers/api/users.ts | 48 ++++++++++++++++++++++ server/helpers/custom-validators/users.ts | 13 +++--- server/initializers/constants.ts | 2 +- .../initializers/migrations/0245-user-blocked.ts | 40 ++++++++++++++++++ server/lib/oauth-model.ts | 7 ++-- server/middlewares/oauth.ts | 2 +- server/middlewares/validators/users.ts | 21 ++++++++++ server/models/account/user.ts | 7 ++++ server/models/oauth/oauth-token.ts | 8 ++-- server/tests/api/check-params/users.ts | 16 +++++++- server/tests/api/users/users.ts | 31 +++++++++++--- server/tests/utils/users/users.ts | 22 ++++++++++ 12 files changed, 196 insertions(+), 21 deletions(-) create mode 100644 server/initializers/migrations/0245-user-blocked.ts (limited to 'server') diff --git a/server/controllers/api/users.ts b/server/controllers/api/users.ts index 3d2586c3a..8f429d0b5 100644 --- a/server/controllers/api/users.ts +++ b/server/controllers/api/users.ts @@ -32,6 +32,7 @@ import { import { deleteMeValidator, usersAskResetPasswordValidator, + usersBlockingValidator, usersResetPasswordValidator, videoImportsSortValidator, videosSortValidator @@ -108,6 +109,19 @@ usersRouter.get('/', asyncMiddleware(listUsers) ) +usersRouter.post('/:id/block', + authenticate, + ensureUserHasRight(UserRight.MANAGE_USERS), + asyncMiddleware(usersBlockingValidator), + asyncMiddleware(blockUser) +) +usersRouter.post('/:id/unblock', + authenticate, + ensureUserHasRight(UserRight.MANAGE_USERS), + asyncMiddleware(usersBlockingValidator), + asyncMiddleware(unblockUser) +) + usersRouter.get('/:id', authenticate, ensureUserHasRight(UserRight.MANAGE_USERS), @@ -278,6 +292,22 @@ async function getUserVideoQuotaUsed (req: express.Request, res: express.Respons return res.json(data) } +async function unblockUser (req: express.Request, res: express.Response, next: express.NextFunction) { + const user: UserModel = res.locals.user + + await changeUserBlock(res, user, false) + + return res.status(204).end() +} + +async function blockUser (req: express.Request, res: express.Response, next: express.NextFunction) { + const user: UserModel = res.locals.user + + await changeUserBlock(res, user, true) + + return res.status(204).end() +} + function getUser (req: express.Request, res: express.Response, next: express.NextFunction) { return res.json((res.locals.user as UserModel).toFormattedJSON()) } @@ -423,3 +453,21 @@ async function resetUserPassword (req: express.Request, res: express.Response, n function success (req: express.Request, res: express.Response, next: express.NextFunction) { res.end() } + +async function changeUserBlock (res: express.Response, user: UserModel, block: boolean) { + const oldUserAuditView = new UserAuditView(user.toFormattedJSON()) + + user.blocked = block + + await sequelizeTypescript.transaction(async t => { + await OAuthTokenModel.deleteUserToken(user.id, t) + + await user.save({ transaction: t }) + }) + + auditLogger.update( + res.locals.oauth.token.User.Account.Actor.getIdentifier(), + new UserAuditView(user.toFormattedJSON()), + oldUserAuditView + ) +} diff --git a/server/helpers/custom-validators/users.ts b/server/helpers/custom-validators/users.ts index ce1323e94..4a0d79ae5 100644 --- a/server/helpers/custom-validators/users.ts +++ b/server/helpers/custom-validators/users.ts @@ -2,7 +2,7 @@ import 'express-validator' import * as validator from 'validator' import { UserRole } from '../../../shared' import { CONSTRAINTS_FIELDS, NSFW_POLICY_TYPES } from '../../initializers' -import { exists, isFileValid } from './misc' +import { exists, isFileValid, isBooleanValid } from './misc' import { values } from 'lodash' const USERS_CONSTRAINTS_FIELDS = CONSTRAINTS_FIELDS.USERS @@ -29,17 +29,17 @@ function isUserDescriptionValid (value: string) { return value === null || (exists(value) && validator.isLength(value, CONSTRAINTS_FIELDS.USERS.DESCRIPTION)) } -function isBoolean (value: any) { - return typeof value === 'boolean' || (typeof value === 'string' && validator.isBoolean(value)) -} - const nsfwPolicies = values(NSFW_POLICY_TYPES) function isUserNSFWPolicyValid (value: any) { return exists(value) && nsfwPolicies.indexOf(value) !== -1 } function isUserAutoPlayVideoValid (value: any) { - return isBoolean(value) + return isBooleanValid(value) +} + +function isUserBlockedValid (value: any) { + return isBooleanValid(value) } function isUserRoleValid (value: any) { @@ -57,6 +57,7 @@ function isAvatarFile (files: { [ fieldname: string ]: Express.Multer.File[] } | // --------------------------------------------------------------------------- export { + isUserBlockedValid, isUserPasswordValid, isUserRoleValid, isUserVideoQuotaValid, diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts index 80eb3f1e7..0a651beed 100644 --- a/server/initializers/constants.ts +++ b/server/initializers/constants.ts @@ -15,7 +15,7 @@ let config: IConfig = require('config') // --------------------------------------------------------------------------- -const LAST_MIGRATION_VERSION = 240 +const LAST_MIGRATION_VERSION = 245 // --------------------------------------------------------------------------- diff --git a/server/initializers/migrations/0245-user-blocked.ts b/server/initializers/migrations/0245-user-blocked.ts new file mode 100644 index 000000000..67afea5ed --- /dev/null +++ b/server/initializers/migrations/0245-user-blocked.ts @@ -0,0 +1,40 @@ +import * as Sequelize from 'sequelize' +import { createClient } from 'redis' +import { CONFIG } from '../constants' +import { JobQueue } from '../../lib/job-queue' +import { initDatabaseModels } from '../database' + +async function up (utils: { + transaction: Sequelize.Transaction + queryInterface: Sequelize.QueryInterface + sequelize: Sequelize.Sequelize +}): Promise { + { + const data = { + type: Sequelize.BOOLEAN, + allowNull: true, + defaultValue: null + } + await utils.queryInterface.addColumn('user', 'blocked', data) + } + + { + const query = 'UPDATE "user" SET "blocked" = false' + await utils.sequelize.query(query) + } + + { + const data = { + type: Sequelize.BOOLEAN, + allowNull: false, + defaultValue: null + } + await utils.queryInterface.changeColumn('user', 'blocked', data) + } +} + +function down (options) { + throw new Error('Not implemented.') +} + +export { up, down } diff --git a/server/lib/oauth-model.ts b/server/lib/oauth-model.ts index 3adcce7b0..f13c25795 100644 --- a/server/lib/oauth-model.ts +++ b/server/lib/oauth-model.ts @@ -1,3 +1,4 @@ +import { AccessDeniedError} from 'oauth2-server' import { logger } from '../helpers/logger' import { UserModel } from '../models/account/user' import { OAuthClientModel } from '../models/oauth/oauth-client' @@ -34,6 +35,8 @@ async function getUser (usernameOrEmail: string, password: string) { const passwordMatch = await user.isPasswordMatch(password) if (passwordMatch === false) return null + if (user.blocked) throw new AccessDeniedError('User is blocked.') + return user } @@ -67,9 +70,7 @@ async function saveToken (token: TokenInfo, client: OAuthClientModel, user: User } const tokenCreated = await OAuthTokenModel.create(tokenToCreate) - const tokenToReturn = Object.assign(tokenCreated, { client, user }) - - return tokenToReturn + return Object.assign(tokenCreated, { client, user }) } // --------------------------------------------------------------------------- diff --git a/server/middlewares/oauth.ts b/server/middlewares/oauth.ts index a6f28dd5b..5233b66bd 100644 --- a/server/middlewares/oauth.ts +++ b/server/middlewares/oauth.ts @@ -39,7 +39,7 @@ function token (req: express.Request, res: express.Response, next: express.NextF if (err) { return res.status(err.status) .json({ - error: 'Authentication failed.', + error: err.message, code: err.name }) .end() diff --git a/server/middlewares/validators/users.ts b/server/middlewares/validators/users.ts index 3c207c81f..94d8ab53b 100644 --- a/server/middlewares/validators/users.ts +++ b/server/middlewares/validators/users.ts @@ -74,6 +74,26 @@ const usersRemoveValidator = [ } ] +const usersBlockingValidator = [ + param('id').isInt().not().isEmpty().withMessage('Should have a valid id'), + + async (req: express.Request, res: express.Response, next: express.NextFunction) => { + logger.debug('Checking usersRemove parameters', { parameters: req.params }) + + if (areValidationErrors(req, res)) return + if (!await checkUserIdExist(req.params.id, res)) return + + const user = res.locals.user + if (user.username === 'root') { + return res.status(400) + .send({ error: 'Cannot block the root user' }) + .end() + } + + return next() + } +] + const deleteMeValidator = [ async (req: express.Request, res: express.Response, next: express.NextFunction) => { const user: UserModel = res.locals.oauth.token.User @@ -230,6 +250,7 @@ export { usersAddValidator, deleteMeValidator, usersRegisterValidator, + usersBlockingValidator, usersRemoveValidator, usersUpdateValidator, usersUpdateMeValidator, diff --git a/server/models/account/user.ts b/server/models/account/user.ts index 1b1fc5ee8..ea6d63312 100644 --- a/server/models/account/user.ts +++ b/server/models/account/user.ts @@ -21,6 +21,7 @@ import { hasUserRight, USER_ROLE_LABELS, UserRight } from '../../../shared' import { User, UserRole } from '../../../shared/models/users' import { isUserAutoPlayVideoValid, + isUserBlockedValid, isUserNSFWPolicyValid, isUserPasswordValid, isUserRoleValid, @@ -100,6 +101,12 @@ export class UserModel extends Model { @Column autoPlayVideo: boolean + @AllowNull(false) + @Default(false) + @Is('UserBlocked', value => throwIfNotValid(value, isUserBlockedValid, 'blocked boolean')) + @Column + blocked: boolean + @AllowNull(false) @Is('UserRole', value => throwIfNotValid(value, isUserRoleValid, 'role')) @Column diff --git a/server/models/oauth/oauth-token.ts b/server/models/oauth/oauth-token.ts index 026c30135..4c53848dc 100644 --- a/server/models/oauth/oauth-token.ts +++ b/server/models/oauth/oauth-token.ts @@ -3,6 +3,7 @@ import { logger } from '../../helpers/logger' import { AccountModel } from '../account/account' import { UserModel } from '../account/user' import { OAuthClientModel } from './oauth-client' +import { Transaction } from 'sequelize' export type OAuthTokenInfo = { refreshToken: string @@ -125,7 +126,7 @@ export class OAuthTokenModel extends Model { } as OAuthTokenInfo }) .catch(err => { - logger.info('getRefreshToken error.', { err }) + logger.error('getRefreshToken error.', { err }) throw err }) } @@ -163,11 +164,12 @@ export class OAuthTokenModel extends Model { }) } - static deleteUserToken (userId: number) { + static deleteUserToken (userId: number, t?: Transaction) { const query = { where: { userId - } + }, + transaction: t } return OAuthTokenModel.destroy(query) diff --git a/server/tests/api/check-params/users.ts b/server/tests/api/check-params/users.ts index 60165ae22..b3fb61f6c 100644 --- a/server/tests/api/check-params/users.ts +++ b/server/tests/api/check-params/users.ts @@ -8,7 +8,7 @@ import { UserRole, VideoImport, VideoImportState } from '../../../../shared' import { createUser, flushTests, getMyUserInformation, getMyUserVideoRating, getUsersList, immutableAssign, killallServers, makeGetRequest, makePostBodyRequest, makeUploadRequest, makePutBodyRequest, registerUser, removeUser, runServer, ServerInfo, setAccessTokensToServers, - updateUser, uploadVideo, userLogin, deleteMe + updateUser, uploadVideo, userLogin, deleteMe, unblockUser, blockUser } from '../../utils' import { checkBadCountPagination, checkBadSortPagination, checkBadStartPagination } from '../../utils/requests/check-api-params' import { getMagnetURI, getMyVideoImports, getYoutubeVideoUrl, importVideo } from '../../utils/videos/video-imports' @@ -455,17 +455,29 @@ describe('Test users API validators', function () { }) }) - describe('When removing an user', function () { + describe('When blocking/unblocking/removing user', function () { it('Should fail with an incorrect id', async function () { await removeUser(server.url, 'blabla', server.accessToken, 400) + await blockUser(server.url, 'blabla', server.accessToken, 400) + await unblockUser(server.url, 'blabla', server.accessToken, 400) }) it('Should fail with the root user', async function () { await removeUser(server.url, rootId, server.accessToken, 400) + await blockUser(server.url, rootId, server.accessToken, 400) + await unblockUser(server.url, rootId, server.accessToken, 400) }) it('Should return 404 with a non existing id', async function () { await removeUser(server.url, 4545454, server.accessToken, 404) + await blockUser(server.url, 4545454, server.accessToken, 404) + await unblockUser(server.url, 4545454, server.accessToken, 404) + }) + + it('Should fail with a non admin user', async function () { + await removeUser(server.url, userId, userAccessToken, 403) + await blockUser(server.url, userId, userAccessToken, 403) + await unblockUser(server.url, userId, userAccessToken, 403) }) }) diff --git a/server/tests/api/users/users.ts b/server/tests/api/users/users.ts index c9e8eb6f9..77aa00f60 100644 --- a/server/tests/api/users/users.ts +++ b/server/tests/api/users/users.ts @@ -7,7 +7,7 @@ import { 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, - deleteMe + deleteMe, blockUser, unblockUser } from '../../utils/index' import { follow } from '../../utils/server/follows' import { setAccessTokensToServers } from '../../utils/users/login' @@ -45,28 +45,28 @@ describe('Test users', function () { const client = { id: 'client', secret: server.client.secret } const res = await login(server.url, client, server.user, 400) - expect(res.body.error).to.equal('Authentication failed.') + expect(res.body.error).to.contain('client is invalid') }) it('Should not login with an invalid client secret', async function () { const client = { id: server.client.id, secret: 'coucou' } const res = await login(server.url, client, server.user, 400) - expect(res.body.error).to.equal('Authentication failed.') + expect(res.body.error).to.contain('client is invalid') }) it('Should not login with an invalid username', async function () { const user = { username: 'captain crochet', password: server.user.password } const res = await login(server.url, server.client, user, 400) - expect(res.body.error).to.equal('Authentication failed.') + expect(res.body.error).to.contain('credentials are invalid') }) it('Should not login with an invalid password', async function () { const user = { username: server.user.username, password: 'mew_three' } const res = await login(server.url, server.client, user, 400) - expect(res.body.error).to.equal('Authentication failed.') + expect(res.body.error).to.contain('credentials are invalid') }) it('Should not be able to upload a video', async function () { @@ -493,6 +493,27 @@ describe('Test users', function () { } }) + it('Should block and unblock a user', async function () { + const user16 = { + username: 'user_16', + password: 'my super password' + } + const resUser = await createUser(server.url, server.accessToken, user16.username, user16.password) + const user16Id = resUser.body.user.id + + accessToken = await userLogin(server, user16) + + await getMyUserInformation(server.url, accessToken, 200) + await blockUser(server.url, user16Id, server.accessToken) + + await getMyUserInformation(server.url, accessToken, 401) + await userLogin(server, user16, 400) + + await unblockUser(server.url, user16Id, server.accessToken) + accessToken = await userLogin(server, user16) + await getMyUserInformation(server.url, accessToken, 200) + }) + after(async function () { killallServers([ server ]) diff --git a/server/tests/utils/users/users.ts b/server/tests/utils/users/users.ts index e24e721bd..7e15fc86e 100644 --- a/server/tests/utils/users/users.ts +++ b/server/tests/utils/users/users.ts @@ -134,6 +134,26 @@ function removeUser (url: string, userId: number | string, accessToken: string, .expect(expectedStatus) } +function blockUser (url: string, userId: number | string, accessToken: string, expectedStatus = 204) { + const path = '/api/v1/users' + + return request(url) + .post(path + '/' + userId + '/block') + .set('Accept', 'application/json') + .set('Authorization', 'Bearer ' + accessToken) + .expect(expectedStatus) +} + +function unblockUser (url: string, userId: number | string, accessToken: string, expectedStatus = 204) { + const path = '/api/v1/users' + + return request(url) + .post(path + '/' + userId + '/unblock') + .set('Accept', 'application/json') + .set('Authorization', 'Bearer ' + accessToken) + .expect(expectedStatus) +} + function updateMyUser (options: { url: string accessToken: string, @@ -234,6 +254,8 @@ export { updateUser, updateMyUser, getUserInformation, + blockUser, + unblockUser, askResetPassword, resetPassword, updateMyAvatar -- cgit v1.2.3