spec(106): add plan, tasks, research, data-model, quickstart

This commit is contained in:
Ahmed Darrazi 2026-02-22 03:39:42 +01:00
parent ffd0e3ca03
commit 49ab849ca1
5 changed files with 481 additions and 0 deletions

View File

@ -0,0 +1,43 @@
# Data Model: 106 — Required Permissions Sidebar Context Fix
**Date**: 2026-02-22 | **Branch**: `106-required-permissions-sidebar-context`
## Summary
No data model changes. This feature modifies middleware control flow only.
## Entities Affected
None. No database tables, models, or relationships are created or modified.
## State Transitions
| Component | Before | After |
|---|---|---|
| `EnsureFilamentTenantSelected` middleware | Always calls `Filament::setTenant()` when `{tenant}` route param present | Checks workspace-scoped page allowlist first; skips `setTenant()` for matched pages |
| `Filament::getTenant()` on Required Permissions page | Returns resolved `Tenant` instance (triggers tenant sidebar) | Returns `null` (triggers workspace sidebar) |
| `configureNavigationForRequest()` on Required Permissions page | Renders tenant-scoped sidebar | Renders workspace-scoped sidebar |
| `rememberLastTenantId()` on Required Permissions page | Called (updates session) | Skipped (no session side-effect) |
## Middleware Decision Flow (After Fix)
```
Request arrives
├── /livewire/update?
│ └── Check referer against:
│ ├── /admin/operations/{run} (existing)
│ └── /admin/tenants/{tenant}/required-permissions (NEW)
│ └── Match → workspace nav, return
├── /admin/operations/{run} → workspace nav (existing)
├── /admin/operations → workspace nav (existing)
├── Route has {tenant} param?
│ ├── Authorization checks (all 8 — unchanged)
│ ├── Is workspace-scoped page? (NEW check)
│ │ ├── YES → configureNavigationForRequest() WITHOUT setTenant()
│ │ └── NO → Filament::setTenant() + rememberLastTenantId() + configureNavigation (existing)
│ └── return next
└── ... existing flow continues
```

View File

@ -0,0 +1,139 @@
# Implementation Plan: 106 — Required Permissions Sidebar Context Fix
**Branch**: `106-required-permissions-sidebar-context` | **Date**: 2025-02-22 | **Spec**: [spec.md](spec.md)
**Input**: Feature specification from `/specs/106-required-permissions-sidebar-context/spec.md`
## Summary
Fix the `EnsureFilamentTenantSelected` middleware so that the Required Permissions page (`/admin/tenants/{tenant}/required-permissions`) renders the **workspace sidebar** instead of the tenant sidebar. The page uses `{tenant}` for data display and authorization only — it is a workspace-scoped page registered in the admin panel. The middleware currently calls `Filament::setTenant()` for all routes with a `{tenant}` parameter, which incorrectly triggers tenant-scoped navigation. The fix adds a workspace-scoped page allowlist check that skips `setTenant()` while preserving all 8 authorization checks.
## Technical Context
**Language/Version**: PHP 8.4 (Laravel 12)
**Primary Dependencies**: Filament v5, Livewire v4
**Storage**: N/A — no data changes
**Testing**: Pest v4 (`vendor/bin/sail artisan test`)
**Target Platform**: Web (Sail Docker)
**Project Type**: Web (Laravel + Filament admin panel)
**Performance Goals**: N/A — string comparison in middleware (negligible)
**Constraints**: Zero regression for tenant-scoped pages; zero data model changes
**Scale/Scope**: 1 middleware file, 1 new test file, ~20 lines of logic
## Constitution Check
*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*
- [x] Inventory-first: N/A — no inventory changes
- [x] Read/write separation: N/A — no writes, pure navigation rendering fix
- [x] Graph contract path: N/A — no Graph calls introduced or modified
- [x] Deterministic capabilities: N/A — no capability changes
- [x] RBAC-UX: Two planes remain separated. The fix preserves all 8 authorization checks (workspace membership, tenant access, deny-as-not-found 404). Only sidebar rendering is changed. No cross-plane leakage.
- [x] Workspace isolation: Non-member access remains 404. Workspace context is preserved. The fix explicitly avoids changing workspace context switching behavior.
- [x] RBAC-UX destructive actions: N/A — no actions added or modified
- [x] RBAC-UX global search: N/A — no global search changes
- [x] Tenant isolation: All tenant reads remain scoped. The tenant param is still resolved and authorized — it is just not set as Filament active tenant for sidebar purposes.
- [x] Run observability: N/A — no operations, no long-running work
- [x] Automation: N/A — no queued/scheduled work
- [x] Data minimization: N/A — no logging changes
- [x] Badge semantics (BADGE-001): N/A — no badges
- [x] Filament UI Action Surface Contract: **Exemption** — no Filament Resource/Page/RelationManager created or modified. Only middleware navigation logic changes.
- [x] Filament UI UX-001 (Layout and IA): **Exemption** — no new screens, no layout changes. Only sidebar context selection is fixed.
**Post-Phase-1 re-check**: All gates still pass. No data model, no new UI surfaces, no Graph calls.
## Project Structure
### Documentation (this feature)
```text
specs/106-required-permissions-sidebar-context/
├── plan.md # This file
├── research.md # Phase 0 output — middleware analysis
├── data-model.md # Phase 1 output — state transition documentation
├── quickstart.md # Phase 1 output — implementation steps
└── tasks.md # Phase 2 output (created by /speckit.tasks)
```
### Source Code (repository root)
```text
app/Support/Middleware/
└── EnsureFilamentTenantSelected.php # Modified: add workspace-scoped allowlist
tests/Feature/RequiredPermissions/
└── RequiredPermissionsSidebarTest.php # New: sidebar context assertions
```
**Structure Decision**: Existing Laravel web application structure. No new directories needed. Changes are localized to the existing middleware file and a new test file in the existing test directory.
## Implementation Design
### Approach: Path-based Allowlist in Middleware
Follow the existing precedent set by `/admin/operations` and `/admin/operations/{run}` handling.
#### Change 1: Workspace-scoped page helper method
Add a private method to the middleware:
```php
private function isWorkspaceScopedPageWithTenant(string $path): bool
{
return preg_match('#^/admin/tenants/[^/]+/required-permissions$#', $path) === 1;
}
```
#### Change 2: Livewire `/livewire/update` referer check
In the existing `/livewire/update` block (after the operations check), add:
```php
if (preg_match('#^/admin/tenants/[^/]+/required-permissions$#', $refererPath) === 1) {
$this->configureNavigationForRequest($panel);
return $next($request);
}
```
#### Change 3: Skip `setTenant()` for workspace-scoped pages
In the `$tenantParameter !== null` block, **after** all 8 authorization checks pass but **before** `Filament::setTenant($tenant, true)`:
```php
if ($this->isWorkspaceScopedPageWithTenant($path)) {
$this->configureNavigationForRequest($panel);
return $next($request);
}
```
This preserves:
- All authorization (workspace membership, tenant access, deny-as-not-found)
- Workspace sidebar rendering (tenant is null -> workspace nav)
This skips:
- `Filament::setTenant()` (no tenant sidebar)
- `rememberLastTenantId()` (no session side-effect)
**FR-005 clarification (context bar / tenant name):** Because `Filament::setTenant()` is skipped, `Filament::getTenant()` will return `null`. The context bar heading is **not** sourced from Filament global tenant state — the `TenantRequiredPermissions` page resolves `$scopedTenant` directly from the `{tenant}` route parameter via `resolveScopedTenant()` and passes it to the view. The page heading (`$this->heading`) is set from `$scopedTenant->display_name`. Therefore, skipping `setTenant()` does NOT affect the tenant name display. T007 MUST verify this by asserting the tenant name appears in the response.
### Testing Plan
| Test | Type | What it asserts |
|---|---|---|
| Required Permissions shows workspace nav items | Feature | Response contains "Operations", "Manage workspaces" nav items |
| Required Permissions does not show tenant nav items | Feature | Response does not contain "Inventory", "Backups" nav items |
| Other `{tenant}` pages still get tenant sidebar | Feature | e.g., ProviderConnection page has tenant nav |
| Existing access tests still pass | Feature | No regression in `RequiredPermissionsAccessTest` |
| Existing filter tests still pass | Feature | No regression in `RequiredPermissionsFiltersTest` |
### Agent Output Contract (Filament v5 Blueprint section 15)
1. **Livewire v4.0+ compliance**: Yes — no Livewire components created/modified; middleware is plain PHP.
2. **Provider registration**: N/A — no providers changed.
3. **Global search**: N/A — no resources added; Required Permissions page is not globally searchable.
4. **Destructive actions**: N/A — no actions added.
5. **Asset strategy**: N/A — no assets added.
6. **Testing plan**: One new test file covering sidebar context; existing test files provide regression coverage.
## Complexity Tracking
No constitution violations. No complexity justifications needed.

View File

@ -0,0 +1,69 @@
# Quickstart: 106 — Required Permissions Sidebar Context Fix
**Branch**: `106-required-permissions-sidebar-context`
## What This Changes
The `EnsureFilamentTenantSelected` middleware is updated to recognize the Required Permissions page (`/admin/tenants/{tenant}/required-permissions`) as a workspace-scoped page. The page retains its `{tenant}` route parameter for data display and authorization, but the middleware no longer sets Filament's tenant context when serving this page. This causes the sidebar to render workspace-level navigation instead of tenant-level navigation.
## Files Modified
| File | Change |
|---|---|
| `app/Support/Middleware/EnsureFilamentTenantSelected.php` | Add workspace-scoped page allowlist check; add Livewire referer check |
| `tests/Feature/RequiredPermissions/RequiredPermissionsSidebarTest.php` | New test file — sidebar context assertions |
## Implementation Steps
### 1. Add workspace-scoped page path helper
In `EnsureFilamentTenantSelected`, add a private method:
```php
private function isWorkspaceScopedPageWithTenant(string $path): bool
{
return preg_match('#^/admin/tenants/[^/]+/required-permissions$#', $path) === 1;
}
```
### 2. Add Livewire referer check (before existing checks)
In the `/livewire/update` block, add:
```php
if (preg_match('#^/admin/tenants/[^/]+/required-permissions$#', $refererPath) === 1) {
$this->configureNavigationForRequest($panel);
return $next($request);
}
```
### 3. Split `{tenant}` param handling
In the `$tenantParameter !== null` block, after all 8 authorization checks pass, add:
```php
if ($this->isWorkspaceScopedPageWithTenant($path)) {
// Workspace-scoped page: authorize but do NOT set Filament tenant context.
// This preserves workspace sidebar while still validating tenant access.
$this->configureNavigationForRequest($panel);
return $next($request);
}
```
Place this **before** `Filament::setTenant($tenant, true)`.
### 4. Write tests
Create `tests/Feature/RequiredPermissions/RequiredPermissionsSidebarTest.php` with assertions:
- Sidebar shows workspace navigation items (Operations, Manage workspaces)
- Sidebar does NOT show tenant navigation items (Inventory, Backups & Restore)
- Livewire updates preserve workspace sidebar
- Other `{tenant}` pages still get tenant sidebar
## Verification
```bash
vendor/bin/sail artisan test --compact --filter=RequiredPermissionsSidebar
vendor/bin/sail artisan test --compact tests/Feature/RequiredPermissions/
vendor/bin/sail bin pint --dirty --format agent
```

View File

@ -0,0 +1,71 @@
# Research: 106 — Required Permissions Sidebar Context Fix
**Date**: 2026-02-22 | **Branch**: `106-required-permissions-sidebar-context`
## RQ-001: How does `EnsureFilamentTenantSelected` currently handle workspace-scoped pages with `{tenant}` params?
**Finding**: The middleware at `app/Support/Middleware/EnsureFilamentTenantSelected.php` follows this flow:
1. **Livewire `/livewire/update`** — checks referer for known workspace-scoped patterns (currently only `/admin/operations/{run}`). If matched, calls `configureNavigationForRequest()` without setting tenant and returns early.
2. **`/admin/operations/{run}`** — early return with workspace nav (no tenant set).
3. **`/admin/operations`** — early return with workspace nav (no tenant set).
4. **Route has `{tenant}` param** (line ~74131) — resolves tenant, validates workspace membership + tenant access, then **always** calls `Filament::setTenant($tenant, true)` and `configureNavigationForRequest()`. This is the root cause: `required-permissions` passes through here and gets tenant sidebar.
5. **Allowlisted workspace-scoped paths** (line ~133141) — string prefix checks for `/admin/w/`, `/admin/workspaces`, `/admin/operations`, plus an exact-match array. These get workspace nav without tenant.
**Root cause**: Step 4 has no exclusion mechanism. Any page with a `{tenant}` route parameter — including `TenantRequiredPermissions` — gets `Filament::setTenant()` called, triggering tenant sidebar.
**Decision**: Add a path-based allowlist check **inside** step 4, before `Filament::setTenant()` is called. When the path matches a workspace-scoped page that uses `{tenant}` for data scoping (not sidebar scoping), skip `setTenant()` but still perform all authorization checks.
**Alternatives considered**:
- *Page-level override (e.g., `$isWorkspaceScoped` property)*: Would require Filament page resolution in middleware (too expensive / wrong layer).
- *Separate middleware for Required Permissions route*: Duplicates authorization logic; fragile.
- *Clear tenant after middleware runs*: Race condition with `configureNavigationForRequest()` which already ran.
## RQ-002: What pages currently use the allowlist pattern?
**Finding**: The middleware already has two precedent patterns for workspace-scoped exemptions:
| Pattern | Type | Location in middleware |
|---|---|---|
| `/admin/operations` | Exact match | Line ~67 |
| `/admin/operations/{run}` | Regex | Line ~61 |
| `/livewire/update` + referer `/admin/operations/{run}` | Referer check | Line ~4956 |
| `/admin/w/`, `/admin/workspaces`, etc. | Prefix/exact | Line ~133141 |
**Decision**: Follow the same pattern (regex match + referer check for Livewire) for `/admin/tenants/{tenant}/required-permissions`.
## RQ-003: How does `configureNavigationForRequest` determine sidebar?
**Finding** (lines 152247):
- If `Filament::getTenant()` is filled → `$panel->navigation(true)` (default tenant nav).
- If tenant is null → builds a custom `NavigationBuilder` with workspace-scoped items only (Manage workspaces, Operations, Alert targets/rules/deliveries, Alerts, Audit Log).
**Decision**: No changes needed to `configureNavigationForRequest`. The fix is upstream: don't call `setTenant()` for workspace-scoped pages, so the method naturally shows workspace nav.
## RQ-004: What authorization must still run for exempted pages?
**Finding**: The `{tenant}` param resolution block (lines 74131) performs:
1. Null user check → early return
2. `HasTenants` interface check → abort 404
3. Tenant resolution by `external_id` (including soft-deleted) → abort 404
4. Workspace ID null check → abort 404
5. Tenant workspace ownership check → abort 404
6. Workspace existence check → abort 404
7. User workspace membership check → abort 404
8. `canAccessTenant()` → abort 404
**Decision**: All 8 checks must still run. Only `Filament::setTenant()` and `app(WorkspaceContext::class)->rememberLastTenantId()` are skipped for exempted workspace-scoped pages. The tenant is still resolved and authorized — it just isn't set as Filament's active tenant.
## RQ-005: Livewire update handling
**Finding**: For Livewire `/livewire/update` requests, the middleware inspects the `Referer` header to determine context. Currently only `/admin/operations/{run}` is checked.
**Decision**: Add `/admin/tenants/{tenant}/required-permissions` to the referer pattern check. The regex `#^/admin/tenants/[^/]+/required-permissions$#` will match referers from the Required Permissions page, ensuring Livewire re-renders (filter changes) also get workspace sidebar.
## RQ-006: Impact on `rememberLastTenantId()`
**Finding**: When `Filament::setTenant()` is called in step 4, `rememberLastTenantId()` stores the tenant in session. This enables "return to last tenant" convenience.
**Decision**: For workspace-scoped exempted pages, `rememberLastTenantId()` should NOT be called. Visiting Required Permissions should not change the user's "last active tenant" context. The tenant is used only for data display on that page.
**Rationale**: If we called `rememberLastTenantId()`, navigating to Required Permissions would change the user's default tenant context globally — unexpected side effect for a workspace-level page.

View File

@ -0,0 +1,159 @@
# Tasks: Required Permissions Sidebar Context Fix
**Input**: Design documents from `/specs/106-required-permissions-sidebar-context/`
**Prerequisites**: plan.md (required), spec.md (required for user stories), research.md, data-model.md, quickstart.md
**Tests**: This feature changes runtime behavior (middleware navigation logic), so tests are REQUIRED (Pest).
**Operations**: N/A — no long-running/remote/queued/scheduled work.
**RBAC**: No authorization changes. All 8 existing authorization checks are preserved. Only sidebar rendering is affected.
**Filament UI Action Surfaces**: **Exemption** — no Filament Resource/Page/RelationManager created or modified.
**Filament UI UX-001**: **Exemption** — no new screens, no layout changes.
**Badges**: N/A — no badge changes.
**Organization**: Tasks are grouped by user story to enable independent implementation and testing of each story.
## Format: `[ID] [P?] [Story] Description`
- **[P]**: Can run in parallel (different files, no dependencies)
- **[Story]**: Which user story this task belongs to (e.g., US1, US2)
- Include exact file paths in descriptions
---
## Phase 1: Setup
**Purpose**: No project initialization needed — this is a fix in an existing codebase. Phase is empty.
(No tasks — feature modifies existing files only.)
---
## Phase 2: Foundational (Blocking Prerequisites)
**Purpose**: No foundational infrastructure changes needed. The middleware, test helpers, and page classes already exist.
(No tasks — all prerequisites are already in place.)
**Checkpoint**: Foundation ready — user story implementation can begin immediately.
---
## Phase 3: User Story 1 — Consistent Workspace Sidebar on Required Permissions (Priority: P1) MVP
**Goal**: When navigating to `/admin/tenants/{tenant}/required-permissions`, the sidebar always shows workspace-level navigation instead of tenant-scoped navigation (Inventory, Backups & Restore, Directory, Governance, Findings). **Note**: Actual workspace nav item labels MUST be derived from `configureNavigationForRequest()` at implementation time — do not hardcode assumed names.
**Independent Test**: Navigate to `/admin/tenants/{tenant}/required-permissions` while a tenant context is active in the session. Verify workspace navigation items are present and tenant-scoped items are absent. Verify Livewire filter updates preserve workspace sidebar.
### Tests for User Story 1
- [x] T001 [US1] Create sidebar context test file at tests/Feature/RequiredPermissions/RequiredPermissionsSidebarTest.php with test cases: (a) page shows workspace nav items — **derive actual item labels from `configureNavigationForRequest()` output at implementation time** (do NOT hardcode assumed labels from the spec), (b) page does not show tenant nav items (Inventory, Backups), (c) direct URL navigation shows workspace sidebar, (d) Livewire filter updates preserve workspace sidebar context, (e) non-member access still returns 404 after middleware change (explicit FR-002 regression guard)
- [x] T002 [US1] Add regression test in tests/Feature/RequiredPermissions/RequiredPermissionsSidebarTest.php asserting that tenant-scoped pages (e.g., a page under `/admin/t/{tenant}/`) still show tenant sidebar — no regression
### Implementation for User Story 1
- [x] T003 [US1] Add `isWorkspaceScopedPageWithTenant()` private helper method to app/Support/Middleware/EnsureFilamentTenantSelected.php — regex match for `/admin/tenants/{tenant}/required-permissions`
- [x] T004 [US1] Add Livewire `/livewire/update` referer check for Required Permissions page in app/Support/Middleware/EnsureFilamentTenantSelected.php — add regex match in the existing `/livewire/update` block (after operations check, before the block returns)
- [x] T005 [US1] Add workspace-scoped page bypass in the `$tenantParameter !== null` block of app/Support/Middleware/EnsureFilamentTenantSelected.php — after all 8 authorization checks pass but before `Filament::setTenant($tenant, true)`, call `isWorkspaceScopedPageWithTenant($path)` and return early with `configureNavigationForRequest()` (skip `setTenant` and `rememberLastTenantId`)
- [x] T006 [US1] Run tests: `vendor/bin/sail artisan test --compact tests/Feature/RequiredPermissions/` — verify all new sidebar tests pass AND all existing Required Permissions tests still pass (no regression)
**Checkpoint**: User Story 1 complete — Required Permissions page always shows workspace sidebar. All existing functionality preserved.
---
## Phase 4: User Story 2 — Context Bar Reflects Tenant Scope (Priority: P2)
**Goal**: The context bar (top bar) correctly shows the tenant name for orientation while the sidebar displays workspace navigation.
**Independent Test**: Navigate to Required Permissions for a specific tenant. Verify the page heading or context area displays the tenant name. Verify sidebar shows workspace navigation.
### Implementation for User Story 2
- [x] T007 [US2] Verify context bar behavior in tests/Feature/RequiredPermissions/RequiredPermissionsSidebarTest.php — add assertion that the page response contains the tenant display name. **Note**: The page resolves `$scopedTenant` from the route param via `resolveScopedTenant()` (NOT from `Filament::getTenant()`), so the tenant name is available even though `Filament::setTenant()` is skipped. Verify the heading renders the tenant's `display_name`.
**Checkpoint**: User Story 2 complete — tenant name visible in page context while sidebar remains workspace-scoped.
---
## Phase 5: Polish & Cross-Cutting Concerns
**Purpose**: Code quality, formatting, and final validation.
- [x] T008 Run `vendor/bin/sail bin pint --dirty --format agent` to ensure code style compliance on app/Support/Middleware/EnsureFilamentTenantSelected.php
- [x] T009 Run full Required Permissions test suite: `vendor/bin/sail artisan test --compact tests/Feature/RequiredPermissions/` — final regression check
- [x] T010 Run quickstart.md validation: verify all steps described in specs/106-required-permissions-sidebar-context/quickstart.md are implemented
---
## Dependencies & Execution Order
### Phase Dependencies
- **Setup (Phase 1)**: Empty — no setup needed
- **Foundational (Phase 2)**: Empty — no foundational work needed
- **User Story 1 (Phase 3)**: Can start immediately — core fix
- **User Story 2 (Phase 4)**: Can start after US1 (depends on sidebar fix being in place to verify both sidebar + context bar)
- **Polish (Phase 5)**: Depends on US1 + US2 completion
### User Story Dependencies
- **User Story 1 (P1)**: No dependencies — can start immediately. This IS the MVP.
- **User Story 2 (P2)**: Soft dependency on US1 (needs the sidebar fix to verify context bar works alongside workspace sidebar). Can technically be verified independently since the page already renders tenant name.
### Within Each User Story
- Tests (T001, T002) can be written in parallel with each other
- Implementation tasks (T003, T004, T005) are sequential — each builds on the previous
- T006 (test run) depends on T001T005 all being complete
### Parallel Opportunities
- T001 and T002 can run in parallel (different test cases, same new file)
- T003, T004, T005 modify the same file sequentially — no parallelism
- T007 (US2) can start after T005 is complete
---
## Parallel Example: User Story 1
```bash
# Write both test groups in parallel:
Task T001: "Create sidebar context tests in tests/Feature/RequiredPermissions/RequiredPermissionsSidebarTest.php"
Task T002: "Add regression test for tenant-scoped pages in tests/Feature/RequiredPermissions/RequiredPermissionsSidebarTest.php"
# Then implement sequentially (same file):
Task T003: "Add isWorkspaceScopedPageWithTenant() helper"
Task T004: "Add Livewire referer check"
Task T005: "Add workspace-scoped page bypass in tenant param block"
# Then validate:
Task T006: "Run all Required Permissions tests"
```
---
## Implementation Strategy
### MVP First (User Story 1 Only)
1. Skip Phase 1 + Phase 2 (empty)
2. Complete Phase 3: User Story 1 (T001T006)
3. **STOP and VALIDATE**: All Required Permissions tests pass, sidebar shows workspace nav
4. This is a shippable fix.
### Incremental Delivery
1. User Story 1 (T001T006): Sidebar fix → Test → Ship (MVP!)
2. User Story 2 (T007): Context bar verification → Test → Ship
3. Polish (T008T010): Code style + final validation
---
## Notes
- Total tasks: 10
- Tasks per user story: US1 = 6, US2 = 1, Polish = 3
- Parallel opportunities: T001 + T002 (test writing)
- All middleware changes are in a single file: `app/Support/Middleware/EnsureFilamentTenantSelected.php`
- All new tests are in a single file: `tests/Feature/RequiredPermissions/RequiredPermissionsSidebarTest.php`
- Suggested MVP scope: User Story 1 only (T001T006) — this fully resolves the sidebar bug
- Format validation: All tasks follow `- [ ] [TaskID] [P?] [Story?] Description with file path` format