From 5a61f99467c3db822a4365ecba3f443b602747a0 Mon Sep 17 00:00:00 2001 From: Katharina Irrgang Date: Fri, 22 Jun 2018 20:28:01 +0200 Subject: [PATCH] Dynamic Routing: Added migration for routes.yaml file (#9692) refs #9601 - the home.hbs behaviour for the index collection (`/`) is hardcoded in Ghost - we would like to migrate all existing routes.yaml files - we only replace the file if the contents of the routes.yaml file equals the old routes.yaml format (with home.hbs as template) - updated README of settings folder - if we don't remove the home.hbs template from the default routes.yaml file, home.hbs will be rendered for any page of the index collection - the backwards compatible behaviour was different - only render home.hbs for page 1 - remember: the default routes.yaml file reflects how Ghost was working without dynamic routing --- content/settings/README.md | 1 - core/server/lib/security/tokens.js | 16 +- core/server/models/invite.js | 2 +- .../services/settings/default-routes.yaml | 1 - .../services/settings/ensure-settings.js | 23 ++- .../services/settings/ensure-settings_spec.js | 154 +++++++++++++++--- .../utils/fixtures/settings/badroutes.yaml | 1 - .../utils/fixtures/settings/goodroutes.yaml | 1 - core/test/utils/fixtures/settings/routes.yaml | 1 - 9 files changed, 168 insertions(+), 32 deletions(-) diff --git a/content/settings/README.md b/content/settings/README.md index eaaad52042..b1111041d2 100644 --- a/content/settings/README.md +++ b/content/settings/README.md @@ -13,7 +13,6 @@ collections: /: permalink: '{globals.permalinks}' template: - - home - index taxonomies: diff --git a/core/server/lib/security/tokens.js b/core/server/lib/security/tokens.js index 8899514301..4756a46832 100644 --- a/core/server/lib/security/tokens.js +++ b/core/server/lib/security/tokens.js @@ -1,6 +1,20 @@ const crypto = require('crypto'); -module.exports.generateHash = function generateHash(options) { +module.exports.generateFromContent = function generateFromContent(options) { + options = options || {}; + + const hash = crypto.createHash('sha256'), + content = options.content; + + let text = ''; + + hash.update(content); + + text += [content, hash.digest('base64')].join('|'); + return new Buffer(text).toString('base64'); +}; + +module.exports.generateFromEmail = function generateFromEmail(options) { options = options || {}; const hash = crypto.createHash('sha256'), diff --git a/core/server/models/invite.js b/core/server/models/invite.js index dfb7a7717f..3e79b3d3cd 100644 --- a/core/server/models/invite.js +++ b/core/server/models/invite.js @@ -34,7 +34,7 @@ Invite = ghostBookshelf.Model.extend({ } data.expires = Date.now() + constants.ONE_WEEK_MS; - data.token = security.tokens.generateHash({ + data.token = security.tokens.generateFromEmail({ email: data.email, expires: data.expires, secret: settingsCache.get('db_hash') diff --git a/core/server/services/settings/default-routes.yaml b/core/server/services/settings/default-routes.yaml index 7b53c6542f..b3df485a14 100644 --- a/core/server/services/settings/default-routes.yaml +++ b/core/server/services/settings/default-routes.yaml @@ -4,7 +4,6 @@ collections: /: permalink: '{globals.permalinks}' # special 1.0 compatibility setting. See the docs for details. template: - - home - index taxonomies: diff --git a/core/server/services/settings/ensure-settings.js b/core/server/services/settings/ensure-settings.js index 3add825f04..589b760bfd 100644 --- a/core/server/services/settings/ensure-settings.js +++ b/core/server/services/settings/ensure-settings.js @@ -3,7 +3,11 @@ const fs = require('fs-extra'), path = require('path'), debug = require('ghost-ignition').debug('services:settings:ensure-settings'), common = require('../../lib/common'), - config = require('../../config'); + security = require('../../lib/security'), + config = require('../../config'), + yamlMigrations = { + homeTemplate: 'cm91dGVzOmNvbGxlY3Rpb25zOi86cGVybWFsaW5rOid7Z2xvYmFscy5wZXJtYWxpbmtzfScjc3BlY2lhbDEuMGNvbXBhdGliaWxpdHlzZXR0aW5nLlNlZXRoZWRvY3Nmb3JkZXRhaWxzLnRlbXBsYXRlOi1ob21lLWluZGV4dGF4b25vbWllczp0YWc6L3RhZy97c2x1Z30vYXV0aG9yOi9hdXRob3Ive3NsdWd9L3wwUUg4SHRFQWZvbHBBSmVTYWkyOEwwSGFNMGFIOU5SczhZWGhMcExmZ2c0PQ==' + }; /** * Makes sure that all supported settings files are in the @@ -24,7 +28,22 @@ module.exports = function ensureSettingsFiles(knownSettings) { defaultFileName = `default-${fileName}`, filePath = path.join(contentPath, fileName); - return fs.access(filePath) + return fs.readFile(filePath, 'utf8') + .then((content) => { + content = content.replace(/\s/g, ''); + + /** + * routes.yaml migrations: + * + * 1. We have removed the "home.hbs" template from the default collection, because + * this is a hardcoded behaviour in Ghost. If we don't remove the home.hbs every page of the + * index collection get's rendered with the home template, but this is not the correct behaviour + * < 1.24. The index collection is `/`. + */ + if (security.tokens.generateFromContent({content: content}) === yamlMigrations.homeTemplate) { + throw new common.errors.ValidationError({code: 'ENOENT'}); + } + }) .catch({code: 'ENOENT'}, () => { // CASE: file doesn't exist, copy it from our defaults return fs.copy( diff --git a/core/test/unit/services/settings/ensure-settings_spec.js b/core/test/unit/services/settings/ensure-settings_spec.js index d847236acd..826534cd6b 100644 --- a/core/test/unit/services/settings/ensure-settings_spec.js +++ b/core/test/unit/services/settings/ensure-settings_spec.js @@ -13,6 +13,8 @@ const sinon = require('sinon'), describe('UNIT > Settings Service:', function () { beforeEach(function () { configUtils.set('paths:contentPath', path.join(__dirname, '../../../utils/fixtures/')); + sandbox.stub(fs, 'readFile'); + sandbox.stub(fs, 'copy'); }); afterEach(function () { @@ -22,10 +24,12 @@ describe('UNIT > Settings Service:', function () { describe('Ensure settings files', function () { it('returns yaml file from settings folder if it exists', function () { - const fsAccessSpy = sandbox.spy(fs, 'access'); + fs.readFile.withArgs(path.join(__dirname, '../../../utils/fixtures/settings/badroutes.yaml'), 'utf8').resolves('content'); + fs.readFile.withArgs(path.join(__dirname, '../../../utils/fixtures/settings/goodroutes.yaml'), 'utf8').resolves('content'); return ensureSettings(['goodroutes', 'badroutes']).then(() => { - fsAccessSpy.callCount.should.be.eql(2); + fs.readFile.callCount.should.be.eql(2); + fs.copy.called.should.be.false(); }); }); @@ -34,20 +38,16 @@ describe('UNIT > Settings Service:', function () { const expectedContentPath = path.join(__dirname, '../../../utils/fixtures/settings/globals.yaml'); const fsError = new Error('not found'); fsError.code = 'ENOENT'; - const fsAccessStub = sandbox.stub(fs, 'access'); - const fsCopyStub = sandbox.stub(fs, 'copy').resolves(); - fsAccessStub.onFirstCall().resolves(); - // route file in settings directotry is not found - fsAccessStub.onSecondCall().rejects(fsError); + fs.readFile.withArgs(path.join(__dirname, '../../../utils/fixtures/settings/routes.yaml'), 'utf8').resolves('content'); + fs.readFile.withArgs(path.join(__dirname, '../../../utils/fixtures/settings/globals.yaml'), 'utf8').rejects(fsError); + fs.copy.withArgs(expectedDefaultSettingsPath, expectedContentPath).resolves(); return ensureSettings(['routes', 'globals']) - .then(() => { - fsAccessStub.calledTwice.should.be.true(); - }).then(() => { - fsCopyStub.calledWith(expectedDefaultSettingsPath, expectedContentPath).should.be.true(); - fsCopyStub.calledOnce.should.be.true(); - }); + .then(() => { + fs.readFile.calledTwice.should.be.true(); + fs.copy.calledOnce.should.be.true(); + }); }); it('copies default settings file if no file found', function () { @@ -55,13 +55,13 @@ describe('UNIT > Settings Service:', function () { const expectedContentPath = path.join(__dirname, '../../../utils/fixtures/settings/routes.yaml'); const fsError = new Error('not found'); fsError.code = 'ENOENT'; - const fsAccessStub = sandbox.stub(fs, 'access').rejects(fsError); - const fsCopyStub = sandbox.stub(fs, 'copy').resolves(); + + fs.readFile.withArgs(path.join(__dirname, '../../../utils/fixtures/settings/routes.yaml'), 'utf8').rejects(fsError); + fs.copy.withArgs(expectedDefaultSettingsPath, expectedContentPath).resolves(); return ensureSettings(['routes']).then(() => { - fsAccessStub.calledOnce.should.be.true(); - fsCopyStub.calledWith(expectedDefaultSettingsPath, expectedContentPath).should.be.true(); - fsCopyStub.calledOnce.should.be.true(); + fs.readFile.calledOnce.should.be.true(); + fs.copy.calledOnce.should.be.true(); }); }); @@ -69,12 +69,120 @@ describe('UNIT > Settings Service:', function () { const expectedContentPath = path.join(__dirname, '../../../utils/fixtures/settings/'); const fsError = new Error('no permission'); fsError.code = 'EPERM'; - const fsAccessStub = sandbox.stub(fs, 'access').rejects(new Error('Oopsi!')); - return ensureSettings(['routes']).catch((error) => { - should.exist(error); - error.message.should.be.eql(`Error trying to access settings files in ${expectedContentPath}.`); - fsAccessStub.calledOnce.should.be.true(); + fs.readFile.withArgs(path.join(__dirname, '../../../utils/fixtures/settings/routes.yaml'), 'utf8').rejects(fsError); + + return ensureSettings(['routes']) + .then(()=> { + throw new Error('Expected test to fail'); + }) + .catch((error) => { + should.exist(error); + error.message.should.be.eql(`Error trying to access settings files in ${expectedContentPath}.`); + fs.readFile.calledOnce.should.be.true(); + fs.copy.called.should.be.false(); + }); + }); + }); + + describe('Migrations: home.hbs', function () { + it('routes.yaml has modifications', function () { + fs.readFile.withArgs(path.join(__dirname, '../../../utils/fixtures/settings/routes.yaml'), 'utf8').resolves('' + + 'routes:\n' + + '\n' + + 'collections:\n' + + ' /:\n' + + ' permalink: \'{globals.permalinks}\' # special 1.0 compatibility setting. See the docs for details.\n' + + ' template:\n' + + ' - index\n' + + '\n' + + 'taxonomies:\n' + + ' tag: /tag/{slug}/\n' + + ' author: /author/{slug}/' + '' + + '\n' + ); + + return ensureSettings(['routes']).then(() => { + fs.readFile.callCount.should.be.eql(1); + fs.copy.called.should.be.false(); + }); + }); + + it('routes.yaml is old routes.yaml', function () { + const expectedDefaultSettingsPath = path.join(__dirname, '../../../../server/services/settings/default-routes.yaml'); + const expectedContentPath = path.join(__dirname, '../../../utils/fixtures/settings/routes.yaml'); + + fs.readFile.withArgs(path.join(__dirname, '../../../utils/fixtures/settings/routes.yaml'), 'utf8').resolves('' + + 'routes:\n' + + '\n' + + 'collections:\n' + + ' /:\n' + + ' permalink: \'{globals.permalinks}\' # special 1.0 compatibility setting. See the docs for details.\n' + + ' template:\n' + + ' - home\n' + + ' - index\n' + + '\n' + + 'taxonomies:\n' + + ' tag: /tag/{slug}/\n' + + ' author: /author/{slug}/' + + '\n' + ); + + fs.copy.withArgs(expectedDefaultSettingsPath, expectedContentPath).resolves(); + + return ensureSettings(['routes']).then(() => { + fs.readFile.callCount.should.be.eql(1); + fs.copy.called.should.be.true(); + }); + }); + + it('routes.yaml is old routes.yaml', function () { + const expectedDefaultSettingsPath = path.join(__dirname, '../../../../server/services/settings/default-routes.yaml'); + const expectedContentPath = path.join(__dirname, '../../../utils/fixtures/settings/routes.yaml'); + + fs.readFile.withArgs(path.join(__dirname, '../../../utils/fixtures/settings/routes.yaml'), 'utf8').resolves('' + + 'routes:\n' + + '\n\n' + + 'collections:\n' + + ' /:\n' + + ' permalink: \'{globals.permalinks}\' # special 1.0 compatibility setting. See the docs for details.\n' + + ' template:\n' + + ' - home\n' + + ' - index\n' + + '\n\r' + + 'taxonomies: \n' + + ' tag: /tag/{slug}/\n' + + ' author: /author/{slug}/' + + '\t' + + '\n' + ); + + fs.copy.withArgs(expectedDefaultSettingsPath, expectedContentPath).resolves(); + + return ensureSettings(['routes']).then(() => { + fs.readFile.callCount.should.be.eql(1); + fs.copy.called.should.be.true(); + }); + }); + + it('routes.yaml has modifications, do not replace', function () { + fs.readFile.withArgs(path.join(__dirname, '../../../utils/fixtures/settings/routes.yaml'), 'utf8').resolves('' + + 'routes:\n' + + ' /about/: about' + + '\n' + + 'collections:\n' + + ' /:\n' + + ' permalink: \'{globals.permalinks}\' # special 1.0 compatibility setting. See the docs for details.\n' + + '\n' + + 'taxonomies:\n' + + ' tag: /categories/{slug}/\n' + + ' author: /author/{slug}/' + '' + + '\n' + ); + + return ensureSettings(['routes']).then(() => { + fs.readFile.callCount.should.be.eql(1); + fs.copy.called.should.be.false(); }); }); }); diff --git a/core/test/utils/fixtures/settings/badroutes.yaml b/core/test/utils/fixtures/settings/badroutes.yaml index 76b549099d..cad6451091 100644 --- a/core/test/utils/fixtures/settings/badroutes.yaml +++ b/core/test/utils/fixtures/settings/badroutes.yaml @@ -4,7 +4,6 @@ collections: / permalink: '{globals.permalinks}' template: - - home - index taxonomies: diff --git a/core/test/utils/fixtures/settings/goodroutes.yaml b/core/test/utils/fixtures/settings/goodroutes.yaml index b11f52a613..e497c6eb1a 100644 --- a/core/test/utils/fixtures/settings/goodroutes.yaml +++ b/core/test/utils/fixtures/settings/goodroutes.yaml @@ -4,7 +4,6 @@ collections: /: permalink: '{globals.permalinks}' template: - - home - index taxonomies: diff --git a/core/test/utils/fixtures/settings/routes.yaml b/core/test/utils/fixtures/settings/routes.yaml index b11f52a613..e497c6eb1a 100644 --- a/core/test/utils/fixtures/settings/routes.yaml +++ b/core/test/utils/fixtures/settings/routes.yaml @@ -4,7 +4,6 @@ collections: /: permalink: '{globals.permalinks}' template: - - home - index taxonomies: