From f967a5ecdc4557ffeeead40fdc08b4b6fa4bb216 Mon Sep 17 00:00:00 2001
From: Nick O'Leary <nick.oleary@gmail.com>
Date: Sun, 29 Mar 2015 22:27:07 +0100
Subject: [PATCH] Fix auth on comms link and for anon user

The move to honour scope level of token broke the comms link
checking as well as the permissions checking for anon users.
---
 red/api/auth/index.js                 |  2 +-
 red/api/auth/permissions.js           |  2 +-
 red/api/auth/strategies.js            |  4 ++--
 red/comms.js                          | 12 ++++++++++--
 test/red/api/auth/permissions_spec.js |  1 +
 test/red/api/auth/strategies_spec.js  | 23 ++++++++++++++++++++---
 test/red/comms_spec.js                | 20 ++++++++++++++------
 7 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/red/api/auth/index.js b/red/api/auth/index.js
index 137ac6d49..6c0938468 100644
--- a/red/api/auth/index.js
+++ b/red/api/auth/index.js
@@ -101,7 +101,7 @@ module.exports = {
     errorHandler: function(err,req,res,next) {
         //TODO: standardize json response
         //TODO: audit log statment
-        console.log(err.stack);
+        //console.log(err.stack);
         //log.log({level:"audit",type:"auth",msg:err.toString()});
         return server.errorHandler()(err,req,res,next);
     },
diff --git a/red/api/auth/permissions.js b/red/api/auth/permissions.js
index e0398c7ba..132269a5a 100644
--- a/red/api/auth/permissions.js
+++ b/red/api/auth/permissions.js
@@ -38,7 +38,7 @@ function hasPermission(userScope,permission) {
     }
     
     if (util.isArray(permission)) {
-        for (var i=0;i<permission.length;i++) {
+        for (i=0;i<permission.length;i++) {
             if (!hasPermission(userScope,permission[i])) {
                 return false;
             }
diff --git a/red/api/auth/strategies.js b/red/api/auth/strategies.js
index e85c8f276..5f3e2ccdf 100644
--- a/red/api/auth/strategies.js
+++ b/red/api/auth/strategies.js
@@ -80,7 +80,7 @@ var passwordTokenExchange = function(client, username, password, scope, done) {
 
     Users.authenticate(username,password).then(function(user) {
         if (user) {
-            if (permissions.hasPermission(user,scope)) {
+            if (permissions.hasPermission(user.permissions,scope)) {
                 loginAttempts = loginAttempts.filter(function(logEntry) {
                     return logEntry.user !== username;
                 });
@@ -107,7 +107,7 @@ AnonymousStrategy.prototype.authenticate = function(req) {
     var self = this;
     Users.default().then(function(anon) {
         if (anon) {
-            self.success(anon);
+            self.success(anon,{scope:anon.permissions});
         } else {
             self.fail(401);
         }
diff --git a/red/comms.js b/red/comms.js
index 3460b7631..28aa9fde4 100644
--- a/red/comms.js
+++ b/red/comms.js
@@ -87,14 +87,22 @@ function start() {
                             Tokens.get(msg.auth).then(function(client) {
                                 if (client) {
                                     Users.get(client.user).then(function(user) {
-                                        completeConnection(client.scope,true);
+                                        if (user) {
+                                            completeConnection(client.scope,true);
+                                        } else {
+                                            completeConnection(null,false);
+                                        }
                                     });
                                 } else {
                                     completeConnection(null,false);
                                 }
                             });
                         } else {
-                            completeConnection(anonymousUser,false);
+                            if (anonymousUser) {
+                                completeConnection(anonymousUser.permissions,false);
+                            } else {
+                                completeConnection(null,false);
+                            }
                             //TODO: duplicated code - pull non-auth message handling out
                             if (msg.subscribe) {
                                 handleRemoteSubscription(ws,msg.subscribe);
diff --git a/test/red/api/auth/permissions_spec.js b/test/red/api/auth/permissions_spec.js
index 69a8f0293..72d5119b1 100644
--- a/test/red/api/auth/permissions_spec.js
+++ b/test/red/api/auth/permissions_spec.js
@@ -38,6 +38,7 @@ describe("Auth permissions", function() {
         it('an array of permissions', function() {
             permissions.hasPermission(["*"],["foo.read","foo.write"]).should.be.true;
             permissions.hasPermission("read",["foo.read","foo.write"]).should.be.false;
+            permissions.hasPermission("read",["foo.read","bar.read"]).should.be.true;
         });
     });
 });
diff --git a/test/red/api/auth/strategies_spec.js b/test/red/api/auth/strategies_spec.js
index 692dff65b..3e46db0c0 100644
--- a/test/red/api/auth/strategies_spec.js
+++ b/test/red/api/auth/strategies_spec.js
@@ -30,6 +30,7 @@ describe("Auth strategies", function() {
         afterEach(function() {
             if (userAuthentication) {
                 userAuthentication.restore();
+                userAuthentication = null;
             }
         });
 
@@ -48,10 +49,26 @@ describe("Auth strategies", function() {
                 }
             });
         });
+        
+        it('Handles scope overreach',function(done) {
+            userAuthentication = sinon.stub(Users,"authenticate",function(username,password) {
+                return when.resolve({username:"user",permissions:"read"});
+            });
+
+            strategies.passwordTokenExchange({},"user","password","*",function(err,token) {
+                try {
+                    should.not.exist(err);
+                    token.should.be.false;
+                    done();
+                } catch(e) {
+                    done(e);
+                }
+            });
+        });
 
         it('Creates new token on authentication success',function(done) {
             userAuthentication = sinon.stub(Users,"authenticate",function(username,password) {
-                return when.resolve({username:"user"});
+                return when.resolve({username:"user",permissions:"*"});
             });
             var tokenDetails = {};
             var tokenCreate = sinon.stub(Tokens,"create",function(username,client,scope) {
@@ -61,13 +78,13 @@ describe("Auth strategies", function() {
                 return when.resolve({accessToken: "123456"});
             });
 
-            strategies.passwordTokenExchange({id:"myclient"},"user","password","scope",function(err,token) {
+            strategies.passwordTokenExchange({id:"myclient"},"user","password","read",function(err,token) {
                 try {
                     should.not.exist(err);
                     token.should.equal("123456");
                     tokenDetails.should.have.property("username","user");
                     tokenDetails.should.have.property("client","myclient");
-                    tokenDetails.should.have.property("scope","scope");
+                    tokenDetails.should.have.property("scope","read");
                     done();
                 } catch(e) {
                     done(e);
diff --git a/test/red/comms_spec.js b/test/red/comms_spec.js
index 940d6587a..2195f34d4 100644
--- a/test/red/comms_spec.js
+++ b/test/red/comms_spec.js
@@ -344,9 +344,9 @@ describe("comms", function() {
             });
             getToken = sinon.stub(Tokens,"get",function(token) {
                 if (token == "1234") {
-                    return when.resolve({user:"fred"});
+                    return when.resolve({user:"fred",scope:["*"]});
                 } else if (token == "5678") {
-                    return when.resolve({user:"barney"});
+                    return when.resolve({user:"barney",scope:["*"]});
                 } else {
                     return when.resolve(null);
                 }
@@ -401,8 +401,12 @@ describe("comms", function() {
             });
             
             ws.on('close', function() {
-                received.should.equal(2);
-                done();
+                try {
+                    received.should.equal(2);
+                    done();
+                } catch(err) {
+                    done(err);
+                }
             });
         });
         
@@ -466,8 +470,12 @@ describe("comms", function() {
                 ws.close();
             });
             ws.on('close', function() {
-                count.should.equal(1);
-                done();
+                try {
+                    count.should.equal(1);
+                    done();
+                } catch(err) {
+                    done(err);
+                }
             });
         });
     });