Skip to content

Commit 9e87e9b

Browse files
committed
fix(auth): add an SDK path for pre-registered OAuth clients
... and exercise it in conformance tests
1 parent 98bb263 commit 9e87e9b

2 files changed

Lines changed: 290 additions & 34 deletions

File tree

conformance/src/bin/client.rs

Lines changed: 20 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -236,50 +236,36 @@ async fn perform_oauth_flow(
236236
Ok(AuthClient::new(reqwest::Client::default(), am))
237237
}
238238

239-
/// Like `perform_oauth_flow` but uses pre-registered client credentials.
239+
/// Like `perform_oauth_flow` but uses pre-registered client credentials,
240+
/// exercising the SDK's high-level `OAuthState` path (no DCR).
240241
async fn perform_oauth_flow_preregistered(
241242
server_url: &str,
242243
client_id: &str,
243244
client_secret: &str,
244245
) -> anyhow::Result<AuthClient<reqwest::Client>> {
245-
let mut manager = AuthorizationManager::new(server_url).await?;
246-
let metadata = manager.discover_metadata().await?;
247-
manager.set_metadata(metadata);
246+
let mut oauth = OAuthState::new(server_url, None).await?;
248247

249-
// Configure with pre-registered credentials
250248
let config = rmcp::transport::auth::OAuthClientConfig::new(client_id, REDIRECT_URI)
251249
.with_client_secret(client_secret);
252-
manager.configure_client(config)?;
250+
oauth
251+
.start_authorization_with_preregistered_client(config)
252+
.await?;
253+
254+
let auth_url = oauth.get_authorization_url().await?;
255+
let callback = headless_authorize(&auth_url).await?;
256+
oauth
257+
.handle_callback_with_issuer(
258+
&callback.code,
259+
&callback.csrf_token,
260+
callback.issuer.as_deref(),
261+
)
262+
.await?;
253263

254-
let scopes = manager.select_scopes(None, &[]);
255-
let scope_refs: Vec<&str> = scopes.iter().map(|s| s.as_str()).collect();
256-
let auth_url = manager.get_authorization_url(&scope_refs).await?;
264+
let am = oauth
265+
.into_authorization_manager()
266+
.ok_or_else(|| anyhow::anyhow!("Failed to get authorization manager"))?;
257267

258-
// Headless redirect
259-
let http = reqwest::Client::builder()
260-
.redirect(reqwest::redirect::Policy::none())
261-
.build()?;
262-
let resp = http.get(&auth_url).send().await?;
263-
let location = resp
264-
.headers()
265-
.get("location")
266-
.and_then(|v| v.to_str().ok())
267-
.ok_or_else(|| anyhow::anyhow!("No Location header"))?;
268-
let redirect_url = url::Url::parse(location)?;
269-
let code = redirect_url
270-
.query_pairs()
271-
.find(|(k, _)| k == "code")
272-
.map(|(_, v)| v.to_string())
273-
.ok_or_else(|| anyhow::anyhow!("No code"))?;
274-
let state = redirect_url
275-
.query_pairs()
276-
.find(|(k, _)| k == "state")
277-
.map(|(_, v)| v.to_string())
278-
.ok_or_else(|| anyhow::anyhow!("No state"))?;
279-
280-
manager.exchange_code_for_token(&code, &state).await?;
281-
282-
Ok(AuthClient::new(reqwest::Client::default(), manager))
268+
Ok(AuthClient::new(reqwest::Client::default(), am))
283269
}
284270

285271
/// Run the standard auth flow, then connect and exercise the server.

crates/rmcp/src/transport/auth.rs

Lines changed: 270 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2803,6 +2803,34 @@ impl AuthorizationSession {
28032803
})
28042804
}
28052805

2806+
/// create a session using pre-registered client credentials, skipping
2807+
/// dynamic client registration and URL-based client IDs.
2808+
///
2809+
/// The manager must already have discovered authorization server metadata.
2810+
///
2811+
/// On failure, the manager is returned alongside the error so callers can
2812+
/// retry without losing the original configuration and stores.
2813+
pub async fn with_preregistered_client(
2814+
mut auth_manager: AuthorizationManager,
2815+
config: OAuthClientConfig,
2816+
) -> Result<Self, (AuthorizationManager, AuthError)> {
2817+
let redirect_uri = config.redirect_uri.clone();
2818+
let scopes = config.scopes.clone();
2819+
if let Err(e) = auth_manager.configure_client(config) {
2820+
return Err((auth_manager, e));
2821+
}
2822+
let scope_refs: Vec<&str> = scopes.iter().map(|s| s.as_str()).collect();
2823+
let auth_url = match auth_manager.get_authorization_url(&scope_refs).await {
2824+
Ok(url) => url,
2825+
Err(e) => return Err((auth_manager, e)),
2826+
};
2827+
Ok(Self {
2828+
auth_manager,
2829+
auth_url,
2830+
redirect_uri,
2831+
})
2832+
}
2833+
28062834
/// create session for scope upgrade flow (existing manager + pre-computed auth url)
28072835
pub fn for_scope_upgrade(
28082836
auth_manager: AuthorizationManager,
@@ -3073,6 +3101,50 @@ impl OAuthState {
30733101
}
30743102
}
30753103

3104+
/// start authorization using pre-registered client credentials,
3105+
/// skipping dynamic client registration.
3106+
///
3107+
/// Use this when the client was registered with the authorization server
3108+
/// out of band and already holds a `client_id` (and optionally a
3109+
/// `client_secret`). If `config.scopes` is empty, scopes are selected
3110+
/// using the SDK's normal scope-selection policy.
3111+
pub async fn start_authorization_with_preregistered_client(
3112+
&mut self,
3113+
mut config: OAuthClientConfig,
3114+
) -> Result<(), AuthError> {
3115+
let placeholder = self.placeholder().await?;
3116+
let old = std::mem::replace(self, placeholder);
3117+
let OAuthState::Unauthorized(mut manager) = old else {
3118+
*self = old;
3119+
return Err(AuthError::InternalError(
3120+
"Already in session state".to_string(),
3121+
));
3122+
};
3123+
let metadata = match manager.discover_metadata().await {
3124+
Ok(metadata) => metadata,
3125+
Err(e) => {
3126+
*self = OAuthState::Unauthorized(manager);
3127+
return Err(e);
3128+
}
3129+
};
3130+
manager.metadata = Some(metadata);
3131+
if config.scopes.is_empty() {
3132+
config.scopes = manager.select_scopes(None, &[]);
3133+
} else {
3134+
manager.add_offline_access_if_supported(&mut config.scopes);
3135+
}
3136+
match AuthorizationSession::with_preregistered_client(manager, config).await {
3137+
Ok(session) => {
3138+
*self = OAuthState::Session(session);
3139+
Ok(())
3140+
}
3141+
Err((manager, e)) => {
3142+
*self = OAuthState::Unauthorized(manager);
3143+
Err(e)
3144+
}
3145+
}
3146+
}
3147+
30763148
/// complete authorization
30773149
pub async fn complete_authorization(&mut self) -> Result<(), AuthError> {
30783150
let placeholder = self.placeholder().await?;
@@ -3583,6 +3655,204 @@ mod tests {
35833655
);
35843656
}
35853657

3658+
fn preregistered_as_metadata_response() -> HttpResponse {
3659+
http_response(
3660+
200,
3661+
serde_json::json!({
3662+
"issuer": "https://auth.example.com",
3663+
"authorization_endpoint": "https://auth.example.com/authorize",
3664+
"token_endpoint": "https://auth.example.com/token",
3665+
"registration_endpoint": "https://auth.example.com/register",
3666+
"scopes_supported": ["read", "write", "offline_access"]
3667+
}),
3668+
)
3669+
}
3670+
3671+
/// discovery responses for the preregistered-client tests: a 401 challenge
3672+
/// pointing at protected resource metadata, the PRM document, then the
3673+
/// authorization server metadata.
3674+
fn preregistered_discovery_responses() -> Vec<HttpResponse> {
3675+
let challenge = oauth2::http::Response::builder()
3676+
.status(401)
3677+
.header(
3678+
"www-authenticate",
3679+
r#"Bearer resource_metadata="https://mcp.example.com/.well-known/oauth-protected-resource""#,
3680+
)
3681+
.body(Vec::new())
3682+
.unwrap();
3683+
vec![
3684+
challenge,
3685+
http_response(
3686+
200,
3687+
serde_json::json!({
3688+
"resource": "https://mcp.example.com/mcp",
3689+
"authorization_servers": ["https://auth.example.com"]
3690+
}),
3691+
),
3692+
preregistered_as_metadata_response(),
3693+
]
3694+
}
3695+
3696+
fn auth_url_query(auth_url: &str) -> HashMap<String, String> {
3697+
Url::parse(auth_url)
3698+
.unwrap()
3699+
.query_pairs()
3700+
.map(|(k, v)| (k.into_owned(), v.into_owned()))
3701+
.collect()
3702+
}
3703+
3704+
#[tokio::test]
3705+
async fn preregistered_client_skips_registration_endpoint() {
3706+
let client = RecordingOAuthHttpClient::with_responses(preregistered_discovery_responses());
3707+
let mut state = super::OAuthState::new_with_oauth_http_client(
3708+
"https://mcp.example.com/mcp",
3709+
Arc::new(client.clone()),
3710+
)
3711+
.await
3712+
.unwrap();
3713+
3714+
let config = OAuthClientConfig {
3715+
client_id: "preregistered-client".to_string(),
3716+
client_secret: Some("secret".to_string()),
3717+
scopes: vec!["read".to_string()],
3718+
redirect_uri: "http://localhost:8080/callback".to_string(),
3719+
application_type: None,
3720+
};
3721+
state
3722+
.start_authorization_with_preregistered_client(config)
3723+
.await
3724+
.unwrap();
3725+
3726+
// the registration endpoint was advertised but must not be called
3727+
let requests = client.requests();
3728+
assert!(
3729+
requests
3730+
.iter()
3731+
.all(|request| !request.uri.contains("/register")),
3732+
"registration endpoint should not be called: {requests:?}"
3733+
);
3734+
3735+
let auth_url = state.get_authorization_url().await.unwrap();
3736+
let query = auth_url_query(&auth_url);
3737+
assert_eq!(query.get("client_id").unwrap(), "preregistered-client");
3738+
assert!(matches!(state, super::OAuthState::Session(_)));
3739+
}
3740+
3741+
#[tokio::test]
3742+
async fn preregistered_client_selects_default_scopes_when_none_provided() {
3743+
let client = RecordingOAuthHttpClient::with_responses(preregistered_discovery_responses());
3744+
let mut state = super::OAuthState::new_with_oauth_http_client(
3745+
"https://mcp.example.com/mcp",
3746+
Arc::new(client.clone()),
3747+
)
3748+
.await
3749+
.unwrap();
3750+
3751+
let config = OAuthClientConfig {
3752+
client_id: "preregistered-client".to_string(),
3753+
client_secret: None,
3754+
scopes: Vec::new(),
3755+
redirect_uri: "http://localhost:8080/callback".to_string(),
3756+
application_type: None,
3757+
};
3758+
state
3759+
.start_authorization_with_preregistered_client(config)
3760+
.await
3761+
.unwrap();
3762+
3763+
// empty config scopes fall back to the discovered scopes_supported
3764+
let auth_url = state.get_authorization_url().await.unwrap();
3765+
let query = auth_url_query(&auth_url);
3766+
assert_eq!(query.get("scope").unwrap(), "read write offline_access");
3767+
}
3768+
3769+
#[tokio::test]
3770+
async fn preregistered_client_uses_explicit_scopes_and_adds_offline_access() {
3771+
let client = RecordingOAuthHttpClient::with_responses(preregistered_discovery_responses());
3772+
let mut state = super::OAuthState::new_with_oauth_http_client(
3773+
"https://mcp.example.com/mcp",
3774+
Arc::new(client.clone()),
3775+
)
3776+
.await
3777+
.unwrap();
3778+
3779+
let config = OAuthClientConfig {
3780+
client_id: "preregistered-client".to_string(),
3781+
client_secret: None,
3782+
scopes: vec!["read".to_string()],
3783+
redirect_uri: "http://localhost:8080/callback".to_string(),
3784+
application_type: None,
3785+
};
3786+
state
3787+
.start_authorization_with_preregistered_client(config)
3788+
.await
3789+
.unwrap();
3790+
3791+
// explicit scopes are preserved; offline_access is appended per SEP-2207
3792+
let auth_url = state.get_authorization_url().await.unwrap();
3793+
let query = auth_url_query(&auth_url);
3794+
assert_eq!(query.get("scope").unwrap(), "read offline_access");
3795+
}
3796+
3797+
#[tokio::test]
3798+
async fn preregistered_client_recovers_unauthorized_state_after_discovery_failure() {
3799+
// first discovery attempt fails: protected resource metadata reports a
3800+
// mismatched resource identifier, which discover_metadata rejects
3801+
let challenge = oauth2::http::Response::builder()
3802+
.status(401)
3803+
.header(
3804+
"www-authenticate",
3805+
r#"Bearer resource_metadata="https://mcp.example.com/.well-known/oauth-protected-resource""#,
3806+
)
3807+
.body(Vec::new())
3808+
.unwrap();
3809+
let client = RecordingOAuthHttpClient::with_responses(vec![
3810+
challenge,
3811+
http_response(
3812+
200,
3813+
serde_json::json!({
3814+
"resource": "https://other.example.com/mcp",
3815+
"authorization_servers": ["https://auth.example.com"]
3816+
}),
3817+
),
3818+
]);
3819+
let mut state = super::OAuthState::new_with_oauth_http_client(
3820+
"https://mcp.example.com/mcp",
3821+
Arc::new(client.clone()),
3822+
)
3823+
.await
3824+
.unwrap();
3825+
3826+
let config = OAuthClientConfig {
3827+
client_id: "preregistered-client".to_string(),
3828+
client_secret: None,
3829+
scopes: vec!["read".to_string()],
3830+
redirect_uri: "http://localhost:8080/callback".to_string(),
3831+
application_type: None,
3832+
};
3833+
let err = state
3834+
.start_authorization_with_preregistered_client(config.clone())
3835+
.await
3836+
.unwrap_err();
3837+
assert!(!matches!(err, AuthError::InternalError(_)), "{err:?}");
3838+
assert!(
3839+
matches!(state, super::OAuthState::Unauthorized(_)),
3840+
"state should return to Unauthorized after a transient failure"
3841+
);
3842+
3843+
// retrying with the same state succeeds once the server responds
3844+
client
3845+
.responses
3846+
.lock()
3847+
.unwrap()
3848+
.extend(preregistered_discovery_responses());
3849+
state
3850+
.start_authorization_with_preregistered_client(config)
3851+
.await
3852+
.unwrap();
3853+
assert!(matches!(state, super::OAuthState::Session(_)));
3854+
}
3855+
35863856
#[tokio::test]
35873857
async fn discovery_get_follows_same_origin_redirects() {
35883858
let client = RecordingOAuthHttpClient::with_responses(vec![

0 commit comments

Comments
 (0)