Skip to content

[BUG][SECURITY]: token_teams narrowing not enforced consistently across Layer 2 permission paths #3924

@crivetimihai

Description

@crivetimihai

Bug Summary

PR #3919 added token_teams enforcement to the check_any_team=True path in PermissionService. During review, we identified several pre-existing code paths where token_teams (the normalized session/token scope) is either not forwarded to the permission service or is forwarded but ignored in downstream branches. These paths allow a narrowed or public-only token to satisfy RBAC checks using permissions from teams outside its scope.

None of these were introduced by #3919 — they predate it. This is a tracking issue. Gaps 1, 3, and 4 are concrete implementation bugs suitable for child issues; gaps 2 and 6 involve design/policy decisions around token_teams=[]; gap 5 is a UI consistency item.

Expected behavior

A session narrowed to token_teams=["team-a"] should only satisfy Layer 2 (RBAC) checks using permissions from team-A roles. A public-only token (token_teams=[]) should not benefit from team-scoped roles or admin bypass for non-admin operations.

Actual behavior

Several code paths ignore or skip token_teams, allowing narrowed/public-only tokens to satisfy RBAC using out-of-scope team roles. See gap inventory below.


Affected Component

  • mcpgateway - API
  • mcpgateway - UI (admin panel)

Gap inventory

1. Explicit team_id path bypasses token_teams narrowing

File: mcpgateway/services/permission_service.py:488-490

token_teams is correctly forwarded into get_user_permissions() at line 134 and then into _get_user_roles() at line 255. However, when the RBAC middleware resolves an explicit team_id from the route (e.g. PUT /teams/{team_id}), _get_user_roles() enters the if team_id: branch at line 488 which loads roles for that team without consulting the token_teams parameter it received. A session narrowed to team-A can therefore pass a Layer 2 check for team-B if the user has a role in team-B.

The middleware at mcpgateway/middleware/rbac.py:555-579 prefers kwargs["team_id"] and only falls back to check_any_team=True when no team can be resolved, so the narrowing fix from #3919 does not cover this path.

Risk: Medium-High. Mutable endpoints protected only by @require_permission with a path-derived team_id (e.g. team update, token creation for a specific team) are reachable.

2. Admin bypass fires for non-admin.* permissions with token_teams=[]

File: mcpgateway/services/permission_service.py:125-131

check_permission() correctly blocks admin.* for public-only tokens at line 125, but the unconditional admin bypass at line 130 still fires for every other permission. A DB-admin user with a public-only token (token_teams=[]) passes RBAC for teams.read, tools.create, etc.

Verified directly: check_permission("admin@test.com", "teams.read", token_teams=[]) returns True.

Risk: Medium. Layer 1 limits visibility, but routes without resource-ID gating (e.g. GET /teams at mcpgateway/routers/teams.py:142) may expose data. Requires a design decision on whether token_teams=[] should suppress admin bypass entirely or only for admin.*.

3. check_admin_permission() / require_admin_permission do not accept token scope

Files:

  • mcpgateway/services/permission_service.py:384-408check_admin_permission() accepts only user_email, calls _is_user_admin() and get_user_permissions() without token_teams.
  • mcpgateway/middleware/rbac.py:787-792require_admin_permission decorator calls check_admin_permission(user_context["email"]) without forwarding user_context.get("token_teams").

RBAC endpoints (/rbac/* at mcpgateway/routers/rbac.py) are protected by this decorator, so a narrowed or public-only token with DB admin rights can reach role-management routes.

Risk: Medium. Requires the caller to already be a DB admin, limiting exploitability.

4. Token scope containment computed from unscoped permission set

File: mcpgateway/routers/tokens.py:92-96

_get_caller_permissions() calls get_user_permissions(user_email, team_id) without token_teams. Token create/update flows reuse this for custom scope validation. A narrowed session can derive caller_permissions from an out-of-scope team and mint or widen a team token for that team.

Risk: High. This is the most actionable gap — a narrowed session should not be able to create tokens with permissions from teams outside its scope.

5. Admin UI menu visibility uses unscoped permission set

File: mcpgateway/admin.py:506-510

get_hidden_sections_for_user() (defined at mcpgateway/admin.py:458) batch-fetches permissions with include_all_teams=True but no token_teams. The fallback path at line 536 correctly passes token_teams to check_permission(), so this is a fast-path inconsistency: the menu may show sections from out-of-scope team roles.

Risk: Low. UI cosmetic issue — actual access to admin sections is gated separately.

6. token_teams=[] retains team-scoped non-admin permissions at Layer 2

File: mcpgateway/services/permission_service.py:507

token_teams is correctly forwarded into _get_user_roles(), but the filtering guard (if token_teams is not None and len(token_teams) > 0) intentionally skips the empty-list case. This means token_teams=[] (public-only) still aggregates permissions from all team-scoped roles. Layer 1 is supposed to handle visibility, but routes like GET /teams at mcpgateway/routers/teams.py:142 may not be fully gated by Layer 1 token scoping.

Risk: Medium. This is current documented behavior, not clearly a bug — but it warrants review given that token_teams=[] can arise from stale JWT/DB intersections (resolve_session_teams() returns [] when no overlap remains). Requires a design decision.


Suggested priority

  1. Gap 4 (token minting): Highest priority — allows privilege escalation via token creation.
  2. Gap 1 (explicit team_id bypass): High — enables cross-team RBAC bypass on mutable routes.
  3. Gap 2 + 6 (public-only + admin bypass): Medium — needs design decision on whether token_teams=[] should suppress admin bypass and/or team roles at Layer 2.
  4. Gap 3 (admin permission unscoped): Medium — limited to DB admins.
  5. Gap 5 (admin UI menu): Low — cosmetic.

Recommendation

Gaps 1, 3, and 4 are concrete implementation bugs that can each be addressed with a focused child issue. Gaps 2 and 6 require a design decision on the semantics of token_teams=[] before implementation. Gap 5 is low-priority cleanup.

Suggested fixes:

  • Gap 1: Add a token_teams check to the if team_id: branch in _get_user_roles() — reject if team_id not in token_teams when token_teams is a non-empty list.
  • Gap 2 + 6: Design decision — should token_teams=[] suppress admin bypass entirely and/or filter out team-scoped roles at Layer 2?
  • Gap 3: Add token_teams parameter to check_admin_permission() and forward it from require_admin_permission.
  • Gap 4: Pass current_user.get("token_teams") to get_user_permissions() in _get_caller_permissions().
  • Gap 5: Forward token_teams in the batch path of get_hidden_sections_for_user().

Metadata

Metadata

Labels

MUSTP1: Non-negotiable, critical requirements without which the product is non-functional or unsafebugSomething isn't workingrbacRole-based Access ControlsecurityImproves security

Type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions