Reset password form sends 200 status when user does not exist

Reset password sends 200 status codes to avoid user enumeration.
This commit is contained in:
Clement Michaud 2017-11-09 00:12:38 +01:00
parent 792afbc476
commit f47d3c2b0b
4 changed files with 54 additions and 29 deletions

View File

@ -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));
};
}

View File

@ -26,13 +26,18 @@ export default class PasswordResetHandler implements IdentityValidable {
}
preValidationInit(req: express.Request): BluebirdPromise<Identity> {
const userid: string = objectPath.get<express.Request, string>(req, "query.userid");
const that = this;
const userid: string =
objectPath.get<express.Request, string>(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));
});
}

View File

@ -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);
});
});

View File

@ -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);
});
});
});