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

Avoid an OIDC refresh token call if the JWT refresh token has expired #43081

Merged

Conversation

sberyozkin
Copy link
Member

@sberyozkin sberyozkin commented Sep 6, 2024

Fixes #41830.

This PR is really about an optimization as opposed to a bug fix. If Quarkus OIDC can figure out that the refresh token has expired, then there is no point to attempt to use it and make another call to refresh the tokens, as it will fail anyway.

So the PR checks if the refresh token is in JWT format as in the case of #41830, and if yes, it checks the expiry exp claim, which is a number of seconds from the epoch, and if it is less than the current time, it has expired, and no another refresh call is made.
I've done a minor code refactoring, where the refresh token is checked, just to avoid some duplication, but the only new change which is made here is this refresh token expiry check.

Added a test to confirm it is effective.

This PR will most likely benefit Keycloak users only as it is the only provider I know of which allocates refresh tokens in the JWT format. Most other providers issue binary refresh tokens. In the future, we might be able to support checking the expiry of binary refresh tokens too if their expiry time is returned as a JSON property...

Copy link

quarkus-bot bot commented Sep 6, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit d6f491f.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@sberyozkin
Copy link
Member Author

Let me merge, thanks @gastaldi. And as always, @pedroigor, ping me any time please should you have any questions, thanks

@sberyozkin sberyozkin merged commit 787ee38 into quarkusio:main Sep 6, 2024
23 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.16 - main milestone Sep 6, 2024
@sberyozkin sberyozkin deleted the oidc_check_expired_refresh_token branch September 6, 2024 14:35
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.

quarkus-oidc does not check expiry timestamp of refresh token, resulting in failed refresh call.
2 participants