From fe6e915c0d4807ae0c7f07d752b3715c9a1027a3 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 7 Feb 2014 18:00:13 +0100 Subject: [PATCH] Always set database access parameters from req2params Fixes privileged database access from unauthorized users while fetching torque tiles or feature attributes (unreleased feature). Closes #132. Includes testcase, which closes #119 --- lib/cartodb/cartodb_windshaft.js | 2 +- lib/cartodb/server_options.js | 11 ++ test/acceptance/templates.js | 194 +++++++++++++++++++++++++++ test/unit/cartodb/req2params.test.js | 6 +- 4 files changed, 209 insertions(+), 4 deletions(-) diff --git a/lib/cartodb/cartodb_windshaft.js b/lib/cartodb/cartodb_windshaft.js index 92750536..98b3f574 100644 --- a/lib/cartodb/cartodb_windshaft.js +++ b/lib/cartodb/cartodb_windshaft.js @@ -23,7 +23,7 @@ var CartodbWindshaft = function(serverOptions) { serverOptions.beforeStateChange = function(req, callback) { var err = null; - if ( ! req.params.hasOwnProperty('dbuser') ) { + if ( ! req.params.hasOwnProperty('_authorizedByApiKey') ) { 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 458ffd23..f79551ca 100644 --- a/lib/cartodb/server_options.js +++ b/lib/cartodb/server_options.js @@ -527,6 +527,8 @@ console.log("Checking authorization from signer " + signer + " for resource " + return; } + _.extend(req.params, { _authorizedByApiKey: true }); + // authorized by api key, login as the given username and stop that.setDBAuth(user, req.params, function(err) { callback(err, true); // authorized (or error) @@ -656,6 +658,15 @@ console.log("Checking authorization from signer " + signer + " for resource " + if (!_.isNull(data)) _.extend(req.params, {geom_type: data}); + // 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 + }); + that.addCacheChannel(req, function(err) { if (req.profiler) req.profiler.done('addCacheChannel'); callback(err, req); diff --git a/test/acceptance/templates.js b/test/acceptance/templates.js index 364d1d3d..388ce5ee 100644 --- a/test/acceptance/templates.js +++ b/test/acceptance/templates.js @@ -950,6 +950,200 @@ suite('template_api', function() { ); }); + test("can instanciate a template with torque layer by id", function(done) { + + // This map fetches data from a private table + var template = { + version: '0.0.1', + name: 'acceptance1', + auth: { method: 'token', valid_tokens: ['valid1','valid2'] }, + layergroup: { + version: '1.1.0', + layers: [ + { type: 'torque', options: { + sql: "select * from test_table_private_1 LIMIT 0", + cartocss: "Map { -torque-frame-count:1; -torque-resolution:1; -torque-aggregation-function:'count(*)'; -torque-time-attribute:'updated_at'; }" + } } + ] + } + }; + + var template_params = {}; + + var errors = []; + var expected_failure = false; + var tpl_id; + var layergroupid; + Step( + function postTemplate(err, res) + { + var next = this; + var post_request = { + url: '/tiles/template?api_key=1234', + method: 'POST', + headers: {host: 'localhost', 'Content-Type': 'application/json' }, + data: JSON.stringify(template) + } + assert.response(server, post_request, {}, + function(res) { next(null, res); }); + }, + function instanciateNoAuth(err, res) + { + if ( err ) throw err; + assert.equal(res.statusCode, 200, res.body); + var parsed = JSON.parse(res.body); + assert.ok(parsed.hasOwnProperty('template_id'), + "Missing 'template_id' from response body: " + res.body); + tpl_id = parsed.template_id; + var post_request = { + url: '/tiles/template/' + tpl_id, + method: 'POST', + headers: {host: 'localhost', 'Content-Type': 'application/json' }, + data: JSON.stringify(template_params) + } + var next = this; + assert.response(server, post_request, {}, + function(res) { next(null, res); }); + }, + function instanciateAuth(err, res) + { + if ( err ) throw err; + assert.equal(res.statusCode, 401, + 'Unexpected success instanciating template with no auth: ' + + res.statusCode + ': ' + res.body); + var parsed = JSON.parse(res.body); + assert.ok(parsed.hasOwnProperty('error'), + "Missing 'error' from response body: " + res.body); + assert.ok(parsed.error.match(/unauthorized/i), + 'Unexpected error for unauthorized instance : ' + parsed.error); + var post_request = { + url: '/tiles/template/' + tpl_id + '?auth_token=valid2', + method: 'POST', + headers: {host: 'localhost', 'Content-Type': 'application/json' }, + data: JSON.stringify(template_params) + } + var next = this; + assert.response(server, post_request, {}, + function(res) { next(null, res); }); + }, + function fetchTileNoAuth(err, res) { + if ( err ) throw err; + assert.equal(res.statusCode, 200, + 'Instantiating template: ' + res.statusCode + ': ' + res.body); + var parsed = JSON.parse(res.body); + assert.ok(parsed.hasOwnProperty('layergroupid'), + "Missing 'layergroupid' from response body: " + res.body); + layergroupid = parsed.layergroupid; + assert.ok(layergroupid.match(/^localhost@/), + "Returned layergroupid does not start with signer name: " + + layergroupid); + assert.ok(parsed.hasOwnProperty('last_updated'), + "Missing 'last_updated' from response body: " + res.body); + // TODO: check value of last_updated ? + var get_request = { + url: '/tiles/layergroup/' + layergroupid + ':cb0/0/0/0/0.json.torque', + method: 'GET', + headers: {host: 'localhost' }, + encoding: 'binary' + } + var next = this; + assert.response(server, get_request, {}, + function(res) { next(null, res); }); + }, + function fetchTileAuth(err, res) { + if ( err ) throw err; + assert.equal(res.statusCode, 401, + 'Fetching tile with no auth: ' + res.statusCode + ': ' + res.body); + var parsed = JSON.parse(res.body); + assert.ok(parsed.hasOwnProperty('error'), + "Missing 'error' from response body: " + res.body); + assert.ok(parsed.error.match(/permission denied/i), + 'Unexpected error for unauthorized instance ' + + '(expected /permission denied): ' + parsed.error); + var get_request = { + url: '/tiles/layergroup/' + layergroupid + ':cb1/0/0/0/0.json.torque?auth_token=valid1', + method: 'GET', + headers: {host: 'localhost' }, + encoding: 'binary' + } + var next = this; + assert.response(server, get_request, {}, + function(res) { next(null, res); }); + }, + function checkTile(err, res) { + if ( err ) throw err; + assert.equal(res.statusCode, 200, + 'Unexpected error for authorized instance: ' + + res.statusCode + ' -- ' + res.body); + assert.equal(res.headers['content-type'], "application/json; charset=utf-8"); + return null; + }, + function deleteTemplate(err) + { + if ( err ) throw err; + var del_request = { + url: '/tiles/template/' + tpl_id + '?api_key=1234', + method: 'DELETE', + headers: {host: 'localhost'} + } + var next = this; + assert.response(server, del_request, {}, + function(res) { next(null, res); }); + }, + function fetchTileDeleted(err, res) { + if ( err ) throw err; + assert.equal(res.statusCode, 204, + 'Deleting template: ' + res.statusCode + ':' + res.body); + var get_request = { + url: '/tiles/layergroup/' + layergroupid + ':cb2/0/0/0/0.json.torque?auth_token=valid1', + method: 'GET', + headers: {host: 'localhost' }, + encoding: 'binary' + } + var next = this; + assert.response(server, get_request, {}, + function(res) { next(null, res); }); + }, + function checkTileDeleted(err, res) { + if ( err ) throw err; + assert.equal(res.statusCode, 401, + 'Unexpected statusCode fetch tile after signature revokal: ' + + res.statusCode + ':' + res.body); + var parsed = JSON.parse(res.body); + assert.ok(parsed.hasOwnProperty('error'), + "Missing 'error' from response body: " + res.body); + assert.ok(parsed.error.match(/permission denied/i), + 'Unexpected error for unauthorized access : ' + parsed.error); + return null; + }, + function finish(err) { + if ( err ) errors.push(err); + redis_client.keys("map_*|localhost", function(err, keys) { + if ( err ) errors.push(err.message); + var todrop = _.map(keys, function(m) { + if ( m.match(/^map_(tpl|crt)|/) ) + return m; + }); + if ( todrop.length ) { + errors.push(new Error("Unexpected keys in redis: " + todrop)); + redis_client.del(todrop, function(err) { + if ( err ) errors.push(err.message); + if ( errors.length ) { + done(new Error(errors)); + } + else done(null); + }); + } else { + if ( errors.length ) { + done(new Error(errors)); + } + else done(null); + } + }); + } + ); + }); + test("can instanciate a template by id with open auth", function(done) { // This map fetches data from a private table diff --git a/test/unit/cartodb/req2params.test.js b/test/unit/cartodb/req2params.test.js index e10bd19e..bd2cdb6a 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, 'test_cartodb_user_1_db', 'could forge dbname: '+ req.params.dbname); - assert.ok(!req.params.hasOwnProperty('dbuser'), 'could inject dbuser ('+req.params.dbuser+')'); + assert.ok(req.params.dbuser === 'testpublicuser', 'could inject dbuser ('+req.params.dbuser+')'); done(); }); }); @@ -37,7 +37,7 @@ suite('req2params', function() { // database_name for user "localhost" (see test/support/prepare_db.sh) assert.equal(req.params.dbname, 'test_cartodb_user_1_db'); // unauthenticated request gets no dbuser - assert.ok(!req.params.hasOwnProperty('dbuser'), 'could inject dbuser ('+req.params.dbuser+')'); + assert.ok(req.params.dbuser === 'testpublicuser', 'could inject dbuser ('+req.params.dbuser+')'); done(); }); }); @@ -57,7 +57,7 @@ suite('req2params', function() { opts.req2params({headers: { host:'localhost' }, query: {map_key: '1235'} }, function(err, req) { // wrong key resets params to no user - assert.ok(!req.params.hasOwnProperty('dbuser'), 'could inject dbuser ('+req.params.dbuser+')'); + assert.ok(req.params.dbuser === 'testpublicuser', 'could inject dbuser ('+req.params.dbuser+')'); done(); }); });