Fix LDAP binding non working on servers with restricted ACL rules and add unit tests

docker-compose.dev.yml

@@ -8,10 +8,3 @@ services:
       - ./node_modules:/usr/src/node_modules
       - ./config.yml:/etc/auth-server/config.yml:ro
 
-  ldap-admin:
-    image: osixia/phpldapadmin:0.6.11
-    ports:
-      - 9090:80
-    environment:
-      - PHPLDAPADMIN_LDAP_HOSTS=ldap 
-      - PHPLDAPADMIN_HTTPS=false

docker-compose.yml

@@ -1,29 +1,12 @@
-
 version: '2'
 services:
   auth:
     build: .
-    depends_on:
-      - ldap
     restart: always
     volumes:
       - ./config.template.yml:/etc/auth-server/config.yml:ro
       - ./notifications:/var/lib/auth-server/notifications
 
-  ldap:
-    image: dinkel/openldap
-    environment:
-      - SLAPD_ORGANISATION=MyCompany
-      - SLAPD_DOMAIN=example.com
-      - SLAPD_PASSWORD=password
-      - SLAPD_ADDITIONAL_MODULES=memberof
-      - SLAPD_ADDITIONAL_SCHEMAS=openldap
-      - SLAPD_FORCE_RECONFIGURE=true
-    expose:
-      - "389"
-    volumes:
-      - ./example/ldap:/etc/ldap.dist/prepopulate
-
   nginx:
     image: nginx:alpine
     volumes:
@@ -35,3 +18,26 @@ services:
       - auth
     ports:
       - "8080:443"
+
+  openldap:
+    image: clems4ever/openldap
+    ports:
+      - "389:389"
+    environment:
+      - SLAPD_ORGANISATION=MyCompany
+      - SLAPD_DOMAIN=example.com
+      - SLAPD_PASSWORD=password
+      - SLAPD_CONFIG_PASSWORD=password
+      - SLAPD_ADDITIONAL_MODULES=memberof
+      - SLAPD_ADDITIONAL_SCHEMAS=openldap
+      - SLAPD_FORCE_RECONFIGURE=true
+    volumes:
+      - ./example/ldap:/etc/ldap.dist/prepopulate
+
+  openldap-admin:
+    image: osixia/phpldapadmin:0.6.11
+    ports:
+      - 9090:80
+    environment:
+      - PHPLDAPADMIN_LDAP_HOSTS=openldap
+      - PHPLDAPADMIN_HTTPS=false

example/ldap/access.rules

@@ -0,0 +1,4 @@
+olcAccess: {0}to attrs=userPassword,shadowLastChange by self write by anonymou
+ s auth by * none 
+# olcAccess: {1}to dn.base="" by * read 
+# olcAccess: {2}to * by * read 

src/server/lib/ErrorReplies.ts

@@ -1,8 +1,9 @@
 import express = require("express");
 import { Winston } from "winston";
+import BluebirdPromise = require("bluebird");
 
-function replyWithError(res: express.Response, code: number, logger: Winston) {
+function replyWithError(res: express.Response, code: number, logger: Winston): (err: Error) => void {
-  return function (err: Error) {
+  return function (err: Error): void {
     logger.error("Reply with error %d: %s", code, err);
     res.status(code);
     res.send();

src/server/lib/LdapClient.ts

@@ -18,7 +18,7 @@ export class LdapClient {
   private options: LdapConfiguration;
   private ldapjs: Ldapjs;
   private logger: Winston;
-  private client: ldapjs.ClientAsync;
+  private adminClient: ldapjs.ClientAsync;
 
   constructor(options: LdapConfiguration, ldapjs: Ldapjs, logger: Winston) {
     this.options = options;
@@ -28,100 +28,122 @@ export class LdapClient {
     this.connect();
   }
 
-  connect(): void {
+  private createClient(): ldapjs.ClientAsync {
-    const ldap_client = this.ldapjs.createClient({
+    const ldapClient = this.ldapjs.createClient({
       url: this.options.url,
       reconnect: true
     });
 
-    ldap_client.on("error", function (err: Error) {
+    ldapClient.on("error", function (err: Error) {
       console.error("LDAP Error:", err.message);
     });
 
-    this.client = BluebirdPromise.promisifyAll(ldap_client) as ldapjs.ClientAsync;
+    return BluebirdPromise.promisifyAll(ldapClient) as ldapjs.ClientAsync;
   }
 
-  private build_user_dn(username: string): string {
+  connect(): BluebirdPromise<void> {
-    let user_name_attr = this.options.user_name_attribute;
+    const userDN = this.options.user;
-    // if not provided, default to cn
+    const password = this.options.password;
-    if (!user_name_attr) user_name_attr = "cn";
 
-    const additional_user_dn = this.options.additional_user_dn;
+    this.adminClient = this.createClient();
+    return this.adminClient.bindAsync(userDN, password);
+  }
+
+  private buildUserDN(username: string): string {
+    let userNameAttribute = this.options.user_name_attribute;
+    // if not provided, default to cn
+    if (!userNameAttribute) userNameAttribute = "cn";
+
+    const additionalUserDN = this.options.additional_user_dn;
     const base_dn = this.options.base_dn;
 
-    let user_dn = util.format("%s=%s", user_name_attr, username);
+    let userDN = util.format("%s=%s", userNameAttribute, username);
-    if (additional_user_dn) user_dn += util.format(",%s", additional_user_dn);
+    if (additionalUserDN) userDN += util.format(",%s", additionalUserDN);
-    user_dn += util.format(",%s", base_dn);
+    userDN += util.format(",%s", base_dn);
-    return user_dn;
+    return userDN;
   }
 
-  bind(username: string, password: string): BluebirdPromise<void> {
+  checkPassword(username: string, password: string): BluebirdPromise<void> {
-    const user_dn = this.build_user_dn(username);
+    const userDN = this.buildUserDN(username);
+    const that = this;
+    const ldapClient = this.createClient();
 
-    this.logger.debug("LDAP: Bind user %s", user_dn);
+    this.logger.debug("LDAP: Check password for user '%s'", userDN);
-    return this.client.bindAsync(user_dn, password)
+    return ldapClient.bindAsync(userDN, password)
+      .then(function () {
+        return ldapClient.unbindAsync();
+      })
       .error(function (err: Error) {
-        throw new exceptions.LdapBindError(err.message);
+        return BluebirdPromise.reject(new exceptions.LdapBindError(err.message));
       });
   }
 
-  private search_in_ldap(base: string, query: ldapjs.SearchOptions): BluebirdPromise<any> {
+  private search(base: string, query: ldapjs.SearchOptions): BluebirdPromise<any> {
-    this.logger.debug("LDAP: Search for %s in %s", JSON.stringify(query), base);
+    const that = this;
-    return new BluebirdPromise((resolve, reject) => {
+
-      this.client.searchAsync(base, query)
+    that.logger.debug("LDAP: Search for '%s' in '%s'", JSON.stringify(query), base);
-        .then(function (res: EventEmitter) {
+    return that.adminClient.searchAsync(base, query)
-          const doc: SearchEntry[] = [];
+      .then(function (res: EventEmitter) {
+        const doc: SearchEntry[] = [];
+
+        return new BluebirdPromise((resolve, reject) => {
           res.on("searchEntry", function (entry: SearchEntry) {
+            that.logger.debug("Entry retrieved from LDAP is '%s'", JSON.stringify(entry.object));
             doc.push(entry.object);
           });
           res.on("error", function (err: Error) {
+            that.logger.error("LDAP: Error received during search '%s'.", JSON.stringify(err));
             reject(new exceptions.LdapSearchError(err.message));
           });
           res.on("end", function () {
+            that.logger.debug("LDAP: Result of search is '%s'.", JSON.stringify(doc));
             resolve(doc);
           });
-        })
-        .catch(function (err: Error) {
-          reject(new exceptions.LdapSearchError(err.message));
         });
-    });
+      })
+      .catch(function (err: Error) {
+        return BluebirdPromise.reject(new exceptions.LdapSearchError(err.message));
+      });
   }
 
-  get_groups(username: string): BluebirdPromise<string[]> {
+  retrieveGroups(username: string): BluebirdPromise<string[]> {
-    const user_dn = this.build_user_dn(username);
+    const userDN = this.buildUserDN(username);
+    const password = this.options.password;
 
-    let group_name_attr = this.options.group_name_attribute;
+    let groupNameAttribute = this.options.group_name_attribute;
-    if (!group_name_attr) group_name_attr = "cn";
+    if (!groupNameAttribute) groupNameAttribute = "cn";
 
-    const additional_group_dn = this.options.additional_group_dn;
+    const additionalGroupDN = this.options.additional_group_dn;
     const base_dn = this.options.base_dn;
 
-    let group_dn = base_dn;
+    let groupDN = base_dn;
-    if (additional_group_dn)
+    if (additionalGroupDN)
-      group_dn = util.format("%s,", additional_group_dn) + group_dn;
+      groupDN = util.format("%s,", additionalGroupDN) + groupDN;
 
     const query = {
       scope: "sub",
-      attributes: [group_name_attr],
+      attributes: [groupNameAttribute],
-      filter: "member=" + user_dn
+      filter: "member=" + userDN
     };
 
     const that = this;
     this.logger.debug("LDAP: get groups of user %s", username);
-    return this.search_in_ldap(group_dn, query)
+    const groups: string[] = [];
+    return that.search(groupDN, query)
       .then(function (docs) {
-        const groups = [];
         for (let i = 0; i < docs.length; ++i) {
           groups.push(docs[i].cn);
         }
-        that.logger.debug("LDAP: got groups %s", groups);
+        that.logger.debug("LDAP: got groups '%s'", groups);
+      })
+      .then(function () {
         return BluebirdPromise.resolve(groups);
       });
   }
 
-  get_emails(username: string): BluebirdPromise<string[]> {
+  retrieveEmails(username: string): BluebirdPromise<string[]> {
     const that = this;
-    const user_dn = this.build_user_dn(username);
+    const user_dn = this.buildUserDN(username);
 
     const query = {
       scope: "base",
@@ -129,8 +151,8 @@ export class LdapClient {
       attributes: ["mail"]
     };
 
-    this.logger.debug("LDAP: get emails of user %s", username);
+    this.logger.debug("LDAP: get emails of user '%s'", username);
-    return this.search_in_ldap(user_dn, query)
+    return this.search(user_dn, query)
       .then(function (docs) {
         const emails = [];
         for (let i = 0; i < docs.length; ++i) {
@@ -140,15 +162,15 @@ export class LdapClient {
             emails.concat(docs[i].mail);
           }
         }
-        that.logger.debug("LDAP: got emails %s", emails);
+        that.logger.debug("LDAP: got emails '%s'", emails);
         return BluebirdPromise.resolve(emails);
       });
   }
 
-  update_password(username: string, new_password: string): BluebirdPromise<void> {
+  updatePassword(username: string, newPassword: string): BluebirdPromise<void> {
-    const user_dn = this.build_user_dn(username);
+    const user_dn = this.buildUserDN(username);
 
-    const encoded_password = Dovehash.encode("SSHA", new_password);
+    const encoded_password = Dovehash.encode("SSHA", newPassword);
     const change = {
       operation: "replace",
       modification: {
@@ -157,13 +179,12 @@ export class LdapClient {
     };
 
     const that = this;
-    this.logger.debug("LDAP: update password of user %s", username);
+    this.logger.debug("LDAP: update password of user '%s'", username);
 
-    this.logger.debug("LDAP: bind admin");
+    that.logger.debug("LDAP: modify password");
-    return this.client.bindAsync(this.options.user, this.options.password)
+    return that.adminClient.modifyAsync(user_dn, change)
       .then(function () {
-        that.logger.debug("LDAP: modify password");
+        return that.adminClient.unbindAsync();
-        return that.client.modifyAsync(user_dn, change);
       });
   }
 }

src/server/lib/routes/firstfactor/post.ts

@@ -35,21 +35,28 @@ export default function (req: express.Request, res: express.Response): BluebirdP
 
     return regulator.regulate(username)
         .then(function () {
-            return ldap.bind(username, password);
+            logger.info("1st factor: No regulation applied.");
+            return ldap.checkPassword(username, password);
         })
         .then(function () {
+            logger.info("1st factor: LDAP binding successful");
             authSession.userid = username;
             authSession.first_factor = true;
-            logger.info("1st factor: LDAP binding successful");
             logger.debug("1st factor: Retrieve email from LDAP");
-            return BluebirdPromise.join(ldap.get_emails(username), ldap.get_groups(username));
+            return BluebirdPromise.join(ldap.retrieveEmails(username), ldap.retrieveGroups(username));
         })
         .then(function (data: [string[], string[]]) {
             const emails: string[] = data[0];
             const groups: string[] = data[1];
 
-            if (!emails && emails.length <= 0) throw new Error("No email found");
+            if (!emails || emails.length <= 0) {
+                const errMessage = "No emails found. The user should have at least one email address to reset password.";
+                logger.error("1s factor: %s", errMessage);
+                return BluebirdPromise.reject(new Error(errMessage));
+            }
+
             logger.debug("1st factor: Retrieved email are %s", emails);
+            logger.debug("1st factor: Retrieved groups are %s", groups);
             authSession.email = emails[0];
             authSession.groups = groups;
 
@@ -61,7 +68,7 @@ export default function (req: express.Request, res: express.Response): BluebirdP
         .catch(exceptions.LdapSearchError, ErrorReplies.replyWithError500(res, logger))
         .catch(exceptions.LdapBindError, function (err: Error) {
             regulator.mark(username, false);
-            ErrorReplies.replyWithError401(res, logger)(err);
+            return ErrorReplies.replyWithError401(res, logger)(err);
         })
         .catch(exceptions.AuthenticationRegulationError, ErrorReplies.replyWithError403(res, logger))
         .catch(exceptions.DomainAccessDenied, ErrorReplies.replyWithError401(res, logger))

src/server/lib/routes/password-reset/form/post.ts

@@ -26,7 +26,7 @@ export default function (req: express.Request, res: express.Response): BluebirdP
 
     logger.info("POST reset-password: User %s wants to reset his/her password", userid);
 
-    return ldap.update_password(userid, new_password)
+    return ldap.updatePassword(userid, new_password)
         .then(function () {
             logger.info("POST reset-password: Password reset for user '%s'", userid);
             AuthenticationSession.reset(req);

src/server/lib/routes/password-reset/identity/PasswordResetHandler.ts

@@ -26,7 +26,7 @@ export default class PasswordResetHandler implements IdentityValidable {
             return BluebirdPromise.reject(new exceptions.AccessDeniedError("No user id provided"));
 
         const ldap = ServerVariables.getLdapClient(req.app);
-        return ldap.get_emails(userid)
+        return ldap.retrieveEmails(userid)
             .then(function (emails: string[]) {
                 if (!emails && emails.length <= 0) throw new Error("No email found");
 

src/types/ldapjs-async.d.ts

@@ -5,6 +5,7 @@ import { EventEmitter } from "events";
 declare module "ldapjs" {
     export interface ClientAsync {
         bindAsync(username: string, password: string): BluebirdPromise<void>;
+        unbindAsync(): BluebirdPromise<void>;
         searchAsync(base: string, query: ldapjs.SearchOptions): BluebirdPromise<EventEmitter>;
         modifyAsync(userdn: string, change: ldapjs.Change): BluebirdPromise<void>;
     }

test/server/DataPersistence.test.ts

@@ -7,6 +7,7 @@ import { UserConfiguration } from "../../src/types/Configuration";
 import { GlobalDependencies } from "../../src/types/Dependencies";
 import * as tmp from "tmp";
 import U2FMock = require("./mocks/u2f");
+import { LdapjsClientMock } from "./mocks/ldapjs";
 
 
 const requestp = BluebirdPromise.promisifyAll(request) as request.Request;
@@ -23,14 +24,10 @@ const requests = require("./requests")(PORT);
 describe("test data persistence", function () {
   let u2f: U2FMock.U2FMock;
   let tmpDir: tmp.SynchrounousResult;
-  const ldap_client = {
+  const ldapClient = LdapjsClientMock();
-    bind: sinon.stub(),
-    search: sinon.stub(),
-    on: sinon.spy()
-  };
   const ldap = {
     createClient: sinon.spy(function () {
-      return ldap_client;
+      return ldapClient;
     })
   };
 
@@ -51,11 +48,12 @@ describe("test data persistence", function () {
       })
     };
 
-    ldap_client.bind.withArgs("cn=test_ok,ou=users,dc=example,dc=com",
+    ldapClient.bind.withArgs("cn=test_ok,ou=users,dc=example,dc=com",
-      "password").yields(undefined);
+      "password").yields();
-    ldap_client.bind.withArgs("cn=test_nok,ou=users,dc=example,dc=com",
+    ldapClient.bind.withArgs("cn=test_nok,ou=users,dc=example,dc=com",
       "password").yields("error");
-    ldap_client.search.yields(undefined, search_res);
+    ldapClient.search.yields(undefined, search_res);
+    ldapClient.unbind.yields();
 
     tmpDir = tmp.dirSync({ unsafeCleanup: true });
     config = {

test/server/LdapClient.test.ts

@@ -14,22 +14,16 @@ import { LdapjsMock, LdapjsClientMock } from "./mocks/ldapjs";
 
 describe("test ldap validation", function () {
   let ldap: LdapClient.LdapClient;
-  let ldap_client: LdapjsClientMock;
+  let ldapClient: LdapjsClientMock;
   let ldapjs: LdapjsMock;
-  let ldap_config: LdapConfiguration;
+  let ldapConfig: LdapConfiguration;
 
   beforeEach(function () {
-    ldap_client = {
+    ldapClient = LdapjsClientMock();
-      bind: sinon.stub(),
-      search: sinon.stub(),
-      modify: sinon.stub(),
-      on: sinon.stub()
-    } as any;
-
     ldapjs = LdapjsMock();
-    ldapjs.createClient.returns(ldap_client);
+    ldapjs.createClient.returns(ldapClient);
 
-    ldap_config = {
+    ldapConfig = {
       url: "http://localhost:324",
       user: "admin",
       password: "password",
@@ -37,45 +31,47 @@ describe("test ldap validation", function () {
       additional_user_dn: "ou=users"
     };
 
-    ldap = new LdapClient.LdapClient(ldap_config, ldapjs, winston);
+    ldap = new LdapClient.LdapClient(ldapConfig, ldapjs, winston);
-    return ldap.connect();
   });
 
-  describe("test binding", test_binding);
+  describe("test checking password", test_checking_password);
   describe("test get emails from username", test_get_emails);
   describe("test get groups from username", test_get_groups);
   describe("test update password", test_update_password);
 
-  function test_binding() {
+  function test_checking_password() {
-    function test_bind() {
+    function test_check_password_internal() {
       const username = "username";
       const password = "password";
-      return ldap.bind(username, password);
+      return ldap.checkPassword(username, password);
     }
 
     it("should bind the user if good credentials provided", function () {
-      ldap_client.bind.yields();
+      ldapClient.bind.yields();
-      return test_bind();
+      ldapClient.unbind.yields();
+      return test_check_password_internal();
     });
 
     it("should bind the user with correct DN", function () {
-      ldap_config.user_name_attribute = "uid";
+      ldapConfig.user_name_attribute = "uid";
       const username = "user";
       const password = "password";
-      ldap_client.bind.withArgs("uid=user,ou=users,dc=example,dc=com").yields();
+      ldapClient.bind.withArgs("uid=user,ou=users,dc=example,dc=com").yields();
-      return ldap.bind(username, password);
+      ldapClient.unbind.yields();
+      return ldap.checkPassword(username, password);
     });
 
     it("should default to cn user search filter if no filter provided", function () {
       const username = "user";
       const password = "password";
-      ldap_client.bind.withArgs("cn=user,ou=users,dc=example,dc=com").yields();
+      ldapClient.bind.withArgs("cn=user,ou=users,dc=example,dc=com").yields();
-      return ldap.bind(username, password);
+      ldapClient.unbind.yields();
+      return ldap.checkPassword(username, password);
     });
 
     it("should not bind the user if wrong credentials provided", function () {
-      ldap_client.bind.yields("wrong credentials");
+      ldapClient.bind.yields("wrong credentials");
-      const promise = test_bind();
+      const promise = test_check_password_internal();
       return promise.catch(function () {
         return BluebirdPromise.resolve();
       });
@@ -101,9 +97,9 @@ describe("test ldap validation", function () {
     });
 
     it("should retrieve the email of an existing user", function () {
-      ldap_client.search.yields(undefined, res_emitter);
+      ldapClient.search.yields(undefined, res_emitter);
 
-      return ldap.get_emails("user")
+      return ldap.retrieveEmails("user")
         .then(function (emails) {
           assert.deepEqual(emails, [expected_doc.object.mail]);
           return BluebirdPromise.resolve();
@@ -111,9 +107,9 @@ describe("test ldap validation", function () {
     });
 
     it("should retrieve email for user with uid name attribute", function () {
-      ldap_config.user_name_attribute = "uid";
+      ldapConfig.user_name_attribute = "uid";
-      ldap_client.search.withArgs("uid=username,ou=users,dc=example,dc=com").yields(undefined, res_emitter);
+      ldapClient.search.withArgs("uid=username,ou=users,dc=example,dc=com").yields(undefined, res_emitter);
-      return ldap.get_emails("username")
+      return ldap.retrieveEmails("username")
         .then(function (emails) {
           assert.deepEqual(emails, ["user@example.com"]);
           return BluebirdPromise.resolve();
@@ -124,9 +120,9 @@ describe("test ldap validation", function () {
       const expected_doc = {
         mail: ["user@example.com"]
       };
-      ldap_client.search.yields("Error while searching mails");
+      ldapClient.search.yields("Error while searching mails");
 
-      return ldap.get_emails("user")
+      return ldap.retrieveEmails("user")
         .catch(function () {
           return BluebirdPromise.resolve();
         });
@@ -159,8 +155,8 @@ describe("test ldap validation", function () {
     });
 
     it("should retrieve the groups of an existing user", function () {
-      ldap_client.search.yields(undefined, res_emitter);
+      ldapClient.search.yields(undefined, res_emitter);
-      return ldap.get_groups("user")
+      return ldap.retrieveGroups("user")
         .then(function (groups) {
           assert.deepEqual(groups, ["group1", "group2"]);
           return BluebirdPromise.resolve();
@@ -168,29 +164,29 @@ describe("test ldap validation", function () {
     });
 
     it("should reduce the scope to additional_group_dn", function (done) {
-      ldap_config.additional_group_dn = "ou=groups";
+      ldapConfig.additional_group_dn = "ou=groups";
-      ldap_client.search.yields(undefined, res_emitter);
+      ldapClient.search.yields(undefined, res_emitter);
-      ldap.get_groups("user")
+      ldap.retrieveGroups("user")
       .then(function() {
-        assert.equal(ldap_client.search.getCall(0).args[0], "ou=groups,dc=example,dc=com");
+        assert.equal(ldapClient.search.getCall(0).args[0], "ou=groups,dc=example,dc=com");
         done();
       });
     });
 
     it("should use default group_name_attr if not provided", function (done) {
-      ldap_client.search.yields(undefined, res_emitter);
+      ldapClient.search.yields(undefined, res_emitter);
-      ldap.get_groups("user")
+      ldap.retrieveGroups("user")
       .then(function() {
-        assert.equal(ldap_client.search.getCall(0).args[0], "dc=example,dc=com");
+        assert.equal(ldapClient.search.getCall(0).args[0], "dc=example,dc=com");
-        assert.equal(ldap_client.search.getCall(0).args[1].filter, "member=cn=user,ou=users,dc=example,dc=com");
+        assert.equal(ldapClient.search.getCall(0).args[1].filter, "member=cn=user,ou=users,dc=example,dc=com");
-        assert.deepEqual(ldap_client.search.getCall(0).args[1].attributes, ["cn"]);
+        assert.deepEqual(ldapClient.search.getCall(0).args[1].attributes, ["cn"]);
         done();
       });
     });
 
     it("should fail on error with search method", function () {
-      ldap_client.search.yields("error");
+      ldapClient.search.yields("error");
-      return ldap.get_groups("user")
+      return ldap.retrieveGroups("user")
         .catch(function () {
           return BluebirdPromise.resolve();
         });
@@ -207,36 +203,39 @@ describe("test ldap validation", function () {
       };
       const userdn = "cn=user,ou=users,dc=example,dc=com";
 
-      ldap_client.bind.yields(undefined);
+      ldapClient.bind.yields();
-      ldap_client.modify.yields(undefined);
+      ldapClient.unbind.yields();
+      ldapClient.modify.yields();
 
-      return ldap.update_password("user", "new-password")
+      return ldap.updatePassword("user", "new-password")
         .then(function () {
-          assert.deepEqual(ldap_client.modify.getCall(0).args[0], userdn);
+          assert.deepEqual(ldapClient.modify.getCall(0).args[0], userdn);
-          assert.deepEqual(ldap_client.modify.getCall(0).args[1].operation, change.operation);
+          assert.deepEqual(ldapClient.modify.getCall(0).args[1].operation, change.operation);
 
-          const userPassword = ldap_client.modify.getCall(0).args[1].modification.userPassword;
+          const userPassword = ldapClient.modify.getCall(0).args[1].modification.userPassword;
           assert(/{SSHA}/.test(userPassword));
           return BluebirdPromise.resolve();
-        });
+        })
+        .catch(function(err) { return BluebirdPromise.reject(new Error("It should fail")); });
     });
 
     it("should fail when ldap throws an error", function () {
-      ldap_client.bind.yields(undefined);
+      ldapClient.bind.yields(undefined);
-      ldap_client.modify.yields("Error");
+      ldapClient.modify.yields("Error");
 
-      return ldap.update_password("user", "new-password")
+      return ldap.updatePassword("user", "new-password")
         .catch(function () {
           return BluebirdPromise.resolve();
         });
     });
 
     it("should update password of user using particular user name attribute", function () {
-      ldap_config.user_name_attribute = "uid";
+      ldapConfig.user_name_attribute = "uid";
 
-      ldap_client.bind.yields(undefined);
+      ldapClient.bind.yields();
-      ldap_client.modify.withArgs("uid=username,ou=users,dc=example,dc=com").yields();
+      ldapClient.unbind.yields();
-      return ldap.update_password("username", "newpass");
+      ldapClient.modify.withArgs("uid=username,ou=users,dc=example,dc=com").yields();
+      return ldap.updatePassword("username", "newpass");
     });
   }
 });

test/server/Server.test.ts

@@ -1,6 +1,7 @@
 
 import Server from "../../src/server/lib/Server";
 import LdapClient = require("../../src/server/lib/LdapClient");
+import { LdapjsClientMock } from "./mocks/ldapjs";
 
 import BluebirdPromise = require("bluebird");
 import speakeasy = require("speakeasy");
@@ -51,16 +52,11 @@ describe("test the server", function () {
       }
     };
 
-    const ldap_client = {
+    const ldapClient = LdapjsClientMock();
-      bind: sinon.stub(),
-      search: sinon.stub(),
-      modify: sinon.stub(),
-      on: sinon.spy()
-    };
     const ldap = {
       Change: sinon.spy(),
       createClient: sinon.spy(function () {
-        return ldap_client;
+        return ldapClient;
       })
     };
 
@@ -76,7 +72,7 @@ describe("test the server", function () {
       })
     };
 
-    const ldap_document = {
+    const ldapDocument = {
       object: {
         mail: "test_ok@example.com",
       }
@@ -84,20 +80,21 @@ describe("test the server", function () {
 
     const search_res = {
       on: sinon.spy(function (event: string, fn: (s: any) => void) {
-        if (event != "error") fn(ldap_document);
+        if (event != "error") fn(ldapDocument);
       })
     };
 
-    ldap_client.bind.withArgs("cn=test_ok,ou=users,dc=example,dc=com",
+    ldapClient.bind.withArgs("cn=test_ok,ou=users,dc=example,dc=com",
-      "password").yields(undefined);
+      "password").yields();
-    ldap_client.bind.withArgs("cn=admin,dc=example,dc=com",
+    ldapClient.bind.withArgs("cn=admin,dc=example,dc=com",
-      "password").yields(undefined);
+      "password").yields();
 
-    ldap_client.bind.withArgs("cn=test_nok,ou=users,dc=example,dc=com",
+    ldapClient.bind.withArgs("cn=test_nok,ou=users,dc=example,dc=com",
       "password").yields("error");
 
-    ldap_client.modify.yields(undefined);
+    ldapClient.unbind.yields();
-    ldap_client.search.yields(undefined, search_res);
+    ldapClient.modify.yields();
+    ldapClient.search.yields(undefined, search_res);
 
     const deps = {
       u2f: u2f,
@@ -241,11 +238,11 @@ describe("test the server", function () {
           return requests.register_totp(j, transporter);
         })
         .then(function (base32_secret: string) {
-          const real_token = speakeasy.totp({
+          const realToken = speakeasy.totp({
             secret: base32_secret,
             encoding: "base32"
           });
-          return requests.totp(j, real_token);
+          return requests.totp(j, realToken);
         })
         .then(function (res: request.RequestResponse) {
           assert.equal(res.statusCode, 200, "second factor failed");
@@ -254,14 +251,11 @@ describe("test the server", function () {
         .then(function (res: request.RequestResponse) {
           assert.equal(res.statusCode, 204, "verify failed");
           return BluebirdPromise.resolve();
-        });
+        })
+        .catch(function (err: Error) { return BluebirdPromise.reject(err); });
     });
 
     it("should keep session variables when login page is reloaded", function () {
-      const real_token = speakeasy.totp({
-        secret: "totp_secret",
-        encoding: "base32"
-      });
       const j = requestp.jar();
       return requests.login(j)
         .then(function (res: request.RequestResponse) {
@@ -269,11 +263,18 @@ describe("test the server", function () {
           return requests.first_factor(j);
         })
         .then(function (res: request.RequestResponse) {
-          assert.equal(res.statusCode, 204, "first factor failed");
+          assert.equal(res.statusCode, 302, "first factor failed");
-          return requests.totp(j, real_token);
+          return requests.register_totp(j, transporter);
+        })
+        .then(function (base32_secret: string) {
+          const realToken = speakeasy.totp({
+            secret: base32_secret,
+            encoding: "base32"
+          });
+          return requests.totp(j, realToken);
         })
         .then(function (res: request.RequestResponse) {
-          assert.equal(res.statusCode, 204, "second factor failed");
+          assert.equal(res.statusCode, 200, "second factor failed");
           return requests.login(j);
         })
         .then(function (res: request.RequestResponse) {
@@ -284,9 +285,7 @@ describe("test the server", function () {
           assert.equal(res.statusCode, 204, "verify failed");
           return BluebirdPromise.resolve();
         })
-        .catch(function (err: Error) {
+        .catch(function (err: Error) { return BluebirdPromise.reject(err); });
-          console.error(err);
-        });
     });
 
     it("should return status code 204 when user is authenticated using u2f", function () {

test/server/ServerConfig.test.ts

@@ -1,6 +1,6 @@
 
 import assert = require("assert");
-import sinon = require ("sinon");
+import sinon = require("sinon");
 import nedb = require("nedb");
 import express = require("express");
 import winston = require("winston");
@@ -36,7 +36,10 @@ describe("test server configuration", function () {
       winston: winston,
       ldapjs: {
         createClient: sinon.spy(function () {
-          return { on: sinon.spy() };
+          return {
+            on: sinon.spy(),
+            bind: sinon.spy()
+          };
         })
       },
       session: sessionMock as any

test/server/mocks/LdapClient.ts

@@ -2,19 +2,19 @@
 import sinon = require("sinon");
 
 export interface LdapClientMock {
-    bind: sinon.SinonStub;
+    checkPassword: sinon.SinonStub;
-    get_emails: sinon.SinonStub;
+    retrieveEmails: sinon.SinonStub;
-    get_groups: sinon.SinonStub;
+    retrieveGroups: sinon.SinonStub;
-    search_in_ldap: sinon.SinonStub;
+    search: sinon.SinonStub;
-    update_password: sinon.SinonStub;
+    updatePassword: sinon.SinonStub;
 }
 
 export function LdapClientMock(): LdapClientMock {
     return {
-        bind: sinon.stub(),
+        checkPassword: sinon.stub(),
-        get_emails: sinon.stub(),
+        retrieveEmails: sinon.stub(),
-        get_groups: sinon.stub(),
+        retrieveGroups: sinon.stub(),
-        search_in_ldap: sinon.stub(),
+        search: sinon.stub(),
-        update_password: sinon.stub()
+        updatePassword: sinon.stub()
     };
 }

test/server/mocks/ldapjs.ts

@@ -7,6 +7,7 @@ export interface LdapjsMock {
 
 export interface LdapjsClientMock {
     bind: sinon.SinonStub;
+    unbind: sinon.SinonStub;
     search: sinon.SinonStub;
     modify: sinon.SinonStub;
     on: sinon.SinonStub;
@@ -21,6 +22,7 @@ export function LdapjsMock(): LdapjsMock {
 export function LdapjsClientMock(): LdapjsClientMock {
     return {
         bind: sinon.stub(),
+        unbind: sinon.stub(),
         search: sinon.stub(),
         modify: sinon.stub(),
         on: sinon.stub()

test/server/routes/firstfactor/post.test.ts

@@ -72,8 +72,8 @@ describe("test the first factor validation route", function () {
   });
 
   it("should redirect client to second factor page", function () {
-    ldapMock.bind.withArgs("username").returns(BluebirdPromise.resolve());
+    ldapMock.checkPassword.withArgs("username").returns(BluebirdPromise.resolve());
-    ldapMock.get_emails.returns(BluebirdPromise.resolve(emails));
+    ldapMock.retrieveEmails.returns(BluebirdPromise.resolve(emails));
     const authSession = AuthenticationSession.get(req as any);
     return FirstFactorPost.default(req as any, res as any)
       .then(function () {
@@ -82,55 +82,60 @@ describe("test the first factor validation route", function () {
       });
   });
 
-  it("should retrieve email from LDAP", function (done) {
+  it("should retrieve email from LDAP", function () {
-    res.redirect = sinon.spy(function () { done(); });
+    ldapMock.checkPassword.returns(BluebirdPromise.resolve());
-    ldapMock.bind.returns(BluebirdPromise.resolve());
+    ldapMock.retrieveEmails = sinon.stub().withArgs("username").returns(BluebirdPromise.resolve([{ mail: ["test@example.com"] }]));
-    ldapMock.get_emails = sinon.stub().withArgs("username").returns(BluebirdPromise.resolve([{ mail: ["test@example.com"] }]));
+    return FirstFactorPost.default(req as any, res as any);
-    FirstFactorPost.default(req as any, res as any);
   });
 
-  it("should set email as session variables", function () {
+  it("should set first email address as user session variable", function () {
     const emails = ["test_ok@example.com"];
     const authSession = AuthenticationSession.get(req as any);
-    ldapMock.bind.returns(BluebirdPromise.resolve());
+    ldapMock.checkPassword.returns(BluebirdPromise.resolve());
-    ldapMock.get_emails.returns(BluebirdPromise.resolve(emails));
+    ldapMock.retrieveEmails.returns(BluebirdPromise.resolve(emails));
     return FirstFactorPost.default(req as any, res as any)
       .then(function () {
         assert.equal("test_ok@example.com", authSession.email);
       });
   });
 
-  it("should return status code 401 when LDAP binding throws", function (done) {
+  it("should return status code 401 when LDAP binding throws", function () {
-    res.send = sinon.spy(function () {
+    ldapMock.checkPassword.returns(BluebirdPromise.reject(new exceptions.LdapBindError("Bad credentials")));
-      assert.equal(401, res.status.getCall(0).args[0]);
+    return FirstFactorPost.default(req as any, res as any)
-      assert.equal(regulator.mark.getCall(0).args[0], "username");
+      .then(function () {
-      done();
+        assert.equal(401, res.status.getCall(0).args[0]);
-    });
+        assert.equal(regulator.mark.getCall(0).args[0], "username");
-    ldapMock.bind.returns(BluebirdPromise.reject(new exceptions.LdapBindError("Bad credentials")));
+      });
-    FirstFactorPost.default(req as any, res as any);
   });
 
-  it("should return status code 500 when LDAP search throws", function (done) {
+  it("should return status code 500 when LDAP search throws", function () {
-    res.send = sinon.spy(function () {
+    ldapMock.checkPassword.returns(BluebirdPromise.resolve());
-      assert.equal(500, res.status.getCall(0).args[0]);
+    ldapMock.retrieveEmails.returns(BluebirdPromise.reject(new exceptions.LdapSearchError("error while retrieving emails")));
-      done();
+    return FirstFactorPost.default(req as any, res as any)
-    });
+      .then(function () {
-    ldapMock.bind.returns(BluebirdPromise.resolve());
+        assert.equal(500, res.status.getCall(0).args[0]);
-    ldapMock.get_emails.returns(BluebirdPromise.reject(new exceptions.LdapSearchError("error while retrieving emails")));
+      });
-    FirstFactorPost.default(req as any, res as any);
   });
 
-  it("should return status code 403 when regulator rejects authentication", function (done) {
+  it("should return status code 403 when regulator rejects authentication", function () {
     const err = new exceptions.AuthenticationRegulationError("Authentication regulation...");
     regulator.regulate.returns(BluebirdPromise.reject(err));
+    return FirstFactorPost.default(req as any, res as any)
+      .then(function () {
+        assert.equal(403, res.status.getCall(0).args[0]);
+        assert.equal(1, res.send.callCount);
+      });
+  });
 
-    res.send = sinon.spy(function () {
+  it("should fail when admin user does not have rights to retrieve attribute mail", function () {
-      assert.equal(403, res.status.getCall(0).args[0]);
+    ldapMock.checkPassword.returns(BluebirdPromise.resolve());
-      done();
+    ldapMock.retrieveEmails = sinon.stub().withArgs("username").returns(BluebirdPromise.resolve([]));
-    });
+    ldapMock.retrieveGroups = sinon.stub().withArgs("username").returns(BluebirdPromise.resolve(["group1"]));
-    ldapMock.bind.returns(BluebirdPromise.resolve());
+    return FirstFactorPost.default(req as any, res as any)
-    ldapMock.get_emails.returns(BluebirdPromise.resolve());
+      .then(function () {
-    FirstFactorPost.default(req as any, res as any);
+        assert.equal(500, res.status.getCall(0).args[0]);
+        assert.equal(1, res.send.callCount);
+      });
   });
 });
 

test/server/routes/password-reset/identity/PasswordResetHandler.test.ts

@@ -82,7 +82,7 @@ describe("test reset password identity check", function () {
         });
 
         it("should fail if ldap fail", function (done) {
-            ldap_client.get_emails.returns(BluebirdPromise.reject("Internal error"));
+            ldap_client.retrieveEmails.returns(BluebirdPromise.reject("Internal error"));
             new PasswordResetHandler().preValidationInit(req as any)
                 .catch(function (err: Error) {
                     done();
@@ -91,16 +91,16 @@ describe("test reset password identity check", function () {
 
         it("should perform a search in ldap to find email address", function (done) {
             configuration.ldap.user_name_attribute = "uid";
-            ldap_client.get_emails.returns(BluebirdPromise.resolve([]));
+            ldap_client.retrieveEmails.returns(BluebirdPromise.resolve([]));
             new PasswordResetHandler().preValidationInit(req as any)
                 .then(function () {
-                    assert.equal("user", ldap_client.get_emails.getCall(0).args[0]);
+                    assert.equal("user", ldap_client.retrieveEmails.getCall(0).args[0]);
                     done();
                 });
         });
 
         it("should returns identity when ldap replies", function (done) {
-            ldap_client.get_emails.returns(BluebirdPromise.resolve(["test@example.com"]));
+            ldap_client.retrieveEmails.returns(BluebirdPromise.resolve(["test@example.com"]));
             new PasswordResetHandler().preValidationInit(req as any)
                 .then(function () {
                     done();

test/server/routes/password-reset/post.test.ts

@@ -16,7 +16,7 @@ describe("test reset password route", function () {
   let req: ExpressMock.RequestMock;
   let res: ExpressMock.ResponseMock;
   let user_data_store: UserDataStore;
-  let ldap_client: LdapClientMock;
+  let ldapClient: LdapClientMock;
   let configuration: any;
   let authSession: AuthenticationSession.AuthenticationSession;
 
@@ -64,8 +64,8 @@ describe("test reset password route", function () {
     mocks.logger = winston;
     mocks.config = configuration;
 
-    ldap_client = LdapClientMock();
+    ldapClient = LdapClientMock();
-    mocks.ldap = ldap_client;
+    mocks.ldap = ldapClient;
 
     res = ExpressMock.ResponseMock();
   });
@@ -79,8 +79,8 @@ describe("test reset password route", function () {
       req.body = {};
       req.body.password = "new-password";
 
-      ldap_client.update_password.returns(BluebirdPromise.resolve());
+      ldapClient.updatePassword.returns(BluebirdPromise.resolve());
-      ldap_client.bind.returns(BluebirdPromise.resolve());
+      ldapClient.checkPassword.returns(BluebirdPromise.resolve());
       return PasswordResetFormPost.default(req as any, res as any)
         .then(function () {
           const authSession = AuthenticationSession.get(req as any);
@@ -111,8 +111,8 @@ describe("test reset password route", function () {
       req.body = {};
       req.body.password = "new-password";
 
-      ldap_client.bind.yields(undefined);
+      ldapClient.checkPassword.yields(undefined);
-      ldap_client.update_password.returns(BluebirdPromise.reject("Internal error with LDAP"));
+      ldapClient.updatePassword.returns(BluebirdPromise.reject("Internal error with LDAP"));
       res.send = sinon.spy(function () {
         assert.equal(res.status.getCall(0).args[0], 500);
         done();