fix(auth): parse WWW-Authenticate challenges separately

[zamoshchin-openai]

AuthorizationManager searches for parameter names across an entire WWW-Authenticate value, which can combine unrelated challenges. For example, DPoP scope="dpop:read", Bearer resource_metadata="/metadata", scope="read" selects dpop:read for initial Bearer authorization.

Parse challenge boundaries and quoted parameter values before selecting a challenge. Prefer a Bearer metadata challenge and keep its parameters together; retain metadata pointers from other schemes as pointer-only discovery fallbacks. Apply the same parsing to repeated header fields and Bearer insufficient-scope responses.

Regression coverage includes mixed schemes, separate Bearer realms, quoted decoys and escapes, parameter-name boundaries, case and whitespace, repeated versus combined fields, and scope/error fallback ordering.

Validation:

• Focused auth unit tests: 252 passed.
• cargo +1.96 clippy -p rmcp --all-targets --all-features -- -D warnings
• cargo +nightly fmt --all -- --check
• git diff --check

[github-actions]

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

Labels come from the PR labeler. Please review the change.

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

[DaleSeo]

With DPoP resource_metadata="/m" and Bearer scope="read", the foreign metadata challenge takes precedence, so the Bearer scope hint is dropped and nothing is selected initially.

[DaleSeo]

The bare bool in (bool, WWWAuthenticateParams) doesn't say what it means at call sites like .find(|(_, params)| ...) or for (bearer, params) in .... Would a small named struct read better here?

Suggested change

	fn parse_www_authenticate_challenges(
	header: &str,
	base_url: &Url,
	) -> Vec<(bool, WWWAuthenticateParams)> {
	fn parse_www_authenticate_challenges(
	header: &str,
	base_url: &Url,
	) -> Vec<Challenge> {

with something like:

struct Challenge {
    bearer: bool,
    params: WWWAuthenticateParams,
}

[DaleSeo]

first_challenge only holds Bearer challenges with an error or error_description, so calling it error_challenge would match what it stores.

[connor4312]

This drops headers with no auth scheme entirely. For resource_metadata="/m", scope="read", the first part is parsed as a parameter, challenges is still empty, and everything after it is skipped. The old substring parser handled this, and so did the 403 path (error="insufficient_scope" with no scheme now becomes AuthorizationFailed instead of InsufficientScope).

Those headers don't follow RFC 9110, but some servers send them. Could we treat leading parameters as an implicit Bearer challenge?

[connor4312]

+1 to DaleSeo's note: with DPoP resource_metadata="/m", Bearer scope="read" the Bearer scope hint is lost. Per RFC 9728, resource_metadata points at metadata for the whole protected resource, not one scheme, so borrowing just the pointer for the selected Bearer challenge doesn't mix unrelated challenge parameters. Something like:

let foreign_url = other_metadata.and_then(|p| p.resource_metadata_url);
scope_challenge
    .or(first_challenge)
    .map(|mut p| {
        if p.resource_metadata_url.is_none() {
            p.resource_metadata_url = foreign_url.clone();
        }
        p
    })
    .or_else(|| foreign_url.map(|url| WWWAuthenticateParams {
        resource_metadata_url: Some(url),
        ..Default::default()
    }))

The foreign_metadata_first and foreign_metadata_fallback cases would need updating to match.

[connor4312]

Nit, non-blocking: the splitter treats any " as a quote toggle, even in the middle of an unquoted value, while parse_next_header_value only treats a leading " as quoted. A malformed value like realm=x"y, scope="read" would make the splitter misread everything up to the next quote. The old code was no more robust, so this isn't a regression. Still, a single small tokenizer that reads scheme / name=value / token68 and returns the challenges might be easier to follow than a splitter whose closure tracks state plus a second value parser.

[connor4312]

HeaderValue::to_str() only accepts visible ASCII, but RFC 9110 still allows obs-text (0x80–0xFF) inside quoted strings, and http/hyper will store those bytes in a HeaderValue. So a header like Basic realm="Bücher", Bearer resource_metadata="/m" is valid, but this line drops it silently. When everything comes in one combined field, a non-ASCII realm on another scheme also wipes out the Bearer challenge.

This isn't new in this PR, but since the goal here is to survive mixed, combined headers, it might be worth parsing String::from_utf8_lossy(value.as_bytes()) instead. Every character the parser splits on is ASCII and non-ASCII can only appear inside quoted strings, so U+FFFD replacements can't change the parse. The same applies at the 403 path above.

[connor4312]

One thing of note is that the transport paths don't get the upgraded logic we have here yet. So two things this PR fixes still exist in those paths:

1. Only the first WWW-Authenticate field is read. The reqwest and unix-socket transports (common/reqwest/streamable_http_client.rs, common/unix_socket.rs) use headers().get(WWW_AUTHENTICATE), and AuthRequiredError / InsufficientScopeError carry a single String. So in the reactive resolve_metadata_from_challenge flow, WWW-Authenticate: DPoP … followed by a separate WWW-Authenticate: Bearer resource_metadata=… hands the parser only the DPoP field. The new repeated-field tests go through AuthorizationManager's own get_all paths, so they don't cover this one.
2. extract_scope_from_header (common/http_header.rs) still has the substring bug. It sets InsufficientScopeError.required_scope on 403s, so DPoP scope="dpop:read", Bearer error="insufficient_scope", scope="read" still yields dpop:read. It also matches foo_scope= and stops at the first ", which breaks escaped quotes.

The fix for both is probably the same centralization: have the transports join every WWW-Authenticate field with ", " (RFC 9110 allows combining them that way, and the new parser handles the combined form), and derive required_scope from WWWAuthenticateParams::parse instead of the separate helper.