From 5a48c662d6867cdcdd5db7fa3c250c0a52f146ae Mon Sep 17 00:00:00 2001 From: Andris Reinman Date: Thu, 24 Nov 2022 15:57:11 +0200 Subject: [PATCH] Make sure that wrapped middleware does not return anything --- api.js | 320 +++++++++++++++++++++++++-------------------------- lib/tools.js | 77 +++++++------ 2 files changed, 200 insertions(+), 197 deletions(-) diff --git a/api.js b/api.js index f0c3c22..17636b0 100644 --- a/api.js +++ b/api.js @@ -215,106 +215,105 @@ server.get( server.use(restify.plugins.gzipResponse()); -server.use( - tools.responseWrapper(async (req, res) => { - if (['public_get', 'public_post', 'acmeToken'].includes(req.route.name)) { - // skip token check for public pages +server.use(async (req, res) => { + if (['public_get', 'public_post', 'acmeToken'].includes(req.route.name)) { + // skip token check for public pages + return; + } + + let accessToken = + req.query.accessToken || + req.headers['x-access-token'] || + (req.headers.authorization ? req.headers.authorization.replace(/^Bearer\s+/i, '').trim() : false) || + false; + + if (req.query.accessToken) { + // delete or it will conflict with Joi schemes + delete req.query.accessToken; + } + + if (req.params.accessToken) { + // delete or it will conflict with Joi schemes + delete req.params.accessToken; + } + + if (req.headers['x-access-token']) { + req.headers['x-access-token'] = ''; + } + + if (req.headers.authorization) { + req.headers.authorization = ''; + } + + let tokenRequired = false; + + let fail = () => { + res.status(403); + res.charSet('utf-8'); + return res.json({ + error: 'Invalid accessToken value', + code: 'InvalidToken' + }); + }; + + req.validate = permission => { + if (!permission.granted) { + let err = new Error('Not enough privileges'); + err.responseCode = 403; + err.code = 'MissingPrivileges'; + throw err; + } + }; + + // hard coded master token + if (config.api.accessToken) { + tokenRequired = true; + if (config.api.accessToken === accessToken) { + req.role = 'root'; + req.user = 'root'; return; } + } - let accessToken = - req.query.accessToken || - req.headers['x-access-token'] || - (req.headers.authorization ? req.headers.authorization.replace(/^Bearer\s+/i, '').trim() : false) || - false; + if (config.api.accessControl.enabled || accessToken) { + tokenRequired = true; + if (accessToken && accessToken.length === 40 && /^[a-fA-F0-9]{40}$/.test(accessToken)) { + let tokenData; + let tokenHash = crypto.createHash('sha256').update(accessToken).digest('hex'); - if (req.query.accessToken) { - // delete or it will conflict with Joi schemes - delete req.query.accessToken; - } - - if (req.params.accessToken) { - // delete or it will conflict with Joi schemes - delete req.params.accessToken; - } - - if (req.headers['x-access-token']) { - req.headers['x-access-token'] = ''; - } - - if (req.headers.authorization) { - req.headers.authorization = ''; - } - - let tokenRequired = false; - - let fail = () => { - res.status(403); - res.charSet('utf-8'); - return res.json({ - error: 'Invalid accessToken value', - code: 'InvalidToken' - }); - }; - - req.validate = permission => { - if (!permission.granted) { - let err = new Error('Not enough privileges'); - err.responseCode = 403; - err.code = 'MissingPrivileges'; + try { + let key = 'tn:token:' + tokenHash; + tokenData = await db.redis.hgetall(key); + } catch (err) { + err.responseCode = 500; + err.code = 'InternalDatabaseError'; throw err; } - }; - // hard coded master token - if (config.api.accessToken) { - tokenRequired = true; - if (config.api.accessToken === accessToken) { - req.role = 'root'; - req.user = 'root'; - return; - } - } - - if (config.api.accessControl.enabled || accessToken) { - tokenRequired = true; - if (accessToken && accessToken.length === 40 && /^[a-fA-F0-9]{40}$/.test(accessToken)) { - let tokenData; - let tokenHash = crypto.createHash('sha256').update(accessToken).digest('hex'); - - try { - let key = 'tn:token:' + tokenHash; - tokenData = await db.redis.hgetall(key); - } catch (err) { - err.responseCode = 500; - err.code = 'InternalDatabaseError'; - throw err; + if (tokenData && tokenData.user && tokenData.role && config.api.roles[tokenData.role]) { + let signData; + if ('authVersion' in tokenData) { + // cast value to number + tokenData.authVersion = Number(tokenData.authVersion) || 0; + signData = { + token: accessToken, + user: tokenData.user, + authVersion: tokenData.authVersion, + role: tokenData.role + }; + } else { + signData = { + token: accessToken, + user: tokenData.user, + role: tokenData.role + }; } - if (tokenData && tokenData.user && tokenData.role && config.api.roles[tokenData.role]) { - let signData; - if ('authVersion' in tokenData) { - // cast value to number - tokenData.authVersion = Number(tokenData.authVersion) || 0; - signData = { - token: accessToken, - user: tokenData.user, - authVersion: tokenData.authVersion, - role: tokenData.role - }; - } else { - signData = { - token: accessToken, - user: tokenData.user, - role: tokenData.role - }; - } + let signature = crypto.createHmac('sha256', config.api.accessControl.secret).update(JSON.stringify(signData)).digest('hex'); - let signature = crypto.createHmac('sha256', config.api.accessControl.secret).update(JSON.stringify(signData)).digest('hex'); - - if (signature !== tokenData.s) { - // rogue token or invalidated secret - /* + if (signature !== tokenData.s) { + // rogue token or invalidated secret + /* // do not delete just in case there is something wrong with the check try { await db.redis @@ -325,98 +324,97 @@ server.use( // ignore } */ - } else if (tokenData.ttl && !isNaN(tokenData.ttl) && Number(tokenData.ttl) > 0) { - let tokenTTL = Number(tokenData.ttl); - let tokenLifetime = config.api.accessControl.tokenLifetime || consts.ACCESS_TOKEN_MAX_LIFETIME; + } else if (tokenData.ttl && !isNaN(tokenData.ttl) && Number(tokenData.ttl) > 0) { + let tokenTTL = Number(tokenData.ttl); + let tokenLifetime = config.api.accessControl.tokenLifetime || consts.ACCESS_TOKEN_MAX_LIFETIME; - // check if token is not too old - if ((Date.now() - Number(tokenData.created)) / 1000 < tokenLifetime) { - // token is still usable, increase session length - try { - await db.redis - .multi() - .expire('tn:token:' + tokenHash, tokenTTL) - .exec(); - } catch (err) { - // ignore - } - req.role = tokenData.role; - req.user = tokenData.user; - - // make a reference to original method, otherwise might be overrided - let setAuthToken = userHandler.setAuthToken.bind(userHandler); - - req.accessToken = { - hash: tokenHash, - user: tokenData.user, - // if called then refreshes token data for current hash - update: async () => setAuthToken(tokenData.user, accessToken) - }; - } else { - // expired token, clear it - try { - await db.redis - .multi() - .del('tn:token:' + tokenHash) - .exec(); - } catch (err) { - // ignore - } + // check if token is not too old + if ((Date.now() - Number(tokenData.created)) / 1000 < tokenLifetime) { + // token is still usable, increase session length + try { + await db.redis + .multi() + .expire('tn:token:' + tokenHash, tokenTTL) + .exec(); + } catch (err) { + // ignore } - } else { req.role = tokenData.role; req.user = tokenData.user; - } - if (req.params && req.params.user === 'me' && /^[0-9a-f]{24}$/i.test(req.user)) { - req.params.user = req.user; - } + // make a reference to original method, otherwise might be overrided + let setAuthToken = userHandler.setAuthToken.bind(userHandler); - if (!req.role) { - return fail(); + req.accessToken = { + hash: tokenHash, + user: tokenData.user, + // if called then refreshes token data for current hash + update: async () => setAuthToken(tokenData.user, accessToken) + }; + } else { + // expired token, clear it + try { + await db.redis + .multi() + .del('tn:token:' + tokenHash) + .exec(); + } catch (err) { + // ignore + } } + } else { + req.role = tokenData.role; + req.user = tokenData.user; + } - if (/^[0-9a-f]{24}$/i.test(req.user)) { - let tokenAuthVersion = Number(tokenData.authVersion) || 0; - let userData = await db.users.collection('users').findOne( - { - _id: new ObjectId(req.user) - }, - { projection: { authVersion: true } } - ); - let userAuthVersion = Number(userData && userData.authVersion) || 0; - if (!userData || tokenAuthVersion < userAuthVersion) { - // unknown user or expired session - try { - /* + if (req.params && req.params.user === 'me' && /^[0-9a-f]{24}$/i.test(req.user)) { + req.params.user = req.user; + } + + if (!req.role) { + return fail(); + } + + if (/^[0-9a-f]{24}$/i.test(req.user)) { + let tokenAuthVersion = Number(tokenData.authVersion) || 0; + let userData = await db.users.collection('users').findOne( + { + _id: new ObjectId(req.user) + }, + { projection: { authVersion: true } } + ); + let userAuthVersion = Number(userData && userData.authVersion) || 0; + if (!userData || tokenAuthVersion < userAuthVersion) { + // unknown user or expired session + try { + /* // do not delete just in case there is something wrong with the check await db.redis .multi() .del('tn:token:' + tokenHash) .exec(); */ - } catch (err) { - // ignore - } - return fail(); + } catch (err) { + // ignore } + return fail(); } - - return; } + + return; } } + } - if (tokenRequired) { - // no valid token found - return fail(); - } + if (tokenRequired) { + // no valid token found + return fail(); + } - // allow all - req.role = 'root'; - req.user = 'root'; - }) -); + // allow all + req.role = 'root'; + req.user = 'root'; +}); logger.token('user-ip', req => ((req.params && req.params.ip) || '').toString().substr(0, 40) || '-'); logger.token('user-sess', req => (req.params && req.params.sess) || '-'); diff --git a/lib/tools.js b/lib/tools.js index 99a4b2d..57f64dd 100644 --- a/lib/tools.js +++ b/lib/tools.js @@ -642,48 +642,53 @@ module.exports = { return metaData || {}; }, - responseWrapper: middleware => async (req, res) => { - try { - await middleware(req, res); - } catch (err) { - let data = { - error: err.formattedMessage || err.message - }; + responseWrapper(middleware) { + return async (req, res) => { + req._localId = crypto.randomBytes(8).toString('hex'); + try { + await middleware(req, res); + } catch (err) { + let data = { + error: err.formattedMessage || err.message + }; - switch (err.code) { - case 'ALREADYEXISTS': - err.responseCode = err.responseCode || 400; - err.code = 'MailboxExistsError'; - break; - case 'NONEXISTENT': - err.responseCode = err.responseCode || 404; - err.code = 'NoSuchMailbox'; - break; - } + switch (err.code) { + case 'ALREADYEXISTS': + err.responseCode = err.responseCode || 400; + err.code = 'MailboxExistsError'; + break; + case 'NONEXISTENT': + err.responseCode = err.responseCode || 404; + err.code = 'NoSuchMailbox'; + break; + } - if (err.responseCode) { - res.status(err.responseCode); - } + if (err.responseCode) { + res.status(err.responseCode); + } - if (err.code) { - data.code = err.code; - } + if (err.code) { + data.code = err.code; + } - if (err.details && typeof err.details === 'object') { - for (let key of Object.keys(err.details)) { - if (!data[key]) { - data[key] = err.details[key]; + if (err.details && typeof err.details === 'object') { + for (let key of Object.keys(err.details)) { + if (!data[key]) { + data[key] = err.details[key]; + } } } + + log.http( + 'Error', + `${req.method} ${req.url} sess=${(req.params && req.params.sess) || '-'} user=${req.user ? req.user : '-'} error=${JSON.stringify( + err.stack + )}` + ); + + res.charSet('utf-8'); + res.json(data); } - - log.http( - 'Error', - `${req.method} ${req.url} sess=${(req.params && req.params.sess) || '-'} user=${req.user ? req.user : '-'} error=${JSON.stringify(err.stack)}` - ); - - res.charSet('utf-8'); - return res.json(data); - } + }; } };