Skip to content

Conversation

Sopka
Copy link
Contributor

@Sopka Sopka commented Jul 11, 2025

The issue is in OidcClientImpl.java, where add() is used instead of set() for JWT bearer authentication parameters. Other authentication methods correctly use set().

This causes a mismatch between the expected behavior (replacing previous values) and the actual behavior (accumulating values).

Replaced the add() method calls with set() in the JWT bearer authentication code path:

// Replace these lines:
body.add(OidcConstants.CLIENT_ASSERTION, clientAssertion);
body.add(OidcConstants.CLIENT_ASSERTION_TYPE, OidcConstants.JWT_BEARER_CLIENT_ASSERTION_TYPE);

// With:
body.set(OidcConstants.CLIENT_ASSERTION, clientAssertion);
body.set(OidcConstants.CLIENT_ASSERTION_TYPE, OidcConstants.JWT_BEARER_CLIENT_ASSERTION_TYPE);

This would ensure only the newest key-value pairs are included in the request, eliminating the duplication problem.

@geoand
Copy link
Contributor

geoand commented Jul 11, 2025

Thanks a lot for the contribution!

Is there any chance we could have a test for this?

This comment has been minimized.

@sberyozkin
Copy link
Member

sberyozkin commented Jul 11, 2025

Thanks @Sopka, @geoand. Indeed, would be good to have a test.

The test which sends a single request is here.
The wiremock stub is here.

I guess the simplest option is to do 2 calls in that test and tune the stub definition to fail if more than one client_assertion parameter is used. Or may be you can just update https://github.com/quarkusio/quarkus/blob/main/integration-tests/oidc-client-wiremock/src/main/java/io/quarkus/it/keycloak/OidcRequestCustomizer.java to check if the body contains only one of those parameters.

Have a look please, I can help if you'd like

Sopka added 2 commits July 11, 2025 12:42
Ensure that all subsequent requests to the OIDC server's token endpoint include the correct number of form parameters in the POST data.
@Sopka Sopka force-pushed the fix-oidc-client-parameter-accumulation branch from e1cbcfa to dce6637 Compare July 11, 2025 13:58
@Sopka
Copy link
Contributor Author

Sopka commented Jul 11, 2025

Added a test that makes multiple requests to the OIDC token endpoint using a filter configuration that always forces new token requests. This approach avoids issues with simulating token expiration and eliminates the need to wait for tokens to expire. The test verifies that each request sends the expected form parameters.

Copy link

quarkus-bot bot commented Jul 11, 2025

Status for workflow Quarkus CI

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

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

Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Sopka.

I think we can probably keep both commits, without squashing, as each of them is quite self-contained.
@geoand I'll merge soon if there are no objections, I can easily squash if preferred

@geoand
Copy link
Contributor

geoand commented Jul 11, 2025

Go ahead and merge as is

@sberyozkin sberyozkin merged commit 29eded4 into quarkusio:main Jul 11, 2025
26 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.25 - main milestone Jul 11, 2025
@gsmet gsmet modified the milestones: 3.25.0.CR1, 3.24.4 Jul 16, 2025
@jmartisk jmartisk modified the milestones: 3.24.4, 3.20.3 Aug 13, 2025
@jmartisk jmartisk moved this to 3.20.3 in Backports for 3.20 Aug 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: 3.20.3

Development

Successfully merging this pull request may close these issues.

JWT Bearer Auth Form Parameters Accumulate with Each Request in OidcClientImpl

5 participants