Merge pull request #3457 from sebgie/issue#3426

Transfer ownership end point

core/client/controllers/modals/transfer-owner.js

@@ -2,27 +2,24 @@ var TransferOwnerController = Ember.Controller.extend({
     actions: {
         confirmAccept: function () {
             var user = this.get('model'),
+                url = this.get('ghostPaths.url').api('users', 'owner'),
                 self = this;
 
             self.get('popover').closePopovers();
 
-            // Get owner role
+            ic.ajax.request(url, {
-            this.store.find('role').then(function (result) {
+                type: 'PUT',
-                return result.findBy('name', 'Owner');
+                data: {
-            }).then(function (ownerRole) {
+                    owner: [{
-                // remove roles and assign owner role
+                        'id': user.get('id')
-                user.get('roles').clear();
+                    }]
-                user.get('roles').pushObject(ownerRole);
+                }
-
-                return user.saveOnly('roles');
             }).then(function () {
                 self.notifications.closePassive();
                 self.notifications.showSuccess('Ownership successfully transferred to ' + user.get('name'));
-            }).catch(function (errors) {
+            }).catch(function (error) {
                 self.notifications.closePassive();
-
+                self.notifications.showAPIError(error);
-                errors = Ember.isArray(errors) ? errors : Ember.A([errors]);
-                self.notifications.showErrors(errors);
             });
         },
 

core/server/api/roles.js

@@ -41,6 +41,9 @@ roles = {
                     // TODO: replace with better filter when bluebird lands
                     _.each(foundRoles.toJSON(), function (role) {
                         permissionMap.push(canThis(options.context).assign.role(role).then(function () {
+                            if (role.name === 'Owner') {
+                                return null;
+                            }
                             return role;
                         }, function () {
                             return null;

core/server/api/users.js

@@ -142,6 +142,11 @@ users = {
                 }
 
                 newUser = checkedUserData.users[0];
+
+                if (_.isEmpty(newUser.roles)) {
+                    return when.reject(new errors.BadRequestError('No role provided.'));
+                }
+
                 roleId = parseInt(newUser.roles[0].id || newUser.roles[0], 10);
 
                 // Make sure user is allowed to add a user with this role
@@ -281,8 +286,26 @@ users = {
                 return when.reject(new errors.ValidationError(error.message));
             });
         });
-    }
+    },
 
+    /**
+     *
+     */
+    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 () {
+                    return when.resolve({owner: [{message: 'Ownership transferred successfully.'}]});
+                }).catch(function (error) {
+                    return when.reject(new errors.ValidationError(error.message));
+                });
+            });
+        }).catch(function (error) {
+            return errors.handleAPIError(error);
+        });
+    }
 };
 
 module.exports = users;

core/server/data/fixtures/index.js

@@ -58,7 +58,7 @@ createOwner = function () {
     var user = fixtures.users[0];
 
     return models.Role.findOne({name: 'Owner'}).then(function (ownerRole) {
-        user.role = ownerRole.id;
+        user.roles = [ownerRole.id];
         user.password = utils.uid(50);
 
         logInfo('Creating owner');

core/server/models/role.js

@@ -60,8 +60,9 @@ Role = ghostBookshelf.Model.extend({
         }
 
         if (action === 'assign' && loadedPermissions.user) {
-            if (_.any(loadedPermissions.user.roles, { 'name': 'Owner' }) ||
+            if (_.any(loadedPermissions.user.roles, { 'name': 'Owner' })) {
-                _.any(loadedPermissions.user.roles, { 'name': 'Administrator' })) {
+                checkAgainst = ['Owner', 'Administrator', 'Editor', 'Author'];
+            } else if (_.any(loadedPermissions.user.roles, { 'name': 'Administrator' })) {
                 checkAgainst = ['Administrator', 'Editor', 'Author'];
             } else if (_.any(loadedPermissions.user.roles, { 'name': 'Editor' })) {
                 checkAgainst = ['Author'];

core/server/models/user.js

@@ -40,12 +40,6 @@ User = ghostBookshelf.Model.extend({
         /*jshint unused:false*/
 
         var self = this;
-        // disabling sanitization until we can implement a better version
-        // this.set('name', this.sanitize('name'));
-        // this.set('email', this.sanitize('email'));
-        // this.set('location', this.sanitize('location'));
-        // this.set('website', this.sanitize('website'));
-        // this.set('bio', this.sanitize('bio'));
 
         ghostBookshelf.Model.prototype.saving.apply(this, arguments);
 
@@ -184,8 +178,8 @@ User = ghostBookshelf.Model.extend({
         }, options);
 
         //TODO: there are multiple statuses that make a user "active" or "invited" - we a way to translate/map them:
-            //TODO (cont'd from above): * valid "active" statuses: active, warn-1, warn-2, warn-3, warn-4, locked
+        //TODO (cont'd from above): * valid "active" statuses: active, warn-1, warn-2, warn-3, warn-4, locked
-            //TODO (cont'd from above): * valid "invited" statuses" invited, invited-pending
+        //TODO (cont'd from above): * valid "invited" statuses" invited, invited-pending
 
         // Filter on the status.  A status of 'all' translates to no filter since we want all statuses
         if (options.status && options.status !== 'all') {
@@ -361,7 +355,9 @@ User = ghostBookshelf.Model.extend({
                     return Role.findOne({id: roleId});
                 }).then(function (roleToAssign) {
                     if (roleToAssign && roleToAssign.get('name') === 'Owner') {
-                        return self.transferOwnership(user, roleToAssign, options.context);
+                        return when.reject(
+                            new errors.ValidationError('This method does not support assigning the owner role')
+                        );
                     } else {
                         // assign all other roles
                         return user.roles().updatePivot({role_id: roleId});
@@ -386,24 +382,23 @@ User = ghostBookshelf.Model.extend({
      */
     add: function (data, options) {
         var self = this,
-            // Clone the _user so we don't expose the hashed password unnecessarily
             userData = this.filterData(data),
-            // Get the role we're going to assign to this user, or the author role if there isn't one
+            roles;
-            // TODO: don't reference Author role by ID!
-            roles = data.roles || [1];
-
-        // remove roles from the object
-        delete data.roles;
-
-        // check for too many roles
-        if (roles.length > 1) {
-            return when.reject(new errors.ValidationError('Only one role per user is supported at the moment.'));
-        }
 
         options = this.filterOptions(options, 'add');
         options.withRelated = _.union([ 'roles' ], options.include);
+        return Role.findOne({name: 'Author'}).then(function (authorRole) {
+            // Get the role we're going to assign to this user, or the author role if there isn't one
+            roles = data.roles || authorRole.id;
+            // check for too many roles
+            if (roles.length > 1) {
+                return when.reject(new errors.ValidationError('Only one role per user is supported at the moment.'));
+            }
+            // remove roles from the object
+            delete data.roles;
 
-        return validatePasswordLength(userData.password).then(function () {
+            return validatePasswordLength(userData.password);
+        }).then(function () {
             return self.forge().fetch();
         }).then(function () {
             // Generate a new password hash
@@ -720,23 +715,40 @@ User = ghostBookshelf.Model.extend({
         });
     },
 
-    transferOwnership: function (user, ownerRole, context) {
+    transferOwnership: function (object, options) {
-        var adminRole;
+        var adminRole,
+            ownerRole,
+            contextUser,
+            assignUser;
+
         // Get admin role
         return Role.findOne({name: 'Administrator'}).then(function (result) {
             adminRole = result;
-            return User.findOne({id: context.user});
+            return Role.findOne({name: 'Owner'});
-        }).then(function (contextUser) {
+        }).then(function (result) {
+            ownerRole = result;
+            return User.findOne({id: options.context.user});
+        }).then(function (ctxUser) {
             // check if user has the owner role
-            var currentRoles = contextUser.toJSON().roles;
+            var currentRoles = ctxUser.toJSON().roles;
             if (!_.contains(currentRoles, ownerRole.id)) {
                 return when.reject(new errors.NoPermissionError('Only owners are able to transfer the owner role.'));
             }
+            contextUser = ctxUser;
+            return User.findOne({id: object.id});
+        }).then(function (user) {
+            
+            var currentRoles = user.toJSON().roles;
+            if (!_.contains(currentRoles, adminRole.id)) {
+                return when.reject(new errors.ValidationError('Only administrators can be assigned the owner role.'));
+            }
+
+            assignUser = user;
             // 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});
+            return assignUser.roles().updatePivot({role_id: ownerRole.id});
         });
     },
 

core/server/routes/api.js

@@ -27,6 +27,7 @@ apiRoutes = function (middleware) {
     router.get('/users/slug/:slug', api.http(api.users.read));
     router.get('/users/email/:email', api.http(api.users.read));
     router.put('/users/password', api.http(api.users.changePassword));
+    router.put('/users/owner', api.http(api.users.transferOwnership));
     router.put('/users/:id', api.http(api.users.edit));
     router.post('/users', api.http(api.users.add));
     router.del('/users/:id', api.http(api.users.destroy));

core/test/integration/api/api_users_spec.js

@@ -856,47 +856,41 @@ describe('Users API', function () {
     });
 
     describe('Transfer ownership', function () {
-// Temporarily commenting this test out until #3426 is fixed
+       it('Owner can transfer ownership', function (done) {
-//        it('Owner can transfer ownership', function (done) {
+           // transfer ownership to admin user id:2
-//            // transfer ownership to admin user id:2
+           UserAPI.transferOwnership(
-//            UserAPI.edit(
+               {owner: [
-//                {users: [
+                   {id: userIdFor.admin}
-//                    {name: 'Joe Blogger', roles: [roleIdFor.owner]}
+               ]}, context.owner
-//                ]}, _.extend({}, context.owner, {id: userIdFor.admin})
+           ).then(function (response) {
-//            ).then(function (response) {
+               should.exist(response);
-//                should.exist(response);
+               should.exist(response.owner);
-//                should.not.exist(response.meta);
+               response.owner.should.have.length(1);
-//                should.exist(response.users);
+               response.owner[0].message.should.eql('Ownership transferred successfully.');
-//                response.users.should.have.length(1);
+               done();
-//                testUtils.API.checkResponse(response.users[0], 'user', ['roles']);
+           }).catch(done);
-//                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);
-//        });
 
         it('Owner CANNOT downgrade own role', function (done) {
             // Cannot change own role to admin
-            UserAPI.edit(
+            UserAPI.transferOwnership(
-                {users: [
+                {owner: [
-                    {name: 'Joe Blogger', roles: [roleIdFor.admin]}
+                    {id: userIdFor.owner}
-                ]}, _.extend({}, context.owner, {id: userIdFor.owner})
+                ]}, context.owner
             ).then(function (response) {
                 done(new Error('Owner should not be able to downgrade their role'));
                 }).catch(function (error) {
-                    error.type.should.eql('NoPermissionError');
+                    error.type.should.eql('ValidationError');
                     done();
                 });
         });
 
         it('Admin CANNOT transfer ownership', function (done) {
             // transfer ownership to user id: 2
-            UserAPI.edit(
+            UserAPI.transferOwnership(
-                {users: [
+                {owner: [
-                    {name: 'Joe Blogger', roles: [roleIdFor.owner]}
+                    {id: userIdFor.editor}
-                ]}, _.extend({}, context.admin, {id: userIdFor.admin})
+                ]}, context.admin
             ).then(function () {
                     done(new Error('Admin is not denied transferring ownership.'));
                 }).catch(function (error) {
@@ -907,10 +901,10 @@ describe('Users API', function () {
 
         it('Editor CANNOT transfer ownership', function (done) {
             // transfer ownership to user id: 2
-            UserAPI.edit(
+            UserAPI.transferOwnership(
-                {users: [
+                {owner: [
-                    {name: 'Joe Blogger', roles: [roleIdFor.owner]}
+                    {id: userIdFor.admin}
-                ]}, _.extend({}, context.editor, {id: userIdFor.admin})
+                ]}, context.editor
             ).then(function () {
                 done(new Error('Admin is not denied transferring ownership.'));
             }).catch(function (error) {
@@ -921,10 +915,10 @@ describe('Users API', function () {
 
         it('Author CANNOT transfer ownership', function (done) {
             // transfer ownership to user id: 2
-            UserAPI.edit(
+            UserAPI.transferOwnership(
-                {users: [
+                {owner: [
-                    {name: 'Joe Blogger', roles: [roleIdFor.owner]}
+                    {id: userIdFor.admin}
-                ]}, _.extend({}, context.author, {id: userIdFor.admin})
+                ]}, context.author
             ).then(function () {
                     done(new Error('Admin is not denied transferring ownership.'));
                 }).catch(function (error) {

core/test/integration/model/model_users_spec.js

@@ -25,7 +25,7 @@ describe('User Model', function run() {
     should.exist(UserModel);
 
     describe('Registration', function runRegistration() {
-        beforeEach(testUtils.setup());
+        beforeEach(testUtils.setup('roles'));
 
         it('can add first', function (done) {
             var userData = testUtils.DataGenerator.forModel.users[0];

core/test/utils/index.js

@@ -101,6 +101,10 @@ fixtures = {
             }));
         });
     },
+    insertRoles: function insertRoles() {
+        var knex = config.database.knex;
+        return knex('roles').insert(DataGenerator.forKnex.roles);
+    },
 
     initOwnerUser: function initOwnerUser() {
         var user = DataGenerator.Content.users[0],
@@ -258,6 +262,7 @@ toDoList = {
     },
     'permission': function insertPermission() { return fixtures.insertOne('permissions', 'createPermission'); },
     'role': function insertRole() { return fixtures.insertOne('roles', 'createRole'); },
+    'roles': function insertRoles() { return fixtures.insertRoles(); },
     'tag': function insertRole() { return fixtures.insertOne('tags', 'createTag'); },
     'posts': function insertPosts() { return fixtures.insertPosts(); },
     'apps': function insertApps() { return fixtures.insertApps(); },