From d9a55609531545b656b584d46b912cba365585f3 Mon Sep 17 00:00:00 2001 From: Anar Azadaliyev Date: Fri, 13 Feb 2026 16:56:07 +0200 Subject: [PATCH] feat(auth): add token_endpoint_auth_method to OAuthClientConfig (#648) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(auth): add token_endpoint_auth_method to OAuthClientConfig Some OAuth providers (e.g. HubSpot) require client credentials to be sent as POST body parameters (client_secret_post) instead of via HTTP Basic Auth header. The oauth2 crate defaults to BasicAuth, and rmcp had no way to override this, causing TokenExchangeFailed errors. Add an optional `token_endpoint_auth_method` field to OAuthClientConfig that accepts "client_secret_post" (RequestBody) and "client_secret_basic" (BasicAuth). Unknown values are silently ignored, preserving the default. Co-Authored-By: Claude Opus 4.6 * refactor(auth): derive token_endpoint_auth_method from server metadata Move auth method selection from per-client config to server's AuthorizationMetadata, which is the correct OAuth 2.0 approach. Servers like HubSpot advertise token_endpoint_auth_methods_supported in their metadata; reading it from there avoids manual configuration and prevents TokenExchangeFailed errors with non-BasicAuth providers. Co-Authored-By: Claude Opus 4.6 * refactor(auth): read token_endpoint_auth_methods_supported from additional_fields Move token_endpoint_auth_methods_supported out of AuthorizationMetadata as an explicit field and read it from the serde(flatten) additional_fields HashMap instead. This avoids serializing `null` when the field is absent, which broke Zod validation in downstream consumers like MCP Inspector. Co-Authored-By: Claude Opus 4.6 * feat(auth): prefer basic auth when both methods supported and improve test assertions When token_endpoint_auth_methods_supported contains both client_secret_post and client_secret_basic, default to basic auth per RFC 6749 ยง2.3.1. Update configure_client tests to assert actual AuthType instead of is_some(). Co-Authored-By: Claude Opus 4.6 * style(auth): apply cargo fmt formatting * style(auth): apply nightly cargo fmt import grouping * revert: undo .gitignore change --------- Co-authored-by: Anar Azadaliyev Co-authored-by: Claude Opus 4.6 --- crates/rmcp/src/transport/auth.rs | 142 +++++++++++++++++++++++++++++- 1 file changed, 138 insertions(+), 4 deletions(-) diff --git a/crates/rmcp/src/transport/auth.rs b/crates/rmcp/src/transport/auth.rs index ad2d69a..9c9a1d0 100644 --- a/crates/rmcp/src/transport/auth.rs +++ b/crates/rmcp/src/transport/auth.rs @@ -2,7 +2,7 @@ use std::{collections::HashMap, sync::Arc, time::Duration}; use async_trait::async_trait; use oauth2::{ - AuthUrl, AuthorizationCode, ClientId, ClientSecret, CsrfToken, EmptyExtraTokenFields, + AuthType, AuthUrl, AuthorizationCode, ClientId, ClientSecret, CsrfToken, EmptyExtraTokenFields, PkceCodeChallenge, PkceCodeVerifier, RedirectUrl, RefreshToken, RequestTokenError, Scope, StandardTokenResponse, TokenResponse, TokenUrl, basic::{BasicClient, BasicTokenType}, @@ -548,6 +548,23 @@ impl AuthorizationManager { client_builder = client_builder.set_client_secret(ClientSecret::new(secret)); } + let uses_secret_post = metadata + .additional_fields + .get("token_endpoint_auth_methods_supported") + .and_then(|v| v.as_array()) + .map(|arr| { + let has_basic = arr + .iter() + .any(|m| m.as_str() == Some("client_secret_basic")); + let has_post = arr.iter().any(|m| m.as_str() == Some("client_secret_post")); + has_post && !has_basic + }) + .unwrap_or(false); + + if uses_secret_post { + client_builder = client_builder.set_auth_type(AuthType::RequestBody); + } + self.oauth_client = Some(client_builder); Ok(()) } @@ -1770,14 +1787,14 @@ impl OAuthState { #[cfg(test)] mod tests { - use std::sync::Arc; + use std::{collections::HashMap, sync::Arc}; - use oauth2::{CsrfToken, PkceCodeVerifier}; + use oauth2::{AuthType, CsrfToken, PkceCodeVerifier}; use url::Url; use super::{ AuthError, AuthorizationManager, AuthorizationMetadata, InMemoryStateStore, - ScopeUpgradeConfig, StateStore, StoredAuthorizationState, is_https_url, + OAuthClientConfig, ScopeUpgradeConfig, StateStore, StoredAuthorizationState, is_https_url, }; // -- url helpers -- @@ -2263,6 +2280,123 @@ mod tests { manager.set_state_store(TrackingStateStore::default()); } + /// Helper: create an AuthorizationManager with minimal metadata so + /// `configure_client` can be exercised without a live server. + async fn manager_with_metadata( + metadata_override: Option, + ) -> AuthorizationManager { + let mut mgr = AuthorizationManager::new("http://localhost").await.unwrap(); + mgr.set_metadata(metadata_override.unwrap_or(AuthorizationMetadata { + authorization_endpoint: "http://localhost/authorize".to_string(), + token_endpoint: "http://localhost/token".to_string(), + ..Default::default() + })); + mgr + } + + fn test_client_config() -> OAuthClientConfig { + OAuthClientConfig { + client_id: "my-client".to_string(), + client_secret: Some("my-secret".to_string()), + scopes: vec![], + redirect_uri: "http://localhost/callback".to_string(), + } + } + + #[tokio::test] + async fn test_configure_client_uses_client_secret_post_from_metadata() { + let mut additional_fields = HashMap::new(); + additional_fields.insert( + "token_endpoint_auth_methods_supported".to_string(), + serde_json::json!(["client_secret_post"]), + ); + let meta = AuthorizationMetadata { + authorization_endpoint: "http://localhost/authorize".to_string(), + token_endpoint: "http://localhost/token".to_string(), + additional_fields, + ..Default::default() + }; + let mut mgr = manager_with_metadata(Some(meta)).await; + mgr.configure_client(test_client_config()).unwrap(); + assert!(matches!( + mgr.oauth_client.as_ref().unwrap().auth_type(), + AuthType::RequestBody + )); + } + + #[tokio::test] + async fn test_configure_client_defaults_to_basic_auth() { + let mut mgr = manager_with_metadata(None).await; + mgr.configure_client(test_client_config()).unwrap(); + assert!(matches!( + mgr.oauth_client.as_ref().unwrap().auth_type(), + AuthType::BasicAuth + )); + } + + #[tokio::test] + async fn test_configure_client_with_explicit_basic_in_metadata() { + let mut additional_fields = HashMap::new(); + additional_fields.insert( + "token_endpoint_auth_methods_supported".to_string(), + serde_json::json!(["client_secret_basic"]), + ); + let meta = AuthorizationMetadata { + authorization_endpoint: "http://localhost/authorize".to_string(), + token_endpoint: "http://localhost/token".to_string(), + additional_fields, + ..Default::default() + }; + let mut mgr = manager_with_metadata(Some(meta)).await; + mgr.configure_client(test_client_config()).unwrap(); + assert!(matches!( + mgr.oauth_client.as_ref().unwrap().auth_type(), + AuthType::BasicAuth + )); + } + + #[tokio::test] + async fn test_configure_client_ignores_unsupported_auth_methods_in_metadata() { + let mut additional_fields = HashMap::new(); + additional_fields.insert( + "token_endpoint_auth_methods_supported".to_string(), + serde_json::json!(["private_key_jwt"]), + ); + let meta = AuthorizationMetadata { + authorization_endpoint: "http://localhost/authorize".to_string(), + token_endpoint: "http://localhost/token".to_string(), + additional_fields, + ..Default::default() + }; + let mut mgr = manager_with_metadata(Some(meta)).await; + // Unsupported method should fall through to default (basic auth) + mgr.configure_client(test_client_config()).unwrap(); + assert!(matches!( + mgr.oauth_client.as_ref().unwrap().auth_type(), + AuthType::BasicAuth + )); + } + + #[tokio::test] + async fn test_configure_client_prefers_basic_when_both_methods_supported() { + let mut additional_fields = HashMap::new(); + additional_fields.insert( + "token_endpoint_auth_methods_supported".to_string(), + serde_json::json!(["client_secret_post", "client_secret_basic"]), + ); + let meta = AuthorizationMetadata { + authorization_endpoint: "http://localhost/authorize".to_string(), + token_endpoint: "http://localhost/token".to_string(), + additional_fields, + ..Default::default() + }; + let mut mgr = manager_with_metadata(Some(meta)).await; + mgr.configure_client(test_client_config()).unwrap(); + assert!(matches!( + mgr.oauth_client.as_ref().unwrap().auth_type(), + AuthType::BasicAuth + )); + } // -- metadata deserialization -- #[test]