express: Check access before `expressConfigure` middleware

There are no guarantees about the order of execution of hook
functions, which means that a plugin's `expressConfigure` hook
function could theoretically register a handler/middleware before the
access check middleware is registered. If that happens, the plugin's
handler would run before the access check, which would be bad. Avoid
the problem by explicitly installing the `webaccess.checkAccess`
middleware before running the `expressConfigure` hook.

src/ep.json

@@ -50,12 +50,6 @@
         "expressCreateServer": "ep_etherpad-lite/node/hooks/express/padurlsanitize"
       }
     },
-    {
-      "name": "webaccess",
-      "hooks": {
-        "expressConfigure": "ep_etherpad-lite/node/hooks/express/webaccess"
-      }
-    },
     {
       "name": "apicalls",
       "hooks": {

src/node/hooks/express.js

@@ -12,6 +12,7 @@ const SessionStore = require('../db/SessionStore');
 const settings = require('../utils/Settings');
 const stats = require('../stats');
 const util = require('util');
+const webaccess = require('./express/webaccess');
 
 const logger = log4js.getLogger('http');
 let serverName;
@@ -203,6 +204,7 @@ exports.restartServer = async () => {
   app.use(exports.sessionMiddleware);
 
   app.use(cookieParser(settings.sessionKey, {}));
+  app.use(webaccess.checkAccess);
 
   await Promise.all([
     hooks.aCallAll('expressConfigure', {app}),

src/node/hooks/express/webaccess.js

@@ -203,7 +203,10 @@ const checkAccess = async (req, res, next) => {
   res.status(403).send('Forbidden');
 };
 
-exports.expressConfigure = (hookName, args, cb) => {
-  args.app.use((req, res, next) => { checkAccess(req, res, next).catch(next); });
-  return cb();
+/**
+ * Express middleware to authenticate the user and check authorization. Must be installed after the
+ * express-session middleware.
+ */
+exports.checkAccess = (req, res, next) => {
+  checkAccess(req, res, next).catch((err) => next(err || new Error(err)));
 };