TenantAtlas/specs/303-admin-directory-groups-cutover/plan.md
ahmido 1cd8d48474 feat: cut over admin directory groups (#358)
## Summary
- cut over `EntraGroupResource` to an environment-bound admin Directory Groups surface in the workspace-first runtime
- adopt workspace-scoped admin list/detail URLs and add the bounded `Directory > Groups` navigation entry in the admin panel
- keep workspace-home navigation clean while preserving existing scoped list, detail, and global-search behavior
- update focused feature coverage and add a browser smoke for the rendered sidebar drilldown path
- include the Spec 303 package under `specs/303-admin-directory-groups-cutover/`

## Testing
- updated focused Pest coverage for admin navigation segregation, Entra group admin scoping, Entra group global search scoping, and directory group browsing
- added browser smoke coverage in `apps/platform/tests/Browser/Spec303AdminDirectoryGroupsCutoverSmokeTest.php`

## Filament / Runtime Notes
- remains compliant with Filament v5 on Livewire v4
- no provider registration changes; provider registration location remains `apps/platform/bootstrap/providers.php`
- `EntraGroupResource` remains eligible for global search because it has a View page
- no destructive actions were added or changed; confirmation and authorization behavior is unchanged
- no asset registration changes; existing `cd apps/platform && php artisan filament:assets` deploy posture is unchanged

Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de>
Reviewed-on: #358
2026-05-14 22:44:44 +00:00

268 lines
18 KiB
Markdown

# Implementation Plan: Admin Directory Groups Cutover
**Branch**: `303-admin-directory-groups-cutover` | **Date**: 2026-05-14 | **Spec**: [spec.md](spec.md)
**Input**: Feature specification from `/specs/303-admin-directory-groups-cutover/spec.md`
## Summary
Cut over `EntraGroupResource` from a stale admin-hidden posture to an explicit secondary environment-bound Directory Groups surface. The implementation keeps workspace-home navigation clean, preserves server-side context/RBAC boundaries, keeps global search scoped to the active Managed Environment, and avoids any broad Identity Center or M365 admin mirror.
## Technical Context
**Language/Version**: PHP 8.4.15
**Primary Dependencies**: Laravel 12.52.0, Filament 5.2.1, Livewire 4.1.4, Pest 4.3.1
**Storage**: PostgreSQL via existing `entra_groups` persistence; no schema changes
**Testing**: Pest feature tests through Laravel Sail
**Validation Lanes**: confidence, formatting/diff-check
**Target Platform**: Laravel Sail locally; Dokploy container deployment for staging/production
**Project Type**: Web application under `apps/platform`
**Performance Goals**: No new query families or polling. Existing scoped list/search queries must remain bounded to one Managed Environment.
**Constraints**: No migrations, no assets, no provider registration changes, no Graph adapter changes, no new actions, no legacy `/admin/t` route revival.
**Scale/Scope**: One existing Filament resource plus focused tests.
## UI / Surface Guardrail Plan
- **Guardrail scope**: changed operator-facing navigation/resource/search surfaces.
- **Native vs custom classification summary**: native Filament resource navigation, table, View page, and global search.
- **Shared-family relevance**: navigation and global search.
- **State layers in scope**: shell, route context, remembered environment context, resource page, record resolution.
- **Audience modes in scope**: operator-MSP and support-platform where existing capability paths already permit.
- **Decision/diagnostic/raw hierarchy plan**: Directory Groups is secondary context. List remains scan-oriented; View remains diagnostics/evidence-oriented. Raw/provider detail is not promoted.
- **Raw/support gating plan**: unchanged from existing detail patterns.
- **One-primary-action / duplicate-truth control**: primary interaction is inspect/open. This spec adds no new mutation action.
- **Handling modes by drift class or surface**: review-mandatory for stale hidden navigation and route-context hardening.
- **Repository-signal treatment**: Spec 302's audit matrix is the active signal; Entra Groups product IA is resolved here.
- **Special surface test profiles**: standard-native-filament.
- **Required tests or manual smoke**: focused feature tests for navigation, list/detail scope, search scope, search URL canonicality, no legacy route URLs, and regression surfaces. A focused browser smoke is required for the rendered Filament sidebar link because this implementation changes an operator-facing navigation flow.
- **Exception path and spread control**: none. `WorkspaceScopedTenantRoutes` is adopted for Entra Groups and verified with focused route/search/reference/browser tests.
- **Active feature PR close-out entry**: Guardrail / Exception / Smoke Coverage.
## Shared Pattern & System Fit
- **Cross-cutting feature marker**: yes.
- **Systems touched**: `EntraGroupResource`, `ListEntraGroups`, `ViewEntraGroup`, `AdminPanelProvider`, `NavigationScope`, `OperateHubShell`, `ScopesGlobalSearchToTenant`, `ResolvesPanelTenantContext`, `WorkspaceScopedTenantRoutes`, and focused Filament tests.
- **Shared abstractions reused**: `NavigationScope::shouldRegisterEnvironmentNavigation()`, `OperateHubShell::tenantOwnedPanelContext()`, `ScopesGlobalSearchToTenant`, `InteractsWithTenantOwnedRecords`, existing Filament resource authorization and URL helpers.
- **New abstraction introduced? why?**: none.
- **Why the existing abstraction was sufficient or insufficient**: Existing helpers already encode environment surface detection and scoped tenant-owned access. The only confirmed gap is `EntraGroupResource::shouldRegisterNavigation()` returning false for every admin context.
- **Bounded deviation / spread control**: No direct admin route exception is retained; the explicit `Groups` navigation item is a local environment-bound admin entry and not a new Directory suite.
## OperationRun UX Impact
- **Touches OperationRun start/completion/link UX?**: no new behavior.
- **Shared OperationRun UX contract/layer reused**: existing directory group sync, if still visible, remains on the current provider-operation start path.
- **Delegated start/completion UX behaviors**: N/A for new behavior.
- **Local surface-owned behavior that remains**: existing Groups list header actions remain unchanged unless a focused test proves they violate this spec's no-new-action contract.
- **Queued DB-notification policy**: N/A.
- **Terminal notification path**: N/A.
- **Exception required?**: none.
## Provider Boundary / Platform Core
- **Shared provider/platform boundary touched?**: yes, bounded to labels/navigation/search around one existing provider-owned resource.
- **Classification**: mixed.
- **Provider-owned seams**: Entra Group source object, provider IDs, group type labels, Microsoft Entra source wording.
- **Platform-core seams**: workspace/environment context, navigation visibility, RBAC, search destination safety.
- **Neutral terms preserved**: Workspace, Managed Environment, Directory Groups, Directory inventory.
- **Why no accidental provider deepening**: No new identity model, Graph contract, provider framework, capability registry, persisted taxonomy, or route family is introduced.
## Technical Approach
1. Confirm current repo posture from Spec 302 and the existing `EntraGroupResource` implementation.
2. Update tests first so they encode the new product decision:
- workspace-home sidebar stays clean
- environment context shows Groups
- no-context list/search denies or returns no results
- cross-environment and cross-workspace access denies as not found
- global search is environment-scoped
- search URLs point to canonical admin View destinations and never `/admin/t`
3. Replace the blanket admin-hidden `shouldRegisterNavigation()` rule with the shared environment navigation rule.
4. Verify whether `EntraGroupResource` can safely adopt `WorkspaceScopedTenantRoutes`.
- Prefer adopting it if focused route/search/reference tests stay green.
- If the rendered sidebar does not receive the resource auto-navigation item, add a bounded explicit `Groups` navigation item in the admin panel contract.
5. Preserve existing list/detail/search server-side scope helpers.
6. Update customer/operator copy only where needed to remove stale "tenant" wording and avoid M365 Admin mirror implications.
7. Run focused validation commands and formatting checks.
## Existing Repository Surfaces Likely Affected
- `apps/platform/app/Filament/Resources/EntraGroupResource.php`
- `apps/platform/app/Filament/Resources/EntraGroupResource/Pages/ListEntraGroups.php`
- `apps/platform/app/Filament/Resources/EntraGroupResource/Pages/ViewEntraGroup.php`
- `apps/platform/app/Support/Navigation/NavigationScope.php` only if a minimal helper adjustment is required; no contract split.
- `apps/platform/tests/Feature/Filament/PanelNavigationSegregationTest.php`
- `apps/platform/tests/Feature/Filament/EntraGroupAdminScopeTest.php`
- `apps/platform/tests/Feature/Filament/EntraGroupGlobalSearchScopeTest.php`
- `apps/platform/tests/Feature/DirectoryGroups/BrowseGroupsTest.php`
- `apps/platform/tests/Feature/Filament/EntraGroupEnterpriseDetailPageTest.php`
- `apps/platform/tests/Feature/Filament/EntraGroupResolvedReferencePresentationTest.php`
- `apps/platform/tests/Feature/Filament/PolicyVersionResolvedReferenceLinksTest.php`
## Domain / Model Implications
- No new domain model.
- No new persisted entity or table.
- No new status, enum, reason family, or taxonomy.
- `EntraGroup` remains provider-owned directory cache data scoped to one Managed Environment.
- Workspace ownership remains derived through the Managed Environment relationship and existing entitlement checks.
## UI / Filament Implications
- Filament v5 targets Livewire v4.0+; this repo currently uses Livewire 4.1.4.
- `shouldRegisterNavigation()` is navigation only, not authorization. Resource/page authorization remains server-side.
- `EntraGroupResource` already has a View page, satisfying Filament global-search destination eligibility.
- Global-search URL generation must remain explicit because this resource has environment-sensitive destinations.
- Destructive actions are not added. Existing URL-only actions must not be treated as confirmation-protected mutations.
- No assets are added; deploy-time `cd apps/platform && php artisan filament:assets` posture is unchanged.
## RBAC / Policy Implications
- Navigation visibility requires active environment context, but that is not the authorization boundary.
- Existing list/detail access must continue through workspace membership, environment access, and resource policy/capability checks.
- Non-members must receive deny-as-not-found for environment-owned data.
- Members without capability must receive the existing capability denial outcome after membership is established.
- Global search must not bypass `canAccessTenant()` or resource policies.
## Global Search Plan
- Keep `ScopesGlobalSearchToTenant` as the primary query scope.
- Keep or adjust `getGlobalSearchResultUrl()` so result URLs use the active/canonical admin View destination for the record's Managed Environment.
- Add explicit assertions that:
- no active environment produces no results
- active environment returns only matching local groups
- cross-workspace groups are absent
- result URLs do not contain `/admin/t`
- result URLs resolve to View destinations, not legacy tenant-panel routes
## Data / Migration Implications
- No migrations.
- No seed changes.
- No new indexes.
- No data backfill.
- No Graph contract or adapter changes.
## Testing Strategy
- Use Pest feature tests plus one focused Pest Browser smoke for the rendered sidebar navigation and View drilldown.
- Update `PanelNavigationSegregationTest` so Entra Groups is no longer protected by the blanket admin-hidden dataset and is instead tested as environment-visible and workspace-home-hidden.
- Update `BrowseGroupsTest` stale hidden navigation assertion.
- Extend `EntraGroupAdminScopeTest` for no-context, cross-environment, cross-workspace, and canonical destination behavior.
- Extend `EntraGroupGlobalSearchScopeTest` for search scoping and URL canonicality.
- Run the user-specified focused suite plus Directory Groups, inventory, governance artifact, and operation legacy-route regression tests.
## Constitution Check
*GATE: Must pass before implementation. Re-check after design and implementation.*
- Inventory-first: no Inventory or snapshot truth changes.
- Read/write separation: read-oriented Groups surface; no new write/change function.
- Single Graph contract path: no Graph calls or contract changes.
- Deterministic capabilities: no capability resolver changes.
- Proportionality: no new structure or abstraction; bounded repair.
- No premature abstraction: no Directory framework or Identity Center.
- First provider is not platform core: Microsoft Entra semantics remain provider-owned and bounded.
- Persisted truth: no new persistence.
- State: no new state/status family.
- Shared pattern first: reuse existing navigation/context/search helpers.
- Workspace isolation: workspace membership remains an isolation boundary.
- Tenant/environment isolation: Managed Environment access remains required for list/detail/search.
- RBAC-UX: UI visibility is not authorization; server-side checks remain mandatory.
- Test governance: focused feature tests, one explicit browser smoke for rendered navigation, low helper cost, no hidden heavy family.
- Filament native first: native resource navigation/table/View/search; no custom UI.
## Test Governance Check
- **Test purpose / classification by changed surface**: Feature plus focused Browser smoke for rendered navigation.
- **Affected validation lanes**: confidence and formatting/diff-check.
- **Why this lane mix is the narrowest sufficient proof**: The behavior is Filament resource navigation, scoped resource access, and global search URL generation; focused feature tests prove backend/context/search behavior, while one browser smoke proves the actual rendered sidebar link and row drilldown.
- **Narrowest proving command(s)**:
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Filament/PanelNavigationSegregationTest.php tests/Feature/Filament/AdminTenantSurfaceParityTest.php tests/Feature/Filament/AdminSharedSurfacePanelParityTest.php tests/Feature/Filament/TenantOwnedResourceScopeParityTest.php tests/Feature/Filament/EntraGroupAdminScopeTest.php tests/Feature/Filament/EntraGroupGlobalSearchScopeTest.php tests/Feature/Filament/PolicyResourceAdminSearchParityTest.php tests/Feature/Filament/PolicyVersionAdminSearchParityTest.php`
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/DirectoryGroups/BrowseGroupsTest.php tests/Feature/Filament/EntraGroupEnterpriseDetailPageTest.php tests/Feature/Filament/EntraGroupResolvedReferencePresentationTest.php tests/Feature/Filament/PolicyVersionResolvedReferenceLinksTest.php tests/Browser/Spec303AdminDirectoryGroupsCutoverSmokeTest.php`
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Filament/InventoryCoverageAdminTenantParityTest.php tests/Feature/Filament/InventoryHubDbOnlyTest.php tests/Feature/Filament/InventoryPagesTest.php tests/Feature/Filament/GovernanceArtifacts/GovernanceArtifactAdminPanelRegistrationTest.php tests/Feature/Filament/GovernanceArtifacts/GovernanceArtifactEnvironmentContextTest.php tests/Feature/Filament/GovernanceArtifacts/GovernanceArtifactLegacyTenantPanelGuardTest.php tests/Feature/Operations/LegacyRunRoutesNotFoundTest.php`
- `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent`
- `git diff --check`
- **Fixture / helper / factory / seed / context cost risks**: low; reuse existing helpers.
- **Expensive defaults or shared helper growth introduced?**: no.
- **Heavy-family additions, promotions, or visibility changes**: one focused browser smoke was added after implementation revealed that resource-level feature assertions did not prove the rendered sidebar link.
- **Surface-class relief / special coverage rule**: standard-native-filament relief.
- **Closing validation and reviewer handoff**: verify the browser lane remains explicit and focused, no workspace-home leakage occurs, and no new mutation action is introduced.
- **Budget / baseline / trend follow-up**: none expected.
- **Review-stop questions**: Did Groups leak onto workspace home? Did any search result cross environment/workspace? Did any URL contain `/admin/t`? Did implementation add a new action or route alias?
- **Escalation path**: follow-up-spec for broader navigation-contract split.
- **Active feature PR close-out entry**: Guardrail / Exception / Smoke Coverage.
- **Why no dedicated follow-up spec is needed now**: This spec resolves the only confirmed blocker; broader contract split remains conditional.
## Project Structure
### Documentation (this feature)
```text
specs/303-admin-directory-groups-cutover/
+-- checklists/
| +-- requirements.md
+-- plan.md
+-- spec.md
+-- tasks.md
```
### Source Code (likely implementation surfaces)
```text
apps/platform/app/Filament/Resources/EntraGroupResource.php
apps/platform/app/Filament/Resources/EntraGroupResource/Pages/ListEntraGroups.php
apps/platform/app/Filament/Resources/EntraGroupResource/Pages/ViewEntraGroup.php
apps/platform/app/Support/Navigation/NavigationScope.php
apps/platform/tests/Feature/Filament/
apps/platform/tests/Feature/DirectoryGroups/
```
**Structure Decision**: Implement in the existing Filament resource and focused tests. Do not create new base folders.
## Complexity Tracking
| Violation | Why Needed | Simpler Alternative Rejected Because |
|---|---|---|
| None | N/A | N/A |
## Implementation Phases
### Phase 1 - Preparation and Test Contract
Read Spec 301, Spec 302, this package, and the current Entra Groups resource/tests. Update tests to encode the new environment-bound contract before changing runtime code.
### Phase 2 - Navigation Contract
Change `EntraGroupResource::shouldRegisterNavigation()` to use the shared environment-navigation decision, preserving workspace-home absence.
### Phase 3 - Route and Search Destination Safety
Adopt `WorkspaceScopedTenantRoutes` for Entra Groups and prove scoped canonical View URLs with no `/admin/t` links.
### Phase 4 - Scoped Access and Copy
Preserve list/detail/search scoping. Adjust only minimal stale copy such as empty-state wording if needed to say "managed environment" rather than "tenant".
### Phase 5 - Regression Validation
Run focused suites for Entra Groups, tenant-owned surface parity, Inventory, Policy, governance artifacts, and operation routing. Run Pint and `git diff --check`.
## Rollout Considerations
- No environment variables.
- No database migrations.
- No queues or scheduled workers are changed.
- No storage or volume changes.
- No Dokploy runtime configuration change.
- No asset registration; `filament:assets` deployment requirement is unchanged.
- Staging validation should focus on sidebar visibility and scoped Groups search in a seeded/test workspace and environment.
## Risk Controls
- Do not revive `/admin/t/{tenant}` or `/admin/tenants/{tenant}`.
- Do not add compatibility aliases.
- Do not add group create/edit/delete/membership actions.
- Do not introduce an Identity Center or Directory suite.
- Do not split navigation contracts beyond minimal test reshaping needed for Groups.
- Do not touch tenant-panel dead code in this spec.
- If broader route side effects appear, stop and move them to a separate route/navigation-contract spec.