From bf56e5b00451eed276fb41548d4b39026c2dd362 Mon Sep 17 00:00:00 2001 From: bbdu3 Date: Wed, 4 Nov 2026 16:06:14 +0800 Subject: [PATCH 01/13] fix: support completing OAuth authorization across requests Signed-off-by: unknown --- CHANGELOG.md | 1 + docs/client/authorization.md | 36 ++++ src/Client.php | 6 + .../Auth/AuthorizationHandlerInterface.php | 3 +- src/Client/Auth/FileCredentialStorage.php | 57 ++++- src/Client/Auth/InMemoryCredentialStorage.php | 31 ++- src/Client/Auth/OAuthAuthenticator.php | 125 +++++++++-- src/Client/Auth/PendingAuthorization.php | 92 +++++++++ .../PendingAuthorizationStorageInterface.php | 27 +++ .../Auth/RedirectAuthorizationHandler.php | 25 +++ .../AuthorizationPendingException.php | 23 +++ tests/Integration/ClientOAuthTest.php | 43 ++++ .../Fixture/oauth/finish-authorization.php | 24 +++ .../Client/Auth/FileCredentialStorageTest.php | 50 +++++ .../Client/Auth/OAuthAuthenticatorTest.php | 194 ++++++++++++++++++ 15 files changed, 712 insertions(+), 25 deletions(-) create mode 100644 src/Client/Auth/PendingAuthorization.php create mode 100644 src/Client/Auth/PendingAuthorizationStorageInterface.php create mode 100644 src/Client/Auth/RedirectAuthorizationHandler.php create mode 100644 src/Exception/AuthorizationPendingException.php create mode 100644 tests/Integration/Fixture/oauth/finish-authorization.php diff --git a/CHANGELOG.md b/CHANGELOG.md index aaf2c89f4..d49cb7561 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,6 +122,7 @@ All notable changes to `mcp/sdk` will be documented in this file. 0.9.0 ----- +* Add `RedirectAuthorizationHandler` and `OAuthAuthenticator::finishAuthorization()` to complete an authorization code flow in a later request. `PendingAuthorizationStorageInterface` supports expiring, single-use authorization state without changing existing credential storage implementations. ### TL;DR * Concurrent requests of one session keep their own answers, session changes and fibers diff --git a/docs/client/authorization.md b/docs/client/authorization.md index 41d3aaad5..dbed615cf 100644 --- a/docs/client/authorization.md +++ b/docs/client/authorization.md @@ -72,6 +72,42 @@ final class AcmeAuthorizationHandler implements AuthorizationHandlerInterface The default redirect URI is `http://127.0.0.1:8765/callback`. Change it with `setRedirectUri()` if the port is in use or if the authorization server expects another one. The URI must use `https` or a loopback host (e.g. `127.0.0.1`), and must not have a fragment or userinfo. Otherwise, `build()` throws an `InvalidArgumentException`. The loopback handler listens on the host and port of this URI, so it only works with `http` on a loopback host. For an `https` URI, implement your own handler. +### Web application callbacks + +For a callback that arrives in a later HTTP request, use `RedirectAuthorizationHandler` +with persistent storage scoped to the browser session that starts the flow: + +```php +use Mcp\Client\Auth\FileCredentialStorage; +use Mcp\Client\Auth\OAuth; +use Mcp\Client\Auth\RedirectAuthorizationHandler; +use Mcp\Exception\AuthorizationPendingException; + +$auth = OAuth::forApplication('My Web App') + ->setRedirectUri('https://app.example.com/oauth/callback') + ->setAuthorizationHandler(new RedirectAuthorizationHandler()) + ->setCredentialStorage(new FileCredentialStorage($sessionCredentialPath)) + ->build(); +``` + +Catch `AuthorizationPendingException` around the MCP operation and redirect the browser +to `$e->authorizationUrl`. The SDK saves the state, PKCE verifier and authorization +context before the exception reaches your application. In the callback route, rebuild +the authenticator with the same configuration and session storage and call +`$auth->finishAuthorization($callbackParameters)`, then retry the MCP operation. +The callback may also throw `AuthorizationPendingException` if the server refuses +`offline_access` and authorization needs to restart without that scope. + +Pending flows expire after ten minutes and are consumed once, before issuer/code +validation and token exchange; restart authorization if completion fails. Custom +storage must implement `PendingAuthorizationStorageInterface` with atomic consumption. +`InMemoryCredentialStorage` cannot persist state across independent PHP requests. +Resolve the storage from the current browser session, never from callback parameters, +and do not share it between users. In long-running workers, create an authenticator +per user request; tokens are also held in the authenticator's memory. + +### The redirect URI + ## Storing Credentials By default, the client keeps its tokens in memory. A command line application then sends the user to the browser on every run. Use `FileCredentialStorage` to keep them between runs: diff --git a/src/Client.php b/src/Client.php index 9b32825d4..aad83954b 100644 --- a/src/Client.php +++ b/src/Client.php @@ -17,6 +17,7 @@ use Mcp\Client\Protocol; use Mcp\Client\Transport\TransportInterface; use Mcp\Exception\AuthorizationException; +use Mcp\Exception\AuthorizationPendingException; use Mcp\Exception\ConnectionException; use Mcp\Exception\ExceptionInterface; use Mcp\Exception\InvalidArgumentException; @@ -114,6 +115,11 @@ public function connect(TransportInterface $transport): void $transport->close(); + // The application has to send the user to the authorization server first. + if ($e instanceof AuthorizationPendingException) { + throw $e; + } + // Anything else, like a malformed initialize result, is not // transient and would fail every retry the same way. if (!$e instanceof ConnectionException) { diff --git a/src/Client/Auth/AuthorizationHandlerInterface.php b/src/Client/Auth/AuthorizationHandlerInterface.php index 4abf2628d..1e8d1d67b 100644 --- a/src/Client/Auth/AuthorizationHandlerInterface.php +++ b/src/Client/Auth/AuthorizationHandlerInterface.php @@ -23,7 +23,8 @@ interface AuthorizationHandlerInterface * * @return array the callback's query parameters * - * @throws \Mcp\Exception\AuthorizationException when the user could not be sent, or did not come back + * @throws \Mcp\Exception\AuthorizationException when the user could not be sent, or did not come back + * @throws \Mcp\Exception\AuthorizationPendingException to persist the flow and redirect from a web application */ public function authorize(string $authorizationUrl, string $redirectUri): array; } diff --git a/src/Client/Auth/FileCredentialStorage.php b/src/Client/Auth/FileCredentialStorage.php index 7567f3560..c01d7f195 100644 --- a/src/Client/Auth/FileCredentialStorage.php +++ b/src/Client/Auth/FileCredentialStorage.php @@ -19,7 +19,7 @@ * * @author Christopher Hertel */ -final class FileCredentialStorage implements CredentialStorageInterface +final class FileCredentialStorage implements PendingAuthorizationStorageInterface { public function __construct(private readonly string $path) { @@ -74,15 +74,57 @@ public function saveToken(string $issuer, string $resource, AccessToken $token): }); } + public function savePendingAuthorization(PendingAuthorization $authorization): void + { + $this->update(static function (array $data) use ($authorization): array { + foreach ($data['pending'] as $state => $pending) { + if (($pending['expires_at'] ?? 0) <= time()) { + unset($data['pending'][$state]); + } + } + + $data['pending'][$authorization->state] = $authorization->jsonSerialize(); + + return $data; + }); + } + + public function consumePendingAuthorization(string $state): ?PendingAuthorization + { + $pending = null; + $this->update(static function (array $data) use ($state, &$pending): array { + $pending = $data['pending'][$state] ?? null; + unset($data['pending'][$state]); + + return $data; + }); + + if (!\is_array($pending)) { + return null; + } + + try { + return PendingAuthorization::fromArray($pending); + } catch (InvalidArgumentException) { + return null; + } + } + public function forget(?string $issuer = null): void { $this->update(static function (array $data) use ($issuer): array { if (null === $issuer) { - return ['registrations' => [], 'tokens' => []]; + return ['registrations' => [], 'tokens' => [], 'pending' => []]; } unset($data['registrations'][$issuer], $data['tokens'][$issuer]); + foreach ($data['pending'] as $state => $pending) { + if (($pending['metadata']['issuer'] ?? null) === $issuer) { + unset($data['pending'][$state]); + } + } + return $data; }); } @@ -94,12 +136,12 @@ public function synchronize(callable $operation): mixed } /** - * @return array{registrations: array>, tokens: array>>} + * @return array{registrations: array>, tokens: array>>, pending: array>} */ private function read(): array { if (!is_file($this->path)) { - return ['registrations' => [], 'tokens' => []]; + return ['registrations' => [], 'tokens' => [], 'pending' => []]; } $contents = file_get_contents($this->path); @@ -107,19 +149,20 @@ private function read(): array if (!\is_array($decoded)) { // Not worth failing a connection over: the worst case is one more authorization. - return ['registrations' => [], 'tokens' => []]; + return ['registrations' => [], 'tokens' => [], 'pending' => []]; } return [ 'registrations' => \is_array($decoded['registrations'] ?? null) ? $decoded['registrations'] : [], 'tokens' => \is_array($decoded['tokens'] ?? null) ? array_filter($decoded['tokens'], 'is_array') : [], + 'pending' => \is_array($decoded['pending'] ?? null) ? array_filter($decoded['pending'], 'is_array') : [], ]; } /** * Applies a change under an exclusive lock, so another process can't overwrite it. * - * @param callable(array{registrations: array>, tokens: array>>}): array{registrations: array>, tokens: array>>} $change + * @param callable(array{registrations: array>, tokens: array>>, pending: array>}): array{registrations: array>, tokens: array>>, pending: array>} $change */ private function update(callable $change): void { @@ -164,7 +207,7 @@ private function underLock(string $suffix, callable $operation): mixed } /** - * @param array{registrations: array>, tokens: array>>} $data + * @param array{registrations: array>, tokens: array>>, pending: array>} $data */ private function write(array $data): void { diff --git a/src/Client/Auth/InMemoryCredentialStorage.php b/src/Client/Auth/InMemoryCredentialStorage.php index d5fe7f3d4..996b54bd1 100644 --- a/src/Client/Auth/InMemoryCredentialStorage.php +++ b/src/Client/Auth/InMemoryCredentialStorage.php @@ -16,7 +16,7 @@ * * @author Christopher Hertel */ -final class InMemoryCredentialStorage implements CredentialStorageInterface +final class InMemoryCredentialStorage implements PendingAuthorizationStorageInterface { /** @var array */ private array $registrations = []; @@ -24,6 +24,28 @@ final class InMemoryCredentialStorage implements CredentialStorageInterface /** @var array> */ private array $tokens = []; + /** @var array */ + private array $pending = []; + + public function savePendingAuthorization(PendingAuthorization $authorization): void + { + foreach ($this->pending as $state => $pending) { + if ($pending->expiresAt <= time()) { + unset($this->pending[$state]); + } + } + + $this->pending[$authorization->state] = $authorization; + } + + public function consumePendingAuthorization(string $state): ?PendingAuthorization + { + $pending = $this->pending[$state] ?? null; + unset($this->pending[$state]); + + return $pending; + } + public function getClientRegistration(string $issuer): ?ClientRegistration { return $this->registrations[$issuer] ?? null; @@ -49,11 +71,18 @@ public function forget(?string $issuer = null): void if (null === $issuer) { $this->registrations = []; $this->tokens = []; + $this->pending = []; return; } unset($this->registrations[$issuer], $this->tokens[$issuer]); + + foreach ($this->pending as $state => $pending) { + if ($pending->metadata->issuer === $issuer) { + unset($this->pending[$state]); + } + } } public function synchronize(callable $operation): mixed diff --git a/src/Client/Auth/OAuthAuthenticator.php b/src/Client/Auth/OAuthAuthenticator.php index 74abf7b7d..d12f54ba6 100644 --- a/src/Client/Auth/OAuthAuthenticator.php +++ b/src/Client/Auth/OAuthAuthenticator.php @@ -12,6 +12,7 @@ namespace Mcp\Client\Auth; use Mcp\Exception\AuthorizationException; +use Mcp\Exception\AuthorizationPendingException; use Mcp\Exception\OAuthErrorException; use Mcp\Exception\TokenEndpointUnavailableException; use Mcp\Server\Transport\Http\OAuth\SecureUrl; @@ -167,6 +168,57 @@ public function handleChallenge(RequestInterface $request, ResponseInterface $re return true; } + /** + * @param array $callback + */ + public function finishAuthorization(array $callback): void + { + foreach (['state', 'iss', 'code', 'error', 'error_description'] as $key) { + if (\array_key_exists($key, $callback) && !\is_string($callback[$key])) { + throw new AuthorizationException(\sprintf('The authorization response parameter "%s" must be a string.', $key)); + } + } + + $state = $callback['state'] ?? ''; + + if ('' === $state || !($this->storage instanceof PendingAuthorizationStorageInterface)) { + throw new AuthorizationException('No pending authorization is available for this callback.'); + } + + $pending = $this->storage->consumePendingAuthorization($state); + + if (null === $pending || $pending->expiresAt <= time()) { + throw new AuthorizationException('The authorization response does not match a pending authorization.'); + } + + $this->verifyCallbackOrigin($callback, $pending->state, $pending->metadata); + $this->token = null; + $this->requestedScopes = []; + $this->issuer = $pending->metadata->issuer; + $this->resource = $pending->resource; + $this->audience = $pending->audience; + $this->offlineAccessRefused = $pending->offlineAccessRefused; + $scopes = $pending->scopes; + + try { + $this->verifyCallback($callback, $pending->state, $pending->metadata); + $token = $this->tokenEndpoint->request($pending->metadata, $pending->client, self::withoutEmptyValues([ + 'grant_type' => Grant::AuthorizationCode->value, + 'code' => $callback['code'], + 'redirect_uri' => $pending->redirectUri, + 'code_verifier' => $pending->codeVerifier, + 'resource' => $pending->resource, + ]), $this->configuration->privateKeyPem, $this->configuration->signingAlgorithm); + } catch (AuthorizationException $e) { + $scopes = $this->reduceScopes($scopes, $e); + $token = $this->grant($pending->metadata, $pending->client, $scopes, $pending->resource); + } + + $this->requestedScopes = $scopes; + $this->token = $token->withGrantedScopes($scopes); + $this->storage->saveToken($pending->metadata->issuer, $pending->audience, $this->token); + } + /** * Find the resource and the server, on every challenge as a resource may move. * @@ -452,17 +504,29 @@ private function grantWithFallback(AuthorizationServerMetadata $metadata, Client try { return [$this->grant($metadata, $client, $scopes, $resource), $scopes]; } catch (AuthorizationException $e) { - $reduced = array_values(array_diff($scopes, ['offline_access'])); + $reduced = $this->reduceScopes($scopes, $e); - if ($reduced === $scopes || !$e instanceof OAuthErrorException || !\in_array($e->error, ['invalid_scope', 'not_allowed'], true)) { - throw $e; - } + return [$this->grant($metadata, $client, $reduced, $resource), $reduced]; + } + } - $this->logger->info('The authorization server refused the offline_access scope; retrying without it, at the cost of a refresh token.', ['exception' => $e]); - $this->offlineAccessRefused = true; + /** + * @param string[] $scopes + * + * @return string[] + */ + private function reduceScopes(array $scopes, AuthorizationException $e): array + { + $reduced = array_values(array_diff($scopes, ['offline_access'])); - return [$this->grant($metadata, $client, $reduced, $resource), $reduced]; + if ($reduced === $scopes || !$e instanceof OAuthErrorException || !\in_array($e->error, ['invalid_scope', 'not_allowed'], true)) { + throw $e; } + + $this->logger->info('The authorization server refused the offline_access scope; retrying without it, at the cost of a refresh token.', ['exception' => $e]); + $this->offlineAccessRefused = true; + + return $reduced; } /** @@ -514,7 +578,28 @@ private function grant(AuthorizationServerMetadata $metadata, ClientRegistration $this->logger->info('Starting the authorization code flow', ['authorization_endpoint' => $metadata->authorizationEndpoint, 'scope' => $parameters['scope'] ?? null]); - $callback = $this->authorizationHandler->authorize($url, $this->configuration->redirectUri); + try { + $callback = $this->authorizationHandler->authorize($url, $this->configuration->redirectUri); + } catch (AuthorizationPendingException $e) { + if (!$this->storage instanceof PendingAuthorizationStorageInterface) { + throw new AuthorizationException('A pending authorization requires PendingAuthorizationStorageInterface.'); + } + + $this->storage->savePendingAuthorization(new PendingAuthorization( + $state, + $pkce->verifier, + $this->configuration->redirectUri, + $resource, + $resource, + $scopes, + $metadata, + $client, + time() + 600, + $this->offlineAccessRefused, + )); + + throw $e; + } $this->verifyCallback($callback, $state, $metadata); @@ -578,6 +663,22 @@ private function requestAuthorizationGrant(AuthorizationServerMetadata $metadata * @phpstan-assert non-empty-array{code: string} $callback */ private function verifyCallback(array $callback, string $state, AuthorizationServerMetadata $metadata): void + { + $this->verifyCallbackOrigin($callback, $state, $metadata); + + if (isset($callback['error'])) { + throw new OAuthErrorException(\sprintf('The authorization server denied the request: %s%s', $callback['error'], isset($callback['error_description']) ? ' ('.$callback['error_description'].')' : ''), $callback['error']); + } + + if (!isset($callback['code']) || '' === $callback['code']) { + throw new AuthorizationException('The authorization response carried no authorization code.'); + } + } + + /** + * @param array $callback + */ + private function verifyCallbackOrigin(array $callback, string $state, AuthorizationServerMetadata $metadata): void { // RFC 6749 section 10.12: required, as any local process can reach the loopback listener. if (!isset($callback['state']) || !hash_equals($state, $callback['state'])) { @@ -592,14 +693,6 @@ private function verifyCallback(array $callback, string $state, AuthorizationSer } elseif ($metadata->issuerParameterSupported) { throw new AuthorizationException(\sprintf('The authorization server "%s" advertises the iss parameter, but its response did not carry one.', $metadata->issuer)); } - - if (isset($callback['error'])) { - throw new OAuthErrorException(\sprintf('The authorization server denied the request: %s%s', $callback['error'], isset($callback['error_description']) ? ' ('.$callback['error_description'].')' : ''), $callback['error']); - } - - if (!isset($callback['code']) || '' === $callback['code']) { - throw new AuthorizationException('The authorization response carried no authorization code.'); - } } /** diff --git a/src/Client/Auth/PendingAuthorization.php b/src/Client/Auth/PendingAuthorization.php new file mode 100644 index 000000000..09b860d53 --- /dev/null +++ b/src/Client/Auth/PendingAuthorization.php @@ -0,0 +1,92 @@ + + */ + public function jsonSerialize(): array + { + return [ + 'state' => $this->state, + 'code_verifier' => $this->codeVerifier, + 'redirect_uri' => $this->redirectUri, + 'resource' => $this->resource, + 'audience' => $this->audience, + 'scopes' => $this->scopes, + 'metadata' => [ + 'issuer' => $this->metadata->issuer, + 'authorization_endpoint' => $this->metadata->authorizationEndpoint, + 'token_endpoint' => $this->metadata->tokenEndpoint, + 'code_challenge_methods_supported' => $this->metadata->codeChallengeMethodsSupported, + 'authorization_response_iss_parameter_supported' => $this->metadata->issuerParameterSupported, + ], + 'client' => $this->client->jsonSerialize(), + 'expires_at' => $this->expiresAt, + 'offline_access_refused' => $this->offlineAccessRefused, + ]; + } + + /** + * @param array $data + */ + public static function fromArray(array $data): self + { + foreach (['state', 'code_verifier', 'redirect_uri', 'resource', 'audience'] as $key) { + if (!\is_string($data[$key] ?? null) || '' === $data[$key]) { + throw new InvalidArgumentException(\sprintf('Pending authorization is missing "%s".', $key)); + } + } + + if (!\is_int($data['expires_at'] ?? null) || !\is_array($data['scopes'] ?? null) || !\is_array($data['metadata'] ?? null) || !\is_array($data['client'] ?? null)) { + throw new InvalidArgumentException('Invalid pending authorization.'); + } + + foreach ($data['scopes'] as $scope) { + if (!\is_string($scope)) { + throw new InvalidArgumentException('Invalid pending authorization scope.'); + } + } + + return new self( + $data['state'], $data['code_verifier'], $data['redirect_uri'], + $data['resource'], $data['audience'], array_values($data['scopes']), + AuthorizationServerMetadata::fromArray($data['metadata']), + ClientRegistration::fromArray($data['client']), $data['expires_at'], + true === ($data['offline_access_refused'] ?? false), + ); + } +} diff --git a/src/Client/Auth/PendingAuthorizationStorageInterface.php b/src/Client/Auth/PendingAuthorizationStorageInterface.php new file mode 100644 index 000000000..6e110c1bf --- /dev/null +++ b/src/Client/Auth/PendingAuthorizationStorageInterface.php @@ -0,0 +1,27 @@ +authorize($this->authorizationServerUrl.'/nothing-here', 'http://127.0.0.1:8765/callback'); } + public function testCompletesRedirectAuthorizationInAnotherProcess(): void + { + $path = $this->workingDirectory.'/credentials.json'; + $auth = OAuth::forApplication('Integration Test Client') + ->setAuthorizationHandler(new RedirectAuthorizationHandler()) + ->setCredentialStorage(new FileCredentialStorage($path)) + ->build(); + $client = Client::builder()->setClientInfo('integration-client', '1.0.0')->setInitTimeout(5)->build(); + + try { + $client->connect(new HttpTransport($this->resourceServerUrl.'/mcp', auth: $auth)); + $this->fail('Expected authorization to pause for a redirect.'); + } catch (AuthorizationPendingException $e) { + $callback = (new HeadlessAuthorizationHandler())->authorize($e->authorizationUrl, 'http://127.0.0.1:8765/callback'); + } + + unset($client, $auth); + $command = [\PHP_BINARY, __DIR__.'/Fixture/oauth/finish-authorization.php', $path, json_encode($callback)]; + $process = new Process($command); + $competitor = new Process($command); + $process->start(); + $competitor->start(); + $process->wait(); + $competitor->wait(); + $this->assertNotSame($process->isSuccessful(), $competitor->isSuccessful()); + $winner = $process->isSuccessful() ? $process : $competitor; + $loser = $process->isSuccessful() ? $competitor : $process; + $this->assertSame('authorized', $winner->getOutput()); + $this->assertStringContainsString('does not match a pending authorization', $loser->getErrorOutput().$loser->getOutput()); + + $client = $this->connect(new FileCredentialStorage($path)); + $this->assertStringContainsString('the token was accepted', json_encode($client->callTool('whoami', [])->content)); + $client->disconnect(); + $this->assertSame([], json_decode(file_get_contents($path), true)['pending']); + + $replay = new Process($command); + $replay->run(); + $this->assertFalse($replay->isSuccessful()); + $this->assertStringContainsString('does not match a pending authorization', $replay->getErrorOutput().$replay->getOutput()); + } + private function connect(?FileCredentialStorage $storage = null): Client { $client = Client::builder() diff --git a/tests/Integration/Fixture/oauth/finish-authorization.php b/tests/Integration/Fixture/oauth/finish-authorization.php new file mode 100644 index 000000000..b419cce78 --- /dev/null +++ b/tests/Integration/Fixture/oauth/finish-authorization.php @@ -0,0 +1,24 @@ +setAuthorizationHandler(new RedirectAuthorizationHandler()) + ->setCredentialStorage(new FileCredentialStorage($argv[1])) + ->build(); +$auth->finishAuthorization(json_decode($argv[2], true, flags: \JSON_THROW_ON_ERROR)); + +echo 'authorized'; diff --git a/tests/Unit/Client/Auth/FileCredentialStorageTest.php b/tests/Unit/Client/Auth/FileCredentialStorageTest.php index b5df96942..d27adc249 100644 --- a/tests/Unit/Client/Auth/FileCredentialStorageTest.php +++ b/tests/Unit/Client/Auth/FileCredentialStorageTest.php @@ -12,8 +12,10 @@ namespace Mcp\Tests\Unit\Client\Auth; use Mcp\Client\Auth\AccessToken; +use Mcp\Client\Auth\AuthorizationServerMetadata; use Mcp\Client\Auth\ClientRegistration; use Mcp\Client\Auth\FileCredentialStorage; +use Mcp\Client\Auth\PendingAuthorization; use Mcp\Client\Auth\TokenEndpointAuthMethod; use PHPUnit\Framework\Attributes\TestDox; use PHPUnit\Framework\TestCase; @@ -125,6 +127,54 @@ public function testFileIsNeverExposedWhileBeingWritten(): void $this->assertCount(1, glob(\dirname($this->path).'/*') ?: [], 'The temporary file should not survive the write.'); } + public function testPendingAuthorizationsAreConsumedAcrossInstances(): void + { + $first = new FileCredentialStorage($this->path); + $second = new FileCredentialStorage($this->path); + $first->savePendingAuthorization($this->pending('one')); + $second->savePendingAuthorization($this->pending('two')); + $first->saveToken('https://auth.example.com', 'https://mcp.example.com/mcp', new AccessToken('token')); + + $this->assertEquals($this->pending('one'), $second->consumePendingAuthorization('one')); + $this->assertNull($first->consumePendingAuthorization('one')); + $this->assertSame('two', $first->consumePendingAuthorization('two')?->state); + $this->assertSame('token', $second->getToken('https://auth.example.com', 'https://mcp.example.com/mcp')?->accessToken); + } + + public function testForgetRemovesOnlyTheIssuersPendingAuthorizations(): void + { + $storage = new FileCredentialStorage($this->path); + $storage->savePendingAuthorization($this->pending('one')); + $storage->savePendingAuthorization($this->pending('two', 'https://other.example.com')); + $storage->forget('https://auth.example.com'); + + $this->assertNull($storage->consumePendingAuthorization('one')); + $this->assertSame('two', $storage->consumePendingAuthorization('two')?->state); + $storage->savePendingAuthorization($this->pending('three')); + $storage->forget(); + $this->assertNull($storage->consumePendingAuthorization('three')); + } + + public function testMalformedPendingAuthorizationIsDiscarded(): void + { + $storage = new FileCredentialStorage($this->path); + $storage->savePendingAuthorization($this->pending('one')); + $data = json_decode(file_get_contents($this->path), true); + unset($data['pending']['one']['code_verifier']); + + foreach ([$data['pending']['one'], 'corrupt record'] as $record) { + $data['pending']['one'] = $record; + file_put_contents($this->path, json_encode($data)); + + $this->assertNull($storage->consumePendingAuthorization('one')); + } + } + + private function pending(string $state, string $issuer = 'https://auth.example.com'): PendingAuthorization + { + return new PendingAuthorization($state, 'verifier', 'https://app.example.com/callback', 'https://mcp.example.com/mcp', 'https://mcp.example.com/mcp', ['mcp:read'], new AuthorizationServerMetadata($issuer, $issuer.'/authorize', $issuer.'/token', issuerParameterSupported: true), new ClientRegistration('client', 'secret'), 4102444800); + } + #[TestDox('a corrupted file costs one authorization, not a crash')] public function testToleratesACorruptedFile(): void { diff --git a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php index 8935e2366..3e9b490fd 100644 --- a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php +++ b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php @@ -17,14 +17,18 @@ use Mcp\Client\Auth\ClientRegistration; use Mcp\Client\Auth\CredentialStorageInterface; use Mcp\Client\Auth\CrossAppAccess; +use Mcp\Client\Auth\FileCredentialStorage; use Mcp\Client\Auth\InMemoryCredentialStorage; use Mcp\Client\Auth\MetadataDiscovery; use Mcp\Client\Auth\OAuth; use Mcp\Client\Auth\OAuthAuthenticator; use Mcp\Client\Auth\OAuthConfiguration; +use Mcp\Client\Auth\PendingAuthorization; +use Mcp\Client\Auth\RedirectAuthorizationHandler; use Mcp\Client\Auth\TokenEndpoint; use Mcp\Client\Auth\TokenEndpointAuthMethod; use Mcp\Exception\AuthorizationException; +use Mcp\Exception\AuthorizationPendingException; use Mcp\Exception\OAuthErrorException; use Nyholm\Psr7\Factory\Psr17Factory; use Nyholm\Psr7\Response; @@ -548,6 +552,196 @@ public function testSkipsRegistrationForAKnownClient(): void $this->assertSame('known-client', $server->authorization['client_id'] ?? null); } + public function testFinishesAuthorizationInAnotherRequest(): void + { + $path = tempnam(sys_get_temp_dir(), 'mcp-pending-'); + $server = new FakeOAuthServer(resourceScopes: ['mcp:read']); + $authenticator = $this->authenticator($server, new RedirectAuthorizationHandler(), new FileCredentialStorage($path)); + + try { + try { + $authenticator->handleChallenge($this->request(), $server->challenge()); + $this->fail('Expected a redirect.'); + } catch (AuthorizationPendingException $e) { + $server->recordAuthorization($e->authorizationUrl); + } + + unset($authenticator); + $saved = json_decode(file_get_contents($path), true); + $this->assertCount(1, $saved['pending']); + $resumed = $this->authenticator($server, storage: new FileCredentialStorage($path)); + $resumed->finishAuthorization([ + 'state' => $server->authorization['state'], + 'iss' => 'https://auth.example.com', + 'code' => 'test-authorization-code', + ]); + + $this->assertSame('Bearer access-token-1', $resumed->authenticate($this->request())->getHeaderLine('Authorization')); + $this->assertSame($server->authorization['code_challenge'], rtrim(strtr(base64_encode(hash('sha256', $server->token['code_verifier'], true)), '+/', '-_'), '=')); + $this->assertSame($server->authorization['redirect_uri'], $server->token['redirect_uri']); + $this->assertSame($server->authorization['resource'], $server->token['resource']); + $stored = (new FileCredentialStorage($path))->getToken('https://auth.example.com', self::ENDPOINT); + $this->assertSame(['mcp:read'], $stored?->scopes); + $this->assertSame([], json_decode(file_get_contents($path), true)['pending']); + } finally { + unlink($path); + @unlink($path.'.lock'); + } + } + + #[DataProvider('invalidDeferredCallbacks')] + public function testRejectsInvalidDeferredCallbacks(array $replacement): void + { + $server = new FakeOAuthServer(serverScopes: ['mcp:read', 'offline_access']); + $storage = new InMemoryCredentialStorage(); + $callback = $this->startPending($server, $storage); + $resumed = $this->authenticator($server, storage: $storage); + + try { + $resumed->finishAuthorization(array_filter(array_replace($callback, $replacement), static fn ($value) => null !== $value)); + $this->fail('Expected an invalid callback to be rejected.'); + } catch (AuthorizationException $e) { + $this->assertNotInstanceOf(AuthorizationPendingException::class, $e); + $this->assertNotContains('https://auth.example.com/token', $server->requested); + $this->assertNull($storage->getToken('https://auth.example.com', self::ENDPOINT)); + } + } + + public static function invalidDeferredCallbacks(): iterable + { + yield 'unknown state' => [['state' => 'unknown']]; + yield 'empty state' => [['state' => '']]; + yield 'missing state' => [['state' => null]]; + yield 'array state' => [['state' => ['attacker']]]; + yield 'wrong issuer' => [['iss' => 'https://attacker.example.com']]; + yield 'missing issuer' => [['iss' => null]]; + yield 'array issuer' => [['iss' => ['attacker']]]; + yield 'empty code' => [['code' => '']]; + yield 'missing code' => [['code' => null]]; + yield 'array code' => [['code' => ['attacker']]]; + yield 'denied' => [['error' => 'access_denied']]; + yield 'untrusted scope error' => [['iss' => 'https://attacker.example.com', 'error' => 'invalid_scope']]; + yield 'scope error in issuer' => [['iss' => 'https://invalid_scope.example.com']]; + } + + public function testDeferredCallbackCanOnlyBeConsumedOnce(): void + { + $server = new FakeOAuthServer(); + $storage = new InMemoryCredentialStorage(); + $callback = $this->startPending($server, $storage); + $this->authenticator($server, storage: $storage)->finishAuthorization($callback); + + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('does not match a pending authorization'); + $this->authenticator($server, storage: $storage)->finishAuthorization($callback); + } + + public function testDeferredAuthorizationExpires(): void + { + $server = new FakeOAuthServer(); + $storage = new InMemoryCredentialStorage(); + $callback = $this->startPending($server, $storage); + $pending = $storage->consumePendingAuthorization($callback['state']); + $data = $pending->jsonSerialize(); + $data['expires_at'] = time() - 1; + $storage->savePendingAuthorization(PendingAuthorization::fromArray($data)); + + $this->expectException(AuthorizationException::class); + $this->authenticator($server, storage: $storage)->finishAuthorization($callback); + } + + public function testCallbackCannotBeCompletedWithAnotherUsersStorage(): void + { + $server = new FakeOAuthServer(); + $callback = $this->startPending($server, new InMemoryCredentialStorage()); + + $this->expectException(AuthorizationException::class); + $this->authenticator($server, storage: new InMemoryCredentialStorage())->finishAuthorization($callback); + } + + #[DataProvider('offlineAccessRefusals')] + public function testDeferredOfflineAccessFallback(bool $atCallback): void + { + $server = new FakeOAuthServer(resourceScopes: ['mcp:read'], serverScopes: ['mcp:read', 'offline_access'], refuseOfflineAccess: !$atCallback); + $storage = new InMemoryCredentialStorage(); + $callback = $this->startPending($server, $storage); + + if ($atCallback) { + $callback['error'] = 'invalid_scope'; + unset($callback['code']); + } + + try { + $this->authenticator($server, new RedirectAuthorizationHandler(), $storage)->finishAuthorization($callback); + $this->fail('Expected a new redirect without offline_access.'); + } catch (AuthorizationPendingException $e) { + $server->recordAuthorization($e->authorizationUrl); + } + + $this->assertSame('mcp:read', $server->authorization['scope']); + $this->assertNotSame($callback['state'], $server->authorization['state']); + $resumed = $this->authenticator($server, storage: $storage); + $resumed->finishAuthorization(['code' => 'new-code', 'state' => $server->authorization['state'], 'iss' => 'https://auth.example.com']); + $this->assertSame('Bearer access-token-1', $resumed->authenticate($this->request())->getHeaderLine('Authorization')); + } + + public static function offlineAccessRefusals(): iterable + { + yield 'authorization endpoint' => [true]; + yield 'token endpoint' => [false]; + } + + public function testSynchronousAuthorizationPreservesErrorResponses(): void + { + $server = new FakeOAuthServer(); + $handler = $this->createStub(AuthorizationHandlerInterface::class); + $handler->method('authorize')->willReturnCallback(static function (string $url): array { + parse_str((string) parse_url($url, \PHP_URL_QUERY), $query); + + return ['error' => 'access_denied', 'state' => $query['state'], 'iss' => 'https://auth.example.com']; + }); + + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('The authorization server denied the request: access_denied'); + $this->authenticator($server, $handler)->handleChallenge($this->request(), $server->challenge()); + } + + public function testSynchronousAuthorizationSupportsExistingCredentialStorage(): void + { + $server = new FakeOAuthServer(); + $storage = $this->createMock(CredentialStorageInterface::class); + $storage->expects($this->once())->method('saveToken')->with('https://auth.example.com', self::ENDPOINT, $this->isInstanceOf(AccessToken::class)); + + $authenticator = $this->authenticator($server, storage: $storage); + $this->assertTrue($authenticator->handleChallenge($this->request(), $server->challenge())); + $this->assertSame('Bearer access-token-1', $authenticator->authenticate($this->request())->getHeaderLine('Authorization')); + } + + public function testDeferredAuthorizationRequiresPendingStorage(): void + { + $server = new FakeOAuthServer(); + $authenticator = $this->authenticator($server, new RedirectAuthorizationHandler(), $this->createStub(CredentialStorageInterface::class)); + + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('requires PendingAuthorizationStorageInterface'); + $authenticator->handleChallenge($this->request(), $server->challenge()); + } + + /** + * @return array + */ + private function startPending(FakeOAuthServer $server, CredentialStorageInterface $storage, ?OAuth $configuration = null): array + { + try { + $this->authenticator($server, new RedirectAuthorizationHandler(), $storage, $configuration)->handleChallenge($this->request(), $server->challenge()); + $this->fail('Expected an authorization redirect.'); + } catch (AuthorizationPendingException $e) { + $server->recordAuthorization($e->authorizationUrl); + } + + return ['code' => 'test-code', 'state' => $server->authorization['state'], 'iss' => 'https://auth.example.com']; + } + /** * @return iterable */ From 62f572995e59ab2ee39d998ab94aaf7ce667edbc Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 10 Nov 2026 02:06:09 +0100 Subject: [PATCH 02/13] [Client] Pass synchronize() through in the deferred authorization test doubles --- tests/Unit/Client/Auth/OAuthAuthenticatorTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php index 3e9b490fd..91b6f3f7b 100644 --- a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php +++ b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php @@ -710,6 +710,7 @@ public function testSynchronousAuthorizationSupportsExistingCredentialStorage(): { $server = new FakeOAuthServer(); $storage = $this->createMock(CredentialStorageInterface::class); + $storage->method('synchronize')->willReturnCallback(static fn (callable $operation): mixed => $operation()); $storage->expects($this->once())->method('saveToken')->with('https://auth.example.com', self::ENDPOINT, $this->isInstanceOf(AccessToken::class)); $authenticator = $this->authenticator($server, storage: $storage); @@ -720,7 +721,9 @@ public function testSynchronousAuthorizationSupportsExistingCredentialStorage(): public function testDeferredAuthorizationRequiresPendingStorage(): void { $server = new FakeOAuthServer(); - $authenticator = $this->authenticator($server, new RedirectAuthorizationHandler(), $this->createStub(CredentialStorageInterface::class)); + $storage = $this->createStub(CredentialStorageInterface::class); + $storage->method('synchronize')->willReturnCallback(static fn (callable $operation): mixed => $operation()); + $authenticator = $this->authenticator($server, new RedirectAuthorizationHandler(), $storage); $this->expectException(AuthorizationException::class); $this->expectExceptionMessage('requires PendingAuthorizationStorageInterface'); From 5312a6613b4affc91af032512031147cc859af33 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 10 Nov 2026 02:06:50 +0100 Subject: [PATCH 03/13] [Client] Keep the full authorization server metadata in a pending authorization --- .../Auth/AuthorizationServerMetadata.php | 21 ++++++++++++++++++- src/Client/Auth/PendingAuthorization.php | 8 +------ .../Auth/AuthorizationServerMetadataTest.php | 20 ++++++++++++++++++ 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/Client/Auth/AuthorizationServerMetadata.php b/src/Client/Auth/AuthorizationServerMetadata.php index 68bc79b63..29561afd6 100644 --- a/src/Client/Auth/AuthorizationServerMetadata.php +++ b/src/Client/Auth/AuthorizationServerMetadata.php @@ -21,7 +21,7 @@ * * @author Christopher Hertel */ -final class AuthorizationServerMetadata +final class AuthorizationServerMetadata implements \JsonSerializable { /** * @param string $issuer the issuer identifier, which must match how the document was found @@ -111,6 +111,25 @@ public static function fromArray(array $data): self ); } + /** + * @return array + */ + public function jsonSerialize(): array + { + return array_filter([ + 'issuer' => $this->issuer, + 'authorization_endpoint' => $this->authorizationEndpoint, + 'token_endpoint' => $this->tokenEndpoint, + 'registration_endpoint' => $this->registrationEndpoint, + 'scopes_supported' => $this->scopesSupported, + 'grant_types_supported' => $this->grantTypesSupported, + 'token_endpoint_auth_methods_supported' => $this->tokenEndpointAuthMethodsSupported, + 'code_challenge_methods_supported' => $this->codeChallengeMethodsSupported, + 'client_id_metadata_document_supported' => $this->clientIdMetadataDocumentSupported, + 'authorization_response_iss_parameter_supported' => $this->issuerParameterSupported, + ], static fn (mixed $value): bool => null !== $value); + } + /** * The fixed endpoints a 2025-03-26 era server without metadata is assumed to expose. * diff --git a/src/Client/Auth/PendingAuthorization.php b/src/Client/Auth/PendingAuthorization.php index 09b860d53..353608e47 100644 --- a/src/Client/Auth/PendingAuthorization.php +++ b/src/Client/Auth/PendingAuthorization.php @@ -47,13 +47,7 @@ public function jsonSerialize(): array 'resource' => $this->resource, 'audience' => $this->audience, 'scopes' => $this->scopes, - 'metadata' => [ - 'issuer' => $this->metadata->issuer, - 'authorization_endpoint' => $this->metadata->authorizationEndpoint, - 'token_endpoint' => $this->metadata->tokenEndpoint, - 'code_challenge_methods_supported' => $this->metadata->codeChallengeMethodsSupported, - 'authorization_response_iss_parameter_supported' => $this->metadata->issuerParameterSupported, - ], + 'metadata' => $this->metadata->jsonSerialize(), 'client' => $this->client->jsonSerialize(), 'expires_at' => $this->expiresAt, 'offline_access_refused' => $this->offlineAccessRefused, diff --git a/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php b/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php index da816a7b9..a8575f311 100644 --- a/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php +++ b/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php @@ -42,6 +42,26 @@ public function testFromArray(): void $this->assertFalse($metadata->supportsScope('mcp:write')); } + #[TestDox('the metadata survives being stored and read back')] + public function testRoundTrip(): void + { + $document = [ + 'issuer' => 'https://auth.example.com', + 'authorization_endpoint' => 'https://login.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' => ['private_key_jwt'], + 'code_challenge_methods_supported' => ['S256'], + 'client_id_metadata_document_supported' => true, + 'authorization_response_iss_parameter_supported' => true, + ]; + + $this->assertSame($document, AuthorizationServerMetadata::fromArray($document)->jsonSerialize()); + $this->assertEquals(AuthorizationServerMetadata::forLegacyServer('https://auth.example.com'), AuthorizationServerMetadata::fromArray(AuthorizationServerMetadata::forLegacyServer('https://auth.example.com')->jsonSerialize())); + } + #[TestDox('a document missing an endpoint the client needs is rejected')] public function testRequiresEndpoints(): void { From a5e7787145b2b948197ac90e10268a0608960c69 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 10 Nov 2026 02:09:06 +0100 Subject: [PATCH 04/13] [Client] Finish a deferred authorization with the checks of the normal flow Resolve the client again instead of storing it, so a configured secret stays out of the storage. Check the expected issuer, forget a registration the server refused, and drop the audience field, which always equals the resource. --- src/Client/Auth/OAuthAuthenticator.php | 90 +++++++++++-------- src/Client/Auth/PendingAuthorization.php | 30 ++++--- .../Client/Auth/FileCredentialStorageTest.php | 11 ++- .../Client/Auth/OAuthAuthenticatorTest.php | 71 +++++++++++++++ 4 files changed, 153 insertions(+), 49 deletions(-) diff --git a/src/Client/Auth/OAuthAuthenticator.php b/src/Client/Auth/OAuthAuthenticator.php index d12f54ba6..db82bb23f 100644 --- a/src/Client/Auth/OAuthAuthenticator.php +++ b/src/Client/Auth/OAuthAuthenticator.php @@ -169,7 +169,12 @@ public function handleChallenge(RequestInterface $request, ResponseInterface $re } /** - * @param array $callback + * Completes an authorization that RedirectAuthorizationHandler sent to the browser. + * + * @param array $callback the query parameters of the request to the redirect URI + * + * @throws AuthorizationPendingException when the authorization has to start again without offline_access + * @throws AuthorizationException when the callback does not complete a pending authorization of this storage */ public function finishAuthorization(array $callback): void { @@ -181,42 +186,62 @@ public function finishAuthorization(array $callback): void $state = $callback['state'] ?? ''; - if ('' === $state || !($this->storage instanceof PendingAuthorizationStorageInterface)) { + if ('' === $state || !$this->storage instanceof PendingAuthorizationStorageInterface) { throw new AuthorizationException('No pending authorization is available for this callback.'); } + // Consumed before anything is checked, so a callback can't be tried twice. $pending = $this->storage->consumePendingAuthorization($state); if (null === $pending || $pending->expiresAt <= time()) { throw new AuthorizationException('The authorization response does not match a pending authorization.'); } - $this->verifyCallbackOrigin($callback, $pending->state, $pending->metadata); + $metadata = $pending->metadata; + + if (null !== $this->configuration->expectedIssuer && $metadata->issuer !== $this->configuration->expectedIssuer) { + throw new AuthorizationException(\sprintf('The pending authorization belongs to the authorization server "%s", but only "%s" is expected.', $metadata->issuer, $this->configuration->expectedIssuer)); + } + + // The secret of a configured client stays out of the storage, so the client is resolved again. + $client = $this->resolveClient($metadata, $pending->scopes, null); + + if ($client->clientId !== $pending->clientId) { + throw new AuthorizationException(\sprintf('The client "%s" of the pending authorization is no longer the client for "%s"; start the authorization again.', $pending->clientId, $metadata->issuer)); + } + $this->token = null; + $this->audience = null; $this->requestedScopes = []; - $this->issuer = $pending->metadata->issuer; + $this->issuer = $metadata->issuer; $this->resource = $pending->resource; - $this->audience = $pending->audience; $this->offlineAccessRefused = $pending->offlineAccessRefused; $scopes = $pending->scopes; try { - $this->verifyCallback($callback, $pending->state, $pending->metadata); - $token = $this->tokenEndpoint->request($pending->metadata, $pending->client, self::withoutEmptyValues([ + $this->verifyCallback($callback, $pending->state, $metadata); + $token = $this->requestToken($metadata, $client, self::withoutEmptyValues([ 'grant_type' => Grant::AuthorizationCode->value, 'code' => $callback['code'], 'redirect_uri' => $pending->redirectUri, 'code_verifier' => $pending->codeVerifier, 'resource' => $pending->resource, - ]), $this->configuration->privateKeyPem, $this->configuration->signingAlgorithm); + ])); } catch (AuthorizationException $e) { + if (self::isForgottenClient($e, $client)) { + $this->storage->forget($metadata->issuer); + + throw $e; + } + $scopes = $this->reduceScopes($scopes, $e); - $token = $this->grant($pending->metadata, $pending->client, $scopes, $pending->resource); + $token = $this->grant($metadata, $client, $scopes, $pending->resource); } - $this->requestedScopes = $scopes; $this->token = $token->withGrantedScopes($scopes); - $this->storage->saveToken($pending->metadata->issuer, $pending->audience, $this->token); + $this->requestedScopes = $scopes; + $this->audience = $pending->resource; + $this->storage->saveToken($metadata->issuer, $pending->resource, $this->token); } /** @@ -586,16 +611,15 @@ private function grant(AuthorizationServerMetadata $metadata, ClientRegistration } $this->storage->savePendingAuthorization(new PendingAuthorization( - $state, - $pkce->verifier, - $this->configuration->redirectUri, - $resource, - $resource, - $scopes, - $metadata, - $client, - time() + 600, - $this->offlineAccessRefused, + state: $state, + codeVerifier: $pkce->verifier, + redirectUri: $this->configuration->redirectUri, + resource: $resource, + scopes: $scopes, + metadata: $metadata, + clientId: $client->clientId, + expiresAt: time() + 600, + offlineAccessRefused: $this->offlineAccessRefused, )); throw $e; @@ -663,22 +687,6 @@ private function requestAuthorizationGrant(AuthorizationServerMetadata $metadata * @phpstan-assert non-empty-array{code: string} $callback */ private function verifyCallback(array $callback, string $state, AuthorizationServerMetadata $metadata): void - { - $this->verifyCallbackOrigin($callback, $state, $metadata); - - if (isset($callback['error'])) { - throw new OAuthErrorException(\sprintf('The authorization server denied the request: %s%s', $callback['error'], isset($callback['error_description']) ? ' ('.$callback['error_description'].')' : ''), $callback['error']); - } - - if (!isset($callback['code']) || '' === $callback['code']) { - throw new AuthorizationException('The authorization response carried no authorization code.'); - } - } - - /** - * @param array $callback - */ - private function verifyCallbackOrigin(array $callback, string $state, AuthorizationServerMetadata $metadata): void { // RFC 6749 section 10.12: required, as any local process can reach the loopback listener. if (!isset($callback['state']) || !hash_equals($state, $callback['state'])) { @@ -693,6 +701,14 @@ private function verifyCallbackOrigin(array $callback, string $state, Authorizat } elseif ($metadata->issuerParameterSupported) { throw new AuthorizationException(\sprintf('The authorization server "%s" advertises the iss parameter, but its response did not carry one.', $metadata->issuer)); } + + if (isset($callback['error'])) { + throw new OAuthErrorException(\sprintf('The authorization server denied the request: %s%s', $callback['error'], isset($callback['error_description']) ? ' ('.$callback['error_description'].')' : ''), $callback['error']); + } + + if (!isset($callback['code']) || '' === $callback['code']) { + throw new AuthorizationException('The authorization response carried no authorization code.'); + } } /** diff --git a/src/Client/Auth/PendingAuthorization.php b/src/Client/Auth/PendingAuthorization.php index 353608e47..a1b2254c0 100644 --- a/src/Client/Auth/PendingAuthorization.php +++ b/src/Client/Auth/PendingAuthorization.php @@ -26,10 +26,9 @@ public function __construct( public readonly string $codeVerifier, public readonly string $redirectUri, public readonly string $resource, - public readonly string $audience, public readonly array $scopes, public readonly AuthorizationServerMetadata $metadata, - public readonly ClientRegistration $client, + public readonly string $clientId, public readonly int $expiresAt, public readonly bool $offlineAccessRefused = false, ) { @@ -45,10 +44,9 @@ public function jsonSerialize(): array 'code_verifier' => $this->codeVerifier, 'redirect_uri' => $this->redirectUri, 'resource' => $this->resource, - 'audience' => $this->audience, 'scopes' => $this->scopes, 'metadata' => $this->metadata->jsonSerialize(), - 'client' => $this->client->jsonSerialize(), + 'client_id' => $this->clientId, 'expires_at' => $this->expiresAt, 'offline_access_refused' => $this->offlineAccessRefused, ]; @@ -56,31 +54,41 @@ public function jsonSerialize(): array /** * @param array $data + * + * @throws InvalidArgumentException when the record is incomplete or malformed */ public static function fromArray(array $data): self { - foreach (['state', 'code_verifier', 'redirect_uri', 'resource', 'audience'] as $key) { + foreach (['state', 'code_verifier', 'redirect_uri', 'resource', 'client_id'] as $key) { if (!\is_string($data[$key] ?? null) || '' === $data[$key]) { throw new InvalidArgumentException(\sprintf('Pending authorization is missing "%s".', $key)); } } - if (!\is_int($data['expires_at'] ?? null) || !\is_array($data['scopes'] ?? null) || !\is_array($data['metadata'] ?? null) || !\is_array($data['client'] ?? null)) { + if (!\is_int($data['expires_at'] ?? null) || !\is_array($data['scopes'] ?? null) || !\is_array($data['metadata'] ?? null)) { throw new InvalidArgumentException('Invalid pending authorization.'); } + $scopes = []; + foreach ($data['scopes'] as $scope) { if (!\is_string($scope)) { throw new InvalidArgumentException('Invalid pending authorization scope.'); } + + $scopes[] = $scope; } return new self( - $data['state'], $data['code_verifier'], $data['redirect_uri'], - $data['resource'], $data['audience'], array_values($data['scopes']), - AuthorizationServerMetadata::fromArray($data['metadata']), - ClientRegistration::fromArray($data['client']), $data['expires_at'], - true === ($data['offline_access_refused'] ?? false), + state: $data['state'], + codeVerifier: $data['code_verifier'], + redirectUri: $data['redirect_uri'], + resource: $data['resource'], + scopes: $scopes, + metadata: AuthorizationServerMetadata::fromArray($data['metadata']), + clientId: $data['client_id'], + expiresAt: $data['expires_at'], + offlineAccessRefused: true === ($data['offline_access_refused'] ?? false), ); } } diff --git a/tests/Unit/Client/Auth/FileCredentialStorageTest.php b/tests/Unit/Client/Auth/FileCredentialStorageTest.php index d27adc249..05e19b242 100644 --- a/tests/Unit/Client/Auth/FileCredentialStorageTest.php +++ b/tests/Unit/Client/Auth/FileCredentialStorageTest.php @@ -172,7 +172,16 @@ public function testMalformedPendingAuthorizationIsDiscarded(): void private function pending(string $state, string $issuer = 'https://auth.example.com'): PendingAuthorization { - return new PendingAuthorization($state, 'verifier', 'https://app.example.com/callback', 'https://mcp.example.com/mcp', 'https://mcp.example.com/mcp', ['mcp:read'], new AuthorizationServerMetadata($issuer, $issuer.'/authorize', $issuer.'/token', issuerParameterSupported: true), new ClientRegistration('client', 'secret'), 4102444800); + return new PendingAuthorization( + state: $state, + codeVerifier: 'verifier', + redirectUri: 'https://app.example.com/callback', + resource: 'https://mcp.example.com/mcp', + scopes: ['mcp:read'], + metadata: new AuthorizationServerMetadata($issuer, $issuer.'/authorize', $issuer.'/token', codeChallengeMethodsSupported: ['S256'], issuerParameterSupported: true), + clientId: 'client', + expiresAt: 4102444800, + ); } #[TestDox('a corrupted file costs one authorization, not a crash')] diff --git a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php index 91b6f3f7b..cb8cf5579 100644 --- a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php +++ b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php @@ -730,6 +730,77 @@ public function testDeferredAuthorizationRequiresPendingStorage(): void $authenticator->handleChallenge($this->request(), $server->challenge()); } + #[TestDox('the secret of a configured client stays out of a pending authorization')] + public function testDeferredAuthorizationDoesNotStoreTheClientSecret(): void + { + $path = tempnam(sys_get_temp_dir(), 'mcp-pending-'); + $server = new FakeOAuthServer(tokenEndpointAuthMethods: ['client_secret_post']); + $configuration = OAuth::forApplication('test')->setClientCredentials('known-client', 'known-secret')->setExpectedIssuer('https://auth.example.com'); + + try { + $callback = $this->startPending($server, new FileCredentialStorage($path), $configuration); + $this->assertStringNotContainsString('known-secret', (string) file_get_contents($path)); + + $this->authenticator($server, storage: new FileCredentialStorage($path), configuration: $configuration)->finishAuthorization($callback); + $this->assertSame('known-client', $server->token['client_id'] ?? null); + $this->assertSame('known-secret', $server->token['client_secret'] ?? null); + } finally { + array_map('unlink', glob($path.'*') ?: []); + } + } + + #[TestDox('a pending authorization is not finished with an authorization server other than the expected one')] + public function testDeferredAuthorizationChecksTheExpectedIssuer(): void + { + $server = new FakeOAuthServer(); + $storage = new InMemoryCredentialStorage(); + $callback = $this->startPending($server, $storage); + $configuration = OAuth::forApplication('test')->setClientCredentials('known-client')->setExpectedIssuer('https://other.example.com'); + + try { + $this->authenticator($server, storage: $storage, configuration: $configuration)->finishAuthorization($callback); + $this->fail('Expected the pending authorization to be refused.'); + } catch (AuthorizationException $e) { + $this->assertStringContainsString('only "https://other.example.com" is expected', $e->getMessage()); + $this->assertNotContains('https://auth.example.com/token', $server->requested); + } + } + + #[TestDox('a pending authorization started with another client is not finished')] + public function testDeferredAuthorizationRefusesAnotherClient(): void + { + $server = new FakeOAuthServer(); + $storage = new InMemoryCredentialStorage(); + $callback = $this->startPending($server, $storage, OAuth::forApplication('test')->setClientCredentials('known-client')->setExpectedIssuer('https://auth.example.com')); + $configuration = OAuth::forApplication('test')->setClientCredentials('other-client')->setExpectedIssuer('https://auth.example.com'); + + try { + $this->authenticator($server, storage: $storage, configuration: $configuration)->finishAuthorization($callback); + $this->fail('Expected the pending authorization to be refused.'); + } catch (AuthorizationException $e) { + $this->assertStringContainsString('"known-client"', $e->getMessage()); + $this->assertNotContains('https://auth.example.com/token', $server->requested); + } + } + + #[TestDox('a registration the authorization server forgot is dropped when the callback is refused')] + public function testDeferredAuthorizationForgetsARefusedRegistration(): void + { + $server = new FakeOAuthServer(); + $storage = new InMemoryCredentialStorage(); + $callback = $this->startPending($server, $storage); + $server->unknownClient = 'registered-client'; + + try { + $this->authenticator($server, storage: $storage)->finishAuthorization($callback); + $this->fail('Expected invalid_client.'); + } catch (OAuthErrorException $e) { + $this->assertSame('invalid_client', $e->error); + } + + $this->assertNull($storage->getClientRegistration('https://auth.example.com')); + } + /** * @return array */ From 1d951eb57fead02604cb47f8a9ea8d38919f1452 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 10 Nov 2026 02:09:48 +0100 Subject: [PATCH 05/13] [Client] Refuse RedirectAuthorizationHandler without a credential storage --- src/Client/Auth/OAuth.php | 5 +++++ tests/Unit/Client/Auth/OAuthTest.php | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/Client/Auth/OAuth.php b/src/Client/Auth/OAuth.php index 01dd51d8a..f25a423ab 100644 --- a/src/Client/Auth/OAuth.php +++ b/src/Client/Auth/OAuth.php @@ -281,6 +281,11 @@ public function setLogger(LoggerInterface $logger): self public function build(): OAuthAuthenticator { + // The pending authorization would be lost with the request that started it. + if ($this->authorizationHandler instanceof RedirectAuthorizationHandler && null === $this->storage) { + throw new InvalidArgumentException('RedirectAuthorizationHandler finishes the authorization in a later request, so it needs a persistent storage. Call OAuth::setCredentialStorage().'); + } + $httpClient = null === $this->httpClient ? NoRedirects::discover() : NoRedirects::apply($this->httpClient); $requestFactory = $this->requestFactory ?? Psr17FactoryDiscovery::findRequestFactory(); $streamFactory = $this->streamFactory ?? Psr17FactoryDiscovery::findStreamFactory(); diff --git a/tests/Unit/Client/Auth/OAuthTest.php b/tests/Unit/Client/Auth/OAuthTest.php index 72c9327be..dcd64dab5 100644 --- a/tests/Unit/Client/Auth/OAuthTest.php +++ b/tests/Unit/Client/Auth/OAuthTest.php @@ -19,6 +19,7 @@ use Mcp\Client\Auth\OAuth; use Mcp\Client\Auth\OAuthAuthenticator; use Mcp\Client\Auth\OAuthConfiguration; +use Mcp\Client\Auth\RedirectAuthorizationHandler; use Mcp\Client\Auth\TokenEndpointAuthMethod; use Mcp\Exception\AuthorizationException; use Mcp\Exception\InvalidArgumentException; @@ -98,6 +99,15 @@ public function testConfiguredClientNeedsAnExpectedIssuer(string $clientId, ?str OAuth::forApplication('My App')->setClientCredentials($clientId, $clientSecret)->build(); } + #[TestDox('the redirect handler needs a storage that outlives the request')] + public function testRedirectHandlerNeedsAStorage(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('setCredentialStorage()'); + + OAuth::forApplication('My App')->setAuthorizationHandler(new RedirectAuthorizationHandler())->build(); + } + #[TestDox('a client that registers itself needs no expected issuer')] public function testDynamicRegistrationNeedsNoExpectedIssuer(): void { From 7d1b0b3f07b6d890e92357fdc1d18f7d5cf3fb16 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 10 Nov 2026 02:10:57 +0100 Subject: [PATCH 06/13] [Client] Don't log a pending authorization as a transport error --- src/Client/Transport/HttpTransport.php | 4 +++ .../Client/Transport/HttpTransportTest.php | 36 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/Client/Transport/HttpTransport.php b/src/Client/Transport/HttpTransport.php index 7e2f1f2e4..5c4c36a28 100644 --- a/src/Client/Transport/HttpTransport.php +++ b/src/Client/Transport/HttpTransport.php @@ -16,6 +16,7 @@ use Mcp\Client\Auth\AuthenticatorInterface; use Mcp\Client\MonotonicClock; use Mcp\Exception\AuthorizationException; +use Mcp\Exception\AuthorizationPendingException; use Mcp\Exception\ConnectionException; use Mcp\Exception\InvalidArgumentException; use Mcp\Exception\RequestCancelledException; @@ -266,6 +267,9 @@ public function send(string $data): void try { $response = $this->httpClient->sendRequest($request); + } catch (AuthorizationPendingException $e) { + // Not an error: the application sends the user to the authorization server. + throw $e; } catch (AuthorizationException $e) { // Not a connection problem, and not one a second attempt would solve: the // authenticator already did everything it could. diff --git a/tests/Unit/Client/Transport/HttpTransportTest.php b/tests/Unit/Client/Transport/HttpTransportTest.php index 9131352fa..f2960eca8 100644 --- a/tests/Unit/Client/Transport/HttpTransportTest.php +++ b/tests/Unit/Client/Transport/HttpTransportTest.php @@ -24,6 +24,7 @@ use Mcp\Client\Transport\HttpTransport; use Mcp\Client\Transport\TransportInterface; use Mcp\Exception\AuthorizationException; +use Mcp\Exception\AuthorizationPendingException; use Mcp\Exception\ConnectionException; use Mcp\Exception\InvalidArgumentException; use Mcp\Exception\RequestCancelledException; @@ -173,6 +174,41 @@ public function testConnectReportsARefusal(int $status): void $this->assertCount(1, $httpClient->requests); } + #[TestDox('connect() hands a pending authorization over as it is, without retrying or logging an error')] + public function testConnectHandsOverAPendingAuthorization(): void + { + $httpClient = RecordingHttpClient::queued([new Response(401, ['WWW-Authenticate' => 'Bearer realm="mcp"'])]); + $auth = new class implements AuthenticatorInterface { + public function authenticate(RequestInterface $request): RequestInterface + { + return $request; + } + + public function handleChallenge(RequestInterface $request, ResponseInterface $response): bool + { + throw new AuthorizationPendingException('https://auth.example.com/authorize?state=abc'); + } + }; + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->never())->method('error'); + + $client = Client::builder() + ->setClientInfo('test-client', '1.0.0') + ->setInitTimeout(1) + ->setMaxRetries(2) + ->setLogger($logger) + ->build(); + + try { + $client->connect(new HttpTransport('https://example.test/mcp', [], $httpClient, $this->factory, $this->factory, $logger, auth: $auth)); + $this->fail('The client should not have connected.'); + } catch (AuthorizationPendingException $e) { + $this->assertSame('https://auth.example.com/authorize?state=abc', $e->authorizationUrl); + } + + $this->assertCount(1, $httpClient->requests); + } + #[DataProvider('refusedStatusProvider')] #[TestDox('a refused request after connecting fails with an AuthorizationException ($_dataName)')] public function testRequestReportsARefusal(int $status): void From b75379dedcab00ce9b19bd3e72fc413ba8fb5231 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 10 Nov 2026 02:12:01 +0100 Subject: [PATCH 07/13] [Client] Fix PHPStan in the deferred authorization tests and describe them with TestDox --- tests/Integration/ClientOAuthTest.php | 9 +++++---- .../Fixture/oauth/finish-authorization.php | 4 ++-- .../Unit/Client/Auth/FileCredentialStorageTest.php | 5 ++++- tests/Unit/Client/Auth/OAuthAuthenticatorTest.php | 14 ++++++++++++-- 4 files changed, 23 insertions(+), 9 deletions(-) diff --git a/tests/Integration/ClientOAuthTest.php b/tests/Integration/ClientOAuthTest.php index d57c0275c..b4bfd7cde 100644 --- a/tests/Integration/ClientOAuthTest.php +++ b/tests/Integration/ClientOAuthTest.php @@ -82,7 +82,7 @@ public function testAuthorizesFromNothing(): void $client = $this->connect(); $this->assertSame(['whoami'], array_map(static fn ($tool) => $tool->name, $client->listTools()->tools)); - $this->assertStringContainsString('the token was accepted', (string) json_encode($client->callTool('whoami', [])->content)); + $this->assertStringContainsString('the token was accepted', (string) json_encode($client->callTool('whoami', [])->content, \JSON_THROW_ON_ERROR)); $client->disconnect(); } @@ -202,6 +202,7 @@ public function testHeadlessHandlerNeedsARedirect(): void (new HeadlessAuthorizationHandler())->authorize($this->authorizationServerUrl.'/nothing-here', 'http://127.0.0.1:8765/callback'); } + #[TestDox('an authorization started in one process is finished in another, and only once')] public function testCompletesRedirectAuthorizationInAnotherProcess(): void { $path = $this->workingDirectory.'/credentials.json'; @@ -219,7 +220,7 @@ public function testCompletesRedirectAuthorizationInAnotherProcess(): void } unset($client, $auth); - $command = [\PHP_BINARY, __DIR__.'/Fixture/oauth/finish-authorization.php', $path, json_encode($callback)]; + $command = [\PHP_BINARY, __DIR__.'/Fixture/oauth/finish-authorization.php', $path, json_encode($callback, \JSON_THROW_ON_ERROR)]; $process = new Process($command); $competitor = new Process($command); $process->start(); @@ -233,9 +234,9 @@ public function testCompletesRedirectAuthorizationInAnotherProcess(): void $this->assertStringContainsString('does not match a pending authorization', $loser->getErrorOutput().$loser->getOutput()); $client = $this->connect(new FileCredentialStorage($path)); - $this->assertStringContainsString('the token was accepted', json_encode($client->callTool('whoami', [])->content)); + $this->assertStringContainsString('the token was accepted', json_encode($client->callTool('whoami', [])->content, \JSON_THROW_ON_ERROR)); $client->disconnect(); - $this->assertSame([], json_decode(file_get_contents($path), true)['pending']); + $this->assertSame([], json_decode((string) file_get_contents($path), true)['pending']); $replay = new Process($command); $replay->run(); diff --git a/tests/Integration/Fixture/oauth/finish-authorization.php b/tests/Integration/Fixture/oauth/finish-authorization.php index b419cce78..6c3c61c9a 100644 --- a/tests/Integration/Fixture/oauth/finish-authorization.php +++ b/tests/Integration/Fixture/oauth/finish-authorization.php @@ -17,8 +17,8 @@ $auth = OAuth::forApplication('Integration Test Client') ->setAuthorizationHandler(new RedirectAuthorizationHandler()) - ->setCredentialStorage(new FileCredentialStorage($argv[1])) + ->setCredentialStorage(new FileCredentialStorage($_SERVER['argv'][1])) ->build(); -$auth->finishAuthorization(json_decode($argv[2], true, flags: \JSON_THROW_ON_ERROR)); +$auth->finishAuthorization(json_decode($_SERVER['argv'][2], true, flags: \JSON_THROW_ON_ERROR)); echo 'authorized'; diff --git a/tests/Unit/Client/Auth/FileCredentialStorageTest.php b/tests/Unit/Client/Auth/FileCredentialStorageTest.php index 05e19b242..747f51e07 100644 --- a/tests/Unit/Client/Auth/FileCredentialStorageTest.php +++ b/tests/Unit/Client/Auth/FileCredentialStorageTest.php @@ -127,6 +127,7 @@ public function testFileIsNeverExposedWhileBeingWritten(): void $this->assertCount(1, glob(\dirname($this->path).'/*') ?: [], 'The temporary file should not survive the write.'); } + #[TestDox('a pending authorization is consumed once, whichever instance reads it')] public function testPendingAuthorizationsAreConsumedAcrossInstances(): void { $first = new FileCredentialStorage($this->path); @@ -141,6 +142,7 @@ public function testPendingAuthorizationsAreConsumedAcrossInstances(): void $this->assertSame('token', $second->getToken('https://auth.example.com', 'https://mcp.example.com/mcp')?->accessToken); } + #[TestDox('forgetting an authorization server drops only its pending authorizations')] public function testForgetRemovesOnlyTheIssuersPendingAuthorizations(): void { $storage = new FileCredentialStorage($this->path); @@ -155,11 +157,12 @@ public function testForgetRemovesOnlyTheIssuersPendingAuthorizations(): void $this->assertNull($storage->consumePendingAuthorization('three')); } + #[TestDox('a malformed pending authorization is discarded')] public function testMalformedPendingAuthorizationIsDiscarded(): void { $storage = new FileCredentialStorage($this->path); $storage->savePendingAuthorization($this->pending('one')); - $data = json_decode(file_get_contents($this->path), true); + $data = json_decode((string) file_get_contents($this->path), true); unset($data['pending']['one']['code_verifier']); foreach ([$data['pending']['one'], 'corrupt record'] as $record) { diff --git a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php index cb8cf5579..f3ec10a1a 100644 --- a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php +++ b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php @@ -552,6 +552,7 @@ public function testSkipsRegistrationForAKnownClient(): void $this->assertSame('known-client', $server->authorization['client_id'] ?? null); } + #[TestDox('an authorization sent to the browser is finished by another authenticator')] public function testFinishesAuthorizationInAnotherRequest(): void { $path = tempnam(sys_get_temp_dir(), 'mcp-pending-'); @@ -567,7 +568,7 @@ public function testFinishesAuthorizationInAnotherRequest(): void } unset($authenticator); - $saved = json_decode(file_get_contents($path), true); + $saved = json_decode((string) file_get_contents($path), true); $this->assertCount(1, $saved['pending']); $resumed = $this->authenticator($server, storage: new FileCredentialStorage($path)); $resumed->finishAuthorization([ @@ -582,7 +583,7 @@ public function testFinishesAuthorizationInAnotherRequest(): void $this->assertSame($server->authorization['resource'], $server->token['resource']); $stored = (new FileCredentialStorage($path))->getToken('https://auth.example.com', self::ENDPOINT); $this->assertSame(['mcp:read'], $stored?->scopes); - $this->assertSame([], json_decode(file_get_contents($path), true)['pending']); + $this->assertSame([], json_decode((string) file_get_contents($path), true)['pending']); } finally { unlink($path); @unlink($path.'.lock'); @@ -590,6 +591,7 @@ public function testFinishesAuthorizationInAnotherRequest(): void } #[DataProvider('invalidDeferredCallbacks')] + #[TestDox('a callback that does not match the pending authorization is refused before the code is spent ($_dataName)')] public function testRejectsInvalidDeferredCallbacks(array $replacement): void { $server = new FakeOAuthServer(serverScopes: ['mcp:read', 'offline_access']); @@ -624,6 +626,7 @@ public static function invalidDeferredCallbacks(): iterable yield 'scope error in issuer' => [['iss' => 'https://invalid_scope.example.com']]; } + #[TestDox('a callback finishes its authorization only once')] public function testDeferredCallbackCanOnlyBeConsumedOnce(): void { $server = new FakeOAuthServer(); @@ -636,12 +639,14 @@ public function testDeferredCallbackCanOnlyBeConsumedOnce(): void $this->authenticator($server, storage: $storage)->finishAuthorization($callback); } + #[TestDox('an expired pending authorization is not finished')] public function testDeferredAuthorizationExpires(): void { $server = new FakeOAuthServer(); $storage = new InMemoryCredentialStorage(); $callback = $this->startPending($server, $storage); $pending = $storage->consumePendingAuthorization($callback['state']); + $this->assertNotNull($pending); $data = $pending->jsonSerialize(); $data['expires_at'] = time() - 1; $storage->savePendingAuthorization(PendingAuthorization::fromArray($data)); @@ -650,6 +655,7 @@ public function testDeferredAuthorizationExpires(): void $this->authenticator($server, storage: $storage)->finishAuthorization($callback); } + #[TestDox('a callback is not finished with the storage of another user')] public function testCallbackCannotBeCompletedWithAnotherUsersStorage(): void { $server = new FakeOAuthServer(); @@ -660,6 +666,7 @@ public function testCallbackCannotBeCompletedWithAnotherUsersStorage(): void } #[DataProvider('offlineAccessRefusals')] + #[TestDox('a refused offline_access starts the authorization again without it ($_dataName)')] public function testDeferredOfflineAccessFallback(bool $atCallback): void { $server = new FakeOAuthServer(resourceScopes: ['mcp:read'], serverScopes: ['mcp:read', 'offline_access'], refuseOfflineAccess: !$atCallback); @@ -691,6 +698,7 @@ public static function offlineAccessRefusals(): iterable yield 'token endpoint' => [false]; } + #[TestDox('a handler that returns the callback still reports a refusal')] public function testSynchronousAuthorizationPreservesErrorResponses(): void { $server = new FakeOAuthServer(); @@ -706,6 +714,7 @@ public function testSynchronousAuthorizationPreservesErrorResponses(): void $this->authenticator($server, $handler)->handleChallenge($this->request(), $server->challenge()); } + #[TestDox('a storage without pending authorizations still works with a handler that returns the callback')] public function testSynchronousAuthorizationSupportsExistingCredentialStorage(): void { $server = new FakeOAuthServer(); @@ -718,6 +727,7 @@ public function testSynchronousAuthorizationSupportsExistingCredentialStorage(): $this->assertSame('Bearer access-token-1', $authenticator->authenticate($this->request())->getHeaderLine('Authorization')); } + #[TestDox('the redirect handler needs a storage that keeps pending authorizations')] public function testDeferredAuthorizationRequiresPendingStorage(): void { $server = new FakeOAuthServer(); From 033c0085798beec9b1f5832d0a66cb69fbab9388 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 10 Nov 2026 02:13:07 +0100 Subject: [PATCH 08/13] [Client] Document finishing the authorization in a later request --- CHANGELOG.md | 2 +- docs/client/authorization.md | 82 ++++++++++++++++++++++-------------- 2 files changed, 51 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d49cb7561..1f8edb2a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -103,6 +103,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * 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. * Add `OAuth::setExpectedIssuer()`, required with `setClientCredentials()`, so a hostile MCP server can't receive the secret or an authorization code. +* Add `RedirectAuthorizationHandler` and `OAuthAuthenticator::finishAuthorization()`, to finish the authorization in a later request (e.g. the callback route of a web application). Its storage implements `PendingAuthorizationStorageInterface`, as `FileCredentialStorage` does. * Add `LoopbackAuthorizationHandler` and `HeadlessAuthorizationHandler` behind `AuthorizationHandlerInterface`, and `InMemoryCredentialStorage` and `FileCredentialStorage` behind `CredentialStorageInterface`. * [BC Break] A `401` or `403` answer to a request, a notification or the resumption of an event stream fails with an `AuthorizationException` that contains the `WWW-Authenticate` header, instead of a `RequestException`. `Client::connect()` wraps it in a `ConnectionException`, without a retry. * Fix `Client::connect()` leaving the previous transport open when called again: it closes the current transport first, so no stdio process or HTTP session is left behind. @@ -122,7 +123,6 @@ All notable changes to `mcp/sdk` will be documented in this file. 0.9.0 ----- -* Add `RedirectAuthorizationHandler` and `OAuthAuthenticator::finishAuthorization()` to complete an authorization code flow in a later request. `PendingAuthorizationStorageInterface` supports expiring, single-use authorization state without changing existing credential storage implementations. ### TL;DR * Concurrent requests of one session keep their own answers, session changes and fibers diff --git a/docs/client/authorization.md b/docs/client/authorization.md index dbed615cf..1024072c1 100644 --- a/docs/client/authorization.md +++ b/docs/client/authorization.md @@ -72,58 +72,74 @@ final class AcmeAuthorizationHandler implements AuthorizationHandlerInterface The default redirect URI is `http://127.0.0.1:8765/callback`. Change it with `setRedirectUri()` if the port is in use or if the authorization server expects another one. The URI must use `https` or a loopback host (e.g. `127.0.0.1`), and must not have a fragment or userinfo. Otherwise, `build()` throws an `InvalidArgumentException`. The loopback handler listens on the host and port of this URI, so it only works with `http` on a loopback host. For an `https` URI, implement your own handler. -### Web application callbacks +## Storing Credentials -For a callback that arrives in a later HTTP request, use `RedirectAuthorizationHandler` -with persistent storage scoped to the browser session that starts the flow: +By default, the client keeps its tokens in memory. A command line application then sends the user to the browser on every run. Use `FileCredentialStorage` to keep them between runs: ```php use Mcp\Client\Auth\FileCredentialStorage; use Mcp\Client\Auth\OAuth; -use Mcp\Client\Auth\RedirectAuthorizationHandler; -use Mcp\Exception\AuthorizationPendingException; -$auth = OAuth::forApplication('My Web App') - ->setRedirectUri('https://app.example.com/oauth/callback') - ->setAuthorizationHandler(new RedirectAuthorizationHandler()) - ->setCredentialStorage(new FileCredentialStorage($sessionCredentialPath)) +$auth = OAuth::forApplication('My App') + ->setCredentialStorage(new FileCredentialStorage($_SERVER['HOME'].'/.config/my-app/credentials.json')) ->build(); ``` -Catch `AuthorizationPendingException` around the MCP operation and redirect the browser -to `$e->authorizationUrl`. The SDK saves the state, PKCE verifier and authorization -context before the exception reaches your application. In the callback route, rebuild -the authenticator with the same configuration and session storage and call -`$auth->finishAuthorization($callbackParameters)`, then retry the MCP operation. -The callback may also throw `AuthorizationPendingException` if the server refuses -`offline_access` and authorization needs to restart without that scope. +The file contains tokens and client secrets. The SDK creates it with the mode `0600` in a directory with the mode `0700`, and doesn't encrypt it. Windows ignores these modes, so store the file in a directory that only you can read (e.g. your user profile). The SDK also creates two lock files next to it, so processes can share the file and only one of them refreshes a token at a time. To use a keychain or a secrets manager, implement `CredentialStorageInterface`. Its `synchronize()` method must run one operation at a time across all processes that share the storage. -Pending flows expire after ten minutes and are consumed once, before issuer/code -validation and token exchange; restart authorization if completion fails. Custom -storage must implement `PendingAuthorizationStorageInterface` with atomic consumption. -`InMemoryCredentialStorage` cannot persist state across independent PHP requests. -Resolve the storage from the current browser session, never from callback parameters, -and do not share it between users. In long-running workers, create an authenticator -per user request; tokens are also held in the authenticator's memory. +The storage keys everything by the issuer of the authorization server. If a server moves to another authorization server, the client registers again and doesn't send its old client ID to the new one. It also registers again if you change the redirect URI, if it needs scopes that its registration doesn't include, or if the token endpoint answers `invalid_client` for a client that it registered. -### The redirect URI +## Finishing the Authorization in a Later Request -## Storing Credentials - -By default, the client keeps its tokens in memory. A command line application then sends the user to the browser on every run. Use `FileCredentialStorage` to keep them between runs: +A web application can't wait for the browser within one request. It redirects the user to the authorization server, which sends the user back to a callback route of the application. Use `RedirectAuthorizationHandler` for this, with one storage per user session: ```php use Mcp\Client\Auth\FileCredentialStorage; use Mcp\Client\Auth\OAuth; +use Mcp\Client\Auth\OAuthAuthenticator; +use Mcp\Client\Auth\RedirectAuthorizationHandler; -$auth = OAuth::forApplication('My App') - ->setCredentialStorage(new FileCredentialStorage($_SERVER['HOME'].'/.config/my-app/credentials.json')) - ->build(); +function createAuthenticator(): OAuthAuthenticator +{ + return OAuth::forApplication('Acme Shop') + ->setRedirectUri('https://shop.example.com/oauth/callback') + ->setAuthorizationHandler(new RedirectAuthorizationHandler()) + ->setCredentialStorage(new FileCredentialStorage('/var/lib/acme/mcp/'.hash('sha256', session_id()).'.json')) + ->build(); +} ``` -The file contains tokens and client secrets. The SDK creates it with the mode `0600` in a directory with the mode `0700`, and doesn't encrypt it. Windows ignores these modes, so store the file in a directory that only you can read (e.g. your user profile). The SDK also creates two lock files next to it, so processes can share the file and only one of them refreshes a token at a time. To use a keychain or a secrets manager, implement `CredentialStorageInterface`. Its `synchronize()` method must run one operation at a time across all processes that share the storage. +When the client needs a token, the handler throws an `AuthorizationPendingException`. Before that, the SDK stores the `state`, the PKCE verifier and the metadata of the authorization server. Redirect the browser to the URL of the exception: -The storage keys everything by the issuer of the authorization server. If a server moves to another authorization server, the client registers again and doesn't send its old client ID to the new one. It also registers again if you change the redirect URI, if it needs scopes that its registration doesn't include, or if the token endpoint answers `invalid_client` for a client that it registered. +```php +use Mcp\Client\Transport\HttpTransport; +use Mcp\Exception\AuthorizationPendingException; + +try { + $client->connect(new HttpTransport('https://mcp.example.com/mcp', auth: createAuthenticator())); +} catch (AuthorizationPendingException $e) { + header('Location: '.$e->authorizationUrl); + exit; +} +``` + +`connect()` and the requests after it throw this exception directly. In the callback route, pass the query parameters to `finishAuthorization()` of an authenticator with the same configuration and storage: + +```php +createAuthenticator()->finishAuthorization($_GET); + +// back to the page that needs the MCP server +header('Location: /orders'); +``` + +The SDK checks the callback like any other, exchanges the code and stores the token. The next `connect()` with this storage uses it. If the authorization server refuses `offline_access`, `finishAuthorization()` throws an `AuthorizationPendingException` again, to authorize without that scope. + +A pending authorization expires after 10 minutes. The SDK removes it before it checks the callback, so each callback works once. If `finishAuthorization()` throws an `AuthorizationException`, send the user through the authorization again. + +!!! warning + The SDK finds the pending authorization by the `state` parameter of the callback. Select the storage from the user session, never from the callback. With a storage that all users share, one user can finish an authorization that another user started. + +`build()` throws an `InvalidArgumentException` if you use `RedirectAuthorizationHandler` without `setCredentialStorage()`. For your own storage, implement `PendingAuthorizationStorageInterface`. Its `consumePendingAuthorization()` method must read and remove the entry in one step, so two requests can't use the same callback. ## Using a Pre-registered Client @@ -294,4 +310,6 @@ try { } ``` +An `AuthorizationPendingException` is never wrapped, see [Finishing the Authorization in a Later Request](#finishing-the-authorization-in-a-later-request). + After `connect()`, requests throw the `AuthorizationException` directly. The client doesn't retry a failed authorization. If the token endpoint is unavailable during a refresh, the request fails and the client keeps the refresh token. Pass a PSR-3 logger with `setLogger()` to see each step. From 6460aede10ea0efaf8856c7ab34188db97f2cad1 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 10 Nov 2026 09:24:00 +0100 Subject: [PATCH 09/13] [Client] Require a started session in the deferred authorization docs --- docs/client/authorization.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/client/authorization.md b/docs/client/authorization.md index 1024072c1..9c08ea39b 100644 --- a/docs/client/authorization.md +++ b/docs/client/authorization.md @@ -109,6 +109,8 @@ function createAuthenticator(): OAuthAuthenticator } ``` +Start the PHP session in both routes before you call this function. Without a session, `session_id()` returns an empty string, and all users share one file. + When the client needs a token, the handler throws an `AuthorizationPendingException`. Before that, the SDK stores the `state`, the PKCE verifier and the metadata of the authorization server. Redirect the browser to the URL of the exception: ```php From 8a725a3316b86b03eb850d47a5103338cd46d67d Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 10 Nov 2026 09:32:02 +0100 Subject: [PATCH 10/13] [Client] Register the web application in the deferred authorization docs as web --- docs/client/authorization.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/client/authorization.md b/docs/client/authorization.md index 9c08ea39b..edc4fc8f9 100644 --- a/docs/client/authorization.md +++ b/docs/client/authorization.md @@ -102,6 +102,7 @@ use Mcp\Client\Auth\RedirectAuthorizationHandler; function createAuthenticator(): OAuthAuthenticator { return OAuth::forApplication('Acme Shop') + ->setApplicationType('web') ->setRedirectUri('https://shop.example.com/oauth/callback') ->setAuthorizationHandler(new RedirectAuthorizationHandler()) ->setCredentialStorage(new FileCredentialStorage('/var/lib/acme/mcp/'.hash('sha256', session_id()).'.json')) @@ -111,6 +112,8 @@ function createAuthenticator(): OAuthAuthenticator Start the PHP session in both routes before you call this function. Without a session, `session_id()` returns an empty string, and all users share one file. +The client registers as a `native` application by default. Authorization servers may refuse an `https` redirect URI for it, so a web application calls `setApplicationType('web')`. + When the client needs a token, the handler throws an `AuthorizationPendingException`. Before that, the SDK stores the `state`, the PKCE verifier and the metadata of the authorization server. Redirect the browser to the URL of the exception: ```php From 92788f2325cd5f1172536a77afa9da2988b63d4c Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 10 Nov 2026 09:59:43 +0100 Subject: [PATCH 11/13] [Client] Remove the hidden credential lock files after the OAuth tests --- tests/Integration/ClientOAuthTest.php | 9 +++------ .../Unit/Client/Auth/OAuthAuthenticatorTest.php | 16 +++++++++++++--- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/tests/Integration/ClientOAuthTest.php b/tests/Integration/ClientOAuthTest.php index b4bfd7cde..f77fdac4c 100644 --- a/tests/Integration/ClientOAuthTest.php +++ b/tests/Integration/ClientOAuthTest.php @@ -69,11 +69,7 @@ protected function tearDown(): void $this->authorizationServer->stop(); $this->resourceServer->stop(); - foreach (glob($this->workingDirectory.'/*') ?: [] as $file) { - is_dir($file) ? $this->removeDirectory($file) : unlink($file); - } - - @rmdir($this->workingDirectory); + $this->removeDirectory($this->workingDirectory); } #[TestDox('a client with no credentials at all ends up calling a protected tool')] @@ -319,7 +315,8 @@ private static function freePort(): int private function removeDirectory(string $directory): void { - foreach (glob($directory.'/*') ?: [] as $file) { + foreach (array_diff(scandir($directory) ?: [], ['.', '..']) as $name) { + $file = $directory.'/'.$name; is_dir($file) ? $this->removeDirectory($file) : unlink($file); } diff --git a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php index f3ec10a1a..b67b3b764 100644 --- a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php +++ b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php @@ -585,8 +585,7 @@ public function testFinishesAuthorizationInAnotherRequest(): void $this->assertSame(['mcp:read'], $stored?->scopes); $this->assertSame([], json_decode((string) file_get_contents($path), true)['pending']); } finally { - unlink($path); - @unlink($path.'.lock'); + $this->removeStorage($path); } } @@ -755,7 +754,7 @@ public function testDeferredAuthorizationDoesNotStoreTheClientSecret(): void $this->assertSame('known-client', $server->token['client_id'] ?? null); $this->assertSame('known-secret', $server->token['client_secret'] ?? null); } finally { - array_map('unlink', glob($path.'*') ?: []); + $this->removeStorage($path); } } @@ -826,6 +825,17 @@ private function startPending(FakeOAuthServer $server, CredentialStorageInterfac return ['code' => 'test-code', 'state' => $server->authorization['state'], 'iss' => 'https://auth.example.com']; } + private function removeStorage(string $path): void + { + $lockPrefix = \dirname($path).'/.'.basename($path); + + foreach ([$path, $lockPrefix.'.lock', $lockPrefix.'.refresh.lock'] as $file) { + if (is_file($file)) { + unlink($file); + } + } + } + /** * @return iterable */ From dcd83bafb7977545f81d6137d513903065e7450d Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 10 Nov 2026 10:27:02 +0100 Subject: [PATCH 12/13] [Client] Finish a deferred authorization with its own dynamic registration --- src/Client/Auth/OAuthAuthenticator.php | 9 +++-- src/Client/Auth/PendingAuthorization.php | 7 ++++ .../Client/Auth/OAuthAuthenticatorTest.php | 34 +++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/Client/Auth/OAuthAuthenticator.php b/src/Client/Auth/OAuthAuthenticator.php index db82bb23f..2668cefcf 100644 --- a/src/Client/Auth/OAuthAuthenticator.php +++ b/src/Client/Auth/OAuthAuthenticator.php @@ -203,8 +203,8 @@ public function finishAuthorization(array $callback): void throw new AuthorizationException(\sprintf('The pending authorization belongs to the authorization server "%s", but only "%s" is expected.', $metadata->issuer, $this->configuration->expectedIssuer)); } - // The secret of a configured client stays out of the storage, so the client is resolved again. - $client = $this->resolveClient($metadata, $pending->scopes, null); + // Another flow may have replaced the registration of the issuer; a configured client is resolved again. + $client = $pending->registration ?? $this->resolveClient($metadata, $pending->scopes, null); if ($client->clientId !== $pending->clientId) { throw new AuthorizationException(\sprintf('The client "%s" of the pending authorization is no longer the client for "%s"; start the authorization again.', $pending->clientId, $metadata->issuer)); @@ -229,7 +229,9 @@ public function finishAuthorization(array $callback): void ])); } catch (AuthorizationException $e) { if (self::isForgottenClient($e, $client)) { - $this->storage->forget($metadata->issuer); + if ($client->clientId === $this->storage->getClientRegistration($metadata->issuer)?->clientId) { + $this->storage->forget($metadata->issuer); + } throw $e; } @@ -620,6 +622,7 @@ private function grant(AuthorizationServerMetadata $metadata, ClientRegistration clientId: $client->clientId, expiresAt: time() + 600, offlineAccessRefused: $this->offlineAccessRefused, + registration: $client->dynamic ? $client : null, )); throw $e; diff --git a/src/Client/Auth/PendingAuthorization.php b/src/Client/Auth/PendingAuthorization.php index a1b2254c0..78126bc18 100644 --- a/src/Client/Auth/PendingAuthorization.php +++ b/src/Client/Auth/PendingAuthorization.php @@ -31,6 +31,7 @@ public function __construct( public readonly string $clientId, public readonly int $expiresAt, public readonly bool $offlineAccessRefused = false, + public readonly ?ClientRegistration $registration = null, ) { } @@ -49,6 +50,7 @@ public function jsonSerialize(): array 'client_id' => $this->clientId, 'expires_at' => $this->expiresAt, 'offline_access_refused' => $this->offlineAccessRefused, + 'registration' => $this->registration?->jsonSerialize(), ]; } @@ -69,6 +71,10 @@ public static function fromArray(array $data): self throw new InvalidArgumentException('Invalid pending authorization.'); } + if (null !== ($data['registration'] ?? null) && !\is_array($data['registration'])) { + throw new InvalidArgumentException('Invalid pending authorization registration.'); + } + $scopes = []; foreach ($data['scopes'] as $scope) { @@ -89,6 +95,7 @@ public static function fromArray(array $data): self clientId: $data['client_id'], expiresAt: $data['expires_at'], offlineAccessRefused: true === ($data['offline_access_refused'] ?? false), + registration: \is_array($data['registration'] ?? null) ? ClientRegistration::fromArray($data['registration']) : null, ); } } diff --git a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php index b67b3b764..b3da8d235 100644 --- a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php +++ b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php @@ -792,6 +792,40 @@ public function testDeferredAuthorizationRefusesAnotherClient(): void } } + #[TestDox('a pending authorization is finished with its client after another flow registered a new one')] + public function testDeferredAuthorizationKeepsItsClientAfterAReRegistration(): void + { + $server = new FakeOAuthServer(); + $storage = new InMemoryCredentialStorage(); + $callback = $this->startPending($server, $storage); + $storage->saveClientRegistration('https://auth.example.com', new ClientRegistration('newer-client', 'newer-secret', TokenEndpointAuthMethod::ClientSecretPost, true, scopes: ['mcp:read', 'mcp:write'])); + + $authenticator = $this->authenticator($server, storage: $storage); + $authenticator->finishAuthorization($callback); + + $this->assertSame('registered-client', $server->token['client_id'] ?? null); + $this->assertSame('newer-client', $storage->getClientRegistration('https://auth.example.com')?->clientId); + } + + #[TestDox('a refused callback keeps the registration of another flow')] + public function testDeferredAuthorizationKeepsANewerRegistrationOnInvalidClient(): void + { + $server = new FakeOAuthServer(); + $storage = new InMemoryCredentialStorage(); + $callback = $this->startPending($server, $storage); + $storage->saveClientRegistration('https://auth.example.com', new ClientRegistration('newer-client', 'newer-secret', TokenEndpointAuthMethod::ClientSecretPost, true)); + $server->unknownClient = 'registered-client'; + + try { + $this->authenticator($server, storage: $storage)->finishAuthorization($callback); + $this->fail('Expected invalid_client.'); + } catch (OAuthErrorException $e) { + $this->assertSame('invalid_client', $e->error); + } + + $this->assertSame('newer-client', $storage->getClientRegistration('https://auth.example.com')?->clientId); + } + #[TestDox('a registration the authorization server forgot is dropped when the callback is refused')] public function testDeferredAuthorizationForgetsARefusedRegistration(): void { From 849aa6d9eddcdcff0b7d184831f98911646d2677 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 10 Nov 2026 10:44:55 +0100 Subject: [PATCH 13/13] [Client] Drop a refused registration with forgetRefusedClient() on a deferred callback --- src/Client/Auth/OAuthAuthenticator.php | 4 +- .../Client/Auth/OAuthAuthenticatorTest.php | 81 +++++++++++++++++++ 2 files changed, 82 insertions(+), 3 deletions(-) diff --git a/src/Client/Auth/OAuthAuthenticator.php b/src/Client/Auth/OAuthAuthenticator.php index 2668cefcf..0e1ca2a07 100644 --- a/src/Client/Auth/OAuthAuthenticator.php +++ b/src/Client/Auth/OAuthAuthenticator.php @@ -229,9 +229,7 @@ public function finishAuthorization(array $callback): void ])); } catch (AuthorizationException $e) { if (self::isForgottenClient($e, $client)) { - if ($client->clientId === $this->storage->getClientRegistration($metadata->issuer)?->clientId) { - $this->storage->forget($metadata->issuer); - } + $this->forgetRefusedClient($metadata->issuer, $client); throw $e; } diff --git a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php index b3da8d235..2d1e65e0a 100644 --- a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php +++ b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php @@ -24,6 +24,7 @@ use Mcp\Client\Auth\OAuthAuthenticator; use Mcp\Client\Auth\OAuthConfiguration; use Mcp\Client\Auth\PendingAuthorization; +use Mcp\Client\Auth\PendingAuthorizationStorageInterface; use Mcp\Client\Auth\RedirectAuthorizationHandler; use Mcp\Client\Auth\TokenEndpoint; use Mcp\Client\Auth\TokenEndpointAuthMethod; @@ -826,6 +827,86 @@ public function testDeferredAuthorizationKeepsANewerRegistrationOnInvalidClient( $this->assertSame('newer-client', $storage->getClientRegistration('https://auth.example.com')?->clientId); } + #[TestDox('a refused callback checks and drops the registration under the storage lock')] + public function testDeferredAuthorizationForgetsUnderTheLock(): void + { + $server = new FakeOAuthServer(); + $storage = new class implements PendingAuthorizationStorageInterface { + /** @var array */ + public array $synchronized = []; + + private readonly InMemoryCredentialStorage $storage; + + private bool $locked = false; + + public function __construct() + { + $this->storage = new InMemoryCredentialStorage(); + } + + public function savePendingAuthorization(PendingAuthorization $authorization): void + { + $this->storage->savePendingAuthorization($authorization); + } + + public function consumePendingAuthorization(string $state): ?PendingAuthorization + { + return $this->storage->consumePendingAuthorization($state); + } + + public function getClientRegistration(string $issuer): ?ClientRegistration + { + $this->synchronized['get'] = $this->locked; + + return $this->storage->getClientRegistration($issuer); + } + + public function saveClientRegistration(string $issuer, ClientRegistration $registration): void + { + $this->storage->saveClientRegistration($issuer, $registration); + } + + public function getToken(string $issuer, string $resource): ?AccessToken + { + return $this->storage->getToken($issuer, $resource); + } + + public function saveToken(string $issuer, string $resource, AccessToken $token): void + { + $this->storage->saveToken($issuer, $resource, $token); + } + + public function forget(?string $issuer = null): void + { + $this->synchronized['forget'] = $this->locked; + $this->storage->forget($issuer); + } + + public function synchronize(callable $operation): mixed + { + $this->locked = true; + + try { + return $operation(); + } finally { + $this->locked = false; + } + } + }; + $callback = $this->startPending($server, $storage); + $server->unknownClient = 'registered-client'; + + try { + $this->authenticator($server, storage: $storage)->finishAuthorization($callback); + $this->fail('Expected invalid_client.'); + } catch (OAuthErrorException $e) { + $this->assertSame('invalid_client', $e->error); + } + + $this->assertSame(['get' => true, 'forget' => true], $storage->synchronized); + $this->assertNull($storage->getClientRegistration('https://auth.example.com')); + } + #[TestDox('a registration the authorization server forgot is dropped when the callback is refused')] public function testDeferredAuthorizationForgetsARefusedRegistration(): void {