Subscribers: finish permission handling

no issue
- add some more tests, optimise tests and finish tests
- subscriber model checks external context permissions in permissible fn
- add missing permissions for subscriber csv
This commit is contained in:
kirrg001 2016-04-29 12:58:40 +02:00 committed by Hannah Wolfe
parent a836081f54
commit ef605c5191
7 changed files with 162 additions and 55 deletions

View File

@ -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
];

View File

@ -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({

View File

@ -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

View File

@ -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.",

View File

@ -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();
});
});

View File

@ -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);
});

View File

@ -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