[GitHub] [incubator-pinot] sajjad-moradi commented on pull request #6440: Add authentication for write endpoints of Controller resources

[GitBox <gi...@apache.org>]

sajjad-moradi commented on pull request #6440:
URL: https://github.com/apache/incubator-pinot/pull/6440#issuecomment-759930512


   > Great feature addition! This is long time due for Pinot. Thanks for investing your time on this area.
   > 
   > Out of curiosity, have you considered implementing authorization declaratively at a servlet filter level rather than on each individual endpoints? Authorization and authentication being an orthogonal concern, I feel it would be less intrusive in the code base if basic CRUD rules could be defined across all endpoints rather than requiring a specific implementation on every endpoint.
   
   At first I actually did consider declarative approach. I did a small POC in which I defined an AuthFilter and registered it to get called before execution of all endpoints. Then I added an annotation for the write endpoints and in the AuthFilter only performed authentication for the methods having the new annotation. That way adding authentication, which is a separate concern, was less intrusive and was all addressed in one place - AuthFilter class.
   
   The reason I couldn't continue that route was that some of the endpoints are table specific and for them, table name is an input to the authentication procedure. Users need to be able to modify/delete their tables, but not other people's table. The problem here is that table name is a runtime parameter and in some methods it's a path parameter (or api parameter) and for some other methods like adding schema it's deep inside the json body of the post request. For these endpoints, extracting table name needs to happen within the endpoint and not doable in the AuthFilter. That's why I abandoned the aspect oriented way of defining annotation and AuthFilter and instead used the current approach which is more verbose and not that pretty.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org