From 77326081655a6ee52e3bea82d933a8b7509df4e5 Mon Sep 17 00:00:00 2001 From: Jack Amadeo Date: Wed, 15 Jul 2026 12:10:11 -0400 Subject: [PATCH] fix(auth): add an SDK path for pre-registered OAuth clients ... and exercise it in conformance tests --- conformance/src/bin/client.rs | 54 ++++++++++++------------------- crates/rmcp/src/transport/auth.rs | 50 ++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 34 deletions(-) diff --git a/conformance/src/bin/client.rs b/conformance/src/bin/client.rs index 850d74f3..872868c9 100644 --- a/conformance/src/bin/client.rs +++ b/conformance/src/bin/client.rs @@ -236,50 +236,36 @@ async fn perform_oauth_flow( Ok(AuthClient::new(reqwest::Client::default(), am)) } -/// Like `perform_oauth_flow` but uses pre-registered client credentials. +/// Like `perform_oauth_flow` but uses pre-registered client credentials, +/// exercising the SDK's high-level `OAuthState` path (no DCR). async fn perform_oauth_flow_preregistered( server_url: &str, client_id: &str, client_secret: &str, ) -> anyhow::Result> { - let mut manager = AuthorizationManager::new(server_url).await?; - let metadata = manager.discover_metadata().await?; - manager.set_metadata(metadata); + let mut oauth = OAuthState::new(server_url, None).await?; - // Configure with pre-registered credentials let config = rmcp::transport::auth::OAuthClientConfig::new(client_id, REDIRECT_URI) .with_client_secret(client_secret); - manager.configure_client(config)?; + oauth + .start_authorization_with_preregistered_client(config) + .await?; + + let auth_url = oauth.get_authorization_url().await?; + let callback = headless_authorize(&auth_url).await?; + oauth + .handle_callback_with_issuer( + &callback.code, + &callback.csrf_token, + callback.issuer.as_deref(), + ) + .await?; - let scopes = manager.select_scopes(None, &[]); - let scope_refs: Vec<&str> = scopes.iter().map(|s| s.as_str()).collect(); - let auth_url = manager.get_authorization_url(&scope_refs).await?; + let am = oauth + .into_authorization_manager() + .ok_or_else(|| anyhow::anyhow!("Failed to get authorization manager"))?; - // Headless redirect - let http = reqwest::Client::builder() - .redirect(reqwest::redirect::Policy::none()) - .build()?; - let resp = http.get(&auth_url).send().await?; - let location = resp - .headers() - .get("location") - .and_then(|v| v.to_str().ok()) - .ok_or_else(|| anyhow::anyhow!("No Location header"))?; - let redirect_url = url::Url::parse(location)?; - let code = redirect_url - .query_pairs() - .find(|(k, _)| k == "code") - .map(|(_, v)| v.to_string()) - .ok_or_else(|| anyhow::anyhow!("No code"))?; - let state = redirect_url - .query_pairs() - .find(|(k, _)| k == "state") - .map(|(_, v)| v.to_string()) - .ok_or_else(|| anyhow::anyhow!("No state"))?; - - manager.exchange_code_for_token(&code, &state).await?; - - Ok(AuthClient::new(reqwest::Client::default(), manager)) + Ok(AuthClient::new(reqwest::Client::default(), am)) } /// Run the standard auth flow, then connect and exercise the server. diff --git a/crates/rmcp/src/transport/auth.rs b/crates/rmcp/src/transport/auth.rs index b1f505ea..2c33fa42 100644 --- a/crates/rmcp/src/transport/auth.rs +++ b/crates/rmcp/src/transport/auth.rs @@ -2803,6 +2803,26 @@ impl AuthorizationSession { }) } + /// create a session using pre-registered client credentials, skipping + /// dynamic client registration and URL-based client IDs. + /// + /// The manager must already have discovered authorization server metadata. + pub async fn with_preregistered_client( + mut auth_manager: AuthorizationManager, + config: OAuthClientConfig, + ) -> Result { + let redirect_uri = config.redirect_uri.clone(); + let scopes = config.scopes.clone(); + auth_manager.configure_client(config)?; + let scope_refs: Vec<&str> = scopes.iter().map(|s| s.as_str()).collect(); + let auth_url = auth_manager.get_authorization_url(&scope_refs).await?; + Ok(Self { + auth_manager, + auth_url, + redirect_uri, + }) + } + /// create session for scope upgrade flow (existing manager + pre-computed auth url) pub fn for_scope_upgrade( auth_manager: AuthorizationManager, @@ -3073,6 +3093,36 @@ impl OAuthState { } } + /// start authorization using pre-registered client credentials, + /// skipping dynamic client registration. + /// + /// Use this when the client was registered with the authorization server + /// out of band and already holds a `client_id` (and optionally a + /// `client_secret`). If `config.scopes` is empty, scopes are selected + /// from the discovered authorization server metadata. + pub async fn start_authorization_with_preregistered_client( + &mut self, + mut config: OAuthClientConfig, + ) -> Result<(), AuthError> { + let placeholder = self.placeholder().await?; + if let OAuthState::Unauthorized(mut manager) = std::mem::replace(self, placeholder) { + let metadata = manager.discover_metadata().await?; + manager.metadata = Some(metadata); + if config.scopes.is_empty() { + config.scopes = manager.select_scopes(None, &[]); + } else { + manager.add_offline_access_if_supported(&mut config.scopes); + } + let session = AuthorizationSession::with_preregistered_client(manager, config).await?; + *self = OAuthState::Session(session); + Ok(()) + } else { + Err(AuthError::InternalError( + "Already in session state".to_string(), + )) + } + } + /// complete authorization pub async fn complete_authorization(&mut self) -> Result<(), AuthError> { let placeholder = self.placeholder().await?;