refactor: unify ApiKeyManager to use AgentApiKey model (#19)
Some checks failed
CI / PHP 8.4 (pull_request) Failing after 1m35s
CI / PHP 8.3 (pull_request) Failing after 1m46s

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 <noreply@anthropic.com>
This commit is contained in:
darbs-claude 2026-02-23 06:09:05 +00:00
parent d58222cf81
commit 6ebd527204
4 changed files with 303 additions and 55 deletions

View file

@ -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`

View file

@ -68,18 +68,17 @@
</td>
<td class="px-6 py-4 whitespace-nowrap">
<code class="text-sm bg-zinc-100 dark:bg-zinc-700 px-2 py-1 rounded font-mono">
{{ $key->prefix }}_****
{{ $key->getMaskedKey() }}
</code>
</td>
<td class="px-6 py-4 whitespace-nowrap">
<div class="flex gap-1">
@foreach($key->scopes ?? [] as $scope)
<td class="px-6 py-4">
<div class="flex flex-wrap gap-1">
@foreach($key->permissions ?? [] as $permission)
<span class="inline-flex items-center px-2 py-0.5 rounded text-xs font-medium
{{ $scope === 'read' ? 'bg-blue-100 text-blue-800 dark:bg-blue-900/30 dark:text-blue-300' : '' }}
{{ $scope === 'write' ? 'bg-amber-100 text-amber-800 dark:bg-amber-900/30 dark:text-amber-300' : '' }}
{{ $scope === 'delete' ? 'bg-red-100 text-red-800 dark:bg-red-900/30 dark:text-red-300' : '' }}
{{ str_ends_with($permission, '.read') ? 'bg-blue-100 text-blue-800 dark:bg-blue-900/30 dark:text-blue-300' : '' }}
{{ str_ends_with($permission, '.write') || str_ends_with($permission, '.send') || str_ends_with($permission, '.instantiate') ? 'bg-amber-100 text-amber-800 dark:bg-amber-900/30 dark:text-amber-300' : '' }}
">
{{ $scope }}
{{ $permission }}
</span>
@endforeach
</div>
@ -131,11 +130,11 @@
<div class="space-y-3">
<div>
<p class="text-xs font-medium text-zinc-700 dark:text-zinc-300 mb-1">{{ __('mcp::mcp.keys.auth.header_recommended') }}</p>
<pre class="bg-zinc-100 dark:bg-zinc-900 rounded-lg p-2 overflow-x-auto text-xs"><code class="text-zinc-800 dark:text-zinc-200">Authorization: Bearer hk_abc123_****</code></pre>
<pre class="bg-zinc-100 dark:bg-zinc-900 rounded-lg p-2 overflow-x-auto text-xs"><code class="text-zinc-800 dark:text-zinc-200">Authorization: Bearer ak_****</code></pre>
</div>
<div>
<p class="text-xs font-medium text-zinc-700 dark:text-zinc-300 mb-1">{{ __('mcp::mcp.keys.auth.header_api_key') }}</p>
<pre class="bg-zinc-100 dark:bg-zinc-900 rounded-lg p-2 overflow-x-auto text-xs"><code class="text-zinc-800 dark:text-zinc-200">X-API-Key: hk_abc123_****</code></pre>
<pre class="bg-zinc-100 dark:bg-zinc-900 rounded-lg p-2 overflow-x-auto text-xs"><code class="text-zinc-800 dark:text-zinc-200">X-API-Key: ak_****</code></pre>
</div>
</div>
</div>
@ -179,37 +178,24 @@
@enderror
</div>
<!-- Scopes -->
<!-- Permissions -->
<div>
<core:label>{{ __('mcp::mcp.keys.create_modal.permissions_label') }}</core:label>
<div class="mt-2 space-y-2">
<label class="flex items-center gap-2">
<input
type="checkbox"
wire:click="toggleScope('read')"
{{ in_array('read', $newKeyScopes) ? 'checked' : '' }}
class="rounded border-zinc-300 text-cyan-600 focus:ring-cyan-500"
>
<span class="text-sm text-zinc-700 dark:text-zinc-300">{{ __('mcp::mcp.keys.create_modal.permission_read') }}</span>
</label>
<label class="flex items-center gap-2">
<input
type="checkbox"
wire:click="toggleScope('write')"
{{ in_array('write', $newKeyScopes) ? 'checked' : '' }}
class="rounded border-zinc-300 text-cyan-600 focus:ring-cyan-500"
>
<span class="text-sm text-zinc-700 dark:text-zinc-300">{{ __('mcp::mcp.keys.create_modal.permission_write') }}</span>
</label>
<label class="flex items-center gap-2">
<input
type="checkbox"
wire:click="toggleScope('delete')"
{{ in_array('delete', $newKeyScopes) ? 'checked' : '' }}
class="rounded border-zinc-300 text-zinc-600 focus:ring-cyan-500"
>
<span class="text-sm text-zinc-700 dark:text-zinc-300">{{ __('mcp::mcp.keys.create_modal.permission_delete') }}</span>
</label>
@foreach($this->availablePermissions() as $permission => $description)
<label class="flex items-start gap-2">
<input
type="checkbox"
wire:click="togglePermission('{{ $permission }}')"
{{ in_array($permission, $newKeyPermissions) ? 'checked' : '' }}
class="mt-0.5 rounded border-zinc-300 text-cyan-600 focus:ring-cyan-500"
>
<span class="text-sm text-zinc-700 dark:text-zinc-300">
<span class="font-mono text-xs text-zinc-500">{{ $permission }}</span>
<span class="block text-xs text-zinc-500 dark:text-zinc-400">{{ $description }}</span>
</span>
</label>
@endforeach
</div>
</div>

View file

@ -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(),
]);
}
}

View file

@ -0,0 +1,255 @@
<?php
declare(strict_types=1);
/**
* Integration tests for ApiKeyManager admin UI component.
*
* Verifies that ApiKeyManager consistently uses AgentApiKey model
* for all create, list, and revoke operations.
*/
use Carbon\Carbon;
use Core\Mod\Agentic\Models\AgentApiKey;
use Core\Mod\Agentic\Services\AgentApiKeyService;
use Core\Tenant\Models\Workspace;
// =========================================================================
// Model Consistency Tests
// =========================================================================
describe('ApiKeyManager model consistency', function () {
it('ApiKeyManager uses AgentApiKey class', function () {
$source = file_get_contents(__DIR__.'/../../View/Modal/Admin/ApiKeyManager.php');
expect($source)
->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();
}
});
});