Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-03 23:00:14 -05:00
Code Issues Activity

Renamed lang and session_secret default settings (#14791)

Browse source 
refs: https://github.com/TryGhost/Toolbox/issues/327

- lang / locale has had a lot of churn, but we decided this setting should always be locale
- session_secret is too generic as we have multiples of these
This commit is contained in:
Hannah Wolfe 2022-05-12 15:07:05 +01:00 • committed by GitHub
parent 1fabd76391
commit de118b0b04
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 112 additions and 77 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
core/bridge.js
View file

@ -22,7 +22,7 @@ const settingsCache = require('./shared/settings-cache');
const urlService = require('./server/services/url');
const routeSettings = require('./server/services/route-settings');
// Listen to settings.lang.edited, similar to the member service and models/base/listeners
// Listen to settings.locale.edited, similar to the member service and models/base/listeners
const events = require('./server/lib/common/events');
const messages = {
@ -33,9 +33,8 @@ class Bridge {
init() {
/**
* When locale changes, we reload theme translations
* @deprecated: the term "lang" was deprecated in favor of "locale" publicly in 4.0
*/
events.on('settings.lang.edited', (model) => {
events.on('settings.locale.edited', (model) => {
debug('Active theme init18n');
this.getActiveTheme().initI18n({locale: model.get('value')});
});
@ -54,7 +53,7 @@ class Bridge {
async activateTheme(loadedTheme, checkedTheme) {
let settings = {
locale: settingsCache.get('lang')
locale: settingsCache.get('locale')
};
// no need to check the score, activation should be used in combination with validate.check
// Use the two theme objects to set the current active theme

4
core/frontend/helpers/price.js
View file

@ -58,8 +58,8 @@ module.exports = function price(planOrAmount, options) {
}
options = options || {};
options.hash = options.hash || {};
// NOTE: potentially breaking place once site.lang is removed in favor of site.locale
const {currency, numberFormat = 'short', currencyFormat = 'symbol', locale = _.get(options, 'data.site.lang', 'en')} = options.hash;
const {currency, numberFormat = 'short', currencyFormat = 'symbol', locale = _.get(options, 'data.site.locale', 'en')} = options.hash;
if (plan) {
return formatter({
amount: plan.amount && (plan.amount / 100),

4
core/server/api/shared/serializers/input/utils/settings-key-group-mapper.js
View file

@ -6,7 +6,7 @@ const keyGroupMapping = {
db_hash: 'core',
next_update_check: 'core',
notifications: 'core',
session_secret: 'core',
admin_session_secret: 'core',
theme_session_secret: 'core',
ghost_public_key: 'core',
ghost_private_key: 'core',
@ -16,7 +16,7 @@ const keyGroupMapping = {
cover_image: 'site',
icon: 'site',
accent_color: 'site',
lang: 'site',
locale: 'site',
timezone: 'site',
codeinjection_head: 'site',
codeinjection_foot: 'site',

4
core/server/api/shared/serializers/input/utils/settings-key-type-mapper.js
View file

@ -1,7 +1,7 @@
// NOTE: mapping is based on maping present in migration - 3.22/07-update-type-for-settings
const keyTypeMapping = {
db_hash: 'string',
session_secret: 'string',
admin_session_secret: 'string',
theme_session_secret: 'string',
ghost_public_key: 'string',
ghost_private_key: 'string',
@ -11,7 +11,7 @@ const keyTypeMapping = {
cover_image: 'string',
icon: 'string',
accent_color: 'string',
lang: 'string',
locale: 'string',
timezone: 'string',
codeinjection_head: 'string',
codeinjection_foot: 'string',

12
core/server/data/importer/importers/data/settings.js
View file

@ -12,11 +12,13 @@ const {WRITABLE_KEYS_ALLOWLIST} = require('../../../../../shared/labs');
const labsDefaults = JSON.parse(defaultSettings.labs.labs.defaultValue);
const ignoredSettings = ['slack_url', 'members_from_address', 'members_support_address'];
// NOTE: drop support in Ghost 5.0
const deprecatedSupportedSettingsMap = {
default_locale: 'lang',
// Importer maintains as much backwards compatibility as possible
const renamedSettingsMap = {
default_locale: 'locale',
lang: 'locale',
active_timezone: 'timezone'
};
const deprecatedSupportedSettingsOneToManyMap = {
// NOTE: intentionally ignoring slack_url setting
slack: [{
@ -99,8 +101,8 @@ class SettingsImporter extends BaseImporter {
// NOTE: import settings removed in v3 and move them to ignored once Ghost v4 changes are done
this.dataToImport = this.dataToImport.map((data) => {
if (deprecatedSupportedSettingsMap[data.key]) {
data.key = deprecatedSupportedSettingsMap[data.key];
if (renamedSettingsMap[data.key]) {
data.key = renamedSettingsMap[data.key];
}
return data;

51
core/server/data/migrations/versions/5.0/2022-05-11-13-12-rename-settings.js Normal file
View file

@ -0,0 +1,51 @@
const logging = require('@tryghost/logging');
const _ = require('lodash');
const {createTransactionalMigration} = require('../../utils');
const renameMappings = [{
from: 'lang',
to: 'locale'
}, {
from: 'session_secret',
to: 'admin_session_secret'
}];
module.exports = createTransactionalMigration(
async function up(knex) {
const keys = _.flatMap(renameMappings, (m) => {
return [m.from, m.to];
});
const settings = await knex('settings').select('key', 'value').whereIn('key', keys);
// eslint-disable-next-line no-restricted-syntax
for (const renameMapping of renameMappings) {
if (_.find(settings, {key: renameMapping.to}) && _.find(settings, {key: renameMapping.from})) {
let updatedValue = _.find(settings, {key: renameMapping.from}).value;
// CASE: default settings were added already, update them with old values & remove old settings
logging.info(`Updating ${renameMapping.to} with value from ${renameMapping.from}`);
await knex('settings')
.where('key', renameMapping.to)
.update('value', updatedValue);
logging.info(`Deleting ${renameMapping.from}`);
await knex('settings')
.where('key', renameMapping.from)
.del();
} else if (_.find(settings, {key: renameMapping.from})) {
// CASE: old settings exist, update them
logging.info(`Renaming ${renameMapping.from} to ${renameMapping.to}`);
await knex('settings')
.where('key', renameMapping.from)
.update('key', renameMapping.to);
} else {
// CASE: old settings don't exist, let default settings create them
logging.warn(`Setting ${renameMapping.from} is missing, skipping update`);
}
}
},
async function down() {
// no-op: we can't rollback as there are irreversible migrations in 5.0
}
);

4
core/server/data/schema/default-settings/default-settings.json
View file

@ -20,7 +20,7 @@
"defaultValue": "[]",
"type": "array"
},
"session_secret": {
"admin_session_secret": {
"defaultValue": null,
"type": "string"
},
@ -98,7 +98,7 @@
},
"type": "string"
},
"lang": {
"locale": {
"defaultValue": "en",
"validations": {
"isEmpty": false

3
core/server/models/settings.js
View file

@ -53,8 +53,7 @@ function parseDefaultSettings() {
const dynamicDefault = {
db_hash: () => uuid.v4(),
public_hash: () => crypto.randomBytes(15).toString('hex'),
// @TODO: session_secret would ideally be named "admin_session_secret"
session_secret: () => crypto.randomBytes(32).toString('hex'),
admin_session_secret: () => crypto.randomBytes(32).toString('hex'),
theme_session_secret: () => crypto.randomBytes(32).toString('hex'),
members_public_key: () => getMembersKey('public'),
members_private_key: () => getMembersKey('private'),

2
core/server/services/auth/session/express-session.js
View file

@ -15,7 +15,7 @@ function getExpressSessionMiddleware() {
if (!unoExpressSessionMiddleware) {
unoExpressSessionMiddleware = session({
store: sessionStore,
secret: settingsCache.get('session_secret'),
secret: settingsCache.get('admin_session_secret'),
resave: false,
saveUninitialized: false,
name: 'ghost-admin-api-session',

4
core/shared/settings-cache/public.js
View file

@ -11,8 +11,8 @@ module.exports = {
cover_image: 'cover_image',
facebook: 'facebook',
twitter: 'twitter',
lang: 'lang',
locale: 'lang',
lang: 'locale',
locale: 'locale',
timezone: 'timezone',
codeinjection_head: 'codeinjection_head',
codeinjection_foot: 'codeinjection_foot',

8
test/e2e-api/admin/__snapshots__/settings.test.js.snap
View file

@ -139,7 +139,7 @@ Object {
"flags": null,
"group": "site",
"id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
"key": "lang",
"key": "locale",
"type": "string",
"updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"value": "ua",
@ -192,7 +192,7 @@ exports[`Settings API Can edit a setting 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "4084",
"content-length": "4086",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
@ -300,7 +300,7 @@ Object {
"flags": null,
"group": "site",
"id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
"key": "lang",
"key": "locale",
"type": "string",
"updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"value": "en",
@ -1112,7 +1112,7 @@ exports[`Settings API Can request all settings 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "18287",
"content-length": "18289",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",

2
test/e2e-api/admin/settings.test.js
View file

@ -134,7 +134,7 @@ describe('Settings API', function () {
value: 'twitter description'
},
{
key: 'lang',
key: 'locale',
value: 'ua'
},
{

26
test/integration/importer/v2.test.js
View file

@ -900,7 +900,7 @@ describe('Importer', function () {
});
});
it('imports settings fields deprecated in v2 and removed in v3: slack hook, permalinks', function () {
it('can import default_locale and active_timezone', function () {
// Prevent events from being fired to avoid side-effects
const EventRegistry = require('../../../core/server/lib/common/events');
sinon.stub(EventRegistry, 'emit').callsFake(() => {});
@ -920,7 +920,7 @@ describe('Importer', function () {
return dataImporter.doImport(exportData, importOptions)
.then(function (imported) {
imported.problems.length.should.eql(0);
return models.Settings.findOne(_.merge({key: 'lang'}, testUtils.context.internal));
return models.Settings.findOne(_.merge({key: 'locale'}, testUtils.context.internal));
})
.then(function (result) {
result.attributes.value.should.eql('ua');
@ -933,6 +933,28 @@ describe('Importer', function () {
});
});
it('can import lang', function () {
// Prevent events from being fired to avoid side-effects
const EventRegistry = require('../../../core/server/lib/common/events');
sinon.stub(EventRegistry, 'emit').callsFake(() => {});
const exportData = exportedBodyV2().db[0];
exportData.data.settings[0] = testUtils.DataGenerator.forKnex.createSetting({
key: 'lang',
value: 'ua'
});
return dataImporter.doImport(exportData, importOptions)
.then(function (imported) {
imported.problems.length.should.eql(0);
return models.Settings.findOne(_.merge({key: 'locale'}, testUtils.context.internal));
})
.then(function (result) {
result.attributes.value.should.eql('ua');
});
});
it('does import settings with string booleans', function () {
const exportData = exportedBodyV2().db[0];

2
test/integration/settings/settings.test.js
View file

@ -20,7 +20,7 @@ describe('Settings', function () {
'next_update_check',
'notifications',
'version_notifications',
'session_secret',
'admin_session_secret',
'theme_session_secret',
'ghost_public_key',
'ghost_private_key',

50
test/regression/api/admin/settings.test.js
View file

@ -35,7 +35,7 @@ const defaultSettingsKeyTypes = [
group: 'site'
},
{
key: 'lang',
key: 'locale',
type: 'string',
group: 'site'
},
@ -679,8 +679,8 @@ describe('Settings API (canary)', function () {
jsonResponse.settings[0].value.should.match(jsonObjectRegex);
});
it('Can edit deprecated lang setting', function () {
return request.get(localUtils.API.getApiQuery('settings/lang/'))
it('Can edit newly introduced locale setting', function () {
return request.get(localUtils.API.getApiQuery('settings/locale/'))
.set('Origin', config.get('url'))
.set('Accept', 'application/json')
.expect('Content-Type', /json/)
@ -689,7 +689,7 @@ describe('Settings API (canary)', function () {
let jsonResponse = res.body;
should.exist(jsonResponse);
should.exist(jsonResponse.settings);
jsonResponse.settings = [{key: 'lang', value: 'ua'}];
jsonResponse.settings = [{key: 'locale', value: 'ge'}];
return jsonResponse;
})
@ -710,50 +710,12 @@ describe('Settings API (canary)', function () {
jsonResponse.settings.length.should.eql(1);
testUtils.API.checkResponseValue(jsonResponse.settings[0], ['id', 'group', 'key', 'value', 'type', 'flags', 'created_at', 'updated_at']);
jsonResponse.settings[0].key.should.eql('lang');
jsonResponse.settings[0].value.should.eql('ua');
jsonResponse.settings[0].key.should.eql('locale');
jsonResponse.settings[0].value.should.eql('ge');
});
});
});
// @TODO: swap this test for the one above when renaming the setting is in place
// it('Can edit newly introduced locale setting', function () {
// return request.get(localUtils.API.getApiQuery('settings/locale/'))
// .set('Origin', config.get('url'))
// .set('Accept', 'application/json')
// .expect('Content-Type', /json/)
// .expect('Cache-Control', testUtils.cacheRules.private)
// .then(function (res) {
// let jsonResponse = res.body;
// should.exist(jsonResponse);
// should.exist(jsonResponse.settings);
// jsonResponse.settings = [{key: 'locale', value: 'ge'}];
// return jsonResponse;
// })
// .then((editedSetting) => {
// return request.put(localUtils.API.getApiQuery('settings/'))
// .set('Origin', config.get('url'))
// .send(editedSetting)
// .expect('Content-Type', /json/)
// .expect('Cache-Control', testUtils.cacheRules.private)
// .expect(200)
// .then(function (res) {
// should.exist(res.headers['x-cache-invalidate']);
// const jsonResponse = res.body;
// should.exist(jsonResponse);
// should.exist(jsonResponse.settings);
// jsonResponse.settings.length.should.eql(1);
// testUtils.API.checkResponseValue(jsonResponse.settings[0], ['id', 'group', 'key', 'value', 'type', 'flags', 'created_at', 'updated_at']);
// jsonResponse.settings[0].key.should.eql('locale');
// jsonResponse.settings[0].value.should.eql('ge');
// });
// });
// });
it('Can read timezone', function (done) {
request.get(localUtils.API.getApiQuery('settings/timezone/'))
.set('Origin', config.get('url'))

2
test/unit/server/data/schema/integrity.test.js
View file

@ -37,7 +37,7 @@ describe('DB version integrity', function () {
// Only these variables should need updating
const currentSchemaHash = '2f4266e6e5087ad92dd30f3e721d46e5';
const currentFixturesHash = '2219972fb91a30f8d740e05afd3a033c';
const currentSettingsHash = 'ffd899a82b0ad2886e92d8244bcbca6a';
const currentSettingsHash = '53606d11dafcd3f4ab1acb3962122082';
const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01';
// If this test is failing, then it is likely a change has been made that requires a DB version bump,

4
test/utils/fixtures/default-settings.json
View file

@ -20,7 +20,7 @@
"defaultValue": "[]",
"type": "array"
},
"session_secret": {
"admin_session_secret": {
"defaultValue": null,
"type": "string"
},
@ -98,7 +98,7 @@
},
"type": "string"
},
"lang": {
"locale": {
"defaultValue": "en",
"validations": {
"isEmpty": false