-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Secret callback revisited #480
Changes from 5 commits
96cb28b
798b033
8d96f89
ddb1736
7d60544
6199e60
7a39153
fdfb2ef
e67f5d3
2e27e84
f83432f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
[*] | ||
indent_style = space | ||
indent_size = 2 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ $ npm install jsonwebtoken | |
|
||
`secretOrPrivateKey` is a string, buffer, or object containing either the secret for HMAC algorithms or the PEM | ||
encoded private key for RSA and ECDSA. In case of a private key with passphrase an object `{ key, passphrase }` can be used (based on [crypto documentation](https://nodejs.org/api/crypto.html#crypto_sign_sign_private_key_output_format)), in this case be sure you pass the `algorithm` option. | ||
If `jwt.verify` is called asynchronous, `secretOrPublicKey` can be a function that should fetch the secret or public key. See below for a detailed example | ||
|
||
`options`: | ||
|
||
|
@@ -197,6 +198,17 @@ jwt.verify(token, cert, { algorithms: ['RS256'] }, function (err, payload) { | |
// if token alg != RS256, err == invalid signature | ||
}); | ||
|
||
// Verify using getKey callback | ||
function getKey(header, callback) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In order to use this function for JWK + openid cases we would need to add the issuer as well. The options are:
What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the sake of progress, i would like to keep it as is. Let's make an separate issue for it and i will pick it up after this one ;-) |
||
// fetch secret or public key | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd add some comment encouraging the users to cache the key with the kid so they avoid fetching it multiple times. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will add https://github.com/auth0/node-jwks-rsa as an example to load the keys, that should make it more clear. |
||
const key = ...; | ||
callback(null, key); | ||
} | ||
|
||
verify(token, getKey, options, function(err, decoded) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. jwt.verify |
||
console.log(decoded.foo) // bar | ||
}); | ||
|
||
``` | ||
|
||
### jwt.decode(token [, options]) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,10 @@ var jws = require('jws'); | |
var fs = require('fs'); | ||
var path = require('path'); | ||
var sinon = require('sinon'); | ||
var JsonWebTokenError = require('../lib/JsonWebTokenError'); | ||
|
||
var assert = require('chai').assert; | ||
var expect = require('chai').expect; | ||
|
||
describe('verify', function() { | ||
var pub = fs.readFileSync(path.join(__dirname, 'pub.pem')); | ||
|
@@ -67,6 +69,88 @@ describe('verify', function() { | |
}); | ||
}); | ||
|
||
describe('secret or token as callback', function () { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function is not properly indented with the rest of the file and doesn't follow the newly introduced .editorconfig There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to make this a thing with #486 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with @jfromaniello on this one. Since the bad indentation is introduced in this PR there's no need to wait for a different one to fix it. |
||
var token = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJmb28iOiJiYXIiLCJpYXQiOjE0MzcwMTg1ODIsImV4cCI6MTQzNzAxODU5Mn0.3aR3vocmgRpG05rsI9MpR6z2T_BGtMQaPq2YR6QaroU'; | ||
var key = 'key'; | ||
|
||
var payload = { foo: 'bar', iat: 1437018582, exp: 1437018592 }; | ||
var options = {algorithms: ['HS256'], ignoreExpiration: true}; | ||
|
||
it('without callback', function (done) { | ||
jwt.verify(token, key, options, function (err, p) { | ||
assert.isNull(err); | ||
assert.deepEqual(p, payload); | ||
done(); | ||
}); | ||
}); | ||
|
||
it('simple callback', function (done) { | ||
var keyFunc = function(header, callback) { | ||
callback(undefined, key); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just saw that even when you are passing the |
||
}; | ||
|
||
jwt.verify(token, keyFunc, options, function (err, p) { | ||
assert.isNull(err); | ||
assert.deepEqual(p, payload); | ||
done(); | ||
}); | ||
}); | ||
|
||
it('should error if called synchronously', function (done) { | ||
var keyFunc = function(header, callback) { | ||
callback(undefined, key); | ||
}; | ||
|
||
expect(function () { | ||
jwt.verify(token, keyFunc, options); | ||
}).to.throw(JsonWebTokenError, /verify must be called asynchronous if secret or public key is provided as a callback/); | ||
|
||
done(); | ||
}); | ||
|
||
it('simple error', function (done) { | ||
var keyFunc = function(header, callback) { | ||
callback(new Error('key not found')); | ||
}; | ||
|
||
jwt.verify(token, keyFunc, options, function (err, p) { | ||
assert.equal(err.name, 'JsonWebTokenError'); | ||
assert.match(err.message, /error in secret or public key callback/); | ||
assert.isUndefined(p); | ||
done(); | ||
}); | ||
}); | ||
|
||
it('delayed callback', function (done) { | ||
var keyFunc = function(header, callback) { | ||
setTimeout(function() { | ||
callback(undefined, key); | ||
}, 25); | ||
}; | ||
|
||
jwt.verify(token, keyFunc, options, function (err, p) { | ||
assert.isNull(err); | ||
assert.deepEqual(p, payload); | ||
done(); | ||
}); | ||
}); | ||
|
||
it('delayed error', function (done) { | ||
var keyFunc = function(header, callback) { | ||
setTimeout(function() { | ||
callback(new Error('key not found')); | ||
}, 25); | ||
}; | ||
|
||
jwt.verify(token, keyFunc, options, function (err, p) { | ||
assert.equal(err.name, 'JsonWebTokenError'); | ||
assert.match(err.message, /error in secret or public key callback/); | ||
assert.isUndefined(p); | ||
done(); | ||
}); | ||
}); | ||
}); | ||
|
||
describe('expiration', function () { | ||
// { foo: 'bar', iat: 1437018582, exp: 1437018592 } | ||
var token = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJmb28iOiJiYXIiLCJpYXQiOjE0MzcwMTg1ODIsImV4cCI6MTQzNzAxODU5Mn0.3aR3vocmgRpG05rsI9MpR6z2T_BGtMQaPq2YR6QaroU'; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is in wrong place, right? it should be under
jwt.verify
section, where we should indicatesecretOrPublicKey
can be a function too.