From d57d6f2605f4ac4a81f9a8594433bb7b65f108b9 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Sun, 21 Aug 2016 10:08:40 +0200 Subject: [PATCH] Server: fix makefriends validation and tests --- server.js | 8 +- server/helpers/custom-validators/index.js | 2 + server/helpers/custom-validators/misc.js | 11 +- server/helpers/custom-validators/pods.js | 21 +++ server/middlewares/validators/pods.js | 27 +-- server/tests/api/check-params.js | 191 +++++++++++----------- 6 files changed, 145 insertions(+), 115 deletions(-) create mode 100644 server/helpers/custom-validators/pods.js diff --git a/server.js b/server.js index d38c5830f..676597fae 100644 --- a/server.js +++ b/server.js @@ -53,7 +53,13 @@ app.use(bodyParser.json()) app.use(bodyParser.urlencoded({ extended: false })) // Validate some params for the API app.use(expressValidator({ - customValidators: Object.assign({}, customValidators.misc, customValidators.users, customValidators.videos) + customValidators: Object.assign( + {}, + customValidators.misc, + customValidators.pods, + customValidators.users, + customValidators.videos + ) })) // ----------- Views, routes and static files ----------- diff --git a/server/helpers/custom-validators/index.js b/server/helpers/custom-validators/index.js index ab3066822..96b5b20b9 100644 --- a/server/helpers/custom-validators/index.js +++ b/server/helpers/custom-validators/index.js @@ -1,11 +1,13 @@ 'use strict' const miscValidators = require('./misc') +const podsValidators = require('./pods') const usersValidators = require('./users') const videosValidators = require('./videos') const validators = { misc: miscValidators, + pods: podsValidators, users: usersValidators, videos: videosValidators } diff --git a/server/helpers/custom-validators/misc.js b/server/helpers/custom-validators/misc.js index 13904ea1b..782ae3dee 100644 --- a/server/helpers/custom-validators/misc.js +++ b/server/helpers/custom-validators/misc.js @@ -1,11 +1,8 @@ 'use strict' -const validator = require('express-validator').validator - const miscValidators = { exists: exists, - isArray: isArray, - isEachUrl: isEachUrl + isArray: isArray } function exists (value) { @@ -16,12 +13,6 @@ function isArray (value) { return Array.isArray(value) } -function isEachUrl (urls) { - return urls.every(function (url) { - return validator.isURL(url) - }) -} - // --------------------------------------------------------------------------- module.exports = miscValidators diff --git a/server/helpers/custom-validators/pods.js b/server/helpers/custom-validators/pods.js new file mode 100644 index 000000000..28d04a05d --- /dev/null +++ b/server/helpers/custom-validators/pods.js @@ -0,0 +1,21 @@ +'use strict' + +const validator = require('express-validator').validator + +const miscValidators = require('./misc') + +const podsValidators = { + isEachUniqueUrlValid: isEachUniqueUrlValid +} + +function isEachUniqueUrlValid (urls) { + return miscValidators.isArray(urls) && + urls.length !== 0 && + urls.every(function (url) { + return validator.isURL(url) && urls.indexOf(url) === urls.lastIndexOf(url) + }) +} + +// --------------------------------------------------------------------------- + +module.exports = podsValidators diff --git a/server/middlewares/validators/pods.js b/server/middlewares/validators/pods.js index 7c4d04aff..3c605c45e 100644 --- a/server/middlewares/validators/pods.js +++ b/server/middlewares/validators/pods.js @@ -10,23 +10,24 @@ const validatorsPod = { } function makeFriends (req, res, next) { - req.checkBody('urls', 'Should have an array of urls').isArray() - req.checkBody('urls', 'Should be an url').isEachUrl() + req.checkBody('urls', 'Should have an array of unique urls').isEachUniqueUrlValid() logger.debug('Checking makeFriends parameters', { parameters: req.body }) - friends.hasFriends(function (err, hasFriends) { - if (err) { - logger.error('Cannot know if we have friends.', { error: err }) - res.sendStatus(500) - } + checkErrors(req, res, function () { + friends.hasFriends(function (err, hasFriends) { + if (err) { + logger.error('Cannot know if we have friends.', { error: err }) + res.sendStatus(500) + } - if (hasFriends === true) { - // We need to quit our friends before make new ones - res.sendStatus(409) - } else { - return next() - } + if (hasFriends === true) { + // We need to quit our friends before make new ones + res.sendStatus(409) + } else { + return next() + } + }) }) } diff --git a/server/tests/api/check-params.js b/server/tests/api/check-params.js index ec666417c..4f7b26561 100644 --- a/server/tests/api/check-params.js +++ b/server/tests/api/check-params.js @@ -44,6 +44,106 @@ describe('Test parameters validator', function () { describe('Of the pods API', function () { const path = '/api/v1/pods/' + describe('When making friends', function () { + let userAccessToken = null + + before(function (done) { + usersUtils.createUser(server.url, server.accessToken, 'user1', 'password', function () { + server.user = { + username: 'user1', + password: 'password' + } + + loginUtils.loginAndGetAccessToken(server, function (err, accessToken) { + if (err) throw err + + userAccessToken = accessToken + + done() + }) + }) + }) + + describe('When making friends', function () { + const body = { + urls: [ 'http://localhost:9002' ] + } + + it('Should fail without urls', function (done) { + request(server.url) + .post(path + '/makefriends') + .set('Authorization', 'Bearer ' + server.accessToken) + .set('Accept', 'application/json') + .expect(400, done) + }) + + it('Should fail with urls is not an array', function (done) { + request(server.url) + .post(path + '/makefriends') + .send({ urls: 'http://localhost:9002' }) + .set('Authorization', 'Bearer ' + server.accessToken) + .set('Accept', 'application/json') + .expect(400, done) + }) + + it('Should fail if the array is not composed by urls', function (done) { + request(server.url) + .post(path + '/makefriends') + .send({ urls: [ 'http://localhost:9002', 'localhost:coucou' ] }) + .set('Authorization', 'Bearer ' + server.accessToken) + .set('Accept', 'application/json') + .expect(400, done) + }) + + it('Should fail if urls are not unique', function (done) { + request(server.url) + .post(path + '/makefriends') + .send({ urls: [ 'http://localhost:9002', 'http://localhost:9002' ] }) + .set('Authorization', 'Bearer ' + server.accessToken) + .set('Accept', 'application/json') + .expect(400, done) + }) + + it('Should fail with a invalid token', function (done) { + request(server.url) + .post(path + '/makefriends') + .send(body) + .set('Authorization', 'Bearer faketoken') + .set('Accept', 'application/json') + .expect(401, done) + }) + + it('Should fail if the user is not an administrator', function (done) { + request(server.url) + .post(path + '/makefriends') + .send(body) + .set('Authorization', 'Bearer ' + userAccessToken) + .set('Accept', 'application/json') + .expect(403, done) + }) + }) + + describe('When quitting friends', function () { + it('Should fail with a invalid token', function (done) { + request(server.url) + .get(path + '/quitfriends') + .query({ start: 'hello' }) + .set('Authorization', 'Bearer faketoken') + .set('Accept', 'application/json') + .expect(401, done) + }) + + it('Should fail if the user is not an administrator', function (done) { + request(server.url) + .get(path + '/quitfriends') + .query({ start: 'hello' }) + .set('Authorization', 'Bearer ' + userAccessToken) + .set('Accept', 'application/json') + .expect(403, done) + }) + }) + }) + describe('When adding a pod', function () { it('Should fail with nothing', function (done) { const data = {} @@ -86,97 +186,6 @@ describe('Test parameters validator', function () { requestsUtils.makePostBodyRequest(server.url, path, null, data, done, 200) }) }) - - describe('For the friends API', function () { - let userAccessToken = null - - before(function (done) { - usersUtils.createUser(server.url, server.accessToken, 'user1', 'password', function () { - server.user = { - username: 'user1', - password: 'password' - } - - loginUtils.loginAndGetAccessToken(server, function (err, accessToken) { - if (err) throw err - - userAccessToken = accessToken - - done() - }) - }) - }) - - describe('When making friends', function () { - const body = { - urls: [ 'http://localhost:9002' ] - } - - it('Should fail without urls', function (done) { - request(server.url) - .post(path + '/makefriends') - .set('Authorization', 'Bearer faketoken') - .set('Accept', 'application/json') - .expect(401, done) - }) - - it('Should fail with urls is not an array', function (done) { - request(server.url) - .post(path + '/makefriends') - .send({ urls: 'http://localhost:9002' }) - .set('Authorization', 'Bearer faketoken') - .set('Accept', 'application/json') - .expect(401, done) - }) - - it('Should fail if the array is not composed by urls', function (done) { - request(server.url) - .post(path + '/makefriends') - .send({ urls: [ 'http://localhost:9002', 'localhost:coucou' ] }) - .set('Authorization', 'Bearer faketoken') - .set('Accept', 'application/json') - .expect(401, done) - }) - - it('Should fail with a invalid token', function (done) { - request(server.url) - .post(path + '/makefriends') - .send(body) - .set('Authorization', 'Bearer faketoken') - .set('Accept', 'application/json') - .expect(401, done) - }) - - it('Should fail if the user is not an administrator', function (done) { - request(server.url) - .post(path + '/makefriends') - .send(body) - .set('Authorization', 'Bearer ' + userAccessToken) - .set('Accept', 'application/json') - .expect(403, done) - }) - }) - - describe('When quitting friends', function () { - it('Should fail with a invalid token', function (done) { - request(server.url) - .get(path + '/quitfriends') - .query({ start: 'hello' }) - .set('Authorization', 'Bearer faketoken') - .set('Accept', 'application/json') - .expect(401, done) - }) - - it('Should fail if the user is not an administrator', function (done) { - request(server.url) - .get(path + '/quitfriends') - .query({ start: 'hello' }) - .set('Authorization', 'Bearer ' + userAccessToken) - .set('Accept', 'application/json') - .expect(403, done) - }) - }) - }) }) describe('Of the videos API', function () {