From c7e4f76b9c4ddfe01d7ea302a0e86535d06894ed Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Thu, 16 Mar 2017 01:25:55 +0100 Subject: [PATCH] Add an LDAP user search filter in the configuration filte to specify the user attribute to search for in LDAP --- config.template.yml | 4 +- example/ldap/base.ldif | 15 +++++++ src/index.js | 3 +- src/lib/ldap.js | 28 ++++++++---- src/lib/routes/first_factor.js | 8 ++-- src/lib/routes/reset_password.js | 3 +- test/unitary/routes/test_first_factor.js | 20 ++++++++- test/unitary/routes/test_reset_password.js | 12 ++++- test/unitary/test_data_persistence.js | 2 +- test/unitary/test_ldap.js | 52 ++++++++++++++++++++-- test/unitary/test_server.js | 3 +- test/unitary/test_server_config.js | 14 ++++-- 12 files changed, 138 insertions(+), 26 deletions(-) diff --git a/config.template.yml b/config.template.yml index bba35f996..e1ab52449 100644 --- a/config.template.yml +++ b/config.template.yml @@ -3,9 +3,11 @@ logs_level: info # Configuration of LDAP +# Example: for user john, the DN will be cn=john,ou=users,dc=example,dc=com ldap: url: ldap://ldap - base_dn: ou=users,dc=example,dc=com + user_search_base: ou=users,dc=example,dc=com + user_search_filter: cn user: cn=admin,dc=example,dc=com password: password diff --git a/example/ldap/base.ldif b/example/ldap/base.ldif index ba1f727d9..1e46c0ef7 100644 --- a/example/ldap/base.ldif +++ b/example/ldap/base.ldif @@ -29,3 +29,18 @@ uid: user uidnumber: 1000 userpassword: {SHA}W6ph5Mm5Pz8GgiULbPgzG37mj9g= +dn: uid=useruid,ou=users,dc=example,dc=com +cn: useruid +gidnumber: 500 +givenname: user +homedirectory: /home/user1 +loginshell: /bin/sh +objectclass: inetOrgPerson +objectclass: posixAccount +objectclass: top +mail: useruid@example.com +sn: User +uid: useruid +uidnumber: 1001 +userpassword: {SHA}W6ph5Mm5Pz8GgiULbPgzG37mj9g= + diff --git a/src/index.js b/src/index.js index 2a0a00a17..0719b0ab1 100755 --- a/src/index.js +++ b/src/index.js @@ -25,7 +25,8 @@ var yaml_config = YAML.load(config_path); var config = { port: process.env.PORT || 8080, ldap_url: yaml_config.ldap.url || 'ldap://127.0.0.1:389', - ldap_users_dn: yaml_config.ldap.base_dn, + ldap_user_search_base: yaml_config.ldap.user_search_base, + ldap_user_search_filter: yaml_config.ldap.user_search_filter, ldap_user: yaml_config.ldap.user, ldap_password: yaml_config.ldap.password, session_domain: yaml_config.session.domain, diff --git a/src/lib/ldap.js b/src/lib/ldap.js index 778ffe49a..01c5289e1 100644 --- a/src/lib/ldap.js +++ b/src/lib/ldap.js @@ -1,6 +1,6 @@ module.exports = { - validate: validateCredentials, + validate: validate_credentials, get_email: retrieve_email, update_password: update_password } @@ -10,8 +10,12 @@ var Promise = require('bluebird'); var exceptions = require('./exceptions'); var Dovehash = require('dovehash'); -function validateCredentials(ldap_client, username, password, users_dn) { - var userDN = util.format("cn=%s,%s", username, users_dn); +function validate_credentials(ldap_client, username, password, user_base, user_filter) { + // if not provided, default to cn + if(!user_filter) user_filter = 'cn'; + + var userDN = util.format("%s=%s,%s", user_filter, username, user_base); + console.log(userDN); var bind_promised = Promise.promisify(ldap_client.bind, { context: ldap_client }); return bind_promised(userDN, password) .error(function(err) { @@ -20,16 +24,19 @@ function validateCredentials(ldap_client, username, password, users_dn) { }); } -function retrieve_email(ldap_client, username, users_dn) { - var userDN = util.format("cn=%s,%s", username, users_dn); +function retrieve_email(ldap_client, username, user_base, user_filter) { + // if not provided, default to cn + if(!user_filter) user_filter = 'cn'; + + var userDN = util.format("%s=%s,%s", user_filter, username, user_base); + console.log(userDN); var search_promised = Promise.promisify(ldap_client.search, { context: ldap_client }); var query = {}; query.sizeLimit = 1; query.attributes = ['mail']; - var base_dn = userDN; return new Promise(function(resolve, reject) { - search_promised(base_dn, query) + search_promised(userDN, query) .then(function(res) { var doc; res.on('searchEntry', function(entry) { @@ -49,7 +56,12 @@ function retrieve_email(ldap_client, username, users_dn) { } function update_password(ldap_client, ldap, username, new_password, config) { - var userDN = util.format("cn=%s,%s", username, config.ldap_users_dn); + var user_filter = config.ldap_user_search_filter; + // if not provided, default to cn + if(!user_filter) user_filter = 'cn'; + + var userDN = util.format("%s=%s,%s", user_filter, username, + config.ldap_user_search_base); var encoded_password = Dovehash.encode('SSHA', new_password); var change = new ldap.Change({ operation: 'replace', diff --git a/src/lib/routes/first_factor.js b/src/lib/routes/first_factor.js index fa5a3c1b1..3b9c824fe 100644 --- a/src/lib/routes/first_factor.js +++ b/src/lib/routes/first_factor.js @@ -23,18 +23,20 @@ function first_factor(req, res) { logger.debug('1st factor: Start bind operation against LDAP'); logger.debug('1st factor: username=%s', username); - logger.debug('1st factor: base_dn=%s', config.ldap_users_dn); + logger.debug('1st factor: base_dn=%s', config.ldap_user_search_base); + logger.debug('1st factor: user_filter=%s', config.ldap_user_search_filter); regulator.regulate(username) .then(function() { - return ldap.validate(ldap_client, username, password, config.ldap_users_dn); + return ldap.validate(ldap_client, username, password, config.ldap_user_search_base, config.ldap_user_search_filter); }) .then(function() { objectPath.set(req, 'session.auth_session.userid', username); objectPath.set(req, 'session.auth_session.first_factor', true); logger.info('1st factor: LDAP binding successful'); logger.debug('1st factor: Retrieve email from LDAP'); - return ldap.get_email(ldap_client, username, config.ldap_users_dn) + return ldap.get_email(ldap_client, username, config.ldap_user_search_base, + config.ldap_user_search_filter) }) .then(function(doc) { var email = objectPath.get(doc, 'mail'); diff --git a/src/lib/routes/reset_password.js b/src/lib/routes/reset_password.js index 006dfa1f7..8b46cf159 100644 --- a/src/lib/routes/reset_password.js +++ b/src/lib/routes/reset_password.js @@ -27,7 +27,8 @@ function pre_check(req) { var ldap_client = req.app.get('ldap client'); var config = req.app.get('config'); - return ldap.get_email(ldap_client, userid, config.ldap_users_dn) + return ldap.get_email(ldap_client, userid, config.ldap_user_search_base, + config.ldap_user_search_filter) .then(function(doc) { var email = objectPath.get(doc, 'mail'); diff --git a/test/unitary/routes/test_first_factor.js b/test/unitary/routes/test_first_factor.js index aabd085fc..548b6dd73 100644 --- a/test/unitary/routes/test_first_factor.js +++ b/test/unitary/routes/test_first_factor.js @@ -18,7 +18,8 @@ describe('test the first factor validation route', function() { search: sinon.stub() } var config = { - ldap_users_dn: 'dc=example,dc=com' + ldap_user_search_base: 'ou=users,dc=example,dc=com', + ldap_user_search_filter: 'uid' } var search_doc = { @@ -42,7 +43,7 @@ describe('test the first factor validation route', function() { var app_get = sinon.stub(); app_get.withArgs('ldap client').returns(ldap_interface_mock); - app_get.withArgs('config').returns(ldap_interface_mock); + app_get.withArgs('config').returns(config); app_get.withArgs('logger').returns(winston); app_get.withArgs('authentication regulator').returns(regulator); @@ -79,6 +80,21 @@ describe('test the first factor validation route', function() { }); }); + it('should bind user based on LDAP DN', function(done) { + ldap_interface_mock.bind = sinon.spy(function(dn) { + if(dn == 'uid=username,ou=users,dc=example,dc=com') done(); + }); + first_factor(req, res); + }); + + it('should retrieve email from LDAP', function(done) { + ldap_interface_mock.bind.yields(undefined); + ldap_interface_mock.search = sinon.spy(function(dn) { + if(dn == 'uid=username,ou=users,dc=example,dc=com') done(); + }); + first_factor(req, res); + }); + it('should return status code 401 when LDAP binding fails', function(done) { res.send = sinon.spy(function(data) { assert.equal(401, res.status.getCall(0).args[0]); diff --git a/test/unitary/routes/test_reset_password.js b/test/unitary/routes/test_reset_password.js index 8ceea448f..14bff5a32 100644 --- a/test/unitary/routes/test_reset_password.js +++ b/test/unitary/routes/test_reset_password.js @@ -46,7 +46,8 @@ describe('test reset password', function() { req.app.get.withArgs('ldap client').returns(ldap_client); config = {}; - config.ldap_users_dn = 'dc=example,dc=com'; + config.ldap_user_search_base = 'dc=example,dc=com'; + config.ldap_user_search_filter = 'cn'; req.app.get.withArgs('config').returns(config); res = {}; @@ -75,6 +76,15 @@ describe('test reset password', function() { }); }); + it('should perform a search in ldap to find email address', function(done) { + config.ldap_user_search_filter = 'uid'; + ldap_client.search = sinon.spy(function(dn) { + console.log(dn); + if(dn == 'uid=user,dc=example,dc=com') done(); + }); + reset_password.icheck_interface.pre_check_callback(req); + }); + it('should returns identity when ldap replies', function(done) { var doc = {}; doc.object = {}; diff --git a/test/unitary/test_data_persistence.js b/test/unitary/test_data_persistence.js index 32effd2c4..a20b323ae 100644 --- a/test/unitary/test_data_persistence.js +++ b/test/unitary/test_data_persistence.js @@ -54,7 +54,7 @@ describe('test data persistence', function() { port: PORT, totp_secret: 'totp_secret', ldap_url: 'ldap://127.0.0.1:389', - ldap_users_dn: 'ou=users,dc=example,dc=com', + ldap_user_search_base: 'ou=users,dc=example,dc=com', session_secret: 'session_secret', session_max_age: 50000, store_directory: tmpDir.name, diff --git a/test/unitary/test_ldap.js b/test/unitary/test_ldap.js index cbc348b43..9d301d239 100644 --- a/test/unitary/test_ldap.js +++ b/test/unitary/test_ldap.js @@ -25,9 +25,8 @@ describe('test ldap validation', function() { function test_validate() { var username = 'user'; var password = 'password'; - var ldap_url = 'http://ldap'; var users_dn = 'dc=example,dc=com'; - return ldap.validate(ldap_client, username, password, ldap_url, users_dn); + return ldap.validate(ldap_client, username, password, users_dn); } it('should bind the user if good credentials provided', function() { @@ -35,6 +34,29 @@ describe('test ldap validation', function() { return test_validate(); }); + it('should bind the user with correct DN', function(done) { + var username = 'user'; + var password = 'password'; + var user_search_base = 'dc=example,dc=com'; + var user_search_filter = 'uid'; + ldap_client.bind = sinon.spy(function(dn) { + if(dn == 'uid=user,dc=example,dc=com') done(); + }); + ldap.validate(ldap_client, username, password, user_search_base, + user_search_filter); + }); + + it('should default to cn user search filter if no filter provided', function(done) { + var username = 'user'; + var password = 'password'; + var user_search_base = 'dc=example,dc=com'; + ldap_client.bind = sinon.spy(function(dn) { + if(dn == 'cn=user,dc=example,dc=com') done(); + }); + ldap.validate(ldap_client, username, password, user_search_base, + undefined); + }); + // cover an issue with promisify context it('should promisify correctly', function() { function LdapClient() { @@ -76,6 +98,14 @@ describe('test ldap validation', function() { }) }); + it('should use the user filter', function(done) { + ldap_client.search = sinon.spy(function(dn) { + if(dn == 'uid=username,ou=users,dc=example,dc=com') done(); + }); + ldap.get_email(ldap_client, 'username', 'ou=users,dc=example,dc=com', + 'uid') + }); + it('should fail on error with search method', function(done) { var expected_doc = {}; expected_doc.mail = []; @@ -97,7 +127,7 @@ describe('test ldap validation', function() { change.modification.userPassword = 'new-password'; var config = {}; - config.ldap_users_dn = 'dc=example,dc=com'; + config.ldap_user_search_base = 'dc=example,dc=com'; config.ldap_user = 'admin'; var userdn = 'cn=user,dc=example,dc=com'; @@ -135,6 +165,22 @@ describe('test ldap validation', function() { done(); }) }); + + it('should use the user filter', function(done) { + var ldapjs = {}; + ldapjs.Change = sinon.spy(); + + var config = {}; + config.ldap_user_search_base = 'ou=users,dc=example,dc=com'; + config.ldap_user_search_filter = 'uid'; + config.ldap_user = 'admin'; + + ldap_client.bind.yields(undefined); + ldap_client.modify = sinon.spy(function(dn) { + if(dn == 'uid=username,ou=users,dc=example,dc=com') done(); + }); + ldap.update_password(ldap_client, ldapjs, 'username', 'newpass', config) + }); } }); diff --git a/test/unitary/test_server.js b/test/unitary/test_server.js index 8f6632e47..f9252b848 100644 --- a/test/unitary/test_server.js +++ b/test/unitary/test_server.js @@ -33,7 +33,8 @@ describe('test the server', function() { port: PORT, totp_secret: 'totp_secret', ldap_url: 'ldap://127.0.0.1:389', - ldap_users_dn: 'ou=users,dc=example,dc=com', + ldap_user_search_base: 'ou=users,dc=example,dc=com', + ldap_user_search_filter: 'cn', ldap_user: 'cn=admin,dc=example,dc=com', ldap_password: 'password', session_secret: 'session_secret', diff --git a/test/unitary/test_server_config.js b/test/unitary/test_server_config.js index 1e3d74b9e..4d6f9212b 100644 --- a/test/unitary/test_server_config.js +++ b/test/unitary/test_server_config.js @@ -4,9 +4,11 @@ var server = require('../../src/lib/server'); var assert = require('assert'); describe('test server configuration', function() { - it('should set cookie scope to domain set in the config', function() { - var config = {}; - config.session_domain = 'example.com'; + var deps; + var config; + + before(function() { + config = {}; config.notifier = { gmail: { user: 'user@example.com', @@ -22,13 +24,17 @@ describe('test server configuration', function() { return transporter;   }); - var deps = {}; + deps = {}; deps.nedb = require('nedb'); deps.nodemailer = nodemailer; deps.session = sinon.spy(function() { return function(req, res, next) { next(); }; }); + }); + + it('should set cookie scope to domain set in the config', function() { + config.session_domain = 'example.com'; server.run(config, undefined, deps); assert(deps.session.calledOnce);