fix(sso): SSO users with platform_admin role can now access teams in admin UI#4080
Conversation
- Fixed list_teams endpoint to check RBAC permissions in addition to is_admin flag - SSO users with platform_admin role now correctly see all teams - Maintains backward compatibility with local admins using is_admin flag - Added missing docstring parameter for flake8 compliance Closes #4070 Signed-off-by: Bogdan-Marius-Catanus <bogdan-marius.catanus@ibm.com>
Signed-off-by: Bogdan-Marius-Catanus <bogdan-marius.catanus@ibm.com>
…servers-in-admin-ui-invalid-team-name-error-despite-effective-permissions
There was a problem hiding this comment.
@bogdanmariusc10 - thanks for the contribution!
Summary
The PR attempts to fix SSO users with platform_admin RBAC role being blocked from the GET /teams endpoint. The diagnosis in the issue is correct, but the implementation is broken: the permissions key checked in current_user_ctx is never set by get_current_user_with_permissions, so the RBAC branch of the fix is dead code and the endpoint's behavior is unchanged from before.
Findings
Blocking
mcpgateway/routers/teams.py:178-184 — The fix is a no-op
get_current_user_with_permissions (middleware/rbac.py:401-414) never populates a permissions key in the returned dict. The dict contains: email, full_name, is_admin, ip_address, user_agent, db, auth_method, request_id, team_id, token_teams, token_use, plugin_context_table, plugin_global_context — and nothing else. Both "*" in current_user_ctx.get("permissions", []) and "teams.*" in current_user_ctx.get("permissions", []) will always evaluate to False, leaving the only active branch identical to the original is_admin check. SSO users with platform_admin are still blocked in production.
The correct fix is to call PermissionService.check_admin_permission() directly inside list_teams, mirroring exactly what the require_admin decorator already does at rbac.py:854-860:
from mcpgateway.services.permission_service import PermissionService
from mcpgateway.db import fresh_db_session
# Inside list_teams:
with fresh_db_session() as db_perm:
permission_service = PermissionService(db_perm)
has_admin_team_access = await permission_service.check_admin_permission(
current_user_ctx["email"],
token_teams=current_user_ctx.get("token_teams"),
)check_admin_permission already handles both is_admin=True (DB flag) and wildcard RBAC permissions (, admin.) via _is_user_admin + get_user_permissions, so no additional branching is needed.
tests/unit/mcpgateway/routers/test_teams.py:69,79 — Test fixtures mask the bug (Test coverage)
The mock context dicts inject a "permissions" key (e.g., "permissions": ["*"]) that does not exist in the real production context. This causes the tests to validate behavior that can never occur at runtime — the fix appears to pass tests while silently failing in production. The fixtures should be corrected to match the actual structure returned by get_current_user_with_permissions, and a dedicated test case should be added: an SSO user with is_admin=False but platform_admin RBAC role in the DB should receive the full team list from list_teams, with PermissionService.check_admin_permission mocked to return True.
mcpgateway/routers/teams.py:99,364 — Same SSO gap in adjacent endpoints
Two other endpoints in the same file have the same root problem. create_team (line 99) uses is_admin to decide whether to bypass allow_team_creation and skip_limits. update_team (line 364) uses is_admin for skip_limits. Both will still misclassify SSO platform_admin users as non-admins. The same check_admin_permission pattern should be applied to both.
Please resolve conflicts in observability.py and add extra unit tests to cover the changes.
…min RBAC Replace broken permissions key checks with proper PermissionService.check_admin_permission() calls in create_team, list_teams, and update_team endpoints. This fixes SSO users with platform_admin RBAC role (is_admin=False in DB) being blocked from team management. - Use fresh_db_session() and PermissionService for admin checks in 3 endpoints - Fix test fixtures to remove non-existent permissions key - Add mock_permission_check helper for consistent test mocking - Add 3 SSO platform_admin test cases - Fix 50+ tests to properly mock PermissionService and fresh_db_session Closes #4070 Related to #4080 Signed-off-by: Bogdan-Marius-Catanus <bogdan-marius.catanus@ibm.com>
…min RBAC Replace broken permissions key checks with proper PermissionService.check_admin_permission() calls in create_team, list_teams, and update_team endpoints. This fixes SSO users with platform_admin RBAC role (is_admin=False in DB) being blocked from team management. - Use fresh_db_session() and PermissionService for admin checks in 3 endpoints - Fix test fixtures to remove non-existent permissions key - Add mock_permission_check helper for consistent test mocking - Add 3 SSO platform_admin test cases - Fix 50+ tests to properly mock PermissionService and fresh_db_session Closes #4070 Related to #4080 Signed-off-by: Bogdan-Marius-Catanus <bogdan-marius.catanus@ibm.com>
4044aed to
91862b5
Compare
…servers-in-admin-ui-invalid-team-name-error-despite-effective-permissions
marekdano
left a comment
There was a problem hiding this comment.
Fixes a real bug where SSO/Entra ID users with platform_admin RBAC role were blocked from viewing teams in the admin UI because GET /teams checked the legacy is_admin DB flag rather than RBAC permissions. The three affected endpoints (create_team, list_teams, update_team) now call PermissionService.check_admin_permission() which correctly handles both the legacy flag and RBAC.
LGTM 🚀
Signed-off-by: Bogdan-Marius-Catanus <bogdan-marius.catanus@ibm.com>
…servers-in-admin-ui-invalid-team-name-error-despite-effective-permissions
…x SSO admin bypass Closes #4452 This commit addresses two related Layer-1 hygiene issues in the StreamableHTTP transport that prevented SSO platform_admins from getting proper admin bypass: Issue A: Eight near-identical inline copies of the admin-bypass + public-only secure-default derivation scattered across MCP handlers (call_tool, list_tools, list_prompts, read_resource, list_resources, list_resource_templates, completion, get_prompt). Issue B: The primary auth path (_StreamableHttpAuthHandler._auth_jwt) computed effective DB/RBAC admin status but stored the raw JWT is_admin claim in user_context, causing SSO platform_admins to be treated as non-admins for Layer-1 visibility filtering. Changes: 1. Created get_scoped_visibility_from_user_context() helper in auth_context.py - Implements secure default: empty/None contexts return (None, []) not (None, None) - Handles admin bypass: is_admin=True with teams=None returns (None, None) - Documented with examples and security notes 2. Fixed _auth_jwt primary path (lines 5162-5175) - Computes effective_is_admin = db_user_is_admin or jwt_is_admin - Uses effective admin for both is_admin and permission_is_admin fields - Ensures SSO platform_admins get Layer-1 admin bypass 3. Fixed _normalize_jwt_payload fallback path (lines 2097-2128) - Added DB admin check via is_user_admin() for stateful sessions - Computes effective_is_admin = db_user_is_admin or jwt_is_admin - Maintains parity with primary auth path 4. Replaced 8 inline derivations with centralized helper calls: - call_tool (line 1647) - read_resource (line 2176) - list_tools (line 2297) - get_prompt (line 2363) - list_prompts (line 2440) - list_resources (line 2546) - list_resource_templates (line 2670) - completion (line 2814) 5. Added comprehensive test coverage (10 new tests, all passing): - Empty/None context security tests - Admin bypass scenarios (JWT admin, DB admin, combined) - SSO platform_admin regression tests (issue #4070) - Basic-auth/dev-mode admin tests - _normalize_jwt_payload effective admin tests Security Impact: - Fixes issue #4070: SSO users with platform_admin RBAC role now get Layer-1 admin bypass on StreamableHTTP transport, matching REST API behavior - Maintains secure defaults: unauthenticated/empty contexts get public-only access, never admin bypass - Preserves backward compatibility: basic-auth admins continue to work in dev mode Related: #4070, #4080 Signed-off-by: Bogdan-Marius-Catanus <bogdan-marius.catanus@ibm.com>
🔗 Related Issue
Closes #4070
📝 Summary
Fixed a bug where SSO-authenticated users with
platform_adminRBAC role could not edit gateways or virtual servers in the admin UI, receiving "invalid team name" errors despite having["*"](wildcard) effective permissions.Root Cause: The
GET /teamsendpoint incorrectly checked only the databaseis_adminflag instead of RBAC permissions. SSO users have the correct RBAC role (platform_adminwith["*"]permissions) but lack the database flag (is_admin=False), causing them to be treated as regular users.Solution: Updated the
list_teamsendpoint to check both the legacyis_adminflag AND RBAC permissions (["*"]or["teams.*"]), aligning with ContextForge's two-layer security model.🏷️ Type of Change
🧪 Verification
make lintmake testmake coverage✅ Checklist
make black isort pre-commit)📓 Notes
Changes Made
1.
mcpgateway/routers/teams.py(Primary Fix)2.
mcpgateway/observability.py(Linting Fix)messageparameter documentation to fix flake8 DAR101 errorImpact
Before:
platform_adminrole: blocked from editing in admin UIis_admin=True: worked correctlyAfter:
platform_adminrole: can now edit in admin UIis_admin=True: continue to workWhy This Matters
ContextForge has two separate concepts:
is_adminflag - Legacy database field (EmailUser.is_admin)platform_adminrole - Modern RBAC role with["*"]permissionsSSO users get the RBAC role but not the database flag. The fix ensures both mechanisms are checked, maintaining backward compatibility while supporting SSO properly.
Code Quality Fix
Added missing docstring parameter documentation for flake8 compliance.