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.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 > -- -Regards, Mayuresh R. Gharat (862) 250-7125
