Conversation
…ansport shortcut Closes #558. BearerTokenAuthMiddleware only protected the MCP leg. serve(auth=BearerTokenAuth(...)) wires both transports from one config. Adds: BearerTokenAuth dataclass, A2ABearerAuthMiddleware (path-exempts agent-card per A2A spec 4.1, returns HTTP 401, stashes scope['user']), and the serve(auth=) kwarg. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ot reject, OPTIONS, telemetry)
Address findings from code-reviewer / security-reviewer / ad-tech-protocol-expert
review:
- WWW-Authenticate header on every 401 (RFC 7235 §3.1, RFC 6750 §3).
Browsers and HTTP libraries that follow the spec now surface the
bearer challenge instead of treating the 401 as opaque.
- 401 body uses RFC 6750 §3.1 error codes ("invalid_token",
"error_description") instead of free-form "unauthenticated".
- OPTIONS preflight bypasses auth so CORS works for browser-origin
buyers — without this the preflight 401s and the buyer never gets
the chance to retry with a token.
- Async validator now rejected at boot in _wrap_a2a_with_auth via
inspect.iscoroutinefunction. Production deployments fail at
serve() time instead of on first traffic.
- Auth-rejection telemetry: each rejection branch logs a coarse
reason (missing_header / wrong_scheme / invalid_token / etc.) so
SOC dashboards can detect scanning. Validator exceptions still log
exception() for the operator stack.
- mcp_inner = _wrap_mcp_with_auth(mcp_inner, auth) — assign back so
a future refactor switching to fresh-callable wrappers doesn't
silently drop auth.
- serve(auth: BearerTokenAuth | None = None) typed instead of Any.
- Tests added: WWW-Authenticate header, RFC 6750 body shape, OPTIONS
bypass, async-validator boot rejection, sync validator passes
boot, validator-exception 401 through full ASGI stack.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Updated based on expert review (code-reviewer + security-reviewer + ad-tech-protocol-expert): Must-fix landed:
Should-fix landed:
New tests: WWW-Authenticate header, RFC 6750 body shape, OPTIONS bypass, async-validator boot rejection, sync validator passes boot, validator-exception 401 survives full ASGI stack. 26 in this file + 97 elsewhere green. Lint clean. Open question I didn't act on: ad-tech-protocol-expert wondered whether |
|
Acknowledged. The must-fix and should-fix items all look well-reasoned, and the rationale for deferring the Generated by Claude Code |
|
Got it — standing by. The Generated by Claude Code |
…ames Reviewer flagged that the hardcoded /.well-known/agent-card.json literal could silently leak auth on a renamed route if a2a-sdk's canonical path moves. Two changes: 1. Reference a2a.utils.constants.AGENT_CARD_WELL_KNOWN_PATH directly so the 1.0 path tracks a2a-sdk automatically. Legacy 0.3 alias /.well-known/agent.json stays as a literal (no constant for it). 2. New test_discovery_paths_match_a2a_sdk_routes inspects every path that create_agent_card_routes registers and asserts each is in _A2A_DISCOVERY_PATHS. If a future a2a-sdk version adds a new well-known route (extensions, capability descriptor, etc.), this test fails first — adopters update the frozenset rather than silently 401'ing on the renamed/added route. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressing reviewer feedback: Drift-guard for _A2A_DISCOVERY_PATHS (the non-blocking concern from your verdict):
On the v0.3 compat CI passing on the drift-guard commit. The PR is ready for the merge-with-caveats verdict. |
|
Drift-guard commit reviewed — the
The module docstring (the
These two spots read as contradictions of the Quick fix — replace both On the v0.3 swallow: the in-code callouts in Reviewed by Claude Code. Session: https://claude.ai/code/session_01X1xZipeENxR1dKLLGH2kKw Generated by Claude Code |
|
Fixed in 2f2a7a8. Three stale spots updated in
104 tests green, ruff clean. Generated by Claude Code |
Closes #558.
The bug
`BearerTokenAuthMiddleware` only protected the MCP leg. Adopters who applied it via `serve(transport="both", asgi_middleware=[(BearerTokenAuthMiddleware, {...})])` got the A2A leg silently unauthenticated — the middleware's body-peek discovery logic (looking for `"method": "initialize"` etc.) doesn't match A2A's JSON-RPC namespace, and the path-based dispatcher made the cross-leg coverage dependent on which app the closure captured first. Salesagent shipped a workaround that left A2A open in production.
The fix
A new public API: `serve(handler, transport="both", auth=BearerTokenAuth(...))`.
`BearerTokenAuth` is a frozen dataclass — single source of truth driving both transports' auth from one config. `serve(auth=...)` is wired on `streamable-http` / `a2a` / `both` / `sse` (and warns + ignores on `stdio`, which has no HTTP layer).
Internals
Why ASGI middleware (not a `ServerCallContextBuilder`)?
The bot's triage on #558 originally proposed wiring auth via `create_jsonrpc_routes(context_builder=...)`. That looked clean but failed under test: a2a-sdk's v0.3 compat adapter wraps the entire dispatch in `except Exception` and converts builder-raised `HTTPException(401)` into HTTP 200 with a JSON-RPC error body. That violates the spec-canonical HTTP 401 contract and leaks the auth path as a 200 to unauthenticated callers. ASGI-layer wrapping returns proper HTTP 401 every time.
Implementation gotcha resolved
Per the security-reviewer triage: under `transport="both"`, both legs are wrapped before the `_dispatch` closure captures sub-app references — capturing the unwrapped refs would silently bypass auth on whichever leg got wrapped second. The lifespan composer keeps unwrapped `mcp_inner` / `a2a_inner` references for `.router.lifespan_context` so startup/shutdown still composes correctly.
Tests
20 new in `tests/test_serve_auth_both.py`:
96 tests green across `test_auth_middleware.py`, `test_a2a_server.py`, `test_serve_*.py`, `test_mcp_middleware_composition.py`. Lint clean.
Test plan
🤖 Generated with Claude Code