From fda7661dad88e946ee5bd26fadf8445aed7c9423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 12:23:52 +0100 Subject: [PATCH 01/15] Create authorizedByAPIKey middleware --- lib/cartodb/controllers/named_maps_admin.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 0f54ddf3..8fcaaac0 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -198,6 +198,26 @@ NamedMapsAdminController.prototype.list = function(req, res, next) { ); }; +NamedMapsAdminController.prototype.authorizedByAPIKey = function (action) { + return function authorizedByAPIKeyMiddleware (req, res, next) { + const { user } = res.locals; + + this.authApi.authorizedByAPIKey(user, req, (err, authenticated) => { + if (err) { + return next(err); + } + + if (!authenticated) { + const error = new Error(`Only authenticated user can ${action} templated maps`); + error.http_status = 403; + return next(error); + } + + next(); + }); + }.bind(this); +}; + function finishFn(controller, req, res, description, status, next) { return function finish(err, body){ if (err) { From e4ed6ee1cccf59dec7e0c5b39e629ec686ad9b3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 12:34:50 +0100 Subject: [PATCH 02/15] Use authorizedByAPIKey middleware --- lib/cartodb/controllers/named_maps_admin.js | 43 +++++---------------- 1 file changed, 10 insertions(+), 33 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 8fcaaac0..87259f7e 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -24,6 +24,7 @@ NamedMapsAdminController.prototype.register = function (app) { app.base_url_templated + '/', cors(), userMiddleware, + this.authorizedByAPIKey('create'), this.create.bind(this) ); @@ -31,6 +32,7 @@ NamedMapsAdminController.prototype.register = function (app) { app.base_url_templated + '/:template_id', cors(), userMiddleware, + this.authorizedByAPIKey('update'), this.update.bind(this) ); @@ -38,6 +40,7 @@ NamedMapsAdminController.prototype.register = function (app) { app.base_url_templated + '/:template_id', cors(), userMiddleware, + this.authorizedByAPIKey('get'), this.retrieve.bind(this) ); @@ -45,6 +48,7 @@ NamedMapsAdminController.prototype.register = function (app) { app.base_url_templated + '/:template_id', cors(), userMiddleware, + this.authorizedByAPIKey('delete'), this.destroy.bind(this) ); @@ -52,6 +56,7 @@ NamedMapsAdminController.prototype.register = function (app) { app.base_url_templated + '/', cors(), userMiddleware, + this.authorizedByAPIKey('list'), this.list.bind(this) ); @@ -67,12 +72,7 @@ NamedMapsAdminController.prototype.create = function(req, res, next) { var cdbuser = res.locals.user; step( - function checkPerms(){ - self.authApi.authorizedByAPIKey(cdbuser, req, this); - }, - function addTemplate(err, authenticated) { - assert.ifError(err); - ifUnauthenticated(authenticated, 'Only authenticated users can get template maps'); + function addTemplate() { ifInvalidContentType(req, 'template POST data must be of type application/json'); var cfg = req.body; self.templateMaps.addTemplate(cdbuser, cfg, this); @@ -93,12 +93,7 @@ NamedMapsAdminController.prototype.update = function(req, res, next) { var tpl_id; step( - function checkPerms(){ - self.authApi.authorizedByAPIKey(cdbuser, req, this); - }, - function updateTemplate(err, authenticated) { - assert.ifError(err); - ifUnauthenticated(authenticated, 'Only authenticated user can update templated maps'); + function updateTemplate() { ifInvalidContentType(req, 'template PUT data must be of type application/json'); template = req.body; @@ -122,13 +117,7 @@ NamedMapsAdminController.prototype.retrieve = function(req, res, next) { var cdbuser = res.locals.user; var tpl_id; step( - function checkPerms(){ - self.authApi.authorizedByAPIKey(cdbuser, req, this); - }, - function getTemplate(err, authenticated) { - assert.ifError(err); - ifUnauthenticated(authenticated, 'Only authenticated users can get template maps'); - + function getTemplate() { tpl_id = templateName(req.params.template_id); self.templateMaps.getTemplate(cdbuser, tpl_id, this); }, @@ -156,13 +145,7 @@ NamedMapsAdminController.prototype.destroy = function(req, res, next) { var cdbuser = res.locals.user; var tpl_id; step( - function checkPerms(){ - self.authApi.authorizedByAPIKey(cdbuser, req, this); - }, - function deleteTemplate(err, authenticated) { - assert.ifError(err); - ifUnauthenticated(authenticated, 'Only authenticated users can delete template maps'); - + function deleteTemplate() { tpl_id = templateName(req.params.template_id); self.templateMaps.delTemplate(cdbuser, tpl_id, this); }, @@ -181,13 +164,7 @@ NamedMapsAdminController.prototype.list = function(req, res, next) { var cdbuser = res.locals.user; step( - function checkPerms(){ - self.authApi.authorizedByAPIKey(cdbuser, req, this); - }, - function listTemplates(err, authenticated) { - assert.ifError(err); - ifUnauthenticated(authenticated, 'Only authenticated user can list templated maps'); - + function listTemplates() { self.templateMaps.listTemplates(cdbuser, this); }, function prepareResponse(err, tpl_ids){ From ba008ab5181f1c96fba269271a3b3f0abf64968d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 12:36:56 +0100 Subject: [PATCH 03/15] Remove unused function --- lib/cartodb/controllers/named_maps_admin.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 87259f7e..854d6df0 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -212,14 +212,6 @@ function finishFn(controller, req, res, description, status, next) { }; } -function ifUnauthenticated(authenticated, description) { - if (!authenticated) { - var err = new Error(description); - err.http_status = 403; - throw err; - } -} - function ifInvalidContentType(req, description) { if (!req.is('application/json')) { throw new Error(description); From f136993c5057d408334c5583ce1b8f1ad3430377 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 12:44:56 +0100 Subject: [PATCH 04/15] Use checkContentType middleware --- lib/cartodb/controllers/named_maps_admin.js | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 854d6df0..77280015 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -24,6 +24,7 @@ NamedMapsAdminController.prototype.register = function (app) { app.base_url_templated + '/', cors(), userMiddleware, + this.checkContentType('POST'), this.authorizedByAPIKey('create'), this.create.bind(this) ); @@ -32,6 +33,7 @@ NamedMapsAdminController.prototype.register = function (app) { app.base_url_templated + '/:template_id', cors(), userMiddleware, + this.checkContentType('PUT'), this.authorizedByAPIKey('update'), this.update.bind(this) ); @@ -73,7 +75,6 @@ NamedMapsAdminController.prototype.create = function(req, res, next) { step( function addTemplate() { - ifInvalidContentType(req, 'template POST data must be of type application/json'); var cfg = req.body; self.templateMaps.addTemplate(cdbuser, cfg, this); }, @@ -94,8 +95,6 @@ NamedMapsAdminController.prototype.update = function(req, res, next) { step( function updateTemplate() { - ifInvalidContentType(req, 'template PUT data must be of type application/json'); - template = req.body; tpl_id = templateName(req.params.template_id); self.templateMaps.updTemplate(cdbuser, tpl_id, template, this); @@ -195,6 +194,15 @@ NamedMapsAdminController.prototype.authorizedByAPIKey = function (action) { }.bind(this); }; +NamedMapsAdminController.prototype.checkContentType = function (action) { + return function checkContentTypeMiddleware (req, res, next) { + if (!req.is('application/json')) { + return next(new Error(`template ${action} data must be of type application/json`)); + } + next(); + }; +}; + function finishFn(controller, req, res, description, status, next) { return function finish(err, body){ if (err) { @@ -211,9 +219,3 @@ function finishFn(controller, req, res, description, status, next) { } }; } - -function ifInvalidContentType(req, description) { - if (!req.is('application/json')) { - throw new Error(description); - } -} From bf814c444242dfdddf2580f9da960224d8efd408 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 13:05:01 +0100 Subject: [PATCH 05/15] keep error label --- lib/cartodb/controllers/named_maps_admin.js | 23 ++++++++++++--------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 77280015..b46de702 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -24,8 +24,8 @@ NamedMapsAdminController.prototype.register = function (app) { app.base_url_templated + '/', cors(), userMiddleware, - this.checkContentType('POST'), - this.authorizedByAPIKey('create'), + this.checkContentType('POST', 'POST TEMPLATE'), + this.authorizedByAPIKey('create', 'POST TEMPLATE'), this.create.bind(this) ); @@ -33,8 +33,8 @@ NamedMapsAdminController.prototype.register = function (app) { app.base_url_templated + '/:template_id', cors(), userMiddleware, - this.checkContentType('PUT'), - this.authorizedByAPIKey('update'), + this.checkContentType('PUT', 'PUT TEMPLATE'), + this.authorizedByAPIKey('update', 'PUT TEMPLATE'), this.update.bind(this) ); @@ -42,7 +42,7 @@ NamedMapsAdminController.prototype.register = function (app) { app.base_url_templated + '/:template_id', cors(), userMiddleware, - this.authorizedByAPIKey('get'), + this.authorizedByAPIKey('get', 'GET TEMPLATE'), this.retrieve.bind(this) ); @@ -50,7 +50,7 @@ NamedMapsAdminController.prototype.register = function (app) { app.base_url_templated + '/:template_id', cors(), userMiddleware, - this.authorizedByAPIKey('delete'), + this.authorizedByAPIKey('delete', 'DELETE TEMPLATE'), this.destroy.bind(this) ); @@ -58,7 +58,7 @@ NamedMapsAdminController.prototype.register = function (app) { app.base_url_templated + '/', cors(), userMiddleware, - this.authorizedByAPIKey('list'), + this.authorizedByAPIKey('list', 'GET TEMPLATE LIST'), this.list.bind(this) ); @@ -174,7 +174,7 @@ NamedMapsAdminController.prototype.list = function(req, res, next) { ); }; -NamedMapsAdminController.prototype.authorizedByAPIKey = function (action) { +NamedMapsAdminController.prototype.authorizedByAPIKey = function (action, label) { return function authorizedByAPIKeyMiddleware (req, res, next) { const { user } = res.locals; @@ -186,6 +186,7 @@ NamedMapsAdminController.prototype.authorizedByAPIKey = function (action) { if (!authenticated) { const error = new Error(`Only authenticated user can ${action} templated maps`); error.http_status = 403; + error.label = label; return next(error); } @@ -194,10 +195,12 @@ NamedMapsAdminController.prototype.authorizedByAPIKey = function (action) { }.bind(this); }; -NamedMapsAdminController.prototype.checkContentType = function (action) { +NamedMapsAdminController.prototype.checkContentType = function (action, label) { return function checkContentTypeMiddleware (req, res, next) { if (!req.is('application/json')) { - return next(new Error(`template ${action} data must be of type application/json`)); + const error = new Error(`template ${action} data must be of type application/json`); + error.label = label; + return next(error); } next(); }; From 519d49bd10e094568b4ebc8a0ebdfc0f24d580c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 15:04:44 +0100 Subject: [PATCH 06/15] Remove finish function and respond in the main middleware --- lib/cartodb/controllers/named_maps_admin.js | 67 ++++++++++----------- 1 file changed, 32 insertions(+), 35 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index b46de702..ca7b4901 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -80,9 +80,12 @@ NamedMapsAdminController.prototype.create = function(req, res, next) { }, function prepareResponse(err, tpl_id){ assert.ifError(err); - return { template_id: tpl_id }; - }, - finishFn(self, req, res, 'POST TEMPLATE', null, next) + + res.status(200); + + const method = req.query.callback ? 'jsonp' : 'json'; + res[method]({ template_id: tpl_id }); + } ); }; @@ -102,9 +105,11 @@ NamedMapsAdminController.prototype.update = function(req, res, next) { function prepareResponse(err){ assert.ifError(err); - return { template_id: tpl_id }; - }, - finishFn(self, req, res, 'PUT TEMPLATE', null, next) + res.status(200); + + const method = req.query.callback ? 'jsonp' : 'json'; + res[method]({ template_id: tpl_id }); + } ); }; @@ -123,16 +128,19 @@ NamedMapsAdminController.prototype.retrieve = function(req, res, next) { function prepareResponse(err, tpl_val) { assert.ifError(err); if ( ! tpl_val ) { - err = new Error("Cannot find template '" + tpl_id + "' of user '" + cdbuser + "'"); - err.http_status = 404; - throw err; + const error = new Error(`Cannot find template '${tpl_id}' of user '${cdbuser}'`); + error.http_status = 404; + return next(error); } // auth_id was added by ourselves, // so we remove it before returning to the user delete tpl_val.auth_id; - return { template: tpl_val }; - }, - finishFn(self, req, res, 'GET TEMPLATE', null, next) + + res.status(200); + + const method = req.query.callback ? 'jsonp' : 'json'; + res[method]({ template: tpl_val }); + } ); }; @@ -150,9 +158,12 @@ NamedMapsAdminController.prototype.destroy = function(req, res, next) { }, function prepareResponse(err/*, tpl_val*/){ assert.ifError(err); - return ''; - }, - finishFn(self, req, res, 'DELETE TEMPLATE', 204, next) + + res.status(204); + + const method = req.query.callback ? 'jsonp' : 'json'; + res[method](''); + } ); }; @@ -168,9 +179,12 @@ NamedMapsAdminController.prototype.list = function(req, res, next) { }, function prepareResponse(err, tpl_ids){ assert.ifError(err); - return { template_ids: tpl_ids }; - }, - finishFn(self, req, res, 'GET TEMPLATE LIST', null, next) + + res.status(200); + + const method = req.query.callback ? 'jsonp' : 'json'; + res[method]({ template_ids: tpl_ids }); + } ); }; @@ -205,20 +219,3 @@ NamedMapsAdminController.prototype.checkContentType = function (action, label) { next(); }; }; - -function finishFn(controller, req, res, description, status, next) { - return function finish(err, body){ - if (err) { - err.label = description; - next(err); - } else { - res.status(status || 200); - - if (req.query && req.query.callback) { - res.jsonp(body); - } else { - res.json(body); - } - } - }; -} From 70932c23df9f7fa764a0ba4db2007a03b6465f14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 15:17:29 +0100 Subject: [PATCH 07/15] Remove step and assert dependencies --- lib/cartodb/controllers/named_maps_admin.js | 156 ++++++++------------ 1 file changed, 64 insertions(+), 92 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index ca7b4901..2efac8cc 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -1,11 +1,8 @@ -var step = require('step'); -var assert = require('assert'); var templateName = require('../backends/template_maps').templateName; var cors = require('../middleware/cors'); var userMiddleware = require('../middleware/user'); - /** * @param {AuthApi} authApi * @param {PgConnection} pgConnection @@ -69,123 +66,98 @@ NamedMapsAdminController.prototype.register = function (app) { }; NamedMapsAdminController.prototype.create = function(req, res, next) { - var self = this; + const cdbuser = res.locals.user; + const cfg = req.body; - var cdbuser = res.locals.user; - - step( - function addTemplate() { - var cfg = req.body; - self.templateMaps.addTemplate(cdbuser, cfg, this); - }, - function prepareResponse(err, tpl_id){ - assert.ifError(err); - - res.status(200); - - const method = req.query.callback ? 'jsonp' : 'json'; - res[method]({ template_id: tpl_id }); + this.templateMaps.addTemplate(cdbuser, cfg, (err, tpl_id) => { + if (err) { + return next(err); } - ); + + res.status(200); + + const method = req.query.callback ? 'jsonp' : 'json'; + res[method]({ template_id: tpl_id }); + }); }; NamedMapsAdminController.prototype.update = function(req, res, next) { - var self = this; + const cdbuser = res.locals.user; + const template = req.body; + const tpl_id = templateName(req.params.template_id); - var cdbuser = res.locals.user; - var template; - var tpl_id; - - step( - function updateTemplate() { - template = req.body; - tpl_id = templateName(req.params.template_id); - self.templateMaps.updTemplate(cdbuser, tpl_id, template, this); - }, - function prepareResponse(err){ - assert.ifError(err); - - res.status(200); - - const method = req.query.callback ? 'jsonp' : 'json'; - res[method]({ template_id: tpl_id }); + this.templateMaps.updTemplate(cdbuser, tpl_id, template, (err) => { + if (err) { + return next(err); } - ); + + res.status(200); + + const method = req.query.callback ? 'jsonp' : 'json'; + res[method]({ template_id: tpl_id }); + }); }; NamedMapsAdminController.prototype.retrieve = function(req, res, next) { - var self = this; - req.profiler.start('windshaft-cartodb.get_template'); - var cdbuser = res.locals.user; - var tpl_id; - step( - function getTemplate() { - tpl_id = templateName(req.params.template_id); - self.templateMaps.getTemplate(cdbuser, tpl_id, this); - }, - function prepareResponse(err, tpl_val) { - assert.ifError(err); - if ( ! tpl_val ) { - const error = new Error(`Cannot find template '${tpl_id}' of user '${cdbuser}'`); - error.http_status = 404; - return next(error); - } - // auth_id was added by ourselves, - // so we remove it before returning to the user - delete tpl_val.auth_id; + const cdbuser = res.locals.user; + const tpl_id = templateName(req.params.template_id); - res.status(200); - - const method = req.query.callback ? 'jsonp' : 'json'; - res[method]({ template: tpl_val }); + this.templateMaps.getTemplate(cdbuser, tpl_id, (err, tpl_val) => { + if (err) { + return next(err); } - ); + + if (!tpl_val) { + const error = new Error(`Cannot find template '${tpl_id}' of user '${cdbuser}'`); + error.http_status = 404; + return next(error); + } + // auth_id was added by ourselves, + // so we remove it before returning to the user + delete tpl_val.auth_id; + + res.status(200); + + const method = req.query.callback ? 'jsonp' : 'json'; + res[method]({ template: tpl_val }); + }); }; NamedMapsAdminController.prototype.destroy = function(req, res, next) { - var self = this; - req.profiler.start('windshaft-cartodb.delete_template'); - var cdbuser = res.locals.user; - var tpl_id; - step( - function deleteTemplate() { - tpl_id = templateName(req.params.template_id); - self.templateMaps.delTemplate(cdbuser, tpl_id, this); - }, - function prepareResponse(err/*, tpl_val*/){ - assert.ifError(err); + const cdbuser = res.locals.user; + const tpl_id = templateName(req.params.template_id); - res.status(204); - - const method = req.query.callback ? 'jsonp' : 'json'; - res[method](''); + this.templateMaps.delTemplate(cdbuser, tpl_id, (err/*, tpl_val*/) => { + if (err) { + return next(err); } - ); + + res.status(204); + + const method = req.query.callback ? 'jsonp' : 'json'; + res[method](''); + }); }; NamedMapsAdminController.prototype.list = function(req, res, next) { - var self = this; req.profiler.start('windshaft-cartodb.get_template_list'); - var cdbuser = res.locals.user; + const cdbuser = res.locals.user; - step( - function listTemplates() { - self.templateMaps.listTemplates(cdbuser, this); - }, - function prepareResponse(err, tpl_ids){ - assert.ifError(err); - - res.status(200); - - const method = req.query.callback ? 'jsonp' : 'json'; - res[method]({ template_ids: tpl_ids }); + this.templateMaps.listTemplates(cdbuser, (err, tpl_ids) => { + if (err) { + return next(err); } - ); + + res.status(200); + + const method = req.query.callback ? 'jsonp' : 'json'; + res[method]({ template_ids: tpl_ids }); + }); }; NamedMapsAdminController.prototype.authorizedByAPIKey = function (action, label) { From cf2b73e4732ad3e068399f6f50cc074dd75c1e42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 15:19:52 +0100 Subject: [PATCH 08/15] Move to up intermediate middlewares --- lib/cartodb/controllers/named_maps_admin.js | 64 ++++++++++----------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 2efac8cc..4d866b9b 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -65,6 +65,38 @@ 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, req, (err, authenticated) => { + if (err) { + return next(err); + } + + if (!authenticated) { + const error = new Error(`Only authenticated user can ${action} templated maps`); + error.http_status = 403; + error.label = label; + return next(error); + } + + next(); + }); + }.bind(this); +}; + +NamedMapsAdminController.prototype.checkContentType = function (action, label) { + return function checkContentTypeMiddleware (req, res, next) { + if (!req.is('application/json')) { + const error = new Error(`template ${action} data must be of type application/json`); + error.label = label; + return next(error); + } + next(); + }; +}; + NamedMapsAdminController.prototype.create = function(req, res, next) { const cdbuser = res.locals.user; const cfg = req.body; @@ -159,35 +191,3 @@ NamedMapsAdminController.prototype.list = function(req, res, next) { res[method]({ template_ids: tpl_ids }); }); }; - -NamedMapsAdminController.prototype.authorizedByAPIKey = function (action, label) { - return function authorizedByAPIKeyMiddleware (req, res, next) { - const { user } = res.locals; - - this.authApi.authorizedByAPIKey(user, req, (err, authenticated) => { - if (err) { - return next(err); - } - - if (!authenticated) { - const error = new Error(`Only authenticated user can ${action} templated maps`); - error.http_status = 403; - error.label = label; - return next(error); - } - - next(); - }); - }.bind(this); -}; - -NamedMapsAdminController.prototype.checkContentType = function (action, label) { - return function checkContentTypeMiddleware (req, res, next) { - if (!req.is('application/json')) { - const error = new Error(`template ${action} data must be of type application/json`); - error.label = label; - return next(error); - } - next(); - }; -}; From 64d601179d5697edec684ade653eea1cf5c35785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 15:22:17 +0100 Subject: [PATCH 09/15] Use const instead of var --- lib/cartodb/controllers/named_maps_admin.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 4d866b9b..4a0e97bf 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -1,7 +1,6 @@ -var templateName = require('../backends/template_maps').templateName; - -var cors = require('../middleware/cors'); -var userMiddleware = require('../middleware/user'); +const { templateName } = require('../backends/template_maps'); +const cors = require('../middleware/cors'); +const userMiddleware = require('../middleware/user'); /** * @param {AuthApi} authApi From a00c2b1eef028a1fd8dbb7d69c60fe8873613e5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 16:15:48 +0100 Subject: [PATCH 10/15] Now main middlewares return a named function with the right context bound --- lib/cartodb/controllers/named_maps_admin.js | 152 +++++++++++--------- 1 file changed, 81 insertions(+), 71 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 4a0e97bf..6f991728 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -22,7 +22,7 @@ NamedMapsAdminController.prototype.register = function (app) { userMiddleware, this.checkContentType('POST', 'POST TEMPLATE'), this.authorizedByAPIKey('create', 'POST TEMPLATE'), - this.create.bind(this) + this.create() ); app.put( @@ -31,7 +31,7 @@ NamedMapsAdminController.prototype.register = function (app) { userMiddleware, this.checkContentType('PUT', 'PUT TEMPLATE'), this.authorizedByAPIKey('update', 'PUT TEMPLATE'), - this.update.bind(this) + this.update() ); app.get( @@ -39,7 +39,7 @@ NamedMapsAdminController.prototype.register = function (app) { cors(), userMiddleware, this.authorizedByAPIKey('get', 'GET TEMPLATE'), - this.retrieve.bind(this) + this.retrieve() ); app.delete( @@ -47,7 +47,7 @@ NamedMapsAdminController.prototype.register = function (app) { cors(), userMiddleware, this.authorizedByAPIKey('delete', 'DELETE TEMPLATE'), - this.destroy.bind(this) + this.destroy() ); app.get( @@ -55,7 +55,7 @@ NamedMapsAdminController.prototype.register = function (app) { cors(), userMiddleware, this.authorizedByAPIKey('list', 'GET TEMPLATE LIST'), - this.list.bind(this) + this.list() ); app.options( @@ -96,97 +96,107 @@ NamedMapsAdminController.prototype.checkContentType = function (action, label) { }; }; -NamedMapsAdminController.prototype.create = function(req, res, next) { - const cdbuser = res.locals.user; - const cfg = req.body; +NamedMapsAdminController.prototype.create = function () { + return function createTemplateMiddleware (req, res, next) { + const cdbuser = res.locals.user; + const cfg = req.body; - this.templateMaps.addTemplate(cdbuser, cfg, (err, tpl_id) => { - if (err) { - return next(err); - } + this.templateMaps.addTemplate(cdbuser, cfg, (err, tpl_id) => { + if (err) { + return next(err); + } - res.status(200); + res.status(200); - const method = req.query.callback ? 'jsonp' : 'json'; - res[method]({ template_id: tpl_id }); - }); + const method = req.query.callback ? 'jsonp' : 'json'; + res[method]({ template_id: tpl_id }); + }); + }.bind(this); }; -NamedMapsAdminController.prototype.update = function(req, res, next) { - const cdbuser = res.locals.user; - const template = req.body; - const tpl_id = templateName(req.params.template_id); +NamedMapsAdminController.prototype.update = function () { + return function updateTemplateMiddleware (req, res, next) { + const cdbuser = res.locals.user; + const template = req.body; + const tpl_id = templateName(req.params.template_id); - this.templateMaps.updTemplate(cdbuser, tpl_id, template, (err) => { - if (err) { - return next(err); - } + this.templateMaps.updTemplate(cdbuser, tpl_id, template, (err) => { + if (err) { + return next(err); + } - res.status(200); + res.status(200); - const method = req.query.callback ? 'jsonp' : 'json'; - res[method]({ template_id: tpl_id }); - }); + const method = req.query.callback ? 'jsonp' : 'json'; + res[method]({ template_id: tpl_id }); + }); + }.bind(this); }; -NamedMapsAdminController.prototype.retrieve = function(req, res, next) { - req.profiler.start('windshaft-cartodb.get_template'); +NamedMapsAdminController.prototype.retrieve = function () { + return function updateTemplateMiddleware (req, res, next) { + req.profiler.start('windshaft-cartodb.get_template'); - const cdbuser = res.locals.user; - const tpl_id = templateName(req.params.template_id); + const cdbuser = res.locals.user; + const tpl_id = templateName(req.params.template_id); - this.templateMaps.getTemplate(cdbuser, tpl_id, (err, tpl_val) => { - if (err) { - return next(err); - } + this.templateMaps.getTemplate(cdbuser, tpl_id, (err, tpl_val) => { + if (err) { + return next(err); + } - if (!tpl_val) { - const error = new Error(`Cannot find template '${tpl_id}' of user '${cdbuser}'`); - error.http_status = 404; - return next(error); - } - // auth_id was added by ourselves, - // so we remove it before returning to the user - delete tpl_val.auth_id; + if (!tpl_val) { + const error = new Error(`Cannot find template '${tpl_id}' of user '${cdbuser}'`); + error.http_status = 404; + return next(error); + } + // auth_id was added by ourselves, + // so we remove it before returning to the user + delete tpl_val.auth_id; - res.status(200); + res.status(200); - const method = req.query.callback ? 'jsonp' : 'json'; - res[method]({ template: tpl_val }); - }); + const method = req.query.callback ? 'jsonp' : 'json'; + res[method]({ template: tpl_val }); + }); + }.bind(this); }; -NamedMapsAdminController.prototype.destroy = function(req, res, next) { - req.profiler.start('windshaft-cartodb.delete_template'); +NamedMapsAdminController.prototype.destroy = function () { + return function destroyTemplateMiddleware (req, res, next) { + req.profiler.start('windshaft-cartodb.delete_template'); - const cdbuser = res.locals.user; - const tpl_id = templateName(req.params.template_id); + const cdbuser = res.locals.user; + const tpl_id = templateName(req.params.template_id); - this.templateMaps.delTemplate(cdbuser, tpl_id, (err/*, tpl_val*/) => { - if (err) { - return next(err); - } + this.templateMaps.delTemplate(cdbuser, tpl_id, (err/*, tpl_val*/) => { + if (err) { + return next(err); + } - res.status(204); + res.status(204); - const method = req.query.callback ? 'jsonp' : 'json'; - res[method](''); - }); + const method = req.query.callback ? 'jsonp' : 'json'; + res[method](''); + }); + }.bind(this); }; -NamedMapsAdminController.prototype.list = function(req, res, next) { - req.profiler.start('windshaft-cartodb.get_template_list'); +NamedMapsAdminController.prototype.list = function () { + return function listTemplatesMiddleware (req, res, next) { + req.profiler.start('windshaft-cartodb.get_template_list'); - const cdbuser = res.locals.user; + const cdbuser = res.locals.user; - this.templateMaps.listTemplates(cdbuser, (err, tpl_ids) => { - if (err) { - return next(err); - } + this.templateMaps.listTemplates(cdbuser, (err, tpl_ids) => { + if (err) { + return next(err); + } - res.status(200); + res.status(200); - const method = req.query.callback ? 'jsonp' : 'json'; - res[method]({ template_ids: tpl_ids }); - }); + const method = req.query.callback ? 'jsonp' : 'json'; + res[method]({ template_ids: tpl_ids }); + }); + }.bind(this); }; From f186e4736be061502401706f65d0c93480c9c300 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 16:19:00 +0100 Subject: [PATCH 11/15] Use template string --- lib/cartodb/controllers/named_maps_admin.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 6f991728..c28a2746 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -16,8 +16,10 @@ function NamedMapsAdminController(authApi, templateMaps) { module.exports = NamedMapsAdminController; NamedMapsAdminController.prototype.register = function (app) { + const prefix = app.base_url_templated; + app.post( - app.base_url_templated + '/', + `${prefix}/`, cors(), userMiddleware, this.checkContentType('POST', 'POST TEMPLATE'), @@ -26,7 +28,7 @@ NamedMapsAdminController.prototype.register = function (app) { ); app.put( - app.base_url_templated + '/:template_id', + `${prefix}/:template_id`, cors(), userMiddleware, this.checkContentType('PUT', 'PUT TEMPLATE'), @@ -35,7 +37,7 @@ NamedMapsAdminController.prototype.register = function (app) { ); app.get( - app.base_url_templated + '/:template_id', + `${prefix}/:template_id`, cors(), userMiddleware, this.authorizedByAPIKey('get', 'GET TEMPLATE'), @@ -43,7 +45,7 @@ NamedMapsAdminController.prototype.register = function (app) { ); app.delete( - app.base_url_templated + '/:template_id', + `${prefix}/:template_id`, cors(), userMiddleware, this.authorizedByAPIKey('delete', 'DELETE TEMPLATE'), @@ -51,7 +53,7 @@ NamedMapsAdminController.prototype.register = function (app) { ); app.get( - app.base_url_templated + '/', + `${prefix}/`, cors(), userMiddleware, this.authorizedByAPIKey('list', 'GET TEMPLATE LIST'), @@ -59,7 +61,7 @@ NamedMapsAdminController.prototype.register = function (app) { ); app.options( - app.base_url_templated + '/:template_id', + `${prefix}/:template_id`, cors('Content-Type') ); }; From 5d3726de444d60f056329f7a0e869a8d544a36ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 16:24:19 +0100 Subject: [PATCH 12/15] Use original variable name --- lib/cartodb/controllers/named_maps_admin.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index c28a2746..9b3701ba 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -16,10 +16,10 @@ function NamedMapsAdminController(authApi, templateMaps) { module.exports = NamedMapsAdminController; NamedMapsAdminController.prototype.register = function (app) { - const prefix = app.base_url_templated; + const { base_url_templated } = app; app.post( - `${prefix}/`, + `${base_url_templated}/`, cors(), userMiddleware, this.checkContentType('POST', 'POST TEMPLATE'), @@ -28,7 +28,7 @@ NamedMapsAdminController.prototype.register = function (app) { ); app.put( - `${prefix}/:template_id`, + `${base_url_templated}/:template_id`, cors(), userMiddleware, this.checkContentType('PUT', 'PUT TEMPLATE'), @@ -37,7 +37,7 @@ NamedMapsAdminController.prototype.register = function (app) { ); app.get( - `${prefix}/:template_id`, + `${base_url_templated}/:template_id`, cors(), userMiddleware, this.authorizedByAPIKey('get', 'GET TEMPLATE'), @@ -45,7 +45,7 @@ NamedMapsAdminController.prototype.register = function (app) { ); app.delete( - `${prefix}/:template_id`, + `${base_url_templated}/:template_id`, cors(), userMiddleware, this.authorizedByAPIKey('delete', 'DELETE TEMPLATE'), @@ -53,7 +53,7 @@ NamedMapsAdminController.prototype.register = function (app) { ); app.get( - `${prefix}/`, + `${base_url_templated}/`, cors(), userMiddleware, this.authorizedByAPIKey('list', 'GET TEMPLATE LIST'), @@ -61,7 +61,7 @@ NamedMapsAdminController.prototype.register = function (app) { ); app.options( - `${prefix}/:template_id`, + `${base_url_templated}/:template_id`, cors('Content-Type') ); }; From adb9e55fb278373b90b6c093d43917ebb3dd49cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 16:30:42 +0100 Subject: [PATCH 13/15] Avoid snake_case notation --- lib/cartodb/controllers/named_maps_admin.js | 38 ++++++++++----------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 9b3701ba..df24bdc2 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -101,9 +101,9 @@ NamedMapsAdminController.prototype.checkContentType = function (action, label) { NamedMapsAdminController.prototype.create = function () { return function createTemplateMiddleware (req, res, next) { const cdbuser = res.locals.user; - const cfg = req.body; + const template = req.body; - this.templateMaps.addTemplate(cdbuser, cfg, (err, tpl_id) => { + this.templateMaps.addTemplate(cdbuser, template, (err, templateId) => { if (err) { return next(err); } @@ -111,18 +111,18 @@ NamedMapsAdminController.prototype.create = function () { res.status(200); const method = req.query.callback ? 'jsonp' : 'json'; - res[method]({ template_id: tpl_id }); + res[method]({ template_id: templateId }); }); }.bind(this); }; NamedMapsAdminController.prototype.update = function () { return function updateTemplateMiddleware (req, res, next) { - const cdbuser = res.locals.user; + const { user } = res.locals; const template = req.body; - const tpl_id = templateName(req.params.template_id); + const templateId = templateName(req.params.template_id); - this.templateMaps.updTemplate(cdbuser, tpl_id, template, (err) => { + this.templateMaps.updTemplate(user, templateId, template, (err) => { if (err) { return next(err); } @@ -130,7 +130,7 @@ NamedMapsAdminController.prototype.update = function () { res.status(200); const method = req.query.callback ? 'jsonp' : 'json'; - res[method]({ template_id: tpl_id }); + res[method]({ template_id: templateId }); }); }.bind(this); }; @@ -140,26 +140,26 @@ NamedMapsAdminController.prototype.retrieve = function () { req.profiler.start('windshaft-cartodb.get_template'); const cdbuser = res.locals.user; - const tpl_id = templateName(req.params.template_id); + const templateId = templateName(req.params.template_id); - this.templateMaps.getTemplate(cdbuser, tpl_id, (err, tpl_val) => { + this.templateMaps.getTemplate(cdbuser, templateId, (err, template) => { if (err) { return next(err); } - if (!tpl_val) { - const error = new Error(`Cannot find template '${tpl_id}' of user '${cdbuser}'`); + if (!template) { + const error = new Error(`Cannot find template '${templateId}' of user '${cdbuser}'`); error.http_status = 404; return next(error); } // auth_id was added by ourselves, // so we remove it before returning to the user - delete tpl_val.auth_id; + delete template.auth_id; res.status(200); const method = req.query.callback ? 'jsonp' : 'json'; - res[method]({ template: tpl_val }); + res[method]({ template }); }); }.bind(this); }; @@ -168,10 +168,10 @@ NamedMapsAdminController.prototype.destroy = function () { return function destroyTemplateMiddleware (req, res, next) { req.profiler.start('windshaft-cartodb.delete_template'); - const cdbuser = res.locals.user; - const tpl_id = templateName(req.params.template_id); + const { user } = res.locals; + const templateId = templateName(req.params.template_id); - this.templateMaps.delTemplate(cdbuser, tpl_id, (err/*, tpl_val*/) => { + this.templateMaps.delTemplate(user, templateId, (err/* , tpl_val */) => { if (err) { return next(err); } @@ -188,9 +188,9 @@ NamedMapsAdminController.prototype.list = function () { return function listTemplatesMiddleware (req, res, next) { req.profiler.start('windshaft-cartodb.get_template_list'); - const cdbuser = res.locals.user; + const { user } = res.locals; - this.templateMaps.listTemplates(cdbuser, (err, tpl_ids) => { + this.templateMaps.listTemplates(user, (err, templateIds) => { if (err) { return next(err); } @@ -198,7 +198,7 @@ NamedMapsAdminController.prototype.list = function () { res.status(200); const method = req.query.callback ? 'jsonp' : 'json'; - res[method]({ template_ids: tpl_ids }); + res[method]({ template_ids: templateIds }); }); }.bind(this); }; From dfef7ff3c0c8d27c832c328d521c4b43ff38f958 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 18:34:54 +0100 Subject: [PATCH 14/15] Use spread assignment --- lib/cartodb/controllers/named_maps_admin.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index df24bdc2..1653c840 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -100,10 +100,10 @@ NamedMapsAdminController.prototype.checkContentType = function (action, label) { NamedMapsAdminController.prototype.create = function () { return function createTemplateMiddleware (req, res, next) { - const cdbuser = res.locals.user; + const { user } = res.locals; const template = req.body; - this.templateMaps.addTemplate(cdbuser, template, (err, templateId) => { + this.templateMaps.addTemplate(user, template, (err, templateId) => { if (err) { return next(err); } @@ -139,16 +139,16 @@ NamedMapsAdminController.prototype.retrieve = function () { return function updateTemplateMiddleware (req, res, next) { req.profiler.start('windshaft-cartodb.get_template'); - const cdbuser = res.locals.user; + const { user } = res.locals; const templateId = templateName(req.params.template_id); - this.templateMaps.getTemplate(cdbuser, templateId, (err, template) => { + this.templateMaps.getTemplate(user, templateId, (err, template) => { if (err) { return next(err); } if (!template) { - const error = new Error(`Cannot find template '${templateId}' of user '${cdbuser}'`); + const error = new Error(`Cannot find template '${templateId}' of user '${user}'`); error.http_status = 404; return next(error); } From 1ba240d099c7a09396b93edd14f4ab022fc93110 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 3 Jan 2018 13:15:11 +0100 Subject: [PATCH 15/15] Rename middleware function --- lib/cartodb/controllers/named_maps_admin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 1653c840..a4d05467 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -136,7 +136,7 @@ NamedMapsAdminController.prototype.update = function () { }; NamedMapsAdminController.prototype.retrieve = function () { - return function updateTemplateMiddleware (req, res, next) { + return function retrieveTemplateMiddleware (req, res, next) { req.profiler.start('windshaft-cartodb.get_template'); const { user } = res.locals;