Skip to content

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

Description

@DaleSeo

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).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Medium: important but non-blocking improvementT-enhancementNew features and enhancementsT-securitySecurity-related changesT-transportTransport layer changesenhancementNew feature or requestready for workIssue is well-defined and ready to be picked up

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions