feat(106): Required Permissions sidebar stays on workspace nav (#129)

## 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
This commit is contained in:
ahmido 2026-02-22 02:42:44 +00:00
parent 6a15fe978a
commit 33a2b1a242
8 changed files with 616 additions and 0 deletions

View File

@ -54,6 +54,7 @@ ## Code Style
PHP 8.4.15: Follow standard conventions
## Recent Changes
- 106-required-permissions-sidebar-context: Middleware sidebar-context fix for workspace-scoped pages
- 105-entra-admin-roles-evidence-findings: Added PHP 8.4 (Laravel 12) + Filament v5, Livewire v4, Pest v4
- 104-provider-permission-posture: Added PHP 8.4 (Laravel 12) + Filament v5, Livewire v4, Pest v4
- 103-ia-scope-filter-semantics: Added PHP 8.4 (Laravel 12) + Filament v5, Livewire v4, `OperateHubShell` support class

View File

@ -0,0 +1,36 @@
# Specification Quality Checklist: Required Permissions Sidebar Context Fix
**Purpose**: Validate specification completeness and quality before proceeding to planning
**Created**: 2025-02-22
**Feature**: [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
- This is a small, focused bug fix spec — no data model changes, no new UI screens.
- All constitution alignment sections are N/A or explicitly exempted with rationale.
- The spec references implementation-level details (middleware name, method names) in the Problem Statement for context — this is intentional to clearly identify the root cause for the implementer, while the Requirements and Success Criteria remain technology-agnostic.

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,98 @@
# Feature Specification: Required Permissions Sidebar Context Fix
**Feature Branch**: `106-required-permissions-sidebar-context`
**Created**: 2025-02-22
**Status**: Draft
**Input**: User description: "Fix Required Permissions page sidebar context: when navigating from tenant panel to the workspace-scoped Required Permissions page, the sidebar incorrectly shows tenant navigation instead of workspace navigation"
## Spec Scope Fields *(mandatory)*
- **Scope**: workspace
- **Primary Routes**: `/admin/tenants/{tenant}/required-permissions`
- **Data Ownership**: No data changes — navigation/sidebar rendering only
- **RBAC**: Existing workspace membership + `canAccessTenant()` check (no changes)
## Problem Statement
The Required Permissions page (`TenantRequiredPermissions`) is registered in the **admin panel** (workspace scope) with slug `tenants/{tenant}/required-permissions`. Its URL is `/admin/tenants/{tenant}/required-permissions`.
The `EnsureFilamentTenantSelected` middleware detects the `{tenant}` route parameter, resolves the tenant, and calls `Filament::setTenant($tenant, true)`. This causes `configureNavigationForRequest()` to render the **tenant-scoped sidebar** (Inventory, Backups & Restore, Directory, Governance, Findings...) instead of the workspace sidebar.
**Result**: When a user navigates to Required Permissions from **any** context, the sidebar shows tenant navigation — even though the page is workspace-scoped and registered only in the admin panel.
**Expected behavior**: The Required Permissions page should show the **workspace sidebar** (Tenants, Policies, Settings, Monitoring) because it is a workspace-level page that happens to be scoped to a specific tenant for data display purposes.
## User Scenarios & Testing *(mandatory)*
### User Story 1 — Consistent Workspace Sidebar on Required Permissions (Priority: P1)
An admin views the Required Permissions page for a specific tenant. Regardless of how they arrived at the page (from workspace Tenant View, from a verification report "next steps" link, or from the tenant panel Findings page), the sidebar always shows workspace-level navigation.
**Why this priority**: This is the core and only fix — sidebar context must match the page's panel registration.
**Independent Test**: Navigate to `/admin/tenants/{tenant}/required-permissions` while a tenant context is active in the session. Verify the sidebar shows workspace navigation items (Tenants, Policies, Monitoring/Operations, Settings) and does NOT show tenant-scoped items (Inventory, Backups & Restore, Directory, Governance, Findings).
**Acceptance Scenarios**:
1. **Given** the user is on the Tenant View page (workspace context), **When** they click "Open Required Permissions", **Then** the Required Permissions page loads with the workspace sidebar.
2. **Given** the user is browsing tenant-scoped pages (e.g., `/admin/t/{tenant}/findings`), **When** they navigate to `/admin/tenants/{tenant}/required-permissions`, **Then** the sidebar switches to workspace navigation.
3. **Given** the user directly enters the URL `/admin/tenants/{tenant}/required-permissions` in the browser, **Then** the page loads with the workspace sidebar.
4. **Given** the user is on the Required Permissions page and changes filters (status, type, search), **When** Livewire re-renders the page, **Then** the sidebar remains in workspace context (no flicker or context switch).
---
### User Story 2 — Context Bar Reflects Tenant Scope (Priority: P2)
The context bar (top bar) correctly shows the tenant name as the active tenant context while the user is on the Required Permissions page. The page is workspace-scoped for sidebar purposes, but the context bar should still indicate which tenant's permissions are being viewed.
**Why this priority**: Provides orientation — the user needs to know which tenant's permissions they are looking at, even though the sidebar is workspace-level.
**Independent Test**: Navigate to Required Permissions for a specific tenant. Verify the context bar shows the tenant name. Verify the sidebar shows workspace navigation.
**Acceptance Scenarios**:
1. **Given** the user is on `/admin/tenants/{tenant}/required-permissions`, **Then** the context bar displays the tenant name and the sidebar displays workspace navigation.
---
### Edge Cases
- What happens when the user navigates to Required Permissions for a tenant they don't have access to? → Existing behavior: 404 (no change needed).
- What happens during Livewire update requests (e.g., filter changes)? → The middleware must also handle `/livewire/update` requests with a referer pointing to the Required Permissions page, ensuring the sidebar stays in workspace context.
- What happens if the `{tenant}` route parameter is invalid or the tenant is soft-deleted? → Existing behavior: 404 from `TenantRequiredPermissions::mount()` (no change needed).
## Requirements *(mandatory)*
**Constitution alignment (required):** This feature introduces no new Graph calls, no write/change behavior, no queued/scheduled work, and no data mutations. It is a pure navigation/sidebar rendering fix. No `OperationRun` or `AuditLog` entries needed.
**Constitution alignment (RBAC-UX):** No authorization changes. The page already enforces workspace membership + `canAccessTenant()`. 404 semantics for non-members are unchanged. This fix only affects which sidebar navigation is rendered.
**Constitution alignment (OPS-EX-AUTH-001):** Not applicable — no auth endpoints involved.
**Constitution alignment (BADGE-001):** Not applicable — no badge changes.
**Constitution alignment (Filament Action Surfaces):** No new Resources/Pages/RelationManagers. The existing `TenantRequiredPermissions` page is unchanged in functionality. **Exemption**: Only middleware navigation logic is modified.
**Constitution alignment (UX-001 — Layout & Information Architecture):** No new screens. The Required Permissions page layout is unchanged. **Exemption**: Only sidebar context selection is fixed.
### Functional Requirements
- **FR-001**: The `EnsureFilamentTenantSelected` middleware MUST recognize the Required Permissions page path (`/admin/tenants/{tenant}/required-permissions`) as a workspace-scoped page and NOT set Filament tenant context for sidebar rendering purposes.
- **FR-002**: The middleware MUST still resolve the `{tenant}` route parameter for access control (workspace membership + tenant access check) — the tenant authorization logic must remain intact.
- **FR-003**: The sidebar MUST show workspace-level navigation items when the user is on the Required Permissions page, regardless of any previously active tenant context in the session.
- **FR-004**: Livewire update requests (filter changes on the Required Permissions page) MUST preserve the workspace sidebar context — the sidebar must not switch to tenant context during re-renders.
- **FR-005**: The context bar SHOULD continue to display the tenant name for orientation, even though the sidebar shows workspace navigation.
## Success Criteria *(mandatory)*
### Measurable Outcomes
- **SC-001**: When navigating to Required Permissions from any context (workspace Tenant View, tenant panel, direct URL), the sidebar always shows workspace navigation — 100% consistency.
- **SC-002**: No regression in Required Permissions page functionality — all existing filters, permission display, and "Re-run verification" link continue to work.
- **SC-003**: No regression in other pages that use the `{tenant}` route parameter — tenant-scoped pages (e.g., ProviderConnectionResource) continue to show tenant sidebar correctly.
## Assumptions
- The fix is localized to the `EnsureFilamentTenantSelected` middleware. The `TenantRequiredPermissions` page class itself does not need changes.
- A path-based allowlist approach (similar to how `/admin/operations` is already handled) is the most consistent mechanism.
- The `configureNavigationForRequest()` method's behavior (show workspace nav when no Filament tenant is set) is correct — the issue is that the middleware incorrectly sets the tenant for this workspace-scoped 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