From 0414d28e2b09215c98f5c0757461ee315c182104 Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Thu, 15 Jun 2017 00:22:16 +0200 Subject: [PATCH] Fix LDAP binding non working on servers with restricted ACL rules and add unit tests --- docker-compose.dev.yml | 7 - docker-compose.yml | 40 +++--- example/ldap/access.rules | 4 + src/server/lib/ErrorReplies.ts | 5 +- src/server/lib/LdapClient.ts | 131 ++++++++++-------- src/server/lib/routes/firstfactor/post.ts | 17 ++- .../lib/routes/password-reset/form/post.ts | 2 +- .../identity/PasswordResetHandler.ts | 2 +- src/types/ldapjs-async.d.ts | 1 + ...stence.test.ts => DataPersistence.test.ts} | 18 ++- test/server/LdapClient.test.ts | 119 ++++++++-------- test/server/Server.test.ts | 57 ++++---- ...er_config.test.ts => ServerConfig.test.ts} | 7 +- test/server/mocks/LdapClient.ts | 20 +-- test/server/mocks/ldapjs.ts | 2 + test/server/routes/firstfactor/post.test.ts | 73 +++++----- .../identity/PasswordResetHandler.test.ts | 8 +- .../server/routes/password-reset/post.test.ts | 14 +- 18 files changed, 283 insertions(+), 244 deletions(-) create mode 100644 example/ldap/access.rules rename test/server/{data_persistence.test.ts => DataPersistence.test.ts} (91%) rename test/server/{server_config.test.ts => ServerConfig.test.ts} (93%) diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 79b5208b5..765f50145 100644 --- a/docker-compose.dev.yml +++ b/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 diff --git a/docker-compose.yml b/docker-compose.yml index bfaaeb331..7245e3d6b 100644 --- a/docker-compose.yml +++ b/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 diff --git a/example/ldap/access.rules b/example/ldap/access.rules new file mode 100644 index 000000000..125b4ce9a --- /dev/null +++ b/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 diff --git a/src/server/lib/ErrorReplies.ts b/src/server/lib/ErrorReplies.ts index 98e0b575e..24a400310 100644 --- a/src/server/lib/ErrorReplies.ts +++ b/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) { - return function (err: Error) { +function replyWithError(res: express.Response, code: number, logger: Winston): (err: Error) => void { + return function (err: Error): void { logger.error("Reply with error %d: %s", code, err); res.status(code); res.send(); diff --git a/src/server/lib/LdapClient.ts b/src/server/lib/LdapClient.ts index 5ef545419..bf16a6d1b 100644 --- a/src/server/lib/LdapClient.ts +++ b/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 { - const ldap_client = this.ldapjs.createClient({ + private createClient(): ldapjs.ClientAsync { + 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 { - let user_name_attr = this.options.user_name_attribute; - // if not provided, default to cn - if (!user_name_attr) user_name_attr = "cn"; + connect(): BluebirdPromise { + const userDN = this.options.user; + const password = this.options.password; - 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); - if (additional_user_dn) user_dn += util.format(",%s", additional_user_dn); - user_dn += util.format(",%s", base_dn); - return user_dn; + let userDN = util.format("%s=%s", userNameAttribute, username); + if (additionalUserDN) userDN += util.format(",%s", additionalUserDN); + userDN += util.format(",%s", base_dn); + return userDN; } - bind(username: string, password: string): BluebirdPromise { - const user_dn = this.build_user_dn(username); + checkPassword(username: string, password: string): BluebirdPromise { + const userDN = this.buildUserDN(username); + const that = this; + const ldapClient = this.createClient(); - this.logger.debug("LDAP: Bind user %s", user_dn); - return this.client.bindAsync(user_dn, password) + this.logger.debug("LDAP: Check password for user '%s'", userDN); + 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 { - this.logger.debug("LDAP: Search for %s in %s", JSON.stringify(query), base); - return new BluebirdPromise((resolve, reject) => { - this.client.searchAsync(base, query) - .then(function (res: EventEmitter) { - const doc: SearchEntry[] = []; + private search(base: string, query: ldapjs.SearchOptions): BluebirdPromise { + const that = this; + + that.logger.debug("LDAP: Search for '%s' in '%s'", JSON.stringify(query), base); + return that.adminClient.searchAsync(base, query) + .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 { - const user_dn = this.build_user_dn(username); + retrieveGroups(username: string): BluebirdPromise { + const userDN = this.buildUserDN(username); + const password = this.options.password; - let group_name_attr = this.options.group_name_attribute; - if (!group_name_attr) group_name_attr = "cn"; + let groupNameAttribute = this.options.group_name_attribute; + 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; - if (additional_group_dn) - group_dn = util.format("%s,", additional_group_dn) + group_dn; + let groupDN = base_dn; + if (additionalGroupDN) + groupDN = util.format("%s,", additionalGroupDN) + groupDN; const query = { scope: "sub", - attributes: [group_name_attr], - filter: "member=" + user_dn + attributes: [groupNameAttribute], + 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 { + retrieveEmails(username: string): BluebirdPromise { 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); - return this.search_in_ldap(user_dn, query) + this.logger.debug("LDAP: get emails of user '%s'", username); + 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 { - const user_dn = this.build_user_dn(username); + updatePassword(username: string, newPassword: string): BluebirdPromise { + 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"); - return this.client.bindAsync(this.options.user, this.options.password) + that.logger.debug("LDAP: modify password"); + return that.adminClient.modifyAsync(user_dn, change) .then(function () { - that.logger.debug("LDAP: modify password"); - return that.client.modifyAsync(user_dn, change); + return that.adminClient.unbindAsync(); }); } } diff --git a/src/server/lib/routes/firstfactor/post.ts b/src/server/lib/routes/firstfactor/post.ts index 033f40dd1..aef3ff0fa 100644 --- a/src/server/lib/routes/firstfactor/post.ts +++ b/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)) diff --git a/src/server/lib/routes/password-reset/form/post.ts b/src/server/lib/routes/password-reset/form/post.ts index 9160b68c7..5a2cd45dd 100644 --- a/src/server/lib/routes/password-reset/form/post.ts +++ b/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); diff --git a/src/server/lib/routes/password-reset/identity/PasswordResetHandler.ts b/src/server/lib/routes/password-reset/identity/PasswordResetHandler.ts index e41a1e0de..898d55325 100644 --- a/src/server/lib/routes/password-reset/identity/PasswordResetHandler.ts +++ b/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"); diff --git a/src/types/ldapjs-async.d.ts b/src/types/ldapjs-async.d.ts index e5fad359e..669c15e64 100644 --- a/src/types/ldapjs-async.d.ts +++ b/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; + unbindAsync(): BluebirdPromise; searchAsync(base: string, query: ldapjs.SearchOptions): BluebirdPromise; modifyAsync(userdn: string, change: ldapjs.Change): BluebirdPromise; } diff --git a/test/server/data_persistence.test.ts b/test/server/DataPersistence.test.ts similarity index 91% rename from test/server/data_persistence.test.ts rename to test/server/DataPersistence.test.ts index 1beac313d..e6c33f7d6 100644 --- a/test/server/data_persistence.test.ts +++ b/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 = { - bind: sinon.stub(), - search: sinon.stub(), - on: sinon.spy() - }; + const ldapClient = LdapjsClientMock(); 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", - "password").yields(undefined); - ldap_client.bind.withArgs("cn=test_nok,ou=users,dc=example,dc=com", + ldapClient.bind.withArgs("cn=test_ok,ou=users,dc=example,dc=com", + "password").yields(); + 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 = { diff --git a/test/server/LdapClient.test.ts b/test/server/LdapClient.test.ts index 994c79439..8d1abe947 100644 --- a/test/server/LdapClient.test.ts +++ b/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 = { - bind: sinon.stub(), - search: sinon.stub(), - modify: sinon.stub(), - on: sinon.stub() - } as any; - + ldapClient = LdapjsClientMock(); 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); - return ldap.connect(); + ldap = new LdapClient.LdapClient(ldapConfig, ldapjs, winston); }); - 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_bind() { + function test_checking_password() { + 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(); - return test_bind(); + ldapClient.bind.yields(); + 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(); - return ldap.bind(username, password); + ldapClient.bind.withArgs("uid=user,ou=users,dc=example,dc=com").yields(); + 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(); - return ldap.bind(username, password); + ldapClient.bind.withArgs("cn=user,ou=users,dc=example,dc=com").yields(); + 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"); - const promise = test_bind(); + ldapClient.bind.yields("wrong credentials"); + 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"; - ldap_client.search.withArgs("uid=username,ou=users,dc=example,dc=com").yields(undefined, res_emitter); - return ldap.get_emails("username") + ldapConfig.user_name_attribute = "uid"; + ldapClient.search.withArgs("uid=username,ou=users,dc=example,dc=com").yields(undefined, res_emitter); + 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); - return ldap.get_groups("user") + ldapClient.search.yields(undefined, res_emitter); + 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"; - ldap_client.search.yields(undefined, res_emitter); - ldap.get_groups("user") + ldapConfig.additional_group_dn = "ou=groups"; + ldapClient.search.yields(undefined, res_emitter); + 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); - ldap.get_groups("user") + ldapClient.search.yields(undefined, res_emitter); + ldap.retrieveGroups("user") .then(function() { - assert.equal(ldap_client.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.deepEqual(ldap_client.search.getCall(0).args[1].attributes, ["cn"]); + assert.equal(ldapClient.search.getCall(0).args[0], "dc=example,dc=com"); + assert.equal(ldapClient.search.getCall(0).args[1].filter, "member=cn=user,ou=users,dc=example,dc=com"); + 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"); - return ldap.get_groups("user") + ldapClient.search.yields("error"); + 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); - ldap_client.modify.yields(undefined); + ldapClient.bind.yields(); + 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(ldap_client.modify.getCall(0).args[1].operation, change.operation); + assert.deepEqual(ldapClient.modify.getCall(0).args[0], userdn); + 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); - ldap_client.modify.yields("Error"); + ldapClient.bind.yields(undefined); + 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); - ldap_client.modify.withArgs("uid=username,ou=users,dc=example,dc=com").yields(); - return ldap.update_password("username", "newpass"); + ldapClient.bind.yields(); + ldapClient.unbind.yields(); + ldapClient.modify.withArgs("uid=username,ou=users,dc=example,dc=com").yields(); + return ldap.updatePassword("username", "newpass"); }); } }); diff --git a/test/server/Server.test.ts b/test/server/Server.test.ts index 0fa64f037..5b4673fee 100644 --- a/test/server/Server.test.ts +++ b/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 = { - bind: sinon.stub(), - search: sinon.stub(), - modify: sinon.stub(), - on: sinon.spy() - }; + const ldapClient = LdapjsClientMock(); 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", - "password").yields(undefined); - ldap_client.bind.withArgs("cn=admin,dc=example,dc=com", - "password").yields(undefined); + ldapClient.bind.withArgs("cn=test_ok,ou=users,dc=example,dc=com", + "password").yields(); + ldapClient.bind.withArgs("cn=admin,dc=example,dc=com", + "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); - ldap_client.search.yields(undefined, search_res); + ldapClient.unbind.yields(); + 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"); - return requests.totp(j, real_token); + assert.equal(res.statusCode, 302, "first factor failed"); + 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) { - console.error(err); - }); + .catch(function (err: Error) { return BluebirdPromise.reject(err); }); }); it("should return status code 204 when user is authenticated using u2f", function () { diff --git a/test/server/server_config.test.ts b/test/server/ServerConfig.test.ts similarity index 93% rename from test/server/server_config.test.ts rename to test/server/ServerConfig.test.ts index 83570f021..4773539af 100644 --- a/test/server/server_config.test.ts +++ b/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 diff --git a/test/server/mocks/LdapClient.ts b/test/server/mocks/LdapClient.ts index 17495c699..416071cbd 100644 --- a/test/server/mocks/LdapClient.ts +++ b/test/server/mocks/LdapClient.ts @@ -2,19 +2,19 @@ import sinon = require("sinon"); export interface LdapClientMock { - bind: sinon.SinonStub; - get_emails: sinon.SinonStub; - get_groups: sinon.SinonStub; - search_in_ldap: sinon.SinonStub; - update_password: sinon.SinonStub; + checkPassword: sinon.SinonStub; + retrieveEmails: sinon.SinonStub; + retrieveGroups: sinon.SinonStub; + search: sinon.SinonStub; + updatePassword: sinon.SinonStub; } export function LdapClientMock(): LdapClientMock { return { - bind: sinon.stub(), - get_emails: sinon.stub(), - get_groups: sinon.stub(), - search_in_ldap: sinon.stub(), - update_password: sinon.stub() + checkPassword: sinon.stub(), + retrieveEmails: sinon.stub(), + retrieveGroups: sinon.stub(), + search: sinon.stub(), + updatePassword: sinon.stub() }; } diff --git a/test/server/mocks/ldapjs.ts b/test/server/mocks/ldapjs.ts index 957f4a9eb..dcd1bacde 100644 --- a/test/server/mocks/ldapjs.ts +++ b/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() diff --git a/test/server/routes/firstfactor/post.test.ts b/test/server/routes/firstfactor/post.test.ts index e38bbec00..25baab59a 100644 --- a/test/server/routes/firstfactor/post.test.ts +++ b/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.get_emails.returns(BluebirdPromise.resolve(emails)); + ldapMock.checkPassword.withArgs("username").returns(BluebirdPromise.resolve()); + 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) { - res.redirect = sinon.spy(function () { done(); }); - ldapMock.bind.returns(BluebirdPromise.resolve()); - ldapMock.get_emails = sinon.stub().withArgs("username").returns(BluebirdPromise.resolve([{ mail: ["test@example.com"] }])); - FirstFactorPost.default(req as any, res as any); + it("should retrieve email from LDAP", function () { + ldapMock.checkPassword.returns(BluebirdPromise.resolve()); + ldapMock.retrieveEmails = sinon.stub().withArgs("username").returns(BluebirdPromise.resolve([{ mail: ["test@example.com"] }])); + return 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.get_emails.returns(BluebirdPromise.resolve(emails)); + ldapMock.checkPassword.returns(BluebirdPromise.resolve()); + 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) { - res.send = sinon.spy(function () { - assert.equal(401, res.status.getCall(0).args[0]); - assert.equal(regulator.mark.getCall(0).args[0], "username"); - done(); - }); - ldapMock.bind.returns(BluebirdPromise.reject(new exceptions.LdapBindError("Bad credentials"))); - FirstFactorPost.default(req as any, res as any); + it("should return status code 401 when LDAP binding throws", function () { + ldapMock.checkPassword.returns(BluebirdPromise.reject(new exceptions.LdapBindError("Bad credentials"))); + return FirstFactorPost.default(req as any, res as any) + .then(function () { + assert.equal(401, res.status.getCall(0).args[0]); + assert.equal(regulator.mark.getCall(0).args[0], "username"); + }); }); - it("should return status code 500 when LDAP search throws", function (done) { - res.send = sinon.spy(function () { - assert.equal(500, res.status.getCall(0).args[0]); - done(); - }); - ldapMock.bind.returns(BluebirdPromise.resolve()); - 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 500 when LDAP search throws", function () { + ldapMock.checkPassword.returns(BluebirdPromise.resolve()); + ldapMock.retrieveEmails.returns(BluebirdPromise.reject(new exceptions.LdapSearchError("error while retrieving emails"))); + return FirstFactorPost.default(req as any, res as any) + .then(function () { + assert.equal(500, res.status.getCall(0).args[0]); + }); }); - 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 () { - assert.equal(403, res.status.getCall(0).args[0]); - done(); - }); - ldapMock.bind.returns(BluebirdPromise.resolve()); - ldapMock.get_emails.returns(BluebirdPromise.resolve()); - FirstFactorPost.default(req as any, res as any); + it("should fail when admin user does not have rights to retrieve attribute mail", function () { + ldapMock.checkPassword.returns(BluebirdPromise.resolve()); + ldapMock.retrieveEmails = sinon.stub().withArgs("username").returns(BluebirdPromise.resolve([])); + ldapMock.retrieveGroups = sinon.stub().withArgs("username").returns(BluebirdPromise.resolve(["group1"])); + return FirstFactorPost.default(req as any, res as any) + .then(function () { + assert.equal(500, res.status.getCall(0).args[0]); + assert.equal(1, res.send.callCount); + }); }); }); diff --git a/test/server/routes/password-reset/identity/PasswordResetHandler.test.ts b/test/server/routes/password-reset/identity/PasswordResetHandler.test.ts index 3b90b8938..f69edf5b3 100644 --- a/test/server/routes/password-reset/identity/PasswordResetHandler.test.ts +++ b/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(); diff --git a/test/server/routes/password-reset/post.test.ts b/test/server/routes/password-reset/post.test.ts index 9548998d8..9362de8a5 100644 --- a/test/server/routes/password-reset/post.test.ts +++ b/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(); - mocks.ldap = ldap_client; + ldapClient = LdapClientMock(); + 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()); - ldap_client.bind.returns(BluebirdPromise.resolve()); + ldapClient.updatePassword.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); - ldap_client.update_password.returns(BluebirdPromise.reject("Internal error with LDAP")); + ldapClient.checkPassword.yields(undefined); + ldapClient.updatePassword.returns(BluebirdPromise.reject("Internal error with LDAP")); res.send = sinon.spy(function () { assert.equal(res.status.getCall(0).args[0], 500); done();