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