From 5a1c6ee7ed847baeab68f7644dc602e0897d2259 Mon Sep 17 00:00:00 2001
From: Julian Lam <julian@nodebb.org>
Date: Fri, 24 Apr 2020 11:49:56 -0400
Subject: [PATCH] fix: response hook logic

After some more thought, a response hook should be checking for
whether headers are sent, and executing (or not executing) the
default logic in that case.

Before, we were relying on hooks to call data.next() to continue
execution, but it makes more sense to have the listener either
send a response or not, and handle the behaviour afterwards.
---
 src/middleware/user.js | 35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/src/middleware/user.js b/src/middleware/user.js
index 99f8e4c133..e2d88f84d7 100644
--- a/src/middleware/user.js
+++ b/src/middleware/user.js
@@ -16,31 +16,26 @@ const controllers = {
 };
 
 module.exports = function (middleware) {
-	function authenticate(req, res, next, callback) {
+	async function authenticate(req, res, next, callback) {
 		if (req.loggedIn) {
 			return next();
 		}
-		if (plugins.hasListeners('response:middleware.authenticate')) {
-			return plugins.fireHook('response:middleware.authenticate', {
-				req: req,
-				res: res,
-				next: function (err) {
-					if (err) {
-						return next(err);
-					}
-
-					auth.setAuthVars(req, res, function () {
-						if (req.loggedIn && req.user && req.user.uid) {
-							return next();
-						}
-
-						callback();
-					});
-				},
+
+		await plugins.fireHook('response:middleware.authenticate', {
+			req: req,
+			res: res,
+			next: function () {},	// no-op for backwards compatibility
+		});
+
+		if (!res.headersSent) {
+			auth.setAuthVars(req, res, function () {
+				if (req.loggedIn && req.user && req.user.uid) {
+					return next();
+				}
+
+				callback();
 			});
 		}
-
-		callback();
 	}
 
 	middleware.authenticate = function middlewareAuthenticate(req, res, next) {