Skip to content

Commit 5e5a2fe

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 5e5a2fe

2 files changed

Lines changed: 189 additions & 7 deletions

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()

crates/rmcp/tests/test_client_credentials.rs

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,29 @@ async fn resource_metadata_handler(req: Request<Body>) -> Result<Response<Body>,
3737
async fn auth_server_metadata_handler(req: Request<Body>) -> Result<Response<Body>, Infallible> {
3838
let host = req.headers().get("host").unwrap().to_str().unwrap();
3939
let base_url = format!("http://{}", host);
40-
Ok(json_response(serde_json::json!({
41-
"issuer": base_url,
42-
"authorization_endpoint": format!("{}/authorize", base_url),
43-
"token_endpoint": format!("{}/token", base_url),
40+
Ok(auth_server_metadata_response(&base_url, &base_url))
41+
}
42+
43+
async fn path_inserted_auth_server_metadata_handler(
44+
req: Request<Body>,
45+
) -> Result<Response<Body>, Infallible> {
46+
let host = req.headers().get("host").unwrap().to_str().unwrap();
47+
let base_url = format!("http://{}", host);
48+
Ok(auth_server_metadata_response(
49+
&format!("{}/mcp", base_url),
50+
&base_url,
51+
))
52+
}
53+
54+
fn auth_server_metadata_response(issuer: &str, endpoint_base_url: &str) -> Response<Body> {
55+
json_response(serde_json::json!({
56+
"issuer": issuer,
57+
"authorization_endpoint": format!("{}/authorize", endpoint_base_url),
58+
"token_endpoint": format!("{}/token", endpoint_base_url),
4459
"token_endpoint_auth_methods_supported": ["client_secret_post", "client_secret_basic"],
4560
"grant_types_supported": ["client_credentials"],
4661
"scopes_supported": ["read", "write"]
47-
})))
62+
}))
4863
}
4964

5065
async fn token_handler(req: Request<Body>) -> Result<Response<Body>, Infallible> {
@@ -144,7 +159,7 @@ async fn start_path_insert_metadata_server() -> (String, SocketAddr) {
144159
let app = Router::new()
145160
.route(
146161
"/.well-known/oauth-authorization-server/mcp",
147-
get(auth_server_metadata_handler),
162+
get(path_inserted_auth_server_metadata_handler),
148163
)
149164
.route("/token", post(token_handler));
150165

0 commit comments

Comments
 (0)