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

feat: also allow localhost equivalent IP addresses #3778

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

dsonck92
Copy link
Contributor

@dsonck92 dsonck92 commented Oct 3, 2024

Overview

Instead of only checking for "localhost", also validate through net.ParseIP + IsLoopback whether the host is numerically localhost

What this PR does / why we need it

Currently, localhost is already used as a special case that allows any port and http scheme if no particular redirectURIs are configured. When integrating with ownClouds desktop client, it actually sends an arbitrary http://127.0.0.1:xyz redirect uri. Considering that the go standard library already has the functionality to check if a given IP is a loopback address, we can extend the == "localhost" with also a check for IsLoopback() which should allow the ownCloud desktop client to be accepted.

Special notes for your reviewer

Instead of only checking for "localhost", also validate through
net.ParseIP + IsLoopback whether the host is numerically localhost

Signed-off-by: Daniel Sonck <daniel@sonck.nl>
@dsonck92 dsonck92 force-pushed the also-allow-numeric-localhost branch from 0386dbd to af7599e Compare October 3, 2024 15:05
@nabokihms nabokihms added the release-note/new-feature Release note: Exciting New Features label Oct 7, 2024
@nabokihms
Copy link
Member

Great improvement! Thanks.

@nabokihms nabokihms merged commit e7c0682 into dexidp:master Oct 7, 2024
10 checks passed
@dsonck92 dsonck92 deleted the also-allow-numeric-localhost branch October 8, 2024 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/new-feature Release note: Exciting New Features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants