fix(resource,metadata,verifier): preserve issuer identity, derive-only slash handling, escaped-path PRM - #23
Open
RobertoIskandarani wants to merge 1 commit into
Open
Conversation
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:NewClientrejects 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 atNewClientasmetadata: issuer mismatchsince discovery is eager.core/resource/verifier: the verifier stores the configured issuer verbatim and matches a token'sissbyte-for-byte; a trailing-slash mismatch is nowErrIssuerMismatch.core/resource:resource.Newrejects 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 comparesEscapedPath()against the escaped well-known path, keeping the discovery endpoint publicly reachable (RFC 9728 §3.2) for percent-encoded resource identifiers..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.-race: green (core 89.6%, mcp 94.2%, http 96.1%, mark3labs 85.7% coverage).develop), both adapters: PRM served verbatim at the slash-stripped well-known path, RFC 9728 challenge on 401, authenticated MCPinitialize+tools/call, insufficient-scope and wrong-audience rejections.