mirror of
https://github.com/TryGhost/Ghost.git
synced 2024-09-21 18:08:51 +03:00
Removed permissions handling of "author_id"
refs https://github.com/TryGhost/Toolbox/issues/230 - Single author logic can be removed as we dropped `author_id` column from the post model
This commit is contained in:
Naz
naz
parent
36e54656b6
commit
945345d990
@ -154,14 +154,6 @@ module.exports.extendModel = function extendModel(Post, Posts, ghostBookshelf) {
|
||||
}
|
||||
|
||||
attrs.author = attrs.authors[0];
|
||||
delete attrs.author_id;
|
||||
} else {
|
||||
// CASE: we return `post.author=id` with or without requested columns.
|
||||
// @NOTE: this serialization should be moved into api layer, it's not being moved as it's deprecated
|
||||
if (!options.columns || (options.columns && options.columns.indexOf('author') !== -1)) {
|
||||
attrs.author = attrs.author_id;
|
||||
delete attrs.author_id;
|
||||
}
|
||||
}
|
||||
|
||||
// CASE: `posts.authors` was not requested, but fetched in specific cases (see top)
|
||||
@ -366,10 +358,6 @@ module.exports.extendModel = function extendModel(Post, Posts, ghostBookshelf) {
|
||||
isAdd = (action === 'add');
|
||||
isDestroy = (action === 'destroy');
|
||||
|
||||
function isChanging(attr) {
|
||||
return unsafeAttrs[attr] && unsafeAttrs[attr] !== postModel.get(attr);
|
||||
}
|
||||
|
||||
function isChangingAuthors() {
|
||||
if (!unsafeAttrs.authors) {
|
||||
return false;
|
||||
@ -385,14 +373,10 @@ module.exports.extendModel = function extendModel(Post, Posts, ghostBookshelf) {
|
||||
function isOwner() {
|
||||
let isCorrectOwner = true;
|
||||
|
||||
if (!unsafeAttrs.author_id && !unsafeAttrs.authors) {
|
||||
if (!unsafeAttrs.authors) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (unsafeAttrs.author_id) {
|
||||
isCorrectOwner = unsafeAttrs.author_id && unsafeAttrs.author_id === context.user;
|
||||
}
|
||||
|
||||
if (unsafeAttrs.authors) {
|
||||
isCorrectOwner = isCorrectOwner && unsafeAttrs.authors.length && unsafeAttrs.authors[0].id === context.user;
|
||||
}
|
||||
@ -409,13 +393,13 @@ module.exports.extendModel = function extendModel(Post, Posts, ghostBookshelf) {
|
||||
}
|
||||
|
||||
if (isContributor && isEdit) {
|
||||
hasUserPermission = !isChanging('author_id') && !isChangingAuthors() && isCoAuthor();
|
||||
hasUserPermission = !isChangingAuthors() && isCoAuthor();
|
||||
} else if (isContributor && isAdd) {
|
||||
hasUserPermission = isOwner();
|
||||
} else if (isContributor && isDestroy) {
|
||||
hasUserPermission = isPrimaryAuthor();
|
||||
} else if (isAuthor && isEdit) {
|
||||
hasUserPermission = isCoAuthor() && !isChanging('author_id') && !isChangingAuthors();
|
||||
hasUserPermission = isCoAuthor() && !isChangingAuthors();
|
||||
} else if (isAuthor && isAdd) {
|
||||
hasUserPermission = isOwner();
|
||||
} else if (postModel) {
|
||||
@ -435,7 +419,6 @@ module.exports.extendModel = function extendModel(Post, Posts, ghostBookshelf) {
|
||||
// @TODO: we need a concept for making a diff between incoming authors and existing authors
|
||||
// @TODO: for now we simply re-use the new concept of `excludedAttrs`
|
||||
// We only check the primary author of `authors`, any other change will be ignored.
|
||||
// By this we can deprecate `author_id` more easily.
|
||||
if (isContributor || isAuthor) {
|
||||
return {
|
||||
excludedAttrs: ['authors'].concat(excludedAttrs)
|
||||
|
||||
@ -417,35 +417,6 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects if changing author id', function (done) {
|
||||
const mockPostObj = {
|
||||
get: sinon.stub(),
|
||||
related: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {status: 'draft', author_id: 2};
|
||||
|
||||
mockPostObj.get.withArgs('author_id').returns(1);
|
||||
|
||||
models.Post.permissible(
|
||||
mockPostObj,
|
||||
'edit',
|
||||
context,
|
||||
unsafeAttrs,
|
||||
testUtils.permissions.contributor,
|
||||
false,
|
||||
true,
|
||||
true
|
||||
).then(() => {
|
||||
done(new Error('Permissible function should have rejected.'));
|
||||
}).catch((error) => {
|
||||
error.should.be.an.instanceof(errors.NoPermissionError);
|
||||
should(mockPostObj.get.calledOnce).be.true();
|
||||
should(mockPostObj.related.called).be.false();
|
||||
done();
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects if changing authors.0', function (done) {
|
||||
const mockPostObj = {
|
||||
get: sinon.stub(),
|
||||
@ -510,10 +481,9 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
related: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {status: 'published', author_id: 1};
|
||||
const unsafeAttrs = {status: 'published'};
|
||||
|
||||
mockPostObj.get.withArgs('status').returns('published');
|
||||
mockPostObj.get.withArgs('author_id').returns(1);
|
||||
mockPostObj.related.withArgs('authors').returns({models: [{id: 1}]});
|
||||
|
||||
models.Post.permissible(
|
||||
@ -529,7 +499,7 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
done(new Error('Permissible function should have rejected.'));
|
||||
}).catch((error) => {
|
||||
error.should.be.an.instanceof(errors.NoPermissionError);
|
||||
should(mockPostObj.get.callCount).eql(3);
|
||||
should(mockPostObj.get.callCount).eql(2);
|
||||
should(mockPostObj.related.callCount).eql(1);
|
||||
done();
|
||||
});
|
||||
@ -541,10 +511,8 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
related: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {status: 'draft', author_id: 2};
|
||||
const unsafeAttrs = {status: 'draft', authors: [{id: 2}]};
|
||||
|
||||
mockPostObj.get.withArgs('status').returns('draft');
|
||||
mockPostObj.get.withArgs('author_id').returns(1);
|
||||
mockPostObj.related.withArgs('authors').returns({models: [{id: 1}]});
|
||||
|
||||
models.Post.permissible(
|
||||
@ -560,8 +528,7 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
done(new Error('Permissible function should have rejected.'));
|
||||
}).catch((error) => {
|
||||
error.should.be.an.instanceof(errors.NoPermissionError);
|
||||
should(mockPostObj.get.callCount).eql(1);
|
||||
should(mockPostObj.related.callCount).eql(0);
|
||||
should(mockPostObj.related.callCount).eql(1);
|
||||
done();
|
||||
});
|
||||
});
|
||||
@ -572,10 +539,9 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
related: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {status: 'draft', author_id: 1};
|
||||
const unsafeAttrs = {status: 'draft'};
|
||||
|
||||
mockPostObj.get.withArgs('status').returns('draft');
|
||||
mockPostObj.get.withArgs('author_id').returns(1);
|
||||
mockPostObj.related.withArgs('authors').returns({models: [{id: 1}]});
|
||||
|
||||
return models.Post.permissible(
|
||||
@ -590,7 +556,7 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
).then((result) => {
|
||||
should.exist(result);
|
||||
should(result.excludedAttrs).deepEqual(['authors', 'tags']);
|
||||
should(mockPostObj.get.callCount).eql(3);
|
||||
should(mockPostObj.get.callCount).eql(2);
|
||||
should(mockPostObj.related.callCount).eql(1);
|
||||
});
|
||||
});
|
||||
@ -602,7 +568,7 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
get: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {status: 'published', author_id: 1};
|
||||
const unsafeAttrs = {status: 'published', authors: [{id: 1}]};
|
||||
|
||||
models.Post.permissible(
|
||||
mockPostObj,
|
||||
@ -627,7 +593,7 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
get: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {status: 'draft', author_id: 2};
|
||||
const unsafeAttrs = {status: 'draft', authors: [{id: 2}]};
|
||||
|
||||
models.Post.permissible(
|
||||
mockPostObj,
|
||||
@ -672,56 +638,6 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects if same logged in user and `authors.0`, but different author_id', function (done) {
|
||||
const mockPostObj = {
|
||||
get: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {status: 'draft', author_id: 3, authors: [{id: 1}]};
|
||||
|
||||
models.Post.permissible(
|
||||
mockPostObj,
|
||||
'add',
|
||||
context,
|
||||
unsafeAttrs,
|
||||
testUtils.permissions.contributor,
|
||||
false,
|
||||
true,
|
||||
true
|
||||
).then(() => {
|
||||
done(new Error('Permissible function should have rejected.'));
|
||||
}).catch((error) => {
|
||||
error.should.be.an.instanceof(errors.NoPermissionError);
|
||||
should(mockPostObj.get.called).be.false();
|
||||
done();
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects if different logged in user and `authors.0`, but correct author_id', function (done) {
|
||||
const mockPostObj = {
|
||||
get: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {status: 'draft', author_id: 1, authors: [{id: 2}]};
|
||||
|
||||
models.Post.permissible(
|
||||
mockPostObj,
|
||||
'add',
|
||||
context,
|
||||
unsafeAttrs,
|
||||
testUtils.permissions.contributor,
|
||||
false,
|
||||
true,
|
||||
true
|
||||
).then(() => {
|
||||
done(new Error('Permissible function should have rejected.'));
|
||||
}).catch((error) => {
|
||||
error.should.be.an.instanceof(errors.NoPermissionError);
|
||||
should(mockPostObj.get.called).be.false();
|
||||
done();
|
||||
});
|
||||
});
|
||||
|
||||
it('resolves if same logged in user and `authors.0`', function (done) {
|
||||
const mockPostObj = {
|
||||
get: sinon.stub()
|
||||
@ -745,29 +661,6 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
done();
|
||||
}).catch(done);
|
||||
});
|
||||
|
||||
it('resolves if none of the above cases are true', function () {
|
||||
const mockPostObj = {
|
||||
get: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {status: 'draft', author_id: 1};
|
||||
|
||||
return models.Post.permissible(
|
||||
mockPostObj,
|
||||
'add',
|
||||
context,
|
||||
unsafeAttrs,
|
||||
testUtils.permissions.contributor,
|
||||
false,
|
||||
true,
|
||||
true
|
||||
).then((result) => {
|
||||
should.exist(result);
|
||||
should(result.excludedAttrs).deepEqual(['authors', 'tags']);
|
||||
should(mockPostObj.get.called).be.false();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('Destroying', function () {
|
||||
@ -865,10 +758,9 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
related: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {author_id: 2};
|
||||
const unsafeAttrs = {authors: {id: 2}};
|
||||
|
||||
mockPostObj.related.withArgs('authors').returns({models: [{id: 2}]});
|
||||
mockPostObj.get.withArgs('author_id').returns(2);
|
||||
|
||||
models.Post.permissible(
|
||||
mockPostObj,
|
||||
@ -948,36 +840,6 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects if changing author', function (done) {
|
||||
const mockPostObj = {
|
||||
get: sinon.stub(),
|
||||
related: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {author_id: 2};
|
||||
|
||||
mockPostObj.get.withArgs('author_id').returns(1);
|
||||
mockPostObj.related.withArgs('authors').returns({models: [{id: 1}]});
|
||||
|
||||
models.Post.permissible(
|
||||
mockPostObj,
|
||||
'edit',
|
||||
context,
|
||||
unsafeAttrs,
|
||||
testUtils.permissions.author,
|
||||
false,
|
||||
true,
|
||||
true
|
||||
).then(() => {
|
||||
done(new Error('Permissible function should have rejected.'));
|
||||
}).catch((error) => {
|
||||
error.should.be.an.instanceof(errors.NoPermissionError);
|
||||
should(mockPostObj.get.calledOnce).be.true();
|
||||
should(mockPostObj.related.calledOnce).be.true();
|
||||
done();
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects if changing authors', function (done) {
|
||||
const mockPostObj = {
|
||||
get: sinon.stub(),
|
||||
@ -1007,15 +869,13 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects if changing authors and author_id', function (done) {
|
||||
it('rejects if changing authors', function (done) {
|
||||
const mockPostObj = {
|
||||
get: sinon.stub(),
|
||||
related: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {authors: [{id: 1}], author_id: 2};
|
||||
const unsafeAttrs = {authors: [{id: 2}]};
|
||||
|
||||
mockPostObj.get.withArgs('author_id').returns(1);
|
||||
mockPostObj.related.withArgs('authors').returns({models: [{id: 1}]});
|
||||
|
||||
models.Post.permissible(
|
||||
@ -1031,21 +891,18 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
done(new Error('Permissible function should have rejected.'));
|
||||
}).catch((error) => {
|
||||
error.should.be.an.instanceof(errors.NoPermissionError);
|
||||
should(mockPostObj.get.calledOnce).be.true();
|
||||
should(mockPostObj.related.calledOnce).be.true();
|
||||
should(mockPostObj.related.calledTwice).be.true();
|
||||
done();
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects if changing authors and author_id', function (done) {
|
||||
it('rejects if changing authors', function (done) {
|
||||
const mockPostObj = {
|
||||
get: sinon.stub(),
|
||||
related: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {authors: [{id: 2}], author_id: 1};
|
||||
const unsafeAttrs = {authors: [{id: 2}]};
|
||||
|
||||
mockPostObj.get.withArgs('author_id').returns(1);
|
||||
mockPostObj.related.withArgs('authors').returns({models: [{id: 1}]});
|
||||
|
||||
models.Post.permissible(
|
||||
@ -1061,7 +918,6 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
done(new Error('Permissible function should have rejected.'));
|
||||
}).catch((error) => {
|
||||
error.should.be.an.instanceof(errors.NoPermissionError);
|
||||
mockPostObj.get.callCount.should.eql(1);
|
||||
mockPostObj.related.callCount.should.eql(2);
|
||||
done();
|
||||
});
|
||||
@ -1069,13 +925,11 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
|
||||
it('resolves if none of the above cases are true', function () {
|
||||
const mockPostObj = {
|
||||
get: sinon.stub(),
|
||||
related: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {author_id: 1};
|
||||
const unsafeAttrs = {};
|
||||
|
||||
mockPostObj.get.withArgs('author_id').returns(1);
|
||||
mockPostObj.related.withArgs('authors').returns({models: [{id: 1}]});
|
||||
|
||||
return models.Post.permissible(
|
||||
@ -1088,7 +942,6 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
true,
|
||||
true
|
||||
).then(() => {
|
||||
should(mockPostObj.get.calledOnce).be.true();
|
||||
should(mockPostObj.related.calledOnce).be.true();
|
||||
});
|
||||
});
|
||||
@ -1100,7 +953,7 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
get: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {author_id: 2};
|
||||
const unsafeAttrs = {authors: [{id: 2}]};
|
||||
|
||||
models.Post.permissible(
|
||||
mockPostObj,
|
||||
@ -1147,27 +1000,6 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
done();
|
||||
});
|
||||
});
|
||||
|
||||
it('resolves if none of the above cases are true', function () {
|
||||
const mockPostObj = {
|
||||
get: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {author_id: 1};
|
||||
|
||||
return models.Post.permissible(
|
||||
mockPostObj,
|
||||
'add',
|
||||
context,
|
||||
unsafeAttrs,
|
||||
testUtils.permissions.author,
|
||||
false,
|
||||
true,
|
||||
true
|
||||
).then(() => {
|
||||
should(mockPostObj.get.called).be.false();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@ -1178,10 +1010,9 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
related: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {author_id: 2};
|
||||
const unsafeAttrs = {authors: [{id: 2}]};
|
||||
|
||||
mockPostObj.related.withArgs('authors').returns({models: [{id: 2}]});
|
||||
mockPostObj.get.withArgs('author_id').returns(2);
|
||||
|
||||
models.Post.permissible(
|
||||
mockPostObj,
|
||||
@ -1207,9 +1038,7 @@ describe('Unit: models/post: uses database (@TODO: fix me)', function () {
|
||||
get: sinon.stub()
|
||||
};
|
||||
const context = {user: 1};
|
||||
const unsafeAttrs = {author_id: 2};
|
||||
|
||||
mockPostObj.get.withArgs('author_id').returns(2);
|
||||
const unsafeAttrs = {authors: [{id: 2}]};
|
||||
|
||||
return models.Post.permissible(
|
||||
mockPostObj,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user