From bbe51050481636508780349f536e59e295e20d2d Mon Sep 17 00:00:00 2001 From: Jacob Gable Date: Thu, 15 Aug 2013 18:22:08 -0500 Subject: [PATCH] Edit Post Permissions --- core/client/views/base.js | 35 ++++++++++++++++++++++++--- core/client/views/editor.js | 9 ++++--- core/ghost.js | 8 +++++- core/server/api.js | 32 +++++++++++++++++++++--- core/server/models/post.js | 20 +++++++++++---- core/server/models/user.js | 39 +++++++++++++++--------------- core/server/permissions/index.js | 14 +++++++++-- core/test/unit/permissions_spec.js | 2 +- 8 files changed, 121 insertions(+), 38 deletions(-) diff --git a/core/client/views/base.js b/core/client/views/base.js index aab620edfc..fb93786a7e 100644 --- a/core/client/views/base.js +++ b/core/client/views/base.js @@ -54,13 +54,14 @@ // by `addSubview`, which will in-turn remove any // children of those views, and so on. removeSubviews: function () { - var i, l, children = this.subviews; + var children = this.subviews; + if (!children) { return this; } - for (i = 0, l = children.length; i < l; i += 1) { - children[i].remove(); - } + + _(children).invoke("remove"); + this.subviews = []; return this; }, @@ -72,6 +73,32 @@ this.removeSubviews(); } return Backbone.View.prototype.remove.apply(this, arguments); + }, + + // Used in API request fail handlers to parse a standard api error + // response json for the message to display + getRequestErrorMessage: function (request) { + var message; + + // Can't really continue without a request + if (!request) { + return null; + } + + // Seems like a sensible default + message = request.statusText; + + // If a non 200 response + if (request.status !== 200) { + try { + // Try to parse out the error, or default to "Unknown" + message = request.responseJSON.error || "Unknown Error"; + } catch (e) { + message = "The server returned an error (" + (request.status || "?") + ")."; + } + } + + return message; } }); diff --git a/core/client/views/editor.js b/core/client/views/editor.js index 11c1c82a00..0029e7d15e 100644 --- a/core/client/views/editor.js +++ b/core/client/views/editor.js @@ -157,17 +157,20 @@ if (e) { e.preventDefault(); } - var model = this.model; + var view = this, + model = this.model; this.savePost().then(function () { Ghost.notifications.addItem({ type: 'success', message: 'Your post was saved as ' + model.get('status'), status: 'passive' }); - }, function () { + }, function (request) { + var message = view.getRequestErrorMessage(request) || model.validationError; + Ghost.notifications.addItem({ type: 'error', - message: model.validationError, + message: message, status: 'passive' }); }); diff --git a/core/ghost.js b/core/ghost.js index afd8315928..4d679ef490 100644 --- a/core/ghost.js +++ b/core/ghost.js @@ -15,6 +15,7 @@ var config = require('./../config'), models = require('./server/models'), plugins = require('./server/plugins'), requireTree = require('./server/require-tree'), + permissions = require('./server/permissions'), // Variables appRoot = path.resolve(__dirname, '../'), @@ -124,9 +125,14 @@ Ghost.prototype.init = function () { var self = this; return when.join(instance.dataProvider.init(), instance.getPaths()).then(function () { + // Initialize plugins return self.initPlugins(); - }, errors.logAndThrowError).then(function () { + }).then(function () { + // Initialize the settings cache return self.updateSettingsCache(); + }).then(function () { + // Initialize the permissions actions and objects + return permissions.init(); }, errors.logAndThrowError); }; diff --git a/core/server/api.js b/core/server/api.js index 7dc8946309..1ec46fcccb 100644 --- a/core/server/api.js +++ b/core/server/api.js @@ -5,6 +5,8 @@ var Ghost = require('../ghost'), _ = require('underscore'), when = require('when'), errors = require('./errorHandling'), + permissions = require('./permissions'), + canThis = permissions.canThis, ghost = new Ghost(), dataProvider = ghost.dataProvider, @@ -40,7 +42,15 @@ posts = { // **takes:** a json object with all the properties which should be updated edit: function edit(postData) { // **returns:** a promise for the resulting post in a json object - return dataProvider.Post.edit(postData); + if (!this.user) { + return when.reject("You do not have permission to edit this post."); + } + + return canThis(this.user).edit.post(postData.id).then(function () { + return dataProvider.Post.edit(postData); + }, function () { + return when.reject("You do not have permission to edit this post."); + }); }, // #### Add @@ -48,7 +58,15 @@ posts = { // **takes:** a json object representing a post, add: function add(postData) { // **returns:** a promise for the resulting post in a json object - return dataProvider.Post.add(postData); + if (!this.user) { + return when.reject("You do not have permission to add posts."); + } + + return canThis(this.user).create.post().then(function () { + return dataProvider.Post.add(postData); + }, function () { + return when.reject("You do not have permission to add posts."); + }); }, // #### Destroy @@ -56,7 +74,15 @@ posts = { // **takes:** an identifier (id or slug?) destroy: function destroy(args) { // **returns:** a promise for a json response with the id of the deleted post - return dataProvider.Post.destroy(args.id); + if (!this.user) { + return when.reject("You do not have permission to remove posts."); + } + + return canThis(this.user).remove.post(args.id).then(function () { + return dataProvider.Post.destroy(args.id); + }, function () { + return when.reject("You do not have permission to remove posts."); + }); } }; diff --git a/core/server/models/post.js b/core/server/models/post.js index 4b590e71a3..2d96d48b04 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -220,20 +220,30 @@ Post = GhostBookshelf.Model.extend({ }, errors.logAndThrowError); } - // TODO: This logic is temporary, will probably need to be updated - + // Check if any permissions apply for this user and post. hasPermission = _.any(userPermissions, function (perm) { - if (perm.get('object_type') !== 'post') { + // Check for matching action type and object type + if (perm.get('action_type') !== action_type || + perm.get('object_type') !== 'post') { return false; } - // True, if no object_id specified, or it matches + // If asking whether we can create posts, + // and we have a create posts permission then go ahead and say yes + if (action_type === 'create' && perm.get('action_type') === action_type) { + return true; + } + + // Check for either no object id or a matching one return !perm.get('object_id') || perm.get('object_id') === postModel.id; }); // If this is the author of the post, allow it. - hasPermission = hasPermission || userId === postModel.get('author_id'); + // Moved below the permissions checks because there may not be a postModel + // in the case like canThis(user).create.post() + hasPermission = hasPermission || (postModel && userId === postModel.get('author_id')); + // Resolve if we have appropriate permissions if (hasPermission) { return when.resolve(); } diff --git a/core/server/models/user.js b/core/server/models/user.js index 19c7ac978b..a23aafe78e 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -54,15 +54,9 @@ User = GhostBookshelf.Model.extend({ */ add: function (_user) { - var User = this, - // Clone the _user so we don't expose the hashed password unnecessarily - userData = _.extend({}, _user), - fail = false, - userRoles = { - - "role_id": 1, - "user_id": 1 - }; + var self = this, + // Clone the _user so we don't expose the hashed password unnecessarily + userData = _.extend({}, _user); /** * This only allows one user to be added to the database, otherwise fails. @@ -70,20 +64,27 @@ User = GhostBookshelf.Model.extend({ * @author javorszky */ return this.forge().fetch().then(function (user) { - + // Check if user exists if (user) { - fail = true; - } - - if (fail) { return when.reject(new Error('A user is already registered. Only one user for now!')); } - return nodefn.call(bcrypt.hash, _user.password, null, null).then(function (hash) { - userData.password = hash; - GhostBookshelf.Model.add.call(UserRole, userRoles); - return GhostBookshelf.Model.add.call(User, userData); - }, errors.logAndThrowError); + // Hash the provided password with bcrypt + return nodefn.call(bcrypt.hash, _user.password, null, null); + }).then(function (hash) { + // Assign the hashed password + userData.password = hash; + // Save the user with the hashed password + return GhostBookshelf.Model.add.call(self, userData); + }).then(function (addedUser) { + // Assign the userData to our created user so we can pass it back + userData = addedUser; + + // Add this user to the admin role (assumes admin = role_id: 1) + return UserRole.add({role_id: 1, user_id: addedUser.id}); + }).then(function (addedUserRole) { + // Return the added user as expected + return when.resolve(userData); }, errors.logAndThrowError); /** diff --git a/core/server/permissions/index.js b/core/server/permissions/index.js index f91172724f..fbd10a39ff 100644 --- a/core/server/permissions/index.js +++ b/core/server/permissions/index.js @@ -13,6 +13,14 @@ var _ = require('underscore'), CanThisResult, exported; +function hasActionsMap() { + // Just need to find one key in the actionsMap + + return _.any(exported.actionsMap, function (val, key) { + return Object.hasOwnProperty(key); + }); +} + // Base class for canThis call results CanThisResult = function () { this.userPermissionLoad = false; @@ -98,14 +106,16 @@ CanThisResult.prototype.beginCheck = function (user) { var self = this, userId = user.id || user; + if (!hasActionsMap()) { + throw new Error("No actions map found, please call permissions.init() before use."); + } + // TODO: Switch logic based on object type; user, role, post. // Kick off the fetching of the user data this.userPermissionLoad = UserProvider.effectivePermissions(userId); // Iterate through the actions and their related object types - // We should have loaded these through a permissions.init() call previously - // TODO: Throw error if not init() yet? _.each(exported.actionsMap, function (obj_types, act_type) { // Build up the object type handlers; // the '.post()' parts in canThis(user).edit.post() diff --git a/core/test/unit/permissions_spec.js b/core/test/unit/permissions_spec.js index 71ee254044..cadfe06c08 100644 --- a/core/test/unit/permissions_spec.js +++ b/core/test/unit/permissions_spec.js @@ -224,7 +224,7 @@ describe('permissions', function () { // TODO: Verify updatedUser.related('permissions') has the permission? - var canThisResult = permissions.canThis(updatedUser); + var canThisResult = permissions.canThis(updatedUser.id); should.exist(canThisResult.edit); should.exist(canThisResult.edit.post);