fix(chore): centralize Layer-1 visibility filter across REST endpoints#4654
Open
bogdanmariusc10 wants to merge 1 commit intomainfrom
Open
Conversation
Replace 22 inline Layer-1 derivations with centralized get_scoped_resource_access_context() calls for consistency and maintainability. Changes: - Refactor 18 endpoints to use centralized helper - Preserve 4 exceptions (A2A federation, custom contracts, audit capture) - Update 13 test mocks from 3-tuple to 2-tuple pattern - Add regression tests for basic-auth admin behavior Bug fix: Basic-auth/dev-mode admins now correctly receive admin bypass (None, None) instead of public-only scope. The inline pattern missed non-JWT admin contexts; the centralized helper handles them correctly. Closes #4451 Signed-off-by: Bogdan-Marius-Catanus <bogdan-marius.catanus@ibm.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Related Issue
Closes #4451
📝 Summary
Centralized Layer-1 visibility filtering across mcpgateway/main.py REST endpoints by replacing 22 inline derivations with calls to
get_scoped_resource_access_context(request, user).Problem: The codebase contained 22 near-identical copies of the Layer-1 admin-bypass + public-only-secure-default derivation pattern. These copies drifted independently (different comments, missing assignments, mixed audit-context patterns), creating maintenance burden and regression risk.
Solution: Replace inline derivations with centralized helper from
mcpgateway/auth_context.py, preserving 4 exceptions that require custom handling for federation, audit capture, or custom return contracts.Bug Fix: The centralized helper correctly enables Layer-1 admin bypass for basic-auth/dev-mode admins (non-JWT contexts). The inline pattern missed this case because it only checked
is_adminfrom the JWT payload. This is a latent bug-fix documented in regression tests.🏷️ Type of Change
🧪 Verification
make lintmake testmake coverage✅ Checklist
make black isort pre-commit)📓 Notes
Changes Made
1. mcpgateway/main.py (22 refactorings)
Replaced inline pattern:
With centralized helper:
Endpoints refactored:
handle_completion,get_server,update_server,sse_endpoint,server_get_tools,server_get_resources,server_get_prompts,list_a2a_agents,list_tools,list_resources,list_resource_templates,list_prompts,read_resource,get_resource,subscribe_resource,get_prompt_no_args,get_prompt,get_prompt_with_args,get_gateway,update_gateway,delete_gatewaytools/list,gateways/list,resources/list,resources/subscribe,prompts/list,prompts/get,completion/complete2. Preserved 4 Exceptions
These endpoints retain the 3-tuple pattern for specific reasons:
get_a2a_agentuser_emailfor federation visibilityinvoke_a2a_agentuser_emailfor federationuser_idconstruction_get_internal_a2a_scope_context(user_email, teams)tools/list_req_email,_req_is_adminfor audit before applying rule3. Test Updates (13 methods)
Updated mocks from 3-tuple to 2-tuple pattern:
test_main.py (9 methods):
test_handle_completion_endpoint,test_handle_completion_endpoint_admin_bypass,test_handle_completion_endpoint_defaults_to_public_scope_when_token_teams_nonetest_list_tags_passes_token_scope,test_get_entities_by_tag_passes_public_only_scope,test_list_tags_admin_bypass_passes_unrestricted_scope,test_list_tags_defaults_to_public_scope_when_token_teams_none,test_get_entities_by_tag_admin_bypass_passes_unrestricted_scope,test_get_entities_by_tag_defaults_to_public_scope_when_token_teams_nonetest_rpc_completion_complete,test_rpc_completion_complete_admin_bypass,test_rpc_completion_complete_defaults_to_public_scope_when_token_teams_nonetest_main_extended.py (4 methods):
test_server_get_tools_admin_bypass,test_server_get_resources_public_scope,test_server_get_prompts_public_scope,test_list_tools_admin_bypass_and_public_only_default4. Regression Tests Added (2 new tests)
Added comprehensive tests documenting the basic-auth admin behavior change:
test_get_scoped_resource_access_context_basic_auth_admin_bypass: Verifies basic-auth admins get(None, None)for unrestricted access (bug-fix)test_get_scoped_resource_access_context_basic_auth_non_admin_public_only: Verifies non-admin basic-auth users maintain secure default(email, [])Impact
Code Quality:
Bug Fix:
(None, None)(email, [])due to inline pattern only checking JWTis_adminConsistency:
routers/teams.pymigration pattern from PR SSO (Entra ID) users cannot edit gateways/servers in admin UI — 'invalid team name' error despite ["*"] effective permissions #4070Design Decisions
Preserved 4 exceptions: These endpoints have legitimate reasons to use the 3-tuple pattern (federation, audit, custom contracts). Documented inline with comments.
Regression tests over inline comments: Added comprehensive tests documenting the basic-auth admin behavior change rather than just inline comments, ensuring the fix is verified.
Minimal scope: Only refactored main.py REST endpoints. Excluded
streamablehttp_transport.py(companion issue) androuters/teams.py(already migrated).Related Work