fix(transport): include app_root_path in enforced OAuth audience#5498
Open
C0d3N1nja97342 wants to merge 1 commit into
Open
fix(transport): include app_root_path in enforced OAuth audience#5498C0d3N1nja97342 wants to merge 1 commit into
C0d3N1nja97342 wants to merge 1 commit into
Conversation
Issue IBM#5172: _build_server_resource_url derived the enforced access-token aud from settings.app_domain only (f'{app_domain}/servers/{id}/mcp'), dropping app_root_path. On path-prefixed deployments (APP_ROOT_PATH set, e.g. gateway mounted under /gw), this diverged from the advertised Protected Resource Metadata URL (built via _build_public_base_url, which includes scope root_path) — so tokens minted for the advertised resource failed audience validation. Include settings.app_root_path between app_domain and /servers/ in the enforced audience. Backward compatible: empty app_root_path leaves the URL unchanged. Adds two tests: root_path included when set; URL unchanged when app_root_path is empty. Signed-off-by: keaixiaozhu <keaipiglet@163.com>
d33189d to
0f2663f
Compare
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 #5172
📝 Summary
_build_server_resource_urlinmcpgateway/transports/streamablehttp_transport.pyderives the enforced access-tokenaudfromsettings.app_domainonly (f"{app_domain}/servers/{id}/mcp"), droppingapp_root_path. On path-prefixed deployments (APP_ROOT_PATHset — e.g. gateway mounted under/gwbehind an ingress), this diverges from the advertised Protected Resource Metadata URL, which is built via_build_public_base_urland includesscope["root_path"]:{scheme}://{host}{root_path}/servers/{id}/mcp{APP_DOMAIN}/servers/{id}/mcp← missingroot_pathSo tokens minted for the advertised resource fail audience validation on path-prefixed deployments.
Fix: include
settings.app_root_pathbetweenapp_domainand/servers/in the enforced audience. Backward compatible — an emptyapp_root_pathleaves the URL unchanged.📏 Reviewability
triage—triage. Submitting the fix for review; happy to wait for triage removal / maintainer acceptance first if preferred.🏷️ Type of Change
Changes
mcpgateway/transports/streamablehttp_transport.py:_build_server_resource_urlnow appendssettings.app_root_path(normalized: stripped, leading/ensured) betweenapp_domainand/servers/{id}/mcp.tests/unit/mcpgateway/transports/test_streamablehttp_transport.py: two tests —app_root_path="/gw"→ URL includes/gw; emptyapp_root_path→ URL unchanged.Testing
uv run pytest tests/unit/mcpgateway/transports/test_streamablehttp_transport.py -k build_server_resource_url→ 2 passed.uv run pytest tests/unit/mcpgateway/test_auth.py -k "resource or audience or canonical or oauth"→ 111 passed.ruff checkclean on the changed file.Notes for Reviewer
_build_public_base_urlalready includesscope["root_path"], using the operator-setsettings.app_root_path(same trust anchor asapp_domain, consistent with the function's existing security note)._build_server_resource_url(the enforced-aud path). The advertised-resource path (_build_resource_metadata_url→_build_public_base_url) already includesroot_pathand is unchanged.