diff --git a/install/package.json b/install/package.json index 26177af029..0387ae1cb2 100644 --- a/install/package.json +++ b/install/package.json @@ -45,7 +45,6 @@ "commander": "^7.1.0", "compare-versions": "3.6.0", "compression": "^1.7.4", - "connect-ensure-login": "^0.1.1", "connect-flash": "^0.1.1", "connect-mongo": "4.3.0", "connect-multiparty": "^2.2.0", diff --git a/src/controllers/index.js b/src/controllers/index.js index 2ff79c0627..324d8609ed 100644 --- a/src/controllers/index.js +++ b/src/controllers/index.js @@ -109,7 +109,7 @@ Controllers.login = async function (req, res) { req.session.returnTo = req.headers['x-return-to']; } - // Occasionally, x-return-to is passed a full url. Also, connect-ensure-login passes the relative path. Strip both. + // Occasionally, x-return-to is passed a full url. req.session.returnTo = req.session.returnTo && req.session.returnTo.replace(nconf.get('base_url'), '').replace(nconf.get('relative_path'), ''); data.alternate_logins = loginStrategies.length > 0; diff --git a/src/middleware/index.js b/src/middleware/index.js index 6590477306..f2487ca939 100644 --- a/src/middleware/index.js +++ b/src/middleware/index.js @@ -5,7 +5,6 @@ const path = require('path'); const csrf = require('csurf'); const validator = require('validator'); const nconf = require('nconf'); -const ensureLoggedIn = require('connect-ensure-login'); const toobusy = require('toobusy-js'); const LRU = require('lru-cache'); const util = require('util'); @@ -52,7 +51,13 @@ middleware.applyCSRF = function (req, res, next) { }; middleware.applyCSRFasync = util.promisify(middleware.applyCSRF); -middleware.ensureLoggedIn = ensureLoggedIn.ensureLoggedIn(`${relative_path}/login`); +middleware.ensureLoggedIn = (req, res, next) => { + if (!req.loggedIn) { + return controllers.helpers.notAllowed(req, res); + } + + setImmediate(next); +}; Object.assign(middleware, { admin: require('./admin'), diff --git a/src/middleware/user.js b/src/middleware/user.js index 1df25e7995..e71c2f3c82 100644 --- a/src/middleware/user.js +++ b/src/middleware/user.js @@ -83,6 +83,7 @@ module.exports = function (middleware) { } middleware.authenticate = helpers.try(async (req, res, next) => { + winston.warn(`[middleware] middleware.authenticate has been deprecated, page and API routes are now automatically authenticated via setup(Page|API)Route. Use middleware.authenticateRequest (if not using route helper) and middleware.ensureLoggedIn instead. (request path: ${req.path})`); if (!await authenticate(req, res)) { return; } @@ -92,7 +93,7 @@ module.exports = function (middleware) { next(); }); - middleware.authenticateOrGuest = helpers.try(async (req, res, next) => { + middleware.authenticateRequest = helpers.try(async (req, res, next) => { if (!await authenticate(req, res)) { return; } diff --git a/src/routes/admin.js b/src/routes/admin.js index fb7a96e370..97ecb66d7e 100644 --- a/src/routes/admin.js +++ b/src/routes/admin.js @@ -61,15 +61,15 @@ module.exports = function (app, name, middleware, controllers) { function apiRoutes(router, name, middleware, controllers) { - router.get(`/api/${name}/users/csv`, middleware.authenticate, helpers.tryRoute(controllers.admin.users.getCSV)); - router.get(`/api/${name}/groups/:groupname/csv`, middleware.authenticate, helpers.tryRoute(controllers.admin.groups.getCSV)); - router.get(`/api/${name}/analytics`, middleware.authenticate, helpers.tryRoute(controllers.admin.dashboard.getAnalytics)); - router.get(`/api/${name}/advanced/cache/dump`, middleware.authenticate, helpers.tryRoute(controllers.admin.cache.dump)); + router.get(`/api/${name}/users/csv`, middleware.ensureLoggedIn, helpers.tryRoute(controllers.admin.users.getCSV)); + router.get(`/api/${name}/groups/:groupname/csv`, middleware.ensureLoggedIn, helpers.tryRoute(controllers.admin.groups.getCSV)); + router.get(`/api/${name}/analytics`, middleware.ensureLoggedIn, helpers.tryRoute(controllers.admin.dashboard.getAnalytics)); + router.get(`/api/${name}/advanced/cache/dump`, middleware.ensureLoggedIn, helpers.tryRoute(controllers.admin.cache.dump)); const multipart = require('connect-multiparty'); const multipartMiddleware = multipart(); - const middlewares = [multipartMiddleware, middleware.validateFiles, middleware.applyCSRF, middleware.authenticate]; + const middlewares = [multipartMiddleware, middleware.validateFiles, middleware.applyCSRF, middleware.ensureLoggedIn]; router.post(`/api/${name}/category/uploadpicture`, middlewares, helpers.tryRoute(controllers.admin.uploads.uploadCategoryPicture)); router.post(`/api/${name}/uploadfavicon`, middlewares, helpers.tryRoute(controllers.admin.uploads.uploadFavicon)); diff --git a/src/routes/api.js b/src/routes/api.js index e0b2694cf9..413f603c31 100644 --- a/src/routes/api.js +++ b/src/routes/api.js @@ -8,7 +8,7 @@ module.exports = function (app, middleware, controllers) { const router = express.Router(); app.use('/api', router); - router.get('/config', middleware.applyCSRF, middleware.authenticateOrGuest, controllers.api.getConfig); + router.get('/config', middleware.applyCSRF, middleware.authenticateRequest, controllers.api.getConfig); router.get('/self', controllers.user.getCurrentUser); router.get('/user/uid/:uid', middleware.canViewUsers, controllers.user.getUserByUID); @@ -21,7 +21,7 @@ module.exports = function (app, middleware, controllers) { router.get('/categories/:cid/moderators', controllers.api.getModerators); router.get('/recent/posts/:term?', controllers.posts.getRecentPosts); - router.get('/unread/total', middleware.authenticate, controllers.unread.unreadTotal); + router.get('/unread/total', middleware.authenticateRequest, middleware.ensureLoggedIn, controllers.unread.unreadTotal); router.get('/topic/teaser/:topic_id', controllers.topics.teaser); router.get('/topic/pagination/:topic_id', controllers.topics.pagination); @@ -30,5 +30,5 @@ module.exports = function (app, middleware, controllers) { const middlewares = [middleware.maintenanceMode, multipartMiddleware, middleware.validateFiles, middleware.applyCSRF]; router.post('/post/upload', middlewares, uploadsController.uploadPost); - router.post('/user/:userslug/uploadpicture', middlewares.concat([middleware.exposeUid, middleware.authenticate, middleware.canViewUsers, middleware.checkAccountPermissions]), controllers.accounts.edit.uploadPicture); + router.post('/user/:userslug/uploadpicture', middlewares.concat([middleware.exposeUid, middleware.authenticateRequest, middleware.ensureLoggedIn, middleware.canViewUsers, middleware.checkAccountPermissions]), controllers.accounts.edit.uploadPicture); }; diff --git a/src/routes/helpers.js b/src/routes/helpers.js index f9fd7ba47e..75ce628658 100644 --- a/src/routes/helpers.js +++ b/src/routes/helpers.js @@ -1,15 +1,17 @@ 'use strict'; const helpers = module.exports; +const middleware = require('../middleware'); const controllerHelpers = require('../controllers/helpers'); helpers.setupPageRoute = function (router, name, middleware, middlewares, controller) { middlewares = [ middleware.maintenanceMode, middleware.registrationComplete, - middleware.pageView, + middleware.authenticateRequest, middleware.pluginHooks, - ].concat(middlewares); + middleware.pageView, + ...middlewares]; router.get( name, @@ -28,6 +30,14 @@ helpers.setupAdminPageRoute = function (router, name, middleware, middlewares, c }; helpers.setupApiRoute = function (router, verb, name, middlewares, controller) { + middlewares = [ + middleware.maintenanceMode, + middleware.registrationComplete, + middleware.authenticateRequest, + middleware.pluginHooks, + ...middlewares, + ]; + router[verb](name, middlewares, helpers.tryRoute(controller, (err, res) => { controllerHelpers.formatApiResponse(400, res, err); })); diff --git a/src/routes/index.js b/src/routes/index.js index 1b804013e4..550eb94b2c 100644 --- a/src/routes/index.js +++ b/src/routes/index.js @@ -75,7 +75,7 @@ _mounts.category = (app, name, middleware, controllers) => { setupPageRoute(app, '/popular', middleware, [], controllers.popular.get); setupPageRoute(app, '/recent', middleware, [], controllers.recent.get); setupPageRoute(app, '/top', middleware, [], controllers.top.get); - setupPageRoute(app, '/unread', middleware, [middleware.authenticate], controllers.unread.get); + setupPageRoute(app, '/unread', middleware, [middleware.ensureLoggedIn], controllers.unread.get); setupPageRoute(app, `/${name}/:category_id/:slug/:topic_index`, middleware, [], controllers.category.get); setupPageRoute(app, `/${name}/:category_id/:slug?`, middleware, [], controllers.category.get); @@ -100,7 +100,6 @@ module.exports = async function (app, middleware) { router.render = function (...args) { app.render(...args); }; - const ensureLoggedIn = require('connect-ensure-login'); // Allow plugins/themes to mount some routes elsewhere const remountable = ['admin', 'category', 'topic', 'post', 'users', 'user', 'groups', 'tags']; @@ -125,8 +124,8 @@ module.exports = async function (app, middleware) { }); router.all('(/+api|/+api/*?)', middleware.prepareAPI); - router.all(`(/+api/admin|/+api/admin/*?${mounts.admin !== 'admin' ? `|/+api/${mounts.admin}|/+api/${mounts.admin}/*?` : ''})`, middleware.authenticate, middleware.admin.checkPrivileges); - router.all(`(/+admin|/+admin/*?${mounts.admin !== 'admin' ? `|/+${mounts.admin}|/+${mounts.admin}/*?` : ''})`, ensureLoggedIn.ensureLoggedIn(`${nconf.get('relative_path')}/login?local=1`), middleware.applyCSRF, middleware.admin.checkPrivileges); + router.all(`(/+api/admin|/+api/admin/*?${mounts.admin !== 'admin' ? `|/+api/${mounts.admin}|/+api/${mounts.admin}/*?` : ''})`, middleware.authenticateRequest, middleware.ensureLoggedIn, middleware.admin.checkPrivileges); + router.all(`(/+admin|/+admin/*?${mounts.admin !== 'admin' ? `|/+${mounts.admin}|/+${mounts.admin}/*?` : ''})`, middleware.ensureLoggedIn, middleware.applyCSRF, middleware.admin.checkPrivileges); app.use(middleware.stripLeadingSlashes); diff --git a/src/routes/user.js b/src/routes/user.js index 809aa9a83c..d11666cb2b 100644 --- a/src/routes/user.js +++ b/src/routes/user.js @@ -52,7 +52,7 @@ module.exports = function (app, name, middleware, controllers) { res.redirect(`${nconf.get('relative_path')}/api/v3/users/${res.locals.uid}/sessions/${req.params.uuid}`); }); - setupPageRoute(app, '/notifications', middleware, [middleware.authenticate], controllers.accounts.notifications.get); + setupPageRoute(app, '/notifications', middleware, [middleware.ensureLoggedIn], controllers.accounts.notifications.get); setupPageRoute(app, `/${name}/:userslug/chats/:roomid?`, middleware, middlewares, controllers.accounts.chats.get); - setupPageRoute(app, '/chats/:roomid?', middleware, [middleware.authenticate], controllers.accounts.chats.redirectToChat); + setupPageRoute(app, '/chats/:roomid?', middleware, [middleware.ensureLoggedIn], controllers.accounts.chats.redirectToChat); }; diff --git a/src/routes/write/admin.js b/src/routes/write/admin.js index d70acc6654..96cb386beb 100644 --- a/src/routes/write/admin.js +++ b/src/routes/write/admin.js @@ -8,7 +8,7 @@ const routeHelpers = require('../helpers'); const { setupApiRoute } = routeHelpers; module.exports = function () { - const middlewares = [middleware.authenticate]; + const middlewares = [middleware.ensureLoggedIn]; setupApiRoute(router, 'put', '/settings/:setting', [...middlewares, middleware.checkRequired.bind(null, ['value'])], controllers.write.admin.updateSetting); diff --git a/src/routes/write/categories.js b/src/routes/write/categories.js index 5e4dc45dae..db8492dba4 100644 --- a/src/routes/write/categories.js +++ b/src/routes/write/categories.js @@ -8,10 +8,10 @@ const routeHelpers = require('../helpers'); const { setupApiRoute } = routeHelpers; module.exports = function () { - const middlewares = [middleware.authenticate]; + const middlewares = [middleware.ensureLoggedIn]; setupApiRoute(router, 'post', '/', [...middlewares, middleware.checkRequired.bind(null, ['name'])], controllers.write.categories.create); - setupApiRoute(router, 'get', '/:cid', [middleware.authenticateOrGuest], controllers.write.categories.get); + setupApiRoute(router, 'get', '/:cid', [], controllers.write.categories.get); setupApiRoute(router, 'put', '/:cid', [...middlewares], controllers.write.categories.update); setupApiRoute(router, 'delete', '/:cid', [...middlewares], controllers.write.categories.delete); diff --git a/src/routes/write/files.js b/src/routes/write/files.js index 9dd8f0d30d..f0d2aab037 100644 --- a/src/routes/write/files.js +++ b/src/routes/write/files.js @@ -8,7 +8,7 @@ const routeHelpers = require('../helpers'); const { setupApiRoute } = routeHelpers; module.exports = function () { - const middlewares = [middleware.authenticate]; + const middlewares = [middleware.ensureLoggedIn]; // setupApiRoute(router, 'put', '/', [ // ...middlewares, diff --git a/src/routes/write/groups.js b/src/routes/write/groups.js index 676f587f9b..0245690fcb 100644 --- a/src/routes/write/groups.js +++ b/src/routes/write/groups.js @@ -8,7 +8,7 @@ const routeHelpers = require('../helpers'); const { setupApiRoute } = routeHelpers; module.exports = function () { - const middlewares = [middleware.authenticate]; + const middlewares = [middleware.ensureLoggedIn]; setupApiRoute(router, 'post', '/', [...middlewares, middleware.checkRequired.bind(null, ['name'])], controllers.write.groups.create); setupApiRoute(router, 'head', '/:slug', [middleware.assert.group], controllers.write.groups.exists); diff --git a/src/routes/write/index.js b/src/routes/write/index.js index d4679f7667..be236e1355 100644 --- a/src/routes/write/index.js +++ b/src/routes/write/index.js @@ -42,7 +42,7 @@ Write.reload = async (params) => { router.use('/api/v3/utilities', require('./utilities')()); router.get('/api/v3/ping', writeControllers.utilities.ping.get); - router.post('/api/v3/ping', middleware.authenticate, writeControllers.utilities.ping.post); + router.post('/api/v3/ping', middleware.authenticateRequest, middleware.ensureLoggedIn, writeControllers.utilities.ping.post); /** * Plugins can add routes to the Write API by attaching a listener to the diff --git a/src/routes/write/posts.js b/src/routes/write/posts.js index 967d1b6a0f..b6831890a0 100644 --- a/src/routes/write/posts.js +++ b/src/routes/write/posts.js @@ -8,9 +8,9 @@ const routeHelpers = require('../helpers'); const { setupApiRoute } = routeHelpers; module.exports = function () { - const middlewares = [middleware.authenticate]; + const middlewares = [middleware.ensureLoggedIn]; - setupApiRoute(router, 'get', '/:pid', [middleware.authenticateOrGuest], controllers.write.posts.get); + setupApiRoute(router, 'get', '/:pid', [], controllers.write.posts.get); // There is no POST route because you POST to a topic to create a new post. Intuitive, no? setupApiRoute(router, 'put', '/:pid', [...middlewares, middleware.checkRequired.bind(null, ['content'])], controllers.write.posts.edit); setupApiRoute(router, 'delete', '/:pid', [...middlewares, middleware.assert.post], controllers.write.posts.purge); @@ -26,8 +26,8 @@ module.exports = function () { setupApiRoute(router, 'put', '/:pid/bookmark', [...middlewares, middleware.assert.post], controllers.write.posts.bookmark); setupApiRoute(router, 'delete', '/:pid/bookmark', [...middlewares, middleware.assert.post], controllers.write.posts.unbookmark); - setupApiRoute(router, 'get', '/:pid/diffs', [middleware.authenticateOrGuest, middleware.assert.post], controllers.write.posts.getDiffs); - setupApiRoute(router, 'get', '/:pid/diffs/:since', [middleware.authenticateOrGuest, middleware.assert.post], controllers.write.posts.loadDiff); + setupApiRoute(router, 'get', '/:pid/diffs', [middleware.assert.post], controllers.write.posts.getDiffs); + setupApiRoute(router, 'get', '/:pid/diffs/:since', [middleware.assert.post], controllers.write.posts.loadDiff); setupApiRoute(router, 'put', '/:pid/diffs/:since', [...middlewares, middleware.assert.post], controllers.write.posts.restoreDiff); setupApiRoute(router, 'delete', '/:pid/diffs/:timestamp', [...middlewares, middleware.assert.post], controllers.write.posts.deleteDiff); diff --git a/src/routes/write/topics.js b/src/routes/write/topics.js index 41851e6eb0..4b8d41d293 100644 --- a/src/routes/write/topics.js +++ b/src/routes/write/topics.js @@ -8,14 +8,14 @@ const routeHelpers = require('../helpers'); const { setupApiRoute } = routeHelpers; module.exports = function () { - const middlewares = [middleware.authenticate]; + const middlewares = [middleware.ensureLoggedIn]; const multipart = require('connect-multiparty'); const multipartMiddleware = multipart(); - setupApiRoute(router, 'post', '/', [middleware.authenticateOrGuest, middleware.checkRequired.bind(null, ['cid', 'title', 'content'])], controllers.write.topics.create); - setupApiRoute(router, 'get', '/:tid', [middleware.authenticateOrGuest], controllers.write.topics.get); - setupApiRoute(router, 'post', '/:tid', [middleware.authenticateOrGuest, middleware.checkRequired.bind(null, ['content']), middleware.assert.topic], controllers.write.topics.reply); + setupApiRoute(router, 'post', '/', [middleware.checkRequired.bind(null, ['cid', 'title', 'content'])], controllers.write.topics.create); + setupApiRoute(router, 'get', '/:tid', [], controllers.write.topics.get); + setupApiRoute(router, 'post', '/:tid', [middleware.checkRequired.bind(null, ['content']), middleware.assert.topic], controllers.write.topics.reply); setupApiRoute(router, 'delete', '/:tid', [...middlewares], controllers.write.topics.purge); setupApiRoute(router, 'put', '/:tid/state', [...middlewares], controllers.write.topics.restore); @@ -35,13 +35,13 @@ module.exports = function () { setupApiRoute(router, 'put', '/:tid/tags', [...middlewares, middleware.checkRequired.bind(null, ['tags']), middleware.assert.topic], controllers.write.topics.addTags); setupApiRoute(router, 'delete', '/:tid/tags', [...middlewares, middleware.assert.topic], controllers.write.topics.deleteTags); - setupApiRoute(router, 'get', '/:tid/thumbs', middleware.authenticateOrGuest, controllers.write.topics.getThumbs); + setupApiRoute(router, 'get', '/:tid/thumbs', [], controllers.write.topics.getThumbs); setupApiRoute(router, 'post', '/:tid/thumbs', [multipartMiddleware, middleware.validateFiles, ...middlewares], controllers.write.topics.addThumb); setupApiRoute(router, 'put', '/:tid/thumbs', [...middlewares, middleware.checkRequired.bind(null, ['tid'])], controllers.write.topics.migrateThumbs); setupApiRoute(router, 'delete', '/:tid/thumbs', [...middlewares, middleware.checkRequired.bind(null, ['path'])], controllers.write.topics.deleteThumb); setupApiRoute(router, 'put', '/:tid/thumbs/order', [...middlewares, middleware.checkRequired.bind(null, ['path', 'order'])], controllers.write.topics.reorderThumbs); - setupApiRoute(router, 'get', '/:tid/events', [middleware.authenticateOrGuest, middleware.assert.topic], controllers.write.topics.getEvents); + setupApiRoute(router, 'get', '/:tid/events', [middleware.assert.topic], controllers.write.topics.getEvents); return router; }; diff --git a/src/routes/write/users.js b/src/routes/write/users.js index 0138a9c1da..118d8ba27e 100644 --- a/src/routes/write/users.js +++ b/src/routes/write/users.js @@ -13,7 +13,7 @@ function guestRoutes() { } function authenticatedRoutes() { - const middlewares = [middleware.authenticate]; + const middlewares = [middleware.ensureLoggedIn]; setupApiRoute(router, 'post', '/', [...middlewares, middleware.checkRequired.bind(null, ['username'])], controllers.write.users.create); setupApiRoute(router, 'delete', '/', [...middlewares, middleware.checkRequired.bind(null, ['uids'])], controllers.write.users.deleteMany);