fix: include transport path in protected resource metadata URL (RFC 9728)#2665
Open
afischh wants to merge 2 commits into
Open
fix: include transport path in protected resource metadata URL (RFC 9728)#2665afischh wants to merge 2 commits into
afischh wants to merge 2 commits into
Conversation
…endpoint When a client uses HTTP Basic authentication (RFC 6749 §2.3.1), its client_id and client_secret arrive in the Authorization header rather than the request body. ClientAuthenticator already validates Basic auth correctly for `client_secret_basic` clients, but it failed early with "Missing client_id" when client_id was absent from form data. TokenHandler also required client_id in form data for TokenRequest validation, causing a second failure path. Changes: - ClientAuthenticator.authenticate_request: extract client_id from Basic auth header when not present in form body, before the missing-id check - TokenHandler.handle: populate client_id from the already-authenticated client_info when absent from form data, so TokenRequest validates cleanly Two new tests cover the authorization_code and refresh_token grant flows with client_id supplied only via Authorization header. Fixes modelcontextprotocol#1315 Signed-off-by: afischh <afischh@gmail.com>
…728) Per RFC 9728 §3, the `resource` field in `/.well-known/oauth-protected-resource` must identify the actual protected endpoint URL — e.g. `http://localhost:8000/mcp`, not the bare server base `http://localhost:8000/`. Without the path, VS Code Copilot and other spec-compliant clients reject the server with: Protected Resource Metadata resource "http://localhost:8000/" does not match MCP server resolved resource "http://localhost:8000/mcp" Fix: append `streamable_http_path` / `sse_path` to `resource_server_url` before passing it to `create_protected_resource_routes` and `build_resource_metadata_url` in both the lowlevel and mcpserver transports. Fixes modelcontextprotocol#1264 Signed-off-by: Alex Fisch <afischh@gmail.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.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.
$(cat <<'EOF'
Summary
Fixes #1264
The
resourcefield in/.well-known/oauth-protected-resourcewas set to thebare
resource_server_url(e.g.http://localhost:8000/) instead of the actualprotected endpoint URL (e.g.
http://localhost:8000/mcp).Per RFC 9728 §3, the
resource identifier must match the URL that clients use to access the protected
resource. This caused VS Code Copilot (and other spec-compliant clients) to reject
the server with:
Changes
src/mcp/server/lowlevel/server.py: appendstreamable_http_pathtoresource_server_urlbefore passing it tocreate_protected_resource_routesand
build_resource_metadata_url.src/mcp/server/mcpserver/server.py: same fix forsse_path.tests/server/auth/test_protected_resource.py: 4 new tests verifyingthe URL construction and metadata response for
/mcpand/ssetransport paths.Tests
Note: PR #2189 (same fix, closed 2026-03-05 without merge) was used as reference
for the correct approach.
EOF
)