From 4f3a8f6b38a004012341305339732c563a9cf343 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Thu, 17 Aug 2017 11:52:58 +0100 Subject: [PATCH] API express app routing & middleware improvements (#8883) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit no issue - Split routes out from the API app 🎨 - Use the same pattern as the blog app - General cleanup/unification across all of the `app.js` files - Split middleware config out from API routes - Logical groupings make it easier to see WTF is going on 😬 --- core/server/admin/app.js | 14 ++- core/server/api/app.js | 218 +--------------------------------- core/server/api/middleware.js | 34 ++++++ core/server/api/routes.js | 193 ++++++++++++++++++++++++++++++ core/server/app.js | 12 +- core/server/blog/app.js | 7 +- core/server/blog/routes.js | 18 ++- 7 files changed, 258 insertions(+), 238 deletions(-) create mode 100644 core/server/api/middleware.js create mode 100644 core/server/api/routes.js diff --git a/core/server/admin/app.js b/core/server/admin/app.js index 1da56c707c..dc761fb38e 100644 --- a/core/server/admin/app.js +++ b/core/server/admin/app.js @@ -1,17 +1,21 @@ var debug = require('ghost-ignition').debug('admin'), - config = require('../config'), express = require('express'), + + // App requires + config = require('../config'), + utils = require('../utils'), + + // Middleware // Admin only middleware adminMiddleware = require('./middleware'), + serveStatic = require('express').static, - // Global/shared middleware? + // Global/shared middleware cacheControl = require('../middleware/cache-control'), urlRedirects = require('../middleware/url-redirects'), errorHandler = require('../middleware/error-handler'), maintenance = require('../middleware/maintenance'), - prettyURLs = require('../middleware/pretty-urls'), - serveStatic = require('express').static, - utils = require('../utils'); + prettyURLs = require('../middleware/pretty-urls'); module.exports = function setupAdminApp() { debug('Admin setup start'); diff --git a/core/server/api/app.js b/core/server/api/app.js index 8065681ec9..491658b489 100644 --- a/core/server/api/app.js +++ b/core/server/api/app.js @@ -1,231 +1,21 @@ // # API routes var debug = require('ghost-ignition').debug('api'), express = require('express'), - tmpdir = require('os').tmpdir, - // This essentially provides the controllers for the routes - api = require('../api'), + // routes + routes = require('./routes'), // Include the middleware // API specific - auth = require('../auth'), - cors = require('../middleware/api/cors'), // routes only?! - brute = require('../middleware/brute'), // routes only versionMatch = require('../middleware/api/version-match'), // global - // Handling uploads & imports - upload = require('multer')({dest: tmpdir()}), // routes only - validation = require('../middleware/validation'), // routes only - // Shared bodyParser = require('body-parser'), // global, shared cacheControl = require('../middleware/cache-control'), // global, shared urlRedirects = require('../middleware/url-redirects'), - prettyURLs = require('../middleware/pretty-urls'), maintenance = require('../middleware/maintenance'), // global, shared - errorHandler = require('../middleware/error-handler'), // global, shared - - // Temporary - // @TODO find a more appy way to do this! - labs = require('../middleware/labs'), - - /** - * Authentication for public endpoints - * @TODO find a better way to bundle these authentication packages - * - * IMPORTANT - * - cors middleware MUST happen before pretty urls, because otherwise cors header can get lost - * - cors middleware MUST happen after authenticateClient, because authenticateClient reads the trusted domains - */ - authenticatePublic = [ - auth.authenticate.authenticateClient, - auth.authenticate.authenticateUser, - auth.authorize.requiresAuthorizedUserPublicAPI, - cors, - prettyURLs - ], - // Require user for private endpoints - authenticatePrivate = [ - auth.authenticate.authenticateClient, - auth.authenticate.authenticateUser, - auth.authorize.requiresAuthorizedUser, - cors, - prettyURLs - ]; - -// @TODO refactor/clean this up - how do we want the routing to work long term? -function apiRoutes() { - var apiRouter = express.Router(); - - // alias delete with del - apiRouter.del = apiRouter.delete; - - // ## CORS pre-flight check - apiRouter.options('*', cors); - - // ## Configuration - apiRouter.get('/configuration', api.http(api.configuration.read)); - apiRouter.get('/configuration/:key', authenticatePrivate, api.http(api.configuration.read)); - - // ## Posts - apiRouter.get('/posts', authenticatePublic, api.http(api.posts.browse)); - - apiRouter.post('/posts', authenticatePrivate, api.http(api.posts.add)); - apiRouter.get('/posts/:id', authenticatePublic, api.http(api.posts.read)); - apiRouter.get('/posts/slug/:slug', authenticatePublic, api.http(api.posts.read)); - apiRouter.put('/posts/:id', authenticatePrivate, api.http(api.posts.edit)); - apiRouter.del('/posts/:id', authenticatePrivate, api.http(api.posts.destroy)); - - // ## Schedules - apiRouter.put('/schedules/posts/:id', [ - auth.authenticate.authenticateClient, - auth.authenticate.authenticateUser - ], api.http(api.schedules.publishPost)); - - // ## Settings - apiRouter.get('/settings', authenticatePrivate, api.http(api.settings.browse)); - apiRouter.get('/settings/:key', authenticatePrivate, api.http(api.settings.read)); - apiRouter.put('/settings', authenticatePrivate, api.http(api.settings.edit)); - - // ## Users - apiRouter.get('/users', authenticatePublic, api.http(api.users.browse)); - apiRouter.get('/users/:id', authenticatePublic, api.http(api.users.read)); - apiRouter.get('/users/slug/:slug', authenticatePublic, api.http(api.users.read)); - apiRouter.get('/users/email/:email', authenticatePublic, api.http(api.users.read)); - - apiRouter.put('/users/password', authenticatePrivate, api.http(api.users.changePassword)); - apiRouter.put('/users/owner', authenticatePrivate, api.http(api.users.transferOwnership)); - apiRouter.put('/users/:id', authenticatePrivate, api.http(api.users.edit)); - - apiRouter.post('/users', authenticatePrivate, api.http(api.users.add)); - apiRouter.del('/users/:id', authenticatePrivate, api.http(api.users.destroy)); - - // ## Tags - apiRouter.get('/tags', authenticatePublic, api.http(api.tags.browse)); - apiRouter.get('/tags/:id', authenticatePublic, api.http(api.tags.read)); - apiRouter.get('/tags/slug/:slug', authenticatePublic, api.http(api.tags.read)); - apiRouter.post('/tags', authenticatePrivate, api.http(api.tags.add)); - apiRouter.put('/tags/:id', authenticatePrivate, api.http(api.tags.edit)); - apiRouter.del('/tags/:id', authenticatePrivate, api.http(api.tags.destroy)); - - // ## Subscribers - apiRouter.get('/subscribers', labs.subscribers, authenticatePrivate, api.http(api.subscribers.browse)); - apiRouter.get('/subscribers/csv', labs.subscribers, authenticatePrivate, api.http(api.subscribers.exportCSV)); - apiRouter.post('/subscribers/csv', - labs.subscribers, - authenticatePrivate, - upload.single('subscribersfile'), - validation.upload({type: 'subscribers'}), - api.http(api.subscribers.importCSV) - ); - apiRouter.get('/subscribers/:id', labs.subscribers, authenticatePrivate, api.http(api.subscribers.read)); - apiRouter.post('/subscribers', labs.subscribers, authenticatePublic, api.http(api.subscribers.add)); - apiRouter.put('/subscribers/:id', labs.subscribers, authenticatePrivate, api.http(api.subscribers.edit)); - apiRouter.del('/subscribers/:id', labs.subscribers, authenticatePrivate, api.http(api.subscribers.destroy)); - - // ## Roles - apiRouter.get('/roles/', authenticatePrivate, api.http(api.roles.browse)); - - // ## Clients - apiRouter.get('/clients/slug/:slug', api.http(api.clients.read)); - - // ## Slugs - apiRouter.get('/slugs/:type/:name', authenticatePrivate, api.http(api.slugs.generate)); - - // ## Themes - apiRouter.get('/themes/', authenticatePrivate, api.http(api.themes.browse)); - - apiRouter.get('/themes/:name/download', - authenticatePrivate, - api.http(api.themes.download) - ); - - apiRouter.post('/themes/upload', - authenticatePrivate, - upload.single('theme'), - validation.upload({type: 'themes'}), - api.http(api.themes.upload) - ); - - apiRouter.put('/themes/:name/activate', - authenticatePrivate, - api.http(api.themes.activate) - ); - - apiRouter.del('/themes/:name', - authenticatePrivate, - api.http(api.themes.destroy) - ); - - // ## Notifications - apiRouter.get('/notifications', authenticatePrivate, api.http(api.notifications.browse)); - apiRouter.post('/notifications', authenticatePrivate, api.http(api.notifications.add)); - apiRouter.del('/notifications/:id', authenticatePrivate, api.http(api.notifications.destroy)); - - // ## DB - apiRouter.get('/db', authenticatePrivate, api.http(api.db.exportContent)); - apiRouter.post('/db', - authenticatePrivate, - upload.single('importfile'), - validation.upload({type: 'db'}), - api.http(api.db.importContent) - ); - apiRouter.del('/db', authenticatePrivate, api.http(api.db.deleteAllContent)); - - // ## Mail - apiRouter.post('/mail', authenticatePrivate, api.http(api.mail.send)); - apiRouter.post('/mail/test', authenticatePrivate, api.http(api.mail.sendTest)); - - // ## Slack - apiRouter.post('/slack/test', authenticatePrivate, api.http(api.slack.sendTest)); - - // ## Authentication - apiRouter.post('/authentication/passwordreset', - brute.globalReset, - brute.userReset, - api.http(api.authentication.generateResetToken) - ); - apiRouter.put('/authentication/passwordreset', brute.globalBlock, api.http(api.authentication.resetPassword)); - apiRouter.post('/authentication/invitation', api.http(api.authentication.acceptInvitation)); - apiRouter.get('/authentication/invitation', api.http(api.authentication.isInvitation)); - apiRouter.post('/authentication/setup', api.http(api.authentication.setup)); - apiRouter.put('/authentication/setup', authenticatePrivate, api.http(api.authentication.updateSetup)); - apiRouter.get('/authentication/setup', api.http(api.authentication.isSetup)); - apiRouter.post('/authentication/token', - brute.globalBlock, - brute.userLogin, - auth.authenticate.authenticateClient, - auth.oauth.generateAccessToken - ); - - apiRouter.post('/authentication/revoke', authenticatePrivate, api.http(api.authentication.revoke)); - - // ## Uploads - // @TODO: rename endpoint to /images/upload (or similar) - apiRouter.post('/uploads', - authenticatePrivate, - upload.single('uploadimage'), - validation.upload({type: 'images'}), - api.http(api.uploads.add) - ); - - apiRouter.post('/uploads/icon', - authenticatePrivate, - upload.single('uploadimage'), - validation.upload({type: 'icons'}), - validation.blogIcon(), - api.http(api.uploads.add) - ); - - // ## Invites - apiRouter.get('/invites', authenticatePrivate, api.http(api.invites.browse)); - apiRouter.get('/invites/:id', authenticatePrivate, api.http(api.invites.read)); - apiRouter.post('/invites', authenticatePrivate, api.http(api.invites.add)); - apiRouter.del('/invites/:id', authenticatePrivate, api.http(api.invites.destroy)); - - return apiRouter; -} + errorHandler = require('../middleware/error-handler'); // global, shared module.exports = function setupApiApp() { debug('API setup start'); @@ -259,7 +49,7 @@ module.exports = function setupApiApp() { apiApp.use(cacheControl('private')); // Routing - apiApp.use(apiRoutes()); + apiApp.use(routes()); // API error handling apiApp.use(errorHandler.resourceNotFound); diff --git a/core/server/api/middleware.js b/core/server/api/middleware.js new file mode 100644 index 0000000000..cb6c9f763d --- /dev/null +++ b/core/server/api/middleware.js @@ -0,0 +1,34 @@ +var prettyURLs = require('../middleware/pretty-urls'), + cors = require('../middleware/api/cors'), + auth = require('../auth'); + +/** + * Auth Middleware Packages + * + * IMPORTANT + * - cors middleware MUST happen before pretty urls, because otherwise cors header can get lost + * - cors middleware MUST happen after authenticateClient, because authenticateClient reads the trusted domains + */ + +/** + * Authentication for public endpoints + */ +module.exports.authenticatePublic = [ + auth.authenticate.authenticateClient, + auth.authenticate.authenticateUser, + // This is a labs-enabled middleware + auth.authorize.requiresAuthorizedUserPublicAPI, + cors, + prettyURLs +]; + +/** + * Authentication for private endpoints + */ +module.exports.authenticatePrivate = [ + auth.authenticate.authenticateClient, + auth.authenticate.authenticateUser, + auth.authorize.requiresAuthorizedUser, + cors, + prettyURLs +]; diff --git a/core/server/api/routes.js b/core/server/api/routes.js new file mode 100644 index 0000000000..07fcb4bb21 --- /dev/null +++ b/core/server/api/routes.js @@ -0,0 +1,193 @@ +var express = require('express'), + // This essentially provides the controllers for the routes + api = require('../api'), + + // Middleware + mw = require('./middleware'), + + // API specific + auth = require('../auth'), + cors = require('../middleware/api/cors'), + brute = require('../middleware/brute'), + + // Handling uploads & imports + tmpdir = require('os').tmpdir, + upload = require('multer')({dest: tmpdir()}), + validation = require('../middleware/validation'), + + // Temporary + // @TODO find a more appy way to do this! + labs = require('../middleware/labs'); + +// @TODO refactor/clean this up - how do we want the routing to work long term? +module.exports = function apiRoutes() { + var apiRouter = express.Router(); + + // alias delete with del + apiRouter.del = apiRouter.delete; + + // ## CORS pre-flight check + apiRouter.options('*', cors); + + // ## Configuration + apiRouter.get('/configuration', api.http(api.configuration.read)); + apiRouter.get('/configuration/:key', mw.authenticatePrivate, api.http(api.configuration.read)); + + // ## Posts + apiRouter.get('/posts', mw.authenticatePublic, api.http(api.posts.browse)); + + apiRouter.post('/posts', mw.authenticatePrivate, api.http(api.posts.add)); + apiRouter.get('/posts/:id', mw.authenticatePublic, api.http(api.posts.read)); + apiRouter.get('/posts/slug/:slug', mw.authenticatePublic, api.http(api.posts.read)); + apiRouter.put('/posts/:id', mw.authenticatePrivate, api.http(api.posts.edit)); + apiRouter.del('/posts/:id', mw.authenticatePrivate, api.http(api.posts.destroy)); + + // ## Schedules + apiRouter.put('/schedules/posts/:id', [ + auth.authenticate.authenticateClient, + auth.authenticate.authenticateUser + ], api.http(api.schedules.publishPost)); + + // ## Settings + apiRouter.get('/settings', mw.authenticatePrivate, api.http(api.settings.browse)); + apiRouter.get('/settings/:key', mw.authenticatePrivate, api.http(api.settings.read)); + apiRouter.put('/settings', mw.authenticatePrivate, api.http(api.settings.edit)); + + // ## Users + apiRouter.get('/users', mw.authenticatePublic, api.http(api.users.browse)); + apiRouter.get('/users/:id', mw.authenticatePublic, api.http(api.users.read)); + apiRouter.get('/users/slug/:slug', mw.authenticatePublic, api.http(api.users.read)); + apiRouter.get('/users/email/:email', mw.authenticatePublic, api.http(api.users.read)); + + apiRouter.put('/users/password', mw.authenticatePrivate, api.http(api.users.changePassword)); + apiRouter.put('/users/owner', mw.authenticatePrivate, api.http(api.users.transferOwnership)); + apiRouter.put('/users/:id', mw.authenticatePrivate, api.http(api.users.edit)); + + apiRouter.post('/users', mw.authenticatePrivate, api.http(api.users.add)); + apiRouter.del('/users/:id', mw.authenticatePrivate, api.http(api.users.destroy)); + + // ## Tags + apiRouter.get('/tags', mw.authenticatePublic, api.http(api.tags.browse)); + apiRouter.get('/tags/:id', mw.authenticatePublic, api.http(api.tags.read)); + apiRouter.get('/tags/slug/:slug', mw.authenticatePublic, api.http(api.tags.read)); + apiRouter.post('/tags', mw.authenticatePrivate, api.http(api.tags.add)); + apiRouter.put('/tags/:id', mw.authenticatePrivate, api.http(api.tags.edit)); + apiRouter.del('/tags/:id', mw.authenticatePrivate, api.http(api.tags.destroy)); + + // ## Subscribers + apiRouter.get('/subscribers', labs.subscribers, mw.authenticatePrivate, api.http(api.subscribers.browse)); + apiRouter.get('/subscribers/csv', labs.subscribers, mw.authenticatePrivate, api.http(api.subscribers.exportCSV)); + apiRouter.post('/subscribers/csv', + labs.subscribers, + mw.authenticatePrivate, + upload.single('subscribersfile'), + validation.upload({type: 'subscribers'}), + api.http(api.subscribers.importCSV) + ); + apiRouter.get('/subscribers/:id', labs.subscribers, mw.authenticatePrivate, api.http(api.subscribers.read)); + apiRouter.post('/subscribers', labs.subscribers, mw.authenticatePublic, api.http(api.subscribers.add)); + apiRouter.put('/subscribers/:id', labs.subscribers, mw.authenticatePrivate, api.http(api.subscribers.edit)); + apiRouter.del('/subscribers/:id', labs.subscribers, mw.authenticatePrivate, api.http(api.subscribers.destroy)); + + // ## Roles + apiRouter.get('/roles/', mw.authenticatePrivate, api.http(api.roles.browse)); + + // ## Clients + apiRouter.get('/clients/slug/:slug', api.http(api.clients.read)); + + // ## Slugs + apiRouter.get('/slugs/:type/:name', mw.authenticatePrivate, api.http(api.slugs.generate)); + + // ## Themes + apiRouter.get('/themes/', mw.authenticatePrivate, api.http(api.themes.browse)); + + apiRouter.get('/themes/:name/download', + mw.authenticatePrivate, + api.http(api.themes.download) + ); + + apiRouter.post('/themes/upload', + mw.authenticatePrivate, + upload.single('theme'), + validation.upload({type: 'themes'}), + api.http(api.themes.upload) + ); + + apiRouter.put('/themes/:name/activate', + mw.authenticatePrivate, + api.http(api.themes.activate) + ); + + apiRouter.del('/themes/:name', + mw.authenticatePrivate, + api.http(api.themes.destroy) + ); + + // ## Notifications + apiRouter.get('/notifications', mw.authenticatePrivate, api.http(api.notifications.browse)); + apiRouter.post('/notifications', mw.authenticatePrivate, api.http(api.notifications.add)); + apiRouter.del('/notifications/:id', mw.authenticatePrivate, api.http(api.notifications.destroy)); + + // ## DB + apiRouter.get('/db', mw.authenticatePrivate, api.http(api.db.exportContent)); + apiRouter.post('/db', + mw.authenticatePrivate, + upload.single('importfile'), + validation.upload({type: 'db'}), + api.http(api.db.importContent) + ); + apiRouter.del('/db', mw.authenticatePrivate, api.http(api.db.deleteAllContent)); + + // ## Mail + apiRouter.post('/mail', mw.authenticatePrivate, api.http(api.mail.send)); + apiRouter.post('/mail/test', mw.authenticatePrivate, api.http(api.mail.sendTest)); + + // ## Slack + apiRouter.post('/slack/test', mw.authenticatePrivate, api.http(api.slack.sendTest)); + + // ## Authentication + apiRouter.post('/authentication/passwordreset', + brute.globalReset, + brute.userReset, + api.http(api.authentication.generateResetToken) + ); + apiRouter.put('/authentication/passwordreset', brute.globalBlock, api.http(api.authentication.resetPassword)); + apiRouter.post('/authentication/invitation', api.http(api.authentication.acceptInvitation)); + apiRouter.get('/authentication/invitation', api.http(api.authentication.isInvitation)); + apiRouter.post('/authentication/setup', api.http(api.authentication.setup)); + apiRouter.put('/authentication/setup', mw.authenticatePrivate, api.http(api.authentication.updateSetup)); + apiRouter.get('/authentication/setup', api.http(api.authentication.isSetup)); + apiRouter.post('/authentication/token', + brute.globalBlock, + brute.userLogin, + auth.authenticate.authenticateClient, + auth.oauth.generateAccessToken + ); + + apiRouter.post('/authentication/revoke', mw.authenticatePrivate, api.http(api.authentication.revoke)); + + // ## Uploads + // @TODO: rename endpoint to /images/upload (or similar) + apiRouter.post('/uploads', + mw.authenticatePrivate, + upload.single('uploadimage'), + validation.upload({type: 'images'}), + api.http(api.uploads.add) + ); + + apiRouter.post('/uploads/icon', + mw.authenticatePrivate, + upload.single('uploadimage'), + validation.upload({type: 'icons'}), + validation.blogIcon(), + api.http(api.uploads.add) + ); + + // ## Invites + apiRouter.get('/invites', mw.authenticatePrivate, api.http(api.invites.browse)); + apiRouter.get('/invites/:id', mw.authenticatePrivate, api.http(api.invites.read)); + apiRouter.post('/invites', mw.authenticatePrivate, api.http(api.invites.add)); + apiRouter.del('/invites/:id', mw.authenticatePrivate, api.http(api.invites.destroy)); + + return apiRouter; +}; diff --git a/core/server/app.js b/core/server/app.js index 227e388165..c60bd9353b 100644 --- a/core/server/app.js +++ b/core/server/app.js @@ -1,16 +1,16 @@ var debug = require('ghost-ignition').debug('app'), express = require('express'), - // app requires - config = require('./config'), + // App requires + config = require('./config'), // middleware - compress = require('compression'), - netjet = require('netjet'), + compress = require('compression'), + netjet = require('netjet'), // local middleware - ghostLocals = require('./middleware/ghost-locals'), - logRequest = require('./middleware/log-request'); + ghostLocals = require('./middleware/ghost-locals'), + logRequest = require('./middleware/log-request'); module.exports = function setupParentApp() { debug('ParentApp setup start'); diff --git a/core/server/blog/app.js b/core/server/blog/app.js index 9aac43079c..5c08d64fa2 100644 --- a/core/server/blog/app.js +++ b/core/server/blog/app.js @@ -1,5 +1,6 @@ var debug = require('ghost-ignition').debug('blog'), path = require('path'), + express = require('express'), // App requires config = require('../config'), @@ -12,12 +13,14 @@ var debug = require('ghost-ignition').debug('blog'), // routes routes = require('./routes'), - // local middleware + // Global/shared middleware cacheControl = require('../middleware/cache-control'), urlRedirects = require('../middleware/url-redirects'), errorHandler = require('../middleware/error-handler'), maintenance = require('../middleware/maintenance'), prettyURLs = require('../middleware/pretty-urls'), + + // local middleware servePublicFile = require('../middleware/serve-public-file'), staticTheme = require('../middleware/static-theme'), customRedirects = require('../middleware/custom-redirects'), @@ -29,7 +32,7 @@ var debug = require('ghost-ignition').debug('blog'), module.exports = function setupBlogApp() { debug('Blog setup start'); - var blogApp = require('express')(); + var blogApp = express(); // ## App - specific code // set the view engine diff --git a/core/server/blog/routes.js b/core/server/blog/routes.js index d3d4aa38d2..8d332453d5 100644 --- a/core/server/blog/routes.js +++ b/core/server/blog/routes.js @@ -1,13 +1,11 @@ -var express = require('express'), - path = require('path'), - config = require('../config'), - frontend = require('../controllers/frontend'), - channels = require('../controllers/frontend/channels'), - utils = require('../utils'), +var express = require('express'), + path = require('path'), + config = require('../config'), + frontend = require('../controllers/frontend'), + channels = require('../controllers/frontend/channels'), + utils = require('../utils'); - frontendRoutes; - -frontendRoutes = function frontendRoutes() { +module.exports = function frontendRoutes() { var router = express.Router(), routeKeywords = config.get('routeKeywords'); @@ -44,5 +42,3 @@ frontendRoutes = function frontendRoutes() { return router; }; - -module.exports = frontendRoutes;