## Summary
Fixes the sidebar context bug where navigating to the **Required Permissions** page (`/admin/tenants/{id}/required-permissions`) would switch the sidebar from workspace navigation to tenant-scoped navigation, confusing users.
## Problem
The `EnsureFilamentTenantSelected` middleware detected a tenant ID in the URL and called `setTenant()`, which switched the entire sidebar to tenant-scoped navigation. The Required Permissions page is logically a **workspace-level** page that happens to reference a tenant — it should keep showing workspace nav.
## Changes
### Middleware (`EnsureFilamentTenantSelected.php`)
- **`isWorkspaceScopedPageWithTenant()`** — new private helper that detects workspace-scoped pages containing a tenant parameter via regex
- **Livewire referer bypass** — checks if a Livewire request originates from a workspace-scoped page and preserves workspace nav
- **`setTenant()` bypass** — skips tenant activation and `rememberLastTenantId()` for workspace-scoped pages
### Tests
- **`RequiredPermissionsSidebarTest.php`** (NEW) — 7 tests covering:
- Workspace nav visible on required-permissions page
- Tenant nav absent on required-permissions page
- Direct URL access preserves workspace nav
- 404 for non-member tenants
- 404 for tenants without entitlement
- Tenant pages still show tenant sidebar (regression guard)
- Scoped tenant resolves correctly on tenant pages
### Pre-existing test fixes
- **`RequiredPermissionsEmptyStateTest`** — fixed URL assertion (dynamic `TenantResource::getUrl()` instead of hardcoded `/admin/onboarding`)
- **`RequiredPermissionsLinksTest`** — fixed URL assertion + multiline HTML `data-testid` assertion
- **`RequiredPermissionsFiltersTest`** — fixed `entra_permissions` config leak from branch 105
## Test Results
| Suite | Result |
|-------|--------|
| RequiredPermissions (26 tests) | **26 pass** (73 assertions) |
| Full regression (1571 tests) | **1562 pass**, 2 fail (pre-existing OpsUx), 7 skipped |
The 2 failures are pre-existing in `OpsUx/OperationCatalogCoverageTest` and `OpsUx/OperationSummaryKeysSpecTest` — unrelated to this feature.
## Spec Artifacts
- `specs/106-required-permissions-sidebar-context/plan.md`
- `specs/106-required-permissions-sidebar-context/tasks.md`
- `specs/106-required-permissions-sidebar-context/research.md`
- `specs/106-required-permissions-sidebar-context/data-model.md`
- `specs/106-required-permissions-sidebar-context/quickstart.md`
Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de>
Reviewed-on: #129
7.1 KiB
Implementation Plan: 106 — Required Permissions Sidebar Context Fix
Branch: 106-required-permissions-sidebar-context | Date: 2025-02-22 | Spec: 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.
- Inventory-first: N/A — no inventory changes
- Read/write separation: N/A — no writes, pure navigation rendering fix
- Graph contract path: N/A — no Graph calls introduced or modified
- Deterministic capabilities: N/A — no capability changes
- 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.
- Workspace isolation: Non-member access remains 404. Workspace context is preserved. The fix explicitly avoids changing workspace context switching behavior.
- RBAC-UX destructive actions: N/A — no actions added or modified
- RBAC-UX global search: N/A — no global search changes
- 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.
- Run observability: N/A — no operations, no long-running work
- Automation: N/A — no queued/scheduled work
- Data minimization: N/A — no logging changes
- Badge semantics (BADGE-001): N/A — no badges
- Filament UI Action Surface Contract: Exemption — no Filament Resource/Page/RelationManager created or modified. Only middleware navigation logic changes.
- 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)
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)
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:
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:
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):
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)
- Livewire v4.0+ compliance: Yes — no Livewire components created/modified; middleware is plain PHP.
- Provider registration: N/A — no providers changed.
- Global search: N/A — no resources added; Required Permissions page is not globally searchable.
- Destructive actions: N/A — no actions added.
- Asset strategy: N/A — no assets added.
- Testing plan: One new test file covering sidebar context; existing test files provide regression coverage.
Complexity Tracking
No constitution violations. No complexity justifications needed.