Implements LIST `$expand` parity with GET by forwarding caller-provided, contract-allowlisted expands. Key changes: - Entra Admin Roles scan now requests `expand=principal` for role assignments so `principal.displayName` can render. - `$expand` normalization/sanitization: top-level comma split (commas inside balanced parentheses preserved), trim, dedupe, allowlist exact match, caps (max 10 tokens, max 200 chars/token). - Diagnostics when expands are removed/truncated (non-prod warning, production low-noise). Tests: - Adds/extends unit coverage for Graph contract sanitization, list request shaping, and the EntraAdminRolesReportService. Spec artifacts included under `specs/112-list-expand-parity/`. Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de> Reviewed-on: #136
6.8 KiB
| description |
|---|
| Task list for Graph Contracts — LIST $expand Parity Fix |
Tasks: Graph Contracts — LIST $expand Parity Fix
Input: Design documents from specs/112-list-expand-parity/
Prerequisites: plan.md (required), spec.md (required for user stories), research.md, data-model.md, contracts/
Tests: REQUIRED (Pest) — this feature changes runtime Graph request shapes.
Phase 1: Setup (Shared)
Purpose: Confirm inputs and establish a baseline before changing runtime behavior.
- T001 [Shared] Read implementation plan in specs/112-list-expand-parity/plan.md
- T002 [P] [Shared] Verify $expand allowlist exists for Entra role assignments in config/graph_contracts.php
- T003 [P] [Shared] Identify existing Graph tests to extend in tests/Unit/GraphContractRegistryTest.php and tests/Unit/MicrosoftGraphClientListPoliciesSelectTest.php
Phase 2: Foundational (Blocking Prerequisites)
Purpose: Establish pre-change confidence and shared test baseline.
- T004 [Shared] Run baseline focused tests listed in specs/112-list-expand-parity/quickstart.md
Checkpoint: Baseline established; safe to implement.
Phase 3: User Story 1 — Admin roles report shows real principal names (Priority: P1) 🎯 MVP
Goal: Admin Roles report requests principal expansion and renders real principal display names when Graph provides them.
Independent Test: Simulate role assignment list retrieval and assert report output uses principal.displayName when present, else falls back to Unknown.
Tests for User Story 1
- T005 [P] [US1] Create report service test scaffolding in tests/Unit/EntraAdminRolesReportServiceTest.php (RefreshDatabase + mock GraphClientInterface)
- T006 [US1] Implement tests asserting
entraRoleAssignmentscall includesexpand => 'principal'and payload uses principal display name in tests/Unit/EntraAdminRolesReportServiceTest.php
Implementation for User Story 1
- T007 [US1] Pass
expand => 'principal'when fetching assignments in app/Services/EntraAdminRoles/EntraAdminRolesReportService.php - T008 [US1] Forward caller-provided
expandinto LIST query input in app/Services/Graph/MicrosoftGraphClient.php - T009 [US1] Normalize + cap
$expandin app/Services/Graph/GraphContractRegistry.php (split string on top-level commas only; preserve commas inside balanced parentheses; trim, drop empty, dedupe, maxTokenLen=200, maxItems=10 after allowlist)
Regression tests for User Story 1
- T010 [US1] Add LIST
$expandrequest-shape test coverage in tests/Unit/MicrosoftGraphClientListPoliciesSelectTest.php - T011 [US1] Add
$expandnormalization/cap test coverage in tests/Unit/GraphContractRegistryTest.php - T012 [US1] Run story tests in tests/Unit/EntraAdminRolesReportServiceTest.php plus Graph tests in tests/Unit/MicrosoftGraphClientListPoliciesSelectTest.php
Checkpoint: Entra Admin Roles report can render real principal names when upstream provides them.
Phase 4: User Story 2 — Engineers can request explicit LIST expansions safely (Priority: P2)
Goal: LIST supports explicit expansions safely and discoverably (document option shape, allowlist enforcement).
Independent Test: Inspect outgoing LIST request and confirm $expand is included only when explicitly provided and allowlisted.
Tests for User Story 2
- T013 [P] [US2] Add test asserting LIST omits
$expandwhenexpandis not provided in tests/Unit/MicrosoftGraphClientListPoliciesSelectTest.php - T014 [P] [US2] Add tests asserting (a) disallowed expand tokens are removed (exact-match allowlist, no partial matches) and (b) allowlisted subquery/select-style expand tokens that contain commas inside parentheses are preserved as a single token (no naive comma-splitting) in tests/Unit/GraphContractRegistryTest.php
Implementation for User Story 2
- T015 [US2] Document supported
listPolicies()options (select/expand/filter/top/platform + input shapes) in app/Services/Graph/GraphClientInterface.php - T016 [US2] Ensure
GraphContractCheckquery shape remains compatible (select + expand) in app/Console/Commands/GraphContractCheck.php
Checkpoint: Engineers can safely request allowlisted expansions on LIST without silent capability mismatch.
Phase 5: User Story 3 — Misuse is detectable in non-production (Priority: P3)
Goal: When $expand is sanitized (removed or truncated), maintainers get a diagnostic signal in non-production with low noise in production.
Independent Test: Trigger mixed allowed/disallowed expand and assert a non-production warning is emitted with policy type + removed values.
Tests for User Story 3
- T017 [US3] Add Log::spy-based test for non-production diagnostics when
$expandis sanitized in tests/Unit/GraphContractRegistryTest.php - T018 [US3] Add production-noise guard test (debug-level or suppressed warn) for
$expandsanitization in tests/Unit/GraphContractRegistryTest.php
Implementation for User Story 3
- T019 [US3] Emit structured diagnostic logs when
$expandis removed/truncated in app/Services/Graph/GraphContractRegistry.php
Checkpoint: Sanitization is visible during dev/staging without spamming production logs.
Phase 6: Polish & Cross-Cutting Concerns
Purpose: Keep the change safe, consistent, and shippable.
- T020 [Shared] Run formatting on touched files (Pint) after changes in app/Services/Graph/GraphContractRegistry.php
- T021 [Shared] Run focused suite listed in specs/112-list-expand-parity/quickstart.md
Dependencies & Execution Order
Phase Dependencies
- Setup (Phase 1) → Foundational (Phase 2) → US1 (Phase 3) → US2 (Phase 4) → US3 (Phase 5) → Polish (Phase 6)
User Story Dependencies
- US1 depends on LIST
$expandparity (implemented as part of US1 via app/Services/Graph/MicrosoftGraphClient.php + app/Services/Graph/GraphContractRegistry.php). - US2 extends US1 with discoverability + additional safety regression coverage.
- US3 depends on the sanitization logic (adds diagnostics + tests).
Parallel Opportunities
- Phase 1: T002 and T003 can run in parallel.
- US1: T005 can be done in parallel with T008/T009 (different files).
- US2: T013 and T014 can run in parallel.
Parallel Example: User Story 1
- In parallel:
- T005 (test scaffolding) in tests/Unit/EntraAdminRolesReportServiceTest.php
- T008 (LIST forwards expand) in app/Services/Graph/MicrosoftGraphClient.php
- T009 (sanitize/normalize/cap expand) in app/Services/Graph/GraphContractRegistry.php
Implementation Strategy
MVP First (User Story 1 Only)
- Complete Phase 1–2
- Complete US1 (Phase 3)
- Stop and ship if acceptance + regression tests pass
Then harden (US2 + US3)
- Add discoverability + extra regression guards (US2)
- Add diagnostics behavior + tests (US3)