diff --git a/CHANGELOG.md b/CHANGELOG.md index aaf2c89f4..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. diff --git a/docs/client/authorization.md b/docs/client/authorization.md index 41d3aaad5..edc4fc8f9 100644 --- a/docs/client/authorization.md +++ b/docs/client/authorization.md @@ -89,6 +89,63 @@ The file contains tokens and client secrets. The SDK creates it with the mode `0 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. +## Finishing the Authorization in a Later Request + +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; + +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')) + ->build(); +} +``` + +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 +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 If an administrator created a client for you, or if the authorization server doesn't support dynamic registration, pass the client ID: @@ -258,4 +315,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. 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/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/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/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/src/Client/Auth/OAuthAuthenticator.php b/src/Client/Auth/OAuthAuthenticator.php index 74abf7b7d..0e1ca2a07 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,82 @@ public function handleChallenge(RequestInterface $request, ResponseInterface $re return true; } + /** + * 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 + { + 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.'); + } + + // 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.'); + } + + $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)); + } + + // 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)); + } + + $this->token = null; + $this->audience = null; + $this->requestedScopes = []; + $this->issuer = $metadata->issuer; + $this->resource = $pending->resource; + $this->offlineAccessRefused = $pending->offlineAccessRefused; + $scopes = $pending->scopes; + + try { + $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, + ])); + } catch (AuthorizationException $e) { + if (self::isForgottenClient($e, $client)) { + $this->forgetRefusedClient($metadata->issuer, $client); + + throw $e; + } + + $scopes = $this->reduceScopes($scopes, $e); + $token = $this->grant($metadata, $client, $scopes, $pending->resource); + } + + $this->token = $token->withGrantedScopes($scopes); + $this->requestedScopes = $scopes; + $this->audience = $pending->resource; + $this->storage->saveToken($metadata->issuer, $pending->resource, $this->token); + } + /** * Find the resource and the server, on every challenge as a resource may move. * @@ -452,17 +529,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 +603,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: $state, + codeVerifier: $pkce->verifier, + redirectUri: $this->configuration->redirectUri, + resource: $resource, + scopes: $scopes, + metadata: $metadata, + clientId: $client->clientId, + expiresAt: time() + 600, + offlineAccessRefused: $this->offlineAccessRefused, + registration: $client->dynamic ? $client : null, + )); + + throw $e; + } $this->verifyCallback($callback, $state, $metadata); diff --git a/src/Client/Auth/PendingAuthorization.php b/src/Client/Auth/PendingAuthorization.php new file mode 100644 index 000000000..78126bc18 --- /dev/null +++ b/src/Client/Auth/PendingAuthorization.php @@ -0,0 +1,101 @@ + + */ + public function jsonSerialize(): array + { + return [ + 'state' => $this->state, + 'code_verifier' => $this->codeVerifier, + 'redirect_uri' => $this->redirectUri, + 'resource' => $this->resource, + 'scopes' => $this->scopes, + 'metadata' => $this->metadata->jsonSerialize(), + 'client_id' => $this->clientId, + 'expires_at' => $this->expiresAt, + 'offline_access_refused' => $this->offlineAccessRefused, + 'registration' => $this->registration?->jsonSerialize(), + ]; + } + + /** + * @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', '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)) { + 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) { + if (!\is_string($scope)) { + throw new InvalidArgumentException('Invalid pending authorization scope.'); + } + + $scopes[] = $scope; + } + + return new self( + 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), + registration: \is_array($data['registration'] ?? null) ? ClientRegistration::fromArray($data['registration']) : null, + ); + } +} 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 @@ +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/src/Exception/AuthorizationPendingException.php b/src/Exception/AuthorizationPendingException.php new file mode 100644 index 000000000..ad3ffd82a --- /dev/null +++ b/src/Exception/AuthorizationPendingException.php @@ -0,0 +1,23 @@ +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')] @@ -80,7 +78,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(); } @@ -200,6 +198,48 @@ 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'; + $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, \JSON_THROW_ON_ERROR)]; + $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, \JSON_THROW_ON_ERROR)); + $client->disconnect(); + $this->assertSame([], json_decode((string) 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() @@ -275,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/Integration/Fixture/oauth/finish-authorization.php b/tests/Integration/Fixture/oauth/finish-authorization.php new file mode 100644 index 000000000..6c3c61c9a --- /dev/null +++ b/tests/Integration/Fixture/oauth/finish-authorization.php @@ -0,0 +1,24 @@ +setAuthorizationHandler(new RedirectAuthorizationHandler()) + ->setCredentialStorage(new FileCredentialStorage($_SERVER['argv'][1])) + ->build(); +$auth->finishAuthorization(json_decode($_SERVER['argv'][2], true, flags: \JSON_THROW_ON_ERROR)); + +echo 'authorized'; 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 { diff --git a/tests/Unit/Client/Auth/FileCredentialStorageTest.php b/tests/Unit/Client/Auth/FileCredentialStorageTest.php index b5df96942..747f51e07 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,66 @@ 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); + $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); + } + + #[TestDox('forgetting an authorization server drops only its pending authorizations')] + 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')); + } + + #[TestDox('a malformed pending authorization is discarded')] + public function testMalformedPendingAuthorizationIsDiscarded(): void + { + $storage = new FileCredentialStorage($this->path); + $storage->savePendingAuthorization($this->pending('one')); + $data = json_decode((string) 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: $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')] public function testToleratesACorruptedFile(): void { diff --git a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php index 8935e2366..2d1e65e0a 100644 --- a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php +++ b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php @@ -17,14 +17,19 @@ 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\PendingAuthorizationStorageInterface; +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 +553,404 @@ 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-'); + $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((string) 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((string) file_get_contents($path), true)['pending']); + } finally { + $this->removeStorage($path); + } + } + + #[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']); + $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']]; + } + + #[TestDox('a callback finishes its authorization only once')] + 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); + } + + #[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)); + + $this->expectException(AuthorizationException::class); + $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(); + $callback = $this->startPending($server, new InMemoryCredentialStorage()); + + $this->expectException(AuthorizationException::class); + $this->authenticator($server, storage: new InMemoryCredentialStorage())->finishAuthorization($callback); + } + + #[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); + $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]; + } + + #[TestDox('a handler that returns the callback still reports a refusal')] + 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()); + } + + #[TestDox('a storage without pending authorizations still works with a handler that returns the callback')] + public function testSynchronousAuthorizationSupportsExistingCredentialStorage(): void + { + $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); + $this->assertTrue($authenticator->handleChallenge($this->request(), $server->challenge())); + $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(); + $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'); + $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 { + $this->removeStorage($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 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 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 + { + $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 + */ + 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']; + } + + 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 */ 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 { 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