From 51ac3f653251f59ace09255f8851fc5ee6b5759a Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Mon, 22 Jun 2015 21:11:35 +0100 Subject: [PATCH] Refactor to using pipeline for the API refs #2758 - Post, Tag & User API methods are refactored to use pipeline - Each functional code block is a named task function - Each function takes options, manipulates it, and returns options back - Tasks like permissions can reject if they don't pass, causing the pipeline to fail - Tasks like validating and converting options might be abstracted out into utils - the same for each endpoint - Tasks like the data call can be extremely complex if needs be (like for some user endpoints) - Option validation is mostly factored out to utils - Option conversion is factored out to utils - API utils have 100% test coverage - Minor updates to inline docs, more to do here --- core/server/api/posts.js | 269 ++++++++++------ core/server/api/tags.js | 248 ++++++++++----- core/server/api/users.js | 511 +++++++++++++++++++++---------- core/server/api/utils.js | 54 +++- core/test/unit/api_utils_spec.js | 249 +++++++++++++++ 5 files changed, 993 insertions(+), 338 deletions(-) create mode 100644 core/test/unit/api_utils_spec.js diff --git a/core/server/api/posts.js b/core/server/api/posts.js index 2403e592ba..51ca23a26d 100644 --- a/core/server/api/posts.js +++ b/core/server/api/posts.js @@ -6,28 +6,20 @@ var Promise = require('bluebird'), canThis = require('../permissions').canThis, errors = require('../errors'), utils = require('./utils'), + pipeline = require('../utils/pipeline'), docName = 'posts', allowedIncludes = ['created_by', 'updated_by', 'published_by', 'author', 'tags', 'fields', 'next', 'previous'], posts; -// ## Helpers -function prepareInclude(include) { - include = include || ''; - include = _.intersection(include.split(','), allowedIncludes); - - return include; -} - /** - * ## Posts API Methods + * ### Posts API Methods * * **See:** [API Methods](index.js.html#api%20methods) */ posts = { - /** - * ### Browse + * ## Browse * Find a paginated set of posts * * Will only return published posts unless we have an authenticated user and an alternative status @@ -35,50 +27,86 @@ posts = { * * Will return without static pages unless told otherwise * - * Can return posts for a particular tag by passing a tag slug in * * @public * @param {{context, page, limit, status, staticPages, tag, featured}} options (optional) - * @returns {Promise(Posts)} Posts Collection with Meta + * @returns {Promise} Posts Collection with Meta */ browse: function browse(options) { - options = options || {}; + var tasks; - if (!(options.context && options.context.user)) { - options.status = 'published'; + /** + * ### Handle Permissions + * We need to either be an authorised user, or only return published posts. + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + if (!(options.context && options.context.user)) { + options.status = 'published'; + } + + return options; } - if (options.include) { - options.include = prepareInclude(options.include); + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function modelQuery(options) { + return dataProvider.Post.findPage(options); } - return dataProvider.Post.findPage(options); + // Push all of our tasks into a `tasks` array in the correct order + 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, options); }, /** - * ### Read + * ## Read * Find a post, by ID, UUID, or Slug * * @public - * @param {{id_or_slug (required), context, status, include, ...}} options - * @return {Promise(Post)} Post + * @param {Object} options + * @return {Promise} Post */ read: function read(options) { var attrs = ['id', 'slug', 'status', 'uuid'], - data = _.pick(options, attrs); + tasks; - options = _.omit(options, attrs); - - // only published posts if no user is present - if (!data.uuid && !(options.context && options.context.user)) { - data.status = 'published'; + /** + * ### Handle Permissions + * We need to either be an authorised user, or only return published posts. + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + if (!options.data.uuid && !(options.context && options.context.user)) { + options.data.status = 'published'; + } + return options; } - if (options.include) { - options.include = prepareInclude(options.include); + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function modelQuery(options) { + return dataProvider.Post.findOne(options.data, _.omit(options, ['data'])); } - return dataProvider.Post.findOne(data, options).then(function (result) { + // Push all of our tasks into a `tasks` array in the correct order + tasks = [utils.validate(docName, 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) { + // @TODO make this a formatResponse task? if (result) { return {posts: [result.toJSON(options)]}; } @@ -88,7 +116,7 @@ posts = { }, /** - * ### Edit + * ## Edit * Update properties of a post * * @public @@ -97,34 +125,54 @@ posts = { * @return {Promise(Post)} Edited Post */ edit: function edit(object, options) { - return canThis(options.context).edit.post(options.id).then(function () { - return utils.checkObject(object, docName, options.id).then(function (checkedPostData) { - if (options.include) { - options.include = prepareInclude(options.include); - } + var tasks; - return dataProvider.Post.edit(checkedPostData.posts[0], options); - }).then(function (result) { - if (result) { - var post = result.toJSON(options); - - // If previously was not published and now is (or vice versa), signal the change - post.statusChanged = false; - if (result.updated('status') !== result.get('status')) { - post.statusChanged = true; - } - return {posts: [post]}; - } - - return Promise.reject(new errors.NotFoundError('Post not found.')); + /** + * ### Handle Permissions + * We need to be an authorised user to perform this action + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + return canThis(options.context).edit.post(options.id).then(function permissionGranted() { + return options; + }).catch(function handleError(error) { + return errors.handleAPIError(error, 'You do not have permission to edit posts.'); }); - }, function () { - return Promise.reject(new errors.NoPermissionError('You do not have permission to edit posts.')); + } + + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function modelQuery(options) { + return dataProvider.Post.edit(options.data.posts[0], _.omit(options, ['data'])); + } + + // Push all of our tasks into a `tasks` array in the correct order + 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) { + if (result) { + var post = result.toJSON(options); + + // If previously was not published and now is (or vice versa), signal the change + post.statusChanged = false; + if (result.updated('status') !== result.get('status')) { + post.statusChanged = true; + } + return {posts: [post]}; + } + + return Promise.reject(new errors.NotFoundError('Post not found.')); }); }, /** - * ### Add + * ## Add * Create a new post along with any tags * * @public @@ -133,31 +181,49 @@ posts = { * @return {Promise(Post)} Created Post */ add: function add(object, options) { - options = options || {}; + var tasks; - return canThis(options.context).add.post().then(function () { - return utils.checkObject(object, docName).then(function (checkedPostData) { - if (options.include) { - options.include = prepareInclude(options.include); - } - - return dataProvider.Post.add(checkedPostData.posts[0], options); - }).then(function (result) { - var post = result.toJSON(options); - - if (post.status === 'published') { - // When creating a new post that is published right now, signal the change - post.statusChanged = true; - } - return {posts: [post]}; + /** + * ### Handle Permissions + * We need to be an authorised user to perform this action + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + return canThis(options.context).add.post().then(function permissionGranted() { + return options; + }).catch(function () { + return Promise.reject(new errors.NoPermissionError('You do not have permission to add posts.')); }); - }, function () { - return Promise.reject(new errors.NoPermissionError('You do not have permission to add posts.')); + } + + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function modelQuery(options) { + return dataProvider.Post.add(options.data.posts[0], _.omit(options, ['data'])); + } + + // Push all of our tasks into a `tasks` array in the correct order + 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) { + var post = result.toJSON(options); + + if (post.status === 'published') { + // When creating a new post that is published right now, signal the change + post.statusChanged = true; + } + return {posts: [post]}; }); }, /** - * ### Destroy + * ## Destroy * Delete a post, cleans up tag relations, but not unused tags * * @public @@ -165,26 +231,53 @@ posts = { * @return {Promise(Post)} Deleted Post */ destroy: function destroy(options) { - return canThis(options.context).destroy.post(options.id).then(function () { - var readOptions = _.extend({}, options, {status: 'all'}); - return posts.read(readOptions).then(function (result) { + var tasks; + + /** + * ### Handle Permissions + * We need to be an authorised user to perform this action + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + return canThis(options.context).destroy.post(options.id).then(function permissionGranted() { + options.status = 'all'; + return options; + }).catch(function handleError(error) { + return errors.handleAPIError(error, 'You do not have permission to remove posts.'); + }); + } + + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function modelQuery(options) { + return posts.read(options).then(function (result) { return dataProvider.Post.destroy(options).then(function () { - var deletedObj = result; - - if (deletedObj.posts) { - _.each(deletedObj.posts, function (post) { - post.statusChanged = true; - }); - } - - return deletedObj; + return result; }); }); - }, function () { - return Promise.reject(new errors.NoPermissionError('You do not have permission to remove posts.')); + } + + // Push all of our tasks into a `tasks` array in the correct order + 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, options).then(function formatResponse(result) { + var deletedObj = result; + + if (deletedObj.posts) { + _.each(deletedObj.posts, function (post) { + post.statusChanged = true; + }); + } + + return deletedObj; }); } - }; module.exports = posts; diff --git a/core/server/api/tags.js b/core/server/api/tags.js index 282300ea4d..821056af98 100644 --- a/core/server/api/tags.js +++ b/core/server/api/tags.js @@ -6,148 +6,238 @@ var Promise = require('bluebird'), dataProvider = require('../models'), errors = require('../errors'), utils = require('./utils'), + pipeline = require('../utils/pipeline'), docName = 'tags', allowedIncludes = ['post_count'], tags; -// ## Helpers -function prepareInclude(include) { - include = include || ''; - include = _.intersection(include.split(','), allowedIncludes); - - return include; -} - /** - * ## Tags API Methods + * ### Tags API Methods * * **See:** [API Methods](index.js.html#api%20methods) */ tags = { /** - * ### Browse + * ## Browse * @param {{context}} options - * @returns {Promise(Tags)} Tags Collection + * @returns {Promise} Tags Collection */ browse: function browse(options) { - options = options || {}; + var tasks; - return canThis(options.context).browse.tag().then(function () { - if (options.include) { - options.include = prepareInclude(options.include); - } + /** + * ### Handle Permissions + * We need to be an authorised user to perform this action + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + return canThis(options.context).browse.tag().then(function permissionGranted() { + return options; + }).catch(function handleError(error) { + return errors.handleAPIError(error, 'You do not have permission to browse tags.'); + }); + } + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function doQuery(options) { return dataProvider.Tag.findPage(options); - }, function () { - return Promise.reject(new errors.NoPermissionError('You do not have permission to browse tags.')); - }); + } + + // Push all of our tasks into a `tasks` array in the correct order + 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, options); }, /** - * ### Read + * ## Read * @param {{id}} options - * @return {Promise(Tag)} Tag + * @return {Promise} Tag */ read: function read(options) { - options = options || {}; - var attrs = ['id', 'slug'], - data = _.pick(options, attrs); + tasks; - return canThis(options.context).read.tag().then(function () { - if (options.include) { - options.include = prepareInclude(options.include); + /** + * ### Handle Permissions + * We need to be an authorised user to perform this action + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + return canThis(options.context).read.tag().then(function permissionGranted() { + return options; + }).catch(function handleError(error) { + return errors.handleAPIError(error, 'You do not have permission to read tags.'); + }); + } + + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function doQuery(options) { + return dataProvider.Tag.findOne(options.data, _.omit(options, ['data'])); + } + + // Push all of our tasks into a `tasks` array in the correct order + tasks = [utils.validate(docName, 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) { + if (result) { + return {tags: [result.toJSON(options)]}; } - return dataProvider.Tag.findOne(data, options).then(function (result) { - if (result) { - return {tags: [result.toJSON(options)]}; - } - - return Promise.reject(new errors.NotFoundError('Tag not found.')); - }); - }, function () { - return Promise.reject(new errors.NoPermissionError('You do not have permission to read tags.')); + return Promise.reject(new errors.NotFoundError('Tag not found.')); }); }, /** - * ### Add tag + * ## Add * @param {Tag} object the tag to create * @returns {Promise(Tag)} Newly created Tag */ add: function add(object, options) { - options = options || {}; + var tasks; - return canThis(options.context).add.tag(object).then(function () { - if (options.include) { - options.include = prepareInclude(options.include); - } - - return utils.checkObject(object, docName).then(function (checkedTagData) { - return dataProvider.Tag.add(checkedTagData.tags[0], options); - }).then(function (result) { - var tag = result.toJSON(options); - - return {tags: [tag]}; + /** + * ### Handle Permissions + * We need to be an authorised user to perform this action + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + return canThis(options.context).add.tag(options.data).then(function permissionGranted() { + return options; + }).catch(function handleError(error) { + return errors.handleAPIError(error, 'You do not have permission to add tags.'); }); - }, function () { - return Promise.reject(new errors.NoPermissionError('You do not have permission to add tags.')); + } + + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function doQuery(options) { + return dataProvider.Tag.add(options.data.tags[0], _.omit(options, ['data'])); + } + + // Push all of our tasks into a `tasks` array in the correct order + 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) { + var tag = result.toJSON(options); + + return {tags: [tag]}; }); }, /** - * ### edit tag + * ## Edit * * @public * @param {Tag} object Tag or specific properties to update - * @param {{id (required), context, include,...}} options - * @return {Promise(Tag)} Edited Tag + * @param {{id, context, include}} options + * @return {Promise} Edited Tag */ edit: function edit(object, options) { - options = options || {}; + var tasks; - return canThis(options.context).edit.tag(options.id).then(function () { - if (options.include) { - options.include = prepareInclude(options.include); + /** + * ### Handle Permissions + * We need to be an authorised user to perform this action + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + return canThis(options.context).edit.tag(options.id).then(function permissionGranted() { + return options; + }).catch(function handleError(error) { + return errors.handleAPIError(error, 'You do not have permission to edit tags.'); + }); + } + + /** + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function doQuery(options) { + return dataProvider.Tag.edit(options.data.tags[0], _.omit(options, ['data'])); + } + + // Push all of our tasks into a `tasks` array in the correct order + 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) { + if (result) { + var tag = result.toJSON(options); + + return {tags: [tag]}; } - return utils.checkObject(object, docName, options.id).then(function (checkedTagData) { - return dataProvider.Tag.edit(checkedTagData.tags[0], options); - }).then(function (result) { - if (result) { - var tag = result.toJSON(options); - - return {tags: [tag]}; - } - - return Promise.reject(new errors.NotFoundError('Tag not found.')); - }); - }, function () { - return Promise.reject(new errors.NoPermissionError('You do not have permission to edit tags.')); + return Promise.reject(new errors.NotFoundError('Tag not found.')); }); }, /** - * ### Destroy + * ## Destroy * * @public - * @param {{id (required), context,...}} options - * @return {Promise(Tag)} Deleted Tag + * @param {{id, context}} options + * @return {Promise} Deleted Tag */ destroy: function destroy(options) { - options = options || {}; + var tasks; - return canThis(options.context).destroy.tag(options.id).then(function () { + /** + * ### Handle Permissions + * We need to be an authorised user to perform this action + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + return canThis(options.context).destroy.tag(options.id).then(function permissionGranted() { + return options; + }).catch(function handleError(error) { + return errors.handleAPIError(error, 'You do not have permission to remove tags.'); + }); + } + + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function doQuery(options) { return tags.read(options).then(function (result) { return dataProvider.Tag.destroy(options).then(function () { return result; }); }); - }, function () { - return Promise.reject(new errors.NoPermissionError('You do not have permission to remove tags.')); - }); + } + + // Push all of our tasks into a `tasks` array in the correct order + 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, options); } }; diff --git a/core/server/api/users.js b/core/server/api/users.js index f0be0fb8f5..ed317cf34c 100644 --- a/core/server/api/users.js +++ b/core/server/api/users.js @@ -10,6 +10,7 @@ var Promise = require('bluebird'), globalUtils = require('../utils'), config = require('../config'), mail = require('./mail'), + pipeline = require('../utils/pipeline'), docName = 'users', // TODO: implement created_by, updated_by @@ -17,13 +18,6 @@ var Promise = require('bluebird'), users, sendInviteEmail; -// ## Helpers -function prepareInclude(include) { - include = include || ''; - include = _.intersection(include.split(','), allowedIncludes); - return include; -} - sendInviteEmail = function sendInviteEmail(user) { var emailData; @@ -67,50 +61,89 @@ sendInviteEmail = function sendInviteEmail(user) { }); }; /** - * ## Posts API Methods + * ### Users API Methods * * **See:** [API Methods](index.js.html#api%20methods) */ users = { - /** * ## Browse * Fetch all users * @param {{context}} options (optional) - * @returns {Promise(Users)} Users Collection + * @returns {Promise} Users Collection */ browse: function browse(options) { - options = options || {}; - return canThis(options.context).browse.user().then(function () { - if (options.include) { - options.include = prepareInclude(options.include); - } + var tasks; + + /** + * ### Handle Permissions + * We need to either be an authorised user, or only return published posts. + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + return canThis(options.context).browse.user().then(function permissionGranted() { + return options; + }).catch(function handleError(error) { + return errors.handleAPIError(error, 'You do not have permission to browse users.'); + }); + } + + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function doQuery(options) { return dataProvider.User.findPage(options); - }).catch(function (error) { - return errors.handleAPIError(error, 'You do not have permission to browse users.'); - }); + } + + // Push all of our tasks into a `tasks` array in the correct order + 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, options); }, /** - * ### Read + * ## Read * @param {{id, context}} options - * @returns {Promise(User)} User + * @returns {Promise} User */ read: function read(options) { - var attrs = ['id', 'slug', 'email', 'status'], - data = _.pick(options, attrs); + var attrs = ['id', 'slug', 'status', 'email'], + tasks; - options = _.omit(options, attrs); + /** + * ### Handle Permissions + * Convert 'me' safely + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + if (options.data.id === 'me' && options.context && options.context.user) { + options.data.id = options.context.user; + } - if (options.include) { - options.include = prepareInclude(options.include); + return options; } - if (data.id === 'me' && options.context && options.context.user) { - data.id = options.context.user; + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function doQuery(options) { + return dataProvider.User.findOne(options.data, _.omit(options, ['data'])); } - return dataProvider.User.findOne(data, options).then(function (result) { + // Push all of our tasks into a `tasks` array in the correct order + tasks = [utils.validate(docName, 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) { if (result) { return {users: [result.toJSON(options)]}; } @@ -120,42 +153,54 @@ users = { }, /** - * ### Edit + * ## Edit * @param {User} object the user details to edit * @param {{id, context}} options - * @returns {Promise(User)} + * @returns {Promise} */ edit: function edit(object, options) { - var editOperation; - if (options.id === 'me' && options.context && options.context.user) { - options.id = options.context.user; - } - - if (options.include) { - options.include = prepareInclude(options.include); - } - - return utils.checkObject(object, docName, options.id).then(function (data) { - // Edit operation - editOperation = function () { - return dataProvider.User.edit(data.users[0], options) - .then(function (result) { - if (result) { - return {users: [result.toJSON(options)]}; - } - - return Promise.reject(new errors.NotFoundError('User not found.')); - }); - }; - - // Check permissions - return canThis(options.context).edit.user(options.id).then(function () { - // if roles aren't in the payload, proceed with the edit - if (!(data.users[0].roles && data.users[0].roles[0])) { - return editOperation(); + 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 role = data.users[0].roles[0], + options.data = data; + return options; + }); + } + + /** + * ### Handle Permissions + * We need to be an authorised user to perform this action + * Edit user allows the related role object to be updated as well, with some rules: + * - No change permitted to the role of the owner + * - no change permitted to the role of the context user (user making the request) + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + if (options.id === 'me' && options.context && options.context.user) { + options.id = options.context.user; + } + + return canThis(options.context).edit.user(options.id).then(function () { + // if roles aren't in the payload, proceed with the edit + if (!(options.data.users[0].roles && options.data.users[0].roles[0])) { + return options; + } + + // @TODO move role permissions out of here + var role = options.data.users[0].roles[0], roleId = parseInt(role.id || role, 10), editedUserId = parseInt(options.id, 10); @@ -176,95 +221,64 @@ users = { } } else if (roleId !== contextRoleId) { return canThis(options.context).assign.role(role).then(function () { - return editOperation(); + return options; }); } } - return editOperation(); + return options; }); }); + }).catch(function handleError(error) { + return errors.handleAPIError(error, 'You do not have permission to edit this user'); }); - }).catch(function (error) { - return errors.handleAPIError(error, 'You do not have permission to edit this user'); + } + + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function doQuery(options) { + return dataProvider.User.edit(options.data.users[0], _.omit(options, ['data'])); + } + + // Push all of our tasks into a `tasks` array in the correct order + tasks = [validate, handlePermissions, utils.convertOptions(allowedIncludes), doQuery]; + + return pipeline(tasks, object, options).then(function formatResponse(result) { + if (result) { + return {users: [result.toJSON(options)]}; + } + + return Promise.reject(new errors.NotFoundError('User not found.')); }); }, /** - * ### Add user + * ## Add user * The newly added user is invited to join the blog via email. * @param {User} object the user to create - * @returns {Promise(User}} Newly created user + * @param {{context}} options + * @returns {Promise} Newly created user */ add: function add(object, options) { - var addOperation, - newUser, - user; + var tasks; - if (options.include) { - options.include = prepareInclude(options.include); - } - - return utils.checkObject(object, docName).then(function (data) { - newUser = data.users[0]; - - addOperation = function () { - if (newUser.email) { - newUser.name = object.users[0].email.substring(0, newUser.email.indexOf('@')); - newUser.password = globalUtils.uid(50); - newUser.status = 'invited'; - } else { - return Promise.reject(new errors.BadRequestError('No email provided.')); - } - - return dataProvider.User.getByEmail( - newUser.email - ).then(function (foundUser) { - if (!foundUser) { - return dataProvider.User.add(newUser, options); - } else { - // only invitations for already invited users are resent - if (foundUser.get('status') === 'invited' || foundUser.get('status') === 'invited-pending') { - return foundUser; - } else { - return Promise.reject(new errors.BadRequestError('User is already registered.')); - } - } - }).then(function (invitedUser) { - user = invitedUser.toJSON(options); - return sendInviteEmail(user); - }).then(function () { - // If status was invited-pending and sending the invitation succeeded, set status to invited. - if (user.status === 'invited-pending') { - return dataProvider.User.edit( - {status: 'invited'}, _.extend({}, options, {id: user.id}) - ).then(function (editedUser) { - user = editedUser.toJSON(options); - }); - } - }).then(function () { - return Promise.resolve({users: [user]}); - }).catch(function (error) { - if (error && error.errorType === 'EmailError') { - error.message = 'Error sending email: ' + error.message + ' Please check your email settings and resend the invitation.'; - errors.logWarn(error.message); - - // If sending the invitation failed, set status to invited-pending - return dataProvider.User.edit({status: 'invited-pending'}, {id: user.id}).then(function (user) { - return dataProvider.User.findOne({id: user.id, status: 'all'}, options).then(function (user) { - return {users: [user]}; - }); - }); - } - return Promise.reject(error); - }); - }; - - // Check permissions - return canThis(options.context).add.user(object).then(function () { + /** + * ### Handle Permissions + * We need to be an authorised user to perform this action + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + var newUser = options.data.users[0]; + return canThis(options.context).add.user(options.data).then(function () { if (newUser.roles && newUser.roles[0]) { var roleId = parseInt(newUser.roles[0].id || newUser.roles[0], 10); + // @TODO move this logic to permissible // Make sure user is allowed to add a user with this role return dataProvider.Role.findOne({id: roleId}).then(function (role) { if (role.get('name') === 'Owner') { @@ -273,25 +287,114 @@ users = { return canThis(options.context).assign.role(role); }).then(function () { - return addOperation(); + return options; }); } - return addOperation(); + return options; + }).catch(function handleError(error) { + return errors.handleAPIError(error, 'You do not have permission to add this user'); }); - }).catch(function (error) { - return errors.handleAPIError(error, 'You do not have permission to add this user'); - }); + } + + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function doQuery(options) { + var newUser = options.data.users[0], + user; + + if (newUser.email) { + newUser.name = newUser.email.substring(0, newUser.email.indexOf('@')); + newUser.password = globalUtils.uid(50); + newUser.status = 'invited'; + } else { + return Promise.reject(new errors.BadRequestError('No email provided.')); + } + + return dataProvider.User.getByEmail( + newUser.email + ).then(function (foundUser) { + if (!foundUser) { + return dataProvider.User.add(newUser, options); + } else { + // only invitations for already invited users are resent + if (foundUser.get('status') === 'invited' || foundUser.get('status') === 'invited-pending') { + return foundUser; + } else { + return Promise.reject(new errors.BadRequestError('User is already registered.')); + } + } + }).then(function (invitedUser) { + user = invitedUser.toJSON(options); + return sendInviteEmail(user); + }).then(function () { + // If status was invited-pending and sending the invitation succeeded, set status to invited. + if (user.status === 'invited-pending') { + return dataProvider.User.edit( + {status: 'invited'}, _.extend({}, options, {id: user.id}) + ).then(function (editedUser) { + user = editedUser.toJSON(options); + }); + } + }).then(function () { + return Promise.resolve({users: [user]}); + }).catch(function (error) { + if (error && error.errorType === 'EmailError') { + error.message = 'Error sending email: ' + error.message + ' Please check your email settings and resend the invitation.'; + errors.logWarn(error.message); + + // If sending the invitation failed, set status to invited-pending + return dataProvider.User.edit({status: 'invited-pending'}, {id: user.id}).then(function (user) { + return dataProvider.User.findOne({id: user.id, status: 'all'}, options).then(function (user) { + return {users: [user]}; + }); + }); + } + return Promise.reject(error); + }); + } + + // Push all of our tasks into a `tasks` array in the correct order + tasks = [utils.validate(docName), handlePermissions, utils.convertOptions(allowedIncludes), doQuery]; + + return pipeline(tasks, object, options); }, /** - * ### Destroy + * ## Destroy * @param {{id, context}} options - * @returns {Promise(User)} + * @returns {Promise} */ destroy: function destroy(options) { - return canThis(options.context).destroy.user(options.id).then(function () { - return users.read(_.merge(options, {status: 'all'})).then(function (result) { + var tasks; + + /** + * ### Handle Permissions + * We need to be an authorised user to perform this action + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + return canThis(options.context).destroy.user(options.id).then(function permissionGranted() { + options.status = 'all'; + return options; + }).catch(function handleError(error) { + return errors.handleAPIError(error, 'You do not have permission to destroy this user.'); + }); + } + + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function doQuery(options) { + return users.read(options).then(function (result) { return dataProvider.Base.transaction(function (t) { options.transacting = t; @@ -314,54 +417,124 @@ users = { }, function (error) { return errors.handleAPIError(error); }); - }).catch(function (error) { - return errors.handleAPIError(error, 'You do not have permission to destroy this user'); - }); + } + + // Push all of our tasks into a `tasks` array in the correct order + 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, options); }, /** - * ### Change Password + * ## Change Password * @param {password} object * @param {{context}} options - * @returns {Promise(password}} success message + * @returns {Promise} success message */ changePassword: function changePassword(object, options) { - var oldPassword, - newPassword, - ne2Password, - userId; + var tasks; + /** + * ### Validation + * Ensure we have valid options - special validation just for password + * @TODO change User.changePassword to take an object not 4 args + * @param {Object} object + * @param {Object} options + * @returns {Object} options + */ + function validate(object, options) { + options = options || {}; + return utils.checkObject(object, 'password').then(function (data) { + options.data = { + oldPassword: data.password[0].oldPassword, + newPassword: data.password[0].newPassword, + ne2Password: data.password[0].ne2Password, + userId: parseInt(data.password[0].user_id) + }; + return options; + }); + } - return utils.checkObject(object, 'password').then(function (checkedPasswordReset) { - oldPassword = checkedPasswordReset.password[0].oldPassword; - newPassword = checkedPasswordReset.password[0].newPassword; - ne2Password = checkedPasswordReset.password[0].ne2Password; - userId = parseInt(checkedPasswordReset.password[0].user_id); - }).then(function () { - return canThis(options.context).edit.user(userId); - }).then(function () { - return dataProvider.User.changePassword(oldPassword, newPassword, ne2Password, userId, options); - }).then(function () { + /** + * ### Handle Permissions + * We need to be an authorised user to perform this action + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + return canThis(options.context).edit.user(options.data.userId).then(function permissionGranted() { + return options; + }).catch(function (error) { + return errors.handleAPIError(error, 'You do not have permission to change the password for this user'); + }); + } + + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function doQuery(options) { + return dataProvider.User.changePassword( + options.data.oldPassword, + options.data.newPassword, + options.data.ne2Password, + options.data.userId, + _.omit(options, ['data']) + ); + } + + // Push all of our tasks into a `tasks` array in the correct order + tasks = [validate, 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() { return Promise.resolve({password: [{message: 'Password changed successfully.'}]}); - }).catch(function (error) { - // return Promise.reject(new errors.ValidationError(error.message)); - return errors.handleAPIError(error, 'You do not have permission to change the password for this user'); }); }, /** - * + * ## Transfer Ownership + * @param {owner} object + * @param {Object} options + * @returns {Promise} */ transferOwnership: function transferOwnership(object, options) { - return dataProvider.Role.findOne({name: 'Owner'}).then(function (ownerRole) { - return canThis(options.context).assign.role(ownerRole); - }).then(function () { - return utils.checkObject(object, 'owner').then(function (checkedOwnerTransfer) { - return dataProvider.User.transferOwnership(checkedOwnerTransfer.owner[0], options).then(function (updatedUsers) { - return Promise.resolve({users: updatedUsers}); - }).catch(function (error) { - return Promise.reject(new errors.ValidationError(error.message)); - }); + var tasks; + + /** + * ### Handle Permissions + * We need to be an authorised user to perform this action + * @param {Object} options + * @returns {Object} options + */ + function handlePermissions(options) { + return dataProvider.Role.findOne({name: 'Owner'}).then(function (ownerRole) { + return canThis(options.context).assign.role(ownerRole); + }).then(function () { + return options; + }).catch(function (error) { + return errors.handleAPIError(error); }); + } + + /** + * ### Model Query + * Make the call to the Model layer + * @param {Object} options + * @returns {Object} options + */ + function doQuery(options) { + return dataProvider.User.transferOwnership(options.data.owner[0], _.omit(options, ['data'])); + } + + // Push all of our tasks into a `tasks` array in the correct order + 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) { + return Promise.resolve({users: result}); }).catch(function (error) { return errors.handleAPIError(error); }); diff --git a/core/server/api/utils.js b/core/server/api/utils.js index 3165805e51..07808aee3a 100644 --- a/core/server/api/utils.js +++ b/core/server/api/utils.js @@ -7,6 +7,57 @@ var Promise = require('bluebird'), utils; utils = { + validate: function validate(docName, attrs) { + return function doValidate() { + var object, options; + if (arguments.length === 2) { + object = arguments[0]; + options = _.clone(arguments[1]) || {}; + } else if (arguments.length === 1) { + options = _.clone(arguments[0]) || {}; + } else { + options = {}; + } + + if (attrs) { + options.data = _.pick(options, attrs); + options = _.omit(options, attrs); + } + + if (object) { + return utils.checkObject(object, docName, options.id).then(function (data) { + options.data = data; + return Promise.resolve(options); + }); + } + + return Promise.resolve(options); + }; + }, + + prepareInclude: function prepareInclude(include, allowedIncludes) { + include = include || ''; + include = _.intersection(include.split(','), allowedIncludes); + + return include; + }, + /** + * @param {Array} allowedIncludes + * @returns {Function} doConversion + */ + convertOptions: function convertOptions(allowedIncludes) { + /** + * Convert our options from API-style to Model-style + * @param {Object} options + * @returns {Object} options + */ + return function doConversion(options) { + if (options.include) { + options.include = utils.prepareInclude(options.include, allowedIncludes); + } + return options; + }; + }, /** * ### Check Object * Check an object passed to the API is in the correct format @@ -21,7 +72,6 @@ utils = { } // convert author property to author_id to match the name in the database - // TODO: rename object in database if (docName === 'posts') { if (object.posts[0].hasOwnProperty('author')) { object.posts[0].author_id = object.posts[0].author; @@ -36,7 +86,7 @@ utils = { return Promise.resolve(object); }, checkFileExists: function (options, filename) { - return options[filename] && options[filename].type && options[filename].path; + return !!(options[filename] && options[filename].type && options[filename].path); }, checkFileIsValid: function (file, types, extensions) { var type = file.type, diff --git a/core/test/unit/api_utils_spec.js b/core/test/unit/api_utils_spec.js new file mode 100644 index 0000000000..0c90769f2f --- /dev/null +++ b/core/test/unit/api_utils_spec.js @@ -0,0 +1,249 @@ +/*globals describe, it, beforeEach, afterEach */ +/*jshint expr:true*/ +var should = require('should'), + sinon = require('sinon'), + _ = require('lodash'), + Promise = require('bluebird'), + + apiUtils = require('../../server/api/utils'); + +describe('API Utils', function () { + var sandbox; + + beforeEach(function () { + sandbox = sinon.sandbox.create(); + }); + + afterEach(function () { + sandbox.restore(); + }); + + describe('validate', function () { + it('should create options when passed no args', function (done) { + apiUtils.validate()().then(function (options) { + options.should.eql({}); + done(); + }).catch(done); + }); + + it('should pick data attrs when passed them', function (done) { + apiUtils.validate('test', ['id'])( + {id: 'test', status: 'all', uuid: 'other-test'} + ).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'); + done(); + }).catch(done); + }); + + it('should check data if an object is passed', function (done) { + var object = {test: [{id: 1}]}, + checkObjectStub = sandbox.stub(apiUtils, 'checkObject').returns(Promise.resolve(object)); + + apiUtils.validate('test')(object, {}).then(function (options) { + checkObjectStub.calledOnce.should.be.true; + checkObjectStub.calledWith(object, 'test').should.be.true; + options.should.have.ownProperty('data'); + options.data.should.have.ownProperty('test'); + done(); + }).catch(done); + }); + + it('should handle options being undefined', function (done) { + apiUtils.validate()(undefined).then(function (options) { + options.should.eql({}); + done(); + }).catch(done); + }); + + it('should handle options being undefined when provided with object', function (done) { + var object = {test: [{id: 1}]}, + checkObjectStub = sandbox.stub(apiUtils, 'checkObject').returns(Promise.resolve(object)); + + apiUtils.validate('test')(object, undefined).then(function (options) { + checkObjectStub.calledOnce.should.be.true; + checkObjectStub.calledWith(object, 'test').should.be.true; + options.should.have.ownProperty('data'); + options.data.should.have.ownProperty('test'); + done(); + }).catch(done); + }); + }); + + describe('prepareInclude', function () { + it('should handle empty items', function () { + apiUtils.prepareInclude('', []).should.eql([]); + }); + + it('should be empty if there are no allowed includes', function () { + apiUtils.prepareInclude('a,b,c', []).should.eql([]); + }); + + it('should return correct includes', function () { + apiUtils.prepareInclude('a,b,c', ['a']).should.eql(['a']); + apiUtils.prepareInclude('a,b,c', ['a', 'c']).should.eql(['a', 'c']); + apiUtils.prepareInclude('a,b,c', ['a', 'd']).should.eql(['a']); + apiUtils.prepareInclude('a,b,c', ['d']).should.eql([]); + }); + }); + + describe('convertOptions', function () { + it('should not call prepareInclude if there is no include option', function () { + var prepareIncludeStub = sandbox.stub(apiUtils, 'prepareInclude'); + apiUtils.convertOptions(['a', 'b', 'c'])({}).should.eql({}); + prepareIncludeStub.called.should.be.false; + }); + + it('should pass options.include to prepareInclude if provided', function () { + var expectedResult = ['a', 'b'], + prepareIncludeStub = sandbox.stub(apiUtils, 'prepareInclude').returns(expectedResult), + allowed = ['a', 'b', 'c'], + options = {include: 'a,b'}, + actualResult; + actualResult = apiUtils.convertOptions(allowed)(_.clone(options)); + + prepareIncludeStub.calledOnce.should.be.true; + prepareIncludeStub.calledWith(options.include, allowed).should.be.true; + + actualResult.should.have.hasOwnProperty('include'); + actualResult.include.should.be.an.Array; + actualResult.include.should.eql(expectedResult); + }); + }); + + describe('checkObject', function () { + it('throws an error if the object is empty', function (done) { + apiUtils.checkObject({}, 'test').then(function () { + done('This should have thrown an error'); + }).catch(function (error) { + should.exist(error); + error.errorType.should.eql('BadRequestError'); + done(); + }); + }); + + it('throws an error if the object key is empty', function (done) { + apiUtils.checkObject({test: []}, 'test').then(function () { + done('This should have thrown an error'); + }).catch(function (error) { + should.exist(error); + error.errorType.should.eql('BadRequestError'); + done(); + }); + }); + + it('throws an error if the object key is array with empty object', function (done) { + apiUtils.checkObject({test: [{}]}, 'test').then(function () { + done('This should have thrown an error'); + }).catch(function (error) { + should.exist(error); + error.errorType.should.eql('BadRequestError'); + done(); + }); + }); + + it('passed through a simple, correct object', function (done) { + var object = {test: [{id: 1}]}; + apiUtils.checkObject(_.cloneDeep(object), 'test').then(function (data) { + should.exist(data); + data.should.have.ownProperty('test'); + object.should.eql(data); + done(); + }).catch(done); + }); + + it('should do author_id to author conversion for posts', function (done) { + var object = {posts: [{id: 1, author: 4}]}; + apiUtils.checkObject(_.cloneDeep(object), 'posts').then(function (data) { + should.exist(data); + data.should.have.ownProperty('posts'); + data.should.not.eql(object); + data.posts.should.be.an.Array; + data.posts[0].should.have.ownProperty('author_id'); + data.posts[0].should.not.have.ownProperty('author'); + done(); + }).catch(done); + }); + + it('should not do author_id to author conversion for posts if not needed', function (done) { + var object = {posts: [{id: 1, author_id: 4}]}; + apiUtils.checkObject(_.cloneDeep(object), 'posts').then(function (data) { + should.exist(data); + data.should.have.ownProperty('posts'); + data.should.eql(object); + data.posts.should.be.an.Array; + data.posts[0].should.have.ownProperty('author_id'); + data.posts[0].should.not.have.ownProperty('author'); + done(); + }).catch(done); + }); + + it('should throw error if invalid editId if provided', function (done) { + var object = {test: [{id: 1}]}; + apiUtils.checkObject(_.cloneDeep(object), 'test', 3).then(function () { + done('This should have thrown an error'); + }).catch(function (error) { + should.exist(error); + error.errorType.should.eql('BadRequestError'); + done(); + }); + }); + + it('should ignore undefined editId', function (done) { + var object = {test: [{id: 1}]}; + apiUtils.checkObject(_.cloneDeep(object), 'test', undefined).then(function (data) { + should.exist(data); + data.should.eql(object); + done(); + }).catch(done); + }); + + it('should ignore editId if object has no id', function (done) { + var object = {test: [{uuid: 1}]}; + apiUtils.checkObject(_.cloneDeep(object), 'test', 3).then(function (data) { + should.exist(data); + data.should.eql(object); + done(); + }).catch(done); + }); + }); + + describe('checkFileExists', function () { + it('should return true if file exists in input', function () { + apiUtils.checkFileExists({test: {type: 'file', path: 'path'}}, 'test').should.be.true; + }); + + it('should return false if file does not exist in input', function () { + apiUtils.checkFileExists({test: {type: 'file', path: 'path'}}, 'notthere').should.be.false; + }); + + it('should return false if file is incorrectly structured', function () { + apiUtils.checkFileExists({test: 'notafile'}, 'test').should.be.false; + }); + }); + + describe('checkFileIsValid', function () { + it('returns true if file has valid extension and type', function () { + apiUtils.checkFileIsValid({name: 'test.txt', type: 'text'}, ['text'], ['.txt']).should.be.true; + apiUtils.checkFileIsValid({name: 'test.jpg', type: 'jpeg'}, ['text', 'jpeg'], ['.txt', '.jpg']).should.be.true; + }); + + it('returns false if file has invalid extension', function () { + apiUtils.checkFileIsValid({name: 'test.txt', type: 'text'}, ['text'], ['.tar']).should.be.false; + apiUtils.checkFileIsValid({name: 'test', type: 'text'}, ['text'], ['.txt']).should.be.false; + }); + + it('returns false if file has invalid type', function () { + apiUtils.checkFileIsValid({name: 'test.txt', type: 'text'}, ['archive'], ['.txt']).should.be.false; + }); + }); +});