Skip to content

Commit 9604e0e

Browse files
[Server] Bind Streamable HTTP sessions to the principal that opened them (#627)
* [Server] Bind Streamable HTTP sessions to the principal that opened them * [Server] Fix the session binding on unreadable and busy sessions A session opened without a principal now records it, so a session the store fails to read serves no one. Who opened a session is checked before waiting for its lock, so another principal can't tell the session is busy, and an initialize never takes the lock. * [Server] Derive the session principal from the access token Bind tokens without a subject to their issuer and client, and refuse an invalid principal attribute. * [Server] Answer 503 when the session owner cannot be read A failed read no longer sends the owner to re-initialize, and a store outage no longer escapes from a DELETE or a transport's shutdown. * [Server] Stop a claim that is not UTF-8 from failing the session principal * [Server] Stop refused requests from draining another principal's queue --------- Co-authored-by: Christopher Hertel <mail@christopher-hertel.de>
1 parent 5d026c4 commit 9604e0e

18 files changed

Lines changed: 976 additions & 74 deletions

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ All notable changes to `mcp/sdk` will be documented in this file.
4040
* [BC Break] A handler calling `\Fiber::suspend()` itself with anything but a `NotificationSuspension` or `RequestSuspension` fails the request without a scheduler, also when no event loop is involved. It used to be resumed with `null`.
4141
* Fix a request failing with `503`, and a resumed request losing its session writes, while another request of the session holds the session lock across an await: both wait for the lock on the event loop.
4242
* Fix a handler still running after its request was answered, e.g. without a fiber scheduler: it no longer fires `ResponseEvent` or `ErrorEvent`, what it throws is logged, and calling the `ClientGateway` throws.
43+
* [BC Break] Bind handshake-era sessions to the principal that opened them: `AccessToken::getPrincipal()`, or the `StreamableHttpTransport::PRINCIPAL_ATTRIBUTE` request attribute when set. Requests of another principal, and to sessions opened before the upgrade, are answered with `404`, and to a session the store cannot read with `503`. `Protocol::processInput()`, the transports' `handleMessage()`, `handleSessionEnd()`, `handlePostRequest()` and `handleDeleteRequest()`, and the `onMessage()` and `onSessionEnd()` listeners take a `?string $principal` as last argument.
44+
* [BC Break] Answer malformed JSON and an `initialize` sent with a session id with HTTP `400`, and a failure before the session is checked with `500`, instead of `200`. Such an answer no longer carries, or removes, the messages queued for the session in `Mcp-Session-Id`.
4345

4446
### Client
4547

‎docs/run/authorization.md‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,11 +313,27 @@ challenge a client steps up from. Use `$token->hasScope()` for decisions that de
313313
arguments beyond the tool name. With a `ScopePolicy`, the token's scopes include those implied by
314314
its hierarchy.
315315

316+
## Sessions and the Principal
317+
318+
A handshake-era session only serves the principal that opened it, as the
319+
[security best practices](https://modelcontextprotocol.io/specification/2025-11-25/basic/security_best_practices#session-hijacking)
320+
recommend. The principal is the issuer, client and subject of the access token. They stay the same
321+
when the client refreshes its token, so the session survives the refresh.
322+
323+
When another principal sends the session's `Mcp-Session-Id`, the server answers 404, as for an
324+
unknown session, and a `DELETE` does not end the session.
325+
326+
A token without a `sub` claim only identifies its issuer and client, so all tokens of one client share
327+
their sessions. If your authorization server identifies users by another claim, set the
328+
`StreamableHttpTransport::PRINCIPAL_ATTRIBUTE` request attribute from it. A session opened without a
329+
token only serves requests without a token.
330+
316331
## Authenticating Outside the SDK
317332

318333
If your application or framework already authenticates the request, skip `AuthorizationMiddleware`
319334
and hand the result to the transport as the PSR-7 request attribute `AccessToken::class`. Both HTTP
320-
transports read it from there, so handlers get it through `RequestContext::getAccessToken()` as usual:
335+
transports read it from there, so handlers get it through `RequestContext::getAccessToken()` as usual,
336+
and a session only serves the principal of the token that opened it:
321337

322338
```php
323339
use Mcp\Server\Authorization\AccessToken;
@@ -326,6 +342,16 @@ $request = $request->withAttribute(AccessToken::class, new AccessToken($scopes,
326342
$transport = new StreamableHttpTransport($request);
327343
```
328344

345+
If the claims do not identify the user, or there is no token, set the
346+
`StreamableHttpTransport::PRINCIPAL_ATTRIBUTE` attribute as well. It takes a non-empty string, so cast
347+
an integer user id. The server answers any other value with a 500:
348+
349+
```php
350+
use Mcp\Server\Transport\StreamableHttpTransport;
351+
352+
$request = $request->withAttribute(StreamableHttpTransport::PRINCIPAL_ATTRIBUTE, (string) $userId);
353+
```
354+
329355
## Custom Token Validators
330356

331357
Implement `AuthorizationTokenValidatorInterface` for other token formats, e.g. opaque tokens

‎src/Server/Authorization/AccessToken.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,16 @@ public function getClientId(): ?string
8181
return \is_string($clientId) ? $clientId : null;
8282
}
8383

84+
/**
85+
* Who the token identifies: its issuer, client and subject, which outlive a token refresh.
86+
*/
87+
public function getPrincipal(): string
88+
{
89+
$issuer = $this->getClaim('iss');
90+
91+
return serialize([\is_string($issuer) ? $issuer : null, $this->getClientId(), $this->getSubject()]);
92+
}
93+
8494
/**
8595
* Never serialized: the token must not outlive its request in a session store.
8696
*/

‎src/Server/Protocol.php‎

Lines changed: 134 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ class Protocol
7474

7575
public const SESSION_LOGGING_LEVEL = '_mcp.logging_level';
7676

77+
private const SESSION_NOT_FOUND_MESSAGE = 'Session not found or has expired.';
78+
79+
/** Session key for the fingerprint of the principal that opened the session, null when it was opened without one */
80+
private const SESSION_PRINCIPAL = '_mcp.principal';
81+
7782
/**
7883
* Deliberately generic: unexpected throwables carry internal details such as file paths, class
7984
* names and argument types, which must not be handed to the peer. The full exception is logged.
@@ -147,7 +152,7 @@ public function connect(TransportInterface $transport): void
147152
{
148153
$transport->onMessage($this->processInput(...));
149154

150-
$transport->onSessionEnd($this->destroySession(...));
155+
$transport->onSessionEnd($this->endSession(...));
151156

152157
// The transport keeps these callbacks, so they reference it weakly to not keep it alive.
153158
$transportRef = \WeakReference::create($transport);
@@ -175,13 +180,14 @@ public function connect(TransportInterface $transport): void
175180
*
176181
* @param TransportInterface<mixed> $transport
177182
* @param AccessToken|null $accessToken the token the input was authorized with, if the transport authorizes
183+
* @param string|null $principal who the input was authenticated as, if the transport authenticates
178184
*/
179-
public function processInput(TransportInterface $transport, string $input, ?Uuid $sessionId, ?AccessToken $accessToken = null): void
185+
public function processInput(TransportInterface $transport, string $input, ?Uuid $sessionId, ?AccessToken $accessToken = null, ?string $principal = null): void
180186
{
181187
// Last line of defense: a malformed message must never escape as a PHP error and take the
182188
// server process down.
183189
try {
184-
$this->doProcessInput($transport, $input, $sessionId, $accessToken);
190+
$this->doProcessInput($transport, $input, $sessionId, $accessToken, $principal);
185191
} catch (\Throwable $e) {
186192
$this->logger->error(\sprintf('Uncaught exception while processing input: %s', $e->getMessage()), ['exception' => $e]);
187193

@@ -192,7 +198,7 @@ public function processInput(TransportInterface $transport, string $input, ?Uuid
192198
}
193199

194200
try {
195-
$this->sendResponse($transport, Error::forInternalError(self::INTERNAL_ERROR_MESSAGE, $id), null);
201+
$this->sendResponse($transport, Error::forInternalError(self::INTERNAL_ERROR_MESSAGE, $id), null, ['status_code' => 500]);
196202
} catch (\Throwable $e) {
197203
$this->logger->error(\sprintf('Failed to send internal error response: %s', $e->getMessage()), ['exception' => $e]);
198204
}
@@ -226,7 +232,7 @@ private static function findResponseId(string $input): string|int|null
226232
/**
227233
* @param TransportInterface<mixed> $transport
228234
*/
229-
private function doProcessInput(TransportInterface $transport, string $input, ?Uuid $sessionId, ?AccessToken $accessToken): void
235+
private function doProcessInput(TransportInterface $transport, string $input, ?Uuid $sessionId, ?AccessToken $accessToken, ?string $principal): void
230236
{
231237
$this->logger->info('Received message to process.');
232238
$this->logger->debug('Received message payload.', ['message' => $input]);
@@ -238,7 +244,8 @@ private function doProcessInput(TransportInterface $transport, string $input, ?U
238244
} catch (\JsonException $e) {
239245
$this->logger->warning('Failed to decode json message.', ['exception' => $e]);
240246
$error = Error::forParseError($e->getMessage());
241-
$this->sendResponse($transport, $error, null);
247+
// The status keeps a refusal from carrying the queue of a session nobody checked yet, here and below.
248+
$this->sendResponse($transport, $error, null, ['status_code' => 400]);
242249

243250
return;
244251
} catch (InvalidInputMessageException $e) {
@@ -247,13 +254,29 @@ private function doProcessInput(TransportInterface $transport, string $input, ?U
247254
}
248255

249256
// Taken before the session is read and held until it is saved, so no other request of
250-
// the session reads a state this one is about to replace. A session created here for
251-
// an `initialize` is not known to anyone else yet and needs no lock, and an unknown
252-
// session id is refused by resolveSession() without one; the session is looked up
253-
// again under the lock, as it may have ended while this request waited.
254-
$takesLock = null !== $sessionId && null !== $this->sessionLock && $this->sessionManager->exists($sessionId);
257+
// the session reads a state this one is about to replace. An `initialize` never reads an
258+
// existing session and an unknown session id is refused by resolveSession(), so neither
259+
// needs the lock. The session is looked up again under the lock, as it may have ended
260+
// while this request waited.
261+
$takesLock = null !== $sessionId && null !== $this->sessionLock && !$message instanceof InitializeRequest && $this->sessionManager->exists($sessionId);
255262

256263
if ($takesLock) {
264+
// Who opened the session is checked before waiting for the lock too, so that another
265+
// principal can neither tell the session is busy nor keep it busy.
266+
try {
267+
$serves = $this->servesPrincipal($this->sessionManager->createWithId($sessionId), $principal);
268+
} catch (SessionStoreException $e) {
269+
$this->refuseUnavailableStore($transport, $sessionId, self::findResponseId($input), $e);
270+
271+
return;
272+
}
273+
274+
if (!$serves) {
275+
$this->refuseOtherPrincipal($transport, $sessionId);
276+
277+
return;
278+
}
279+
257280
try {
258281
$refusal = $this->acquireSessionLock($sessionId) ? null : Error::forServerError(self::SESSION_BUSY_MESSAGE, self::findResponseId($input));
259282
} catch (SessionLockException) {
@@ -268,7 +291,7 @@ private function doProcessInput(TransportInterface $transport, string $input, ?U
268291
}
269292

270293
try {
271-
$this->handleMessage($transport, $sessionId, $message, $accessToken);
294+
$this->handleMessage($transport, $sessionId, $message, $accessToken, $principal);
272295
} finally {
273296
if ($takesLock) {
274297
$this->releaseSessionLock($sessionId);
@@ -279,9 +302,9 @@ private function doProcessInput(TransportInterface $transport, string $input, ?U
279302
/**
280303
* @param TransportInterface<mixed> $transport
281304
*/
282-
private function handleMessage(TransportInterface $transport, ?Uuid $sessionId, MessageInterface|InvalidInputMessageException $message, ?AccessToken $accessToken): void
305+
private function handleMessage(TransportInterface $transport, ?Uuid $sessionId, MessageInterface|InvalidInputMessageException $message, ?AccessToken $accessToken, ?string $principal): void
283306
{
284-
$session = $this->resolveSession($transport, $sessionId, $message);
307+
$session = $this->resolveSession($transport, $sessionId, $message, $principal);
285308
if (null === $session) {
286309
return;
287310
}
@@ -1107,19 +1130,22 @@ private function errorForThrowable(\Throwable $e, Request $request, SessionInter
11071130
*
11081131
* @param TransportInterface<mixed> $transport
11091132
* @param Uuid|null $sessionId The session ID from the transport
1133+
* @param string|null $principal Who the messages were authenticated as
11101134
*/
1111-
private function resolveSession(TransportInterface $transport, ?Uuid $sessionId, MessageInterface|InvalidInputMessageException $message): ?SessionInterface
1135+
private function resolveSession(TransportInterface $transport, ?Uuid $sessionId, MessageInterface|InvalidInputMessageException $message, ?string $principal): ?SessionInterface
11121136
{
11131137
if ($message instanceof InitializeRequest) {
11141138
// Spec: An initialize request must not have a session ID.
11151139
if ($sessionId) {
11161140
$error = Error::forInvalidRequest('A session ID MUST NOT be sent with an "initialize" request.');
1117-
$this->sendResponse($transport, $error, null);
1141+
$this->sendResponse($transport, $error, null, ['status_code' => 400]);
11181142

11191143
return null;
11201144
}
11211145

11221146
$session = $this->sessionManager->create();
1147+
$session->set(self::SESSION_PRINCIPAL, self::fingerprint($principal));
1148+
11231149
$this->logger->debug('Created new session for initialize', [
11241150
'session_id' => $session->getId()->toRfc4122(),
11251151
]);
@@ -1140,25 +1166,88 @@ private function resolveSession(TransportInterface $transport, ?Uuid $sessionId,
11401166
try {
11411167
$exists = $this->sessionManager->exists($sessionId);
11421168
} catch (SessionStoreException $e) {
1143-
// Not a 404: the session may well exist, so the client should retry instead of re-initializing.
1144-
$this->logger->error('Session store is unavailable.', [
1145-
'session_id' => $sessionId->toRfc4122(),
1146-
'exception' => $e,
1147-
]);
1148-
$error = Error::forInternalError('Session store is unavailable.', self::findRequestId($message));
1149-
$this->sendResponse($transport, $error, null, ['status_code' => 503]);
1169+
$this->refuseUnavailableStore($transport, $sessionId, self::findRequestId($message), $e);
11501170

11511171
return null;
11521172
}
11531173

11541174
if (!$exists) {
1155-
$error = Error::forInvalidRequest('Session not found or has expired.');
1175+
$error = Error::forInvalidRequest(self::SESSION_NOT_FOUND_MESSAGE);
11561176
$this->sendResponse($transport, $error, null, ['status_code' => 404]);
11571177

11581178
return null;
11591179
}
11601180

1161-
return $this->sessionManager->createWithId($sessionId);
1181+
$session = $this->sessionManager->createWithId($sessionId);
1182+
1183+
try {
1184+
$serves = $this->servesPrincipal($session, $principal);
1185+
} catch (SessionStoreException $e) {
1186+
$this->refuseUnavailableStore($transport, $sessionId, self::findRequestId($message), $e);
1187+
1188+
return null;
1189+
}
1190+
1191+
if (!$serves) {
1192+
$this->refuseOtherPrincipal($transport, $sessionId);
1193+
1194+
return null;
1195+
}
1196+
1197+
return $session;
1198+
}
1199+
1200+
/**
1201+
* Whether the session serves the principal: the one that opened it, or no principal for a session opened without one.
1202+
*
1203+
* A session that does not record who opened it serves no one.
1204+
*
1205+
* @throws SessionStoreException when the session exists but its data cannot be read
1206+
*/
1207+
private function servesPrincipal(SessionInterface $session, ?string $principal): bool
1208+
{
1209+
if ($session->get(self::SESSION_PRINCIPAL, false) === self::fingerprint($principal)) {
1210+
return true;
1211+
}
1212+
1213+
if ([] === $session->all()) {
1214+
throw new SessionStoreException(\sprintf('Session "%s" could not be read.', $session->getId()->toRfc4122()));
1215+
}
1216+
1217+
return false;
1218+
}
1219+
1220+
/**
1221+
* Not a 404: the session may well exist, so the client should retry instead of re-initializing.
1222+
*
1223+
* @param TransportInterface<mixed> $transport
1224+
*/
1225+
private function refuseUnavailableStore(TransportInterface $transport, Uuid $sessionId, string|int|null $requestId, SessionStoreException $exception): void
1226+
{
1227+
$this->logger->error('Session store is unavailable.', [
1228+
'session_id' => $sessionId->toRfc4122(),
1229+
'exception' => $exception,
1230+
]);
1231+
$this->sendResponse($transport, Error::forInternalError('Session store is unavailable.', $requestId), null, ['status_code' => 503]);
1232+
}
1233+
1234+
/**
1235+
* @param TransportInterface<mixed> $transport
1236+
*/
1237+
private function refuseOtherPrincipal(TransportInterface $transport, Uuid $sessionId): void
1238+
{
1239+
$this->logger->warning('Refused a request of another principal than the one that opened the session.', [
1240+
'session_id' => $sessionId->toRfc4122(),
1241+
]);
1242+
1243+
// Answered like an unknown session, so that another principal cannot tell the session exists.
1244+
$error = Error::forInvalidRequest(self::SESSION_NOT_FOUND_MESSAGE);
1245+
$this->sendResponse($transport, $error, null, ['status_code' => 404]);
1246+
}
1247+
1248+
private static function fingerprint(?string $principal): ?string
1249+
{
1250+
return null === $principal ? null : hash('sha256', $principal);
11621251
}
11631252

11641253
/**
@@ -1173,6 +1262,26 @@ private static function findRequestId(MessageInterface|InvalidInputMessageExcept
11731262
};
11741263
}
11751264

1265+
/**
1266+
* Ends a session at the request of a client, which must be the principal that opened it.
1267+
*
1268+
* @return bool false when the session does not exist, or is another principal's
1269+
*
1270+
* @throws SessionStoreException when who opened the session cannot be established; the session is not ended
1271+
*/
1272+
private function endSession(Uuid $sessionId, ?string $principal = null): bool
1273+
{
1274+
if ($this->sessionManager->exists($sessionId) && !$this->servesPrincipal($this->sessionManager->createWithId($sessionId), $principal)) {
1275+
$this->logger->warning('Refused to end a session for another principal than the one that opened it.', [
1276+
'session_id' => $sessionId->toRfc4122(),
1277+
]);
1278+
1279+
return false;
1280+
}
1281+
1282+
return $this->destroySession($sessionId);
1283+
}
1284+
11761285
/**
11771286
* Destroy a specific session.
11781287
*

‎src/Server/Transport/BaseTransport.php‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Mcp\Server\Transport;
1313

14+
use Mcp\Exception\SessionStoreException;
1415
use Mcp\Schema\JsonRpc\Error;
1516
use Mcp\Schema\JsonRpc\Response;
1617
use Mcp\Server\Authorization\AccessToken;
@@ -166,20 +167,34 @@ protected function resumeHandlerFiber(\Fiber $fiber, ?Uuid $sessionId, Response|
166167
}
167168
}
168169

169-
protected function handleMessage(string $payload, ?Uuid $sessionId, ?AccessToken $accessToken = null): void
170+
protected function handleMessage(string $payload, ?Uuid $sessionId, ?AccessToken $accessToken = null, ?string $principal = null): void
170171
{
171172
if (\is_callable($this->messageListener)) {
172-
($this->messageListener)($this, $payload, $sessionId, $accessToken);
173+
($this->messageListener)($this, $payload, $sessionId, $accessToken, $principal);
174+
}
175+
}
176+
177+
/**
178+
* Ends the session of a transport shutting down, which has no client to report a store failure to.
179+
*/
180+
protected function endSessionOnShutdown(?Uuid $sessionId): void
181+
{
182+
try {
183+
$this->handleSessionEnd($sessionId);
184+
} catch (SessionStoreException $e) {
185+
$this->logger->error('Session store is unavailable, the session was not ended.', ['exception' => $e]);
173186
}
174187
}
175188

176189
/**
177190
* @return bool false when the listener reported that the session does not exist
191+
*
192+
* @throws SessionStoreException when the listener cannot tell who opened the session
178193
*/
179-
protected function handleSessionEnd(?Uuid $sessionId): bool
194+
protected function handleSessionEnd(?Uuid $sessionId, ?string $principal = null): bool
180195
{
181196
if ($sessionId && \is_callable($this->sessionEndListener)) {
182-
return false !== ($this->sessionEndListener)($sessionId);
197+
return false !== ($this->sessionEndListener)($sessionId, $principal);
183198
}
184199

185200
return true;

0 commit comments

Comments
 (0)