-
Notifications
You must be signed in to change notification settings - Fork 119
feat(notary): add JWT-based authorization mode #817
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
Conversation
yuroitaki
left a comment
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.
Great stuff! Got some comments (some are only nits and cosmetics though).
This mode is an alternative to whitelist authorization mode. It extracts the JWT from the authorization header (bearer token), validates token's signature, claimed expiry times and additional (user-configurable) claims.
yuroitaki
left a comment
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.
Thanks for the changes~ few more comments
yuroitaki
left a comment
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.
Almost there! Mainly comments on logging 🙏
yuroitaki
left a comment
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.
Thanks so much again for the contribution! @kubkon
It was my pleasure! |
Closes #812
This PR implements the proposal issue for JSON Web Token based authorization in the notary server. I tried my best to keep the addition of JWT validation mechanism the least invasive and thus the extraction logic is handled in existing
AuthorizationMiddlewarestruct where we guide the extractor which header and token/API key to look for based on the server's config.Server's config is now expecting an enum
This way, only one mode is possible and this is enforced at config parsing level.
I have also added an integration test which mirrors that for the whitelist authorization but where we use JWT token for authorization.
Finally, I've updated the README and openapi spec to reflect the proposed changes.
Lemme know what you think! Hopefully this PR will make reading and deciding on the proposal in #812 a little easier since it outlines the number of required changes to make it a reality.
cc @yuroitaki