Skip to content

Commit 78525b1

Browse files
authored
fix: Ensure x-forwarded-port header is used in Forwarded header (#115)
## What I'm Changing This PR ensures that a `x-forwarded-port` header is used when constructing the `forwarded` header. ### Backstory We have been experiencing an issue in a Kubernetes environment where Links were being written with `href` values with incorrect origins. To be specific, they would point to `localhost:8001` rather than `localhost`, where `localhost` is the public-facing host and `:8001` is the port of the internal upstream STAC API. The network layout was such that we had a Traefik proxy in front of the STAC Auth Proxy. That proxy sets the `x-forwarded-port` header on requests that were sent to the STAC Auth Proxy. However, the request did _not_ contain a `forwarded` header, so we would create one. The problem was that our current logic would _not_ make use of the `x-forwarded-port` when created the `forwarded` header. Despite us forwarding the `x-forwarded-port`, the upstream STAC API would defer to the `forwarded` header and thus ignore the provided `x-forwarded-port`. This meant that the upstream STAC API would generate links for `http://localhost:8001`. Digging through `stac-fastapi` code, we can see that the port first is read from the `host` header and then possibly overridden by the value in `forwarded`: https://github.com/stac-utils/stac-fastapi/blob/3b24f86bc538b8c1d6b86008845d5541f4f481e8/stac_fastapi/api/stac_fastapi/api/middleware.py#L87-L106 The current problematic flow looks something like this: ```mermaid sequenceDiagram autonumber participant Client as Client participant Traefik as Traefik Proxy participant Auth as STAC Auth Proxy participant STAC as STAC API Client->>Traefik: HTTP request Note left of Traefik: host: http://localhost Traefik->>Auth: Forward request Note left of Auth: host: http://localhost Note left of Auth: x-forwarded-for: 192.168.65.1 Note left of Auth: x-forwarded-host: localhost Note left of Auth: x-forwarded-port: 80 Note left of Auth: x-forwarded-proto: http Note left of Auth: x-forwarded-server: 26a4cc50fa1a Auth->>STAC: Forward request (proxied) Note left of STAC: host: http://stac:8001 Note left of STAC: x-forwarded-for: 192.168.65.1 Note left of STAC: x-forwarded-host: localhost Note left of STAC: x-forwarded-port: 80 Note left of STAC: x-forwarded-proto: http Note left of STAC: x-forwarded-server: 26a4cc50fa1a Note left of STAC: via: 1.1 stac-auth-proxy Note left of STAC: forwarded: for=192.168.65.1 host=localhost proto=http path=/stac STAC-->>Auth: Response document Note left of Auth: links point to http://localhost:8001/... Auth-->>Client: Response (unchanged body) ``` Unfortunately, our link rewriting middleware would miss these links as it was looking for the internal upstream url `stac:8001` and not `localhost:8001`. This is maybe besides the point, as the link rewriting middleware is really about cleaning up paths and in an ideal world we expect the upstream STAC API to properly make use of the `forwarded` header to properly construct links (which stac-fastapi-pgstac does decently well). <details> <summary>some more thoughts about <code>x-forwarded-port</code></summary> Reading the MDN docs on the `forwarded` header[^1], we see: > The alternative and de-facto standard versions of this header are the [X-forwarded-For](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/X-forwarded-For), [X-forwarded-Host](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/X-forwarded-Host) and [X-forwarded-Proto](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/X-forwarded-Proto) headers. The lack of mention of `x-forwarded-port` led me to believe that the `X-forwarded-Host` included the port. However, that doesn't not seem accurate when reviewing Traefik behavior. </details> ## How I did it The fix was pretty simple: ensure that the `host` within the `forwarded` header contained the x-forwarded port. ## How you can test it Docker image of this build available here: https://github.com/developmentseed/stac-auth-proxy/releases/tag/v0.10.2-rc2
1 parent 6396563 commit 78525b1

File tree

2 files changed

+47
-2
lines changed

2 files changed

+47
-2
lines changed

src/stac_auth_proxy/handlers/reverse_proxy.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,13 @@ def __post_init__(self):
3535
)
3636

3737
def _prepare_headers(self, request: Request) -> MutableHeaders:
38-
"""Prepare headers for the proxied request."""
38+
"""
39+
Prepare headers for the proxied request. Construct a Forwarded header to inform
40+
the upstream API about the original request context, which will allow it to
41+
properly construct URLs in responses (namely, in the Links). If there are
42+
existing X-Forwarded-*/Forwarded headers (typically, in situations where the
43+
STAC Auth Proxy is behind a proxy like Traefik or NGINX), we use those values.
44+
"""
3945
headers = MutableHeaders(request.headers)
4046
headers.setdefault("Via", f"1.1 {self.proxy_name}")
4147

@@ -44,18 +50,29 @@ def _prepare_headers(self, request: Request) -> MutableHeaders:
4450
)
4551
proxy_proto = headers.get("X-Forwarded-Proto", request.url.scheme)
4652
proxy_host = headers.get("X-Forwarded-Host", request.url.netloc)
53+
proxy_port = str(headers.get("X-Forwarded-Port", request.url.port))
4754
proxy_path = headers.get("X-Forwarded-Path", request.base_url.path)
55+
56+
# NOTE: If we don't include a port, it's possible that the upstream server may
57+
# mistakenly use the port from the Host header (which may be the internal port
58+
# of the upstream server) when constructing URLs.
59+
forwarded_host = proxy_host
60+
if proxy_port:
61+
forwarded_host = f"{forwarded_host}:{proxy_port}"
62+
4863
headers.setdefault(
4964
"Forwarded",
50-
f"for={proxy_client};host={proxy_host};proto={proxy_proto};path={proxy_path}",
65+
f"for={proxy_client};host={forwarded_host};proto={proxy_proto};path={proxy_path}",
5166
)
5267

5368
# NOTE: This is useful if the upstream API does not support the Forwarded header
69+
# and there were no existing X-Forwarded-* headers on the incoming request.
5470
if self.legacy_forwarded_headers:
5571
headers.setdefault("X-Forwarded-For", proxy_client)
5672
headers.setdefault("X-Forwarded-Host", proxy_host)
5773
headers.setdefault("X-Forwarded-Path", proxy_path)
5874
headers.setdefault("X-Forwarded-Proto", proxy_proto)
75+
headers.setdefault("X-Forwarded-Port", proxy_port)
5976

6077
# Set host to the upstream host
6178
if self.override_host:

tests/test_reverse_proxy.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,3 +282,31 @@ async def test_nginx_headers_behavior(scope_overrides, headers, expected_forward
282282
assert f"{key}={expected_value}" in forwarded, (
283283
f"Expected {key}={expected_value} in {forwarded}"
284284
)
285+
286+
287+
@pytest.mark.parametrize("legacy_headers", [False, True])
288+
@pytest.mark.asyncio
289+
async def test_x_forwarded_port_in_forwarded_header(legacy_headers):
290+
"""Test that x-forwarded-port is included in the Forwarded header."""
291+
headers = [
292+
(b"host", b"localhost:8000"),
293+
(b"user-agent", b"test-agent"),
294+
(b"x-forwarded-port", b"443"),
295+
(b"x-forwarded-proto", b"https"),
296+
(b"x-forwarded-host", b"api.example.com"),
297+
]
298+
request = create_request(headers=headers)
299+
handler = ReverseProxyHandler(
300+
upstream="http://upstream-api.com", legacy_forwarded_headers=legacy_headers
301+
)
302+
result_headers = handler._prepare_headers(request)
303+
304+
# Check that the Forwarded header includes the port
305+
forwarded = result_headers["Forwarded"]
306+
assert "host=api.example.com:443" in forwarded, (
307+
f"Expected host=api.example.com:443 in {forwarded}"
308+
)
309+
assert "proto=https" in forwarded
310+
311+
# Check that the x-forwarded-port header is preserved
312+
assert result_headers["X-Forwarded-Port"] == "443"

0 commit comments

Comments
 (0)