diff --git a/docs/user-guide/configuration.md b/docs/user-guide/configuration.md index fc6da1e..012e0f2 100644 --- a/docs/user-guide/configuration.md +++ b/docs/user-guide/configuration.md @@ -323,10 +323,10 @@ These settings configure the CORS behavior when `PROXY_OPTIONS` is `false` (the ### `ITEMS_FILTER_PATH` -: Regex pattern used to identify request paths that require the application of the items filter +: Regex patterns used to identify request paths that require the application of the items filter. See [Filter paths and path params](#filter-paths-and-path-params). - - **Type:** Regex string - - **Required:** No, defaults to `^(/collections/([^/]+)/items(/[^/]+)?$|/search$)` + - **Type:** Regex string, or a JSON array of regex strings + - **Required:** No, defaults to `["^(?:/collections/(?P[^/]+)/items(?:/(?P[^/]+))?|/search)$"]` - **Example:** `^(/collections/([^/]+)/items(/[^/]+)?$|/search$|/custom$)` ### `COLLECTIONS_FILTER_CLS` @@ -355,8 +355,58 @@ These settings configure the CORS behavior when `PROXY_OPTIONS` is `false` (the ### `COLLECTIONS_FILTER_PATH` -: Regex pattern used to identify request paths that require the application of the collections filter +: Regex patterns used to identify request paths that require the application of the collections filter. See [Filter paths and path params](#filter-paths-and-path-params). - - **Type:** Regex string - - **Required:** No, defaults to `^/collections(/[^/]+)?$` + - **Type:** Regex string, or a JSON array of regex strings + - **Required:** No, defaults to `["^/collections(?:/(?P[^/]+))?$"]` - **Example:** `^.*?/collections(/[^/]+)?$` + +### Filter paths and path params + +`ITEMS_FILTER_PATH` and `COLLECTIONS_FILTER_PATH` do two jobs: + +1. **Scope**: They select the request paths a filter applies to. +2. **Information**: They declare the request "path parameters" information handed to the filter. + +**Path params.** Named capture groups in the pattern that matched become `req.path_params`. A pattern declaring no named groups (the default) instead falls back to built-in extraction, which recognizes `/collections/{collection_id}` optionally followed by `items`, `bulk_items`, or `queryables` and an item ID. Patterns using the fallback are named in a log line at startup. + +**Supporting several patterns.** Covering all of your endpoints may require more than one pattern, especially if using named capture groups which do not support redefinition of the group name. To provide more than one pattern, provide the input as a JSON array. + +**Authentication.** A path matching either setting always requires authentication, whatever `DEFAULT_PUBLIC` is set to, and is marked accordingly in the OpenAPI spec. A separate `PRIVATE_ENDPOINTS` entry is not needed. + +**Example: the Aggregation extension.** The [STAC API Aggregation extension](https://github.com/stac-api-extensions/aggregation) adds four endpoints, which `stac-fastapi` registers as: + +| Path | Methods | +| --- | --- | +| `/aggregate` | GET | +| `/aggregations` | GET | +| `/collections/{collection_id}/aggregate` | GET | +| `/collections/{collection_id}/aggregations` | GET | + +None are covered by the defaults. The two collection-scoped endpoints belong to the collections filter, and each needs its own pattern because both declare `collection_id`: + +``` +COLLECTIONS_FILTER_PATH='[ + "^/collections(?:/(?P[^/]+))?$", + "^/collections/(?P[^/]+)/aggregate$", + "^/collections/(?P[^/]+)/aggregations$" +]' +``` + +The two root-level endpoints aggregate across Items, so they belong to the items filter alongside `/search`: + +``` +ITEMS_FILTER_PATH='[ + "^(?:/collections/(?P[^/]+)/items(?:/(?P[^/]+))?|/search)$", + "^/aggregate$", + "^/aggregations$" +]' +``` + +A pattern passes whatever it declares, so a path carrying more variables passes more of them: + +``` +"^/mosaic/(?P[^/]+)/(?P[^/]+)/(?P[^/]+)/(?P[^/]+)\\.png$" +``` + +gives the filter `zoom`, `x`, `y`, and `collection_id`. diff --git a/src/stac_auth_proxy/config.py b/src/stac_auth_proxy/config.py index ea14056..9669722 100644 --- a/src/stac_auth_proxy/config.py +++ b/src/stac_auth_proxy/config.py @@ -2,6 +2,7 @@ import importlib import json +import re from typing import Annotated, Any, Literal, Optional, Sequence, TypeAlias, Union from pydantic import BaseModel, Field, field_validator, model_validator @@ -28,6 +29,36 @@ def str2list(x: str | Sequence[str] | None) -> Sequence[str] | None: return x +# NoDecode: pydantic-settings JSON-decodes Sequence fields before validators run, +# which would reject a plain regex string. +FilterPaths: TypeAlias = Annotated[Sequence[str], NoDecode] + + +def str2patterns(x: str | Sequence[str] | None) -> Sequence[str]: + """ + Parse a filter path or paths setting into a list of regex patterns. + + Used as a Pydantic validator, this function supports: + - Single path pattern input as a string + - Multiple path patterns input as a JSON encoded string + - Directly passing multiple path parameters + """ + if x is None: + return [] + + if isinstance(x, str): + patterns = json.loads(x) if x.startswith("[") else [x] + else: + patterns = list(x) + + for pattern in patterns: + try: + re.compile(pattern) + except re.error as e: + raise ValueError(f"{pattern!r} is not a valid regular expression: {e}") + return patterns + + class _ClassInput(BaseModel): """Input model for dynamically loading a class or function.""" @@ -129,9 +160,13 @@ class Settings(BaseSettings): # Filters items_filter: Optional[_ClassInput] = None - items_filter_path: str = r"^(/collections/([^/]+)/items(/[^/]+)?$|/search$)" + items_filter_path: FilterPaths = [ + r"^(?:/collections/(?P[^/]+)/items(?:/(?P[^/]+))?|/search)$" + ] collections_filter: Optional[_ClassInput] = None - collections_filter_path: str = r"^/collections(/[^/]+)?$" + collections_filter_path: FilterPaths = [ + r"^/collections(?:/(?P[^/]+))?$" + ] model_config = SettingsConfigDict( env_nested_delimiter="_", @@ -151,6 +186,17 @@ def parse_audience(cls, v) -> Sequence[str] | None: """Parse a comma separated string list of audiences into a list.""" return str2list(v) + @field_validator("items_filter_path", "collections_filter_path", mode="before") + @classmethod + def parse_filter_paths(cls, v) -> Sequence[str]: + """ + Parse the regex patterns identifying paths that a filter applies to. + + Named capture groups in a pattern become the ``req.path_params`` passed to + the filter. A pattern declaring none falls back to the built-in extraction. + """ + return str2patterns(v) + @field_validator("root_path_skip_prefixes", mode="before") @classmethod def parse_root_path_skip_prefixes(cls, v) -> Sequence[str]: diff --git a/src/stac_auth_proxy/middleware/AuthenticationExtensionMiddleware.py b/src/stac_auth_proxy/middleware/AuthenticationExtensionMiddleware.py index 03539bf..72bb5b8 100644 --- a/src/stac_auth_proxy/middleware/AuthenticationExtensionMiddleware.py +++ b/src/stac_auth_proxy/middleware/AuthenticationExtensionMiddleware.py @@ -3,7 +3,7 @@ import logging import re from dataclasses import dataclass, field -from typing import Any, Optional +from typing import Any, Sequence from urllib.parse import urlparse from starlette.datastructures import Headers @@ -35,8 +35,8 @@ class AuthenticationExtensionMiddleware(JsonResponseMiddleware): "https://stac-extensions.github.io/authentication/v1.1.0/schema.json" ) - items_filter_path: Optional[str] = None - collections_filter_path: Optional[str] = None + items_filter_path: str | Sequence[str] | None = None + collections_filter_path: str | Sequence[str] | None = None root_path: str = "" json_content_type_expr: str = r"application/(geo\+)?json" diff --git a/src/stac_auth_proxy/middleware/Cql2BuildFilterMiddleware.py b/src/stac_auth_proxy/middleware/Cql2BuildFilterMiddleware.py index e8b3186..8ba4985 100644 --- a/src/stac_auth_proxy/middleware/Cql2BuildFilterMiddleware.py +++ b/src/stac_auth_proxy/middleware/Cql2BuildFilterMiddleware.py @@ -3,7 +3,7 @@ import logging import re from dataclasses import dataclass -from typing import Any, Awaitable, Callable, Optional +from typing import Any, Awaitable, Callable, Optional, Sequence from cql2 import Expr, ValidationError from fastapi import HTTPException @@ -32,12 +32,26 @@ class Cql2BuildFilterMiddleware: # Filters collections_filter: Optional[Callable] = None - collections_filter_path: str = r"^/collections(/[^/]+)?$" + collections_filter_path: str | Sequence[str] = ( + r"^/collections(?:/(?P[^/]+))?$", + ) items_filter: Optional[Callable] = None - items_filter_path: str = r"^(/collections/([^/]+)/items(/[^/]+)?$|/search$)" + items_filter_path: str | Sequence[str] = ( + r"^(?:/collections/(?P[^/]+)/items(?:/(?P[^/]+))?|/search)$", + ) def __post_init__(self): """Set required conformances based on the filter functions.""" + for attr in ("collections_filter_path", "items_filter_path"): + object.__setattr__(self, attr, requests.as_patterns(getattr(self, attr))) + for pattern in getattr(self, attr): + if not re.compile(pattern).groupindex: + logger.info( + "Filter path %r declares no named capture groups, " + "falling back to built-in path param extraction.", + pattern, + ) + required_conformances = set() if self.collections_filter: logger.debug("Appending required conformance for collections filter") @@ -77,7 +91,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: logger.debug("Skipping CQL2 filter build for OPTIONS request") return await self.app(scope, receive, send) - filter_builder = self._get_filter(request.url.path) + filter_builder, path_params = self._get_filter(request.url.path) if not filter_builder: return await self.app(scope, receive, send) @@ -88,7 +102,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: "path": request.url.path, "method": request.method, "query_params": dict(request.query_params), - "path_params": requests.extract_variables(request.url.path), + "path_params": path_params, "headers": dict(request.headers), }, **scope["state"], @@ -112,13 +126,22 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: def _get_filter( self, path: str - ) -> Optional[Callable[..., Awaitable[str | dict[str, Any]]]]: - """Get the CQL2 filter builder for the given path.""" + ) -> tuple[Optional[Callable[..., Awaitable[str | dict[str, Any]]]], dict]: + """Get the CQL2 filter builder for the given path and its path params.""" endpoint_filters = [ (self.collections_filter_path, self.collections_filter), (self.items_filter_path, self.items_filter), ] - for expr, builder in endpoint_filters: - if re.match(expr, path): - return builder - return None + for patterns, builder in endpoint_filters: + for expr in patterns: + match = re.match(expr, path) + if match: + return builder, self._path_params(match, path) + return None, {} + + @staticmethod + def _path_params(match: re.Match, path: str) -> dict: + """Get the path params declared by a matched pattern's named groups.""" + if match.re.groupindex: + return {k: v for k, v in match.groupdict().items() if v is not None} + return requests.extract_variables(path) diff --git a/src/stac_auth_proxy/middleware/UpdateOpenApiMiddleware.py b/src/stac_auth_proxy/middleware/UpdateOpenApiMiddleware.py index 8f42955..e786197 100644 --- a/src/stac_auth_proxy/middleware/UpdateOpenApiMiddleware.py +++ b/src/stac_auth_proxy/middleware/UpdateOpenApiMiddleware.py @@ -2,7 +2,7 @@ import re from dataclasses import dataclass -from typing import Any, Optional +from typing import Any, Optional, Sequence from starlette.datastructures import Headers from starlette.requests import Request @@ -28,8 +28,8 @@ class OpenApiMiddleware(JsonResponseMiddleware): auth_scheme_name: str = "oidcAuth" auth_scheme_override: Optional[dict] = None - items_filter_path: Optional[str] = None - collections_filter_path: Optional[str] = None + items_filter_path: str | Sequence[str] | None = None + collections_filter_path: str | Sequence[str] | None = None json_content_type_expr: str = r"application/(vnd\.oai\.openapi\+json?|json)" diff --git a/src/stac_auth_proxy/utils/requests.py b/src/stac_auth_proxy/utils/requests.py index 190e263..2f536c4 100644 --- a/src/stac_auth_proxy/utils/requests.py +++ b/src/stac_auth_proxy/utils/requests.py @@ -26,6 +26,13 @@ def extract_variables(url: str) -> dict: return {k: v for k, v in match.groupdict().items() if v} if match else {} +def as_patterns(value: str | Sequence[str] | None) -> Sequence[str]: + """Normalize a filter path setting to a list of regex patterns.""" + if value is None: + return [] + return [value] if isinstance(value, str) else list(value) + + def dict_to_bytes(d: dict) -> bytes: """Convert a dictionary to a body.""" return json.dumps(d, separators=(",", ":")).encode("utf-8") @@ -56,8 +63,8 @@ def find_match( private_endpoints: EndpointMethods, public_endpoints: EndpointMethods, default_public: bool, - items_filter_path: Optional[str] = None, - collections_filter_path: Optional[str] = None, + items_filter_path: str | Sequence[str] | None = None, + collections_filter_path: str | Sequence[str] | None = None, ) -> "MatchResult": """Check if the given path and method match any of the regex patterns and methods in the endpoints.""" primary_endpoints = private_endpoints if default_public else public_endpoints @@ -70,7 +77,7 @@ def find_match( # If we have filter paths configured, check those as well (these are always considered to use auth if they match, regardless of default_public) for filter_path in [items_filter_path, collections_filter_path]: - if filter_path and re.match(filter_path, path): + if any(re.match(pattern, path) for pattern in as_patterns(filter_path)): return MatchResult(uses_auth=True) # If default_public and no match found in private_endpoints, it's public diff --git a/tests/test_config.py b/tests/test_config.py index f0ac533..ffa7b90 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -151,3 +151,50 @@ def test_cors_model_config(): ] assert cors_settings.allow_methods == ["GET", "POST"] assert cors_settings.allow_headers == ["Authorization", "Content-Type"] + + +def test_items_and_collections_path_parameters(): + """Tests related to parsing Collections/Items regexes from inputs.""" + common_kwargs = { + "upstream_url": "https://example.com", + "oidc_discovery_url": "https://example.com/.well-known/openid-configuration", + } + + # Single pattern case + settings = Settings( + **common_kwargs, + items_filter_path=r"^/collections/([^/]+)/items$", + ) + assert settings.items_filter_path == [r"^/collections/([^/]+)/items$"] + + # Don't split on commas (valid regex) + settings = Settings( + **common_kwargs, + items_filter_path=r"^/collections/([^/]{2,64})/items$", + ) + assert settings.items_filter_path == [r"^/collections/([^/]{2,64})/items$"] + + # JSON array decoded into list[str] + settings = Settings( + **common_kwargs, + collections_filter_path='["^/a$", "^/b$"]', + ) + assert settings.collections_filter_path == ["^/a$", "^/b$"] + + # Directly provided list[str] unaltered + custom_paths = [ + r"^/collections(?:/(?P[^/]+))?$", + r"^/collections/(?P[^/]+)/aggregate$", + ] + settings = Settings( + **common_kwargs, + collections_filter_path=custom_paths, + ) + assert settings.collections_filter_path == custom_paths + + # Reject invalid regex at load + with pytest.raises(ValueError, match="not a valid regular expression"): + settings = Settings( + **common_kwargs, + items_filter_path=r"^/collections(?:/(?P[^/]+))?$", + r"^/collections/(?P[^/]+)/aggregate$", + r"^/collections/(?P[^/]+)/aggregations$", +] + +AGGREGATION_ITEMS_FILTER_PATH = [ + r"^(?:/collections/(?P[^/]+)/items(?:/(?P[^/]+))?|/search)$", + r"^/aggregate$", + r"^/aggregations$", +] + + +def build_middleware(**kwargs) -> Cql2BuildFilterMiddleware: + """Build the filter middleware with a no-op filter.""" + kwargs.setdefault("collections_filter", lambda ctx: "true") + kwargs.setdefault("items_filter", lambda ctx: "true") + return Cql2BuildFilterMiddleware(app=None, **kwargs) + class TestOptionsRequest: """Test middleware behavior with OPTIONS requests.""" @@ -125,3 +146,105 @@ async def search_get(request: Request): # Test GET request SHOULD return 200 for good user get_response = client.get("/search", params={"user": "good"}) assert get_response.status_code == 200 + + +class TestFilterPathParams: + """Test extraction of path params from the configured filter paths.""" + + @pytest.mark.parametrize( + "path,expected", + [ + ("/collections/123", {"collection_id": "123"}), + ("/collections/123/items", {"collection_id": "123"}), + ("/collections/123/items/456", {"collection_id": "123", "item_id": "456"}), + ("/search", {}), + ], + ) + def test_default_patterns_reproduce_the_builtin_extraction(self, path, expected): + """The defaults yield exactly what the built-in extractor yielded.""" + mw = build_middleware() + _, path_params = mw._get_filter(path) + assert path_params == expected + + def test_a_pattern_extracts_its_own_named_groups(self): + """A custom endpoint gets whatever params its own pattern declares.""" + mw = build_middleware( + collections_filter_path=r"^/mosaic/(?P[^/]+)/(?P[^/]+)/(?P[^/]+)/(?P[^/]+)\.png$" + ) + filter_builder, path_params = mw._get_filter( + "/mosaic/8/12/34/my-collection.png" + ) + + assert filter_builder is not None + assert path_params == { + "zoom": "8", + "x": "12", + "y": "34", + "collection_id": "my-collection", + } + + @pytest.mark.parametrize( + "path", ["/collections/123/queryables", "/collections/123/bulk_items"] + ) + def test_defaults_route_no_filter_to_queryables_or_bulk_items(self, path): + """The defaults leave these paths alone, so no filter and no params.""" + mw = build_middleware() + assert mw._get_filter(path) == (None, {}) + + @pytest.mark.parametrize( + "path,expected", + [ + ("/collections/123/queryables", {"collection_id": "123"}), + ("/collections/123/bulk_items", {"collection_id": "123"}), + ], + ) + def test_routing_a_path_in_without_named_groups_uses_builtin_extraction( + self, path, expected + ): + """Widening the pattern is enough; the built-in extractor supplies the params.""" + mw = build_middleware( + collections_filter_path=r"^/collections/[^/]+/(queryables|bulk_items)$", + ) + assert mw._get_filter(path)[1] == expected + + def test_a_non_participating_alternative_does_not_leak_nulls(self): + """Groups in an unmatched alternative are dropped, not passed as None.""" + mw = build_middleware( + collections_filter_path=[r"^(?:/a/(?P\d+)|/b/(?P\d+))$"], + ) + assert mw._get_filter("/b/7")[1] == {"y": "7"} + + def test_a_pattern_without_named_groups_falls_back_to_builtin_extraction(self): + """A pre-existing config keeps the params it has always received.""" + mw = build_middleware(items_filter_path=r"^/collections/([^/]+)/items$") + assert mw._get_filter("/collections/abc/items")[1] == {"collection_id": "abc"} + + def test_a_string_is_normalized_when_constructed_directly(self): + """Middleware constructed with a string keeps working.""" + mw = build_middleware(items_filter_path=r"^/collections/([^/]+)/items$") + assert mw.items_filter_path == [r"^/collections/([^/]+)/items$"] + + @pytest.mark.parametrize( + "path,expected", + [ + ("/collections/123/aggregate", {"collection_id": "123"}), + ("/collections/123/aggregations", {"collection_id": "123"}), + ("/collections/123", {"collection_id": "123"}), + ("/collections/123/items", {"collection_id": "123"}), + ("/aggregate", {}), + ("/aggregations", {}), + ("/search", {}), + ], + ) + def test_the_documented_aggregation_config_covers_every_endpoint( + self, path, expected + ): + """The Aggregation extension example in the docs works as written.""" + mw = build_middleware( + collections_filter_path=AGGREGATION_COLLECTIONS_FILTER_PATH, + items_filter_path=AGGREGATION_ITEMS_FILTER_PATH, + ) + filter_builder, path_params = mw._get_filter(path) + + assert filter_builder is not None + assert path_params == expected diff --git a/tests/test_utils.py b/tests/test_utils.py index 1b43914..795a213 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -5,6 +5,7 @@ from stac_auth_proxy.utils.requests import ( extract_variables, + find_match, get_base_url, parse_forwarded_header, ) @@ -130,3 +131,27 @@ def test_get_base_url(headers, expected_url): result = get_base_url(request) assert result == expected_url + + +@pytest.mark.parametrize( + "collections_filter_path", + [ + r"^/collections/(?P[^/]+)/aggregations$", + [ + r"^/collections/(?P[^/]+)/aggregate$", + r"^/collections/(?P[^/]+)/aggregations$", + ], + ], + ids=["bare string", "sequence"], +) +def test_find_match_authenticates_filter_paths(collections_filter_path): + """Any path carrying a filter requires auth, whatever default_public says.""" + match = find_match( + "/collections/123/aggregations", + "GET", + private_endpoints={}, + public_endpoints={}, + default_public=True, + collections_filter_path=collections_filter_path, + ) + assert match.uses_auth is True