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

Allow the userinfo endpoint to be configured via options. #75

Merged
merged 3 commits into from
Mar 5, 2020

Conversation

mpinkston
Copy link

Currently, the userinfo endpoint is hard-coded into the library. However, to make this work in some environments (specifically China), it's necessary to use a different endpoint.

Since options are set at plug compile-time, this also allows runtime configuration of this value by setting a :system tuple to reference an environment variable containing the appropriate setting.

I realize this may be a bit edge-case, but I figure it may be important for projects that aim to deploy in environments with this restriction. (And now that EKS is finally available in China, it may be more important than before to allow this to be set as a runtime config so folks can use the same build)

I hope this can be of use.

…ful for environments where the url is different such as China
@Hanspagh Hanspagh self-assigned this Mar 3, 2020
@Hanspagh Hanspagh self-requested a review March 3, 2020 13:55
Copy link
Contributor

@Hanspagh Hanspagh left a comment

Choose a reason for hiding this comment

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

This looks good to me, I know there has not been a history of unit tests in this repo, but could this be the time to add some tests related to this?

@mpinkston
Copy link
Author

no problem. I'll add some in the morning 👍

@mpinkston
Copy link
Author

@Hanspagh Allrighty, I've added tests to cover the basics. Let me know what you think.

@Hanspagh Hanspagh self-requested a review March 5, 2020 10:04
Copy link
Contributor

@Hanspagh Hanspagh left a comment

Choose a reason for hiding this comment

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

Really great work and nice to see the test actually found some small potentiall problems.
Thank you

@Hanspagh Hanspagh merged commit 6a430de into ueberauth:master Mar 5, 2020
@mpinkston mpinkston deleted the userinfo-endpoint branch March 6, 2020 00:49
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