I am going to initiate a vote thread for this. On Mon, Mar 28, 2016 at 1:28 PM, Ashish Singh <asi...@cloudera.com> wrote:
> 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 > -- Regards, Ashish