From 3acb256c96ffe1288be12b65955ec9f16b74b84e Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Wed, 4 Nov 2026 23:03:07 +0100 Subject: [PATCH 01/10] [Client] Add OAuth metadata discovery and challenge parsing Finds and validates the documents that say who guards an MCP server: protected resource metadata (RFC 9728) and authorization server metadata (RFC 8414 and OpenID Connect), plus the WWW-Authenticate challenge that points at them. All of it is @internal, since nothing uses it until the authenticator lands. Endpoints have to pass SecureUrl, and a document naming another issuer than it was fetched for is discarded. --- src/Client/Auth/AuthorizationChallenge.php | 91 ++++++++ .../Auth/AuthorizationServerMetadata.php | 131 ++++++++++++ src/Client/Auth/MetadataDiscovery.php | 181 ++++++++++++++++ src/Client/Auth/ProtectedResourceMetadata.php | 91 ++++++++ .../Auth/AuthorizationChallengeTest.php | 82 ++++++++ .../Auth/AuthorizationServerMetadataTest.php | 140 +++++++++++++ .../Client/Auth/MetadataDiscoveryTest.php | 197 ++++++++++++++++++ .../Auth/ProtectedResourceMetadataTest.php | 58 ++++++ 8 files changed, 971 insertions(+) create mode 100644 src/Client/Auth/AuthorizationChallenge.php create mode 100644 src/Client/Auth/AuthorizationServerMetadata.php create mode 100644 src/Client/Auth/MetadataDiscovery.php create mode 100644 src/Client/Auth/ProtectedResourceMetadata.php create mode 100644 tests/Unit/Client/Auth/AuthorizationChallengeTest.php create mode 100644 tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php create mode 100644 tests/Unit/Client/Auth/MetadataDiscoveryTest.php create mode 100644 tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php diff --git a/src/Client/Auth/AuthorizationChallenge.php b/src/Client/Auth/AuthorizationChallenge.php new file mode 100644 index 00000000..889b92a7 --- /dev/null +++ b/src/Client/Auth/AuthorizationChallenge.php @@ -0,0 +1,91 @@ + + */ +final class AuthorizationChallenge +{ + /** + * @param array $parameters the challenge's auth-param list, keys lowercased + */ + private function __construct(public readonly array $parameters) + { + } + + public static function fromResponse(ResponseInterface $response): self + { + foreach ($response->getHeader('WWW-Authenticate') as $header) { + if (preg_match('/^\s*Bearer\b\s*(.*)$/is', $header, $matches)) { + return new self(self::parseParameters($matches[1])); + } + } + + return new self([]); + } + + /** + * The URL of the resource's metadata document, when the challenge named one. + */ + public function getResourceMetadataUrl(): ?string + { + return $this->parameters['resource_metadata'] ?? null; + } + + /** + * The scopes the rejected request needed. + * + * @return string[] + */ + public function getScopes(): array + { + $scope = $this->parameters['scope'] ?? ''; + + return array_values(array_filter(preg_split('/\s+/', trim($scope)) ?: [])); + } + + public function getError(): ?string + { + return $this->parameters['error'] ?? null; + } + + /** + * Parse a comma-separated auth-param list, tolerating both quoted and bare values. + * + * @return array + */ + private static function parseParameters(string $parameters): array + { + if (!preg_match_all('/([A-Za-z0-9_-]+)\s*=\s*(?:"((?:[^"\\\\]|\\\\.)*)"|([^,\s]*))/', $parameters, $matches, \PREG_SET_ORDER)) { + return []; + } + + $parsed = []; + + foreach ($matches as $match) { + $quoted = $match[2] ?? ''; + $value = '' !== $quoted ? $quoted : ($match[3] ?? ''); + // RFC 9110 quoted-string escaping is one backslash before one character; a + // C-style unescape would turn a literal "\n" in the header into a newline. + $parsed[strtolower($match[1])] = stripslashes($value); + } + + return $parsed; + } +} diff --git a/src/Client/Auth/AuthorizationServerMetadata.php b/src/Client/Auth/AuthorizationServerMetadata.php new file mode 100644 index 00000000..60e691ba --- /dev/null +++ b/src/Client/Auth/AuthorizationServerMetadata.php @@ -0,0 +1,131 @@ + + */ +final class AuthorizationServerMetadata +{ + /** + * @param string $issuer the issuer identifier, which must match how the document was found + * @param string $authorizationEndpoint where the user is sent to approve the request + * @param string $tokenEndpoint where codes and refresh tokens are exchanged + * @param ?string $registrationEndpoint where a client can register itself, when the server allows it + * @param string[] $scopesSupported scopes the server knows about + * @param string[] $grantTypesSupported grant types the server accepts + * @param string[] $tokenEndpointAuthMethodsSupported client authentication methods the token endpoint accepts + * @param string[] $codeChallengeMethodsSupported PKCE challenge methods the server accepts + * @param bool $clientIdMetadataDocumentSupported whether a URL may be used as the client id + * @param bool $issuerParameterSupported whether authorization responses carry an "iss" parameter + */ + public function __construct( + public readonly string $issuer, + public readonly string $authorizationEndpoint, + public readonly string $tokenEndpoint, + public readonly ?string $registrationEndpoint = null, + public readonly array $scopesSupported = [], + public readonly array $grantTypesSupported = [], + public readonly array $tokenEndpointAuthMethodsSupported = [], + public readonly array $codeChallengeMethodsSupported = [], + public readonly bool $clientIdMetadataDocumentSupported = false, + public readonly bool $issuerParameterSupported = false, + ) { + if ('' === trim($issuer)) { + throw new InvalidArgumentException('The authorization server metadata must name an issuer.'); + } + } + + /** + * @param array $data + */ + public static function fromArray(array $data): self + { + foreach (['issuer', 'authorization_endpoint', 'token_endpoint'] as $required) { + if (!isset($data[$required]) || !\is_string($data[$required])) { + throw new InvalidArgumentException(\sprintf('The authorization server metadata is missing a string "%s".', $required)); + } + } + + foreach (['authorization_endpoint', 'token_endpoint', 'registration_endpoint'] as $endpoint) { + if (\is_string($data[$endpoint] ?? null) && !self::isTransportSecure($data[$endpoint])) { + throw new InvalidArgumentException(\sprintf('The authorization server metadata declares "%s" as "%s", which is neither an HTTPS URL nor a loopback address.', $endpoint, $data[$endpoint])); + } + } + + /* @var array{issuer: string, authorization_endpoint: string, token_endpoint: string} $data */ + return new self( + $data['issuer'], + $data['authorization_endpoint'], + $data['token_endpoint'], + \is_string($data['registration_endpoint'] ?? null) ? $data['registration_endpoint'] : null, + self::strings($data['scopes_supported'] ?? null), + self::strings($data['grant_types_supported'] ?? null), + self::strings($data['token_endpoint_auth_methods_supported'] ?? null), + self::strings($data['code_challenge_methods_supported'] ?? null), + true === ($data['client_id_metadata_document_supported'] ?? null), + true === ($data['authorization_response_iss_parameter_supported'] ?? null), + ); + } + + /** + * The fixed endpoints a 2025-03-26 era server without metadata is assumed to expose. + */ + public static function forLegacyServer(string $issuer): self + { + $base = rtrim($issuer, '/'); + + return new self($issuer, $base.'/authorize', $base.'/token', $base.'/register'); + } + + public function supportsGrant(string $grantType): bool + { + // RFC 8414 section 2 also defaults to implicit, which OAuth 2.1 removed. + return [] === $this->grantTypesSupported + ? 'authorization_code' === $grantType + : \in_array($grantType, $this->grantTypesSupported, true); + } + + public function supportsScope(string $scope): bool + { + return \in_array($scope, $this->scopesSupported, true); + } + + /** + * Whether a URL is https or loopback, the only endpoints credentials may go to. + */ + public static function isTransportSecure(string $url): bool + { + try { + SecureUrl::parse($url, 'URL'); + + return true; + } catch (InvalidArgumentException) { + return false; + } + } + + /** + * @return string[] + */ + private static function strings(mixed $value): array + { + return \is_array($value) ? array_values(array_filter($value, 'is_string')) : []; + } +} diff --git a/src/Client/Auth/MetadataDiscovery.php b/src/Client/Auth/MetadataDiscovery.php new file mode 100644 index 00000000..0690f489 --- /dev/null +++ b/src/Client/Auth/MetadataDiscovery.php @@ -0,0 +1,181 @@ + + */ +final class MetadataDiscovery +{ + public function __construct( + private readonly ClientInterface $httpClient, + private readonly RequestFactoryInterface $requestFactory, + private readonly LoggerInterface $logger = new NullLogger(), + ) { + } + + /** + * Fetch the protected resource metadata for an MCP endpoint. + * + * @param string $resourceUrl the MCP endpoint being called + * @param ?string $metadataUrl the location the challenge named, which wins over any guess + */ + public function discoverProtectedResource(string $resourceUrl, ?string $metadataUrl = null): ?ProtectedResourceMetadata + { + foreach (null !== $metadataUrl ? [$metadataUrl] : self::protectedResourceCandidates($resourceUrl) as $candidate) { + $document = $this->fetch($candidate); + + if (null === $document) { + continue; + } + + try { + return ProtectedResourceMetadata::fromArray($document); + } catch (InvalidArgumentException $e) { + $this->logger->warning('Ignoring malformed protected resource metadata', ['url' => $candidate, 'exception' => $e]); + } + } + + return null; + } + + /** + * Fetch an authorization server's metadata, discarding it if its issuer differs (RFC 8414 section 3.3). + * + * @param string $issuer the issuer identifier, as advertised by the resource + */ + public function discoverAuthorizationServer(string $issuer): ?AuthorizationServerMetadata + { + foreach (self::authorizationServerCandidates($issuer) as $candidate) { + $document = $this->fetch($candidate); + + if (null === $document) { + continue; + } + + try { + $metadata = AuthorizationServerMetadata::fromArray($document); + } catch (InvalidArgumentException $e) { + $this->logger->warning('Ignoring malformed authorization server metadata', ['url' => $candidate, 'exception' => $e]); + + continue; + } + + if ($metadata->issuer !== $issuer) { + $this->logger->error('Discarding authorization server metadata: the document\'s issuer does not match the issuer it was requested for.', [ + 'expected_issuer' => $issuer, + 'metadata_issuer' => $metadata->issuer, + 'url' => $candidate, + ]); + + return null; + } + + return $metadata; + } + + return null; + } + + /** + * @return string[] + */ + public static function protectedResourceCandidates(string $resourceUrl): array + { + $parts = parse_url($resourceUrl); + + if (!\is_array($parts)) { + return []; + } + + $origin = self::origin($parts); + $path = trim($parts['path'] ?? '', '/'); + $root = $origin.'/.well-known/oauth-protected-resource'; + + return '' === $path ? [$root] : [$root.'/'.$path, $root]; + } + + /** + * @return string[] + */ + public static function authorizationServerCandidates(string $issuer): array + { + $parts = parse_url($issuer); + + if (!\is_array($parts)) { + return []; + } + + $origin = self::origin($parts); + $path = trim($parts['path'] ?? '', '/'); + + if ('' === $path) { + return [ + $origin.'/.well-known/oauth-authorization-server', + $origin.'/.well-known/openid-configuration', + ]; + } + + return [ + $origin.'/.well-known/oauth-authorization-server/'.$path, + $origin.'/.well-known/openid-configuration/'.$path, + $origin.'/'.$path.'/.well-known/openid-configuration', + ]; + } + + /** + * @return array|null + */ + private function fetch(string $url): ?array + { + $request = $this->requestFactory->createRequest('GET', $url)->withHeader('Accept', 'application/json'); + + try { + $response = $this->httpClient->sendRequest($request); + } catch (ClientExceptionInterface $e) { + $this->logger->debug('Metadata request failed', ['url' => $url, 'exception' => $e]); + + return null; + } + + if (200 !== $response->getStatusCode()) { + $this->logger->debug('Metadata not available', ['url' => $url, 'status' => $response->getStatusCode()]); + + return null; + } + + $decoded = json_decode((string) $response->getBody(), true); + + return \is_array($decoded) ? $decoded : null; + } + + /** + * @param array $parts + */ + private static function origin(array $parts): string + { + $origin = ($parts['scheme'] ?? 'https').'://'.($parts['host'] ?? ''); + + return isset($parts['port']) ? $origin.':'.$parts['port'] : $origin; + } +} diff --git a/src/Client/Auth/ProtectedResourceMetadata.php b/src/Client/Auth/ProtectedResourceMetadata.php new file mode 100644 index 00000000..da7e306b --- /dev/null +++ b/src/Client/Auth/ProtectedResourceMetadata.php @@ -0,0 +1,91 @@ + + */ +final class ProtectedResourceMetadata +{ + /** + * @param string $resource the canonical URI the resource identifies itself by + * @param string[] $authorizationServers issuer identifiers of the servers that can mint tokens for it + * @param ?string[] $scopesSupported scopes the resource understands, or null when it does not say + */ + public function __construct( + public readonly string $resource, + public readonly array $authorizationServers = [], + public readonly ?array $scopesSupported = null, + ) { + if ('' === trim($resource)) { + throw new InvalidArgumentException('The protected resource metadata must name a resource.'); + } + } + + /** + * @param array $data + */ + public static function fromArray(array $data): self + { + if (!isset($data['resource']) || !\is_string($data['resource'])) { + throw new InvalidArgumentException('The protected resource metadata is missing a string "resource".'); + } + + $scopes = $data['scopes_supported'] ?? null; + + return new self( + $data['resource'], + array_values(array_filter((array) ($data['authorization_servers'] ?? []), 'is_string')), + \is_array($scopes) ? array_values(array_filter($scopes, 'is_string')) : null, + ); + } + + /** + * Whether a token minted for this resource may be sent to the given URL. + */ + public function covers(string $url): bool + { + return self::isWithin($url, $this->resource); + } + + /** + * Whether $url sits at or below the resource identifier $resource. + */ + public static function isWithin(string $url, string $resource): bool + { + $resource = parse_url($resource); + $target = parse_url($url); + + if (!\is_array($resource) || !\is_array($target)) { + return false; + } + + foreach (['scheme', 'host', 'port'] as $part) { + if (($resource[$part] ?? null) !== ($target[$part] ?? null)) { + return false; + } + } + + $resourcePath = rtrim($resource['path'] ?? '', '/'); + $targetPath = $target['path'] ?? ''; + + return '' === $resourcePath + || $targetPath === $resourcePath + || str_starts_with($targetPath, $resourcePath.'/'); + } +} diff --git a/tests/Unit/Client/Auth/AuthorizationChallengeTest.php b/tests/Unit/Client/Auth/AuthorizationChallengeTest.php new file mode 100644 index 00000000..68cc6b16 --- /dev/null +++ b/tests/Unit/Client/Auth/AuthorizationChallengeTest.php @@ -0,0 +1,82 @@ + 'Bearer scope="mcp:read mcp:write", resource_metadata="https://mcp.example.com/.well-known/oauth-protected-resource/mcp", error="insufficient_scope"', + ])); + + $this->assertSame('https://mcp.example.com/.well-known/oauth-protected-resource/mcp', $challenge->getResourceMetadataUrl()); + $this->assertSame(['mcp:read', 'mcp:write'], $challenge->getScopes()); + $this->assertSame('insufficient_scope', $challenge->getError()); + } + + #[TestDox('a challenge without parameters yields nothing rather than failing')] + public function testParsesBareChallenge(): void + { + $challenge = AuthorizationChallenge::fromResponse(new Response(401, ['WWW-Authenticate' => 'Bearer'])); + + $this->assertNull($challenge->getResourceMetadataUrl()); + $this->assertSame([], $challenge->getScopes()); + $this->assertNull($challenge->getError()); + } + + #[TestDox('a response with no challenge header at all yields nothing')] + public function testHandlesMissingHeader(): void + { + $challenge = AuthorizationChallenge::fromResponse(new Response(401)); + + $this->assertSame([], $challenge->parameters); + } + + #[TestDox('unquoted parameter values are accepted')] + public function testParsesUnquotedValues(): void + { + $challenge = AuthorizationChallenge::fromResponse(new Response(401, [ + 'WWW-Authenticate' => 'Bearer error=invalid_token, scope=mcp:read', + ])); + + $this->assertSame('invalid_token', $challenge->getError()); + $this->assertSame(['mcp:read'], $challenge->getScopes()); + } + + #[TestDox('a quoted backslash unescapes one character, not a C escape sequence')] + public function testUnescapesTheHttpWay(): void + { + $challenge = AuthorizationChallenge::fromResponse(new Response(401, [ + 'WWW-Authenticate' => 'Bearer error="say \\"hi\\" now", scope="a\\nb"', + ])); + + $this->assertSame('say "hi" now', $challenge->getError()); + $this->assertSame(['anb'], $challenge->getScopes()); + } + + #[TestDox('a Basic challenge alongside Bearer does not shadow it')] + public function testIgnoresOtherSchemes(): void + { + $response = (new Response(401)) + ->withAddedHeader('WWW-Authenticate', 'Basic realm="internal"') + ->withAddedHeader('WWW-Authenticate', 'Bearer scope="mcp:read"'); + + $this->assertSame(['mcp:read'], AuthorizationChallenge::fromResponse($response)->getScopes()); + } +} diff --git a/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php b/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php new file mode 100644 index 00000000..5ece4a63 --- /dev/null +++ b/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php @@ -0,0 +1,140 @@ + 'https://auth.example.com', + 'authorization_endpoint' => 'https://auth.example.com/authorize', + 'token_endpoint' => 'https://auth.example.com/token', + 'registration_endpoint' => 'https://auth.example.com/register', + 'scopes_supported' => ['mcp:read', 'offline_access'], + 'grant_types_supported' => ['authorization_code', 'refresh_token'], + 'token_endpoint_auth_methods_supported' => ['client_secret_basic'], + 'client_id_metadata_document_supported' => true, + 'authorization_response_iss_parameter_supported' => true, + ]); + + $this->assertSame('https://auth.example.com/register', $metadata->registrationEndpoint); + $this->assertTrue($metadata->clientIdMetadataDocumentSupported); + $this->assertTrue($metadata->issuerParameterSupported); + $this->assertTrue($metadata->supportsScope('offline_access')); + $this->assertFalse($metadata->supportsScope('mcp:write')); + } + + #[TestDox('a document missing an endpoint the client needs is rejected')] + public function testRequiresEndpoints(): void + { + $this->expectException(InvalidArgumentException::class); + + AuthorizationServerMetadata::fromArray([ + 'issuer' => 'https://auth.example.com', + 'authorization_endpoint' => 'https://auth.example.com/authorize', + ]); + } + + #[TestDox('an unstated grant list means the authorization code grant, and nothing else')] + public function testDefaultGrantTypes(): void + { + $metadata = AuthorizationServerMetadata::fromArray([ + 'issuer' => 'https://auth.example.com', + 'authorization_endpoint' => 'https://auth.example.com/authorize', + 'token_endpoint' => 'https://auth.example.com/token', + ]); + + $this->assertTrue($metadata->supportsGrant('authorization_code')); + $this->assertFalse($metadata->supportsGrant('refresh_token')); + } + + // These URLs come out of a document whose location a hostile MCP server chose, and + // one of them is opened in the user's browser. + #[TestDox('endpoints that are not HTTPS are refused')] + public function testRejectsInsecureEndpoints(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('neither an HTTPS URL nor a loopback address'); + + AuthorizationServerMetadata::fromArray([ + 'issuer' => 'https://auth.example.com', + 'authorization_endpoint' => 'https://auth.example.com/authorize', + 'token_endpoint' => 'http://auth.example.com/token', + ]); + } + + #[DataProvider('loopbackProvider')] + #[TestDox('plain HTTP is allowed on a loopback address: $_dataName')] + public function testAllowsLoopbackOverHttp(string $endpoint): void + { + $metadata = AuthorizationServerMetadata::fromArray([ + 'issuer' => 'http://localhost:8080', + 'authorization_endpoint' => $endpoint.'/authorize', + 'token_endpoint' => $endpoint.'/token', + ]); + + $this->assertSame($endpoint.'/token', $metadata->tokenEndpoint); + } + + /** + * @return iterable + */ + public static function loopbackProvider(): iterable + { + yield 'localhost' => ['http://localhost:8080']; + yield 'the loopback address' => ['http://127.0.0.1:8080']; + yield 'anywhere in 127/8' => ['http://127.13.2.9:8080']; + yield 'IPv6 loopback' => ['http://[::1]:8080']; + yield 'a name under .localhost' => ['http://mcp.localhost:8080']; + } + + /** + * @return iterable + */ + public static function lookalikeProvider(): iterable + { + yield 'localhost as a subdomain' => ['http://localhost.evil.example.com']; + yield 'not an IP address in 127/8' => ['http://127.999.1.1']; + } + + #[DataProvider('lookalikeProvider')] + #[TestDox('a host that merely looks like loopback is still refused: $_dataName')] + public function testRejectsLookalikeHosts(string $endpoint): void + { + $this->expectException(InvalidArgumentException::class); + + AuthorizationServerMetadata::fromArray([ + 'issuer' => 'https://auth.example.com', + 'authorization_endpoint' => $endpoint.'/authorize', + 'token_endpoint' => 'https://auth.example.com/token', + ]); + } + + #[TestDox('a server that publishes nothing gets the endpoint names its era prescribed')] + public function testLegacyEndpoints(): void + { + $metadata = AuthorizationServerMetadata::forLegacyServer('https://mcp.example.com'); + + $this->assertSame('https://mcp.example.com/authorize', $metadata->authorizationEndpoint); + $this->assertSame('https://mcp.example.com/token', $metadata->tokenEndpoint); + $this->assertSame('https://mcp.example.com/register', $metadata->registrationEndpoint); + $this->assertFalse($metadata->issuerParameterSupported); + } +} diff --git a/tests/Unit/Client/Auth/MetadataDiscoveryTest.php b/tests/Unit/Client/Auth/MetadataDiscoveryTest.php new file mode 100644 index 00000000..bc516c2f --- /dev/null +++ b/tests/Unit/Client/Auth/MetadataDiscoveryTest.php @@ -0,0 +1,197 @@ + + */ + public static function protectedResourceProvider(): iterable + { + yield 'a path is tried before the host root' => [ + 'https://mcp.example.com/mcp', + [ + 'https://mcp.example.com/.well-known/oauth-protected-resource/mcp', + 'https://mcp.example.com/.well-known/oauth-protected-resource', + ], + ]; + + yield 'a root endpoint has only one location' => [ + 'https://mcp.example.com/', + ['https://mcp.example.com/.well-known/oauth-protected-resource'], + ]; + + yield 'the port is part of the origin' => [ + 'http://localhost:8000/api/mcp', + [ + 'http://localhost:8000/.well-known/oauth-protected-resource/api/mcp', + 'http://localhost:8000/.well-known/oauth-protected-resource', + ], + ]; + } + + #[DataProvider('protectedResourceProvider')] + #[TestDox('protected resource metadata is looked for at: $_dataName')] + public function testProtectedResourceCandidates(string $url, array $expected): void + { + $this->assertSame($expected, MetadataDiscovery::protectedResourceCandidates($url)); + } + + /** + * @return iterable + */ + public static function authorizationServerProvider(): iterable + { + yield 'an issuer without a path' => [ + 'https://auth.example.com', + [ + 'https://auth.example.com/.well-known/oauth-authorization-server', + 'https://auth.example.com/.well-known/openid-configuration', + ], + ]; + + // RFC 8414 inserts the path into the well-known segment, OpenID Connect appends it. + yield 'an issuer with a path' => [ + 'https://auth.example.com/tenant1', + [ + 'https://auth.example.com/.well-known/oauth-authorization-server/tenant1', + 'https://auth.example.com/.well-known/openid-configuration/tenant1', + 'https://auth.example.com/tenant1/.well-known/openid-configuration', + ], + ]; + } + + #[DataProvider('authorizationServerProvider')] + #[TestDox('authorization server metadata is looked for at: $_dataName')] + public function testAuthorizationServerCandidates(string $issuer, array $expected): void + { + $this->assertSame($expected, MetadataDiscovery::authorizationServerCandidates($issuer)); + } + + #[TestDox('the first location that answers wins, and later ones are not requested')] + public function testStopsAtTheFirstDocument(): void + { + $client = new RecordingClient([ + 'https://auth.example.com/.well-known/oauth-authorization-server' => new Response(404), + 'https://auth.example.com/.well-known/openid-configuration' => self::json([ + 'issuer' => 'https://auth.example.com', + 'authorization_endpoint' => 'https://auth.example.com/authorize', + 'token_endpoint' => 'https://auth.example.com/token', + ]), + ]); + + $metadata = $this->discovery($client)->discoverAuthorizationServer('https://auth.example.com'); + + $this->assertNotNull($metadata); + $this->assertSame('https://auth.example.com/token', $metadata->tokenEndpoint); + $this->assertCount(2, $client->requested); + } + + #[TestDox('metadata declaring a different issuer than it was fetched for is discarded')] + public function testRejectsIssuerMismatch(): void + { + $client = new RecordingClient([ + 'https://auth.example.com/.well-known/oauth-authorization-server' => self::json([ + 'issuer' => 'https://attacker.example.com', + 'authorization_endpoint' => 'https://attacker.example.com/authorize', + 'token_endpoint' => 'https://attacker.example.com/token', + ]), + ]); + + $this->assertNull($this->discovery($client)->discoverAuthorizationServer('https://auth.example.com')); + } + + #[TestDox('a metadata URL named by the challenge is used instead of guessing')] + public function testHonoursTheChallengeMetadataUrl(): void + { + $client = new RecordingClient([ + 'https://mcp.example.com/custom/location.json' => self::json([ + 'resource' => 'https://mcp.example.com/mcp', + 'authorization_servers' => ['https://auth.example.com'], + ]), + ]); + + $metadata = $this->discovery($client)->discoverProtectedResource('https://mcp.example.com/mcp', 'https://mcp.example.com/custom/location.json'); + + $this->assertNotNull($metadata); + $this->assertSame(['https://auth.example.com'], $metadata->authorizationServers); + $this->assertSame(['https://mcp.example.com/custom/location.json'], $client->requested); + } + + #[TestDox('a transport failure is not fatal, the next location is still tried')] + public function testSurvivesATransportFailure(): void + { + $client = new RecordingClient([ + 'https://mcp.example.com/.well-known/oauth-protected-resource/mcp' => new class('connection refused') extends \RuntimeException implements ClientExceptionInterface {}, + 'https://mcp.example.com/.well-known/oauth-protected-resource' => self::json([ + 'resource' => 'https://mcp.example.com', + 'authorization_servers' => ['https://auth.example.com'], + ]), + ]); + + $this->assertNotNull($this->discovery($client)->discoverProtectedResource('https://mcp.example.com/mcp')); + } + + private function discovery(ClientInterface $client): MetadataDiscovery + { + return new MetadataDiscovery($client, new Psr17Factory()); + } + + /** + * @param array $payload + */ + private static function json(array $payload): Response + { + return new Response(200, ['Content-Type' => 'application/json'], (string) json_encode($payload)); + } +} + +/** + * A PSR-18 client that answers from a table and remembers what it was asked for. + */ +final class RecordingClient implements ClientInterface +{ + /** @var string[] */ + public array $requested = []; + + /** + * @param array $responses + */ + public function __construct(private readonly array $responses) + { + } + + public function sendRequest(RequestInterface $request): ResponseInterface + { + $url = (string) $request->getUri(); + $this->requested[] = $url; + $response = $this->responses[$url] ?? new Response(404); + + if ($response instanceof \Throwable) { + throw $response; + } + + return $response; + } +} diff --git a/tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php b/tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php new file mode 100644 index 00000000..23d05305 --- /dev/null +++ b/tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php @@ -0,0 +1,58 @@ + + */ + public static function coverageProvider(): iterable + { + yield 'the same URL' => ['https://mcp.example.com/mcp', 'https://mcp.example.com/mcp', true]; + yield 'a resource at the host root' => ['https://mcp.example.com', 'https://mcp.example.com/mcp', true]; + yield 'an endpoint below the resource' => ['https://mcp.example.com/api', 'https://mcp.example.com/api/mcp', true]; + yield 'a sibling path' => ['https://mcp.example.com/other', 'https://mcp.example.com/mcp', false]; + yield 'a path that merely shares a prefix' => ['https://mcp.example.com/api', 'https://mcp.example.com/api-v2/mcp', false]; + yield 'a different host' => ['https://evil.example.com/mcp', 'https://mcp.example.com/mcp', false]; + yield 'a different scheme' => ['http://mcp.example.com/mcp', 'https://mcp.example.com/mcp', false]; + yield 'a different port' => ['http://localhost:9000/mcp', 'http://localhost:8000/mcp', false]; + } + + #[DataProvider('coverageProvider')] + #[TestDox('a token is allowed at the endpoint for: $_dataName')] + public function testCovers(string $resource, string $endpoint, bool $expected): void + { + $this->assertSame($expected, (new ProtectedResourceMetadata($resource))->covers($endpoint)); + } + + #[TestDox('scopes_supported is kept apart from an absent list')] + public function testDistinguishesEmptyAndAbsentScopes(): void + { + $this->assertNull(ProtectedResourceMetadata::fromArray(['resource' => 'https://mcp.example.com'])->scopesSupported); + $this->assertSame([], ProtectedResourceMetadata::fromArray(['resource' => 'https://mcp.example.com', 'scopes_supported' => []])->scopesSupported); + } + + #[TestDox('a document without a resource is rejected')] + public function testRequiresResource(): void + { + $this->expectException(InvalidArgumentException::class); + + ProtectedResourceMetadata::fromArray(['authorization_servers' => ['https://auth.example.com']]); + } +} From 08db076b986493ecc036a57e0d31104faefbfbc7 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Wed, 4 Nov 2026 23:20:44 +0100 Subject: [PATCH 02/10] [Client] Isolate the Bearer challenge and decode its values per RFC 9110 --- src/Client/Auth/AuthorizationChallenge.php | 52 ++++++++----- .../Auth/AuthorizationChallengeTest.php | 75 +++++++++++++++++++ 2 files changed, 109 insertions(+), 18 deletions(-) diff --git a/src/Client/Auth/AuthorizationChallenge.php b/src/Client/Auth/AuthorizationChallenge.php index 889b92a7..43f37a4e 100644 --- a/src/Client/Auth/AuthorizationChallenge.php +++ b/src/Client/Auth/AuthorizationChallenge.php @@ -32,8 +32,10 @@ private function __construct(public readonly array $parameters) public static function fromResponse(ResponseInterface $response): self { foreach ($response->getHeader('WWW-Authenticate') as $header) { - if (preg_match('/^\s*Bearer\b\s*(.*)$/is', $header, $matches)) { - return new self(self::parseParameters($matches[1])); + $parameters = self::parseBearerParameters($header); + + if (null !== $parameters) { + return new self($parameters); } } @@ -55,9 +57,7 @@ public function getResourceMetadataUrl(): ?string */ public function getScopes(): array { - $scope = $this->parameters['scope'] ?? ''; - - return array_values(array_filter(preg_split('/\s+/', trim($scope)) ?: [])); + return preg_split('/\s+/', $this->parameters['scope'] ?? '', -1, \PREG_SPLIT_NO_EMPTY) ?: []; } public function getError(): ?string @@ -66,26 +66,42 @@ public function getError(): ?string } /** - * Parse a comma-separated auth-param list, tolerating both quoted and bare values. + * Read the Bearer challenge out of a field value that may list several challenges. * - * @return array + * @return array|null null when the field value holds no Bearer challenge */ - private static function parseParameters(string $parameters): array + private static function parseBearerParameters(string $header): ?array { - if (!preg_match_all('/([A-Za-z0-9_-]+)\s*=\s*(?:"((?:[^"\\\\]|\\\\.)*)"|([^,\s]*))/', $parameters, $matches, \PREG_SET_ORDER)) { - return []; + if (!preg_match_all('/(?:[^",]|"(?:[^"\\\\]|\\\\.)*")+/s', $header, $members)) { + return null; } - $parsed = []; + $parameters = null; + + foreach ($members[0] as $member) { + if (preg_match('/^\s*([^\s=,"]+)(?:\s+(?!=)|\s*$)(.*)$/s', $member, $scheme)) { + if (null !== $parameters) { + break; + } - foreach ($matches as $match) { - $quoted = $match[2] ?? ''; - $value = '' !== $quoted ? $quoted : ($match[3] ?? ''); - // RFC 9110 quoted-string escaping is one backslash before one character; a - // C-style unescape would turn a literal "\n" in the header into a newline. - $parsed[strtolower($match[1])] = stripslashes($value); + if (0 !== strcasecmp($scheme[1], 'Bearer')) { + continue; + } + + $parameters = []; + $member = $scheme[2]; + } + + if (null === $parameters) { + continue; + } + + if (preg_match('/^\s*([^\s=,"]+)\s*=\s*(?:"((?:[^"\\\\]|\\\\.)*)"|((?!=)[^,\s]*))\s*$/s', $member, $parameter)) { + // RFC 9110 quoted-pair: only the backslash goes, unlike stripslashes() which maps \0 to NUL. + $parameters[strtolower($parameter[1])] = $parameter[3] ?? (preg_replace('/\\\\(.)/s', '$1', $parameter[2] ?? '') ?? ''); + } } - return $parsed; + return $parameters; } } diff --git a/tests/Unit/Client/Auth/AuthorizationChallengeTest.php b/tests/Unit/Client/Auth/AuthorizationChallengeTest.php index 68cc6b16..69c3f0d8 100644 --- a/tests/Unit/Client/Auth/AuthorizationChallengeTest.php +++ b/tests/Unit/Client/Auth/AuthorizationChallengeTest.php @@ -13,6 +13,7 @@ use Mcp\Client\Auth\AuthorizationChallenge; use Nyholm\Psr7\Response; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\TestDox; use PHPUnit\Framework\TestCase; @@ -79,4 +80,78 @@ public function testIgnoresOtherSchemes(): void $this->assertSame(['mcp:read'], AuthorizationChallenge::fromResponse($response)->getScopes()); } + + /** + * @return iterable}> + */ + public static function combinedHeaderProvider(): iterable + { + yield 'Bearer after another scheme' => ['Basic realm="internal", Bearer resource_metadata="https://good"', ['resource_metadata' => 'https://good']]; + yield 'Bearer after a token68 scheme' => ['Negotiate abc==, Bearer scope="a"', ['scope' => 'a']]; + yield 'Bearer after a bare scheme' => ['Negotiate, Bearer scope="a"', ['scope' => 'a']]; + yield 'Bearer before a token68 scheme' => ['Bearer scope="a", Negotiate abc==', ['scope' => 'a']]; + yield 'parameters of a later scheme do not override' => ['Bearer resource_metadata="https://good", scope="a", Digest realm="x", scope="evil", resource_metadata="https://evil"', ['resource_metadata' => 'https://good', 'scope' => 'a']]; + yield 'parameters of an earlier scheme are not picked up' => ['Basic realm="simple", Newauth realm="apps", type=1, title="Login to \\"apps\\"", Bearer scope="a"', ['scope' => 'a']]; + yield 'a scheme inside a quoted value is no scheme' => ['Basic realm="a, Bearer scope=\\"evil\\""', []]; + yield 'a comma inside a quoted value does not split' => ['Bearer error="use Basic realm=x, scope=evil", scope="a"', ['error' => 'use Basic realm=x, scope=evil', 'scope' => 'a']]; + yield 'an empty list member is skipped' => ['Bearer scope="a",, error="b"', ['scope' => 'a', 'error' => 'b']]; + yield 'the scheme is case-insensitive' => ['bEARER scope="a"', ['scope' => 'a']]; + yield 'a scheme merely starting with Bearer' => ['Bearer-x scope="a"', []]; + } + + /** + * @param array $expected + */ + #[DataProvider('combinedHeaderProvider')] + #[TestDox('the Bearer challenge is isolated for: $_dataName')] + public function testIsolatesTheBearerChallenge(string $header, array $expected): void + { + $challenge = AuthorizationChallenge::fromResponse(new Response(401, ['WWW-Authenticate' => $header])); + + $this->assertSame($expected, $challenge->parameters); + } + + /** + * @return iterable + */ + public static function scopeProvider(): iterable + { + yield 'zero among others' => ['Bearer scope="0 mcp:read"', ['0', 'mcp:read']]; + yield 'bare zero' => ['Bearer scope=0', ['0']]; + yield 'surrounding and repeated whitespace' => ['Bearer scope=" a 0 b "', ['a', '0', 'b']]; + yield 'empty' => ['Bearer scope=""', []]; + yield 'only whitespace' => ['Bearer scope=" "', []]; + } + + /** + * @param string[] $expected + */ + #[DataProvider('scopeProvider')] + #[TestDox('the scopes are split for: $_dataName')] + public function testSplitsScopes(string $header, array $expected): void + { + $challenge = AuthorizationChallenge::fromResponse(new Response(401, ['WWW-Authenticate' => $header])); + + $this->assertSame($expected, $challenge->getScopes()); + } + + /** + * @return iterable + */ + public static function quotedPairProvider(): iterable + { + yield 'an escaped zero is a zero' => ['"a\\0b"', 'a0b']; + yield 'an escaped backslash stays one backslash' => ['"a\\\\0b"', 'a\\0b']; + yield 'an escaped quote' => ['"a\\"b"', 'a"b']; + yield 'a bare value is left alone' => ['a\\0b', 'a\\0b']; + } + + #[DataProvider('quotedPairProvider')] + #[TestDox('the value is decoded for: $_dataName')] + public function testDecodesQuotedPairs(string $value, string $expected): void + { + $challenge = AuthorizationChallenge::fromResponse(new Response(401, ['WWW-Authenticate' => 'Bearer error='.$value])); + + $this->assertSame($expected, $challenge->getError()); + } } From 754af0d6a330af9616bf05fef2dad5787408fe90 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Wed, 4 Nov 2026 23:20:45 +0100 Subject: [PATCH 03/10] [Client] Tighten resource containment and keep slash and query in metadata URLs --- src/Client/Auth/MetadataDiscovery.php | 8 ++-- src/Client/Auth/ProtectedResourceMetadata.php | 38 +++++++++++++++++-- .../Client/Auth/MetadataDiscoveryTest.php | 37 ++++++++++++++++++ .../Auth/ProtectedResourceMetadataTest.php | 26 +++++++++++++ 4 files changed, 101 insertions(+), 8 deletions(-) diff --git a/src/Client/Auth/MetadataDiscovery.php b/src/Client/Auth/MetadataDiscovery.php index 0690f489..95e3bdc2 100644 --- a/src/Client/Auth/MetadataDiscovery.php +++ b/src/Client/Auth/MetadataDiscovery.php @@ -108,11 +108,11 @@ public static function protectedResourceCandidates(string $resourceUrl): array return []; } - $origin = self::origin($parts); - $path = trim($parts['path'] ?? '', '/'); - $root = $origin.'/.well-known/oauth-protected-resource'; + $root = self::origin($parts).'/.well-known/oauth-protected-resource'; + $path = $parts['path'] ?? ''; + $suffix = ('/' === $path ? '' : $path).(isset($parts['query']) ? '?'.$parts['query'] : ''); - return '' === $path ? [$root] : [$root.'/'.$path, $root]; + return '' === $suffix ? [$root] : [$root.$suffix, $root]; } /** diff --git a/src/Client/Auth/ProtectedResourceMetadata.php b/src/Client/Auth/ProtectedResourceMetadata.php index da7e306b..5520141e 100644 --- a/src/Client/Auth/ProtectedResourceMetadata.php +++ b/src/Client/Auth/ProtectedResourceMetadata.php @@ -75,17 +75,47 @@ public static function isWithin(string $url, string $resource): bool return false; } - foreach (['scheme', 'host', 'port'] as $part) { - if (($resource[$part] ?? null) !== ($target[$part] ?? null)) { - return false; - } + if (self::origin($resource) !== self::origin($target)) { + return false; + } + + if (isset($resource['query']) && ($target['query'] ?? null) !== $resource['query']) { + return false; } $resourcePath = rtrim($resource['path'] ?? '', '/'); $targetPath = $target['path'] ?? ''; + if (self::hasDotSegment($resourcePath) || self::hasDotSegment($targetPath)) { + return false; + } + return '' === $resourcePath || $targetPath === $resourcePath || str_starts_with($targetPath, $resourcePath.'/'); } + + /** + * @param array{scheme?: string, host?: string, port?: int} $parts + * + * @return array{string, string, ?int} + */ + private static function origin(array $parts): array + { + $scheme = strtolower($parts['scheme'] ?? ''); + + return [$scheme, strtolower($parts['host'] ?? ''), $parts['port'] ?? ['https' => 443, 'http' => 80][$scheme] ?? null]; + } + + private static function hasDotSegment(string $path): bool + { + // Rejected rather than resolved, since the server may resolve them differently than we would. + foreach (explode('/', rawurldecode($path)) as $segment) { + if ('.' === $segment || '..' === $segment) { + return true; + } + } + + return false; + } } diff --git a/tests/Unit/Client/Auth/MetadataDiscoveryTest.php b/tests/Unit/Client/Auth/MetadataDiscoveryTest.php index bc516c2f..f9acdbd9 100644 --- a/tests/Unit/Client/Auth/MetadataDiscoveryTest.php +++ b/tests/Unit/Client/Auth/MetadataDiscoveryTest.php @@ -42,6 +42,43 @@ public static function protectedResourceProvider(): iterable ['https://mcp.example.com/.well-known/oauth-protected-resource'], ]; + yield 'no path has only one location' => [ + 'https://mcp.example.com', + ['https://mcp.example.com/.well-known/oauth-protected-resource'], + ]; + + yield 'a trailing slash is kept' => [ + 'https://mcp.example.com/mcp/', + [ + 'https://mcp.example.com/.well-known/oauth-protected-resource/mcp/', + 'https://mcp.example.com/.well-known/oauth-protected-resource', + ], + ]; + + yield 'a query is kept' => [ + 'https://mcp.example.com/mcp?x=1', + [ + 'https://mcp.example.com/.well-known/oauth-protected-resource/mcp?x=1', + 'https://mcp.example.com/.well-known/oauth-protected-resource', + ], + ]; + + yield 'a trailing slash and a query are kept' => [ + 'https://mcp.example.com/mcp/?tenant=a', + [ + 'https://mcp.example.com/.well-known/oauth-protected-resource/mcp/?tenant=a', + 'https://mcp.example.com/.well-known/oauth-protected-resource', + ], + ]; + + yield 'a query on the root endpoint' => [ + 'https://mcp.example.com/?x=1', + [ + 'https://mcp.example.com/.well-known/oauth-protected-resource?x=1', + 'https://mcp.example.com/.well-known/oauth-protected-resource', + ], + ]; + yield 'the port is part of the origin' => [ 'http://localhost:8000/api/mcp', [ diff --git a/tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php b/tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php index 23d05305..88e26ce0 100644 --- a/tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php +++ b/tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php @@ -32,6 +32,32 @@ public static function coverageProvider(): iterable yield 'a different host' => ['https://evil.example.com/mcp', 'https://mcp.example.com/mcp', false]; yield 'a different scheme' => ['http://mcp.example.com/mcp', 'https://mcp.example.com/mcp', false]; yield 'a different port' => ['http://localhost:9000/mcp', 'http://localhost:8000/mcp', false]; + + yield 'the same query' => ['https://mcp.example.com/mcp?tenant=a', 'https://mcp.example.com/mcp?tenant=a', true]; + yield 'the same query below the resource' => ['https://mcp.example.com/mcp?tenant=a', 'https://mcp.example.com/mcp/sub?tenant=a', true]; + yield 'an endpoint query on a resource without one' => ['https://mcp.example.com/mcp', 'https://mcp.example.com/mcp?tenant=a', true]; + yield 'a different query' => ['https://mcp.example.com/mcp?tenant=a', 'https://mcp.example.com/mcp?tenant=b', false]; + yield 'a missing query' => ['https://mcp.example.com/mcp?tenant=a', 'https://mcp.example.com/mcp', false]; + + yield 'a dot segment leaving the resource' => ['https://mcp.example.com/api', 'https://mcp.example.com/api/../other', false]; + yield 'a nested dot segment leaving the resource' => ['https://mcp.example.com/api', 'https://mcp.example.com/api/x/../../other', false]; + yield 'an encoded dot segment' => ['https://mcp.example.com/api', 'https://mcp.example.com/api/%2e%2e/other', false]; + yield 'a half encoded dot segment' => ['https://mcp.example.com/api', 'https://mcp.example.com/api/.%2e/other', false]; + yield 'a dot segment with an encoded slash' => ['https://mcp.example.com/api', 'https://mcp.example.com/api/..%2fother', false]; + yield 'a single dot segment' => ['https://mcp.example.com/api', 'https://mcp.example.com/api/./mcp', false]; + yield 'a dot segment in the resource' => ['https://mcp.example.com/api/..', 'https://mcp.example.com/api/../mcp', false]; + yield 'dots that are no dot segment' => ['https://mcp.example.com/api', 'https://mcp.example.com/api/a..b/...', true]; + yield 'dots in the query' => ['https://mcp.example.com/api', 'https://mcp.example.com/api/x?y=/../z', true]; + + yield 'an explicit default https port on the endpoint' => ['https://mcp.example.com/api', 'https://mcp.example.com:443/api', true]; + yield 'an explicit default https port on the resource' => ['https://mcp.example.com:443/api', 'https://mcp.example.com/api', true]; + yield 'an explicit default http port' => ['http://localhost/api', 'http://localhost:80/api', true]; + yield 'an uppercase host' => ['https://mcp.example.com/api', 'https://MCP.example.com/api', true]; + yield 'an uppercase scheme' => ['https://mcp.example.com/api', 'HTTPS://mcp.example.com/api', true]; + yield 'the https port with the http scheme' => ['https://mcp.example.com/api', 'http://mcp.example.com:443/api', false]; + yield 'the http port with the https scheme' => ['https://mcp.example.com/api', 'https://mcp.example.com:80/api', false]; + yield 'a non-default port' => ['https://mcp.example.com/api', 'https://mcp.example.com:8443/api', false]; + yield 'a path differing in case' => ['https://mcp.example.com/api', 'https://mcp.example.com/API', false]; } #[DataProvider('coverageProvider')] From 6e1df85a1a5c8ce7b2c92e908d17e1612b2648f5 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Wed, 4 Nov 2026 23:57:43 +0100 Subject: [PATCH 04/10] [Client] Keep challenge parameters with several spaces before the equals sign --- src/Client/Auth/AuthorizationChallenge.php | 2 +- tests/Unit/Client/Auth/AuthorizationChallengeTest.php | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Client/Auth/AuthorizationChallenge.php b/src/Client/Auth/AuthorizationChallenge.php index 43f37a4e..83d802c0 100644 --- a/src/Client/Auth/AuthorizationChallenge.php +++ b/src/Client/Auth/AuthorizationChallenge.php @@ -79,7 +79,7 @@ private static function parseBearerParameters(string $header): ?array $parameters = null; foreach ($members[0] as $member) { - if (preg_match('/^\s*([^\s=,"]+)(?:\s+(?!=)|\s*$)(.*)$/s', $member, $scheme)) { + if (preg_match('/^\s*([^\s=,"]+)(?:\s++(?!=)|\s*$)(.*)$/s', $member, $scheme)) { if (null !== $parameters) { break; } diff --git a/tests/Unit/Client/Auth/AuthorizationChallengeTest.php b/tests/Unit/Client/Auth/AuthorizationChallengeTest.php index 69c3f0d8..73f041db 100644 --- a/tests/Unit/Client/Auth/AuthorizationChallengeTest.php +++ b/tests/Unit/Client/Auth/AuthorizationChallengeTest.php @@ -95,6 +95,9 @@ public static function combinedHeaderProvider(): iterable yield 'a scheme inside a quoted value is no scheme' => ['Basic realm="a, Bearer scope=\\"evil\\""', []]; yield 'a comma inside a quoted value does not split' => ['Bearer error="use Basic realm=x, scope=evil", scope="a"', ['error' => 'use Basic realm=x, scope=evil', 'scope' => 'a']]; yield 'an empty list member is skipped' => ['Bearer scope="a",, error="b"', ['scope' => 'a', 'error' => 'b']]; + yield 'several spaces before the equals sign' => ['Bearer scope="read", error ="invalid_token", resource_metadata="https://good"', ['scope' => 'read', 'error' => 'invalid_token', 'resource_metadata' => 'https://good']]; + yield 'several tabs before the equals sign' => ["Bearer scope=\"read\", error\t\t=\"t\"", ['scope' => 'read', 'error' => 't']]; + yield 'several spaces around the equals sign' => ['Bearer scope="read", error = "t"', ['scope' => 'read', 'error' => 't']]; yield 'the scheme is case-insensitive' => ['bEARER scope="a"', ['scope' => 'a']]; yield 'a scheme merely starting with Bearer' => ['Bearer-x scope="a"', []]; } From ecb24c71ed3b770a78b664f092231e342088e991 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Wed, 4 Nov 2026 23:57:44 +0100 Subject: [PATCH 05/10] [Client] Refuse insecure issuers and allow metadata without an authorization endpoint --- .../Auth/AuthorizationServerMetadata.php | 20 +++++-- src/Client/Auth/MetadataDiscovery.php | 6 ++ .../Auth/AuthorizationServerMetadataTest.php | 56 ++++++++++++++++++- .../Client/Auth/MetadataDiscoveryTest.php | 46 +++++++++++++++ 4 files changed, 120 insertions(+), 8 deletions(-) diff --git a/src/Client/Auth/AuthorizationServerMetadata.php b/src/Client/Auth/AuthorizationServerMetadata.php index 60e691ba..19aea26d 100644 --- a/src/Client/Auth/AuthorizationServerMetadata.php +++ b/src/Client/Auth/AuthorizationServerMetadata.php @@ -25,7 +25,7 @@ final class AuthorizationServerMetadata { /** * @param string $issuer the issuer identifier, which must match how the document was found - * @param string $authorizationEndpoint where the user is sent to approve the request + * @param ?string $authorizationEndpoint where the user is sent to approve the request, when a supported grant uses it * @param string $tokenEndpoint where codes and refresh tokens are exchanged * @param ?string $registrationEndpoint where a client can register itself, when the server allows it * @param string[] $scopesSupported scopes the server knows about @@ -37,7 +37,7 @@ final class AuthorizationServerMetadata */ public function __construct( public readonly string $issuer, - public readonly string $authorizationEndpoint, + public readonly ?string $authorizationEndpoint, public readonly string $tokenEndpoint, public readonly ?string $registrationEndpoint = null, public readonly array $scopesSupported = [], @@ -57,7 +57,15 @@ public function __construct( */ public static function fromArray(array $data): self { - foreach (['issuer', 'authorization_endpoint', 'token_endpoint'] as $required) { + $grantTypes = self::strings($data['grant_types_supported'] ?? null); + $requiredFields = ['issuer', 'token_endpoint']; + + // RFC 8414 section 2 requires this endpoint only when a supported grant uses it. + if ([] === $grantTypes || [] !== array_intersect($grantTypes, ['authorization_code', 'implicit'])) { + $requiredFields[] = 'authorization_endpoint'; + } + + foreach ($requiredFields as $required) { if (!isset($data[$required]) || !\is_string($data[$required])) { throw new InvalidArgumentException(\sprintf('The authorization server metadata is missing a string "%s".', $required)); } @@ -69,14 +77,14 @@ public static function fromArray(array $data): self } } - /* @var array{issuer: string, authorization_endpoint: string, token_endpoint: string} $data */ + /* @var array{issuer: string, token_endpoint: string} $data */ return new self( $data['issuer'], - $data['authorization_endpoint'], + \is_string($data['authorization_endpoint'] ?? null) ? $data['authorization_endpoint'] : null, $data['token_endpoint'], \is_string($data['registration_endpoint'] ?? null) ? $data['registration_endpoint'] : null, self::strings($data['scopes_supported'] ?? null), - self::strings($data['grant_types_supported'] ?? null), + $grantTypes, self::strings($data['token_endpoint_auth_methods_supported'] ?? null), self::strings($data['code_challenge_methods_supported'] ?? null), true === ($data['client_id_metadata_document_supported'] ?? null), diff --git a/src/Client/Auth/MetadataDiscovery.php b/src/Client/Auth/MetadataDiscovery.php index 95e3bdc2..de07162b 100644 --- a/src/Client/Auth/MetadataDiscovery.php +++ b/src/Client/Auth/MetadataDiscovery.php @@ -66,6 +66,12 @@ public function discoverProtectedResource(string $resourceUrl, ?string $metadata */ public function discoverAuthorizationServer(string $issuer): ?AuthorizationServerMetadata { + if (!AuthorizationServerMetadata::isTransportSecure($issuer)) { + $this->logger->error('Refusing authorization server discovery: the issuer is neither an HTTPS URL nor a loopback address.', ['issuer' => $issuer]); + + return null; + } + foreach (self::authorizationServerCandidates($issuer) as $candidate) { $document = $this->fetch($candidate); diff --git a/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php b/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php index 5ece4a63..84d50136 100644 --- a/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php +++ b/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php @@ -52,6 +52,60 @@ public function testRequiresEndpoints(): void ]); } + #[TestDox('the authorization endpoint may be absent when no supported grant uses it')] + public function testAuthorizationEndpointIsOptionalWithoutARedirectGrant(): void + { + $metadata = AuthorizationServerMetadata::fromArray([ + 'issuer' => 'https://auth.example.com', + 'token_endpoint' => 'https://auth.example.com/token', + 'grant_types_supported' => ['client_credentials'], + ]); + + $this->assertNull($metadata->authorizationEndpoint); + $this->assertTrue($metadata->supportsGrant('client_credentials')); + } + + /** + * @return iterable}> + */ + public static function authorizationEndpointRequiredProvider(): iterable + { + yield 'no grant list' => [[]]; + yield 'an empty grant list' => [['grant_types_supported' => []]]; + yield 'the authorization code grant' => [['grant_types_supported' => ['client_credentials', 'authorization_code']]]; + yield 'the implicit grant' => [['grant_types_supported' => ['implicit']]]; + } + + /** + * @param array $document + */ + #[DataProvider('authorizationEndpointRequiredProvider')] + #[TestDox('the authorization endpoint is required for: $_dataName')] + public function testAuthorizationEndpointIsRequiredForARedirectGrant(array $document): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('authorization_endpoint'); + + AuthorizationServerMetadata::fromArray($document + [ + 'issuer' => 'https://auth.example.com', + 'token_endpoint' => 'https://auth.example.com/token', + ]); + } + + #[TestDox('an insecure authorization endpoint is refused even when no grant uses it')] + public function testRejectsAnInsecureUnusedAuthorizationEndpoint(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('neither an HTTPS URL nor a loopback address'); + + AuthorizationServerMetadata::fromArray([ + 'issuer' => 'https://auth.example.com', + 'authorization_endpoint' => 'http://auth.example.com/authorize', + 'token_endpoint' => 'https://auth.example.com/token', + 'grant_types_supported' => ['client_credentials'], + ]); + } + #[TestDox('an unstated grant list means the authorization code grant, and nothing else')] public function testDefaultGrantTypes(): void { @@ -65,8 +119,6 @@ public function testDefaultGrantTypes(): void $this->assertFalse($metadata->supportsGrant('refresh_token')); } - // These URLs come out of a document whose location a hostile MCP server chose, and - // one of them is opened in the user's browser. #[TestDox('endpoints that are not HTTPS are refused')] public function testRejectsInsecureEndpoints(): void { diff --git a/tests/Unit/Client/Auth/MetadataDiscoveryTest.php b/tests/Unit/Client/Auth/MetadataDiscoveryTest.php index f9acdbd9..8985a61a 100644 --- a/tests/Unit/Client/Auth/MetadataDiscoveryTest.php +++ b/tests/Unit/Client/Auth/MetadataDiscoveryTest.php @@ -159,6 +159,52 @@ public function testRejectsIssuerMismatch(): void $this->assertNull($this->discovery($client)->discoverAuthorizationServer('https://auth.example.com')); } + #[TestDox('an issuer that is neither https nor loopback is not even requested')] + public function testRefusesAnInsecureIssuer(): void + { + $client = new RecordingClient([ + 'http://auth.example.com/.well-known/oauth-authorization-server' => self::json([ + 'issuer' => 'http://auth.example.com', + 'authorization_endpoint' => 'https://attacker.example.com/authorize', + 'token_endpoint' => 'https://attacker.example.com/token', + ]), + ]); + + $this->assertNull($this->discovery($client)->discoverAuthorizationServer('http://auth.example.com')); + $this->assertSame([], $client->requested); + } + + #[TestDox('a loopback issuer may use plain http')] + public function testAllowsALoopbackIssuer(): void + { + $client = new RecordingClient([ + 'http://localhost:8080/.well-known/oauth-authorization-server' => self::json([ + 'issuer' => 'http://localhost:8080', + 'authorization_endpoint' => 'http://localhost:8080/authorize', + 'token_endpoint' => 'http://localhost:8080/token', + ]), + ]); + + $this->assertNotNull($this->discovery($client)->discoverAuthorizationServer('http://localhost:8080')); + } + + #[TestDox('metadata without an authorization endpoint is usable when no grant needs it')] + public function testAcceptsMetadataWithoutAnAuthorizationEndpoint(): void + { + $client = new RecordingClient([ + 'https://auth.example.com/.well-known/oauth-authorization-server' => self::json([ + 'issuer' => 'https://auth.example.com', + 'token_endpoint' => 'https://auth.example.com/token', + 'grant_types_supported' => ['client_credentials'], + ]), + ]); + + $metadata = $this->discovery($client)->discoverAuthorizationServer('https://auth.example.com'); + + $this->assertNotNull($metadata); + $this->assertNull($metadata->authorizationEndpoint); + } + #[TestDox('a metadata URL named by the challenge is used instead of guessing')] public function testHonoursTheChallengeMetadataUrl(): void { From 119e7e63d563e0550b37cf1fb7a35929694d76d7 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Thu, 5 Nov 2026 00:52:18 +0100 Subject: [PATCH 06/10] [Client] Read WWW-Authenticate field lines as one combined value --- src/Client/Auth/AuthorizationChallenge.php | 18 ++++---------- .../Auth/AuthorizationChallengeTest.php | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/Client/Auth/AuthorizationChallenge.php b/src/Client/Auth/AuthorizationChallenge.php index 83d802c0..9cca35d7 100644 --- a/src/Client/Auth/AuthorizationChallenge.php +++ b/src/Client/Auth/AuthorizationChallenge.php @@ -31,15 +31,7 @@ private function __construct(public readonly array $parameters) public static function fromResponse(ResponseInterface $response): self { - foreach ($response->getHeader('WWW-Authenticate') as $header) { - $parameters = self::parseBearerParameters($header); - - if (null !== $parameters) { - return new self($parameters); - } - } - - return new self([]); + return new self(self::parseBearerParameters($response->getHeaderLine('WWW-Authenticate'))); } /** @@ -68,12 +60,12 @@ public function getError(): ?string /** * Read the Bearer challenge out of a field value that may list several challenges. * - * @return array|null null when the field value holds no Bearer challenge + * @return array */ - private static function parseBearerParameters(string $header): ?array + private static function parseBearerParameters(string $header): array { if (!preg_match_all('/(?:[^",]|"(?:[^"\\\\]|\\\\.)*")+/s', $header, $members)) { - return null; + return []; } $parameters = null; @@ -102,6 +94,6 @@ private static function parseBearerParameters(string $header): ?array } } - return $parameters; + return $parameters ?? []; } } diff --git a/tests/Unit/Client/Auth/AuthorizationChallengeTest.php b/tests/Unit/Client/Auth/AuthorizationChallengeTest.php index 73f041db..11105105 100644 --- a/tests/Unit/Client/Auth/AuthorizationChallengeTest.php +++ b/tests/Unit/Client/Auth/AuthorizationChallengeTest.php @@ -81,6 +81,30 @@ public function testIgnoresOtherSchemes(): void $this->assertSame(['mcp:read'], AuthorizationChallenge::fromResponse($response)->getScopes()); } + /** + * @return iterable}> + */ + public static function splitHeaderProvider(): iterable + { + yield 'one challenge across two lines' => [['Bearer scope="read"', 'resource_metadata="https://good"'], ['scope' => 'read', 'resource_metadata' => 'https://good']]; + yield 'a bare scheme on its own line' => [['Negotiate', 'Bearer scope="a"'], ['scope' => 'a']]; + yield 'a later line with another scheme' => [['Bearer scope="a"', 'Digest scope="evil"'], ['scope' => 'a']]; + yield 'an unterminated quote swallows the next line' => [['Basic realm="x', 'Bearer scope="a"'], []]; + } + + /** + * @param string[] $lines + * @param array $expected + */ + #[DataProvider('splitHeaderProvider')] + #[TestDox('field lines read as one comma-joined value for: $_dataName')] + public function testCombinesFieldLines(array $lines, array $expected): void + { + $challenge = AuthorizationChallenge::fromResponse(new Response(401, ['WWW-Authenticate' => $lines])); + + $this->assertSame($expected, $challenge->parameters); + } + /** * @return iterable}> */ From 803b7c5d2e4cf5e84de2b7572a1ec2b42c00a575 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Thu, 5 Nov 2026 00:52:19 +0100 Subject: [PATCH 07/10] [Client] Try later metadata locations after an issuer mismatch and keep issuer paths intact --- src/Client/Auth/MetadataDiscovery.php | 6 +- .../Client/Auth/MetadataDiscoveryTest.php | 85 +++++++++++++++++++ 2 files changed, 88 insertions(+), 3 deletions(-) diff --git a/src/Client/Auth/MetadataDiscovery.php b/src/Client/Auth/MetadataDiscovery.php index de07162b..aa454f94 100644 --- a/src/Client/Auth/MetadataDiscovery.php +++ b/src/Client/Auth/MetadataDiscovery.php @@ -88,13 +88,13 @@ public function discoverAuthorizationServer(string $issuer): ?AuthorizationServe } if ($metadata->issuer !== $issuer) { - $this->logger->error('Discarding authorization server metadata: the document\'s issuer does not match the issuer it was requested for.', [ + $this->logger->warning('Discarding authorization server metadata: the document\'s issuer does not match the issuer it was requested for.', [ 'expected_issuer' => $issuer, 'metadata_issuer' => $metadata->issuer, 'url' => $candidate, ]); - return null; + continue; } return $metadata; @@ -133,7 +133,7 @@ public static function authorizationServerCandidates(string $issuer): array } $origin = self::origin($parts); - $path = trim($parts['path'] ?? '', '/'); + $path = substr(rtrim($parts['path'] ?? '', '/'), 1); if ('' === $path) { return [ diff --git a/tests/Unit/Client/Auth/MetadataDiscoveryTest.php b/tests/Unit/Client/Auth/MetadataDiscoveryTest.php index 8985a61a..0b3e05e7 100644 --- a/tests/Unit/Client/Auth/MetadataDiscoveryTest.php +++ b/tests/Unit/Client/Auth/MetadataDiscoveryTest.php @@ -109,6 +109,32 @@ public static function authorizationServerProvider(): iterable ]; // RFC 8414 inserts the path into the well-known segment, OpenID Connect appends it. + yield 'a terminating slash on an issuer without a path' => [ + 'https://auth.example.com/', + [ + 'https://auth.example.com/.well-known/oauth-authorization-server', + 'https://auth.example.com/.well-known/openid-configuration', + ], + ]; + + yield 'a terminating slash on an issuer path' => [ + 'https://auth.example.com/tenant1/', + [ + 'https://auth.example.com/.well-known/oauth-authorization-server/tenant1', + 'https://auth.example.com/.well-known/openid-configuration/tenant1', + 'https://auth.example.com/tenant1/.well-known/openid-configuration', + ], + ]; + + yield 'an empty leading segment in an issuer path' => [ + 'https://auth.example.com//tenant1', + [ + 'https://auth.example.com/.well-known/oauth-authorization-server//tenant1', + 'https://auth.example.com/.well-known/openid-configuration//tenant1', + 'https://auth.example.com//tenant1/.well-known/openid-configuration', + ], + ]; + yield 'an issuer with a path' => [ 'https://auth.example.com/tenant1', [ @@ -159,6 +185,65 @@ public function testRejectsIssuerMismatch(): void $this->assertNull($this->discovery($client)->discoverAuthorizationServer('https://auth.example.com')); } + /** + * @return iterable + */ + public static function issuerMismatchFallbackProvider(): iterable + { + yield 'an issuer without a path' => [ + 'https://auth.example.com', + 'https://auth.example.com/.well-known/oauth-authorization-server', + 'https://auth.example.com/.well-known/openid-configuration', + ]; + + yield 'an issuer with a path' => [ + 'https://auth.example.com/tenant1', + 'https://auth.example.com/.well-known/oauth-authorization-server/tenant1', + 'https://auth.example.com/.well-known/openid-configuration/tenant1', + ]; + } + + #[DataProvider('issuerMismatchFallbackProvider')] + #[TestDox('a document naming another issuer does not stop a later location: $_dataName')] + public function testContinuesAfterAnIssuerMismatch(string $issuer, string $mismatched, string $matching): void + { + $client = new RecordingClient([ + $mismatched => self::json([ + 'issuer' => 'https://attacker.example.com', + 'authorization_endpoint' => 'https://attacker.example.com/authorize', + 'token_endpoint' => 'https://attacker.example.com/token', + ]), + $matching => self::json([ + 'issuer' => $issuer, + 'authorization_endpoint' => 'https://auth.example.com/authorize', + 'token_endpoint' => 'https://auth.example.com/token', + ]), + ]); + + $metadata = $this->discovery($client)->discoverAuthorizationServer($issuer); + + $this->assertNotNull($metadata); + $this->assertSame('https://auth.example.com/token', $metadata->tokenEndpoint); + $this->assertSame([$mismatched, $matching], $client->requested); + } + + #[TestDox('every location is tried before giving up on a mismatching issuer')] + public function testTriesEveryLocationOnIssuerMismatch(): void + { + $attacker = self::json([ + 'issuer' => 'https://attacker.example.com', + 'authorization_endpoint' => 'https://attacker.example.com/authorize', + 'token_endpoint' => 'https://attacker.example.com/token', + ]); + $client = new RecordingClient([ + 'https://auth.example.com/.well-known/oauth-authorization-server' => $attacker, + 'https://auth.example.com/.well-known/openid-configuration' => $attacker, + ]); + + $this->assertNull($this->discovery($client)->discoverAuthorizationServer('https://auth.example.com')); + $this->assertCount(2, $client->requested); + } + #[TestDox('an issuer that is neither https nor loopback is not even requested')] public function testRefusesAnInsecureIssuer(): void { From 83c349a08d6684f710bacf96bf9a4a059060066b Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Thu, 5 Nov 2026 01:01:14 +0100 Subject: [PATCH 08/10] [Client] Strip only the terminating slash of a resource path --- src/Client/Auth/ProtectedResourceMetadata.php | 7 ++++++- tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php | 9 +++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Client/Auth/ProtectedResourceMetadata.php b/src/Client/Auth/ProtectedResourceMetadata.php index 5520141e..ce2cb0a7 100644 --- a/src/Client/Auth/ProtectedResourceMetadata.php +++ b/src/Client/Auth/ProtectedResourceMetadata.php @@ -83,7 +83,12 @@ public static function isWithin(string $url, string $resource): bool return false; } - $resourcePath = rtrim($resource['path'] ?? '', '/'); + $resourcePath = $resource['path'] ?? ''; + + if (str_ends_with($resourcePath, '/')) { + $resourcePath = substr($resourcePath, 0, -1); + } + $targetPath = $target['path'] ?? ''; if (self::hasDotSegment($resourcePath) || self::hasDotSegment($targetPath)) { diff --git a/tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php b/tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php index 88e26ce0..b97df5e4 100644 --- a/tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php +++ b/tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php @@ -49,6 +49,15 @@ public static function coverageProvider(): iterable yield 'dots that are no dot segment' => ['https://mcp.example.com/api', 'https://mcp.example.com/api/a..b/...', true]; yield 'dots in the query' => ['https://mcp.example.com/api', 'https://mcp.example.com/api/x?y=/../z', true]; + yield 'a terminating slash on the root resource' => ['https://mcp.example.com/', 'https://mcp.example.com/mcp', true]; + yield 'a terminating slash on the resource, endpoint without it' => ['https://mcp.example.com/api/', 'https://mcp.example.com/api', true]; + yield 'a terminating slash on the resource, endpoint below it' => ['https://mcp.example.com/api/', 'https://mcp.example.com/api/x', true]; + yield 'a terminating slash on the resource, sharing a prefix' => ['https://mcp.example.com/api/', 'https://mcp.example.com/api-v2', false]; + yield 'an empty last segment, endpoint below it' => ['https://mcp.example.com/api//', 'https://mcp.example.com/api//x', true]; + yield 'an empty last segment does not cover the parent' => ['https://mcp.example.com/api//', 'https://mcp.example.com/api', false]; + yield 'an empty last segment does not cover a sibling' => ['https://mcp.example.com/api//', 'https://mcp.example.com/api/x', false]; + yield 'only slashes is not the root' => ['https://mcp.example.com//', 'https://mcp.example.com/mcp', false]; + yield 'an explicit default https port on the endpoint' => ['https://mcp.example.com/api', 'https://mcp.example.com:443/api', true]; yield 'an explicit default https port on the resource' => ['https://mcp.example.com:443/api', 'https://mcp.example.com/api', true]; yield 'an explicit default http port' => ['http://localhost/api', 'http://localhost:80/api', true]; From c5ec79b4223d5039d77aa923826e9e54b41c33cb Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Thu, 5 Nov 2026 01:15:27 +0100 Subject: [PATCH 09/10] [Client] Refuse legacy endpoints on an insecure issuer --- src/Client/Auth/AuthorizationServerMetadata.php | 6 ++++++ .../Auth/AuthorizationServerMetadataTest.php | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/Client/Auth/AuthorizationServerMetadata.php b/src/Client/Auth/AuthorizationServerMetadata.php index 19aea26d..16ce6620 100644 --- a/src/Client/Auth/AuthorizationServerMetadata.php +++ b/src/Client/Auth/AuthorizationServerMetadata.php @@ -94,9 +94,15 @@ public static function fromArray(array $data): self /** * The fixed endpoints a 2025-03-26 era server without metadata is assumed to expose. + * + * @throws InvalidArgumentException when the issuer is neither https nor loopback */ public static function forLegacyServer(string $issuer): self { + if (!self::isTransportSecure($issuer)) { + throw new InvalidArgumentException(\sprintf('The legacy authorization server "%s" is neither an HTTPS URL nor a loopback address.', $issuer)); + } + $base = rtrim($issuer, '/'); return new self($issuer, $base.'/authorize', $base.'/token', $base.'/register'); diff --git a/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php b/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php index 84d50136..4ade0858 100644 --- a/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php +++ b/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php @@ -179,6 +179,23 @@ public function testRejectsLookalikeHosts(string $endpoint): void ]); } + #[TestDox('legacy endpoints are not built on an insecure issuer')] + public function testRefusesAnInsecureLegacyIssuer(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('neither an HTTPS URL nor a loopback address'); + + AuthorizationServerMetadata::forLegacyServer('http://mcp.example.com'); + } + + #[TestDox('legacy endpoints may use plain HTTP on a loopback address')] + public function testAllowsALoopbackLegacyIssuer(): void + { + $metadata = AuthorizationServerMetadata::forLegacyServer('http://localhost:8080'); + + $this->assertSame('http://localhost:8080/token', $metadata->tokenEndpoint); + } + #[TestDox('a server that publishes nothing gets the endpoint names its era prescribed')] public function testLegacyEndpoints(): void { From 2869b330c05dcb69b8190713339944be642ee92a Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Thu, 5 Nov 2026 01:56:20 +0100 Subject: [PATCH 10/10] [Client] Refuse to approve a bearer token for an insecure URL --- src/Client/Auth/ProtectedResourceMetadata.php | 5 +++++ tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php | 3 +++ 2 files changed, 8 insertions(+) diff --git a/src/Client/Auth/ProtectedResourceMetadata.php b/src/Client/Auth/ProtectedResourceMetadata.php index ce2cb0a7..aefb6751 100644 --- a/src/Client/Auth/ProtectedResourceMetadata.php +++ b/src/Client/Auth/ProtectedResourceMetadata.php @@ -68,6 +68,11 @@ public function covers(string $url): bool */ public static function isWithin(string $url, string $resource): bool { + // RFC 6750 section 5.3: bearer tokens require TLS, whatever the resource identifier says. + if (!AuthorizationServerMetadata::isTransportSecure($url)) { + return false; + } + $resource = parse_url($resource); $target = parse_url($url); diff --git a/tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php b/tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php index b97df5e4..060b70bb 100644 --- a/tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php +++ b/tests/Unit/Client/Auth/ProtectedResourceMetadataTest.php @@ -33,6 +33,9 @@ public static function coverageProvider(): iterable yield 'a different scheme' => ['http://mcp.example.com/mcp', 'https://mcp.example.com/mcp', false]; yield 'a different port' => ['http://localhost:9000/mcp', 'http://localhost:8000/mcp', false]; + yield 'plain http on a public host' => ['http://mcp.example.com/mcp', 'http://mcp.example.com/mcp', false]; + yield 'plain http on a loopback host' => ['http://localhost:8000/mcp', 'http://localhost:8000/mcp', true]; + yield 'the same query' => ['https://mcp.example.com/mcp?tenant=a', 'https://mcp.example.com/mcp?tenant=a', true]; yield 'the same query below the resource' => ['https://mcp.example.com/mcp?tenant=a', 'https://mcp.example.com/mcp/sub?tenant=a', true]; yield 'an endpoint query on a resource without one' => ['https://mcp.example.com/mcp', 'https://mcp.example.com/mcp?tenant=a', true];