-
Notifications
You must be signed in to change notification settings - Fork 256
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
feat: auth cache #643
feat: auth cache #643
Conversation
currently called directly in `convert_key`, should be a generic layer usable by other handlers as well
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.
Progress seems good to me 😄
84d12a9
to
5dc8f0a
Compare
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.
This is exciting, thanks Oddbjorn!!
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
also comment out broken test and add TODO to it, and revert auth manifest changes
2361fd1
to
a6265c2
Compare
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.
Nice 🥳, thanks @oddgrd
LGTM
This PR adds a cache in front of the reverse proxy in the
ShuttleAuthLayer
that tries to upgrade a cookie or api-key bearer token to a JWT. The cache is first checked for the key (cookie or api-key), if it is there set it on the request in theShuttleAuthLayer
. If it isn't, request an upgrade from theauth
server through the proxy. If it succeeds, extract the token from the proxy response and set it in the cache, before setting it on the request and proceeding.It would have been more elegant to have the cache be a layer on the proxy client, but I had some difficulty implementing this. It's certainly something we should consider in the future.
All tests pass except for one assertion in the
api_create_get_delete_projects
test, which is commented out. It was indeed broken, but it was testing functionality we don't have.