From 853b518a5151e43da47008dcb15fafbb1a7fd9d6 Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Thu, 5 Apr 2018 16:11:47 +0200 Subject: [PATCH] Sanitize incoming model relation data refs #9548 - we always receive date strings from the client in ISO format - we ensure that we transform these strings into JS dates for comparison - when the client sends relations, we need to ensure that relations are checked as well - will only work for the post model for now, because this is the only model which uses `bookshelf-relations` - added unit tests - removed some model tests, which do the same --- core/server/models/base/index.js | 37 ++++++++++-- core/server/models/post.js | 6 ++ .../integration/model/model_posts_spec.js | 45 -------------- core/test/unit/models/base/index_spec.js | 60 ++++++++++++++++++- 4 files changed, 97 insertions(+), 51 deletions(-) diff --git a/core/server/models/base/index.js b/core/server/models/base/index.js index 8b93416180..c5471373c7 100644 --- a/core/server/models/base/index.js +++ b/core/server/models/base/index.js @@ -496,14 +496,17 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({ * @IMPORTANT * Before the new client data get's inserted again, the dates get's re-transformed into * proper strings, see `format`. + * + * @IMPORTANT + * Sanitize relations. */ sanitizeData: function sanitizeData(data) { var tableName = _.result(this.prototype, 'tableName'), date; - _.each(data, function (value, key) { + _.each(data, (value, property) => { if (value !== null - && schema.tables[tableName].hasOwnProperty(key) - && schema.tables[tableName][key].type === 'dateTime' + && schema.tables[tableName].hasOwnProperty(property) + && schema.tables[tableName][property].type === 'dateTime' && typeof value === 'string' ) { date = new Date(value); @@ -511,12 +514,36 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({ // CASE: client sends `0000-00-00 00:00:00` if (isNaN(date)) { throw new common.errors.ValidationError({ - message: common.i18n.t('errors.models.base.invalidDate', {key: key}), + message: common.i18n.t('errors.models.base.invalidDate', {key: property}), code: 'DATE_INVALID' }); } - data[key] = moment(value).toDate(); + data[property] = moment(value).toDate(); + } + + if (this.prototype.relationships && this.prototype.relationships.indexOf(property) !== -1) { + _.each(data[property], (relation, indexInArr) => { + _.each(relation, (value, relationProperty) => { + if (value !== null + && schema.tables[this.prototype.relationshipBelongsTo[property]].hasOwnProperty(relationProperty) + && schema.tables[this.prototype.relationshipBelongsTo[property]][relationProperty].type === 'dateTime' + && typeof value === 'string' + ) { + date = new Date(value); + + // CASE: client sends `0000-00-00 00:00:00` + if (isNaN(date)) { + throw new common.errors.ValidationError({ + message: common.i18n.t('errors.models.base.invalidDate', {key: relationProperty}), + code: 'DATE_INVALID' + }); + } + + data[property][indexInArr][relationProperty] = moment(value).toDate(); + } + }); + }); } }); diff --git a/core/server/models/post.js b/core/server/models/post.js index b3f6d88cd2..16c4c95ff7 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -39,6 +39,12 @@ Post = ghostBookshelf.Model.extend({ relationships: ['tags', 'authors'], + // NOTE: look up object, not super nice, but was easy to implement + relationshipBelongsTo: { + tags: 'tags', + authors: 'users' + }, + /** * The base model keeps only the columns, which are defined in the schema. * We have to add the relations on top, otherwise bookshelf-relations diff --git a/core/test/integration/model/model_posts_spec.js b/core/test/integration/model/model_posts_spec.js index 9d1b0ca176..94193157b8 100644 --- a/core/test/integration/model/model_posts_spec.js +++ b/core/test/integration/model/model_posts_spec.js @@ -592,27 +592,6 @@ describe('Post Model', function () { }); }); - it('draft -> scheduled: invalid published_at update', function (done) { - PostModel.findOne({status: 'draft'}).then(function (results) { - var post; - - should.exist(results); - post = results.toJSON(); - post.status.should.equal('draft'); - - // @TODO: add unit test for valid and invalid formats - return PostModel.edit({ - status: 'scheduled', - published_at: '0000-00-00 00:00:00' - }, _.extend({}, context, {id: post.id})); - }).catch(function (err) { - should.exist(err); - (err instanceof common.errors.ValidationError).should.eql(true); - err.code.should.eql('DATE_INVALID'); - done(); - }); - }); - it('draft -> scheduled: expect update of published_at', function (done) { var newPublishedAt = moment().add(1, 'day').toDate(); @@ -976,30 +955,6 @@ describe('Post Model', function () { }).catch(done); }); - it('send invalid published_at date', function (done) { - var postId = testUtils.DataGenerator.Content.posts[0].id; - - PostModel - .findOne({ - id: postId - }) - .then(function (results) { - var post; - should.exist(results); - post = results.toJSON(); - post.id.should.equal(postId); - - return PostModel.edit({published_at: '0000-00-00 00:00:00'}, _.extend({}, context, {id: postId})); - }) - .then(function () { - done(new Error('This test should fail.')); - }) - .catch(function (err) { - err.statusCode.should.eql(422); - done(); - }); - }); - it('send empty date', function (done) { var postId = testUtils.DataGenerator.Content.posts[0].id; diff --git a/core/test/unit/models/base/index_spec.js b/core/test/unit/models/base/index_spec.js index a9c43b3d8b..9a1a852a3f 100644 --- a/core/test/unit/models/base/index_spec.js +++ b/core/test/unit/models/base/index_spec.js @@ -18,7 +18,65 @@ describe('Models: base', function () { sandbox.restore(); }); - describe('setEmptyValuesToNull', function () { + describe('fn: sanitizeData', function () { + it('date is invalid', function () { + const data = testUtils.DataGenerator.forKnex.createPost({updated_at: '0000-00-00 00:00:00'}); + + try { + models.Base.Model.sanitizeData + .bind({prototype: {tableName: 'posts'}})(data); + } catch (err) { + err.code.should.eql('DATE_INVALID'); + } + }); + + it('expect date transformation', function () { + const data = testUtils.DataGenerator.forKnex.createPost({updated_at: '2018-04-01 07:53:07'}); + + data.updated_at.should.be.a.String(); + + models.Base.Model.sanitizeData + .bind({prototype: {tableName: 'posts'}})(data); + + data.updated_at.should.be.a.Date(); + }); + + it('date is JS date, ignore', function () { + const data = testUtils.DataGenerator.forKnex.createPost({updated_at: new Date()}); + + data.updated_at.should.be.a.Date(); + + models.Base.Model.sanitizeData + .bind({prototype: {tableName: 'posts'}})(data); + + data.updated_at.should.be.a.Date(); + }); + + it('expect date transformation for nested relations', function () { + const data = testUtils.DataGenerator.forKnex.createPost({ + authors: [{ + name: 'Thomas', + updated_at: '2018-04-01 07:53:07' + }] + }); + + data.authors[0].updated_at.should.be.a.String(); + + models.Base.Model.sanitizeData + .bind({ + prototype: { + tableName: 'posts', + relationships: ['authors'], + relationshipBelongsTo: {authors: 'users'} + } + })(data); + + data.authors[0].name.should.eql('Thomas'); + data.authors[0].updated_at.should.be.a.Date(); + }); + }); + + describe('fn: setEmptyValuesToNull', function () { it('resets given empty value to null', function () { const base = models.Base.Model.forge({a: '', b: ''});