TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2024-11-27 10:42:45 +03:00
Code Issues Projects Releases Wiki Activity

Fixed __GHOST_URL__ appearing in settings values after edit (#12738)

Browse Source 
refs https://github.com/TryGhost/Ghost/pull/12736
refs https://github.com/TryGhost/Team/issues/467

knex's `parse()` method is only called on data when directly fetched from the db. This was causing problems when model instances are passed around via events for example because `.get('key')` will return data that was directly set on the model without having gone through the `parse()` transformations. The result of this inconsistency was settings appearing correct when Ghost started up but then being broken as soon as a setting was changed.

- moved absolute/relative->transform-ready URL transformations from the API input serializers to the model's `format()` method and replaced with a relative->absolute transform in API input serializers
    - results in consistency because `.get()` on a settings model will always return an URL
- removed transform-ready->absolute transforms from the API output serializers as that is now handled at the model-layer
This commit is contained in:
Kevin Ansfield 2021-03-08 18:41:43 +00:00 committed by GitHub
parent c83fb3fc5c
commit 021cfecb59
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 95 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
core/server/api/canary/utils/serializers/input/settings.js
View File

@ -159,9 +159,7 @@ module.exports = {
setting.key = 'lang';
}
if (['cover_image', 'icon', 'logo', 'portal_button_icon'].includes(setting.key)) {
setting = url.forSetting(setting);
}
setting = url.forSetting(setting);
});
// Ignore all deprecated settings

4
core/server/api/canary/utils/serializers/input/utils/url.js
View File

@ -58,8 +58,8 @@ const forTag = (attrs) => {
};
const forSetting = (attrs) => {
if (attrs.value) {
attrs.value = handleImageUrl(attrs.value);
if (attrs.value && ['cover_image', 'logo', 'icon', 'portal_button_icon', 'og_image', 'twitter_image'].includes(attrs.key)) {
attrs.value = urlUtils.relativeToAbsolute(attrs.value);
}
return attrs;

16
core/server/api/canary/utils/serializers/output/utils/url.js
View File

@ -73,22 +73,6 @@ const forTag = (id, attrs, options) => {
};
const forSettings = (attrs) => {
// @TODO: https://github.com/TryGhost/Ghost/issues/10106
// @NOTE: Admin & Content API return a different format, need to mappers
if (_.isArray(attrs)) {
attrs.forEach((obj) => {
if (['cover_image', 'logo', 'icon', 'portal_button_icon'].includes(obj.key) && obj.value) {
obj.value = urlUtils.transformReadyToAbsolute(obj.value);
}
});
} else {
['cover_image', 'logo', 'icon', 'portal_button_icon'].forEach((attr) => {
if (attrs[attr]) {
attrs[attr] = urlUtils.transformReadyToAbsolute(attrs[attr]);
}
});
}
return attrs;
};

4
core/server/api/v2/utils/serializers/input/settings.js
View File

@ -139,9 +139,7 @@ module.exports = {
setting.value = JSON.parse(setting.value).isActive;
}
if (['cover_image', 'icon', 'logo'].includes(setting.key)) {
setting = url.forSetting(setting);
}
setting = url.forSetting(setting);
});
// Ignore all deprecated settings

4
core/server/api/v2/utils/serializers/input/utils/url.js
View File

@ -58,8 +58,8 @@ const forTag = (attrs) => {
};
const forSetting = (attrs) => {
if (attrs.value) {
attrs.value = handleImageUrl(attrs.value);
if (attrs.value && ['cover_image', 'logo', 'icon', 'portal_button_icon', 'og_image', 'twitter_image'].includes(attrs.key)) {
attrs.value = urlUtils.relativeToAbsolute(attrs.value);
}
return attrs;

16
core/server/api/v2/utils/serializers/output/utils/url.js
View File

@ -81,22 +81,6 @@ const forTag = (id, attrs, options) => {
};
const forSettings = (attrs) => {
// @TODO: https://github.com/TryGhost/Ghost/issues/10106
// @NOTE: Admin & Content API return a different format, need to mappers
if (_.isArray(attrs)) {
attrs.forEach((obj) => {
if (['cover_image', 'logo', 'icon'].includes(obj.key) && obj.value) {
obj.value = urlUtils.transformReadyToAbsolute(obj.value);
}
});
} else {
['cover_image', 'logo', 'icon'].forEach((attr) => {
if (attrs[attr]) {
attrs[attr] = urlUtils.transformReadyToAbsolute(attrs[attr]);
}
});
}
return attrs;
};

4
core/server/api/v3/utils/serializers/input/settings.js
View File

@ -155,9 +155,7 @@ module.exports = {
setting.value = JSON.parse(setting.value).isActive;
}
if (['cover_image', 'icon', 'logo', 'portal_button_icon'].includes(setting.key)) {
setting = url.forSetting(setting);
}
setting = url.forSetting(setting);
});
// Ignore all deprecated settings

4
core/server/api/v3/utils/serializers/input/utils/url.js
View File

@ -58,8 +58,8 @@ const forTag = (attrs) => {
};
const forSetting = (attrs) => {
if (attrs.value) {
attrs.value = handleImageUrl(attrs.value);
if (attrs.value && ['cover_image', 'logo', 'icon', 'portal_button_icon', 'og_image', 'twitter_image'].includes(attrs.key)) {
attrs.value = urlUtils.relativeToAbsolute(attrs.value);
}
return attrs;

16
core/server/api/v3/utils/serializers/output/utils/url.js
View File

@ -73,22 +73,6 @@ const forTag = (id, attrs, options) => {
};
const forSettings = (attrs) => {
// @TODO: https://github.com/TryGhost/Ghost/issues/10106
// @NOTE: Admin & Content API return a different format, need to mappers
if (_.isArray(attrs)) {
attrs.forEach((obj) => {
if (['cover_image', 'logo', 'icon', 'portal_button_icon'].includes(obj.key) && obj.value) {
obj.value = urlUtils.transformReadyToAbsolute(obj.value);
}
});
} else {
['cover_image', 'logo', 'icon', 'portal_button_icon'].forEach((attr) => {
if (attrs[attr]) {
attrs[attr] = urlUtils.transformReadyToAbsolute(attrs[attr]);
}
});
}
return attrs;
};

5
core/server/models/settings.js
View File

@ -145,6 +145,11 @@ Settings = ghostBookshelf.Model.extend({
attrs.value = attrs.value.toString();
}
}
if (attrs.value && ['cover_image', 'logo', 'icon', 'portal_button_icon', 'og_image', 'twitter_image'].includes(attrs.key)) {
attrs.value = urlUtils.toTransformReady(attrs.value);
}
return attrs;
},

4
test/api-acceptance/admin/settings_spec.js
View File

@ -199,7 +199,7 @@ describe('Settings API', function () {
should.equal(putBody.settings[6].value, 'SEO description');
putBody.settings[7].key.should.eql('og_image');
should.equal(putBody.settings[7].value, '/content/images/2019/07/facebook.png');
should.equal(putBody.settings[7].value, `${config.get('url')}/content/images/2019/07/facebook.png`);
putBody.settings[8].key.should.eql('og_title');
should.equal(putBody.settings[8].value, 'facebook title');
@ -208,7 +208,7 @@ describe('Settings API', function () {
should.equal(putBody.settings[9].value, 'facebook description');
putBody.settings[10].key.should.eql('twitter_image');
should.equal(putBody.settings[10].value, '/content/images/2019/07/twitter.png');
should.equal(putBody.settings[10].value, `${config.get('url')}/content/images/2019/07/twitter.png`);
putBody.settings[11].key.should.eql('twitter_title');
should.equal(putBody.settings[11].value, 'twitter title');

49
test/regression/api/canary/admin/settings_spec.js
View File

@ -4,6 +4,7 @@ const supertest = require('supertest');
const config = require('../../../../../core/shared/config');
const testUtils = require('../../../../utils');
const localUtils = require('./utils');
const db = require('../../../../../core/server/data/db');
const ghost = testUtils.startGhost;
// NOTE: in future iterations these fields should be fetched from a central module.
@ -756,6 +757,54 @@ describe('Settings API (canary)', function () {
putBody.settings[0].key.should.eql('slack_username');
putBody.settings[0].value.should.eql('can edit me');
});
it('Can edit URLs without internal storage format leaking', async function () {
const settingsToChange = {
settings: [
{key: 'cover_image', value: `${config.get('url')}/content/images/cover_image.png`},
{key: 'logo', value: `${config.get('url')}/content/images/logo.png`},
{key: 'icon', value: `${config.get('url')}/content/images/icon.png`},
{key: 'portal_button_icon', value: `${config.get('url')}/content/images/portal_button_icon.png`},
{key: 'og_image', value: `${config.get('url')}/content/images/og_image.png`},
{key: 'twitter_image', value: `${config.get('url')}/content/images/twitter_image.png`}
]
};
const {body} = await request.put(localUtils.API.getApiQuery('settings/'))
.set('Origin', config.get('url'))
.send(settingsToChange)
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(200);
const responseSettings = body.settings.reduce((acc, setting) => {
acc[setting.key] = setting.value;
return acc;
}, {});
responseSettings.should.have.property('cover_image', `${config.get('url')}/content/images/cover_image.png`);
responseSettings.should.have.property('logo', `${config.get('url')}/content/images/logo.png`);
responseSettings.should.have.property('icon', `${config.get('url')}/content/images/icon.png`);
responseSettings.should.have.property('portal_button_icon', `${config.get('url')}/content/images/portal_button_icon.png`);
responseSettings.should.have.property('og_image', `${config.get('url')}/content/images/og_image.png`);
responseSettings.should.have.property('twitter_image', `${config.get('url')}/content/images/twitter_image.png`);
const dbSettingsRows = await db.knex('settings')
.select('key', 'value')
.whereIn('key', ['cover_image', 'logo', 'icon', 'portal_button_icon', 'og_image', 'twitter_image']);
const dbSettings = dbSettingsRows.reduce((acc, setting) => {
acc[setting.key] = setting.value;
return acc;
}, {});
dbSettings.should.have.property('cover_image', '__GHOST_URL__/content/images/cover_image.png');
dbSettings.should.have.property('logo', '__GHOST_URL__/content/images/logo.png');
dbSettings.should.have.property('icon', '__GHOST_URL__/content/images/icon.png');
dbSettings.should.have.property('portal_button_icon', '__GHOST_URL__/content/images/portal_button_icon.png');
dbSettings.should.have.property('og_image', '__GHOST_URL__/content/images/og_image.png');
dbSettings.should.have.property('twitter_image', '__GHOST_URL__/content/images/twitter_image.png');
});
});
describe('As Admin', function () {

36
test/unit/models/settings_spec.js
View File

@ -181,6 +181,30 @@ describe('Unit: models/settings', function () {
});
});
describe('format', function () {
it('transforms urls when persisting to db', function () {
const setting = models.Settings.forge();
let returns = setting.format({key: 'cover_image', value: 'http://127.0.0.1:2369/cover_image.png', type: 'string'});
should.equal(returns.value, '__GHOST_URL__/cover_image.png');
returns = setting.format({key: 'logo', value: 'http://127.0.0.1:2369/logo.png', type: 'string'});
should.equal(returns.value, '__GHOST_URL__/logo.png');
returns = setting.format({key: 'icon', value: 'http://127.0.0.1:2369/icon.png', type: 'string'});
should.equal(returns.value, '__GHOST_URL__/icon.png');
returns = setting.format({key: 'portal_button_icon', value: 'http://127.0.0.1:2369/portal_button_icon.png', type: 'string'});
should.equal(returns.value, '__GHOST_URL__/portal_button_icon.png');
returns = setting.format({key: 'og_image', value: 'http://127.0.0.1:2369/og_image.png', type: 'string'});
should.equal(returns.value, '__GHOST_URL__/og_image.png');
returns = setting.format({key: 'twitter_image', value: 'http://127.0.0.1:2369/twitter_image.png', type: 'string'});
should.equal(returns.value, '__GHOST_URL__/twitter_image.png');
});
});
describe('parse', function () {
it('ensure correct parsing when fetching from db', function () {
const setting = models.Settings.forge();
@ -206,22 +230,22 @@ describe('Unit: models/settings', function () {
returns = setting.parse({key: 'something', value: 'null'});
should.equal(returns.value, 'null');
returns = setting.parse({key: 'cover_image', value: '__GHOST_URL__/cover_image.png'});
returns = setting.parse({key: 'cover_image', value: '__GHOST_URL__/cover_image.png', type: 'string'});
should.equal(returns.value, 'http://127.0.0.1:2369/cover_image.png');
returns = setting.parse({key: 'logo', value: '__GHOST_URL__/logo.png'});
returns = setting.parse({key: 'logo', value: '__GHOST_URL__/logo.png', type: 'string'});
should.equal(returns.value, 'http://127.0.0.1:2369/logo.png');
returns = setting.parse({key: 'icon', value: '__GHOST_URL__/icon.png'});
returns = setting.parse({key: 'icon', value: '__GHOST_URL__/icon.png', type: 'string'});
should.equal(returns.value, 'http://127.0.0.1:2369/icon.png');
returns = setting.parse({key: 'portal_button_icon', value: '__GHOST_URL__/portal_button_icon.png'});
returns = setting.parse({key: 'portal_button_icon', value: '__GHOST_URL__/portal_button_icon.png', type: 'string'});
should.equal(returns.value, 'http://127.0.0.1:2369/portal_button_icon.png');
returns = setting.parse({key: 'og_image', value: '__GHOST_URL__/og_image.png'});
returns = setting.parse({key: 'og_image', value: '__GHOST_URL__/og_image.png', type: 'string'});
should.equal(returns.value, 'http://127.0.0.1:2369/og_image.png');
returns = setting.parse({key: 'twitter_image', value: '__GHOST_URL__/twitter_image.png'});
returns = setting.parse({key: 'twitter_image', value: '__GHOST_URL__/twitter_image.png', type: 'string'});
should.equal(returns.value, 'http://127.0.0.1:2369/twitter_image.png');
});
});