fix(auth): select supported DCR authentication methods

[dux-web]

Summary

• select a supported dynamic-registration token endpoint authentication method
  from authorization-server metadata
• preserve none when the metadata field is omitted or public registration is
  supported
• reject malformed, empty, or unsupported advertised methods before POSTing
• require a non-empty returned secret for confidential registrations

The supported preference order is none, client_secret_basic, then
client_secret_post. Existing OAuth client configuration continues to apply
the selected Basic or POST authentication method after registration.

Proposal: #1355, opened first under the repository's discussion guidance.

Validation

Validated on Rust 1.96.1:

cargo test -p rmcp --lib --features auth transport::auth::tests::
cargo clippy --locked -p rmcp --lib --features auth -- -D warnings
rustfmt --edition 2024 --check crates/rmcp/src/transport/auth.rs

The test command passes all 210 authentication tests, including method
selection, rejection before registration, and missing or empty confidential
client secrets. Library Clippy and formatting pass; stable rustfmt reports
the repository's existing nightly-only configuration warnings.

The first test command generated this library workspace's ignored Cargo.lock.
A broader --tests Clippy invocation was attempted but hit an existing stdio
integration test requiring the unselected transport-io feature. No assertions
or unrelated test sources were changed. Full workspace/feature checks remain
for CI.

Prepared with AI assistance.

[github-actions]

@bolinfest is on triage rotation this week and is the reviewer for this one.

Automated classification did not run for this one, so it needs a full manual pass.

_{Automated triage. Labels are a first pass and the assigned maintainer has the final say.}

[DaleSeo]

Before this change, a server advertising only methods the SDK can't do (e.g. private_key_jwt) still got a none registration attempt, and some of those accept public DCR anyway; failing before the POST turns them into a hard error. Would falling back to none and letting the server reject the request be the safer default here?

Suggested change

	.ok_or_else(|| {
	AuthError::RegistrationFailed(
	"No supported client authentication method".to_string(),
	)
	})?,
	.unwrap_or("none"),

[DaleSeo]

RFC 7591 §3.2.1 lets the server return a different token_endpoint_auth_method than the one requested, but the response's value is never read. Should the secret check (and the Basic/POST choice) follow what the server actually registered?

Suggested change

	if auth_method != "none" && config.client_secret.is_none() {
	return Err(AuthError::RegistrationFailed(
	"Confidential registration returned no client secret".to_string(),
	));
	}
	let registered_method = reg_response
	.additional_fields
	.get("token_endpoint_auth_method")
	.and_then(|method| method.as_str())
	.unwrap_or(auth_method);
	if registered_method != "none" && config.client_secret.is_none() {
	return Err(AuthError::RegistrationFailed(
	"Confidential registration returned no client secret".to_string(),
	));
	}

[DaleSeo]

configure_client re-derives Basic vs. POST from the metadata on its own, so the method sent at registration and the one used at the token endpoint only agree because the two preference orders happen to match. Would a shared helper that both call keep them from drifting apart?

+fn select_token_endpoint_auth_method(metadata: &AuthorizationMetadata) -> Option<&'static str> {
+    let Some(methods) = metadata.additional_fields.get("token_endpoint_auth_methods_supported") else {
+        return Some("none");
+    };
+    let methods = methods.as_array().map(Vec::as_slice).unwrap_or_default();
+    ["none", "client_secret_basic", "client_secret_post"]
+        .into_iter()
+        .find(|method| methods.iter().any(|value| value.as_str() == Some(*method)))
+}

[DaleSeo]

methods.as_array() is re-evaluated for every candidate inside find; resolving the slice once reads more directly and treats a non-array the same as an empty list.

Suggested change

	let auth_method = match metadata
	.additional_fields
	.get("token_endpoint_auth_methods_supported")
	{
	None => "none",
	Some(methods) => ["none", "client_secret_basic", "client_secret_post"]
	.into_iter()
	.find(|method| {
	methods.as_array().is_some_and(|methods| {
	methods.iter().any(|value| value.as_str() == Some(*method))
	})
	})
	.ok_or_else(|| {
	AuthError::RegistrationFailed(
	"No supported client authentication method".to_string(),
	)
	})?,
	};
	let auth_method = match metadata
	.additional_fields
	.get("token_endpoint_auth_methods_supported")
	{
	None => "none",
	Some(methods) => {
	let methods = methods.as_array().map(Vec::as_slice).unwrap_or_default();
	["none", "client_secret_basic", "client_secret_post"]
	.into_iter()
	.find(|method| methods.iter().any(|value| value.as_str() == Some(*method)))
	.ok_or_else(|| {
	AuthError::RegistrationFailed(
	"No supported client authentication method".to_string(),
	)
	})?
	}
	};

[DaleSeo]

An explicit "token_endpoint_auth_methods_supported": null currently fails registration, while omitting the field succeeds. Should null be treated like an omitted field?

Suggested change

	None => "none",
	None | Some(serde_json::Value::Null) => "none",

[DaleSeo]

Comparing a matches! result to a bool inside one assert_eq! is hard to read; naming the boolean first makes the intent clearer.

Suggested change

	if expected != "none" {
	assert_eq!(
	matches!(
	manager.oauth_client.as_ref().unwrap().auth_type(),
	AuthType::RequestBody
	),
	expected == "client_secret_post"
	);
	}
	if expected != "none" {
	let uses_post = matches!(
	manager.oauth_client.as_ref().unwrap().auth_type(),
	AuthType::RequestBody
	);
	assert_eq!(uses_post, expected == "client_secret_post");
	}