fix(auth): pass WWW-Authenticate scopes to DCR registration request (#705)
* fix(auth): pass WWW-Authenticate scopes to DCR registration request
When an MCP server returns a 401 with `WWW-Authenticate: Bearer scope="..."`,
the scopes are parsed but never included in the Dynamic Client Registration
(DCR) request. Per RFC 7591, the DCR request should include a `scope` field
so the authorization server knows what scopes the client intends to use.
Servers that enforce scope-matching between registration and authorization
will reject the flow without this.
Changes:
- Add optional `scope` field to `ClientRegistrationRequest` with
`skip_serializing_if` for backward compatibility
- Update `register_client()` to accept scopes parameter and include
them in the DCR request body and returned `OAuthClientConfig`
- Thread scopes from `AuthorizationSession::new()` into both
`register_client()` call sites
- Re-export `oauth2::TokenResponse` trait so consumers can extract
scopes from token responses
- Add serialization tests for the new `scope` field
* refactor(auth): change register_client to accept &[&str] instead of &[String]
Avoids unnecessary Vec<String> allocation in callers that already have &[&str].
* fix(auth): make ClientRegistrationRequest crate-private
* refactor(auth): stop re-exporting oauth2 TokenResponse trait
* style(auth): merge TokenResponse into grouped oauth2 import
Fix nightly rustfmt check by consolidating the separate
`use oauth2::TokenResponse` into the existing `use oauth2::{...}` block.
This commit is contained in:
parent
2d90b76501
commit
434ccb7812
1 changed files with 42 additions and 4 deletions
|
|
@ -438,12 +438,14 @@ pub struct AuthorizationManager {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||||
pub struct ClientRegistrationRequest {
|
pub(crate) struct ClientRegistrationRequest {
|
||||||
pub client_name: String,
|
pub client_name: String,
|
||||||
pub redirect_uris: Vec<String>,
|
pub redirect_uris: Vec<String>,
|
||||||
pub grant_types: Vec<String>,
|
pub grant_types: Vec<String>,
|
||||||
pub token_endpoint_auth_method: String,
|
pub token_endpoint_auth_method: String,
|
||||||
pub response_types: Vec<String>,
|
pub response_types: Vec<String>,
|
||||||
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
|
pub scope: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||||
|
|
@ -683,6 +685,7 @@ impl AuthorizationManager {
|
||||||
&mut self,
|
&mut self,
|
||||||
name: &str,
|
name: &str,
|
||||||
redirect_uri: &str,
|
redirect_uri: &str,
|
||||||
|
scopes: &[&str],
|
||||||
) -> Result<OAuthClientConfig, AuthError> {
|
) -> Result<OAuthClientConfig, AuthError> {
|
||||||
if self.metadata.is_none() {
|
if self.metadata.is_none() {
|
||||||
return Err(AuthError::NoAuthorizationSupport);
|
return Err(AuthError::NoAuthorizationSupport);
|
||||||
|
|
@ -705,6 +708,11 @@ impl AuthorizationManager {
|
||||||
],
|
],
|
||||||
token_endpoint_auth_method: "none".to_string(), // public client
|
token_endpoint_auth_method: "none".to_string(), // public client
|
||||||
response_types: vec!["code".to_string()],
|
response_types: vec!["code".to_string()],
|
||||||
|
scope: if scopes.is_empty() {
|
||||||
|
None
|
||||||
|
} else {
|
||||||
|
Some(scopes.join(" "))
|
||||||
|
},
|
||||||
};
|
};
|
||||||
|
|
||||||
let response = match self
|
let response = match self
|
||||||
|
|
@ -758,7 +766,7 @@ impl AuthorizationManager {
|
||||||
// as a password, which is not a goal of the client secret.
|
// as a password, which is not a goal of the client secret.
|
||||||
client_secret: reg_response.client_secret.filter(|s| !s.is_empty()),
|
client_secret: reg_response.client_secret.filter(|s| !s.is_empty()),
|
||||||
redirect_uri: redirect_uri.to_string(),
|
redirect_uri: redirect_uri.to_string(),
|
||||||
scopes: vec![],
|
scopes: scopes.iter().map(|s| s.to_string()).collect(),
|
||||||
};
|
};
|
||||||
|
|
||||||
self.configure_client(config.clone())?;
|
self.configure_client(config.clone())?;
|
||||||
|
|
@ -1526,7 +1534,7 @@ impl AuthorizationSession {
|
||||||
} else {
|
} else {
|
||||||
// Fallback to dynamic registration
|
// Fallback to dynamic registration
|
||||||
auth_manager
|
auth_manager
|
||||||
.register_client(client_name.unwrap_or("MCP Client"), redirect_uri)
|
.register_client(client_name.unwrap_or("MCP Client"), redirect_uri, scopes)
|
||||||
.await
|
.await
|
||||||
.map_err(|e| {
|
.map_err(|e| {
|
||||||
AuthError::RegistrationFailed(format!("Dynamic registration failed: {}", e))
|
AuthError::RegistrationFailed(format!("Dynamic registration failed: {}", e))
|
||||||
|
|
@ -1535,7 +1543,7 @@ impl AuthorizationSession {
|
||||||
} else {
|
} else {
|
||||||
// Fallback to dynamic registration
|
// Fallback to dynamic registration
|
||||||
match auth_manager
|
match auth_manager
|
||||||
.register_client(client_name.unwrap_or("MCP Client"), redirect_uri)
|
.register_client(client_name.unwrap_or("MCP Client"), redirect_uri, scopes)
|
||||||
.await
|
.await
|
||||||
{
|
{
|
||||||
Ok(config) => config,
|
Ok(config) => config,
|
||||||
|
|
@ -2831,4 +2839,34 @@ mod tests {
|
||||||
"expected InternalError when OAuth client is not configured, got: {err:?}"
|
"expected InternalError when OAuth client is not configured, got: {err:?}"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// -- ClientRegistrationRequest serialization --
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn client_registration_request_includes_scope_when_present() {
|
||||||
|
let req = super::ClientRegistrationRequest {
|
||||||
|
client_name: "test".to_string(),
|
||||||
|
redirect_uris: vec!["http://localhost/callback".to_string()],
|
||||||
|
grant_types: vec!["authorization_code".to_string()],
|
||||||
|
token_endpoint_auth_method: "none".to_string(),
|
||||||
|
response_types: vec!["code".to_string()],
|
||||||
|
scope: Some("read write".to_string()),
|
||||||
|
};
|
||||||
|
let json = serde_json::to_value(&req).unwrap();
|
||||||
|
assert_eq!(json["scope"], "read write");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn client_registration_request_omits_scope_when_none() {
|
||||||
|
let req = super::ClientRegistrationRequest {
|
||||||
|
client_name: "test".to_string(),
|
||||||
|
redirect_uris: vec!["http://localhost/callback".to_string()],
|
||||||
|
grant_types: vec!["authorization_code".to_string()],
|
||||||
|
token_endpoint_auth_method: "none".to_string(),
|
||||||
|
response_types: vec!["code".to_string()],
|
||||||
|
scope: None,
|
||||||
|
};
|
||||||
|
let json = serde_json::to_value(&req).unwrap();
|
||||||
|
assert!(!json.as_object().unwrap().contains_key("scope"));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue