Add tests for LDAP auth plugin

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)

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() {

src/plugins/auth/local.js

@@ -37,8 +37,6 @@ function localAuth(manager, client, user, password, callback) {
 
 module.exports = {
 	auth: localAuth,
-	isEnabled: function() {
+	isEnabled: () => true
-		return true;
-	}
 };
 

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()) {

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();
+	});
+});
+