From c31ffb98b099f269807732e3540541cded88ea0d Mon Sep 17 00:00:00 2001 From: Nick O'Leary Date: Thu, 5 Feb 2015 23:43:35 +0000 Subject: [PATCH] Tie auth middleware to needsPermission api --- package.json | 1 + red/api/auth/index.js | 28 +++++++++----- red/api/auth/permissions.js | 14 ------- red/api/auth/users.js | 33 +++-------------- red/api/index.js | 5 +-- red/api/library.js | 5 ++- red/red.js | 4 +- test/red/api/auth/index_spec.js | 53 --------------------------- test/red/api/auth/permissions_spec.js | 23 ------------ test/red/api/auth/users_spec.js | 3 +- test/red/api/library_spec.js | 2 + 11 files changed, 36 insertions(+), 135 deletions(-) diff --git a/package.json b/package.json index d74315260..3f35b9911 100644 --- a/package.json +++ b/package.json @@ -23,6 +23,7 @@ "dependencies": { "express": "3.17.2", "when": "3.4.6", + "bcryptjs": "2.1.0", "nopt": "3.0.1", "mqtt": "0.3.x", "ws": "0.4.32", diff --git a/red/api/auth/index.js b/red/api/auth/index.js index 0745493ff..2ed55528c 100644 --- a/red/api/auth/index.js +++ b/red/api/auth/index.js @@ -20,8 +20,9 @@ var oauth2orize = require("oauth2orize"); var strategies = require("./strategies"); var Tokens = require("./tokens"); var Users = require("./users"); +var permissions = require("./permissions"); -var settings = require("../../settings"); +var settings = null; var log = require("../../log"); @@ -33,22 +34,29 @@ var server = oauth2orize.createServer(); server.exchange(oauth2orize.exchange.password(strategies.passwordTokenExchange)); -function init() { +function init(_settings) { + settings = _settings; if (settings.adminAuth) { Users.init(settings.adminAuth); Tokens.init(settings) } } -function authenticate(req,res,next) { - if (settings.adminAuth) { - if (/^\/auth\/.*/.test(req.originalUrl)) { - next(); +function needsPermission(permission) { + return function(req,res,next) { + if (settings.adminAuth) { + return passport.authenticate(['bearer','anon'],{ session: false })(req,res,function() { + if (!req.user) { + return next(); + } + if (permissions.hasPermission(req.user,permission)) { + return next(); + } + return res.send(401); + }); } else { - return passport.authenticate(['bearer','anon'], { session: false })(req,res,next); + next(); } - } else { - next(); } } @@ -83,7 +91,7 @@ function revoke(req,res) { module.exports = { init: init, - authenticate: authenticate, + needsPermission: needsPermission, ensureClientSecret: ensureClientSecret, authenticateClient: authenticateClient, getToken: getToken, diff --git a/red/api/auth/permissions.js b/red/api/auth/permissions.js index 4933807d4..2183a29a0 100644 --- a/red/api/auth/permissions.js +++ b/red/api/auth/permissions.js @@ -19,18 +19,6 @@ var util = require('util'); var readRE = /^((.+)\.)?read$/ var writeRE = /^((.+)\.)?write$/ -function needsPermission(perm) { - return function(req,res,next) { - if (!req.user) { - return next(); - } - if (hasPermission(req.user,perm)) { - return next(); - } - return res.send(401); - } -} - function hasPermission(user,permission) { if (!user.permissions) { return false; @@ -45,6 +33,4 @@ function hasPermission(user,permission) { module.exports = { hasPermission: hasPermission, - needsPermission: needsPermission, - } diff --git a/red/api/auth/users.js b/red/api/auth/users.js index fbab6d2d8..15b05687a 100644 --- a/red/api/auth/users.js +++ b/red/api/auth/users.js @@ -15,30 +15,8 @@ **/ var when = require("when"); -var crypto = require("crypto"); var util = require("util"); -/* - adminAuth: { - type: "credentials", - users: [{ - username: "nol", - password: "5f4dcc3b5aa765d61d8327deb882cf99" // password - permissions: "* read write" - }], - default: { - permissions: "* read write" - } - }, - - adminAuth: { - type: "credentials", - users: function(username) {return when.resolve(user)}, - authenticate: function(username,password) { return when.resolve(user);} - default: function() { return when.resolve(defaultUser) } - } -*/ - -//{username:"nick",password:crypto.createHash('md5').update("foo",'utf8').digest('hex')} +var bcrypt = require('bcryptjs'); var users = {}; var passwords = {}; @@ -47,10 +25,11 @@ var defaultUser = null; function authenticate(username,password) { var user = users[username]; if (user) { - var pass = crypto.createHash('md5').update(password,'utf8').digest('hex'); - if (pass == passwords[username]) { - return when.resolve(user); - } + return when.promise(function(resolve,reject) { + bcrypt.compare(password, passwords[username], function(err, res) { + resolve(res?user:null); + }); + }); } return when.resolve(null); } diff --git a/red/api/index.js b/red/api/index.js index 13f819f76..aadf861a8 100644 --- a/red/api/index.js +++ b/red/api/index.js @@ -26,7 +26,7 @@ var library = require("./library"); var info = require("./info"); var auth = require("./auth"); -var needsPermission = require("./auth/permissions").needsPermission; +var needsPermission = auth.needsPermission; var settings = require("../settings"); @@ -38,7 +38,7 @@ var errorHandler = function(err,req,res,next) { function init(adminApp) { - auth.init(); + auth.init(settings); // Editor if (!settings.disableEditor) { @@ -55,7 +55,6 @@ function init(adminApp) { if (settings.adminAuth) { //TODO: all passport references ought to be in ./auth adminApp.use(passport.initialize()); - adminApp.use(auth.authenticate); adminApp.post("/auth/token", auth.ensureClientSecret, auth.authenticateClient, diff --git a/red/api/library.js b/red/api/library.js index a355bb54e..d704d726e 100644 --- a/red/api/library.js +++ b/red/api/library.js @@ -17,10 +17,11 @@ var redApp = null; var storage = require("../storage"); var log = require("../log"); +var needsPermission = require("./auth").needsPermission; function createLibrary(type) { if (redApp) { - redApp.get(new RegExp("/library/"+type+"($|\/(.*))"),function(req,res) { + redApp.get(new RegExp("/library/"+type+"($|\/(.*))"),needsPermission("library.read"),function(req,res) { var path = req.params[1]||""; storage.getLibraryEntry(type,path).then(function(result) { if (typeof result === "string") { @@ -42,7 +43,7 @@ function createLibrary(type) { }); }); - redApp.post(new RegExp("/library/"+type+"\/(.*)"),function(req,res) { + redApp.post(new RegExp("/library/"+type+"\/(.*)"),needsPermission("library.write"),function(req,res) { var path = req.params[0]; var fullBody = ''; req.on('data', function(chunk) { diff --git a/red/red.js b/red/red.js index 96f5f8351..84dcc3bf4 100644 --- a/red/red.js +++ b/red/red.js @@ -23,7 +23,7 @@ var util = require("./util"); var fs = require("fs"); var settings = require("./settings"); var credentials = require("./nodes/credentials"); -var permissions = require("./api/auth/permissions"); +var auth = require("./api/auth"); var path = require('path'); @@ -52,7 +52,7 @@ var RED = { settings:settings, util: util, auth: { - needsPermission: permissions.needsPermission + needsPermission: auth.needsPermission }, version: function () { var p = require(path.join(process.env.NODE_RED_HOME,"package.json")); diff --git a/test/red/api/auth/index_spec.js b/test/red/api/auth/index_spec.js index b1ef3e7b0..d773612b5 100644 --- a/test/red/api/auth/index_spec.js +++ b/test/red/api/auth/index_spec.js @@ -27,59 +27,6 @@ var settings = require("../../../../red/settings"); describe("api auth middleware",function() { - describe("authenticate",function() { - it("does not trigger on auth paths", sinon.test(function(done) { - this.stub(passport,"authenticate",function() { - return function() { - settings.reset(); - done(new Error("authentication not applied to auth path")); - } - }); - settings.init({adminAuth:{}}); - var req = { - originalUrl: "/auth/token" - }; - auth.authenticate(req,null,function() { - settings.reset(); - done(); - }); - - })); - it("does trigger on non-auth paths", sinon.test(function(done) { - this.stub(passport,"authenticate",function() { - return function() { - settings.reset(); - done(); - } - }); - settings.init({adminAuth:{}}); - var req = { - originalUrl: "/" - }; - auth.authenticate(req,null,function() { - settings.reset(); - done(new Error("authentication applied to non-auth path")); - }); - - })); - it("does not trigger on non-auth paths with auth disabled", sinon.test(function(done) { - this.stub(passport,"authenticate",function() { - return function() { - settings.reset(); - done(new Error("authentication applied when disabled")); - } - }); - settings.init({}); - var req = { - originalUrl: "/" - }; - auth.authenticate(req,null,function() { - settings.reset(); - done(); - }); - - })); - }); describe("ensureClientSecret", function() { it("leaves client_secret alone if not present",function(done) { diff --git a/test/red/api/auth/permissions_spec.js b/test/red/api/auth/permissions_spec.js index ebb82b4f2..eb1c27ec1 100644 --- a/test/red/api/auth/permissions_spec.js +++ b/test/red/api/auth/permissions_spec.js @@ -35,27 +35,4 @@ describe("Auth permissions", function() { permissions.hasPermission({permissions:"read"},"node.write").should.be.false; }); }); - - describe("needsPermission middleware", function() { - it('passes if no user on request',function(done) { - var needsPermission = permissions.needsPermission("*"); - needsPermission({},null,function() { - done(); - }); - }); - it('passes if user has required permission',function(done) { - var needsPermission = permissions.needsPermission("read"); - needsPermission({user:{permissions:"read"}},null,function() { - done(); - }); - }); - it('rejects if user does not have required permission',function(done) { - var needsPermission = permissions.needsPermission("write"); - needsPermission({user:{permissions:"read"}},{send: function(code) { - code.should.equal(401); - done(); - }},null); - }); - - }); }); diff --git a/test/red/api/auth/users_spec.js b/test/red/api/auth/users_spec.js index c2aad2f12..8bc895b6d 100644 --- a/test/red/api/auth/users_spec.js +++ b/test/red/api/auth/users_spec.js @@ -27,7 +27,8 @@ describe("Users", function() { type:"credentials", users:[{ username:"fred", - password:"5f4dcc3b5aa765d61d8327deb882cf99", // 'password' + password:'$2a$08$LpYMefvGZ3MjAfZGzcoyR.1BcfHh4wy4NpbN.cEny5aHnWOqjKOXK', + // 'password' -> require('bcryptjs').hashSync('password', 8); permissions:"*" }] }); diff --git a/test/red/api/library_spec.js b/test/red/api/library_spec.js index 2f321724c..66b2693dd 100644 --- a/test/red/api/library_spec.js +++ b/test/red/api/library_spec.js @@ -24,6 +24,7 @@ var app = express(); var RED = require("../../../red/red.js"); var storage = require("../../../red/storage"); var library = require("../../../red/api/library"); +var auth = require("../../../red/api/auth"); describe("library api", function() { @@ -166,6 +167,7 @@ describe("library api", function() { app = express(); app.use(express.json()); library.init(app); + auth.init({}); RED.library.register("test"); });