tqt/Ghost
2
1
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2023-12-13 21:00:40 +01:00
Code Issues 1 Releases Wiki Activity

ACL and strict rules for Settings API

Browse source 
Ref #2061

- Add canThis permission checks to settings api calls
- Add strict rules about accessing core settings without internal: true
- Omit core settings in browse() call unless internal: true
- Update unit tests to call api.settings with contexts
- Add a couple unit tests for new scenarios
- Update all api.settings calls in the app to call with internal context
- Re-arrange permissions.init in server startup so config.theme.update
can access settings without permissions error
This commit is contained in:
Jacob Gable 2014-05-06 19:49:25 -05:00
parent 1fb958834c
commit 298077582b
16 changed files with 193 additions and 69 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
core/server/api/db.js
View file

@ -44,7 +44,7 @@ db = {
return when.reject({type: 'InternalServerError', message: 'Please select a .json file to import.'});
}
return api.settings.read({ key: 'databaseVersion' }).then(function (response) {
return api.settings.read.call({ internal: true }, { key: 'databaseVersion' }).then(function (response) {
var setting = response.settings[0];
return when(setting.value);

71
core/server/api/settings.js
View file

@ -2,6 +2,7 @@ var _ = require('lodash'),
dataProvider = require('../models'),
when = require('when'),
config = require('../config'),
canThis = require('../permissions').canThis,
settings,
settingsFilter,
updateSettingsCache,
@ -145,9 +146,19 @@ settings = {
// **takes:** options object
browse: function browse(options) {
//TODO: omit where type==core
var self = this;
// **returns:** a promise for a settings json object
return when(settingsResult(settingsCache, options.type));
return canThis(this).browse.setting().then(function () {
var result = settingsResult(settingsCache, options.type);
// Omit core settings unless internal request
if (!self.internal) {
result.settings = _.filter(result.settings, function (setting) { return setting.type !== 'core'; });
}
return result;
});
},
// #### Read
@ -158,16 +169,24 @@ settings = {
options = { key: options };
}
var setting = settingsCache[options.key],
result = {};
var self = this;
if (!setting) {
return when.reject({type: 'NotFound', message: 'Unable to find setting: ' + options.key});
}
return canThis(this).read.setting(options.key).then(function () {
var setting = settingsCache[options.key],
result = {};
result[options.key] = setting;
if (!setting) {
return when.reject({type: 'NotFound', message: 'Unable to find setting: ' + options.key});
}
return when(settingsResult(result));
if (!self.internal && setting.type === 'core') {
return when.reject({type: 'NoPermission', message: 'Attempted to access core setting on external request' });
}
result[options.key] = setting;
return settingsResult(result);
});
},
// #### Edit
@ -175,7 +194,24 @@ settings = {
// **takes:** either a json object representing a collection of settings, or a key and value pair
edit: function edit(key, value) {
var self = this,
type;
type,
canEditAllSettings = function (settingsInfo) {
var checks = _.map(settingsInfo, function (settingInfo) {
var setting = settingsCache[settingInfo.key];
if (!setting) {
return when.reject({type: 'NotFound', message: 'Unable to find setting: ' + settingInfo.key});
}
if (!self.internal && setting.type === 'core') {
return when.reject({type: 'NoPermission', message: 'Attempted to access core setting on external request' });
}
return canThis(self).edit.setting(settingInfo.key);
});
return when.all(checks);
};
// Allow shorthand syntax
if (_.isString(key)) {
@ -192,7 +228,9 @@ settings = {
return setting.key === 'type' || setting.key === 'availableThemes' || setting.key === 'availableApps';
});
return dataProvider.Settings.edit(key, {user: self.user}).then(function (result) {
return canEditAllSettings(key).then(function () {
return dataProvider.Settings.edit(key, {user: self.user});
}).then(function (result) {
var readResult = readSettingsResult(result);
return updateSettingsCache(readResult).then(function () {
@ -200,10 +238,17 @@ settings = {
}).then(function () {
return settingsResult(readResult, type);
});
}).otherwise(function (error) {
}).catch(function (error) {
// In case something goes wrong it is most likely because of an invalid key
// or because of a badly formatted request.
return when.reject({type: 'BadRequest', message: error.message});
// Check for actual javascript error, not api error
if (error instanceof Error) {
return when.reject({type: 'BadRequest', message: error.message});
}
// Pass along API error
return when.reject(error);
});
}
};

6
core/server/api/users.js
View file

@ -114,21 +114,21 @@ users = {
generateResetToken: function generateResetToken(email) {
var expires = Date.now() + ONE_DAY;
return settings.read('dbHash').then(function (response) {
return settings.read.call({ internal: true }, 'dbHash').then(function (response) {
var dbHash = response.settings[0].value;
return dataProvider.User.generateResetToken(email, expires, dbHash);
});
},
validateToken: function validateToken(token) {
return settings.read('dbHash').then(function (response) {
return settings.read.call({ internal: true }, 'dbHash').then(function (response) {
var dbHash = response.settings[0].value;
return dataProvider.User.validateToken(token, dbHash);
});
},
resetPassword: function resetPassword(token, newPassword, ne2Password) {
return settings.read('dbHash').then(function (response) {
return settings.read.call({ internal: true }, 'dbHash').then(function (response) {
var dbHash = response.settings[0].value;
return dataProvider.User.resetPassword(token, newPassword, ne2Password, dbHash);
});

6
core/server/apps/index.js
View file

@ -9,7 +9,7 @@ var _ = require('lodash'),
function getInstalledApps() {
return api.settings.read('installedApps').then(function (response) {
return api.settings.read.call({ internal: true }, 'installedApps').then(function (response) {
var installed = response.settings[0];
installed.value = installed.value || '[]';
@ -28,7 +28,7 @@ function saveInstalledApps(installedApps) {
return getInstalledApps().then(function (currentInstalledApps) {
var updatedAppsInstalled = _.uniq(installedApps.concat(currentInstalledApps));
return api.settings.edit.call({user: 1}, 'installedApps', updatedAppsInstalled);
return api.settings.edit.call({internal: true}, 'installedApps', updatedAppsInstalled);
});
}
@ -38,7 +38,7 @@ module.exports = {
try {
// We have to parse the value because it's a string
api.settings.read('activeApps').then(function (response) {
api.settings.read.call({ internal: true }, 'activeApps').then(function (response) {
var aApps = response.settings[0];
appsToLoad = JSON.parse(aApps.value) || [];

9
core/server/config/theme.js
View file

@ -17,11 +17,12 @@ function theme() {
// there would be a race condition where the ./models/base
// tries to access the config() object before it is created.
function update(settings, configUrl) {
// TODO: Pass the context into this method instead of hard coding internal: true?
return when.all([
settings.read('title'),
settings.read('description'),
settings.read('logo'),
settings.read('cover')
settings.read.call({ internal: true }, 'title'),
settings.read.call({ internal: true }, 'description'),
settings.read.call({ internal: true }, 'logo'),
settings.read.call({ internal: true }, 'cover')
]).then(function (globals) {
// normalise the URL by removing any trailing slash
themeConfig.url = configUrl.replace(/\/$/, '');

2
core/server/config/url.js
View file

@ -148,7 +148,7 @@ function urlFor(context, data, absolute) {
// - post - a json object representing a post
// - absolute (optional, default:false) - boolean whether or not the url should be absolute
function urlForPost(settings, post, absolute) {
return settings.read('permalinks').then(function (response) {
return settings.read.call({ internal: true }, 'permalinks').then(function (response) {
var permalinks = response.settings[0];
return urlFor('post', {post: post, permalinks: permalinks}, absolute);

14
core/server/controllers/frontend.js
View file

@ -21,7 +21,7 @@ var moment = require('moment'),
staticPostPermalink = new Route(null, '/:slug/:edit?');
function getPostPage(options) {
return api.settings.read('postsPerPage').then(function (response) {
return api.settings.read.call({ internal: true }, 'postsPerPage').then(function (response) {
var postPP = response.settings[0],
postsPerPage = parseInt(postPP.value, 10);
@ -122,7 +122,7 @@ frontendControllers = {
// Render the page of posts
filters.doFilter('prePostsRender', page.posts).then(function (posts) {
api.settings.read('activeTheme').then(function (response) {
api.settings.read.call({ internal: true }, 'activeTheme').then(function (response) {
var activeTheme = response.settings[0],
paths = config().paths.availableThemes[activeTheme.value],
view = paths.hasOwnProperty('tag.hbs') ? 'tag' : 'index',
@ -143,7 +143,7 @@ frontendControllers = {
editFormat,
usingStaticPermalink = false;
api.settings.read('permalinks').then(function (response) {
api.settings.read.call({ internal: true }, 'permalinks').then(function (response) {
var permalink = response.settings[0],
postLookup;
@ -198,7 +198,7 @@ frontendControllers = {
setReqCtx(req, post);
filters.doFilter('prePostsRender', post).then(function (post) {
api.settings.read('activeTheme').then(function (response) {
api.settings.read.call({ internal: true }, 'activeTheme').then(function (response) {
var activeTheme = response.settings[0],
paths = config().paths.availableThemes[activeTheme.value],
view = template.getThemeViewForPost(paths, post);
@ -277,9 +277,9 @@ frontendControllers = {
}
return when.settle([
api.settings.read('title'),
api.settings.read('description'),
api.settings.read('permalinks')
api.settings.read.call({ internal: true }, 'title'),
api.settings.read.call({ internal: true }, 'description'),
api.settings.read.call({ internal: true }, 'permalinks')
]).then(function (result) {
var options = {};

6
core/server/helpers/index.js
View file

@ -385,7 +385,7 @@ coreHelpers.body_class = function (options) {
classes.push('page');
}
return api.settings.read('activeTheme').then(function (response) {
return api.settings.read.call({ internal: true }, 'activeTheme').then(function (response) {
var activeTheme = response.settings[0],
paths = config().paths.availableThemes[activeTheme.value],
view;
@ -528,8 +528,8 @@ coreHelpers.meta_description = function (options) {
coreHelpers.e = function (key, defaultString, options) {
var output;
when.all([
api.settings.read('defaultLang'),
api.settings.read('forceI18n')
api.settings.read.call({ internal: true }, 'defaultLang'),
api.settings.read.call({ internal: true }, 'forceI18n')
]).then(function (values) {
if (values[0].settings.value === 'en' &&
_.isEmpty(options.hash) &&

10
core/server/index.js
View file

@ -52,7 +52,7 @@ function doFirstRun() {
}
function initDbHashAndFirstRun() {
return when(api.settings.read('dbHash')).then(function (response) {
return api.settings.read.call({ internal: true }, 'dbHash').then(function (response) {
var hash = response.settings[0].value,
initHash;
@ -60,7 +60,7 @@ function initDbHashAndFirstRun() {
if (dbHash === null) {
initHash = uuid.v4();
return when(api.settings.edit.call({user: 1}, 'dbHash', initHash)).then(function (response) {
return api.settings.edit.call({ internal: true }, 'dbHash', initHash).then(function (response) {
dbHash = response.settings[0].value;
return dbHash;
}).then(doFirstRun);
@ -193,6 +193,10 @@ function init(server) {
}).then(function () {
// Initialize the settings cache
return api.init();
}).then(function () {
// Initialize the permissions actions and objects
// NOTE: Must be done before the config.theme.update and initDbHashAndFirstRun calls
return permissions.init();
}).then(function () {
// We must pass the api.settings object
// into this method due to circular dependencies.
@ -201,8 +205,6 @@ function init(server) {
return when.join(
// Check for or initialise a dbHash.
initDbHashAndFirstRun(),
// Initialize the permissions actions and objects
permissions.init(),
// Initialize mail
mailer.init(),
// Initialize apps

2
core/server/mail.js
View file

@ -104,7 +104,7 @@ GhostMailer.prototype.send = function (message) {
return when.reject(new Error('Email Error: Incomplete message data.'));
}
return api.settings.read('email').then(function (response) {
return api.settings.read.call({ internal: true }, 'email').then(function (response) {
var email = response.settings[0],
to = message.to || email.value;

2
core/server/middleware/index.js
View file

@ -150,7 +150,7 @@ function manageAdminAndTheme(req, res, next) {
expressServer.enable(expressServer.get('activeTheme'));
expressServer.disable('admin');
}
api.settings.read('activeTheme').then(function (response) {
api.settings.read.call({ internal: true }, 'activeTheme').then(function (response) {
var activeTheme = response.settings[0];
// Check if the theme changed

2
core/server/middleware/middleware.js
View file

@ -170,7 +170,7 @@ var middleware = {
// to allow unit testing
forwardToExpressStatic: function (req, res, next) {
api.settings.read('activeTheme').then(function (response) {
api.settings.read.call({ internal: true }, 'activeTheme').then(function (response) {
var activeTheme = response.settings[0];
// For some reason send divides the max age number by 1000
express['static'](path.join(config().paths.themePath, activeTheme.value), {maxAge: ONE_HOUR_MS})(req, res, next);

10
core/server/update-check.js
View file

@ -50,9 +50,9 @@ function updateCheckData() {
ops = [],
mailConfig = config().mail;
ops.push(api.settings.read('dbHash').otherwise(errors.rejectError));
ops.push(api.settings.read('activeTheme').otherwise(errors.rejectError));
ops.push(api.settings.read('activeApps')
ops.push(api.settings.read.call({ internal: true }, 'dbHash').otherwise(errors.rejectError));
ops.push(api.settings.read.call({ internal: true }, 'activeTheme').otherwise(errors.rejectError));
ops.push(api.settings.read.call({ internal: true }, 'activeApps')
.then(function (response) {
var apps = response.settings[0];
try {
@ -171,7 +171,7 @@ function updateCheck() {
// No update check
deferred.resolve();
} else {
api.settings.read('nextUpdateCheck').then(function (result) {
api.settings.read.call({ internal: true }, 'nextUpdateCheck').then(function (result) {
var nextUpdateCheck = result.settings[0];
if (nextUpdateCheck && nextUpdateCheck.value && nextUpdateCheck.value > moment().unix()) {
@ -191,7 +191,7 @@ function updateCheck() {
}
function showUpdateNotification() {
return api.settings.read('displayUpdateNotification').then(function (response) {
return api.settings.read.call({ internal: true }, 'displayUpdateNotification').then(function (response) {
var display = response.settings[0];
// Version 0.4 used boolean to indicate the need for an update. This special case is

2
core/test/functional/routes/api/settings_test.js
View file

@ -219,7 +219,7 @@ describe('Settings API', function () {
request.put(testUtils.API.getApiQuery('settings/'))
.set('X-CSRF-Token', csrfToken)
.send(jsonResponse)
.expect(400)
.expect(404)
.end(function (err, res) {
if (err) {
return done(err);

114
core/test/integration/api/api_settings_spec.js
View file

@ -1,13 +1,34 @@
/*globals describe, before, beforeEach, afterEach, it */
var testUtils = require('../../utils'),
should = require('should'),
_ = require('lodash'),
// Stuff we are testing
permissions = require('../../../server/permissions'),
DataGenerator = require('../../utils/fixtures/data-generator'),
SettingsAPI = require('../../../server/api/settings');
describe('Settings API', function () {
var defaultContext = {
user: 1
},
internalContext = {
internal: true
},
callApiWithContext = function (context, method) {
return SettingsAPI[method].apply(context, _.toArray(arguments).slice(2));
},
getErrorDetails = function (done) {
return function (err) {
if (err instanceof Error) {
return done(err);
}
done(new Error(err.message));
};
};
before(function (done) {
testUtils.clearData().then(function () {
done();
@ -22,6 +43,9 @@ describe('Settings API', function () {
.then(function () {
return SettingsAPI.updateSettingsCache();
})
.then(function () {
return permissions.init();
})
.then(function () {
done();
}).catch(done);
@ -30,55 +54,84 @@ describe('Settings API', function () {
afterEach(function (done) {
testUtils.clearData().then(function () {
done();
}).catch(done);
}).catch(getErrorDetails(done));
});
it('can browse', function (done) {
return SettingsAPI.browse('blog').then(function (results) {
return callApiWithContext(defaultContext, 'browse', 'blog').then(function (results) {
should.exist(results);
testUtils.API.checkResponse(results, 'settings');
results.settings.length.should.be.above(0);
testUtils.API.checkResponse(results.settings[0], 'setting');
// Check for a core setting
should.not.exist(_.find(results.settings, function (setting) { return setting.type === 'core'; }));
done();
}).catch(done);
}).catch(getErrorDetails(done));
});
it('returns core settings for internal requests when browsing', function (done){
return callApiWithContext(internalContext, 'browse', 'blog').then(function (results) {
should.exist(results);
testUtils.API.checkResponse(results, 'settings');
results.settings.length.should.be.above(0);
testUtils.API.checkResponse(results.settings[0], 'setting');
// Check for a core setting
should.exist(_.find(results.settings, function (setting) { return setting.type === 'core'; }));
done();
}).catch(getErrorDetails(done));
});
it('can read by string', function (done) {
return SettingsAPI.read('title').then(function (response) {
return callApiWithContext(defaultContext, 'read', 'title').then(function (response) {
should.exist(response);
testUtils.API.checkResponse(response, 'settings');
response.settings.length.should.equal(1);
testUtils.API.checkResponse(response.settings[0], 'setting');
done();
}).catch(done);
}).catch(getErrorDetails(done));
});
it('cannot read core settings if not an internal request', function (done) {
return callApiWithContext(defaultContext, 'read', 'databaseVersion').then(function (response) {
done(new Error('Allowed to read databaseVersion with external request'));
}).catch(function (err) {
should.exist(err);
err.message.should.equal('Attempted to access core setting on external request');
done();
});
});
it('can read core settings if an internal request', function (done) {
return callApiWithContext(internalContext, 'read', 'databaseVersion').then(function (response) {
should.exist(response);
testUtils.API.checkResponse(response, 'settings');
response.settings.length.should.equal(1);
testUtils.API.checkResponse(response.settings[0], 'setting');
done();
}).catch(getErrorDetails(done));
});
it('can read by object key', function (done) {
return SettingsAPI.read({ key: 'title' }).then(function (response) {
return callApiWithContext(defaultContext, 'read', { key: 'title' }).then(function (response) {
should.exist(response);
testUtils.API.checkResponse(response, 'settings');
response.settings.length.should.equal(1);
testUtils.API.checkResponse(response.settings[0], 'setting');
done();
}).catch(done);
}).catch(getErrorDetails(done));
});
it('can edit', function (done) {
return SettingsAPI.edit({ settings: [{ key: 'title', value: 'UpdatedGhost'}]}).then(function (response) {
should.exist(response);
testUtils.API.checkResponse(response, 'settings');
response.settings.length.should.equal(1);
testUtils.API.checkResponse(response.settings[0], 'setting');
done();
}).catch(done);
})
it('can edit, by key/value', function (done) {
return SettingsAPI.edit('title', 'UpdatedGhost').then(function (response) {
return callApiWithContext(defaultContext, 'edit', { settings: [{ key: 'title', value: 'UpdatedGhost'}]}).then(function (response) {
should.exist(response);
testUtils.API.checkResponse(response, 'settings');
response.settings.length.should.equal(1);
@ -87,4 +140,27 @@ describe('Settings API', function () {
done();
}).catch(done);
});
it('can edit, by key/value', function (done) {
return callApiWithContext(defaultContext, 'edit', 'title', 'UpdatedGhost').then(function (response) {
should.exist(response);
testUtils.API.checkResponse(response, 'settings');
response.settings.length.should.equal(1);
testUtils.API.checkResponse(response.settings[0], 'setting');
done();
}).catch(getErrorDetails(done));
});
it('cannot edit a core setting if not an internal request', function (done) {
return callApiWithContext(defaultContext, 'edit', 'databaseVersion', '999').then(function (response) {
done(new Error('Allowed to edit a core setting as external request'));
}).catch(function (err) {
should.exist(err);
err.message.should.equal('Attempted to access core setting on external request');
done();
});
});
});

4
core/test/integration/update_check_spec.js
View file

@ -49,7 +49,7 @@ describe('Update Check', function () {
var updateCheckData = updateCheck.__get__('updateCheckData');
updateCheckData().then(function (data) {
data.should.exist;
should.exist(data);
data.ghost_version.should.equal(packageInfo.version);
data.node_version.should.equal(process.versions.node);
data.env.should.equal(process.env.NODE_ENV);
@ -65,6 +65,6 @@ describe('Update Check', function () {
data.npm_version.should.not.be.empty;
done();
}).otherwise(done);
}).catch(done);
});
});