Hi Mayuresh, Sorry for the delay. The updated KIP states that there is no compatibility impact, but that doesn't seem right. The fact that we changed the type of Session.principal to `Principal` means that any code that expects it to be `KafkaPrincipal` will break. Either because of declared types (likely) or if it accesses `getPrincipalType` (unlikely since the value is always the same). It's a bit annoying, but we should add a new field to `Session` with the original principal. We can potentially deprecate the existing one, if we're sure we don't need it (or we can leave it for now).
Ismael On Mon, Feb 27, 2017 at 6:40 PM, Mayuresh Gharat <gharatmayures...@gmail.com > wrote: > Hi Ismael, Joel, Becket > > Would you mind taking a look at this. We require 2 more binding votes for > the KIP to pass. > > Thanks, > > Mayuresh > > On Thu, Feb 23, 2017 at 10:57 AM, Dong Lin <lindon...@gmail.com> wrote: > > > +1 (non-binding) > > > > On Wed, Feb 22, 2017 at 10:52 PM, Manikumar <manikumar.re...@gmail.com> > > wrote: > > > > > +1 (non-binding) > > > > > > On Thu, Feb 23, 2017 at 3:27 AM, Mayuresh Gharat < > > > gharatmayures...@gmail.com > > > > wrote: > > > > > > > Hi Jun, > > > > > > > > Thanks a lot for the comments and reviews. > > > > I agree we should log the username. > > > > What I meant by creating KafkaPrincipal was, after this KIP we would > > not > > > be > > > > required to create KafkaPrincipal and if we want to maintain the old > > > > logging, we will have to create it as we do today. > > > > I will take care that we specify the Principal name in the log. > > > > > > > > Thanks again for all the reviews. > > > > > > > > Thanks, > > > > > > > > Mayuresh > > > > > > > > On Wed, Feb 22, 2017 at 1:45 PM, Jun Rao <j...@confluent.io> wrote: > > > > > > > > > Hi, Mayuresh, > > > > > > > > > > For logging the user name, we could do either way. We just need to > > make > > > > > sure the expected user name is logged. Also, currently, we are > > already > > > > > creating a KafkaPrincipal on every request. +1 on the latest KIP. > > > > > > > > > > Thanks, > > > > > > > > > > Jun > > > > > > > > > > > > > > > On Tue, Feb 21, 2017 at 8:05 PM, Mayuresh Gharat < > > > > > gharatmayures...@gmail.com > > > > > > wrote: > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > Thanks for the comments. > > > > > > > > > > > > I will mention in the KIP : how this change doesn't affect the > > > default > > > > > > authorizer implementation. > > > > > > > > > > > > Regarding, Currently, we log the principal name in the request > log > > in > > > > > > RequestChannel, which has the format of "principalType + > SEPARATOR > > + > > > > > > name;". > > > > > > It would be good if we can keep the same convention after this > KIP. > > > One > > > > > way > > > > > > to do that is to convert java.security.Principal to > KafkaPrincipal > > > for > > > > > > logging the requests. > > > > > > --- > This would mean we have to create a new KafkaPrincipal on > > each > > > > > > request. Would it be OK to just specify the name of the > principal. > > > > > > Is there any major reason, we don't want to change the logging > > > format? > > > > > > > > > > > > Thanks, > > > > > > > > > > > > Mayuresh > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Feb 20, 2017 at 10:18 PM, Jun Rao <j...@confluent.io> > > wrote: > > > > > > > > > > > > > Hi, Mayuresh, > > > > > > > > > > > > > > Thanks for the updated KIP. A couple of more comments. > > > > > > > > > > > > > > 1. Do we convert java.security.Principal to KafkaPrincipal for > > > > > > > authorization check in SimpleAclAuthorizer? If so, it would be > > > useful > > > > > to > > > > > > > mention that in the wiki so that people can understand how this > > > > change > > > > > > > doesn't affect the default authorizer implementation. > > > > > > > > > > > > > > 2. Currently, we log the principal name in the request log in > > > > > > > RequestChannel, which has the format of "principalType + > > SEPARATOR > > > + > > > > > > > name;". > > > > > > > It would be good if we can keep the same convention after this > > KIP. > > > > One > > > > > > way > > > > > > > to do that is to convert java.security.Principal to > > KafkaPrincipal > > > > for > > > > > > > logging the requests. > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > > > > > > > On Fri, Feb 17, 2017 at 5:35 PM, Mayuresh Gharat < > > > > > > > gharatmayures...@gmail.com > > > > > > > > wrote: > > > > > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > > > > > I have updated the KIP. Would you mind taking another look? > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > Mayuresh > > > > > > > > > > > > > > > > On Fri, Feb 17, 2017 at 4:42 PM, Mayuresh Gharat < > > > > > > > > gharatmayures...@gmail.com > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > > > > > > > Sure sounds good to me. > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > > Mayuresh > > > > > > > > > > > > > > > > > > On Fri, Feb 17, 2017 at 1:54 PM, Jun Rao <j...@confluent.io > > > > > > wrote: > > > > > > > > > > > > > > > > > >> Hi, Mani, > > > > > > > > >> > > > > > > > > >> Good point on using PrincipalBuilder for SASL. It seems > that > > > > > > > > >> PrincipalBuilder already has access to Authenticator. So, > we > > > > could > > > > > > > just > > > > > > > > >> enable that in SaslChannelBuilder. We probably could do > that > > > in > > > > a > > > > > > > > separate > > > > > > > > >> KIP? > > > > > > > > >> > > > > > > > > >> Hi, Mayuresh, > > > > > > > > >> > > > > > > > > >> If you don't think there is a concrete use case for using > > > > > > > > >> PrincipalBuilder in > > > > > > > > >> kafka-acls.sh, perhaps we could do the simpler approach > for > > > now? > > > > > > > > >> > > > > > > > > >> Thanks, > > > > > > > > >> > > > > > > > > >> Jun > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> On Fri, Feb 17, 2017 at 12:23 PM, Mayuresh Gharat < > > > > > > > > >> gharatmayures...@gmail.com> wrote: > > > > > > > > >> > > > > > > > > >> > @Manikumar, > > > > > > > > >> > > > > > > > > > >> > Can you give an example how you are planning to use > > > > > > > PrincipalBuilder? > > > > > > > > >> > > > > > > > > > >> > @Jun > > > > > > > > >> > Yes, that is right. To give a brief overview, we just > > > extract > > > > > the > > > > > > > cert > > > > > > > > >> and > > > > > > > > >> > hand it over to a third party library for creating a > > > > Principal. > > > > > So > > > > > > > we > > > > > > > > >> > cannot create a Principal from just a string. > > > > > > > > >> > The main motive behind adding the PrincipalBuilder for > > > > > > kafk-acls.sh > > > > > > > > was > > > > > > > > >> > that someone else (who can generate a Principal from map > > of > > > > > > > propertie, > > > > > > > > >> > <String, String> for example) can use it. > > > > > > > > >> > As I said, Linkedin is fine with not making any changes > to > > > > > > > > Kafka-acls.sh > > > > > > > > >> > for now. But we thought that it would be a good > > improvement > > > to > > > > > the > > > > > > > > tool > > > > > > > > >> and > > > > > > > > >> > it makes it more flexible and usable. > > > > > > > > >> > > > > > > > > > >> > Let us know your thoughts, if you would like us to make > > > > > > > kafka-acls.sh > > > > > > > > >> more > > > > > > > > >> > flexible and usable and not limited to Authorizer coming > > out > > > > of > > > > > > the > > > > > > > > box. > > > > > > > > >> > > > > > > > > > >> > Thanks, > > > > > > > > >> > > > > > > > > > >> > Mayuresh > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > On Thu, Feb 16, 2017 at 10:18 PM, Manikumar < > > > > > > > > manikumar.re...@gmail.com> > > > > > > > > >> > wrote: > > > > > > > > >> > > > > > > > > > >> > > Hi Jun, > > > > > > > > >> > > > > > > > > > > >> > > yes, we can just customize rules to send full > principal > > > > > name. I > > > > > > > was > > > > > > > > >> > > just thinking to > > > > > > > > >> > > use PrinciplaBuilder interface for implementing SASL > > rules > > > > > also. > > > > > > > So > > > > > > > > >> that > > > > > > > > >> > > the interface > > > > > > > > >> > > will be consistent across protocols. > > > > > > > > >> > > > > > > > > > > >> > > Thanks > > > > > > > > >> > > > > > > > > > > >> > > On Fri, Feb 17, 2017 at 1:07 AM, Jun Rao < > > > j...@confluent.io> > > > > > > > wrote: > > > > > > > > >> > > > > > > > > > > >> > > > Hi, Radai, Mayuresh, > > > > > > > > >> > > > > > > > > > > > >> > > > Thanks for the explanation. Good point on a > pluggable > > > > > > authorizer > > > > > > > > can > > > > > > > > >> > > > customize how acls are added. However, earlier, > > Mayuresh > > > > was > > > > > > > > saying > > > > > > > > >> > that > > > > > > > > >> > > in > > > > > > > > >> > > > LinkedIn's customized authorizer, it's not possible > to > > > > > create > > > > > > a > > > > > > > > >> > principal > > > > > > > > >> > > > from string. If that's the case, will adding the > > > principal > > > > > > > builder > > > > > > > > >> in > > > > > > > > >> > > > kafka-acl.sh help? If the principal can be > constructed > > > > from > > > > > a > > > > > > > > >> string, > > > > > > > > >> > > > wouldn't it be simpler to just let kafka-acl.sh do > > > > > > authorization > > > > > > > > >> based > > > > > > > > >> > on > > > > > > > > >> > > > that string name and not be aware of the principal > > > > builder? > > > > > If > > > > > > > you > > > > > > > > >> > still > > > > > > > > >> > > > think there is a need, perhaps you can add a more > > > concrete > > > > > use > > > > > > > > case > > > > > > > > >> > that > > > > > > > > >> > > > can't be done otherwise? > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > >> > > > Hi, Mani, > > > > > > > > >> > > > > > > > > > > > >> > > > For SASL, if the authorizer needs the full kerberos > > > > > principal > > > > > > > > name, > > > > > > > > >> > > > currently, the user can just customize " > > > > > > > > sasl.kerberos.principal.to. > > > > > > > > >> > > > local.rules" > > > > > > > > >> > > > to return the full principal name as the name for > > > > > > authorization, > > > > > > > > >> right? > > > > > > > > >> > > > > > > > > > > > >> > > > Thanks, > > > > > > > > >> > > > > > > > > > > > >> > > > Jun > > > > > > > > >> > > > > > > > > > > > >> > > > On Wed, Feb 15, 2017 at 10:25 AM, Mayuresh Gharat < > > > > > > > > >> > > > gharatmayures...@gmail.com> wrote: > > > > > > > > >> > > > > > > > > > > > >> > > > > @Jun thanks for the comments.Please see the > replies > > > > > inline. > > > > > > > > >> > > > > > > > > > > > > >> > > > > Currently kafka-acl.sh just creates an ACL path in > > ZK > > > > with > > > > > > the > > > > > > > > >> > > principal > > > > > > > > >> > > > > name string. > > > > > > > > >> > > > > ----> Yes, the kafka-acl.sh calls the addAcl() on > > the > > > > > > inbuilt > > > > > > > > >> > > > > SimpleAclAuthorizer which in turn creates an ACL > in > > ZK > > > > > with > > > > > > > the > > > > > > > > >> > > Principal > > > > > > > > >> > > > > name string. This is because we supply the > > > > > > SimpleAclAuthorizer > > > > > > > > as > > > > > > > > >> a > > > > > > > > >> > > > > commandline argument in the Kafka-acls.sh command. > > > > > > > > >> > > > > > > > > > > > > >> > > > > The authorizer module in the broker reads the > > > principal > > > > > name > > > > > > > > >> > > > > string from the acl path in ZK and creates the > > > expected > > > > > > > > >> > KafkaPrincipal > > > > > > > > >> > > > for > > > > > > > > >> > > > > matching. As you can see, the expected principal > is > > > > > created > > > > > > on > > > > > > > > the > > > > > > > > >> > > broker > > > > > > > > >> > > > > side, not by the kafka-acl.sh tool. > > > > > > > > >> > > > > ----> This is considering the fact that the user > is > > > > using > > > > > > the > > > > > > > > >> > > > > SimpleAclAuthorizer on the broker side and not his > > own > > > > > > custom > > > > > > > > >> > > Authorizer. > > > > > > > > >> > > > > The SimpleAclAuthorizer will take the Principal it > > > gets > > > > > from > > > > > > > the > > > > > > > > >> > > Session > > > > > > > > >> > > > > class . Currently the Principal is KafkaPrincipal. > > > This > > > > > > > > >> > KafkaPrincipal > > > > > > > > >> > > is > > > > > > > > >> > > > > generated from the name of the actual channel > > > Principal, > > > > > in > > > > > > > > >> > > SocketServer > > > > > > > > >> > > > > class when processing completed receives. > > > > > > > > >> > > > > With this KIP, this will no longer be the case as > > the > > > > > > Session > > > > > > > > >> class > > > > > > > > >> > > will > > > > > > > > >> > > > > store a java.security.Principal instead of > specific > > > > > > > > >> KafkaPrincipal. > > > > > > > > >> > So > > > > > > > > >> > > > the > > > > > > > > >> > > > > SimpleAclAuthorizer will construct the > > KafkaPrincipal > > > > from > > > > > > the > > > > > > > > >> > channel > > > > > > > > >> > > > > Principal it gets from the Session class. > > > > > > > > >> > > > > User might not want to use the SimpleAclAuthorizer > > but > > > > use > > > > > > > > his/her > > > > > > > > >> > own > > > > > > > > >> > > > > custom Authorizer. > > > > > > > > >> > > > > > > > > > > > > >> > > > > The broker already has the ability to > > > > > > > > >> > > > > configure PrincipalBuilder. That's why I am not > sure > > > if > > > > > > there > > > > > > > > is a > > > > > > > > >> > need > > > > > > > > >> > > > for > > > > > > > > >> > > > > kafka-acl.sh to customize PrincipalBuilder. > > > > > > > > >> > > > > ----> This is exactly the reason why we want to > > > propose > > > > a > > > > > > > > >> > > > PrincipalBuilder > > > > > > > > >> > > > > in kafka-acls.sh so that the Principal generated > by > > > the > > > > > > > > >> > > PrincipalBuilder > > > > > > > > >> > > > on > > > > > > > > >> > > > > broker is consistent with that generated while > > > creating > > > > > ACLs > > > > > > > > using > > > > > > > > >> > the > > > > > > > > >> > > > > kafka-acls.sh command line tool. > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > *To summarize the above discussions :* > > > > > > > > >> > > > > What if we only make the following changes: pass > the > > > > java > > > > > > > > >> principal > > > > > > > > >> > in > > > > > > > > >> > > > > session and in > > > > > > > > >> > > > > SimpleAuthorizer, construct KafkaPrincipal from > java > > > > > > principal > > > > > > > > >> name. > > > > > > > > >> > > Will > > > > > > > > >> > > > > that work for LinkedIn? > > > > > > > > >> > > > > ------> Yes, this works for Linkedin as we are not > > > using > > > > > the > > > > > > > > >> > > > kafka-acls.sh > > > > > > > > >> > > > > tool to create/update/add ACLs, for now. > > > > > > > > >> > > > > > > > > > > > > >> > > > > Do you think there is a use case for a customized > > > > > authorizer > > > > > > > and > > > > > > > > >> > > > kafka-acl > > > > > > > > >> > > > > at the > > > > > > > > >> > > > > same time? If not, it's better not to complicate > the > > > > > > kafka-acl > > > > > > > > >> api. > > > > > > > > >> > > > > -----> At Linkedin, we don't use this tool for > now. > > So > > > > we > > > > > > are > > > > > > > > fine > > > > > > > > >> > with > > > > > > > > >> > > > the > > > > > > > > >> > > > > minimal change for now. > > > > > > > > >> > > > > > > > > > > > > >> > > > > Initially, our change was minimal, just getting > the > > > > Kafka > > > > > to > > > > > > > > >> preserve > > > > > > > > >> > > the > > > > > > > > >> > > > > channel principal. Since there was a discussion > how > > > > > > > > kafka-acls.sh > > > > > > > > >> > would > > > > > > > > >> > > > > work with this change, on the ticket, we designed > a > > > > > detailed > > > > > > > > >> solution > > > > > > > > >> > > to > > > > > > > > >> > > > > make this tool generally usable with all sorts of > > > > > > combinations > > > > > > > > of > > > > > > > > >> > > > > Authorizers and PrincipalBuilders and give more > > > > > flexibility > > > > > > to > > > > > > > > the > > > > > > > > >> > end > > > > > > > > >> > > > > users. > > > > > > > > >> > > > > Without the changes proposed for kafka-acls.sh in > > this > > > > > KIP, > > > > > > it > > > > > > > > >> cannot > > > > > > > > >> > > be > > > > > > > > >> > > > > used with a custom Authorizer/PrinipalBuilder but > > will > > > > > only > > > > > > > work > > > > > > > > >> with > > > > > > > > >> > > > > SimpleAclAuthorizer. > > > > > > > > >> > > > > > > > > > > > > >> > > > > Although, I would actually like it to work for > > general > > > > > > > scenario, > > > > > > > > >> I am > > > > > > > > >> > > > fine > > > > > > > > >> > > > > with separating it under a separate KIP and limit > > the > > > > > scope > > > > > > of > > > > > > > > >> this > > > > > > > > >> > > KIP. > > > > > > > > >> > > > > I will update the KIP accordingly and put this > under > > > > > > rejected > > > > > > > > >> > > > alternatives > > > > > > > > >> > > > > and create a new KIP for the Kafka-acls.sh > changes. > > > > > > > > >> > > > > > > > > > > > > >> > > > > @Manikumar > > > > > > > > >> > > > > Since we are limiting the scope of this KIP by not > > > > making > > > > > > any > > > > > > > > >> changes > > > > > > > > >> > > to > > > > > > > > >> > > > > kafka-acls.sh, I will cover your concern in a > > separate > > > > KIP > > > > > > > that > > > > > > > > I > > > > > > > > >> > will > > > > > > > > >> > > > put > > > > > > > > >> > > > > up for kafka-acls.sh. Does that work? > > > > > > > > >> > > > > > > > > > > > > >> > > > > Thanks, > > > > > > > > >> > > > > > > > > > > > > >> > > > > Mayuresh > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > On Wed, Feb 15, 2017 at 9:18 AM, radai < > > > > > > > > >> radai.rosenbl...@gmail.com> > > > > > > > > >> > > > wrote: > > > > > > > > >> > > > > > > > > > > > > >> > > > > > @jun: > > > > > > > > >> > > > > > "Currently kafka-acl.sh just creates an ACL path > > in > > > ZK > > > > > > with > > > > > > > > the > > > > > > > > >> > > > principal > > > > > > > > >> > > > > > name string" - yes, but not directly. all it > > > actually > > > > > does > > > > > > > it > > > > > > > > >> > spin-up > > > > > > > > >> > > > the > > > > > > > > >> > > > > > Authorizer and call Authorizer.addAcl() on it. > > > > > > > > >> > > > > > the vanilla Authorizer goes to ZK. > > > > > > > > >> > > > > > but generally speaking, users can plug in their > > own > > > > > > > > Authorizers > > > > > > > > >> > (that > > > > > > > > >> > > > can > > > > > > > > >> > > > > > store/load ACLs to/from wherever). > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > it would be nice if users who customize > > Authorizers > > > > (and > > > > > > > > >> > > > > PrincipalBuilders) > > > > > > > > >> > > > > > did not immediately lose the ability to use > > > > kafka-acl.sh > > > > > > > with > > > > > > > > >> their > > > > > > > > >> > > new > > > > > > > > >> > > > > > Authorizers. > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > On Wed, Feb 15, 2017 at 5:50 AM, Manikumar < > > > > > > > > >> > > manikumar.re...@gmail.com> > > > > > > > > >> > > > > > wrote: > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > Sorry, I am late to this discussion. > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > PrincipalBuilder is only used for SSL > Protocol. > > > > > > > > >> > > > > > > For SASL, we use "sasl.kerberos.principal.to. > > > > > > local.rules" > > > > > > > > >> config > > > > > > > > >> > > to > > > > > > > > >> > > > > map > > > > > > > > >> > > > > > > SASL principal names to short names. To make > it > > > > > > > consistent, > > > > > > > > >> > > > > > > Do we also need to pass the SASL full > principal > > > name > > > > > to > > > > > > > > >> > authorizer > > > > > > > > >> > > ? > > > > > > > > >> > > > > > > We may need to use PrincipalBuilder for > mapping > > > SASL > > > > > > > names. > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > Related JIRA is here: > > > > > > > > >> > > > > > > https://issues.apache.org/ > > jira/browse/KAFKA-2854 > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > On Wed, Feb 15, 2017 at 7:47 AM, Jun Rao < > > > > > > > j...@confluent.io> > > > > > > > > >> > wrote: > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > Hi, Radai, > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > Currently kafka-acl.sh just creates an ACL > > path > > > in > > > > > ZK > > > > > > > with > > > > > > > > >> the > > > > > > > > >> > > > > > principal > > > > > > > > >> > > > > > > > name string. The authorizer module in the > > broker > > > > > reads > > > > > > > the > > > > > > > > >> > > > principal > > > > > > > > >> > > > > > name > > > > > > > > >> > > > > > > > string from the acl path in ZK and creates > the > > > > > > expected > > > > > > > > >> > > > > KafkaPrincipal > > > > > > > > >> > > > > > > for > > > > > > > > >> > > > > > > > matching. As you can see, the expected > > principal > > > > is > > > > > > > > created > > > > > > > > >> on > > > > > > > > >> > > the > > > > > > > > >> > > > > > broker > > > > > > > > >> > > > > > > > side, not by the kafka-acl.sh tool. The > broker > > > > > already > > > > > > > has > > > > > > > > >> the > > > > > > > > >> > > > > ability > > > > > > > > >> > > > > > to > > > > > > > > >> > > > > > > > configure PrincipalBuilder. That's why I am > > not > > > > sure > > > > > > if > > > > > > > > >> there > > > > > > > > >> > is > > > > > > > > >> > > a > > > > > > > > >> > > > > need > > > > > > > > >> > > > > > > for > > > > > > > > >> > > > > > > > kafka-acl.sh to customize PrincipalBuilder. > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > Thanks, > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > Jun > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > On Mon, Feb 13, 2017 at 7:01 PM, radai < > > > > > > > > >> > > radai.rosenbl...@gmail.com > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > wrote: > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > if i understand correctly, kafka-acls.sh > > spins > > > > up > > > > > an > > > > > > > > >> instance > > > > > > > > >> > > of > > > > > > > > >> > > > > (the > > > > > > > > >> > > > > > > > > custom, in our case) Authorizer, and calls > > > > things > > > > > > like > > > > > > > > >> > > > > addAcls(acls: > > > > > > > > >> > > > > > > > > Set[Acl], resource: Resource) on it, which > > are > > > > > > defined > > > > > > > > in > > > > > > > > >> the > > > > > > > > >> > > > > > > interface, > > > > > > > > >> > > > > > > > > hence expected to be "extensible". > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > (side note: if Authorizer and > > PrincipalBuilder > > > > are > > > > > > > > >> defined as > > > > > > > > >> > > > > > > extensible > > > > > > > > >> > > > > > > > > interfaces, why doesnt class Acl, which is > > in > > > > the > > > > > > > > >> signature > > > > > > > > >> > for > > > > > > > > >> > > > > > > > Authorizer > > > > > > > > >> > > > > > > > > calls, use java.security.Principal?) > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > we would like to be able to use the > standard > > > > > > kafka-acl > > > > > > > > >> > command > > > > > > > > >> > > > line > > > > > > > > >> > > > > > for > > > > > > > > >> > > > > > > > > defining ACLs even when replacing the > > vanilla > > > > > > > Authorizer > > > > > > > > >> and > > > > > > > > >> > > > > > > > > PrincipalBuilder (even though we have a > > > > management > > > > > > UI > > > > > > > > for > > > > > > > > >> > these > > > > > > > > >> > > > > > > > operations > > > > > > > > >> > > > > > > > > within linkedin) - simply because thats > the > > > > > correct > > > > > > > > thing > > > > > > > > >> to > > > > > > > > >> > do > > > > > > > > >> > > > > from > > > > > > > > >> > > > > > an > > > > > > > > >> > > > > > > > > extensibility point of view. > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > On Mon, Feb 13, 2017 at 1:39 PM, Jun Rao < > > > > > > > > >> j...@confluent.io> > > > > > > > > >> > > > wrote: > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > Hi, Mayuresh, > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > I seems to me that there are two common > > use > > > > > cases > > > > > > of > > > > > > > > >> > > > authorizer. > > > > > > > > >> > > > > > (1) > > > > > > > > >> > > > > > > > Use > > > > > > > > >> > > > > > > > > > the default SimpleAuthorizer and the > > > kafka-acl > > > > > to > > > > > > do > > > > > > > > >> > > > > authorization. > > > > > > > > >> > > > > > > (2) > > > > > > > > >> > > > > > > > > Use > > > > > > > > >> > > > > > > > > > a customized authorizer and an external > > tool > > > > for > > > > > > > > >> > > authorization. > > > > > > > > >> > > > > Do > > > > > > > > >> > > > > > > you > > > > > > > > >> > > > > > > > > > think there is a use case for a > customized > > > > > > > authorizer > > > > > > > > >> and > > > > > > > > >> > > > > kafka-acl > > > > > > > > >> > > > > > > at > > > > > > > > >> > > > > > > > > the > > > > > > > > >> > > > > > > > > > same time? If not, it's better not to > > > > complicate > > > > > > the > > > > > > > > >> > > kafka-acl > > > > > > > > >> > > > > api. > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > Thanks, > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > Jun > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > On Mon, Feb 13, 2017 at 10:35 AM, > Mayuresh > > > > > Gharat > > > > > > < > > > > > > > > >> > > > > > > > > > gharatmayures...@gmail.com> wrote: > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > Hi Jun, > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > Thanks for the review and comments. > > Please > > > > > find > > > > > > > the > > > > > > > > >> > replies > > > > > > > > >> > > > > > inline > > > > > > > > >> > > > > > > : > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > This is so that in the future, we can > > > extend > > > > > to > > > > > > > > types > > > > > > > > >> > like > > > > > > > > >> > > > > group. > > > > > > > > >> > > > > > > > > > > ---> Yep, I did think the same. But > > since > > > > the > > > > > > > > >> > SocketServer > > > > > > > > >> > > > was > > > > > > > > >> > > > > > > always > > > > > > > > >> > > > > > > > > > > creating User type, it wasn't actually > > > used. > > > > > If > > > > > > we > > > > > > > > go > > > > > > > > >> > ahead > > > > > > > > >> > > > > with > > > > > > > > >> > > > > > > > > changes > > > > > > > > >> > > > > > > > > > in > > > > > > > > >> > > > > > > > > > > this KIP, we will give this power of > > > > creating > > > > > > > > >> different > > > > > > > > >> > > > > Principal > > > > > > > > >> > > > > > > > types > > > > > > > > >> > > > > > > > > > to > > > > > > > > >> > > > > > > > > > > the PrincipalBuilder (which users can > > > define > > > > > > there > > > > > > > > >> own). > > > > > > > > >> > In > > > > > > > > >> > > > > that > > > > > > > > >> > > > > > > way > > > > > > > > >> > > > > > > > > > Kafka > > > > > > > > >> > > > > > > > > > > will not have to deal with handling > > this. > > > So > > > > > the > > > > > > > > >> > Principal > > > > > > > > >> > > > > > building > > > > > > > > >> > > > > > > > and > > > > > > > > >> > > > > > > > > > > Authorization will be opaque to Kafka > > > which > > > > > > seems > > > > > > > > >> like an > > > > > > > > >> > > > > > expected > > > > > > > > >> > > > > > > > > > > behavior. > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > Hmm, normally, the configurations you > > > > specify > > > > > > for > > > > > > > > >> > plug-ins > > > > > > > > >> > > > > refer > > > > > > > > >> > > > > > to > > > > > > > > >> > > > > > > > > those > > > > > > > > >> > > > > > > > > > > needed to construct the plug-in > object. > > > So, > > > > > it's > > > > > > > > kind > > > > > > > > >> of > > > > > > > > >> > > > weird > > > > > > > > >> > > > > to > > > > > > > > >> > > > > > > use > > > > > > > > >> > > > > > > > > > that > > > > > > > > >> > > > > > > > > > > to call a method. For example, why > can't > > > > > > > > >> > > > > > > > principalBuilderService.rest. > > > > > > > > >> > > > > > > > > > url > > > > > > > > >> > > > > > > > > > > be passed in through the configure() > > > method > > > > > and > > > > > > > the > > > > > > > > >> > > > > > implementation > > > > > > > > >> > > > > > > > can > > > > > > > > >> > > > > > > > > > use > > > > > > > > >> > > > > > > > > > > that to build principal. This way, > there > > > is > > > > > > only a > > > > > > > > >> single > > > > > > > > >> > > > > method > > > > > > > > >> > > > > > to > > > > > > > > >> > > > > > > > > > compute > > > > > > > > >> > > > > > > > > > > the principal in a consistent way in > the > > > > > broker > > > > > > > and > > > > > > > > in > > > > > > > > >> > the > > > > > > > > >> > > > > > > kafka-acl > > > > > > > > >> > > > > > > > > > tool. > > > > > > > > >> > > > > > > > > > > ----> We can do that as well. But > since > > > the > > > > > rest > > > > > > > url > > > > > > > > >> is > > > > > > > > >> > not > > > > > > > > >> > > > > > related > > > > > > > > >> > > > > > > > to > > > > > > > > >> > > > > > > > > > the > > > > > > > > >> > > > > > > > > > > Principal, it seems out of place to me > > to > > > > pass > > > > > > it > > > > > > > > >> every > > > > > > > > >> > > time > > > > > > > > >> > > > we > > > > > > > > >> > > > > > > have > > > > > > > > >> > > > > > > > to > > > > > > > > >> > > > > > > > > > > create a Principal. I should replace > > > > > > > > >> "principalConfigs" > > > > > > > > >> > > with > > > > > > > > >> > > > > > > > > > > "principalProperties". > > > > > > > > >> > > > > > > > > > > I was trying to differentiate the > > > > > > > configs/properties > > > > > > > > >> that > > > > > > > > >> > > are > > > > > > > > >> > > > > > used > > > > > > > > >> > > > > > > to > > > > > > > > >> > > > > > > > > > > create the PrincipalBuilder class and > > the > > > > > > > > >> > > > Principal/Principals > > > > > > > > >> > > > > > > > itself. > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > For LinkedIn's use case, do you > actually > > > use > > > > > the > > > > > > > > >> > kafka-acl > > > > > > > > >> > > > > tool? > > > > > > > > >> > > > > > My > > > > > > > > >> > > > > > > > > > > understanding is that LinkedIn does > > > > > > authorization > > > > > > > > >> through > > > > > > > > >> > > an > > > > > > > > >> > > > > > > external > > > > > > > > >> > > > > > > > > > tool. > > > > > > > > >> > > > > > > > > > > ----> For Linkedin's use case we don't > > > > > actually > > > > > > > use > > > > > > > > >> the > > > > > > > > >> > > > > kafka-acl > > > > > > > > >> > > > > > > > tool > > > > > > > > >> > > > > > > > > > > right now. As per the discussion that > we > > > had > > > > > on > > > > > > > > >> > > > > > > > > > > https://issues.apache.org/ > > > > > > jira/browse/KAFKA-4454, > > > > > > > > we > > > > > > > > >> > > thought > > > > > > > > >> > > > > > that > > > > > > > > >> > > > > > > it > > > > > > > > >> > > > > > > > > > would > > > > > > > > >> > > > > > > > > > > be good to make kafka-acl tool > changes, > > to > > > > > make > > > > > > it > > > > > > > > >> > flexible > > > > > > > > >> > > > and > > > > > > > > >> > > > > > we > > > > > > > > >> > > > > > > > > might > > > > > > > > >> > > > > > > > > > be > > > > > > > > >> > > > > > > > > > > even able to use it in future. > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > It seems it's simpler if kafka-acl > > doesn't > > > > to > > > > > > need > > > > > > > > to > > > > > > > > >> > > > > understand > > > > > > > > >> > > > > > > the > > > > > > > > >> > > > > > > > > > > principal builder. The tool does > > > > authorization > > > > > > > based > > > > > > > > >> on a > > > > > > > > >> > > > > string > > > > > > > > >> > > > > > > > name, > > > > > > > > >> > > > > > > > > > > which is expected to match the > principal > > > > name. > > > > > > > So, I > > > > > > > > >> am > > > > > > > > >> > > > > wondering > > > > > > > > >> > > > > > > why > > > > > > > > >> > > > > > > > > the > > > > > > > > >> > > > > > > > > > > tool needs to know the principal > > builder. > > > > > > > > >> > > > > > > > > > > ----> If we don't make this change, I > am > > > not > > > > > > sure > > > > > > > > how > > > > > > > > >> > > > > clients/end > > > > > > > > >> > > > > > > > users > > > > > > > > >> > > > > > > > > > > will be able to use this tool if they > > have > > > > > there > > > > > > > own > > > > > > > > >> > > > Authorizer > > > > > > > > >> > > > > > > that > > > > > > > > >> > > > > > > > > does > > > > > > > > >> > > > > > > > > > > Authorization based on Principal, that > > has > > > > > more > > > > > > > > >> > information > > > > > > > > >> > > > > apart > > > > > > > > >> > > > > > > > from > > > > > > > > >> > > > > > > > > > name > > > > > > > > >> > > > > > > > > > > and type. > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > What if we only make the following > > > changes: > > > > > pass > > > > > > > the > > > > > > > > >> java > > > > > > > > >> > > > > > principal > > > > > > > > >> > > > > > > > in > > > > > > > > >> > > > > > > > > > > session and in > > > > > > > > >> > > > > > > > > > > SimpleAuthorizer, construct > > KafkaPrincipal > > > > > from > > > > > > > java > > > > > > > > >> > > > principal > > > > > > > > >> > > > > > > name. > > > > > > > > >> > > > > > > > > Will > > > > > > > > >> > > > > > > > > > > that work for LinkedIn? > > > > > > > > >> > > > > > > > > > > ----> This can work for Linkedin but > as > > > > > > explained > > > > > > > > >> above, > > > > > > > > >> > it > > > > > > > > >> > > > > does > > > > > > > > >> > > > > > > not > > > > > > > > >> > > > > > > > > seem > > > > > > > > >> > > > > > > > > > > like a complete design from open > source > > > > point > > > > > of > > > > > > > > view. > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > Thanks, > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > Mayuresh > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > On Thu, Feb 9, 2017 at 11:29 AM, Jun > > Rao < > > > > > > > > >> > j...@confluent.io > > > > > > > > >> > > > > > > > > > > > >> > > > > > wrote: > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > Hi, Mayuresh, > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > Thanks for the reply. A few more > > > comments > > > > > > below. > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > On Wed, Feb 8, 2017 at 9:14 PM, > > Mayuresh > > > > > > Gharat > > > > > > > < > > > > > > > > >> > > > > > > > > > > > gharatmayures...@gmail.com> > > > > > > > > >> > > > > > > > > > > > wrote: > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > Hi Jun, > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > Thanks for the review. Please find > > the > > > > > > > responses > > > > > > > > >> > > inline. > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > 1. It seems the problem that you > are > > > > > trying > > > > > > to > > > > > > > > >> > address > > > > > > > > >> > > is > > > > > > > > >> > > > > > that > > > > > > > > >> > > > > > > > java > > > > > > > > >> > > > > > > > > > > > > principal returned from > KafkaChannel > > > may > > > > > > have > > > > > > > > >> > > additional > > > > > > > > >> > > > > > fields > > > > > > > > >> > > > > > > > > than > > > > > > > > >> > > > > > > > > > > name > > > > > > > > >> > > > > > > > > > > > > that are needed during > > authorization. > > > > Have > > > > > > you > > > > > > > > >> > > > considered a > > > > > > > > >> > > > > > > > > > customized > > > > > > > > >> > > > > > > > > > > > > PrincipleBuilder that extracts all > > > > needed > > > > > > > fields > > > > > > > > >> from > > > > > > > > >> > > > java > > > > > > > > >> > > > > > > > > principal > > > > > > > > >> > > > > > > > > > > and > > > > > > > > >> > > > > > > > > > > > > squeezes them as a json in the > name > > of > > > > the > > > > > > > > >> returned > > > > > > > > >> > > > > > principal? > > > > > > > > >> > > > > > > > > Then, > > > > > > > > >> > > > > > > > > > > the > > > > > > > > >> > > > > > > > > > > > > authorizer can just parse the json > > and > > > > > > extract > > > > > > > > >> needed > > > > > > > > >> > > > > fields. > > > > > > > > >> > > > > > > > > > > > > ---> Yes we had thought about > this. > > We > > > > > use a > > > > > > > > third > > > > > > > > >> > > party > > > > > > > > >> > > > > > > library > > > > > > > > >> > > > > > > > > that > > > > > > > > >> > > > > > > > > > > > takes > > > > > > > > >> > > > > > > > > > > > > in the passed in cert and creates > > the > > > > > > > Principal. > > > > > > > > >> This > > > > > > > > >> > > > > > Principal > > > > > > > > >> > > > > > > > is > > > > > > > > >> > > > > > > > > > then > > > > > > > > >> > > > > > > > > > > > > used by the library to make the > > > decision > > > > > > > > >> (ALLOW/DENY) > > > > > > > > >> > > > when > > > > > > > > >> > > > > we > > > > > > > > >> > > > > > > > call > > > > > > > > >> > > > > > > > > it > > > > > > > > >> > > > > > > > > > > in > > > > > > > > >> > > > > > > > > > > > > the Authorizer. It does not have > an > > > API > > > > to > > > > > > > > create > > > > > > > > >> the > > > > > > > > >> > > > > > Principal > > > > > > > > >> > > > > > > > > from > > > > > > > > >> > > > > > > > > > a > > > > > > > > >> > > > > > > > > > > > > String. If it did support, still > we > > > > would > > > > > > have > > > > > > > > to > > > > > > > > >> be > > > > > > > > >> > > > aware > > > > > > > > >> > > > > of > > > > > > > > >> > > > > > > the > > > > > > > > >> > > > > > > > > > > > internal > > > > > > > > >> > > > > > > > > > > > > details of the library, like the > > field > > > > > > values > > > > > > > it > > > > > > > > >> > > creates > > > > > > > > >> > > > > from > > > > > > > > >> > > > > > > the > > > > > > > > >> > > > > > > > > > > certs, > > > > > > > > >> > > > > > > > > > > > > defaults and so on. > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > 2. Could you explain how the > default > > > > > > > authorizer > > > > > > > > >> works > > > > > > > > >> > > > now? > > > > > > > > >> > > > > > > > > Currently, > > > > > > > > >> > > > > > > > > > > the > > > > > > > > >> > > > > > > > > > > > > code just compares the two > principal > > > > > > objects. > > > > > > > > Are > > > > > > > > >> we > > > > > > > > >> > > > > > converting > > > > > > > > >> > > > > > > > the > > > > > > > > >> > > > > > > > > > > java > > > > > > > > >> > > > > > > > > > > > > principal to a KafkaPrincipal > there? > > > > > > > > >> > > > > > > > > > > > > ---> The SimpleAclAuthorizer > > currently > > > > > > expects > > > > > > > > >> that, > > > > > > > > >> > > the > > > > > > > > >> > > > > > > > Principal > > > > > > > > >> > > > > > > > > it > > > > > > > > >> > > > > > > > > > > > > fetches from the Session object is > > an > > > > > > instance > > > > > > > > of > > > > > > > > >> > > > > > > KafkaPrincipal. > > > > > > > > >> > > > > > > > > It > > > > > > > > >> > > > > > > > > > > then > > > > > > > > >> > > > > > > > > > > > > uses it compare with the > > > KafkaPrincipal > > > > > > > > extracted > > > > > > > > >> > from > > > > > > > > >> > > > the > > > > > > > > >> > > > > > > stored > > > > > > > > >> > > > > > > > > > ACLs. > > > > > > > > >> > > > > > > > > > > > In > > > > > > > > >> > > > > > > > > > > > > this case, we can construct the > > > > > > KafkaPrincipal > > > > > > > > >> object > > > > > > > > >> > > on > > > > > > > > >> > > > > the > > > > > > > > >> > > > > > > fly > > > > > > > > >> > > > > > > > by > > > > > > > > >> > > > > > > > > > > using > > > > > > > > >> > > > > > > > > > > > > the name of the Principal as > > follows : > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > *val principal = > session.principal* > > > > > > > > >> > > > > > > > > > > > > *val kafkaPrincipal = new > > > > > > > > >> > > KafkaPrincipal(KafkaPrincipal. > > > > > > > > >> > > > > > > > USER_TYPE, > > > > > > > > >> > > > > > > > > > > > > principal.getName)* > > > > > > > > >> > > > > > > > > > > > > I was also planning to get rid of > > the > > > > > > > > >> principalType > > > > > > > > >> > > field > > > > > > > > >> > > > > in > > > > > > > > >> > > > > > > > > > > > > KafkaPrincipal as > > > > > > > > >> > > > > > > > > > > > > it is always set to *"*User*"* in > > the > > > > > > > > SocketServer > > > > > > > > >> > > > > currently. > > > > > > > > >> > > > > > > > After > > > > > > > > >> > > > > > > > > > > this > > > > > > > > >> > > > > > > > > > > > > KIP, it will no longer be used in > > > > > > > SocketServer. > > > > > > > > >> But > > > > > > > > >> > to > > > > > > > > >> > > > > > maintain > > > > > > > > >> > > > > > > > > > > backwards > > > > > > > > >> > > > > > > > > > > > > compatibility of kafka-acls.sh, I > > > > > preserved > > > > > > > it. > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > This is so that in the future, we > can > > > > extend > > > > > > to > > > > > > > > >> types > > > > > > > > >> > > like > > > > > > > > >> > > > > > group. > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > 3. Do we need to add the following > > > > method > > > > > in > > > > > > > > >> > > > > > PrincipalBuilder? > > > > > > > > >> > > > > > > > The > > > > > > > > >> > > > > > > > > > > > configs > > > > > > > > >> > > > > > > > > > > > > are already passed in through > > > > configure() > > > > > > and > > > > > > > an > > > > > > > > >> > > > > > implementation > > > > > > > > >> > > > > > > > can > > > > > > > > >> > > > > > > > > > > cache > > > > > > > > >> > > > > > > > > > > > > it and use it in buildPrincipal(). > > > It's > > > > > also > > > > > > > not > > > > > > > > >> > clear > > > > > > > > >> > > to > > > > > > > > >> > > > > me > > > > > > > > >> > > > > > > > where > > > > > > > > >> > > > > > > > > we > > > > > > > > >> > > > > > > > > > > > call > > > > > > > > >> > > > > > > > > > > > > the new and the old method, and > > > whether > > > > > both > > > > > > > > will > > > > > > > > >> be > > > > > > > > >> > > > called > > > > > > > > >> > > > > > or > > > > > > > > >> > > > > > > > one > > > > > > > > >> > > > > > > > > of > > > > > > > > >> > > > > > > > > > > > them > > > > > > > > >> > > > > > > > > > > > > will be called. > > > > > > > > >> > > > > > > > > > > > > Principal > buildPrincipal(Map<String, > > > ?> > > > > > > > > >> > > > principalConfigs); > > > > > > > > >> > > > > > > > > > > > > ---> My thought was that the > > > configure() > > > > > > > method > > > > > > > > >> will > > > > > > > > >> > be > > > > > > > > >> > > > > used > > > > > > > > >> > > > > > to > > > > > > > > >> > > > > > > > > build > > > > > > > > >> > > > > > > > > > > the > > > > > > > > >> > > > > > > > > > > > > PrincipalBuilder class object > > itself. > > > It > > > > > > > follows > > > > > > > > >> the > > > > > > > > >> > > same > > > > > > > > >> > > > > way > > > > > > > > >> > > > > > > as > > > > > > > > >> > > > > > > > > > > > Authorizer > > > > > > > > >> > > > > > > > > > > > > gets configured. The > > > > > > > buildPrincipal(Map<String, > > > > > > > > ?> > > > > > > > > >> > > > > > > > > principalConfigs) > > > > > > > > >> > > > > > > > > > > will > > > > > > > > >> > > > > > > > > > > > > be used to build individual > > > principals. > > > > > > > > >> > > > > > > > > > > > > Let me give an example, with the > > > > > > > kafka-acls.sh : > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > - bin/kafka-acls.sh > > > > --principalBuilder > > > > > > > > >> > > > > > > > > > > > > userDefinedPackage.kafka. > > > > > > > > >> > security.PrincipalBuilder > > > > > > > > >> > > > > > > > > > > > > --principalBuilder-properties > > > > > > > > >> > > > > > > > > > > > > principalBuilderService.rest.u > > > rl=URL > > > > > > > > >> > --authorizer > > > > > > > > >> > > > > > > > > > > > > kafka.security.auth. > > > > > SimpleAclAuthorizer > > > > > > > > >> > > > > > > > --authorizer-properties > > > > > > > > >> > > > > > > > > > > > > zookeeper.connect=localhost: > 2181 > > > > --add > > > > > > > > >> > > > > --allow-principal > > > > > > > > >> > > > > > > > > name=bob > > > > > > > > >> > > > > > > > > > > > > type=USER_PRINCIPAL > > > --allow-principal > > > > > > > > >> > > > > > > name=ALPHA-GAMMA-SERVICE > > > > > > > > >> > > > > > > > > > > > > type=SERVICE_PRINCIPAL > > > --allow-hosts > > > > > > > > >> Host1,Host2 > > > > > > > > >> > > > > > > --operations > > > > > > > > >> > > > > > > > > > > > Read,Write > > > > > > > > >> > > > > > > > > > > > > --topic Test-topic > > > > > > > > >> > > > > > > > > > > > > 1. > *userDefinedPackage.kafka. > > > > > > > > >> > > > > > security.PrincipalBuilder* > > > > > > > > >> > > > > > > is > > > > > > > > >> > > > > > > > > the > > > > > > > > >> > > > > > > > > > > > user > > > > > > > > >> > > > > > > > > > > > > defined PrincipalBuilder > > class. > > > > > > > > >> > > > > > > > > > > > > 2. > > > *principalBuilderService.rest. > > > > > > > url=URL* > > > > > > > > >> can > > > > > > > > >> > > be a > > > > > > > > >> > > > > > > remote > > > > > > > > >> > > > > > > > > > > service > > > > > > > > >> > > > > > > > > > > > > that provides you an HTTP > > > endpoint > > > > > > which > > > > > > > > >> takes > > > > > > > > >> > > in a > > > > > > > > >> > > > > set > > > > > > > > >> > > > > > > of > > > > > > > > >> > > > > > > > > > > > > parameters and > > > > > > > > >> > > > > > > > > > > > > provides you with the > > Principal. > > > > > > > > >> > > > > > > > > > > > > 3. *name=bob > > > type=USER_PRINCIPAL* > > > > > can > > > > > > be > > > > > > > > >> used > > > > > > > > >> > by > > > > > > > > >> > > > > > > > > > PrincipalBuilder > > > > > > > > >> > > > > > > > > > > > to > > > > > > > > >> > > > > > > > > > > > > create UserPrincipal with > name > > > as > > > > > bob > > > > > > > > >> > > > > > > > > > > > > 4. *name=ALPHA-GAMMA-SERVICE > > > > > > > > >> > > type=SERVICE_PRINCIPAL > > > > > > > > >> > > > > > *can > > > > > > > > >> > > > > > > be > > > > > > > > >> > > > > > > > > > used > > > > > > > > >> > > > > > > > > > > by > > > > > > > > >> > > > > > > > > > > > > PrincipalBuilder to create a > > > > > > > > >> ServicePrincipal > > > > > > > > >> > > with > > > > > > > > >> > > > > name > > > > > > > > >> > > > > > > as > > > > > > > > >> > > > > > > > > > > > > ALPHA-GAMMA-SERVICE. > > > > > > > > >> > > > > > > > > > > > > - This seems more flexible and > > > > > intuitive > > > > > > to > > > > > > > > me > > > > > > > > >> > from > > > > > > > > >> > > > end > > > > > > > > >> > > > > > > user's > > > > > > > > >> > > > > > > > > > > > > perspective. > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > Hmm, normally, the configurations > you > > > > > specify > > > > > > > for > > > > > > > > >> > > plug-ins > > > > > > > > >> > > > > > refer > > > > > > > > >> > > > > > > to > > > > > > > > >> > > > > > > > > > those > > > > > > > > >> > > > > > > > > > > > needed to construct the plug-in > > object. > > > > So, > > > > > > it's > > > > > > > > >> kind > > > > > > > > >> > of > > > > > > > > >> > > > > weird > > > > > > > > >> > > > > > to > > > > > > > > >> > > > > > > > use > > > > > > > > >> > > > > > > > > > > that > > > > > > > > >> > > > > > > > > > > > to call a method. For example, why > > can't > > > > > > > > >> > > > > > > > > principalBuilderService.rest. > > > > > > > > >> > > > > > > > > > > url > > > > > > > > >> > > > > > > > > > > > be passed in through the configure() > > > > method > > > > > > and > > > > > > > > the > > > > > > > > >> > > > > > > implementation > > > > > > > > >> > > > > > > > > can > > > > > > > > >> > > > > > > > > > > use > > > > > > > > >> > > > > > > > > > > > that to build principal. This way, > > there > > > > is > > > > > > > only a > > > > > > > > >> > single > > > > > > > > >> > > > > > method > > > > > > > > >> > > > > > > to > > > > > > > > >> > > > > > > > > > > compute > > > > > > > > >> > > > > > > > > > > > the principal in a consistent way in > > the > > > > > > broker > > > > > > > > and > > > > > > > > >> in > > > > > > > > >> > > the > > > > > > > > >> > > > > > > > kafka-acl > > > > > > > > >> > > > > > > > > > > tool. > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > For LinkedIn's use case, do you > > actually > > > > use > > > > > > the > > > > > > > > >> > > kafka-acl > > > > > > > > >> > > > > > tool? > > > > > > > > >> > > > > > > My > > > > > > > > >> > > > > > > > > > > > understanding is that LinkedIn does > > > > > > > authorization > > > > > > > > >> > through > > > > > > > > >> > > > an > > > > > > > > >> > > > > > > > external > > > > > > > > >> > > > > > > > > > > tool. > > > > > > > > >> > > > > > > > > > > > It seems it's simpler if kafka-acl > > > doesn't > > > > > to > > > > > > > need > > > > > > > > >> to > > > > > > > > >> > > > > > understand > > > > > > > > >> > > > > > > > the > > > > > > > > >> > > > > > > > > > > > principal builder. The tool does > > > > > authorization > > > > > > > > based > > > > > > > > >> > on a > > > > > > > > >> > > > > > string > > > > > > > > >> > > > > > > > > name, > > > > > > > > >> > > > > > > > > > > > which is expected to match the > > principal > > > > > name. > > > > > > > So, > > > > > > > > >> I am > > > > > > > > >> > > > > > wondering > > > > > > > > >> > > > > > > > why > > > > > > > > >> > > > > > > > > > the > > > > > > > > >> > > > > > > > > > > > tool needs to know the principal > > > builder. > > > > > What > > > > > > > if > > > > > > > > we > > > > > > > > >> > only > > > > > > > > >> > > > > make > > > > > > > > >> > > > > > > the > > > > > > > > >> > > > > > > > > > > > following changes: pass the java > > > principal > > > > > in > > > > > > > > >> session > > > > > > > > >> > and > > > > > > > > >> > > > in > > > > > > > > >> > > > > > > > > > > > SimpleAuthorizer, construct > > > KafkaPrincipal > > > > > > from > > > > > > > > java > > > > > > > > >> > > > > principal > > > > > > > > >> > > > > > > > name. > > > > > > > > >> > > > > > > > > > Will > > > > > > > > >> > > > > > > > > > > > that work for LinkedIn? > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > Principal > buildPrincipal(Map<String, > > > ?> > > > > > > > > >> > > principalConfigs) > > > > > > > > >> > > > > > will > > > > > > > > >> > > > > > > be > > > > > > > > >> > > > > > > > > > > called > > > > > > > > >> > > > > > > > > > > > > from the commandline client > > > > kafka-acls.sh > > > > > > > while > > > > > > > > >> the > > > > > > > > >> > > other > > > > > > > > >> > > > > API > > > > > > > > >> > > > > > > can > > > > > > > > >> > > > > > > > > be > > > > > > > > >> > > > > > > > > > > > called > > > > > > > > >> > > > > > > > > > > > > at runtime when Kafka receives a > > > client > > > > > > > request > > > > > > > > >> over > > > > > > > > >> > > > > request > > > > > > > > >> > > > > > > > > channel. > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > 4. The KIP has "If users use there > > > > custom > > > > > > > > >> > > > PrincipalBuilder, > > > > > > > > >> > > > > > > they > > > > > > > > >> > > > > > > > > will > > > > > > > > >> > > > > > > > > > > > have > > > > > > > > >> > > > > > > > > > > > > to implement there custom > Authorizer > > > as > > > > > the > > > > > > > out > > > > > > > > of > > > > > > > > >> > box > > > > > > > > >> > > > > > > Authorizer > > > > > > > > >> > > > > > > > > > that > > > > > > > > >> > > > > > > > > > > > > Kafka provides uses > KafkaPrincipal." > > > > This > > > > > is > > > > > > > not > > > > > > > > >> > ideal > > > > > > > > >> > > > for > > > > > > > > >> > > > > > > > existing > > > > > > > > >> > > > > > > > > > > > users. > > > > > > > > >> > > > > > > > > > > > > Could we avoid that? > > > > > > > > >> > > > > > > > > > > > > ---> Yes, this is possible to > avoid > > if > > > > we > > > > > do > > > > > > > > >> point 2. > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > Thanks, > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > Mayuresh > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > On Wed, Feb 8, 2017 at 3:31 PM, > Jun > > > Rao > > > > < > > > > > > > > >> > > > j...@confluent.io> > > > > > > > > >> > > > > > > > wrote: > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > Hi, Mayuresh, > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > Thanks for the KIP. A few > comments > > > > > below. > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > 1. It seems the problem that you > > are > > > > > > trying > > > > > > > to > > > > > > > > >> > > address > > > > > > > > >> > > > is > > > > > > > > >> > > > > > > that > > > > > > > > >> > > > > > > > > java > > > > > > > > >> > > > > > > > > > > > > > principal returned from > > KafkaChannel > > > > may > > > > > > > have > > > > > > > > >> > > > additional > > > > > > > > >> > > > > > > fields > > > > > > > > >> > > > > > > > > > than > > > > > > > > >> > > > > > > > > > > > name > > > > > > > > >> > > > > > > > > > > > > > that are needed during > > > authorization. > > > > > Have > > > > > > > you > > > > > > > > >> > > > > considered a > > > > > > > > >> > > > > > > > > > > customized > > > > > > > > >> > > > > > > > > > > > > > PrincipleBuilder that extracts > all > > > > > needed > > > > > > > > fields > > > > > > > > >> > from > > > > > > > > >> > > > > java > > > > > > > > >> > > > > > > > > > principal > > > > > > > > >> > > > > > > > > > > > and > > > > > > > > >> > > > > > > > > > > > > > squeezes them as a json in the > > name > > > of > > > > > the > > > > > > > > >> returned > > > > > > > > >> > > > > > > principal? > > > > > > > > >> > > > > > > > > > Then, > > > > > > > > >> > > > > > > > > > > > the > > > > > > > > >> > > > > > > > > > > > > > authorizer can just parse the > json > > > and > > > > > > > extract > > > > > > > > >> > needed > > > > > > > > >> > > > > > fields. > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > 2. Could you explain how the > > default > > > > > > > > authorizer > > > > > > > > >> > works > > > > > > > > >> > > > > now? > > > > > > > > >> > > > > > > > > > Currently, > > > > > > > > >> > > > > > > > > > > > the > > > > > > > > >> > > > > > > > > > > > > > code just compares the two > > principal > > > > > > > objects. > > > > > > > > >> Are > > > > > > > > >> > we > > > > > > > > >> > > > > > > converting > > > > > > > > >> > > > > > > > > the > > > > > > > > >> > > > > > > > > > > > java > > > > > > > > >> > > > > > > > > > > > > > principal to a KafkaPrincipal > > there? > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > 3. Do we need to add the > following > > > > > method > > > > > > in > > > > > > > > >> > > > > > > PrincipalBuilder? > > > > > > > > >> > > > > > > > > The > > > > > > > > >> > > > > > > > > > > > > configs > > > > > > > > >> > > > > > > > > > > > > > are already passed in through > > > > > configure() > > > > > > > and > > > > > > > > an > > > > > > > > >> > > > > > > implementation > > > > > > > > >> > > > > > > > > can > > > > > > > > >> > > > > > > > > > > > cache > > > > > > > > >> > > > > > > > > > > > > > it and use it in > buildPrincipal(). > > > > It's > > > > > > also > > > > > > > > not > > > > > > > > >> > > clear > > > > > > > > >> > > > to > > > > > > > > >> > > > > > me > > > > > > > > >> > > > > > > > > where > > > > > > > > >> > > > > > > > > > we > > > > > > > > >> > > > > > > > > > > > > call > > > > > > > > >> > > > > > > > > > > > > > the new and the old method, and > > > > whether > > > > > > both > > > > > > > > >> will > > > > > > > > >> > be > > > > > > > > >> > > > > called > > > > > > > > >> > > > > > > or > > > > > > > > >> > > > > > > > > one > > > > > > > > >> > > > > > > > > > of > > > > > > > > >> > > > > > > > > > > > > them > > > > > > > > >> > > > > > > > > > > > > > will be called. > > > > > > > > >> > > > > > > > > > > > > > Principal > > buildPrincipal(Map<String, > > > > ?> > > > > > > > > >> > > > > principalConfigs); > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > 4. The KIP has "If users use > there > > > > > custom > > > > > > > > >> > > > > PrincipalBuilder, > > > > > > > > >> > > > > > > > they > > > > > > > > >> > > > > > > > > > will > > > > > > > > >> > > > > > > > > > > > > have > > > > > > > > >> > > > > > > > > > > > > > to implement there custom > > Authorizer > > > > as > > > > > > the > > > > > > > > out > > > > > > > > >> of > > > > > > > > >> > > box > > > > > > > > >> > > > > > > > Authorizer > > > > > > > > >> > > > > > > > > > > that > > > > > > > > >> > > > > > > > > > > > > > Kafka provides uses > > KafkaPrincipal." > > > > > This > > > > > > is > > > > > > > > not > > > > > > > > >> > > ideal > > > > > > > > >> > > > > for > > > > > > > > >> > > > > > > > > existing > > > > > > > > >> > > > > > > > > > > > > users. > > > > > > > > >> > > > > > > > > > > > > > Could we avoid that? > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > Thanks, > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > Jun > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > On Fri, Feb 3, 2017 at 11:25 AM, > > > > > Mayuresh > > > > > > > > >> Gharat < > > > > > > > > >> > > > > > > > > > > > > > gharatmayures...@gmail.com > > > > > > > > >> > > > > > > > > > > > > > > wrote: > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > Hi All, > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > It seems that there is no > > further > > > > > > concern > > > > > > > > with > > > > > > > > >> > the > > > > > > > > >> > > > > > KIP-111. > > > > > > > > >> > > > > > > > At > > > > > > > > >> > > > > > > > > > this > > > > > > > > >> > > > > > > > > > > > > point > > > > > > > > >> > > > > > > > > > > > > > > we would like to start the > > voting > > > > > > process. > > > > > > > > The > > > > > > > > >> > KIP > > > > > > > > >> > > > can > > > > > > > > >> > > > > be > > > > > > > > >> > > > > > > > found > > > > > > > > >> > > > > > > > > > at > > > > > > > > >> > > > > > > > > > > > > > > https://cwiki.apache.org/ > > > > > > > > >> > confluence/pages/viewpage > > > > > > > > >> > > . > > > > > > > > >> > > > > > > > > > > > > > action?pageId=67638388 > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > Thanks, > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > Mayuresh > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > Thanks, > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > -- > > > > > > > > >> > > > > > > > > > > > > -Regards, > > > > > > > > >> > > > > > > > > > > > > Mayuresh R. Gharat > > > > > > > > >> > > > > > > > > > > > > (862) 250-7125 > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > Thanks, > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > Jun > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > -- > > > > > > > > >> > > > > > > > > > > -Regards, > > > > > > > > >> > > > > > > > > > > Mayuresh R. Gharat > > > > > > > > >> > > > > > > > > > > (862) 250-7125 > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > -- > > > > > > > > >> > > > > -Regards, > > > > > > > > >> > > > > Mayuresh R. Gharat > > > > > > > > >> > > > > (862) 250-7125 > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > -- > > > > > > > > >> > -Regards, > > > > > > > > >> > Mayuresh R. Gharat > > > > > > > > >> > (862) 250-7125 > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > -Regards, > > > > > > > > > Mayuresh R. Gharat > > > > > > > > > (862) 250-7125 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > -Regards, > > > > > > > > Mayuresh R. Gharat > > > > > > > > (862) 250-7125 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > -Regards, > > > > > > Mayuresh R. Gharat > > > > > > (862) 250-7125 > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > -Regards, > > > > Mayuresh R. Gharat > > > > (862) 250-7125 > > > > > > > > > > > > > -- > -Regards, > Mayuresh R. Gharat > (862) 250-7125 >
