From 66a77cd2553b5e409c3dce96a3880fd01cf2118a Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 23 Sep 2013 11:59:03 +0200 Subject: [PATCH] Do not let anonymous requests use authorized renderer caches Puts dbuser in params, for correct use by Windshaft renderer cache. Before this fix, and after commit 1c9f63c9, the renderer cache key did not contain the db user. --- NEWS.md | 1 + lib/cartodb/carto_data.js | 2 +- lib/cartodb/cartodb_windshaft.js | 2 +- lib/cartodb/server_options.js | 2 +- test/acceptance/multilayer.js | 48 ++++++++++++++++++++++++++++ test/unit/cartodb/req2params.test.js | 6 ++-- 6 files changed, 55 insertions(+), 6 deletions(-) diff --git a/NEWS.md b/NEWS.md index aaff3a15..49b13e06 100644 --- a/NEWS.md +++ b/NEWS.md @@ -5,6 +5,7 @@ "[ zoom > 3]" CartoCSS snippets (note the space) * Fix backward compatibility handling of sqlapi.host configuration (#82) * Fix error for invalid text-name in CartoCSS (#81) +* Do not let anonymous requests use authorized renderer caches 1.3.4 ------ diff --git a/lib/cartodb/carto_data.js b/lib/cartodb/carto_data.js index 8b568c05..ad5296b6 100644 --- a/lib/cartodb/carto_data.js +++ b/lib/cartodb/carto_data.js @@ -143,7 +143,7 @@ module.exports = function() { that.getId(req, function(err, user_id) { if (err) throw err; var dbuser = _.template(global.settings.postgres_auth_user, {user_id: user_id}); - _.extend(req, {dbuser:dbuser}); + _.extend(req.params, {dbuser:dbuser}); callback(err, true); }); } else { diff --git a/lib/cartodb/cartodb_windshaft.js b/lib/cartodb/cartodb_windshaft.js index b20212e4..b436194a 100644 --- a/lib/cartodb/cartodb_windshaft.js +++ b/lib/cartodb/cartodb_windshaft.js @@ -17,7 +17,7 @@ var CartodbWindshaft = function(serverOptions) { serverOptions.beforeStateChange = function(req, callback) { var err = null; - if ( ! req.hasOwnProperty('dbuser') ) { + if ( ! req.params.hasOwnProperty('dbuser') ) { err = new Error("map state cannot be changed by unauthenticated request!"); } callback(err, req); diff --git a/lib/cartodb/server_options.js b/lib/cartodb/server_options.js index da4d5318..5f5dc892 100644 --- a/lib/cartodb/server_options.js +++ b/lib/cartodb/server_options.js @@ -359,7 +359,7 @@ module.exports = function(){ req.params.interactivity = req.params.interactivity || 'cartodb_id'; req.params.processXML = function(req, xml, callback) { - var dbuser = req.dbuser ? req.dbuser : global.settings.postgres.user; + var dbuser = req.params.dbuser || global.settings.postgres.user; if ( ! me.rx_dbuser ) me.rx_dbuser = /(<\/Parameter>)/g; xml = xml.replace(me.rx_dbuser, "$1" + dbuser + "$2"); callback(null, xml); diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index 6c5cd9a0..8349ef6a 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -559,6 +559,54 @@ suite('multilayer', function() { next(err); }); }, + function do_get_tile_unauth(err) + { + if ( err ) throw err; + var next = this; + assert.response(server, { + url: '/tiles/layergroup/' + expected_token + ':cb0/0/0/0.png', + method: 'GET', + headers: {host: 'localhost' }, + encoding: 'binary' + }, {}, function(res) { + assert.equal(res.statusCode, 401); + var re = RegExp('permission denied'); + assert.ok(res.body.match(re), 'No "permission denied" error: ' + res.body); + next(err); + }); + }, + function do_get_grid_layer0_unauth(err) + { + if ( err ) throw err; + var next = this; + assert.response(server, { + url: '/tiles/layergroup/' + expected_token + + '/0/0/0/0.grid.json', + headers: {host: 'localhost' }, + method: 'GET' + }, {}, function(res) { + assert.equal(res.statusCode, 401); + var re = RegExp('permission denied'); + assert.ok(res.body.match(re), 'No "permission denied" error: ' + res.body); + next(err); + }); + }, + function do_get_grid_layer1_unauth(err) + { + if ( err ) throw err; + var next = this; + assert.response(server, { + url: '/tiles/layergroup/' + expected_token + + '/1/0/0/0.grid.json', + headers: {host: 'localhost' }, + method: 'GET' + }, {}, function(res) { + assert.equal(res.statusCode, 401); + var re = RegExp('permission denied'); + assert.ok(res.body.match(re), 'No "permission denied" error: ' + res.body); + next(err); + }); + }, function finish(err) { var errors = []; if ( err ) { diff --git a/test/unit/cartodb/req2params.test.js b/test/unit/cartodb/req2params.test.js index 3b7bb845..d09351c3 100644 --- a/test/unit/cartodb/req2params.test.js +++ b/test/unit/cartodb/req2params.test.js @@ -21,7 +21,7 @@ suite('req2params', function() { assert.ok(req.hasOwnProperty('params'), 'request has params'); assert.ok(req.params.hasOwnProperty('interactivity'), 'request params have interactivity'); assert.equal(req.params.dbname, 'cartodb_test_user_1_db', 'could forge dbname: '+ req.params.dbname); - assert.ok(!req.hasOwnProperty('dbuser'), 'could inject dbuser ('+req.params.dbuser+')'); + assert.ok(!req.params.hasOwnProperty('dbuser'), 'could inject dbuser ('+req.params.dbuser+')'); done(); }); }); @@ -53,11 +53,11 @@ suite('req2params', function() { // database_name for user "localhost" (see test/support/prepare_db.sh) assert.equal(req.params.dbname, 'cartodb_test_user_1_db'); // id for user "localhost" (see test/support/prepare_db.sh) - assert.equal(req.dbuser, 'test_cartodb_user_1'); + assert.equal(req.params.dbuser, 'test_cartodb_user_1'); opts.req2params({headers: { host:'localhost' }, query: {map_key: '1235'} }, function(err, req) { // wrong key resets params to no user - assert.ok(!req.hasOwnProperty('dbuser'), 'could inject dbuser ('+req.params.dbuser+')'); + assert.ok(!req.params.hasOwnProperty('dbuser'), 'could inject dbuser ('+req.params.dbuser+')'); done(); }); });