fix: stop normalising resource and issuer identifiers - #22
Closed
RobertoIskandarani wants to merge 2 commits into
Closed
fix: stop normalising resource and issuer identifiers#22RobertoIskandarani wants to merge 2 commits into
RobertoIskandarani wants to merge 2 commits into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
RFC 8414 §3.3 and RFC 9728 §3.3 require the advertised issuer/resource
to be identical to the configured value — a simple string comparison —
and both well-known URLs are formed by inserting the well-known path
segment into the identifier verbatim (RFC 8414 §3 / RFC 9728 §3). The
SDK instead stripped slashes in five places:
- AuthplaneClient.create rewrote the configured issuer with rstrip("/").
The rewritten value became the verifier's expected iss claim, so an
AS whose issuer identifier legitimately ends in "/" had every token
rejected (RFC 9068 requires iss to carry the slash).
- build_prm_url and build_metadata_url used path.strip("/"), dropping
the trailing slash the insertion rule requires be preserved; both are
now pure insertion of the parsed path.
- MetadataCache rstripped both sides of the issuer comparison,
weakening the §3.3 identical-match MUST that defeats metadata
substitution.
Identifiers are now validated at construction instead (absolute
http(s) URL with an authority, no fragment — RFC 8707 §2) via the new
internal validate_identifier helper, and never transformed.
Conformance: extends the rfc9728 well-known-path case with the
trailing-slash resource datum and adds two issuer variants — metadata
issuer differing only by a trailing slash is rejected, and a token
whose iss matches a configured trailing-slash issuer verifies end to
end (discovery at the trailing-slash well-known URL included).
Migration: if a configured issuer or resource differs from the
authorization server's actual identifier by a trailing slash, correct
the config — the SDK no longer silently reconciles them.
RobertoIskandarani
force-pushed
the
fix/identifier-identity-trailing-slash
branch
from
8f7b224 to
245d31a
Compare
July 29, 2026 14:09
ruff's newer formatter changed how it renders code blocks embedded in Markdown; CI installs the latest ruff, so the format check fails on files this branch does not otherwise touch. Formatting-only, no content changes. Expected to become a no-op once the in-flight change that already carries this reformatting lands on main.
Copy link
Copy Markdown
Contributor
Author
|
Closing — superseded. The identifier-handling change is being reworked against the corrected RFC 8414 §3.1 / RFC 9728 §3.1 (derivation strips the terminating slash) vs §3.3 (identity preserved verbatim) scope. A replacement PR will follow. |
Sorry, something went wrong.
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
RFC 8414 §3.3 and RFC 9728 §3.3 require the advertised issuer/resource to be identical to the configured value — a simple string comparison that exists to defeat metadata substitution — and both well-known URLs are formed by inserting the well-known path segment into the identifier verbatim (RFC 8414 §3 / RFC 9728 §3). The SDK stripped slashes in five places; this PR removes all five and replaces silent rewriting with construction-time validation.
The user-visible bug
AuthplaneClient.create rewrote the configured issuer (issuer.rstrip("/")), and that value is what the verifier uses as the expected iss claim. For an authorization server whose issuer identifier legitimately ends in /, RFC 9068 requires the token's iss to carry the trailing slash — so every token was rejected for such a deployment. Verified by reintroducing the strip: the new end-to-end conformance variant fails (discovery resolves the wrong well-known URL), and passes with the fix.
The five sites
Validation instead of repair
New authplane/internal/identifiers.py: identifiers must be absolute http(s) URLs with an authority and no fragment (RFC 8707 §2 forbids fragments). Trailing slashes, host case, and explicit ports are legal variations and preserved verbatim. Applied at AuthplaneClient.create (issuer) and AuthplaneResource (resource).
Conformance
Migration
If your configured issuer or resource differs from your authorization server's actual identifier by a trailing slash, correct the config — the SDK no longer silently reconciles them. (CHANGELOG entry included.)
Validation
520 unit + 104 conformance tests green (against the amended catalog), ruff lint and format clean.