From 3bc2afadfcd8ab52ba69c87c640f0f54ae50e45a Mon Sep 17 00:00:00 2001 From: Roberto Iskandarani Date: Tue, 28 Jul 2026 16:58:10 -0300 Subject: [PATCH] fix(sdk): stop rewriting issuer and resource identifiers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 trailing slashes in four places: - AuthplaneClient.create rewrote the configured issuer. The rewritten value became the expected `iss` at token verification, so an AS whose issuer identifier legitimately ends in "/" had every token rejected (RFC 9068 requires `iss` to carry the slash). - The PRM document URL/path helpers stripped the resource path's trailing slash, serving /mcp where RFC 9728 §3 requires /mcp/. - buildMetadataUrl stripped the issuer path's trailing slash against the RFC 8414 §3 insertion rule. - MetadataCache stripped 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) and never transformed; derivation is pure string insertion via a shared helper, since WHATWG URL serialises an empty path as "/" and cannot preserve the identifier exactly. Conformance: extends the rfc9728 well-known-path case with the trailing-slash resource datum and adds the two issuer variants (metadata issuer differing only by a trailing slash is rejected; a token whose iss matches a trailing-slash issuer verifies end to end). 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. --- CHANGELOG.md | 3 +- packages/sdk/conformance-tests/helpers.ts | 21 +++-- .../test_jwt_and_dpop_conformance.test.ts | 31 +++++++ .../test_rfc8414_conformance.test.ts | 23 +++++ packages/sdk/src/core/client.ts | 6 +- .../sdk/src/core/fetching/documentCache.ts | 8 +- packages/sdk/src/core/fetching/metadataUrl.ts | 29 ++++--- packages/sdk/src/core/identifiers.ts | 85 +++++++++++++++++++ packages/sdk/src/core/prm.ts | 36 +++----- packages/sdk/src/core/resource.ts | 6 +- .../sdk/tests/core/prmDocumentUrl.test.ts | 16 ++-- 11 files changed, 207 insertions(+), 57 deletions(-) create mode 100644 packages/sdk/src/core/identifiers.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index de62178..dc21c76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,12 +16,13 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht ### Changed -- `@authplane/sdk` — `oauthProtectedResourceMetadataDocumentUrl(resource)` now normalises trailing slashes (RFC 9728 §3.1) and throws a typed `TypeError` on invalid URLs. **Migration**: drop any deliberate trailing slash from `resource` before upgrading — the canonical form is no-trailing-slash. +- `@authplane/sdk` — resource and issuer identifiers are never rewritten (RFC 8414 §3.3 / RFC 9728 §3.3 require the advertised value to be *identical* to the configured one). `oauthProtectedResourceMetadataDocumentUrl` / `oauthProtectedResourceMetadataPath` and the RFC 8414 metadata URL are formed by pure insertion, preserving the identifier's path exactly — including any trailing slash — and AS-metadata issuer comparison is now an exact string match. Identifiers are validated at construction (absolute http(s) URL with an authority and no fragment) and throw a typed `TypeError` otherwise; trailing slashes, host case, and explicit ports are legal and preserved. **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. - `@authplane/mcp` — now accepts a DPoP-bound token presented under the `DPoP` scheme (RFC 9449 §7.1) instead of rejecting it with 401. - `@authplane/fastmcp` — stricter RFC 6750 §2.1 `Authorization` parsing, consistent with the other adapters. ### Fixed +- `@authplane/sdk` — a configured issuer whose identifier legitimately ends in `/` no longer has every token rejected. The trailing slash was silently stripped at client creation and the stripped value compared against the token's `iss`, which RFC 9068 requires to carry the slash; discovery now also resolves the RFC 8414 well-known URL for such issuers correctly. - `@authplane/mcp` — a non-URL `aud` claim now returns 401 `invalid_token` (RFC 8707) instead of a 500. ## [0.2.0] - 2026-05-22 diff --git a/packages/sdk/conformance-tests/helpers.ts b/packages/sdk/conformance-tests/helpers.ts index 0fe79fb..5df1863 100644 --- a/packages/sdk/conformance-tests/helpers.ts +++ b/packages/sdk/conformance-tests/helpers.ts @@ -108,6 +108,16 @@ export interface MockAsServerHandle { export interface MockAsServerOptions { keypair: Es256Keypair; metadataOverrides?: Record; + /** + * Suffix appended to the server origin to form the advertised issuer + * (e.g. "/" to simulate an AS whose issuer identifier ends in a slash). + */ + issuerSuffix?: string; + /** + * Path the metadata document is served at. Defaults to the RFC 8414 + * well-known path for an issuer with no path component. + */ + metadataPath?: string; includeTokenEndpoint?: boolean; includeIntrospectionEndpoint?: boolean; includeRevocationEndpoint?: boolean; @@ -162,7 +172,7 @@ export async function createMockAsServer( const origin = `http://127.0.0.1:${addr.port}`; const baseMetadata: Record = { - issuer: origin, + issuer: `${origin}${options.issuerSuffix ?? ""}`, jwks_uri: `${origin}/.well-known/jwks.json`, }; if (options.includeTokenEndpoint !== false) { @@ -179,7 +189,9 @@ export async function createMockAsServer( ...(options.metadataOverrides ?? {}), }; - const metadataUrl = `${origin}/.well-known/oauth-authorization-server`; + const metadataPath = + options.metadataPath ?? "/.well-known/oauth-authorization-server"; + const metadataUrl = `${origin}${metadataPath}`; const jwksUrl = typeof metadata.jwks_uri === "string" ? metadata.jwks_uri @@ -202,10 +214,7 @@ export async function createMockAsServer( server.on("request", async (req, res) => { try { const url = req.url ?? ""; - if ( - req.method === "GET" && - url === "/.well-known/oauth-authorization-server" - ) { + if (req.method === "GET" && url === metadataPath) { sendJson(res, 200, metadata); return; } diff --git a/packages/sdk/conformance-tests/test_jwt_and_dpop_conformance.test.ts b/packages/sdk/conformance-tests/test_jwt_and_dpop_conformance.test.ts index 8571e0d..2a59174 100644 --- a/packages/sdk/conformance-tests/test_jwt_and_dpop_conformance.test.ts +++ b/packages/sdk/conformance-tests/test_jwt_and_dpop_conformance.test.ts @@ -93,6 +93,37 @@ conformanceCase( } finally { await fixture.close(); } + + // Variant: a token whose `iss` is identical to a configured + // trailing-slash issuer MUST verify — the issuer is never rewritten, + // end to end: RFC 8414 metadata is discovered at the trailing-slash + // well-known URL, the advertised issuer matches exactly, and the + // token's `iss` matches exactly. + const keypair = await generateEs256Keypair(); + const server = await createMockAsServer({ + keypair, + issuerSuffix: "/", + metadataPath: "/.well-known/oauth-authorization-server/", + }); + try { + const client = await AuthplaneClient.create({ + issuer: `${server.origin}/`, + fetchSettings: NO_SSRF, + }); + const resource = client.resource({ + resource: `${server.origin}/api`, + scopes: ["read:data"], + }); + const tokenFactory = createTokenFactory(keypair); + const token = await tokenFactory({ + iss: `${server.origin}/`, + aud: `${server.origin}/api`, + }); + const claims = await resource.verify(token); + expect(claims.sub).toBe("user123"); + } finally { + await server.close(); + } }, ); diff --git a/packages/sdk/conformance-tests/test_rfc8414_conformance.test.ts b/packages/sdk/conformance-tests/test_rfc8414_conformance.test.ts index 338a6cb..3fce709 100644 --- a/packages/sdk/conformance-tests/test_rfc8414_conformance.test.ts +++ b/packages/sdk/conformance-tests/test_rfc8414_conformance.test.ts @@ -45,6 +45,25 @@ conformanceCase( } finally { await server.close(); } + + // Variant: RFC 8414 §3.3 requires the returned issuer to be identical + // to the configured one — a trailing-slash difference is equivalent + // per RFC 3986 §6.2.3 but not identical, so it MUST be rejected. + const slashKeypair = await generateEs256Keypair(); + const slashServer = await createMockAsServer({ + keypair: slashKeypair, + issuerSuffix: "/", + }); + try { + await expect( + AuthplaneClient.create({ + issuer: slashServer.origin, + fetchSettings: NO_SSRF, + }), + ).rejects.toThrow(/issuer mismatch/); + } finally { + await slashServer.close(); + } }, ); @@ -359,6 +378,10 @@ conformanceCase( "https://api.example.com/v2/mcp", "/.well-known/oauth-protected-resource/v2/mcp", ], + [ + "https://api.example.com/mcp/", + "/.well-known/oauth-protected-resource/mcp/", + ], ]; for (const [resource, expectedPath] of cases) { const url = oauthProtectedResourceMetadataDocumentUrl(resource); diff --git a/packages/sdk/src/core/client.ts b/packages/sdk/src/core/client.ts index d782987..d30a106 100644 --- a/packages/sdk/src/core/client.ts +++ b/packages/sdk/src/core/client.ts @@ -16,6 +16,7 @@ import { CircuitBreaker } from "./circuitBreaker.js"; import { shouldTripCircuit } from "./circuitPolicy.js"; import type { ASCredentials } from "./credentials.js"; import type { DPoPProvider } from "./dpop.js"; +import { assertHttpIdentifier } from "./identifiers.js"; import { JWKSCache, type JwksDocument, @@ -83,7 +84,10 @@ export class AuthplaneClient { dpopProvider?: DPoPProvider | undefined; }): Promise { const client = new AuthplaneClient(); - client.issuer = options.issuer.replace(/\/+$/g, ""); + // RFC 8414 §3.3 — the issuer is an opaque identifier compared with + // simple string equality (against metadata and token `iss`); it is + // validated but never rewritten. + client.issuer = assertHttpIdentifier(options.issuer, "issuer"); client.authProvider = toAuthProvider(options.auth); const resolvedDevMode = options.devMode ?? false; diff --git a/packages/sdk/src/core/fetching/documentCache.ts b/packages/sdk/src/core/fetching/documentCache.ts index 327d574..cd057f7 100644 --- a/packages/sdk/src/core/fetching/documentCache.ts +++ b/packages/sdk/src/core/fetching/documentCache.ts @@ -243,7 +243,7 @@ export class MetadataCache extends DocumentCache> { ...config, }); - this.expectedIssuer = (options.expectedIssuer ?? "").replace(/\/+$/g, ""); + this.expectedIssuer = options.expectedIssuer ?? ""; this.allowHttp = options.allowHttp ?? false; } @@ -271,14 +271,14 @@ export class MetadataCache extends DocumentCache> { private validateMetadata( metadata: Record, ): Record { - const rawIssuer = - typeof metadata.issuer === "string" ? metadata.issuer : ""; - const issuer = rawIssuer.replace(/\/+$/g, ""); + const issuer = typeof metadata.issuer === "string" ? metadata.issuer : ""; if (!issuer) { throw new MetadataFetchError( "AS metadata missing required 'issuer' field.", ); } + // RFC 8414 §3.3 — the returned issuer MUST be identical to the + // configured one; simple string comparison, no normalisation. if (this.expectedIssuer && issuer !== this.expectedIssuer) { throw new MetadataFetchError( `AS metadata issuer mismatch: expected '${this.expectedIssuer}', got '${issuer}'.`, diff --git a/packages/sdk/src/core/fetching/metadataUrl.ts b/packages/sdk/src/core/fetching/metadataUrl.ts index af89a61..4f30e2b 100644 --- a/packages/sdk/src/core/fetching/metadataUrl.ts +++ b/packages/sdk/src/core/fetching/metadataUrl.ts @@ -1,15 +1,18 @@ -/** Build RFC 8414 metadata URL from issuer. */ -export function buildMetadataUrl(issuer: string): string { - const parsed = new URL(issuer); - const path = parsed.pathname.replace(/^\/+|\/+$/g, ""); - - if (path) { - parsed.pathname = `/.well-known/oauth-authorization-server/${path}`; - } else { - parsed.pathname = "/.well-known/oauth-authorization-server"; - } - parsed.search = ""; - parsed.hash = ""; +import { + assertHttpIdentifier, + splitHttpIdentifier, +} from "../identifiers.js"; - return parsed.toString(); +/** + * Build the RFC 8414 metadata URL from the issuer identifier. + * + * RFC 8414 §3 forms the URL by inserting `/.well-known/oauth-authorization-server` + * between the authority and path of the issuer — a pure string insertion that + * preserves the issuer's path exactly (including any trailing slash). + */ +export function buildMetadataUrl(issuer: string): string { + const { base, path } = splitHttpIdentifier( + assertHttpIdentifier(issuer, "issuer"), + ); + return `${base}/.well-known/oauth-authorization-server${path}`; } diff --git a/packages/sdk/src/core/identifiers.ts b/packages/sdk/src/core/identifiers.ts new file mode 100644 index 0000000..586f289 --- /dev/null +++ b/packages/sdk/src/core/identifiers.ts @@ -0,0 +1,85 @@ +/** + * Shared handling for resource and issuer identifiers. + * + * 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, not RFC 3986 + * equivalence. The SDK therefore never rewrites an identifier: well-known URLs + * are formed by inserting the well-known path segment between the authority and + * the identifier's path (a pure string insertion, RFC 8414 §3 / RFC 9728 §3), + * and validation rejects structurally unusable identifiers instead of + * repairing them. + */ + +/** + * Validate that `value` is an absolute http(s) URL with an authority and no + * fragment (RFC 8707 §2 forbids fragments in resource identifiers). Returns + * `value` unchanged — trailing slashes, host case, and explicit ports are all + * legal identifier variations and are preserved verbatim. + * + * @throws TypeError when the identifier is structurally invalid. + */ +export function assertHttpIdentifier(value: string, label: string): string { + let parsed: URL; + try { + parsed = new URL(value); + } catch (cause) { + throw new TypeError( + `${label} is not a valid URL (got ${JSON.stringify(value)})`, + { cause }, + ); + } + if (parsed.protocol !== "https:" && parsed.protocol !== "http:") { + throw new TypeError( + `${label} must be an http or https URL (got ${JSON.stringify(value)})`, + ); + } + // WHATWG accepts scheme-relative forms like "https:example.com"; the RFC + // 3986 absolute-URI form used for identifiers requires an explicit + // authority, and splitHttpIdentifier relies on it. + if (!/^[a-zA-Z][a-zA-Z0-9+.-]*:\/\//u.test(value)) { + throw new TypeError( + `${label} must include an authority ("//") (got ${JSON.stringify(value)})`, + ); + } + if (value.includes("#")) { + throw new TypeError( + `${label} must not contain a fragment (got ${JSON.stringify(value)})`, + ); + } + return value; +} + +/** + * Split a validated absolute http(s) identifier into the part before the path + * (scheme + authority, verbatim) and its path component (verbatim). Query and + * fragment are excluded from both. + * + * WHATWG `URL` cannot be used for this: it serialises an empty path as "/" and + * rewrites path bytes, either of which would break the RFC 8414 §3 / + * RFC 9728 §3 insertion rule that the derived well-known URL preserves the + * identifier exactly. + */ +export function splitHttpIdentifier(identifier: string): { + base: string; + path: string; +} { + const authorityStart = identifier.indexOf("://") + 3; + const rest = identifier.slice(authorityStart); + const authorityEnd = rest.search(/[/?#]/u); + if (authorityEnd === -1) { + return { base: identifier, path: "" }; + } + const base = identifier.slice(0, authorityStart + authorityEnd); + if (rest[authorityEnd] !== "/") { + return { base, path: "" }; + } + const pathAndAfter = rest.slice(authorityEnd); + const queryOrFragment = pathAndAfter.search(/[?#]/u); + return { + base, + path: + queryOrFragment === -1 + ? pathAndAfter + : pathAndAfter.slice(0, queryOrFragment), + }; +} diff --git a/packages/sdk/src/core/prm.ts b/packages/sdk/src/core/prm.ts index 1ce4f4d..bc68850 100644 --- a/packages/sdk/src/core/prm.ts +++ b/packages/sdk/src/core/prm.ts @@ -1,4 +1,5 @@ import { ALLOWED_ALGORITHMS } from "./constants.js"; +import { assertHttpIdentifier, splitHttpIdentifier } from "./identifiers.js"; export interface ProtectedResourceMetadata { resource: string; @@ -70,34 +71,21 @@ export function buildPrm( return doc; } -function parseResourceUrl(resource: string): URL { - try { - return new URL(resource); - } catch (cause) { - throw new TypeError( - `resource is not a valid URL (got ${JSON.stringify(resource)})`, - { cause }, - ); - } -} - -function resourceMetadataSuffix(parsed: URL): string { - return parsed.pathname.replace(/\/+$/u, ""); -} - /** * RFC 9728 §3.1 — absolute URL of the Protected Resource Metadata document for `resource`. * - * Path template: `/.well-known/oauth-protected-resource{resource-path}`. - * Trailing slashes on the resource path are dropped so - * `https://api.example.com/mcp/` and `https://api.example.com/mcp` yield the - * same document URL. + * Formed by inserting `/.well-known/oauth-protected-resource` between the + * authority and path of the resource identifier (RFC 9728 §3) — a pure string + * insertion that preserves the path exactly, so `https://api.example.com/mcp/` + * and `https://api.example.com/mcp` yield distinct document URLs. */ export function oauthProtectedResourceMetadataDocumentUrl( resource: string, ): string { - const parsed = parseResourceUrl(resource); - return `${parsed.origin}/.well-known/oauth-protected-resource${resourceMetadataSuffix(parsed)}`; + const { base, path } = splitHttpIdentifier( + assertHttpIdentifier(resource, "resource"), + ); + return `${base}/.well-known/oauth-protected-resource${path}`; } /** @@ -109,6 +97,8 @@ export function oauthProtectedResourceMetadataDocumentUrl( * @throws TypeError when `resource` is not a valid absolute URL. */ export function oauthProtectedResourceMetadataPath(resource: string): string { - const parsed = parseResourceUrl(resource); - return `/.well-known/oauth-protected-resource${resourceMetadataSuffix(parsed)}`; + const { path } = splitHttpIdentifier( + assertHttpIdentifier(resource, "resource"), + ); + return `/.well-known/oauth-protected-resource${path}`; } diff --git a/packages/sdk/src/core/resource.ts b/packages/sdk/src/core/resource.ts index c0c3a4b..f54b908 100644 --- a/packages/sdk/src/core/resource.ts +++ b/packages/sdk/src/core/resource.ts @@ -10,6 +10,7 @@ import { } from "../auth/introspection.js"; import { VerifiedClaims } from "./claims.js"; import { ALLOWED_ALGORITHMS, CLOCK_SKEW_SECONDS } from "./constants.js"; +import { assertHttpIdentifier } from "./identifiers.js"; import type { ASCredentials } from "./credentials.js"; import { type DPoPAlgorithm, @@ -178,7 +179,10 @@ export class AuthplaneResource { } this.issuer = options.issuer; - this.resource = options.resource; + // RFC 8707 §2 — the resource identifier is opaque; validated for + // structure, never rewritten (it is compared verbatim against `aud` + // and advertised verbatim in PRM). + this.resource = assertHttpIdentifier(options.resource, "resource"); this.scopes = Object.freeze([...options.scopes]); this.allowedAlgorithms = Object.freeze(allowedAlgorithms); this.clockSkewSeconds = options.clockSkewSeconds ?? CLOCK_SKEW_SECONDS; diff --git a/packages/sdk/tests/core/prmDocumentUrl.test.ts b/packages/sdk/tests/core/prmDocumentUrl.test.ts index b1bb4eb..64fa8f2 100644 --- a/packages/sdk/tests/core/prmDocumentUrl.test.ts +++ b/packages/sdk/tests/core/prmDocumentUrl.test.ts @@ -13,10 +13,10 @@ describe("oauthProtectedResourceMetadataDocumentUrl (RFC 9728 §3.1)", () => { ); }); - it("uses empty suffix when resource path is /", () => { + it("preserves a bare / resource path (RFC 9728 §3 insertion)", () => { expect( oauthProtectedResourceMetadataDocumentUrl("https://rs.example.com/"), - ).toBe("https://rs.example.com/.well-known/oauth-protected-resource"); + ).toBe("https://rs.example.com/.well-known/oauth-protected-resource/"); }); it("preserves nested resource paths", () => { @@ -29,11 +29,11 @@ describe("oauthProtectedResourceMetadataDocumentUrl (RFC 9728 §3.1)", () => { ); }); - it("strips trailing slashes on the resource path", () => { + it("preserves a trailing slash on the resource path", () => { expect( oauthProtectedResourceMetadataDocumentUrl("https://rs.example.com/mcp/"), ).toBe( - "https://rs.example.com/.well-known/oauth-protected-resource/mcp", + "https://rs.example.com/.well-known/oauth-protected-resource/mcp/", ); }); @@ -51,9 +51,9 @@ describe("oauthProtectedResourceMetadataPath (RFC 9728 §3.1, path only)", () => ); }); - it("returns the bare .well-known path when the resource path is /", () => { + it("preserves a bare / resource path (RFC 9728 §3 insertion)", () => { expect(oauthProtectedResourceMetadataPath("https://rs.example.com/")).toBe( - "/.well-known/oauth-protected-resource", + "/.well-known/oauth-protected-resource/", ); }); @@ -63,10 +63,10 @@ describe("oauthProtectedResourceMetadataPath (RFC 9728 §3.1, path only)", () => ).toBe("/.well-known/oauth-protected-resource/mcp"); }); - it("strips a trailing slash on the resource path", () => { + it("preserves a trailing slash on the resource path", () => { expect( oauthProtectedResourceMetadataPath("https://rs.example.com/mcp/"), - ).toBe("/.well-known/oauth-protected-resource/mcp"); + ).toBe("/.well-known/oauth-protected-resource/mcp/"); }); it("preserves nested paths", () => {