From 001f96035b37bf0369eb047e034e79e6e49ff41e Mon Sep 17 00:00:00 2001 From: S Date: Thu, 23 Mar 2017 08:47:51 +0100 Subject: [PATCH] Switch to bcryptjs and make password comparison async - PasswordCompareAsync prevents timeouts on resource constraint devices - All password.compare calls are now async - Updated tests to accept async functions --- package.json | 2 +- src/helper.js | 4 +-- src/server.js | 69 +++++++++++++++++++++++------------------ test/tests/passwords.js | 17 ++++++++-- 4 files changed, 57 insertions(+), 35 deletions(-) diff --git a/package.json b/package.json index 5615941c..63f68dce 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "node": ">=4.2.0" }, "dependencies": { - "bcrypt-nodejs": "0.0.3", + "bcryptjs": "2.4.3", "cheerio": "0.22.0", "colors": "1.1.2", "commander": "2.9.0", diff --git a/src/helper.js b/src/helper.js index c6e971c1..830b91c9 100644 --- a/src/helper.js +++ b/src/helper.js @@ -6,7 +6,7 @@ var path = require("path"); var os = require("os"); var fs = require("fs"); var net = require("net"); -var bcrypt = require("bcrypt-nodejs"); +var bcrypt = require("bcryptjs"); var Helper = { config: null, @@ -125,5 +125,5 @@ function passwordHash(password) { } function passwordCompare(password, expected) { - return bcrypt.compareSync(password, expected); + return bcrypt.compare(password, expected); } diff --git a/src/server.js b/src/server.js index 636b5ab6..28b35436 100644 --- a/src/server.js +++ b/src/server.js @@ -192,27 +192,33 @@ function init(socket, client) { }); return; } - if (!Helper.password.compare(old || "", client.config.password)) { - socket.emit("change-password", { - error: "The current password field does not match your account password" + + Helper.password + .compare(old || "", client.config.password) + .then(matching => { + if (!matching) { + socket.emit("change-password", { + error: "The current password field does not match your account password" + }); + return; + } + const hash = Helper.password.hash(p1); + + client.setPassword(hash, success => { + const obj = {}; + + if (success) { + obj.success = "Successfully updated your password, all your other sessions were logged out"; + obj.token = client.config.token; + } else { + obj.error = "Failed to update your password"; + } + + socket.emit("change-password", obj); + }); + }).catch(error => { + log.error(`Error while checking users password. Error: ${error}`); }); - return; - } - - var hash = Helper.password.hash(p1); - - client.setPassword(hash, function(success) { - var obj = {}; - - if (success) { - obj.success = "Successfully updated your password, all your other sessions were logged out"; - obj.token = client.config.token; - } else { - obj.error = "Failed to update your password"; - } - - socket.emit("change-password", obj); - }); } ); } @@ -267,19 +273,22 @@ function localAuth(client, user, password, callback) { return callback(false); } - var result = Helper.password.compare(password, client.config.password); + Helper.password + .compare(password, client.config.password) + .then(matching => { + if (Helper.password.requiresUpdate(client.config.password)) { + const hash = Helper.password.hash(password); - if (result && Helper.password.requiresUpdate(client.config.password)) { - var hash = Helper.password.hash(password); - - client.setPassword(hash, function(success) { - if (success) { - log.info(`User ${colors.bold(client.name)} logged in and their hashed password has been updated to match new security requirements`); + client.setPassword(hash, success => { + if (success) { + log.info(`User ${colors.bold(client.name)} logged in and their hashed password has been updated to match new security requirements`); + } + }); } + callback(matching); + }).catch(error => { + log.error(`Error while checking users password. Error: ${error}`); }); - } - - return callback(result); } function ldapAuth(client, user, password, callback) { diff --git a/test/tests/passwords.js b/test/tests/passwords.js index d074477d..48d45c9d 100644 --- a/test/tests/passwords.js +++ b/test/tests/passwords.js @@ -10,14 +10,27 @@ describe("Client passwords", function() { // Generated with third party tool to test implementation let comparedPassword = Helper.password.compare(inputPassword, "$2a$11$zrPPcfZ091WNfs6QrRHtQeUitlgrJcecfZhxOFiQs0FWw7TN3Q1oS"); - expect(comparedPassword).to.be.true; + return comparedPassword.then(result => { + expect(result).to.be.true; + }); + }); + + it("wrong hashed password should not match", function() { + // Compare against a fake hash + let comparedPassword = Helper.password.compare(inputPassword, "$2a$11$zrPPcfZ091WRONGPASSWORDitlgrJcecfZhxOFiQs0FWw7TN3Q1oS"); + + return comparedPassword.then(result => { + expect(result).to.be.false; + }); }); it("freshly hashed password should match", function() { let hashedPassword = Helper.password.hash(inputPassword); let comparedPassword = Helper.password.compare(inputPassword, hashedPassword); - expect(comparedPassword).to.be.true; + return comparedPassword.then((result) => { + expect(result).to.be.true; + }); }); it("shout passwords should be marked as old", function() {