diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c882ab7..64eb61ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ All notable changes to `mcp/sdk` will be documented in this file. * Add client component for building MCP clients * Add `Builder::setReferenceHandler()` to allow custom `ReferenceHandlerInterface` implementations (e.g. authorization decorators) * Add elicitation enum schema types per SEP-1330: `TitledEnumSchemaDefinition`, `MultiSelectEnumSchemaDefinition`, `TitledMultiSelectEnumSchemaDefinition` +* Allow `StrictOidcDiscoveryMetadataPolicy` to accept metadata without `code_challenge_methods_supported` (defaults to S256 downstream) +* Add OAuth 2.0 Dynamic Client Registration middleware (RFC 7591) 0.4.0 ----- diff --git a/examples/server/oauth-microsoft/README.md b/examples/server/oauth-microsoft/README.md index 74239498..aceda056 100644 --- a/examples/server/oauth-microsoft/README.md +++ b/examples/server/oauth-microsoft/README.md @@ -198,8 +198,9 @@ Microsoft's JWKS endpoint is public. Ensure your container can reach: ### `code_challenge_methods_supported` missing in discovery metadata -This example configures `OidcDiscovery` with `MicrosoftOidcMetadataPolicy`, so this -field can be missing or malformed and will not fail discovery. +The default `StrictOidcDiscoveryMetadataPolicy` accepts metadata without `code_challenge_methods_supported` +(defaults to S256 downstream). The `MicrosoftOidcMetadataPolicy` in this example demonstrates +how to implement a custom policy via `OidcDiscoveryMetadataPolicyInterface`. ### Graph API errors diff --git a/src/Exception/ClientRegistrationException.php b/src/Exception/ClientRegistrationException.php new file mode 100644 index 00000000..636006b0 --- /dev/null +++ b/src/Exception/ClientRegistrationException.php @@ -0,0 +1,16 @@ +responseFactory = $responseFactory ?? Psr17FactoryDiscovery::findResponseFactory(); + $this->streamFactory = $streamFactory ?? Psr17FactoryDiscovery::findStreamFactory(); + } + + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + $path = $request->getUri()->getPath(); + + if ('POST' === $request->getMethod() && self::REGISTRATION_PATH === $path) { + return $this->handleRegistration($request); + } + + $response = $handler->handle($request); + + if ('GET' === $request->getMethod() && '/.well-known/oauth-authorization-server' === $path) { + return $this->enrichAuthServerMetadata($response); + } + + return $response; + } + + private function handleRegistration(ServerRequestInterface $request): ResponseInterface + { + $body = $request->getBody()->__toString(); + $data = json_decode($body, true); + + if (!\is_array($data)) { + return $this->jsonResponse(400, [ + 'error' => 'invalid_client_metadata', + 'error_description' => 'Request body must be valid JSON.', + ]); + } + + try { + $result = $this->registrar->register($data); + } catch (ClientRegistrationException $e) { + return $this->jsonResponse(400, [ + 'error' => 'invalid_client_metadata', + 'error_description' => $e->getMessage(), + ]); + } + + return $this->jsonResponse(201, $result); + } + + private function enrichAuthServerMetadata(ResponseInterface $response): ResponseInterface + { + if (200 !== $response->getStatusCode()) { + return $response; + } + + $stream = $response->getBody(); + + if ($stream->isSeekable()) { + $stream->rewind(); + } + + $metadata = json_decode($stream->__toString(), true); + + if (!\is_array($metadata)) { + return $response; + } + + $metadata['registration_endpoint'] = rtrim($this->localBaseUrl, '/').self::REGISTRATION_PATH; + + return $this->jsonResponse(200, $metadata, [ + 'Cache-Control' => $response->getHeaderLine('Cache-Control'), + ]); + } + + /** + * @param array $data + * @param array $extraHeaders + */ + private function jsonResponse(int $status, array $data, array $extraHeaders = []): ResponseInterface + { + $response = $this->responseFactory + ->createResponse($status) + ->withHeader('Content-Type', 'application/json') + ->withBody($this->streamFactory->createStream( + json_encode($data, \JSON_THROW_ON_ERROR | \JSON_UNESCAPED_SLASHES), + )); + + foreach ($extraHeaders as $name => $value) { + if ('' !== $value) { + $response = $response->withHeader($name, $value); + } + } + + return $response; + } +} diff --git a/src/Server/Transport/Http/OAuth/ClientRegistrarInterface.php b/src/Server/Transport/Http/OAuth/ClientRegistrarInterface.php new file mode 100644 index 00000000..349d611b --- /dev/null +++ b/src/Server/Transport/Http/OAuth/ClientRegistrarInterface.php @@ -0,0 +1,42 @@ + $registrationRequest Client metadata from the registration request body + * + * @return array Registration response including client_id and optional client_secret + * + * @throws ClientRegistrationException If registration fails (e.g. invalid metadata, storage error) + */ + public function register(array $registrationRequest): array; +} diff --git a/src/Server/Transport/Http/OAuth/StrictOidcDiscoveryMetadataPolicy.php b/src/Server/Transport/Http/OAuth/StrictOidcDiscoveryMetadataPolicy.php index b89a3f8a..d69dc8a8 100644 --- a/src/Server/Transport/Http/OAuth/StrictOidcDiscoveryMetadataPolicy.php +++ b/src/Server/Transport/Http/OAuth/StrictOidcDiscoveryMetadataPolicy.php @@ -28,19 +28,20 @@ public function isValid(mixed $metadata): bool || '' === trim($metadata['token_endpoint']) || !\is_string($metadata['jwks_uri']) || '' === trim($metadata['jwks_uri']) - || !isset($metadata['code_challenge_methods_supported']) ) { return false; } - if (!\is_array($metadata['code_challenge_methods_supported']) || [] === $metadata['code_challenge_methods_supported']) { - return false; - } - - foreach ($metadata['code_challenge_methods_supported'] as $method) { - if (!\is_string($method) || '' === trim($method)) { + if (isset($metadata['code_challenge_methods_supported'])) { + if (!\is_array($metadata['code_challenge_methods_supported']) || [] === $metadata['code_challenge_methods_supported']) { return false; } + + foreach ($metadata['code_challenge_methods_supported'] as $method) { + if (!\is_string($method) || '' === trim($method)) { + return false; + } + } } return true; diff --git a/tests/Unit/Server/Transport/Http/Middleware/ClientRegistrationMiddlewareTest.php b/tests/Unit/Server/Transport/Http/Middleware/ClientRegistrationMiddlewareTest.php new file mode 100644 index 00000000..ada2ff7a --- /dev/null +++ b/tests/Unit/Server/Transport/Http/Middleware/ClientRegistrationMiddlewareTest.php @@ -0,0 +1,263 @@ +factory = new Psr17Factory(); + } + + #[TestDox('POST /register with valid JSON delegates to registrar and returns 201')] + public function testRegistrationSuccess(): void + { + $registrar = $this->createMock(ClientRegistrarInterface::class); + $registrar->expects($this->once()) + ->method('register') + ->with(['redirect_uris' => ['https://example.com/callback']]) + ->willReturn(['client_id' => 'new-client', 'client_secret' => 's3cret']); + + $middleware = $this->createMiddleware($registrar); + + $request = $this->factory->createServerRequest('POST', 'http://localhost:8000/register') + ->withBody($this->factory->createStream(json_encode(['redirect_uris' => ['https://example.com/callback']]))); + + $response = $middleware->process($request, $this->createPassthroughHandler(404)); + + $this->assertSame(201, $response->getStatusCode()); + $this->assertSame('application/json', $response->getHeaderLine('Content-Type')); + + $payload = json_decode($response->getBody()->__toString(), true, 512, \JSON_THROW_ON_ERROR); + $this->assertSame('new-client', $payload['client_id']); + $this->assertSame('s3cret', $payload['client_secret']); + } + + #[TestDox('POST /register with invalid JSON returns 400')] + public function testRegistrationWithInvalidJson(): void + { + $registrar = $this->createStub(ClientRegistrarInterface::class); + + $middleware = $this->createMiddleware($registrar); + + $request = $this->factory->createServerRequest('POST', 'http://localhost:8000/register') + ->withBody($this->factory->createStream('not json')); + + $response = $middleware->process($request, $this->createPassthroughHandler(404)); + + $this->assertSame(400, $response->getStatusCode()); + + $payload = json_decode($response->getBody()->__toString(), true, 512, \JSON_THROW_ON_ERROR); + $this->assertSame('invalid_client_metadata', $payload['error']); + $this->assertSame('Request body must be valid JSON.', $payload['error_description']); + } + + #[TestDox('POST /register returns 400 when registrar throws ClientRegistrationException')] + public function testRegistrationWithRegistrarException(): void + { + $registrar = $this->createMock(ClientRegistrarInterface::class); + $registrar->expects($this->once()) + ->method('register') + ->willThrowException(new ClientRegistrationException('redirect_uris is required')); + + $middleware = $this->createMiddleware($registrar); + + $request = $this->factory->createServerRequest('POST', 'http://localhost:8000/register') + ->withBody($this->factory->createStream('{}')); + + $response = $middleware->process($request, $this->createPassthroughHandler(404)); + + $this->assertSame(400, $response->getStatusCode()); + + $payload = json_decode($response->getBody()->__toString(), true, 512, \JSON_THROW_ON_ERROR); + $this->assertSame('invalid_client_metadata', $payload['error']); + $this->assertSame('redirect_uris is required', $payload['error_description']); + } + + #[TestDox('GET /.well-known/oauth-authorization-server enriches response with registration_endpoint')] + public function testMetadataEnrichment(): void + { + $registrar = $this->createStub(ClientRegistrarInterface::class); + $middleware = $this->createMiddleware($registrar); + + $upstreamMetadata = [ + 'issuer' => 'http://localhost:8000', + 'authorization_endpoint' => 'http://localhost:8000/authorize', + 'token_endpoint' => 'http://localhost:8000/token', + ]; + + $request = $this->factory->createServerRequest('GET', 'http://localhost:8000/.well-known/oauth-authorization-server'); + $handler = $this->createJsonHandler(200, $upstreamMetadata); + + $response = $middleware->process($request, $handler); + + $this->assertSame(200, $response->getStatusCode()); + + $payload = json_decode($response->getBody()->__toString(), true, 512, \JSON_THROW_ON_ERROR); + $this->assertSame('http://localhost:8000/register', $payload['registration_endpoint']); + $this->assertSame('http://localhost:8000/authorize', $payload['authorization_endpoint']); + } + + #[TestDox('GET /.well-known/oauth-authorization-server preserves Cache-Control header')] + public function testMetadataEnrichmentPreservesCacheControl(): void + { + $registrar = $this->createStub(ClientRegistrarInterface::class); + $middleware = $this->createMiddleware($registrar); + + $request = $this->factory->createServerRequest('GET', 'http://localhost:8000/.well-known/oauth-authorization-server'); + $handler = $this->createJsonHandler(200, ['issuer' => 'http://localhost:8000'], 'max-age=3600'); + + $response = $middleware->process($request, $handler); + + $this->assertSame('max-age=3600', $response->getHeaderLine('Cache-Control')); + } + + #[TestDox('GET /.well-known/oauth-authorization-server with non-200 status passes through unchanged')] + public function testMetadataNon200PassesThrough(): void + { + $registrar = $this->createStub(ClientRegistrarInterface::class); + $middleware = $this->createMiddleware($registrar); + + $request = $this->factory->createServerRequest('GET', 'http://localhost:8000/.well-known/oauth-authorization-server'); + $handler = $this->createPassthroughHandler(500); + + $response = $middleware->process($request, $handler); + + $this->assertSame(500, $response->getStatusCode()); + } + + #[TestDox('non-matching routes pass through to next handler')] + public function testNonMatchingRoutePassesThrough(): void + { + $registrar = $this->createStub(ClientRegistrarInterface::class); + $middleware = $this->createMiddleware($registrar); + + $request = $this->factory->createServerRequest('GET', 'http://localhost:8000/mcp'); + $handler = $this->createPassthroughHandler(204); + + $response = $middleware->process($request, $handler); + + $this->assertSame(204, $response->getStatusCode()); + } + + #[TestDox('constructor rejects empty localBaseUrl')] + public function testConstructorRejectsEmptyBaseUrl(): void + { + $this->expectException(InvalidArgumentException::class); + + new ClientRegistrationMiddleware( + $this->createStub(ClientRegistrarInterface::class), + '', + $this->factory, + $this->factory, + ); + } + + #[TestDox('localBaseUrl trailing slash is normalized in registration_endpoint')] + public function testTrailingSlashNormalization(): void + { + $registrar = $this->createStub(ClientRegistrarInterface::class); + + $middleware = new ClientRegistrationMiddleware( + $registrar, + 'http://localhost:8000/', + $this->factory, + $this->factory, + ); + + $request = $this->factory->createServerRequest('GET', 'http://localhost:8000/.well-known/oauth-authorization-server'); + $handler = $this->createJsonHandler(200, ['issuer' => 'http://localhost:8000']); + + $response = $middleware->process($request, $handler); + + $payload = json_decode($response->getBody()->__toString(), true, 512, \JSON_THROW_ON_ERROR); + $this->assertSame('http://localhost:8000/register', $payload['registration_endpoint']); + } + + private function createMiddleware(ClientRegistrarInterface $registrar): ClientRegistrationMiddleware + { + return new ClientRegistrationMiddleware( + $registrar, + 'http://localhost:8000', + $this->factory, + $this->factory, + ); + } + + private function createPassthroughHandler(int $status): RequestHandlerInterface + { + $factory = $this->factory; + + return new class($factory, $status) implements RequestHandlerInterface { + public function __construct( + private readonly ResponseFactoryInterface $factory, + private readonly int $status, + ) { + } + + public function handle(ServerRequestInterface $request): ResponseInterface + { + return $this->factory->createResponse($this->status); + } + }; + } + + /** + * @param array $data + */ + private function createJsonHandler(int $status, array $data, string $cacheControl = ''): RequestHandlerInterface + { + $factory = $this->factory; + + return new class($factory, $status, $data, $cacheControl) implements RequestHandlerInterface { + /** + * @param array $data + */ + public function __construct( + private readonly ResponseFactoryInterface $factory, + private readonly int $status, + private readonly array $data, + private readonly string $cacheControl, + ) { + } + + public function handle(ServerRequestInterface $request): ResponseInterface + { + $response = $this->factory->createResponse($this->status) + ->withHeader('Content-Type', 'application/json') + ->withBody((new Psr17Factory())->createStream( + json_encode($this->data, \JSON_THROW_ON_ERROR | \JSON_UNESCAPED_SLASHES), + )); + + if ('' !== $this->cacheControl) { + $response = $response->withHeader('Cache-Control', $this->cacheControl); + } + + return $response; + } + }; + } +} diff --git a/tests/Unit/Server/Transport/Http/OAuth/OidcDiscoveryTest.php b/tests/Unit/Server/Transport/Http/OAuth/OidcDiscoveryTest.php index 14eb54f1..3842822d 100644 --- a/tests/Unit/Server/Transport/Http/OAuth/OidcDiscoveryTest.php +++ b/tests/Unit/Server/Transport/Http/OAuth/OidcDiscoveryTest.php @@ -44,14 +44,14 @@ public function testInvalidIssuerUrlThrows(): void $discovery->discover('invalid-issuer'); } - #[TestDox('strict discovery rejects metadata without code challenge methods')] - public function testDiscoverRejectsMetadataWithoutCodeChallengeMethodsSupported(): void + #[TestDox('discovery accepts metadata without code_challenge_methods_supported')] + public function testDiscoverAcceptsMetadataWithoutCodeChallengeMethodsSupported(): void { $this->skipIfPsrHttpClientIsMissing(); $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', @@ -59,10 +59,10 @@ public function testDiscoverRejectsMetadataWithoutCodeChallengeMethodsSupported( ]; $httpClient = $this->createMock(ClientInterface::class); - $httpClient->expects($this->exactly(2)) + $httpClient->expects($this->once()) ->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( @@ -70,9 +70,10 @@ public function testDiscoverRejectsMetadataWithoutCodeChallengeMethodsSupported( requestFactory: $factory, ); - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage('Failed to discover authorization server metadata'); - $discovery->discover($issuer); + $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')] diff --git a/tests/Unit/Server/Transport/Http/OAuth/StrictOidcDiscoveryMetadataPolicyTest.php b/tests/Unit/Server/Transport/Http/OAuth/StrictOidcDiscoveryMetadataPolicyTest.php index 3d1e8a7d..16b2898b 100644 --- a/tests/Unit/Server/Transport/Http/OAuth/StrictOidcDiscoveryMetadataPolicyTest.php +++ b/tests/Unit/Server/Transport/Http/OAuth/StrictOidcDiscoveryMetadataPolicyTest.php @@ -22,8 +22,8 @@ */ class StrictOidcDiscoveryMetadataPolicyTest extends TestCase { - #[TestDox('metadata without code challenge methods is invalid in strict mode')] - public function testMissingCodeChallengeMethodsIsInvalid(): void + #[TestDox('metadata without code challenge methods is valid (defaults to S256 downstream)')] + public function testMissingCodeChallengeMethodsIsValid(): void { $policy = new StrictOidcDiscoveryMetadataPolicy(); $metadata = [ @@ -32,7 +32,7 @@ public function testMissingCodeChallengeMethodsIsInvalid(): void 'jwks_uri' => 'https://auth.example.com/jwks', ]; - $this->assertFalse($policy->isValid($metadata)); + $this->assertTrue($policy->isValid($metadata)); } #[TestDox('valid code challenge methods list is accepted in strict mode')]