[BUG][PERFORMANCE]: Auth and RBAC middleware create duplicate DB sessions alongside observability middleware

[ja8zyjits]

🐞 Bug Summary

The auth_middleware.py and rbac.py middleware components create their own database sessions using SessionLocal(), which leads to duplicate session creation when running alongside the observability middleware. This issue was identified during the code review of PR #3600 (which fixed duplicate sessions in observability middleware) and represents the same architectural problem in other middleware components.

Impact: Each request that passes through multiple middleware layers (observability + auth + RBAC) may create up to 4 separate database sessions instead of sharing a single request-scoped session, leading to:

• Increased database connection pool pressure
• Higher memory usage per request
• Potential connection exhaustion under load
• Increased risk of SQLite StaticPool segfaults (similar to issue [BUG][PERFORMANCE]: Observability middleware opens duplicate DB session per request #3467)
• Suboptimal performance in PostgreSQL/PgBouncer deployments

---

🧩 Affected Component

• mcpgateway - API
• mcpgateway - UI (admin panel)
• mcpgateway.wrapper - stdio wrapper
• Federation or Transports
• CLI, Makefiles, or shell scripts
• Container setup (Docker/Podman/Compose)
• Other (explain below)

Specific Files:

• mcpgateway/middleware/auth_middleware.py - 3 instances of SessionLocal()
• mcpgateway/middleware/rbac.py - 1 instance of SessionLocal()

---

🔁 Steps to Reproduce

1. Enable observability middleware with database tracing:

   OBSERVABILITY_ENABLED=true
   STRUCTURED_LOGGING_DATABASE_ENABLED=true

2. Configure authentication and RBAC:

   AUTH_REQUIRED=true
   RBAC_ENABLED=true

3. Make an authenticated API request that triggers all three middleware:

   curl -H "Authorization: Bearer $TOKEN" http://localhost:4444/api/v1/servers

4. Observe database session creation in logs (with debug logging enabled):

   LOG_LEVEL=DEBUG

5. Check for multiple session creation messages:

   • [OBSERVABILITY] DB session created: <id1>
   • [AUTH] DB session created: <id2> (if logging added)
   • [RBAC] DB session created: <id3> (if logging added)
   • [GET_DB] DB session created: <id4> (from route handler)

Expected: 1 session per request (shared via request.state.db)
Actual: Up to 4 sessions per request (one per middleware + route handler)

---

🤔 Expected Behavior

Following the pattern established in PR #3600 for observability middleware, all middleware should:

1. Check for existing session in request.state.db before creating a new one
2. Reuse the shared session if available
3. Create and store a session in request.state.db if none exists
4. Track ownership to ensure proper cleanup (only the creating middleware should close the session)
5. Handle errors properly with rollback and connection invalidation

Example pattern (from observability middleware):

# Check if session already exists
db = getattr(request.state, "db", None)
if db is None:
    # Create new session and mark ownership
    db = SessionLocal()
    request.state.db = db
    session_owned_by_middleware = True
else:
    # Reuse existing session
    session_owned_by_middleware = False

---

📓 Logs / Error Output

Current behavior (inferred from code inspection):

# mcpgateway/middleware/auth_middleware.py
# Multiple locations where SessionLocal() is called directly:

# Location 1: Token validation
db = SessionLocal()
try:
    # ... token validation logic ...
finally:
    db.close()

# Location 2: User lookup
db = SessionLocal()
try:
    # ... user lookup logic ...
finally:
    db.close()

# Location 3: Permission checks
db = SessionLocal()
try:
    # ... permission logic ...
finally:
    db.close()

# mcpgateway/middleware/rbac.py
# Location: RBAC enforcement
db = SessionLocal()
try:
    # ... RBAC logic ...
finally:
    db.close()

No error logs currently, but this creates the same conditions that led to issue #3467:

• Connection pool exhaustion under load
• SQLite StaticPool segfaults with concurrent requests
• Suboptimal connection reuse in PostgreSQL/PgBouncer scenarios

---

🧠 Environment Info

Key	Value
Version or commit	main (post PR #3600)
Runtime	Python 3.11+
Platform / OS	All platforms
Container	Docker, Podman, or native
Database	SQLite (default) or PostgreSQL

Affected Configurations:

• Any deployment with AUTH_REQUIRED=true and OBSERVABILITY_ENABLED=true
• Any deployment with RBAC_ENABLED=true and OBSERVABILITY_ENABLED=true
• High-concurrency deployments (>100 concurrent requests)
• PostgreSQL deployments with PgBouncer connection pooling

---

🧩 Additional Context

Relationship to PR 3600 and Issue #3467

This issue is a direct extension of the work done in PR #3600, which fixed duplicate session creation in observability middleware. The same architectural problem exists in auth and RBAC middleware but was marked as "Out of Scope (Future Work)" in the PR #3600 code review.

From PR #3600 Code Review (Section 6):

Locations:

• mcpgateway/middleware/auth_middleware.py - 3 instances of SessionLocal()
• mcpgateway/middleware/rbac.py - 1 instance of SessionLocal()

Issue: These middleware also create their own sessions, which could lead to additional duplicate sessions if they run alongside observability middleware.

Recommendation: Consider extending the session-sharing pattern to these middleware in a future PR.

Proposed Solution

Apply the same session-sharing pattern from PR #3600 to auth and RBAC middleware:

1. Update auth_middleware.py:

   • Replace all 3 SessionLocal() calls with session reuse logic
   • Add ownership tracking for proper cleanup
   • Add connection invalidation in error paths

2. Update rbac.py:

   • Replace the SessionLocal() call with session reuse logic
   • Add ownership tracking for proper cleanup
   • Add connection invalidation in error paths

3. Ensure proper middleware ordering:

   • Document the expected middleware execution order
   • Ensure the first middleware in the chain creates the session
   • Subsequent middleware reuse the shared session

4. Add comprehensive tests:

   • Test session reuse across multiple middleware
   • Test error handling in each middleware
   • Integration test verifying single session per request with all middleware active

Performance Impact

Current state (with all middleware enabled):

• Observability middleware: 1 session (fixed in PR [Fix][Observability]: Fix duplicate DB session in observability middleware #3600)
• Auth middleware: 3 sessions (ISSUE)
• RBAC middleware: 1 session (ISSUE)
• Route handler: 1 session (via get_db())
• Total: 6 sessions per authenticated request

Expected state (after fix):

• Total: 1 shared session per request

Improvement: 83% reduction in database sessions

Related Issues and PRs

• Issue [BUG][PERFORMANCE]: Observability middleware opens duplicate DB session per request #3467: Original issue about observability middleware duplicate sessions
• PR [Fix][Observability]: Fix duplicate DB session in observability middleware #3600: Fixed observability middleware
• This issue: Extends the fix to auth and RBAC middleware

Security Considerations

• Session sharing must maintain transaction isolation
• Each middleware must properly handle rollback on errors
• Connection invalidation is critical for broken connections (PostgreSQL/PgBouncer)
• No security implications from sharing read-only session state

Testing Strategy

1. Unit tests for each middleware:

   • Test session creation when none exists
   • Test session reuse when one exists
   • Test ownership tracking
   • Test error handling and cleanup

2. Integration tests:

   • Test with all middleware enabled
   • Verify single session per request
   • Test concurrent requests (load testing)
   • Test error scenarios across middleware boundaries

3. Performance tests:

   • Measure session count reduction
   • Measure connection pool pressure reduction
   • Test under high concurrency (1000+ concurrent requests)

---

📋 Implementation Checklist

• Update auth_middleware.py with session sharing pattern
• Update rbac.py with session sharing pattern
• Add connection invalidation in error paths
• Add comprehensive unit tests
• Add integration tests for multi-middleware scenarios
• Update documentation (AGENTS.md, architecture docs)
• Performance testing to verify improvement
• Update logging to debug level (not info)

---

🔗 References

• PR [Fix][Observability]: Fix duplicate DB session in observability middleware #3600 Code Review
• Original Issue: [BUG][PERFORMANCE]: Observability middleware opens duplicate DB session per request #3467 - Observability middleware duplicate sessions
• Fixed PR: [Fix][Observability]: Fix duplicate DB session in observability middleware #3600 - Fix duplicate DB session in observability middleware
• Reference Implementation: mcpgateway/middleware/observability_middleware.py (lines 100-120, 200-240)
• Session Management: mcpgateway/main.py:get_db() (lines 2362-2442)
• Related to: [PERFORMANCE]: Observability feature causes major performance regression #2012

[crivetimihai]

Thanks @ja8zyjits. Acknowledged — the get_db() → fresh_db_session() migration is in progress. Legacy middleware paths still create sessions via the deprecated get_db() which can lead to session accumulation alongside the observability middleware's sessions. This is tracked tech debt, not a new regression. Related to the broader auth consolidation epic #3538. Targeting 1.1.0.