Skip to content

fix(auth): populate ctx.auth_principal from bearer ContextVar#574

Merged
bokelley merged 2 commits intomainfrom
bokelley/issue-571-bearer-auth-principal
May 4, 2026
Merged

fix(auth): populate ctx.auth_principal from bearer ContextVar#574
bokelley merged 2 commits intomainfrom
bokelley/issue-571-bearer-auth-principal

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 4, 2026

Summary

Closes #571.

For bearer-token adopters using BearerTokenAuthMiddleware, RequestContext.auth_principal was always None because the middleware never constructs an AuthInfo — the principal lives in the adcp.server.auth.current_principal ContextVar that auth_context_factory reads into ToolContext.caller_identity. Adopters trying to read "who's calling?" inside a decisioning handler either had to reach into the framework-private ContextVar themselves, or misread ctx.caller_identity (which the framework dispatch helper mutates downstream into a composite cache scope key for idempotency, e.g. core.stores.accounts.SalesagentAccountStore:wonderstruck:default).

Three coordinated parts:

  • A. Code fix. _build_request_context in src/adcp/decisioning/dispatch.py now falls back to current_principal.get() when auth_info is absent or its principal is None. Local import inside the function avoids the server→decisioning import cycle. Outside a bearer flow current_principal.get() returns None — the desired no-op for unauthenticated dev fixtures and signed-request flows that already pass AuthInfo.
  • B. Docstrings. RequestContext docstring in src/adcp/decisioning/context.py clarifies that auth_principal is sourced from AuthInfo.principal on signed-request flows AND the bearer ContextVar on bearer flows, and that caller_identity is mutated downstream into a composite cache scope key — adopters should not read it for identity decisions. _build_request_context param doc updated to match.
  • C. Examples + docs. examples/mcp_with_auth_middleware.py carries an inline comment pointing at ctx.auth_principal as the right read for "who's calling?". docs/handler-authoring.md gains a "Reading 'who's calling?' — auth_principal vs caller_identity" subsection that disambiguates the two fields and their layers.

Behavior shift (call out)

Bearer-flow callers that previously saw ctx.auth_principal == None will now see the bearer principal (e.g. principal-acme-ops). Adopter code that gated on if ctx.auth_principal is None: ... for bearer flows will silently change behavior. This is intended — the previous None was the bug filed against the SDK by the known bearer adopter (salesagent) — but worth flagging for any other bearer adopter on is None checks.

auth_info is unchanged for bearer flows: still None. Only auth_principal is populated from the ContextVar fallback. The deeper structural gap (BearerTokenAuthMiddleware not constructing an AuthInfo) is a separate follow-up flagged by the security-reviewer in triage.

Test plan

  • pytest tests/test_decisioning_dispatch.py tests/test_decisioning_context_state_resolve.py tests/test_testing_decisioning.py tests/test_auth_middleware.py tests/test_serve_auth_both.py -v — 152 passed
  • pytest tests/ -v -k "auth or dispatch or context" — 566 passed, 12 skipped, 1 xfailed
  • ruff check src/ — clean
  • mypy src/adcp/ — clean
  • New regression test test_build_request_context_falls_back_to_bearer_context_var exercises the new fallback path explicitly
  • New test test_build_request_context_auth_info_takes_precedence_over_bearer_var pins the precedence rule (signed-request AuthInfo wins when both sources are populated)
  • Manual smoke from a bearer adopter confirming ctx.auth_principal now reads the principal in their handler

bokelley and others added 2 commits May 4, 2026 10:03
For bearer-token adopters using BearerTokenAuthMiddleware,
RequestContext.auth_principal was always None because the middleware
never constructs an AuthInfo — the principal lives in the
adcp.server.auth.current_principal ContextVar that auth_context_factory
reads. Adopters trying to read "who's calling?" inside a decisioning
handler had to reach into the framework-private ContextVar themselves,
or misread ctx.caller_identity (which the framework dispatch helper
mutates downstream into a composite cache scope key for idempotency,
e.g. core.stores.accounts.SalesagentAccountStore:wonderstruck:default).

_build_request_context now falls back to current_principal.get() when
no AuthInfo is present (or its principal is None), so ctx.auth_principal
is the right read on both signed-request and bearer flows. Updates
context.py + dispatch.py docstrings to document the two sources, the
example to comment on the correct field to read, and handler-authoring.md
with a section disambiguating auth_principal vs caller_identity.

User-visible behavior shift: bearer-flow callers that previously saw
ctx.auth_principal == None will now see the bearer principal. Adopter
code that gated on `if ctx.auth_principal is None: ...` for bearer flows
will silently change behavior — this is intended (the previous None was
the bug) but worth surfacing in the changelog.

Closes #571

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous comment claimed the local import of adcp.server.auth.current_principal
sidestepped a server→decisioning import cycle, but adcp.server.auth does not
import from adcp.decisioning, so there is no cycle. Reword the justification to
state the actual reason: keeping the layering local without forcing a top-level
dep on adcp.server.auth.

Comment-only change; no behavior or import structure changes.
@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 4, 2026

Reworded the local-import justification comment in src/adcp/decisioning/dispatch.py per review feedback. The previous comment claimed the local import sidesteps a server→decisioning import cycle, but adcp.server.auth does not import from adcp.decisioning (verified its imports), so there is no real cycle. The local-import pattern is still correct — it just avoids forcing a top-level dep on adcp.server.auth from dispatch.py — and the comment now says that instead.

Comment-only change; no behavior, no code movement.

@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 4, 2026

Confirmed — 073beec is visible on the branch. The updated comment ("Local import keeps the layering local — read the bearer ContextVar without forcing a top-level dep on adcp.server.auth") is accurate and avoids the misleading cycle claim. No further action needed here.


Generated by Claude Code

@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 4, 2026

Acknowledged — thanks for confirming. No further action from me; standing by for review.


Generated by Claude Code

@bokelley bokelley merged commit 6c49b31 into main May 4, 2026
16 checks passed
bokelley added a commit that referenced this pull request May 4, 2026
…xt for bearer flows

Completes the bearer-adopter typed surface started by #574. PR #574
made ctx.auth_principal populate from the BearerTokenAuthMiddleware
ContextVar; this addresses the deeper structural gap — ctx.auth_info
itself was None on bearer flows, so adopters couldn't branch on
ctx.auth_info.kind to discriminate signed-request vs bearer.

_build_request_context now synthesizes
AuthInfo(kind="bearer", principal=<from current_principal>,
credential=None) when no AuthInfo is supplied and the bearer ContextVar
is populated. credential=None is passed explicitly to bypass the
AuthInfo.__post_init__ flat-field synthesis path and its accompanying
DeprecationWarning (see context.py:396-426: the sentinel default fires
synthesis, an explicit None does not). The synthesized AuthInfo carries
only kind + principal — bearer tokens are opaque to the SDK; adopters
who want richer data (key_id, scopes) can write their own
context_factory.

User-visible behavior shift for bearer adopters:
ctx.auth_info: None → AuthInfo(kind="bearer", ...). Adopter code that
gated on `if ctx.auth_info is None` to detect bearer flow now needs
`ctx.auth_info.kind == "bearer"` instead — mirrors the #574 callout
style.

Closes #576
bokelley added a commit that referenced this pull request May 4, 2026
…xt for bearer flows (#577)

Completes the bearer-adopter typed surface started by #574. PR #574
made ctx.auth_principal populate from the BearerTokenAuthMiddleware
ContextVar; this addresses the deeper structural gap — ctx.auth_info
itself was None on bearer flows, so adopters couldn't branch on
ctx.auth_info.kind to discriminate signed-request vs bearer.

_build_request_context now synthesizes
AuthInfo(kind="bearer", principal=<from current_principal>,
credential=None) when no AuthInfo is supplied and the bearer ContextVar
is populated. credential=None is passed explicitly to bypass the
AuthInfo.__post_init__ flat-field synthesis path and its accompanying
DeprecationWarning (see context.py:396-426: the sentinel default fires
synthesis, an explicit None does not). The synthesized AuthInfo carries
only kind + principal — bearer tokens are opaque to the SDK; adopters
who want richer data (key_id, scopes) can write their own
context_factory.

User-visible behavior shift for bearer adopters:
ctx.auth_info: None → AuthInfo(kind="bearer", ...). Adopter code that
gated on `if ctx.auth_info is None` to detect bearer flow now needs
`ctx.auth_info.kind == "bearer"` instead — mirrors the #574 callout
style.

Closes #576
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ctx.caller_identity is composite scope-key, not bare principal_id (docs misleading)

1 participant