+1 (non-binding) Thanks for the detailed KIP.
On Wed, May 16, 2018 at 5:59 PM, Mickael Maison <mickael.mai...@gmail.com> wrote: > Thanks for the KIP, > +1 (non binding) > > On Wed, May 16, 2018 at 2:51 AM, Ron Dagostino <rndg...@gmail.com> wrote: > > Hi Jun. I think you are getting at the fact that OAuth 2 is a flexible > > framework that allows different installations to do things differently. > It > > is true that the principal name in Kafka could come from any claim in the > > token. Most of the time it would come from the 'sub' claim, but it could > > certainly come from another claim, or it could be only indirectly based > on > > a claim value (maybe certain text would be trimmed or prefixed/suffixed). > > The point, which I think you are getting at, is that because the > framework > > is flexible we need to accommodate that flexibility. The callback > handler > > class defined by the listener.name.sasl_ssl.oauthbearer.sasl.server. > > callback.handler.class configuration value gives us the required > > flexibility. As an example, I have an implementation that leverages a > > popular open source JOSE library to parse the compact serialization, > > retrieve the public key if it has not yet been retrieved, verify the > > digital signature, and map the 'sub' claim to the OAuthBearerToken's > > principal name (which becomes the SASL authorization ID, which becomes > the > > Kafka principal name). I could just as easily have mapped a different > > claim to the OAuthBearerToken's principal name, manipulated the 'sub' > claim > > value in some way, etc. I write the callback handler code, so I complete > > flexibility to do whatever my OAuth 2 installation requires me to do. > > > > Ron > > > > On Tue, May 15, 2018 at 1:39 PM, Jun Rao <j...@confluent.io> wrote: > > > >> Hi, Ron, > >> > >> Thanks for the reply. I understood your answers to #2 and #3. > >> > >> For #1, will the server map all clients' principal name to the value > >> associated with "sub" claim? How do we support mapping different > clients to > >> different principal names? > >> > >> Jun > >> > >> On Mon, May 14, 2018 at 7:02 PM, Ron Dagostino <rndg...@gmail.com> > wrote: > >> > >> > Hi Jun. Thanks for the +1 vote. > >> > > >> > Regarding the first question about token claims, yes, you have it > correct > >> > about translating the OAuth token to a principle name via a JAAS > module > >> > option in the default unsecured case. Specifically, the OAuth SASL > >> Server > >> > implementation is responsible for setting the authorization ID, and it > >> gets > >> > the authorization ID from the OAuthBearerToken's principalName() > method. > >> > The listener.name.sasl_ssl.oauthbearer.sasl.server. > >> callback.handler.class > >> > is responsible for handling an instance of > OAuthBearerValidatorCallback > >> to > >> > accept a token compact serialization from the client and return an > >> instance > >> > of OAuthBearerToken (assuming the compact serialization validates), > and > >> in > >> > the default unsecured case the builtin unsecured validator callback > >> handler > >> > defines the OAuthBearerToken.principalName() method to return the > 'sub' > >> > claim value by default (with the actual claim it uses being > configurable > >> > via the unsecuredValidatorPrincipalClaimName JAAS module option). So > >> that > >> > is how we translate from a token to a principal name in the default > >> > unsecured (out-of-the-box) case. > >> > > >> > For production use cases, the implementation associated with > >> > listener.name.sasl_ssl.oauthbearer.sasl.server.callback.handler.class > >> can > >> > do whatever it wants. As an example, I have written a class that > wraps a > >> > com.nimbusds.jwt.SignedJWT instance (see > >> > https://connect2id.com/products/nimbus-jose-jwt/) and presents it as > an > >> > OAuthBearerToken: > >> > > >> > public class NimbusSignedJwtOAuthBearerToken implements > >> OAuthBearerToken { > >> > private final SignedJWT signedJwt; > >> > private final String principalName; > >> > private final Set<String> scope; > >> > private final Long startTimeMs; > >> > private final long lifetimeMs; > >> > > >> > /** > >> > * Constructor > >> > * > >> > * @param signedJwt > >> > * the mandatory signed JWT > >> > * @param principalClaimName > >> > * the mandatory claim name identifying the claim from > >> which > >> > the > >> > * principal name will be extracted. The claim must > exist > >> > and must be > >> > * a String. > >> > * @param optionalScopeClaimName > >> > * the optional claim name identifying the claim from > >> which > >> > any scope > >> > * will be extracted. If specified and the claim exists > >> then > >> > the > >> > * value must be either a String or a String List. > >> > * @throws ParseException > >> > * if the principal claim does not exist or is not a > >> > String; the > >> > * scope claim is neither a String nor a String List; > the > >> > 'exp' > >> > * claim does not exist or is not a number; the 'iat' > >> claim > >> > exists > >> > * but is not a number; or the 'nbf' claim exists and > is > >> > not a > >> > * number. > >> > */ > >> > public NimbusSignedJwtOAuthBearerToken(SignedJWT signedJwt, > String > >> > principalClaimName, > >> > String optionalScopeClaimName) throws ParseException { > >> > // etc... > >> > } > >> > > >> > The callback handler runs the following code if the digital signature > >> > validates: > >> > > >> > callback.token(new NimbusSignedJwtOAuthBearerToken(signedJwt, > "sub", > >> > null)); > >> > > >> > I hope that answers the first question. If not let me know what I > >> > missed/misunderstood and I'll be glad to try to address it. > >> > > >> > Regarding the second question, the classes OAuthBearerTokenCallback > and > >> > OAuthBearerValidatorCallback implement the Callback interface -- they > are > >> > the callbacks that the AuthenticateCallbackHandler implementations > need > >> to > >> > handle. Specifically, unless the unsecured functionality is what is > >> > desired, the two configuration values [listener.name.sasl_ssl. > >> oauthbearer. > >> > ]sasl.login.callback.handler.class and > >> > listener.name.sasl_ssl.oauthbearer.sasl.server.callback.handler.class > >> > define the callback handlers that need to handle > OAuthBearerTokenCallback > >> > and OAuthBearerValidatorCallback, respectively. > >> > > >> > Regarding the third question, yes, I see your point that the way the > spec > >> > is worded could be taken to imply that the error code is a single > >> > character: "A single ASCII..." ( > >> > https://tools.ietf.org/html/rfc6749#section-5.2). However, it is > not a > >> > single character. See the end of that section 5.2 for an example that > >> > shows "error":"invalid_request" as the response. > >> > > >> > Thanks again for the +1 vote, Jun, and please do let me know if I can > >> cover > >> > anything else. > >> > > >> > Ron > >> > > >> > > >> > On Mon, May 14, 2018 at 7:10 PM, Jun Rao <j...@confluent.io> wrote: > >> > > >> > > Hi, Ron, > >> > > > >> > > Thanks for the KIP. +1 from me. Just a few minor comments below. > >> > > > >> > > 1. It seems that we can translate an OAuth token to a principle name > >> > > through the claim name configured in JASS. However, it's not clear > to > >> me > >> > > how an OAuth token is mapped to a claim. Could you clarify that? > >> > > > >> > > 2. The wiki has the following code. It seems that > >> > OAuthBearerTokenCallback > >> > > should implement AuthenticateCallbackHandler? Ditto > >> > > for OAuthBearerValidatorCallback. > >> > > > >> > > public class OAuthBearerTokenCallback implements Callback > >> > > > >> > > 3. In OAuthBearerTokenCallback, we have the following method. The > OAuth > >> > > spec says the error code is a single ASCII. So, should we return a > Char > >> > or > >> > > a String? > >> > > > >> > > public String errorCode() > >> > > > >> > > Jun > >> > > > >> > > > >> > > On Thu, May 3, 2018 at 8:55 PM, Ron Dagostino <rndg...@gmail.com> > >> wrote: > >> > > > >> > > > Hi everyone. I would like to start the vote for KIP-255: > >> > > > https://cwiki.apache.org/confluence/pages/viewpage. > >> > > action?pageId=75968876 > >> > > > > >> > > > This KIP proposes to add the following functionality related to > >> > > > SASL/OAUTHBEARER: > >> > > > > >> > > > 1) Allow clients (both brokers when SASL/OAUTHBEARER is the > >> > inter-broker > >> > > > protocol as well as non-broker clients) to flexibly retrieve an > >> access > >> > > > token from an OAuth 2 authorization server based on the > declaration > >> of > >> > a > >> > > > custom login CallbackHandler implementation and have that access > >> token > >> > > > transparently and automatically transmitted to a broker for > >> > > authentication. > >> > > > > >> > > > 2) Allow brokers to flexibly validate provided access tokens when > a > >> > > client > >> > > > establishes a connection based on the declaration of a custom SASL > >> > Server > >> > > > CallbackHandler implementation. > >> > > > > >> > > > 3) Provide implementations of the above retrieval and validation > >> > features > >> > > > based on an unsecured JSON Web Token that function out-of-the-box > >> with > >> > > > minimal configuration required (i.e. implementations of the two > types > >> > of > >> > > > callback handlers mentioned above will be used by default with no > >> need > >> > to > >> > > > explicitly declare them). > >> > > > > >> > > > 4) Allow clients (both brokers when SASL/OAUTHBEARER is the > >> > inter-broker > >> > > > protocol as well as non-broker clients) to transparently retrieve > a > >> new > >> > > > access token in the background before the existing access token > >> expires > >> > > in > >> > > > case the client has to open new connections. > >> > > > > >> > > > Thanks, > >> > > > > >> > > > Ron > >> > > > > >> > > > >> > > >> >
