From 1b63dcd4e56b54200807975698a52cccc9640453 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 7 Feb 2018 11:10:50 +0100 Subject: [PATCH 01/53] add api keys to prepare db for testing --- test/support/prepare_db.sh | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/support/prepare_db.sh b/test/support/prepare_db.sh index 03d97fc7..b40c0add 100755 --- a/test/support/prepare_db.sh +++ b/test/support/prepare_db.sh @@ -138,4 +138,26 @@ EOF fi +# API Key Master +cat < Date: Wed, 7 Feb 2018 11:59:00 +0100 Subject: [PATCH 02/53] add apikeys keys to be removed after each test --- test/support/test_helper.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/support/test_helper.js b/test/support/test_helper.js index 7d5c6a7c..74ab47db 100644 --- a/test/support/test_helper.js +++ b/test/support/test_helper.js @@ -113,7 +113,9 @@ afterEach(function(done) { 'rails:test_windshaft_cartodb_user_1_db:my_table': true, 'rails:users:localhost:map_key': true, 'rails:users:cartodb250user': true, - 'rails:users:localhost': true + 'rails:users:localhost': true, + 'api_keys:localhost:1234': true, + 'api_keys:localhost:default_public': true }; var databasesTasks = { 0: 'users', 5: 'meta'}; From c1535b1a124a370b4fd65e587c997f500e9ae1fd Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 7 Feb 2018 12:40:36 +0100 Subject: [PATCH 03/53] refactor setDBAuth to not use step --- lib/cartodb/backends/pg_connection.js | 57 ++++++++++++--------------- 1 file changed, 25 insertions(+), 32 deletions(-) diff --git a/lib/cartodb/backends/pg_connection.js b/lib/cartodb/backends/pg_connection.js index d98bb703..d6ce648b 100644 --- a/lib/cartodb/backends/pg_connection.js +++ b/lib/cartodb/backends/pg_connection.js @@ -21,42 +21,35 @@ module.exports = PgConnection; // @param callback function(err) // PgConnection.prototype.setDBAuth = function(username, params, callback) { - var self = this; - - var user_params = {}; var auth_user = global.environment.postgres_auth_user; var auth_pass = global.environment.postgres_auth_pass; - step( - function getId() { - self.metadataBackend.getUserId(username, this); - }, - function(err, user_id) { - assert.ifError(err); - user_params.user_id = user_id; - var dbuser = _.template(auth_user, user_params); - _.extend(params, {dbuser:dbuser}); - // skip looking up user_password if postgres_auth_pass - // doesn't contain the "user_password" label - if (!auth_pass || ! auth_pass.match(/\buser_password\b/) ) { - return null; - } - - self.metadataBackend.getUserDBPass(username, this); - }, - function(err, user_password) { - assert.ifError(err); - user_params.user_password = user_password; - if ( auth_pass ) { - var dbpass = _.template(auth_pass, user_params); - _.extend(params, {dbpassword:dbpass}); - } - return true; - }, - function finish(err) { - callback(err); + this.metadataBackend.getUserId(username, (err, userId) => { + if (err) { + return callback(err); } - ); + + const userParams = { + user_id: userId + }; + + _.extend(params, { dbuser: _.template(auth_user, userParams) }); + + if (!auth_pass || !auth_pass.match(/\buser_password\b/)) { + return callback(); + } + + this.metadataBackend.getUserDBPass(username, (err, userPassword) => { + if (err) { + return callback(err); + } + + userParams.user_password = userPassword; + _.extend(params, { dbpassword: _.template(auth_pass, userParams) }); + + callback(); + }); + }); }; // Set db connection parameters to those for the given username From 880e3f388d8e6e44ba46c4448f19f39c59e00b34 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 7 Feb 2018 12:46:10 +0100 Subject: [PATCH 04/53] remove use of _.extend calls --- lib/cartodb/backends/pg_connection.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/backends/pg_connection.js b/lib/cartodb/backends/pg_connection.js index d6ce648b..162c0f61 100644 --- a/lib/cartodb/backends/pg_connection.js +++ b/lib/cartodb/backends/pg_connection.js @@ -33,7 +33,7 @@ PgConnection.prototype.setDBAuth = function(username, params, callback) { user_id: userId }; - _.extend(params, { dbuser: _.template(auth_user, userParams) }); + params.dbuser = _.template(auth_user, userParams); if (!auth_pass || !auth_pass.match(/\buser_password\b/)) { return callback(); @@ -45,8 +45,8 @@ PgConnection.prototype.setDBAuth = function(username, params, callback) { } userParams.user_password = userPassword; - _.extend(params, { dbpassword: _.template(auth_pass, userParams) }); - + params.dbpassword = _.template(auth_pass, userParams); + callback(); }); }); From bde86323fd8ca90b1376ec524af963bf9bb26ba0 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 7 Feb 2018 15:36:24 +0100 Subject: [PATCH 05/53] use master api key in setDBAuth --- lib/cartodb/backends/pg_connection.js | 28 +++++---------------------- 1 file changed, 5 insertions(+), 23 deletions(-) diff --git a/lib/cartodb/backends/pg_connection.js b/lib/cartodb/backends/pg_connection.js index 162c0f61..80b14bc0 100644 --- a/lib/cartodb/backends/pg_connection.js +++ b/lib/cartodb/backends/pg_connection.js @@ -21,35 +21,17 @@ module.exports = PgConnection; // @param callback function(err) // PgConnection.prototype.setDBAuth = function(username, params, callback) { - var auth_user = global.environment.postgres_auth_user; - var auth_pass = global.environment.postgres_auth_pass; - - this.metadataBackend.getUserId(username, (err, userId) => { + this.metadataBackend.getMasterApikey(username, (err, apikey)=> { if (err) { return callback(err); } - - const userParams = { - user_id: userId - }; - params.dbuser = _.template(auth_user, userParams); + params.dbuser = apikey.databaseRole; + params.dbpassword = apikey.databasePassword; - if (!auth_pass || !auth_pass.match(/\buser_password\b/)) { - return callback(); - } - - this.metadataBackend.getUserDBPass(username, (err, userPassword) => { - if (err) { - return callback(err); - } - - userParams.user_password = userPassword; - params.dbpassword = _.template(auth_pass, userParams); - - callback(); - }); + callback(); }); + }; // Set db connection parameters to those for the given username From 85c1c987af5d3014bc6b4ba7e58fd1fb84d0c90e Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 7 Feb 2018 15:49:32 +0100 Subject: [PATCH 06/53] refactor setDBConn to not use step --- lib/cartodb/backends/pg_connection.js | 40 ++++++++++++--------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/lib/cartodb/backends/pg_connection.js b/lib/cartodb/backends/pg_connection.js index 80b14bc0..8355f585 100644 --- a/lib/cartodb/backends/pg_connection.js +++ b/lib/cartodb/backends/pg_connection.js @@ -46,36 +46,30 @@ PgConnection.prototype.setDBAuth = function(username, params, callback) { // @param callback function(err) // PgConnection.prototype.setDBConn = function(dbowner, params, callback) { - var self = this; - // Add default database connection parameters - // if none given _.defaults(params, { dbuser: global.environment.postgres.user, dbpassword: global.environment.postgres.password, dbhost: global.environment.postgres.host, dbport: global.environment.postgres.port }); - step( - function getConnectionParams() { - self.metadataBackend.getUserDBConnectionParams(dbowner, this); - }, - function extendParams(err, dbParams){ - assert.ifError(err); - // we don't want null values or overwrite a non public user - if (params.dbuser !== 'publicuser' || !dbParams.dbuser) { - delete dbParams.dbuser; - } - if ( dbParams ) { - _.extend(params, dbParams); - } - return null; - }, - function finish(err) { - callback(err); - } - ); -}; + this.metadataBackend.getUserDBConnectionParams(dbowner, (err, dbParams) => { + if (err) { + return callback(err); + } + + // we don’t want null values or overwrite a non public user + if (params.dbuser !== 'publicuser' || !dbParams.dbuser) { + delete dbParams.dbuser; + } + + if (dbParams) { + _.extend(params, dbParams); + } + + callback(); + }); +}; /** * Returns a `cartodb-psql` object for a given username. From 4c76a921b1cb48a945ff80096ee562789bca90f9 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 7 Feb 2018 16:02:13 +0100 Subject: [PATCH 07/53] use res.locals instead of req.params --- lib/cartodb/controllers/layergroup.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 0276527d..d9251dd0 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -283,7 +283,7 @@ LayergroupController.prototype.tileOrLayer = function (req, res, next) { function mapController$getTileOrGrid() { self.tileBackend.getTile( new MapStoreMapConfigProvider(self.mapStore, res.locals.user, self.userLimitsApi, res.locals), - req.params, this + res.locals, this ); }, function mapController$finalize(err, tile, headers, stats) { From 95538707c9b108ed01ebcb5d02763e8f5f95b889 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 7 Feb 2018 17:14:13 +0100 Subject: [PATCH 08/53] add parameter asMaster to setDBAuth --- lib/cartodb/api/auth_api.js | 6 ++-- lib/cartodb/backends/pg_connection.js | 33 +++++++++++++------ lib/cartodb/backends/pg_query_runner.js | 3 +- .../adapter/mapconfig-named-layers-adapter.js | 3 +- .../mapconfig/provider/named-map-provider.js | 3 +- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index 562614c3..e18f1d0d 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -105,8 +105,9 @@ AuthApi.prototype.authorize = function(req, res, callback) { return self.authorizedBySigner(res, this); } + const asMaster = false; // authorized by api key, login as the given username and stop - self.pgConnection.setDBAuth(user, res.locals, function(err) { + self.pgConnection.setDBAuth(user, res.locals, asMaster, function(err) { callback(err, true); // authorized (or error) }); }, @@ -129,7 +130,8 @@ AuthApi.prototype.authorize = function(req, res, callback) { return callback(null, false); } - self.pgConnection.setDBAuth(user, res.locals, function(err) { + const asMaster = true; + self.pgConnection.setDBAuth(user, res.locals, asMaster, function(err) { req.profiler.done('setDBAuth'); callback(err, true); // authorized (or error) }); diff --git a/lib/cartodb/backends/pg_connection.js b/lib/cartodb/backends/pg_connection.js index 8355f585..16c2e565 100644 --- a/lib/cartodb/backends/pg_connection.js +++ b/lib/cartodb/backends/pg_connection.js @@ -20,18 +20,30 @@ module.exports = PgConnection; // // @param callback function(err) // -PgConnection.prototype.setDBAuth = function(username, params, callback) { - this.metadataBackend.getMasterApikey(username, (err, apikey)=> { - if (err) { - return callback(err); - } +PgConnection.prototype.setDBAuth = function(username, params, asMaster, callback) { + if (asMaster) { + this.metadataBackend.getMasterApikey(username, (err, apikey) => { + if (err) { + return callback(err); + } - params.dbuser = apikey.databaseRole; - params.dbpassword = apikey.databasePassword; + params.dbuser = apikey.databaseRole; + params.dbpassword = apikey.databasePassword; - callback(); - }); + return callback(); + }); + } else { + this.metadataBackend.getApikey(username, params.api_key || params.map_key, (err, apikey) => { + if (err) { + return callback(err); + } + params.dbuser = apikey.databaseRole; + params.dbpassword = apikey.databasePassword; + + return callback(); + }); + } }; // Set db connection parameters to those for the given username @@ -85,7 +97,8 @@ PgConnection.prototype.getConnection = function(username, callback) { require('debug')('cachechan')("getConn1"); step( function setAuth() { - self.setDBAuth(username, params, this); + const asMaster = true; + self.setDBAuth(username, params, asMaster, this); }, function setConn(err) { assert.ifError(err); diff --git a/lib/cartodb/backends/pg_query_runner.js b/lib/cartodb/backends/pg_query_runner.js index bf57f166..41874513 100644 --- a/lib/cartodb/backends/pg_query_runner.js +++ b/lib/cartodb/backends/pg_query_runner.js @@ -22,7 +22,8 @@ PgQueryRunner.prototype.run = function(username, query, callback) { step( function setAuth() { - self.pgConnection.setDBAuth(username, params, this); + const asMaster = true; + self.pgConnection.setDBAuth(username, params, asMaster, this); }, function setConn(err) { assert.ifError(err); diff --git a/lib/cartodb/models/mapconfig/adapter/mapconfig-named-layers-adapter.js b/lib/cartodb/models/mapconfig/adapter/mapconfig-named-layers-adapter.js index 17c2059f..dd876664 100644 --- a/lib/cartodb/models/mapconfig/adapter/mapconfig-named-layers-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/mapconfig-named-layers-adapter.js @@ -109,7 +109,8 @@ MapConfigNamedLayersAdapter.prototype.getMapConfig = function (user, requestMapC if (_.some(layers, isNamedTypeLayer)) { // Lazy load dbAuth - this.pgConnection.setDBAuth(user, dbAuth, function(err) { + const asMaster = true; + this.pgConnection.setDBAuth(user, dbAuth, asMaster, function(err) { if (err) { return callback(err); } diff --git a/lib/cartodb/models/mapconfig/provider/named-map-provider.js b/lib/cartodb/models/mapconfig/provider/named-map-provider.js index 17c66c29..9e995c53 100644 --- a/lib/cartodb/models/mapconfig/provider/named-map-provider.js +++ b/lib/cartodb/models/mapconfig/provider/named-map-provider.js @@ -235,7 +235,8 @@ NamedMapMapConfigProvider.prototype.setDBParams = function(cdbuser, params, call var self = this; step( function setAuth() { - self.pgConnection.setDBAuth(cdbuser, params, this); + const asMaster = true; + self.pgConnection.setDBAuth(cdbuser, params, asMaster, this); }, function setConn(err) { assert.ifError(err); From 6b5d6648de7420a76158daf07812e19bc25850e8 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 7 Feb 2018 17:14:46 +0100 Subject: [PATCH 09/53] fix unit test --- test/unit/cartodb/prepare-context.test.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/test/unit/cartodb/prepare-context.test.js b/test/unit/cartodb/prepare-context.test.js index dbbfb8bf..61ba0c55 100644 --- a/test/unit/cartodb/prepare-context.test.js +++ b/test/unit/cartodb/prepare-context.test.js @@ -103,8 +103,20 @@ describe('prepare-context', function() { }); it('sets also dbuser for authenticated requests', function(done){ - var req = { headers: { host: 'localhost' }, query: { map_key: '1234' }}; - var res = { set: function () {} }; + var req = { + headers: { + host: 'localhost' + }, + query: { + api_key: '1234' + } + }; + var res = { + set: function () {}, + locals: { + api_key: '1234' + } + }; // FIXME: review authorize-pgconnsetup workflow, It might we are doing authorization twice. authorize(prepareRequest(req), prepareResponse(res), function (err) { From a4dbc1bac27498afd67841c9da303a773f6c2137 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 7 Feb 2018 18:20:56 +0100 Subject: [PATCH 10/53] remove step and check existance of proper api key --- lib/cartodb/api/auth_api.js | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index e18f1d0d..e4525929 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -47,6 +47,13 @@ AuthApi.prototype.authorizedBySigner = function(res, callback) { }); }; +function isValidApiKey(apikey) { + return apikey.type !== null && + apikey.user !== null && + apikey.databasePassword !== null && + apikey.databaseRole !== null; +} + // Check if a request is authorized by api_key // // @param user @@ -61,23 +68,19 @@ AuthApi.prototype.authorizedByAPIKey = function(user, req, callback) { givenKey = req.body.api_key || req.body.map_key; } if ( ! givenKey ) { - return callback(null, 0); // no api key, no authorization... + return callback(null, false); // no api key, no authorization... } - var self = this; - - step( - function () { - self.metadataBackend.getUserMapKey(user, this); - }, - function checkApiKey(err, val){ - assert.ifError(err); - return val && givenKey === val; - }, - function finish(err, authorized) { - callback(err, authorized); + this.metadataBackend.getApikey(user, givenKey, (err, apikey) => { + if (err) { + return callback(err); } - ); + if ( !isValidApiKey(apikey)) { + return callback(null, false); + } + + return callback(null, true); + }); }; /** From ea0542dcb1590925b891fe2a4a97c93a6907cab4 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 7 Feb 2018 18:48:59 +0100 Subject: [PATCH 11/53] remove use of step --- lib/cartodb/api/auth_api.js | 78 ++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 41 deletions(-) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index e4525929..0f31e721 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -1,6 +1,3 @@ -var assert = require('assert'); -var step = require('step'); - /** * * @param {PgConnection} pgConnection @@ -91,53 +88,52 @@ AuthApi.prototype.authorizedByAPIKey = function(user, req, callback) { * @param callback function(err, allowed) is access allowed not? */ AuthApi.prototype.authorize = function(req, res, callback) { - var self = this; var user = res.locals.user; - step( - function () { - self.authorizedByAPIKey(user, req, this); - }, - function checkApiKey(err, authorized){ - req.profiler.done('authorizedByAPIKey'); - assert.ifError(err); - - // if not authorized by api_key, continue - if (!authorized) { - // not authorized by api_key, check if authorized by signer - return self.authorizedBySigner(res, this); - } + this.authorizedByAPIKey(user, req, (err, isAuthorizedByApikey) => { + if (err) { + return callback(err); + } + if (isAuthorizedByApikey) { const asMaster = false; - // authorized by api key, login as the given username and stop - self.pgConnection.setDBAuth(user, res.locals, asMaster, function(err) { - callback(err, true); // authorized (or error) + return this.pgConnection.setDBAuth(user, res.locals, asMaster, function (err) { + req.profiler.done('setDBAuth'); + + if (err) { + return callback(err); + } + + callback(null, true); }); - }, - function checkSignAuthorized(err, authorized) { + } + + this.authorizedBySigner(res, (err, isAuthorizedBySigner) => { if (err) { return callback(err); } + + if (isAuthorizedBySigner) { + const asMaster = true; + return this.pgConnection.setDBAuth(user, res.locals, asMaster, function (err) { + req.profiler.done('setDBAuth'); + + if (err) { + return callback(err); + } - if ( ! authorized ) { - // request not authorized by signer. - - // if no signer name was given, let dbparams and - // PostgreSQL do the rest. - // - if ( ! res.locals.signer ) { - return callback(null, true); // authorized so far - } - - // if signer name was given, return no authorization - return callback(null, false); + callback(null, true); + }); } - const asMaster = true; - self.pgConnection.setDBAuth(user, res.locals, asMaster, function(err) { - req.profiler.done('setDBAuth'); - callback(err, true); // authorized (or error) - }); - } - ); + // if no signer name was given, let dbparams and + // PostgreSQL do the rest. + if (!res.locals.signer) { + return callback(null, true); // authorized so far + } + + // if signer name was given, return no authorization + return callback(null, false); + }); + }); }; From 41f36065723d8b932684309d57ce0b472f04300b Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 7 Feb 2018 19:12:14 +0100 Subject: [PATCH 12/53] return unauthorized error when api key not found --- lib/cartodb/api/auth_api.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index 0f31e721..fc490585 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -73,7 +73,12 @@ AuthApi.prototype.authorizedByAPIKey = function(user, req, callback) { return callback(err); } if ( !isValidApiKey(apikey)) { - return callback(null, false); + const error = new Error('Unauthorized'); + error.type = 'auth'; + error.subtype = 'api-key-not-found'; + error.http_status = 401; + + return callback(error); } return callback(null, true); From 8136a1e1361e4260854dee4648e7524598fec8fa Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 7 Feb 2018 19:12:26 +0100 Subject: [PATCH 13/53] fix test --- test/acceptance/analysis/error-cases.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/analysis/error-cases.js b/test/acceptance/analysis/error-cases.js index 376d7606..2e2ea922 100644 --- a/test/acceptance/analysis/error-cases.js +++ b/test/acceptance/analysis/error-cases.js @@ -188,7 +188,7 @@ describe('analysis-layers error cases', function() { ] ); - var testClient = new TestClient(mapConfig, 11111); + var testClient = new TestClient(mapConfig); //No apikey provided -> using default public apikey testClient.getLayergroup({ response: AUTH_ERROR_RESPONSE }, function(err, layergroupResult) { assert.ok(!err, err); From d9a34f33846055bd09d59689a6f5d1e48b932d82 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 8 Feb 2018 11:13:21 +0100 Subject: [PATCH 14/53] add cartodb250user api keys to redis --- test/support/prepare_db.sh | 22 ++++++++++++++++++++++ test/support/test_helper.js | 4 +++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/test/support/prepare_db.sh b/test/support/prepare_db.sh index b40c0add..8348347a 100755 --- a/test/support/prepare_db.sh +++ b/test/support/prepare_db.sh @@ -160,4 +160,26 @@ cat < Date: Thu, 8 Feb 2018 11:29:17 +0100 Subject: [PATCH 15/53] let select apikey type in setDBAuth: regular, default, master --- lib/cartodb/api/auth_api.js | 19 ++++++++++------ lib/cartodb/backends/pg_connection.js | 22 ++++++++++++++----- lib/cartodb/backends/pg_query_runner.js | 3 +-- .../adapter/mapconfig-named-layers-adapter.js | 4 +--- .../mapconfig/provider/named-map-provider.js | 3 +-- 5 files changed, 32 insertions(+), 19 deletions(-) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index fc490585..3f92c6d9 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -101,8 +101,7 @@ AuthApi.prototype.authorize = function(req, res, callback) { } if (isAuthorizedByApikey) { - const asMaster = false; - return this.pgConnection.setDBAuth(user, res.locals, asMaster, function (err) { + return this.pgConnection.setDBAuth(user, res.locals, 'regular', function (err) { req.profiler.done('setDBAuth'); if (err) { @@ -119,8 +118,7 @@ AuthApi.prototype.authorize = function(req, res, callback) { } if (isAuthorizedBySigner) { - const asMaster = true; - return this.pgConnection.setDBAuth(user, res.locals, asMaster, function (err) { + return this.pgConnection.setDBAuth(user, res.locals, 'master', function (err) { req.profiler.done('setDBAuth'); if (err) { @@ -131,10 +129,17 @@ AuthApi.prototype.authorize = function(req, res, callback) { }); } - // if no signer name was given, let dbparams and - // PostgreSQL do the rest. + // if no signer name was given, use default api key if (!res.locals.signer) { - return callback(null, true); // authorized so far + return this.pgConnection.setDBAuth(user, res.locals, 'default', function (err) { + req.profiler.done('setDBAuth'); + + if (err) { + return callback(err); + } + + callback(null, true); + }); } // if signer name was given, return no authorization diff --git a/lib/cartodb/backends/pg_connection.js b/lib/cartodb/backends/pg_connection.js index 16c2e565..3c8dd143 100644 --- a/lib/cartodb/backends/pg_connection.js +++ b/lib/cartodb/backends/pg_connection.js @@ -20,8 +20,8 @@ module.exports = PgConnection; // // @param callback function(err) // -PgConnection.prototype.setDBAuth = function(username, params, asMaster, callback) { - if (asMaster) { +PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callback) { + if (apikeyType === 'master') { this.metadataBackend.getMasterApikey(username, (err, apikey) => { if (err) { return callback(err); @@ -32,7 +32,7 @@ PgConnection.prototype.setDBAuth = function(username, params, asMaster, callback return callback(); }); - } else { + } else if (apikeyType === 'regular') { this.metadataBackend.getApikey(username, params.api_key || params.map_key, (err, apikey) => { if (err) { return callback(err); @@ -43,6 +43,19 @@ PgConnection.prototype.setDBAuth = function(username, params, asMaster, callback return callback(); }); + } else if (apikeyType === 'default') { + this.metadataBackend.getApikey(username, 'default_public', (err, apikey) => { + if (err) { + return callback(err); + } + + params.dbuser = apikey.databaseRole; + params.dbpassword = apikey.databasePassword; + + return callback(); + }); + } else { + return callback(new Error(`Invalid Apikey type: ${apikeyType}, valid ones: master, regular, default`)); } }; @@ -97,8 +110,7 @@ PgConnection.prototype.getConnection = function(username, callback) { require('debug')('cachechan')("getConn1"); step( function setAuth() { - const asMaster = true; - self.setDBAuth(username, params, asMaster, this); + self.setDBAuth(username, params, 'master', this); }, function setConn(err) { assert.ifError(err); diff --git a/lib/cartodb/backends/pg_query_runner.js b/lib/cartodb/backends/pg_query_runner.js index 41874513..fbbc1ee2 100644 --- a/lib/cartodb/backends/pg_query_runner.js +++ b/lib/cartodb/backends/pg_query_runner.js @@ -22,8 +22,7 @@ PgQueryRunner.prototype.run = function(username, query, callback) { step( function setAuth() { - const asMaster = true; - self.pgConnection.setDBAuth(username, params, asMaster, this); + self.pgConnection.setDBAuth(username, params, 'master', this); }, function setConn(err) { assert.ifError(err); diff --git a/lib/cartodb/models/mapconfig/adapter/mapconfig-named-layers-adapter.js b/lib/cartodb/models/mapconfig/adapter/mapconfig-named-layers-adapter.js index dd876664..f652e675 100644 --- a/lib/cartodb/models/mapconfig/adapter/mapconfig-named-layers-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/mapconfig-named-layers-adapter.js @@ -108,9 +108,7 @@ MapConfigNamedLayersAdapter.prototype.getMapConfig = function (user, requestMapC var dbAuth = {}; if (_.some(layers, isNamedTypeLayer)) { - // Lazy load dbAuth - const asMaster = true; - this.pgConnection.setDBAuth(user, dbAuth, asMaster, function(err) { + this.pgConnection.setDBAuth(user, dbAuth, 'master', function(err) { if (err) { return callback(err); } diff --git a/lib/cartodb/models/mapconfig/provider/named-map-provider.js b/lib/cartodb/models/mapconfig/provider/named-map-provider.js index 9e995c53..67d29f73 100644 --- a/lib/cartodb/models/mapconfig/provider/named-map-provider.js +++ b/lib/cartodb/models/mapconfig/provider/named-map-provider.js @@ -235,8 +235,7 @@ NamedMapMapConfigProvider.prototype.setDBParams = function(cdbuser, params, call var self = this; step( function setAuth() { - const asMaster = true; - self.pgConnection.setDBAuth(cdbuser, params, asMaster, this); + self.pgConnection.setDBAuth(cdbuser, params, 'master', this); }, function setConn(err) { assert.ifError(err); From b0e9df1400721579dac96e5ff4fb0741cd5e9776 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 8 Feb 2018 12:04:03 +0100 Subject: [PATCH 16/53] add pgConnection.getDatabaseParams --- lib/cartodb/backends/pg_connection.js | 59 +++++++++++-------- lib/cartodb/backends/pg_query_runner.js | 44 ++++++-------- .../mapconfig/provider/named-map-provider.js | 24 ++++---- 3 files changed, 62 insertions(+), 65 deletions(-) diff --git a/lib/cartodb/backends/pg_connection.js b/lib/cartodb/backends/pg_connection.js index 3c8dd143..a145d0d2 100644 --- a/lib/cartodb/backends/pg_connection.js +++ b/lib/cartodb/backends/pg_connection.js @@ -1,5 +1,3 @@ -var assert = require('assert'); -var step = require('step'); var PSQL = require('cartodb-psql'); var _ = require('underscore'); @@ -72,8 +70,8 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba // PgConnection.prototype.setDBConn = function(dbowner, params, callback) { _.defaults(params, { - dbuser: global.environment.postgres.user, - dbpassword: global.environment.postgres.password, + // dbuser: global.environment.postgres.user, + // dbpassword: global.environment.postgres.password, dbhost: global.environment.postgres.host, dbport: global.environment.postgres.port }); @@ -103,28 +101,37 @@ PgConnection.prototype.setDBConn = function(dbowner, params, callback) { */ PgConnection.prototype.getConnection = function(username, callback) { - var self = this; - - var params = {}; - require('debug')('cachechan')("getConn1"); - step( - function setAuth() { - self.setDBAuth(username, params, 'master', this); - }, - function setConn(err) { - assert.ifError(err); - self.setDBConn(username, params, this); - }, - function openConnection(err) { - assert.ifError(err); - return callback(err, new PSQL({ - user: params.dbuser, - pass: params.dbpass, - host: params.dbhost, - port: params.dbport, - dbname: params.dbname - })); + + this.getDatabaseParams(username, (err, databaseParams) => { + if (err) { + return callback(err); } - ); + return callback(err, new PSQL({ + user: databaseParams.dbuser, + pass: databaseParams.dbpass, + host: databaseParams.dbhost, + port: databaseParams.dbport, + dbname: databaseParams.dbname + })); + + }); +}; + +PgConnection.prototype.getDatabaseParams = function(username, callback) { + const databaseParams = {}; + + this.setDBAuth(username, databaseParams, 'master', err => { + if (err) { + return callback(err); + } + + this.setDBConn(username, databaseParams, err => { + if (err) { + return callback(err); + } + + callback(null, databaseParams); + }); + }); }; diff --git a/lib/cartodb/backends/pg_query_runner.js b/lib/cartodb/backends/pg_query_runner.js index fbbc1ee2..d3a16719 100644 --- a/lib/cartodb/backends/pg_query_runner.js +++ b/lib/cartodb/backends/pg_query_runner.js @@ -1,6 +1,4 @@ -var assert = require('assert'); var PSQL = require('cartodb-psql'); -var step = require('step'); function PgQueryRunner(pgConnection) { this.pgConnection = pgConnection; @@ -16,31 +14,23 @@ module.exports = PgQueryRunner; * @param {Function} callback function({Error}, {Array}) second argument is guaranteed to be an array */ PgQueryRunner.prototype.run = function(username, query, callback) { - var self = this; - var params = {}; - - step( - function setAuth() { - self.pgConnection.setDBAuth(username, params, 'master', this); - }, - function setConn(err) { - assert.ifError(err); - self.pgConnection.setDBConn(username, params, this); - }, - function executeQuery(err) { - assert.ifError(err); - var psql = new PSQL({ - user: params.dbuser, - pass: params.dbpass, - host: params.dbhost, - port: params.dbport, - dbname: params.dbname - }); - psql.query(query, function(err, resultSet) { - resultSet = resultSet || {}; - return callback(err, resultSet.rows || []); - }); + this.pgConnection.getDatabaseParams(username, (err, databaseParams) => { + if (err) { + return callback(err); } - ); + + const psql = new PSQL({ + user: databaseParams.dbuser, + pass: databaseParams.dbpass, + host: databaseParams.dbhost, + port: databaseParams.dbport, + dbname: databaseParams.dbname + }); + + psql.query(query, function (err, resultSet) { + resultSet = resultSet || {}; + return callback(err, resultSet.rows || []); + }); + }); }; diff --git a/lib/cartodb/models/mapconfig/provider/named-map-provider.js b/lib/cartodb/models/mapconfig/provider/named-map-provider.js index 67d29f73..9b085dbc 100644 --- a/lib/cartodb/models/mapconfig/provider/named-map-provider.js +++ b/lib/cartodb/models/mapconfig/provider/named-map-provider.js @@ -232,19 +232,19 @@ function configHash(config) { module.exports.configHash = configHash; NamedMapMapConfigProvider.prototype.setDBParams = function(cdbuser, params, callback) { - var self = this; - step( - function setAuth() { - self.pgConnection.setDBAuth(cdbuser, params, 'master', this); - }, - function setConn(err) { - assert.ifError(err); - self.pgConnection.setDBConn(cdbuser, params, this); - }, - function finish(err) { - callback(err); + this.pgConnection.getDatabaseParams(cdbuser, (err, databaseParams) => { + if (err) { + return callback(err); } - ); + + params.dbuser = databaseParams.dbuser; + params.dbpass = databaseParams.dbpass; + params.dbhost = databaseParams.dbhost; + params.dbport = databaseParams.dbport; + params.dbname = databaseParams.dbname; + + callback(); + }); }; NamedMapMapConfigProvider.prototype.getTemplateName = function() { From 1c50dd6b48f792a4916de22a292b0c725e46a68a Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 8 Feb 2018 12:34:24 +0100 Subject: [PATCH 17/53] add first tests for auth --- test/acceptance/auth/authorization.js | 57 +++++++++++++++++++++++++++ test/support/prepare_db.sh | 11 ++++++ test/support/sql/windshaft.test.sql | 50 +++++++++++++++++++++++ test/support/test_helper.js | 3 +- 4 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 test/acceptance/auth/authorization.js diff --git a/test/acceptance/auth/authorization.js b/test/acceptance/auth/authorization.js new file mode 100644 index 00000000..e5f493b4 --- /dev/null +++ b/test/acceptance/auth/authorization.js @@ -0,0 +1,57 @@ +require('../../support/test_helper'); + +const assert = require('../../support/assert'); +const TestClient = require('../../support/test-client'); + +describe('authorization', function() { + it('should create a layergroup with regular apikey token', function(done) { + const apikeyToken = 'regular1'; + const mapConfig = { + version: '1.7.0', + layers: [ + { + options: { + sql: 'select * FROM test_table_localhost_regular1', + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + } + ] + }; + const testClient = new TestClient(mapConfig, apikeyToken); + + testClient.getLayergroup(function (err, layergroupResult) { + assert.ifError(err); + + assert.ok(layergroupResult.layergroupid); + + testClient.drain(done); + }); + }); + + it('should fail if apikey does not gran access to table', function (done) { + const mapConfig = { + version: '1.7.0', + layers: [ + { + options: { + sql: 'select * FROM test_table_localhost_regular1', + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + } + ] + }; + const testClient = new TestClient(mapConfig); //no apikey provided, using default + + testClient.getLayergroup({response: {status:403}}, function (err, layergroupResult) { + assert.ifError(err); + + assert.ok(layergroupResult.hasOwnProperty('errors')); + assert.equal(layergroupResult.errors.length, 1); + assert.ok(layergroupResult.errors[0].match(/permission denied/), layergroupResult.errors[0]); + + testClient.drain(done); + }); + }); +}); diff --git a/test/support/prepare_db.sh b/test/support/prepare_db.sh index 8348347a..b2ac8688 100755 --- a/test/support/prepare_db.sh +++ b/test/support/prepare_db.sh @@ -160,6 +160,17 @@ cat < Date: Thu, 8 Feb 2018 12:35:44 +0100 Subject: [PATCH 18/53] remove comment --- lib/cartodb/middleware/context/db-conn-setup.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/cartodb/middleware/context/db-conn-setup.js b/lib/cartodb/middleware/context/db-conn-setup.js index dec48f6d..068d77c2 100644 --- a/lib/cartodb/middleware/context/db-conn-setup.js +++ b/lib/cartodb/middleware/context/db-conn-setup.js @@ -12,8 +12,6 @@ module.exports = function dbConnSetupMiddleware(pgConnection) { return next(err); } - // Add default database connection parameters - // if none given _.defaults(res.locals, { dbuser: global.environment.postgres.user, dbpassword: global.environment.postgres.password, From e1a2ee2381fdcc9b6353d5b5ffcc4f9e3a8ffda2 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 8 Feb 2018 13:07:25 +0100 Subject: [PATCH 19/53] control API access grants --- lib/cartodb/api/auth_api.js | 9 +++++++++ test/acceptance/auth/authorization.js | 29 ++++++++++++++++++++++++++- test/support/prepare_db.sh | 11 ++++++++++ test/support/test_helper.js | 3 ++- 4 files changed, 50 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index 3f92c6d9..ca4c2724 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -81,6 +81,15 @@ AuthApi.prototype.authorizedByAPIKey = function(user, req, callback) { return callback(error); } + if (!apikey.grantsMaps) { + const error = new Error('Forbidden'); + error.type = 'auth'; + error.subtype = 'api-key-does-not-grant-access'; + error.http_status = 403; + + return callback(error); + } + return callback(null, true); }); }; diff --git a/test/acceptance/auth/authorization.js b/test/acceptance/auth/authorization.js index e5f493b4..7904d424 100644 --- a/test/acceptance/auth/authorization.js +++ b/test/acceptance/auth/authorization.js @@ -44,7 +44,7 @@ describe('authorization', function() { }; const testClient = new TestClient(mapConfig); //no apikey provided, using default - testClient.getLayergroup({response: {status:403}}, function (err, layergroupResult) { + testClient.getLayergroup({ response: { status: 403 } }, function (err, layergroupResult) { //TODO 401 assert.ifError(err); assert.ok(layergroupResult.hasOwnProperty('errors')); @@ -54,4 +54,31 @@ describe('authorization', function() { testClient.drain(done); }); }); + + it('should forbide access to API if API key does not grant access', function (done) { + const apikeyToken = 'regular2'; + const mapConfig = { + version: '1.7.0', + layers: [ + { + options: { + sql: 'select * FROM test_table_localhost_regular1', + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + } + ] + }; + const testClient = new TestClient(mapConfig, apikeyToken); + + testClient.getLayergroup({ response: { status: 403 } }, function (err, layergroupResult) { + assert.ifError(err); + + assert.ok(layergroupResult.hasOwnProperty('errors')); + assert.equal(layergroupResult.errors.length, 1); + assert.ok(layergroupResult.errors[0].match(/Forbidden/), layergroupResult.errors[0]); + + testClient.drain(done); + }); + }); }); diff --git a/test/support/prepare_db.sh b/test/support/prepare_db.sh index b2ac8688..f4e3388f 100755 --- a/test/support/prepare_db.sh +++ b/test/support/prepare_db.sh @@ -171,6 +171,17 @@ cat < Date: Thu, 8 Feb 2018 14:43:12 +0100 Subject: [PATCH 20/53] fix test typo --- test/acceptance/auth/authorization.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/auth/authorization.js b/test/acceptance/auth/authorization.js index 7904d424..da139638 100644 --- a/test/acceptance/auth/authorization.js +++ b/test/acceptance/auth/authorization.js @@ -29,7 +29,7 @@ describe('authorization', function() { }); }); - it('should fail if apikey does not gran access to table', function (done) { + it('should fail if apikey does not grant access to table', function (done) { const mapConfig = { version: '1.7.0', layers: [ From 8bdb82c7be9c778d48c6571f527040751b08cb6d Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 8 Feb 2018 14:48:00 +0100 Subject: [PATCH 21/53] add test should fail creating a layergroup with default apikey token --- test/acceptance/auth/authorization.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/acceptance/auth/authorization.js b/test/acceptance/auth/authorization.js index da139638..c82b2a37 100644 --- a/test/acceptance/auth/authorization.js +++ b/test/acceptance/auth/authorization.js @@ -29,6 +29,33 @@ describe('authorization', function() { }); }); + it('should fail creating a layergroup with default apikey token', function (done) { + const apikeyToken = 'default_public'; + const mapConfig = { + version: '1.7.0', + layers: [ + { + options: { + sql: 'select * FROM test_table_localhost_regular1', + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + } + ] + }; + const testClient = new TestClient(mapConfig, apikeyToken); + + testClient.getLayergroup({ response: { status: 403 } }, function (err, layergroupResult) { + assert.ifError(err); + + assert.ok(layergroupResult.hasOwnProperty('errors')); + assert.equal(layergroupResult.errors.length, 1); + assert.ok(layergroupResult.errors[0].match(/permission denied/), layergroupResult.errors[0]); + + testClient.drain(done); + }); + }); + it('should fail if apikey does not grant access to table', function (done) { const mapConfig = { version: '1.7.0', From 455202cd1a9891a0bd5d9e3d17a01cd41cca40d0 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 8 Feb 2018 14:49:42 +0100 Subject: [PATCH 22/53] organize prepare db api keys --- test/support/prepare_db.sh | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/support/prepare_db.sh b/test/support/prepare_db.sh index f4e3388f..56fcc3c9 100755 --- a/test/support/prepare_db.sh +++ b/test/support/prepare_db.sh @@ -138,6 +138,10 @@ EOF fi +# API keys ============================== + +# User localhost ----------------------- + # API Key Master cat < Date: Thu, 8 Feb 2018 14:50:17 +0100 Subject: [PATCH 23/53] add test should create a layergroup with default apikey token --- test/acceptance/auth/authorization.js | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/acceptance/auth/authorization.js b/test/acceptance/auth/authorization.js index c82b2a37..22df63e0 100644 --- a/test/acceptance/auth/authorization.js +++ b/test/acceptance/auth/authorization.js @@ -56,6 +56,31 @@ describe('authorization', function() { }); }); + it('should create a layergroup with default apikey token', function (done) { + const apikeyToken = 'default_public'; + const mapConfig = { + version: '1.7.0', + layers: [ + { + options: { + sql: 'select * FROM test_table', + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + } + ] + }; + const testClient = new TestClient(mapConfig, apikeyToken); + + testClient.getLayergroup(function (err, layergroupResult) { + assert.ifError(err); + + assert.ok(layergroupResult.layergroupid); + + testClient.drain(done); + }); + }); + it('should fail if apikey does not grant access to table', function (done) { const mapConfig = { version: '1.7.0', From c7780e9f42ec1c2dd3790940d69dd0edb48104d4 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 8 Feb 2018 15:20:43 +0100 Subject: [PATCH 24/53] add tests should create a layergroup with a source analysis --- test/acceptance/auth/authorization.js | 74 +++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/test/acceptance/auth/authorization.js b/test/acceptance/auth/authorization.js index 22df63e0..b5ceadcb 100644 --- a/test/acceptance/auth/authorization.js +++ b/test/acceptance/auth/authorization.js @@ -133,4 +133,78 @@ describe('authorization', function() { testClient.drain(done); }); }); + + it('should create a layergroup with a source analysis using a default apikey token', function (done) { + const apikeyToken = 'default_public'; + const mapConfig = { + version: '1.7.0', + layers: [ + { + type: 'cartodb', + options: { + source: { + id: 'HEAD' + }, + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + } + ], + analyses: [ + { + id: 'HEAD', + type: 'source', + params: { + query: 'select * from populated_places_simple_reduced' + } + } + ] + }; + const testClient = new TestClient(mapConfig, apikeyToken); + + testClient.getLayergroup(function (err, layergroupResult) { + assert.ifError(err); + + assert.ok(layergroupResult.layergroupid); + + testClient.drain(done); + }); + }); + + it('should create a layergroup with a source analysis using a regular apikey token', function (done) { + const apikeyToken = 'regular1'; + const mapConfig = { + version: '1.7.0', + layers: [ + { + type: 'cartodb', + options: { + source: { + id: 'HEAD' + }, + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + } + ], + analyses: [ + { + id: 'HEAD', + type: 'source', + params: { + query: 'select * from test_table_localhost_regular1' + } + } + ] + }; + const testClient = new TestClient(mapConfig, apikeyToken); + + testClient.getLayergroup(function (err, layergroupResult) { + assert.ifError(err); + + assert.ok(layergroupResult.layergroupid); + + testClient.drain(done); + }); + }); }); From a8de436424a4049af494e166a31e532e58a631ff Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 8 Feb 2018 18:54:14 +0100 Subject: [PATCH 25/53] add test should create a layergroup with a buffer analysis using a regular apikey token AND grant privileges to master and regular roles in bootstraping sql --- test/acceptance/auth/authorization.js | 44 +++++++++++++++++++++++++++ test/support/sql/windshaft.test.sql | 4 +++ 2 files changed, 48 insertions(+) diff --git a/test/acceptance/auth/authorization.js b/test/acceptance/auth/authorization.js index b5ceadcb..4f13d0d0 100644 --- a/test/acceptance/auth/authorization.js +++ b/test/acceptance/auth/authorization.js @@ -207,4 +207,48 @@ describe('authorization', function() { testClient.drain(done); }); }); + + it('should create a layergroup with a buffer analysis using a regular apikey token', function (done) { + const apikeyToken = 'regular1'; + const mapConfig = { + version: '1.7.0', + layers: [ + { + type: 'cartodb', + options: { + source: { + id: 'HEAD1' + }, + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + } + ], + analyses: [ + { + id: "HEAD1", + type: "buffer", + params: { + source: { + id: 'HEAD2', + type: 'source', + params: { + query: 'select * from test_table_localhost_regular1' + } + }, + radius: 50000 + } + } + ] + }; + const testClient = new TestClient(mapConfig, apikeyToken); + + testClient.getLayergroup(function (err, layergroupResult) { + assert.ifError(err); + + assert.ok(layergroupResult.layergroupid); + + testClient.drain(done); + }); + }); }); diff --git a/test/support/sql/windshaft.test.sql b/test/support/sql/windshaft.test.sql index 848c9c0a..2dd03160 100644 --- a/test/support/sql/windshaft.test.sql +++ b/test/support/sql/windshaft.test.sql @@ -27,6 +27,8 @@ CREATE USER :TESTUSER WITH PASSWORD ':TESTPASS'; DROP USER IF EXISTS test_windshaft_regular1; CREATE USER test_windshaft_regular1 WITH PASSWORD 'regular1'; +GRANT test_windshaft_regular1 to :TESTUSER; + -- first table CREATE TABLE test_table ( updated_at timestamp without time zone DEFAULT now(), @@ -193,6 +195,7 @@ INSERT INTO CDB_TableMetadata (tabname, updated_at) VALUES ('test_table_private_ -- GRANT SELECT ON CDB_TableMetadata TO :PUBLICUSER; GRANT SELECT ON CDB_TableMetadata TO :TESTUSER; +GRANT SELECT ON CDB_TableMetadata TO test_windshaft_regular1; -- for analysis -- long name table CREATE TABLE @@ -755,6 +758,7 @@ GRANT SELECT ON TABLE analysis_rent_listings TO :PUBLICUSER; -- GRANT SELECT, UPDATE, INSERT, DELETE ON cdb_analysis_catalog TO :TESTUSER; +GRANT SELECT, UPDATE, INSERT, DELETE ON cdb_analysis_catalog TO test_windshaft_regular1; -- for analysis DROP EXTENSION IF EXISTS crankshaft; CREATE SCHEMA IF NOT EXISTS cdb_crankshaft; From 04f60baec56028216b046f76d90c9656491b3580 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 8 Feb 2018 19:01:58 +0100 Subject: [PATCH 26/53] Set the master role inheritance from regular roles as TBA --- test/acceptance/auth/authorization.js | 1 + test/support/sql/windshaft.test.sql | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/test/acceptance/auth/authorization.js b/test/acceptance/auth/authorization.js index 4f13d0d0..24b185d2 100644 --- a/test/acceptance/auth/authorization.js +++ b/test/acceptance/auth/authorization.js @@ -208,6 +208,7 @@ describe('authorization', function() { }); }); + // Warning: TBA it('should create a layergroup with a buffer analysis using a regular apikey token', function (done) { const apikeyToken = 'regular1'; const mapConfig = { diff --git a/test/support/sql/windshaft.test.sql b/test/support/sql/windshaft.test.sql index 2dd03160..d69e5691 100644 --- a/test/support/sql/windshaft.test.sql +++ b/test/support/sql/windshaft.test.sql @@ -195,7 +195,7 @@ INSERT INTO CDB_TableMetadata (tabname, updated_at) VALUES ('test_table_private_ -- GRANT SELECT ON CDB_TableMetadata TO :PUBLICUSER; GRANT SELECT ON CDB_TableMetadata TO :TESTUSER; -GRANT SELECT ON CDB_TableMetadata TO test_windshaft_regular1; -- for analysis +GRANT SELECT ON CDB_TableMetadata TO test_windshaft_regular1; -- for analysis. Warning: TBA -- long name table CREATE TABLE @@ -758,7 +758,7 @@ GRANT SELECT ON TABLE analysis_rent_listings TO :PUBLICUSER; -- GRANT SELECT, UPDATE, INSERT, DELETE ON cdb_analysis_catalog TO :TESTUSER; -GRANT SELECT, UPDATE, INSERT, DELETE ON cdb_analysis_catalog TO test_windshaft_regular1; -- for analysis +GRANT SELECT, UPDATE, INSERT, DELETE ON cdb_analysis_catalog TO test_windshaft_regular1; -- for analysis. Warning: TBA DROP EXTENSION IF EXISTS crankshaft; CREATE SCHEMA IF NOT EXISTS cdb_crankshaft; From 52a1ed869c031d7ad1b400770de12c8749dcba58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 8 Feb 2018 19:28:43 +0100 Subject: [PATCH 27/53] Update to development version of cartodb-redis --- package.json | 2 +- yarn.lock | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index e71f7d8d..6e682c9c 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "camshaft": "0.60.0", "cartodb-psql": "0.10.2", "cartodb-query-tables": "0.3.0", - "cartodb-redis": "0.15.0", + "cartodb-redis": "cartodb/node-cartodb-redis#project-auth-api", "debug": "^3.1.0", "dot": "~1.0.2", "express": "~4.16.0", diff --git a/yarn.lock b/yarn.lock index 8499231c..fb36531b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11,7 +11,7 @@ node-pre-gyp "~0.6.30" protozero "1.5.1" -"@carto/tilelive-bridge@github:cartodb/tilelive-bridge#2.5.1-cdb1": +"@carto/tilelive-bridge@cartodb/tilelive-bridge#2.5.1-cdb1": version "2.5.1-cdb1" resolved "https://codeload.github.com/cartodb/tilelive-bridge/tar.gz/b0b5559f948e77b337bc9a9ae0bf6ec4249fba21" dependencies: @@ -23,7 +23,7 @@ version "1.0.5" resolved "https://registry.yarnpkg.com/@mapbox/sphericalmercator/-/sphericalmercator-1.0.5.tgz#70237b9774095ed1cfdbcea7a8fd1fc82b2691f2" -"abaculus@github:cartodb/abaculus#2.0.3-cdb2": +abaculus@cartodb/abaculus#2.0.3-cdb2: version "2.0.3-cdb2" resolved "https://codeload.github.com/cartodb/abaculus/tar.gz/6468e0e3fddb2b23f60b9a3156117cff0307f6dc" dependencies: @@ -243,7 +243,7 @@ camshaft@0.60.0: dot "^1.0.3" request "^2.69.0" -"canvas@github:cartodb/node-canvas#1.6.2-cdb2": +canvas@cartodb/node-canvas#1.6.2-cdb2: version "1.6.2-cdb2" resolved "https://codeload.github.com/cartodb/node-canvas/tar.gz/8acf04557005c633f9e68524488a2657c04f3766" dependencies: @@ -269,7 +269,7 @@ carto@CartoDB/carto#0.15.1-cdb1: optimist "~0.6.0" underscore "~1.6.0" -"carto@github:cartodb/carto#0.15.1-cdb3": +carto@cartodb/carto#0.15.1-cdb3: version "0.15.1-cdb3" resolved "https://codeload.github.com/cartodb/carto/tar.gz/945f5efb74fd1af1f5e1f69f409f9567f94fb5a7" dependencies: @@ -295,9 +295,9 @@ cartodb-query-tables@0.3.0: version "0.3.0" resolved "https://registry.yarnpkg.com/cartodb-query-tables/-/cartodb-query-tables-0.3.0.tgz#56e18d869666eb2e8e2cb57d0baf3acc923f8756" -cartodb-redis@0.15.0: - version "0.15.0" - resolved "https://registry.yarnpkg.com/cartodb-redis/-/cartodb-redis-0.15.0.tgz#509ab9f62b8cae0838bcb8db1cb9d6355704ace3" +cartodb-redis@cartodb/node-cartodb-redis#project-auth-api: + version "0.15.1" + resolved "https://codeload.github.com/cartodb/node-cartodb-redis/tar.gz/2d17fe82d66972edce5c727c679a335845ea5eb0" dependencies: dot "~1.0.2" redis-mpool "^0.5.0" @@ -2206,7 +2206,7 @@ through@2: version "2.3.8" resolved "https://registry.yarnpkg.com/through/-/through-2.3.8.tgz#0dd4c9ffaabc357960b1b724115d7e0e86a2e1f5" -"tilelive-mapnik@github:cartodb/tilelive-mapnik#0.6.18-cdb5": +tilelive-mapnik@cartodb/tilelive-mapnik#0.6.18-cdb5: version "0.6.18-cdb5" resolved "https://codeload.github.com/cartodb/tilelive-mapnik/tar.gz/cec846025e60837c60af193d600d972917ea8d35" dependencies: From 1d3045c799ea4886454ef40bdde75b430e533afb Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Fri, 9 Feb 2018 12:33:33 +0100 Subject: [PATCH 28/53] add tests should create/fail creating named maps and regular api key --- test/acceptance/auth/authorization.js | 82 +++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/test/acceptance/auth/authorization.js b/test/acceptance/auth/authorization.js index 24b185d2..b62ec32e 100644 --- a/test/acceptance/auth/authorization.js +++ b/test/acceptance/auth/authorization.js @@ -2,6 +2,7 @@ require('../../support/test_helper'); const assert = require('../../support/assert'); const TestClient = require('../../support/test-client'); +const mapnik = require('windshaft').mapnik; describe('authorization', function() { it('should create a layergroup with regular apikey token', function(done) { @@ -252,4 +253,85 @@ describe('authorization', function() { testClient.drain(done); }); }); + + it('should create and get a named map tile using a regular apikey token', function (done) { + const apikeyToken = 'regular1'; + + const template = { + version: '0.0.1', + name: 'auth-api-template', + placeholders: { + buffersize: { + type: 'number', + default: 0 + } + }, + layergroup: { + version: '1.7.0', + layers: [{ + type: 'cartodb', + options: { + sql: 'select * from test_table_localhost_regular1', + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0', + } + }] + } + }; + + const testClient = new TestClient(template, apikeyToken); + + testClient.getTile(0, 0, 0, function (err, res, tile) { + assert.ifError(err); + + assert.equal(res.statusCode, 200); + assert.ok(tile instanceof mapnik.Image); + + testClient.drain(done); + }); + }); + + it('should fail creating a named map using a regular apikey token and a private table', function (done) { + const apikeyToken = 'regular1'; + + const template = { + version: '0.0.1', + name: 'auth-api-template-private', + placeholders: { + buffersize: { + type: 'number', + default: 0 + } + }, + layergroup: { + version: '1.7.0', + layers: [{ + type: 'cartodb', + options: { + sql: 'select * from populated_places_simple_reduced_private', + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0', + } + }] + } + }; + + const testClient = new TestClient(template, apikeyToken); + + const PERMISSION_DENIED_RESPONSE = { + status: 403, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + testClient.getTile(0, 0, 0, { response: PERMISSION_DENIED_RESPONSE }, function (err, res, body) { + assert.ifError(err); + + assert.ok(body.hasOwnProperty('errors')); + assert.equal(body.errors.length, 1); + assert.ok(body.errors[0].match(/permission denied/), body.errors[0]); + + testClient.drain(done); + }); + }); }); From 8721f56269288de14baa39b0b6e52bb2f73a6667 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Fri, 9 Feb 2018 19:19:18 +0100 Subject: [PATCH 29/53] add auth test for getting tiles --- test/acceptance/auth/authorization.js | 101 ++++++++++++++++++++++++-- 1 file changed, 95 insertions(+), 6 deletions(-) diff --git a/test/acceptance/auth/authorization.js b/test/acceptance/auth/authorization.js index b62ec32e..351c686d 100644 --- a/test/acceptance/auth/authorization.js +++ b/test/acceptance/auth/authorization.js @@ -4,6 +4,13 @@ const assert = require('../../support/assert'); const TestClient = require('../../support/test-client'); const mapnik = require('windshaft').mapnik; +const PERMISSION_DENIED_RESPONSE = { + status: 403, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } +}; + describe('authorization', function() { it('should create a layergroup with regular apikey token', function(done) { const apikeyToken = 'regular1'; @@ -30,6 +37,68 @@ describe('authorization', function() { }); }); + it('should create and get a named map tile using a regular apikey token', function (done) { + const apikeyToken = 'regular1'; + const mapConfig = { + version: '1.7.0', + layers: [ + { + options: { + sql: 'select * FROM test_table_localhost_regular1', + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + } + ] + }; + const testClient = new TestClient(mapConfig, apikeyToken); + + testClient.getTile(0, 0, 0, function (err, res, tile) { + assert.ifError(err); + + assert.equal(res.statusCode, 200); + assert.ok(tile instanceof mapnik.Image); + + testClient.drain(done); + }); + }); + + it('should fail getting a named map tile with default apikey token', function (done) { + const apikeyTokenCreate = 'regular1'; + const apikeyTokenGet = 'default_public'; + const mapConfig = { + version: '1.7.0', + layers: [ + { + options: { + sql: 'select * FROM test_table_localhost_regular1', + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + } + ] + }; + + const testClientCreate = new TestClient(mapConfig, apikeyTokenCreate); + testClientCreate.getLayergroup(function (err, layergroupResult) { + assert.ifError(err); + const layergroupId = layergroupResult.layergroupid; + + const testClientGet = new TestClient({}, apikeyTokenGet); + + testClientGet.getTile(0, 0, 0, { layergroupid: layergroupId, response: PERMISSION_DENIED_RESPONSE}, function(err, res, body) { + + assert.ifError(err); + + assert.ok(body.hasOwnProperty('errors')); + assert.equal(body.errors.length, 1); + assert.ok(body.errors[0].match(/permission denied/), body.errors[0]); + + testClientGet.drain(done); + }); + }); + }); + it('should fail creating a layergroup with default apikey token', function (done) { const apikeyToken = 'default_public'; const mapConfig = { @@ -82,6 +151,32 @@ describe('authorization', function() { }); }); + it('should create and get a tile with default apikey token', function (done) { + const apikeyToken = 'default_public'; + const mapConfig = { + version: '1.7.0', + layers: [ + { + options: { + sql: 'select * FROM test_table', + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + } + ] + }; + const testClient = new TestClient(mapConfig, apikeyToken); + + testClient.getTile(0, 0, 0, function (err, res, tile) { + assert.ifError(err); + + assert.equal(res.statusCode, 200); + assert.ok(tile instanceof mapnik.Image); + + testClient.drain(done); + }); + }); + it('should fail if apikey does not grant access to table', function (done) { const mapConfig = { version: '1.7.0', @@ -318,12 +413,6 @@ describe('authorization', function() { const testClient = new TestClient(template, apikeyToken); - const PERMISSION_DENIED_RESPONSE = { - status: 403, - headers: { - 'Content-Type': 'application/json; charset=utf-8' - } - }; testClient.getTile(0, 0, 0, { response: PERMISSION_DENIED_RESPONSE }, function (err, res, body) { assert.ifError(err); From 041cd40ec205f4bdd6b1a6954da57f27bb8064f9 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Fri, 9 Feb 2018 19:26:52 +0100 Subject: [PATCH 30/53] please jshint --- test/acceptance/auth/authorization.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/acceptance/auth/authorization.js b/test/acceptance/auth/authorization.js index 351c686d..d7df8b32 100644 --- a/test/acceptance/auth/authorization.js +++ b/test/acceptance/auth/authorization.js @@ -86,7 +86,12 @@ describe('authorization', function() { const testClientGet = new TestClient({}, apikeyTokenGet); - testClientGet.getTile(0, 0, 0, { layergroupid: layergroupId, response: PERMISSION_DENIED_RESPONSE}, function(err, res, body) { + const params = { + layergroupid: layergroupId, + response: PERMISSION_DENIED_RESPONSE + }; + + testClientGet.getTile(0, 0, 0, params, function(err, res, body) { assert.ifError(err); From 890f0d1ef64046b7ffe199ada8e53bcf2b4e0afd Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 14 Feb 2018 17:31:05 +0100 Subject: [PATCH 31/53] add fallback for using metadata fallback --- lib/cartodb/api/auth_api.js | 23 ++- lib/cartodb/backends/pg_connection.js | 24 +++ .../acceptance/auth/authorization-fallback.js | 159 ++++++++++++++++++ test/support/prepare_db.sh | 13 ++ test/support/test_helper.js | 1 + 5 files changed, 216 insertions(+), 4 deletions(-) create mode 100644 test/acceptance/auth/authorization-fallback.js diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index ca4c2724..1ac2a1cc 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -1,3 +1,5 @@ +var _ = require('underscore'); // AUTH_FALLBACK + /** * * @param {PgConnection} pgConnection @@ -45,10 +47,10 @@ AuthApi.prototype.authorizedBySigner = function(res, callback) { }; function isValidApiKey(apikey) { - return apikey.type !== null && - apikey.user !== null && - apikey.databasePassword !== null && - apikey.databaseRole !== null; + return apikey.type && + apikey.user && + apikey.databasePassword && + apikey.databaseRole; } // Check if a request is authorized by api_key @@ -72,6 +74,19 @@ AuthApi.prototype.authorizedByAPIKey = function(user, req, callback) { if (err) { return callback(err); } + + //Remove this block when Auth fallback is not used anymore + // AUTH_FALLBACK + if (!apikey.databaseRole && apikey.user_id && global.environment.postgres_auth_user) { + apikey.databaseRole = _.template(global.environment.postgres_auth_user, apikey); + } + + //Remove this block when Auth fallback is not used anymore + // AUTH_FALLBACK + if (!apikey.databasePassword && global.environment.postgres.password) { + apikey.databasePassword = global.environment.postgres.password; + } + if ( !isValidApiKey(apikey)) { const error = new Error('Unauthorized'); error.type = 'auth'; diff --git a/lib/cartodb/backends/pg_connection.js b/lib/cartodb/backends/pg_connection.js index a145d0d2..d5d3ecdb 100644 --- a/lib/cartodb/backends/pg_connection.js +++ b/lib/cartodb/backends/pg_connection.js @@ -28,6 +28,12 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba params.dbuser = apikey.databaseRole; params.dbpassword = apikey.databasePassword; + //Remove this block when Auth fallback is not used anymore + // AUTH_FALLBACK + if (!params.dbuser && apikey.user_id && global.environment.postgres_auth_user) { + params.dbuser = _.template(global.environment.postgres_auth_user, apikey); + } + return callback(); }); } else if (apikeyType === 'regular') { @@ -39,6 +45,18 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba params.dbuser = apikey.databaseRole; params.dbpassword = apikey.databasePassword; + //Remove this block when Auth fallback is not used anymore + // AUTH_FALLBACK + if (!params.dbuser && apikey.user_id && apikey.type === 'master' && global.environment.postgres_auth_user) { + params.dbuser = _.template(global.environment.postgres_auth_user, apikey); + } + + //Remove this block when Auth fallback is not used anymore + // AUTH_FALLBACK + if (!params.dbpassword && global.environment.postgres.password) { + params.dbpassword = global.environment.postgres.password; + } + return callback(); }); } else if (apikeyType === 'default') { @@ -50,6 +68,12 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba params.dbuser = apikey.databaseRole; params.dbpassword = apikey.databasePassword; + //Remove this block when Auth fallback is not used anymore + // AUTH_FALLBACK + if (!params.dbpassword && global.environment.postgres.password) { + params.dbpassword = global.environment.postgres.password; + } + return callback(); }); } else { diff --git a/test/acceptance/auth/authorization-fallback.js b/test/acceptance/auth/authorization-fallback.js new file mode 100644 index 00000000..032a8d08 --- /dev/null +++ b/test/acceptance/auth/authorization-fallback.js @@ -0,0 +1,159 @@ +//Remove this file when Auth fallback is not used anymore +// AUTH_FALLBACK + +const assert = require('../../support/assert'); +const testHelper = require('../../support/test_helper'); +const CartodbWindshaft = require('../../../lib/cartodb/server'); +const serverOptions = require('../../../lib/cartodb/server_options'); +const server = new CartodbWindshaft(serverOptions); +var LayergroupToken = require('../../../lib/cartodb/models/layergroup-token'); + +function singleLayergroupConfig(sql, cartocss) { + return { + version: '1.7.0', + layers: [ + { + type: 'mapnik', + options: { + sql: sql, + cartocss: cartocss, + cartocss_version: '2.3.0' + } + } + ] + }; +} + +function createRequest(layergroup, userHost, apiKey) { + var url = layergroupUrl; + if (apiKey) { + url += '?api_key=' + apiKey; + } + return { + url: url, + method: 'POST', + headers: { + host: userHost || 'localhost', + 'Content-Type': 'application/json' + }, + data: JSON.stringify(layergroup) + }; +} + +var layergroupUrl = '/api/v1/map'; +var pointSqlMaster = "select * from test_table_private_1"; +var pointSqlPublic = "select * from test_table"; +var keysToDelete; + +describe('authorization fallback', function () { + beforeEach(function () { + keysToDelete = {}; + }); + + afterEach(function (done) { + testHelper.deleteRedisKeys(keysToDelete, done); + }); + + it("succeed with master", function (done) { + var layergroup = singleLayergroupConfig(pointSqlMaster, '#layer { marker-fill:red; }'); + + assert.response(server, + createRequest(layergroup, 'user_previous_to_project_auth', '4444'), + { + status: 200 + }, + function (res, err) { + assert.ifError(err); + + var parsed = JSON.parse(res.body); + assert.ok(parsed.layergroupid); + assert.equal(res.headers['x-layergroup-id'], parsed.layergroupid); + + keysToDelete['map_cfg|' + LayergroupToken.parse(parsed.layergroupid).token] = 0; + keysToDelete['user:user_previous_to_project_auth:mapviews:global'] = 5; + + done(); + } + ); + }); + + + it("succeed with default - sending default_public", function (done) { + var layergroup = singleLayergroupConfig(pointSqlPublic, '#layer { marker-fill:red; }'); + + assert.response(server, + createRequest(layergroup, 'user_previous_to_project_auth', 'default_public'), + { + status: 200 + }, + function (res, err) { + assert.ifError(err); + + var parsed = JSON.parse(res.body); + assert.ok(parsed.layergroupid); + assert.equal(res.headers['x-layergroup-id'], parsed.layergroupid); + + keysToDelete['map_cfg|' + LayergroupToken.parse(parsed.layergroupid).token] = 0; + keysToDelete['user:user_previous_to_project_auth:mapviews:global'] = 5; + + done(); + } + ); + }); + + + it("succeed with default - sending no api key token", function (done) { + var layergroup = singleLayergroupConfig(pointSqlPublic, '#layer { marker-fill:red; }'); + + assert.response(server, + createRequest(layergroup, 'user_previous_to_project_auth'), + { + status: 200 + }, + function (res, err) { + assert.ifError(err); + + var parsed = JSON.parse(res.body); + assert.ok(parsed.layergroupid); + assert.equal(res.headers['x-layergroup-id'], parsed.layergroupid); + + keysToDelete['map_cfg|' + LayergroupToken.parse(parsed.layergroupid).token] = 0; + keysToDelete['user:user_previous_to_project_auth:mapviews:global'] = 5; + + done(); + } + ); + }); + + it("fail with non-existent api key", function (done) { + var layergroup = singleLayergroupConfig(pointSqlMaster, '#layer { marker-fill:red; }'); + + assert.response(server, + createRequest(layergroup, 'user_previous_to_project_auth', 'THIS-API-KEY-DOESNT-EXIST'), + { + status: 401 + }, + function (res, err) { + assert.ifError(err); + + done(); + } + ); + }); + + it("fail with default", function (done) { + var layergroup = singleLayergroupConfig(pointSqlMaster, '#layer { marker-fill:red; }'); + + assert.response(server, + createRequest(layergroup, 'user_previous_to_project_auth', 'default_public'), + { + status: 403 + }, + function (res, err) { + assert.ifError(err); + + done(); + } + ); + }); +}); \ No newline at end of file diff --git a/test/support/prepare_db.sh b/test/support/prepare_db.sh index 56fcc3c9..59405389 100755 --- a/test/support/prepare_db.sh +++ b/test/support/prepare_db.sh @@ -131,6 +131,19 @@ HMSET rails:users:cartodb250user id ${TESTUSERID} \ map_key 4321 EOF + +# Remove this block when Auth fallback is not used anymore +# AUTH_FALLBACK + # A user to test auth fallback to no api keys mode + cat < Date: Thu, 15 Feb 2018 11:36:42 +0100 Subject: [PATCH 32/53] create lib for getting api key token from request --- .../api/get_api_key_token_from_request.js | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 lib/cartodb/api/get_api_key_token_from_request.js diff --git a/lib/cartodb/api/get_api_key_token_from_request.js b/lib/cartodb/api/get_api_key_token_from_request.js new file mode 100644 index 00000000..6173b7c6 --- /dev/null +++ b/lib/cartodb/api/get_api_key_token_from_request.js @@ -0,0 +1,62 @@ +'use strict'; + +const basicAuth = require('basic-auth'); + +module.exports = function getApiKeyTokenFromRequest(req) { + let apiKeyToken = null; + + for (var getter of apiKeyGetters) { + (apiKeyToken = getter(req)); + if (apiKeyTokenFound(apiKeyToken)) { + break; + } + } + + return apiKeyToken; +}; + +//-------------------------------------------------------------------------------- + +const apiKeyGetters = [ + getApikeyTokenFromHeaderAuthorization, + getApikeyTokenFromRequestQueryString, + getApikeyTokenFromRequestBody, +]; + +function getApikeyTokenFromHeaderAuthorization(req) { + const credentials = basicAuth(req); + + if (credentials) { + return credentials.pass; + } else { + return null; + } +} + +function getApikeyTokenFromRequestQueryString(req) { + if (req.query.api_key) { + return req.query.api_key; + } + + if (req.query.map_key) { + return req.query.map_key; + } + + return null; +} + +function getApikeyTokenFromRequestBody(req) { + if (req.body && req.body.api_key) { + return req.body.api_key; + } + + if (req.body && req.body.map_key) { + return req.body.map_key; + } + + return null; +} + +function apiKeyTokenFound(apiKeyToken) { + return !!apiKeyToken; +} From 5db0e9c8d8aba2c50882a92c4da38f9f96651058 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 15 Feb 2018 12:50:42 +0100 Subject: [PATCH 33/53] add middleware for apikeyToken --- lib/cartodb/api/auth_api.js | 17 +++++------- lib/cartodb/backends/pg_connection.js | 2 +- lib/cartodb/controllers/named_maps_admin.js | 2 +- .../middleware/context/apikey-token.js | 9 +++++++ lib/cartodb/middleware/context/index.js | 2 ++ .../lib}/get_api_key_token_from_request.js | 2 +- package.json | 1 + test/unit/cartodb/prepare-context.test.js | 26 +++++++++++++++++++ yarn.lock | 6 +++++ 9 files changed, 54 insertions(+), 13 deletions(-) create mode 100644 lib/cartodb/middleware/context/apikey-token.js rename lib/cartodb/{api => middleware/lib}/get_api_key_token_from_request.js (97%) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index ca4c2724..22aca1ca 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -54,21 +54,18 @@ function isValidApiKey(apikey) { // Check if a request is authorized by api_key // // @param user -// @param req express request object +// @param res express response object // @param callback function(err, authorized) // NOTE: authorized is expected to be 0 or 1 (integer) // -AuthApi.prototype.authorizedByAPIKey = function(user, req, callback) { - var givenKey = req.query.api_key || req.query.map_key; - if ( ! givenKey && req.body ) { - // check also in request body - givenKey = req.body.api_key || req.body.map_key; - } - if ( ! givenKey ) { +AuthApi.prototype.authorizedByAPIKey = function(user, res, callback) { + const apikeyToken = res.locals.apikeyToken; + + if ( ! apikeyToken ) { return callback(null, false); // no api key, no authorization... } - this.metadataBackend.getApikey(user, givenKey, (err, apikey) => { + this.metadataBackend.getApikey(user, apikeyToken, (err, apikey) => { if (err) { return callback(err); } @@ -104,7 +101,7 @@ AuthApi.prototype.authorizedByAPIKey = function(user, req, callback) { AuthApi.prototype.authorize = function(req, res, callback) { var user = res.locals.user; - this.authorizedByAPIKey(user, req, (err, isAuthorizedByApikey) => { + this.authorizedByAPIKey(user, res, (err, isAuthorizedByApikey) => { if (err) { return callback(err); } diff --git a/lib/cartodb/backends/pg_connection.js b/lib/cartodb/backends/pg_connection.js index a145d0d2..37023c90 100644 --- a/lib/cartodb/backends/pg_connection.js +++ b/lib/cartodb/backends/pg_connection.js @@ -31,7 +31,7 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba return callback(); }); } else if (apikeyType === 'regular') { - this.metadataBackend.getApikey(username, params.api_key || params.map_key, (err, apikey) => { + this.metadataBackend.getApikey(username, params.apikeyToken, (err, apikey) => { if (err) { return callback(err); } diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index a4d05467..5839e147 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -70,7 +70,7 @@ NamedMapsAdminController.prototype.authorizedByAPIKey = function (action, label) return function authorizedByAPIKeyMiddleware (req, res, next) { const { user } = res.locals; - this.authApi.authorizedByAPIKey(user, req, (err, authenticated) => { + this.authApi.authorizedByAPIKey(user, res, (err, authenticated) => { if (err) { return next(err); } diff --git a/lib/cartodb/middleware/context/apikey-token.js b/lib/cartodb/middleware/context/apikey-token.js new file mode 100644 index 00000000..f18cf60a --- /dev/null +++ b/lib/cartodb/middleware/context/apikey-token.js @@ -0,0 +1,9 @@ +'use strict'; + +const getApikeyTokenFromRequest = require('../lib/get_api_key_token_from_request'); + +module.exports = () => function apikeyTokenMiddleware(req, res, next) { + res.locals.apikeyToken = getApikeyTokenFromRequest(req); + + return next(); +}; diff --git a/lib/cartodb/middleware/context/index.js b/lib/cartodb/middleware/context/index.js index 640aafd6..96b261a4 100644 --- a/lib/cartodb/middleware/context/index.js +++ b/lib/cartodb/middleware/context/index.js @@ -1,6 +1,7 @@ const locals = require('./locals'); const cleanUpQueryParams = require('./clean-up-query-params'); const layergroupToken = require('./layergroup-token'); +const apikeyToken = require('./apikey-token'); const authorize = require('./authorize'); const dbConnSetup = require('./db-conn-setup'); @@ -9,6 +10,7 @@ module.exports = function prepareContextMiddleware(authApi, pgConnection) { locals, cleanUpQueryParams(), layergroupToken, + apikeyToken(), authorize(authApi), dbConnSetup(pgConnection) ]; diff --git a/lib/cartodb/api/get_api_key_token_from_request.js b/lib/cartodb/middleware/lib/get_api_key_token_from_request.js similarity index 97% rename from lib/cartodb/api/get_api_key_token_from_request.js rename to lib/cartodb/middleware/lib/get_api_key_token_from_request.js index 6173b7c6..5fbc74c2 100644 --- a/lib/cartodb/api/get_api_key_token_from_request.js +++ b/lib/cartodb/middleware/lib/get_api_key_token_from_request.js @@ -6,7 +6,7 @@ module.exports = function getApiKeyTokenFromRequest(req) { let apiKeyToken = null; for (var getter of apiKeyGetters) { - (apiKeyToken = getter(req)); + apiKeyToken = getter(req); if (apiKeyTokenFound(apiKeyToken)) { break; } diff --git a/package.json b/package.json index 563aca4c..b2dc6652 100644 --- a/package.json +++ b/package.json @@ -24,6 +24,7 @@ "Simon Martin " ], "dependencies": { + "basic-auth": "^2.0.0", "body-parser": "^1.18.2", "camshaft": "0.61.2", "cartodb-psql": "0.10.2", diff --git a/test/unit/cartodb/prepare-context.test.js b/test/unit/cartodb/prepare-context.test.js index 61ba0c55..acb7707e 100644 --- a/test/unit/cartodb/prepare-context.test.js +++ b/test/unit/cartodb/prepare-context.test.js @@ -10,6 +10,7 @@ var TemplateMaps = require('../../../lib/cartodb/backends/template_maps'); const cleanUpQueryParamsMiddleware = require('../../../lib/cartodb/middleware/context/clean-up-query-params'); const authorizeMiddleware = require('../../../lib/cartodb/middleware/context/authorize'); const dbConnSetupMiddleware = require('../../../lib/cartodb/middleware/context/db-conn-setup'); +const apikeyTokenMiddleware = require('../../../lib/cartodb/middleware/context/apikey-token'); const localsMiddleware = require('../../../lib/cartodb/middleware/context/locals'); var windshaft = require('windshaft'); @@ -23,6 +24,7 @@ describe('prepare-context', function() { let cleanUpQueryParams; let dbConnSetup; let authorize; + let setApikeyToken; before(function() { var redisPool = new RedisPool(global.environment.redis); @@ -35,6 +37,7 @@ describe('prepare-context', function() { cleanUpQueryParams = cleanUpQueryParamsMiddleware(); authorize = authorizeMiddleware(authApi); dbConnSetup = dbConnSetupMiddleware(pgConnection); + setApikeyToken = apikeyTokenMiddleware(); }); @@ -180,4 +183,27 @@ describe('prepare-context', function() { }); }); + describe.only('Set apikey token', function(){ + it('from query param', function (done) { + var req = { + headers: { + host: 'localhost' + }, + query: { + api_quey: '1234', + } + }; + var res = {}; + setApikeyToken(prepareRequest(req), prepareResponse(res), function (err) { + if (err) { + return done(err); + } + var query = res.locals; + console.log(query); + + assert.equal('1234', query.apikeyToken); + done(); + }); + }); + }); }); diff --git a/yarn.lock b/yarn.lock index d8a290a4..c556c94e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -143,6 +143,12 @@ balanced-match@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/balanced-match/-/balanced-match-1.0.0.tgz#89b4d199ab2bee49de164ea02b89ce462d71b767" +basic-auth@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/basic-auth/-/basic-auth-2.0.0.tgz#015db3f353e02e56377755f962742e8981e7bbba" + dependencies: + safe-buffer "5.1.1" + bcrypt-pbkdf@^1.0.0: version "1.0.1" resolved "https://registry.yarnpkg.com/bcrypt-pbkdf/-/bcrypt-pbkdf-1.0.1.tgz#63bc5dcb61331b92bc05fd528953c33462a06f8d" From 140441b777c9a2f619487c1b477a41b9850f069c Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 15 Feb 2018 12:53:01 +0100 Subject: [PATCH 34/53] fix test --- test/unit/cartodb/prepare-context.test.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/unit/cartodb/prepare-context.test.js b/test/unit/cartodb/prepare-context.test.js index acb7707e..cc58be4c 100644 --- a/test/unit/cartodb/prepare-context.test.js +++ b/test/unit/cartodb/prepare-context.test.js @@ -183,14 +183,14 @@ describe('prepare-context', function() { }); }); - describe.only('Set apikey token', function(){ + describe('Set apikey token', function(){ it('from query param', function (done) { var req = { headers: { host: 'localhost' }, query: { - api_quey: '1234', + api_key: '1234', } }; var res = {}; @@ -199,7 +199,6 @@ describe('prepare-context', function() { return done(err); } var query = res.locals; - console.log(query); assert.equal('1234', query.apikeyToken); done(); From fc420c2c0f123ff372022471e12a78ca8309f5c1 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 15 Feb 2018 15:19:09 +0100 Subject: [PATCH 35/53] use for compatibility res.locals.api_key instead of res.locals.apikeyToken --- lib/cartodb/api/auth_api.js | 2 +- lib/cartodb/backends/pg_connection.js | 2 +- lib/cartodb/middleware/context/apikey-token.js | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index 22aca1ca..9b24a16e 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -59,7 +59,7 @@ function isValidApiKey(apikey) { // NOTE: authorized is expected to be 0 or 1 (integer) // AuthApi.prototype.authorizedByAPIKey = function(user, res, callback) { - const apikeyToken = res.locals.apikeyToken; + const apikeyToken = res.locals.api_key; if ( ! apikeyToken ) { return callback(null, false); // no api key, no authorization... diff --git a/lib/cartodb/backends/pg_connection.js b/lib/cartodb/backends/pg_connection.js index 37023c90..b7d81721 100644 --- a/lib/cartodb/backends/pg_connection.js +++ b/lib/cartodb/backends/pg_connection.js @@ -31,7 +31,7 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba return callback(); }); } else if (apikeyType === 'regular') { - this.metadataBackend.getApikey(username, params.apikeyToken, (err, apikey) => { + this.metadataBackend.getApikey(username, params.api_key, (err, apikey) => { if (err) { return callback(err); } diff --git a/lib/cartodb/middleware/context/apikey-token.js b/lib/cartodb/middleware/context/apikey-token.js index f18cf60a..db43d843 100644 --- a/lib/cartodb/middleware/context/apikey-token.js +++ b/lib/cartodb/middleware/context/apikey-token.js @@ -3,7 +3,6 @@ const getApikeyTokenFromRequest = require('../lib/get_api_key_token_from_request'); module.exports = () => function apikeyTokenMiddleware(req, res, next) { - res.locals.apikeyToken = getApikeyTokenFromRequest(req); - + res.locals.api_key = getApikeyTokenFromRequest(req); return next(); }; From 3e916c6054ba7c3c210f539969a8be31530c11f5 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 15 Feb 2018 15:20:05 +0100 Subject: [PATCH 36/53] check if req.query exist before getting req.query.api_key/map_key --- lib/cartodb/middleware/lib/get_api_key_token_from_request.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/middleware/lib/get_api_key_token_from_request.js b/lib/cartodb/middleware/lib/get_api_key_token_from_request.js index 5fbc74c2..3da6f1d6 100644 --- a/lib/cartodb/middleware/lib/get_api_key_token_from_request.js +++ b/lib/cartodb/middleware/lib/get_api_key_token_from_request.js @@ -34,11 +34,11 @@ function getApikeyTokenFromHeaderAuthorization(req) { } function getApikeyTokenFromRequestQueryString(req) { - if (req.query.api_key) { + if (req.query && req.query.api_key) { return req.query.api_key; } - if (req.query.map_key) { + if (req.query && req.query.map_key) { return req.query.map_key; } From 18dbeea003cb323d7b4e4e21dadbff808bcd69a1 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 15 Feb 2018 15:20:52 +0100 Subject: [PATCH 37/53] get apikey token from request in named maps admin middleware --- lib/cartodb/controllers/named_maps_admin.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 5839e147..fe484f7f 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -1,6 +1,13 @@ const { templateName } = require('../backends/template_maps'); const cors = require('../middleware/cors'); const userMiddleware = require('../middleware/user'); +const localsMiddleware = require('../middleware/context/locals'); +const apikeyTokenMiddleware = require('../middleware/context/apikey-token'); + +const apikeyMiddleware = [ + localsMiddleware, + apikeyTokenMiddleware(), +]; /** * @param {AuthApi} authApi @@ -22,6 +29,7 @@ NamedMapsAdminController.prototype.register = function (app) { `${base_url_templated}/`, cors(), userMiddleware, + apikeyMiddleware, this.checkContentType('POST', 'POST TEMPLATE'), this.authorizedByAPIKey('create', 'POST TEMPLATE'), this.create() @@ -31,6 +39,7 @@ NamedMapsAdminController.prototype.register = function (app) { `${base_url_templated}/:template_id`, cors(), userMiddleware, + apikeyMiddleware, this.checkContentType('PUT', 'PUT TEMPLATE'), this.authorizedByAPIKey('update', 'PUT TEMPLATE'), this.update() @@ -40,6 +49,7 @@ NamedMapsAdminController.prototype.register = function (app) { `${base_url_templated}/:template_id`, cors(), userMiddleware, + apikeyMiddleware, this.authorizedByAPIKey('get', 'GET TEMPLATE'), this.retrieve() ); @@ -48,6 +58,7 @@ NamedMapsAdminController.prototype.register = function (app) { `${base_url_templated}/:template_id`, cors(), userMiddleware, + apikeyMiddleware, this.authorizedByAPIKey('delete', 'DELETE TEMPLATE'), this.destroy() ); @@ -56,6 +67,7 @@ NamedMapsAdminController.prototype.register = function (app) { `${base_url_templated}/`, cors(), userMiddleware, + apikeyMiddleware, this.authorizedByAPIKey('list', 'GET TEMPLATE LIST'), this.list() ); @@ -69,7 +81,6 @@ NamedMapsAdminController.prototype.register = function (app) { NamedMapsAdminController.prototype.authorizedByAPIKey = function (action, label) { return function authorizedByAPIKeyMiddleware (req, res, next) { const { user } = res.locals; - this.authApi.authorizedByAPIKey(user, res, (err, authenticated) => { if (err) { return next(err); From 11aa4d12bd64d37a619dbbd1911d9e148eb22f24 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 15 Feb 2018 15:27:41 +0100 Subject: [PATCH 38/53] add tests for getting api key token from requests --- test/unit/cartodb/prepare-context.test.js | 42 ++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/test/unit/cartodb/prepare-context.test.js b/test/unit/cartodb/prepare-context.test.js index cc58be4c..eabad095 100644 --- a/test/unit/cartodb/prepare-context.test.js +++ b/test/unit/cartodb/prepare-context.test.js @@ -200,7 +200,47 @@ describe('prepare-context', function() { } var query = res.locals; - assert.equal('1234', query.apikeyToken); + assert.equal('1234', query.api_key); + done(); + }); + }); + + it('from body param', function (done) { + var req = { + headers: { + host: 'localhost' + }, + body: { + api_key: '1234', + } + }; + var res = {}; + setApikeyToken(prepareRequest(req), prepareResponse(res), function (err) { + if (err) { + return done(err); + } + var query = res.locals; + + assert.equal('1234', query.api_key); + done(); + }); + }); + + it('from http header', function (done) { + var req = { + headers: { + host: 'localhost', + authorization: 'Basic dXNlcjoxMjM0', // user: user, password: 1234 + } + }; + var res = {}; + setApikeyToken(prepareRequest(req), prepareResponse(res), function (err) { + if (err) { + return done(err); + } + var query = res.locals; + + assert.equal('1234', query.api_key); done(); }); }); From cda2616a8a4eeae7f4dd2a78f1b431e94c65b954 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 15 Feb 2018 17:49:47 +0100 Subject: [PATCH 39/53] get and check api key credentials from api key: username and token --- lib/cartodb/api/auth_api.js | 10 +++ lib/cartodb/controllers/named_maps_admin.js | 4 +- .../middleware/context/apikey-credentials.js | 10 +++ .../middleware/context/apikey-token.js | 8 -- lib/cartodb/middleware/context/index.js | 4 +- .../get_api_key_credentials_from_request.js | 77 +++++++++++++++++++ .../lib/get_api_key_token_from_request.js | 62 --------------- test/unit/cartodb/prepare-context.test.js | 33 ++++++-- 8 files changed, 127 insertions(+), 81 deletions(-) create mode 100644 lib/cartodb/middleware/context/apikey-credentials.js delete mode 100644 lib/cartodb/middleware/context/apikey-token.js create mode 100644 lib/cartodb/middleware/lib/get_api_key_credentials_from_request.js delete mode 100644 lib/cartodb/middleware/lib/get_api_key_token_from_request.js diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index 9b24a16e..ef874483 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -60,6 +60,7 @@ function isValidApiKey(apikey) { // AuthApi.prototype.authorizedByAPIKey = function(user, res, callback) { const apikeyToken = res.locals.api_key; + const apikeyUsername = res.locals.apikeyUsername; if ( ! apikeyToken ) { return callback(null, false); // no api key, no authorization... @@ -78,6 +79,15 @@ AuthApi.prototype.authorizedByAPIKey = function(user, res, callback) { return callback(error); } + if (apikeyUsername && (apikeyUsername !== res.locals.user)) { + const error = new Error('Forbidden'); + error.type = 'auth'; + error.subtype = 'api-key-username-mismatch'; + error.http_status = 403; + + return callback(error); + } + if (!apikey.grantsMaps) { const error = new Error('Forbidden'); error.type = 'auth'; diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index fe484f7f..b1c6a451 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -2,11 +2,11 @@ const { templateName } = require('../backends/template_maps'); const cors = require('../middleware/cors'); const userMiddleware = require('../middleware/user'); const localsMiddleware = require('../middleware/context/locals'); -const apikeyTokenMiddleware = require('../middleware/context/apikey-token'); +const apikeyCredentialsMiddleware = require('../middleware/context/apikey-credentials'); const apikeyMiddleware = [ localsMiddleware, - apikeyTokenMiddleware(), + apikeyCredentialsMiddleware(), ]; /** diff --git a/lib/cartodb/middleware/context/apikey-credentials.js b/lib/cartodb/middleware/context/apikey-credentials.js new file mode 100644 index 00000000..820bb8c6 --- /dev/null +++ b/lib/cartodb/middleware/context/apikey-credentials.js @@ -0,0 +1,10 @@ +'use strict'; + +const getApikeyCredentialsFromRequest = require('../lib/get_api_key_credentials_from_request'); + +module.exports = () => function apikeyTokenMiddleware(req, res, next) { + const apikeyCredentials = getApikeyCredentialsFromRequest(req); + res.locals.api_key = apikeyCredentials.token; + res.locals.apikeyUsername = apikeyCredentials.username; + return next(); +}; diff --git a/lib/cartodb/middleware/context/apikey-token.js b/lib/cartodb/middleware/context/apikey-token.js deleted file mode 100644 index db43d843..00000000 --- a/lib/cartodb/middleware/context/apikey-token.js +++ /dev/null @@ -1,8 +0,0 @@ -'use strict'; - -const getApikeyTokenFromRequest = require('../lib/get_api_key_token_from_request'); - -module.exports = () => function apikeyTokenMiddleware(req, res, next) { - res.locals.api_key = getApikeyTokenFromRequest(req); - return next(); -}; diff --git a/lib/cartodb/middleware/context/index.js b/lib/cartodb/middleware/context/index.js index 96b261a4..8922739f 100644 --- a/lib/cartodb/middleware/context/index.js +++ b/lib/cartodb/middleware/context/index.js @@ -1,7 +1,7 @@ const locals = require('./locals'); const cleanUpQueryParams = require('./clean-up-query-params'); const layergroupToken = require('./layergroup-token'); -const apikeyToken = require('./apikey-token'); +const apikeyCredentials = require('./apikey-credentials'); const authorize = require('./authorize'); const dbConnSetup = require('./db-conn-setup'); @@ -10,7 +10,7 @@ module.exports = function prepareContextMiddleware(authApi, pgConnection) { locals, cleanUpQueryParams(), layergroupToken, - apikeyToken(), + apikeyCredentials(), authorize(authApi), dbConnSetup(pgConnection) ]; diff --git a/lib/cartodb/middleware/lib/get_api_key_credentials_from_request.js b/lib/cartodb/middleware/lib/get_api_key_credentials_from_request.js new file mode 100644 index 00000000..397e972b --- /dev/null +++ b/lib/cartodb/middleware/lib/get_api_key_credentials_from_request.js @@ -0,0 +1,77 @@ +'use strict'; + +const basicAuth = require('basic-auth'); + +module.exports = function getApiKeyCredentialsFromRequest(req) { + let apikeyCredentials = { + token: null, + username: null, + }; + + for (var getter of apikeyGetters) { + apikeyCredentials = getter(req); + if (apikeyTokenFound(apikeyCredentials)) { + break; + } + } + + return apikeyCredentials; +}; + +//-------------------------------------------------------------------------------- + +const apikeyGetters = [ + getApikeyTokenFromHeaderAuthorization, + getApikeyTokenFromRequestQueryString, + getApikeyTokenFromRequestBody, +]; + +function getApikeyTokenFromHeaderAuthorization(req) { + const credentials = basicAuth(req); + + if (credentials) { + return { + username: credentials.username, + token: credentials.pass + }; + } else { + return { + username: null, + token: null, + }; + } +} + +function getApikeyTokenFromRequestQueryString(req) { + let token = null; + + if (req.query && req.query.api_key) { + token = req.query.api_key; + } else if (req.query && req.query.map_key) { + token = req.query.map_key; + } + + return { + username: null, + token: token, + }; +} + +function getApikeyTokenFromRequestBody(req) { + let token = null; + + if (req.body && req.body.api_key) { + token = req.body.api_key; + } else if (req.body && req.body.map_key) { + token = req.body.map_key; + } + + return { + username: null, + token: token, + }; +} + +function apikeyTokenFound(apikey) { + return !!apikey && !!apikey.token; +} diff --git a/lib/cartodb/middleware/lib/get_api_key_token_from_request.js b/lib/cartodb/middleware/lib/get_api_key_token_from_request.js deleted file mode 100644 index 3da6f1d6..00000000 --- a/lib/cartodb/middleware/lib/get_api_key_token_from_request.js +++ /dev/null @@ -1,62 +0,0 @@ -'use strict'; - -const basicAuth = require('basic-auth'); - -module.exports = function getApiKeyTokenFromRequest(req) { - let apiKeyToken = null; - - for (var getter of apiKeyGetters) { - apiKeyToken = getter(req); - if (apiKeyTokenFound(apiKeyToken)) { - break; - } - } - - return apiKeyToken; -}; - -//-------------------------------------------------------------------------------- - -const apiKeyGetters = [ - getApikeyTokenFromHeaderAuthorization, - getApikeyTokenFromRequestQueryString, - getApikeyTokenFromRequestBody, -]; - -function getApikeyTokenFromHeaderAuthorization(req) { - const credentials = basicAuth(req); - - if (credentials) { - return credentials.pass; - } else { - return null; - } -} - -function getApikeyTokenFromRequestQueryString(req) { - if (req.query && req.query.api_key) { - return req.query.api_key; - } - - if (req.query && req.query.map_key) { - return req.query.map_key; - } - - return null; -} - -function getApikeyTokenFromRequestBody(req) { - if (req.body && req.body.api_key) { - return req.body.api_key; - } - - if (req.body && req.body.map_key) { - return req.body.map_key; - } - - return null; -} - -function apiKeyTokenFound(apiKeyToken) { - return !!apiKeyToken; -} diff --git a/test/unit/cartodb/prepare-context.test.js b/test/unit/cartodb/prepare-context.test.js index eabad095..61d7b514 100644 --- a/test/unit/cartodb/prepare-context.test.js +++ b/test/unit/cartodb/prepare-context.test.js @@ -10,7 +10,7 @@ var TemplateMaps = require('../../../lib/cartodb/backends/template_maps'); const cleanUpQueryParamsMiddleware = require('../../../lib/cartodb/middleware/context/clean-up-query-params'); const authorizeMiddleware = require('../../../lib/cartodb/middleware/context/authorize'); const dbConnSetupMiddleware = require('../../../lib/cartodb/middleware/context/db-conn-setup'); -const apikeyTokenMiddleware = require('../../../lib/cartodb/middleware/context/apikey-token'); +const apikeyCredentialsMiddleware = require('../../../lib/cartodb/middleware/context/apikey-credentials'); const localsMiddleware = require('../../../lib/cartodb/middleware/context/locals'); var windshaft = require('windshaft'); @@ -24,7 +24,7 @@ describe('prepare-context', function() { let cleanUpQueryParams; let dbConnSetup; let authorize; - let setApikeyToken; + let setApikeyCredentials; before(function() { var redisPool = new RedisPool(global.environment.redis); @@ -37,7 +37,7 @@ describe('prepare-context', function() { cleanUpQueryParams = cleanUpQueryParamsMiddleware(); authorize = authorizeMiddleware(authApi); dbConnSetup = dbConnSetupMiddleware(pgConnection); - setApikeyToken = apikeyTokenMiddleware(); + setApikeyCredentials = apikeyCredentialsMiddleware(); }); @@ -194,7 +194,7 @@ describe('prepare-context', function() { } }; var res = {}; - setApikeyToken(prepareRequest(req), prepareResponse(res), function (err) { + setApikeyCredentials(prepareRequest(req), prepareResponse(res), function (err) { if (err) { return done(err); } @@ -215,7 +215,7 @@ describe('prepare-context', function() { } }; var res = {}; - setApikeyToken(prepareRequest(req), prepareResponse(res), function (err) { + setApikeyCredentials(prepareRequest(req), prepareResponse(res), function (err) { if (err) { return done(err); } @@ -230,11 +230,30 @@ describe('prepare-context', function() { var req = { headers: { host: 'localhost', - authorization: 'Basic dXNlcjoxMjM0', // user: user, password: 1234 + authorization: 'Basic bG9jYWxob3N0OjEyMzQ=', // user: localhost, password: 1234 } }; var res = {}; - setApikeyToken(prepareRequest(req), prepareResponse(res), function (err) { + setApikeyCredentials(prepareRequest(req), prepareResponse(res), function (err) { + if (err) { + return done(err); + } + var query = res.locals; + + assert.equal('1234', query.api_key); + done(); + }); + }); + + it('fail from http header with user mismatch', function (done) { + var req = { + headers: { + host: 'localhost', + authorization: 'Basic YW5vdGhlcl91c2VyOjEyMzQ=', // user: another_user, password: 1234 + } + }; + var res = {}; + setApikeyCredentials(prepareRequest(req), prepareResponse(res), function (err) { if (err) { return done(err); } From e84d88b7a35378042ccdf9a9aed85d2da588f627 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Thu, 15 Feb 2018 17:53:15 +0100 Subject: [PATCH 40/53] remove test --- test/unit/cartodb/prepare-context.test.js | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/test/unit/cartodb/prepare-context.test.js b/test/unit/cartodb/prepare-context.test.js index 61d7b514..2eb7e890 100644 --- a/test/unit/cartodb/prepare-context.test.js +++ b/test/unit/cartodb/prepare-context.test.js @@ -244,24 +244,5 @@ describe('prepare-context', function() { done(); }); }); - - it('fail from http header with user mismatch', function (done) { - var req = { - headers: { - host: 'localhost', - authorization: 'Basic YW5vdGhlcl91c2VyOjEyMzQ=', // user: another_user, password: 1234 - } - }; - var res = {}; - setApikeyCredentials(prepareRequest(req), prepareResponse(res), function (err) { - if (err) { - return done(err); - } - var query = res.locals; - - assert.equal('1234', query.api_key); - done(); - }); - }); }); }); From 4ac224688c3c036796a969380f5a3054de9a09e3 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Fri, 16 Feb 2018 11:20:04 +0100 Subject: [PATCH 41/53] in fallback mode, use default api key if api key token doesnt exist --- lib/cartodb/api/auth_api.js | 2 ++ lib/cartodb/backends/pg_connection.js | 9 +++++- .../acceptance/auth/authorization-fallback.js | 32 ++++++++++++++++--- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index 1ac2a1cc..0b93e308 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -88,6 +88,8 @@ AuthApi.prototype.authorizedByAPIKey = function(user, req, callback) { } if ( !isValidApiKey(apikey)) { + return callback(null, true); // AUTH_FALLBACK :S If api key not found, use default_public + const error = new Error('Unauthorized'); error.type = 'auth'; error.subtype = 'api-key-not-found'; diff --git a/lib/cartodb/backends/pg_connection.js b/lib/cartodb/backends/pg_connection.js index d5d3ecdb..c3eff05b 100644 --- a/lib/cartodb/backends/pg_connection.js +++ b/lib/cartodb/backends/pg_connection.js @@ -36,7 +36,7 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba return callback(); }); - } else if (apikeyType === 'regular') { + } else if (apikeyType === 'regular') { //Actually it can be any type of api key this.metadataBackend.getApikey(username, params.api_key || params.map_key, (err, apikey) => { if (err) { return callback(err); @@ -57,6 +57,13 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba params.dbpassword = global.environment.postgres.password; } + //Remove this block when Auth fallback is not used anymore + // AUTH_FALLBACK + // If api key not found use default + if (!params.dbuser && !params.dbpassword) { + return this.setDBAuth(username, params, 'default', callback); + } + return callback(); }); } else if (apikeyType === 'default') { diff --git a/test/acceptance/auth/authorization-fallback.js b/test/acceptance/auth/authorization-fallback.js index 032a8d08..8546c8ff 100644 --- a/test/acceptance/auth/authorization-fallback.js +++ b/test/acceptance/auth/authorization-fallback.js @@ -101,7 +101,6 @@ describe('authorization fallback', function () { ); }); - it("succeed with default - sending no api key token", function (done) { var layergroup = singleLayergroupConfig(pointSqlPublic, '#layer { marker-fill:red; }'); @@ -125,17 +124,24 @@ describe('authorization fallback', function () { ); }); - it("fail with non-existent api key", function (done) { - var layergroup = singleLayergroupConfig(pointSqlMaster, '#layer { marker-fill:red; }'); + it("succeed with non-existent api key - defaults to default", function (done) { + var layergroup = singleLayergroupConfig(pointSqlPublic, '#layer { marker-fill:red; }'); assert.response(server, createRequest(layergroup, 'user_previous_to_project_auth', 'THIS-API-KEY-DOESNT-EXIST'), { - status: 401 + status: 200 }, function (res, err) { assert.ifError(err); + var parsed = JSON.parse(res.body); + assert.ok(parsed.layergroupid); + assert.equal(res.headers['x-layergroup-id'], parsed.layergroupid); + + keysToDelete['map_cfg|' + LayergroupToken.parse(parsed.layergroupid).token] = 0; + keysToDelete['user:user_previous_to_project_auth:mapviews:global'] = 5; + done(); } ); @@ -156,4 +162,20 @@ describe('authorization fallback', function () { } ); }); -}); \ No newline at end of file + + it("fail with non-existent api key - defaults to default", function (done) { + var layergroup = singleLayergroupConfig(pointSqlMaster, '#layer { marker-fill:red; }'); + + assert.response(server, + createRequest(layergroup, 'user_previous_to_project_auth', 'THIS-API-KEY-DOESNT-EXIST'), + { + status: 403 + }, + function (res, err) { + assert.ifError(err); + + done(); + } + ); + }); +}); From 5823859b2abed3d398735b3f65bda7cb186e2d23 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Fri, 16 Feb 2018 12:00:05 +0100 Subject: [PATCH 42/53] update package.json to use the same branch form carto redis --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index b2dc6652..5256f4d7 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "camshaft": "0.61.2", "cartodb-psql": "0.10.2", "cartodb-query-tables": "0.3.0", - "cartodb-redis": "cartodb/node-cartodb-redis#project-auth-api", + "cartodb-redis": "cartodb/node-cartodb-redis#project-auth-api-fallback", "debug": "^3.1.0", "dot": "~1.0.2", "express": "~4.16.0", From 7e14247ea959a089e54b5a068811db21eaf5a54c Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Mon, 19 Feb 2018 17:06:59 +0100 Subject: [PATCH 43/53] remove cause of unreachable code/dead code. Not necessary because carto redis assures at least the default api key --- lib/cartodb/api/auth_api.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index 677ccc86..43f19d4e 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -86,8 +86,6 @@ AuthApi.prototype.authorizedByAPIKey = function(user, res, callback) { } if ( !isValidApiKey(apikey)) { - return callback(null, true); // AUTH_FALLBACK :S If api key not found, use default_public - const error = new Error('Unauthorized'); error.type = 'auth'; error.subtype = 'api-key-not-found'; From 47ccb7ded8a359d66b9bb618b30651c8b7364f50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 19 Feb 2018 17:57:46 +0100 Subject: [PATCH 44/53] Point to main development branch of cartodb-redis --- package.json | 2 +- yarn.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 5256f4d7..b2dc6652 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "camshaft": "0.61.2", "cartodb-psql": "0.10.2", "cartodb-query-tables": "0.3.0", - "cartodb-redis": "cartodb/node-cartodb-redis#project-auth-api-fallback", + "cartodb-redis": "cartodb/node-cartodb-redis#project-auth-api", "debug": "^3.1.0", "dot": "~1.0.2", "express": "~4.16.0", diff --git a/yarn.lock b/yarn.lock index c556c94e..6ce5a4b7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -303,7 +303,7 @@ cartodb-query-tables@0.3.0: cartodb-redis@cartodb/node-cartodb-redis#project-auth-api: version "0.15.1" - resolved "https://codeload.github.com/cartodb/node-cartodb-redis/tar.gz/2d17fe82d66972edce5c727c679a335845ea5eb0" + resolved "https://codeload.github.com/cartodb/node-cartodb-redis/tar.gz/ffc1dccaa5ad433a1582d8e3d5926063bd3851d3" dependencies: dot "~1.0.2" redis-mpool "^0.5.0" From 2e3abfb2cd1608fb480a5460be881dd0fd31251b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 19 Feb 2018 18:28:58 +0100 Subject: [PATCH 45/53] Catch "name not found" errors from metadata backend and set http code status 404 --- lib/cartodb/api/auth_api.js | 20 ++++++++++++-------- lib/cartodb/backends/pg_connection.js | 11 ++++++++++- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index 43f19d4e..bacccda7 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -70,6 +70,10 @@ AuthApi.prototype.authorizedByAPIKey = function(user, res, callback) { this.metadataBackend.getApikey(user, apikeyToken, (err, apikey) => { if (err) { + if (err.message && -1 !== err.message.indexOf('name not found')) { + err.http_status = 404; + } + return callback(err); } @@ -100,7 +104,7 @@ AuthApi.prototype.authorizedByAPIKey = function(user, res, callback) { error.subtype = 'api-key-username-mismatch'; error.http_status = 403; - return callback(error); + return callback(error); } if (!apikey.grantsMaps) { @@ -137,8 +141,8 @@ AuthApi.prototype.authorize = function(req, res, callback) { if (err) { return callback(err); - } - + } + callback(null, true); }); } @@ -147,17 +151,17 @@ AuthApi.prototype.authorize = function(req, res, callback) { if (err) { return callback(err); } - + if (isAuthorizedBySigner) { return this.pgConnection.setDBAuth(user, res.locals, 'master', function (err) { req.profiler.done('setDBAuth'); - + if (err) { return callback(err); - } + } callback(null, true); - }); + }); } // if no signer name was given, use default api key @@ -170,7 +174,7 @@ AuthApi.prototype.authorize = function(req, res, callback) { } callback(null, true); - }); + }); } // if signer name was given, return no authorization diff --git a/lib/cartodb/backends/pg_connection.js b/lib/cartodb/backends/pg_connection.js index e4f9700e..71ec2928 100644 --- a/lib/cartodb/backends/pg_connection.js +++ b/lib/cartodb/backends/pg_connection.js @@ -22,6 +22,9 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba if (apikeyType === 'master') { this.metadataBackend.getMasterApikey(username, (err, apikey) => { if (err) { + if (err.message && -1 !== err.message.indexOf('name not found')) { + err.http_status = 404; + } return callback(err); } @@ -39,6 +42,9 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba } else if (apikeyType === 'regular') { //Actually it can be any type of api key this.metadataBackend.getApikey(username, params.api_key, (err, apikey) => { if (err) { + if (err.message && -1 !== err.message.indexOf('name not found')) { + err.http_status = 404; + } return callback(err); } @@ -64,11 +70,14 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba return this.setDBAuth(username, params, 'default', callback); } - return callback(); + return callback(); }); } else if (apikeyType === 'default') { this.metadataBackend.getApikey(username, 'default_public', (err, apikey) => { if (err) { + if (err.message && -1 !== err.message.indexOf('name not found')) { + err.http_status = 404; + } return callback(err); } From 603ef4044c10ce1c6b46dac54f620638ee528cd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 19 Feb 2018 18:48:02 +0100 Subject: [PATCH 46/53] Reduce cyclomatic complexity --- lib/cartodb/api/auth_api.js | 55 +++++++++++++++++++++------ lib/cartodb/backends/pg_connection.js | 11 ++++-- 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index bacccda7..8782f907 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -70,7 +70,7 @@ AuthApi.prototype.authorizedByAPIKey = function(user, res, callback) { this.metadataBackend.getApikey(user, apikeyToken, (err, apikey) => { if (err) { - if (err.message && -1 !== err.message.indexOf('name not found')) { + if (isNameNotFoundError(err)) { err.http_status = 404; } @@ -79,15 +79,8 @@ AuthApi.prototype.authorizedByAPIKey = function(user, res, callback) { //Remove this block when Auth fallback is not used anymore // AUTH_FALLBACK - if (!apikey.databaseRole && apikey.user_id && global.environment.postgres_auth_user) { - apikey.databaseRole = _.template(global.environment.postgres_auth_user, apikey); - } - - //Remove this block when Auth fallback is not used anymore - // AUTH_FALLBACK - if (!apikey.databasePassword && global.environment.postgres.password) { - apikey.databasePassword = global.environment.postgres.password; - } + apikey.databaseRole = composeUserDatabase(apikey); + apikey.databasePassword = composeDatabasePassword(apikey); if ( !isValidApiKey(apikey)) { const error = new Error('Unauthorized'); @@ -98,7 +91,7 @@ AuthApi.prototype.authorizedByAPIKey = function(user, res, callback) { return callback(error); } - if (apikeyUsername && (apikeyUsername !== res.locals.user)) { + if (!usernameMatches(apikeyUsername, res.locals.user)) { const error = new Error('Forbidden'); error.type = 'auth'; error.subtype = 'api-key-username-mismatch'; @@ -120,6 +113,46 @@ AuthApi.prototype.authorizedByAPIKey = function(user, res, callback) { }); }; +//Remove this block when Auth fallback is not used anymore +// AUTH_FALLBACK +function composeUserDatabase (apikey) { + if (shouldComposeUserDatabase(apikey)) { + return _.template(global.environment.postgres_auth_user, apikey); + } + + return apikey.databaseRole; +} + +//Remove this block when Auth fallback is not used anymore +// AUTH_FALLBACK +function composeDatabasePassword (apikey) { + if (shouldComposeDatabasePassword(apikey)) { + return global.environment.postgres.password; + } + + return apikey.databasePassword; +} + +//Remove this block when Auth fallback is not used anymore +// AUTH_FALLBACK +function shouldComposeDatabasePassword (apikey) { + return !apikey.databasePassword && global.environment.postgres.password; +} + +//Remove this block when Auth fallback is not used anymore +// AUTH_FALLBACK +function shouldComposeUserDatabase(apikey) { + return !apikey.databaseRole && apikey.user_id && global.environment.postgres_auth_user; +} + +function isNameNotFoundError (err) { + return err.message && -1 !== err.message.indexOf('name not found'); +} + +function usernameMatches (apikeyUsername, requestUsername) { + return !(apikeyUsername && (apikeyUsername !== requestUsername)); +} + /** * Check access authorization * diff --git a/lib/cartodb/backends/pg_connection.js b/lib/cartodb/backends/pg_connection.js index 71ec2928..290fb6ef 100644 --- a/lib/cartodb/backends/pg_connection.js +++ b/lib/cartodb/backends/pg_connection.js @@ -22,7 +22,7 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba if (apikeyType === 'master') { this.metadataBackend.getMasterApikey(username, (err, apikey) => { if (err) { - if (err.message && -1 !== err.message.indexOf('name not found')) { + if (isNameNotFoundError(err)) { err.http_status = 404; } return callback(err); @@ -42,7 +42,7 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba } else if (apikeyType === 'regular') { //Actually it can be any type of api key this.metadataBackend.getApikey(username, params.api_key, (err, apikey) => { if (err) { - if (err.message && -1 !== err.message.indexOf('name not found')) { + if (isNameNotFoundError(err)) { err.http_status = 404; } return callback(err); @@ -75,7 +75,7 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba } else if (apikeyType === 'default') { this.metadataBackend.getApikey(username, 'default_public', (err, apikey) => { if (err) { - if (err.message && -1 !== err.message.indexOf('name not found')) { + if (isNameNotFoundError(err)) { err.http_status = 404; } return callback(err); @@ -97,6 +97,11 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba } }; +function isNameNotFoundError (err) { + return err.message && -1 !== err.message.indexOf('name not found'); +} + + // Set db connection parameters to those for the given username // // @param dbowner cartodb username of database owner, From 7c7d606aa78421b5b24eb47fc85f8f2605efdf56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 19 Feb 2018 19:05:13 +0100 Subject: [PATCH 47/53] Remove trailing spaces --- .../lib/get_api_key_credentials_from_request.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/cartodb/middleware/lib/get_api_key_credentials_from_request.js b/lib/cartodb/middleware/lib/get_api_key_credentials_from_request.js index 397e972b..e3c04675 100644 --- a/lib/cartodb/middleware/lib/get_api_key_credentials_from_request.js +++ b/lib/cartodb/middleware/lib/get_api_key_credentials_from_request.js @@ -28,7 +28,7 @@ const apikeyGetters = [ function getApikeyTokenFromHeaderAuthorization(req) { const credentials = basicAuth(req); - + if (credentials) { return { username: credentials.username, @@ -36,7 +36,7 @@ function getApikeyTokenFromHeaderAuthorization(req) { }; } else { return { - username: null, + username: null, token: null, }; } @@ -44,13 +44,13 @@ function getApikeyTokenFromHeaderAuthorization(req) { function getApikeyTokenFromRequestQueryString(req) { let token = null; - + if (req.query && req.query.api_key) { token = req.query.api_key; } else if (req.query && req.query.map_key) { token = req.query.map_key; } - + return { username: null, token: token, @@ -59,7 +59,7 @@ function getApikeyTokenFromRequestQueryString(req) { function getApikeyTokenFromRequestBody(req) { let token = null; - + if (req.body && req.body.api_key) { token = req.body.api_key; } else if (req.body && req.body.map_key) { From 59ca00b33b814cad978c79ba61b12a5cc7b11102 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Tue, 20 Feb 2018 12:31:36 +0100 Subject: [PATCH 48/53] move apikey credentials getter to middleware file --- .../middleware/context/apikey-credentials.js | 78 ++++++++++++++++++- .../get_api_key_credentials_from_request.js | 77 ------------------ 2 files changed, 76 insertions(+), 79 deletions(-) delete mode 100644 lib/cartodb/middleware/lib/get_api_key_credentials_from_request.js diff --git a/lib/cartodb/middleware/context/apikey-credentials.js b/lib/cartodb/middleware/context/apikey-credentials.js index 820bb8c6..bde45da4 100644 --- a/lib/cartodb/middleware/context/apikey-credentials.js +++ b/lib/cartodb/middleware/context/apikey-credentials.js @@ -1,10 +1,84 @@ 'use strict'; -const getApikeyCredentialsFromRequest = require('../lib/get_api_key_credentials_from_request'); - module.exports = () => function apikeyTokenMiddleware(req, res, next) { const apikeyCredentials = getApikeyCredentialsFromRequest(req); res.locals.api_key = apikeyCredentials.token; res.locals.apikeyUsername = apikeyCredentials.username; return next(); }; + +//-------------------------------------------------------------------------------- + +const basicAuth = require('basic-auth'); + +function getApikeyCredentialsFromRequest(req) { + let apikeyCredentials = { + token: null, + username: null, + }; + + for (let getter of apikeyGetters) { + apikeyCredentials = getter(req); + if (apikeyTokenFound(apikeyCredentials)) { + break; + } + } + + return apikeyCredentials; +} + +const apikeyGetters = [ + getApikeyTokenFromHeaderAuthorization, + getApikeyTokenFromRequestQueryString, + getApikeyTokenFromRequestBody, +]; + +function getApikeyTokenFromHeaderAuthorization(req) { + const credentials = basicAuth(req); + + if (credentials) { + return { + username: credentials.username, + token: credentials.pass + }; + } else { + return { + username: null, + token: null, + }; + } +} + +function getApikeyTokenFromRequestQueryString(req) { + let token = null; + + if (req.query && req.query.api_key) { + token = req.query.api_key; + } else if (req.query && req.query.map_key) { + token = req.query.map_key; + } + + return { + username: null, + token: token, + }; +} + +function getApikeyTokenFromRequestBody(req) { + let token = null; + + if (req.body && req.body.api_key) { + token = req.body.api_key; + } else if (req.body && req.body.map_key) { + token = req.body.map_key; + } + + return { + username: null, + token: token, + }; +} + +function apikeyTokenFound(apikey) { + return !!apikey && !!apikey.token; +} diff --git a/lib/cartodb/middleware/lib/get_api_key_credentials_from_request.js b/lib/cartodb/middleware/lib/get_api_key_credentials_from_request.js deleted file mode 100644 index e3c04675..00000000 --- a/lib/cartodb/middleware/lib/get_api_key_credentials_from_request.js +++ /dev/null @@ -1,77 +0,0 @@ -'use strict'; - -const basicAuth = require('basic-auth'); - -module.exports = function getApiKeyCredentialsFromRequest(req) { - let apikeyCredentials = { - token: null, - username: null, - }; - - for (var getter of apikeyGetters) { - apikeyCredentials = getter(req); - if (apikeyTokenFound(apikeyCredentials)) { - break; - } - } - - return apikeyCredentials; -}; - -//-------------------------------------------------------------------------------- - -const apikeyGetters = [ - getApikeyTokenFromHeaderAuthorization, - getApikeyTokenFromRequestQueryString, - getApikeyTokenFromRequestBody, -]; - -function getApikeyTokenFromHeaderAuthorization(req) { - const credentials = basicAuth(req); - - if (credentials) { - return { - username: credentials.username, - token: credentials.pass - }; - } else { - return { - username: null, - token: null, - }; - } -} - -function getApikeyTokenFromRequestQueryString(req) { - let token = null; - - if (req.query && req.query.api_key) { - token = req.query.api_key; - } else if (req.query && req.query.map_key) { - token = req.query.map_key; - } - - return { - username: null, - token: token, - }; -} - -function getApikeyTokenFromRequestBody(req) { - let token = null; - - if (req.body && req.body.api_key) { - token = req.body.api_key; - } else if (req.body && req.body.map_key) { - token = req.body.map_key; - } - - return { - username: null, - token: token, - }; -} - -function apikeyTokenFound(apikey) { - return !!apikey && !!apikey.token; -} From 521b441da5e60d96feae59dafba341b96eb547d0 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Tue, 20 Feb 2018 12:53:33 +0100 Subject: [PATCH 49/53] default apikey is returned by metadata module if no apikey found, remove this code because is never going to be run --- lib/cartodb/backends/pg_connection.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/cartodb/backends/pg_connection.js b/lib/cartodb/backends/pg_connection.js index 290fb6ef..d5c75a9c 100644 --- a/lib/cartodb/backends/pg_connection.js +++ b/lib/cartodb/backends/pg_connection.js @@ -53,23 +53,18 @@ PgConnection.prototype.setDBAuth = function(username, params, apikeyType, callba //Remove this block when Auth fallback is not used anymore // AUTH_FALLBACK + // master apikey has been recreated from user's metadata if (!params.dbuser && apikey.user_id && apikey.type === 'master' && global.environment.postgres_auth_user) { params.dbuser = _.template(global.environment.postgres_auth_user, apikey); } //Remove this block when Auth fallback is not used anymore // AUTH_FALLBACK + // default apikey has been recreated from user's metadata if (!params.dbpassword && global.environment.postgres.password) { params.dbpassword = global.environment.postgres.password; } - //Remove this block when Auth fallback is not used anymore - // AUTH_FALLBACK - // If api key not found use default - if (!params.dbuser && !params.dbpassword) { - return this.setDBAuth(username, params, 'default', callback); - } - return callback(); }); } else if (apikeyType === 'default') { From 8867cdbc02ed47afd8ae5f6f7c482e603872fb07 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Mon, 26 Feb 2018 15:57:42 +0100 Subject: [PATCH 50/53] use anonymous function instead of arrow function in middleware export to don't bind this --- lib/cartodb/middleware/context/apikey-credentials.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/cartodb/middleware/context/apikey-credentials.js b/lib/cartodb/middleware/context/apikey-credentials.js index bde45da4..5302050e 100644 --- a/lib/cartodb/middleware/context/apikey-credentials.js +++ b/lib/cartodb/middleware/context/apikey-credentials.js @@ -1,10 +1,12 @@ 'use strict'; -module.exports = () => function apikeyTokenMiddleware(req, res, next) { - const apikeyCredentials = getApikeyCredentialsFromRequest(req); - res.locals.api_key = apikeyCredentials.token; - res.locals.apikeyUsername = apikeyCredentials.username; - return next(); +module.exports = function() { + return function apikeyTokenMiddleware(req, res, next) { + const apikeyCredentials = getApikeyCredentialsFromRequest(req); + res.locals.api_key = apikeyCredentials.token; + res.locals.apikeyUsername = apikeyCredentials.username; + return next(); + }; }; //-------------------------------------------------------------------------------- From 26df09b13fa828f542dab91d0ed1a0cedf147d26 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 28 Feb 2018 11:42:44 +0100 Subject: [PATCH 51/53] require debug at the top of file --- lib/cartodb/backends/pg_connection.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/backends/pg_connection.js b/lib/cartodb/backends/pg_connection.js index d5c75a9c..f389c687 100644 --- a/lib/cartodb/backends/pg_connection.js +++ b/lib/cartodb/backends/pg_connection.js @@ -1,5 +1,6 @@ var PSQL = require('cartodb-psql'); var _ = require('underscore'); +const debug = require('debug')('cachechan'); function PgConnection(metadataBackend) { this.metadataBackend = metadataBackend; @@ -141,7 +142,7 @@ PgConnection.prototype.setDBConn = function(dbowner, params, callback) { */ PgConnection.prototype.getConnection = function(username, callback) { - require('debug')('cachechan')("getConn1"); + debug("getConn1"); this.getDatabaseParams(username, (err, databaseParams) => { if (err) { From 102b11b1b5e16726312c9eb82c55b4f8d6bb165b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 28 Feb 2018 13:10:46 +0100 Subject: [PATCH 52/53] Follow middleware naming convention --- lib/cartodb/middleware/context/apikey-credentials.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/middleware/context/apikey-credentials.js b/lib/cartodb/middleware/context/apikey-credentials.js index 5302050e..225c8ab6 100644 --- a/lib/cartodb/middleware/context/apikey-credentials.js +++ b/lib/cartodb/middleware/context/apikey-credentials.js @@ -1,6 +1,6 @@ 'use strict'; -module.exports = function() { +module.exports = function apikeyToken () { return function apikeyTokenMiddleware(req, res, next) { const apikeyCredentials = getApikeyCredentialsFromRequest(req); res.locals.api_key = apikeyCredentials.token; From 8eba5dcc01b4025084d5f69d76b843c37bdfc1c6 Mon Sep 17 00:00:00 2001 From: Eneko Lakasta Date: Wed, 28 Feb 2018 17:07:31 +0100 Subject: [PATCH 53/53] update cartodb-redis to 0.16.0 --- package.json | 2 +- yarn.lock | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index c904691d..8b87d90a 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "camshaft": "0.61.2", "cartodb-psql": "0.10.2", "cartodb-query-tables": "0.3.0", - "cartodb-redis": "cartodb/node-cartodb-redis#project-auth-api", + "cartodb-redis": "0.16.0", "debug": "^3.1.0", "dot": "~1.0.2", "express": "~4.16.0", diff --git a/yarn.lock b/yarn.lock index ff5f9d35..0c7d698c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -301,9 +301,9 @@ cartodb-query-tables@0.3.0: version "0.3.0" resolved "https://registry.yarnpkg.com/cartodb-query-tables/-/cartodb-query-tables-0.3.0.tgz#56e18d869666eb2e8e2cb57d0baf3acc923f8756" -cartodb-redis@cartodb/node-cartodb-redis#project-auth-api: - version "0.15.1" - resolved "https://codeload.github.com/cartodb/node-cartodb-redis/tar.gz/ffc1dccaa5ad433a1582d8e3d5926063bd3851d3" +cartodb-redis@0.16.0: + version "0.16.0" + resolved "https://registry.yarnpkg.com/cartodb-redis/-/cartodb-redis-0.16.0.tgz#969312fd329b24a76bf6e5a4dd961445f2eda734" dependencies: dot "~1.0.2" redis-mpool "^0.5.0"