-
Notifications
You must be signed in to change notification settings - Fork 204
Handle discovery based lookup when using http and nested realms (keycloak style) #211
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
Open
epugh
wants to merge
9
commits into
omniauth:master
Choose a base branch
from
epugh:lower_level_http_fix
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
79b702c
update the local patch
epugh e6e550b
Retest with nested realm from keycloak
epugh 847aad2
add debugging
epugh 20857a0
more logging
epugh a7f7458
Full cycle works!
epugh 6b86d25
now tested on 4.0
epugh 94e75f0
remove debugging output
epugh c6d14b0
Remove old debugging code
epugh 361385b
Proc use wasnt actually needed
epugh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 thought there was a reason why HTTPS was enforced. Is this only done for supporting development? I'm a bit concerned this could inadvertently open security holes.
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.
So, in my use case, I am running Keycloak on HTTP, and I'm using it for testing of the OpenID integration with the Quepid (http://github.com/o19s/quepid) project. I am explicitly setting up a HTTP url, so it seems like under the principle of least surprise that we should accept that, instead of silently converting to HTTPS and then having an error. I think at a minimum if we aren't willing to accept HTTPS, then a clear error message should be thrown, not a "I tried to do a HTTPS lookup on your HTTP url and it failed due to werid ssl issues" that we have today.
My feeling is that if I set up security with HTTP, well then, that is on me.
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 did the workaround (reference in the related issue) that let me use HTTP. A bit clunky. It feels like either HTTP works or it doesn't work, but not "it works in some cases and not others!". However, there may be nuances of all of this that I am not understanding ;-).