From d1bdd7a3826ef7796499f023a260415aa9f88eca Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Fri, 13 Feb 2026 02:29:38 +0100 Subject: [PATCH] feat(spec-090): action surface contract compliance --- .github/agents/copilot-instructions.md | 2 +- .../Resources/BackupScheduleResource.php | 25 +- .../Pages/ListBackupSchedules.php | 19 +- app/Filament/Resources/BackupSetResource.php | 2 +- .../BackupItemsRelationManager.php | 7 +- app/Filament/Resources/FindingResource.php | 97 +++++--- .../PolicyResource/Pages/ListPolicies.php | 4 +- .../PolicyResource/Pages/ViewPolicy.php | 58 ++++- .../Resources/PolicyVersionResource.php | 7 +- .../Resources/ProviderConnectionResource.php | 19 +- .../Pages/ListProviderConnections.php | 117 ++++++++- app/Filament/Resources/TenantResource.php | 231 ++++++++++-------- .../TenantResource/Pages/ListTenants.php | 5 + .../Workspaces/Pages/CreateWorkspace.php | 15 ++ .../Workspaces/Pages/EditWorkspace.php | 21 ++ .../Workspaces/Pages/ListWorkspaces.php | 19 +- .../Workspaces/Pages/ViewWorkspace.php | 10 +- .../Workspaces/WorkspaceResource.php | 77 +++++- app/Support/Rbac/UiEnforcement.php | 10 + .../ActionSurface/ActionSurfaceExemptions.php | 5 - .../checklists/requirements.md | 35 +++ .../action-surface-contract-v1.openapi.yaml | 139 +++++++++++ .../data-model.md | 59 +++++ .../plan.md | 195 +++++++++++++++ .../quickstart.md | 31 +++ .../research.md | 84 +++++++ .../spec.md | 222 +++++++++++++++++ .../tasks.md | 158 ++++++++++++ tests/Feature/090/ActionSurfaceSmokeTest.php | 28 +++ tests/Feature/090/AuditLoggingTest.php | 114 +++++++++ tests/Feature/090/EmptyStateCtasTest.php | 100 ++++++++ tests/Feature/090/RbacSemanticsTest.php | 51 ++++ ...rkspaceManagedTenantAdminMigrationTest.php | 14 +- 33 files changed, 1790 insertions(+), 190 deletions(-) create mode 100644 specs/090-action-surface-contract-compliance/checklists/requirements.md create mode 100644 specs/090-action-surface-contract-compliance/contracts/action-surface-contract-v1.openapi.yaml create mode 100644 specs/090-action-surface-contract-compliance/data-model.md create mode 100644 specs/090-action-surface-contract-compliance/plan.md create mode 100644 specs/090-action-surface-contract-compliance/quickstart.md create mode 100644 specs/090-action-surface-contract-compliance/research.md create mode 100644 specs/090-action-surface-contract-compliance/spec.md create mode 100644 specs/090-action-surface-contract-compliance/tasks.md create mode 100644 tests/Feature/090/ActionSurfaceSmokeTest.php create mode 100644 tests/Feature/090/AuditLoggingTest.php create mode 100644 tests/Feature/090/EmptyStateCtasTest.php create mode 100644 tests/Feature/090/RbacSemanticsTest.php diff --git a/.github/agents/copilot-instructions.md b/.github/agents/copilot-instructions.md index 26df7a3..bd971cf 100644 --- a/.github/agents/copilot-instructions.md +++ b/.github/agents/copilot-instructions.md @@ -47,8 +47,8 @@ ## Code Style PHP 8.4.15: Follow standard conventions ## Recent Changes +- 090-action-surface-contract-compliance: Added PHP 8.4.15 - 087-legacy-runs-removal: Added PHP 8.4.15 + Laravel 12, Filament v5, Livewire v4 - 088-remove-tenant-graphoptions-legacy: Added PHP 8.4.15 (Laravel 12) + Filament v5, Livewire v4, Pest v4 -- 086-retire-legacy-runs-into-operation-runs: Spec docs updated (PHP 8.4.15 + Laravel 12, Filament v5, Livewire v4) diff --git a/app/Filament/Resources/BackupScheduleResource.php b/app/Filament/Resources/BackupScheduleResource.php index 3974097..29e8374 100644 --- a/app/Filament/Resources/BackupScheduleResource.php +++ b/app/Filament/Resources/BackupScheduleResource.php @@ -25,6 +25,10 @@ use App\Support\OpsUx\OperationUxPresenter; use App\Support\OpsUx\OpsUxBrowserEvents; use App\Support\Rbac\UiEnforcement; +use App\Support\Ui\ActionSurface\ActionSurfaceDeclaration; +use App\Support\Ui\ActionSurface\Enums\ActionSurfaceInspectAffordance; +use App\Support\Ui\ActionSurface\Enums\ActionSurfaceProfile; +use App\Support\Ui\ActionSurface\Enums\ActionSurfaceSlot; use BackedEnum; use DateTimeZone; use Filament\Actions\Action; @@ -168,6 +172,17 @@ public static function canDeleteAny(): bool return $resolver->can($user, $tenant, Capabilities::TENANT_BACKUP_SCHEDULES_MANAGE); } + public static function actionSurfaceDeclaration(): ActionSurfaceDeclaration + { + return ActionSurfaceDeclaration::forResource(ActionSurfaceProfile::CrudListAndEdit) + ->satisfy(ActionSurfaceSlot::ListHeader, 'Header actions include capability-gated create.') + ->satisfy(ActionSurfaceSlot::InspectAffordance, ActionSurfaceInspectAffordance::ClickableRow->value) + ->satisfy(ActionSurfaceSlot::ListRowMoreMenu, 'Row-level secondary actions are grouped under "More".') + ->satisfy(ActionSurfaceSlot::ListBulkMoreGroup, 'Bulk actions are grouped under "More".') + ->satisfy(ActionSurfaceSlot::ListEmptyState, 'List page defines a capability-gated empty-state create CTA.') + ->satisfy(ActionSurfaceSlot::DetailHeader, 'Edit page provides save/cancel controls.'); + } + public static function form(Schema $schema): Schema { return $schema @@ -242,6 +257,9 @@ public static function table(Table $table): Table { return $table ->defaultSort('next_run_at', 'asc') + ->recordUrl(fn (BackupSchedule $record): ?string => static::canEdit($record) + ? static::getUrl('edit', ['record' => $record]) + : null) ->columns([ TextColumn::make('is_enabled') ->label('Enabled') @@ -520,7 +538,10 @@ public static function table(Table $table): Table ->requireCapability(Capabilities::TENANT_BACKUP_SCHEDULES_MANAGE) ->destructive() ->apply(), - ])->icon('heroicon-o-ellipsis-vertical'), + ]) + ->label('More') + ->icon('heroicon-o-ellipsis-vertical') + ->color('gray'), ]) ->bulkActions([ BulkActionGroup::make([ @@ -724,7 +745,7 @@ public static function table(Table $table): Table ->requireCapability(Capabilities::TENANT_BACKUP_SCHEDULES_MANAGE) ->destructive() ->apply(), - ]), + ])->label('More'), ]); } diff --git a/app/Filament/Resources/BackupScheduleResource/Pages/ListBackupSchedules.php b/app/Filament/Resources/BackupScheduleResource/Pages/ListBackupSchedules.php index b7febad..1a20a1b 100644 --- a/app/Filament/Resources/BackupScheduleResource/Pages/ListBackupSchedules.php +++ b/app/Filament/Resources/BackupScheduleResource/Pages/ListBackupSchedules.php @@ -12,8 +12,21 @@ class ListBackupSchedules extends ListRecords protected function getHeaderActions(): array { - return [ - Actions\CreateAction::make(), - ]; + return [$this->makeCreateAction()]; + } + + protected function getTableEmptyStateActions(): array + { + return [$this->makeCreateAction()]; + } + + private function makeCreateAction(): Actions\CreateAction + { + return Actions\CreateAction::make() + ->label('New backup schedule') + ->disabled(fn (): bool => ! BackupScheduleResource::canCreate()) + ->tooltip(fn (): ?string => BackupScheduleResource::canCreate() + ? null + : 'You do not have permission to create backup schedules.'); } } diff --git a/app/Filament/Resources/BackupSetResource.php b/app/Filament/Resources/BackupSetResource.php index 8f26d7f..5c76249 100644 --- a/app/Filament/Resources/BackupSetResource.php +++ b/app/Filament/Resources/BackupSetResource.php @@ -470,7 +470,7 @@ public static function table(Table $table): Table ) ->requireCapability(Capabilities::TENANT_DELETE) ->apply(), - ]), + ])->label('More'), ]); } diff --git a/app/Filament/Resources/BackupSetResource/RelationManagers/BackupItemsRelationManager.php b/app/Filament/Resources/BackupSetResource/RelationManagers/BackupItemsRelationManager.php index 75e0d25..30a19c2 100644 --- a/app/Filament/Resources/BackupSetResource/RelationManagers/BackupItemsRelationManager.php +++ b/app/Filament/Resources/BackupSetResource/RelationManagers/BackupItemsRelationManager.php @@ -337,12 +337,15 @@ public function table(Table $table): Table ->hidden(fn (BackupItem $record) => ! $record->policy_id) ->openUrlInNewTab(true), $removeItem, - ])->icon('heroicon-o-ellipsis-vertical'), + ]) + ->label('More') + ->icon('heroicon-o-ellipsis-vertical') + ->color('gray'), ]) ->bulkActions([ Actions\BulkActionGroup::make([ $bulkRemove, - ]), + ])->label('More'), ]); } diff --git a/app/Filament/Resources/FindingResource.php b/app/Filament/Resources/FindingResource.php index e2e075a..e8118c0 100644 --- a/app/Filament/Resources/FindingResource.php +++ b/app/Filament/Resources/FindingResource.php @@ -14,6 +14,10 @@ use App\Support\Badges\BadgeRenderer; use App\Support\Rbac\UiEnforcement; use App\Support\Rbac\UiTooltips; +use App\Support\Ui\ActionSurface\ActionSurfaceDeclaration; +use App\Support\Ui\ActionSurface\Enums\ActionSurfaceInspectAffordance; +use App\Support\Ui\ActionSurface\Enums\ActionSurfaceProfile; +use App\Support\Ui\ActionSurface\Enums\ActionSurfaceSlot; use BackedEnum; use Filament\Actions; use Filament\Actions\BulkAction; @@ -88,6 +92,17 @@ public static function canView(Model $record): bool return true; } + public static function actionSurfaceDeclaration(): ActionSurfaceDeclaration + { + return ActionSurfaceDeclaration::forResource(ActionSurfaceProfile::CrudListAndView) + ->satisfy(ActionSurfaceSlot::ListHeader, 'Header action supports acknowledging all matching findings.') + ->satisfy(ActionSurfaceSlot::InspectAffordance, ActionSurfaceInspectAffordance::ViewAction->value) + ->satisfy(ActionSurfaceSlot::ListRowMoreMenu, 'Secondary row actions are grouped under "More".') + ->satisfy(ActionSurfaceSlot::ListBulkMoreGroup, 'Bulk actions are grouped under "More".') + ->exempt(ActionSurfaceSlot::ListEmptyState, 'Findings are generated by drift detection and intentionally have no create CTA.') + ->exempt(ActionSurfaceSlot::DetailHeader, 'View page intentionally has no additional header actions.'); + } + public static function form(Schema $schema): Schema { return $schema; @@ -319,46 +334,48 @@ public static function table(Table $table): Table }), ]) ->actions([ - Actions\Action::make('acknowledge') - ->label('Acknowledge') - ->icon('heroicon-o-check') - ->color('gray') - ->requiresConfirmation() - ->visible(fn (Finding $record): bool => $record->status === Finding::STATUS_NEW) - ->authorize(function (Finding $record): bool { - $user = auth()->user(); - - if (! $user instanceof User) { - return false; - } - - return $user->can('update', $record); - }) - ->action(function (Finding $record): void { - $tenant = Tenant::current(); - $user = auth()->user(); - - if (! $tenant || ! $user instanceof User) { - return; - } - - if ((int) $record->tenant_id !== (int) $tenant->getKey()) { - Notification::make() - ->title('Finding belongs to a different tenant') - ->danger() - ->send(); - - return; - } - - $record->acknowledge($user); - - Notification::make() - ->title('Finding acknowledged') - ->success() - ->send(); - }), Actions\ViewAction::make(), + Actions\ActionGroup::make([ + UiEnforcement::forAction( + Actions\Action::make('acknowledge') + ->label('Acknowledge') + ->icon('heroicon-o-check') + ->color('gray') + ->requiresConfirmation() + ->visible(fn (Finding $record): bool => $record->status === Finding::STATUS_NEW) + ->action(function (Finding $record): void { + $tenant = Tenant::current(); + $user = auth()->user(); + + if (! $tenant || ! $user instanceof User) { + return; + } + + if ((int) $record->tenant_id !== (int) $tenant->getKey()) { + Notification::make() + ->title('Finding belongs to a different tenant') + ->danger() + ->send(); + + return; + } + + $record->acknowledge($user); + + Notification::make() + ->title('Finding acknowledged') + ->success() + ->send(); + }), + ) + ->preserveVisibility() + ->requireCapability(Capabilities::TENANT_FINDINGS_ACKNOWLEDGE) + ->tooltip(UiTooltips::INSUFFICIENT_PERMISSION) + ->apply(), + ]) + ->label('More') + ->icon('heroicon-o-ellipsis-vertical') + ->color('gray'), ]) ->bulkActions([ BulkActionGroup::make([ @@ -418,7 +435,7 @@ public static function table(Table $table): Table ->requireCapability(Capabilities::TENANT_FINDINGS_ACKNOWLEDGE) ->tooltip(UiTooltips::INSUFFICIENT_PERMISSION) ->apply(), - ]), + ])->label('More'), ]); } diff --git a/app/Filament/Resources/PolicyResource/Pages/ListPolicies.php b/app/Filament/Resources/PolicyResource/Pages/ListPolicies.php index 2cab3db..7d8edc5 100644 --- a/app/Filament/Resources/PolicyResource/Pages/ListPolicies.php +++ b/app/Filament/Resources/PolicyResource/Pages/ListPolicies.php @@ -37,6 +37,9 @@ private function makeSyncAction(): Actions\Action ->label('Sync from Intune') ->icon('heroicon-o-arrow-path') ->color('primary') + ->requiresConfirmation() + ->modalHeading('Sync policies from Intune') + ->modalDescription('This queues a background sync operation for supported policy types in the current tenant.') ->action(function (self $livewire): void { $tenant = Tenant::current(); $user = auth()->user(); @@ -94,7 +97,6 @@ private function makeSyncAction(): Actions\Action ) ->requireCapability(Capabilities::TENANT_SYNC) ->tooltip('You do not have permission to sync policies.') - ->destructive() ->apply(); } } diff --git a/app/Filament/Resources/PolicyResource/Pages/ViewPolicy.php b/app/Filament/Resources/PolicyResource/Pages/ViewPolicy.php index 699b86d..55d53c3 100644 --- a/app/Filament/Resources/PolicyResource/Pages/ViewPolicy.php +++ b/app/Filament/Resources/PolicyResource/Pages/ViewPolicy.php @@ -4,10 +4,13 @@ use App\Filament\Resources\PolicyResource; use App\Jobs\CapturePolicySnapshotJob; +use App\Services\Intune\AuditLogger; use App\Services\OperationRunService; use App\Services\Operations\BulkSelectionIdentity; +use App\Support\Auth\Capabilities; use App\Support\OperationRunLinks; use App\Support\OpsUx\OperationUxPresenter; +use App\Support\Rbac\UiEnforcement; use Filament\Actions\Action; use Filament\Forms; use Filament\Notifications\Notification; @@ -23,7 +26,12 @@ class ViewPolicy extends ViewRecord protected function getActions(): array { - return [ + return [$this->makeCaptureSnapshotAction()]; + } + + private function makeCaptureSnapshotAction(): Action + { + $action = UiEnforcement::forAction( Action::make('capture_snapshot') ->label('Capture snapshot') ->requiresConfirmation() @@ -39,7 +47,7 @@ protected function getActions(): array ->default(true) ->helperText('Captures policy scope tag IDs.'), ]) - ->action(function (array $data) { + ->action(function (array $data, AuditLogger $auditLogger) { $policy = $this->record; $tenant = $policy->tenant; @@ -61,6 +69,9 @@ protected function getActions(): array /** @var OperationRunService $runs */ $runs = app(OperationRunService::class); + $includeAssignments = (bool) ($data['include_assignments'] ?? false); + $includeScopeTags = (bool) ($data['include_scope_tags'] ?? false); + $opRun = $runs->enqueueBulkOperation( tenant: $tenant, type: 'policy.capture_snapshot', @@ -68,13 +79,13 @@ protected function getActions(): array 'entra_tenant_id' => (string) ($tenant->tenant_id ?? $tenant->external_id), ], selectionIdentity: $selectionIdentity, - dispatcher: function ($operationRun) use ($tenant, $policy, $user, $data): void { + dispatcher: function ($operationRun) use ($tenant, $policy, $user, $includeAssignments, $includeScopeTags): void { CapturePolicySnapshotJob::dispatch( tenantId: (int) $tenant->getKey(), userId: (int) $user->getKey(), policyId: (int) $policy->getKey(), - includeAssignments: (bool) ($data['include_assignments'] ?? false), - includeScopeTags: (bool) ($data['include_scope_tags'] ?? false), + includeAssignments: $includeAssignments, + includeScopeTags: $includeScopeTags, createdBy: $user->email ? Str::limit($user->email, 255, '') : null, operationRun: $operationRun, context: [], @@ -83,8 +94,8 @@ protected function getActions(): array initiator: $user, extraContext: [ 'policy_id' => (int) $policy->getKey(), - 'include_assignments' => (bool) ($data['include_assignments'] ?? false), - 'include_scope_tags' => (bool) ($data['include_scope_tags'] ?? false), + 'include_assignments' => $includeAssignments, + 'include_scope_tags' => $includeScopeTags, ], emitQueuedNotification: false, ); @@ -105,6 +116,26 @@ protected function getActions(): array return; } + + $auditLogger->log( + tenant: $tenant, + action: 'policy.capture_snapshot_dispatched', + resourceType: 'operation_run', + resourceId: (string) $opRun->getKey(), + status: 'success', + context: [ + 'metadata' => [ + 'policy_id' => (int) $policy->getKey(), + 'operation_run_id' => (int) $opRun->getKey(), + 'include_assignments' => $includeAssignments, + 'include_scope_tags' => $includeScopeTags, + ], + ], + actorId: (int) $user->getKey(), + actorEmail: $user->email, + actorName: $user->name, + ); + OperationUxPresenter::queuedToast('policy.capture_snapshot') ->actions([ \Filament\Actions\Action::make('view_run') @@ -115,7 +146,16 @@ protected function getActions(): array $this->redirect(OperationRunLinks::view($opRun, $tenant)); }) - ->color('primary'), - ]; + ->color('primary') + ) + ->requireCapability(Capabilities::TENANT_SYNC) + ->tooltip('You do not have permission to capture policy snapshots.') + ->apply(); + + if (! $action instanceof Action) { + throw new \RuntimeException('Capture snapshot action must resolve to a Filament action.'); + } + + return $action; } } diff --git a/app/Filament/Resources/PolicyVersionResource.php b/app/Filament/Resources/PolicyVersionResource.php index 4ff27d1..994a158 100644 --- a/app/Filament/Resources/PolicyVersionResource.php +++ b/app/Filament/Resources/PolicyVersionResource.php @@ -837,14 +837,17 @@ public static function table(Table $table): Table return $action; })(), - ])->icon('heroicon-o-ellipsis-vertical'), + ]) + ->label('More') + ->icon('heroicon-o-ellipsis-vertical') + ->color('gray'), ]) ->bulkActions([ BulkActionGroup::make([ $bulkPruneVersions, $bulkRestoreVersions, $bulkForceDeleteVersions, - ]), + ])->label('More'), ]); } diff --git a/app/Filament/Resources/ProviderConnectionResource.php b/app/Filament/Resources/ProviderConnectionResource.php index ce2490d..ac3e057 100644 --- a/app/Filament/Resources/ProviderConnectionResource.php +++ b/app/Filament/Resources/ProviderConnectionResource.php @@ -20,6 +20,10 @@ use App\Support\OperationRunLinks; use App\Support\Providers\ProviderReasonCodes; use App\Support\Rbac\UiEnforcement; +use App\Support\Ui\ActionSurface\ActionSurfaceDeclaration; +use App\Support\Ui\ActionSurface\Enums\ActionSurfaceInspectAffordance; +use App\Support\Ui\ActionSurface\Enums\ActionSurfaceProfile; +use App\Support\Ui\ActionSurface\Enums\ActionSurfaceSlot; use App\Support\Workspaces\WorkspaceContext; use BackedEnum; use Filament\Actions; @@ -64,6 +68,17 @@ public static function getNavigationParentItem(): ?string return 'Integrations'; } + public static function actionSurfaceDeclaration(): ActionSurfaceDeclaration + { + return ActionSurfaceDeclaration::forResource(ActionSurfaceProfile::CrudListAndView) + ->satisfy(ActionSurfaceSlot::ListHeader, 'Header actions include capability-gated create.') + ->satisfy(ActionSurfaceSlot::InspectAffordance, ActionSurfaceInspectAffordance::ClickableRow->value) + ->satisfy(ActionSurfaceSlot::ListRowMoreMenu, 'Secondary row actions are grouped under "More".') + ->exempt(ActionSurfaceSlot::ListBulkMoreGroup, 'Provider connections intentionally omit bulk actions.') + ->satisfy(ActionSurfaceSlot::ListEmptyState, 'List page defines empty-state guidance and CTA.') + ->exempt(ActionSurfaceSlot::DetailHeader, 'View page has no additional header mutations in this resource.'); + } + public static function canCreate(): bool { $tenant = static::resolveTenantForCreate(); @@ -416,6 +431,8 @@ public static function table(Table $table): Table { return $table ->modifyQueryUsing(function (Builder $query): Builder { + $query->with('tenant'); + $tenantExternalId = static::resolveRequestedTenantExternalId(); if (! is_string($tenantExternalId) || $tenantExternalId === '') { @@ -1080,7 +1097,7 @@ public static function table(Table $table): Table ->requireCapability(Capabilities::PROVIDER_MANAGE) ->apply(), ]) - ->label('Actions') + ->label('More') ->icon('heroicon-o-ellipsis-vertical') ->color('gray'), ]) diff --git a/app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php b/app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php index 8a8bbb2..8dc5ea4 100644 --- a/app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php +++ b/app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php @@ -3,9 +3,12 @@ namespace App\Filament\Resources\ProviderConnectionResource\Pages; use App\Filament\Resources\ProviderConnectionResource; +use App\Models\Tenant; +use App\Models\User; +use App\Services\Auth\CapabilityResolver; use App\Support\Auth\Capabilities; -use App\Support\Rbac\UiEnforcement; use Filament\Actions; +use Filament\Facades\Filament; use Filament\Resources\Pages\ListRecords; class ListProviderConnections extends ListRecords @@ -14,17 +17,115 @@ class ListProviderConnections extends ListRecords protected function getHeaderActions(): array { + /** @var CapabilityResolver $resolver */ + $resolver = app(CapabilityResolver::class); + return [ - UiEnforcement::forAction( - Actions\CreateAction::make() - ->authorize(fn (): bool => true) - ) - ->requireCapability(Capabilities::PROVIDER_MANAGE) - ->tooltip('You do not have permission to create provider connections.') - ->apply(), + Actions\CreateAction::make() + ->label('New connection') + ->url(function (): string { + $tenantExternalId = $this->resolveTenantExternalIdForCreateAction(); + + if (! is_string($tenantExternalId) || $tenantExternalId === '') { + return ProviderConnectionResource::getUrl('create'); + } + + return ProviderConnectionResource::getUrl('create', [ + 'tenant_id' => $tenantExternalId, + ]); + }) + ->visible(function () use ($resolver): bool { + $tenant = $this->resolveTenantForCreateAction(); + $user = auth()->user(); + + if (! $tenant instanceof Tenant) { + return true; + } + + if (! $user instanceof User) { + return false; + } + + return $resolver->isMember($user, $tenant); + }) + ->disabled(function () use ($resolver): bool { + $tenant = $this->resolveTenantForCreateAction(); + $user = auth()->user(); + + if (! $tenant instanceof Tenant) { + return true; + } + + if (! $user instanceof User) { + return true; + } + + if (! $resolver->isMember($user, $tenant)) { + return true; + } + + return ! $resolver->can($user, $tenant, Capabilities::PROVIDER_MANAGE); + }) + ->tooltip(function () use ($resolver): ?string { + $tenant = $this->resolveTenantForCreateAction(); + + if (! $tenant instanceof Tenant) { + return 'Select a tenant to create provider connections.'; + } + + $user = auth()->user(); + + if (! $user instanceof User) { + return null; + } + + if (! $resolver->isMember($user, $tenant)) { + return null; + } + + if (! $resolver->can($user, $tenant, Capabilities::PROVIDER_MANAGE)) { + return 'You do not have permission to create provider connections.'; + } + + return null; + }) + ->authorize(fn (): bool => true), ]; } + private function resolveTenantExternalIdForCreateAction(): ?string + { + $filterValue = data_get($this->getTableFilterState('tenant'), 'value'); + + if (is_string($filterValue) && $filterValue !== '') { + return $filterValue; + } + + $requested = ProviderConnectionResource::resolveRequestedTenantExternalId() + ?? ProviderConnectionResource::resolveContextTenantExternalId(); + + if (is_string($requested) && $requested !== '') { + return $requested; + } + + $filamentTenant = Filament::getTenant(); + + return $filamentTenant instanceof Tenant ? (string) $filamentTenant->external_id : null; + } + + private function resolveTenantForCreateAction(): ?Tenant + { + $tenantExternalId = $this->resolveTenantExternalIdForCreateAction(); + + if (! is_string($tenantExternalId) || $tenantExternalId === '') { + return null; + } + + return Tenant::query() + ->where('external_id', $tenantExternalId) + ->first(); + } + public function getTableEmptyStateHeading(): ?string { return 'No provider connections found'; diff --git a/app/Filament/Resources/TenantResource.php b/app/Filament/Resources/TenantResource.php index c40c0f8..27efc67 100644 --- a/app/Filament/Resources/TenantResource.php +++ b/app/Filament/Resources/TenantResource.php @@ -33,10 +33,15 @@ use App\Support\OpsUx\OperationUxPresenter; use App\Support\OpsUx\OpsUxBrowserEvents; use App\Support\Rbac\UiEnforcement; +use App\Support\Ui\ActionSurface\ActionSurfaceDeclaration; +use App\Support\Ui\ActionSurface\Enums\ActionSurfaceInspectAffordance; +use App\Support\Ui\ActionSurface\Enums\ActionSurfaceProfile; +use App\Support\Ui\ActionSurface\Enums\ActionSurfaceSlot; use App\Support\Workspaces\WorkspaceContext; use BackedEnum; use Filament\Actions; use Filament\Actions\ActionGroup; +use Filament\Actions\BulkActionGroup; use Filament\Forms; use Filament\Infolists; use Filament\Notifications\Notification; @@ -136,6 +141,17 @@ public static function canDeleteAny(): bool return static::userCanDeleteAnyTenant($user); } + public static function actionSurfaceDeclaration(): ActionSurfaceDeclaration + { + return ActionSurfaceDeclaration::forResource(ActionSurfaceProfile::CrudListAndView) + ->satisfy(ActionSurfaceSlot::ListHeader, 'List page provides a capability-gated create action.') + ->satisfy(ActionSurfaceSlot::InspectAffordance, ActionSurfaceInspectAffordance::ViewAction->value) + ->satisfy(ActionSurfaceSlot::ListRowMoreMenu, 'Row-level secondary actions are grouped under "More".') + ->satisfy(ActionSurfaceSlot::ListBulkMoreGroup, 'Bulk actions are grouped under "More".') + ->satisfy(ActionSurfaceSlot::ListEmptyState, 'Create action is reused in the list empty state.') + ->satisfy(ActionSurfaceSlot::DetailHeader, 'Tenant view page exposes header actions via an action group.'); + } + private static function userCanManageAnyTenant(User $user): bool { return $user->tenantMemberships() @@ -681,127 +697,132 @@ public static function table(Table $table): Table ->preserveVisibility() ->requireCapability(Capabilities::TENANT_DELETE) ->apply(), - ]), + ]) + ->label('More') + ->icon('heroicon-o-ellipsis-vertical') + ->color('gray'), ]) ->bulkActions([ - Actions\BulkAction::make('syncSelected') - ->label('Sync selected') - ->icon('heroicon-o-arrow-path') - ->color('warning') - ->requiresConfirmation() - ->visible(fn (): bool => auth()->user() instanceof User) - ->authorize(fn (): bool => auth()->user() instanceof User) - ->disabled(function (Collection $records): bool { - $user = auth()->user(); + BulkActionGroup::make([ + Actions\BulkAction::make('syncSelected') + ->label('Sync selected') + ->icon('heroicon-o-arrow-path') + ->color('warning') + ->requiresConfirmation() + ->visible(fn (): bool => auth()->user() instanceof User) + ->authorize(fn (): bool => auth()->user() instanceof User) + ->disabled(function (Collection $records): bool { + $user = auth()->user(); - if (! $user instanceof User) { - return true; - } + if (! $user instanceof User) { + return true; + } - if ($records->isEmpty()) { - return true; - } + if ($records->isEmpty()) { + return true; + } - /** @var CapabilityResolver $resolver */ - $resolver = app(CapabilityResolver::class); + /** @var CapabilityResolver $resolver */ + $resolver = app(CapabilityResolver::class); - return $records - ->filter(fn ($record) => $record instanceof Tenant) - ->contains(fn (Tenant $tenant): bool => ! $resolver->can($user, $tenant, Capabilities::TENANT_SYNC)); - }) - ->tooltip(function (Collection $records): ?string { - $user = auth()->user(); + return $records + ->filter(fn ($record) => $record instanceof Tenant) + ->contains(fn (Tenant $tenant): bool => ! $resolver->can($user, $tenant, Capabilities::TENANT_SYNC)); + }) + ->tooltip(function (Collection $records): ?string { + $user = auth()->user(); - if (! $user instanceof User) { - return UiTooltips::insufficientPermission(); - } + if (! $user instanceof User) { + return UiTooltips::insufficientPermission(); + } - if ($records->isEmpty()) { - return null; - } + if ($records->isEmpty()) { + return null; + } - /** @var CapabilityResolver $resolver */ - $resolver = app(CapabilityResolver::class); + /** @var CapabilityResolver $resolver */ + $resolver = app(CapabilityResolver::class); - $isDenied = $records - ->filter(fn ($record) => $record instanceof Tenant) - ->contains(fn (Tenant $tenant): bool => ! $resolver->can($user, $tenant, Capabilities::TENANT_SYNC)); + $isDenied = $records + ->filter(fn ($record) => $record instanceof Tenant) + ->contains(fn (Tenant $tenant): bool => ! $resolver->can($user, $tenant, Capabilities::TENANT_SYNC)); - return $isDenied ? UiTooltips::insufficientPermission() : null; - }) - ->action(function (Collection $records, AuditLogger $auditLogger): void { - $user = auth()->user(); + return $isDenied ? UiTooltips::insufficientPermission() : null; + }) + ->action(function (Collection $records, AuditLogger $auditLogger): void { + $user = auth()->user(); - if (! $user instanceof User) { - return; - } + if (! $user instanceof User) { + return; + } - /** @var CapabilityResolver $resolver */ - $resolver = app(CapabilityResolver::class); + /** @var CapabilityResolver $resolver */ + $resolver = app(CapabilityResolver::class); - $eligible = $records - ->filter(fn ($record) => $record instanceof Tenant && $record->isActive()) - ->filter(fn (Tenant $tenant) => $resolver->can($user, $tenant, Capabilities::TENANT_SYNC)); + $eligible = $records + ->filter(fn ($record) => $record instanceof Tenant && $record->isActive()) + ->filter(fn (Tenant $tenant) => $resolver->can($user, $tenant, Capabilities::TENANT_SYNC)); - if ($eligible->isEmpty()) { - Notification::make() - ->title('Bulk sync skipped') - ->body('No eligible tenants selected.') - ->icon('heroicon-o-information-circle') - ->info() - ->sendToDatabase($user) + if ($eligible->isEmpty()) { + Notification::make() + ->title('Bulk sync skipped') + ->body('No eligible tenants selected.') + ->icon('heroicon-o-information-circle') + ->info() + ->sendToDatabase($user) + ->send(); + + return; + } + + $tenantContext = Tenant::current() ?? $eligible->first(); + + if (! $tenantContext) { + return; + } + + $ids = $eligible->pluck('id')->toArray(); + $count = $eligible->count(); + + /** @var BulkSelectionIdentity $selection */ + $selection = app(BulkSelectionIdentity::class); + $selectionIdentity = $selection->fromIds($ids); + + /** @var OperationRunService $runs */ + $runs = app(OperationRunService::class); + + $opRun = $runs->enqueueBulkOperation( + tenant: $tenantContext, + type: 'tenant.sync', + targetScope: [ + 'entra_tenant_id' => (string) ($tenantContext->tenant_id ?? $tenantContext->external_id), + ], + selectionIdentity: $selectionIdentity, + dispatcher: function ($operationRun) use ($tenantContext, $user, $ids): void { + BulkTenantSyncJob::dispatch( + tenantId: (int) $tenantContext->getKey(), + userId: (int) $user->getKey(), + tenantIds: $ids, + operationRun: $operationRun, + ); + }, + initiator: $user, + extraContext: [ + 'tenant_count' => $count, + ], + emitQueuedNotification: false, + ); + + OperationUxPresenter::queuedToast('tenant.sync') + ->actions([ + Actions\Action::make('view_run') + ->label('View run') + ->url(OperationRunLinks::view($opRun, $tenantContext)), + ]) ->send(); - - return; - } - - $tenantContext = Tenant::current() ?? $eligible->first(); - - if (! $tenantContext) { - return; - } - - $ids = $eligible->pluck('id')->toArray(); - $count = $eligible->count(); - - /** @var BulkSelectionIdentity $selection */ - $selection = app(BulkSelectionIdentity::class); - $selectionIdentity = $selection->fromIds($ids); - - /** @var OperationRunService $runs */ - $runs = app(OperationRunService::class); - - $opRun = $runs->enqueueBulkOperation( - tenant: $tenantContext, - type: 'tenant.sync', - targetScope: [ - 'entra_tenant_id' => (string) ($tenantContext->tenant_id ?? $tenantContext->external_id), - ], - selectionIdentity: $selectionIdentity, - dispatcher: function ($operationRun) use ($tenantContext, $user, $ids): void { - BulkTenantSyncJob::dispatch( - tenantId: (int) $tenantContext->getKey(), - userId: (int) $user->getKey(), - tenantIds: $ids, - operationRun: $operationRun, - ); - }, - initiator: $user, - extraContext: [ - 'tenant_count' => $count, - ], - emitQueuedNotification: false, - ); - - OperationUxPresenter::queuedToast('tenant.sync') - ->actions([ - Actions\Action::make('view_run') - ->label('View run') - ->url(OperationRunLinks::view($opRun, $tenantContext)), - ]) - ->send(); - }) - ->deselectRecordsAfterCompletion(), + }) + ->deselectRecordsAfterCompletion(), + ])->label('More'), ]) ->headerActions([]); } diff --git a/app/Filament/Resources/TenantResource/Pages/ListTenants.php b/app/Filament/Resources/TenantResource/Pages/ListTenants.php index 48fcad2..f532cb3 100644 --- a/app/Filament/Resources/TenantResource/Pages/ListTenants.php +++ b/app/Filament/Resources/TenantResource/Pages/ListTenants.php @@ -18,4 +18,9 @@ protected function getHeaderActions(): array ->tooltip(fn (): ?string => TenantResource::canCreate() ? null : 'You do not have permission to register tenants.'), ]; } + + protected function getTableEmptyStateActions(): array + { + return $this->getHeaderActions(); + } } diff --git a/app/Filament/Resources/Workspaces/Pages/CreateWorkspace.php b/app/Filament/Resources/Workspaces/Pages/CreateWorkspace.php index 7b1a167..1fe51fa 100644 --- a/app/Filament/Resources/Workspaces/Pages/CreateWorkspace.php +++ b/app/Filament/Resources/Workspaces/Pages/CreateWorkspace.php @@ -5,6 +5,7 @@ use App\Filament\Resources\Workspaces\WorkspaceResource; use App\Models\User; use App\Models\WorkspaceMembership; +use App\Services\Audit\WorkspaceAuditLogger; use App\Support\Workspaces\WorkspaceContext; use Filament\Resources\Pages\CreateRecord; @@ -30,6 +31,20 @@ protected function afterCreate(): void ], ); + app(WorkspaceAuditLogger::class)->log( + workspace: $this->record, + action: 'workspace.created', + actor: $user, + resourceType: 'workspace', + resourceId: (string) $this->record->getKey(), + context: [ + 'metadata' => [ + 'workspace_id' => (int) $this->record->getKey(), + 'slug' => (string) $this->record->slug, + ], + ], + ); + app(WorkspaceContext::class)->setCurrentWorkspace($this->record, $user, request()); } } diff --git a/app/Filament/Resources/Workspaces/Pages/EditWorkspace.php b/app/Filament/Resources/Workspaces/Pages/EditWorkspace.php index 3d0763a..60124f6 100644 --- a/app/Filament/Resources/Workspaces/Pages/EditWorkspace.php +++ b/app/Filament/Resources/Workspaces/Pages/EditWorkspace.php @@ -3,9 +3,30 @@ namespace App\Filament\Resources\Workspaces\Pages; use App\Filament\Resources\Workspaces\WorkspaceResource; +use App\Models\User; +use App\Services\Audit\WorkspaceAuditLogger; use Filament\Resources\Pages\EditRecord; class EditWorkspace extends EditRecord { protected static string $resource = WorkspaceResource::class; + + protected function afterSave(): void + { + $user = auth()->user(); + + app(WorkspaceAuditLogger::class)->log( + workspace: $this->record, + action: 'workspace.updated', + actor: $user instanceof User ? $user : null, + resourceType: 'workspace', + resourceId: (string) $this->record->getKey(), + context: [ + 'metadata' => [ + 'workspace_id' => (int) $this->record->getKey(), + 'slug' => (string) $this->record->slug, + ], + ], + ); + } } diff --git a/app/Filament/Resources/Workspaces/Pages/ListWorkspaces.php b/app/Filament/Resources/Workspaces/Pages/ListWorkspaces.php index d60b41f..a454a9c 100644 --- a/app/Filament/Resources/Workspaces/Pages/ListWorkspaces.php +++ b/app/Filament/Resources/Workspaces/Pages/ListWorkspaces.php @@ -12,8 +12,21 @@ class ListWorkspaces extends ListRecords protected function getHeaderActions(): array { - return [ - Actions\CreateAction::make(), - ]; + return [$this->makeCreateAction()]; + } + + protected function getTableEmptyStateActions(): array + { + return [$this->makeCreateAction()]; + } + + private function makeCreateAction(): Actions\CreateAction + { + return Actions\CreateAction::make() + ->label('New workspace') + ->disabled(fn (): bool => ! WorkspaceResource::canCreate()) + ->tooltip(fn (): ?string => WorkspaceResource::canCreate() + ? null + : 'You do not have permission to create workspaces.'); } } diff --git a/app/Filament/Resources/Workspaces/Pages/ViewWorkspace.php b/app/Filament/Resources/Workspaces/Pages/ViewWorkspace.php index aa29f70..4b4384c 100644 --- a/app/Filament/Resources/Workspaces/Pages/ViewWorkspace.php +++ b/app/Filament/Resources/Workspaces/Pages/ViewWorkspace.php @@ -3,6 +3,9 @@ namespace App\Filament\Resources\Workspaces\Pages; use App\Filament\Resources\Workspaces\WorkspaceResource; +use App\Models\Workspace; +use App\Support\Auth\Capabilities; +use App\Support\Rbac\WorkspaceUiEnforcement; use Filament\Actions; use Filament\Resources\Pages\ViewRecord; @@ -13,7 +16,12 @@ class ViewWorkspace extends ViewRecord protected function getHeaderActions(): array { return [ - Actions\EditAction::make(), + WorkspaceUiEnforcement::forTableAction( + Actions\EditAction::make(), + fn (): ?Workspace => $this->record, + ) + ->requireCapability(Capabilities::WORKSPACE_MANAGE) + ->apply(), ]; } } diff --git a/app/Filament/Resources/Workspaces/WorkspaceResource.php b/app/Filament/Resources/Workspaces/WorkspaceResource.php index 9885c80..690509e 100644 --- a/app/Filament/Resources/Workspaces/WorkspaceResource.php +++ b/app/Filament/Resources/Workspaces/WorkspaceResource.php @@ -5,6 +5,14 @@ use App\Filament\Resources\Workspaces\RelationManagers\WorkspaceMembershipsRelationManager; use App\Models\User; use App\Models\Workspace; +use App\Services\Auth\WorkspaceCapabilityResolver; +use App\Support\Auth\Capabilities; +use App\Support\Rbac\WorkspaceUiEnforcement; +use App\Support\Ui\ActionSurface\ActionSurfaceDeclaration; +use App\Support\Ui\ActionSurface\Enums\ActionSurfaceInspectAffordance; +use App\Support\Ui\ActionSurface\Enums\ActionSurfaceProfile; +use App\Support\Ui\ActionSurface\Enums\ActionSurfaceSlot; +use App\Support\Workspaces\WorkspaceContext; use BackedEnum; use Filament\Actions; use Filament\Forms; @@ -13,6 +21,7 @@ use Filament\Tables; use Filament\Tables\Table; use Illuminate\Database\Eloquent\Builder; +use Illuminate\Database\Eloquent\Model; use UnitEnum; class WorkspaceResource extends Resource @@ -33,6 +42,67 @@ class WorkspaceResource extends Resource protected static string|UnitEnum|null $navigationGroup = 'Settings'; + public static function canCreate(): bool + { + $user = auth()->user(); + + if (! $user instanceof User) { + return false; + } + + /** @var WorkspaceCapabilityResolver $resolver */ + $resolver = app(WorkspaceCapabilityResolver::class); + + $workspaceId = app(WorkspaceContext::class)->currentWorkspaceId(request()); + + if (is_int($workspaceId)) { + $workspace = Workspace::query()->whereKey($workspaceId)->first(); + + if ($workspace instanceof Workspace) { + return $resolver->isMember($user, $workspace) + && $resolver->can($user, $workspace, Capabilities::WORKSPACE_MANAGE); + } + } + + foreach ($user->workspaces()->get() as $workspace) { + if (! $workspace instanceof Workspace) { + continue; + } + + if ($resolver->can($user, $workspace, Capabilities::WORKSPACE_MANAGE)) { + return true; + } + } + + return false; + } + + public static function canEdit(Model $record): bool + { + $user = auth()->user(); + + if (! $user instanceof User || ! $record instanceof Workspace) { + return false; + } + + /** @var WorkspaceCapabilityResolver $resolver */ + $resolver = app(WorkspaceCapabilityResolver::class); + + return $resolver->isMember($user, $record) + && $resolver->can($user, $record, Capabilities::WORKSPACE_MANAGE); + } + + public static function actionSurfaceDeclaration(): ActionSurfaceDeclaration + { + return ActionSurfaceDeclaration::forResource(ActionSurfaceProfile::CrudListAndView) + ->satisfy(ActionSurfaceSlot::ListHeader, 'List page provides a capability-gated create action.') + ->satisfy(ActionSurfaceSlot::InspectAffordance, ActionSurfaceInspectAffordance::ViewAction->value) + ->exempt(ActionSurfaceSlot::ListRowMoreMenu, 'Workspace list intentionally uses only primary View/Edit row actions.') + ->exempt(ActionSurfaceSlot::ListBulkMoreGroup, 'Workspace list intentionally omits bulk actions.') + ->satisfy(ActionSurfaceSlot::ListEmptyState, 'List page defines a capability-gated empty-state create CTA.') + ->satisfy(ActionSurfaceSlot::DetailHeader, 'Workspace view page exposes a capability-gated edit action.'); + } + public static function getEloquentQuery(): Builder { $query = parent::getEloquentQuery(); @@ -79,7 +149,12 @@ public static function table(Table $table): Table ]) ->actions([ Actions\ViewAction::make(), - Actions\EditAction::make(), + WorkspaceUiEnforcement::forTableAction( + Actions\EditAction::make(), + fn (): ?Workspace => null, + ) + ->requireCapability(Capabilities::WORKSPACE_MANAGE) + ->apply(), ]); } diff --git a/app/Support/Rbac/UiEnforcement.php b/app/Support/Rbac/UiEnforcement.php index e8daa36..a1ef87d 100644 --- a/app/Support/Rbac/UiEnforcement.php +++ b/app/Support/Rbac/UiEnforcement.php @@ -398,6 +398,16 @@ private function resolveTenantWithRecord(?Model $record = null): ?Tenant return $record; } + // If a record has an eagerly-loaded `tenant` relation, prefer it. + // This avoids relying on Filament::getTenant() for list pages that are not tenant-scoped. + if ($record instanceof Model && method_exists($record, 'relationLoaded') && $record->relationLoaded('tenant')) { + $relatedTenant = $record->getRelation('tenant'); + + if ($relatedTenant instanceof Tenant) { + return $relatedTenant; + } + } + // If a record is set from forTableAction, try to resolve it if ($this->record !== null) { $resolved = $this->record instanceof Closure diff --git a/app/Support/Ui/ActionSurface/ActionSurfaceExemptions.php b/app/Support/Ui/ActionSurface/ActionSurfaceExemptions.php index dc81d40..b8a7901 100644 --- a/app/Support/Ui/ActionSurface/ActionSurfaceExemptions.php +++ b/app/Support/Ui/ActionSurface/ActionSurfaceExemptions.php @@ -35,16 +35,11 @@ public static function baseline(): self 'App\\Filament\\Pages\\TenantRequiredPermissions' => 'Permissions page retrofit deferred; capability checks already enforced by dedicated tests.', 'App\\Filament\\Pages\\Workspaces\\ManagedTenantOnboardingWizard' => 'Onboarding wizard has dedicated conformance tests and remains exempt in spec 082.', 'App\\Filament\\Pages\\Workspaces\\ManagedTenantsLanding' => 'Managed-tenant landing retrofit deferred to workspace feature track.', - 'App\\Filament\\Resources\\BackupScheduleResource' => 'Backup schedule resource retrofit deferred to backup scheduling track.', 'App\\Filament\\Resources\\BackupSetResource' => 'Backup set resource retrofit deferred to backup set track.', 'App\\Filament\\Resources\\BackupSetResource\\RelationManagers\\BackupItemsRelationManager' => 'Backup items relation manager retrofit deferred to backup set track.', - 'App\\Filament\\Resources\\FindingResource' => 'Finding resource retrofit deferred to drift track.', - 'App\\Filament\\Resources\\ProviderConnectionResource' => 'Provider connection resource retrofit deferred to provider-connection track.', 'App\\Filament\\Resources\\RestoreRunResource' => 'Restore run resource retrofit deferred to restore track.', - 'App\\Filament\\Resources\\TenantResource' => 'Tenant resource retrofit deferred to tenant administration track.', 'App\\Filament\\Resources\\TenantResource\\RelationManagers\\TenantMembershipsRelationManager' => 'Tenant memberships relation manager retrofit deferred to RBAC membership track.', 'App\\Filament\\Resources\\Workspaces\\RelationManagers\\WorkspaceMembershipsRelationManager' => 'Workspace memberships relation manager retrofit deferred to workspace RBAC track.', - 'App\\Filament\\Resources\\Workspaces\\WorkspaceResource' => 'Workspace resource retrofit deferred to workspace management track.', ]); } diff --git a/specs/090-action-surface-contract-compliance/checklists/requirements.md b/specs/090-action-surface-contract-compliance/checklists/requirements.md new file mode 100644 index 0000000..8b49c2a --- /dev/null +++ b/specs/090-action-surface-contract-compliance/checklists/requirements.md @@ -0,0 +1,35 @@ +# Specification Quality Checklist: Action Surface Contract Compliance & RBAC Hardening + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-02-12 +**Feature**: [specs/090-action-surface-contract-compliance/spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- All checks pass for planning readiness. +- The spec includes governance-required references to "Filament Action Surfaces" only in the constitution alignment section; no code-level or framework-specific implementation guidance is provided. diff --git a/specs/090-action-surface-contract-compliance/contracts/action-surface-contract-v1.openapi.yaml b/specs/090-action-surface-contract-compliance/contracts/action-surface-contract-v1.openapi.yaml new file mode 100644 index 0000000..f987312 --- /dev/null +++ b/specs/090-action-surface-contract-compliance/contracts/action-surface-contract-v1.openapi.yaml @@ -0,0 +1,139 @@ +openapi: 3.1.0 +info: + title: TenantPilot Internal UI Contracts + version: 1.0.0 + description: | + This OpenAPI document is used as a schema bundle for internal (non-HTTP) contracts. + + Spec 090 does not introduce public HTTP endpoints; Filament/Livewire actions are + executed via framework internals. We still publish schemas here so the feature + has explicit, reviewable contracts under `specs/.../contracts/`. + +paths: {} + +components: + schemas: + ActionSurfaceSlot: + type: string + description: Named surface slot required by the constitution Action Surface Contract. + enum: + - list.header + - list.inspect_affordance + - list.row.more_menu + - list.bulk.more_group + - list.empty_state + - detail.header + + ActionSurfaceInspectAffordance: + type: string + description: Accepted inspection affordances for a record list/table. + enum: + - clickable_row + - view_action + - primary_linked_column + + ActionSurfaceProfile: + type: string + description: Profile that determines which slots are required. + enum: + - crud_list_and_view + - list_only_read_only + - run_log + - relation_manager + + ActionSurfaceDeclaration: + type: object + additionalProperties: false + required: + - component + - profile + - slots + properties: + component: + type: string + description: Fully-qualified class name of the Filament Resource/Page/RelationManager. + examples: + - App\\Filament\\Resources\\PolicyResource + profile: + $ref: '#/components/schemas/ActionSurfaceProfile' + slots: + type: object + description: Declared satisfaction/exemption for each slot. + additionalProperties: + type: object + additionalProperties: false + properties: + status: + type: string + enum: [satisfy, exempt] + value: + type: string + description: Freeform value (e.g., inspect affordance type). + reason: + type: string + description: Non-empty exemption reason when status=exempt. + + RbacDecision: + type: object + additionalProperties: false + required: + - is_member + - has_capability + - outcome + properties: + is_member: + type: boolean + has_capability: + type: boolean + outcome: + type: string + description: Server-side outcome required by RBAC-UX. + enum: + - allow + - deny_as_not_found + - deny_as_forbidden + + AuditLogEntry: + type: object + additionalProperties: false + required: + - action + - status + - recorded_at + properties: + tenant_id: + type: integer + nullable: true + workspace_id: + type: integer + nullable: true + actor_id: + type: integer + nullable: true + actor_email: + type: string + nullable: true + actor_name: + type: string + nullable: true + action: + type: string + description: Stable action id string. + examples: + - policy.capture_snapshot_dispatched + resource_type: + type: string + nullable: true + resource_id: + type: string + nullable: true + status: + type: string + enum: [success] + metadata: + type: object + description: Sanitized metadata payload. + additionalProperties: true + recorded_at: + type: string + format: date-time diff --git a/specs/090-action-surface-contract-compliance/data-model.md b/specs/090-action-surface-contract-compliance/data-model.md new file mode 100644 index 0000000..b6962dd --- /dev/null +++ b/specs/090-action-surface-contract-compliance/data-model.md @@ -0,0 +1,59 @@ +# Data Model — Spec 090 (Action Surface Contract Compliance & RBAC Hardening) + +## Summary +Spec 090 is primarily a UI + authorization + auditability retrofit. **No new tables are required**. The feature standardizes how existing entities are presented and how actions are gated and audited. + +## Entities (Existing) + +### `AuditLog` +- **Purpose**: Durable audit trail for security/ops-relevant actions. +- **Key fields (observed from loggers)**: + - `tenant_id` (nullable) + - `workspace_id` (nullable) + - `actor_id`, `actor_email`, `actor_name` + - `action` (stable action id string) + - `resource_type`, `resource_id` + - `status` (`success` for Spec 090 scope) + - `metadata` (sanitized) + - `recorded_at` +- **Writers**: + - Tenant-scoped: `App\Services\Intune\AuditLogger` + - Workspace-scoped: `App\Services\Audit\WorkspaceAuditLogger` + +### `OperationRun` +- **Purpose**: Observability record for queued/remote operations. +- **Spec 090 usage**: Operation-start actions must enqueue work and link to the run (“View run”). + +### Tenant-scoped domain entities +- `Tenant` +- `Policy` / `PolicyVersion` +- `BackupSchedule` (+ executions via `OperationRun` type `backup_schedule_run`) +- `ProviderConnection` +- `Finding` +- `InventoryItem` +- `EntraGroup` + +### Workspace-scoped domain entities +- `Workspace` +- workspace memberships (used for isolation boundary) + +## Relationships (High-level) +- `Workspace` has many `Tenant`. +- `Tenant` has many `Policy`, `BackupSchedule`, `ProviderConnection`, `Finding`, `InventoryItem`, `EntraGroup`. +- `OperationRun` is scoped by `(workspace_id, tenant_id)` when applicable. +- `AuditLog` is scoped either by `tenant_id` (tenant-plane audits) or `workspace_id` (workspace-plane audits). + +## Authorization/Capability Model (Existing) +- Canonical capability registry: `App\Support\Auth\Capabilities`. +- Capability resolution: `App\Services\Auth\CapabilityResolver`. +- Filament gating helpers: + - `App\Support\Rbac\UiEnforcement` (tenant scope) + - `App\Support\Rbac\WorkspaceUiEnforcement` (workspace scope) + +## State Transitions (Relevant) +- “Side-effect action executed” → writes an `AuditLog` entry (success only for Spec 090). +- “Operation start action executed” → creates/reuses an `OperationRun`, enqueues background work, then links user to the run. + +## Non-goals / Deferred +- BackupSchedule retention (soft delete/restore/force delete) is explicitly deferred. +- No schema changes are required for action-surface declarations; these live in code. diff --git a/specs/090-action-surface-contract-compliance/plan.md b/specs/090-action-surface-contract-compliance/plan.md new file mode 100644 index 0000000..50e7e5d --- /dev/null +++ b/specs/090-action-surface-contract-compliance/plan.md @@ -0,0 +1,195 @@ +# Implementation Plan: Action Surface Contract Compliance & RBAC Hardening (Spec 090) + +**Branch**: `090-action-surface-contract-compliance` | **Date**: 2026-02-12 | **Spec**: `specs/090-action-surface-contract-compliance/spec.md` +**Input**: Feature specification from `specs/090-action-surface-contract-compliance/spec.md` + +## Summary + +Bring a focused set of Filament v5 action surfaces into compliance with the repo’s **Filament UI Action Surface Contract**, and harden RBAC semantics using the existing enforcement helpers. + +This work primarily: +- Shrinks `ActionSurfaceExemptions::baseline()` for the in-scope components. +- Adds `actionSurfaceDeclaration()` to those components (or explicit exemptions with reasons). +- Standardizes action ordering/grouping (“View → Edit → More → Destructive last”; max 2 visible row actions). +- Ensures **successful** side-effect actions create `AuditLog` entries. + +## Technical Context + +**Language/Version**: PHP 8.4.15 +**Framework**: Laravel 12 +**Admin UI**: Filament v5 + Livewire v4.0+ +**Storage**: PostgreSQL (Sail) +**Testing**: Pest v4 (`vendor/bin/sail artisan test`) +**Target Platform**: Docker/Sail local; Dokploy staging/prod +**Project Type**: Laravel monolith +**Performance Goals**: Keep “start” surfaces enqueue-only; preserve guard test runtime +**Constraints**: +- No new dependencies +- BackupSchedule retention is explicitly deferred +- Use existing RBAC + audit primitives; no new tables +**Scale/Scope**: Multi-tenant admin UI; targeted subset of Resources/Pages + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +- ✅ Filament v5 targets Livewire v4.0+ (no Livewire v3) +- ✅ Authorization is enforced server-side (UI visibility is not security) +- ✅ RBAC-UX semantics: non-member → 404; member missing capability → 403 (and disabled + tooltip in UI) +- ✅ Destructive actions execute via `->action(...)` and include `->requiresConfirmation()` +- ✅ Action Surface Contract enforced via guard tests; exemptions must be explicit and minimized +- ✅ Audit logs for successful side-effect actions (per spec clarification) + +## Project Structure + +### Documentation (this feature) + +```text +specs/090-action-surface-contract-compliance/ +├── spec.md +├── plan.md +├── research.md +├── data-model.md +├── quickstart.md +└── contracts/ + └── action-surface-contract-v1.openapi.yaml +``` + +### Source Code (relevant to this spec) + +```text +app/ +├── Filament/Resources/ +│ ├── ProviderConnectionResource.php +│ ├── BackupScheduleResource.php +│ ├── FindingResource.php +│ ├── TenantResource.php +│ ├── PolicyResource/Pages/ListPolicies.php +│ └── PolicyResource/Pages/ViewPolicy.php +├── Filament/Resources/Workspaces/WorkspaceResource.php +├── Support/Rbac/UiEnforcement.php +├── Support/Rbac/WorkspaceUiEnforcement.php +└── Support/Ui/ActionSurface/ + ├── ActionSurfaceDeclaration.php + └── ActionSurfaceExemptions.php + +tests/Feature/Guards/ +├── ActionSurfaceContractTest.php +└── ActionSurfaceValidatorTest.php +``` + +**Structure Decision**: Laravel monolith; modifications are limited to existing Filament resources/pages + guard tests. + +## Phase 0 — Outline & Research (DOCS COMPLETE) + +Completed in `specs/090-action-surface-contract-compliance/research.md`. + +Key findings: +- Action Surface Contract enforcement already exists via guard tests and `ActionSurfaceDeclaration`. +- Many in-scope resources are currently excluded by `ActionSurfaceExemptions::baseline()`. +- RBAC enforcement helpers exist and already implement 404/403 semantics: `UiEnforcement`, `WorkspaceUiEnforcement`. +- Audit logging services exist and sanitize metadata. + +## Phase 1 — Design & Contracts (DOCS COMPLETE) + +Outputs: +- `data-model.md`: No schema changes required. +- `contracts/action-surface-contract-v1.openapi.yaml`: Internal contract artifact (no new HTTP endpoints). +- `quickstart.md`: Local verification commands and checklist. + +Design choices: +- “View” requirement is interpreted as an **inspection affordance** (clickable row, primary linked column, or explicit View action) consistent with the constitution. +- Audit logs are required for successful dispatch/execution surfaces only. +- Directory groups (Entra groups) and inventory items already follow the inspection-affordance pattern (clickable rows) and have explicit declarations; they are verification-only in this spec to avoid expanding scope. + +Note: Phases 0–1 are complete for documentation and design artifacts. Implementation and verification work begins in Phase 2 and is tracked in `specs/090-action-surface-contract-compliance/tasks.md`. + +## Phase 2 — Implementation Plan + +### Step 1 — Make contract guard the primary gate + +Target guard tests: +- `tests/Feature/Guards/ActionSurfaceContractTest.php` +- `tests/Feature/Guards/ActionSurfaceValidatorTest.php` + +Workflow: +1) Add `actionSurfaceDeclaration()` to one component. +2) Remove its baseline exemption. +3) Run guard tests. +4) Repeat. + +### Step 2 — Shrink baseline exemptions + +File: `app/Support/Ui/ActionSurface/ActionSurfaceExemptions.php` + +Remove baseline exemptions for components once they have declarations: +- `ProviderConnectionResource` +- `BackupScheduleResource` +- `FindingResource` +- `TenantResource` +- `Workspaces/WorkspaceResource` + +### Step 3 — Add action surface declarations (per component) + +Add `actionSurfaceDeclaration()` and ensure required slots are satisfied. + +Expected profile direction (final selection should match existing profiles in the repo): +- CRUD list + inspect + edit: use a CRUD-capable profile; keep row actions to 2 max. +- List-only read-only resources should use a read-only profile (already in repo patterns). + +### Step 4 — Fix known hotspots + +#### (P1) Policy: Capture snapshot dispatch + +File: `app/Filament/Resources/PolicyResource/Pages/ViewPolicy.php` +- Wrap action with `UiEnforcement::forAction(...)` using the canonical capability. +- Ensure the server-side `before(...)` aborts with 404/403 appropriately. +- Add audit logging on successful dispatch via `App\Services\Intune\AuditLogger`. + +#### (P1) Policy: Sync action confirmation semantics + +File: `app/Filament/Resources/PolicyResource/Pages/ListPolicies.php` +- Remove misuse of `destructive()` for non-destructive sync/run actions. +- Keep confirmation if required, but with non-destructive semantics. + +#### (P2) Findings: action ordering + grouping + +File: `app/Filament/Resources/FindingResource.php` +- Ensure `View` is first. +- Move secondary actions (like acknowledge) into `More`. +- Ensure bulk actions are grouped and capability gated via `UiEnforcement`. + +#### (P2/P3) Backup schedules: empty-state CTA + grouping + +File: `app/Filament/Resources/BackupScheduleResource.php` +- Ensure empty-state action exists (create) and is capability gated. +- Ensure “run now / retry” actions are gated and audited (audit already exists for manual dispatch; verify consistency). + +#### (P1/P2) Tenant + Workspaces: RBAC semantics + bulk grouping + +Files: +- `app/Filament/Resources/TenantResource.php` +- `app/Filament/Resources/Workspaces/WorkspaceResource.php` + +Ensure: +- Destructive actions are last and confirmed. +- Bulk actions are grouped under a consistent `More` group. +- Workspaces use `WorkspaceUiEnforcement` + `WorkspaceAuditLogger` for workspace-scoped side effects. + +### Step 5 — Testing plan + +Minimum programmatic verification: +- Guard tests stay green as exemptions shrink. +- Add or extend focused Pest feature tests for: + - 404 vs 403 RBAC semantics for at least one representative mutation action. + - Successful audit log creation for at least one enqueue/dispatch surface. + +### Step 6 — Formatting + +Run formatter before final handoff: +- `vendor/bin/sail bin pint --dirty` + +## Deploy / Ops Notes + +- No migrations expected. +- No new Filament assets expected; if any assets are registered, ensure deploy runs `php artisan filament:assets`. diff --git a/specs/090-action-surface-contract-compliance/quickstart.md b/specs/090-action-surface-contract-compliance/quickstart.md new file mode 100644 index 0000000..6ed1388 --- /dev/null +++ b/specs/090-action-surface-contract-compliance/quickstart.md @@ -0,0 +1,31 @@ +# Quickstart — Spec 090 (Action Surface Contract Compliance & RBAC Hardening) + +## Prereqs +- Run inside Sail. + +## Run the guard tests (fast feedback) +- `vendor/bin/sail artisan test --compact tests/Feature/Guards/ActionSurfaceContractTest.php` + +## Run targeted RBAC/action tests (after implementation) +Planned additions for Spec 090 will include feature tests for: +- Policy “Capture snapshot” authorization + audit log +- Findings list action ordering + acknowledge gating +- Provider connections action surface + RBAC gating +- Backup schedules action surface + empty-state CTA gating +- Workspace resource access semantics (non-member 404, member missing capability 403) + +Run the smallest set first, e.g.: +- `vendor/bin/sail artisan test --compact --filter=ActionSurfaceContract` + +## Run only Spec 090 tests +- `vendor/bin/sail artisan test --compact tests/Feature/090/` +- `vendor/bin/sail artisan test --compact --filter=Spec090` + +## Formatting +- `vendor/bin/sail bin pint --dirty` + +## Manual verification checklist (post-implementation) +- Confirm each in-scope list/table provides an inspection affordance (View action *or* clickable row/primary link), consistent “More” grouping, and ≤2 primary row actions. +- Confirm destructive actions require confirmation. +- Confirm tenant/workspace isolation: non-members get 404 semantics; members without capability get 403 on execution and disabled + tooltip in UI. +- Confirm successful side-effect actions create an `audit_logs` entry with sanitized metadata. diff --git a/specs/090-action-surface-contract-compliance/research.md b/specs/090-action-surface-contract-compliance/research.md new file mode 100644 index 0000000..5340add --- /dev/null +++ b/specs/090-action-surface-contract-compliance/research.md @@ -0,0 +1,84 @@ +# Research — Spec 090 (Action Surface Contract Compliance & RBAC Hardening) + +## Decisions + +### Decision 1 — Use the repo’s existing Action Surface Contract guard +- **Chosen**: Retrofit in-scope Filament Resources/Pages/RelationManagers by adding `actionSurfaceDeclaration()` and removing baseline exemptions in `App\Support\Ui\ActionSurface\ActionSurfaceExemptions::baseline()`. +- **Rationale**: The repository already enforces the constitution’s Filament Action Surface Contract through `tests/Feature/Guards/ActionSurfaceContractTest.php`. Aligning to that system avoids bespoke checks and keeps CI gates meaningful. +- **Alternatives considered**: + - Ad-hoc “View button exists” UI assertions per resource → rejected (doesn’t cover grouping/bulk/empty-state semantics; duplicates existing guard). + +### Decision 2 — Interpret “View affordance” as an *inspection affordance* (not always a literal View button) +- **Chosen**: Treat the spec’s “visible View” requirement as “a visible inspection affordance”, matching the constitution’s accepted forms: + - clickable rows via `recordUrl()` (preferred when View would be the only row action) + - a primary linked column + - a dedicated `View` row action (when there are other row actions like Edit) +- **Rationale**: The constitution explicitly forbids a lone View row button and prefers clickable rows in that case. The repo already uses this approach for read-only, list-only surfaces (e.g., inventory items). +- **Alternatives considered**: + - Add `ViewAction` everywhere → rejected (would create “lone View” buttons on read-only lists, conflicting with constitution and existing tests). + +### Decision 3 — Reuse central RBAC UI enforcement helpers for all Filament actions +- **Chosen**: For tenant-scoped actions, use `App\Support\Rbac\UiEnforcement`. + For workspace-scoped actions, use `App\Support\Rbac\WorkspaceUiEnforcement`. +- **Rationale**: These helpers implement the constitution’s RBAC-UX semantics (non-member 404, member missing capability 403, disabled + tooltip in UI) and validate capabilities against the canonical registry. +- **Alternatives considered**: + - Inline `->authorize(...)`, ad-hoc `->visible(...)`, or `->disabled(...)` checks → rejected (too easy to drift; violates “central helpers” expectation). + +### Decision 4 — Audit successful side-effect actions via existing audit log services +- **Chosen**: + - Tenant-scoped audit entries: `App\Services\Intune\AuditLogger` (writes to `audit_logs` with `tenant_id`). + - Workspace-scoped audit entries: `App\Services\Audit\WorkspaceAuditLogger` (writes to `audit_logs` with `workspace_id`, `tenant_id = null`). +- **Rationale**: The repo already uses these loggers and sanitizes metadata via `AuditContextSanitizer`. +- **Alternatives considered**: + - Logging denied/cancelled attempts → rejected (explicitly out of scope for Spec 090). + +## Current Repo State (Key Findings) + +### Action Surface Contract infrastructure exists +- Guard tests: + - `tests/Feature/Guards/ActionSurfaceContractTest.php` + - `tests/Feature/Guards/ActionSurfaceValidatorTest.php` +- Runtime support: + - `app/Support/Ui/ActionSurface/*` +- Many “legacy” resources are exempt via: + - `app/Support/Ui/ActionSurface/ActionSurfaceExemptions.php` + +### In-scope components are currently exempt from the contract guard +The constitution guard currently exempts (among others): +- `App\Filament\Resources\ProviderConnectionResource` +- `App\Filament\Resources\BackupScheduleResource` +- `App\Filament\Resources\FindingResource` +- `App\Filament\Resources\TenantResource` +- `App\Filament\Resources\Workspaces\WorkspaceResource` + +Exact baseline exemptions to remove in Spec 090 implementation: +- `App\Filament\Resources\ProviderConnectionResource` +- `App\Filament\Resources\BackupScheduleResource` +- `App\Filament\Resources\FindingResource` +- `App\Filament\Resources\TenantResource` +- `App\Filament\Resources\Workspaces\WorkspaceResource` + +Spec 090’s intent is to shrink that exemption list by bringing these in-scope surfaces into compliance. + +### RBAC helpers are present and already used on some surfaces +- `UiEnforcement` provides: + - non-member hidden + server abort(404) + - member missing capability disabled + tooltip + server abort(403) + - destructive confirmation via `->requiresConfirmation()` (generic destructive copy) + +### Notable hotspot: Policy “Capture snapshot” +- Implemented in `app/Filament/Resources/PolicyResource/Pages/ViewPolicy.php`. +- Currently: + - has confirmation + correct domain copy + - queues an `OperationRun` + - **needs** capability-first gating and audit logging for successful dispatch. + +### Notable semantics mismatch: Policy list “Sync from Intune” +- Implemented in `app/Filament/Resources/PolicyResource/Pages/ListPolicies.php`. +- The action uses `UiEnforcement::destructive()` to obtain confirmation, which applies generic destructive modal copy. +- Per Spec 090 + constitution: sync/run actions should not be presented as destructive. + +## Open Questions Resolved By Repo Evidence +- **How is action-surface compliance enforced?** → Via `ActionSurfaceValidator` + `ActionSurfaceContractTest`. +- **How to log audits?** → Use `AuditLogger` / `WorkspaceAuditLogger` with sanitized metadata. +- **How to apply RBAC consistently?** → Wrap Filament actions with `UiEnforcement` / `WorkspaceUiEnforcement`. diff --git a/specs/090-action-surface-contract-compliance/spec.md b/specs/090-action-surface-contract-compliance/spec.md new file mode 100644 index 0000000..f856751 --- /dev/null +++ b/specs/090-action-surface-contract-compliance/spec.md @@ -0,0 +1,222 @@ +# Feature Specification: Action Surface Contract Compliance & RBAC Hardening + +**Feature Branch**: `090-action-surface-contract-compliance` +**Created**: 2026-02-12 +**Status**: Draft +**Input**: User description: "Action Surface Contract Compliance & RBAC Hardening" + +## Clarifications + +### Session 2026-02-12 + +- Q: Should BackupSchedule retention (soft delete / restore / force delete) be included in Spec 090? → A: Defer it to a separate follow-up spec (Spec 090 covers only P0/P1 + empty-state CTAs). +- Q: Should the system record audit trail entries for denied/cancelled attempts, or only for successful executions? → A: Audit successful executions only. +- Q: Should row-click navigation be removed, or kept while adding an inspection affordance? → A: Keep row-click navigation where it exists; the inspection affordance may be clickable rows, a primary linked column, or a dedicated “View” action (avoid rendering a lone “View” row action button on read-only lists). + +## User Scenarios & Testing *(mandatory)* + + + +### User Story 1 - Safe admin actions (Priority: P1) + +As an admin, I can only execute actions that create changes or side-effects when I am entitled to the scope (tenant/workspace) and have the required capability. + +**Why this priority**: Prevents unauthorized changes and accidental destructive operations; closes RBAC gaps. + +**Independent Test**: Can be fully tested by attempting a small set of in-scope actions as (a) non-member, (b) member without capability, (c) member with capability. + +**Acceptance Scenarios**: + +1. **Given** I am not a member of the tenant/workspace scope, **When** I try to access pages or execute actions in that scope, **Then** I receive a not found result (deny-as-not-found). +2. **Given** I am a member of the tenant/workspace scope but I lack the capability for an action with side-effects, **When** I attempt the action, **Then** the UI communicates it is unavailable and direct execution is denied. +3. **Given** I am a member of the tenant/workspace scope and I have the capability, **When** I execute the action, **Then** the action completes and an audit trail entry is recorded. + +--- + +### User Story 2 - Consistent action surfaces (Priority: P2) + +As an admin, I can reliably discover how to inspect and manage records across the admin panel because each list/table provides a visible inspection affordance and consistent action grouping and ordering. + +**Why this priority**: Improves enterprise UX consistency, accessibility, and reduces operator error. + +**Independent Test**: Can be tested by reviewing targeted list/table screens for a clear inspection affordance, consistent ordering, and consistent “More” grouping. + +**Acceptance Scenarios**: + +1. **Given** a record list/table for an in-scope resource, **When** I view the table’s inspection affordance, **Then** I can navigate to a record detail page using at least one of: clickable row navigation, a primary linked column, or a dedicated “View” action. +2. **Given** a record list/table has row-click navigation, **When** I cannot (or prefer not to) use row-click navigation, **Then** I can still navigate to record details using a visible alternative inspection affordance (a primary linked column and/or a dedicated “View” action). +3. **Given** a list/table is read-only and “View” would be the only row action, **When** the UI is rendered, **Then** it SHOULD avoid rendering a lone “View” row action button and instead use clickable rows or a primary linked column. + +--- + +### User Story 3 - Productive empty states (Priority: P3) + +As an admin, when a resource has no records yet, the empty state clearly indicates what to do next and offers a “New …” call-to-action when I am allowed to create records. + +**Why this priority**: Reduces onboarding friction and “dead-end” screens. + +**Independent Test**: Can be tested by visiting empty list/table screens for user-created resources and verifying the create call-to-action is present only when permitted. + +**Acceptance Scenarios**: + +1. **Given** there are no workspaces yet, **When** I open the workspaces list, **Then** I see a create call-to-action if I am allowed to create. +2. **Given** there are no backup schedules yet, **When** I open the backup schedules list, **Then** I see a create call-to-action if I am allowed to create. + +--- + +### Edge Cases + +- A user attempts to execute a side-effect action via a direct request while the UI action is disabled. +- Bulk actions: mixed selection where some records are outside scope or not authorized. +- A destructive action is triggered unintentionally; confirmation must prevent accidental execution. +- Accessibility: users who cannot use row-click navigation must still discover a usable inspection affordance. +- Two admins attempt the same side-effect action concurrently; the system must behave predictably (no partial execution without an audit trail). + +## Requirements *(mandatory)* + +**Constitution alignment (required):** If this feature introduces any Microsoft Graph calls, any write/change behavior, +or any long-running/queued/scheduled work, the spec MUST describe contract registry updates, safety gates +(preview/confirmation/audit), tenant isolation, run observability (`OperationRun` type/identity/visibility), and tests. +If security-relevant DB-only actions intentionally skip `OperationRun`, the spec MUST describe `AuditLog` entries. + +**Constitution alignment (RBAC-UX):** If this feature introduces or changes authorization behavior, the spec MUST: +- state which authorization plane(s) are involved (tenant `/admin/t/{tenant}` vs platform `/system`), +- ensure any cross-plane access is deny-as-not-found (404), +- explicitly define 404 vs 403 semantics: + - non-member / not entitled to workspace scope OR tenant scope → 404 (deny-as-not-found) + - member but missing capability → 403 +- describe how authorization is enforced server-side (Gates/Policies) for every mutation/operation-start/credential change, +- reference the canonical capability registry (no raw capability strings; no role-string checks in feature code), +- ensure global search is tenant-scoped and non-member-safe (no hints; inaccessible results treated as 404 semantics), +- ensure destructive-like actions require confirmation (`->requiresConfirmation()`), +- include at least one positive and one negative authorization test, and note any RBAC regression tests added/updated. + +**Constitution alignment (OPS-EX-AUTH-001):** OIDC/SAML login handshakes may perform synchronous outbound HTTP (e.g., token exchange) +on `/auth/*` endpoints without an `OperationRun`. This MUST NOT be used for Monitoring/Operations pages. + +**Constitution alignment (BADGE-001):** If this feature changes status-like badges (status/outcome/severity/risk/availability/boolean), +the spec MUST describe how badge semantics stay centralized (no ad-hoc mappings) and which tests cover any new/changed values. + +**Constitution alignment (Filament Action Surfaces):** If this feature adds or modifies any Filament Resource / RelationManager / Page, +the spec MUST include a “UI Action Matrix” (see below) and explicitly state whether the Action Surface Contract is satisfied. +If the contract is not satisfied, the spec MUST include an explicit exemption with rationale. + +### Functional Requirements + +- **FR-001 (Scope boundaries)**: The system MUST preserve deny-as-not-found for tenant/workspace scope boundaries: non-members or not-entitled users MUST receive a not found result for pages and actions in that scope. +- **FR-002 (403 semantics)**: When a user is a member of the tenant/workspace scope but lacks a required capability, the system MUST deny execution of the action with a forbidden result. +- **FR-003 (UI communication)**: For members without required capability, the UI MUST communicate that the action is unavailable (disabled state + explanatory hint). +- **FR-004 (Side-effect protection)**: Any action that creates/updates/deletes data or triggers external side-effects MUST require capability-first gating and MUST be enforced server-side. +- **FR-005 (Destructive confirmations)**: Destructive actions (archive, deactivate, delete, force delete) MUST require an explicit confirmation step. +- **FR-006 (Inspect affordance)**: Each in-scope list/table MUST provide a record inspection affordance per record. Accepted forms: clickable row navigation, a primary linked column, or a dedicated “View” action. Where row-click navigation already exists, it SHOULD be preserved; any dedicated “View” action is additive for accessibility and discoverability. +- **FR-007 (Action order and grouping)**: Row actions MUST follow the baseline order “View → Edit → Secondary → Destructive (last)”, and secondary actions MUST be grouped under a consistently labeled “More” group. +- **FR-008 (Presentation semantics)**: Sync/run-style actions MUST NOT be presented as destructive. +- **FR-009 (Empty-state CTAs)**: User-created resources in scope MUST provide an empty-state create call-to-action when the user is allowed to create. +- **FR-010 (Auditability)**: For every executed side-effect action in scope, the system MUST record an audit trail entry including who performed it, what was attempted, the scope (tenant/workspace), and the outcome. +- **FR-011 (Audit scope)**: Denied or cancelled attempts MUST NOT be required to create audit trail entries for this feature. + +#### In-scope hotspots + +The following hotspots MUST be brought into compliance: + +- Policy “Capture snapshot” action: requires Tenant Sync capability and must be treated as a side-effect action. +- Workspace CRUD actions: capability-first gating across create/edit and related header actions. +- Tenant archive/deactivate: destructive confirmation compliance. +- Provider connections: ensure a visible inspection affordance and consistent “More” grouping/order. +- Directory groups (Entra groups) and inventory items: verify they remain compliant with the inspection-affordance rule (clickable rows) and do not regress. +- Findings list: action ordering consistency (View first). + +### Canonical Capability Mapping (Spec 090) + +All capability checks in this feature MUST use constants from `App\Support\Auth\Capabilities`: + +| Hotspot / Surface | Required capability constant(s) | +|---|---| +| Policy list + view: “Sync from Intune”, “Capture snapshot” | `Capabilities::TENANT_SYNC` | +| Findings acknowledge actions (row, bulk, list-header mass acknowledge) | `Capabilities::TENANT_FINDINGS_ACKNOWLEDGE` | +| Backup schedules: create/edit/delete | `Capabilities::TENANT_BACKUP_SCHEDULES_MANAGE` | +| Backup schedules: run/retry (row + bulk) | `Capabilities::TENANT_BACKUP_SCHEDULES_RUN` | +| Provider connections: create/edit/credentials/default/enable/disable | `Capabilities::PROVIDER_MANAGE` | +| Provider connections: check connection / inventory sync / compliance snapshot | `Capabilities::PROVIDER_RUN` | +| Tenants: edit/admin consent | `Capabilities::TENANT_MANAGE` | +| Tenants: deactivate/force delete/restore | `Capabilities::TENANT_DELETE` | +| Tenants: sync | `Capabilities::TENANT_SYNC` | +| Workspaces: create/edit | `Capabilities::WORKSPACE_MANAGE` | + +## UI Action Matrix *(mandatory when Filament is changed)* + +If this feature adds/modifies any Filament Resource / RelationManager / Page, fill out the matrix below. + +For each surface, list the user-facing actions (intent) and note the label when it is explicitly customized. +Do not rely on framework-default labels being stable across versions. + +Also capture whether actions are destructive (confirmation? typed confirmation?), RBAC gating (capability + enforcement helper), +and whether the mutation writes an audit log. + +| Surface | Location | Header Actions | Inspect Affordance (List/Table) | Row Actions (max 2 visible) | Bulk Actions (grouped) | Empty-State CTA(s) | View Header Actions | Create/Edit Save+Cancel | Audit log? | Notes / Exemptions | +|---|---|---|---|---|---|---|---|---|---| +| Policies (list) | Tenant panel (tenant scope) | “Sync from Intune” | Dedicated “View” row action | Visible: “View”; Group: “More” → “Ignore” / “Restore” / “Sync” / “Export to Backup” | Group: “More” → “Ignore Policies” / “Restore Policies” / “Sync Policies” / “Export to Backup” | “Sync from Intune” (same as header) | — | — | Yes | Sync/run-style actions must not be presented as destructive; side-effect actions are capability-gated and enforced server-side. +| Policy (view) | Tenant panel (tenant scope) | — | — (view page) | — | — | — | “Capture snapshot” (confirmation) | — | Yes | Capture snapshot is treated as a side-effect action. +| Workspaces (list + CRUD) | Admin panel (workspace scope) | Create workspace (Create action) | Dedicated “View” row action | Visible: “View” / “Edit” | — | (If table is empty) Create workspace | — | Save + Cancel | Yes | Ensure capability-first gating for create/edit and consistent action ordering. +| Tenants (list) | Admin panel (platform scope) | — | Dedicated “View” row action (label: “View”) | Action group (ellipsis) containing: “View” / “Sync” / “Open” / “Edit” / “Restore” / “Admin consent” / “Open in Entra” / “Verify configuration” / “Deactivate” / “Force delete” | “Sync selected” | — | — | Save + Cancel | Yes | Destructive actions (“Deactivate”, “Force delete”) require confirmation; side-effect actions (“Sync”, “Verify configuration”) require capability gating. +| Provider connections (list) | Admin panel (platform scope; tenant-filtered) | Create provider connection (Create action) | Clickable rows (record URL to view) | Action group (target label: “More”) containing: “Edit” / “Check connection” / “Inventory sync” / “Compliance snapshot” / “Set as default” / “Update credentials” / “Enable connection” / “Disable connection” | — | (If table is empty) Create provider connection | — | Save + Cancel | Yes | Current code uses a group label “Actions”; normalize to “More” to match FR-007. +| Directory groups (Entra groups list) | Admin panel (tenant scope) | None (intentionally) | Clickable rows (record URL) | None (read-only list) | None | None | — | — | No | Inspection affordance is clickable rows; no row actions. +| Inventory items (list) | Admin panel (tenant scope) | None (intentionally) | Clickable rows (record URL) | None (read-only list) | None | None | — | — | No | Inspection affordance is clickable rows; no row actions. +| Findings (list) | Tenant panel (tenant scope) | — | Dedicated “View” row action | Visible: “View”; Secondary: “Acknowledge” (confirmation) | Group: “Acknowledge selected” (confirmation) | — | — | — | Yes | Action ordering must match baseline (View first; secondary actions grouped). +| Backup schedules (list) | Tenant panel (tenant scope) | Create backup schedule (Create action) | “Edit” (opens record details/edit form) | Action group (ellipsis) containing: “Run now” / “Retry” / “Edit” / “Delete” | Group: “Run now” / “Retry” / “Delete” | (If table is empty) Create backup schedule | — | Save + Cancel | Yes | Retention/archival is deferred; destructive actions (“Delete”, bulk delete) require confirmation. + +### Key Entities *(include if feature involves data)* + +- **Capability**: A named permission that grants access to a class of actions (e.g., “Tenant Sync”, “Workspace Manage”). +- **Scope entitlement**: Whether a user is a member of / entitled to a tenant or workspace. +- **Admin action surface**: Where a user can discover and trigger an action (header, row actions, bulk actions, empty state). +- **Destructive action**: An action that archives, deactivates, deletes, or force deletes data. +- **Audit trail entry**: A record of an attempted or completed action, including actor, scope, and outcome. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001 (Contract compliance)**: 100% of the in-scope resources satisfy Action Surface Contract v1 (clear inspection affordance, consistent “More” group, correct action ordering). +- **SC-002 (RBAC hardening)**: 100% of in-scope side-effect actions are denied for members without capability and are deny-as-not-found for non-members. +- **SC-003 (Confirmation compliance)**: 100% of in-scope destructive actions require confirmation prior to execution. +- **SC-004 (Discoverability)**: For each in-scope list/table, at least one inspection affordance is clearly visible and usable to access record details (clickable rows, a primary linked column, and/or a dedicated “View” action). +- **SC-005 (Auditability)**: 100% of executed in-scope side-effect actions produce an audit trail entry with actor + scope + outcome. + +## Assumptions + +- Capability names used in this spec map to the canonical capability registry. +- “Capture snapshot” is considered a side-effect action and therefore requires both confirmation and capability gating. +- BackupSchedule retention (soft delete / restore / force delete) is explicitly deferred to a follow-up spec. +- Existing row-click navigation patterns should remain unchanged; this feature ensures a clear inspection affordance (clickable rows, primary linked columns, or a dedicated “View” action where appropriate). + +## Out of Scope + +- Navigation or information architecture redesign. +- Broad refactors of policies/capabilities outside the listed hotspots. +- Changing the established 404 vs 403 boundary semantics. +- BackupSchedule retention behavior changes (soft delete, restore, force delete). + +## Rollout Notes + +- This change primarily affects user experience and authorization enforcement; it should be safe to roll out behind existing access controls. +- Optional retention/archival behavior (if introduced later) requires a separate rollout decision. + +## Testing Notes + +- Add at least one positive and one negative authorization test per critical hotspot. +- Add regression assertions that each in-scope list/table has a clear inspection affordance and consistent ordering. +- Verify that denied attempts do not create audit trail entries for this feature. + + diff --git a/specs/090-action-surface-contract-compliance/tasks.md b/specs/090-action-surface-contract-compliance/tasks.md new file mode 100644 index 0000000..26762de --- /dev/null +++ b/specs/090-action-surface-contract-compliance/tasks.md @@ -0,0 +1,158 @@ +--- + +description: "Tasks for Spec 090 implementation" + +--- + +# Tasks: Action Surface Contract Compliance & RBAC Hardening (Spec 090) + +**Input**: Design documents from `specs/090-action-surface-contract-compliance/` + +**Docs used**: +- `spec.md` (user stories + acceptance + UI Action Matrix) +- `plan.md` (implementation phases + repo paths) +- `research.md`, `data-model.md`, `quickstart.md`, `contracts/` + +**Tests**: REQUIRED (Pest) — this spec changes runtime behavior (RBAC + Filament action surfaces). + +## Phase 1: Setup (Shared Infrastructure) + +- [X] T001 Confirm current plan/spec alignment in specs/090-action-surface-contract-compliance/plan.md +- [x] T002 Reconcile “View action” vs “inspection affordance” wording in specs/090-action-surface-contract-compliance/spec.md (match constitution: clickable rows OR View action OR primary link) +- [X] T003 [P] Baseline-run guard tests via specs/090-action-surface-contract-compliance/quickstart.md (ensure current failures are understood) +- [X] T004 [P] Inventory in-scope exemptions in app/Support/Ui/ActionSurface/ActionSurfaceExemptions.php and list the exact classes to remove in specs/090-action-surface-contract-compliance/research.md + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**⚠️ CRITICAL**: No user story work should start until this phase is complete. + +- [x] T005 Update the UI Action Matrix in specs/090-action-surface-contract-compliance/spec.md with the exact intended actions + gating per in-scope surface +- [X] T006 [P] Identify the canonical capability constants for each hotspot in app/Support/Auth/Capabilities.php and record them in specs/090-action-surface-contract-compliance/spec.md +- [X] T007 Create Spec 090 test folder tests/Feature/090/ with a short README section in specs/090-action-surface-contract-compliance/quickstart.md describing how to run only Spec 090 tests +- [X] T008 [P] Add a Spec 090 RBAC semantics test skeleton in tests/Feature/090/RbacSemanticsTest.php (non-member 404 vs member missing capability 403) +- [X] T009 [P] Add a Spec 090 audit logging test skeleton in tests/Feature/090/AuditLoggingTest.php (successful side-effect → audit entry; denied attempts → no audit required) + +**Checkpoint**: Foundations ready — user stories can now proceed in priority order. + +--- + +## Phase 3: User Story 1 — Safe admin actions (Priority: P1) 🎯 MVP + +**Goal**: Capability-first gating + correct 404/403 semantics + successful audit logging for in-scope side-effect actions. + +**Independent Test**: For one or more representative side-effect actions, verify: +- non-member cannot access/execute (404 semantics) +- member without capability cannot execute (403 semantics) +- member with capability can execute/dispatch and an audit log is written + +### Tests for User Story 1 + +- [X] T010 [P] [US1] Implement “non-member → 404” coverage for a tenant-scoped side-effect action in tests/Feature/090/RbacSemanticsTest.php +- [X] T011 [P] [US1] Implement “member missing capability → 403” coverage for the same action in tests/Feature/090/RbacSemanticsTest.php +- [X] T012 [P] [US1] Implement “success → audit log entry created” coverage in tests/Feature/090/AuditLoggingTest.php +- [X] T013 [P] [US1] Implement “denied/cancelled attempts do not require audits” assertion in tests/Feature/090/AuditLoggingTest.php + +### Implementation for User Story 1 + +- [X] T014 [US1] Add capability-first gating to Policy “Capture snapshot” action in app/Filament/Resources/PolicyResource/Pages/ViewPolicy.php +- [X] T015 [US1] Add successful audit logging for “Capture snapshot” dispatch using App\\Services\\Intune\\AuditLogger in app/Filament/Resources/PolicyResource/Pages/ViewPolicy.php +- [X] T016 [US1] Remove misuse of destructive semantics for Policy “Sync from Intune” confirmation in app/Filament/Resources/PolicyResource/Pages/ListPolicies.php (keep confirmation, but non-destructive) +- [X] T017 [US1] Ensure server-side enforcement (404/403 semantics) for these actions is applied via App\\Support\\Rbac\\UiEnforcement in app/Filament/Resources/PolicyResource/Pages/ViewPolicy.php and app/Filament/Resources/PolicyResource/Pages/ListPolicies.php +- [X] T018 [US1] Run focused tests for US1 via `vendor/bin/sail artisan test --compact tests/Feature/090/RbacSemanticsTest.php` +- [X] T019 [US1] Run focused tests for US1 via `vendor/bin/sail artisan test --compact tests/Feature/090/AuditLoggingTest.php` + +**Checkpoint**: US1 is fully demonstrable and independently testable. + +--- + +## Phase 4: User Story 2 — Consistent action surfaces (Priority: P2) + +**Goal**: Every in-scope list/table has an inspection affordance and consistent action ordering/grouping; Action Surface Contract exemptions shrink. + +**Independent Test**: Guard tests pass while removing baseline exemptions for the in-scope components; visual review shows consistent “View/Edit/More” ordering. + +### Tests for User Story 2 + +- [X] T020 [P] [US2] Keep tests/Feature/Guards/ActionSurfaceContractTest.php green while removing baseline exemptions (guard is the primary gate) +- [X] T021 [P] [US2] Add a small regression test in tests/Feature/090/ActionSurfaceSmokeTest.php asserting in-scope resources are no longer baseline-exempt (optional helper to catch accidental re-exemption) + +### Implementation for User Story 2 + +- [X] T022 [US2] Add actionSurfaceDeclaration() to app/Filament/Resources/ProviderConnectionResource.php (satisfy header/row/bulk/empty-state slots or explicit exemptions with reasons) +- [X] T023 [US2] Add actionSurfaceDeclaration() to app/Filament/Resources/BackupScheduleResource.php (include empty-state CTA slot definition) +- [X] T024 [US2] Add actionSurfaceDeclaration() to app/Filament/Resources/FindingResource.php +- [X] T025 [US2] Add actionSurfaceDeclaration() to app/Filament/Resources/TenantResource.php +- [X] T026 [US2] Add actionSurfaceDeclaration() to app/Filament/Resources/Workspaces/WorkspaceResource.php + +- [X] T027 [US2] Normalize ProviderConnection row action group to match baseline ordering + grouping (keep clickable-row inspection; rename group label to “More”; keep destructive-like actions last; ensure max 2 visible row actions) in app/Filament/Resources/ProviderConnectionResource.php +- [X] T028 [US2] Reorder Finding row actions so View is first; move secondary actions (e.g., acknowledge) into a “More” ActionGroup in app/Filament/Resources/FindingResource.php +- [X] T029 [US2] Ensure bulk actions are grouped via BulkActionGroup labelled “More” in app/Filament/Resources/FindingResource.php +- [X] T030 [US2] Ensure destructive actions are not primary, require confirmation, and appear last in app/Filament/Resources/TenantResource.php + +- [X] T031 [US2] Remove baseline exemptions for the updated components from app/Support/Ui/ActionSurface/ActionSurfaceExemptions.php (only after declarations exist) +- [X] T032 [US2] Run guard tests via `vendor/bin/sail artisan test --compact tests/Feature/Guards/ActionSurfaceContractTest.php` + +**Checkpoint**: US2 passes guard tests and provides consistent action surfaces. + +--- + +## Phase 5: User Story 3 — Productive empty states (Priority: P3) + +**Goal**: Empty state CTAs exist for in-scope resources and are capability-gated. + +**Independent Test**: When lists are empty, the “New …” CTA is shown only for users who can create. + +### Tests for User Story 3 + +- [X] T033 [P] [US3] Add empty-state CTA visibility test for Workspaces list in tests/Feature/090/EmptyStateCtasTest.php +- [X] T034 [P] [US3] Add empty-state CTA visibility test for Backup schedules list in tests/Feature/090/EmptyStateCtasTest.php + +### Implementation for User Story 3 + +- [X] T035 [US3] Add capability-gated empty-state “New workspace” action on the Workspaces list in app/Filament/Resources/Workspaces/WorkspaceResource.php +- [X] T036 [US3] Add capability-gated empty-state “New backup schedule” action on the Backup schedules list in app/Filament/Resources/BackupScheduleResource.php +- [X] T037 [US3] Run focused tests via `vendor/bin/sail artisan test --compact tests/Feature/090/EmptyStateCtasTest.php` + +**Checkpoint**: US3 delivers clear empty states with correct gating. + +--- + +## Phase 6: Polish & Cross-Cutting Concerns + +- [X] T038 [P] Run formatter on modified code in app/, tests/, and specs/090-action-surface-contract-compliance/ via `vendor/bin/sail bin pint --dirty` +- [X] T039 Run Spec 090 focused pack: `vendor/bin/sail artisan test --compact tests/Feature/090/` +- [X] T040 Run guard pack: `vendor/bin/sail artisan test --compact tests/Feature/Guards/ActionSurfaceContractTest.php` +- [X] T041 Update specs/090-action-surface-contract-compliance/quickstart.md with the final minimal commands for: guard tests, Spec 090 tests, and pint +- [X] T042 [P] Verify global search remains tenant-scoped by confirming app/Filament/Concerns/ScopesGlobalSearchToTenant.php behavior and keeping existing scoping tests green: tests/Feature/Filament/TenantScopingTest.php and tests/Feature/Spec080WorkspaceManagedTenantAdminMigrationTest.php + +--- + +## Dependencies & Execution Order + +### Phase dependencies + +- Setup (Phase 1) → Foundational (Phase 2) → US1 (Phase 3) → US2 (Phase 4) → US3 (Phase 5) → Polish (Phase 6) + +### User story dependency graph + +- US1 is the MVP and can ship first. +- US2 and US3 can start after Foundational, but should be delivered in priority order (P2 then P3). + +## Parallel execution examples + +### US1 parallel work + +- [P] T010 + T011 + T012 + T013 can be implemented in parallel (different test sections/files). +- [P] T014 and T016 can be worked on in parallel (different Filament pages), then converged for T017. + +### US2 parallel work + +- [P] T022–T026 declarations can be started in parallel (different resources), but exemption removal T031 must be last. +- [P] T027–T030 action reordering can proceed in parallel after each resource compiles. + +### US3 parallel work + +- [P] T033 and T034 can be done in parallel (same file, different test blocks). +- [P] T035 and T036 can be done in parallel (different resources). diff --git a/tests/Feature/090/ActionSurfaceSmokeTest.php b/tests/Feature/090/ActionSurfaceSmokeTest.php new file mode 100644 index 0000000..5a1b69a --- /dev/null +++ b/tests/Feature/090/ActionSurfaceSmokeTest.php @@ -0,0 +1,28 @@ +assertFalse($exemptions->hasClass(ProviderConnectionResource::class)); + $this->assertFalse($exemptions->hasClass(BackupScheduleResource::class)); + $this->assertFalse($exemptions->hasClass(FindingResource::class)); + $this->assertFalse($exemptions->hasClass(TenantResource::class)); + $this->assertFalse($exemptions->hasClass(WorkspaceResource::class)); + } +} diff --git a/tests/Feature/090/AuditLoggingTest.php b/tests/Feature/090/AuditLoggingTest.php new file mode 100644 index 0000000..39c8dbe --- /dev/null +++ b/tests/Feature/090/AuditLoggingTest.php @@ -0,0 +1,114 @@ +actingAs($user); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + $policy = Policy::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + ]); + + Livewire::test(ViewPolicy::class, ['record' => $policy->getRouteKey()]) + ->callAction('capture_snapshot', data: [ + 'include_assignments' => true, + 'include_scope_tags' => true, + ]) + ->assertHasNoActionErrors(); + + Queue::assertPushed(CapturePolicySnapshotJob::class); + + $audit = AuditLog::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('action', 'policy.capture_snapshot_dispatched') + ->latest('id') + ->first(); + + $this->assertNotNull($audit); + $this->assertSame('success', $audit?->status); + $this->assertSame('operation_run', $audit?->resource_type); + $this->assertSame((int) $policy->getKey(), (int) ($audit?->metadata['policy_id'] ?? 0)); + } + + public function test_spec090_does_not_require_audit_logs_for_denied_capture_snapshot_attempts(): void + { + Queue::fake(); + + [$user, $tenant] = createUserWithTenant(role: 'readonly'); + $this->actingAs($user); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + $policy = Policy::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + ]); + + Livewire::test(ViewPolicy::class, ['record' => $policy->getRouteKey()]) + ->callAction('capture_snapshot', data: [ + 'include_assignments' => true, + 'include_scope_tags' => true, + ]) + ->assertSuccessful(); + + Queue::assertNothingPushed(); + + $this->assertSame( + 0, + AuditLog::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('action', 'policy.capture_snapshot_dispatched') + ->count(), + ); + } + + public function test_spec090_does_not_require_audit_logs_for_cancelled_capture_snapshot_modals(): void + { + Queue::fake(); + + [$user, $tenant] = createUserWithTenant(role: 'owner'); + $this->actingAs($user); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + $policy = Policy::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + ]); + + Livewire::test(ViewPolicy::class, ['record' => $policy->getRouteKey()]) + ->mountAction('capture_snapshot') + ->assertSuccessful(); + + Queue::assertNothingPushed(); + + $this->assertSame( + 0, + AuditLog::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('action', 'policy.capture_snapshot_dispatched') + ->count(), + ); + } +} diff --git a/tests/Feature/090/EmptyStateCtasTest.php b/tests/Feature/090/EmptyStateCtasTest.php new file mode 100644 index 0000000..b80b6d1 --- /dev/null +++ b/tests/Feature/090/EmptyStateCtasTest.php @@ -0,0 +1,100 @@ +create([ + 'archived_at' => now(), + ]); + + $user = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => 'owner', + ]); + + $user->forceFill([ + 'last_workspace_id' => (int) $workspace->getKey(), + ])->save(); + + $this->actingAs($user) + ->withSession([WorkspaceContext::SESSION_KEY => (int) $workspace->getKey()]); + + Livewire::test(ListWorkspaces::class) + ->assertTableEmptyStateActionsExistInOrder(['create']) + ->assertActionVisible('create') + ->assertActionEnabled('create'); + } + + public function test_spec090_disables_workspace_empty_state_create_cta_without_workspace_manage(): void + { + $workspace = Workspace::factory()->create([ + 'archived_at' => now(), + ]); + + $user = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => 'manager', + ]); + + $user->forceFill([ + 'last_workspace_id' => (int) $workspace->getKey(), + ])->save(); + + $this->actingAs($user) + ->withSession([WorkspaceContext::SESSION_KEY => (int) $workspace->getKey()]); + + Livewire::test(ListWorkspaces::class) + ->assertTableEmptyStateActionsExistInOrder(['create']) + ->assertActionVisible('create') + ->assertActionDisabled('create'); + } + + public function test_spec090_shows_backup_schedule_empty_state_create_cta_for_users_with_manage_capability(): void + { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + $this->actingAs($user); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + Livewire::test(ListBackupSchedules::class) + ->assertTableEmptyStateActionsExistInOrder(['create']) + ->assertActionVisible('create') + ->assertActionEnabled('create'); + } + + public function test_spec090_disables_backup_schedule_empty_state_create_cta_without_manage_capability(): void + { + [$user, $tenant] = createUserWithTenant(role: 'readonly'); + $this->actingAs($user); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + Livewire::test(ListBackupSchedules::class) + ->assertTableEmptyStateActionsExistInOrder([]) + ->assertActionHidden('create'); + } +} diff --git a/tests/Feature/090/RbacSemanticsTest.php b/tests/Feature/090/RbacSemanticsTest.php new file mode 100644 index 0000000..569dc0a --- /dev/null +++ b/tests/Feature/090/RbacSemanticsTest.php @@ -0,0 +1,51 @@ +create(); + $workspaceB = Workspace::factory()->create(); + + $user = User::factory()->create(); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspaceA->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => 'owner', + ]); + + $this->actingAs($user) + ->withSession([WorkspaceContext::SESSION_KEY => (int) $workspaceA->getKey()]) + ->get('/admin/workspaces/'.(int) $workspaceB->getKey().'/edit') + ->assertNotFound(); + } + + public function test_spec090_returns_403_for_workspace_members_missing_workspace_manage_on_workspace_edit(): void + { + $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) + ->withSession([WorkspaceContext::SESSION_KEY => (int) $workspace->getKey()]) + ->get('/admin/workspaces/'.(int) $workspace->getKey().'/edit') + ->assertForbidden(); + } +} diff --git a/tests/Feature/Spec080WorkspaceManagedTenantAdminMigrationTest.php b/tests/Feature/Spec080WorkspaceManagedTenantAdminMigrationTest.php index 46d69d4..64e2014 100644 --- a/tests/Feature/Spec080WorkspaceManagedTenantAdminMigrationTest.php +++ b/tests/Feature/Spec080WorkspaceManagedTenantAdminMigrationTest.php @@ -58,7 +58,7 @@ ->withSession([WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id]) ->get("/admin/tenants/{$tenant->external_id}") ->assertOk() - ->assertSee("/admin/tenants/{$tenant->external_id}/provider-connections", false); + ->assertSee('/admin/provider-connections?tenant_id='.$tenant->external_id, false); }); it('returns 404 for non-members on the workspace-managed tenant view route', function (): void { @@ -160,12 +160,14 @@ 'tenant_id' => (int) $tenant->getKey(), ]); - $this->actingAs($user) + $this->followingRedirects() + ->actingAs($user) ->withSession([WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id]) ->get("/admin/tenants/{$tenant->external_id}/provider-connections") ->assertOk(); - $this->actingAs($user) + $this->followingRedirects() + ->actingAs($user) ->withSession([WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id]) ->get("/admin/tenants/{$tenant->external_id}/provider-connections/{$connection->getKey()}/edit") ->assertOk(); @@ -174,12 +176,14 @@ it('returns 403 for workspace members missing mutation capability on provider connections', function (): void { [$user, $tenant] = createUserWithTenant(role: 'readonly', workspaceRole: 'readonly'); - $this->actingAs($user) + $this->followingRedirects() + ->actingAs($user) ->withSession([WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id]) ->get("/admin/tenants/{$tenant->external_id}/provider-connections") ->assertOk(); - $this->actingAs($user) + $this->followingRedirects() + ->actingAs($user) ->withSession([WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id]) ->get("/admin/tenants/{$tenant->external_id}/provider-connections/create") ->assertForbidden(); -- 2.45.2