189 lines
8.0 KiB
Markdown
189 lines
8.0 KiB
Markdown
# Implementation Plan: RBAC UI Enforcement Helper v1
|
||
|
||
**Branch**: `066-rbac-ui-enforcement-helper` | **Date**: 2026-01-28 | **Spec**: [spec.md](./spec.md)
|
||
**Input**: Feature specification from `/specs/066-rbac-ui-enforcement-helper/spec.md`
|
||
|
||
## Summary
|
||
|
||
Provide a single, centrally maintained enforcement helper (`UiEnforcement`) that codifies the RBAC-UX constitution rules for tenant-scoped Filament actions:
|
||
|
||
- Non-member → 404 (deny-as-not-found), hidden in UI
|
||
- Member without capability → 403 on execution, visible-but-disabled in UI with standard tooltip
|
||
- Member with capability → enabled
|
||
- Destructive actions → `requiresConfirmation()` + clear warning
|
||
|
||
The helper wraps/augments Filament Actions (header, table row, bulk) to provide default UI + server-side enforcement, and ships with regression tests + a CI-failing guard against ad-hoc authorization patterns.
|
||
|
||
## Technical Context
|
||
|
||
**Language/Version**: PHP 8.4 (Laravel 12)
|
||
**Primary Dependencies**: Filament v5, Livewire v4
|
||
**Storage**: PostgreSQL (existing tables — no new tables)
|
||
**Testing**: Pest v4 (Feature + Unit tests)
|
||
**Target Platform**: Docker / Sail local, Dokploy VPS (Linux)
|
||
**Project Type**: Web / Monolith (backend + Filament admin)
|
||
**Performance Goals**: No additional DB queries beyond request-scope cached membership (FR-012)
|
||
**Constraints**: DB-only at render time (FR-013); no outbound HTTP
|
||
**Scale/Scope**: ~40+ tenant-scoped action surfaces; v1 migrates 3–6 exemplar surfaces
|
||
|
||
## Constitution Check
|
||
|
||
*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*
|
||
|
||
| Principle | Status | Notes |
|
||
|-----------|--------|-------|
|
||
| Inventory-first | N/A | No Inventory changes |
|
||
| Read/write separation | ✔ | Helper enforces existing gates; no new writes |
|
||
| Graph contract path | N/A | No Graph calls |
|
||
| Deterministic capabilities | ✔ | Uses existing `Capabilities` registry |
|
||
| RBAC-UX planes | ✔ | Tenant-plane only; cross-plane logic untouched |
|
||
| Tenant isolation | ✔ | 404 for non-members; capability check requires membership first |
|
||
| Run observability | N/A | No long-running work; helper is request-scope only |
|
||
| Data minimization | ✔ | No additional logging beyond existing deny logs |
|
||
| Badge semantics | N/A | No badge changes |
|
||
|
||
## Existing RBAC Primitives (Research)
|
||
|
||
| Component | Location | Purpose |
|
||
|-----------|----------|---------|
|
||
| `Capabilities` | `app/Support/Auth/Capabilities.php` | Canonical tenant capability registry (constants) |
|
||
| `PlatformCapabilities` | `app/Support/Auth/PlatformCapabilities.php` | Platform-plane capabilities |
|
||
| `RoleCapabilityMap` | `app/Services/Auth/RoleCapabilityMap.php` | Role → capabilities mapping |
|
||
| `CapabilityResolver` | `app/Services/Auth/CapabilityResolver.php` | Request-scope cached role/capability resolution |
|
||
| `User::canAccessTenant()` | `app/Models/User.php:123` | Membership check |
|
||
| `AuthServiceProvider` | `app/Providers/AuthServiceProvider.php` | Registers Gates for all capabilities |
|
||
| Existing ad-hoc patterns | `app/Filament/**` | 50+ `->visible(fn ...)` / `->disabled(fn ...)` calls — target for migration |
|
||
|
||
## Project Structure
|
||
|
||
### Documentation (this feature)
|
||
|
||
```text
|
||
specs/066-rbac-ui-enforcement-helper/
|
||
├── spec.md # Feature specification
|
||
├── plan.md # This file
|
||
├── research.md # (no separate file needed — inline above)
|
||
├── data-model.md # (no schema changes)
|
||
├── quickstart.md # Adoption guide
|
||
├── checklists/
|
||
│ └── requirements.md # Spec quality checklist
|
||
└── tasks.md # Phase 2 output (/speckit.tasks)
|
||
```
|
||
|
||
### Source Code (repository root)
|
||
|
||
```text
|
||
app/
|
||
├── Support/
|
||
│ └── Rbac/
|
||
│ ├── UiEnforcement.php # Central facade/builder
|
||
│ ├── TenantAccessContext.php # DTO: tenant, user, isMember, capabilityCheck
|
||
│ └── UiTooltips.php # Standardized tooltip strings
|
||
├── Services/Auth/
|
||
│ ├── CapabilityResolver.php # (existing, reused)
|
||
│ └── RoleCapabilityMap.php # (existing, reused)
|
||
├── Filament/
|
||
│ └── Resources/... # 3–6 exemplar migrations
|
||
|
||
tests/
|
||
├── Feature/
|
||
│ └── Rbac/
|
||
│ └── UiEnforcementTest.php # Integration tests
|
||
│ └── Guards/
|
||
│ └── NoAdHocFilamentAuthPatternsTest.php # CI-failing guard (file-scan)
|
||
├── Unit/
|
||
│ └── Support/Rbac/
|
||
│ └── UiEnforcementTest.php # Unit tests
|
||
|
||
```
|
||
|
||
**Structure Decision**: All new code lives in `app/Support/Rbac/` (helper) + tests; no new models/tables required.
|
||
|
||
## Key Design Decisions
|
||
|
||
### UiEnforcement API (FR-001)
|
||
|
||
```php
|
||
use App\Support\Rbac\UiEnforcement;
|
||
|
||
// Basic usage
|
||
UiEnforcement::forAction($action)
|
||
->requireMembership() // default: true
|
||
->requireCapability(Capabilities::PROVIDER_MANAGE)
|
||
->destructive() // optional: adds confirmation
|
||
->apply();
|
||
|
||
// Table/row action (receives record or record-accessor closure)
|
||
UiEnforcement::forTableAction(Action $action, Model|Closure $record)
|
||
->requireCapability(Capabilities::TENANT_DELETE)
|
||
->destructive()
|
||
->apply();
|
||
|
||
// Mixed visibility support (keep business visibility, add RBAC visibility)
|
||
UiEnforcement::forAction($action)
|
||
->preserveVisibility()
|
||
->requireCapability(Capabilities::TENANT_MANAGE)
|
||
->apply();
|
||
|
||
// Bulk action (all-or-nothing)
|
||
UiEnforcement::forBulkAction($action)
|
||
->requireCapability(Capabilities::TENANT_MANAGE)
|
||
->apply();
|
||
```
|
||
|
||
Internally:
|
||
1. Resolves current tenant + user via `Filament::getTenant()` + `auth()->user()`
|
||
2. Checks membership via `CapabilityResolver` (request-scope cached)
|
||
3. Sets `->hidden()` for non-members (FR-002a)
|
||
4. Sets `->disabled()` + `->tooltip()` for members without capability (FR-004)
|
||
5. Wraps handler with server-side guard (FR-005): `abort(404)` / `abort(403)`
|
||
|
||
### Tooltip Copy (FR-008)
|
||
|
||
```php
|
||
class UiTooltips
|
||
{
|
||
public const INSUFFICIENT_PERMISSION = 'You don\'t have permission to do this. Ask a tenant admin.';
|
||
public const DESTRUCTIVE_CONFIRM_TITLE = 'Are you sure?';
|
||
public const DESTRUCTIVE_CONFIRM_DESCRIPTION = 'This action cannot be undone.';
|
||
}
|
||
```
|
||
|
||
### Destructive Confirmation (FR-007)
|
||
|
||
`->destructive()` calls:
|
||
- `$action->requiresConfirmation()`
|
||
- `$action->modalHeading(UiTooltips::DESTRUCTIVE_CONFIRM_TITLE)`
|
||
- `$action->modalDescription(UiTooltips::DESTRUCTIVE_CONFIRM_DESCRIPTION)`
|
||
|
||
### All-or-nothing Bulk (FR-010a)
|
||
|
||
Before rendering, bulk action checks all selected records. If any record fails capability check for the member, action is disabled.
|
||
|
||
### Guardrail (FR-011)
|
||
|
||
`tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php` scans `app/Filament/**` for forbidden patterns like:
|
||
- `Gate::allows(...)` / `Gate::denies(...)`
|
||
- `abort_if(...)` / `abort_unless(...)`
|
||
|
||
It uses a legacy allowlist so CI fails only for **new** violations, and the allowlist should shrink as resources are migrated.
|
||
|
||
## v1 Migration Targets (FR-009)
|
||
|
||
| Surface | File | Current Pattern | Notes |
|
||
|---------|------|-----------------|-------|
|
||
| TenantResource table actions | `TenantResource.php` | Multiple `->visible(fn ...)` + `->disabled(fn ...)` | High-traffic, high-value |
|
||
| ProviderConnectionResource actions | `EditProviderConnection.php` | Multiple `canAccessTenant` + capability checks inline | Complex, good test case |
|
||
| BackupSetResource table actions | `BackupSetResource.php` | Many `->disabled(fn ...)` closures | Destructive actions |
|
||
| PolicyResource ListPolicies sync | `ListPolicies.php` | Inline checks | Good example |
|
||
| EntraGroupResource sync | `ListEntraGroups.php` | Inline checks | Good example |
|
||
| FindingResource actions | `FindingResource.php` | `->authorize(fn ...)` inline | Good example |
|
||
|
||
## Complexity Tracking
|
||
|
||
> No constitution violations. Complexity is low (helper + tests + 3–6 migrations).
|
||
|
||
| Violation | Why Needed | Simpler Alternative Rejected Because |
|
||
|-----------|------------|-------------------------------------|
|
||
| — | — | — |
|