fix(auth): ignore query parameters when matching resources (#1177)
This commit is contained in:
parent
f713ebd1a6
commit
248cbca688
1 changed files with 47 additions and 7 deletions
|
|
@ -2559,14 +2559,12 @@ impl AuthorizationManager {
|
||||||
let expected_path = expected.path();
|
let expected_path = expected.path();
|
||||||
let actual_path = actual.path();
|
let actual_path = actual.path();
|
||||||
|
|
||||||
// URL query part supported, even if it is discouraged in RFC 8707
|
// Query parameters may carry transport hints rather than resource identity.
|
||||||
if expected_path == actual_path && expected.query() == actual.query() {
|
if expected_path == actual_path {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
expected_path.starts_with(actual_path)
|
expected_path.starts_with(actual_path)
|
||||||
&& expected.query().is_none()
|
|
||||||
&& actual.query().is_none()
|
|
||||||
&& (actual_path.ends_with('/')
|
&& (actual_path.ends_with('/')
|
||||||
|| expected_path.as_bytes().get(actual_path.len()) == Some(&b'/'))
|
|| expected_path.as_bytes().get(actual_path.len()) == Some(&b'/'))
|
||||||
}
|
}
|
||||||
|
|
@ -4913,6 +4911,32 @@ mod tests {
|
||||||
assert!(matches!(state, super::OAuthState::Session(_)));
|
assert!(matches!(state, super::OAuthState::Session(_)));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn preregistered_client_uses_metadata_resource_when_server_url_has_query() {
|
||||||
|
let client = RecordingOAuthHttpClient::with_responses(preregistered_discovery_responses());
|
||||||
|
let mut state = super::OAuthState::new_with_oauth_http_client(
|
||||||
|
"https://mcp.example.com/mcp?oauth=initialize",
|
||||||
|
Arc::new(client.clone()),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let request = AuthorizationRequest::new("http://localhost:8080/callback")
|
||||||
|
.with_preregistered_client("preregistered-client");
|
||||||
|
state.start_authorization(request).await.unwrap();
|
||||||
|
|
||||||
|
let auth_url = state.get_authorization_url().await.unwrap();
|
||||||
|
let query = auth_url_query(&auth_url);
|
||||||
|
assert_eq!(
|
||||||
|
query.get("resource").map(String::as_str),
|
||||||
|
Some("https://mcp.example.com/mcp")
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
client.requests()[0].uri,
|
||||||
|
"https://mcp.example.com/mcp?oauth=initialize"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn authorization_session_selects_default_scopes_when_none_provided() {
|
async fn authorization_session_selects_default_scopes_when_none_provided() {
|
||||||
let client = RecordingOAuthHttpClient::with_responses(preregistered_discovery_responses());
|
let client = RecordingOAuthHttpClient::with_responses(preregistered_discovery_responses());
|
||||||
|
|
@ -5549,6 +5573,22 @@ mod tests {
|
||||||
&Url::parse("https://mcp.example.com/mcp?query=param").unwrap(),
|
&Url::parse("https://mcp.example.com/mcp?query=param").unwrap(),
|
||||||
&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(
|
||||||
|
&Url::parse("https://mcp.example.com/mcp?oauth=initialize").unwrap(),
|
||||||
|
&Url::parse("https://mcp.example.com/mcp").unwrap()
|
||||||
|
));
|
||||||
|
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::is_resource_identifier_valid(
|
||||||
|
&Url::parse("https://mcp.example.com/mcp?query=value1").unwrap(),
|
||||||
|
&Url::parse("https://mcp.example.com/mcp?query=value2").unwrap()
|
||||||
|
));
|
||||||
|
assert!(AuthorizationManager::is_resource_identifier_valid(
|
||||||
|
&Url::parse("https://mcp.example.com/mcp/tools?oauth=initialize").unwrap(),
|
||||||
|
&Url::parse("https://mcp.example.com/mcp?query=value1").unwrap()
|
||||||
|
));
|
||||||
|
|
||||||
assert!(!AuthorizationManager::is_resource_identifier_valid(
|
assert!(!AuthorizationManager::is_resource_identifier_valid(
|
||||||
&Url::parse("https://mcp.example.com/mcp").unwrap(),
|
&Url::parse("https://mcp.example.com/mcp").unwrap(),
|
||||||
|
|
@ -5571,12 +5611,12 @@ mod tests {
|
||||||
&Url::parse("https://real.example.com/mcp").unwrap()
|
&Url::parse("https://real.example.com/mcp").unwrap()
|
||||||
));
|
));
|
||||||
assert!(!AuthorizationManager::is_resource_identifier_valid(
|
assert!(!AuthorizationManager::is_resource_identifier_valid(
|
||||||
&Url::parse("https://mcp.example.com/mcp").unwrap(),
|
&Url::parse("https://mcp.example.com/mcp-tools?oauth=initialize").unwrap(),
|
||||||
&Url::parse("https://mcp.example.com/mcp?query=value1").unwrap()
|
&Url::parse("https://mcp.example.com/mcp?query=value1").unwrap()
|
||||||
));
|
));
|
||||||
assert!(!AuthorizationManager::is_resource_identifier_valid(
|
assert!(!AuthorizationManager::is_resource_identifier_valid(
|
||||||
&Url::parse("https://mcp.example.com/mcp?query=value1").unwrap(),
|
&Url::parse("https://mcp.example.com/mcp?oauth=initialize").unwrap(),
|
||||||
&Url::parse("https://mcp.example.com/mcp?query=value2").unwrap()
|
&Url::parse("https://real.example.com/mcp?oauth=initialize").unwrap()
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue