Skip to content

Conversation

@sberyozkin
Copy link
Member

Fixes #46972.

The current OIDC opaque token check has been proved with #46990 to be incomplete.
Signed JWT tokens have 3 parts separated by 2 dots which is what the current light weight check does, but it can be just a concidence, the binary tokens may have random 2 dots in the sequence as well.
The side-effect highlighted by #46972 is that a binary refresh token with 2 dots is assumed to be JWT, and the parsing exception escapes causing a failure.
While #46972 can be fixed by only adding another exception catch block as done in this PR, I've also updated the opaque token check which is done in other code branches - it did not cause side-effects so far but might if not fixed

@JozefDropco
Copy link

@quarkus-bot
Copy link

quarkus-bot bot commented Mar 25, 2025

Status for workflow Quarkus CI

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

✅ 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.

@gsmet
Copy link
Member

gsmet commented Mar 25, 2025

Please also add the triage/backport label or this PR won't be included in 3.21 (and then won't be included in 3.20 as it won't have any bake time). Not doing it myself silently so that you get the memo :).

@sberyozkin
Copy link
Member Author

sberyozkin commented Mar 25, 2025

Hi @JozefDropco

is it safe to modify the isJwtTokenExpired as its used even here https://github.com/sberyozkin/quarkus/blob/6a746d33e18bf52b0e330fb1eacffae2074e022f/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java#L388

isJwtTokenExpired has been updated in this PR to avoid a duplicate attempt to read claims since the isOpaqueToken check is also trying to read claims now to correctly determine if the token is opaque or not

@sberyozkin sberyozkin merged commit 439e910 into quarkusio:main Mar 25, 2025
27 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.22 - main milestone Mar 25, 2025
@sberyozkin sberyozkin deleted the azure_opaque_refresh_token branch March 25, 2025 16:07
@gsmet gsmet modified the milestones: 3.22 - main, 3.21.1 Apr 1, 2025
@jmartisk jmartisk modified the milestones: 3.21.1, 3.20.1 Apr 16, 2025
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.

OIDC -Azure refresh token doesnt have expiration date

5 participants