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 custom params during token exchange #1010

Merged
merged 1 commit into from
Mar 12, 2021

Conversation

dopsonbr
Copy link
Contributor

I have a use case where I need to pass custom params, like roles, during the token exchange. Are you open to accepting a PR that enables this behavior?

@damienbod
Copy link
Owner

damienbod commented Mar 12, 2021

@dopsonbr LGTM thanks

I need to validate that this will work per spec. Most have custom params just for the authorize request.

What STS do you use?

Greetings Damien

@dopsonbr
Copy link
Contributor Author

My company uses an on-prem installation of PING so the STS isn't public.

@damienbod
Copy link
Owner

@dopsonbr Looking into to this. Main problem is that the custom params are sent then for both the authorize and the token request with this change. I'm not sure other STS will work when send additional custom params to the token end point. I need to validate this first.

@dopsonbr
Copy link
Contributor Author

@dopsonbr Looking into to this. Main problem is that the custom params are sent then for both the authorize and the token request with this change. I'm not sure other STS will work when send additional custom params to the token end point. I need to validate this first.

@damienbod I agree this is potentially problematic. What if I moved it into a new config parameter customTokenParams in the OpenIdConfiguration object?

@damienbod
Copy link
Owner

@dopsonbr Yes this would be great, then the PR will not effect anything else.

Greetings Damien

@dopsonbr
Copy link
Contributor Author

@damienbod I just made the change and added tests. Please let me know what you think

@damienbod damienbod merged commit f62274e into damienbod:main Mar 12, 2021
@damienbod
Copy link
Owner

Thanks @dopsonbr , works great. Will be released in version 11.6.4

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.

2 participants