-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 keycloak provider #227
Conversation
Excellent, thank you for taking the time to do this :-) |
LGTM 👍 - @JoelSpeed do we have any additional processes that need completing when adding a new provider? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've typically asked people to become maintainers of the provider and add themselves for the new files to the owners file, though I'm happy to let this one slide as it's pretty simple (and is mainly just the group email fetching code)
Need one small change then we can merge this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
@Ofinka Do you mind adding yourself to the owners file for future changes? https://github.com/pusher/oauth2_proxy/blob/master/.github/CODEOWNERS#L12 |
Has this one stalled now, as it seemed to be close to getting merged 😃 |
Hi guys, I'm sorry but I'm afraid I do not have enough time to be part of another project :( Would definitely appreciate if anyone else could take the responsibility. |
Understood. I think we should merge this as is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi guys, I'm sorry but I'm afraid I do not have enough time to be part of another project :(
I appreciate this...
If you can fix up the extra lines added to the changelog then I'm happy to merge as is, thanks! 🙂
Would love to see this move along. |
If we can fix @JoelSpeed's comment above, then I can get this merged. |
The author of the pull request has not responded since August 19, not sure when or if they will. |
I have fixed the CHANGELOG error. Just need to get it re-reviewed by @JoelSpeed or @steakunderscore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, LGTM
Thank you and congrats on your first contribution! 🎉 |
Thanks for your help!! |
Adding integration of keycloak provider based on forgotten PR https://github.com/bitly/oauth2_proxy/pull/366/files
Solves #127
How Has This Been Tested?
Tested manually. I set up keycloak and reference it in configuration of oauth2_proxy using this provider.