From 09c92a191d363b82013a8d0e7601144bb54c994d Mon Sep 17 00:00:00 2001 From: muralx Date: Mon, 15 Jun 2026 12:00:24 +0100 Subject: [PATCH] feat: enforce inbound DPoP in MCP adapters, bound TokenCache, add require_scopes Adapters (authplane-mcp, authplane-fastmcp): - Forward a DPoPRequestContext from verify_token to AuthplaneResource.verify so inbound_dpop=InboundDPoPOptions(required=True) enforces the proof check end-to-end. htu origin is always the operator-configured resource URI, never the inbound Host / X-Forwarded-Proto headers. - Reconstruct htu from scope["raw_path"] to preserve percent-encoding (e.g. %2F) on the wire under ASGI, falling back to request.url.path. - authplane-mcp adds AuthplaneRequestContextMiddleware, get_current_request(), and install_request_context(mcp) (idempotent) to publish the active request on a ContextVar so the verifier can build a DPoPRequestContext. - Cache the in-flight verify task per request on request.state so a repeat verify_token within the same request reuses it instead of re-entering the inbound DPoP replay store. Cross-request replay protection is unaffected. Core (authplane): - Bound TokenCache with a configurable max_entries cap (default 10_000, exposed as TokenCache.DEFAULT_MAX_ENTRIES) and LRU eviction; plumbed through AuthplaneClient.create(cache_max_entries=...). - Add VerifiedClaims.require_scopes(scopes) plural AND-style scope helper. Empty input is a no-op; raises InsufficientScopeError listing all missing scopes and the token's available scopes on failure. - Export TokenCache from authplane.__init__. - require_scope (singular) now renders an empty scope set as (none) instead of [], consistent with the plural helper. Docs and demos run adapter setup, the async server entry point, and aclose() in a single asyncio.run(main()), keeping the client's locks, HTTP pool, and background refresh tasks on one event loop. --- CHANGELOG.md | 13 + authplane-fastmcp/authplane_fastmcp/auth.py | 7 + .../authplane_fastmcp/verifier.py | 162 +++++- authplane-fastmcp/demo/mcpserver.py | 44 +- authplane-fastmcp/tests/conftest.py | 6 +- authplane-fastmcp/tests/test_auth_factory.py | 36 +- authplane-fastmcp/tests/test_verifier.py | 3 +- .../tests/test_verifier_dpop_cache.py | 496 ++++++++++++++++++ authplane-mcp/authplane_mcp/__init__.py | 6 +- .../authplane_mcp/_request_context.py | 91 ++++ authplane-mcp/authplane_mcp/auth.py | 84 +++ authplane-mcp/authplane_mcp/verifier.py | 187 ++++++- authplane-mcp/demo/mcpserver.py | 84 +-- authplane-mcp/docs/user-guide.md | 14 +- authplane-mcp/tests/test_auth.py | 24 +- authplane-mcp/tests/test_request_context.py | 201 +++++++ authplane-mcp/tests/test_verifier.py | 35 +- .../tests/test_verifier_dpop_cache.py | 492 +++++++++++++++++ authplane/__init__.py | 2 + authplane/_dpop_adapter.py | 153 ++++++ authplane/cache.py | 65 ++- authplane/client.py | 40 +- authplane/verifier/claims.py | 38 +- llm.txt | 43 +- tests/test_cache.py | 117 +++++ tests/verifier/test_claims.py | 115 ++++ 26 files changed, 2409 insertions(+), 149 deletions(-) create mode 100644 authplane-fastmcp/tests/test_verifier_dpop_cache.py create mode 100644 authplane-mcp/authplane_mcp/_request_context.py create mode 100644 authplane-mcp/tests/test_request_context.py create mode 100644 authplane-mcp/tests/test_verifier_dpop_cache.py create mode 100644 authplane/_dpop_adapter.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ec5ff9..1df53bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- `TokenCache` is now bounded by a configurable `max_entries` cap (default `10_000`, exposed as `TokenCache.DEFAULT_MAX_ENTRIES` and a read-only `cache.max_entries` property) and evicts the least-recently-used entry on overflow; both `get` and `set` bump the touched key to MRU. Plumbed through `AuthplaneClient.create(cache_max_entries=...)`. Token-exchange cache keys are high-cardinality (the subject token is part of the key), so the cap keeps long-lived clients bounded. +- `VerifiedClaims.require_scopes(scopes: Iterable[str])` — plural AND-style helper that requires all listed scopes. Empty input is a no-op; on failure the raised `InsufficientScopeError` carries the full requested tuple on `required_scopes` and names every missing scope plus the token's available scopes in the message. +- `authplane-mcp`: new public surface — `AuthplaneRequestContextMiddleware`, `get_current_request()`, `install_request_context(mcp)` — an ASGI middleware that publishes the active request on a `ContextVar` so the verifier can build a `DPoPRequestContext`. +- `authplane-fastmcp`, `authplane-mcp`: `AuthplaneTokenVerifier` caches the in-flight verify task per request (keyed by access token on `request.state`), so a repeat `verify_token` within the same HTTP request awaits the same task rather than re-entering the inbound DPoP replay store. Cross-request replay protection is unaffected (distinct requests get distinct caches). + +### Fixed +- `authplane-fastmcp`, `authplane-mcp`: inbound DPoP proof-of-possession is now enforced end-to-end. `AuthplaneTokenVerifier.verify_token` forwards a `DPoPRequestContext` (method + reconstructed `htu` + proof header) to `AuthplaneResource.verify`, so `inbound_dpop=InboundDPoPOptions(required=True)` checks the proof on every request. The `htu` origin is always the operator-configured resource URI, never the inbound `Host` / `X-Forwarded-Proto` headers. Operators using `required=True` with `authplane-mcp` should call `install_request_context(mcp)` after constructing `FastMCP` so the verifier can read the per-request context; if it is not installed the request fails closed (401) rather than skipping the check. +- `authplane-fastmcp`, `authplane-mcp`: DPoP `htu` reconstruction reads `scope["raw_path"]` to preserve percent-encoding (e.g. `%2F`) on the wire under ASGI, falling back to `request.url.path` when the server omits `raw_path`. +- `authplane-mcp`: `install_request_context(mcp)` is idempotent — repeated calls on the same `FastMCP` instance are no-ops. +- `require_scope` (singular) now renders an empty token scope set as `(none)` instead of `[]`, matching the plural helper's output. Logging pipelines keyed on the old `Token has scopes: []` string should be updated. +- Docs and demos now run adapter setup, the async server entry point (`run_streamable_http_async` / `run_async`), and `aclose()` in a single `asyncio.run(main())`, keeping the client's locks, HTTP pool, and background JWKS/metadata refresh tasks on one event loop. + ## [0.2.0] - 2026-05-20 ### Security diff --git a/authplane-fastmcp/authplane_fastmcp/auth.py b/authplane-fastmcp/authplane_fastmcp/auth.py index 16e31a8..ecf4fdd 100644 --- a/authplane-fastmcp/authplane_fastmcp/auth.py +++ b/authplane-fastmcp/authplane_fastmcp/auth.py @@ -112,6 +112,7 @@ async def authplane_auth( metadata_refresh_seconds: int | None = None, cache_ttl_buffer_seconds: float | None = None, default_ttl_seconds: float | None = None, + cache_max_entries: int | None = None, circuit_breaker_threshold: int | None = None, circuit_breaker_cooldown_seconds: float | None = None, clock_skew_seconds: int | None = None, @@ -158,6 +159,11 @@ async def authplane_auth( before cache expiry (default ``30.0``). default_ttl_seconds: Fallback token cache TTL used when token responses do not include expiry metadata (default ``3600.0``). + cache_max_entries: Maximum number of tokens kept in the LRU cache + before the least-recently-used entry is evicted (default + :attr:`TokenCache.DEFAULT_MAX_ENTRIES` = 10 000). Token-exchange + keys are high-cardinality; raise this cap for long-lived servers + with many distinct subjects. circuit_breaker_threshold: Number of transient failures before opening the AS circuit breaker (default ``5``). circuit_breaker_cooldown_seconds: Cooldown before allowing a @@ -228,6 +234,7 @@ async def authplane_auth( "metadata_refresh_seconds": metadata_refresh_seconds, "cache_ttl_buffer_seconds": cache_ttl_buffer_seconds, "default_ttl_seconds": default_ttl_seconds, + "cache_max_entries": cache_max_entries, "circuit_breaker_threshold": circuit_breaker_threshold, "circuit_breaker_cooldown_seconds": circuit_breaker_cooldown_seconds, } diff --git a/authplane-fastmcp/authplane_fastmcp/verifier.py b/authplane-fastmcp/authplane_fastmcp/verifier.py index e4dc54a..25a86ac 100644 --- a/authplane-fastmcp/authplane_fastmcp/verifier.py +++ b/authplane-fastmcp/authplane_fastmcp/verifier.py @@ -5,15 +5,29 @@ to FastMCP's ``AccessToken`` with the full JWT payload in ``claims``. """ +import asyncio import logging +from collections.abc import Callable from typing import Any, cast - -from authplane import AuthplaneError, AuthplaneResource +from urllib.parse import urlsplit + +from authplane import AuthplaneError, AuthplaneResource, DPoPRequestContext +from authplane._dpop_adapter import ( + BuiltDPoPRequestContext, + get_or_create_verify_cache, + raw_request_path, + read_dpop_header, +) from fastmcp.server.auth import AccessToken, TokenVerifier +from fastmcp.server.dependencies import get_http_request as _default_get_http_request +from starlette.requests import Request logger = logging.getLogger(__name__) +__all__ = ["AuthplaneTokenVerifier"] + + class AuthplaneTokenVerifier(TokenVerifier): """FastMCP TokenVerifier backed by AuthplaneResource. @@ -23,9 +37,39 @@ class AuthplaneTokenVerifier(TokenVerifier): tool handlers via FastMCP's native ``CurrentAccessToken()`` dependency or ``get_access_token()`` function. - All security-critical logic (signature verification, claim validation, - JWKS caching, SSRF protection) is handled by the core SDK. This class - is a thin adapter that maps between the two interfaces. + DPoP (RFC 9449) + --------------- + + When the underlying :class:`AuthplaneResource` was created with + ``inbound_dpop=InboundDPoPOptions(...)``, this verifier pulls the + active HTTP request via + :func:`fastmcp.server.dependencies.get_http_request`, builds a + :class:`~authplane.DPoPRequestContext` (method + reconstructed + ``htu`` + proof header), and forwards it to + :meth:`AuthplaneResource.verify`. The ``htu`` origin + (scheme + host + port) is taken from the operator-configured + resource URI, never from the inbound ``Host`` / + ``X-Forwarded-Proto`` headers — letting an upstream decide which + ``htu`` the proof is checked against would neuter DPoP's + cross-endpoint anti-replay. Only the path varies per call. + + Per-request verify cache + ------------------------ + + FastMCP's standard HTTP stack invokes ``verify_token`` exactly once + per request (Starlette ``AuthenticationMiddleware`` → + ``BearerAuthBackend`` → ``TokenVerifier.verify_token``). The first + call's in-flight verify task is stashed on ``request.state`` keyed by + the access token; any subsequent invocation within the same request + awaits the same task instead of re-entering the inbound DPoP replay + store. The cache is defensive: it mirrors the TS adapter's + ``AsyncLocalStorage`` pattern and pre-empts a class of regressions + where a future framework change (transport rewrite, custom auth + provider, ASGI wrapper) would silently double-call ``verify_token`` + and the second call's proof would be rejected as + ``DPoPReplayDetected``. Different requests get distinct + ``request.state`` objects so cross-request replay protection is + preserved. Scope enforcement is FastMCP's responsibility via ``@mcp.tool(auth=require_scopes(...))``. @@ -36,6 +80,8 @@ def __init__( verifier: AuthplaneResource, base_url: str | None = None, required_scopes: list[str] | None = None, + *, + get_http_request: Callable[[], Request] | None = None, ) -> None: """Initialize the token verifier. @@ -47,9 +93,26 @@ def __init__( ``TokenVerifier`` for PRM generation. required_scopes: Scopes required for all requests. Passed to the parent ``TokenVerifier``. + get_http_request: Override for the active-request lookup + (defaults to + ``fastmcp.server.dependencies.get_http_request``). Tests + inject a fake to drive the DPoP / per-request-cache + paths without spinning up an ASGI app. """ super().__init__(base_url=base_url, required_scopes=required_scopes) self._verifier = verifier + self._get_http_request = get_http_request or _default_get_http_request + + # ``AuthplaneResource.resource`` is operator-configured and must be a + # string URI — guard against mis-wired mocks (a bare ``MagicMock`` with + # no ``resource`` set silently produces ``MagicMock://MagicMock`` here + # and would corrupt ``htu`` reconstruction in production). + if not isinstance(verifier.resource, str): + raise TypeError( + f"verifier.resource must be a str URI, got {type(verifier.resource).__name__}" + ) + split = urlsplit(verifier.resource) + self._resource_origin = f"{split.scheme}://{split.netloc}" @property def verifier(self) -> AuthplaneResource: @@ -67,25 +130,57 @@ def scopes_supported(self) -> list[str]: return list(self._verifier.scopes) async def verify_token(self, token: str) -> AccessToken | None: - """Validate a JWT and return a FastMCP AccessToken. + """Validate a JWT and return a FastMCP ``AccessToken``. - Called by FastMCP once per authenticated request. Delegates to - ``AuthplaneResource.verify()`` for all validation, then maps the - resulting ``VerifiedClaims`` to a FastMCP ``AccessToken`` with the - full JWT payload in ``claims``. + Pulls the active HTTP request to build a per-request + :class:`DPoPRequestContext` (RFC 9449 §7.1) and to scope the + verify-task cache. The cache hit path re-awaits the original + :class:`asyncio.Task`, so a cached :class:`AuthplaneError` + re-raises with the same type on every call within the request — + the ``AuthplaneError → None`` translation happens once here, not + inside the cached coroutine, which keeps the cached failure + diagnosable. Args: - token: The raw JWT string (FastMCP strips ``'Bearer '`` - before calling this method). + token: The raw JWT string (FastMCP strips the ``Bearer `` + prefix before calling this method). Returns: - ``AccessToken`` on successful validation with ``token``, - ``client_id``, ``scopes``, ``expires_at``, and ``claims`` - (full JWT payload dict) fields populated. Returns ``None`` - on any validation failure (FastMCP responds with 401). + ``AccessToken`` on successful validation. ``None`` on any + ``AuthplaneError`` (FastMCP responds with 401). """ try: - claims = await self._verifier.verify(token) + request: Request | None = self._get_http_request() + except RuntimeError as exc: + # ``fastmcp.server.dependencies.get_http_request`` raises a bare + # ``RuntimeError("No active HTTP request found.")`` when called + # outside an HTTP request context (unit tests, background tasks + # with no snapshotted request). Match the message to avoid + # silently degrading to ``dpop_request=None`` if a future + # FastMCP release surfaces an unrelated ``RuntimeError`` from + # this dependency — that would re-introduce the silent-pass + # this PR fixed (PRM advertising DPoP-required while the + # verifier never sees a proof). Drop this narrow when the + # upstream public surface adopts a typed exception. + if "No active HTTP request" not in str(exc): + raise + request = None + + try: + if request is None: + claims = await self._verifier.verify(token, dpop_request=None) + else: + cache = get_or_create_verify_cache(request) + task = cache.get(token) + if task is None: + # No await between cache miss and cache write — concurrent + # verify_token(token) calls on the same loop cannot race. + dpop_request = self._build_dpop_request_context(request) + task = asyncio.create_task( + self._verifier.verify(token, dpop_request=dpop_request) + ) + cache[token] = task + claims = await task except AuthplaneError as error: logger.debug( "authplane.token_verification_failed", @@ -100,3 +195,36 @@ async def verify_token(self, token: str) -> AccessToken | None: expires_at=claims.expires_at, claims=cast("dict[str, Any]", claims.raw), # full JWT payload ) + + def _build_dpop_request_context(self, request: Request) -> DPoPRequestContext: + """Build the per-request DPoP context. + + Always returns a context — the resource verifier inspects the + access token's ``cnf`` claim plus the context's ``proof`` to + decide whether DPoP enforcement applies. When the resource is + not configured for inbound DPoP, the verifier's Mode-3 path + rejects any DPoP signal regardless of what is passed here. + + Cross-SDK note: the TS sibling ``buildDpopRequestContext`` + returns ``undefined`` when no ``DPoP`` header is present; + Python intentionally always builds the context with + ``proof=None``. Both shapes are behaviorally equivalent in + the core verifier (Mode 3 path treats absent and ``None`` + proofs the same), but a DPoP-bound token with no proof + yields a more specific ``DPoPProofMissingError`` here + instead of ``DPoPBindingMismatchError``. The error-type + contract is pinned per language by design. + """ + # ``raw_request_path`` reads ``scope["raw_path"]`` to preserve + # percent-encoding for DPoP ``htu`` parity with the TS sibling. + # ``request.url.query`` is sourced from ``scope["query_string"]`` + # without percent-decoding, so it is already on-wire-safe. + url = f"{self._resource_origin}{raw_request_path(request)}" + query = request.url.query + if query: + url = f"{url}?{query}" + return BuiltDPoPRequestContext( + method=request.method.upper(), + url=url, + proof=read_dpop_header(request), + ) diff --git a/authplane-fastmcp/demo/mcpserver.py b/authplane-fastmcp/demo/mcpserver.py index 58a89d1..0b494ab 100644 --- a/authplane-fastmcp/demo/mcpserver.py +++ b/authplane-fastmcp/demo/mcpserver.py @@ -16,11 +16,8 @@ from authplane_fastmcp import authplane_auth -if __name__ == "__main__": - logging.basicConfig( - level=logging.DEBUG, format="%(asctime)s %(name)s %(levelname)s %(message)s" - ) +async def main() -> None: load_dotenv(os.path.join(os.path.dirname(__file__), ".env")) resource = os.environ.get("RESOURCE_URL", "http://localhost:8080/mcp") @@ -32,18 +29,16 @@ client_id = os.environ.get("CLIENT_ID", resource) client_secret = os.environ["CLIENT_SECRET"] - auth_result = asyncio.run( - authplane_auth( - issuer=issuer, - base_url=base_url, - scopes=["tools/add", "tools/multiply"], - dev_mode=True, # Enables local testing - as_credentials=ASCredentials( - client_id=client_id, - client_secret=client_secret, - ), - revocation_checker=IntrospectionRevocation(), - ) + auth_result = await authplane_auth( + issuer=issuer, + base_url=base_url, + scopes=["tools/add", "tools/multiply"], + dev_mode=True, # Enables local testing + as_credentials=ASCredentials( + client_id=client_id, + client_secret=client_secret, + ), + revocation_checker=IntrospectionRevocation(), ) mcp = FastMCP("Calculator Service", **auth_result) @@ -67,4 +62,19 @@ def multiply(a: float, b: float) -> float: # equivalent demo in ``authplane-mcp/demo/mcpserver.py``, which uses the # low-level MCP server and surfaces the elicitation correctly. - mcp.run(transport="http", port=port, log_level="DEBUG") + # The adapter setup, server, and aclose() must share one event loop — + # auth_result holds async resources (locks, httpx pool, background JWKS + # refresh tasks) bound to the running loop. ``run_async`` is FastMCP's + # async entry point and keeps everything on the same loop. + try: + await mcp.run_async(transport="http", port=port, log_level="DEBUG") + finally: + await auth_result.aclose() + + +if __name__ == "__main__": + logging.basicConfig( + level=logging.DEBUG, format="%(asctime)s %(name)s %(levelname)s %(message)s" + ) + + asyncio.run(main()) diff --git a/authplane-fastmcp/tests/conftest.py b/authplane-fastmcp/tests/conftest.py index f6db4f7..37fe958 100644 --- a/authplane-fastmcp/tests/conftest.py +++ b/authplane-fastmcp/tests/conftest.py @@ -63,8 +63,12 @@ def mock_verifier(valid_claims: VerifiedClaims) -> AsyncMock: mock = AsyncMock(spec=AuthplaneResource) type(mock).scopes = PropertyMock(return_value=["tools/query", "tools/write"]) + type(mock).resource = PropertyMock(return_value="https://api.example.com/mcp") - async def verify_side_effect(token: str) -> VerifiedClaims: + async def verify_side_effect( + token: str, *, dpop_request: object | None = None + ) -> VerifiedClaims: + _ = dpop_request # accept but ignore — covered by dedicated DPoP tests if token == "valid_token": return valid_claims raise AuthplaneError("Invalid token") diff --git a/authplane-fastmcp/tests/test_auth_factory.py b/authplane-fastmcp/tests/test_auth_factory.py index 0412598..2b1bb06 100644 --- a/authplane-fastmcp/tests/test_auth_factory.py +++ b/authplane-fastmcp/tests/test_auth_factory.py @@ -18,7 +18,9 @@ async def test_authplane_auth_parameter_propagation(): dpop_provider = MagicMock(spec=DPoPProvider) mock_client = MagicMock() - mock_client.resource = MagicMock(return_value=MagicMock()) + _mock_resource = MagicMock() + _mock_resource.resource = "https://api.example.com/mcp" + mock_client.resource = MagicMock(return_value=_mock_resource) with patch("authplane_fastmcp.auth.AuthplaneClient") as mock_client_cls: mock_client_cls.create = AsyncMock(return_value=mock_client) @@ -32,6 +34,7 @@ async def test_authplane_auth_parameter_propagation(): metadata_refresh_seconds=7200, cache_ttl_buffer_seconds=15.0, default_ttl_seconds=900.0, + cache_max_entries=500, circuit_breaker_threshold=7, circuit_breaker_cooldown_seconds=45.0, clock_skew_seconds=60, @@ -49,6 +52,7 @@ async def test_authplane_auth_parameter_propagation(): assert client_kwargs["metadata_refresh_seconds"] == 7200 assert client_kwargs["cache_ttl_buffer_seconds"] == 15.0 assert client_kwargs["default_ttl_seconds"] == 900.0 + assert client_kwargs["cache_max_entries"] == 500 assert client_kwargs["circuit_breaker_threshold"] == 7 assert client_kwargs["circuit_breaker_cooldown_seconds"] == 45.0 assert client_kwargs["fetch_settings"] is custom_fetch_settings @@ -66,7 +70,9 @@ async def test_authplane_auth_parameter_propagation(): async def test_authplane_auth_none_filtering(): """Verify that None values are NOT passed to AuthplaneClient.create or client.resource.""" mock_client = MagicMock() - mock_client.resource = MagicMock(return_value=MagicMock()) + _mock_resource = MagicMock() + _mock_resource.resource = "https://api.example.com/mcp" + mock_client.resource = MagicMock(return_value=_mock_resource) with patch("authplane_fastmcp.auth.AuthplaneClient") as mock_client_cls: mock_client_cls.create = AsyncMock(return_value=mock_client) @@ -87,6 +93,7 @@ async def test_authplane_auth_none_filtering(): assert "metadata_refresh_seconds" not in client_kwargs assert "cache_ttl_buffer_seconds" not in client_kwargs assert "default_ttl_seconds" not in client_kwargs + assert "cache_max_entries" not in client_kwargs assert "circuit_breaker_threshold" not in client_kwargs assert "circuit_breaker_cooldown_seconds" not in client_kwargs @@ -100,7 +107,9 @@ async def test_authplane_auth_none_filtering(): async def test_authplane_auth_revocation_checker_default_is_none(): """When revocation_checker is not passed, None is forwarded (no revocation checking).""" mock_client = MagicMock() - mock_client.resource = MagicMock(return_value=MagicMock()) + _mock_resource = MagicMock() + _mock_resource.resource = "https://api.example.com/mcp" + mock_client.resource = MagicMock(return_value=_mock_resource) with patch("authplane_fastmcp.auth.AuthplaneClient") as mock_client_cls: mock_client_cls.create = AsyncMock(return_value=mock_client) @@ -122,7 +131,9 @@ async def my_checker(claims: VerifiedClaims, raw_token: str) -> bool: return False mock_client = MagicMock() - mock_client.resource = MagicMock(return_value=MagicMock()) + _mock_resource = MagicMock() + _mock_resource.resource = "https://api.example.com/mcp" + mock_client.resource = MagicMock(return_value=_mock_resource) with patch("authplane_fastmcp.auth.AuthplaneClient") as mock_client_cls: mock_client_cls.create = AsyncMock(return_value=mock_client) @@ -140,7 +151,9 @@ async def my_checker(claims: VerifiedClaims, raw_token: str) -> bool: async def test_authplane_auth_resource_derivation(): """Verify resource URL construction from base_url and mcp_path.""" mock_client = MagicMock() - mock_client.resource = MagicMock(return_value=MagicMock()) + _mock_resource = MagicMock() + _mock_resource.resource = "https://api.example.com/mcp" + mock_client.resource = MagicMock(return_value=_mock_resource) with patch("authplane_fastmcp.auth.AuthplaneClient") as mock_client_cls: mock_client_cls.create = AsyncMock(return_value=mock_client) @@ -179,7 +192,9 @@ async def test_authplane_auth_as_credentials_passthrough(): creds = ASCredentials(client_id="client_id", client_secret="secret") mock_client = MagicMock() - mock_client.resource = MagicMock(return_value=MagicMock()) + _mock_resource = MagicMock() + _mock_resource.resource = "https://api.example.com/mcp" + mock_client.resource = MagicMock(return_value=_mock_resource) with patch("authplane_fastmcp.auth.AuthplaneClient") as mock_client_cls: mock_client_cls.create = AsyncMock(return_value=mock_client) @@ -197,7 +212,9 @@ async def test_authplane_auth_as_credentials_passthrough(): async def test_authplane_auth_returns_auth_result(): """authplane_auth() returns an AuthplaneAuthResult with auth, token_verifier, and client.""" mock_client = MagicMock() - mock_client.resource = MagicMock(return_value=MagicMock()) + _mock_resource = MagicMock() + _mock_resource.resource = "https://api.example.com/mcp" + mock_client.resource = MagicMock(return_value=_mock_resource) with ( patch("authplane_fastmcp.auth.AuthplaneClient") as mock_client_cls, @@ -282,7 +299,9 @@ async def test_authplane_auth_result_aclose_idempotent(): async def test_authplane_auth_resource_matches_default_mcp_path(): """Resource passed to verifier must equal base_url + default mcp_path (/mcp).""" mock_client = MagicMock() - mock_client.resource = MagicMock(return_value=MagicMock()) + _mock_resource = MagicMock() + _mock_resource.resource = "https://api.example.com/mcp" + mock_client.resource = MagicMock(return_value=_mock_resource) with patch("authplane_fastmcp.auth.AuthplaneClient") as mock_client_cls: mock_client_cls.create = AsyncMock(return_value=mock_client) @@ -305,6 +324,7 @@ async def test_verify_token_non_authplane_error_propagates(): from authplane_fastmcp import AuthplaneTokenVerifier mock_verifier = AsyncMock() + mock_verifier.resource = "https://api.example.com/mcp" mock_verifier.verify.side_effect = RuntimeError("unexpected") tv = AuthplaneTokenVerifier(mock_verifier) diff --git a/authplane-fastmcp/tests/test_verifier.py b/authplane-fastmcp/tests/test_verifier.py index 0f78844..ec406c7 100644 --- a/authplane-fastmcp/tests/test_verifier.py +++ b/authplane-fastmcp/tests/test_verifier.py @@ -59,7 +59,8 @@ async def test_verify_token_expired_returns_none( """verify_token() with expired token returns None.""" from authplane.errors import TokenExpiredError - async def verify_expired(token: str) -> VerifiedClaims: + async def verify_expired(token: str, *, dpop_request: object | None = None) -> VerifiedClaims: + _ = dpop_request raise TokenExpiredError("Token has expired") mock_verifier.verify.side_effect = verify_expired diff --git a/authplane-fastmcp/tests/test_verifier_dpop_cache.py b/authplane-fastmcp/tests/test_verifier_dpop_cache.py new file mode 100644 index 0000000..d7c8be6 --- /dev/null +++ b/authplane-fastmcp/tests/test_verifier_dpop_cache.py @@ -0,0 +1,496 @@ +"""DPoP context forwarding + per-request verify cache tests. + +Covers two coupled changes: + +* ``verify_token`` must build a ``DPoPRequestContext`` from the active + HTTP request and forward it to ``AuthplaneResource.verify`` so the + proof check actually runs. Without it, a resource configured for + ``inbound_dpop=InboundDPoPOptions(required=True)`` silently accepts + bearer-only requests even though PRM advertises the requirement. +* Repeated ``verify_token`` calls within the same HTTP request must + collapse onto a single underlying ``verify`` so the inbound replay + store cannot see the same proof ``jti`` twice within one request. + Distinct requests get distinct caches so cross-request replay + protection is preserved. +""" + +from __future__ import annotations + +import time +from typing import Any +from unittest.mock import AsyncMock, PropertyMock + +import pytest +from authplane import AuthplaneResource, DPoPReplayDetectedError, VerifiedClaims +from authplane._dpop_adapter import get_or_create_verify_cache +from starlette.requests import Request + +from authplane_fastmcp import AuthplaneTokenVerifier + + +def _make_request( + *, + method: str = "POST", + path: str = "/mcp", + query: str = "", + host: str = "testserver", + headers: dict[str, str] | None = None, +) -> Request: + """Synthesize a Starlette ``Request`` suitable for driving the verifier. + + The verifier reconstructs ``htu`` from the operator-configured + resource origin, so the ``host`` here is intentionally divergent — + the tests assert the request's ``Host`` header is *not* what ends up + in the proof binding. + """ + raw_headers: list[tuple[bytes, bytes]] = [] + if headers: + for key, value in headers.items(): + raw_headers.append((key.lower().encode("latin-1"), value.encode("latin-1"))) + scope: dict[str, Any] = { + "type": "http", + "method": method, + "path": path, + "raw_path": path.encode("latin-1"), + "query_string": query.encode("latin-1"), + "headers": raw_headers, + "scheme": "http", + "server": (host, 80), + "client": None, + "root_path": "", + "http_version": "1.1", + } + return Request(scope) + + +def _mock_verifier( + *, + resource: str = "https://api.example.com/mcp", + claims: VerifiedClaims | None = None, + raise_exc: BaseException | None = None, +) -> AsyncMock: + """Build an ``AuthplaneResource`` mock with the verify contract pinned. + + Returning a ``VerifiedClaims`` (or raising) on every call lets the + cache tests count invocations via ``mock.verify.call_count``. + """ + if claims is None and raise_exc is None: + now = int(time.time()) + claims = VerifiedClaims( + sub="user_123", + client_id="client_456", + scopes=("tools/query",), + issuer="https://auth.example.com", + audience=("https://api.example.com",), + expires_at=now + 3600, + issued_at=now, + jti="token-id-123", + kid="test-key-1", + raw={ + "iss": "https://auth.example.com", + "aud": "https://api.example.com", + "sub": "user_123", + "client_id": "client_456", + "scope": "tools/query", + "exp": now + 3600, + "nbf": now, + "iat": now, + "jti": "token-id-123", + }, + ) + mock = AsyncMock(spec=AuthplaneResource) + type(mock).scopes = PropertyMock(return_value=["tools/query"]) + type(mock).resource = PropertyMock(return_value=resource) + + async def verify(token: str, *, dpop_request: object | None = None) -> VerifiedClaims: + _ = token, dpop_request + if raise_exc is not None: + raise raise_exc + assert claims is not None + return claims + + mock.verify = AsyncMock(side_effect=verify) + return mock + + +def _make_token_verifier( + mock_resource: AsyncMock, request: Request | None +) -> AuthplaneTokenVerifier: + """Token verifier wired to a fake ``get_http_request`` for tests. + + ``request=None`` simulates ``RuntimeError("No active HTTP request found.")`` + that ``fastmcp.server.dependencies.get_http_request`` raises outside a + request context. + """ + + def fake_get_http_request() -> Request: + if request is None: + raise RuntimeError("No active HTTP request found.") + return request + + return AuthplaneTokenVerifier( + mock_resource, + base_url="https://api.example.com", + get_http_request=fake_get_http_request, + ) + + +# --------------------------------------------------------------------------- +# DPoP context is built and forwarded to AuthplaneResource.verify +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_dpop_context_forwarded_to_verify() -> None: + """verify_token must pass a DPoPRequestContext to verify(). + + Before this change the bare ``verify(token)`` call left + ``dpop_request`` at its default ``None``, which the core verifier + treats as "no proof presented" — so any ``inbound_dpop=required=True`` + configuration silently accepted bearer-only requests. + """ + mock = _mock_verifier() + request = _make_request( + method="POST", + path="/mcp", + headers={"DPoP": "fake.proof.jwt"}, + ) + verifier = _make_token_verifier(mock, request) + + result = await verifier.verify_token("valid_token") + + assert result is not None + assert mock.verify.await_count == 1 + kwargs = mock.verify.await_args.kwargs + ctx = kwargs["dpop_request"] + assert ctx is not None + assert ctx.method == "POST" + assert ctx.url == "https://api.example.com/mcp" + assert ctx.proof == "fake.proof.jwt" + + +@pytest.mark.asyncio +async def test_dpop_context_proof_none_when_header_absent() -> None: + """Context is still built (so htm/htu reach the verifier), proof=None. + + Lets the core verifier's required-mode path fail closed when the + access token is DPoP-bound but the client did not present a proof. + """ + mock = _mock_verifier() + request = _make_request(headers={"authorization": "Bearer xyz"}) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("valid_token") + + ctx = mock.verify.await_args.kwargs["dpop_request"] + assert ctx is not None + assert ctx.proof is None + assert ctx.method == "POST" + assert ctx.url == "https://api.example.com/mcp" + + +@pytest.mark.asyncio +async def test_dpop_context_header_lookup_is_case_insensitive() -> None: + """RFC 7230 says HTTP header names are case-insensitive; verify the wire.""" + mock = _mock_verifier() + request = _make_request(headers={"dpop": "lower.case.proof"}) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("valid_token") + + assert mock.verify.await_args.kwargs["dpop_request"].proof == "lower.case.proof" + + +@pytest.mark.asyncio +async def test_htu_origin_from_configured_resource_not_host_header() -> None: + """htu's origin comes from the configured resource, never from Host. + + Mirrors the TS sibling: an upstream that controls the Host / + X-Forwarded-Proto headers must not be able to decide which htu the + DPoP proof is validated against. + """ + mock = _mock_verifier(resource="https://api.example.com/mcp") + request = _make_request( + host="attacker.example.com", + headers={ + "DPoP": "p", + "host": "attacker.example.com", + "x-forwarded-proto": "http", + }, + ) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("valid_token") + + ctx = mock.verify.await_args.kwargs["dpop_request"] + assert ctx.url.startswith("https://api.example.com") + assert "attacker" not in ctx.url + + +@pytest.mark.asyncio +async def test_htu_includes_request_path_and_query() -> None: + """htu = origin + path + (?query) per RFC 9449 §4.2.""" + mock = _mock_verifier() + request = _make_request(path="/mcp/tools", query="x=1&y=2", headers={"DPoP": "p"}) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("valid_token") + + assert ( + mock.verify.await_args.kwargs["dpop_request"].url + == "https://api.example.com/mcp/tools?x=1&y=2" + ) + + +@pytest.mark.asyncio +async def test_htu_preserves_percent_encoded_path_from_raw_path() -> None: + """htu uses ``scope['raw_path']`` so percent-encoding survives. + + ASGI populates ``scope['path']`` as the percent-decoded path, but the + DPoP proof was signed over the on-wire (still-encoded) URL. The TS + sibling reads ``IncomingMessage.url`` (raw bytes), so reading + ``raw_path`` here keeps cross-SDK proof binding identical. + """ + mock = _mock_verifier() + # Decoded path: "/mcp/users/a/b" ; raw: "/mcp/users/a%2Fb" — a client + # that signed the encoded form must bind against the encoded htu. + raw_headers: list[tuple[bytes, bytes]] = [(b"dpop", b"p")] + scope: dict[str, Any] = { + "type": "http", + "method": "POST", + "path": "/mcp/users/a/b", + "raw_path": b"/mcp/users/a%2Fb", + "query_string": b"", + "headers": raw_headers, + "scheme": "http", + "server": ("testserver", 80), + "client": None, + "root_path": "", + "http_version": "1.1", + } + request = Request(scope) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("valid_token") + + assert ( + mock.verify.await_args.kwargs["dpop_request"].url + == "https://api.example.com/mcp/users/a%2Fb" + ) + + +@pytest.mark.asyncio +async def test_htu_falls_back_to_decoded_path_when_raw_path_absent() -> None: + """Servers that omit ``raw_path`` still get a sensible htu. + + Some ASGI servers (and synthetic test scopes) do not populate + ``raw_path``. The helper must fall back to ``request.url.path`` + rather than blowing up — for the no-percent-encoding case the two + are equal anyway. + """ + mock = _mock_verifier() + scope: dict[str, Any] = { + "type": "http", + "method": "POST", + "path": "/mcp", + "query_string": b"", + "headers": [(b"dpop", b"p")], + "scheme": "http", + "server": ("testserver", 80), + "client": None, + "root_path": "", + "http_version": "1.1", + } + request = Request(scope) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("valid_token") + + assert mock.verify.await_args.kwargs["dpop_request"].url == "https://api.example.com/mcp" + + +@pytest.mark.asyncio +async def test_no_http_request_context_falls_back_to_no_dpop() -> None: + """Outside an HTTP request, the verifier still works (unit-test ergonomics). + + fastmcp.server.dependencies.get_http_request() raises RuntimeError + when called without an active request — the adapter must verify + without a DPoP context rather than throw. + """ + mock = _mock_verifier() + verifier = _make_token_verifier(mock, request=None) + + result = await verifier.verify_token("valid_token") + + assert result is not None + assert mock.verify.await_args.kwargs["dpop_request"] is None + + +# --------------------------------------------------------------------------- +# Per-request verify cache +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_repeat_verify_token_collapses_onto_one_verify() -> None: + """Two verify_token calls in one request → one underlying verify call. + + The cache is what protects DPoP-required mode from a + DPoPReplayDetected error in case any future framework change starts + invoking ``verify_token`` more than once per HTTP request. + """ + mock = _mock_verifier() + request = _make_request(headers={"DPoP": "p"}) + verifier = _make_token_verifier(mock, request) + + first = await verifier.verify_token("the_token") + second = await verifier.verify_token("the_token") + + assert first is not None + assert second is not None + assert mock.verify.await_count == 1 + + +@pytest.mark.asyncio +async def test_cache_is_scoped_per_request() -> None: + """Distinct requests do not share the cache; replay protection survives.""" + mock = _mock_verifier() + req_one = _make_request(headers={"DPoP": "p1"}) + req_two = _make_request(headers={"DPoP": "p2"}) + + requests: list[Request] = [req_one, req_two] + + def pop_request() -> Request: + return requests.pop(0) + + verifier = AuthplaneTokenVerifier( + mock, + base_url="https://api.example.com", + get_http_request=pop_request, + ) + + await verifier.verify_token("t") + await verifier.verify_token("t") + + assert mock.verify.await_count == 2 + + +@pytest.mark.asyncio +async def test_cache_keyed_by_token_not_by_request_slot() -> None: + """Different tokens within one request get verified independently. + + A scenario that never arises on the standard FastMCP HTTP path + (BearerAuthBackend extracts one Authorization per request), but + keying by token is cheap and removes a footgun the TS adapter + technically carries (a different ``verify_token(otherToken)`` call + inside one request would reuse the first call's result there). + """ + mock = _mock_verifier() + request = _make_request(headers={"DPoP": "p"}) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("token_a") + await verifier.verify_token("token_b") + + assert mock.verify.await_count == 2 + + +@pytest.mark.asyncio +async def test_cached_exception_reraises_typed_on_replay_within_request() -> None: + """Acceptance: cached typed error re-raises on replay; not masked to None only the 2nd time. + + Both calls of ``verify_token`` map AuthplaneError → None, but the + underlying cached task carries the typed exception unchanged so an + operator awaiting the slot on ``request.state`` directly still + sees the original DPoPReplayDetectedError. This is what keeps the + failure diagnosable rather than collapsing to a generic 401 reason. + """ + err = DPoPReplayDetectedError("DPoP proof jti has already been seen") + mock = _mock_verifier(raise_exc=err) + request = _make_request(headers={"DPoP": "p"}) + verifier = _make_token_verifier(mock, request) + + first = await verifier.verify_token("t") + second = await verifier.verify_token("t") + + assert first is None + assert second is None + assert mock.verify.await_count == 1 + + cache = get_or_create_verify_cache(request) + cached_task = cache["t"] + with pytest.raises(DPoPReplayDetectedError): + await cached_task + + +@pytest.mark.asyncio +async def test_request_state_cache_stashes_task() -> None: + """The cache slot is populated with an asyncio Task keyed by the access token.""" + import asyncio + + mock = _mock_verifier() + request = _make_request(headers={"DPoP": "p"}) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("t") + + cache = get_or_create_verify_cache(request) + assert "t" in cache + assert isinstance(cache["t"], asyncio.Task) + + +@pytest.mark.asyncio +async def test_no_request_context_means_no_cache_either() -> None: + """Outside a request: each call hits verify(); nothing is cached anywhere.""" + mock = _mock_verifier() + verifier = _make_token_verifier(mock, request=None) + + await verifier.verify_token("t") + await verifier.verify_token("t") + + assert mock.verify.await_count == 2 + + +# --------------------------------------------------------------------------- +# Constructor and request-lookup guards (review hardening) +# --------------------------------------------------------------------------- + + +def test_init_rejects_non_string_resource() -> None: + """A mis-wired mock (no ``spec``) would silently corrupt htu reconstruction. + + ``urlsplit`` accepts a ``MagicMock`` without raising and produces a + bogus ``_resource_origin`` like ``MagicMock://MagicMock`` — guard + against this by failing fast in ``__init__``. + """ + mock = AsyncMock(spec=AuthplaneResource) + type(mock).resource = PropertyMock(return_value=object()) # not a str + type(mock).scopes = PropertyMock(return_value=[]) + + with pytest.raises(TypeError, match=r"verifier\.resource must be a str URI"): + AuthplaneTokenVerifier(mock, base_url="https://api.example.com") + + +@pytest.mark.asyncio +async def test_unrelated_runtimeerror_from_get_http_request_propagates() -> None: + """Only the documented 'No active HTTP request found.' is swallowed. + + A future FastMCP release surfacing an unrelated ``RuntimeError`` from + ``get_http_request`` (context manager not entered, etc.) must not be + masked as ``dpop_request=None`` — that would silently re-introduce + the no-proof-forwarded silent-pass this module fixed. + """ + mock = _mock_verifier() + + def broken_get_http_request() -> Request: + raise RuntimeError("FastMCP internal: context not initialized") + + verifier = AuthplaneTokenVerifier( + mock, + base_url="https://api.example.com", + get_http_request=broken_get_http_request, + ) + + with pytest.raises(RuntimeError, match="context not initialized"): + await verifier.verify_token("t") + assert mock.verify.await_count == 0 diff --git a/authplane-mcp/authplane_mcp/__init__.py b/authplane-mcp/authplane_mcp/__init__.py index ce15b3d..9fa26c2 100644 --- a/authplane-mcp/authplane_mcp/__init__.py +++ b/authplane-mcp/authplane_mcp/__init__.py @@ -17,15 +17,19 @@ except _PackageNotFoundError: # pragma: no cover - source tree without an install __version__ = "0.0.0+unknown" -from .auth import AuthplaneAuthResult, authplane_mcp_auth, require_scope +from ._request_context import AuthplaneRequestContextMiddleware, get_current_request +from .auth import AuthplaneAuthResult, authplane_mcp_auth, install_request_context, require_scope from .url_elicitation import to_url_elicitation_required_error from .verifier import AuthplaneTokenVerifier __all__ = [ "AuthplaneAuthResult", + "AuthplaneRequestContextMiddleware", "AuthplaneTokenVerifier", "__version__", "authplane_mcp_auth", + "get_current_request", + "install_request_context", "require_scope", "to_url_elicitation_required_error", ] diff --git a/authplane-mcp/authplane_mcp/_request_context.py b/authplane-mcp/authplane_mcp/_request_context.py new file mode 100644 index 0000000..b2d44a9 --- /dev/null +++ b/authplane-mcp/authplane_mcp/_request_context.py @@ -0,0 +1,91 @@ +"""Per-request ASGI context for the authplane-mcp adapter. + +The MCP SDK's ``TokenVerifier`` protocol exposes only +``verify_token(token: str)`` with no per-request hook — unlike FastMCP, +which ships ``fastmcp.server.dependencies.get_http_request()``. To +forward a :class:`~authplane.DPoPRequestContext` to +:meth:`AuthplaneResource.verify`, the verifier needs to reach the active +:class:`~starlette.requests.Request` from inside ``verify_token``. + +This module provides: + +* :data:`_current_request` — a :class:`contextvars.ContextVar` holding + the active Starlette ``Request`` for the duration of the ASGI scope. +* :func:`get_current_request` — the default lookup the verifier uses to + read that ContextVar; raises ``RuntimeError("No active HTTP request + found.")`` outside an HTTP request (message matches FastMCP's + ``get_http_request`` so the verifier's narrow-on-message fallback + catches both). +* :class:`AuthplaneRequestContextMiddleware` — an ASGI middleware that + populates the ContextVar at the start of each HTTP request and clears + it on exit. Must run *before* MCP's ``AuthenticationMiddleware`` so + ``BearerAuthBackend.authenticate(conn)`` finds the request set when it + calls ``verify_token``. + +Install via :func:`authplane_mcp.install_request_context` (wraps the +Starlette app returned by ``mcp.streamable_http_app()``); see the +``authplane_mcp_auth`` docstring for the integration pattern. +""" + +from __future__ import annotations + +from contextvars import ContextVar +from typing import TYPE_CHECKING + +from starlette.requests import Request + +if TYPE_CHECKING: + from starlette.types import ASGIApp, Receive, Scope, Send + +_current_request: ContextVar[Request | None] = ContextVar( + "authplane_mcp_current_request", default=None +) + +_NO_ACTIVE_REQUEST_MESSAGE = "No active HTTP request found." + + +def get_current_request() -> Request: + """Return the active Starlette ``Request`` for the current ASGI scope. + + Raises: + RuntimeError: If called outside an HTTP request scope (no active + request set by :class:`AuthplaneRequestContextMiddleware`). + The message is intentionally identical to the one FastMCP's + ``get_http_request`` raises so the verifier's narrow-on- + message fallback catches both sources without branching. + """ + request = _current_request.get() + if request is None: + raise RuntimeError(_NO_ACTIVE_REQUEST_MESSAGE) + return request + + +class AuthplaneRequestContextMiddleware: + """ASGI middleware exposing the active Starlette ``Request`` via ContextVar. + + Must run **before** MCP's ``AuthenticationMiddleware`` so the + ContextVar is populated when ``BearerAuthBackend.authenticate(conn)`` + calls into :meth:`AuthplaneTokenVerifier.verify_token`. The + middleware is a no-op for non-HTTP scopes (lifespan, websocket). + + Use :func:`authplane_mcp.install_request_context` to install on a + ``FastMCP`` instance; that helper wraps the Starlette app returned by + ``mcp.streamable_http_app()`` so ``mcp.run(transport="streamable- + http")`` picks the middleware up transparently. + """ + + def __init__(self, app: ASGIApp) -> None: + self.app = app + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + if scope["type"] != "http": + await self.app(scope, receive, send) + return + # Building the Request once here and storing on the ContextVar lets + # the verifier reuse ``request.state`` (same underlying scope state + # object) for the per-request verify cache. + token = _current_request.set(Request(scope)) + try: + await self.app(scope, receive, send) + finally: + _current_request.reset(token) diff --git a/authplane-mcp/authplane_mcp/auth.py b/authplane-mcp/authplane_mcp/auth.py index 58adff2..a186b82 100644 --- a/authplane-mcp/authplane_mcp/auth.py +++ b/authplane-mcp/authplane_mcp/auth.py @@ -20,11 +20,16 @@ from authplane.oauth import TokenExchangeOptions, TokenResponse from mcp.server.auth.middleware.auth_context import get_access_token as _get_access_token from mcp.server.auth.settings import AuthSettings +from mcp.server.fastmcp import FastMCP from pydantic import AnyHttpUrl +from starlette.applications import Starlette +from ._request_context import AuthplaneRequestContextMiddleware from .url_elicitation import to_url_elicitation_required_error from .verifier import AuthplaneTokenVerifier +_INSTALLED_FLAG = "_authplane_request_context_installed" + def _wrap_client_for_elicitation(client: AuthplaneClient) -> AuthplaneClient: """Translate ``client.exchange`` consent errors into MCP ``-32042``. @@ -71,6 +76,78 @@ async def add(a: float, b: float) -> float: raise PermissionError(f"Missing required scope: {scope}") +def install_request_context(mcp: FastMCP) -> None: + """Install :class:`AuthplaneRequestContextMiddleware` on a ``FastMCP`` server. + + Wraps ``mcp.streamable_http_app`` so the Starlette app it returns + runs :class:`AuthplaneRequestContextMiddleware` before MCP's + ``AuthenticationMiddleware``. That middleware publishes the active + :class:`starlette.requests.Request` on a ContextVar, which + :meth:`AuthplaneTokenVerifier.verify_token` reads to forward a + :class:`~authplane.DPoPRequestContext` to + :meth:`AuthplaneResource.verify`. + + The MCP SDK's ``FastMCP`` wires its middleware list internally with + no public hook for user middleware, so this is the least-invasive way + to slot ours in without subclassing or monkeypatching the SDK. + + Without this call, the verifier still works for non-DPoP flows, but + DPoP-bound requests fail closed: :func:`get_current_request` raises, + the verifier passes ``dpop_request=None``, and the core rejects bound + tokens with ``DPoPBindingMismatchError`` (and rejects bearer-only + tokens under ``inbound_dpop=InboundDPoPOptions(required=True)``). + The misconfiguration surfaces as a 401 on the first request rather + than as a silent bypass, so an operator who skips this call will + notice immediately. + + Args: + mcp: A ``FastMCP`` instance (typically + ``FastMCP("...", **authplane_mcp_auth_result)``). Safe to + call before tools are registered. + + Example:: + + async def main() -> None: + result = await authplane_mcp_auth(issuer=..., resource=..., ...) + mcp = FastMCP("My Server", **result) + install_request_context(mcp) # required for inbound DPoP + async with result: + await mcp.run_streamable_http_async() + + asyncio.run(main()) + + Idempotent: a second call on the same ``FastMCP`` instance is a no-op. + Without this guard, repeated installs would chain wrappers and the + inner middleware's ``ContextVar.reset`` would fire against a token + created by the outer wrapper, raising + ``RuntimeError: was created in a different Context`` at + request time. + """ + if getattr(mcp, _INSTALLED_FLAG, False): + return + + original_streamable_http_app = mcp.streamable_http_app + + def streamable_http_app() -> Starlette: + app = original_streamable_http_app() + # ``Starlette.add_middleware`` rejects calls after the app has built + # its middleware stack (first request). ``streamable_http_app`` is + # invoked once at startup before serving begins, so wrapping is safe + # here and runs before MCP's AuthenticationMiddleware on every call. + app.add_middleware(AuthplaneRequestContextMiddleware) + return app + + # Fragility: instance-attribute assignment works only because FastMCP + # exposes ``streamable_http_app`` as a plain method, not a ``@property`` + # or ``@cached_property``. If a future MCP SDK release changes that, the + # assignment will silently no-op (or raise AttributeError) and DPoP + # enforcement will fall back to ``dpop_request=None`` on every request. + # Track https://github.com/modelcontextprotocol/python-sdk for a public + # subclassing hook or per-app middleware API and migrate to it when available. + mcp.streamable_http_app = streamable_http_app + setattr(mcp, _INSTALLED_FLAG, True) + + class AuthplaneAuthResult: """Return value of ``authplane_mcp_auth()``. @@ -137,6 +214,7 @@ async def authplane_mcp_auth( metadata_refresh_seconds: int | None = None, cache_ttl_buffer_seconds: float | None = None, default_ttl_seconds: float | None = None, + cache_max_entries: int | None = None, circuit_breaker_threshold: int | None = None, circuit_breaker_cooldown_seconds: float | None = None, clock_skew_seconds: int | None = None, @@ -207,6 +285,11 @@ async def authplane_mcp_auth( before cache expiry (default ``30.0``). default_ttl_seconds: Fallback token cache TTL used when token responses do not include expiry metadata (default ``3600.0``). + cache_max_entries: Maximum number of tokens kept in the LRU cache + before the least-recently-used entry is evicted (default + :attr:`TokenCache.DEFAULT_MAX_ENTRIES` = 10 000). Token-exchange + keys are high-cardinality; raise this cap for long-lived servers + with many distinct subjects. circuit_breaker_threshold: Number of transient failures before opening the AS circuit breaker (default ``5``). circuit_breaker_cooldown_seconds: Cooldown before allowing a @@ -267,6 +350,7 @@ async def authplane_mcp_auth( "metadata_refresh_seconds": metadata_refresh_seconds, "cache_ttl_buffer_seconds": cache_ttl_buffer_seconds, "default_ttl_seconds": default_ttl_seconds, + "cache_max_entries": cache_max_entries, "circuit_breaker_threshold": circuit_breaker_threshold, "circuit_breaker_cooldown_seconds": circuit_breaker_cooldown_seconds, } diff --git a/authplane-mcp/authplane_mcp/verifier.py b/authplane-mcp/authplane_mcp/verifier.py index d62dfe0..feed00f 100644 --- a/authplane-mcp/authplane_mcp/verifier.py +++ b/authplane-mcp/authplane_mcp/verifier.py @@ -5,35 +5,120 @@ ``AuthplaneResource`` and mapping results to MCP's ``AccessToken``. """ +import asyncio import logging - -from authplane import AuthplaneError, AuthplaneResource +from collections.abc import Callable +from urllib.parse import urlsplit + +from authplane import AuthplaneError, AuthplaneResource, DPoPRequestContext +from authplane._dpop_adapter import ( + BuiltDPoPRequestContext, + get_or_create_verify_cache, + raw_request_path, + read_dpop_header, +) from mcp.server.auth.provider import AccessToken, TokenVerifier +from starlette.requests import Request + +from ._request_context import get_current_request as _default_get_http_request logger = logging.getLogger(__name__) +__all__ = ["AuthplaneTokenVerifier"] + + class AuthplaneTokenVerifier(TokenVerifier): """MCP SDK TokenVerifier backed by AuthplaneResource. Validates JWTs once per request via the core Authplane SDK and returns an MCP ``AccessToken`` populated with standard OAuth 2.1 claims - (``client_id``, ``scopes``, ``expires_at``, ``resource``). - - All security-critical logic (signature verification, claim validation, - JWKS caching, SSRF protection) is handled by the core SDK. This class + (``client_id``, ``scopes``, ``expires_at``, ``resource``). All + security-critical logic (signature verification, claim validation, + JWKS caching, SSRF protection) is handled by the core SDK; this class is a thin adapter that maps between the two interfaces. + + DPoP (RFC 9449) + --------------- + + When the underlying :class:`AuthplaneResource` was created with + ``inbound_dpop=InboundDPoPOptions(...)``, this verifier pulls the + active HTTP request via :func:`get_current_request` (populated by + :class:`AuthplaneRequestContextMiddleware`), builds a + :class:`~authplane.DPoPRequestContext` (method + reconstructed + ``htu`` + proof header), and forwards it to + :meth:`AuthplaneResource.verify`. The ``htu`` origin + (scheme + host + port) is taken from the operator-configured + resource URI, never from the inbound ``Host`` / + ``X-Forwarded-Proto`` headers — letting an upstream decide which + ``htu`` the proof is checked against would neuter DPoP's + cross-endpoint anti-replay. Only the path varies per call. + + The MCP SDK ships no per-request dependency analogous to + FastMCP's ``get_http_request()``, so the adapter installs a small + ASGI middleware (:class:`AuthplaneRequestContextMiddleware`) that + publishes the active :class:`starlette.requests.Request` on a + ContextVar before MCP's ``AuthenticationMiddleware`` runs. Call + :func:`authplane_mcp.install_request_context` on your ``FastMCP`` + instance to wire that middleware in — otherwise + :func:`get_current_request` raises, the verifier passes + ``dpop_request=None``, and DPoP-bound requests fail closed + (``DPoPBindingMismatchError``). Bearer-only tokens are likewise + rejected when the resource is configured with + ``inbound_dpop=InboundDPoPOptions(required=True)``. The + misconfiguration surfaces as a 401, not as a silent bypass. + + Per-request verify cache + ------------------------ + + MCP's standard HTTP stack invokes ``verify_token`` exactly once per + request (Starlette ``AuthenticationMiddleware`` → + ``BearerAuthBackend`` → ``TokenVerifier.verify_token``). The first + call's in-flight verify task is stashed on ``request.state`` keyed by + the access token; any subsequent invocation within the same request + awaits the same task instead of re-entering the inbound DPoP replay + store. The cache is defensive: it mirrors the TS adapter's + ``AsyncLocalStorage`` pattern and pre-empts a class of regressions + where a future framework change (transport rewrite, custom auth + provider, ASGI wrapper) would silently double-call ``verify_token`` + and the second call's proof would be rejected as + ``DPoPReplayDetected``. Different requests get distinct + ``request.state`` objects so cross-request replay protection is + preserved. """ - def __init__(self, verifier: AuthplaneResource) -> None: + def __init__( + self, + verifier: AuthplaneResource, + *, + get_http_request: Callable[[], Request] | None = None, + ) -> None: """Initialize the token verifier. Args: verifier: A fully initialized ``AuthplaneResource`` instance, typically created via ``AuthplaneClient.create()`` and ``client.resource()``. + get_http_request: Override for the active-request lookup + (defaults to :func:`get_current_request`, which reads the + ContextVar set by + :class:`AuthplaneRequestContextMiddleware`). Tests inject + a fake to drive the DPoP / per-request-cache paths + without spinning up an ASGI app. """ self._verifier = verifier + self._get_http_request = get_http_request or _default_get_http_request + + # ``AuthplaneResource.resource`` is operator-configured and must be a + # string URI — guard against mis-wired mocks (a bare ``MagicMock`` with + # no ``resource`` set silently produces ``MagicMock://MagicMock`` here + # and would corrupt ``htu`` reconstruction in production). + if not isinstance(verifier.resource, str): + raise TypeError( + f"verifier.resource must be a str URI, got {type(verifier.resource).__name__}" + ) + split = urlsplit(verifier.resource) + self._resource_origin = f"{split.scheme}://{split.netloc}" @property def verifier(self) -> AuthplaneResource: @@ -41,24 +126,61 @@ def verifier(self) -> AuthplaneResource: return self._verifier async def verify_token(self, token: str) -> AccessToken | None: - """Validate a JWT and return an MCP AccessToken. + """Validate a JWT and return an MCP ``AccessToken``. - Called by the MCP server once per authenticated request. Delegates - to ``AuthplaneResource.verify()`` for all validation, then maps the - resulting ``VerifiedClaims`` to an MCP ``AccessToken``. + Pulls the active HTTP request to build a per-request + :class:`DPoPRequestContext` (RFC 9449 §7.1) and to scope the + verify-task cache. The cache hit path re-awaits the original + :class:`asyncio.Task`, so a cached :class:`AuthplaneError` + re-raises with the same type on every call within the request — + the ``AuthplaneError → None`` translation happens once here, not + inside the cached coroutine, which keeps the cached failure + diagnosable. Args: - token: The raw JWT string (the server strips ``'Bearer '`` - before calling this method). + token: The raw JWT string (MCP strips the ``Bearer `` + prefix before calling this method). Returns: ``AccessToken`` on successful validation with ``token``, ``client_id``, ``scopes``, ``expires_at``, and ``resource`` - fields populated. Returns ``None`` on any validation failure - (the MCP server responds with 401). + fields populated. Returns ``None`` on any + ``AuthplaneError`` (MCP responds with 401). """ try: - claims = await self._verifier.verify(token) + request: Request | None = self._get_http_request() + except RuntimeError as exc: + # ``get_current_request`` (and FastMCP's ``get_http_request``) + # raise a bare ``RuntimeError("No active HTTP request found.")`` + # when called outside an HTTP request context (unit tests, + # background tasks, or — for the MCP SDK path — any request + # served without ``AuthplaneRequestContextMiddleware`` installed). + # Match the message to avoid silently degrading to + # ``dpop_request=None`` if a future framework change surfaces an + # unrelated ``RuntimeError`` from the dependency — that would + # re-introduce the silent-pass this PR fixed (PRM advertising + # DPoP-required while the verifier never sees a proof). Drop + # this narrow when both upstream surfaces adopt a typed + # exception. + if "No active HTTP request" not in str(exc): + raise + request = None + + try: + if request is None: + claims = await self._verifier.verify(token, dpop_request=None) + else: + cache = get_or_create_verify_cache(request) + task = cache.get(token) + if task is None: + # No await between cache miss and cache write — concurrent + # verify_token(token) calls on the same loop cannot race. + dpop_request = self._build_dpop_request_context(request) + task = asyncio.create_task( + self._verifier.verify(token, dpop_request=dpop_request) + ) + cache[token] = task + claims = await task except AuthplaneError as error: logger.debug( "authplane.token_verification_failed", @@ -77,3 +199,36 @@ async def verify_token(self, token: str) -> AccessToken | None: expires_at=claims.expires_at, resource=str(resource), ) + + def _build_dpop_request_context(self, request: Request) -> DPoPRequestContext: + """Build the per-request DPoP context. + + Always returns a context — the resource verifier inspects the + access token's ``cnf`` claim plus the context's ``proof`` to + decide whether DPoP enforcement applies. When the resource is + not configured for inbound DPoP, the verifier's Mode-3 path + rejects any DPoP signal regardless of what is passed here. + + Cross-SDK note: the TS sibling ``buildDpopRequestContext`` + returns ``undefined`` when no ``DPoP`` header is present; + Python intentionally always builds the context with + ``proof=None``. Both shapes are behaviorally equivalent in + the core verifier (Mode 3 path treats absent and ``None`` + proofs the same), but a DPoP-bound token with no proof + yields a more specific ``DPoPProofMissingError`` here + instead of ``DPoPBindingMismatchError``. The error-type + contract is pinned per language by design. + """ + # ``raw_request_path`` reads ``scope["raw_path"]`` to preserve + # percent-encoding for DPoP ``htu`` parity with the TS sibling. + # ``request.url.query`` is sourced from ``scope["query_string"]`` + # without percent-decoding, so it is already on-wire-safe. + url = f"{self._resource_origin}{raw_request_path(request)}" + query = request.url.query + if query: + url = f"{url}?{query}" + return BuiltDPoPRequestContext( + method=request.method.upper(), + url=url, + proof=read_dpop_header(request), + ) diff --git a/authplane-mcp/demo/mcpserver.py b/authplane-mcp/demo/mcpserver.py index 0db623d..56f6630 100644 --- a/authplane-mcp/demo/mcpserver.py +++ b/authplane-mcp/demo/mcpserver.py @@ -16,15 +16,13 @@ from mcp.server.auth.middleware.auth_context import get_access_token from mcp.server.fastmcp import FastMCP -from authplane_mcp import authplane_mcp_auth, require_scope +from authplane_mcp import authplane_mcp_auth, install_request_context, require_scope -if __name__ == "__main__": - import logging +GOOGLE_CALENDAR_RESOURCE_URI = "https://www.googleapis.com/calendar/v3" +GOOGLE_CALENDAR_SCOPE = "https://www.googleapis.com/auth/calendar" - logging.basicConfig( - level=logging.DEBUG, format="%(asctime)s %(name)s %(levelname)s %(message)s" - ) +async def main() -> None: load_dotenv(os.path.join(os.path.dirname(__file__), ".env")) resource = os.environ.get("RESOURCE_URL", "http://localhost:8080/mcp") @@ -38,33 +36,38 @@ dpop_pem = dpop_key.private_bytes(Encoding.PEM, PrivateFormat.PKCS8, NoEncryption()) dpop_provider = DPoPProvider(DPoPKeyMaterial.from_pem(dpop_pem)) - auth_result = asyncio.run( - authplane_mcp_auth( - issuer=os.environ.get("ISSUER_URL", "http://localhost:9000"), - resource=resource, - scopes=["tools/add", "tools/multiply", "tools/consent_demo"], - # Advertise scopes in the PRM so OAuth-discovery clients (Claude - # Code, Inspector, etc.) request them on token mint. Side effect: - # every request must carry all three scopes. See the docstring - # on ``enforce_scopes_on_all_requests`` for why this exists, and - # the demo README for what users see in the consent prompt. The - # per-tool ``require_scope()`` calls below intentionally stay — - # they are the granular pattern, become no-ops under request-level - # enforcement, and remain correct once the upstream SDK gains a - # separate "supported" field and this flag goes away. - enforce_scopes_on_all_requests=True, - dev_mode=True, # Enables local testing - dpop=dpop_provider, - as_credentials=ASCredentials( - client_id=client_id, - client_secret=os.environ["CLIENT_SECRET"], - ), - revocation_checker=IntrospectionRevocation(), - ) + auth_result = await authplane_mcp_auth( + issuer=os.environ.get("ISSUER_URL", "http://localhost:9000"), + resource=resource, + scopes=["tools/add", "tools/multiply", "tools/consent_demo"], + # Advertise scopes in the PRM so OAuth-discovery clients (Claude + # Code, Inspector, etc.) request them on token mint. Side effect: + # every request must carry all three scopes. See the docstring + # on ``enforce_scopes_on_all_requests`` for why this exists, and + # the demo README for what users see in the consent prompt. The + # per-tool ``require_scope()`` calls below intentionally stay — + # they are the granular pattern, become no-ops under request-level + # enforcement, and remain correct once the upstream SDK gains a + # separate "supported" field and this flag goes away. + enforce_scopes_on_all_requests=True, + dev_mode=True, # Enables local testing + dpop=dpop_provider, + as_credentials=ASCredentials( + client_id=client_id, + client_secret=os.environ["CLIENT_SECRET"], + ), + revocation_checker=IntrospectionRevocation(), ) mcp = FastMCP("Calculator Service", port=port, json_response=True, **auth_result) + # Required for inbound DPoP enforcement: publishes the active HTTP request + # on a ContextVar so the verifier can build a DPoPRequestContext and + # forward it to AuthplaneResource.verify. Without this call DPoP-bound + # requests fail closed (DPoPBindingMismatchError) — the misconfiguration + # surfaces as a 401, not as a silent bypass. + install_request_context(mcp) + @mcp.tool() async def add(a: float, b: float) -> float: """Add two numbers""" @@ -77,9 +80,6 @@ async def multiply(a: float, b: float) -> float: require_scope("tools/multiply") return a * b - GOOGLE_CALENDAR_RESOURCE_URI = "https://www.googleapis.com/calendar/v3" - GOOGLE_CALENDAR_SCOPE = "https://www.googleapis.com/auth/calendar" - @mcp.tool() async def consent_demo() -> dict[str, str]: """Exchange the inbound user token for a Google Calendar token via RFC 8693. @@ -113,5 +113,21 @@ async def consent_demo() -> dict[str, str]: "scope": downstream.scope or "", } - # mcp.run() starts its own event loop, so don't wrap it in asyncio.run() - mcp.run(transport="streamable-http") + # The adapter setup, server, and aclose() must share one event loop — + # auth_result holds async resources (locks, httpx pool, background JWKS + # refresh tasks) bound to the running loop. Using the async server entry + # point keeps everything on the same loop. + try: + await mcp.run_streamable_http_async() + finally: + await auth_result.aclose() + + +if __name__ == "__main__": + import logging + + logging.basicConfig( + level=logging.DEBUG, format="%(asctime)s %(name)s %(levelname)s %(message)s" + ) + + asyncio.run(main()) diff --git a/authplane-mcp/docs/user-guide.md b/authplane-mcp/docs/user-guide.md index d4acd1c..76b852b 100644 --- a/authplane-mcp/docs/user-guide.md +++ b/authplane-mcp/docs/user-guide.md @@ -374,15 +374,7 @@ When `fetch_settings` is provided, `dev_mode` is ignored for both metadata and J ## Resource Cleanup -`authplane_mcp_auth()` returns an `AuthplaneAuthResult` that holds background JWKS / metadata refresh tasks and an HTTP connection pool. For a long-running server that exits with the process, the README quickstart shape is sufficient — the OS reaps everything on exit: - -```python -auth_result = asyncio.run(authplane_mcp_auth(...)) -mcp = FastMCP("My Server", **auth_result) -mcp.run(transport="streamable-http") # starts its own event loop and blocks -``` - -`mcp.run()` starts its own event loop internally, so it cannot share a loop with `await auth_result.aclose()`. If you need explicit teardown (tests, or an embedded server that should release resources without exiting the process), drive the MCP SDK's async server entry point so the auth setup, server, and `aclose()` share one loop: +`authplane_mcp_auth()` returns an `AuthplaneAuthResult` that holds an HTTP connection pool and, once requests start flowing, background JWKS / metadata refresh tasks. All of these are bound to the event loop that ran `authplane_mcp_auth()` — `asyncio.Lock`, the `httpx.AsyncClient`, and any tasks spawned by the caches. The adapter setup, the MCP server, and `aclose()` must share one loop. That means driving the MCP SDK's async server entry point from inside `asyncio.run(main())`: ```python import asyncio @@ -398,7 +390,9 @@ async def main() -> None: asyncio.run(main()) ``` -`auth_result.aclose()` closes the underlying `AuthplaneClient`, cancels its background tasks, and releases connections. In test code, skipping it surfaces as leaked tasks, open sockets, and `ResourceWarning`. Each transport has an async equivalent: `run_streamable_http_async`, `run_sse_async`, `run_stdio_async`. +Each transport has an async equivalent: `run_streamable_http_async`, `run_sse_async`, `run_stdio_async`. Do not wrap only the setup in `asyncio.run(authplane_mcp_auth(...))` and then call the sync `mcp.run()` — `mcp.run()` starts a fresh event loop, the auth resources stay bound to the loop that was closed when the setup `asyncio.run()` returned, and the first request fails with cross-loop errors. + +`auth_result.aclose()` closes the underlying `AuthplaneClient`, cancels its background tasks, and releases connections. Skipping it on a long-running server that exits with the process is harmless (the OS reaps everything on exit); skipping it in tests or embedded servers surfaces as leaked tasks, open sockets, and `ResourceWarning`. ## Error Handling diff --git a/authplane-mcp/tests/test_auth.py b/authplane-mcp/tests/test_auth.py index 91456eb..3271bbf 100644 --- a/authplane-mcp/tests/test_auth.py +++ b/authplane-mcp/tests/test_auth.py @@ -68,7 +68,7 @@ def test_require_scope_raises_when_scopes_empty(): async def test_authplane_mcp_auth_returns_auth_result(): """Verify return type and AuthSettings fields.""" mock_client = MagicMock() - mock_client.resource = MagicMock(return_value=MagicMock()) + mock_client.resource = MagicMock(return_value=MagicMock(resource="https://api.example.com")) with patch("authplane_mcp.auth.AuthplaneClient") as mock_client_cls: mock_client_cls.create = AsyncMock(return_value=mock_client) @@ -104,7 +104,7 @@ async def test_authplane_mcp_auth_parameter_propagation(): dpop_provider = MagicMock(spec=DPoPProvider) mock_client = MagicMock() - mock_client.resource = MagicMock(return_value=MagicMock()) + mock_client.resource = MagicMock(return_value=MagicMock(resource="https://api.example.com")) with patch("authplane_mcp.auth.AuthplaneClient") as mock_client_cls: mock_client_cls.create = AsyncMock(return_value=mock_client) @@ -118,6 +118,7 @@ async def test_authplane_mcp_auth_parameter_propagation(): metadata_refresh_seconds=7200, cache_ttl_buffer_seconds=15.0, default_ttl_seconds=900.0, + cache_max_entries=500, circuit_breaker_threshold=7, circuit_breaker_cooldown_seconds=45.0, clock_skew_seconds=60, @@ -135,6 +136,7 @@ async def test_authplane_mcp_auth_parameter_propagation(): assert client_kwargs["metadata_refresh_seconds"] == 7200 assert client_kwargs["cache_ttl_buffer_seconds"] == 15.0 assert client_kwargs["default_ttl_seconds"] == 900.0 + assert client_kwargs["cache_max_entries"] == 500 assert client_kwargs["circuit_breaker_threshold"] == 7 assert client_kwargs["circuit_breaker_cooldown_seconds"] == 45.0 assert client_kwargs["fetch_settings"] is custom_fetch_settings @@ -152,7 +154,7 @@ async def test_authplane_mcp_auth_parameter_propagation(): async def test_authplane_mcp_auth_none_filtering(): """Verify that None values are NOT passed to AuthplaneClient.create or client.resource.""" mock_client = MagicMock() - mock_client.resource = MagicMock(return_value=MagicMock()) + mock_client.resource = MagicMock(return_value=MagicMock(resource="https://api.example.com")) with patch("authplane_mcp.auth.AuthplaneClient") as mock_client_cls: mock_client_cls.create = AsyncMock(return_value=mock_client) @@ -173,6 +175,7 @@ async def test_authplane_mcp_auth_none_filtering(): assert "metadata_refresh_seconds" not in client_kwargs assert "cache_ttl_buffer_seconds" not in client_kwargs assert "default_ttl_seconds" not in client_kwargs + assert "cache_max_entries" not in client_kwargs assert "circuit_breaker_threshold" not in client_kwargs assert "circuit_breaker_cooldown_seconds" not in client_kwargs @@ -186,7 +189,7 @@ async def test_authplane_mcp_auth_none_filtering(): async def test_authplane_mcp_auth_revocation_checker_default_is_none(): """When revocation_checker is not passed, None is forwarded (no revocation checking).""" mock_client = MagicMock() - mock_client.resource = MagicMock(return_value=MagicMock()) + mock_client.resource = MagicMock(return_value=MagicMock(resource="https://api.example.com")) with patch("authplane_mcp.auth.AuthplaneClient") as mock_client_cls: mock_client_cls.create = AsyncMock(return_value=mock_client) @@ -208,7 +211,7 @@ async def my_checker(claims: VerifiedClaims, raw_token: str) -> bool: return False mock_client = MagicMock() - mock_client.resource = MagicMock(return_value=MagicMock()) + mock_client.resource = MagicMock(return_value=MagicMock(resource="https://api.example.com")) with patch("authplane_mcp.auth.AuthplaneClient") as mock_client_cls: mock_client_cls.create = AsyncMock(return_value=mock_client) @@ -229,7 +232,7 @@ async def test_authplane_mcp_auth_as_credentials_passthrough(): creds = ASCredentials(client_id="client_id", client_secret="secret") mock_client = MagicMock() - mock_client.resource = MagicMock(return_value=MagicMock()) + mock_client.resource = MagicMock(return_value=MagicMock(resource="https://api.example.com")) with patch("authplane_mcp.auth.AuthplaneClient") as mock_client_cls: mock_client_cls.create = AsyncMock(return_value=mock_client) @@ -298,7 +301,7 @@ async def test_authplane_mcp_auth_scopes_not_enforced_globally(): per-tool enforcement via require_scope(). """ mock_client = MagicMock() - mock_client.resource = MagicMock(return_value=MagicMock()) + mock_client.resource = MagicMock(return_value=MagicMock(resource="https://api.example.com")) with patch("authplane_mcp.auth.AuthplaneClient") as mock_client_cls: mock_client_cls.create = AsyncMock(return_value=mock_client) @@ -324,7 +327,12 @@ async def test_authplane_mcp_auth_scopes_not_enforced_globally(): @pytest.mark.asyncio async def test_verify_token_non_authplane_error_propagates(): """Unexpected exceptions from AuthplaneResource.verify() propagate (HTTP 500).""" - mock_verifier = AsyncMock() + from unittest.mock import PropertyMock + + from authplane import AuthplaneResource + + mock_verifier = AsyncMock(spec=AuthplaneResource) + type(mock_verifier).resource = PropertyMock(return_value="https://api.example.com") mock_verifier.verify.side_effect = RuntimeError("unexpected") tv = AuthplaneTokenVerifier(mock_verifier) diff --git a/authplane-mcp/tests/test_request_context.py b/authplane-mcp/tests/test_request_context.py new file mode 100644 index 0000000..bc8b0e1 --- /dev/null +++ b/authplane-mcp/tests/test_request_context.py @@ -0,0 +1,201 @@ +"""Tests for the ASGI plumbing that publishes the active Request via ContextVar. + +The MCP SDK's ``TokenVerifier`` protocol has no per-request hook, so +``authplane-mcp`` ships :class:`AuthplaneRequestContextMiddleware` which +sets a ContextVar that :func:`get_current_request` reads from inside +``verify_token``. These tests pin the public behavior of that plumbing +end-to-end (middleware → ContextVar → lookup) so the DPoP path the +verifier relies on doesn't silently regress. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +import pytest +from mcp.server.fastmcp import FastMCP + +if TYPE_CHECKING: + from starlette.types import Message + +from authplane_mcp import ( + AuthplaneRequestContextMiddleware, + get_current_request, + install_request_context, +) +from authplane_mcp._request_context import ( + _current_request, # pyright: ignore[reportPrivateUsage] +) + + +def test_get_current_request_outside_scope_raises() -> None: + """Outside any ASGI scope, the lookup raises with the FastMCP-matching message. + + The verifier's narrow-on-message fallback catches + ``"No active HTTP request found."`` from both this lookup and from + FastMCP's ``get_http_request`` — keep the wording in sync so the + narrow doesn't bit-rot. + """ + # Ensure no leftover ContextVar from a previous test + assert _current_request.get() is None + with pytest.raises(RuntimeError, match=r"No active HTTP request found\."): + get_current_request() + + +@pytest.mark.asyncio +async def test_middleware_publishes_and_clears_request() -> None: + """During the ASGI call, ``get_current_request`` returns the active Request. + + Asserts the contextvar contract: set on entry, cleared on exit even + if downstream raises (the ``finally`` in the middleware is what + guarantees no cross-request leak). + """ + seen: dict[str, Any] = {} + + async def downstream(scope: Any, receive: Any, send: Any) -> None: + request = get_current_request() + seen["method"] = request.method + seen["path"] = request.url.path + await send({"type": "http.response.start", "status": 200, "headers": []}) + await send({"type": "http.response.body", "body": b""}) + + app = AuthplaneRequestContextMiddleware(downstream) + scope: dict[str, Any] = { + "type": "http", + "method": "POST", + "path": "/mcp/tools", + "raw_path": b"/mcp/tools", + "query_string": b"", + "headers": [], + "scheme": "http", + "server": ("testserver", 80), + "client": None, + "root_path": "", + "http_version": "1.1", + } + + async def receive() -> dict[str, Any]: + return {"type": "http.request", "body": b"", "more_body": False} + + sent: list[Message] = [] + + async def send(message: Message) -> None: + sent.append(message) + + await app(scope, receive, send) + + assert seen == {"method": "POST", "path": "/mcp/tools"} + # Cleared on exit so subsequent code outside the request context + # cannot accidentally pick up the stale Request. + assert _current_request.get() is None + + +@pytest.mark.asyncio +async def test_middleware_clears_contextvar_on_exception() -> None: + """Downstream raising must not leave the ContextVar populated. + + The middleware's ``finally`` is what guarantees the next request on + the same task / loop doesn't see a stale Request — exercise that + explicitly so a future edit doesn't accidentally drop the ``finally``. + """ + + async def boom(scope: Any, receive: Any, send: Any) -> None: + _ = get_current_request() # confirm it's set during the call + raise RuntimeError("downstream failure") + + app = AuthplaneRequestContextMiddleware(boom) + scope: dict[str, Any] = { + "type": "http", + "method": "GET", + "path": "/", + "raw_path": b"/", + "query_string": b"", + "headers": [], + "scheme": "http", + "server": ("testserver", 80), + "client": None, + "root_path": "", + "http_version": "1.1", + } + + async def receive() -> dict[str, Any]: + return {"type": "http.request", "body": b"", "more_body": False} + + async def send(_: Message) -> None: + return None + + with pytest.raises(RuntimeError, match="downstream failure"): + await app(scope, receive, send) + + assert _current_request.get() is None + + +@pytest.mark.asyncio +async def test_middleware_skips_non_http_scope() -> None: + """Lifespan / websocket scopes are passed through without touching the ContextVar. + + Setting the ContextVar for non-HTTP scopes would not break anything + today, but the middleware is documented as HTTP-only and a future + reader should be able to trust that. + """ + called = False + + async def downstream(scope: Any, receive: Any, send: Any) -> None: + nonlocal called + called = True + # ContextVar should be untouched for non-HTTP scopes. + assert _current_request.get() is None + + async def receive() -> Message: + return {"type": "lifespan.startup"} + + async def send(_: Message) -> None: + return None + + app = AuthplaneRequestContextMiddleware(downstream) + await app({"type": "lifespan"}, receive, send) + assert called + assert _current_request.get() is None + + +# --------------------------------------------------------------------------- +# install_request_context — monkeypatches FastMCP.streamable_http_app +# --------------------------------------------------------------------------- + + +def test_install_request_context_wraps_streamable_http_app() -> None: + """The helper installs the middleware on the Starlette app FastMCP builds. + + ``FastMCP`` wires its middleware list internally with no public hook, + so the helper patches ``mcp.streamable_http_app`` to add ours after + the fact. Asserting on ``app.user_middleware`` membership is the + most direct way to verify the install without spinning up a server. + """ + mcp: FastMCP[Any] = FastMCP("test") + install_request_context(mcp) + app = mcp.streamable_http_app() + middleware_classes = [m.cls for m in app.user_middleware] + assert AuthplaneRequestContextMiddleware in middleware_classes + # Must be the outermost user middleware (added via ``add_middleware`` → + # inserted at index 0) so it sets the ContextVar before MCP's + # ``AuthenticationMiddleware`` runs ``verify_token``. + assert app.user_middleware[0].cls is AuthplaneRequestContextMiddleware + + +def test_install_request_context_is_idempotent() -> None: + """A second install on the same FastMCP must be a no-op. + + Without a guard, repeated installs chain wrappers: the inner + middleware's ``ContextVar.reset`` then fires against a token created + by the outer wrapper, raising ``RuntimeError: was created in + a different Context`` at request time. The guard keeps a single + middleware entry on the Starlette app regardless of how many times + the helper is called. + """ + mcp: FastMCP[Any] = FastMCP("test") + install_request_context(mcp) + install_request_context(mcp) + install_request_context(mcp) + app = mcp.streamable_http_app() + middleware_classes = [m.cls for m in app.user_middleware] + assert middleware_classes.count(AuthplaneRequestContextMiddleware) == 1 diff --git a/authplane-mcp/tests/test_verifier.py b/authplane-mcp/tests/test_verifier.py index 43ae1f2..88b2f71 100644 --- a/authplane-mcp/tests/test_verifier.py +++ b/authplane-mcp/tests/test_verifier.py @@ -1,14 +1,27 @@ import logging from dataclasses import replace -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, PropertyMock import pytest -from authplane import AuthplaneError, TokenExpiredError, VerifiedClaims +from authplane import AuthplaneError, AuthplaneResource, TokenExpiredError, VerifiedClaims from mcp.server.auth.provider import AccessToken from authplane_mcp.verifier import AuthplaneTokenVerifier +def _make_resource_mock(resource: str = "https://api.example.com") -> AsyncMock: + """Build an ``AuthplaneResource`` mock with the resource URI pinned. + + The verifier's ``__init__`` now reads ``verifier.resource`` (a string + URI) to reconstruct the DPoP ``htu`` origin, so bare ``AsyncMock()`` + is no longer enough — the ``resource`` property must be a real + string. + """ + mock = AsyncMock(spec=AuthplaneResource) + type(mock).resource = PropertyMock(return_value=resource) + return mock + + @pytest.fixture def valid_claims() -> VerifiedClaims: return VerifiedClaims( @@ -27,7 +40,7 @@ def valid_claims() -> VerifiedClaims: @pytest.mark.asyncio async def test_verify_token_success(valid_claims: VerifiedClaims) -> None: - mock_verifier = AsyncMock() + mock_verifier = _make_resource_mock() mock_verifier.verify.return_value = valid_claims adapter = AuthplaneTokenVerifier(mock_verifier) @@ -40,24 +53,26 @@ async def test_verify_token_success(valid_claims: VerifiedClaims) -> None: assert result.expires_at == 1700000000 assert result.resource == "https://api.example.com" - mock_verifier.verify.assert_awaited_once_with("valid_jwt") + # ``verify`` now receives ``dpop_request=None`` outside of a request context + # (no active ASGI middleware running), but is still called exactly once. + mock_verifier.verify.assert_awaited_once_with("valid_jwt", dpop_request=None) @pytest.mark.asyncio async def test_verify_token_failure() -> None: - mock_verifier = AsyncMock() + mock_verifier = _make_resource_mock() mock_verifier.verify.side_effect = AuthplaneError("Invalid token") adapter = AuthplaneTokenVerifier(mock_verifier) result = await adapter.verify_token("invalid_jwt") assert result is None - mock_verifier.verify.assert_awaited_once_with("invalid_jwt") + mock_verifier.verify.assert_awaited_once_with("invalid_jwt", dpop_request=None) @pytest.mark.asyncio async def test_verify_token_with_list_audience(valid_claims: VerifiedClaims) -> None: - mock_verifier = AsyncMock() + mock_verifier = _make_resource_mock() # Simulate audience being a list from a mocked verifier. claims_with_list_aud: VerifiedClaims = replace( valid_claims, audience=["https://api.example.com"] @@ -74,7 +89,7 @@ async def test_verify_token_with_list_audience(valid_claims: VerifiedClaims) -> def test_verifier_property() -> None: """verifier property exposes the underlying AuthplaneResource.""" - mock_verifier = AsyncMock() + mock_verifier = _make_resource_mock() adapter = AuthplaneTokenVerifier(mock_verifier) assert adapter.verifier is mock_verifier @@ -87,7 +102,7 @@ async def test_verify_token_failure_logs_typed_error_at_debug( # contract requires it), but operators must see *which* typed error caused # the 401 in their logs. Debug-level because invalid tokens are expected # steady-state and shouldn't page on-call. - mock_verifier = AsyncMock() + mock_verifier = _make_resource_mock() mock_verifier.verify.side_effect = TokenExpiredError("expired at 2026") adapter = AuthplaneTokenVerifier(mock_verifier) @@ -111,7 +126,7 @@ async def test_verify_token_failure_silent_above_debug( ) -> None: # At default (INFO) level the structured event must not surface — invalid # tokens are too common in steady state to log at info. - mock_verifier = AsyncMock() + mock_verifier = _make_resource_mock() mock_verifier.verify.side_effect = AuthplaneError("bad token") adapter = AuthplaneTokenVerifier(mock_verifier) diff --git a/authplane-mcp/tests/test_verifier_dpop_cache.py b/authplane-mcp/tests/test_verifier_dpop_cache.py new file mode 100644 index 0000000..809011e --- /dev/null +++ b/authplane-mcp/tests/test_verifier_dpop_cache.py @@ -0,0 +1,492 @@ +"""DPoP context forwarding + per-request verify cache tests for authplane-mcp. + +Mirrors ``authplane-fastmcp/tests/test_verifier_dpop_cache.py`` so the +two adapters' DPoP-enforcement contracts stay observably identical. The +only adapter-specific seam is the ``get_http_request`` lookup: FastMCP +ships ``fastmcp.server.dependencies.get_http_request``; the MCP SDK has +no equivalent, so this package ships +:class:`AuthplaneRequestContextMiddleware` and the verifier reads from +:func:`authplane_mcp.get_current_request`. The tests bypass that +plumbing by injecting a fake ``get_http_request`` directly. + +Covers: + +* ``verify_token`` must build a ``DPoPRequestContext`` from the active + request and forward it to ``AuthplaneResource.verify``. Without it, + a resource configured for ``inbound_dpop=required=True`` silently + accepts bearer-only requests even though PRM advertises the + requirement — the same silent-pass that landed first in the FastMCP + sibling. +* Repeated ``verify_token`` calls within the same request collapse + onto a single underlying ``verify`` so the inbound replay store + cannot see the same proof ``jti`` twice within one request. Distinct + requests get distinct caches so cross-request replay protection is + preserved. +""" + +from __future__ import annotations + +import time +from typing import Any +from unittest.mock import AsyncMock, PropertyMock + +import pytest +from authplane import AuthplaneResource, DPoPReplayDetectedError, VerifiedClaims +from authplane._dpop_adapter import get_or_create_verify_cache +from starlette.requests import Request + +from authplane_mcp import AuthplaneTokenVerifier + + +def _make_request( + *, + method: str = "POST", + path: str = "/mcp", + query: str = "", + host: str = "testserver", + headers: dict[str, str] | None = None, +) -> Request: + """Synthesize a Starlette ``Request`` suitable for driving the verifier. + + The verifier reconstructs ``htu`` from the operator-configured + resource origin, so the ``host`` here is intentionally divergent — + the tests assert the request's ``Host`` header is *not* what ends up + in the proof binding. + """ + raw_headers: list[tuple[bytes, bytes]] = [] + if headers: + for key, value in headers.items(): + raw_headers.append((key.lower().encode("latin-1"), value.encode("latin-1"))) + scope: dict[str, Any] = { + "type": "http", + "method": method, + "path": path, + "raw_path": path.encode("latin-1"), + "query_string": query.encode("latin-1"), + "headers": raw_headers, + "scheme": "http", + "server": (host, 80), + "client": None, + "root_path": "", + "http_version": "1.1", + } + return Request(scope) + + +def _mock_verifier( + *, + resource: str = "https://api.example.com/mcp", + claims: VerifiedClaims | None = None, + raise_exc: BaseException | None = None, +) -> AsyncMock: + """Build an ``AuthplaneResource`` mock with the verify contract pinned. + + Returning a ``VerifiedClaims`` (or raising) on every call lets the + cache tests count invocations via ``mock.verify.call_count``. + """ + if claims is None and raise_exc is None: + now = int(time.time()) + claims = VerifiedClaims( + sub="user_123", + client_id="client_456", + scopes=("tools/query",), + issuer="https://auth.example.com", + audience=("https://api.example.com",), + expires_at=now + 3600, + issued_at=now, + jti="token-id-123", + kid="test-key-1", + raw={ + "iss": "https://auth.example.com", + "aud": "https://api.example.com", + "sub": "user_123", + "client_id": "client_456", + "scope": "tools/query", + "exp": now + 3600, + "nbf": now, + "iat": now, + "jti": "token-id-123", + }, + ) + mock = AsyncMock(spec=AuthplaneResource) + type(mock).resource = PropertyMock(return_value=resource) + + async def verify(token: str, *, dpop_request: object | None = None) -> VerifiedClaims: + _ = token, dpop_request + if raise_exc is not None: + raise raise_exc + assert claims is not None + return claims + + mock.verify = AsyncMock(side_effect=verify) + return mock + + +def _make_token_verifier( + mock_resource: AsyncMock, request: Request | None +) -> AuthplaneTokenVerifier: + """Token verifier wired to a fake ``get_http_request`` for tests. + + ``request=None`` simulates ``RuntimeError("No active HTTP request found.")`` + that :func:`authplane_mcp.get_current_request` raises when the ASGI + middleware was not installed (or the call is outside any request). + """ + + def fake_get_http_request() -> Request: + if request is None: + raise RuntimeError("No active HTTP request found.") + return request + + return AuthplaneTokenVerifier(mock_resource, get_http_request=fake_get_http_request) + + +# --------------------------------------------------------------------------- +# DPoP context is built and forwarded to AuthplaneResource.verify +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_dpop_context_forwarded_to_verify() -> None: + """verify_token must pass a DPoPRequestContext to verify(). + + Before this change ``authplane-mcp`` carried the same silent-pass + bug the FastMCP sibling shipped: ``verify(token)`` with no + ``dpop_request=`` made any ``inbound_dpop=required=True`` config a + no-op even though PRM advertised the requirement. + """ + mock = _mock_verifier() + request = _make_request( + method="POST", + path="/mcp", + headers={"DPoP": "fake.proof.jwt"}, + ) + verifier = _make_token_verifier(mock, request) + + result = await verifier.verify_token("valid_token") + + assert result is not None + assert mock.verify.await_count == 1 + kwargs = mock.verify.await_args.kwargs + ctx = kwargs["dpop_request"] + assert ctx is not None + assert ctx.method == "POST" + assert ctx.url == "https://api.example.com/mcp" + assert ctx.proof == "fake.proof.jwt" + + +@pytest.mark.asyncio +async def test_dpop_context_proof_none_when_header_absent() -> None: + """Context is still built (so htm/htu reach the verifier), proof=None. + + Lets the core verifier's required-mode path fail closed when the + access token is DPoP-bound but the client did not present a proof. + """ + mock = _mock_verifier() + request = _make_request(headers={"authorization": "Bearer xyz"}) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("valid_token") + + ctx = mock.verify.await_args.kwargs["dpop_request"] + assert ctx is not None + assert ctx.proof is None + assert ctx.method == "POST" + assert ctx.url == "https://api.example.com/mcp" + + +@pytest.mark.asyncio +async def test_dpop_context_header_lookup_is_case_insensitive() -> None: + """RFC 7230 says HTTP header names are case-insensitive; verify the wire.""" + mock = _mock_verifier() + request = _make_request(headers={"dpop": "lower.case.proof"}) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("valid_token") + + assert mock.verify.await_args.kwargs["dpop_request"].proof == "lower.case.proof" + + +@pytest.mark.asyncio +async def test_htu_origin_from_configured_resource_not_host_header() -> None: + """htu's origin comes from the configured resource, never from Host. + + Mirrors the TS sibling: an upstream that controls the Host / + X-Forwarded-Proto headers must not be able to decide which htu the + DPoP proof is validated against. + """ + mock = _mock_verifier(resource="https://api.example.com/mcp") + request = _make_request( + host="attacker.example.com", + headers={ + "DPoP": "p", + "host": "attacker.example.com", + "x-forwarded-proto": "http", + }, + ) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("valid_token") + + ctx = mock.verify.await_args.kwargs["dpop_request"] + assert ctx.url.startswith("https://api.example.com") + assert "attacker" not in ctx.url + + +@pytest.mark.asyncio +async def test_htu_includes_request_path_and_query() -> None: + """htu = origin + path + (?query) per RFC 9449 §4.2.""" + mock = _mock_verifier() + request = _make_request(path="/mcp/tools", query="x=1&y=2", headers={"DPoP": "p"}) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("valid_token") + + assert ( + mock.verify.await_args.kwargs["dpop_request"].url + == "https://api.example.com/mcp/tools?x=1&y=2" + ) + + +@pytest.mark.asyncio +async def test_htu_preserves_percent_encoded_path_from_raw_path() -> None: + """htu uses ``scope['raw_path']`` so percent-encoding survives. + + ASGI populates ``scope['path']`` as the percent-decoded path, but the + DPoP proof was signed over the on-wire (still-encoded) URL. The TS + sibling reads ``IncomingMessage.url`` (raw bytes), so reading + ``raw_path`` here keeps cross-SDK proof binding identical. + """ + mock = _mock_verifier() + # Decoded path: "/mcp/users/a/b" ; raw: "/mcp/users/a%2Fb" — a client + # that signed the encoded form must bind against the encoded htu. + raw_headers: list[tuple[bytes, bytes]] = [(b"dpop", b"p")] + scope: dict[str, Any] = { + "type": "http", + "method": "POST", + "path": "/mcp/users/a/b", + "raw_path": b"/mcp/users/a%2Fb", + "query_string": b"", + "headers": raw_headers, + "scheme": "http", + "server": ("testserver", 80), + "client": None, + "root_path": "", + "http_version": "1.1", + } + request = Request(scope) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("valid_token") + + assert ( + mock.verify.await_args.kwargs["dpop_request"].url + == "https://api.example.com/mcp/users/a%2Fb" + ) + + +@pytest.mark.asyncio +async def test_htu_falls_back_to_decoded_path_when_raw_path_absent() -> None: + """Servers that omit ``raw_path`` still get a sensible htu. + + Some ASGI servers (and synthetic test scopes) do not populate + ``raw_path``. The helper must fall back to ``request.url.path`` + rather than blowing up — for the no-percent-encoding case the two + are equal anyway. + """ + mock = _mock_verifier() + scope: dict[str, Any] = { + "type": "http", + "method": "POST", + "path": "/mcp", + "query_string": b"", + "headers": [(b"dpop", b"p")], + "scheme": "http", + "server": ("testserver", 80), + "client": None, + "root_path": "", + "http_version": "1.1", + } + request = Request(scope) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("valid_token") + + assert mock.verify.await_args.kwargs["dpop_request"].url == "https://api.example.com/mcp" + + +@pytest.mark.asyncio +async def test_no_http_request_context_falls_back_to_no_dpop() -> None: + """Outside an HTTP request, the verifier still works (unit-test ergonomics). + + :func:`authplane_mcp.get_current_request` raises ``RuntimeError`` when + called without an active request set by the middleware — the adapter + must verify without a DPoP context rather than throw. + """ + mock = _mock_verifier() + verifier = _make_token_verifier(mock, request=None) + + result = await verifier.verify_token("valid_token") + + assert result is not None + assert mock.verify.await_args.kwargs["dpop_request"] is None + + +# --------------------------------------------------------------------------- +# Per-request verify cache +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_repeat_verify_token_collapses_onto_one_verify() -> None: + """Two verify_token calls in one request → one underlying verify call. + + The cache is what protects DPoP-required mode from a + DPoPReplayDetected error in case any future framework change starts + invoking ``verify_token`` more than once per HTTP request. + """ + mock = _mock_verifier() + request = _make_request(headers={"DPoP": "p"}) + verifier = _make_token_verifier(mock, request) + + first = await verifier.verify_token("the_token") + second = await verifier.verify_token("the_token") + + assert first is not None + assert second is not None + assert mock.verify.await_count == 1 + + +@pytest.mark.asyncio +async def test_cache_is_scoped_per_request() -> None: + """Distinct requests do not share the cache; replay protection survives.""" + mock = _mock_verifier() + req_one = _make_request(headers={"DPoP": "p1"}) + req_two = _make_request(headers={"DPoP": "p2"}) + + requests: list[Request] = [req_one, req_two] + + def pop_request() -> Request: + return requests.pop(0) + + verifier = AuthplaneTokenVerifier(mock, get_http_request=pop_request) + + await verifier.verify_token("t") + await verifier.verify_token("t") + + assert mock.verify.await_count == 2 + + +@pytest.mark.asyncio +async def test_cache_keyed_by_token_not_by_request_slot() -> None: + """Different tokens within one request get verified independently. + + A scenario that never arises on MCP's standard HTTP path + (``BearerAuthBackend`` extracts one ``Authorization`` per request), + but keying by token is cheap and removes a footgun the TS adapter + technically carries (a different ``verify_token(otherToken)`` call + inside one request would reuse the first call's result there). + """ + mock = _mock_verifier() + request = _make_request(headers={"DPoP": "p"}) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("token_a") + await verifier.verify_token("token_b") + + assert mock.verify.await_count == 2 + + +@pytest.mark.asyncio +async def test_cached_exception_reraises_typed_on_replay_within_request() -> None: + """Acceptance: cached typed error re-raises on replay; not masked to None only the 2nd time. + + Both calls of ``verify_token`` map AuthplaneError → None, but the + underlying cached task carries the typed exception unchanged so an + operator awaiting the slot on ``request.state`` directly still + sees the original DPoPReplayDetectedError. This is what keeps the + failure diagnosable rather than collapsing to a generic 401 reason. + """ + err = DPoPReplayDetectedError("DPoP proof jti has already been seen") + mock = _mock_verifier(raise_exc=err) + request = _make_request(headers={"DPoP": "p"}) + verifier = _make_token_verifier(mock, request) + + first = await verifier.verify_token("t") + second = await verifier.verify_token("t") + + assert first is None + assert second is None + assert mock.verify.await_count == 1 + + cache = get_or_create_verify_cache(request) + cached_task = cache["t"] + with pytest.raises(DPoPReplayDetectedError): + await cached_task + + +@pytest.mark.asyncio +async def test_request_state_cache_stashes_task() -> None: + """The cache slot is populated with an asyncio Task keyed by the access token.""" + import asyncio + + mock = _mock_verifier() + request = _make_request(headers={"DPoP": "p"}) + verifier = _make_token_verifier(mock, request) + + await verifier.verify_token("t") + + cache = get_or_create_verify_cache(request) + assert "t" in cache + assert isinstance(cache["t"], asyncio.Task) + + +@pytest.mark.asyncio +async def test_no_request_context_means_no_cache_either() -> None: + """Outside a request: each call hits verify(); nothing is cached anywhere.""" + mock = _mock_verifier() + verifier = _make_token_verifier(mock, request=None) + + await verifier.verify_token("t") + await verifier.verify_token("t") + + assert mock.verify.await_count == 2 + + +# --------------------------------------------------------------------------- +# Constructor and request-lookup guards (mirrors fastmcp hardening) +# --------------------------------------------------------------------------- + + +def test_init_rejects_non_string_resource() -> None: + """A mis-wired mock (no ``spec``) would silently corrupt htu reconstruction. + + ``urlsplit`` accepts a ``MagicMock`` without raising and produces a + bogus ``_resource_origin`` like ``MagicMock://MagicMock`` — guard + against this by failing fast in ``__init__``. + """ + mock = AsyncMock(spec=AuthplaneResource) + type(mock).resource = PropertyMock(return_value=object()) # not a str + + with pytest.raises(TypeError, match=r"verifier\.resource must be a str URI"): + AuthplaneTokenVerifier(mock) + + +@pytest.mark.asyncio +async def test_unrelated_runtimeerror_from_get_http_request_propagates() -> None: + """Only the documented 'No active HTTP request found.' is swallowed. + + A future change surfacing an unrelated ``RuntimeError`` from + :func:`get_current_request` (ContextVar machinery breakage, etc.) + must not be masked as ``dpop_request=None`` — that would silently + re-introduce the no-proof-forwarded silent-pass this module fixed. + """ + mock = _mock_verifier() + + def broken_get_http_request() -> Request: + raise RuntimeError("ContextVar internal: scope not initialized") + + verifier = AuthplaneTokenVerifier(mock, get_http_request=broken_get_http_request) + + with pytest.raises(RuntimeError, match="scope not initialized"): + await verifier.verify_token("t") + assert mock.verify.await_count == 0 diff --git a/authplane/__init__.py b/authplane/__init__.py index 373586b..fec76e7 100644 --- a/authplane/__init__.py +++ b/authplane/__init__.py @@ -11,6 +11,7 @@ # Client # Authentication from .auth_provider import AuthProvider, ClientCredentialsProvider +from .cache import TokenCache from .client import AuthplaneClient from .credentials import ASCredentials from .dpop import ( @@ -109,6 +110,7 @@ "ProtocolError", "RevocationChecker", "ServerError", + "TokenCache", "TokenExpiredError", "TokenMissingError", "TokenRevokedError", diff --git a/authplane/_dpop_adapter.py b/authplane/_dpop_adapter.py new file mode 100644 index 0000000..71da8e8 --- /dev/null +++ b/authplane/_dpop_adapter.py @@ -0,0 +1,153 @@ +"""Shared DPoP-context plumbing for the MCP and FastMCP adapters. + +The ``authplane-mcp`` and ``authplane-fastmcp`` packages both bridge +``AuthplaneResource.verify`` to a Starlette-based ``TokenVerifier``. +They each need the same four pieces of glue: + +* a concrete ``DPoPRequestContext`` shape built from the active request, +* a case-insensitive ``DPoP`` header reader, +* a path reader that preserves the on-wire percent-encoding (so + ``htu`` binds identically against the TS sibling), and +* a per-request verify-task cache anchored on ``request.state`` so + repeated ``verify_token`` invocations within one HTTP request do + not re-enter the inbound DPoP replay store. + +These live here so the two adapters do not drift. The module is +underscore-prefixed and intentionally not re-exported from +``authplane.__init__``; nothing outside the adapter packages should +import from it. + +The core ``authplane-sdk`` wheel does *not* take a runtime dependency +on Starlette. The helpers below duck-type the few attributes they +need; the ``_RequestLike`` Protocol pins that contract structurally +so the adapter packages can pass ``starlette.requests.Request`` and +type-check without forcing Starlette into the core's import graph. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Protocol + +if TYPE_CHECKING: + import asyncio + + from .verifier import VerifiedClaims + + +__all__ = [ + "BuiltDPoPRequestContext", + "VerifyTaskCache", + "get_or_create_verify_cache", + "raw_request_path", + "read_dpop_header", +] + + +REQ_STATE_KEY = "_authplane_verify_tasks" +"""Attribute name on ``request.state`` that holds the per-request verify-task cache. + +Stringly-typed on purpose: ``request.state`` is shared per ASGI *scope* +(not per ``Request`` instance), so a ``WeakKeyDictionary[Request, ...]`` +would split the cache across middleware-built and handler-built +``Request`` objects pointing at the same scope. +""" + + +VerifyTaskCache = dict[str, "asyncio.Task[VerifiedClaims]"] + + +class _HeadersLike(Protocol): + """Minimal slice of ``starlette.datastructures.Headers``.""" + + def get(self, key: str, default: str | None = ...) -> str | None: ... + + +class _URLLike(Protocol): + """Minimal slice of ``starlette.datastructures.URL``.""" + + @property + def path(self) -> str: ... + + +class _RequestLike(Protocol): + """Structural subset of ``starlette.requests.Request`` consumed here. + + Defined as a Protocol so this module does not have to import + Starlette — keeping it out of the core SDK's dependency graph. + The adapters call these helpers with the real + ``starlette.requests.Request`` and it satisfies the protocol + structurally. + """ + + @property + def headers(self) -> _HeadersLike: ... + + @property + def scope(self) -> dict[str, object]: ... + + @property + def state(self) -> object: ... + + @property + def url(self) -> _URLLike: ... + + +class BuiltDPoPRequestContext: + """Concrete ``DPoPRequestContext`` built from the active HTTP request. + + Structural conformance to ``authplane.DPoPRequestContext`` is all + the core verifier checks. ``__slots__`` keeps the per-request + construction cost negligible. + """ + + __slots__ = ("method", "proof", "url") + + def __init__(self, method: str, url: str, proof: str | None) -> None: + self.method = method + self.url = url + self.proof = proof + + +def read_dpop_header(request: _RequestLike) -> str | None: + """Read the ``DPoP`` request header (case-insensitive, first value). + + Starlette's ``Headers.get`` already does case-insensitive lookup + and returns the first occurrence when a header is repeated. Strict + rejection of repeated ``DPoP`` headers (RFC 9449 §4.3 #1) is + tracked separately and is intentionally not enforced here so this + layer does not overlap with that work. + """ + return request.headers.get("dpop") + + +def raw_request_path(request: _RequestLike) -> str: + """Return the request path with percent-encoding preserved. + + ASGI populates ``scope["path"]`` as the percent-decoded path, but + the client signed its DPoP ``htu`` over the on-wire (percent-encoded) + target. Prefer ``scope["raw_path"]`` (raw bytes) so a path + containing e.g. ``%2F`` binds identically here and in the TS + sibling, which builds ``htu`` from ``IncomingMessage.url``. Falls + back to the decoded path for the rare ASGI server that omits + ``raw_path``. + """ + raw = request.scope.get("raw_path") + if isinstance(raw, (bytes, bytearray)): + return bytes(raw).decode("latin-1") + return request.url.path + + +def get_or_create_verify_cache(request: _RequestLike) -> VerifyTaskCache: + """Return the per-request verify-task cache, creating it on first access. + + Lives on ``request.state`` so it is scoped to the ASGI request and + disappears with it; cross-request replay protection is preserved. + The adapters' tests use this accessor to inspect the cache without + manipulating ``request.state`` or the stringly-typed slot directly. + """ + state = request.state + cache: VerifyTaskCache | None = getattr(state, REQ_STATE_KEY, None) + if cache is None: + cache = {} + setattr(state, REQ_STATE_KEY, cache) + return cache diff --git a/authplane/cache.py b/authplane/cache.py index 826feaf..8505cbc 100644 --- a/authplane/cache.py +++ b/authplane/cache.py @@ -1,6 +1,7 @@ -"""Token cache with TTL buffer for client_credentials tokens.""" +"""Token cache with TTL buffer and bounded LRU for client_credentials tokens.""" import time +from collections import OrderedDict from dataclasses import dataclass @@ -16,29 +17,72 @@ class CacheEntry: class TokenCache: - """In-memory token cache with configurable TTL buffer. + """In-memory token cache with TTL buffer and bounded LRU eviction. - Tokens are evicted `ttl_buffer_seconds` before their actual expiry - to avoid using tokens that are about to expire. + Tokens are evicted ``ttl_buffer_seconds`` before their actual expiry to + avoid using tokens that are about to expire. + + The cache is bounded by ``max_entries`` (default 10 000) and evicts the + least-recently-used entry on overflow. Token-exchange cache keys are + high-cardinality because the subject token is part of the key, so + never-re-read keys would otherwise accumulate without bound. + + Recency is tracked via :class:`collections.OrderedDict` — every ``get`` + that hits an entry calls ``move_to_end`` so the iterator's first key + is always the LRU victim. """ + DEFAULT_MAX_ENTRIES = 10_000 + """Default cap on cached entries.""" + def __init__( self, ttl_buffer_seconds: float = 30.0, default_ttl_seconds: float = 3600.0, + max_entries: int = DEFAULT_MAX_ENTRIES, ) -> None: + # Strict `type() is int` rather than `isinstance(..., int)` so that + # `bool` (a subclass of `int`) is rejected — a stray `True` would + # otherwise silently produce a cap-1 cache. Catches runtime-typed + # inputs from JSON config that bypass the static type check. + if type(max_entries) is not int or max_entries <= 0: + raise ValueError(f"max_entries must be a positive integer, got {max_entries!r}") self._ttl_buffer = ttl_buffer_seconds self._default_ttl = default_ttl_seconds - self._entries: dict[str, CacheEntry] = {} + self._max_entries = max_entries + self._entries: OrderedDict[str, CacheEntry] = OrderedDict() + + def __len__(self) -> int: + """Number of stored entries — useful for tests and operator alerts. + + Steady-state size tracking ``max_entries`` signals the cap is too low. + """ + return len(self._entries) + + @property + def max_entries(self) -> int: + """The configured cap on stored entries. + + Exposed so operators can correlate ``len(cache)`` against the cap + without having to re-thread the constructor argument through their + own config layer. + """ + return self._max_entries def get(self, key: str) -> CacheEntry | None: - """Get a cached entry if it exists and hasn't expired.""" + """Get a cached entry if it exists and hasn't expired. + + On hit, bumps the entry to MRU so the next overflow eviction + targets a colder key. + """ entry = self._entries.get(key) if entry is None: return None if time.monotonic() >= entry.expires_at: del self._entries[key] return None + # Bump to MRU. + self._entries.move_to_end(key) return entry def set( @@ -53,6 +97,9 @@ def set( ttl = (expires_in if expires_in > 0 else self._default_ttl) - self._ttl_buffer if ttl <= 0: return + # `move_to_end` after insertion in case the key already exists — + # a re-set is treated as a touch (the entry the caller just wrote + # is by definition the most-recently-used). self._entries[key] = CacheEntry( access_token=access_token, token_type=token_type, @@ -60,6 +107,12 @@ def set( scope=scope, expires_at=time.monotonic() + ttl, ) + self._entries.move_to_end(key) + # Evict the LRU victim(s) until we're at-or-under the cap. The + # constructor pins ``max_entries``, so in practice this trims + # exactly one entry on overflow. + while len(self._entries) > self._max_entries: + self._entries.popitem(last=False) def delete(self, key: str) -> None: """Remove a cached entry.""" diff --git a/authplane/client.py b/authplane/client.py index 497303e..26bec7b 100644 --- a/authplane/client.py +++ b/authplane/client.py @@ -92,12 +92,46 @@ async def create( metadata_refresh_seconds: int = 3600, cache_ttl_buffer_seconds: float = 30.0, default_ttl_seconds: float = 3600.0, + cache_max_entries: int = TokenCache.DEFAULT_MAX_ENTRIES, circuit_breaker_threshold: int = 5, circuit_breaker_cooldown_seconds: float = 30.0, ) -> Self: """Create and initialize the client. Discovers AS metadata and starts JWKS background refresh. + + Args: + issuer: Authorization-server issuer URL (the prefix RFC 8414 metadata + is fetched from). Trailing slash is stripped. + auth: Client authentication for OAuth endpoints. Accepts either a raw + :class:`AuthProvider` or an :class:`ASCredentials` shorthand (which + is materialised as :class:`ClientCredentialsProvider`). + dpop: Optional :class:`DPoPProvider` for sender-constrained outbound + requests against the AS (token / introspection / revocation). + dev_mode: When True, relaxes SSRF and HTTPS-only fetch policies for + local development. Falls back to the ``AUTHPLANE_DEV_MODE`` + environment variable when omitted. + fetch_settings: Explicit :class:`FetchSettings` override. When None, + derived from ``dev_mode``. + jwks_refresh_seconds: Background JWKS refresh interval (must be > 0). + metadata_refresh_seconds: Background metadata refresh interval + (must be > 0). + cache_ttl_buffer_seconds: Safety margin subtracted from each token's + lifetime before the entry is considered expired. Same shape as + java-sdk ``TokenCacheConfig.ttlBufferSeconds`` and ts-sdk + ``TokenCache`` ctor. Default 30s. + default_ttl_seconds: Fallback lifetime applied when the AS response + omits ``expires_in``. Cross-SDK parity with java-sdk + ``TokenCacheConfig.defaultTtlSeconds``. Default 3600s. + cache_max_entries: Maximum number of cached tokens before + least-recently-used eviction kicks in. Default + :attr:`TokenCache.DEFAULT_MAX_ENTRIES` (10_000). Must be a + positive integer (``bool``/``float`` rejected — see + :class:`TokenCache`). + circuit_breaker_threshold: Consecutive AS failures before the + circuit opens. Default 5. + circuit_breaker_cooldown_seconds: Half-open probe interval after the + circuit trips. Default 30s. """ client = cls() client._issuer = issuer.rstrip("/") @@ -135,7 +169,11 @@ async def create( ) # Resilience - client._token_cache = TokenCache(cache_ttl_buffer_seconds, default_ttl_seconds) + client._token_cache = TokenCache( + cache_ttl_buffer_seconds, + default_ttl_seconds, + cache_max_entries, + ) client._circuit_breaker = CircuitBreaker( circuit_breaker_threshold, circuit_breaker_cooldown_seconds, diff --git a/authplane/verifier/claims.py b/authplane/verifier/claims.py index 1aa99b3..e85991c 100644 --- a/authplane/verifier/claims.py +++ b/authplane/verifier/claims.py @@ -2,7 +2,7 @@ from __future__ import annotations -from collections.abc import Mapping +from collections.abc import Iterable, Mapping from dataclasses import dataclass, field from types import MappingProxyType from typing import TYPE_CHECKING, Any, cast @@ -55,11 +55,45 @@ def has_scope(self, scope: str) -> bool: def require_scope(self, scope: str) -> None: """Raise InsufficientScopeError when the validated token lacks the scope.""" if not self.has_scope(scope): + available = list(self.scopes) if self.scopes else "(none)" raise InsufficientScopeError( - f"Token missing required scope '{scope}'. Token has scopes: {list(self.scopes)}", + f"Token missing required scope '{scope}'. Token has scopes: {available}", required_scopes=(scope,), ) + def require_scopes(self, scopes: Iterable[str]) -> None: + """Raise InsufficientScopeError unless the token carries every scope in ``scopes``. + + Empty input is a no-op — no required scopes ⇒ always satisfied. A + handler that calls ``claims.require_scopes(required)`` doesn't have to + special-case empty iterables. + + On failure the raised :class:`InsufficientScopeError` carries the full + requested tuple on ``required_scopes`` (not just the missing ones, so + adapters that surface ``scope="…"`` in the WWW-Authenticate challenge + keep emitting the complete required set), and its message names every + missing scope plus the scopes the token carries — rendered verbatim + into the RFC 6750 ``error_description``. + """ + # Materialise once: the caller may pass any iterable (generator, set, + # frozenset, etc.). We need to iterate twice — once to find missing + # entries and once to populate the error's ``required_scopes`` — and + # the raised error embeds the requested tuple in its public field, + # so the snapshot has to be ordered + indexable. + required = tuple(scopes) + if not required: + return + missing = [s for s in required if not self.has_scope(s)] + if not missing: + return + scope_word = "scopes" if len(missing) > 1 else "scope" + missing_quoted = ", ".join(repr(s) for s in missing) + available = list(self.scopes) if self.scopes else "(none)" + raise InsufficientScopeError( + f"Token missing required {scope_word} {missing_quoted}. Token has scopes: {available}", + required_scopes=required, + ) + def has_claim(self, key: str, value: Any = _MISSING) -> bool: """Check whether a claim exists and optionally matches an expected value.""" if key not in self.raw: diff --git a/llm.txt b/llm.txt index 721e51e..2eaf4a9 100644 --- a/llm.txt +++ b/llm.txt @@ -53,25 +53,34 @@ import asyncio from authplane_mcp import authplane_mcp_auth, require_scope from mcp.server.fastmcp import FastMCP -auth_result = asyncio.run(authplane_mcp_auth( - issuer="https://auth.company.com", - resource="https://mcp.company.com", - scopes=["tools/query"], -)) -mcp = FastMCP("My Server", json_response=True, **auth_result) - -@mcp.tool() -async def query(sql: str) -> str: - require_scope("tools/query") - ... - -mcp.run(transport="streamable-http") -# remember to ``await auth_result.aclose()`` on shutdown +async def main() -> None: + auth_result = await authplane_mcp_auth( + issuer="https://auth.company.com", + resource="https://mcp.company.com", + scopes=["tools/query"], + ) + mcp = FastMCP("My Server", json_response=True, **auth_result) + + @mcp.tool() + async def query(sql: str) -> str: + require_scope("tools/query") + ... + + try: + await mcp.run_streamable_http_async() + finally: + await auth_result.aclose() + +asyncio.run(main()) ``` -The MCP SDK's `mcp.run()` is sync (it owns the loop), so the adapter setup -runs via `asyncio.run(...)` first. FastMCP exposes `run_async()` so the -adapter setup and server can share one loop. +`authplane_mcp_auth()` creates async resources (locks, HTTP client, background +JWKS/metadata caches) bound to the running loop. The adapter setup, server, +and `aclose()` must share one loop — call `mcp.run_streamable_http_async()` +(or `run_sse_async` / `run_stdio_async`) from inside `asyncio.run(main())`. +Wrapping only the setup in `asyncio.run(...)` and then calling the sync +`mcp.run()` strands those resources on a closed loop and the first request +will fail. ## Architectural rules (do not break) diff --git a/tests/test_cache.py b/tests/test_cache.py index 77754a4..d3fc5cd 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -79,3 +79,120 @@ def test_cache_key_with_resource(): def test_cache_key_resource_only(): key = TokenCache.cache_key("", "https://api.example.com") assert key == "|https://api.example.com" + + +# --------------------------------------------------------------------------- +# Bounded LRU +# --------------------------------------------------------------------------- + + +def test_default_max_entries_matches_cross_sdk_reference(): + """The default cap is 10_000 entries. + + Matches the Java SDK reference (``TokenCacheConfig.DEFAULT_MAX_ENTRIES``) + so the same workload hits the same eviction watermark across SDKs. + Verified against the Java source at + ``core/src/main/java/ai/authplane/sdk/core/TokenCacheConfig.java`` — + ``public static final int DEFAULT_MAX_ENTRIES = 10_000;``. If either + side moves, this test catches the drift before a workload notices + it the hard way. + """ + assert TokenCache.DEFAULT_MAX_ENTRIES == 10_000 + + +def test_rejects_non_positive_max_entries(): + """A non-positive cap is a programmer-supplied bug, not a runtime + condition — fail fast at construction so the misconfiguration surfaces + immediately rather than when the cache happens to overflow.""" + import pytest + + with pytest.raises(ValueError): + TokenCache(max_entries=0) + with pytest.raises(ValueError): + TokenCache(max_entries=-1) + + +def test_rejects_bool_max_entries(): + """``bool`` is a subclass of ``int`` in Python, so a stray ``True`` + would otherwise silently produce a cap-1 cache. Reject at construction.""" + import pytest + + with pytest.raises(ValueError): + TokenCache(max_entries=True) # type: ignore[arg-type] + with pytest.raises(ValueError): + TokenCache(max_entries=False) # type: ignore[arg-type] + + +def test_rejects_float_max_entries(): + """A fractional cap is a programmer bug — reject at construction even + when the value is integer-valued (e.g. ``10_000.0``) for symmetry with + the bool rejection.""" + import pytest + + with pytest.raises(ValueError): + TokenCache(max_entries=1.5) # type: ignore[arg-type] + with pytest.raises(ValueError): + TokenCache(max_entries=10_000.0) # type: ignore[arg-type] + + +def test_max_entries_property_exposes_configured_cap(): + """Operators can read the cap back without re-threading the constructor + argument through their own config layer.""" + assert TokenCache().max_entries == TokenCache.DEFAULT_MAX_ENTRIES + assert TokenCache(max_entries=42).max_entries == 42 + + +def test_evicts_lru_when_cap_exceeded(): + """Inserting a third entry into a cap-2 cache must evict the + least-recently-used entry.""" + cache = TokenCache(ttl_buffer_seconds=0, max_entries=2) + cache.set("k1", "v1", "Bearer", expires_in=3600) + cache.set("k2", "v2", "Bearer", expires_in=3600) + assert len(cache) == 2 + cache.set("k3", "v3", "Bearer", expires_in=3600) + assert len(cache) == 2 + assert cache.get("k1") is None + assert cache.get("k2") is not None + assert cache.get("k3") is not None + + +def test_get_bumps_entry_to_mru(): + """A ``get`` hit must promote the entry to MRU so the next overflow + eviction targets a colder key — pins the touch-on-read behavior.""" + cache = TokenCache(ttl_buffer_seconds=0, max_entries=2) + cache.set("k1", "v1", "Bearer", expires_in=3600) + cache.set("k2", "v2", "Bearer", expires_in=3600) + # Touch k1 — k2 is now the LRU victim. + assert cache.get("k1") is not None + cache.set("k3", "v3", "Bearer", expires_in=3600) + assert cache.get("k1") is not None + assert cache.get("k2") is None + assert cache.get("k3") is not None + + +def test_reset_does_not_grow_size(): + """Re-setting an existing key must not grow the cache.""" + cache = TokenCache(ttl_buffer_seconds=0, max_entries=2) + cache.set("k1", "v1", "Bearer", expires_in=3600) + cache.set("k1", "v1-updated", "Bearer", expires_in=3600) + assert len(cache) == 1 + entry = cache.get("k1") + assert entry is not None + assert entry.access_token == "v1-updated" + + +def test_reset_bumps_entry_to_mru(): + """Touch-on-write: re-setting an existing key bumps it to MRU. + + Without this, the entry stays LRU and gets evicted when a new entry + lands — which would surprise callers who treat ``set`` as a + "I care about this entry" signal. + """ + cache = TokenCache(ttl_buffer_seconds=0, max_entries=2) + cache.set("k1", "v1", "Bearer", expires_in=3600) + cache.set("k2", "v2", "Bearer", expires_in=3600) + cache.set("k1", "v1-updated", "Bearer", expires_in=3600) # bump k1 to MRU + cache.set("k3", "v3", "Bearer", expires_in=3600) + assert cache.get("k1") is not None + assert cache.get("k2") is None + assert cache.get("k3") is not None diff --git a/tests/verifier/test_claims.py b/tests/verifier/test_claims.py index 72e38e0..fbf2f8a 100644 --- a/tests/verifier/test_claims.py +++ b/tests/verifier/test_claims.py @@ -1,5 +1,6 @@ """Tests for VerifiedClaims dataclass.""" +from collections.abc import Iterator from types import MappingProxyType import pytest @@ -65,6 +66,120 @@ def test_require_scope_failure(sample_claims: VerifiedClaims) -> None: assert "read:data" in str(exc_info.value) # Shows granted scopes +def test_require_scopes_empty_is_noop(sample_claims: VerifiedClaims) -> None: + """Empty input must not raise — callers need not special-case empty iterables.""" + sample_claims.require_scopes([]) + sample_claims.require_scopes(()) + # Generator input also counts as empty. + empty_gen: list[str] = [] + sample_claims.require_scopes(s for s in empty_gen) + + +def test_require_scopes_all_present(sample_claims: VerifiedClaims) -> None: + """All-present input must not raise.""" + sample_claims.require_scopes(["read:data", "write:data"]) + + +def test_require_scopes_consumes_non_empty_generator( + sample_claims: VerifiedClaims, +) -> None: + """A non-empty single-use generator must be materialised once and used in + both the cardinality check and the failure-path ``required_scopes`` field. + + Pins the ``tuple(scopes)`` snapshot in the implementation — without it, + iterating the generator twice (once to find missing, once to populate the + error) would silently exhaust the second pass. + """ + + def gen() -> Iterator[str]: + yield "read:data" + yield "admin" + + with pytest.raises(InsufficientScopeError) as exc_info: + sample_claims.require_scopes(gen()) + + # The generator's `admin` value must surface in both the message and the + # structured ``required_scopes`` tuple — proves materialisation happened + # before the second iteration. + assert "'admin'" in str(exc_info.value) + assert exc_info.value.required_scopes == ("read:data", "admin") + + +def test_require_scopes_single_missing(sample_claims: VerifiedClaims) -> None: + """A single missing scope renders as ``required scope ''``.""" + with pytest.raises(InsufficientScopeError) as exc_info: + sample_claims.require_scopes(["read:data", "admin"]) + + msg = str(exc_info.value) + assert "'admin'" in msg + # Singular "scope" form for one missing entry. + assert "required scope " in msg + assert "required scopes " not in msg + # Surfaces the token's available scopes for the operator. + assert "read:data" in msg + # ``required_scopes`` carries the FULL requested tuple, not just missing. + assert exc_info.value.required_scopes == ("read:data", "admin") + + +def test_require_scopes_multiple_missing(sample_claims: VerifiedClaims) -> None: + """Multiple missing scopes render as ``required scopes 'a', 'b'`` (plural).""" + with pytest.raises(InsufficientScopeError) as exc_info: + sample_claims.require_scopes(["delete:data", "admin"]) + + msg = str(exc_info.value) + assert "'delete:data'" in msg + assert "'admin'" in msg + # Plural form. + assert "required scopes " in msg + assert exc_info.value.required_scopes == ("delete:data", "admin") + + +def test_require_scopes_no_scopes_token() -> None: + """A token without scopes renders ``Token has scopes: (none)`` so an operator can tell + the difference between 'wrong scope' and 'no scopes at all'.""" + no_scope_claims = VerifiedClaims( + sub="user123", + client_id="client456", + scopes=(), + issuer="https://auth.example.com", + audience=("https://api.example.com",), + expires_at=1234567890, + issued_at=1234567800, + jti="token-id-123", + kid="test-key-1", + raw=freeze_value({"sub": "user123"}), + ) + with pytest.raises(InsufficientScopeError) as exc_info: + no_scope_claims.require_scopes(["read:data"]) + + assert "(none)" in str(exc_info.value) + + +def test_require_scope_no_scopes_token_matches_plural_rendering() -> None: + """``require_scope`` (singular) renders an empty scope set the same way the + plural helper does — ``(none)`` rather than ``[]`` — so the two errors are + indistinguishable to an operator reading the WWW-Authenticate text.""" + no_scope_claims = VerifiedClaims( + sub="user123", + client_id="client456", + scopes=(), + issuer="https://auth.example.com", + audience=("https://api.example.com",), + expires_at=1234567890, + issued_at=1234567800, + jti="token-id-123", + kid="test-key-1", + raw=freeze_value({"sub": "user123"}), + ) + with pytest.raises(InsufficientScopeError) as exc_info: + no_scope_claims.require_scope("read:data") + + msg = str(exc_info.value) + assert "(none)" in msg + # Negative pin: must not regress to the old `[]` rendering. + assert "Token has scopes: []" not in msg + + def test_has_claim_key_only(sample_claims: VerifiedClaims) -> None: """has_claim should check key existence when value not provided.""" assert sample_claims.has_claim("sub") is True