diff --git a/docs/user-guide/configuration.md b/docs/user-guide/configuration.md index ae82c3c..fc6da1e 100644 --- a/docs/user-guide/configuration.md +++ b/docs/user-guide/configuration.md @@ -92,6 +92,21 @@ The application is configurable via environment variables. > [!NOTE] > This is independent of the upstream API's path. The proxy will handle removing this prefix from incoming requests and adding it to outgoing links. +### `ROOT_PATH_SKIP_PREFIXES` + +: Path prefixes that should **not** get `ROOT_PATH` added + + - **Type:** comma-separated list of path prefixes + - **Required:** No, defaults to `''` (add `ROOT_PATH` to all same-host links) + - **Example:** `/raster,/vector,/browser` + + > [!NOTE] + > The proxy usually **adds** `ROOT_PATH` to same-host links that do not have it yet (e.g. `/collections` → `/stac/collections`). That is intentional: upstream often returns links without `ROOT_PATH`. + > + > The problem is when other apps live on the same host (e.g. a tiler at `/raster`). Their links look like same-host links too, so the proxy would turn `/raster/...` into `/stac/raster/...` and break them. List those other apps' path prefixes here so they are left alone. (If a link still uses the upstream hostname, the proxy can still rewrite the host to the public one; only the `ROOT_PATH` add is skipped.) + > + > Matching is by path segment: `/raster` matches `/raster` and `/raster/...`, but not `/rasterfoo`. + ## Authentication ### `OIDC_DISCOVERY_URL` diff --git a/docs/user-guide/tips.md b/docs/user-guide/tips.md index 1f14440..67323f6 100644 --- a/docs/user-guide/tips.md +++ b/docs/user-guide/tips.md @@ -39,6 +39,8 @@ The proxy can be optionally served from a non-root path (e.g., `/api/v1`). Addit - Update the OpenAPI specification to include the `ROOT_PATH` in the servers field - Handle requests that don't match the `ROOT_PATH` with a 404 response +If other apps share the same host (e.g. the proxy at `/stac` and a tiler at `/raster`), their links would also get `ROOT_PATH` added by mistake. Set `ROOT_PATH_SKIP_PREFIXES` to those paths (e.g. `/raster`) so the proxy leaves them alone. + ## Non-OIDC Workaround If the upstream server utilizes RS256 JWTs but does not utilize a proper OIDC server, the proxy can be configured to work around this by setting the `OIDC_DISCOVERY_URL` to a statically-hosted OIDC discovery document that points to a valid JWKS endpoint. diff --git a/helm/values.schema.json b/helm/values.schema.json index 9ca36a2..df705d4 100644 --- a/helm/values.schema.json +++ b/helm/values.schema.json @@ -70,6 +70,11 @@ "description": "Path prefix for the proxy API. Also prefixes probe httpGet.path when those paths are unset.", "default": "" }, + "ROOT_PATH_SKIP_PREFIXES": { + "type": "string", + "description": "Comma-separated path prefixes that should not get ROOT_PATH added (e.g. /raster,/vector,/browser when those apps share the same host).", + "default": "" + }, "OIDC_DISCOVERY_URL": { "type": "string", "pattern": "^https?://.+", diff --git a/helm/values.yaml b/helm/values.yaml index 6b94f90..ae1ece7 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -148,6 +148,7 @@ env: # Used with ROOT_PATH to derive probe httpGet.path when not set explicitly HEALTHZ_PREFIX: "/healthz" # ROOT_PATH: "/stac" # If set, probes default to {ROOT_PATH}{HEALTHZ_PREFIX} + # ROOT_PATH_SKIP_PREFIXES: "/raster,/vector,/browser" # Other apps on the same host; do not add ROOT_PATH OIDC_DISCOVERY_INTERNAL_URL: "http://localhost:8081/.well-known/openid-configuration" DEFAULT_PUBLIC: false PRIVATE_ENDPOINTS: | diff --git a/pyproject.toml b/pyproject.toml index d06a16b..36d6a1d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,7 +16,7 @@ dependencies = [ "fastapi>=0.115.5", "httpx[http2]>=0.28.0", "jinja2>=3.1.6", - "pydantic-settings>=2.6.1", + "pydantic-settings>=2.7.0", "pyjwt>=2.10.1", "starlette>=1.0.1", "starlette-cramjam>=0.4.0", diff --git a/src/stac_auth_proxy/app.py b/src/stac_auth_proxy/app.py index 1c56d0c..706a3ce 100644 --- a/src/stac_auth_proxy/app.py +++ b/src/stac_auth_proxy/app.py @@ -175,6 +175,7 @@ def configure_app( ProcessLinksMiddleware, upstream_url=str(settings.upstream_url), root_path=settings.root_path, + root_path_skip_prefixes=settings.root_path_skip_prefixes, ) if settings.root_path: diff --git a/src/stac_auth_proxy/config.py b/src/stac_auth_proxy/config.py index 9c11ad9..ea14056 100644 --- a/src/stac_auth_proxy/config.py +++ b/src/stac_auth_proxy/config.py @@ -2,11 +2,11 @@ import importlib import json -from typing import Any, Literal, Optional, Sequence, TypeAlias, Union +from typing import Annotated, Any, Literal, Optional, Sequence, TypeAlias, Union from pydantic import BaseModel, Field, field_validator, model_validator from pydantic.networks import HttpUrl -from pydantic_settings import BaseSettings, SettingsConfigDict +from pydantic_settings import BaseSettings, NoDecode, SettingsConfigDict METHODS = Literal["GET", "POST", "PUT", "DELETE", "PATCH"] EndpointMethods: TypeAlias = dict[str, Sequence[METHODS]] @@ -79,6 +79,9 @@ class Settings(BaseSettings): allowed_jwt_audiences: Optional[Sequence[str]] = None root_path: str = "" + # NoDecode: pydantic-settings JSON-decodes Sequence fields before validators, + # which rejects the documented comma-separated env form (/raster,/vector). + root_path_skip_prefixes: Annotated[Sequence[str], NoDecode] = () override_host: bool = True healthz_prefix: str = Field(pattern=_PREFIX_PATTERN, default="/healthz") upstream_timeout: float = 15.0 @@ -147,3 +150,29 @@ def _default_oidc_discovery_internal_url(cls, data: Any) -> Any: def parse_audience(cls, v) -> Sequence[str] | None: """Parse a comma separated string list of audiences into a list.""" return str2list(v) + + @field_validator("root_path_skip_prefixes", mode="before") + @classmethod + def parse_root_path_skip_prefixes(cls, v) -> Sequence[str]: + """ + Parse and normalize path prefixes that should not get ``ROOT_PATH`` added. + + Accepts a comma-separated string or sequence. Drops empty entries and + trailing slashes. Each prefix must start with ``/``. A bare ``/`` is + rejected because it would match every path. + """ + values = str2list(v) + if not values: + return () + + prefixes: list[str] = [] + for value in values: + prefix = value.strip().rstrip("/") + if not prefix: + if value.strip(): + raise ValueError(f"Path prefix {value!r} would match every path") + continue + if not prefix.startswith("/"): + raise ValueError(f"Path prefix {value!r} must start with '/'") + prefixes.append(prefix) + return tuple(prefixes) diff --git a/src/stac_auth_proxy/middleware/ProcessLinksMiddleware.py b/src/stac_auth_proxy/middleware/ProcessLinksMiddleware.py index 112f638..385c5ec 100644 --- a/src/stac_auth_proxy/middleware/ProcessLinksMiddleware.py +++ b/src/stac_auth_proxy/middleware/ProcessLinksMiddleware.py @@ -3,7 +3,7 @@ import logging import re from dataclasses import dataclass -from typing import Any, Optional +from typing import Any, Optional, Sequence from urllib.parse import ParseResult, urlparse, urlunparse from starlette.datastructures import Headers @@ -29,6 +29,7 @@ class ProcessLinksMiddleware(JsonResponseMiddleware): root_path: Optional[str] = None json_content_type_expr: str = r"application/(geo\+)?json" + root_path_skip_prefixes: Sequence[str] = () def should_transform_response(self, request: Request, scope: Scope) -> bool: """Only transform responses with JSON content type.""" @@ -57,6 +58,13 @@ def transform_json(self, data: dict[str, Any], request: Request) -> dict[str, An ) return data + def _matches_skip_prefix(self, path: str) -> bool: + """Return whether path is exactly a skip prefix or under one of them.""" + return any( + path == prefix or path.startswith(f"{prefix}/") + for prefix in self.root_path_skip_prefixes + ) + def _update_link( self, link: dict[str, Any], request_url: ParseResult, upstream_url: ParseResult ) -> None: @@ -82,16 +90,18 @@ def _update_link( ) return - # If the link path is not a descendant of the upstream path, don't transform it + # Skip links outside the upstream path, unless they match a skip prefix + # (those still need host rewrite, just not root_path). if upstream_url.path != "/" and not parsed_link.path.startswith( upstream_url.path ): - logger.debug( - "Ignoring link %s because it is not descendant of upstream path (%s)", - link["href"], - upstream_url.path, - ) - return + if not self._matches_skip_prefix(parsed_link.path): + logger.debug( + "Ignoring link %s because it is not descendant of upstream path (%s)", + link["href"], + upstream_url.path, + ) + return # Replace the upstream host with the client's host if parsed_link.netloc == upstream_url.netloc: @@ -105,14 +115,15 @@ def _update_link( path=parsed_link.path[len(upstream_url.path) :] ) - # Add the root_path to the link if it exists - if self.root_path and not ( - parsed_link.path.startswith(f"{self.root_path}/") - or parsed_link.path == self.root_path + # Add root_path unless this link is for another app on the same host + # (listed in root_path_skip_prefixes), or it already has root_path. + path = parsed_link.path + if ( + self.root_path + and not self._matches_skip_prefix(path) + and not (path.startswith(f"{self.root_path}/") or path == self.root_path) ): - parsed_link = parsed_link._replace( - path=f"{self.root_path}{parsed_link.path}" - ) + parsed_link = parsed_link._replace(path=f"{self.root_path}{path}") updated_href = urlunparse(parsed_link) if updated_href == link["href"]: diff --git a/tests/test_config.py b/tests/test_config.py index a6dcce1..f0ac533 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,5 +1,7 @@ """Test config classes.""" +import pytest + from stac_auth_proxy.config import CorsSettings, Settings @@ -53,6 +55,53 @@ def test_settings_model_config(): assert settings.allowed_jwt_audiences == [""] +def test_root_path_skip_prefixes(): + """Test parsing and normalization of root_path_skip_prefixes.""" + common_kwargs = { + "upstream_url": "https://example.com", + "oidc_discovery_url": "https://example.com/.well-known/openid-configuration", + } + + # Defaults to empty (feature disabled) + settings = Settings(**common_kwargs) + assert list(settings.root_path_skip_prefixes) == [] + + # Comma-separated string, with trailing slashes normalized + settings = Settings( + **common_kwargs, + root_path_skip_prefixes="/raster/,/vector,/browser", + ) + assert list(settings.root_path_skip_prefixes) == ["/raster", "/vector", "/browser"] + + # List input + settings = Settings(**common_kwargs, root_path_skip_prefixes=["/raster"]) + assert list(settings.root_path_skip_prefixes) == ["/raster"] + + # Empty entries are dropped + settings = Settings(**common_kwargs, root_path_skip_prefixes="/raster,,") + assert list(settings.root_path_skip_prefixes) == ["/raster"] + + # Prefixes must start with a slash + with pytest.raises(ValueError): + Settings(**common_kwargs, root_path_skip_prefixes="raster") + + # A bare "/" would mean "skip everything" — reject it + with pytest.raises(ValueError): + Settings(**common_kwargs, root_path_skip_prefixes="/") + + +def test_root_path_skip_prefixes_from_environment(monkeypatch): + """Comma-separated env value (e.g. /raster,/vector) must load as a list.""" + monkeypatch.setenv("UPSTREAM_URL", "https://example.com") + monkeypatch.setenv( + "OIDC_DISCOVERY_URL", "https://example.com/.well-known/openid-configuration" + ) + monkeypatch.setenv("ROOT_PATH_SKIP_PREFIXES", "/raster,/vector,/browser") + + settings = Settings() + assert list(settings.root_path_skip_prefixes) == ["/raster", "/vector", "/browser"] + + def test_settings_model_config_with_environment_variables(monkeypatch): """Test that the model config is set correctly with environment variables.""" monkeypatch.setenv("UPSTREAM_URL", "https://example.com") diff --git a/tests/test_process_links.py b/tests/test_process_links.py index 2896f0f..f422377 100644 --- a/tests/test_process_links.py +++ b/tests/test_process_links.py @@ -510,6 +510,111 @@ def test_transform_upstream_links_nested_objects(): ) +@pytest.mark.parametrize( + "root_path_skip_prefixes,input_links,expected_links", + [ + # Skip-listed paths stay put; normal STAC paths still get /stac + ( + ["/raster", "/vector"], + [ + {"rel": "self", "href": "http://proxy.example.com/collections"}, + { + "rel": "xyz", + "href": "http://proxy.example.com/raster/collections/c/items/i/{z}/{x}/{y}", + }, + { + "rel": "tilejson", + "href": "http://proxy.example.com/vector/tilejson.json", + }, + ], + [ + "http://proxy.example.com/stac/collections", + "http://proxy.example.com/raster/collections/c/items/i/{z}/{x}/{y}", + "http://proxy.example.com/vector/tilejson.json", + ], + ), + # No skip list: even /raster gets /stac added (the bug without this setting) + ( + (), + [ + {"rel": "xyz", "href": "http://proxy.example.com/raster/tiles"}, + ], + [ + "http://proxy.example.com/stac/raster/tiles", + ], + ), + # "/raster" matches /raster and /raster/..., but not /rasterfoo + ( + ["/raster"], + [ + {"rel": "self", "href": "http://proxy.example.com/rasterfoo/tiles"}, + {"rel": "exact", "href": "http://proxy.example.com/raster"}, + {"rel": "child", "href": "http://proxy.example.com/raster/tiles"}, + ], + [ + "http://proxy.example.com/stac/rasterfoo/tiles", + "http://proxy.example.com/raster", + "http://proxy.example.com/raster/tiles", + ], + ), + ], +) +def test_transform_with_root_path_skip_prefixes( + root_path_skip_prefixes, input_links, expected_links +): + """Skip-listed paths keep their path; STAC links still get root_path.""" + middleware = ProcessLinksMiddleware( + app=None, + upstream_url="http://upstream.example.com", + root_path="/stac", + root_path_skip_prefixes=root_path_skip_prefixes, + ) + request_scope = { + "type": "http", + "path": "/test", + "headers": [ + (b"host", b"proxy.example.com"), + (b"content-type", b"application/json"), + ], + } + + data = {"links": input_links} + transformed = middleware.transform_json(data, Request(request_scope)) + + for i, expected in enumerate(expected_links): + assert transformed["links"][i]["href"] == expected + + +def test_transform_upstream_netloc_links_honor_skip_prefixes(): + """ + Skip-listed links on the upstream host: rewrite host, do not add root_path. + + The link is outside the upstream path (/api), so without the skip list it + would be ignored entirely. With the skip list, we still swap the host. + """ + middleware = ProcessLinksMiddleware( + app=None, + upstream_url="http://upstream.example.com/api", + root_path="/stac", + root_path_skip_prefixes=["/raster"], + ) + request_scope = { + "type": "http", + "path": "/test", + "headers": [ + (b"host", b"proxy.example.com"), + (b"content-type", b"application/json"), + ], + } + + data = { + "links": [{"rel": "xyz", "href": "http://upstream.example.com/raster/tiles"}] + } + transformed = middleware.transform_json(data, Request(request_scope)) + + assert transformed["links"][0]["href"] == "http://proxy.example.com/raster/tiles" + + @pytest.mark.parametrize( "headers,expected_base_url", [ diff --git a/uv.lock b/uv.lock index dee2a6b..e946742 100644 --- a/uv.lock +++ b/uv.lock @@ -2075,7 +2075,7 @@ requires-dist = [ { name = "mkdocs-material", extras = ["imaging"], marker = "extra == 'docs'", specifier = ">=9.6.16" }, { name = "mkdocstrings", extras = ["python"], marker = "extra == 'docs'", specifier = ">=0.30.0" }, { name = "prometheus-fastapi-instrumentator", marker = "extra == 'metrics'", specifier = ">=8.0.2" }, - { name = "pydantic-settings", specifier = ">=2.6.1" }, + { name = "pydantic-settings", specifier = ">=2.7.0" }, { name = "pyjwt", specifier = ">=2.10.1" }, { name = "starlette", specifier = ">=1.0.1" }, { name = "starlette-cramjam", specifier = ">=0.4.0" },