Backend Multi-Tenancy Issues - Critical bugs and missing features #969
Description
Backend Multi-Tenancy Issues
Summary
Critical bugs and missing features in the multi-tenancy implementation that break data isolation and access control.
Issues
1. Redundant Access Control Filter
Location: Multiple service files
mcpgateway/services/server_service.py:556mcpgateway/services/tool_service.py:608mcpgateway/services/gateway_service.py:834
A redundant filter is applied AFTER the main access control logic that adds no security value and creates confusion.
Current Code:
# Lines 540-550: Proper access control
access_conditions = []
access_conditions.append(DbServer.owner_email == user_email) # User's own resources
if team_ids:
access_conditions.append(and_(DbServer.team_id.in_(team_ids), DbServer.visibility.in_(["team", "public"])))
access_conditions.append(DbServer.visibility == "public") # Public resources
query = query.where(or_(*access_conditions))
# Line 556: PROBLEMATIC redundant filter
query = query.where(~((DbServer.owner_email != user_email) & (DbServer.visibility == "private")))Proposed Fix:
# Remove line 556 entirely from all three files
# The access control logic in lines 540-550 is sufficient and correct2. Missing Visibility Transition Validation
Location: mcpgateway/services/server_service.py:678
When changing visibility from private to team/public, there's no validation that:
- The team exists and is active
- The user has permission to make resources public
- The team_id is valid when changing to "team" visibility
Current Code:
# Line 677-678: Direct assignment without validation
if server_update.visibility is not None:
server.visibility = server_update.visibilityProposed Fix:
if server_update.visibility is not None:
new_visibility = server_update.visibility
# Validate visibility transitions
if new_visibility == "team":
if not server.team_id and not server_update.team_id:
raise ValueError("Cannot set visibility to 'team' without a team_id")
# Verify team exists and user is a member
team_id = server_update.team_id or server.team_id
team = db.query(EmailTeam).filter(
EmailTeam.id == team_id,
EmailTeam.is_active.is_(True)
).first()
if not team:
raise ValueError(f"Team {team_id} not found or inactive")
# Verify user is a member of the team
membership = db.query(EmailTeamMember).filter(
EmailTeamMember.team_id == team_id,
EmailTeamMember.user_email == current_user_email,
EmailTeamMember.is_active.is_(True)
).first()
if not membership:
raise ValueError("User is not a member of the specified team")
elif new_visibility == "public":
# Optional: Check if user has permission to make resources public
# This could be a platform-level permission
pass
server.visibility = new_visibility3. Incomplete Cross-Team Naming Conflict Detection
Location: mcpgateway/services/server_service.py:360-369
The naming conflict check has gaps:
- No check for private resources with the same name (user could have multiple private resources with same name)
- Missing
elifbranch for private visibility case
Current Code:
if visibility.lower() == "public":
existing_server = db.execute(select(DbServer).where(
DbServer.name == server_in.name,
DbServer.visibility == "public"
)).scalar_one_or_none()
elif visibility.lower() == "team" and team_id:
existing_server = db.execute(select(DbServer).where(
DbServer.name == server_in.name,
DbServer.visibility == "team",
DbServer.team_id == team_id
)).scalar_one_or_none()
# Missing: private visibility checkProposed Fix:
# Check for naming conflicts based on visibility rules
if visibility.lower() == "public":
# Public resources must be globally unique
existing_server = db.execute(select(DbServer).where(
DbServer.name == server_in.name,
DbServer.visibility == "public",
DbServer.is_active.is_(True)
)).scalar_one_or_none()
if existing_server:
raise ServerNameConflictError(...)
elif visibility.lower() == "team" and team_id:
# Team resources must be unique within the team
existing_server = db.execute(select(DbServer).where(
DbServer.name == server_in.name,
DbServer.visibility == "team",
DbServer.team_id == team_id,
DbServer.is_active.is_(True)
)).scalar_one_or_none()
if existing_server:
raise ServerNameConflictError(...)
elif visibility.lower() == "private":
# Private resources must be unique per user
existing_server = db.execute(select(DbServer).where(
DbServer.name == server_in.name,
DbServer.visibility == "private",
DbServer.owner_email == owner_email,
DbServer.is_active.is_(True)
)).scalar_one_or_none()
if existing_server:
raise ServerNameConflictError(...)4. Missing Composite Indexes for Performance
Location: Database schema
Only single column index on team_id exists, no composite indexes for the common query pattern.
The access control queries use complex OR conditions with multiple fields:
WHERE (owner_email = ?)
OR (team_id IN (?, ?) AND visibility IN ('team', 'public'))
OR (visibility = 'public')Fix: Add composite indexes for better query performance:
# In migration file
op.create_index('idx_resources_team_visibility', 'resources', ['team_id', 'visibility'])
op.create_index('idx_resources_owner_visibility', 'resources', ['owner_email', 'visibility'])
op.create_index('idx_servers_team_visibility', 'servers', ['team_id', 'visibility'])
op.create_index('idx_servers_owner_visibility', 'servers', ['owner_email', 'visibility'])
# Repeat for other resource tables5. No Resource Access Audit Trail
Location: Resource access methods
No logging of who accessed what resources when. Only PermissionAuditLog exists for permission checks, not resource access.
Fix:
# In resource access methods
async def log_resource_access(
db: Session,
user_email: str,
resource_type: str,
resource_id: str,
action: str,
team_id: Optional[str] = None
):
"""Log resource access for audit trail."""
audit_entry = ResourceAccessLog(
user_email=user_email,
resource_type=resource_type,
resource_id=resource_id,
action=action,
team_id=team_id,
timestamp=utc_now(),
ip_address=request.client.host
)
db.add(audit_entry)
# Don't wait for commit to not slow down access6. Missing CASCADE on Resource Foreign Keys
Location: Database models in db.py
Most resource tables have team_id foreign keys WITHOUT ondelete="CASCADE":
- Line 1498: Tool model - no CASCADE
- Line 1708: Resource model - no CASCADE
- Line 1924: Prompt model - no CASCADE
- Line 2129: Server model - no CASCADE
- Line 2298: Gateway model - no CASCADE
- Line 2367: Tag model - no CASCADE
- Line 2464: A2A Agent model - no CASCADE
- Line 2637: ONLY ApiKey has CASCADE (correct implementation)
When a team is deleted, resources remain with invalid team_id references.