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.url=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
