ahmido
6a86c5901a
Kontext / Ziel Diese PR standardisiert Tenant‑RBAC Enforcement in der Filament‑UI: statt ad-hoc Gate::*, abort_if/abort_unless und kopierten ->visible()/->disabled()‑Closures gibt es jetzt eine zentrale, wiederverwendbare Implementierung für Actions (Header/Table/Bulk). Links zur Spec: spec.md plan.md quickstart.md Was ist drin Neue zentrale Helper-API: UiEnforcement (Tenant-plane RBAC‑UX “source of truth” für Filament Actions) Standardisierte Tooltip-Texte und Context-DTO (UiTooltips, TenantAccessContext) Migration vieler tenant‑scoped Filament Action-Surfaces auf das Standardpattern (ohne ad-hoc Auth-Patterns) CI‑Guard (Test) gegen neue ad-hoc Patterns in app/Filament/**: verbietet Gate::allows/denies/check/authorize, use Illuminate\Support\Facades\Gate, abort_if/abort_unless Legacy-Allowlist ist aktuell leer (neue Verstöße failen sofort) RBAC-UX Semantik (konsequent & testbar) Non-member: UI Actions hidden (kein Tenant‑Leak); Execution wird blockiert (Filament hidden→disabled chain), Defense‑in‑depth enthält zusätzlich serverseitige Guards. Member ohne Capability: Action visible aber disabled + Standard-Tooltip; Execution wird blockiert (keine Side Effects). Member mit Capability: Action enabled und ausführbar. Destructive actions: über ->destructive() immer mit ->requiresConfirmation() + klare Warntexte (Execution bleibt über ->action(...)). Wichtig: In Filament v5 sind hidden/disabled Actions typischerweise “silently blocked” (200, keine Ausführung). Die Tests prüfen daher UI‑State + “no side effects”, nicht nur HTTP‑Statuscodes. Sicherheit / Scope Keine neuen DB-Tabellen, keine Migrations, keine Microsoft Graph Calls (DB‑only bei Render; kein outbound HTTP). Tenant Isolation bleibt Isolation‑Boundary (deny-as-not-found auf Tenant‑Ebene, Capability erst nach Membership). Kein Asset-Setup erforderlich; keine neuen Filament Assets. Compliance Notes (Repo-Regeln) Filament v5 / Livewire v4.0+ kompatibel. Keine Änderungen an Provider‑Registrierung (Laravel 11+/12: providers.php bleibt der Ort; hier unverändert). Global Search: keine gezielte Änderung am Global‑Search-Verhalten in dieser PR. Tests / Qualität Pest Feature/Unit Tests für Member/Non-member/Tooltip/Destructive/Regression‑Guard. Guard-Test: “No ad-hoc Filament auth patterns”. Full suite laut Tasks: vendor/bin/sail artisan test --compact → 837 passed, 5 skipped. Checklist: requirements.md vollständig (16/16). Review-Fokus API‑Usage in neuen/angepassten Filament Actions: UiEnforcement::forAction/forTableAction/forBulkAction(...)->requireCapability(...)->apply() Guard-Test soll “red” werden, sobald jemand neue ad-hoc Auth‑Patterns einführt (by design). Co-authored-by: Ahmed Darrazi <ahmeddarrazi@MacBookPro.fritz.box> Reviewed-on: #81
164 lines
11 KiB
Markdown
164 lines
11 KiB
Markdown
# 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.
|
||
|
||
## 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)*
|
||
|
||
<!--
|
||
IMPORTANT: User stories should be PRIORITIZED as user journeys ordered by importance.
|
||
Each user story/journey must be INDEPENDENTLY TESTABLE - meaning if you implement just ONE of them,
|
||
you should still have a viable MVP (Minimum Viable Product) that delivers value.
|
||
|
||
Assign priorities (P1, P2, P3, etc.) to each story, where P1 is the most critical.
|
||
Think of each story as a standalone slice of functionality that can be:
|
||
- Developed independently
|
||
- Tested independently
|
||
- Deployed independently
|
||
- Demonstrated to users independently
|
||
-->
|
||
|
||
### 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
|
||
|
||
<!--
|
||
ACTION REQUIRED: The content in this section represents placeholders.
|
||
Fill them out with the right 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: 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).
|
||
|
||
## 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.
|
||
|
||
<!--
|
||
ACTION REQUIRED: The content in this section represents placeholders.
|
||
Fill them out with the right functional 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.
|
||
|
||
### 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)*
|
||
|
||
<!--
|
||
ACTION REQUIRED: Define measurable success criteria.
|
||
These must be technology-agnostic and measurable.
|
||
-->
|
||
|
||
### 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.
|