feat: classify authorization-required errors (#1056)
This commit is contained in:
parent
3240b6e782
commit
65f05e9924
5 changed files with 142 additions and 0 deletions
|
|
@ -390,6 +390,11 @@ name = "test_client_credentials"
|
||||||
required-features = ["auth"]
|
required-features = ["auth"]
|
||||||
path = "tests/test_client_credentials.rs"
|
path = "tests/test_client_credentials.rs"
|
||||||
|
|
||||||
|
[[test]]
|
||||||
|
name = "test_auth_error_classification"
|
||||||
|
required-features = ["auth", "client", "transport-streamable-http-client"]
|
||||||
|
path = "tests/test_auth_error_classification.rs"
|
||||||
|
|
||||||
[[test]]
|
[[test]]
|
||||||
name = "test_unix_socket_transport"
|
name = "test_unix_socket_transport"
|
||||||
required-features = [
|
required-features = [
|
||||||
|
|
|
||||||
|
|
@ -111,6 +111,17 @@ impl ClientInitializeError {
|
||||||
}
|
}
|
||||||
None
|
None
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns whether client initialization failed because authorization is required.
|
||||||
|
///
|
||||||
|
/// This covers both missing or expired local OAuth authorization and an HTTP
|
||||||
|
/// authorization challenge from the MCP server.
|
||||||
|
pub fn is_authorization_required(&self) -> bool {
|
||||||
|
matches!(
|
||||||
|
self,
|
||||||
|
Self::TransportError { error, .. } if error.is_authorization_required()
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Helper function to get the next message from the stream
|
/// Helper function to get the next message from the stream
|
||||||
|
|
|
||||||
|
|
@ -272,6 +272,27 @@ impl DynamicTransportError {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(crate) fn is_authorization_required(&self) -> bool {
|
||||||
|
let mut error = Some(self.error.as_ref() as &(dyn std::error::Error + 'static));
|
||||||
|
while let Some(current) = error {
|
||||||
|
#[cfg(feature = "auth")]
|
||||||
|
if matches!(
|
||||||
|
current.downcast_ref::<auth::AuthError>(),
|
||||||
|
Some(auth::AuthError::AuthorizationRequired)
|
||||||
|
) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(feature = "transport-streamable-http-client")]
|
||||||
|
if current.is::<streamable_http_client::AuthRequiredError>() {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
error = current.source();
|
||||||
|
}
|
||||||
|
false
|
||||||
|
}
|
||||||
|
|
||||||
pub fn downcast<T: Transport<R> + 'static, R: ServiceRole>(self) -> Result<T::Error, Self> {
|
pub fn downcast<T: Transport<R> + 'static, R: ServiceRole>(self) -> Result<T::Error, Self> {
|
||||||
if !self.is::<T, R>() {
|
if !self.is::<T, R>() {
|
||||||
Err(self)
|
Err(self)
|
||||||
|
|
|
||||||
85
crates/rmcp/tests/test_auth_error_classification.rs
Normal file
85
crates/rmcp/tests/test_auth_error_classification.rs
Normal file
|
|
@ -0,0 +1,85 @@
|
||||||
|
use std::{any::TypeId, error::Error};
|
||||||
|
|
||||||
|
use rmcp::{
|
||||||
|
service::ClientInitializeError,
|
||||||
|
transport::{
|
||||||
|
AuthError, DynamicTransportError,
|
||||||
|
streamable_http_client::{AuthRequiredError, InsufficientScopeError, StreamableHttpError},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
use thiserror::Error;
|
||||||
|
|
||||||
|
type TestHttpError = StreamableHttpError<std::io::Error>;
|
||||||
|
|
||||||
|
#[derive(Debug, Error)]
|
||||||
|
#[error("outer transport wrapper")]
|
||||||
|
struct OuterError(#[source] TestHttpError);
|
||||||
|
|
||||||
|
fn initialization_error(error: impl Error + Send + Sync + 'static) -> ClientInitializeError {
|
||||||
|
ClientInitializeError::TransportError {
|
||||||
|
error: DynamicTransportError::from_parts(
|
||||||
|
"test transport",
|
||||||
|
TypeId::of::<()>(),
|
||||||
|
Box::new(error),
|
||||||
|
),
|
||||||
|
context: "initialize".into(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn classifies_local_authorization_required() {
|
||||||
|
let error = TestHttpError::Auth(AuthError::AuthorizationRequired);
|
||||||
|
|
||||||
|
assert!(initialization_error(error).is_authorization_required());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn classifies_http_authorization_challenge() {
|
||||||
|
let error =
|
||||||
|
TestHttpError::AuthRequired(AuthRequiredError::new("Bearer realm=\"mcp\"".to_owned()));
|
||||||
|
|
||||||
|
assert!(initialization_error(error).is_authorization_required());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn classifies_authorization_required_through_multiple_sources() {
|
||||||
|
let error = OuterError(TestHttpError::Auth(AuthError::AuthorizationRequired));
|
||||||
|
|
||||||
|
assert!(initialization_error(error).is_authorization_required());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn does_not_classify_unrelated_transport_errors() {
|
||||||
|
let closed = TestHttpError::TransportChannelClosed;
|
||||||
|
let refresh = TestHttpError::Auth(AuthError::TokenRefreshFailed("timeout".to_owned()));
|
||||||
|
let scope = TestHttpError::InsufficientScope(InsufficientScopeError::new(
|
||||||
|
"Bearer error=\"insufficient_scope\"".to_owned(),
|
||||||
|
Some("admin".to_owned()),
|
||||||
|
));
|
||||||
|
|
||||||
|
assert!(!initialization_error(closed).is_authorization_required());
|
||||||
|
assert!(!initialization_error(refresh).is_authorization_required());
|
||||||
|
assert!(!initialization_error(scope).is_authorization_required());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn does_not_classify_non_transport_initialization_errors() {
|
||||||
|
assert!(!ClientInitializeError::Cancelled.is_authorization_required());
|
||||||
|
assert!(
|
||||||
|
!ClientInitializeError::ConnectionClosed("server closed the connection".to_owned())
|
||||||
|
.is_authorization_required()
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn http_challenge_remains_available_as_an_error_source() {
|
||||||
|
let error =
|
||||||
|
TestHttpError::AuthRequired(AuthRequiredError::new("Bearer realm=\"mcp\"".to_owned()));
|
||||||
|
|
||||||
|
let source = error.source().expect("auth challenge should be a source");
|
||||||
|
let challenge = source
|
||||||
|
.downcast_ref::<AuthRequiredError>()
|
||||||
|
.expect("source should retain the challenge type");
|
||||||
|
|
||||||
|
assert_eq!(challenge.www_authenticate_header, "Bearer realm=\"mcp\"");
|
||||||
|
}
|
||||||
|
|
@ -256,6 +256,26 @@ let client_service = ClientInfo::default();
|
||||||
let client = client_service.serve(transport).await?;
|
let client = client_service.serve(transport).await?;
|
||||||
```
|
```
|
||||||
|
|
||||||
|
If initialization reports that authorization is required, return to the
|
||||||
|
application's authorization flow:
|
||||||
|
|
||||||
|
```rust ignore
|
||||||
|
let client = match client_service.serve(transport).await {
|
||||||
|
Ok(client) => client,
|
||||||
|
Err(error) if error.is_authorization_required() => {
|
||||||
|
// Prompt the user and start the application's authorization flow again.
|
||||||
|
return Err(error.into());
|
||||||
|
}
|
||||||
|
Err(error) => return Err(error.into()),
|
||||||
|
};
|
||||||
|
```
|
||||||
|
|
||||||
|
The predicate covers both missing or expired local OAuth authorization and an
|
||||||
|
HTTP 401 challenge from the MCP server. Other failures, including transient
|
||||||
|
token-refresh errors and insufficient scope, return `false`. The original error
|
||||||
|
is preserved for logging or more detailed handling; the SDK does not start an
|
||||||
|
authorization flow automatically.
|
||||||
|
|
||||||
### 6. Handle scope upgrades
|
### 6. Handle scope upgrades
|
||||||
|
|
||||||
If a server returns 403 with `insufficient_scope`, you can request a scope
|
If a server returns 403 with `insufficient_scope`, you can request a scope
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue