fix: Async key provider and errors should be resolved internally#335
fix: Async key provider and errors should be resolved internally#335NikitaIT wants to merge 4 commits intofastify:mainfrom
Conversation
NikitaIT
left a comment
There was a problem hiding this comment.
This PR seems to work, I use it as a patch in my code. So far no problems have been noticed. All comments in the review are left for ease of review. They are not open issues on my part, or my todo. Close them as you become familiar. Thank you
| verifyResult.then(result => callback(null, result), error => wrapError(error, callback)) | ||
| } else { | ||
| const verifyResult = verifier(token) | ||
| callback(null, verifyResult) |
There was a problem hiding this comment.
verifyResult moved in order to support promise based result.
| } | ||
| }) | ||
| fastify.get('/', async function (request, reply) { | ||
| request.headers.jwt = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.hQOxra1Zo9z61vCqe6_86kVfLqKI0WxDnkiJ_upW0sM' |
There was a problem hiding this comment.
Hardcoded request.headers.jwt is for key=this secret reused from cache
Generated with https://jwt.io/
There was a problem hiding this comment.
please generate this dynamically in code.
| }) | ||
| fastify.get('/', async function (request, reply) { | ||
| request.headers.jwt = | ||
| 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c' |
There was a problem hiding this comment.
please generate this dynamically in code.
| } | ||
| }) | ||
| fastify.get('/', async function (request, reply) { | ||
| request.headers.jwt = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.hQOxra1Zo9z61vCqe6_86kVfLqKI0WxDnkiJ_upW0sM' |
There was a problem hiding this comment.
please generate this dynamically in code.
| }) | ||
| fastify.get('/', async function (request, reply) { | ||
| request.headers.jwt = | ||
| 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c' |
There was a problem hiding this comment.
please generate this dynamically in code.
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no suggestions.
Comments skipped due to low confidence (2)
jwt.js:490
- [nitpick] The variable 'verifyResult' is declared but not initialized. Consider initializing it.
let verifyResult
jwt.js:534
- Ensure that all error cases handled by the 'wrapError' function are covered by tests.
function wrapError (error, callback) {
False positive tests: test: Async key provider errors should be resolved internally
Fix & test fix: feat: Async key provider
Related issue: #334
Checklist
npm run testandnpm run benchmarkand the Code of conduct