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 token exchange login method #288

Merged
merged 2 commits into from
Aug 12, 2021

Conversation

bclouser
Copy link

Signed-off-by: Ben Clouser [email protected]

For your consideration...

I don't necessarily expect this to get merged as is. This API method requires a feature be enabled in keycloak to allow a token exchange... I am not sure what you do with features like this.

Additionally, since it requires a custom feature in keycloak, i didnt include the test i wrote, since it fails due to 501 (not implemented)

FWIW We use this feature in our keycloak service, so i figured I would try to upstream the feature 🤷

@Nerzal
Copy link
Owner

Nerzal commented Jun 25, 2021

We could change the API of the function to expect the GrantType as parameter possibly?
Then it could be usable for more than only your custom function

@bclouser
Copy link
Author

you know, looking at this a little more clearly, maybe this shouldnt even exist at all. If we make the grant-type customizable, we might as well just call client.GetToken() directly and pass in the TokenOptions{} ... and maybe that's what i should just do in my own code and forget about adding this function.

That being said, LoginClientTokenExchange() is a nice convenience function for future API users looking to retrieve a user's tokens via API, and prevents having to dig through keycloak docs to learn of the correct grant-type and oidc params.

I'd propose renaming the function to GetUserToken() though, as it's much more clear.

Thoughts?

Copy link
Owner

@Nerzal Nerzal left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution

@Nerzal
Copy link
Owner

Nerzal commented Aug 12, 2021

Hi, thank you for your contribution, now merging.
The release of a new tag will take some more time, as the build pipelines are currently broken.

@Nerzal Nerzal merged commit 3bf86c5 into Nerzal:main Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants