From de275bfc500c4dbba80342ff23d967d8b19fe0a5 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 18 Jul 2012 11:00:24 +0200 Subject: [PATCH] Delegate user permission to PostgreSQL (closes #18) If the request is authenticated (with map_key) then we log as the database owner, otherwise we log as the default user. The default user is now "publicuser" by default. Raises dependency on Windshaft to 0.4.9+, to get the grainstore version allowing override of database username. Add test for req2params function, particularly authentication, Add test for authenticated / unauthenticated access --- config/environments/development.js | 3 +- config/environments/production.js | 3 +- config/environments/test.js | 3 +- lib/cartodb/carto_data.js | 28 +++++------- package.json | 3 +- run_tests.sh | 1 + test/acceptance/cache_validator.js | 2 +- test/acceptance/server.js | 31 ++++++++++++- test/prepare_db.sh | 1 + test/sql/gadm4.sql | 4 +- test/sql/windshaft.test.sql | 37 +++++++++++++--- test/{ => support}/test_helper.js | 4 +- test/unit/cartodb/req2params.test.js | 66 ++++++++++++++++++++++++++++ 13 files changed, 150 insertions(+), 36 deletions(-) rename test/{ => support}/test_helper.js (59%) create mode 100644 test/unit/cartodb/req2params.test.js diff --git a/config/environments/development.js b/config/environments/development.js index b1643a5c..790c6630 100644 --- a/config/environments/development.js +++ b/config/environments/development.js @@ -6,7 +6,8 @@ var config = { ,cache_enabled: false ,postgres: { type: "postgis", - user: "tileuser", + user: "publicuser", + db_user: 'development_cartodb_user_<%= user_id %>', host: '127.0.0.1', port: 5432, extent: "-20005048.4188,-20005048.4188,20005048.4188,20005048.4188", diff --git a/config/environments/production.js b/config/environments/production.js index 9df860c1..b747b209 100644 --- a/config/environments/production.js +++ b/config/environments/production.js @@ -5,7 +5,8 @@ var config = { ,enable_cors: true ,cache_enabled: true ,postgres: { - user: "tileuser", + user: "publicuser", + db_user: 'cartodb_user_<%= user_id %>', host: '127.0.0.1', port: 6432, extent: "-20005048.4188,-20005048.4188,20005048.4188,20005048.4188", diff --git a/config/environments/test.js b/config/environments/test.js index 5b574a81..4abba30b 100644 --- a/config/environments/test.js +++ b/config/environments/test.js @@ -5,7 +5,8 @@ var config = { ,enable_cors: true ,cache_enabled: false ,postgres: { - user: "tileuser", + user: "publicuser", + db_user: 'test_cartodb_user_<%= user_id %>', host: '127.0.0.1', port: 5432, srid: 4326, diff --git a/lib/cartodb/carto_data.js b/lib/cartodb/carto_data.js index 29569c46..1ec2c518 100644 --- a/lib/cartodb/carto_data.js +++ b/lib/cartodb/carto_data.js @@ -82,26 +82,18 @@ module.exports = function() { }, function checkIfInternal(err, check_result){ if (err) throw err; - - if (check_result === 1){ - callback(err, true); // Internal access so early exit with access. + if (check_result === 1) { + // authorized by key, login as db owner + that.getId(req, function(err, user_id) { + if (err) throw new Error(err); + var dbuser = _.template(global.settings.postgres.db_user, {user_id: user_id}); + _.extend(req.params, {dbuser:dbuser}); + callback(err, true); + }); } else { - return true; // continue to check if the table is public/private + // log to db as unprivileged user + callback(err, true); } - }, - function (err, data){ - if (err) throw err; - that.getDatabase(req, this); - }, - function(err, data){ - if (err) throw err; - var redisKey = _.template(that.table_key, {database_name: data, table_name: req.params.table}); - - that.retrieve(that.table_metadata_db, redisKey, 'privacy', this); - }, - function(err, data){ - if (err) throw err; - callback(err, data); } ); }; diff --git a/package.json b/package.json index 1e7eb07d..85f4e3e4 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,8 @@ "cluster": "0.6.4", "node-varnish": "0.1.1", "underscore" : "1.1.x", - "windshaft" : "0.4.8", + "grainstore" : "~0.3.0", + "windshaft" : "~0.4.9", "step": "0.0.x", "generic-pool": "1.0.x", "redis": "0.6.7", diff --git a/run_tests.sh b/run_tests.sh index 70748883..635d29dd 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -34,6 +34,7 @@ PATH=node_modules/.bin/:$PATH echo "Running tests" mocha -u tdd \ test/unit/cartodb/redis_pool.test.js \ + test/unit/cartodb/req2params.test.js \ test/acceptance/cache_validator.js \ test/acceptance/server.js diff --git a/test/acceptance/cache_validator.js b/test/acceptance/cache_validator.js index 9599b680..06a5ba3d 100644 --- a/test/acceptance/cache_validator.js +++ b/test/acceptance/cache_validator.js @@ -1,6 +1,6 @@ var assert = require('../support/assert'); var net = require('net'); -require(__dirname + '/../test_helper'); +require(__dirname + '/../support/test_helper'); var CacheValidator = require(__dirname + '/../../lib/cartodb/cache_validator'); var tests = module.exports = {}; diff --git a/test/acceptance/server.js b/test/acceptance/server.js index 92386ee5..d8bd3448 100644 --- a/test/acceptance/server.js +++ b/test/acceptance/server.js @@ -2,7 +2,7 @@ var assert = require('../support/assert'); var tests = module.exports = {}; var _ = require('underscore'); var querystring = require('querystring'); -require(__dirname + '/../test_helper'); +require(__dirname + '/../support/test_helper'); var CartodbWindshaft = require(__dirname + '/../../lib/cartodb/cartodb_windshaft'); var serverOptions = require(__dirname + '/../../lib/cartodb/server_options'); @@ -198,6 +198,33 @@ suite('server', function() { headers: { 'Content-Type': 'image/png' } }, function() { done(); }); }); - + + test("get'ing a tile with data from private table should succeed when authenticated", function(done){ + // NOTE: may fail if grainstore < 0.3.0 is used by Windshaft + var sql = querystring.stringify({sql: "SELECT * FROM test_table_private_1", map_key: 1234}) + assert.response(server, { + headers: {host: 'vizzuality.localhost.lan'}, + url: '/tiles/gadm4/6/31/24.png?' + sql, + method: 'GET' + },{ + status: 200, + headers: { 'Content-Type': 'image/png' } + }, function() { done(); }); + }); + + test("get'ing a tile with data from private table should fail when unauthenticated", function(done){ + var sql = querystring.stringify({ + sql: "SELECT * FROM test_table_private_1", + cache_buster:2 // this is to avoid getting the cached response + }); + assert.response(server, { + headers: {host: 'vizzuality.localhost.lan'}, + url: '/tiles/gadm4/6/31/24.png?' + sql, + method: 'GET' + },{ + status: 500, + }, function() { done(); }); + }); + }); diff --git a/test/prepare_db.sh b/test/prepare_db.sh index f7658093..3e3a0419 100755 --- a/test/prepare_db.sh +++ b/test/prepare_db.sh @@ -28,6 +28,7 @@ psql "${TEST_DB}" < ./sql/gadm4.sql echo "preparing redis..." echo "HSET rails:users:vizzuality id 1" | redis-cli -p ${REDIS_PORT} -n 5 echo 'HSET rails:users:vizzuality database_name "'"${TEST_DB}"'"' | redis-cli -p ${REDIS_PORT} -n 5 +echo "SADD rails:users:vizzuality:map_key 1234" | redis-cli -p ${REDIS_PORT} -n 5 echo 'HSET rails:'"${TEST_DB}"':my_table infowindow "this, that, the other"' | redis-cli -p ${REDIS_PORT} -n 0 echo "Finished preparing data. Run tests with expresso." diff --git a/test/sql/gadm4.sql b/test/sql/gadm4.sql index aba12fa5..4f257192 100644 --- a/test/sql/gadm4.sql +++ b/test/sql/gadm4.sql @@ -23,7 +23,7 @@ CREATE TABLE gadm4 ( ); GRANT ALL ON TABLE gadm4 TO postgres; -GRANT ALL ON TABLE gadm4 TO tileuser; +GRANT ALL ON TABLE gadm4 TO publicuser; CREATE SEQUENCE gadm4_seq START WITH 1 INCREMENT BY 1 @@ -100,5 +100,5 @@ CREATE INDEX bdll25_provincias_4326_2_the_geom_webmercator_idx ON gadm4 USING gi -- development_cartodb_user_3 role CREATE USER development_cartodb_user_3; GRANT ALL ON TABLE gadm4 TO development_cartodb_user_3; -GRANT SELECT ON TABLE gadm4 TO tileuser; +GRANT SELECT ON TABLE gadm4 TO publicuser; diff --git a/test/sql/windshaft.test.sql b/test/sql/windshaft.test.sql index 25a9899f..d4aaba32 100644 --- a/test/sql/windshaft.test.sql +++ b/test/sql/windshaft.test.sql @@ -15,8 +15,11 @@ SET search_path = public, pg_catalog; SET default_tablespace = ''; SET default_with_oids = false; --- tileuser role -CREATE USER tileuser; +-- publicuser role +CREATE USER publicuser; + +-- db owner role +CREATE USER test_cartodb_user_1; -- first table CREATE TABLE test_table ( @@ -59,8 +62,8 @@ ALTER TABLE ONLY test_table ADD CONSTRAINT test_table_pkey PRIMARY KEY (cartodb_ CREATE INDEX test_table_the_geom_idx ON test_table USING gist (the_geom); CREATE INDEX test_table_the_geom_webmercator_idx ON test_table USING gist (the_geom_webmercator); -GRANT ALL ON TABLE test_table TO postgres; -GRANT ALL ON TABLE test_table TO tileuser; +GRANT ALL ON TABLE test_table TO test_cartodb_user_1; +GRANT SELECT ON TABLE test_table TO publicuser; -- second table CREATE TABLE test_table_2 ( @@ -103,7 +106,8 @@ ALTER TABLE ONLY test_table_2 ADD CONSTRAINT test_table_2_pkey PRIMARY KEY (cart CREATE INDEX test_table_2_the_geom_idx ON test_table_2 USING gist (the_geom); CREATE INDEX test_table_2_the_geom_webmercator_idx ON test_table_2 USING gist (the_geom_webmercator); -GRANT ALL ON TABLE test_table_2 TO tileuser; +GRANT ALL ON TABLE test_table_2 TO test_cartodb_user_1; +GRANT SELECT ON TABLE test_table_2 TO publicuser; -- third table CREATE TABLE test_table_3 ( @@ -146,5 +150,24 @@ ALTER TABLE ONLY test_table_3 ADD CONSTRAINT test_table_3_pkey PRIMARY KEY (cart CREATE INDEX test_table_3_the_geom_idx ON test_table_3 USING gist (the_geom); CREATE INDEX test_table_3_the_geom_webmercator_idx ON test_table_3 USING gist (the_geom_webmercator); -GRANT ALL ON TABLE test_table_3 TO postgres; -GRANT ALL ON TABLE test_table_3 TO tileuser; +GRANT ALL ON TABLE test_table_3 TO test_cartodb_user_1; +GRANT SELECT ON TABLE test_table_3 TO publicuser; + +-- private table +CREATE TABLE test_table_private_1 ( + updated_at timestamp without time zone DEFAULT now(), + created_at timestamp without time zone DEFAULT now(), + cartodb_id integer NOT NULL, + name character varying, + address character varying, + the_geom geometry, + the_geom_webmercator geometry, + CONSTRAINT enforce_dims_the_geom CHECK ((st_ndims(the_geom) = 2)), + CONSTRAINT enforce_dims_the_geom_webmercator CHECK ((st_ndims(the_geom_webmercator) = 2)), + CONSTRAINT enforce_geotype_the_geom CHECK (((geometrytype(the_geom) = 'POINT'::text) OR (the_geom IS NULL))), + CONSTRAINT enforce_geotype_the_geom_webmercator CHECK (((geometrytype(the_geom_webmercator) = 'POINT'::text) OR (the_geom_webmercator IS NULL))), + CONSTRAINT enforce_srid_the_geom CHECK ((st_srid(the_geom) = 4326)), + CONSTRAINT enforce_srid_the_geom_webmercator CHECK ((st_srid(the_geom_webmercator) = 3857)) +); + +GRANT ALL ON TABLE test_table_private_1 TO test_cartodb_user_1; diff --git a/test/test_helper.js b/test/support/test_helper.js similarity index 59% rename from test/test_helper.js rename to test/support/test_helper.js index 4ab4300a..3c59cac0 100644 --- a/test/test_helper.js +++ b/test/support/test_helper.js @@ -8,8 +8,8 @@ var _ = require('underscore'); // set environment specific variables -global.settings = require(__dirname + '/../config/settings'); -global.environment = require(__dirname + '/../config/environments/test'); +global.settings = require(__dirname + '/../../config/settings'); +global.environment = require(__dirname + '/../../config/environments/test'); _.extend(global.settings, global.environment); diff --git a/test/unit/cartodb/req2params.test.js b/test/unit/cartodb/req2params.test.js new file mode 100644 index 00000000..ab417993 --- /dev/null +++ b/test/unit/cartodb/req2params.test.js @@ -0,0 +1,66 @@ +var assert = require('assert') + , _ = require('underscore') + , redis = require('redis') + , test_helper = require('../../support/test_helper') + , tests = module.exports = {}; + +suite('req2params', function() { + + // configure redis pool instance to use in tests + var opts = require('../../../lib/cartodb/server_options'); + + test('can be found in server_options', function(){ + assert.ok(_.isFunction(opts.req2params)); + }); + + test('cleans up request', function(done){ + opts.req2params({headers: { host:'h1' }, query: {dbuser:'hacker',dbname:'secret'}}, function(err, req) { + if ( err ) { console.log(err); throw new Error(err); } + 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 have interactivity'); + assert.ok(_.isNull(req.params.dbname), 'could forge dbname'); + assert.ok(!req.params.hasOwnProperty('dbuser'), 'could inject dbuser'); + done(); + }); + }); + + test('sets dbname from redis metadata', function(done){ + opts.req2params({headers: { host:'vizzuality' }, query: {} }, function(err, req) { + if ( err ) { console.log(err); throw new Error(err); } + //console.dir(req); + 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 have interactivity'); + // database_name for user "vizzuality" (see test/support/prepare_db.sh) + assert.equal(req.params.dbname, 'cartodb_test_user_1_db'); + // unauthenticated request gets no dbuser + assert.ok(!req.params.hasOwnProperty('dbuser'), 'could inject dbuser'); + done(); + }); + }); + + test('sets also dbuser for authenticated requests', function(done){ + opts.req2params({headers: { host:'vizzuality' }, query: {map_key: '1234'} }, function(err, req) { + if ( err ) { console.log(err); throw new Error(err); } + //console.dir(req); + 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 have interactivity'); + // database_name for user "vizzuality" (see test/support/prepare_db.sh) + assert.equal(req.params.dbname, 'cartodb_test_user_1_db'); + // id for user "vizzuality" (see test/support/prepare_db.sh) + assert.equal(req.params.dbuser, 'test_cartodb_user_1'); + + opts.req2params({headers: { host:'vizzuality' }, query: {map_key: '1235'} }, function(err, req) { + // wrong key resets params to no user + assert.ok(!req.params.hasOwnProperty('dbuser'), 'could inject dbuser'); + done(); + }); + }); + }); + +});