Skip to content

RFC 9728 resource is used instead of base url when possible #962

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
fkonecny wants to merge 2 commits into
base: main
Choose a base branch
from
+146 −28
Open

RFC 9728 resource is used instead of base url when possible #962

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50ccb1f
feat(transport): Resource from OAuth2 resource metadata document is p…
filip-konecny-tfs Jul 7, 2026
1e453b1
feat(transport): Write discovered_resource after successful oauth res…
filip-konecny-tfs Jul 8, 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
174 changes: 146 additions & 28 deletions crates/rmcp/src/transport/auth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,8 @@ pub struct AuthorizationManager {
www_auth_scopes: RwLock<Vec<String>>,
/// scopes_supported from protected resource metadata (RFC 9728)
resource_scopes: RwLock<Vec<String>>,
/// resource indicator from protected resource metadata, used for RFC 8707 `resource`
discovered_resource: RwLock<Option<String>>,
/// OIDC Dynamic Client Registration `application_type` (SEP-837)
application_type: Option<String>,
}
Expand Down Expand Up @@ -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()),
};

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1351,6 +1354,14 @@ impl AuthorizationManager {
Ok(auth_url.to_string())
}

async fn oauth_resource(&self) -> String {
self.discovered_resource
.read()
Comment on lines +1358 to +1359

@DaleSeo DaleSeo Jul 7, 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.

Shouldn't we store this before reading it?

@fkonecny fkonecny Jul 8, 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.

I am sorry (focused so much on PR text that I muddled stash pop).

I have patched it in the next commit.

I have kept your strict check about path equality.

.await
.clone()
.unwrap_or_else(|| self.base_url.to_string())
}

/// get the current granted scopes
pub async fn get_current_scopes(&self) -> Vec<String> {
self.current_scopes.read().await.clone()
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1858,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() {
Expand Down Expand Up @@ -1921,27 +1937,51 @@ 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
)));
}

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;
}

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;
}

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())
provided_path.starts_with(reference_path)
&& provided.query().is_none()
&& reference.query().is_none()
}

async fn discover_resource_metadata_url(&self) -> Result<Option<Url>, AuthError> {
Expand Down Expand Up @@ -3242,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"]
}),
),
Expand All @@ -3264,6 +3304,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://mcp.example.com")
);
assert_eq!(
client.requests(),
vec![
Expand Down Expand Up @@ -3484,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()
));
}

Expand Down Expand Up @@ -4421,6 +4481,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(
Expand Down
Loading