fix(core,mcp,spring): preserve issuer and resource identity, strip the slash only at derivation - #19
Open
RobertoIskandarani wants to merge 2 commits into
Open
fix(core,mcp,spring): preserve issuer and resource identity, strip the slash only at derivation#19RobertoIskandarani wants to merge 2 commits into
RobertoIskandarani wants to merge 2 commits into
Conversation
…e slash only at derivation Identifiers are identity, not something to normalise. The SDK reconciled them in several places, and each of those was a silent rewrite of a value the operator configured. Identity is now preserved verbatim (RFC 8414/9728 §3.3) and slash removal happens only at derivation (§3.1) — the two concerns the previous code conflated: - The configured issuer is stored byte-for-byte and the AS-metadata issuer comparison no longer strips either side. RFC 8414 §4 specifies that comparison as code-point-for-code-point with no normalisation, and the strict form is what turns an unavoidable derivation collision into a clean discovery failure rather than a silent bind to a different issuer's metadata. - The Protected Resource Metadata path drops the terminating slash of the resource path, so a resource configured as /mcp/ is served at, and derived by a conformant client as, .../oauth-protected-resource/mcp. The identifier itself is unchanged; only the derived publication URL loses the slash. Both halves matter together: preserving identity without fixing derivation publishes a document no conformant client will request, and fixing derivation without preserving identity keeps rewriting the operator's configuration. Conformance coverage follows the behaviour — the RFC 8414, 9068 and 9728 suites gain the identity and derivation rows the catalog already carries. Verified against the pinned catalog revision: 737 core, 62 mcp and 115 spring tests green, up from 727 core on the base. Also verified to compose with the catalog-alignment assertion that lands separately: the both-directions check passes with this change applied.
Five tests in DocumentCacheTest asserted against wall time, three of them as outright races. get_triggersBackgroundRefreshAt80PercentTtl slept 900 ms into an unsynchronised 1 s TTL, then gave an asynchronous refresh 300 ms to finish before asserting the fetch count — so on a loaded runner it fails with "expected >= 2, was 1", the refresh having been triggered but not completed. Two siblings slept 1500 ms into a 1 s TTL purely to force expiry. DocumentCache now takes a java.time.Clock; the 5-arg public constructor supplies Clock.systemUTC(), so no caller changes. Tests advance the clock instead of sleeping, and the one asynchronous assertion is ordered by joining the refresh rather than by a fixed delay. No public API change and no token-validation path touched. Observed here, not hypothesised: this test failed CI on the port branch that carries none of this code, and passes on a re-run of the same commit. The class now runs in 0.1 s instead of 5.8 s, with no Thread.sleep left in it.
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
1 participant
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
Identifiers are identity, not something to normalise. The SDK reconciled them in
several places, and each was a silent rewrite of a value the operator configured.
Identity is preserved verbatim (RFC 8414/9728 §3.3); slash removal happens only
at derivation (§3.1). Those are the two concerns the previous code conflated.
comparison no longer strips either side. RFC 8414 §4 specifies that comparison
as code-point-for-code-point with no normalisation. Derivation is many-to-one:
…/tenantand…/tenant/share one well-known URL and the RFC provides no wayto host both. The strict comparison is what turns that unavoidable collision
into a clean discovery failure rather than a silent bind to a different issuer's
metadata — the impersonation §3.3 exists to defeat.
a resource configured as
/mcp/is served at, and derived by a conformant clientas,
.../oauth-protected-resource/mcp. Before this the SDK registered a path noconformant client would request. The identifier itself is untouched; only the
derived publication URL loses the slash.
The two halves have to move together. Preserving identity without fixing
derivation publishes a document nobody requests; fixing derivation without
preserving identity keeps rewriting the operator's configuration.
Conformance
The RFC 8414, 9068 and 9728 suites gain the identity and derivation rows the
catalog already carries — no new case ids, so this adopts nothing new from the
catalog.
Verification
Full suite against the pinned catalog revision:
All green,
BUILD SUCCESS.Also verified to compose with the catalog-alignment assertion landing in the
pin PR: applying that assertion on top of this change, the both-directions check
passes. The two are deliberately separate — one is CI mechanics, the other is SDK
behaviour — but they will be on
maintogether, so the combination was checkedrather than assumed.
Scope
Deliberately excludes the conformance catalog pin and its enforcement, which are
their own PR. The file sets are disjoint, so neither blocks the other.