Hi, Kirk,
Thanks for the reply.
JR1. If we have no clear usage, it's probably better to omit it. We could
always add a new param in the future if needed, right?
JR2. Could we document it? Also, when is FileJwtRetriever chosen?
Jun
On Tue, Apr 15, 2025 at 1:04 PM Kirk True <k...@kirktrue.pro> wrote:
> Hi Jun!
>
> I really appreciate your feedback.
>
> Responses below...
>
> On Fri, Apr 11, 2025, at 1:47 PM, Jun Rao wrote:
> > Hi, Kirk,
> >
> > Thanks for the KIP. A few comments below.
> >
> > JR1. OAuthBearerConfigurable.configure(): Could you describe the possible
> > values for saslMechanism? Is it always OAUTHBEARER?
>
> In this case, yes, it's always OAUTHBEARER. I wanted to keep the same
> signature in case there was a reason in the future that something else
> would be used. Maybe I'm trying to make it _too_ future-proof?
>
> > JR2. If sasl.oauthbearer.jwt.retriever.class is not configured, what
> other
> > configs lead to the selection of JwtBearerJwtRetriever?
>
> If sasl.oauthbearer.jwt.retriever.class is not configured, the default
> implementation (DefaultJwtRetriever) will use the
> sasl.oauthbearer.grant.type configuration to determine the implementation.
>
> If the grant type is "client_credentials" (the default),
> ClientCredentialsJwtRetriever will be used. If the grant type value is
> "urn:ietf:params:oauth:grant-type:jwt-bearer", then JwtBearerJwtRetriever
> is used.
>
> > JR3. If sasl.oauthbearer.jwt.validator.class is not configured, how do we
> > choose which built-in class to use?
>
> If sasl.oauthbearer.jwt.validator.class is not configured, the default
> implementation (DefaultJwtValidator) will use the presence of the existing
> sasl.oauthbearer.jwks.endpoint.url configuration to make the determination.
> If there is a value for sasl.oauthbearer.jwks.endpoint.url,
> BrokerJwtValidator will be used, otherwise ClientJwtValidator is used.
>
> I will make this more clear in the KIP.
>
> > JR4. Could we document what type can be returned for Object in the
> > following?
> > public interface AssertionJwtTemplate extends OAuthBearerConfigurable {
> >
> > /**
> > * Returns a map containing zero or more header values.
> > *
> > * @returns Values to include in the JWT header
> > */
> > Map<String, Object> header();
> >
> > /**
> > * Returns a map containing zero or more JWT payload claim values.
> > *
> > * @returns Values to include in the JWT payload
> > */
> > Map<String, Object> payload();
>
> JSON/JWT supports arbitrary data structures, so it's difficult to give a
> succinct list, but I will expound on this in the KIP.
>
> > JR5. "alg": RS256 : Should RS256 be quoted?
>
> Yes. Thanks :)
>
> > JR6. "Step 2 is to create the JWT payload from the claims provided to
> > {@link #create(Map)}": The parameter for create() is not a Map.
>
> Thanks for pointing that out. They used to be maps, but now they're a
> little more involved.
>
> > JR7. For the new configuration table, could we document the type of each
> > new config?
>
> Good idea. I'll update the KIP.
>
> > JR8. Could we describe the format of sasl.oauthbearer.assertion.file?
>
> It's a serialized JWT, but I'll elaborate on that in the KIP for clarity.
>
> > JR9. sasl.oauthbearer.assertion.private.key.passphrase: In the "Mutually
> > exclusive configuration" part, it should say "If the
> > sasl.oauthbearer.assertion.file is used, this will be ignored.", right?
>
> Yes, we could ignore (or log) incompatible configuration instead of
> throwing an error. I'll double check the configuration to make sure the
> mutually exclusive ones are explained correctly.
>
> > JR10. sasl.oauthbearer.assertion.expiration.minutes and
> > sasl.oauthbearer.assertion.not.before.minutes: Why are they in minutes
> > instead of seconds?
>
> Simply because this design was based on a previous implementation that
> used minutes vs. seconds. I did find minute units odd, but there seemed to
> be precedent for it in offsets.retention.minutes and log.retention.minutes.
>
> JWT expiration being expressed in minutes is fairly typical from what I've
> seen in OAuth applications. However, using seconds (or even milliseconds)
> fits with Kafka convention better and may allow users sub-minute
> expiration, if desired.
>
> > JR11. "Two of the JAAS options (clientId and clientSecret) for the
> existing
> > client_credentials grant type are deprecated." Should we
> > include sasl.oauthbearer.scope too?
>
> Good catch. "scope" in the sasl.jaas.config configuration is deprecated in
> favor of the sasl.oauthbearer.scope configuration. I'll update the KIP.
>
> Thanks,
> Kirk
>
> > Thanks,
> >
> > Jun
> >
> > On Fri, Mar 14, 2025 at 11:52 AM Kirk True <k...@kirktrue.pro> wrote:
> >
> > > Hi all,
> > >
> > > I would like to start a discussion for KIP-1139: Add support for OAuth
> > > jwt-bearer grant type:
> > >
> > > https://cwiki.apache.org/confluence/x/uIxEF
> > >
> > > The proposal is twofold:
> > >
> > > * Add support for the OAuth 2.0 JWT Bearer grant type to avoid use of
> > > plaintext client secrets
> > > * Promote internal APIs for public use by this and future OAuth work
> > >
> > > Thanks!
> > > Kirk
> >
>
