diff --git a/.github/workflows/ci-build-test.yml b/.github/workflows/ci-build-test.yml index dc788d08e..faab32ff8 100644 --- a/.github/workflows/ci-build-test.yml +++ b/.github/workflows/ci-build-test.yml @@ -52,7 +52,7 @@ jobs: - name: 🔧 Set up Node.js uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: - node-version: '20' + node-version: '22' - name: 📦 Install pinned npm dependencies for tests run: npm ci diff --git a/docs/concepts/transports/transports.md b/docs/concepts/transports/transports.md index eaacda709..e631ebd87 100644 --- a/docs/concepts/transports/transports.md +++ b/docs/concepts/transports/transports.md @@ -145,6 +145,39 @@ var transport = new HttpClientTransport(new HttpClientTransportOptions }); ``` +#### OAuth PKCE metadata validation (1.x) + +When using with an HTTP transport, authorization server metadata must advertise `S256` in `code_challenge_methods_supported`. A document that omits that field or advertises only other methods is skipped while the client tries alternative well-known endpoints. If none supplies valid metadata with `S256`, authentication fails before requesting an authorization code or exchanging tokens. This is a compatibility tightening: servers that implement PKCE but do not advertise it must update their metadata. + +When protected resource metadata is available, authorization server metadata must also include an `issuer` that exactly matches the selected authorization server identifier, as required by [RFC 8414, Section 3.3](https://www.rfc-editor.org/rfc/rfc8414.html#section-3.3). Comparison is ordinal and uses the original URI strings: differences in case, trailing slashes, or percent-encoding are rejected even when URI normalization would make them equivalent. A missing or mismatched issuer immediately stops discovery before authorization or token exchange; the client does not try another metadata endpoint or use synthetic defaults. For the MCP 2025-03-26 legacy path without protected resource metadata, these issuer checks remain exempt because discovery uses the MCP server's origin, which need not equal the canonical issuer. PKCE requirements still apply on that path. + +These are **metadata issuer** checks, not RFC 9207 authorization-response `iss` validation. The 1.x code-only callback cannot provide a response issuer for SDK validation; that separate response check is not included. PKCE is not a substitute for issuer validation. + +The 1.x SDK's authorization callback returns only a code, so the SDK cannot validate response `state`. [RFC 9700, Section 4.7.1](https://www.rfc-editor.org/rfc/rfc9700.html#section-4.7.1) permits relying on PKCE for CSRF protection only when authorization server support has been ensured. Metadata verification checks **advertised support**, not the server's actual implementation. The authorization server must correctly enforce PKCE, including the [downgrade defenses in Section 4.8.2](https://www.rfc-editor.org/rfc/rfc9700.html#section-4.8.2). + +**Legacy compatibility escape hatch.** By default, the 1.x client also rejects authentication when authorization server metadata cannot be discovered; it no longer assumes `S256` support. For a trusted legacy deployment whose PKCE support you have independently established, you can explicitly restore the MCP 2025-03-26 default endpoint fallback at application startup: + +```csharp +AppContext.SetSwitch( + "ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallback", + true); +``` + +Alternatively, configure the same switch in the application's project file: + +```xml + + + +``` + +The switch is **disabled when absent or `false`**. When enabled, it restores default `/authorize`, `/token`, and `/register` paths on the MCP server's origin only when protected resource metadata is unavailable and no usable authorization server metadata document was discovered. It never bypasses PKCE validation of a discovered document. The client still sends an `S256` challenge and a code verifier, and logs a warning whenever it uses the unverified fallback. + +> [!WARNING] +> Enabling this switch does not verify PKCE support or add state validation. Use it only after independently establishing that your trusted authorization server correctly enforces PKCE, including downgrade defenses. The setting is **process-wide**, not a per-server allowlist: it affects every OAuth client in the process. Do not enable it in a process that also connects to untrusted authorization servers, or toggle it around individual requests. Prefer fixing metadata discovery and advertising `S256` so the switch can remain disabled. + #### Resuming sessions Streamable HTTP supports session resumption. Save the session ID, server capabilities, and server info from the original session, then use to reconnect: diff --git a/package-lock.json b/package-lock.json index 34d53809d..638bb697a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,11 +1,11 @@ { - "name": "csharp-sdk", + "name": "halter73-refactored-garbanzo", "lockfileVersion": 3, "requires": true, "packages": { "": { "dependencies": { - "@modelcontextprotocol/conformance": "0.1.13", + "@modelcontextprotocol/conformance": "0.1.15", "@modelcontextprotocol/server-everything": "2026.1.26", "@modelcontextprotocol/server-memory": "2026.1.26" } @@ -23,12 +23,13 @@ } }, "node_modules/@modelcontextprotocol/conformance": { - "version": "0.1.13", - "resolved": "https://registry.npmjs.org/@modelcontextprotocol/conformance/-/conformance-0.1.13.tgz", - "integrity": "sha512-J9cC7Dsi7hTn7sxorvxDZ+wC8OlHAkOmVxVm1/IBQ8HEpYJu8r0eAUEw68vTmjBPP5WcU8Jld4RZ3cOFp/Ih1g==", + "version": "0.1.15", + "resolved": "https://registry.npmjs.org/@modelcontextprotocol/conformance/-/conformance-0.1.15.tgz", + "integrity": "sha512-B1eNYpv5kas9YFC40su7SWhKHR2DwYzJRpiX6dfWzDWMcDr71myKVj//PwyqUHC7oucs3EJqcqnwvSuUwvenoQ==", "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.26.0", + "@octokit/rest": "^22.0.0", "commander": "^14.0.2", "eventsource-parser": "^3.0.6", "express": "^5.1.0", @@ -110,6 +111,204 @@ "mcp-server-memory": "dist/index.js" } }, + "node_modules/@octokit/auth-token": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/@octokit/auth-token/-/auth-token-6.0.0.tgz", + "integrity": "sha512-P4YJBPdPSpWTQ1NU4XYdvHvXJJDxM6YwpS0FZHRgP7YFkdVxsWcpWGy/NVqlAA7PcPCnMacXlRm1y2PFZRWL/w==", + "license": "MIT", + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/core": { + "version": "7.0.8", + "resolved": "https://registry.npmjs.org/@octokit/core/-/core-7.0.8.tgz", + "integrity": "sha512-L7y8eYc+AwxGr2PWI4WFt1VG4TiJ66c26BD16mXpYIlXxG0SMigM1+m4aTSlYyBr5BlQsGAlz8uDCoZN4SEMcg==", + "license": "MIT", + "dependencies": { + "@octokit/auth-token": "^6.0.0", + "@octokit/graphql": "^9.0.5", + "@octokit/request": "^10.0.16", + "@octokit/request-error": "^7.1.2", + "@octokit/types": "^18.0.0", + "before-after-hook": "^4.0.0", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/endpoint": { + "version": "11.0.5", + "resolved": "https://registry.npmjs.org/@octokit/endpoint/-/endpoint-11.0.5.tgz", + "integrity": "sha512-iXa654H3yFafF/ieHkukfbgWo2rmXD2ceD0ZOtrPhw1bc3FDch1d9N/TNs0FQ1/cIbwb7kspUX8jzIs8nzb9DQ==", + "license": "MIT", + "dependencies": { + "@octokit/types": "^18.0.0", + "universal-user-agent": "^7.0.2" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/graphql": { + "version": "9.0.5", + "resolved": "https://registry.npmjs.org/@octokit/graphql/-/graphql-9.0.5.tgz", + "integrity": "sha512-bt/hm03LeU6Vy7FwTrkkC9p3XGT/lBwClglMqxBSe5/q0E5CdJTXeAqEI0vlw89/LF/G6tryTIH8HirZ3prMVg==", + "license": "MIT", + "dependencies": { + "@octokit/request": "^10.0.16", + "@octokit/types": "^18.0.0", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/openapi-types": { + "version": "29.0.1", + "resolved": "https://registry.npmjs.org/@octokit/openapi-types/-/openapi-types-29.0.1.tgz", + "integrity": "sha512-9qWOMFNxxLokERcms42rU0PTLqQmVs7g5E41TI4mCOxmpFayD1rfC7XxOL55cG9MBZLFlC31BrR37myMKardwg==", + "license": "MIT" + }, + "node_modules/@octokit/plugin-paginate-rest": { + "version": "14.0.0", + "resolved": "https://registry.npmjs.org/@octokit/plugin-paginate-rest/-/plugin-paginate-rest-14.0.0.tgz", + "integrity": "sha512-fNVRE7ufJiAA3XUrha2omTA39M6IXIc6GIZLvlbsm8QOQCYvpq/LkMNGyFlB1d8hTDzsAXa3OKtybdMAYsV/fw==", + "license": "MIT", + "dependencies": { + "@octokit/types": "^16.0.0" + }, + "engines": { + "node": ">= 20" + }, + "peerDependencies": { + "@octokit/core": ">=6" + } + }, + "node_modules/@octokit/plugin-paginate-rest/node_modules/@octokit/openapi-types": { + "version": "27.0.0", + "resolved": "https://registry.npmjs.org/@octokit/openapi-types/-/openapi-types-27.0.0.tgz", + "integrity": "sha512-whrdktVs1h6gtR+09+QsNk2+FO+49j6ga1c55YZudfEG+oKJVvJLQi3zkOm5JjiUXAagWK2tI2kTGKJ2Ys7MGA==", + "license": "MIT" + }, + "node_modules/@octokit/plugin-paginate-rest/node_modules/@octokit/types": { + "version": "16.0.0", + "resolved": "https://registry.npmjs.org/@octokit/types/-/types-16.0.0.tgz", + "integrity": "sha512-sKq+9r1Mm4efXW1FCk7hFSeJo4QKreL/tTbR0rz/qx/r1Oa2VV83LTA/H/MuCOX7uCIJmQVRKBcbmWoySjAnSg==", + "license": "MIT", + "dependencies": { + "@octokit/openapi-types": "^27.0.0" + } + }, + "node_modules/@octokit/plugin-request-log": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/@octokit/plugin-request-log/-/plugin-request-log-6.0.0.tgz", + "integrity": "sha512-UkOzeEN3W91/eBq9sPZNQ7sUBvYCqYbrrD8gTbBuGtHEuycE4/awMXcYvx6sVYo7LypPhmQwwpUe4Yyu4QZN5Q==", + "license": "MIT", + "engines": { + "node": ">= 20" + }, + "peerDependencies": { + "@octokit/core": ">=6" + } + }, + "node_modules/@octokit/plugin-rest-endpoint-methods": { + "version": "17.0.0", + "resolved": "https://registry.npmjs.org/@octokit/plugin-rest-endpoint-methods/-/plugin-rest-endpoint-methods-17.0.0.tgz", + "integrity": "sha512-B5yCyIlOJFPqUUeiD0cnBJwWJO8lkJs5d8+ze9QDP6SvfiXSz1BF+91+0MeI1d2yxgOhU/O+CvtiZ9jSkHhFAw==", + "license": "MIT", + "dependencies": { + "@octokit/types": "^16.0.0" + }, + "engines": { + "node": ">= 20" + }, + "peerDependencies": { + "@octokit/core": ">=6" + } + }, + "node_modules/@octokit/plugin-rest-endpoint-methods/node_modules/@octokit/openapi-types": { + "version": "27.0.0", + "resolved": "https://registry.npmjs.org/@octokit/openapi-types/-/openapi-types-27.0.0.tgz", + "integrity": "sha512-whrdktVs1h6gtR+09+QsNk2+FO+49j6ga1c55YZudfEG+oKJVvJLQi3zkOm5JjiUXAagWK2tI2kTGKJ2Ys7MGA==", + "license": "MIT" + }, + "node_modules/@octokit/plugin-rest-endpoint-methods/node_modules/@octokit/types": { + "version": "16.0.0", + "resolved": "https://registry.npmjs.org/@octokit/types/-/types-16.0.0.tgz", + "integrity": "sha512-sKq+9r1Mm4efXW1FCk7hFSeJo4QKreL/tTbR0rz/qx/r1Oa2VV83LTA/H/MuCOX7uCIJmQVRKBcbmWoySjAnSg==", + "license": "MIT", + "dependencies": { + "@octokit/openapi-types": "^27.0.0" + } + }, + "node_modules/@octokit/request": { + "version": "10.0.16", + "resolved": "https://registry.npmjs.org/@octokit/request/-/request-10.0.16.tgz", + "integrity": "sha512-A0zWGjHzISIb+9ccG8s0dq7LKO5zVpJLRICjgUb+sJxEWqn8RUHB1rD3AE51+PECvXHIxqZ1VVvs4fHTSD9nUQ==", + "license": "MIT", + "dependencies": { + "@octokit/endpoint": "^11.0.5", + "@octokit/request-error": "^7.1.2", + "@octokit/types": "^18.0.0", + "content-type": "^3.0.0", + "json-with-bigint": "^3.5.12", + "universal-user-agent": "^7.0.2" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/request-error": { + "version": "7.1.2", + "resolved": "https://registry.npmjs.org/@octokit/request-error/-/request-error-7.1.2.tgz", + "integrity": "sha512-XZRuT3xZ84D3gYErI1DZvhJ33dCWVV6uzBtWkaBB4TvA/L6eOeTZodxLFVB44bBEEo3vEx7y00UfX1tBLrtLRg==", + "license": "MIT", + "dependencies": { + "@octokit/types": "^18.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/request/node_modules/content-type": { + "version": "3.1.1", + "resolved": "https://registry.npmjs.org/content-type/-/content-type-3.1.1.tgz", + "integrity": "sha512-GW4qUsfFo59d0HbUibDlWv5wPz+vAAcaTWbKIuKCf0JkC7wWkSyf8f13IpXn5JkeMlB2P8iTSSIjwXljorg2vA==", + "license": "MIT", + "engines": { + "node": ">=22" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" + } + }, + "node_modules/@octokit/rest": { + "version": "22.0.1", + "resolved": "https://registry.npmjs.org/@octokit/rest/-/rest-22.0.1.tgz", + "integrity": "sha512-Jzbhzl3CEexhnivb1iQ0KJ7s5vvjMWcmRtq5aUsKmKDrRW6z3r84ngmiFKFvpZjpiU/9/S6ITPFRpn5s/3uQJw==", + "license": "MIT", + "dependencies": { + "@octokit/core": "^7.0.6", + "@octokit/plugin-paginate-rest": "^14.0.0", + "@octokit/plugin-request-log": "^6.0.0", + "@octokit/plugin-rest-endpoint-methods": "^17.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/types": { + "version": "18.0.0", + "resolved": "https://registry.npmjs.org/@octokit/types/-/types-18.0.0.tgz", + "integrity": "sha512-l6bAF43PNxkJp6g+W4PjoUSSkxHomXw2nOum5CTftJz1NlV3vu93NImgOYtLf6CbBUb5j+fiuzW0PPQ5JTSvZA==", + "license": "MIT", + "dependencies": { + "@octokit/openapi-types": "^29.0.1" + } + }, "node_modules/accepts": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/accepts/-/accepts-2.0.0.tgz", @@ -156,6 +355,12 @@ } } }, + "node_modules/before-after-hook": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/before-after-hook/-/before-after-hook-4.0.0.tgz", + "integrity": "sha512-q6tR3RPqIB1pMiTRMFcZwuG5T8vwp+vUvEG0vuI6B+Rikh5BfPp2fQ82c925FOs+b0lcFQ8CFrL+KbilfZFhOQ==", + "license": "Apache-2.0" + }, "node_modules/body-parser": { "version": "2.2.2", "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-2.2.2.tgz", @@ -743,6 +948,12 @@ "integrity": "sha512-fQhoXdcvc3V28x7C7BMs4P5+kNlgUURe2jmUT1T//oBRMDrqy1QPelJimwZGo7Hg9VPV3EQV5Bnq4hbFy2vetA==", "license": "BSD-2-Clause" }, + "node_modules/json-with-bigint": { + "version": "3.5.12", + "resolved": "https://registry.npmjs.org/json-with-bigint/-/json-with-bigint-3.5.12.tgz", + "integrity": "sha512-uwbF/wSSuOgC7qqlq27Xp5B6a2MHVug3t0idZdTqu0JnlFvgJuH7ju+KAk/J06C7GfhoYy2gnb9wz2INqcne7w==", + "license": "MIT" + }, "node_modules/jszip": { "version": "3.10.1", "resolved": "https://registry.npmjs.org/jszip/-/jszip-3.10.1.tgz", @@ -1229,6 +1440,12 @@ "node": ">=20.18.1" } }, + "node_modules/universal-user-agent": { + "version": "7.0.3", + "resolved": "https://registry.npmjs.org/universal-user-agent/-/universal-user-agent-7.0.3.tgz", + "integrity": "sha512-TmnEAEAsBJVZM/AADELsK76llnwcf9vMKuPz8JflO1frO8Lchitr0fNaN9d+Ap0BjKtqWqd/J17qeDnXh8CL2A==", + "license": "ISC" + }, "node_modules/unpipe": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/unpipe/-/unpipe-1.0.0.tgz", diff --git a/package.json b/package.json index b05f143aa..a324ac45a 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "private": true, "description": "Pinned npm dependencies for MCP C# SDK integration and conformance tests", "dependencies": { - "@modelcontextprotocol/conformance": "0.1.13", + "@modelcontextprotocol/conformance": "0.1.15", "@modelcontextprotocol/server-everything": "2026.1.26", "@modelcontextprotocol/server-memory": "2026.1.26" } diff --git a/samples/ProtectedMcpClient/README.md b/samples/ProtectedMcpClient/README.md index 81ae67cee..7930f244c 100644 --- a/samples/ProtectedMcpClient/README.md +++ b/samples/ProtectedMcpClient/README.md @@ -69,6 +69,8 @@ The client is configured with: - **OAuth Server**: `https://localhost:7029` - **Protected Resource**: `http://localhost:7071` +The authorization server must advertise `S256` PKCE support in its metadata. When protected resource metadata is available, its metadata `issuer` must also exactly match the selected authorization server identifier. The test server satisfies both requirements by default. The 1.x SDK rejects missing PKCE advertisement and, by default, unavailable authorization server metadata; see [OAuth PKCE metadata validation](../../docs/concepts/transports/transports.md#oauth-pkce-metadata-validation-1x) for the issuer matching rules, narrowly scoped process-wide legacy compatibility switch, and security requirements. + ## Available Tools Once authenticated, the client can access weather tools including: diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthOptions.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthOptions.cs index 483e3643e..3134d9668 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthOptions.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthOptions.cs @@ -3,6 +3,19 @@ namespace ModelContextProtocol.Authentication; /// /// Provides configuration options for the . /// +/// +/// Authorization server metadata must advertise the S256 PKCE method. If metadata discovery fails, +/// authorization fails before requesting an authorization code or exchanging tokens. When protected resource +/// metadata is available, the authorization server metadata issuer must exactly match the selected authorization +/// server identifier, without normalizing URI case, trailing slashes, or percent-encoding. Missing or mismatched +/// issuers immediately stop discovery. These issuer checks are skipped for the 2025-03-26 legacy path without +/// protected resource metadata, where discovery uses the server origin rather than the canonical issuer. For trusted legacy +/// deployments without protected resource metadata and without usable authorization server metadata, the process-wide +/// ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallback switch +/// can explicitly enable the 2025-03-26 default endpoint fallback. It is disabled by default and does not +/// bypass PKCE validation of discovered metadata. Hosts enabling it must independently establish that +/// their authorization servers correctly enforce PKCE, including downgrade defenses. +/// public sealed class ClientOAuthOptions { /// diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs index ecef8e15e..e0ca6775e 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -334,8 +334,15 @@ static bool IsValidClientMetadataDocumentUri(Uri uri) private async Task GetAuthServerMetadataAsync(Uri authServerUri, string? resourceUri, CancellationToken cancellationToken) { + // Tracks whether at least one well-known endpoint returned a structurally valid metadata document. + // This distinguishes "no metadata document was found" (eligible for the opt-in 2025-03-26 legacy fallback) + // from "a metadata document exists but failed PKCE validation" (must not fall back to synthesized defaults). + var metadataDocumentFound = false; + McpException? pkceValidationFailure = null; + foreach (var wellKnownEndpoint in GetWellKnownAuthorizationServerMetadataUris(authServerUri)) { + AuthorizationServerMetadata? metadata; try { var response = await _httpClient.GetAsync(wellKnownEndpoint, cancellationToken).ConfigureAwait(false); @@ -345,7 +352,7 @@ private async Task GetAuthServerMetadataAsync(Uri a } using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); - var metadata = await JsonSerializer.DeserializeAsync(stream, McpJsonUtilities.JsonContext.Default.AuthorizationServerMetadata, cancellationToken).ConfigureAwait(false); + metadata = await JsonSerializer.DeserializeAsync(stream, McpJsonUtilities.JsonContext.Default.AuthorizationServerMetadata, cancellationToken).ConfigureAwait(false); if (metadata is null) { @@ -363,23 +370,100 @@ private async Task GetAuthServerMetadataAsync(Uri a ThrowFailedToHandleUnauthorizedResponse($"AuthorizationEndpoint must use HTTP or HTTPS. '{metadata.AuthorizationEndpoint}' does not meet this requirement."); } - metadata.ResponseTypesSupported ??= ["code"]; - metadata.GrantTypesSupported ??= ["authorization_code", "refresh_token"]; - metadata.TokenEndpointAuthMethodsSupported ??= ["client_secret_post"]; - metadata.CodeChallengeMethodsSupported ??= ["S256"]; + // Validate the issuer in the metadata document per RFC 8414 Section 3.3: + // the issuer value MUST be identical to the issuer identifier used to construct + // the well-known URL. + // Skip validation in legacy backcompat mode (resourceUri is null) because the + // authServerUri was derived from the server origin rather than from Protected + // Resource Metadata, so it may not match the server's canonical issuer. + // Note: resourceUri is null exclusively in the 2025-03-26 legacy path. For newer + // protocol versions, ExtractProtectedResourceMetadata throws if the PRM document + // omits the resource field (VerifyResourceMatch returns false for null Resource), + // so we never reach this point with resourceUri == null in non-legacy flows. + if (resourceUri is not null && metadata.Issuer is null) + { + ThrowFailedToHandleUnauthorizedResponse( + $"Authorization server metadata from '{wellKnownEndpoint}' did not provide the required issuer (RFC 8414 Section 2)."); + } + + // RFC 8414 requires an identical issuer value, so do not normalize URI case, + // trailing slashes, or percent-encoding before comparison. + if (resourceUri is not null && + !string.Equals(metadata.Issuer!.OriginalString, authServerUri.OriginalString, StringComparison.Ordinal)) + { + ThrowFailedToHandleUnauthorizedResponse( + $"Authorization server metadata issuer '{metadata.Issuer}' does not match the expected issuer '{authServerUri}' (RFC 8414 Section 3.3)."); + } - return metadata; + // A structurally valid metadata document was discovered. Even if it fails PKCE validation + // below, its existence disqualifies the legacy fallback that would otherwise synthesize S256. + metadataDocumentFound = true; + } + catch (McpException) + { + // Metadata validation failures are security signals and must not fall back to + // another well-known endpoint. + throw; } catch (Exception ex) { LogErrorFetchingAuthServerMetadata(ex, wellKnownEndpoint); + continue; + } + + try + { + // Per the MCP spec, clients MUST verify PKCE support via authorization server metadata and refuse + // to proceed without it. Rather than failing on the first document that lacks it, skip to the next + // discovery endpoint: different well-known endpoints (OAuth 2.0 vs OpenID Connect) can return + // different documents for the same server, and OpenID Connect metadata commonly includes the field. + var codeChallengeMethods = metadata.CodeChallengeMethodsSupported; + if (codeChallengeMethods is null) + { + ThrowFailedToHandleUnauthorizedResponse( + $"Authorization server metadata from '{wellKnownEndpoint}' does not include 'code_challenge_methods_supported'. MCP clients require PKCE support and must refuse to proceed."); + } + + if (!codeChallengeMethods.Contains("S256")) + { + var advertisedMethods = codeChallengeMethods.Count > 0 + ? string.Join(", ", codeChallengeMethods) + : ""; + ThrowFailedToHandleUnauthorizedResponse( + $"Authorization server metadata from '{wellKnownEndpoint}' does not advertise required PKCE method 'S256' in 'code_challenge_methods_supported'. Advertised methods: {advertisedMethods}."); + } + } + catch (Exception ex) + { + LogAuthServerMetadataMissingPkceSupport(ex, wellKnownEndpoint); + pkceValidationFailure = ex as McpException ?? new McpException(ex.Message, ex); + continue; } + + metadata.ResponseTypesSupported ??= ["code"]; + metadata.GrantTypesSupported ??= ["authorization_code", "refresh_token"]; + metadata.TokenEndpointAuthMethodsSupported ??= ["client_secret_post"]; + + return metadata; + } + + // A discovered metadata document that failed PKCE validation must not be replaced with synthesized + // defaults. Surface the specific PKCE failure regardless of whether PRM was available. + if (metadataDocumentFound && pkceValidationFailure is not null) + { + throw pkceValidationFailure; } if (resourceUri is null) { - // 2025-03-26 backcompat: when PRM is unavailable and auth server metadata discovery - // also fails, fall back to default endpoint paths per the 2025-03-26 spec. + if (!AppContext.TryGetSwitch("ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallback", out var enabled) || !enabled) + { + ThrowFailedToHandleUnauthorizedResponse("Authorization server metadata could not be discovered, so PKCE support could not be verified."); + } + + // 2025-03-26 backcompat: when PRM is unavailable and no auth server metadata document was + // discovered, allow explicitly opted-in hosts to use the default endpoint paths. + LogUsingLegacyOAuthMetadataFallback(authServerUri); return BuildDefaultAuthServerMetadata(authServerUri); } @@ -989,6 +1073,12 @@ private static void ThrowFailedToHandleUnauthorizedResponse(string message) => [LoggerMessage(Level = LogLevel.Error, Message = "Error fetching auth server metadata from {Endpoint}")] partial void LogErrorFetchingAuthServerMetadata(Exception ex, Uri endpoint); + [LoggerMessage(Level = LogLevel.Warning, Message = "Authorization server metadata from {Endpoint} does not satisfy PKCE requirements; skipping it and trying the next discovery endpoint.")] + partial void LogAuthServerMetadataMissingPkceSupport(Exception ex, Uri endpoint); + + [LoggerMessage(Level = LogLevel.Warning, Message = "Using legacy OAuth metadata fallback for {AuthorizationServer} because ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallback is enabled. PKCE support could not be verified. Use this switch only for trusted deployments whose PKCE support has been independently established.")] + partial void LogUsingLegacyOAuthMetadataFallback(Uri authorizationServer); + [LoggerMessage(Level = LogLevel.Information, Message = "Performing dynamic client registration with {RegistrationEndpoint}")] partial void LogPerformingDynamicClientRegistration(Uri registrationEndpoint); diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs index c4979fb10..25ef4d25c 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using ModelContextProtocol; using ModelContextProtocol.AspNetCore.Authentication; using ModelContextProtocol.Authentication; @@ -19,9 +20,11 @@ namespace ModelContextProtocol.AspNetCore.Tests.OAuth; +[Collection(nameof(DisableParallelization))] public class AuthTests : OAuthTestBase { private const string ClientMetadataDocumentUrl = $"{OAuthServerUrl}/client-metadata/cimd-client.json"; + private const string LegacyOAuthMetadataFallbackSwitch = "ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallback"; public AuthTests(ITestOutputHelper outputHelper) : base(outputHelper) @@ -131,6 +134,142 @@ public async Task CanAuthenticate_WithClientMetadataDocument() transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); } + [Theory] + [InlineData(null)] + [InlineData(false)] + [InlineData(true)] + public async Task CannotAuthenticate_WhenMetadataOmitsPkceMethods(bool? allowLegacyFallback) + { + using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); + TestOAuthServer.CodeChallengeMethodsSupported = null; + await using var app = await StartMcpServerAsync(); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + + // No discovery endpoint advertises PKCE, so metadata discovery is exhausted. The precise PKCE reason + // is logged as each endpoint is skipped. + Assert.Contains( + MockLoggerProvider.LogMessages, + m => m.Exception?.Message.Contains("code_challenge_methods_supported") == true); + } + + [Theory] + [InlineData(null)] + [InlineData(false)] + [InlineData(true)] + public async Task CannotAuthenticate_WhenMetadataLacksS256PkceMethod(bool? allowLegacyFallback) + { + using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); + TestOAuthServer.CodeChallengeMethodsSupported = ["plain"]; + await using var app = await StartMcpServerAsync(); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + + Assert.Contains( + MockLoggerProvider.LogMessages, + m => m.Exception?.Message.Contains("required PKCE method 'S256'") == true); + } + + [Theory] + [InlineData(null)] + [InlineData(false)] + [InlineData(true)] + public async Task CanAuthenticate_WhenFirstMetadataEndpointOmitsPkce_ButAnotherAdvertisesIt(bool? allowLegacyFallback) + { + using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); + // The OAuth 2.0 authorization server metadata endpoint is tried before the OpenID Connect one. + // Simulate a server where only the OpenID Connect document advertises PKCE support, and verify the + // client falls through to it rather than failing on the first PKCE-less document. + TestOAuthServer.MetadataPathsWithoutPkceSupport.Add("/.well-known/oauth-authorization-server"); + await using var app = await StartMcpServerAsync(); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + await using var client = await McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.Contains("/.well-known/oauth-authorization-server", TestOAuthServer.MetadataRequests); + Assert.Contains("/.well-known/openid-configuration", TestOAuthServer.MetadataRequests); + Assert.Equal(1, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(1, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + } + + [Theory] + [InlineData(null)] + [InlineData(false)] + [InlineData(true)] + public async Task CannotAuthenticate_WhenAuthorizationServerMetadataIsUnavailable_WithProtectedResourceMetadata(bool? allowLegacyFallback) + { + using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); + TestOAuthServer.DisabledMetadataPaths.Add("/.well-known/oauth-authorization-server"); + TestOAuthServer.DisabledMetadataPaths.Add("/.well-known/openid-configuration"); + await using var app = await StartMcpServerAsync(); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + } + [Fact] public async Task UsesDynamicClientRegistration_WhenCimdNotSupported() { @@ -692,6 +831,135 @@ public async Task CanAuthenticate_WithAuthorizationServerPathFallbacks() TestOAuthServer.MetadataRequests); } + [Theory] + [InlineData(OAuthServerUrl)] + [InlineData(OAuthServerUrl + "/tenant")] + public async Task CanAuthenticate_WhenAuthorizationServerMetadataIssuerMatches(string expectedIssuer) + { + Builder.Services.Configure(McpAuthenticationDefaults.AuthenticationScheme, options => + { + options.ResourceMetadata!.AuthorizationServers = [expectedIssuer]; + }); + + await using var app = await StartMcpServerAsync(); + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + await using var client = await McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal(1, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(1, TestOAuthServer.TokenRequestCount); + } + + [Theory] + [InlineData(OAuthServerUrl, "https://attacker.example")] + [InlineData(OAuthServerUrl, OAuthServerUrl + "/")] + [InlineData(OAuthServerUrl, "HTTPS://LOCALHOST:7029")] + [InlineData(OAuthServerUrl + "/tenant", OAuthServerUrl + "/%74enant")] + public async Task CannotAuthenticate_WhenAuthorizationServerMetadataIssuerMismatches(string expectedIssuer, string metadataIssuer) + { + using var switchScope = new LegacyFallbackSwitchScope(true); + TestOAuthServer.MetadataIssuerOverride = metadataIssuer; + Builder.Services.Configure(McpAuthenticationDefaults.AuthenticationScheme, options => + { + options.ResourceMetadata!.AuthorizationServers = [expectedIssuer]; + }); + + await using var app = await StartMcpServerAsync(); + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + var ex = await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Contains("does not match the expected issuer", ex.Message); + Assert.Single(TestOAuthServer.MetadataRequests); + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + } + + [Fact] + public async Task CannotAuthenticate_WhenAuthorizationServerMetadataOmitsIssuer() + { + using var switchScope = new LegacyFallbackSwitchScope(true); + TestOAuthServer.IncludeIssuerInMetadata = false; + + await using var app = await StartMcpServerAsync(); + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + var ex = await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Contains("did not provide the required issuer", ex.Message); + Assert.Single(TestOAuthServer.MetadataRequests); + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + } + + [Fact] + public async Task AuthorizationRedirectDelegate_DoesNotSkipMetadataIssuerValidation() + { + TestOAuthServer.MetadataIssuerOverride = "https://attacker.example"; + var authorizationRedirectCalled = false; + await using var app = await StartMcpServerAsync(); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = (uri, redirect, ct) => + { + authorizationRedirectCalled = true; + return HandleAuthorizationUrlAsync(uri, redirect, ct); + }, + }, + }, HttpClient, LoggerFactory); + + var ex = await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Contains("does not match the expected issuer", ex.Message); + Assert.False(authorizationRedirectCalled); + Assert.Single(TestOAuthServer.MetadataRequests); + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + } + [Fact] public async Task CanAuthenticate_WithResourceMetadataPathFallbacks() { @@ -1045,8 +1313,10 @@ public async Task ResourceMetadata_PreservesExplicitTrailingSlash() transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); } - [Fact] - public async Task CanAuthenticate_WithLegacyServerWithoutProtectedResourceMetadata() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task CanAuthenticate_WithLegacyServerWithoutProtectedResourceMetadata(bool includeIssuer) { // 2025-03-26 backcompat: server does NOT serve PRM, but DOES serve auth server metadata. // The client should fall back to using the MCP server's origin as the auth server @@ -1084,7 +1354,7 @@ public async Task CanAuthenticate_WithLegacyServerWithoutProtectedResourceMetada context.Response.ContentType = "application/json"; await context.Response.WriteAsync($$""" { - "issuer": "{{OAuthServerUrl}}", + {{(includeIssuer ? $"\"issuer\": \"{OAuthServerUrl}\"," : "")}} "authorization_endpoint": "{{McpServerUrl}}/authorize", "token_endpoint": "{{McpServerUrl}}/token", "registration_endpoint": "{{McpServerUrl}}/register", @@ -1158,14 +1428,23 @@ await context.Response.WriteAsync($$""" await using var client = await McpClient.CreateAsync( transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal(1, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(1, TestOAuthServer.TokenRequestCount); } - [Fact] - public async Task CanAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() + [Theory] + [InlineData(null)] + [InlineData(false)] + [InlineData(true)] + public async Task LegacyServerUsingDefaultEndpointFallback_RequiresExplicitOptIn(bool? allowLegacyFallback) { + using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); + Assert.Equal(allowLegacyFallback.HasValue, AppContext.TryGetSwitch(LegacyOAuthMetadataFallbackSwitch, out var enabled)); + Assert.Equal(allowLegacyFallback.GetValueOrDefault(), enabled); + // 2025-03-26 backcompat: server does NOT serve PRM AND does NOT serve auth server metadata. - // The client should fall back to default endpoint paths (/authorize, /token, /register) - // on the MCP server's origin. + // Only explicitly opted-in clients can use default endpoint paths on the MCP server's origin. TestOAuthServer.ExpectResource = false; // Use JwtBearer as the challenge scheme so the 401 response does NOT include resource_metadata. @@ -1179,6 +1458,9 @@ public async Task CanAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() await using var app = Builder.Build(); + var oauthRequests = 0; + var authorizationRedirectCalled = false; + // Capture HttpClient for use in the proxy middleware. var httpClient = HttpClient; @@ -1204,6 +1486,7 @@ public async Task CanAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() var path = context.Request.Path.Value; if (path is "/authorize" or "/token" or "/register") { + Interlocked.Increment(ref oauthRequests); var targetUrl = $"{OAuthServerUrl}{path}{context.Request.QueryString}"; using var proxyRequest = new HttpRequestMessage(new HttpMethod(context.Request.Method), targetUrl); @@ -1246,6 +1529,109 @@ public async Task CanAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() app.MapMcp().RequireAuthorization(); await app.StartAsync(TestContext.Current.CancellationToken); + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = (uri, redirect, ct) => + { + authorizationRedirectCalled = true; + return HandleAuthorizationUrlAsync(uri, redirect, ct); + }, + }, + }, HttpClient, LoggerFactory); + + if (allowLegacyFallback == true) + { + await using var client = await McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.True(authorizationRedirectCalled); + Assert.Equal(2, oauthRequests); + Assert.Equal(1, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(1, TestOAuthServer.TokenRequestCount); + Assert.Contains(MockLoggerProvider.LogMessages, + m => m.LogLevel == LogLevel.Warning && m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + } + else + { + var ex = await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Contains("PKCE support could not be verified", ex.Message); + Assert.False(authorizationRedirectCalled); + Assert.Equal(0, oauthRequests); + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + } + } + + [Theory] + [InlineData(null, false)] + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task CannotAuthenticate_WithLegacyServerWhoseMetadataLacksPkceSupport(bool? allowLegacyFallback, bool advertisePlain) + { + using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); + // 2025-03-26 backcompat regression guard: PRM is unavailable (resourceUri is null), but the server + // DOES serve an auth server metadata document that omits PKCE support or advertises only plain. + // The client must refuse to proceed rather than falling back to synthesized S256 defaults, since a + // discovered metadata document that fails PKCE validation disqualifies the legacy default fallback. + TestOAuthServer.ExpectResource = false; + + // Use JwtBearer as the challenge scheme so the 401 response does NOT include resource_metadata. + Builder.Services.Configure(options => options.DefaultChallengeScheme = JwtBearerDefaults.AuthenticationScheme); + + Builder.Services.Configure(JwtBearerDefaults.AuthenticationScheme, options => + { + options.TokenValidationParameters.ValidateAudience = false; + }); + + await using var app = Builder.Build(); + + app.Use(async (context, next) => + { + // Return 404 for PRM to simulate a legacy server that doesn't support RFC 9728. + if (context.Request.Path.StartsWithSegments("/.well-known/oauth-protected-resource")) + { + context.Response.StatusCode = StatusCodes.Status404NotFound; + return; + } + + // Serve metadata that omits PKCE support or advertises only plain. + if (context.Request.Path.StartsWithSegments("/.well-known/oauth-authorization-server") || + context.Request.Path.StartsWithSegments("/.well-known/openid-configuration")) + { + context.Response.ContentType = "application/json"; + await context.Response.WriteAsync($$""" + { + "issuer": "{{OAuthServerUrl}}", + {{(advertisePlain ? "\"code_challenge_methods_supported\": [\"plain\"]," : "")}} + "authorization_endpoint": "{{McpServerUrl}}/authorize", + "token_endpoint": "{{McpServerUrl}}/token", + "registration_endpoint": "{{McpServerUrl}}/register", + "response_types_supported": ["code"], + "grant_types_supported": ["authorization_code", "refresh_token"], + "token_endpoint_auth_methods_supported": ["client_secret_post"] + } + """); + return; + } + + await next(); + }); + + app.UseAuthentication(); + app.UseAuthorization(); + app.MapMcp().RequireAuthorization(); + await app.StartAsync(TestContext.Current.CancellationToken); + await using var transport = new HttpClientTransport(new() { Endpoint = new(McpServerUrl), @@ -1258,7 +1644,27 @@ public async Task CanAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() }, }, HttpClient, LoggerFactory); - await using var client = await McpClient.CreateAsync( - transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + var ex = await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + // The specific PKCE failure reason is surfaced rather than a generic discovery failure or a + // silently-synthesized S256 fallback. + Assert.Contains("code_challenge_methods_supported", ex.Message); + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + } + + private sealed class LegacyFallbackSwitchScope : IDisposable + { + private readonly object? _previousValue = AppContext.GetData(LegacyOAuthMetadataFallbackSwitch); + + public LegacyFallbackSwitchScope(bool? enabled) + { + // Switch data can be removed again, unlike a value installed with AppContext.SetSwitch. + AppContext.SetData(LegacyOAuthMetadataFallbackSwitch, enabled?.ToString()); + } + + public void Dispose() => AppContext.SetData(LegacyOAuthMetadataFallbackSwitch, _previousValue); } } diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/TestAttributes.cs b/tests/ModelContextProtocol.AspNetCore.Tests/TestAttributes.cs new file mode 100644 index 000000000..83a5192d3 --- /dev/null +++ b/tests/ModelContextProtocol.AspNetCore.Tests/TestAttributes.cs @@ -0,0 +1,4 @@ +namespace ModelContextProtocol.AspNetCore.Tests; + +[CollectionDefinition(nameof(DisableParallelization), DisableParallelization = true)] +public sealed class DisableParallelization; diff --git a/tests/ModelContextProtocol.ConformanceClient/Program.cs b/tests/ModelContextProtocol.ConformanceClient/Program.cs index b5e048dd0..19d954a18 100644 --- a/tests/ModelContextProtocol.ConformanceClient/Program.cs +++ b/tests/ModelContextProtocol.ConformanceClient/Program.cs @@ -19,6 +19,12 @@ var scenario = args[0]; var endpoint = args[1]; +if (scenario == "auth/2025-03-26-oauth-endpoint-fallback") +{ + // Only the trusted legacy conformance fixture opts into metadata-free endpoint discovery. + AppContext.SetSwitch("ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallback", true); +} + McpClientOptions options = new() { ClientInfo = new() diff --git a/tests/ModelContextProtocol.TestOAuthServer/OAuthServerMetadata.cs b/tests/ModelContextProtocol.TestOAuthServer/OAuthServerMetadata.cs index c05a45ba2..c1f834fb7 100644 --- a/tests/ModelContextProtocol.TestOAuthServer/OAuthServerMetadata.cs +++ b/tests/ModelContextProtocol.TestOAuthServer/OAuthServerMetadata.cs @@ -12,7 +12,8 @@ internal sealed class OAuthServerMetadata /// REQUIRED. The authorization server's issuer identifier, which is a URL that uses the "https" scheme and has no query or fragment components. /// [JsonPropertyName("issuer")] - public required string Issuer { get; init; } + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public string? Issuer { get; init; } /// /// Gets or sets the authorization endpoint URL. diff --git a/tests/ModelContextProtocol.TestOAuthServer/Program.cs b/tests/ModelContextProtocol.TestOAuthServer/Program.cs index 1da81c2a7..3ba6e9e92 100644 --- a/tests/ModelContextProtocol.TestOAuthServer/Program.cs +++ b/tests/ModelContextProtocol.TestOAuthServer/Program.cs @@ -28,6 +28,8 @@ public sealed class Program private readonly ConcurrentDictionary _clients = new(); private readonly ConcurrentQueue _metadataRequests = new(); + private int _authorizationRequestCount; + private int _tokenRequestCount; private readonly RSA _rsa; private readonly string _keyId; @@ -90,9 +92,39 @@ public Program(ILoggerProvider? loggerProvider = null, IConnectionListenerFactor /// public bool ExpectResource { get; set; } = true; + /// + /// Gets or sets a value indicating whether authorization server metadata includes an issuer. + /// + public bool IncludeIssuerInMetadata { get; set; } = true; + + /// + /// Gets or sets an issuer value that overrides the authorization server's metadata issuer. + /// + public string? MetadataIssuerOverride { get; set; } + + /// + /// Gets or sets the code challenge methods advertised by metadata endpoints. + /// + /// + /// The default value is ["S256"]. + /// + public List? CodeChallengeMethodsSupported { get; set; } = ["S256"]; + + /// + /// Gets the set of metadata paths that should omit code_challenge_methods_supported from their + /// response, simulating a server whose discovery endpoints advertise differing PKCE support. + /// + public HashSet MetadataPathsWithoutPkceSupport { get; } = new(StringComparer.OrdinalIgnoreCase); + public HashSet DisabledMetadataPaths { get; } = new(StringComparer.OrdinalIgnoreCase); public IReadOnlyCollection MetadataRequests => _metadataRequests.ToArray(); + /// Gets the number of requests received by the authorization endpoint. + public int AuthorizationRequestCount => Volatile.Read(ref _authorizationRequestCount); + + /// Gets the number of requests received by the token endpoint. + public int TokenRequestCount => Volatile.Read(ref _tokenRequestCount); + /// /// Entry point for the application. /// @@ -212,7 +244,7 @@ IResult HandleMetadataRequest(HttpContext context, string? issuerPath = null) var metadata = new OAuthServerMetadata { - Issuer = $"{_url}{issuerPath}", + Issuer = IncludeIssuerInMetadata ? MetadataIssuerOverride ?? $"{_url}{issuerPath}" : null, AuthorizationEndpoint = $"{_url}/authorize", TokenEndpoint = $"{_url}/token", JwksUri = $"{_url}/.well-known/jwks.json", @@ -222,7 +254,9 @@ IResult HandleMetadataRequest(HttpContext context, string? issuerPath = null) ScopesSupported = ["openid", "profile", "email", "mcp:tools"], TokenEndpointAuthMethodsSupported = ["client_secret_post"], ClaimsSupported = ["sub", "iss", "name", "email", "aud"], - CodeChallengeMethodsSupported = ["S256"], + CodeChallengeMethodsSupported = MetadataPathsWithoutPkceSupport.Contains(context.Request.Path) + ? null + : CodeChallengeMethodsSupported, GrantTypesSupported = ["authorization_code", "refresh_token"], IntrospectionEndpoint = $"{_url}/introspect", RegistrationEndpoint = $"{_url}/register", @@ -284,6 +318,8 @@ IResult HandleMetadataRequest(HttpContext context, string? issuerPath = null) [FromQuery] string? state, [FromQuery] string? resource) => { + Interlocked.Increment(ref _authorizationRequestCount); + // Validate client if (!_clients.TryGetValue(client_id, out var client)) { @@ -365,6 +401,8 @@ IResult HandleMetadataRequest(HttpContext context, string? issuerPath = null) // Token endpoint app.MapPost("/token", async (HttpContext context) => { + Interlocked.Increment(ref _tokenRequestCount); + var form = await context.Request.ReadFormAsync(); // Authenticate client