canThis() improvements

- Handle passing undefined user to canThis
  - Add existence check to parseContext if statement
  - Add unit test that passes undefined to canThis
- Allow internal canThis() checks
  - Allow passing 'internal' or { internal: true } as context
  - Do not lookup user permissions unless context.user found
  - If context.internal, resolve immediately
  - Add unit tests for passing 'internal' and { internal: true }

core/server/api/posts.js

@@ -21,7 +21,6 @@ posts = {
         options = options || {};
 
         // **returns:** a promise for a page of posts in a json object
-
         return dataProvider.Post.findPage(options).then(function (result) {
             var i = 0,
                 omitted = result;
@@ -37,7 +36,6 @@ posts = {
     // **takes:** an identifier (id or slug?)
     read: function read(args) {
         // **returns:** a promise for a single post in a json object
-
         return dataProvider.Post.findOne(args).then(function (result) {
             var omitted;
 
@@ -64,11 +62,7 @@ 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
-        if (!this.user) {
-            return when.reject({code: 403, message: 'You do not have permission to edit this post.'});
-        }
-        var self = this;
-        return canThis(self.user).edit.post(postData.id).then(function () {
+        return canThis(this.user).edit.post(postData.id).then(function () {
             return checkPostData(postData).then(function (checkedPostData) {
                 return dataProvider.Post.edit(checkedPostData.posts[0]);
             }).then(function (result) {
@@ -88,10 +82,6 @@ posts = {
     // **takes:** a json object representing a post,
     add: function add(postData) {
         // **returns:** a promise for the resulting post in a json object
-        if (!this.user) {
-            return when.reject({code: 403, message: 'You do not have permission to add posts.'});
-        }
-
         return canThis(this.user).create.post().then(function () {
             return checkPostData(postData).then(function (checkedPostData) {
                 return dataProvider.Post.add(checkedPostData.posts[0]);
@@ -109,10 +99,6 @@ 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
-        if (!this.user) {
-            return when.reject({code: 403, message: 'You do not have permission to remove posts.'});
-        }
-
         return canThis(this.user).remove.post(args.id).then(function () {
             return posts.read({id : args.id, status: 'all'}).then(function (result) {
                 return dataProvider.Post.destroy(args.id).then(function () {

core/server/permissions/index.js

@@ -26,14 +26,21 @@ function hasActionsMap() {
 function parseContext(context) {
     // Parse what's passed to canThis.beginCheck for standard user and app scopes
     var parsed = {
+            internal: false,
             user: null,
             app: null
         };
     
-    // Handle legacy passing of just userId or user model first
-    if (context.id) {
+    if (context && (context === 'internal' || context.internal)) {
+        parsed.internal = true;
+    }
+
+    // @TODO: Refactor canThis() references to pass { user: id } explicitly instead of primitives.
+    if (context && context.id) {
+        // Handle passing of just user.id string
         parsed.user = context.id;
     } else if (_.isNumber(context)) {
+        // Handle passing of just user id number
         parsed.user = context;
     } else if (_.isObject(context)) {
         // Otherwise, use the new hotness { user: id, app: id } format
@@ -60,6 +67,11 @@ CanThisResult.prototype.buildObjectTypeHandlers = function (obj_types, act_type,
         obj_type_handlers[obj_type] = function (modelOrId) {
             var modelId;
 
+            // If it's an internal request, resolve immediately
+            if (context.internal) {
+                return when.resolve();
+            }
+
             if (_.isNumber(modelOrId) || _.isString(modelOrId)) {
                 // It's an id already, do nothing
                 modelId = modelOrId;
@@ -153,8 +165,14 @@ CanThisResult.prototype.beginCheck = function (context) {
         throw new Error("No actions map found, please call permissions.init() before use.");
     }
 
-    // Kick off loading of effective user permissions
+    // Kick off loading of effective user permissions if necessary
+    if (context.user) {
         userPermissionLoad = effectivePerms.user(context.user);
+    } else {
+        // Resolve null if no context.user to prevent db call
+        userPermissionLoad = when.resolve(null);
+    }
+    
 
     // Kick off loading of effective app permissions if necessary
     if (context.app) {
@@ -164,6 +182,7 @@ CanThisResult.prototype.beginCheck = function (context) {
         appPermissionLoad = when.resolve(null);
     }
 
+    // Wait for both user and app permissions to load
     permissionsLoad = when.all([userPermissionLoad, appPermissionLoad]).then(function (result) {
         return {
             user: result[0],

core/test/unit/permissions_spec.js

@@ -310,7 +310,7 @@ describe('Permissions', function () {
             })
             .otherwise(function () {
                 permissableStub.restore();
-                permissableStub.calledWith(123, { user: testUser.id, app: null }, 'edit').should.equal(true);
+                permissableStub.calledWith(123, { user: testUser.id, app: null, internal: false }, 'edit').should.equal(true);
                 done();
             });
     });
@@ -336,6 +336,7 @@ describe('Permissions', function () {
             })
             .otherwise(done);
     });
+
     it('does not allow an app to edit a post without permission', function (done) {
         // Change the author of the post so the author override doesn't affect the test
         PostProvider.edit({id: 1, 'author_id': 2})
@@ -386,6 +387,7 @@ describe('Permissions', function () {
                     });
             }).otherwise(done);
     });
+
     it('allows an app to edit a post with permission', function (done) {
         permissions.canThis({ app: 'Kudos', user: 1 })
             .edit
@@ -397,4 +399,42 @@ describe('Permissions', function () {
                 done(new Error("Allowed an edit of post 1"));
             });
     });
+
+    it('checks for null context passed and rejects', function (done) {
+        permissions.canThis(undefined)
+            .edit
+            .post(1)
+            .then(function () {
+                done(new Error("Should not allow editing post"));
+            })
+            .otherwise(function () {
+                done();
+            });
+    });
+
+    it('allows \'internal\' to be passed for internal requests', function (done) {
+        // Using tag here because post implements the custom permissable interface
+        permissions.canThis('internal')
+            .edit
+            .tag(1)
+            .then(function () {
+                done();
+            })
+            .otherwise(function () {
+                done(new Error("Should allow editing post with 'internal'"));
+            });
+    });
+
+    it('allows { internal: true } to be passed for internal requests', function (done) {
+        // Using tag here because post implements the custom permissable interface
+        permissions.canThis({ internal: true })
+            .edit
+            .tag(1)
+            .then(function () {
+                done();
+            })
+            .otherwise(function () {
+                done(new Error("Should allow editing post with { internal: true }"));
+            });
+    });
 });