Skip to content

Commit

Permalink
Merge pull request #352 from OpenPathSec/master
Browse files Browse the repository at this point in the history
Fix for #328 - treat JWT.decode() null return the same as if it threw an exception
  • Loading branch information
nelsonic authored Sep 8, 2020
2 parents 4e168c2 + c09fdb2 commit 7a457cb
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 1 deletion.
13 changes: 13 additions & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,14 @@ internals.authenticate = async function(token, options, request, h) {
try {
decoded = JWT.decode(token, { complete: options.complete || false });
} catch (e) {
// fix for https://github.com/dwyl/hapi-auth-jwt2/issues/328 -
// JWT.decode() can fail either by throwing an exception or by
// returning null, so here we just fall through to the following
// block that tests if decoded is not set, so that we can handle
// both failure types at once
}

if (!decoded) {
return {
error: internals.raiseError(
options,
Expand Down Expand Up @@ -226,6 +234,11 @@ internals.authenticate = async function(token, options, request, h) {
}
// see: https://github.com/dwyl/hapi-auth-jwt2/issues/130
try {
// note: at this point, we know options.verify must be non-null,
// because options.validate or options.verify are required to have
// been provided, and if options.validate were non-null, then we
// would have hit the above block and already returned out of this
// function
let { isValid, credentials } = await options.verify(decoded, request);
if (!isValid) {
return {
Expand Down
6 changes: 5 additions & 1 deletion test/basic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ test("Malformed JWT", async function(t) {
// console.log(response.result);
// console.log(' '); // blank line
t.equal(response.statusCode, 401, "INVALID Token should fail");
// t.equal(JSON.parse(response.result).msg, 'Invalid Token', "INVALID Token should fail");

// assert on the response message, so we can ensure this case fails
// early (after decode()) with "Invalid token format" instead of too
// late (after verify) with "Invalid token"
t.equal(response.result.message, 'Invalid token format', 'Message should be "Invalid token format"');
t.end();
});

Expand Down
22 changes: 22 additions & 0 deletions test/verify_func.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,27 @@ test("Access a route that has no auth strategy", async function(t) {
t.end();
});

test("customVerify malformed JWT", async function(t) {
// this test verifies the fix for
// https://github.com/dwyl/hapi-auth-jwt2/issues/328

const options = {
method: "GET",
url: "/required",
headers: { authorization: "Bearer my.invalid.token" }
};
// server.inject lets us simulate an http request
const response = await server.inject(options);
// console.log(response.result);
t.equal(response.statusCode, 401, "INVALID Token should fail");

// assert on the response message, so we can ensure this case fails
// early (after decode()) with "Invalid token format" before it ever
// even attempts to call our customVerify function
t.equal(response.result.message, 'Invalid token format', "INVALID Token should fail");
t.end();
});

test("customVerify simulate error condition", async function(t) {
const payload = { id: 123, "name": "Charlie", error: true }
const token = JWT.sign(payload, 'SecretDoesNOTGetVerified');
Expand All @@ -40,6 +61,7 @@ test("customVerify with fail condition", async function(t) {
// server.inject lets us simulate an http request
const response = await server.inject(options);
t.equal(response.statusCode, 401, "GET /required with customVerify rejected");
t.equal(response.result.message, 'Invalid credentials', "GET /required with customVerify rejected");
t.end();
});

Expand Down

0 comments on commit 7a457cb

Please sign in to comment.