From a55fb0bafea2dbddd80bbc3459eec018d1f6a144 Mon Sep 17 00:00:00 2001 From: Katharina Irrgang Date: Fri, 28 Oct 2016 15:07:46 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=A8=20public=20config=20endpoint=20(#7?= =?UTF-8?q?631)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes #7628 With this PR we expose a public configuration endpoint. When /ghost is requested, we don't load and render the configurations into the template anymore. Instead, Ghost-Admin can request the public configuration endpoint. * 🎨 make configuration endpoint public * 🔥 remove loading configurations in admin app - do not render them into the default html page * ✨ load client credentials in configuration endpoint - this is not a security issue, because we have exposed this information anyway before (by rendering them into the requested html page) * 🎨 extend existing configuration integration test * ✨ tests: add ghost-auth to data generator * ✨ add functional test * 🔥 remove type/value pattern * 🎨 do not return stringified JSON objects --- core/server/admin/controller.js | 33 +------------ core/server/api/app.js | 2 +- core/server/api/configuration.js | 46 +++++++++++------ .../routes/api/configuration_spec.js | 41 ++++++++++++++++ .../integration/api/api_configuration_spec.js | 49 +++++++++++++------ core/test/utils/fixtures/data-generator.js | 3 +- 6 files changed, 111 insertions(+), 63 deletions(-) create mode 100644 core/test/functional/routes/api/configuration_spec.js diff --git a/core/server/admin/controller.js b/core/server/admin/controller.js index 3252c64835..57d18ffb0e 100644 --- a/core/server/admin/controller.js +++ b/core/server/admin/controller.js @@ -1,8 +1,6 @@ var debug = require('debug')('ghost:admin:controller'), _ = require('lodash'), - Promise = require('bluebird'), api = require('../api'), - config = require('../config'), logging = require('../logging'), updateCheck = require('../update-check'), i18n = require('../i18n'); @@ -14,35 +12,6 @@ module.exports = function adminController(req, res) { /*jslint unparam:true*/ debug('index called'); - function renderIndex() { - var configuration, - fetch = { - configuration: api.configuration.read().then(function (res) { return res.configuration[0]; }), - client: api.clients.read({slug: 'ghost-admin'}).then(function (res) { return res.clients[0]; }), - ghostAuth: api.clients.read({slug: 'ghost-auth'}) - .then(function (res) { return res.clients[0]; }) - .catch(function () { - return; - }) - }; - - return Promise.props(fetch).then(function renderIndex(result) { - configuration = result.configuration; - - configuration.clientId = {value: result.client.slug, type: 'string'}; - configuration.clientSecret = {value: result.client.secret, type: 'string'}; - - if (result.ghostAuth && config.get('auth:type') === 'ghost') { - configuration.ghostAuthId = {value: result.ghostAuth.uuid, type: 'string'}; - } - - debug('rendering default template'); - res.render('default', { - configuration: configuration - }); - }); - } - updateCheck().then(function then() { return updateCheck.showUpdateNotification(); }).then(function then(updateVersion) { @@ -64,6 +33,6 @@ module.exports = function adminController(req, res) { } }); }).finally(function noMatterWhat() { - renderIndex(); + res.render('default'); }).catch(logging.logError); }; diff --git a/core/server/api/app.js b/core/server/api/app.js index 5b0cd4a06a..6468dfbef2 100644 --- a/core/server/api/app.js +++ b/core/server/api/app.js @@ -59,7 +59,7 @@ function apiRoutes() { apiRouter.options('*', cors); // ## Configuration - apiRouter.get('/configuration', authenticatePrivate, api.http(api.configuration.read)); + apiRouter.get('/configuration', api.http(api.configuration.read)); apiRouter.get('/configuration/:key', authenticatePrivate, api.http(api.configuration.read)); apiRouter.get('/configuration/timezones', authenticatePrivate, api.http(api.configuration.read)); diff --git a/core/server/api/configuration.js b/core/server/api/configuration.js index b841ea795f..c385032a7d 100644 --- a/core/server/api/configuration.js +++ b/core/server/api/configuration.js @@ -3,17 +3,11 @@ var _ = require('lodash'), config = require('../config'), ghostVersion = require('../utils/ghost-version'), + models = require('../models'), Promise = require('bluebird'), configuration; -function labsFlag(key) { - return { - value: (config[key] === true), - type: 'bool' - }; -} - function fetchAvailableTimezones() { var timezones = require('../data/timezones.json'); return timezones; @@ -30,18 +24,22 @@ function getAboutConfig() { function getBaseConfig() { return { - fileStorage: {value: (config.fileStorage !== false), type: 'bool'}, - useGravatar: {value: !config.isPrivacyDisabled('useGravatar'), type: 'bool'}, - publicAPI: labsFlag('publicAPI'), - blogUrl: {value: config.get('url').replace(/\/$/, ''), type: 'string'}, - blogTitle: {value: config.get('theme').title, type: 'string'}, - routeKeywords: {value: JSON.stringify(config.get('routeKeywords')), type: 'json'} + fileStorage: config.get('fileStorage') !== false, + useGravatar: !config.isPrivacyDisabled('useGravatar'), + publicAPI: config.get('publicAPI') === true, + blogUrl: config.get('url').replace(/\/$/, ''), + blogTitle: config.get('theme').title, + routeKeywords: config.get('routeKeywords') }; } /** * ## Configuration API Methods * + * We need to load the client credentials dynamically. + * For example: on bootstrap ghost-auth get's created and if we load them here in parallel, + * it can happen that we won't get any client credentials or wrong credentials. + * * **See:** [API Methods](index.js.html#api%20methods) */ configuration = { @@ -54,9 +52,29 @@ configuration = { */ read: function read(options) { options = options || {}; + var ops = {}; if (!options.key) { - return Promise.resolve({configuration: [getBaseConfig()]}); + ops.ghostAdmin = models.Client.findOne({slug: 'ghost-admin'}); + + if (config.get('auth:type') === 'ghost') { + ops.ghostAuth = models.Client.findOne({slug: 'ghost-auth'}); + } + + return Promise.props(ops) + .then(function (result) { + var configuration = getBaseConfig(); + + configuration.clientId = result.ghostAdmin.get('slug'); + configuration.clientSecret = result.ghostAdmin.get('secret'); + + if (result.ghostAuth) { + configuration.ghostAuthId = result.ghostAuth.get('uuid'); + configuration.ghostAuthUrl = config.get('auth:url'); + } + + return {configuration: [configuration]}; + }); } if (options.key === 'about') { diff --git a/core/test/functional/routes/api/configuration_spec.js b/core/test/functional/routes/api/configuration_spec.js new file mode 100644 index 0000000000..4ad557c33b --- /dev/null +++ b/core/test/functional/routes/api/configuration_spec.js @@ -0,0 +1,41 @@ +var testUtils = require('../../../utils'), + should = require('should'), + supertest = require('supertest'), + ghost = testUtils.startGhost, + request; + +describe('Configuration API', function () { + var accesstoken = ''; + + before(function (done) { + // starting ghost automatically populates the db + // TODO: prevent db init, and manage bringing up the DB with fixtures ourselves + ghost().then(function (ghostServer) { + request = supertest.agent(ghostServer.rootApp); + }).then(function () { + return testUtils.doAuth(request, 'posts'); + }).then(function (token) { + accesstoken = token; + done(); + }).catch(done); + }); + + after(function (done) { + testUtils.clearData().then(function () { + done(); + }).catch(done); + }); + + describe('success', function () { + it('can retrieve public configuration', function (done) { + request.get(testUtils.API.getApiQuery('configuration/')) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(200) + .end(function (err, res) { + should.exist(res.body.configuration); + done(); + }); + }); + }); +}); diff --git a/core/test/integration/api/api_configuration_spec.js b/core/test/integration/api/api_configuration_spec.js index 95cedd8494..0e6490bbb8 100644 --- a/core/test/integration/api/api_configuration_spec.js +++ b/core/test/integration/api/api_configuration_spec.js @@ -1,18 +1,25 @@ -var testUtils = require('../../utils'), - should = require('should'), - rewire = require('rewire'), +var testUtils = require('../../utils'), + configUtils = require('../../utils/configUtils'), + should = require('should'), + rewire = require('rewire'), // Stuff we are testing - ConfigurationAPI = rewire('../../../server/api/configuration'); + ConfigurationAPI = rewire('../../../server/api/configuration'); describe('Configuration API', function () { // Keep the DB clean before(testUtils.teardown); - afterEach(testUtils.teardown); + beforeEach(testUtils.setup('clients')); + afterEach(function () { + configUtils.restore(); + return testUtils.teardown(); + }); should.exist(ConfigurationAPI); it('can read basic config and get all expected properties', function (done) { + configUtils.set('auth:type', 'ghost'); + ConfigurationAPI.read().then(function (response) { var props; @@ -21,17 +28,29 @@ describe('Configuration API', function () { response.configuration.should.be.an.Array().with.lengthOf(1); props = response.configuration[0]; - // Check the structure - props.should.have.property('blogUrl').which.is.an.Object().with.properties('type', 'value'); - props.should.have.property('blogTitle').which.is.an.Object().with.properties('type', 'value'); - props.should.have.property('routeKeywords').which.is.an.Object().with.properties('type', 'value'); - props.should.have.property('fileStorage').which.is.an.Object().with.properties('type', 'value'); - props.should.have.property('useGravatar').which.is.an.Object().with.properties('type', 'value'); - props.should.have.property('publicAPI').which.is.an.Object().with.properties('type', 'value'); + props.blogUrl.should.eql('http://127.0.0.1:2369'); + props.routeKeywords.should.eql({ + tag: 'tag', + author: 'author', + page: 'page', + preview: 'p', + private: 'private', + subscribe: 'subscribe', + amp: 'amp' + }); - // Check a few values - props.blogUrl.should.have.property('value', 'http://127.0.0.1:2369'); - props.fileStorage.should.have.property('value', true); + props.fileStorage.should.eql(true); + props.useGravatar.should.eql(true); + props.publicAPI.should.eql(false); + props.clientId.should.eql('ghost-admin'); + props.clientSecret.should.eql('not_available'); + props.ghostAuthUrl.should.eql('http://devauth.ghost.org:8080'); + + // value not available, because settings API was not called yet + props.hasOwnProperty('blogTitle').should.eql(true); + + // uuid + props.hasOwnProperty('ghostAuthId').should.eql(true); done(); }).catch(done); diff --git a/core/test/utils/fixtures/data-generator.js b/core/test/utils/fixtures/data-generator.js index a5ab2b48f9..7eb4f6742c 100644 --- a/core/test/utils/fixtures/data-generator.js +++ b/core/test/utils/fixtures/data-generator.js @@ -442,7 +442,8 @@ DataGenerator.forKnex = (function () { clients = [ createClient({name: 'Ghost Admin', slug: 'ghost-admin', type: 'ua'}), - createClient({name: 'Ghost Scheduler', slug: 'ghost-scheduler', type: 'web'}) + createClient({name: 'Ghost Scheduler', slug: 'ghost-scheduler', type: 'web'}), + createClient({name: 'Ghost Auth', slug: 'ghost-auth', type: 'web'}) ]; roles_users = [