Thanks Rajini, that helps. A few comments:

1. The `AuthCallbackHandler` interface already exists and we are making
breaking changes (removing a parameter from `configure` and adding
additional methods). Is the reasoning that it was not a public interface
before? It would be good to clearly separate public versus non-public
interfaces in the security code (and we should tweak Gradle to publish
javadoc for the public ones).

2. It seems like there is an ordering when it comes to the invocation of
callbacks. At least the current code assumes that `NameCallback` is called
first. If I am interpreting this correctly, we should specify that ordering.

3. The approach taken by `ScramCredentialCallback` is different than the
one taken by `PlainAuthenticateCallback`. The former lets the user pass the
credentials information while the latter passes the credentials and lets
the user do the authentication. It would be good to explain the
inconsistency.

4. We reference `ScramCredential` in a few places, so it would be good to
define that class too.

Ismael

On Wed, Dec 14, 2016 at 7:32 AM, Rajini Sivaram <
rajinisiva...@googlemail.com> wrote:

> Have added sample callback handlers for PLAIN and SCRAM.
>
> On Tue, Dec 13, 2016 at 4:10 PM, Rajini Sivaram <
> rajinisiva...@googlemail.com> wrote:
>
> > Ismael,
> >
> > Thank you for the review. I will add an example.
> >
> > On Tue, Dec 13, 2016 at 1:07 PM, Ismael Juma <ism...@juma.me.uk> wrote:
> >
> >> Hi Rajini,
> >>
> >> Thanks for the KIP. I think this is useful and users have asked for
> >> something like that. I like that you have a scenarios section, do you
> >> think
> >> you could provide a rough sketch of what a callback handler would look
> >> like
> >> for the first 2 scenarios? They seem to be the common ones, so it would
> >> help to see a concrete example.
> >>
> >> Ismael
> >>
> >> On Tue, Oct 11, 2016 at 7:28 AM, Rajini Sivaram <
> >> rajinisiva...@googlemail.com> wrote:
> >>
> >> > Hi all,
> >> >
> >> > I have just created KIP-86 make callback handlers in SASL configurable
> >> so
> >> > that credential providers for SASL/PLAIN (and SASL/SCRAM when it is
> >> > implemented) can be used with custom credential callbacks:
> >> >
> >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >> > 86%3A+Configurable+SASL+callback+handlers
> >> >
> >> > Comments and suggestions are welcome.
> >> >
> >> > Thank you...
> >> >
> >> >
> >> > Regards,
> >> >
> >> > Rajini
> >> >
> >>
> >
> >
> >
> > --
> > Regards,
> >
> > Rajini
> >
>
>
>
> --
> Regards,
>
> Rajini
>

Reply via email to