From 0462607296f2a1a5b3ac3a3d98eb254e5fdfd605 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Fri, 25 Sep 2015 17:11:22 +0100 Subject: [PATCH] Don't allow published_by override - published_by should be set by business logic, rather than by users Credits: An anonymous researcher working with Beyond Security's SecuriTeam Secure Disclosure program --- core/server/models/post.js | 16 +++- core/test/integration/api/api_users_spec.js | 1 - .../integration/model/model_posts_spec.js | 74 +++++++++++++++++-- 3 files changed, 80 insertions(+), 11 deletions(-) diff --git a/core/server/models/post.js b/core/server/models/post.js index c9718d4612..ac89eba606 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -140,15 +140,23 @@ Post = ghostBookshelf.Model.extend({ // this.set('title', this.sanitize('title').trim()); this.set('title', this.get('title').trim()); - if ((this.hasChanged('status') || !this.get('published_at')) && this.get('status') === 'published') { - if (!this.get('published_at')) { - this.set('published_at', new Date()); - } + // ### Business logic for published_at and published_by + // If the current status is 'published' and published_at is not set, set it to now + if (this.get('status') === 'published' && !this.get('published_at')) { + this.set('published_at', new Date()); + } + // If the current status is 'published' and the status has just changed ensure published_by is set correctly + if (this.get('status') === 'published' && this.hasChanged('status')) { // unless published_by is set and we're importing, set published_by to contextUser if (!(this.get('published_by') && options.importing)) { this.set('published_by', this.contextUser(options)); } + } else { + // In any other case (except import), `published_by` should not be changed + if (this.hasChanged('published_by') && !options.importing) { + this.set('published_by', this.previous('published_by')); + } } if (this.hasChanged('slug') || !this.get('slug')) { diff --git a/core/test/integration/api/api_users_spec.js b/core/test/integration/api/api_users_spec.js index 2b2fd42294..211a97f714 100644 --- a/core/test/integration/api/api_users_spec.js +++ b/core/test/integration/api/api_users_spec.js @@ -389,7 +389,6 @@ describe('Users API', function () { {users: [{name: 'newname', password: 'newpassword'}]}, _.extend({}, context.author, {id: userIdFor.author}) ).then(function () { return ModelUser.User.findOne({id: userIdFor.author}).then(function (response) { - console.log(response); response.get('name').should.eql('newname'); response.get('password').should.not.eql('newpassword'); done(); diff --git a/core/test/integration/model/model_posts_spec.js b/core/test/integration/model/model_posts_spec.js index b1801dfc77..20539087b7 100644 --- a/core/test/integration/model/model_posts_spec.js +++ b/core/test/integration/model/model_posts_spec.js @@ -346,7 +346,7 @@ describe('Post Model', function () { }); describe('edit', function () { - it('change title', function (done) { + it('can change title', function (done) { var postId = 1; PostModel.findOne({id: postId}).then(function (results) { @@ -368,7 +368,7 @@ describe('Post Model', function () { }).catch(done); }); - it('publish draft post', function (done) { + it('can publish draft post', function (done) { var postId = 4; PostModel.findOne({id: postId, status: 'draft'}).then(function (results) { @@ -390,7 +390,7 @@ describe('Post Model', function () { }).catch(done); }); - it('can edit: unpublish published post', function (done) { + it('can unpublish published post', function (done) { var postId = 1; PostModel.findOne({id: postId}).then(function (results) { @@ -412,7 +412,7 @@ describe('Post Model', function () { }).catch(done); }); - it('convert draft post to page and back', function (done) { + it('can convert draft post to page and back', function (done) { var postId = 4; PostModel.findOne({id: postId, status: 'draft'}).then(function (results) { @@ -443,7 +443,7 @@ describe('Post Model', function () { }).catch(done); }); - it('convert published post to page and back', function (done) { + it('can convert published post to page and back', function (done) { var postId = 1; PostModel.findOne({id: postId}).then(function (results) { @@ -480,7 +480,7 @@ describe('Post Model', function () { }).catch(done); }); - it('change type and status at the same time', function (done) { + it('can change type and status at the same time', function (done) { var postId = 4; PostModel.findOne({id: postId, status: 'draft'}).then(function (results) { @@ -512,6 +512,68 @@ describe('Post Model', function () { done(); }).catch(done); }); + + it.only('can save a draft without setting published_by or published_at', function (done) { + var newPost = testUtils.DataGenerator.forModel.posts[2], + postId; + + PostModel.add(newPost, context).then(function (results) { + var post; + should.exist(results); + post = results.toJSON(); + postId = post.id; + + post.status.should.equal('draft'); + should.not.exist(post.published_by); + should.not.exist(post.published_at); + + // Test changing an unrelated property + return PostModel.edit({title: 'Hello World'}, _.extend({}, context, {id: postId})); + }).then(function (edited) { + should.exist(edited); + edited.attributes.status.should.equal('draft'); + should.not.exist(edited.attributes.published_by); + should.not.exist(edited.attributes.published_at); + + // Test changing status and published_by on its own + return PostModel.edit({published_by: 4}, _.extend({}, context, {id: postId})); + }).then(function (edited) { + should.exist(edited); + edited.attributes.status.should.equal('draft'); + should.not.exist(edited.attributes.published_by); + should.not.exist(edited.attributes.published_at); + + done(); + }).catch(done); + }); + + it('cannot override the published_by setting', function (done) { + var postId = 4; + + PostModel.findOne({id: postId, status: 'draft'}).then(function (results) { + var post; + should.exist(results); + post = results.toJSON(); + post.id.should.equal(postId); + post.status.should.equal('draft'); + + // Test changing status and published_by at the same time + return PostModel.edit({status: 'published', published_by: 4}, _.extend({}, context, {id: postId})); + }).then(function (edited) { + should.exist(edited); + edited.attributes.status.should.equal('published'); + edited.attributes.published_by.should.equal(context.context.user); + + // Test changing status and published_by on its own + return PostModel.edit({published_by: 4}, _.extend({}, context, {id: postId})); + }).then(function (edited) { + should.exist(edited); + edited.attributes.status.should.equal('published'); + edited.attributes.published_by.should.equal(context.context.user); + + done(); + }).catch(done); + }); }); describe('add', function () {