diff --git a/docs/handler-authoring.md b/docs/handler-authoring.md index b5958ec0..acd97de4 100644 --- a/docs/handler-authoring.md +++ b/docs/handler-authoring.md @@ -295,6 +295,30 @@ and leak the presence of an auth path to attackers). Full worked example: `examples/mcp_with_auth_middleware.py`. Integration test proving the composition: `tests/test_mcp_middleware_composition.py`. +#### Reading "who's calling?" — `auth_principal` vs `caller_identity` + +The two identity-shaped fields adopters see have distinct purposes and +the wrong read silently misroutes authorization decisions. + +- **`ctx.auth_principal`** — the verified caller's identity label. + Read this for per-principal ACLs ("can principal X mutate this + buy?"). Sourced from `AuthInfo.principal` on signed-request flows + and from the `current_principal` ContextVar on bearer flows. + `None` for unauthenticated dev fixtures. +- **`ctx.caller_identity`** — a cache scope key, not a principal. + At the transport layer (`ToolContext.caller_identity`) it's the + bare principal; the framework dispatch helper mutates it into a + composite key (`.:`) + before the handler sees a decisioning `RequestContext`. The + idempotency middleware reads the composite form. Treat as opaque + inside dispatch handlers — log or forward, but don't parse, + compare, or rewrite. + +Skill-middleware and `context_factory` code (which run at the +transport layer, before dispatch hydration) still read +`context.caller_identity` for legitimate cache / rate-limit keying; +the composite mutation happens later, in `_build_request_context`. + #### Pattern 2a — custom middleware (when the shipped one doesn't fit) Subclass `BearerTokenAuthMiddleware` to tighten the discovery bypass, diff --git a/examples/mcp_with_auth_middleware.py b/examples/mcp_with_auth_middleware.py index adb29020..91c32094 100644 --- a/examples/mcp_with_auth_middleware.py +++ b/examples/mcp_with_auth_middleware.py @@ -63,6 +63,15 @@ async def get_adcp_capabilities( return capabilities_response(["media_buy"]) async def get_products(self, params: Any, context: ToolContext | None = None) -> dict[str, Any]: + # On the dispatch path the handler receives a decisioning + # ``RequestContext``; read ``ctx.auth_principal`` (not + # ``ctx.caller_identity``) for "who's calling?". The + # framework mutates ``caller_identity`` downstream into a + # composite cache scope key for idempotency — it is not a + # principal label by the time a handler sees it. On bearer + # flows like this one ``auth_principal`` is sourced from the + # :data:`adcp.server.auth.current_principal` ContextVar that + # the middleware populates. tenant = context.tenant_id if context is not None else None return products_response(_products_for_tenant(tenant)) diff --git a/src/adcp/decisioning/context.py b/src/adcp/decisioning/context.py index 21b61851..16e5b63c 100644 --- a/src/adcp/decisioning/context.py +++ b/src/adcp/decisioning/context.py @@ -475,23 +475,38 @@ class RequestContext(ToolContext, Generic[TMeta]): DB queries, and stamp audit logs. ``auth_principal`` — "who's calling?" - The verified caller's identity label. The string varies by - auth shape: ``agent_url`` for AdCP v3 signed-request agents - (the documented convention; the SDK's signed-request adapter - wrappers ship in 4.5.0), OAuth subject claim for bearer - flows, mTLS subject for client-cert flows. Read it for - per-principal ACLs *within* an account ("can principal X - mutate this buy?"). + The verified caller's identity label and the typed read for + adopter authorization checks. Populated from two sources + depending on the auth flow: + + * **Signed-request flows** — sourced from + :class:`AuthInfo.principal` (``agent_url`` for AdCP v3 + signed-request agents per spec). + * **Bearer-token flows** — sourced from the + :data:`adcp.server.auth.current_principal` ContextVar that + :class:`BearerTokenAuthMiddleware` populates + (``Principal.caller_identity`` from the validator). + + Read it for per-principal ACLs *within* an account ("can + principal X mutate this buy?"). ``None`` for unauthenticated + dev fixtures. ``caller_identity`` — "what's the cache scope key?" - Composite framework-set key - (``.:``) used by - the idempotency middleware to scope the replay cache. - Treat as opaque. Adopter code may log or forward it - (rate-limiting, audit) but should not parse, compare, or - rewrite it — the format is framework-internal and any - adopter assumption about its shape will break when the - scope-key composition changes. + Starts as the bare principal at the transport layer + (:class:`ToolContext.caller_identity`), then the framework + dispatch helper mutates it into the composite scope key + (``.:``) before + the handler sees the :class:`RequestContext`. The + idempotency middleware reads the composite form to scope + the replay cache. + + **Do not read this for identity decisions** — by the time a + handler observes the field it's a cache key, not a + principal label. Use ``auth_principal`` for "who's calling?" + and treat ``caller_identity`` as opaque (log / forward only; + don't parse, compare, or rewrite). The composite format is + framework-internal and any adopter assumption about its + shape will break when the scope-key composition changes. ``tenant_id`` — "which transport tenant?" Inherited from :class:`ToolContext`; set by the transport @@ -519,11 +534,15 @@ class RequestContext(ToolContext, Generic[TMeta]): ``NotImplementedError`` on every call. v6.1 wires the backing fetchers. :param auth_principal: Typed convenience field carrying the - verified principal label (sourced from - :class:`AuthInfo.principal` when present). Distinct from - ``account.id`` (which the framework's idempotency middleware - uses for cache scope) — middleware reading "who authenticated - this request" gets a load-bearing field name. + verified principal label. Sourced from + :class:`AuthInfo.principal` on signed-request flows and from + the :data:`adcp.server.auth.current_principal` ContextVar + on bearer-token flows (the framework's + :class:`BearerTokenAuthMiddleware` populates the + ContextVar; the dispatch helper reads it when ``auth_info`` + is absent). The right read for "who's calling?" — distinct + from ``caller_identity``, which the framework mutates into + a composite cache scope key for idempotency. """ # Default factories so ``RequestContext()`` works in tests; in diff --git a/src/adcp/decisioning/dispatch.py b/src/adcp/decisioning/dispatch.py index 70afbe16..e36a613a 100644 --- a/src/adcp/decisioning/dispatch.py +++ b/src/adcp/decisioning/dispatch.py @@ -1006,8 +1006,15 @@ def _build_request_context( composite key. :param account: Resolved account from the platform's :class:`AccountStore.resolve`. - :param auth_info: Optional verified principal info — when present, - ``auth_principal`` is populated from ``auth_info.principal``. + :param auth_info: Optional verified principal info — when present + and carrying a non-``None`` principal, ``auth_principal`` is + populated from ``auth_info.principal``. Otherwise the helper + falls back to :data:`adcp.server.auth.current_principal` — + the ContextVar :class:`BearerTokenAuthMiddleware` populates — + so bearer-flow callers get a typed read for "who's calling?" + without reaching into framework-private state. Returns + ``None`` outside both flows (no-op for unauthenticated dev + fixtures). :param store: The AccountStore that produced ``account``. Required for the production cache-isolation guarantee; the dispatch adapter always supplies it. Test fixtures may pass ``None`` @@ -1024,7 +1031,31 @@ def _build_request_context( from adcp.decisioning.context import RequestContext from adcp.decisioning.resolve import _NotYetWiredResolver - auth_principal = auth_info.principal if auth_info is not None else None + # ``auth_principal`` is the typed "who's calling?" read for + # adopter handlers. Two sources populate it: + # + # * Signed-request flows hydrate ``AuthInfo`` upstream and the + # adapter passes it as ``auth_info``; ``auth_info.principal`` + # carries the verified caller label. + # * Bearer-token flows (:class:`BearerTokenAuthMiddleware`) never + # construct an ``AuthInfo``; they stash the principal in the + # :data:`adcp.server.auth.current_principal` ContextVar instead. + # Read it as the fallback so bearer adopters can gate on + # ``ctx.auth_principal`` without reaching into the framework- + # private ContextVar themselves. ``.get()`` returns ``None`` + # outside a bearer flow — that's the desired no-op for non- + # bearer callers (signed-request without ``AuthInfo``, + # unauthenticated dev fixtures). + # + # Local import keeps the layering local — read the bearer ContextVar + # without forcing a top-level dep on adcp.server.auth. + from adcp.server.auth import current_principal as _current_principal + + auth_principal = ( + auth_info.principal + if auth_info is not None and auth_info.principal is not None + else _current_principal.get() + ) # ctx_metadata credential gate — fail-closed before any platform # method sees the metadata. Buyers can populate ``context`` diff --git a/tests/test_decisioning_dispatch.py b/tests/test_decisioning_dispatch.py index 757e1932..057ffcce 100644 --- a/tests/test_decisioning_dispatch.py +++ b/tests/test_decisioning_dispatch.py @@ -425,8 +425,8 @@ def test_build_request_context_uses_composite_key_when_store_supplied() -> None: def test_build_request_context_with_no_auth() -> None: - """Unauthenticated dev path (singleton fixtures): auth_principal - is None, auth_info is None.""" + """Unauthenticated dev path (singleton fixtures): no AuthInfo and + no bearer ContextVar populated → auth_principal is None.""" tool_ctx = ToolContext() account: Account[Any] = Account(id="dev") ctx = _build_request_context(tool_ctx, account, None) @@ -434,6 +434,43 @@ def test_build_request_context_with_no_auth() -> None: assert ctx.auth_principal is None +def test_build_request_context_falls_back_to_bearer_context_var() -> None: + """Bearer-flow callers populate :data:`adcp.server.auth.current_principal` + via :class:`BearerTokenAuthMiddleware`; the dispatch helper must read + the ContextVar when no ``AuthInfo`` is provided so adopters can read + ``ctx.auth_principal`` instead of reaching into framework-private + state. Regression test for issue #571.""" + from adcp.server.auth import current_principal + + tool_ctx = ToolContext() + account: Account[Any] = Account(id="acct") + token = current_principal.set("principal-from-bearer") + try: + ctx = _build_request_context(tool_ctx, account, None) + finally: + current_principal.reset(token) + assert ctx.auth_info is None + assert ctx.auth_principal == "principal-from-bearer" + + +def test_build_request_context_auth_info_takes_precedence_over_bearer_var() -> None: + """When both ``AuthInfo`` and the bearer ContextVar are populated + (e.g. a custom middleware stack that hydrates both), the explicit + ``AuthInfo.principal`` wins. Bearer fallback is strictly the + "no AuthInfo" path.""" + from adcp.server.auth import current_principal + + tool_ctx = ToolContext() + account: Account[Any] = Account(id="acct") + auth = AuthInfo(kind="signed_request", principal="signed-buyer", key_id="kid") + token = current_principal.set("principal-from-bearer") + try: + ctx = _build_request_context(tool_ctx, account, auth) + finally: + current_principal.reset(token) + assert ctx.auth_principal == "signed-buyer" + + def test_build_request_context_supplies_stubs_when_no_state_resolver() -> None: """Default state/resolve are the v6.0 stubs — adopter call sites work without explicit wiring."""