Compare commits
4 Commits
dev
...
spec/066-r
| Author | SHA1 | Date | |
|---|---|---|---|
|
c48c99a857 | ||
|
|
a53bb3f708 | ||
|
|
07dda36d6e | ||
|
|
e5ad9b6cf8 |
@ -1,8 +1,8 @@
|
|||||||
# Specification Quality Checklist: RBAC UI Enforcement Helper v1
|
# Specification Quality Checklist: RBAC UI Enforcement Helper v2 (Suite-wide, Mixed Visibility, Record-Scoped)
|
||||||
|
|
||||||
**Purpose**: Validate specification completeness and quality before proceeding to planning
|
**Purpose**: Validate specification completeness and quality before proceeding to planning
|
||||||
**Created**: 2026-01-28
|
**Created**: 2026-01-30
|
||||||
**Feature**: [specs/066-rbac-ui-enforcement-helper/spec.md](../spec.md)
|
**Feature**: [spec.md](../spec.md)
|
||||||
|
|
||||||
## Content Quality
|
## Content Quality
|
||||||
|
|
||||||
@ -31,4 +31,6 @@ ## Feature Readiness
|
|||||||
|
|
||||||
## Notes
|
## Notes
|
||||||
|
|
||||||
- No blockers found in this iteration.
|
- Validation run: 2026-01-30
|
||||||
|
- Clarifications captured under `### Session 2026-01-30` and integrated into requirements/design sections.
|
||||||
|
- All checks passed; spec is ready for `/speckit.plan`.
|
||||||
|
|||||||
31
specs/066-rbac-ui-enforcement-helper/contracts/guardrails.md
Normal file
31
specs/066-rbac-ui-enforcement-helper/contracts/guardrails.md
Normal file
@ -0,0 +1,31 @@
|
|||||||
|
# Contract: RBAC Guardrails for Filament (v2)
|
||||||
|
|
||||||
|
**Branch**: `066-rbac-ui-enforcement-helper-v2`
|
||||||
|
**Date**: 2026-01-30
|
||||||
|
**Spec**: `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/spec.md`
|
||||||
|
|
||||||
|
## Goal
|
||||||
|
|
||||||
|
Prevent re-introducing ad-hoc RBAC authorization patterns inside `app/Filament/**` by keeping a CI-failing, allowlist-driven guard.
|
||||||
|
|
||||||
|
## Guard contract
|
||||||
|
|
||||||
|
- The guard is CI-failing for new violations (no “warnings only” mode).
|
||||||
|
- Legacy violations are allowed only via an explicit allowlist.
|
||||||
|
- Every v2 migration batch removes allowlist entries for the migrated files.
|
||||||
|
|
||||||
|
## Forbidden patterns (examples)
|
||||||
|
|
||||||
|
The exact pattern list is defined in the guard test, but v2 migrations must remove ad-hoc patterns such as:
|
||||||
|
|
||||||
|
- `Gate::...` checks inside Filament resources/pages when the enforcement helper is intended.
|
||||||
|
- `abort_*` calls used as the primary UI enforcement mechanism inside Filament resources/pages.
|
||||||
|
- Raw policy ability string literals (instead of `Capabilities::*` constants).
|
||||||
|
- Custom RBAC helper patterns that bypass `UiEnforcement`.
|
||||||
|
|
||||||
|
## Allowed patterns
|
||||||
|
|
||||||
|
- Capability checks via `UiEnforcement` and `Capabilities::*` registry constants.
|
||||||
|
- Defense-in-depth server-side authorization inside action handlers (403 for members without capability; 404 for non-members where reachable).
|
||||||
|
- Filament action UX patterns: `->requiresConfirmation()` for destructive/high-impact actions.
|
||||||
|
|
||||||
@ -0,0 +1,83 @@
|
|||||||
|
# Contract: UiEnforcement v2 (RBAC UI Enforcement Helper)
|
||||||
|
|
||||||
|
**Branch**: `066-rbac-ui-enforcement-helper-v2`
|
||||||
|
**Date**: 2026-01-30
|
||||||
|
**Spec**: `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/spec.md`
|
||||||
|
|
||||||
|
## Scope
|
||||||
|
|
||||||
|
- Tenant plane only (`/admin/t/{tenant}`).
|
||||||
|
- Platform plane (`/system`) is out of scope for v2.
|
||||||
|
|
||||||
|
## RBAC-UX Contract (summary)
|
||||||
|
|
||||||
|
1) **Non-member**
|
||||||
|
- Actions are hidden and cannot execute.
|
||||||
|
- Direct access/execution attempts must deny-as-not-found (404) where reachable.
|
||||||
|
|
||||||
|
2) **Member without capability**
|
||||||
|
- Action is visible but disabled.
|
||||||
|
- Disabled tooltip is standardized via `UiTooltips` (default copy is defined in the feature spec).
|
||||||
|
- Disabled action cannot execute (Filament no-op is acceptable).
|
||||||
|
|
||||||
|
3) **Member with capability**
|
||||||
|
- Action is enabled and executes.
|
||||||
|
- Destructive/high-impact actions require confirmation (`->requiresConfirmation()`).
|
||||||
|
|
||||||
|
## Mixed visibility composition (business + RBAC)
|
||||||
|
|
||||||
|
### `preserveVisibility()`
|
||||||
|
|
||||||
|
**Contract**
|
||||||
|
- The helper MUST NOT write `->visible(...)` or `->hidden(...)`.
|
||||||
|
- The helper MAY still set disabled state + tooltip + confirmation + server-side guard.
|
||||||
|
|
||||||
|
**Restriction**
|
||||||
|
- Allowed only on tenant-scoped surfaces where routing already denies non-members (404).
|
||||||
|
- Forbidden for record-scoped / cross-tenant lists (must not leak discoverability).
|
||||||
|
|
||||||
|
### `andVisibleWhen(callable $businessVisible)`
|
||||||
|
|
||||||
|
**Contract**
|
||||||
|
- The helper combines business visibility AND RBAC visibility.
|
||||||
|
- If business visibility is false, the action is not visible (even for authorized members).
|
||||||
|
|
||||||
|
### `andHiddenWhen(callable $businessHidden)`
|
||||||
|
|
||||||
|
**Contract**
|
||||||
|
- The helper combines business-hidden OR RBAC-hidden semantics.
|
||||||
|
|
||||||
|
## Tenant context providers
|
||||||
|
|
||||||
|
### `tenantFromFilament()` (default)
|
||||||
|
|
||||||
|
- Uses Filament tenant context (`Filament::getTenant()` or equivalent).
|
||||||
|
- Intended for tenant-scoped pages/actions.
|
||||||
|
|
||||||
|
### `tenantFromRecord()`
|
||||||
|
|
||||||
|
- Treats `$record` as the tenant.
|
||||||
|
- Intended for record-scoped surfaces like `TenantResource` row actions.
|
||||||
|
|
||||||
|
### `tenantFrom(callable $resolver)`
|
||||||
|
|
||||||
|
- Maps a record (or selection) to a tenant instance.
|
||||||
|
- Intended for “record → tenant” mapping surfaces.
|
||||||
|
|
||||||
|
## Bulk preflight authorization
|
||||||
|
|
||||||
|
### Default behavior (v2)
|
||||||
|
|
||||||
|
- All-or-nothing **authorization**: if any selected record is unauthorized, the bulk action is disabled and cannot execute.
|
||||||
|
- Business eligibility is handled separately by the action implementation (e.g., skip inactive/archived records with clear feedback).
|
||||||
|
|
||||||
|
### Preflight hooks
|
||||||
|
|
||||||
|
- Custom: `preflightSelection(callable $preflight)`
|
||||||
|
- Built-ins:
|
||||||
|
- `preflightByTenantMembership()`
|
||||||
|
- `preflightByCapability()`
|
||||||
|
|
||||||
|
**Performance**
|
||||||
|
- Preflight should use set-based DB queries where feasible; avoid N+1 membership checks.
|
||||||
|
|
||||||
75
specs/066-rbac-ui-enforcement-helper/data-model.md
Normal file
75
specs/066-rbac-ui-enforcement-helper/data-model.md
Normal file
@ -0,0 +1,75 @@
|
|||||||
|
# Data Model: RBAC UI Enforcement Helper v2
|
||||||
|
|
||||||
|
**Branch**: `066-rbac-ui-enforcement-helper-v2`
|
||||||
|
**Date**: 2026-01-30
|
||||||
|
**Spec**: `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/spec.md`
|
||||||
|
|
||||||
|
## Overview
|
||||||
|
|
||||||
|
This feature does not introduce new database tables. It defines contracts and behaviors that compose existing tenant-plane RBAC (membership + capability gates) into consistent Filament UI states across tenant-scoped and record-scoped surfaces.
|
||||||
|
|
||||||
|
## Entities (existing)
|
||||||
|
|
||||||
|
### Tenant
|
||||||
|
|
||||||
|
The isolation boundary for all tenant-plane actions.
|
||||||
|
|
||||||
|
**Identity**
|
||||||
|
- `tenants.id` (PK)
|
||||||
|
- A user is a “member” if `user->canAccessTenant($tenant)` is true.
|
||||||
|
|
||||||
|
### User
|
||||||
|
|
||||||
|
Authenticated tenant-plane actor.
|
||||||
|
|
||||||
|
**Key behaviors**
|
||||||
|
- Must support membership checks (`canAccessTenant($tenant)`).
|
||||||
|
- Capability checks are performed via Gates/Policies using `Capabilities::*` identifiers.
|
||||||
|
|
||||||
|
### TenantMembership
|
||||||
|
|
||||||
|
Joins a User to a Tenant with a role mapped to capabilities (Feature 065).
|
||||||
|
|
||||||
|
**Key behaviors**
|
||||||
|
- Role → capabilities mapping is centralized and deterministic.
|
||||||
|
- Requests should avoid repeated membership DB queries (request-scope cache).
|
||||||
|
|
||||||
|
### Capability (registry)
|
||||||
|
|
||||||
|
Canonical capability keys live in `app/Support/Auth/Capabilities.php` and must be referenced via constants (`Capabilities::*`).
|
||||||
|
|
||||||
|
## Entities (conceptual, not persisted)
|
||||||
|
|
||||||
|
### UiEnforcementContext
|
||||||
|
|
||||||
|
Context used to evaluate membership/capability for an action.
|
||||||
|
|
||||||
|
**Fields**
|
||||||
|
- `user`: current authenticated user (or null)
|
||||||
|
- `tenantResolver`: one of:
|
||||||
|
- `tenantFromFilament` (tenant-scoped)
|
||||||
|
- `tenantFromRecord` (record == tenant)
|
||||||
|
- `tenantFrom(callable)` (record → tenant mapping)
|
||||||
|
- `record`: optional record for table/record actions
|
||||||
|
- `capability`: required capability identifier (from `Capabilities::*`)
|
||||||
|
|
||||||
|
### UiEnforcementDecision
|
||||||
|
|
||||||
|
The derived UI behavior applied to a Filament action.
|
||||||
|
|
||||||
|
**Fields**
|
||||||
|
- `isVisible`: boolean (non-members must not be able to discover actions)
|
||||||
|
- `isEnabled`: boolean (members without capability see disabled action)
|
||||||
|
- `disabledTooltip`: string (standardized via `UiTooltips`)
|
||||||
|
- `requiresConfirmation`: boolean (destructive/high-impact actions)
|
||||||
|
|
||||||
|
### BulkSelectionAuthorization (preflight)
|
||||||
|
|
||||||
|
Derived authorization status for a bulk action selection.
|
||||||
|
|
||||||
|
**Fields**
|
||||||
|
- `selectedIds`: list of record IDs
|
||||||
|
- `resolvedTenants`: list of tenant IDs derived via tenant resolver (record == tenant or mapping)
|
||||||
|
- `unauthorizedCount`: count of records failing authorization (determines all-or-nothing disable)
|
||||||
|
- `ineligibleCount`: optional count of business-ineligible records (may be skipped with deterministic feedback; does not affect authorization disable by default)
|
||||||
|
|
||||||
@ -1,58 +1,51 @@
|
|||||||
# Implementation Plan: RBAC UI Enforcement Helper v1
|
# Implementation Plan: RBAC UI Enforcement Helper v2 (Suite-wide, Mixed Visibility, Record-Scoped)
|
||||||
|
|
||||||
**Branch**: `066-rbac-ui-enforcement-helper` | **Date**: 2026-01-28 | **Spec**: [spec.md](./spec.md)
|
**Branch**: `066-rbac-ui-enforcement-helper-v2` | **Date**: 2026-01-30 | **Spec**: `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/spec.md`
|
||||||
**Input**: Feature specification from `/specs/066-rbac-ui-enforcement-helper/spec.md`
|
**Input**: Feature specification from `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/spec.md`
|
||||||
|
|
||||||
|
**Note**: This template is filled in by the `/speckit.plan` command. See `.specify/scripts/` for helper scripts.
|
||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
Provide a single, centrally maintained enforcement helper (`UiEnforcement`) that codifies the RBAC-UX constitution rules for tenant-scoped Filament actions:
|
- Extend the RBAC UI enforcement suite (Feature 066 v1) with explicit primitives for:
|
||||||
|
- mixed visibility composition (business visibility + RBAC visibility),
|
||||||
- Non-member → 404 (deny-as-not-found), hidden in UI
|
- record-scoped tenant resolution (record == tenant, or record → tenant mapping),
|
||||||
- Member without capability → 403 on execution, visible-but-disabled in UI with standard tooltip
|
- deterministic bulk preflight authorization with all-or-nothing semantics (authorization-only by default).
|
||||||
- Member with capability → enabled
|
- Migrate the highest-risk remaining Filament surfaces (BackupSet/RestoreRun mixed visibility, TenantResource record-scoped actions), plus Tier 2 allowlist “easy wins”, shrinking the CI-failing guard allowlist.
|
||||||
- Destructive actions → `requiresConfirmation()` + clear warning
|
- Add focused integration tests per migrated surface to assert the RBAC-UX contract (hidden/disabled + tooltip + non-execution), and keep render DB-only (no outbound HTTP).
|
||||||
|
|
||||||
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
|
## Technical Context
|
||||||
|
|
||||||
|
<!--
|
||||||
|
ACTION REQUIRED: Replace the content in this section with the technical details
|
||||||
|
for the project. The structure here is presented in advisory capacity to guide
|
||||||
|
the iteration process.
|
||||||
|
-->
|
||||||
|
|
||||||
**Language/Version**: PHP 8.4 (Laravel 12)
|
**Language/Version**: PHP 8.4 (Laravel 12)
|
||||||
**Primary Dependencies**: Filament v5, Livewire v4
|
**Primary Dependencies**: Laravel 12 + Filament v5 + Livewire v4
|
||||||
**Storage**: PostgreSQL (existing tables — no new tables)
|
**Storage**: PostgreSQL (Sail)
|
||||||
**Testing**: Pest v4 (Feature + Unit tests)
|
**Testing**: Pest (PHPUnit)
|
||||||
**Target Platform**: Docker / Sail local, Dokploy VPS (Linux)
|
**Target Platform**: Web application (Sail/Docker locally; container deploy via Dokploy)
|
||||||
**Project Type**: Web / Monolith (backend + Filament admin)
|
**Project Type**: web
|
||||||
**Performance Goals**: No additional DB queries beyond request-scope cached membership (FR-012)
|
**Performance Goals**: Membership/capability evaluation is O(1) per request after initial load; bulk preflight avoids N+1 (set-based where feasible)
|
||||||
**Constraints**: DB-only at render time (FR-013); no outbound HTTP
|
**Constraints**: DB-only render/hydration; no outbound HTTP during render; tenant isolation; hidden/disabled actions must not execute; destructive actions require confirmation
|
||||||
**Scale/Scope**: ~40+ tenant-scoped action surfaces; v1 migrates 3–6 exemplar surfaces
|
**Scale/Scope**: Suite-wide helper primitives + targeted migrations (Tier 1 + Tier 2 hitlist) rather than a full Filament sweep
|
||||||
|
|
||||||
## Constitution Check
|
## Constitution Check
|
||||||
|
|
||||||
*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*
|
*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*
|
||||||
|
|
||||||
| Principle | Status | Notes |
|
- Inventory-first, snapshots-second: N/A (helper only; no inventory semantics changed).
|
||||||
|-----------|--------|-------|
|
- Read/write separation: PASS (no new write flows introduced; existing destructive flows remain confirmation-gated and server-authorized).
|
||||||
| Inventory-first | N/A | No Inventory changes |
|
- Single contract path to Graph: PASS (helper + UI rendering remain DB-only; no Graph calls introduced).
|
||||||
| Read/write separation | ✔ | Helper enforces existing gates; no new writes |
|
- Deterministic capabilities: PASS (migrated surfaces reference `Capabilities::*` only; no ad-hoc ability strings).
|
||||||
| Graph contract path | N/A | No Graph calls |
|
- RBAC Standard: PASS (tenant plane only; platform plane out of scope; non-member is deny-as-not-found/404).
|
||||||
| Deterministic capabilities | ✔ | Uses existing `Capabilities` registry |
|
- Tenant isolation: PASS (record-scoped tenant resolution evaluates membership per tenant record; non-members receive 404 on direct access).
|
||||||
| RBAC-UX planes | ✔ | Tenant-plane only; cross-plane logic untouched |
|
- Operations / run observability standard: N/A (no changes to OperationRun orchestration; only UI authorization enforcement).
|
||||||
| Tenant isolation | ✔ | 404 for non-members; capability check requires membership first |
|
- Automation (locks + idempotency): N/A (no new queued ops introduced).
|
||||||
| Run observability | N/A | No long-running work; helper is request-scope only |
|
- Data minimization & safe logging: PASS (no new persisted payloads; enforcement must not log secrets/PII).
|
||||||
| Data minimization | ✔ | No additional logging beyond existing deny logs |
|
- Badge semantics (BADGE-001): N/A.
|
||||||
| 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
|
## Project Structure
|
||||||
|
|
||||||
@ -60,129 +53,91 @@ ### Documentation (this feature)
|
|||||||
|
|
||||||
```text
|
```text
|
||||||
specs/066-rbac-ui-enforcement-helper/
|
specs/066-rbac-ui-enforcement-helper/
|
||||||
├── spec.md # Feature specification
|
├── plan.md # This file (/speckit.plan command output)
|
||||||
├── plan.md # This file
|
├── research.md # Phase 0 output (/speckit.plan command)
|
||||||
├── research.md # (no separate file needed — inline above)
|
├── data-model.md # Phase 1 output (/speckit.plan command)
|
||||||
├── data-model.md # (no schema changes)
|
├── quickstart.md # Phase 1 output (/speckit.plan command)
|
||||||
├── quickstart.md # Adoption guide
|
├── contracts/ # Phase 1 output (/speckit.plan command)
|
||||||
├── checklists/
|
└── tasks.md # Phase 2 output (/speckit.tasks command - NOT created by /speckit.plan)
|
||||||
│ └── requirements.md # Spec quality checklist
|
|
||||||
└── tasks.md # Phase 2 output (/speckit.tasks)
|
|
||||||
```
|
```
|
||||||
|
|
||||||
### Source Code (repository root)
|
### Source Code (repository root)
|
||||||
|
|
||||||
```text
|
```text
|
||||||
app/
|
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/
|
├── Filament/
|
||||||
│ └── Resources/... # 3–6 exemplar migrations
|
│ └── Resources/
|
||||||
|
│ ├── BackupSetResource.php
|
||||||
|
│ ├── RestoreRunResource.php
|
||||||
|
│ ├── TenantResource.php
|
||||||
|
│ ├── InventoryItemResource.php
|
||||||
|
│ ├── InventorySyncRunResource.php
|
||||||
|
│ ├── EntraGroupSyncRunResource.php
|
||||||
|
│ └── ProviderConnectionResource.php
|
||||||
|
├── Support/
|
||||||
|
│ └── Auth/
|
||||||
|
│ ├── UiEnforcement.php # expected from Feature 066 v1 (verify in implementation worktree)
|
||||||
|
│ └── UiTooltips.php # expected from Feature 066 v1 (verify in implementation worktree)
|
||||||
|
└── Support/Auth/Capabilities.php
|
||||||
|
|
||||||
tests/
|
tests/
|
||||||
├── Feature/
|
├── Feature/
|
||||||
│ └── Rbac/
|
│ ├── Filament/
|
||||||
│ └── UiEnforcementTest.php # Integration tests
|
|
||||||
│ └── Guards/
|
│ └── Guards/
|
||||||
│ └── NoAdHocFilamentAuthPatternsTest.php # CI-failing guard (file-scan)
|
└── Unit/
|
||||||
├── Unit/
|
└── Auth/
|
||||||
│ └── Support/Rbac/
|
|
||||||
│ └── UiEnforcementTest.php # Unit tests
|
|
||||||
|
|
||||||
```
|
```
|
||||||
|
|
||||||
**Structure Decision**: All new code lives in `app/Support/Rbac/` (helper) + tests; no new models/tables required.
|
**Structure Decision**: Single Laravel web application with Filament resources. v2 work focuses on `app/Support/Auth` helper primitives, a small set of Filament resources/pages, and regression tests.
|
||||||
|
|
||||||
## 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
|
## Complexity Tracking
|
||||||
|
|
||||||
> No constitution violations. Complexity is low (helper + tests + 3–6 migrations).
|
No constitution violations required for this feature.
|
||||||
|
|
||||||
| Violation | Why Needed | Simpler Alternative Rejected Because |
|
## Phase 0 — Research (output: `research.md`)
|
||||||
|-----------|------------|-------------------------------------|
|
|
||||||
| — | — | — |
|
See: `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/research.md`
|
||||||
|
|
||||||
|
Goals:
|
||||||
|
- Confirm Filament v5 action/table semantics needed for enforcement (visibility/disabled/tooltips; bulk action lifecycle).
|
||||||
|
- Confirm the helper can compose business visibility deterministically without requiring “read back” of existing closures.
|
||||||
|
- Confirm bulk selection preflight can be implemented without N+1 membership checks (set-based queries where feasible).
|
||||||
|
- Verify the exact locations of the Feature 066 v1 helper + guard + allowlist in the implementation worktree (spec branch may not contain v1 code).
|
||||||
|
|
||||||
|
## Phase 1 — Design & Contracts (outputs: `data-model.md`, `contracts/`, `quickstart.md`)
|
||||||
|
|
||||||
|
See:
|
||||||
|
- `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/data-model.md`
|
||||||
|
- `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/contracts/`
|
||||||
|
- `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/quickstart.md`
|
||||||
|
|
||||||
|
Design focus:
|
||||||
|
- Helper API: `preserveVisibility()` (tenant-scoped only), `andVisibleWhen(...)`, `andHiddenWhen(...)`.
|
||||||
|
- Tenant resolution modes: `tenantFromFilament()` (default), `tenantFromRecord()` (record == tenant), and `tenantFrom(callable)` for custom mapping.
|
||||||
|
- Bulk preflight: deterministic authorization-only all-or-nothing default; business eligibility handled separately with clear user feedback.
|
||||||
|
- Test strategy: per-surface Filament integration tests for hidden/disabled/tooltips and no-execution; DB-only rendering enforced via `Http::preventStrayRequests()`.
|
||||||
|
|
||||||
|
## Phase 1 — Agent Context Update
|
||||||
|
|
||||||
|
After Phase 1 artifacts are generated, update Codex context from the plan:
|
||||||
|
|
||||||
|
- `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/.specify/scripts/bash/update-agent-context.sh codex`
|
||||||
|
|
||||||
|
## Phase 2 — Implementation Outline (tasks created in `/speckit.tasks`)
|
||||||
|
|
||||||
|
- Extend `UiEnforcement` helper primitives (mixed visibility composition + tenant resolvers + bulk preflight API).
|
||||||
|
- Migrate Tier 1 surfaces:
|
||||||
|
- BackupSet (resource + relation managers) for mixed visibility.
|
||||||
|
- RestoreRun (resource + actions) for mixed visibility.
|
||||||
|
- TenantResource row actions + bulk actions for record-scoped tenant enforcement and authorization-only bulk preflight.
|
||||||
|
- Migrate Tier 2 allowlist “easy wins” (Inventory + Entra group runs + ProviderConnection mixed visibility where present).
|
||||||
|
- Shrink the CI-failing allowlist: remove entries for every migrated file in the same batch.
|
||||||
|
- Add integration tests per migrated surface:
|
||||||
|
- non-member: hidden + cannot execute (deny-as-not-found / 404 where reachable),
|
||||||
|
- member without capability: visible but disabled + standard tooltip + cannot execute,
|
||||||
|
- member with capability: enabled and executes; destructive actions require confirmation.
|
||||||
|
- Run quality gates per batch: `./vendor/bin/sail bin pint --dirty` + targeted Pest suites + guard tests.
|
||||||
|
|
||||||
|
## Constitution Check (Post-Design)
|
||||||
|
|
||||||
|
Re-check result: PASS. Design artifacts keep enforcement DB-only, tenant-plane only, capabilities-first, and preserve deny-as-not-found behavior for non-members (including record-scoped tenant surfaces).
|
||||||
|
|||||||
@ -1,262 +1,63 @@
|
|||||||
# Quickstart: UiEnforcement Helper
|
# Quickstart: RBAC UI Enforcement Helper v2
|
||||||
|
|
||||||
> Adoption guide for developers adding RBAC enforcement to Filament actions.
|
**Branch**: `066-rbac-ui-enforcement-helper-v2`
|
||||||
|
**Date**: 2026-01-30
|
||||||
|
**Spec**: `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/spec.md`
|
||||||
|
|
||||||
## TL;DR
|
## Local setup
|
||||||
|
|
||||||
Replace ad-hoc `->visible(fn ...)` / `->disabled(fn ...)` closures with `UiEnforcement`.
|
1. Start the app stack:
|
||||||
|
- `./vendor/bin/sail up -d`
|
||||||
|
2. Install dependencies (if needed):
|
||||||
|
- `./vendor/bin/sail composer install`
|
||||||
|
|
||||||
```php
|
## Recipes (once implemented)
|
||||||
// ❌ Before (ad-hoc)
|
|
||||||
Action::make('sync')
|
|
||||||
->visible(fn () => auth()->user()->can('provider:manage', Filament::getTenant()))
|
|
||||||
->disabled(fn () => ! auth()->user()->can('provider:manage', Filament::getTenant()))
|
|
||||||
->action(function () {
|
|
||||||
// no server-side guard
|
|
||||||
});
|
|
||||||
|
|
||||||
// ✅ After (UiEnforcement)
|
> Scope: Tenant plane only (`/admin/t/{tenant}`). Platform plane (`/system`) is out of scope.
|
||||||
UiEnforcement::forAction(
|
|
||||||
Action::make('sync')
|
|
||||||
->action(fn () => $this->sync())
|
|
||||||
)
|
|
||||||
->requireCapability(Capabilities::PROVIDER_MANAGE)
|
|
||||||
->apply();
|
|
||||||
```
|
|
||||||
|
|
||||||
## When to Use
|
### Tenant-scoped action (default tenant from Filament)
|
||||||
|
|
||||||
| Scenario | Use UiEnforcement? |
|
- Use `UiEnforcement` with the required `Capabilities::*` constant.
|
||||||
|----------|-------------------|
|
- Non-members should never see the action (routing already denies them).
|
||||||
| Tenant-scoped action (header, table, bulk) | ✅ Yes |
|
- Members without capability see a disabled action + standard tooltip.
|
||||||
| Platform-scoped action (/system panel) | ❌ No (use Gate directly) |
|
|
||||||
| Read-only navigation link | ❌ No (use `->visible()` for nav items) |
|
|
||||||
| Destructive action (delete, detach, restore) | ✅ Yes, with `->destructive()` |
|
|
||||||
|
|
||||||
## API Reference
|
### Mixed visibility (business visibility + RBAC)
|
||||||
|
|
||||||
### Header / Page Actions
|
- If the surface already has business visibility rules, use composition:
|
||||||
|
- **Tenant-scoped**: you may use `preserveVisibility()` to keep existing visibility closures unchanged.
|
||||||
|
- **Record-scoped / cross-tenant lists**: do not use `preserveVisibility()`; instead use `andVisibleWhen(...)` / `andHiddenWhen(...)` so non-members cannot discover actions.
|
||||||
|
|
||||||
```php
|
### Record-scoped tenant (record == tenant)
|
||||||
use App\Support\Rbac\UiEnforcement;
|
|
||||||
use App\Support\Auth\Capabilities;
|
|
||||||
|
|
||||||
protected function getHeaderActions(): array
|
- Configure enforcement with `tenantFromRecord()` so membership/capability checks are evaluated per row/record tenant.
|
||||||
{
|
- Non-members must be deny-as-not-found (404) on direct access/execution attempts.
|
||||||
return [
|
|
||||||
UiEnforcement::forAction(
|
|
||||||
Action::make('createBackup')
|
|
||||||
->action(fn () => $this->createBackup())
|
|
||||||
)
|
|
||||||
->requireCapability(Capabilities::BACKUP_CREATE)
|
|
||||||
->apply(),
|
|
||||||
|
|
||||||
UiEnforcement::forAction(
|
### Bulk actions (authorization-only all-or-nothing)
|
||||||
Action::make('deleteAllBackups')
|
|
||||||
->action(fn () => $this->deleteAll())
|
|
||||||
)
|
|
||||||
->requireCapability(Capabilities::BACKUP_MANAGE)
|
|
||||||
->destructive()
|
|
||||||
->apply(),
|
|
||||||
];
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
### Table Row Actions
|
- Use bulk preflight to disable the action if any selected record is unauthorized.
|
||||||
|
- Keep business eligibility separate (e.g., skip inactive/archived records with deterministic feedback).
|
||||||
|
|
||||||
```php
|
## Notes on Filament execution semantics
|
||||||
public static function table(Table $table): Table
|
|
||||||
{
|
|
||||||
return $table
|
|
||||||
->columns([...])
|
|
||||||
->actions([
|
|
||||||
UiEnforcement::forTableAction(
|
|
||||||
Action::make('restore')
|
|
||||||
->action(fn (Policy $record) => $record->restore()),
|
|
||||||
fn () => $this->getRecord() // record accessor
|
|
||||||
)
|
|
||||||
->requireCapability(Capabilities::RESTORE_EXECUTE)
|
|
||||||
->destructive()
|
|
||||||
->apply(),
|
|
||||||
]);
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
### Bulk Actions
|
- Hidden actions do not execute (silent no-op). This is acceptable for the non-member contract.
|
||||||
|
- Disabled actions do not execute (silent no-op). This is acceptable for member-without-capability.
|
||||||
|
- Server-side authorization remains required (UI is not security). Where reachable:
|
||||||
|
- Non-member: deny-as-not-found (404)
|
||||||
|
- Member without capability: 403
|
||||||
|
|
||||||
```php
|
## Manual QA checklist (once implemented)
|
||||||
->bulkActions([
|
|
||||||
UiEnforcement::forBulkAction(
|
|
||||||
BulkAction::make('deleteSelected')
|
|
||||||
->action(fn (Collection $records) => $records->each->delete())
|
|
||||||
)
|
|
||||||
->requireCapability(Capabilities::TENANT_DELETE)
|
|
||||||
->destructive()
|
|
||||||
->apply(),
|
|
||||||
])
|
|
||||||
```
|
|
||||||
|
|
||||||
## Behavior Matrix
|
- Log in as a tenant member without the capability:
|
||||||
|
- Action visible but disabled, with tooltip “Insufficient permission — ask a tenant Owner.”
|
||||||
|
- Clicking does not execute.
|
||||||
|
- Remove membership:
|
||||||
|
- Action hidden; direct access returns 404.
|
||||||
|
- Bulk selection with mixed authorization:
|
||||||
|
- Bulk action is disabled; no partial execution.
|
||||||
|
|
||||||
| User Status | UI State | Server Response |
|
## Test run (once implemented)
|
||||||
|-------------|----------|-----------------|
|
|
||||||
| Non-member | Hidden | Blocked (no execution, 200) |
|
|
||||||
| Member, no capability | Visible, disabled + tooltip | Blocked (no execution, 200) |
|
|
||||||
| Member, has capability | Enabled | Executes |
|
|
||||||
| Member, destructive action | Confirmation modal | Executes after confirm |
|
|
||||||
|
|
||||||
> **Note on 404/403 Responses:** In Filament v5, hidden actions are automatically
|
- `./vendor/bin/sail bin pint --dirty`
|
||||||
> treated as disabled, so execution is blocked silently (returns 200 with no side
|
- `./vendor/bin/sail artisan test tests/Feature/Guards --compact`
|
||||||
> effects). True 404 enforcement happens at the page/routing level via tenant
|
- `./vendor/bin/sail artisan test tests/Feature/Filament --compact`
|
||||||
> middleware. The UiEnforcement helper includes defense-in-depth server-side
|
- `./vendor/bin/sail artisan test tests/Feature/Rbac --compact`
|
||||||
> guards that abort(404/403) if somehow reached, but the primary protection is
|
|
||||||
> Filament's isHidden/isDisabled chain.
|
|
||||||
|
|
||||||
## Tooltip Customization
|
|
||||||
|
|
||||||
Default tooltip: *"You don't have permission to do this. Ask a tenant admin."*
|
|
||||||
|
|
||||||
Override per-action:
|
|
||||||
|
|
||||||
```php
|
|
||||||
UiEnforcement::forAction($action)
|
|
||||||
->requireCapability(Capabilities::PROVIDER_MANAGE)
|
|
||||||
->tooltip('Contact your organization owner to enable this feature.')
|
|
||||||
->apply();
|
|
||||||
```
|
|
||||||
|
|
||||||
## Testing
|
|
||||||
|
|
||||||
Test both UI state and execution blocking:
|
|
||||||
|
|
||||||
```php
|
|
||||||
it('hides sync action for non-members', function () {
|
|
||||||
$user = User::factory()->create();
|
|
||||||
$tenant = Tenant::factory()->create();
|
|
||||||
// user is NOT a member
|
|
||||||
|
|
||||||
actingAs($user);
|
|
||||||
Filament::setTenant($tenant, true);
|
|
||||||
|
|
||||||
Livewire::test(ListPolicies::class)
|
|
||||||
->assertActionHidden('sync');
|
|
||||||
});
|
|
||||||
|
|
||||||
it('blocks action execution for non-members (no side effects)', function () {
|
|
||||||
$user = User::factory()->create();
|
|
||||||
$tenant = Tenant::factory()->create();
|
|
||||||
Queue::fake();
|
|
||||||
|
|
||||||
actingAs($user);
|
|
||||||
Filament::setTenant($tenant, true);
|
|
||||||
|
|
||||||
// Hidden actions are blocked silently (200 but no execution)
|
|
||||||
Livewire::test(ListPolicies::class)
|
|
||||||
->mountAction('sync')
|
|
||||||
->callMountedAction()
|
|
||||||
->assertSuccessful();
|
|
||||||
|
|
||||||
// Verify no side effects occurred
|
|
||||||
Queue::assertNothingPushed();
|
|
||||||
});
|
|
||||||
|
|
||||||
it('disables sync action for readonly members', function () {
|
|
||||||
[$user, $tenant] = createUserWithTenant(role: 'readonly');
|
|
||||||
|
|
||||||
actingAs($user);
|
|
||||||
Filament::setTenant($tenant, true);
|
|
||||||
|
|
||||||
Livewire::test(ListPolicies::class)
|
|
||||||
->assertActionVisible('sync')
|
|
||||||
->assertActionDisabled('sync');
|
|
||||||
});
|
|
||||||
|
|
||||||
it('shows disabled tooltip for readonly members', function () {
|
|
||||||
[$user, $tenant] = createUserWithTenant(role: 'readonly');
|
|
||||||
|
|
||||||
actingAs($user);
|
|
||||||
Filament::setTenant($tenant, true);
|
|
||||||
|
|
||||||
Livewire::test(ListPolicies::class)
|
|
||||||
->assertActionHasTooltip('sync', UiTooltips::INSUFFICIENT_PERMISSION);
|
|
||||||
});
|
|
||||||
```
|
|
||||||
|
|
||||||
## Migration Checklist
|
|
||||||
|
|
||||||
When migrating an existing action:
|
|
||||||
|
|
||||||
- [ ] Remove `->visible(fn ...)` closure (UiEnforcement handles this)
|
|
||||||
- [ ] Remove `->disabled(fn ...)` closure (UiEnforcement handles this)
|
|
||||||
- [ ] Remove inline `Gate::check()` / `abort_unless()` from action handler
|
|
||||||
- [ ] Wrap action with `UiEnforcement::forAction(...)->requireCapability(...)->apply()`
|
|
||||||
- [ ] Add `->destructive()` if action modifies/deletes data
|
|
||||||
- [ ] Add test for non-member (hidden + no execution)
|
|
||||||
- [ ] Add test for member without capability (disabled + tooltip)
|
|
||||||
- [ ] Add test for member with capability (enabled + executes)
|
|
||||||
|
|
||||||
### Real Example: ListPolicies Sync Action
|
|
||||||
|
|
||||||
```php
|
|
||||||
// Before (ad-hoc)
|
|
||||||
Action::make('sync')
|
|
||||||
->icon('heroicon-o-arrow-path')
|
|
||||||
->action(function (): void {
|
|
||||||
$tenant = Tenant::current();
|
|
||||||
if (! $tenant) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
// ... sync logic
|
|
||||||
})
|
|
||||||
->disabled(fn (): bool => ! Gate::allows(Capabilities::TENANT_SYNC, Tenant::current()))
|
|
||||||
|
|
||||||
// After (UiEnforcement)
|
|
||||||
UiEnforcement::forAction(
|
|
||||||
Action::make('sync')
|
|
||||||
->icon('heroicon-o-arrow-path')
|
|
||||||
->action(function (): void {
|
|
||||||
$tenant = Tenant::current();
|
|
||||||
if (! $tenant) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
// ... sync logic
|
|
||||||
})
|
|
||||||
)
|
|
||||||
->requireCapability(Capabilities::TENANT_SYNC)
|
|
||||||
->destructive()
|
|
||||||
->apply()
|
|
||||||
```
|
|
||||||
|
|
||||||
## Common Mistakes
|
|
||||||
|
|
||||||
### ❌ Forgetting `->apply()`
|
|
||||||
|
|
||||||
```php
|
|
||||||
// This does nothing!
|
|
||||||
UiEnforcement::forAction($action)
|
|
||||||
->requireCapability(Capabilities::PROVIDER_MANAGE);
|
|
||||||
// missing ->apply()
|
|
||||||
```
|
|
||||||
|
|
||||||
### ❌ Using with non-tenant panels
|
|
||||||
|
|
||||||
```php
|
|
||||||
// UiEnforcement is tenant-scoped only!
|
|
||||||
// For /system panel, use Gate::check() directly
|
|
||||||
```
|
|
||||||
|
|
||||||
### ❌ Mixing old and new patterns
|
|
||||||
|
|
||||||
```php
|
|
||||||
// Don't mix - pick one
|
|
||||||
UiEnforcement::forAction(
|
|
||||||
Action::make('sync')
|
|
||||||
->visible(fn () => someOtherCheck()) // ❌ conflict
|
|
||||||
)
|
|
||||||
->requireCapability(Capabilities::PROVIDER_MANAGE)
|
|
||||||
->apply();
|
|
||||||
```
|
|
||||||
|
|
||||||
## Questions?
|
|
||||||
|
|
||||||
See [spec.md](./spec.md) for full requirements or [plan.md](./plan.md) for implementation details.
|
|
||||||
|
|||||||
89
specs/066-rbac-ui-enforcement-helper/research.md
Normal file
89
specs/066-rbac-ui-enforcement-helper/research.md
Normal file
@ -0,0 +1,89 @@
|
|||||||
|
# Research: RBAC UI Enforcement Helper v2 (Suite-wide, Mixed Visibility, Record-Scoped)
|
||||||
|
|
||||||
|
**Branch**: `066-rbac-ui-enforcement-helper-v2`
|
||||||
|
**Date**: 2026-01-30
|
||||||
|
**Spec**: `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/spec.md`
|
||||||
|
**Plan**: `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/plan.md`
|
||||||
|
|
||||||
|
## Decisions
|
||||||
|
|
||||||
|
### D-001 — Mixed visibility needs explicit composition primitives
|
||||||
|
|
||||||
|
**Decision**: `UiEnforcement` supports mixed visibility via explicit composition methods:
|
||||||
|
- `preserveVisibility()` is allowed only on tenant-scoped surfaces where routing already denies non-members (404).
|
||||||
|
- Record-scoped / cross-tenant lists must use `andVisibleWhen(...)` / `andHiddenWhen(...)` (or default RBAC visibility), so non-members cannot discover actions.
|
||||||
|
|
||||||
|
**Rationale**: Filament does not provide a safe way to introspect/merge existing `->visible()` / `->hidden()` closures. Explicit composition avoids brittle “read back and wrap” behavior and preserves the non-member “cannot discover” contract.
|
||||||
|
|
||||||
|
**Alternatives considered**:
|
||||||
|
- Allow `preserveVisibility()` everywhere: rejected because it can leak action discoverability on record-scoped lists.
|
||||||
|
- Implicitly merge existing closures (helper tries to wrap previous `visible/hidden`): rejected as fragile and hard to review/test.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### D-002 — Tenant resolution must be explicit and per-surface
|
||||||
|
|
||||||
|
**Decision**: `UiEnforcement` supports tenant resolution modes:
|
||||||
|
- `tenantFromFilament()` (default) for tenant-scoped pages/actions.
|
||||||
|
- `tenantFromRecord()` for record-scoped resources where record == tenant.
|
||||||
|
- `tenantFrom(callable)` for record → tenant mapping.
|
||||||
|
|
||||||
|
**Rationale**: v2 targets record-scoped surfaces where `Filament::getTenant()` is intentionally null; the helper must not hard-depend on Filament tenancy context.
|
||||||
|
|
||||||
|
**Alternatives considered**:
|
||||||
|
- Require `Filament::getTenant()` always: rejected because it blocks record-scoped resources by design.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### D-003 — Bulk preflight is authorization-only by default
|
||||||
|
|
||||||
|
**Decision**: Default bulk “all-or-nothing” preflight applies to authorization only:
|
||||||
|
- If any selected record is unauthorized, the bulk action is disabled for members and cannot execute.
|
||||||
|
- Business eligibility (e.g., inactive/archived) remains separate and may be skipped with deterministic feedback.
|
||||||
|
|
||||||
|
**Rationale**: Mixing “eligibility” into authorization preflight tends to create confusing UX for operators and makes migrations riskier (unexpected disablement). Keeping them separate preserves predictable security semantics and reduces incidental coupling.
|
||||||
|
|
||||||
|
**Alternatives considered**:
|
||||||
|
- Disable bulk actions when any record is ineligible: rejected as overly strict and likely to regress existing workflows.
|
||||||
|
- Partial execution for authorized subset: rejected (v2 contract requires all-or-nothing for authorization).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### D-004 — Record-scoped non-member denial is 404
|
||||||
|
|
||||||
|
**Decision**: For record-scoped tenant pages/actions, non-members are denied as not found (404) on direct access/execution attempts.
|
||||||
|
|
||||||
|
**Rationale**: Prevents tenant/record enumeration and aligns with the constitution’s deny-as-not-found rule for non-members.
|
||||||
|
|
||||||
|
**Alternatives considered**:
|
||||||
|
- Return 403 for non-members: rejected as it can leak record existence.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### D-005 — Standard disabled tooltip copy
|
||||||
|
|
||||||
|
**Decision**: Default `UiTooltips` message for member-without-capability is:
|
||||||
|
> “Insufficient permission — ask a tenant Owner.”
|
||||||
|
|
||||||
|
**Rationale**: Clear, non-leaky, and gives the user the correct next step without exposing capability names or internal policy details.
|
||||||
|
|
||||||
|
**Alternatives considered**:
|
||||||
|
- “Insufficient permission.”: rejected as too vague for operators.
|
||||||
|
- Include capability identifier in tooltip: rejected as noisy and may leak internal policy shape; can be added selectively where appropriate.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### D-006 — Tests are the contract (per surface)
|
||||||
|
|
||||||
|
**Decision**: Each migrated surface gets at least one focused integration test covering:
|
||||||
|
- non-member: hidden + cannot execute (deny-as-not-found/404 where reachable),
|
||||||
|
- member without capability: visible but disabled + standard tooltip + cannot execute,
|
||||||
|
- member with capability: enabled executes (and destructive actions require confirmation).
|
||||||
|
|
||||||
|
Additionally, DB-only render is enforced for migrated pages/actions using `Http::preventStrayRequests()`.
|
||||||
|
|
||||||
|
**Rationale**: The helper exists to standardize behavior; tests must assert the user-facing contract and prevent drift.
|
||||||
|
|
||||||
|
**Alternatives considered**:
|
||||||
|
- Rely on unit tests for helper only: rejected because Filament execution semantics and closures are integration-sensitive.
|
||||||
|
|
||||||
@ -1,163 +1,288 @@
|
|||||||
# Feature Specification: RBAC UI Enforcement Helper v1
|
# Feature Specification: RBAC UI Enforcement Helper v2 (Suite-wide, Mixed Visibility, Record-Scoped)
|
||||||
|
|
||||||
**Feature Branch**: `066-rbac-ui-enforcement-helper`
|
**Feature Branch**: `066-rbac-ui-enforcement-helper-v2`
|
||||||
**Created**: 2026-01-28
|
**Created**: 2026-01-30
|
||||||
**Status**: Draft
|
**Status**: Draft
|
||||||
**Input**: Provide a suite-wide, consistent way to enforce tenant RBAC for admin UI actions (buttons/actions in lists, records, and bulk actions) without copy/paste authorization logic.
|
**Input**: Make the RBAC UI enforcement helper suite-ready by supporting mixed visibility, record-scoped tenancy, and deterministic bulk preflight authorization (follow-up to Feature 066 v1).
|
||||||
|
|
||||||
## Clarifications
|
## Clarifications
|
||||||
|
|
||||||
### Session 2026-01-28
|
### Session 2026-01-28 (carried forward from Feature 066 v1)
|
||||||
|
|
||||||
- Q: For Bulk Actions with mixed-permission records (some authorized, some not), what should the default behavior be? → A: All-or-nothing (if any selected record would be unauthorized, the bulk action is disabled for members and execution fails with 403 for members / 404 for non-members).
|
- Q: For Bulk Actions with mixed-permission records (some authorized, some not), what should the default behavior be? → A: All-or-nothing (if any selected record would be unauthorized, the bulk action is disabled for members; if execution is forced, it must fail with 403 for members / 404 for non-members).
|
||||||
- Q: Should the helper render actions at all for non-members (in case a tenant page is reachable via misrouting), or always hide them? → A: Hide for non-members in UI, but still enforce 404 server-side for any execution attempt.
|
- Q: Should the helper render actions at all for non-members (in case a tenant page is reachable via misrouting), or always hide them? → A: Hide for non-members in UI, but still enforce 404 server-side for any execution attempt.
|
||||||
- Q: How strict should the “no ad-hoc authorization patterns in app/Filament/**” guard be in v1? → A: CI-failing (new ad-hoc patterns fail tests/CI).
|
- Q: How strict should the “no ad-hoc authorization patterns in app/Filament/**” guard be in v1? → A: CI-failing (new ad-hoc patterns fail tests/CI).
|
||||||
|
|
||||||
## User Scenarios & Testing *(mandatory)*
|
### Session 2026-01-30
|
||||||
|
|
||||||
<!--
|
- Q: For mixed visibility, should `UiEnforcement::preserveVisibility()` be restricted to tenant-scoped surfaces (tenant routing already denies non-members), and forbidden for record-scoped / cross-tenant lists? → A: Yes — tenant-scoped only; record-scoped must use `andVisibleWhen(...)` / `andHiddenWhen(...)` (or default RBAC visibility).
|
||||||
IMPORTANT: User stories should be PRIORITIZED as user journeys ordered by importance.
|
- Q: For bulk actions, should v2’s default “all-or-nothing” preflight apply to authorization only, or to authorization + business eligibility? → A: Authorization only — disable if any selected record is unauthorized; business-ineligible records may be skipped with a clear notification.
|
||||||
Each user story/journey must be INDEPENDENTLY TESTABLE - meaning if you implement just ONE of them,
|
- Q: For record-scoped tenant pages/actions (e.g., TenantResource row actions / edit/view), should a non-member be denied as 404 or 403 on direct access/execution? → A: 404 (deny-as-not-found).
|
||||||
you should still have a viable MVP (Minimum Viable Product) that delivers value.
|
- Q: What should the default `UiTooltips` message be for member without capability (disabled action tooltip)? → A: “Insufficient permission — ask a tenant Owner.”
|
||||||
|
- Q: Which authorization plane(s) are in scope for `UiEnforcement` v2? → A: Tenant plane only (tenant users/memberships with `Capabilities::*`); platform plane (`/system`) is out of scope.
|
||||||
|
|
||||||
Assign priorities (P1, P2, P3, etc.) to each story, where P1 is the most critical.
|
## Problem Statement
|
||||||
Think of each story as a standalone slice of functionality that can be:
|
|
||||||
- Developed independently
|
|
||||||
- Tested independently
|
|
||||||
- Deployed independently
|
|
||||||
- Demonstrated to users independently
|
|
||||||
-->
|
|
||||||
|
|
||||||
### User Story 1 - Tenant member sees consistent disabled UX (Priority: P1)
|
Feature 066 v1 introduced a canonical RBAC UI enforcement helper (`UiEnforcement`) and a CI-failing file-scan guard (`NoAdHocFilamentAuthPatternsTest`) to prevent drift. This removed many ad-hoc patterns, but several high-value Filament surfaces remain difficult to migrate due to:
|
||||||
|
|
||||||
As a tenant member, I can clearly see which actions exist, and when I lack permission the action is visible but disabled with an explanatory tooltip.
|
1. Mixed visibility (business visibility + RBAC visibility)
|
||||||
|
2. Record-scoped tenancy (resources that are not tenant-scoped; the record *is* the tenant)
|
||||||
|
3. Bulk actions needing predictable all-or-nothing semantics without N+1 or partial execution surprises
|
||||||
|
4. Filament execution semantics (hidden/disabled blocks execution as a no-op; 404/403 cannot always be asserted at action execution time)
|
||||||
|
|
||||||
**Why this priority**: Prevents confusion and reduces support load while keeping the UI predictable for members.
|
v2 makes the helper suite-ready by adding explicit primitives for these cases and migrating remaining high-risk surfaces.
|
||||||
|
|
||||||
**Independent Test**: Can be tested by visiting a tenant-scoped admin page as a member with insufficient permissions and verifying the action is disabled, shows the standard tooltip, and cannot be executed.
|
## Goals
|
||||||
|
|
||||||
**Acceptance Scenarios**:
|
### G1 — Suite-wide RBAC UX consistency
|
||||||
|
|
||||||
1. **Given** a tenant member without the required capability, **When** they view an action on a tenant-scoped page, **Then** the action is visible but disabled and shows the standard “insufficient permission” tooltip.
|
A tenant member sees a consistent UI pattern everywhere:
|
||||||
2. **Given** a tenant member without the required capability, **When** they attempt to execute the action (including direct invocation, bypassing the UI), **Then** the server rejects with 403.
|
|
||||||
|
|
||||||
---
|
- **Non-member**: cannot discover or act (deny-as-not-found for pages; actions hidden + cannot execute)
|
||||||
|
- **Member without capability**: action visible but disabled with a standard tooltip; cannot execute
|
||||||
|
- **Member with capability**: action enabled; executes normally
|
||||||
|
|
||||||
### User Story 2 - Non-members cannot infer tenant resources (Priority: P2)
|
### G2 — Eliminate remaining ad-hoc RBAC in Filament
|
||||||
|
|
||||||
As a non-member of a tenant, I cannot discover tenant-scoped resources or actions; the system responds as “not found”.
|
Remove `Gate::...`, `abort_*`, policy ability string literals, and custom RBAC patterns from remaining targeted Filament surfaces, replacing them with `UiEnforcement` and canonical `Capabilities::*`.
|
||||||
|
|
||||||
**Why this priority**: Prevents tenant enumeration and cross-tenant information leakage.
|
### G3 — Support mixed visibility and record-scoped tenancy
|
||||||
|
|
||||||
**Independent Test**: Can be tested by attempting to access tenant-scoped pages/actions as a user without membership and verifying 404 behavior.
|
Provide first-class APIs so teams can migrate without special casing per resource.
|
||||||
|
|
||||||
**Acceptance Scenarios**:
|
### G4 — Keep enforcement testable and stable
|
||||||
|
|
||||||
1. **Given** a user who is not entitled to the tenant scope, **When** they attempt any tenant-scoped page or action, **Then** the system responds as 404 (deny-as-not-found).
|
Guard remains stable (file scan), allowlist shrinks, and we add targeted integration tests for each migrated surface.
|
||||||
|
|
||||||
---
|
## Non-Goals
|
||||||
|
|
||||||
### User Story 3 - Maintainers add actions safely by default (Priority: P3)
|
- Redesign pages or change business flows beyond RBAC visibility/disable/confirm behavior.
|
||||||
|
- Rebuild authentication (063/064) or expand RBAC model semantics (065) beyond capabilities usage.
|
||||||
|
- Convert every remaining Filament file in one shot; v2 targets a defined set and continues shrinking allowlist incrementally.
|
||||||
|
|
||||||
As a maintainer, I can add new tenant-scoped actions using one standard pattern, and regression guards prevent introducing ad-hoc authorization logic.
|
## Terms
|
||||||
|
|
||||||
**Why this priority**: Reduces RBAC regressions as the app grows and makes reviews easier.
|
- **Member**: user has `canAccessTenant($tenant)` / membership.
|
||||||
|
- **Capability**: canonical string key defined in `app/Support/Auth/Capabilities.php` (single source of truth).
|
||||||
|
- **Tenant-scoped page**: Filament tenant context is set (`Filament::getTenant()` non-null).
|
||||||
|
- **Record-scoped tenant**: record itself defines tenant context (e.g., tenant list rows where each record is a Tenant).
|
||||||
|
- **Business visibility**: visibility logic unrelated to RBAC (trashed state, filter state, record status, etc.).
|
||||||
|
- **Authorization plane**: v2 applies to the tenant plane (`/admin/t/{tenant}`) only; the platform plane (`/system`) is out of scope.
|
||||||
|
|
||||||
**Independent Test**: Can be tested by introducing a sample ad-hoc authorization pattern and confirming automated checks/tests flag it.
|
## RBAC-UX Contract (v2)
|
||||||
|
|
||||||
**Acceptance Scenarios**:
|
This feature follows the RBAC constitution rules and clarifies Filament execution reality.
|
||||||
|
|
||||||
1. **Given** a maintainer adds a new tenant-scoped action, **When** they use the central enforcement helper, **Then** member/non-member semantics and tooltip behavior match the standard without additional per-page customization.
|
### 5.1 Non-member
|
||||||
2. **Given** a maintainer introduces a new ad-hoc authorization mapping in tenant-scoped admin UI code, **When** automated checks run, **Then** the change is flagged to prevent drift.
|
|
||||||
|
|
||||||
---
|
- **Pages/routes**: MUST be deny-as-not-found (404) using route/middleware scoping (already handled by tenant routing + membership guard).
|
||||||
|
- **Actions**: MUST be hidden and cannot execute.
|
||||||
|
|
||||||
[Add more user stories as needed, each with an assigned priority]
|
**Note**: Filament treats hidden as non-executable (silent no-op). This is acceptable and is the enforced behavior.
|
||||||
|
|
||||||
### Edge Cases
|
### 5.2 Member without capability
|
||||||
|
|
||||||
<!--
|
- Action is visible but disabled.
|
||||||
ACTION REQUIRED: The content in this section represents placeholders.
|
- Tooltip MUST be from `UiTooltips` (or a standard override).
|
||||||
Fill them out with the right edge cases.
|
- Execution is blocked for normal UI interaction (Filament disabled no-op).
|
||||||
-->
|
- Server-side authorization remains required for any mutation action (Policy/Gate). Where an execution attempt is reachable, it MUST be forbidden (403) for members without the capability.
|
||||||
|
- Default tooltip text: “Insufficient permission — ask a tenant Owner.”
|
||||||
|
|
||||||
- Membership is revoked while the user has the page open (execution must still enforce 404 semantics).
|
### 5.3 Member with capability
|
||||||
- Capability changes mid-session (UI may be stale; server enforcement remains correct).
|
|
||||||
- Bulk actions with mixed-permission records: all-or-nothing (disable + tooltip for members; 403 on execution for members; 404 semantics for non-members).
|
|
||||||
- Target record is deleted/archived between render and execution (no information leakage in errors).
|
|
||||||
|
|
||||||
## Requirements *(mandatory)*
|
- Enabled and executes.
|
||||||
|
- Any destructive action MUST require confirmation (`->requiresConfirmation()`).
|
||||||
|
|
||||||
**Constitution alignment (required):** If this feature introduces any Microsoft Graph calls, any write/change behavior,
|
### 5.4 Bulk actions (default)
|
||||||
or any long-running/queued/scheduled work, the spec MUST describe contract registry updates, safety gates
|
|
||||||
(preview/confirmation/audit), tenant isolation, run observability (`OperationRun` type/identity/visibility), and tests.
|
|
||||||
If security-relevant DB-only actions intentionally skip `OperationRun`, the spec MUST describe `AuditLog` entries.
|
|
||||||
|
|
||||||
**Constitution alignment (RBAC-UX):** This feature defines a default pattern for tenant-plane admin actions. The implementation MUST:
|
All-or-nothing (authorization): if any selected record fails authorization, the bulk action is disabled (members) and cannot execute; no partial execution.
|
||||||
- enforce membership as an isolation boundary (non-member / not entitled → 404 deny-as-not-found),
|
Business eligibility (e.g., inactive/archived) is handled separately: ineligible records may be skipped with deterministic user feedback.
|
||||||
- enforce capability denials as 403 (after membership is established),
|
|
||||||
- keep actions visible-but-disabled with a standard tooltip for members lacking capability (except allowed sensitive exceptions),
|
|
||||||
- enforce authorization server-side for every mutation/operation-start/credential change,
|
|
||||||
- use the canonical capability registry (no raw capability string literals),
|
|
||||||
- ensure destructive-like actions require confirmation,
|
|
||||||
- ship regression tests and a guard against new ad-hoc authorization patterns.
|
|
||||||
|
|
||||||
**Constitution alignment (OPS-EX-AUTH-001):** OIDC/SAML login handshakes may perform synchronous outbound HTTP (e.g., token exchange)
|
## Requirements
|
||||||
on `/auth/*` endpoints without an `OperationRun`. This MUST NOT be used for Monitoring/Operations pages.
|
|
||||||
|
|
||||||
**Constitution alignment (BADGE-001):** If this feature changes status-like badges (status/outcome/severity/risk/availability/boolean),
|
|
||||||
the spec MUST describe how badge semantics stay centralized (no ad-hoc mappings) and which tests cover any new/changed values.
|
|
||||||
|
|
||||||
<!--
|
|
||||||
ACTION REQUIRED: The content in this section represents placeholders.
|
|
||||||
Fill them out with the right functional requirements.
|
|
||||||
-->
|
|
||||||
|
|
||||||
### Functional Requirements
|
### Functional Requirements
|
||||||
|
|
||||||
- **FR-001**: The system MUST provide a single, centrally maintained enforcement mechanism that can be applied to tenant-scoped admin actions (including header actions, record actions, and bulk actions).
|
- **FR-001 Mixed Visibility Support**: `UiEnforcement` MUST support combining RBAC visibility with existing business visibility patterns without overriding them.
|
||||||
- **FR-002**: For tenant-scoped actions, the system MUST enforce membership as deny-as-not-found: users not entitled to the tenant scope MUST receive 404 semantics for action execution.
|
- A resource can keep business `->visible()` conditions (trashed/state/filter).
|
||||||
- **FR-002a**: For users not entitled to the tenant scope, the UI SHOULD NOT render tenant-scoped actions (default: hidden), while server-side execution MUST still enforce 404 semantics.
|
- RBAC adds membership/capability constraints without losing business visibility behavior.
|
||||||
- **FR-003**: For tenant members, the system MUST enforce capability denial as 403 when executing an action without permission.
|
|
||||||
- **FR-004**: For tenant members lacking capability, the UI MUST render actions as visible-but-disabled and MUST show a standard tooltip explaining the missing permission.
|
|
||||||
- **FR-005**: The enforcement mechanism MUST also enforce the same rules server-side (UI state is never sufficient).
|
|
||||||
- **FR-006**: The enforcement mechanism MUST be capability-first and MUST reference capabilities only via the canonical capability registry (no ad-hoc string literals).
|
|
||||||
- **FR-007**: The enforcement mechanism MUST provide a standard confirmation behavior for destructive-like actions, including a clear warning message.
|
|
||||||
- **FR-008**: The system MUST provide standardized, non-leaky error and tooltip messages:
|
|
||||||
- 404 semantics for non-members without hints.
|
|
||||||
- 403 responses for insufficient capability without object details.
|
|
||||||
- **FR-009**: v1 MUST include limited adoption by migrating 3–6 exemplar action surfaces to the new pattern to prove the approach.
|
|
||||||
- **FR-010**: v1 MUST include regression tests that cover: non-member → 404, member without capability → disabled UI + 403 on execution, member with capability → allowed.
|
|
||||||
- **FR-010a**: For bulk actions with mixed-permission records, the default behavior MUST be all-or-nothing (members see disabled + tooltip; execution denies with 403; non-members receive 404 semantics).
|
|
||||||
- **FR-011**: v1 MUST include an automated, CI-failing guard that flags new ad-hoc authorization patterns in tenant-scoped admin UI code.
|
|
||||||
- **FR-012**: The enforcement mechanism MUST avoid introducing avoidable performance regressions (no per-record membership lookups during render).
|
|
||||||
- **FR-013**: The enforcement mechanism MUST NOT trigger outbound HTTP calls during render; it is DB-only.
|
|
||||||
|
|
||||||
### Key Entities *(include if feature involves data)*
|
- **FR-002 Record-Scoped Tenant Context**: `UiEnforcement` MUST support record-scoped tenant resolution, where `$record` determines the tenant context.
|
||||||
|
- Table action closures receive `$record`, and enforcement evaluates membership/capability against that record-tenant.
|
||||||
|
- No hard dependency on `Filament::getTenant()` for these surfaces.
|
||||||
|
|
||||||
- **Tenant**: The isolation boundary for all tenant-scoped UI and actions.
|
- **FR-003 Bulk All-or-Nothing Preflight**: `UiEnforcement` MUST support bulk actions with a preflight authorization check that is deterministic, avoids N+1 DB queries (set-based where possible), and disables actions for mixed authorization selections.
|
||||||
- **User**: The authenticated actor attempting to view or execute actions.
|
- The default preflight decision is based on authorization only; business eligibility remains separate (records may be skipped with clear feedback).
|
||||||
- **Membership**: Whether a user is entitled to a tenant scope.
|
|
||||||
- **Capability**: A named permission from the canonical capability registry.
|
|
||||||
- **Action**: A discrete operation exposed in the tenant-scoped admin interface.
|
|
||||||
|
|
||||||
### Assumptions
|
- **FR-004 Canonical Capability Enforcement**: All v2-migrated surfaces MUST reference capabilities only via `Capabilities::*` constants.
|
||||||
|
|
||||||
- Default tooltip language is English (i18n may be added later).
|
- **FR-005 Stable Guardrails**: The file-scan guard MUST remain CI-failing and allowlist-driven.
|
||||||
- Non-destructive bulk actions are in scope for v1; destructive bulk actions may be supported but are not required for v1 completion.
|
- Allowlist entries MUST be removed as surfaces are migrated.
|
||||||
- Global search tenant scoping is out of scope for this spec (covered by separate work), but this feature must not introduce new leaks.
|
- New ad-hoc patterns in `app/Filament/**` MUST fail CI.
|
||||||
|
|
||||||
## Success Criteria *(mandatory)*
|
- **FR-006 Suite-wide Migration Targets (v2)**: v2 MUST migrate the following surface categories:
|
||||||
|
- **Tier 1 (high impact / high risk)**:
|
||||||
|
- Backup/Restore surfaces that currently contain ad-hoc RBAC patterns:
|
||||||
|
- BackupSet resources / relation managers (mixed visibility)
|
||||||
|
- RestoreRun resources / actions (mixed visibility)
|
||||||
|
- TenantResource table actions (record-scoped tenant)
|
||||||
|
- **Tier 2 (remaining allowlist “easy wins”)**:
|
||||||
|
- Inventory item / inventory sync run resources
|
||||||
|
- Any remaining Entra group run list pages
|
||||||
|
- ProviderConnection pages where mixed visibility exists (if not completed in v1)
|
||||||
|
- v2 MUST produce a measurable allowlist reduction (at minimum: remove allowlist entries for all migrated files).
|
||||||
|
|
||||||
<!--
|
- **FR-007 Tests for Each Migrated Surface**: Each migrated surface MUST have at least one focused integration test asserting:
|
||||||
ACTION REQUIRED: Define measurable success criteria.
|
- Non-member: action hidden + cannot execute
|
||||||
These must be technology-agnostic and measurable.
|
- Member no cap: visible disabled + tooltip
|
||||||
-->
|
- Member with cap: enabled executes
|
||||||
|
|
||||||
|
### Non-Functional Requirements
|
||||||
|
|
||||||
|
- **NFR-001 DB-only Rendering**: Helper and migrations MUST not introduce outbound HTTP during rendering (no Graph calls; DB only).
|
||||||
|
- **NFR-002 Performance / Query Discipline**: Membership/capability evaluation should be O(1) per request once membership is loaded (no repeated membership queries per action render). Bulk preflight MUST be set-based where feasible.
|
||||||
|
|
||||||
|
## Design
|
||||||
|
|
||||||
|
### 7.1 `UiEnforcement` API extensions (v2)
|
||||||
|
|
||||||
|
#### 7.1.1 Preserve / Compose Visibility (mixed visibility)
|
||||||
|
|
||||||
|
Add explicit composition methods:
|
||||||
|
|
||||||
|
- `preserveVisibility()`
|
||||||
|
- Means: do not write `->visible(...)` / `->hidden(...)` closures.
|
||||||
|
- Only apply disabled/tooltip/confirmation, and always enforce server-side authorization for any mutation action (Policy/Gate), independent of UI visibility/disabled state.
|
||||||
|
- Optional: add additional defensive guards (e.g., explicit deny-as-not-found behavior) where the action/page lifecycle makes it reachable.
|
||||||
|
- Restriction: tenant-scoped surfaces only (record-scoped / cross-tenant lists MUST NOT use this).
|
||||||
|
- `andVisibleWhen(callable $businessVisible)`
|
||||||
|
- Wrap business-visible closure and AND it with RBAC membership/capability visibility checks.
|
||||||
|
- `andHiddenWhen(callable $businessHidden)`
|
||||||
|
- Combine business-hidden logic with RBAC hidden logic.
|
||||||
|
|
||||||
|
**Rule**: Default `apply()` sets RBAC visibility itself. Mixed-visibility surfaces MUST explicitly opt in to preservation or composition.
|
||||||
|
|
||||||
|
#### 7.1.2 Tenant context providers
|
||||||
|
|
||||||
|
Introduce explicit tenant resolver configuration:
|
||||||
|
|
||||||
|
- `tenantFromFilament()` (default)
|
||||||
|
- `tenantFromRecord()` (for record == tenant)
|
||||||
|
- `tenantFrom(callable $resolver)` (custom mapping for pages where record maps to tenant)
|
||||||
|
|
||||||
|
This MUST work for:
|
||||||
|
|
||||||
|
- header actions (no record parameter)
|
||||||
|
- table actions (record available in closure)
|
||||||
|
- bulk actions (records collection or selection IDs)
|
||||||
|
|
||||||
|
#### 7.1.3 Bulk preflight
|
||||||
|
|
||||||
|
For bulk actions add:
|
||||||
|
|
||||||
|
- `preflightSelection(callable $preflight)` OR built-in modes:
|
||||||
|
- `preflightByTenantMembership()` using IDs and tenant context
|
||||||
|
- `preflightByCapability()` for all selected records
|
||||||
|
|
||||||
|
**Default for v2**: disable bulk action if any unauthorized (all-or-nothing).
|
||||||
|
|
||||||
|
### 7.2 Execution semantics (documented)
|
||||||
|
|
||||||
|
Spec and quickstart MUST explicitly state:
|
||||||
|
|
||||||
|
- hidden/disabled actions are blocked by Filament as no-ops, which is acceptable security behavior
|
||||||
|
- 404/403 semantics are asserted at routing/page access and by server-side checks where reachable
|
||||||
|
|
||||||
|
### 7.3 Migration policy
|
||||||
|
|
||||||
|
- Migrate 1–2 files per batch.
|
||||||
|
- Remove allowlist entry in the same batch.
|
||||||
|
- Run Pint + targeted tests + guard test each batch.
|
||||||
|
|
||||||
|
## User Stories & Testing
|
||||||
|
|
||||||
|
### US1 — Mixed Visibility: Backup/Restore surfaces become UiEnforcement-compliant (Priority: P1)
|
||||||
|
|
||||||
|
As a tenant admin, I want backup/restore actions to follow the same RBAC UX patterns while preserving business visibility logic (trashed, filter state).
|
||||||
|
|
||||||
|
**Acceptance Criteria**
|
||||||
|
|
||||||
|
- Existing visibility behavior remains unchanged.
|
||||||
|
- RBAC behavior is consistent (hidden/disabled + tooltip).
|
||||||
|
- No ad-hoc `Gate/abort` patterns remain on migrated surfaces.
|
||||||
|
|
||||||
|
### US2 — Record-Scoped Tenant: TenantResource actions enforced correctly (Priority: P1)
|
||||||
|
|
||||||
|
As a tenant-plane user with multiple tenant memberships, I want tenant list row actions to be enforced per tenant record without leaking other tenants.
|
||||||
|
|
||||||
|
**Acceptance Criteria**
|
||||||
|
|
||||||
|
- Row actions use record-tenant context.
|
||||||
|
- Non-member never sees row actions for that tenant (hidden).
|
||||||
|
- Member lacking capability sees disabled + tooltip.
|
||||||
|
|
||||||
|
### US3 — Bulk action preflight prevents partial execution (Priority: P2)
|
||||||
|
|
||||||
|
As an operator, bulk actions should be disabled if selection contains unauthorized records.
|
||||||
|
|
||||||
|
**Acceptance Criteria**
|
||||||
|
|
||||||
|
- All-or-nothing semantics.
|
||||||
|
- No partial execution.
|
||||||
|
- Deterministic feedback (standard tooltip message).
|
||||||
|
|
||||||
|
### US4 — Guard allowlist shrinks with each migration (Priority: P2)
|
||||||
|
|
||||||
|
As a maintainer, I want CI to fail if new ad-hoc RBAC patterns are added in Filament.
|
||||||
|
|
||||||
|
**Acceptance Criteria**
|
||||||
|
|
||||||
|
- Allowlist reduction is measurable (count down).
|
||||||
|
- New violations fail tests.
|
||||||
|
|
||||||
|
## Verification Checklist (manual + automated)
|
||||||
|
|
||||||
|
### Automated
|
||||||
|
|
||||||
|
- `vendor/bin/sail bin pint --dirty`
|
||||||
|
- `vendor/bin/sail artisan test tests/Feature/Guards --compact`
|
||||||
|
- Focused tests per migrated surface
|
||||||
|
- RBAC suites: `tests/Feature/Rbac` + `tests/Unit/Support/Rbac`
|
||||||
|
|
||||||
|
### Manual spot checks (UI)
|
||||||
|
|
||||||
|
- Login as Readonly → verify disabled tooltips and no execution.
|
||||||
|
- Remove membership → verify actions disappear and cannot execute.
|
||||||
|
- Bulk action selection with mixed permissions → action disabled.
|
||||||
|
|
||||||
|
## Rollout / Migration Strategy
|
||||||
|
|
||||||
|
- Keep the guard allowlist for legacy files.
|
||||||
|
- Every migration PR reduces allowlist entries.
|
||||||
|
- Any new Filament file MUST NOT introduce forbidden patterns.
|
||||||
|
|
||||||
|
## v2 Deliverables
|
||||||
|
|
||||||
|
### Code
|
||||||
|
|
||||||
|
- `UiEnforcement` enhancements: mixed visibility + record-tenant context + bulk preflight support.
|
||||||
|
- Migrated Tier 1 and Tier 2 target surfaces.
|
||||||
|
- Expanded tests for v2 targets.
|
||||||
|
|
||||||
|
### Docs
|
||||||
|
|
||||||
|
- Update the feature quickstart with mixed-visibility recipes and record-tenant examples.
|
||||||
|
- Update spec/constitution references if needed (no new constitution rules unless discovered gaps).
|
||||||
|
|
||||||
|
### Quality
|
||||||
|
|
||||||
|
- Guard remains CI-failing and stable.
|
||||||
|
- Allowlist shrunk measurably.
|
||||||
|
|
||||||
|
## Success Criteria
|
||||||
|
|
||||||
### Measurable Outcomes
|
### Measurable Outcomes
|
||||||
|
|
||||||
- **SC-001**: For all migrated tenant-scoped action surfaces, 100% of non-member execution attempts are denied with 404 semantics (verified by automated tests).
|
- **SC-001**: For every migrated surface, tests assert the RBAC-UX contract: non-member hidden/no-exec, member-no-cap disabled + tooltip, member-with-cap enabled + executes.
|
||||||
- **SC-002**: For all migrated tenant-scoped action surfaces, 100% of member-but-unauthorized execution attempts are denied with 403 (verified by automated tests).
|
- **SC-002**: Guard allowlist removes entries for every migrated file, and CI continues to fail for newly introduced ad-hoc auth patterns in `app/Filament/**`.
|
||||||
- **SC-003**: For all migrated tenant-scoped action surfaces, members lacking capability see the action visible-but-disabled with the standard tooltip (verified by automated tests and/or UI assertions).
|
- **SC-003**: Bulk preflight authorization is deterministic and does not degrade into per-record membership queries (verified via query count assertions on representative bulk selection tests).
|
||||||
- **SC-004**: At least one automated guard exists that flags newly introduced ad-hoc authorization patterns in tenant-scoped admin UI code.
|
- **SC-004**: Rendering/hydration tests for migrated pages/actions enforce DB-only rendering (stray HTTP requests are prevented during render).
|
||||||
- **SC-005**: v1 demonstrates adoption by migrating 3–6 exemplar action surfaces, reducing duplicate authorization wiring in those areas.
|
|
||||||
|
|||||||
@ -1,254 +1,215 @@
|
|||||||
# Tasks: RBAC UI Enforcement Helper v1
|
---
|
||||||
|
|
||||||
**Input**: Design documents from `/specs/066-rbac-ui-enforcement-helper/`
|
description: "Task list for RBAC UI Enforcement Helper v2"
|
||||||
**Prerequisites**: plan.md ✓, spec.md ✓, quickstart.md ✓
|
|
||||||
|
|
||||||
**Tests**: REQUIRED (Pest) — this feature changes runtime authorization behavior.
|
|
||||||
**RBAC**: This feature IS the RBAC enforcement helper — all tasks enforce constitution RBAC-UX rules.
|
|
||||||
|
|
||||||
**Organization**: Tasks grouped by user story for independent implementation.
|
|
||||||
|
|
||||||
## Format: `[ID] [P?] [Story?] Description`
|
|
||||||
|
|
||||||
- **[P]**: Can run in parallel (different files, no dependencies)
|
|
||||||
- **[Story]**: US1/US2/US3 for user story phases; omitted for Setup/Foundational/Polish
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Phase 1: Setup
|
# Tasks: RBAC UI Enforcement Helper v2 (Suite-wide, Mixed Visibility, Record-Scoped)
|
||||||
|
|
||||||
**Purpose**: Create helper infrastructure with no external dependencies
|
**Input**: Design documents from `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/`
|
||||||
|
**Prerequisites**: `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/plan.md` (required), `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/spec.md` (required), `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/research.md`, `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/data-model.md`, `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/contracts/`, `/Users/ahmeddarrazi/Documents/projects/TenantAtlas-066-rbac-ui-enforcement-helper-v2/specs/066-rbac-ui-enforcement-helper/quickstart.md`
|
||||||
|
|
||||||
- [X] T001 Create directory structure `app/Support/Rbac/`
|
**Tests**: REQUIRED (Pest) for runtime behavior changes
|
||||||
- [X] T002 [P] Create `UiTooltips.php` with tooltip constants in `app/Support/Rbac/UiTooltips.php`
|
**RBAC**: Tenant plane only (`/admin/t/{tenant}`); non-member is deny-as-not-found (404); member without capability is disabled + tooltip and cannot execute; destructive actions require confirmation
|
||||||
- [X] T003 [P] Create `TenantAccessContext.php` DTO in `app/Support/Rbac/TenantAccessContext.php`
|
**Organization**: Tasks are grouped by user story (US1–US4) to enable independent delivery.
|
||||||
|
|
||||||
|
## Phase 1: Setup (Shared Infrastructure)
|
||||||
|
|
||||||
|
**Purpose**: Prepare local environment + validate baseline
|
||||||
|
|
||||||
|
- [X] T001 Start containers with `./vendor/bin/sail up -d` (script: `./vendor/bin/sail`)
|
||||||
|
- [X] T002 Run baseline guard suite with `./vendor/bin/sail artisan test tests/Feature/Guards --compact` (folder: `tests/Feature/Guards`)
|
||||||
|
- [X] T003 Run baseline tenant RBAC suite with `./vendor/bin/sail artisan test tests/Feature/Rbac --compact` (folder: `tests/Feature/Rbac`)
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Phase 2: Foundational (Blocking Prerequisites)
|
## Phase 2: Foundational (Blocking Prerequisites)
|
||||||
|
|
||||||
**Purpose**: Core `UiEnforcement` helper — MUST complete before any user story tests
|
**Purpose**: Shared helper + guardrails required by all story migrations
|
||||||
|
|
||||||
**⚠️ CRITICAL**: No user story work can begin until this phase is complete
|
**Checkpoint**: `UiEnforcement` supports mixed visibility, record-scoped tenant resolution, and bulk preflight; guard exists and is allowlist-driven
|
||||||
|
|
||||||
- [X] T004 Implement `UiEnforcement::forAction()` static method in `app/Support/Rbac/UiEnforcement.php`
|
- [X] T004 Create `UiTooltips` with the v2 default disabled tooltip copy in `app/Support/Auth/UiTooltips.php`
|
||||||
- [X] T005 Implement `->requireMembership()` method (default: true) in `app/Support/Rbac/UiEnforcement.php`
|
- [X] T005 Create/verify v1 baseline `UiEnforcement` helper in `app/Support/Auth/UiEnforcement.php` (apply hidden/disabled/tooltip + server-side guard using `Capabilities::*`)
|
||||||
- [X] T006 Implement `->requireCapability(string $capability)` method in `app/Support/Rbac/UiEnforcement.php`
|
- [X] T006 [P] Add mixed visibility APIs (`preserveVisibility()` tenant-scoped only, `andVisibleWhen()`, `andHiddenWhen()`) in `app/Support/Auth/UiEnforcement.php`
|
||||||
- [X] T007 Implement `->destructive()` method (confirmation modal) in `app/Support/Rbac/UiEnforcement.php`
|
- [X] T007 [P] Add tenant resolver APIs (`tenantFromFilament()`, `tenantFromRecord()`, `tenantFrom(callable)`) in `app/Support/Auth/UiEnforcement.php`
|
||||||
- [X] T008 Implement `->tooltip(string $message)` override method in `app/Support/Rbac/UiEnforcement.php`
|
- [X] T008 [P] Add bulk preflight APIs (`preflightSelection()`, `preflightByTenantMembership()`, `preflightByCapability()`) with authorization-only all-or-nothing default in `app/Support/Auth/UiEnforcement.php`
|
||||||
- [X] T009 Implement `->apply()` method (sets hidden/disabled/guards) in `app/Support/Rbac/UiEnforcement.php`
|
- [X] T009 [P] Add helper unit tests (mixed visibility restriction + tenant resolvers + preflight decisions) in `tests/Unit/Auth/UiEnforcementTest.php`
|
||||||
- [X] T010 Implement `UiEnforcement::forTableAction()` static method in `app/Support/Rbac/UiEnforcement.php`
|
- [X] T010 [P] Add query-count regression test for bulk preflight (no N+1 membership lookups) in `tests/Unit/Auth/UiEnforcementBulkPreflightQueryCountTest.php`
|
||||||
- [X] T011 Implement `UiEnforcement::forBulkAction()` static method with all-or-nothing logic in `app/Support/Rbac/UiEnforcement.php`
|
- [X] T011 Add CI-failing allowlist-driven guard for ad-hoc Filament auth patterns in `tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php`
|
||||||
|
- [X] T012 Run the new guard test with `./vendor/bin/sail artisan test tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php --compact` (test: `tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php`)
|
||||||
**Checkpoint**: `UiEnforcement` class ready — user story tests can now be written
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Phase 3: User Story 1 — Tenant member sees consistent disabled UX (Priority: P1) 🎯 MVP
|
## Phase 3: User Story 1 — Mixed Visibility (Backup/Restore) (Priority: P1) 🎯 MVP
|
||||||
|
|
||||||
**Goal**: Members lacking capability see actions visible-but-disabled with standard tooltip; 403 on execution
|
**Goal**: Backup/Restore actions preserve business visibility while enforcing RBAC UX (hidden/disabled + standard tooltip) via `UiEnforcement`
|
||||||
|
|
||||||
**Independent Test**: Visit tenant page as member with insufficient permission → action disabled with tooltip, cannot execute
|
**Independent Test**: A non-member cannot access tenant routes (404); a member without capability sees actions disabled with the standard tooltip and cannot execute; a member with capability can execute (with confirmation for destructive actions)
|
||||||
|
|
||||||
### Tests for User Story 1
|
### Tests for User Story 1
|
||||||
|
|
||||||
- [X] T012 [P] [US1] Test: member without capability sees disabled action + tooltip in `tests/Feature/Rbac/UiEnforcementMemberDisabledTest.php`
|
- [X] T013 [P] [US1] Add BackupSet RBAC UX integration tests (include `Http::preventStrayRequests()`) in `tests/Feature/Filament/BackupSetUiEnforcementTest.php`
|
||||||
- [X] T013 [P] [US1] Test: member without capability is blocked from execution in `tests/Feature/Rbac/UiEnforcementMemberDisabledTest.php`
|
- [X] T014 [P] [US1] Add RestoreRun RBAC UX integration tests (include `Http::preventStrayRequests()`) in `tests/Feature/Filament/RestoreRunUiEnforcementTest.php`
|
||||||
- [X] T014 [P] [US1] Test: member with capability sees enabled action + can execute in `tests/Feature/Rbac/UiEnforcementMemberDisabledTest.php`
|
|
||||||
- [X] T014a [P] [US1] Test: destructive action shows confirmation modal before execution in `tests/Feature/Rbac/UiEnforcementDestructiveTest.php`
|
|
||||||
|
|
||||||
### Implementation for User Story 1
|
### Implementation for User Story 1
|
||||||
|
|
||||||
- [X] T015 [US1] Validate `->apply()` correctly sets `->disabled()` + `->tooltip()` for members lacking capability (logic in T009; this task verifies + adjusts if needed) in `app/Support/Rbac/UiEnforcement.php`
|
- [X] T015 [US1] Migrate mixed-visibility actions + bulk actions to `UiEnforcement` in `app/Filament/Resources/BackupSetResource.php`
|
||||||
- [X] T016 [US1] Validate `->apply()` correctly blocks unauthorized execution (via Filament's isDisabled check + defense-in-depth abort) in `app/Support/Rbac/UiEnforcement.php`
|
- [X] T016 [US1] Migrate relation manager actions (incl. standardized tooltip copy via `UiTooltips`) in `app/Filament/Resources/BackupSetResource/RelationManagers/BackupItemsRelationManager.php`
|
||||||
- ~~T017 [US1] Migrate TenantResource table actions to UiEnforcement~~ **OUT OF SCOPE v1**: TenantResource is record==tenant, not tenant-scoped
|
- [X] T017 [US1] Migrate mixed-visibility actions + bulk actions to `UiEnforcement` in `app/Filament/Resources/RestoreRunResource.php`
|
||||||
- [X] T018 [US1] Migrate ProviderConnectionResource actions to UiEnforcement (mixed visibility via `preserveVisibility()`) in `app/Filament/Resources/ProviderConnectionResource.php`
|
- [X] T018 [US1] Replace `Gate`/`abort_*` access checks with `UiEnforcement` (tenant-scoped: non-member 404, member without cap 403) in `app/Filament/Resources/RestoreRunResource/Pages/CreateRestoreRun.php`
|
||||||
|
- [X] T019 [US1] Remove allowlist entries for migrated Backup/Restore files in `tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php`
|
||||||
**Checkpoint**: US1 complete — members see consistent disabled UX with tooltip (exemplar: ListPolicies)
|
- [X] T020 [US1] Run story tests with `./vendor/bin/sail artisan test tests/Feature/Filament/BackupSetUiEnforcementTest.php tests/Feature/Filament/RestoreRunUiEnforcementTest.php --compact` (tests: `tests/Feature/Filament/BackupSetUiEnforcementTest.php`)
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Phase 4: User Story 2 — Non-members cannot infer tenant resources (Priority: P2)
|
## Phase 4: User Story 2 — Record-Scoped Tenant (TenantResource actions) (Priority: P1)
|
||||||
|
|
||||||
**Goal**: Non-members receive 404 (deny-as-not-found) for all tenant-scoped actions; actions hidden in UI
|
**Goal**: Tenant list row actions are enforced per-tenant record (record-scoped tenancy) with non-member deny-as-not-found (404) and standardized member disabled UX
|
||||||
|
|
||||||
**Independent Test**: Access tenant page as non-member → actions hidden, execution returns 404
|
**Independent Test**: A user sees only tenants they are a member of; direct access to another tenant record denies 404; row actions for permitted tenants are disabled/enabled per capability with standard tooltip
|
||||||
|
|
||||||
### Tests for User Story 2
|
### Tests for User Story 2
|
||||||
|
|
||||||
- [X] T019 [P] [US2] Test: non-member sees action hidden in UI in `tests/Feature/Rbac/UiEnforcementNonMemberHiddenTest.php`
|
- [X] T021 [P] [US2] Extend tenant row action tests to assert disabled + tooltip + no-exec (include `Http::preventStrayRequests()`) in `tests/Feature/Filament/TenantActionsAuthorizationTest.php`
|
||||||
- [X] T020 [P] [US2] Test: non-member action is blocked (via Filament hidden-action semantics) in `tests/Feature/Rbac/UiEnforcementNonMemberHiddenTest.php`
|
- [X] T022 [P] [US2] Add record-scoped non-member 404 tests for tenant view/edit routes in `tests/Feature/Filament/TenantPortfolioContextSwitchTest.php`
|
||||||
- [X] T021 [P] [US2] Test: membership revoked mid-session still enforces protection in `tests/Feature/Rbac/UiEnforcementNonMemberHiddenTest.php`
|
|
||||||
|
|
||||||
### Implementation for User Story 2
|
### Implementation for User Story 2
|
||||||
|
|
||||||
- [X] T022 [US2] Validate `->apply()` correctly sets `->hidden()` for non-members (logic in T009; this task verifies + adjusts if needed) in `app/Support/Rbac/UiEnforcement.php`
|
- [X] T023 [US2] Migrate TenantResource row actions to `UiEnforcement` using `tenantFromRecord()` (remove ad-hoc `Gate`/`abort_*`) in `app/Filament/Resources/TenantResource.php`
|
||||||
- [X] T023 [US2] Validate `->apply()` blocks non-member execution (via Filament's isHidden → isDisabled chain; 404 server-side guard is defense-in-depth) in `app/Support/Rbac/UiEnforcement.php`
|
- [X] T024 [US2] Migrate record-scoped edit-page destructive actions to `UiEnforcement` (remove ad-hoc `Gate`/`abort_*`) in `app/Filament/Resources/TenantResource/Pages/EditTenant.php`
|
||||||
- [X] T024 [US2] Migrate BackupSetResource actions (row + bulk) to UiEnforcement (mixed visibility via `preserveVisibility()`) in `app/Filament/Resources/BackupSetResource.php`
|
- [X] T025 [US2] Remove allowlist entries for migrated TenantResource files in `tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php`
|
||||||
- [X] T025 [US2] Migrate PolicyResource sync actions to UiEnforcement in `app/Filament/Resources/PolicyResource/Pages/ListPolicies.php`
|
- [X] T026 [US2] Run story tests with `./vendor/bin/sail artisan test tests/Feature/Filament/TenantActionsAuthorizationTest.php tests/Feature/Filament/TenantPortfolioContextSwitchTest.php --compact` (tests: `tests/Feature/Filament/TenantActionsAuthorizationTest.php`)
|
||||||
|
|
||||||
**Checkpoint**: US2 complete — non-members receive 404 semantics, no information leakage
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Phase 5: User Story 3 — Maintainers add actions safely by default (Priority: P3)
|
## Phase 5: User Story 3 — Bulk Preflight (All-or-Nothing Authorization) (Priority: P2)
|
||||||
|
|
||||||
**Goal**: CI-failing guard flags new ad-hoc authorization patterns; standard pattern documented
|
**Goal**: Bulk actions disable deterministically when selection contains any unauthorized record; no partial execution
|
||||||
|
|
||||||
**Independent Test**: Introduce ad-hoc `Gate::allows` or `abort_unless()` in Filament → guard test fails
|
**Independent Test**: Selecting tenants with mixed authorization disables the bulk action; selecting only authorized tenants executes; business-ineligible records may be skipped with clear feedback
|
||||||
|
|
||||||
### Tests for User Story 3
|
### Tests for User Story 3
|
||||||
|
|
||||||
- [X] T026 [P] [US3] Guard test: scan `app/Filament/**` for forbidden ad-hoc patterns (Gate + abort helpers) in `tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php`
|
- [X] T027 [P] [US3] Update bulk sync tests to assert all-or-nothing authorization disable (include `Http::preventStrayRequests()`) in `tests/Feature/Filament/TenantPortfolioContextSwitchTest.php`
|
||||||
- [X] T027 [P] [US3] Unit test: UiEnforcement uses only canonical Capabilities constants in `tests/Unit/Support/Rbac/UiEnforcementTest.php`
|
|
||||||
|
|
||||||
### Implementation for User Story 3
|
### Implementation for User Story 3
|
||||||
|
|
||||||
- [X] T028 [US3] Replace Pest-Arch guard with stable file-scan guard (CI-failing, allowlist for legacy only) in `tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php`
|
- [X] T028 [US3] Add authorization-only bulk preflight for `syncSelected` (disable when any selected tenant lacks `Capabilities::TENANT_SYNC`) in `app/Filament/Resources/TenantResource.php`
|
||||||
- [X] T029 [US3] Migrate EntraGroupResource sync actions to UiEnforcement in `app/Filament/Resources/EntraGroupResource/Pages/ListEntraGroups.php`
|
- [X] T029 [US3] Ensure bulk action feedback cleanly separates authorization vs eligibility (skip inactive with notification; do not partially execute unauthorized) in `app/Filament/Resources/TenantResource.php`
|
||||||
- [X] T030 [US3] Remove Gate facade usage from FindingResource (migrate auth to canonical checks) in `app/Filament/Resources/FindingResource.php`
|
- [X] T030 [US3] Run story tests with `./vendor/bin/sail artisan test tests/Feature/Filament/TenantPortfolioContextSwitchTest.php --compact` (test: `tests/Feature/Filament/TenantPortfolioContextSwitchTest.php`)
|
||||||
|
|
||||||
**Checkpoint**: US3 complete — guardrail prevents regression (file-scan), exemplar surfaces migrated
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Phase 6: Polish & Cross-Cutting Concerns
|
## Phase 6: User Story 4 — Guard Allowlist Shrink + Tier 2 Migrations (Priority: P2)
|
||||||
|
|
||||||
**Purpose**: Cleanup, additional tests, documentation
|
**Goal**: CI fails for new ad-hoc auth patterns in `app/Filament/**`; allowlist count shrinks measurably by migrating remaining targets
|
||||||
|
|
||||||
- [X] T031 [P] PHPDoc blocks present on all public methods in `app/Support/Rbac/UiEnforcement.php`
|
**Independent Test**: Guard fails if a new forbidden pattern is introduced; allowlist removes entries as files are migrated; each migrated Tier 2 surface has a focused RBAC UX test
|
||||||
- [X] T032 [P] Update quickstart.md with migration examples in `specs/066-rbac-ui-enforcement-helper/quickstart.md`
|
|
||||||
- [X] T033 Run Pint formatter on new files with `vendor/bin/sail bin pint app/Support/Rbac`
|
### Tests for User Story 4
|
||||||
- [X] T034 Run full test suite with `vendor/bin/sail artisan test --compact` — 837 passed, 5 skipped
|
|
||||||
- [X] T035 Validate quickstart.md examples work in codebase (ListPolicies migration verified)
|
- [X] T031 [P] [US4] Extend inventory resource RBAC UX tests (include `Http::preventStrayRequests()`) in `tests/Feature/Filament/InventoryItemResourceTest.php` and `tests/Feature/Filament/InventorySyncRunResourceTest.php`
|
||||||
|
- [X] T032 [P] [US4] Extend Entra group runs RBAC UX tests (include `Http::preventStrayRequests()`) in `tests/Feature/Filament/EntraGroupSyncRunResourceTest.php`
|
||||||
|
- [X] T033 [P] [US4] Add provider connections RBAC UX tests for mixed visibility (include `Http::preventStrayRequests()`) in `tests/Feature/Filament/ProviderConnectionsUiEnforcementTest.php`
|
||||||
|
|
||||||
|
### Implementation for User Story 4
|
||||||
|
|
||||||
|
- [X] T034 [US4] Migrate inventory resources away from ad-hoc `Gate`/`abort_*` to `UiEnforcement` in `app/Filament/Resources/InventoryItemResource.php`, `app/Filament/Resources/InventoryItemResource/Pages/ListInventoryItems.php`, and `app/Filament/Resources/InventorySyncRunResource.php`
|
||||||
|
- [X] T035 [US4] Migrate Entra group list/run start surfaces away from ad-hoc `Gate`/`abort_*` to `UiEnforcement` in `app/Filament/Resources/EntraGroupResource/Pages/ListEntraGroups.php` and `app/Filament/Resources/EntraGroupSyncRunResource/Pages/ListEntraGroupSyncRuns.php`
|
||||||
|
- [X] T036 [US4] Migrate provider connection mixed-visibility actions away from ad-hoc `Gate`/`abort_*` to `UiEnforcement` in `app/Filament/Resources/ProviderConnectionResource.php`, `app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php`, and `app/Filament/Resources/ProviderConnectionResource/Pages/EditProviderConnection.php`
|
||||||
|
- [X] T037 [US4] Remove allowlist entries for migrated Tier 2 files and note the allowlist count reduction in `tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php`
|
||||||
|
- [X] T038 [US4] Run story tests with `./vendor/bin/sail artisan test tests/Feature/Filament/InventoryItemResourceTest.php tests/Feature/Filament/InventorySyncRunResourceTest.php tests/Feature/Filament/EntraGroupSyncRunResourceTest.php tests/Feature/Filament/ProviderConnectionsUiEnforcementTest.php --compact` (tests: `tests/Feature/Filament/ProviderConnectionsUiEnforcementTest.php`)
|
||||||
|
- [X] T039 [US4] Run guard suite with `./vendor/bin/sail artisan test tests/Feature/Guards --compact` (folder: `tests/Feature/Guards`)
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Phase 7: Follow-up — Findings capability cleanup (Mini-feature)
|
## Phase 7: Polish & Cross-Cutting Concerns
|
||||||
|
|
||||||
**Purpose**: Avoid overloading broad capabilities (e.g. `TENANT_SYNC`) for findings acknowledgement.
|
**Purpose**: Reduce regressions and ensure suite-wide consistency
|
||||||
|
|
||||||
- [X] T036 Add `Capabilities::TENANT_FINDINGS_ACKNOWLEDGE` in `app/Support/Auth/Capabilities.php`
|
- [X] T040 [P] Ensure all destructive actions touched in v2 include `->requiresConfirmation()` in `app/Filament/Resources/BackupSetResource.php`, `app/Filament/Resources/RestoreRunResource.php`, and `app/Filament/Resources/TenantResource/Pages/EditTenant.php`
|
||||||
- [X] T037 Grant `TENANT_FINDINGS_ACKNOWLEDGE` to Owner/Manager/Operator (not Readonly) + update role-matrix tests
|
- [X] T041 Run formatting with `./vendor/bin/sail bin pint --dirty` (script: `./vendor/bin/pint`)
|
||||||
- [X] T038 Update Finding list acknowledge action to require `TENANT_FINDINGS_ACKNOWLEDGE` in `app/Filament/Resources/FindingResource/Pages/ListFindings.php`
|
- [X] T042 Run focused test suites with `./vendor/bin/sail artisan test tests/Unit/Auth tests/Feature/Filament tests/Feature/Guards tests/Feature/Rbac --compact` (folders: `tests/Feature/Filament`)
|
||||||
- [X] T039 Refactor `FindingPolicy::update()` to use `CapabilityResolver` with `TENANT_FINDINGS_ACKNOWLEDGE` (remove ad-hoc `Gate::forUser(...)->allows(...)`)
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Phase 8: Follow-up — Legacy allowlist shrink (Stepwise)
|
|
||||||
|
|
||||||
**Purpose**: Keep shrinking the Filament guard allowlist with one-file migrations.
|
|
||||||
|
|
||||||
- [X] T040 Remove `BackupScheduleResource.php` from the legacy allowlist after migration
|
|
||||||
- [X] T041 Migrate `ListEntraGroupSyncRuns.php` to UiEnforcement + add a focused Livewire test
|
|
||||||
- [X] T042 Remove `ListEntraGroupSyncRuns.php` from the legacy allowlist after migration
|
|
||||||
- [X] T043 Migrate `ListProviderConnections.php` create action to UiEnforcement + add a focused Livewire test
|
|
||||||
- [X] T044 Remove `ListProviderConnections.php` from the legacy allowlist after migration
|
|
||||||
- [X] T045 Migrate `DriftLanding.php` generation permission check to `CapabilityResolver` (remove Gate facade) + add a focused Livewire test
|
|
||||||
- [X] T046 Remove `DriftLanding.php` from the legacy allowlist after migration
|
|
||||||
- [X] T047 Migrate `RegisterTenant.php` page-level checks to `CapabilityResolver` + replace `abort_unless()` with `abort()`
|
|
||||||
- [X] T048 Remove `RegisterTenant.php` from the legacy allowlist after migration
|
|
||||||
- [X] T049 Migrate `EditProviderConnection.php` actions + save guards to `UiEnforcement`/`CapabilityResolver` (remove Gate facade + abort_unless) + add a focused Livewire test
|
|
||||||
- [X] T050 Remove `EditProviderConnection.php` from the legacy allowlist after migration
|
|
||||||
- [X] T051 Migrate `CreateRestoreRun.php` page authorization to `CapabilityResolver` (remove Gate facade + abort_unless) + add a focused Livewire test
|
|
||||||
- [X] T052 Remove `CreateRestoreRun.php` from the legacy allowlist after migration
|
|
||||||
- [X] T053 Migrate `InventoryItemResource.php` resource authorization to `CapabilityResolver` (remove Gate facade) + add a focused Pest test
|
|
||||||
- [X] T054 Remove `InventoryItemResource.php` from the legacy allowlist after migration
|
|
||||||
- [X] T055 Migrate `VersionsRelationManager.php` restore action to `UiEnforcement`/`CapabilityResolver` (remove Gate facade + abort_unless) + add a focused Livewire test
|
|
||||||
- [X] T056 Remove `VersionsRelationManager.php` from the legacy allowlist after migration
|
|
||||||
- [X] T057 Migrate `BackupItemsRelationManager.php` actions to `UiEnforcement` (remove Gate facade) + add a focused Livewire test
|
|
||||||
- [X] T058 Remove `BackupItemsRelationManager.php` from the legacy allowlist after migration
|
|
||||||
- [X] T059 Migrate `PolicyVersionResource.php` actions/bulk actions off ad-hoc patterns to `UiEnforcement`/`CapabilityResolver` (remove Gate facade + abort_unless) while preserving metadata-only restore behavior
|
|
||||||
- [X] T060 Remove `PolicyVersionResource.php` from the legacy allowlist after migration
|
|
||||||
- [X] T061 Migrate `RestoreRunResource.php` actions/bulk actions off ad-hoc patterns to `UiEnforcement`/`CapabilityResolver` (remove Gate facade + abort_unless)
|
|
||||||
- [X] T062 Remove `RestoreRunResource.php` from the legacy allowlist after migration
|
|
||||||
- [X] T063 Fix `UiEnforcement` server-side guard to use Filament lifecycle hooks (`->before()`) to preserve Filament action parameter injection
|
|
||||||
- [X] T064 Migrate `PolicyResource.php` actions/bulk actions off ad-hoc patterns to `UiEnforcement` (remove Gate facade + abort_unless)
|
|
||||||
- [X] T065 Remove `PolicyResource.php` from the legacy allowlist after migration
|
|
||||||
- [X] T066 Migrate `EditTenant.php` archive action off ad-hoc patterns to `UiEnforcement` (remove Gate facade + abort_unless)
|
|
||||||
- [X] T067 Remove `EditTenant.php` from the legacy allowlist after migration
|
|
||||||
- [X] T068 Migrate `TenantMembershipsRelationManager.php` actions off ad-hoc patterns to `UiEnforcement` (remove Gate facade)
|
|
||||||
- [X] T069 Remove `TenantMembershipsRelationManager.php` from the legacy allowlist after migration
|
|
||||||
- [X] T070 Migrate `TenantResource.php` off ad-hoc patterns to `CapabilityResolver` (remove Gate facade + abort_unless)
|
|
||||||
- [X] T071 Remove `TenantResource.php` from the legacy allowlist after migration
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Dependencies & Execution Order
|
## Dependencies & Execution Order
|
||||||
|
|
||||||
### Phase Dependencies
|
### User Story Dependency Graph
|
||||||
|
|
||||||
- **Setup (Phase 1)**: No dependencies — can start immediately
|
```text
|
||||||
- **Foundational (Phase 2)**: Depends on Setup — BLOCKS all user stories
|
Phase 1 (Setup)
|
||||||
- **User Stories (Phase 3–5)**: All depend on Foundational; can proceed in parallel or by priority
|
↓
|
||||||
- **Polish (Phase 6)**: Depends on all user stories
|
Phase 2 (Foundation: UiEnforcement v2 + guard baseline)
|
||||||
|
↓
|
||||||
### User Story Dependencies
|
US1 (Backup/Restore mixed visibility) ─┬─→ US4 (Tier 2 migrations + allowlist shrink)
|
||||||
|
├─→ US2 (TenantResource record-scoped actions)
|
||||||
- **US1 (P1)**: Foundational only — no cross-story dependencies
|
└─→ US3 (Bulk preflight all-or-nothing)
|
||||||
- **US2 (P2)**: Foundational only — no cross-story dependencies
|
```
|
||||||
- **US3 (P3)**: Foundational only — no cross-story dependencies
|
|
||||||
|
|
||||||
### Within Each User Story
|
|
||||||
|
|
||||||
- Tests MUST be written FIRST and FAIL before implementation
|
|
||||||
- Wire logic in `UiEnforcement.php` before migrating Filament surfaces
|
|
||||||
- Migrate surfaces one at a time, verify tests pass
|
|
||||||
|
|
||||||
### Parallel Opportunities
|
### Parallel Opportunities
|
||||||
|
|
||||||
- T002 + T003 (Setup) can run in parallel
|
- Phase 2 tasks marked `[P]` can run in parallel (different files).
|
||||||
- All test tasks (T012–T014, T019–T021, T026–T027) can run in parallel
|
- Within US1, BackupSet and RestoreRun migrations can be done in parallel (different resources) if coordinated to avoid conflicting changes in `app/Support/Auth/UiEnforcement.php`.
|
||||||
- US1, US2, US3 can run in parallel after Foundational
|
- Within US4, Inventory vs EntraGroup vs ProviderConnection migrations can be parallelized (different files), but coordinate allowlist edits in `tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php`.
|
||||||
- T031 + T032 (Polish) can run in parallel
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Parallel Example: User Story 1
|
## Parallel Example: User Story 1
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
# Launch all tests for US1 together:
|
Task: "Add BackupSet RBAC UX integration tests in tests/Feature/Filament/BackupSetUiEnforcementTest.php"
|
||||||
T012: "Test: member without capability sees disabled action + tooltip"
|
Task: "Add RestoreRun RBAC UX integration tests in tests/Feature/Filament/RestoreRunUiEnforcementTest.php"
|
||||||
T013: "Test: member without capability receives 403 on execution"
|
Task: "Migrate BackupSet mixed-visibility actions to UiEnforcement in app/Filament/Resources/BackupSetResource.php"
|
||||||
T014: "Test: member with capability sees enabled action + can execute"
|
Task: "Migrate RestoreRun mixed-visibility actions to UiEnforcement in app/Filament/Resources/RestoreRunResource.php"
|
||||||
|
```
|
||||||
|
|
||||||
# Then implement sequentially:
|
---
|
||||||
T015 → T016 → T017 → T018
|
|
||||||
|
## Parallel Example: User Story 2
|
||||||
|
|
||||||
|
```bash
|
||||||
|
Task: "Extend tenant row action tests to assert disabled + tooltip + no-exec in tests/Feature/Filament/TenantActionsAuthorizationTest.php"
|
||||||
|
Task: "Add record-scoped non-member 404 tests for tenant view/edit routes in tests/Feature/Filament/TenantPortfolioContextSwitchTest.php"
|
||||||
|
Task: "Migrate TenantResource row actions to UiEnforcement using tenantFromRecord() in app/Filament/Resources/TenantResource.php"
|
||||||
|
Task: "Migrate record-scoped edit-page destructive actions to UiEnforcement in app/Filament/Resources/TenantResource/Pages/EditTenant.php"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Parallel Example: User Story 3
|
||||||
|
|
||||||
|
```bash
|
||||||
|
Task: "Update bulk sync tests to assert all-or-nothing authorization disable in tests/Feature/Filament/TenantPortfolioContextSwitchTest.php"
|
||||||
|
Task: "Add authorization-only bulk preflight for syncSelected in app/Filament/Resources/TenantResource.php"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Parallel Example: User Story 4
|
||||||
|
|
||||||
|
```bash
|
||||||
|
Task: "Extend inventory resource RBAC UX tests in tests/Feature/Filament/InventoryItemResourceTest.php"
|
||||||
|
Task: "Extend Entra group runs RBAC UX tests in tests/Feature/Filament/EntraGroupSyncRunResourceTest.php"
|
||||||
|
Task: "Add provider connections RBAC UX tests for mixed visibility in tests/Feature/Filament/ProviderConnectionsUiEnforcementTest.php"
|
||||||
|
Task: "Migrate Entra group run list surfaces to UiEnforcement in app/Filament/Resources/EntraGroupSyncRunResource.php"
|
||||||
|
Task: "Migrate provider connection pages/actions to UiEnforcement in app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php"
|
||||||
```
|
```
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Implementation Strategy
|
## Implementation Strategy
|
||||||
|
|
||||||
### MVP First (User Story 1 Only)
|
### MVP First (User Story 1)
|
||||||
|
|
||||||
1. Complete Phase 1: Setup (T001–T003)
|
1. Complete Phase 1 + Phase 2
|
||||||
2. Complete Phase 2: Foundational (T004–T011)
|
2. Complete US1 (BackupSet + RestoreRun mixed visibility enforcement)
|
||||||
3. Complete Phase 3: User Story 1 (T012–T018)
|
3. Validate with `tests/Feature/Filament/BackupSetUiEnforcementTest.php` and `tests/Feature/Filament/RestoreRunUiEnforcementTest.php`
|
||||||
4. **STOP and VALIDATE**: Members see disabled + tooltip + 403
|
|
||||||
5. Deploy/demo if ready
|
|
||||||
|
|
||||||
### Incremental Delivery
|
### Incremental Delivery
|
||||||
|
|
||||||
1. Setup + Foundational → `UiEnforcement` ready
|
1. US1 → migrate highest-risk backup/restore surfaces + tests + allowlist reduction
|
||||||
2. US1 → Consistent disabled UX for members (MVP!)
|
2. US2 → record-scoped tenant actions enforced per row + 404 non-member on direct access
|
||||||
3. US2 → Non-member 404 enforcement
|
3. US3 → bulk preflight all-or-nothing authorization
|
||||||
4. US3 → CI-failing guardrail + all 6 surfaces migrated
|
4. US4 → Tier 2 migrations + measurable allowlist shrink + guard stability
|
||||||
5. Polish → Docs, cleanup, full test suite
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Summary
|
|
||||||
|
|
||||||
| Metric | Count |
|
|
||||||
|--------|-------|
|
|
||||||
| Total tasks | 40 |
|
|
||||||
| Setup tasks | 3 |
|
|
||||||
| Foundational tasks | 8 |
|
|
||||||
| US1 tasks | 8 |
|
|
||||||
| US2 tasks | 7 |
|
|
||||||
| US3 tasks | 5 |
|
|
||||||
| Polish tasks | 5 |
|
|
||||||
| Follow-up tasks | 4 |
|
|
||||||
| Parallel opportunities | 13 |
|
|
||||||
| MVP scope | Phases 1–3 (T001–T018) |
|
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user