Release 2.14.1

Safe user extraction

NEWS.md

@@ -1,5 +1,19 @@
 # Changelog
 
+## 2.14.1
+
+Released 2015-09-30
+
+Enhancements:
+ - Remove app dependency from controllers
+
+Announcements:
+ - Upgrades windshaft to [1.0.1](https://github.com/CartoDB/Windshaft/releases/tag/1.0.1)
+
+Improvements:
+ - Safer user extraction from request Host header
+
+
 ## 2.14.0
 
 Released 2015-09-30

lib/cartodb/controllers/layergroup.js

@@ -5,12 +5,12 @@ var util = require('util');
 var BaseController = require('./base');
 
 var cors = require('../middleware/cors');
+var userMiddleware = require('../middleware/user');
 
 var MapStoreMapConfigProvider = require('../models/mapconfig/map_store_provider');
 var TablesCacheEntry = require('../cache/model/database_tables_entry');
 
 /**
- * @param app
  * @param {AuthApi} authApi
  * @param {PgConnection} pgConnection
  * @param {MapStore} mapStore
@@ -23,11 +23,10 @@ var TablesCacheEntry = require('../cache/model/database_tables_entry');
  * @param {LayergroupAffectedTables} layergroupAffectedTables
  * @constructor
  */
-function LayergroupController(app, authApi, pgConnection, mapStore, tileBackend, previewBackend, attributesBackend,
+function LayergroupController(authApi, pgConnection, mapStore, tileBackend, previewBackend, attributesBackend,
                               surrogateKeysCache, userLimitsApi, queryTablesApi, layergroupAffectedTables) {
     BaseController.call(this, authApi, pgConnection);
 
-    this.app = app;
     this.mapStore = mapStore;
     this.tileBackend = tileBackend;
     this.previewBackend = previewBackend;
@@ -44,13 +43,28 @@ module.exports = LayergroupController;
 
 
 LayergroupController.prototype.register = function(app) {
-    app.get(app.base_url_mapconfig + '/:token/:z/:x/:y@:scale_factor?x.:format', cors(), this.tile.bind(this));
-    app.get(app.base_url_mapconfig + '/:token/:z/:x/:y.:format', cors(), this.tile.bind(this));
-    app.get(app.base_url_mapconfig + '/:token/:layer/:z/:x/:y.(:format)', cors(), this.layer.bind(this));
-    app.get(app.base_url_mapconfig + '/:token/:layer/attributes/:fid', cors(), this.attributes.bind(this));
-    app.get(app.base_url_mapconfig + '/static/center/:token/:z/:lat/:lng/:width/:height.:format', cors(),
+    app.get(app.base_url_mapconfig +
+        '/:token/:z/:x/:y@:scale_factor?x.:format', cors(), userMiddleware,
+        this.tile.bind(this));
+
+    app.get(app.base_url_mapconfig +
+        '/:token/:z/:x/:y.:format', cors(), userMiddleware,
+        this.tile.bind(this));
+
+    app.get(app.base_url_mapconfig +
+        '/:token/:layer/:z/:x/:y.(:format)', cors(), userMiddleware,
+        this.layer.bind(this));
+
+    app.get(app.base_url_mapconfig +
+        '/:token/:layer/attributes/:fid', cors(), userMiddleware,
+        this.attributes.bind(this));
+
+    app.get(app.base_url_mapconfig +
+        '/static/center/:token/:z/:lat/:lng/:width/:height.:format', cors(), userMiddleware,
         this.center.bind(this));
-    app.get(app.base_url_mapconfig + '/static/bbox/:token/:west,:south,:east,:north/:width/:height.:format', cors(),
+
+    app.get(app.base_url_mapconfig +
+        '/static/bbox/:token/:west,:south,:east,:north/:width/:height.:format', cors(), userMiddleware,
         this.bbox.bind(this));
 };
 

lib/cartodb/controllers/map.js

@@ -7,6 +7,7 @@ var util = require('util');
 var BaseController = require('./base');
 
 var cors = require('../middleware/cors');
+var userMiddleware = require('../middleware/user');
 
 var MapConfig = windshaft.model.MapConfig;
 var Datasource = windshaft.model.Datasource;
@@ -19,7 +20,6 @@ var NamedMapMapConfigProvider = require('../models/mapconfig/named_map_provider'
 var CreateLayergroupMapConfigProvider = require('../models/mapconfig/create_layergroup_provider');
 
 /**
- * @param app
  * @param {AuthApi} authApi
  * @param {PgConnection} pgConnection
  * @param {TemplateMaps} templateMaps
@@ -31,12 +31,11 @@ var CreateLayergroupMapConfigProvider = require('../models/mapconfig/create_laye
  * @param {LayergroupAffectedTables} layergroupAffectedTables
  * @constructor
  */
-function MapController(app, authApi, pgConnection, templateMaps, mapBackend, metadataBackend, queryTablesApi,
+function MapController(authApi, pgConnection, templateMaps, mapBackend, metadataBackend, queryTablesApi,
                        surrogateKeysCache, userLimitsApi, layergroupAffectedTables) {
 
     BaseController.call(this, authApi, pgConnection);
 
-    this.app = app;
     this.pgConnection = pgConnection;
     this.templateMaps = templateMaps;
     this.mapBackend = mapBackend;
@@ -55,10 +54,10 @@ module.exports = MapController;
 
 
 MapController.prototype.register = function(app) {
-    app.get(app.base_url_mapconfig, cors(), this.createGet.bind(this));
-    app.post(app.base_url_mapconfig, cors(), this.createPost.bind(this));
-    app.get(app.base_url_templated + '/:template_id/jsonp', cors(), this.jsonp.bind(this));
-    app.post(app.base_url_templated + '/:template_id', cors(), this.instantiate.bind(this));
+    app.get(app.base_url_mapconfig, cors(), userMiddleware, this.createGet.bind(this));
+    app.post(app.base_url_mapconfig, cors(), userMiddleware, this.createPost.bind(this));
+    app.get(app.base_url_templated + '/:template_id/jsonp', cors(), userMiddleware, this.jsonp.bind(this));
+    app.post(app.base_url_templated + '/:template_id', cors(), userMiddleware, this.instantiate.bind(this));
     app.options(app.base_url_mapconfig, cors('Content-Type'));
 };
 

lib/cartodb/controllers/named_maps.js

@@ -7,14 +7,14 @@ var util = require('util');
 var BaseController = require('./base');
 
 var cors = require('../middleware/cors');
+var userMiddleware = require('../middleware/user');
 
 var TablesCacheEntry = require('../cache/model/database_tables_entry');
 
-function NamedMapsController(app, authApi, pgConnection, namedMapProviderCache, tileBackend, previewBackend,
+function NamedMapsController(authApi, pgConnection, namedMapProviderCache, tileBackend, previewBackend,
                              surrogateKeysCache, tablesExtentApi, metadataBackend) {
     BaseController.call(this, authApi, pgConnection);
 
-    this.app = app;
     this.namedMapProviderCache = namedMapProviderCache;
     this.tileBackend = tileBackend;
     this.previewBackend = previewBackend;
@@ -28,10 +28,13 @@ util.inherits(NamedMapsController, BaseController);
 module.exports = NamedMapsController;
 
 NamedMapsController.prototype.register = function(app) {
-    app.get(app.base_url_templated + '/:template_id/:layer/:z/:x/:y.(:format)', cors(), this.tile.bind(this));
-    app.get(
-        app.base_url_mapconfig + '/static/named/:template_id/:width/:height.:format', cors(), this.staticMap.bind(this)
-    );
+    app.get(app.base_url_templated +
+        '/:template_id/:layer/:z/:x/:y.(:format)', cors(), userMiddleware,
+        this.tile.bind(this));
+
+    app.get(app.base_url_mapconfig +
+        '/static/named/:template_id/:width/:height.:format', cors(), userMiddleware,
+        this.staticMap.bind(this));
 };
 
 NamedMapsController.prototype.sendResponse = function(req, res, resource, headers, namedMapProvider) {

lib/cartodb/controllers/named_maps_admin.js

@@ -6,19 +6,20 @@ var util = require('util');
 var BaseController = require('./base');
 
 var cors = require('../middleware/cors');
+var userMiddleware = require('../middleware/user');
 
 
 /**
- * @param {TemplateMaps} templateMaps
  * @param {AuthApi} authApi
  * @param {PgConnection} pgConnection
+ * @param {TemplateMaps} templateMaps
  * @constructor
  */
-function NamedMapsAdminController(templateMaps, authApi, pgConnection) {
+function NamedMapsAdminController(authApi, pgConnection, templateMaps) {
     BaseController.call(this, authApi, pgConnection);
 
-    this.templateMaps = templateMaps;
     this.authApi = authApi;
+    this.templateMaps = templateMaps;
 }
 
 util.inherits(NamedMapsAdminController, BaseController);
@@ -26,11 +27,11 @@ util.inherits(NamedMapsAdminController, BaseController);
 module.exports = NamedMapsAdminController;
 
 NamedMapsAdminController.prototype.register = function(app) {
-    app.post(app.base_url_templated, cors(), this.create.bind(this));
-    app.put(app.base_url_templated + '/:template_id', cors(), this.update.bind(this));
-    app.get(app.base_url_templated + '/:template_id', cors(), this.retrieve.bind(this));
-    app.delete(app.base_url_templated + '/:template_id', cors(), this.destroy.bind(this));
-    app.get(app.base_url_templated, cors(), this.list.bind(this));
+    app.post(app.base_url_templated, cors(), userMiddleware, this.create.bind(this));
+    app.put(app.base_url_templated + '/:template_id', cors(), userMiddleware, this.update.bind(this));
+    app.get(app.base_url_templated + '/:template_id', cors(), userMiddleware, this.retrieve.bind(this));
+    app.delete(app.base_url_templated + '/:template_id', cors(), userMiddleware, this.destroy.bind(this));
+    app.get(app.base_url_templated, cors(), userMiddleware, this.list.bind(this));
     app.options(app.base_url_templated + '/:template_id', cors('Content-Type'));
 };
 

lib/cartodb/middleware/user.js

@@ -0,0 +1,7 @@
+var CdbRequest = require('../models/cdb_request');
+var cdbRequest = new CdbRequest();
+
+module.exports = function userMiddleware(req, res, next) {
+    req.context.user = cdbRequest.userByReq(req);
+    next();
+};

lib/cartodb/models/cdb_request.js

@@ -8,7 +8,7 @@ module.exports = CdbRequest;
 
 
 CdbRequest.prototype.userByReq = function(req) {
-    var host = req.headers.host;
+    var host = req.headers.host || '';
     if (req.params.user) {
         return req.params.user;
     }

lib/cartodb/server.js

@@ -27,9 +27,6 @@ var NamedMapProviderCache = require('./cache/named_map_provider_cache');
 var PgQueryRunner = require('./backends/pg_query_runner');
 var PgConnection = require('./backends/pg_connection');
 
-var CdbRequest = require('./models/cdb_request');
-var cdbRequest = new CdbRequest();
-
 var timeoutErrorTilePath = __dirname + '/../../assets/render-timeout-fallback.png';
 var timeoutErrorTile = require('fs').readFileSync(timeoutErrorTilePath, {encoding: null});
 
@@ -157,13 +154,7 @@ module.exports = function(serverOptions) {
      * Routing
      ******************************************************************************************************************/
 
-    app.all('*', function(req, res, next) {
-        req.context.user = cdbRequest.userByReq(req);
-        next();
-    });
-
     new controller.Layergroup(
-        app,
         authApi,
         pgConnection,
         mapStore,
@@ -177,7 +168,6 @@ module.exports = function(serverOptions) {
     ).register(app);
 
     new controller.Map(
-        app,
         authApi,
         pgConnection,
         templateMaps,
@@ -190,7 +180,6 @@ module.exports = function(serverOptions) {
     ).register(app);
 
     new controller.NamedMaps(
-        app,
         authApi,
         pgConnection,
         namedMapProviderCache,
@@ -201,7 +190,7 @@ module.exports = function(serverOptions) {
         metadataBackend
     ).register(app);
 
-    new controller.NamedMapsAdmin(templateMaps, authApi, pgConnection).register(app);
+    new controller.NamedMapsAdmin(authApi, pgConnection, templateMaps).register(app);
 
     new controller.ServerInfo().register(app);
 

npm-shrinkwrap.json

@@ -1,6 +1,6 @@
 {
   "name": "windshaft-cartodb",
-  "version": "2.14.0",
+  "version": "2.14.1",
   "dependencies": {
     "body-parser": {
       "version": "1.14.1",
@@ -441,7 +441,7 @@
             },
             "inherits": {
               "version": "2.0.1",
-              "from": "inherits@~2.0.1",
+              "from": "inherits@2",
               "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.1.tgz"
             }
           }
@@ -827,8 +827,8 @@
       "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.6.0.tgz"
     },
     "windshaft": {
-      "version": "1.0.0",
-      "from": "windshaft@~1.0.0",
+      "version": "1.0.1",
+      "from": "windshaft@~1.0.1",
       "dependencies": {
         "mapnik": {
           "version": "1.4.15-cdb2",

package.json

@@ -1,7 +1,7 @@
 {
     "private": true,
     "name": "windshaft-cartodb",
-    "version": "2.14.0",
+    "version": "2.14.1",
     "description": "A map tile server for CartoDB",
     "keywords": [
       "cartodb"
@@ -29,7 +29,7 @@
         "node-statsd": "~0.0.7",
         "underscore"  : "~1.6.0",
         "dot": "~1.0.2",
-        "windshaft": "~1.0.0",
+        "windshaft": "~1.0.1",
         "step": "~0.0.6",
         "queue-async": "~1.0.7",
         "request": "~2.62.0",

test/acceptance/multilayer_server.js

@@ -333,7 +333,7 @@ describe('tests from old api translated to multilayer', function() {
                 assert.ok(!res.headers.hasOwnProperty('x-cache-channel'));
 
                 // TODO when affected tables query makes the request to fail layergroup should be removed
-                keysToDelete['map_cfg|f14693f2d7b6dcf4629724b3d1efe22d'] = 0;
+                keysToDelete['map_cfg|4fb7bd7008322ce66f22d20aebba1ab0'] = 0;
                 keysToDelete['user:localhost:mapviews:global'] = 5;
 
                 var parsed = JSON.parse(res.body);

test/unit/cartodb/cdb_request.test.js

@@ -57,4 +57,28 @@ describe('req2params', function() {
 
         assert.equal(user, undefined);
     });
+
+    it('should not fail for undefined host header', function() {
+        var userFromHostConfig = global.environment.user_from_host;
+        global.environment.user_from_host = null;
+
+        var cdbRequest = new CdbRequest();
+        var user = cdbRequest.userByReq(createRequest(undefined));
+
+        global.environment.user_from_host = userFromHostConfig;
+
+        assert.equal(user, undefined);
+    });
+
+    it('should not fail for null host header', function() {
+        var userFromHostConfig = global.environment.user_from_host;
+        global.environment.user_from_host = null;
+
+        var cdbRequest = new CdbRequest();
+        var user = cdbRequest.userByReq(createRequest(null));
+
+        global.environment.user_from_host = userFromHostConfig;
+
+        assert.equal(user, undefined);
+    });
 });

test/unit/cartodb/ported/tile_stats.test.js

@@ -1,18 +1,13 @@
 require('../../../support/test_helper.js');
 
 var assert = require('assert');
-var cartodbServer = require('../../../../lib/cartodb/server');
-var serverOptions = require('../../../../lib/cartodb/server_options');
-var StatsClient = require('../../../../lib/cartodb/stats/client');
 
 var LayergroupController = require('../../../../lib/cartodb/controllers/layergroup');
 
 describe('tile stats', function() {
 
-    var statsClientGetInstanceFn = StatsClient.getInstance;
-
     after(function() {
-        StatsClient.getInstance = statsClientGetInstanceFn;
+        global.statsClient = null;
     });
 
 
@@ -28,7 +23,7 @@ describe('tile stats', function() {
             }
         });
 
-        var layergroupController = new LayergroupController(cartodbServer(serverOptions));
+        var layergroupController = new LayergroupController();
 
         var reqMock = {
             params: {
@@ -70,7 +65,7 @@ describe('tile stats', function() {
             send: function() {}
         };
 
-        var layergroupController = new LayergroupController(cartodbServer(serverOptions));
+        var layergroupController = new LayergroupController();
 
         layergroupController.finalizeGetTileOrGrid('Another error happened', reqMock, resMock, null, null);
 
@@ -79,9 +74,7 @@ describe('tile stats', function() {
     });
 
     function mockStatsClientGetInstance(instance) {
-        StatsClient.getInstance = function() {
-            return instance;
-        };
+        global.statsClient = instance;
     }
 
 });