ahmido
3490fb9e2c
Summary
Implements Spec 067 “RBAC Troubleshooting & Tenant UI Bugfix Pack v1” for the tenant admin plane (/admin) with strict RBAC UX semantics:
Non-member tenant scope ⇒ 404 (deny-as-not-found)
Member lacking capability ⇒ 403 server-side, while the UI stays visible-but-disabled with standardized tooltips
What changed
Tenant view header actions now use centralized UI enforcement (no “normal click → error page” for readonly members).
Archived tenants remain resolvable in tenant-scoped routes for entitled members; an “Archived” banner is shown.
Adds tenant-scoped diagnostics page (/admin/t/{tenant}/diagnostics) with safe repair actions (confirmation + authorization + audit log).
Adds/updates targeted Pest tests to lock the 404 vs 403 semantics and action UX.
Implementation notes
Livewire v4.0+ compliance: Uses Filament v5 + Livewire v4 conventions; widget Blade views render a single root element.
Provider registration: Laravel 11+ providers stay in providers.php (no changes required).
Global search: No global search behavior/resources changed in this PR.
Destructive actions:
Tenant archive/restore/force delete and diagnostics repairs execute via ->action(...) and include ->requiresConfirmation().
Server-side authorization is enforced (non-members 404, insufficient capability 403).
Assets: No new assets. No change to php artisan filament:assets expectations.
Tests
Ran:
vendor/bin/sail bin pint --dirty
vendor/bin/sail artisan test --compact (focused files for Spec 067)
Co-authored-by: Ahmed Darrazi <ahmeddarrazi@MacBookPro.fritz.box>
Reviewed-on: #84
157 lines
8.7 KiB
Markdown
157 lines
8.7 KiB
Markdown
---
|
||
|
||
description: "Task list for feature implementation"
|
||
---
|
||
|
||
# Tasks: RBAC Troubleshooting & Tenant UI Bugfix Pack v1
|
||
|
||
**Input**: Design documents from `/specs/067-rbac-troubleshooting/`
|
||
**Prerequisites**: plan.md (required), spec.md (required for user stories), checklists/requirements.md (required), research.md, data-model.md, contracts/, quickstart.md
|
||
|
||
**Tests**: Required (Pest) for all runtime behavior changes.
|
||
|
||
---
|
||
|
||
## Phase 1: Setup (Shared Infrastructure)
|
||
|
||
**Purpose**: Ensure local environment is ready for implementation + testing.
|
||
|
||
- [x] T001 Start Sail services and ensure DB is ready using vendor/bin/sail (vendor/bin/sail)
|
||
- [x] T002 [P] Run baseline tests to capture current behavior (tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php)
|
||
|
||
---
|
||
|
||
## Phase 2: Foundational (Blocking Prerequisites)
|
||
|
||
**Purpose**: Shared primitives that user stories depend on.
|
||
|
||
- [x] T003 [P] Review and, if needed, extend standardized RBAC tooltip strings in app/Support/Rbac/UiTooltips.php (app/Support/Rbac/UiTooltips.php)
|
||
- [x] T004 [P] Confirm the tenant lifecycle capability used for archive/restore is Capabilities::TENANT_DELETE (app/Support/Auth/Capabilities.php)
|
||
|
||
**Checkpoint**: Foundation ready — user story work can begin.
|
||
|
||
---
|
||
|
||
## Phase 3: User Story 1 — Permission-safe tenant UI interactions (Priority: P1) 🎯 MVP
|
||
|
||
**Goal**: Readonly tenant members can browse tenant pages without normal-click navigation leading to error pages; protected actions are disabled with tooltips and blocked server-side.
|
||
|
||
**Independent Test**: As a `readonly` member, `ViewTenant` shows Edit/Deactivate as disabled+tooltip and Deactivate cannot be executed.
|
||
|
||
### Tests (write first)
|
||
|
||
- [x] T005 [P] [US1] Add ViewTenant header action UX tests for readonly vs owner in tests/Feature/Filament/TenantViewHeaderUiEnforcementTest.php (tests/Feature/Filament/TenantViewHeaderUiEnforcementTest.php)
|
||
- [x] T006 [P] [US1] Add regression test that readonly cannot execute the archive action (silently blocked by Filament; ensure no state change) in tests/Feature/Filament/TenantViewHeaderUiEnforcementTest.php (tests/Feature/Filament/TenantViewHeaderUiEnforcementTest.php)
|
||
|
||
### Implementation
|
||
|
||
- [x] T007 [US1] Replace unconditional EditAction in ViewTenant header with a visible-but-disabled URL action + tooltip in app/Filament/Resources/TenantResource/Pages/ViewTenant.php (app/Filament/Resources/TenantResource/Pages/ViewTenant.php)
|
||
- [x] T008 [US1] Wrap Deactivate (archive) header action with UiEnforcement + Capabilities::TENANT_DELETE + destructive confirmation, and enforce server-side authorization (Gate/Policy) in the mutation handler, in app/Filament/Resources/TenantResource/Pages/ViewTenant.php (app/Filament/Resources/TenantResource/Pages/ViewTenant.php)
|
||
- [x] T009 [US1] Ensure no ad-hoc auth patterns were added to app/Filament/** and update allowlist only if unavoidable (tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php)
|
||
|
||
---
|
||
|
||
## Phase 4: User Story 2 — Archived tenant remains viewable for entitled members (Priority: P1)
|
||
|
||
**Goal**: Archived (soft-deleted) tenants remain resolvable in tenant-scoped routes for entitled members; UI clearly indicates archived status; non-members still get 404.
|
||
|
||
**Independent Test**: Archive a tenant; member can access `/admin/t/{external_id}` and can mount `ViewTenant`; non-member receives 404.
|
||
|
||
### Tests (write first)
|
||
|
||
- [x] T010 [P] [US2] Add archived-tenant access test for members vs non-members in tests/Feature/TenantRBAC/ArchivedTenantRouteAccessTest.php (tests/Feature/TenantRBAC/ArchivedTenantRouteAccessTest.php)
|
||
- [x] T011 [P] [US2] Add UI assertion that archived tenants show an explicit archived indicator on ViewTenant in tests/Feature/Filament/ArchivedTenantViewTest.php (tests/Feature/Filament/ArchivedTenantViewTest.php)
|
||
|
||
### Implementation
|
||
|
||
- [x] T012 [US2] Update Tenant model route binding to resolve archived tenants by external_id using withTrashed() override in app/Models/Tenant.php (app/Models/Tenant.php)
|
||
- [x] T013 [US2] Add an “Archived” indicator on the tenant view header (e.g., subheading or banner) in app/Filament/Resources/TenantResource/Pages/ViewTenant.php (app/Filament/Resources/TenantResource/Pages/ViewTenant.php)
|
||
- [x] T014 [US2] Add a consistent icon for the Restore action in tenant list row menu in app/Filament/Resources/TenantResource.php (app/Filament/Resources/TenantResource.php)
|
||
|
||
---
|
||
|
||
## Phase 5: User Story 3 — Diagnose & repair tenant membership invariants (Priority: P2)
|
||
|
||
**Goal**: Authorized tenant members can see diagnostics for membership integrity (missing owner, duplicates) and run safe repair actions.
|
||
|
||
**Independent Test**: Seed a missing-owner scenario; diagnostics reports it; authorized user can repair by promoting a member to owner; unauthorized user is blocked.
|
||
|
||
### Tests (write first)
|
||
|
||
- [x] T015 [P] [US3] Add tenant diagnostics access tests (member OK, non-member 404) in tests/Feature/TenantRBAC/TenantDiagnosticsAccessTest.php (tests/Feature/TenantRBAC/TenantDiagnosticsAccessTest.php)
|
||
- [x] T016 [P] [US3] Add repair action test for missing owner (promote member to owner) in tests/Feature/Filament/TenantDiagnosticsRepairsTest.php (tests/Feature/Filament/TenantDiagnosticsRepairsTest.php)
|
||
- [x] T017 [P] [US3] Add negative authorization test: member without required capability sees repair action disabled + tooltip (and cannot execute; silently blocked by Filament) in tests/Feature/Filament/TenantDiagnosticsRepairsTest.php (tests/Feature/Filament/TenantDiagnosticsRepairsTest.php)
|
||
- [x] T018 [P] [US3] Add guardrail test that diagnostics/repairs never use external GUID where internal tenant PK is required (prevents bigint cast errors) in tests/Feature/TenantRBAC/TenantGuidVsBigintGuardTest.php (tests/Feature/TenantRBAC/TenantGuidVsBigintGuardTest.php)
|
||
- [x] T019 [P] [US3] Add duplicate membership diagnostics + merge repair test (seed duplicates, assert finding, execute merge, assert exactly 1 membership remains) in tests/Feature/Filament/TenantDiagnosticsRepairsTest.php (tests/Feature/Filament/TenantDiagnosticsRepairsTest.php)
|
||
- [x] T020 [P] [US3] Add audit trail test: each repair action writes an AuditLog entry with stable action id + tenant + actor in tests/Feature/Filament/TenantDiagnosticsRepairsTest.php (tests/Feature/Filament/TenantDiagnosticsRepairsTest.php)
|
||
|
||
### Implementation
|
||
|
||
- [x] T021 [US3] Implement tenant-scoped diagnostics Filament page route under `/admin/t/{tenant}/diagnostics` in app/Filament/Pages/TenantDiagnostics.php (app/Filament/Pages/TenantDiagnostics.php)
|
||
- [x] T022 [US3] Implement a diagnostics service to compute findings (missing owner, duplicates) in app/Services/Auth/TenantDiagnosticsService.php (app/Services/Auth/TenantDiagnosticsService.php)
|
||
- [x] T023 [US3] Implement diagnostics page actions with UiEnforcement + confirmation + server-side authorization enforcement + audit logging (bootstrap owner, merge duplicates) in app/Filament/Pages/TenantDiagnostics.php (app/Filament/Pages/TenantDiagnostics.php)
|
||
- [x] T024 [US3] Ensure all tenant membership queries in new/changed code use the internal tenant key `(int) $tenant->getKey()` (never `$tenant->tenant_id`) and refactor any offenders found during implementation (app/Services/Auth/TenantDiagnosticsService.php)
|
||
|
||
---
|
||
|
||
## Phase 6: Polish & Cross-Cutting Concerns
|
||
|
||
- [x] T025 [P] Run formatter on touched files (dirty only) via vendor/bin/sail (vendor/bin/sail)
|
||
- [x] T026 Run targeted test suite covering all new tests + auth guard (tests/Feature/Guards/NoAdHocFilamentAuthPatternsTest.php)
|
||
|
||
---
|
||
|
||
## Dependencies & Execution Order
|
||
|
||
### Phase Dependencies
|
||
|
||
- **Phase 1 (Setup)** → blocks everything else
|
||
- **Phase 2 (Foundational)** → blocks all user stories
|
||
- **US1 / US2 / US3** can start after Phase 2, but US1 and US2 both touch `ViewTenant`, so coordinate to avoid merge conflicts
|
||
- **Polish** after all desired stories are complete
|
||
|
||
### User Story Dependencies
|
||
|
||
- **US1 (P1)**: independent after Phase 2
|
||
- **US2 (P1)**: independent after Phase 2
|
||
- **US3 (P2)**: independent after Phase 2 (reuses shared auth + tenant scoping)
|
||
|
||
---
|
||
|
||
## Parallel Execution Examples
|
||
|
||
### US1
|
||
|
||
- In parallel:
|
||
- T005 (tests) and T006 (tests)
|
||
- Then:
|
||
- T007 + T008 (implementation)
|
||
|
||
### US2
|
||
|
||
- In parallel:
|
||
- T010 (route access tests) and T011 (UI indicator test)
|
||
- Then:
|
||
- T012 (route binding) and T013/T014 (UI fixes)
|
||
|
||
### US3
|
||
|
||
- In parallel:
|
||
- T015–T020 (tests)
|
||
- Then:
|
||
- T021–T024 (implementation)
|
||
|
||
---
|
||
|
||
## Implementation Strategy
|
||
|
||
### Suggested MVP Scope
|
||
|
||
- MVP = **User Story 1** only (T005–T009), because it fixes the most user-visible and security-adjacent UX issues and is easiest to validate independently.
|
||
|
||
### Incremental Delivery
|
||
|
||
1. US1 → validate UX + server-side enforcement
|
||
2. US2 → validate archived access + banner + restore icon
|
||
3. US3 → add diagnostics + repairs with strict authorization
|