Add token audience param to oidc-client #16215
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@mkowa42 Hi, thanks, I think it looks fine, can the existing |
|
@mkowa42 Or is it also required for the |
|
@mkowa42 the reason I'm a little bit concerned about adding a dedicated |
|
Hi @sberyozkin, You mean due to Before I created this PR I tried to set the audience param via Any concern if any param would be allowed to set via |
|
I gave it a try (mkowa42@78befad#diff-d3ce45c2ea11d0d5d0787a10420ccbba327ae4d8fca848dbdbe3ccb90da93ac9R144-R154) and it's working for me with: Above solution is not ideal from a user's point of view. As grant-type is set to Any reason why these extra parameters should be set specific to the grant-type? Instead any extra param could directly go to |
|
Hi @mkowa42
I guess so, the concrete parameter support, ex, for And referring to your example,
I agree, so far we haven't had a requirement to use some extra properties for the I'd like to propose to tweak the PR to make sure both and work. We'd only need one extra enum type for Also,
Well, because the extra parameters can be grant type specific
I can see it will work, and in case when the endpoint needs to deal with at least 2 grant types at the same time, it can save on the configuring for example, thanks |
|
Hi @sberyozkin, Thanks for explaining the backgrounds.
Updating the javadoc in order to mention grant-type Currently the expected key for retrieving the grant-type specific grant-options is not the same due to using the I will update the code accordingly if you agree. |
mkowa42
force-pushed
the
add-token-audience-to-oidc-client
branch
from
d0d6423 to
100fec4
Compare
mkowa42
commented
Apr 5, 2021
...ons/oidc-client/runtime/src/main/java/io/quarkus/oidc/client/runtime/OidcClientRecorder.java
Outdated
Show resolved
Hide resolved
sberyozkin
reviewed
Apr 5, 2021
...ons/oidc-client/runtime/src/main/java/io/quarkus/oidc/client/runtime/OidcClientRecorder.java
Outdated
Show resolved
Hide resolved
Test Failures⚙️ jvm-linux-jdk11📦 extensions/oidc-client/deployment# Tests: 12
+ Success: 11
- Failures: 1
- Errors: 0
! Skipped: 0❌
|
|
@mkowa42 - please also squash the commits once you are finished |
- Allows to set custom request parameters for the client_credentials grant (e.g. the intended audience) on the oidc-client - Add an example to the docs
mkowa42
force-pushed
the
add-token-audience-to-oidc-client
branch
from
100fec4 to
ce7a641
Compare
|
@sberyozkin, The changes are rebased and squashed. Please have again a look. Thanks. |
sberyozkin
reviewed
Apr 6, 2021
| tokenGrantParams.add(OidcConstants.PASSWORD_GRANT_PASSWORD, | ||
| passwordGrantOptions.get(OidcConstants.PASSWORD_GRANT_PASSWORD)); | ||
| grantOptions.get(OidcConstants.PASSWORD_GRANT_PASSWORD)); | ||
| } else if (grantOptions != null && oidcConfig.grant.getType() == Grant.Type.CLIENT) { |
There was a problem hiding this comment.
Just a note to myself - throw the configuration exception if the password options (in the branch above) if the options are null.
|
@mkowa42 LGTM, thanks I'll follow up later on with the NPE options check for the |
sberyozkin
approved these changes
Apr 6, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR extends the OIDC Client by adding the token audience param for requesting access tokens with an expected aud claim from the token endpoint.
Background: Some IdPs like Auth0 require the intended audience to be specified when requesting the access token.