From 9ae98b07053ea1d4bfc13d29f5ea108c084006a5 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Fri, 15 May 2026 15:59:14 +0200 Subject: [PATCH] fix: tighten workspace RBAC access boundaries --- apps/platform/app/Models/User.php | 2 +- .../Auth/WorkspaceRoleCapabilityMap.php | 5 - ...spaceFirstManagedEnvironmentAccessTest.php | 2 +- .../Rbac/AdminPanelAccessBoundaryTest.php | 38 ++ ...ngExceptionLifecycleAccessBoundaryTest.php | 99 +++++ .../ManagedEnvironmentAccessBoundaryTest.php | 107 +++++ .../Rbac/OperationRunAccessBoundaryTest.php | 90 +++++ .../ProviderConnectionAccessBoundaryTest.php | 81 ++++ .../Rbac/ReviewPackAccessBoundaryTest.php | 88 ++++ .../Rbac/RoleMatrix/ManagerAccessTest.php | 2 +- .../Rbac/SystemPanelAccessBoundaryTest.php | 40 ++ ...rshipsRelationManagerUiEnforcementTest.php | 24 ++ .../WorkspaceRoleCapabilityBoundaryTest.php | 102 +++++ .../Unit/Auth/CapabilityResolverTest.php | 2 +- .../Auth/WorkspaceRoleCapabilityMapTest.php | 35 ++ .../checklists/requirements.md | 48 +++ .../plan.md | 348 ++++++++++++++++ .../spec.md | 381 ++++++++++++++++++ .../tasks.md | 204 ++++++++++ 19 files changed, 1689 insertions(+), 9 deletions(-) create mode 100644 apps/platform/tests/Feature/Rbac/AdminPanelAccessBoundaryTest.php create mode 100644 apps/platform/tests/Feature/Rbac/FindingExceptionLifecycleAccessBoundaryTest.php create mode 100644 apps/platform/tests/Feature/Rbac/ManagedEnvironmentAccessBoundaryTest.php create mode 100644 apps/platform/tests/Feature/Rbac/OperationRunAccessBoundaryTest.php create mode 100644 apps/platform/tests/Feature/Rbac/ProviderConnectionAccessBoundaryTest.php create mode 100644 apps/platform/tests/Feature/Rbac/ReviewPackAccessBoundaryTest.php create mode 100644 apps/platform/tests/Feature/Rbac/SystemPanelAccessBoundaryTest.php create mode 100644 apps/platform/tests/Feature/Rbac/WorkspaceRoleCapabilityBoundaryTest.php create mode 100644 apps/platform/tests/Unit/Auth/WorkspaceRoleCapabilityMapTest.php create mode 100644 specs/309-rbac-role-matrix-access-boundary-audit/checklists/requirements.md create mode 100644 specs/309-rbac-role-matrix-access-boundary-audit/plan.md create mode 100644 specs/309-rbac-role-matrix-access-boundary-audit/spec.md create mode 100644 specs/309-rbac-role-matrix-access-boundary-audit/tasks.md diff --git a/apps/platform/app/Models/User.php b/apps/platform/app/Models/User.php index 406163dc..38c82610 100644 --- a/apps/platform/app/Models/User.php +++ b/apps/platform/app/Models/User.php @@ -69,7 +69,7 @@ protected function casts(): array public function canAccessPanel(Panel $panel): bool { - return true; + return $panel->getId() === 'admin'; } public function tenants(): BelongsToMany diff --git a/apps/platform/app/Services/Auth/WorkspaceRoleCapabilityMap.php b/apps/platform/app/Services/Auth/WorkspaceRoleCapabilityMap.php index 8735bdd2..f33cfc5d 100644 --- a/apps/platform/app/Services/Auth/WorkspaceRoleCapabilityMap.php +++ b/apps/platform/app/Services/Auth/WorkspaceRoleCapabilityMap.php @@ -47,7 +47,6 @@ class WorkspaceRoleCapabilityMap WorkspaceRole::Manager->value => [ Capabilities::WORKSPACE_VIEW, Capabilities::WORKSPACE_MEMBERSHIP_VIEW, - Capabilities::WORKSPACE_MEMBERSHIP_MANAGE, Capabilities::WORKSPACE_MANAGED_ENVIRONMENT_ONBOARD, Capabilities::WORKSPACE_MANAGED_ENVIRONMENT_ONBOARD_IDENTIFY, Capabilities::WORKSPACE_MANAGED_ENVIRONMENT_ONBOARD_CANCEL, @@ -102,10 +101,6 @@ public static function getCapabilities(WorkspaceRole|string $role): array RoleCapabilityMap::getCapabilities($roleValue), ); - if ($roleValue === WorkspaceRole::Manager->value) { - $capabilities[] = Capabilities::TENANT_MEMBERSHIP_MANAGE; - } - return array_values(array_unique($capabilities)); } diff --git a/apps/platform/tests/Feature/Auth/WorkspaceFirstManagedEnvironmentAccessTest.php b/apps/platform/tests/Feature/Auth/WorkspaceFirstManagedEnvironmentAccessTest.php index b84accd7..0de929be 100644 --- a/apps/platform/tests/Feature/Auth/WorkspaceFirstManagedEnvironmentAccessTest.php +++ b/apps/platform/tests/Feature/Auth/WorkspaceFirstManagedEnvironmentAccessTest.php @@ -76,7 +76,7 @@ /** @var \Filament\Panel $panel */ $panel = app(PanelRegistry::class)->get('admin'); - $allowedDecision = app(ManagedEnvironmentAccessScopeResolver::class)->decision($user, $allowedTenant, Capabilities::WORKSPACE_MEMBERSHIP_MANAGE); + $allowedDecision = app(ManagedEnvironmentAccessScopeResolver::class)->decision($user, $allowedTenant, Capabilities::WORKSPACE_SETTINGS_VIEW); $defaultTenant = $user->getDefaultTenant($panel); $tenants = $user->getTenants($panel); diff --git a/apps/platform/tests/Feature/Rbac/AdminPanelAccessBoundaryTest.php b/apps/platform/tests/Feature/Rbac/AdminPanelAccessBoundaryTest.php new file mode 100644 index 00000000..3cf36dc9 --- /dev/null +++ b/apps/platform/tests/Feature/Rbac/AdminPanelAccessBoundaryTest.php @@ -0,0 +1,38 @@ +make(); + + expect($user->canAccessPanel(app(PanelRegistry::class)->get('admin')))->toBeTrue() + ->and($user->canAccessPanel(app(PanelRegistry::class)->get('system')))->toBeFalse(); +}); + +it('redirects unauthenticated direct admin panel access to admin login', function (): void { + $this->get('/admin')->assertRedirectContains('/admin/login'); +}); + +it('denies platform sessions on admin panel routes as not found', function (): void { + $platformUser = PlatformUser::factory()->create(); + + $this->actingAs($platformUser, 'platform') + ->get('/admin') + ->assertNotFound(); +}); + +it('does not render workspace admin surfaces for users without workspace authority', function (): void { + $user = User::factory()->create(); + + $this->actingAs($user) + ->get('/admin') + ->assertRedirect('/admin/choose-workspace'); + + $this->actingAs($user) + ->get('/admin/alerts') + ->assertRedirect('/admin/choose-workspace'); +}); diff --git a/apps/platform/tests/Feature/Rbac/FindingExceptionLifecycleAccessBoundaryTest.php b/apps/platform/tests/Feature/Rbac/FindingExceptionLifecycleAccessBoundaryTest.php new file mode 100644 index 00000000..9713cf34 --- /dev/null +++ b/apps/platform/tests/Feature/Rbac/FindingExceptionLifecycleAccessBoundaryTest.php @@ -0,0 +1,99 @@ +value); + [$readonly] = createUserWithTenant(tenant: $tenant, role: WorkspaceRole::Readonly->value); + + $finding = Finding::factory()->for($tenant)->create([ + 'workspace_id' => (int) $tenant->workspace_id, + ]); + $exception = FindingException::query()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'managed_environment_id' => (int) $tenant->getKey(), + 'finding_id' => (int) $finding->getKey(), + 'requested_by_user_id' => (int) $owner->getKey(), + 'owner_user_id' => (int) $owner->getKey(), + 'status' => FindingException::STATUS_PENDING, + 'current_validity_state' => FindingException::VALIDITY_MISSING_SUPPORT, + 'request_reason' => 'Boundary denial proof', + 'requested_at' => now(), + 'review_due_at' => now()->addDays(7), + 'evidence_summary' => ['reference_count' => 0], + ]); + + session()->put(WorkspaceContext::SESSION_KEY, (int) $tenant->workspace_id); + + foreach (['approve', 'reject'] as $ability) { + $response = Gate::forUser($readonly)->inspect($ability, $exception); + + expect($response->denied())->toBeTrue() + ->and($response->status())->not->toBe(404); + } +}); + +it('denies same-workspace wrong-environment finding exceptions as not found', function (): void { + $workspace = Workspace::factory()->create(); + $allowedTenant = ManagedEnvironment::factory()->active()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + $deniedTenant = ManagedEnvironment::factory()->active()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + $user = User::factory()->create(); + $owner = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => WorkspaceRole::Manager->value, + ]); + ManagedEnvironmentMembership::query()->create([ + 'managed_environment_id' => (int) $allowedTenant->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => WorkspaceRole::Readonly->value, + 'source' => 'manual', + ]); + app(CapabilityResolver::class)->clearCache(); + app(ManagedEnvironmentAccessScopeResolver::class)->clearCache(); + + $finding = Finding::factory()->for($deniedTenant)->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + $exception = FindingException::query()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'managed_environment_id' => (int) $deniedTenant->getKey(), + 'finding_id' => (int) $finding->getKey(), + 'requested_by_user_id' => (int) $owner->getKey(), + 'owner_user_id' => (int) $owner->getKey(), + 'status' => FindingException::STATUS_PENDING, + 'current_validity_state' => FindingException::VALIDITY_MISSING_SUPPORT, + 'request_reason' => 'Wrong environment boundary proof', + 'requested_at' => now(), + 'review_due_at' => now()->addDays(7), + 'evidence_summary' => ['reference_count' => 0], + ]); + + $allowedTenant->makeCurrent(); + session()->put(WorkspaceContext::SESSION_KEY, (int) $workspace->getKey()); + app(WorkspaceContext::class)->rememberLastTenantId((int) $workspace->getKey(), (int) $allowedTenant->getKey()); + + $response = Gate::forUser($user)->inspect('view', $exception); + + expect($response->denied())->toBeTrue() + ->and($response->status())->toBe(404); +}); diff --git a/apps/platform/tests/Feature/Rbac/ManagedEnvironmentAccessBoundaryTest.php b/apps/platform/tests/Feature/Rbac/ManagedEnvironmentAccessBoundaryTest.php new file mode 100644 index 00000000..24a7b297 --- /dev/null +++ b/apps/platform/tests/Feature/Rbac/ManagedEnvironmentAccessBoundaryTest.php @@ -0,0 +1,107 @@ +create(); + $allowedTenant = ManagedEnvironment::factory()->active()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + $deniedTenant = ManagedEnvironment::factory()->active()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + $user = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => WorkspaceRole::Manager->value, + ]); + ManagedEnvironmentMembership::query()->create([ + 'managed_environment_id' => (int) $allowedTenant->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => WorkspaceRole::Readonly->value, + 'source' => 'manual', + ]); + app(CapabilityResolver::class)->clearCache(); + app(ManagedEnvironmentAccessScopeResolver::class)->clearCache(); + + $snapshot = EvidenceSnapshot::query()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'managed_environment_id' => (int) $deniedTenant->getKey(), + 'status' => EvidenceSnapshotStatus::Active->value, + 'completeness_state' => EvidenceCompletenessState::Complete->value, + 'summary' => ['finding_count' => 1], + 'generated_at' => now(), + ]); + + $review = EnvironmentReview::factory()->ready()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'managed_environment_id' => (int) $deniedTenant->getKey(), + 'evidence_snapshot_id' => (int) $snapshot->getKey(), + ]); + + $this->actingAs($user) + ->withSession([WorkspaceContext::SESSION_KEY => (int) $workspace->getKey()]) + ->get(EnvironmentReviewResource::getUrl('view', ['record' => $review], panel: 'admin', tenant: $allowedTenant)) + ->assertNotFound(); +}); + +it('denies same-workspace wrong-environment evidence direct access as not found', function (): void { + $workspace = Workspace::factory()->create(); + $allowedTenant = ManagedEnvironment::factory()->active()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + $deniedTenant = ManagedEnvironment::factory()->active()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + $user = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => WorkspaceRole::Manager->value, + ]); + ManagedEnvironmentMembership::query()->create([ + 'managed_environment_id' => (int) $allowedTenant->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => WorkspaceRole::Readonly->value, + 'source' => 'manual', + ]); + app(CapabilityResolver::class)->clearCache(); + app(ManagedEnvironmentAccessScopeResolver::class)->clearCache(); + + $snapshot = EvidenceSnapshot::query()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'managed_environment_id' => (int) $deniedTenant->getKey(), + 'status' => EvidenceSnapshotStatus::Active->value, + 'completeness_state' => EvidenceCompletenessState::Complete->value, + 'summary' => ['finding_count' => 1], + 'generated_at' => now(), + ]); + + $allowedTenant->makeCurrent(); + Filament::setTenant($allowedTenant, true); + + $this->actingAs($user) + ->withSession([WorkspaceContext::SESSION_KEY => (int) $workspace->getKey()]) + ->get(EvidenceSnapshotResource::getUrl('view', ['record' => $snapshot], panel: 'admin', tenant: $allowedTenant)) + ->assertNotFound(); +}); diff --git a/apps/platform/tests/Feature/Rbac/OperationRunAccessBoundaryTest.php b/apps/platform/tests/Feature/Rbac/OperationRunAccessBoundaryTest.php new file mode 100644 index 00000000..8d68fe93 --- /dev/null +++ b/apps/platform/tests/Feature/Rbac/OperationRunAccessBoundaryTest.php @@ -0,0 +1,90 @@ +create(); + $otherWorkspace = Workspace::factory()->create(); + $user = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => WorkspaceRole::Readonly->value, + ]); + + $run = OperationRun::factory()->tenantlessForWorkspace($otherWorkspace)->create([ + 'type' => 'environment.review.compose', + ]); + + $response = Gate::forUser($user)->inspect('view', $run); + + expect($response->denied())->toBeTrue() + ->and($response->status())->toBe(404); +}); + +it('denies same-workspace wrong-environment operation runs as not found', function (): void { + $workspace = Workspace::factory()->create(); + $allowedTenant = ManagedEnvironment::factory()->active()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + $deniedTenant = ManagedEnvironment::factory()->active()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + $user = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => WorkspaceRole::Operator->value, + ]); + ManagedEnvironmentMembership::query()->create([ + 'managed_environment_id' => (int) $allowedTenant->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => WorkspaceRole::Operator->value, + 'source' => 'manual', + ]); + app(ManagedEnvironmentAccessScopeResolver::class)->clearCache(); + + $run = OperationRun::factory()->forTenant($deniedTenant)->create([ + 'type' => 'provider.connection.check', + ]); + + $response = Gate::forUser($user)->inspect('view', $run); + + expect($response->denied())->toBeTrue() + ->and($response->status())->toBe(404); +}); + +it('keeps in-scope operation capability denials distinct from scope boundaries', function (): void { + $workspace = Workspace::factory()->create(); + $tenant = ManagedEnvironment::factory()->active()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + $user = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => WorkspaceRole::Readonly->value, + ]); + + $run = OperationRun::factory()->forTenant($tenant)->create([ + 'type' => 'inventory.sync', + ]); + + $response = Gate::forUser($user)->inspect('view', $run); + + expect($response->denied())->toBeTrue() + ->and($response->status())->not->toBe(404); +}); diff --git a/apps/platform/tests/Feature/Rbac/ProviderConnectionAccessBoundaryTest.php b/apps/platform/tests/Feature/Rbac/ProviderConnectionAccessBoundaryTest.php new file mode 100644 index 00000000..bd871c8d --- /dev/null +++ b/apps/platform/tests/Feature/Rbac/ProviderConnectionAccessBoundaryTest.php @@ -0,0 +1,81 @@ +create(); + $tenant = ManagedEnvironment::factory()->active()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + + $connection = ProviderConnection::factory()->withCredential()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'managed_environment_id' => (int) $tenant->getKey(), + ]); + + foreach (WorkspaceRole::cases() as $role) { + $user = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => $role->value, + ]); + + session()->put(WorkspaceContext::SESSION_KEY, (int) $workspace->getKey()); + app(CapabilityResolver::class)->clearCache(); + app(ManagedEnvironmentAccessScopeResolver::class)->clearCache(); + + expect(Gate::forUser($user)->allows('manageDedicatedCredential', $connection))->toBe($role === WorkspaceRole::Owner); + expect(Gate::forUser($user)->allows('deleteDedicatedCredential', $connection))->toBe($role === WorkspaceRole::Owner); + } +}); + +it('denies same-workspace wrong-environment provider connections as not found', function (): void { + $workspace = Workspace::factory()->create(); + $allowedTenant = ManagedEnvironment::factory()->active()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + $deniedTenant = ManagedEnvironment::factory()->active()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + $user = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => WorkspaceRole::Manager->value, + ]); + ManagedEnvironmentMembership::query()->create([ + 'managed_environment_id' => (int) $allowedTenant->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => WorkspaceRole::Readonly->value, + 'source' => 'manual', + ]); + app(CapabilityResolver::class)->clearCache(); + app(ManagedEnvironmentAccessScopeResolver::class)->clearCache(); + + $connection = ProviderConnection::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'managed_environment_id' => (int) $deniedTenant->getKey(), + ]); + + session()->put(WorkspaceContext::SESSION_KEY, (int) $workspace->getKey()); + + $response = Gate::forUser($user)->inspect('view', $connection); + + expect($response->denied())->toBeTrue() + ->and($response->status())->toBe(404); +}); diff --git a/apps/platform/tests/Feature/Rbac/ReviewPackAccessBoundaryTest.php b/apps/platform/tests/Feature/Rbac/ReviewPackAccessBoundaryTest.php new file mode 100644 index 00000000..476e3bf4 --- /dev/null +++ b/apps/platform/tests/Feature/Rbac/ReviewPackAccessBoundaryTest.php @@ -0,0 +1,88 @@ +active()->create(); + $otherTenant = ManagedEnvironment::factory()->active()->create(); + + [$user] = createUserWithTenant($otherTenant, role: WorkspaceRole::Owner->value); + + $pack = ReviewPack::factory()->ready()->create([ + 'managed_environment_id' => (int) $targetTenant->getKey(), + 'workspace_id' => (int) $targetTenant->workspace_id, + ]); + + $this->actingAs($user) + ->get(ReviewPackResource::getUrl('view', ['record' => $pack], panel: 'admin', tenant: $targetTenant)) + ->assertNotFound(); +}); + +it('denies same-workspace wrong-environment review pack direct access as not found', function (): void { + $workspace = Workspace::factory()->create(); + $allowedTenant = ManagedEnvironment::factory()->active()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + $deniedTenant = ManagedEnvironment::factory()->active()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + $user = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => WorkspaceRole::Manager->value, + ]); + ManagedEnvironmentMembership::query()->create([ + 'managed_environment_id' => (int) $allowedTenant->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => WorkspaceRole::Readonly->value, + 'source' => 'manual', + ]); + app(CapabilityResolver::class)->clearCache(); + app(ManagedEnvironmentAccessScopeResolver::class)->clearCache(); + + $pack = ReviewPack::factory()->ready()->create([ + 'managed_environment_id' => (int) $deniedTenant->getKey(), + 'workspace_id' => (int) $workspace->getKey(), + ]); + + $allowedTenant->makeCurrent(); + Filament::setTenant($allowedTenant, true); + + $this->actingAs($user) + ->withSession([WorkspaceContext::SESSION_KEY => (int) $workspace->getKey()]) + ->get(ReviewPackResource::getUrl('view', ['record' => $pack], panel: 'admin', tenant: $allowedTenant)) + ->assertNotFound(); +}); + +it('denies readonly review pack mutation server-side', function (): void { + [$user, $tenant] = createUserWithTenant(role: WorkspaceRole::Readonly->value); + + $pack = ReviewPack::factory()->ready()->create([ + 'managed_environment_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + ]); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + $response = Gate::forUser($user)->inspect('delete', $pack); + + expect($response->denied())->toBeTrue() + ->and($response->status())->not->toBe(404); +}); diff --git a/apps/platform/tests/Feature/Rbac/RoleMatrix/ManagerAccessTest.php b/apps/platform/tests/Feature/Rbac/RoleMatrix/ManagerAccessTest.php index 99937fa8..fe64889f 100644 --- a/apps/platform/tests/Feature/Rbac/RoleMatrix/ManagerAccessTest.php +++ b/apps/platform/tests/Feature/Rbac/RoleMatrix/ManagerAccessTest.php @@ -23,6 +23,6 @@ expect($gate->allows(Capabilities::AUDIT_VIEW, $tenant))->toBeTrue(); - expect($gate->allows(Capabilities::TENANT_MEMBERSHIP_MANAGE, $tenant))->toBeTrue(); + expect($gate->allows(Capabilities::TENANT_MEMBERSHIP_MANAGE, $tenant))->toBeFalse(); expect($gate->allows(Capabilities::TENANT_DELETE, $tenant))->toBeFalse(); }); diff --git a/apps/platform/tests/Feature/Rbac/SystemPanelAccessBoundaryTest.php b/apps/platform/tests/Feature/Rbac/SystemPanelAccessBoundaryTest.php new file mode 100644 index 00000000..5393fa4c --- /dev/null +++ b/apps/platform/tests/Feature/Rbac/SystemPanelAccessBoundaryTest.php @@ -0,0 +1,40 @@ +make([ + 'capabilities' => [PlatformCapabilities::ACCESS_SYSTEM_PANEL], + ]); + + expect($platformUser->canAccessPanel(app(PanelRegistry::class)->get('system')))->toBeTrue() + ->and($platformUser->canAccessPanel(app(PanelRegistry::class)->get('admin')))->toBeFalse(); +}); + +it('denies ordinary workspace users on system panel routes as not found', function (): void { + [$user] = createUserWithTenant(role: 'owner'); + + $this->actingAs($user) + ->get('/system') + ->assertNotFound(); + + $this->actingAs($user) + ->get('/system/directory/workspaces') + ->assertNotFound(); +}); + +it('keeps missing system capability as forbidden for platform users', function (): void { + $platformUser = PlatformUser::factory()->create([ + 'capabilities' => [], + 'is_active' => true, + ]); + + $this->actingAs($platformUser, 'platform') + ->get('/system') + ->assertForbidden(); +}); diff --git a/apps/platform/tests/Feature/Rbac/WorkspaceMembershipsRelationManagerUiEnforcementTest.php b/apps/platform/tests/Feature/Rbac/WorkspaceMembershipsRelationManagerUiEnforcementTest.php index facf7d53..33c7f552 100644 --- a/apps/platform/tests/Feature/Rbac/WorkspaceMembershipsRelationManagerUiEnforcementTest.php +++ b/apps/platform/tests/Feature/Rbac/WorkspaceMembershipsRelationManagerUiEnforcementTest.php @@ -77,4 +77,28 @@ return $action->getTooltip() === 'You do not have permission to manage workspace memberships.'; }); }); + + it('shows membership actions as visible but disabled for manager members', function () { + $workspace = Workspace::factory()->create(); + $user = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => 'manager', + ]); + + $this->actingAs($user); + + Livewire::test(WorkspaceMembershipsRelationManager::class, [ + 'ownerRecord' => $workspace, + 'pageClass' => EditWorkspace::class, + ]) + ->assertTableActionVisible('add_member') + ->assertTableActionDisabled('add_member') + ->assertTableActionVisible('change_role') + ->assertTableActionDisabled('change_role') + ->assertTableActionVisible('remove') + ->assertTableActionDisabled('remove'); + }); }); diff --git a/apps/platform/tests/Feature/Rbac/WorkspaceRoleCapabilityBoundaryTest.php b/apps/platform/tests/Feature/Rbac/WorkspaceRoleCapabilityBoundaryTest.php new file mode 100644 index 00000000..7eaefae2 --- /dev/null +++ b/apps/platform/tests/Feature/Rbac/WorkspaceRoleCapabilityBoundaryTest.php @@ -0,0 +1,102 @@ +create(); + + foreach (WorkspaceRole::cases() as $role) { + $user = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => $role->value, + ]); + + expect(Gate::forUser($user)->allows(Capabilities::WORKSPACE_MEMBERSHIP_VIEW, $workspace))->toBe($role !== WorkspaceRole::Readonly); + expect(Gate::forUser($user)->allows(Capabilities::WORKSPACE_MEMBERSHIP_MANAGE, $workspace))->toBe($role === WorkspaceRole::Owner); + } +}); + +it('denies non-owner workspace membership service mutations', function (string $role): void { + $workspace = Workspace::factory()->create(); + $actor = User::factory()->create(); + $member = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $actor->getKey(), + 'role' => $role, + ]); + + app(WorkspaceMembershipManager::class)->addMember( + workspace: $workspace, + actor: $actor, + member: $member, + role: WorkspaceRole::Readonly->value, + ); +})->throws(\DomainException::class, 'Forbidden.')->with([ + 'manager' => [WorkspaceRole::Manager->value], + 'operator' => [WorkspaceRole::Operator->value], + 'readonly' => [WorkspaceRole::Readonly->value], +]); + +it('allows owner workspace membership service mutations', function (): void { + $workspace = Workspace::factory()->create(); + $owner = User::factory()->create(); + $member = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $owner->getKey(), + 'role' => WorkspaceRole::Owner->value, + ]); + + $membership = app(WorkspaceMembershipManager::class)->addMember( + workspace: $workspace, + actor: $owner, + member: $member, + role: WorkspaceRole::Readonly->value, + ); + + expect($membership->workspace_id)->toBe((int) $workspace->getKey()) + ->and($membership->user_id)->toBe((int) $member->getKey()) + ->and($membership->role)->toBe(WorkspaceRole::Readonly->value); +}); + +it('denies manager managed-environment access-scope management', function (): void { + $workspace = Workspace::factory()->create(); + $tenant = ManagedEnvironment::factory()->active()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + $manager = User::factory()->create(); + $member = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $manager->getKey(), + 'role' => WorkspaceRole::Manager->value, + ]); + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $member->getKey(), + 'role' => WorkspaceRole::Readonly->value, + ]); + + app(ManagedEnvironmentMembershipManager::class)->grantScope( + tenant: $tenant, + actor: $manager, + member: $member, + ); +})->throws(\DomainException::class, 'Forbidden.'); diff --git a/apps/platform/tests/Unit/Auth/CapabilityResolverTest.php b/apps/platform/tests/Unit/Auth/CapabilityResolverTest.php index 0cbc9dcb..22fb33e0 100644 --- a/apps/platform/tests/Unit/Auth/CapabilityResolverTest.php +++ b/apps/platform/tests/Unit/Auth/CapabilityResolverTest.php @@ -30,7 +30,7 @@ expect($resolver->can($manager, $tenant, Capabilities::PROVIDER_MANAGE))->toBeTrue(); expect($resolver->can($manager, $tenant, Capabilities::TENANT_BACKUP_SCHEDULES_MANAGE))->toBeTrue(); expect($resolver->can($manager, $tenant, Capabilities::TENANT_BACKUP_SCHEDULES_RUN))->toBeTrue(); - expect($resolver->can($manager, $tenant, Capabilities::TENANT_MEMBERSHIP_MANAGE))->toBeTrue(); + expect($resolver->can($manager, $tenant, Capabilities::TENANT_MEMBERSHIP_MANAGE))->toBeFalse(); expect($resolver->can($manager, $tenant, Capabilities::TENANT_ROLE_MAPPING_MANAGE))->toBeFalse(); expect($resolver->isMember($operator, $tenant))->toBeTrue(); diff --git a/apps/platform/tests/Unit/Auth/WorkspaceRoleCapabilityMapTest.php b/apps/platform/tests/Unit/Auth/WorkspaceRoleCapabilityMapTest.php new file mode 100644 index 00000000..286b31de --- /dev/null +++ b/apps/platform/tests/Unit/Auth/WorkspaceRoleCapabilityMapTest.php @@ -0,0 +1,35 @@ +toBe(array_values(array_unique($capabilities))); + + foreach ($capabilities as $capability) { + expect(Capabilities::isKnown($capability))->toBeTrue(); + } + } +}); + +it('keeps workspace and tenant membership management owner only', function (): void { + expect(WorkspaceRoleCapabilityMap::rolesWithCapability(Capabilities::WORKSPACE_MEMBERSHIP_MANAGE)) + ->toBe([WorkspaceRole::Owner->value]); + + expect(WorkspaceRoleCapabilityMap::hasCapability(WorkspaceRole::Owner, Capabilities::WORKSPACE_MEMBERSHIP_MANAGE))->toBeTrue(); + expect(WorkspaceRoleCapabilityMap::hasCapability(WorkspaceRole::Owner, Capabilities::TENANT_MEMBERSHIP_MANAGE))->toBeTrue(); + + foreach ([WorkspaceRole::Manager, WorkspaceRole::Operator, WorkspaceRole::Readonly] as $role) { + expect(WorkspaceRoleCapabilityMap::hasCapability($role, Capabilities::WORKSPACE_MEMBERSHIP_MANAGE))->toBeFalse(); + expect(WorkspaceRoleCapabilityMap::hasCapability($role, Capabilities::TENANT_MEMBERSHIP_MANAGE))->toBeFalse(); + } + + expect(RoleCapabilityMap::hasCapability(TenantRole::Manager, Capabilities::TENANT_MEMBERSHIP_MANAGE))->toBeFalse(); +}); diff --git a/specs/309-rbac-role-matrix-access-boundary-audit/checklists/requirements.md b/specs/309-rbac-role-matrix-access-boundary-audit/checklists/requirements.md new file mode 100644 index 00000000..23b57d47 --- /dev/null +++ b/specs/309-rbac-role-matrix-access-boundary-audit/checklists/requirements.md @@ -0,0 +1,48 @@ +# Specification Quality Checklist: RBAC Role Matrix & Access Boundary Audit + +**Purpose**: Validate specification completeness and quality before implementation planning/implementation. +**Created**: 2026-05-15 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No application implementation was performed during preparation. +- [x] Focus is on security, trust, auditability, and boundary correctness. +- [x] The spec is repo-based and names the current evidence anchors. +- [x] All mandatory repo-specific sections are completed or explicitly marked N/A. +- [x] The candidate check required by SPEC-GATE-001 is completed. +- [x] Candidate selection rationale and completed-spec guardrail result are recorded. + +## Requirement Completeness + +- [x] No `[NEEDS CLARIFICATION]` markers remain. +- [x] Functional requirements are testable and boundary-oriented. +- [x] Acceptance criteria cover role inventory, owner-only contradictions, panel boundaries, workspace isolation, environment isolation, sensitive actions, and no RBAC redesign. +- [x] Edge cases are identified. +- [x] Scope is clearly bounded to audit-first minimal hardening. +- [x] Dependencies and assumptions are identified. + +## Constitution Alignment + +- [x] Workspace isolation and managed-environment isolation are explicit. +- [x] RBAC-UX server-side source-of-truth rules are explicit. +- [x] 404 vs 403 semantics are explicit. +- [x] Capability registry usage is explicit. +- [x] Test governance and lane classification are explicit. +- [x] Proportionality review confirms no new persisted truth, role model, table, enum/status family, or broad framework is planned. + +## Feature Readiness + +- [x] `spec.md` exists. +- [x] `plan.md` exists. +- [x] `tasks.md` exists. +- [x] Tasks are ordered by read-only inventory, classification, tests first, minimal fixes, validation, and close-out. +- [x] Tasks include focused tests and validation commands. +- [x] Follow-up candidates are listed instead of hidden in scope. +- [x] Related completed specs are treated as context only and are not modified. + +## Notes + +- Preparation found a repo-real path correction: `WorkspaceRoleCapabilityMap.php` is under `apps/platform/app/Services/Auth/`, not `apps/platform/app/Support/Auth/`. +- Preparation found a high-risk static contradiction to verify during implementation: Manager currently receives `WORKSPACE_MEMBERSHIP_MANAGE` and `TENANT_MEMBERSHIP_MANAGE`, while the Constitution says Manager must not manage tenant memberships. +- Preparation did not modify application code, tests, migrations, resources, routes, policies, models, services, jobs, views, or assets. diff --git a/specs/309-rbac-role-matrix-access-boundary-audit/plan.md b/specs/309-rbac-role-matrix-access-boundary-audit/plan.md new file mode 100644 index 00000000..1c92e58d --- /dev/null +++ b/specs/309-rbac-role-matrix-access-boundary-audit/plan.md @@ -0,0 +1,348 @@ +# Implementation Plan: RBAC Role Matrix & Access Boundary Audit + +**Branch**: `309-rbac-role-matrix-access-boundary-audit` | **Date**: 2026-05-15 | **Spec**: [spec.md](spec.md) +**Input**: Feature specification from `/specs/309-rbac-role-matrix-access-boundary-audit/spec.md` + +## Summary + +Audit and harden TenantPilot's RBAC and panel-access boundaries across workspace, managed-environment, provider connection, review, review pack, evidence, finding exception, OperationRun, Admin panel, and System panel surfaces. + +The implementation is audit-first: + +1. Inventory repo-real role/capability grants and direct access boundaries. +2. Classify Constitution/runtime contradictions. +3. Write focused tests for confirmed contradictions and boundary bugs. +4. Apply only minimal runtime fixes. +5. Re-run focused regression lanes and document remaining product decisions. + +## Technical Context + +**Language/Version**: PHP 8.4.15, Laravel 12.52.0 +**Primary Dependencies**: Filament 5.2.1, Livewire 4.1.4, Pest 4.3.1, Laravel Sail +**Storage**: PostgreSQL; no schema change expected +**Testing**: Pest 4 Unit and Feature tests; Browser only if Feature/Filament tests cannot prove a panel/action boundary +**Validation Lanes**: confidence; optional browser; formatting/diff-check +**Target Platform**: Laravel Sail locally; Dokploy container deployment for staging/production +**Project Type**: Laravel/Filament app under `apps/platform` +**Performance Goals**: No broad query redesign; authorization checks should reuse existing resolvers and avoid new global scans +**Constraints**: no new RBAC architecture, no migrations by default, no new roles, no new assets, no UI redesign, no broad route cleanup +**Scale/Scope**: focused security-boundary audit and minimal fixes over existing role maps/policies/panels/tests + +## UI / Surface Guardrail Plan + +- **Guardrail scope**: no new operator-facing surface; direct access/action authorization and possible minor visibility alignment only. +- **Native vs custom classification summary**: existing Filament v5 panels/resources/pages/actions. +- **Shared-family relevance**: panel access, workspace/environment context, resource authorization, direct action execution, global search safety. +- **State layers in scope**: panel guard, middleware, route model binding, policies, Filament page/action access, Livewire action execution. +- **Audience modes in scope**: workspace users, customer-safe readers, platform/system users. +- **Decision/diagnostic/raw hierarchy plan**: no presentation hierarchy change. +- **Raw/support gating plan**: verify raw/provider/support details remain gated by existing policies. +- **One-primary-action / duplicate-truth control**: no new actions. +- **Handling modes by drift class or surface**: security blocker and high-risk contradiction fix in 309; product-decision-needed stays deferred. +- **Repository-signal treatment**: review-mandatory for role-map, panel, policy, and sensitive action changes. +- **Special surface test profiles**: standard-native-filament plus direct Feature/Livewire action tests. +- **Required tests or manual smoke**: focused Unit/Feature tests; browser not required unless direct behavior cannot be proven otherwise. +- **Exception path and spread control**: no exceptions planned. +- **Active feature PR close-out entry**: RBAC Inventory / Boundary Proof / Remaining Decisions. + +## Shared Pattern & System Fit + +- **Cross-cutting feature marker**: yes. +- **Systems touched**: role maps, capability resolvers, policies, gates, panels, workspace context, Filament resources/pages/actions, existing RBAC tests. +- **Shared abstractions reused**: `Capabilities`, `PlatformCapabilities`, `WorkspaceRoleCapabilityMap`, `RoleCapabilityMap`, `WorkspaceCapabilityResolver`, `CapabilityResolver`, `ManagedEnvironmentAccessScopeResolver`, model policies, Gate definitions, Filament access methods. +- **New abstraction introduced? why?**: none planned. +- **Why the existing abstraction was sufficient or insufficient**: the repo already has canonical maps and resolvers; the issue is suspected incorrect grants or missing enforcement, not absent infrastructure. +- **Bounded deviation / spread control**: any fix must remain local to existing maps/policies/panel checks and must not add a public permission framework. + +## OperationRun UX Impact + +- **Touches OperationRun start/completion/link UX?**: authorization only. +- **Central contract reused**: `OperationRunPolicy`, `OperationRunCapabilityResolver`, `OperationRunLinks`. +- **Delegated UX behaviors**: all existing start/completion/link feedback remains unchanged. +- **Surface-owned behavior kept local**: no new operation UI or notifications. +- **Queued DB-notification policy**: N/A. +- **Terminal notification path**: existing central lifecycle. +- **Exception path**: none. + +## Provider Boundary & Portability Fit + +- **Shared provider/platform boundary touched?**: provider credential authorization is audited. +- **Provider-owned seams**: credential rotation/delete/dedicated credential management. +- **Platform-core seams**: ProviderConnection resource/policy and workspace/environment access boundaries. +- **Neutral platform terms / contracts preserved**: provider connection, provider credential, workspace, managed environment, operation. +- **Retained provider-specific semantics and why**: existing Microsoft/Intune details stay in provider-owned logic. +- **Bounded extraction or follow-up path**: no provider registry redesign; provider-specific follow-up only if audit finds structural drift. + +## Constitution Check + +- Inventory-first: PASS. No inventory or Graph sync behavior changes are planned. +- Read/write separation: PASS. Mutations remain behind existing action flows; this spec audits and fixes authorization. +- Graph contract path: PASS. No Graph endpoint or contract change. +- Deterministic capabilities: NEEDS IMPLEMENTATION PROOF. Role/capability grants must become deterministic and tested. +- RBAC-UX: NEEDS IMPLEMENTATION PROOF. Constitution says Manager must not manage tenant memberships; current maps appear to grant Manager membership-management authority. +- Workspace isolation: NEEDS IMPLEMENTATION PROOF. Cross-workspace direct URLs must be tested for representative sensitive resources. +- Tenant/managed-environment isolation: NEEDS IMPLEMENTATION PROOF. Same-workspace wrong-environment direct URLs must be tested. +- Cross-plane access: PARTIAL PASS. Existing `/system` tests prove several tenant-session denials; Spec 309 should keep/extend focused proof. +- Run observability: PASS. No new run behavior; OperationRun access must remain policy-gated. +- TEST-GOV-001: PASS if tests stay focused and no broad helper defaults are introduced. +- PROP-001 / BLOAT-001: PASS. No new tables, roles, capability framework, enum, or UI taxonomy planned. +- PERSIST-001: PASS. No new persisted truth. +- STATE-001: PASS. No new lifecycle/status family. +- UI-SEM-001: PASS. No presentation framework. +- XCUT-001: PASS if implementation reuses existing auth/policy/gate paths. +- PROV-001: PASS. Provider credential authorization is audited without expanding provider semantics. +- UI-FIL-001: PASS if any visibility alignment stays Filament-native and avoids assets/ad-hoc styling. + +## Initial Read-Only Repo Findings + +### RBAC Inventory + +| Role | Current inspected sensitive capabilities | Initial match assessment | Action | +|---|---|---|---| +| Workspace Owner | `workspace_membership.manage`, `tenant_membership.manage`, provider manage/dedicated, review/evidence manage, finding exception approve | Mostly expected | Keep unless tests reveal bypass | +| Workspace Manager | `workspace_membership.manage`, appended `tenant_membership.manage`, provider manage, review/evidence manage, finding exception approve | High-risk contradiction for membership; other manage grants need classification | Add tests, fix membership if Owner-only confirmed | +| Workspace Operator | membership view, provider view/run, review/evidence view, findings triage, audit view | Mostly expected | Prove no owner-only mutation | +| Workspace Readonly | workspace/settings/alerts/baselines/audit view plus tenant/review/evidence/provider view via merged tenant role | Mostly expected but verify customer-safe download/mutation boundaries | Prove read-only cannot mutate | +| Platform/System | `PlatformUser` + `PlatformCapabilities`, separate `platform` guard | Expected separate plane | Prove no ordinary workspace access to `/system` and no implicit customer data access | + +### Constitution vs Runtime + +- `apps/platform/app/Services/Auth/WorkspaceRoleCapabilityMap.php` currently grants Manager `Capabilities::WORKSPACE_MEMBERSHIP_MANAGE`. +- `WorkspaceRoleCapabilityMap::getCapabilities()` appends `Capabilities::TENANT_MEMBERSHIP_MANAGE` for Manager. +- `apps/platform/app/Services/Auth/RoleCapabilityMap.php` does not grant tenant Manager `TENANT_MEMBERSHIP_MANAGE`, which makes the WorkspaceRoleCapabilityMap append the direct source of the tenant-membership Manager grant. +- `.specify/memory/constitution.md` states: "Manager ... MUST NOT manage tenant memberships (Owner-only)." +- Existing tests currently assert Manager can manage tenant membership: + - `apps/platform/tests/Unit/Auth/CapabilityResolverTest.php` + - `apps/platform/tests/Feature/Rbac/RoleMatrix/ManagerAccessTest.php` + +### Panel Boundary Findings + +- `apps/platform/app/Models/User.php::canAccessPanel()` currently returns `true`. +- Admin panel uses `web` guard plus `ensure-correct-guard:web`, `ensure-workspace-selected`, and `ensure-filament-tenant-selected`. +- System panel uses `authGuard('platform')`, `ensure-correct-guard:platform`, and `ensure-platform-capability:platform.access_system_panel`. +- Existing tests already cover several system-panel cross-plane denials: + - `apps/platform/tests/Feature/Auth/CrossScopeAccessTest.php` + - `apps/platform/tests/Feature/System/Spec113/AuthorizationSemanticsTest.php` + - `apps/platform/tests/Feature/System/Spec114/SystemConsoleAccessSemanticsTest.php` +- Spec 309 still needs a dedicated boundary test family tying those semantics to the RBAC matrix and current `canAccessPanel()` posture. + +## Proposed Minimal Fix Strategy + +1. Start with static tests for `WorkspaceRoleCapabilityMap` and `RoleCapabilityMap`. +2. If Constitution Owner-only semantics are confirmed, remove Manager's `WORKSPACE_MEMBERSHIP_MANAGE` grant and the Manager-specific appended `TENANT_MEMBERSHIP_MANAGE`. +3. Update existing tests that assert Manager membership management so they become denial tests. +4. Verify whether `User::canAccessPanel()` can be tightened without breaking admin login/workspace chooser flow. If not, document the middleware/policy proof and add direct route tests. +5. Add or adjust policy/action tests for confirmed direct-access bypasses only. +6. Align Filament visibility only after server-side policy behavior is correct. +7. Do not change provider/review/evidence/finding-operation grants unless the audit classifies them as confirmed security bugs rather than product decisions. + +## Project Structure + +### Documentation (this feature) + +```text +specs/309-rbac-role-matrix-access-boundary-audit/ +|-- spec.md +|-- plan.md +|-- tasks.md +`-- checklists/ + `-- requirements.md +``` + +### Likely Source Code Surfaces For Later Implementation + +```text +apps/platform/app/Support/Auth/ +|-- Capabilities.php +|-- PlatformCapabilities.php +`-- WorkspaceRole.php + +apps/platform/app/Support/ +`-- TenantRole.php + +apps/platform/app/Services/Auth/ +|-- WorkspaceRoleCapabilityMap.php +`-- RoleCapabilityMap.php + +apps/platform/app/Models/ +|-- User.php +`-- PlatformUser.php + +apps/platform/app/Providers/ +|-- AuthServiceProvider.php +`-- Filament/ + |-- AdminPanelProvider.php + `-- SystemPanelProvider.php + +apps/platform/app/Policies/ +|-- OperationRunPolicy.php +|-- ProviderConnectionPolicy.php +|-- EnvironmentReviewPolicy.php +|-- ReviewPackPolicy.php +|-- EvidenceSnapshotPolicy.php +`-- FindingExceptionPolicy.php + +apps/platform/tests/Unit/Auth/ +apps/platform/tests/Feature/Rbac/ +apps/platform/tests/Feature/Auth/ +apps/platform/tests/Feature/System/ +apps/platform/tests/Feature/ProviderConnections/ +apps/platform/tests/Feature/ReviewPack/ +apps/platform/tests/Feature/Reviews/ +apps/platform/tests/Feature/Findings/ +apps/platform/tests/Feature/Operations/ +``` + +**Structure Decision**: Use existing Laravel/Filament/Pest test families where equivalent coverage exists. Create new `tests/Feature/Rbac/*BoundaryTest.php` files only when no repo-real focused equivalent exists. + +## Technical Approach + +1. Inventory capabilities from `Capabilities::all()` and `PlatformCapabilities::all()`. +2. Inventory role grants from `WorkspaceRoleCapabilityMap::getCapabilities()` and `RoleCapabilityMap::getCapabilities()`. +3. Categorize capabilities into workspace administration, managed-environment administration, provider connections, findings/exceptions, reviews, review packs, evidence, OperationRuns, reports, support requests, and system panel. +4. Compare membership, credential, lifecycle, review/export, and operation actions against Constitution RBAC-UX statements. +5. Add focused tests for confirmed mismatches and direct route/action boundaries. +6. Apply only the smallest map/policy/panel/resource changes needed for confirmed bugs. +7. Run focused regressions for affected review, review-pack, provider, finding, and operation lanes. +8. Close out with role inventory, fixed contradictions, deferred product decisions, and validation results. + +## Data / Model Implications + +- No migration expected. +- No new model expected. +- No new capability persistence expected. +- Static role-to-capability maps may change. +- Existing `workspace_memberships` and `managed_environment_memberships` data is not migrated by default. + +## Policy / Authorization Implications + +- `WorkspaceMembershipPolicy` and `WorkspaceRoleCapabilityMap` are the primary owner-only membership-management proof targets. +- Provider credential actions should continue to rely on `ProviderConnectionPolicy`. +- Review and Review Pack actions should continue to rely on `EnvironmentReviewPolicy` and `ReviewPackPolicy`. +- FindingException lifecycle actions should continue to rely on `FindingExceptionPolicy`. +- OperationRun visibility/action access should continue to rely on `OperationRunPolicy` and `OperationRunCapabilityResolver`. +- Service-level mutations must call Gate/Policy where UI actions execute state changes. + +## Filament v5 / Livewire v4 Compliance Notes + +- Filament is v5.2.1 and Livewire is v4.1.4. +- Panel providers are registered in `apps/platform/bootstrap/providers.php`, not `bootstrap/app.php`. +- This spec does not add a new panel provider. +- Any changed globally searchable resource must either keep an Edit/View page or disable global search. +- Destructive actions must execute via `Action::make(...)->action(...)`, include `->requiresConfirmation()`, and still authorize server-side. +- No new assets are planned. Existing panel theme assets remain resolved through `PanelThemeAsset`; deployment should keep the existing `cd apps/platform && php artisan filament:assets` step where registered Filament assets are published. + +## Test Strategy + +### New Or Updated Focused Tests + +- `apps/platform/tests/Unit/Auth/WorkspaceRoleCapabilityMapTest.php` +- `apps/platform/tests/Feature/Rbac/WorkspaceRoleCapabilityBoundaryTest.php` +- `apps/platform/tests/Feature/Rbac/AdminPanelAccessBoundaryTest.php` +- `apps/platform/tests/Feature/Rbac/SystemPanelAccessBoundaryTest.php` +- `apps/platform/tests/Feature/Rbac/ManagedEnvironmentAccessBoundaryTest.php` +- `apps/platform/tests/Feature/Rbac/ProviderConnectionAccessBoundaryTest.php` +- `apps/platform/tests/Feature/Rbac/ReviewPackAccessBoundaryTest.php` +- `apps/platform/tests/Feature/Rbac/OperationRunAccessBoundaryTest.php` +- `apps/platform/tests/Feature/Rbac/FindingExceptionLifecycleAccessBoundaryTest.php` + +Use existing tests instead when they already provide the exact proof and update names only if a new focused file improves reviewability. + +### Regression Lanes + +- Review/customer workspace: + - `apps/platform/tests/Feature/Reviews/CustomerReviewWorkspaceAuthorizationTest.php` + - `apps/platform/tests/Feature/Reviews/CustomerReviewWorkspacePackAccessTest.php` +- ReviewPack: + - `apps/platform/tests/Feature/ReviewPack/ReviewPackRbacTest.php` + - `apps/platform/tests/Feature/ReviewPack/ReviewPackDownloadTest.php` +- ProviderConnection: + - existing `apps/platform/tests/Feature/ProviderConnections/*Authorization*` and credential security tests if provider boundaries change. +- OperationRun: + - `apps/platform/tests/Feature/Rbac/OperationRunWorkspaceFirstAuthorizationTest.php` + - `apps/platform/tests/Feature/Operations/TenantlessOperationRunViewerTest.php` + - existing operation link/legacy route guard tests if policy or links change. + +### Validation Commands + +```bash +cd apps/platform && ./vendor/bin/sail artisan test --compact \ + tests/Unit/Auth/WorkspaceRoleCapabilityMapTest.php \ + tests/Feature/Rbac/WorkspaceRoleCapabilityBoundaryTest.php \ + tests/Feature/Rbac/AdminPanelAccessBoundaryTest.php \ + tests/Feature/Rbac/SystemPanelAccessBoundaryTest.php \ + tests/Feature/Rbac/ManagedEnvironmentAccessBoundaryTest.php \ + tests/Feature/Rbac/ProviderConnectionAccessBoundaryTest.php \ + tests/Feature/Rbac/ReviewPackAccessBoundaryTest.php \ + tests/Feature/Rbac/OperationRunAccessBoundaryTest.php \ + tests/Feature/Rbac/FindingExceptionLifecycleAccessBoundaryTest.php +``` + +```bash +cd apps/platform && ./vendor/bin/sail artisan test --compact \ + tests/Feature/Reviews/CustomerReviewWorkspaceAuthorizationTest.php \ + tests/Feature/Reviews/CustomerReviewWorkspacePackAccessTest.php \ + tests/Feature/ReviewPack/ReviewPackRbacTest.php \ + tests/Feature/ReviewPack/ReviewPackDownloadTest.php +``` + +If OperationRun policy or links are touched: + +```bash +cd apps/platform && ./vendor/bin/sail artisan test --compact \ + tests/Feature/Rbac/OperationRunWorkspaceFirstAuthorizationTest.php \ + tests/Feature/Operations/TenantlessOperationRunViewerTest.php \ + tests/Feature/Guards/OperationRunLinkContractGuardTest.php +``` + +Final: + +```bash +cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent +git diff --check +``` + +## Implementation Phases + +1. **Read-only inventory**: confirm role maps, panel guards, policies, routes, resources, and existing tests. +2. **Classification**: distinguish confirmed security bugs, product decisions, docs drift, UI-only mismatches, and out-of-scope findings. +3. **Tests first**: add failing tests for confirmed contradictions. +4. **Minimal fixes**: adjust maps/policies/panel/resource access only where confirmed. +5. **Regression validation**: run focused affected lanes. +6. **Close-out**: record inventory, fixed contradictions, deferred decisions, tests, and remaining risks. + +## Risk Controls + +- Do not remove Manager capabilities unrelated to membership unless classified as confirmed bugs. +- Do not tighten `canAccessPanel()` until direct route/middleware behavior and workspace chooser flow are understood. +- Do not implement support impersonation, break-glass redesign, or system-to-workspace implicit access. +- Do not add new capability aliases. +- Do not create migrations unless the existing model cannot represent the confirmed fix. +- Do not broaden test helpers or factories globally. + +## Rollout / Deployment Considerations + +- No migration expected. +- No env vars expected. +- No queue/cron changes expected. +- No storage volume changes expected. +- No frontend build expected. +- Any changed authorization behavior must be validated in Staging before Production promotion. +- Existing Dokploy deployment should keep current asset publication and cache-clearing steps; no new asset step is introduced by this spec. + +## Complexity Tracking + +| Violation | Why Needed | Simpler Alternative Rejected Because | +|---|---|---| +| None planned | N/A | N/A | + +## Open Product Decisions To Preserve If Unclear + +- Manager membership-management authority. +- Manager managed-environment access-scope authority. +- Manager provider credential rotation/delete authority. +- Manager accepted-risk approval authority. +- Readonly Review Pack download authority. +- System/platform support access to customer workspaces. + +Unclear items must be recorded as product-decision-needed rather than fixed blindly. diff --git a/specs/309-rbac-role-matrix-access-boundary-audit/spec.md b/specs/309-rbac-role-matrix-access-boundary-audit/spec.md new file mode 100644 index 00000000..2bf7bc9e --- /dev/null +++ b/specs/309-rbac-role-matrix-access-boundary-audit/spec.md @@ -0,0 +1,381 @@ +# Feature Specification: RBAC Role Matrix & Access Boundary Audit + +**Feature Branch**: `309-rbac-role-matrix-access-boundary-audit` +**Created**: 2026-05-15 +**Status**: Ready for implementation +**Input**: User-provided Spec 309 draft: "RBAC Role Matrix & Access Boundary Audit" + +## Spec Candidate Check *(mandatory - SPEC-GATE-001)* + +- **Problem**: TenantPilot is becoming a governance-of-record platform, but repo analysis shows a possible security-boundary contradiction: the Constitution says Manager must not manage tenant memberships, while the current role capability map appears to grant Manager-level membership-management authority. +- **Today's failure**: The product may present deterministic workspace and managed-environment isolation while runtime capability maps, panel access, policies, and direct routes disagree. That can create privilege escalation, workspace/environment leakage, provider credential exposure, or customer-visible review artifact access outside intended scope. +- **User-visible improvement**: Operators and reviewers can trust that workspace isolation, managed-environment isolation, capability checks, and panel boundaries agree server-side. Sensitive membership, provider, review, review-pack, accepted-risk, and operation surfaces are proven by direct tests rather than navigation visibility. +- **Smallest enterprise-capable version**: Run an audit-first repo verification over the existing capability registry, role maps, panel providers, workspace context, policies, resources, and tests; add focused failing tests for confirmed contradictions; apply only minimal corrections to existing role maps, policies, panel access, or Filament visibility after server-side proof. +- **Explicit non-goals**: No new RBAC model, no new role family, no permission UI, no identity federation, no SCIM, no support impersonation redesign, no billing lifecycle work, no navigation cleanup, no route-family redesign, no migrations by default, no customer portal work, and no broad policy framework. +- **Permanent complexity imported**: Focused Unit/Feature tests and possibly small edits to existing capability maps, policies, panel access, or visibility helpers. No new table, persisted entity, enum/status family, capability registry architecture, frontend asset, or public abstraction. +- **Why now**: The roadmap prioritizes customer-safe review/productization, but RBAC ambiguity is a foundation risk. Spec 309 reduces security and trust risk before additional customer-facing governance surfaces are built on top. +- **Why not local**: The suspected contradiction spans `WorkspaceRoleCapabilityMap`, Constitution role semantics, `User::canAccessPanel()`, panel middleware, policies, and existing tests. A local patch to one resource would not prove the boundary. +- **Approval class**: Core Enterprise +- **Red flags triggered**: Cross-surface authorization audit and broad test matrix. Defense: this is a security-boundary verification pass, not a new framework; all work reuses existing capability/policy/gate infrastructure and fixes only confirmed contradictions. +- **Score**: Nutzen: 2 | Dringlichkeit: 2 | Scope: 2 | Komplexitaet: 1 | Produktnaehe: 2 | Wiederverwendung: 2 | **Gesamt: 11/12** +- **Decision**: approve + +## Spec Scope Fields *(mandatory)* + +- **Scope**: workspace +- **Primary Routes**: + - `/admin` + - `/admin/login` + - workspace and managed-environment admin surfaces under the existing Admin panel + - existing environment-scoped resource routes for Provider Connections, Environment Reviews, Review Packs, Evidence, Finding Exceptions, and Operation Runs + - `/system` + - representative `/system/*` pages and operation-detail routes +- **Data Ownership**: + - No new persisted data. + - `workspace_memberships` remains the workspace role source. + - `managed_environment_memberships` remains an access-scope/membership surface as currently implemented; this spec audits whether any remaining role-bearing or owner-only authority contradicts the Constitution. + - Provider Connections, Environment Reviews, Review Packs, Evidence, Finding Exceptions, Stored Reports, and tenant-bound OperationRuns must remain workspace + managed-environment scoped. + - Workspace-bound OperationRuns may remain workspace-only, but tenant-bound runs must still enforce environment entitlement before capability checks. +- **RBAC**: + - Workspace membership is the first isolation boundary. + - Managed-environment entitlement is the second isolation boundary. + - Capability/policy authorization is third. + - UI visibility is last and never the security boundary. + - Non-member or out-of-scope access must deny as not found (`404`) where repo policy semantics already use boundary hiding. + - In-scope members missing a capability must receive `403` where policies define capability denial. + +This is not a canonical-view spec. No new default filter behavior is introduced. + +## Cross-Cutting / Shared Pattern Reuse *(mandatory)* + +- **Cross-cutting feature?**: yes. +- **Interaction class(es)**: RBAC capability resolution, panel access, route authorization, Filament resource/page access, action authorization, global search safety, operation drilldown links, provider credential actions, review/export/download access, and accepted-risk lifecycle actions. +- **Systems touched**: + - `apps/platform/app/Support/Auth/Capabilities.php` + - `apps/platform/app/Services/Auth/WorkspaceRoleCapabilityMap.php` + - `apps/platform/app/Services/Auth/RoleCapabilityMap.php` + - `apps/platform/app/Providers/AuthServiceProvider.php` + - `apps/platform/app/Models/User.php` + - `apps/platform/app/Providers/Filament/AdminPanelProvider.php` + - `apps/platform/app/Providers/Filament/SystemPanelProvider.php` + - `apps/platform/app/Support/Workspaces/WorkspaceContext.php` + - `apps/platform/app/Filament/Concerns/WorkspaceScopedTenantRoutes.php` + - relevant policies/resources/pages for ProviderConnection, EnvironmentReview, ReviewPack, EvidenceSnapshot, StoredReport, FindingException, and OperationRun. +- **Existing pattern(s) to extend**: capability registry constants, `WorkspaceCapabilityResolver`, `CapabilityResolver`, `ManagedEnvironmentAccessScopeResolver`, model policies, Gate definitions, Filament `canAccess()` / `canViewAny()` / policy checks, existing cross-plane middleware, and existing 404/403 semantics. +- **Shared contract / presenter / builder / renderer to reuse**: Existing capability/policy/gate infrastructure only. Do not introduce a new RBAC abstraction. +- **Why the existing shared path is sufficient or insufficient**: The repo already has a central capability registry and role maps. The issue is whether current mappings, panels, policies, and tests agree with the Constitution, not absence of infrastructure. +- **Allowed deviation and why**: None planned. If implementation discovers that a minimal helper is required to remove duplicated direct checks, it must be private, narrow, and justified in close-out. +- **Consistency impact**: Role maps, policies, direct URLs, Filament action execution, global search, operation links, and panel guards must all derive the same outcome for the same actor/workspace/environment/capability tuple. +- **Review focus**: Block UI-only security, raw role-string checks, direct route bypasses, cross-plane access, Manager/Operator/Readonly owner-only grants, and speculative RBAC redesign. + +## OperationRun UX Impact *(mandatory)* + +- **Touches OperationRun start/completion/link UX?**: yes, authorization only. +- **Shared OperationRun UX contract/layer reused**: `OperationRunPolicy`, `OperationRunCapabilityResolver`, `OperationRunLinks`, and existing Monitoring/Operations route helpers. +- **Delegated start/completion UX behaviors**: Existing operation start/link/display UX remains unchanged; this spec verifies that run visibility and action capability checks remain scope-safe. +- **Local surface-owned behavior that remains**: No new operation UI. Any touched action keeps its existing initiation input and feedback path. +- **Queued DB-notification policy**: N/A. +- **Terminal notification path**: Existing central lifecycle mechanism. +- **Exception required?**: none. + +## Provider Boundary / Platform Core Check *(mandatory)* + +- **Shared provider/platform boundary touched?**: yes, provider credential access is audited. +- **Boundary classification**: mixed. ProviderConnection records are platform-core integration records bound to a workspace and managed environment; provider credential operations remain high-privilege provider-owned behavior. +- **Seams affected**: ProviderConnection policies/resources/actions, provider management capabilities, dedicated credential actions, verification/start surfaces. +- **Neutral platform terms preserved or introduced**: provider connection, provider credential, managed environment, workspace, operation. +- **Provider-specific semantics retained and why**: Existing Microsoft/Intune specifics remain inside provider connection and Graph-facing code; this spec does not introduce provider-specific platform terminology. +- **Why this does not deepen provider coupling accidentally**: The audit checks access boundaries only and does not add provider contracts, endpoints, or provider taxonomy. +- **Follow-up path**: Support Access Governance remains separate; provider capability registry redesign is out of scope. + +## UI / Surface Guardrail Impact *(mandatory)* + +No new operator-facing surface is introduced. UI changes are allowed only after server-side authorization is correct and only to align visibility/disabled state with existing policy outcomes. + +| Surface / Change | Operator-facing surface change? | Native vs Custom | Shared-Family Relevance | State Layers Touched | Exception Needed? | Low-Impact / `N/A` Note | +|---|---|---|---|---|---|---| +| Admin panel access boundary | no new surface | Existing Filament panel | panel access, workspace context | middleware, panel gate, route | no | direct URL behavior must be tested | +| System panel access boundary | no new surface | Existing Filament system panel | cross-plane access | guard, middleware, panel auth | no | ordinary workspace users must not access `/system` | +| Existing resource/actions visibility | possible minor alignment only | Native Filament resources/actions | action execution and visibility | action policy, page access | no | visibility follows server-side policy | + +## Decision-First Surface Role *(mandatory when operator-facing surfaces are changed)* + +N/A - this spec does not create or materially redesign an operator-facing surface. Existing surfaces remain decision-owned by their current resources/pages. + +## Audience-Aware Disclosure *(mandatory when operator-facing surfaces are changed)* + +N/A - no customer-facing content or disclosure hierarchy changes. Existing raw/support diagnostics must remain capability-gated or hidden according to current policies. + +## UI/UX Surface Classification *(mandatory when operator-facing surfaces are changed)* + +N/A - no new or redesigned list/detail/workbench surface. Existing Filament v5 action rules still apply to any touched resource/action: destructive actions require `->requiresConfirmation()` and server-side authorization. + +## Operator Surface Contract *(mandatory when operator-facing surfaces are changed)* + +N/A - no new operator surface contract. Direct access/action tests are the proof mechanism for this spec. + +## Proportionality Review *(mandatory when structural complexity is introduced)* + +- **New source of truth?**: no. +- **New persisted entity/table/artifact?**: no. +- **New abstraction?**: no planned abstraction. +- **New enum/state/reason family?**: no. +- **New cross-domain UI framework/taxonomy?**: no. +- **Current operator problem**: possible mismatch between role/capability truth, Constitution semantics, panel access, and direct route/action authorization. +- **Existing structure is insufficient because**: it may contain incorrect grants or missing policy checks; the structure itself is sufficient if corrected and tested. +- **Narrowest correct implementation**: inventory, classify, write focused tests, and adjust only existing maps/policies/panel checks where repo truth confirms a bug. +- **Ownership cost**: focused RBAC boundary tests and close-out inventory. +- **Alternative intentionally rejected**: new permission framework or role model. That would import complexity without being required for this security audit. +- **Release truth**: current-release trust and security boundary hardening. + +### Compatibility posture + +The product is pre-production. Compatibility shims, legacy capability aliases, dual-read logic, and data migrations are out of scope unless a repo-verified security blocker cannot be represented by the current role/capability model. + +## Testing / Lane / Runtime Impact *(mandatory for runtime behavior changes)* + +- **Test purpose / classification**: Unit and Feature. Browser is not required unless direct panel/action behavior cannot be proven by Feature/Filament tests. +- **Validation lane(s)**: confidence for RBAC/panel/policy tests; optional browser only for unprovable rendered panel interaction. +- **Why this classification and these lanes are sufficient**: Static role-map tests prove deterministic grants. Feature tests prove direct URLs, policies, action execution, and cross-scope denial. Browser smoke is unnecessary when Feature/Livewire tests prove direct route/action boundaries. +- **New or expanded test families**: `apps/platform/tests/Unit/Auth/WorkspaceRoleCapabilityMapTest.php` and focused `apps/platform/tests/Feature/Rbac/*BoundaryTest.php` files or repo-real equivalents. +- **Fixture / helper cost impact**: moderate and feature-local. Tests need users, workspace memberships, managed environments, environment-scope rows, ProviderConnections, ReviewPacks, EnvironmentReviews, FindingExceptions, OperationRuns, and platform users. Shared helper defaults must stay cheap. +- **Heavy-family visibility / justification**: none expected. +- **Special surface test profile**: standard-native-filament and access-boundary feature tests. +- **Standard-native relief or required special coverage**: feature tests are preferred over browser smoke for panel boundaries and policies. +- **Reviewer handoff**: Confirm 404 vs 403 semantics, direct route/action proof, no UI-only security, no new RBAC model, no assets, no migrations, and no broadened test defaults. +- **Budget / baseline / trend impact**: low to moderate focused confidence-lane growth; no heavy-governance family. +- **Escalation needed**: none unless implementation discovers a structural RBAC model gap. +- **Active feature PR close-out entry**: RBAC Inventory / Boundary Proof / Remaining Decisions. +- **Planned validation commands**: + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Unit/Auth/WorkspaceRoleCapabilityMapTest.php` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Rbac/WorkspaceRoleCapabilityBoundaryTest.php tests/Feature/Rbac/AdminPanelAccessBoundaryTest.php tests/Feature/Rbac/SystemPanelAccessBoundaryTest.php` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Rbac/ManagedEnvironmentAccessBoundaryTest.php tests/Feature/Rbac/ProviderConnectionAccessBoundaryTest.php tests/Feature/Rbac/ReviewPackAccessBoundaryTest.php tests/Feature/Rbac/OperationRunAccessBoundaryTest.php tests/Feature/Rbac/FindingExceptionLifecycleAccessBoundaryTest.php` + - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Reviews/CustomerReviewWorkspaceAuthorizationTest.php tests/Feature/Reviews/CustomerReviewWorkspacePackAccessTest.php tests/Feature/ReviewPack/ReviewPackRbacTest.php tests/Feature/ReviewPack/ReviewPackDownloadTest.php` + - `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent` + - `git diff --check` + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Role Matrix Truth Is Audited (Priority: P1) + +As a platform maintainer, I need a repo-derived inventory of roles and capabilities so owner-only contradictions can be identified before new customer-facing governance surfaces are built. + +**Why this priority**: This is the trigger for the spec and the prerequisite for deciding whether runtime behavior should change. + +**Independent Test**: A focused role-map test can prove whether Owner, Manager, Operator, Readonly, and Platform/System grants match the intended sensitive-boundary matrix. + +**Acceptance Scenarios**: + +1. **Given** the current capability registry and role maps, **When** the implementation inventories all role grants, **Then** the close-out lists each sensitive grant, match/mismatch classification, and action. +2. **Given** the Constitution says Manager must not manage tenant memberships, **When** the role map grants Manager membership-management capability, **Then** the mismatch is either fixed as a confirmed bug or explicitly recorded as product-decision-needed. + +--- + +### User Story 2 - Panel And Scope Boundaries Are Proven Directly (Priority: P1) + +As a security reviewer, I need direct URL proof for `/admin`, `/system`, workspace isolation, and managed-environment isolation so hidden navigation cannot be mistaken for security. + +**Why this priority**: Panel and scope bypasses would be security blockers. + +**Independent Test**: Feature tests can authenticate ordinary workspace users, platform users, non-members, wrong-workspace members, and wrong-environment members, then assert direct URLs/actions return the repo-standard denial. + +**Acceptance Scenarios**: + +1. **Given** an ordinary workspace user, **When** they request `/system`, **Then** the system returns deny-as-not-found and no system content is visible. +2. **Given** a user without valid workspace authority, **When** they request direct `/admin` workspace surfaces, **Then** access is denied by authentication/workspace middleware or policy, not only hidden navigation. +3. **Given** a member of workspace A, **When** they request workspace B review packs, findings, provider connections, or operation runs, **Then** the response denies as not found. +4. **Given** a member scoped to environment A, **When** they request environment B records in the same workspace, **Then** the response denies as not found before capability checks. + +--- + +### User Story 3 - Sensitive Actions Are Server-Side Gated (Priority: P2) + +As an enterprise operator, I need sensitive membership, provider credential, review/review-pack, accepted-risk, and operation actions to be impossible for unauthorized roles even through direct action execution. + +**Why this priority**: Mutation boundaries are higher risk than read-only view boundaries, and UI visibility is explicitly not a security boundary. + +**Independent Test**: Filament/Livewire Feature tests and policy tests can call representative actions directly and assert non-owner or missing-capability actors are denied. + +**Acceptance Scenarios**: + +1. **Given** a Manager/Operator/Readonly actor, **When** they attempt owner-only membership management, **Then** the server denies execution. +2. **Given** a Manager/Operator actor without high-privilege provider authority, **When** they attempt provider credential rotation/delete/dedicated credential management, **Then** the server denies execution. +3. **Given** a Readonly actor, **When** they attempt finding exception approval/rejection, review publication/archive/export mutation, review-pack mutation, or operation action, **Then** the server denies execution. +4. **Given** an Owner or allowed role, **When** they perform existing allowed flows covered by regression tests, **Then** the flow still works. + +## Edge Cases + +- A user has workspace membership but no current workspace context. +- A user has workspace membership but a stale remembered managed-environment context. +- A user has platform/system authority but no workspace membership. +- A user has both web and platform sessions; cross-plane session separation must still hide the other plane. +- A record has a workspace_id / managed_environment_id mismatch. +- An OperationRun is workspace-bound with no managed_environment_id. +- An OperationRun is tenant-bound but its environment is deleted or outside the current workspace. +- A Filament URL-only action is navigation-only; confirmation behavior must not be assumed unless the action executes via `->action(...)`. +- A resource is globally searchable but lacks an Edit/View page; global search must be disabled or the page must exist. + +## Functional Requirements *(mandatory)* + +- **FR-001 Role Map Inventory**: Implementation MUST produce a repo-derived inventory of all roles and assigned capabilities, including Owner, Manager, Operator, Readonly, Platform/System, and any repo-real special roles. +- **FR-002 Constitution Alignment**: Implementation MUST compare Constitution RBAC statements against runtime mappings and classify mismatches as confirmed bug, intentional product decision, docs/constitution drift, unclear/product-decision-needed, or out of scope. +- **FR-003 Owner-only Enforcement**: Confirmed owner-only capabilities MUST NOT be granted to Manager, Operator, Readonly, or customer-safe actors. +- **FR-004 Admin Panel Boundary Enforcement**: Direct access to `/admin` surfaces MUST be tested. `User::canAccessPanel()` MUST NOT be the only boundary when it is permissive. +- **FR-005 System Panel Boundary Enforcement**: Direct access to `/system` MUST be tested. Ordinary workspace users MUST NOT access the System panel through workspace roles. +- **FR-006 Workspace Isolation**: Cross-workspace direct access MUST be denied for at least Environment Review, Review Pack, FindingException or decision surface, OperationRun, and ProviderConnection or equivalent sensitive resource. +- **FR-007 Managed Environment Isolation**: Same-workspace wrong-environment direct access MUST be denied for at least Review/ReviewPack, Evidence or StoredReport, FindingException, and OperationRun. +- **FR-008 Provider Connection Boundary**: ProviderConnection view/manage/verify/update/delete/dedicated-credential capabilities MUST match the target role model and be proven server-side. +- **FR-009 Review / Review Pack Boundary**: Customer-safe review and review-pack surfaces MUST preserve existing access rules, redaction, and no hidden environment leakage in summaries, counts, exports, or download URLs. +- **FR-010 FindingException / Decision Boundary**: Approval, rejection, renewal, revocation, closure, and accepted-risk lifecycle actions MUST be capability-gated server-side. +- **FR-011 OperationRun Boundary**: OperationRun visibility and action permissions MUST be workspace/environment scope-safe; run links MUST NOT grant access to users who cannot view the underlying run. +- **FR-012 Service-level Authorization**: Critical mutation paths MUST be checked for policy/gate enforcement beyond hidden UI actions. +- **FR-013 No New RBAC System**: Implementation MUST reuse existing capabilities, policies, gates, middleware, and resource checks. +- **FR-014 Close-out Inventory Format**: Close-out MUST include `Role -> Capabilities -> Sensitive? -> Matches target? -> Action`. + +## Non-Functional Requirements + +- **NFR-001 Minimality**: Fix only confirmed contradictions and direct boundary bugs. +- **NFR-002 Auditability**: Sensitive access decisions must be explainable by role, capability, workspace, environment, and policy. +- **NFR-003 Determinism**: Same user, workspace, environment, and capability set must always produce the same authorization outcome. +- **NFR-004 Testability**: Each corrected boundary must have focused Unit or Feature coverage. +- **NFR-005 No UI-only Security**: Navigation visibility, hidden actions, and disabled actions are insufficient proof. +- **NFR-006 No New Assets**: No CSS, JS, Vite, or design-system changes. +- **NFR-007 No Migration By Default**: Stop and document if a migration appears necessary. + +## Security Requirements + +- **SEC-001**: Workspace isolation is mandatory for all workspace-owned records. +- **SEC-002**: Managed-environment isolation is mandatory for all environment-owned records. +- **SEC-003**: Panel access must be enforced server-side. +- **SEC-004**: Membership management must be Owner-only unless a repo/product decision explicitly says otherwise. +- **SEC-005**: Provider credential-level operations must be high privilege only. +- **SEC-006**: Readonly/customer-safe users must not mutate anything. +- **SEC-007**: Platform/system users must not implicitly access customer workspace data through system authority. +- **SEC-008**: Direct URLs to denied objects must follow repo-standard denial semantics: `404` for non-member/out-of-scope and `403` for in-scope missing capability where existing policies define it. +- **SEC-009**: No test may rely only on navigation invisibility. + +## Repo Evidence Anchors + +Initial preparation found these repo-real paths: + +| Area | Repo-real path | Preparation note | +|---|---|---| +| Capability registry | `apps/platform/app/Support/Auth/Capabilities.php` | Canonical tenant/workspace capability names | +| Workspace role map | `apps/platform/app/Services/Auth/WorkspaceRoleCapabilityMap.php` | User draft said `Support/Auth`; repo path is `Services/Auth` | +| Managed-environment role map | `apps/platform/app/Services/Auth/RoleCapabilityMap.php` | Current tenant-role capabilities | +| Workspace roles | `apps/platform/app/Support/Auth/WorkspaceRole.php` | Owner, Manager, Operator, Readonly | +| Tenant roles | `apps/platform/app/Support/TenantRole.php` | Owner, Manager, Operator, Readonly | +| Platform capabilities | `apps/platform/app/Support/Auth/PlatformCapabilities.php` | System panel capabilities | +| Auth/provider wiring | `apps/platform/app/Providers/AuthServiceProvider.php` | Gate and policy registration | +| User panel access | `apps/platform/app/Models/User.php` | `canAccessPanel()` currently returns true | +| Admin panel | `apps/platform/app/Providers/Filament/AdminPanelProvider.php` | `/admin`, web guard, workspace/environment middleware | +| System panel | `apps/platform/app/Providers/Filament/SystemPanelProvider.php` | `/system`, platform guard, platform capability middleware | +| Provider registration | `apps/platform/bootstrap/providers.php` | Laravel 12 provider registration location | +| Workspace context | `apps/platform/app/Support/Workspaces/WorkspaceContext.php` | Workspace session/context enforcement | +| Environment routes | `apps/platform/app/Filament/Concerns/WorkspaceScopedTenantRoutes.php` | Managed-environment scoped URL model | +| OperationRun policy | `apps/platform/app/Policies/OperationRunPolicy.php` | Workspace/environment/run access | +| ProviderConnection policy | `apps/platform/app/Policies/ProviderConnectionPolicy.php` | Provider credential/scope boundary | +| Review policy | `apps/platform/app/Policies/EnvironmentReviewPolicy.php` | Review access/mutation | +| ReviewPack policy | `apps/platform/app/Policies/ReviewPackPolicy.php` | Review pack access/mutation | +| Evidence policy | `apps/platform/app/Policies/EvidenceSnapshotPolicy.php` | Evidence access/mutation | +| FindingException policy | `apps/platform/app/Policies/FindingExceptionPolicy.php` | Accepted-risk/decision lifecycle | +| Constitution | `.specify/memory/constitution.md` | RBAC-UX owner-only and cross-plane rules | + +## Initial Repo-Derived Inventory Snapshot + +This preparation did not implement changes, but read-only inspection found: + +- `WorkspaceRoleCapabilityMap` grants Owner `WORKSPACE_MEMBERSHIP_MANAGE` and `TENANT_MEMBERSHIP_MANAGE` through merged tenant-role capabilities. +- `WorkspaceRoleCapabilityMap` grants Manager `WORKSPACE_MEMBERSHIP_MANAGE` directly and appends `TENANT_MEMBERSHIP_MANAGE` in `getCapabilities()`. +- `RoleCapabilityMap` grants tenant Manager provider manage, review pack manage, environment review manage, evidence manage, and many governance mutation capabilities, while it does not grant `TENANT_ROLE_MAPPING_MANAGE`. +- Operator has operational and read scopes but no `TENANT_MEMBERSHIP_MANAGE`, `TENANT_MANAGE`, `PROVIDER_MANAGE`, or `TENANT_BACKUP_SCHEDULES_MANAGE`. +- Readonly has view-only provider/review/evidence/audit-style capabilities and no mutation capabilities from the inspected map. +- Platform users are separate `PlatformUser` records using the `platform` guard and `PlatformCapabilities`. +- Existing system-panel tests already prove several tenant-session-to-`/system` denials, but Spec 309 should add/extend focused boundary tests for the current risk matrix. + +## Suspected Contradictions To Verify + +- Manager membership-management grant appears to contradict the Constitution line: "Manager ... MUST NOT manage tenant memberships (Owner-only)." +- `User::canAccessPanel()` returning true may be acceptable only if admin/system middleware, guards, workspace context, and policies definitively enforce access. Implementation must prove this before changing it. +- Existing tests currently assert Manager can manage tenant membership in `apps/platform/tests/Unit/Auth/CapabilityResolverTest.php` and `apps/platform/tests/Feature/Rbac/RoleMatrix/ManagerAccessTest.php`; if Owner-only is confirmed, those tests must be updated as failing proof before runtime fix. +- Provider manage and credential-level capabilities for Manager may be intentional or too broad; classify before changing. +- Review/ReviewPack/Evidence manage capabilities for Manager may be intentional governance workflow authority; classify before changing. +- FindingException approval via workspace capability currently grants Owner and Manager from the inspected map; classify whether Manager approval is intended. + +## Acceptance Criteria + +- **AC-001 Role Inventory Produced**: Close-out includes a role/capability inventory for Owner, Manager, Operator, Readonly, Platform/System, and repo-real special roles. +- **AC-002 Owner-only Contradictions Resolved Or Classified**: Manager/Operator grants of owner-only membership-management capability are fixed or explicitly classified. +- **AC-003 Admin Panel Boundary Tested**: Tests prove users without valid admin/workspace authority cannot access `/admin` surfaces through direct URLs. +- **AC-004 System Panel Boundary Tested**: Tests prove ordinary workspace users cannot access `/system`. +- **AC-005 Workspace Isolation Tested**: Cross-workspace direct access is denied for EnvironmentReview, ReviewPack, FindingException or decision surface, OperationRun, and ProviderConnection/equivalent. +- **AC-006 Managed Environment Isolation Tested**: Same-workspace wrong-environment direct access is denied for Review/ReviewPack, Evidence or StoredReport, FindingException, and OperationRun. +- **AC-007 Sensitive Actions Tested**: At least one sensitive action from each group is tested against unauthorized roles: membership management, provider credential management, review/review-pack mutation, accepted-risk lifecycle mutation, and operation action/view. +- **AC-008 UI Visibility Is Not The Only Guard**: Every fixed sensitive action has a direct server-side access/action test. +- **AC-009 No Broad RBAC Redesign**: No new role model, table, public permission framework, or capability registry redesign is introduced. +- **AC-010 Focused Tests Pass**: Focused RBAC/panel/access-boundary tests pass. +- **AC-011 Existing Product Flows Still Work**: Existing Review, Review Pack, Findings, and OperationRun scenarios covered by regression tests still pass for Owner and allowed Manager/Operator roles. +- **AC-012 Filament v5 Contract Preserved**: Filament remains v5 with Livewire v4; provider registration remains in `apps/platform/bootstrap/providers.php`; no new assets are introduced; deploy continues to include the existing Filament asset publication step where registered assets are used. + +## Assumptions + +- Spec 308 is merged or cleanly separated before implementation. +- The product remains pre-production, so removing incorrect capability grants does not require data migration shims. +- Existing capability names remain canonical; no new capability strings are introduced unless implementation proves an existing owner-only capability cannot express the boundary. +- Browser tests are optional and only needed if Feature/Filament tests cannot prove a panel or action boundary. + +## Risks + +- Removing Manager membership authority may break existing tests and workflows that assumed Manager could manage memberships. +- Constitution may be newer than current product intent; unclear cases must be product-decision-needed instead of silently fixed. +- Tightening `canAccessPanel()` without understanding workspace-selection flow could block legitimate login/chooser flows. +- Provider/manage and review/manage authority may be too broad for this spec if the repo treats them as product decisions. +- Adding broad fixtures could worsen test-suite cost; keep RBAC tests focused and helper setup opt-in. + +## Open Questions + +- Can Manager manage workspace membership? Recommended default: no. +- Can Manager manage managed-environment membership/access scope? Recommended default: no unless product explicitly approves partial scope management. +- Can Manager rotate or delete provider credentials? Recommended default: no. +- Can Manager approve accepted risks? Recommended default: existing policy unless unsafe, but classify. +- Can Readonly download Review Packs? Recommended default: capability-gated according to existing review-pack policy. +- Should `User::canAccessPanel()` be restrictive? Recommended default: restrictive where it does not break workspace selection, with middleware/policies still enforcing fine-grained boundaries. +- Can platform/system users access workspace data? Recommended default: no implicit access; support access governance stays separate. + +## Candidate Selection Rationale + +- **Selected candidate**: `309 - RBAC Role Matrix & Access Boundary Audit`. +- **Source locations**: + - explicit user-provided Spec 309 draft on 2026-05-15 + - `.specify/memory/constitution.md` RBAC-UX rules for workspace/tenant isolation, owner-only tenant membership management, cross-plane denial, and server-side authorization + - `docs/product/roadmap.md` enterprise access boundary/security hardening context + - `docs/product/spec-candidates.md` enterprise access boundary/support access governance context, with support-access implementation kept out of scope +- **Why selected**: The repo-read found a high-risk static mismatch between Constitution owner-only membership semantics and the current Manager grants in `WorkspaceRoleCapabilityMap`. This security-boundary warning should be verified before further customer-facing productization. +- **Why close alternatives were deferred**: + - Customer Review Workspace v1 Completion is deferred until RBAC/access boundaries are verified. + - Product Truth / Docs Drift Reconciliation is deferred until Spec 309 distinguishes runtime truth from docs drift. + - Support Access Governance v1 is deferred because it is a new support/impersonation product slice, not the minimal role-matrix audit. +- **Roadmap relationship**: Foundation hardening before customer-safe governance productization. +- **Smallest viable implementation slice**: inventory, classify, focused tests, minimal fixes for confirmed contradictions, and close-out decisions. + +## Completed-Spec Guardrail Result + +Related existing specs are context only and must not be rewritten: + +- `specs/285-workspace-rbac-environment-access/` has implementation-completed/validated task markers and browser-smoke proof; use only as workspace-first RBAC context. +- `specs/276-support-access-governance/` has completed task markers and review outcome; support access remains a follow-up, not part of 309. +- `specs/301-admin-inventory-navigation-cutover/`, `specs/302-tenant-owned-surface-route-audit/`, `specs/303-admin-directory-groups-cutover/`, and `specs/304-tenant-panel-dead-code-retirement/` are completed/reviewed context for panel and route behavior. +- `specs/307-decision-register-evidence-operationrun-link-polish/` and `specs/308-decision-register-summary-review-pack/` are Decision Register / Review Pack context only. +- No existing `specs/309-*` package existed before this preparation. + +## Follow-up Candidates + +- Support Access Governance v1: audited support/impersonation, TTL, reason, approval, banner, and exportable access logs. +- Product Truth / Docs Drift Reconciliation: update roadmap/ledger/candidates after repo truth is confirmed. +- Commercial Entitlements / Billing-State Enforcement: plan lifecycle and workspace entitlement gates. +- Customer Review Workspace v1 Completion: continue customer-facing productization after security boundary verification. +- Route / Panel Access Contract Audit: broader route duplication and canonical-route cleanup if 309 uncovers structural route drift. + +## Implementation Done Definition + +Spec 309 is done when the role/capability inventory is documented, owner-only contradictions are fixed or classified, `/admin` and `/system` boundaries are tested, cross-workspace and cross-environment boundaries are tested, sensitive actions are server-side denied for unauthorized roles, no new RBAC architecture is introduced, focused tests pass, Pint dirty passes, `git diff --check` passes, and remaining product decisions are listed as follow-ups. diff --git a/specs/309-rbac-role-matrix-access-boundary-audit/tasks.md b/specs/309-rbac-role-matrix-access-boundary-audit/tasks.md new file mode 100644 index 00000000..98374af7 --- /dev/null +++ b/specs/309-rbac-role-matrix-access-boundary-audit/tasks.md @@ -0,0 +1,204 @@ +# Tasks: RBAC Role Matrix & Access Boundary Audit + +**Input**: Design documents from `/specs/309-rbac-role-matrix-access-boundary-audit/` +**Prerequisites**: `spec.md`, `plan.md`, `checklists/requirements.md` +**Tests**: Required. Use Pest 4 Unit/Feature tests; Browser only if Feature/Filament tests cannot prove the access boundary. + +## Test Governance Checklist + +- [x] Lane assignment is named and is the narrowest sufficient proof for the changed behavior. +- [x] New or changed tests stay in Unit/Feature unless a panel interaction cannot be proven otherwise. +- [x] Shared helpers, factories, seeds, fixtures, and context defaults stay cheap by default. +- [x] Planned validation commands cover the changed boundaries without pulling unrelated lane cost. +- [x] Browser coverage, if any, is explicit and justified. +- [x] Close-out records fixed contradictions, deferred decisions, and validation results. + +## Phase 1: Read-Only Inventory + +**Purpose**: Verify repo truth before any runtime or test changes. + +- [x] T001 Confirm current branch is `309-rbac-role-matrix-access-boundary-audit` and working tree state is understood. +- [x] T002 Read `.specify/memory/constitution.md` RBAC, workspace isolation, tenant isolation, cross-plane, and testing sections. +- [x] T003 [P] Inspect `apps/platform/app/Support/Auth/Capabilities.php` and `apps/platform/app/Support/Auth/PlatformCapabilities.php`. +- [x] T004 [P] Inspect `apps/platform/app/Services/Auth/WorkspaceRoleCapabilityMap.php` and `apps/platform/app/Services/Auth/RoleCapabilityMap.php`. +- [x] T005 [P] Inspect `apps/platform/app/Support/Auth/WorkspaceRole.php` and `apps/platform/app/Support/TenantRole.php`. +- [x] T006 [P] Inspect `apps/platform/app/Models/User.php` and `apps/platform/app/Models/PlatformUser.php`. +- [x] T007 [P] Inspect `apps/platform/app/Providers/AuthServiceProvider.php` and `apps/platform/bootstrap/providers.php`. +- [x] T008 [P] Inspect `apps/platform/app/Providers/Filament/AdminPanelProvider.php` and `apps/platform/app/Providers/Filament/SystemPanelProvider.php`. +- [x] T009 [P] Inspect `apps/platform/app/Support/Workspaces/WorkspaceContext.php` and `apps/platform/app/Filament/Concerns/WorkspaceScopedTenantRoutes.php`. +- [x] T010 [P] Inspect `apps/platform/app/Policies/WorkspaceMembershipPolicy.php`, `apps/platform/app/Policies/WorkspacePolicy.php`, and workspace membership Filament relation-manager surfaces. +- [x] T011 [P] Inspect `apps/platform/app/Policies/ProviderConnectionPolicy.php` and provider connection resources/actions. +- [x] T012 [P] Inspect `apps/platform/app/Policies/EnvironmentReviewPolicy.php`, `apps/platform/app/Policies/ReviewPackPolicy.php`, `apps/platform/app/Policies/EvidenceSnapshotPolicy.php`, and related resources/pages. +- [x] T013 [P] Inspect `apps/platform/app/Policies/FindingExceptionPolicy.php` and finding exception lifecycle services/actions. +- [x] T014 [P] Inspect `apps/platform/app/Policies/OperationRunPolicy.php`, `apps/platform/app/Support/Operations/OperationRunCapabilityResolver.php`, and operation link/view helpers. +- [x] T015 [P] Inspect existing RBAC, panel, provider, review-pack, customer-review, finding-exception, and OperationRun authorization tests under `apps/platform/tests/`. +- [x] T016 Produce a close-out-ready initial inventory table: `Role -> Capabilities -> Sensitive? -> Matches target? -> Action`. + +## Phase 2: Classification + +**Purpose**: Decide what is a confirmed bug versus intentional product behavior or docs drift. + +- [x] T017 Classify Manager `WORKSPACE_MEMBERSHIP_MANAGE` grant in `apps/platform/app/Services/Auth/WorkspaceRoleCapabilityMap.php`. +- [x] T018 Classify Manager `TENANT_MEMBERSHIP_MANAGE` grant appended in `WorkspaceRoleCapabilityMap::getCapabilities()`. +- [x] T019 Classify Manager provider manage and dedicated credential boundaries in `apps/platform/app/Policies/ProviderConnectionPolicy.php`. +- [x] T020 Classify Manager review, review-pack, evidence, and finding-exception approval/manage capabilities. +- [x] T021 Classify `apps/platform/app/Models/User.php::canAccessPanel()` as safe-by-middleware or a confirmed defense-in-depth gap. +- [x] T022 Classify `/system` cross-plane behavior using existing system-panel tests and direct route behavior. +- [x] T023 Identify confirmed security blockers that must be fixed in Spec 309. +- [x] T024 Identify product-decision-needed items that must not be fixed blindly. +- [x] T025 Identify docs-only drift and follow-up candidates outside Spec 309. +- [x] T026 Record the minimal runtime fix list before editing application code. + +## Phase 3: Tests First - Role And Panel Boundaries + +**Purpose**: Add failing or tightening proof before runtime fixes. + +- [x] T027 [P] Add or update `apps/platform/tests/Unit/Auth/WorkspaceRoleCapabilityMapTest.php` to assert known capabilities, unknown capability rejection, and Owner-only membership-management expectations once confirmed. +- [x] T028 [P] Update `apps/platform/tests/Unit/Auth/CapabilityResolverTest.php` if Manager tenant-membership management is confirmed incorrect. +- [x] T029 [P] Update `apps/platform/tests/Feature/Rbac/RoleMatrix/ManagerAccessTest.php` if Manager tenant-membership management is confirmed incorrect. +- [x] T030 [P] Add or update `apps/platform/tests/Feature/Rbac/WorkspaceRoleCapabilityBoundaryTest.php` for Manager/Operator/Readonly owner-only membership denial and Owner positive coverage. +- [x] T031 [P] Add or update `apps/platform/tests/Feature/Rbac/AdminPanelAccessBoundaryTest.php` to prove direct `/admin` workspace surfaces deny unauthenticated users, users without valid workspace authority, and wrong-plane actors. +- [x] T032 [P] Add or update `apps/platform/tests/Feature/Rbac/SystemPanelAccessBoundaryTest.php` or extend existing system tests to prove ordinary workspace users cannot access `/system` or representative `/system/*` routes. + +## Phase 4: Tests First - Workspace And Environment Isolation + +**Purpose**: Prove direct object access boundaries for representative high-risk resources. + +- [x] T033 [P] Add or update `apps/platform/tests/Feature/Rbac/ManagedEnvironmentAccessBoundaryTest.php` for same-workspace wrong-environment denial. +- [x] T034 [P] Add or update EnvironmentReview direct access coverage in `apps/platform/tests/Feature/Rbac/ManagedEnvironmentAccessBoundaryTest.php` or an existing EnvironmentReview authorization test. +- [x] T035 [P] Add or update ReviewPack cross-workspace and wrong-environment coverage in `apps/platform/tests/Feature/Rbac/ReviewPackAccessBoundaryTest.php` or existing ReviewPack authorization tests. +- [x] T036 [P] Add or update EvidenceSnapshot or StoredReport wrong-environment coverage using repo-real policies/resources. +- [x] T037 [P] Add or update FindingException cross-workspace and wrong-environment coverage in `apps/platform/tests/Feature/Rbac/FindingExceptionLifecycleAccessBoundaryTest.php` or existing finding exception authorization tests. +- [x] T038 [P] Add or update OperationRun workspace-bound and environment-bound access coverage in `apps/platform/tests/Feature/Rbac/OperationRunAccessBoundaryTest.php` or existing OperationRun authorization tests. +- [x] T039 [P] Add or update ProviderConnection cross-workspace and wrong-environment coverage in `apps/platform/tests/Feature/Rbac/ProviderConnectionAccessBoundaryTest.php` or existing ProviderConnection authorization tests. + +## Phase 5: Tests First - Sensitive Actions + +**Purpose**: Prove server-side authorization for mutation/action execution, not only navigation visibility. + +- [x] T040 Add membership-management direct action or policy tests proving non-Owner denial and last-owner guard preservation. +- [x] T041 Add ProviderConnection credential-level denial tests for Manager/Operator/Readonly where high-privilege-only classification is confirmed. +- [x] T042 Add Review/ReviewPack mutation denial tests for Readonly and any non-owner role classified as unauthorized. +- [x] T043 Add FindingException approval/rejection/renew/revoke lifecycle denial tests for unauthorized roles. +- [x] T044 Add OperationRun view/start/cancel/retry/action denial tests for unauthorized or out-of-scope roles using existing operation action surfaces. +- [x] T045 Ensure each destructive action changed by this spec is tested for server-side authorization and existing `->requiresConfirmation()` posture. + +## Phase 6: Minimal Runtime Fixes + +**Purpose**: Fix only confirmed contradictions and direct boundary bugs. + +- [x] T046 Remove or adjust Manager `WORKSPACE_MEMBERSHIP_MANAGE` in `apps/platform/app/Services/Auth/WorkspaceRoleCapabilityMap.php` if Owner-only is confirmed. +- [x] T047 Remove or adjust Manager-specific `TENANT_MEMBERSHIP_MANAGE` append in `WorkspaceRoleCapabilityMap::getCapabilities()` if Owner-only is confirmed. +- [x] T048 Update `apps/platform/app/Models/User.php::canAccessPanel()` only if direct tests prove current permissive behavior is unsafe or can be tightened without breaking login/workspace selection. +- [x] T049 Add missing `Gate::authorize(...)` or policy checks in confirmed service/action mutation paths only. No missing mutation path was confirmed beyond the fixed role-map grants. +- [x] T050 Fix missing policy logic in ProviderConnection, ReviewPack, EnvironmentReview, EvidenceSnapshot, FindingException, or OperationRun policies only for confirmed direct-access bypasses. No direct-access policy bypass was confirmed. +- [x] T051 Align Filament action visibility/disabled state with server-side policy only after the policy/gate behavior is correct. +- [x] T052 Ensure any changed destructive Filament execution action still uses `->action(...)`, `->requiresConfirmation()`, and server-side authorization. No destructive action implementation changed. +- [x] T053 Do not introduce new roles, tables, migrations, capability aliases, broad resolvers, or UI redesigns. + +## Phase 7: Focused Validation + +**Purpose**: Run the smallest honest lanes for changed behavior. + +- [x] T054 Run `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Unit/Auth/WorkspaceRoleCapabilityMapTest.php`. +- [x] T055 Run focused RBAC boundary tests under `apps/platform/tests/Feature/Rbac/` that were added or updated for Spec 309. +- [x] T056 Run existing panel cross-plane tests if panel access changed: `apps/platform/tests/Feature/Auth/CrossScopeAccessTest.php`, `apps/platform/tests/Feature/System/Spec113/AuthorizationSemanticsTest.php`, and `apps/platform/tests/Feature/System/Spec114/SystemConsoleAccessSemanticsTest.php`. +- [x] T057 Run review/customer workspace regressions: `apps/platform/tests/Feature/Reviews/CustomerReviewWorkspaceAuthorizationTest.php`, `apps/platform/tests/Feature/Reviews/CustomerReviewWorkspacePackAccessTest.php`, `apps/platform/tests/Feature/ReviewPack/ReviewPackRbacTest.php`, and `apps/platform/tests/Feature/ReviewPack/ReviewPackDownloadTest.php`. +- [x] T058 Run provider connection authorization regressions if provider boundaries changed. +- [x] T059 Run OperationRun authorization/link regressions if OperationRun policy or links changed. No OperationRun policy/link code changed; existing OperationRun RBAC regression was still run. +- [x] T060 Run `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent`. +- [x] T061 Run `git diff --check`. +- [x] T062 Run browser smoke only if Feature/Filament tests cannot prove a changed panel/action boundary; otherwise document why browser was not needed. + +## Phase 8: Close-Out + +**Purpose**: Leave implementation reviewers with exact security and product-truth decisions. + +- [x] T063 Record final role/capability inventory in close-out notes. +- [x] T064 Record confirmed contradictions fixed. +- [x] T065 Record product decisions deferred. +- [x] T066 Record docs-only drift and follow-up candidates. +- [x] T067 Record tests run, tests not run, and why. +- [x] T068 Confirm no new RBAC model/table/capability framework/migration was introduced. +- [x] T069 Confirm Filament v5 / Livewire v4 compliance, provider registration location, global-search resource posture, destructive-action confirmation/authorization, no new asset strategy, and testing plan coverage. +- [x] T070 Confirm remaining risks and recommended next spec after 309. + +## Dependencies & Execution Order + +- Phase 1 blocks all later phases. +- Phase 2 blocks test writing and runtime edits. +- Phases 3, 4, and 5 can run partly in parallel once classification is complete, but each worker must own disjoint test files. +- Phase 6 starts only after tests for confirmed contradictions exist. +- Phase 7 starts after runtime fixes. +- Phase 8 closes after validation. + +## Parallel Execution Examples + +- T003-T015 can run in parallel as read-only inspection. +- T027-T032 can run in parallel with disjoint test files. +- T033-T039 can run in parallel with disjoint test files. +- T041-T044 can run in parallel when each action group owns separate policies/tests. +- T054-T059 can run in parallel if the Sail environment supports concurrent lanes; otherwise run serially. + +## Implementation Strategy + +1. MVP = role-map contradiction proof + `/admin` and `/system` direct boundary tests. +2. Add workspace/environment isolation proof for the representative resource set. +3. Add sensitive-action tests for the confirmed bug set. +4. Apply minimal fixes. +5. Validate focused lanes and close out with inventory and decisions. + +## Deferred Follow-Ups / Non-Goals + +- Support Access Governance v1. +- Product Truth / Docs Drift Reconciliation. +- Customer Review Workspace v1 Completion. +- Route / Panel Access Contract Audit beyond confirmed 309 bugs. +- Commercial entitlements or billing state. +- New RBAC management UI or permission matrix. + +## Implementation Close-Out Notes + +### RBAC Inventory + +| Role | Capabilities | Sensitive? | Matches target? | Action | +|---|---|---:|---|---| +| Workspace Owner | Workspace membership manage, tenant membership manage, provider manage + dedicated credential manage, review/review-pack/evidence manage, finding exception approve, OperationRun view by capability | Yes | Yes | Kept. Owner positive tests added. | +| Workspace Manager | Workspace membership view, provider manage/run, review/review-pack/evidence manage, finding exception approve, workspace settings/manage, alerts/baselines/audit | Yes | Partially | Fixed confirmed membership-management contradiction. Provider/review/evidence/finding manage remains product-decision-needed because existing runtime and tests treat it as intended. | +| Workspace Operator | Workspace membership view, provider view/run, review/evidence view, findings triage, audit, allowed operation visibility | Medium/High | Yes | Kept and covered by boundary tests. | +| Workspace Readonly | Workspace/settings/alerts/baselines/audit view plus customer-safe tenant/review/evidence/provider/review-pack view where scoped | Medium | Yes | Kept and mutation denials covered. | +| Platform/System | Separate `platform` guard + `PlatformCapabilities`; no implicit admin/workspace access | Critical | Yes | Kept; `/system` direct tests added and cross-plane regressions passed. | + +### Confirmed Contradictions Fixed + +- Manager no longer receives `Capabilities::WORKSPACE_MEMBERSHIP_MANAGE` from `WorkspaceRoleCapabilityMap`. +- Manager no longer receives the Manager-only appended `Capabilities::TENANT_MEMBERSHIP_MANAGE` from `WorkspaceRoleCapabilityMap::getCapabilities()`. +- `User::canAccessPanel()` is now restricted to the `admin` panel; ordinary web users no longer advertise access to the `system` panel. + +### Product Decisions Deferred + +- Manager `PROVIDER_MANAGE`, `REVIEW_PACK_MANAGE`, `ENVIRONMENT_REVIEW_MANAGE`, `EVIDENCE_MANAGE`, and `FINDING_EXCEPTION_APPROVE` remain unchanged because repo-real policies and existing regressions treat them as current product behavior. +- Provider dedicated credential management remains high-privilege only through `PROVIDER_MANAGE_DEDICATED`; tests prove Manager/Operator/Readonly denial and Owner allowance. +- Support access governance, commercial lifecycle enforcement, and broader route/panel contract cleanup remain follow-up candidates. + +### Validation Results + +- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Unit/Auth/WorkspaceRoleCapabilityMapTest.php tests/Unit/Auth/CapabilityResolverTest.php tests/Feature/Rbac/RoleMatrix/ManagerAccessTest.php tests/Feature/Rbac/WorkspaceRoleCapabilityBoundaryTest.php tests/Feature/Rbac/AdminPanelAccessBoundaryTest.php tests/Feature/Rbac/SystemPanelAccessBoundaryTest.php` — 17 passed. +- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Rbac/ManagedEnvironmentAccessBoundaryTest.php tests/Feature/Rbac/ProviderConnectionAccessBoundaryTest.php tests/Feature/Rbac/ReviewPackAccessBoundaryTest.php tests/Feature/Rbac/OperationRunAccessBoundaryTest.php tests/Feature/Rbac/FindingExceptionLifecycleAccessBoundaryTest.php` — 12 passed. +- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Auth/WorkspaceFirstManagedEnvironmentAccessTest.php tests/Feature/Rbac/WorkspaceMembershipsRelationManagerUiEnforcementTest.php tests/Feature/Auth/CrossScopeAccessTest.php tests/Feature/Auth/SystemPanelAuthTest.php` — 14 passed. +- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/System/Spec113/AuthorizationSemanticsTest.php tests/Feature/System/Spec114/SystemConsoleAccessSemanticsTest.php` — 22 passed. +- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Reviews/CustomerReviewWorkspaceAuthorizationTest.php tests/Feature/Reviews/CustomerReviewWorkspacePackAccessTest.php tests/Feature/ReviewPack/ReviewPackRbacTest.php tests/Feature/ReviewPack/ReviewPackDownloadTest.php` — 30 passed. +- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Rbac/ProviderConnectionWorkspaceFirstPolicyTest.php tests/Feature/Rbac/OperationRunWorkspaceFirstAuthorizationTest.php tests/Feature/ProviderConnections/ManageCapabilityEnforcementTest.php tests/Feature/ProviderConnections/ProviderConnectionAuthorizationTest.php` — 13 passed. +- `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent` — passed. +- `git diff --check` — passed. + +Browser smoke was not run because the changed behavior is backend authorization, role maps, panel contract checks, and Filament/Livewire action enforcement proven by Feature/Livewire tests. No frontend assets, layouts, forms, or navigation redesign were introduced. + +### Filament / Runtime Compliance + +- Filament v5 remains on Livewire v4.1.4. +- Panel providers remain registered through `apps/platform/bootstrap/providers.php`. +- New/changed resources are not introduced; existing touched resources keep their current global-search posture. +- No destructive action implementation changed. Existing workspace membership destructive removal still uses an execution action with confirmation and the server-side workspace capability guard. +- No assets were added or changed; no `filament:assets` deploy impact. +- No new RBAC model, role, table, migration, capability alias, public permission framework, or UI redesign was introduced.