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