From ca612dd02a5753575133c2655958d3f3fdbef122 Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 28 Sep 2017 11:43:12 +0200 Subject: [PATCH] res.locals in context middlewares --- lib/cartodb/middleware/context/authorize.js | 7 +------ .../context/clean-up-query-params.js | 4 ++-- .../middleware/context/db-conn-setup.js | 15 ++++----------- lib/cartodb/middleware/context/index.js | 2 ++ .../middleware/context/layergroup-token.js | 18 +++++++++--------- lib/cartodb/middleware/context/locals.js | 7 +++++++ 6 files changed, 25 insertions(+), 28 deletions(-) create mode 100644 lib/cartodb/middleware/context/locals.js diff --git a/lib/cartodb/middleware/context/authorize.js b/lib/cartodb/middleware/context/authorize.js index bf95a78a..bab100b0 100644 --- a/lib/cartodb/middleware/context/authorize.js +++ b/lib/cartodb/middleware/context/authorize.js @@ -2,14 +2,9 @@ const _ = require('underscore'); module.exports = function authorizeMiddleware (authApi) { return function (req, res, next) { - // FIXME: Temporary hack to share data between middlewares. Express overrides req.params to - // parse url params to an object and it's performed after matching path and controller. - res.locals = {}; - _.extend(res.locals, req.params); - req.profiler.done('req2params.setup'); - authApi.authorize(req, (err, authorized) => { + authApi.authorize(req, res, (err, authorized) => { req.profiler.done('authorize'); if (err) { return next(err); diff --git a/lib/cartodb/middleware/context/clean-up-query-params.js b/lib/cartodb/middleware/context/clean-up-query-params.js index 7b0b56eb..4cc1ceb6 100644 --- a/lib/cartodb/middleware/context/clean-up-query-params.js +++ b/lib/cartodb/middleware/context/clean-up-query-params.js @@ -24,8 +24,8 @@ module.exports = function cleanUpQueryParamsMiddleware () { req.query = _.pick(req.query, allowedQueryParams); - // bring all query values onto req.params object - _.extend(req.params, req.query); + // bring all query values onto res.locals object + _.extend(res.locals, req.query); next(); }; diff --git a/lib/cartodb/middleware/context/db-conn-setup.js b/lib/cartodb/middleware/context/db-conn-setup.js index 97efb77d..cc7df0d5 100644 --- a/lib/cartodb/middleware/context/db-conn-setup.js +++ b/lib/cartodb/middleware/context/db-conn-setup.js @@ -4,9 +4,8 @@ module.exports = function dbConnSetupMiddleware(pgConnection) { return function (req, res, next) { const user = req.context.user; - // FIXME: this function shouldn't be able to change `req.params`. It should return an - // object with the user's conf and it should be merge with default here. - pgConnection.setDBConn(user, req.params, (err) => { + res.locals.db = {} + pgConnection.setDBConn(user, res.locals.db, (err) => { if (err) { if (err.message && -1 !== err.message.indexOf('name not found')) { err.http_status = 404; @@ -17,20 +16,14 @@ module.exports = function dbConnSetupMiddleware(pgConnection) { // Add default database connection parameters // if none given - _.defaults(req.params, { + _.defaults(res.locals.db, { dbuser: global.environment.postgres.user, dbpassword: global.environment.postgres.password, dbhost: global.environment.postgres.host, dbport: global.environment.postgres.port }); - // FIXME: Temporary hack to share data between middlewares. Express overrides req.params to - // parse url params to an object and it's performed after matching path and controller. - if (!res.locals) { - res.locals = {}; - } - _.defaults(res.locals, req.params); - + req.profiler.done('req2params'); next(null, req); diff --git a/lib/cartodb/middleware/context/index.js b/lib/cartodb/middleware/context/index.js index 411b6f93..d660dcaf 100644 --- a/lib/cartodb/middleware/context/index.js +++ b/lib/cartodb/middleware/context/index.js @@ -1,3 +1,4 @@ +const locals = require('./locals') const cleanUpQueryParams = require('./clean-up-query-params'); const layergroupToken = require('./layergroup-token'); const authorize = require('./authorize'); @@ -5,6 +6,7 @@ const dbConnSetup = require('./db-conn-setup'); module.exports = function prepareContextMiddleware(authApi, pgConnection) { return [ + locals, cleanUpQueryParams(), layergroupToken, authorize(authApi), diff --git a/lib/cartodb/middleware/context/layergroup-token.js b/lib/cartodb/middleware/context/layergroup-token.js index d1ccb3be..b90d5e13 100644 --- a/lib/cartodb/middleware/context/layergroup-token.js +++ b/lib/cartodb/middleware/context/layergroup-token.js @@ -1,22 +1,22 @@ var LayergroupToken = require('../../models/layergroup-token'); module.exports = function layergroupTokenMiddleware(req, res, next) { - if (!req.params.hasOwnProperty('token')) { + if (!res.locals.hasOwnProperty('token')) { return next(); } var user = req.context.user; - var layergroupToken = LayergroupToken.parse(req.params.token); - req.params.token = layergroupToken.token; - req.params.cache_buster = layergroupToken.cacheBuster; + var layergroupToken = LayergroupToken.parse(res.locals.token); + res.locals.token = layergroupToken.token; + res.locals.cache_buster = layergroupToken.cacheBuster; if (layergroupToken.signer) { - req.params.signer = layergroupToken.signer; - if (!req.params.signer) { - req.params.signer = user; - } else if (req.params.signer !== user) { - var err = new Error(`Cannot use map signature of user "${req.params.signer}" on db of user "${user}"`); + res.locals.signer = layergroupToken.signer; + if (!res.locals.signer) { + res.locals.signer = user; + } else if (res.locals.signer !== user) { + var err = new Error(`Cannot use map signature of user "${res.locals.signer}" on db of user "${user}"`); err.type = 'auth'; err.http_status = 403; if (req.query && req.query.callback) { diff --git a/lib/cartodb/middleware/context/locals.js b/lib/cartodb/middleware/context/locals.js new file mode 100644 index 00000000..ec847788 --- /dev/null +++ b/lib/cartodb/middleware/context/locals.js @@ -0,0 +1,7 @@ +module.exports = function layergroupTokenMiddleware(req, res, next) { + // FIXME: Temporary hack to share data between middlewares. Express overrides req.params to + // parse url params to an object and it's performed after matching path and controller. + res.locals = {}; + _.extend(res.locals, req.params); +} +