feat: give every MCP client its own token, rate limit and log name

This commit is contained in:
fromme committed 2026-10-08 12:37:49 +02:00
1 parent 3af41c06ab
commit bfdedefae3
8 files changed
+134 -33

No files matched your search

+3 -2
View File
@@ -85,8 +85,9 @@ APP_CMS_API_BASE_URL="https://www.ep-reisen.de/"
# TYPO3 API key (X-Api-Key), sent by the typo3.client scoped client and by app:content:sync; # TYPO3 API key (X-Api-Key), sent by the typo3.client scoped client and by app:content:sync;
# the real value per stage goes in .env.local # the real value per stage goes in .env.local
APP_CMS_API_KEY= APP_CMS_API_KEY=
# Static bearer token n8n sends to the chatbot MCP endpoint (/mcp); empty refuses every request # Bearer tokens for the chatbot MCP endpoint (/mcp), one per client: "name:token,name:token"
APP_MCP_TOKEN= # (e.g. "n8n:…,claude-desktop:…"); empty refuses every request
APP_MCP_TOKENS=
APP_WEBSITE_BASE_URL="https://www.ep-reisen.de/" APP_WEBSITE_BASE_URL="https://www.ep-reisen.de/"
APP_BASE_URL="https://my.ep-reisen.de" APP_BASE_URL="https://my.ep-reisen.de"
APP_CUSTOMER_SERVICE_EMAIL="[email protected]" APP_CUSTOMER_SERVICE_EMAIL="[email protected]"
+1 -1
View File
@@ -17,7 +17,7 @@ framework:
log: true log: true
rate_limiter: rate_limiter:
# Per MCP token. A chatbot turn makes a few tool calls; this catches runaway loops. # Per MCP client. A chatbot turn makes a few tool calls; this catches runaway loops.
chatbot_mcp: chatbot_mcp:
policy: sliding_window policy: sliding_window
limit: 60 limit: 60
+3 -3
View File
@@ -15,11 +15,11 @@ security:
mailjet: mailjet:
password: '%env(MAILJET_WEBHOOK_BASIC_PASSWORD)%' password: '%env(MAILJET_WEBHOOK_BASIC_PASSWORD)%'
roles: [ ROLE_MAILJET_WEBHOOK ] roles: [ ROLE_MAILJET_WEBHOOK ]
# Never consulted: McpAccessTokenHandler builds the user from the client name its
# token belongs to. Named only because a firewall must have a provider.
mcp_provider: mcp_provider:
memory: memory:
users: users: []
n8n:
roles: [ ROLE_CHATBOT_MCP ]
role_hierarchy: role_hierarchy:
# ROLE_CUSTOMER_EXPERT gates the booking-draft and log surfaces, which used to be # ROLE_CUSTOMER_EXPERT gates the booking-draft and log surfaces, which used to be
+1 -1
View File
@@ -340,7 +340,7 @@ services:
App\Security\McpAccessTokenHandler: App\Security\McpAccessTokenHandler:
arguments: arguments:
$token: '%env(APP_MCP_TOKEN)%' $tokens: '%env(APP_MCP_TOKENS)%'
App\Service\CmsDataProvider: App\Service\CmsDataProvider:
arguments: arguments:
+12 -7
View File
@@ -15,19 +15,24 @@ TYPO3 sends no dates or prices, and nothing flows back to TYPO3.
`POST /mcp` (Streamable HTTP; `GET`/`DELETE` are part of the transport), served by `symfony/mcp-bundle` `POST /mcp` (Streamable HTTP; `GET`/`DELETE` are part of the transport), served by `symfony/mcp-bundle`
from `config/packages/mcp.yaml`. Tools only, no resources or prompts. from `config/packages/mcp.yaml`. Tools only, no resources or prompts.
Authentication is a static bearer token, `Authorization: Bearer <APP_MCP_TOKEN>`, checked by Authentication is a static bearer token per client, `Authorization: Bearer <token>`, checked by
`App\Security\McpAccessTokenHandler` on the `mcp` firewall. If `APP_MCP_TOKEN` is empty, every request is `App\Security\McpAccessTokenHandler` on the `mcp` firewall. `APP_MCP_TOKENS` lists the clients as
refused. It is not the OAuth2 client-credentials flow of `/api`: n8n's MCP Client Tool node sends a fixed `name:token` pairs separated by commas, e.g. `APP_MCP_TOKENS="n8n:3f9c…,claude-desktop:a71e…"`. Give every
system its own token, so one can be revoked by removing its entry without touching the others. Use tokens
without commas, e.g. `openssl rand -hex 32`. A malformed list (no name, a duplicate name, a token shared by
two clients) fails loudly with a 500 on `/mcp` rather than locking a client out silently. If the list is
empty, every request is refused. It is not the OAuth2 client-credentials flow of `/api`: n8n's MCP Client Tool node sends a fixed
header and cannot renew a token every 10 minutes. header and cannot renew a token every 10 minutes.
Tool calls are rate-limited per token (`chatbot_mcp` limiter, 60 per minute). The limit is there to stop The client name becomes the authenticated user. Tool calls are rate-limited per client (`chatbot_mcp`
limiter, 60 per minute), so testing from Claude Desktop does not eat into n8n's budget. The limit is there to stop
runaway agent loops. Each call is logged on the `chatbot` channel (`var/log/<env>.chatbot.log`, file runaway agent loops. Each call is logged on the `chatbot` channel (`var/log/<env>.chatbot.log`, file
only) with the tool, arguments, result count and duration. Nothing identifies the person chatting. only) with the client, tool, arguments, result count and duration. Nothing identifies the person chatting.
### n8n ### n8n
MCP Client Tool node: endpoint `https://<host>/mcp`, transport *HTTP Streamable*, authentication *Bearer* MCP Client Tool node: endpoint `https://<host>/mcp`, transport *HTTP Streamable*, authentication *Bearer*
with the stage's `APP_MCP_TOKEN`. The agent prompt should tell the bot to cite each answer's `url`, and with n8n's own token from the stage's `APP_MCP_TOKENS`. The agent prompt should tell the bot to cite each answer's `url`, and
that prices and availability are a snapshot (`dataAsOf`) and only the booking page is binding. that prices and availability are a snapshot (`dataAsOf`) and only the booking page is binding.
## Tools ## Tools
@@ -109,4 +114,4 @@ After a complete run that changed something, the codes the trip join cannot plac
|---|---| |---|---|
| `APP_CMS_API_BASE_URL` | TYPO3 base URL per stage (also used by `CmsDataProvider`) | | `APP_CMS_API_BASE_URL` | TYPO3 base URL per stage (also used by `CmsDataProvider`) |
| `APP_CMS_API_KEY` | TYPO3 `X-Api-Key`: the content feed and its `contentUrl` pages, and `typo3.client` (`CmsDataProvider`) | | `APP_CMS_API_KEY` | TYPO3 `X-Api-Key`: the content feed and its `contentUrl` pages, and `typo3.client` (`CmsDataProvider`) |
| `APP_MCP_TOKEN` | Bearer token n8n sends to `/mcp` | | `APP_MCP_TOKENS` | Bearer tokens for `/mcp`, one per client: `name:token,name:token` |
+9 -6
View File
@@ -12,12 +12,13 @@ use Symfony\Bundle\SecurityBundle\Security;
use Symfony\Component\RateLimiter\RateLimiterFactory; use Symfony\Component\RateLimiter\RateLimiterFactory;
/** /**
* What every chatbot tool call goes through: the per-token rate limit, the call log, and * What every chatbot tool call goes through: the per-client rate limit, the call log, and
* the answer as compact JSON. * the answer as compact JSON.
* *
* The SDK would pretty-print an array result and repeat it as structured content; both * The SDK would pretty-print an array result and repeat it as structured content; both
* cost the model tokens and neither helps n8n. The log records tool, arguments, result * cost the model tokens and neither helps n8n. The log records tool, arguments, result
* count and duration, never anything about the person chatting, which n8n does not send. * count, duration and the calling client (n8n, claude-desktop, …), never anything about
* the person chatting, which n8n does not send.
*/ */
class ToolCallRunner class ToolCallRunner
{ {
@@ -37,10 +38,11 @@ class ToolCallRunner
public function run(string $tool, array $arguments, callable $call): CallToolResult public function run(string $tool, array $arguments, callable $call): CallToolResult
{ {
$arguments = array_filter($arguments, static fn (mixed $value): bool => null !== $value); $arguments = array_filter($arguments, static fn (mixed $value): bool => null !== $value);
$limit = $this->chatbotMcpLimiter->create($this->security->getUser()?->getUserIdentifier() ?? 'anonymous')->consume(); $client = $this->security->getUser()?->getUserIdentifier() ?? 'anonymous';
$limit = $this->chatbotMcpLimiter->create($client)->consume();
if (!$limit->isAccepted()) { if (!$limit->isAccepted()) {
$this->chatbotLogger->warning('MCP tool call rate-limited', ['tool' => $tool, 'arguments' => $arguments]); $this->chatbotLogger->warning('MCP tool call rate-limited', ['client' => $client, 'tool' => $tool, 'arguments' => $arguments]);
throw new ToolCallException(sprintf('Too many tool calls. Retry after %s.', $limit->getRetryAfter()->format('H:i:s'))); throw new ToolCallException(sprintf('Too many tool calls. Retry after %s.', $limit->getRetryAfter()->format('H:i:s')));
} }
@@ -50,16 +52,17 @@ class ToolCallRunner
try { try {
$result = $call(); $result = $call();
} catch (ToolCallException $e) { } catch (ToolCallException $e) {
$this->chatbotLogger->info('MCP tool call rejected', ['tool' => $tool, 'arguments' => $arguments, 'error' => $e->getMessage()]); $this->chatbotLogger->info('MCP tool call rejected', ['client' => $client, 'tool' => $tool, 'arguments' => $arguments, 'error' => $e->getMessage()]);
throw $e; throw $e;
} catch (\Throwable $e) { } catch (\Throwable $e) {
$this->chatbotLogger->error('MCP tool call failed', ['tool' => $tool, 'arguments' => $arguments, 'exception' => $e]); $this->chatbotLogger->error('MCP tool call failed', ['client' => $client, 'tool' => $tool, 'arguments' => $arguments, 'exception' => $e]);
throw $e; throw $e;
} }
$this->chatbotLogger->info('MCP tool call', [ $this->chatbotLogger->info('MCP tool call', [
'client' => $client,
'tool' => $tool, 'tool' => $tool,
'arguments' => $arguments, 'arguments' => $arguments,
'resultCount' => is_array($result['results'] ?? null) ? count($result['results']) : 1, 'resultCount' => is_array($result['results'] ?? null) ? count($result['results']) : 1,
+60 -7
View File
@@ -5,33 +5,86 @@ declare(strict_types=1);
namespace App\Security; namespace App\Security;
use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Core\Exception\BadCredentialsException;
use Symfony\Component\Security\Core\User\InMemoryUser;
use Symfony\Component\Security\Http\AccessToken\AccessTokenHandlerInterface; use Symfony\Component\Security\Http\AccessToken\AccessTokenHandlerInterface;
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
/** /**
* Checks the static bearer token n8n sends to the chatbot MCP endpoint. * Checks the static bearer tokens of the systems calling the chatbot MCP endpoint.
* *
* The data behind it is public, but the endpoint must not become a free LLM-tooling API * The data behind it is public, but the endpoint must not become a free LLM-tooling API
* for whoever finds it. A static token rather than the OAuth2 client-credentials grant of * for whoever finds it. Static tokens rather than the OAuth2 client-credentials grant of
* /api: n8n's MCP Client Tool node sends a fixed header and cannot refresh a 10-minute token. * /api: n8n's MCP Client Tool node sends a fixed header and cannot refresh a 10-minute token.
* An unconfigured token refuses every request. *
* Every client has its own token, configured as `name:token` pairs separated by commas
* (APP_MCP_TOKENS="n8n:3f9c…,claude-desktop:a71e…"), so one can be revoked without the
* others. The name becomes the authenticated user: each client gets its own rate-limit
* bucket and is named in the tool-call log. An empty list refuses every request.
*/ */
final class McpAccessTokenHandler implements AccessTokenHandlerInterface final class McpAccessTokenHandler implements AccessTokenHandlerInterface
{ {
public const USER = 'n8n'; public const ROLE = 'ROLE_CHATBOT_MCP';
/** @var array<string, string> token => client name */
private readonly array $clients;
/**
* @throws \InvalidArgumentException for a malformed list, so a typo fails loudly instead of locking a client out
*/
public function __construct( public function __construct(
#[\SensitiveParameter] #[\SensitiveParameter]
private readonly string $token, string $tokens,
) { ) {
$this->clients = self::parse($tokens);
} }
public function getUserBadgeFrom(#[\SensitiveParameter] string $accessToken): UserBadge public function getUserBadgeFrom(#[\SensitiveParameter] string $accessToken): UserBadge
{ {
if ('' === $this->token || !hash_equals($this->token, $accessToken)) { $client = null;
// Every token is compared, so the time taken does not tell which one came close.
foreach ($this->clients as $token => $name) {
if (hash_equals($token, $accessToken)) {
$client = $name;
}
}
if (null === $client) {
throw new BadCredentialsException('Invalid MCP token.'); throw new BadCredentialsException('Invalid MCP token.');
} }
return new UserBadge(self::USER); return new UserBadge($client, static fn (string $name): InMemoryUser => new InMemoryUser($name, null, [self::ROLE]));
}
/**
* @return array<string, string> token => client name
*/
private static function parse(string $tokens): array
{
$clients = [];
foreach (explode(',', $tokens) as $entry) {
if ('' === trim($entry)) {
continue;
}
[$name, $token] = array_map('trim', explode(':', $entry, 2)) + [1 => ''];
if (1 !== preg_match('/^[a-z0-9_-]+$/i', $name) || '' === $token) {
throw new \InvalidArgumentException('APP_MCP_TOKENS entries must look like "name:token", with a name of letters, digits, "-" and "_".');
}
if (in_array($name, $clients, true)) {
throw new \InvalidArgumentException(sprintf('APP_MCP_TOKENS names client "%s" twice.', $name));
}
if (isset($clients[$token])) {
throw new \InvalidArgumentException(sprintf('APP_MCP_TOKENS gives clients "%s" and "%s" the same token.', $clients[$token], $name));
}
$clients[$token] = $name;
}
return $clients;
} }
} }
+45 -6
View File
@@ -5,27 +5,66 @@ declare(strict_types=1);
namespace App\Tests\Security; namespace App\Tests\Security;
use App\Security\McpAccessTokenHandler; use App\Security\McpAccessTokenHandler;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Core\Exception\BadCredentialsException;
class McpAccessTokenHandlerTest extends TestCase class McpAccessTokenHandlerTest extends TestCase
{ {
public function testTheConfiguredTokenAuthenticatesTheN8nUser(): void public function testEachTokenAuthenticatesItsOwnClient(): void
{ {
self::assertSame(McpAccessTokenHandler::USER, (new McpAccessTokenHandler('s3cret'))->getUserBadgeFrom('s3cret')->getUserIdentifier()); $handler = new McpAccessTokenHandler('n8n:token-a, claude-desktop:token-b');
$badge = $handler->getUserBadgeFrom('token-b');
self::assertSame('claude-desktop', $badge->getUserIdentifier());
self::assertSame(['ROLE_CHATBOT_MCP'], $badge->getUser()->getRoles());
self::assertSame('n8n', $handler->getUserBadgeFrom('token-a')->getUserIdentifier());
} }
public function testAWrongTokenIsRejected(): void public function testATokenMayContainColons(): void
{
self::assertSame('n8n', (new McpAccessTokenHandler('n8n:a:b:c'))->getUserBadgeFrom('a:b:c')->getUserIdentifier());
}
public function testAnUnknownTokenIsRejected(): void
{ {
$this->expectException(BadCredentialsException::class); $this->expectException(BadCredentialsException::class);
(new McpAccessTokenHandler('s3cret'))->getUserBadgeFrom('guess'); (new McpAccessTokenHandler('n8n:token-a'))->getUserBadgeFrom('token-b');
} }
public function testAnUnconfiguredTokenRejectsEverything(): void public function testAClientNameIsNotAToken(): void
{ {
$this->expectException(BadCredentialsException::class); $this->expectException(BadCredentialsException::class);
(new McpAccessTokenHandler(''))->getUserBadgeFrom(''); (new McpAccessTokenHandler('n8n:token-a'))->getUserBadgeFrom('n8n');
}
public function testAnEmptyListRejectsEverything(): void
{
$this->expectException(BadCredentialsException::class);
(new McpAccessTokenHandler(' '))->getUserBadgeFrom('');
}
/**
* @return iterable<string, array{string}>
*/
public static function malformedLists(): iterable
{
yield 'token without name' => ['token-a'];
yield 'empty token' => ['n8n:'];
yield 'invalid name' => ['n 8 n:token-a'];
yield 'duplicate name' => ['n8n:token-a,n8n:token-b'];
yield 'shared token' => ['n8n:token-a,claude-desktop:token-a'];
}
#[DataProvider('malformedLists')]
public function testAMalformedListFailsLoudly(string $tokens): void
{
$this->expectException(\InvalidArgumentException::class);
new McpAccessTokenHandler($tokens);
} }
} }