[Server] feat: relax StrictOidcDiscoveryMetadataPolicy and add Dynamic Client Registration middleware (RFC 7591) (#269)
* feat: relax OIDC discovery policy and add Dynamic Client Registration 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 * fix: move metadata policy note under server.php in Files section 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. * fix: address Copilot review comments on ClientRegistrationMiddleware - 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 * fix: harden ClientRegistrationMiddleware and improve test coverage - 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 * fix: reject JSON array bodies in metadata enrichment 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. * fix: add RFC 7591 error codes, Content-Type validation, and docblock safety warning Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Christopher Hertel <mail@christopher-hertel.de> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
simon chrzanowski
GitHub
parent
5d57b685a2
commit
863cfc1e78
2 changed files with 963 additions and 110 deletions
|
|
@ -0,0 +1,125 @@
|
|||
<?php
|
||||
|
||||
/*
|
||||
* This file is part of the official PHP MCP SDK.
|
||||
*
|
||||
* A collaboration between Symfony and the PHP Foundation.
|
||||
*
|
||||
* For the full copyright and license information, please view the LICENSE
|
||||
* file that was distributed with this source code.
|
||||
*/
|
||||
|
||||
namespace Mcp\Tests\Unit\Server\Transport\Http\OAuth;
|
||||
|
||||
use Mcp\Server\Transport\Http\OAuth\LenientOidcDiscoveryMetadataPolicy;
|
||||
use PHPUnit\Framework\Attributes\TestDox;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
/**
|
||||
* Tests LenientOidcDiscoveryMetadataPolicy validation behavior.
|
||||
*
|
||||
* @author Simon Chrzanowski <simon.chrzanowski@quentic.com>
|
||||
*/
|
||||
class LenientOidcDiscoveryMetadataPolicyTest extends TestCase
|
||||
{
|
||||
#[TestDox('metadata without code challenge methods is valid (defaults to S256 downstream)')]
|
||||
public function testMissingCodeChallengeMethodsIsValid(): void
|
||||
{
|
||||
$policy = new LenientOidcDiscoveryMetadataPolicy();
|
||||
$metadata = [
|
||||
'authorization_endpoint' => 'https://auth.example.com/authorize',
|
||||
'token_endpoint' => 'https://auth.example.com/token',
|
||||
'jwks_uri' => 'https://auth.example.com/jwks',
|
||||
];
|
||||
|
||||
$this->assertTrue($policy->isValid($metadata));
|
||||
}
|
||||
|
||||
#[TestDox('valid code challenge methods list is accepted')]
|
||||
public function testValidCodeChallengeMethodsIsAccepted(): void
|
||||
{
|
||||
$policy = new LenientOidcDiscoveryMetadataPolicy();
|
||||
$metadata = [
|
||||
'authorization_endpoint' => 'https://auth.example.com/authorize',
|
||||
'token_endpoint' => 'https://auth.example.com/token',
|
||||
'jwks_uri' => 'https://auth.example.com/jwks',
|
||||
'code_challenge_methods_supported' => ['S256'],
|
||||
];
|
||||
|
||||
$this->assertTrue($policy->isValid($metadata));
|
||||
}
|
||||
|
||||
#[TestDox('empty code challenge methods list is invalid')]
|
||||
public function testEmptyCodeChallengeMethodsIsInvalid(): void
|
||||
{
|
||||
$policy = new LenientOidcDiscoveryMetadataPolicy();
|
||||
$metadata = [
|
||||
'authorization_endpoint' => 'https://auth.example.com/authorize',
|
||||
'token_endpoint' => 'https://auth.example.com/token',
|
||||
'jwks_uri' => 'https://auth.example.com/jwks',
|
||||
'code_challenge_methods_supported' => [],
|
||||
];
|
||||
|
||||
$this->assertFalse($policy->isValid($metadata));
|
||||
}
|
||||
|
||||
#[TestDox('non string code challenge method is invalid')]
|
||||
public function testNonStringCodeChallengeMethodIsInvalid(): void
|
||||
{
|
||||
$policy = new LenientOidcDiscoveryMetadataPolicy();
|
||||
$metadata = [
|
||||
'authorization_endpoint' => 'https://auth.example.com/authorize',
|
||||
'token_endpoint' => 'https://auth.example.com/token',
|
||||
'jwks_uri' => 'https://auth.example.com/jwks',
|
||||
'code_challenge_methods_supported' => ['S256', 123],
|
||||
];
|
||||
|
||||
$this->assertFalse($policy->isValid($metadata));
|
||||
}
|
||||
|
||||
#[TestDox('missing required fields is invalid')]
|
||||
public function testMissingRequiredFieldsIsInvalid(): void
|
||||
{
|
||||
$policy = new LenientOidcDiscoveryMetadataPolicy();
|
||||
|
||||
$this->assertFalse($policy->isValid([
|
||||
'authorization_endpoint' => 'https://auth.example.com/authorize',
|
||||
'token_endpoint' => 'https://auth.example.com/token',
|
||||
// missing jwks_uri
|
||||
]));
|
||||
}
|
||||
|
||||
#[TestDox('empty string endpoint is invalid')]
|
||||
public function testEmptyStringEndpointIsInvalid(): void
|
||||
{
|
||||
$policy = new LenientOidcDiscoveryMetadataPolicy();
|
||||
|
||||
$this->assertFalse($policy->isValid([
|
||||
'authorization_endpoint' => '',
|
||||
'token_endpoint' => 'https://auth.example.com/token',
|
||||
'jwks_uri' => 'https://auth.example.com/jwks',
|
||||
]));
|
||||
}
|
||||
|
||||
#[TestDox('null code challenge methods is invalid')]
|
||||
public function testNullCodeChallengeMethodsIsInvalid(): void
|
||||
{
|
||||
$policy = new LenientOidcDiscoveryMetadataPolicy();
|
||||
$metadata = [
|
||||
'authorization_endpoint' => 'https://auth.example.com/authorize',
|
||||
'token_endpoint' => 'https://auth.example.com/token',
|
||||
'jwks_uri' => 'https://auth.example.com/jwks',
|
||||
'code_challenge_methods_supported' => null,
|
||||
];
|
||||
|
||||
$this->assertFalse($policy->isValid($metadata));
|
||||
}
|
||||
|
||||
#[TestDox('non-array input is invalid')]
|
||||
public function testNonArrayInputIsInvalid(): void
|
||||
{
|
||||
$policy = new LenientOidcDiscoveryMetadataPolicy();
|
||||
|
||||
$this->assertFalse($policy->isValid('not an array'));
|
||||
}
|
||||
}
|
||||
|
|
@ -12,6 +12,7 @@
|
|||
namespace Mcp\Tests\Unit\Server\Transport\Http\OAuth;
|
||||
|
||||
use Mcp\Exception\RuntimeException;
|
||||
use Mcp\Server\Transport\Http\OAuth\LenientOidcDiscoveryMetadataPolicy;
|
||||
use Mcp\Server\Transport\Http\OAuth\OidcDiscovery;
|
||||
use Nyholm\Psr7\Factory\Psr17Factory;
|
||||
use PHPUnit\Framework\Attributes\TestDox;
|
||||
|
|
@ -51,7 +52,7 @@ class OidcDiscoveryTest extends TestCase
|
|||
|
||||
$factory = new Psr17Factory();
|
||||
$issuer = 'https://auth.example.com';
|
||||
$metadataWithoutCodeChallengeMethods = [
|
||||
$metadata = [
|
||||
'issuer' => $issuer,
|
||||
'authorization_endpoint' => 'https://auth.example.com/oauth2/v2.0/authorize',
|
||||
'token_endpoint' => 'https://auth.example.com/oauth2/v2.0/token',
|
||||
|
|
@ -62,7 +63,7 @@ class OidcDiscoveryTest extends TestCase
|
|||
$httpClient->expects($this->exactly(2))
|
||||
->method('sendRequest')
|
||||
->willReturn($factory->createResponse(200)->withBody(
|
||||
$factory->createStream(json_encode($metadataWithoutCodeChallengeMethods, \JSON_THROW_ON_ERROR)),
|
||||
$factory->createStream(json_encode($metadata, \JSON_THROW_ON_ERROR)),
|
||||
));
|
||||
|
||||
$discovery = new OidcDiscovery(
|
||||
|
|
@ -75,6 +76,39 @@ class OidcDiscoveryTest extends TestCase
|
|||
$discovery->discover($issuer);
|
||||
}
|
||||
|
||||
#[TestDox('lenient discovery accepts metadata without code challenge methods')]
|
||||
public function testDiscoverAcceptsMetadataWithoutCodeChallengeMethodsUsingLenientPolicy(): void
|
||||
{
|
||||
$this->skipIfPsrHttpClientIsMissing();
|
||||
|
||||
$factory = new Psr17Factory();
|
||||
$issuer = 'https://auth.example.com';
|
||||
$metadata = [
|
||||
'issuer' => $issuer,
|
||||
'authorization_endpoint' => 'https://auth.example.com/oauth2/v2.0/authorize',
|
||||
'token_endpoint' => 'https://auth.example.com/oauth2/v2.0/token',
|
||||
'jwks_uri' => 'https://auth.example.com/discovery/v2.0/keys',
|
||||
];
|
||||
|
||||
$httpClient = $this->createMock(ClientInterface::class);
|
||||
$httpClient->expects($this->once())
|
||||
->method('sendRequest')
|
||||
->willReturn($factory->createResponse(200)->withBody(
|
||||
$factory->createStream(json_encode($metadata, \JSON_THROW_ON_ERROR)),
|
||||
));
|
||||
|
||||
$discovery = new OidcDiscovery(
|
||||
httpClient: $httpClient,
|
||||
requestFactory: $factory,
|
||||
metadataPolicy: new LenientOidcDiscoveryMetadataPolicy(),
|
||||
);
|
||||
|
||||
$result = $discovery->discover($issuer);
|
||||
|
||||
$this->assertSame($metadata['authorization_endpoint'], $result['authorization_endpoint']);
|
||||
$this->assertArrayNotHasKey('code_challenge_methods_supported', $result);
|
||||
}
|
||||
|
||||
#[TestDox('discover falls back to the next metadata URL when first response is invalid')]
|
||||
public function testDiscoverFallsBackOnInvalidMetadataResponse(): void
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in a new issue