feat(auth): add token_endpoint_auth_method to OAuthClientConfig (#648)
* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * style(auth): apply cargo fmt formatting * style(auth): apply nightly cargo fmt import grouping * revert: undo .gitignore change --------- Co-authored-by: Anar Azadaliyev <anar.azadaliye@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
bb534a7a68
commit
d9a5560953
1 changed files with 138 additions and 4 deletions
|
|
@ -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<AuthorizationMetadata>,
|
||||
) -> 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]
|
||||
|
|
|
|||
Loading…
Reference in a new issue