diff --git a/core/server/api/subscribers.js b/core/server/api/subscribers.js index 6e765fdbf2..4a1b8b43d5 100644 --- a/core/server/api/subscribers.js +++ b/core/server/api/subscribers.js @@ -41,7 +41,7 @@ subscribers = { // Push all of our tasks into a `tasks` array in the correct order tasks = [ utils.validate(docName, {opts: utils.browseDefaultOptions}), - // TODO: handlePermissions + utils.handlePermissions(docName, 'browse'), doQuery ]; @@ -71,7 +71,7 @@ subscribers = { // Push all of our tasks into a `tasks` array in the correct order tasks = [ utils.validate(docName, {attrs: attrs}), - // TODO: handlePermissions + utils.handlePermissions(docName, 'read'), doQuery ]; @@ -119,7 +119,7 @@ subscribers = { // Push all of our tasks into a `tasks` array in the correct order tasks = [ utils.validate(docName), - // TODO: handlePermissions + utils.handlePermissions(docName, 'add'), doQuery ]; @@ -154,7 +154,7 @@ subscribers = { // Push all of our tasks into a `tasks` array in the correct order tasks = [ utils.validate(docName, {opts: utils.idDefaultOptions}), - // TODO: handlePermissions + utils.handlePermissions(docName, 'edit'), doQuery ]; @@ -192,7 +192,7 @@ subscribers = { // Push all of our tasks into a `tasks` array in the correct order tasks = [ utils.validate(docName, {opts: utils.idDefaultOptions}), - // TODO: handlePermissions + utils.handlePermissions(docName, 'destroy'), doQuery ]; @@ -246,7 +246,7 @@ subscribers = { } tasks = [ - // TODO: handlePermissions + utils.handlePermissions(docName, 'browse'), exportSubscribers ]; @@ -363,7 +363,7 @@ subscribers = { tasks = [ validate, - // TODO: handlePermissions + utils.handlePermissions(docName, 'add'), importCSV ]; diff --git a/core/server/models/subscriber.js b/core/server/models/subscriber.js index 7149ea2ddf..3b2ce1ee1e 100644 --- a/core/server/models/subscriber.js +++ b/core/server/models/subscriber.js @@ -1,10 +1,36 @@ -var ghostBookshelf = require('./base'), - +var ghostBookshelf = require('./base'), + errors = require('../errors'), + events = require('../events'), + i18n = require('../i18n'), + Promise = require('bluebird'), + uuid = require('node-uuid'), Subscriber, Subscribers; Subscriber = ghostBookshelf.Model.extend({ - tableName: 'subscribers' + tableName: 'subscribers', + + emitChange: function emitChange(event) { + events.emit('subscriber' + '.' + event, this); + }, + defaults: function defaults() { + return { + uuid: uuid.v4(), + status: 'subscribed' + }; + }, + initialize: function initialize() { + ghostBookshelf.Model.prototype.initialize.apply(this, arguments); + this.on('created', function onCreated(model) { + model.emitChange('added'); + }); + this.on('updated', function onUpdated(model) { + model.emitChange('edited'); + }); + this.on('destroyed', function onDestroyed(model) { + model.emitChange('deleted'); + }); + } }, { orderDefaultOptions: function orderDefaultOptions() { @@ -32,8 +58,22 @@ Subscriber = ghostBookshelf.Model.extend({ } return options; - } + }, + permissible: function permissible(postModelOrId, action, context, loadedPermissions, hasUserPermission, hasAppPermission) { + // CASE: external is only allowed to add and edit subscribers + if (context.external) { + if (['add', 'edit'].indexOf(action) !== -1) { + return Promise.resolve(); + } + } + + if (hasUserPermission && hasAppPermission) { + return Promise.resolve(); + } + + return Promise.reject(new errors.NoPermissionError(i18n.t('errors.models.subscriber.notEnoughPermission'))); + } }); Subscribers = ghostBookshelf.Collection.extend({ diff --git a/core/server/permissions/index.js b/core/server/permissions/index.js index 551a7ffd00..2a6115eaea 100644 --- a/core/server/permissions/index.js +++ b/core/server/permissions/index.js @@ -25,11 +25,17 @@ 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, - public: true - }; + internal: false, + external: false, + user: null, + app: null, + public: true + }; + + if (context && (context === 'external' || context.external)) { + parsed.external = true; + parsed.public = false; + } if (context && (context === 'internal' || context.internal)) { parsed.internal = true; @@ -117,8 +123,10 @@ CanThisResult.prototype.buildObjectTypeHandlers = function (objTypes, actType, c role: Models.Role, user: Models.User, permission: Models.Permission, - setting: Models.Settings + setting: Models.Settings, + subscriber: Models.Subscriber }; + // Iterate through the object types, i.e. ['post', 'tag', 'user'] return _.reduce(objTypes, function (objTypeHandlers, objType) { // Grab the TargetModel through the objectTypeModelMap diff --git a/core/server/translations/en.json b/core/server/translations/en.json index c7145e8c75..fbe2b170e7 100644 --- a/core/server/translations/en.json +++ b/core/server/translations/en.json @@ -197,6 +197,9 @@ } }, "models": { + "subscriber": { + "notEnoughPermission": "You do not have permission to perform this action" + }, "post": { "untitled": "(Untitled)", "valueCannotBeBlank": "Value in {key} cannot be blank.", diff --git a/core/test/integration/api/api_subscription_spec.js b/core/test/integration/api/api_subscription_spec.js index 9767504e0d..e5c2302ef1 100644 --- a/core/test/integration/api/api_subscription_spec.js +++ b/core/test/integration/api/api_subscription_spec.js @@ -3,16 +3,15 @@ var testUtils = require('../../utils'), should = require('should'), Promise = require('bluebird'), _ = require('lodash'), - // Stuff we are testing context = testUtils.context, - + errors = require('../../../server/errors'), SubscribersAPI = require('../../../server/api/subscribers'); describe('Subscribers API', function () { // Keep the DB clean before(testUtils.teardown); afterEach(testUtils.teardown); - beforeEach(testUtils.setup('users:roles', 'permission', 'perms:init', 'subscriber')); + beforeEach(testUtils.setup('users:roles', 'perms:subscriber', 'perms:init', 'subscriber')); should.exist(SubscribersAPI); @@ -34,6 +33,26 @@ describe('Subscribers API', function () { }).catch(done); }); + it('can add a subscriber (editor)', function (done) { + SubscribersAPI.add({subscribers: [newSubscriber]}, testUtils.context.editor) + .then(function (results) { + should.exist(results); + should.exist(results.subscribers); + results.subscribers.length.should.be.above(0); + done(); + }).catch(done); + }); + + it('can add a subscriber (author)', function (done) { + SubscribersAPI.add({subscribers: [newSubscriber]}, testUtils.context.author) + .then(function (results) { + should.exist(results); + should.exist(results.subscribers); + results.subscribers.length.should.be.above(0); + done(); + }).catch(done); + }); + it('can add a subscriber (external)', function (done) { SubscribersAPI.add({subscribers: [newSubscriber]}, testUtils.context.external) .then(function (results) { @@ -45,11 +64,14 @@ describe('Subscribers API', function () { }); it('CANNOT add subscriber without context', function (done) { - SubscribersAPI.add({subscribers: [newSubscriber]}).then(function () { - done(new Error('Add subscriber is not denied without authentication.')); - }, function () { - done(); - }).catch(done); + SubscribersAPI.add({subscribers: [newSubscriber]}) + .then(function () { + done(new Error('Add subscriber without context should have no access.')); + }) + .catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + done(); + }); }); }); @@ -67,8 +89,8 @@ describe('Subscribers API', function () { }).catch(done); }); - it('can edit a subscriber (editor)', function (done) { - SubscribersAPI.edit({subscribers: [{email: newSubscriberEmail}]}, _.extend({}, context.editor, {id: firstSubscriber})) + it('can edit subscriber (external)', function (done) { + SubscribersAPI.edit({subscribers: [{email: newSubscriberEmail}]}, _.extend({}, context.external, {id: firstSubscriber})) .then(function (results) { should.exist(results); should.exist(results.subscribers); @@ -77,31 +99,44 @@ describe('Subscribers API', function () { }).catch(done); }); - // needs permissions to work properly - it.skip('CANNOT edit subscriber (external)', function (done) { - SubscribersAPI.edit({subscribers: [{email: newSubscriberEmail}]}, _.extend({}, context.external, {id: firstSubscriber})) - .then(function () { - done(new Error('Edit subscriber is not denied with external context.')); - }, function () { - done(); - }).catch(done); + it('CANNOT edit a subscriber (editor)', function (done) { + SubscribersAPI.edit({subscribers: [{email: newSubscriberEmail}]}, _.extend({}, context.editor, {id: firstSubscriber})) + .then(function () { + done(new Error('Edit subscriber as author should have no access.')); + }) + .catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + done(); + }); + }); + + it('CANNOT edit subscriber (author)', function (done) { + SubscribersAPI.edit({subscribers: [{email: newSubscriberEmail}]}, _.extend({}, context.author, {id: firstSubscriber})) + .then(function () { + done(new Error('Edit subscriber as author should have no access.')); + }) + .catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + done(); + }); }); it('CANNOT edit subscriber that doesn\'t exit', function (done) { SubscribersAPI.edit({subscribers: [{email: newSubscriberEmail}]}, _.extend({}, context.internal, {id: 999})) - .then(function () { - done(new Error('Edit non-existent subscriber is possible.')); - }, function (err) { - should.exist(err); - err.message.should.eql('Subscriber not found.'); - done(); - }).catch(done); + .then(function () { + done(new Error('Edit non-existent subscriber is possible.')); + }, function (err) { + should.exist(err); + (err instanceof errors.NotFoundError).should.eql(true); + done(); + }).catch(done); }); }); describe('Destroy', function () { var firstSubscriber = 1; - it('can destroy subscriber', function (done) { + + it('can destroy subscriber as admin', function (done) { SubscribersAPI.destroy(_.extend({}, testUtils.context.admin, {id: firstSubscriber})) .then(function (results) { should.not.exist(results); @@ -109,6 +144,17 @@ describe('Subscribers API', function () { done(); }).catch(done); }); + + it('CANNOT destroy subscriber', function (done) { + SubscribersAPI.destroy(_.extend({}, testUtils.context.editor, {id: firstSubscriber})) + .then(function () { + done(new Error('Destroy subscriber should not be possible as editor.')); + }, function (err) { + should.exist(err); + (err instanceof errors.NoPermissionError).should.eql(true); + done(); + }).catch(done); + }); }); describe('Browse', function () { @@ -131,13 +177,15 @@ describe('Subscribers API', function () { }).catch(done); }); - // needs permissions to work properly - it.skip('CANNOT browse subscriber (external)', function (done) { - SubscribersAPI.browse(testUtils.context.external).then(function () { - done(new Error('Browse subscriber is not denied with external context.')); - }, function () { - done(); - }).catch(done); + it('CANNOT browse subscriber (external)', function (done) { + SubscribersAPI.browse(testUtils.context.external) + .then(function () { + done(new Error('Browse subscriber should be denied with external context.')); + }) + .catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + done(); + }); }); }); @@ -163,12 +211,12 @@ describe('Subscribers API', function () { }).catch(done); }); - it('cannot fetch a subscriber which doesn\'t exist', function (done) { + it('CANNOT fetch a subscriber which doesn\'t exist', function (done) { SubscribersAPI.read({context: {user: 1}, id: 999}).then(function () { done(new Error('Should not return a result')); }).catch(function (err) { should.exist(err); - err.message.should.eql('Subscriber not found.'); + (err instanceof errors.NotFoundError).should.eql(true); done(); }); }); diff --git a/core/test/unit/api_utils_spec.js b/core/test/unit/api_utils_spec.js index 2d1dc230a9..421352c92f 100644 --- a/core/test/unit/api_utils_spec.js +++ b/core/test/unit/api_utils_spec.js @@ -442,7 +442,7 @@ describe('API Utils', function () { describe('handlePublicPermissions', function () { it('should return empty options if passed empty options', function (done) { apiUtils.handlePublicPermissions('tests', 'test')({}).then(function (options) { - options.should.eql({context: {app: null, internal: false, public: true, user: null}}); + options.should.eql({context: {app: null, external: false, internal: false, public: true, user: null}}); done(); }).catch(done); }); @@ -451,7 +451,7 @@ describe('API Utils', function () { var aPPStub = sandbox.stub(apiUtils, 'applyPublicPermissions').returns(Promise.resolve({})); apiUtils.handlePublicPermissions('tests', 'test')({}).then(function (options) { aPPStub.calledOnce.should.eql(true); - options.should.eql({context: {app: null, internal: false, public: true, user: null}}); + options.should.eql({context: {app: null, external: false, internal: false, public: true, user: null}}); done(); }).catch(done); }); @@ -467,7 +467,7 @@ describe('API Utils', function () { apiUtils.handlePublicPermissions('tests', 'test')({context: {user: 1}}).then(function (options) { cTStub.calledOnce.should.eql(true); cTMethodStub.test.test.calledOnce.should.eql(true); - options.should.eql({context: {app: null, internal: false, public: false, user: 1}}); + options.should.eql({context: {app: null, external: false, internal: false, public: false, user: 1}}); done(); }).catch(done); }); diff --git a/core/test/unit/permissions_spec.js b/core/test/unit/permissions_spec.js index eb24f2e742..76ed5d76b6 100644 --- a/core/test/unit/permissions_spec.js +++ b/core/test/unit/permissions_spec.js @@ -50,12 +50,14 @@ describe('Permissions', function () { it('should return public for no context', function () { permissions.parseContext().should.eql({ internal: false, + external: false, user: null, app: null, public: true }); permissions.parseContext({}).should.eql({ internal: false, + external: false, user: null, app: null, public: true @@ -65,12 +67,14 @@ describe('Permissions', function () { it('should return public for random context', function () { permissions.parseContext('public').should.eql({ internal: false, + external: false, user: null, app: null, public: true }); permissions.parseContext({client: 'thing'}).should.eql({ internal: false, + external: false, user: null, app: null, public: true @@ -80,6 +84,7 @@ describe('Permissions', function () { it('should return user if user populated', function () { permissions.parseContext({user: 1}).should.eql({ internal: false, + external: false, user: 1, app: null, public: false @@ -89,6 +94,7 @@ describe('Permissions', function () { it('should return app if app populated', function () { permissions.parseContext({app: 5}).should.eql({ internal: false, + external: false, user: null, app: 5, public: false @@ -98,6 +104,7 @@ describe('Permissions', function () { it('should return internal if internal provided', function () { permissions.parseContext({internal: true}).should.eql({ internal: true, + external: false, user: null, app: null, public: false @@ -105,6 +112,7 @@ describe('Permissions', function () { permissions.parseContext('internal').should.eql({ internal: true, + external: false, user: null, app: null, public: false