From 6ebd52720446d03098807740bb292721f9ebd259 Mon Sep 17 00:00:00 2001 From: darbs-claude Date: Mon, 23 Feb 2026 06:09:05 +0000 Subject: [PATCH] refactor: unify ApiKeyManager to use AgentApiKey model (#19) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switch View/Modal/Admin/ApiKeyManager.php from Core\Api\Models\ApiKey to Core\Mod\Agentic\Models\AgentApiKey and AgentApiKeyService, bringing the workspace-owner admin UI into consistency with all other services. Changes: - Replace Core\Api\Models\ApiKey import with AgentApiKey + AgentApiKeyService - Use AgentApiKeyService::create() for key generation - Use AgentApiKey::forWorkspace() scoping in revokeKey() and render() - Rename newKeyScopes → newKeyPermissions, toggleScope → togglePermission - Expose availablePermissions() from AgentApiKey for the create form - Update blade template: permissions field, getMaskedKey(), togglePermission, dynamic permission checkboxes from AgentApiKey::availablePermissions() - Add tests/Feature/ApiKeyManagerTest.php with integration coverage - Mark CQ-002 resolved in TODO.md Co-Authored-By: Claude Sonnet 4.6 --- TODO.md | 6 +- View/Blade/admin/api-key-manager.blade.php | 62 ++--- View/Modal/Admin/ApiKeyManager.php | 35 +-- tests/Feature/ApiKeyManagerTest.php | 255 +++++++++++++++++++++ 4 files changed, 303 insertions(+), 55 deletions(-) create mode 100644 tests/Feature/ApiKeyManagerTest.php diff --git a/TODO.md b/TODO.md index 9be662d..c69793b 100644 --- a/TODO.md +++ b/TODO.md @@ -136,10 +136,12 @@ Production-quality task list for the AI agent orchestration package. - Issue: Two similar models for same purpose - Fix: Consolidate into single model, or clarify distinct purposes -- [ ] **CQ-002: ApiKeyManager uses Core\Api\ApiKey, not AgentApiKey** +- [x] **CQ-002: ApiKeyManager uses Core\Api\ApiKey, not AgentApiKey** (FIXED 2026-02-23) - Location: `View/Modal/Admin/ApiKeyManager.php` - Issue: Livewire component uses different API key model than services - - Fix: Unify on AgentApiKey or document distinction + - Fix: Switched to `AgentApiKey` model and `AgentApiKeyService` throughout + - Updated blade template to use `permissions`, `getMaskedKey()`, `togglePermission()` + - Added integration tests in `tests/Feature/ApiKeyManagerTest.php` - [ ] **CQ-003: ForAgentsController cache key not namespaced** - Location: `Controllers/ForAgentsController.php` diff --git a/View/Blade/admin/api-key-manager.blade.php b/View/Blade/admin/api-key-manager.blade.php index 7226a73..33a8d23 100644 --- a/View/Blade/admin/api-key-manager.blade.php +++ b/View/Blade/admin/api-key-manager.blade.php @@ -68,18 +68,17 @@ - {{ $key->prefix }}_**** + {{ $key->getMaskedKey() }} - -
- @foreach($key->scopes ?? [] as $scope) + +
+ @foreach($key->permissions ?? [] as $permission) - {{ $scope }} + {{ $permission }} @endforeach
@@ -131,11 +130,11 @@

{{ __('mcp::mcp.keys.auth.header_recommended') }}

- 
Authorization: Bearer hk_abc123_****
+ 
Authorization: Bearer ak_****

{{ __('mcp::mcp.keys.auth.header_api_key') }}

- 
X-API-Key: hk_abc123_****
+ 
X-API-Key: ak_****
@@ -179,37 +178,24 @@ @enderror - +
{{ __('mcp::mcp.keys.create_modal.permissions_label') }}
- - - + @foreach($this->availablePermissions() as $permission => $description) + + @endforeach
diff --git a/View/Modal/Admin/ApiKeyManager.php b/View/Modal/Admin/ApiKeyManager.php index 88a8663..c5e9718 100644 --- a/View/Modal/Admin/ApiKeyManager.php +++ b/View/Modal/Admin/ApiKeyManager.php @@ -4,7 +4,8 @@ declare(strict_types=1); namespace Core\Mod\Agentic\View\Modal\Admin; -use Core\Api\Models\ApiKey; +use Core\Mod\Agentic\Models\AgentApiKey; +use Core\Mod\Agentic\Services\AgentApiKeyService; use Core\Tenant\Models\Workspace; use Livewire\Attributes\Layout; use Livewire\Component; @@ -25,7 +26,7 @@ class ApiKeyManager extends Component public string $newKeyName = ''; - public array $newKeyScopes = ['read', 'write']; + public array $newKeyPermissions = []; public string $newKeyExpiry = 'never'; @@ -43,7 +44,7 @@ class ApiKeyManager extends Component { $this->showCreateModal = true; $this->newKeyName = ''; - $this->newKeyScopes = ['read', 'write']; + $this->newKeyPermissions = []; $this->newKeyExpiry = 'never'; } @@ -52,6 +53,11 @@ class ApiKeyManager extends Component $this->showCreateModal = false; } + public function availablePermissions(): array + { + return AgentApiKey::availablePermissions(); + } + public function createKey(): void { $this->validate([ @@ -65,15 +71,14 @@ class ApiKeyManager extends Component default => null, }; - $result = ApiKey::generate( - workspaceId: $this->workspace->id, - userId: auth()->id(), + $key = app(AgentApiKeyService::class)->create( + workspace: $this->workspace, name: $this->newKeyName, - scopes: $this->newKeyScopes, + permissions: $this->newKeyPermissions, expiresAt: $expiresAt, ); - $this->newPlainKey = $result['plain_key']; + $this->newPlainKey = $key->plainTextKey; $this->showCreateModal = false; $this->showNewKeyModal = true; @@ -88,25 +93,25 @@ class ApiKeyManager extends Component public function revokeKey(int $keyId): void { - $key = $this->workspace->apiKeys()->findOrFail($keyId); + $key = AgentApiKey::forWorkspace($this->workspace)->findOrFail($keyId); $key->revoke(); session()->flash('message', 'API key revoked.'); } - public function toggleScope(string $scope): void + public function togglePermission(string $permission): void { - if (in_array($scope, $this->newKeyScopes)) { - $this->newKeyScopes = array_values(array_diff($this->newKeyScopes, [$scope])); + if (in_array($permission, $this->newKeyPermissions)) { + $this->newKeyPermissions = array_values(array_diff($this->newKeyPermissions, [$permission])); } else { - $this->newKeyScopes[] = $scope; + $this->newKeyPermissions[] = $permission; } } public function render() { - return view('mcp::admin.api-key-manager', [ - 'keys' => $this->workspace->apiKeys()->orderByDesc('created_at')->get(), + return view('agentic::admin.api-key-manager', [ + 'keys' => AgentApiKey::forWorkspace($this->workspace)->orderByDesc('created_at')->get(), ]); } } diff --git a/tests/Feature/ApiKeyManagerTest.php b/tests/Feature/ApiKeyManagerTest.php new file mode 100644 index 0000000..9123e11 --- /dev/null +++ b/tests/Feature/ApiKeyManagerTest.php @@ -0,0 +1,255 @@ +toContain('Core\Mod\Agentic\Models\AgentApiKey') + ->not->toContain('Core\Api\Models\ApiKey') + ->not->toContain('Core\Api\ApiKey'); + }); + + it('ApiKeyManager uses AgentApiKeyService', function () { + $source = file_get_contents(__DIR__.'/../../View/Modal/Admin/ApiKeyManager.php'); + + expect($source)->toContain('Core\Mod\Agentic\Services\AgentApiKeyService'); + }); + + it('ApiKeyManager does not reference old scopes property', function () { + $source = file_get_contents(__DIR__.'/../../View/Modal/Admin/ApiKeyManager.php'); + + expect($source) + ->not->toContain('newKeyScopes') + ->not->toContain('toggleScope'); + }); + + it('blade template uses permissions not scopes', function () { + $source = file_get_contents(__DIR__.'/../../View/Blade/admin/api-key-manager.blade.php'); + + expect($source) + ->toContain('$key->permissions') + ->not->toContain('$key->scopes'); + }); + + it('blade template uses getMaskedKey not prefix', function () { + $source = file_get_contents(__DIR__.'/../../View/Blade/admin/api-key-manager.blade.php'); + + expect($source) + ->toContain('getMaskedKey()') + ->not->toContain('$key->prefix'); + }); + + it('blade template calls togglePermission not toggleScope', function () { + $source = file_get_contents(__DIR__.'/../../View/Blade/admin/api-key-manager.blade.php'); + + expect($source) + ->toContain('togglePermission') + ->not->toContain('toggleScope'); + }); +}); + +// ========================================================================= +// AgentApiKey Integration Tests (via service, as used by ApiKeyManager) +// ========================================================================= + +describe('ApiKeyManager key creation integration', function () { + it('creates an AgentApiKey via service', function () { + $workspace = createWorkspace(); + $service = app(AgentApiKeyService::class); + + $key = $service->create( + workspace: $workspace, + name: 'Workspace MCP Key', + permissions: [AgentApiKey::PERM_PLANS_READ, AgentApiKey::PERM_SESSIONS_READ], + ); + + expect($key)->toBeInstanceOf(AgentApiKey::class) + ->and($key->name)->toBe('Workspace MCP Key') + ->and($key->workspace_id)->toBe($workspace->id) + ->and($key->permissions)->toContain(AgentApiKey::PERM_PLANS_READ) + ->and($key->plainTextKey)->toStartWith('ak_'); + }); + + it('plain text key is only available once after creation', function () { + $workspace = createWorkspace(); + $service = app(AgentApiKeyService::class); + + $key = $service->create($workspace, 'One-time key'); + + expect($key->plainTextKey)->not->toBeNull(); + + $freshKey = AgentApiKey::find($key->id); + expect($freshKey->plainTextKey)->toBeNull(); + }); + + it('creates key with expiry date', function () { + $workspace = createWorkspace(); + $service = app(AgentApiKeyService::class); + $expiresAt = now()->addDays(30); + + $key = $service->create( + workspace: $workspace, + name: 'Expiring Key', + expiresAt: $expiresAt, + ); + + expect($key->expires_at)->not->toBeNull() + ->and($key->expires_at->toDateString())->toBe($expiresAt->toDateString()); + }); + + it('creates key with no expiry when null passed', function () { + $workspace = createWorkspace(); + $service = app(AgentApiKeyService::class); + + $key = $service->create($workspace, 'Permanent Key', expiresAt: null); + + expect($key->expires_at)->toBeNull(); + }); +}); + +// ========================================================================= +// Workspace Scoping (used by ApiKeyManager::revokeKey and render) +// ========================================================================= + +describe('ApiKeyManager workspace scoping', function () { + it('forWorkspace scope returns only keys for given workspace', function () { + $workspace1 = createWorkspace(); + $workspace2 = createWorkspace(); + + $key1 = createApiKey($workspace1, 'Key for workspace 1'); + $key2 = createApiKey($workspace2, 'Key for workspace 2'); + + $keys = AgentApiKey::forWorkspace($workspace1)->get(); + + expect($keys)->toHaveCount(1) + ->and($keys->first()->id)->toBe($key1->id); + }); + + it('forWorkspace accepts workspace model', function () { + $workspace = createWorkspace(); + createApiKey($workspace, 'Key'); + + $keys = AgentApiKey::forWorkspace($workspace)->get(); + + expect($keys)->toHaveCount(1); + }); + + it('forWorkspace accepts workspace ID', function () { + $workspace = createWorkspace(); + createApiKey($workspace, 'Key'); + + $keys = AgentApiKey::forWorkspace($workspace->id)->get(); + + expect($keys)->toHaveCount(1); + }); + + it('forWorkspace prevents cross-workspace key access', function () { + $workspace1 = createWorkspace(); + $workspace2 = createWorkspace(); + + $key = createApiKey($workspace1, 'Workspace 1 key'); + + // Attempting to find workspace1's key while scoped to workspace2 + $found = AgentApiKey::forWorkspace($workspace2)->find($key->id); + + expect($found)->toBeNull(); + }); +}); + +// ========================================================================= +// Revoke Integration (as used by ApiKeyManager::revokeKey) +// ========================================================================= + +describe('ApiKeyManager key revocation integration', function () { + it('revokes a key via service', function () { + $workspace = createWorkspace(); + $key = createApiKey($workspace, 'Key to revoke'); + $service = app(AgentApiKeyService::class); + + expect($key->isActive())->toBeTrue(); + + $service->revoke($key); + + expect($key->fresh()->isRevoked())->toBeTrue(); + }); + + it('revoked key is inactive', function () { + $workspace = createWorkspace(); + $key = createApiKey($workspace, 'Key to revoke'); + + $key->revoke(); + + expect($key->isActive())->toBeFalse() + ->and($key->isRevoked())->toBeTrue(); + }); + + it('revoking clears validation', function () { + $workspace = createWorkspace(); + $key = createApiKey($workspace, 'Key to revoke'); + $service = app(AgentApiKeyService::class); + + $plainKey = $key->plainTextKey; + $service->revoke($key); + + $validated = $service->validate($plainKey); + expect($validated)->toBeNull(); + }); +}); + +// ========================================================================= +// Available Permissions (used by ApiKeyManager::availablePermissions) +// ========================================================================= + +describe('ApiKeyManager available permissions', function () { + it('AgentApiKey provides available permissions list', function () { + $permissions = AgentApiKey::availablePermissions(); + + expect($permissions) + ->toBeArray() + ->toHaveKey(AgentApiKey::PERM_PLANS_READ) + ->toHaveKey(AgentApiKey::PERM_PLANS_WRITE) + ->toHaveKey(AgentApiKey::PERM_SESSIONS_READ) + ->toHaveKey(AgentApiKey::PERM_SESSIONS_WRITE); + }); + + it('permission constants match available permissions keys', function () { + $permissions = AgentApiKey::availablePermissions(); + + expect(array_keys($permissions)) + ->toContain(AgentApiKey::PERM_PLANS_READ) + ->toContain(AgentApiKey::PERM_PHASES_WRITE) + ->toContain(AgentApiKey::PERM_TEMPLATES_READ); + }); + + it('key can be created with any available permission', function () { + $workspace = createWorkspace(); + $allPermissions = array_keys(AgentApiKey::availablePermissions()); + + $key = createApiKey($workspace, 'Full Access', $allPermissions); + + expect($key->permissions)->toBe($allPermissions); + + foreach ($allPermissions as $permission) { + expect($key->hasPermission($permission))->toBeTrue(); + } + }); +}); -- 2.45.3