auth: report protected resource metadata discovery failures as structured AuthError variants
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 55/100
- Issue type
- Feature
- Clarity
- Mostly clear
- Activity status
- Active
- Tech stack
- rust
- Domain
- authentication, backend-api-design
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.
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).
- Dominant language
- Rust
- Stars
- 3.9k
- Forks
- 645
- Avg merge
- 4d 13h
- Merged PRs (30d)
- 36
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.
More from modelcontextprotocol/rust-sdk
-
streamable-http server: a supplied Mcp-Method contradicting an initialize body is silently accepted Openbug P1 ready for work T-transport
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
modelcontextprotocol/rust-sdk#1271 ·
-
bug P2 ready for work T-model
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
modelcontextprotocol/rust-sdk#1242 ·
-
P3 question T-documentation T-enhancement
Difficulty 1/5 Under an hour Newbie friendliness 86/100
modelcontextprotocol/rust-sdk#1155 ·
-
bug P1 ready for work T-model T-service
Difficulty 3/5 1-2 days Newbie friendliness 72/100
modelcontextprotocol/rust-sdk#1283 · 1 comment ·
-
bug P0 ready for work T-security T-transport
modelcontextprotocol/rust-sdk#1272 · 1 assignee ·
All issues in modelcontextprotocol/rust-sdk
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
ontola/atomic-server#1625 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
VirusTotal/yara-x#777 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
stratum-mining/stratum#2404 ·
-
bug ci good first issue
Difficulty 2/5 1-3 hours Newbie friendliness 88/100