Skip to content
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

fix: Async key provider and errors should be resolved internally #335

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 29 additions & 21 deletions jwt.js
Original file line number Diff line number Diff line change
Expand Up @@ -487,34 +487,21 @@ function fastifyJwt (fastify, options, next) {
},
function verify (secretOrPublicKey, callback) {
try {
let verifyResult
if (useLocalVerifier) {
const verifierOptions = mergeOptionsWithKey(options.verify || options, secretOrPublicKey)
const localVerifier = createVerifier(verifierOptions)
const verifyResult = localVerifier(token)
callback(null, verifyResult)
verifyResult = localVerifier(token)
} else {
verifyResult = verifier(token)
}
if (verifyResult && typeof verifyResult.then === 'function') {
verifyResult.then(result => callback(null, result), error => wrapError(error, callback))
} else {
const verifyResult = verifier(token)
callback(null, verifyResult)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

verifyResult moved in order to support promise based result.

}
} catch (error) {
if (error.code === TokenError.codes.expired) {
return callback(new AuthorizationTokenExpiredError())
}

if (error.code === TokenError.codes.invalidKey ||
error.code === TokenError.codes.invalidSignature ||
error.code === TokenError.codes.invalidClaimValue
) {
return callback(typeof messagesOptions.authorizationTokenInvalid === 'function'
? new AuthorizationTokenInvalidError(error.message)
: new AuthorizationTokenInvalidError())
}

if (error.code === TokenError.codes.missingSignature) {
return callback(new AuthorizationTokenUnsignedError())
}

return callback(error)
mcollina marked this conversation as resolved.
Show resolved Hide resolved
return wrapError(error, callback)
}
},
function checkIfIsTrusted (result, callback) {
Expand Down Expand Up @@ -543,6 +530,27 @@ function fastifyJwt (fastify, options, next) {
}
})
}

function wrapError (error, callback) {
if (error.code === TokenError.codes.expired) {
return callback(new AuthorizationTokenExpiredError())
}

if (error.code === TokenError.codes.invalidKey ||
error.code === TokenError.codes.invalidSignature ||
error.code === TokenError.codes.invalidClaimValue
) {
return callback(typeof messagesOptions.authorizationTokenInvalid === 'function'
? new AuthorizationTokenInvalidError(error.message)
: new AuthorizationTokenInvalidError())
}

if (error.code === TokenError.codes.missingSignature) {
return callback(new AuthorizationTokenUnsignedError())
}

return callback(error)
}
}
mcollina marked this conversation as resolved.
Show resolved Hide resolved

module.exports = fp(fastifyJwt, {
Expand Down
130 changes: 130 additions & 0 deletions test/jwt-async.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
'use strict'

const test = require('tap').test
const Fastify = require('fastify')
const jwt = require('../jwt')

test('Async key provider should be resolved internally', async function (t) {
const fastify = Fastify()
fastify.register(jwt, {
secret: {
private: 'supersecret',
public: async () => Promise.resolve('supersecret')
},
verify: {
extractToken: (request) => request.headers.jwt,
key: () => Promise.resolve('supersecret')
}
})
fastify.get('/', async function (request, reply) {
const token = await reply.jwtSign({ user: 'test' })
request.headers.jwt = token
await request.jwtVerify()
return reply.send(request.user)
})
const response = await fastify.inject({
method: 'get',
url: '/',
headers: {
jwt: 'supersecret'
}
})
t.ok(response)
t.comment("Should be 'undefined'")
t.match(response.json(), { user: 'test' })
})

test('Async key provider errors should be resolved internally', async function (t) {
const fastify = Fastify()
fastify.register(jwt, {
secret: {
public: async () => Promise.resolve('key used per request, false not allowed')
},
verify: {
extractToken: (request) => request.headers.jwt,
key: () => Promise.resolve('key not used')
}
})
fastify.get('/', async function (request, reply) {
request.headers.jwt =
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c'
Copy link
Member

Choose a reason for hiding this comment

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

please generate this dynamically in code.

// call to local verifier without cache
await request.jwtVerify()
return reply.send(typeof request.user.then)
})
const response = await fastify.inject({
method: 'get',
url: '/'
})

t.equal(response.statusCode, 401)
})

test('Async key provider should be resolved internally with cache', async function (t) {
const fastify = Fastify()
fastify.register(jwt, {
secret: {
public: async () => false
},
verify: {
extractToken: (request) => request.headers.jwt,
key: () => Promise.resolve('this secret reused from cache')
}
})
fastify.get('/', async function (request, reply) {
request.headers.jwt = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.hQOxra1Zo9z61vCqe6_86kVfLqKI0WxDnkiJ_upW0sM'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hardcoded request.headers.jwt is for key=this secret reused from cache
Generated with https://jwt.io/

Copy link
Member

Choose a reason for hiding this comment

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

please generate this dynamically in code.

await new Promise((resolve, reject) => request.jwtVerify((err, payload) => {
if (err) {
reject(err)
return
}
resolve(payload)
}))
await new Promise((resolve, reject) => request.jwtVerify((err, payload) => {
if (err) {
reject(err)
return
}
resolve(payload)
}))
return reply.send(request.user)
})
mcollina marked this conversation as resolved.
Show resolved Hide resolved
const response = await fastify.inject({
method: 'get',
url: '/'
})
t.equal(response.statusCode, 200)
t.match(response.json(), { name: 'John Doe' })
})

test('Async key provider errors should be resolved internally with cache', async function (t) {
const fastify = Fastify()
fastify.register(jwt, {
secret: {
public: async () => false
},
verify: {
extractToken: (request) => request.headers.jwt,
key: () => Promise.resolve('this secret reused from cache')
}
})
fastify.get('/', async function (request, reply) {
request.headers.jwt =
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c'
Copy link
Member

Choose a reason for hiding this comment

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

please generate this dynamically in code.

// call to plugin root level verifier
await new Promise((resolve, reject) => request.jwtVerify((err, payload) => {
if (err) {
reject(err)
return
}
resolve(payload)
}))
return reply.send(typeof request.user.then)
})
const response = await fastify.inject({
method: 'get',
url: '/'
})

t.equal(response.statusCode, 401)
})