From ecc9ea1226a0af873240f27b7b5d953539e80b93 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 3 Mar 2014 17:18:06 +0100 Subject: [PATCH] Use 403 for forbidden, not 401 Includes upgrade of windshaft to 0.19.3 Includes upgrade of redis-mpool to 0.0.4 --- lib/cartodb/cartodb_windshaft.js | 14 +++++++------- lib/cartodb/server_options.js | 6 +++++- npm-shrinkwrap.json | 25 +++++++++++++------------ package.json | 4 ++-- test/acceptance/multilayer.js | 8 ++++---- test/acceptance/server.js | 30 +++++++++++++++--------------- test/acceptance/templates.js | 26 +++++++++++++------------- 7 files changed, 59 insertions(+), 54 deletions(-) diff --git a/lib/cartodb/cartodb_windshaft.js b/lib/cartodb/cartodb_windshaft.js index 01df38f8..7038989a 100644 --- a/lib/cartodb/cartodb_windshaft.js +++ b/lib/cartodb/cartodb_windshaft.js @@ -204,7 +204,7 @@ var CartodbWindshaft = function(serverOptions) { if ( err ) throw err; if (authenticated !== 1) { err = new Error("Only authenticated user can create templated maps"); - err.http_status = 401; + err.http_status = 403; throw err; } var next = this; @@ -253,7 +253,7 @@ var CartodbWindshaft = function(serverOptions) { if ( err ) throw err; if (authenticated !== 1) { err = new Error("Only authenticated user can list templated maps"); - err.http_status = 401; + err.http_status = 403; throw err; } if ( ! req.headers['content-type'] || req.headers['content-type'].split(';')[0] != 'application/json' ) @@ -313,7 +313,7 @@ var CartodbWindshaft = function(serverOptions) { if ( err ) throw err; if (authenticated !== 1) { err = new Error("Only authenticated users can get template maps"); - err.http_status = 401; + err.http_status = 403; throw err; } tpl_id = req.params.template_id.split('@'); @@ -374,7 +374,7 @@ var CartodbWindshaft = function(serverOptions) { if ( err ) throw err; if (authenticated !== 1) { err = new Error("Only authenticated users can delete template maps"); - err.http_status = 401; + err.http_status = 403; throw err; } tpl_id = req.params.template_id.split('@'); @@ -425,7 +425,7 @@ var CartodbWindshaft = function(serverOptions) { if ( err ) throw err; if (authenticated !== 1) { err = new Error("Only authenticated user can list templated maps"); - err.http_status = 401; + err.http_status = 403; throw err; } templateMaps.listTemplates(cdbuser, this); @@ -517,12 +517,12 @@ var CartodbWindshaft = function(serverOptions) { authorized = signedMaps.authorizedByCert(cert, auth_token); } catch (err) { // we catch to add http_status - err.http_status = 401; + err.http_status = 403; throw err; } if ( ! authorized ) { err = new Error('Unauthorized template instanciation'); - err.http_status = 401; + err.http_status = 403; throw err; } /*if ( (! req.headers['content-type'] || req.headers['content-type'].split(';')[0] != 'application/json') && req.query.callback === undefined) { diff --git a/lib/cartodb/server_options.js b/lib/cartodb/server_options.js index 3b8b50d1..854aca9e 100644 --- a/lib/cartodb/server_options.js +++ b/lib/cartodb/server_options.js @@ -766,7 +766,11 @@ module.exports = function(){ function gatekeep(err, authorized){ if (req.profiler) req.profiler.done('authorize'); if(err) throw err; - if(!authorized) throw new Error("Sorry, you are unauthorized (permission denied)"); + if(!authorized) { + err = new Error("Sorry, you are unauthorized (permission denied)"); + err.http_status = 403; + throw err; + } return null; }, function getDatabase(err){ diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 810d7737..ae02781d 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,6 +1,6 @@ { "name": "windshaft-cartodb", - "version": "1.8.4", + "version": "1.8.5", "dependencies": { "node-varnish": { "version": "0.2.0", @@ -10,11 +10,11 @@ "version": "1.3.3" }, "windshaft": { - "version": "0.19.1", - "from": "http://github.com/CartoDB/Windshaft/tarball/0.19.2", + "version": "0.19.3", + "from": "http://github.com/CartoDB/Windshaft/tarball/0.19.3", "dependencies": { "grainstore": { - "version": "0.18.0", + "version": "0.18.1", "dependencies": { "mapnik-reference": { "version": "5.0.7" @@ -191,7 +191,7 @@ } }, "readable-stream": { - "version": "1.0.26", + "version": "1.0.26-2", "dependencies": { "string_decoder": { "version": "0.10.25-1" @@ -387,7 +387,8 @@ "version": "0.3.0" }, "redis-mpool": { - "version": "0.0.3", + "version": "0.0.4", + "from": "http://github.com/CartoDB/node-redis-mpool/tarball/0.0.4", "dependencies": { "generic-pool": { "version": "2.0.4" @@ -415,7 +416,7 @@ "version": "0.1.15" }, "readable-stream": { - "version": "1.0.25-1", + "version": "1.0.26", "dependencies": { "string_decoder": { "version": "0.10.25-1" @@ -424,14 +425,14 @@ } } }, - "redis": { - "version": "0.8.6" + "semver": { + "version": "1.1.4" }, "strftime": { "version": "0.6.2" }, - "semver": { - "version": "1.1.4" + "redis": { + "version": "0.8.6" }, "mocha": { "version": "1.14.0", @@ -477,7 +478,7 @@ } }, "graceful-fs": { - "version": "2.0.1" + "version": "2.0.2" }, "inherits": { "version": "2.0.1" diff --git a/package.json b/package.json index 5a62c38b..c70d057a 100644 --- a/package.json +++ b/package.json @@ -24,11 +24,11 @@ "dependencies": { "node-varnish": "http://github.com/Vizzuality/node-varnish/tarball/v0.2.0", "underscore" : "~1.3.3", - "windshaft" : "http://github.com/CartoDB/Windshaft/tarball/0.19.2", + "windshaft" : "http://github.com/CartoDB/Windshaft/tarball/0.19.3", "step": "0.0.x", "request": "2.9.202", "cartodb-redis": "~0.3.0", - "redis-mpool": "~0.0.2", + "redis-mpool": "http://github.com/CartoDB/node-redis-mpool/tarball/0.0.4", "mapnik": "~0.7.22", "lzma": "~1.2.3", "log4js": "~0.6.10" diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index afc5c622..c248e8d6 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -130,7 +130,7 @@ suite('multilayer', function() { headers: {host: 'localhost' }, encoding: 'binary' }, {}, function(res) { - assert.equal(res.statusCode, 401, res.statusCode + ':' + res.body); + assert.equal(res.statusCode, 403, res.statusCode + ':' + res.body); var parsed = JSON.parse(res.body); var msg = parsed.error; // TODO: should it be "errors" ? assert.ok(msg.match(/permission denied/i), msg); @@ -674,7 +674,7 @@ suite('multilayer', function() { headers: {host: 'localhost' }, encoding: 'binary' }, {}, function(res) { - assert.equal(res.statusCode, 401); + assert.equal(res.statusCode, 403); var re = RegExp('permission denied'); assert.ok(res.body.match(re), 'No "permission denied" error: ' + res.body); next(err); @@ -690,7 +690,7 @@ suite('multilayer', function() { headers: {host: 'localhost' }, method: 'GET' }, {}, function(res) { - assert.equal(res.statusCode, 401); + assert.equal(res.statusCode, 403); var re = RegExp('permission denied'); assert.ok(res.body.match(re), 'No "permission denied" error: ' + res.body); next(err); @@ -706,7 +706,7 @@ suite('multilayer', function() { headers: {host: 'localhost' }, method: 'GET' }, {}, function(res) { - assert.equal(res.statusCode, 401); + assert.equal(res.statusCode, 403); var re = RegExp('permission denied'); assert.ok(res.body.match(re), 'No "permission denied" error: ' + res.body); next(err); diff --git a/test/acceptance/server.js b/test/acceptance/server.js index 7045f5cc..0392083a 100644 --- a/test/acceptance/server.js +++ b/test/acceptance/server.js @@ -134,7 +134,7 @@ suite('server', function() { method: 'GET' },{ }, function(res) { - assert.equal(res.statusCode, 401, res.statusCode + ':' + res.body); + assert.equal(res.statusCode, 403, res.statusCode + ':' + res.body); assert.deepEqual(JSON.parse(res.body), {error: 'Sorry, you are unauthorized (permission denied)'}); assert.ok(!res.headers.hasOwnProperty('cache-control')); @@ -151,7 +151,7 @@ suite('server', function() { method: 'GET' },{ }, function(res) { - // FIXME: should be 401 Unauthorized + // FIXME: should be 403 Forbidden or 404 User Not Found assert.equal(res.statusCode, 400, res.statusCode + ': ' + res.body); assert.deepEqual(JSON.parse(res.body), {error:"missing unknown_user's database_name in redis (try CARTODB/script/restore_redis)"}); @@ -319,7 +319,7 @@ suite('server', function() { headers: {host: 'localhost', 'Content-Type': 'application/x-www-form-urlencoded' }, data: querystring.stringify({style: 'Map { background-color:#aaa; }'}) },{}, function(res) { - // FIXME: should be 401 Unauthorized + // FIXME: should be 403 Forbidden assert.equal(res.statusCode, 400, res.statusCode + ': ' + res.body); assert.ok(res.body.indexOf('map state cannot be changed by unauthenticated request') != -1, res.body); @@ -417,7 +417,7 @@ suite('server', function() { method: 'DELETE', headers: {host: 'localhost'}, },{}, function(res) { - // FIXME: should be 401 Unauthorized + // FIXME: should be 403 Forbidden assert.equal(res.statusCode, 400, res.body); assert.ok(res.body.indexOf('map state cannot be changed by unauthenticated request') != -1, res.body); // check that the style wasn't really deleted ! @@ -531,7 +531,7 @@ suite('server', function() { url: '/tiles/test_table_private_1/infowindow', method: 'GET' },{}, function(res) { - // FIXME: should be 401 Unauthorized + // FIXME: should be 403 Forbidden assert.equal(res.statusCode, 500, res.statusCode + ': ' + res.body); done(); }); @@ -546,7 +546,7 @@ suite('server', function() { method: 'GET' },{ }, function(res) { - // FIXME: should be 401 Unauthorized + // FIXME: should be 403 Forbidden assert.equal(res.statusCode, 500, res.statusCode + ': ' + res.body); assert.deepEqual(JSON.parse(res.body), {error:"missing unknown_user's database_name in redis (try CARTODB/script/restore_redis)"}); @@ -614,8 +614,8 @@ suite('server', function() { url: '/tiles/test_table_private_1/6/31/24.grid.json', method: 'GET' },{}, function(res) { - // 401 Unauthorized - assert.equal(res.statusCode, 401, res.statusCode + ': ' + res.body); + // 403 Forbidden + assert.equal(res.statusCode, 403, res.statusCode + ': ' + res.body); done(); }); }); @@ -629,7 +629,7 @@ suite('server', function() { method: 'GET' },{ }, function(res) { - // FIXME: should be 401 Unauthorized + // FIXME: should be 403 Forbidden assert.equal(res.statusCode, 400, res.statusCode + ': ' + res.body); assert.deepEqual(JSON.parse(res.body), {error:"missing unknown_user's database_name in redis (try CARTODB/script/restore_redis)"}); @@ -763,8 +763,8 @@ suite('server', function() { method: 'GET' },{ }, function(res) { - // 401 Unauthorized - assert.equal(res.statusCode, 401, res.statusCode + ': ' + res.body); + // 403 Forbidden + assert.equal(res.statusCode, 403, res.statusCode + ': ' + res.body); done(); }); }); @@ -780,7 +780,7 @@ suite('server', function() { method: 'GET' },{ }, function(res) { - // FIXME: should be 401 Unauthorized + // FIXME: should be 403 Forbidden assert.equal(res.statusCode, 400, res.statusCode + ': ' + res.body); assert.deepEqual(JSON.parse(res.body), {error:"missing unknown_user's database_name in redis (try CARTODB/script/restore_redis)"}); @@ -804,8 +804,8 @@ suite('server', function() { method: 'GET' },{ }, function(res) { - // 401 Unauthorized - assert.equal(res.statusCode, 401, res.statusCode + ': ' + res.body); + // 403 Forbidden + assert.equal(res.statusCode, 403, res.statusCode + ': ' + res.body); // Failed in 1.6.0 of https://github.com/CartoDB/Windshaft-cartodb/issues/107 assert.ok(!res.headers.hasOwnProperty('cache-control'), "Unexpected Cache-Control: " + res.headers['cache-control']); @@ -1278,7 +1278,7 @@ suite('server', function() { url: '/tiles/test_table_private_1/map_metadata', method: 'GET' },{}, function(res) { - // FIXME: should be 401 instead + // FIXME: should be 403 instead assert.equal(res.statusCode, 500, res.statusCode + ': ' + res.body); assert.ok(!res.headers.hasOwnProperty('cache-control')); done(); diff --git a/test/acceptance/templates.js b/test/acceptance/templates.js index 37754b76..a75bf82f 100644 --- a/test/acceptance/templates.js +++ b/test/acceptance/templates.js @@ -74,7 +74,7 @@ suite('template_api', function() { function postTemplate(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 401); + assert.equal(res.statusCode, 403); var parsed = JSON.parse(res.body); assert.ok(parsed.hasOwnProperty('error'), res.body); err = parsed.error; @@ -369,7 +369,7 @@ suite('template_api', function() { function litsTemplates(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 401, res.statusCode + ': ' + res.body); + assert.equal(res.statusCode, 403, res.statusCode + ': ' + res.body); var parsed = JSON.parse(res.body); assert.ok(parsed.hasOwnProperty('error'), 'Missing error from response: ' + res.body); @@ -582,7 +582,7 @@ suite('template_api', function() { function getTemplate(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 401, res.statusCode + ": " + res.body); + assert.equal(res.statusCode, 403, res.statusCode + ": " + res.body); var parsedBody = JSON.parse(res.body); assert.ok(parsedBody.hasOwnProperty('error'), res.body); assert.ok(parsedBody.error.match(/only.*authenticated.*user/i), @@ -691,7 +691,7 @@ suite('template_api', function() { function deleteTemplate(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 401, res.statusCode + ": " + res.body); + assert.equal(res.statusCode, 403, res.statusCode + ": " + res.body); var parsed = JSON.parse(res.body); assert.ok(parsed.hasOwnProperty('error'), "Missing 'error' from response body: " + res.body); @@ -819,7 +819,7 @@ suite('template_api', function() { // See https://github.com/CartoDB/Windshaft-cartodb/issues/173 function instanciateForeignDB(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 401, + assert.equal(res.statusCode, 403, 'Unexpected success instanciating template with no auth: ' + res.statusCode + ': ' + res.body); var parsed = JSON.parse(res.body); @@ -882,7 +882,7 @@ suite('template_api', function() { }, function fetchTileAuth(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 401, + assert.equal(res.statusCode, 403, 'Fetching tile with no auth: ' + res.statusCode + ': ' + res.body); var parsed = JSON.parse(res.body); assert.ok(parsed.hasOwnProperty('error'), @@ -963,7 +963,7 @@ suite('template_api', function() { }, function checkTileDeleted(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 401, + assert.equal(res.statusCode, 403, 'Unexpected statusCode fetch tile after signature revokal: ' + res.statusCode + ':' + res.body); var parsed = JSON.parse(res.body); @@ -1059,7 +1059,7 @@ suite('template_api', function() { function instanciateAuth(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 401, + assert.equal(res.statusCode, 403, 'Unexpected success instanciating template with no auth: ' + res.statusCode + ': ' + res.body); var parsed = JSON.parse(res.body); @@ -1103,7 +1103,7 @@ suite('template_api', function() { }, function fetchTileAuth(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 401, + assert.equal(res.statusCode, 403, 'Fetching tile with no auth: ' + res.statusCode + ': ' + res.body); var parsed = JSON.parse(res.body); assert.ok(parsed.hasOwnProperty('error'), @@ -1157,7 +1157,7 @@ suite('template_api', function() { }, function checkTileDeleted(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 401, + assert.equal(res.statusCode, 403, 'Unexpected statusCode fetch tile after signature revokal: ' + res.statusCode + ':' + res.body); var parsed = JSON.parse(res.body); @@ -1255,7 +1255,7 @@ suite('template_api', function() { function instanciateAuth(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 401, + assert.equal(res.statusCode, 403, 'Unexpected success instanciating template with no auth: ' + res.statusCode + ': ' + res.body); var parsed = JSON.parse(res.body); @@ -1299,7 +1299,7 @@ suite('template_api', function() { }, function fetchAttributeAuth(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 401, + assert.equal(res.statusCode, 403, 'Fetching tile with no auth: ' + res.statusCode + ': ' + res.body); var parsed = JSON.parse(res.body); assert.ok(parsed.hasOwnProperty('error'), @@ -1353,7 +1353,7 @@ suite('template_api', function() { }, function checkTileDeleted(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 401, + assert.equal(res.statusCode, 403, 'Unexpected statusCode fetch tile after signature revokal: ' + res.statusCode + ':' + res.body); var parsed = JSON.parse(res.body);