diff --git a/lib/cartodb/controllers/analyses.js b/lib/cartodb/controllers/analyses.js index cf1f04ae..26d602dd 100644 --- a/lib/cartodb/controllers/analyses.js +++ b/lib/cartodb/controllers/analyses.js @@ -9,7 +9,7 @@ var BaseController = require('./base'); var cors = require('../middleware/cors'); var userMiddleware = require('../middleware/user'); -const prepareContextMiddleware = require('../middleware/prepare-context'); +const prepareContextMiddleware = require('../middleware/context'); function AnalysesController(authApi, pgConnection) { BaseController.call(this, authApi, pgConnection); diff --git a/lib/cartodb/controllers/base.js b/lib/cartodb/controllers/base.js index b5551382..7462f6ba 100644 --- a/lib/cartodb/controllers/base.js +++ b/lib/cartodb/controllers/base.js @@ -1,6 +1,6 @@ var debug = require('debug')('windshaft:cartodb'); -function BaseController(authApi, pgConnection) { +function BaseController() { } module.exports = BaseController; diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index c33d3676..56168b91 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -15,7 +15,7 @@ var MapStoreMapConfigProvider = require('../models/mapconfig/provider/map-store- var QueryTables = require('cartodb-query-tables'); -const prepareContextMiddleware = require('../middleware/prepare-context'); +const prepareContextMiddleware = require('../middleware/context'); /** * @param {AuthApi} authApi diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 2384db9d..fc1df434 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -20,7 +20,7 @@ var NamedMapsCacheEntry = require('../cache/model/named_maps_entry'); var NamedMapMapConfigProvider = require('../models/mapconfig/provider/named-map-provider'); var CreateLayergroupMapConfigProvider = require('../models/mapconfig/provider/create-layergroup-provider'); -const prepareContextMiddleware = require('../middleware/prepare-context'); +const prepareContextMiddleware = require('../middleware/context'); /** * @param {AuthApi} authApi diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index fbd92200..a67a9464 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -9,7 +9,7 @@ var BaseController = require('./base'); var cors = require('../middleware/cors'); var userMiddleware = require('../middleware/user'); var allowQueryParams = require('../middleware/allow-query-params'); -const prepareContextMiddleware = require('../middleware/prepare-context'); +const prepareContextMiddleware = require('../middleware/context'); function NamedMapsController(authApi, pgConnection, namedMapProviderCache, tileBackend, previewBackend, surrogateKeysCache, tablesExtentApi, metadataBackend) { diff --git a/lib/cartodb/middleware/context/authorize.js b/lib/cartodb/middleware/context/authorize.js new file mode 100644 index 00000000..65bed758 --- /dev/null +++ b/lib/cartodb/middleware/context/authorize.js @@ -0,0 +1,30 @@ +const _ = require('underscore'); + +module.exports = function authorizeMiddleware (authApi) { + return function (req, res, next) { + // bring all query values onto req.params object + _.extend(req.params, req.query); + + // 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. + req.locals = {}; + _.extend(req.locals, req.params); + + req.profiler.done('req2params.setup'); + + authApi.authorize(req, (err, authorized) => { + req.profiler.done('authorize'); + if (err) { + return next(err); + } + + if(!authorized) { + err = new Error("Sorry, you are unauthorized (permission denied)"); + err.http_status = 403; + return next(err); + } + + return next(); + }); + }; +}; diff --git a/lib/cartodb/middleware/context/clean-up-query-params.js b/lib/cartodb/middleware/context/clean-up-query-params.js new file mode 100644 index 00000000..d756396c --- /dev/null +++ b/lib/cartodb/middleware/context/clean-up-query-params.js @@ -0,0 +1,37 @@ +const _ = require('underscore'); + +// Whitelist query parameters and attach format +const REQUEST_QUERY_PARAMS_WHITELIST = [ + 'config', + 'map_key', + 'api_key', + 'auth_token', + 'callback', + 'zoom', + 'lon', + 'lat', + // analysis + 'filters' // json +]; + +module.exports = function cleanUpQueryParamsMiddleware () { + return function cleanUpQueryParams (req, res, next) { + var allowedQueryParams = REQUEST_QUERY_PARAMS_WHITELIST; + + if (Array.isArray(req.context.allowedQueryParams)) { + allowedQueryParams = allowedQueryParams.concat(req.context.allowedQueryParams); + } + + req.query = _.pick(req.query, allowedQueryParams); + + // bring all query values onto req.params object + _.extend(req.params, req.query); + + // 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. + req.locals = {}; + _.extend(req.locals, req.params); + + next(); + }; +}; diff --git a/lib/cartodb/middleware/context/db-conn-setup.js b/lib/cartodb/middleware/context/db-conn-setup.js new file mode 100644 index 00000000..14499177 --- /dev/null +++ b/lib/cartodb/middleware/context/db-conn-setup.js @@ -0,0 +1,36 @@ +const _ = require('underscore'); + +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) => { + if (err) { + if (err.message && -1 !== err.message.indexOf('name not found')) { + err.http_status = 404; + } + req.profiler.done('req2params'); + return next(err, req); + } + + // Add default database connection parameters + // if none given + _.defaults(req.params, { + 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. + _.defaults(req.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 new file mode 100644 index 00000000..9d87b6ee --- /dev/null +++ b/lib/cartodb/middleware/context/index.js @@ -0,0 +1,13 @@ +const cleanUpQueryParams = require('./clean-up-query-params'); +const parseTokenParam = require('./parse-token-param'); +const authorize = require('./authorize'); +const dbConnSetup = require('./db-conn-setup'); + +module.exports = function prepareContextMiddleware(authApi, pgConnection) { + return [ + cleanUpQueryParams(), + parseTokenParam(), + authorize(authApi), + dbConnSetup(pgConnection) + ]; +}; diff --git a/lib/cartodb/middleware/context/parse-token-param.js b/lib/cartodb/middleware/context/parse-token-param.js new file mode 100644 index 00000000..e2a0c2c5 --- /dev/null +++ b/lib/cartodb/middleware/context/parse-token-param.js @@ -0,0 +1,47 @@ +module.exports = function parseTokenParamMiddleware () { + return function parseTokenParam (req, res, next) { + // jshint maxcomplexity:7 + if (!req.params.token) { + return next(); + } + + var user = req.context.user; + + // Token might match the following patterns: + // - {user}@{tpl_id}@{token}:{cache_buster} + var tksplit = req.params.token.split(':'); + + req.params.token = tksplit[0]; + + if ( tksplit.length > 1 ) { + req.params.cache_buster= tksplit[1]; + } + + tksplit = req.params.token.split('@'); + + if ( tksplit.length > 1 ) { + req.params.signer = tksplit.shift(); + + 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}"` + ); + err.http_status = 403; + req.profiler.done('req2params'); + + return next(err); + } + + // skip template hash + if (tksplit.length > 1) { + tksplit.shift(); + } + + req.params.token = tksplit.shift(); + } + + next(); + }; +}; diff --git a/lib/cartodb/middleware/prepare-context.js b/lib/cartodb/middleware/context/prepare-context.js similarity index 100% rename from lib/cartodb/middleware/prepare-context.js rename to lib/cartodb/middleware/context/prepare-context.js diff --git a/test/unit/cartodb/prepare-context.test.js b/test/unit/cartodb/prepare-context.test.js index 7bd2415e..a9119c13 100644 --- a/test/unit/cartodb/prepare-context.test.js +++ b/test/unit/cartodb/prepare-context.test.js @@ -7,7 +7,10 @@ var PgConnection = require('../../../lib/cartodb/backends/pg_connection'); var AuthApi = require('../../../lib/cartodb/api/auth_api'); var TemplateMaps = require('../../../lib/cartodb/backends/template_maps'); -var prepareContextMiddleware = require('../../../lib/cartodb/middleware/prepare-context'); +const cleanUpQueryParamsMiddleware = require('../../../lib/cartodb/middleware/context/clean-up-query-params'); +const authorizeMiddleware = require('../../../lib/cartodb/middleware/context/authorize'); +const dbConnSetupMiddleware = require('../../../lib/cartodb/middleware/context/db-conn-setup'); + var windshaft = require('windshaft'); describe('prepare-context', function() { @@ -16,8 +19,10 @@ describe('prepare-context', function() { var test_pubuser = global.environment.postgres.user; var test_database = test_user + '_db'; + let cleanUpQueryParams; + let dbConnSetup; + let authorize; - var prepareContext; before(function() { var redisPool = new RedisPool(global.environment.redis); var mapStore = new windshaft.storage.MapStore(); @@ -26,12 +31,16 @@ describe('prepare-context', function() { var templateMaps = new TemplateMaps(redisPool); var authApi = new AuthApi(pgConnection, metadataBackend, mapStore, templateMaps); - prepareContext = prepareContextMiddleware(authApi, pgConnection); + cleanUpQueryParams = cleanUpQueryParamsMiddleware(); + authorize = authorizeMiddleware(authApi); + dbConnSetup = dbConnSetupMiddleware(pgConnection); }); it('can be found in server_options', function(){ - assert.ok(_.isFunction(prepareContext)); + assert.ok(_.isFunction(authorize)); + assert.ok(_.isFunction(dbConnSetup)); + assert.ok(_.isFunction(cleanUpQueryParams)); }); function prepareRequest(req) { @@ -46,22 +55,22 @@ describe('prepare-context', function() { it('cleans up request', function(done){ var req = {headers: { host:'localhost' }, query: {dbuser:'hacker',dbname:'secret'}}; var res = {}; - prepareContext(prepareRequest(req), res, function(err, req) { + + cleanUpQueryParams(prepareRequest(req), res, function(err) { if ( err ) { done(err); return; } assert.ok(_.isObject(req.query), 'request has query'); assert.ok(!req.query.hasOwnProperty('dbuser'), 'dbuser was removed from query'); assert.ok(req.hasOwnProperty('params'), 'request has params'); assert.ok(!req.params.hasOwnProperty('interactivity'), 'request params do not have interactivity'); - assert.equal(req.params.dbname, test_database, 'could forge dbname: '+ req.params.dbname); - assert.ok(req.params.dbuser === test_pubuser, 'could inject dbuser ('+req.params.dbuser+')'); done(); }); }); it('sets dbname from redis metadata', function(done){ - var req = {headers: { host:'localhost' }, query: {} }; + var req = {headers: { host:'localhost' }, query: {}, locals: {} }; var res = {}; - prepareContext(prepareRequest(req), res, function(err, req) { + + dbConnSetup(prepareRequest(req), res, function(err) { if ( err ) { done(err); return; } assert.ok(_.isObject(req.query), 'request has query'); assert.ok(!req.query.hasOwnProperty('dbuser'), 'dbuser was removed from query'); @@ -74,31 +83,38 @@ describe('prepare-context', function() { }); it('sets also dbuser for authenticated requests', function(done){ - var req = {headers: { host:'localhost' }, query: {map_key: '1234'} }; - var res = {}; - prepareContext(prepareRequest(req), res, function(err, req) { - if ( err ) { done(err); return; } - assert.ok(_.isObject(req.query), 'request has query'); - assert.ok(!req.query.hasOwnProperty('dbuser'), 'dbuser was removed from query'); - assert.ok(req.hasOwnProperty('params'), 'request has params'); - assert.ok(!req.params.hasOwnProperty('interactivity'), 'request params do not have interactivity'); - assert.equal(req.params.dbname, test_database); - assert.equal(req.params.dbuser, test_user); + var req = { headers: { host: 'localhost' }, query: { map_key: '1234' }, locals: {} }; + var res = {}; - req = { - headers: { - host:'localhost' - }, - query: { - map_key: '1235' - } - }; - prepareContext(prepareRequest(req), res, function(err, req) { - // wrong key resets params to no user - assert.ok(req.params.dbuser === test_pubuser, 'could inject dbuser ('+req.params.dbuser+')'); - done(); - }); - }); + // FIXME: review authorize-pgconnsetup workflow, It might we are doing authorization twice. + authorize(prepareRequest(req), res, function (err) { + if (err) { done(err); return; } + dbConnSetup(req, res, function(err) { + if ( err ) { done(err); return; } + assert.ok(_.isObject(req.query), 'request has query'); + assert.ok(!req.query.hasOwnProperty('dbuser'), 'dbuser was removed from query'); + assert.ok(req.hasOwnProperty('params'), 'request has params'); + assert.ok(!req.params.hasOwnProperty('interactivity'), 'request params do not have interactivity'); + assert.equal(req.params.dbname, test_database); + assert.equal(req.params.dbuser, test_user); + + req = { + headers: { + host:'localhost' + }, + query: { + map_key: '1235' + }, + locals: {} + }; + + dbConnSetup(prepareRequest(req), res, function(err, req) { + // wrong key resets params to no user + assert.ok(req.params.dbuser === test_pubuser, 'could inject dbuser ('+req.params.dbuser+')'); + done(); + }); + }); + }); }); it('it should remove invalid params', function(done) { @@ -114,14 +130,16 @@ describe('prepare-context', function() { api_key: 'test', style: 'override', config: config - } + }, + locals: {} }; var res = {}; - prepareContext(prepareRequest(req), res, function(err, req) { + cleanUpQueryParams(prepareRequest(req), res, function (err) { if ( err ) { return done(err); } + var query = req.params; assert.deepEqual(config, query.config); assert.equal('test', query.api_key);