Skip to content

[Server] feat: relax StrictOidcDiscoveryMetadataPolicy and add Dynamic Client Registration middleware (RFC 7591)#269

Open
simonchrz wants to merge 5 commits intomodelcontextprotocol:mainfrom
simonchrz:main
Open

[Server] feat: relax StrictOidcDiscoveryMetadataPolicy and add Dynamic Client Registration middleware (RFC 7591)#269
simonchrz wants to merge 5 commits intomodelcontextprotocol:mainfrom
simonchrz:main

Conversation

@simonchrz
Copy link
Copy Markdown
Contributor

Add two features to the HTTP transport layer:

  1. LenientOidcDiscoveryMetadataPolicy — an alternative OIDC discovery metadata validation policy that does not require
    code_challenge_methods_supported
  2. ClientRegistrationMiddleware — PSR-15 middleware implementing OAuth 2.0 Dynamic Client Registration (RFC 7591)

Motivation and Context

LenientOidcDiscoveryMetadataPolicy: Several identity providers (FusionAuth, Microsoft Entra ID) omit code_challenge_methods_supported from
their OIDC discovery response despite fully supporting PKCE with S256. The existing OidcDiscoveryMetadataPolicy rejects these responses,
making it impossible to use the SDK's OAuth transport with those providers without a custom policy. This provides a ready-made workaround.

ClientRegistrationMiddleware: The MCP specification requires servers to support OAuth 2.0 Dynamic Client Registration (RFC 7591) so that MCP
clients can register themselves automatically. This middleware handles POST /register by delegating to a ClientRegistrarInterface
implementation and enriches /.well-known/oauth-authorization-server responses with the registration_endpoint. The ClientRegistrarInterface
keeps the actual registration logic (e.g. calling FusionAuth, storing in a database) pluggable.

How Has This Been Tested?

  • Unit tests: 16 tests covering both features
    • LenientOidcDiscoveryMetadataPolicyTest (7 cases via data providers): valid metadata with/without code_challenge_methods_supported, missing
      required fields, empty strings, non-array input
    • ClientRegistrationMiddlewareTest (9 cases): successful registration (201), invalid JSON (400), registrar exception (400), metadata
      enrichment with registration_endpoint, Cache-Control preservation, non-200 passthrough, non-matching route passthrough, empty localBaseUrl
      rejection, trailing slash normalization
  • Tested in a real Symfony application against FusionAuth as the identity provider (the motivation for these changes)

Breaking Changes

None. Both features are purely additive — new classes and interfaces only.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the https://modelcontextprotocol.io
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

  • ClientRegistrarInterface::register() accepts and returns array<string, mixed> to stay flexible with RFC 7591's extensible metadata fields
  • ClientRegistrationMiddleware follows the same patterns as OAuthProxyMiddleware (PSR-17 factory discovery, optional DI overrides, anonymous
    handler classes in tests)
  • The registration path (/register) is extracted to a class constant for consistency
  • The middleware rewinds the response stream before reading in enrichAuthServerMetadata to handle cases where the stream cursor was already
    advanced

@sveneld
Copy link
Copy Markdown
Contributor

sveneld commented Mar 17, 2026

There is an examples for microsoft connection. There is a trouble not only with OIDC discovery.

https://github.com/modelcontextprotocol/php-sdk/blob/main/examples/server/oauth-microsoft/MicrosoftOidcMetadataPolicy.php
https://github.com/modelcontextprotocol/php-sdk/blob/main/examples/server/oauth-microsoft/MicrosoftJwtTokenValidator.php

Examples should be also updated if alternative OIDC discovery is accepted.

There is also alternative way - in case when code_challenge_methods_supported is missed we can set it's value to default S256

@simonchrz
Copy link
Copy Markdown
Contributor Author

There is an examples for microsoft connection. There is a trouble not only with OIDC discovery.

https://github.com/modelcontextprotocol/php-sdk/blob/main/examples/server/oauth-microsoft/MicrosoftOidcMetadataPolicy.php https://github.com/modelcontextprotocol/php-sdk/blob/main/examples/server/oauth-microsoft/MicrosoftJwtTokenValidator.php

Examples should be also updated if alternative OIDC discovery is accepted.

There is also alternative way - in case when code_challenge_methods_supported is missed we can set it's value to default S256

Good point — I've removed LenientOidcDiscoveryMetadataPolicy and relaxed StrictOidcDiscoveryMetadataPolicy to accept missing code_challenge_methods_supported. If the field is present it's still validated strictly; if absent, the downstream OAuthProxyMiddleware already defaults to ['S256']. The interface stays for custom policies (like the Microsoft example).

@simonchrz simonchrz closed this Mar 18, 2026
@simonchrz simonchrz reopened this Mar 18, 2026
@simonchrz simonchrz changed the title feat: add LenientOidcDiscoveryMetadataPolicy and Dynamic Client Registration middleware (RFC 7591) feat: relax StrictOidcDiscoveryMetadataPolicy and add Dynamic Client Registration middleware (RFC 7591) Mar 18, 2026
@chr-hertel
Copy link
Copy Markdown
Member

is the MicrosoftOidcMetadataPolicy still needed then?

@simonchrz
Copy link
Copy Markdown
Contributor Author

is the MicrosoftOidcMetadataPolicy still needed then?

i guess just as an example how to implement OidcDiscoveryMetadataPolicyInterface

@sveneld
Copy link
Copy Markdown
Contributor

sveneld commented Mar 29, 2026

I thought about this more, and I think my earlier comment was not precise enough.

I agree we should support real-world providers like Microsoft that omit code_challenge_methods_supported, but I don’t think this behavior should live in
StrictOidcDiscoveryMetadataPolicy.

I would prefer to:

  • keep StrictOidcDiscoveryMetadataPolicy RFC-aligned and document it as such
  • add a separate LenientOidcDiscoveryMetadataPolicy for non-compliant providers
  • update the Microsoft example to use the built-in lenient policy instead of its own custom one

That keeps the naming and behavior clearer: Strict stays strict, and the compatibility workaround is explicit.

@Spomky
Copy link
Copy Markdown

Spomky commented Mar 30, 2026

Hi,

The goal of the MCP SDK should be to implement the MCP protocol (JSON-RPC, tools, resources, prompts); not to become an OAuth server.

The spec says the server must support OAuth, but that doesn't mean the SDK itself needs to embed a full OAuth implementation. Bundling Dynamic Client Registration (RFC 7591) directly into the SDK is a nonsense to me.

@chr-hertel
Copy link
Copy Markdown
Member

Hi @Spomky,
thanks for checking in here - very much appreciated!
I agree that the SDKs are sometimes a bit opinionated and take assumptions that are already solved on framework level, but that's not that specific for the PHP SDK, see TS or Python. We try to have an open approach here tho - rather opt-in. No matter if discovering tools/resources/etc, session management, or that middleware auth default.

Providing something out of the box doesn't mean that we have to use it in context of Symfony Framework, Laravel etc.
On the other hand I'd like to limit it to some reasonable defaults here - maybe also on top of a framework agnostic lib, which is optional then.

@simonchrz simonchrz requested a review from soyuka as a code owner March 30, 2026 13:28
@chr-hertel chr-hertel added this to the 0.5.0 milestone Mar 30, 2026
@chr-hertel chr-hertel requested a review from Copilot March 30, 2026 20:07
@chr-hertel chr-hertel added the Server Issues & PRs related to the Server component label Mar 30, 2026
@chr-hertel chr-hertel changed the title feat: relax StrictOidcDiscoveryMetadataPolicy and add Dynamic Client Registration middleware (RFC 7591) [Server] feat: relax StrictOidcDiscoveryMetadataPolicy and add Dynamic Client Registration middleware (RFC 7591) Mar 30, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds HTTP-transport OAuth enhancements to improve compatibility with real-world OIDC providers and to support OAuth 2.0 Dynamic Client Registration as required by MCP servers.

Changes:

  • Introduce LenientOidcDiscoveryMetadataPolicy as an alternative to the strict discovery metadata validator.
  • Add ClientRegistrationMiddleware (PSR-15) + ClientRegistrarInterface to implement RFC 7591 registration handling and metadata enrichment.
  • Update the Microsoft OAuth example to use the new built-in lenient policy and remove the example-specific policy.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/Unit/Server/Transport/Http/OAuth/OidcDiscoveryTest.php Adds coverage ensuring lenient policy accepts missing code_challenge_methods_supported.
tests/Unit/Server/Transport/Http/OAuth/LenientOidcDiscoveryMetadataPolicyTest.php New unit tests for lenient discovery metadata validation rules.
tests/Unit/Server/Transport/Http/Middleware/ClientRegistrationMiddlewareTest.php New unit tests for RFC 7591 middleware behavior and metadata enrichment.
src/Server/Transport/Http/OAuth/LenientOidcDiscoveryMetadataPolicy.php New metadata policy that relaxes the strict requirement for PKCE method listing.
src/Server/Transport/Http/OAuth/ClientRegistrarInterface.php New SPI for pluggable dynamic client registration implementations.
src/Server/Transport/Http/Middleware/ClientRegistrationMiddleware.php New PSR-15 middleware handling /register and enriching auth server metadata.
src/Exception/ClientRegistrationException.php New exception type for registrar failures.
examples/server/oauth-microsoft/tests/Unit/MicrosoftOidcMetadataPolicyTest.php Removes example-specific tests tied to the removed policy.
examples/server/oauth-microsoft/server.php Switches example to LenientOidcDiscoveryMetadataPolicy.
examples/server/oauth-microsoft/README.md Updates documentation to match the new built-in policy usage.
examples/server/oauth-microsoft/MicrosoftOidcMetadataPolicy.php Removes the example-specific metadata policy implementation.
CHANGELOG.md Documents the two new additive features.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@chr-hertel chr-hertel added the needs more work Not ready to be merged yet, needs additional follow-up from the author(s). label Mar 30, 2026
@simonchrz simonchrz requested a review from Copilot March 31, 2026 06:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

… middleware (RFC 7591)

- Add LenientOidcDiscoveryMetadataPolicy for providers that omit
  code_challenge_methods_supported (e.g. FusionAuth, Microsoft Entra ID)
- Keep StrictOidcDiscoveryMetadataPolicy RFC-aligned
- Add ClientRegistrationMiddleware handling POST /register and enriching
  /.well-known/oauth-authorization-server with registration_endpoint
- Update Microsoft example to use built-in LenientOidcDiscoveryMetadataPolicy
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Address review comment: the LenientOidcDiscoveryMetadataPolicy bullet
was a behavioral note, not a file entry. Inline it into the server.php
description to keep the Files list consistent.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Decode JSON with assoc=true for registrar data so nested objects
  are associative arrays, not stdClass instances
- Add Cache-Control: no-store to all error responses (400), not just
  the success response (201)
- Rewind response body stream before returning unmodified response
  when metadata is not valid JSON, preventing empty body on read
- Revert spurious .gitignore trailing newline
- Use JSON_THROW_ON_ERROR consistently in enrichAuthServerMetadata()
- Add comment clarifying the second json_decode in handleRegistration()
- Assert Cache-Control: no-store on registrar exception error response
- Add test for non-object JSON body in metadata enrichment path
- Extract createPlainTextHandler helper for non-JSON response tests
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Treat JSON list arrays (e.g. [] or ["..."]) as invalid metadata in
enrichAuthServerMetadata() to prevent corrupting the response shape
by adding registration_endpoint to a non-object.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs more work Not ready to be merged yet, needs additional follow-up from the author(s). Server Issues & PRs related to the Server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants