Skip to content

Conversation

@sberyozkin
Copy link
Member

@sberyozkin sberyozkin commented Nov 7, 2019

Fixes #5301.
Related to #5002.

This PR:

  • Removes two RefreshToken classes, and makes RT only reachable via AccessTokenCredential. IMHO this better represents the purpose of RT, and lets us delay providing something directly for the user code to use to refresh the access tokens which should ideally be done only by the adapter (Incorrect extension metadata for Jaeger #5002), or via an injected OidcClient (yet to be considered)
  • Adds the raw token to JsonWebToken
  • Moves IdTokenCredential and AccessTokenCredential out of runtime sub-package
  • Updates the test

Quickstart will be updated next. Only the way Refresh tokens are accessed will be updated which will be fine as there is no any support yet for those RTs anyway. RTs are not referenced in the guide yet, so updating the way they are accessed should be just fine and in time

@sberyozkin sberyozkin added this to the 1.1.0 milestone Nov 7, 2019
@sberyozkin
Copy link
Member Author

sberyozkin commented Nov 7, 2019

@pedroigor thanks; I've been thinking of the alternative, keep one RefreshToken but rename it to RefreshTokenCredential to have the naming consistent, and still avoid injecting it directly but only make visible through AccessTokenCredential but I do not feel like we'd make much progress in this case as far as the simplification is concerned :-), so just making it available as String should be enough, probably it would still be of interest to the adapter only for the preemptive refreshment...
Thanks

@stuartwdouglas stuartwdouglas merged commit 5f07d6e into quarkusio:master Nov 8, 2019
@cescoffier
Copy link
Member

@sberyozkin This PR is a breaking change.
We cannot delete classes that have been exposed in APIs.

@gsmet
Copy link
Member

gsmet commented Nov 14, 2019

I won't backport this one as it is a breaking change.

@gsmet
Copy link
Member

gsmet commented Nov 14, 2019

Apparently, people want it anyway.

@sberyozkin if users are required to change their applications, I need a blurb explaining how.

@gsmet gsmet removed the backport? label Nov 14, 2019
@gsmet gsmet modified the milestones: 1.1.0, 1.0.0.Final Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JsonWebToken raw_token claim is not set in the OIDC flows

5 participants