Skip to content

Conversation

@andylim-duo
Copy link

Multi-Tenancy Code Review Summary

Overview

This PR adds TODO: Multi-tenancy comments throughout the codebase to identify areas that need updates to support multi-tenancy. The review focused on code that maintains state between MCP server calls, configuration without tenant scoping, and client code that needs to specify tenant identifiers.

Key Findings

🔴 Critical Issues

1. Shared State Managers

Files:

  • src/mcp/server/fastmcp/tools/tool_manager.py
  • src/mcp/server/fastmcp/resources/resource_manager.py
  • src/mcp/server/fastmcp/prompts/manager.py
  • src/mcp/server/lowlevel/server.py

Issue: Tools, resources, and prompts are stored in shared dictionaries without tenant scoping. A tool/resource/prompt registered by Tenant A is accessible to Tenant B.

Required Changes:

  • Add tenant_id parameter to all manager methods
  • Scope storage by tenant (e.g., dict[tuple[tenant_id, name], Tool])
  • OR create separate manager instances per tenant

2. Authentication & Authorization

Files:

  • src/mcp/server/auth/provider.py
  • src/mcp/server/auth/middleware/auth_context.py

Issue: AccessToken, RefreshToken, and AuthorizationCode lack tenant_id fields. Cannot enforce tenant-based access control.

Required Changes:

  • Add tenant_id field to AccessToken, RefreshToken, and AuthorizationCode
  • Create get_tenant_id() helper to extract tenant from auth context
  • Ensure all resource/tool/prompt access is filtered by tenant_id from token

3. Session Management

Files:

  • src/mcp/server/session.py
  • src/mcp/server/streamable_http_manager.py
  • src/mcp/shared/context.py

Issue: Sessions and request contexts don't track tenant_id. All requests processed in same global context.

Required Changes:

  • Add tenant_id field to ServerSession
  • Add tenant_id field to RequestContext
  • Partition _server_instances by tenant in StreamableHTTPSessionManager
  • Consider composite session keys: (tenant_id, session_id)

🟡 Medium Priority Issues

4. Configuration Management

Files:

  • src/mcp/server/fastmcp/server.py

Issue: Settings loaded from environment variables without tenant-specific overrides. Auth providers, event stores, and session managers are shared across tenants.

Required Changes:

  • Support tenant-specific configuration overrides
  • Scope event stores by tenant_id to prevent cross-tenant data leakage
  • Consider tenant-scoped config sources (database, per-tenant env files)

5. Client-Side Changes

Files:

  • src/mcp/client/session.py

Issue: Client doesn't track or send tenant_id to server. Server cannot determine which tenant context to use.

Required Changes:

  • Add tenant_id parameter to ClientSession.__init__()
  • Include tenant_id in InitializeRequestParams or clientInfo metadata
  • Send tenant_id with each request (in metadata or headers)

🔵 Testing Gaps

6. Test Coverage

Files:

  • tests/server/fastmcp/test_integration.py

Issue: No multi-tenant test scenarios.

Required Changes:

Add tests for:

  • Multiple tenants with isolated resources, tools, and prompts
  • Tenant-scoped authentication and authorization
  • Cross-tenant isolation verification (Tenant A cannot access Tenant B's data)
  • Session management with tenant context
  • Configuration isolation between tenants

Implementation Approaches

Option 1: Tenant-Scoped Storage (Recommended)

Pros: Simpler to implement, less architectural changes

Cons: Requires passing tenant_id through all layers

Steps:

  1. Add tenant_id to AccessToken, RefreshToken, AuthorizationCode
  2. Add tenant_id to RequestContext and ServerSession
  3. Update managers to use composite keys: (tenant_id, resource_id)
  4. Extract tenant_id from token in request handlers
  5. Filter all operations by tenant_id

Option 2: Tenant-Isolated Instances

Pros: Stronger isolation, cleaner separation

Cons: More complex, requires tenant registry/factory pattern

Steps:

  1. Create tenant registry/factory for manager instances
  2. Route requests to tenant-specific manager instances
  3. Maintain separate state per tenant
  4. Add lifecycle management for tenant instances

Files Modified

Total: 12 source files + 1 test file

Server Core (4 files)

  • src/mcp/server/fastmcp/server.py
  • src/mcp/server/lowlevel/server.py
  • src/mcp/server/session.py
  • src/mcp/server/streamable_http_manager.py

Managers (3 files)

  • src/mcp/server/fastmcp/resources/resource_manager.py
  • src/mcp/server/fastmcp/tools/tool_manager.py
  • src/mcp/server/fastmcp/prompts/manager.py

Authentication (2 files)

  • src/mcp/server/auth/provider.py
  • src/mcp/server/auth/middleware/auth_context.py

Client (1 file)

  • src/mcp/client/session.py

Shared (1 file)

  • src/mcp/shared/context.py

Tests (1 file)

  • tests/server/fastmcp/test_integration.py

How to Find All TODOs

Search for "TODO: Multi-tenancy" in the codebase to find all marked locations. Each TODO comment includes:

  • Description of the issue
  • Specific changes needed
  • Impact on multi-tenancy support

Next Steps

  1. Prioritize: Start with authentication system (Critical)
  2. Design: Choose implementation approach (Option 1 recommended)
  3. Implement: Add tenant_id fields and update managers
  4. Test: Create comprehensive multi-tenant test suite
  5. Document: Update API documentation with multi-tenancy patterns

Security Considerations

⚠️ Critical: Without these changes, the current implementation allows cross-tenant data access. Do not deploy in multi-tenant environments without implementing proper tenant isolation.

Detailed TODO Locations

Authentication System

# src/mcp/server/auth/provider.py
class AccessToken(BaseModel):
    # TODO: Multi-tenancy - AccessToken lacks tenant_id field. This is critical - need to add
    # tenant_id to ensure access tokens are scoped to specific tenants. All resource/tool/prompt
    # access should be filtered by the tenant_id from the access token to prevent cross-tenant access.

Session Management

# src/mcp/server/session.py
class ServerSession(...):
    # TODO: Multi-tenancy - ServerSession does not track tenant_id. Need to add tenant_id field
    # to identify which tenant this session belongs to. This is critical for isolating tenant data
    # and ensuring requests are processed in the correct tenant context.

State Managers

# src/mcp/server/fastmcp/tools/tool_manager.py
class ToolManager:
    # TODO: Multi-tenancy - Tools are stored in a shared dictionary without tenant scoping.
    # Need to either: (1) add tenant_id parameter to all methods and scope storage by tenant
    # (e.g., dict[tuple[tenant_id, tool_name], Tool]), or (2) create separate ToolManager
    # instances per tenant. Tools registered by one tenant should not be accessible to others.

Client Integration

# src/mcp/client/session.py
class ClientSession(...):
    # TODO: Multi-tenancy - ClientSession does not track or send tenant_id. Need to add tenant_id
    # field and pass it to the server during initialization and with each request (either in request
    # metadata or as a header). This allows the server to process requests in the correct tenant context.

- Added comprehensive TODO comments identifying areas requiring tenant isolation (auth tokens, sessions, managers, request contexts)
- Documented shared state concerns in tool, resource, and prompt managers that need tenant scoping
- Noted missing tenant_id fields in core models (AccessToken, AuthorizationCode, RefreshToken, RequestContext, ServerSession, ClientSession)
- Identified need for test coverage of multi-tenant scenarios an
@andylim-duo
Copy link
Author

Sorry - this was an accidental push.

@andylim-duo andylim-duo closed this Nov 6, 2025
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.

1 participant