-
-
Notifications
You must be signed in to change notification settings - Fork 100
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
Ignore authorization headers if not Bearer #325
Conversation
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.
lgtm
I'm a bit conflicted if this is major or not. Anybody has an opinion? |
Although it stills throw Error, but changing the return from |
Co-authored-by: KaKa <[email protected]> Signed-off-by: cberescu <[email protected]>
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.
Co-authored-by: Gürgün Dayıoğlu <[email protected]> Signed-off-by: cberescu <[email protected]>
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.
looks good to me
@gurgunday not sure why one of the tests failed... any ideas ? |
It was a CI issue, fixed now |
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.
lgtm
Hey guys. Appreciate the work guys have done with this package. Could you potentially update the changelog for the 8.0.0 to clarify what the exact breaking change is and what users need to adjust when upgrading? https://github.com/fastify/fastify-jwt/releases/tag/v8.0.0. I had to dig in to find this PR to read the comments to find out what was considered BREAKING about it: #325 (comment) |
Fixes: #318
Checklist
npm run test
andnpm run benchmark
and the Code of conduct