Hello Harsha, Pinging again. This is a minor KIP and it has been lying around for quite some time. If providing supported principal types via a config is what you suggest, I am fine with it.
On Wed, Mar 9, 2016 at 12:32 PM, Ashish Singh <asi...@cloudera.com> wrote: > Hi Harsha, > > On Wed, Mar 9, 2016 at 12:04 PM, Harsha <ka...@harsha.io> wrote: > >> Why we need to add this additional method just for validation. This will >> invalidate all the existing authorizer implementations. > > As PrincipalTypes is implementation specific, wouldn't it be required for > users to know what principal types they can use in add/removeAcls? > > All the existing authorizer implementations will continue to work, as the > method by default will return List(User), as User is the only principal > type that is supported out of the box as of now. Let me know if I missed > your question here. > > >> Why can't we add >> the logic for validation and pass it as authorizer config. >> > Do you mean passing PrincipalTypes as authorizer config? If I am getting > your question correctly, then we are asking users to be aware of what > PrincipalTypes an authorizer supports. That defeats the purpose of > validation, right? > >> >> -Harsha >> >> On Mon, Mar 7, 2016, at 04:33 PM, Ashish Singh wrote: >> > + Parth, Harsha >> > >> > On Mon, Mar 7, 2016 at 4:32 PM, Ashish Singh <asi...@cloudera.com> >> wrote: >> > >> > > Thanks Gwen. >> > > >> > > @Parth, @Harsha pinging you guys for your feedback. Based on >> discussion on >> > > JIRA, we have following open questions. >> > > >> > > 1. >> > > >> > > How to allow an authorizer implementation to specify supported >> > > principal types? >> > > >> > > An alternative of providing supported Principal types via >> interface is >> > > via a config option. Having a config option will be helpful for >> certain >> > > third party implementations that uses SimpleAclAuthorizer but >> support more >> > > PrincipalTypes. However, it requires adds one more config. >> > > >> > > 2. >> > > >> > > ACLs validation should be done by client or by authorizer? >> > > >> > > Once this method is added we expect the Client of the authorizer >> to do >> > > the validation on principal types and the authorizer will still >> not do any >> > > validation by it self. As an alternative we can add the validation >> at >> > > Authorizer level. Having validation done at client side enables >> clients to >> > > fail fast for invalid principal types, whereas implementing it at >> > > authorization level removes the requirement of having the >> validation done >> > > on each client implementation. >> > > >> > > >> > > On Mon, Mar 7, 2016 at 3:47 PM, Gwen Shapira <g...@confluent.io> >> wrote: >> > > >> > > Ashish, >> > >> >> > >> I'm neutral on this (+0), but would be good to have feedback from >> > >> Harsha and Parth. If you can get their "sounds good", we can probably >> > >> get it through fairly soon and in time for 0.10.0. >> > >> >> > >> Gwen >> > >> >> > >> On Wed, Mar 2, 2016 at 9:47 AM, Ashish Singh <asi...@cloudera.com> >> wrote: >> > >> > Here is link to the KIP, >> > >> > >> > >> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Enhance+Authorizer+interface+to+be+aware+of+supported+Principal+Types >> > >> > >> > >> > On Wed, Mar 2, 2016 at 9:46 AM, Ashish Singh <asi...@cloudera.com> >> > >> wrote: >> > >> > >> > >> >> Hi Guys, >> > >> >> >> > >> >> I would like to initiate a discuss thread on KIP-50. Kafka >> authorizer >> > >> is >> > >> >> agnostic of principal types it supports, so are the acls CRUD >> methods >> > >> >> in kafka.security.auth.Authorizer. The intent behind is to keep >> Kafka >> > >> >> authorization pluggable, which is really great. However, this >> leads to >> > >> Acls >> > >> >> CRUD methods not performing any check on validity of acls, as >> they are >> > >> not >> > >> >> aware of what principal types Authorizer implementation supports. >> This >> > >> >> opens up space for lots of user errors, KAFKA-3097 >> > >> >> <https://issues.apache.org/jira/browse/KAFKA-3097> for an >> instance. >> > >> >> >> > >> >> This KIP proposes adding a getSupportedPrincipalTypes method to >> > >> authorizer >> > >> >> and use that for acls verification during acls CRUD. >> > >> >> >> > >> >> Feedbacks and comments are welcome. >> > >> >> >> > >> >> -- >> > >> >> >> > >> >> Regards, >> > >> >> Ashish >> > >> >> >> > >> > >> > >> > >> > >> > >> > >> > -- >> > >> > >> > >> > Regards, >> > >> > Ashish >> > >> >> > > >> > > -- >> > > >> > > Regards, >> > > Ashish >> > > >> > >> > >> > >> > -- >> > >> > Regards, >> > Ashish >> > > > > -- > > Regards, > Ashish > -- Regards, Ashish
