From 77a5501f6413aff2f2a626b929dfda486fa9a3e6 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 6 Sep 2017 16:35:40 +0200 Subject: [PATCH] Fix tests and user quota --- config/test-1.yaml | 2 +- server/controllers/api/users.ts | 30 +++-- server/initializers/checker.ts | 2 +- server/middlewares/validators/users.ts | 61 +++++---- server/middlewares/validators/videos.ts | 9 ++ server/models/user/user.ts | 26 ++-- server/tests/api/check-params/users.ts | 159 ++++++++++++++++++++---- server/tests/api/users.ts | 4 +- server/tests/utils/users.ts | 2 +- 9 files changed, 221 insertions(+), 74 deletions(-) diff --git a/config/test-1.yaml b/config/test-1.yaml index f1514e9cc..4e9f29435 100644 --- a/config/test-1.yaml +++ b/config/test-1.yaml @@ -22,7 +22,7 @@ admin: email: 'admin1@example.com' user: - video_quota: 1024 * 1024 * 5 + video_quota: 5242880 # 5MB signup: limit: 4 diff --git a/server/controllers/api/users.ts b/server/controllers/api/users.ts index 6922661ae..1ecaaf93f 100644 --- a/server/controllers/api/users.ts +++ b/server/controllers/api/users.ts @@ -8,6 +8,7 @@ import { ensureIsAdmin, ensureUserRegistrationAllowed, usersAddValidator, + usersRegisterValidator, usersUpdateValidator, usersUpdateMeValidator, usersRemoveValidator, @@ -25,6 +26,7 @@ import { UserUpdate, UserUpdateMe } from '../../../shared' +import { UserInstance } from '../../models' const usersRouter = express.Router() @@ -61,8 +63,8 @@ usersRouter.post('/', usersRouter.post('/register', ensureUserRegistrationAllowed, - usersAddValidator, - createUser + usersRegisterValidator, + registerUser ) usersRouter.put('/me', @@ -99,11 +101,6 @@ export { function createUser (req: express.Request, res: express.Response, next: express.NextFunction) { const body: UserCreate = req.body - // On registration, we set the user video quota - if (body.videoQuota === undefined) { - body.videoQuota = CONFIG.USER.VIDEO_QUOTA - } - const user = db.User.build({ username: body.username, password: body.password, @@ -118,6 +115,23 @@ function createUser (req: express.Request, res: express.Response, next: express. .catch(err => next(err)) } +function registerUser (req: express.Request, res: express.Response, next: express.NextFunction) { + const body: UserCreate = req.body + + const user = db.User.build({ + username: body.username, + password: body.password, + email: body.email, + displayNSFW: false, + role: USER_ROLES.USER, + videoQuota: CONFIG.USER.VIDEO_QUOTA + }) + + user.save() + .then(() => res.type('json').status(204).end()) + .catch(err => next(err)) +} + function getUserInformation (req: express.Request, res: express.Response, next: express.NextFunction) { db.User.loadByUsername(res.locals.oauth.token.user.username) .then(user => res.json(user.toFormattedJSON())) @@ -180,7 +194,7 @@ function updateMe (req: express.Request, res: express.Response, next: express.Ne function updateUser (req: express.Request, res: express.Response, next: express.NextFunction) { const body: UserUpdate = req.body - const user = res.locals.user + const user: UserInstance = res.locals.user if (body.email !== undefined) user.email = body.email if (body.videoQuota !== undefined) user.videoQuota = body.videoQuota diff --git a/server/initializers/checker.ts b/server/initializers/checker.ts index 97606ef31..eb9e9e280 100644 --- a/server/initializers/checker.ts +++ b/server/initializers/checker.ts @@ -22,7 +22,7 @@ function checkMissedConfig () { 'webserver.https', 'webserver.hostname', 'webserver.port', 'database.hostname', 'database.port', 'database.suffix', 'database.username', 'database.password', 'storage.certs', 'storage.videos', 'storage.logs', 'storage.thumbnails', 'storage.previews', 'storage.torrents', 'storage.cache', - 'cache.previews.size', 'admin.email', 'signup.enabled', 'signup.limit', 'transcoding.enabled', 'transcoding.threads' + 'cache.previews.size', 'admin.email', 'signup.enabled', 'signup.limit', 'transcoding.enabled', 'transcoding.threads', 'user.video_quota' ] const miss: string[] = [] diff --git a/server/middlewares/validators/users.ts b/server/middlewares/validators/users.ts index ebb343535..aec6324bf 100644 --- a/server/middlewares/validators/users.ts +++ b/server/middlewares/validators/users.ts @@ -6,7 +6,7 @@ import * as validator from 'validator' import { database as db } from '../../initializers/database' import { checkErrors } from './utils' import { isSignupAllowed, logger } from '../../helpers' -import { VideoInstance } from '../../models' +import { UserInstance, VideoInstance } from '../../models' function usersAddValidator (req: express.Request, res: express.Response, next: express.NextFunction) { req.checkBody('username', 'Should have a valid username').isUserUsernameValid() @@ -17,16 +17,19 @@ function usersAddValidator (req: express.Request, res: express.Response, next: e logger.debug('Checking usersAdd parameters', { parameters: req.body }) checkErrors(req, res, () => { - db.User.loadByUsernameOrEmail(req.body.username, req.body.email) - .then(user => { - if (user) return res.status(409).send('User already exists.') + checkUserDoesNotAlreadyExist(req.body.username, req.body.email, res, next) + }) +} - next() - }) - .catch(err => { - logger.error('Error in usersAdd request validator.', err) - return res.sendStatus(500) - }) +function usersRegisterValidator (req: express.Request, res: express.Response, next: express.NextFunction) { + req.checkBody('username', 'Should have a valid username').isUserUsernameValid() + req.checkBody('password', 'Should have a valid password').isUserPasswordValid() + req.checkBody('email', 'Should have a valid email').isEmail() + + logger.debug('Checking usersRegister parameters', { parameters: req.body }) + + checkErrors(req, res, () => { + checkUserDoesNotAlreadyExist(req.body.username, req.body.email, res, next) }) } @@ -36,18 +39,16 @@ function usersRemoveValidator (req: express.Request, res: express.Response, next logger.debug('Checking usersRemove parameters', { parameters: req.params }) checkErrors(req, res, () => { - db.User.loadById(req.params.id) - .then(user => { - if (!user) return res.status(404).send('User not found') + checkUserExists(req.params.id, res, (err, user) => { + if (err) { + logger.error('Error in usersRemoveValidator.', err) + return res.sendStatus(500) + } - if (user.username === 'root') return res.status(400).send('Cannot remove the root user') + if (user.username === 'root') return res.status(400).send('Cannot remove the root user') - next() - }) - .catch(err => { - logger.error('Error in usersRemove request validator.', err) - return res.sendStatus(500) - }) + next() + }) }) } @@ -69,7 +70,7 @@ function usersUpdateMeValidator (req: express.Request, res: express.Response, ne req.checkBody('email', 'Should have a valid email attribute').optional().isEmail() req.checkBody('displayNSFW', 'Should have a valid display Not Safe For Work attribute').optional().isUserDisplayNSFWValid() - logger.debug('Checking usersUpdate parameters', { parameters: req.body }) + logger.debug('Checking usersUpdateMe parameters', { parameters: req.body }) checkErrors(req, res, next) } @@ -123,6 +124,7 @@ function ensureUserRegistrationAllowed (req: express.Request, res: express.Respo export { usersAddValidator, + usersRegisterValidator, usersRemoveValidator, usersUpdateValidator, usersUpdateMeValidator, @@ -133,16 +135,29 @@ export { // --------------------------------------------------------------------------- -function checkUserExists (id: number, res: express.Response, callback: () => void) { +function checkUserExists (id: number, res: express.Response, callback: (err: Error, user: UserInstance) => void) { db.User.loadById(id) .then(user => { if (!user) return res.status(404).send('User not found') res.locals.user = user - callback() + callback(null, user) }) .catch(err => { logger.error('Error in user request validator.', err) return res.sendStatus(500) }) } + +function checkUserDoesNotAlreadyExist (username: string, email: string, res: express.Response, callback: () => void) { + db.User.loadByUsernameOrEmail(username, email) + .then(user => { + if (user) return res.status(409).send('User already exists.') + + callback() + }) + .catch(err => { + logger.error('Error in usersAdd request validator.', err) + return res.sendStatus(500) + }) +} diff --git a/server/middlewares/validators/videos.ts b/server/middlewares/validators/videos.ts index ba8c2d834..249da668d 100644 --- a/server/middlewares/validators/videos.ts +++ b/server/middlewares/validators/videos.ts @@ -36,6 +36,12 @@ function videosAddValidator (req: express.Request, res: express.Response, next: } return db.Video.getDurationFromFile(videoFile.path) + .catch(err => { + logger.error('Invalid input file in videosAddValidator.', err) + res.status(400).send('Invalid input file.') + + return undefined + }) }) .then(duration => { // Previous test failed, abort @@ -51,7 +57,10 @@ function videosAddValidator (req: express.Request, res: express.Response, next: .catch(err => { logger.error('Error in video add validator', err) res.sendStatus(500) + + return undefined }) + }) } diff --git a/server/models/user/user.ts b/server/models/user/user.ts index 9bf13ad24..79a595528 100644 --- a/server/models/user/user.ts +++ b/server/models/user/user.ts @@ -242,25 +242,26 @@ loadByUsernameOrEmail = function (username: string, email: string) { // --------------------------------------------------------------------------- function getOriginalVideoFileTotalFromUser (user: UserInstance) { + // attributes = [] because we don't want other fields than the sum const query = { - attributes: [ - Sequelize.fn('COUNT', Sequelize.col('User.Author.Video.VideoFile.size'), 'totalVideoBytes') - ], where: { - id: user.id + resolution: 0 // Original, TODO: improve readability }, include: [ { - model: User['sequelize'].models.Author, - required: true, + attributes: [], + model: User['sequelize'].models.Video, include: [ { - model: User['sequelize'].models.Video, - required: true, + attributes: [], + model: User['sequelize'].models.Author, include: [ { - model: User['sequelize'].models.VideoFile, - required: true + attributes: [], + model: User['sequelize'].models.User, + where: { + id: user.id + } } ] } @@ -269,8 +270,5 @@ function getOriginalVideoFileTotalFromUser (user: UserInstance) { ] } - // FIXME: cast to any because of bad typing... - return User.findAll(query).then((res: any) => { - return res.totalVideoBytes - }) + return User['sequelize'].models.VideoFile.sum('size', query) } diff --git a/server/tests/api/check-params/users.ts b/server/tests/api/check-params/users.ts index 643a82afd..ef78c8262 100644 --- a/server/tests/api/check-params/users.ts +++ b/server/tests/api/check-params/users.ts @@ -43,7 +43,8 @@ describe('Test users API validators', function () { const username = 'user1' const password = 'my super password' - await createUser(server.url, server.accessToken, username, password) + const videoQuota = 42000000 + await createUser(server.url, server.accessToken, username, password, videoQuota) const videoAttributes = {} await uploadVideo(server.url, server.accessToken, videoAttributes) @@ -90,7 +91,8 @@ describe('Test users API validators', function () { const fields = { username: 'ji', email: 'test@example.com', - password: 'my_super_password' + password: 'my_super_password', + videoQuota: 42000000 } await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields }) @@ -100,7 +102,8 @@ describe('Test users API validators', function () { const fields = { username: 'my_super_username_which_is_very_long', email: 'test@example.com', - password: 'my_super_password' + password: 'my_super_password', + videoQuota: 42000000 } await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields }) @@ -110,7 +113,8 @@ describe('Test users API validators', function () { const fields = { username: 'my username', email: 'test@example.com', - password: 'my_super_password' + password: 'my_super_password', + videoQuota: 42000000 } await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields }) @@ -119,7 +123,8 @@ describe('Test users API validators', function () { it('Should fail with a missing email', async function () { const fields = { username: 'ji', - password: 'my_super_password' + password: 'my_super_password', + videoQuota: 42000000 } await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields }) @@ -129,7 +134,8 @@ describe('Test users API validators', function () { const fields = { username: 'my_super_username_which_is_very_long', email: 'test_example.com', - password: 'my_super_password' + password: 'my_super_password', + videoQuota: 42000000 } await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields }) @@ -139,7 +145,8 @@ describe('Test users API validators', function () { const fields = { username: 'my_username', email: 'test@example.com', - password: 'bla' + password: 'bla', + videoQuota: 42000000 } await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields }) @@ -151,7 +158,8 @@ describe('Test users API validators', function () { email: 'test@example.com', password: 'my super long password which is very very very very very very very very very very very very very very' + 'very very very very very very very very very very very very very very very veryv very very very very' + - 'very very very very very very very very very very very very very very very very very very very very long' + 'very very very very very very very very very very very very very very very very very very very very long', + videoQuota: 42000000 } await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields }) @@ -161,7 +169,8 @@ describe('Test users API validators', function () { const fields = { username: 'my_username', email: 'test@example.com', - password: 'my super password' + password: 'my super password', + videoQuota: 42000000 } await makePostBodyRequest({ url: server.url, path, token: 'super token', fields, statusCodeExpected: 401 }) @@ -171,7 +180,8 @@ describe('Test users API validators', function () { const fields = { username: 'user1', email: 'test@example.com', - password: 'my super password' + password: 'my super password', + videoQuota: 42000000 } await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields, statusCodeExpected: 409 }) @@ -181,17 +191,40 @@ describe('Test users API validators', function () { const fields = { username: 'my_username', email: 'user1@example.com', - password: 'my super password' + password: 'my super password', + videoQuota: 42000000 } await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields, statusCodeExpected: 409 }) }) + it('Should fail without a videoQuota', async function () { + const fields = { + username: 'my_username', + email: 'user1@example.com', + password: 'my super password' + } + + await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields }) + }) + + it('Should fail with an invalid videoQuota', async function () { + const fields = { + username: 'my_username', + email: 'user1@example.com', + password: 'my super password', + videoQuota: -5 + } + + await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields }) + }) + it('Should succeed with the correct params', async function () { const fields = { username: 'user2', email: 'test@example.com', - password: 'my super password' + password: 'my super password', + videoQuota: -1 } await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields, statusCodeExpected: 204 }) @@ -208,18 +241,20 @@ describe('Test users API validators', function () { const fields = { username: 'user3', email: 'test@example.com', - password: 'my super password' + password: 'my super password', + videoQuota: 42000000 } await makePostBodyRequest({ url: server.url, path, token: userAccessToken, fields, statusCodeExpected: 403 }) }) }) - describe('When updating a user', function () { - before(async function () { - const res = await getUsersList(server.url) + describe('When updating my account', function () { + it('Should fail with an invalid email attribute', async function () { + const fields = { + email: 'blabla' + } - userId = res.body.data[1].id - rootId = res.body.data[2].id + await makePutBodyRequest({ url: server.url, path: path + 'me', token: server.accessToken, fields }) }) it('Should fail with a too small password', async function () { @@ -227,7 +262,7 @@ describe('Test users API validators', function () { password: 'bla' } - await makePutBodyRequest({ url: server.url, path: path + userId, token: userAccessToken, fields }) + await makePutBodyRequest({ url: server.url, path: path + 'me', token: userAccessToken, fields }) }) it('Should fail with a too long password', async function () { @@ -237,7 +272,7 @@ describe('Test users API validators', function () { 'very very very very very very very very very very very very very very very very very very very very long' } - await makePutBodyRequest({ url: server.url, path: path + userId, token: userAccessToken, fields }) + await makePutBodyRequest({ url: server.url, path: path + 'me', token: userAccessToken, fields }) }) it('Should fail with an invalid display NSFW attribute', async function () { @@ -245,7 +280,7 @@ describe('Test users API validators', function () { displayNSFW: -1 } - await makePutBodyRequest({ url: server.url, path: path + userId, token: userAccessToken, fields }) + await makePutBodyRequest({ url: server.url, path: path + 'me', token: userAccessToken, fields }) }) it('Should fail with an non authenticated user', async function () { @@ -253,16 +288,60 @@ describe('Test users API validators', function () { password: 'my super password' } - await makePutBodyRequest({ url: server.url, path: path + userId, token: 'super token', fields, statusCodeExpected: 401 }) + await makePutBodyRequest({ url: server.url, path: path + 'me', token: 'super token', fields, statusCodeExpected: 401 }) }) it('Should succeed with the correct params', async function () { const fields = { password: 'my super password', - displayNSFW: true + displayNSFW: true, + email: 'super_email@example.com' } - await makePutBodyRequest({ url: server.url, path: path + userId, token: userAccessToken, fields, statusCodeExpected: 204 }) + await makePutBodyRequest({ url: server.url, path: path + 'me', token: userAccessToken, fields, statusCodeExpected: 204 }) + }) + }) + + describe('When updating a user', function () { + + before(async function () { + const res = await getUsersList(server.url) + + 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' + } + + await makePutBodyRequest({ url: server.url, path: path + userId, token: server.accessToken, fields }) + }) + + it('Should fail with an invalid videoQuota attribute', async function () { + const fields = { + videoQuota: -90 + } + + await makePutBodyRequest({ url: server.url, path: path + userId, token: server.accessToken, fields }) + }) + + it('Should fail with an non authenticated user', async function () { + const fields = { + videoQuota: 42 + } + + await makePutBodyRequest({ url: server.url, path: path + userId, token: 'super token', fields, statusCodeExpected: 401 }) + }) + + it('Should succeed with the correct params', async function () { + const fields = { + email: 'email@example.com', + videoQuota: 42 + } + + await makePutBodyRequest({ url: server.url, path: path + userId, token: server.accessToken, fields, statusCodeExpected: 204 }) }) }) @@ -491,6 +570,38 @@ describe('Test users API validators', function () { }) }) + describe('When having a video quota', function () { + it('Should fail with a user having too many video', async function () { + const fields = { + videoQuota: 42 + } + + await makePutBodyRequest({ url: server.url, path: path + rootId, token: server.accessToken, fields, statusCodeExpected: 204 }) + + const videoAttributes = {} + await uploadVideo(server.url, server.accessToken, videoAttributes, 403) + }) + + it('Should fail with a registered user having too many video', async function () { + this.timeout(10000) + + server.user = { + username: 'user3', + email: 'test3@example.com', + password: 'my super password' + } + userAccessToken = await loginAndGetAccessToken(server) + + const videoAttributes = { fixture: 'video_short2.webm' } + await uploadVideo(server.url, userAccessToken, videoAttributes) + await uploadVideo(server.url, userAccessToken, videoAttributes) + await uploadVideo(server.url, userAccessToken, videoAttributes) + await uploadVideo(server.url, userAccessToken, videoAttributes) + await uploadVideo(server.url, userAccessToken, videoAttributes) + await uploadVideo(server.url, userAccessToken, videoAttributes, 403) + }) + }) + after(async function () { killallServers([ server, serverWithRegistrationDisabled ]) diff --git a/server/tests/api/users.ts b/server/tests/api/users.ts index 104d783bb..04c68d4ea 100644 --- a/server/tests/api/users.ts +++ b/server/tests/api/users.ts @@ -319,9 +319,9 @@ describe('Test users', function () { }) it('Should be able to update another user', async function () { - await updateUser(server.url, userId, server.accessToken, 'updated2@example.com', 42 ) + await updateUser(server.url, userId, accessToken, 'updated2@example.com', 42) - const res = await getUserInformation(server.url, server.accessToken, userId) + const res = await getUserInformation(server.url, accessToken, userId) const user = res.body expect(user.username).to.equal('user_1') diff --git a/server/tests/utils/users.ts b/server/tests/utils/users.ts index 1c3f6826e..e5f3eb1b3 100644 --- a/server/tests/utils/users.ts +++ b/server/tests/utils/users.ts @@ -118,7 +118,7 @@ function updateUser (url: string, userId: number, accessToken: string, email: st const path = '/api/v1/users/' + userId const toSend = {} - if (email !== undefined && email !== null) toSend['password'] = email + if (email !== undefined && email !== null) toSend['email'] = email if (videoQuota !== undefined && videoQuota !== null) toSend['videoQuota'] = videoQuota return request(url) -- 2.41.0