From bf35dcfc5096a77b19bc2b50261b34c98fce7011 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sat, 18 Dec 2021 16:30:17 -0500 Subject: [PATCH] webaccess: Move `preAuthorize` to its own middleware --- src/node/hooks/express.js | 2 + src/node/hooks/express/webaccess.js | 116 +++++++++++++++++----------- 2 files changed, 71 insertions(+), 47 deletions(-) diff --git a/src/node/hooks/express.js b/src/node/hooks/express.js index 94d914009..807127a01 100644 --- a/src/node/hooks/express.js +++ b/src/node/hooks/express.js @@ -204,6 +204,8 @@ exports.restartServer = async () => { app.use(exports.sessionMiddleware); app.use(cookieParser(settings.sessionKey, {})); + // If webaccess.preAuthorize explicitly grants access, webaccess.checkAccess will skip all checks. + app.use(webaccess.preAuthorize); app.use(webaccess.checkAccess); await Promise.all([ diff --git a/src/node/hooks/express/webaccess.js b/src/node/hooks/express/webaccess.js index 9ab338498..16d3bb49b 100644 --- a/src/node/hooks/express/webaccess.js +++ b/src/node/hooks/express/webaccess.js @@ -26,6 +26,14 @@ const staticPathsRE = new RegExp(`^/(?:${[ 'tests/frontend(?:/.*)?', ].join('|')})$`); +// Promisified wrapper around hooks.aCallFirst. +const aCallFirst = (hookName, context, pred = null) => new Promise((resolve, reject) => { + hooks.aCallFirst(hookName, context, (err, r) => err != null ? reject(err) : resolve(r), pred); +}); + +const aCallFirst0 = + async (hookName, context, pred = null) => (await aCallFirst(hookName, context, pred))[0]; + exports.normalizeAuthzLevel = (level) => { if (!level) return false; switch (level) { @@ -56,16 +64,56 @@ exports.userCanModify = (padId, req) => { // Exported so that tests can set this to 0 to avoid unnecessary test slowness. exports.authnFailureDelayMs = 1000; +const preAuthorize = async (req, res, next) => { + const requireAdmin = req.path.toLowerCase().startsWith('/admin'); + const locals = res.locals._webaccess = {requireAdmin, skip: false}; + + // /////////////////////////////////////////////////////////////////////////////////////////////// + // Step 1: Check the preAuthorize hook for early permit/deny (permit is only allowed for non-admin + // pages). If any plugin explicitly grants or denies access, skip the remaining steps. Plugins can + // use the preAuthzFailure hook to override the default 403 error. + // /////////////////////////////////////////////////////////////////////////////////////////////// + + let results; + const preAuthorizeNext = (...args) => { locals.skip = true; next(...args); }; + try { + results = await aCallFirst('preAuthorize', {req, res, next: preAuthorizeNext}, + // This predicate will cause aCallFirst to call the hook functions one at a time until one + // of them returns a non-empty list, with an exception: If the request is for an /admin + // page, truthy entries are filtered out before checking to see whether the list is empty. + // This prevents plugin authors from accidentally granting admin privileges to the general + // public. + (r) => (locals.skip || (r != null && r.filter((x) => (!requireAdmin || !x)).length > 0))); + } catch (err) { + httpLogger.error(`Error in preAuthorize hook: ${err.stack || err.toString()}`); + if (!locals.skip) res.status(500).send('Internal Server Error'); + return; + } + if (locals.skip) return; + if (staticPathsRE.test(req.path)) results.push(true); + if (requireAdmin) { + // Filter out all 'true' entries to prevent plugin authors from accidentally granting admin + // privileges to the general public. + results = results.filter((x) => !x); + } + if (results.length > 0) { + // Access was explicitly granted or denied. If any value is false then access is denied. + if (!results.every((x) => x)) { + // Access explicitly denied. + if (await aCallFirst0('preAuthzFailure', {req, res})) return; + // No plugin handled the pre-authentication authorization failure. + return res.status(403).send('Forbidden'); + } + // Access explicitly granted. + locals.skip = true; + return next('route'); + } + next(); +}; + const checkAccess = async (req, res, next) => { - // Promisified wrapper around hooks.aCallFirst. - const aCallFirst = (hookName, context, pred = null) => new Promise((resolve, reject) => { - hooks.aCallFirst(hookName, context, (err, r) => err != null ? reject(err) : resolve(r), pred); - }); - - const aCallFirst0 = - async (hookName, context, pred = null) => (await aCallFirst(hookName, context, pred))[0]; - - const requireAdmin = req.path.toLowerCase().indexOf('/admin') === 0; + const {locals: {_webaccess: {requireAdmin, skip}}} = res; + if (skip) return next('route'); // This helper is used in steps 2 and 4 below, so it may be called twice per access: once before // authentication is checked and once after (if settings.requireAuthorization is true). @@ -99,43 +147,6 @@ const checkAccess = async (req, res, next) => { return await grant(await aCallFirst0('authorize', {req, res, next, resource: req.path})); }; - // /////////////////////////////////////////////////////////////////////////////////////////////// - // Step 1: Check the preAuthorize hook for early permit/deny (permit is only allowed for non-admin - // pages). If any plugin explicitly grants or denies access, skip the remaining steps. Plugins can - // use the preAuthzFailure hook to override the default 403 error. - // /////////////////////////////////////////////////////////////////////////////////////////////// - - let results; - let skip = false; - const preAuthorizeNext = (...args) => { skip = true; next(...args); }; - try { - results = await aCallFirst('preAuthorize', {req, res, next: preAuthorizeNext}, - // This predicate will cause aCallFirst to call the hook functions one at a time until one - // of them returns a non-empty list, with an exception: If the request is for an /admin - // page, truthy entries are filtered out before checking to see whether the list is empty. - // This prevents plugin authors from accidentally granting admin privileges to the general - // public. - (r) => (skip || (r != null && r.filter((x) => (!requireAdmin || !x)).length > 0))); - } catch (err) { - httpLogger.error(`Error in preAuthorize hook: ${err.stack || err.toString()}`); - if (!skip) res.status(500).send('Internal Server Error'); - return; - } - if (skip) return; - if (staticPathsRE.test(req.path)) results.push(true); - if (requireAdmin) { - // Filter out all 'true' entries to prevent plugin authors from accidentally granting admin - // privileges to the general public. - results = results.filter((x) => !x); - } - if (results.length > 0) { - // Access was explicitly granted or denied. If any value is false then access is denied. - if (results.every((x) => x)) return next(); - if (await aCallFirst0('preAuthzFailure', {req, res})) return; - // No plugin handled the pre-authentication authorization failure. - return res.status(403).send('Forbidden'); - } - // /////////////////////////////////////////////////////////////////////////////////////////////// // Step 2: Try to just access the thing. If access fails (perhaps authentication has not yet // completed, or maybe different credentials are required), go to the next step. @@ -203,9 +214,20 @@ const checkAccess = async (req, res, next) => { res.status(403).send('Forbidden'); }; +/** + * Express middleware that allows plugins to explicitly grant/deny access via the `preAuthorize` + * hook before `checkAccess` is run. If access is explicitly granted: + * - `next('route')` will be called, which can be used to bypass later checks + * - `checkAccess` will simply call `next('route')` + */ +exports.preAuthorize = (req, res, next) => { + preAuthorize(req, res, next).catch((err) => next(err || new Error(err))); +}; + /** * Express middleware to authenticate the user and check authorization. Must be installed after the - * express-session middleware. + * express-session middleware. If the request is pre-authorized, this middleware simply calls + * `next('route')`. */ exports.checkAccess = (req, res, next) => { checkAccess(req, res, next).catch((err) => next(err || new Error(err)));