-
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
Improve performance, fix bug in authorization schema #168
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
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 great!
I'm quite sure that BearerType should be case insensitive. |
My PR did not change any behaviour regarding case sensitivity. Probably you wanted to comment it on PR #167 by @dancastillo , who changed Eitherway, maybe open an issue to discuss it. |
I am quite certain the spec requires |
lexik/LexikJWTAuthenticationBundle#411 I think this is just a string comparaison, and there will most probably be bearer and Bearer in the wilde. This is really just string comparison, so I'm not sure why not do it. |
Please open an issue and discuss this there. Please also refer also to RFCs. This PR did not change that behaviour as I already explained. |
While programming I realized we have a bug in the code. We did not check if between
Bearer
and key was atleast one space. This bug is fixed, see unit tests.before:
Missing Header:
Invalid BearerType:
Invalid key:
Valid Request:
after:
Missing Header:
Invalid BearerType:
Invalid key:
Valid Request:
Checklist
npm run test
andnpm run benchmark
and the Code of conduct