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

Added extra validation for some settings of array type

Browse source 
refs https://github.com/TryGhost/Team/issues/754
refs a7dec233ba

- Additional validation protects from problems like the ones in refed commit from even getting through to the database.
- At the moment only used notificatons and couple more settings to ensure they are arrays when passed into the API. This is to avoid making big change in settings straight away - this is a problematic area which needs cautious approach.
- Ideally in the future the list of settings to check the "array" type (and other types) should be automatically generated based on the default-settings.json (or whatever way we define settings in the db a that moment)
- There's an ugly code-tripplication going on in this change. This is a separate topic that will be addressed once we work on API cleanup.
This commit is contained in:
Naz 2021-06-28 14:25:29 +04:00
parent 4fe723cd1b
commit d9ddc2db6a
6 changed files with 120 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
core/server/api/canary/utils/validators/input/settings.js
View file

@ -1,7 +1,7 @@
const Promise = require('bluebird');
const _ = require('lodash');
const i18n = require('../../../../../../shared/i18n');
const {NotFoundError} = require('@tryghost/errors');
const {NotFoundError, ValidationError} = require('@tryghost/errors');
module.exports = {
read(apiConfig, frame) {
@ -27,6 +27,30 @@ module.exports = {
})
}));
}
// TODO: the below array is INCOMPLETE
// it should include all setting values that have array as a type
const arrayTypeSettings = [
'notifications',
'navigation',
'secondary_navigation'
];
if (arrayTypeSettings.includes(setting.key)) {
const typeError = new ValidationError({
message: `Value in ${setting.key} should be an array.`,
property: 'value'
});
try {
const value = JSON.parse(setting.value);
if (!_.isArray(value)) {
errors.push(typeError);
}
} catch (err) {
errors.push(typeError);
}
}
});
if (errors.length) {

24
core/server/api/v2/utils/validators/input/settings.js
View file

@ -33,6 +33,30 @@ module.exports = {
);
}
}
// TODO: the below array is INCOMPLETE
// it should include all setting values that have array as a type
const arrayTypeSettings = [
'notifications',
'navigation',
'secondary_navigation'
];
if (arrayTypeSettings.includes(setting.key)) {
const typeError = new ValidationError({
message: `Value in ${setting.key} should be an array.`,
property: 'value'
});
try {
const value = JSON.parse(setting.value);
if (!_.isArray(value)) {
errors.push(typeError);
}
} catch (err) {
errors.push(typeError);
}
}
});
if (errors.length) {

26
core/server/api/v3/utils/validators/input/settings.js
View file

@ -1,7 +1,7 @@
const Promise = require('bluebird');
const _ = require('lodash');
const i18n = require('../../../../../../shared/i18n');
const {NotFoundError} = require('@tryghost/errors');
const {NotFoundError, ValidationError} = require('@tryghost/errors');
module.exports = {
read(apiConfig, frame) {
@ -27,6 +27,30 @@ module.exports = {
})
}));
}
// TODO: the below array is INCOMPLETE
// it should include all setting values that have array as a type
const arrayTypeSettings = [
'notifications',
'navigation',
'secondary_navigation'
];
if (arrayTypeSettings.includes(setting.key)) {
const typeError = new ValidationError({
message: `Value in ${setting.key} should be an array.`,
property: 'value'
});
try {
const value = JSON.parse(setting.value);
if (!_.isArray(value)) {
errors.push(typeError);
}
} catch (err) {
errors.push(typeError);
}
}
});
if (errors.length) {

17
test/regression/api/canary/admin/settings_spec.js
View file

@ -1227,10 +1227,25 @@ describe('Settings API (canary)', function () {
dbSettings.should.have.property('twitter_image', '__GHOST_URL__/content/images/twitter_image.png');
});
it('Can only send array values for keys defined with array type', async function () {
const settingsToChange = {
settings: [
{key: 'navigation', value: 'not an array'}
]
};
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(422);
});
it('Cannot edit notifications key through API', async function () {
const settingsToChange = {
settings: [
{key: 'notifications', value: `anything`}
{key: 'notifications', value: JSON.stringify(['do not touch me'])}
]
};

15
test/regression/api/v2/admin/settings_spec.js
View file

@ -708,6 +708,21 @@ describe('Settings API (v2)', function () {
localUtils.API.checkResponse(putBody, 'settings');
});
it('Can only send array values for keys defined with array type', async function () {
const settingsToChange = {
settings: [
{key: 'navigation', value: 'not an array'}
]
};
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(422);
});
});
describe('As Admin', function () {

15
test/regression/api/v3/admin/settings_spec.js
View file

@ -702,6 +702,21 @@ describe('Settings API (v3)', function () {
putBody.settings[0].key.should.eql('slack_username');
putBody.settings[0].value.should.eql('can edit me');
});
it('Can only send array values for keys defined with array type', async function () {
const settingsToChange = {
settings: [
{key: 'navigation', value: 'not an array'}
]
};
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(422);
});
});
describe('As Admin', function () {