-
Notifications
You must be signed in to change notification settings - Fork 28
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: validate authorization schema #167
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.
Minor things
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
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.
I dont like it for the reason that it is doing over and over unnecessary string operations.
Should we use @fastify/error to create FST_BEARER_AUTH_MISSING_AUTHORIZATION_HEADER and FST_BEARER_AUTH_INVALID_AUTHORIZATION_HEADER and use that instead? Would also be better passing them to the callback instead of native Node Errors? |
|
I am ok with this. I would probably create a follow up PR in the few days to straighten this up. |
Is verifying authtype suppose to be case sensitive ? I'm think it should not be. Found this : lexik/LexikJWTAuthenticationBundle#411 |
Checklist
This PR fixes issue where any string with same length as
bearer
passes validation ex:AAAAAA auth_key
.fixes #164
npm run test
andnpm run benchmark
and the Code of conduct