Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
+1 On Fri, Feb 3, 2017 at 11:25 AM, Mayuresh Gharat 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 >
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
Bumping up this thread. Thanks, Mayuresh On Fri, Feb 3, 2017 at 5:09 PM, radai wrote: > +1 > > 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 > > > -- -Regards, Mayuresh R. Gharat (862) 250-7125
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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 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 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 >
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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. 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 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 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. Principal buildPrincipal(Map 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 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 Prin
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
Hi, Mayuresh, Thanks for the reply. A few more comments below. On Wed, Feb 8, 2017 at 9:14 PM, Mayuresh Gharat 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 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 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 jav
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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 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
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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 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
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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 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 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 > > >
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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 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 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, > > > > > > M
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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 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 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 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 > > t
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
@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 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 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 > 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 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/
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
@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 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 > 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 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 h
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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 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 Aut
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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 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, > > >
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
@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, 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 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 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 m
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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, > 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 > 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 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. >
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
Hi Jun, Sure sounds good to me. Thanks, Mayuresh On Fri, Feb 17, 2017 at 1:54 PM, Jun Rao 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, > > 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 > > 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 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
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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 wrote: > Hi Jun, > > Sure sounds good to me. > > Thanks, > > Mayuresh > > On Fri, Feb 17, 2017 at 1:54 PM, Jun Rao 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, >> > 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 >> > 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 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
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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 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 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, > >> > 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 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 authoriz
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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 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 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, > > >> > 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 > 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? > > >> > > > > > >> > > > > > >>
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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 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 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 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, > > > >> > 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 > > 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
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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 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 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 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, > > > > >> > 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.c
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
+1 (non-binding) On Thu, Feb 23, 2017 at 3:27 AM, Mayuresh Gharat 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 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 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 > 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, > > > > > >> > 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. > > >
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
+1 (non-binding) On Wed, Feb 22, 2017 at 10:52 PM, Manikumar 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 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 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 > > 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 > > > > k
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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 wrote: > +1 (non-binding) > > On Wed, Feb 22, 2017 at 10:52 PM, Manikumar > 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 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 > 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 > > > 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, > > > > > >
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
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 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 wrote: > > > +1 (non-binding) > > > > On Wed, Feb 22, 2017 at 10:52 PM, Manikumar > > 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 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 > > 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 > > > > > wr
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
Hi Ismael, Yeah. I agree that it might break the clients if the user is using the kafkaPrincipal directly. But since KafkaPrincipal is also a Java Principal and I think, it would be a right thing to do replace the kafkaPrincipal with Java Principal at this stage than later. We can mention in the KIP, that it would break the clients that are using the KafkaPrincipal directly and they will have to use the PrincipalType directly, if they are using it as its only one value and use the name from the Principal directly or create a KafkaPrincipal from Java Principal as we are doing in SimpleAclAuthorizer with this KIP. Thanks, Mayuresh On Mon, Feb 27, 2017 at 10:56 AM, Ismael Juma wrote: > 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 wrote: > > > > > +1 (non-binding) > > > > > > On Wed, Feb 22, 2017 at 10:52 PM, Manikumar > > > > 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 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 > > > 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 > th
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
Breaking clients without a deprecation period is something we only do as a last resort. Is there strong justification for doing it here? Ismael On Mon, Feb 27, 2017 at 11:28 PM, Mayuresh Gharat < gharatmayures...@gmail.com> wrote: > Hi Ismael, > > Yeah. I agree that it might break the clients if the user is using the > kafkaPrincipal directly. But since KafkaPrincipal is also a Java Principal > and I think, it would be a right thing to do replace the kafkaPrincipal > with Java Principal at this stage than later. > > We can mention in the KIP, that it would break the clients that are using > the KafkaPrincipal directly and they will have to use the PrincipalType > directly, if they are using it as its only one value and use the name from > the Principal directly or create a KafkaPrincipal from Java Principal as we > are doing in SimpleAclAuthorizer with this KIP. > > Thanks, > > Mayuresh > > > > On Mon, Feb 27, 2017 at 10:56 AM, Ismael Juma wrote: > > > 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 > 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 > 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 > > > > wrote: > > > > > > > > > > > > > > > > > Hi, Mayuresh, > > > > > > > > > > > > > > > > > > Thanks for the updated KIP. A couple of more comments. > > > > > > > > > > > > > > > > > > 1. Do we convert java.security.Principal to KafkaPrincipal > > for > >
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
Hi, Ismael, Good point on compatibility. Hi, Mayuresh, Given that, it seems that it's better to just add the raw principal as a new field in Session for now and deprecate the KafkaPrincipal field in the future if needed? Thanks, Jun On Mon, Feb 27, 2017 at 5:05 PM, Ismael Juma wrote: > Breaking clients without a deprecation period is something we only do as a > last resort. Is there strong justification for doing it here? > > Ismael > > On Mon, Feb 27, 2017 at 11:28 PM, Mayuresh Gharat < > gharatmayures...@gmail.com> wrote: > > > Hi Ismael, > > > > Yeah. I agree that it might break the clients if the user is using the > > kafkaPrincipal directly. But since KafkaPrincipal is also a Java > Principal > > and I think, it would be a right thing to do replace the kafkaPrincipal > > with Java Principal at this stage than later. > > > > We can mention in the KIP, that it would break the clients that are using > > the KafkaPrincipal directly and they will have to use the PrincipalType > > directly, if they are using it as its only one value and use the name > from > > the Principal directly or create a KafkaPrincipal from Java Principal as > we > > are doing in SimpleAclAuthorizer with this KIP. > > > > Thanks, > > > > Mayuresh > > > > > > > > On Mon, Feb 27, 2017 at 10:56 AM, Ismael Juma wrote: > > > > > 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 > > 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 > > 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
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
Hi Jun/Ismael, Thanks for the comments. I agree. What I was thinking was, we get the KIP passed now and wait till major kafka version release. We can then make this change, but for now we can wait. Does that work? If there are concerns, we can make the addition of extra field of type Principal to Session and then deprecate the KafkaPrincipal later. I am fine either ways. What do you think? Thanks, Mayuresh On Tue, Feb 28, 2017 at 9:53 AM, Jun Rao wrote: > Hi, Ismael, > > Good point on compatibility. > > Hi, Mayuresh, > > Given that, it seems that it's better to just add the raw principal as a > new field in Session for now and deprecate the KafkaPrincipal field in the > future if needed? > > Thanks, > > Jun > > On Mon, Feb 27, 2017 at 5:05 PM, Ismael Juma wrote: > > > Breaking clients without a deprecation period is something we only do as > a > > last resort. Is there strong justification for doing it here? > > > > Ismael > > > > On Mon, Feb 27, 2017 at 11:28 PM, Mayuresh Gharat < > > gharatmayures...@gmail.com> wrote: > > > > > Hi Ismael, > > > > > > Yeah. I agree that it might break the clients if the user is using the > > > kafkaPrincipal directly. But since KafkaPrincipal is also a Java > > Principal > > > and I think, it would be a right thing to do replace the kafkaPrincipal > > > with Java Principal at this stage than later. > > > > > > We can mention in the KIP, that it would break the clients that are > using > > > the KafkaPrincipal directly and they will have to use the PrincipalType > > > directly, if they are using it as its only one value and use the name > > from > > > the Principal directly or create a KafkaPrincipal from Java Principal > as > > we > > > are doing in SimpleAclAuthorizer with this KIP. > > > > > > Thanks, > > > > > > Mayuresh > > > > > > > > > > > > On Mon, Feb 27, 2017 at 10:56 AM, Ismael Juma > wrote: > > > > > > > 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 > > > 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 > > > 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
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
If we deprecate KafkaPrincipal, then the Authorizer interface will also need to change - i.e., deprecate the getAcls(KafkaPrincipal) method. On Tue, Feb 28, 2017 at 10:11 AM, Mayuresh Gharat < gharatmayures...@gmail.com> wrote: > Hi Jun/Ismael, > > Thanks for the comments. > > I agree. > What I was thinking was, we get the KIP passed now and wait till major > kafka version release. We can then make this change, but for now we can > wait. Does that work? > > If there are concerns, we can make the addition of extra field of type > Principal to Session and then deprecate the KafkaPrincipal later. > > I am fine either ways. What do you think? > > Thanks, > > Mayuresh > > On Tue, Feb 28, 2017 at 9:53 AM, Jun Rao wrote: > > > Hi, Ismael, > > > > Good point on compatibility. > > > > Hi, Mayuresh, > > > > Given that, it seems that it's better to just add the raw principal as a > > new field in Session for now and deprecate the KafkaPrincipal field in > the > > future if needed? > > > > Thanks, > > > > Jun > > > > On Mon, Feb 27, 2017 at 5:05 PM, Ismael Juma wrote: > > > > > Breaking clients without a deprecation period is something we only do > as > > a > > > last resort. Is there strong justification for doing it here? > > > > > > Ismael > > > > > > On Mon, Feb 27, 2017 at 11:28 PM, Mayuresh Gharat < > > > gharatmayures...@gmail.com> wrote: > > > > > > > Hi Ismael, > > > > > > > > Yeah. I agree that it might break the clients if the user is using > the > > > > kafkaPrincipal directly. But since KafkaPrincipal is also a Java > > > Principal > > > > and I think, it would be a right thing to do replace the > kafkaPrincipal > > > > with Java Principal at this stage than later. > > > > > > > > We can mention in the KIP, that it would break the clients that are > > using > > > > the KafkaPrincipal directly and they will have to use the > PrincipalType > > > > directly, if they are using it as its only one value and use the name > > > from > > > > the Principal directly or create a KafkaPrincipal from Java Principal > > as > > > we > > > > are doing in SimpleAclAuthorizer with this KIP. > > > > > > > > Thanks, > > > > > > > > Mayuresh > > > > > > > > > > > > > > > > On Mon, Feb 27, 2017 at 10:56 AM, Ismael Juma > > wrote: > > > > > > > > > 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 > > > > 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 > > > > > 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
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
Hi, Joel, Good point on the getAcls() method. KafkaPrincipal is also tied to ACL, which is used in pretty much every method in Authorizer. Now, I am not sure if it's easy to deprecate KafkaPrincipal. Hi, Mayuresh, Given the above, it seems that the easiest thing is to add a new Principal field in Session. We want to make it clear that it's ignored in the default implementation, but a customizer authorizer could take advantage of that. Thanks, Jun On Tue, Feb 28, 2017 at 10:52 AM, Joel Koshy wrote: > If we deprecate KafkaPrincipal, then the Authorizer interface will also > need to change - i.e., deprecate the getAcls(KafkaPrincipal) method. > > On Tue, Feb 28, 2017 at 10:11 AM, Mayuresh Gharat < > gharatmayures...@gmail.com> wrote: > > > Hi Jun/Ismael, > > > > Thanks for the comments. > > > > I agree. > > What I was thinking was, we get the KIP passed now and wait till major > > kafka version release. We can then make this change, but for now we can > > wait. Does that work? > > > > If there are concerns, we can make the addition of extra field of type > > Principal to Session and then deprecate the KafkaPrincipal later. > > > > I am fine either ways. What do you think? > > > > Thanks, > > > > Mayuresh > > > > On Tue, Feb 28, 2017 at 9:53 AM, Jun Rao wrote: > > > > > Hi, Ismael, > > > > > > Good point on compatibility. > > > > > > Hi, Mayuresh, > > > > > > Given that, it seems that it's better to just add the raw principal as > a > > > new field in Session for now and deprecate the KafkaPrincipal field in > > the > > > future if needed? > > > > > > Thanks, > > > > > > Jun > > > > > > On Mon, Feb 27, 2017 at 5:05 PM, Ismael Juma > wrote: > > > > > > > Breaking clients without a deprecation period is something we only do > > as > > > a > > > > last resort. Is there strong justification for doing it here? > > > > > > > > Ismael > > > > > > > > On Mon, Feb 27, 2017 at 11:28 PM, Mayuresh Gharat < > > > > gharatmayures...@gmail.com> wrote: > > > > > > > > > Hi Ismael, > > > > > > > > > > Yeah. I agree that it might break the clients if the user is using > > the > > > > > kafkaPrincipal directly. But since KafkaPrincipal is also a Java > > > > Principal > > > > > and I think, it would be a right thing to do replace the > > kafkaPrincipal > > > > > with Java Principal at this stage than later. > > > > > > > > > > We can mention in the KIP, that it would break the clients that are > > > using > > > > > the KafkaPrincipal directly and they will have to use the > > PrincipalType > > > > > directly, if they are using it as its only one value and use the > name > > > > from > > > > > the Principal directly or create a KafkaPrincipal from Java > Principal > > > as > > > > we > > > > > are doing in SimpleAclAuthorizer with this KIP. > > > > > > > > > > Thanks, > > > > > > > > > > Mayuresh > > > > > > > > > > > > > > > > > > > > On Mon, Feb 27, 2017 at 10:56 AM, Ismael Juma > > > wrote: > > > > > > > > > > > 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 > > > >
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
Hi Jun, Sure. I had an offline discussion with Joel on how we can deprecate the KafkaPrincipal from Session and Authorizer. I will update the KIP to see if we can address all the concerns here. If not we can keep the KafkaPrincipal. Thanks, Mayuresh On Tue, Feb 28, 2017 at 1:53 PM, Jun Rao wrote: > Hi, Joel, > > Good point on the getAcls() method. KafkaPrincipal is also tied to ACL, > which is used in pretty much every method in Authorizer. Now, I am not sure > if it's easy to deprecate KafkaPrincipal. > > Hi, Mayuresh, > > Given the above, it seems that the easiest thing is to add a new Principal > field in Session. We want to make it clear that it's ignored in the default > implementation, but a customizer authorizer could take advantage of that. > > Thanks, > > Jun > > On Tue, Feb 28, 2017 at 10:52 AM, Joel Koshy wrote: > > > If we deprecate KafkaPrincipal, then the Authorizer interface will also > > need to change - i.e., deprecate the getAcls(KafkaPrincipal) method. > > > > On Tue, Feb 28, 2017 at 10:11 AM, Mayuresh Gharat < > > gharatmayures...@gmail.com> wrote: > > > > > Hi Jun/Ismael, > > > > > > Thanks for the comments. > > > > > > I agree. > > > What I was thinking was, we get the KIP passed now and wait till major > > > kafka version release. We can then make this change, but for now we can > > > wait. Does that work? > > > > > > If there are concerns, we can make the addition of extra field of type > > > Principal to Session and then deprecate the KafkaPrincipal later. > > > > > > I am fine either ways. What do you think? > > > > > > Thanks, > > > > > > Mayuresh > > > > > > On Tue, Feb 28, 2017 at 9:53 AM, Jun Rao wrote: > > > > > > > Hi, Ismael, > > > > > > > > Good point on compatibility. > > > > > > > > Hi, Mayuresh, > > > > > > > > Given that, it seems that it's better to just add the raw principal > as > > a > > > > new field in Session for now and deprecate the KafkaPrincipal field > in > > > the > > > > future if needed? > > > > > > > > Thanks, > > > > > > > > Jun > > > > > > > > On Mon, Feb 27, 2017 at 5:05 PM, Ismael Juma > > wrote: > > > > > > > > > Breaking clients without a deprecation period is something we only > do > > > as > > > > a > > > > > last resort. Is there strong justification for doing it here? > > > > > > > > > > Ismael > > > > > > > > > > On Mon, Feb 27, 2017 at 11:28 PM, Mayuresh Gharat < > > > > > gharatmayures...@gmail.com> wrote: > > > > > > > > > > > Hi Ismael, > > > > > > > > > > > > Yeah. I agree that it might break the clients if the user is > using > > > the > > > > > > kafkaPrincipal directly. But since KafkaPrincipal is also a Java > > > > > Principal > > > > > > and I think, it would be a right thing to do replace the > > > kafkaPrincipal > > > > > > with Java Principal at this stage than later. > > > > > > > > > > > > We can mention in the KIP, that it would break the clients that > are > > > > using > > > > > > the KafkaPrincipal directly and they will have to use the > > > PrincipalType > > > > > > directly, if they are using it as its only one value and use the > > name > > > > > from > > > > > > the Principal directly or create a KafkaPrincipal from Java > > Principal > > > > as > > > > > we > > > > > > are doing in SimpleAclAuthorizer with this KIP. > > > > > > > > > > > > Thanks, > > > > > > > > > > > > Mayuresh > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Feb 27, 2017 at 10:56 AM, Ismael Juma > > > > > wrote: > > > > > > > > > > > > > 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) > > > > > > > > > > >
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
Hi Jun,
Sorry for the delayed reply.
I agree that the easiest thing will be to add an additional field in the
Session class and we should be OK.
But having a KafkaPrincipal and java Principal with in the same class looks
little weird.
So we can do this and slowly deprecate the usage of KafkaPrincipal in
public api's.
We add new apis and make changes to the existing apis as follows :
- Changes to Session class :
@Deprecated
case class Session(principal: KafkaPrincipal, clientAddress: InetAddress) {
val sanitizedUser = QuotaId.sanitize(principal.getName)
}
*@Deprecated .. (NEW)*
*case class Session(principal: KafkaPrincipal, clientAddress: InetAddress,
channelPrincipal: Java.security.Principal) {val sanitizedUser =
QuotaId.sanitize(principal.getName)}*
*(NEW)*
*case class Session(principal: Java.security.Principal, clientAddress:
InetAddress) {val sanitizedUser = QuotaId.sanitize(principal.getName)}*
- Changes to Authorizer Interface :
@Deprecated
def getAcls(principal: KafkaPrincipal): Map[Resource, Set[Acl]]
*(NEW)*
*def getAcls(principal: Java.security.Principal): Map[Resource, Set[Acl]]*
- Changes to Acl class :
@Deprecated
case class Acl(principal: KafkaPrincipal, permissionType: PermissionType,
host: String, operation: Operation)
*(NEW)*
*case class Acl(principal: Java.security.Principal, permissionType:
PermissionType, host: String, operation: Operation) *
The one in Bold are the new api's. We will remove them eventually, probably
in next major release.
We don't want to get rid of KafkaPrincipal class and it will be used in the
same way as it does right now for out of box authorizer and commandline
tool. We would only be removing its direct usage from public apis.
Doing the above deprecation will help us to support other implementation of
Java.security.Principal as well which seems necessary especially since
Kafka provides pluggable Authorizer and PrincipalBuilder.
Let me know your thoughts on this.
Thanks,
Mayuresh
On Tue, Feb 28, 2017 at 2:33 PM, Mayuresh Gharat wrote:
> Hi Jun,
>
> Sure.
> I had an offline discussion with Joel on how we can deprecate the
> KafkaPrincipal from Session and Authorizer.
> I will update the KIP to see if we can address all the concerns here. If
> not we can keep the KafkaPrincipal.
>
> Thanks,
>
> Mayuresh
>
> On Tue, Feb 28, 2017 at 1:53 PM, Jun Rao wrote:
>
>> Hi, Joel,
>>
>> Good point on the getAcls() method. KafkaPrincipal is also tied to ACL,
>> which is used in pretty much every method in Authorizer. Now, I am not
>> sure
>> if it's easy to deprecate KafkaPrincipal.
>>
>> Hi, Mayuresh,
>>
>> Given the above, it seems that the easiest thing is to add a new Principal
>> field in Session. We want to make it clear that it's ignored in the
>> default
>> implementation, but a customizer authorizer could take advantage of that.
>>
>> Thanks,
>>
>> Jun
>>
>> On Tue, Feb 28, 2017 at 10:52 AM, Joel Koshy wrote:
>>
>> > If we deprecate KafkaPrincipal, then the Authorizer interface will also
>> > need to change - i.e., deprecate the getAcls(KafkaPrincipal) method.
>> >
>> > On Tue, Feb 28, 2017 at 10:11 AM, Mayuresh Gharat <
>> > gharatmayures...@gmail.com> wrote:
>> >
>> > > Hi Jun/Ismael,
>> > >
>> > > Thanks for the comments.
>> > >
>> > > I agree.
>> > > What I was thinking was, we get the KIP passed now and wait till major
>> > > kafka version release. We can then make this change, but for now we
>> can
>> > > wait. Does that work?
>> > >
>> > > If there are concerns, we can make the addition of extra field of type
>> > > Principal to Session and then deprecate the KafkaPrincipal later.
>> > >
>> > > I am fine either ways. What do you think?
>> > >
>> > > Thanks,
>> > >
>> > > Mayuresh
>> > >
>> > > On Tue, Feb 28, 2017 at 9:53 AM, Jun Rao wrote:
>> > >
>> > > > Hi, Ismael,
>> > > >
>> > > > Good point on compatibility.
>> > > >
>> > > > Hi, Mayuresh,
>> > > >
>> > > > Given that, it seems that it's better to just add the raw principal
>> as
>> > a
>> > > > new field in Session for now and deprecate the KafkaPrincipal field
>> in
>> > > the
>> > > > future if needed?
>> > > >
>> > > > Thanks,
>> > > >
>> > > > Jun
>> > > >
>> > > > On Mon, Feb 27, 2017 at 5:05 PM, Ismael Juma
>> > wrote:
>> > > >
>> > > > > Breaking clients without a deprecation period is something we
>> only do
>> > > as
>> > > > a
>> > > > > last resort. Is there strong justification for doing it here?
>> > > > >
>> > > > > Ismael
>> > > > >
>> > > > > On Mon, Feb 27, 2017 at 11:28 PM, Mayuresh Gharat <
>> > > > > gharatmayures...@gmail.com> wrote:
>> > > > >
>> > > > > > Hi Ismael,
>> > > > > >
>> > > > > > Yeah. I agree that it might break the clients if the user is
>> using
>> > > the
>> > > > > > kafkaPrincipal directly. But since KafkaPrincipal is also a Java
>> > > > > Principal
>> > > > > > and I think, it would be a right thing to do replace the
>> > > kafkaPrincipal
>> > > > > > with Java Principal at this stage than later.
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
Hi, Mayuresh,
One reason to have KafkaPrincipal in ACL is that we can extend it to
support group in the future. Have you thought about how to support that in
your new proposal?
Another reason that we had KafkaPrincipal is simplicity. It can be
constructed from a simple string and makes matching easier. If we
expose java.security.Principal,
then I guess that when an ACL is set, we have to be able to construct
a java.security.Principal
from some string to match the java.security.Principal generated from the
SSL or SASL library. How do we make sure that the same type of
java.security.Principal
can be created and will match?
Thanks,
Jun
On Wed, Mar 15, 2017 at 8:48 PM, Mayuresh Gharat wrote:
> Hi Jun,
>
> Sorry for the delayed reply.
> I agree that the easiest thing will be to add an additional field in the
> Session class and we should be OK.
> But having a KafkaPrincipal and java Principal with in the same class looks
> little weird.
>
> So we can do this and slowly deprecate the usage of KafkaPrincipal in
> public api's.
>
> We add new apis and make changes to the existing apis as follows :
>
>
>- Changes to Session class :
>
> @Deprecated
> case class Session(principal: KafkaPrincipal, clientAddress: InetAddress) {
> val sanitizedUser = QuotaId.sanitize(principal.getName)
> }
>
>
> *@Deprecated .. (NEW)*
>
>
> *case class Session(principal: KafkaPrincipal, clientAddress: InetAddress,
> channelPrincipal: Java.security.Principal) {val sanitizedUser =
> QuotaId.sanitize(principal.getName)}*
>
> *(NEW)*
>
>
> *case class Session(principal: Java.security.Principal, clientAddress:
> InetAddress) {val sanitizedUser = QuotaId.sanitize(principal.get
> Name)}*
>
>
>- Changes to Authorizer Interface :
>
> @Deprecated
> def getAcls(principal: KafkaPrincipal): Map[Resource, Set[Acl]]
>
> *(NEW)*
> *def getAcls(principal: Java.security.Principal): Map[Resource, Set[Acl]]*
>
>
>- Changes to Acl class :
>
> @Deprecated
> case class Acl(principal: KafkaPrincipal, permissionType: PermissionType,
> host: String, operation: Operation)
>
>*(NEW)*
>
>
> *case class Acl(principal: Java.security.Principal, permissionType:
> PermissionType, host: String, operation: Operation) *
> The one in Bold are the new api's. We will remove them eventually, probably
> in next major release.
> We don't want to get rid of KafkaPrincipal class and it will be used in the
> same way as it does right now for out of box authorizer and commandline
> tool. We would only be removing its direct usage from public apis.
> Doing the above deprecation will help us to support other implementation of
> Java.security.Principal as well which seems necessary especially since
> Kafka provides pluggable Authorizer and PrincipalBuilder.
>
> Let me know your thoughts on this.
>
> Thanks,
>
> Mayuresh
>
> On Tue, Feb 28, 2017 at 2:33 PM, Mayuresh Gharat <
> gharatmayures...@gmail.com
> > wrote:
>
> > Hi Jun,
> >
> > Sure.
> > I had an offline discussion with Joel on how we can deprecate the
> > KafkaPrincipal from Session and Authorizer.
> > I will update the KIP to see if we can address all the concerns here. If
> > not we can keep the KafkaPrincipal.
> >
> > Thanks,
> >
> > Mayuresh
> >
> > On Tue, Feb 28, 2017 at 1:53 PM, Jun Rao wrote:
> >
> >> Hi, Joel,
> >>
> >> Good point on the getAcls() method. KafkaPrincipal is also tied to ACL,
> >> which is used in pretty much every method in Authorizer. Now, I am not
> >> sure
> >> if it's easy to deprecate KafkaPrincipal.
> >>
> >> Hi, Mayuresh,
> >>
> >> Given the above, it seems that the easiest thing is to add a new
> Principal
> >> field in Session. We want to make it clear that it's ignored in the
> >> default
> >> implementation, but a customizer authorizer could take advantage of
> that.
> >>
> >> Thanks,
> >>
> >> Jun
> >>
> >> On Tue, Feb 28, 2017 at 10:52 AM, Joel Koshy
> wrote:
> >>
> >> > If we deprecate KafkaPrincipal, then the Authorizer interface will
> also
> >> > need to change - i.e., deprecate the getAcls(KafkaPrincipal) method.
> >> >
> >> > On Tue, Feb 28, 2017 at 10:11 AM, Mayuresh Gharat <
> >> > gharatmayures...@gmail.com> wrote:
> >> >
> >> > > Hi Jun/Ismael,
> >> > >
> >> > > Thanks for the comments.
> >> > >
> >> > > I agree.
> >> > > What I was thinking was, we get the KIP passed now and wait till
> major
> >> > > kafka version release. We can then make this change, but for now we
> >> can
> >> > > wait. Does that work?
> >> > >
> >> > > If there are concerns, we can make the addition of extra field of
> type
> >> > > Principal to Session and then deprecate the KafkaPrincipal later.
> >> > >
> >> > > I am fine either ways. What do you think?
> >> > >
> >> > > Thanks,
> >> > >
> >> > > Mayuresh
> >> > >
> >> > > On Tue, Feb 28, 2017 at 9:53 AM, Jun Rao wrote:
> >> > >
> >> > > > Hi, Ismael,
> >> > > >
> >> > > > Good point on compatibility.
> >> > > >
> >> > > > Hi, Mayuresh,
> >> > > >
> >> > > > Given that, it seems that it's better to just
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
Hi Jun,
Please find the replies inline.
One reason to have KafkaPrincipal in ACL is that we can extend it to
support group in the future. Have you thought about how to support that in
your new proposal?
---> This is a feature of PrincipalBuilder and Authorizer, which are
pluggable.
The type of Principal should be opaque to core Kafka. If we want to add
support to group, we can add that to KafkaPrincipal class and modify the
SimpleAclAuthorizer to add/modify/check the ACL accordingly.
Another reason that we had KafkaPrincipal is simplicity. It can be
constructed from a simple string and makes matching easier. If we
expose java.security.Principal,then I guess that when an ACL is set, we
have to be able to construct
a java.security.Principal from some string to match the
java.security.Principal generated from the
SSL or SASL library. How do we make sure that the same type of
java.security.Principal
can be created and will match?
> Again this will be determined by the plugged in Authorizer and
PrincipalBuilder. Your PrincipalBuilder can make sure that it creates a
Principal whose name matches the string you specified while creating the
ACL. The Authorizer should make sure that it extracts the String from the
Principal and do the matching.
In our earlier discussions, we discussed about having a PrincipalBuilder
class specifier as a command line argument for the kafka-acls.sh to handle
this case but we decided that it would be an overkill at this stage.
Thanks,
Mayuresh
On Mon, Mar 20, 2017 at 7:42 AM, Jun Rao wrote:
> Hi, Mayuresh,
>
> One reason to have KafkaPrincipal in ACL is that we can extend it to
> support group in the future. Have you thought about how to support that in
> your new proposal?
>
> Another reason that we had KafkaPrincipal is simplicity. It can be
> constructed from a simple string and makes matching easier. If we
> expose java.security.Principal,
> then I guess that when an ACL is set, we have to be able to construct
> a java.security.Principal
> from some string to match the java.security.Principal generated from the
> SSL or SASL library. How do we make sure that the same type of
> java.security.Principal
> can be created and will match?
>
> Thanks,
>
> Jun
>
>
> On Wed, Mar 15, 2017 at 8:48 PM, Mayuresh Gharat <
> gharatmayures...@gmail.com
> > wrote:
>
> > Hi Jun,
> >
> > Sorry for the delayed reply.
> > I agree that the easiest thing will be to add an additional field in the
> > Session class and we should be OK.
> > But having a KafkaPrincipal and java Principal with in the same class
> looks
> > little weird.
> >
> > So we can do this and slowly deprecate the usage of KafkaPrincipal in
> > public api's.
> >
> > We add new apis and make changes to the existing apis as follows :
> >
> >
> >- Changes to Session class :
> >
> > @Deprecated
> > case class Session(principal: KafkaPrincipal, clientAddress:
> InetAddress) {
> > val sanitizedUser = QuotaId.sanitize(principal.getName)
> > }
> >
> >
> > *@Deprecated .. (NEW)*
> >
> >
> > *case class Session(principal: KafkaPrincipal, clientAddress:
> InetAddress,
> > channelPrincipal: Java.security.Principal) {val sanitizedUser =
> > QuotaId.sanitize(principal.getName)}*
> >
> > *(NEW)*
> >
> >
> > *case class Session(principal: Java.security.Principal, clientAddress:
> > InetAddress) {val sanitizedUser = QuotaId.sanitize(principal.get
> > Name)}*
> >
> >
> >- Changes to Authorizer Interface :
> >
> > @Deprecated
> > def getAcls(principal: KafkaPrincipal): Map[Resource, Set[Acl]]
> >
> > *(NEW)*
> > *def getAcls(principal: Java.security.Principal): Map[Resource,
> Set[Acl]]*
> >
> >
> >- Changes to Acl class :
> >
> > @Deprecated
> > case class Acl(principal: KafkaPrincipal, permissionType: PermissionType,
> > host: String, operation: Operation)
> >
> >*(NEW)*
> >
> >
> > *case class Acl(principal: Java.security.Principal, permissionType:
> > PermissionType, host: String, operation: Operation) *
> > The one in Bold are the new api's. We will remove them eventually,
> probably
> > in next major release.
> > We don't want to get rid of KafkaPrincipal class and it will be used in
> the
> > same way as it does right now for out of box authorizer and commandline
> > tool. We would only be removing its direct usage from public apis.
> > Doing the above deprecation will help us to support other implementation
> of
> > Java.security.Principal as well which seems necessary especially since
> > Kafka provides pluggable Authorizer and PrincipalBuilder.
> >
> > Let me know your thoughts on this.
> >
> > Thanks,
> >
> > Mayuresh
> >
> > On Tue, Feb 28, 2017 at 2:33 PM, Mayuresh Gharat <
> > gharatmayures...@gmail.com
> > > wrote:
> >
> > > Hi Jun,
> > >
> > > Sure.
> > > I had an offline discussion with Joel on how we can deprecate the
> > > KafkaPrincipal from Session and Authorizer.
> > > I will update the KIP to see if we can address all the concerns here.
> If
> > > not we can keep the KafkaPrincip
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
on logging - i think the best we can do is call principal.toString() and
log that. the built-in one prints "type/name" but in some contexts that
might not always make sense (using the certificate addr might make more
sense if my security is cert-based)
on string representation - again this is the built-in code behaviour but
honestly i think its more of a best effort than a guarantee (again, for
certs - do you expect people to somehow feed a trust store in there?) the
earlier version of the KIP had an attempt to accomodate different principal
builders who may have other string representations, but we dont need it
internally and it caused too much debate on this thread. so as it stands
right now if you plug a different security scheme (authorizer +
authenticator) your logs may look different and setting up ACLs is on you -
but since this only affects advanced users its probably acceptable for now?
On Wed, Mar 22, 2017 at 8:07 PM, Mayuresh Gharat wrote:
> Hi Jun,
>
> Please find the replies inline.
>
> One reason to have KafkaPrincipal in ACL is that we can extend it to
> support group in the future. Have you thought about how to support that in
> your new proposal?
> ---> This is a feature of PrincipalBuilder and Authorizer, which are
> pluggable.
> The type of Principal should be opaque to core Kafka. If we want to add
> support to group, we can add that to KafkaPrincipal class and modify the
> SimpleAclAuthorizer to add/modify/check the ACL accordingly.
>
>
> Another reason that we had KafkaPrincipal is simplicity. It can be
> constructed from a simple string and makes matching easier. If we
> expose java.security.Principal,then I guess that when an ACL is set, we
> have to be able to construct
> a java.security.Principal from some string to match the
> java.security.Principal generated from the
> SSL or SASL library. How do we make sure that the same type of
> java.security.Principal
> can be created and will match?
> > Again this will be determined by the plugged in Authorizer and
> PrincipalBuilder. Your PrincipalBuilder can make sure that it creates a
> Principal whose name matches the string you specified while creating the
> ACL. The Authorizer should make sure that it extracts the String from the
> Principal and do the matching.
> In our earlier discussions, we discussed about having a PrincipalBuilder
> class specifier as a command line argument for the kafka-acls.sh to handle
> this case but we decided that it would be an overkill at this stage.
>
> Thanks,
>
> Mayuresh
>
> On Mon, Mar 20, 2017 at 7:42 AM, Jun Rao wrote:
>
> > Hi, Mayuresh,
> >
> > One reason to have KafkaPrincipal in ACL is that we can extend it to
> > support group in the future. Have you thought about how to support that
> in
> > your new proposal?
> >
> > Another reason that we had KafkaPrincipal is simplicity. It can be
> > constructed from a simple string and makes matching easier. If we
> > expose java.security.Principal,
> > then I guess that when an ACL is set, we have to be able to construct
> > a java.security.Principal
> > from some string to match the java.security.Principal generated from the
> > SSL or SASL library. How do we make sure that the same type of
> > java.security.Principal
> > can be created and will match?
> >
> > Thanks,
> >
> > Jun
> >
> >
> > On Wed, Mar 15, 2017 at 8:48 PM, Mayuresh Gharat <
> > gharatmayures...@gmail.com
> > > wrote:
> >
> > > Hi Jun,
> > >
> > > Sorry for the delayed reply.
> > > I agree that the easiest thing will be to add an additional field in
> the
> > > Session class and we should be OK.
> > > But having a KafkaPrincipal and java Principal with in the same class
> > looks
> > > little weird.
> > >
> > > So we can do this and slowly deprecate the usage of KafkaPrincipal in
> > > public api's.
> > >
> > > We add new apis and make changes to the existing apis as follows :
> > >
> > >
> > >- Changes to Session class :
> > >
> > > @Deprecated
> > > case class Session(principal: KafkaPrincipal, clientAddress:
> > InetAddress) {
> > > val sanitizedUser = QuotaId.sanitize(principal.getName)
> > > }
> > >
> > >
> > > *@Deprecated .. (NEW)*
> > >
> > >
> > > *case class Session(principal: KafkaPrincipal, clientAddress:
> > InetAddress,
> > > channelPrincipal: Java.security.Principal) {val sanitizedUser =
> > > QuotaId.sanitize(principal.getName)}*
> > >
> > > *(NEW)*
> > >
> > >
> > > *case class Session(principal: Java.security.Principal, clientAddress:
> > > InetAddress) {val sanitizedUser = QuotaId.sanitize(principal.get
> > > Name)}*
> > >
> > >
> > >- Changes to Authorizer Interface :
> > >
> > > @Deprecated
> > > def getAcls(principal: KafkaPrincipal): Map[Resource, Set[Acl]]
> > >
> > > *(NEW)*
> > > *def getAcls(principal: Java.security.Principal): Map[Resource,
> > Set[Acl]]*
> > >
> > >
> > >- Changes to Acl class :
> > >
> > > @Deprecated
> > > case class Acl(principal: KafkaPrincipal, permissionType:
> PermissionType,
> > > host: String,
Re: [VOTE] KIP-111 Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.
Hi, Mayuresh,
Please find my replies inlined below.
On Wed, Mar 22, 2017 at 8:07 PM, Mayuresh Gharat wrote:
> Hi Jun,
>
> Please find the replies inline.
>
> One reason to have KafkaPrincipal in ACL is that we can extend it to
> support group in the future. Have you thought about how to support that in
> your new proposal?
> ---> This is a feature of PrincipalBuilder and Authorizer, which are
> pluggable.
> The type of Principal should be opaque to core Kafka. If we want to add
> support to group, we can add that to KafkaPrincipal class and modify the
> SimpleAclAuthorizer to add/modify/check the ACL accordingly.
>
>
If we do want the extension to support groups in the future, wouldn't it be
better to make KafkaPrincipal as first class citizen as it is now to start
with?
>
> Another reason that we had KafkaPrincipal is simplicity. It can be
> constructed from a simple string and makes matching easier. If we
> expose java.security.Principal,then I guess that when an ACL is set, we
> have to be able to construct
> a java.security.Principal from some string to match the
> java.security.Principal generated from the
> SSL or SASL library. How do we make sure that the same type of
> java.security.Principal
> can be created and will match?
> > Again this will be determined by the plugged in Authorizer and
> PrincipalBuilder. Your PrincipalBuilder can make sure that it creates a
> Principal whose name matches the string you specified while creating the
> ACL. The Authorizer should make sure that it extracts the String from the
> Principal and do the matching.
> In our earlier discussions, we discussed about having a PrincipalBuilder
> class specifier as a command line argument for the kafka-acls.sh to handle
> this case but we decided that it would be an overkill at this stage.
>
>
If you can create a java Principal from a string for matching, then you
could also just use KafkaPrincipal, right? In the earlier discussion, I
didn't get the exact use case of being able to create arbitrary java
principal from a string name plus some configuration params.
Thanks,
Jun
> Thanks,
>
> Mayuresh
>
> On Mon, Mar 20, 2017 at 7:42 AM, Jun Rao wrote:
>
> > Hi, Mayuresh,
> >
> > One reason to have KafkaPrincipal in ACL is that we can extend it to
> > support group in the future. Have you thought about how to support that
> in
> > your new proposal?
> >
> > Another reason that we had KafkaPrincipal is simplicity. It can be
> > constructed from a simple string and makes matching easier. If we
> > expose java.security.Principal,
> > then I guess that when an ACL is set, we have to be able to construct
> > a java.security.Principal
> > from some string to match the java.security.Principal generated from the
> > SSL or SASL library. How do we make sure that the same type of
> > java.security.Principal
> > can be created and will match?
> >
> > Thanks,
> >
> > Jun
> >
> >
> > On Wed, Mar 15, 2017 at 8:48 PM, Mayuresh Gharat <
> > gharatmayures...@gmail.com
> > > wrote:
> >
> > > Hi Jun,
> > >
> > > Sorry for the delayed reply.
> > > I agree that the easiest thing will be to add an additional field in
> the
> > > Session class and we should be OK.
> > > But having a KafkaPrincipal and java Principal with in the same class
> > looks
> > > little weird.
> > >
> > > So we can do this and slowly deprecate the usage of KafkaPrincipal in
> > > public api's.
> > >
> > > We add new apis and make changes to the existing apis as follows :
> > >
> > >
> > >- Changes to Session class :
> > >
> > > @Deprecated
> > > case class Session(principal: KafkaPrincipal, clientAddress:
> > InetAddress) {
> > > val sanitizedUser = QuotaId.sanitize(principal.getName)
> > > }
> > >
> > >
> > > *@Deprecated .. (NEW)*
> > >
> > >
> > > *case class Session(principal: KafkaPrincipal, clientAddress:
> > InetAddress,
> > > channelPrincipal: Java.security.Principal) {val sanitizedUser =
> > > QuotaId.sanitize(principal.getName)}*
> > >
> > > *(NEW)*
> > >
> > >
> > > *case class Session(principal: Java.security.Principal, clientAddress:
> > > InetAddress) {val sanitizedUser = QuotaId.sanitize(principal.get
> > > Name)}*
> > >
> > >
> > >- Changes to Authorizer Interface :
> > >
> > > @Deprecated
> > > def getAcls(principal: KafkaPrincipal): Map[Resource, Set[Acl]]
> > >
> > > *(NEW)*
> > > *def getAcls(principal: Java.security.Principal): Map[Resource,
> > Set[Acl]]*
> > >
> > >
> > >- Changes to Acl class :
> > >
> > > @Deprecated
> > > case class Acl(principal: KafkaPrincipal, permissionType:
> PermissionType,
> > > host: String, operation: Operation)
> > >
> > >*(NEW)*
> > >
> > >
> > > *case class Acl(principal: Java.security.Principal, permissionType:
> > > PermissionType, host: String, operation: Operation) *
> > > The one in Bold are the new api's. We will remove them eventually,
> > probably
> > > in next major release.
> > > We don't want to get rid of KafkaPrincipal class and it will be used in
> > the
>
