We're using rack-oauth2 as the basis for our Oauth and OIDC server. Recently we were prototyping an identity bridge project using Keycloak. It was configured to use private_key_jwt client authentication and would encounter a 400 during the code exchange for the identity and access tokens. Through some debugging I determined that Keycloak does not send the client_id
param when using private_key_jwt client authentication, but rack-oauth2 requires it. I thought this was a bug in Keycloak but then I reviewed the RFCs and discovered that it is in fact optional to send the client_id
param. Below I provide links to the RFCs, and potential solutions to this problem in rack-oauth2.
Per https://www.rfc-editor.org/rfc/rfc7523.html#section-2.2, a token request can be of grant type
, authorization_code
, and still use JWT bearer for authentication by specifying the client_assertion_type
and client_assertion
params. Moreover, per https://www.rfc-editor.org/rfc/rfc7521.html#section-4.2, which is the base RFC that RFC7523 extends, the client_id is optional in JWT bearer token requests.
However, the server token code is written in a way that will not allow this scenario to occur. This is because it uses the grant_type_for
method to determine which grant type class to use, and the JWT bearer grant type is only used for authorization requests, not token requests. Moreover, the AuthorizationCode grant type class inherits from TokenRequest class, which inherits from AbstractRequest, and AbstractRequest requires the client_id
param. Neither the TokenRequest or the AuthorizationCodeRequest classes override the requirement, whereas the JWTBearer class does. The same is true for the ClientCredentials token request class. Consequently, client_id is required even though the spec states that it is optional for client authentication when using a client assertion token.
I've been trying to think of how to fix the code to accommodate this behavior, but because attr_required/attr_optional are evaluated during the class load, I'm not sure what the best way to move forward is.
My first thought would be when the token request class is initialized, the params are checked for the client_assertion
param, and if present, the client_id
param would be optional. But again, the macros are evaluated at class load, so that's not really possible unless we modify attr_required to take a proc in order to evaluate an if/unless condition at the instance level.
So my next thought is to make the client_id
param optional, and then append client_id
to the required_attributes array during initialization if the client_assertion
param is missing. However, this option feels very much like a hack.
The last option that I can think of is to just make the client_id
param always optional. On the one hand this option actually seems best, since the best current security practice is to use private_key_jwt auth instead of a shared secret. However, on the other hand, I know that using a shared secret is still very common practice, and this library tries to build in the behavior that the protocol (in all its forms) requires.
I'm happy to address this issue but I'm not sure what the best way forward is. I welcome any guidance.