diff --git a/docs/principal-resolution.md b/docs/principal-resolution.md index a5879008d..90aad1e94 100644 --- a/docs/principal-resolution.md +++ b/docs/principal-resolution.md @@ -56,8 +56,21 @@ Returning `Principal` authenticates the request. Returning `None` applies the usual missing-bearer policy, including explicitly configured discovery and `allow_unauthenticated` behavior. Raising `PrincipalResolverError(401)` or `PrincipalResolverError(403)` rejects regardless of that opt-in. Responses contain -only generic `unauthenticated`/`forbidden` error bodies. A 401 carries the shared -Bearer challenge; 403 does not. Other resolver exceptions and invalid return +only generic `unauthenticated`/`forbidden` error bodies unless the resolver +supplies a stable code. A 401 carries the shared Bearer challenge; 403 does not. + +When your integrators branch on documented rejection codes, pass one as `code`: + +```python +raise PrincipalResolverError(403, code="identity_org_mismatch") +# -> 403 {"error": "identity_org_mismatch"} +``` + +`code` must match `^[a-z][a-z0-9_]{0,63}$`. It is a fixed identifier from your +documented set, never a message or a value derived from the request. A code that +doesn't match falls back to the generic body for that status, and the auth log +records only the coarse reason `invalid_code`, never the value. Exception text +is never sent. Without `code` the bodies are unchanged. Other resolver exceptions and invalid return values fail closed with generic 401; auth logs contain only coarse reason codes, never callback exception text or credential values. diff --git a/src/adcp/server/auth.py b/src/adcp/server/auth.py index a13a5c0db..c3bafeeeb 100644 --- a/src/adcp/server/auth.py +++ b/src/adcp/server/auth.py @@ -74,6 +74,7 @@ async def validate_token(token: str) -> Principal | None: import inspect import json import logging +import re import warnings from collections.abc import Awaitable, Callable, Collection, Mapping, Sequence from contextvars import ContextVar @@ -267,17 +268,24 @@ def __call__(self, request: AuthRequest) -> Awaitable[Principal | None]: ... """ +_RESOLVER_ERROR_CODE = re.compile(r"[a-z][a-z0-9_]{0,63}") + + class PrincipalResolverError(Exception): - """Intentional resolver rejection. Only the status is sent to the caller. + """Intentional resolver rejection. Only the status and code reach the caller. 401 means identity was not established; 403 means it is forbidden by the - resolver's policy. Responses use fixed generic bodies, never exception text. + resolver's policy. Responses never include exception text. The body is + ``{"error": code}`` when ``code`` is a stable machine-readable identifier + matching ``^[a-z][a-z0-9_]{0,63}$`` (for example ``identity_org_mismatch``), + otherwise the generic ``unauthenticated``/``forbidden`` body. """ - def __init__(self, status_code: Literal[401, 403] = 401) -> None: + def __init__(self, status_code: Literal[401, 403] = 401, *, code: str | None = None) -> None: if status_code not in (401, 403): raise ValueError("PrincipalResolverError status_code must be 401 or 403") self.status_code = status_code + self.code = code super().__init__("Principal resolution rejected") @@ -313,10 +321,17 @@ async def _resolve_request_principal( def _resolver_rejection(error: PrincipalResolverError) -> JSONResponse: + code = error.code + if code is not None and not ( + isinstance(code, str) and _RESOLVER_ERROR_CODE.fullmatch(code) is not None + ): + # Never echo a code that could carry free text; log no value either. + logger.warning("principal resolver code rejected", extra={"reason": "invalid_code"}) + code = None if error.status_code == 403: - return JSONResponse({"error": "forbidden"}, status_code=403) + return JSONResponse({"error": code or "forbidden"}, status_code=403) return JSONResponse( - {"error": "unauthenticated"}, + {"error": code or "unauthenticated"}, status_code=401, headers={"WWW-Authenticate": _WWW_AUTHENTICATE_CHALLENGE}, ) diff --git a/tests/test_auth_principal_resolver.py b/tests/test_auth_principal_resolver.py index 8e8c31901..36a190d23 100644 --- a/tests/test_auth_principal_resolver.py +++ b/tests/test_auth_principal_resolver.py @@ -268,6 +268,128 @@ def resolve(request: AuthRequest) -> Any: assert not errors +@pytest.mark.asyncio +@pytest.mark.parametrize("leg", ["mcp", "a2a"]) +@pytest.mark.parametrize("status", [401, 403]) +@pytest.mark.parametrize("code", ["identity_org_mismatch", "network_policy_denied", "x"]) +async def test_resolver_rejection_carries_a_stable_code( + leg: str, status: Any, code: str, caplog: pytest.LogCaptureFixture +) -> None: + def resolve(request: AuthRequest) -> Principal | None: + error = PrincipalResolverError(status, code=code) + error.args = ("private-token",) + raise error + + config = BearerTokenAuth(validate_token=lambda token: None, resolve_principal=resolve) + with caplog.at_level(logging.DEBUG, logger="adcp.server.auth"): + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=resolver_app(leg, config)), base_url="http://test" + ) as client: + response = await client.post("/", json=CALL) + assert response.status_code == status + assert response.json() == {"error": code} + assert ("www-authenticate" in response.headers) == (status == 401) + assert "private-token" not in caplog.text + response.text + + +@pytest.mark.asyncio +@pytest.mark.parametrize("leg", ["mcp", "a2a"]) +@pytest.mark.parametrize("status", [401, 403]) +@pytest.mark.parametrize( + "code", + [ + "", + "Identity_org_mismatch", + "1dentity", + "_identity", + "identity-org-mismatch", + "identity org mismatch", + "identity_org_mismatch\n", + "a" * 65, + "token=private-token", + 42, + ], +) +async def test_resolver_rejection_falls_back_on_an_unsafe_code( + leg: str, status: Any, code: Any, caplog: pytest.LogCaptureFixture +) -> None: + def resolve(request: AuthRequest) -> Principal | None: + raise PrincipalResolverError(status, code=code) + + config = BearerTokenAuth(validate_token=lambda token: None, resolve_principal=resolve) + with caplog.at_level(logging.DEBUG, logger="adcp.server.auth"): + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=resolver_app(leg, config)), base_url="http://test" + ) as client: + response = await client.post("/", json=CALL) + assert response.status_code == status + assert response.json() == {"error": "forbidden" if status == 403 else "unauthenticated"} + assert ("www-authenticate" in response.headers) == (status == 401) + rejected = [r for r in caplog.records if getattr(r, "reason", None) == "invalid_code"] + assert len(rejected) == 1 + assert rejected[0].args == () + if code not in ("", "a" * 65): + assert str(code) not in caplog.text + response.text + + +@pytest.mark.asyncio +async def test_combined_server_resolver_code_on_both_transports() -> None: + class Seller(ADCPHandler): + advertised_tools = {"get_products"} + + async def get_products(self, params: Any, context: Any = None) -> dict[str, Any]: + raise AssertionError("a rejected caller must not reach the handler") + + async def resolve(request: AuthRequest) -> Principal | None: + raise PrincipalResolverError(403, code="identity_org_mismatch") + + app = _build_mcp_and_a2a_app( + Seller(), + name="seller", + port=3001, + host="127.0.0.1", + instructions=None, + test_controller=None, + validation=None, + stateless_http=True, + enable_dns_rebinding_protection=False, + context_factory=auth_context_factory, + auth=BearerTokenAuth(validate_token=lambda token: None, resolve_principal=resolve), + ) + params = {"buying_mode": "brief", "brief": "video"} + mcp = { + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": {"name": "get_products", "arguments": params}, + } + a2a = { + "jsonrpc": "2.0", + "id": 1, + "method": "SendMessage", + "params": { + "message": { + "messageId": "m1", + "role": "ROLE_USER", + "parts": [{"data": {"skill": "get_products", "parameters": params}}], + } + }, + } + async with LifespanManager(app): + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=app), base_url="http://test", follow_redirects=True + ) as client: + for path, body in [("/mcp", mcp), ("/", a2a)]: + response = await client.post( + path, + json=body, + headers={"accept": "application/json, text/event-stream", "A2A-Version": "1.0"}, + ) + assert response.status_code == 403, response.text + assert response.json() == {"error": "identity_org_mismatch"} + assert "www-authenticate" not in response.headers + + @pytest.mark.asyncio @pytest.mark.parametrize("leg", ["mcp", "a2a"]) @pytest.mark.parametrize("allow", [False, True]) diff --git a/tests/type_checks/principal_resolver.py b/tests/type_checks/principal_resolver.py index d10b99283..60ce847a8 100644 --- a/tests/type_checks/principal_resolver.py +++ b/tests/type_checks/principal_resolver.py @@ -42,3 +42,17 @@ async def __call__(self, request: AuthRequest) -> Principal | None: unauthorized = PrincipalResolverError(401) forbidden = PrincipalResolverError(403) + +# A stable machine-readable code answers 403 {"error": "identity_org_mismatch"}. +coded = PrincipalResolverError(403, code="identity_org_mismatch") +coded_unauthenticated = PrincipalResolverError(401, code="identity_required") +code: str | None = coded.code + + +def resolve_with_code(request: AuthRequest) -> Principal | None: + if request.state.get("verified_proxy_org") != request.headers.get("x-org"): + raise PrincipalResolverError(403, code="identity_org_mismatch") + return None + + +BearerTokenAuth(validate_token=lambda token: None, resolve_principal=resolve_with_code)