From 42b990c4bc1c75de6f717498eb547c0bf336a534 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Sun, 8 Nov 2026 13:06:02 +0100 Subject: [PATCH 1/3] [Client] Add client ID metadata documents and a console authorization handler setClientMetadataUrl() uses a published document as the client id where the authorization server supports it, and otherwise registers as before. The URL has to be https with a path. ConsoleAuthorizationHandler prints the authorization URL and reads back the redirected URL, for a shell without a browser. It refuses a bare code, which would skip the state and iss checks. --- CHANGELOG.md | 1 + docs/client/authorization.md | 13 +++- .../Auth/ConsoleAuthorizationHandler.php | 73 +++++++++++++++++++ src/Client/Auth/OAuth.php | 32 ++++++++ src/Client/Auth/OAuthAuthenticator.php | 9 +++ src/Client/Auth/OAuthConfiguration.php | 21 +++--- tests/Conformance/client.php | 6 ++ .../conformance-baseline-2025-11-25.yml | 1 - .../conformance-baseline-2026-07-28.yml | 1 - .../Client/Auth/AuthorizationHandlerTest.php | 69 ++++++++++++++++++ .../Client/Auth/OAuthAuthenticatorTest.php | 43 +++++++++++ tests/Unit/Client/Auth/OAuthTest.php | 36 +++++++++ 12 files changed, 293 insertions(+), 12 deletions(-) create mode 100644 src/Client/Auth/ConsoleAuthorizationHandler.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 78f51623f..13e406311 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * [BC Break] Fix `HttpTransport` reading a response body that is not an SSE stream without a limit, which could exhaust client memory on a large body. It now reads up to the new `maxResponseBodyBytes` argument (default: 8 MiB): a successful response over that limit fails the request, and an error status keeps its body cut at the limit. Raise the limit if your server sends larger results. * [BC Break] `HttpTransport` turns off redirects on a discovered `symfony/http-client` client and treats every `3xx` answer as an error that names the status and target. An endpoint that is reached through a redirect stops working, so configure its final URL. A client you pass must not follow redirects to another origin. * Fix `HttpTransport` taking an `Mcp-Session-Id` from any response: it replaced the session mid-way and, on `2026-07-28`, made `getSessionId()` return a value and `close()` send a `DELETE`. Only the answer to `initialize` sets it. +* Add `OAuth::setClientMetadataUrl()`, to use a client ID metadata document where the authorization server supports it, and `ConsoleAuthorizationHandler` for a shell without a browser. * Add the `auth` parameter of `HttpTransport` and `AuthenticatorInterface`, to authenticate every request and retry the ones the server refuses. `BearerToken` sends a token you already have, and `AuthenticatingHttpClient` offers the same behavior as a PSR-18 decorator. * Add OAuth authorization: pass `OAuth::forApplication('My App')->build()` as `auth` and the client answers a `401` with discovery, dynamic or pre-registered client identity, the authorization code flow with PKCE and a retry. It refreshes tokens, handles step-up challenges and treats the server's metadata as untrusted input. See [Authorization](docs/client/authorization.md). * `AuthenticatingHttpClient` and `OAuth` turn off redirects on a `symfony/http-client` client, so a redirect can't carry a token or client secret to another path or origin. diff --git a/docs/client/authorization.md b/docs/client/authorization.md index 83c914334..3bd4b64f3 100644 --- a/docs/client/authorization.md +++ b/docs/client/authorization.md @@ -47,9 +47,10 @@ $auth = OAuth::forApplication('My App') ->build(); ``` -The SDK contains two handlers: +The SDK contains three handlers: - `LoopbackAuthorizationHandler` is the default. It opens the system browser and listens on the port of the redirect URI until the browser returns. It also writes the URL to `STDERR`, so you can open it yourself if no browser starts. Use it for desktop and command line applications. +- `ConsoleAuthorizationHandler` prints the authorization URL and reads the URL of the redirect from the standard input. Use it in a remote shell or a container without a browser. The user must paste the whole URL, because the SDK checks its `state` and `iss`. - `HeadlessAuthorizationHandler` requests the authorization URL with an HTTP client and reads the code from the redirect. It only works if the authorization server grants access without asking the user (e.g. a test server). For anything else, implement `AuthorizationHandlerInterface`. It has one method: @@ -104,6 +105,16 @@ The MCP server names its authorization server. Without an expected issuer, a hos The client reads from the metadata of the authorization server how to send the secret. +## Using a URL as the Client ID + +If you publish a client ID metadata document, the authorization server can read your client's metadata from its URL. You then don't need a registration per server. The URL must use `https` and have a path. It can't contain a fragment, credentials or dot segments: + +```php +OAuth::forApplication('My App')->setClientMetadataUrl('https://app.example.com/client-metadata.json'); +``` + +The client only uses this URL if the authorization server supports these documents. Otherwise, it registers. A client ID that you pass with `setClientCredentials()` takes precedence. + ## Choosing Scopes By default, the client requests the scopes of the challenge. If there are none, it requests all scopes in `scopes_supported` of the resource. If there are none either, it sends no `scope` parameter. Use `setScopes()` to request your own: diff --git a/src/Client/Auth/ConsoleAuthorizationHandler.php b/src/Client/Auth/ConsoleAuthorizationHandler.php new file mode 100644 index 000000000..711d92f3a --- /dev/null +++ b/src/Client/Auth/ConsoleAuthorizationHandler.php @@ -0,0 +1,73 @@ + + */ +final class ConsoleAuthorizationHandler implements AuthorizationHandlerInterface +{ + /** + * @param resource $input stream the redirected URL is read from + * @param resource $output stream the prompt is written to + */ + public function __construct( + private readonly mixed $input = \STDIN, + private readonly mixed $output = \STDERR, + ) { + } + + public function authorize(string $authorizationUrl, string $redirectUri): array + { + fwrite($this->output, \sprintf( + "\nOpen this URL to authorize the application:\n\n %s\n\nThen paste the full URL you were redirected to: ", + $authorizationUrl, + )); + + $answer = fgets($this->input); + + if (false === $answer || '' === trim($answer)) { + throw new AuthorizationException('No authorization response was provided.'); + } + + $answer = trim($answer); + $query = parse_url($answer, \PHP_URL_QUERY); + + if (!\is_string($query) || '' === $query) { + // A bare code cannot carry the state and issuer the response is checked + // against, so accepting one would quietly skip both checks. + throw new AuthorizationException('Paste the whole URL you were redirected to, not just the code: the rest of it is what proves the response belongs to this request.'); + } + + parse_str($query, $parameters); + + $callback = []; + + foreach ($parameters as $name => $value) { + if (\is_string($name) && \is_scalar($value)) { + $callback[$name] = (string) $value; + } + } + + return $callback; + } +} diff --git a/src/Client/Auth/OAuth.php b/src/Client/Auth/OAuth.php index 063441103..676d50bbf 100644 --- a/src/Client/Auth/OAuth.php +++ b/src/Client/Auth/OAuth.php @@ -36,6 +36,7 @@ final class OAuth private ?string $clientId = null; private ?string $clientSecret = null; private ?string $expectedIssuer = null; + private ?string $clientMetadataUrl = null; private string $applicationType = 'native'; private ?string $clientUri = null; private ?string $softwareId = null; @@ -103,6 +104,36 @@ public function setExpectedIssuer(string $issuer): self return $this; } + /** + * Publish the client's identity at a URL and use that URL as the client id. + * + * Servers that support client id metadata documents read the client's metadata from + * this URL, so one identity works everywhere instead of one registration per server. + * The document must be reachable by the authorization server. A client id set with + * {@see self::setClientCredentials()} takes precedence. + * + * @param string $url an https URL with a path, and without a fragment, credentials or + * dot segments, as client id metadata documents require + */ + public function setClientMetadataUrl(string $url): self + { + $parts = parse_url($url); + + if (false === filter_var($url, \FILTER_VALIDATE_URL) + || !\is_array($parts) + || 'https' !== strtolower($parts['scheme'] ?? '') + || '' === trim($parts['path'] ?? '', '/') + || isset($parts['fragment']) || isset($parts['user']) || isset($parts['pass']) + || [] !== array_intersect(explode('/', rawurldecode($parts['path'] ?? '')), ['.', '..']) + ) { + throw new InvalidArgumentException(\sprintf('The client metadata URL must be an https URL with a path, and without a fragment, credentials or dot segments, got "%s".', $url)); + } + + $this->clientMetadataUrl = $url; + + return $this; + } + /** * Request these scopes instead of those the server asks for; a step-up challenge still adds its own. */ @@ -221,6 +252,7 @@ public function build(): OAuthAuthenticator $this->redirectUri, $this->clientId, $this->clientSecret, + $this->clientMetadataUrl, $this->scopes, $this->offlineAccess, $this->legacyDiscovery, diff --git a/src/Client/Auth/OAuthAuthenticator.php b/src/Client/Auth/OAuthAuthenticator.php index 3485b83cf..de15fb382 100644 --- a/src/Client/Auth/OAuthAuthenticator.php +++ b/src/Client/Auth/OAuthAuthenticator.php @@ -513,6 +513,15 @@ private function resolveClient(AuthorizationServerMetadata $metadata, array $sco ); } + // A client that can publish its metadata at a URL does not need to register at + // all: the URL is the client id, and every server it meets reads the same + // document instead of minting another identity for it. + if ($metadata->clientIdMetadataDocumentSupported && null !== $configuration->clientMetadataUrl) { + $this->logger->debug('Using the client metadata document URL as the client id', ['client_id' => $configuration->clientMetadataUrl]); + + return new ClientRegistration($configuration->clientMetadataUrl, null, TokenEndpointAuthMethod::negotiate($metadata->tokenEndpointAuthMethodsSupported, false)); + } + // Another process may register at the same time, and its tokens would not work with this client. return $this->storage->synchronize(function () use ($metadata, $configuration, $scopes, $scopesSupported): ClientRegistration { $stored = $this->storage->getClientRegistration($metadata->issuer); diff --git a/src/Client/Auth/OAuthConfiguration.php b/src/Client/Auth/OAuthConfiguration.php index 5f47818be..10b3fa16a 100644 --- a/src/Client/Auth/OAuthConfiguration.php +++ b/src/Client/Auth/OAuthConfiguration.php @@ -23,15 +23,17 @@ final class OAuthConfiguration { /** - * @param ClientMetadata $clientMetadata what the client says about itself when registering - * @param string $redirectUri where the authorization server sends the user back - * @param ?string $clientId a client id issued out of band, skipping registration - * @param ?string $clientSecret the matching secret, when there is one - * @param string[] $scopes scopes to request regardless of what is advertised - * @param bool $offlineAccess whether to ask for a refresh token where the server offers one - * @param bool $legacyDiscovery whether to fall back to pre-2025-06-18 discovery when a - * server publishes no protected resource metadata - * @param ?string $expectedIssuer the only authorization server to authorize with + * @param ClientMetadata $clientMetadata what the client says about itself when registering + * @param string $redirectUri where the authorization server sends the user back + * @param ?string $clientId a client id issued out of band, skipping registration + * @param ?string $clientSecret the matching secret, when there is one + * @param ?string $clientMetadataUrl a URL serving the client metadata document, usable as a + * client id against servers that accept one + * @param string[] $scopes scopes to request regardless of what is advertised + * @param bool $offlineAccess whether to ask for a refresh token where the server offers one + * @param bool $legacyDiscovery whether to fall back to pre-2025-06-18 discovery when a + * server publishes no protected resource metadata + * @param ?string $expectedIssuer the only authorization server to authorize with * * @throws InvalidArgumentException when the redirect URI is insecure, or a client id is set without an expected issuer */ @@ -40,6 +42,7 @@ public function __construct( public readonly string $redirectUri = OAuth::DEFAULT_REDIRECT_URI, public readonly ?string $clientId = null, public readonly ?string $clientSecret = null, + public readonly ?string $clientMetadataUrl = null, public readonly array $scopes = [], public readonly bool $offlineAccess = true, public readonly bool $legacyDiscovery = false, diff --git a/tests/Conformance/client.php b/tests/Conformance/client.php index a51aea8bd..99157f77f 100644 --- a/tests/Conformance/client.php +++ b/tests/Conformance/client.php @@ -148,6 +148,12 @@ function buildAuthenticator(string $url, string $scenario, array $context, Psr\L $oauth->setLegacyDiscovery(true); } + // The client metadata document only needs to be a stable URL the authorization + // server recognises; the harness never dereferences it. + if ('auth/basic-cimd' === $scenario) { + $oauth->setClientMetadataUrl('https://conformance-test.local/client-metadata.json'); + } + return $oauth->build(); } diff --git a/tests/Conformance/conformance-baseline-2025-11-25.yml b/tests/Conformance/conformance-baseline-2025-11-25.yml index 699cafdd2..c53cc62c3 100644 --- a/tests/Conformance/conformance-baseline-2025-11-25.yml +++ b/tests/Conformance/conformance-baseline-2025-11-25.yml @@ -1,7 +1,6 @@ client: # Elicitation defaults (SEP-1034) are not filled in when accepting. - elicitation-sep1034-client-defaults - - auth/basic-cimd - auth/client-credentials-jwt - auth/client-credentials-basic - auth/cross-app-access-complete-flow diff --git a/tests/Conformance/conformance-baseline-2026-07-28.yml b/tests/Conformance/conformance-baseline-2026-07-28.yml index cf86e6a7e..8bd7b97ee 100644 --- a/tests/Conformance/conformance-baseline-2026-07-28.yml +++ b/tests/Conformance/conformance-baseline-2026-07-28.yml @@ -1,3 +1,2 @@ client: - http-invalid-tool-headers:sep-2243-invalid-tool-tools-list-gate - - auth/basic-cimd diff --git a/tests/Unit/Client/Auth/AuthorizationHandlerTest.php b/tests/Unit/Client/Auth/AuthorizationHandlerTest.php index 22c61ed67..ea4b39cf9 100644 --- a/tests/Unit/Client/Auth/AuthorizationHandlerTest.php +++ b/tests/Unit/Client/Auth/AuthorizationHandlerTest.php @@ -11,6 +11,7 @@ namespace Mcp\Tests\Unit\Client\Auth; +use Mcp\Client\Auth\ConsoleAuthorizationHandler; use Mcp\Client\Auth\HeadlessAuthorizationHandler; use Mcp\Client\Auth\LoopbackAuthorizationHandler; use Mcp\Exception\AuthorizationException; @@ -191,6 +192,74 @@ public function testHeadlessHandlerNeedsARedirectWithALocation(int $status, arra (new HeadlessAuthorizationHandler($client))->authorize('https://auth.example.com/authorize', 'http://127.0.0.1:8765/callback'); } + #[TestDox('the console handler accepts a pasted redirect URL, parameters and all')] + public function testConsoleHandlerParsesAPastedUrl(): void + { + $this->assertSame( + ['code' => 'abc', 'state' => 'xyz'], + $this->console("http://127.0.0.1:8765/callback?code=abc&state=xyz\n"), + ); + } + + // A bare code carries neither state nor iss, so accepting one would silently skip + // both of the checks that tie the response to this request. + #[TestDox('the console handler refuses a bare code, which would skip the response checks')] + public function testConsoleHandlerRefusesABareCode(): void + { + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('Paste the whole URL'); + + $this->console("abc\n"); + } + + #[TestDox('the console handler shows the URL it wants opened')] + public function testConsoleHandlerPrintsTheUrl(): void + { + $output = fopen('php://memory', 'r+'); + $this->assertIsResource($output); + + $this->console("http://127.0.0.1:8765/callback?code=abc\n", $output); + rewind($output); + + $this->assertStringContainsString('https://auth.example.com/authorize', (string) stream_get_contents($output)); + fclose($output); + } + + #[TestDox('the console handler gives up when nothing is pasted')] + public function testConsoleHandlerRequiresAnAnswer(): void + { + $this->expectException(AuthorizationException::class); + + $this->console("\n"); + } + + /** + * @param resource|null $output + * + * @return array + */ + private function console(string $answer, mixed $output = null): array + { + $input = fopen('php://memory', 'r+'); + $this->assertIsResource($input); + fwrite($input, $answer); + rewind($input); + + $sink = $output ?? fopen('php://memory', 'r+'); + $this->assertIsResource($sink); + + try { + return (new ConsoleAuthorizationHandler($input, $sink)) + ->authorize('https://auth.example.com/authorize', 'http://127.0.0.1:8765/callback'); + } finally { + fclose($input); + + if (null === $output) { + fclose($sink); + } + } + } + #[TestDox('the loopback handler shows the URL when no browser can be opened')] public function testLoopbackHandlerShowsTheUrlWithoutABrowser(): void { diff --git a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php index 9b0da27a0..78f6408c4 100644 --- a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php +++ b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php @@ -1371,6 +1371,47 @@ public function testExpectedIssuerBindsAPublicClient(): void $authenticator->handleChallenge($this->request(), $server->challenge()); } + #[TestDox('a client metadata URL is the client id where the server supports it')] + public function testUsesTheClientMetadataUrlAsClientId(): void + { + $server = new FakeOAuthServer(clientMetadataDocuments: true); + $authenticator = $this->authenticator($server, configuration: OAuth::forApplication('test') + ->setClientMetadataUrl('https://app.example.com/client.json')); + + $authenticator->handleChallenge($this->request(), $server->challenge()); + + $this->assertNotContains('https://auth.example.com/register', $server->requested); + $this->assertSame('https://app.example.com/client.json', $server->authorization['client_id'] ?? null); + $this->assertSame('https://app.example.com/client.json', $server->token['client_id'] ?? null); + } + + #[TestDox('a client metadata URL is ignored where the server does not support it')] + public function testRegistersWithoutClientMetadataSupport(): void + { + $server = new FakeOAuthServer(); + $authenticator = $this->authenticator($server, configuration: OAuth::forApplication('test') + ->setClientMetadataUrl('https://app.example.com/client.json')); + + $authenticator->handleChallenge($this->request(), $server->challenge()); + + $this->assertContains('https://auth.example.com/register', $server->requested); + $this->assertNotSame('https://app.example.com/client.json', $server->authorization['client_id'] ?? null); + } + + #[TestDox('a configured client id wins over a client metadata URL')] + public function testConfiguredClientIdWinsOverTheClientMetadataUrl(): void + { + $server = new FakeOAuthServer(clientMetadataDocuments: true); + $authenticator = $this->authenticator($server, configuration: OAuth::forApplication('test') + ->setClientCredentials('known-client') + ->setExpectedIssuer('https://auth.example.com') + ->setClientMetadataUrl('https://app.example.com/client.json')); + + $authenticator->handleChallenge($this->request(), $server->challenge()); + + $this->assertSame('known-client', $server->authorization['client_id'] ?? null); + } + /** * @return iterable */ @@ -1573,6 +1614,7 @@ public function __construct( private readonly ?array $codeChallengeMethods = ['S256'], private readonly ?string $authorizationEndpoint = null, private readonly bool $announcesIssuer = true, + private readonly bool $clientMetadataDocuments = false, ) { } @@ -1620,6 +1662,7 @@ public function sendRequest(RequestInterface $request): ResponseInterface 'authorization_endpoint' => null !== $this->authorizationEndpoint ? $this->authorizationEndpoint : ($this->hasAuthorizationEndpoint ? ($this->insecureEndpoints ? 'http' : 'https').'://auth.example.com/authorize' : null), 'token_endpoint' => ($this->insecureEndpoints ? 'http' : 'https').'://auth.example.com/token', 'registration_endpoint' => ($this->insecureEndpoints ? 'http' : 'https').'://auth.example.com/register', + 'client_id_metadata_document_supported' => $this->clientMetadataDocuments, 'scopes_supported' => $this->serverScopes, 'grant_types_supported' => $this->hasAuthorizationEndpoint ? $this->grantTypes : ['client_credentials'], 'code_challenge_methods_supported' => $this->codeChallengeMethods, diff --git a/tests/Unit/Client/Auth/OAuthTest.php b/tests/Unit/Client/Auth/OAuthTest.php index 59de70d90..a9a25b9a5 100644 --- a/tests/Unit/Client/Auth/OAuthTest.php +++ b/tests/Unit/Client/Auth/OAuthTest.php @@ -21,6 +21,7 @@ use Mcp\Exception\InvalidArgumentException; use Nyholm\Psr7\Factory\Psr17Factory; use Nyholm\Psr7\Response; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\TestDox; use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\TestCase; @@ -101,6 +102,41 @@ public function testDynamicRegistrationNeedsNoExpectedIssuer(): void $this->assertNull($configuration->expectedIssuer); } + /** + * @return iterable + */ + public static function invalidClientMetadataUrlProvider(): iterable + { + yield 'plain http' => ['http://app.example.com/client.json']; + yield 'no path' => ['https://app.example.com']; + yield 'the root path' => ['https://app.example.com/']; + yield 'not a URL' => ['client.json']; + yield 'a space in the host' => ['https://app example.com/client.json']; + yield 'a fragment' => ['https://app.example.com/client.json#main']; + yield 'a username' => ['https://user@app.example.com/client.json']; + yield 'a password' => ['https://user:secret@app.example.com/client.json']; + yield 'a single-dot segment' => ['https://app.example.com/./client.json']; + yield 'a double-dot segment' => ['https://app.example.com/a/../client.json']; + yield 'an encoded dot segment' => ['https://app.example.com/a/%2e%2e/client.json']; + } + + #[DataProvider('invalidClientMetadataUrlProvider')] + #[TestDox('a client metadata URL needs https and a path: $_dataName')] + public function testRejectsAnInvalidClientMetadataUrl(string $url): void + { + $this->expectException(InvalidArgumentException::class); + + OAuth::forApplication('My App')->setClientMetadataUrl($url); + } + + #[TestDox('a client metadata URL may contain dots and a query')] + public function testAcceptsAValidClientMetadataUrl(): void + { + $url = 'https://app.example.com/v1.2/client.json?tenant=acme'; + + $this->assertSame($url, $this->configurationOf(OAuth::forApplication('My App')->setClientMetadataUrl($url))->clientMetadataUrl); + } + #[TestWith(['http://app.example.com/callback'])] #[TestWith(['http://0.0.0.0:8765/callback'])] #[TestWith(['http://192.168.1.5:8765/callback'])] From 0acf8705497397e7c047a288ce2d107103f8c104 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 10 Nov 2026 00:56:10 +0100 Subject: [PATCH 2/3] [Client] Reject a backslash in the client metadata URL --- src/Client/Auth/OAuth.php | 23 ++++++++++------------- tests/Unit/Client/Auth/OAuthTest.php | 4 +++- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/Client/Auth/OAuth.php b/src/Client/Auth/OAuth.php index 676d50bbf..3e3016f4b 100644 --- a/src/Client/Auth/OAuth.php +++ b/src/Client/Auth/OAuth.php @@ -14,6 +14,7 @@ use Http\Discovery\Psr17FactoryDiscovery; use Mcp\Client\Transport\NoRedirects; use Mcp\Exception\InvalidArgumentException; +use Mcp\Server\Transport\Http\OAuth\SecureUrl; use Psr\Http\Client\ClientInterface; use Psr\Http\Message\RequestFactoryInterface; use Psr\Http\Message\StreamFactoryInterface; @@ -105,23 +106,19 @@ public function setExpectedIssuer(string $issuer): self } /** - * Publish the client's identity at a URL and use that URL as the client id. - * - * Servers that support client id metadata documents read the client's metadata from - * this URL, so one identity works everywhere instead of one registration per server. - * The document must be reachable by the authorization server. A client id set with - * {@see self::setClientCredentials()} takes precedence. - * - * @param string $url an https URL with a path, and without a fragment, credentials or - * dot segments, as client id metadata documents require + * Use the URL of a client ID metadata document as the client id where the server supports it. */ public function setClientMetadataUrl(string $url): self { - $parts = parse_url($url); + try { + $parts = SecureUrl::parse($url, 'client metadata URL'); + } catch (InvalidArgumentException) { + $parts = null; + } - if (false === filter_var($url, \FILTER_VALIDATE_URL) - || !\is_array($parts) - || 'https' !== strtolower($parts['scheme'] ?? '') + if (null === $parts + || false === filter_var($url, \FILTER_VALIDATE_URL) + || 'https' !== strtolower($parts['scheme']) || '' === trim($parts['path'] ?? '', '/') || isset($parts['fragment']) || isset($parts['user']) || isset($parts['pass']) || [] !== array_intersect(explode('/', rawurldecode($parts['path'] ?? '')), ['.', '..']) diff --git a/tests/Unit/Client/Auth/OAuthTest.php b/tests/Unit/Client/Auth/OAuthTest.php index a9a25b9a5..6c8dee754 100644 --- a/tests/Unit/Client/Auth/OAuthTest.php +++ b/tests/Unit/Client/Auth/OAuthTest.php @@ -118,10 +118,12 @@ public static function invalidClientMetadataUrlProvider(): iterable yield 'a single-dot segment' => ['https://app.example.com/./client.json']; yield 'a double-dot segment' => ['https://app.example.com/a/../client.json']; yield 'an encoded dot segment' => ['https://app.example.com/a/%2e%2e/client.json']; + yield 'a backslash dot segment' => ['https://app.example.com/a\\..\\client.json']; + yield 'plain http on loopback' => ['http://localhost/client.json']; } #[DataProvider('invalidClientMetadataUrlProvider')] - #[TestDox('a client metadata URL needs https and a path: $_dataName')] + #[TestDox('an invalid client metadata URL is refused: $_dataName')] public function testRejectsAnInvalidClientMetadataUrl(string $url): void { $this->expectException(InvalidArgumentException::class); From 4039bfe5b9338a9db8afe33281e7f9b4f02c288d Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 10 Nov 2026 00:56:11 +0100 Subject: [PATCH 3/3] [Client] Name the client metadata document when it is refused, and share the callback parsing --- docs/client/authorization.md | 2 + src/Client/Auth/AuthorizationCallback.php | 40 +++++++++++++++++++ .../Auth/ConsoleAuthorizationHandler.php | 20 ++-------- .../Auth/HeadlessAuthorizationHandler.php | 10 +---- .../Auth/LoopbackAuthorizationHandler.php | 10 +---- src/Client/Auth/OAuthAuthenticator.php | 7 ++-- tests/Conformance/client.php | 3 +- .../Client/Auth/AuthorizationHandlerTest.php | 22 ++++++++-- .../Client/Auth/OAuthAuthenticatorTest.php | 19 +++++++++ 9 files changed, 90 insertions(+), 43 deletions(-) create mode 100644 src/Client/Auth/AuthorizationCallback.php diff --git a/docs/client/authorization.md b/docs/client/authorization.md index 3bd4b64f3..afafa4b7e 100644 --- a/docs/client/authorization.md +++ b/docs/client/authorization.md @@ -113,6 +113,8 @@ If you publish a client ID metadata document, the authorization server can read OAuth::forApplication('My App')->setClientMetadataUrl('https://app.example.com/client-metadata.json'); ``` +The document must be a JSON object whose `client_id` is exactly this URL. It must also contain `client_name` and `redirect_uris`, and `redirect_uris` must list the redirect URI of the client (by default `http://127.0.0.1:8765/callback`). The authorization server must be able to fetch the document. + The client only uses this URL if the authorization server supports these documents. Otherwise, it registers. A client ID that you pass with `setClientCredentials()` takes precedence. ## Choosing Scopes diff --git a/src/Client/Auth/AuthorizationCallback.php b/src/Client/Auth/AuthorizationCallback.php new file mode 100644 index 000000000..239412790 --- /dev/null +++ b/src/Client/Auth/AuthorizationCallback.php @@ -0,0 +1,40 @@ + + */ +final class AuthorizationCallback +{ + /** + * @return array + */ + public static function parameters(string $query): array + { + parse_str($query, $parameters); + + $callback = []; + + foreach ($parameters as $name => $value) { + if (\is_string($name) && \is_scalar($value)) { + $callback[$name] = (string) $value; + } + } + + return $callback; + } +} diff --git a/src/Client/Auth/ConsoleAuthorizationHandler.php b/src/Client/Auth/ConsoleAuthorizationHandler.php index 711d92f3a..63b5a4b60 100644 --- a/src/Client/Auth/ConsoleAuthorizationHandler.php +++ b/src/Client/Auth/ConsoleAuthorizationHandler.php @@ -14,13 +14,7 @@ use Mcp\Exception\AuthorizationException; /** - * Prints the authorization URL and reads back whatever the browser landed on. - * - * The fallback for environments where nothing can listen on a loopback port -- a remote - * shell, a container without a browser, a redirect URI the authorization server insists - * must be a real https address. The user pastes the whole redirected URL rather than - * just the code, so `state` and `iss` survive the round trip and can still be checked -- - * a bare code would leave both with nothing to compare against, and is refused. + * Prints the authorization URL and reads the redirected URL back, for a shell without a browser. * * @author Christopher Hertel */ @@ -53,19 +47,13 @@ public function authorize(string $authorizationUrl, string $redirectUri): array $query = parse_url($answer, \PHP_URL_QUERY); if (!\is_string($query) || '' === $query) { - // A bare code cannot carry the state and issuer the response is checked - // against, so accepting one would quietly skip both checks. throw new AuthorizationException('Paste the whole URL you were redirected to, not just the code: the rest of it is what proves the response belongs to this request.'); } - parse_str($query, $parameters); - - $callback = []; + $callback = AuthorizationCallback::parameters($query); - foreach ($parameters as $name => $value) { - if (\is_string($name) && \is_scalar($value)) { - $callback[$name] = (string) $value; - } + if (!isset($callback['code']) && !isset($callback['error'])) { + throw new AuthorizationException('The pasted URL carries neither a code nor an error. Paste the URL you were redirected to after signing in, not the one you opened.'); } return $callback; diff --git a/src/Client/Auth/HeadlessAuthorizationHandler.php b/src/Client/Auth/HeadlessAuthorizationHandler.php index 6a7a68c0b..1db2fd55b 100644 --- a/src/Client/Auth/HeadlessAuthorizationHandler.php +++ b/src/Client/Auth/HeadlessAuthorizationHandler.php @@ -52,15 +52,7 @@ public function authorize(string $authorizationUrl, string $redirectUri): array throw new AuthorizationException(\sprintf('The authorization server redirected to "%s", which carries no query parameters.', $location)); } - parse_str($query, $parameters); - - $callback = []; - - foreach ($parameters as $name => $value) { - if (\is_string($name) && \is_scalar($value)) { - $callback[$name] = (string) $value; - } - } + $callback = AuthorizationCallback::parameters($query); // Only the names, as the query carries the authorization code. $this->logger->debug('Authorization endpoint redirected', ['parameters' => array_keys($callback)]); diff --git a/src/Client/Auth/LoopbackAuthorizationHandler.php b/src/Client/Auth/LoopbackAuthorizationHandler.php index e3663e993..6c7ec3a8f 100644 --- a/src/Client/Auth/LoopbackAuthorizationHandler.php +++ b/src/Client/Auth/LoopbackAuthorizationHandler.php @@ -107,15 +107,7 @@ private function serve($connection, string $callbackPath, ?string $state): ?arra } $target = parse_url($matches[1]); - parse_str(\is_string($target['query'] ?? null) ? $target['query'] : '', $parameters); - - $callback = []; - - foreach ($parameters as $name => $value) { - if (\is_string($name) && \is_scalar($value)) { - $callback[$name] = (string) $value; - } - } + $callback = AuthorizationCallback::parameters(\is_string($target['query'] ?? null) ? $target['query'] : ''); $isCallback = [] !== $callback && $callbackPath === ($target['path'] ?? null) diff --git a/src/Client/Auth/OAuthAuthenticator.php b/src/Client/Auth/OAuthAuthenticator.php index de15fb382..3ea78b574 100644 --- a/src/Client/Auth/OAuthAuthenticator.php +++ b/src/Client/Auth/OAuthAuthenticator.php @@ -130,6 +130,10 @@ public function handleChallenge(RequestInterface $request, ResponseInterface $re try { $token = $this->refresh($refreshable, $metadata, $client, $needed, $resource, $request); } catch (OAuthErrorException $e) { + if ($client->clientId === $this->configuration->clientMetadataUrl) { + throw new OAuthErrorException(\sprintf('The authorization server "%s" answered invalid_client for the client metadata document "%s". Check that it can fetch the document and that its "client_id" and "redirect_uris" match.', $metadata->issuer, $client->clientId), $e->error); + } + if (!$client->dynamic) { throw new OAuthErrorException(\sprintf('The authorization server "%s" answered invalid_client for the configured client "%s". Check the client ID and secret given to setClientCredentials().', $metadata->issuer, $client->clientId), $e->error); } @@ -513,9 +517,6 @@ private function resolveClient(AuthorizationServerMetadata $metadata, array $sco ); } - // A client that can publish its metadata at a URL does not need to register at - // all: the URL is the client id, and every server it meets reads the same - // document instead of minting another identity for it. if ($metadata->clientIdMetadataDocumentSupported && null !== $configuration->clientMetadataUrl) { $this->logger->debug('Using the client metadata document URL as the client id', ['client_id' => $configuration->clientMetadataUrl]); diff --git a/tests/Conformance/client.php b/tests/Conformance/client.php index 99157f77f..776e17005 100644 --- a/tests/Conformance/client.php +++ b/tests/Conformance/client.php @@ -148,8 +148,7 @@ function buildAuthenticator(string $url, string $scenario, array $context, Psr\L $oauth->setLegacyDiscovery(true); } - // The client metadata document only needs to be a stable URL the authorization - // server recognises; the harness never dereferences it. + // The harness never fetches the document. if ('auth/basic-cimd' === $scenario) { $oauth->setClientMetadataUrl('https://conformance-test.local/client-metadata.json'); } diff --git a/tests/Unit/Client/Auth/AuthorizationHandlerTest.php b/tests/Unit/Client/Auth/AuthorizationHandlerTest.php index ea4b39cf9..560e13697 100644 --- a/tests/Unit/Client/Auth/AuthorizationHandlerTest.php +++ b/tests/Unit/Client/Auth/AuthorizationHandlerTest.php @@ -143,8 +143,6 @@ public function testLoopbackHandlerTimesOut(): void ->authorize('https://auth.example.com/authorize', \sprintf('http://127.0.0.1:%d/callback', self::freePort())); } - // A bare code carries neither state nor iss, so accepting one would silently skip - // both of the checks that tie the response to this request. #[TestDox('the headless handler sends its request with the given HTTP client')] public function testHeadlessHandlerUsesTheGivenClient(): void { @@ -201,8 +199,6 @@ public function testConsoleHandlerParsesAPastedUrl(): void ); } - // A bare code carries neither state nor iss, so accepting one would silently skip - // both of the checks that tie the response to this request. #[TestDox('the console handler refuses a bare code, which would skip the response checks')] public function testConsoleHandlerRefusesABareCode(): void { @@ -212,6 +208,24 @@ public function testConsoleHandlerRefusesABareCode(): void $this->console("abc\n"); } + #[TestDox('the console handler refuses the authorization URL pasted back instead of the redirect')] + public function testConsoleHandlerRefusesAUrlWithoutCodeOrError(): void + { + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('neither a code nor an error'); + + $this->console("https://auth.example.com/authorize?response_type=code&state=xyz\n"); + } + + #[TestDox('the console handler passes an error response on')] + public function testConsoleHandlerPassesAnErrorOn(): void + { + $this->assertSame( + ['error' => 'access_denied', 'state' => 'xyz'], + $this->console("http://127.0.0.1:8765/callback?error=access_denied&state=xyz\n"), + ); + } + #[TestDox('the console handler shows the URL it wants opened')] public function testConsoleHandlerPrintsTheUrl(): void { diff --git a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php index 78f6408c4..971d06226 100644 --- a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php +++ b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php @@ -1398,6 +1398,25 @@ public function testRegistersWithoutClientMetadataSupport(): void $this->assertNotSame('https://app.example.com/client.json', $server->authorization['client_id'] ?? null); } + #[TestDox('a client metadata URL the token endpoint refuses on a refresh names the document')] + public function testClientMetadataUrlRefusedOnRefreshNamesTheDocument(): void + { + $server = new FakeOAuthServer(refreshTokens: true, clientMetadataDocuments: true); + $server->unknownClient = 'https://app.example.com/client.json'; + $storage = new InMemoryCredentialStorage(); + $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('expired', 'Bearer', time() - 60, 'refresh')); + $configuration = OAuth::forApplication('test')->setClientMetadataUrl('https://app.example.com/client.json'); + + try { + $this->authenticator($server, storage: $storage, configuration: $configuration)->handleChallenge($this->request(), $server->challenge()); + $this->fail('The refused client should have failed.'); + } catch (OAuthErrorException $e) { + $this->assertSame('invalid_client', $e->error); + $this->assertStringContainsString('client metadata document', $e->getMessage()); + $this->assertStringNotContainsString('setClientCredentials()', $e->getMessage()); + } + } + #[TestDox('a configured client id wins over a client metadata URL')] public function testConfiguredClientIdWinsOverTheClientMetadataUrl(): void {