diff --git a/core/server/api/posts.js b/core/server/api/posts.js index 51ca23a26db..d8a2170a34e 100644 --- a/core/server/api/posts.js +++ b/core/server/api/posts.js @@ -17,6 +17,7 @@ var Promise = require('bluebird'), * * **See:** [API Methods](index.js.html#api%20methods) */ + posts = { /** * ## Browse @@ -33,7 +34,9 @@ posts = { * @returns {Promise} Posts Collection with Meta */ browse: function browse(options) { - var tasks; + var extraOptions = ['tag', 'author', 'status', 'staticPages', 'featured'], + permittedOptions = utils.browseDefaultOptions.concat(extraOptions), + tasks; /** * ### Handle Permissions @@ -60,7 +63,12 @@ posts = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName), handlePermissions, utils.convertOptions(allowedIncludes), modelQuery]; + tasks = [ + utils.validate(docName, {opts: permittedOptions}), + handlePermissions, + utils.convertOptions(allowedIncludes), + modelQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, options); @@ -102,7 +110,12 @@ posts = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName, attrs), handlePermissions, utils.convertOptions(allowedIncludes), modelQuery]; + tasks = [ + utils.validate(docName, {attrs: attrs}), + handlePermissions, + utils.convertOptions(allowedIncludes), + modelQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, options).then(function formatResponse(result) { @@ -152,7 +165,12 @@ posts = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName), handlePermissions, utils.convertOptions(allowedIncludes), modelQuery]; + tasks = [ + utils.validate(docName, {opts: utils.idDefaultOptions}), + handlePermissions, + utils.convertOptions(allowedIncludes), + modelQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, object, options).then(function formatResponse(result) { @@ -208,7 +226,12 @@ posts = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName), handlePermissions, utils.convertOptions(allowedIncludes), modelQuery]; + tasks = [ + utils.validate(docName), + handlePermissions, + utils.convertOptions(allowedIncludes), + modelQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, object, options).then(function formatResponse(result) { @@ -263,7 +286,12 @@ posts = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName), handlePermissions, utils.convertOptions(allowedIncludes), modelQuery]; + tasks = [ + utils.validate(docName, {opts: utils.idDefaultOptions}), + handlePermissions, + utils.convertOptions(allowedIncludes), + modelQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, options).then(function formatResponse(result) { diff --git a/core/server/api/roles.js b/core/server/api/roles.js index 87bf7cc46f1..ae94977bd48 100644 --- a/core/server/api/roles.js +++ b/core/server/api/roles.js @@ -30,7 +30,8 @@ roles = { * @returns {Promise(Roles)} Roles Collection */ browse: function browse(options) { - var tasks; + var permittedOptions = ['permissions'], + tasks; /** * ### Handle Permissions @@ -57,7 +58,11 @@ roles = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName), handlePermissions, modelQuery]; + tasks = [ + utils.validate(docName, {opts: permittedOptions}), + handlePermissions, + modelQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, options).then(function formatResponse(results) { diff --git a/core/server/api/slugs.js b/core/server/api/slugs.js index cd8dc2b38d0..f26b0759964 100644 --- a/core/server/api/slugs.js +++ b/core/server/api/slugs.js @@ -26,7 +26,9 @@ slugs = { * @returns {Promise(String)} Unique string */ generate: function (options) { - var tasks; + var opts = ['type'], + attrs = ['name'], + tasks; // `allowedTypes` is used to define allowed slug types and map them against its model class counterpart allowedTypes = { @@ -60,11 +62,15 @@ slugs = { * @returns {Object} options */ function modelQuery(options) { - return dataProvider.Base.Model.generateSlug(allowedTypes[options.type], options.name, {status: 'all'}); + return dataProvider.Base.Model.generateSlug(allowedTypes[options.type], options.data.name, {status: 'all'}); } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName), handlePermissions, modelQuery]; + tasks = [ + utils.validate(docName, {opts: opts, attrs: attrs}), + handlePermissions, + modelQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, options).then(function (slug) { diff --git a/core/server/api/tags.js b/core/server/api/tags.js index 821056af98b..11465baf2d0 100644 --- a/core/server/api/tags.js +++ b/core/server/api/tags.js @@ -51,7 +51,12 @@ tags = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName), handlePermissions, utils.convertOptions(allowedIncludes), doQuery]; + tasks = [ + utils.validate(docName, {opts: utils.browseDefaultOptions}), + handlePermissions, + utils.convertOptions(allowedIncludes), + doQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, options); @@ -91,7 +96,12 @@ tags = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName, attrs), handlePermissions, utils.convertOptions(allowedIncludes), doQuery]; + tasks = [ + utils.validate(docName, {attrs: attrs}), + handlePermissions, + utils.convertOptions(allowedIncludes), + doQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, options).then(function formatResponse(result) { @@ -136,7 +146,12 @@ tags = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName), handlePermissions, utils.convertOptions(allowedIncludes), doQuery]; + tasks = [ + utils.validate(docName), + handlePermissions, + utils.convertOptions(allowedIncludes), + doQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, object, options).then(function formatResponse(result) { @@ -181,7 +196,12 @@ tags = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName), handlePermissions, utils.convertOptions(allowedIncludes), doQuery]; + tasks = [ + utils.validate(docName, {opts: utils.idDefaultOptions}), + handlePermissions, + utils.convertOptions(allowedIncludes), + doQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, object, options).then(function formatResponse(result) { @@ -234,7 +254,12 @@ tags = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName), handlePermissions, utils.convertOptions(allowedIncludes), doQuery]; + tasks = [ + utils.validate(docName, {opts: utils.idDefaultOptions}), + handlePermissions, + utils.convertOptions(allowedIncludes), + doQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, options); diff --git a/core/server/api/users.js b/core/server/api/users.js index 508c671330c..782f9d5eddf 100644 --- a/core/server/api/users.js +++ b/core/server/api/users.js @@ -73,7 +73,9 @@ users = { * @returns {Promise} Users Collection */ browse: function browse(options) { - var tasks; + var extraOptions = ['role', 'status'], + permittedOptions = utils.browseDefaultOptions.concat(extraOptions), + tasks; /** * ### Handle Permissions @@ -100,7 +102,12 @@ users = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName), handlePermissions, utils.convertOptions(allowedIncludes), doQuery]; + tasks = [ + utils.validate(docName, {opts: permittedOptions}), + handlePermissions, + utils.convertOptions(allowedIncludes), + doQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, options); @@ -112,7 +119,7 @@ users = { * @returns {Promise} User */ read: function read(options) { - var attrs = ['id', 'slug', 'status', 'email'], + var attrs = ['id', 'slug', 'status', 'email', 'role'], tasks; /** @@ -140,7 +147,12 @@ users = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName, attrs), handlePermissions, utils.convertOptions(allowedIncludes), doQuery]; + tasks = [ + utils.validate(docName, {attrs: attrs}), + handlePermissions, + utils.convertOptions(allowedIncludes), + doQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, options).then(function formatResponse(result) { @@ -159,24 +171,12 @@ users = { * @returns {Promise} */ edit: function edit(object, options) { - var tasks; - /** - * ### Validate - * Special validation which handles roles - * @param {Post} object - * @param {Object} options - * @returns {Object} options - */ - function validate(object, options) { - options = options || {}; - return utils.checkObject(object, docName, options.id).then(function (data) { - if (data.users[0].roles && data.users[0].roles[0]) { - options.editRoles = true; - } + var extraOptions = ['editRoles'], + permittedOptions = extraOptions.concat(utils.idDefaultOptions), + tasks; - options.data = data; - return options; - }); + if (object.users && object.users[0] && object.users[0].roles && object.users[0].roles[0]) { + options.editRoles = true; } /** @@ -245,7 +245,12 @@ users = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [validate, handlePermissions, utils.convertOptions(allowedIncludes), doQuery]; + tasks = [ + utils.validate(docName, {opts: permittedOptions}), + handlePermissions, + utils.convertOptions(allowedIncludes), + doQuery + ]; return pipeline(tasks, object, options).then(function formatResponse(result) { if (result) { @@ -359,7 +364,12 @@ users = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName), handlePermissions, utils.convertOptions(allowedIncludes), doQuery]; + tasks = [ + utils.validate(docName), + handlePermissions, + utils.convertOptions(allowedIncludes), + doQuery + ]; return pipeline(tasks, object, options); }, @@ -420,7 +430,12 @@ users = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate(docName), handlePermissions, utils.convertOptions(allowedIncludes), doQuery]; + tasks = [ + utils.validate(docName, {opts: utils.idDefaultOptions}), + handlePermissions, + utils.convertOptions(allowedIncludes), + doQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, options); @@ -463,7 +478,12 @@ users = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate('password'), handlePermissions, utils.convertOptions(allowedIncludes), doQuery]; + tasks = [ + utils.validate('password'), + handlePermissions, + utils.convertOptions(allowedIncludes), + doQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, object, options).then(function formatResponse() { @@ -507,7 +527,12 @@ users = { } // Push all of our tasks into a `tasks` array in the correct order - tasks = [utils.validate('owner'), handlePermissions, utils.convertOptions(allowedIncludes), doQuery]; + tasks = [ + utils.validate('owner'), + handlePermissions, + utils.convertOptions(allowedIncludes), + doQuery + ]; // Pipeline calls each task passing the result of one to be the arguments for the next return pipeline(tasks, object, options).then(function formatResult(result) { diff --git a/core/server/api/utils.js b/core/server/api/utils.js index 07808aee3a8..6944ddb66ba 100644 --- a/core/server/api/utils.js +++ b/core/server/api/utils.js @@ -1,15 +1,46 @@ // # API Utils // Shared helpers for working with the API -var Promise = require('bluebird'), - _ = require('lodash'), - path = require('path'), - errors = require('../errors'), +var Promise = require('bluebird'), + _ = require('lodash'), + path = require('path'), + errors = require('../errors'), + validation = require('../data/validation'), utils; utils = { - validate: function validate(docName, attrs) { + // ## Default Options + // Various default options for different types of endpoints + + // ### Auto Default Options + // Handled / Added automatically by the validate function + // globalDefaultOptions - valid for every api endpoint + globalDefaultOptions: ['context', 'include'], + // dataDefaultOptions - valid for all endpoints which take object as well as options + dataDefaultOptions: ['data'], + + // ### Manual Default Options + // These must be provided by the endpoint + // browseDefaultOptions - valid for all browse api endpoints + browseDefaultOptions: ['page', 'limit'], + // idDefaultOptions - valid whenever an id is valid + idDefaultOptions: ['id'], + + /** + * ## Validate + * Prepare to validate the object and options passed to an endpoint + * @param {String} docName + * @param {Object} extras + * @returns {Function} doValidate + */ + validate: function validate(docName, extras) { + /** + * ### Do Validate + * Validate the object and options passed to an endpoint + * @argument object + * @argument options + */ return function doValidate() { - var object, options; + var object, options, permittedOptions; if (arguments.length === 2) { object = arguments[0]; options = _.clone(arguments[1]) || {}; @@ -19,22 +50,87 @@ utils = { options = {}; } - if (attrs) { - options.data = _.pick(options, attrs); - options = _.omit(options, attrs); + // Setup permitted options, starting with the global defaults + permittedOptions = utils.globalDefaultOptions; + + // Add extra permitted options if any are passed in + if (extras && extras.opts) { + permittedOptions = permittedOptions.concat(extras.opts); + } + + // This request will have a data key added during validation + if ((extras && extras.attrs) || object) { + permittedOptions = permittedOptions.concat(utils.dataDefaultOptions); + } + + // If an 'attrs' object is passed, we use this to pick from options and convert them to data + if (extras && extras.attrs) { + options.data = _.pick(options, extras.attrs); + options = _.omit(options, extras.attrs); } + /** + * ### Check Options + * Ensure that the options provided match exactly with what is permitted + * - incorrect option keys are sanitized + * - incorrect option values are validated + * @param {object} options + * @returns {Promise} + */ + function checkOptions(options) { + // @TODO: should we throw an error if there are incorrect options provided? + options = _.pick(options, permittedOptions); + + var validationErrors = utils.validateOptions(options); + + if (_.isEmpty(validationErrors)) { + return Promise.resolve(options); + } + + return errors.logAndRejectError(validationErrors); + } + + // If we got an object, check that too if (object) { return utils.checkObject(object, docName, options.id).then(function (data) { options.data = data; - return Promise.resolve(options); + + return checkOptions(options); }); } - return Promise.resolve(options); + // Otherwise just check options and return + return checkOptions(options); }; }, + validateOptions: function validateOptions(options) { + var globalValidations = { + id: {matches: /^\d+|me$/}, + uuid: {isUUID: true}, + page: {matches: /^\d+$/}, + limit: {matches: /^\d+|all$/}, + name: {} + }, + // these values are sanitised/validated separately + noValidation = ['data', 'context', 'include'], + errors = []; + + _.each(options, function (value, key) { + // data is validated elsewhere + if (noValidation.indexOf(key) === -1) { + if (globalValidations[key]) { + errors = errors.concat(validation.validate(value, key, globalValidations[key])); + } else { + // all other keys should be an alphanumeric string + -, like slug, tag, author, status, etc + errors = errors.concat(validation.validate(value, key, {matches: /^[a-z0-9\-]+$/})); + } + } + }); + + return errors; + }, + prepareInclude: function prepareInclude(include, allowedIncludes) { include = include || ''; include = _.intersection(include.split(','), allowedIncludes); diff --git a/core/server/data/validation/index.js b/core/server/data/validation/index.js index 49ed1342c94..592add8c304 100644 --- a/core/server/data/validation/index.js +++ b/core/server/data/validation/index.js @@ -162,6 +162,7 @@ validate = function validate(value, key, validations) { }; module.exports = { + validate: validate, validator: validator, validateSchema: validateSchema, validateSettings: validateSettings, diff --git a/core/test/integration/api/api_posts_spec.js b/core/test/integration/api/api_posts_spec.js index c8f812964ee..9fab4da78c5 100644 --- a/core/test/integration/api/api_posts_spec.js +++ b/core/test/integration/api/api_posts_spec.js @@ -27,7 +27,10 @@ describe('Post API', function () { results.posts.length.should.be.above(0); testUtils.API.checkResponse(results.posts[0], 'post'); done(); - }).catch(done); + }).catch(function (err) { + console.log(err); + done(err); + }); }); it('can read', function (done) { diff --git a/core/test/integration/api/api_slugs_spec.js b/core/test/integration/api/api_slugs_spec.js index c0c9aa888c4..b670482a4bf 100644 --- a/core/test/integration/api/api_slugs_spec.js +++ b/core/test/integration/api/api_slugs_spec.js @@ -62,8 +62,8 @@ describe('Slug API', function () { }).catch(done); }); - it('rejects unknown types', function (done) { - SlugAPI.generate({context: {user: 1}, type: 'unknown type', name: 'A fancy Title'}) + it('rejects unknown types with BadRequestError', function (done) { + SlugAPI.generate({context: {user: 1}, type: 'unknown-type', name: 'A fancy Title'}) .then(function () { done(new Error('Generate a slug for an unknown type is not rejected.')); }).catch(function (error) { @@ -71,4 +71,14 @@ describe('Slug API', function () { done(); }).catch(done); }); + + it('rejects invalid types with ValidationError', function (done) { + SlugAPI.generate({context: {user: 1}, type: 'unknown type', name: 'A fancy Title'}) + .then(function () { + done(new Error('Generate a slug for an unknown type is not rejected.')); + }).catch(function (errors) { + errors.should.have.enumerable(0).with.property('errorType', 'ValidationError'); + done(); + }).catch(done); + }); }); diff --git a/core/test/integration/api/api_users_spec.js b/core/test/integration/api/api_users_spec.js index c6526c8cd24..096566e7280 100644 --- a/core/test/integration/api/api_users_spec.js +++ b/core/test/integration/api/api_users_spec.js @@ -23,6 +23,17 @@ describe('Users API', function () { beforeEach(testUtils.setup('users:roles', 'users', 'user:token', 'perms:user', 'perms:role', 'perms:setting', 'perms:init')); afterEach(testUtils.teardown); + function checkForErrorType(type, done) { + return function checkForErrorType(error) { + if (error.errorType) { + error.errorType.should.eql(type); + done(); + } else { + done(error); + } + }; + } + it('dateTime fields are returned as Date objects', function (done) { var userData = testUtils.DataGenerator.forModel.users[0]; @@ -211,10 +222,7 @@ describe('Users API', function () { UserAPI.edit({users: [{id: userIdFor.owner, name: 'Override'}]}, options) .then(function () { done(new Error('ID mismatches should not be permitted')); - }).catch(function (error) { - error.errorType.should.eql('BadRequestError'); - done(); - }); + }).catch(checkForErrorType('BadRequestError', done)); }); it('Owner can edit all roles', function (done) { @@ -419,10 +427,7 @@ describe('Users API', function () { UserAPI.add({users: [newUser]}, _.extend({}, context.owner, {include: 'roles'})) .then(function () { done(new Error('Owner should not be able to add an owner')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('Can add an Admin', function (done) { @@ -492,10 +497,7 @@ describe('Users API', function () { UserAPI.add({users: [newUser]}, _.extend({}, context.admin, {include: 'roles'})) .then(function () { done(new Error('Admin should not be able to add an owner')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('Can add an Admin', function (done) { // Can add admin @@ -562,10 +564,7 @@ describe('Users API', function () { UserAPI.add({users: [newUser]}, _.extend({}, context.editor, {include: 'roles'})) .then(function () { done(new Error('Editor should not be able to add an owner')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('Can add an Author', function (done) { @@ -587,10 +586,7 @@ describe('Users API', function () { UserAPI.add({users: [newUser]}, _.extend({}, context.author, {include: 'roles'})) .then(function () { done(new Error('Author should not be able to add an owner')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('CANNOT add an Author', function (done) { @@ -598,10 +594,7 @@ describe('Users API', function () { UserAPI.add({users: [newUser]}, _.extend({}, context.author, {include: 'roles'})) .then(function () { done(new Error('Author should not be able to add an author')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); }); }); @@ -621,10 +614,7 @@ describe('Users API', function () { UserAPI.destroy(_.extend({}, context.owner, {id: userIdFor.owner})) .then(function () { done(new Error('Owner should not be able to delete itself')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('Can destroy admin, editor, author', function (done) { @@ -652,10 +642,7 @@ describe('Users API', function () { UserAPI.destroy(_.extend({}, context.admin, {id: userIdFor.owner})) .then(function () { done(new Error('Admin should not be able to delete owner')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('Can destroy admin, editor, author', function (done) { @@ -684,30 +671,21 @@ describe('Users API', function () { UserAPI.destroy(_.extend({}, context.editor, {id: userIdFor.owner})) .then(function () { done(new Error('Editor should not be able to delete owner')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('CANNOT destroy admin', function (done) { UserAPI.destroy(_.extend({}, context.editor, {id: userIdFor.admin})) .then(function () { done(new Error('Editor should not be able to delete admin')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('CANNOT destroy other editor', function (done) { UserAPI.destroy(_.extend({}, context.editor, {id: userIdFor.editor2})) .then(function () { done(new Error('Editor should not be able to delete other editor')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('Can destroy self', function (done) { @@ -732,50 +710,35 @@ describe('Users API', function () { UserAPI.destroy(_.extend({}, context.author, {id: userIdFor.owner})) .then(function () { done(new Error('Author should not be able to delete owner')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('CANNOT destroy admin', function (done) { UserAPI.destroy(_.extend({}, context.author, {id: userIdFor.admin})) .then(function () { done(new Error('Author should not be able to delete admin')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('CANNOT destroy editor', function (done) { UserAPI.destroy(_.extend({}, context.author, {id: userIdFor.editor})) .then(function () { done(new Error('Author should not be able to delete editor')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('CANNOT destroy other author', function (done) { UserAPI.destroy(_.extend({}, context.author, {id: userIdFor.author2})) .then(function () { done(new Error('Author should not be able to delete other author')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('CANNOT destroy self', function (done) { UserAPI.destroy(_.extend({}, context.author, {id: userIdFor.author})) .then(function () { done(new Error('Author should not be able to delete self')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); }); }); @@ -798,10 +761,7 @@ describe('Users API', function () { UserAPI.edit({users: [{id: userIdFor.owner, name: 'Override', roles: [roleIdFor.author]}]}, options) .then(function () { done(new Error('ID mismatches should not be permitted')); - }).catch(function (error) { - error.errorType.should.eql('BadRequestError'); - done(); - }); + }).catch(checkForErrorType('BadRequestError', done)); }); describe('Owner', function () { @@ -937,10 +897,7 @@ describe('Users API', function () { users: [{name: newName, roles: [roleIdFor.author]}] }, options).then(function () { done(new Error('Author should not be able to downgrade owner')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }).catch(done); + }).catch(checkForErrorType('NoPermissionError', done)); }); }); }); @@ -965,10 +922,7 @@ describe('Users API', function () { _.extend({}, context.editor, {id: userIdFor.editor}, {include: 'roles'}) ).then(function () { done(new Error('Editor should not be able to upgrade their role')); - }, function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }).catch(done); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('CANNOT assign author role to other Editor', function (done) { @@ -977,10 +931,7 @@ describe('Users API', function () { _.extend({}, context.editor, {id: userIdFor.editor2}, {include: 'roles'}) ).then(function () { done(new Error('Editor should not be able to change the roles of other editors')); - }, function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }).catch(done); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('CANNOT assign author role to admin', function (done) { @@ -989,10 +940,7 @@ describe('Users API', function () { _.extend({}, context.editor, {id: userIdFor.admin}, {include: 'roles'}) ).then(function () { done(new Error('Editor should not be able to change the roles of admins')); - }, function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }).catch(done); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('CANNOT assign admin role to author', function (done) { UserAPI.edit( @@ -1000,10 +948,7 @@ describe('Users API', function () { _.extend({}, context.editor, {id: userIdFor.author}, {include: 'roles'}) ).then(function () { done(new Error('Editor should not be able to upgrade the role of authors')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }).catch(done); + }).catch(checkForErrorType('NoPermissionError', done)); }); }); @@ -1014,10 +959,7 @@ describe('Users API', function () { _.extend({}, context.author, {id: userIdFor.author}, {include: 'roles'}) ).then(function () { done(new Error('Author should not be able to upgrade their role')); - }, function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }).catch(done); + }).catch(checkForErrorType('NoPermissionError', done)); }); }); }); @@ -1046,10 +988,7 @@ describe('Users API', function () { context.owner ).then(function () { done(new Error('Owner should not be able to downgrade their role')); - }).catch(function (error) { - error.errorType.should.eql('ValidationError'); - done(); - }); + }).catch(checkForErrorType('ValidationError', done)); }); it('Admin CANNOT transfer ownership', function (done) { @@ -1059,10 +998,7 @@ describe('Users API', function () { context.admin ).then(function () { done(new Error('Admin is not denied transferring ownership.')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('Editor CANNOT transfer ownership', function (done) { @@ -1072,10 +1008,7 @@ describe('Users API', function () { context.editor ).then(function () { done(new Error('Admin is not denied transferring ownership.')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); it('Author CANNOT transfer ownership', function (done) { @@ -1085,10 +1018,7 @@ describe('Users API', function () { context.author ).then(function () { done(new Error('Admin is not denied transferring ownership.')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); }); @@ -1121,10 +1051,7 @@ describe('Users API', function () { UserAPI.changePassword(payload, _.extend({}, context.owner, {id: userIdFor.owner})) .then(function () { done(new Error('Password change is not denied.')); - }).catch(function (error) { - error.errorType.should.eql('ValidationError'); - done(); - }); + }).catch(checkForErrorType('ValidationError', done)); }); it('Owner can\'t change password without matching passwords', function (done) { @@ -1139,10 +1066,7 @@ describe('Users API', function () { UserAPI.changePassword(payload, _.extend({}, context.owner, {id: userIdFor.owner})) .then(function () { done(new Error('Password change is not denied.')); - }).catch(function (error) { - error.errorType.should.eql('ValidationError'); - done(); - }); + }).catch(checkForErrorType('ValidationError', done)); }); it('Owner can\'t change editor password without matching passwords', function (done) { @@ -1156,10 +1080,7 @@ describe('Users API', function () { UserAPI.changePassword(payload, _.extend({}, context.owner, {id: userIdFor.owner})) .then(function () { done(new Error('Password change is not denied.')); - }).catch(function (error) { - error.errorType.should.eql('ValidationError'); - done(); - }); + }).catch(checkForErrorType('ValidationError', done)); }); it('Owner can\'t change editor password without short passwords', function (done) { @@ -1173,10 +1094,7 @@ describe('Users API', function () { UserAPI.changePassword(payload, _.extend({}, context.owner, {id: userIdFor.owner})) .then(function () { done(new Error('Password change is not denied.')); - }).catch(function (error) { - error.errorType.should.eql('ValidationError'); - done(); - }); + }).catch(checkForErrorType('ValidationError', done)); }); it('Owner can change password for editor', function (done) { @@ -1205,10 +1123,7 @@ describe('Users API', function () { UserAPI.changePassword(payload, _.extend({}, context.editor, {id: userIdFor.editor})) .then(function () { done(new Error('Password change is not denied.')); - }).catch(function (error) { - error.errorType.should.eql('NoPermissionError'); - done(); - }); + }).catch(checkForErrorType('NoPermissionError', done)); }); }); }); diff --git a/core/test/unit/api_utils_spec.js b/core/test/unit/api_utils_spec.js index 0c90769f2f1..9381cf7f91d 100644 --- a/core/test/unit/api_utils_spec.js +++ b/core/test/unit/api_utils_spec.js @@ -18,6 +18,15 @@ describe('API Utils', function () { sandbox.restore(); }); + describe('Default Options', function () { + it('should provide a set of default options', function () { + apiUtils.globalDefaultOptions.should.eql(['context', 'include']); + apiUtils.browseDefaultOptions.should.eql(['page', 'limit']); + apiUtils.dataDefaultOptions.should.eql(['data']); + apiUtils.idDefaultOptions.should.eql(['id']); + }); + }); + describe('validate', function () { it('should create options when passed no args', function (done) { apiUtils.validate()().then(function (options) { @@ -27,7 +36,7 @@ describe('API Utils', function () { }); it('should pick data attrs when passed them', function (done) { - apiUtils.validate('test', ['id'])( + apiUtils.validate('test', {attrs: ['id']})( {id: 'test', status: 'all', uuid: 'other-test'} ).then(function (options) { options.should.have.ownProperty('data'); @@ -35,12 +44,29 @@ describe('API Utils', function () { options.should.not.have.ownProperty('id'); options.data.id.should.eql('test'); + options.data.should.not.have.ownProperty('status'); + options.should.not.have.ownProperty('status'); + + options.should.not.have.ownProperty('uuid'); + done(); + }).catch(done); + }); + + it('should pick data attrs & leave options if passed', function (done) { + apiUtils.validate('test', {attrs: ['id'], opts: ['status', 'uuid']})( + {id: 'test', status: 'all', uuid: 'ffecea44-393c-4273-b784-e1928975ecfb'} + ).then(function (options) { + options.should.have.ownProperty('data'); + options.data.should.have.ownProperty('id'); + options.should.not.have.ownProperty('id'); + options.data.id.should.eql('test'); + options.data.should.not.have.ownProperty('status'); options.should.have.ownProperty('status'); options.status.should.eql('all'); options.should.have.ownProperty('uuid'); - options.uuid.should.eql('other-test'); + options.uuid.should.eql('ffecea44-393c-4273-b784-e1928975ecfb'); done(); }).catch(done); }); @@ -77,6 +103,139 @@ describe('API Utils', function () { done(); }).catch(done); }); + + it('should remove unknown options', function (done) { + apiUtils.validate('test')({magic: 'stuff', rubbish: 'stuff'}).then(function (options) { + options.should.not.have.ownProperty('data'); + options.should.not.have.ownProperty('rubbish'); + options.should.not.have.ownProperty('magic'); + done(); + }).catch(done); + }); + + it('should always allow context & include options', function (done) { + apiUtils.validate('test')({context: 'stuff', include: 'stuff'}).then(function (options) { + options.should.not.have.ownProperty('data'); + options.should.have.ownProperty('context'); + options.context.should.eql('stuff'); + options.should.have.ownProperty('include'); + options.include.should.eql('stuff'); + done(); + }).catch(done); + }); + + it('should allow page & limit options when browseDefaultOptions passed', function (done) { + apiUtils.validate('test', {opts: apiUtils.browseDefaultOptions})( + {context: 'stuff', include: 'stuff', page: 1, limit: 5} + ).then(function (options) { + options.should.not.have.ownProperty('data'); + options.should.have.ownProperty('context'); + options.context.should.eql('stuff'); + options.should.have.ownProperty('include'); + options.include.should.eql('stuff'); + options.should.have.ownProperty('page'); + options.page.should.eql(1); + options.should.have.ownProperty('limit'); + options.limit.should.eql(5); + done(); + }).catch(done); + }); + + it('should allow idDefaultOptions when passed', function (done) { + // test read + apiUtils.validate('test', {opts: apiUtils.idDefaultOptions})( + {id: 5, context: 'stuff'} + ).then(function (options) { + options.should.not.have.ownProperty('data'); + options.should.not.have.ownProperty('include'); + options.should.not.have.ownProperty('page'); + options.should.not.have.ownProperty('limit'); + + options.should.have.ownProperty('context'); + options.context.should.eql('stuff'); + options.should.have.ownProperty('id'); + options.id.should.eql(5); + + done(); + }).catch(done); + }); + + it('should reject if invalid options are passed', function (done) { + apiUtils.validate('test', {opts: apiUtils.browseDefaultOptions})( + {context: 'internal', include: 'stuff', page: 1, limit: 'none'} + ).then(function () { + done(new Error('Should have thrown a validation error')); + }).catch(function (err) { + err.should.have.enumerable('0').with.property('errorType', 'ValidationError'); + done(); + }); + }); + }); + + describe('validateOptions', function () { + var valid, invalid; + + function check(key, valid, invalid) { + _.each(valid, function (value) { + var options = {}; + options[key] = value; + apiUtils.validateOptions(options).should.eql([]); + }); + + _.each(invalid, function (value) { + var options = {}, errors; + options[key] = value; + + errors = apiUtils.validateOptions(options); + errors.should.be.an.Array.and.have.lengthOf(1); + errors.should.have.enumerable('0').with.property('errorType', 'ValidationError'); + }); + } + + it('can validate `id`', function () { + valid = [1, '1', 304, '304']; + invalid = ['test', 'de305d54']; + + check('id', valid, invalid); + }); + + it('can validate `uuid`', function () { + valid = ['de305d54-75b4-431b-adb2-eb6b9e546014']; + invalid = ['de305d54-75b4-431b-adb2']; + + check('uuid', valid, invalid); + }); + + it('can validate `page`', function () { + valid = [1, '1', 304, '304']; + invalid = ['me', 'test', 'de305d54', -1, '-1']; + + check('page', valid, invalid); + }); + + it('can validate `limit`', function () { + valid = [1, '1', 304, '304', 'all']; + invalid = ['me', 'test', 'de305d54', -1, '-1']; + + check('limit', valid, invalid); + }); + + it('can validate `slug` or `status` or `author` etc as a-z, 0-9 and -', function () { + valid = ['hello-world', 'hello', '1-2-3', 1, '-1', -1]; + invalid = ['hello_world', '!things', '?other-things', 'thing"', '`ticks`']; + + check('slug', valid, invalid); + check('status', valid, invalid); + check('author', valid, invalid); + }); + + it('gives no errors for `context`, `include` and `data`', function () { + apiUtils.validateOptions({ + context: {user: 1}, + include: '"super,@random!,string?and', + data: {object: 'thing'} + }).should.eql([]); + }); }); describe('prepareInclude', function () {