-
Notifications
You must be signed in to change notification settings - Fork 615
[BUG][SECURITY]: token_teams narrowing not enforced consistently across Layer 2 permission paths #3924
Description
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-408—check_admin_permission()accepts onlyuser_email, calls_is_user_admin()andget_user_permissions()withouttoken_teams.mcpgateway/middleware/rbac.py:787-792—require_admin_permissiondecorator callscheck_admin_permission(user_context["email"])without forwardinguser_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
- Gap 4 (token minting): Highest priority — allows privilege escalation via token creation.
- Gap 1 (explicit team_id bypass): High — enables cross-team RBAC bypass on mutable routes.
- 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. - Gap 3 (admin permission unscoped): Medium — limited to DB admins.
- 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_teamscheck to theif team_id:branch in_get_user_roles()— reject ifteam_id not in token_teamswhentoken_teamsis 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_teamsparameter tocheck_admin_permission()and forward it fromrequire_admin_permission. - Gap 4: Pass
current_user.get("token_teams")toget_user_permissions()in_get_caller_permissions(). - Gap 5: Forward
token_teamsin the batch path ofget_hidden_sections_for_user().