Repository navigation
Conversation
Keep the release/1.x OAuth client fail-closed when PKCE support cannot be verified from authorization server metadata. Retain the historical no-metadata fallback only behind an explicit process-wide compatibility switch, with a warning and deployment requirements. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This was referenced Oct 22, 2026
Adapt the metadata-only checks and fail-fast McpException handling from #1605 (4aade88), preserving its 2025-03-26 legacy exemption. Add exact-string matching and pre-authorization rejection coverage without introducing response-aware callbacks or RFC 9207 response validation. Update the test-only conformance pin from 0.1.13 to 0.1.15 for corrected tenant issuer fixtures (modelcontextprotocol/conformance#152; 50c3cf1a430274ebdaaddeb95ef2c786a6bca290). Keep the PKCE requirements and fallback switch unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The pinned conformance 0.1.15 bundle imports fs.globSync, which is unavailable in Node 20. Align the build/test workflow with the Node 22 runtime on main without changing the setup-node action, dependency pins, SDK behavior, or test expectations. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
tarekgh
reviewed
Oct 23, 2026
tarekgh
approved these changes
Oct 23, 2026
tarekgh
left a comment
Contributor
There was a problem hiding this comment.
I left a minor comment, LGTM, otherwise!
Undo the incidental worktree-directory name captured by npm during the conformance dependency update. Preserve every dependency entry. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backport #1700 (7369a26) to
release/1.x, with an explicitly approved 1.x security-default adaptation for metadata-free legacy servers. Also backport only the RFC 8414 authorization-server metadata-issuer checks from #1605 (4aade88), preserving its intentional legacy exemption.The first commit preserves #1700's production change and four regression tests, with cherry-pick attribution. Two test-file conflicts were resolved without importing unrelated scope-selection/offline-access changes. The second commit adds the opt-in compatibility switch described below. The third adds the source-equivalent metadata-issuer checks, related tests/documentation, and the approved test-only conformance package update needed for compliant issuer fixtures. A follow-up changes only the build/test workflow's Node pin from 20 to 22 for the conformance bundle's runtime prerequisite.
S256incode_challenge_methods_supported; no longer synthesize that field for discovered documents.issuerto exactly match the selected authorization server identifier usingOriginalStringandStringComparison.Ordinal. Missing/mismatched issuers immediately stop discovery before authorization or token exchange; they are not swallowed, retried against another endpoint, or replaced by synthetic defaults.This supersedes the proposed state-validation backport #1948 with a focused 1.x approach, rather than importing the callback architecture from #1605/#1726.
Deliberate 1.x security-default adaptation
Original #1700 retains the MCP 2025-03-26 default-endpoint fallback when protected resource metadata is unavailable and authorization server metadata discovery finds no usable document. That fallback synthesizes
S256, which does not establish PKCE support. Because the existing 1.x callback API returns only a code and cannot provide response state for SDK validation, this backport makes that fallback disabled by default.ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallbackis a default-off, process-wide AppContext switch. When absent orfalse, the client rejects that metadata-free path before requesting an authorization code or exchanging tokens. A host can explicitly restore only the original eligible fallback at startup:The switch retains the original
resourceUri is nullguard,/authorize,/token, and/registerdefaults, and unchangedS256challenge/verifier behavior. It cannot bypass discovered metadata that omitsS256, and the client logs a warning whenever it actually uses this unverified fallback.Enabling the switch is not verification of PKCE support and does not add state validation. Hosts must independently establish that a trusted deployment correctly enforces PKCE, including downgrade defenses. This is not a per-server allowlist: it affects all OAuth clients in the process. Do not enable it in a process that also connects to untrusted authorization servers or toggle it around individual requests.
The API remarks, transport guide, and protected-client sample documentation explain configuration and security requirements. The conformance client opts in only for its trusted
auth/2025-03-26-oauth-endpoint-fallbackfixture; other scenarios retain the secure default.Security rationale
RFC 9700 §2.1 explicitly states that clients which have ensured authorization-server PKCE support "MAY rely on the CSRF protection provided by PKCE". §4.7.1 describes PKCE as an alternative to
statefor CSRF protection and requires clients to establish server support before relying on it. This backport strengthens that prerequisite for existing code-only callbacks without requiring a new callback API or SDK response-state validation.This verifies advertised support, not the server's actual verifier or downgrade implementation. Authorization servers must correctly enforce PKCE, including the §4.8.2 downgrade defenses. This is an alternative for the CSRF protection requirement, not a claim that state validation is universally unnecessary or that this approach is security-equivalent to every state-validation design.
Metadata issuer validation is included; response issuer validation remains deferred
RFC 8414 §3.3 requires the metadata issuer to be identical to the issuer identifier used for discovery. This PR includes #1605's metadata-only checks without importing its response-aware callback API. Comparisons do not normalize URI case, trailing slashes, or percent-encoding, so previously accepted equivalent-looking issuer strings may now be rejected. The source's
McpExceptionrethrow path is retained: metadata validation failures stop discovery immediately instead of being logged and silently retried.As in #1605, these issuer checks are exempt when
resourceUriis null in the MCP 2025-03-26 legacy path: discovery uses the MCP server's origin rather than an authorization server identifier obtained from protected resource metadata, so it need not equal the canonical issuer. PKCE requirements and the default-off fallback switch remain unchanged.RFC 9207 authorization-response
issvalidation addresses authorization-server mix-up, a separate threat; PKCE is not a substitute for issuer validation. That response check still requires response-aware callback plumbing and remains deferred for servicing scope. Including metadata checks is not a claim that response-issuer validation is unnecessary or that 1.x clients are immune to mix-up attacks.Test fixture dependency update
The test-only
@modelcontextprotocol/conformancepin is updated from0.1.13to0.1.15, the earliest stable published release correcting the tenant issuer fixtures. In0.1.13,auth/metadata-var2andauth/metadata-var3advertise an authorization server identifier ending in/tenant1but return a root metadata issuer, which the new SDK checks correctly reject. modelcontextprotocol/conformance#152, commit 50c3cf1, fixes the server-side issuer to includeroutePrefixwhile preserving the tested discovery paths.This is not an SDK/client validation bypass or a rewritten fixture. The manifest changes one pin; npm updates its lock entry and adds 20 transitive package entries for the new Octokit dependency chain, leaving existing other package versions unchanged. The newer harness also includes scenario specification-version annotations and a shared auth fixture fix that creates a fresh stateless MCP server per request. The existing selected scenarios and full suite pass without unrelated SDK behavior fixes, further upgrades, skipped scenarios, or weakened assertions.
CI runtime prerequisite: The
0.1.15conformance bundle statically importsfs.globSync. The original Node 20 CI pin cannot load it and fails withSyntaxError: The requested module 'fs' does not provide an export named 'globSync'before the conformance tests start; this is not an issuer-validation regression. Earlier local validation used Node 24.20.0, where that import works, and therefore missed the CI runtime mismatch. The follow-up changes onlynode-version: '20'to'22'in.github/workflows/ci-build-test.yml, matching main. The pinned setup-node action, other workflows, npm/dependency pins, SDK behavior, and test expectations are unchanged.Release note
Compatibility tightening: OAuth authentication now requires discovered authorization server metadata to advertise
S256PKCE support, even if the server implements PKCE without advertising it. When protected resource metadata is available, authorization server metadata must also provide anissuerexactly matching the selected authorization server identifier, without normalizing case, trailing slashes, or percent-encoding. Metadata validation failures immediately stop discovery before authorization or token exchange. The original 2025-03-26 metadata-issuer exemption is preserved when protected resource metadata is unavailable; RFC 9207 authorization-response issuer validation is not added.Metadata-free legacy authentication is also rejected by default. Trusted 2025-03-26 deployments with independently established PKCE support can explicitly restore the historical no-metadata fallback using the process-wide
ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallbackAppContext switch; enabling it emits a warning and never bypasses PKCE validation of discovered metadata.Validation
--filter 'FullyQualifiedName~Conformance') ran under a checksum-verified, process-local Node 22.23.4, not the shared host's Node 24: 72 passed, 2 existing Windows skips on each of .NET 8, 9, and 10 in both configurations (432 passed, 12 skips across the six runs). Thefs.globSyncmodule import and conformance CLI startup also succeeded. The portable runtime was removed afterward; shared machine settings and the host Node version were not changed.Local process tests used a test-process-only current-directory PATH entry for Windows executable lookup. npm installed the approved pinned test-dependency update without lifecycle scripts. No machine-wide settings or repository test behavior were weakened.