diff --git a/core/server/api/users.js b/core/server/api/users.js index 855a7df90f..e06b1e526a 100644 --- a/core/server/api/users.js +++ b/core/server/api/users.js @@ -13,8 +13,17 @@ var when = require('when'), docName = 'users', ONE_DAY = 60 * 60 * 24 * 1000, + // TODO: implement created_by, updated_by + allowedIncludes = ['permissions', 'roles', 'roles.permissions'], users; +// ## Helpers +function prepareInclude(include) { + include = _.intersection(include.split(','), allowedIncludes); + return include; +} + + /** * ## Posts API Methods @@ -32,6 +41,9 @@ users = { browse: function browse(options) { options = options || {}; return canThis(options.context).browse.user().then(function () { + if (options.include) { + options.include = prepareInclude(options.include); + } return dataProvider.User.findAll(options).then(function (result) { return { users: result.toJSON() }; }); @@ -51,6 +63,10 @@ users = { options = _.omit(options, attrs); + if (options.include) { + options.include = prepareInclude(options.include); + } + if (data.id === 'me' && options.context && options.context.user) { data.id = options.context.user; } @@ -78,6 +94,10 @@ users = { return canThis(options.context).edit.user(options.id).then(function () { return utils.checkObject(object, docName).then(function (checkedUserData) { + if (options.include) { + options.include = prepareInclude(options.include); + } + return dataProvider.User.edit(checkedUserData.users[0], options); }).then(function (result) { if (result) { @@ -103,6 +123,10 @@ users = { return canThis(options.context).add.user().then(function () { return utils.checkObject(object, docName).then(function (checkedUserData) { // if the user is created by users.register(), use id: 1 as the creator for now + if (options.include) { + options.include = prepareInclude(options.include); + } + if (options.context.internal) { options.context.user = 1; } @@ -145,6 +169,10 @@ users = { return canThis(options.context).add.user().then(function () { return utils.checkObject(object, docName).then(function (checkedUserData) { + if (options.include) { + options.include = prepareInclude(options.include); + } + newUser = checkedUserData.users[0]; if (newUser.email) { diff --git a/core/server/models/base.js b/core/server/models/base.js index 91fa6f6535..9d808b316c 100644 --- a/core/server/models/base.js +++ b/core/server/models/base.js @@ -120,6 +120,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({ toJSON: function (options) { var attrs = _.extend({}, this.attributes), self = this; + options = options || {}; if (options && options.shallow) { return attrs; @@ -129,12 +130,17 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({ return attrs.id; } + if (options && options.include) { + this.include = _.union(this.include, options.include); + } + _.each(this.relations, function (relation, key) { if (key.substring(0, 7) !== '_pivot_') { // if include is set, expand to full object - // toMany relationships are included with ids if not expanded - if (_.contains(self.include, key)) { - attrs[key] = relation.toJSON(); + // 'toMany' relationships are included with ids if not expanded + var fullKey = _.isEmpty(options.name) ? key : options.name + '.' + key; + if (_.contains(self.include, fullKey)) { + attrs[key] = relation.toJSON({name: fullKey, include: self.include}); } else if (relation.hasOwnProperty('length')) { attrs[key] = relation.toJSON({idOnly: true}); } diff --git a/core/server/models/user.js b/core/server/models/user.js index 60cc24a3c1..411e895f42 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -39,7 +39,7 @@ User = ghostBookshelf.Model.extend({ tableName: 'users', saving: function (newPage, attr, options) { - /*jshint unused:false*/ + /*jshint unused:false*/ var self = this; // disabling sanitization until we can implement a better version @@ -94,8 +94,9 @@ User = ghostBookshelf.Model.extend({ // these are the only options that can be passed to Bookshelf / Knex. validOptions = { findOne: ['withRelated'], + findAll: ['withRelated'], add: ['user'], - edit: ['user'] + edit: ['user', 'withRelated'] }; if (validOptions[methodName]) { @@ -105,6 +106,42 @@ User = ghostBookshelf.Model.extend({ return options; }, + /** + * ### Find All + * + * @param options + * @returns {*} + */ + findAll: function (options) { + options = options || {}; + options.withRelated = _.union([ 'roles' ], options.include); + return ghostBookshelf.Model.findAll.call(this, options); + }, + + /** + * ### Find One + * @extends ghostBookshelf.Model.findOne to include roles + * **See:** [ghostBookshelf.Model.findOne](base.js.html#Find%20One) + */ + findOne: function (data, options) { + options = options || {}; + options.withRelated = _.union([ 'roles' ], options.include); + + return ghostBookshelf.Model.findOne.call(this, data, options); + }, + + /** + * ### Edit + * @extends ghostBookshelf.Model.edit to handle returning the full object + * **See:** [ghostBookshelf.Model.edit](base.js.html#edit) + */ + edit: function (data, options) { + options = options || {}; + options.withRelated = _.union([ 'roles' ], options.include); + + return ghostBookshelf.Model.edit.call(this, data, options); + }, + /** * ## Add * Naive user add @@ -121,6 +158,7 @@ User = ghostBookshelf.Model.extend({ userData = this.filterData(data); options = this.filterOptions(options, 'add'); + options.withRelated = _.union([ 'roles' ], options.include); /** * This only allows one user to be added to the database, otherwise fails. diff --git a/core/server/permissions/effective.js b/core/server/permissions/effective.js index d4c4db61dc..9042f94fa6 100644 --- a/core/server/permissions/effective.js +++ b/core/server/permissions/effective.js @@ -6,7 +6,7 @@ var _ = require('lodash'), var effective = { user: function (id) { - return User.findOne({id: id}, { withRelated: ['permissions', 'roles.permissions'] }) + return User.findOne({id: id}, { include: ['permissions', 'roles.permissions'] }) .then(function (foundUser) { var seenPerms = {}, rolePerms = _.map(foundUser.related('roles').models, function (role) { diff --git a/core/test/functional/routes/api/users_test.js b/core/test/functional/routes/api/users_test.js index 09fe9e05f0..17e6295a40 100644 --- a/core/test/functional/routes/api/users_test.js +++ b/core/test/functional/routes/api/users_test.js @@ -68,7 +68,7 @@ describe('User API', function () { testUtils.API.checkResponse(jsonResponse, 'users'); jsonResponse.users.should.have.length(1); - testUtils.API.checkResponse(jsonResponse.users[0], 'user'); + testUtils.API.checkResponse(jsonResponse.users[0], 'user', ['roles']); testUtils.API.isISO8601(jsonResponse.users[0].last_login).should.be.true; testUtils.API.isISO8601(jsonResponse.users[0].created_at).should.be.true; @@ -94,7 +94,7 @@ describe('User API', function () { testUtils.API.checkResponse(jsonResponse, 'users'); jsonResponse.users.should.have.length(1); - testUtils.API.checkResponse(jsonResponse.users[0], 'user'); + testUtils.API.checkResponse(jsonResponse.users[0], 'user', ['roles']); done(); }); }); @@ -115,7 +115,53 @@ describe('User API', function () { testUtils.API.checkResponse(jsonResponse, 'users'); jsonResponse.users.should.have.length(1); - testUtils.API.checkResponse(jsonResponse.users[0], 'user'); + testUtils.API.checkResponse(jsonResponse.users[0], 'user', ['roles']); + done(); + }); + }); + + it('can retrieve a user with role', function (done) { + request.get(testUtils.API.getApiQuery('users/me/?include=roles')) + .set('Authorization', 'Bearer ' + accesstoken) + .expect('Content-Type', /json/) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + should.not.exist(res.headers['x-cache-invalidate']); + var jsonResponse = res.body; + jsonResponse.users.should.exist; + testUtils.API.checkResponse(jsonResponse, 'users'); + + jsonResponse.users.should.have.length(1); + testUtils.API.checkResponse(jsonResponse.users[0], 'user', ['roles']); + testUtils.API.checkResponse(jsonResponse.users[0].roles[0], 'role'); + done(); + }); + }); + + it('can retrieve a user with role and permissions', function (done) { + request.get(testUtils.API.getApiQuery('users/me/?include=roles,roles.permissions')) + .set('Authorization', 'Bearer ' + accesstoken) + .expect('Content-Type', /json/) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + should.not.exist(res.headers['x-cache-invalidate']); + var jsonResponse = res.body; + jsonResponse.users.should.exist; + testUtils.API.checkResponse(jsonResponse, 'users'); + + jsonResponse.users.should.have.length(1); + testUtils.API.checkResponse(jsonResponse.users[0], 'user', ['roles']); + testUtils.API.checkResponse(jsonResponse.users[0].roles[0], 'role', ['permissions']); + testUtils.API.checkResponse(jsonResponse.users[0].roles[0].permissions[0], 'permission'); + done(); }); }); @@ -152,7 +198,7 @@ describe('User API', function () { changedValue = 'joe-bloggs.ghost.org', dataToSend; jsonResponse.users[0].should.exist; - testUtils.API.checkResponse(jsonResponse.users[0], 'user'); + testUtils.API.checkResponse(jsonResponse.users[0], 'user', ['roles']); dataToSend = { users: [{website: changedValue}]}; @@ -171,7 +217,7 @@ describe('User API', function () { putBody.users[0].should.exist; putBody.users[0].website.should.eql(changedValue); putBody.users[0].email.should.eql(jsonResponse.users[0].email); - testUtils.API.checkResponse(putBody.users[0], 'user'); + testUtils.API.checkResponse(putBody.users[0], 'user', ['roles']); done(); }); }); diff --git a/core/test/integration/api/api_users_spec.js b/core/test/integration/api/api_users_spec.js index e2eb977897..193c3d6b92 100644 --- a/core/test/integration/api/api_users_spec.js +++ b/core/test/integration/api/api_users_spec.js @@ -41,7 +41,7 @@ describe('Users API', function () { testUtils.API.checkResponse(results, 'users'); should.exist(results.users); results.users.should.have.length(1); - testUtils.API.checkResponse(results.users[0], 'user'); + testUtils.API.checkResponse(results.users[0], 'user', ['roles']); results.users[0].name.should.equal('Hello World'); done(); }).catch(done); @@ -83,9 +83,9 @@ describe('Users API', function () { testUtils.API.checkResponse(results, 'users'); should.exist(results.users); results.users.should.have.length(3); - testUtils.API.checkResponse(results.users[0], 'user'); - testUtils.API.checkResponse(results.users[1], 'user'); - testUtils.API.checkResponse(results.users[2], 'user'); + testUtils.API.checkResponse(results.users[0], 'user', ['roles']); + testUtils.API.checkResponse(results.users[1], 'user', ['roles']); + testUtils.API.checkResponse(results.users[2], 'user', ['roles']); done(); }).catch(done); @@ -97,9 +97,9 @@ describe('Users API', function () { testUtils.API.checkResponse(results, 'users'); should.exist(results.users); results.users.should.have.length(3); - testUtils.API.checkResponse(results.users[0], 'user'); - testUtils.API.checkResponse(results.users[1], 'user'); - testUtils.API.checkResponse(results.users[2], 'user'); + testUtils.API.checkResponse(results.users[0], 'user', ['roles']); + testUtils.API.checkResponse(results.users[1], 'user', ['roles']); + testUtils.API.checkResponse(results.users[2], 'user', ['roles']); done(); }).catch(done); }); @@ -110,9 +110,9 @@ describe('Users API', function () { testUtils.API.checkResponse(results, 'users'); should.exist(results.users); results.users.should.have.length(3); - testUtils.API.checkResponse(results.users[0], 'user'); - testUtils.API.checkResponse(results.users[1], 'user'); - testUtils.API.checkResponse(results.users[2], 'user'); + testUtils.API.checkResponse(results.users[0], 'user', ['roles']); + testUtils.API.checkResponse(results.users[1], 'user', ['roles']); + testUtils.API.checkResponse(results.users[2], 'user', ['roles']); done(); }).catch(done); }); @@ -130,7 +130,7 @@ describe('Users API', function () { should.exist(results); testUtils.API.checkResponse(results, 'users'); results.users[0].id.should.eql(1); - testUtils.API.checkResponse(results.users[0], 'user'); + testUtils.API.checkResponse(results.users[0], 'user', ['roles']); results.users[0].created_at.should.be.a.Date; @@ -143,7 +143,7 @@ describe('Users API', function () { should.exist(results); testUtils.API.checkResponse(results, 'users'); results.users[0].id.should.eql(1); - testUtils.API.checkResponse(results.users[0], 'user'); + testUtils.API.checkResponse(results.users[0], 'user', ['roles']); done(); }).catch(done); }); @@ -153,7 +153,7 @@ describe('Users API', function () { should.exist(results); testUtils.API.checkResponse(results, 'users'); results.users[0].id.should.eql(1); - testUtils.API.checkResponse(results.users[0], 'user'); + testUtils.API.checkResponse(results.users[0], 'user', ['roles']); done(); }).catch(done); }); @@ -163,7 +163,7 @@ describe('Users API', function () { should.exist(results); testUtils.API.checkResponse(results, 'users'); results.users[0].id.should.eql(1); - testUtils.API.checkResponse(results.users[0], 'user'); + testUtils.API.checkResponse(results.users[0], 'user', ['roles']); done(); }).catch(done); }); @@ -173,7 +173,7 @@ describe('Users API', function () { should.exist(response); testUtils.API.checkResponse(response, 'users'); response.users.should.have.length(1); - testUtils.API.checkResponse(response.users[0], 'user'); + 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(); @@ -185,7 +185,7 @@ describe('Users API', function () { should.exist(response); testUtils.API.checkResponse(response, 'users'); response.users.should.have.length(1); - testUtils.API.checkResponse(response.users[0], 'user'); + testUtils.API.checkResponse(response.users[0], 'user', ['roles']); response.users[0].name.should.eql('Joe Blogger'); done(); @@ -205,7 +205,7 @@ describe('Users API', function () { should.exist(response); testUtils.API.checkResponse(response, 'users'); response.users.should.have.length(1); - testUtils.API.checkResponse(response.users[0], 'user'); + testUtils.API.checkResponse(response.users[0], 'user', ['roles']); response.users[0].name.should.eql('Timothy Bogendath'); done(); }).catch(done); diff --git a/core/test/unit/permissions_spec.js b/core/test/unit/permissions_spec.js index f284988a13..4c6a626d8c 100644 --- a/core/test/unit/permissions_spec.js +++ b/core/test/unit/permissions_spec.js @@ -142,7 +142,7 @@ describe('Permissions', function () { return testUser.permissions().attach(testPermission); }); }).then(function () { - return UserProvider.findOne({id: 1}, { withRelated: ['permissions']}); + return UserProvider.findOne({id: 1}, { include: ['permissions']}); }).then(function (updatedUser) { should.exist(updatedUser); diff --git a/core/test/utils/api.js b/core/test/utils/api.js index 2f44eeaf72..83075b525e 100644 --- a/core/test/utils/api.js +++ b/core/test/utils/api.js @@ -22,7 +22,10 @@ var url = require('url'), notification: ['type', 'message', 'status', 'id', 'dismissible', 'location'], slugs: ['slugs'], slug: ['slug'], - accesstoken: ['access_token', 'refresh_token', 'expires_in', 'token_type'] + accesstoken: ['access_token', 'refresh_token', 'expires_in', 'token_type'], + role: ['id', 'uuid', 'name', 'description', 'created_at', 'created_by', 'updated_at', 'updated_by'], + permission: ['id', 'uuid', 'name', 'object_type', 'action_type', 'object_id', 'created_at', 'created_by', + 'updated_at', 'updated_by'] }; function getApiQuery(route) { @@ -52,8 +55,11 @@ function checkResponseValue(jsonResponse, properties) { } } -function checkResponse(jsonResponse, objectType) { - checkResponseValue(jsonResponse, expectedProperties[objectType]); +function checkResponse(jsonResponse, objectType, additionalProperties) { + var checkProperties = expectedProperties[objectType]; + checkProperties = additionalProperties ? checkProperties.concat(additionalProperties) : checkProperties; + + checkResponseValue(jsonResponse, checkProperties); } function isISO8601(date) {