modelcontextprotocol / modelcontextprotocol/rust-sdk

auth: report protected resource metadata discovery failures as structured AuthError variants

Open
#1,265 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement P2 ready for work T-enhancement T-security T-transport
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 MetadataError stops 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.