fix: rewrite auth server URLs for remote deployments#99
Open
salmon-21 wants to merge 2 commits intohyprmcp:mainfrom
Open
fix: rewrite auth server URLs for remote deployments#99salmon-21 wants to merge 2 commits intohyprmcp:mainfrom
salmon-21 wants to merge 2 commits intohyprmcp:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves OAuth behavior when the authorization server is only reachable on an internal network but advertises public URLs (typical reverse-proxy / remote deployment setup). It aims to ensure external clients receive reachable gateway URLs while the gateway itself can still talk directly to the internal auth server (notably for JWKS).
Changes:
- Rewrites the authorization redirect URL to use the configured public
host. - Rewrites discovery metadata URL fields from internal auth-server URLs to public gateway URLs.
- Adds reverse-proxying for auth-server endpoints (token/jwks/userinfo/etc.) derived from metadata, and avoids JWKS self-fetch circular dependencies by using the internal auth server address.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
oauth/oauth.go |
Builds internal JWKS fetch URL and registers reverse-proxy routes derived from auth-server metadata; adds authServerProxyPaths. |
oauth/authorization_server_metadata.go |
Rewrites metadata URL fields from internal auth-server URLs to the public host. |
oauth/authorization.go |
Rewrites the authorization redirect to use the public host. |
oauth/oauth_test.go |
Adds unit tests for authServerProxyPaths(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When the authorization server (e.g. Dex) runs on an internal network with its issuer set to the public gateway URL, three issues arise: 1. The authorization redirect sends users to the internal server URL 2. Discovery metadata exposes internal URLs to external clients 3. JWKS fetching creates a circular dependency (gateway fetches from itself) This commit fixes all three: - Rewrite the authorization redirect to use the public host URL - Add rewriteMetadataURLs() to rewrite all discovery metadata URLs from internal to public - Build the JWKS URI from the internal server address to break the circular dependency - Reverse-proxy auth server endpoints (derived from metadata) so external clients can reach token, keys, userinfo, etc. through the public gateway URL Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Handle url.Parse error in authorization redirect instead of ignoring - Move rewriteMetadataURLs inside authorizationProxyEnabled guard so URLs are only rewritten when the proxy routes exist - Restrict metadata URL rewriting to known endpoint fields (allowlist) to avoid rewriting issuer which would break token validation - Use URL parsing for scheme/host comparison instead of string prefix matching to prevent false matches on similar hostnames - Use url.JoinPath for JWKS URI construction and return errors for unparseable URIs instead of silently falling back - Validate paths start with "/" before registering with ServeMux to prevent panics on relative URIs - Add test case for relative URI rejection Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
11602de to
da47e79
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
When the authorization server (e.g. Dex) runs on an internal network with its issuer set to the public gateway URL, external clients receive internal URLs they cannot reach, and the gateway encounters a circular dependency fetching JWKS from itself.
This PR fixes three related issues that arise in remote/reverse-proxy deployments where
authorization.serverdiffers fromhost:rewriteMetadataURLs()to rewrite all URL fields in the/.well-known/oauth-authorization-serverresponse from internal to publicjwks_uriand prepends the internalauthorization.serveraddress, so the gateway fetches keys directly from the auth server instead of from itselfauthorizationProxyEnabledis set, reverse-proxies auth server endpoints (token, keys, userinfo, etc.) derived from the metadata so external clients can reach them through the public gateway URLExample scenario
Without this fix, external clients receive URLs like
http://dex:5556/tokenin the discovery metadata and cannot complete the OAuth flow.Test plan
authServerProxyPaths()(5 cases: standard extraction, deduplication, empty metadata, invalid values, prefix patterns)go build ./...passes🤖 Generated with Claude Code