From cde79d59a0e613277f488c86878ec20720cc1a19 Mon Sep 17 00:00:00 2001 From: BrocksiNet Date: Mon, 26 Oct 2026 20:31:54 +0100 Subject: [PATCH 01/10] [Server] Bind `requestState` to the request it was minted for The 2026-07-28 specification recommends binding a requestState to the authenticated principal and the originating request. Add Builder::setRequestStateBinder() and RequestStateBinderInterface: the binder's value is covered by the state's MAC on mint and recomputed on verify, so a state echoed on another call or under another principal is refused like a forged one. OriginatingRequestBinder binds the principal, the method, the tool, prompt or resource and a digest of the arguments. Opt-in: without a binder, the wire format and verification are unchanged. --- CHANGELOG.md | 1 + docs/handlers/input-required.md | 25 +++ docs/run/server-builder.md | 6 + src/Server/Builder.php | 22 +++ .../Stateless/OriginatingRequestBinder.php | 76 +++++++++ .../Stateless/RequestStateBinderInterface.php | 44 +++++ src/Server/Stateless/RequestStateCodec.php | 25 ++- src/Server/Stateless/StatelessProtocol.php | 25 ++- .../OriginatingRequestBinderTest.php | 96 +++++++++++ .../Stateless/RequestStateCodecTest.php | 47 ++++++ .../Stateless/StatelessProtocolTest.php | 159 +++++++++++++++++- 11 files changed, 516 insertions(+), 10 deletions(-) create mode 100644 src/Server/Stateless/OriginatingRequestBinder.php create mode 100644 src/Server/Stateless/RequestStateBinderInterface.php create mode 100644 tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index a580f842f..aa6080939 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * Fix a nullable tool parameter with a default (e.g. `?string $category = 'books'`) receiving the default when the client explicitly sends `null`: `ReferenceHandler` now passes the `null` through. * Fix a `subscriptions/listen` stream that can carry nothing (the server announces none of the requested types, or the client asks for none) being held open for the full subscription lifetime: it is acknowledged with an empty `notifications` filter and closed gracefully right away, instead of pinning a PHP-FPM worker. Adds `NotificationFilter::isEmpty()`. * [BC Break] Answer `server/discover` with `DiscoverHandler`, which a handler added through `Builder::addRequestHandler()` can replace, and dispatch `RequestEvent`, `ResponseEvent` and `ErrorEvent` for it. A `StatelessProtocol` built without the `Builder` needs `DiscoverHandler` and a `MessageFactory` with `DiscoverRequest` to answer it. +* Add `Builder::setRequestStateBinder()` binding the modern era's `requestState` to the request it was minted for, as the 2026-07-28 specification recommends: the binder's value is covered by the state's MAC, and a state echoed on a request that binds to another value is refused like a forged one. `OriginatingRequestBinder` binds the authenticated principal, the method, the tool, prompt or resource, and a digest of the arguments; `RequestStateBinderInterface` takes custom bindings. Without a binder, nothing changes. ### Client diff --git a/docs/handlers/input-required.md b/docs/handlers/input-required.md index 5139e936b..b9a5b6cee 100644 --- a/docs/handlers/input-required.md +++ b/docs/handlers/input-required.md @@ -98,6 +98,31 @@ The **same key must reach every process that might serve the retry**. A per-proc value works only for a single-process deployment. Nothing secret belongs in the payload — it is signed, not encrypted. +### Binding the state to the request + +A valid state verifies on any request of the TTL, so a client can echo it on another call, +another tool, or under another user's token. The specification recommends binding it to the +authenticated principal and the originating request. `Builder::setRequestStateBinder()` does +that for the modern era: the binder's value is covered by the MAC of every state the request +mints, and a retry that binds to another value is refused like a forged state. The value +itself never goes on the wire. + +```php +use Mcp\Server\Stateless\OriginatingRequestBinder; + +->setRequestState($_ENV['MCP_REQUEST_STATE_KEY'], ttl: 600) +->setRequestStateBinder(new OriginatingRequestBinder()) +``` + +`OriginatingRequestBinder` binds the method, the tool or prompt name and a digest of its +arguments (or the resource URI), and the token's issuer, client and subject. For anything +else, implement `RequestStateBinderInterface`, which receives the request as the client sent +it and its `AccessToken`. Bind only what stays the same across the rounds of one call; +`_meta` does not. + +States minted before the binder was configured no longer verify, so turning it on fails the +calls in flight for at most one TTL. + ## Capabilities A server must not ask for input the client cannot provide. The SDK checks each ask against diff --git a/docs/run/server-builder.md b/docs/run/server-builder.md index f0ddac651..dd3169e9f 100644 --- a/docs/run/server-builder.md +++ b/docs/run/server-builder.md @@ -109,6 +109,7 @@ required to serve either one. Each knob is covered in depth elsewhere: use Mcp\Schema\Enum\CacheScope; use Mcp\Schema\Enum\ProtocolVersion; use Mcp\Server; +use Mcp\Server\Stateless\OriginatingRequestBinder; use Mcp\Server\Subscription\Psr16NotificationBus; use Mcp\Server\Wire\CachePolicy; @@ -119,6 +120,10 @@ $server = Server::builder() // Without a key, the server refuses every state the client sends back. ->setRequestState($_ENV['MCP_REQUEST_STATE_KEY'], ttl: 600) + // Binds that state to the principal and the call it was minted for, so a + // client cannot echo it on another call or under another token. + ->setRequestStateBinder(new OriginatingRequestBinder()) + // Bounds the input-required shim, which fulfils an `InputRequiredResult` // over a handshake-era connection. `withoutInputRequiredShim()` turns it off. // Defaults to 8 rounds and a 600 second timeout for each input request. @@ -334,6 +339,7 @@ $server = Server::builder() | `setModernVersions()` | versions | Narrow the revisions the modern (2026-07-28) leg answers for | | `withoutModernEra()` | - | Serve the handshake era only | | `setRequestState()` | key, ttl? | Signing key and lifetime for multi round-trip request state | +| `setRequestStateBinder()` | binder | Bind request state to the request and principal it was minted for | | `setInputRequiredLimits()` | maxRounds, roundTimeout | Bound the input-required shim on handshake-era connections | | `withoutInputRequiredShim()` | - | Do not fulfil an `InputRequiredResult` over a handshake-era connection | | `setCachePolicy()` | policy | Set the `ttlMs`/`cacheScope` hints on cacheable results | diff --git a/src/Server/Builder.php b/src/Server/Builder.php index e97f20677..dfe706504 100644 --- a/src/Server/Builder.php +++ b/src/Server/Builder.php @@ -59,6 +59,7 @@ use Mcp\Server\Session\SessionManager; use Mcp\Server\Session\SessionManagerInterface; use Mcp\Server\Session\SessionStoreInterface; +use Mcp\Server\Stateless\RequestStateBinderInterface; use Mcp\Server\Stateless\RequestStateCodec; use Mcp\Server\Stateless\StandardHeaderValidator; use Mcp\Server\Stateless\StatelessProtocol; @@ -148,6 +149,8 @@ final class Builder private int $requestStateTtl = 600; + private ?RequestStateBinderInterface $requestStateBinder = null; + /** * @var array> */ @@ -389,6 +392,24 @@ public function setRequestState(string $key, int $ttl = 600): self return $this; } + /** + * Binds every `requestState` the modern era mints to the request it was + * minted for, e.g. with {@see Stateless\OriginatingRequestBinder} + * to the principal, method and arguments: a state echoed on a request that + * binds to another value fails verification like a forged one. Has no + * effect without {@see self::setRequestState()}. + * + * States minted before a binder was configured no longer verify, so + * turning one on fails the multi round-trip calls in flight for at most + * the configured TTL. + */ + public function setRequestStateBinder(RequestStateBinderInterface $binder): self + { + $this->requestStateBinder = $binder; + + return $this; + } + /** * Configures the server's pagination limit. */ @@ -1026,6 +1047,7 @@ public function buildStateless(array $supportedVersions = [ProtocolVersion::V202 notificationBus: $this->notificationBus, extensionMethods: $this->extensionMethods, eventDispatcher: $parts['eventDispatcher'], + requestStateBinder: $this->requestStateBinder, ); } diff --git a/src/Server/Stateless/OriginatingRequestBinder.php b/src/Server/Stateless/OriginatingRequestBinder.php new file mode 100644 index 000000000..c9e257c46 --- /dev/null +++ b/src/Server/Stateless/OriginatingRequestBinder.php @@ -0,0 +1,76 @@ + + */ +final class OriginatingRequestBinder implements RequestStateBinderInterface +{ + public function bind(Request $request, ?AccessToken $accessToken): string + { + [$target, $arguments] = match (true) { + $request instanceof CallToolRequest => [$request->name, $request->arguments], + $request instanceof GetPromptRequest => [$request->name, $request->arguments ?? []], + $request instanceof ReadResourceRequest => [$request->uri, []], + default => [null, []], + }; + + return json_encode([ + 'method' => $request::getMethod(), + 'target' => $target, + 'arguments' => hash('sha256', json_encode(self::canonical($arguments), \JSON_THROW_ON_ERROR)), + 'principal' => null === $accessToken ? null : [ + 'iss' => \is_string($issuer = $accessToken->getClaim('iss')) ? $issuer : null, + 'client' => $accessToken->getClientId(), + 'sub' => $accessToken->getSubject(), + ], + ], \JSON_THROW_ON_ERROR); + } + + /** + * Sorts object members recursively, so a client re-encoding the same + * arguments in another member order still binds to the same digest. + */ + private static function canonical(mixed $value): mixed + { + if (!\is_array($value)) { + return $value; + } + + if (!array_is_list($value)) { + ksort($value, \SORT_STRING); + } + + return array_map(self::canonical(...), $value); + } +} diff --git a/src/Server/Stateless/RequestStateBinderInterface.php b/src/Server/Stateless/RequestStateBinderInterface.php new file mode 100644 index 000000000..228910648 --- /dev/null +++ b/src/Server/Stateless/RequestStateBinderInterface.php @@ -0,0 +1,44 @@ + + */ +interface RequestStateBinderInterface +{ + /** + * @param Request $request the request as the client sent it, before any `RequestEvent` listener ran + * @param AccessToken|null $accessToken the token the request was authorized with, null on an unauthorized transport + */ + public function bind(Request $request, ?AccessToken $accessToken): string; +} diff --git a/src/Server/Stateless/RequestStateCodec.php b/src/Server/Stateless/RequestStateCodec.php index 9dcf477bf..e0a9e9ed7 100644 --- a/src/Server/Stateless/RequestStateCodec.php +++ b/src/Server/Stateless/RequestStateCodec.php @@ -23,6 +23,11 @@ * {@see self::verify()} refuses anything whose MAC or expiry does not hold. * Signed, not encrypted: nothing secret belongs in the payload. * + * A codec {@see self::withBinding() bound} to a value also covers that value + * with the MAC, without putting it on the wire: a state minted under one + * binding fails verification under any other, and so does a state minted + * without one. The wire format is the same either way. + * * @author Christopher Hertel */ final class RequestStateCodec @@ -35,6 +40,7 @@ final class RequestStateCodec public function __construct( private readonly string $key, private readonly int $ttlSeconds = 600, + private readonly ?string $binding = null, ) { if (\strlen($this->key) < self::MINIMUM_KEY_BYTES) { throw new InvalidArgumentException(\sprintf('The requestState signing key must be at least %d bytes, got %d.', self::MINIMUM_KEY_BYTES, \strlen($this->key))); @@ -45,6 +51,16 @@ public function __construct( } } + /** + * A codec with the same key and TTL whose states only verify under the + * same binding, e.g. the value a {@see RequestStateBinderInterface} + * computed for the request at hand. + */ + public function withBinding(string $binding): self + { + return new self($this->key, $this->ttlSeconds, $binding); + } + /** * @param array $payload server context to carry to the retry — never secrets */ @@ -61,7 +77,8 @@ public function mint(array $payload, ?int $now = null): string /** * @return array the payload that was sealed * - * @throws RequestStateException when the value is malformed, unsigned by this key, or expired + * @throws RequestStateException when the value is malformed, unsigned by this key, bound to + * something else than this codec, or expired */ public function verify(string $state, ?int $now = null): array { @@ -108,7 +125,11 @@ public function verify(string $state, ?int $now = null): array private function sign(string $body): string { - return hash_hmac(self::ALGORITHM, $body, $this->key, true); + // The body is base64url, so it never contains the '.' separating it + // from the binding: a bound and an unbound input can never collide. + $input = null === $this->binding ? $body : $body.'.'.$this->binding; + + return hash_hmac(self::ALGORITHM, $input, $this->key, true); } private static function encode(string $raw): string diff --git a/src/Server/Stateless/StatelessProtocol.php b/src/Server/Stateless/StatelessProtocol.php index 62a16fab7..68fc95227 100644 --- a/src/Server/Stateless/StatelessProtocol.php +++ b/src/Server/Stateless/StatelessProtocol.php @@ -113,6 +113,7 @@ public function __construct( private readonly ?NotificationBusInterface $notificationBus = null, private readonly array $extensionMethods = [], private readonly ?EventDispatcherInterface $eventDispatcher = null, + private readonly ?RequestStateBinderInterface $requestStateBinder = null, ) { $this->codec = $codec ?? new Rev2026Codec($configuration->serverInfo, $cachePolicy); @@ -506,8 +507,16 @@ private function dispatch(string $method, array $decoded, RequestMeta $meta, str $session->set(Protocol::SESSION_LOGGING_LEVEL, $meta->logLevel->value); } + // Bound to the request as it arrived, before a RequestEvent listener + // could change it: the retry has to bind to the same value. + $requestStateCodec = $this->requestStateCodec; + + if (null !== $requestStateCodec && null !== $this->requestStateBinder) { + $requestStateCodec = $requestStateCodec->withBinding($this->requestStateBinder->bind($request, $accessToken)); + } + try { - $input = $this->liftInputContext($decoded['params'] ?? null); + $input = $this->liftInputContext($decoded['params'] ?? null, $requestStateCodec); } catch (RequestStateException $e) { // Invalid params, not an authorization failure: the client only // echoes what it was given, and the reason stays out of the answer. @@ -520,8 +529,8 @@ private function dispatch(string $method, array $decoded, RequestMeta $meta, str $session->set(InputContext::class, $input); } - if (null !== $this->requestStateCodec) { - $session->set(RequestStateCodec::class, $this->requestStateCodec); + if (null !== $requestStateCodec) { + $session->set(RequestStateCodec::class, $requestStateCodec); } // What ClientGateway::progress() reads to find the progress token, and @@ -679,7 +688,9 @@ private function run(RequestHandlerInterface $handler, Request $request, Session { $fiber = new \Fiber(static fn (): mixed => $handler->handle($request, $session)); $input = $session->get(InputContext::class); - $replay = new ElicitationReplay($input instanceof InputContext ? $input : null, $this->requestStateCodec); + // The codec bound to this request, which dispatch() put on the session. + $codec = $session->get(RequestStateCodec::class); + $replay = new ElicitationReplay($input instanceof InputContext ? $input : null, $codec instanceof RequestStateCodec ? $codec : null); $suspended = $fiber->start(); @@ -902,7 +913,7 @@ private function toErrorResult(Request $request, Session $session, string $metho * * @throws RequestStateException when a state is present but does not verify */ - private function liftInputContext(?array $params): ?InputContext + private function liftInputContext(?array $params, ?RequestStateCodec $requestStateCodec): ?InputContext { $responses = \is_array($params['inputResponses'] ?? null) ? $params['inputResponses'] : null; $state = \is_string($params['requestState'] ?? null) ? $params['requestState'] : null; @@ -922,11 +933,11 @@ private function liftInputContext(?array $params): ?InputContext if (null !== $state) { // A server with no codec never minted a state, so this one cannot // have come from here. - if (null === $this->requestStateCodec) { + if (null === $requestStateCodec) { throw new RequestStateException('mac'); } - $payload = $this->requestStateCodec->verify($state); + $payload = $requestStateCodec->verify($state); } return new InputContext($responses ?? [], $payload); diff --git a/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php b/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php new file mode 100644 index 000000000..f549bc6b2 --- /dev/null +++ b/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php @@ -0,0 +1,96 @@ + $scopes + */ + private static function token(string $subject = 'alice', string $clientId = 'app', array $scopes = ['read']): AccessToken + { + return new AccessToken($scopes, ['iss' => 'https://auth.example.com', 'client_id' => $clientId, 'sub' => $subject]); + } + + /** + * @param array $arguments + */ + private static function call(string $tool = 'book', array $arguments = ['to' => 'BER', 'seats' => 2]): CallToolRequest + { + return (new CallToolRequest($tool, $arguments))->withId(1); + } + + #[TestDox('a retry of the same call binds to the same value')] + public function testRetryBindsToTheSameValue(): void + { + $binder = new OriginatingRequestBinder(); + + // A retry carries another id, another `_meta`, and a refreshed token. + $first = self::call()->withMeta(['progressToken' => 'p1']); + $retry = (new CallToolRequest('book', ['seats' => 2, 'to' => 'BER']))->withId(2)->withMeta(['progressToken' => 'p2']); + + $this->assertSame( + $binder->bind($first, self::token(scopes: ['read'])), + $binder->bind($retry, self::token(scopes: ['read', 'write'])), + ); + } + + /** + * @return iterable + */ + public static function otherRequests(): iterable + { + yield 'other tool' => [self::call(tool: 'cancel'), self::token()]; + yield 'other arguments' => [self::call(arguments: ['to' => 'CDG', 'seats' => 2]), self::token()]; + yield 'nested arguments differ' => [self::call(arguments: ['to' => 'BER', 'seats' => [2]]), self::token()]; + yield 'other subject' => [self::call(), self::token(subject: 'mallory')]; + yield 'other client' => [self::call(), self::token(clientId: 'other-app')]; + yield 'no token' => [self::call(), null]; + yield 'prompt of the same name' => [(new GetPromptRequest('book', ['to' => 'BER', 'seats' => '2']))->withId(1), self::token()]; + yield 'other method' => [(new ListToolsRequest())->withId(1), self::token()]; + } + + #[DataProvider('otherRequests')] + #[TestDox('another request or principal binds to another value: $_dataName')] + public function testOtherRequestsBindElsewhere(Request $request, ?AccessToken $token): void + { + $binder = new OriginatingRequestBinder(); + + $this->assertNotSame($binder->bind(self::call(), self::token()), $binder->bind($request, $token)); + } + + #[TestDox('a resource read binds to its URI')] + public function testResourceReadBindsToItsUri(): void + { + $binder = new OriginatingRequestBinder(); + + $this->assertSame( + $binder->bind((new ReadResourceRequest('file:///a'))->withId(1), null), + $binder->bind((new ReadResourceRequest('file:///a'))->withId(2), null), + ); + $this->assertNotSame( + $binder->bind((new ReadResourceRequest('file:///a'))->withId(1), null), + $binder->bind((new ReadResourceRequest('file:///b'))->withId(1), null), + ); + } +} diff --git a/tests/Unit/Server/Stateless/RequestStateCodecTest.php b/tests/Unit/Server/Stateless/RequestStateCodecTest.php index 6b84b8f98..db6386dc4 100644 --- a/tests/Unit/Server/Stateless/RequestStateCodecTest.php +++ b/tests/Unit/Server/Stateless/RequestStateCodecTest.php @@ -112,6 +112,53 @@ public function testFailureReasonsAreOpaque(): void } } + #[TestDox('a bound state verifies under the same binding')] + public function testBoundRoundTrip(): void + { + $codec = (new RequestStateCodec(self::KEY))->withBinding('tools/call|book'); + + $this->assertSame(['round' => 1], $codec->withBinding('tools/call|book')->verify($codec->mint(['round' => 1]))); + } + + #[TestDox('a bound state is refused under another binding, like a forged one')] + public function testBoundStateRefusedUnderAnotherBinding(): void + { + $codec = new RequestStateCodec(self::KEY); + $state = $codec->withBinding('tools/call|book')->mint(['round' => 1]); + + $this->expectExceptionObject(new RequestStateException('mac')); + $codec->withBinding('tools/call|cancel')->verify($state); + } + + #[TestDox('an unbound state is refused by a bound codec, and a bound one by an unbound codec')] + public function testBindingIsNeitherAddedNorStripped(): void + { + $unbound = new RequestStateCodec(self::KEY); + $bound = $unbound->withBinding(''); + + foreach ([[$unbound, $bound], [$bound, $unbound]] as [$minter, $verifier]) { + try { + $verifier->verify($minter->mint(['round' => 1])); + $this->fail('Expected rejection.'); + } catch (RequestStateException $e) { + $this->assertSame('mac', $e->getMessage()); + } + } + } + + #[TestDox('the binding stays off the wire, which keeps its shape')] + public function testBindingStaysOffTheWire(): void + { + $state = (new RequestStateCodec(self::KEY))->withBinding('alice@example.com')->mint(['round' => 1], now: 1_000); + [$body] = explode('.', $state); + + $this->assertCount(2, explode('.', $state)); + $this->assertSame( + ['exp' => 1_600, 'data' => ['round' => 1]], + json_decode(base64_decode(strtr($body, '-_', '+/')), true), + ); + } + #[TestDox('a key too short to be safe is refused at construction')] public function testShortKeyRefused(): void { diff --git a/tests/Unit/Server/Stateless/StatelessProtocolTest.php b/tests/Unit/Server/Stateless/StatelessProtocolTest.php index a542efffb..514054057 100644 --- a/tests/Unit/Server/Stateless/StatelessProtocolTest.php +++ b/tests/Unit/Server/Stateless/StatelessProtocolTest.php @@ -37,10 +37,13 @@ use Mcp\Schema\Result\ReadResourceResult; use Mcp\Schema\ServerCapabilities; use Mcp\Server; +use Mcp\Server\Authorization\AccessToken; use Mcp\Server\Handler\Request\RequestHandlerInterface; use Mcp\Server\RequestContext; use Mcp\Server\Session\SessionInterface; +use Mcp\Server\Stateless\OriginatingRequestBinder; use Mcp\Server\Stateless\RequestMeta; +use Mcp\Server\Stateless\RequestStateBinderInterface; use Mcp\Server\Stateless\StatelessProtocol; use Mcp\Server\Stateless\StatelessResult; use Mcp\Server\Subscription\InMemoryNotificationBus; @@ -584,7 +587,7 @@ public function testUndeclaredGatewayElicitationIsRefused(): void * A handler asking twice: the second round has to remember the first * round's answer, which is what the `requestState` is for. */ - private static function twoAskProtocol(bool $signed = true): StatelessProtocol + private static function twoAskProtocol(bool $signed = true, ?RequestStateBinderInterface $binder = null): StatelessProtocol { $builder = Server::builder() ->setServerInfo('test-server', '1.0.0') @@ -611,6 +614,10 @@ static function (RequestContext $context): string { $builder->setRequestState(str_repeat('k', 32)); } + if (null !== $binder) { + $builder->setRequestStateBinder($binder); + } + return $builder->buildStateless([ProtocolVersion::V2026_07_28]); } @@ -657,6 +664,156 @@ public function testNamedAsksCarryAcrossRounds(): void $this->assertSame('morning/aisle', $third['body']['result']['content'][0]['text']); } + /** + * A {@see self::book()} that can carry other arguments and an access token. + * + * @param array $params + * @param array $arguments + * + * @return array + */ + private static function bookAs(StatelessProtocol $protocol, ?AccessToken $token, array $params = [], array $arguments = []): array + { + $result = $protocol->handle(json_encode([ + 'jsonrpc' => '2.0', + 'id' => 1, + 'method' => 'tools/call', + 'params' => [ + 'name' => 'books_flight', + 'arguments' => (object) $arguments, + '_meta' => [ + RequestMeta::PROTOCOL_VERSION => ProtocolVersion::V2026_07_28->value, + RequestMeta::CLIENT_CAPABILITIES => ['elicitation' => new \stdClass()], + ], + ...$params, + ], + ], \JSON_THROW_ON_ERROR), [ + 'MCP-Protocol-Version' => ProtocolVersion::V2026_07_28->value, + 'Mcp-Method' => 'tools/call', + 'Mcp-Name' => 'books_flight', + ], $token); + + return [ + 'status' => $result->httpStatus, + 'body' => json_decode($result->toJson(), true, flags: \JSON_THROW_ON_ERROR), + ]; + } + + private static function alice(string $subject = 'alice'): AccessToken + { + return new AccessToken(['flights'], ['iss' => 'https://auth.example.com', 'client_id' => 'app', 'sub' => $subject]); + } + + /** + * The second round of a {@see self::twoAskProtocol()} call, which mints + * the state carrying the first answer. + */ + private function mintedState(StatelessProtocol $protocol, ?AccessToken $token, array $arguments = []): string + { + $round = self::bookAs($protocol, $token, [ + 'inputResponses' => ['when' => ['action' => 'accept', 'content' => ['v' => 'morning']]], + ], $arguments); + + $this->assertIsString($round['body']['result']['requestState'] ?? null); + + return $round['body']['result']['requestState']; + } + + #[TestDox('a bound state verifies on a retry of the same call by the same principal')] + public function testBoundStateVerifiesOnTheSameCall(): void + { + $protocol = self::twoAskProtocol(binder: new OriginatingRequestBinder()); + $state = $this->mintedState($protocol, self::alice(), ['to' => 'BER']); + + $answer = self::bookAs($protocol, self::alice(), [ + 'inputResponses' => ['seat' => ['action' => 'accept', 'content' => ['v' => 'aisle']]], + 'requestState' => $state, + ], ['to' => 'BER']); + + $this->assertSame(200, $answer['status']); + $this->assertSame('morning/aisle', $answer['body']['result']['content'][0]['text']); + } + + /** + * @return iterable, ?AccessToken}> + */ + public static function otherCalls(): iterable + { + yield 'other arguments' => [['to' => 'CDG'], self::alice()]; + yield 'other principal' => [['to' => 'BER'], self::alice('mallory')]; + yield 'no principal' => [['to' => 'BER'], null]; + } + + /** + * @param array $arguments + */ + #[DataProvider('otherCalls')] + #[TestDox('a bound state echoed on another call is refused like a forged one: $_dataName')] + public function testBoundStateRefusedOnAnotherCall(array $arguments, ?AccessToken $token): void + { + $protocol = self::twoAskProtocol(binder: new OriginatingRequestBinder()); + $state = $this->mintedState($protocol, self::alice(), ['to' => 'BER']); + + $answer = self::bookAs($protocol, $token, [ + 'inputResponses' => ['seat' => ['action' => 'accept', 'content' => ['v' => 'aisle']]], + 'requestState' => $state, + ], $arguments); + + $this->assertSame(400, $answer['status']); + $this->assertSame(Error::INVALID_PARAMS, $answer['body']['error']['code']); + $this->assertSame('The supplied requestState failed verification.', $answer['body']['error']['message']); + } + + #[TestDox('the binder sees the request as sent and its access token')] + public function testBinderSeesTheRequestAndToken(): void + { + $binder = new class implements RequestStateBinderInterface { + /** @var list, ?string}> */ + public array $calls = []; + + public function bind(Request $request, ?AccessToken $accessToken): string + { + \assert($request instanceof CallToolRequest); + $this->calls[] = [$request->name, $request->arguments, $accessToken?->getSubject()]; + + return 'fixed'; + } + }; + + $this->mintedState(self::twoAskProtocol(binder: $binder), self::alice(), ['to' => 'BER']); + + $this->assertSame([['books_flight', ['to' => 'BER'], 'alice']], $binder->calls); + } + + #[TestDox('a state minted before a binder was configured is refused once one is')] + public function testUnboundStateRefusedByABindingServer(): void + { + $state = $this->mintedState(self::twoAskProtocol(), self::alice()); + + $answer = self::bookAs(self::twoAskProtocol(binder: new OriginatingRequestBinder()), self::alice(), [ + 'inputResponses' => ['seat' => ['action' => 'accept', 'content' => ['v' => 'aisle']]], + 'requestState' => $state, + ]); + + $this->assertSame(400, $answer['status']); + $this->assertSame(Error::INVALID_PARAMS, $answer['body']['error']['code']); + } + + #[TestDox('without a binder, a state verifies on any call, as before')] + public function testUnboundStateIsNotTiedToTheCall(): void + { + $protocol = self::twoAskProtocol(); + $state = $this->mintedState($protocol, self::alice(), ['to' => 'BER']); + + $answer = self::bookAs($protocol, self::alice('mallory'), [ + 'inputResponses' => ['seat' => ['action' => 'accept', 'content' => ['v' => 'aisle']]], + 'requestState' => $state, + ], ['to' => 'CDG']); + + $this->assertSame(200, $answer['status']); + $this->assertSame('morning/aisle', $answer['body']['result']['content'][0]['text']); + } + #[TestDox('carrying an answer to the next round without a signing key fails loudly')] public function testCarryingAnAnswerNeedsASigningKey(): void { From c60cdfd9946dcf2685b3f1dfc97fe35c02581170 Mon Sep 17 00:00:00 2001 From: BrocksiNet Date: Tue, 27 Oct 2026 08:34:39 +0100 Subject: [PATCH 02/10] [Server] Take the `requestState` binder as an argument of `setRequestState()` --- CHANGELOG.md | 2 +- docs/handlers/input-required.md | 7 ++--- docs/run/server-builder.md | 11 +++---- src/Server/Builder.php | 31 +++++++------------ .../Stateless/OriginatingRequestBinder.php | 2 +- .../Stateless/RequestStateBinderInterface.php | 2 +- .../Stateless/StatelessProtocolTest.php | 6 +--- 7 files changed, 22 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa6080939..50ec7e646 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * Fix a nullable tool parameter with a default (e.g. `?string $category = 'books'`) receiving the default when the client explicitly sends `null`: `ReferenceHandler` now passes the `null` through. * Fix a `subscriptions/listen` stream that can carry nothing (the server announces none of the requested types, or the client asks for none) being held open for the full subscription lifetime: it is acknowledged with an empty `notifications` filter and closed gracefully right away, instead of pinning a PHP-FPM worker. Adds `NotificationFilter::isEmpty()`. * [BC Break] Answer `server/discover` with `DiscoverHandler`, which a handler added through `Builder::addRequestHandler()` can replace, and dispatch `RequestEvent`, `ResponseEvent` and `ErrorEvent` for it. A `StatelessProtocol` built without the `Builder` needs `DiscoverHandler` and a `MessageFactory` with `DiscoverRequest` to answer it. -* Add `Builder::setRequestStateBinder()` binding the modern era's `requestState` to the request it was minted for, as the 2026-07-28 specification recommends: the binder's value is covered by the state's MAC, and a state echoed on a request that binds to another value is refused like a forged one. `OriginatingRequestBinder` binds the authenticated principal, the method, the tool, prompt or resource, and a digest of the arguments; `RequestStateBinderInterface` takes custom bindings. Without a binder, nothing changes. +* Add a `binder` argument to `Builder::setRequestState()` binding the modern era's `requestState` to the request it was minted for, as the 2026-07-28 specification recommends: the binder's value is covered by the state's MAC, and a state echoed on a request that binds to another value is refused like a forged one. `OriginatingRequestBinder` binds the authenticated principal, the method, the tool, prompt or resource, and a digest of the arguments; `RequestStateBinderInterface` takes custom bindings. Without a binder, nothing changes. ### Client diff --git a/docs/handlers/input-required.md b/docs/handlers/input-required.md index b9a5b6cee..78a97b51d 100644 --- a/docs/handlers/input-required.md +++ b/docs/handlers/input-required.md @@ -102,16 +102,15 @@ it is signed, not encrypted. A valid state verifies on any request of the TTL, so a client can echo it on another call, another tool, or under another user's token. The specification recommends binding it to the -authenticated principal and the originating request. `Builder::setRequestStateBinder()` does -that for the modern era: the binder's value is covered by the MAC of every state the request +authenticated principal and the originating request. The `binder` argument of +`Builder::setRequestState()` does that for the modern era: the binder's value is covered by the MAC of every state the request mints, and a retry that binds to another value is refused like a forged state. The value itself never goes on the wire. ```php use Mcp\Server\Stateless\OriginatingRequestBinder; -->setRequestState($_ENV['MCP_REQUEST_STATE_KEY'], ttl: 600) -->setRequestStateBinder(new OriginatingRequestBinder()) +->setRequestState($_ENV['MCP_REQUEST_STATE_KEY'], ttl: 600, binder: new OriginatingRequestBinder()) ``` `OriginatingRequestBinder` binds the method, the tool or prompt name and a digest of its diff --git a/docs/run/server-builder.md b/docs/run/server-builder.md index dd3169e9f..d2667e457 100644 --- a/docs/run/server-builder.md +++ b/docs/run/server-builder.md @@ -118,11 +118,9 @@ $server = Server::builder() // The same key must reach every process that might serve the retry. // The key needs at least 32 bytes, otherwise build() throws. TTL in seconds, default 600. // Without a key, the server refuses every state the client sends back. - ->setRequestState($_ENV['MCP_REQUEST_STATE_KEY'], ttl: 600) - - // Binds that state to the principal and the call it was minted for, so a - // client cannot echo it on another call or under another token. - ->setRequestStateBinder(new OriginatingRequestBinder()) + // The optional binder ties each state to the principal and the call it was + // minted for, so a client cannot echo it on another call or under another token. + ->setRequestState($_ENV['MCP_REQUEST_STATE_KEY'], ttl: 600, binder: new OriginatingRequestBinder()) // Bounds the input-required shim, which fulfils an `InputRequiredResult` // over a handshake-era connection. `withoutInputRequiredShim()` turns it off. @@ -338,8 +336,7 @@ $server = Server::builder() | `setProtocolVersion()` | protocolVersion | Pin the handshake to one protocol revision | | `setModernVersions()` | versions | Narrow the revisions the modern (2026-07-28) leg answers for | | `withoutModernEra()` | - | Serve the handshake era only | -| `setRequestState()` | key, ttl? | Signing key and lifetime for multi round-trip request state | -| `setRequestStateBinder()` | binder | Bind request state to the request and principal it was minted for | +| `setRequestState()` | key, ttl?, binder? | Signing key, lifetime and optional binding to the originating request for multi round-trip request state | | `setInputRequiredLimits()` | maxRounds, roundTimeout | Bound the input-required shim on handshake-era connections | | `withoutInputRequiredShim()` | - | Do not fulfil an `InputRequiredResult` over a handshake-era connection | | `setCachePolicy()` | policy | Set the `ttlMs`/`cacheScope` hints on cacheable results | diff --git a/src/Server/Builder.php b/src/Server/Builder.php index dfe706504..71a47f7c2 100644 --- a/src/Server/Builder.php +++ b/src/Server/Builder.php @@ -381,30 +381,21 @@ public function setCachePolicy(CachePolicy $policy): self * The same key must reach every instance that might serve the retry, so a * per-process random value only works for a single-process deployment. * - * @param string $key at least 32 bytes - * @param int $ttl how long a minted state stays valid, in seconds + * A binder ties every state the modern era mints to the request it was + * minted for, e.g. {@see Stateless\OriginatingRequestBinder} to the + * principal, method and arguments: a state echoed on a request that binds + * to another value fails verification like a forged one. States minted + * before a binder was configured no longer verify, so adding one fails the + * multi round-trip calls in flight for at most the TTL. + * + * @param string $key at least 32 bytes + * @param int $ttl how long a minted state stays valid, in seconds + * @param RequestStateBinderInterface|null $binder binds each state to its originating request */ - public function setRequestState(string $key, int $ttl = 600): self + public function setRequestState(string $key, int $ttl = 600, ?RequestStateBinderInterface $binder = null): self { $this->requestStateKey = $key; $this->requestStateTtl = $ttl; - - return $this; - } - - /** - * Binds every `requestState` the modern era mints to the request it was - * minted for, e.g. with {@see Stateless\OriginatingRequestBinder} - * to the principal, method and arguments: a state echoed on a request that - * binds to another value fails verification like a forged one. Has no - * effect without {@see self::setRequestState()}. - * - * States minted before a binder was configured no longer verify, so - * turning one on fails the multi round-trip calls in flight for at most - * the configured TTL. - */ - public function setRequestStateBinder(RequestStateBinderInterface $binder): self - { $this->requestStateBinder = $binder; return $this; diff --git a/src/Server/Stateless/OriginatingRequestBinder.php b/src/Server/Stateless/OriginatingRequestBinder.php index c9e257c46..938f9ddd1 100644 --- a/src/Server/Stateless/OriginatingRequestBinder.php +++ b/src/Server/Stateless/OriginatingRequestBinder.php @@ -32,7 +32,7 @@ * * @see https://modelcontextprotocol.io/specification/2026-07-28/basic/patterns/mrtr#server-requirements-basic-workflow * - * @author Björn Meyer + * @author Björn Meyer */ final class OriginatingRequestBinder implements RequestStateBinderInterface { diff --git a/src/Server/Stateless/RequestStateBinderInterface.php b/src/Server/Stateless/RequestStateBinderInterface.php index 228910648..6fc42abc0 100644 --- a/src/Server/Stateless/RequestStateBinderInterface.php +++ b/src/Server/Stateless/RequestStateBinderInterface.php @@ -32,7 +32,7 @@ * @see https://modelcontextprotocol.io/specification/2026-07-28/basic/patterns/mrtr#server-requirements-basic-workflow * @see OriginatingRequestBinder for the binding the specification recommends * - * @author Björn Meyer + * @author Björn Meyer */ interface RequestStateBinderInterface { diff --git a/tests/Unit/Server/Stateless/StatelessProtocolTest.php b/tests/Unit/Server/Stateless/StatelessProtocolTest.php index 514054057..9bb2202e5 100644 --- a/tests/Unit/Server/Stateless/StatelessProtocolTest.php +++ b/tests/Unit/Server/Stateless/StatelessProtocolTest.php @@ -611,11 +611,7 @@ static function (RequestContext $context): string { ); if ($signed) { - $builder->setRequestState(str_repeat('k', 32)); - } - - if (null !== $binder) { - $builder->setRequestStateBinder($binder); + $builder->setRequestState(str_repeat('k', 32), binder: $binder); } return $builder->buildStateless([ProtocolVersion::V2026_07_28]); From 39e2b80e9f8ea2af8a6ed33238831c210e646bd7 Mon Sep 17 00:00:00 2001 From: BrocksiNet Date: Tue, 27 Oct 2026 08:41:06 +0100 Subject: [PATCH 03/10] [Docs] Explain which `requestState` only the binder can bind --- docs/handlers/input-required.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/handlers/input-required.md b/docs/handlers/input-required.md index 78a97b51d..8fe74f5c9 100644 --- a/docs/handlers/input-required.md +++ b/docs/handlers/input-required.md @@ -101,11 +101,16 @@ it is signed, not encrypted. ### Binding the state to the request A valid state verifies on any request of the TTL, so a client can echo it on another call, -another tool, or under another user's token. The specification recommends binding it to the -authenticated principal and the originating request. The `binder` argument of -`Builder::setRequestState()` does that for the modern era: the binder's value is covered by the MAC of every state the request -mints, and a retry that binds to another value is refused like a forged state. The value -itself never goes on the wire. +another tool, or under another user's token. The +[specification](https://modelcontextprotocol.io/specification/2026-07-28/basic/patterns/mrtr#server-requirements-basic-workflow) +recommends binding it to the authenticated principal and the originating request. + +A handler that mints its own state can put that identity into the payload and compare it on +the retry. It cannot do so for the state the SDK mints when it carries answers to the next +round, as for a handler that calls `elicit()` twice. The `binder` argument of +`Builder::setRequestState()` binds both on the modern era: the binder's value is covered by +the MAC of every state the request mints, and a retry that binds to another value is refused +like a forged state. The value itself never goes on the wire. ```php use Mcp\Server\Stateless\OriginatingRequestBinder; From 0ab6a4d2f1682979914e3ff85d560b18ea525e78 Mon Sep 17 00:00:00 2001 From: BrocksiNet Date: Wed, 4 Nov 2026 11:33:15 +0100 Subject: [PATCH 04/10] [Server] Harden `OriginatingRequestBinder` against colliding principals and arguments Sorting a numeric-key map could turn it into a list, so `{"1": "B", "0": "A"}` and `["A", "B"]` bound to the same digest although the handler and schema validation see an object and an array. Lists stay arrays and every other array becomes an object. Without a subject, tokens of different users of one client shared a principal, so a state minted for one verified under the other. Such a request now binds to a value of its own, and a `principal` resolver identifies users by another claim. --- CHANGELOG.md | 2 +- docs/handlers/input-required.md | 19 ++++- .../Stateless/OriginatingRequestBinder.php | 67 +++++++++++++++-- .../OriginatingRequestBinderTest.php | 73 +++++++++++++++++++ 4 files changed, 150 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50ec7e646..72f4ffe98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * Fix a nullable tool parameter with a default (e.g. `?string $category = 'books'`) receiving the default when the client explicitly sends `null`: `ReferenceHandler` now passes the `null` through. * Fix a `subscriptions/listen` stream that can carry nothing (the server announces none of the requested types, or the client asks for none) being held open for the full subscription lifetime: it is acknowledged with an empty `notifications` filter and closed gracefully right away, instead of pinning a PHP-FPM worker. Adds `NotificationFilter::isEmpty()`. * [BC Break] Answer `server/discover` with `DiscoverHandler`, which a handler added through `Builder::addRequestHandler()` can replace, and dispatch `RequestEvent`, `ResponseEvent` and `ErrorEvent` for it. A `StatelessProtocol` built without the `Builder` needs `DiscoverHandler` and a `MessageFactory` with `DiscoverRequest` to answer it. -* Add a `binder` argument to `Builder::setRequestState()` binding the modern era's `requestState` to the request it was minted for, as the 2026-07-28 specification recommends: the binder's value is covered by the state's MAC, and a state echoed on a request that binds to another value is refused like a forged one. `OriginatingRequestBinder` binds the authenticated principal, the method, the tool, prompt or resource, and a digest of the arguments; `RequestStateBinderInterface` takes custom bindings. Without a binder, nothing changes. +* Add a `binder` argument to `Builder::setRequestState()` binding the modern era's `requestState` to the request it was minted for, as the 2026-07-28 specification recommends: the binder's value is covered by the state's MAC, and a state echoed on a request that binds to another value is refused like a forged one. `OriginatingRequestBinder` binds the authenticated principal, the method, the tool, prompt or resource, and a digest of the arguments; a token without a subject fails closed, and a `principal` resolver identifies users by another claim. `RequestStateBinderInterface` takes custom bindings. Without a binder, nothing changes. ### Client diff --git a/docs/handlers/input-required.md b/docs/handlers/input-required.md index 8fe74f5c9..5df361c91 100644 --- a/docs/handlers/input-required.md +++ b/docs/handlers/input-required.md @@ -119,8 +119,23 @@ use Mcp\Server\Stateless\OriginatingRequestBinder; ``` `OriginatingRequestBinder` binds the method, the tool or prompt name and a digest of its -arguments (or the resource URI), and the token's issuer, client and subject. For anything -else, implement `RequestStateBinderInterface`, which receives the request as the client sent +arguments (or the resource URI), and the token's issuer, client and subject. + +A token without a subject (`sub`) identifies no user, so the binder cannot tell two users of +one client apart. It fails closed: such a request binds to a value of its own, and a state it +mints is refused on the retry. If your authorization server identifies users by another claim, +pass a resolver that returns a value stable across a token refresh, or `null` for a token that +identifies nobody: + +```php +use Mcp\Server\Authorization\AccessToken; + +new OriginatingRequestBinder( + principal: static fn (AccessToken $token): ?string => \is_string($oid = $token->getClaim('oid')) ? $oid : null, +) +``` + +For anything else, implement `RequestStateBinderInterface`, which receives the request as the client sent it and its `AccessToken`. Bind only what stays the same across the rounds of one call; `_meta` does not. diff --git a/src/Server/Stateless/OriginatingRequestBinder.php b/src/Server/Stateless/OriginatingRequestBinder.php index 938f9ddd1..11e2f15b5 100644 --- a/src/Server/Stateless/OriginatingRequestBinder.php +++ b/src/Server/Stateless/OriginatingRequestBinder.php @@ -30,12 +30,27 @@ * binds to no principal, so its state does not verify on an authorized one, * nor the other way round. * + * A token without a subject identifies no user: tokens of different users of + * one client would share a principal. Such a request binds to a value of its + * own, so the states it mints never verify. Pass a `$principal` resolver when + * the authorization server identifies users by another claim. + * * @see https://modelcontextprotocol.io/specification/2026-07-28/basic/patterns/mrtr#server-requirements-basic-workflow * * @author Björn Meyer */ final class OriginatingRequestBinder implements RequestStateBinderInterface { + /** + * @param (\Closure(AccessToken): ?string)|null $principal identifies the user and client a token was issued + * for, stable across a token refresh; null when the token + * identifies nobody. Defaults to issuer, client and subject. + */ + public function __construct( + private readonly ?\Closure $principal = null, + ) { + } + public function bind(Request $request, ?AccessToken $accessToken): string { [$target, $arguments] = match (true) { @@ -49,17 +64,51 @@ public function bind(Request $request, ?AccessToken $accessToken): string 'method' => $request::getMethod(), 'target' => $target, 'arguments' => hash('sha256', json_encode(self::canonical($arguments), \JSON_THROW_ON_ERROR)), - 'principal' => null === $accessToken ? null : [ - 'iss' => \is_string($issuer = $accessToken->getClaim('iss')) ? $issuer : null, - 'client' => $accessToken->getClientId(), - 'sub' => $accessToken->getSubject(), - ], + 'principal' => null === $accessToken ? null : $this->principal($accessToken), + ], \JSON_THROW_ON_ERROR); + } + + /** + * @return array{identified: string}|array{unidentified: string} + */ + private function principal(AccessToken $accessToken): array + { + $principal = null === $this->principal ? self::defaultPrincipal($accessToken) : ($this->principal)($accessToken); + + if (null === $principal) { + return ['unidentified' => bin2hex(random_bytes(16))]; + } + + return ['identified' => $principal]; + } + + private static function defaultPrincipal(AccessToken $accessToken): ?string + { + $subject = $accessToken->getSubject(); + + if (null === $subject) { + return null; + } + + $issuer = $accessToken->getClaim('iss'); + + return json_encode([ + 'iss' => \is_string($issuer) ? $issuer : null, + 'client' => $accessToken->getClientId(), + 'sub' => $subject, ], \JSON_THROW_ON_ERROR); } /** * Sorts object members recursively, so a client re-encoding the same * arguments in another member order still binds to the same digest. + * + * A list stays a JSON array and any other array becomes a JSON object, + * the same split schema validation and the handler see: a map with + * numeric keys out of order, e.g. `{"1": "B", "0": "A"}`, must not sort + * into the list `["A", "B"]`. The arguments are decoded into PHP arrays, + * so a numeric-key object whose members arrive in list order is a list + * to the whole SDK and binds as one. */ private static function canonical(mixed $value): mixed { @@ -67,10 +116,12 @@ private static function canonical(mixed $value): mixed return $value; } - if (!array_is_list($value)) { - ksort($value, \SORT_STRING); + if (array_is_list($value)) { + return array_map(self::canonical(...), $value); } - return array_map(self::canonical(...), $value); + ksort($value, \SORT_STRING); + + return (object) array_map(self::canonical(...), $value); } } diff --git a/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php b/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php index f549bc6b2..7cc3fad94 100644 --- a/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php +++ b/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php @@ -79,6 +79,79 @@ public function testOtherRequestsBindElsewhere(Request $request, ?AccessToken $t $this->assertNotSame($binder->bind(self::call(), self::token()), $binder->bind($request, $token)); } + /** + * @return iterable + */ + public static function sameArguments(): iterable + { + yield 'nested member order' => ['{"trip": {"to": "BER", "from": "HAM"}}', '{"trip": {"from": "HAM", "to": "BER"}}']; + yield 'numeric-key map member order' => ['{"items": {"2": "C", "0": "A", "1": "B"}}', '{"items": {"1": "B", "2": "C", "0": "A"}}']; + yield 'list inside a map' => ['{"b": [1, 2], "a": {"x": [3]}}', '{"a": {"x": [3]}, "b": [1, 2]}']; + } + + #[DataProvider('sameArguments')] + #[TestDox('arguments that differ only in member order bind to the same value: $_dataName')] + public function testArgumentsInAnotherMemberOrderBindToTheSameValue(string $first, string $retry): void + { + $binder = new OriginatingRequestBinder(); + + $this->assertSame( + $binder->bind(self::call(arguments: json_decode($first, true, flags: \JSON_THROW_ON_ERROR)), self::token()), + $binder->bind(self::call(arguments: json_decode($retry, true, flags: \JSON_THROW_ON_ERROR)), self::token()), + ); + } + + /** + * @return iterable + */ + public static function otherArguments(): iterable + { + yield 'numeric-key map and list' => ['{"items": {"1": "B", "0": "A"}}', '{"items": ["A", "B"]}']; + yield 'list order' => ['{"items": ["A", "B"]}', '{"items": ["B", "A"]}']; + } + + #[DataProvider('otherArguments')] + #[TestDox('arguments the handler sees differently bind to other values: $_dataName')] + public function testArgumentsTheHandlerSeesDifferentlyBindElsewhere(string $first, string $retry): void + { + $binder = new OriginatingRequestBinder(); + + $this->assertNotSame( + $binder->bind(self::call(arguments: json_decode($first, true, flags: \JSON_THROW_ON_ERROR)), self::token()), + $binder->bind(self::call(arguments: json_decode($retry, true, flags: \JSON_THROW_ON_ERROR)), self::token()), + ); + } + + #[TestDox('a token without a subject fails closed: no two requests bind to the same value')] + public function testTokenWithoutSubjectBindsToAValueOfItsOwn(): void + { + $binder = new OriginatingRequestBinder(); + $token = new AccessToken(['read'], ['iss' => 'https://auth.example.com', 'client_id' => 'app', 'email' => 'alice@example.com']); + + $this->assertNotSame($binder->bind(self::call(), $token), $binder->bind(self::call(), $token)); + } + + #[TestDox('a principal resolver identifies users by another claim')] + public function testPrincipalResolverIdentifiesUsersByAnotherClaim(): void + { + $binder = new OriginatingRequestBinder( + static fn (AccessToken $token): ?string => \is_string($oid = $token->getClaim('oid')) ? $oid : null, + ); + $token = static fn (string $oid, string $jti): AccessToken => new AccessToken(['read'], ['client_id' => 'app', 'oid' => $oid, 'jti' => $jti]); + + $this->assertSame($binder->bind(self::call(), $token('alice', 'a')), $binder->bind(self::call(), $token('alice', 'b'))); + $this->assertNotSame($binder->bind(self::call(), $token('alice', 'a')), $binder->bind(self::call(), $token('bob', 'a'))); + $this->assertNotSame($binder->bind(self::call(), $token('alice', 'a')), $binder->bind(self::call(), null)); + } + + #[TestDox('a principal resolver that identifies nobody fails closed')] + public function testPrincipalResolverReturningNullFailsClosed(): void + { + $binder = new OriginatingRequestBinder(static fn (AccessToken $token): ?string => null); + + $this->assertNotSame($binder->bind(self::call(), self::token()), $binder->bind(self::call(), self::token())); + } + #[TestDox('a resource read binds to its URI')] public function testResourceReadBindsToItsUri(): void { From 6ba0af54e43f832d64f31772d7f05e4cd274dd2f Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Wed, 4 Nov 2026 23:17:22 +0100 Subject: [PATCH 05/10] [Server] Fail the request when the `requestState` binder throws --- .../Stateless/RequestStateBinderInterface.php | 3 ++- src/Server/Stateless/StatelessProtocol.php | 9 ++++++++- .../Server/Stateless/StatelessProtocolTest.php | 17 +++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/Server/Stateless/RequestStateBinderInterface.php b/src/Server/Stateless/RequestStateBinderInterface.php index 6fc42abc0..a612a382e 100644 --- a/src/Server/Stateless/RequestStateBinderInterface.php +++ b/src/Server/Stateless/RequestStateBinderInterface.php @@ -22,7 +22,8 @@ * covered by the MAC of every state minted while answering the request, and * recomputed to verify the state the client echoes on the retry. A state * presented on a request that binds to another value fails verification like - * a forged one. The value never goes on the wire. + * a forged one. The value never goes on the wire. A binder that throws fails + * the request with an internal error. * * Bind only what stays the same across the rounds of one call: the request * object carries neither `requestState` nor `inputResponses`, but `_meta` diff --git a/src/Server/Stateless/StatelessProtocol.php b/src/Server/Stateless/StatelessProtocol.php index 68fc95227..52e717fda 100644 --- a/src/Server/Stateless/StatelessProtocol.php +++ b/src/Server/Stateless/StatelessProtocol.php @@ -512,7 +512,14 @@ private function dispatch(string $method, array $decoded, RequestMeta $meta, str $requestStateCodec = $this->requestStateCodec; if (null !== $requestStateCodec && null !== $this->requestStateBinder) { - $requestStateCodec = $requestStateCodec->withBinding($this->requestStateBinder->bind($request, $accessToken)); + try { + $requestStateCodec = $requestStateCodec->withBinding($this->requestStateBinder->bind($request, $accessToken)); + } catch (\Throwable $e) { + // Fails closed: an unbound codec would mint and accept states the binder is there to refuse. + $this->logger->error('The requestState binder failed on a modern-era request.', ['method' => $method, 'exception' => $e]); + + return StatelessResult::error(Error::forInternalError(self::INTERNAL_ERROR_MESSAGE, $id), 500); + } } try { diff --git a/tests/Unit/Server/Stateless/StatelessProtocolTest.php b/tests/Unit/Server/Stateless/StatelessProtocolTest.php index 9bb2202e5..8e837a4f5 100644 --- a/tests/Unit/Server/Stateless/StatelessProtocolTest.php +++ b/tests/Unit/Server/Stateless/StatelessProtocolTest.php @@ -781,6 +781,23 @@ public function bind(Request $request, ?AccessToken $accessToken): string $this->assertSame([['books_flight', ['to' => 'BER'], 'alice']], $binder->calls); } + #[TestDox('a binder that throws fails the request with an internal error instead of escaping to the transport')] + public function testThrowingBinderFailsTheRequest(): void + { + $binder = new class implements RequestStateBinderInterface { + public function bind(Request $request, ?AccessToken $accessToken): string + { + throw new \RuntimeException('No principal claim on the token.'); + } + }; + + $answer = self::bookAs(self::twoAskProtocol(binder: $binder), self::alice(), arguments: ['to' => 'BER']); + + $this->assertSame(500, $answer['status']); + $this->assertSame(Error::INTERNAL_ERROR, $answer['body']['error']['code']); + $this->assertStringNotContainsString('principal', $answer['body']['error']['message']); + } + #[TestDox('a state minted before a binder was configured is refused once one is')] public function testUnboundStateRefusedByABindingServer(): void { From e3abcb8a851348c72ece5d1faa36e585e5bd402f Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Wed, 4 Nov 2026 23:23:59 +0100 Subject: [PATCH 06/10] [Server] Throw the SDK's `RuntimeException` when `OriginatingRequestBinder` cannot encode --- .../Stateless/OriginatingRequestBinder.php | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/Server/Stateless/OriginatingRequestBinder.php b/src/Server/Stateless/OriginatingRequestBinder.php index 11e2f15b5..667f6ae64 100644 --- a/src/Server/Stateless/OriginatingRequestBinder.php +++ b/src/Server/Stateless/OriginatingRequestBinder.php @@ -11,6 +11,7 @@ namespace Mcp\Server\Stateless; +use Mcp\Exception\RuntimeException; use Mcp\Schema\JsonRpc\Request; use Mcp\Schema\Request\CallToolRequest; use Mcp\Schema\Request\GetPromptRequest; @@ -60,12 +61,12 @@ public function bind(Request $request, ?AccessToken $accessToken): string default => [null, []], }; - return json_encode([ + return self::encode([ 'method' => $request::getMethod(), 'target' => $target, - 'arguments' => hash('sha256', json_encode(self::canonical($arguments), \JSON_THROW_ON_ERROR)), + 'arguments' => hash('sha256', self::encode(self::canonical($arguments))), 'principal' => null === $accessToken ? null : $this->principal($accessToken), - ], \JSON_THROW_ON_ERROR); + ]); } /** @@ -92,11 +93,20 @@ private static function defaultPrincipal(AccessToken $accessToken): ?string $issuer = $accessToken->getClaim('iss'); - return json_encode([ + return self::encode([ 'iss' => \is_string($issuer) ? $issuer : null, 'client' => $accessToken->getClientId(), 'sub' => $subject, - ], \JSON_THROW_ON_ERROR); + ]); + } + + private static function encode(mixed $value): string + { + try { + return json_encode($value, \JSON_THROW_ON_ERROR); + } catch (\JsonException $e) { + throw new RuntimeException(\sprintf('Failed to encode the requestState binding as JSON: %s', $e->getMessage()), previous: $e); + } } /** From e83136a30994bbc32ce30bf5973db5890915a4b0 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Wed, 4 Nov 2026 23:23:59 +0100 Subject: [PATCH 07/10] [Server] Pin that `1` and `1.0` bind to the same `requestState` --- src/Server/Stateless/OriginatingRequestBinder.php | 3 ++- tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Server/Stateless/OriginatingRequestBinder.php b/src/Server/Stateless/OriginatingRequestBinder.php index 667f6ae64..aca42520b 100644 --- a/src/Server/Stateless/OriginatingRequestBinder.php +++ b/src/Server/Stateless/OriginatingRequestBinder.php @@ -118,7 +118,8 @@ private static function encode(mixed $value): string * numeric keys out of order, e.g. `{"1": "B", "0": "A"}`, must not sort * into the list `["A", "B"]`. The arguments are decoded into PHP arrays, * so a numeric-key object whose members arrive in list order is a list - * to the whole SDK and binds as one. + * to the whole SDK and binds as one. Numbers bind by value: `1` and `1.0` + * encode alike, so a client re-encoding a number does not break its retry. */ private static function canonical(mixed $value): mixed { diff --git a/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php b/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php index 7cc3fad94..2d34d494d 100644 --- a/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php +++ b/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php @@ -87,11 +87,12 @@ public static function sameArguments(): iterable yield 'nested member order' => ['{"trip": {"to": "BER", "from": "HAM"}}', '{"trip": {"from": "HAM", "to": "BER"}}']; yield 'numeric-key map member order' => ['{"items": {"2": "C", "0": "A", "1": "B"}}', '{"items": {"1": "B", "2": "C", "0": "A"}}']; yield 'list inside a map' => ['{"b": [1, 2], "a": {"x": [3]}}', '{"a": {"x": [3]}, "b": [1, 2]}']; + yield 'integral float' => ['{"seats": 1}', '{"seats": 1.0}']; } #[DataProvider('sameArguments')] - #[TestDox('arguments that differ only in member order bind to the same value: $_dataName')] - public function testArgumentsInAnotherMemberOrderBindToTheSameValue(string $first, string $retry): void + #[TestDox('arguments that differ only in encoding bind to the same value: $_dataName')] + public function testArgumentsInAnotherEncodingBindToTheSameValue(string $first, string $retry): void { $binder = new OriginatingRequestBinder(); From 9f3a072757911ed0623cc2eddd15cc8faaca29af Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Wed, 4 Nov 2026 23:59:18 +0100 Subject: [PATCH 08/10] [Server] Keep NUL-prefixed argument names in the `requestState` binding --- .../Stateless/OriginatingRequestBinder.php | 18 +++++++++++++----- .../Stateless/OriginatingRequestBinderTest.php | 1 + 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/Server/Stateless/OriginatingRequestBinder.php b/src/Server/Stateless/OriginatingRequestBinder.php index aca42520b..b53b54ca6 100644 --- a/src/Server/Stateless/OriginatingRequestBinder.php +++ b/src/Server/Stateless/OriginatingRequestBinder.php @@ -113,10 +113,12 @@ private static function encode(mixed $value): string * Sorts object members recursively, so a client re-encoding the same * arguments in another member order still binds to the same digest. * - * A list stays a JSON array and any other array becomes a JSON object, - * the same split schema validation and the handler see: a map with - * numeric keys out of order, e.g. `{"1": "B", "0": "A"}`, must not sort - * into the list `["A", "B"]`. The arguments are decoded into PHP arrays, + * A list stays a JSON array and any other array becomes sorted pairs + * under `{"map": [[key, value], ...]}`, the same split schema validation + * and the handler see: a map with numeric keys out of order, e.g. + * `{"1": "B", "0": "A"}`, must not sort into the list `["A", "B"]`. Not + * cast to an object, as `json_encode()` drops object members whose name + * starts with NUL. The arguments are decoded into PHP arrays, * so a numeric-key object whose members arrive in list order is a list * to the whole SDK and binds as one. Numbers bind by value: `1` and `1.0` * encode alike, so a client re-encoding a number does not break its retry. @@ -133,6 +135,12 @@ private static function canonical(mixed $value): mixed ksort($value, \SORT_STRING); - return (object) array_map(self::canonical(...), $value); + $members = []; + + foreach ($value as $key => $member) { + $members[] = [(string) $key, self::canonical($member)]; + } + + return ['map' => $members]; } } diff --git a/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php b/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php index 2d34d494d..7d834ae0e 100644 --- a/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php +++ b/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php @@ -109,6 +109,7 @@ public static function otherArguments(): iterable { yield 'numeric-key map and list' => ['{"items": {"1": "B", "0": "A"}}', '{"items": ["A", "B"]}']; yield 'list order' => ['{"items": ["A", "B"]}', '{"items": ["B", "A"]}']; + yield 'member named with a leading NUL' => ['{"options": {"visible": true, "\\u0000mode": "preview"}}', '{"options": {"visible": true, "\\u0000mode": "execute"}}']; } #[DataProvider('otherArguments')] From 989a854fec486a16c8c5c4e34632853c831eb64a Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Thu, 5 Nov 2026 00:31:36 +0100 Subject: [PATCH 09/10] [Server] Encode the canonical arguments deep enough for any decoded request --- .../Stateless/OriginatingRequestBinder.php | 5 ++++- .../OriginatingRequestBinderTest.php | 21 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/Server/Stateless/OriginatingRequestBinder.php b/src/Server/Stateless/OriginatingRequestBinder.php index b53b54ca6..20d408339 100644 --- a/src/Server/Stateless/OriginatingRequestBinder.php +++ b/src/Server/Stateless/OriginatingRequestBinder.php @@ -42,6 +42,9 @@ */ final class OriginatingRequestBinder implements RequestStateBinderInterface { + // A canonical map nests three levels deep, and requests decode up to json_decode()'s default of 512. + private const ENCODE_DEPTH = 3 * 512; + /** * @param (\Closure(AccessToken): ?string)|null $principal identifies the user and client a token was issued * for, stable across a token refresh; null when the token @@ -103,7 +106,7 @@ private static function defaultPrincipal(AccessToken $accessToken): ?string private static function encode(mixed $value): string { try { - return json_encode($value, \JSON_THROW_ON_ERROR); + return json_encode($value, \JSON_THROW_ON_ERROR, self::ENCODE_DEPTH); } catch (\JsonException $e) { throw new RuntimeException(\sprintf('Failed to encode the requestState binding as JSON: %s', $e->getMessage()), previous: $e); } diff --git a/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php b/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php index 7d834ae0e..6e30cc558 100644 --- a/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php +++ b/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php @@ -154,6 +154,27 @@ public function testPrincipalResolverReturningNullFailsClosed(): void $this->assertNotSame($binder->bind(self::call(), self::token()), $binder->bind(self::call(), self::token())); } + #[TestDox('deeply nested arguments still bind, and their deepest leaf counts')] + public function testDeeplyNestedArgumentsBind(): void + { + $nested = static function (string $leaf): array { + $value = $leaf; + + for ($depth = 0; $depth < 300; ++$depth) { + $value = ['k' => $value]; + } + + return $value; + }; + + $binder = new OriginatingRequestBinder(); + + $this->assertNotSame( + $binder->bind(self::call(arguments: $nested('preview')), self::token()), + $binder->bind(self::call(arguments: $nested('execute')), self::token()), + ); + } + #[TestDox('a resource read binds to its URI')] public function testResourceReadBindsToItsUri(): void { From 514abc3b570a43cac02e589def1836c83df8d78c Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Thu, 5 Nov 2026 00:40:02 +0100 Subject: [PATCH 10/10] [Server] Keep issuer and client bound when a resolver names the `requestState` subject --- CHANGELOG.md | 2 +- docs/handlers/input-required.md | 4 +-- .../Stateless/OriginatingRequestBinder.php | 35 +++++++------------ .../OriginatingRequestBinderTest.php | 20 ++++++++--- 4 files changed, 32 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72f4ffe98..eabe8da0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * Fix a nullable tool parameter with a default (e.g. `?string $category = 'books'`) receiving the default when the client explicitly sends `null`: `ReferenceHandler` now passes the `null` through. * Fix a `subscriptions/listen` stream that can carry nothing (the server announces none of the requested types, or the client asks for none) being held open for the full subscription lifetime: it is acknowledged with an empty `notifications` filter and closed gracefully right away, instead of pinning a PHP-FPM worker. Adds `NotificationFilter::isEmpty()`. * [BC Break] Answer `server/discover` with `DiscoverHandler`, which a handler added through `Builder::addRequestHandler()` can replace, and dispatch `RequestEvent`, `ResponseEvent` and `ErrorEvent` for it. A `StatelessProtocol` built without the `Builder` needs `DiscoverHandler` and a `MessageFactory` with `DiscoverRequest` to answer it. -* Add a `binder` argument to `Builder::setRequestState()` binding the modern era's `requestState` to the request it was minted for, as the 2026-07-28 specification recommends: the binder's value is covered by the state's MAC, and a state echoed on a request that binds to another value is refused like a forged one. `OriginatingRequestBinder` binds the authenticated principal, the method, the tool, prompt or resource, and a digest of the arguments; a token without a subject fails closed, and a `principal` resolver identifies users by another claim. `RequestStateBinderInterface` takes custom bindings. Without a binder, nothing changes. +* Add a `binder` argument to `Builder::setRequestState()` binding the modern era's `requestState` to the request it was minted for, as the 2026-07-28 specification recommends: the binder's value is covered by the state's MAC, and a state echoed on a request that binds to another value is refused like a forged one. `OriginatingRequestBinder` binds the authenticated principal, the method, the tool, prompt or resource, and a digest of the arguments; a token without a subject fails closed, and a `subject` resolver identifies users by another claim. `RequestStateBinderInterface` takes custom bindings. Without a binder, nothing changes. ### Client diff --git a/docs/handlers/input-required.md b/docs/handlers/input-required.md index 5df361c91..83035b08e 100644 --- a/docs/handlers/input-required.md +++ b/docs/handlers/input-required.md @@ -125,13 +125,13 @@ A token without a subject (`sub`) identifies no user, so the binder cannot tell one client apart. It fails closed: such a request binds to a value of its own, and a state it mints is refused on the retry. If your authorization server identifies users by another claim, pass a resolver that returns a value stable across a token refresh, or `null` for a token that -identifies nobody: +identifies nobody. The issuer and client stay bound either way: ```php use Mcp\Server\Authorization\AccessToken; new OriginatingRequestBinder( - principal: static fn (AccessToken $token): ?string => \is_string($oid = $token->getClaim('oid')) ? $oid : null, + subject: static fn (AccessToken $token): ?string => \is_string($oid = $token->getClaim('oid')) ? $oid : null, ) ``` diff --git a/src/Server/Stateless/OriginatingRequestBinder.php b/src/Server/Stateless/OriginatingRequestBinder.php index 20d408339..1acd267fd 100644 --- a/src/Server/Stateless/OriginatingRequestBinder.php +++ b/src/Server/Stateless/OriginatingRequestBinder.php @@ -33,8 +33,9 @@ * * A token without a subject identifies no user: tokens of different users of * one client would share a principal. Such a request binds to a value of its - * own, so the states it mints never verify. Pass a `$principal` resolver when - * the authorization server identifies users by another claim. + * own, so the states it mints never verify. Pass a `$subject` resolver when + * the authorization server identifies users by another claim; the issuer + * and client stay part of the principal either way. * * @see https://modelcontextprotocol.io/specification/2026-07-28/basic/patterns/mrtr#server-requirements-basic-workflow * @@ -46,12 +47,12 @@ final class OriginatingRequestBinder implements RequestStateBinderInterface private const ENCODE_DEPTH = 3 * 512; /** - * @param (\Closure(AccessToken): ?string)|null $principal identifies the user and client a token was issued - * for, stable across a token refresh; null when the token - * identifies nobody. Defaults to issuer, client and subject. + * @param (\Closure(AccessToken): ?string)|null $subject identifies the user a token was issued for, stable + * across a token refresh; null when the token identifies + * nobody. Defaults to the `sub` claim. */ public function __construct( - private readonly ?\Closure $principal = null, + private readonly ?\Closure $subject = null, ) { } @@ -73,34 +74,24 @@ public function bind(Request $request, ?AccessToken $accessToken): string } /** - * @return array{identified: string}|array{unidentified: string} + * @return array{identified: array{iss: ?string, client: ?string, sub: string}}|array{unidentified: string} */ private function principal(AccessToken $accessToken): array { - $principal = null === $this->principal ? self::defaultPrincipal($accessToken) : ($this->principal)($accessToken); - - if (null === $principal) { - return ['unidentified' => bin2hex(random_bytes(16))]; - } - - return ['identified' => $principal]; - } - - private static function defaultPrincipal(AccessToken $accessToken): ?string - { - $subject = $accessToken->getSubject(); + $subject = null === $this->subject ? $accessToken->getSubject() : ($this->subject)($accessToken); if (null === $subject) { - return null; + return ['unidentified' => bin2hex(random_bytes(16))]; } $issuer = $accessToken->getClaim('iss'); - return self::encode([ + // Not up to the resolver: a state stays with the issuer and client it was minted for. + return ['identified' => [ 'iss' => \is_string($issuer) ? $issuer : null, 'client' => $accessToken->getClientId(), 'sub' => $subject, - ]); + ]]; } private static function encode(mixed $value): string diff --git a/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php b/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php index 6e30cc558..c5ee21f83 100644 --- a/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php +++ b/tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php @@ -133,8 +133,8 @@ public function testTokenWithoutSubjectBindsToAValueOfItsOwn(): void $this->assertNotSame($binder->bind(self::call(), $token), $binder->bind(self::call(), $token)); } - #[TestDox('a principal resolver identifies users by another claim')] - public function testPrincipalResolverIdentifiesUsersByAnotherClaim(): void + #[TestDox('a subject resolver identifies users by another claim')] + public function testSubjectResolverIdentifiesUsersByAnotherClaim(): void { $binder = new OriginatingRequestBinder( static fn (AccessToken $token): ?string => \is_string($oid = $token->getClaim('oid')) ? $oid : null, @@ -146,8 +146,20 @@ public function testPrincipalResolverIdentifiesUsersByAnotherClaim(): void $this->assertNotSame($binder->bind(self::call(), $token('alice', 'a')), $binder->bind(self::call(), null)); } - #[TestDox('a principal resolver that identifies nobody fails closed')] - public function testPrincipalResolverReturningNullFailsClosed(): void + #[TestDox('a subject resolver keeps the issuer and client in the principal')] + public function testSubjectResolverKeepsIssuerAndClient(): void + { + $binder = new OriginatingRequestBinder( + static fn (AccessToken $token): ?string => \is_string($oid = $token->getClaim('oid')) ? $oid : null, + ); + $token = static fn (string $issuer, string $clientId): AccessToken => new AccessToken(['read'], ['iss' => $issuer, 'client_id' => $clientId, 'oid' => 'alice']); + + $this->assertNotSame($binder->bind(self::call(), $token('https://auth.example.com', 'app')), $binder->bind(self::call(), $token('https://auth.example.com', 'other-app'))); + $this->assertNotSame($binder->bind(self::call(), $token('https://auth.example.com', 'app')), $binder->bind(self::call(), $token('https://other.example.com', 'app'))); + } + + #[TestDox('a subject resolver that identifies nobody fails closed')] + public function testSubjectResolverReturningNullFailsClosed(): void { $binder = new OriginatingRequestBinder(static fn (AccessToken $token): ?string => null);