diff --git a/app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php b/app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php index aca7057..a5ad6cf 100644 --- a/app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php +++ b/app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php @@ -4,7 +4,7 @@ use App\Filament\Resources\BackupSetResource; use App\Support\Auth\Capabilities; -use App\Support\Auth\UiEnforcement; +use App\Support\Rbac\UiEnforcement; use Filament\Actions; use Filament\Resources\Pages\ListRecords; @@ -19,16 +19,25 @@ private function tableHasRecords(): bool protected function getHeaderActions(): array { + $create = Actions\CreateAction::make(); + UiEnforcement::forAction($create) + ->requireCapability(Capabilities::TENANT_SYNC) + ->apply(); + return [ - UiEnforcement::for(Capabilities::TENANT_SYNC)->apply(Actions\CreateAction::make()) - ->visible(fn (): bool => $this->tableHasRecords()), + $create->visible(fn (): bool => $this->tableHasRecords()), ]; } protected function getTableEmptyStateActions(): array { + $create = Actions\CreateAction::make(); + UiEnforcement::forAction($create) + ->requireCapability(Capabilities::TENANT_SYNC) + ->apply(); + return [ - UiEnforcement::for(Capabilities::TENANT_SYNC)->apply(Actions\CreateAction::make()), + $create, ]; } } diff --git a/app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php b/app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php index ee33227..dffa962 100644 --- a/app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php +++ b/app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php @@ -98,7 +98,14 @@ protected function getHeaderActions(): array return null; }) - ->authorize(fn (): bool => true), + ->authorize(function () use ($resolver): bool { + $tenant = $this->resolveTenantForCreateAction(); + $user = auth()->user(); + + return $tenant instanceof Tenant + && $user instanceof User + && $resolver->isMember($user, $tenant); + }), ]; } @@ -175,7 +182,14 @@ private function makeEmptyStateCreateAction(): Actions\CreateAction return null; }) - ->authorize(fn (): bool => true); + ->authorize(function () use ($resolver): bool { + $tenant = $this->resolveTenantForCreateAction(); + $user = auth()->user(); + + return $tenant instanceof Tenant + && $user instanceof User + && $resolver->isMember($user, $tenant); + }); } private function resolveTenantExternalIdForCreateAction(): ?string diff --git a/app/Filament/Resources/RestoreRunResource/Pages/ListRestoreRuns.php b/app/Filament/Resources/RestoreRunResource/Pages/ListRestoreRuns.php index ec9a501..c957675 100644 --- a/app/Filament/Resources/RestoreRunResource/Pages/ListRestoreRuns.php +++ b/app/Filament/Resources/RestoreRunResource/Pages/ListRestoreRuns.php @@ -4,7 +4,7 @@ use App\Filament\Resources\RestoreRunResource; use App\Support\Auth\Capabilities; -use App\Support\Auth\UiEnforcement; +use App\Support\Rbac\UiEnforcement; use Filament\Actions; use Filament\Resources\Pages\ListRecords; @@ -19,16 +19,25 @@ private function tableHasRecords(): bool protected function getHeaderActions(): array { + $create = Actions\CreateAction::make(); + UiEnforcement::forAction($create) + ->requireCapability(Capabilities::TENANT_MANAGE) + ->apply(); + return [ - UiEnforcement::for(Capabilities::TENANT_MANAGE)->apply(Actions\CreateAction::make()) - ->visible(fn (): bool => $this->tableHasRecords()), + $create->visible(fn (): bool => $this->tableHasRecords()), ]; } protected function getTableEmptyStateActions(): array { + $create = Actions\CreateAction::make(); + UiEnforcement::forAction($create) + ->requireCapability(Capabilities::TENANT_MANAGE) + ->apply(); + return [ - UiEnforcement::for(Capabilities::TENANT_MANAGE)->apply(Actions\CreateAction::make()), + $create, ]; } } diff --git a/app/Jobs/FetchAssignmentsJob.php b/app/Jobs/FetchAssignmentsJob.php index a492a4a..72350d6 100644 --- a/app/Jobs/FetchAssignmentsJob.php +++ b/app/Jobs/FetchAssignmentsJob.php @@ -2,7 +2,9 @@ namespace App\Jobs; +use App\Jobs\Middleware\TrackOperationRun; use App\Models\BackupItem; +use App\Models\OperationRun; use App\Services\AssignmentBackupService; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; @@ -15,6 +17,8 @@ class FetchAssignmentsJob implements ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; + public ?OperationRun $operationRun = null; + /** * The number of times the job may be attempted. */ @@ -32,8 +36,19 @@ public function __construct( public int $backupItemId, public string $tenantExternalId, public string $policyExternalId, - public array $policyPayload - ) {} + public array $policyPayload, + ?OperationRun $operationRun = null, + ) { + $this->operationRun = $operationRun; + } + + /** + * @return array + */ + public function middleware(): array + { + return [new TrackOperationRun]; + } /** * Execute the job. diff --git a/app/Jobs/RestoreAssignmentsJob.php b/app/Jobs/RestoreAssignmentsJob.php index ed20750..92b6f67 100644 --- a/app/Jobs/RestoreAssignmentsJob.php +++ b/app/Jobs/RestoreAssignmentsJob.php @@ -2,6 +2,8 @@ namespace App\Jobs; +use App\Jobs\Middleware\TrackOperationRun; +use App\Models\OperationRun; use App\Models\RestoreRun; use App\Models\Tenant; use App\Services\AssignmentRestoreService; @@ -16,6 +18,8 @@ class RestoreAssignmentsJob implements ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; + public ?OperationRun $operationRun = null; + public int $tries = 1; public int $backoff = 0; @@ -33,7 +37,18 @@ public function __construct( public array $foundationMapping = [], public ?string $actorEmail = null, public ?string $actorName = null, - ) {} + ?OperationRun $operationRun = null, + ) { + $this->operationRun = $operationRun; + } + + /** + * @return array + */ + public function middleware(): array + { + return [new TrackOperationRun]; + } /** * Execute the job. diff --git a/app/Services/AssignmentBackupService.php b/app/Services/AssignmentBackupService.php index ba56088..38796c2 100644 --- a/app/Services/AssignmentBackupService.php +++ b/app/Services/AssignmentBackupService.php @@ -9,6 +9,8 @@ use App\Services\Graph\GroupResolver; use App\Services\Graph\ScopeTagResolver; use App\Services\Providers\MicrosoftGraphOptionsResolver; +use App\Support\OpsUx\RunFailureSanitizer; +use App\Support\Providers\ProviderReasonCodes; use Illuminate\Support\Facades\Log; class AssignmentBackupService @@ -19,6 +21,7 @@ public function __construct( private readonly AssignmentFilterResolver $assignmentFilterResolver, private readonly ScopeTagResolver $scopeTagResolver, private readonly MicrosoftGraphOptionsResolver $graphOptionsResolver, + private readonly OperationRunService $operationRunService, ) {} /** @@ -82,6 +85,8 @@ public function enrichWithAssignments( 'metadata' => $metadata, ]); + $this->recordFetchOperationRun($backupItem, $tenant, $metadata); + Log::warning('No assignments fetched for policy', [ 'tenant_id' => $tenantId, 'policy_id' => $policyId, @@ -121,6 +126,8 @@ public function enrichWithAssignments( 'metadata' => $metadata, ]); + $this->recordFetchOperationRun($backupItem, $tenant, $metadata); + Log::info('Assignments enriched for backup item', [ 'tenant_id' => $tenantId, 'policy_id' => $policyId, @@ -132,6 +139,60 @@ public function enrichWithAssignments( return $backupItem->refresh(); } + /** + * @param array $captureMetadata + */ + public function recordFetchOperationRun(BackupItem $backupItem, Tenant $tenant, array $captureMetadata = []): void + { + $run = $this->operationRunService->ensureRunWithIdentity( + tenant: $tenant, + type: 'assignments.fetch', + identityInputs: [ + 'backup_item_id' => (int) $backupItem->getKey(), + ], + context: [ + 'backup_set_id' => (int) $backupItem->backup_set_id, + 'backup_item_id' => (int) $backupItem->getKey(), + 'policy_id' => is_numeric($backupItem->policy_id) ? (int) $backupItem->policy_id : null, + 'policy_identifier' => (string) $backupItem->policy_identifier, + ], + ); + + if ($run->status === 'completed') { + return; + } + + $this->operationRunService->updateRun($run, 'running'); + + $fetchFailed = (bool) ($captureMetadata['assignments_fetch_failed'] ?? false); + + $reasonCandidate = $captureMetadata['assignments_fetch_error_code'] + ?? $captureMetadata['assignments_fetch_error'] + ?? ProviderReasonCodes::UnknownError; + + $reasonCode = RunFailureSanitizer::normalizeReasonCode( + $this->normalizeReasonCandidate($reasonCandidate) + ); + + $this->operationRunService->updateRun( + $run, + status: 'completed', + outcome: $fetchFailed ? 'failed' : 'succeeded', + summaryCounts: [ + 'total' => 1, + 'processed' => $fetchFailed ? 0 : 1, + 'failed' => $fetchFailed ? 1 : 0, + ], + failures: $fetchFailed + ? [[ + 'code' => 'assignments.fetch_failed', + 'reason_code' => $reasonCode, + 'message' => (string) ($captureMetadata['assignments_fetch_error'] ?? 'Assignments fetch failed'), + ]] + : [], + ); + } + /** * Resolve scope tag IDs to display names. */ @@ -233,4 +294,24 @@ private function extractAssignmentFilterIds(array $assignments): array return array_values(array_unique($filterIds)); } + + private function normalizeReasonCandidate(mixed $candidate): string + { + if (! is_string($candidate) && ! is_numeric($candidate)) { + return ProviderReasonCodes::UnknownError; + } + + $raw = trim((string) $candidate); + + if ($raw === '') { + return ProviderReasonCodes::UnknownError; + } + + $raw = preg_replace('/(?skipOutcome($assignment, null, null, 'Assignment filter mapping is unavailable.'); $summary['skipped']++; - $this->logAssignmentOutcome( - status: 'skipped', - tenant: $tenant, - assignment: $assignment, - restoreRun: $restoreRun, - actorEmail: $actorEmail, - actorName: $actorName, - metadata: [ - 'policy_id' => $policyId, - 'policy_type' => $policyType, - 'assignment_filter_id' => $filterId, - 'reason' => 'Assignment filter mapping is unavailable.', - ] - ); continue; } @@ -169,20 +153,6 @@ public function restore( 'Assignment filter mapping missing for filter ID.' ); $summary['skipped']++; - $this->logAssignmentOutcome( - status: 'skipped', - tenant: $tenant, - assignment: $assignment, - restoreRun: $restoreRun, - actorEmail: $actorEmail, - actorName: $actorName, - metadata: [ - 'policy_id' => $policyId, - 'policy_type' => $policyType, - 'assignment_filter_id' => $filterId, - 'reason' => 'Assignment filter mapping missing for filter ID.', - ] - ); continue; } @@ -201,20 +171,6 @@ public function restore( if ($mappedGroupId === 'SKIP') { $outcomes[] = $this->skipOutcome($assignment, $groupId, $mappedGroupId); $summary['skipped']++; - $this->logAssignmentOutcome( - status: 'skipped', - tenant: $tenant, - assignment: $assignment, - restoreRun: $restoreRun, - actorEmail: $actorEmail, - actorName: $actorName, - metadata: [ - 'policy_id' => $policyId, - 'policy_type' => $policyType, - 'group_id' => $groupId, - 'mapped_group_id' => $mappedGroupId, - ] - ); continue; } @@ -262,20 +218,6 @@ public function restore( $meta['mapped_group_id'] ); $summary['success']++; - $this->logAssignmentOutcome( - status: 'created', - tenant: $tenant, - assignment: $meta['assignment'], - restoreRun: $restoreRun, - actorEmail: $actorEmail, - actorName: $actorName, - metadata: [ - 'policy_id' => $policyId, - 'policy_type' => $policyType, - 'group_id' => $meta['group_id'], - 'mapped_group_id' => $meta['mapped_group_id'], - ] - ); } } else { $reason = $assignResponse->meta['error_message'] ?? 'Graph assign failed'; @@ -294,22 +236,6 @@ public function restore( $assignResponse ); $summary['failed']++; - $this->logAssignmentOutcome( - status: 'failed', - tenant: $tenant, - assignment: $meta['assignment'], - restoreRun: $restoreRun, - actorEmail: $actorEmail, - actorName: $actorName, - metadata: [ - 'policy_id' => $policyId, - 'policy_type' => $policyType, - 'group_id' => $meta['group_id'], - 'mapped_group_id' => $meta['mapped_group_id'], - 'graph_error_message' => $assignResponse->meta['error_message'] ?? null, - 'graph_error_code' => $assignResponse->meta['error_code'] ?? null, - ], - ); } } @@ -397,20 +323,6 @@ public function restore( if ($createResponse->successful()) { $outcomes[] = $this->successOutcome($meta['assignment'], $meta['group_id'], $meta['mapped_group_id']); $summary['success']++; - $this->logAssignmentOutcome( - status: 'created', - tenant: $tenant, - assignment: $meta['assignment'], - restoreRun: $restoreRun, - actorEmail: $actorEmail, - actorName: $actorName, - metadata: [ - 'policy_id' => $policyId, - 'policy_type' => $policyType, - 'group_id' => $meta['group_id'], - 'mapped_group_id' => $meta['mapped_group_id'], - ] - ); } else { $outcomes[] = $this->failureOutcome( $meta['assignment'], @@ -420,22 +332,6 @@ public function restore( $createResponse ); $summary['failed']++; - $this->logAssignmentOutcome( - status: 'failed', - tenant: $tenant, - assignment: $meta['assignment'], - restoreRun: $restoreRun, - actorEmail: $actorEmail, - actorName: $actorName, - metadata: [ - 'policy_id' => $policyId, - 'policy_type' => $policyType, - 'group_id' => $meta['group_id'], - 'mapped_group_id' => $meta['mapped_group_id'], - 'graph_error_message' => $createResponse->meta['error_message'] ?? null, - 'graph_error_code' => $createResponse->meta['error_code'] ?? null, - ], - ); } usleep(100000); @@ -597,40 +493,4 @@ private function failureOutcome( 'graph_client_request_id' => $response?->meta['client_request_id'] ?? null, ], static fn ($value) => $value !== null); } - - private function logAssignmentOutcome( - string $status, - Tenant $tenant, - array $assignment, - ?RestoreRun $restoreRun, - ?string $actorEmail, - ?string $actorName, - array $metadata - ): void { - $action = match ($status) { - 'created' => 'restore.assignment.created', - 'failed' => 'restore.assignment.failed', - default => 'restore.assignment.skipped', - }; - - $statusLabel = match ($status) { - 'created' => 'success', - 'failed' => 'failed', - default => 'warning', - }; - - $this->auditLogger->log( - tenant: $tenant, - action: $action, - context: [ - 'metadata' => $metadata, - 'assignment' => $this->sanitizeAssignment($assignment), - ], - actorEmail: $actorEmail, - actorName: $actorName, - status: $statusLabel, - resourceType: 'restore_run', - resourceId: $restoreRun ? (string) $restoreRun->id : null - ); - } } diff --git a/app/Services/Auth/CapabilityResolver.php b/app/Services/Auth/CapabilityResolver.php index 152130b..a88ec83 100644 --- a/app/Services/Auth/CapabilityResolver.php +++ b/app/Services/Auth/CapabilityResolver.php @@ -93,7 +93,7 @@ private function getMembership(User $user, Tenant $tenant): ?array { $cacheKey = "membership_{$user->id}_{$tenant->id}"; - if (! isset($this->resolvedMemberships[$cacheKey])) { + if (! array_key_exists($cacheKey, $this->resolvedMemberships)) { $membership = TenantMembership::query() ->where('user_id', $user->id) ->where('tenant_id', $tenant->id) @@ -105,6 +105,47 @@ private function getMembership(User $user, Tenant $tenant): ?array return $this->resolvedMemberships[$cacheKey]; } + /** + * Prime membership cache for a set of tenants in one query. + * + * Used to avoid N+1 queries for bulk selection authorization. + * + * @param array $tenantIds + */ + public function primeMemberships(User $user, array $tenantIds): void + { + $tenantIds = array_values(array_unique(array_map(static fn ($id): int => (int) $id, $tenantIds))); + + if ($tenantIds === []) { + return; + } + + $missingTenantIds = []; + foreach ($tenantIds as $tenantId) { + $cacheKey = "membership_{$user->id}_{$tenantId}"; + if (! array_key_exists($cacheKey, $this->resolvedMemberships)) { + $missingTenantIds[] = $tenantId; + } + } + + if ($missingTenantIds === []) { + return; + } + + $memberships = TenantMembership::query() + ->where('user_id', $user->id) + ->whereIn('tenant_id', $missingTenantIds) + ->get(['tenant_id', 'role', 'source', 'source_ref']); + + $byTenantId = $memberships->keyBy('tenant_id'); + + foreach ($missingTenantIds as $tenantId) { + $cacheKey = "membership_{$user->id}_{$tenantId}"; + $membership = $byTenantId->get($tenantId); + $this->resolvedMemberships[$cacheKey] = $membership?->toArray(); + } + } + /** * Clear cached memberships (useful for testing or after membership changes) */ diff --git a/app/Services/Graph/AssignmentFetcher.php b/app/Services/Graph/AssignmentFetcher.php index 29aa0b5..3200292 100644 --- a/app/Services/Graph/AssignmentFetcher.php +++ b/app/Services/Graph/AssignmentFetcher.php @@ -7,7 +7,7 @@ class AssignmentFetcher { public function __construct( - private readonly MicrosoftGraphClient $graphClient, + private readonly GraphClientInterface $graphClient, private readonly GraphContractRegistry $contracts, ) {} diff --git a/app/Services/Graph/AssignmentFilterResolver.php b/app/Services/Graph/AssignmentFilterResolver.php index 764c134..57f5375 100644 --- a/app/Services/Graph/AssignmentFilterResolver.php +++ b/app/Services/Graph/AssignmentFilterResolver.php @@ -9,7 +9,7 @@ class AssignmentFilterResolver { public function __construct( - private readonly MicrosoftGraphClient $graphClient, + private readonly GraphClientInterface $graphClient, private readonly MicrosoftGraphOptionsResolver $graphOptionsResolver, ) {} diff --git a/app/Services/Graph/GroupResolver.php b/app/Services/Graph/GroupResolver.php index afac6a4..55e4f33 100644 --- a/app/Services/Graph/GroupResolver.php +++ b/app/Services/Graph/GroupResolver.php @@ -8,7 +8,7 @@ class GroupResolver { public function __construct( - private readonly MicrosoftGraphClient $graphClient, + private readonly GraphClientInterface $graphClient, ) {} /** diff --git a/app/Services/Intune/BackupService.php b/app/Services/Intune/BackupService.php index 255e2e1..adad3e6 100644 --- a/app/Services/Intune/BackupService.php +++ b/app/Services/Intune/BackupService.php @@ -290,21 +290,27 @@ private function snapshotPolicy( $captured = $captureResult['captured']; $payload = $captured['payload']; $metadata = $captured['metadata'] ?? []; + $backupItem = $this->createBackupItemFromVersion( + tenant: $tenant, + backupSet: $backupSet, + policy: $policy, + version: $version, + payload: is_array($payload) ? $payload : [], + assignments: $captured['assignments'] ?? null, + scopeTags: $captured['scope_tags'] ?? null, + metadata: is_array($metadata) ? $metadata : [], + warnings: $captured['warnings'] ?? [], + ); - return [ - $this->createBackupItemFromVersion( + if ($includeAssignments) { + $this->assignmentBackupService->recordFetchOperationRun( + backupItem: $backupItem, tenant: $tenant, - backupSet: $backupSet, - policy: $policy, - version: $version, - payload: is_array($payload) ? $payload : [], - assignments: $captured['assignments'] ?? null, - scopeTags: $captured['scope_tags'] ?? null, - metadata: is_array($metadata) ? $metadata : [], - warnings: $captured['warnings'] ?? [], - ), - null, - ]; + captureMetadata: is_array($metadata) ? $metadata : [], + ); + } + + return [$backupItem, null]; } /** diff --git a/app/Services/Intune/PolicyCaptureOrchestrator.php b/app/Services/Intune/PolicyCaptureOrchestrator.php index ab266c5..967d157 100644 --- a/app/Services/Intune/PolicyCaptureOrchestrator.php +++ b/app/Services/Intune/PolicyCaptureOrchestrator.php @@ -7,6 +7,7 @@ use App\Models\Tenant; use App\Services\Graph\AssignmentFetcher; use App\Services\Graph\AssignmentFilterResolver; +use App\Services\Graph\GraphException; use App\Services\Graph\GroupResolver; use App\Services\Graph\ScopeTagResolver; use App\Services\Providers\MicrosoftGraphOptionsResolver; @@ -108,6 +109,9 @@ public function capture( } catch (\Throwable $e) { $captureMetadata['assignments_fetch_failed'] = true; $captureMetadata['assignments_fetch_error'] = $e->getMessage(); + $captureMetadata['assignments_fetch_error_code'] = $e instanceof GraphException + ? ($e->status ?? null) + : (is_numeric($e->getCode()) ? (int) $e->getCode() : null); Log::warning('Failed to fetch assignments during capture', [ 'tenant_id' => $tenant->id, @@ -295,6 +299,9 @@ public function ensureVersionHasAssignments( } catch (\Throwable $e) { $metadata['assignments_fetch_failed'] = true; $metadata['assignments_fetch_error'] = $e->getMessage(); + $metadata['assignments_fetch_error_code'] = $e instanceof GraphException + ? ($e->status ?? null) + : (is_numeric($e->getCode()) ? (int) $e->getCode() : null); Log::warning('Failed to backfill assignments for version', [ 'version_id' => $version->id, diff --git a/app/Services/Intune/RestoreService.php b/app/Services/Intune/RestoreService.php index 319602e..fe76a9c 100644 --- a/app/Services/Intune/RestoreService.php +++ b/app/Services/Intune/RestoreService.php @@ -4,6 +4,7 @@ use App\Models\BackupItem; use App\Models\BackupSet; +use App\Models\OperationRun; use App\Models\Policy; use App\Models\PolicyVersion; use App\Models\ProviderConnection; @@ -14,8 +15,10 @@ use App\Services\Graph\GraphContractRegistry; use App\Services\Graph\GraphErrorMapper; use App\Services\Graph\GraphLogger; +use App\Services\OperationRunService; use App\Services\Providers\ProviderConnectionResolver; use App\Services\Providers\ProviderGateway; +use App\Support\OpsUx\RunFailureSanitizer; use App\Support\Providers\ProviderReasonCodes; use Carbon\CarbonImmutable; use Illuminate\Support\Arr; @@ -34,6 +37,7 @@ public function __construct( private readonly GraphContractRegistry $contracts, private readonly ConfigurationPolicyTemplateResolver $templateResolver, private readonly AssignmentRestoreService $assignmentRestoreService, + private readonly OperationRunService $operationRunService, private readonly FoundationMappingService $foundationMappingService, private readonly ?ProviderConnectionResolver $providerConnections = null, private readonly ?ProviderGateway $providerGateway = null, @@ -378,6 +382,36 @@ public function execute( $results = $foundationEntries; $hardFailures = $foundationFailures; + $assignmentRestoreRun = null; + $assignmentRestoreTotals = [ + 'success' => 0, + 'failed' => 0, + 'skipped' => 0, + ]; + $assignmentRestoreFailures = []; + + $assignmentRestoreItemCount = $policyItems + ->filter(fn (BackupItem $policyItem): bool => is_array($policyItem->assignments) && $policyItem->assignments !== []) + ->count(); + + if (! $dryRun && $assignmentRestoreItemCount > 0) { + $assignmentRestoreRun = $this->operationRunService->ensureRunWithIdentity( + tenant: $tenant, + type: 'assignments.restore', + identityInputs: [ + 'restore_run_id' => (int) $restoreRun->getKey(), + ], + context: [ + 'restore_run_id' => (int) $restoreRun->getKey(), + 'backup_set_id' => (int) $backupSet->getKey(), + 'assignment_item_count' => (int) $assignmentRestoreItemCount, + ], + ); + + if ($assignmentRestoreRun->status !== 'completed') { + $this->operationRunService->updateRun($assignmentRestoreRun, 'running'); + } + } foreach ($policyItems as $item) { $context = [ @@ -761,6 +795,41 @@ public function execute( $assignmentSummary = $assignmentOutcomes['summary'] ?? null; + if (is_array($assignmentSummary)) { + $assignmentRestoreTotals['success'] += (int) ($assignmentSummary['success'] ?? 0); + $assignmentRestoreTotals['failed'] += (int) ($assignmentSummary['failed'] ?? 0); + $assignmentRestoreTotals['skipped'] += (int) ($assignmentSummary['skipped'] ?? 0); + } + + if (is_array($assignmentOutcomes)) { + foreach ($assignmentOutcomes['outcomes'] ?? [] as $assignmentOutcome) { + if (! is_array($assignmentOutcome)) { + continue; + } + + if (($assignmentOutcome['status'] ?? null) !== 'failed') { + continue; + } + + $message = (string) ($assignmentOutcome['reason'] + ?? $assignmentOutcome['graph_error_message'] + ?? 'Assignment restore failed'); + + $reasonCandidate = $assignmentOutcome['graph_error_code'] + ?? $assignmentOutcome['reason'] + ?? $assignmentOutcome['graph_error_message'] + ?? ProviderReasonCodes::UnknownError; + + $assignmentRestoreFailures[] = [ + 'code' => 'assignments.restore_failed', + 'reason_code' => RunFailureSanitizer::normalizeReasonCode( + $this->normalizeFailureReasonCandidate($reasonCandidate) + ), + 'message' => $message, + ]; + } + } + if (is_array($assignmentSummary) && ($assignmentSummary['failed'] ?? 0) > 0 && $itemStatus === 'applied') { $itemStatus = 'partial'; $resultReason = 'Assignments restored with failures'; @@ -956,6 +1025,56 @@ public function execute( ]), ]); + if ($assignmentRestoreRun instanceof OperationRun) { + $assignmentAttempted = $assignmentRestoreTotals['success'] + $assignmentRestoreTotals['failed']; + + $assignmentRunOutcome = 'succeeded'; + + if ($assignmentRestoreTotals['failed'] > 0 && $assignmentRestoreTotals['success'] > 0) { + $assignmentRunOutcome = 'partially_succeeded'; + } elseif ($assignmentRestoreTotals['failed'] > 0) { + $assignmentRunOutcome = 'failed'; + } + + $this->operationRunService->updateRun( + $assignmentRestoreRun, + status: 'completed', + outcome: $assignmentRunOutcome, + summaryCounts: [ + 'total' => $assignmentAttempted, + 'processed' => $assignmentRestoreTotals['success'], + 'failed' => $assignmentRestoreTotals['failed'], + ], + failures: $assignmentRestoreFailures, + ); + + $assignmentAuditStatus = match (true) { + $assignmentRestoreTotals['failed'] > 0 && $assignmentRestoreTotals['success'] === 0 => 'failed', + $assignmentRestoreTotals['failed'] > 0 || $assignmentRestoreTotals['skipped'] > 0 => 'partial', + default => 'success', + }; + + $this->auditLogger->log( + tenant: $tenant, + action: 'restore.assignments.summary', + context: [ + 'metadata' => [ + 'restore_run_id' => (int) $restoreRun->getKey(), + 'backup_set_id' => (int) $backupSet->getKey(), + 'total' => $assignmentAttempted, + 'succeeded' => (int) $assignmentRestoreTotals['success'], + 'failed' => (int) $assignmentRestoreTotals['failed'], + 'skipped' => (int) $assignmentRestoreTotals['skipped'], + ], + ], + actorEmail: $actorEmail, + actorName: $actorName, + resourceType: 'restore_run', + resourceId: (string) $restoreRun->getKey(), + status: $assignmentAuditStatus, + ); + } + $this->auditLogger->log( tenant: $tenant, action: $dryRun ? 'restore.previewed' : 'restore.executed', @@ -1025,6 +1144,26 @@ private function resolveRestoreMode(string $policyType): string return $restore; } + private function normalizeFailureReasonCandidate(mixed $candidate): string + { + if (! is_string($candidate) && ! is_numeric($candidate)) { + return ProviderReasonCodes::UnknownError; + } + + $raw = trim((string) $candidate); + + if ($raw === '') { + return ProviderReasonCodes::UnknownError; + } + + $raw = preg_replace('/(?contracts->get($policyType); diff --git a/app/Services/Intune/VersionService.php b/app/Services/Intune/VersionService.php index ea8c4f5..568618a 100644 --- a/app/Services/Intune/VersionService.php +++ b/app/Services/Intune/VersionService.php @@ -7,6 +7,7 @@ use App\Models\Tenant; use App\Services\Graph\AssignmentFetcher; use App\Services\Graph\AssignmentFilterResolver; +use App\Services\Graph\GraphException; use App\Services\Graph\GroupResolver; use App\Services\Graph\ScopeTagResolver; use App\Services\Providers\MicrosoftGraphOptionsResolver; @@ -182,6 +183,9 @@ public function captureFromGraph( } catch (\Throwable $e) { $assignmentMetadata['assignments_fetch_failed'] = true; $assignmentMetadata['assignments_fetch_error'] = $e->getMessage(); + $assignmentMetadata['assignments_fetch_error_code'] = $e instanceof GraphException + ? ($e->status ?? null) + : (is_numeric($e->getCode()) ? (int) $e->getCode() : null); } } diff --git a/app/Support/Auth/UiEnforcement.php b/app/Support/Auth/UiEnforcement.php deleted file mode 100644 index 9686556..0000000 --- a/app/Support/Auth/UiEnforcement.php +++ /dev/null @@ -1,526 +0,0 @@ -): bool|null - */ - private ?\Closure $bulkPreflight = null; - - public function __construct(private string $capability) - { - } - - public static function for(string $capability): self - { - return new self($capability); - } - - public function preserveVisibility(): self - { - if ($this->tenantResolverMode !== self::TENANT_RESOLVER_FILAMENT) { - throw new LogicException('preserveVisibility() is allowed only for tenant-scoped (tenantFromFilament) surfaces.'); - } - - $this->preserveVisibility = true; - - return $this; - } - - public function andVisibleWhen(callable $businessVisible): self - { - $this->businessVisible = \Closure::fromCallable($businessVisible); - - return $this; - } - - public function andHiddenWhen(callable $businessHidden): self - { - $this->businessHidden = \Closure::fromCallable($businessHidden); - - return $this; - } - - public function tenantFromFilament(): self - { - $this->tenantResolverMode = self::TENANT_RESOLVER_FILAMENT; - $this->customTenantResolver = null; - - return $this; - } - - public function tenantFromRecord(): self - { - if ($this->preserveVisibility) { - throw new LogicException('preserveVisibility() is forbidden for record-scoped surfaces.'); - } - - $this->tenantResolverMode = self::TENANT_RESOLVER_RECORD; - $this->customTenantResolver = null; - - return $this; - } - - public function tenantFrom(callable $resolver): self - { - if ($this->preserveVisibility) { - throw new LogicException('preserveVisibility() is forbidden for record-scoped surfaces.'); - } - - $this->tenantResolverMode = self::TENANT_RESOLVER_CUSTOM; - $this->customTenantResolver = \Closure::fromCallable($resolver); - - return $this; - } - - /** - * Custom bulk authorization preflight for selection. - * - * Signature: fn (Collection $records): bool - */ - public function preflightSelection(callable $preflight): self - { - $this->bulkPreflightMode = self::BULK_PREFLIGHT_CUSTOM; - $this->bulkPreflight = \Closure::fromCallable($preflight); - - return $this; - } - - public function preflightByTenantMembership(): self - { - $this->bulkPreflightMode = self::BULK_PREFLIGHT_TENANT_MEMBERSHIP; - $this->bulkPreflight = null; - - return $this; - } - - public function preflightByCapability(): self - { - $this->bulkPreflightMode = self::BULK_PREFLIGHT_CAPABILITY; - $this->bulkPreflight = null; - - return $this; - } - - public function apply(Action $action): Action - { - $this->assertMixedVisibilityConfigIsValid(); - - if (! $this->preserveVisibility) { - $this->applyVisibility($action); - } - - if ($action->isBulk()) { - $action->disabled(function () use ($action): bool { - /** @var Collection $records */ - $records = collect($action->getSelectedRecords()); - - return $this->bulkIsDisabled($records); - }); - - $action->tooltip(function () use ($action): ?string { - /** @var Collection $records */ - $records = collect($action->getSelectedRecords()); - - return $this->bulkDisabledTooltip($records); - }); - } else { - $action->disabled(fn (?Model $record = null): bool => $this->isDisabled($record)); - $action->tooltip(fn (?Model $record = null): ?string => $this->disabledTooltip($record)); - } - - return $action; - } - - public function isAllowed(?Model $record = null): bool - { - return ! $this->isDisabled($record); - } - - public function authorizeOrAbort(?Model $record = null): void - { - $user = auth()->user(); - abort_unless($user instanceof User, 403); - - $tenant = $this->resolveTenant($record); - - if (! ($tenant instanceof Tenant)) { - abort(404); - } - - abort_unless($this->isMemberOfTenant($user, $tenant), 404); - abort_unless(Gate::forUser($user)->allows($this->capability, $tenant), 403); - } - - /** - * Server-side enforcement for bulk selections. - * - * - If any selected tenant is not a membership: 404 (deny-as-not-found). - * - If all are memberships but any lacks capability: 403. - * - * @param Collection $records - */ - public function authorizeBulkSelectionOrAbort(Collection $records): void - { - $user = auth()->user(); - abort_unless($user instanceof User, 403); - - $tenantIds = $this->resolveTenantIdsForRecords($records); - - if ($tenantIds === []) { - abort(403); - } - - $membershipTenantIds = $this->membershipTenantIds($user, $tenantIds); - - if (count($membershipTenantIds) !== count($tenantIds)) { - abort(404); - } - - $allowedTenantIds = $this->capabilityTenantIds($user, $tenantIds); - - if (count($allowedTenantIds) !== count($tenantIds)) { - abort(403); - } - } - - /** - * Public helper for evaluating bulk selection authorization decisions. - * - * @param Collection $records - */ - public function bulkSelectionIsAuthorized(User $user, Collection $records): bool - { - return $this->bulkSelectionIsAuthorizedInternal($user, $records); - } - - private function applyVisibility(Action $action): void - { - $canApplyMemberVisibility = ! ($action->isBulk() && $this->tenantResolverMode !== self::TENANT_RESOLVER_FILAMENT); - - $businessVisible = $this->businessVisible; - $businessHidden = $this->businessHidden; - - if ($businessVisible instanceof \Closure) { - $action->visible(function () use ($action, $businessVisible, $canApplyMemberVisibility): bool { - if (! (bool) $action->evaluate($businessVisible)) { - return false; - } - - if (! $canApplyMemberVisibility) { - return true; - } - - $record = $action->getRecord(); - - return $this->isMember($record instanceof Model ? $record : null); - }); - } - - if ($businessHidden instanceof \Closure) { - $action->hidden(function () use ($action, $businessHidden, $canApplyMemberVisibility): bool { - if ($canApplyMemberVisibility) { - $record = $action->getRecord(); - - if (! $this->isMember($record instanceof Model ? $record : null)) { - return true; - } - } - - return (bool) $action->evaluate($businessHidden); - }); - - return; - } - - if (! $canApplyMemberVisibility) { - return; - } - - if (! ($businessVisible instanceof \Closure)) { - $action->hidden(function () use ($action): bool { - $record = $action->getRecord(); - - return ! $this->isMember($record instanceof Model ? $record : null); - }); - } - } - - private function assertMixedVisibilityConfigIsValid(): void - { - if ($this->preserveVisibility && ($this->businessVisible instanceof \Closure || $this->businessHidden instanceof \Closure)) { - throw new LogicException('preserveVisibility() cannot be combined with andVisibleWhen()/andHiddenWhen().'); - } - - if ($this->preserveVisibility && $this->tenantResolverMode !== self::TENANT_RESOLVER_FILAMENT) { - throw new LogicException('preserveVisibility() is allowed only for tenant-scoped (tenantFromFilament) surfaces.'); - } - } - - private function isDisabled(?Model $record = null): bool - { - $user = auth()->user(); - - if (! ($user instanceof User)) { - return true; - } - - $tenant = $this->resolveTenant($record); - - if (! ($tenant instanceof Tenant)) { - return true; - } - - if (! $this->isMemberOfTenant($user, $tenant)) { - return true; - } - - return ! Gate::forUser($user)->allows($this->capability, $tenant); - } - - private function disabledTooltip(?Model $record = null): ?string - { - $user = auth()->user(); - - if (! ($user instanceof User)) { - return null; - } - - $tenant = $this->resolveTenant($record); - - if (! ($tenant instanceof Tenant)) { - return null; - } - - if (! $this->isMemberOfTenant($user, $tenant)) { - return null; - } - - if (Gate::forUser($user)->allows($this->capability, $tenant)) { - return null; - } - - return UiTooltips::insufficientPermission(); - } - - private function bulkIsDisabled(Collection $records): bool - { - $user = auth()->user(); - - if (! ($user instanceof User)) { - return true; - } - - return ! $this->bulkSelectionIsAuthorizedInternal($user, $records); - } - - private function bulkDisabledTooltip(Collection $records): ?string - { - $user = auth()->user(); - - if (! ($user instanceof User)) { - return null; - } - - if ($this->bulkSelectionIsAuthorizedInternal($user, $records)) { - return null; - } - - return UiTooltips::insufficientPermission(); - } - - private function bulkSelectionIsAuthorizedInternal(User $user, Collection $records): bool - { - if ($this->bulkPreflightMode === self::BULK_PREFLIGHT_CUSTOM && $this->bulkPreflight instanceof \Closure) { - return (bool) ($this->bulkPreflight)($records); - } - - $tenantIds = $this->resolveTenantIdsForRecords($records); - - if ($tenantIds === []) { - return false; - } - - return match ($this->bulkPreflightMode) { - self::BULK_PREFLIGHT_TENANT_MEMBERSHIP => count($this->membershipTenantIds($user, $tenantIds)) === count($tenantIds), - self::BULK_PREFLIGHT_CAPABILITY => count($this->capabilityTenantIds($user, $tenantIds)) === count($tenantIds), - default => false, - }; - } - - /** - * @param Collection $records - * @return array - */ - private function resolveTenantIdsForRecords(Collection $records): array - { - if ($this->tenantResolverMode === self::TENANT_RESOLVER_FILAMENT) { - $tenant = Filament::getTenant(); - - return $tenant instanceof Tenant ? [(int) $tenant->getKey()] : []; - } - - if ($this->tenantResolverMode === self::TENANT_RESOLVER_RECORD) { - $ids = $records - ->filter(fn (Model $record): bool => $record instanceof Tenant) - ->map(fn (Tenant $tenant): int => (int) $tenant->getKey()) - ->all(); - - return array_values(array_unique($ids)); - } - - if ($this->tenantResolverMode === self::TENANT_RESOLVER_CUSTOM && $this->customTenantResolver instanceof \Closure) { - $ids = []; - - foreach ($records as $record) { - if (! ($record instanceof Model)) { - continue; - } - - $resolved = ($this->customTenantResolver)($record); - - if ($resolved instanceof Tenant) { - $ids[] = (int) $resolved->getKey(); - continue; - } - - if (is_int($resolved)) { - $ids[] = $resolved; - } - } - - return array_values(array_unique($ids)); - } - - return []; - } - - private function isMember(?Model $record = null): bool - { - $user = auth()->user(); - - if (! ($user instanceof User)) { - return false; - } - - $tenant = $this->resolveTenant($record); - - if (! ($tenant instanceof Tenant)) { - return false; - } - - return $this->isMemberOfTenant($user, $tenant); - } - - private function isMemberOfTenant(User $user, Tenant $tenant): bool - { - return Gate::forUser($user)->allows(Capabilities::TENANT_VIEW, $tenant); - } - - private function resolveTenant(?Model $record = null): ?Tenant - { - return match ($this->tenantResolverMode) { - self::TENANT_RESOLVER_FILAMENT => Filament::getTenant() instanceof Tenant ? Filament::getTenant() : null, - self::TENANT_RESOLVER_RECORD => $record instanceof Tenant ? $record : null, - self::TENANT_RESOLVER_CUSTOM => $this->resolveTenantViaCustomResolver($record), - default => null, - }; - } - - private function resolveTenantViaCustomResolver(?Model $record): ?Tenant - { - if (! ($this->customTenantResolver instanceof \Closure)) { - return null; - } - - if (! ($record instanceof Model)) { - return null; - } - - $resolved = ($this->customTenantResolver)($record); - - if ($resolved instanceof Tenant) { - return $resolved; - } - - return null; - } - - /** - * @param array $tenantIds - * @return array - */ - private function membershipTenantIds(User $user, array $tenantIds): array - { - /** @var array $ids */ - $ids = DB::table('tenant_memberships') - ->where('user_id', (int) $user->getKey()) - ->whereIn('tenant_id', $tenantIds) - ->pluck('tenant_id') - ->map(fn ($id): int => (int) $id) - ->all(); - - return array_values(array_unique($ids)); - } - - /** - * @param array $tenantIds - * @return array - */ - private function capabilityTenantIds(User $user, array $tenantIds): array - { - $roles = RoleCapabilityMap::rolesWithCapability($this->capability); - - if ($roles === []) { - return []; - } - - /** @var array $ids */ - $ids = DB::table('tenant_memberships') - ->where('user_id', (int) $user->getKey()) - ->whereIn('tenant_id', $tenantIds) - ->whereIn('role', $roles) - ->pluck('tenant_id') - ->map(fn ($id): int => (int) $id) - ->all(); - - return array_values(array_unique($ids)); - } -} diff --git a/app/Support/OperationCatalog.php b/app/Support/OperationCatalog.php index d0009ba..67ed751 100644 --- a/app/Support/OperationCatalog.php +++ b/app/Support/OperationCatalog.php @@ -32,6 +32,8 @@ public static function labels(): array 'backup_schedule_retention' => 'Backup schedule retention', 'backup_schedule_purge' => 'Backup schedule purge', 'restore.execute' => 'Restore execution', + 'assignments.fetch' => 'Assignment fetch', + 'assignments.restore' => 'Assignment restore', 'directory_role_definitions.sync' => 'Role definitions sync', 'restore_run.delete' => 'Delete restore runs', 'restore_run.restore' => 'Restore restore runs', @@ -64,6 +66,7 @@ public static function expectedDurationSeconds(string $operationType): ?int 'compliance.snapshot' => 180, 'entra_group_sync' => 120, 'drift_generate_findings' => 240, + 'assignments.fetch', 'assignments.restore' => 60, default => null, }; } diff --git a/app/Support/Rbac/UiEnforcement.php b/app/Support/Rbac/UiEnforcement.php index a1ef87d..769d456 100644 --- a/app/Support/Rbac/UiEnforcement.php +++ b/app/Support/Rbac/UiEnforcement.php @@ -188,6 +188,39 @@ public function apply(): Action|BulkAction return $this->action; } + /** + * Evaluate whether a bulk selection is authorized (all-or-nothing). + * + * - If any selected tenant is not a membership: false. + * - If all are memberships but any lacks capability: false. + */ + public function bulkSelectionIsAuthorized(User $user, Collection $records): bool + { + $tenantIds = $this->resolveTenantIdsFromRecords($records); + + if ($tenantIds === []) { + return false; + } + + /** @var CapabilityResolver $resolver */ + $resolver = app(CapabilityResolver::class); + $resolver->primeMemberships($user, $tenantIds); + + foreach ($tenantIds as $tenantId) { + $tenant = $this->makeTenantStub($tenantId); + + if (! $resolver->isMember($user, $tenant)) { + return false; + } + + if ($this->capability !== null && ! $resolver->can($user, $tenant, $this->capability)) { + return false; + } + } + + return true; + } + /** * Hide action for non-members. * @@ -286,6 +319,19 @@ private function applyDisabledState(): void $tooltip = $this->customTooltip ?? AuthUiTooltips::insufficientPermission(); $this->action->disabled(function (?Model $record = null) { + if ($this->isBulk && $this->action instanceof BulkAction) { + $user = auth()->user(); + + if (! $user instanceof User) { + return true; + } + + /** @var Collection $records */ + $records = collect($this->action->getSelectedRecords()); + + return ! $this->bulkSelectionIsAuthorized($user, $records); + } + $context = $this->resolveContextWithRecord($record); // Non-members are hidden, so this only affects members @@ -298,6 +344,23 @@ private function applyDisabledState(): void // Only show tooltip when actually disabled $this->action->tooltip(function (?Model $record = null) use ($tooltip) { + if ($this->isBulk && $this->action instanceof BulkAction) { + $user = auth()->user(); + + if (! $user instanceof User) { + return $tooltip; + } + + /** @var Collection $records */ + $records = collect($this->action->getSelectedRecords()); + + if (! $this->bulkSelectionIsAuthorized($user, $records)) { + return $tooltip; + } + + return null; + } + $context = $this->resolveContextWithRecord($record); if ($context->isMember && ! $context->hasCapability) { @@ -332,6 +395,21 @@ private function applyDestructiveConfirmation(): void private function applyServerSideGuard(): void { $this->action->before(function (?Model $record = null): void { + if ($this->isBulk && $this->action instanceof BulkAction) { + $user = auth()->user(); + + if (! $user instanceof User) { + abort(403); + } + + /** @var Collection $records */ + $records = collect($this->action->getSelectedRecords()); + + $this->authorizeBulkSelectionOrAbort($user, $records); + + return; + } + $context = $this->resolveContextWithRecord($record); // Non-member → 404 (deny-as-not-found) @@ -346,6 +424,99 @@ private function applyServerSideGuard(): void }); } + /** + * Server-side enforcement for bulk selections. + * + * - If any selected tenant is not a membership: 404 (deny-as-not-found). + * - If all are memberships but any lacks capability: 403. + */ + private function authorizeBulkSelectionOrAbort(User $user, Collection $records): void + { + $tenantIds = $this->resolveTenantIdsFromRecords($records); + + if ($tenantIds === []) { + abort(403); + } + + /** @var CapabilityResolver $resolver */ + $resolver = app(CapabilityResolver::class); + $resolver->primeMemberships($user, $tenantIds); + + foreach ($tenantIds as $tenantId) { + $tenant = $this->makeTenantStub($tenantId); + + if (! $resolver->isMember($user, $tenant)) { + abort(404); + } + } + + if ($this->capability === null) { + return; + } + + foreach ($tenantIds as $tenantId) { + $tenant = $this->makeTenantStub($tenantId); + + if (! $resolver->can($user, $tenant, $this->capability)) { + abort(403); + } + } + } + + /** + * @param Collection $records + * @return array + */ + private function resolveTenantIdsFromRecords(Collection $records): array + { + $tenantIds = []; + + foreach ($records as $record) { + if ($record instanceof Tenant) { + $tenantIds[] = (int) $record->getKey(); + + continue; + } + + if ($record instanceof Model) { + $tenantId = $record->getAttribute('tenant_id'); + if ($tenantId !== null) { + $tenantIds[] = (int) $tenantId; + + continue; + } + + if (method_exists($record, 'relationLoaded') && $record->relationLoaded('tenant')) { + $relatedTenant = $record->getRelation('tenant'); + + if ($relatedTenant instanceof Tenant) { + $tenantIds[] = (int) $relatedTenant->getKey(); + + continue; + } + } + } + } + + if ($tenantIds === []) { + $tenant = Filament::getTenant(); + if ($tenant instanceof Tenant) { + $tenantIds[] = (int) $tenant->getKey(); + } + } + + return array_values(array_unique($tenantIds)); + } + + private function makeTenantStub(int $tenantId): Tenant + { + $tenant = new Tenant; + $tenant->forceFill(['id' => $tenantId]); + $tenant->exists = true; + + return $tenant; + } + /** * Resolve the current access context with an optional record. */ diff --git a/composer.json b/composer.json index 3478e5d..29c045c 100644 --- a/composer.json +++ b/composer.json @@ -55,6 +55,9 @@ "@php artisan config:clear --ansi", "@php artisan test" ], + "test:pgsql": [ + "@php vendor/bin/pest -c phpunit.pgsql.xml" + ], "post-autoload-dump": [ "Illuminate\\Foundation\\ComposerScripts::postAutoloadDump", "@php artisan package:discover --ansi", diff --git a/database/migrations/2026_02_15_005041_ensure_workspace_id_fks_on_tenant_owned_tables.php b/database/migrations/2026_02_15_005041_ensure_workspace_id_fks_on_tenant_owned_tables.php new file mode 100644 index 0000000..5b491b3 --- /dev/null +++ b/database/migrations/2026_02_15_005041_ensure_workspace_id_fks_on_tenant_owned_tables.php @@ -0,0 +1,102 @@ + + */ + private function tenantOwnedTables(): array + { + return [ + 'policies', + 'policy_versions', + 'backup_sets', + 'backup_items', + 'restore_runs', + 'backup_schedules', + 'inventory_items', + 'inventory_links', + 'entra_groups', + 'findings', + 'entra_role_definitions', + 'tenant_permissions', + ]; + } + + private function ensureWorkspaceForeignKey(string $tableName, string $constraintName): void + { + // Add as NOT VALID (fast) then VALIDATE (safe) — Postgres best practice. + DB::unprepared(<<tenantOwnedTables() as $tableName) { + $constraintName = sprintf('%s_workspace_fk', $tableName); + + $this->ensureWorkspaceForeignKey($tableName, $constraintName); + } + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + if (DB::getDriverName() !== 'pgsql') { + return; + } + + foreach ($this->tenantOwnedTables() as $tableName) { + $constraintName = sprintf('%s_workspace_fk', $tableName); + + DB::unprepared(<< + + + + tests/Unit + + + tests/Feature + + + + + + app + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/routes/web.php b/routes/web.php index e84aa68..c92d6c9 100644 --- a/routes/web.php +++ b/routes/web.php @@ -117,7 +117,7 @@ return $workspace; }); -Route::middleware(['web', 'auth', 'ensure-workspace-member']) +Route::middleware(['web', 'auth', 'ensure-correct-guard:web', 'ensure-workspace-member']) ->prefix('/admin/w/{workspace}') ->group(function (): void { Route::get('/', fn () => redirect()->route('admin.workspace.managed-tenants.index', ['workspace' => request()->route('workspace')])) diff --git a/specs/094-assignment-ops-observability-hardening/checklists/requirements.md b/specs/094-assignment-ops-observability-hardening/checklists/requirements.md new file mode 100644 index 0000000..5fc00fb --- /dev/null +++ b/specs/094-assignment-ops-observability-hardening/checklists/requirements.md @@ -0,0 +1,35 @@ +# Specification Quality Checklist: 094 Assignment Ops Observability Hardening + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-02-14 +**Feature**: [specs/094-assignment-ops-observability-hardening/spec.md](specs/094-assignment-ops-observability-hardening/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 + +- Spec is intentionally ship-safety hardening only; no new domain features. +- Spec was rewritten to follow the official template structure and removed code/file/class references. diff --git a/specs/094-assignment-ops-observability-hardening/contracts/assignment-ops.openapi.yaml b/specs/094-assignment-ops-observability-hardening/contracts/assignment-ops.openapi.yaml new file mode 100644 index 0000000..3c9e34c --- /dev/null +++ b/specs/094-assignment-ops-observability-hardening/contracts/assignment-ops.openapi.yaml @@ -0,0 +1,127 @@ +openapi: 3.0.3 +info: + title: TenantPilot - Assignment Operations (Internal) + version: "1.0" + description: | + Internal contract describing the user-triggered operation start surfaces and Monitoring read surfaces + relevant to assignment fetch/restore observability. + +servers: + - url: / + +paths: + /admin/t/{tenant}/backup-items/{backupItem}/assignments/fetch: + post: + summary: Start assignment fetch/enrichment + parameters: + - in: path + name: tenant + required: true + schema: { type: string } + - in: path + name: backupItem + required: true + schema: { type: integer } + responses: + "202": + description: Accepted; operation run created/reused and job enqueued + content: + application/json: + schema: + $ref: "#/components/schemas/OperationRunStartResponse" + "403": { description: Forbidden (member missing capability) } + "404": { description: Not found (non-member / wrong plane) } + + /admin/t/{tenant}/restore-runs/{restoreRun}/assignments/restore: + post: + summary: Start assignment restore + parameters: + - in: path + name: tenant + required: true + schema: { type: string } + - in: path + name: restoreRun + required: true + schema: { type: integer } + responses: + "202": + description: Accepted; operation run created/reused and job enqueued + content: + application/json: + schema: + $ref: "#/components/schemas/OperationRunStartResponse" + "403": { description: Forbidden (member missing capability) } + "404": { description: Not found (non-member / wrong plane) } + + /admin/monitoring/operations: + get: + summary: List operation runs (Monitoring) + responses: + "200": + description: OK + content: + application/json: + schema: + type: array + items: + $ref: "#/components/schemas/OperationRunSummary" + + /admin/monitoring/operations/{operationRun}: + get: + summary: Get operation run detail (Monitoring) + parameters: + - in: path + name: operationRun + required: true + schema: { type: integer } + responses: + "200": + description: OK + content: + application/json: + schema: + $ref: "#/components/schemas/OperationRunDetail" + "404": { description: Not found (non-member / no entitlement) } + +components: + schemas: + OperationRunStartResponse: + type: object + required: [operationRunId] + properties: + operationRunId: + type: integer + + OperationRunSummary: + type: object + required: [id, type, status, outcome, createdAt] + properties: + id: { type: integer } + type: { type: string } + status: { type: string } + outcome: { type: string } + createdAt: { type: string, format: date-time } + + OperationRunDetail: + allOf: + - $ref: "#/components/schemas/OperationRunSummary" + - type: object + properties: + startedAt: { type: string, format: date-time, nullable: true } + completedAt: { type: string, format: date-time, nullable: true } + context: + type: object + additionalProperties: true + failures: + type: array + items: + $ref: "#/components/schemas/FailureItem" + + FailureItem: + type: object + required: [code, reasonCode, message] + properties: + code: { type: string } + reasonCode: { type: string } + message: { type: string } diff --git a/specs/094-assignment-ops-observability-hardening/data-model.md b/specs/094-assignment-ops-observability-hardening/data-model.md new file mode 100644 index 0000000..30c3f51 --- /dev/null +++ b/specs/094-assignment-ops-observability-hardening/data-model.md @@ -0,0 +1,50 @@ +# Data Model — 094 Assignment Operations Observability Hardening + +This feature introduces **no new entities**. It strengthens how existing operational artifacts are created, deduplicated, and displayed. + +## Entities (existing) + +### OperationRun + +Represents a single observable operation for Monitoring. + +- Ownership: + - `workspace_id` required. + - `tenant_id` present for tenant-bound runs. +- Key fields: + - `type`: operation type identifier (string). + - `status`: queued/running/completed. + - `outcome`: pending/succeeded/failed/etc. + - `run_identity_hash`: stable identity used for active-run dedupe. + - `context`: structured details for Monitoring (inputs, progress, counters). + - `failures`: array of failure items with stable `code`, normalized `reason_code`, and sanitized `message`. + - Counter-like fields may be stored in `context` depending on existing conventions. + +### AuditLog + +Immutable audit record for security/ops-relevant mutations. + +- Scope invariant: if `tenant_id` is present, `workspace_id` must also be present. +- This spec requires exactly one audit entry per assignment restore execution. + +### RestoreRun + +Represents a restore execution request and its lifecycle. Assignment restores are performed under an existing restore run. + +### BackupItem + +Represents an item within a backup set; assignment fetch/enrichment is performed for a specific backup item. + +## Relationships (conceptual) + +- One tenant has many operation runs. +- One restore run should be associated with one or more operation runs depending on orchestration; this spec requires that assignment restore execution is observable via at least one operation run. +- One backup item can have a related fetch/enrichment operation run. + +## State transitions + +### OperationRun lifecycle + +- `queued` → `running` → `completed` +- Outcome set on completion to indicate success or failure. + diff --git a/specs/094-assignment-ops-observability-hardening/plan.md b/specs/094-assignment-ops-observability-hardening/plan.md new file mode 100644 index 0000000..51be80f --- /dev/null +++ b/specs/094-assignment-ops-observability-hardening/plan.md @@ -0,0 +1,174 @@ +# Implementation Plan: 094 Assignment Operations Observability Hardening + +**Branch**: `094-assignment-ops-observability-hardening` | **Date**: 2026-02-15 | **Spec**: [spec.md](spec.md) +**Input**: Feature specification from [spec.md](spec.md) + +## Summary + +This work hardens ship-safety by making assignment fetch/restore executions fully observable in Monitoring (via `OperationRun`), improving Graph testability by using the Graph client interface, and closing a small set of authorization inconsistencies (cross-plane guard leak, policy bypass, and 404/403 ordering). + +## Technical Context + +**Language/Version**: PHP 8.4 (Laravel 12) +**Primary Dependencies**: Filament v5, Livewire v4, Laravel Sail, Microsoft Graph integration +**Storage**: PostgreSQL (Sail) +**Testing**: Pest v4 (`./vendor/bin/sail artisan test`) +**Target Platform**: Linux container runtime (Sail/Dokploy) +**Project Type**: Laravel web application (admin UI via Filament) +**Performance Goals**: Monitoring pages must remain DB-only and render quickly (no external calls during render). +**Constraints**: No secrets/tokens in logs or run failures; RBAC-UX semantics must match constitution (404 vs 403). +**Scale/Scope**: Small hardening change set; no new domain entities; focused on two jobs + a few UI/policy surfaces. + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +- Inventory-first: PASS (no inventory semantics changed). +- Read/write separation: PASS (restore remains a user-triggered operation; change is observability/auditability). +- Graph contract path: PASS-BY-DESIGN (plan includes removing concrete client injections in assignment-related services). +- Deterministic capabilities: PASS (no new capability model; micro-fixes ensure enforcement uses canonical patterns). +- RBAC-UX planes: PASS (plan includes closing remaining cross-plane guard leak; non-member 404, member missing capability 403). +- Workspace isolation: PASS (operations remain tenant-bound; Monitoring remains DB-only). +- Destructive confirmations: PASS (no new destructive actions added; enforcement fixes must preserve confirmations). +- Global search safety: N/A (no global search changes). +- Tenant isolation: PASS (no cross-tenant views added; monitoring surfaces already entitlement-checked). +- Run observability: PASS-BY-DESIGN (this spec exists to bring remaining assignment jobs under `OperationRun`). +- Automation: PASS (dedupe identity clarified; existing partial unique index patterns used). +- Data minimization: PASS (failures must be sanitized; no raw payloads stored). +- Badge semantics: N/A (no badge mapping changes). +- Filament UI Action Surface Contract: PASS (no new resources; micro-fixes must not remove inspect affordances). + +## Project Structure + +### Documentation (this feature) + +```text +specs/094-assignment-ops-observability-hardening/ +├── plan.md +├── spec.md +├── tasks.md +├── research.md +├── data-model.md +├── quickstart.md +├── contracts/ +│ └── assignment-ops.openapi.yaml +└── checklists/ + └── requirements.md +``` + +### Source Code (repository root) + +```text +app/ +├── Jobs/ +│ ├── FetchAssignmentsJob.php +│ ├── RestoreAssignmentsJob.php +│ └── Middleware/TrackOperationRun.php +├── Services/ +│ ├── OperationRunService.php +│ ├── AssignmentBackupService.php +│ ├── AssignmentRestoreService.php +│ └── Graph/ +│ ├── GraphClientInterface.php +│ ├── MicrosoftGraphClient.php +│ ├── NullGraphClient.php +│ ├── AssignmentFetcher.php +│ ├── AssignmentFilterResolver.php +│ └── GroupResolver.php +├── Filament/ +│ └── Resources/... +└── Http/ + └── Middleware/EnsureCorrectGuard.php + +routes/ +└── web.php + +tests/ +├── Feature/ +└── Unit/ +``` + +**Structure Decision**: Use the existing Laravel structure; changes are limited to the job layer, Graph service DI, a few Filament surfaces, and targeted Pest tests. + +## Phase 0 — Outline & Research + +Artifacts: + +- [research.md](research.md) + +Research outcomes (resolved decisions): + +- Operation run identity & dedupe: tenant + type + target/scope. +- Counters semantics: total attempted, processed succeeded, failed failed. +- Failure convention: operation-specific `code` + normalized `reason_code`. +- Audit log granularity: exactly one entry per restore execution. + +## Phase 1 — Design & Contracts + +Artifacts: + +- [data-model.md](data-model.md) +- [contracts/assignment-ops.openapi.yaml](contracts/assignment-ops.openapi.yaml) +- [quickstart.md](quickstart.md) + +Design notes: + +- No new persistent entities. +- Operation run tracking must use existing dedupe/index patterns. +- Monitoring surfaces remain DB-only. + +## Phase 1 — Agent Context Update + +Run: + +- `.specify/scripts/bash/update-agent-context.sh copilot` + +## Phase 1 — Constitution Check (re-evaluation) + +Re-check result: PASS expected once implementation removes concrete Graph client injections and adds OperationRun tracking for the two remaining assignment jobs. + +## Phase 2 — Implementation Plan + +### Step 1 — OperationRun tracking for assignment fetch/restore + +- Locate job dispatch/start surfaces. +- Ensure each execution creates/reuses an `OperationRun`: + - Dedupe identity: + - Fetch: tenant + type + backup item (or equivalent policy-version identifier). + - Restore: tenant + type + restore run identifier. +- Ensure Tracking middleware is applied and the job exposes a run handle per existing conventions. +- Ensure failure details: + - `code`: operation-specific namespace. + - `reason_code`: normalized cause. + - message: sanitized. +- Ensure counters match the agreed semantics. + +### Step 2 — Audit log for assignment restore execution + +- Ensure exactly one audit log entry is written per assignment restore execution. + +### Step 3 — Graph client interface enforcement + +- Update assignment-related services to accept the Graph client interface. + +### Step 4 — Authorization micro-fixes + +- Close cross-plane guard leak on workspace-scoped admin routes. +- Remove any policy bypasses on Provider Connections list surfaces. +- Fix membership (404) vs capability (403) ordering for backup item surfaces. +- Ensure legacy UI enforcement helpers are not used where the canonical helper exists. + +### Step 5 — Tests + formatting + +- Add targeted Pest regression coverage for: + - operation run tracking (success/failure) + - guard leak + - policy enforcement + - 404/403 ordering + - Graph interface mockability +- Run `./vendor/bin/sail bin pint --dirty`. +- Run targeted tests then widen as needed. + +## Complexity Tracking + +No constitution violations are required for this feature. diff --git a/specs/094-assignment-ops-observability-hardening/quickstart.md b/specs/094-assignment-ops-observability-hardening/quickstart.md new file mode 100644 index 0000000..c55f8ed --- /dev/null +++ b/specs/094-assignment-ops-observability-hardening/quickstart.md @@ -0,0 +1,33 @@ +# Quickstart — 094 Assignment Operations Observability Hardening + +## Prerequisites + +- Docker + Laravel Sail +- PHP/Composer dependencies installed (via Sail) + +## Setup + +- Start services: `./vendor/bin/sail up -d` +- Install PHP deps (if needed): `./vendor/bin/sail composer install` +- Run migrations: `./vendor/bin/sail artisan migrate` + +## Running tests (targeted) + +Run the smallest set first, then widen: + +- Feature tests added for this spec (once implemented): + - `./vendor/bin/sail artisan test --compact tests/Feature/Operations` + - `./vendor/bin/sail artisan test --compact tests/Feature/Auth` + - `./vendor/bin/sail artisan test --compact tests/Feature/Rbac` + +## Formatting + +- Run Pint on touched files: `./vendor/bin/sail bin pint --dirty` + +## Manual verification (admin) + +- Trigger an assignment fetch and confirm a Monitoring → Operations entry appears. +- Trigger an assignment restore and confirm: + - Monitoring shows a run with failure codes (if any) and counters. + - Exactly one audit log entry is created for the restore execution. + diff --git a/specs/094-assignment-ops-observability-hardening/research.md b/specs/094-assignment-ops-observability-hardening/research.md new file mode 100644 index 0000000..1e07853 --- /dev/null +++ b/specs/094-assignment-ops-observability-hardening/research.md @@ -0,0 +1,59 @@ +# Research — 094 Assignment Operations Observability Hardening + +This document resolves implementation-relevant questions raised by the spec and records the key decisions. + +## Decision 1 — Operation run identity + dedupe + +- Decision: Use **dedupe per tenant + operation type + operation target/scope**. + - Fetch target/scope: backup item identifier (or equivalent policy-version identifier). + - Restore target/scope: restore run identifier. +- Rationale: Prevents operator confusion while allowing independent operations to proceed in parallel. +- Alternatives considered: + - Dedupe per tenant only → collapses unrelated runs and obscures what is actually running. + - No dedupe → increases confusion and makes tests non-deterministic. + +## Decision 2 — How jobs become OperationRun-tracked + +- Decision: Prefer passing an existing operation run identifier into queued jobs when a user-triggered start surface exists; otherwise the job must create/reuse a canonical run using the standard service. +- Rationale: Start surfaces should remain enqueue-only while ensuring a single umbrella run record exists for Monitoring. +- Alternatives considered: + - Create runs only inside jobs → makes it harder to link UI initiation to the run and can lead to duplicate runs. + +## Decision 3 — Counters semantics + +- Decision: `total` = items attempted, `processed` = succeeded, `failed` = failed. +- Rationale: Works for both read-only fetch and destructive restore, and is easy to interpret in Monitoring. +- Alternatives considered: + - “total discovered” semantics → ambiguous when discovery and execution are separate steps. + - Different semantics per operation type → harder to reason about and to test. + +## Decision 4 — Failure structure: stable code + normalized reason_code + +- Decision: Use operation-specific failure `code` namespaces (e.g., `assignments.fetch_failed`, `assignments.restore_failed`) and store the underlying cause as a normalized `reason_code`. +- Rationale: Operators can identify what failed (operation) and why (normalized cause) consistently. +- Alternatives considered: + - Generic failure codes only → loses context; Monitoring becomes less actionable. + - Reusing unrelated restore codes for assignment operations → conflates domains and increases ambiguity. + +## Decision 5 — Audit log granularity for assignment restores + +- Decision: Write **exactly one audit log entry per assignment restore execution** (per restore run). +- Rationale: Satisfies auditability while keeping log volume predictable and reviewable. +- Alternatives considered: + - Per-item audit logs → noisy for large restores. + - Both summary + per-item → overkill for this hardening scope. + +## Decision 6 — Graph client abstraction + +- Decision: Ensure assignment-related services depend on the **Graph client interface**, not a concrete implementation. +- Rationale: Enables deterministic non-production tests and allows safe stub/null implementations. +- Alternatives considered: + - Concrete client type-hints → blocks mocking and makes tests fragile. + +## Decision 7 — Authorization semantics & cross-plane safety + +- Decision: Enforce deny-as-not-found (404) for non-members/cross-plane access and forbidden (403) for members lacking capability. +- Rationale: Matches constitution RBAC-UX rules and prevents route/resource enumeration. +- Alternatives considered: + - Using 403 for non-members → leaks existence and violates deny-as-not-found standard. + diff --git a/specs/094-assignment-ops-observability-hardening/spec.md b/specs/094-assignment-ops-observability-hardening/spec.md new file mode 100644 index 0000000..28f823b --- /dev/null +++ b/specs/094-assignment-ops-observability-hardening/spec.md @@ -0,0 +1,137 @@ +# Feature Specification: Assignment Operations Observability Hardening + +**Feature Branch**: `094-assignment-ops-observability-hardening` +**Created**: 2026-02-14 +**Status**: Draft +**Input**: User description: "Harden assignment operation observability and close remaining authorization inconsistencies so operations are fully traceable, diagnosable, and correctly access-controlled." + +## Spec Scope Fields *(mandatory)* + +- **Scope**: canonical-view +- **Primary Routes**: + - Admin Monitoring → Operations (list + detail views) + - Provider Connections list + - Backup Sets list + - Restore Runs list + - Backup Items relationship view under a backup set +- **Data Ownership**: operational activity records must be scoped to the correct workspace and tenant context where applicable; no new domain entities are introduced. +- **RBAC**: all affected admin surfaces require workspace membership; actions that change configuration or trigger restores require explicit permissions as defined in the central capability registry. + +### Canonical-view Constraints + +- **Default behavior when tenant-context is active**: Monitoring → Operations defaults to showing operational records for the currently selected tenant context (if one is selected). +- **Entitlement checks**: Monitoring → Operations MUST not reveal tenant-owned operational records unless the actor is entitled to that tenant scope within the active workspace context. + +## Clarifications + +### Session 2026-02-14 + +- Q: For FR-005 / SC-004, what should “identity scope” mean for deduping “active” runs? → A: Dedupe per tenant and operation target/scope. +- Q: For FR-003 (“summary counters”) what exact semantics should total / processed / failed follow? → A: total = items attempted; processed = succeeded; failed = failed. +- Q: For FR-004 (“stable failure code”), which convention should we standardize on for assignment fetch/restore runs? → A: Operation-specific code namespaces + normalized reason_code for the cause. +- Q: For FR-006 (identity scope = tenant + operation type + target/scope), which identifiers should define “target/scope” for these two operations? → A: Fetch targets the backup item (or policy version) identifier; restore targets the restore run identifier. +- Q: For US1 / FR-002 (“restore is observable/auditable”), what audit log granularity do you want for assignment restore? → A: One audit log entry per assignment restore execution (per restore run). + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 — Observe assignment operations end-to-end (Priority: P1) + +Workspace administrators need to see assignment-related operations (both read-only fetch and destructive restore) in Monitoring so they can confirm what ran, what changed, and why something failed, without relying on server logs. + +**Why this priority**: Assignment restore is high-risk; missing visibility creates operational and audit gaps. + +**Independent Test**: Trigger both an assignment fetch and an assignment restore; verify each produces a monitoring-visible run record with correct lifecycle and failure details. + +**Acceptance Scenarios**: + +1. **Given** an administrator triggers an assignment fetch, **When** the operation starts and completes, **Then** Monitoring shows a run record with start/end timestamps, final outcome, and summary counters. +2. **Given** an administrator triggers an assignment restore, **When** the operation starts and completes, **Then** Monitoring shows a run record including a clear indication that it was a change-making operation. + - And exactly one audit log entry is written for the restore execution. +3. **Given** an assignment fetch or restore fails due to an external dependency error, **When** the run completes, **Then** Monitoring shows a stable failure code and a sanitized, user-readable message. +4. **Given** the same assignment operation is triggered multiple times concurrently for the same tenant and scope, **When** the system creates tracking records, **Then** the admin sees a single “active” run per identity (or an equivalent deduped representation). + +--- + +### User Story 2 — Enforce correct access control semantics on affected admin surfaces (Priority: P2) + +Workspace administrators and platform operators must not be able to cross authentication “planes” accidentally, and the admin UI must not expose bypasses that let users initiate sensitive actions without authorization. + +**Why this priority**: Prevents cross-plane leakage and closes known authorization inconsistencies. + +**Independent Test**: Attempt access with the wrong authentication plane and with insufficient permissions; verify outcomes are deny-as-not-found (404) or forbidden (403) per policy. + +**Acceptance Scenarios**: + +1. **Given** a user is authenticated in a different auth plane, **When** they attempt to access workspace-scoped admin routes, **Then** the response is deny-as-not-found (404). +2. **Given** a user is not a member of the workspace, **When** they attempt to view backup items under a backup set, **Then** the response is deny-as-not-found (404) and does not reveal record existence. +3. **Given** a user is a workspace member but lacks the required permission, **When** they attempt a protected action (such as managing provider connections), **Then** the response is forbidden (403). + +--- + +### User Story 3 — Validate assignment operations safely in non-production contexts (Priority: P3) + +Platform engineers need to validate that assignment operations behave correctly without requiring live external dependencies, so regressions can be caught early. + +**Why this priority**: Improves reliability and reduces the risk of shipping changes that only fail when external services are slow/unavailable. + +**Independent Test**: Run automated tests that simulate both successful and failing external interactions; verify monitoring records and authorization behaviors. + +**Acceptance Scenarios**: + +1. **Given** external interactions are simulated as “successful”, **When** the operation runs, **Then** the run is marked successful and includes expected summary counters. +2. **Given** external interactions are simulated as “failing”, **When** the operation runs, **Then** the run is marked failed with a stable failure code and sanitized message. + +### Edge Cases + +- External dependency timeouts, throttling, or transient failures. +- Retries and duplicate dispatches (ensure tracking remains coherent and non-spammy). +- Missing or inconsistent tenant/workspace context (must fail safely, not leak). +- Partial completion (some items processed, some failed): counters and failure details must remain interpretable. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: The system MUST create and maintain an operational tracking record (OperationRun) for every assignment fetch operation execution. +- **FR-002**: The system MUST create and maintain an operational tracking record (OperationRun) for every assignment restore operation execution. +- **FR-003**: Tracking records MUST include lifecycle state (queued/running/completed), timestamps, outcome, and summary counters sufficient to understand progress and results. + - Counter semantics: `total` = items attempted, `processed` = succeeded, `failed` = failed. +- **FR-004**: Failed runs MUST include a stable failure code and a sanitized, user-readable failure message. + - Failure convention: use operation-specific `code` namespaces for the run, and store the underlying cause as a normalized `reason_code`. +- **FR-005**: The system MUST prevent duplicate “active” runs for the same tenant and identity scope (tenant + operation type + operation target/scope), or otherwise present a deduped representation that avoids operator confusion. +- **FR-006**: Identity scope MUST be defined as: + - For assignment fetch operations: tenant + operation type + backup item identifier (or equivalent policy-version identifier). + - For assignment restore operations: tenant + operation type + restore run identifier. +- **FR-007**: Monitoring pages MUST render using persisted operational data only (no outbound calls during page render). +- **FR-008**: Cross-plane access MUST be deny-as-not-found (404) on affected routes. +- **FR-009**: Authorization MUST follow consistent semantics: + - non-member / not entitled → 404 (deny-as-not-found) + - member without required permission → 403 +- **FR-010**: Any action that can change configuration or trigger a restore MUST be server-authorized; UI visibility MUST NOT be treated as authorization. +- **FR-011**: Sensitive or destructive-like actions MUST require explicit confirmation. +- **FR-012**: Each assignment restore execution MUST write exactly one audit log entry for the restore run. + +### Dependencies & Assumptions + +- Assignment fetch and assignment restore operations can be triggered by administrators or scheduled/queued execution paths. +- Monitoring users have access to the Operations area only when they have appropriate workspace membership and permissions. +- External dependency failures may occur and must be represented consistently (stable failure codes + sanitized messages). + +## UI Action Matrix *(mandatory when the admin UI is changed)* + +| 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 | +|---|---|---|---|---|---|---|---|---|---|---| +| Provider Connections | Admin → Provider Connections | Create connection (permission-gated) | View connection details | Edit / Delete (permission-gated; destructive confirms) | None | Create connection | N/A | Save + Cancel | Yes | Ensures no authorization bypass exists on header/empty-state CTAs. | +| Backup Sets | Admin → Backups | None (unchanged) | View backup set | Actions unchanged | None | None | N/A | N/A | N/A | Only enforcement helper consistency is affected. | +| Restore Runs | Admin → Restores | None (unchanged) | View restore run | Actions unchanged | None | None | N/A | N/A | Yes | Restore operations must be observable via Monitoring. | +| Backup Items | Under a backup set | None | View backup item details | Actions unchanged | None | None | N/A | N/A | N/A | Membership/404 checks must occur before capability/403 checks. | +| Monitoring → Operations | Admin → Monitoring | None | View operation run details | None | None | None | N/A | N/A | Yes | Read-only view; must not call external services during render. | + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: 100% of assignment fetch and assignment restore executions appear in Monitoring with a completed outcome (success or failure) and timestamps. +- **SC-002**: For failed runs, operators can identify a stable failure code and a readable failure message from Monitoring within 60 seconds, without checking server logs. +- **SC-003**: Automated tests verify deny-as-not-found (404) vs forbidden (403) semantics for the affected surfaces. +- **SC-004**: Duplicate active-run confusion is eliminated: repeated triggers produce a single active run per identity scope (tenant + operation type + operation target/scope) or equivalent deduped visibility. diff --git a/specs/094-assignment-ops-observability-hardening/tasks.md b/specs/094-assignment-ops-observability-hardening/tasks.md new file mode 100644 index 0000000..8800cd5 --- /dev/null +++ b/specs/094-assignment-ops-observability-hardening/tasks.md @@ -0,0 +1,172 @@ +# Tasks: 094 Assignment Operations Observability Hardening + +**Input**: Design documents from `specs/094-assignment-ops-observability-hardening/` +**Prerequisites**: `specs/094-assignment-ops-observability-hardening/plan.md` (required), `specs/094-assignment-ops-observability-hardening/spec.md` (required), `specs/094-assignment-ops-observability-hardening/research.md`, `specs/094-assignment-ops-observability-hardening/data-model.md`, `specs/094-assignment-ops-observability-hardening/contracts/assignment-ops.openapi.yaml` + +**Tests**: Required (Pest) for runtime behavior changes. + +--- + +## Phase 1: Setup (Shared Infrastructure) + +**Purpose**: Ensure the implementation has the correct local workflow context and that the existing conventions to extend are understood. + +- [X] T001 Run SpecKit prerequisites check via `.specify/scripts/bash/check-prerequisites.sh --json` and record `FEATURE_DIR` + `AVAILABLE_DOCS` for this feature +- [X] T002 Review existing OperationRun tracking conventions in `app/Jobs/Middleware/TrackOperationRun.php` and `app/Services/OperationRunService.php` +- [X] T003 Review existing OperationRun test patterns in `tests/Feature/TrackOperationRunMiddlewareTest.php` and `tests/Feature/OperationRunServiceTest.php` + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**Purpose**: Confirm the persistence + dedupe primitives exist and match the spec’s requirements before adding new run types and audits. + +**⚠️ CRITICAL**: Complete this phase before implementing any user story. + +- [X] T004 Verify `operation_runs` schema supports `run_identity_hash`, `summary_counts`, and `failure_summary` per `database/migrations/2026_01_16_180642_create_operation_runs_table.php` +- [X] T005 Verify active-run dedupe constraints exist and align with `OperationRunService::ensureRunWithIdentity()` in `app/Services/OperationRunService.php` and `database/migrations/2026_02_10_004939_add_unique_index_for_backup_schedule_scheduled_operation_runs.php` +- [X] T006 [P] Identify existing restore audit logging expectations to preserve by reviewing `tests/Feature/RestoreAuditLoggingTest.php` and `app/Services/Intune/AuditLogger.php` +- [X] T034 [P] Verify active-run dedupe constraints cover the new assignment run identity shapes; if not, add a DB-enforced partial unique index/migration + regression test for dedupe behavior + +**Checkpoint**: Foundation ready — user story work can begin. + +--- + +## Phase 3: User Story 1 — Observe assignment operations end-to-end (Priority: P1) 🎯 MVP + +**Goal**: Assignment fetch (during backup capture when assignments are included) and assignment restore (during restore execution) are observable via `OperationRun` with stable failure codes, normalized `reason_code`, correct counters, and exactly one audit log entry per assignment restore execution. + +**Independent Test**: Trigger (a) backup creation with assignments included and (b) a restore that applies assignments; verify `operation_runs` rows exist and `audit_logs` contains exactly one assignment-restore summary entry. + +### Tests for User Story 1 (write first) + +- [X] T007 [P] [US1] Add test for assignment restore OperationRun lifecycle + counters in `tests/Feature/Operations/AssignmentRestoreOperationRunTest.php` +- [X] T008 [P] [US1] Add test for stable failure `code` + normalized `reason_code` on assignment restore failure in `tests/Feature/Operations/AssignmentRestoreOperationRunFailureTest.php` +- [X] T009 [P] [US1] Add test that assignment restore writes exactly one audit log entry per restore execution in `tests/Feature/Audit/AssignmentRestoreAuditSummaryTest.php` +- [X] T010 [P] [US1] Add test that backup capture with assignments included records an assignment-fetch OperationRun entry for the resulting backup item in `tests/Feature/Operations/AssignmentFetchOperationRunTest.php` +- [X] T035 [P] [US1] Add regression test for Monitoring pages being DB-only at render time (no outbound calls) when viewing Operations list + detail in `tests/Feature/Monitoring/OperationsDbOnlyRenderTest.php` + +### Implementation for User Story 1 + +- [X] T011 [US1] Add assignment-restore OperationRun creation + dedupe by `restore_run_id` in `app/Services/Intune/RestoreService.php` (use `OperationRunService::ensureRunWithIdentity()`; identity inputs include `restore_run_id`) +- [X] T012 [US1] Update assignment-restore OperationRun summary counts (`total` attempted, `processed` succeeded, `failed` failed) in `app/Services/Intune/RestoreService.php` and `app/Services/OperationRunService.php` +- [X] T013 [US1] Record stable failure `code` namespaces + normalized `reason_code` + sanitized messages for assignment restore failures in `app/Services/Intune/RestoreService.php` (leverage `OperationRunService` failure sanitization) +- [X] T014 [US1] Remove per-assignment audit log emission from `app/Services/AssignmentRestoreService.php` (replace per-item `auditLogger->log(...)` calls with in-memory aggregation only) +- [X] T015 [US1] Add exactly one audit log entry per assignment restore execution in `app/Services/Intune/RestoreService.php` using `app/Services/Intune/AuditLogger.php` (resourceType `restore_run`, resourceId = restore run id) +- [X] T016 [US1] Add assignment-fetch OperationRun creation + dedupe keyed by `backup_item_id` in `app/Services/AssignmentBackupService.php` (wrap the Graph fetch inside `enrichWithAssignments()` using `OperationRunService::ensureRunWithIdentity()`) +- [X] T017 [US1] Ensure assignment-related job classes can be OperationRun-tracked if they are used in the future by adding `public ?OperationRun $operationRun` + `middleware()` returning `TrackOperationRun` in `app/Jobs/FetchAssignmentsJob.php` and `app/Jobs/RestoreAssignmentsJob.php` +- [X] T036 [US1] Identify and document all start surfaces that can trigger assignment fetch/restore, and ensure each path creates/reuses the same run identity (avoid “tracked in one path, untracked in another” gaps) + +**Checkpoint**: US1 delivers Monitoring visibility + auditability for assignment restore and assignment fetch during backup capture. + +--- + +## Phase 4: User Story 2 — Enforce correct access control semantics on affected admin surfaces (Priority: P2) + +**Goal**: Cross-plane access returns 404, non-member access returns 404, and member-without-capability returns 403. Remove any authorization bypasses. + +**Independent Test**: Access the affected routes/surfaces with wrong guard and with insufficient permissions; assert 404 vs 403 semantics. + +### Tests for User Story 2 (write first) + +- [X] T018 [P] [US2] Add regression test for `/admin/w/{workspace}` guard enforcement (cross-plane 404) in `tests/Feature/Guards/AdminWorkspaceRoutesGuardTest.php` +- [X] T019 [P] [US2] Add regression test ensuring Provider Connection create CTA does not bypass authorization in `tests/Feature/ProviderConnections/ProviderConnectionListAuthorizationTest.php` +- [X] T020 [P] [US2] Add regression test for membership (404) before capability (403) in backup items relation manager in `tests/Feature/Rbac/BackupItemsRelationManagerSemanticsTest.php` + +### Implementation for User Story 2 + +- [X] T021 [US2] Add missing `ensure-correct-guard:web` middleware to the `/admin/w/{workspace}` route group in `routes/web.php` +- [X] T022 [US2] Remove the `->authorize(fn (): bool => true)` bypass from header and empty-state create actions in `app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php` +- [X] T023 [US2] Fix membership (404) vs capability (403) ordering for backup items under a backup set in `app/Filament/Resources/BackupSetResource/RelationManagers/BackupItemsRelationManager.php` +- [X] T024 [US2] Swap legacy enforcement helper imports to canonical RBAC helper in `app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php` and `app/Filament/Resources/RestoreRunResource/Pages/ListRestoreRuns.php` +- [X] T037 [US2] Verify any destructive-like Provider Connections actions still require explicit confirmation (no regressions), and that server authorization remains enforced for both header and empty-state CTAs + +**Checkpoint**: US2 closes cross-plane leak + removes bypasses + restores correct 404/403 semantics. + +--- + +## Phase 5: User Story 3 — Validate assignment operations safely in non-production contexts (Priority: P3) + +**Goal**: Assignment-related Graph services depend only on `GraphClientInterface`, enabling tests to run with `NullGraphClient` or fakes without concrete client coupling. + +**Independent Test**: Resolve the assignment Graph services from the container with Graph disabled and run a minimal flow that exercises the interface contract. + +### Tests for User Story 3 (write first) + +- [X] T025 [P] [US3] Add unit test asserting assignment Graph services resolve with `GraphClientInterface` binding in `tests/Feature/Graph/AssignmentGraphServiceResolutionTest.php` +- [X] T026 [P] [US3] Add test using a fake `GraphClientInterface` to simulate assignment fetch failures without real HTTP in `tests/Feature/Operations/AssignmentFetchOperationRunFailureTest.php` + +### Implementation for User Story 3 + +- [X] T027 [P] [US3] Replace `MicrosoftGraphClient` constructor type-hint with `GraphClientInterface` in `app/Services/Graph/AssignmentFetcher.php` +- [X] T028 [P] [US3] Replace `MicrosoftGraphClient` constructor type-hint with `GraphClientInterface` in `app/Services/Graph/GroupResolver.php` +- [X] T029 [P] [US3] Replace `MicrosoftGraphClient` constructor type-hint with `GraphClientInterface` in `app/Services/Graph/AssignmentFilterResolver.php` +- [X] T030 [US3] Confirm container bindings remain canonical and no concrete client is injected directly by reviewing `app/Providers/AppServiceProvider.php` + +**Checkpoint**: US3 enables deterministic tests and non-prod validation for assignment operations. + +--- + +## Phase 6: Polish & Cross-Cutting Concerns + +**Purpose**: Formatting + targeted verification commands + confidence checks. + +- [X] T031 [P] Run formatting for touched files via `./vendor/bin/sail bin pint --dirty` (see `specs/094-assignment-ops-observability-hardening/quickstart.md`) +- [X] T032 Run targeted tests via `./vendor/bin/sail artisan test --compact tests/Feature/Operations tests/Feature/Rbac tests/Feature/Guards tests/Feature/Audit` (see `specs/094-assignment-ops-observability-hardening/quickstart.md`) +- [X] T033 Run the full test suite via `./vendor/bin/sail artisan test --compact` (see `specs/094-assignment-ops-observability-hardening/quickstart.md`) +- [X] T038 Update any requirement references in tasks/spec if FR numbering changes (keep traceability from FR-001..FR-012 to the task IDs) + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **Setup (Phase 1)**: No dependencies. +- **Foundational (Phase 2)**: Depends on Phase 1. +- **US1 (Phase 3)**: Depends on Phase 2. No dependency on US2/US3. +- **US2 (Phase 4)**: Depends on Phase 2. Independent from US1/US3. +- **US3 (Phase 5)**: Depends on Phase 2. Independent from US1/US2. +- **Polish (Phase 6)**: Depends on completing the desired stories. + +### User Story Dependencies + +- **US1 (P1)**: Standalone MVP. +- **US2 (P2)**: Standalone hardening. +- **US3 (P3)**: Standalone testability hardening. + +--- + +## Parallel Example: User Story 1 + +Parallelizable test tasks: T007, T008, T009, T010 (different files under `tests/Feature/...`). + +--- + +## Parallel Example: User Story 2 + +Parallelizable test tasks: T018, T019, T020 (different files under `tests/Feature/...`). + +Parallelizable implementation tasks (after tests land): T021 + T022 (different files under `routes/` vs `app/Filament/...`). + +--- + +## Parallel Example: User Story 3 + +Parallelizable implementation tasks: T027, T028, T029 (different files under `app/Services/Graph/...`). + +--- + +## Implementation Strategy + +### MVP First (User Story 1 Only) + +1. Complete Phase 1 (Setup) + Phase 2 (Foundational) +2. Complete Phase 3 (US1) including tests +3. Validate Monitoring visibility + audit log semantics + +### Incremental Delivery + +1. US1 → deploy/demo +2. US2 → deploy/demo +3. US3 → deploy/demo diff --git a/tests/Feature/Audit/AssignmentRestoreAuditSummaryTest.php b/tests/Feature/Audit/AssignmentRestoreAuditSummaryTest.php new file mode 100644 index 0000000..3d2962e --- /dev/null +++ b/tests/Feature/Audit/AssignmentRestoreAuditSummaryTest.php @@ -0,0 +1,131 @@ +instance(GraphClientInterface::class, new class implements GraphClientInterface + { + public function listPolicies(string $policyType, array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function getPolicy(string $policyType, string $policyId, array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function getOrganization(array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function applyPolicy(string $policyType, string $policyId, array $payload, array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function request(string $method, string $path, array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function getServicePrincipalPermissions(array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + }); + + $tenant = Tenant::factory()->create([ + 'tenant_id' => 'tenant-assignment-audit-summary', + ]); + ensureDefaultProviderConnection($tenant); + + $policy = Policy::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'external_id' => 'policy-assignment-audit-summary', + 'policy_type' => 'settingsCatalogPolicy', + ]); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + ]); + + $backupItem = BackupItem::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'backup_set_id' => (int) $backupSet->getKey(), + 'policy_id' => (int) $policy->getKey(), + 'policy_identifier' => (string) $policy->external_id, + 'policy_type' => (string) $policy->policy_type, + 'assignments' => [ + [ + 'id' => 'assignment-1', + 'target' => [ + '@odata.type' => '#microsoft.graph.groupAssignmentTarget', + 'groupId' => 'group-source-1', + ], + ], + [ + 'id' => 'assignment-2', + 'target' => [ + '@odata.type' => '#microsoft.graph.groupAssignmentTarget', + 'groupId' => 'group-source-2', + ], + ], + ], + 'payload' => [ + '@odata.type' => '#microsoft.graph.deviceManagementConfigurationPolicy', + ], + ]); + + $user = User::factory()->create([ + 'email' => 'assignment.audit.summary@example.com', + ]); + $this->actingAs($user); + + $restoreRun = app(RestoreService::class)->execute( + tenant: $tenant, + backupSet: $backupSet, + selectedItemIds: [(int) $backupItem->getKey()], + dryRun: false, + actorEmail: $user->email, + actorName: $user->name, + groupMapping: [ + 'group-source-1' => 'group-target-1', + 'group-source-2' => 'group-target-2', + ], + ); + + $summaryEntries = AuditLog::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('action', 'restore.assignments.summary') + ->where('resource_type', 'restore_run') + ->where('resource_id', (string) $restoreRun->getKey()) + ->get(); + + expect($summaryEntries)->toHaveCount(1); + expect($summaryEntries->first()?->metadata['succeeded'] ?? null)->toBe(2); + expect($summaryEntries->first()?->metadata['failed'] ?? null)->toBe(0); + + $perAssignmentEntryCount = AuditLog::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->whereIn('action', [ + 'restore.assignment.created', + 'restore.assignment.failed', + 'restore.assignment.skipped', + ]) + ->count(); + + expect($perAssignmentEntryCount)->toBe(0); +}); diff --git a/tests/Feature/Graph/AssignmentGraphServiceResolutionTest.php b/tests/Feature/Graph/AssignmentGraphServiceResolutionTest.php new file mode 100644 index 0000000..b6366e5 --- /dev/null +++ b/tests/Feature/Graph/AssignmentGraphServiceResolutionTest.php @@ -0,0 +1,61 @@ +instance(GraphClientInterface::class, $fake); + + $fetcher = app(AssignmentFetcher::class); + $groupResolver = app(GroupResolver::class); + $filterResolver = app(AssignmentFilterResolver::class); + + $fetcherProperty = new \ReflectionProperty(AssignmentFetcher::class, 'graphClient'); + $fetcherProperty->setAccessible(true); + $groupResolverProperty = new \ReflectionProperty(GroupResolver::class, 'graphClient'); + $groupResolverProperty->setAccessible(true); + $filterResolverProperty = new \ReflectionProperty(AssignmentFilterResolver::class, 'graphClient'); + $filterResolverProperty->setAccessible(true); + + expect($fetcherProperty->getValue($fetcher))->toBe($fake); + expect($groupResolverProperty->getValue($groupResolver))->toBe($fake); + expect($filterResolverProperty->getValue($filterResolver))->toBe($fake); +}); diff --git a/tests/Feature/Guards/AdminWorkspaceRoutesGuardTest.php b/tests/Feature/Guards/AdminWorkspaceRoutesGuardTest.php new file mode 100644 index 0000000..c4ae48f --- /dev/null +++ b/tests/Feature/Guards/AdminWorkspaceRoutesGuardTest.php @@ -0,0 +1,21 @@ +create(); + $workspace = $tenant->workspace; + + expect($workspace)->not->toBeNull(); + + $platformUser = PlatformUser::factory()->create(); + + $workspaceRouteKey = (string) ($workspace->slug ?? $workspace->getKey()); + + $this->actingAs($platformUser, 'platform') + ->get("/admin/w/{$workspaceRouteKey}/ping") + ->assertNotFound(); +}); diff --git a/tests/Feature/Monitoring/OperationsDbOnlyRenderTest.php b/tests/Feature/Monitoring/OperationsDbOnlyRenderTest.php new file mode 100644 index 0000000..e118cb7 --- /dev/null +++ b/tests/Feature/Monitoring/OperationsDbOnlyRenderTest.php @@ -0,0 +1,39 @@ +create([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'type' => 'restore.execute', + 'status' => 'completed', + 'outcome' => 'succeeded', + 'initiator_name' => 'System', + ]); + + $this->actingAs($user); + Bus::fake(); + Filament::setTenant(null, true); + + assertNoOutboundHttp(function () use ($tenant, $run): void { + $this->withSession([WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id]) + ->get('/admin/operations') + ->assertOk() + ->assertSee('All'); + + $this->withSession([WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id]) + ->get(route('admin.operations.view', ['run' => (int) $run->getKey()])) + ->assertOk() + ->assertSee('Operation run'); + }); + + Bus::assertNothingDispatched(); +}); diff --git a/tests/Feature/OperationRunServiceTest.php b/tests/Feature/OperationRunServiceTest.php index 7a69368..76840c1 100644 --- a/tests/Feature/OperationRunServiceTest.php +++ b/tests/Feature/OperationRunServiceTest.php @@ -36,6 +36,77 @@ expect(OperationRun::query()->count())->toBe(1); }); +it('dedupes assignment run identities by type and scope', function () { + $tenant = Tenant::factory()->create(); + + $service = new OperationRunService; + + $fetchRunA = $service->ensureRunWithIdentity( + tenant: $tenant, + type: 'assignments.fetch', + identityInputs: [ + 'backup_item_id' => 101, + ], + context: [ + 'backup_item_id' => 101, + 'phase' => 'capture', + ], + ); + + $fetchRunB = $service->ensureRunWithIdentity( + tenant: $tenant, + type: 'assignments.fetch', + identityInputs: [ + 'backup_item_id' => 101, + ], + context: [ + 'backup_item_id' => 101, + 'phase' => 'capture-again', + ], + ); + + $fetchRunDifferentScope = $service->ensureRunWithIdentity( + tenant: $tenant, + type: 'assignments.fetch', + identityInputs: [ + 'backup_item_id' => 102, + ], + context: [ + 'backup_item_id' => 102, + 'phase' => 'capture', + ], + ); + + $restoreRunA = $service->ensureRunWithIdentity( + tenant: $tenant, + type: 'assignments.restore', + identityInputs: [ + 'restore_run_id' => 501, + ], + context: [ + 'restore_run_id' => 501, + 'phase' => 'execute', + ], + ); + + $restoreRunB = $service->ensureRunWithIdentity( + tenant: $tenant, + type: 'assignments.restore', + identityInputs: [ + 'restore_run_id' => 501, + ], + context: [ + 'restore_run_id' => 501, + 'phase' => 'execute-again', + ], + ); + + expect($fetchRunA->getKey())->toBe($fetchRunB->getKey()); + expect($restoreRunA->getKey())->toBe($restoreRunB->getKey()); + expect($fetchRunA->getKey())->not->toBe($fetchRunDifferentScope->getKey()); + expect($fetchRunA->getKey())->not->toBe($restoreRunA->getKey()); +}); + it('does not replace the initiator when deduping', function () { $tenant = Tenant::factory()->create(); $userA = User::factory()->create(); diff --git a/tests/Feature/Operations/AssignmentFetchOperationRunFailureTest.php b/tests/Feature/Operations/AssignmentFetchOperationRunFailureTest.php new file mode 100644 index 0000000..51677e0 --- /dev/null +++ b/tests/Feature/Operations/AssignmentFetchOperationRunFailureTest.php @@ -0,0 +1,129 @@ +instance(GraphClientInterface::class, new class implements GraphClientInterface + { + public function listPolicies(string $policyType, array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function getPolicy(string $policyType, string $policyId, array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function getOrganization(array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function applyPolicy(string $policyType, string $policyId, array $payload, array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function request(string $method, string $path, array $options = []): GraphResponse + { + return new GraphResponse( + success: false, + data: [], + status: 400, + errors: [ + ['code' => 'BadRequest', 'message' => 'Bad request'], + ], + warnings: [], + meta: [ + 'error_code' => 'BadRequest', + 'error_message' => 'Assignment list request failed', + ], + ); + } + + public function getServicePrincipalPermissions(array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + }); + + $tenant = Tenant::factory()->create([ + 'tenant_id' => 'tenant-assignment-fetch-failure', + 'status' => 'active', + ]); + ensureDefaultProviderConnection($tenant); + + $policy = Policy::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'external_id' => 'policy-assignment-fetch-failure', + 'policy_type' => 'settingsCatalogPolicy', + 'platform' => 'windows', + ]); + + $this->mock(PolicySnapshotService::class, function (MockInterface $mock) use ($policy): void { + $mock->shouldReceive('fetch') + ->once() + ->andReturn([ + 'payload' => [ + '@odata.type' => '#microsoft.graph.deviceManagementConfigurationPolicy', + 'id' => (string) $policy->external_id, + 'name' => 'Policy snapshot', + 'roleScopeTagIds' => ['0'], + ], + 'metadata' => [], + 'warnings' => [], + ]); + }); + + $this->mock(ScopeTagResolver::class, function (MockInterface $mock) use ($tenant): void { + $mock->shouldReceive('resolve') + ->once() + ->with(['0'], $tenant) + ->andReturn([ + ['id' => '0', 'displayName' => 'Default'], + ]); + }); + + $backupSet = app(BackupService::class)->createBackupSet( + tenant: $tenant, + policyIds: [(int) $policy->getKey()], + actorEmail: 'assignment.fetch.failure@example.com', + actorName: 'Assignment Fetch Failure', + includeAssignments: true, + includeScopeTags: true, + ); + + $backupItem = $backupSet->items()->first(); + expect($backupItem)->not->toBeNull(); + expect($backupItem?->metadata['assignments_fetch_failed'] ?? false)->toBeTrue(); + + $assignmentFetchRun = OperationRun::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('type', 'assignments.fetch') + ->latest('id') + ->first(); + + expect($assignmentFetchRun)->not->toBeNull(); + expect($assignmentFetchRun?->status)->toBe('completed'); + expect($assignmentFetchRun?->outcome)->toBe('failed'); + expect($assignmentFetchRun?->summary_counts ?? [])->toMatchArray([ + 'total' => 1, + 'processed' => 0, + 'failed' => 1, + ]); + expect($assignmentFetchRun?->failure_summary[0]['code'] ?? null)->toBe('assignments.fetch_failed'); + expect(ProviderReasonCodes::isKnown((string) ($assignmentFetchRun?->failure_summary[0]['reason_code'] ?? '')))->toBeTrue(); +}); diff --git a/tests/Feature/Operations/AssignmentFetchOperationRunTest.php b/tests/Feature/Operations/AssignmentFetchOperationRunTest.php new file mode 100644 index 0000000..b9082c9 --- /dev/null +++ b/tests/Feature/Operations/AssignmentFetchOperationRunTest.php @@ -0,0 +1,114 @@ +create([ + 'tenant_id' => 'tenant-assignment-fetch-run', + 'status' => 'active', + ]); + ensureDefaultProviderConnection($tenant); + + $policy = Policy::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'external_id' => 'policy-assignment-fetch-run', + 'policy_type' => 'settingsCatalogPolicy', + 'platform' => 'windows', + ]); + + $this->mock(PolicySnapshotService::class, function (MockInterface $mock) use ($policy): void { + $mock->shouldReceive('fetch') + ->once() + ->andReturn([ + 'payload' => [ + '@odata.type' => '#microsoft.graph.deviceManagementConfigurationPolicy', + 'id' => (string) $policy->external_id, + 'name' => 'Policy snapshot', + 'roleScopeTagIds' => ['0'], + ], + 'metadata' => [], + 'warnings' => [], + ]); + }); + + $this->mock(AssignmentFetcher::class, function (MockInterface $mock): void { + $mock->shouldReceive('fetch') + ->once() + ->andReturn([ + [ + 'id' => 'assignment-1', + 'target' => [ + '@odata.type' => '#microsoft.graph.groupAssignmentTarget', + 'groupId' => 'group-1', + ], + ], + ]); + }); + + $this->mock(GroupResolver::class, function (MockInterface $mock): void { + $mock->shouldReceive('resolveGroupIds') + ->once() + ->andReturn([ + 'group-1' => [ + 'id' => 'group-1', + 'displayName' => 'Group One', + 'orphaned' => false, + ], + ]); + }); + + $this->mock(AssignmentFilterResolver::class, function (MockInterface $mock): void { + $mock->shouldReceive('resolve') + ->once() + ->andReturn([]); + }); + + $this->mock(ScopeTagResolver::class, function (MockInterface $mock) use ($tenant): void { + $mock->shouldReceive('resolve') + ->once() + ->with(['0'], $tenant) + ->andReturn([ + ['id' => '0', 'displayName' => 'Default'], + ]); + }); + + $backupSet = app(BackupService::class)->createBackupSet( + tenant: $tenant, + policyIds: [(int) $policy->getKey()], + actorEmail: 'assignment.fetch.run@example.com', + actorName: 'Assignment Fetch', + includeAssignments: true, + includeScopeTags: true, + ); + + $backupItem = $backupSet->items()->first(); + expect($backupItem)->not->toBeNull(); + + $assignmentFetchRun = OperationRun::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('type', 'assignments.fetch') + ->latest('id') + ->first(); + + expect($assignmentFetchRun)->not->toBeNull(); + expect($assignmentFetchRun?->status)->toBe('completed'); + expect($assignmentFetchRun?->outcome)->toBe('succeeded'); + expect($assignmentFetchRun?->context['backup_item_id'] ?? null)->toBe((int) $backupItem?->getKey()); + expect($assignmentFetchRun?->summary_counts ?? [])->toMatchArray([ + 'total' => 1, + 'processed' => 1, + 'failed' => 0, + ]); +}); diff --git a/tests/Feature/Operations/AssignmentRestoreOperationRunFailureTest.php b/tests/Feature/Operations/AssignmentRestoreOperationRunFailureTest.php new file mode 100644 index 0000000..eb41e66 --- /dev/null +++ b/tests/Feature/Operations/AssignmentRestoreOperationRunFailureTest.php @@ -0,0 +1,135 @@ +instance(GraphClientInterface::class, new class implements GraphClientInterface + { + public function listPolicies(string $policyType, array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function getPolicy(string $policyType, string $policyId, array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function getOrganization(array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function applyPolicy(string $policyType, string $policyId, array $payload, array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function request(string $method, string $path, array $options = []): GraphResponse + { + return new GraphResponse( + success: false, + data: [], + status: 400, + errors: [ + ['code' => 'BadRequest', 'message' => 'Bad request'], + ], + warnings: [], + meta: [ + 'error_code' => 'BadRequest', + 'error_message' => 'Bad request while restoring assignments', + ], + ); + } + + public function getServicePrincipalPermissions(array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + }); + + $tenant = Tenant::factory()->create([ + 'tenant_id' => 'tenant-assignment-restore-failure', + ]); + ensureDefaultProviderConnection($tenant); + + $policy = Policy::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'external_id' => 'policy-assignment-restore-failure', + 'policy_type' => 'settingsCatalogPolicy', + ]); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + ]); + + $backupItem = BackupItem::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'backup_set_id' => (int) $backupSet->getKey(), + 'policy_id' => (int) $policy->getKey(), + 'policy_identifier' => (string) $policy->external_id, + 'policy_type' => (string) $policy->policy_type, + 'assignments' => [ + [ + 'id' => 'assignment-1', + 'target' => [ + '@odata.type' => '#microsoft.graph.groupAssignmentTarget', + 'groupId' => 'group-source-1', + ], + ], + ], + 'payload' => [ + '@odata.type' => '#microsoft.graph.deviceManagementConfigurationPolicy', + ], + ]); + + $user = User::factory()->create([ + 'email' => 'assignment.restore.failure@example.com', + ]); + $this->actingAs($user); + + app(RestoreService::class)->execute( + tenant: $tenant, + backupSet: $backupSet, + selectedItemIds: [(int) $backupItem->getKey()], + dryRun: false, + actorEmail: $user->email, + actorName: $user->name, + groupMapping: [ + 'group-source-1' => 'group-target-1', + ], + ); + + $assignmentRun = OperationRun::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('type', 'assignments.restore') + ->latest('id') + ->first(); + + expect($assignmentRun)->not->toBeNull(); + expect($assignmentRun?->status)->toBe('completed'); + expect($assignmentRun?->outcome)->toBe('failed'); + expect($assignmentRun?->summary_counts ?? [])->toMatchArray([ + 'total' => 1, + 'processed' => 0, + 'failed' => 1, + ]); + + $failure = $assignmentRun?->failure_summary[0] ?? []; + + expect($failure['code'] ?? null)->toBe('assignments.restore_failed'); + expect($failure['reason_code'] ?? null)->toBe(ProviderReasonCodes::ProviderConnectionInvalid); + expect((string) ($failure['message'] ?? ''))->not->toBe(''); +}); diff --git a/tests/Feature/Operations/AssignmentRestoreOperationRunTest.php b/tests/Feature/Operations/AssignmentRestoreOperationRunTest.php new file mode 100644 index 0000000..c7e5327 --- /dev/null +++ b/tests/Feature/Operations/AssignmentRestoreOperationRunTest.php @@ -0,0 +1,117 @@ +instance(GraphClientInterface::class, new class implements GraphClientInterface + { + public function listPolicies(string $policyType, array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function getPolicy(string $policyType, string $policyId, array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function getOrganization(array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function applyPolicy(string $policyType, string $policyId, array $payload, array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function request(string $method, string $path, array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + + public function getServicePrincipalPermissions(array $options = []): GraphResponse + { + return new GraphResponse(true, []); + } + }); + + $tenant = Tenant::factory()->create([ + 'tenant_id' => 'tenant-assignment-restore-success', + ]); + ensureDefaultProviderConnection($tenant); + + $policy = Policy::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'external_id' => 'policy-assignment-restore-success', + 'policy_type' => 'settingsCatalogPolicy', + ]); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + ]); + + $backupItem = BackupItem::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'backup_set_id' => (int) $backupSet->getKey(), + 'policy_id' => (int) $policy->getKey(), + 'policy_identifier' => (string) $policy->external_id, + 'policy_type' => (string) $policy->policy_type, + 'assignments' => [ + [ + 'id' => 'assignment-1', + 'target' => [ + '@odata.type' => '#microsoft.graph.groupAssignmentTarget', + 'groupId' => 'group-source-1', + ], + ], + ], + 'payload' => [ + '@odata.type' => '#microsoft.graph.deviceManagementConfigurationPolicy', + ], + ]); + + $user = User::factory()->create([ + 'email' => 'assignment.restore.success@example.com', + ]); + $this->actingAs($user); + + $restoreRun = app(RestoreService::class)->execute( + tenant: $tenant, + backupSet: $backupSet, + selectedItemIds: [(int) $backupItem->getKey()], + dryRun: false, + actorEmail: $user->email, + actorName: $user->name, + groupMapping: [ + 'group-source-1' => 'group-target-1', + ], + ); + + $assignmentRun = OperationRun::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('type', 'assignments.restore') + ->latest('id') + ->first(); + + expect($assignmentRun)->not->toBeNull(); + expect($assignmentRun?->status)->toBe('completed'); + expect($assignmentRun?->outcome)->toBe('succeeded'); + expect($assignmentRun?->context['restore_run_id'] ?? null)->toBe((int) $restoreRun->getKey()); + expect($assignmentRun?->summary_counts ?? [])->toMatchArray([ + 'total' => 1, + 'processed' => 1, + 'failed' => 0, + ]); +}); diff --git a/tests/Feature/ProviderConnections/ProviderConnectionListAuthorizationTest.php b/tests/Feature/ProviderConnections/ProviderConnectionListAuthorizationTest.php new file mode 100644 index 0000000..995ef60 --- /dev/null +++ b/tests/Feature/ProviderConnections/ProviderConnectionListAuthorizationTest.php @@ -0,0 +1,23 @@ +tenants()->detach((int) $tenant->getKey()); + app(\App\Services\Auth\CapabilityResolver::class)->clearCache(); + + $this->actingAs($user); + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + Livewire::test(ListProviderConnections::class) + ->assertActionExists('create', function (Action $action): bool { + return $action->isAuthorized() === false; + }); +}); diff --git a/tests/Feature/Rbac/BackupItemsRelationManagerSemanticsTest.php b/tests/Feature/Rbac/BackupItemsRelationManagerSemanticsTest.php new file mode 100644 index 0000000..92901e9 --- /dev/null +++ b/tests/Feature/Rbac/BackupItemsRelationManagerSemanticsTest.php @@ -0,0 +1,60 @@ +actingAs($owner); + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + ]); + $backupItem = BackupItem::factory()->for($backupSet)->for($tenant)->create(); + + $outsider = User::factory()->create(); + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'user_id' => (int) $outsider->getKey(), + 'role' => 'owner', + ]); + + $this->actingAs($outsider); + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + $this->get(BackupSetResource::getUrl('view', ['record' => $backupSet], tenant: $tenant)) + ->assertNotFound(); +}); + +it('keeps actions visible but disabled for members missing capability', function (): void { + [$readonlyUser, $tenant] = createUserWithTenant(role: 'readonly'); + + $this->actingAs($readonlyUser); + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + ]); + $backupItem = BackupItem::factory()->for($backupSet)->for($tenant)->create(); + + Livewire::test(BackupItemsRelationManager::class, [ + 'ownerRecord' => $backupSet, + 'pageClass' => EditBackupSet::class, + ]) + ->assertTableActionVisible('remove', $backupItem) + ->assertTableActionDisabled('remove', $backupItem); +}); diff --git a/tests/Feature/WorkspaceIsolation/WorkspaceIdForeignKeyConstraintTest.php b/tests/Feature/WorkspaceIsolation/WorkspaceIdForeignKeyConstraintTest.php new file mode 100644 index 0000000..ac44034 --- /dev/null +++ b/tests/Feature/WorkspaceIsolation/WorkspaceIdForeignKeyConstraintTest.php @@ -0,0 +1,48 @@ +markTestSkipped('Postgres-only: validates FK constraints via pg_constraint.'); + } + + $tables = [ + 'policies', + 'policy_versions', + 'backup_sets', + 'backup_items', + 'restore_runs', + 'backup_schedules', + 'inventory_items', + 'inventory_links', + 'entra_groups', + 'findings', + 'entra_role_definitions', + 'tenant_permissions', + ]; + + foreach ($tables as $table) { + $sql = <<<'SQL' + SELECT c.conname, c.convalidated + FROM pg_constraint c + JOIN pg_class rel ON rel.oid = c.conrelid + JOIN pg_class ref ON ref.oid = c.confrelid + JOIN pg_attribute att ON att.attrelid = rel.oid AND att.attnum = ANY(c.conkey) + WHERE c.contype = 'f' + AND rel.relname = ? + AND ref.relname = 'workspaces' + AND att.attname = 'workspace_id' + SQL; + + $constraints = DB::select( + $sql, + [$table], + ); + + expect($constraints)->not->toBeEmpty(); + + $allValidated = collect($constraints)->every(fn ($c): bool => (bool) $c->convalidated); + expect($allValidated)->toBeTrue(); + } +}); diff --git a/tests/Unit/AssignmentRestoreServiceTest.php b/tests/Unit/AssignmentRestoreServiceTest.php index 78308ff..7f04b8a 100644 --- a/tests/Unit/AssignmentRestoreServiceTest.php +++ b/tests/Unit/AssignmentRestoreServiceTest.php @@ -1,6 +1,5 @@ graphClient = Mockery::mock(GraphClientInterface::class); - $this->auditLogger = Mockery::mock(AuditLogger::class); $this->filterResolver = Mockery::mock(AssignmentFilterResolver::class); $this->filterResolver->shouldReceive('resolve')->andReturn([])->byDefault(); @@ -39,7 +36,6 @@ $this->graphClient, app(GraphContractRegistry::class), app(GraphLogger::class), - $this->auditLogger, $this->filterResolver, app(MicrosoftGraphOptionsResolver::class), ); @@ -80,11 +76,6 @@ )) ->andReturn(new GraphResponse(success: true, data: [])); - $this->auditLogger - ->shouldReceive('log') - ->once() - ->andReturn(new AuditLog); - $result = $this->service->restore( $tenant, 'deviceManagementScript', @@ -125,11 +116,6 @@ )) ->andReturn(new GraphResponse(success: true, data: [])); - $this->auditLogger - ->shouldReceive('log') - ->once() - ->andReturn(new AuditLog); - $result = $this->service->restore( $tenant, 'appProtectionPolicy', @@ -187,11 +173,6 @@ )) ->andReturn(new GraphResponse(success: true, data: [])); - $this->auditLogger - ->shouldReceive('log') - ->once() - ->andReturn(new AuditLog); - $result = $this->service->restore( $tenant, 'settingsCatalogPolicy', @@ -245,11 +226,6 @@ )) ->andReturn(new GraphResponse(success: true, data: [])); - $this->auditLogger - ->shouldReceive('log') - ->once() - ->andReturn(new AuditLog); - $result = $this->service->restore( $tenant, 'settingsCatalogPolicy', diff --git a/tests/Unit/Auth/UiEnforcementBulkPreflightQueryCountTest.php b/tests/Unit/Auth/UiEnforcementBulkPreflightQueryCountTest.php index 5f98ccd..362b220 100644 --- a/tests/Unit/Auth/UiEnforcementBulkPreflightQueryCountTest.php +++ b/tests/Unit/Auth/UiEnforcementBulkPreflightQueryCountTest.php @@ -2,7 +2,8 @@ use App\Models\Tenant; use App\Support\Auth\Capabilities; -use App\Support\Auth\UiEnforcement; +use App\Support\Rbac\UiEnforcement; +use Filament\Actions\Action; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\DB; @@ -18,9 +19,10 @@ ]); } - $enforcement = UiEnforcement::for(Capabilities::TENANT_SYNC) - ->tenantFromRecord() - ->preflightByCapability(); + $action = Action::make('test')->action(fn () => null); + + $enforcement = UiEnforcement::forAction($action) + ->requireCapability(Capabilities::TENANT_SYNC); $membershipQueries = 0; @@ -33,4 +35,3 @@ expect($enforcement->bulkSelectionIsAuthorized($user, $tenants))->toBeTrue(); expect($membershipQueries)->toBe(1); }); - diff --git a/tests/Unit/Auth/UiEnforcementTest.php b/tests/Unit/Auth/UiEnforcementTest.php index 74b4284..4e8c1fb 100644 --- a/tests/Unit/Auth/UiEnforcementTest.php +++ b/tests/Unit/Auth/UiEnforcementTest.php @@ -2,31 +2,22 @@ use App\Models\Tenant; use App\Support\Auth\Capabilities; -use App\Support\Auth\UiEnforcement; use App\Support\Auth\UiTooltips; +use App\Support\Rbac\UiEnforcement; use Filament\Actions\Action; -use Filament\Facades\Filament; use Illuminate\Foundation\Testing\RefreshDatabase; uses(RefreshDatabase::class); -it('forbids preserveVisibility on record-scoped tenant resolution', function () { - expect(fn () => UiEnforcement::for(Capabilities::TENANT_VIEW)->tenantFromRecord()->preserveVisibility()) - ->toThrow(LogicException::class); - - expect(fn () => UiEnforcement::for(Capabilities::TENANT_VIEW)->preserveVisibility()->tenantFromRecord()) - ->toThrow(LogicException::class); -}); - it('hides actions for non-members on record-scoped surfaces', function () { $tenant = Tenant::factory()->create(); [$user] = createUserWithTenant(); - $action = Action::make('test'); + $action = Action::make('test')->action(fn () => null); - UiEnforcement::for(Capabilities::TENANT_VIEW) - ->tenantFromRecord() - ->apply($action); + UiEnforcement::forAction($action) + ->requireCapability(Capabilities::TENANT_VIEW) + ->apply(); $this->actingAs($user); $action->record($tenant); @@ -38,11 +29,11 @@ $tenant = Tenant::factory()->create(); [$user] = createUserWithTenant($tenant, role: 'readonly'); - $action = Action::make('test'); + $action = Action::make('test')->action(fn () => null); - UiEnforcement::for(Capabilities::TENANT_SYNC) - ->tenantFromRecord() - ->apply($action); + UiEnforcement::forAction($action) + ->requireCapability(Capabilities::TENANT_SYNC) + ->apply(); $this->actingAs($user); $action->record($tenant); @@ -56,11 +47,11 @@ $tenant = Tenant::factory()->create(); [$user] = createUserWithTenant($tenant, role: 'owner'); - $action = Action::make('test'); + $action = Action::make('test')->action(fn () => null); - UiEnforcement::for(Capabilities::TENANT_SYNC) - ->tenantFromRecord() - ->apply($action); + UiEnforcement::forAction($action) + ->requireCapability(Capabilities::TENANT_SYNC) + ->apply(); $this->actingAs($user); $action->record($tenant); @@ -70,36 +61,21 @@ expect($action->getTooltip())->toBeNull(); }); -it('supports mixed visibility composition via andVisibleWhen', function () { +it('preserveVisibility combines existing visibility with membership checks', function () { $tenant = Tenant::factory()->create(); [$user] = createUserWithTenant($tenant, role: 'owner'); - Filament::setTenant($tenant, true); + $action = Action::make('test') + ->action(fn () => null) + ->visible(fn (): bool => false); - $action = Action::make('test'); - - UiEnforcement::for(Capabilities::TENANT_VIEW) - ->andVisibleWhen(fn (): bool => false) - ->apply($action); - - $this->actingAs($user); - - expect($action->isHidden())->toBeTrue(); -}); - -it('supports mixed visibility composition via andHiddenWhen', function () { - $tenant = Tenant::factory()->create(); - [$user] = createUserWithTenant($tenant, role: 'owner'); - - Filament::setTenant($tenant, true); - - $action = Action::make('test'); - - UiEnforcement::for(Capabilities::TENANT_VIEW) - ->andHiddenWhen(fn (): bool => true) - ->apply($action); + UiEnforcement::forAction($action) + ->preserveVisibility() + ->requireCapability(Capabilities::TENANT_VIEW) + ->apply(); $this->actingAs($user); + $action->record($tenant); expect($action->isHidden())->toBeTrue(); }); @@ -113,9 +89,10 @@ $tenantB->getKey() => ['role' => 'readonly'], ]); - $enforcement = UiEnforcement::for(Capabilities::TENANT_SYNC) - ->tenantFromRecord() - ->preflightByCapability(); + $action = Action::make('test')->action(fn () => null); + + $enforcement = UiEnforcement::forAction($action) + ->requireCapability(Capabilities::TENANT_SYNC); expect($enforcement->bulkSelectionIsAuthorized($user, collect([$tenantA, $tenantB])))->toBeFalse(); @@ -125,4 +102,3 @@ expect($enforcement->bulkSelectionIsAuthorized($user, collect([$tenantA, $tenantB])))->toBeTrue(); }); -