Skip to content

Commit b429fc1

Browse files
RFC 9728 resource is used instead of base url when possible (#962)
* feat(transport)!: use declared OAuth resource * fix(transport): validate OAuth resource paths --------- Co-authored-by: Filip Konečný <filip.konecny@thermofisher.com>
1 parent 82a6c48 commit b429fc1

1 file changed

Lines changed: 159 additions & 51 deletions

File tree

crates/rmcp/src/transport/auth.rs

Lines changed: 159 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,6 +1029,8 @@ pub struct AuthorizationManager {
10291029
www_auth_scopes: RwLock<Vec<String>>,
10301030
/// scopes_supported from protected resource metadata (RFC 9728)
10311031
resource_scopes: RwLock<Vec<String>>,
1032+
/// resource indicator from protected resource metadata, used for RFC 8707 `resource`
1033+
discovered_resource: RwLock<Option<String>>,
10321034
/// OIDC Dynamic Client Registration `application_type` (SEP-837)
10331035
application_type: Option<String>,
10341036
allow_missing_issuer: bool,
@@ -1276,6 +1278,7 @@ impl AuthorizationManager {
12761278
scope_upgrade_config: ScopeUpgradeConfig::default(),
12771279
www_auth_scopes: RwLock::new(Vec::new()),
12781280
resource_scopes: RwLock::new(Vec::new()),
1281+
discovered_resource: RwLock::new(None),
12791282
application_type: Some(DEFAULT_APPLICATION_TYPE.to_string()),
12801283
allow_missing_issuer: false,
12811284
};
@@ -1745,7 +1748,7 @@ impl AuthorizationManager {
17451748
let mut auth_request = oauth_client
17461749
.authorize_url(CsrfToken::new_random)
17471750
.set_pkce_challenge(pkce_challenge)
1748-
.add_extra_param("resource", self.base_url.to_string());
1751+
.add_extra_param("resource", self.oauth_resource().await);
17491752

17501753
// add request scopes
17511754
for scope in scopes {
@@ -1783,6 +1786,14 @@ impl AuthorizationManager {
17831786
Ok(auth_url.to_string())
17841787
}
17851788

1789+
async fn oauth_resource(&self) -> String {
1790+
self.discovered_resource
1791+
.read()
1792+
.await
1793+
.clone()
1794+
.unwrap_or_else(|| self.base_url.to_string())
1795+
}
1796+
17861797
/// get the current granted scopes
17871798
pub async fn get_current_scopes(&self) -> Vec<String> {
17881799
self.current_scopes.read().await.clone()
@@ -2024,7 +2035,7 @@ impl AuthorizationManager {
20242035
let token_result = match oauth_client
20252036
.exchange_code(AuthorizationCode::new(code.to_string()))
20262037
.set_pkce_verifier(pkce_verifier)
2027-
.add_extra_param("resource", self.base_url.to_string())
2038+
.add_extra_param("resource", self.oauth_resource().await)
20282039
.request_async(&OAuth2HttpClient {
20292040
client: self.http_client.as_ref(),
20302041
redirect_policy: OAuthHttpRedirectPolicy::Stop,
@@ -2422,6 +2433,11 @@ impl AuthorizationManager {
24222433

24232434
self.validate_resource_metadata_resource(&resource_metadata)?;
24242435

2436+
self.discovered_resource
2437+
.write()
2438+
.await
2439+
.replace(resource_metadata.resource.clone().unwrap_or_default());
2440+
24252441
// store scopes_supported from protected resource metadata for select_scopes()
24262442
if let Some(scopes) = resource_metadata.scopes_supported
24272443
&& !scopes.is_empty()
@@ -2485,43 +2501,53 @@ impl AuthorizationManager {
24852501
));
24862502
};
24872503

2488-
if !Self::resource_identifiers_match(self.base_url.as_str(), resource) {
2504+
let Ok(resource_url) = Url::parse(resource) else {
2505+
return Err(AuthError::MetadataError(
2506+
"Protected resource metadata resource field is not a valid URL".to_string(),
2507+
));
2508+
};
2509+
2510+
if resource_url.fragment().is_some() {
2511+
return Err(AuthError::MetadataError(
2512+
"Protected resource metadata resource does not permit fragment in URL as specified by RFC 8707".to_string()
2513+
));
2514+
}
2515+
2516+
if !Self::is_resource_identifier_valid(&self.base_url, &resource_url) {
24892517
return Err(AuthError::MetadataError(format!(
2490-
"Protected resource metadata resource mismatch: expected '{}', got '{}'",
2518+
"Protected resource metadata resource mismatch: reference '{}', permitted '{}'",
24912519
self.base_url, resource
24922520
)));
24932521
}
24942522

24952523
Ok(())
24962524
}
24972525

2498-
fn resource_identifiers_match(expected: &str, actual: &str) -> bool {
2499-
expected == actual
2500-
|| (Self::is_root_resource_identifier(expected)
2501-
&& actual == expected.trim_end_matches('/'))
2502-
|| (Self::is_root_resource_identifier(actual)
2503-
&& expected == actual.trim_end_matches('/'))
2504-
|| Self::root_resource_identifier_covers_path(actual, expected)
2505-
}
2506-
2507-
fn is_root_resource_identifier(value: &str) -> bool {
2508-
Url::parse(value)
2509-
.is_ok_and(|url| url.path() == "/" && url.query().is_none() && url.fragment().is_none())
2510-
}
2526+
fn is_resource_identifier_valid(expected: &Url, actual: &Url) -> bool {
2527+
if expected == actual {
2528+
return true;
2529+
}
25112530

2512-
fn root_resource_identifier_covers_path(root_resource: &str, path_resource: &str) -> bool {
2513-
let Ok(root_resource) = Url::parse(root_resource) else {
2514-
return false;
2515-
};
2516-
let Ok(path_resource) = Url::parse(path_resource) else {
2531+
if expected.scheme() != actual.scheme()
2532+
|| expected.host_str() != actual.host_str()
2533+
|| expected.port_or_known_default() != actual.port_or_known_default()
2534+
{
25172535
return false;
2518-
};
2536+
}
2537+
2538+
let expected_path = expected.path();
2539+
let actual_path = actual.path();
2540+
2541+
// URL query part supported, even if it is discouraged in RFC 8707
2542+
if expected_path == actual_path && expected.query() == actual.query() {
2543+
return true;
2544+
}
25192545

2520-
root_resource.path() == "/"
2521-
&& root_resource.query().is_none()
2522-
&& root_resource.fragment().is_none()
2523-
&& path_resource.path() != "/"
2524-
&& Self::is_same_origin(&root_resource, &path_resource)
2546+
expected_path.starts_with(actual_path)
2547+
&& expected.query().is_none()
2548+
&& actual.query().is_none()
2549+
&& (actual_path.ends_with('/')
2550+
|| expected_path.as_bytes().get(actual_path.len()) == Some(&b'/'))
25252551
}
25262552

25272553
async fn discover_resource_metadata_url(&self) -> Result<Option<Url>, AuthError> {
@@ -4075,7 +4101,7 @@ mod tests {
40754101
http_response(
40764102
200,
40774103
serde_json::json!({
4078-
"resource": "https://mcp.example.com/mcp",
4104+
"resource": "https://mcp.example.com",
40794105
"authorization_servers": ["https://auth.example.com"]
40804106
}),
40814107
),
@@ -4098,6 +4124,10 @@ mod tests {
40984124
let metadata = manager.resolve_metadata().await.unwrap().metadata;
40994125

41004126
assert_eq!(metadata.token_endpoint, "https://auth.example.com/token");
4127+
assert_eq!(
4128+
manager.discovered_resource.read().await.as_deref(),
4129+
Some("https://mcp.example.com")
4130+
);
41014131
assert_eq!(
41024132
client.requests(),
41034133
vec![
@@ -5325,35 +5355,55 @@ mod tests {
53255355
}
53265356

53275357
#[test]
5328-
fn resource_identifier_matching_allows_only_root_trailing_slash_difference() {
5329-
assert!(AuthorizationManager::resource_identifiers_match(
5330-
"https://mcp.example.com/",
5331-
"https://mcp.example.com"
5358+
fn resource_identifier_matching_allows_matching_host_or_parent_path() {
5359+
assert!(AuthorizationManager::is_resource_identifier_valid(
5360+
&Url::parse("https://mcp.example.com/").unwrap(),
5361+
&Url::parse("https://mcp.example.com").unwrap()
53325362
));
5333-
assert!(AuthorizationManager::resource_identifiers_match(
5334-
"https://mcp.example.com",
5335-
"https://mcp.example.com/"
5363+
assert!(AuthorizationManager::is_resource_identifier_valid(
5364+
&Url::parse("https://mcp.example.com").unwrap(),
5365+
&Url::parse("https://mcp.example.com/").unwrap()
53365366
));
5337-
assert!(AuthorizationManager::resource_identifiers_match(
5338-
"https://mcp.example.com/mcp",
5339-
"https://mcp.example.com"
5367+
assert!(AuthorizationManager::is_resource_identifier_valid(
5368+
&Url::parse("https://mcp.example.com/mcp").unwrap(),
5369+
&Url::parse("https://mcp.example.com").unwrap()
5370+
));
5371+
assert!(AuthorizationManager::is_resource_identifier_valid(
5372+
&Url::parse("https://mcp.example.com/mcp/tools").unwrap(),
5373+
&Url::parse("https://mcp.example.com/mcp").unwrap()
5374+
));
5375+
assert!(AuthorizationManager::is_resource_identifier_valid(
5376+
&Url::parse("https://mcp.example.com/mcp?query=param").unwrap(),
5377+
&Url::parse("https://mcp.example.com/mcp?query=param").unwrap()
53405378
));
53415379

5342-
assert!(!AuthorizationManager::resource_identifiers_match(
5343-
"https://mcp.example.com/mcp",
5344-
"https://mcp.example.com/mcp/"
5380+
assert!(!AuthorizationManager::is_resource_identifier_valid(
5381+
&Url::parse("https://mcp.example.com/mcp").unwrap(),
5382+
&Url::parse("https://mcp.example.com/mcp/").unwrap()
53455383
));
5346-
assert!(!AuthorizationManager::resource_identifiers_match(
5347-
"https://mcp.example.com/mcp",
5348-
"https://real.example.com/mcp"
5384+
assert!(!AuthorizationManager::is_resource_identifier_valid(
5385+
&Url::parse("https://mcp.example.com/mcp-tools").unwrap(),
5386+
&Url::parse("https://mcp.example.com/mcp").unwrap()
53495387
));
5350-
assert!(!AuthorizationManager::resource_identifiers_match(
5351-
"https://mcp.example.com/mcp",
5352-
"https://real.example.com"
5388+
assert!(!AuthorizationManager::is_resource_identifier_valid(
5389+
&Url::parse("https://mcp.example.com/mcp").unwrap(),
5390+
&Url::parse("https://mcp.example.com/mcp-tools").unwrap()
53535391
));
5354-
assert!(!AuthorizationManager::resource_identifiers_match(
5355-
"https://mcp.example.com/mcp",
5356-
"https://mcp.example.com?resource=mcp"
5392+
assert!(!AuthorizationManager::is_resource_identifier_valid(
5393+
&Url::parse("https://mcp.example.com/mcp").unwrap(),
5394+
&Url::parse("https://mcp.example.com/mcp/tools").unwrap()
5395+
));
5396+
assert!(!AuthorizationManager::is_resource_identifier_valid(
5397+
&Url::parse("https://mcp.example.com/mcp").unwrap(),
5398+
&Url::parse("https://real.example.com/mcp").unwrap()
5399+
));
5400+
assert!(!AuthorizationManager::is_resource_identifier_valid(
5401+
&Url::parse("https://mcp.example.com/mcp").unwrap(),
5402+
&Url::parse("https://mcp.example.com/mcp?query=value1").unwrap()
5403+
));
5404+
assert!(!AuthorizationManager::is_resource_identifier_valid(
5405+
&Url::parse("https://mcp.example.com/mcp?query=value1").unwrap(),
5406+
&Url::parse("https://mcp.example.com/mcp?query=value2").unwrap()
53575407
));
53585408
}
53595409

@@ -6313,6 +6363,64 @@ mod tests {
63136363
assert!(scope.contains("write"));
63146364
}
63156365

6366+
#[tokio::test]
6367+
async fn authorization_url_uses_discovered_resource() {
6368+
let base_url = "https://mcp.example.com/mcp";
6369+
let auth_endpoint = "https://auth.example.com/authorize";
6370+
let mut manager = AuthorizationManager::new(base_url).await.unwrap();
6371+
6372+
let metadata = AuthorizationMetadata {
6373+
authorization_endpoint: auth_endpoint.to_string(),
6374+
token_endpoint: "https://auth.example.com/token".to_string(),
6375+
registration_endpoint: None,
6376+
issuer: None,
6377+
jwks_uri: None,
6378+
scopes_supported: None,
6379+
response_types_supported: Some(vec!["code".to_string()]),
6380+
code_challenge_methods_supported: Some(vec!["S256".to_string()]),
6381+
additional_fields: std::collections::HashMap::new(),
6382+
};
6383+
manager.set_metadata(metadata);
6384+
manager.configure_client_id("test-client-id").unwrap();
6385+
*manager.discovered_resource.write().await = Some("https://mcp.example.com".to_string());
6386+
6387+
let auth_url = manager.get_authorization_url(&["read"]).await.unwrap();
6388+
let parsed = Url::parse(&auth_url).unwrap();
6389+
let params: std::collections::HashMap<_, _> = parsed.query_pairs().collect();
6390+
6391+
assert_eq!(
6392+
params.get("resource").map(|v| v.as_ref()),
6393+
Some("https://mcp.example.com")
6394+
);
6395+
}
6396+
6397+
#[tokio::test]
6398+
async fn authorization_url_uses_default_resource_without_protected_resource_document() {
6399+
let base_url = "https://mcp.example.com/mcp";
6400+
let auth_endpoint = "https://auth.example.com/authorize";
6401+
let mut manager = AuthorizationManager::new(base_url).await.unwrap();
6402+
6403+
let metadata = AuthorizationMetadata {
6404+
authorization_endpoint: auth_endpoint.to_string(),
6405+
token_endpoint: "https://auth.example.com/token".to_string(),
6406+
registration_endpoint: None,
6407+
issuer: None,
6408+
jwks_uri: None,
6409+
scopes_supported: None,
6410+
response_types_supported: Some(vec!["code".to_string()]),
6411+
code_challenge_methods_supported: Some(vec!["S256".to_string()]),
6412+
additional_fields: std::collections::HashMap::new(),
6413+
};
6414+
manager.set_metadata(metadata);
6415+
manager.configure_client_id("test-client-id").unwrap();
6416+
6417+
let auth_url = manager.get_authorization_url(&["read"]).await.unwrap();
6418+
let parsed = Url::parse(&auth_url).unwrap();
6419+
let params: std::collections::HashMap<_, _> = parsed.query_pairs().collect();
6420+
6421+
assert_eq!(params.get("resource").map(|v| v.as_ref()), Some(base_url));
6422+
}
6423+
63166424
#[test]
63176425
fn authorization_callback_parses_optional_issuer() {
63186426
let callback = AuthorizationCallback::from_redirect_url(

0 commit comments

Comments
 (0)