Split prepare context middleware and fix unit test

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);

lib/cartodb/controllers/base.js

@@ -1,6 +1,6 @@
 var debug = require('debug')('windshaft:cartodb');
 
-function BaseController(authApi, pgConnection) {
+function BaseController() {
 }
 
 module.exports = BaseController;

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

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

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) {

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();
+        });
+    };
+};

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();
+    };
+};

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);
+        });
+    };
+};

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)
+    ];
+};

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();
+    };
+};

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);