Skip to content

fix(auth): pass WWW-Authenticate scopes to DCR registration request #705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
peschee wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix(auth): pass WWW-Authenticate scopes to DCR registration request #705

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e8da53e
fix(auth): pass WWW-Authenticate scopes to DCR registration request
peschee Feb 27, 2026
8f2252a
refactor(auth): change register_client to accept &[&str] instead of &…
peschee Feb 27, 2026
3e08474
fix(auth): make ClientRegistrationRequest crate-private
peschee Feb 28, 2026
174456a
refactor(auth): stop re-exporting oauth2 TokenResponse trait
peschee Feb 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 44 additions & 5 deletions crates/rmcp/src/transport/auth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ use std::{
};

use async_trait::async_trait;
use oauth2::TokenResponse;
use oauth2::{
AsyncHttpClient, AuthType, AuthUrl, AuthorizationCode, ClientId, ClientSecret, CsrfToken,
EmptyExtraTokenFields, HttpClientError, HttpRequest, HttpResponse, PkceCodeChallenge,
PkceCodeVerifier, RedirectUrl, RefreshToken, RequestTokenError, Scope, StandardTokenResponse,
TokenResponse, TokenUrl,
TokenUrl,
basic::{BasicClient, BasicTokenType},
};
use reqwest::{
Expand Down Expand Up @@ -400,12 +401,14 @@ pub struct AuthorizationManager {
}

#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct ClientRegistrationRequest {
pub(crate) struct ClientRegistrationRequest {
pub client_name: String,
pub redirect_uris: Vec<String>,
pub grant_types: Vec<String>,
pub token_endpoint_auth_method: String,
pub response_types: Vec<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub scope: Option<String>,
Comment on lines +410 to +411
Copy link
Member

@DaleSeo DaleSeo Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ClientRegistrationRequest is a public struct that doesn't have #[non_exhaustive]. Adding the new scope field means that any downstream code constructing this struct directly will fail to compile. If this type is part of the crate's public API, consider adding #[non_exhaustive] to the struct. This would be a breaking change, but it’s a good way to future-proof it.

Copy link
Author

@peschee peschee Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! However, adding #[non_exhaustive] is itself a breaking change (it prevents downstream from constructing the struct with literal syntax), so it should probably be done as a separate PR that also considers the other public structs like ClientRegistrationResponse. Happy to open a follow-up for that if you'd like — or would you prefer to include it in this PR?

Copy link
Member

@DaleSeo DaleSeo Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took another look and noticed this struct isn't actually re-exported from the crate. Would it make sense to just scope it to pub(crate) here?

Copy link
Author

@peschee peschee Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to pub(crate) in 3e08474.

}

#[derive(Debug, Clone, Serialize, Deserialize)]
Expand Down Expand Up @@ -645,6 +648,7 @@ impl AuthorizationManager {
&mut self,
name: &str,
redirect_uri: &str,
scopes: &[&str],
) -> Result<OAuthClientConfig, AuthError> {
if self.metadata.is_none() {
return Err(AuthError::NoAuthorizationSupport);
Expand All @@ -667,6 +671,11 @@ impl AuthorizationManager {
],
token_endpoint_auth_method: "none".to_string(), // public client
response_types: vec!["code".to_string()],
scope: if scopes.is_empty() {
None
} else {
Some(scopes.join(" "))
},
};

let response = match self
Expand Down Expand Up @@ -720,7 +729,7 @@ impl AuthorizationManager {
// as a password, which is not a goal of the client secret.
client_secret: reg_response.client_secret.filter(|s| !s.is_empty()),
redirect_uri: redirect_uri.to_string(),
scopes: vec![],
scopes: scopes.iter().map(|s| s.to_string()).collect(),
};

self.configure_client(config.clone())?;
Expand Down Expand Up @@ -1490,7 +1499,7 @@ impl AuthorizationSession {
} else {
// Fallback to dynamic registration
auth_manager
.register_client(client_name.unwrap_or("MCP Client"), redirect_uri)
.register_client(client_name.unwrap_or("MCP Client"), redirect_uri, scopes)
.await
.map_err(|e| {
AuthError::RegistrationFailed(format!("Dynamic registration failed: {}", e))
Expand All @@ -1499,7 +1508,7 @@ impl AuthorizationSession {
} else {
// Fallback to dynamic registration
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
{
Ok(config) => config,
Expand Down Expand Up @@ -2792,4 +2801,34 @@ mod tests {
"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"));
}
}