modelcontextprotocol / modelcontextprotocol/rust-sdk
auth: report protected resource metadata discovery failures as structured AuthError variants
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 3.9k
- Forks
- 645
- Avg merge
- 4d 13h
- Merged PRs (30d)
- 36
Description
Problem
AuthorizationManager reports every protected-resource-metadata discovery failure as AuthError::MetadataError(String). A caller that wants to react differently to "the server's metadata is misconfigured" and "the network is down" has to match on the message text.
The authorization-server side already does this properly — AuthorizationServerMismatch { expected_issuer, received_issuer } and AuthorizationServerMissingIssuer { expected_issuer } carry their facts as fields. The protected-resource side does not.
Every MetadataError site in crates/rmcp/src/transport/auth.rs today (seven, all in discovery):
| Site | Message | What it actually is |
|---|---|---|
discovery_failed |
OAuth metadata discovery failed for {url}\n Caused by: … |
transport failure, with a source error chain flattened into the string |
validate_resource_metadata_resource ×4 |
missing required resource field / resource field is not a valid URL / does not permit fragment … RFC 8707 / resource mismatch: reference '…', permitted '…' |
the document is not this resource's metadata |
read_resource_metadata (Advertised) |
the server advertised {url} as protected resource metadata, but the document carries neither \resource` nor an authorization server reference` |
the document is not metadata at all (#1204) |
authorization_metadata_from_resource_metadata |
protected resource metadata at {url} names authorization servers {…}, but none published usable metadata |
the document's authorization servers are unreachable (#1264) |
The transport case is the most costly: discovery_failed takes an OAuthHttpClientError and formats its chain into a String, so std::error::Error::source() is gone by the time the caller sees it.
Proposal
AuthError is #[non_exhaustive], so adding variants is not a breaking change. Something along the lines of:
/// A discovery request could not be completed.
#[error("OAuth metadata discovery failed for {url}")]
DiscoveryRequestFailed {
url: Url,
#[source]
source: OAuthHttpClientError,
},
/// The document at `url` is not this resource's protected resource metadata.
#[error("protected resource metadata at {url} is unusable: {reason}")]
ProtectedResourceMetadataInvalid {
url: Url,
reason: ProtectedResourceMetadataError, // MissingResource | ResourceNotAUrl | ResourceHasFragment | ResourceMismatch { expected, actual } | NotAMetadataDocument
},
/// The document named authorization servers and none of them published usable metadata.
#[error("protected resource metadata at {resource_metadata_url} names authorization servers {}, but none published usable metadata", authorization_servers.join(", "))]
AuthorizationServersUnavailable {
resource_metadata_url: Url,
authorization_servers: Vec<String>,
},
Exact shape open for discussion — the point is that the three kinds of failure become three variants, and the transport error keeps its source() chain.
Scope
- Convert all seven sites in one change, so
MetadataErrorstops being the catch-all for discovery. Leaving it for genuinely unclassified cases is fine. - Tests that assert
error.to_string()keep working if the#[error]messages are kept; tests can additionally match on the variant. - Public API Check in CI should pass (variant addition on a
#[non_exhaustive]enum).
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in crates/rmcp/src/transport/auth.rs by locating AuthError and all seven MetadataError sites in protected-resource-metadata discovery. Trace discovery_failed, validation, advertised metadata, and authorization-server lookup paths, then inspect tests asserting error.to_string(). Done means the three failure categories are structured, the transport source chain is preserved, existing wording remains compatible, and the Public API Check passes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- authentication, backend-api-design
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100