feat(mcp): request-scoped identity via app-scoped runtime#351
Conversation
gbrlcustodio
changed the title
gbrlcustodio
force-pushed
the
feat/302-request-scoped-identity
branch
from
cc399f4 to
0e28851
Compare
gbrlcustodio
force-pushed
the
feat/302-request-scoped-identity
branch
from
0e28851 to
fb1aca9
Compare
Evolve ServicesContainer into an app-scoped McpRuntime (core/runtime.py), built once at server construction and shared across Streamable HTTP sessions via an idempotent initialize(). The runtime carries a parsed AuthStrategy sum type: StartupIdentity resolves one credential from settings (stdio/local), while RequestScopedIdentity wires the shared client with a stateless RequestContextBearerAuth that reads each caller's validated bearer from the request context per outbound call. This makes the hosted resource-server profile act on behalf of each calling user (#302) rather than as a single startup identity. One shared client serves every concurrent caller as themselves: the adapter holds no per-request state and reads the token inline in the httpx auth flow, so under a multi-worker (Gunicorn + uvicorn) deployment one request's bearer cannot authenticate another's call. The hosted profile also boots with no global Pipefy credential. The HTTP transport stays loopback-only; the remaining blocker to an off-loopback bind is now the DNS-rebinding host/Origin allowlist (#303), not a shared identity.
…identity auth Move credential resolution and the missing-auth fail-fast out of the runtime into StartupIdentity.from_configured_credential, called at the composition root (_select_auth_source). Both identity variants now carry an httpx.Auth, so McpRuntime wires the one shared client uniformly with no per-variant branching; the match/assert_never and _build_startup_client are gone. Keeps the single-shared-client model and the stateless RequestContextBearerAuth; AuthStrategy is renamed AuthSource to reflect the role. Resolution/precedence/ fail-fast tests move to TestSelectAuthSource in test_server.py, where resolution now lives.
CallableBearerAuth had no production construction site: every reference was a docstring or an __all__ entry, and only its own unit tests exercised it. It was added with the precedence-chain consolidation and never wired. RefreshableBearerAuth already subsumes its behaviour (per-request provider) plus the 401 retry the stored-session path needs. Reword the RefreshableBearerAuth and RequestContextBearerAuth docstrings that referenced it so no dangling reference remains.
The remote profile acts on behalf of the caller, and the composition root (`_select_auth_source`) picks `RequestScopedIdentity` only when a resource server is present. Without `PIPEFY_MCP_RS_RESOURCE_SERVER_URL` the builder returns None, so the server would silently fall back to the single startup credential and per-request on-behalf-of identity would never engage. Refuse to start instead, so a misconfigured remote deployment fails loudly.
resolve_mcp_settings now returns the fully-resolved Settings (argv folds into the mcp model, the rest from the environment), mirroring resolve_cli_settings. build_pipefy_mcp_server takes that Settings and reads profile/host/port from it, dropping the remote_mode override, the settings-derived default, and the _resolve_bind helper. McpRuntime exposes its settings so the lifespan reads them from the runtime instead of the module global. run_server passes the resolved Settings straight through, so server.py no longer imports the process-global singleton.
gbrlcustodio
force-pushed
the
feat/302-request-scoped-identity
branch
from
fb1aca9 to
90f20cd
Compare
Move resource-server construction, the remote-profile fail-fast, and the identity selection into McpRuntime.for_profile, the runtime's one build step from resolved settings. The runtime now owns both the outbound identity and the inbound (verifier, auth) pair (exposed as inbound_auth), so the remote-profile decision lives in one place instead of being split across run_server, _select_auth_source, and build_pipefy_mcp_server. build_pipefy_mcp_server drops its resource_server kwarg and reads runtime.inbound_auth; run_server drops its resource-server block. This ticks the "runtime owns verifier" box toward the per-app runtime and lets downstream depend on the runtime rather than an ad-hoc resolve.
There was a problem hiding this comment.
Summary
I checked out 3e849bf3 locally: ruff clean, 1585 unit tests green, CI green. The refactor toward McpRuntime as composition root looks sound and the remote fail-fast without a resource server is a good guard.
The blocker is request-scoped identity under the transport we actually ship. Streamable HTTP runs in stateful mode (stateless_http defaults to false and we never override it). The MCP server task is spawned during initialize and later tools/call messages run inside that same task. RequestContextBearerAuth reads get_access_token(), which comes from auth_context_var set by AuthContextMiddleware in the ASGI request task. That context does not update inside the long-lived server task, so outbound Pipefy calls keep replaying the session initializer's bearer.
I reproduced this on the real app: initialize with token A, tools/call on the same session with token B, outbound Authorization is still A. That breaks mid-session token refresh (stale outbound token leads to persistent 401s) and creates a confused-deputy path if another principal reuses a session id. Loopback binding limits exposure today, but the profile is meant to serve concurrent remote callers and does not deliver per-request identity yet.
Required before merge
Read the token from the current MCP message context (e.g. the Starlette Request on request_ctx, where .user.access_token.token is set per message) instead of get_access_token(). Alternatively serve with stateless_http=True and document that choice.
Add a transport-level test: init with bearer A, tools/call with bearer B on the same stateful session, assert outbound header is B. This test fails today and is the acceptance check for #302. The existing 24-task unit test only exercises contextvars in the same asyncio task, so it stays green while this bug is live.
Verification
uv sync --locked --all-extras --dev
uv run ruff check packages/mcp packages/auth
uv run pytest packages/mcp/tests packages/auth/tests -m "not integration" -qStateful OBO probe (fails today): init session with bearer A, tools/call with bearer B on same session, capture outbound Authorization header.
| present (a call ran outside the resource-server request scope), so a missing | ||
| identity fails loudly instead of issuing an unauthenticated Pipefy call. | ||
| """ | ||
| access = get_access_token() |
There was a problem hiding this comment.
This reads get_access_token(), which pulls from auth_context_var set by AuthContextMiddleware in the ASGI request task. Under stateful streamable HTTP (the default: we never set stateless_http=True), the tool handler runs in the session's long-lived server task spawned at initialize. That task keeps the contextvar snapshot from init, not the bearer on the current tools/call.
I reproduced on the real app: init with token A, tools/call with token B on the same session, outbound header is still A. The per-message request_ctx already carries the current Starlette Request with the right .user.access_token, but this path does not use it.
Read the bearer from the current MCP request context (or opt into stateless_http=True with that documented and tested). A stateful integration test with two different bearers on one session is the acceptance check for #302.
Under stateful Streamable HTTP the tool handler runs in a long-lived per-session task whose captured auth_context_var is frozen at the session's initialize bearer. The outbound adapter read that via the SDK's get_access_token(), so it replayed the session-initializer's token on every later call: mid-session token refresh produced persistent 401s, and a reused session id ran as the initializer rather than the current caller. Read the bearer from request_ctx instead, which the low-level server resets per JSON-RPC message from that message's own validated request, so one shared client acts as the current caller on each call. Wrap the read in a private _get_access_token that mirrors the SDK helper's AccessToken | None contract, so a future SDK fix swaps back in one place. Add a transport-level test that reproduces the two-task topology (session task frozen at bearer A, message carrying bearer B) and asserts the outbound header is B; it fails against the old get_access_token() read.
adriannoes
left a comment
adriannoes
left a comment
There was a problem hiding this comment.
The blocker is addressed in 08bb66d.
Reading the bearer from per-message request_ctx instead of the session-frozen auth_context_var is the right fix, and test_stateful_session_task_reads_current_message_bearer reproduces the two-task topology we called out.
Good to mark ready once you're happy with the draft state, @gbrlcustodio 👍🏻
…olved Settings The 'remote'-over-stdio rule now lives only in the McpSettings validator: main resolves the launch flags and renders the resulting ValueError as a usage error (exit 2), rather than re-encoding the rule at the argv boundary. run_server no longer resolves or accepts launch flags; it takes a fully-resolved Settings. This keeps the exit-2 usage-error boundary around resolution alone, so a failure while building or serving surfaces as itself instead of a misframed usage error. Test call sites resolve explicitly via resolve_mcp_settings.
…-anchored comments The #302 changelog entry credited credential resolution to _select_auth_source, which exists nowhere in the tree; the composition root is McpRuntime.for_profile / StartupIdentity.from_configured_credential. Drop the transient #302 marker from the request-identity test docstring, and rephrase the loopback-bind notes (server.py, AGENTS.md) and a request-identity test docstring to describe the system as it is rather than as a delta from a previous version.
adriannoes
left a comment
adriannoes
left a comment
There was a problem hiding this comment.
Blocker from the earlier review is addressed in 08bb66d: bearer is read from per-message request_ctx, and test_stateful_session_task_reads_current_message_bearer covers the stateful session topology.
Follow-up commits clean up composition root (run_server takes resolved Settings) and docs (CHANGELOG, AGENTS.md). CI green, 1587 unit tests pass locally on caad107.
LGTM for merge from my side.
2 participants
What
Wires per-request on-behalf-of identity for the MCP
remoteprofile's resource-server transport (#302). An app-scopedMcpRuntimeowns one sharedPipefyClient; each request resolves its identity from its own validated inbound bearer, so one server serves concurrent callers as themselves rather than as a single startup identity. The stdio/local profile is unchanged: it still runs as the one credential resolved at startup.Closes #302.
Changes
core/container.py) with a singleMcpRuntime(core/runtime.py) built once at server construction. Tools resolve the client per call from the lifespan context, so identity is request-scoped without re-registering the tool table. Under the hosted profile a statelessRequestContextBearerAuthreads each caller's validated bearer from the request context; under stdio the runtime holds one startup credential.StartupIdentity.from_configured_credential, called by_select_auth_sourceat the composition root. Both identity variants (StartupIdentity,RequestScopedIdentity) now carry anhttpx.Auth, so the runtime wires the shared client uniformly with no per-variant branching.Settings.resolve_mcp_settingsreturns the fullSettings,build_pipefy_mcp_server(settings)reads the profile/host/port from it, andserver.pyreads no process globals on the serve path. This mirrors the CLI's composition-root pattern (resolve_cli_settings).remoteprofile has no resource server. LaunchingremotewithoutPIPEFY_MCP_RS_RESOURCE_SERVER_URLnow raises at startup with an actionable message instead of silently degrading to the startup credential (there would be no per-request identity to act as).CallableBearerAuthadapter. It had no production construction site;RefreshableBearerAuthalready covers the per-request-provider behaviour plus the 401 refresh-and-retry the stored-session path needs.Testing
packages/mcp: full suite green (unit; the live-integration tests skip cleanly without credentials).packages/auth: full suite green.Notes
This is the stepping stone toward the single per-app runtime tracked in #346. A follow-up (
refactor/pipefy-engine-session) takes this further into an engine/session split in the SDK. The module-levelsettings = Settings()singleton survives only for two per-request readers (unified-envelope check, graphql timeout) that have no settings handle yet; threading those through the runtime is a separable follow-up.