+1 (non-binding)

On Thu, Feb 23, 2017 at 3:27 AM, Mayuresh Gharat <gharatmayures...@gmail.com
> wrote:

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

Reply via email to