Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keyclock-js client library expects refresh tokens being a JWT #210

Closed
jp7677 opened this issue Mar 9, 2022 · 2 comments · Fixed by #242
Closed

Keyclock-js client library expects refresh tokens being a JWT #210

jp7677 opened this issue Mar 9, 2022 · 2 comments · Fixed by #242
Labels
enhancement New feature or request

Comments

@jp7677
Copy link
Contributor

jp7677 commented Mar 9, 2022

This is a follow up from #199 (comment)

The keycloak-js client library (https://github.com/keycloak/keycloak/tree/main/adapters/oidc/js) needs some changes in the refresh token. it expects a kind-of proper token where it wants to decode the second part of a string separated by a . and wants to check the nonce in it. My evil hack, which does work, for generating the refresh token looks like the following, whereas the nonce is passed down from AuthorizationCodeHandler.tokenResponse.

val jti = UUID.randomUUID().toString()
val refreshToken = if (nonce != null) "." + Base64.getEncoder().encodeToString("{\"nonce\":\"$nonce\",\"jti\":\"$jti\"}".toByteArray()) else jti

Response from @tommytroen :

That sounds really strange - the refresh token is supposed to be an opaque string containing whatever. Do you mean that keycloak requires the refresh token to be a JWT? Maybe I havent understood the use case right - could you elaborate a bit more on what you send, what you receive and what errors keycloak-js is reporting?

Response from @jp7677 :

The refresh token being an opaque string is also my understanding, but seems that unfortunately keycloak enhanced the specs for themself. See here https://github.com/keycloak/keycloak/blob/main/adapters/oidc/js/src/keycloak.js#L952 where it wants to decode the refresh token like an id-token and here https://github.com/keycloak/keycloak/blob/main/adapters/oidc/js/src/keycloak.js#L765 where it validates the nonce element in the refresh token.

Any ideas how to handle this case? Being compatible with keycloak-js would obviously be nice, though I would understand the hesitation considering the "re-interpretation" of the specifications by Keycloak server and client.

@ybelMekk ybelMekk added the enhancement New feature or request label Apr 5, 2022
@jp7677
Copy link
Contributor Author

jp7677 commented Apr 12, 2022

Any thoughts on how to handle the keycloak specs interpretation? I guess it come down to these options:

  • Just ignore this requirement since according to specs the refresh token is supposed to be just opaque string, which in turn make this project unfortunately incompatible with the keycloak client library
  • Take over my "hacky" solution which just implements the bare minimum to reach compatibility
  • Make the refresh token a proper JWT

@tommytroen
Copy link
Collaborator

@jp7677 sorry about the late response! I'm thinking we should implement this so that keycloak client can be used, it is a minor change after all and shouldn't affect other clients. Haven't decided if we should go the "hacky" route or go with a proper signed JWT - it does seem that the keykloak client only decodes and not validates the token so that leaves all possibilities open :)

tommytroen added a commit that referenced this issue Apr 22, 2022
* includes nonce from auth request in a plain JWT
* see #210
tommytroen added a commit that referenced this issue Apr 27, 2022
* feat: support keycloak refresh token format
* includes nonce from auth request in a plain JWT
* see #210

Co-authored-by: Youssef Bel Mekki <38552193+ybelMekk@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants