diff --git a/common/auth/README.md b/common/auth/README.md index 27d24f61..099f625f 100644 --- a/common/auth/README.md +++ b/common/auth/README.md @@ -198,7 +198,9 @@ This section summarizes how the resource extractor functions in `veda_auth.resou | Path pattern | Method | Resource | Tenant source for resource ID | Resource ID returned (shape) | Notes | |------------------------------------|--------|---------------------------|--------------------------------------------------------|-----------------------------------------|-----------------------------------------------------------------------------------------| | `/collections` | `POST` | Create collection request | Request body field `eic:tenant` (or `TENANT_FIELD`), or public | `stac:collection:{tenant}:*` or public | Uses the same body-based extraction helper as the STAC collection write case. | -| `/collections/{collection_id}` | `DELETE` | Delete collection | _none_ (no tenant used) | `collection:{collection_id}` | Ingest delete uses an ID-scoped resource (`collection:{id}`) without tenant component. Tenant-aware deletes will be handled in Phase 2. | +| `/collections/{collection_id}` | `DELETE` | Delete collection | `collection_tenant_resolver`, else URL tenant, else public | `stac:collection:{tenant}:*` or public | Same Keycloak resource shape as STAC; Ingest PEP uses `INGEST_PROTECTED_ROUTES` (POST + DELETE). | + +The Ingest app passes `protected_routes=INGEST_PROTECTED_ROUTES` to `PEPMiddleware` so **POST** `/collections` and **DELETE** `/collections/{collection_id}` both invoke UMA (`extract_ingest_resource_id`). ### See Also diff --git a/common/auth/tests/test_pep_middleware.py b/common/auth/tests/test_pep_middleware.py index cd5998a7..13b387a7 100644 --- a/common/auth/tests/test_pep_middleware.py +++ b/common/auth/tests/test_pep_middleware.py @@ -5,6 +5,7 @@ import pytest from veda_auth.pep_middleware import ( DEFAULT_PROTECTED_ROUTES, + INGEST_PROTECTED_ROUTES, STAC_PROTECTED_ROUTES, PEPMiddleware, ) @@ -112,3 +113,32 @@ def test_search_no_match(self, middleware): _request("/api/stac/search", "POST") ) assert result is None + + +class TestIngestProtectedRoutes: + """INGEST_PROTECTED_ROUTES (ingest collection POST and DELETE endpoints)""" + + @pytest.fixture + def middleware(self): + """PEP middleware mock for ingest endpoints""" + app = MagicMock() + return PEPMiddleware( + app, + pdp_client=MagicMock(), + resource_extractor=MagicMock(), + protected_routes=INGEST_PROTECTED_ROUTES, + ) + + def test_post_collections_matches_create(self, middleware): + """POST /collections matches with scope create""" + result = middleware._get_matching_scope_and_route( + _request("/collections", "POST") + ) + assert result == ("create", "POST") + + def test_delete_collection_matches_delete_scope(self, middleware): + """DELETE /collections matches with scope delete""" + result = middleware._get_matching_scope_and_route( + _request("/collections/my-collection", "DELETE") + ) + assert result == ("delete", "DELETE") diff --git a/common/auth/tests/test_resource_extractors.py b/common/auth/tests/test_resource_extractors.py index a8422169..f47c9425 100644 --- a/common/auth/tests/test_resource_extractors.py +++ b/common/auth/tests/test_resource_extractors.py @@ -1,12 +1,14 @@ """Tests for resource extractors""" import json +from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock import pytest from veda_auth.resource_extractors import ( STAC_COLLECTION_PUBLIC, STAC_COLLECTION_TEMPLATE, + STAC_ITEM_PUBLIC, STAC_ITEM_TEMPLATE, _extract_collection_resource_id_from_post_body, _extract_tenant_from_body, @@ -174,6 +176,17 @@ async def test_get_item_with_tenant(self): result = await extract_stac_resource_id(request) assert result == STAC_ITEM_TEMPLATE.format("test-tenant") + @pytest.mark.asyncio + async def test_get_item_without_tenant_uses_public(self): + """Test extracting resource ID for GET item without tenant (defaults to public)""" + request = MagicMock(spec=Request) + request.url.path = "/collections/test-collection/items/test-item" + request.method = "GET" + request.state = MagicMock() + + result = await extract_stac_resource_id(request) + assert result == STAC_ITEM_TEMPLATE.format("public") + @pytest.mark.asyncio async def test_post_items_with_tenant(self): """Test extracting resource ID for POST items with tenant""" @@ -196,16 +209,177 @@ async def test_post_bulk_items_with_tenant(self): result = await extract_stac_resource_id(request) assert result == STAC_COLLECTION_TEMPLATE.format("test-tenant") + @pytest.mark.asyncio + async def test_item_paths_use_collection_tenant_resolver_when_available(self): + """Item endpoints should use collection_tenant_resolver when configured on app state""" + resolver = AsyncMock(return_value="resolver-tenant") + + def _build_request(path: str, method: str) -> Request: + request = MagicMock(spec=Request) + request.url.path = path + request.method = method + request.state = MagicMock() + app = MagicMock() + app.state.collection_tenant_resolver = resolver + request.app = app + return request + + item_request = _build_request( + "/collections/test-collection/items/test-item", "GET" + ) + item_result = await extract_stac_resource_id(item_request) + assert item_result == STAC_ITEM_TEMPLATE.format("resolver-tenant") + + items_request = _build_request("/collections/test-collection/items", "POST") + items_result = await extract_stac_resource_id(items_request) + assert items_result == STAC_ITEM_TEMPLATE.format("resolver-tenant") + + @pytest.mark.asyncio + async def test_collection_tenant_resolver_failure_falls_back_to_public(self): + """When collection_tenant_resolver fails, item requests fall back to public""" + resolver = AsyncMock(side_effect=Exception("resolver failed")) + + def _build_request(path: str, method: str) -> Request: + request = MagicMock(spec=Request) + request.url.path = path + request.method = method + request.state = MagicMock() + app = MagicMock() + app.state.collection_tenant_resolver = resolver + request.app = app + return request + + item_request = _build_request( + "/collections/test-collection/items/test-item", "GET" + ) + item_result = await extract_stac_resource_id(item_request) + assert item_result == STAC_ITEM_PUBLIC + + items_request = _build_request("/collections/test-collection/items", "POST") + items_result = await extract_stac_resource_id(items_request) + assert items_result == STAC_COLLECTION_PUBLIC + + delete_collection_request = _build_request( + "/collections/test-collection", "DELETE" + ) + delete_collection_result = await extract_stac_resource_id( + delete_collection_request + ) + assert delete_collection_result == STAC_COLLECTION_PUBLIC + + @pytest.mark.asyncio + async def test_delete_collection_uses_collection_tenant_resolver_when_available( + self, + ): + """DELETE /collections/{id} should use collection_tenant_resolver""" + resolver = AsyncMock(return_value="some-tenant") + request = MagicMock(spec=Request) + request.url.path = "/collections/test-collection" + request.method = "DELETE" + request.state = MagicMock() + app = MagicMock() + app.state.collection_tenant_resolver = resolver + request.app = app + + result = await extract_stac_resource_id(request) + assert result == STAC_COLLECTION_TEMPLATE.format("some-tenant") + resolver.assert_awaited_once_with(request, "test-collection") + + @pytest.mark.asyncio + async def test_delete_collection_resolver_none_falls_back_to_url_tenant(self): + """When resolver returns None, fall back to request.state.tenant if present""" + resolver = AsyncMock(return_value=None) + request = MagicMock(spec=Request) + request.url.path = "/collections/my-col" + request.method = "DELETE" + request.state = SimpleNamespace(tenant="url-tenant") + request.app = SimpleNamespace( + state=SimpleNamespace(collection_tenant_resolver=resolver) + ) + + result = await extract_stac_resource_id(request) + assert result == STAC_COLLECTION_TEMPLATE.format("url-tenant") + class TestExtractIngestResourceId: """Test Ingest API resource ID extraction""" - async def test_delete_collection_returns_collection_id(self): - """DELETE /collections/{id} should return collection-specific resource ID""" + async def test_delete_collection_falls_back_to_url_tenant_without_resolver(self): + """DELETE with no resolver uses request.state.tenant when tenant-prefixed path set it""" request = MagicMock(spec=Request) request.url.path = "/collections/test-collection" request.method = "DELETE" request.state.tenant = "test-tenant" + request.app = SimpleNamespace(state=SimpleNamespace()) resource_id = await extract_ingest_resource_id(request) - assert resource_id == "collection:test-collection" + assert resource_id == STAC_COLLECTION_TEMPLATE.format("test-tenant") + + async def test_delete_collection_without_resolver_or_url_tenant_is_public(self): + """DELETE with no resolver and no state.tenant uses stac:collection:public:*""" + request = MagicMock(spec=Request) + request.url.path = "/collections/foo" + request.method = "DELETE" + request.state = SimpleNamespace() + request.app = SimpleNamespace(state=SimpleNamespace()) + + assert await extract_ingest_resource_id(request) == STAC_COLLECTION_PUBLIC + + async def test_delete_collection_resolver_none_falls_back_to_url_tenant(self): + """When resolver returns None, use request.state.tenant if set (tenant-prefixed paths)""" + resolver = AsyncMock(return_value=None) + request = MagicMock(spec=Request) + request.url.path = "/collections/my-col" + request.method = "DELETE" + request.state = SimpleNamespace(tenant="url-tenant") + request.app = SimpleNamespace( + state=SimpleNamespace(collection_tenant_resolver=resolver) + ) + + assert await extract_ingest_resource_id( + request + ) == STAC_COLLECTION_TEMPLATE.format("url-tenant") + + async def test_delete_collection_uses_resolver_tenant(self): + """DELETE uses resolved tenant (from database lookup) for Keycloak resource ID""" + resolver = AsyncMock(return_value="veda") + request = MagicMock(spec=Request) + request.url.path = "/collections/foo" + request.method = "DELETE" + request.state = SimpleNamespace() + request.app = SimpleNamespace( + state=SimpleNamespace(collection_tenant_resolver=resolver) + ) + + rid = await extract_ingest_resource_id(request) + assert rid == STAC_COLLECTION_TEMPLATE.format("veda") + resolver.assert_awaited_once_with(request, "foo") + + async def test_delete_collection_resolver_raises_falls_back_to_public(self): + """Resolver failures fall back to _stac_collection_resource_id (public if no URL tenant)""" + resolver = AsyncMock(side_effect=RuntimeError("db unavailable")) + request = MagicMock(spec=Request) + request.url.path = "/collections/foo" + request.method = "DELETE" + request.state = SimpleNamespace() + request.app = SimpleNamespace( + state=SimpleNamespace(collection_tenant_resolver=resolver) + ) + + assert await extract_ingest_resource_id(request) == STAC_COLLECTION_PUBLIC + + async def test_delete_collection_magicmock_state_resolver_still_works(self): + """Resolver runs even when request.state is a MagicMock (no real tenant).""" + resolver = AsyncMock(return_value="tenant-a") + request = MagicMock(spec=Request) + request.url.path = "/collections/bar" + request.method = "DELETE" + request.state = MagicMock() + request.app = SimpleNamespace( + state=SimpleNamespace(collection_tenant_resolver=resolver) + ) + + assert await extract_ingest_resource_id( + request + ) == STAC_COLLECTION_TEMPLATE.format("tenant-a") + resolver.assert_awaited_once_with(request, "bar") diff --git a/common/auth/veda_auth/pep_middleware.py b/common/auth/veda_auth/pep_middleware.py index 1ed01d9b..edc54582 100644 --- a/common/auth/veda_auth/pep_middleware.py +++ b/common/auth/veda_auth/pep_middleware.py @@ -12,7 +12,10 @@ TokenError, ) from veda_auth.resource_extractors import ( + COLLECTIONS_BULK_ITEMS_PATH_RE, COLLECTIONS_CREATE_PATH_RE, + COLLECTIONS_ITEM_PATH_RE, + COLLECTIONS_ITEMS_PATH_RE, COLLECTIONS_PATH_RE, ) @@ -46,12 +49,26 @@ class ProtectedRoute: DEFAULT_PROTECTED_ROUTES: Sequence[ProtectedRoute] = (CREATE_COLLECTION_ROUTE,) +INGEST_PROTECTED_ROUTES: Sequence[ProtectedRoute] = ( + CREATE_COLLECTION_ROUTE, + ProtectedRoute(path_re=COLLECTIONS_PATH_RE, method="DELETE", scope="delete"), +) STAC_PROTECTED_ROUTES: Sequence[ProtectedRoute] = ( # Collections ProtectedRoute(path_re=COLLECTIONS_CREATE_PATH_RE, method="POST", scope="create"), ProtectedRoute(path_re=COLLECTIONS_PATH_RE, method="PUT", scope="update"), ProtectedRoute(path_re=COLLECTIONS_PATH_RE, method="PATCH", scope="update"), + ProtectedRoute(path_re=COLLECTIONS_PATH_RE, method="DELETE", scope="delete"), + # Items + ProtectedRoute(path_re=COLLECTIONS_ITEMS_PATH_RE, method="POST", scope="create"), + ProtectedRoute(path_re=COLLECTIONS_ITEM_PATH_RE, method="PUT", scope="update"), + ProtectedRoute(path_re=COLLECTIONS_ITEM_PATH_RE, method="PATCH", scope="update"), + ProtectedRoute(path_re=COLLECTIONS_ITEM_PATH_RE, method="DELETE", scope="delete"), + # Bulk items + ProtectedRoute( + path_re=COLLECTIONS_BULK_ITEMS_PATH_RE, method="POST", scope="create" + ), ) diff --git a/common/auth/veda_auth/resource_extractors.py b/common/auth/veda_auth/resource_extractors.py index 9901f713..d8e1eb44 100644 --- a/common/auth/veda_auth/resource_extractors.py +++ b/common/auth/veda_auth/resource_extractors.py @@ -10,7 +10,7 @@ import logging import os import re -from typing import Any, Dict, Optional +from typing import Any, Awaitable, Callable, Dict, Optional from fastapi import HTTPException, Request @@ -28,6 +28,8 @@ COLLECTIONS_ITEMS_PATH_RE = r".*?/collections/([^/]+)/items$" COLLECTIONS_BULK_ITEMS_PATH_RE = r".*?/collections/([^/]+)/bulk_items$" +CollectionTenantResolver = Callable[[Request, str], Awaitable[Optional[str]]] + _COLLECTIONS_CREATE_PATH_PATTERN = re.compile(COLLECTIONS_CREATE_PATH_RE) _COLLECTIONS_PATH_PATTERN = re.compile(COLLECTIONS_PATH_RE) _COLLECTIONS_ITEM_PATH_PATTERN = re.compile(COLLECTIONS_ITEM_PATH_RE) @@ -38,13 +40,57 @@ def _stac_collection_resource_id(request: Request) -> str: """Return tenant-based or public STAC collection resource ID.""" tenant = getattr(request.state, "tenant", None) - return STAC_COLLECTION_TEMPLATE.format(tenant) if tenant else STAC_COLLECTION_PUBLIC + if not isinstance(tenant, str) or not tenant: + return STAC_COLLECTION_PUBLIC + return STAC_COLLECTION_TEMPLATE.format(tenant) def _stac_item_resource_id(request: Request) -> str: """Return tenant-based or public STAC item resource ID.""" tenant = getattr(request.state, "tenant", None) - return STAC_ITEM_TEMPLATE.format(tenant) if tenant else STAC_ITEM_PUBLIC + if not isinstance(tenant, str) or not tenant: + return STAC_ITEM_PUBLIC + return STAC_ITEM_TEMPLATE.format(tenant) + + +def _get_collection_tenant_resolver( + request: Request, +) -> Optional[CollectionTenantResolver]: + """Return optional collection-tenant resolver from app state if configured""" + app = getattr(request, "app", None) + if app is None: + return None + state = getattr(app, "state", None) + return getattr(state, "collection_tenant_resolver", None) + + +async def _collection_tenant_for_item( + request: Request, collection_id: str +) -> Optional[str]: + """Resolve collection tenant for item operations""" + resolver = _get_collection_tenant_resolver(request) + if not resolver: + logger.debug( + "No collection_tenant_resolver configured on app.state for collection %s", + collection_id, + ) + return None + try: + tenant = await resolver(request, collection_id) + if not tenant: + logger.debug( + "collection_tenant_resolver returned no tenant for collection %s", + collection_id, + ) + return tenant + except Exception as e: + logger.warning( + "Failed to resolve collection tenant for item ops %s: %s", + collection_id, + e, + exc_info=True, + ) + return None def _extract_tenant_from_body( @@ -87,30 +133,70 @@ async def _extract_collection_resource_id_from_post_body( return None -async def extract_stac_resource_id(request: Request) -> Optional[str]: - """Extract resource ID for STAC API requests - Resource ID format matches Keycloak resource definitions (wildcard patterns): - - Collections: STAC_COLLECTION_TEMPLATE or STAC_COLLECTION_PUBLIC - - Items: STAC_ITEM_TEMPLATE or STAC_ITEM_PUBLIC - """ - path = request.url.path - method = request.method - +async def _extract_collection_stac_resource_id( + request: Request, path: str, method: str +) -> Optional[str]: + """Extract resource ID for collection endpoints, or None if not a collection path""" if _COLLECTIONS_CREATE_PATH_PATTERN.match(path) and method == "POST": return await _extract_collection_resource_id_from_post_body(request) - if _COLLECTIONS_PATH_PATTERN.match(path): + match = _COLLECTIONS_PATH_PATTERN.match(path) + if match: if method in ("PUT", "PATCH"): return await _extract_collection_resource_id_from_post_body(request) + if method == "DELETE": + collection_id = match.group(1) + tenant = await _collection_tenant_for_item(request, collection_id) + if tenant: + return STAC_COLLECTION_TEMPLATE.format(tenant) + return _stac_collection_resource_id(request) return _stac_collection_resource_id(request) + return None + + +async def _extract_item_stac_resource_id(request: Request, path: str) -> Optional[str]: + """Extract resource ID for item endpoints based on path, or None""" if _COLLECTIONS_ITEM_PATH_PATTERN.match(path): + # For single item operations, prefer collection tenant when available + match = _COLLECTIONS_ITEM_PATH_PATTERN.match(path) + collection_id = match.group(1) if match else None + if collection_id: + tenant = await _collection_tenant_for_item(request, collection_id) + if tenant: + return STAC_ITEM_TEMPLATE.format(tenant) return _stac_item_resource_id(request) - if _COLLECTIONS_ITEMS_PATH_PATTERN.match( - path - ) or _COLLECTIONS_BULK_ITEMS_PATH_PATTERN.match(path): - return _stac_collection_resource_id(request) + for pattern in ( + _COLLECTIONS_ITEMS_PATH_PATTERN, + _COLLECTIONS_BULK_ITEMS_PATH_PATTERN, + ): + if match := pattern.match(path): + tenant = await _collection_tenant_for_item(request, match.group(1)) + if tenant: + return STAC_ITEM_TEMPLATE.format(tenant) + return _stac_collection_resource_id(request) + + return None + + +async def extract_stac_resource_id(request: Request) -> Optional[str]: + """Extract resource ID for STAC API requests + + Resource ID format matches Keycloak resource definitions (wildcard patterns): + - Collections: STAC_COLLECTION_TEMPLATE or STAC_COLLECTION_PUBLIC + - Items: STAC_ITEM_TEMPLATE or STAC_ITEM_PUBLIC + """ + path = request.url.path + method = request.method + + collection_id = await _extract_collection_stac_resource_id(request, path, method) + if collection_id is not None: + return collection_id + + item_id = await _extract_item_stac_resource_id(request, path) + if item_id is not None: + return item_id if "/queryables" in path or "/search" in path: return None @@ -129,6 +215,23 @@ async def extract_ingest_resource_id(request: Request) -> Optional[str]: match = re.match(r".*?/collections/([^/]+)$", path) if match and method == "DELETE": collection_id = match.group(1) - return f"collection:{collection_id}" + tenant = await _collection_tenant_for_item(request, collection_id) + if tenant: + resource_id = STAC_COLLECTION_TEMPLATE.format(tenant) + logger.debug( + "Ingest DELETE /collections/%s: resolved tenant=%s -> %s", + collection_id, + tenant, + resource_id, + ) + return resource_id + fallback_resource_id = _stac_collection_resource_id(request) + logger.info( + "Ingest DELETE /collections/%s: falling back to resource_id=%s (resolver_none_or_missing), path=%s", + collection_id, + fallback_resource_id, + path, + ) + return fallback_resource_id return None diff --git a/ingest_api/infrastructure/config.py b/ingest_api/infrastructure/config.py index 4a29c676..ee45b073 100644 --- a/ingest_api/infrastructure/config.py +++ b/ingest_api/infrastructure/config.py @@ -95,6 +95,10 @@ class IngestorConfig(BaseSettings): None, description="Name or ARN of the AWS Secrets Manager secret containing Keycloak UMA resource server client_id and client_secret. Use a full ARN for cross-account access.", ) + tenant_filter_field: str = Field( + "eic:tenant", + description="Collection field name used for tenant ownership checks", + ) keycloak_secret_kms_key_arn: Optional[str] = Field( None, diff --git a/ingest_api/infrastructure/construct.py b/ingest_api/infrastructure/construct.py index 5b5562bb..dc06da83 100644 --- a/ingest_api/infrastructure/construct.py +++ b/ingest_api/infrastructure/construct.py @@ -46,6 +46,7 @@ def __init__( "CLIENT_ID": config.keycloak_ingest_api_client_id, "OPENID_CONFIGURATION_URL": str(config.openid_configuration_url), "GIT_SHA": config.git_sha, + "VEDA_TENANT_FILTER_FIELD": config.tenant_filter_field, } if config.keycloak_uma_resource_server_client_secret_name: @@ -261,6 +262,7 @@ def __init__( "CLIENT_ID": config.keycloak_ingest_api_client_id, "OPENID_CONFIGURATION_URL": str(config.openid_configuration_url), "GIT_SHA": config.git_sha, + "VEDA_TENANT_FILTER_FIELD": config.tenant_filter_field, } if config.keycloak_uma_resource_server_client_secret_name: diff --git a/ingest_api/runtime/src/collection_publisher.py b/ingest_api/runtime/src/collection_publisher.py index e8ac7ae0..8c9e1c3b 100644 --- a/ingest_api/runtime/src/collection_publisher.py +++ b/ingest_api/runtime/src/collection_publisher.py @@ -1,11 +1,16 @@ +import logging import os +from typing import Optional from pypgstac.db import PgstacDB +from src.config import settings from src.schemas import DashboardCollection from src.utils import IngestionType, get_db_credentials, load_into_pgstac from src.vedaloader import VEDALoader from stac_pydantic import Item +logger = logging.getLogger(__name__) + class CollectionPublisher: def ingest(self, collection: DashboardCollection): @@ -30,6 +35,54 @@ def delete(self, collection_id: str): loader = VEDALoader(db=db) loader.delete_collection(collection_id) + def get_collection_tenant(self, collection_id: str) -> Optional[str]: + """Return tenant field from collection JSON in PgSTAC, or None if not found""" + tenant_field = settings.tenant_filter_field + creds = get_db_credentials(os.environ["DB_SECRET_ARN"]) + try: + with PgstacDB(dsn=creds.dsn_string, debug=True) as db: + collection_content = db.query_one( + "SELECT content FROM collections WHERE id=%s", + (collection_id,), + ) + except Exception: + logger.warning( + "Could not load collection %s for tenant lookup (tenant_field=%s)", + collection_id, + tenant_field, + ) + return None + content_dict = collection_content + if not isinstance(content_dict, dict): + logger.info( + "Collection %s content payload is not a dict during tenant lookup", + collection_id, + ) + return None + + logger.info( + "collection_content tenant_field_present for %s: tenant_field_present=%s", + collection_id, + tenant_field in content_dict, + ) + + tenant_value = content_dict.get(tenant_field) + if tenant_value: + logger.info( + "Resolved tenant for collection %s: tenant_field=%s tenant=%s", + collection_id, + tenant_field, + tenant_value, + ) + return str(tenant_value) + + logger.info( + "Collection %s has no value for tenant_field=%s in content", + collection_id, + tenant_field, + ) + return None + class ItemPublisher: def ingest(self, item: Item): diff --git a/ingest_api/runtime/src/config.py b/ingest_api/runtime/src/config.py index f6a59888..a30c4796 100644 --- a/ingest_api/runtime/src/config.py +++ b/ingest_api/runtime/src/config.py @@ -30,6 +30,11 @@ class Settings(BaseSettings): None, description="Name or ARN of the AWS Secrets Manager secret containing Keycloak UMA resource server client_id and client_secret. Use a full ARN for cross-account access.", ) + tenant_filter_field: str = Field( + "eic:tenant", + validation_alias="VEDA_TENANT_FILTER_FIELD", + description="Collection field name used to resolve tenant ownership.", + ) settings = Settings() diff --git a/ingest_api/runtime/src/main.py b/ingest_api/runtime/src/main.py index 9ec6f1da..2fc2bad2 100644 --- a/ingest_api/runtime/src/main.py +++ b/ingest_api/runtime/src/main.py @@ -1,4 +1,5 @@ import logging +from typing import Optional import src.dependencies as dependencies import src.schemas as schemas @@ -11,7 +12,7 @@ from src.monitoring import ObservabilityMiddleware, logger, metrics, tracer from src.utils import get_keycloak_client_credentials from veda_auth.keycloak_client import KeycloakPDPClient, parse_keycloak_from_openid_url -from veda_auth.pep_middleware import PEPMiddleware +from veda_auth.pep_middleware import INGEST_PROTECTED_ROUTES, PEPMiddleware from veda_auth.resource_extractors import extract_ingest_resource_id from fastapi import Depends, FastAPI, HTTPException, Security @@ -44,6 +45,16 @@ item_publisher = ItemPublisher() +async def collection_tenant_resolver( + _request: Request, collection_id: str +) -> Optional[str]: + """Resolve tenant from the collection record in PgSTAC""" + return collection_publisher.get_collection_tenant(collection_id) + + +app.state.collection_tenant_resolver = collection_tenant_resolver + + @app.get( "/ingestions", response_model=schemas.ListIngestionResponse, tags=["Ingestion"] ) @@ -296,6 +307,7 @@ def _get_keycloak_pdp_client() -> KeycloakPDPClient: PEPMiddleware, pdp_client=_get_keycloak_pdp_client, resource_extractor=extract_ingest_resource_id, + protected_routes=INGEST_PROTECTED_ROUTES, ) else: pep_logger.info( diff --git a/ingest_api/runtime/tests/test_pep_integration.py b/ingest_api/runtime/tests/test_pep_integration.py index daf69020..98146c7c 100644 --- a/ingest_api/runtime/tests/test_pep_integration.py +++ b/ingest_api/runtime/tests/test_pep_integration.py @@ -198,3 +198,51 @@ def test_post_collection_nonexistent_tenant_returns_404( assert response.status_code == 404 assert "does not exist" in response.json()["detail"] assert "nonexistent-tenant" in response.json()["detail"] + + def test_delete_collection_no_token_returns_401(self, pep_client): + """DELETE /collections/{id} without Bearer is rejected""" + response = pep_client.delete(f"{COLLECTIONS_ENDPOINT}/any-id") + assert response.status_code == 401 + + def test_delete_collection_pep_uses_tenant_resource( + self, pep_client, mock_pdp_client + ): + """DELETE checks PDP for stac:collection:{tenant}:* when collection has a tenant""" + mock_pdp_client.check_permission.return_value = True + collection_id = "pep-delete-tenant-test" + + with patch( + "src.main.collection_publisher.get_collection_tenant", + return_value="veda", + ), patch("src.main.collection_publisher.delete"): + response = pep_client.delete( + f"{COLLECTIONS_ENDPOINT}/{collection_id}", + headers={"Authorization": "Bearer fake-valid-token"}, + ) + + assert response.status_code == 200 + mock_pdp_client.check_permission.assert_called_once() + kwargs = mock_pdp_client.check_permission.call_args.kwargs + assert kwargs.get("resource_id") == "stac:collection:veda:*" + assert kwargs.get("scope") == "delete" + + def test_delete_collection_pep_denied_returns_403( + self, pep_client, mock_pdp_client + ): + """DELETE denied by PDP returns 403""" + mock_pdp_client.check_permission.side_effect = PermissionDeniedError( + resource_id="stac:collection:veda:*", scope="delete" + ) + collection_id = "pep-delete-denied" + + with patch( + "src.main.collection_publisher.get_collection_tenant", + return_value="veda", + ), patch("src.main.collection_publisher.delete"): + response = pep_client.delete( + f"{COLLECTIONS_ENDPOINT}/{collection_id}", + headers={"Authorization": "Bearer fake-valid-token"}, + ) + + assert response.status_code == 403 + assert "do not have permission" in response.json()["detail"] diff --git a/stac_api/runtime/setup.py b/stac_api/runtime/setup.py index e330b299..7dbd4190 100644 --- a/stac_api/runtime/setup.py +++ b/stac_api/runtime/setup.py @@ -35,7 +35,7 @@ description="", python_requires=">=3.12", packages=find_namespace_packages(exclude=["tests*"]), - package_data={"veda": ["stac/templates/*.html"]}, + package_data={"src": ["templates/*.html"]}, include_package_data=True, zip_safe=False, install_requires=inst_reqs, diff --git a/stac_api/runtime/src/app.py b/stac_api/runtime/src/app.py index 26b48ebc..112511f4 100644 --- a/stac_api/runtime/src/app.py +++ b/stac_api/runtime/src/app.py @@ -3,6 +3,7 @@ """ from contextlib import asynccontextmanager +from typing import Optional from aws_lambda_powertools.metrics import MetricUnit from src.config import ( @@ -95,6 +96,25 @@ async def lifespan(app: FastAPI): ], ) + +async def collection_tenant_resolver( + request: Request, collection_id: str +) -> Optional[str]: + """Resolve a collection's tenant from the database for PEP""" + try: + from stac_fastapi.types.errors import NotFoundError + + collection = await api.client.get_collection(collection_id, request=request) + tenant_field = api_settings.tenant_filter_field + return collection.get(tenant_field) or None + except NotFoundError: + return None + except Exception: + return None + + +api.app.state.collection_tenant_resolver = collection_tenant_resolver + if api_settings.openid_configuration_url and api_settings.enable_stac_auth_proxy: # Use stac-auth-proxy when authentication is enabled, which it will be for production envs app = configure_app( @@ -143,6 +163,8 @@ async def lifespan(app: FastAPI): # Use standard FastAPI app when authentication is disabled app = api.app +app.state.collection_tenant_resolver = api.app.state.collection_tenant_resolver + def _get_keycloak_pdp_client(): """Build Keycloak PDP client for PEP from UMA resource server credentials stored in AWS Secrets Manager.""" @@ -221,11 +243,10 @@ def _get_keycloak_pdp_client(): @app.get("/index.html", response_class=HTMLResponse) async def viewer_page(request: Request): """Search viewer.""" - path = api_settings.root_path or "" return templates.TemplateResponse( request, "stac-viewer.html", - {"endpoint": str(request.url).replace("/index.html", path)}, + {"endpoint": str(request.url).replace("/index.html", "")}, media_type="text/html", ) diff --git a/stac_api/runtime/tests/test_pep_integration.py b/stac_api/runtime/tests/test_pep_integration.py index 9e1d0a6b..02860a15 100644 --- a/stac_api/runtime/tests/test_pep_integration.py +++ b/stac_api/runtime/tests/test_pep_integration.py @@ -121,7 +121,7 @@ def _item(collection_id: str, item_id: Optional[str] = None) -> dict: class TestPEPIntegration: - """Integration tests for PEP middleware for POST /collections endpoint""" + """Integration tests for PEP middleware for STAC collection and item endpoints""" @pytest.mark.asyncio async def test_post_collection_no_token_returns_401(self, pep_client): @@ -155,6 +155,45 @@ async def test_post_collection_authorized_succeeds( headers={"Authorization": "Bearer fake-valid-token"}, ) + @pytest.mark.asyncio + async def test_post_item_with_tenant_uses_item_resource( + self, pep_client, mock_pdp_client + ): + """POST /collections/{collection_id}/items with tenant should use item resource ID""" + mock_pdp_client.check_permission.return_value = True + collection = _collection(tenant="veda") + + # Create a collection with tenant + create_collection_response = await pep_client.post( + COLLECTIONS_ENDPOINT, + json=collection, + headers={"Authorization": "Bearer fake-valid-token"}, + ) + assert create_collection_response.status_code == 201 + + item = _item(collection["id"]) + + response = await pep_client.post( + f"{COLLECTIONS_ENDPOINT}/{collection['id']}/items", + json=item, + headers={"Authorization": "Bearer fake-valid-token"}, + ) + assert response.status_code in (200, 201) + + calls = mock_pdp_client.check_permission.call_args_list + resource_ids = [c.kwargs.get("resource_id") for c in calls] + assert "stac:item:veda:*" in resource_ids + + # Cleanup + await pep_client.delete( + f"{COLLECTIONS_ENDPOINT}/{collection['id']}/items/{item['id']}", + headers={"Authorization": "Bearer fake-valid-token"}, + ) + await pep_client.delete( + f"{COLLECTIONS_ENDPOINT}/{collection['id']}", + headers={"Authorization": "Bearer fake-valid-token"}, + ) + @pytest.mark.asyncio async def test_post_collection_denied_returns_403( self, pep_client, mock_pdp_client @@ -354,3 +393,27 @@ async def test_patch_collection_authorized_succeeds( await pep_client.delete( f"{COLLECTIONS_ENDPOINT}/{collection['id']}", headers=AUTH_HEADERS ) + + @pytest.mark.asyncio + async def test_delete_collection_uses_resolved_tenant_for_pep_resource_id( + self, pep_client, mock_pdp_client + ): + """DELETE /collections/{id} passes tenant from collection JSON to PDP""" + mock_pdp_client.check_permission.return_value = True + collection = _collection(tenant="veda") + await pep_client.post( + COLLECTIONS_ENDPOINT, + json=collection, + headers=AUTH_HEADERS, + ) + mock_pdp_client.reset_mock() + + response = await pep_client.delete( + f"{COLLECTIONS_ENDPOINT}/{collection['id']}", + headers=AUTH_HEADERS, + ) + assert response.status_code in (200, 204) + mock_pdp_client.check_permission.assert_called_once() + call_kwargs = mock_pdp_client.check_permission.call_args.kwargs + assert call_kwargs.get("resource_id") == "stac:collection:veda:*" + assert call_kwargs.get("scope") == "delete"