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.
This commit is contained in:
Nick O'Leary 2015-03-29 22:27:07 +01:00
parent c8d6dc2531
commit f967a5ecdc
7 changed files with 49 additions and 15 deletions

View File

@ -101,7 +101,7 @@ module.exports = {
errorHandler: function(err,req,res,next) { errorHandler: function(err,req,res,next) {
//TODO: standardize json response //TODO: standardize json response
//TODO: audit log statment //TODO: audit log statment
console.log(err.stack); //console.log(err.stack);
//log.log({level:"audit",type:"auth",msg:err.toString()}); //log.log({level:"audit",type:"auth",msg:err.toString()});
return server.errorHandler()(err,req,res,next); return server.errorHandler()(err,req,res,next);
}, },

View File

@ -38,7 +38,7 @@ function hasPermission(userScope,permission) {
} }
if (util.isArray(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])) { if (!hasPermission(userScope,permission[i])) {
return false; return false;
} }

View File

@ -80,7 +80,7 @@ var passwordTokenExchange = function(client, username, password, scope, done) {
Users.authenticate(username,password).then(function(user) { Users.authenticate(username,password).then(function(user) {
if (user) { if (user) {
if (permissions.hasPermission(user,scope)) { if (permissions.hasPermission(user.permissions,scope)) {
loginAttempts = loginAttempts.filter(function(logEntry) { loginAttempts = loginAttempts.filter(function(logEntry) {
return logEntry.user !== username; return logEntry.user !== username;
}); });
@ -107,7 +107,7 @@ AnonymousStrategy.prototype.authenticate = function(req) {
var self = this; var self = this;
Users.default().then(function(anon) { Users.default().then(function(anon) {
if (anon) { if (anon) {
self.success(anon); self.success(anon,{scope:anon.permissions});
} else { } else {
self.fail(401); self.fail(401);
} }

View File

@ -87,14 +87,22 @@ function start() {
Tokens.get(msg.auth).then(function(client) { Tokens.get(msg.auth).then(function(client) {
if (client) { if (client) {
Users.get(client.user).then(function(user) { Users.get(client.user).then(function(user) {
completeConnection(client.scope,true); if (user) {
completeConnection(client.scope,true);
} else {
completeConnection(null,false);
}
}); });
} else { } else {
completeConnection(null,false); completeConnection(null,false);
} }
}); });
} else { } else {
completeConnection(anonymousUser,false); if (anonymousUser) {
completeConnection(anonymousUser.permissions,false);
} else {
completeConnection(null,false);
}
//TODO: duplicated code - pull non-auth message handling out //TODO: duplicated code - pull non-auth message handling out
if (msg.subscribe) { if (msg.subscribe) {
handleRemoteSubscription(ws,msg.subscribe); handleRemoteSubscription(ws,msg.subscribe);

View File

@ -38,6 +38,7 @@ describe("Auth permissions", function() {
it('an array of permissions', function() { it('an array of permissions', function() {
permissions.hasPermission(["*"],["foo.read","foo.write"]).should.be.true; permissions.hasPermission(["*"],["foo.read","foo.write"]).should.be.true;
permissions.hasPermission("read",["foo.read","foo.write"]).should.be.false; permissions.hasPermission("read",["foo.read","foo.write"]).should.be.false;
permissions.hasPermission("read",["foo.read","bar.read"]).should.be.true;
}); });
}); });
}); });

View File

@ -30,6 +30,7 @@ describe("Auth strategies", function() {
afterEach(function() { afterEach(function() {
if (userAuthentication) { if (userAuthentication) {
userAuthentication.restore(); 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) { it('Creates new token on authentication success',function(done) {
userAuthentication = sinon.stub(Users,"authenticate",function(username,password) { userAuthentication = sinon.stub(Users,"authenticate",function(username,password) {
return when.resolve({username:"user"}); return when.resolve({username:"user",permissions:"*"});
}); });
var tokenDetails = {}; var tokenDetails = {};
var tokenCreate = sinon.stub(Tokens,"create",function(username,client,scope) { var tokenCreate = sinon.stub(Tokens,"create",function(username,client,scope) {
@ -61,13 +78,13 @@ describe("Auth strategies", function() {
return when.resolve({accessToken: "123456"}); 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 { try {
should.not.exist(err); should.not.exist(err);
token.should.equal("123456"); token.should.equal("123456");
tokenDetails.should.have.property("username","user"); tokenDetails.should.have.property("username","user");
tokenDetails.should.have.property("client","myclient"); tokenDetails.should.have.property("client","myclient");
tokenDetails.should.have.property("scope","scope"); tokenDetails.should.have.property("scope","read");
done(); done();
} catch(e) { } catch(e) {
done(e); done(e);

View File

@ -344,9 +344,9 @@ describe("comms", function() {
}); });
getToken = sinon.stub(Tokens,"get",function(token) { getToken = sinon.stub(Tokens,"get",function(token) {
if (token == "1234") { if (token == "1234") {
return when.resolve({user:"fred"}); return when.resolve({user:"fred",scope:["*"]});
} else if (token == "5678") { } else if (token == "5678") {
return when.resolve({user:"barney"}); return when.resolve({user:"barney",scope:["*"]});
} else { } else {
return when.resolve(null); return when.resolve(null);
} }
@ -401,8 +401,12 @@ describe("comms", function() {
}); });
ws.on('close', function() { ws.on('close', function() {
received.should.equal(2); try {
done(); received.should.equal(2);
done();
} catch(err) {
done(err);
}
}); });
}); });
@ -466,8 +470,12 @@ describe("comms", function() {
ws.close(); ws.close();
}); });
ws.on('close', function() { ws.on('close', function() {
count.should.equal(1); try {
done(); count.should.equal(1);
done();
} catch(err) {
done(err);
}
}); });
}); });
}); });