From 50ccb1f30016dab9368531daf638c3bc4ed6e45e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20Kone=C4=8Dn=C3=BD?= Date: Tue, 7 Jul 2026 10:58:43 +0200 Subject: [PATCH 1/4] feat(transport): Resource from OAuth2 resource metadata document is preferred over default base url if present --- crates/rmcp/src/transport/auth.rs | 77 ++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 2 deletions(-) diff --git a/crates/rmcp/src/transport/auth.rs b/crates/rmcp/src/transport/auth.rs index 35c4c164..3543fe8c 100644 --- a/crates/rmcp/src/transport/auth.rs +++ b/crates/rmcp/src/transport/auth.rs @@ -781,6 +781,8 @@ pub struct AuthorizationManager { www_auth_scopes: RwLock>, /// scopes_supported from protected resource metadata (RFC 9728) resource_scopes: RwLock>, + /// resource indicator from protected resource metadata, used for RFC 8707 `resource` + discovered_resource: RwLock>, /// OIDC Dynamic Client Registration `application_type` (SEP-837) application_type: Option, } @@ -995,6 +997,7 @@ impl AuthorizationManager { scope_upgrade_config: ScopeUpgradeConfig::default(), www_auth_scopes: RwLock::new(Vec::new()), resource_scopes: RwLock::new(Vec::new()), + discovered_resource: RwLock::new(None), application_type: Some(DEFAULT_APPLICATION_TYPE.to_string()), }; @@ -1314,7 +1317,7 @@ impl AuthorizationManager { let mut auth_request = oauth_client .authorize_url(CsrfToken::new_random) .set_pkce_challenge(pkce_challenge) - .add_extra_param("resource", self.base_url.to_string()); + .add_extra_param("resource", self.oauth_resource().await); // add request scopes for scope in scopes { @@ -1351,6 +1354,14 @@ impl AuthorizationManager { Ok(auth_url.to_string()) } + async fn oauth_resource(&self) -> String { + self.discovered_resource + .read() + .await + .clone() + .unwrap_or_else(|| self.base_url.to_string()) + } + /// get the current granted scopes pub async fn get_current_scopes(&self) -> Vec { self.current_scopes.read().await.clone() @@ -1562,7 +1573,7 @@ impl AuthorizationManager { let token_result = match oauth_client .exchange_code(AuthorizationCode::new(code.to_string())) .set_pkce_verifier(pkce_verifier) - .add_extra_param("resource", self.base_url.to_string()) + .add_extra_param("resource", self.oauth_resource().await) .request_async(&OAuth2HttpClient { client: self.http_client.as_ref(), redirect_policy: OAuthHttpRedirectPolicy::Stop, @@ -3264,6 +3275,10 @@ mod tests { let metadata = manager.discover_metadata().await.unwrap(); assert_eq!(metadata.token_endpoint, "https://auth.example.com/token"); + assert_eq!( + manager.discovered_resource.read().await.as_deref(), + Some("https://api.mcp.example.com") + ); assert_eq!( client.requests(), vec![ @@ -4421,6 +4436,64 @@ mod tests { assert!(scope.contains("write")); } + #[tokio::test] + async fn authorization_url_uses_discovered_resource() { + let base_url = "https://mcp.example.com/mcp"; + let auth_endpoint = "https://auth.example.com/authorize"; + let mut manager = AuthorizationManager::new(base_url).await.unwrap(); + + let metadata = AuthorizationMetadata { + authorization_endpoint: auth_endpoint.to_string(), + token_endpoint: "https://auth.example.com/token".to_string(), + registration_endpoint: None, + issuer: None, + jwks_uri: None, + scopes_supported: None, + response_types_supported: Some(vec!["code".to_string()]), + code_challenge_methods_supported: Some(vec!["S256".to_string()]), + additional_fields: std::collections::HashMap::new(), + }; + manager.set_metadata(metadata); + manager.configure_client_id("test-client-id").unwrap(); + *manager.discovered_resource.write().await = Some("https://mcp.example.com".to_string()); + + let auth_url = manager.get_authorization_url(&["read"]).await.unwrap(); + let parsed = Url::parse(&auth_url).unwrap(); + let params: std::collections::HashMap<_, _> = parsed.query_pairs().collect(); + + assert_eq!( + params.get("resource").map(|v| v.as_ref()), + Some("https://mcp.example.com") + ); + } + + #[tokio::test] + async fn authorization_url_uses_default_resource_without_protected_resource_document() { + let base_url = "https://mcp.example.com/mcp"; + let auth_endpoint = "https://auth.example.com/authorize"; + let mut manager = AuthorizationManager::new(base_url).await.unwrap(); + + let metadata = AuthorizationMetadata { + authorization_endpoint: auth_endpoint.to_string(), + token_endpoint: "https://auth.example.com/token".to_string(), + registration_endpoint: None, + issuer: None, + jwks_uri: None, + scopes_supported: None, + response_types_supported: Some(vec!["code".to_string()]), + code_challenge_methods_supported: Some(vec!["S256".to_string()]), + additional_fields: std::collections::HashMap::new(), + }; + manager.set_metadata(metadata); + manager.configure_client_id("test-client-id").unwrap(); + + let auth_url = manager.get_authorization_url(&["read"]).await.unwrap(); + let parsed = Url::parse(&auth_url).unwrap(); + let params: std::collections::HashMap<_, _> = parsed.query_pairs().collect(); + + assert_eq!(params.get("resource").map(|v| v.as_ref()), Some(base_url)); + } + #[test] fn authorization_callback_parses_optional_issuer() { let callback = AuthorizationCallback::from_redirect_url( From 1e453b1e008e7fd8a6c880befe9efe3f7f784a2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20Kone=C4=8Dn=C3=BD?= Date: Wed, 8 Jul 2026 15:20:57 +0200 Subject: [PATCH 2/4] feat(transport): Write discovered_resource after successful oauth resource metadata document validation --- crates/rmcp/src/transport/auth.rs | 99 ++++++++++++++++++++++--------- 1 file changed, 72 insertions(+), 27 deletions(-) diff --git a/crates/rmcp/src/transport/auth.rs b/crates/rmcp/src/transport/auth.rs index 3543fe8c..2357ef1d 100644 --- a/crates/rmcp/src/transport/auth.rs +++ b/crates/rmcp/src/transport/auth.rs @@ -1869,6 +1869,11 @@ impl AuthorizationManager { self.validate_resource_metadata_resource(&resource_metadata)?; + self.discovered_resource + .write() + .await + .replace(resource_metadata.resource.clone().unwrap_or_default()); + // store scopes_supported from protected resource metadata for select_scopes() if let Some(scopes) = resource_metadata.scopes_supported { if !scopes.is_empty() { @@ -1932,9 +1937,21 @@ impl AuthorizationManager { )); }; - if !Self::resource_identifiers_match(self.base_url.as_str(), resource) { + let Some(resource_url) = Url::parse(resource).ok() else { + return Err(AuthError::MetadataError( + "Protected resource metadata resource field is not a valid URL".to_string(), + )); + }; + + if resource_url.fragment().is_some() { + return Err(AuthError::MetadataError( + "Protected resource metadata resource does not permit fragment in URL as specified by RFC 8707".to_string() + )); + } + + if !Self::is_resource_identifier_valid(&self.base_url, &resource_url) { return Err(AuthError::MetadataError(format!( - "Protected resource metadata resource mismatch: expected '{}', got '{}'", + "Protected resource metadata resource mismatch: reference '{}', permitted '{}'", self.base_url, resource ))); } @@ -1942,17 +1959,29 @@ impl AuthorizationManager { Ok(()) } - fn resource_identifiers_match(expected: &str, actual: &str) -> bool { - expected == actual - || (Self::is_root_resource_identifier(expected) - && actual == expected.trim_end_matches('/')) - || (Self::is_root_resource_identifier(actual) - && expected == actual.trim_end_matches('/')) - } + fn is_resource_identifier_valid(provided: &Url, reference: &Url) -> bool { + if provided == reference { + return true; + } - fn is_root_resource_identifier(value: &str) -> bool { - Url::parse(value) - .is_ok_and(|url| url.path() == "/" && url.query().is_none() && url.fragment().is_none()) + if provided.scheme() != reference.scheme() + || provided.host_str() != reference.host_str() + || provided.port_or_known_default() != reference.port_or_known_default() + { + return false; + } + + let provided_path = provided.path(); + let reference_path = reference.path(); + + // URL query part supported, even if it is discouraged in RFC 8707 + if provided_path == reference_path && provided.query() == reference.query() { + return true; + } + + provided_path.starts_with(reference_path) + && provided.query().is_none() + && reference.query().is_none() } async fn discover_resource_metadata_url(&self) -> Result, AuthError> { @@ -3253,7 +3282,7 @@ mod tests { http_response( 200, serde_json::json!({ - "resource": "https://mcp.example.com/mcp", + "resource": "https://mcp.example.com", "authorization_servers": ["https://auth.example.com"] }), ), @@ -3277,7 +3306,7 @@ mod tests { assert_eq!(metadata.token_endpoint, "https://auth.example.com/token"); assert_eq!( manager.discovered_resource.read().await.as_deref(), - Some("https://api.mcp.example.com") + Some("https://mcp.example.com") ); assert_eq!( client.requests(), @@ -3499,23 +3528,39 @@ mod tests { } #[test] - fn resource_identifier_matching_allows_only_root_trailing_slash_difference() { - assert!(AuthorizationManager::resource_identifiers_match( - "https://mcp.example.com/", - "https://mcp.example.com" + fn resource_identifier_matching_allows_matching_host_or_parent_path() { + assert!(AuthorizationManager::is_resource_identifier_valid( + &Url::parse("https://mcp.example.com/").unwrap(), + &Url::parse("https://mcp.example.com").unwrap() + )); + assert!(AuthorizationManager::is_resource_identifier_valid( + &Url::parse("https://mcp.example.com").unwrap(), + &Url::parse("https://mcp.example.com/").unwrap() )); - assert!(AuthorizationManager::resource_identifiers_match( - "https://mcp.example.com", - "https://mcp.example.com/" + assert!(AuthorizationManager::is_resource_identifier_valid( + &Url::parse("https://mcp.example.com/mcp").unwrap(), + &Url::parse("https://mcp.example.com").unwrap() + )); + assert!(AuthorizationManager::is_resource_identifier_valid( + &Url::parse("https://mcp.example.com/mcp/tools").unwrap(), + &Url::parse("https://mcp.example.com/mcp").unwrap() )); - assert!(!AuthorizationManager::resource_identifiers_match( - "https://mcp.example.com/mcp", - "https://mcp.example.com/mcp/" + assert!(!AuthorizationManager::is_resource_identifier_valid( + &Url::parse("https://mcp.example.com/mcp").unwrap(), + &Url::parse("https://mcp.example.com/mcp/").unwrap() )); - assert!(!AuthorizationManager::resource_identifiers_match( - "https://mcp.example.com/mcp", - "https://real.example.com/mcp" + assert!(!AuthorizationManager::is_resource_identifier_valid( + &Url::parse("https://mcp.example.com/mcp").unwrap(), + &Url::parse("https://mcp.example.com/mcp-tools").unwrap() + )); + assert!(!AuthorizationManager::is_resource_identifier_valid( + &Url::parse("https://mcp.example.com/mcp").unwrap(), + &Url::parse("https://mcp.example.com/mcp/tools").unwrap() + )); + assert!(!AuthorizationManager::is_resource_identifier_valid( + &Url::parse("https://mcp.example.com/mcp").unwrap(), + &Url::parse("https://real.example.com/mcp").unwrap() )); } From 02ff4e2f2e2d614fc384419ee8c2be4343bad16c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20Kone=C4=8Dn=C3=BD?= Date: Tue, 14 Jul 2026 09:43:22 +0200 Subject: [PATCH 3/4] feat(auth): preserve efficient resource path parsing and comparison --- crates/rmcp/src/transport/auth.rs | 51 ++++++++++++------------------ crates/rmcp/tests/test_sampling.rs | 2 +- 2 files changed, 21 insertions(+), 32 deletions(-) diff --git a/crates/rmcp/src/transport/auth.rs b/crates/rmcp/src/transport/auth.rs index a3801c28..aa393d26 100644 --- a/crates/rmcp/src/transport/auth.rs +++ b/crates/rmcp/src/transport/auth.rs @@ -2001,40 +2001,29 @@ impl AuthorizationManager { Ok(()) } - fn is_resource_identifier_valid(provided: &Url, reference: &Url) -> bool { - if provided == reference { + fn is_resource_identifier_valid(expected: &Url, actual: &Url) -> bool { + if expected == actual { return true; } - if provided.scheme() != reference.scheme() - || provided.host_str() != reference.host_str() - || provided.port_or_known_default() != reference.port_or_known_default() + if expected.scheme() != actual.scheme() + || expected.host_str() != actual.host_str() + || expected.port_or_known_default() != actual.port_or_known_default() { return false; } - let provided_path = provided.path(); - let reference_path = reference.path(); + let expected_path = expected.path(); + let actual_path = actual.path(); // URL query part supported, even if it is discouraged in RFC 8707 - if provided_path == reference_path && provided.query() == reference.query() { + if expected_path == actual_path && expected.query() == actual.query() { return true; } - provided_path.starts_with(reference_path) - && provided.query().is_none() - && reference.query().is_none() - } - - fn resource_identifiers_match(expected: &str, actual: &str) -> bool { - let Ok(expected) = Url::parse(expected) else { - return false; - }; - let Ok(actual) = Url::parse(actual) else { - return false; - }; - - Self::is_resource_identifier_valid(&expected, &actual) + expected_path.starts_with(actual_path) + && expected.query().is_none() + && actual.query().is_none() } async fn discover_resource_metadata_url(&self) -> Result, AuthError> { @@ -3891,9 +3880,9 @@ mod tests { &Url::parse("https://mcp.example.com/mcp/tools").unwrap(), &Url::parse("https://mcp.example.com/mcp").unwrap() )); - assert!(AuthorizationManager::resource_identifiers_match( - "https://mcp.example.com/mcp", - "https://mcp.example.com" + assert!(AuthorizationManager::is_resource_identifier_valid( + &Url::parse("https://mcp.example.com/mcp?query=param").unwrap(), + &Url::parse("https://mcp.example.com/mcp?query=param").unwrap() )); assert!(!AuthorizationManager::is_resource_identifier_valid( @@ -3912,13 +3901,13 @@ mod tests { &Url::parse("https://mcp.example.com/mcp").unwrap(), &Url::parse("https://real.example.com/mcp").unwrap() )); - assert!(!AuthorizationManager::resource_identifiers_match( - "https://mcp.example.com/mcp", - "https://real.example.com" + assert!(!AuthorizationManager::is_resource_identifier_valid( + &Url::parse("https://mcp.example.com/mcp").unwrap(), + &Url::parse("https://mcp.example.com/mcp?query=value1").unwrap() )); - assert!(!AuthorizationManager::resource_identifiers_match( - "https://mcp.example.com/mcp", - "https://mcp.example.com?resource=mcp" + assert!(!AuthorizationManager::is_resource_identifier_valid( + &Url::parse("https://mcp.example.com/mcp?query=value1").unwrap(), + &Url::parse("https://mcp.example.com/mcp?query=value2").unwrap() )); } diff --git a/crates/rmcp/tests/test_sampling.rs b/crates/rmcp/tests/test_sampling.rs index b108ff41..27a4e556 100644 --- a/crates/rmcp/tests/test_sampling.rs +++ b/crates/rmcp/tests/test_sampling.rs @@ -375,7 +375,7 @@ fn test_tool_result_content_requires_content() { #[case::array(serde_json::json!([{ "city": "SF", "temp": 72 }, { "city": "NY", "temp": 65 }]))] #[case::string(serde_json::json!("sunny"))] #[case::integer(serde_json::json!(42))] -#[case::float(serde_json::json!(3.14))] +#[case::float(serde_json::json!(2.5))] #[case::boolean(serde_json::json!(true))] fn tool_result_content_round_trips_non_object_structured_content( #[case] structured: serde_json::Value, From 0ad8259c232aa368ae1e02c88bea8780a24a1cef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20Kone=C4=8Dn=C3=BD?= Date: Fri, 17 Jul 2026 12:33:15 +0200 Subject: [PATCH 4/4] Revert back test value for test tool_result_content_round_trips_non_object_structured_content --- crates/rmcp/tests/test_sampling.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rmcp/tests/test_sampling.rs b/crates/rmcp/tests/test_sampling.rs index 27a4e556..b108ff41 100644 --- a/crates/rmcp/tests/test_sampling.rs +++ b/crates/rmcp/tests/test_sampling.rs @@ -375,7 +375,7 @@ fn test_tool_result_content_requires_content() { #[case::array(serde_json::json!([{ "city": "SF", "temp": 72 }, { "city": "NY", "temp": 65 }]))] #[case::string(serde_json::json!("sunny"))] #[case::integer(serde_json::json!(42))] -#[case::float(serde_json::json!(2.5))] +#[case::float(serde_json::json!(3.14))] #[case::boolean(serde_json::json!(true))] fn tool_result_content_round_trips_non_object_structured_content( #[case] structured: serde_json::Value,