feat: fix OAuth redirect_uri hint for proxied/iframe deployments#4417
Open
feat: fix OAuth redirect_uri hint for proxied/iframe deployments#4417
Conversation
…hint Uvicorn's ProxyHeadersMiddleware handles X-Forwarded-Proto (scheme) and X-Forwarded-For (client IP) but not X-Forwarded-Host. When the gateway is behind a reverse proxy, request.base_url still shows the internal host, causing the OAuth redirect_uri hint in the admin UI to display the wrong callback URL. Add ForwardedHostMiddleware that rewrites the ASGI host header and scope["server"] from X-Forwarded-Host, making request.base_url reflect the proxy's public address. This fixes the redirect_uri hint in all four add/edit gateway and A2A agent forms without template changes. The middleware mirrors the approach in Uvicorn PR #2811 (Kludex/uvicorn#965) and can be removed when upstream merges that support. Closes #4354 Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
…limitations Extract _proxy_trusted_hosts variable so ProxyHeadersMiddleware and ForwardedHostMiddleware always use the same trust setting. Document that ForwardedHostMiddleware only acts for wildcard trust (no per-IP checking) and that default port values follow RFC 2616. Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
The middleware had a trusted_hosts parameter that was a no-op for non-wildcard values, making it look like it did per-IP trust checking when it did not. Remove the parameter entirely -- trust decisions belong to the caller (main.py gates registration). Also clean up docstrings to document port parsing behavior accurately. Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
madhav165
requested review from
brian-hussey,
crivetimihai and
kevalmahajan
as code owners
- Swap registration order so ForwardedHostMiddleware is inner to ProxyHeadersMiddleware, ensuring scope["scheme"] is already corrected when deriving the default port for scope["server"]. - Replace uvicorn._types imports with stdlib typing equivalents to avoid coupling to a private module. - Inline the trusted_hosts literal instead of a stray module-level constant. - Update docstring to match the corrected registration order. Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
madhav165
added
ica
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
1 participant
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.
✨ Feature / Enhancement PR
🔗 Epic / Issue
Closes #4354
🚀 Summary (1-2 sentences)
Add
ForwardedHostMiddlewarethat rewrites the ASGIhostheader fromX-Forwarded-Host, so thatrequest.base_urlreflects the proxy's public address when the gateway is behind a reverse proxy. This fixes the OAuthredirect_urihint in the admin UI showing the gateway's internal address instead of the proxy URL.🧪 Checks
make lintpassesmake testpasses📓 Notes
Problem
The admin UI shows an OAuth redirect_uri hint (
💡 Use: {base_url}oauth/callback) on gateway and A2A agent add/edit forms. When the gateway is behind a reverse proxy, this hint shows the internal gateway address because Uvicorn'sProxyHeadersMiddlewarehandlesX-Forwarded-Proto(scheme) andX-Forwarded-For(client IP) but notX-Forwarded-Host(host). This is a known upstream gap: encode/uvicorn#965, with an unmerged PR #2811.Solution
A small ASGI middleware (
ForwardedHostMiddleware) registered alongsideProxyHeadersMiddlewarethat:X-Forwarded-Host, takes the first comma-separated valuehostheader inscope["headers"]and thescope["server"]tupleSecurity review
_build_server_resource_url): usessettings.app_domain, immune to host rewritingsettings.app_domain+ allowlist, immuneX-Forwarded-Hostdirectly (pre-existing), no regressiontrusted_hosts="*": pre-existing decision onProxyHeadersMiddleware, not introduced by this changeFiles changed
mcpgateway/middleware/forwarded_host.pymcpgateway/main.pytests/unit/mcpgateway/middleware/test_forwarded_host_middleware.pytests/playwright/test_forwarded_host_redirect_hint.pyCan be removed when Uvicorn merges upstream
X-Forwarded-Hostsupport.