TenantAtlas/specs/066-rbac-ui-enforcement-helper/spec.md

# Feature Specification: RBAC UI Enforcement Helper v2 (Suite-wide, Mixed Visibility, Record-Scoped)

**Feature Branch**: `066-rbac-ui-enforcement-helper-v2`  
**Created**: 2026-01-30  
**Status**: Draft  
**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

### 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; 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: 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).

### 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).
- 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.
- 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).
- 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.

## Problem Statement

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:

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)

v2 makes the helper suite-ready by adding explicit primitives for these cases and migrating remaining high-risk surfaces.

## Goals

### G1 — Suite-wide RBAC UX consistency

A tenant member sees a consistent UI pattern everywhere:

- **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

### G2 — Eliminate remaining ad-hoc RBAC in Filament

Remove `Gate::...`, `abort_*`, policy ability string literals, and custom RBAC patterns from remaining targeted Filament surfaces, replacing them with `UiEnforcement` and canonical `Capabilities::*`.

### G3 — Support mixed visibility and record-scoped tenancy

Provide first-class APIs so teams can migrate without special casing per resource.

### G4 — Keep enforcement testable and stable

Guard remains stable (file scan), allowlist shrinks, and we add targeted integration tests for each migrated surface.

## Non-Goals

- 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.

## Terms

- **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.

## RBAC-UX Contract (v2)

This feature follows the RBAC constitution rules and clarifies Filament execution reality.

### 5.1 Non-member

- **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.

**Note**: Filament treats hidden as non-executable (silent no-op). This is acceptable and is the enforced behavior.

### 5.2 Member without capability

- Action is visible but disabled.
- Tooltip MUST be from `UiTooltips` (or a standard override).
- 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.”

### 5.3 Member with capability

- Enabled and executes.
- Any destructive action MUST require confirmation (`->requiresConfirmation()`).

### 5.4 Bulk actions (default)

All-or-nothing (authorization): if any selected record fails authorization, the bulk action is disabled (members) and cannot execute; no partial execution.
Business eligibility (e.g., inactive/archived) is handled separately: ineligible records may be skipped with deterministic user feedback.

## Requirements

### Functional Requirements

- **FR-001 Mixed Visibility Support**: `UiEnforcement` MUST support combining RBAC visibility with existing business visibility patterns without overriding them.
  - A resource can keep business `->visible()` conditions (trashed/state/filter).
  - RBAC adds membership/capability constraints without losing business visibility behavior.

- **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.

- **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.
  - The default preflight decision is based on authorization only; business eligibility remains separate (records may be skipped with clear feedback).

- **FR-004 Canonical Capability Enforcement**: All v2-migrated surfaces MUST reference capabilities only via `Capabilities::*` constants.

- **FR-005 Stable Guardrails**: The file-scan guard MUST remain CI-failing and allowlist-driven.
  - Allowlist entries MUST be removed as surfaces are migrated.
  - New ad-hoc patterns in `app/Filament/**` MUST fail CI.

- **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:
  - Non-member: action hidden + cannot execute
  - 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

- **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**: 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**: 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**: Rendering/hydration tests for migrated pages/actions enforce DB-only rendering (stray HTTP requests are prevented during render).