New authentication mechanism: checks in advance if credentials are provided

in order to do a single request to redis to retrieve the required database
connection parameters.

.gitignore

@@ -8,3 +8,4 @@ node_modules*
 tools/munin/cdbsqlapi.conf
 test/redis.pid
 test/test.log
+test/acceptance/oauth/venv/*

NEWS.md

@@ -1,6 +1,13 @@
-1.12.2 - 2014-mm-dd
+1.13.0 - 2014-mm-dd
 -------------------
 
+New features:
+
+ * New authentication mechanism: checks in advance if credentials are provided
+   in order to do a single request to redis to retrieve the required database
+   connection parameters.
+
+
 1.12.1 - 2014-08-05
 -------------------
 

app/auth/apikey.js

@@ -1,11 +1,23 @@
 /**
  * this module allows to auth user using an pregenerated api key
  */
-function ApikeyAuth() {
+function ApikeyAuth(req) {
+    this.req = req;
 }
 
 module.exports = ApikeyAuth;
 
+ApikeyAuth.prototype.verifyCredentials = function(options, callback) {
+    verifyRequest(this.req, options.apiKey, callback);
+};
+
+ApikeyAuth.prototype.hasCredentials = function() {
+    return !!(this.req.query.api_key
+        || this.req.query.map_key
+        || (this.req.body && this.req.body.api_key)
+        || (this.req.body && this.req.body.map_key));
+};
+
 /**
  * Get id of authorized user
  *
@@ -13,7 +25,7 @@ module.exports = ApikeyAuth;
  * @param {String} requiredApi - the API associated to the user, req must contain it
  * @param {Function} callback - err, boolean (whether the request is authenticated or not)
  */
-ApikeyAuth.prototype.verifyRequest = function (req, requiredApi, callback) {
+function verifyRequest(req, requiredApi, callback) {
 
     var valid = false;
 
@@ -31,4 +43,4 @@ ApikeyAuth.prototype.verifyRequest = function (req, requiredApi, callback) {
     }
 
     callback(null, valid);
-};
+}

app/auth/auth_api.js

@@ -0,0 +1,34 @@
+var ApiKeyAuth = require('./apikey'),
+    OAuthAuth  = require('./oauth');
+
+function AuthApi(req, requestParams) {
+    this.req = req;
+    this.authBacked = getAuthBackend(req, requestParams);
+
+    this._hasCredentials = null;
+}
+
+AuthApi.prototype.hasCredentials = function() {
+    if (this._hasCredentials === null) {
+        this._hasCredentials = this.authBacked.hasCredentials();
+    }
+    return this._hasCredentials;
+};
+
+AuthApi.prototype.verifyCredentials = function(options, callback) {
+    if (this.hasCredentials()) {
+        this.authBacked.verifyCredentials(options, callback);
+    } else {
+        callback(null, false);
+    }
+};
+
+function getAuthBackend(req, requestParams) {
+    if (requestParams.api_key) {
+        return new ApiKeyAuth(req);
+    } else {
+        return new OAuthAuth(req);
+    }
+}
+
+module.exports = AuthApi;

app/auth/oauth.js

@@ -157,4 +157,28 @@ var oAuth = function(){
   return me;
 }();
 
-module.exports = oAuth;
+function OAuthAuth(req) {
+    this.req = req;
+    this.isOAuthRequest = null;
+}
+
+OAuthAuth.prototype.verifyCredentials = function(options, callback) {
+    if (this.hasCredentials()) {
+        oAuth.verifyRequest(this.req, callback, options.requestProtocol);
+    } else {
+        callback(null, false);
+    }
+};
+
+OAuthAuth.prototype.hasCredentials = function() {
+    if (this.isOAuthRequest === null) {
+        var passed_tokens = oAuth.parseTokens(this.req);
+        this.isOAuthRequest = !_.isEmpty(passed_tokens);
+    }
+
+    return this.isOAuthRequest;
+};
+
+
+module.exports = OAuthAuth;
+module.exports.backend = oAuth;

app/controllers/app.js

@@ -36,8 +36,7 @@ var express = require('express')
     , PSQL        = require(global.settings.app_root + '/app/models/psql')
     , PSQLWrapper = require(global.settings.app_root + '/app/sql/psql_wrapper')
     , CdbRequest  = require(global.settings.app_root + '/app/models/cartodb_request')
-    , oAuth       = require(global.settings.app_root + '/app/auth/oauth')
-    , ApiKeyAuth  = require(global.settings.app_root + '/app/auth/apikey')
+    , AuthApi     = require(global.settings.app_root + '/app/auth/auth_api')
     , _           = require('underscore')
     , LRU         = require('lru-cache')
     , formats     = require(global.settings.app_root + '/app/models/formats')
@@ -51,7 +50,6 @@ var metadataBackend = MetadataDB({
     reapIntervalMillis: global.settings.redisReapIntervalMillis
 });
 var cdbReq = new CdbRequest();
-var apiKeyAuth = new ApiKeyAuth(metadataBackend, cdbReq);
 
 // Set default configuration 
 global.settings.db_pubuser = global.settings.db_pubuser || "publicuser";
@@ -283,11 +281,12 @@ function handleQuery(req, res) {
 
         var formatter;
 
-        var cdbuser = cdbReq.userByReq(req);
+        var cdbUsername = cdbReq.userByReq(req),
+            authApi = new AuthApi(req, params),
+            dbParams;
 
         if ( req.profiler ) req.profiler.done('init');
 
-        var dbParams;
         // 1. Get database from redis via the username stored in the host header subdomain
         // 2. Run the request through OAuth to get R/W user id if signed
         // 3. Get the list of tables affected by the query
@@ -296,12 +295,18 @@ function handleQuery(req, res) {
         Step(
             function getDatabaseConnectionParams() {
                 checkAborted('getDatabaseConnectionParams');
-                metadataBackend.getAllUserDBParams(cdbuser, this);
+                // If the request is providing credentials it may require every DB parameters
+                if (authApi.hasCredentials()) {
+                    metadataBackend.getAllUserDBParams(cdbUsername, this);
+                } else {
+                    metadataBackend.getUserDBPublicConnectionParams(cdbUsername, this);
+                }
             },
             function authenticate(err, userDBParams) {
+                console.log(err);
                 if (err) {
                     err.http_status = 404;
-                    err.message = "Sorry, we can't find CartoDB user '" + cdbuser
+                    err.message = "Sorry, we can't find CartoDB user '" + cdbUsername
                         + "'. Please check that you have entered the correct domain.";
                     throw err;
                 }
@@ -312,11 +317,7 @@ function handleQuery(req, res) {
                 dbopts.dbname = dbParams.dbname;
                 dbopts.user = (!!dbParams.dbpublicuser) ? dbParams.dbpublicuser : global.settings.db_pubuser;
 
-                if (api_key) {
-                    apiKeyAuth.verifyRequest(req, dbParams.apikey, this);
-                } else {
-                    oAuth.verifyRequest(req, this, requestProtocol);
-                }
+                authApi.verifyCredentials({apiKey: dbParams.apikey, requestProtocol: requestProtocol}, this);
             },
             function setDBAuth(err, isAuthenticated) {
                 if (err) {

npm-shrinkwrap.json

@@ -1,6 +1,6 @@
 {
   "name": "cartodb_sql_api",
-  "version": "1.12.2",
+  "version": "1.13.0",
   "dependencies": {
     "underscore": {
       "version": "1.3.3"
@@ -48,8 +48,8 @@
       }
     },
     "cartodb-redis": {
-      "version": "0.6.0",
-      "from": "git://github.com/CartoDB/node-cartodb-redis.git#0.6.0",
+      "version": "0.7.0",
+      "from": "git://github.com/CartoDB/node-cartodb-redis.git#0.7.0",
       "dependencies": {
         "strftime": {
           "version": "0.6.2"

package.json

@@ -5,7 +5,7 @@
     "keywords": [
       "cartodb"
     ],
-    "version": "1.12.2",
+    "version": "1.13.0",
     "repository": {
         "type": "git",
         "url": "git://github.com/CartoDB/CartoDB-SQL-API.git"
@@ -20,7 +20,7 @@
         "underscore.string": "~1.1.6",
         "pg": "git://github.com/CartoDB/node-postgres.git#2.6.2-cdb1",
         "express": "~2.5.11",
-        "cartodb-redis": "~0.6.0",
+        "cartodb-redis": "git://github.com/CartoDB/node-cartodb-redis.git#0.7.0",
         "step": "0.0.x",
         "topojson": "0.0.8",
         "oauth-client": "0.2.0",

test/unit/apikeyauth.test.js

@@ -0,0 +1,93 @@
+require('../helper');
+
+var _            = require('underscore')
+    , ApikeyAuth = require('../../app/auth/apikey')
+    , assert     = require('assert')
+    ;
+
+suite('has credentials', function() {
+
+    var noCredentialsRequests = [
+        {
+            des: 'there is not api_key/map_key in the request query',
+            req: {query:{}}
+        },
+        {
+            des: 'api_key is undefined`ish in the request query',
+            req: {query:{api_key:null}}
+        },
+        {
+            des: 'map_key is undefined`ish in the request query',
+            req: {query:{map_key:null}}
+        },
+        {
+            des: 'there is not api_key/map_key in the request body',
+            req: {query:{}, body:{}}
+        },
+        {
+            des: 'api_key is undefined`ish in the request body',
+            req: {query:{}, body:{api_key:null}}
+        },
+        {
+            des: 'map_key is undefined`ish in the request body',
+            req: {query:{}, body:{map_key:null}}
+        }
+    ];
+
+    noCredentialsRequests.forEach(function(request) {
+        test('has no credentials if ' + request.des, function() {
+            testCredentials(request.req, false)
+        });
+    });
+
+    var credentialsRequests = [
+        {
+            des: 'there is api_key in the request query',
+            req: {query:{api_key: 'foo'}}
+        },
+        {
+            des: 'there is api_key in the request query',
+            req: {query:{map_key: 'foo'}}
+        },
+        {
+            des: 'there is api_key in the request body',
+            req: {query:{}, body:{api_key:'foo'}}
+        },
+        {
+            des: 'there is map_key in the request body',
+            req: {query:{}, body:{map_key:'foo'}}
+        }
+    ];
+
+    credentialsRequests.forEach(function(request) {
+        test('has credentials if ' + request.des, function() {
+            testCredentials(request.req, true)
+        });
+    });
+
+    function testCredentials(req, hasCredentials) {
+        var apiKeyAuth = new ApikeyAuth(req);
+        assert.equal(apiKeyAuth.hasCredentials(), hasCredentials);
+    }
+
+});
+
+suite('verify credentials', function() {
+
+    test('verifyCredentials callbacks with true value when request api_key is the same', function(done) {
+        testVerifyCredentials({query:{api_key: 'foo'}}, {apiKey: 'foo'}, true, done);
+    });
+
+    test('verifyCredentials callbacks with true value when request api_key is different', function(done) {
+        testVerifyCredentials({query:{api_key: 'foo'}}, {apiKey: 'bar'}, false, done);
+    });
+
+    function testVerifyCredentials(req, options, shouldBeValid, done) {
+        var apiKeyAuth = new ApikeyAuth(req);
+        apiKeyAuth.verifyCredentials(options, function(err, validCredentials) {
+            assert.equal(validCredentials, shouldBeValid);
+            done();
+        });
+    }
+
+});

test/unit/oauth.test.js

@@ -2,7 +2,8 @@ require('../helper');
 
 var _        = require('underscore')
     , redis  = require("redis")
-    , oAuth  = require('../../app/auth/oauth')
+    , OAuthAuth  = require('../../app/auth/oauth')
+    , oAuth  = require('../../app/auth/oauth').backend
     , assert = require('assert')
     , tests  = module.exports = {}
     , oauth_data_1 = {
@@ -107,4 +108,19 @@ test('returns null user for no oauth', function(done){
     });
 });
 
+test('OAuthAuth reports it has credentials', function(done) {
+    var req = {query:{}, headers:{authorization:real_oauth_header}};
+    var oAuthAuth = new OAuthAuth(req);
+    assert.ok(oAuthAuth.hasCredentials());
+    done();
+});
+
+test('OAuthAuth reports it has no credentials', function(done) {
+    var req = {query:{}, headers:{}};
+    var oAuthAuth = new OAuthAuth(req);
+    assert.equal(oAuthAuth.hasCredentials(), false);
+    done();
+});
+
+
 });