diff --git a/server/src/lib/IdentityCheckMiddleware.ts b/server/src/lib/IdentityCheckMiddleware.ts index 4ecbed863..4a6486595 100644 --- a/server/src/lib/IdentityCheckMiddleware.ts +++ b/server/src/lib/IdentityCheckMiddleware.ts @@ -144,6 +144,10 @@ export function get_start_validation(handler: IdentityValidable, handler.preValidationResponse(req, res); return BluebirdPromise.resolve(); }) + .catch(Exceptions.IdentityError, function (err: Error) { + handler.preValidationResponse(req, res); + return BluebirdPromise.resolve(); + }) .catch(ErrorReplies.replyWithError401(req, res, vars.logger)); }; } diff --git a/server/src/lib/routes/password-reset/identity/PasswordResetHandler.ts b/server/src/lib/routes/password-reset/identity/PasswordResetHandler.ts index 66458fb18..2baf2fc8d 100644 --- a/server/src/lib/routes/password-reset/identity/PasswordResetHandler.ts +++ b/server/src/lib/routes/password-reset/identity/PasswordResetHandler.ts @@ -26,13 +26,18 @@ export default class PasswordResetHandler implements IdentityValidable { } preValidationInit(req: express.Request): BluebirdPromise { - const userid: string = objectPath.get(req, "query.userid"); + const that = this; + const userid: string = + objectPath.get(req, "query.userid"); + return BluebirdPromise.resolve() + .then(function () { + that.logger.debug(req, "User '%s' requested a password reset", userid); + if (!userid) + return BluebirdPromise.reject( + new exceptions.AccessDeniedError("No user id provided")); - this.logger.debug(req, "User '%s' requested a password reset", userid); - if (!userid) - return BluebirdPromise.reject(new exceptions.AccessDeniedError("No user id provided")); - - return this.emailsRetriever.retrieve(userid) + return that.emailsRetriever.retrieve(userid); + }) .then(function (emails: string[]) { if (!emails && emails.length <= 0) throw new Error("No email found"); const identity = { @@ -40,6 +45,9 @@ export default class PasswordResetHandler implements IdentityValidable { userid: userid }; return BluebirdPromise.resolve(identity); + }) + .catch(function (err: Error) { + return BluebirdPromise.reject(new exceptions.IdentityError(err.message)); }); } diff --git a/server/test/IdentityCheckMiddleware.test.ts b/server/test/IdentityCheckMiddleware.test.ts index 9ed4f47de..0dbd159a0 100644 --- a/server/test/IdentityCheckMiddleware.test.ts +++ b/server/test/IdentityCheckMiddleware.test.ts @@ -16,6 +16,8 @@ import IdentityValidatorMock = require("./mocks/IdentityValidator"); import { RequestLoggerStub } from "./mocks/RequestLoggerStub"; import { ServerVariablesMock, ServerVariablesMockBuilder } from "./mocks/ServerVariablesMockBuilder"; +import { PRE_VALIDATION_TEMPLATE } + from "../src/lib/IdentityCheckPreValidationTemplate"; describe("test identity check process", function () { @@ -77,7 +79,8 @@ throws a first factor error", function () { }); }); - it("should send 401 if email is missing in provided identity", function () { + // In that case we answer with 200 to avoid user enumeration. + it("should send 200 if email is missing in provided identity", function () { const identity = { userid: "abc" }; identityValidable.preValidationInit @@ -87,14 +90,12 @@ throws a first factor error", function () { return callback(req as any, res as any, undefined) .then(function () { - return BluebirdPromise.reject("Should fail"); - }) - .catch(function () { - Assert(res.redirect.calledWith("/error/401")); + Assert(identityValidable.preValidationResponse.called); }); }); - it("should send 401 if userid is missing in provided identity", + // In that case we answer with 200 to avoid user enumeration. + it("should send 200 if userid is missing in provided identity", function () { const endpoint = "/protected"; const identity = { email: "abc@example.com" }; @@ -106,10 +107,7 @@ throws a first factor error", function () { return callback(req as any, res as any, undefined) .then(function () { - return BluebirdPromise.reject(new Error("It should fail")); - }) - .catch(function (err: Error) { - Assert(res.redirect.calledWith("/error/401")); + Assert(identityValidable.preValidationResponse.called); }); }); diff --git a/server/test/routes/password-reset/identity/PasswordResetHandler.test.ts b/server/test/routes/password-reset/identity/PasswordResetHandler.test.ts index eeaf693fa..f7d906818 100644 --- a/server/test/routes/password-reset/identity/PasswordResetHandler.test.ts +++ b/server/test/routes/password-reset/identity/PasswordResetHandler.test.ts @@ -1,5 +1,6 @@ -import PasswordResetHandler from "../../../../src/lib/routes/password-reset/identity/PasswordResetHandler"; +import PasswordResetHandler + from "../../../../src/lib/routes/password-reset/identity/PasswordResetHandler"; import PasswordUpdater = require("../../../../src/lib/ldap/PasswordUpdater"); import { UserDataStore } from "../../../../src/lib/storage/UserDataStore"; import Sinon = require("sinon"); @@ -7,7 +8,8 @@ import winston = require("winston"); import assert = require("assert"); import BluebirdPromise = require("bluebird"); import ExpressMock = require("../../../mocks/express"); -import { ServerVariablesMock, ServerVariablesMockBuilder } from "../../../mocks/ServerVariablesMockBuilder"; +import { ServerVariablesMock, ServerVariablesMockBuilder } + from "../../../mocks/ServerVariablesMockBuilder"; import { ServerVariables } from "../../../../src/lib/ServerVariables"; describe("test reset password identity check", function () { @@ -43,36 +45,49 @@ describe("test reset password identity check", function () { mocks = s.mocks; vars = s.variables; - mocks.userDataStore.saveU2FRegistrationStub.returns(BluebirdPromise.resolve({})); - mocks.userDataStore.retrieveU2FRegistrationStub.returns(BluebirdPromise.resolve({})); - mocks.userDataStore.produceIdentityValidationTokenStub.returns(BluebirdPromise.resolve({})); - mocks.userDataStore.consumeIdentityValidationTokenStub.returns(BluebirdPromise.resolve({})); + mocks.userDataStore.saveU2FRegistrationStub + .returns(BluebirdPromise.resolve({})); + mocks.userDataStore.retrieveU2FRegistrationStub + .returns(BluebirdPromise.resolve({})); + mocks.userDataStore.produceIdentityValidationTokenStub + .returns(BluebirdPromise.resolve({})); + mocks.userDataStore.consumeIdentityValidationTokenStub + .returns(BluebirdPromise.resolve({})); res = ExpressMock.ResponseMock(); }); describe("test reset password identity pre check", () => { it("should fail when no userid is provided", function () { req.query.userid = undefined; - const handler = new PasswordResetHandler(vars.logger, vars.ldapEmailsRetriever); + const handler = new PasswordResetHandler(vars.logger, + vars.ldapEmailsRetriever); return handler.preValidationInit(req as any) - .then(function () { return BluebirdPromise.reject("It should fail"); }) + .then(function () { + return BluebirdPromise.reject("It should fail"); + }) .catch(function (err: Error) { return BluebirdPromise.resolve(); }); }); it("should fail if ldap fail", function () { - mocks.ldapEmailsRetriever.retrieveStub.returns(BluebirdPromise.reject("Internal error")); - new PasswordResetHandler(vars.logger, vars.ldapEmailsRetriever).preValidationInit(req as any) - .then(function () { return BluebirdPromise.reject(new Error("should not be here")); }, + mocks.ldapEmailsRetriever.retrieveStub + .returns(BluebirdPromise.reject("Internal error")); + new PasswordResetHandler(vars.logger, vars.ldapEmailsRetriever) + .preValidationInit(req as any) + .then(function () { + return BluebirdPromise.reject(new Error("should not be here")); + }, function (err: Error) { return BluebirdPromise.resolve(); }); }); it("should returns identity when ldap replies", function () { - mocks.ldapEmailsRetriever.retrieveStub.returns(BluebirdPromise.resolve(["test@example.com"])); - return new PasswordResetHandler(vars.logger, vars.ldapEmailsRetriever).preValidationInit(req as any); + mocks.ldapEmailsRetriever.retrieveStub + .returns(BluebirdPromise.resolve(["test@example.com"])); + return new PasswordResetHandler(vars.logger, vars.ldapEmailsRetriever) + .preValidationInit(req as any); }); }); });