fix: treat resource metadata JSON parse failure as soft error - #810
Merged
alexhancock merged 1 commit intoApr 16, 2026
Merged
Conversation
In fetch_resource_metadata_from_url, a JSON parse failure on the response body caused a fatal AuthError::MetadataError, preventing discover_metadata() from falling through to direct .well-known/oauth-authorization-server discovery (Strategy B). MCP servers that return HTTP 200 with non-JSON content (e.g. HTML) at their base URL caused the OAuth flow to abort entirely, even when the server had a valid .well-known/oauth-authorization-server endpoint. Return Ok(None) on parse failure, consistent with how HTTP errors are already handled in the same function.
github-actions
Bot
added
T-core
Contributor
Author
|
Note that an alternative here would be to not try to parse this response as metadata at all, since fetching the MCP server URL itself and treating it as the metadata isn't part of the discovery flow in the spec. But I am assuming this variation from the spec is intentional for some reason (compatibility with non-compliant servers maybe?) so kept this change minimal. |
jh-block
added a commit
to aaif-goose/goose
that referenced
this pull request
Apr 16, 2026
…vers When a remote MCP server returns HTTP 200 with non-JSON content at its base URL, rmcp's resource metadata discovery fatally errors instead of falling through to .well-known/oauth-authorization-server discovery. This prevents the OAuth browser flow from ever opening. Add a fallback in oauth_flow that catches the discovery failure and retries by fetching .well-known/oauth-authorization-server directly. Also log the actual error when oauth_flow fails instead of silently swallowing it. Upstream fix: modelcontextprotocol/rust-sdk#810
This was referenced Apr 16, 2026
alexhancock
approved these changes
Apr 16, 2026
Merged
88plug
added a commit
to 88plug/goose-plus
that referenced
this pull request
Jul 5, 2026
…ata bug OAuthState::start_authorization_with_metadata_url fails fatally for some remote MCP servers (non-JSON HTTP 200 at the base URL, auth server on a different host, path-based deployments), silently breaking the OAuth browser flow. Add a spec-compliant fallback that discovers Protected Resource Metadata (RFC 9728) and Authorization Server Metadata (RFC 8414) directly via well-known URLs, then builds the session through rmcp's public APIs. Ports jhugo/fix-issue-8453 (upstream commits 1637c3d15, 74c283e2b), working around modelcontextprotocol/rust-sdk#810.
DaleSeo
pushed a commit
that referenced
this pull request
Sep 12, 2026
…ce metadata (#1204) * fix(auth): ignore non-metadata JSON when probing for protected resource 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 #810, which made a non-JSON body at the base URL a soft failure for the same reason. * fix(auth): stop a 200 from the resource ending metadata discovery 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". * fix(auth): keep trying the well-known candidates past a non-metadata document discover_resource_metadata_url returned the first .well-known candidate that answered 200 and left the loop; the document itself was fetched afterwards, outside the loop. A candidate answering 200 with something that is not metadata is only recognised at that point, by which time the remaining candidates have been skipped and discovery gives up with nothing. The probe already had the body in hand and threw it away, so the winning candidate was requested twice. Read the body where the candidate is probed instead: a candidate that is not the document costs one request and the loop moves on to the next one, and the candidate that is the document is requested once. The second half of the old function, which walks the authorization servers a document names, is unchanged; it now takes the document as an argument so the challenge path keeps sharing it. resolve_metadata_tries_the_next_well_known_candidate_past_a_non_metadata_document covers the loop; without this change the second candidate is never requested and the run ends on the issuer of an authorization server it was never meant to reach. * fix(auth): report an advertised url that is not the metadata document A WWW-Authenticate challenge naming a resource_metadata url is the server saying the document is there. The previous commits made a document carrying neither resource nor an authorization server reference a soft failure everywhere, which on that path drops out of resolve_metadata_from_challenge, continues with authorization server discovery and settles on the legacy endpoints, silently losing the RFC 8707 resource binding the document was supposed to carry. Before those commits it surfaced as "Protected resource metadata missing required resource field". Where the url came from decides what that document means. A .well-known candidate is a guess, so its answer only rules out that candidate and the loop goes on. An advertised url has no better alternative to move on to, so say what the server got wrong instead of degrading quietly. Reading a non-200 or a body that is not JSON stays a soft failure on both paths. resolve_metadata_from_challenge_reports_an_advertised_url_without_metadata covers the challenge path; without this change it resolves to LegacyEndpointFallback. * fix(auth): let a candidate fail validation without ending discovery parse_resource_metadata decides what a body that is not the metadata document means from where the url came from, and it decides it on the presence of resource, authorization_server and authorization_servers alone. What comes after is fatal whatever the origin: validate_resource_metadata_resource rejects a missing resource, a resource that is not a URL, one carrying a fragment, and one that does not match the base url, and each of those propagates out of resolve_metadata. A .well-known candidate handled by a catch-all route reaches it. The {"error":"not_found","resource":"/.well-known/oauth-protected-resource"} such a route answers with carries resource, so the presence check lets it through, and the relative path then fails to parse as a URL. That is the shape this branch opened on, one field further along: the remaining candidates are skipped, authorization server discovery is skipped, and the run ends on an error instead of the legacy endpoints. Read the body and validate it in the same place, now read_resource_metadata, so the origin governing the first decision governs the second one as well. A guess that fails validation rules out that candidate and the loop goes on; an advertised url still reports what the server got wrong. authorization_metadata_from_resource_metadata takes a document that has already been accepted. resolve_metadata_tries_the_next_well_known_candidate_past_an_unusable_document covers it; without this change it fails with "Protected resource metadata resource field is not a valid URL". * fix(auth): keep probing the candidates when an advertised url answers nothing An advertised url reading a non-200 or a body that is not JSON is a soft failure: the server said the document is at that url and nothing is being served there, which rules out the url and not the document. The .well-known candidates are derived from the base url rather than from that pointer, so they are still worth probing, but the pointer was read ahead of the loop and returned out of discover_resource_metadata either way. A challenge naming https://host/.well-known/oauth-protected-resource that 404s takes the run straight to authorization server discovery, while the document sits unread on https://host/mcp/.well-known/oauth-protected-resource. Fall through to the loop instead. Carrying on past a candidate's own 401 already let one url be requested twice, because the pointer that challenge names can be a later candidate of the same run, and it is requested again when the loop reaches it; not returning on the first pointer adds the same overlap. Keep the urls this run has requested and skip the ones already read. resolve_metadata_probes_the_candidates_past_an_advertised_url_that_is_not_served covers the fall-through; without it the run reaches the authorization server metadata of a server it was never pointed at. resolve_metadata_requests_an_advertised_url_that_is_also_a_candidate_once covers the repeat. * fix(auth): walk an authorization server named by both fields once A protected resource metadata document can name its authorization server in authorization_servers, and servers written against the earlier draft also fill the singular authorization_server. Filling both with the same value is common, and the two are concatenated into the candidate list unfiltered, so every well-known form of that one server's discovery url is requested twice before the walk gives up on it. well_known_paths already keeps its candidates distinct. Do the same here, comparing the trimmed value the loop goes on to use. resolve_metadata_requests_an_authorization_server_named_twice_once covers it; without this change the discovery url is requested twice. --------- Co-authored-by: easyinplay <easyinplay@users.noreply.github.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
2 participants
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.
Summary
fetch_resource_metadata_from_urlreturns a fatalAuthError::MetadataErrorwhen the resource metadata response body cannot be parsed as JSON. This preventsdiscover_metadata()from falling through to direct.well-known/oauth-authorization-serverdiscovery (Strategy B).Problem
MCP servers that return HTTP 200 with non-JSON content (e.g. HTML) at their base URL cause the resource metadata probe to "succeed" at the HTTP level (200 = "this is the metadata URL"), but then the JSON deserialization fails fatally. The OAuth flow aborts entirely even though the server has a valid
.well-known/oauth-authorization-serverendpoint.Users see "Auth required" with no OAuth browser flow ever opening.
Fix
Treat JSON parse errors as a soft failure — return
Ok(None)with adebug!log — consistent with how HTTP errors (non-200 status codes) are already handled in the same function. This allowsdiscover_metadata()to continue and attempt direct authorization server metadata discovery.Test plan
cargo check -p rmcp --features auth.well-known/oauth-authorization-serverdiscovery instead of aborting