Skip to content

[BUG]: Redundant database queries in PermissionService.check_permission() #2695

@madhav165

Description

@madhav165

Bug Summary

PermissionService.check_permission() executes redundant database queries on every permission check, causing unnecessary performance overhead. The same tables are queried multiple times within a single permission check operation.


Affected Component

Select the area of the project impacted:

  • 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)

Steps to Reproduce

  1. Enable SQLAlchemy query logging (echo=True)
  2. Make any authenticated API request that triggers RBAC permission check
  3. Observe the SQL queries in logs

Redundancy #1 - UserRole table (when audit_enabled=True):

SELECT ... FROM user_roles JOIN roles ...  -- Query 1: get_user_permissions() -> _get_user_roles()
SELECT ... FROM user_roles JOIN roles ...  -- Query 2: _get_roles_for_audit() -> _get_user_roles()

Redundancy #2 - EmailTeamMember table (teams. permission fallback):*

SELECT ... FROM email_team_members ...  -- Query 1: _is_team_member()
SELECT ... FROM email_team_members ...  -- Query 2: _get_user_team_role()

Expected Behavior

Each table should be queried at most once per permission check. The data fetched in the first query should be reused for subsequent operations within the same check.


Logs / Error Output

No errors - this is a performance issue. Query logging shows duplicate SELECT statements:

-- Duplicate UserRole queries in check_permission():
2024-XX-XX INFO sqlalchemy.engine SELECT user_roles.id, ... FROM user_roles JOIN roles ON ...
2024-XX-XX INFO sqlalchemy.engine SELECT user_roles.id, ... FROM user_roles JOIN roles ON ...

-- Duplicate EmailTeamMember queries in _check_team_fallback_permissions():
2024-XX-XX INFO sqlalchemy.engine SELECT email_team_members.* FROM email_team_members WHERE user_email=? AND team_id=?
2024-XX-XX INFO sqlalchemy.engine SELECT email_team_members.* FROM email_team_members WHERE user_email=? AND team_id=?

Environment Info

Key Value
Version or commit main branch
Runtime Python 3.11+
Platform / OS All
Container All

Additional Context (optional)

Root Cause Analysis:

  1. UserRole redundancy (permission_service.py:110-138):

    • get_user_permissions() calls _get_user_roles() but only returns permission strings, discarding role objects
    • _get_roles_for_audit() calls _get_user_roles() again to get role names for audit logging
  2. EmailTeamMember redundancy (permission_service.py:560-564):

    • _is_team_member() queries EmailTeamMember to check existence (returns bool)
    • _get_user_team_role() queries the same table with same filters to get role string
    • The first query already has all the data needed

Impact:

  • Non-admin users with audit enabled: 2 extra DB queries per permission check
  • Non-admin users hitting team fallback: 2 extra DB queries per permission check
  • High-traffic deployments see significant unnecessary database load

Proposed Fix:

  1. Call _get_user_roles() once in check_permission() and reuse for both permissions and audit
  2. Combine _is_team_member() and _get_user_team_role() into single query

Metadata

Metadata

Assignees

Labels

SHOULDP2: Important but not vital; high-value items that are not crucial for the immediate releasebugSomething isn't workingperformancePerformance related items

Type

No fields configured for Bug.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions