From c5eda57f1e298bc6796514b90a21f0d031dd8d7a Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Thu, 14 Apr 2016 18:32:43 +0100 Subject: [PATCH] Minor internal apps improvements refs #6589 - add internalAppsPath as a proper config path - middleware/routes will be setup for any internal apps which have the function - this should be refactored into some sort of proper hooks system as part of apps - internal apps get permission to do anything the proxy allows --- core/server/apps/loader.js | 5 +++-- core/server/apps/permissions.js | 1 - core/server/apps/proxy.js | 9 +++++++-- core/server/config/index.js | 4 +++- core/server/middleware/index.js | 11 ++++++++--- core/server/routes/frontend.js | 18 ++++++++++++------ core/test/unit/apps_spec.js | 33 +++++++++++++++++++++++++++++++++ core/test/unit/config_spec.js | 1 + 8 files changed, 67 insertions(+), 15 deletions(-) diff --git a/core/server/apps/loader.js b/core/server/apps/loader.js index 719f69a1ff..e0a0afe0b4 100644 --- a/core/server/apps/loader.js +++ b/core/server/apps/loader.js @@ -17,7 +17,7 @@ function isInternalApp(name) { // Get the full path to an app by name function getAppAbsolutePath(name) { if (isInternalApp(name)) { - return path.join(config.paths.corePath, '/server/apps/', name); + return path.join(config.paths.internalAppPath, name); } return path.join(config.paths.appPath, name); @@ -49,7 +49,8 @@ function getAppByName(name, permissions) { var AppClass = loadApp(getAppRelativePath(name), isInternalApp(name)), appProxy = new AppProxy({ name: name, - permissions: permissions + permissions: permissions, + internal: isInternalApp(name) }), app; diff --git a/core/server/apps/permissions.js b/core/server/apps/permissions.js index 921434022b..cdb82eb501 100644 --- a/core/server/apps/permissions.js +++ b/core/server/apps/permissions.js @@ -1,4 +1,3 @@ - var fs = require('fs'), Promise = require('bluebird'), path = require('path'), diff --git a/core/server/apps/proxy.js b/core/server/apps/proxy.js index 257c457349..e8558b3322 100644 --- a/core/server/apps/proxy.js +++ b/core/server/apps/proxy.js @@ -5,7 +5,7 @@ var _ = require('lodash'), i18n = require('../i18n'), generateProxyFunctions; -generateProxyFunctions = function (name, permissions) { +generateProxyFunctions = function (name, permissions, isInternal) { var getPermission = function (perm) { return permissions[perm]; }, @@ -21,6 +21,11 @@ generateProxyFunctions = function (name, permissions) { }); }, runIfPermissionToMethod = function (perm, method, wrappedFunc, context, args) { + // internal apps get all permissions + if (isInternal) { + return wrappedFunc.apply(context, args); + } + var permValue = getPermissionToMethod(perm, method); if (!permValue) { @@ -92,7 +97,7 @@ function AppProxy(options) { throw new Error(i18n.t('errors.apps.mustProvideAppPermissions.error')); } - _.extend(this, generateProxyFunctions(options.name, options.permissions)); + _.extend(this, generateProxyFunctions(options.name, options.permissions, options.internal)); } module.exports = AppProxy; diff --git a/core/server/config/index.js b/core/server/config/index.js index 9786ecaf48..2ca4561632 100644 --- a/core/server/config/index.js +++ b/core/server/config/index.js @@ -169,6 +169,7 @@ ConfigManager.prototype.set = function (config) { themePath: path.resolve(contentPath, 'themes'), appPath: path.resolve(contentPath, 'apps'), imagesPath: path.resolve(contentPath, 'images'), + internalAppPath: path.join(corePath, '/server/apps/'), imagesRelPath: 'content/images', adminViews: path.join(corePath, '/server/views/'), @@ -190,7 +191,8 @@ ConfigManager.prototype.set = function (config) { author: 'author', page: 'page', preview: 'p', - private: 'private' + private: 'private', + subscribe: 'subscribe' }, internalApps: ['private-blogging'], slugs: { diff --git a/core/server/middleware/index.js b/core/server/middleware/index.js index 5a35207257..d8d3971b1e 100644 --- a/core/server/middleware/index.js +++ b/core/server/middleware/index.js @@ -25,7 +25,6 @@ var bodyParser = require('body-parser'), themeHandler = require('./theme-handler'), uncapitalise = require('./uncapitalise'), cors = require('./cors'), - privateBlogging = require('../apps/private-blogging'), ClientPasswordStrategy = require('passport-oauth2-client-password').Strategy, BearerStrategy = require('passport-http-bearer').Strategy, @@ -110,8 +109,14 @@ setupMiddleware = function setupMiddleware(blogApp, adminApp) { // Theme only config blogApp.use(staticTheme()); - // setup middleware for private blogs - privateBlogging.setupMiddleware(blogApp); + // setup middleware for internal apps + // @TODO: refactor this to be a proper app middleware hook for internal & external apps + config.internalApps.forEach(function (appName) { + var app = require(path.join(config.paths.internalAppPath, appName)); + if (app.hasOwnProperty('setupMiddleware')) { + app.setupMiddleware(blogApp); + } + }); // Serve sitemap.xsl file blogApp.use(serveSharedFile('sitemap.xsl', 'text/xsl', utils.ONE_DAY_S)); diff --git a/core/server/routes/frontend.js b/core/server/routes/frontend.js index 6ceffe5fc4..5407fa3a2c 100644 --- a/core/server/routes/frontend.js +++ b/core/server/routes/frontend.js @@ -1,9 +1,9 @@ -var frontend = require('../controllers/frontend'), - channels = require('../controllers/frontend/channels'), +var express = require('express'), + path = require('path'), config = require('../config'), - express = require('express'), + frontend = require('../controllers/frontend'), + channels = require('../controllers/frontend/channels'), utils = require('../utils'), - privateBlogging = require('../apps/private-blogging'), frontendRoutes; @@ -34,8 +34,14 @@ frontendRoutes = function frontendRoutes() { // Default router.get('*', frontend.single); - // @TODO: this can be removed once the proper app route hooks have been set up. - privateBlogging.setupRoutes(router); + // setup routes for internal apps + // @TODO: refactor this to be a proper app route hook for internal & external apps + config.internalApps.forEach(function (appName) { + var app = require(path.join(config.paths.internalAppPath, appName)); + if (app.hasOwnProperty('setupRoutes')) { + app.setupRoutes(router); + } + }); return router; }; diff --git a/core/test/unit/apps_spec.js b/core/test/unit/apps_spec.js index bed30f38b9..ded2ffe17a 100644 --- a/core/test/unit/apps_spec.js +++ b/core/test/unit/apps_spec.js @@ -254,6 +254,24 @@ describe('Apps', function () { registerSpy.called.should.equal(false); }); + + it('does allow INTERNAL app to register helper without permission', function () { + var registerSpy = sandbox.spy(helpers, 'registerThemeHelper'), + appProxy = new AppProxy({ + name: 'TestApp', + permissions: {}, + internal: true + }); + + function registerWithoutPermissions() { + appProxy.helpers.register('otherHelper', sandbox.stub().returns('test result')); + } + + registerWithoutPermissions.should.not.throw('The App "TestApp" attempted to perform an action or access a ' + + 'resource (helpers.otherHelper) without permission.'); + + registerSpy.called.should.equal(true); + }); }); describe('Sandbox', function () { @@ -332,6 +350,21 @@ describe('Apps', function () { loadApp.should.throw(/^Unsafe App require[\w\W]*example$/); }); + + it('does allow INTERNAL apps to require modules relatively outside their directory', function () { + var appBox = new AppSandbox({internal: true}), + badAppPath = path.join(__dirname, '..', 'utils', 'fixtures', 'app', 'badoutside.js'), + InternalApp, + loadApp = function () { + InternalApp = appBox.loadApp(badAppPath); + }; + + InternalApp = appBox.loadApp(badAppPath); + + loadApp.should.not.throw(/^Unsafe App require[\w\W]*example$/); + + InternalApp.should.be.a.Function(); + }); }); describe('Dependencies', function () { diff --git a/core/test/unit/config_spec.js b/core/test/unit/config_spec.js index b16c00491d..107cc94cb0 100644 --- a/core/test/unit/config_spec.js +++ b/core/test/unit/config_spec.js @@ -72,6 +72,7 @@ describe('Config', function () { 'corePath', 'themePath', 'appPath', + 'internalAppPath', 'imagesPath', 'imagesRelPath', 'adminViews',