diff --git a/CHANGELOG.md b/CHANGELOG.md index b1e0afe..0c100b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +## [Unreleased] + +### Features +* Per-tool MCP permissions: `mcp.tool_permissions` maps every tool name to an existing host permission, resolved by `McpToolAccessChecker` through the host's standard permission model (unmapped tool = denied, super admin bypass). `McpToolExecutor::execute()` answers unauthorized `tools/call` with a tool error result logged into `mcpLogs` before running the tool callback (BC: the executor now requires `McpToolAccessChecker`), `FilterToolsListRequestHandler` hides unauthorized tools from `tools/list`. +* Log search queries (`JournalLogRepository::findLatest()`, `AuditLogRepository::findLatest()`, `findLatestByContextId()`, `McpLogRepository::findLatestByContextId()`) are bounded and sorted by the built-in `_id` index instead of the unindexed `datetime` sort; new `MongoHelper` with `minObjectIdFor()` / `maxObjectIdFor()` / `sortNewestFirst()` and `FIELD_ID` / `SORT_DESC` constants. See the "Log query bounds" section of `src/Resources/doc/mcp.md` for the slack assumptions. +* `McpRateLimiter` honours a per-caller override from the security token attributes `McpRateLimiter::TOKEN_ATTRIBUTE_KEY` (bucket key) and `McpRateLimiter::TOKEN_ATTRIBUTE_LIMIT` (limit replacing the configured default), so authenticators can rate-limit per personal access token; the limiter now takes the configured `limit` + `interval` and the storage directly (the `anzu_systems_common.mcp.rate_limiter_factory` service is gone). + +### Changes +* BC change: `AbstractLogRepository::FIELD_ID` (protected) moved to `MongoHelper::FIELD_ID`; `McpToolExecutor` requires `McpToolAccessChecker` (its tool-permission check). + ## [12.0.0](https://github.com/anzusystems/common-bundle/compare/11.3.0...12.0.0) (2026-07-22) ### Features diff --git a/src/DependencyInjection/AnzuSystemsCommonExtension.php b/src/DependencyInjection/AnzuSystemsCommonExtension.php index c258d13..636820a 100644 --- a/src/DependencyInjection/AnzuSystemsCommonExtension.php +++ b/src/DependencyInjection/AnzuSystemsCommonExtension.php @@ -71,7 +71,9 @@ use AnzuSystems\CommonBundle\Log\Repository\AuditLogRepository; use AnzuSystems\CommonBundle\Log\Repository\JournalLogRepository; use AnzuSystems\CommonBundle\Mcp\Controller\McpController; +use AnzuSystems\CommonBundle\Mcp\McpRateLimiter; use AnzuSystems\CommonBundle\Mcp\McpToolExecutor; +use AnzuSystems\CommonBundle\Mcp\Security\McpToolAccessChecker; use AnzuSystems\CommonBundle\Messenger\Message\AuditLogMessage; use AnzuSystems\CommonBundle\Messenger\Message\JournalLogMessage; use AnzuSystems\CommonBundle\Request\ParamConverter\ApiFilterParamConverter; @@ -624,15 +626,10 @@ private function loadMcp(LoaderInterface $loader, ContainerBuilder $container): $rateLimiterStorageDefinition->setArgument('$pool', new Reference($mcp['rate_limiter']['cache_pool'])); $container->setDefinition('anzu_systems_common.mcp.rate_limiter_storage', $rateLimiterStorageDefinition); - $rateLimiterFactoryDefinition = new Definition(RateLimiterFactory::class); - $rateLimiterFactoryDefinition->setArgument('$config', [ - 'id' => 'mcp', - 'policy' => 'sliding_window', - 'limit' => $mcp['rate_limiter']['limit'], - 'interval' => $mcp['rate_limiter']['interval'], - ]); - $rateLimiterFactoryDefinition->setArgument('$storage', new Reference('anzu_systems_common.mcp.rate_limiter_storage')); - $container->setDefinition('anzu_systems_common.mcp.rate_limiter_factory', $rateLimiterFactoryDefinition); + $container + ->getDefinition(McpRateLimiter::class) + ->replaceArgument('$limit', $mcp['rate_limiter']['limit']) + ->replaceArgument('$interval', $mcp['rate_limiter']['interval']); $container ->getDefinition(McpController::class) @@ -642,6 +639,10 @@ private function loadMcp(LoaderInterface $loader, ContainerBuilder $container): ->getDefinition(McpToolExecutor::class) ->replaceArgument('$toolErrorExceptions', $mcp['tool_error_exceptions']); + $container + ->getDefinition(McpToolAccessChecker::class) + ->replaceArgument('$toolPermissions', $mcp['tool_permissions']); + $container ->getDefinition(CreateMcpLogCollectionCommand::class) ->replaceArgument('$mcpLogCollectionName', $mongo['collection']) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 0f55572..4a28642 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -361,6 +361,10 @@ private function addMcpSection(): NodeDefinition ->end() ->scalarPrototype()->end() ->end() + ->arrayNode('tool_permissions') + ->useAttributeAsKey('tool') + ->scalarPrototype()->end() + ->end() ->arrayNode('rate_limiter') ->addDefaultsIfNotSet() ->children() diff --git a/src/Helper/MongoHelper.php b/src/Helper/MongoHelper.php new file mode 100644 index 0000000..ad7424f --- /dev/null +++ b/src/Helper/MongoHelper.php @@ -0,0 +1,68 @@ +> $documents + * + * @return list> + */ + public static function sortNewestFirst(array $documents, string $datetimeField): array + { + usort( + $documents, + static fn (array $a, array $b): int => self::datetimeMillis($b, $datetimeField) <=> self::datetimeMillis($a, $datetimeField), + ); + + return $documents; + } + + private static function timestampHex(DateTimeImmutable $datetime): string + { + return sprintf( + self::TIMESTAMP_HEX_FORMAT, + min(self::TIMESTAMP_MAX, max(self::TIMESTAMP_MIN, $datetime->getTimestamp())), + ); + } + + /** + * @param array $document + */ + private static function datetimeMillis(array $document, string $datetimeField): int + { + $datetime = $document[$datetimeField] ?? null; + if ($datetime instanceof UTCDateTime) { + return (int) (string) $datetime; + } + + return self::DATETIME_MILLIS_FALLBACK; + } +} diff --git a/src/Log/Repository/AbstractLogRepository.php b/src/Log/Repository/AbstractLogRepository.php index 9a26886..8e8149d 100644 --- a/src/Log/Repository/AbstractLogRepository.php +++ b/src/Log/Repository/AbstractLogRepository.php @@ -5,6 +5,7 @@ namespace AnzuSystems\CommonBundle\Log\Repository; use AnzuSystems\CommonBundle\Document\Log; +use AnzuSystems\CommonBundle\Helper\MongoHelper; use AnzuSystems\CommonBundle\Repository\Mongo\AbstractAnzuMongoRepository; use DateTimeImmutable; use MongoDB\BSON\UTCDateTime; @@ -24,7 +25,8 @@ abstract class AbstractLogRepository extends AbstractAnzuMongoRepository protected const string MONGO_EXISTS = '$exists'; private const int LIMIT_MIN = 1; - private const int SORT_DESC = -1; + private const string ID_LOWER_BOUND_SLACK = '-1 minute'; + private const string ID_UPPER_BOUND_SLACK = '+1 day'; private const array RAW_ARRAY_TYPE_MAP = [ 'root' => 'array', 'document' => 'array', @@ -41,6 +43,9 @@ public function findLatestByContextId(string $contextId, DateTimeImmutable $from self::FIELD_DATETIME => [ self::MONGO_GTE => new UTCDateTime($from), ], + MongoHelper::FIELD_ID => [ + self::MONGO_GTE => MongoHelper::minObjectIdFor($from->modify(self::ID_LOWER_BOUND_SLACK)), + ], ], $limit); } @@ -58,14 +63,14 @@ protected function findLatestRawDocuments(array $match, int $limit): array { $documents = $this->collection->find($match, [ 'sort' => [ - self::FIELD_DATETIME => self::SORT_DESC, + MongoHelper::FIELD_ID => MongoHelper::SORT_DESC, ], 'limit' => max(self::LIMIT_MIN, $limit), 'maxTimeMS' => $this->queryMaxTimeMs, 'typeMap' => self::RAW_ARRAY_TYPE_MAP, ]); - return array_values($documents->toArray()); + return MongoHelper::sortNewestFirst(array_values($documents->toArray()), self::FIELD_DATETIME); } /** @@ -78,6 +83,10 @@ protected function createDatetimeWindowMatch(DateTimeImmutable $from, DateTimeIm self::MONGO_GTE => new UTCDateTime($from), self::MONGO_LTE => new UTCDateTime($until), ], + MongoHelper::FIELD_ID => [ + self::MONGO_GTE => MongoHelper::minObjectIdFor($from->modify(self::ID_LOWER_BOUND_SLACK)), + self::MONGO_LTE => MongoHelper::maxObjectIdFor($until->modify(self::ID_UPPER_BOUND_SLACK)), + ], ]; } } diff --git a/src/Mcp/Handler/FilterToolsListRequestHandler.php b/src/Mcp/Handler/FilterToolsListRequestHandler.php new file mode 100644 index 0000000..975d928 --- /dev/null +++ b/src/Mcp/Handler/FilterToolsListRequestHandler.php @@ -0,0 +1,54 @@ + + */ +final readonly class FilterToolsListRequestHandler implements RequestHandlerInterface +{ + public function __construct( + private RegistryInterface $registry, + private McpToolAccessChecker $toolAccessChecker, + private int $pageSize, + ) { + } + + public function supports(Request $request): bool + { + return $request instanceof ListToolsRequest; + } + + public function handle(Request $request, SessionInterface $session): Response|Error + { + if (false === $request instanceof ListToolsRequest) { + return Error::forInternalError('Unsupported request.', $request->getId()); + } + + $page = $this->registry->getTools($this->pageSize, $request->cursor); + $grantedTools = []; + foreach ($page->references as $reference) { + if ($reference instanceof Tool && $this->toolAccessChecker->isToolGranted($reference->name)) { + $grantedTools[] = $reference; + } + } + + return new Response( + $request->getId(), + new ListToolsResult($grantedTools, $page->nextCursor), + ); + } +} diff --git a/src/Mcp/Log/McpLogRepository.php b/src/Mcp/Log/McpLogRepository.php index 0870797..cc33112 100644 --- a/src/Mcp/Log/McpLogRepository.php +++ b/src/Mcp/Log/McpLogRepository.php @@ -5,6 +5,7 @@ namespace AnzuSystems\CommonBundle\Mcp\Log; use AnzuSystems\CommonBundle\ApiFilter\ApiQueryMongo; +use AnzuSystems\CommonBundle\Helper\MongoHelper; use DateTimeImmutable; use MongoDB\BSON\UTCDateTime; use MongoDB\Collection; @@ -15,7 +16,7 @@ private const string FIELD_CONTEXT_ID = 'contextId'; private const string MONGO_GTE = '$gte'; private const int LIMIT_MIN = 1; - private const int SORT_DESC = -1; + private const string ID_LOWER_BOUND_SLACK = '-1 minute'; private const array RAW_ARRAY_TYPE_MAP = [ 'root' => 'array', 'document' => 'array', @@ -38,15 +39,18 @@ public function findLatestByContextId(string $contextId, DateTimeImmutable $from self::FIELD_DATETIME => [ self::MONGO_GTE => new UTCDateTime($from), ], + MongoHelper::FIELD_ID => [ + self::MONGO_GTE => MongoHelper::minObjectIdFor($from->modify(self::ID_LOWER_BOUND_SLACK)), + ], ], [ 'sort' => [ - self::FIELD_DATETIME => self::SORT_DESC, + MongoHelper::FIELD_ID => MongoHelper::SORT_DESC, ], 'limit' => max(self::LIMIT_MIN, $limit), 'maxTimeMS' => $this->queryMaxTimeMs, 'typeMap' => self::RAW_ARRAY_TYPE_MAP, ]); - return array_values($documents->toArray()); + return MongoHelper::sortNewestFirst(array_values($documents->toArray()), self::FIELD_DATETIME); } } diff --git a/src/Mcp/McpRateLimiter.php b/src/Mcp/McpRateLimiter.php index 7749851..4e46d20 100644 --- a/src/Mcp/McpRateLimiter.php +++ b/src/Mcp/McpRateLimiter.php @@ -5,16 +5,29 @@ namespace AnzuSystems\CommonBundle\Mcp; use AnzuSystems\CommonBundle\Domain\User\CurrentAnzuUserProvider; +use AnzuSystems\CommonBundle\Helper\StringHelper; use AnzuSystems\Contracts\AnzuApp; +use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\TooManyRequestsHttpException; +use Symfony\Component\RateLimiter\LimiterInterface; use Symfony\Component\RateLimiter\RateLimiterFactory; +use Symfony\Component\RateLimiter\Storage\StorageInterface; final readonly class McpRateLimiter { + public const string TOKEN_ATTRIBUTE_KEY = 'mcp_rate_limit_key'; + public const string TOKEN_ATTRIBUTE_LIMIT = 'mcp_rate_limit'; + + private const string LIMITER_ID = 'mcp'; + private const string LIMITER_POLICY = 'sliding_window'; + public function __construct( - private RateLimiterFactory $mcpLimiter, + private int $limit, + private string $interval, + private StorageInterface $storage, private CurrentAnzuUserProvider $currentUserProvider, + private Security $security, ) { } @@ -30,8 +43,11 @@ public function checkRateLimit(): void throw new AccessDeniedHttpException('Anonymous access to the MCP endpoint is not allowed.'); } - $limiter = $this->mcpLimiter->create((string) $userId); - $limit = $limiter->consume(); + $key = $this->resolveTokenAttribute(self::TOKEN_ATTRIBUTE_KEY); + $limit = $this->createLimiter( + is_string($key) && StringHelper::isNotEmpty($key) ? $key : (string) $userId, + $this->resolveTokenAttribute(self::TOKEN_ATTRIBUTE_LIMIT), + )->consume(); if ($limit->isAccepted()) { return; } @@ -49,4 +65,24 @@ public function checkRateLimit(): void ], ); } + + private function resolveTokenAttribute(string $name): mixed + { + return $this->security->getToken()?->getAttributes()[$name] ?? null; + } + + private function createLimiter(string $key, mixed $limitOverride): LimiterInterface + { + $limit = is_int($limitOverride) && $limitOverride > 0 ? $limitOverride : $this->limit; + + return new RateLimiterFactory( + [ + 'id' => self::LIMITER_ID, + 'policy' => self::LIMITER_POLICY, + 'limit' => $limit, + 'interval' => $this->interval, + ], + $this->storage, + )->create($key); + } } diff --git a/src/Mcp/McpToolExecutor.php b/src/Mcp/McpToolExecutor.php index 403ec05..283ea51 100644 --- a/src/Mcp/McpToolExecutor.php +++ b/src/Mcp/McpToolExecutor.php @@ -7,6 +7,7 @@ use AnzuSystems\CommonBundle\Domain\User\CurrentAnzuUserProvider; use AnzuSystems\CommonBundle\Mcp\Exception\McpToolInputException; use AnzuSystems\CommonBundle\Mcp\Log\McpLogger; +use AnzuSystems\CommonBundle\Mcp\Security\McpToolAccessChecker; use Closure; use Monolog\Attribute\WithMonologChannel; use Psr\Log\LoggerInterface; @@ -18,6 +19,8 @@ { public const string ERROR_KEY = 'error'; + private const string TOOL_ACCESS_DENIED_MESSAGE = 'Access denied — the current MCP user is not allowed to use the tool "%s".'; + /** * @param array, string> $toolErrorExceptions */ @@ -25,6 +28,7 @@ public function __construct( private CurrentAnzuUserProvider $currentUserProvider, private LoggerInterface $logger, private McpLogger $mcpLogger, + private McpToolAccessChecker $toolAccessChecker, private array $toolErrorExceptions = [], ) { } @@ -41,6 +45,12 @@ public function execute(string $toolName, array $params, Closure $callback): arr $error = null; try { + if (false === $this->toolAccessChecker->isToolGranted($toolName)) { + $error = sprintf(self::TOOL_ACCESS_DENIED_MESSAGE, $toolName); + + return [self::ERROR_KEY => $error]; + } + return $callback(); } catch (McpToolInputException $exception) { $error = $exception->getMessage(); diff --git a/src/Mcp/Security/McpToolAccessChecker.php b/src/Mcp/Security/McpToolAccessChecker.php new file mode 100644 index 0000000..3125efe --- /dev/null +++ b/src/Mcp/Security/McpToolAccessChecker.php @@ -0,0 +1,29 @@ + $toolPermissions + */ + public function __construct( + private array $toolPermissions, + private Security $security, + ) { + } + + public function isToolGranted(string $toolName): bool + { + $permission = $this->toolPermissions[$toolName] ?? null; + if (null === $permission) { + return false; + } + + return $this->security->isGranted($permission); + } +} diff --git a/src/Resources/config/mcp.php b/src/Resources/config/mcp.php index 9f1a171..54b4795 100644 --- a/src/Resources/config/mcp.php +++ b/src/Resources/config/mcp.php @@ -9,6 +9,7 @@ use AnzuSystems\CommonBundle\Log\Repository\AuditLogRepository; use AnzuSystems\CommonBundle\Log\Repository\JournalLogRepository; use AnzuSystems\CommonBundle\Mcp\Controller\McpController; +use AnzuSystems\CommonBundle\Mcp\Handler\FilterToolsListRequestHandler; use AnzuSystems\CommonBundle\Mcp\Handler\StrictToolArgumentsRequestHandler; use AnzuSystems\CommonBundle\Mcp\Log\McpLogFinder; use AnzuSystems\CommonBundle\Mcp\Log\McpLogger; @@ -17,6 +18,7 @@ use AnzuSystems\CommonBundle\Mcp\McpToolExecutor; use AnzuSystems\CommonBundle\Mcp\Resolver\McpContextIdResolver; use AnzuSystems\CommonBundle\Mcp\Resolver\McpDateWindowResolver; +use AnzuSystems\CommonBundle\Mcp\Security\McpToolAccessChecker; use AnzuSystems\CommonBundle\Mcp\Tool\GetLogsByContextTool; use AnzuSystems\CommonBundle\Mcp\Tool\SearchAppLogsTool; use AnzuSystems\CommonBundle\Mcp\Tool\SearchAuditLogsTool; @@ -41,6 +43,18 @@ ->tag('monolog.logger', ['channel' => 'mcp']) ; + $services->set(McpToolAccessChecker::class) + ->arg('$toolPermissions', null) + ->arg('$security', service('security.helper')) + ; + + $services->set(FilterToolsListRequestHandler::class) + ->arg('$registry', service('mcp.registry')) + ->arg('$toolAccessChecker', service(McpToolAccessChecker::class)) + ->arg('$pageSize', param('mcp.pagination_limit')) + ->tag('mcp.request_handler') + ; + $services->set(McpLogger::class) ->arg('$mcpLogCollection', service('anzu_mongo_mcp_log_collection')) ; @@ -61,13 +75,17 @@ ->arg('$currentUserProvider', service(CurrentAnzuUserProvider::class)) ->arg('$logger', service('logger')) ->arg('$mcpLogger', service(McpLogger::class)) + ->arg('$toolAccessChecker', service(McpToolAccessChecker::class)) ->arg('$toolErrorExceptions', null) ->tag('monolog.logger', ['channel' => 'mcp']) ; $services->set(McpRateLimiter::class) - ->arg('$mcpLimiter', service('anzu_systems_common.mcp.rate_limiter_factory')) + ->arg('$limit', null) + ->arg('$interval', null) + ->arg('$storage', service('anzu_systems_common.mcp.rate_limiter_storage')) ->arg('$currentUserProvider', service(CurrentAnzuUserProvider::class)) + ->arg('$security', service('security.helper')) ; $services->set(McpController::class) diff --git a/src/Resources/doc/helpers.md b/src/Resources/doc/helpers.md index 980e70b..d1bbc17 100644 --- a/src/Resources/doc/helpers.md +++ b/src/Resources/doc/helpers.md @@ -5,5 +5,6 @@ Common Bundle has some helper classes which should not be used as services and t * [CollectionHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/CollectionHelper.php) * [EmailHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/EmailHelper.php) +* [MongoHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/MongoHelper.php) * [PasswordHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/PasswordHelper.php) * [UuidHelper](https://github.com/anzusystems/common-bundle/blob/main/src/Helper/UuidHelper.php) diff --git a/src/Resources/doc/mcp.md b/src/Resources/doc/mcp.md index d1f9a55..960423f 100644 --- a/src/Resources/doc/mcp.md +++ b/src/Resources/doc/mcp.md @@ -62,10 +62,70 @@ infrastructure after a bundle upgrade. ## Provided services * `McpController` (alias `mcp.server.controller`) — streamable HTTP transport endpoint with DNS-rebinding protection - (`allowed_hosts`) and a per-user sliding-window rate limit. + (`allowed_hosts`) and a sliding-window rate limit (per user, or per caller when the security token carries the + rate-limit attributes — see below). * `McpToolExecutor` — wraps tool callbacks: converts `McpToolInputException`, `AccessDeniedException` and configured `tool_error_exceptions` into tool error results, logs every call to the monolog `mcp` channel and to the `mcpLogs` capped collection. * `StrictToolArgumentsRequestHandler` — rejects tool calls with unknown arguments. +* `McpToolAccessChecker` + `FilterToolsListRequestHandler` — per-tool permissions mapped to host permissions (see + below); the call-side check lives in `McpToolExecutor`. * `SearchAppLogsTool`, `SearchAuditLogsTool`, `GetLogsByContextTool` — diagnostic tools over the shared log collections, correlated by `contextId`. + +## Rate limit override per caller + +`McpRateLimiter` keys the sliding window by the current user id and applies the configured `rate_limiter.limit`. +An authenticator can override both by setting attributes on the security token: + +```php +$token->setAttribute(McpRateLimiter::TOKEN_ATTRIBUTE_KEY, 'pat_' . $personalAccessToken->getId()); +$token->setAttribute(McpRateLimiter::TOKEN_ATTRIBUTE_LIMIT, $personalAccessToken->getRateLimit()); +``` + +`TOKEN_ATTRIBUTE_KEY` (string) selects the bucket (e.g. one bucket per personal access token), `TOKEN_ATTRIBUTE_LIMIT` +(positive `int`) replaces the configured limit for that bucket — `null`, a missing attribute or a non-positive value +keeps the configured default. Tokens without the attributes fall back to the per-user bucket. The configured +`rate_limiter.interval` applies to every bucket. The personal access token authenticator from +[anzusystems/auth-bundle](https://github.com/anzusystems/auth-bundle) sets both attributes. + +## Tool permissions + +Every tool is mapped to an existing permission of the host application in `mcp.tool_permissions` (tool name → +permission name); `McpToolAccessChecker` resolves it through the host's standard permission model +(`Security::isGranted()` → the host voters, super admin bypass). A tool that is not mapped is denied — there is no +implicit access. + +```yaml +anzu_systems_common: + mcp: + tool_permissions: + search_app_logs: cms_log_read + search_audit_logs: cms_log_read + get_logs_by_context: cms_log_read + list_sites: cms_site_read +``` + +* `tools/call` of a tool the current user may not use returns a tool error result (`{"error": "Access denied — …"}`) from + `McpToolExecutor::execute()` before the tool callback runs, and the denied call is written to the `mcpLogs` collection + with the error (so every tool must route through the executor, as the bundle tools do). +* `tools/list` returns only the tools the current user may call — the filter runs after registry pagination, so a + page may come back with fewer tools (even none) while `nextCursor` is still set; clients follow the cursor as usual. + +## Log query bounds + +The log repositories behind the diagnostic tools (`JournalLogRepository::findLatest()`, `AuditLogRepository::findLatest()`, +`findLatestByContextId()` and `McpLogRepository::findLatestByContextId()`) have no index on `datetime`; they bound and sort +every query by the built-in `_id` index instead, deriving the ObjectId range from the `datetime` window via +`MongoHelper::minObjectIdFor()` / `maxObjectIdFor()`. The `datetime` filter stays in place, so the `_id` range only +narrows the scan. Assumptions behind the bounds: + +* Journal and audit records are written asynchronously (Messenger consumer), so the `_id` timestamp is the insertion time, + not the record `datetime`. The range is widened by `-1 minute` below and `+1 day` above the window; a record inserted more + than one day after its `datetime` (consumer outage, failed-message replay) falls outside a historical window. +* Records are selected in insertion (`_id`) order and the returned page is re-sorted by `datetime`; with a `limit`, the + selected newest rows are those inserted last, which can differ from a strict `datetime` ordering when consumers run in + parallel. +* For selective filters (`contextId`, `onlyErrors`) over large windows the scan still fetches every document in the `_id` + range until `limit` is filled, guarded only by `mongo_query_max_time_ms`; a compound `{ 'context.contextId': 1, _id: -1 }` + index on `appLogs` / `auditLogs` is the real fix for by-context lookups on busy collections. diff --git a/tests/Helper/MongoHelperTest.php b/tests/Helper/MongoHelperTest.php new file mode 100644 index 0000000..cc36e29 --- /dev/null +++ b/tests/Helper/MongoHelperTest.php @@ -0,0 +1,51 @@ + + */ + public static function objectIdBoundsProvider(): array + { + return [ + 'utc' => ['6a7f7a00', new DateTimeImmutable('2026-08-14 20:26:40+00:00')], + 'sub-second precision is dropped' => ['6a7f7a00', new DateTimeImmutable('2026-08-14 20:26:40.999+00:00')], + 'offset is normalised to utc' => ['6a7f7a00', new DateTimeImmutable('2026-08-14 22:26:40+02:00')], + 'small timestamp is left padded' => ['0000e100', new DateTimeImmutable('1970-01-01 16:00:00+00:00')], + 'negative timestamp is clamped to zero' => ['00000000', new DateTimeImmutable('1960-01-01 00:00:00+00:00')], + 'post 2106 timestamp is clamped to max' => ['ffffffff', new DateTimeImmutable('2200-01-01 00:00:00+00:00')], + ]; + } + + public function testSortNewestFirst(): void + { + $oldest = [self::DATETIME_FIELD => new UTCDateTime(1_000), 'id' => 'oldest']; + $middle = [self::DATETIME_FIELD => new UTCDateTime(2_000), 'id' => 'middle']; + $newest = [self::DATETIME_FIELD => new UTCDateTime(3_000), 'id' => 'newest']; + $missing = ['id' => 'missing']; + + $sorted = MongoHelper::sortNewestFirst([$middle, $missing, $newest, $oldest], self::DATETIME_FIELD); + + self::assertSame(['newest', 'middle', 'oldest', 'missing'], array_column($sorted, 'id')); + } +} diff --git a/tests/Mcp/Handler/FilterToolsListRequestHandlerTest.php b/tests/Mcp/Handler/FilterToolsListRequestHandlerTest.php new file mode 100644 index 0000000..c4102ac --- /dev/null +++ b/tests/Mcp/Handler/FilterToolsListRequestHandlerTest.php @@ -0,0 +1,69 @@ +createHandler(); + + self::assertTrue($handler->supports(new ListToolsRequest()->withId(self::REQUEST_ID))); + self::assertFalse($handler->supports(new CallToolRequest(self::GRANTED_TOOL_NAME, [])->withId(self::REQUEST_ID))); + } + + public function testHandleReturnsOnlyGrantedTools(): void + { + $response = $this->createHandler() + ->handle(new ListToolsRequest()->withId(self::REQUEST_ID), $this->createStub(SessionInterface::class)); + + self::assertInstanceOf(Response::class, $response); + self::assertInstanceOf(ListToolsResult::class, $response->result); + self::assertCount(1, $response->result->tools); + self::assertSame(self::GRANTED_TOOL_NAME, $response->result->tools[0]->name); + } + + private function createHandler(): FilterToolsListRequestHandler + { + $tools = []; + foreach ([self::GRANTED_TOOL_NAME, self::DENIED_TOOL_NAME] as $toolName) { + $tools[$toolName] = new Tool($toolName, null, ['type' => 'object'], null, null); + } + $registry = $this->createMock(RegistryInterface::class); + $registry->method('getTools') + ->with(self::PAGE_SIZE, null) + ->willReturn(new Page($tools, null)); + + $security = $this->createMock(Security::class); + $security->method('isGranted') + ->willReturnCallback(static fn (mixed $attribute): bool => self::GRANTED_PERMISSION === $attribute); + $toolAccessChecker = new McpToolAccessChecker( + [self::GRANTED_TOOL_NAME => self::GRANTED_PERMISSION, self::DENIED_TOOL_NAME => self::DENIED_PERMISSION], + $security, + ); + + return new FilterToolsListRequestHandler($registry, $toolAccessChecker, self::PAGE_SIZE); + } +} diff --git a/tests/Mcp/McpLogRepositoryTest.php b/tests/Mcp/McpLogRepositoryTest.php new file mode 100644 index 0000000..6e5d7ac --- /dev/null +++ b/tests/Mcp/McpLogRepositoryTest.php @@ -0,0 +1,53 @@ + new UTCDateTime(1_000), + 'id' => 'older', + ]; + $newer = [ + 'datetime' => new UTCDateTime(2_000), + 'id' => 'newer', + ]; + + $cursor = $this->createMock(CursorInterface::class); + $cursor->method('toArray') + ->willReturn([$older, $newer]); + + $collection = $this->createMock(Collection::class); + $collection + ->expects(self::once()) + ->method('find') + ->with( + self::callback(static fn (array $filter): bool => self::CONTEXT_ID === $filter['contextId'] + && (string) new UTCDateTime($from) === (string) $filter['datetime']['$gte'] + && (string) MongoHelper::minObjectIdFor($from->modify('-1 minute')) === (string) $filter[MongoHelper::FIELD_ID]['$gte']), + self::callback(static fn (array $options): bool => [MongoHelper::FIELD_ID => MongoHelper::SORT_DESC] === $options['sort'] + && self::LIMIT === $options['limit']), + ) + ->willReturn($cursor); + + $documents = (new McpLogRepository($collection))->findLatestByContextId(self::CONTEXT_ID, $from, self::LIMIT); + + self::assertSame(['newer', 'older'], array_column($documents, 'id')); + } +} diff --git a/tests/Mcp/McpRateLimiterTest.php b/tests/Mcp/McpRateLimiterTest.php index bea3cfb..bbe7519 100644 --- a/tests/Mcp/McpRateLimiterTest.php +++ b/tests/Mcp/McpRateLimiterTest.php @@ -9,33 +9,38 @@ use AnzuSystems\Contracts\AnzuApp; use AnzuSystems\Contracts\Entity\AnzuUser; use PHPUnit\Framework\TestCase; +use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\TooManyRequestsHttpException; -use Symfony\Component\RateLimiter\RateLimiterFactory; use Symfony\Component\RateLimiter\Storage\InMemoryStorage; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\User\InMemoryUser; final class McpRateLimiterTest extends TestCase { private const int LIMIT = 1; + private const int LIMIT_OVERRIDE = 2; private const int INTERVAL_SECONDS = 60; private const int USER_ID = 42; + private const string TOKEN_KEY = 'pat_7'; + private const string FIREWALL_NAME = 'mcp'; + private const string USER_IDENTIFIER = 'mcp-user'; + private const string INTERVAL = '1 minute'; public function testAnonymousUserIsRejected(): void { - $rateLimiter = new McpRateLimiter( - $this->createLimiterFactory(), - $this->createCurrentUserProvider(AnzuApp::getUserIdAnonymous()), - ); + $rateLimiter = $this->createRateLimiter(AnzuApp::getUserIdAnonymous()); $this->expectException(AccessDeniedHttpException::class); $rateLimiter->checkRateLimit(); } - public function testThrowsWhenLimitExceeded(): void + public function testTokenWithoutAttributesThrowsWhenDefaultLimitExceeded(): void { - $rateLimiter = new McpRateLimiter($this->createLimiterFactory(), $this->createCurrentUserProvider()); + $rateLimiter = $this->createRateLimiter(token: $this->createToken([])); $rateLimiter->checkRateLimit(); try { @@ -53,26 +58,57 @@ public function testThrowsWhenLimitExceeded(): void } } - private function createLimiterFactory(): RateLimiterFactory + public function testTokenAttributesOverrideLimitAndKey(): void { - return new RateLimiterFactory( - [ - 'id' => 'mcp_test', - 'policy' => 'sliding_window', - 'limit' => self::LIMIT, - 'interval' => '1 minute', - ], - new InMemoryStorage(), - ); + $storage = new InMemoryStorage(); + $rateLimiter = $this->createRateLimiter(token: $this->createToken([ + McpRateLimiter::TOKEN_ATTRIBUTE_KEY => self::TOKEN_KEY, + McpRateLimiter::TOKEN_ATTRIBUTE_LIMIT => self::LIMIT_OVERRIDE, + ]), storage: $storage); + $rateLimiter->checkRateLimit(); + $rateLimiter->checkRateLimit(); + + try { + $rateLimiter->checkRateLimit(); + self::fail('Expected ' . TooManyRequestsHttpException::class); + } catch (TooManyRequestsHttpException $exception) { + self::assertSame((string) self::LIMIT_OVERRIDE, $exception->getHeaders()['X-RateLimit-Limit']); + } + + $this->createRateLimiter(token: $this->createToken([]), storage: $storage) + ->checkRateLimit(); } - private function createCurrentUserProvider(int $userId = self::USER_ID): CurrentAnzuUserProvider - { + private function createRateLimiter( + int $userId = self::USER_ID, + ?TokenInterface $token = null, + ?InMemoryStorage $storage = null, + ): McpRateLimiter { $user = $this->createConfiguredMock(AnzuUser::class, ['getId' => $userId]); $currentUserProvider = $this->createMock(CurrentAnzuUserProvider::class); $currentUserProvider->method('getCurrentUser') ->willReturn($user); + $security = $this->createMock(Security::class); + $security->method('getToken') + ->willReturn($token); + + return new McpRateLimiter( + self::LIMIT, + self::INTERVAL, + $storage ?? new InMemoryStorage(), + $currentUserProvider, + $security, + ); + } + + /** + * @param array $attributes + */ + private function createToken(array $attributes): TokenInterface + { + $token = new UsernamePasswordToken(new InMemoryUser(self::USER_IDENTIFIER, null), self::FIREWALL_NAME); + $token->setAttributes($attributes); - return $currentUserProvider; + return $token; } } diff --git a/tests/Mcp/McpToolExecutorTest.php b/tests/Mcp/McpToolExecutorTest.php index 950fba8..223e5a7 100644 --- a/tests/Mcp/McpToolExecutorTest.php +++ b/tests/Mcp/McpToolExecutorTest.php @@ -8,18 +8,21 @@ use AnzuSystems\CommonBundle\Mcp\Exception\McpToolInputException; use AnzuSystems\CommonBundle\Mcp\Log\McpLogger; use AnzuSystems\CommonBundle\Mcp\McpToolExecutor; +use AnzuSystems\CommonBundle\Mcp\Security\McpToolAccessChecker; use AnzuSystems\Contracts\Entity\AnzuUser; use MongoDB\Collection; use MongoDB\InsertOneResult; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; use RuntimeException; +use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\Security\Core\Exception\AccessDeniedException; final class McpToolExecutorTest extends TestCase { private const int USER_ID = 42; private const string TOOL_NAME = 'test_tool'; + private const string TOOL_PERMISSION = 'cms_test_read'; private const string BACKEND_ERROR_MESSAGE = 'Backend is temporarily unavailable, retry the call.'; private array $insertedDocuments = []; @@ -53,6 +56,16 @@ public function testInputExceptionIsReturnedAsToolError(): void self::assertSame('Invalid input.', $this->insertedDocuments[0]['error']); } + public function testMissingToolPermissionIsReturnedAsToolErrorWithoutRunningCallback(): void + { + $result = $this->createExecutor(toolGranted: false) + ->execute(self::TOOL_NAME, [], static fn (): array => throw new RuntimeException('must not run')); + + self::assertStringContainsString('Access denied', $result[McpToolExecutor::ERROR_KEY]); + self::assertStringContainsString(self::TOOL_NAME, $result[McpToolExecutor::ERROR_KEY]); + self::assertSame(McpLogger::LEVEL_NAME_ERROR, $this->insertedDocuments[0]['levelName']); + } + public function testAccessDeniedIsReturnedAsToolError(): void { $result = $this->createExecutor() @@ -98,7 +111,7 @@ public function testUnknownExceptionIsRethrownAndLoggedAsError(): void /** * @param array $toolErrorExceptions */ - private function createExecutor(array $toolErrorExceptions = []): McpToolExecutor + private function createExecutor(array $toolErrorExceptions = [], bool $toolGranted = true): McpToolExecutor { $this->insertedDocuments = []; @@ -116,10 +129,15 @@ private function createExecutor(array $toolErrorExceptions = []): McpToolExecuto $currentUserProvider->method('getCurrentUser') ->willReturn($user); + $security = $this->createMock(Security::class); + $security->method('isGranted') + ->willReturn($toolGranted); + return new McpToolExecutor( $currentUserProvider, new NullLogger(), new McpLogger($collection), + new McpToolAccessChecker([self::TOOL_NAME => self::TOOL_PERMISSION], $security), $toolErrorExceptions, ); } diff --git a/tests/Mcp/Security/McpToolAccessCheckerTest.php b/tests/Mcp/Security/McpToolAccessCheckerTest.php new file mode 100644 index 0000000..83ffb24 --- /dev/null +++ b/tests/Mcp/Security/McpToolAccessCheckerTest.php @@ -0,0 +1,36 @@ +createMock(Security::class); + $security->method('isGranted') + ->willReturnCallback(static fn (mixed $attribute): bool => self::GRANTED_PERMISSION === $attribute); + $checker = new McpToolAccessChecker( + [ + self::GRANTED_TOOL => self::GRANTED_PERMISSION, + self::DENIED_TOOL => self::DENIED_PERMISSION, + ], + $security, + ); + + self::assertTrue($checker->isToolGranted(self::GRANTED_TOOL)); + self::assertFalse($checker->isToolGranted(self::DENIED_TOOL)); + self::assertFalse($checker->isToolGranted(self::UNMAPPED_TOOL)); + } +}