diff --git a/defaults/config.js b/defaults/config.js index f26bb650..cf9dd9a8 100644 --- a/defaults/config.js +++ b/defaults/config.js @@ -464,7 +464,7 @@ module.exports = { // @type string // @default "uid" // - filter: "(objectClass=inetOrgPerson)(memberOf=ou=accounts,dc=example,dc=com)", + filter: "(objectClass=person)(memberOf=ou=accounts,dc=example,dc=com)", // // LDAP search base (search only within this node) diff --git a/src/plugins/auth/ldap.js b/src/plugins/auth/ldap.js index 8506800e..2e98c1c9 100644 --- a/src/plugins/auth/ldap.js +++ b/src/plugins/auth/ldap.js @@ -3,7 +3,7 @@ const Helper = require("../../helper"); const ldap = require("ldapjs"); -function ldapAuthCommon(manager, client, user, bindDN, password, callback) { +function ldapAuthCommon(user, bindDN, password, callback) { const config = Helper.config; const ldapclient = ldap.createClient({ @@ -17,15 +17,12 @@ function ldapAuthCommon(manager, client, user, bindDN, password, callback) { }); ldapclient.bind(bindDN, password, function(err) { - if (!err && !client) { - manager.addUser(user, null); - } ldapclient.unbind(); callback(!err); }); } -function simpleLdapAuth(manager, client, user, password, callback) { +function simpleLdapAuth(user, password, callback) { if (!user) { return callback(false); } @@ -37,13 +34,13 @@ function simpleLdapAuth(manager, client, user, password, callback) { log.info("Auth against LDAP ", config.ldap.url, " with provided bindDN ", bindDN); - ldapAuthCommon(manager, client, user, bindDN, password, callback); + ldapAuthCommon(user, bindDN, password, callback); } /** * LDAP auth using initial DN search (see config comment for ldap.searchDN) */ -function advancedLdapAuth(manager, client, user, password, callback) { +function advancedLdapAuth(user, password, callback) { if (!user) { return callback(false); } @@ -87,7 +84,7 @@ function advancedLdapAuth(manager, client, user, password, callback) { log.info("Auth against LDAP ", config.ldap.url, " with found bindDN ", bindDN); ldapclient.unbind(); - ldapAuthCommon(manager, client, user, bindDN, password, callback); + ldapAuthCommon(user, bindDN, password, callback); }); res.on("error", function(err3) { log.error("LDAP error: ", err3); @@ -105,13 +102,24 @@ function advancedLdapAuth(manager, client, user, password, callback) { } function ldapAuth(manager, client, user, password, callback) { - let auth = function() {}; + // TODO: Enable the use of starttls() as an alternative to ldaps + + // TODO: move this out of here and get rid of `manager` and `client` in + // auth plugin API + function callbackWrapper(valid) { + if (valid && !client) { + manager.addUser(user, null); + } + callback(valid); + } + + let auth; if ("baseDN" in Helper.config.ldap) { auth = simpleLdapAuth; } else { auth = advancedLdapAuth; } - return auth(manager, client, user, password, callback); + return auth(user, password, callbackWrapper); } function isLdapEnabled() { diff --git a/src/plugins/auth/local.js b/src/plugins/auth/local.js index e8e0ff38..3f929768 100644 --- a/src/plugins/auth/local.js +++ b/src/plugins/auth/local.js @@ -37,8 +37,6 @@ function localAuth(manager, client, user, password, callback) { module.exports = { auth: localAuth, - isEnabled: function() { - return true; - } + isEnabled: () => true }; diff --git a/src/server.js b/src/server.js index c4e2b4d8..47e60fd7 100644 --- a/src/server.js +++ b/src/server.js @@ -436,7 +436,7 @@ function performAuthentication(data) { } // Perform password checking - let auth = function() {}; + let auth; if (ldapAuth.isEnabled()) { auth = ldapAuth.auth; } else if (localAuth.isEnabled()) { diff --git a/test/plugins/auth/ldap.js b/test/plugins/auth/ldap.js new file mode 100644 index 00000000..56e4fef7 --- /dev/null +++ b/test/plugins/auth/ldap.js @@ -0,0 +1,151 @@ +"use strict"; + +const ldapAuth = require("../../../src/plugins/auth/ldap"); +const Helper = require("../../../src/helper"); +const ldap = require("ldapjs"); +const expect = require("chai").expect; + +const user = "johndoe"; +const wrongUser = "eve"; +const correctPassword = "loremipsum"; +const wrongPassword = "dolorsitamet"; +const baseDN = "ou=accounts,dc=example,dc=com"; +const primaryKey = "uid"; +const serverPort = 1389; + +function normalizeDN(dn) { + return ldap.parseDN(dn).toString(); +} + +function startLdapServer(callback) { + const server = ldap.createServer(); + + const searchConf = Helper.config.ldap.searchDN; + const userDN = primaryKey + "=" + user + "," + baseDN; + + // Two users are authorized: john doe and the root user in case of + // advanced auth (the user that does the search for john's actual + // bindDN) + const authorizedUsers = {}; + authorizedUsers[normalizeDN(searchConf.rootDN)] = searchConf.rootPassword; + authorizedUsers[normalizeDN(userDN)] = correctPassword; + + function authorize(req, res, next) { + const bindDN = req.connection.ldap.bindDN; + + if (bindDN in authorizedUsers) { + return next(); + } + + return next(new ldap.InsufficientAccessRightsError()); + } + + Object.keys(authorizedUsers).forEach(function(dn) { + server.bind(dn, function(req, res, next) { + const bindDN = req.dn.toString(); + const password = req.credentials; + + if (bindDN in authorizedUsers && authorizedUsers[bindDN] === password) { + req.connection.ldap.bindDN = req.dn; + res.end(); + return next(); + } + + return next(new ldap.InsufficientAccessRightsError()); + }); + }); + + server.search(searchConf.base, authorize, function(req, res) { + const obj = { + dn: userDN, + attributes: { + objectclass: ["person", "top"], + cn: ["john doe"], + sn: ["johnny"], + uid: ["johndoe"], + memberof: [baseDN] + } + }; + + if (req.filter.matches(obj.attributes)) { + // TODO: check req.scope if ldapjs does not + res.send(obj); + } + + res.end(); + }); + + server.listen(serverPort, function() { + console.log("LDAP server listening at %s", server.url); + callback(); + }); + + return server; +} + +function testLdapAuth() { + // Create mock manager and client. When client is true, manager should not + // be used. But ideally the auth plugin should not use any of those. + const manager = {}; + const client = true; + + it("should successfully authenticate with correct password", function(done) { + ldapAuth.auth(manager, client, user, correctPassword, function(valid) { + expect(valid).to.equal(true); + done(); + }); + }); + + it("should fail to authenticate with incorrect password", function(done) { + ldapAuth.auth(manager, client, user, wrongPassword, function(valid) { + expect(valid).to.equal(false); + done(); + }); + }); + + it("should fail to authenticate with incorrect username", function(done) { + ldapAuth.auth(manager, client, wrongUser, correctPassword, function(valid) { + expect(valid).to.equal(false); + done(); + }); + }); +} + +describe("LDAP authentication plugin", function() { + before(function(done) { + this.server = startLdapServer(done); + }); + after(function(done) { + this.server.close(); + done(); + }); + + beforeEach(function(done) { + Helper.config.public = false; + Helper.config.ldap.enable = true; + Helper.config.ldap.url = "ldap://localhost:" + String(serverPort); + Helper.config.ldap.primaryKey = primaryKey; + done(); + }); + + describe("LDAP authentication availability", function() { + it("checks that the configuration is correctly tied to isEnabled()", function(done) { + Helper.config.ldap.enable = true; + expect(ldapAuth.isEnabled()).to.equal(true); + Helper.config.ldap.enable = false; + expect(ldapAuth.isEnabled()).to.equal(false); + done(); + }); + }); + + describe("Simple LDAP authentication (predefined DN pattern)", function() { + Helper.config.ldap.baseDN = baseDN; + testLdapAuth(); + }); + + describe("Advanced LDAP authentication (DN found by a prior search query)", function() { + delete Helper.config.ldap.baseDN; + testLdapAuth(); + }); +}); +