ahmido
33a2b1a242
## 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
99 lines
8.0 KiB
Markdown
99 lines
8.0 KiB
Markdown
# 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.
|