fix(auth): ignore non-metadata JSON when probing for protected resource metadata - #1204
fix(auth): ignore non-metadata JSON when probing for protected resource metadata#1204easyinplay wants to merge 2 commits into
Conversation
…ce metadata The base URL is probed first when looking for RFC 9728 protected resource metadata, and any 200 there is taken to mean "this URL is the metadata document". Every field of ResourceServerMetadata is optional, so an unrelated JSON object deserializes into an all-None value and validation then fails hard with "Protected resource metadata missing required resource field". The error propagates out of resolve_metadata, so the .well-known fallbacks never run. Servers that answer GET / with a JSON health payload hit this even when they publish valid metadata at both well-known locations. Treat a parsed document that carries none of resource, authorization_server or authorization_servers as a soft failure, the same way this function already treats a non-200 status and a body that is not JSON. A document carrying any of those fields still goes through validate_resource_metadata_resource unchanged. This is the JSON-object half of modelcontextprotocol#810, which made a non-JSON body at the base URL a soft failure for the same reason.
github-actions
Bot
added
T-core
| if metadata.resource.is_none() | ||
| && metadata.authorization_server.is_none() | ||
| && metadata.authorization_servers.is_none() | ||
| { | ||
| debug!( | ||
| "response at {resource_metadata_url} is not a protected resource metadata document" | ||
| ); | ||
| return Ok(None); |
There was a problem hiding this comment.
discover_resource_metadata_url has already returned the base URL before rejecting the response body, so returning Ok(None) sends control back to resolve_metadata and skips all the protected-resource well-known candidates.
There was a problem hiding this comment.
You're right, and the first commit message named the mechanism without the fix following through: it says the base URL 200 makes the well-known fallbacks never run, and then only softened the failure that came after. Pushed a second commit that fixes it where you point.
The deciding line is in probe_resource_metadata_url:
match response.status() {
StatusCode::OK => Ok(Some(url.clone())),"200 means this url is the document" holds for the .well-known candidates it is called with in the loop, and not for the first call, which is passed the resource itself. RFC 9728 publishes the document at the well-known URI and advertises it through the resource_metadata parameter of a challenge, so a 200 from the resource is the resource answering. Only the 401 branch says anything at that first call, which is now probe_resource_endpoint_for_challenge; the .well-known probe is unchanged.
What that was costing, from the recorded requests with the old probe and a health payload at the base URL:
GET https://mcp.example.com/
GET https://mcp.example.com/
GET https://auth.example.com/.well-known/oauth-authorization-server
GET https://auth.example.com/.well-known/openid-configuration
The base URL twice, then straight to authorization server discovery. https://mcp.example.com/.well-known/oauth-protected-resource is never requested, so a document published there is unreachable no matter what the fetch does with the body.
The check from the first commit stays. A .well-known url can answer 200 with something unrelated too, and every field of ResourceServerMetadata being optional turns that into an all-None value that fails validation fatally rather than falling through.
Two tests, and both fail with the corresponding half reverted:
resolve_metadata_reaches_the_well_known_document_past_a_non_metadata_base_urlasserts the well-known url is actually requested and that resolution comes back asProtectedResourceMetadata. Restore the old probe call and it fails on the assertion above, printing the four requests.resolve_metadata_ignores_a_well_known_url_that_is_not_a_metadata_documentcovers the remaining guard. Drop the guard and it fails withMetadataError("Protected resource metadata missing required resource field"), the error this PR started from.
The test I had before asserted the fall-through you flagged, so it is gone.
cargo clippy --all-targets --all-features -- -D warnings is clean and cargo test -p rmcp --all-features is 499 passed, with default_http_client_preserves_connection_failure_cause failing identically on an untouched main here (it asserts a connection error string my platform words differently).
probe_resource_metadata_url treats any 200 as "this url is the metadata document". That holds for the .well-known candidates it is called with in the loop, and not for the first call, which is passed the resource itself. RFC 9728 publishes the document at the well-known URI and advertises it through the resource_metadata parameter of a WWW-Authenticate challenge, so a 200 from the resource is the resource answering and nothing more. Because that first probe returned Some(base_url), discovery ended before the .well-known candidates were tried, and a valid document published there was never reached. Rejecting the body later could not recover it: by then the candidates had already been skipped. Split the first probe into probe_resource_endpoint_for_challenge, which reads only the 401 branch. The .well-known probe keeps its behaviour. The check added in the previous commit stays. A .well-known url can also answer 200 with something that is not a metadata document, and every field of ResourceServerMetadata being optional makes that deserialize into an all-None value that then fails validation fatally. resolve_metadata_reaches_the_well_known_document_past_a_non_metadata_base_url asserts the well-known url is actually requested; without this change it fails with the base url requested twice and the protected-resource candidate never probed. resolve_metadata_ignores_a_well_known_url_that_is_not_a_metadata_document covers the remaining guard; without it the run ends in the original "Protected resource metadata missing required resource field".
2 participants
Why
AuthorizationManagerprobes its base URL first when looking for RFC 9728 protected resource metadata, andprobe_resource_metadata_urltreats any200there as "this URL is the metadata document".ResourceServerMetadatahas only optional fields, so an unrelated JSON object deserializes into an all-Nonevalue, andvalidate_resource_metadata_resourcethen fails hard withProtected resource metadata missing required resource field. The error propagates out ofresolve_metadata(), so the.well-knownfallbacks on the following lines never run.Servers that answer
GET /with a JSON health payload hit this. Againsthttps://mcp.tavily.comtoday:The server publishes valid metadata at both well-known locations, using the RFC 9728 path-insertion form.
resolve_metadata()still returnsMetadata error: Protected resource metadata missing required resource field, because it stops at the health payload and never reaches either. The workaround is to configure the endpoint path rather than the host, which is not always what the caller has.This is the JSON-object half of #810. That PR made a non-JSON body at the base URL a soft failure so discovery could continue. A JSON body that is not a metadata document still deserializes, so it takes the hard path instead.
Sampling the origin root of 36 reachable public remote MCP servers, 18 answer
200. All 18 are currently read as "this URL is the protected resource metadata document"; 17 of them escape only because their body is HTML or markdown rather than a JSON object.Standards
RFC 9728 Section 3 requires the metadata document to live at a URL formed by inserting a well-known URI string into the resource identifier, and Section 5.1 lets a
401point at it throughWWW-Authenticate. The 2026-07-28 authorization server discovery requirements list the same two mechanisms and no others. The resource URL itself is not a metadata location under either, so a200from it carries no metadata claim.RFC 9728 Section 3.2 allows additional members in the document, so tightening deserialization is not an option here: real documents carry fields this struct does not model, including the
bearer_methods_supportedin the Tavily document above.What this changes
fetch_resource_metadata_from_urlreturnsOk(None)when the parsed document has none ofresource,authorization_server, orauthorization_servers, with adebug!line, matching how the same function already handles a non-200 status and a body that is not JSON. Discovery then continues to the well-known paths and to authorization server metadata.A document carrying any of those fields still goes through
validate_resource_metadata_resourceunchanged, so a server that advertises a metadata URL throughWWW-Authenticateand serves a malformed document there still gets a hard error.protected_resource_discovery_rejects_missing_resourceandprotected_resource_discovery_rejects_mismatched_resourceboth reach the document through an explicit challenge pointer, and both stay green.Alternative
The narrower reading is that the base URL should never be treated as a metadata location at all, only as a source of a
WWW-Authenticatechallenge. That is what the TypeScript SDK does:discoverOAuthProtectedResourceMetadataonly ever fetches/.well-known/oauth-protected-resource{path}or a URL taken from the challenge. Scoping theStatusCode::OKarm ofprobe_resource_metadata_urlto the well-known probe would have the same effect here, and no existing test covers the base-URL-200 path, so that shape stays green too. I went with the document-shape check because it keeps working for servers that do serve metadata at the endpoint itself. Happy to send the other shape instead if you prefer it.Test plan
resolve_metadata_ignores_non_metadata_json_at_the_base_urlmirrorsresolve_metadata_reports_authorization_server_metadata, with the base URL answering a health payload twice instead of404: once for the probe, once for the fetch, which is what a real server does. Before the change it fails withand after it passes.
cargo test -p rmcp --lib --features authreports373 passed; 1 failed. The one failure isdefault_http_client_preserves_connection_failure_cause, which asserts on an OS connection-refused string and fails the same way on an unmodified checkout of this branch point on a non-English Windows host.