feat(auth): add tenant_id to session and request context#3
feat(auth): add tenant_id to session and request context#3andylim-duo merged 16 commits intomainfrom
Conversation
Thread tenant_id from authentication tokens through the request lifecycle to enable tenant-scoped operations in handlers. Changes: - Add tenant_id field to base RequestContext (inherited by ServerRequestContext) - Add get_tenant_id() helper in auth_context module to extract tenant from auth - Populate tenant_id in both ServerRequestContext instantiation sites in lowlevel/server.py - Add tenant_id property with getter/setter to ServerSession This is iteration 2 of the multi-tenancy implementation, building on the tenant_id field added to auth tokens in iteration 1.
Use proper async with pattern to ensure ServerSession's internal streams are cleaned up correctly, preventing resource warnings.
Add two tests to verify tenant_id doesn't leak between: - Concurrent async requests using the auth contextvar - Separate ServerSession instances These tests validate critical security properties for multi-tenant deployments where isolation between tenants must be guaranteed.
…tring Document the purpose and usage of the tenant_id field for multi-tenant server deployments.
andylim-duo
left a comment
There was a problem hiding this comment.
Review Summary
Overall the PR is well-structured and the get_tenant_id() helper + contextvar approach is clean. The test coverage is thorough, especially the concurrent isolation test. A few things to address before merging:
Key Issues
-
Disconnected
tenant_idonServerSessionvsRequestContext— The PR addstenant_idto both places, but onlyRequestContext.tenant_idis actually populated by the framework (viaget_tenant_id()).ServerSession.tenant_idis never set by any code path, creating two disconnected sources of truth for the same concept. This should be reconciled — either remove the session property for now, or wire it up. -
Mutable
tenant_idsetter onServerSession— Allowing tenant reassignment mid-session is a potential security concern. Consider making it set-once or constructor-initialized. -
Core server depends on auth module —
lowlevel/server.py(used by all transports) now imports from the auth module. This couples the transport-agnostic server to HTTP-specific auth. Worth considering whether this should be made pluggable or at least documented as HTTP-only.
Minor
- Dead code in
test_get_tenant_id_with_tenant— unusedMockAppand firstmiddlewareassignment. - Prefer
anyio.lowlevel.checkpoint()overanyio.sleep(0.01)for context switching in tests.
…uest Wire up session.tenant_id so it is set automatically from the auth contextvar on the first authenticated request (set-once semantics). This connects RequestContext.tenant_id and ServerSession.tenant_id, ensuring the session is bound to a tenant for its lifetime.
Extract _simulate_tenant_binding helper to avoid pyright narrowing session.tenant_id to a literal type after assertion, which caused the subsequent `is None` check to be flagged as always-False.
…on handling Add two E2E tests using Client(server) that exercise the session.tenant_id set-once binding in lowlevel/server.py, covering the previously uncovered branches in _handle_request (line 456) and _handle_notification (line 504).
This line is now covered by the E2E tenant notification test.
Make the tenant_id setter raise ValueError if attempting to change to a different value once already set. This prevents accidental tenant reassignment which could be a security issue. Setting to the same value is allowed (idempotent).
…action Move tenant identification to a transport-agnostic contextvar (tenant_id_var) in the shared layer, removing the hard dependency from lowlevel/server.py on the auth middleware module. AuthContextMiddleware now sets tenant_id_var alongside auth_context_var, and the core server reads from the shared contextvar instead of calling get_tenant_id() from the auth module. This keeps the dependency direction correct (auth → shared, server → shared) and allows other transports to set tenant_id_var through their own mechanisms.
Remove unused MockApp() and AuthContextMiddleware(app) that were immediately overwritten by AuthContextMiddleware(TenantCheckApp()).
Use checkpoint() for deterministic context switching instead of a fixed-duration sleep in the tenant isolation test.
Import checkpoint directly from anyio.lowlevel to fix pyright reportAttributeAccessIssue on the lazy submodule.
…review concerns Add remove_resource() and remove_prompt() methods to MCPServer to match the existing remove_tool() API, enabling dynamic deprovisioning of all resource types for multi-tenant servers. PR review fixes: - Fix heading grammar: "Simple Registration of..." (concern #1) - Add private API warning for _lowlevel_server usage in example (concern #3) - Clarify example server needs TokenVerifier for production (concern #4) - Guard offboard_tenant against KeyError for unprovisioned tools (concern #5) - Add remove_resource/remove_prompt to MCPServer and docs (concern #6) Tests added for both single-tenant and multi-tenant remove operations, including cross-tenant isolation verification.
Summary
This PR threads
tenant_idfrom authentication tokens through the request lifecycle, enabling handlers to access tenant context for multi-tenant isolation.Problem: After adding
tenant_idto auth tokens (PR #2), there was no way for request handlers to access this information during request processing.Solution: Propagate
tenant_idthrough the session and context layers:tenant_idfield to baseRequestContext(inherited byServerRequestContext)get_tenant_id()helper to extract tenant from auth contexttenant_idin bothServerRequestContextinstantiation sitestenant_idproperty toServerSessionfor session-level tenant trackingChanges
src/mcp/shared/_context.py: Addedtenant_id: str | None = Noneto baseRequestContextsrc/mcp/server/auth/middleware/auth_context.py: Addedget_tenant_id()helper functionsrc/mcp/server/lowlevel/server.py: Updated bothServerRequestContextinstantiation sites to populatetenant_id=get_tenant_id()src/mcp/server/session.py: Added_tenant_idfield andtenant_idproperty/setter toServerSessionPart of Multi-Tenancy Implementation
This is Iteration 2 of a 6-part multi-tenancy implementation plan, building on Iteration 1 (PR #2).
After this PR, handlers can access
ctx.tenant_idto determine which tenant is making a request:Test Plan
get_tenant_id()intests/server/auth/middleware/test_auth_context.pytests/server/test_multi_tenancy_session.pywith 6 tests covering:RequestContextwith/without tenant_idServerRequestContexttenant_id inheritanceServerSessiontenant_id propertyget_tenant_id()auth context extractionuv run --frozen pyrightuv run --frozen ruff checkStacked PR
This PR is based on
feature/multi-tenant-auth-tokens(PR #2). Merge PR #2 first, then this PR can be rebased ontomain.