Skip to content

fix(resource,metadata,verifier): preserve issuer identity, derive-only slash handling, escaped-path PRM - #23

Open
RobertoIskandarani wants to merge 1 commit into
mainfrom
fix/issuer-identity-derivation
Open

fix(resource,metadata,verifier): preserve issuer identity, derive-only slash handling, escaped-path PRM#23
RobertoIskandarani wants to merge 1 commit into
mainfrom
fix/issuer-identity-derivation

Conversation

@RobertoIskandarani

Copy link
Copy Markdown

What

Replacement for #22, reworked against the corrected scope: identity is preserved verbatim (RFC 8414/9728 §3.3), derivation strips the terminating slash (§3.1) — the two concerns the previous PR conflated.

  • core/authplane: NewClient rejects an issuer carrying a query or fragment component (RFC 8414 §2 forbids both). Previously the RFC 8414 discovery-URL builder silently dropped them while the OIDC builder carried them along — two discovery attempts targeting different identities.
  • core/internal/metadata: the RFC 8414 §3.3 issuer check compares configured and document issuer byte-for-byte (§4: code-point-for-code-point, no normalization) instead of trailing-slash-insensitively. Surfaces at NewClient as metadata: issuer mismatch since discovery is eager.
  • core/resource/verifier: the verifier stores the configured issuer verbatim and matches a token's iss byte-for-byte; a trailing-slash mismatch is now ErrIssuerMismatch.
  • core/resource: resource.New rejects fragment-bearing resource URIs (RFC 8707 §2). The RFC 9728 §3.1 PRM well-known URL strips a terminating slash at derivation only (/mcp//.well-known/oauth-protected-resource/mcp); the identifier itself is untouched. WellKnownPRMPath()/PRMURL() derive from the escaped path so a percent-encoded octet (e.g. %2F) survives verbatim (RFC 3986 §3.3).
  • http: the PRM discovery bypass compares EscapedPath() against the escaped well-known path, keeping the discovery endpoint publicly reachable (RFC 9728 §3.2) for percent-encoded resource identifiers.
  • ci: the conformance catalog is pinned to a fixed revision, single-sourced in .conformance-catalog-ref (40-hex guarded) so ci.yml and release.yml cannot disagree, with a weekly drift workflow against the catalog tip. The pin ships in this PR because the pinned catalog revision carries the identity/derivation cases this change covers — either half alone would fail the alignment check.

Breaking

All identifier-comparison changes are breaking for configs that relied on silent trailing-slash reconciliation, and construction now fails fast on query/fragment-bearing issuers and fragment-bearing resources. Migration notes for each are in the CHANGELOG's Unreleased section.

Verification

  • go vet, build, golangci-lint (CI-pinned version): clean across all four modules.
  • Full test suites incl. -race: green (core 89.6%, mcp 94.2%, http 96.1%, mark3labs 85.7% coverage).
  • Conformance vs the pinned catalog: 134/134 incl. the catalog-alignment meta-check.
  • Manual E2E against a live authserver (develop), both adapters: PRM served verbatim at the slash-stripped well-known path, RFC 9728 challenge on 401, authenticated MCP initialize + tools/call, insufficient-scope and wrong-audience rejections.

…y slash handling, escaped-path PRM

- core/authplane: NewClient rejects an issuer carrying a query or
  fragment component (RFC 8414 §2) instead of letting the two
  discovery-URL builders diverge on it.
- core/internal/metadata: the RFC 8414 §3.3 issuer check compares
  configured and document issuer byte-for-byte (§4, no normalization)
  instead of trailing-slash-insensitively.
- core/resource/verifier: the token verifier stores the configured
  issuer verbatim and matches iss byte-for-byte; a trailing-slash
  mismatch is now ErrIssuerMismatch.
- core/resource: resource.New rejects fragment-bearing resource URIs
  (RFC 8707 §2); the RFC 9728 §3.1 PRM well-known URL strips a
  terminating slash at derivation only, and WellKnownPRMPath()/PRMURL()
  derive from the escaped path so percent-encoded octets survive
  verbatim (RFC 3986 §3.3).
- http: the PRM discovery bypass compares EscapedPath() against the
  escaped well-known path, so percent-encoded resource identifiers keep
  the discovery endpoint publicly reachable (RFC 9728 §3.2).
- ci: single-source the conformance catalog pin in
  .conformance-catalog-ref (40-hex guarded) with a scheduled drift
  check against the catalog tip.
@RobertoIskandarani
RobertoIskandarani requested a review from a team as a code owner August 5, 2026 11:46

@muralx muralx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants