From 9fddcc7e93c26372205c048cae02a2d9c1451977 Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Sun, 21 May 2017 23:32:09 +0200 Subject: [PATCH] Fix issue with domain access during first factor phase --- src/lib/Exceptions.ts | 8 ++++ src/lib/routes/AuthenticationValidator.ts | 14 +++++- src/lib/routes/FirstFactor.ts | 28 +++++++----- .../routes/AuthenticationValidator.test.ts | 14 ++++++ test/unitary/routes/FirstFactor.test.ts | 43 ++++++++++--------- 5 files changed, 74 insertions(+), 33 deletions(-) diff --git a/src/lib/Exceptions.ts b/src/lib/Exceptions.ts index 38d054c0..56fcbb7d 100644 --- a/src/lib/Exceptions.ts +++ b/src/lib/Exceptions.ts @@ -46,3 +46,11 @@ export class InvalidTOTPError extends Error { Object.setPrototypeOf(this, InvalidTOTPError.prototype); } } + +export class DomainAccessDenied extends Error { + constructor(message?: string) { + super(message); + this.name = "DomainAccessDenied"; + Object.setPrototypeOf(this, DomainAccessDenied.prototype); + } +} diff --git a/src/lib/routes/AuthenticationValidator.ts b/src/lib/routes/AuthenticationValidator.ts index 38b861da..d5ae1178 100644 --- a/src/lib/routes/AuthenticationValidator.ts +++ b/src/lib/routes/AuthenticationValidator.ts @@ -2,9 +2,12 @@ import objectPath = require("object-path"); import BluebirdPromise = require("bluebird"); import express = require("express"); +import AccessController from "../access_control/AccessController"; +import exceptions = require("../Exceptions"); function verify_filter(req: express.Request, res: express.Response) { const logger = req.app.get("logger"); + const accessController: AccessController = req.app.get("access controller"); if (!objectPath.has(req, "session.auth_session")) return BluebirdPromise.reject("No auth_session variable"); @@ -18,17 +21,24 @@ function verify_filter(req: express.Request, res: express.Response) { if (!objectPath.has(req, "session.auth_session.userid")) return BluebirdPromise.reject("No userid variable"); + const username = objectPath.get(req, "session.auth_session.userid"); + const groups = objectPath.get(req, "session.auth_session.groups"); + const host = objectPath.get(req, "headers.host"); const domain = host.split(":")[0]; + const isAllowed = accessController.isDomainAllowedForUser(domain, username, groups); + if (!isAllowed) return BluebirdPromise.reject( + new exceptions.DomainAccessDenied("User '" + username + "' does not have access to " + domain)); + if (!req.session.auth_session.first_factor || !req.session.auth_session.second_factor) - return BluebirdPromise.reject("First or second factor not validated"); + return BluebirdPromise.reject(new exceptions.AccessDeniedError("First or second factor not validated")); return BluebirdPromise.resolve(); } -export = function(req: express.Request, res: express.Response) { +export = function (req: express.Request, res: express.Response) { verify_filter(req, res) .then(function () { res.status(204); diff --git a/src/lib/routes/FirstFactor.ts b/src/lib/routes/FirstFactor.ts index 3a67f468..7d33afc9 100644 --- a/src/lib/routes/FirstFactor.ts +++ b/src/lib/routes/FirstFactor.ts @@ -3,10 +3,13 @@ import exceptions = require("../Exceptions"); import objectPath = require("object-path"); import BluebirdPromise = require("bluebird"); import express = require("express"); +import AccessController from "../access_control/AccessController"; +import AuthenticationRegulator from "../AuthenticationRegulator"; +import { LdapClient } from "../LdapClient"; export = function (req: express.Request, res: express.Response) { - const username = req.body.username; - const password = req.body.password; + const username: string = req.body.username; + const password: string = req.body.password; if (!username || !password) { res.status(401); res.send(); @@ -14,10 +17,10 @@ export = function (req: express.Request, res: express.Response) { } const logger = req.app.get("logger"); - const ldap = req.app.get("ldap"); + const ldap: LdapClient = req.app.get("ldap"); const config = req.app.get("config"); - const regulator = req.app.get("authentication regulator"); - const accessController = req.app.get("access controller"); + const regulator: AuthenticationRegulator = req.app.get("authentication regulator"); + const accessController: AccessController = req.app.get("access controller"); logger.info("1st factor: Starting authentication of user \"%s\"", username); logger.debug("1st factor: Start bind operation against LDAP"); @@ -34,16 +37,14 @@ export = function (req: express.Request, res: express.Response) { logger.debug("1st factor: Retrieve email from LDAP"); return BluebirdPromise.join(ldap.get_emails(username), ldap.get_groups(username)); }) - .then(function (data: string[2]) { - const emails = data[0]; - const groups = data[1]; + .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"); logger.debug("1st factor: Retrieved email are %s", emails); objectPath.set(req, "session.auth_session.email", emails[0]); - - const isAllowed = accessController.isDomainAllowedForUser(username, groups); - if (!isAllowed) throw new Error("User not allowed to visit this domain"); + objectPath.set(req, "session.auth_session.groups", groups); regulator.mark(username, true); res.status(204); @@ -67,6 +68,11 @@ export = function (req: express.Request, res: express.Response) { res.status(403); res.send("Access has been restricted for a few minutes..."); }) + .catch(exceptions.DomainAccessDenied, (err: Error) => { + logger.error("1st factor: ", err); + res.status(401); + res.send("Access denied..."); + }) .catch(function (err: Error) { console.log(err.stack); logger.error("1st factor: Unhandled error %s", err); diff --git a/test/unitary/routes/AuthenticationValidator.test.ts b/test/unitary/routes/AuthenticationValidator.test.ts index 8d54c359..33f27f4a 100644 --- a/test/unitary/routes/AuthenticationValidator.test.ts +++ b/test/unitary/routes/AuthenticationValidator.test.ts @@ -105,6 +105,20 @@ describe("test authentication token verification", function () { it("should not be authenticated when session is partially initialized", function () { return test_unauthorized({ first_factor: true }); }); + + it.only("should not be authenticated when domain is not allowed for user", function () { + req.headers.host = "test.example.com"; + + accessController.isDomainAllowedForUser.returns(false); + accessController.isDomainAllowedForUser.withArgs("test.example.com", "user", ["group1", "group2"]).returns(true); + + return test_authorized({ + first_factor: true, + second_factor: true, + userid: "user", + groups: ["group1", "group2"] + }); + }); }); }); diff --git a/test/unitary/routes/FirstFactor.test.ts b/test/unitary/routes/FirstFactor.test.ts index 8c48a0eb..0ee5b07e 100644 --- a/test/unitary/routes/FirstFactor.test.ts +++ b/test/unitary/routes/FirstFactor.test.ts @@ -11,7 +11,7 @@ import AccessControllerMock = require("../mocks/AccessController"); import { LdapClientMock } from "../mocks/LdapClient"; import ExpressMock = require("../mocks/express"); -describe("test the first factor validation route", function() { +describe("test the first factor validation route", function () { let req: ExpressMock.RequestMock; let res: ExpressMock.ResponseMock; let emails: string[]; @@ -21,7 +21,7 @@ describe("test the first factor validation route", function() { let regulator: AuthenticationRegulatorMock.AuthenticationRegulatorMock; let accessController: AccessControllerMock.AccessControllerMock; - beforeEach(function() { + beforeEach(function () { configuration = { ldap: { base_dn: "ou=users,dc=example,dc=com", @@ -29,8 +29,8 @@ describe("test the first factor validation route", function() { } }; - emails = [ "test_ok@example.com" ]; - groups = [ "group1", "group2" ]; + emails = ["test_ok@example.com"]; + groups = ["group1", "group2" ]; ldapMock = LdapClientMock(); @@ -61,14 +61,17 @@ describe("test the first factor validation route", function() { FirstFactor: false, second_factor: false } + }, + headers: { + host: "home.example.com" } }; res = ExpressMock.ResponseMock(); }); - it("should return status code 204 when LDAP binding succeeds", function() { - return new Promise(function(resolve, reject) { - res.send = sinon.spy(function() { + it("should return status code 204 when LDAP binding succeeds", function () { + return new Promise(function (resolve, reject) { + res.send = sinon.spy(function () { assert.equal("username", req.session.auth_session.userid); assert.equal(204, res.status.getCall(0).args[0]); resolve(); @@ -79,28 +82,28 @@ describe("test the first factor validation route", function() { }); }); - it("should retrieve email from LDAP", function(done) { - res.send = sinon.spy(function() { done(); }); + it("should retrieve email from LDAP", function (done) { + res.send = sinon.spy(function () { done(); }); ldapMock.bind.returns(BluebirdPromise.resolve()); - ldapMock.get_emails = 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"] }])); FirstFactor(req as any, res as any); }); - it("should set email as session variables", function() { - return new Promise(function(resolve, reject) { - res.send = sinon.spy(function() { + it("should set email as session variables", function () { + return new Promise(function (resolve, reject) { + res.send = sinon.spy(function () { assert.equal("test_ok@example.com", req.session.auth_session.email); resolve(); }); - const emails = [ "test_ok@example.com" ]; + const emails = ["test_ok@example.com"]; ldapMock.bind.returns(BluebirdPromise.resolve()); ldapMock.get_emails.returns(BluebirdPromise.resolve(emails)); FirstFactor(req as any, res as any); }); }); - it("should return status code 401 when LDAP binding throws", function(done) { - res.send = sinon.spy(function() { + 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(); @@ -109,8 +112,8 @@ describe("test the first factor validation route", function() { FirstFactor(req as any, res as any); }); - it("should return status code 500 when LDAP search throws", function(done) { - res.send = sinon.spy(function() { + 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(); }); @@ -119,11 +122,11 @@ describe("test the first factor validation route", function() { FirstFactor(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 (done) { const err = new exceptions.AuthenticationRegulationError("Authentication regulation..."); regulator.regulate.returns(BluebirdPromise.reject(err)); - res.send = sinon.spy(function() { + res.send = sinon.spy(function () { assert.equal(403, res.status.getCall(0).args[0]); done(); });