From 5d977f23d40dc2fe32f7886314348f06ad260906 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Thu, 3 Jan 2019 15:23:22 +0000 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Added=20Settings=20endpoint=20to=20?= =?UTF-8?q?V2=20Content=20API?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs #10318 - This settings endpoint returns the commonly used, public information from our settings. - The values are whitelisted each with a custom name for returning from the endpoint --- core/server/api/v2/index.js | 4 + core/server/api/v2/settings-public.js | 12 ++ .../v2/utils/serializers/output/settings.js | 9 +- core/server/services/settings/cache.js | 110 +++++++++++------- core/server/services/settings/public.js | 22 ++++ core/server/web/api/v2/content/routes.js | 3 + .../api/v2/content/settings_spec.js | 60 ++++++++++ .../test/unit/services/settings/cache_spec.js | 36 +++++- 8 files changed, 207 insertions(+), 49 deletions(-) create mode 100644 core/server/api/v2/settings-public.js create mode 100644 core/server/services/settings/public.js create mode 100644 core/test/functional/api/v2/content/settings_spec.js diff --git a/core/server/api/v2/index.js b/core/server/api/v2/index.js index 54c870abb3..b776407d6d 100644 --- a/core/server/api/v2/index.js +++ b/core/server/api/v2/index.js @@ -93,5 +93,9 @@ module.exports = { get configuration() { return shared.pipeline(require('./configuration'), localUtils); + }, + + get publicSettings() { + return shared.pipeline(require('./settings-public'), localUtils); } }; diff --git a/core/server/api/v2/settings-public.js b/core/server/api/v2/settings-public.js new file mode 100644 index 0000000000..1ee9c5087d --- /dev/null +++ b/core/server/api/v2/settings-public.js @@ -0,0 +1,12 @@ +const settingsCache = require('../../services/settings/cache'); + +module.exports = { + docName: 'settings', + + browse: { + permissions: true, + query() { + return settingsCache.getPublic(); + } + } +}; diff --git a/core/server/api/v2/utils/serializers/output/settings.js b/core/server/api/v2/utils/serializers/output/settings.js index e932abd951..e9e97914b3 100644 --- a/core/server/api/v2/utils/serializers/output/settings.js +++ b/core/server/api/v2/utils/serializers/output/settings.js @@ -1,4 +1,5 @@ const _ = require('lodash'); +const utils = require('../../index'); const _private = {}; const deprecatedSettings = ['force_i18n', 'permalinks']; @@ -23,7 +24,13 @@ _private.settingsFilter = (settings, filter) => { module.exports = { browse(models, apiConfig, frame) { - let filteredSettings = _.values(_private.settingsFilter(models, frame.options.type)); + let filteredSettings; + // If this is public, we already have the right data, we just need to add an Array wrapper + if (utils.isContentAPI(frame)) { + filteredSettings = models; + } else { + filteredSettings = _.values(_private.settingsFilter(models, frame.options.type)); + } frame.response = { settings: filteredSettings, diff --git a/core/server/services/settings/cache.js b/core/server/services/settings/cache.js index d666db581e..c2980cdbe8 100644 --- a/core/server/services/settings/cache.js +++ b/core/server/services/settings/cache.js @@ -1,26 +1,50 @@ // It's important to keep the requires absolutely minimal here, // As this cache is used in SO many other areas, we may open ourselves to // circular dependency bugs. -var debug = require('ghost-ignition').debug('settings:cache'), - _ = require('lodash'), - common = require('../../lib/common'), - /** - * ## Cache - * Holds cached settings - * Keyed by setting.key - * Contains the JSON version of the model - * @type {{}} - object of objects - */ - settingsCache = {}, - _private = {}; +const debug = require('ghost-ignition').debug('settings:cache'); +const _ = require('lodash'); +const common = require('../../lib/common'); +const publicSettings = require('./public'); // Local function, only ever used for initialising // We deliberately call "set" on each model so that set is a consistent interface -_private.updateSettingFromModel = function updateSettingFromModel(settingModel) { +const updateSettingFromModel = function updateSettingFromModel(settingModel) { debug('Auto updating', settingModel.get('key')); module.exports.set(settingModel.get('key'), settingModel.toJSON()); }; +/** + * ## Cache + * Holds cached settings + * Keyed by setting.key + * Contains the JSON version of the model + * @type {{}} - object of objects + */ +let settingsCache = {}; + +const doGet = (key, options) => { + if (!settingsCache[key]) { + return; + } + + // Don't try to resolve to the value of the setting + if (options && options.resolve === false) { + return settingsCache[key]; + } + + // Default behaviour is to try to resolve the value and return that + try { + // CASE: if a string contains a number e.g. "1", JSON.parse will auto convert into integer + if (!isNaN(Number(settingsCache[key].value))) { + return settingsCache[key].value; + } + + return JSON.parse(settingsCache[key].value); + } catch (err) { + return settingsCache[key].value; + } +}; + /** * * IMPORTANT: @@ -45,27 +69,8 @@ module.exports = { * @param {object} options * @return {*} */ - get: function get(key, options) { - if (!settingsCache[key]) { - return; - } - - // Don't try to resolve to the value of the setting - if (options && options.resolve === false) { - return settingsCache[key]; - } - - // Default behaviour is to try to resolve the value and return that - try { - // CASE: if a string contains a number e.g. "1", JSON.parse will auto convert into integer - if (!isNaN(Number(settingsCache[key].value))) { - return settingsCache[key].value; - } - - return JSON.parse(settingsCache[key].value); - } catch (err) { - return settingsCache[key].value; - } + get(key, options) { + return doGet(key, options); }, /** * Set a key on the cache @@ -74,7 +79,7 @@ module.exports = { * @param {string} key * @param {object} value json version of settings model */ - set: function set(key, value) { + set(key, value) { settingsCache[key] = _.cloneDeep(value); }, /** @@ -82,9 +87,24 @@ module.exports = { * Uses clone to prevent modifications from being reflected * @return {{}} cache */ - getAll: function getAll() { + getAll() { return _.cloneDeep(settingsCache); }, + + /** + * Get all the publically accessible cache entries with their correct names + * Uses clone to prevent modifications from being reflected + * @return {{}} cache + */ + getPublic() { + let settings = {}; + + _.each(publicSettings, (newKey, key) => { + settings[newKey] = doGet(key) || ''; + }); + + return settings; + }, /** * Initialise the cache * @@ -93,27 +113,27 @@ module.exports = { * @param {Bookshelf.Collection} [settingsCollection] * @return {{}} */ - init: function init(settingsCollection) { + init(settingsCollection) { // First, reset the cache settingsCache = {}; // // if we have been passed a collection of settings, use this to populate the cache if (settingsCollection && settingsCollection.models) { - _.each(settingsCollection.models, _private.updateSettingFromModel); + _.each(settingsCollection.models, updateSettingFromModel); } // Bind to events to automatically keep up-to-date - common.events.on('settings.edited', _private.updateSettingFromModel); - common.events.on('settings.added', _private.updateSettingFromModel); - common.events.on('settings.deleted', _private.updateSettingFromModel); + common.events.on('settings.edited', updateSettingFromModel); + common.events.on('settings.added', updateSettingFromModel); + common.events.on('settings.deleted', updateSettingFromModel); return settingsCache; }, - shutdown: function () { - common.events.removeListener('settings.edited', _private.updateSettingFromModel); - common.events.removeListener('settings.added', _private.updateSettingFromModel); - common.events.removeListener('settings.deleted', _private.updateSettingFromModel); + shutdown() { + common.events.removeListener('settings.edited', updateSettingFromModel); + common.events.removeListener('settings.added', updateSettingFromModel); + common.events.removeListener('settings.deleted', updateSettingFromModel); }, reset() { diff --git a/core/server/services/settings/public.js b/core/server/services/settings/public.js new file mode 100644 index 0000000000..34d8042266 --- /dev/null +++ b/core/server/services/settings/public.js @@ -0,0 +1,22 @@ +/** + * The settings with type "blog" were originally meant to be public + * This has been misused - unsplash and slack are incorrectly stored there + * https://github.com/TryGhost/Ghost/issues/10318 + * + * This file acts as a new whitelist for "public" settings + */ + +module.exports = { + title: 'title', + description: 'description', + logo: 'logo', + icon: 'icon', + cover_image: 'cover_image', + facebook: 'facebook', + twitter: 'twitter', + default_locale: 'lang', + active_timezone: 'timezone', + ghost_head: 'ghost_head', + ghost_foot: 'ghost_foot', + navigation: 'navigation' +}; diff --git a/core/server/web/api/v2/content/routes.js b/core/server/web/api/v2/content/routes.js index 8001b66801..08989def4f 100644 --- a/core/server/web/api/v2/content/routes.js +++ b/core/server/web/api/v2/content/routes.js @@ -28,5 +28,8 @@ module.exports = function apiRoutes() { router.get('/tags/:id', mw.authenticatePublic, apiv2.http(apiv2.tagsPublic.read)); router.get('/tags/slug/:slug', mw.authenticatePublic, apiv2.http(apiv2.tagsPublic.read)); + // ## Settings + router.get('/settings', mw.authenticatePublic, apiv2.http(apiv2.publicSettings.browse)); + return router; }; diff --git a/core/test/functional/api/v2/content/settings_spec.js b/core/test/functional/api/v2/content/settings_spec.js new file mode 100644 index 0000000000..88ba1f239d --- /dev/null +++ b/core/test/functional/api/v2/content/settings_spec.js @@ -0,0 +1,60 @@ +const should = require('should'); +const supertest = require('supertest'); +const _ = require('lodash'); +const testUtils = require('../../../../utils'); +const localUtils = require('./utils'); +const config = require('../../../../../server/config'); + +// Values to test against +const publicSettings = require('../../../../../server/services/settings/public'); +const defaultSettings = require('../../../../../server/data/schema').defaultSettings.blog; + +const ghost = testUtils.startGhost; +let request; + +describe('Settings', function () { + before(function () { + return ghost() + .then(function () { + request = supertest.agent(config.get('url')); + }).then(function () { + return testUtils.initFixtures('api_keys'); + }); + }); + + it('browse settings', function () { + const key = localUtils.getValidKey(); + return request.get(localUtils.API.getApiQuery(`settings/?key=${key}`)) + .set('Origin', testUtils.API.getURL()) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(200) + .then((res) => { + res.headers.vary.should.eql('Accept-Encoding'); + should.exist(res.headers['access-control-allow-origin']); + should.not.exist(res.headers['x-cache-invalidate']); + + const jsonResponse = res.body; + should.exist(jsonResponse.settings); + should.exist(jsonResponse.meta); + + jsonResponse.settings.should.be.an.Object(); + const settings = jsonResponse.settings; + + // Verify we have the right keys for settings + settings.should.have.properties(_.values(publicSettings)); + + // Verify that we are returning the defaults for each value + _.forEach(settings, (value, key) => { + let defaultKey = _.findKey(publicSettings, (v) => v === key); + let defaultValue = _.find(defaultSettings, (setting) => setting.key === defaultKey).defaultValue; + + if (defaultKey === 'navigation') { + defaultValue = JSON.parse(defaultValue); + } + + value.should.eql(defaultValue); + }); + }); + }); +}); diff --git a/core/test/unit/services/settings/cache_spec.js b/core/test/unit/services/settings/cache_spec.js index 6eb7f8fb95..30def94f08 100644 --- a/core/test/unit/services/settings/cache_spec.js +++ b/core/test/unit/services/settings/cache_spec.js @@ -1,10 +1,16 @@ -var rewire = require('rewire'), - should = require('should'), - cache = rewire('../../../../server/services/settings/cache'); +const rewire = require('rewire'); +const should = require('should'); +const _ = require('lodash'); +const publicSettings = require('../../../../server/services/settings/public'); +let cache = rewire('../../../../server/services/settings/cache'); should.equal(true, true); describe('UNIT: settings cache', function () { + beforeEach(function () { + cache = rewire('../../../../server/services/settings/cache'); + }); + it('does not auto convert string into number', function () { cache.set('key1', {value: '1'}); (typeof cache.get('key1')).should.eql('string'); @@ -23,4 +29,28 @@ describe('UNIT: settings cache', function () { cache.get('key2').c.should.eql({d: []}); cache.get('key2').e.should.eql(2); }); + + it('can get all values', function () { + cache.set('key1', {value: '1'}); + cache.get('key1').should.eql('1'); + cache.getAll().should.eql({key1: {value: '1'}}); + }); + + it('correctly filters and formats public values', function () { + cache.set('key1', {value: 'something'}); + cache.set('title', {value: 'hello world'}); + cache.set('active_timezone', {value: 'PST'}); + + cache.getAll().should.eql({ + key1: {value: 'something'}, + title: {value: 'hello world'}, + active_timezone: {value: 'PST'} + }); + + let values = _.zipObject(_.values(publicSettings), _.fill(Array(_.size(publicSettings)), '')); + values.title = 'hello world'; + values.timezone = 'PST'; + + cache.getPublic().should.eql(values); + }); });