From 06f0c1a4417f1e9d9d80c9e291db298498d560e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 05:34:37 +0000 Subject: [PATCH 1/7] fix(client): validate discovered AS metadata by document shape, not well-known path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit discoverAuthorizationServerMetadata() picked its validation schema from which well-known filename resolved, so conforming RFC 8414 metadata served at /.well-known/openid-configuration (permitted by RFC 8414 §5) was parsed against the OIDC Discovery schema and rejected for lacking jwks_uri, subject_types_supported and id_token_signing_alg_values_supported — and the parse threw out of the candidate loop, aborting discovery entirely. - Try the schema implied by the path first, then the other one; a document that fits neither skips to the next candidate URL like the existing 4xx / 502 / CORS failures. - Make OpenIdProviderDiscoveryMetadataSchema a loose object like its component schemas so mixed OIDC/OAuth documents keep their RFC 8414 fields (revocation_endpoint, introspection_endpoint). - Correct the discovery doc comment; issuer validation is unchanged. Fixes #2733 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F9hQUruXNrCqCqmMjfMCBZ --- .../oauth-discovery-schema-by-document.md | 6 ++ packages/client/src/client/auth.ts | 31 ++++-- packages/client/test/client/auth.test.ts | 95 ++++++++++++++++++- packages/core/src/auth.ts | 6 +- 4 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 .changeset/oauth-discovery-schema-by-document.md diff --git a/.changeset/oauth-discovery-schema-by-document.md b/.changeset/oauth-discovery-schema-by-document.md new file mode 100644 index 0000000000..f7d9ebb891 --- /dev/null +++ b/.changeset/oauth-discovery-schema-by-document.md @@ -0,0 +1,6 @@ +--- +'@modelcontextprotocol/core': patch +'@modelcontextprotocol/client': patch +--- + +OAuth discovery now validates authorization server metadata by its document shape instead of by the well-known path it was found under: RFC 8414 metadata served at the `openid-configuration` path (permitted by RFC 8414 §5) is accepted, a document that fits neither schema skips to the next candidate URL instead of aborting discovery, and `OpenIdProviderDiscoveryMetadataSchema` is now a loose object so mixed OIDC/OAuth documents keep their RFC 8414 fields (e.g. `revocation_endpoint`, `introspection_endpoint`). diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 7b25c01fe8..92dd9c46f5 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -1803,8 +1803,12 @@ export function buildDiscoveryUrls(authorizationServerUrl: string | URL): { url: * specifications. * * This function implements a fallback strategy for authorization server discovery: - * 1. Attempts RFC 8414 OAuth metadata discovery first - * 2. If OAuth discovery fails, falls back to OpenID Connect Discovery + * 1. Tries RFC 8414 OAuth metadata discovery URLs first, then OpenID Connect Discovery URLs + * 2. Validates each response by its document shape rather than by the well-known path it was + * found under: the schema implied by the path is tried first, then the other one — + * RFC 8414 §5 explicitly permits plain OAuth 2.0 authorization server metadata to be + * served at the `openid-configuration` path. A document that fits neither schema is + * skipped and the next candidate URL is tried. * * @param authorizationServerUrl - The authorization server URL obtained from the MCP Server's * protected resource metadata, or the MCP server's URL if the @@ -1864,11 +1868,24 @@ export async function discoverAuthorizationServerMetadata( ); } - // Parse and validate based on type - const parsed = - type === 'oauth' - ? OAuthMetadataSchema.parse(await response.json()) - : OpenIdProviderDiscoveryMetadataSchema.parse(await response.json()); + // Validate by document shape, preferring the schema implied by the well-known path. + // RFC 8414 §5 explicitly permits plain OAuth 2.0 authorization server metadata to be + // served at the openid-configuration path, so the path alone cannot decide which + // schema the document must satisfy. A document that fits neither schema is treated + // like the other per-candidate failures above (4xx, 502, CORS): try the next + // candidate URL instead of aborting discovery. + const json: unknown = await response.json(); + const primary = type === 'oauth' ? OAuthMetadataSchema.safeParse(json) : OpenIdProviderDiscoveryMetadataSchema.safeParse(json); + let parsed: AuthorizationServerMetadata; + if (primary.success) { + parsed = primary.data; + } else { + const fallback = type === 'oauth' ? OpenIdProviderDiscoveryMetadataSchema.safeParse(json) : OAuthMetadataSchema.safeParse(json); + if (!fallback.success) { + continue; + } + parsed = fallback.data; + } if (!skipIssuerValidation) { // RFC 8414 §3.3 / OIDC Discovery §4.3: the `issuer` value in the document MUST be diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index 7329ded3ed..f61b7c17af 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -1022,9 +1022,9 @@ describe('OAuth Authorization', () => { it('preserves authorization_response_iss_parameter_supported through OIDC discovery parse', async () => { // OAuth well-known 404s; OIDC well-known returns metadata advertising RFC 9207 support. - // Regression-guard: OpenIdProviderDiscoveryMetadataSchema is a plain z.object(), so the - // field must be declared on the underlying schemas or it gets stripped — making the - // RFC 9207 §2.4 advertised-but-missing reject inert on the OIDC-only discovery path. + // Regression-guard: the field must be declared on the underlying schemas so its typed + // value is visible to the RFC 9207 §2.4 advertised-but-missing reject on the OIDC-only + // discovery path. mockFetch.mockResolvedValueOnce({ ok: false, status: 404 }); mockFetch.mockResolvedValueOnce({ ok: true, @@ -1058,6 +1058,95 @@ describe('OAuth Authorization', () => { expect(mockFetch).toHaveBeenCalledTimes(2); }); + it('accepts RFC 8414 OAuth metadata served at the openid-configuration path', async () => { + // A plain OAuth 2.0 AS (no jwks_uri / subject_types_supported / + // id_token_signing_alg_values_supported) publishing RFC 8414 metadata only at the + // OIDC well-known path — explicitly permitted by RFC 8414 §5. + const rfc8414Metadata = { + issuer: 'https://auth.example.com', + authorization_endpoint: 'https://auth.example.com/oauth/authorize', + token_endpoint: 'https://auth.example.com/oauth/token', + response_types_supported: ['code'], + grant_types_supported: ['authorization_code', 'refresh_token'], + code_challenge_methods_supported: ['S256'], + revocation_endpoint: 'https://auth.example.com/oauth/revoke', + introspection_endpoint: 'https://auth.example.com/oauth/introspect' + }; + + mockFetch.mockResolvedValueOnce({ ok: false, status: 404 }); + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => rfc8414Metadata + }); + + const metadata = await discoverAuthorizationServerMetadata('https://auth.example.com'); + + expect(metadata).toEqual(rfc8414Metadata); + }); + + it('preserves RFC 8414 fields on OIDC discovery documents', async () => { + // OIDC providers commonly mix in OAuth 2.0 metadata fields; a successful OIDC parse + // must not strip them (the discovery schema is a loose object). + const mixedMetadata = { + ...validOpenIdMetadata, + revocation_endpoint: 'https://auth.example.com/revoke', + introspection_endpoint: 'https://auth.example.com/introspect' + }; + + mockFetch.mockResolvedValueOnce({ ok: false, status: 404 }); + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => mixedMetadata + }); + + const metadata = await discoverAuthorizationServerMetadata('https://auth.example.com'); + + expect(metadata).toEqual(mixedMetadata); + }); + + it('skips a candidate whose document fits neither schema and tries the next URL', async () => { + const tenantOidcMetadata = { ...validOpenIdMetadata, issuer: 'https://auth.example.com/tenant1' }; + + // First OAuth URL 404s + mockFetch.mockResolvedValueOnce({ ok: false, status: 404 }); + // Second URL (RFC 8414-style OIDC path) returns a document that is not + // authorization server metadata at all + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ hello: 'world' }) + }); + // Third URL (OIDC Discovery 1.0-style path) succeeds + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => tenantOidcMetadata + }); + + const metadata = await discoverAuthorizationServerMetadata('https://auth.example.com/tenant1'); + + expect(metadata).toEqual(tenantOidcMetadata); + expect(mockFetch).toHaveBeenCalledTimes(3); + }); + + it('still validates the issuer on RFC 8414 documents found at the openid-configuration path', async () => { + mockFetch.mockResolvedValueOnce({ ok: false, status: 404 }); + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ + issuer: 'https://evil.example.com', + authorization_endpoint: 'https://auth.example.com/authorize', + token_endpoint: 'https://auth.example.com/token', + response_types_supported: ['code'] + }) + }); + + await expect(discoverAuthorizationServerMetadata('https://auth.example.com')).rejects.toThrow(IssuerMismatchError); + }); + it('throws on non-502 5xx errors', async () => { mockFetch.mockResolvedValueOnce({ ok: false, diff --git a/packages/core/src/auth.ts b/packages/core/src/auth.ts index e21076d817..56fd2e0e4c 100644 --- a/packages/core/src/auth.ts +++ b/packages/core/src/auth.ts @@ -121,8 +121,12 @@ export const OpenIdProviderMetadataSchema = z.looseObject({ * OpenID Connect Discovery metadata that may include OAuth 2.0 fields * This schema represents the real-world scenario where OIDC providers * return a mix of OpenID Connect and OAuth 2.0 metadata fields + * + * Loose like its component schemas so that RFC 8414 fields the OIDC shape + * does not declare (e.g. `revocation_endpoint`, `introspection_endpoint`) + * survive a successful parse instead of being stripped. */ -export const OpenIdProviderDiscoveryMetadataSchema = z.object({ +export const OpenIdProviderDiscoveryMetadataSchema = z.looseObject({ ...OpenIdProviderMetadataSchema.shape, ...OAuthMetadataSchema.pick({ code_challenge_methods_supported: true From 2f71bde2381a3d4e667e1c034a8baae7a4726082 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 05:48:08 +0000 Subject: [PATCH 2/7] =?UTF-8?q?fix:=20address=20review=20round=201=20?= =?UTF-8?q?=E2=80=94=20validate=20preserved=20fields,=20keep=20discovery?= =?UTF-8?q?=20diagnosable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Declare the RFC 8414 revocation/introspection fields on OpenIdProviderDiscoveryMetadataSchema with their OAuth validators so a successful OIDC parse validates them instead of passing them through the loose object's catchall. - On a fallback parse, drop the top-level fields the path-implied schema rejected so values that failed their declared validators (e.g. an unsafe jwks_uri) cannot ride through the fallback schema's passthrough. - Guard response.json() so a 200 with a non-JSON body skips to the next candidate like the other per-candidate failures. - When every candidate fails schema validation, throw an error naming the URL and the schema issues instead of returning undefined and letting auth() silently guess default endpoints. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F9hQUruXNrCqCqmMjfMCBZ --- packages/client/src/client/auth.ts | 47 +++++++++++++++- packages/client/test/client/auth.test.ts | 70 ++++++++++++++++++++++++ packages/core/src/auth.ts | 14 ++++- 3 files changed, 126 insertions(+), 5 deletions(-) diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 92dd9c46f5..58292d8fdc 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -1846,6 +1846,11 @@ export async function discoverAuthorizationServerMetadata( // Get the list of URLs to try const urlsToTry = buildDiscoveryUrls(authorizationServerUrl); + // First candidate whose 200 response was valid JSON but fit neither metadata schema — + // surfaced when every candidate fails, so a near-miss published document stays + // diagnosable instead of silently degrading into default-endpoint guessing. + let schemaFailure: { url: URL; detail: string } | undefined; + // Try each URL in order for (const { url: endpointUrl, type } of urlsToTry) { const response = await fetchWithCorsRetry(endpointUrl, headers, fetchFn); @@ -1874,7 +1879,14 @@ export async function discoverAuthorizationServerMetadata( // schema the document must satisfy. A document that fits neither schema is treated // like the other per-candidate failures above (4xx, 502, CORS): try the next // candidate URL instead of aborting discovery. - const json: unknown = await response.json(); + let json: unknown; + try { + json = await response.json(); + } catch { + // A 200 whose body is not JSON (e.g. an SPA catch-all serving HTML at the + // well-known path) is not authorization server metadata: try the next candidate. + continue; + } const primary = type === 'oauth' ? OAuthMetadataSchema.safeParse(json) : OpenIdProviderDiscoveryMetadataSchema.safeParse(json); let parsed: AuthorizationServerMetadata; if (primary.success) { @@ -1882,9 +1894,27 @@ export async function discoverAuthorizationServerMetadata( } else { const fallback = type === 'oauth' ? OpenIdProviderDiscoveryMetadataSchema.safeParse(json) : OAuthMetadataSchema.safeParse(json); if (!fallback.success) { + schemaFailure ??= { + url: endpointUrl, + detail: `${summarizeMetadataIssues(primary.error.issues)} (fallback schema: ${summarizeMetadataIssues(fallback.error.issues)})` + }; continue; } - parsed = fallback.data; + // The document failed the schema its fields are declared by, so any top-level + // field the primary parse rejected must not ride through the fallback schema's + // looseObject passthrough unvalidated (e.g. an OIDC document with an unsafe + // jwks_uri would otherwise be returned via the OAuth schema, which does not + // declare that field): drop the fields that failed instead. Fields both + // schemas declare validate identically, so this can only remove fields the + // fallback schema does not know about. + const data: Record = { ...fallback.data }; + for (const issue of primary.error.issues) { + const key = issue.path[0]; + if (typeof key === 'string') { + delete data[key]; + } + } + parsed = data as AuthorizationServerMetadata; } if (!skipIssuerValidation) { @@ -1907,9 +1937,22 @@ export async function discoverAuthorizationServerMetadata( return parsed; } + if (schemaFailure) { + throw new Error( + `Authorization server metadata from ${schemaFailure.url} matched neither the OAuth 2.0 (RFC 8414) nor the OpenID Connect Discovery metadata schema: ${schemaFailure.detail}` + ); + } + return undefined; } +/** + * Formats schema issues from a failed authorization server metadata parse for error messages. + */ +function summarizeMetadataIssues(issues: ReadonlyArray<{ path: ReadonlyArray; message: string }>): string { + return issues.map(issue => `${issue.path.map(String).join('.') || '(document)'}: ${issue.message}`).join('; '); +} + /** * Result of {@linkcode discoverOAuthServerInfo}. */ diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index f61b7c17af..169363c416 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -1131,6 +1131,76 @@ describe('OAuth Authorization', () => { expect(mockFetch).toHaveBeenCalledTimes(3); }); + it('skips a candidate whose 200 body is not JSON', async () => { + // e.g. an SPA catch-all serving HTML at the well-known path + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => { + throw new SyntaxError('Unexpected token < in JSON'); + } + }); + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validOpenIdMetadata + }); + + const metadata = await discoverAuthorizationServerMetadata('https://auth.example.com'); + + expect(metadata).toEqual(validOpenIdMetadata); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('throws a diagnosable error when every candidate document fails validation', async () => { + // The AS publishes a near-miss document (200, valid JSON, fits neither schema): + // discovery must not silently return undefined and degrade to endpoint guessing. + mockFetch.mockResolvedValueOnce({ ok: false, status: 404 }); + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ issuer: 'https://auth.example.com', token_endpoint: 'https://auth.example.com/token' }) + }); + + await expect(discoverAuthorizationServerMetadata('https://auth.example.com')).rejects.toThrow( + /matched neither the OAuth 2\.0 \(RFC 8414\) nor the OpenID Connect Discovery metadata schema/ + ); + }); + + it('drops fields that failed the path-implied schema instead of passing them through the fallback parse', async () => { + // An OIDC-shaped document whose jwks_uri fails SafeUrlSchema: the OIDC parse + // fails, the OAuth fallback succeeds — but the unsafe value must not ride + // through the OAuth schema's looseObject passthrough unvalidated. + mockFetch.mockResolvedValueOnce({ ok: false, status: 404 }); + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ ...validOpenIdMetadata, jwks_uri: 'javascript:alert(1)' }) + }); + + const metadata = await discoverAuthorizationServerMetadata('https://auth.example.com'); + + expect(metadata).toBeDefined(); + expect(metadata).not.toHaveProperty('jwks_uri'); + // Fields that passed validation are kept + expect(metadata?.issuer).toBe('https://auth.example.com'); + expect(metadata).toHaveProperty('subject_types_supported', ['public']); + }); + + it('rejects unsafe RFC 8414 endpoint values on OIDC discovery documents', async () => { + // revocation_endpoint is declared on the discovery schema with its OAuth + // validator (SafeUrlSchema), so an unsafe value fails both parses instead of + // riding through the loose object's passthrough. + mockFetch.mockResolvedValueOnce({ ok: false, status: 404 }); + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ ...validOpenIdMetadata, revocation_endpoint: 'javascript:alert(1)' }) + }); + + await expect(discoverAuthorizationServerMetadata('https://auth.example.com')).rejects.toThrow(/revocation_endpoint/); + }); + it('still validates the issuer on RFC 8414 documents found at the openid-configuration path', async () => { mockFetch.mockResolvedValueOnce({ ok: false, status: 404 }); mockFetch.mockResolvedValueOnce({ diff --git a/packages/core/src/auth.ts b/packages/core/src/auth.ts index 56fd2e0e4c..d5f7735c06 100644 --- a/packages/core/src/auth.ts +++ b/packages/core/src/auth.ts @@ -122,13 +122,21 @@ export const OpenIdProviderMetadataSchema = z.looseObject({ * This schema represents the real-world scenario where OIDC providers * return a mix of OpenID Connect and OAuth 2.0 metadata fields * - * Loose like its component schemas so that RFC 8414 fields the OIDC shape - * does not declare (e.g. `revocation_endpoint`, `introspection_endpoint`) - * survive a successful parse instead of being stripped. + * Loose like its component schemas so that fields neither schema declares + * survive a successful parse instead of being stripped. The RFC 8414 fields + * OAuth 2.0 metadata declares and the OIDC shape does not (revocation and + * introspection endpoints) are declared here explicitly so they are + * validated by their OAuth schema validators rather than passed through. */ export const OpenIdProviderDiscoveryMetadataSchema = z.looseObject({ ...OpenIdProviderMetadataSchema.shape, ...OAuthMetadataSchema.pick({ + revocation_endpoint: true, + revocation_endpoint_auth_methods_supported: true, + revocation_endpoint_auth_signing_alg_values_supported: true, + introspection_endpoint: true, + introspection_endpoint_auth_methods_supported: true, + introspection_endpoint_auth_signing_alg_values_supported: true, code_challenge_methods_supported: true }).shape }); From c62a78c07129f040c83d1d62b55c5ab8699e0bbe Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 06:01:54 +0000 Subject: [PATCH 3/7] =?UTF-8?q?fix:=20address=20review=20round=202=20?= =?UTF-8?q?=E2=80=94=20path-independent=20sanitization,=20safe=20introspec?= =?UTF-8?q?tion=5Fendpoint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Run the sibling schema's parse on BOTH branches and drop the top-level fields it rejected, so a mixed document served at the oauth-authorization-server path gets the same sanitization as at the openid-configuration path (an unsafe jwks_uri / op_policy_uri / mis-typed subject_types_supported can no longer ride through the OAuth schema's passthrough by choice of well-known path). This also covers service_documentation, which the OIDC shape declares as a plain string while the OAuth schema requires a safe URL. - Validate introspection_endpoint with SafeUrlSchema in OAuthMetadataSchema (matching revocation_endpoint), so the field newly declared on the discovery schema rejects javascript:/data:/vbscript: values. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F9hQUruXNrCqCqmMjfMCBZ --- packages/client/src/client/auth.ts | 57 +++++++++++++++++------- packages/client/test/client/auth.test.ts | 49 +++++++++++++++++--- packages/core/src/auth.ts | 2 +- 3 files changed, 87 insertions(+), 21 deletions(-) diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 58292d8fdc..3b0770f5d9 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -1887,16 +1887,25 @@ export async function discoverAuthorizationServerMetadata( // well-known path) is not authorization server metadata: try the next candidate. continue; } - const primary = type === 'oauth' ? OAuthMetadataSchema.safeParse(json) : OpenIdProviderDiscoveryMetadataSchema.safeParse(json); + const [primarySchema, siblingSchema] = + type === 'oauth' + ? [OAuthMetadataSchema, OpenIdProviderDiscoveryMetadataSchema] + : [OpenIdProviderDiscoveryMetadataSchema, OAuthMetadataSchema]; + const primary = primarySchema.safeParse(json); + const sibling = siblingSchema.safeParse(json); let parsed: AuthorizationServerMetadata; if (primary.success) { - parsed = primary.data; + // Even on a successful parse, a top-level field the SIBLING schema declares and + // rejected must not ride through this schema's looseObject passthrough + // unvalidated (e.g. a mixed document with an unsafe jwks_uri served at the + // oauth-authorization-server path) — sanitization must not depend on which + // well-known path served the document. + parsed = dropFieldsRejectedByOtherSchema(primary.data, sibling); } else { - const fallback = type === 'oauth' ? OpenIdProviderDiscoveryMetadataSchema.safeParse(json) : OAuthMetadataSchema.safeParse(json); - if (!fallback.success) { + if (!sibling.success) { schemaFailure ??= { url: endpointUrl, - detail: `${summarizeMetadataIssues(primary.error.issues)} (fallback schema: ${summarizeMetadataIssues(fallback.error.issues)})` + detail: `${summarizeMetadataIssues(primary.error.issues)} (fallback schema: ${summarizeMetadataIssues(sibling.error.issues)})` }; continue; } @@ -1905,16 +1914,9 @@ export async function discoverAuthorizationServerMetadata( // looseObject passthrough unvalidated (e.g. an OIDC document with an unsafe // jwks_uri would otherwise be returned via the OAuth schema, which does not // declare that field): drop the fields that failed instead. Fields both - // schemas declare validate identically, so this can only remove fields the - // fallback schema does not know about. - const data: Record = { ...fallback.data }; - for (const issue of primary.error.issues) { - const key = issue.path[0]; - if (typeof key === 'string') { - delete data[key]; - } - } - parsed = data as AuthorizationServerMetadata; + // schemas declare identically fail together, so this can only remove fields + // the fallback schema either does not declare or declares more loosely. + parsed = dropFieldsRejectedByOtherSchema(sibling.data, primary); } if (!skipIssuerValidation) { @@ -1953,6 +1955,31 @@ function summarizeMetadataIssues(issues: ReadonlyArray<{ path: ReadonlyArray `${issue.path.map(String).join('.') || '(document)'}: ${issue.message}`).join('; '); } +/** + * Drops the top-level fields of a parsed authorization server metadata document that the + * OTHER discovery schema declared and rejected, so a value that failed its declared + * validator (e.g. `SafeUrlSchema` on `jwks_uri` or `service_documentation`) can never ride + * through the accepting schema's looseObject passthrough unvalidated. Fields both schemas + * declare identically fail together — this can only remove fields the accepting schema + * either does not declare or declares more loosely, never one it requires. + */ +function dropFieldsRejectedByOtherSchema( + data: AuthorizationServerMetadata, + other: { success: true } | { success: false; error: { issues: ReadonlyArray<{ path: ReadonlyArray }> } } +): AuthorizationServerMetadata { + if (other.success) { + return data; + } + const result: Record = { ...data }; + for (const issue of other.error.issues) { + const key = issue.path[0]; + if (typeof key === 'string') { + delete result[key]; + } + } + return result as AuthorizationServerMetadata; +} + /** * Result of {@linkcode discoverOAuthServerInfo}. */ diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index 169363c416..5d41f5f869 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -1188,17 +1188,56 @@ describe('OAuth Authorization', () => { }); it('rejects unsafe RFC 8414 endpoint values on OIDC discovery documents', async () => { - // revocation_endpoint is declared on the discovery schema with its OAuth - // validator (SafeUrlSchema), so an unsafe value fails both parses instead of - // riding through the loose object's passthrough. + // revocation_endpoint and introspection_endpoint are declared on the discovery + // schema with their OAuth validators (SafeUrlSchema), so an unsafe value fails + // both parses instead of riding through the loose object's passthrough. + for (const field of ['revocation_endpoint', 'introspection_endpoint']) { + mockFetch.mockReset(); + mockFetch.mockResolvedValueOnce({ ok: false, status: 404 }); + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ ...validOpenIdMetadata, [field]: 'javascript:alert(1)' }) + }); + + await expect(discoverAuthorizationServerMetadata('https://auth.example.com')).rejects.toThrow(new RegExp(field)); + } + }); + + it('sanitizes fields the sibling schema rejects even when the path-implied schema accepts the document', async () => { + // A mixed OIDC document served at the oauth-authorization-server path parses + // successfully with the (loose) OAuth schema — the OIDC-declared jwks_uri must + // still be validated, not passed through because of which path served it. + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ ...validOpenIdMetadata, jwks_uri: 'javascript:alert(1)' }) + }); + + const metadata = await discoverAuthorizationServerMetadata('https://auth.example.com'); + + expect(metadata).toBeDefined(); + expect(metadata).not.toHaveProperty('jwks_uri'); + expect(metadata?.issuer).toBe('https://auth.example.com'); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + it('drops an unsafe service_documentation from OIDC discovery documents', async () => { + // The OIDC shape declares service_documentation as a plain string; the OAuth + // schema requires a safe URL. The sibling-schema sanitization drops the unsafe + // value without rejecting the otherwise valid document. mockFetch.mockResolvedValueOnce({ ok: false, status: 404 }); mockFetch.mockResolvedValueOnce({ ok: true, status: 200, - json: async () => ({ ...validOpenIdMetadata, revocation_endpoint: 'javascript:alert(1)' }) + json: async () => ({ ...validOpenIdMetadata, service_documentation: 'javascript:alert(1)' }) }); - await expect(discoverAuthorizationServerMetadata('https://auth.example.com')).rejects.toThrow(/revocation_endpoint/); + const metadata = await discoverAuthorizationServerMetadata('https://auth.example.com'); + + expect(metadata).toBeDefined(); + expect(metadata).not.toHaveProperty('service_documentation'); + expect(metadata?.issuer).toBe('https://auth.example.com'); }); it('still validates the issuer on RFC 8414 documents found at the openid-configuration path', async () => { diff --git a/packages/core/src/auth.ts b/packages/core/src/auth.ts index d5f7735c06..35371c0010 100644 --- a/packages/core/src/auth.ts +++ b/packages/core/src/auth.ts @@ -62,7 +62,7 @@ export const OAuthMetadataSchema = z.looseObject({ revocation_endpoint: SafeUrlSchema.optional(), revocation_endpoint_auth_methods_supported: z.array(z.string()).optional(), revocation_endpoint_auth_signing_alg_values_supported: z.array(z.string()).optional(), - introspection_endpoint: z.string().optional(), + introspection_endpoint: SafeUrlSchema.optional(), introspection_endpoint_auth_methods_supported: z.array(z.string()).optional(), introspection_endpoint_auth_signing_alg_values_supported: z.array(z.string()).optional(), code_challenge_methods_supported: z.array(z.string()).optional(), From ca75d57f7352153a9ef9437531d56aee09e8ad1e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 06:19:00 +0000 Subject: [PATCH 4/7] =?UTF-8?q?fix:=20address=20review=20round=203=20?= =?UTF-8?q?=E2=80=94=20drop=20invalid=20optional=20fields,=20never=20strip?= =?UTF-8?q?=20validated=20ones?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Invalid values in optional metadata fields now drop the field instead of failing the document: the parse retries once with the rejected top-level fields removed, so a relative revocation_endpoint or unsafe introspection_endpoint no longer aborts an auth flow that never uses them; invalid required fields still fail the parse. - The sibling-schema sanitization only removes fields the accepting schema does NOT declare (looseObject passthrough keys): a field the accepting schema itself declared and validated (e.g. OIDC's string service_documentation) is never removed. - The response.json() guard now swallows only SyntaxError (a non-JSON body); network errors reading the body and aborts propagate again. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F9hQUruXNrCqCqmMjfMCBZ --- .../oauth-discovery-schema-by-document.md | 2 +- packages/client/src/client/auth.ts | 100 ++++++++++++------ packages/client/test/client/auth.test.ts | 40 +++++-- 3 files changed, 101 insertions(+), 41 deletions(-) diff --git a/.changeset/oauth-discovery-schema-by-document.md b/.changeset/oauth-discovery-schema-by-document.md index f7d9ebb891..0c54f351e5 100644 --- a/.changeset/oauth-discovery-schema-by-document.md +++ b/.changeset/oauth-discovery-schema-by-document.md @@ -3,4 +3,4 @@ '@modelcontextprotocol/client': patch --- -OAuth discovery now validates authorization server metadata by its document shape instead of by the well-known path it was found under: RFC 8414 metadata served at the `openid-configuration` path (permitted by RFC 8414 §5) is accepted, a document that fits neither schema skips to the next candidate URL instead of aborting discovery, and `OpenIdProviderDiscoveryMetadataSchema` is now a loose object so mixed OIDC/OAuth documents keep their RFC 8414 fields (e.g. `revocation_endpoint`, `introspection_endpoint`). +OAuth discovery now validates authorization server metadata by its document shape instead of by the well-known path it was found under: RFC 8414 metadata served at the `openid-configuration` path (permitted by RFC 8414 §5) is accepted; an invalid value in an optional field drops that field instead of failing the document; a document that fits neither schema skips to the next candidate URL, and if every candidate fails validation, discovery throws an error naming the schema issues instead of silently falling back to guessed endpoints. `OpenIdProviderDiscoveryMetadataSchema` is now a loose object that declares the RFC 8414 revocation/introspection fields with their OAuth validators, so mixed OIDC/OAuth documents keep those fields validated rather than stripped, and `introspection_endpoint` is validated as a safe URL like `revocation_endpoint`. diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 3b0770f5d9..3ef3abbeac 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -1882,42 +1882,49 @@ export async function discoverAuthorizationServerMetadata( let json: unknown; try { json = await response.json(); - } catch { + } catch (error) { // A 200 whose body is not JSON (e.g. an SPA catch-all serving HTML at the // well-known path) is not authorization server metadata: try the next candidate. - continue; + // Anything other than a body-parse failure (a network error reading the body, + // an abort from the caller's fetchFn) still propagates. + if (error instanceof SyntaxError) { + continue; + } + throw error; } const [primarySchema, siblingSchema] = type === 'oauth' ? [OAuthMetadataSchema, OpenIdProviderDiscoveryMetadataSchema] : [OpenIdProviderDiscoveryMetadataSchema, OAuthMetadataSchema]; - const primary = primarySchema.safeParse(json); - const sibling = siblingSchema.safeParse(json); + // Invalid values in optional fields drop the field rather than the document — a + // relative revocation_endpoint or an unsafe introspection_endpoint must not abort + // an auth flow that never uses those fields. Invalid required fields still fail + // the parse. + const primary = parseMetadataDroppingInvalidOptionalFields(primarySchema, json); let parsed: AuthorizationServerMetadata; + let acceptingSchema: typeof primarySchema; if (primary.success) { - // Even on a successful parse, a top-level field the SIBLING schema declares and - // rejected must not ride through this schema's looseObject passthrough - // unvalidated (e.g. a mixed document with an unsafe jwks_uri served at the - // oauth-authorization-server path) — sanitization must not depend on which - // well-known path served the document. - parsed = dropFieldsRejectedByOtherSchema(primary.data, sibling); + parsed = primary.data; + acceptingSchema = primarySchema; } else { - if (!sibling.success) { + const fallback = parseMetadataDroppingInvalidOptionalFields(siblingSchema, json); + if (!fallback.success) { schemaFailure ??= { url: endpointUrl, - detail: `${summarizeMetadataIssues(primary.error.issues)} (fallback schema: ${summarizeMetadataIssues(sibling.error.issues)})` + detail: `${summarizeMetadataIssues(primary.error.issues)} (fallback schema: ${summarizeMetadataIssues(fallback.error.issues)})` }; continue; } - // The document failed the schema its fields are declared by, so any top-level - // field the primary parse rejected must not ride through the fallback schema's - // looseObject passthrough unvalidated (e.g. an OIDC document with an unsafe - // jwks_uri would otherwise be returned via the OAuth schema, which does not - // declare that field): drop the fields that failed instead. Fields both - // schemas declare identically fail together, so this can only remove fields - // the fallback schema either does not declare or declares more loosely. - parsed = dropFieldsRejectedByOtherSchema(sibling.data, primary); + parsed = fallback.data; + acceptingSchema = siblingSchema; } + // A top-level field the accepting schema does NOT declare must not ride through its + // looseObject passthrough unvalidated when the sibling schema declares and rejects + // it (e.g. a document with an unsafe jwks_uri accepted via the OAuth schema, on + // either well-known path). Fields the accepting schema itself declared and + // validated are never removed. + const siblingSanitizer = acceptingSchema === primarySchema ? siblingSchema : primarySchema; + parsed = dropPassthroughFieldsRejectedBySibling(parsed, acceptingSchema, siblingSanitizer.safeParse(json)); if (!skipIssuerValidation) { // RFC 8414 §3.3 / OIDC Discovery §4.3: the `issuer` value in the document MUST be @@ -1955,25 +1962,58 @@ function summarizeMetadataIssues(issues: ReadonlyArray<{ path: ReadonlyArray `${issue.path.map(String).join('.') || '(document)'}: ${issue.message}`).join('; '); } +type AuthorizationServerMetadataSchema = typeof OAuthMetadataSchema | typeof OpenIdProviderDiscoveryMetadataSchema; + +/** + * Parses authorization server metadata, retrying once with the top-level fields the first + * parse rejected removed. An invalid value in an optional field (e.g. a relative + * `revocation_endpoint`) therefore drops that field instead of failing the whole document, + * while an invalid or missing required field still fails the parse (removing it just turns + * the failure into a missing-field failure). + */ +function parseMetadataDroppingInvalidOptionalFields(schema: AuthorizationServerMetadataSchema, json: unknown) { + const first = schema.safeParse(json); + if (first.success || typeof json !== 'object' || json === null) { + return first; + } + const failedPresentKeys = new Set(); + for (const issue of first.error.issues) { + const key = issue.path[0]; + if (typeof key === 'string' && key in json) { + failedPresentKeys.add(key); + } + } + if (failedPresentKeys.size === 0) { + return first; + } + const stripped: Record = { ...(json as Record) }; + for (const key of failedPresentKeys) { + delete stripped[key]; + } + const second = schema.safeParse(stripped); + return second.success ? second : first; +} + /** * Drops the top-level fields of a parsed authorization server metadata document that the - * OTHER discovery schema declared and rejected, so a value that failed its declared - * validator (e.g. `SafeUrlSchema` on `jwks_uri` or `service_documentation`) can never ride - * through the accepting schema's looseObject passthrough unvalidated. Fields both schemas - * declare identically fail together — this can only remove fields the accepting schema - * either does not declare or declares more loosely, never one it requires. + * accepting schema does NOT declare (looseObject passthrough keys) when the sibling schema + * declares and rejected them, so a value that failed its declared validator (e.g. + * `SafeUrlSchema` on `jwks_uri`) can never reach callers unvalidated. Fields the accepting + * schema itself declared and validated are never removed. */ -function dropFieldsRejectedByOtherSchema( +function dropPassthroughFieldsRejectedBySibling( data: AuthorizationServerMetadata, - other: { success: true } | { success: false; error: { issues: ReadonlyArray<{ path: ReadonlyArray }> } } + acceptingSchema: AuthorizationServerMetadataSchema, + sibling: { success: true } | { success: false; error: { issues: ReadonlyArray<{ path: ReadonlyArray }> } } ): AuthorizationServerMetadata { - if (other.success) { + if (sibling.success) { return data; } + const declaredByAcceptingSchema = acceptingSchema.shape; const result: Record = { ...data }; - for (const issue of other.error.issues) { + for (const issue of sibling.error.issues) { const key = issue.path[0]; - if (typeof key === 'string') { + if (typeof key === 'string' && !(key in declaredByAcceptingSchema)) { delete result[key]; } } diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index 5d41f5f869..1e21d3f100 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -1187,10 +1187,11 @@ describe('OAuth Authorization', () => { expect(metadata).toHaveProperty('subject_types_supported', ['public']); }); - it('rejects unsafe RFC 8414 endpoint values on OIDC discovery documents', async () => { + it('drops unsafe RFC 8414 endpoint values from OIDC discovery documents without failing the document', async () => { // revocation_endpoint and introspection_endpoint are declared on the discovery - // schema with their OAuth validators (SafeUrlSchema), so an unsafe value fails - // both parses instead of riding through the loose object's passthrough. + // schema with their OAuth validators (SafeUrlSchema). An unsafe value in these + // optional, client-unused fields drops the field — it must neither ride through + // the loose object's passthrough nor abort the auth flow. for (const field of ['revocation_endpoint', 'introspection_endpoint']) { mockFetch.mockReset(); mockFetch.mockResolvedValueOnce({ ok: false, status: 404 }); @@ -1200,10 +1201,30 @@ describe('OAuth Authorization', () => { json: async () => ({ ...validOpenIdMetadata, [field]: 'javascript:alert(1)' }) }); - await expect(discoverAuthorizationServerMetadata('https://auth.example.com')).rejects.toThrow(new RegExp(field)); + const metadata = await discoverAuthorizationServerMetadata('https://auth.example.com'); + + expect(metadata).toBeDefined(); + expect(metadata).not.toHaveProperty(field); + expect(metadata?.issuer).toBe('https://auth.example.com'); } }); + it('drops an invalid optional field instead of failing an OAuth metadata document', async () => { + // A document whose only flaw is a relative URL in an optional, client-unused + // field must not abort discovery (on base this hard-failed the OAuth path). + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ ...validOAuthMetadata, revocation_endpoint: '/oauth/revoke' }) + }); + + const metadata = await discoverAuthorizationServerMetadata('https://auth.example.com'); + + expect(metadata).toBeDefined(); + expect(metadata).not.toHaveProperty('revocation_endpoint'); + expect(metadata?.token_endpoint).toBe('https://auth.example.com/token'); + }); + it('sanitizes fields the sibling schema rejects even when the path-implied schema accepts the document', async () => { // A mixed OIDC document served at the oauth-authorization-server path parses // successfully with the (loose) OAuth schema — the OIDC-declared jwks_uri must @@ -1222,22 +1243,21 @@ describe('OAuth Authorization', () => { expect(mockFetch).toHaveBeenCalledTimes(1); }); - it('drops an unsafe service_documentation from OIDC discovery documents', async () => { + it('keeps fields the accepting schema itself validated even when the sibling schema rejects them', async () => { // The OIDC shape declares service_documentation as a plain string; the OAuth - // schema requires a safe URL. The sibling-schema sanitization drops the unsafe - // value without rejecting the otherwise valid document. + // schema requires a URL. A value that passed the accepting schema's own + // declared validator must never be removed by the sibling sanitization. mockFetch.mockResolvedValueOnce({ ok: false, status: 404 }); mockFetch.mockResolvedValueOnce({ ok: true, status: 200, - json: async () => ({ ...validOpenIdMetadata, service_documentation: 'javascript:alert(1)' }) + json: async () => ({ ...validOpenIdMetadata, service_documentation: 'Ask the operations team for the docs portal' }) }); const metadata = await discoverAuthorizationServerMetadata('https://auth.example.com'); expect(metadata).toBeDefined(); - expect(metadata).not.toHaveProperty('service_documentation'); - expect(metadata?.issuer).toBe('https://auth.example.com'); + expect(metadata).toHaveProperty('service_documentation', 'Ask the operations team for the docs portal'); }); it('still validates the issuer on RFC 8414 documents found at the openid-configuration path', async () => { From 95d81cea1326434f00e92847a76154146f7af666 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 06:33:05 +0000 Subject: [PATCH 5/7] =?UTF-8?q?fix:=20address=20review=20round=204=20?= =?UTF-8?q?=E2=80=94=20one=20uniform=20field=20policy,=20explicit=20types,?= =?UTF-8?q?=20JSDoc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unified policy, stated once: every URL-carrying optional field is validated with SafeUrlSchema on both discovery schemas (service_documentation joins the pick, closing the javascript:-scheme hole the OIDC z.string() left); invalid optional fields are dropped, mirroring the strip semantics the OIDC path always had; invalid or missing required fields reject the document. - explicit MetadataParseResult return type on parseMetadataDroppingInvalidOptionalFields (repo convention) - discoverAuthorizationServerMetadata JSDoc: @returns/@throws now state the undefined-vs-throw contract (crossAppAccess's discoverAndRequestJwtAuthGrant checked: it throws its own error on undefined and has no catch-and-continue, so the new, more specific throw propagates the same way) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F9hQUruXNrCqCqmMjfMCBZ --- packages/client/src/client/auth.ts | 11 ++++++++-- packages/client/test/client/auth.test.ts | 28 +++++++++++++++++------- packages/core/src/auth.ts | 5 ++++- 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 3ef3abbeac..4aea257bce 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -1823,8 +1823,11 @@ export function buildDiscoveryUrls(authorizationServerUrl: string | URL): { url: * @param options.fetchFn - Optional fetch function for making HTTP requests, defaults to global fetch * @param options.protocolVersion - MCP protocol version to use, defaults to {@linkcode LATEST_PROTOCOL_VERSION} * @param options.skipIssuerValidation - Skip the RFC 8414 §3.3 `issuer` echo check. **Security-weakening.** - * @returns Promise resolving to authorization server metadata, or undefined if discovery fails + * @returns Promise resolving to authorization server metadata, or undefined when no candidate URL + * yielded a metadata document (404s, CORS failures, non-JSON bodies) * @throws {IssuerMismatchError} when the metadata's `issuer` does not match `authorizationServerUrl` + * @throws {Error} when a candidate returns an HTTP 5xx other than 502, or when every candidate that + * returned a JSON document failed schema validation (the error names the schema issues) */ export async function discoverAuthorizationServerMetadata( authorizationServerUrl: string | URL, @@ -1964,6 +1967,10 @@ function summarizeMetadataIssues(issues: ReadonlyArray<{ path: ReadonlyArray; message: string }> } }; + /** * Parses authorization server metadata, retrying once with the top-level fields the first * parse rejected removed. An invalid value in an optional field (e.g. a relative @@ -1971,7 +1978,7 @@ type AuthorizationServerMetadataSchema = typeof OAuthMetadataSchema | typeof Ope * while an invalid or missing required field still fails the parse (removing it just turns * the failure into a missing-field failure). */ -function parseMetadataDroppingInvalidOptionalFields(schema: AuthorizationServerMetadataSchema, json: unknown) { +function parseMetadataDroppingInvalidOptionalFields(schema: AuthorizationServerMetadataSchema, json: unknown): MetadataParseResult { const first = schema.safeParse(json); if (first.success || typeof json !== 'object' || json === null) { return first; diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index 1e21d3f100..cf14a44cdc 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -1243,21 +1243,33 @@ describe('OAuth Authorization', () => { expect(mockFetch).toHaveBeenCalledTimes(1); }); - it('keeps fields the accepting schema itself validated even when the sibling schema rejects them', async () => { - // The OIDC shape declares service_documentation as a plain string; the OAuth - // schema requires a URL. A value that passed the accepting schema's own - // declared validator must never be removed by the sibling sanitization. + it('applies the URL scheme guard to service_documentation on OIDC discovery documents', async () => { + // service_documentation is a URL in both RFC 8414 and OIDC Discovery; the + // discovery schema takes the OAuth validator (SafeUrlSchema) so an unsafe value + // is dropped like any other invalid optional field, while a valid URL survives. mockFetch.mockResolvedValueOnce({ ok: false, status: 404 }); mockFetch.mockResolvedValueOnce({ ok: true, status: 200, - json: async () => ({ ...validOpenIdMetadata, service_documentation: 'Ask the operations team for the docs portal' }) + json: async () => ({ ...validOpenIdMetadata, service_documentation: 'javascript:alert(1)' }) }); - const metadata = await discoverAuthorizationServerMetadata('https://auth.example.com'); + const dropped = await discoverAuthorizationServerMetadata('https://auth.example.com'); - expect(metadata).toBeDefined(); - expect(metadata).toHaveProperty('service_documentation', 'Ask the operations team for the docs portal'); + expect(dropped).toBeDefined(); + expect(dropped).not.toHaveProperty('service_documentation'); + + mockFetch.mockReset(); + mockFetch.mockResolvedValueOnce({ ok: false, status: 404 }); + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ ...validOpenIdMetadata, service_documentation: 'https://auth.example.com/docs' }) + }); + + const kept = await discoverAuthorizationServerMetadata('https://auth.example.com'); + + expect(kept).toHaveProperty('service_documentation', 'https://auth.example.com/docs'); }); it('still validates the issuer on RFC 8414 documents found at the openid-configuration path', async () => { diff --git a/packages/core/src/auth.ts b/packages/core/src/auth.ts index 35371c0010..e7bef70a28 100644 --- a/packages/core/src/auth.ts +++ b/packages/core/src/auth.ts @@ -126,7 +126,9 @@ export const OpenIdProviderMetadataSchema = z.looseObject({ * survive a successful parse instead of being stripped. The RFC 8414 fields * OAuth 2.0 metadata declares and the OIDC shape does not (revocation and * introspection endpoints) are declared here explicitly so they are - * validated by their OAuth schema validators rather than passed through. + * validated by their OAuth schema validators rather than passed through, + * and `service_documentation` takes the OAuth schema's URL validator so + * every URL-carrying field is scheme-guarded uniformly. */ export const OpenIdProviderDiscoveryMetadataSchema = z.looseObject({ ...OpenIdProviderMetadataSchema.shape, @@ -137,6 +139,7 @@ export const OpenIdProviderDiscoveryMetadataSchema = z.looseObject({ introspection_endpoint: true, introspection_endpoint_auth_methods_supported: true, introspection_endpoint_auth_signing_alg_values_supported: true, + service_documentation: true, code_challenge_methods_supported: true }).shape }); From a55e373b9b33e1e26e9ae6f3c13b93084ca07a94 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 06:44:00 +0000 Subject: [PATCH 6/7] fix: apply the field policy to the deprecated discoverOAuthMetadata path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The introspection_endpoint tightening made the deprecated (still exported) discoverOAuthMetadata throw on documents base accepted — its raw OAuthMetadataSchema.parse was the one call site left outside the drop-invalid-optional-fields policy (repo-wide grep: no others). It now reuses parseMetadataDroppingInvalidOptionalFields, throwing the original ZodError only when required fields fail, with a regression test. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F9hQUruXNrCqCqmMjfMCBZ --- packages/client/src/client/auth.ts | 11 +++++++++-- packages/client/test/client/auth.test.ts | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 4aea257bce..1c0ee5ab82 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -1732,7 +1732,14 @@ export async function discoverOAuthMetadata( throw new Error(`HTTP ${response.status} trying to load well-known OAuth metadata`); } - return OAuthMetadataSchema.parse(await response.json()); + // Same field policy as discoverAuthorizationServerMetadata: an invalid value in an + // optional field drops the field instead of failing the document; invalid or missing + // required fields still reject. + const result = parseMetadataDroppingInvalidOptionalFields(OAuthMetadataSchema, await response.json()); + if (!result.success) { + throw result.error; + } + return result.data as OAuthMetadata; } /** @@ -1969,7 +1976,7 @@ type AuthorizationServerMetadataSchema = typeof OAuthMetadataSchema | typeof Ope type MetadataParseResult = | { success: true; data: AuthorizationServerMetadata } - | { success: false; data?: undefined; error: { issues: ReadonlyArray<{ path: ReadonlyArray; message: string }> } }; + | { success: false; data?: undefined; error: Error & { issues: ReadonlyArray<{ path: ReadonlyArray; message: string }> } }; /** * Parses authorization server metadata, retrying once with the top-level fields the first diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index cf14a44cdc..698009d5fa 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -1209,6 +1209,23 @@ describe('OAuth Authorization', () => { } }); + it('legacy discoverOAuthMetadata drops an invalid optional field instead of throwing', async () => { + // The deprecated path shares the field policy: base returned this document + // (introspection_endpoint was a plain string then), so the tightened validator + // must drop the field, not reject the document. + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ ...validOAuthMetadata, introspection_endpoint: '/oauth/introspect' }) + }); + + const metadata = await discoverOAuthMetadata('https://auth.example.com'); + + expect(metadata).toBeDefined(); + expect(metadata).not.toHaveProperty('introspection_endpoint'); + expect(metadata?.token_endpoint).toBe('https://auth.example.com/token'); + }); + it('drops an invalid optional field instead of failing an OAuth metadata document', async () => { // A document whose only flaw is a relative URL in an optional, client-unused // field must not abort discovery (on base this hard-failed the OAuth path). From 2112499d62a8e12aa0d856d397f0d7b7f6f28a6b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 07:01:18 +0000 Subject: [PATCH 7/7] fix: sanitize the deprecated discovery path too; record the behavior deltas in the migration guide MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - discoverOAuthMetadata now applies the same sibling-schema sanitization as discoverAuthorizationServerMetadata, so OIDC-declared fields it only passes through (jwks_uri et al.) cannot reach callers unvalidated — making the deprecated path consistent with the PR's stated policy. - docs/migration/upgrade-to-v2.md 'OAuth client flow — behavioral changes' gains a bullet for the discovery deltas (validate-by-shape, drop invalid optional fields, URL guards on introspection_endpoint / service_documentation, throw on all-candidates schema failure), per the repo's breaking-changes documentation rule. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F9hQUruXNrCqCqmMjfMCBZ --- docs/migration/upgrade-to-v2.md | 12 ++++++++++++ packages/client/src/client/auth.ts | 12 +++++++++--- packages/client/test/client/auth.test.ts | 17 +++++++++++++++++ 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/docs/migration/upgrade-to-v2.md b/docs/migration/upgrade-to-v2.md index 802c2ec264..608e913079 100644 --- a/docs/migration/upgrade-to-v2.md +++ b/docs/migration/upgrade-to-v2.md @@ -1109,6 +1109,18 @@ OAuth `onUnauthorized` behavior, for composing your own adapter). - **Metadata discovery falls through on 502.** `discoverAuthorizationServerMetadata()` treats `502 Bad Gateway` like 4xx — fall through to the next candidate URL instead of throwing (fixes path-aware discovery behind reverse proxies). Other 5xx still throw. +- **Metadata discovery validates by document shape, not well-known path.** + `discoverAuthorizationServerMetadata()` accepts conforming RFC 8414 metadata served at + the `openid-configuration` path (RFC 8414 §5) instead of rejecting it against the OIDC + schema. An invalid value in an _optional_ metadata field (including a + non-URL or `javascript:`-scheme `introspection_endpoint`/`service_documentation`, which + are now URL-validated like `revocation_endpoint`) drops that field instead of failing + the document; invalid or missing _required_ fields still reject it. A `200` document + that fits neither metadata schema falls through to the next candidate URL, and when + every candidate that returned JSON fails validation, discovery throws an `Error` naming + the schema issues instead of returning `undefined` (returning `undefined` is reserved + for "no metadata found": 404s, CORS failures, non-JSON bodies). The deprecated + `discoverOAuthMetadata()` applies the same drop-invalid-optional-fields policy. - **Scoped credential invalidation on `invalid_client` / `unauthorized_client`.** The `auth()` retry for these errors now issues two scoped calls — `invalidateCredentials('client')` then `invalidateCredentials('tokens')` — instead of diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 1c0ee5ab82..95e33759d6 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -1734,12 +1734,18 @@ export async function discoverOAuthMetadata( // Same field policy as discoverAuthorizationServerMetadata: an invalid value in an // optional field drops the field instead of failing the document; invalid or missing - // required fields still reject. - const result = parseMetadataDroppingInvalidOptionalFields(OAuthMetadataSchema, await response.json()); + // required fields still reject; OIDC-declared fields the OAuth schema only passes + // through are sanitized rather than returned unvalidated. + const json: unknown = await response.json(); + const result = parseMetadataDroppingInvalidOptionalFields(OAuthMetadataSchema, json); if (!result.success) { throw result.error; } - return result.data as OAuthMetadata; + return dropPassthroughFieldsRejectedBySibling( + result.data, + OAuthMetadataSchema, + OpenIdProviderDiscoveryMetadataSchema.safeParse(json) + ) as OAuthMetadata; } /** diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index 698009d5fa..3071c46392 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -1209,6 +1209,23 @@ describe('OAuth Authorization', () => { } }); + it('legacy discoverOAuthMetadata sanitizes OIDC-declared fields it only passes through', async () => { + // Same sanitization as discoverAuthorizationServerMetadata: jwks_uri is not + // declared by the OAuth schema, so an unsafe value must be dropped, not + // returned via the looseObject passthrough. + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ ...validOpenIdMetadata, jwks_uri: 'javascript:alert(1)' }) + }); + + const metadata = await discoverOAuthMetadata('https://auth.example.com'); + + expect(metadata).toBeDefined(); + expect(metadata).not.toHaveProperty('jwks_uri'); + expect(metadata?.issuer).toBe('https://auth.example.com'); + }); + it('legacy discoverOAuthMetadata drops an invalid optional field instead of throwing', async () => { // The deprecated path shares the field policy: base returned this document // (introspection_endpoint was a plain string then), so the tightened validator