feat: add buildHeaderAsync (#8367)

* feat: add buildHeaderAsync

make helphers.notAllowed async

* fix: remove csrf from buildHeader

* fix: remove unused method, use middleware

* fix: /post/pid redirect doesn't need buildHeader

use buildHeaderAsync

src/controllers/404.js

@@ -6,6 +6,7 @@ const validator = require('validator');
 
 const meta = require('../meta');
 const plugins = require('../plugins');
+const middleware = require('../middleware');
 
 exports.handle404 = function handle404(req, res) {
 	const relativePath = nconf.get('relative_path');
@@ -36,14 +37,12 @@ exports.handle404 = function handle404(req, res) {
 	}
 };
 
-exports.send404 = function (req, res) {
+exports.send404 = async function (req, res) {
 	res.status(404);
 	const path = String(req.path || '');
 	if (res.locals.isAPI) {
 		return res.json({ path: validator.escape(path.replace(/^\/api/, '')), title: '[[global:404.title]]' });
 	}
-	const middleware = require('../middleware');
+	await middleware.buildHeaderAsync(req, res);
-	middleware.buildHeader(req, res, function () {
+	res.render('404', { path: validator.escape(path), title: '[[global:404.title]]' });
-		res.render('404', { path: validator.escape(path), title: '[[global:404.title]]' });
-	});
 };

src/controllers/errors.js

@@ -4,8 +4,9 @@ var nconf = require('nconf');
 var winston = require('winston');
 var validator = require('validator');
 var plugins = require('../plugins');
+var middleware = require('../middleware');
 
-exports.handleURIErrors = function handleURIErrors(err, req, res, next) {
+exports.handleURIErrors = async function handleURIErrors(err, req, res, next) {
 	// Handle cases where malformed URIs are passed in
 	if (err instanceof URIError) {
 		const cleanPath = req.path.replace(new RegExp('^' + nconf.get('relative_path')), '');
@@ -23,10 +24,8 @@ exports.handleURIErrors = function handleURIErrors(err, req, res, next) {
 					error: '[[global:400.title]]',
 				});
 			} else {
-				var middleware = require('../middleware');
+				await middleware.buildHeaderAsync(req, res);
-				middleware.buildHeader(req, res, function () {
+				res.status(400).render('400', { error: validator.escape(String(err.message)) });
-					res.status(400).render('400', { error: validator.escape(String(err.message)) });
-				});
 			}
 		}
 	} else {
@@ -46,7 +45,7 @@ exports.handleErrors = function handleErrors(err, req, res, next) { // eslint-di
 			res.status(403).type('text/plain').send(err.message);
 		},
 	};
-	var defaultHandler = function () {
+	var defaultHandler = async function () {
 		// Display NodeBB error page
 		var status = parseInt(err.status, 10);
 		if ((status === 302 || status === 308) && err.path) {
@@ -61,10 +60,8 @@ exports.handleErrors = function handleErrors(err, req, res, next) { // eslint-di
 		if (res.locals.isAPI) {
 			res.json({ path: validator.escape(path), error: err.message });
 		} else {
-			var middleware = require('../middleware');
+			await middleware.buildHeaderAsync(req, res);
-			middleware.buildHeader(req, res, function () {
+			res.render('500', { path: validator.escape(path), error: validator.escape(String(err.message)) });
-				res.render('500', { path: validator.escape(path), error: validator.escape(String(err.message)) });
-			});
 		}
 	};
 

src/controllers/helpers.js

@@ -2,7 +2,6 @@
 
 const nconf = require('nconf');
 const validator = require('validator');
-const winston = require('winston');
 const querystring = require('querystring');
 const _ = require('lodash');
 
@@ -15,21 +14,19 @@ const middleware = require('../middleware');
 
 const helpers = module.exports;
 
-helpers.noScriptErrors = function (req, res, error, httpStatus) {
+helpers.noScriptErrors = async function (req, res, error, httpStatus) {
 	if (req.body.noscript !== 'true') {
 		return res.status(httpStatus).send(error);
 	}
 
-	const middleware = require('../middleware');
 	const httpStatusString = httpStatus.toString();
-	middleware.buildHeader(req, res, function () {
+	await middleware.buildHeaderAsync(req, res);
-		res.status(httpStatus).render(httpStatusString, {
+	res.status(httpStatus).render(httpStatusString, {
-			path: req.path,
+		path: req.path,
-			loggedIn: req.loggedIn,
+		loggedIn: req.loggedIn,
-			error: error,
+		error: error,
-			returnLink: true,
+		returnLink: true,
-			title: '[[global:' + httpStatusString + '.title]]',
+		title: '[[global:' + httpStatusString + '.title]]',
-		});
 	});
 };
 
@@ -104,41 +101,37 @@ helpers.buildTerms = function (url, term, query) {
 	}];
 };
 
-helpers.notAllowed = function (req, res, error) {
+helpers.notAllowed = async function (req, res, error) {
-	plugins.fireHook('filter:helpers.notAllowed', {
+	const data = await plugins.fireHook('filter:helpers.notAllowed', {
 		req: req,
 		res: res,
 		error: error,
-	}, function (err) {
-		if (err) {
-			return winston.error(err);
-		}
-		if (req.loggedIn || req.uid === -1) {
-			if (res.locals.isAPI) {
-				res.status(403).json({
-					path: req.path.replace(/^\/api/, ''),
-					loggedIn: req.loggedIn,
-					error: error,
-					title: '[[global:403.title]]',
-				});
-			} else {
-				middleware.buildHeader(req, res, function () {
-					res.status(403).render('403', {
-						path: req.path,
-						loggedIn: req.loggedIn,
-						error: error,
-						title: '[[global:403.title]]',
-					});
-				});
-			}
-		} else if (res.locals.isAPI) {
-			req.session.returnTo = req.url.replace(/^\/api/, '');
-			res.status(401).json('not-authorized');
-		} else {
-			req.session.returnTo = req.url;
-			res.redirect(nconf.get('relative_path') + '/login');
-		}
 	});
+
+	if (req.loggedIn || req.uid === -1) {
+		if (res.locals.isAPI) {
+			res.status(403).json({
+				path: req.path.replace(/^\/api/, ''),
+				loggedIn: req.loggedIn,
+				error: data.error,
+				title: '[[global:403.title]]',
+			});
+		} else {
+			await middleware.buildHeaderAsync(req, res);
+			res.status(403).render('403', {
+				path: req.path,
+				loggedIn: req.loggedIn,
+				error: data.error,
+				title: '[[global:403.title]]',
+			});
+		}
+	} else if (res.locals.isAPI) {
+		req.session.returnTo = req.url.replace(/^\/api/, '');
+		res.status(401).json('not-authorized');
+	} else {
+		req.session.returnTo = req.url;
+		res.redirect(nconf.get('relative_path') + '/login');
+	}
 };
 
 helpers.redirect = function (res, url) {

src/middleware/header.js

@@ -3,6 +3,7 @@
 var nconf = require('nconf');
 var jsesc = require('jsesc');
 var _ = require('lodash');
+var util = require('util');
 
 var db = require('../database');
 var user = require('../user');
@@ -26,9 +27,6 @@ module.exports = function (middleware) {
 	middleware.buildHeader = helpers.try(async function buildHeader(req, res, next) {
 		res.locals.renderHeader = true;
 		res.locals.isAPI = false;
-		if (req.uid >= 0) {
-			await middleware.applyCSRFAsync(req, res);
-		}
 		const [config] = await Promise.all([
 			controllers.api.loadConfig(req),
 			plugins.fireHook('filter:middleware.buildHeader', { req: req, locals: res.locals }),
@@ -37,6 +35,8 @@ module.exports = function (middleware) {
 		next();
 	});
 
+	middleware.buildHeaderAsync = util.promisify(middleware.buildHeader);
+
 	async function generateHeader(req, res, data) {
 		var registrationType = meta.config.registrationType || 'normal';
 		res.locals.config = res.locals.config || {};

src/middleware/index.js

@@ -33,7 +33,7 @@ middleware.regexes = {
 	timestampedUpload: /^\d+-.+$/,
 };
 
-middleware.applyCSRF = csrf({
+const csurfMiddleware = csrf({
 	cookie: nconf.get('url_parsed').protocol === 'https:' ? {
 		secure: true,
 		sameSite: 'Strict',
@@ -41,7 +41,13 @@ middleware.applyCSRF = csrf({
 	} : true,
 });
 
-middleware.applyCSRFAsync = util.promisify(middleware.applyCSRF);
+middleware.applyCSRF = function (req, res, next) {
+	if (req.uid >= 0) {
+		csurfMiddleware(req, res, next);
+	} else {
+		next();
+	}
+};
 
 middleware.ensureLoggedIn = ensureLoggedIn.ensureLoggedIn(nconf.get('relative_path') + '/login');
 

src/middleware/maintenance.js

@@ -35,8 +35,7 @@ module.exports = function (middleware) {
 		if (res.locals.isAPI) {
 			return res.json(data);
 		}
-		const buildHeaderAsync = util.promisify(middleware.buildHeader);
+		await middleware.buildHeaderAsync(req, res);
-		await buildHeaderAsync(req, res);
 		res.render('503', data);
 	});
 };

src/routes/api.js

@@ -8,13 +8,7 @@ module.exports = function (app, middleware, controllers) {
 	var router = express.Router();
 	app.use('/api', router);
 
-	router.get('/config', function (req, res, next) {
+	router.get('/config', middleware.applyCSRF, controllers.api.getConfig);
-		if (req.uid >= 0) {
-			middleware.applyCSRF(req, res, next);
-		} else {
-			setImmediate(next);
-		}
-	}, controllers.api.getConfig);
 
 	router.get('/me', controllers.user.getCurrentUser);
 	router.get('/user/uid/:uid', middleware.canViewUsers, controllers.user.getUserByUID);

src/routes/helpers.js

@@ -5,7 +5,7 @@ var helpers = module.exports;
 helpers.setupPageRoute = function (router, name, middleware, middlewares, controller) {
 	middlewares = [middleware.maintenanceMode, middleware.registrationComplete, middleware.pageView, middleware.pluginHooks].concat(middlewares);
 
-	router.get(name, middleware.busyCheck, middleware.buildHeader, middlewares, helpers.tryRoute(controller));
+	router.get(name, middleware.busyCheck, middleware.applyCSRF, middleware.buildHeader, middlewares, helpers.tryRoute(controller));
 	router.get('/api' + name, middlewares, helpers.tryRoute(controller));
 };
 

src/routes/index.js

@@ -56,7 +56,7 @@ function topicRoutes(app, middleware, controllers) {
 
 function postRoutes(app, middleware, controllers) {
 	const middlewares = [middleware.maintenanceMode, middleware.registrationComplete, middleware.pluginHooks];
-	app.get('/post/:pid', middleware.busyCheck, middleware.buildHeader, middlewares, controllers.posts.redirectToPost);
+	app.get('/post/:pid', middleware.busyCheck, middlewares, controllers.posts.redirectToPost);
 	app.get('/api/post/:pid', middlewares, controllers.posts.redirectToPost);
 }