Skip to content

Commit

Permalink
Update middleware to throw when token is invalid when credentials are…
Browse files Browse the repository at this point in the history
…n't required
  • Loading branch information
johnnyhalife committed Aug 2, 2016
1 parent 15da043 commit fd58e89
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 13 deletions.
2 changes: 1 addition & 1 deletion lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ module.exports = function(options) {
},
function verifyToken(secret, callback) {
jwt.verify(token, secret, options, function(err, decoded) {
if (err && credentialsRequired) {
if (err) {
callback(new UnauthorizedError('invalid_token', err));
} else {
callback(null, decoded);
Expand Down
38 changes: 26 additions & 12 deletions test/jwt.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,32 @@ describe('failure tests', function () {
});
});

it('should throw error if token is expired even with when credentials are not required', function() {
var secret = 'shhhhhh';
var token = jwt.sign({foo: 'bar', exp: 1382412921}, secret);

req.headers = {};
req.headers.authorization = 'Bearer ' + token;
expressjwt({ secret: secret, credentialsRequired: false })(req, res, function(err) {
assert.ok(err);
assert.equal(err.code, 'invalid_token');
assert.equal(err.message, 'jwt expired');
});
});

it('should throw error if token is invalid even with when credentials are not required', function() {
var secret = 'shhhhhh';
var token = jwt.sign({foo: 'bar', exp: 1382412921}, secret);

req.headers = {};
req.headers.authorization = 'Bearer ' + token;
expressjwt({ secret: "not the secret", credentialsRequired: false })(req, res, function(err) {
assert.ok(err);
assert.equal(err.code, 'invalid_token');
assert.equal(err.message, 'invalid signature');
});
});

});

describe('work tests', function () {
Expand Down Expand Up @@ -216,18 +242,6 @@ describe('work tests', function () {
});
});

it('should work if token is expired and credentials are not required', function() {
var secret = 'shhhhhh';
var token = jwt.sign({foo: 'bar', exp: 1382412921}, secret);

req.headers = {};
req.headers.authorization = 'Bearer ' + token;
expressjwt({ secret: secret, credentialsRequired: false })(req, res, function(err) {
assert(typeof err === 'undefined');
assert(typeof req.user === 'undefined')
});
});

it('should not work if no authorization header', function() {
req = {};
expressjwt({ secret: 'shhhh' })(req, res, function(err) {
Expand Down

3 comments on commit fd58e89

@zoellner
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnnyhalife
What is the reason behind this change?
This causes some issues for us since we have multiple layers of middleware, some with different secrets.
With this change the first middleware that encounters an authorization header with a different secret fails. Previously it worked fine since credentialsRequired is set to false on those middleware layers.

@johnnyhalife
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes come from this PR, after having a conversation with @JfromA (#127) please read the thread.

@DanielMSchmidt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the readme be update on this topic?

Please sign in to comment.