Implements Spec 090 (Action Surface Contract Compliance & RBAC Hardening). Highlights: - Adds/updates action surface declarations and shrinks baseline exemptions. - Standardizes Filament action grouping/order and empty-state CTAs. - Enforces RBAC UX semantics (non-member -> 404, member w/o capability -> disabled + tooltip, server-side 403). - Adds audit logging for successful side-effect actions. - Fixes Provider Connections list context so header create + row actions resolve tenant correctly. Tests (focused): - vendor/bin/sail artisan test --compact tests/Feature/090/ - vendor/bin/sail artisan test --compact tests/Feature/Guards/ActionSurfaceContractTest.php - vendor/bin/sail bin pint --dirty Livewire/Filament: - Filament v5 + Livewire v4 compliant. - No panel provider registration changes (Laravel 11+ registration remains in bootstrap/providers.php). Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de> Reviewed-on: #108
85 lines
5.3 KiB
Markdown
85 lines
5.3 KiB
Markdown
# Research — Spec 090 (Action Surface Contract Compliance & RBAC Hardening)
|
||
|
||
## Decisions
|
||
|
||
### Decision 1 — Use the repo’s existing Action Surface Contract guard
|
||
- **Chosen**: Retrofit in-scope Filament Resources/Pages/RelationManagers by adding `actionSurfaceDeclaration()` and removing baseline exemptions in `App\Support\Ui\ActionSurface\ActionSurfaceExemptions::baseline()`.
|
||
- **Rationale**: The repository already enforces the constitution’s Filament Action Surface Contract through `tests/Feature/Guards/ActionSurfaceContractTest.php`. Aligning to that system avoids bespoke checks and keeps CI gates meaningful.
|
||
- **Alternatives considered**:
|
||
- Ad-hoc “View button exists” UI assertions per resource → rejected (doesn’t cover grouping/bulk/empty-state semantics; duplicates existing guard).
|
||
|
||
### Decision 2 — Interpret “View affordance” as an *inspection affordance* (not always a literal View button)
|
||
- **Chosen**: Treat the spec’s “visible View” requirement as “a visible inspection affordance”, matching the constitution’s accepted forms:
|
||
- clickable rows via `recordUrl()` (preferred when View would be the only row action)
|
||
- a primary linked column
|
||
- a dedicated `View` row action (when there are other row actions like Edit)
|
||
- **Rationale**: The constitution explicitly forbids a lone View row button and prefers clickable rows in that case. The repo already uses this approach for read-only, list-only surfaces (e.g., inventory items).
|
||
- **Alternatives considered**:
|
||
- Add `ViewAction` everywhere → rejected (would create “lone View” buttons on read-only lists, conflicting with constitution and existing tests).
|
||
|
||
### Decision 3 — Reuse central RBAC UI enforcement helpers for all Filament actions
|
||
- **Chosen**: For tenant-scoped actions, use `App\Support\Rbac\UiEnforcement`.
|
||
For workspace-scoped actions, use `App\Support\Rbac\WorkspaceUiEnforcement`.
|
||
- **Rationale**: These helpers implement the constitution’s RBAC-UX semantics (non-member 404, member missing capability 403, disabled + tooltip in UI) and validate capabilities against the canonical registry.
|
||
- **Alternatives considered**:
|
||
- Inline `->authorize(...)`, ad-hoc `->visible(...)`, or `->disabled(...)` checks → rejected (too easy to drift; violates “central helpers” expectation).
|
||
|
||
### Decision 4 — Audit successful side-effect actions via existing audit log services
|
||
- **Chosen**:
|
||
- Tenant-scoped audit entries: `App\Services\Intune\AuditLogger` (writes to `audit_logs` with `tenant_id`).
|
||
- Workspace-scoped audit entries: `App\Services\Audit\WorkspaceAuditLogger` (writes to `audit_logs` with `workspace_id`, `tenant_id = null`).
|
||
- **Rationale**: The repo already uses these loggers and sanitizes metadata via `AuditContextSanitizer`.
|
||
- **Alternatives considered**:
|
||
- Logging denied/cancelled attempts → rejected (explicitly out of scope for Spec 090).
|
||
|
||
## Current Repo State (Key Findings)
|
||
|
||
### Action Surface Contract infrastructure exists
|
||
- Guard tests:
|
||
- `tests/Feature/Guards/ActionSurfaceContractTest.php`
|
||
- `tests/Feature/Guards/ActionSurfaceValidatorTest.php`
|
||
- Runtime support:
|
||
- `app/Support/Ui/ActionSurface/*`
|
||
- Many “legacy” resources are exempt via:
|
||
- `app/Support/Ui/ActionSurface/ActionSurfaceExemptions.php`
|
||
|
||
### In-scope components are currently exempt from the contract guard
|
||
The constitution guard currently exempts (among others):
|
||
- `App\Filament\Resources\ProviderConnectionResource`
|
||
- `App\Filament\Resources\BackupScheduleResource`
|
||
- `App\Filament\Resources\FindingResource`
|
||
- `App\Filament\Resources\TenantResource`
|
||
- `App\Filament\Resources\Workspaces\WorkspaceResource`
|
||
|
||
Exact baseline exemptions to remove in Spec 090 implementation:
|
||
- `App\Filament\Resources\ProviderConnectionResource`
|
||
- `App\Filament\Resources\BackupScheduleResource`
|
||
- `App\Filament\Resources\FindingResource`
|
||
- `App\Filament\Resources\TenantResource`
|
||
- `App\Filament\Resources\Workspaces\WorkspaceResource`
|
||
|
||
Spec 090’s intent is to shrink that exemption list by bringing these in-scope surfaces into compliance.
|
||
|
||
### RBAC helpers are present and already used on some surfaces
|
||
- `UiEnforcement` provides:
|
||
- non-member hidden + server abort(404)
|
||
- member missing capability disabled + tooltip + server abort(403)
|
||
- destructive confirmation via `->requiresConfirmation()` (generic destructive copy)
|
||
|
||
### Notable hotspot: Policy “Capture snapshot”
|
||
- Implemented in `app/Filament/Resources/PolicyResource/Pages/ViewPolicy.php`.
|
||
- Currently:
|
||
- has confirmation + correct domain copy
|
||
- queues an `OperationRun`
|
||
- **needs** capability-first gating and audit logging for successful dispatch.
|
||
|
||
### Notable semantics mismatch: Policy list “Sync from Intune”
|
||
- Implemented in `app/Filament/Resources/PolicyResource/Pages/ListPolicies.php`.
|
||
- The action uses `UiEnforcement::destructive()` to obtain confirmation, which applies generic destructive modal copy.
|
||
- Per Spec 090 + constitution: sync/run actions should not be presented as destructive.
|
||
|
||
## Open Questions Resolved By Repo Evidence
|
||
- **How is action-surface compliance enforced?** → Via `ActionSurfaceValidator` + `ActionSurfaceContractTest`.
|
||
- **How to log audits?** → Use `AuditLogger` / `WorkspaceAuditLogger` with sanitized metadata.
|
||
- **How to apply RBAC consistently?** → Wrap Filament actions with `UiEnforcement` / `WorkspaceUiEnforcement`.
|