Merge pull request #3343 from sebgie/issue#3087

Add edit roles

core/server/api/users.js

@@ -88,6 +88,11 @@ users = {
         }
 
         return canThis(options.context).edit.user(options.id).then(function () {
+        // TODO: add permission check for roles
+            // if (data.roles) {
+            //     return canThis(options.context).assign.role(<role-id>)
+            // }
+        // }.then(function (){
             return utils.checkObject(object, docName).then(function (checkedUserData) {
 
                 if (options.include) {

core/server/models/user.js

@@ -7,6 +7,7 @@ var _              = require('lodash'),
     http           = require('http'),
     crypto         = require('crypto'),
     validator      = require('validator'),
+    Role           = require('./role').Role,
 
     tokenSecurity  = {},
     User,
@@ -298,10 +299,55 @@ User = ghostBookshelf.Model.extend({
      * **See:** [ghostBookshelf.Model.edit](base.js.html#edit)
      */
     edit: function (data, options) {
+        var self = this,
+            adminRole,
+            ownerRole;
+
         options = options || {};
         options.withRelated = _.union([ 'roles' ], options.include);
 
-        return ghostBookshelf.Model.edit.call(this, data, options);
+        return ghostBookshelf.Model.edit.call(this, data, options).then(function (user) {
+
+            if (data.roles) {
+
+                if (user.id === options.context.user) {
+                    return when.reject(new errors.ValidationError('You are not allowed to assign a new role to yourself'));
+                }
+                if (data.roles.length > 1) {
+                    return when.reject(new errors.ValidationError('Only one role per user is supported at the moment.'));
+                }
+
+                return Role.findOne({id: data.roles[0]}).then(function (role) {
+                    if (role.get('name') === 'Owner') {
+                        // Get admin and owner role
+                        return Role.findOne({name: 'Administrator'}).then(function (result) {
+                            adminRole = result;
+                            return Role.findOne({name: 'Owner'});
+                        }).then(function (result) {
+                            ownerRole = result;
+                            return User.findOne({id: options.context.user});
+                        }).then(function (contextUser) {
+                            // check if user has the owner role
+                            var currentRoles = contextUser.toJSON().roles;
+                            if (!_.contains(currentRoles, ownerRole.id)) {
+                                return when.reject(new errors.ValidationError('Only owners are able to transfer the owner role.'));
+                            }
+                            // convert owner to admin
+                            return contextUser.roles().updatePivot({role_id: adminRole.id});
+                        }).then(function () {
+                            // assign owner role to a new user
+                            return user.roles().updatePivot({role_id: ownerRole.id});
+                        });
+                    } else {
+                        // assign all other roles
+                        return user.roles().updatePivot({role_id: data.roles[0]});
+                    }
+                }).then(function () {
+                    return self.findOne(user, options);
+                });
+            }
+            return user;
+        });
     },
 
     /**

core/server/permissions/effective.js

@@ -14,7 +14,7 @@ var effective = {
                     user = foundUser.toJSON();
 
                 // TODO: using 'Owner' as return value is a bit hacky.
-                if (user.roles[0] && user.roles[0].name === 'Owner') {
+                if (_.find(user.roles, { 'name': 'Owner' })) {
                     return 'Owner';
                 }
 

core/test/integration/api/api_tags_spec.js

@@ -16,6 +16,8 @@ describe('Tags API', function () {
         testUtils.initData()
             .then(function () {
                 return testUtils.insertDefaultFixtures();
+            }).then(function () {
+                return testUtils.insertAdminUser();
             }).then(function () {
                 return testUtils.insertEditorUser();
             }).then(function () {
@@ -41,31 +43,31 @@ describe('Tags API', function () {
         }).catch(done);
     });
 
-    it('can browse (admin)', function (done) {
+    it('can browse (owner)', function (done) {
-            TagAPI.browse({context: {user: 1}}).then(function (results) {
+        TagAPI.browse({context: {user: 1}}).then(function (results) {
-                should.exist(results);
+            should.exist(results);
-                should.exist(results.tags);
+            should.exist(results.tags);
-                results.tags.length.should.be.above(0);
+            results.tags.length.should.be.above(0);
-                testUtils.API.checkResponse(results.tags[0], 'tag');
+            testUtils.API.checkResponse(results.tags[0], 'tag');
-                results.tags[0].created_at.should.be.an.instanceof(Date);
+            results.tags[0].created_at.should.be.an.instanceof(Date);
 
-                done();
+            done();
-            }).catch(done);
+        }).catch(done);
-        });
+    });
+
+    it('can browse (admin)', function (done) {
+        TagAPI.browse({context: {user: 2}}).then(function (results) {
+            should.exist(results);
+            should.exist(results.tags);
+            results.tags.length.should.be.above(0);
+            testUtils.API.checkResponse(results.tags[0], 'tag');
+            results.tags[0].created_at.should.be.an.instanceof(Date);
+
+            done();
+        }).catch(done);
+    });
 
     it('can browse (editor)', function (done) {
-            TagAPI.browse({context: {user: 2}}).then(function (results) {
-                should.exist(results);
-                should.exist(results.tags);
-                results.tags.length.should.be.above(0);
-                testUtils.API.checkResponse(results.tags[0], 'tag');
-                results.tags[0].created_at.should.be.an.instanceof(Date);
-
-                done();
-            }).catch(done);
-        });
-
-    it('can browse (author)', function (done) {
         TagAPI.browse({context: {user: 3}}).then(function (results) {
             should.exist(results);
             should.exist(results.tags);
@@ -76,4 +78,16 @@ describe('Tags API', function () {
             done();
         }).catch(done);
     });
+
+    it('can browse (author)', function (done) {
+        TagAPI.browse({context: {user: 4}}).then(function (results) {
+            should.exist(results);
+            should.exist(results.tags);
+            results.tags.length.should.be.above(0);
+            testUtils.API.checkResponse(results.tags[0], 'tag');
+            results.tags[0].created_at.should.be.an.instanceof(Date);
+
+            done();
+        }).catch(done);
+    });
 });

core/test/integration/api/api_users_spec.js

@@ -53,6 +53,8 @@ describe('Users API', function () {
         beforeEach(function (done) {
             testUtils.initData().then(function () {
                 return testUtils.insertDefaultFixtures();
+            }).then(function () {
+                return testUtils.insertAdminUser();
             }).then(function () {
                 return testUtils.insertEditorUser();
             }).then(function () {
@@ -78,42 +80,59 @@ describe('Users API', function () {
             }).catch(done);
         });
 
-        it('admin can browse', function (done) {
+        it('can browse (owner)', function (done) {
             UsersAPI.browse({context: {user: 1}}).then(function (results) {
                 should.exist(results);
                 testUtils.API.checkResponse(results, 'users');
                 should.exist(results.users);
-                results.users.should.have.length(3);
+                results.users.should.have.length(4);
                 testUtils.API.checkResponse(results.users[0], 'user', ['roles']);
                 testUtils.API.checkResponse(results.users[1], 'user', ['roles']);
                 testUtils.API.checkResponse(results.users[2], 'user', ['roles']);
+                testUtils.API.checkResponse(results.users[3], 'user', ['roles']);
 
                 done();
             }).catch(done);
         });
 
-        it('editor can browse', function (done) {
+        it('can browse (admin)', function (done) {
             UsersAPI.browse({context: {user: 2}}).then(function (results) {
                 should.exist(results);
                 testUtils.API.checkResponse(results, 'users');
                 should.exist(results.users);
-                results.users.should.have.length(3);
+                results.users.should.have.length(4);
                 testUtils.API.checkResponse(results.users[0], 'user', ['roles']);
                 testUtils.API.checkResponse(results.users[1], 'user', ['roles']);
                 testUtils.API.checkResponse(results.users[2], 'user', ['roles']);
+                testUtils.API.checkResponse(results.users[3], 'user', ['roles']);
                 done();
             }).catch(done);
         });
 
-        it('author can browse', function (done) {
+        it('can browse (editor)', function (done) {
             UsersAPI.browse({context: {user: 3}}).then(function (results) {
                 should.exist(results);
                 testUtils.API.checkResponse(results, 'users');
                 should.exist(results.users);
-                results.users.should.have.length(3);
+                results.users.should.have.length(4);
                 testUtils.API.checkResponse(results.users[0], 'user', ['roles']);
                 testUtils.API.checkResponse(results.users[1], 'user', ['roles']);
                 testUtils.API.checkResponse(results.users[2], 'user', ['roles']);
+                testUtils.API.checkResponse(results.users[3], 'user', ['roles']);
+                done();
+            }).catch(done);
+        });
+
+        it('can browse (author)', function (done) {
+            UsersAPI.browse({context: {user: 4}}).then(function (results) {
+                should.exist(results);
+                testUtils.API.checkResponse(results, 'users');
+                should.exist(results.users);
+                results.users.should.have.length(4);
+                testUtils.API.checkResponse(results.users[0], 'user', ['roles']);
+                testUtils.API.checkResponse(results.users[1], 'user', ['roles']);
+                testUtils.API.checkResponse(results.users[2], 'user', ['roles']);
+                testUtils.API.checkResponse(results.users[3], 'user', ['roles']);
                 done();
             }).catch(done);
         });
@@ -126,7 +145,7 @@ describe('Users API', function () {
             }).catch(done);
         });
 
-        it('admin can read', function (done) {
+        it('can read (owner)', function (done) {
             UsersAPI.read({id: 1, context: {user: 1}}).then(function (results) {
                 should.exist(results);
                 should.not.exist(results.meta);
@@ -140,19 +159,33 @@ describe('Users API', function () {
             }).catch(done);
         });
 
-        it('editor can read', function (done) {
+        it('can read (admin)', function (done) {
             UsersAPI.read({id: 1, context: {user: 2}}).then(function (results) {
                 should.exist(results);
                 should.not.exist(results.meta);
                 should.exist(results.users);
                 results.users[0].id.should.eql(1);
                 testUtils.API.checkResponse(results.users[0], 'user', ['roles']);
+
+                results.users[0].created_at.should.be.a.Date;
+
+                done();
+            }).catch(done);
+        });
+
+        it('can read (editor)', function (done) {
+            UsersAPI.read({id: 1, context: {user: 3}}).then(function (results) {
+                should.exist(results);
+                should.not.exist(results.meta);
+                should.exist(results.users);
+                results.users[0].id.should.eql(1);
+                testUtils.API.checkResponse(results.users[0], 'user', ['roles']);
                 done();
             }).catch(done);
         });
 
-        it('author can read', function (done) {
+        it('can read (author)', function (done) {
-            UsersAPI.read({id: 1, context: {user: 3}}).then(function (results) {
+            UsersAPI.read({id: 1, context: {user: 4}}).then(function (results) {
                 should.exist(results);
                 should.not.exist(results.meta);
                 should.exist(results.users);
@@ -173,7 +206,7 @@ describe('Users API', function () {
             }).catch(done);
         });
 
-        it('admin can edit', function (done) {
+        it('can edit (owner)', function (done) {
             UsersAPI.edit({users: [{name: 'Joe Blogger'}]}, {id: 1, context: {user: 1}}).then(function (response) {
                 should.exist(response);
                 should.not.exist(response.meta);
@@ -186,28 +219,41 @@ describe('Users API', function () {
             }).catch(done);
         });
 
-        it('editor can edit', function (done) {
+        it('can edit (admin)', function (done) {
             UsersAPI.edit({users: [{name: 'Joe Blogger'}]}, {id: 1, context: {user: 2}}).then(function (response) {
                 should.exist(response);
                 should.not.exist(response.meta);
                 should.exist(response.users);
                 response.users.should.have.length(1);
                 testUtils.API.checkResponse(response.users[0], 'user', ['roles']);
-                response.users[0].name.should.eql('Joe Blogger');
+                response.users[0].name.should.equal('Joe Blogger');
+                response.users[0].updated_at.should.be.a.Date;
+                done();
+            }).catch(done);
+        });
 
+        it('can edit (editor)', function (done) {
+            UsersAPI.edit({users: [{name: 'Joe Blogger'}]}, {id: 1, context: {user: 2}}).then(function (response) {
+                should.exist(response);
+                should.not.exist(response.meta);
+                should.exist(response.users);
+                response.users.should.have.length(1);
+                testUtils.API.checkResponse(response.users[0], 'user', ['roles']);
+                response.users[0].name.should.equal('Joe Blogger');
+                response.users[0].updated_at.should.be.a.Date;
                 done();
             }).catch(done);
         });
 
         it('author can edit only self', function (done) {
             // Test author cannot edit admin user
-            UsersAPI.edit({users: [{name: 'Joe Blogger'}]}, {id: 1, context: {user: 3}}).then(function () {
+            UsersAPI.edit({users: [{name: 'Joe Blogger'}]}, {id: 1, context: {user: 4}}).then(function () {
                 done(new Error('Author should not be able to edit account which is not their own'));
             }).catch(function (error) {
                 error.type.should.eql('NoPermissionError');
             }).finally(function () {
                 // Next test that author CAN edit self
-                return UsersAPI.edit({users: [{name: 'Timothy Bogendath'}]}, {id: 3, context: {user: 3}})
+                return UsersAPI.edit({users: [{name: 'Timothy Bogendath'}]}, {id: 4, context: {user: 4}})
                     .then(function (response) {
                         should.exist(response);
                         should.not.exist(response.meta);
@@ -219,5 +265,48 @@ describe('Users API', function () {
                     }).catch(done);
             });
         });
+
+        it('admin can\'t transfer ownership', function (done) {
+            // transfer ownership to user id: 2
+            UsersAPI.edit({users: [{name: 'Joe Blogger', roles:[4]}]}, {id: 3, context: {user: 2}}).then(function (response) {
+                done(new Error('Admin is not dienied transferring ownership.'));
+            }, function () {
+                done();
+            }).catch(done);
+        });
+
+        it('editor can\'t transfer ownership', function (done) {
+            // transfer ownership to user id: 2
+            UsersAPI.edit({users: [{name: 'Joe Blogger', roles:[4]}]}, {id: 2, context: {user: 3}}).then(function (response) {
+                done(new Error('Admin is not dienied transferring ownership.'));
+            }, function () {
+                done();
+            }).catch(done);
+        });
+
+        it('author can\'t transfer ownership', function (done) {
+            // transfer ownership to user id: 2
+            UsersAPI.edit({users: [{name: 'Joe Blogger', roles:[4]}]}, {id: 2, context: {user: 4}}).then(function (response) {
+                done(new Error('Admin is not dienied transferring ownership.'));
+            }, function () {
+                done();
+            }).catch(done);
+        });
+
+        it('owner can transfer ownership', function (done) {
+            // transfer ownership to user id: 2
+            UsersAPI.edit({users: [{name: 'Joe Blogger', roles:[4]}]}, {id: 2, context: {user: 1}}).then(function (response) {
+                should.exist(response);
+                should.not.exist(response.meta);
+                should.exist(response.users);
+                response.users.should.have.length(1);
+                testUtils.API.checkResponse(response.users[0], 'user', ['roles']);
+                response.users[0].name.should.equal('Joe Blogger');
+                response.users[0].id.should.equal(2);
+                response.users[0].roles[0].should.equal(4);
+                response.users[0].updated_at.should.be.a.Date;
+                done();
+            }).catch(done);
+        });
     });
 });

core/test/utils/index.js

@@ -97,7 +97,7 @@ function insertDefaultUser() {
         .update(user);
 }
 
-function insertEditorUser() {
+function insertAdminUser() {
     var users = [],
         userRoles = [];
 
@@ -110,12 +110,25 @@ function insertEditorUser() {
         });
 }
 
-function insertAuthorUser() {
+function insertEditorUser() {
     var users = [],
         userRoles = [];
 
     users.push(DataGenerator.forKnex.createUser(DataGenerator.Content.users[2]));
-    userRoles.push(DataGenerator.forKnex.createUserRole(3, 3));
+    userRoles.push(DataGenerator.forKnex.createUserRole(3, 2));
+    return knex('users')
+        .insert(users)
+        .then(function () {
+            return knex('roles_users').insert(userRoles);
+        });
+}
+
+function insertAuthorUser() {
+    var users = [],
+        userRoles = [];
+
+    users.push(DataGenerator.forKnex.createUser(DataGenerator.Content.users[3]));
+    userRoles.push(DataGenerator.forKnex.createUserRole(4, 3));
     return knex('users')
         .insert(users)
         .then(function () {
@@ -236,6 +249,7 @@ module.exports = {
     insertMorePosts: insertMorePosts,
     insertMorePostsTags: insertMorePostsTags,
     insertDefaultUser: insertDefaultUser,
+    insertAdminUser: insertAdminUser,
     insertEditorUser: insertEditorUser,
     insertAuthorUser: insertAuthorUser,
     insertDefaultApp: insertDefaultApp,