From 62deda6470dfad720d6412edd7e093fdb27e40cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 28 Feb 2018 19:13:49 +0100 Subject: [PATCH 01/27] Improve naming --- lib/cartodb/api/auth_api.js | 8 ++++---- lib/cartodb/middleware/context/apikey-credentials.js | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index 8782f907..e9f10262 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -62,7 +62,7 @@ function isValidApiKey(apikey) { // AuthApi.prototype.authorizedByAPIKey = function(user, res, callback) { const apikeyToken = res.locals.api_key; - const apikeyUsername = res.locals.apikeyUsername; + const basicAuthUsername = res.locals.basicAuthUsername; if ( ! apikeyToken ) { return callback(null, false); // no api key, no authorization... @@ -91,7 +91,7 @@ AuthApi.prototype.authorizedByAPIKey = function(user, res, callback) { return callback(error); } - if (!usernameMatches(apikeyUsername, res.locals.user)) { + if (!usernameMatches(basicAuthUsername, res.locals.user)) { const error = new Error('Forbidden'); error.type = 'auth'; error.subtype = 'api-key-username-mismatch'; @@ -149,8 +149,8 @@ function isNameNotFoundError (err) { return err.message && -1 !== err.message.indexOf('name not found'); } -function usernameMatches (apikeyUsername, requestUsername) { - return !(apikeyUsername && (apikeyUsername !== requestUsername)); +function usernameMatches (basicAuthUsername, requestUsername) { + return !(basicAuthUsername && (basicAuthUsername !== requestUsername)); } /** diff --git a/lib/cartodb/middleware/context/apikey-credentials.js b/lib/cartodb/middleware/context/apikey-credentials.js index 225c8ab6..b70718ab 100644 --- a/lib/cartodb/middleware/context/apikey-credentials.js +++ b/lib/cartodb/middleware/context/apikey-credentials.js @@ -3,8 +3,10 @@ module.exports = function apikeyToken () { return function apikeyTokenMiddleware(req, res, next) { const apikeyCredentials = getApikeyCredentialsFromRequest(req); + res.locals.api_key = apikeyCredentials.token; - res.locals.apikeyUsername = apikeyCredentials.username; + res.locals.basicAuthUsername = apikeyCredentials.username; + return next(); }; }; From 42deb7abbec043f5a75579e885e56c90308dc38b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 28 Feb 2018 19:20:51 +0100 Subject: [PATCH 02/27] Rename middleware --- lib/cartodb/controllers/named_maps_admin.js | 4 +-- .../{apikey-credentials.js => credentials.js} | 4 +-- lib/cartodb/middleware/context/index.js | 4 +-- test/unit/cartodb/prepare-context.test.js | 30 +++++++++---------- 4 files changed, 21 insertions(+), 21 deletions(-) rename lib/cartodb/middleware/context/{apikey-credentials.js => credentials.js} (94%) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index b1c6a451..3c501ee1 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 apikeyCredentialsMiddleware = require('../middleware/context/apikey-credentials'); +const credentialsMiddleware = require('../middleware/context/credentials'); const apikeyMiddleware = [ localsMiddleware, - apikeyCredentialsMiddleware(), + credentialsMiddleware(), ]; /** diff --git a/lib/cartodb/middleware/context/apikey-credentials.js b/lib/cartodb/middleware/context/credentials.js similarity index 94% rename from lib/cartodb/middleware/context/apikey-credentials.js rename to lib/cartodb/middleware/context/credentials.js index b70718ab..25851e36 100644 --- a/lib/cartodb/middleware/context/apikey-credentials.js +++ b/lib/cartodb/middleware/context/credentials.js @@ -1,7 +1,7 @@ 'use strict'; -module.exports = function apikeyToken () { - return function apikeyTokenMiddleware(req, res, next) { +module.exports = function credentials () { + return function credentialsMiddleware(req, res, next) { const apikeyCredentials = getApikeyCredentialsFromRequest(req); res.locals.api_key = apikeyCredentials.token; diff --git a/lib/cartodb/middleware/context/index.js b/lib/cartodb/middleware/context/index.js index 8922739f..9394cfd0 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 apikeyCredentials = require('./apikey-credentials'); +const credentials = require('./credentials'); const authorize = require('./authorize'); const dbConnSetup = require('./db-conn-setup'); @@ -10,7 +10,7 @@ module.exports = function prepareContextMiddleware(authApi, pgConnection) { locals, cleanUpQueryParams(), layergroupToken, - apikeyCredentials(), + credentials(), authorize(authApi), dbConnSetup(pgConnection) ]; diff --git a/test/unit/cartodb/prepare-context.test.js b/test/unit/cartodb/prepare-context.test.js index 2eb7e890..802063eb 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 apikeyCredentialsMiddleware = require('../../../lib/cartodb/middleware/context/apikey-credentials'); +const credentialsMiddleware = require('../../../lib/cartodb/middleware/context/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 setApikeyCredentials; + let setCredentials; 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); - setApikeyCredentials = apikeyCredentialsMiddleware(); + setCredentials = credentialsMiddleware(); }); @@ -74,7 +74,7 @@ describe('prepare-context', function() { done(); }); }); - + it('cleans up request', function(done){ var req = {headers: { host:'localhost' }, query: {dbuser:'hacker',dbname:'secret'}}; var res = {}; @@ -106,18 +106,18 @@ describe('prepare-context', function() { }); it('sets also dbuser for authenticated requests', function(done){ - var req = { - headers: { - host: 'localhost' - }, + var req = { + headers: { + host: 'localhost' + }, query: { api_key: '1234' } }; - var res = { + var res = { set: function () {}, locals: { - api_key: '1234' + api_key: '1234' } }; @@ -169,7 +169,7 @@ describe('prepare-context', function() { } }; var res = {}; - + cleanUpQueryParams(prepareRequest(req), prepareResponse(res), function (err) { if ( err ) { return done(err); @@ -194,12 +194,12 @@ describe('prepare-context', function() { } }; var res = {}; - setApikeyCredentials(prepareRequest(req), prepareResponse(res), function (err) { + setCredentials(prepareRequest(req), prepareResponse(res), function (err) { if (err) { return done(err); } var query = res.locals; - + assert.equal('1234', query.api_key); done(); }); @@ -215,7 +215,7 @@ describe('prepare-context', function() { } }; var res = {}; - setApikeyCredentials(prepareRequest(req), prepareResponse(res), function (err) { + setCredentials(prepareRequest(req), prepareResponse(res), function (err) { if (err) { return done(err); } @@ -234,7 +234,7 @@ describe('prepare-context', function() { } }; var res = {}; - setApikeyCredentials(prepareRequest(req), prepareResponse(res), function (err) { + setCredentials(prepareRequest(req), prepareResponse(res), function (err) { if (err) { return done(err); } From c0830862c89a82be91fb4439896554163c045687 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 28 Feb 2018 19:21:44 +0100 Subject: [PATCH 03/27] Follow middleware naming convention --- lib/cartodb/middleware/context/authorize.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/middleware/context/authorize.js b/lib/cartodb/middleware/context/authorize.js index a42b5407..b37cf639 100644 --- a/lib/cartodb/middleware/context/authorize.js +++ b/lib/cartodb/middleware/context/authorize.js @@ -1,5 +1,5 @@ -module.exports = function authorizeMiddleware (authApi) { - return function (req, res, next) { +module.exports = function authorize (authApi) { + return function authorizeMiddleware (req, res, next) { req.profiler.done('req2params.setup'); authApi.authorize(req, res, (err, authorized) => { From 48c5a458f3cf1be964389d35beb82c177c562bd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 28 Feb 2018 19:22:22 +0100 Subject: [PATCH 04/27] Remove bad use of profiling step --- lib/cartodb/middleware/context/authorize.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/cartodb/middleware/context/authorize.js b/lib/cartodb/middleware/context/authorize.js index b37cf639..d4af0d0e 100644 --- a/lib/cartodb/middleware/context/authorize.js +++ b/lib/cartodb/middleware/context/authorize.js @@ -1,7 +1,5 @@ module.exports = function authorize (authApi) { return function authorizeMiddleware (req, res, next) { - req.profiler.done('req2params.setup'); - authApi.authorize(req, res, (err, authorized) => { req.profiler.done('authorize'); if (err) { From 59c312ea402b617f135c9fc9ac8b9b1fd70ece48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 28 Feb 2018 19:25:50 +0100 Subject: [PATCH 05/27] Require modules at the beginning of module --- lib/cartodb/middleware/context/authorize.js | 1 + lib/cartodb/middleware/context/credentials.js | 6 +----- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/cartodb/middleware/context/authorize.js b/lib/cartodb/middleware/context/authorize.js index d4af0d0e..a1323fa9 100644 --- a/lib/cartodb/middleware/context/authorize.js +++ b/lib/cartodb/middleware/context/authorize.js @@ -2,6 +2,7 @@ module.exports = function authorize (authApi) { return function authorizeMiddleware (req, res, next) { authApi.authorize(req, res, (err, authorized) => { req.profiler.done('authorize'); + if (err) { return next(err); } diff --git a/lib/cartodb/middleware/context/credentials.js b/lib/cartodb/middleware/context/credentials.js index 25851e36..b2024e99 100644 --- a/lib/cartodb/middleware/context/credentials.js +++ b/lib/cartodb/middleware/context/credentials.js @@ -1,4 +1,4 @@ -'use strict'; +const basicAuth = require('basic-auth'); module.exports = function credentials () { return function credentialsMiddleware(req, res, next) { @@ -11,10 +11,6 @@ module.exports = function credentials () { }; }; -//-------------------------------------------------------------------------------- - -const basicAuth = require('basic-auth'); - function getApikeyCredentialsFromRequest(req) { let apikeyCredentials = { token: null, From dad2e92dd3bef90667f5be6d82773626788582cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 28 Feb 2018 19:26:47 +0100 Subject: [PATCH 06/27] Follow middleware naming convention --- lib/cartodb/middleware/context/db-conn-setup.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/cartodb/middleware/context/db-conn-setup.js b/lib/cartodb/middleware/context/db-conn-setup.js index 068d77c2..17b24af0 100644 --- a/lib/cartodb/middleware/context/db-conn-setup.js +++ b/lib/cartodb/middleware/context/db-conn-setup.js @@ -1,7 +1,7 @@ const _ = require('underscore'); -module.exports = function dbConnSetupMiddleware(pgConnection) { - return function dbConnSetup(req, res, next) { +module.exports = function dbConnSetup (pgConnection) { + return function dbConnSetupMiddleware(req, res, next) { const user = res.locals.user; pgConnection.setDBConn(user, res.locals, (err) => { if (err) { @@ -18,11 +18,11 @@ module.exports = function dbConnSetupMiddleware(pgConnection) { dbhost: global.environment.postgres.host, dbport: global.environment.postgres.port }); - + res.set('X-Served-By-DB-Host', res.locals.dbhost); req.profiler.done('req2params'); - + next(null); }); }; From bfb743b8519124dd531a561851f3a6912290a892 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 28 Feb 2018 19:27:49 +0100 Subject: [PATCH 07/27] Improve profiling steps --- lib/cartodb/middleware/context/db-conn-setup.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/middleware/context/db-conn-setup.js b/lib/cartodb/middleware/context/db-conn-setup.js index 17b24af0..ec58c2a1 100644 --- a/lib/cartodb/middleware/context/db-conn-setup.js +++ b/lib/cartodb/middleware/context/db-conn-setup.js @@ -3,12 +3,15 @@ const _ = require('underscore'); module.exports = function dbConnSetup (pgConnection) { return function dbConnSetupMiddleware(req, res, next) { const user = res.locals.user; + pgConnection.setDBConn(user, res.locals, (err) => { + req.profiler.done('setDBConn'); + if (err) { if (err.message && -1 !== err.message.indexOf('name not found')) { err.http_status = 404; } - req.profiler.done('req2params'); + return next(err); } @@ -21,8 +24,6 @@ module.exports = function dbConnSetup (pgConnection) { res.set('X-Served-By-DB-Host', res.locals.dbhost); - req.profiler.done('req2params'); - next(null); }); }; From faa44e54aec89a3772f20c99591e6124eb34814f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 28 Feb 2018 19:29:10 +0100 Subject: [PATCH 08/27] Cosmetic changes --- lib/cartodb/middleware/context/db-conn-setup.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/middleware/context/db-conn-setup.js b/lib/cartodb/middleware/context/db-conn-setup.js index ec58c2a1..f658001a 100644 --- a/lib/cartodb/middleware/context/db-conn-setup.js +++ b/lib/cartodb/middleware/context/db-conn-setup.js @@ -1,8 +1,8 @@ const _ = require('underscore'); module.exports = function dbConnSetup (pgConnection) { - return function dbConnSetupMiddleware(req, res, next) { - const user = res.locals.user; + return function dbConnSetupMiddleware (req, res, next) { + const { user } = res.locals; pgConnection.setDBConn(user, res.locals, (err) => { req.profiler.done('setDBConn'); @@ -24,7 +24,7 @@ module.exports = function dbConnSetup (pgConnection) { res.set('X-Served-By-DB-Host', res.locals.dbhost); - next(null); + next(); }); }; }; From 9dc4e7c955b0252f1e5b0a89f2dc1cfbaf030088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 28 Feb 2018 19:29:53 +0100 Subject: [PATCH 09/27] Use the right step name for profiling --- lib/cartodb/middleware/context/db-conn-setup.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/middleware/context/db-conn-setup.js b/lib/cartodb/middleware/context/db-conn-setup.js index f658001a..ce3f6ac0 100644 --- a/lib/cartodb/middleware/context/db-conn-setup.js +++ b/lib/cartodb/middleware/context/db-conn-setup.js @@ -5,7 +5,7 @@ module.exports = function dbConnSetup (pgConnection) { const { user } = res.locals; pgConnection.setDBConn(user, res.locals, (err) => { - req.profiler.done('setDBConn'); + req.profiler.done('dbConnSetup'); if (err) { if (err.message && -1 !== err.message.indexOf('name not found')) { From f6f59023b42ee7bd98a2be8d96b00ce264f2bd5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 28 Feb 2018 19:46:46 +0100 Subject: [PATCH 10/27] Ungroup middlewares --- lib/cartodb/controllers/named_maps_admin.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 3c501ee1..296ecf52 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -4,11 +4,6 @@ const userMiddleware = require('../middleware/user'); const localsMiddleware = require('../middleware/context/locals'); const credentialsMiddleware = require('../middleware/context/credentials'); -const apikeyMiddleware = [ - localsMiddleware, - credentialsMiddleware(), -]; - /** * @param {AuthApi} authApi * @param {PgConnection} pgConnection @@ -29,7 +24,8 @@ NamedMapsAdminController.prototype.register = function (app) { `${base_url_templated}/`, cors(), userMiddleware, - apikeyMiddleware, + localsMiddleware, + credentialsMiddleware(), this.checkContentType('POST', 'POST TEMPLATE'), this.authorizedByAPIKey('create', 'POST TEMPLATE'), this.create() @@ -39,7 +35,8 @@ NamedMapsAdminController.prototype.register = function (app) { `${base_url_templated}/:template_id`, cors(), userMiddleware, - apikeyMiddleware, + localsMiddleware, + credentialsMiddleware(), this.checkContentType('PUT', 'PUT TEMPLATE'), this.authorizedByAPIKey('update', 'PUT TEMPLATE'), this.update() @@ -49,7 +46,8 @@ NamedMapsAdminController.prototype.register = function (app) { `${base_url_templated}/:template_id`, cors(), userMiddleware, - apikeyMiddleware, + localsMiddleware, + credentialsMiddleware(), this.authorizedByAPIKey('get', 'GET TEMPLATE'), this.retrieve() ); @@ -58,7 +56,8 @@ NamedMapsAdminController.prototype.register = function (app) { `${base_url_templated}/:template_id`, cors(), userMiddleware, - apikeyMiddleware, + localsMiddleware, + credentialsMiddleware(), this.authorizedByAPIKey('delete', 'DELETE TEMPLATE'), this.destroy() ); @@ -67,7 +66,8 @@ NamedMapsAdminController.prototype.register = function (app) { `${base_url_templated}/`, cors(), userMiddleware, - apikeyMiddleware, + localsMiddleware, + credentialsMiddleware(), this.authorizedByAPIKey('list', 'GET TEMPLATE LIST'), this.list() ); From b0c924ca03c8d81673668c762df36e261de15996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 1 Mar 2018 15:42:03 +0100 Subject: [PATCH 11/27] Follow middleware pattern, should return a function as the actual middleware --- lib/cartodb/controllers/analyses.js | 2 +- lib/cartodb/controllers/layergroup.js | 22 ++++++++++----------- lib/cartodb/controllers/map.js | 2 +- lib/cartodb/controllers/named_maps.js | 4 ++-- lib/cartodb/controllers/named_maps_admin.js | 10 +++++----- lib/cartodb/middleware/user.js | 11 +++++++---- 6 files changed, 27 insertions(+), 24 deletions(-) diff --git a/lib/cartodb/controllers/analyses.js b/lib/cartodb/controllers/analyses.js index fb75c633..db3550c8 100644 --- a/lib/cartodb/controllers/analyses.js +++ b/lib/cartodb/controllers/analyses.js @@ -12,7 +12,7 @@ AnalysesController.prototype.register = function (app) { app.get( `${app.base_url_mapconfig}/analyses/catalog`, cors(), - userMiddleware, + userMiddleware(), this.prepareContext, this.createPGClient(), this.getDataFromQuery({ queryTemplate: catalogQueryTpl, key: 'catalog' }), diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index d9251dd0..3365e270 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -49,7 +49,7 @@ LayergroupController.prototype.register = function(app) { app.get( app.base_url_mapconfig + '/:token/:z/:x/:y@:scale_factor?x.:format', cors(), - userMiddleware, + userMiddleware(), this.prepareContext, this.tile.bind(this), vectorError() @@ -58,7 +58,7 @@ LayergroupController.prototype.register = function(app) { app.get( app.base_url_mapconfig + '/:token/:z/:x/:y.:format', cors(), - userMiddleware, + userMiddleware(), this.prepareContext, this.tile.bind(this), vectorError() @@ -67,7 +67,7 @@ LayergroupController.prototype.register = function(app) { app.get( app.base_url_mapconfig + '/:token/:layer/:z/:x/:y.(:format)', cors(), - userMiddleware, + userMiddleware(), validateLayerRouteMiddleware, this.prepareContext, this.layer.bind(this), @@ -77,7 +77,7 @@ LayergroupController.prototype.register = function(app) { app.get( app.base_url_mapconfig + '/:token/:layer/attributes/:fid', cors(), - userMiddleware, + userMiddleware(), this.prepareContext, this.attributes.bind(this) ); @@ -85,7 +85,7 @@ LayergroupController.prototype.register = function(app) { app.get( app.base_url_mapconfig + '/static/center/:token/:z/:lat/:lng/:width/:height.:format', cors(), - userMiddleware, + userMiddleware(), allowQueryParams(['layer']), this.prepareContext, this.center.bind(this) @@ -94,7 +94,7 @@ LayergroupController.prototype.register = function(app) { app.get( app.base_url_mapconfig + '/static/bbox/:token/:west,:south,:east,:north/:width/:height.:format', cors(), - userMiddleware, + userMiddleware(), allowQueryParams(['layer']), this.prepareContext, this.bbox.bind(this) @@ -121,7 +121,7 @@ LayergroupController.prototype.register = function(app) { app.get( app.base_url_mapconfig + '/:token/dataview/:dataviewName', cors(), - userMiddleware, + userMiddleware(), allowQueryParams(allowedDataviewQueryParams), this.prepareContext, this.dataview.bind(this) @@ -130,7 +130,7 @@ LayergroupController.prototype.register = function(app) { app.get( app.base_url_mapconfig + '/:token/:layer/widget/:dataviewName', cors(), - userMiddleware, + userMiddleware(), allowQueryParams(allowedDataviewQueryParams), this.prepareContext, this.dataview.bind(this) @@ -139,7 +139,7 @@ LayergroupController.prototype.register = function(app) { app.get( app.base_url_mapconfig + '/:token/dataview/:dataviewName/search', cors(), - userMiddleware, + userMiddleware(), allowQueryParams(allowedDataviewQueryParams), this.prepareContext, this.dataviewSearch.bind(this) @@ -148,7 +148,7 @@ LayergroupController.prototype.register = function(app) { app.get( app.base_url_mapconfig + '/:token/:layer/widget/:dataviewName/search', cors(), - userMiddleware, + userMiddleware(), allowQueryParams(allowedDataviewQueryParams), this.prepareContext, this.dataviewSearch.bind(this) @@ -157,7 +157,7 @@ LayergroupController.prototype.register = function(app) { app.get( app.base_url_mapconfig + '/:token/analysis/node/:nodeId', cors(), - userMiddleware, + userMiddleware(), this.prepareContext, this.analysisNodeStatus.bind(this) ); diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 34660ce0..a9077339 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -69,7 +69,7 @@ MapController.prototype.composeCreateMapMiddleware = function (useTemplate = fal return [ cors(), - userMiddleware, + userMiddleware(), allowQueryParams(['aggregation']), this.prepareContext, this.initProfiler(isTemplateInstantiation), diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 92ba4ea6..27e5b7af 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -44,7 +44,7 @@ NamedMapsController.prototype.register = function(app) { app.get( app.base_url_templated + '/:template_id/:layer/:z/:x/:y.(:format)', cors(), - userMiddleware, + userMiddleware(), this.prepareContext, this.getNamedMapProvider('NAMED_MAP_TILE'), this.getAffectedTables(), @@ -61,7 +61,7 @@ NamedMapsController.prototype.register = function(app) { app.get( app.base_url_mapconfig + '/static/named/:template_id/:width/:height.:format', cors(), - userMiddleware, + userMiddleware(), allowQueryParams(['layer', 'zoom', 'lon', 'lat', 'bbox']), this.prepareContext, this.getNamedMapProvider('STATIC_VIZ_MAP'), diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 296ecf52..e8fd8f55 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -23,7 +23,7 @@ NamedMapsAdminController.prototype.register = function (app) { app.post( `${base_url_templated}/`, cors(), - userMiddleware, + userMiddleware(), localsMiddleware, credentialsMiddleware(), this.checkContentType('POST', 'POST TEMPLATE'), @@ -34,7 +34,7 @@ NamedMapsAdminController.prototype.register = function (app) { app.put( `${base_url_templated}/:template_id`, cors(), - userMiddleware, + userMiddleware(), localsMiddleware, credentialsMiddleware(), this.checkContentType('PUT', 'PUT TEMPLATE'), @@ -45,7 +45,7 @@ NamedMapsAdminController.prototype.register = function (app) { app.get( `${base_url_templated}/:template_id`, cors(), - userMiddleware, + userMiddleware(), localsMiddleware, credentialsMiddleware(), this.authorizedByAPIKey('get', 'GET TEMPLATE'), @@ -55,7 +55,7 @@ NamedMapsAdminController.prototype.register = function (app) { app.delete( `${base_url_templated}/:template_id`, cors(), - userMiddleware, + userMiddleware(), localsMiddleware, credentialsMiddleware(), this.authorizedByAPIKey('delete', 'DELETE TEMPLATE'), @@ -65,7 +65,7 @@ NamedMapsAdminController.prototype.register = function (app) { app.get( `${base_url_templated}/`, cors(), - userMiddleware, + userMiddleware(), localsMiddleware, credentialsMiddleware(), this.authorizedByAPIKey('list', 'GET TEMPLATE LIST'), diff --git a/lib/cartodb/middleware/user.js b/lib/cartodb/middleware/user.js index adf06203..2e869b38 100644 --- a/lib/cartodb/middleware/user.js +++ b/lib/cartodb/middleware/user.js @@ -1,8 +1,11 @@ var CdbRequest = require('../models/cdb_request'); -var cdbRequest = new CdbRequest(); -module.exports = function userMiddleware(req, res, next) { - res.locals.user = cdbRequest.userByReq(req); +module.exports = function user () { + var cdbRequest = new CdbRequest(); - next(); + return function userMiddleware(req, res, next) { + res.locals.user = cdbRequest.userByReq(req); + + next(); + }; }; From 3caa1d9c4a30178b28b81169acf0c9010e54cb31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 1 Mar 2018 15:42:46 +0100 Subject: [PATCH 12/27] ES6 cosmetics --- lib/cartodb/middleware/user.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/middleware/user.js b/lib/cartodb/middleware/user.js index 2e869b38..9c7968bc 100644 --- a/lib/cartodb/middleware/user.js +++ b/lib/cartodb/middleware/user.js @@ -1,7 +1,7 @@ -var CdbRequest = require('../models/cdb_request'); +const CdbRequest = require('../models/cdb_request'); module.exports = function user () { - var cdbRequest = new CdbRequest(); + const cdbRequest = new CdbRequest(); return function userMiddleware(req, res, next) { res.locals.user = cdbRequest.userByReq(req); From 2c762813ba65e553ee861cc034e5fe2499b5a9b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 1 Mar 2018 15:52:48 +0100 Subject: [PATCH 13/27] Follow middleware pattern, return a function as the actual middleware --- lib/cartodb/controllers/named_maps_admin.js | 10 +++++----- lib/cartodb/middleware/context/index.js | 2 +- lib/cartodb/middleware/context/locals.js | 9 +++++---- test/unit/cartodb/prepare-context.test.js | 3 ++- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index e8fd8f55..0728b38c 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -24,7 +24,7 @@ NamedMapsAdminController.prototype.register = function (app) { `${base_url_templated}/`, cors(), userMiddleware(), - localsMiddleware, + localsMiddleware(), credentialsMiddleware(), this.checkContentType('POST', 'POST TEMPLATE'), this.authorizedByAPIKey('create', 'POST TEMPLATE'), @@ -35,7 +35,7 @@ NamedMapsAdminController.prototype.register = function (app) { `${base_url_templated}/:template_id`, cors(), userMiddleware(), - localsMiddleware, + localsMiddleware(), credentialsMiddleware(), this.checkContentType('PUT', 'PUT TEMPLATE'), this.authorizedByAPIKey('update', 'PUT TEMPLATE'), @@ -46,7 +46,7 @@ NamedMapsAdminController.prototype.register = function (app) { `${base_url_templated}/:template_id`, cors(), userMiddleware(), - localsMiddleware, + localsMiddleware(), credentialsMiddleware(), this.authorizedByAPIKey('get', 'GET TEMPLATE'), this.retrieve() @@ -56,7 +56,7 @@ NamedMapsAdminController.prototype.register = function (app) { `${base_url_templated}/:template_id`, cors(), userMiddleware(), - localsMiddleware, + localsMiddleware(), credentialsMiddleware(), this.authorizedByAPIKey('delete', 'DELETE TEMPLATE'), this.destroy() @@ -66,7 +66,7 @@ NamedMapsAdminController.prototype.register = function (app) { `${base_url_templated}/`, cors(), userMiddleware(), - localsMiddleware, + localsMiddleware(), credentialsMiddleware(), this.authorizedByAPIKey('list', 'GET TEMPLATE LIST'), this.list() diff --git a/lib/cartodb/middleware/context/index.js b/lib/cartodb/middleware/context/index.js index 9394cfd0..750986c5 100644 --- a/lib/cartodb/middleware/context/index.js +++ b/lib/cartodb/middleware/context/index.js @@ -7,7 +7,7 @@ const dbConnSetup = require('./db-conn-setup'); module.exports = function prepareContextMiddleware(authApi, pgConnection) { return [ - locals, + locals(), cleanUpQueryParams(), layergroupToken, credentials(), diff --git a/lib/cartodb/middleware/context/locals.js b/lib/cartodb/middleware/context/locals.js index 0fdcce50..f6f70923 100644 --- a/lib/cartodb/middleware/context/locals.js +++ b/lib/cartodb/middleware/context/locals.js @@ -1,6 +1,7 @@ -module.exports = function localsMiddleware(req, res, next) { - // save req.params in res.locals - res.locals = Object.assign(req.params || {}, res.locals); +module.exports = function locals () { + return function localsMiddleware (req, res, next) { + res.locals = Object.assign(req.params || {}, res.locals); - next(); + next(); + }; }; diff --git a/test/unit/cartodb/prepare-context.test.js b/test/unit/cartodb/prepare-context.test.js index 802063eb..2f6d80a3 100644 --- a/test/unit/cartodb/prepare-context.test.js +++ b/test/unit/cartodb/prepare-context.test.js @@ -65,10 +65,11 @@ describe('prepare-context', function() { } it('res.locals are created', function(done) { + const locals = localsMiddleware(); let req = {}; let res = {}; - localsMiddleware(prepareRequest(req), prepareResponse(res), function(err) { + locals(prepareRequest(req), prepareResponse(res), function(err) { if ( err ) { done(err); return; } assert.ok(res.hasOwnProperty('locals'), 'response has locals'); done(); From bd93e7dc7e314bfcde2a85e3eebdb42499dfee47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 1 Mar 2018 18:09:49 +0100 Subject: [PATCH 14/27] Follow middleware pattern --- lib/cartodb/middleware/context/index.js | 2 +- .../middleware/context/layergroup-token.js | 56 ++++++++++--------- 2 files changed, 30 insertions(+), 28 deletions(-) diff --git a/lib/cartodb/middleware/context/index.js b/lib/cartodb/middleware/context/index.js index 750986c5..70465895 100644 --- a/lib/cartodb/middleware/context/index.js +++ b/lib/cartodb/middleware/context/index.js @@ -9,7 +9,7 @@ module.exports = function prepareContextMiddleware(authApi, pgConnection) { return [ locals(), cleanUpQueryParams(), - layergroupToken, + layergroupToken(), credentials(), authorize(authApi), dbConnSetup(pgConnection) diff --git a/lib/cartodb/middleware/context/layergroup-token.js b/lib/cartodb/middleware/context/layergroup-token.js index 026d0806..b7ad82f9 100644 --- a/lib/cartodb/middleware/context/layergroup-token.js +++ b/lib/cartodb/middleware/context/layergroup-token.js @@ -1,32 +1,34 @@ var LayergroupToken = require('../../models/layergroup-token'); -module.exports = function layergroupTokenMiddleware(req, res, next) { - if (!res.locals.token) { - return next(); - } - - var user = res.locals.user; - - var layergroupToken = LayergroupToken.parse(res.locals.token); - res.locals.token = layergroupToken.token; - res.locals.cache_buster = layergroupToken.cacheBuster; - - if (layergroupToken.signer) { - res.locals.signer = layergroupToken.signer; - if (!res.locals.signer) { - res.locals.signer = user; - } else if (res.locals.signer !== user) { - var err = new Error(`Cannot use map signature of user "${res.locals.signer}" on db of user "${user}"`); - err.type = 'auth'; - err.http_status = 403; - if (req.query && req.query.callback) { - err.http_status = 200; - } - - req.profiler.done('req2params'); - return next(err); +module.exports = function layergroupToken () { + return function layergroupTokenMiddleware(req, res, next) { + if (!res.locals.token) { + return next(); } - } - return next(); + var user = res.locals.user; + + var layergroupToken = LayergroupToken.parse(res.locals.token); + res.locals.token = layergroupToken.token; + res.locals.cache_buster = layergroupToken.cacheBuster; + + if (layergroupToken.signer) { + res.locals.signer = layergroupToken.signer; + if (!res.locals.signer) { + res.locals.signer = user; + } else if (res.locals.signer !== user) { + var err = new Error(`Cannot use map signature of user "${res.locals.signer}" on db of user "${user}"`); + err.type = 'auth'; + err.http_status = 403; + if (req.query && req.query.callback) { + err.http_status = 200; + } + + req.profiler.done('req2params'); + return next(err); + } + } + + return next(); + }; }; From 5eaee0b71e95f4478a2ede339b6439c5a009ffae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 1 Mar 2018 18:12:07 +0100 Subject: [PATCH 15/27] Follow middleware naming convention --- lib/cartodb/middleware/stats.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/middleware/stats.js b/lib/cartodb/middleware/stats.js index 489ee645..83ff3054 100644 --- a/lib/cartodb/middleware/stats.js +++ b/lib/cartodb/middleware/stats.js @@ -2,10 +2,10 @@ const Profiler = require('../stats/profiler_proxy'); const debug = require('debug')('windshaft:cartodb:stats'); const onHeaders = require('on-headers'); -module.exports = function statsMiddleware(options) { +module.exports = function stats (options) { const { enabled = true, statsClient } = options; - return function stats(req, res, next) { + return function statsMiddleware (req, res, next) { req.profiler = new Profiler({ statsd_client: statsClient, profile: enabled From da18506e4162254b82dc1cb5b028632040a72975 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 1 Mar 2018 18:45:04 +0100 Subject: [PATCH 16/27] Follow middleware factory pattern --- lib/cartodb/middleware/lzma.js | 50 ++++++++++++------------ lib/cartodb/server.js | 2 +- test/unit/cartodb/lzmaMiddleware.test.js | 4 +- 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/lib/cartodb/middleware/lzma.js b/lib/cartodb/middleware/lzma.js index 6655cdeb..e1956352 100644 --- a/lib/cartodb/middleware/lzma.js +++ b/lib/cartodb/middleware/lzma.js @@ -1,30 +1,30 @@ -'use strict'; - const LZMA = require('lzma').LZMA; -const lzmaWorker = new LZMA(); +module.exports = function lzma () { + const lzmaWorker = new LZMA(); -module.exports = function lzmaMiddleware(req, res, next) { - if (!req.query.hasOwnProperty('lzma')) { - return next(); - } - - // Decode (from base64) - var lzma = new Buffer(req.query.lzma, 'base64') - .toString('binary') - .split('') - .map(function(c) { - return c.charCodeAt(0) - 128; - }); - - // Decompress - lzmaWorker.decompress(lzma, function(result) { - try { - delete req.query.lzma; - Object.assign(req.query, JSON.parse(result)); - next(); - } catch (err) { - next(new Error('Error parsing lzma as JSON: ' + err)); + return function lzmaMiddleware (req, res, next) { + if (!req.query.hasOwnProperty('lzma')) { + return next(); } - }); + + // Decode (from base64) + var lzma = new Buffer(req.query.lzma, 'base64') + .toString('binary') + .split('') + .map(function(c) { + return c.charCodeAt(0) - 128; + }); + + // Decompress + lzmaWorker.decompress(lzma, function(result) { + try { + delete req.query.lzma; + Object.assign(req.query, JSON.parse(result)); + next(); + } catch (err) { + next(new Error('Error parsing lzma as JSON: ' + err)); + } + }); + }; }; diff --git a/lib/cartodb/server.js b/lib/cartodb/server.js index 46ff7122..532d155b 100644 --- a/lib/cartodb/server.js +++ b/lib/cartodb/server.js @@ -377,7 +377,7 @@ function bootstrap(opts) { statsClient: global.statsClient })); - app.use(lzmaMiddleware); + app.use(lzmaMiddleware()); // temporary measure until we upgrade to newer version expressjs so we can check err.status app.use(function(err, req, res, next) { diff --git a/test/unit/cartodb/lzmaMiddleware.test.js b/test/unit/cartodb/lzmaMiddleware.test.js index 9a41030a..d12c7d39 100644 --- a/test/unit/cartodb/lzmaMiddleware.test.js +++ b/test/unit/cartodb/lzmaMiddleware.test.js @@ -12,6 +12,7 @@ describe('lzma-middleware', function() { } }; testHelper.lzma_compress_to_base64(JSON.stringify(qo), 1, function(err, data) { + const lzma = lzmaMiddleware(); var req = { headers: { host:'localhost' @@ -21,7 +22,8 @@ describe('lzma-middleware', function() { lzma: data } }; - lzmaMiddleware(req, {}, function(err) { + + lzma(req, {}, function(err) { if ( err ) { return done(err); } From 314508bcd8d88ad0401e817722092a39379cbc8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 1 Mar 2018 18:46:04 +0100 Subject: [PATCH 17/27] Middleware naming convention --- lib/cartodb/middleware/cors.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/middleware/cors.js b/lib/cartodb/middleware/cors.js index 227bb477..ee924a90 100644 --- a/lib/cartodb/middleware/cors.js +++ b/lib/cartodb/middleware/cors.js @@ -1,11 +1,14 @@ module.exports = function cors(extraHeaders) { - return function(req, res, next) { + return function corsMiddleware (req, res, next) { var baseHeaders = "X-Requested-With, X-Prototype-Version, X-CSRF-Token"; + if(extraHeaders) { baseHeaders += ", " + extraHeaders; } + res.set("Access-Control-Allow-Origin", "*"); res.set("Access-Control-Allow-Headers", baseHeaders); + next(); }; }; From e6ba467d9807be9cf12435b08de24ea5d4904243 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 1 Mar 2018 18:47:07 +0100 Subject: [PATCH 18/27] ES6 goodies --- lib/cartodb/middleware/cors.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/middleware/cors.js b/lib/cartodb/middleware/cors.js index ee924a90..65b7cf4f 100644 --- a/lib/cartodb/middleware/cors.js +++ b/lib/cartodb/middleware/cors.js @@ -1,6 +1,6 @@ -module.exports = function cors(extraHeaders) { +module.exports = function cors (extraHeaders) { return function corsMiddleware (req, res, next) { - var baseHeaders = "X-Requested-With, X-Prototype-Version, X-CSRF-Token"; + let baseHeaders = "X-Requested-With, X-Prototype-Version, X-CSRF-Token"; if(extraHeaders) { baseHeaders += ", " + extraHeaders; From ef3ffddec7c642c63ef68a8005078ec4d7462efe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 1 Mar 2018 18:49:44 +0100 Subject: [PATCH 19/27] Cosmetic changes --- lib/cartodb/middleware/allow-query-params.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/middleware/allow-query-params.js b/lib/cartodb/middleware/allow-query-params.js index 7ec31d74..cf90e69b 100644 --- a/lib/cartodb/middleware/allow-query-params.js +++ b/lib/cartodb/middleware/allow-query-params.js @@ -1,8 +1,9 @@ -module.exports = function allowQueryParams(params) { +module.exports = function allowQueryParams (params) { if (!Array.isArray(params)) { throw new Error('allowQueryParams must receive an Array of params'); } - return function allowQueryParamsMiddleware(req, res, next) { + + return function allowQueryParamsMiddleware (req, res, next) { res.locals.allowedQueryParams = params; next(); }; From 5bac36b30f8000dccb87f6186283644594272015 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 1 Mar 2018 18:53:05 +0100 Subject: [PATCH 20/27] Remove bad profiler usage --- lib/cartodb/middleware/context/layergroup-token.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/cartodb/middleware/context/layergroup-token.js b/lib/cartodb/middleware/context/layergroup-token.js index b7ad82f9..00419c8f 100644 --- a/lib/cartodb/middleware/context/layergroup-token.js +++ b/lib/cartodb/middleware/context/layergroup-token.js @@ -24,7 +24,6 @@ module.exports = function layergroupToken () { err.http_status = 200; } - req.profiler.done('req2params'); return next(err); } } From ccc28f36175dc11ab735028745cac69094005675 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 1 Mar 2018 19:09:11 +0100 Subject: [PATCH 21/27] Add profiler step to lzma --- lib/cartodb/middleware/lzma.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/cartodb/middleware/lzma.js b/lib/cartodb/middleware/lzma.js index e1956352..b0a94412 100644 --- a/lib/cartodb/middleware/lzma.js +++ b/lib/cartodb/middleware/lzma.js @@ -21,6 +21,9 @@ module.exports = function lzma () { try { delete req.query.lzma; Object.assign(req.query, JSON.parse(result)); + + req.profiler.done('lzma'); + next(); } catch (err) { next(new Error('Error parsing lzma as JSON: ' + err)); From 416970c819ba0639753356c9c4ccb98aacfd2135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 1 Mar 2018 19:10:35 +0100 Subject: [PATCH 22/27] Remove empty line --- lib/cartodb/middleware/vector-error.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/cartodb/middleware/vector-error.js b/lib/cartodb/middleware/vector-error.js index f42f1c87..75e93b0a 100644 --- a/lib/cartodb/middleware/vector-error.js +++ b/lib/cartodb/middleware/vector-error.js @@ -1,5 +1,4 @@ const fs = require('fs'); - const timeoutErrorVectorTile = fs.readFileSync(__dirname + '/../../../assets/render-timeout-fallback.mvt'); module.exports = function vectorError() { From 0ec9491d217d858d23980be056374e336b3bc40c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 2 Mar 2018 11:16:46 +0100 Subject: [PATCH 23/27] Fix test: Add stub for profiling --- test/unit/cartodb/lzmaMiddleware.test.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/unit/cartodb/lzmaMiddleware.test.js b/test/unit/cartodb/lzmaMiddleware.test.js index d12c7d39..3ad81962 100644 --- a/test/unit/cartodb/lzmaMiddleware.test.js +++ b/test/unit/cartodb/lzmaMiddleware.test.js @@ -20,6 +20,9 @@ describe('lzma-middleware', function() { query: { api_key: 'test', lzma: data + }, + profiler: { + done: function () {} } }; From 7ed717607a4506abc316485ce0da4c7015f1da6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 2 Mar 2018 13:08:57 +0100 Subject: [PATCH 24/27] Missing space before paramenter list --- lib/cartodb/middleware/context/layergroup-token.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/middleware/context/layergroup-token.js b/lib/cartodb/middleware/context/layergroup-token.js index 00419c8f..53587c8d 100644 --- a/lib/cartodb/middleware/context/layergroup-token.js +++ b/lib/cartodb/middleware/context/layergroup-token.js @@ -1,7 +1,7 @@ var LayergroupToken = require('../../models/layergroup-token'); module.exports = function layergroupToken () { - return function layergroupTokenMiddleware(req, res, next) { + return function layergroupTokenMiddleware (req, res, next) { if (!res.locals.token) { return next(); } From 82f1e6753b20499b9cbac17f92f7ee8411a69e0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 2 Mar 2018 13:14:02 +0100 Subject: [PATCH 25/27] Remove unreachable code --- lib/cartodb/middleware/context/layergroup-token.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/cartodb/middleware/context/layergroup-token.js b/lib/cartodb/middleware/context/layergroup-token.js index 53587c8d..af213bf5 100644 --- a/lib/cartodb/middleware/context/layergroup-token.js +++ b/lib/cartodb/middleware/context/layergroup-token.js @@ -14,9 +14,7 @@ module.exports = function layergroupToken () { if (layergroupToken.signer) { res.locals.signer = layergroupToken.signer; - if (!res.locals.signer) { - res.locals.signer = user; - } else if (res.locals.signer !== user) { + if (res.locals.signer !== user) { var err = new Error(`Cannot use map signature of user "${res.locals.signer}" on db of user "${user}"`); err.type = 'auth'; err.http_status = 403; From f2f6b9d49c2d3c5f4a7ff4802de775c440de5aad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 2 Mar 2018 13:29:30 +0100 Subject: [PATCH 26/27] ES6 goodies --- .../middleware/context/layergroup-token.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/cartodb/middleware/context/layergroup-token.js b/lib/cartodb/middleware/context/layergroup-token.js index af213bf5..f611b068 100644 --- a/lib/cartodb/middleware/context/layergroup-token.js +++ b/lib/cartodb/middleware/context/layergroup-token.js @@ -1,26 +1,29 @@ var LayergroupToken = require('../../models/layergroup-token'); +const authErrorMessageTemplate = function (signer, user) { + return `Cannot use map signature of user "${signer}" on db of user "${user}"`; +}; + module.exports = function layergroupToken () { return function layergroupTokenMiddleware (req, res, next) { if (!res.locals.token) { return next(); } - var user = res.locals.user; + const user = res.locals.user; + + const layergroupToken = LayergroupToken.parse(res.locals.token); - var layergroupToken = LayergroupToken.parse(res.locals.token); res.locals.token = layergroupToken.token; res.locals.cache_buster = layergroupToken.cacheBuster; if (layergroupToken.signer) { res.locals.signer = layergroupToken.signer; + if (res.locals.signer !== user) { - var err = new Error(`Cannot use map signature of user "${res.locals.signer}" on db of user "${user}"`); + const err = new Error(authErrorMessageTemplate(res.locals.signer, user)); err.type = 'auth'; - err.http_status = 403; - if (req.query && req.query.callback) { - err.http_status = 200; - } + err.http_status = (req.query && req.query.callback) ? 200: 403; return next(err); } From 8656fcd8d17e4b0cff1ff6dafe69a9412b9d7a76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 2 Mar 2018 14:04:29 +0100 Subject: [PATCH 27/27] Use 'const' --- lib/cartodb/middleware/context/layergroup-token.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/cartodb/middleware/context/layergroup-token.js b/lib/cartodb/middleware/context/layergroup-token.js index f611b068..c4aac23f 100644 --- a/lib/cartodb/middleware/context/layergroup-token.js +++ b/lib/cartodb/middleware/context/layergroup-token.js @@ -1,5 +1,4 @@ -var LayergroupToken = require('../../models/layergroup-token'); - +const LayergroupToken = require('../../models/layergroup-token'); const authErrorMessageTemplate = function (signer, user) { return `Cannot use map signature of user "${signer}" on db of user "${user}"`; };