🎨 invites roles table into a field on the invites table (#7705)

* 🎨  schema change

- simply role_id attribute

* 🎨  update invite model

- remove all methods we don't need
- ensure we remove the relation from the model
- ensure we do not allow to call withRelated

* 🎨  adapt api changes

* 🎨  adapt auth module

* 🎨  adapt tests

* 🎨  better error handling

* schema update

core/server/api/authentication.js

@@ -412,7 +412,7 @@ authentication = {
         function processInvitation(invitation) {
             var data = invitation.invitation[0], inviteToken = globalUtils.decodeBase64URLsafe(data.token);
 
-            return models.Invite.findOne({token: inviteToken, status: 'sent'}, _.merge({}, {include: ['roles']}, options))
+            return models.Invite.findOne({token: inviteToken, status: 'sent'}, options)
                 .then(function (_invite) {
                     invite = _invite;
 
@@ -428,7 +428,7 @@ authentication = {
                         email: data.email,
                         name: data.name,
                         password: data.password,
-                        roles: invite.toJSON().roles
+                        roles: [invite.toJSON().role_id]
                     }, options);
                 })
                 .then(function () {

core/server/api/invites.js

@@ -12,7 +12,7 @@ var _ = require('lodash'),
     config = require('../config'),
     i18n = require('../i18n'),
     docName = 'invites',
-    allowedIncludes = ['created_by', 'updated_by', 'roles'],
+    allowedIncludes = ['created_by', 'updated_by'],
     invites;
 
 invites = {
@@ -169,21 +169,23 @@ invites = {
         }
 
         function validation(options) {
-            var roleId;
-
             if (!options.data.invites[0].email) {
                 return Promise.reject(new errors.ValidationError({message: i18n.t('errors.api.invites.emailIsRequired')}));
             }
 
-            if (!options.data.invites[0].roles || !options.data.invites[0].roles[0]) {
+            if (!options.data.invites[0].role_id) {
                 return Promise.reject(new errors.ValidationError({message: i18n.t('errors.api.invites.roleIsRequired')}));
             }
 
-            roleId = parseInt(options.data.invites[0].roles[0].id || options.data.invites[0].roles[0], 10);
+            options.data.invites[0].role_id = parseInt(options.data.invites[0].role_id, 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) {
+            return dataProvider.Role.findOne({id: options.data.invites[0].role_id}).then(function (role) {
+                if (!role) {
+                    return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.api.invites.roleNotFound')}));
+                }
+
                 if (role.get('name') === 'Owner') {
                     return Promise.reject(new errors.NoPermissionError({message: i18n.t('errors.api.invites.notAllowedToInviteOwner')}));
                 }

core/server/auth/auth-strategies.js

@@ -97,7 +97,7 @@ strategies = {
                         email: profile.email,
                         name: profile.email,
                         password: utils.uid(50),
-                        roles: invite.toJSON().roles
+                        roles: [invite.toJSON().role_id]
                     }, options);
                 })
                 .then(function destroyInvite(_user) {

core/server/data/schema/schema.js

@@ -220,6 +220,7 @@ module.exports = {
     },
     invites: {
         id: {type: 'increments', nullable: false, primary: true},
+        role_id: {type: 'integer', nullable: false, unsigned: true},
         status: {type: 'string', maxlength: 150, nullable: false, defaultTo: 'pending', validations: {isIn: [['pending', 'sent']]}},
         token: {type: 'string', maxlength: 191, nullable: false, unique: true},
         email: {type: 'string', maxlength: 191, nullable: false, unique: true, validations: {isEmail: true}},
@@ -229,11 +230,6 @@ module.exports = {
         updated_at: {type: 'dateTime', nullable: true},
         updated_by: {type: 'integer', nullable: true}
     },
-    invites_roles: {
-        id: {type: 'increments', nullable: false, primary: true},
-        role_id: {type: 'integer', nullable: false},
-        invite_id: {type: 'integer', nullable: false}
-    },
     brute: {
         key: {type: 'string'},
         firstRequest: {type: 'bigInteger'},

core/server/models/invite.js

@@ -2,7 +2,6 @@ var ghostBookshelf = require('./base'),
     globalUtils = require('../utils'),
     crypto = require('crypto'),
     _ = require('lodash'),
-    Promise = require('bluebird'),
     Invite,
     Invites;
 
@@ -15,10 +14,6 @@ Invite = ghostBookshelf.Model.extend({
         var attrs = ghostBookshelf.Model.prototype.toJSON.call(this, options);
         delete attrs.token;
         return attrs;
-    },
-
-    roles: function roles() {
-        return this.belongsToMany('Role');
     }
 }, {
     orderDefaultOptions: function orderDefaultOptions() {
@@ -29,31 +24,6 @@ Invite = ghostBookshelf.Model.extend({
         return options;
     },
 
-    filterData: function filterData(data) {
-        var permittedAttributes = this.prototype.permittedAttributes(),
-            filteredData;
-
-        permittedAttributes.push('roles');
-        filteredData = _.pick(data, permittedAttributes);
-
-        return filteredData;
-    },
-
-    permittedOptions: function permittedOptions(methodName) {
-        var options = ghostBookshelf.Model.permittedOptions(),
-            validOptions = {
-                findOne: ['withRelated'],
-                edit: ['withRelated'],
-                findPage: ['withRelated']
-            };
-
-        if (validOptions[methodName]) {
-            options = options.concat(validOptions[methodName]);
-        }
-
-        return options;
-    },
-
     /**
      * @TODO: can't use base class, because:
      * options.withRelated = _.union(options.withRelated, options.include); is missing
@@ -74,10 +44,7 @@ Invite = ghostBookshelf.Model.extend({
 
     add: function add(data, options) {
         var hash = crypto.createHash('sha256'),
-            text = '',
+            text = '';
-            roles = data.roles,
-            self = this,
-            invite;
 
         options = this.filterOptions(options, 'add');
         options.withRelated = _.union(options.withRelated, options.include);
@@ -90,31 +57,7 @@ Invite = ghostBookshelf.Model.extend({
         hash.update(data.email.toLocaleLowerCase());
         text += [data.expires, data.email, hash.digest('base64')].join('|');
         data.token = new Buffer(text).toString('base64');
-
+        return ghostBookshelf.Model.add.call(this, data, options);
-        delete data.roles;
-
-        return ghostBookshelf.Model.add.call(this, data, options)
-            .then(function (_invite) {
-                invite = _invite;
-
-                return Promise.resolve(roles)
-                    .then(function then(roles) {
-                        roles = _.map(roles, function mapper(role) {
-                            if (_.isString(role)) {
-                                return parseInt(role, 10);
-                            } else if (_.isNumber(role)) {
-                                return role;
-                            } else {
-                                return parseInt(role.id, 10);
-                            }
-                        });
-
-                        return invite.roles().attach(roles, options);
-                    });
-            })
-            .then(function () {
-                return self.findOne({id: invite.id}, options);
-            });
     }
 });
 

core/server/translations/en.json

@@ -394,6 +394,7 @@
                 "inviteExpired": "Invite is expired.",
                 "emailIsRequired": "E-Mail is required.",
                 "roleIsRequired": "Role is required",
+                "roleNotFound": "Role not found",
                 "errorSendingEmail": {
                     "error": "Error sending email: {message}",
                     "help": "Please check your email settings and resend the invitation."

core/test/integration/api/api_authentication_spec.js

@@ -257,10 +257,10 @@ describe('Authentication API', function () {
             it('should allow an invitation to be accepted', function () {
                 var invite;
 
-                return models.Invite.add({email: '123@meins.de', roles: [1]}, _.merge({}, {include: ['roles']}, context.internal))
+                return models.Invite.add({email: '123@meins.de', role_id: testUtils.roles.ids.admin}, context.internal)
                     .then(function (_invite) {
                         invite = _invite;
-                        invite.toJSON().roles.length.should.eql(1);
+                        invite.toJSON().role_id.should.eql(testUtils.roles.ids.admin);
 
                         return models.Invite.edit({status: 'sent'}, _.merge({}, {id: invite.id}, context.internal));
                     })
@@ -287,14 +287,14 @@ describe('Authentication API', function () {
                         }, _.merge({include: ['roles']}, context.internal));
                     })
                     .then(function (user) {
-                        user.toJSON().roles.length.should.eql(1);
+                        user.toJSON().roles.length.should.eql(testUtils.roles.ids.admin);
                     });
             });
 
             it('should not allow an invitation to be accepted: expired', function () {
                 var invite;
 
-                return models.Invite.add({email: '123@meins.de'}, context.internal)
+                return models.Invite.add({email: '123@meins.de', role_id: testUtils.roles.ids.author}, context.internal)
                     .then(function (_invite) {
                         invite = _invite;
 

core/test/integration/api/api_invites_spec.js

@@ -29,30 +29,28 @@ describe('Invites API', function () {
         describe('Add', function () {
             it('add invite 1', function (done) {
                 InvitesAPI.add({
-                    invites: [{email: 'test@example.com', roles: [testUtils.roles.ids.editor]}]
+                    invites: [{email: 'test@example.com', role_id: testUtils.roles.ids.editor}]
-                }, _.merge({}, {include: ['roles']}, testUtils.context.owner))
+                }, testUtils.context.owner)
                     .then(function (response) {
                         response.invites.length.should.eql(1);
-                        response.invites[0].roles.length.should.eql(1);
+                        response.invites[0].role_id.should.eql(testUtils.roles.ids.editor);
-                        response.invites[0].roles[0].name.should.eql('Editor');
                         done();
                     }).catch(done);
             });
 
             it('add invite 2', function (done) {
                 InvitesAPI.add({
-                    invites: [{email: 'test2@example.com', roles: [testUtils.roles.ids.author]}]
+                    invites: [{email: 'test2@example.com', role_id: testUtils.roles.ids.author}]
-                }, _.merge({}, {include: ['roles']}, testUtils.context.owner))
+                }, testUtils.context.owner)
                     .then(function (response) {
                         response.invites.length.should.eql(1);
-                        response.invites[0].roles.length.should.eql(1);
+                        response.invites[0].role_id.should.eql(testUtils.roles.ids.author);
-                        response.invites[0].roles[0].name.should.eql('Author');
                         done();
                     }).catch(done);
             });
 
             it('add invite: empty invites object', function (done) {
-                InvitesAPI.add({invites: []}, _.merge({}, {include: ['roles']}, testUtils.context.owner))
+                InvitesAPI.add({invites: []}, testUtils.context.owner)
                     .then(function () {
                         throw new Error('expected validation error');
                     })
@@ -63,7 +61,7 @@ describe('Invites API', function () {
             });
 
             it('add invite: no email provided', function (done) {
-                InvitesAPI.add({invites: [{status: 'sent'}]}, _.merge({}, {include: ['roles']}, testUtils.context.owner))
+                InvitesAPI.add({invites: [{status: 'sent'}]}, testUtils.context.owner)
                     .then(function () {
                         throw new Error('expected validation error');
                     })
@@ -76,19 +74,17 @@ describe('Invites API', function () {
 
         describe('Browse', function () {
             it('browse invites', function (done) {
-                InvitesAPI.browse(_.merge({}, {include: ['roles']}, testUtils.context.owner))
+                InvitesAPI.browse(testUtils.context.owner)
                     .then(function (response) {
                         response.invites.length.should.eql(2);
 
                         response.invites[0].status.should.eql('sent');
                         response.invites[0].email.should.eql('test1@ghost.org');
-                        response.invites[0].roles.length.should.eql(1);
+                        response.invites[0].role_id.should.eql(testUtils.roles.ids.admin);
-                        response.invites[0].roles[0].name.should.eql('Administrator');
 
                         response.invites[1].status.should.eql('sent');
                         response.invites[1].email.should.eql('test2@ghost.org');
-                        response.invites[1].roles.length.should.eql(1);
+                        response.invites[1].role_id.should.eql(testUtils.roles.ids.author);
-                        response.invites[1].roles[0].name.should.eql('Author');
 
                         should.not.exist(response.invites[0].token);
                         should.exist(response.invites[0].expires);
@@ -104,8 +100,7 @@ describe('Invites API', function () {
         describe('Read', function () {
             it('read invites: not found', function (done) {
                 InvitesAPI.read(_.merge({}, testUtils.context.owner, {
-                    email: 'not-existend@hey.org',
+                    email: 'not-existend@hey.org'
-                    include: ['roles']
                 })).then(function () {
                     throw new Error('expected not found error for invite');
                 }).catch(function (err) {
@@ -115,21 +110,19 @@ describe('Invites API', function () {
             });
 
             it('read invite', function (done) {
-                InvitesAPI.read(_.merge({}, {email: 'test1@ghost.org', include: ['roles']}, testUtils.context.owner))
+                InvitesAPI.read(_.merge({}, {email: 'test1@ghost.org'}, testUtils.context.owner))
                     .then(function (response) {
                         response.invites.length.should.eql(1);
-                        response.invites[0].roles.length.should.eql(1);
+                        response.invites[0].role_id.should.eql(testUtils.roles.ids.admin);
-                        response.invites[0].roles[0].name.should.eql('Administrator');
                         done();
                     }).catch(done);
             });
 
             it('read invite', function (done) {
-                InvitesAPI.read(_.merge({}, testUtils.context.owner, {email: 'test2@ghost.org', include: ['roles']}))
+                InvitesAPI.read(_.merge({}, testUtils.context.owner, {email: 'test2@ghost.org'}))
                     .then(function (response) {
                         response.invites.length.should.eql(1);
-                        response.invites[0].roles.length.should.eql(1);
+                        response.invites[0].role_id.should.eql(testUtils.roles.ids.author);
-                        response.invites[0].roles[0].name.should.eql('Author');
                         done();
                     }).catch(done);
             });
@@ -137,11 +130,10 @@ describe('Invites API', function () {
 
         describe('Destroy', function () {
             it('destroy invite', function (done) {
-                InvitesAPI.destroy(_.merge({}, testUtils.context.owner, {id: 1, include: ['roles']}))
+                InvitesAPI.destroy(_.merge({}, testUtils.context.owner, {id: 1}))
                     .then(function () {
                         return InvitesAPI.read(_.merge({}, testUtils.context.owner, {
-                            email: 'test1@ghost.org',
+                            email: 'test1@ghost.org'
-                            include: ['roles']
                         })).catch(function (err) {
                             (err instanceof errors.NotFoundError).should.eql(true);
                             done();
@@ -180,7 +172,7 @@ describe('Invites API', function () {
             should.not.exist(response.meta);
 
             response.invites.should.have.length(1);
-            testUtils.API.checkResponse(response.invites[0], 'invites', ['roles']);
+            testUtils.API.checkResponse(response.invites[0], 'invites');
             response.invites[0].created_at.should.be.an.instanceof(Date);
         }
 
@@ -190,7 +182,7 @@ describe('Invites API', function () {
                     invites: [
                         {
                             email: 'test@example.com',
-                            roles: [testUtils.roles.ids.owner]
+                            role_id: testUtils.roles.ids.owner
                         }
                     ]
                 }, context.owner).then(function () {
@@ -203,12 +195,12 @@ describe('Invites API', function () {
                     invites: [
                         {
                             email: 'test@example.com',
-                            roles: [testUtils.roles.ids.admin]
+                            role_id: testUtils.roles.ids.admin
                         }
                     ]
-                }, _.merge({}, {include: ['roles']}, testUtils.context.owner)).then(function (response) {
+                }, testUtils.context.owner).then(function (response) {
                     checkAddResponse(response);
-                    response.invites[0].roles[0].name.should.equal('Administrator');
+                    response.invites[0].role_id.should.equal(testUtils.roles.ids.admin);
                     done();
                 }).catch(done);
             });
@@ -218,12 +210,12 @@ describe('Invites API', function () {
                     invites: [
                         {
                             email: 'test@example.com',
-                            roles: [testUtils.roles.ids.editor]
+                            role_id: testUtils.roles.ids.editor
                         }
                     ]
-                }, _.merge({}, {include: ['roles']}, testUtils.context.owner)).then(function (response) {
+                }, testUtils.context.owner).then(function (response) {
                     checkAddResponse(response);
-                    response.invites[0].roles[0].name.should.equal('Editor');
+                    response.invites[0].role_id.should.equal(testUtils.roles.ids.editor);
                     done();
                 }).catch(done);
             });
@@ -233,12 +225,12 @@ describe('Invites API', function () {
                     invites: [
                         {
                             email: 'test@example.com',
-                            roles: [testUtils.roles.ids.author]
+                            role_id: testUtils.roles.ids.author
                         }
                     ]
-                }, _.merge({}, {include: ['roles']}, testUtils.context.owner)).then(function (response) {
+                }, testUtils.context.owner).then(function (response) {
                     checkAddResponse(response);
-                    response.invites[0].roles[0].name.should.equal('Author');
+                    response.invites[0].role_id.should.equal(testUtils.roles.ids.author);
                     done();
                 }).catch(done);
             });
@@ -248,12 +240,12 @@ describe('Invites API', function () {
                     invites: [
                         {
                             email: 'test@example.com',
-                            roles: [testUtils.roles.ids.author.toString()]
+                            role_id: testUtils.roles.ids.author.toString()
                         }
                     ]
-                }, _.merge({}, {include: ['roles']}, testUtils.context.owner)).then(function (response) {
+                }, testUtils.context.owner).then(function (response) {
                     checkAddResponse(response);
-                    response.invites[0].roles[0].name.should.equal('Author');
+                    response.invites[0].role_id.should.equal(testUtils.roles.ids.author);
                     done();
                 }).catch(done);
             });
@@ -265,10 +257,10 @@ describe('Invites API', function () {
                     invites: [
                         {
                             email: 'test@example.com',
-                            roles: [testUtils.roles.ids.owner]
+                            role_id: testUtils.roles.ids.owner
                         }
                     ]
-                }, _.merge({}, {include: ['roles']}, testUtils.context.admin)).then(function () {
+                }, testUtils.context.admin).then(function () {
                     done(new Error('Admin should not be able to add an owner'));
                 }).catch(checkForErrorType('NoPermissionError', done));
             });
@@ -278,12 +270,12 @@ describe('Invites API', function () {
                     invites: [
                         {
                             email: 'test@example.com',
-                            roles: [testUtils.roles.ids.admin]
+                            role_id: testUtils.roles.ids.admin
                         }
                     ]
                 }, _.merge({}, {include: ['roles']}, testUtils.context.admin)).then(function (response) {
                     checkAddResponse(response);
-                    response.invites[0].roles[0].name.should.equal('Administrator');
+                    response.invites[0].role_id.should.equal(testUtils.roles.ids.admin);
                     done();
                 }).catch(done);
             });
@@ -293,12 +285,12 @@ describe('Invites API', function () {
                     invites: [
                         {
                             email: 'test@example.com',
-                            roles: [testUtils.roles.ids.editor]
+                            role_id: testUtils.roles.ids.editor
                         }
                     ]
-                }, _.merge({}, {include: ['roles']}, testUtils.context.admin)).then(function (response) {
+                }, testUtils.context.admin).then(function (response) {
                     checkAddResponse(response);
-                    response.invites[0].roles[0].name.should.equal('Editor');
+                    response.invites[0].role_id.should.equal(testUtils.roles.ids.editor);
                     done();
                 }).catch(done);
             });
@@ -308,12 +300,12 @@ describe('Invites API', function () {
                     invites: [
                         {
                             email: 'test@example.com',
-                            roles: [testUtils.roles.ids.author]
+                            role_id: testUtils.roles.ids.author
                         }
                     ]
-                }, _.merge({}, {include: ['roles']}, testUtils.context.admin)).then(function (response) {
+                }, testUtils.context.admin).then(function (response) {
                     checkAddResponse(response);
-                    response.invites[0].roles[0].name.should.equal('Author');
+                    response.invites[0].role_id.should.equal(testUtils.roles.ids.author);
                     done();
                 }).catch(done);
             });
@@ -325,7 +317,7 @@ describe('Invites API', function () {
                     invites: [
                         {
                             email: 'test@example.com',
-                            roles: [testUtils.roles.ids.owner]
+                            role_id: testUtils.roles.ids.owner
                         }
                     ]
                 }, context.editor).then(function () {
@@ -338,7 +330,7 @@ describe('Invites API', function () {
                     invites: [
                         {
                             email: 'test@example.com',
-                            roles: [testUtils.roles.ids.author]
+                            role_id: testUtils.roles.ids.author
                         }
                     ]
                 }, context.editor).then(function () {
@@ -353,7 +345,7 @@ describe('Invites API', function () {
                     invites: [
                         {
                             email: 'test@example.com',
-                            roles: [testUtils.roles.ids.owner]
+                            role_id: testUtils.roles.ids.owner
                         }
                     ]
                 }, context.author).then(function () {
@@ -366,7 +358,7 @@ describe('Invites API', function () {
                     invites: [
                         {
                             email: 'test@example.com',
-                            roles: [testUtils.roles.ids.author]
+                            role_id: testUtils.roles.ids.author
                         }
                     ]
                 }, context.author).then(function () {

core/test/integration/model/model_invite_spec.js

@@ -11,13 +11,15 @@ describe('Invite Model', function () {
 
         it('create invite', function (done) {
             models.Invite.add({
-                email: 'test@test.de'
+                email: 'test@test.de',
+                role_id: testUtils.roles.ids.admin
             }, testUtils.context.internal)
                 .then(function (invite) {
                     should.exist(invite);
                     should.exist(invite.get('token'));
                     should.exist(invite.get('expires'));
                     should.exist(invite.get('email'));
+                    should.exist(invite.get('role_id'));
                     done();
                 })
                 .catch(done);

core/test/utils/fixtures/data-generator.js

@@ -265,8 +265,7 @@ DataGenerator.forKnex = (function () {
         users,
         roles_users,
         clients,
-        invites,
+        invites;
-        invites_roles;
 
     function createBasic(overrides) {
         var newObj = _.cloneDeep(overrides);
@@ -403,6 +402,7 @@ DataGenerator.forKnex = (function () {
         return _.defaults(newObj, {
             token: uuid.v4(),
             email: 'test@ghost.org',
+            role_id: 1,
             expires: Date.now() + (60 * 1000),
             created_by: 1,
             created_at: new Date(),
@@ -477,13 +477,8 @@ DataGenerator.forKnex = (function () {
     ];
 
     invites = [
-        createInvite({email: 'test1@ghost.org'}),
+        createInvite({email: 'test1@ghost.org', role_id: 1}),
-        createInvite({email: 'test2@ghost.org'})
+        createInvite({email: 'test2@ghost.org', role_id: 3})
-    ];
-
-    invites_roles = [
-        {invite_id: 1, role_id: 1},
-        {invite_id: 2, role_id: 3}
     ];
 
     return {
@@ -505,7 +500,6 @@ DataGenerator.forKnex = (function () {
         createInvite: createInvite,
 
         invites: invites,
-        invites_roles: invites_roles,
         posts: posts,
         tags: tags,
         posts_tags: posts_tags,

core/test/utils/index.js

@@ -392,10 +392,7 @@ fixtures = {
     },
 
     insertInvites: function insertInvites() {
-        return db.knex('invites').insert(DataGenerator.forKnex.invites)
+        return db.knex('invites').insert(DataGenerator.forKnex.invites);
-            .then(function () {
-                return db.knex('invites_roles').insert(DataGenerator.forKnex.invites_roles);
-            });
     }
 };