-
Notifications
You must be signed in to change notification settings - Fork 89
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
[Feature] Allow DynamicAccessController to access the HTTP Request headers in order to implement JWT token authentication #202
Comments
We could do better first class support for this through the components (we'd need to add something into the controller and server to accommodate admin/write/fast client paths) but the router today has something that can be worked with. RouterServer.addOptionalChannelHandler(String key, ChannelHandler channelHandler) is a method that allows one to pass in channelHandlers that get added to the end of the request pipeline in the router server. Here you could potentially apply custom logic in the request pipeline to validate credentials. Maybe kind of a bummer that it's the last in the pipeline (ideally any security checks should happen first to reduce resource commitment). First class support will take a bit longer, but I think with the above you could get something working on at least the read path, and maybe the fix we apply will work off this notion (basically expose adding request handlers on the router and server and controller and then add utility for a credential provider in the admin tool/VenicePushJob/fast client/etc.) |
Probably one approach would be to not rely anymore on DynamicAccessController but on AuthorizerService venice/internal/venice-common/src/main/java/com/linkedin/venice/authorization/AuthorizerService.java Line 25 in 63a7921
I tried to build some adapter between DynamicAccessController and AuthorizerService (an AuthorizerService implementation that forwards to the DynamicAccessController) but there are things about ACLs that cannot be easily adapted. |
Thats definitely the intended approach, DynamicAccessController is the old interface that we mean to deprecate. Guess we should just get to it. |
InDay's coming up 😉 ! |
I think that one of the main problems is that we don't have a clear separation between Authentication (mapping a user request to a principal/role) and Authorization (deciding who can do what). Also the DynamicAccessController and the Authorizer services have another problem, they are used both for performing authorization assertions and to handle ACLs (management) I think that eventually we need to split those three responsibilities into specific APIs. I am drafting an AuthenticationService API here in my fork |
Willingness to contribute
Yes. I can contribute a fix for this bug independently.
Feature Request Proposal
My understanding from code is that the Router, the Controller and the Server expose an HTTP endpoint.
In this case implementing token based authentication (like JWT) is pretty easy, because we can simply require the client to pass the token in a HTTP header.
The only thing we need to do is to allow the DynamicAccessController to access the HTTP request headers (and possibly cache the result of the validation but attaching it to the Netty Channel)
The implementation on the clients should be pretty straighforward, as we only need to let the users configure the token and pass it in every HTTP request as an header
Motivation
Implementing JWT authentication (and some day also OAuth2)
Details
No response
What component(s) does this bug affect?
Controller
: This is the control-plane for Venice. Used to create/update/query stores and their metadata.Router
: This is the stateless query-routing layer for serving read requests.Server
: This is the component that persists all the store data.VenicePushJob
: This is the component that pushes derived data from Hadoop to Venice backend.Thin Client
: This is a stateless client users use to query Venice Router for reading store data.Fast Client
: This is a stateful client users use to query Venice Server for reading store data.Da Vinci Client
: This is an embedded, stateful client that materializes store data locally.Samza
: This is the library users use to make nearline updates to store data.Admin Tool
: This is the stand-alone client used for ad-hoc operations on Venice.The text was updated successfully, but these errors were encountered: