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

Add support for additional dynamic OIDC Token Exchange Request parameters #16373

Open
kistlers opened this issue Jan 8, 2025 · 1 comment
Open
Labels
status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement

Comments

@kistlers
Copy link

kistlers commented Jan 8, 2025

Expected Behavior

In a project I am currently working on, we must send additional request parameters in the OIDC Token Exchange grant request. I want a simpler way to add and define additional parameters to be sent with the token exchange grant request.

Current Behavior

The current implementation of

public final class TokenExchangeOAuth2AuthorizedClientProvider implements OAuth2AuthorizedClientProvider {
does not allow customization to this level. What is possible, is adding a static list of additional parameters to the grant request by passing a customized contextAttributesMapper to the
public final class DefaultOAuth2AuthorizedClientManager implements OAuth2AuthorizedClientManager {
.

Context

To be more specific, when calling OAuth2AuthorizedClientManager::authorize, which will perform a token exchange for this client registration, the request must in my case include an additional parameter to be sent.
This is not possible without completely replacing the implementation of TokenExchangeOAuth2AuthorizedClientProvider for the following reasons:

  • The parametersConverter of the RestClientTokenExchangeTokenResponseClient only has access to the TokenExchangeGrantRequest as an input that might contain per-exchange dynamic parameters.
  • None of the attributes set in the OAuth2AuthorizeRequest passed to OAuth2AuthorizedClientManager::authorize end up in the TokenExchangeGrantRequest object.
  • And even if that were the case, a second call to authorize the client with a different dynamic parameter would not result in another token exchange, as the context thinks the client is already authorized because of this snippet in TokenExchangeOAuth2AuthorizedClientProvider:
    OAuth2AuthorizedClient authorizedClient = context.getAuthorizedClient();
    if (authorizedClient != null && !hasTokenExpired(authorizedClient.getAccessToken())) {
    // If client is already authorized but access token is NOT expired than no
    // need for re-authorization
    return null;
    }

I was able to circumvent these short-comings by:

  • Extending OAuth2AuthorizedClient by adding an additional field for the additional attributes.
  • Copying TokenExchangeOAuth2AuthorizedClientProvider and extending the check for re-authorization using the new field from OAuth2AuthorizedClient.
  • Using a CustomTokenExchangeGrantRequest that extends TokenExchangeGrantRequest and includes my dynamic parameters for the request.
  • Using a parametersConverter that casts the TokenExchangeGrantRequest to a CustomTokenExchangeGrantRequest and sets the additional parameters`
  • Adding the additional attributes to the OAuth2AuthorizationContext.attributes() to compare them to the last request in the next authorize(...) call.
  • Extending the check for re-authorizization and non-expired token by comparing the additional parameters with the last ones.

Yes, this explanation is probably not easy to follow, so I am willing to create a PR that would support my use case in a slightly generalized way for every one to use.
I expect the change to be simpler than what I described above, as my complications mainly arose from the fact that TokenExchangeOAuth2AuthorizedClientProvider is a final class.

@kistlers
Copy link
Author

kistlers commented Jan 10, 2025

Upon implementing a PR to see if my proposed feature can be implemented without too many changes, I realized that my current workaround does not even work as I thought it did.

The custom extended OAuth2AuthorizedClient stored in the context does not retain the additional new fields. I incorrectly assumed that this object would be stored as-is, but it is recreated at a later stage in

return (T) new OAuth2AuthorizedClient(registration, cachedAuthorizedClient.getPrincipalName(),
. My suggested PR recreates the OAuth2AuthorizedClient at this stage with the additional attributes from the authorized client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

1 participant