From e5ad9b6cf8f8c05487fe37d5b256c008e8c732d9 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Wed, 28 Jan 2026 23:10:49 +0100 Subject: [PATCH 1/5] spec: 066 rbac ui enforcement helper v1 --- .../checklists/requirements.md | 34 ++++ specs/066-rbac-ui-enforcement-helper/spec.md | 153 ++++++++++++++++++ 2 files changed, 187 insertions(+) create mode 100644 specs/066-rbac-ui-enforcement-helper/checklists/requirements.md create mode 100644 specs/066-rbac-ui-enforcement-helper/spec.md diff --git a/specs/066-rbac-ui-enforcement-helper/checklists/requirements.md b/specs/066-rbac-ui-enforcement-helper/checklists/requirements.md new file mode 100644 index 0000000..91ef52d --- /dev/null +++ b/specs/066-rbac-ui-enforcement-helper/checklists/requirements.md @@ -0,0 +1,34 @@ +# Specification Quality Checklist: RBAC UI Enforcement Helper v1 + +**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) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- No blockers found in this iteration. diff --git a/specs/066-rbac-ui-enforcement-helper/spec.md b/specs/066-rbac-ui-enforcement-helper/spec.md new file mode 100644 index 0000000..0015978 --- /dev/null +++ b/specs/066-rbac-ui-enforcement-helper/spec.md @@ -0,0 +1,153 @@ +# Feature Specification: RBAC UI Enforcement Helper v1 + +**Feature Branch**: `066-rbac-ui-enforcement-helper` +**Created**: 2026-01-28 +**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. + +## User Scenarios & Testing *(mandatory)* + + + +### User Story 1 - Tenant member sees consistent disabled UX (Priority: P1) + +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. + +**Why this priority**: Prevents confusion and reduces support load while keeping the UI predictable for members. + +**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. + +**Acceptance Scenarios**: + +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. + +--- + +### User Story 2 - Non-members cannot infer tenant resources (Priority: P2) + +As a non-member of a tenant, I cannot discover tenant-scoped resources or actions; the system responds as “not found”. + +**Why this priority**: Prevents tenant enumeration and cross-tenant information leakage. + +**Independent Test**: Can be tested by attempting to access tenant-scoped pages/actions as a user without membership and verifying 404 behavior. + +**Acceptance Scenarios**: + +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). + +--- + +### User Story 3 - Maintainers add actions safely by default (Priority: P3) + +As a maintainer, I can add new tenant-scoped actions using one standard pattern, and regression guards prevent introducing ad-hoc authorization logic. + +**Why this priority**: Reduces RBAC regressions as the app grows and makes reviews easier. + +**Independent Test**: Can be tested by introducing a sample ad-hoc authorization pattern and confirming automated checks/tests flag it. + +**Acceptance Scenarios**: + +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. + +--- + +[Add more user stories as needed, each with an assigned priority] + +### Edge Cases + + + +- 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 (the safe default behavior is enforced consistently). +- Target record is deleted/archived between render and execution (no information leakage in errors). + +## Requirements *(mandatory)* + +**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. + +**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. + +**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. + +**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. + + + +### 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-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-011**: v1 SHOULD include an automated 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)* + +- **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. + +### Assumptions + +- 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. + +## Success Criteria *(mandatory)* + + + +### 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. -- 2.45.2 From 07dda36d6ec8fd73ad0019cf0936dad456a95cd4 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Wed, 28 Jan 2026 23:22:54 +0100 Subject: [PATCH 2/5] spec: clarify 066 rbac ui enforcement defaults --- specs/066-rbac-ui-enforcement-helper/spec.md | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/specs/066-rbac-ui-enforcement-helper/spec.md b/specs/066-rbac-ui-enforcement-helper/spec.md index 0015978..8226d05 100644 --- a/specs/066-rbac-ui-enforcement-helper/spec.md +++ b/specs/066-rbac-ui-enforcement-helper/spec.md @@ -5,6 +5,14 @@ # Feature Specification: RBAC UI Enforcement Helper v1 **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. +## Clarifications + +### Session 2026-01-28 + +- 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: 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)* + +**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 -- 2.45.2 From 95ccc3008c112313f49d389332d989b22077a6ed Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Fri, 30 Jan 2026 17:51:24 +0100 Subject: [PATCH 4/5] feat: enforce Filament RBAC via UiEnforcement v2 --- Agents.md | 8 +- app/Filament/Resources/BackupSetResource.php | 563 +++++++++--------- .../Pages/ListBackupSets.php | 6 +- .../BackupItemsRelationManager.php | 282 ++++----- .../Pages/ListEntraGroups.php | 74 +-- .../Pages/ListEntraGroupSyncRuns.php | 58 +- .../Resources/InventoryItemResource.php | 12 +- .../Pages/ListInventoryItems.php | 212 +++---- .../Resources/InventorySyncRunResource.php | 12 +- .../Resources/ProviderConnectionResource.php | 213 ++----- .../Pages/EditProviderConnection.php | 390 ++++++------ .../Pages/ListProviderConnections.php | 10 +- app/Filament/Resources/RestoreRunResource.php | 99 +-- .../Pages/CreateRestoreRun.php | 6 +- .../Pages/ListRestoreRuns.php | 6 +- app/Filament/Resources/TenantResource.php | 438 +++++--------- .../TenantResource/Pages/EditTenant.php | 53 +- app/Support/Auth/UiEnforcement.php | 526 ++++++++++++++++ app/Support/Auth/UiTooltips.php | 14 + .../Filament/BackupItemsBulkRemoveTest.php | 2 + .../Filament/BackupSetUiEnforcementTest.php | 71 +++ .../EntraGroupSyncRunResourceTest.php | 25 + .../Filament/InventoryItemResourceTest.php | 34 ++ .../Filament/InventorySyncRunResourceTest.php | 16 + .../ProviderConnectionsUiEnforcementTest.php | 77 +++ .../Filament/RestoreRunUiEnforcementTest.php | 83 +++ .../TenantActionsAuthorizationTest.php | 18 +- .../TenantPortfolioContextSwitchTest.php | 88 ++- .../NoAdHocFilamentAuthPatternsTest.php | 118 ++++ ...EnforcementBulkPreflightQueryCountTest.php | 36 ++ tests/Unit/Auth/UiEnforcementTest.php | 128 ++++ 31 files changed, 2196 insertions(+), 1482 deletions(-) create mode 100644 app/Support/Auth/UiEnforcement.php create mode 100644 app/Support/Auth/UiTooltips.php create mode 100644 tests/Feature/Filament/BackupSetUiEnforcementTest.php create mode 100644 tests/Feature/Filament/ProviderConnectionsUiEnforcementTest.php create mode 100644 tests/Feature/Filament/RestoreRunUiEnforcementTest.php create mode 100644 tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php create mode 100644 tests/Unit/Auth/UiEnforcementBulkPreflightQueryCountTest.php create mode 100644 tests/Unit/Auth/UiEnforcementTest.php diff --git a/Agents.md b/Agents.md index 7d3d8d6..c546c58 100644 --- a/Agents.md +++ b/Agents.md @@ -1056,9 +1056,9 @@ ### Replaced Utilities ## Active Technologies -- PHP 8.4.15 (Laravel 12) + Filament v4, Livewire v3 (054-unify-runs-suitewide-session-1768601416) -- PostgreSQL (`operation_runs` + JSONB for summary/failures/context; partial unique index for active-run dedupe) (054-unify-runs-suitewide-session-1768601416) -- PHP 8.4.15 (Laravel 12) + Filament v5 + Livewire v4 (059-unified-badges) +- PHP 8.4 (Laravel 12) + Filament v5 + Livewire v4 +- PostgreSQL (Sail) +- Tailwind CSS v4 ## Recent Changes -- 054-unify-runs-suitewide-session-1768601416: Added PHP 8.4.15 (Laravel 12) + Filament v4, Livewire v3 +- 066-rbac-ui-enforcement-helper-v2-session-1769732329: Planned UiEnforcement v2 (spec + plan + design artifacts) diff --git a/app/Filament/Resources/BackupSetResource.php b/app/Filament/Resources/BackupSetResource.php index ecb62ff..068f781 100644 --- a/app/Filament/Resources/BackupSetResource.php +++ b/app/Filament/Resources/BackupSetResource.php @@ -15,6 +15,7 @@ use App\Services\OperationRunService; use App\Services\Operations\BulkSelectionIdentity; use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use App\Support\Badges\BadgeDomain; use App\Support\Badges\BadgeRenderer; use App\Support\OperationRunLinks; @@ -34,7 +35,6 @@ use Filament\Tables\Filters\TrashedFilter; use Filament\Tables\Table; use Illuminate\Database\Eloquent\Collection; -use Illuminate\Support\Facades\Gate; use UnitEnum; class BackupSetResource extends Resource @@ -47,8 +47,7 @@ class BackupSetResource extends Resource public static function canCreate(): bool { - return ($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_SYNC, $tenant); + return UiEnforcement::for(Capabilities::TENANT_SYNC)->isAllowed(); } public static function form(Schema $schema): Schema @@ -90,123 +89,116 @@ public static function table(Table $table): Table ->url(fn (BackupSet $record) => static::getUrl('view', ['record' => $record])) ->openUrlInNewTab(false), ActionGroup::make([ - Actions\Action::make('restore') - ->label('Restore') - ->color('success') - ->icon('heroicon-o-arrow-uturn-left') - ->requiresConfirmation() - ->visible(fn (BackupSet $record): bool => $record->trashed()) - ->disabled(fn (): bool => ! (($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_MANAGE, $tenant))) - ->action(function (BackupSet $record, AuditLogger $auditLogger) { - $tenant = Tenant::current(); + UiEnforcement::for(Capabilities::TENANT_MANAGE) + ->andVisibleWhen(fn (?BackupSet $record): bool => $record?->trashed() ?? false) + ->apply( + Actions\Action::make('restore') + ->label('Restore') + ->color('success') + ->icon('heroicon-o-arrow-uturn-left') + ->requiresConfirmation() + ->action(function (BackupSet $record, AuditLogger $auditLogger) { + UiEnforcement::for(Capabilities::TENANT_MANAGE)->authorizeOrAbort(); - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::TENANT_MANAGE, $tenant), 403); + $record->restore(); + $record->items()->withTrashed()->restore(); - $record->restore(); - $record->items()->withTrashed()->restore(); + if ($record->tenant) { + $auditLogger->log( + tenant: $record->tenant, + action: 'backup.restored', + resourceType: 'backup_set', + resourceId: (string) $record->id, + status: 'success', + context: ['metadata' => ['name' => $record->name]] + ); + } - if ($record->tenant) { - $auditLogger->log( - tenant: $record->tenant, - action: 'backup.restored', - resourceType: 'backup_set', - resourceId: (string) $record->id, - status: 'success', - context: ['metadata' => ['name' => $record->name]] - ); - } + Notification::make() + ->title('Backup set restored') + ->success() + ->send(); + }), + ), - Notification::make() - ->title('Backup set restored') - ->success() - ->send(); - }), - Actions\Action::make('archive') - ->label('Archive') - ->color('danger') - ->icon('heroicon-o-archive-box-x-mark') - ->requiresConfirmation() - ->visible(fn (BackupSet $record): bool => ! $record->trashed()) - ->disabled(fn (): bool => ! (($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_MANAGE, $tenant))) - ->action(function (BackupSet $record, AuditLogger $auditLogger) { - $tenant = Tenant::current(); + UiEnforcement::for(Capabilities::TENANT_MANAGE) + ->andVisibleWhen(fn (?BackupSet $record): bool => $record ? ! $record->trashed() : false) + ->apply( + Actions\Action::make('archive') + ->label('Archive') + ->color('danger') + ->icon('heroicon-o-archive-box-x-mark') + ->requiresConfirmation() + ->action(function (BackupSet $record, AuditLogger $auditLogger) { + UiEnforcement::for(Capabilities::TENANT_MANAGE)->authorizeOrAbort(); - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::TENANT_MANAGE, $tenant), 403); + $record->delete(); - $record->delete(); + if ($record->tenant) { + $auditLogger->log( + tenant: $record->tenant, + action: 'backup.deleted', + resourceType: 'backup_set', + resourceId: (string) $record->id, + status: 'success', + context: ['metadata' => ['name' => $record->name]] + ); + } - if ($record->tenant) { - $auditLogger->log( - tenant: $record->tenant, - action: 'backup.deleted', - resourceType: 'backup_set', - resourceId: (string) $record->id, - status: 'success', - context: ['metadata' => ['name' => $record->name]] - ); - } + Notification::make() + ->title('Backup set archived') + ->success() + ->send(); + }), + ), - Notification::make() - ->title('Backup set archived') - ->success() - ->send(); - }), - Actions\Action::make('forceDelete') - ->label('Force delete') - ->color('danger') - ->icon('heroicon-o-trash') - ->requiresConfirmation() - ->visible(fn (BackupSet $record): bool => $record->trashed()) - ->disabled(fn (): bool => ! (($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_DELETE, $tenant))) - ->action(function (BackupSet $record, AuditLogger $auditLogger) { - $tenant = Tenant::current(); + UiEnforcement::for(Capabilities::TENANT_DELETE) + ->andVisibleWhen(fn (?BackupSet $record): bool => $record?->trashed() ?? false) + ->apply( + Actions\Action::make('forceDelete') + ->label('Force delete') + ->color('danger') + ->icon('heroicon-o-trash') + ->requiresConfirmation() + ->action(function (BackupSet $record, AuditLogger $auditLogger) { + UiEnforcement::for(Capabilities::TENANT_DELETE)->authorizeOrAbort(); - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::TENANT_DELETE, $tenant), 403); + if ($record->restoreRuns()->withTrashed()->exists()) { + Notification::make() + ->title('Cannot force delete backup set') + ->body('Backup sets referenced by restore runs cannot be removed.') + ->danger() + ->send(); - if ($record->restoreRuns()->withTrashed()->exists()) { - Notification::make() - ->title('Cannot force delete backup set') - ->body('Backup sets referenced by restore runs cannot be removed.') - ->danger() - ->send(); + return; + } - return; - } + if ($record->tenant) { + $auditLogger->log( + tenant: $record->tenant, + action: 'backup.force_deleted', + resourceType: 'backup_set', + resourceId: (string) $record->id, + status: 'success', + context: ['metadata' => ['name' => $record->name]] + ); + } - if ($record->tenant) { - $auditLogger->log( - tenant: $record->tenant, - action: 'backup.force_deleted', - resourceType: 'backup_set', - resourceId: (string) $record->id, - status: 'success', - context: ['metadata' => ['name' => $record->name]] - ); - } + $record->items()->withTrashed()->forceDelete(); + $record->forceDelete(); - $record->items()->withTrashed()->forceDelete(); - $record->forceDelete(); - - Notification::make() - ->title('Backup set permanently deleted') - ->success() - ->send(); - }), + Notification::make() + ->title('Backup set permanently deleted') + ->success() + ->send(); + }), + ), ])->icon('heroicon-o-ellipsis-vertical'), ]) ->bulkActions([ BulkActionGroup::make([ - BulkAction::make('bulk_delete') - ->label('Archive Backup Sets') - ->icon('heroicon-o-archive-box-x-mark') - ->color('danger') - ->requiresConfirmation() - ->disabled(fn (): bool => ! (($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_MANAGE, $tenant))) - ->hidden(function (HasTable $livewire): bool { + UiEnforcement::for(Capabilities::TENANT_MANAGE) + ->andHiddenWhen(function (HasTable $livewire): bool { $trashedFilterState = $livewire->getTableFilterState(TrashedFilter::class) ?? []; $value = $trashedFilterState['value'] ?? null; @@ -214,83 +206,80 @@ public static function table(Table $table): Table return $isOnlyTrashed; }) - ->modalDescription('This archives backup sets (soft delete). Already archived backup sets will be skipped.') - ->form(function (Collection $records) { - if ($records->count() >= 10) { - return [ - Forms\Components\TextInput::make('confirmation') - ->label('Type DELETE to confirm') - ->required() - ->in(['DELETE']) - ->validationMessages([ - 'in' => 'Please type DELETE to confirm.', - ]), - ]; - } + ->apply( + BulkAction::make('bulk_delete') + ->label('Archive Backup Sets') + ->icon('heroicon-o-archive-box-x-mark') + ->color('danger') + ->requiresConfirmation() + ->modalDescription('This archives backup sets (soft delete). Already archived backup sets will be skipped.') + ->form(function (Collection $records) { + if ($records->count() >= 10) { + return [ + Forms\Components\TextInput::make('confirmation') + ->label('Type DELETE to confirm') + ->required() + ->in(['DELETE']) + ->validationMessages([ + 'in' => 'Please type DELETE to confirm.', + ]), + ]; + } - return []; - }) - ->action(function (Collection $records) { - $tenant = Tenant::current(); - $user = auth()->user(); - $count = $records->count(); - $ids = $records->pluck('id')->toArray(); + return []; + }) + ->action(function (Collection $records) { + UiEnforcement::for(Capabilities::TENANT_MANAGE)->authorizeOrAbort(); - if (! $tenant instanceof Tenant) { - return; - } + $tenant = Tenant::current(); + $user = auth()->user(); + $count = $records->count(); + $ids = $records->pluck('id')->toArray(); - abort_unless(Gate::allows(Capabilities::TENANT_MANAGE, $tenant), 403); + $initiator = $user instanceof User ? $user : null; - $initiator = $user instanceof User ? $user : null; + /** @var BulkSelectionIdentity $selection */ + $selection = app(BulkSelectionIdentity::class); + $selectionIdentity = $selection->fromIds($ids); - /** @var BulkSelectionIdentity $selection */ - $selection = app(BulkSelectionIdentity::class); - $selectionIdentity = $selection->fromIds($ids); + /** @var OperationRunService $runs */ + $runs = app(OperationRunService::class); - /** @var OperationRunService $runs */ - $runs = app(OperationRunService::class); - - $opRun = $runs->enqueueBulkOperation( - tenant: $tenant, - type: 'backup_set.delete', - targetScope: [ - 'entra_tenant_id' => (string) ($tenant->tenant_id ?? $tenant->external_id), - ], - selectionIdentity: $selectionIdentity, - dispatcher: function ($operationRun) use ($tenant, $initiator, $ids): void { - BulkBackupSetDeleteJob::dispatch( - tenantId: (int) $tenant->getKey(), - userId: (int) ($initiator?->getKey() ?? 0), - backupSetIds: $ids, - operationRun: $operationRun, + $opRun = $runs->enqueueBulkOperation( + tenant: $tenant, + type: 'backup_set.delete', + targetScope: [ + 'entra_tenant_id' => (string) ($tenant->tenant_id ?? $tenant->external_id), + ], + selectionIdentity: $selectionIdentity, + dispatcher: function ($operationRun) use ($tenant, $initiator, $ids): void { + BulkBackupSetDeleteJob::dispatch( + tenantId: (int) $tenant->getKey(), + userId: (int) ($initiator?->getKey() ?? 0), + backupSetIds: $ids, + operationRun: $operationRun, + ); + }, + initiator: $initiator, + extraContext: [ + 'backup_set_count' => $count, + ], + emitQueuedNotification: false, ); - }, - initiator: $initiator, - extraContext: [ - 'backup_set_count' => $count, - ], - emitQueuedNotification: false, - ); - OperationUxPresenter::queuedToast('backup_set.delete') - ->actions([ - Actions\Action::make('view_run') - ->label('View run') - ->url(OperationRunLinks::view($opRun, $tenant)), - ]) - ->send(); - }) - ->deselectRecordsAfterCompletion(), + OperationUxPresenter::queuedToast('backup_set.delete') + ->actions([ + Actions\Action::make('view_run') + ->label('View run') + ->url(OperationRunLinks::view($opRun, $tenant)), + ]) + ->send(); + }) + ->deselectRecordsAfterCompletion(), + ), - BulkAction::make('bulk_restore') - ->label('Restore Backup Sets') - ->icon('heroicon-o-arrow-uturn-left') - ->color('success') - ->requiresConfirmation() - ->disabled(fn (): bool => ! (($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_MANAGE, $tenant))) - ->hidden(function (HasTable $livewire): bool { + UiEnforcement::for(Capabilities::TENANT_MANAGE) + ->andHiddenWhen(function (HasTable $livewire): bool { $trashedFilterState = $livewire->getTableFilterState(TrashedFilter::class) ?? []; $value = $trashedFilterState['value'] ?? null; @@ -298,69 +287,66 @@ public static function table(Table $table): Table return ! $isOnlyTrashed; }) - ->modalHeading(fn (Collection $records) => "Restore {$records->count()} backup sets?") - ->modalDescription('Archived backup sets will be restored back to the active list. Active backup sets will be skipped.') - ->action(function (Collection $records) { - $tenant = Tenant::current(); - $user = auth()->user(); - $count = $records->count(); - $ids = $records->pluck('id')->toArray(); + ->apply( + BulkAction::make('bulk_restore') + ->label('Restore Backup Sets') + ->icon('heroicon-o-arrow-uturn-left') + ->color('success') + ->requiresConfirmation() + ->modalHeading(fn (Collection $records) => "Restore {$records->count()} backup sets?") + ->modalDescription('Archived backup sets will be restored back to the active list. Active backup sets will be skipped.') + ->action(function (Collection $records) { + UiEnforcement::for(Capabilities::TENANT_MANAGE)->authorizeOrAbort(); - if (! $tenant instanceof Tenant) { - return; - } + $tenant = Tenant::current(); + $user = auth()->user(); + $count = $records->count(); + $ids = $records->pluck('id')->toArray(); - abort_unless(Gate::allows(Capabilities::TENANT_MANAGE, $tenant), 403); + $initiator = $user instanceof User ? $user : null; - $initiator = $user instanceof User ? $user : null; + /** @var BulkSelectionIdentity $selection */ + $selection = app(BulkSelectionIdentity::class); + $selectionIdentity = $selection->fromIds($ids); - /** @var BulkSelectionIdentity $selection */ - $selection = app(BulkSelectionIdentity::class); - $selectionIdentity = $selection->fromIds($ids); + /** @var OperationRunService $runs */ + $runs = app(OperationRunService::class); - /** @var OperationRunService $runs */ - $runs = app(OperationRunService::class); - - $opRun = $runs->enqueueBulkOperation( - tenant: $tenant, - type: 'backup_set.restore', - targetScope: [ - 'entra_tenant_id' => (string) ($tenant->tenant_id ?? $tenant->external_id), - ], - selectionIdentity: $selectionIdentity, - dispatcher: function ($operationRun) use ($tenant, $initiator, $ids): void { - BulkBackupSetRestoreJob::dispatch( - tenantId: (int) $tenant->getKey(), - userId: (int) ($initiator?->getKey() ?? 0), - backupSetIds: $ids, - operationRun: $operationRun, + $opRun = $runs->enqueueBulkOperation( + tenant: $tenant, + type: 'backup_set.restore', + targetScope: [ + 'entra_tenant_id' => (string) ($tenant->tenant_id ?? $tenant->external_id), + ], + selectionIdentity: $selectionIdentity, + dispatcher: function ($operationRun) use ($tenant, $initiator, $ids): void { + BulkBackupSetRestoreJob::dispatch( + tenantId: (int) $tenant->getKey(), + userId: (int) ($initiator?->getKey() ?? 0), + backupSetIds: $ids, + operationRun: $operationRun, + ); + }, + initiator: $initiator, + extraContext: [ + 'backup_set_count' => $count, + ], + emitQueuedNotification: false, ); - }, - initiator: $initiator, - extraContext: [ - 'backup_set_count' => $count, - ], - emitQueuedNotification: false, - ); - OperationUxPresenter::queuedToast('backup_set.restore') - ->actions([ - Actions\Action::make('view_run') - ->label('View run') - ->url(OperationRunLinks::view($opRun, $tenant)), - ]) - ->send(); - }) - ->deselectRecordsAfterCompletion(), + OperationUxPresenter::queuedToast('backup_set.restore') + ->actions([ + Actions\Action::make('view_run') + ->label('View run') + ->url(OperationRunLinks::view($opRun, $tenant)), + ]) + ->send(); + }) + ->deselectRecordsAfterCompletion(), + ), - BulkAction::make('bulk_force_delete') - ->label('Force Delete Backup Sets') - ->icon('heroicon-o-trash') - ->color('danger') - ->requiresConfirmation() - ->disabled(fn (): bool => ! (($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_DELETE, $tenant))) - ->hidden(function (HasTable $livewire): bool { + UiEnforcement::for(Capabilities::TENANT_DELETE) + ->andHiddenWhen(function (HasTable $livewire): bool { $trashedFilterState = $livewire->getTableFilterState(TrashedFilter::class) ?? []; $value = $trashedFilterState['value'] ?? null; @@ -368,75 +354,78 @@ public static function table(Table $table): Table return ! $isOnlyTrashed; }) - ->modalHeading(fn (Collection $records) => "Force delete {$records->count()} backup sets?") - ->modalDescription('This is permanent. Only archived backup sets will be permanently deleted; active backup sets will be skipped.') - ->form(function (Collection $records) { - if ($records->count() >= 10) { - return [ - Forms\Components\TextInput::make('confirmation') - ->label('Type DELETE to confirm') - ->required() - ->in(['DELETE']) - ->validationMessages([ - 'in' => 'Please type DELETE to confirm.', - ]), - ]; - } + ->apply( + BulkAction::make('bulk_force_delete') + ->label('Force Delete Backup Sets') + ->icon('heroicon-o-trash') + ->color('danger') + ->requiresConfirmation() + ->modalHeading(fn (Collection $records) => "Force delete {$records->count()} backup sets?") + ->modalDescription('This is permanent. Only archived backup sets will be permanently deleted; active backup sets will be skipped.') + ->form(function (Collection $records) { + if ($records->count() >= 10) { + return [ + Forms\Components\TextInput::make('confirmation') + ->label('Type DELETE to confirm') + ->required() + ->in(['DELETE']) + ->validationMessages([ + 'in' => 'Please type DELETE to confirm.', + ]), + ]; + } - return []; - }) - ->action(function (Collection $records) { - $tenant = Tenant::current(); - $user = auth()->user(); - $count = $records->count(); - $ids = $records->pluck('id')->toArray(); + return []; + }) + ->action(function (Collection $records) { + UiEnforcement::for(Capabilities::TENANT_DELETE)->authorizeOrAbort(); - if (! $tenant instanceof Tenant) { - return; - } + $tenant = Tenant::current(); + $user = auth()->user(); + $count = $records->count(); + $ids = $records->pluck('id')->toArray(); - abort_unless(Gate::allows(Capabilities::TENANT_DELETE, $tenant), 403); + $initiator = $user instanceof User ? $user : null; - $initiator = $user instanceof User ? $user : null; + /** @var BulkSelectionIdentity $selection */ + $selection = app(BulkSelectionIdentity::class); + $selectionIdentity = $selection->fromIds($ids); - /** @var BulkSelectionIdentity $selection */ - $selection = app(BulkSelectionIdentity::class); - $selectionIdentity = $selection->fromIds($ids); + /** @var OperationRunService $runs */ + $runs = app(OperationRunService::class); - /** @var OperationRunService $runs */ - $runs = app(OperationRunService::class); - - $opRun = $runs->enqueueBulkOperation( - tenant: $tenant, - type: 'backup_set.force_delete', - targetScope: [ - 'entra_tenant_id' => (string) ($tenant->tenant_id ?? $tenant->external_id), - ], - selectionIdentity: $selectionIdentity, - dispatcher: function ($operationRun) use ($tenant, $initiator, $ids): void { - BulkBackupSetForceDeleteJob::dispatch( - tenantId: (int) $tenant->getKey(), - userId: (int) ($initiator?->getKey() ?? 0), - backupSetIds: $ids, - operationRun: $operationRun, + $opRun = $runs->enqueueBulkOperation( + tenant: $tenant, + type: 'backup_set.force_delete', + targetScope: [ + 'entra_tenant_id' => (string) ($tenant->tenant_id ?? $tenant->external_id), + ], + selectionIdentity: $selectionIdentity, + dispatcher: function ($operationRun) use ($tenant, $initiator, $ids): void { + BulkBackupSetForceDeleteJob::dispatch( + tenantId: (int) $tenant->getKey(), + userId: (int) ($initiator?->getKey() ?? 0), + backupSetIds: $ids, + operationRun: $operationRun, + ); + }, + initiator: $initiator, + extraContext: [ + 'backup_set_count' => $count, + ], + emitQueuedNotification: false, ); - }, - initiator: $initiator, - extraContext: [ - 'backup_set_count' => $count, - ], - emitQueuedNotification: false, - ); - OperationUxPresenter::queuedToast('backup_set.force_delete') - ->actions([ - Actions\Action::make('view_run') - ->label('View run') - ->url(OperationRunLinks::view($opRun, $tenant)), - ]) - ->send(); - }) - ->deselectRecordsAfterCompletion(), + OperationUxPresenter::queuedToast('backup_set.force_delete') + ->actions([ + Actions\Action::make('view_run') + ->label('View run') + ->url(OperationRunLinks::view($opRun, $tenant)), + ]) + ->send(); + }) + ->deselectRecordsAfterCompletion(), + ), ]), ]); } diff --git a/app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php b/app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php index 893ed9f..afd21e5 100644 --- a/app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php +++ b/app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php @@ -3,6 +3,8 @@ namespace App\Filament\Resources\BackupSetResource\Pages; use App\Filament\Resources\BackupSetResource; +use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use Filament\Actions; use Filament\Resources\Pages\ListRecords; @@ -13,9 +15,7 @@ class ListBackupSets extends ListRecords protected function getHeaderActions(): array { return [ - Actions\CreateAction::make() - ->disabled(fn (): bool => ! BackupSetResource::canCreate()) - ->tooltip(fn (): ?string => BackupSetResource::canCreate() ? null : 'You do not have permission to create backup sets.'), + UiEnforcement::for(Capabilities::TENANT_SYNC)->apply(Actions\CreateAction::make()), ]; } } diff --git a/app/Filament/Resources/BackupSetResource/RelationManagers/BackupItemsRelationManager.php b/app/Filament/Resources/BackupSetResource/RelationManagers/BackupItemsRelationManager.php index 05b7486..737b890 100644 --- a/app/Filament/Resources/BackupSetResource/RelationManagers/BackupItemsRelationManager.php +++ b/app/Filament/Resources/BackupSetResource/RelationManagers/BackupItemsRelationManager.php @@ -9,6 +9,7 @@ use App\Models\User; use App\Services\OperationRunService; use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use App\Support\Badges\BadgeDomain; use App\Support\Badges\BadgeRenderer; use App\Support\Badges\TagBadgeDomain; @@ -24,7 +25,6 @@ use Illuminate\Contracts\View\View; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Collection; -use Illuminate\Support\Facades\Gate; class BackupItemsRelationManager extends RelationManager { @@ -131,23 +131,21 @@ public function table(Table $table): Table ->action(function (): void { $this->resetTable(); }), - Actions\Action::make('addPolicies') - ->label('Add Policies') - ->icon('heroicon-o-plus') - ->disabled(fn (): bool => ! (($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_SYNC, $tenant))) - ->tooltip(fn (): ?string => (($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_SYNC, $tenant)) ? null : 'You do not have permission to add policies.') - ->modalHeading('Add Policies') - ->modalSubmitAction(false) - ->modalCancelActionLabel('Close') - ->modalContent(function (): View { - $backupSet = $this->getOwnerRecord(); + UiEnforcement::for(Capabilities::TENANT_SYNC)->apply( + Actions\Action::make('addPolicies') + ->label('Add Policies') + ->icon('heroicon-o-plus') + ->modalHeading('Add Policies') + ->modalSubmitAction(false) + ->modalCancelActionLabel('Close') + ->modalContent(function (): View { + $backupSet = $this->getOwnerRecord(); - return view('filament.modals.backup-set-policy-picker', [ - 'backupSetId' => $backupSet->getKey(), - ]); - }), + return view('filament.modals.backup-set-policy-picker', [ + 'backupSetId' => $backupSet->getKey(), + ]); + }), + ), ]) ->actions([ Actions\ActionGroup::make([ @@ -164,174 +162,156 @@ public function table(Table $table): Table }) ->hidden(fn (BackupItem $record) => ! $record->policy_id) ->openUrlInNewTab(true), - Actions\Action::make('remove') - ->label('Remove') - ->color('danger') - ->icon('heroicon-o-x-mark') - ->requiresConfirmation() - ->action(function (BackupItem $record): void { - $backupSet = $this->getOwnerRecord(); + UiEnforcement::for(Capabilities::TENANT_SYNC)->apply( + Actions\Action::make('remove') + ->label('Remove') + ->color('danger') + ->icon('heroicon-o-x-mark') + ->requiresConfirmation() + ->action(function (BackupItem $record): void { + $backupSet = $this->getOwnerRecord(); + $tenant = $backupSet->tenant; - $user = auth()->user(); - if (! $user instanceof User) { - abort(403); - } + UiEnforcement::for(Capabilities::TENANT_SYNC) + ->tenantFromRecord() + ->authorizeOrAbort($tenant); - $tenant = $backupSet->tenant ?? Tenant::current(); + /** @var User $user */ + $user = auth()->user(); - if (! $tenant instanceof Tenant) { - abort(404); - } + $backupItemIds = [(int) $record->getKey()]; - if (! Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $tenant)) { - abort(403); - } + /** @var OperationRunService $opService */ + $opService = app(OperationRunService::class); + $opRun = $opService->ensureRun( + tenant: $tenant, + type: 'backup_set.remove_policies', + inputs: [ + 'backup_set_id' => (int) $backupSet->getKey(), + 'backup_item_ids' => $backupItemIds, + ], + initiator: $user, + ); - if ((int) $tenant->getKey() !== (int) $backupSet->tenant_id) { - abort(403); - } + if (! $opRun->wasRecentlyCreated && in_array($opRun->status, ['queued', 'running'], true)) { + Notification::make() + ->title('Removal already queued') + ->body('A matching remove operation is already queued or running.') + ->info() + ->actions([ + Actions\Action::make('view_run') + ->label('View run') + ->url(OperationRunLinks::view($opRun, $tenant)), + ]) + ->send(); - $backupItemIds = [(int) $record->getKey()]; + return; + } - /** @var OperationRunService $opService */ - $opService = app(OperationRunService::class); - $opRun = $opService->ensureRun( - tenant: $tenant, - type: 'backup_set.remove_policies', - inputs: [ - 'backup_set_id' => (int) $backupSet->getKey(), - 'backup_item_ids' => $backupItemIds, - ], - initiator: $user, - ); + $opService->dispatchOrFail($opRun, function () use ($backupSet, $backupItemIds, $user, $opRun): void { + RemovePoliciesFromBackupSetJob::dispatch( + backupSetId: (int) $backupSet->getKey(), + backupItemIds: $backupItemIds, + initiatorUserId: (int) $user->getKey(), + operationRun: $opRun, + ); + }); - if (! $opRun->wasRecentlyCreated && in_array($opRun->status, ['queued', 'running'], true)) { - Notification::make() - ->title('Removal already queued') - ->body('A matching remove operation is already queued or running.') - ->info() + OpsUxBrowserEvents::dispatchRunEnqueued($this); + OperationUxPresenter::queuedToast((string) $opRun->type) ->actions([ Actions\Action::make('view_run') ->label('View run') ->url(OperationRunLinks::view($opRun, $tenant)), ]) ->send(); - - return; - } - - $opService->dispatchOrFail($opRun, function () use ($backupSet, $backupItemIds, $user, $opRun): void { - RemovePoliciesFromBackupSetJob::dispatch( - backupSetId: (int) $backupSet->getKey(), - backupItemIds: $backupItemIds, - initiatorUserId: (int) $user->getKey(), - operationRun: $opRun, - ); - }); - - OpsUxBrowserEvents::dispatchRunEnqueued($this); - OperationUxPresenter::queuedToast((string) $opRun->type) - ->actions([ - Actions\Action::make('view_run') - ->label('View run') - ->url(OperationRunLinks::view($opRun, $tenant)), - ]) - ->send(); - }), + }), + ), ])->icon('heroicon-o-ellipsis-vertical'), ]) ->bulkActions([ Actions\BulkActionGroup::make([ - Actions\BulkAction::make('bulk_remove') - ->label('Remove selected') - ->icon('heroicon-o-x-mark') - ->color('danger') - ->requiresConfirmation() - ->deselectRecordsAfterCompletion() - ->action(function (Collection $records): void { - if ($records->isEmpty()) { - return; - } + UiEnforcement::for(Capabilities::TENANT_SYNC)->apply( + Actions\BulkAction::make('bulk_remove') + ->label('Remove selected') + ->icon('heroicon-o-x-mark') + ->color('danger') + ->requiresConfirmation() + ->deselectRecordsAfterCompletion() + ->action(function (Collection $records): void { + if ($records->isEmpty()) { + return; + } - $backupSet = $this->getOwnerRecord(); + $backupSet = $this->getOwnerRecord(); + $tenant = $backupSet->tenant; - $user = auth()->user(); - if (! $user instanceof User) { - abort(403); - } + UiEnforcement::for(Capabilities::TENANT_SYNC) + ->tenantFromRecord() + ->authorizeOrAbort($tenant); - $tenant = $backupSet->tenant ?? Tenant::current(); + /** @var User $user */ + $user = auth()->user(); - if (! $tenant instanceof Tenant) { - abort(404); - } + $backupItemIds = $records + ->pluck('id') + ->map(fn (mixed $value): int => (int) $value) + ->filter(fn (int $value): bool => $value > 0) + ->unique() + ->sort() + ->values() + ->all(); - if (! Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $tenant)) { - abort(403); - } + if ($backupItemIds === []) { + return; + } - if ((int) $tenant->getKey() !== (int) $backupSet->tenant_id) { - abort(403); - } + /** @var OperationRunService $opService */ + $opService = app(OperationRunService::class); + $opRun = $opService->ensureRun( + tenant: $tenant, + type: 'backup_set.remove_policies', + inputs: [ + 'backup_set_id' => (int) $backupSet->getKey(), + 'backup_item_ids' => $backupItemIds, + ], + initiator: $user, + ); - $backupItemIds = $records - ->pluck('id') - ->map(fn (mixed $value): int => (int) $value) - ->filter(fn (int $value): bool => $value > 0) - ->unique() - ->sort() - ->values() - ->all(); + if (! $opRun->wasRecentlyCreated && in_array($opRun->status, ['queued', 'running'], true)) { + Notification::make() + ->title('Removal already queued') + ->body('A matching remove operation is already queued or running.') + ->info() + ->actions([ + Actions\Action::make('view_run') + ->label('View run') + ->url(OperationRunLinks::view($opRun, $tenant)), + ]) + ->send(); - if ($backupItemIds === []) { - return; - } + return; + } - /** @var OperationRunService $opService */ - $opService = app(OperationRunService::class); - $opRun = $opService->ensureRun( - tenant: $tenant, - type: 'backup_set.remove_policies', - inputs: [ - 'backup_set_id' => (int) $backupSet->getKey(), - 'backup_item_ids' => $backupItemIds, - ], - initiator: $user, - ); + $opService->dispatchOrFail($opRun, function () use ($backupSet, $backupItemIds, $user, $opRun): void { + RemovePoliciesFromBackupSetJob::dispatch( + backupSetId: (int) $backupSet->getKey(), + backupItemIds: $backupItemIds, + initiatorUserId: (int) $user->getKey(), + operationRun: $opRun, + ); + }); - if (! $opRun->wasRecentlyCreated && in_array($opRun->status, ['queued', 'running'], true)) { - Notification::make() - ->title('Removal already queued') - ->body('A matching remove operation is already queued or running.') - ->info() + OpsUxBrowserEvents::dispatchRunEnqueued($this); + OperationUxPresenter::queuedToast((string) $opRun->type) ->actions([ Actions\Action::make('view_run') ->label('View run') ->url(OperationRunLinks::view($opRun, $tenant)), ]) ->send(); - - return; - } - - $opService->dispatchOrFail($opRun, function () use ($backupSet, $backupItemIds, $user, $opRun): void { - RemovePoliciesFromBackupSetJob::dispatch( - backupSetId: (int) $backupSet->getKey(), - backupItemIds: $backupItemIds, - initiatorUserId: (int) $user->getKey(), - operationRun: $opRun, - ); - }); - - OpsUxBrowserEvents::dispatchRunEnqueued($this); - OperationUxPresenter::queuedToast((string) $opRun->type) - ->actions([ - Actions\Action::make('view_run') - ->label('View run') - ->url(OperationRunLinks::view($opRun, $tenant)), - ]) - ->send(); - }), + }), + ), ]), ]); } diff --git a/app/Filament/Resources/EntraGroupResource/Pages/ListEntraGroups.php b/app/Filament/Resources/EntraGroupResource/Pages/ListEntraGroups.php index c2bca06..3f786b6 100644 --- a/app/Filament/Resources/EntraGroupResource/Pages/ListEntraGroups.php +++ b/app/Filament/Resources/EntraGroupResource/Pages/ListEntraGroups.php @@ -11,11 +11,11 @@ use App\Services\Directory\EntraGroupSelection; use App\Services\OperationRunService; use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use App\Support\OperationRunLinks; use Filament\Actions\Action; use Filament\Notifications\Notification; use Filament\Resources\Pages\ListRecords; -use Illuminate\Support\Facades\Gate; class ListEntraGroups extends ListRecords { @@ -30,80 +30,20 @@ protected function getHeaderActions(): array ->url(fn (): string => EntraGroupSyncRunResource::getUrl('index', tenant: Tenant::current())) ->visible(fn (): bool => (bool) Tenant::current()), - Action::make('sync_groups') + UiEnforcement::for(Capabilities::TENANT_SYNC)->apply(Action::make('sync_groups') ->label('Sync Groups') ->icon('heroicon-o-arrow-path') ->color('warning') - ->visible(function (): bool { - $user = auth()->user(); - - if (! $user instanceof User) { - return false; - } - - $tenant = Tenant::current(); - - if (! $tenant) { - return false; - } - - if (! $user->canAccessTenant($tenant)) { - return false; - } - - return true; - }) - ->disabled(function (): bool { - $user = auth()->user(); - - if (! $user instanceof User) { - return true; - } - - $tenant = Tenant::current(); - - if (! $tenant instanceof Tenant) { - return true; - } - - return ! Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $tenant); - }) - ->tooltip(function (): ?string { - $user = auth()->user(); - - if (! $user instanceof User) { - return null; - } - - $tenant = Tenant::current(); - - if (! $tenant instanceof Tenant) { - return null; - } - - return Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $tenant) - ? null - : 'You do not have permission to sync groups.'; - }) ->action(function (): void { + UiEnforcement::for(Capabilities::TENANT_SYNC)->authorizeOrAbort(); + $user = auth()->user(); - - if (! $user instanceof User) { - abort(403); - } - $tenant = Tenant::current(); - if (! $tenant) { - abort(403); + if (! $user instanceof User || ! $tenant instanceof Tenant) { + return; } - if (! $user->canAccessTenant($tenant)) { - abort(403); - } - - abort_unless(Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $tenant), 403); - $selectionKey = EntraGroupSelection::allGroupsV1(); // --- Phase 3: Canonical Operation Run Start --- @@ -182,7 +122,7 @@ protected function getHeaderActions(): array ]) ->sendToDatabase($user) ->send(); - }), + })), ]; } } diff --git a/app/Filament/Resources/EntraGroupSyncRunResource/Pages/ListEntraGroupSyncRuns.php b/app/Filament/Resources/EntraGroupSyncRunResource/Pages/ListEntraGroupSyncRuns.php index 012d578..f8d7a23 100644 --- a/app/Filament/Resources/EntraGroupSyncRunResource/Pages/ListEntraGroupSyncRuns.php +++ b/app/Filament/Resources/EntraGroupSyncRunResource/Pages/ListEntraGroupSyncRuns.php @@ -10,9 +10,9 @@ use App\Notifications\RunStatusChangedNotification; use App\Services\Directory\EntraGroupSelection; use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use Filament\Actions\Action; use Filament\Resources\Pages\ListRecords; -use Illuminate\Support\Facades\Gate; class ListEntraGroupSyncRuns extends ListRecords { @@ -21,49 +21,22 @@ class ListEntraGroupSyncRuns extends ListRecords protected function getHeaderActions(): array { return [ - Action::make('sync_groups') - ->label('Sync Groups') - ->icon('heroicon-o-arrow-path') - ->color('warning') - ->visible(function (): bool { - $user = auth()->user(); + UiEnforcement::for(Capabilities::TENANT_SYNC)->apply( + Action::make('sync_groups') + ->label('Sync Groups') + ->icon('heroicon-o-arrow-path') + ->color('warning') + ->action(function (): void { + UiEnforcement::for(Capabilities::TENANT_SYNC)->authorizeOrAbort(); - if (! $user instanceof User) { - return false; - } + $user = auth()->user(); + $tenant = Tenant::current(); - $tenant = Tenant::current(); + if (! $user instanceof User || ! $tenant instanceof Tenant) { + return; + } - if (! $tenant) { - return false; - } - - if (! $user->canAccessTenant($tenant)) { - return false; - } - - return Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $tenant); - }) - ->action(function (): void { - $user = auth()->user(); - - if (! $user instanceof User) { - abort(403); - } - - $tenant = Tenant::current(); - - if (! $tenant) { - abort(403); - } - - if (! $user->canAccessTenant($tenant)) { - abort(403); - } - - abort_unless(Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $tenant), 403); - - $selectionKey = EntraGroupSelection::allGroupsV1(); + $selectionKey = EntraGroupSelection::allGroupsV1(); $existing = EntraGroupSyncRun::query() ->where('tenant_id', $tenant->getKey()) @@ -106,7 +79,8 @@ protected function getHeaderActions(): array 'run_id' => (int) $run->getKey(), 'status' => 'queued', ])); - }), + }), + ), ]; } } diff --git a/app/Filament/Resources/InventoryItemResource.php b/app/Filament/Resources/InventoryItemResource.php index 2f07d97..b88707e 100644 --- a/app/Filament/Resources/InventoryItemResource.php +++ b/app/Filament/Resources/InventoryItemResource.php @@ -9,6 +9,7 @@ use App\Services\Inventory\DependencyQueryService; use App\Services\Inventory\DependencyTargets\DependencyTargetResolver; use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use App\Support\Badges\BadgeDomain; use App\Support\Badges\BadgeRenderer; use App\Support\Badges\TagBadgeDomain; @@ -17,6 +18,7 @@ use App\Support\Inventory\InventoryPolicyTypeMeta; use BackedEnum; use Filament\Actions; +use Filament\Facades\Filament; use Filament\Infolists\Components\TextEntry; use Filament\Infolists\Components\ViewEntry; use Filament\Resources\Resource; @@ -26,7 +28,6 @@ use Filament\Tables\Table; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; -use Illuminate\Support\Facades\Gate; use UnitEnum; class InventoryItemResource extends Resource @@ -43,21 +44,18 @@ class InventoryItemResource extends Resource public static function canViewAny(): bool { - $tenant = Tenant::current(); - - return $tenant instanceof Tenant - && Gate::allows(Capabilities::TENANT_VIEW, $tenant); + return UiEnforcement::for(Capabilities::TENANT_VIEW)->isAllowed(); } public static function canView(Model $record): bool { - $tenant = Tenant::current(); + $tenant = Filament::getTenant(); if (! $tenant instanceof Tenant) { return false; } - if (! Gate::allows(Capabilities::TENANT_VIEW, $tenant)) { + if (! UiEnforcement::for(Capabilities::TENANT_VIEW)->isAllowed()) { return false; } diff --git a/app/Filament/Resources/InventoryItemResource/Pages/ListInventoryItems.php b/app/Filament/Resources/InventoryItemResource/Pages/ListInventoryItems.php index 56daeaf..dccfe65 100644 --- a/app/Filament/Resources/InventoryItemResource/Pages/ListInventoryItems.php +++ b/app/Filament/Resources/InventoryItemResource/Pages/ListInventoryItems.php @@ -12,6 +12,7 @@ use App\Services\Inventory\InventorySyncService; use App\Services\OperationRunService; use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use App\Support\Inventory\InventoryPolicyTypeMeta; use App\Support\OperationRunLinks; use App\Support\OpsUx\OperationUxPresenter; @@ -24,7 +25,6 @@ use Filament\Notifications\Notification; use Filament\Resources\Pages\ListRecords; use Filament\Support\Enums\Size; -use Illuminate\Support\Facades\Gate; class ListInventoryItems extends ListRecords { @@ -40,140 +40,91 @@ protected function getHeaderWidgets(): array protected function getHeaderActions(): array { return [ - Action::make('run_inventory_sync') - ->label('Run Inventory Sync') - ->icon('heroicon-o-arrow-path') - ->color('warning') - ->form([ - Select::make('policy_types') - ->label('Policy types') - ->multiple() - ->searchable() - ->preload() - ->native(false) - ->hintActions([ - fn (Select $component): HintAction => HintAction::make('select_all_policy_types') - ->label('Select all') - ->link() - ->size(Size::Small) - ->action(function (InventorySyncService $inventorySyncService) use ($component): void { - $component->state($inventorySyncService->defaultSelectionPayload()['policy_types']); - }), - fn (Select $component): HintAction => HintAction::make('clear_policy_types') - ->label('Clear') - ->link() - ->size(Size::Small) - ->action(function () use ($component): void { - $component->state([]); - }), - ]) - ->options(function (): array { - return collect(InventoryPolicyTypeMeta::supported()) - ->filter(fn (array $meta): bool => filled($meta['type'] ?? null)) - ->groupBy(fn (array $meta): string => (string) ($meta['category'] ?? 'Other')) - ->mapWithKeys(function ($items, string $category): array { - $options = collect($items) - ->mapWithKeys(function (array $meta): array { - $type = (string) $meta['type']; - $label = (string) ($meta['label'] ?? $type); - $platform = (string) ($meta['platform'] ?? 'all'); + UiEnforcement::for(Capabilities::TENANT_SYNC)->apply( + Action::make('run_inventory_sync') + ->label('Run Inventory Sync') + ->icon('heroicon-o-arrow-path') + ->color('warning') + ->form([ + Select::make('policy_types') + ->label('Policy types') + ->multiple() + ->searchable() + ->preload() + ->native(false) + ->hintActions([ + fn (Select $component): HintAction => HintAction::make('select_all_policy_types') + ->label('Select all') + ->link() + ->size(Size::Small) + ->action(function (InventorySyncService $inventorySyncService) use ($component): void { + $component->state($inventorySyncService->defaultSelectionPayload()['policy_types']); + }), + fn (Select $component): HintAction => HintAction::make('clear_policy_types') + ->label('Clear') + ->link() + ->size(Size::Small) + ->action(function () use ($component): void { + $component->state([]); + }), + ]) + ->options(function (): array { + return collect(InventoryPolicyTypeMeta::supported()) + ->filter(fn (array $meta): bool => filled($meta['type'] ?? null)) + ->groupBy(fn (array $meta): string => (string) ($meta['category'] ?? 'Other')) + ->mapWithKeys(function ($items, string $category): array { + $options = collect($items) + ->mapWithKeys(function (array $meta): array { + $type = (string) $meta['type']; + $label = (string) ($meta['label'] ?? $type); + $platform = (string) ($meta['platform'] ?? 'all'); - return [$type => "{$label} • {$platform}"]; - }) - ->all(); + return [$type => "{$label} • {$platform}"]; + }) + ->all(); - return [$category => $options]; - }) - ->all(); - }) - ->columnSpanFull(), - Toggle::make('include_foundations') - ->label('Include foundation types') - ->helperText('Include scope tags, assignment filters, and notification templates.') - ->default(true) - ->dehydrated() - ->rules(['boolean']) - ->columnSpanFull(), - Toggle::make('include_dependencies') - ->label('Include dependencies') - ->helperText('Include dependency extraction where supported.') - ->default(true) - ->dehydrated() - ->rules(['boolean']) - ->columnSpanFull(), - Hidden::make('tenant_id') - ->default(fn (): ?string => Tenant::current()?->getKey()) - ->dehydrated(), - ]) - ->visible(function (): bool { - $user = auth()->user(); - if (! $user instanceof User) { - return false; - } + return [$category => $options]; + }) + ->all(); + }) + ->columnSpanFull(), + Toggle::make('include_foundations') + ->label('Include foundation types') + ->helperText('Include scope tags, assignment filters, and notification templates.') + ->default(true) + ->dehydrated() + ->rules(['boolean']) + ->columnSpanFull(), + Toggle::make('include_dependencies') + ->label('Include dependencies') + ->helperText('Include dependency extraction where supported.') + ->default(true) + ->dehydrated() + ->rules(['boolean']) + ->columnSpanFull(), + Hidden::make('tenant_id') + ->default(fn (): ?string => Tenant::current()?->getKey()) + ->dehydrated(), + ]) + ->action(function (array $data, self $livewire, InventorySyncService $inventorySyncService, AuditLogger $auditLogger): void { + UiEnforcement::for(Capabilities::TENANT_SYNC)->authorizeOrAbort(); - $tenant = Tenant::current(); - if (! $tenant instanceof Tenant) { - return false; - } + $tenant = Tenant::current(); + $user = auth()->user(); - return $user->canAccessTenant($tenant); - }) - ->disabled(function (): bool { - $user = auth()->user(); - if (! $user instanceof User) { - return true; - } + if (! $tenant instanceof Tenant || ! $user instanceof User) { + return; + } - $tenant = Tenant::current(); - if (! $tenant instanceof Tenant) { - return true; - } + $requestedTenantId = $data['tenant_id'] ?? null; + if ($requestedTenantId !== null && (int) $requestedTenantId !== (int) $tenant->getKey()) { + Notification::make() + ->title('Not allowed') + ->danger() + ->send(); - return ! Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $tenant); - }) - ->tooltip(function (): ?string { - $user = auth()->user(); - if (! $user instanceof User) { - return null; - } - - $tenant = Tenant::current(); - if (! $tenant instanceof Tenant) { - return null; - } - - return Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $tenant) - ? null - : 'You do not have permission to start inventory sync.'; - }) - ->action(function (array $data, self $livewire, InventorySyncService $inventorySyncService, AuditLogger $auditLogger): void { - $tenant = Tenant::current(); - if (! $tenant instanceof Tenant) { - abort(404); - } - - $user = auth()->user(); - if (! $user instanceof User) { - abort(403, 'Not allowed'); - } - - if (! $user->canAccessTenant($tenant)) { - abort(404); - } - - if (! Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $tenant)) { - abort(403, 'Not allowed'); - } - - $requestedTenantId = $data['tenant_id'] ?? null; - if ($requestedTenantId !== null && (int) $requestedTenantId !== (int) $tenant->getKey()) { - Notification::make() - ->title('Not allowed') - ->danger() - ->send(); - - abort(403, 'Not allowed'); - } + throw new \Symfony\Component\HttpKernel\Exception\HttpException(403, 'Not allowed'); + } $selectionPayload = $inventorySyncService->defaultSelectionPayload(); if (array_key_exists('policy_types', $data)) { @@ -277,7 +228,8 @@ protected function getHeaderActions(): array ->send(); OpsUxBrowserEvents::dispatchRunEnqueued($livewire); - }), + }), + ), ]; } } diff --git a/app/Filament/Resources/InventorySyncRunResource.php b/app/Filament/Resources/InventorySyncRunResource.php index 44d8670..341eea0 100644 --- a/app/Filament/Resources/InventorySyncRunResource.php +++ b/app/Filament/Resources/InventorySyncRunResource.php @@ -7,11 +7,13 @@ use App\Models\InventorySyncRun; use App\Models\Tenant; use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use App\Support\Badges\BadgeDomain; use App\Support\Badges\BadgeRenderer; use App\Support\OperationRunLinks; use BackedEnum; use Filament\Actions; +use Filament\Facades\Filament; use Filament\Infolists\Components\TextEntry; use Filament\Infolists\Components\ViewEntry; use Filament\Resources\Resource; @@ -21,7 +23,6 @@ use Filament\Tables\Table; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; -use Illuminate\Support\Facades\Gate; use UnitEnum; class InventorySyncRunResource extends Resource @@ -40,21 +41,18 @@ class InventorySyncRunResource extends Resource public static function canViewAny(): bool { - $tenant = Tenant::current(); - - return $tenant instanceof Tenant - && Gate::allows(Capabilities::TENANT_VIEW, $tenant); + return UiEnforcement::for(Capabilities::TENANT_VIEW)->isAllowed(); } public static function canView(Model $record): bool { - $tenant = Tenant::current(); + $tenant = Filament::getTenant(); if (! $tenant instanceof Tenant) { return false; } - if (! Gate::allows(Capabilities::TENANT_VIEW, $tenant)) { + if (! UiEnforcement::for(Capabilities::TENANT_VIEW)->isAllowed()) { return false; } diff --git a/app/Filament/Resources/ProviderConnectionResource.php b/app/Filament/Resources/ProviderConnectionResource.php index 6e2377f..0d3c792 100644 --- a/app/Filament/Resources/ProviderConnectionResource.php +++ b/app/Filament/Resources/ProviderConnectionResource.php @@ -15,6 +15,7 @@ use App\Services\Providers\CredentialManager; use App\Services\Providers\ProviderOperationStartGate; use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use App\Support\Badges\BadgeDomain; use App\Support\Badges\BadgeRenderer; use App\Support\OperationRunLinks; @@ -29,7 +30,6 @@ use Filament\Tables\Filters\SelectFilter; use Filament\Tables\Table; use Illuminate\Database\Eloquent\Builder; -use Illuminate\Support\Facades\Gate; use UnitEnum; class ProviderConnectionResource extends Resource @@ -55,17 +55,17 @@ public static function form(Schema $schema): Schema TextInput::make('display_name') ->label('Display name') ->required() - ->disabled(fn (): bool => ! Gate::allows(Capabilities::PROVIDER_MANAGE, Tenant::current())) + ->disabled(fn (): bool => ! UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->isAllowed()) ->maxLength(255), TextInput::make('entra_tenant_id') ->label('Entra tenant ID') ->required() ->maxLength(255) - ->disabled(fn (): bool => ! Gate::allows(Capabilities::PROVIDER_MANAGE, Tenant::current())) + ->disabled(fn (): bool => ! UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->isAllowed()) ->rules(['uuid']), Toggle::make('is_default') ->label('Default connection') - ->disabled(fn (): bool => ! Gate::allows(Capabilities::PROVIDER_MANAGE, Tenant::current())) + ->disabled(fn (): bool => ! UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->isAllowed()) ->helperText('Exactly one default connection is required per tenant/provider.'), TextInput::make('status') ->label('Status') @@ -146,55 +146,25 @@ public static function table(Table $table): Table ]) ->actions([ Actions\ActionGroup::make([ - Actions\EditAction::make(), + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->apply( + Actions\EditAction::make(), + ), - Actions\Action::make('check_connection') + UiEnforcement::for(Capabilities::PROVIDER_RUN)->apply(Actions\Action::make('check_connection') ->label('Check connection') ->icon('heroicon-o-check-badge') ->color('success') - ->visible(function (ProviderConnection $record): bool { - $user = auth()->user(); - - if (! $user instanceof User) { - return false; - } - - $tenant = Tenant::current(); - - if (! $tenant instanceof Tenant) { - return false; - } - - return $user->canAccessTenant($tenant) && $record->status !== 'disabled'; - }) - ->disabled(function (): bool { - $tenant = Tenant::current(); - - if (! $tenant instanceof Tenant) { - return true; - } - - return ! Gate::allows(Capabilities::PROVIDER_RUN, $tenant); - }) - ->tooltip(function (): ?string { - $tenant = Tenant::current(); - - if (! $tenant instanceof Tenant) { - return null; - } - - return Gate::allows(Capabilities::PROVIDER_RUN, $tenant) - ? null - : 'You do not have permission to run provider operations.'; - }) + ->visible(fn (ProviderConnection $record): bool => $record->status !== 'disabled') ->action(function (ProviderConnection $record, ProviderOperationStartGate $gate): void { + UiEnforcement::for(Capabilities::PROVIDER_RUN)->authorizeOrAbort(); + $tenant = Tenant::current(); $user = auth()->user(); - abort_unless($tenant instanceof Tenant, 404); - abort_unless($user instanceof User, 403); - abort_unless($user->canAccessTenant($tenant), 404); - abort_unless(Gate::allows(Capabilities::PROVIDER_RUN, $tenant), 403); + if (! $tenant instanceof Tenant || ! $user instanceof User) { + return; + } + $initiator = $user; $result = $gate->start( @@ -252,55 +222,23 @@ public static function table(Table $table): Table ->url(OperationRunLinks::view($result->run, $tenant)), ]) ->send(); - }), + })), - Actions\Action::make('inventory_sync') + UiEnforcement::for(Capabilities::PROVIDER_RUN)->apply(Actions\Action::make('inventory_sync') ->label('Inventory sync') ->icon('heroicon-o-arrow-path') ->color('info') - ->visible(function (ProviderConnection $record): bool { - $user = auth()->user(); - - if (! $user instanceof User) { - return false; - } - - $tenant = Tenant::current(); - - if (! $tenant instanceof Tenant) { - return false; - } - - return $user->canAccessTenant($tenant) && $record->status !== 'disabled'; - }) - ->disabled(function (): bool { - $tenant = Tenant::current(); - - if (! $tenant instanceof Tenant) { - return true; - } - - return ! Gate::allows(Capabilities::PROVIDER_RUN, $tenant); - }) - ->tooltip(function (): ?string { - $tenant = Tenant::current(); - - if (! $tenant instanceof Tenant) { - return null; - } - - return Gate::allows(Capabilities::PROVIDER_RUN, $tenant) - ? null - : 'You do not have permission to run provider operations.'; - }) + ->visible(fn (ProviderConnection $record): bool => $record->status !== 'disabled') ->action(function (ProviderConnection $record, ProviderOperationStartGate $gate): void { + UiEnforcement::for(Capabilities::PROVIDER_RUN)->authorizeOrAbort(); + $tenant = Tenant::current(); $user = auth()->user(); - abort_unless($tenant instanceof Tenant, 404); - abort_unless($user instanceof User, 403); - abort_unless($user->canAccessTenant($tenant), 404); - abort_unless(Gate::allows(Capabilities::PROVIDER_RUN, $tenant), 403); + if (! $tenant instanceof Tenant || ! $user instanceof User) { + return; + } + $initiator = $user; $result = $gate->start( @@ -358,55 +296,23 @@ public static function table(Table $table): Table ->url(OperationRunLinks::view($result->run, $tenant)), ]) ->send(); - }), + })), - Actions\Action::make('compliance_snapshot') + UiEnforcement::for(Capabilities::PROVIDER_RUN)->apply(Actions\Action::make('compliance_snapshot') ->label('Compliance snapshot') ->icon('heroicon-o-shield-check') ->color('info') - ->visible(function (ProviderConnection $record): bool { - $user = auth()->user(); - - if (! $user instanceof User) { - return false; - } - - $tenant = Tenant::current(); - - if (! $tenant instanceof Tenant) { - return false; - } - - return $user->canAccessTenant($tenant) && $record->status !== 'disabled'; - }) - ->disabled(function (): bool { - $tenant = Tenant::current(); - - if (! $tenant instanceof Tenant) { - return true; - } - - return ! Gate::allows(Capabilities::PROVIDER_RUN, $tenant); - }) - ->tooltip(function (): ?string { - $tenant = Tenant::current(); - - if (! $tenant instanceof Tenant) { - return null; - } - - return Gate::allows(Capabilities::PROVIDER_RUN, $tenant) - ? null - : 'You do not have permission to run provider operations.'; - }) + ->visible(fn (ProviderConnection $record): bool => $record->status !== 'disabled') ->action(function (ProviderConnection $record, ProviderOperationStartGate $gate): void { + UiEnforcement::for(Capabilities::PROVIDER_RUN)->authorizeOrAbort(); + $tenant = Tenant::current(); $user = auth()->user(); - abort_unless($tenant instanceof Tenant, 404); - abort_unless($user instanceof User, 403); - abort_unless($user->canAccessTenant($tenant), 404); - abort_unless(Gate::allows(Capabilities::PROVIDER_RUN, $tenant), 403); + if (! $tenant instanceof Tenant || ! $user instanceof User) { + return; + } + $initiator = $user; $result = $gate->start( @@ -464,19 +370,21 @@ public static function table(Table $table): Table ->url(OperationRunLinks::view($result->run, $tenant)), ]) ->send(); - }), + })), - Actions\Action::make('set_default') + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->apply(Actions\Action::make('set_default') ->label('Set as default') ->icon('heroicon-o-star') ->color('primary') - ->visible(fn (ProviderConnection $record): bool => Gate::allows(Capabilities::PROVIDER_MANAGE, Tenant::current()) - && $record->status !== 'disabled' - && ! $record->is_default) + ->visible(fn (ProviderConnection $record): bool => $record->status !== 'disabled' && ! $record->is_default) ->action(function (ProviderConnection $record, AuditLogger $auditLogger): void { + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->authorizeOrAbort(); + $tenant = Tenant::current(); - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::PROVIDER_MANAGE, $tenant), 403); + if (! $tenant instanceof Tenant) { + return; + } $record->makeDefault(); @@ -506,14 +414,13 @@ public static function table(Table $table): Table ->title('Default connection updated') ->success() ->send(); - }), + })), - Actions\Action::make('update_credentials') + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->apply(Actions\Action::make('update_credentials') ->label('Update credentials') ->icon('heroicon-o-key') ->color('primary') ->modalDescription('Client secret is stored encrypted and will never be shown again.') - ->visible(fn (): bool => Gate::allows(Capabilities::PROVIDER_MANAGE, Tenant::current())) ->form([ TextInput::make('client_id') ->label('Client ID') @@ -526,9 +433,13 @@ public static function table(Table $table): Table ->maxLength(255), ]) ->action(function (array $data, ProviderConnection $record, CredentialManager $credentials, AuditLogger $auditLogger): void { + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->authorizeOrAbort(); + $tenant = Tenant::current(); - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::PROVIDER_MANAGE, $tenant), 403); + if (! $tenant instanceof Tenant) { + return; + } $credentials->upsertClientSecretCredential( connection: $record, @@ -562,18 +473,21 @@ public static function table(Table $table): Table ->title('Credentials updated') ->success() ->send(); - }), + })), - Actions\Action::make('enable_connection') + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->apply(Actions\Action::make('enable_connection') ->label('Enable connection') ->icon('heroicon-o-play') ->color('success') - ->visible(fn (ProviderConnection $record): bool => Gate::allows(Capabilities::PROVIDER_MANAGE, Tenant::current()) - && $record->status === 'disabled') + ->visible(fn (ProviderConnection $record): bool => $record->status === 'disabled') ->action(function (ProviderConnection $record, AuditLogger $auditLogger): void { + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->authorizeOrAbort(); + $tenant = Tenant::current(); - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::PROVIDER_MANAGE, $tenant), 403); + if (! $tenant instanceof Tenant) { + return; + } $hadCredentials = $record->credential()->exists(); $status = $hadCredentials ? 'connected' : 'needs_consent'; @@ -626,19 +540,22 @@ public static function table(Table $table): Table ->title('Provider connection enabled') ->success() ->send(); - }), + })), - Actions\Action::make('disable_connection') + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->apply(Actions\Action::make('disable_connection') ->label('Disable connection') ->icon('heroicon-o-archive-box-x-mark') ->color('danger') ->requiresConfirmation() - ->visible(fn (ProviderConnection $record): bool => Gate::allows(Capabilities::PROVIDER_MANAGE, Tenant::current()) - && $record->status !== 'disabled') + ->visible(fn (ProviderConnection $record): bool => $record->status !== 'disabled') ->action(function (ProviderConnection $record, AuditLogger $auditLogger): void { + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->authorizeOrAbort(); + $tenant = Tenant::current(); - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::PROVIDER_MANAGE, $tenant), 403); + if (! $tenant instanceof Tenant) { + return; + } $previousStatus = (string) $record->status; @@ -673,7 +590,7 @@ public static function table(Table $table): Table ->title('Provider connection disabled') ->warning() ->send(); - }), + })), ]) ->label('Actions') ->icon('heroicon-o-ellipsis-vertical') diff --git a/app/Filament/Resources/ProviderConnectionResource/Pages/EditProviderConnection.php b/app/Filament/Resources/ProviderConnectionResource/Pages/EditProviderConnection.php index bccec5d..00403c2 100644 --- a/app/Filament/Resources/ProviderConnectionResource/Pages/EditProviderConnection.php +++ b/app/Filament/Resources/ProviderConnectionResource/Pages/EditProviderConnection.php @@ -14,6 +14,7 @@ use App\Services\Providers\CredentialManager; use App\Services\Providers\ProviderOperationStartGate; use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use App\Support\OperationRunLinks; use Filament\Actions; use Filament\Actions\Action; @@ -21,7 +22,6 @@ use Filament\Notifications\Notification; use Filament\Resources\Pages\EditRecord; use Illuminate\Database\Eloquent\Model; -use Illuminate\Support\Facades\Gate; class EditProviderConnection extends EditRecord { @@ -108,89 +108,67 @@ protected function afterSave(): void protected function getHeaderActions(): array { - $tenant = Tenant::current(); - return [ Actions\DeleteAction::make() ->visible(false), Actions\ActionGroup::make([ - Action::make('view_last_check_run') - ->label('View last check run') - ->icon('heroicon-o-eye') - ->color('gray') - ->visible(fn (ProviderConnection $record): bool => $tenant instanceof Tenant - && Gate::allows(Capabilities::PROVIDER_VIEW, $tenant) - && OperationRun::query() - ->where('tenant_id', $tenant->getKey()) - ->where('type', 'provider.connection.check') - ->where('context->provider_connection_id', (int) $record->getKey()) - ->exists()) - ->url(function (ProviderConnection $record): ?string { + UiEnforcement::for(Capabilities::PROVIDER_VIEW) + ->andVisibleWhen(function (ProviderConnection $record): bool { $tenant = Tenant::current(); - if (! $tenant instanceof Tenant) { - return null; - } - - $run = OperationRun::query() - ->where('tenant_id', $tenant->getKey()) - ->where('type', 'provider.connection.check') - ->where('context->provider_connection_id', (int) $record->getKey()) - ->orderByDesc('id') - ->first(); - - if (! $run instanceof OperationRun) { - return null; - } - - return OperationRunLinks::view($run, $tenant); - }), - - Action::make('check_connection') - ->label('Check connection') - ->icon('heroicon-o-check-badge') - ->color('success') - ->visible(function (ProviderConnection $record): bool { - $tenant = Tenant::current(); - $user = auth()->user(); - return $tenant instanceof Tenant - && $user instanceof User - && $user->canAccessTenant($tenant) - && $record->status !== 'disabled'; + && OperationRun::query() + ->where('tenant_id', $tenant->getKey()) + ->where('type', 'provider.connection.check') + ->where('context->provider_connection_id', (int) $record->getKey()) + ->exists(); }) - ->disabled(function (): bool { - $tenant = Tenant::current(); - $user = auth()->user(); + ->apply( + Action::make('view_last_check_run') + ->label('View last check run') + ->icon('heroicon-o-eye') + ->color('gray') + ->url(function (ProviderConnection $record): ?string { + $tenant = Tenant::current(); - if (! $tenant instanceof Tenant || ! $user instanceof User) { - return true; - } + if (! $tenant instanceof Tenant) { + return null; + } - return ! Gate::forUser($user)->allows(Capabilities::PROVIDER_RUN, $tenant); - }) - ->tooltip(function (): ?string { - $tenant = Tenant::current(); - $user = auth()->user(); + $run = OperationRun::query() + ->where('tenant_id', $tenant->getKey()) + ->where('type', 'provider.connection.check') + ->where('context->provider_connection_id', (int) $record->getKey()) + ->orderByDesc('id') + ->first(); - if (! $tenant instanceof Tenant || ! $user instanceof User) { - return null; - } + if (! $run instanceof OperationRun) { + return null; + } - return Gate::forUser($user)->allows(Capabilities::PROVIDER_RUN, $tenant) - ? null - : 'You do not have permission to run provider operations.'; - }) - ->action(function (ProviderConnection $record, ProviderOperationStartGate $gate): void { - $tenant = Tenant::current(); - $user = auth()->user(); + return OperationRunLinks::view($run, $tenant); + }), + ), - abort_unless($tenant instanceof Tenant, 404); - abort_unless($user instanceof User, 403); - abort_unless($user->canAccessTenant($tenant), 404); - abort_unless(Gate::forUser($user)->allows(Capabilities::PROVIDER_RUN, $tenant), 403); - $initiator = $user; + UiEnforcement::for(Capabilities::PROVIDER_RUN) + ->andVisibleWhen(fn (ProviderConnection $record): bool => $record->status !== 'disabled') + ->apply( + Action::make('check_connection') + ->label('Check connection') + ->icon('heroicon-o-check-badge') + ->color('success') + ->action(function (ProviderConnection $record, ProviderOperationStartGate $gate): void { + UiEnforcement::for(Capabilities::PROVIDER_RUN)->authorizeOrAbort(); + + $tenant = Tenant::current(); + $user = auth()->user(); + + if (! $tenant instanceof Tenant || ! $user instanceof User) { + return; + } + + $initiator = $user; $result = $gate->start( tenant: $tenant, @@ -247,29 +225,34 @@ protected function getHeaderActions(): array ->url(OperationRunLinks::view($result->run, $tenant)), ]) ->send(); - }), + }), + ), - Action::make('update_credentials') - ->label('Update credentials') - ->icon('heroicon-o-key') - ->color('primary') - ->modalDescription('Client secret is stored encrypted and will never be shown again.') - ->visible(fn (): bool => $tenant instanceof Tenant && Gate::allows(Capabilities::PROVIDER_MANAGE, $tenant)) - ->form([ - TextInput::make('client_id') - ->label('Client ID') - ->required() - ->maxLength(255), - TextInput::make('client_secret') - ->label('Client secret') - ->password() - ->required() - ->maxLength(255), - ]) - ->action(function (array $data, ProviderConnection $record, CredentialManager $credentials, AuditLogger $auditLogger): void { - $tenant = Tenant::current(); + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->apply( + Action::make('update_credentials') + ->label('Update credentials') + ->icon('heroicon-o-key') + ->color('primary') + ->modalDescription('Client secret is stored encrypted and will never be shown again.') + ->form([ + TextInput::make('client_id') + ->label('Client ID') + ->required() + ->maxLength(255), + TextInput::make('client_secret') + ->label('Client secret') + ->password() + ->required() + ->maxLength(255), + ]) + ->action(function (array $data, ProviderConnection $record, CredentialManager $credentials, AuditLogger $auditLogger): void { + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->authorizeOrAbort(); - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::PROVIDER_MANAGE, $tenant), 403); + $tenant = Tenant::current(); + + if (! $tenant instanceof Tenant) { + return; + } $credentials->upsertClientSecretCredential( connection: $record, @@ -303,24 +286,34 @@ protected function getHeaderActions(): array ->title('Credentials updated') ->success() ->send(); - }), + }), + ), - Action::make('set_default') - ->label('Set as default') - ->icon('heroicon-o-star') - ->color('primary') - ->visible(fn (ProviderConnection $record): bool => $tenant instanceof Tenant - && Gate::allows(Capabilities::PROVIDER_MANAGE, $tenant) - && $record->status !== 'disabled' - && ! $record->is_default - && ProviderConnection::query() - ->where('tenant_id', $tenant->getKey()) - ->where('provider', $record->provider) - ->count() > 1) - ->action(function (ProviderConnection $record, AuditLogger $auditLogger): void { + UiEnforcement::for(Capabilities::PROVIDER_MANAGE) + ->andVisibleWhen(function (ProviderConnection $record): bool { $tenant = Tenant::current(); - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::PROVIDER_MANAGE, $tenant), 403); + return $record->status !== 'disabled' + && ! $record->is_default + && $tenant instanceof Tenant + && ProviderConnection::query() + ->where('tenant_id', $tenant->getKey()) + ->where('provider', $record->provider) + ->count() > 1; + }) + ->apply( + Action::make('set_default') + ->label('Set as default') + ->icon('heroicon-o-star') + ->color('primary') + ->action(function (ProviderConnection $record, AuditLogger $auditLogger): void { + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->authorizeOrAbort(); + + $tenant = Tenant::current(); + + if (! $tenant instanceof Tenant) { + return; + } $record->makeDefault(); @@ -350,52 +343,27 @@ protected function getHeaderActions(): array ->title('Default connection updated') ->success() ->send(); - }), + }), + ), - Action::make('inventory_sync') - ->label('Inventory sync') - ->icon('heroicon-o-arrow-path') - ->color('info') - ->visible(function (ProviderConnection $record): bool { - $tenant = Tenant::current(); - $user = auth()->user(); + UiEnforcement::for(Capabilities::PROVIDER_RUN) + ->andVisibleWhen(fn (ProviderConnection $record): bool => $record->status !== 'disabled') + ->apply( + Action::make('inventory_sync') + ->label('Inventory sync') + ->icon('heroicon-o-arrow-path') + ->color('info') + ->action(function (ProviderConnection $record, ProviderOperationStartGate $gate): void { + UiEnforcement::for(Capabilities::PROVIDER_RUN)->authorizeOrAbort(); - return $tenant instanceof Tenant - && $user instanceof User - && $user->canAccessTenant($tenant) - && $record->status !== 'disabled'; - }) - ->disabled(function (): bool { - $tenant = Tenant::current(); - $user = auth()->user(); + $tenant = Tenant::current(); + $user = auth()->user(); - if (! $tenant instanceof Tenant || ! $user instanceof User) { - return true; - } + if (! $tenant instanceof Tenant || ! $user instanceof User) { + return; + } - return ! Gate::forUser($user)->allows(Capabilities::PROVIDER_RUN, $tenant); - }) - ->tooltip(function (): ?string { - $tenant = Tenant::current(); - $user = auth()->user(); - - if (! $tenant instanceof Tenant || ! $user instanceof User) { - return null; - } - - return Gate::forUser($user)->allows(Capabilities::PROVIDER_RUN, $tenant) - ? null - : 'You do not have permission to run provider operations.'; - }) - ->action(function (ProviderConnection $record, ProviderOperationStartGate $gate): void { - $tenant = Tenant::current(); - $user = auth()->user(); - - abort_unless($tenant instanceof Tenant, 404); - abort_unless($user instanceof User, 403); - abort_unless($user->canAccessTenant($tenant), 404); - abort_unless(Gate::forUser($user)->allows(Capabilities::PROVIDER_RUN, $tenant), 403); - $initiator = $user; + $initiator = $user; $result = $gate->start( tenant: $tenant, @@ -452,52 +420,27 @@ protected function getHeaderActions(): array ->url(OperationRunLinks::view($result->run, $tenant)), ]) ->send(); - }), + }), + ), - Action::make('compliance_snapshot') - ->label('Compliance snapshot') - ->icon('heroicon-o-shield-check') - ->color('info') - ->visible(function (ProviderConnection $record): bool { - $tenant = Tenant::current(); - $user = auth()->user(); + UiEnforcement::for(Capabilities::PROVIDER_RUN) + ->andVisibleWhen(fn (ProviderConnection $record): bool => $record->status !== 'disabled') + ->apply( + Action::make('compliance_snapshot') + ->label('Compliance snapshot') + ->icon('heroicon-o-shield-check') + ->color('info') + ->action(function (ProviderConnection $record, ProviderOperationStartGate $gate): void { + UiEnforcement::for(Capabilities::PROVIDER_RUN)->authorizeOrAbort(); - return $tenant instanceof Tenant - && $user instanceof User - && $user->canAccessTenant($tenant) - && $record->status !== 'disabled'; - }) - ->disabled(function (): bool { - $tenant = Tenant::current(); - $user = auth()->user(); + $tenant = Tenant::current(); + $user = auth()->user(); - if (! $tenant instanceof Tenant || ! $user instanceof User) { - return true; - } + if (! $tenant instanceof Tenant || ! $user instanceof User) { + return; + } - return ! Gate::forUser($user)->allows(Capabilities::PROVIDER_RUN, $tenant); - }) - ->tooltip(function (): ?string { - $tenant = Tenant::current(); - $user = auth()->user(); - - if (! $tenant instanceof Tenant || ! $user instanceof User) { - return null; - } - - return Gate::forUser($user)->allows(Capabilities::PROVIDER_RUN, $tenant) - ? null - : 'You do not have permission to run provider operations.'; - }) - ->action(function (ProviderConnection $record, ProviderOperationStartGate $gate): void { - $tenant = Tenant::current(); - $user = auth()->user(); - - abort_unless($tenant instanceof Tenant, 404); - abort_unless($user instanceof User, 403); - abort_unless($user->canAccessTenant($tenant), 404); - abort_unless(Gate::forUser($user)->allows(Capabilities::PROVIDER_RUN, $tenant), 403); - $initiator = $user; + $initiator = $user; $result = $gate->start( tenant: $tenant, @@ -554,19 +497,24 @@ protected function getHeaderActions(): array ->url(OperationRunLinks::view($result->run, $tenant)), ]) ->send(); - }), + }), + ), - Action::make('enable_connection') - ->label('Enable connection') - ->icon('heroicon-o-play') - ->color('success') - ->visible(fn (ProviderConnection $record): bool => $tenant instanceof Tenant - && Gate::allows(Capabilities::PROVIDER_MANAGE, $tenant) - && $record->status === 'disabled') - ->action(function (ProviderConnection $record, AuditLogger $auditLogger): void { - $tenant = Tenant::current(); + UiEnforcement::for(Capabilities::PROVIDER_MANAGE) + ->andVisibleWhen(fn (ProviderConnection $record): bool => $record->status === 'disabled') + ->apply( + Action::make('enable_connection') + ->label('Enable connection') + ->icon('heroicon-o-play') + ->color('success') + ->action(function (ProviderConnection $record, AuditLogger $auditLogger): void { + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->authorizeOrAbort(); - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::PROVIDER_MANAGE, $tenant), 403); + $tenant = Tenant::current(); + + if (! $tenant instanceof Tenant) { + return; + } $hadCredentials = $record->credential()->exists(); $status = $hadCredentials ? 'connected' : 'needs_consent'; @@ -619,20 +567,25 @@ protected function getHeaderActions(): array ->title('Provider connection enabled') ->success() ->send(); - }), + }), + ), - Action::make('disable_connection') - ->label('Disable connection') - ->icon('heroicon-o-archive-box-x-mark') - ->color('danger') - ->requiresConfirmation() - ->visible(fn (ProviderConnection $record): bool => $tenant instanceof Tenant - && Gate::allows(Capabilities::PROVIDER_MANAGE, $tenant) - && $record->status !== 'disabled') - ->action(function (ProviderConnection $record, AuditLogger $auditLogger): void { - $tenant = Tenant::current(); + UiEnforcement::for(Capabilities::PROVIDER_MANAGE) + ->andVisibleWhen(fn (ProviderConnection $record): bool => $record->status !== 'disabled') + ->apply( + Action::make('disable_connection') + ->label('Disable connection') + ->icon('heroicon-o-archive-box-x-mark') + ->color('danger') + ->requiresConfirmation() + ->action(function (ProviderConnection $record, AuditLogger $auditLogger): void { + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->authorizeOrAbort(); - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::PROVIDER_MANAGE, $tenant), 403); + $tenant = Tenant::current(); + + if (! $tenant instanceof Tenant) { + return; + } $previousStatus = (string) $record->status; @@ -667,7 +620,8 @@ protected function getHeaderActions(): array ->title('Provider connection disabled') ->warning() ->send(); - }), + }), + ), ]) ->label('Actions') ->icon('heroicon-o-ellipsis-vertical') @@ -677,9 +631,7 @@ protected function getHeaderActions(): array protected function getFormActions(): array { - $tenant = Tenant::current(); - - if ($tenant instanceof Tenant && Gate::allows(Capabilities::PROVIDER_MANAGE, $tenant)) { + if (UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->isAllowed()) { return parent::getFormActions(); } @@ -690,9 +642,7 @@ protected function getFormActions(): array protected function handleRecordUpdate(Model $record, array $data): Model { - $tenant = Tenant::current(); - - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::PROVIDER_MANAGE, $tenant), 403); + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->authorizeOrAbort(); return parent::handleRecordUpdate($record, $data); } diff --git a/app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php b/app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php index bf1780c..0e2cbda 100644 --- a/app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php +++ b/app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php @@ -3,6 +3,8 @@ namespace App\Filament\Resources\ProviderConnectionResource\Pages; use App\Filament\Resources\ProviderConnectionResource; +use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use Filament\Actions; use Filament\Resources\Pages\ListRecords; @@ -13,11 +15,9 @@ class ListProviderConnections extends ListRecords protected function getHeaderActions(): array { return [ - Actions\CreateAction::make() - ->disabled(fn (): bool => ! \Illuminate\Support\Facades\Gate::allows(\App\Support\Auth\Capabilities::PROVIDER_MANAGE, \App\Models\Tenant::current())) - ->tooltip(fn (): ?string => \Illuminate\Support\Facades\Gate::allows(\App\Support\Auth\Capabilities::PROVIDER_MANAGE, \App\Models\Tenant::current()) - ? null - : 'You do not have permission to create provider connections.'), + UiEnforcement::for(Capabilities::PROVIDER_MANAGE)->apply( + Actions\CreateAction::make(), + ), ]; } } diff --git a/app/Filament/Resources/RestoreRunResource.php b/app/Filament/Resources/RestoreRunResource.php index 45f256e..ddd7419 100644 --- a/app/Filament/Resources/RestoreRunResource.php +++ b/app/Filament/Resources/RestoreRunResource.php @@ -22,6 +22,7 @@ use App\Services\OperationRunService; use App\Services\Operations\BulkSelectionIdentity; use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use App\Support\Badges\BadgeDomain; use App\Support\Badges\BadgeRenderer; use App\Support\OperationRunLinks; @@ -50,7 +51,6 @@ use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\QueryException; use Illuminate\Support\Facades\Cache; -use Illuminate\Support\Facades\Gate; use Illuminate\Support\Str; use Illuminate\Validation\ValidationException; use UnitEnum; @@ -65,8 +65,7 @@ class RestoreRunResource extends Resource public static function canCreate(): bool { - return ($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_MANAGE, $tenant); + return UiEnforcement::for(Capabilities::TENANT_MANAGE)->isAllowed(); } public static function form(Schema $schema): Schema @@ -748,7 +747,7 @@ public static function table(Table $table): Table ->actions([ Actions\ViewAction::make(), ActionGroup::make([ - Actions\Action::make('rerun') + UiEnforcement::for(Capabilities::TENANT_MANAGE)->preserveVisibility()->apply(Actions\Action::make('rerun') ->label('Rerun') ->icon('heroicon-o-arrow-path') ->color('primary') @@ -761,17 +760,13 @@ public static function table(Table $table): Table && $backupSet !== null && ! $backupSet->trashed(); }) - ->disabled(fn (): bool => ! (($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_MANAGE, $tenant))) ->action(function ( RestoreRun $record, RestoreService $restoreService, \App\Services\Intune\AuditLogger $auditLogger, HasTable $livewire ) { - $currentTenant = Tenant::current(); - - abort_unless($currentTenant instanceof Tenant && Gate::allows(Capabilities::TENANT_MANAGE, $currentTenant), 403); + UiEnforcement::for(Capabilities::TENANT_MANAGE)->authorizeOrAbort(); $tenant = $record->tenant; $backupSet = $record->backupSet; @@ -932,19 +927,15 @@ public static function table(Table $table): Table OpsUxBrowserEvents::dispatchRunEnqueued($livewire); OperationUxPresenter::queuedToast('restore.execute') ->send(); - }), - Actions\Action::make('restore') + })), + UiEnforcement::for(Capabilities::TENANT_MANAGE)->preserveVisibility()->apply(Actions\Action::make('restore') ->label('Restore') ->color('success') ->icon('heroicon-o-arrow-uturn-left') ->requiresConfirmation() ->visible(fn (RestoreRun $record): bool => $record->trashed()) - ->disabled(fn (): bool => ! (($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_MANAGE, $tenant))) ->action(function (RestoreRun $record, \App\Services\Intune\AuditLogger $auditLogger) { - $tenant = Tenant::current(); - - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::TENANT_MANAGE, $tenant), 403); + UiEnforcement::for(Capabilities::TENANT_MANAGE)->authorizeOrAbort(); $record->restore(); @@ -963,19 +954,15 @@ public static function table(Table $table): Table ->title('Restore run restored') ->success() ->send(); - }), - Actions\Action::make('archive') + })), + UiEnforcement::for(Capabilities::TENANT_MANAGE)->preserveVisibility()->apply(Actions\Action::make('archive') ->label('Archive') ->color('danger') ->icon('heroicon-o-archive-box-x-mark') ->requiresConfirmation() ->visible(fn (RestoreRun $record): bool => ! $record->trashed()) - ->disabled(fn (): bool => ! (($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_MANAGE, $tenant))) ->action(function (RestoreRun $record, \App\Services\Intune\AuditLogger $auditLogger) { - $tenant = Tenant::current(); - - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::TENANT_MANAGE, $tenant), 403); + UiEnforcement::for(Capabilities::TENANT_MANAGE)->authorizeOrAbort(); if (! $record->isDeletable()) { Notification::make() @@ -1004,19 +991,15 @@ public static function table(Table $table): Table ->title('Restore run archived') ->success() ->send(); - }), - Actions\Action::make('forceDelete') + })), + UiEnforcement::for(Capabilities::TENANT_DELETE)->preserveVisibility()->apply(Actions\Action::make('forceDelete') ->label('Force delete') ->color('danger') ->icon('heroicon-o-trash') ->requiresConfirmation() ->visible(fn (RestoreRun $record): bool => $record->trashed()) - ->disabled(fn (): bool => ! (($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_DELETE, $tenant))) ->action(function (RestoreRun $record, \App\Services\Intune\AuditLogger $auditLogger) { - $tenant = Tenant::current(); - - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::TENANT_DELETE, $tenant), 403); + UiEnforcement::for(Capabilities::TENANT_DELETE)->authorizeOrAbort(); if ($record->tenant) { $auditLogger->log( @@ -1035,18 +1018,16 @@ public static function table(Table $table): Table ->title('Restore run permanently deleted') ->success() ->send(); - }), + })), ])->icon('heroicon-o-ellipsis-vertical'), ]) ->bulkActions([ BulkActionGroup::make([ - BulkAction::make('bulk_delete') + UiEnforcement::for(Capabilities::TENANT_MANAGE)->preserveVisibility()->apply(BulkAction::make('bulk_delete') ->label('Archive Restore Runs') ->icon('heroicon-o-trash') ->color('danger') ->requiresConfirmation() - ->disabled(fn (): bool => ! (($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_MANAGE, $tenant))) ->hidden(function (HasTable $livewire): bool { $trashedFilterState = $livewire->getTableFilterState(TrashedFilter::class) ?? []; $value = $trashedFilterState['value'] ?? null; @@ -1071,17 +1052,13 @@ public static function table(Table $table): Table return []; }) ->action(function (Collection $records) { + UiEnforcement::for(Capabilities::TENANT_MANAGE)->authorizeOrAbort(); + $tenant = Tenant::current(); $user = auth()->user(); $count = $records->count(); $ids = $records->pluck('id')->toArray(); - if (! $tenant instanceof Tenant) { - return; - } - - abort_unless(Gate::allows(Capabilities::TENANT_MANAGE, $tenant), 403); - $initiator = $user instanceof User ? $user : null; /** @var BulkSelectionIdentity $selection */ @@ -1121,15 +1098,13 @@ public static function table(Table $table): Table ]) ->send(); }) - ->deselectRecordsAfterCompletion(), + ->deselectRecordsAfterCompletion()), - BulkAction::make('bulk_restore') + UiEnforcement::for(Capabilities::TENANT_MANAGE)->preserveVisibility()->apply(BulkAction::make('bulk_restore') ->label('Restore Restore Runs') ->icon('heroicon-o-arrow-uturn-left') ->color('success') ->requiresConfirmation() - ->disabled(fn (): bool => ! (($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_MANAGE, $tenant))) ->hidden(function (HasTable $livewire): bool { $trashedFilterState = $livewire->getTableFilterState(TrashedFilter::class) ?? []; $value = $trashedFilterState['value'] ?? null; @@ -1141,17 +1116,13 @@ public static function table(Table $table): Table ->modalHeading(fn (Collection $records) => "Restore {$records->count()} restore runs?") ->modalDescription('Archived runs will be restored back to the active list. Active runs will be skipped.') ->action(function (Collection $records) { + UiEnforcement::for(Capabilities::TENANT_MANAGE)->authorizeOrAbort(); + $tenant = Tenant::current(); $user = auth()->user(); $count = $records->count(); $ids = $records->pluck('id')->toArray(); - if (! $tenant instanceof Tenant) { - return; - } - - abort_unless(Gate::allows(Capabilities::TENANT_MANAGE, $tenant), 403); - $initiator = $user instanceof User ? $user : null; /** @var BulkSelectionIdentity $selection */ @@ -1202,15 +1173,13 @@ public static function table(Table $table): Table ]) ->send(); }) - ->deselectRecordsAfterCompletion(), + ->deselectRecordsAfterCompletion()), - BulkAction::make('bulk_force_delete') + UiEnforcement::for(Capabilities::TENANT_DELETE)->preserveVisibility()->apply(BulkAction::make('bulk_force_delete') ->label('Force Delete Restore Runs') ->icon('heroicon-o-trash') ->color('danger') ->requiresConfirmation() - ->disabled(fn (): bool => ! (($tenant = Tenant::current()) instanceof Tenant - && Gate::allows(Capabilities::TENANT_DELETE, $tenant))) ->hidden(function (HasTable $livewire): bool { $trashedFilterState = $livewire->getTableFilterState(TrashedFilter::class) ?? []; $value = $trashedFilterState['value'] ?? null; @@ -1231,17 +1200,13 @@ public static function table(Table $table): Table ]), ]) ->action(function (Collection $records) { + UiEnforcement::for(Capabilities::TENANT_DELETE)->authorizeOrAbort(); + $tenant = Tenant::current(); $user = auth()->user(); $count = $records->count(); $ids = $records->pluck('id')->toArray(); - if (! $tenant instanceof Tenant) { - return; - } - - abort_unless(Gate::allows(Capabilities::TENANT_DELETE, $tenant), 403); - $initiator = $user instanceof User ? $user : null; /** @var BulkSelectionIdentity $selection */ @@ -1292,7 +1257,7 @@ public static function table(Table $table): Table ]) ->send(); }) - ->deselectRecordsAfterCompletion(), + ->deselectRecordsAfterCompletion()), ]), ]); } @@ -1491,17 +1456,15 @@ private static function restoreItemGroupedOptions(?int $backupSetId): array public static function createRestoreRun(array $data): RestoreRun { + UiEnforcement::for(Capabilities::TENANT_MANAGE)->authorizeOrAbort(); + /** @var Tenant $tenant */ $tenant = Tenant::current(); - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::TENANT_MANAGE, $tenant), 403); - /** @var BackupSet $backupSet */ - $backupSet = BackupSet::findOrFail($data['backup_set_id']); - - if ($backupSet->tenant_id !== $tenant->id) { - abort(403, 'Backup set does not belong to the active tenant.'); - } + $backupSet = BackupSet::query() + ->where('tenant_id', $tenant->getKey()) + ->findOrFail($data['backup_set_id']); /** @var RestoreService $service */ $service = app(RestoreService::class); diff --git a/app/Filament/Resources/RestoreRunResource/Pages/CreateRestoreRun.php b/app/Filament/Resources/RestoreRunResource/Pages/CreateRestoreRun.php index 19986f2..1ef6b34 100644 --- a/app/Filament/Resources/RestoreRunResource/Pages/CreateRestoreRun.php +++ b/app/Filament/Resources/RestoreRunResource/Pages/CreateRestoreRun.php @@ -6,11 +6,11 @@ use App\Models\BackupSet; use App\Models\Tenant; use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use Filament\Actions\Action; use Filament\Resources\Pages\Concerns\HasWizard; use Filament\Resources\Pages\CreateRecord; use Illuminate\Database\Eloquent\Model; -use Illuminate\Support\Facades\Gate; use Livewire\Attributes\On; class CreateRestoreRun extends CreateRecord @@ -21,9 +21,7 @@ class CreateRestoreRun extends CreateRecord protected function authorizeAccess(): void { - $tenant = Tenant::current(); - - abort_unless($tenant instanceof Tenant && Gate::allows(Capabilities::TENANT_MANAGE, $tenant), 403); + UiEnforcement::for(Capabilities::TENANT_MANAGE)->authorizeOrAbort(); } public function getSteps(): array diff --git a/app/Filament/Resources/RestoreRunResource/Pages/ListRestoreRuns.php b/app/Filament/Resources/RestoreRunResource/Pages/ListRestoreRuns.php index 1868ac5..fb11d2e 100644 --- a/app/Filament/Resources/RestoreRunResource/Pages/ListRestoreRuns.php +++ b/app/Filament/Resources/RestoreRunResource/Pages/ListRestoreRuns.php @@ -3,6 +3,8 @@ namespace App\Filament\Resources\RestoreRunResource\Pages; use App\Filament\Resources\RestoreRunResource; +use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use Filament\Actions; use Filament\Resources\Pages\ListRecords; @@ -13,9 +15,7 @@ class ListRestoreRuns extends ListRecords protected function getHeaderActions(): array { return [ - Actions\CreateAction::make() - ->disabled(fn (): bool => ! RestoreRunResource::canCreate()) - ->tooltip(fn (): ?string => RestoreRunResource::canCreate() ? null : 'You do not have permission to create restore runs.'), + UiEnforcement::for(Capabilities::TENANT_MANAGE)->apply(Actions\CreateAction::make()), ]; } } diff --git a/app/Filament/Resources/TenantResource.php b/app/Filament/Resources/TenantResource.php index b03a866..641c14b 100644 --- a/app/Filament/Resources/TenantResource.php +++ b/app/Filament/Resources/TenantResource.php @@ -20,6 +20,7 @@ use App\Services\OperationRunService; use App\Services\Operations\BulkSelectionIdentity; use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use App\Support\Badges\BadgeDomain; use App\Support\Badges\BadgeRenderer; use App\Support\Badges\TagBadgeDomain; @@ -43,7 +44,6 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Cache; -use Illuminate\Support\Facades\Gate; use Illuminate\Support\Facades\Log; use Illuminate\Support\Str; use Throwable; @@ -73,24 +73,16 @@ public static function canCreate(): bool public static function canEdit(Model $record): bool { - $user = auth()->user(); - - if (! $user instanceof User) { - return false; - } - - return Gate::forUser($user)->allows(Capabilities::TENANT_MANAGE, $record); + return UiEnforcement::for(Capabilities::TENANT_MANAGE) + ->tenantFromRecord() + ->isAllowed($record); } public static function canDelete(Model $record): bool { - $user = auth()->user(); - - if (! $user instanceof User) { - return false; - } - - return Gate::forUser($user)->allows(Capabilities::TENANT_DELETE, $record); + return UiEnforcement::for(Capabilities::TENANT_DELETE) + ->tenantFromRecord() + ->isAllowed($record); } public static function canDeleteAny(): bool @@ -106,36 +98,30 @@ public static function canDeleteAny(): bool private static function userCanManageAnyTenant(User $user): bool { - $tenantIds = $user->tenants()->withTrashed()->pluck('tenants.id'); + $roles = RoleCapabilityMap::rolesWithCapability(Capabilities::TENANT_MANAGE); - if ($tenantIds->isEmpty()) { + if ($roles === []) { return false; } - foreach (Tenant::query()->whereIn('id', $tenantIds)->cursor() as $tenant) { - if (Gate::forUser($user)->allows(Capabilities::TENANT_MANAGE, $tenant)) { - return true; - } - } - - return false; + return $user->tenants() + ->withTrashed() + ->wherePivotIn('role', $roles) + ->exists(); } private static function userCanDeleteAnyTenant(User $user): bool { - $tenantIds = $user->tenants()->withTrashed()->pluck('tenants.id'); + $roles = RoleCapabilityMap::rolesWithCapability(Capabilities::TENANT_DELETE); - if ($tenantIds->isEmpty()) { + if ($roles === []) { return false; } - foreach (Tenant::query()->whereIn('id', $tenantIds)->cursor() as $tenant) { - if (Gate::forUser($user)->allows(Capabilities::TENANT_DELETE, $tenant)) { - return true; - } - } - - return false; + return $user->tenants() + ->withTrashed() + ->wherePivotIn('role', $roles) + ->exists(); } public static function form(Schema $schema): Schema @@ -274,49 +260,19 @@ public static function table(Table $table): Table ->label('View') ->icon('heroicon-o-eye') ->url(fn (Tenant $record) => static::getUrl('view', ['record' => $record], tenant: $record)), - Actions\Action::make('syncTenant') + UiEnforcement::for(Capabilities::TENANT_SYNC)->tenantFromRecord()->apply(Actions\Action::make('syncTenant') ->label('Sync') ->icon('heroicon-o-arrow-path') ->color('warning') ->requiresConfirmation() - ->visible(function (Tenant $record): bool { - if (! $record->isActive()) { - return false; - } - - $user = auth()->user(); - - if (! $user instanceof User) { - return false; - } - - return $user->canAccessTenant($record); - }) - ->disabled(function (Tenant $record): bool { - $user = auth()->user(); - - if (! $user instanceof User) { - return true; - } - - return ! Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $record); - }) - ->tooltip(function (Tenant $record): ?string { - $user = auth()->user(); - - if (! $user instanceof User) { - return null; - } - - return Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $record) - ? null - : 'You do not have permission to sync this tenant.'; - }) + ->visible(fn (Tenant $record): bool => $record->isActive()) ->action(function (Tenant $record, AuditLogger $auditLogger, \Filament\Tables\Contracts\HasTable $livewire): void { + UiEnforcement::for(Capabilities::TENANT_SYNC) + ->tenantFromRecord() + ->authorizeOrAbort($record); + + /** @var User $user */ $user = auth()->user(); - abort_unless($user instanceof User, 403); - abort_unless($user->canAccessTenant($record), 404); - abort_unless(Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $record), 403); /** @var OperationRunService $opService */ $opService = app(OperationRunService::class); @@ -337,7 +293,7 @@ public static function table(Table $table): Table tenant: $record, type: 'policy.sync', inputs: $inputs, - initiator: auth()->user() + initiator: $user ); if (! $opRun->wasRecentlyCreated && $opService->isStaleQueuedRun($opRun)) { @@ -350,7 +306,7 @@ public static function table(Table $table): Table tenant: $record, type: 'policy.sync', inputs: $inputs, - initiator: auth()->user() + initiator: $user ); } @@ -390,44 +346,29 @@ public static function table(Table $table): Table ->url(OperationRunLinks::view($opRun, $record)), ]) ->send(); - }), + })), Actions\Action::make('openTenant') ->label('Open') ->icon('heroicon-o-arrow-right') ->color('primary') ->url(fn (Tenant $record) => \App\Filament\Resources\PolicyResource::getUrl('index', tenant: $record)) ->visible(fn (Tenant $record) => $record->isActive()), - Actions\Action::make('edit') - ->label('Edit') - ->icon('heroicon-o-pencil-square') - ->url(fn (Tenant $record) => static::getUrl('edit', ['record' => $record], tenant: $record)) - ->disabled(fn (Tenant $record): bool => ! static::canEdit($record)) - ->tooltip(fn (Tenant $record): ?string => static::canEdit($record) ? null : 'You do not have permission to edit this tenant.'), - Actions\Action::make('restore') + UiEnforcement::for(Capabilities::TENANT_MANAGE)->tenantFromRecord()->apply( + Actions\Action::make('edit') + ->label('Edit') + ->icon('heroicon-o-pencil-square') + ->url(fn (Tenant $record) => static::getUrl('edit', ['record' => $record], tenant: $record)), + ), + UiEnforcement::for(Capabilities::TENANT_DELETE)->tenantFromRecord()->apply(Actions\Action::make('restore') ->label('Restore') ->color('success') ->successNotificationTitle('Tenant reactivated') ->requiresConfirmation() ->visible(fn (Tenant $record): bool => $record->trashed()) - ->disabled(function (Tenant $record): bool { - $user = auth()->user(); - - if (! $user instanceof User) { - return true; - } - - return ! Gate::forUser($user)->allows(Capabilities::TENANT_DELETE, $record); - }) ->action(function (Tenant $record, AuditLogger $auditLogger): void { - $user = auth()->user(); - - if (! $user instanceof User) { - abort(403); - } - - if (! Gate::forUser($user)->allows(Capabilities::TENANT_DELETE, $record)) { - abort(403); - } + UiEnforcement::for(Capabilities::TENANT_DELETE) + ->tenantFromRecord() + ->authorizeOrAbort($record); $record->restore(); @@ -439,54 +380,25 @@ public static function table(Table $table): Table status: 'success', context: ['metadata' => ['tenant_id' => $record->tenant_id]] ); - }), - Actions\Action::make('admin_consent') + })), + UiEnforcement::for(Capabilities::TENANT_MANAGE)->tenantFromRecord()->apply(Actions\Action::make('admin_consent') ->label('Admin consent') ->icon('heroicon-o-clipboard-document') ->url(fn (Tenant $record) => static::adminConsentUrl($record)) ->visible(fn (Tenant $record) => static::adminConsentUrl($record) !== null) - ->disabled(function (Tenant $record): bool { - $user = auth()->user(); - - if (! $user instanceof User) { - return true; - } - - return ! Gate::forUser($user)->allows(Capabilities::TENANT_MANAGE, $record); - }) - ->tooltip(function (Tenant $record): ?string { - $user = auth()->user(); - - if (! $user instanceof User) { - return null; - } - - return Gate::forUser($user)->allows(Capabilities::TENANT_MANAGE, $record) - ? null - : 'You do not have permission to manage tenant consent.'; - }) - ->openUrlInNewTab(), + ->openUrlInNewTab()), Actions\Action::make('open_in_entra') ->label('Open in Entra') ->icon('heroicon-o-arrow-top-right-on-square') ->url(fn (Tenant $record) => static::entraUrl($record)) ->visible(fn (Tenant $record) => static::entraUrl($record) !== null) ->openUrlInNewTab(), - Actions\Action::make('verify') + UiEnforcement::for(Capabilities::TENANT_MANAGE)->tenantFromRecord()->apply(Actions\Action::make('verify') ->label('Verify configuration') ->icon('heroicon-o-check-badge') ->color('primary') ->requiresConfirmation() ->visible(fn (Tenant $record): bool => $record->isActive()) - ->disabled(function (Tenant $record): bool { - $user = auth()->user(); - - if (! $user instanceof User) { - return true; - } - - return ! Gate::forUser($user)->allows(Capabilities::TENANT_MANAGE, $record); - }) ->action(function ( Tenant $record, TenantConfigService $configService, @@ -494,44 +406,23 @@ public static function table(Table $table): Table RbacHealthService $rbacHealthService, AuditLogger $auditLogger ) { - $user = auth()->user(); - - if (! $user instanceof User) { - abort(403); - } - - if (! Gate::forUser($user)->allows(Capabilities::TENANT_MANAGE, $record)) { - abort(403); - } + UiEnforcement::for(Capabilities::TENANT_MANAGE) + ->tenantFromRecord() + ->authorizeOrAbort($record); static::verifyTenant($record, $configService, $permissionService, $rbacHealthService, $auditLogger); - }), + })), static::rbacAction(), - Actions\Action::make('archive') + UiEnforcement::for(Capabilities::TENANT_DELETE)->tenantFromRecord()->apply(Actions\Action::make('archive') ->label('Deactivate') ->color('danger') ->icon('heroicon-o-archive-box-x-mark') ->requiresConfirmation() ->visible(fn (Tenant $record): bool => ! $record->trashed()) - ->disabled(function (Tenant $record): bool { - $user = auth()->user(); - - if (! $user instanceof User) { - return true; - } - - return ! Gate::forUser($user)->allows(Capabilities::TENANT_DELETE, $record); - }) ->action(function (Tenant $record, AuditLogger $auditLogger) { - $user = auth()->user(); - - if (! $user instanceof User) { - abort(403); - } - - if (! Gate::forUser($user)->allows(Capabilities::TENANT_DELETE, $record)) { - abort(403); - } + UiEnforcement::for(Capabilities::TENANT_DELETE) + ->tenantFromRecord() + ->authorizeOrAbort($record); $record->delete(); @@ -549,40 +440,21 @@ public static function table(Table $table): Table ->body('The tenant has been archived and hidden from lists.') ->success() ->send(); - }), - Actions\Action::make('forceDelete') + })), + UiEnforcement::for(Capabilities::TENANT_DELETE)->tenantFromRecord()->apply(Actions\Action::make('forceDelete') ->label('Force delete') ->color('danger') ->icon('heroicon-o-trash') ->requiresConfirmation() ->visible(fn (?Tenant $record): bool => (bool) $record?->trashed()) - ->disabled(function (?Tenant $record): bool { - if (! $record instanceof Tenant) { - return true; - } - - $user = auth()->user(); - - if (! $user instanceof User) { - return true; - } - - return ! Gate::forUser($user)->allows(Capabilities::TENANT_DELETE, $record); - }) ->action(function (?Tenant $record, AuditLogger $auditLogger) { if ($record === null) { return; } - $user = auth()->user(); - - if (! $user instanceof User) { - abort(403); - } - - if (! Gate::forUser($user)->allows(Capabilities::TENANT_DELETE, $record)) { - abort(403); - } + UiEnforcement::for(Capabilities::TENANT_DELETE) + ->tenantFromRecord() + ->authorizeOrAbort($record); $tenant = Tenant::withTrashed()->find($record->id); @@ -610,107 +482,100 @@ public static function table(Table $table): Table ->title('Tenant permanently deleted') ->success() ->send(); - }), + })), ]), ]) ->bulkActions([ - Actions\BulkAction::make('syncSelected') - ->label('Sync selected') - ->icon('heroicon-o-arrow-path') - ->color('warning') - ->requiresConfirmation() - ->visible(function (): bool { - $user = auth()->user(); + UiEnforcement::for(Capabilities::TENANT_SYNC) + ->tenantFromRecord() + ->preflightByCapability() + ->apply(Actions\BulkAction::make('syncSelected') + ->label('Sync selected') + ->icon('heroicon-o-arrow-path') + ->color('warning') + ->requiresConfirmation() + ->action(function (Collection $records, AuditLogger $auditLogger): void { + UiEnforcement::for(Capabilities::TENANT_SYNC) + ->tenantFromRecord() + ->authorizeBulkSelectionOrAbort($records); - if (! $user instanceof User) { - return false; - } + /** @var User $user */ + $user = auth()->user(); - return $user->tenants() - ->whereIn('role', RoleCapabilityMap::rolesWithCapability(Capabilities::TENANT_SYNC)) - ->exists(); - }) - ->authorize(function (): bool { - $user = auth()->user(); + $eligible = $records + ->filter(fn ($record) => $record instanceof Tenant && $record->isActive()); - if (! $user instanceof User) { - return false; - } + if ($eligible->isEmpty()) { + Notification::make() + ->title('Bulk sync skipped') + ->body('No eligible tenants selected.') + ->icon('heroicon-o-information-circle') + ->info() + ->sendToDatabase($user) + ->send(); - return $user->tenants() - ->whereIn('role', RoleCapabilityMap::rolesWithCapability(Capabilities::TENANT_SYNC)) - ->exists(); - }) - ->action(function (Collection $records, AuditLogger $auditLogger): void { - $user = auth()->user(); + return; + } - if (! $user instanceof User) { - return; - } + if ($eligible->count() !== $records->count()) { + $skipped = $records->count() - $eligible->count(); + $total = $records->count(); - $eligible = $records - ->filter(fn ($record) => $record instanceof Tenant && $record->isActive()) - ->filter(fn (Tenant $tenant) => Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $tenant)); + Notification::make() + ->title('Some tenants were skipped') + ->body("Skipped {$skipped} of {$total} selected tenants (inactive).") + ->warning() + ->sendToDatabase($user) + ->send(); + } - if ($eligible->isEmpty()) { - Notification::make() - ->title('Bulk sync skipped') - ->body('No eligible tenants selected.') - ->icon('heroicon-o-information-circle') - ->info() - ->sendToDatabase($user) + $tenantContext = Tenant::current() ?? $eligible->first(); + + if (! $tenantContext) { + return; + } + + $ids = $eligible->pluck('id')->toArray(); + $count = $eligible->count(); + + /** @var BulkSelectionIdentity $selection */ + $selection = app(BulkSelectionIdentity::class); + $selectionIdentity = $selection->fromIds($ids); + + /** @var OperationRunService $runs */ + $runs = app(OperationRunService::class); + + $opRun = $runs->enqueueBulkOperation( + tenant: $tenantContext, + type: 'tenant.sync', + targetScope: [ + 'entra_tenant_id' => (string) ($tenantContext->tenant_id ?? $tenantContext->external_id), + ], + selectionIdentity: $selectionIdentity, + dispatcher: function ($operationRun) use ($tenantContext, $user, $ids): void { + BulkTenantSyncJob::dispatch( + tenantId: (int) $tenantContext->getKey(), + userId: (int) $user->getKey(), + tenantIds: $ids, + operationRun: $operationRun, + ); + }, + initiator: $user, + extraContext: [ + 'tenant_count' => $count, + ], + emitQueuedNotification: false, + ); + + OperationUxPresenter::queuedToast('tenant.sync') + ->actions([ + Actions\Action::make('view_run') + ->label('View run') + ->url(OperationRunLinks::view($opRun, $tenantContext)), + ]) ->send(); - - return; - } - - $tenantContext = Tenant::current() ?? $eligible->first(); - - if (! $tenantContext) { - return; - } - - $ids = $eligible->pluck('id')->toArray(); - $count = $eligible->count(); - - /** @var BulkSelectionIdentity $selection */ - $selection = app(BulkSelectionIdentity::class); - $selectionIdentity = $selection->fromIds($ids); - - /** @var OperationRunService $runs */ - $runs = app(OperationRunService::class); - - $opRun = $runs->enqueueBulkOperation( - tenant: $tenantContext, - type: 'tenant.sync', - targetScope: [ - 'entra_tenant_id' => (string) ($tenantContext->tenant_id ?? $tenantContext->external_id), - ], - selectionIdentity: $selectionIdentity, - dispatcher: function ($operationRun) use ($tenantContext, $user, $ids): void { - BulkTenantSyncJob::dispatch( - tenantId: (int) $tenantContext->getKey(), - userId: (int) $user->getKey(), - tenantIds: $ids, - operationRun: $operationRun, - ); - }, - initiator: $user, - extraContext: [ - 'tenant_count' => $count, - ], - emitQueuedNotification: false, - ); - - OperationUxPresenter::queuedToast('tenant.sync') - ->actions([ - Actions\Action::make('view_run') - ->label('View run') - ->url(OperationRunLinks::view($opRun, $tenantContext)), - ]) - ->send(); - }) - ->deselectRecordsAfterCompletion(), + }) + ->deselectRecordsAfterCompletion()), ]) ->headerActions([]); } @@ -803,7 +668,7 @@ public static function getRelations(): array public static function rbacAction(): Actions\Action { // ... [RBAC Action Omitted - No Change] ... - return Actions\Action::make('setup_rbac') + return UiEnforcement::for(Capabilities::TENANT_MANAGE)->tenantFromRecord()->apply(Actions\Action::make('setup_rbac') ->label('Setup Intune RBAC') ->icon('heroicon-o-shield-check') ->color('primary') @@ -886,15 +751,6 @@ public static function rbacAction(): Actions\Action ->loadingMessage('Searching groups...'), ]) ->visible(fn (Tenant $record): bool => $record->isActive()) - ->disabled(function (Tenant $record): bool { - $user = auth()->user(); - - if (! $user instanceof User) { - return true; - } - - return ! Gate::forUser($user)->allows(Capabilities::TENANT_MANAGE, $record); - }) ->requiresConfirmation() ->action(function ( array $data, @@ -902,15 +758,9 @@ public static function rbacAction(): Actions\Action RbacOnboardingService $service, AuditLogger $auditLogger ) { - $user = auth()->user(); - - if (! $user instanceof User) { - abort(403); - } - - if (! Gate::forUser($user)->allows(Capabilities::TENANT_MANAGE, $record)) { - abort(403); - } + UiEnforcement::for(Capabilities::TENANT_MANAGE) + ->tenantFromRecord() + ->authorizeOrAbort($record); $cacheKey = RbacDelegatedAuthController::cacheKey($record, auth()->id(), session()->getId()); $token = Cache::get($cacheKey); @@ -989,7 +839,7 @@ public static function rbacAction(): Actions\Action ->body($result['message'] ?? 'Unknown error') ->danger() ->send(); - }); + })); } public static function adminConsentUrl(Tenant $tenant): ?string diff --git a/app/Filament/Resources/TenantResource/Pages/EditTenant.php b/app/Filament/Resources/TenantResource/Pages/EditTenant.php index 19117cf..173a4c0 100644 --- a/app/Filament/Resources/TenantResource/Pages/EditTenant.php +++ b/app/Filament/Resources/TenantResource/Pages/EditTenant.php @@ -4,11 +4,10 @@ use App\Filament\Resources\TenantResource; use App\Models\Tenant; -use App\Models\User; use App\Support\Auth\Capabilities; +use App\Support\Auth\UiEnforcement; use Filament\Actions; use Filament\Resources\Pages\EditRecord; -use Illuminate\Support\Facades\Gate; class EditTenant extends EditRecord { @@ -18,42 +17,24 @@ protected function getHeaderActions(): array { return [ Actions\ViewAction::make(), - Actions\Action::make('archive') - ->label('Archive') - ->color('danger') - ->requiresConfirmation() - ->visible(fn (): bool => $this->record instanceof Tenant && ! $this->record->trashed()) - ->disabled(function (): bool { - $tenant = $this->record; - $user = auth()->user(); + UiEnforcement::for(Capabilities::TENANT_DELETE) + ->tenantFromRecord() + ->apply( + Actions\Action::make('archive') + ->label('Archive') + ->color('danger') + ->requiresConfirmation() + ->visible(fn (): bool => $this->record instanceof Tenant && ! $this->record->trashed()) + ->action(function (): void { + $tenant = $this->record; - if (! $tenant instanceof Tenant || ! $user instanceof User) { - return true; - } + UiEnforcement::for(Capabilities::TENANT_DELETE) + ->tenantFromRecord() + ->authorizeOrAbort($tenant); - return Gate::forUser($user)->denies(Capabilities::TENANT_DELETE, $tenant); - }) - ->tooltip(function (): ?string { - $tenant = $this->record; - $user = auth()->user(); - - if (! $tenant instanceof Tenant || ! $user instanceof User) { - return null; - } - - return Gate::forUser($user)->allows(Capabilities::TENANT_DELETE, $tenant) - ? null - : 'You do not have permission to archive tenants.'; - }) - ->action(function (): void { - $tenant = $this->record; - $user = auth()->user(); - - abort_unless($tenant instanceof Tenant && $user instanceof User, 403); - abort_unless(Gate::forUser($user)->allows(Capabilities::TENANT_DELETE, $tenant), 403); - - $tenant->delete(); - }), + $tenant->delete(); + }), + ), ]; } } diff --git a/app/Support/Auth/UiEnforcement.php b/app/Support/Auth/UiEnforcement.php new file mode 100644 index 0000000..9686556 --- /dev/null +++ b/app/Support/Auth/UiEnforcement.php @@ -0,0 +1,526 @@ +): bool|null + */ + private ?\Closure $bulkPreflight = null; + + public function __construct(private string $capability) + { + } + + public static function for(string $capability): self + { + return new self($capability); + } + + public function preserveVisibility(): self + { + if ($this->tenantResolverMode !== self::TENANT_RESOLVER_FILAMENT) { + throw new LogicException('preserveVisibility() is allowed only for tenant-scoped (tenantFromFilament) surfaces.'); + } + + $this->preserveVisibility = true; + + return $this; + } + + public function andVisibleWhen(callable $businessVisible): self + { + $this->businessVisible = \Closure::fromCallable($businessVisible); + + return $this; + } + + public function andHiddenWhen(callable $businessHidden): self + { + $this->businessHidden = \Closure::fromCallable($businessHidden); + + return $this; + } + + public function tenantFromFilament(): self + { + $this->tenantResolverMode = self::TENANT_RESOLVER_FILAMENT; + $this->customTenantResolver = null; + + return $this; + } + + public function tenantFromRecord(): self + { + if ($this->preserveVisibility) { + throw new LogicException('preserveVisibility() is forbidden for record-scoped surfaces.'); + } + + $this->tenantResolverMode = self::TENANT_RESOLVER_RECORD; + $this->customTenantResolver = null; + + return $this; + } + + public function tenantFrom(callable $resolver): self + { + if ($this->preserveVisibility) { + throw new LogicException('preserveVisibility() is forbidden for record-scoped surfaces.'); + } + + $this->tenantResolverMode = self::TENANT_RESOLVER_CUSTOM; + $this->customTenantResolver = \Closure::fromCallable($resolver); + + return $this; + } + + /** + * Custom bulk authorization preflight for selection. + * + * Signature: fn (Collection $records): bool + */ + public function preflightSelection(callable $preflight): self + { + $this->bulkPreflightMode = self::BULK_PREFLIGHT_CUSTOM; + $this->bulkPreflight = \Closure::fromCallable($preflight); + + return $this; + } + + public function preflightByTenantMembership(): self + { + $this->bulkPreflightMode = self::BULK_PREFLIGHT_TENANT_MEMBERSHIP; + $this->bulkPreflight = null; + + return $this; + } + + public function preflightByCapability(): self + { + $this->bulkPreflightMode = self::BULK_PREFLIGHT_CAPABILITY; + $this->bulkPreflight = null; + + return $this; + } + + public function apply(Action $action): Action + { + $this->assertMixedVisibilityConfigIsValid(); + + if (! $this->preserveVisibility) { + $this->applyVisibility($action); + } + + if ($action->isBulk()) { + $action->disabled(function () use ($action): bool { + /** @var Collection $records */ + $records = collect($action->getSelectedRecords()); + + return $this->bulkIsDisabled($records); + }); + + $action->tooltip(function () use ($action): ?string { + /** @var Collection $records */ + $records = collect($action->getSelectedRecords()); + + return $this->bulkDisabledTooltip($records); + }); + } else { + $action->disabled(fn (?Model $record = null): bool => $this->isDisabled($record)); + $action->tooltip(fn (?Model $record = null): ?string => $this->disabledTooltip($record)); + } + + return $action; + } + + public function isAllowed(?Model $record = null): bool + { + return ! $this->isDisabled($record); + } + + public function authorizeOrAbort(?Model $record = null): void + { + $user = auth()->user(); + abort_unless($user instanceof User, 403); + + $tenant = $this->resolveTenant($record); + + if (! ($tenant instanceof Tenant)) { + abort(404); + } + + abort_unless($this->isMemberOfTenant($user, $tenant), 404); + abort_unless(Gate::forUser($user)->allows($this->capability, $tenant), 403); + } + + /** + * Server-side enforcement for bulk selections. + * + * - If any selected tenant is not a membership: 404 (deny-as-not-found). + * - If all are memberships but any lacks capability: 403. + * + * @param Collection $records + */ + public function authorizeBulkSelectionOrAbort(Collection $records): void + { + $user = auth()->user(); + abort_unless($user instanceof User, 403); + + $tenantIds = $this->resolveTenantIdsForRecords($records); + + if ($tenantIds === []) { + abort(403); + } + + $membershipTenantIds = $this->membershipTenantIds($user, $tenantIds); + + if (count($membershipTenantIds) !== count($tenantIds)) { + abort(404); + } + + $allowedTenantIds = $this->capabilityTenantIds($user, $tenantIds); + + if (count($allowedTenantIds) !== count($tenantIds)) { + abort(403); + } + } + + /** + * Public helper for evaluating bulk selection authorization decisions. + * + * @param Collection $records + */ + public function bulkSelectionIsAuthorized(User $user, Collection $records): bool + { + return $this->bulkSelectionIsAuthorizedInternal($user, $records); + } + + private function applyVisibility(Action $action): void + { + $canApplyMemberVisibility = ! ($action->isBulk() && $this->tenantResolverMode !== self::TENANT_RESOLVER_FILAMENT); + + $businessVisible = $this->businessVisible; + $businessHidden = $this->businessHidden; + + if ($businessVisible instanceof \Closure) { + $action->visible(function () use ($action, $businessVisible, $canApplyMemberVisibility): bool { + if (! (bool) $action->evaluate($businessVisible)) { + return false; + } + + if (! $canApplyMemberVisibility) { + return true; + } + + $record = $action->getRecord(); + + return $this->isMember($record instanceof Model ? $record : null); + }); + } + + if ($businessHidden instanceof \Closure) { + $action->hidden(function () use ($action, $businessHidden, $canApplyMemberVisibility): bool { + if ($canApplyMemberVisibility) { + $record = $action->getRecord(); + + if (! $this->isMember($record instanceof Model ? $record : null)) { + return true; + } + } + + return (bool) $action->evaluate($businessHidden); + }); + + return; + } + + if (! $canApplyMemberVisibility) { + return; + } + + if (! ($businessVisible instanceof \Closure)) { + $action->hidden(function () use ($action): bool { + $record = $action->getRecord(); + + return ! $this->isMember($record instanceof Model ? $record : null); + }); + } + } + + private function assertMixedVisibilityConfigIsValid(): void + { + if ($this->preserveVisibility && ($this->businessVisible instanceof \Closure || $this->businessHidden instanceof \Closure)) { + throw new LogicException('preserveVisibility() cannot be combined with andVisibleWhen()/andHiddenWhen().'); + } + + if ($this->preserveVisibility && $this->tenantResolverMode !== self::TENANT_RESOLVER_FILAMENT) { + throw new LogicException('preserveVisibility() is allowed only for tenant-scoped (tenantFromFilament) surfaces.'); + } + } + + private function isDisabled(?Model $record = null): bool + { + $user = auth()->user(); + + if (! ($user instanceof User)) { + return true; + } + + $tenant = $this->resolveTenant($record); + + if (! ($tenant instanceof Tenant)) { + return true; + } + + if (! $this->isMemberOfTenant($user, $tenant)) { + return true; + } + + return ! Gate::forUser($user)->allows($this->capability, $tenant); + } + + private function disabledTooltip(?Model $record = null): ?string + { + $user = auth()->user(); + + if (! ($user instanceof User)) { + return null; + } + + $tenant = $this->resolveTenant($record); + + if (! ($tenant instanceof Tenant)) { + return null; + } + + if (! $this->isMemberOfTenant($user, $tenant)) { + return null; + } + + if (Gate::forUser($user)->allows($this->capability, $tenant)) { + return null; + } + + return UiTooltips::insufficientPermission(); + } + + private function bulkIsDisabled(Collection $records): bool + { + $user = auth()->user(); + + if (! ($user instanceof User)) { + return true; + } + + return ! $this->bulkSelectionIsAuthorizedInternal($user, $records); + } + + private function bulkDisabledTooltip(Collection $records): ?string + { + $user = auth()->user(); + + if (! ($user instanceof User)) { + return null; + } + + if ($this->bulkSelectionIsAuthorizedInternal($user, $records)) { + return null; + } + + return UiTooltips::insufficientPermission(); + } + + private function bulkSelectionIsAuthorizedInternal(User $user, Collection $records): bool + { + if ($this->bulkPreflightMode === self::BULK_PREFLIGHT_CUSTOM && $this->bulkPreflight instanceof \Closure) { + return (bool) ($this->bulkPreflight)($records); + } + + $tenantIds = $this->resolveTenantIdsForRecords($records); + + if ($tenantIds === []) { + return false; + } + + return match ($this->bulkPreflightMode) { + self::BULK_PREFLIGHT_TENANT_MEMBERSHIP => count($this->membershipTenantIds($user, $tenantIds)) === count($tenantIds), + self::BULK_PREFLIGHT_CAPABILITY => count($this->capabilityTenantIds($user, $tenantIds)) === count($tenantIds), + default => false, + }; + } + + /** + * @param Collection $records + * @return array + */ + private function resolveTenantIdsForRecords(Collection $records): array + { + if ($this->tenantResolverMode === self::TENANT_RESOLVER_FILAMENT) { + $tenant = Filament::getTenant(); + + return $tenant instanceof Tenant ? [(int) $tenant->getKey()] : []; + } + + if ($this->tenantResolverMode === self::TENANT_RESOLVER_RECORD) { + $ids = $records + ->filter(fn (Model $record): bool => $record instanceof Tenant) + ->map(fn (Tenant $tenant): int => (int) $tenant->getKey()) + ->all(); + + return array_values(array_unique($ids)); + } + + if ($this->tenantResolverMode === self::TENANT_RESOLVER_CUSTOM && $this->customTenantResolver instanceof \Closure) { + $ids = []; + + foreach ($records as $record) { + if (! ($record instanceof Model)) { + continue; + } + + $resolved = ($this->customTenantResolver)($record); + + if ($resolved instanceof Tenant) { + $ids[] = (int) $resolved->getKey(); + continue; + } + + if (is_int($resolved)) { + $ids[] = $resolved; + } + } + + return array_values(array_unique($ids)); + } + + return []; + } + + private function isMember(?Model $record = null): bool + { + $user = auth()->user(); + + if (! ($user instanceof User)) { + return false; + } + + $tenant = $this->resolveTenant($record); + + if (! ($tenant instanceof Tenant)) { + return false; + } + + return $this->isMemberOfTenant($user, $tenant); + } + + private function isMemberOfTenant(User $user, Tenant $tenant): bool + { + return Gate::forUser($user)->allows(Capabilities::TENANT_VIEW, $tenant); + } + + private function resolveTenant(?Model $record = null): ?Tenant + { + return match ($this->tenantResolverMode) { + self::TENANT_RESOLVER_FILAMENT => Filament::getTenant() instanceof Tenant ? Filament::getTenant() : null, + self::TENANT_RESOLVER_RECORD => $record instanceof Tenant ? $record : null, + self::TENANT_RESOLVER_CUSTOM => $this->resolveTenantViaCustomResolver($record), + default => null, + }; + } + + private function resolveTenantViaCustomResolver(?Model $record): ?Tenant + { + if (! ($this->customTenantResolver instanceof \Closure)) { + return null; + } + + if (! ($record instanceof Model)) { + return null; + } + + $resolved = ($this->customTenantResolver)($record); + + if ($resolved instanceof Tenant) { + return $resolved; + } + + return null; + } + + /** + * @param array $tenantIds + * @return array + */ + private function membershipTenantIds(User $user, array $tenantIds): array + { + /** @var array $ids */ + $ids = DB::table('tenant_memberships') + ->where('user_id', (int) $user->getKey()) + ->whereIn('tenant_id', $tenantIds) + ->pluck('tenant_id') + ->map(fn ($id): int => (int) $id) + ->all(); + + return array_values(array_unique($ids)); + } + + /** + * @param array $tenantIds + * @return array + */ + private function capabilityTenantIds(User $user, array $tenantIds): array + { + $roles = RoleCapabilityMap::rolesWithCapability($this->capability); + + if ($roles === []) { + return []; + } + + /** @var array $ids */ + $ids = DB::table('tenant_memberships') + ->where('user_id', (int) $user->getKey()) + ->whereIn('tenant_id', $tenantIds) + ->whereIn('role', $roles) + ->pluck('tenant_id') + ->map(fn ($id): int => (int) $id) + ->all(); + + return array_values(array_unique($ids)); + } +} diff --git a/app/Support/Auth/UiTooltips.php b/app/Support/Auth/UiTooltips.php new file mode 100644 index 0000000..05dd60a --- /dev/null +++ b/app/Support/Auth/UiTooltips.php @@ -0,0 +1,14 @@ +create(); $tenant->makeCurrent(); + Filament::setTenant($tenant, true); [$user] = createUserWithTenant(tenant: $tenant, role: 'owner'); diff --git a/tests/Feature/Filament/BackupSetUiEnforcementTest.php b/tests/Feature/Filament/BackupSetUiEnforcementTest.php new file mode 100644 index 0000000..5dcb540 --- /dev/null +++ b/tests/Feature/Filament/BackupSetUiEnforcementTest.php @@ -0,0 +1,71 @@ +create(); + $otherTenant = Tenant::factory()->create(); + + [$user] = createUserWithTenant($otherTenant, role: 'owner'); + + $this->actingAs($user) + ->get(BackupSetResource::getUrl('index', tenant: $tenant)) + ->assertStatus(404); +}); + +test('members without capability see BackupSet actions disabled with standard tooltip and cannot execute', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'readonly'); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'status' => 'completed', + 'deleted_at' => null, + ]); + + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListBackupSets::class) + ->assertTableActionDisabled('archive', $backupSet) + ->assertTableActionExists('archive', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $backupSet) + ->callTableAction('archive', $backupSet); + + expect($backupSet->fresh()->trashed())->toBeFalse(); +}); + +test('members with capability can execute BackupSet actions', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'owner'); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'status' => 'completed', + 'deleted_at' => null, + ]); + + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListBackupSets::class) + ->assertTableActionEnabled('archive', $backupSet) + ->callTableAction('archive', $backupSet); + + expect($backupSet->fresh()->trashed())->toBeTrue(); +}); + diff --git a/tests/Feature/Filament/EntraGroupSyncRunResourceTest.php b/tests/Feature/Filament/EntraGroupSyncRunResourceTest.php index e696853..b0737c2 100644 --- a/tests/Feature/Filament/EntraGroupSyncRunResourceTest.php +++ b/tests/Feature/Filament/EntraGroupSyncRunResourceTest.php @@ -7,13 +7,19 @@ use App\Models\Tenant; use App\Models\User; use App\Notifications\RunStatusChangedNotification; +use App\Support\Auth\UiTooltips; use Filament\Facades\Filament; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Queue; use Livewire\Livewire; uses(RefreshDatabase::class); +beforeEach(function (): void { + Http::preventStrayRequests(); +}); + test('entra group sync runs are listed for the active tenant', function () { $tenant = Tenant::factory()->create(); $otherTenant = Tenant::factory()->create(); @@ -96,3 +102,22 @@ expect($notification->data['actions'][0]['url'] ?? null) ->toBe(EntraGroupSyncRunResource::getUrl('view', ['record' => $run->getKey()], tenant: $tenant)); }); + +test('sync groups action is disabled for readonly users with standard tooltip', function () { + Queue::fake(); + + [$user, $tenant] = createUserWithTenant(role: 'readonly'); + + $this->actingAs($user); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListEntraGroupSyncRuns::class) + ->assertActionVisible('sync_groups') + ->assertActionDisabled('sync_groups') + ->assertActionExists('sync_groups', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission()); + + Queue::assertNothingPushed(); +}); diff --git a/tests/Feature/Filament/InventoryItemResourceTest.php b/tests/Feature/Filament/InventoryItemResourceTest.php index feae540..f46e5bd 100644 --- a/tests/Feature/Filament/InventoryItemResourceTest.php +++ b/tests/Feature/Filament/InventoryItemResourceTest.php @@ -1,12 +1,21 @@ create(); $otherTenant = Tenant::factory()->create(); @@ -39,3 +48,28 @@ ->assertSee('Item A') ->assertDontSee('Item B'); }); + +test('non-members are denied access to inventory item tenant routes (404)', function () { + $tenant = Tenant::factory()->create(); + $otherTenant = Tenant::factory()->create(); + + [$user] = createUserWithTenant($otherTenant, role: 'owner'); + + $this->actingAs($user) + ->get(InventoryItemResource::getUrl('index', tenant: $tenant)) + ->assertStatus(404); +}); + +test('members without capability see inventory sync action disabled with standard tooltip', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'readonly'); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListInventoryItems::class) + ->assertActionVisible('run_inventory_sync') + ->assertActionDisabled('run_inventory_sync') + ->assertActionExists('run_inventory_sync', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission()); +}); diff --git a/tests/Feature/Filament/InventorySyncRunResourceTest.php b/tests/Feature/Filament/InventorySyncRunResourceTest.php index 654eb3a..b17de43 100644 --- a/tests/Feature/Filament/InventorySyncRunResourceTest.php +++ b/tests/Feature/Filament/InventorySyncRunResourceTest.php @@ -4,9 +4,14 @@ use App\Models\InventorySyncRun; use App\Models\Tenant; use App\Models\User; +use Illuminate\Support\Facades\Http; uses(\Illuminate\Foundation\Testing\RefreshDatabase::class); +beforeEach(function (): void { + Http::preventStrayRequests(); +}); + test('inventory sync runs are listed for the active tenant', function () { $tenant = Tenant::factory()->create(); $otherTenant = Tenant::factory()->create(); @@ -35,3 +40,14 @@ ->assertSee(str_repeat('a', 12)) ->assertDontSee(str_repeat('b', 12)); }); + +test('non-members are denied access to inventory sync run tenant routes (404)', function () { + $tenant = Tenant::factory()->create(); + $otherTenant = Tenant::factory()->create(); + + [$user] = createUserWithTenant($otherTenant, role: 'owner'); + + $this->actingAs($user) + ->get(InventorySyncRunResource::getUrl('index', tenant: $tenant)) + ->assertStatus(404); +}); diff --git a/tests/Feature/Filament/ProviderConnectionsUiEnforcementTest.php b/tests/Feature/Filament/ProviderConnectionsUiEnforcementTest.php new file mode 100644 index 0000000..8b8fce9 --- /dev/null +++ b/tests/Feature/Filament/ProviderConnectionsUiEnforcementTest.php @@ -0,0 +1,77 @@ +create(); + $otherTenant = Tenant::factory()->create(); + + [$user] = createUserWithTenant($otherTenant, role: 'owner'); + + $this->actingAs($user) + ->get(ProviderConnectionResource::getUrl('index', tenant: $tenant)) + ->assertStatus(404); +}); + +test('members without capability see provider connection actions disabled with standard tooltip', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'readonly'); + + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'status' => 'needs_consent', + ]); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListProviderConnections::class) + ->assertTableActionVisible('check_connection', $connection) + ->assertTableActionDisabled('check_connection', $connection) + ->assertTableActionExists('check_connection', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $connection); + + Livewire::actingAs($user) + ->test(EditProviderConnection::class, ['record' => $connection->getKey()]) + ->assertActionVisible('check_connection') + ->assertActionDisabled('check_connection') + ->assertActionExists('check_connection', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission()); +}); + +test('members with capability can see provider connection actions enabled', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'owner'); + + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'status' => 'needs_consent', + ]); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListProviderConnections::class) + ->assertTableActionVisible('check_connection', $connection) + ->assertTableActionEnabled('check_connection', $connection); + + Livewire::actingAs($user) + ->test(EditProviderConnection::class, ['record' => $connection->getKey()]) + ->assertActionVisible('check_connection') + ->assertActionEnabled('check_connection'); +}); diff --git a/tests/Feature/Filament/RestoreRunUiEnforcementTest.php b/tests/Feature/Filament/RestoreRunUiEnforcementTest.php new file mode 100644 index 0000000..f438596 --- /dev/null +++ b/tests/Feature/Filament/RestoreRunUiEnforcementTest.php @@ -0,0 +1,83 @@ +create(); + $otherTenant = Tenant::factory()->create(); + + [$user] = createUserWithTenant($otherTenant, role: 'owner'); + + $this->actingAs($user) + ->get(RestoreRunResource::getUrl('index', tenant: $tenant)) + ->assertStatus(404); +}); + +test('members without capability see RestoreRun actions disabled with standard tooltip and cannot execute', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'readonly'); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'status' => 'completed', + ]); + + $restoreRun = RestoreRun::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'backup_set_id' => $backupSet->getKey(), + 'status' => 'completed', + 'deleted_at' => null, + ]); + + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListRestoreRuns::class) + ->assertTableActionDisabled('archive', $restoreRun) + ->assertTableActionExists('archive', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $restoreRun) + ->callTableAction('archive', $restoreRun); + + expect($restoreRun->fresh()->trashed())->toBeFalse(); +}); + +test('members with capability can execute RestoreRun actions', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'owner'); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'status' => 'completed', + ]); + + $restoreRun = RestoreRun::factory()->create([ + 'tenant_id' => $tenant->getKey(), + 'backup_set_id' => $backupSet->getKey(), + 'status' => 'completed', + 'deleted_at' => null, + ]); + + Filament::setTenant($tenant, true); + + Livewire::actingAs($user) + ->test(ListRestoreRuns::class) + ->assertTableActionEnabled('archive', $restoreRun) + ->callTableAction('archive', $restoreRun); + + expect($restoreRun->fresh()->trashed())->toBeTrue(); +}); + diff --git a/tests/Feature/Filament/TenantActionsAuthorizationTest.php b/tests/Feature/Filament/TenantActionsAuthorizationTest.php index fad1880..e3233ec 100644 --- a/tests/Feature/Filament/TenantActionsAuthorizationTest.php +++ b/tests/Feature/Filament/TenantActionsAuthorizationTest.php @@ -4,13 +4,19 @@ use App\Filament\Pages\TenantDashboard; use App\Filament\Resources\TenantResource\Pages\ListTenants; use App\Models\Tenant; +use App\Support\Auth\UiTooltips; use Filament\Facades\Filament; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Gate; use Livewire\Livewire; uses(RefreshDatabase::class); +beforeEach(function (): void { + Http::preventStrayRequests(); +}); + test('readonly users may switch current tenant via ChooseTenant', function () { [$user, $tenantA] = createUserWithTenant(role: 'readonly'); @@ -57,6 +63,7 @@ Livewire::actingAs($user) ->test(ListTenants::class) ->assertTableActionDisabled('archive', $tenant) + ->assertTableActionExists('archive', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $tenant) ->callTableAction('archive', $tenant); expect($tenant->fresh()->trashed())->toBeFalse(); @@ -74,6 +81,7 @@ Livewire::actingAs($user) ->test(ListTenants::class) ->assertTableActionDisabled('forceDelete', $tenant) + ->assertTableActionExists('forceDelete', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $tenant) ->callTableAction('forceDelete', $tenant); expect(Tenant::withTrashed()->find($tenant->getKey()))->not->toBeNull(); @@ -89,6 +97,7 @@ Livewire::actingAs($user) ->test(ListTenants::class) ->assertTableActionDisabled('verify', $tenant) + ->assertTableActionExists('verify', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $tenant) ->callTableAction('verify', $tenant); }); @@ -113,7 +122,8 @@ Livewire::actingAs($user) ->test(ListTenants::class) - ->assertTableActionDisabled('edit', $tenant); + ->assertTableActionDisabled('edit', $tenant) + ->assertTableActionExists('edit', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $tenant); }); test('readonly users cannot open admin consent', function () { @@ -126,7 +136,8 @@ Livewire::actingAs($user) ->test(ListTenants::class) - ->assertTableActionDisabled('admin_consent', $tenant); + ->assertTableActionDisabled('admin_consent', $tenant) + ->assertTableActionExists('admin_consent', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $tenant); }); test('readonly users cannot start tenant sync from tenant menu', function () { @@ -138,5 +149,6 @@ Livewire::actingAs($user) ->test(ListTenants::class) - ->assertTableActionDisabled('syncTenant', $tenant); + ->assertTableActionDisabled('syncTenant', $tenant) + ->assertTableActionExists('syncTenant', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission(), $tenant); }); diff --git a/tests/Feature/Filament/TenantPortfolioContextSwitchTest.php b/tests/Feature/Filament/TenantPortfolioContextSwitchTest.php index 14347cf..cde067b 100644 --- a/tests/Feature/Filament/TenantPortfolioContextSwitchTest.php +++ b/tests/Feature/Filament/TenantPortfolioContextSwitchTest.php @@ -4,14 +4,20 @@ use App\Jobs\BulkTenantSyncJob; use App\Models\Tenant; use App\Models\User; +use App\Support\Auth\UiTooltips; use Filament\Events\TenantSet; use Filament\Facades\Filament; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Bus; +use Illuminate\Support\Facades\Http; use Livewire\Livewire; uses(RefreshDatabase::class); +beforeEach(function (): void { + Http::preventStrayRequests(); +}); + test('tenant-scoped pages return 404 for unauthorized tenant', function () { [$user, $authorizedTenant] = createUserWithTenant(); $unauthorizedTenant = Tenant::factory()->create(); @@ -21,6 +27,40 @@ ->assertNotFound(); }); +test('tenant portfolio tenant view returns 404 for non-member tenant record', function () { + $user = User::factory()->create(); + $this->actingAs($user); + + $authorizedTenant = Tenant::factory()->create(['tenant_id' => 'tenant-portfolio-authorized-view']); + $unauthorizedTenant = Tenant::factory()->create(['tenant_id' => 'tenant-portfolio-unauthorized-view']); + + $user->tenants()->syncWithoutDetaching([ + $authorizedTenant->getKey() => ['role' => 'owner'], + ]); + + $this->get(route('filament.admin.resources.tenants.view', array_merge( + filamentTenantRouteParams($unauthorizedTenant), + ['record' => $unauthorizedTenant], + )))->assertNotFound(); +}); + +test('tenant portfolio tenant edit returns 404 for non-member tenant record', function () { + $user = User::factory()->create(); + $this->actingAs($user); + + $authorizedTenant = Tenant::factory()->create(['tenant_id' => 'tenant-portfolio-authorized-edit']); + $unauthorizedTenant = Tenant::factory()->create(['tenant_id' => 'tenant-portfolio-unauthorized-edit']); + + $user->tenants()->syncWithoutDetaching([ + $authorizedTenant->getKey() => ['role' => 'owner'], + ]); + + $this->get(route('filament.admin.resources.tenants.edit', array_merge( + filamentTenantRouteParams($unauthorizedTenant), + ['record' => $unauthorizedTenant], + )))->assertNotFound(); +}); + test('tenant portfolio lists only tenants the user can access', function () { $user = User::factory()->create(); $this->actingAs($user); @@ -75,7 +115,9 @@ ]); }); -test('tenant portfolio bulk sync is hidden for readonly users', function () { +test('tenant portfolio bulk sync is disabled for readonly users', function () { + Bus::fake(); + $user = User::factory()->create(); $this->actingAs($user); @@ -87,8 +129,48 @@ Filament::setTenant($tenant, true); - Livewire::test(ListTenants::class) - ->assertTableBulkActionHidden('syncSelected'); + $livewire = Livewire::actingAs($user) + ->test(ListTenants::class) + ->selectTableRecords([$tenant]) + ->assertTableBulkActionVisible('syncSelected') + ->assertTableBulkActionDisabled('syncSelected'); + + $actions = $livewire->parseNestedTableBulkActions('syncSelected'); + $livewire->assertActionExists($actions, fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission()); + + $livewire->callTableBulkAction('syncSelected', collect([$tenant])); + + Bus::assertNotDispatched(BulkTenantSyncJob::class); +}); + +test('tenant portfolio bulk sync is disabled when selection includes unauthorized tenants', function () { + Bus::fake(); + + $user = User::factory()->create(); + $this->actingAs($user); + + $tenantA = Tenant::factory()->create(['tenant_id' => 'tenant-bulk-mixed-a']); + $tenantB = Tenant::factory()->create(['tenant_id' => 'tenant-bulk-mixed-b']); + + $user->tenants()->syncWithoutDetaching([ + $tenantA->getKey() => ['role' => 'owner'], + $tenantB->getKey() => ['role' => 'readonly'], + ]); + + Filament::setTenant($tenantA, true); + + $livewire = Livewire::actingAs($user) + ->test(ListTenants::class) + ->selectTableRecords([$tenantA, $tenantB]) + ->assertTableBulkActionVisible('syncSelected') + ->assertTableBulkActionDisabled('syncSelected'); + + $actions = $livewire->parseNestedTableBulkActions('syncSelected'); + $livewire->assertActionExists($actions, fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission()); + + $livewire->callTableBulkAction('syncSelected', collect([$tenantA, $tenantB])); + + Bus::assertNotDispatched(BulkTenantSyncJob::class); }); test('tenant set event updates user tenant preference last used timestamp', function () { diff --git a/tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php b/tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php new file mode 100644 index 0000000..4182760 --- /dev/null +++ b/tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php @@ -0,0 +1,118 @@ + $files */ + $files = collect($directories) + ->filter(fn (string $dir): bool => is_dir($dir)) + ->flatMap(function (string $dir): array { + $iterator = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($dir, FilesystemIterator::SKIP_DOTS) + ); + + $paths = []; + + foreach ($iterator as $file) { + if (! $file->isFile()) { + continue; + } + + $path = $file->getPathname(); + + if (! str_ends_with($path, '.php')) { + continue; + } + + $paths[] = $path; + } + + return $paths; + }) + ->filter(function (string $path) use ($excludedPaths, $self): bool { + if ($self && realpath($path) === $self) { + return false; + } + + foreach ($excludedPaths as $excluded) { + if (str_starts_with($path, $excluded)) { + return false; + } + } + + return true; + }) + ->values(); + + $hits = []; + + foreach ($files as $path) { + $relative = str_replace($root.'/', '', $path); + + if (in_array($relative, $allowlist, true)) { + continue; + } + + $contents = file_get_contents($path); + + if (! is_string($contents) || $contents === '') { + continue; + } + + foreach ($forbiddenPatterns as $pattern) { + if (! preg_match($pattern, $contents)) { + continue; + } + + $lines = preg_split('/\R/', $contents) ?: []; + + foreach ($lines as $index => $line) { + if (preg_match($pattern, $line)) { + $hits[] = $relative.':'.($index + 1).' -> '.trim($line); + } + } + } + } + + expect($hits)->toBeEmpty( + "Ad-hoc Filament auth patterns found (remove allowlist entries as you migrate):\n".implode("\n", $hits) + ); +}); diff --git a/tests/Unit/Auth/UiEnforcementBulkPreflightQueryCountTest.php b/tests/Unit/Auth/UiEnforcementBulkPreflightQueryCountTest.php new file mode 100644 index 0000000..5f98ccd --- /dev/null +++ b/tests/Unit/Auth/UiEnforcementBulkPreflightQueryCountTest.php @@ -0,0 +1,36 @@ +count(25)->create(); + [$user] = createUserWithTenant($tenants->first(), role: 'owner'); + + foreach ($tenants->slice(1) as $tenant) { + $user->tenants()->syncWithoutDetaching([ + $tenant->getKey() => ['role' => 'owner'], + ]); + } + + $enforcement = UiEnforcement::for(Capabilities::TENANT_SYNC) + ->tenantFromRecord() + ->preflightByCapability(); + + $membershipQueries = 0; + + DB::listen(function ($query) use (&$membershipQueries): void { + if (str_contains($query->sql, 'tenant_memberships')) { + $membershipQueries++; + } + }); + + expect($enforcement->bulkSelectionIsAuthorized($user, $tenants))->toBeTrue(); + expect($membershipQueries)->toBe(1); +}); + diff --git a/tests/Unit/Auth/UiEnforcementTest.php b/tests/Unit/Auth/UiEnforcementTest.php new file mode 100644 index 0000000..74b4284 --- /dev/null +++ b/tests/Unit/Auth/UiEnforcementTest.php @@ -0,0 +1,128 @@ + UiEnforcement::for(Capabilities::TENANT_VIEW)->tenantFromRecord()->preserveVisibility()) + ->toThrow(LogicException::class); + + expect(fn () => UiEnforcement::for(Capabilities::TENANT_VIEW)->preserveVisibility()->tenantFromRecord()) + ->toThrow(LogicException::class); +}); + +it('hides actions for non-members on record-scoped surfaces', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant(); + + $action = Action::make('test'); + + UiEnforcement::for(Capabilities::TENANT_VIEW) + ->tenantFromRecord() + ->apply($action); + + $this->actingAs($user); + $action->record($tenant); + + expect($action->isHidden())->toBeTrue(); +}); + +it('disables actions with the standard tooltip for members without the capability', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'readonly'); + + $action = Action::make('test'); + + UiEnforcement::for(Capabilities::TENANT_SYNC) + ->tenantFromRecord() + ->apply($action); + + $this->actingAs($user); + $action->record($tenant); + + expect($action->isHidden())->toBeFalse(); + expect($action->isDisabled())->toBeTrue(); + expect($action->getTooltip())->toBe(UiTooltips::insufficientPermission()); +}); + +it('enables actions for members with the capability', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'owner'); + + $action = Action::make('test'); + + UiEnforcement::for(Capabilities::TENANT_SYNC) + ->tenantFromRecord() + ->apply($action); + + $this->actingAs($user); + $action->record($tenant); + + expect($action->isHidden())->toBeFalse(); + expect($action->isDisabled())->toBeFalse(); + expect($action->getTooltip())->toBeNull(); +}); + +it('supports mixed visibility composition via andVisibleWhen', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'owner'); + + Filament::setTenant($tenant, true); + + $action = Action::make('test'); + + UiEnforcement::for(Capabilities::TENANT_VIEW) + ->andVisibleWhen(fn (): bool => false) + ->apply($action); + + $this->actingAs($user); + + expect($action->isHidden())->toBeTrue(); +}); + +it('supports mixed visibility composition via andHiddenWhen', function () { + $tenant = Tenant::factory()->create(); + [$user] = createUserWithTenant($tenant, role: 'owner'); + + Filament::setTenant($tenant, true); + + $action = Action::make('test'); + + UiEnforcement::for(Capabilities::TENANT_VIEW) + ->andHiddenWhen(fn (): bool => true) + ->apply($action); + + $this->actingAs($user); + + expect($action->isHidden())->toBeTrue(); +}); + +it('disables bulk actions for mixed-authorization selections (capability preflight)', function () { + $tenantA = Tenant::factory()->create(); + $tenantB = Tenant::factory()->create(); + + [$user] = createUserWithTenant($tenantA, role: 'owner'); + $user->tenants()->syncWithoutDetaching([ + $tenantB->getKey() => ['role' => 'readonly'], + ]); + + $enforcement = UiEnforcement::for(Capabilities::TENANT_SYNC) + ->tenantFromRecord() + ->preflightByCapability(); + + expect($enforcement->bulkSelectionIsAuthorized($user, collect([$tenantA, $tenantB])))->toBeFalse(); + + $user->tenants()->syncWithoutDetaching([ + $tenantB->getKey() => ['role' => 'owner'], + ]); + + expect($enforcement->bulkSelectionIsAuthorized($user, collect([$tenantA, $tenantB])))->toBeTrue(); +}); + -- 2.45.2 From d4148020bcead1b7b2ce35e6059fd370f5fc7787 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Fri, 30 Jan 2026 18:18:52 +0100 Subject: [PATCH 5/5] fix: align tooltip + guard after dev merge --- app/Support/Rbac/UiTooltips.php | 2 +- tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/Support/Rbac/UiTooltips.php b/app/Support/Rbac/UiTooltips.php index e6fa6d2..bd8e874 100644 --- a/app/Support/Rbac/UiTooltips.php +++ b/app/Support/Rbac/UiTooltips.php @@ -19,7 +19,7 @@ final class UiTooltips * Tooltip shown when a member lacks the required capability. * Intentionally vague to avoid leaking permission structure. */ - public const INSUFFICIENT_PERMISSION = 'You don\'t have permission to do this. Ask a tenant admin.'; + public const INSUFFICIENT_PERMISSION = 'Insufficient permission — ask a tenant Owner.'; /** * Modal heading for destructive action confirmation. diff --git a/tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php b/tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php index 4182760..20f04a4 100644 --- a/tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php +++ b/tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php @@ -37,7 +37,6 @@ $forbiddenPatterns = [ '/\\bGate::\\b/', - '/\\babort\\s*\\(/', '/\\babort_(?:if|unless)\\b/', ]; -- 2.45.2