Skip to content

Commit 18fba5a

Browse files
committed
fix(auth): validate discovered metadata issuer
Validate authorization server metadata issuers against the issuer implied by standard discovery URLs before trusting advertised endpoints. Reject mismatches while preserving compatibility for metadata that omits issuer. fixes #983
1 parent 98bb263 commit 18fba5a

1 file changed

Lines changed: 168 additions & 1 deletion

File tree

crates/rmcp/src/transport/auth.rs

Lines changed: 168 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1876,14 +1876,86 @@ impl AuthorizationManager {
18761876
}
18771877

18781878
match serde_json::from_slice::<AuthorizationMetadata>(response.body()) {
1879-
Ok(metadata) => Ok(Some(metadata)),
1879+
Ok(metadata) => {
1880+
Self::validate_authorization_metadata_issuer(discovery_url, &metadata)?;
1881+
Ok(Some(metadata))
1882+
}
18801883
Err(err) => {
18811884
debug!("Failed to parse metadata for {}: {}", discovery_url, err);
18821885
Ok(None) // malformed JSON ⇒ try next candidate
18831886
}
18841887
}
18851888
}
18861889

1890+
fn expected_issuer_for_authorization_metadata_url(discovery_url: &Url) -> Option<String> {
1891+
let path = discovery_url.path();
1892+
let oauth_prefix = "/.well-known/oauth-authorization-server";
1893+
let oidc_prefix = "/.well-known/openid-configuration";
1894+
1895+
let issuer_path = if path == oauth_prefix || path == oidc_prefix {
1896+
""
1897+
} else if let Some(suffix) = path.strip_prefix(&format!("{oauth_prefix}/")) {
1898+
// RFC 8414 path-insertion form
1899+
suffix
1900+
} else if let Some(prefix) = path.strip_suffix(oidc_prefix) {
1901+
// OpenID Connect path-appended form
1902+
prefix.trim_start_matches('/')
1903+
} else {
1904+
return None;
1905+
};
1906+
1907+
let mut issuer = discovery_url.clone();
1908+
issuer.set_query(None);
1909+
issuer.set_fragment(None);
1910+
if issuer_path.is_empty() {
1911+
issuer.set_path("");
1912+
} else {
1913+
issuer.set_path(&format!("/{issuer_path}"));
1914+
}
1915+
Some(issuer.to_string())
1916+
}
1917+
1918+
fn issuer_identifiers_match(received_issuer: &str, expected_issuer: &str) -> bool {
1919+
if received_issuer == expected_issuer {
1920+
return true;
1921+
}
1922+
1923+
let trim_root_slash = |issuer: &str| -> String {
1924+
issuer
1925+
.strip_suffix('/')
1926+
.filter(|without_slash| {
1927+
Url::parse(without_slash)
1928+
.map(|url| url.path().is_empty() || url.path() == "/")
1929+
.unwrap_or(false)
1930+
})
1931+
.unwrap_or(issuer)
1932+
.to_string()
1933+
};
1934+
1935+
trim_root_slash(received_issuer) == trim_root_slash(expected_issuer)
1936+
}
1937+
1938+
fn validate_authorization_metadata_issuer(
1939+
discovery_url: &Url,
1940+
metadata: &AuthorizationMetadata,
1941+
) -> Result<(), AuthError> {
1942+
let Some(received_issuer) = metadata.issuer.as_deref() else {
1943+
return Ok(());
1944+
};
1945+
let Some(expected_issuer) =
1946+
Self::expected_issuer_for_authorization_metadata_url(discovery_url)
1947+
else {
1948+
return Ok(());
1949+
};
1950+
if !Self::issuer_identifiers_match(received_issuer, &expected_issuer) {
1951+
return Err(AuthError::AuthorizationServerMismatch {
1952+
expected_issuer,
1953+
received_issuer: received_issuer.to_string(),
1954+
});
1955+
}
1956+
Ok(())
1957+
}
1958+
18871959
async fn discover_oauth_server_via_resource_metadata(
18881960
&self,
18891961
) -> Result<Option<AuthorizationMetadata>, AuthError> {
@@ -3467,6 +3539,101 @@ mod tests {
34673539
);
34683540
}
34693541

3542+
#[tokio::test]
3543+
async fn authorization_metadata_rejects_mismatched_issuer() {
3544+
let client = RecordingOAuthHttpClient::with_responses(vec![
3545+
empty_response(401),
3546+
http_response(
3547+
200,
3548+
serde_json::json!({
3549+
"resource": "https://mcp.example.com/",
3550+
"authorization_servers": ["https://auth.example.com/tenant1"]
3551+
}),
3552+
),
3553+
http_response(
3554+
200,
3555+
serde_json::json!({
3556+
"resource": "https://mcp.example.com/",
3557+
"authorization_servers": ["https://auth.example.com/tenant1"]
3558+
}),
3559+
),
3560+
http_response(
3561+
200,
3562+
serde_json::json!({
3563+
"issuer": "https://evil.example.com/tenant1",
3564+
"authorization_endpoint": "https://evil.example.com/tenant1/authorize",
3565+
"token_endpoint": "https://evil.example.com/tenant1/token"
3566+
}),
3567+
),
3568+
]);
3569+
let manager = AuthorizationManager::new_with_oauth_http_client(
3570+
"https://mcp.example.com/",
3571+
Arc::new(client),
3572+
)
3573+
.await
3574+
.unwrap();
3575+
3576+
let error = manager.discover_metadata().await.unwrap_err();
3577+
3578+
assert!(
3579+
matches!(
3580+
error,
3581+
AuthError::AuthorizationServerMismatch {
3582+
ref expected_issuer,
3583+
ref received_issuer
3584+
} if expected_issuer == "https://auth.example.com/tenant1"
3585+
&& received_issuer == "https://evil.example.com/tenant1"
3586+
),
3587+
"expected authorization server issuer mismatch, got: {error:?}"
3588+
);
3589+
}
3590+
3591+
#[tokio::test]
3592+
async fn authorization_metadata_accepts_oidc_path_appended_issuer() {
3593+
let discovery_url =
3594+
Url::parse("https://auth.example.com/tenant1/.well-known/openid-configuration")
3595+
.unwrap();
3596+
let metadata = AuthorizationMetadata {
3597+
issuer: Some("https://auth.example.com/tenant1".to_string()),
3598+
authorization_endpoint: "https://auth.example.com/tenant1/authorize".to_string(),
3599+
token_endpoint: "https://auth.example.com/tenant1/token".to_string(),
3600+
..Default::default()
3601+
};
3602+
3603+
AuthorizationManager::validate_authorization_metadata_issuer(&discovery_url, &metadata)
3604+
.unwrap();
3605+
}
3606+
3607+
#[tokio::test]
3608+
async fn authorization_metadata_allows_only_root_trailing_slash_equivalence() {
3609+
assert!(AuthorizationManager::issuer_identifiers_match(
3610+
"https://auth.example.com/",
3611+
"https://auth.example.com"
3612+
));
3613+
assert!(!AuthorizationManager::issuer_identifiers_match(
3614+
"https://auth.example.com/tenant1/",
3615+
"https://auth.example.com/tenant1"
3616+
));
3617+
}
3618+
3619+
#[tokio::test]
3620+
async fn authorization_metadata_skips_nonstandard_oidc_path_insertion_validation() {
3621+
let discovery_url =
3622+
Url::parse("https://auth.example.com/.well-known/openid-configuration/tenant1")
3623+
.unwrap();
3624+
let metadata = AuthorizationMetadata {
3625+
issuer: Some(
3626+
"https://auth.example.com/.well-known/openid-configuration/tenant1".to_string(),
3627+
),
3628+
authorization_endpoint: "https://auth.example.com/tenant1/authorize".to_string(),
3629+
token_endpoint: "https://auth.example.com/tenant1/token".to_string(),
3630+
..Default::default()
3631+
};
3632+
3633+
AuthorizationManager::validate_authorization_metadata_issuer(&discovery_url, &metadata)
3634+
.unwrap();
3635+
}
3636+
34703637
#[tokio::test]
34713638
async fn protected_resource_metadata_supports_custom_location_and_oidc_path_append() {
34723639
let challenge = oauth2::http::Response::builder()

0 commit comments

Comments
 (0)