diff --git a/CHANGELOG.md b/CHANGELOG.md index ea51e742..ec77cdb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -95,7 +95,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * Fix the stdio client hanging on Windows while the server stays silent, without enforcing its timeouts: `StdioTransport` now reads only once `stream_select()` reports output. Requires PHP 8.5, as `stream_select()` on Windows reports an idle pipe as readable before that. * [BC Break] Remove JSON-RPC batch support: a JSON array from the server is dropped with a warning. Batching was removed from MCP in `2025-06-18`. * [BC Break] Fix `HttpTransport` reading a response body that is not an SSE stream without a limit, which could exhaust client memory on a large body. It now reads up to the new `maxResponseBodyBytes` argument (default: 8 MiB): a successful response over that limit fails the request, and an error status keeps its body cut at the limit. Raise the limit if your server sends larger results. -* [BC Break] `HttpTransport` turns off redirects on a discovered `symfony/http-client` client and treats every `3xx` answer as an error that names the status and target. An endpoint that is reached through a redirect stops working, so configure its final URL. A client you pass must not follow redirects to another origin. +* [BC Break] `HttpTransport` turns off redirects on a `symfony/http-client` client, discovered or passed, and treats every `3xx` answer as an error that names the status and target. Before, a redirect to another origin received the configured headers, such as an API key, and on a `307` or `308` the request body. If an endpoint of yours may have redirected to a host you don't control, rotate those credentials. An endpoint that is reached through a redirect stops working, so configure its final URL. A client you pass must not follow redirects to another origin. * Fix `HttpTransport` taking an `Mcp-Session-Id` from any response: it replaced the session mid-way and, on `2026-07-28`, made `getSessionId()` return a value and `close()` send a `DELETE`. Only the answer to `initialize` sets it. * Add `OAuth::forServiceAccount()` for the client credentials grant, `OAuth::setPrivateKeyJwt()` for `private_key_jwt` client authentication and `OAuth::setCrossAppAccess()` to reach a server through an enterprise identity. * Add `OAuth::setClientMetadataUrl()`, to use a client ID metadata document where the authorization server supports it, and `ConsoleAuthorizationHandler` for a shell without a browser. diff --git a/docs/client/authorization.md b/docs/client/authorization.md index edc4fc8f..f3a056c2 100644 --- a/docs/client/authorization.md +++ b/docs/client/authorization.md @@ -89,6 +89,8 @@ 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. +Tokens are also stored per resource, grant and client ID. So you can share one storage between a service account and an application for users, or between clients with different client IDs. None of them uses a token of another. Each token also records the client it was issued to, and a client never uses or refreshes a token of another, e.g. after a dynamic registration was replaced. A storage of your own must keep that client ID, which `AccessToken::jsonSerialize()` includes. + ## 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: @@ -246,7 +248,7 @@ OAuth::forApplication('My App')->setScopes('mcp:read', 'mcp:write'); If the server answers with `insufficient_scope`, the client adds the scopes of this challenge and authorizes again. It doesn't authorize again for scopes that it already requested. With any other `403`, the client doesn't authorize again. -If the authorization server supports `offline_access`, the client requests it to get a refresh token. If the server refuses this scope, the client tries again without it. Use `setOfflineAccess(false)` to never request this scope. The server can still issue a refresh token, and the client then uses it. +If the authorization server supports `offline_access`, the client requests it to get a refresh token. If the server refuses this scope, the client tries again without it. Use `setOfflineAccess(false)` to never request this scope. The server can still issue a refresh token, and the client then uses it. It refreshes a token 30 seconds before it expires, so the server doesn't have to reject a request first. A token that lives shorter than that is only refreshed once the server rejects it. ## Connecting to Older Servers @@ -272,11 +274,13 @@ The client treats the MCP server and its metadata as untrusted input. It stops w | Input | Reason | | --- | --- | -| The protected resource metadata names a `resource` that doesn't cover the endpoint | The token would be issued for another server | +| No location of the protected resource metadata names a `resource` that covers the endpoint | The token would be issued for another server | | The authorization server metadata names another `issuer` than the one it was fetched for | The document describes another server | | The MCP server URL, an authorization server URL or an endpoint doesn't use `https` on a host that isn't loopback | Credentials would travel without encryption | | One of these URLs names `0.0.0.0`, `::`, an IPv4 address within IPv6 or an IPv4 address in another form than four decimal numbers (e.g. `127.1`), or writes its host with percent-encoding or non-ASCII characters | The request could reach a local service or a private network without looking like it | -| A server that isn't on loopback names a loopback authorization server or endpoint, unless `setExpectedIssuer()` allows that issuer | It could reach services that only listen on this machine | +| A server that isn't on loopback names a loopback authorization server, unless `setExpectedIssuer()` allows that issuer, or an authorization server that isn't on loopback names a loopback endpoint, even with `setExpectedIssuer()` | It could reach services that only listen on this machine | +| An MCP server whose URL isn't a loopback or private-network IP address names an IP address of a private or link-local network (e.g. `10.0.0.5` or `169.254.169.254`) as authorization server, unless `setExpectedIssuer()` allows that issuer. A DNS name counts as public, so an MCP server at `https://mcp.corp.internal` with its authorization server at `https://10.0.0.5` needs `setExpectedIssuer()` | It could reach services of the network the client runs in | +| An authorization server whose URL isn't a loopback or private-network IP address names such an address as endpoint, even with `setExpectedIssuer()` | The same | | The authorization server metadata doesn't list `S256` in `code_challenge_methods_supported` | The client can't use PKCE with this server | | The authorization endpoint is on another origin than the issuer and the metadata doesn't announce `iss`, without `setExpectedIssuer()` | A response could come from another server, and the code would go to this one | | The authorization response has no `state`, or another `state` | It doesn't answer the request of this client | @@ -286,10 +290,10 @@ The client treats the MCP server and its metadata as untrusted input. It stops w | With `setLegacyDiscovery(true)`, a metadata location answers anything but a `404` and gives no usable metadata | The client only uses the fixed endpoints after a `404` | | The server has no protected resource metadata, without `setLegacyDiscovery(true)` | There is nothing to check the authorization server against | -These checks only look at how a URL writes its host. A DNS name that points at loopback or a private network still gets through. If you run the client on a server, block requests to private networks in your HTTP client or proxy (e.g. with Symfony's `NoPrivateNetworkHttpClient`) and pass that client to `setHttpClient()`. Pass it to `HeadlessAuthorizationHandler` as well, which sends its own request to the authorization endpoint. +These checks only look at how a URL writes its host. A DNS name that points at loopback or a private network still gets through, as the client doesn't resolve names. If you run the client on a server, block requests to private networks in your HTTP client or proxy (e.g. with Symfony's `NoPrivateNetworkHttpClient`) and pass that client to `setHttpClient()`. Pass it to `HeadlessAuthorizationHandler` as well, which sends its own request to the authorization endpoint. !!! warning - A client that you pass to `setHttpClient()` *must not* follow redirects. The client only turns them off on a `Psr18Client` of `symfony/http-client` 6.2 or later. With redirects on, an authorization server can send the authorization code and the client credentials to any address. + A client that you pass to `setHttpClient()` *must not* follow redirects. The client only turns them off on a `Psr18Client` of `symfony/http-client` 6.2 or later. Guzzle doesn't follow redirects through PSR-18, but an HTTPlug adapter around a Guzzle client with the default handler stack, HTTPlug's `RedirectPlugin` and Symfony's `HttplugClient` do. With redirects on, an authorization server can send the authorization code and the client credentials to any address. The client ignores a `resource_metadata` URL on another origin than the MCP server and looks at the well-known URLs instead. It only sends a token to the resource it was issued for. diff --git a/docs/client/transports.md b/docs/client/transports.md index fd38da2d..14403254 100644 --- a/docs/client/transports.md +++ b/docs/client/transports.md @@ -145,15 +145,19 @@ If the server answers a request with a `3xx` status, the request fails at once w `ConnectionException` at once, with no retry and no fallback to the `initialize` handshake. To reach a server that moved, configure its new URL as `endpoint`. -The transport turns off redirects on a `symfony/http-client` client that it discovers. With `auth`, -it also turns them off on a `symfony/http-client` client that you pass as `httpClient`. Any other -client keeps its own configuration. +The transport turns off redirects on a `symfony/http-client` client, whether it discovers the client +or you pass it as `httpClient`. Any other client keeps its own configuration. Such a client *must not* follow redirects to another origin. If it does, your headers and the request body reach that origin, and the transport accepts its answer as the server's. With `auth`, it *must not* follow any redirect, because a redirect on the same origin also receives the token. -To pass a `symfony/http-client` client, create it with `max_redirects` set to `0`: +Guzzle doesn't follow redirects through PSR-18. An HTTPlug adapter around a Guzzle client with the +default handler stack (e.g. `php-http/guzzle7-adapter`), HTTPlug's `RedirectPlugin` and Symfony's +`HttplugClient` do. + +Before `symfony/http-client` 6.2, the transport can't change a client that you pass. Create it with +`max_redirects` set to `0`: ```php use Symfony\Component\HttpClient\HttpClient; diff --git a/src/Client/Auth/AccessToken.php b/src/Client/Auth/AccessToken.php index 3fa49270..dec3e2a2 100644 --- a/src/Client/Auth/AccessToken.php +++ b/src/Client/Auth/AccessToken.php @@ -31,6 +31,7 @@ final class AccessToken implements \JsonSerializable * @param ?int $expiresAt unix timestamp the token expires at, null when the server did not say * @param ?string $refreshToken the refresh token, when the grant produced one * @param string[] $scopes the scopes the authorization server actually granted + * @param ?string $clientId the client the token was issued to, null when unknown */ public function __construct( public readonly string $accessToken, @@ -38,6 +39,7 @@ public function __construct( public readonly ?int $expiresAt = null, public readonly ?string $refreshToken = null, public readonly array $scopes = [], + public readonly ?string $clientId = null, ) { if ('' === trim($accessToken)) { throw new InvalidArgumentException('The access token must not be empty.'); @@ -122,7 +124,20 @@ public function withGrantedScopes(array $scopes): self return $this; } - return new self($this->accessToken, $this->tokenType, $this->expiresAt, $this->refreshToken, $scopes); + return new self($this->accessToken, $this->tokenType, $this->expiresAt, $this->refreshToken, $scopes, $this->clientId); + } + + public function withClientId(string $clientId): self + { + return new self($this->accessToken, $this->tokenType, $this->expiresAt, $this->refreshToken, $this->scopes, $clientId); + } + + /** + * Whether the token may be used by this client: a token and its refresh token belong to the client they were issued to (RFC 6749 section 6). + */ + public function isIssuedTo(?string $clientId): bool + { + return null === $this->clientId || $this->clientId === $clientId; } public function getAuthorizationHeader(): string @@ -141,6 +156,7 @@ public function jsonSerialize(): array 'expires_at' => $this->expiresAt, 'refresh_token' => $this->refreshToken, 'scopes' => $this->scopes, + 'client_id' => $this->clientId, ], static fn (mixed $value): bool => null !== $value && [] !== $value); } @@ -155,6 +171,7 @@ public static function fromArray(array $data): self isset($data['expires_at']) && is_numeric($data['expires_at']) ? (int) $data['expires_at'] : null, \is_string($data['refresh_token'] ?? null) ? $data['refresh_token'] : null, array_values(array_filter((array) ($data['scopes'] ?? []), 'is_string')), + \is_string($data['client_id'] ?? null) ? $data['client_id'] : null, ); } diff --git a/src/Client/Auth/AuthenticatingHttpClient.php b/src/Client/Auth/AuthenticatingHttpClient.php index bbba8238..5aadb7be 100644 --- a/src/Client/Auth/AuthenticatingHttpClient.php +++ b/src/Client/Auth/AuthenticatingHttpClient.php @@ -17,6 +17,7 @@ use Psr\Http\Client\ClientInterface; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\StreamInterface; /** * A PSR-18 client that authenticates every request and retries the ones the @@ -63,7 +64,7 @@ public function sendUnchallenged(RequestInterface $request): ResponseInterface public function sendRequest(RequestInterface $request): ResponseInterface { $body = $request->getBody(); - $position = $body->isSeekable() ? $body->tell() : null; + $position = self::replayPosition($body); $sent = $this->authenticator->authenticate($request); $response = $this->client->sendRequest($sent); @@ -97,8 +98,13 @@ public function sendRequest(RequestInterface $request): ResponseInterface return $response; } + try { + $body->seek($position); + } catch (\RuntimeException) { + return $response; + } + $response->getBody()->close(); - $body->seek($position); $sent = $this->authenticator->authenticate($request); $response = $this->client->sendRequest($sent); @@ -106,4 +112,17 @@ public function sendRequest(RequestInterface $request): ResponseInterface return $response; } + + private static function replayPosition(StreamInterface $body): ?int + { + if (!$body->isSeekable()) { + return null; + } + + try { + return $body->tell(); + } catch (\RuntimeException) { + return null; + } + } } diff --git a/src/Client/Auth/AuthorizationServerMetadata.php b/src/Client/Auth/AuthorizationServerMetadata.php index 29561afd..0038e91d 100644 --- a/src/Client/Auth/AuthorizationServerMetadata.php +++ b/src/Client/Auth/AuthorizationServerMetadata.php @@ -74,6 +74,11 @@ public static function fromArray(array $data): self } } + /** @var string $issuer */ + $issuer = $data['issuer']; + $issuerOnLoopback = SecureUrl::isLoopback($issuer); + $issuerIsPublic = !$issuerOnLoopback && !SecureUrl::isPrivateNetwork($issuer); + foreach (['authorization_endpoint', 'token_endpoint', 'registration_endpoint'] as $endpoint) { if (!\is_string($data[$endpoint] ?? null)) { continue; @@ -84,8 +89,13 @@ public static function fromArray(array $data): self } // Only a server on this machine may send the client back to this machine. - if (SecureUrl::isLoopback($data[$endpoint]) && \is_string($data['issuer'] ?? null) && !SecureUrl::isLoopback($data['issuer'])) { - throw new InvalidArgumentException(\sprintf('The authorization server metadata of "%s" declares "%s" as "%s", which is a loopback address.', $data['issuer'], $endpoint, $data[$endpoint])); + if (!$issuerOnLoopback && SecureUrl::isLoopback($data[$endpoint])) { + throw new InvalidArgumentException(\sprintf('The authorization server metadata of "%s" declares "%s" as "%s", which is a loopback address.', $issuer, $endpoint, $data[$endpoint])); + } + + // Nor may a server outside a private network send the client into one. + if ($issuerIsPublic && SecureUrl::isPrivateNetwork($data[$endpoint])) { + throw new InvalidArgumentException(\sprintf('The authorization server metadata of "%s" declares "%s" as "%s", which is an address of a private network.', $issuer, $endpoint, $data[$endpoint])); } $parts = SecureUrl::parse($data[$endpoint], $endpoint); diff --git a/src/Client/Auth/CredentialStorageInterface.php b/src/Client/Auth/CredentialStorageInterface.php index 0799541c..2b1ee5b6 100644 --- a/src/Client/Auth/CredentialStorageInterface.php +++ b/src/Client/Auth/CredentialStorageInterface.php @@ -27,7 +27,7 @@ public function saveClientRegistration(string $issuer, ClientRegistration $regis /** * @param string $issuer the authorization server's issuer identifier - * @param string $resource the canonical URI of the protected resource the token may be sent to + * @param string $resource the canonical URI of the protected resource the token may be sent to, followed by `#`, the grant and, for a configured client, its client ID */ public function getToken(string $issuer, string $resource): ?AccessToken; diff --git a/src/Client/Auth/MetadataDiscovery.php b/src/Client/Auth/MetadataDiscovery.php index 9e47f8ba..084adeee 100644 --- a/src/Client/Auth/MetadataDiscovery.php +++ b/src/Client/Auth/MetadataDiscovery.php @@ -11,6 +11,7 @@ namespace Mcp\Client\Auth; +use Mcp\Exception\AuthorizationException; use Mcp\Exception\InvalidArgumentException; use Psr\Http\Client\ClientExceptionInterface; use Psr\Http\Client\ClientInterface; @@ -39,9 +40,15 @@ public function __construct( * * @param string $resourceUrl the MCP endpoint being called * @param ?string $metadataUrl the location the challenge named, which wins over any guess + * + * @return ?ProtectedResourceMetadata the first document that covers the endpoint, null if there is none + * + * @throws AuthorizationException if documents were found, but only for other resources */ public function discoverProtectedResource(string $resourceUrl, ?string $metadataUrl = null): ?ProtectedResourceMetadata { + $unrelated = null; + foreach (null !== $metadataUrl ? [$metadataUrl] : self::protectedResourceCandidates($resourceUrl) as $candidate) { if (!AuthorizationServerMetadata::isTransportSecure($candidate)) { $this->logger->error('Refusing protected resource discovery: the metadata URL is neither an HTTPS URL nor a loopback address.', ['url' => $candidate]); @@ -56,10 +63,27 @@ public function discoverProtectedResource(string $resourceUrl, ?string $metadata } try { - return ProtectedResourceMetadata::fromArray($document); + $metadata = ProtectedResourceMetadata::fromArray($document); } catch (InvalidArgumentException $e) { $this->logger->warning('Ignoring malformed protected resource metadata', ['url' => $candidate, 'exception' => $e]); + + continue; } + + // RFC 9728 section 3.3: the document must not be used, but a later location may describe this resource. + if (!$metadata->covers($resourceUrl)) { + $this->logger->warning('Ignoring protected resource metadata for another resource', ['url' => $candidate, 'resource' => $metadata->resource]); + $unrelated ??= $metadata; + + continue; + } + + return $metadata; + } + + if (null !== $unrelated) { + // A token for another resource would be sent to this one: a confused deputy. + throw new AuthorizationException(\sprintf('The protected resource metadata for "%s" declares the unrelated resource "%s"; refusing to authorize against it.', $resourceUrl, $unrelated->resource)); } return null; diff --git a/src/Client/Auth/OAuthAuthenticator.php b/src/Client/Auth/OAuthAuthenticator.php index 0e1ca2a0..5751292b 100644 --- a/src/Client/Auth/OAuthAuthenticator.php +++ b/src/Client/Auth/OAuthAuthenticator.php @@ -14,6 +14,7 @@ use Mcp\Exception\AuthorizationException; use Mcp\Exception\AuthorizationPendingException; use Mcp\Exception\OAuthErrorException; +use Mcp\Exception\RuntimeException; use Mcp\Exception\TokenEndpointUnavailableException; use Mcp\Server\Transport\Http\OAuth\SecureUrl; use Psr\Http\Message\RequestInterface; @@ -42,6 +43,13 @@ final class OAuthAuthenticator implements AuthenticatorInterface private bool $offlineAccessRefused = false; + /** Where the current token came from, so it can be refreshed before it expires. */ + private ?AuthorizationServerMetadata $metadata = null; + private ?ClientRegistration $client = null; + + /** Not refreshed before sending: a failed refresh would repeat on every request, and so would a token shorter-lived than the leeway. */ + private ?AccessToken $noRefreshBeforeSending = null; + public function __construct( private readonly OAuthConfiguration $configuration, private readonly CredentialStorageInterface $storage, @@ -55,7 +63,9 @@ public function __construct( public function authenticate(RequestInterface $request): RequestInterface { - if (null === $this->token || $this->token->isExpired()) { + $token = $this->token; + + if (null === $token) { return $request; } @@ -64,7 +74,15 @@ public function authenticate(RequestInterface $request): RequestInterface return $request; } - return $request->withHeader('Authorization', $this->token->getAuthorizationHeader()); + if ($token->isExpiringSoon()) { + $token = $this->refreshBeforeSending($token, $request, $this->audience); + } + + if ($token->isExpired()) { + return $request; + } + + return $request->withHeader('Authorization', $token->getAuthorizationHeader()); } public function handleChallenge(RequestInterface $request, ResponseInterface $response): bool @@ -102,7 +120,14 @@ public function handleChallenge(RequestInterface $request, ResponseInterface $re $this->issuer = $metadata->issuer; $this->resource = $resource; - $stored = $this->storage->getToken($metadata->issuer, $resource); + $stored = $this->storage->getToken($metadata->issuer, $this->tokenKey($resource)); + + // A dynamic registration may have been replaced since the token was issued, and its successor can't use it. + if (null !== $stored && !$stored->isIssuedTo($this->knownClient($metadata)?->clientId)) { + $this->logger->info('Ignoring a stored token of another client', ['issuer' => $metadata->issuer, 'resource' => $resource]); + $stored = null; + } + $scopes = $this->selectScopes($challenge, $scopesSupported, $metadata, $stored); // The resource never requires offline_access, which only gets a refresh token. $needed = array_values(array_diff($scopes, ['offline_access'])); @@ -120,6 +145,8 @@ public function handleChallenge(RequestInterface $request, ResponseInterface $re $this->logger->debug('Reusing a stored access token', ['issuer' => $metadata->issuer, 'resource' => $resource]); $this->token = $stored; $this->audience = $resource; + $this->metadata = $metadata; + $this->client = null; return true; } @@ -159,11 +186,14 @@ public function handleChallenge(RequestInterface $request, ResponseInterface $re $token = $token->withGrantedScopes($scopes); $this->requestedScopes = $scopes; - $this->storage->saveToken($metadata->issuer, $resource, $token); + $this->storage->saveToken($metadata->issuer, $this->tokenKey($resource), $token); } $this->token = $token; $this->audience = $resource; + $this->metadata = $metadata; + $this->client = $client; + $this->noRefreshBeforeSending = $token->isExpiringSoon() ? $token : null; return true; } @@ -212,6 +242,8 @@ public function finishAuthorization(array $callback): void $this->token = null; $this->audience = null; + $this->metadata = null; + $this->client = null; $this->requestedScopes = []; $this->issuer = $metadata->issuer; $this->resource = $pending->resource; @@ -241,7 +273,10 @@ public function finishAuthorization(array $callback): void $this->token = $token->withGrantedScopes($scopes); $this->requestedScopes = $scopes; $this->audience = $pending->resource; - $this->storage->saveToken($metadata->issuer, $pending->resource, $this->token); + $this->metadata = $metadata; + $this->client = $client; + $this->noRefreshBeforeSending = $this->token->isExpiringSoon() ? $this->token : null; + $this->storage->saveToken($metadata->issuer, $this->tokenKey($pending->resource), $this->token); } /** @@ -257,11 +292,6 @@ private function discover(string $endpoint, AuthorizationChallenge $challenge): return [$endpoint, $this->discoverLegacyServer($endpoint), null]; } - if (!$metadata->covers($endpoint)) { - // A token for another resource would be sent to this one: a confused deputy. - throw new AuthorizationException(\sprintf('The protected resource metadata for "%s" declares the unrelated resource "%s"; refusing to authorize against it.', $endpoint, $metadata->resource)); - } - $issuer = $this->selectIssuer($endpoint, $metadata->authorizationServers); // Checked before the fetch, as even the request is made on the server's behalf. @@ -274,6 +304,10 @@ private function discover(string $endpoint, AuthorizationChallenge $challenge): throw new AuthorizationException(\sprintf('The protected resource metadata for "%s" names "%s" as its authorization server, which is a loopback address. Call OAuth::setExpectedIssuer() if that is intended.', $endpoint, $issuer)); } + if (SecureUrl::isPrivateNetwork($issuer) && !SecureUrl::isPrivateNetwork($endpoint) && !SecureUrl::isLoopback($endpoint) && $issuer !== $this->configuration->expectedIssuer) { + throw new AuthorizationException(\sprintf('The protected resource metadata for "%s" names "%s" as its authorization server, which is an address of a private network. Call OAuth::setExpectedIssuer() if that is intended.', $endpoint, $issuer)); + } + $server = $this->discovery->discoverAuthorizationServer($issuer); if (null === $server) { @@ -422,7 +456,7 @@ private function isUsable(AccessToken $token, array $scopes, RequestInterface $r } /** - * Tokens are stored per server and resource, so a stored one may come from a broader configuration. + * Configurations with the same grant and client share stored tokens, so a stored one may come from a broader configuration. */ private function isWithinScopeLimit(AccessToken $token): bool { @@ -445,6 +479,43 @@ private function scopeLimit(): ?array return $this->configuration->scopes; } + /** + * Saves the round trip of a rejected request; on any failure the server's challenge starts the usual flow. + */ + private function refreshBeforeSending(AccessToken $token, RequestInterface $request, string $resource): AccessToken + { + if (null === $token->refreshToken || null === $this->metadata || $token === $this->noRefreshBeforeSending) { + return $token; + } + + try { + $client = $this->client ?? $this->knownClient($this->metadata); + + if (null === $client) { + return $token; + } + + $refreshed = $this->refresh($token, $this->metadata, $client, array_values(array_diff($token->scopes, ['offline_access'])), $resource, $request); + } catch (AuthorizationException|RuntimeException $e) { + $this->logger->info('Refreshing the access token before sending failed; leaving it to the server\'s challenge.', ['exception' => $e]); + $this->noRefreshBeforeSending = $token; + + return $token; + } + + if (null === $refreshed) { + $this->noRefreshBeforeSending = $token; + + return $token; + } + + $this->token = $refreshed; + $this->client = $client; + $this->noRefreshBeforeSending = $refreshed->isExpiringSoon() ? $refreshed : null; + + return $refreshed; + } + /** * Trade a refresh token for a new access token, one process at a time as a rotated refresh token can only be spent once. * @@ -463,9 +534,9 @@ private function refresh(?AccessToken $stored, AuthorizationServerMetadata $meta return $this->storage->synchronize(function () use ($metadata, $client, $scopes, $resource, $request): ?AccessToken { // Another process may have refreshed the token while this one waited. - $current = $this->storage->getToken($metadata->issuer, $resource); + $current = $this->storage->getToken($metadata->issuer, $this->tokenKey($resource)); - if (null === $current) { + if (null === $current || !$current->isIssuedTo($client->clientId)) { return null; } @@ -481,7 +552,7 @@ private function refresh(?AccessToken $stored, AuthorizationServerMetadata $meta $token = $this->spendRefreshToken($current, $current->refreshToken, $metadata, $client, $resource); if (null !== $token) { - $this->storage->saveToken($metadata->issuer, $resource, $token); + $this->storage->saveToken($metadata->issuer, $this->tokenKey($resource), $token); } return $token; @@ -514,6 +585,7 @@ private function spendRefreshToken(AccessToken $stored, string $refreshToken, Au $token->expiresAt, $token->refreshToken ?? $refreshToken, [] === $token->scopes ? $stored->scopes : $token->scopes, + $token->clientId, ); } @@ -642,7 +714,7 @@ private function grant(AuthorizationServerMetadata $metadata, ClientRegistration */ private function requestToken(AuthorizationServerMetadata $metadata, ClientRegistration $client, array $parameters): AccessToken { - return $this->tokenEndpoint->request($metadata, $client, $parameters, $this->configuration->privateKeyPem, $this->configuration->signingAlgorithm); + return $this->tokenEndpoint->request($metadata, $client, $parameters, $this->configuration->privateKeyPem, $this->configuration->signingAlgorithm)->withClientId($client->clientId); } /** @@ -721,24 +793,10 @@ private function verifyCallback(array $callback, string $state, AuthorizationSer private function resolveClient(AuthorizationServerMetadata $metadata, array $scopes, ?array $scopesSupported): ClientRegistration { $configuration = $this->configuration; + $client = $this->configuredClient($metadata); - if (null !== $configuration->clientId) { - return new ClientRegistration( - $configuration->clientId, - $configuration->clientSecret, - TokenEndpointAuthMethod::negotiate( - $metadata->tokenEndpointAuthMethodsSupported, - null !== $configuration->clientSecret, - null !== $configuration->privateKeyPem, - Grant::ClientCredentials !== $configuration->grant, - ), - ); - } - - if ($metadata->clientIdMetadataDocumentSupported && null !== $configuration->clientMetadataUrl) { - $this->logger->debug('Using the client metadata document URL as the client id', ['client_id' => $configuration->clientMetadataUrl]); - - return new ClientRegistration($configuration->clientMetadataUrl, null, TokenEndpointAuthMethod::negotiate($metadata->tokenEndpointAuthMethodsSupported, false)); + if (null !== $client) { + return $client; } // Another process may register at the same time, and its tokens would not work with this client. @@ -770,6 +828,61 @@ private function resolveClient(AuthorizationServerMetadata $metadata, array $sco }); } + /** + * Where a token is stored: configurations sharing a storage must not take over each other's tokens. + */ + private function tokenKey(string $resource): string + { + $client = $this->configuration->clientId ?? $this->configuration->clientMetadataUrl; + + return $resource.'#'.$this->configuration->grant->value.(null === $client ? '' : ' '.$client); + } + + /** + * The client identity that needs no registration: a configured client or a client metadata document. + */ + private function configuredClient(AuthorizationServerMetadata $metadata): ?ClientRegistration + { + $configuration = $this->configuration; + + if (null !== $configuration->clientId) { + return new ClientRegistration( + $configuration->clientId, + $configuration->clientSecret, + TokenEndpointAuthMethod::negotiate( + $metadata->tokenEndpointAuthMethodsSupported, + null !== $configuration->clientSecret, + null !== $configuration->privateKeyPem, + Grant::ClientCredentials !== $configuration->grant, + ), + ); + } + + if ($metadata->clientIdMetadataDocumentSupported && null !== $configuration->clientMetadataUrl) { + $this->logger->debug('Using the client metadata document URL as the client id', ['client_id' => $configuration->clientMetadataUrl]); + + return new ClientRegistration($configuration->clientMetadataUrl, null, TokenEndpointAuthMethod::negotiate($metadata->tokenEndpointAuthMethodsSupported, false)); + } + + return null; + } + + /** + * The client identity in use, without registering one. + */ + private function knownClient(AuthorizationServerMetadata $metadata): ?ClientRegistration + { + $client = $this->configuredClient($metadata); + + if (null !== $client) { + return $client; + } + + $stored = $this->storage->getClientRegistration($metadata->issuer); + + return null !== $stored && !$stored->isExpired() && $this->matchesRedirectUris($stored) ? $stored : null; + } + /** * @param array $parameters * diff --git a/src/Client/Auth/PendingAuthorization.php b/src/Client/Auth/PendingAuthorization.php index 78126bc1..3c1e5f82 100644 --- a/src/Client/Auth/PendingAuthorization.php +++ b/src/Client/Auth/PendingAuthorization.php @@ -14,7 +14,7 @@ use Mcp\Exception\InvalidArgumentException; /** - * @author PHP MCP SDK contributors + * @author bbdu3 */ final class PendingAuthorization implements \JsonSerializable { diff --git a/src/Client/Auth/PendingAuthorizationStorageInterface.php b/src/Client/Auth/PendingAuthorizationStorageInterface.php index 6e110c1b..9d6e1585 100644 --- a/src/Client/Auth/PendingAuthorizationStorageInterface.php +++ b/src/Client/Auth/PendingAuthorizationStorageInterface.php @@ -14,7 +14,7 @@ /** * Storage must be scoped to the user session that initiated authorization. * - * @author PHP MCP SDK contributors + * @author bbdu3 */ interface PendingAuthorizationStorageInterface extends CredentialStorageInterface { diff --git a/src/Client/Auth/RedirectAuthorizationHandler.php b/src/Client/Auth/RedirectAuthorizationHandler.php index 130cd9e3..b511d63b 100644 --- a/src/Client/Auth/RedirectAuthorizationHandler.php +++ b/src/Client/Auth/RedirectAuthorizationHandler.php @@ -14,7 +14,7 @@ use Mcp\Exception\AuthorizationPendingException; /** - * @author PHP MCP SDK contributors + * @author bbdu3 */ final class RedirectAuthorizationHandler implements AuthorizationHandlerInterface { diff --git a/src/Client/Transport/HttpTransport.php b/src/Client/Transport/HttpTransport.php index 5c4c36a2..a3d777a1 100644 --- a/src/Client/Transport/HttpTransport.php +++ b/src/Client/Transport/HttpTransport.php @@ -160,7 +160,7 @@ public function __construct( $this->listenParser = new SseParser($maxSseBufferBytes); $this->reconnectPolicy = $reconnectPolicy ?? new ReconnectPolicy(); $this->clock = $clock ?? new MonotonicClock(); - $client = $httpClient ?? NoRedirects::discover(); + $client = null === $httpClient ? NoRedirects::discover() : NoRedirects::apply($httpClient); $this->httpClient = null === $auth ? $client : new AuthenticatingHttpClient($client, $auth); $this->requestFactory = $requestFactory ?? Psr17FactoryDiscovery::findRequestFactory(); $this->streamFactory = $streamFactory ?? Psr17FactoryDiscovery::findStreamFactory(); @@ -956,6 +956,10 @@ private function processSseReconnect(): void try { $response = $this->httpClient->sendRequest($request); + } catch (AuthorizationPendingException $e) { + $this->failSseResumption($e); + + return; } catch (AuthorizationException $e) { $this->handleError($e); $this->failSseResumption($e); diff --git a/src/Exception/AuthorizationPendingException.php b/src/Exception/AuthorizationPendingException.php index ad3ffd82..2c96beaa 100644 --- a/src/Exception/AuthorizationPendingException.php +++ b/src/Exception/AuthorizationPendingException.php @@ -12,7 +12,7 @@ namespace Mcp\Exception; /** - * @author PHP MCP SDK contributors + * @author bbdu3 */ final class AuthorizationPendingException extends AuthorizationException { diff --git a/src/Server/Fiber/FiberContextInterface.php b/src/Server/Fiber/FiberContextInterface.php index 0b1c884a..6af30500 100644 --- a/src/Server/Fiber/FiberContextInterface.php +++ b/src/Server/Fiber/FiberContextInterface.php @@ -14,7 +14,7 @@ /** * Carries execution context, e.g. the active OpenTelemetry context, into the fibers request handlers run in. * - * @author bbdu3 + * @author bbdu3 */ interface FiberContextInterface { diff --git a/src/Server/Transport/Http/OAuth/SecureUrl.php b/src/Server/Transport/Http/OAuth/SecureUrl.php index df7b5676..bfd7949b 100644 --- a/src/Server/Transport/Http/OAuth/SecureUrl.php +++ b/src/Server/Transport/Http/OAuth/SecureUrl.php @@ -25,6 +25,9 @@ */ final class SecureUrl { + /** @var list */ + private const PRIVATE_IPV4 = [['10.0.0.0', 8], ['172.16.0.0', 12], ['192.168.0.0', 16], ['169.254.0.0', 16], ['100.64.0.0', 10]]; + /** * @return array{scheme: string, host: string, port?: int, path?: string, query?: string, fragment?: string} * @@ -66,6 +69,65 @@ public static function isLoopback(string $url): bool return \is_string($host) && self::isLoopbackHost($host); } + /** + * Whether a URL names an address of a private or link-local network as an IP literal; a name is not resolved. + */ + public static function isPrivateNetwork(string $url): bool + { + $host = parse_url($url, \PHP_URL_HOST); + $host = \is_string($host) ? trim($host, '[]') : ''; + + if (false === filter_var($host, \FILTER_VALIDATE_IP)) { + return false; + } + + $address = (string) inet_pton($host); + + if (4 === \strlen($address)) { + return self::isWithin($address, self::PRIVATE_IPV4); + } + + if (self::isWithin($address, [['64:ff9b:1::', 48]])) { + return true; + } + + // NAT64 (RFC 6052) and 6to4 (RFC 3056) addresses reach the IPv4 address they carry, loopback included. + $embedded = match (true) { + self::isWithin($address, [['64:ff9b::', 96]]) => substr($address, 12), + self::isWithin($address, [['2002::', 16]]) => substr($address, 2, 4), + default => null, + }; + + if (null !== $embedded) { + return self::isWithin($embedded, [...self::PRIVATE_IPV4, ['127.0.0.0', 8], ['0.0.0.0', 8]]); + } + + return self::isWithin($address, [['fc00::', 7], ['fe80::', 10], ['fec0::', 10]]); + } + + /** + * @param list $ranges + */ + private static function isWithin(string $address, array $ranges): bool + { + foreach ($ranges as [$network, $bits]) { + $prefix = (string) inet_pton($network); + + if (\strlen($prefix) !== \strlen($address)) { + continue; + } + + $bytes = intdiv($bits, 8); + $mask = 0xFF << (8 - $bits % 8) & 0xFF; + + if (0 === substr_compare($address, $prefix, 0, $bytes) && (0 === $bits % 8 || (\ord($address[$bytes]) & $mask) === \ord($prefix[$bytes]))) { + return true; + } + } + + return false; + } + // Each of these can reach this machine or a private network without looking like it. private static function hidesItsAddress(string $host): bool { diff --git a/tests/Unit/Client/Auth/AccessTokenTest.php b/tests/Unit/Client/Auth/AccessTokenTest.php index aa27b540..412171cd 100644 --- a/tests/Unit/Client/Auth/AccessTokenTest.php +++ b/tests/Unit/Client/Auth/AccessTokenTest.php @@ -146,7 +146,7 @@ public function testWithGrantedScopes(): void #[TestDox('a token survives a round trip through storage')] public function testRoundTrip(): void { - $token = new AccessToken('abc', 'Bearer', 4600, 'refresh', ['mcp:read']); + $token = new AccessToken('abc', 'Bearer', 4600, 'refresh', ['mcp:read'], 'client-1'); $restored = AccessToken::fromArray($token->jsonSerialize()); $this->assertEquals($token, $restored); diff --git a/tests/Unit/Client/Auth/AuthenticatingHttpClientTest.php b/tests/Unit/Client/Auth/AuthenticatingHttpClientTest.php index 974081f1..25a0d9f1 100644 --- a/tests/Unit/Client/Auth/AuthenticatingHttpClientTest.php +++ b/tests/Unit/Client/Auth/AuthenticatingHttpClientTest.php @@ -200,8 +200,8 @@ public function testFailedChallengeIsAClientException(): void ->sendRequest((new Psr17Factory())->createRequest('POST', 'https://mcp.example.com/mcp')); } - #[TestDox('a body that fails to seek leaves no rejected response open')] - public function testClosesTheRejectedResponseWhenTheBodyCannotSeek(): void + #[TestDox('a body that fails to seek ends the exchange with the rejected response, inside the PSR-18 contract')] + public function testReturnsTheRejectedResponseWhenTheBodyCannotSeek(): void { $rejected = new Response(401, [], 'body'); $inner = $this->createStub(ClientInterface::class); @@ -212,15 +212,29 @@ public function testClosesTheRejectedResponseWhenTheBodyCannotSeek(): void $body->method('tell')->willReturn(0); $body->method('seek')->willThrowException(new \RuntimeException('Unable to seek.')); - try { - (new AuthenticatingHttpClient($inner, new CountingAuthenticator())) - ->sendRequest((new Psr17Factory())->createRequest('POST', 'https://mcp.example.com/mcp')->withBody($body)); - $this->fail('The seek failure should have reached the caller.'); - } catch (\RuntimeException $e) { - $this->assertSame('Unable to seek.', $e->getMessage()); - } + $response = (new AuthenticatingHttpClient($inner, new CountingAuthenticator())) + ->sendRequest((new Psr17Factory())->createRequest('POST', 'https://mcp.example.com/mcp')->withBody($body)); - $this->assertFalse($rejected->getBody()->isReadable()); + $this->assertSame($rejected, $response); + $this->assertTrue($response->getBody()->isReadable()); + } + + #[TestDox('a body that fails to tell its position is not replayed')] + public function testDoesNotRetryABodyThatCannotTell(): void + { + $body = $this->createStub(StreamInterface::class); + $body->method('isSeekable')->willReturn(true); + $body->method('tell')->willThrowException(new \RuntimeException('Unable to tell.')); + + $inner = new RejectingClient(1); + $authenticator = new CountingAuthenticator(); + + $response = (new AuthenticatingHttpClient($inner, $authenticator)) + ->sendRequest((new Psr17Factory())->createRequest('POST', 'https://mcp.example.com/mcp')->withBody($body)); + + $this->assertSame(401, $response->getStatusCode()); + $this->assertSame(1, $inner->calls); + $this->assertSame(1, $authenticator->challenges); } #[TestDox('a body that cannot be replayed ends the exchange after the challenge')] diff --git a/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php b/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php index a8575f31..dfd14a19 100644 --- a/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php +++ b/tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php @@ -156,6 +156,35 @@ public function testAcceptsLoopbackEndpointsOfALoopbackServer(): void $this->assertSame('http://localhost:9000/token', $metadata->tokenEndpoint); } + #[TestWith(['token_endpoint'])] + #[TestWith(['authorization_endpoint'])] + #[TestWith(['registration_endpoint'])] + #[TestDox('a remote authorization server can not name an endpoint in a private network')] + public function testRejectsAPrivateNetworkEndpointOfARemoteServer(string $endpoint): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('private network'); + + AuthorizationServerMetadata::fromArray([ + 'issuer' => 'https://auth.example.com', + 'authorization_endpoint' => 'https://auth.example.com/authorize', + 'token_endpoint' => 'https://auth.example.com/token', + $endpoint => 'https://169.254.169.254/x', + ]); + } + + #[TestDox('an authorization server in a private network can name endpoints there')] + public function testAcceptsPrivateNetworkEndpointsOfAPrivateServer(): void + { + $metadata = AuthorizationServerMetadata::fromArray([ + 'issuer' => 'https://10.0.0.5', + 'authorization_endpoint' => 'https://10.0.0.5/authorize', + 'token_endpoint' => 'https://10.0.0.6/token', + ]); + + $this->assertSame('https://10.0.0.6/token', $metadata->tokenEndpoint); + } + #[TestDox('an unstated grant list means the authorization code grant, and nothing else')] public function testDefaultGrantTypes(): void { diff --git a/tests/Unit/Client/Auth/MetadataDiscoveryTest.php b/tests/Unit/Client/Auth/MetadataDiscoveryTest.php index a783a274..1781a578 100644 --- a/tests/Unit/Client/Auth/MetadataDiscoveryTest.php +++ b/tests/Unit/Client/Auth/MetadataDiscoveryTest.php @@ -12,6 +12,7 @@ namespace Mcp\Tests\Unit\Client\Auth; use Mcp\Client\Auth\MetadataDiscovery; +use Mcp\Exception\AuthorizationException; use Nyholm\Psr7\Factory\Psr17Factory; use Nyholm\Psr7\Response; use PHPUnit\Framework\Attributes\DataProvider; @@ -307,6 +308,35 @@ public function testHonoursTheChallengeMetadataUrl(): void $this->assertSame(['https://mcp.example.com/custom/location.json'], $client->requested); } + #[TestDox('a document for another resource does not stop a later location')] + public function testContinuesAfterAnUnrelatedResource(): void + { + $path = 'https://mcp.example.com/.well-known/oauth-protected-resource/mcp'; + $root = 'https://mcp.example.com/.well-known/oauth-protected-resource'; + $client = new RecordingClient([ + $path => self::json(['resource' => 'https://mcp.example.com/other']), + $root => self::json(['resource' => 'https://mcp.example.com']), + ]); + + $metadata = $this->discovery($client)->discoverProtectedResource('https://mcp.example.com/mcp'); + + $this->assertSame('https://mcp.example.com', $metadata?->resource); + $this->assertSame([$path, $root], $client->requested); + } + + #[TestDox('documents only for other resources are refused, rather than taken as no metadata')] + public function testRefusesWhenOnlyUnrelatedResourcesAreFound(): void + { + $client = new RecordingClient([ + 'https://mcp.example.com/.well-known/oauth-protected-resource/mcp' => self::json(['resource' => 'https://evil.example.com/mcp']), + ]); + + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('declares the unrelated resource "https://evil.example.com/mcp"'); + + $this->discovery($client)->discoverProtectedResource('https://mcp.example.com/mcp'); + } + #[TestDox('an invalid resource identifier does not stop a later location')] public function testContinuesAfterAnInvalidResource(): void { diff --git a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php index 2d1e65e0..1043f1d2 100644 --- a/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php +++ b/tests/Unit/Client/Auth/OAuthAuthenticatorTest.php @@ -31,6 +31,7 @@ use Mcp\Exception\AuthorizationException; use Mcp\Exception\AuthorizationPendingException; use Mcp\Exception\OAuthErrorException; +use Mcp\Exception\RuntimeException; use Nyholm\Psr7\Factory\Psr17Factory; use Nyholm\Psr7\Response; use PHPUnit\Framework\Attributes\DataProvider; @@ -251,7 +252,7 @@ public function testRefreshesATokenWithoutOfflineAccess(): void { $server = new FakeOAuthServer(resourceScopes: ['mcp:read'], serverScopes: ['mcp:read', 'offline_access'], refreshTokens: true); $storage = new InMemoryCredentialStorage(); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('expired-token', 'Bearer', time() - 60, 'refresh-token', ['mcp:read'])); + $storage->saveToken('https://auth.example.com', self::tokenKey(), new AccessToken('expired-token', 'Bearer', time() - 60, 'refresh-token', ['mcp:read'])); $this->authenticator($server, storage: $storage)->handleChallenge($this->request(), $server->challenge()); @@ -582,7 +583,7 @@ public function testFinishesAuthorizationInAnotherRequest(): void $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); + $stored = (new FileCredentialStorage($path))->getToken('https://auth.example.com', self::tokenKey()); $this->assertSame(['mcp:read'], $stored?->scopes); $this->assertSame([], json_decode((string) file_get_contents($path), true)['pending']); } finally { @@ -605,7 +606,7 @@ public function testRejectsInvalidDeferredCallbacks(array $replacement): void } 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)); + $this->assertNull($storage->getToken('https://auth.example.com', self::tokenKey())); } } @@ -720,7 +721,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)); + $storage->expects($this->once())->method('saveToken')->with('https://auth.example.com', self::tokenKey(), $this->isInstanceOf(AccessToken::class)); $authenticator = $this->authenticator($server, storage: $storage); $this->assertTrue($authenticator->handleChallenge($this->request(), $server->challenge())); @@ -808,6 +809,53 @@ public function testDeferredAuthorizationKeepsItsClientAfterAReRegistration(): v $this->assertSame('newer-client', $storage->getClientRegistration('https://auth.example.com')?->clientId); } + #[TestDox('the token of a pending authorization finished after a re-registration is not used by the new client')] + public function testTheNewClientDoesNotUseTheTokenOfTheReplacedOne(): void + { + $server = new FakeOAuthServer(refreshTokens: true); + $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'])); + $this->authenticator($server, storage: $storage)->finishAuthorization($callback); + $this->assertSame('registered-client', $storage->getToken('https://auth.example.com', self::tokenKey())?->clientId); + $server->authorization = []; + + $authenticator = $this->authenticator($server, storage: $storage); + $authenticator->handleChallenge($this->request(), $server->challenge()); + + $this->assertNotSame([], $server->authorization); + $this->assertSame('newer-client', $server->token['client_id'] ?? null); + $this->assertSame('newer-client', $storage->getToken('https://auth.example.com', self::tokenKey())?->clientId); + } + + #[TestDox('a stored refresh token of another client is not spent')] + public function testDoesNotRefreshATokenOfAnotherRegistration(): void + { + $server = new FakeOAuthServer(refreshTokens: true); + $storage = new InMemoryCredentialStorage(); + $storage->saveClientRegistration('https://auth.example.com', new ClientRegistration('newer-client', 'newer-secret', TokenEndpointAuthMethod::ClientSecretPost, true, scopes: ['mcp:read', 'mcp:write'])); + $storage->saveToken('https://auth.example.com', self::tokenKey(), new AccessToken('expired', 'Bearer', time() - 60, 'older-refresh-token', [], 'older-client')); + + $this->authenticator($server, storage: $storage)->handleChallenge($this->request(), $server->challenge()); + + $this->assertSame('authorization_code', $server->token['grant_type'] ?? null); + $this->assertNotSame([], $server->authorization); + } + + #[TestDox('a stored token without a client is used as before')] + public function testUsesAStoredTokenOfAnUnknownClient(): void + { + $server = new FakeOAuthServer(); + $storage = new InMemoryCredentialStorage(); + $storage->saveToken('https://auth.example.com', self::tokenKey(), new AccessToken('stored', 'Bearer', time() + 3600)); + + $authenticator = $this->authenticator($server, storage: $storage); + $authenticator->handleChallenge($this->request(), $server->challenge()); + + $this->assertSame([], $server->authorization); + $this->assertSame('Bearer stored', $authenticator->authenticate($this->request())->getHeaderLine('Authorization')); + } + #[TestDox('a refused callback keeps the registration of another flow')] public function testDeferredAuthorizationKeepsANewerRegistrationOnInvalidClient(): void { @@ -980,6 +1028,40 @@ public function testRefusesALoopbackIssuerOfARemoteServer(string $issuer): void $this->assertSame([], array_filter($server->requested, static fn (string $url): bool => str_starts_with($url, $issuer))); } + #[TestWith(['https://169.254.169.254'])] + #[TestWith(['https://10.0.0.5'])] + #[TestWith(['https://[fd00::1]'])] + #[TestDox('a remote server can not make the client request an address of a private network')] + public function testRefusesAPrivateNetworkIssuerOfARemoteServer(string $issuer): void + { + $server = new FakeOAuthServer(authorizationServer: $issuer); + + try { + $this->authenticator($server)->handleChallenge($this->request(), $server->challenge()); + $this->fail('The authorization server in a private network was not refused.'); + } catch (AuthorizationException $e) { + $this->assertStringContainsString('private network', $e->getMessage()); + } + + $this->assertSame([], array_filter($server->requested, static fn (string $url): bool => str_starts_with($url, $issuer))); + } + + #[TestDox('an application can allow an authorization server in a private network with the expected issuer')] + public function testExpectedIssuerAllowsAPrivateNetworkIssuer(): void + { + $server = new FakeOAuthServer(authorizationServer: 'https://10.0.0.5'); + $configuration = OAuth::forApplication('test')->setExpectedIssuer('https://10.0.0.5'); + + try { + $this->authenticator($server, configuration: $configuration)->handleChallenge($this->request(), $server->challenge()); + $this->fail('The fake server has no metadata for this issuer.'); + } catch (AuthorizationException $e) { + $this->assertStringContainsString('No usable metadata', $e->getMessage()); + } + + $this->assertNotSame([], array_filter($server->requested, static fn (string $url): bool => str_contains($url, '10.0.0.5'))); + } + #[TestDox('another type of token from the identity provider is not presented as a grant')] public function testCrossAppAccessRefusesAnotherTokenType(): void { @@ -1140,12 +1222,12 @@ public function testRefreshKeepsTheStoredScopes(): void { $server = new FakeOAuthServer(refreshTokens: true); $storage = new InMemoryCredentialStorage(); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('expired', 'Bearer', time() - 60, 'a-refresh-token', ['mcp:read', 'mcp:write'])); + $storage->saveToken('https://auth.example.com', self::tokenKey(), new AccessToken('expired', 'Bearer', time() - 60, 'a-refresh-token', ['mcp:read', 'mcp:write'])); $this->authenticator($server, storage: $storage)->handleChallenge($this->request(), $server->challenge(['mcp:read'])); $this->assertSame('refresh_token', $server->token['grant_type'] ?? null); - $this->assertSame(['mcp:read', 'mcp:write'], $storage->getToken('https://auth.example.com', self::ENDPOINT)?->scopes); + $this->assertSame(['mcp:read', 'mcp:write'], $storage->getToken('https://auth.example.com', self::tokenKey())?->scopes); } #[TestDox('a refresh does not record scopes the authorization server did not grant')] @@ -1153,12 +1235,12 @@ public function testRefreshDoesNotInventScopes(): void { $server = new FakeOAuthServer(refreshTokens: true); $storage = new InMemoryCredentialStorage(); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('expired', 'Bearer', time() - 60, 'a-refresh-token')); + $storage->saveToken('https://auth.example.com', self::tokenKey(), new AccessToken('expired', 'Bearer', time() - 60, 'a-refresh-token')); $this->authenticator($server, storage: $storage)->handleChallenge($this->request(), $server->challenge(['admin'])); $this->assertSame('refresh_token', $server->token['grant_type'] ?? null); - $this->assertSame([], $storage->getToken('https://auth.example.com', self::ENDPOINT)?->scopes); + $this->assertSame([], $storage->getToken('https://auth.example.com', self::tokenKey())?->scopes); } #[TestDox('a new redirect URI gets a new registration, as the old one would be refused')] @@ -1194,7 +1276,7 @@ public function testRegistersAgainWhenTheClientIsUnknown(): void $server->unknownClient = 'forgotten-client'; $storage = new InMemoryCredentialStorage(); $storage->saveClientRegistration('https://auth.example.com', new ClientRegistration('forgotten-client', dynamic: true)); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('expired', 'Bearer', time() - 60, 'a-refresh-token')); + $storage->saveToken('https://auth.example.com', self::tokenKey(), new AccessToken('expired', 'Bearer', time() - 60, 'a-refresh-token')); $authenticator = $this->authenticator($server, storage: $storage); $authenticator->handleChallenge($this->request(), $server->challenge()); @@ -1236,7 +1318,7 @@ public function testRefreshKeepsAClientRegisteredByAnotherWorker(): void $server = new FakeOAuthServer(refreshTokens: true); $server->unknownClient = 'forgotten-client'; $storage = $this->storageWithAnotherWorkerRegistering(); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('expired', 'Bearer', time() - 60, 'a-refresh-token')); + $storage->saveToken('https://auth.example.com', self::tokenKey(), new AccessToken('expired', 'Bearer', time() - 60, 'a-refresh-token')); $this->authenticator($server, storage: $storage)->handleChallenge($this->request(), $server->challenge()); @@ -1285,7 +1367,7 @@ public function testConfiguredClientRefusedOnRefreshFailsFast(): void $server = new FakeOAuthServer(refreshTokens: true); $server->unknownClient = 'known-client'; $storage = new InMemoryCredentialStorage(); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('expired', 'Bearer', time() - 60, 'refresh')); + $storage->saveToken('https://auth.example.com', self::tokenKey(clientId: 'known-client'), new AccessToken('expired', 'Bearer', time() - 60, 'refresh')); $configuration = OAuth::forApplication('test')->setClientCredentials('known-client')->setExpectedIssuer('https://auth.example.com'); try { @@ -1297,7 +1379,7 @@ public function testConfiguredClientRefusedOnRefreshFailsFast(): void } $this->assertSame([], $server->authorization); - $this->assertNotNull($storage->getToken('https://auth.example.com', self::ENDPOINT)); + $this->assertNotNull($storage->getToken('https://auth.example.com', self::tokenKey(clientId: 'known-client'))); } #[TestDox('an error that only mentions a refused scope in its text does not send the user back')] @@ -1379,9 +1461,209 @@ public function authorize(string $authorizationUrl, string $redirectUri): array $this->assertSame('0', $server->token['code'] ?? null); } - /** - * @param ?string[] $grantTypes - */ + #[TestDox('a token that is about to expire is refreshed before the request is sent')] + public function testRefreshesBeforeSending(): void + { + $server = new FakeOAuthServer(refreshTokens: true); + $storage = new InMemoryCredentialStorage(); + $authenticator = $this->authenticator($server, storage: $storage); + $authenticator->handleChallenge($this->request(), $server->challenge()); + self::ageToken($authenticator, $storage); + + $request = $authenticator->authenticate($this->request()); + + $this->assertSame('refresh_token', $server->token['grant_type'] ?? null); + $this->assertSame('Bearer access-token-2', $request->getHeaderLine('Authorization')); + } + + #[TestDox('a refused refresh before sending keeps the token until it expires, and leaves the rest to the server\'s challenge')] + public function testRefusedRefreshBeforeSendingKeepsTheToken(): void + { + $server = new FakeOAuthServer(refreshTokens: true); + $storage = new InMemoryCredentialStorage(); + $authenticator = $this->authenticator($server, storage: $storage); + $authenticator->handleChallenge($this->request(), $server->challenge()); + self::ageToken($authenticator, $storage); + $server->refreshAnswer = new Response(400, [], (string) json_encode(['error' => 'invalid_client'])); + + $request = $authenticator->authenticate($this->request()); + + $this->assertSame('refresh_token', $server->token['grant_type'] ?? null); + $this->assertSame('Bearer access-token-1', $request->getHeaderLine('Authorization')); + } + + #[TestWith([400, 'invalid_grant'])] + #[TestWith([503, 'temporarily_unavailable'])] + #[TestDox('a failed refresh before sending is not tried again for the same token: $_dataName')] + public function testFailedRefreshBeforeSendingIsNotRepeated(int $status, string $error): void + { + $server = new FakeOAuthServer(refreshTokens: true); + $storage = new InMemoryCredentialStorage(); + $authenticator = $this->authenticator($server, storage: $storage); + $authenticator->handleChallenge($this->request(), $server->challenge()); + self::ageToken($authenticator, $storage); + $server->refreshAnswer = new Response($status, [], (string) json_encode(['error' => $error])); + $tokenRequests = \count(array_filter($server->requested, static fn (string $url): bool => str_ends_with($url, '/token'))); + + $authenticator->authenticate($this->request()); + $request = $authenticator->authenticate($this->request()); + + $this->assertCount($tokenRequests + 1, array_filter($server->requested, static fn (string $url): bool => str_ends_with($url, '/token'))); + $this->assertSame('Bearer access-token-1', $request->getHeaderLine('Authorization')); + } + + #[TestDox('a token that lives shorter than the leeway is refreshed before sending at most once')] + public function testShortLivedTokensAreNotRefreshedBeforeEveryRequest(): void + { + $server = new FakeOAuthServer(refreshTokens: true); + $server->expiresIn = 20; + $authenticator = $this->authenticator($server); + $authenticator->handleChallenge($this->request(), $server->challenge()); + $tokenRequests = \count(array_filter($server->requested, static fn (string $url): bool => str_ends_with($url, '/token'))); + + for ($i = 0; $i < 3; ++$i) { + $request = $authenticator->authenticate($this->request()); + } + + $this->assertCount($tokenRequests, array_filter($server->requested, static fn (string $url): bool => str_ends_with($url, '/token'))); + $this->assertSame('Bearer access-token-1', $request->getHeaderLine('Authorization')); + } + + #[TestDox('a storage failure during the refresh before sending keeps the token until it expires')] + public function testStorageFailureBeforeSendingKeepsTheToken(): void + { + $server = new FakeOAuthServer(refreshTokens: true); + $storage = new class implements CredentialStorageInterface { + public bool $broken = false; + + private readonly InMemoryCredentialStorage $storage; + + public function __construct() + { + $this->storage = new InMemoryCredentialStorage(); + } + + public function getClientRegistration(string $issuer): ?ClientRegistration + { + 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->storage->forget($issuer); + } + + public function synchronize(callable $operation): mixed + { + if ($this->broken) { + throw new RuntimeException('Could not open the credential lock file.'); + } + + return $this->storage->synchronize($operation); + } + }; + $authenticator = $this->authenticator($server, storage: $storage); + $authenticator->handleChallenge($this->request(), $server->challenge()); + self::ageToken($authenticator, $storage); + $storage->broken = true; + + $request = $authenticator->authenticate($this->request()); + + $this->assertSame('Bearer access-token-1', $request->getHeaderLine('Authorization')); + } + + #[TestDox('a storage failure while looking up the client of a stored token keeps the token until it expires')] + public function testRegistrationLookupFailureBeforeSendingKeepsTheToken(): void + { + $server = new FakeOAuthServer(refreshTokens: true); + $storage = new class implements CredentialStorageInterface { + public bool $broken = false; + + private readonly InMemoryCredentialStorage $storage; + + public function __construct() + { + $this->storage = new InMemoryCredentialStorage(); + } + + public function getClientRegistration(string $issuer): ?ClientRegistration + { + if ($this->broken) { + throw new RuntimeException('Could not read the keychain.'); + } + + 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->storage->forget($issuer); + } + + public function synchronize(callable $operation): mixed + { + return $this->storage->synchronize($operation); + } + }; + $storage->saveToken('https://auth.example.com', self::tokenKey(), new AccessToken('stored', 'Bearer', time() + 3600, 'a-refresh-token')); + $authenticator = $this->authenticator($server, storage: $storage); + $authenticator->handleChallenge($this->request(), $server->challenge()); + + // The stored token was reused without resolving a client; let it come close to its expiry. + (new \ReflectionProperty($authenticator, 'token'))->setValue($authenticator, new AccessToken('stored', 'Bearer', time() + 10, 'a-refresh-token')); + $storage->broken = true; + + $request = $authenticator->authenticate($this->request()); + + $this->assertSame('Bearer stored', $request->getHeaderLine('Authorization')); + } + + #[TestDox('a token without a refresh token is sent until it expires, without a token request')] + public function testNoRefreshBeforeSendingWithoutARefreshToken(): void + { + $server = new FakeOAuthServer(); + $storage = new InMemoryCredentialStorage(); + $authenticator = $this->authenticator($server, storage: $storage); + $authenticator->handleChallenge($this->request(), $server->challenge()); + self::ageToken($authenticator, $storage); + $requests = \count($server->requested); + + $request = $authenticator->authenticate($this->request()); + + $this->assertSame('Bearer access-token-1', $request->getHeaderLine('Authorization')); + $this->assertCount($requests, $server->requested); + } + #[TestWith([null, 'refresh_token'])] #[TestWith([['authorization_code', 'refresh_token'], 'refresh_token'])] #[TestWith([['authorization_code'], 'authorization_code'])] @@ -1391,7 +1673,7 @@ public function testUsesARefreshTokenUnlessTheGrantIsNotListed(?array $grantType $server = new FakeOAuthServer(refreshTokens: true); $server->grantTypes = $grantTypes; $storage = new InMemoryCredentialStorage(); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('expired', 'Bearer', time() - 60, 'a-refresh-token')); + $storage->saveToken('https://auth.example.com', self::tokenKey(), new AccessToken('expired', 'Bearer', time() - 60, 'a-refresh-token')); $this->authenticator($server, storage: $storage)->handleChallenge($this->request(), $server->challenge()); @@ -1415,19 +1697,52 @@ public function testRefreshesAStoredTokenThatIsAboutToExpire(): void { $server = new FakeOAuthServer(refreshTokens: true); $storage = new InMemoryCredentialStorage(); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('expiring', 'Bearer', time() + 10, 'a-refresh-token')); + $storage->saveToken('https://auth.example.com', self::tokenKey(), new AccessToken('expiring', 'Bearer', time() + 10, 'a-refresh-token')); $this->authenticator($server, storage: $storage)->handleChallenge($this->request(), $server->challenge()); $this->assertSame('refresh_token', $server->token['grant_type'] ?? null); } + #[TestDox('a token stored by another client configuration in the same storage is not taken over')] + public function testDoesNotTakeOverATokenOfAnotherClient(): void + { + $server = new FakeOAuthServer(); + $storage = new InMemoryCredentialStorage(); + $this->authenticator($server, storage: $storage)->handleChallenge($this->request(), $server->challenge(['mcp:read'])); + $this->assertSame('registered-client', $server->authorization['client_id'] ?? null); + + $configured = $this->authenticator($server, storage: $storage, configuration: OAuth::forApplication('test') + ->setClientCredentials('known-client') + ->setExpectedIssuer('https://auth.example.com')); + $configured->handleChallenge($this->request(), $server->challenge(['mcp:read'])); + + $this->assertSame('known-client', $server->authorization['client_id'] ?? null); + $this->assertNotNull($storage->getToken('https://auth.example.com', self::tokenKey())); + $this->assertNotNull($storage->getToken('https://auth.example.com', self::tokenKey(clientId: 'known-client'))); + } + + #[TestDox('a service account does not take over a token stored for a user of the same client')] + public function testServiceAccountDoesNotTakeOverAUserToken(): void + { + $server = new FakeOAuthServer(); + $storage = new InMemoryCredentialStorage(); + $storage->saveToken('https://auth.example.com', self::tokenKey(clientId: 'known-client'), new AccessToken('user-token', 'Bearer', time() + 3600, null, ['mcp:read'])); + + $serviceAccount = $this->authenticator($server, storage: $storage, configuration: OAuth::forServiceAccount('test', 'known-client', 'secret') + ->setExpectedIssuer('https://auth.example.com')); + $serviceAccount->handleChallenge($this->request(), $server->challenge(['mcp:read'])); + + $this->assertSame('client_credentials', $server->token['grant_type'] ?? null); + $this->assertNotSame('Bearer user-token', $serviceAccount->authenticate($this->request())->getHeaderLine('Authorization')); + } + #[TestDox('a step-up keeps the scopes of a stored token')] public function testStepUpKeepsTheScopesOfAStoredToken(): void { $server = new FakeOAuthServer(); $storage = new InMemoryCredentialStorage(); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('stored', 'Bearer', time() + 3600, null, ['mcp:read'])); + $storage->saveToken('https://auth.example.com', self::tokenKey(), new AccessToken('stored', 'Bearer', time() + 3600, null, ['mcp:read'])); $authenticator = $this->authenticator($server, storage: $storage); $authenticator->handleChallenge($this->request(), $server->challenge(['mcp:read'])); @@ -1442,7 +1757,7 @@ public function testStoredOfflineAccessIsNotRequestedWhenTurnedOff(): void { $server = new FakeOAuthServer(serverScopes: ['offline_access']); $storage = new InMemoryCredentialStorage(); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('stored', 'Bearer', time() + 3600, null, ['mcp:read', 'offline_access'])); + $storage->saveToken('https://auth.example.com', self::tokenKey(), new AccessToken('stored', 'Bearer', time() + 3600, null, ['mcp:read', 'offline_access'])); $authenticator = $this->authenticator($server, storage: $storage, configuration: OAuth::forApplication('test')->setOfflineAccess(false)); $authenticator->handleChallenge((new Psr17Factory())->createRequest('POST', self::ENDPOINT)->withHeader('Authorization', 'Bearer stored'), $server->challenge(['mcp:write'], 403, 'insufficient_scope')); @@ -1494,7 +1809,7 @@ public function testOnlyAStepUpKeepsTheScopesOfAStoredToken(): void { $server = new FakeOAuthServer(); $storage = new InMemoryCredentialStorage(); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('expired', 'Bearer', time() - 60, null, ['openid', 'mcp:read'])); + $storage->saveToken('https://auth.example.com', self::tokenKey(), new AccessToken('expired', 'Bearer', time() - 60, null, ['openid', 'mcp:read'])); $this->authenticator($server, storage: $storage, configuration: OAuth::forApplication('test')->setScopes('openid'))->handleChallenge($this->request(), $server->challenge()); @@ -1615,7 +1930,7 @@ public function testAnOutageDoesNotStartANewAuthorization(int $status, string $b $server = new FakeOAuthServer(refreshTokens: true); $server->refreshAnswer = new Response($status, [], $body); $storage = new InMemoryCredentialStorage(); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('expired', 'Bearer', time() - 60, 'still-good')); + $storage->saveToken('https://auth.example.com', self::tokenKey(), new AccessToken('expired', 'Bearer', time() - 60, 'still-good')); try { $this->authenticator($server, storage: $storage)->handleChallenge($this->request(), $server->challenge()); @@ -1625,7 +1940,7 @@ public function testAnOutageDoesNotStartANewAuthorization(int $status, string $b } $this->assertSame([], $server->authorization); - $this->assertSame('still-good', $storage->getToken('https://auth.example.com', self::ENDPOINT)?->refreshToken); + $this->assertSame('still-good', $storage->getToken('https://auth.example.com', self::tokenKey())?->refreshToken); } #[TestWith(['invalid_grant'])] @@ -1636,7 +1951,7 @@ public function testARefusedRefreshTokenStartsANewAuthorization(string $error): $server = new FakeOAuthServer(refreshTokens: true); $server->refreshAnswer = new Response(400, [], (string) json_encode(['error' => $error])); $storage = new InMemoryCredentialStorage(); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('expired', 'Bearer', time() - 60, 'revoked')); + $storage->saveToken('https://auth.example.com', self::tokenKey(), new AccessToken('expired', 'Bearer', time() - 60, 'revoked')); $this->authenticator($server, storage: $storage)->handleChallenge($this->request(), $server->challenge()); @@ -1860,7 +2175,7 @@ public function testClientMetadataUrlRefusedOnRefreshNamesTheDocument(): void $server = new FakeOAuthServer(refreshTokens: true, clientMetadataDocuments: true); $server->unknownClient = 'https://app.example.com/client.json'; $storage = new InMemoryCredentialStorage(); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('expired', 'Bearer', time() - 60, 'refresh')); + $storage->saveToken('https://auth.example.com', self::tokenKey(clientId: 'https://app.example.com/client.json'), new AccessToken('expired', 'Bearer', time() - 60, 'refresh')); $configuration = OAuth::forApplication('test')->setClientMetadataUrl('https://app.example.com/client.json'); try { @@ -1936,7 +2251,7 @@ public function testServiceAccountIgnoresABroaderStoredToken(array $storedScopes { $server = new FakeOAuthServer(); $storage = new InMemoryCredentialStorage(); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('broader', 'Bearer', time() + 3600, null, $storedScopes)); + $storage->saveToken('https://auth.example.com', self::tokenKey('client_credentials', 'job-client'), new AccessToken('broader', 'Bearer', time() + 3600, null, $storedScopes)); $authenticator = $this->authenticator($server, storage: $storage, configuration: OAuth::forServiceAccount('job', 'job-client', 'job-secret') ->setExpectedIssuer('https://auth.example.com') ->setScopes('mcp:read')); @@ -1952,7 +2267,7 @@ public function testServiceAccountDoesNotRefreshABroaderStoredToken(): void { $server = new FakeOAuthServer(refreshTokens: true); $storage = new InMemoryCredentialStorage(); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('broader', 'Bearer', time() - 60, 'broader-refresh', ['mcp:read', 'admin'])); + $storage->saveToken('https://auth.example.com', self::tokenKey('client_credentials', 'job-client'), new AccessToken('broader', 'Bearer', time() - 60, 'broader-refresh', ['mcp:read', 'admin'])); $authenticator = $this->authenticator($server, storage: $storage, configuration: OAuth::forServiceAccount('job', 'job-client', 'job-secret') ->setExpectedIssuer('https://auth.example.com') ->setScopes('mcp:read')); @@ -1998,7 +2313,7 @@ public function testServiceAccountStepUpIgnoresStoredScopes(int $expiresIn): voi ->setScopes('mcp:read')); $authenticator->handleChallenge($this->request(), $server->challenge(['mcp:read'])); $current = $authenticator->authenticate($this->request()); - $storage->saveToken('https://auth.example.com', self::ENDPOINT, new AccessToken('broader', 'Bearer', time() + $expiresIn, null, ['mcp:read', 'admin'])); + $storage->saveToken('https://auth.example.com', self::tokenKey('client_credentials', 'job-client'), new AccessToken('broader', 'Bearer', time() + $expiresIn, null, ['mcp:read', 'admin'])); $server->token = []; $retry = $authenticator->handleChallenge($current, $server->challenge(['admin'], 403, 'insufficient_scope')); @@ -2252,6 +2567,27 @@ public function synchronize(callable $operation): mixed }; } + /** + * The key the authenticator stores a token for ENDPOINT under. + */ + public static function tokenKey(string $grant = 'authorization_code', ?string $clientId = null): string + { + return self::ENDPOINT.'#'.$grant.(null === $clientId ? '' : ' '.$clientId); + } + + /** + * Brings the current token close to its expiry, in the authenticator and in the storage. + */ + private static function ageToken(OAuthAuthenticator $authenticator, CredentialStorageInterface $storage): void + { + $property = new \ReflectionProperty($authenticator, 'token'); + $token = $property->getValue($authenticator); + \assert($token instanceof AccessToken); + $aged = new AccessToken($token->accessToken, $token->tokenType, time() + 10, $token->refreshToken, $token->scopes, $token->clientId); + $property->setValue($authenticator, $aged); + $storage->saveToken('https://auth.example.com', self::tokenKey(), $aged); + } + private function request(): RequestInterface { return (new Psr17Factory())->createRequest('POST', self::ENDPOINT); @@ -2273,7 +2609,7 @@ private function storageWithAnotherProcess(AccessToken $tokenOfOtherProcess): Cr public function __construct(private readonly AccessToken $tokenOfOtherProcess) { $this->storage = new InMemoryCredentialStorage(); - $this->storage->saveToken('https://auth.example.com', OAuthAuthenticatorTest::ENDPOINT, new AccessToken('expired', 'Bearer', time() - 60, 'original')); + $this->storage->saveToken('https://auth.example.com', OAuthAuthenticatorTest::tokenKey(), new AccessToken('expired', 'Bearer', time() - 60, 'original')); } public function getClientRegistration(string $issuer): ?ClientRegistration @@ -2304,7 +2640,7 @@ public function forget(?string $issuer = null): void public function synchronize(callable $operation): mixed { - $this->storage->saveToken('https://auth.example.com', OAuthAuthenticatorTest::ENDPOINT, $this->tokenOfOtherProcess); + $this->storage->saveToken('https://auth.example.com', OAuthAuthenticatorTest::tokenKey(), $this->tokenOfOtherProcess); $this->synchronized = true; try { diff --git a/tests/Unit/Client/Transport/HttpTransportTest.php b/tests/Unit/Client/Transport/HttpTransportTest.php index f2960eca..96c34458 100644 --- a/tests/Unit/Client/Transport/HttpTransportTest.php +++ b/tests/Unit/Client/Transport/HttpTransportTest.php @@ -318,6 +318,27 @@ public function testAuthorizationFailureDuringResumptionIsNotRetried(): void $this->assertCount(2, $httpClient->requests); } + #[TestDox('a pending authorization during a stream resumption reaches the request as it is, without logging an error')] + public function testPendingAuthorizationDuringResumptionReachesTheRequest(): void + { + $clock = new SteppingClock(); + $httpClient = RecordingHttpClient::queued([ + new Response(200, ['Content-Type' => 'text/event-stream'], "id: event-0\ndata:\n\n"), + new AuthorizationPendingException('https://auth.example.com/authorize?state=abc'), + ], $clock); + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->never())->method('error'); + + try { + $this->runRequest($this->createTransport(httpClient: $httpClient, clock: $clock, logger: $logger), 1); + $this->fail('The request should have failed.'); + } catch (AuthorizationPendingException $e) { + $this->assertSame('https://auth.example.com/authorize?state=abc', $e->authorizationUrl); + } + + $this->assertCount(2, $httpClient->requests); + } + #[TestDox('the server-minted session ID is exposed while connected and cleared on close')] public function testSessionIdIsExposedAndClearedOnClose(): void { @@ -722,6 +743,26 @@ public function testReconnectStatusHandling(int $status, int $requests): void $this->assertCount($requests, $httpClient->requests); } + #[TestDox('a Symfony client passed without auth stops following redirects')] + public function testPassedSymfonyClientStopsFollowingRedirects(): void + { + if (!(new \ReflectionClass(Psr18Client::class))->hasMethod('withOptions')) { + $this->markTestSkipped('Psr18Client has no withOptions() before symfony/http-client 6.2.'); + } + + $maxRedirects = null; + $mock = new MockHttpClient(static function (string $method, string $url, array $options) use (&$maxRedirects): MockResponse { + $maxRedirects = $options['max_redirects']; + + return new MockResponse('', ['http_code' => 202]); + }); + + $transport = new HttpTransport('http://localhost/mcp', httpClient: new Psr18Client($mock), requestFactory: $this->factory, streamFactory: $this->factory); + $transport->send('{"jsonrpc":"2.0","method":"notifications/initialized"}'); + + $this->assertSame(0, $maxRedirects); + } + #[TestDox('a discovered Symfony client keeps its configuration and stops following redirects')] public function testDiscoveredClientKeepsItsConfiguration(): void { diff --git a/tests/Unit/Fixtures/RecordingFiberContext.php b/tests/Unit/Fixtures/RecordingFiberContext.php index a94fed76..9e2d1fb0 100644 --- a/tests/Unit/Fixtures/RecordingFiberContext.php +++ b/tests/Unit/Fixtures/RecordingFiberContext.php @@ -15,7 +15,7 @@ use Mcp\Server\Fiber\FiberContextInterface; /** - * @author bbdu3 + * @author bbdu3 */ final class RecordingFiberContext implements FiberContextInterface { diff --git a/tests/Unit/Server/Fiber/FiberContextTest.php b/tests/Unit/Server/Fiber/FiberContextTest.php index f1c0bdb7..6307e86e 100644 --- a/tests/Unit/Server/Fiber/FiberContextTest.php +++ b/tests/Unit/Server/Fiber/FiberContextTest.php @@ -29,7 +29,7 @@ use Psr\Http\Message\ResponseInterface; /** - * @author bbdu3 + * @author bbdu3 */ final class FiberContextTest extends TestCase { diff --git a/tests/Unit/Server/Transport/Http/OAuth/SecureUrlTest.php b/tests/Unit/Server/Transport/Http/OAuth/SecureUrlTest.php index cccd0c14..b357210d 100644 --- a/tests/Unit/Server/Transport/Http/OAuth/SecureUrlTest.php +++ b/tests/Unit/Server/Transport/Http/OAuth/SecureUrlTest.php @@ -129,4 +129,38 @@ public function testDetectsLoopbackUrls(string $url, bool $loopback): void { $this->assertSame($loopback, SecureUrl::isLoopback($url)); } + + /** + * @return iterable + */ + public static function providePrivateNetworkUrls(): iterable + { + yield '10/8' => ['https://10.0.0.5/x', true]; + yield 'upper end of 172.16/12' => ['https://172.31.255.1/x', true]; + yield 'above 172.16/12' => ['https://172.32.0.1/x', false]; + yield '192.168/16' => ['https://192.168.1.1/x', true]; + yield 'link-local metadata service' => ['https://169.254.169.254/x', true]; + yield 'shared address space' => ['https://100.100.100.200/x', true]; + yield 'above the shared address space' => ['https://100.128.0.1/x', false]; + yield 'unique local IPv6' => ['https://[fd00::1]/x', true]; + yield 'link-local IPv6' => ['https://[fe80::1]/x', true]; + yield 'public IPv4' => ['https://8.8.8.8/x', false]; + yield 'public IPv6' => ['https://[2001:db8::1]/x', false]; + yield 'loopback' => ['https://127.0.0.1/x', false]; + yield 'a name, which is not resolved' => ['https://internal.corp/x', false]; + yield 'site-local IPv6' => ['https://[fec0::1]/x', true]; + yield 'NAT64 of a private address' => ['https://[64:ff9b::a00:5]/x', true]; + yield 'NAT64 of the metadata service' => ['https://[64:ff9b::a9fe:a9fe]/x', true]; + yield 'NAT64 of loopback' => ['https://[64:ff9b::7f00:1]/x', true]; + yield 'NAT64 of a public address' => ['https://[64:ff9b::808:808]/x', false]; + yield 'local-use NAT64' => ['https://[64:ff9b:1::808:808]/x', true]; + yield '6to4 of a private address' => ['https://[2002:c0a8:101::1]/x', true]; + yield '6to4 of a public address' => ['https://[2002:808:808::1]/x', false]; + } + + #[DataProvider('providePrivateNetworkUrls')] + public function testDetectsPrivateNetworkUrls(string $url, bool $private): void + { + $this->assertSame($private, SecureUrl::isPrivateNetwork($url)); + } }