diff --git a/Services/AgentApiKeyService.php b/Services/AgentApiKeyService.php index 9fde112..5f7143e 100644 --- a/Services/AgentApiKeyService.php +++ b/Services/AgentApiKeyService.php @@ -156,6 +156,9 @@ class AgentApiKeyService // Clear rate limit cache Cache::forget($this->getRateLimitCacheKey($key)); + + // Clear permitted tools cache so the revoked key can no longer access tools + app(AgentToolRegistry::class)->flushCacheForApiKey($key->id); } /** @@ -164,6 +167,9 @@ class AgentApiKeyService public function updatePermissions(AgentApiKey $key, array $permissions): void { $key->updatePermissions($permissions); + + // Invalidate cached tool list so the new permissions take effect immediately + app(AgentToolRegistry::class)->flushCacheForApiKey($key->id); } /** diff --git a/Services/AgentToolRegistry.php b/Services/AgentToolRegistry.php index 0c21bca..14ac37c 100644 --- a/Services/AgentToolRegistry.php +++ b/Services/AgentToolRegistry.php @@ -9,6 +9,7 @@ use Core\Mcp\Dependencies\HasDependencies; use Core\Mcp\Services\ToolDependencyService; use Core\Mod\Agentic\Mcp\Tools\Agent\Contracts\AgentToolInterface; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Cache; /** * Registry for MCP Agent Server tools. @@ -98,24 +99,57 @@ class AgentToolRegistry ); } + /** + * Cache TTL for permitted tool lists (1 hour). + */ + public const CACHE_TTL = 3600; + /** * Get tools accessible by an API key. * + * Results are cached per API key for {@see CACHE_TTL} seconds to avoid + * repeated O(n) filtering on every request (PERF-002). + * Use {@see flushCacheForApiKey()} to invalidate on permission changes. + * * @return Collection */ public function forApiKey(ApiKey $apiKey): Collection { - return $this->all()->filter(function (AgentToolInterface $tool) use ($apiKey) { - // Check if API key has required scopes - foreach ($tool->requiredScopes() as $scope) { - if (! $apiKey->hasScope($scope)) { - return false; - } - } + $cacheKey = $this->apiKeyCacheKey($apiKey->getKey()); - // Check if API key has tool-level permission - return $this->apiKeyCanAccessTool($apiKey, $tool->name()); + $permittedNames = Cache::remember($cacheKey, self::CACHE_TTL, function () use ($apiKey) { + return $this->all()->filter(function (AgentToolInterface $tool) use ($apiKey) { + // Check if API key has required scopes + foreach ($tool->requiredScopes() as $scope) { + if (! $apiKey->hasScope($scope)) { + return false; + } + } + + // Check if API key has tool-level permission + return $this->apiKeyCanAccessTool($apiKey, $tool->name()); + })->keys()->all(); }); + + return $this->all()->only($permittedNames); + } + + /** + * Flush the cached tool list for an API key. + * + * Call this whenever an API key's permissions or tool scopes change. + */ + public function flushCacheForApiKey(int|string $apiKeyId): void + { + Cache::forget($this->apiKeyCacheKey($apiKeyId)); + } + + /** + * Build the cache key for a given API key ID. + */ + private function apiKeyCacheKey(int|string $apiKeyId): string + { + return "agent_tool_registry:api_key:{$apiKeyId}"; } /** diff --git a/tests/Unit/AgentToolRegistryTest.php b/tests/Unit/AgentToolRegistryTest.php new file mode 100644 index 0000000..58d559c --- /dev/null +++ b/tests/Unit/AgentToolRegistryTest.php @@ -0,0 +1,278 @@ +toolName; } + public function description(): string { return 'Test tool'; } + public function inputSchema(): array { return []; } + public function handle(array $args, array $context = []): array { return ['success' => true]; } + public function requiredScopes(): array { return $this->toolScopes; } + public function category(): string { return $this->toolCategory; } + }; +} + +/** + * Build a minimal ApiKey stub with controllable scopes and tool_scopes. + * + * Extends the real ApiKey so the type-hint in AgentToolRegistry is satisfied. + * Eloquent attribute storage means $key->tool_scopes flows through __get/__set as normal. + */ +function makeApiKey(int $id, array $scopes = [], ?array $toolScopes = null): ApiKey +{ + $key = new class($id, $scopes, $toolScopes) extends ApiKey + { + private int $keyId; + private array $keyScopes; + + public function __construct(int $id, array $scopes, ?array $toolScopes) + { + $this->keyId = $id; + $this->keyScopes = $scopes; + // Store via Eloquent attributes so __get('tool_scopes') returns it correctly + $this->attributes['tool_scopes'] = $toolScopes; + } + + public function getKey(): mixed { return $this->keyId; } + + public function hasScope(string $scope): bool + { + return in_array($scope, $this->keyScopes, true); + } + }; + + return $key; +} + +// ========================================================================= +// Caching – basic behaviour +// ========================================================================= + +describe('forApiKey caching', function () { + beforeEach(function () { + Cache::fake(); + }); + + it('returns the correct tools on first call (cache miss)', function () { + $registry = new AgentToolRegistry; + $registry->register(makeTool('plan.create', ['plans.write'])); + $registry->register(makeTool('session.start', ['sessions.write'])); + + $apiKey = makeApiKey(1, ['plans.write', 'sessions.write']); + + $tools = $registry->forApiKey($apiKey); + + expect($tools->keys()->sort()->values()->all()) + ->toBe(['plan.create', 'session.start']); + }); + + it('stores permitted tool names in cache after first call', function () { + $registry = new AgentToolRegistry; + $registry->register(makeTool('plan.create', ['plans.write'])); + + $apiKey = makeApiKey(42, ['plans.write']); + + $registry->forApiKey($apiKey); + + $cached = Cache::get('agent_tool_registry:api_key:42'); + expect($cached)->toBe(['plan.create']); + }); + + it('returns same result on second call (cache hit)', function () { + $registry = new AgentToolRegistry; + $registry->register(makeTool('plan.create', ['plans.write'])); + $registry->register(makeTool('session.start', ['sessions.write'])); + + $apiKey = makeApiKey(1, ['plans.write']); + + $first = $registry->forApiKey($apiKey)->keys()->all(); + $second = $registry->forApiKey($apiKey)->keys()->all(); + + expect($second)->toBe($first); + }); + + it('filters tools whose required scopes the key lacks', function () { + $registry = new AgentToolRegistry; + $registry->register(makeTool('plan.create', ['plans.write'])); + $registry->register(makeTool('session.start', ['sessions.write'])); + + $apiKey = makeApiKey(1, ['plans.write']); // only plans.write + + $tools = $registry->forApiKey($apiKey); + + expect($tools->has('plan.create'))->toBeTrue() + ->and($tools->has('session.start'))->toBeFalse(); + }); + + it('respects tool_scopes allowlist on the api key', function () { + $registry = new AgentToolRegistry; + $registry->register(makeTool('plan.create', [])); + $registry->register(makeTool('session.start', [])); + + $apiKey = makeApiKey(5, [], ['plan.create']); // explicitly restricted + + $tools = $registry->forApiKey($apiKey); + + expect($tools->has('plan.create'))->toBeTrue() + ->and($tools->has('session.start'))->toBeFalse(); + }); + + it('allows all tools when tool_scopes is null', function () { + $registry = new AgentToolRegistry; + $registry->register(makeTool('plan.create', [])); + $registry->register(makeTool('session.start', [])); + + $apiKey = makeApiKey(7, [], null); // null = unrestricted + + $tools = $registry->forApiKey($apiKey); + + expect($tools)->toHaveCount(2); + }); + + it('caches separately per api key id', function () { + $registry = new AgentToolRegistry; + $registry->register(makeTool('plan.create', ['plans.write'])); + $registry->register(makeTool('session.start', ['sessions.write'])); + + $keyA = makeApiKey(100, ['plans.write']); + $keyB = makeApiKey(200, ['sessions.write']); + + $toolsA = $registry->forApiKey($keyA)->keys()->all(); + $toolsB = $registry->forApiKey($keyB)->keys()->all(); + + expect($toolsA)->toBe(['plan.create']) + ->and($toolsB)->toBe(['session.start']); + + expect(Cache::get('agent_tool_registry:api_key:100'))->toBe(['plan.create']) + ->and(Cache::get('agent_tool_registry:api_key:200'))->toBe(['session.start']); + }); +}); + +// ========================================================================= +// Cache TTL +// ========================================================================= + +describe('cache TTL', function () { + it('declares CACHE_TTL constant as 3600 (1 hour)', function () { + expect(AgentToolRegistry::CACHE_TTL)->toBe(3600); + }); + + it('stores entries in cache after first call', function () { + Cache::fake(); + + $registry = new AgentToolRegistry; + $registry->register(makeTool('plan.create', [])); + + $apiKey = makeApiKey(99, []); + $registry->forApiKey($apiKey); + + expect(Cache::has('agent_tool_registry:api_key:99'))->toBeTrue(); + }); +}); + +// ========================================================================= +// Cache invalidation – flushCacheForApiKey +// ========================================================================= + +describe('flushCacheForApiKey', function () { + beforeEach(function () { + Cache::fake(); + }); + + it('removes the cached entry for the given key id', function () { + $registry = new AgentToolRegistry; + $registry->register(makeTool('plan.create', [])); + + $apiKey = makeApiKey(10, []); + $registry->forApiKey($apiKey); + + expect(Cache::has('agent_tool_registry:api_key:10'))->toBeTrue(); + + $registry->flushCacheForApiKey(10); + + expect(Cache::has('agent_tool_registry:api_key:10'))->toBeFalse(); + }); + + it('re-fetches permitted tools after cache flush', function () { + $registry = new AgentToolRegistry; + $registry->register(makeTool('plan.create', [])); + + $apiKey = makeApiKey(11, []); + + // Prime the cache (only plan.create at this point) + expect($registry->forApiKey($apiKey)->keys()->all())->toBe(['plan.create']); + + $registry->flushCacheForApiKey(11); + + // Register an additional tool – should appear now that cache is gone + $registry->register(makeTool('session.start', [])); + $after = $registry->forApiKey($apiKey)->keys()->sort()->values()->all(); + + expect($after)->toBe(['plan.create', 'session.start']); + }); + + it('does not affect cache entries for other key ids', function () { + $registry = new AgentToolRegistry; + $registry->register(makeTool('plan.create', [])); + + $key12 = makeApiKey(12, []); + $key13 = makeApiKey(13, []); + + $registry->forApiKey($key12); + $registry->forApiKey($key13); + + $registry->flushCacheForApiKey(12); + + expect(Cache::has('agent_tool_registry:api_key:12'))->toBeFalse() + ->and(Cache::has('agent_tool_registry:api_key:13'))->toBeTrue(); + }); + + it('accepts a string key id', function () { + $registry = new AgentToolRegistry; + $registry->register(makeTool('plan.create', [])); + + $apiKey = makeApiKey(20, []); + $registry->forApiKey($apiKey); + + $registry->flushCacheForApiKey('20'); + + expect(Cache::has('agent_tool_registry:api_key:20'))->toBeFalse(); + }); + + it('is a no-op when cache entry does not exist', function () { + $registry = new AgentToolRegistry; + + // Should not throw when nothing is cached + $registry->flushCacheForApiKey(999); + + expect(Cache::has('agent_tool_registry:api_key:999'))->toBeFalse(); + }); +});