Skip to content

fix(auth): ignore non-metadata JSON when probing for protected resource metadata #1204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
easyinplay wants to merge 2 commits into
base: main
Choose a base branch
from
+150 −6
Open

fix(auth): ignore non-metadata JSON when probing for protected resource metadata #1204

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0bfb2d8
fix(auth): ignore non-metadata JSON when probing for protected resour…
easyinplay Aug 23, 2026
09c5e51
fix(auth): stop a 200 from the resource ending metadata discovery
easyinplay Aug 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 150 additions & 6 deletions crates/rmcp/src/transport/auth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2606,9 +2606,7 @@ impl AuthorizationManager {
}

async fn discover_resource_metadata_url(&self) -> Result<Option<Url>, AuthError> {
if let Some(resource_metadata_url) =
self.probe_resource_metadata_url(&self.base_url).await?
{
if let Some(resource_metadata_url) = self.probe_resource_endpoint_for_challenge().await? {
return Ok(Some(resource_metadata_url));
}

Expand All @@ -2631,10 +2629,37 @@ impl AuthorizationManager {
Ok(None)
}

/// Probe `url` with a GET, extracting the resource metadata url from a
/// 200 (the url itself is the metadata document) or from a 401's
/// WWW-Authenticate header value.
/// Probe the resource itself, looking only for a `WWW-Authenticate` challenge
/// that carries a `resource_metadata` pointer.
///
/// A 200 here says nothing about metadata. RFC 9728 publishes the document at
/// the well-known URI and advertises it through the challenge parameter, so the
/// resource answering its own GET is not the document and must not end
/// discovery before the well-known candidates are tried.
/// https://www.rfc-editor.org/rfc/rfc9728.html#name-use-of-www-authenticate-for
async fn probe_resource_endpoint_for_challenge(&self) -> Result<Option<Url>, AuthError> {
let response = self
.discovery_get(&self.base_url)
.await
.map_err(|error| Self::discovery_failed(&self.base_url, error))?;

if response.status() == StatusCode::UNAUTHORIZED {
return Ok(self
.extract_resource_metadata_url_from_www_authenticate(&response)
.await);
}

debug!(
"resource endpoint probe returned {}, no WWW-Authenticate pointer to follow",
response.status()
);
Ok(None)
}

/// Probe a `.well-known` candidate with a GET, extracting the resource metadata
/// url from a 200 (the url itself is the metadata document) or from a 401's
/// WWW-Authenticate header value.
/// https://www.rfc-editor.org/rfc/rfc9728.html#name-obtaining-protected-resourc
async fn probe_resource_metadata_url(&self, url: &Url) -> Result<Option<Url>, AuthError> {
let response = self
.discovery_get(url)
Expand Down Expand Up @@ -2701,6 +2726,23 @@ impl AuthorizationManager {
return Ok(None);
}
};

// Every field of `ResourceServerMetadata` is optional, so an unrelated JSON
// object deserializes into an all-`None` value and then fails validation
// fatally. RFC 9728 requires `resource`, and MCP requires an authorization
// server reference, so a document carrying neither is not a protected
// resource metadata document. Treat it as a soft failure, the same way this
// function already treats a non-200 status and a body that is not JSON.
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);
Comment on lines +2736 to +2743

@DaleSeo DaleSeo Aug 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@easyinplay easyinplay Aug 25, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_url asserts the well-known url is actually requested and that resolution comes back as ProtectedResourceMetadata. 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_document covers the remaining guard. Drop the guard and it fails with MetadataError("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).

@DaleSeo DaleSeo Aug 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When WWW-Authenticate points to the metadata URL, the server is explicitly saying that this document contains the metadata. If the body is {}, it used to raise a hard missing required resource field error. Now it returns Ok(None), so resolve_metadata_from_challenge continues with authorization server discovery and then falls back to the legacy endpoint. As a result, it drops the RFC 8707 resource binding without reporting it.

}

Ok(Some(metadata))
}

Expand Down Expand Up @@ -4853,6 +4895,108 @@ mod tests {
);
}

#[tokio::test]
async fn resolve_metadata_reaches_the_well_known_document_past_a_non_metadata_base_url() {
let document = || {
http_response(
200,
serde_json::json!({
"resource": "https://mcp.example.com/",
"authorization_servers": ["https://auth.example.com"]
}),
)
};
let client = RecordingOAuthHttpClient::with_responses(vec![
// the MCP endpoint answers GET with a health payload, not metadata
http_response(
200,
serde_json::json!({"status": "healthy", "message": "MCP server is running"}),
),
// the well-known candidate carries the real document: probed, then fetched
document(),
document(),
http_response(
200,
serde_json::json!({
"issuer": "https://auth.example.com",
"authorization_endpoint": "https://auth.example.com/authorize",
"token_endpoint": "https://auth.example.com/token"
}),
),
]);
let recorder = client.clone();
let manager = AuthorizationManager::new_with_oauth_http_client(
"https://mcp.example.com/",
Arc::new(client),
)
.await
.unwrap();

let resolution = manager.resolve_metadata().await.unwrap();

assert_eq!(
(
resolution.source,
resolution.metadata.token_endpoint.as_str(),
),
(
AuthorizationMetadataSource::ProtectedResourceMetadata,
"https://auth.example.com/token",
)
);
assert!(
recorder.requests().iter().any(|request| {
request.uri == "https://mcp.example.com/.well-known/oauth-protected-resource"
}),
"the well-known candidate was never probed: {:?}",
recorder.requests()
);
}

#[tokio::test]
async fn resolve_metadata_ignores_a_well_known_url_that_is_not_a_metadata_document() {
let health = || {
http_response(
200,
serde_json::json!({"status": "healthy", "message": "MCP server is running"}),
)
};
let client = RecordingOAuthHttpClient::with_responses(vec![
// the MCP endpoint answers GET with a health payload, not metadata
health(),
// so does the well-known candidate: probed, then fetched
health(),
health(),
http_response(
200,
serde_json::json!({
"issuer": "https://mcp.example.com",
"authorization_endpoint": "https://mcp.example.com/oauth/authorize",
"token_endpoint": "https://mcp.example.com/oauth/token"
}),
),
]);
let manager = AuthorizationManager::new_with_oauth_http_client(
"https://mcp.example.com/",
Arc::new(client),
)
.await
.unwrap();

let resolution = manager.resolve_metadata().await.unwrap();

assert_eq!(
(
resolution.source,
resolution.metadata.token_endpoint.as_str(),
),
(
AuthorizationMetadataSource::AuthorizationServerMetadata,
"https://mcp.example.com/oauth/token",
)
);
}

#[rstest]
#[case::protected_resource_metadata(
AuthorizationMetadataSource::ProtectedResourceMetadata,
Expand Down
Loading