diff --git a/Agents.md b/Agents.md index 7d3d8d6..c546c58 100644 --- a/Agents.md +++ b/Agents.md @@ -1056,9 +1056,9 @@ ### Replaced Utilities ## Active Technologies -- PHP 8.4.15 (Laravel 12) + Filament v4, Livewire v3 (054-unify-runs-suitewide-session-1768601416) -- PostgreSQL (`operation_runs` + JSONB for summary/failures/context; partial unique index for active-run dedupe) (054-unify-runs-suitewide-session-1768601416) -- PHP 8.4.15 (Laravel 12) + Filament v5 + Livewire v4 (059-unified-badges) +- PHP 8.4 (Laravel 12) + Filament v5 + Livewire v4 +- PostgreSQL (Sail) +- Tailwind CSS v4 ## Recent Changes -- 054-unify-runs-suitewide-session-1768601416: Added PHP 8.4.15 (Laravel 12) + Filament v4, Livewire v3 +- 066-rbac-ui-enforcement-helper-v2-session-1769732329: Planned UiEnforcement v2 (spec + plan + design artifacts) diff --git a/app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php b/app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php index 893ed9f..afd21e5 100644 --- a/app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php +++ b/app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php @@ -3,6 +3,8 @@ namespace App\Filament\Resources\BackupSetResource\Pages; use App\Filament\Resources\BackupSetResource; +use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use Filament\Actions; use Filament\Resources\Pages\ListRecords; @@ -13,9 +15,7 @@ class ListBackupSets extends ListRecords protected function getHeaderActions(): array { return [ - Actions\CreateAction::make() - ->disabled(fn (): bool => ! BackupSetResource::canCreate()) - ->tooltip(fn (): ?string => BackupSetResource::canCreate() ? null : 'You do not have permission to create backup sets.'), + UiEnforcement::for(Capabilities::TENANT_SYNC)->apply(Actions\CreateAction::make()), ]; } } diff --git a/app/Filament/Resources/RestoreRunResource/Pages/ListRestoreRuns.php b/app/Filament/Resources/RestoreRunResource/Pages/ListRestoreRuns.php index 1868ac5..fb11d2e 100644 --- a/app/Filament/Resources/RestoreRunResource/Pages/ListRestoreRuns.php +++ b/app/Filament/Resources/RestoreRunResource/Pages/ListRestoreRuns.php @@ -3,6 +3,8 @@ namespace App\Filament\Resources\RestoreRunResource\Pages; use App\Filament\Resources\RestoreRunResource; +use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use Filament\Actions; use Filament\Resources\Pages\ListRecords; @@ -13,9 +15,7 @@ class ListRestoreRuns extends ListRecords protected function getHeaderActions(): array { return [ - Actions\CreateAction::make() - ->disabled(fn (): bool => ! RestoreRunResource::canCreate()) - ->tooltip(fn (): ?string => RestoreRunResource::canCreate() ? null : 'You do not have permission to create restore runs.'), + UiEnforcement::for(Capabilities::TENANT_MANAGE)->apply(Actions\CreateAction::make()), ]; } } diff --git a/app/Support/Auth/UiEnforcement.php b/app/Support/Auth/UiEnforcement.php new file mode 100644 index 0000000..9686556 --- /dev/null +++ b/app/Support/Auth/UiEnforcement.php @@ -0,0 +1,526 @@ +): 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/Auth/UiTooltips.php b/app/Support/Auth/UiTooltips.php new file mode 100644 index 0000000..05dd60a --- /dev/null +++ b/app/Support/Auth/UiTooltips.php @@ -0,0 +1,14 @@ +create(); + $otherTenant = Tenant::factory()->create(); + + [$user] = createUserWithTenant($otherTenant, role: 'owner'); + + $this->actingAs($user) + ->get(BackupSetResource::getUrl('index', tenant: $tenant)) + ->assertStatus(404); +}); + +test('members without capability see BackupSet actions disabled with standard tooltip and cannot execute', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'readonly'); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'status' => 'completed', + 'deleted_at' => null, + ]); + + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListBackupSets::class) + ->assertTableActionDisabled('archive', $backupSet) + ->assertTableActionExists('archive', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $backupSet) + ->callTableAction('archive', $backupSet); + + expect($backupSet->fresh()->trashed())->toBeFalse(); +}); + +test('members with capability can execute BackupSet actions', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'owner'); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'status' => 'completed', + 'deleted_at' => null, + ]); + + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListBackupSets::class) + ->assertTableActionEnabled('archive', $backupSet) + ->callTableAction('archive', $backupSet); + + expect($backupSet->fresh()->trashed())->toBeTrue(); +}); + diff --git a/tests/Feature/Filament/EntraGroupSyncRunResourceTest.php b/tests/Feature/Filament/EntraGroupSyncRunResourceTest.php index e696853..b0737c2 100644 --- a/tests/Feature/Filament/EntraGroupSyncRunResourceTest.php +++ b/tests/Feature/Filament/EntraGroupSyncRunResourceTest.php @@ -7,13 +7,19 @@ use App\Models\Tenant; use App\Models\User; use App\Notifications\RunStatusChangedNotification; +use App\Support\Auth\UiTooltips; use Filament\Facades\Filament; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Queue; use Livewire\Livewire; uses(RefreshDatabase::class); +beforeEach(function (): void { + Http::preventStrayRequests(); +}); + test('entra group sync runs are listed for the active tenant', function () { $tenant = Tenant::factory()->create(); $otherTenant = Tenant::factory()->create(); @@ -96,3 +102,22 @@ expect($notification->data['actions'][0]['url'] ?? null) ->toBe(EntraGroupSyncRunResource::getUrl('view', ['record' => $run->getKey()], tenant: $tenant)); }); + +test('sync groups action is disabled for readonly users with standard tooltip', function () { + Queue::fake(); + + [$user, $tenant] = createUserWithTenant(role: 'readonly'); + + $this->actingAs($user); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListEntraGroupSyncRuns::class) + ->assertActionVisible('sync_groups') + ->assertActionDisabled('sync_groups') + ->assertActionExists('sync_groups', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission()); + + Queue::assertNothingPushed(); +}); diff --git a/tests/Feature/Filament/InventoryItemResourceTest.php b/tests/Feature/Filament/InventoryItemResourceTest.php index feae540..f46e5bd 100644 --- a/tests/Feature/Filament/InventoryItemResourceTest.php +++ b/tests/Feature/Filament/InventoryItemResourceTest.php @@ -1,12 +1,21 @@ create(); $otherTenant = Tenant::factory()->create(); @@ -39,3 +48,28 @@ ->assertSee('Item A') ->assertDontSee('Item B'); }); + +test('non-members are denied access to inventory item tenant routes (404)', function () { + $tenant = Tenant::factory()->create(); + $otherTenant = Tenant::factory()->create(); + + [$user] = createUserWithTenant($otherTenant, role: 'owner'); + + $this->actingAs($user) + ->get(InventoryItemResource::getUrl('index', tenant: $tenant)) + ->assertStatus(404); +}); + +test('members without capability see inventory sync action disabled with standard tooltip', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'readonly'); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListInventoryItems::class) + ->assertActionVisible('run_inventory_sync') + ->assertActionDisabled('run_inventory_sync') + ->assertActionExists('run_inventory_sync', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission()); +}); diff --git a/tests/Feature/Filament/InventorySyncRunResourceTest.php b/tests/Feature/Filament/InventorySyncRunResourceTest.php index 654eb3a..b17de43 100644 --- a/tests/Feature/Filament/InventorySyncRunResourceTest.php +++ b/tests/Feature/Filament/InventorySyncRunResourceTest.php @@ -4,9 +4,14 @@ use App\Models\InventorySyncRun; use App\Models\Tenant; use App\Models\User; +use Illuminate\Support\Facades\Http; uses(\Illuminate\Foundation\Testing\RefreshDatabase::class); +beforeEach(function (): void { + Http::preventStrayRequests(); +}); + test('inventory sync runs are listed for the active tenant', function () { $tenant = Tenant::factory()->create(); $otherTenant = Tenant::factory()->create(); @@ -35,3 +40,14 @@ ->assertSee(str_repeat('a', 12)) ->assertDontSee(str_repeat('b', 12)); }); + +test('non-members are denied access to inventory sync run tenant routes (404)', function () { + $tenant = Tenant::factory()->create(); + $otherTenant = Tenant::factory()->create(); + + [$user] = createUserWithTenant($otherTenant, role: 'owner'); + + $this->actingAs($user) + ->get(InventorySyncRunResource::getUrl('index', tenant: $tenant)) + ->assertStatus(404); +}); diff --git a/tests/Feature/Filament/ProviderConnectionsUiEnforcementTest.php b/tests/Feature/Filament/ProviderConnectionsUiEnforcementTest.php new file mode 100644 index 0000000..8b8fce9 --- /dev/null +++ b/tests/Feature/Filament/ProviderConnectionsUiEnforcementTest.php @@ -0,0 +1,77 @@ +create(); + $otherTenant = Tenant::factory()->create(); + + [$user] = createUserWithTenant($otherTenant, role: 'owner'); + + $this->actingAs($user) + ->get(ProviderConnectionResource::getUrl('index', tenant: $tenant)) + ->assertStatus(404); +}); + +test('members without capability see provider connection actions disabled with standard tooltip', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'readonly'); + + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'status' => 'needs_consent', + ]); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListProviderConnections::class) + ->assertTableActionVisible('check_connection', $connection) + ->assertTableActionDisabled('check_connection', $connection) + ->assertTableActionExists('check_connection', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $connection); + + Livewire::actingAs($user) + ->test(EditProviderConnection::class, ['record' => $connection->getKey()]) + ->assertActionVisible('check_connection') + ->assertActionDisabled('check_connection') + ->assertActionExists('check_connection', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission()); +}); + +test('members with capability can see provider connection actions enabled', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'owner'); + + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'status' => 'needs_consent', + ]); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListProviderConnections::class) + ->assertTableActionVisible('check_connection', $connection) + ->assertTableActionEnabled('check_connection', $connection); + + Livewire::actingAs($user) + ->test(EditProviderConnection::class, ['record' => $connection->getKey()]) + ->assertActionVisible('check_connection') + ->assertActionEnabled('check_connection'); +}); diff --git a/tests/Feature/Filament/RestoreRunUiEnforcementTest.php b/tests/Feature/Filament/RestoreRunUiEnforcementTest.php new file mode 100644 index 0000000..f438596 --- /dev/null +++ b/tests/Feature/Filament/RestoreRunUiEnforcementTest.php @@ -0,0 +1,83 @@ +create(); + $otherTenant = Tenant::factory()->create(); + + [$user] = createUserWithTenant($otherTenant, role: 'owner'); + + $this->actingAs($user) + ->get(RestoreRunResource::getUrl('index', tenant: $tenant)) + ->assertStatus(404); +}); + +test('members without capability see RestoreRun actions disabled with standard tooltip and cannot execute', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'readonly'); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'status' => 'completed', + ]); + + $restoreRun = RestoreRun::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'backup_set_id' => $backupSet->getKey(), + 'status' => 'completed', + 'deleted_at' => null, + ]); + + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListRestoreRuns::class) + ->assertTableActionDisabled('archive', $restoreRun) + ->assertTableActionExists('archive', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $restoreRun) + ->callTableAction('archive', $restoreRun); + + expect($restoreRun->fresh()->trashed())->toBeFalse(); +}); + +test('members with capability can execute RestoreRun actions', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'owner'); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'status' => 'completed', + ]); + + $restoreRun = RestoreRun::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'backup_set_id' => $backupSet->getKey(), + 'status' => 'completed', + 'deleted_at' => null, + ]); + + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListRestoreRuns::class) + ->assertTableActionEnabled('archive', $restoreRun) + ->callTableAction('archive', $restoreRun); + + expect($restoreRun->fresh()->trashed())->toBeTrue(); +}); + diff --git a/tests/Feature/Filament/TenantActionsAuthorizationTest.php b/tests/Feature/Filament/TenantActionsAuthorizationTest.php index fad1880..e3233ec 100644 --- a/tests/Feature/Filament/TenantActionsAuthorizationTest.php +++ b/tests/Feature/Filament/TenantActionsAuthorizationTest.php @@ -4,13 +4,19 @@ use App\Filament\Pages\TenantDashboard; use App\Filament\Resources\TenantResource\Pages\ListTenants; use App\Models\Tenant; +use App\Support\Auth\UiTooltips; use Filament\Facades\Filament; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Gate; use Livewire\Livewire; uses(RefreshDatabase::class); +beforeEach(function (): void { + Http::preventStrayRequests(); +}); + test('readonly users may switch current tenant via ChooseTenant', function () { [$user, $tenantA] = createUserWithTenant(role: 'readonly'); @@ -57,6 +63,7 @@ Livewire::actingAs($user) ->test(ListTenants::class) ->assertTableActionDisabled('archive', $tenant) + ->assertTableActionExists('archive', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $tenant) ->callTableAction('archive', $tenant); expect($tenant->fresh()->trashed())->toBeFalse(); @@ -74,6 +81,7 @@ Livewire::actingAs($user) ->test(ListTenants::class) ->assertTableActionDisabled('forceDelete', $tenant) + ->assertTableActionExists('forceDelete', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $tenant) ->callTableAction('forceDelete', $tenant); expect(Tenant::withTrashed()->find($tenant->getKey()))->not->toBeNull(); @@ -89,6 +97,7 @@ Livewire::actingAs($user) ->test(ListTenants::class) ->assertTableActionDisabled('verify', $tenant) + ->assertTableActionExists('verify', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $tenant) ->callTableAction('verify', $tenant); }); @@ -113,7 +122,8 @@ Livewire::actingAs($user) ->test(ListTenants::class) - ->assertTableActionDisabled('edit', $tenant); + ->assertTableActionDisabled('edit', $tenant) + ->assertTableActionExists('edit', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $tenant); }); test('readonly users cannot open admin consent', function () { @@ -126,7 +136,8 @@ Livewire::actingAs($user) ->test(ListTenants::class) - ->assertTableActionDisabled('admin_consent', $tenant); + ->assertTableActionDisabled('admin_consent', $tenant) + ->assertTableActionExists('admin_consent', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $tenant); }); test('readonly users cannot start tenant sync from tenant menu', function () { @@ -138,5 +149,6 @@ Livewire::actingAs($user) ->test(ListTenants::class) - ->assertTableActionDisabled('syncTenant', $tenant); + ->assertTableActionDisabled('syncTenant', $tenant) + ->assertTableActionExists('syncTenant', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $tenant); }); diff --git a/tests/Feature/Filament/TenantPortfolioContextSwitchTest.php b/tests/Feature/Filament/TenantPortfolioContextSwitchTest.php index 14347cf..cde067b 100644 --- a/tests/Feature/Filament/TenantPortfolioContextSwitchTest.php +++ b/tests/Feature/Filament/TenantPortfolioContextSwitchTest.php @@ -4,14 +4,20 @@ use App\Jobs\BulkTenantSyncJob; use App\Models\Tenant; use App\Models\User; +use App\Support\Auth\UiTooltips; use Filament\Events\TenantSet; use Filament\Facades\Filament; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Bus; +use Illuminate\Support\Facades\Http; use Livewire\Livewire; uses(RefreshDatabase::class); +beforeEach(function (): void { + Http::preventStrayRequests(); +}); + test('tenant-scoped pages return 404 for unauthorized tenant', function () { [$user, $authorizedTenant] = createUserWithTenant(); $unauthorizedTenant = Tenant::factory()->create(); @@ -21,6 +27,40 @@ ->assertNotFound(); }); +test('tenant portfolio tenant view returns 404 for non-member tenant record', function () { + $user = User::factory()->create(); + $this->actingAs($user); + + $authorizedTenant = Tenant::factory()->create(['tenant_id' => 'tenant-portfolio-authorized-view']); + $unauthorizedTenant = Tenant::factory()->create(['tenant_id' => 'tenant-portfolio-unauthorized-view']); + + $user->tenants()->syncWithoutDetaching([ + $authorizedTenant->getKey() => ['role' => 'owner'], + ]); + + $this->get(route('filament.admin.resources.tenants.view', array_merge( + filamentTenantRouteParams($unauthorizedTenant), + ['record' => $unauthorizedTenant], + )))->assertNotFound(); +}); + +test('tenant portfolio tenant edit returns 404 for non-member tenant record', function () { + $user = User::factory()->create(); + $this->actingAs($user); + + $authorizedTenant = Tenant::factory()->create(['tenant_id' => 'tenant-portfolio-authorized-edit']); + $unauthorizedTenant = Tenant::factory()->create(['tenant_id' => 'tenant-portfolio-unauthorized-edit']); + + $user->tenants()->syncWithoutDetaching([ + $authorizedTenant->getKey() => ['role' => 'owner'], + ]); + + $this->get(route('filament.admin.resources.tenants.edit', array_merge( + filamentTenantRouteParams($unauthorizedTenant), + ['record' => $unauthorizedTenant], + )))->assertNotFound(); +}); + test('tenant portfolio lists only tenants the user can access', function () { $user = User::factory()->create(); $this->actingAs($user); @@ -75,7 +115,9 @@ ]); }); -test('tenant portfolio bulk sync is hidden for readonly users', function () { +test('tenant portfolio bulk sync is disabled for readonly users', function () { + Bus::fake(); + $user = User::factory()->create(); $this->actingAs($user); @@ -87,8 +129,48 @@ Filament::setTenant($tenant, true); - Livewire::test(ListTenants::class) - ->assertTableBulkActionHidden('syncSelected'); + $livewire = Livewire::actingAs($user) + ->test(ListTenants::class) + ->selectTableRecords([$tenant]) + ->assertTableBulkActionVisible('syncSelected') + ->assertTableBulkActionDisabled('syncSelected'); + + $actions = $livewire->parseNestedTableBulkActions('syncSelected'); + $livewire->assertActionExists($actions, fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission()); + + $livewire->callTableBulkAction('syncSelected', collect([$tenant])); + + Bus::assertNotDispatched(BulkTenantSyncJob::class); +}); + +test('tenant portfolio bulk sync is disabled when selection includes unauthorized tenants', function () { + Bus::fake(); + + $user = User::factory()->create(); + $this->actingAs($user); + + $tenantA = Tenant::factory()->create(['tenant_id' => 'tenant-bulk-mixed-a']); + $tenantB = Tenant::factory()->create(['tenant_id' => 'tenant-bulk-mixed-b']); + + $user->tenants()->syncWithoutDetaching([ + $tenantA->getKey() => ['role' => 'owner'], + $tenantB->getKey() => ['role' => 'readonly'], + ]); + + Filament::setTenant($tenantA, true); + + $livewire = Livewire::actingAs($user) + ->test(ListTenants::class) + ->selectTableRecords([$tenantA, $tenantB]) + ->assertTableBulkActionVisible('syncSelected') + ->assertTableBulkActionDisabled('syncSelected'); + + $actions = $livewire->parseNestedTableBulkActions('syncSelected'); + $livewire->assertActionExists($actions, fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission()); + + $livewire->callTableBulkAction('syncSelected', collect([$tenantA, $tenantB])); + + Bus::assertNotDispatched(BulkTenantSyncJob::class); }); test('tenant set event updates user tenant preference last used timestamp', function () { diff --git a/tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php b/tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php index c6f5bc9..20f04a4 100644 --- a/tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php +++ b/tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php @@ -1,98 +1,117 @@ toBeTrue("Filament directory not found: {$filamentDir}"); + $directories = [ + $root.'/app/Filament', + ]; - /** - * Legacy allowlist: these files currently contain forbidden patterns. - * - * IMPORTANT: - * - Do NOT add new entries casually. - * - The goal is to shrink this list over time. - * - * Paths are workspace-relative (e.g. app/Filament/Resources/Foo.php). - */ - $legacyAllowlist = [ - // Pages (page-level authorization or legacy patterns) - ]; + $excludedPaths = [ + $root.'/vendor', + $root.'/storage', + $root.'/specs', + $root.'/spechistory', + $root.'/references', + $root.'/public/build', + ]; - $patterns = [ - // Gate facade usage - '/\\bGate::(allows|denies|check|authorize)\\b/', - '/^\\s*use\\s+Illuminate\\\\Support\\\\Facades\\\\Gate\\s*;\\s*$/m', + $allowlist = [ + // NOTE: Shrink this list as files are migrated to UiEnforcement (Feature 066b). + 'app/Filament/Pages/ChooseTenant.php', + 'app/Filament/Pages/DriftLanding.php', + 'app/Filament/Pages/Tenancy/RegisterTenant.php', + 'app/Filament/Resources/BackupScheduleResource.php', + 'app/Filament/Resources/FindingResource.php', + 'app/Filament/Resources/FindingResource/Pages/ListFindings.php', + 'app/Filament/Resources/PolicyResource.php', + 'app/Filament/Resources/PolicyResource/Pages/ListPolicies.php', + 'app/Filament/Resources/PolicyResource/RelationManagers/VersionsRelationManager.php', + 'app/Filament/Resources/PolicyVersionResource.php', + 'app/Filament/Resources/TenantResource/RelationManagers/TenantMembershipsRelationManager.php', + 'app/Filament/System/Pages/Dashboard.php', + ]; - // Ad-hoc abort helpers - '/\\babort_(if|unless)\\s*\\(/', - ]; + $forbiddenPatterns = [ + '/\\bGate::\\b/', + '/\\babort_(?:if|unless)\\b/', + ]; - $iterator = new RecursiveIteratorIterator( - new RecursiveDirectoryIterator($filamentDir, RecursiveDirectoryIterator::SKIP_DOTS) - ); + /** @var Collection $files */ + $files = collect($directories) + ->filter(fn (string $dir): bool => is_dir($dir)) + ->flatMap(function (string $dir): array { + $iterator = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($dir, FilesystemIterator::SKIP_DOTS) + ); - /** @var array> $violations */ - $violations = []; + $paths = []; - foreach ($iterator as $file) { - if ($file->getExtension() !== 'php') { - continue; - } - - $absolutePath = $file->getPathname(); - $relativePath = str_replace(base_path().DIRECTORY_SEPARATOR, '', $absolutePath); - $relativePath = str_replace(DIRECTORY_SEPARATOR, '/', $relativePath); - - if (in_array($relativePath, $legacyAllowlist, true)) { - continue; - } - - $content = file_get_contents($absolutePath); - if (! is_string($content)) { - continue; - } - - $lines = preg_split('/\\R/', $content) ?: []; - - foreach ($lines as $lineNumber => $line) { - foreach ($patterns as $pattern) { - if (preg_match($pattern, $line) === 1) { - $violations[$relativePath][] = ($lineNumber + 1).': '.trim($line); - } + foreach ($iterator as $file) { + if (! $file->isFile()) { + continue; } + + $path = $file->getPathname(); + + if (! str_ends_with($path, '.php')) { + continue; + } + + $paths[] = $path; } - } - if ($violations !== []) { - $messageLines = [ - 'Forbidden ad-hoc auth patterns detected in app/Filament/**.', - 'Migrate to UiEnforcement (preferred) or add a justified temporary entry to the legacy allowlist.', - '', - ]; + return $paths; + }) + ->filter(function (string $path) use ($excludedPaths, $self): bool { + if ($self && realpath($path) === $self) { + return false; + } - foreach ($violations as $path => $hits) { - $messageLines[] = $path; - foreach ($hits as $hit) { - $messageLines[] = ' - '.$hit; + foreach ($excludedPaths as $excluded) { + if (str_starts_with($path, $excluded)) { + return false; } } - expect($violations)->toBeEmpty(implode("\n", $messageLines)); + return true; + }) + ->values(); + + $hits = []; + + foreach ($files as $path) { + $relative = str_replace($root.'/', '', $path); + + if (in_array($relative, $allowlist, true)) { + continue; } - expect(true)->toBeTrue(); - }); + $contents = file_get_contents($path); + + if (! is_string($contents) || $contents === '') { + continue; + } + + foreach ($forbiddenPatterns as $pattern) { + if (! preg_match($pattern, $contents)) { + continue; + } + + $lines = preg_split('/\R/', $contents) ?: []; + + foreach ($lines as $index => $line) { + if (preg_match($pattern, $line)) { + $hits[] = $relative.':'.($index + 1).' -> '.trim($line); + } + } + } + } + + expect($hits)->toBeEmpty( + "Ad-hoc Filament auth patterns found (remove allowlist entries as you migrate):\n".implode("\n", $hits) + ); }); diff --git a/tests/Unit/Auth/UiEnforcementBulkPreflightQueryCountTest.php b/tests/Unit/Auth/UiEnforcementBulkPreflightQueryCountTest.php new file mode 100644 index 0000000..5f98ccd --- /dev/null +++ b/tests/Unit/Auth/UiEnforcementBulkPreflightQueryCountTest.php @@ -0,0 +1,36 @@ +count(25)->create(); + [$user] = createUserWithTenant($tenants->first(), role: 'owner'); + + foreach ($tenants->slice(1) as $tenant) { + $user->tenants()->syncWithoutDetaching([ + $tenant->getKey() => ['role' => 'owner'], + ]); + } + + $enforcement = UiEnforcement::for(Capabilities::TENANT_SYNC) + ->tenantFromRecord() + ->preflightByCapability(); + + $membershipQueries = 0; + + DB::listen(function ($query) use (&$membershipQueries): void { + if (str_contains($query->sql, 'tenant_memberships')) { + $membershipQueries++; + } + }); + + expect($enforcement->bulkSelectionIsAuthorized($user, $tenants))->toBeTrue(); + expect($membershipQueries)->toBe(1); +}); + diff --git a/tests/Unit/Auth/UiEnforcementTest.php b/tests/Unit/Auth/UiEnforcementTest.php new file mode 100644 index 0000000..74b4284 --- /dev/null +++ b/tests/Unit/Auth/UiEnforcementTest.php @@ -0,0 +1,128 @@ + 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'); + + UiEnforcement::for(Capabilities::TENANT_VIEW) + ->tenantFromRecord() + ->apply($action); + + $this->actingAs($user); + $action->record($tenant); + + expect($action->isHidden())->toBeTrue(); +}); + +it('disables actions with the standard tooltip for members without the capability', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'readonly'); + + $action = Action::make('test'); + + UiEnforcement::for(Capabilities::TENANT_SYNC) + ->tenantFromRecord() + ->apply($action); + + $this->actingAs($user); + $action->record($tenant); + + expect($action->isHidden())->toBeFalse(); + expect($action->isDisabled())->toBeTrue(); + expect($action->getTooltip())->toBe(UiTooltips::insufficientPermission()); +}); + +it('enables actions for members with the capability', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'owner'); + + $action = Action::make('test'); + + UiEnforcement::for(Capabilities::TENANT_SYNC) + ->tenantFromRecord() + ->apply($action); + + $this->actingAs($user); + $action->record($tenant); + + expect($action->isHidden())->toBeFalse(); + expect($action->isDisabled())->toBeFalse(); + expect($action->getTooltip())->toBeNull(); +}); + +it('supports mixed visibility composition via andVisibleWhen', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'owner'); + + Filament::setTenant($tenant, true); + + $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); + + $this->actingAs($user); + + expect($action->isHidden())->toBeTrue(); +}); + +it('disables bulk actions for mixed-authorization selections (capability preflight)', function () { + $tenantA = Tenant::factory()->create(); + $tenantB = Tenant::factory()->create(); + + [$user] = createUserWithTenant($tenantA, role: 'owner'); + $user->tenants()->syncWithoutDetaching([ + $tenantB->getKey() => ['role' => 'readonly'], + ]); + + $enforcement = UiEnforcement::for(Capabilities::TENANT_SYNC) + ->tenantFromRecord() + ->preflightByCapability(); + + expect($enforcement->bulkSelectionIsAuthorized($user, collect([$tenantA, $tenantB])))->toBeFalse(); + + $user->tenants()->syncWithoutDetaching([ + $tenantB->getKey() => ['role' => 'owner'], + ]); + + expect($enforcement->bulkSelectionIsAuthorized($user, collect([$tenantA, $tenantB])))->toBeTrue(); +}); +