Implement fallback mechanism to be able to authenticate as usual in case of apikey is not found

app/services/user_database_service.js

@@ -3,7 +3,7 @@
 var step = require('step');
 var _ = require('underscore');
 
-function isValidApiKey(apikey) {
+function isApiKeyFound(apikey) {
     return apikey.type !== null &&
         apikey.user !== null &&
         apikey.databasePassword !== null &&
@@ -27,7 +27,6 @@ function UserDatabaseService(metadataBackend) {
 UserDatabaseService.prototype.getConnectionParams = function (authApi, cdbUsername, callback) {
     var self = this;
 
-    var dbParams;
     var dbopts = {
         port: global.settings.db_port,
         pass: global.settings.db_pubuser_pass
@@ -40,9 +39,7 @@ UserDatabaseService.prototype.getConnectionParams = function (authApi, cdbUserna
         function getDatabaseConnectionParams() {
             self.metadataBackend.getAllUserDBParams(cdbUsername, this);
         },
-        function authenticate(err, userDBParams) {
+        function getApiKey (err, dbParams) {
-            var next = this;
-
             if (err) {
                 err.http_status = 404;
                 err.message = "Sorry, we can't find CartoDB user '" + cdbUsername + "'. " +
@@ -50,18 +47,93 @@ UserDatabaseService.prototype.getConnectionParams = function (authApi, cdbUserna
                 return callback(err);
             }
 
-            dbParams = userDBParams;
+            const next = this;
+
+            if (authApi.getType() !== 'apiKey') {
+                return next(null, dbopts, dbParams);
+            }
+
+            const apikeyToken = authApi.getCredentials();
+
+            self.metadataBackend.getApikey(cdbUsername, apikeyToken, (err, apikey) => {
+                if (err) {
+                    return next(err);
+                }
+
+                if (!isApiKeyFound(apikey)) {
+                    return next(null, dbopts, dbParams);
+                }
+
+                if (!apikey.grantsSql) {
+                    const forbiddenError = new Error('forbidden');
+                    forbiddenError.http_status = 403;
+
+                    return next(forbiddenError);
+                }
+
+                dbParams.apikey = apikeyToken;
+
+                next(null, dbopts, dbParams, apikey);
+            });
+        },
+        function authenticate(err, dbopts, dbParams, apikey) {
+            var next = this;
+
+            if (err) {
+                return next(err);
+            }
 
             dbopts.host = dbParams.dbhost;
             dbopts.dbname = dbParams.dbname;
             dbopts.user = (!!dbParams.dbpublicuser) ? dbParams.dbpublicuser : global.settings.db_pubuser;
 
-            authApi.verifyCredentials({
+            const opts = {
                 metadataBackend: self.metadataBackend,
                 apiKey: dbParams.apikey
-            }, next);
+            };
+
+
+            authApi.verifyCredentials(opts, function (err, isAuthenticated) {
+                if (err) {
+                    return next(err);
+                }
+
+                next(null, isAuthenticated, dbopts, dbParams, apikey);
+            });
         },
-        function getUserLimits (err, isAuthenticated) {
+        function setDBAuth(err, isAuthenticated, dbopts, dbParams, apikey) {
+            const next = this;
+
+            if (err) {
+                return next(err);
+            }
+
+            var user = _.template(global.settings.db_user, {user_id: dbParams.dbuser});
+            var pass = null;
+
+            if (global.settings.hasOwnProperty('db_user_pass')) {
+                pass = _.template(global.settings.db_user_pass, {
+                    user_id: dbParams.dbuser,
+                    user_password: dbParams.dbpass
+                });
+            }
+
+            if (isAuthenticated) {
+                dbopts.authenticated = isAuthenticated;
+                if (apikey) {
+                    dbopts.user = apikey.databaseRole;
+                    dbopts.pass = apikey.databasePassword;
+                } else {
+                    dbopts.user = user;
+                    dbopts.pass = pass;
+                }
+            }
+
+            var authDbOpts = _.defaults({ user: user, pass: pass }, dbopts);
+
+            return next(null, isAuthenticated, dbopts, authDbOpts);
+        },
+        function getUserLimits (err, isAuthenticated, dbopts, authDbOpts) {
             var next = this;
 
             if (err) {
@@ -77,66 +149,6 @@ UserDatabaseService.prototype.getConnectionParams = function (authApi, cdbUserna
                     timeout: isAuthenticated ? timeoutRenderLimit.render : timeoutRenderLimit.renderPublic
                 };
 
-                next(null, isAuthenticated, userLimits);
-            });
-        },
-        function setDBAuth(err, isAuthenticated, userLimits) {
-            if (err) {
-                throw err;
-            }
-
-            var user = _.template(global.settings.db_user, {user_id: dbParams.dbuser});
-            var pass = null;
-            if (global.settings.hasOwnProperty('db_user_pass')) {
-                pass = _.template(global.settings.db_user_pass, {
-                    user_id: dbParams.dbuser,
-                    user_password: dbParams.dbpass
-                });
-            }
-
-            if (_.isBoolean(isAuthenticated) && isAuthenticated) {
-                dbopts.authenticated = isAuthenticated;
-                dbopts.user = user;
-                dbopts.pass = pass;
-            }
-
-            var authDbOpts = _.defaults({user: user, pass: pass}, dbopts);
-
-            return this(null, dbopts, authDbOpts, userLimits);
-        },
-        function getApiKey (err, dbopts, authDbOpts, userLimits) {
-            if (err) {
-                throw err;
-            }
-            const next = this;
-
-            if (authApi.getType() !== 'apiKey') {
-                return next(null, dbopts, authDbOpts, userLimits);
-            }
-
-            self.metadataBackend.getApikey(cdbUsername, authApi.getCredentials(), (err, apiKey) => {
-                if (err) {
-                    return next(err);
-                }
-
-                if (!isValidApiKey(apiKey)) {
-                    const unauthorizedError = new Error('permission denied');
-                    unauthorizedError.http_status = 401;
-
-                    return next(unauthorizedError);
-                }
-
-                if (!apiKey.grantsSql) {
-                    const forbiddenError = new Error('forbidden');
-                    forbiddenError.http_status = 403;
-
-                    return next(forbiddenError);
-                }
-
-                if (apiKey.type !== 'default') {
-                    dbopts = _.extend(dbopts, { user: apiKey.databaseRole, pass: apiKey.databasePassword });
-                }
-
                 next(null, dbopts, authDbOpts, userLimits);
             });
         },

test/acceptance/auth-api.js

@@ -1,6 +1,7 @@
 const assert = require('../support/assert');
 const TestClient = require('../support/test-client');
 const BatchTestClient = require('../support/batch-test-client');
+const JobStatus = require('../../batch/job_status');
 
 describe('Auth API', function () {
     const publicSQL = 'select * from untitle_table_4';
@@ -16,7 +17,7 @@ describe('Auth API', function () {
         });
     });
 
-    it.only('should fail while fetching data (private dataset) and using the default API key', function (done) {
+    it('should fail while fetching data (private dataset) and using the default API key', function (done) {
         this.testClient = new TestClient();
         const expectedResponse = {
             response: {
@@ -59,7 +60,7 @@ describe('Auth API', function () {
         });
     });
 
-    it('should fail while fetching data (scoped dataset) and using the default API key', function (done) {
+    it('should fail while fetching data (scoped dataset) and using regular API key', function (done) {
         this.testClient = new TestClient({ apiKey: 'regular2' });
         const expectedResponse = {
             response: {
@@ -74,21 +75,56 @@ describe('Auth API', function () {
         });
     });
 
-    it('should fail while creating a job with regular api key', function (done) {
+    describe('Fallback', function () {
-        this.testClient = new BatchTestClient({ apiKey: 'regular1' });
+        it('should get result from query using master apikey (fallback) and a granted dataset', function (done) {
-        const expectedResponse = {
+            this.testClient = new TestClient({ apiKey: '4321', host: 'cartofante.cartodb.com' });
-            response: {
+            this.testClient.getResult(scopedSQL, (err, result) => {
-                status: 401
+                assert.ifError(err);
-            }
+                assert.equal(result.length, 4);
-        };
+                done();
+            });
+        });
 
-        this.testClient.createJob({ query: scopedSQL }, expectedResponse, (err, jobResult) => {
+        it('should fail while getting result from query using metadata and scoped dataset', function (done) {
-            if (err) {
+            this.testClient = new TestClient({ host: 'cartofante.cartodb.com' });
-                return done(err);
-            }
 
-            assert.deepEqual(jobResult.job.error, [ 'permission denied' ]);
+            const expectedResponse = {
+                response: {
+                    status: 401
+                },
+                anonymous: true
+            };
 
+            this.testClient.getResult(privateSQL, expectedResponse, (err, result) => {
+                assert.ifError(err);
+                assert.equal(result.error, 'permission denied for relation private_table');
+                done();
+            });
+        });
+    });
+
+    describe('Batch API', function () {
+        it('should create while creating a job with regular api key', function (done) {
+            this.testClient = new BatchTestClient({ apiKey: 'regular1' });
+
+            this.testClient.createJob({ query: scopedSQL }, (err, jobResult) => {
+                if (err) {
+                    return done(err);
+                }
+
+                jobResult.getStatus(function (err, job) {
+                    if (err) {
+                        return done(err);
+                    }
+
+                    assert.equal(job.status, JobStatus.DONE);
+
+                    done();
+                });
+            });
+        });
+
+        afterEach(function (done) {
             this.testClient.drain(done);
         });
     });

test/prepare_db.sh

@@ -155,6 +155,16 @@ HMSET rails:oauth_access_tokens:l0lPbtP68ao8NfStCiA3V3neqfM03JKhToxhUQTR \
  time sometime
 EOF
 
+  cat <<EOF | redis-cli -h ${REDIS_HOST} -p ${REDIS_PORT} -n 5
+HMSET rails:users:cartofante \
+ id 2 \
+ database_name ${TEST_DB} \
+ database_host ${PGHOST} \
+ database_password test_cartodb_user_2_pass \
+ map_key 4321
+SADD rails:users:fallback_1:map_key 4321
+EOF
+
 # delete previous jobs
 cat <<EOF | redis-cli -h ${REDIS_HOST} -p ${REDIS_PORT} -n 5
 EVAL "return redis.call('del', unpack(redis.call('keys', ARGV[1])))" 0 batch:jobs:*

test/support/sql/test.sql

@@ -174,6 +174,11 @@ DROP USER IF EXISTS regular_2;
 CREATE USER regular_2 WITH PASSWORD 'regular2';
 ALTER ROLE regular_2 SET statement_timeout = 2000;
 
+-- fallback user role
+DROP USER IF EXISTS test_cartodb_user_2;
+CREATE USER test_cartodb_user_2 WITH PASSWORD 'test_cartodb_user_2_pass';
+GRANT ALL ON TABLE scoped_table_1 TO test_cartodb_user_2;
+
 -- db owner role
 DROP USER IF EXISTS :TESTUSER;
 CREATE USER :TESTUSER WITH PASSWORD ':TESTPASS';
@@ -210,3 +215,4 @@ INSERT INTO CDB_TableMetadata (tabname, updated_at) VALUES ('private_table'::reg
 INSERT INTO CDB_TableMetadata (tabname, updated_at) VALUES ('scoped_table_1'::regclass, '2015-01-01T23:31:30.123Z');
 
 GRANT SELECT ON CDB_TableMetadata TO :TESTUSER;
+GRANT SELECT ON CDB_TableMetadata TO test_cartodb_user_2;