diff --git a/specs/066-rbac-ui-enforcement-helper/checklists/requirements.md b/specs/066-rbac-ui-enforcement-helper/checklists/requirements.md index 91ef52d..871186f 100644 --- a/specs/066-rbac-ui-enforcement-helper/checklists/requirements.md +++ b/specs/066-rbac-ui-enforcement-helper/checklists/requirements.md @@ -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 -**Created**: 2026-01-28 -**Feature**: [specs/066-rbac-ui-enforcement-helper/spec.md](../spec.md) +**Created**: 2026-01-30 +**Feature**: [spec.md](../spec.md) ## Content Quality @@ -31,4 +31,6 @@ ## Feature Readiness ## 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`. diff --git a/specs/066-rbac-ui-enforcement-helper/contracts/guardrails.md b/specs/066-rbac-ui-enforcement-helper/contracts/guardrails.md new file mode 100644 index 0000000..c61df9f --- /dev/null +++ b/specs/066-rbac-ui-enforcement-helper/contracts/guardrails.md @@ -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. + diff --git a/specs/066-rbac-ui-enforcement-helper/contracts/ui-enforcement.md b/specs/066-rbac-ui-enforcement-helper/contracts/ui-enforcement.md new file mode 100644 index 0000000..bba31b8 --- /dev/null +++ b/specs/066-rbac-ui-enforcement-helper/contracts/ui-enforcement.md @@ -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. + diff --git a/specs/066-rbac-ui-enforcement-helper/data-model.md b/specs/066-rbac-ui-enforcement-helper/data-model.md new file mode 100644 index 0000000..4b27238 --- /dev/null +++ b/specs/066-rbac-ui-enforcement-helper/data-model.md @@ -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) + diff --git a/specs/066-rbac-ui-enforcement-helper/plan.md b/specs/066-rbac-ui-enforcement-helper/plan.md new file mode 100644 index 0000000..22beadd --- /dev/null +++ b/specs/066-rbac-ui-enforcement-helper/plan.md @@ -0,0 +1,143 @@ +# Implementation Plan: 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` +**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 + +- Extend the RBAC UI enforcement suite (Feature 066 v1) with explicit primitives for: + - mixed visibility composition (business visibility + RBAC visibility), + - record-scoped tenant resolution (record == tenant, or record → tenant mapping), + - deterministic bulk preflight authorization with all-or-nothing semantics (authorization-only by default). +- 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. +- 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). + +## Technical Context + + + +**Language/Version**: PHP 8.4 (Laravel 12) +**Primary Dependencies**: Laravel 12 + Filament v5 + Livewire v4 +**Storage**: PostgreSQL (Sail) +**Testing**: Pest (PHPUnit) +**Target Platform**: Web application (Sail/Docker locally; container deploy via Dokploy) +**Project Type**: web +**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 render/hydration; no outbound HTTP during render; tenant isolation; hidden/disabled actions must not execute; destructive actions require confirmation +**Scale/Scope**: Suite-wide helper primitives + targeted migrations (Tier 1 + Tier 2 hitlist) rather than a full Filament sweep + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +- 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). +- Single contract path to Graph: PASS (helper + UI rendering remain DB-only; no Graph calls introduced). +- Deterministic capabilities: PASS (migrated surfaces reference `Capabilities::*` only; no ad-hoc ability strings). +- RBAC Standard: PASS (tenant plane only; platform plane out of scope; non-member is deny-as-not-found/404). +- Tenant isolation: PASS (record-scoped tenant resolution evaluates membership per tenant record; non-members receive 404 on direct access). +- Operations / run observability standard: N/A (no changes to OperationRun orchestration; only UI authorization enforcement). +- Automation (locks + idempotency): N/A (no new queued ops introduced). +- Data minimization & safe logging: PASS (no new persisted payloads; enforcement must not log secrets/PII). +- Badge semantics (BADGE-001): N/A. + +## Project Structure + +### Documentation (this feature) + +```text +specs/066-rbac-ui-enforcement-helper/ +├── plan.md # This file (/speckit.plan command output) +├── research.md # Phase 0 output (/speckit.plan command) +├── data-model.md # Phase 1 output (/speckit.plan command) +├── quickstart.md # Phase 1 output (/speckit.plan command) +├── contracts/ # Phase 1 output (/speckit.plan command) +└── tasks.md # Phase 2 output (/speckit.tasks command - NOT created by /speckit.plan) +``` + +### Source Code (repository root) + +```text +app/ +├── Filament/ +│ └── 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/ +├── Feature/ +│ ├── Filament/ +│ └── Guards/ +└── Unit/ + └── Auth/ +``` + +**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. + +## Complexity Tracking + +No constitution violations required for this feature. + +## 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). diff --git a/specs/066-rbac-ui-enforcement-helper/quickstart.md b/specs/066-rbac-ui-enforcement-helper/quickstart.md new file mode 100644 index 0000000..8f5839b --- /dev/null +++ b/specs/066-rbac-ui-enforcement-helper/quickstart.md @@ -0,0 +1,63 @@ +# Quickstart: 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` + +## Local setup + +1. Start the app stack: + - `./vendor/bin/sail up -d` +2. Install dependencies (if needed): + - `./vendor/bin/sail composer install` + +## Recipes (once implemented) + +> Scope: Tenant plane only (`/admin/t/{tenant}`). Platform plane (`/system`) is out of scope. + +### Tenant-scoped action (default tenant from Filament) + +- Use `UiEnforcement` with the required `Capabilities::*` constant. +- Non-members should never see the action (routing already denies them). +- Members without capability see a disabled action + standard tooltip. + +### Mixed visibility (business visibility + RBAC) + +- 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. + +### Record-scoped tenant (record == tenant) + +- 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. + +### Bulk actions (authorization-only all-or-nothing) + +- 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). + +## Notes on Filament execution semantics + +- 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 + +## Manual QA checklist (once implemented) + +- 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. + +## Test run (once implemented) + +- `./vendor/bin/sail bin pint --dirty` +- `./vendor/bin/sail artisan test tests/Feature/Guards --compact` +- `./vendor/bin/sail artisan test tests/Feature/Filament --compact` +- `./vendor/bin/sail artisan test tests/Feature/Rbac --compact` diff --git a/specs/066-rbac-ui-enforcement-helper/research.md b/specs/066-rbac-ui-enforcement-helper/research.md new file mode 100644 index 0000000..352f6a4 --- /dev/null +++ b/specs/066-rbac-ui-enforcement-helper/research.md @@ -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. + diff --git a/specs/066-rbac-ui-enforcement-helper/spec.md b/specs/066-rbac-ui-enforcement-helper/spec.md index 8226d05..2008257 100644 --- a/specs/066-rbac-ui-enforcement-helper/spec.md +++ b/specs/066-rbac-ui-enforcement-helper/spec.md @@ -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` -**Created**: 2026-01-28 +**Feature Branch**: `066-rbac-ui-enforcement-helper-v2` +**Created**: 2026-01-30 **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 -### 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: 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). +- 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. -### User Story 1 - Tenant member sees consistent disabled UX (Priority: P1) +## Problem Statement -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. +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: -**Why this priority**: Prevents confusion and reduces support load while keeping the UI predictable for members. +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) -**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. +v2 makes the helper suite-ready by adding explicit primitives for these cases and migrating remaining high-risk surfaces. -**Acceptance Scenarios**: +## Goals -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. -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. +### G1 — Suite-wide RBAC UX consistency ---- +A tenant member sees a consistent UI pattern everywhere: -### User Story 2 - Non-members cannot infer tenant resources (Priority: P2) +- **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 -As a non-member of a tenant, I cannot discover tenant-scoped resources or actions; the system responds as “not found”. +### G2 — Eliminate remaining ad-hoc RBAC in Filament -**Why this priority**: Prevents tenant enumeration and cross-tenant information leakage. +Remove `Gate::...`, `abort_*`, policy ability string literals, and custom RBAC patterns from remaining targeted Filament surfaces, replacing them with `UiEnforcement` and canonical `Capabilities::*`. -**Independent Test**: Can be tested by attempting to access tenant-scoped pages/actions as a user without membership and verifying 404 behavior. +### G3 — Support mixed visibility and record-scoped tenancy -**Acceptance Scenarios**: +Provide first-class APIs so teams can migrate without special casing per resource. -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). +### G4 — Keep enforcement testable and stable ---- +Guard remains stable (file scan), allowlist shrinks, and we add targeted integration tests for each migrated surface. -### User Story 3 - Maintainers add actions safely by default (Priority: P3) +## Non-Goals -As a maintainer, I can add new tenant-scoped actions using one standard pattern, and regression guards prevent introducing ad-hoc authorization logic. +- 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. -**Why this priority**: Reduces RBAC regressions as the app grows and makes reviews easier. +## Terms -**Independent Test**: Can be tested by introducing a sample ad-hoc authorization pattern and confirming automated checks/tests flag it. +- **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. -**Acceptance Scenarios**: +## RBAC-UX Contract (v2) -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. -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. +This feature follows the RBAC constitution rules and clarifies Filament execution reality. ---- +### 5.1 Non-member -[Add more user stories as needed, each with an assigned priority] +- **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. -### Edge Cases +**Note**: Filament treats hidden as non-executable (silent no-op). This is acceptable and is the enforced behavior. - +### 5.2 Member without capability -- Membership is revoked while the user has the page open (execution must still enforce 404 semantics). -- 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). +- 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.” -## Requirements *(mandatory)* +### 5.3 Member with capability -**Constitution alignment (required):** If this feature introduces any Microsoft Graph calls, any write/change behavior, -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. +- Enabled and executes. +- Any destructive action MUST require confirmation (`->requiresConfirmation()`). -**Constitution alignment (RBAC-UX):** This feature defines a default pattern for tenant-plane admin actions. The implementation MUST: -- enforce membership as an isolation boundary (non-member / not entitled → 404 deny-as-not-found), -- 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. +### 5.4 Bulk actions (default) -**Constitution alignment (OPS-EX-AUTH-001):** OIDC/SAML login handshakes may perform synchronous outbound HTTP (e.g., token exchange) -on `/auth/*` endpoints without an `OperationRun`. This MUST NOT be used for Monitoring/Operations pages. +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. -**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. - - +## 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-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. -- **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. -- **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. +- **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. -### 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. -- **User**: The authenticated actor attempting to view or execute actions. -- **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. +- **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). -### 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). -- Non-destructive bulk actions are in scope for v1; destructive bulk actions may be supported but are not required for v1 completion. -- Global search tenant scoping is out of scope for this spec (covered by separate work), but this feature must not introduce new leaks. +- **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. -## 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: + - 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 all migrated tenant-scoped action surfaces, 100% of non-member execution attempts are denied with 404 semantics (verified by automated tests). -- **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-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-004**: At least one automated guard exists that flags newly introduced ad-hoc authorization patterns in tenant-scoped admin UI code. -- **SC-005**: v1 demonstrates adoption by migrating 3–6 exemplar action surfaces, reducing duplicate authorization wiring in those areas. +- **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). diff --git a/specs/066-rbac-ui-enforcement-helper/tasks.md b/specs/066-rbac-ui-enforcement-helper/tasks.md new file mode 100644 index 0000000..f29b9a2 --- /dev/null +++ b/specs/066-rbac-ui-enforcement-helper/tasks.md @@ -0,0 +1,215 @@ +--- + +description: "Task list for RBAC UI Enforcement Helper v2" + +--- + +# Tasks: RBAC UI Enforcement Helper v2 (Suite-wide, Mixed Visibility, Record-Scoped) + +**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` + +**Tests**: REQUIRED (Pest) for runtime behavior changes +**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 +**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) + +**Purpose**: Shared helper + guardrails required by all story migrations + +**Checkpoint**: `UiEnforcement` supports mixed visibility, record-scoped tenant resolution, and bulk preflight; guard exists and is allowlist-driven + +- [X] T004 Create `UiTooltips` with the v2 default disabled tooltip copy in `app/Support/Auth/UiTooltips.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 [P] Add mixed visibility APIs (`preserveVisibility()` tenant-scoped only, `andVisibleWhen()`, `andHiddenWhen()`) in `app/Support/Auth/UiEnforcement.php` +- [X] T007 [P] Add tenant resolver APIs (`tenantFromFilament()`, `tenantFromRecord()`, `tenantFrom(callable)`) in `app/Support/Auth/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 [P] Add helper unit tests (mixed visibility restriction + tenant resolvers + preflight decisions) in `tests/Unit/Auth/UiEnforcementTest.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 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`) + +--- + +## Phase 3: User Story 1 — Mixed Visibility (Backup/Restore) (Priority: P1) 🎯 MVP + +**Goal**: Backup/Restore actions preserve business visibility while enforcing RBAC UX (hidden/disabled + standard tooltip) via `UiEnforcement` + +**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 + +- [X] T013 [P] [US1] Add BackupSet RBAC UX integration tests (include `Http::preventStrayRequests()`) in `tests/Feature/Filament/BackupSetUiEnforcementTest.php` +- [X] T014 [P] [US1] Add RestoreRun RBAC UX integration tests (include `Http::preventStrayRequests()`) in `tests/Feature/Filament/RestoreRunUiEnforcementTest.php` + +### Implementation for User Story 1 + +- [X] T015 [US1] Migrate mixed-visibility actions + bulk actions to `UiEnforcement` in `app/Filament/Resources/BackupSetResource.php` +- [X] T016 [US1] Migrate relation manager actions (incl. standardized tooltip copy via `UiTooltips`) in `app/Filament/Resources/BackupSetResource/RelationManagers/BackupItemsRelationManager.php` +- [X] T017 [US1] Migrate mixed-visibility actions + bulk actions to `UiEnforcement` in `app/Filament/Resources/RestoreRunResource.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` +- [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 — Record-Scoped Tenant (TenantResource actions) (Priority: P1) + +**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**: 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 + +- [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] T022 [P] [US2] Add record-scoped non-member 404 tests for tenant view/edit routes in `tests/Feature/Filament/TenantPortfolioContextSwitchTest.php` + +### Implementation for User Story 2 + +- [X] T023 [US2] Migrate TenantResource row actions to `UiEnforcement` using `tenantFromRecord()` (remove ad-hoc `Gate`/`abort_*`) in `app/Filament/Resources/TenantResource.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] T025 [US2] Remove allowlist entries for migrated TenantResource files in `tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.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`) + +--- + +## Phase 5: User Story 3 — Bulk Preflight (All-or-Nothing Authorization) (Priority: P2) + +**Goal**: Bulk actions disable deterministically when selection contains any unauthorized record; no partial execution + +**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 + +- [X] T027 [P] [US3] Update bulk sync tests to assert all-or-nothing authorization disable (include `Http::preventStrayRequests()`) in `tests/Feature/Filament/TenantPortfolioContextSwitchTest.php` + +### Implementation for User Story 3 + +- [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] 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] Run story tests with `./vendor/bin/sail artisan test tests/Feature/Filament/TenantPortfolioContextSwitchTest.php --compact` (test: `tests/Feature/Filament/TenantPortfolioContextSwitchTest.php`) + +--- + +## Phase 6: User Story 4 — Guard Allowlist Shrink + Tier 2 Migrations (Priority: P2) + +**Goal**: CI fails for new ad-hoc auth patterns in `app/Filament/**`; allowlist count shrinks measurably by migrating remaining targets + +**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 + +### Tests for User Story 4 + +- [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: Polish & Cross-Cutting Concerns + +**Purpose**: Reduce regressions and ensure suite-wide consistency + +- [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] T041 Run formatting with `./vendor/bin/sail bin pint --dirty` (script: `./vendor/bin/pint`) +- [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`) + +--- + +## Dependencies & Execution Order + +### User Story Dependency Graph + +```text +Phase 1 (Setup) + ↓ +Phase 2 (Foundation: UiEnforcement v2 + guard baseline) + ↓ +US1 (Backup/Restore mixed visibility) ─┬─→ US4 (Tier 2 migrations + allowlist shrink) + ├─→ US2 (TenantResource record-scoped actions) + └─→ US3 (Bulk preflight all-or-nothing) +``` + +### Parallel Opportunities + +- Phase 2 tasks marked `[P]` can run in parallel (different files). +- 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`. +- Within US4, Inventory vs EntraGroup vs ProviderConnection migrations can be parallelized (different files), but coordinate allowlist edits in `tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php`. + +--- + +## Parallel Example: User Story 1 + +```bash +Task: "Add BackupSet RBAC UX integration tests in tests/Feature/Filament/BackupSetUiEnforcementTest.php" +Task: "Add RestoreRun RBAC UX integration tests in tests/Feature/Filament/RestoreRunUiEnforcementTest.php" +Task: "Migrate BackupSet mixed-visibility actions to UiEnforcement in app/Filament/Resources/BackupSetResource.php" +Task: "Migrate RestoreRun mixed-visibility actions to UiEnforcement in app/Filament/Resources/RestoreRunResource.php" +``` + +--- + +## 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 + +### MVP First (User Story 1) + +1. Complete Phase 1 + Phase 2 +2. Complete US1 (BackupSet + RestoreRun mixed visibility enforcement) +3. Validate with `tests/Feature/Filament/BackupSetUiEnforcementTest.php` and `tests/Feature/Filament/RestoreRunUiEnforcementTest.php` + +### Incremental Delivery + +1. US1 → migrate highest-risk backup/restore surfaces + tests + allowlist reduction +2. US2 → record-scoped tenant actions enforced per row + 404 non-member on direct access +3. US3 → bulk preflight all-or-nothing authorization +4. US4 → Tier 2 migrations + measurable allowlist shrink + guard stability