From eb51d18012b080744bc5ed02a60d005cf70221bd Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 11 Nov 2013 00:50:03 +0100 Subject: [PATCH] Add support for specifying database connection passwords --- NEWS.md | 4 ++ config/environments/development.js.example | 2 + config/environments/production.js.example | 2 + config/environments/staging.js.example | 2 + config/environments/test.js.example | 4 +- lib/cartodb/carto_data.js | 6 ++- lib/cartodb/server_options.js | 5 +++ package.json | 2 +- test/acceptance/multilayer.js | 16 ++++---- test/acceptance/server.js | 10 ++--- test/support/prepare_db.sh | 45 ++++++++++++++++++++-- test/support/sql/gadm4.sql | 10 ++--- test/support/sql/windshaft.test.sql | 22 ++++++----- test/unit/cartodb/req2params.test.js | 6 +-- 14 files changed, 98 insertions(+), 38 deletions(-) diff --git a/NEWS.md b/NEWS.md index 68c090bc..1486df60 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,7 @@ +1.5.0 -- 2013-MM-DD + +* Add support for specifying database connection passwords + 1.4.1 -- 2013-11-08 ------------------- diff --git a/config/environments/development.js.example b/config/environments/development.js.example index 32bbcde7..9d3e704a 100644 --- a/config/environments/development.js.example +++ b/config/environments/development.js.example @@ -11,11 +11,13 @@ var config = { ,cache_enabled: false ,log_format: '[:date] :req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler])' ,postgres_auth_user: 'development_cartodb_user_<%= user_id %>' + ,postgres_auth_pass: 'development_cartodb_user_<%= user_id %>_pass' ,postgres: { // Parameters to pass to datasource plugin of mapnik // See http://github.com/mapnik/mapnik/wiki/PostGIS type: "postgis", user: "publicuser", + password: "public", host: '127.0.0.1', port: 5432, extent: "-20037508.3,-20037508.3,20037508.3,20037508.3", diff --git a/config/environments/production.js.example b/config/environments/production.js.example index 1cc96b8b..2270aab5 100644 --- a/config/environments/production.js.example +++ b/config/environments/production.js.example @@ -11,10 +11,12 @@ var config = { ,cache_enabled: true ,log_format: '[:date] :req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler])' ,postgres_auth_user: 'cartodb_user_<%= user_id %>' + ,postgres_auth_pass: 'cartodb_user_<%= user_id %>_pass' ,postgres: { // Parameters to pass to datasource plugin of mapnik // See http://github.com/mapnik/mapnik/wiki/PostGIS user: "publicuser", + password: "public", host: '127.0.0.1', port: 6432, extent: "-20037508.3,-20037508.3,20037508.3,20037508.3", diff --git a/config/environments/staging.js.example b/config/environments/staging.js.example index 39cd1ebb..bd1537f8 100644 --- a/config/environments/staging.js.example +++ b/config/environments/staging.js.example @@ -11,10 +11,12 @@ var config = { ,cache_enabled: true ,log_format: '[:date] :req[X-Real-IP] :method :req[Host]:url :status :response-time ms (:res[X-Tiler-Profiler]) -> :res[Content-Type]' ,postgres_auth_user: 'cartodb_staging_user_<%= user_id %>' + ,postgres_auth_pass: 'cartodb_staging_user_<%= user_id %>_pass' ,postgres: { // Parameters to pass to datasource plugin of mapnik // See http://github.com/mapnik/mapnik/wiki/PostGIS user: "publicuser", + password: "public", host: '127.0.0.1', port: 6432, extent: "-20037508.3,-20037508.3,20037508.3,20037508.3", diff --git a/config/environments/test.js.example b/config/environments/test.js.example index 69452e0a..1f5f1a79 100644 --- a/config/environments/test.js.example +++ b/config/environments/test.js.example @@ -11,10 +11,12 @@ var config = { ,cache_enabled: false ,log_format: '[:date] :req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler])' ,postgres_auth_user: 'test_cartodb_user_<%= user_id %>' + ,postgres_auth_pass: 'test_cartodb_user_<%= user_id %>_pass' ,postgres: { // Parameters to pass to datasource plugin of mapnik // See http://github.com/mapnik/mapnik/wiki/PostGIS - user: "publicuser", + user: "testpublicuser", + password: "public", host: '127.0.0.1', port: 5432, extent: "-20037508.3,-20037508.3,20037508.3,20037508.3", diff --git a/lib/cartodb/carto_data.js b/lib/cartodb/carto_data.js index ad5296b6..c817bfa8 100644 --- a/lib/cartodb/carto_data.js +++ b/lib/cartodb/carto_data.js @@ -141,9 +141,13 @@ module.exports = function() { if (check_result === 1) { // authorized by key, login as db owner that.getId(req, function(err, user_id) { - if (err) throw err; + if (err) { callback(err); return; } var dbuser = _.template(global.settings.postgres_auth_user, {user_id: user_id}); _.extend(req.params, {dbuser:dbuser}); + if ( global.settings.postgres_auth_pass ) { + var dbpass = _.template(global.settings.postgres_auth_pass, {user_id: user_id}); + _.extend(req.params, {dbpassword:dbpass}); + } callback(err, true); }); } else { diff --git a/lib/cartodb/server_options.js b/lib/cartodb/server_options.js index 5f5dc892..e4b1b630 100644 --- a/lib/cartodb/server_options.js +++ b/lib/cartodb/server_options.js @@ -362,6 +362,11 @@ module.exports = function(){ 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"); + + var dbpass = req.params.dbpassword || global.settings.postgres.password; + if ( ! me.rx_dbpass ) me.rx_dbpass = /(<\/Parameter>)/g; + xml = xml.replace(me.rx_dbpass, "$1" + dbpass + "$2"); + callback(null, xml); } diff --git a/package.json b/package.json index dab62462..f804a37a 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "windshaft-cartodb", - "version": "1.4.1", + "version": "1.5.0", "description": "A map tile server for CartoDB", "url": "https://github.com/CartoDB/Windshaft-cartodb", "licenses": [{ diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index 516401a8..4741d5af 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -99,7 +99,7 @@ suite('multilayer', function() { // Check X-Cache-Channel cc = res.headers['x-cache-channel']; assert.ok(cc); - var dbname = 'cartodb_test_user_1_db' + var dbname = 'test_cartodb_user_1_db' assert.equal(cc.substring(0, dbname.length), dbname); var jsonquery = cc.substring(dbname.length+1); var sentquery = JSON.parse(jsonquery); @@ -156,7 +156,7 @@ suite('multilayer', function() { errors.push(err.message); console.log("Error: " + err); } - redis_client.keys("map_style|cartodb_test_user_1_db|~" + expected_token, function(err, matches) { + redis_client.keys("map_style|test_cartodb_user_1_db|~" + expected_token, function(err, matches) { if ( err ) errors.push(err.message); assert.equal(matches.length, 1, "Missing expected token " + expected_token + " from redis: " + matches); redis_client.del(matches, function(err) { @@ -229,7 +229,7 @@ suite('multilayer', function() { // Check X-Cache-Channel var cc = res.headers['x-cache-channel']; assert.ok(cc); - var dbname = 'cartodb_test_user_1_db' + var dbname = 'test_cartodb_user_1_db' assert.equal(cc.substring(0, dbname.length), dbname); var jsonquery = cc.substring(dbname.length+1); var sentquery = JSON.parse(jsonquery); @@ -262,7 +262,7 @@ suite('multilayer', function() { // Check X-Cache-Channel var cc = res.headers['x-cache-channel']; assert.ok(cc); - var dbname = 'cartodb_test_user_1_db' + var dbname = 'test_cartodb_user_1_db' assert.equal(cc.substring(0, dbname.length), dbname); var jsonquery = cc.substring(dbname.length+1); var sentquery = JSON.parse(jsonquery); @@ -321,7 +321,7 @@ suite('multilayer', function() { errors.push(err.message); console.log("Error: " + err); } - redis_client.keys("map_style|cartodb_test_user_1_db|~" + expected_token, function(err, matches) { + redis_client.keys("map_style|test_cartodb_user_1_db|~" + expected_token, function(err, matches) { if ( err ) errors.push(err.message); assert.equal(matches.length, 1, "Missing expected token " + expected_token + " from redis: " + matches); redis_client.del(matches, function(err) { @@ -417,7 +417,7 @@ suite('multilayer', function() { var next = this; // trip epoch expected_token = expected_token.split(':')[0]; - redis_client.keys("map_style|cartodb_test_user_1_db|~" + expected_token, function(err, matches) { + redis_client.keys("map_style|test_cartodb_user_1_db|~" + expected_token, function(err, matches) { redis_client.del(matches, next); }); }, @@ -525,7 +525,7 @@ suite('multilayer', function() { // Check X-Cache-Channel var cc = res.headers['x-cache-channel']; assert.ok(cc); - var dbname = 'cartodb_test_user_1_db' + var dbname = 'test_cartodb_user_1_db' assert.equal(cc.substring(0, dbname.length), dbname); next(err); }); @@ -613,7 +613,7 @@ suite('multilayer', function() { errors.push(err.message); console.log("Error: " + err); } - redis_client.keys("map_style|cartodb_test_user_1_db|~" + expected_token, function(err, matches) { + redis_client.keys("map_style|test_cartodb_user_1_db|~" + expected_token, function(err, matches) { if ( err ) errors.push(err.message); assert.equal(matches.length, 1, "Missing expected token " + expected_token + " from redis: " + matches); redis_client.del(matches, function(err) { diff --git a/test/acceptance/server.js b/test/acceptance/server.js index 11d8559a..fdfa2c50 100644 --- a/test/acceptance/server.js +++ b/test/acceptance/server.js @@ -102,7 +102,7 @@ suite('server', function() { method: 'GET' },{ status: 200, - headers: { 'X-Cache-Channel': 'cartodb_test_user_1_db:my_table' }, + headers: { 'X-Cache-Channel': 'test_cartodb_user_1_db:my_table' }, }, function(res) { var parsed = JSON.parse(res.body); assert.equal(parsed.style, _.template(default_style, {table: 'my_table'})); @@ -440,7 +440,7 @@ suite('server', function() { method: 'GET' },{ status: 200, - headers: { 'X-Cache-Channel': 'cartodb_test_user_1_db:my_tablez' }, + headers: { 'X-Cache-Channel': 'test_cartodb_user_1_db:my_tablez' }, body: '{"infowindow":null}' }, function() { done(); }); }); @@ -524,7 +524,7 @@ suite('server', function() { },{ status: 200, headers: { 'Content-Type': 'text/javascript; charset=utf-8; charset=utf-8', - 'X-Cache-Channel': 'cartodb_test_user_1_db:gadm4' } + 'X-Cache-Channel': 'test_cartodb_user_1_db:gadm4' } }, function() { done(); }); }); @@ -639,7 +639,7 @@ suite('server', function() { method: 'GET' },{ status: 200, - headers: { 'Content-Type': 'image/png', 'X-Cache-Channel': 'cartodb_test_user_1_db:gadm4' } + headers: { 'Content-Type': 'image/png', 'X-Cache-Channel': 'test_cartodb_user_1_db:gadm4' } }, function() { done(); }); }); @@ -989,7 +989,7 @@ suite('server', function() { var ct = res.headers['content-type']; assert.equal(ct, 'image/png'); var cc = res.headers['x-cache-channel']; - var dbname = 'cartodb_test_user_1_db' + var dbname = 'test_cartodb_user_1_db' assert.equal(cc.substring(0, dbname.length), dbname); var jsonquery = cc.substring(dbname.length+1); var sentquery = JSON.parse(jsonquery); diff --git a/test/support/prepare_db.sh b/test/support/prepare_db.sh index 6dbdfae9..60696072 100755 --- a/test/support/prepare_db.sh +++ b/test/support/prepare_db.sh @@ -16,17 +16,54 @@ die() { exit 1 } -TEST_DB="cartodb_test_user_1_db" +# This is where postgresql connection parameters are read from +TESTENV=../../config/environments/test.js +if [ \! -r ${TESTENV} ]; then + echo "Cannot read ${TESTENV}" >&2 + exit 1 +fi + +TESTUSERID=1 + +TESTUSER=`node -e "console.log(require('${TESTENV}').postgres_auth_user || '')"` +if test -z "$TESTUSER"; then + echo "Missing postgres_auth_user from ${TESTENV}" >&2 + exit 1 +fi +TESTUSER=`echo ${TESTUSER} | sed "s/<%= user_id %>/${TESTUSERID}/"` + +TESTPASS=`node -e "console.log(require('${TESTENV}').postgres_auth_pass || 'test')"` +# TODO: should postgres_auth_pass be optional ? +if test -z "$TESTPASS"; then + echo "Missing postgres_auth_pass from ${TESTENV}" >&2 + exit 1 +fi +TESTPASS=`echo ${TESTPASS} | sed "s/<%= user_id %>/${TESTUSERID}/"` + +#TESTUSER="cartodb_test_user_1" # TODO: extract from psotgres_auth_user +#TESTPASS="cartodb_test_user_1_pass" # TODO: extract from postgres_auth_pass +TEST_DB="${TESTUSER}_db" + if test -z "$REDIS_PORT"; then REDIS_PORT=6333; fi echo "preparing postgres..." dropdb "${TEST_DB}" createdb -Ttemplate_postgis -EUTF8 "${TEST_DB}" || die "Could not create test database" -psql "${TEST_DB}" < ./sql/windshaft.test.sql -psql "${TEST_DB}" < ./sql/gadm4.sql + +PUBLICUSER=`node -e "console.log(require('${TESTENV}').postgres.user || 'xxx')"` +PUBLICPASS=`node -e "console.log(require('${TESTENV}').postgres.password || 'xxx')"` +echo "PUBLICUSER: ${PUBLICUSER}" +echo "PUBLICPASS: ${PUBLICPASS}" + +cat sql/windshaft.test.sql sql/gadm4.sql | + sed "s/:PUBLICUSER/${PUBLICUSER}/" | + sed "s/:PUBLICPASS/${PUBLICPASS}/" | + sed "s/:TESTUSER/${TESTUSER}/" | + sed "s/:TESTPASS/${TESTPASS}/" | + psql ${TEST_DB} echo "preparing redis..." -echo "HSET rails:users:localhost id 1" | redis-cli -p ${REDIS_PORT} -n 5 +echo "HSET rails:users:localhost id ${TESTUSERID}" | redis-cli -p ${REDIS_PORT} -n 5 echo 'HSET rails:users:localhost database_name "'"${TEST_DB}"'"' | redis-cli -p ${REDIS_PORT} -n 5 echo "HSET rails:users:localhost map_key 1234" | redis-cli -p ${REDIS_PORT} -n 5 echo "SADD rails:users:localhost:map_key 1235" | redis-cli -p ${REDIS_PORT} -n 5 diff --git a/test/support/sql/gadm4.sql b/test/support/sql/gadm4.sql index 149b253e..3cd4881a 100644 --- a/test/support/sql/gadm4.sql +++ b/test/support/sql/gadm4.sql @@ -22,15 +22,15 @@ CREATE TABLE gadm4 ( the_geom_webmercator geometry(MultiPolygon,3857) ); -GRANT ALL ON TABLE gadm4 TO postgres; -GRANT ALL ON TABLE gadm4 TO publicuser; +--GRANT ALL ON TABLE gadm4 TO postgres; +GRANT ALL ON TABLE gadm4 TO :PUBLICUSER; CREATE SEQUENCE gadm4_seq START WITH 1 INCREMENT BY 1 NO MINVALUE NO MAXVALUE CACHE 1; -ALTER TABLE public.gadm4_seq OWNER TO postgres; +--ALTER TABLE public.gadm4_seq OWNER TO postgres; SELECT pg_catalog.setval('gadm4_seq', 58, false); ALTER TABLE gadm4 ALTER COLUMN cartodb_id SET DEFAULT nextval('gadm4_seq'::regclass); @@ -98,6 +98,6 @@ ALTER TABLE ONLY gadm4 CREATE INDEX bdll25_provincias_4326_2_the_geom_webmercator_idx ON gadm4 USING gist (the_geom_webmercator); -GRANT ALL ON TABLE gadm4 TO test_cartodb_user_1; -GRANT SELECT ON TABLE gadm4 TO publicuser; +GRANT ALL ON TABLE gadm4 TO :TESTUSER; +GRANT SELECT ON TABLE gadm4 TO :PUBLICUSER; diff --git a/test/support/sql/windshaft.test.sql b/test/support/sql/windshaft.test.sql index de858016..3ea0b615 100644 --- a/test/support/sql/windshaft.test.sql +++ b/test/support/sql/windshaft.test.sql @@ -15,11 +15,13 @@ SET search_path = public, pg_catalog; SET default_tablespace = ''; SET default_with_oids = false; --- publicuser role -CREATE USER publicuser; +-- public user role +DROP USER IF EXISTS :PUBLICUSER; +CREATE USER :PUBLICUSER WITH PASSWORD ':PUBLICPASS'; -- db owner role -CREATE USER test_cartodb_user_1; +DROP USER IF EXISTS :TESTUSER; +CREATE USER :TESTUSER WITH PASSWORD ':TESTPASS'; -- first table CREATE TABLE test_table ( @@ -63,8 +65,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 test_cartodb_user_1; -GRANT SELECT ON TABLE test_table TO publicuser; +GRANT ALL ON TABLE test_table TO :TESTUSER; +GRANT SELECT ON TABLE test_table TO :PUBLICUSER; -- second table CREATE TABLE test_table_2 ( @@ -108,8 +110,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 test_cartodb_user_1; -GRANT SELECT ON TABLE test_table_2 TO publicuser; +GRANT ALL ON TABLE test_table_2 TO :TESTUSER; +GRANT SELECT ON TABLE test_table_2 TO :PUBLICUSER; -- third table CREATE TABLE test_table_3 ( @@ -153,8 +155,8 @@ 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 test_cartodb_user_1; -GRANT SELECT ON TABLE test_table_3 TO publicuser; +GRANT ALL ON TABLE test_table_3 TO :TESTUSER; +GRANT SELECT ON TABLE test_table_3 TO :PUBLICUSER; -- private table CREATE TABLE test_table_private_1 ( @@ -173,4 +175,4 @@ CREATE TABLE test_table_private_1 ( 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; +GRANT ALL ON TABLE test_table_private_1 TO :TESTUSER; diff --git a/test/unit/cartodb/req2params.test.js b/test/unit/cartodb/req2params.test.js index d09351c3..b1e8b12e 100644 --- a/test/unit/cartodb/req2params.test.js +++ b/test/unit/cartodb/req2params.test.js @@ -20,7 +20,7 @@ suite('req2params', function() { 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.equal(req.params.dbname, 'cartodb_test_user_1_db', 'could forge dbname: '+ req.params.dbname); + 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+')'); done(); }); @@ -35,7 +35,7 @@ suite('req2params', function() { assert.ok(req.hasOwnProperty('params'), 'request has params'); assert.ok(req.params.hasOwnProperty('interactivity'), 'request params have interactivity'); // database_name for user "localhost" (see test/support/prepare_db.sh) - assert.equal(req.params.dbname, 'cartodb_test_user_1_db'); + 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+')'); done(); @@ -51,7 +51,7 @@ suite('req2params', function() { assert.ok(req.hasOwnProperty('params'), 'request has params'); assert.ok(req.params.hasOwnProperty('interactivity'), 'request params have interactivity'); // database_name for user "localhost" (see test/support/prepare_db.sh) - assert.equal(req.params.dbname, 'cartodb_test_user_1_db'); + assert.equal(req.params.dbname, 'test_cartodb_user_1_db'); // id for user "localhost" (see test/support/prepare_db.sh) assert.equal(req.params.dbuser, 'test_cartodb_user_1');