ahmido
bda1d90fc4
Implements spec 094 (assignment fetch/restore observability hardening): - Adds OperationRun tracking for assignment fetch (during backup) and assignment restore (during restore execution) - Normalizes failure codes/reason_code and sanitizes failure messages - Ensures exactly one audit log entry per assignment restore execution - Enforces correct guard/membership vs capability semantics on affected admin surfaces - Switches assignment Graph services to depend on GraphClientInterface Also includes Postgres-only FK defense-in-depth check and a discoverable `composer test:pgsql` runner (scoped to the FK constraint test). Tests: - `vendor/bin/sail artisan test --compact` (passed) - `vendor/bin/sail composer test:pgsql` (passed) Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de> Reviewed-on: #113
173 lines
12 KiB
Markdown
173 lines
12 KiB
Markdown
# Tasks: 094 Assignment Operations Observability Hardening
|
||
|
||
**Input**: Design documents from `specs/094-assignment-ops-observability-hardening/`
|
||
**Prerequisites**: `specs/094-assignment-ops-observability-hardening/plan.md` (required), `specs/094-assignment-ops-observability-hardening/spec.md` (required), `specs/094-assignment-ops-observability-hardening/research.md`, `specs/094-assignment-ops-observability-hardening/data-model.md`, `specs/094-assignment-ops-observability-hardening/contracts/assignment-ops.openapi.yaml`
|
||
|
||
**Tests**: Required (Pest) for runtime behavior changes.
|
||
|
||
---
|
||
|
||
## Phase 1: Setup (Shared Infrastructure)
|
||
|
||
**Purpose**: Ensure the implementation has the correct local workflow context and that the existing conventions to extend are understood.
|
||
|
||
- [X] T001 Run SpecKit prerequisites check via `.specify/scripts/bash/check-prerequisites.sh --json` and record `FEATURE_DIR` + `AVAILABLE_DOCS` for this feature
|
||
- [X] T002 Review existing OperationRun tracking conventions in `app/Jobs/Middleware/TrackOperationRun.php` and `app/Services/OperationRunService.php`
|
||
- [X] T003 Review existing OperationRun test patterns in `tests/Feature/TrackOperationRunMiddlewareTest.php` and `tests/Feature/OperationRunServiceTest.php`
|
||
|
||
---
|
||
|
||
## Phase 2: Foundational (Blocking Prerequisites)
|
||
|
||
**Purpose**: Confirm the persistence + dedupe primitives exist and match the spec’s requirements before adding new run types and audits.
|
||
|
||
**⚠️ CRITICAL**: Complete this phase before implementing any user story.
|
||
|
||
- [X] T004 Verify `operation_runs` schema supports `run_identity_hash`, `summary_counts`, and `failure_summary` per `database/migrations/2026_01_16_180642_create_operation_runs_table.php`
|
||
- [X] T005 Verify active-run dedupe constraints exist and align with `OperationRunService::ensureRunWithIdentity()` in `app/Services/OperationRunService.php` and `database/migrations/2026_02_10_004939_add_unique_index_for_backup_schedule_scheduled_operation_runs.php`
|
||
- [X] T006 [P] Identify existing restore audit logging expectations to preserve by reviewing `tests/Feature/RestoreAuditLoggingTest.php` and `app/Services/Intune/AuditLogger.php`
|
||
- [X] T034 [P] Verify active-run dedupe constraints cover the new assignment run identity shapes; if not, add a DB-enforced partial unique index/migration + regression test for dedupe behavior
|
||
|
||
**Checkpoint**: Foundation ready — user story work can begin.
|
||
|
||
---
|
||
|
||
## Phase 3: User Story 1 — Observe assignment operations end-to-end (Priority: P1) 🎯 MVP
|
||
|
||
**Goal**: Assignment fetch (during backup capture when assignments are included) and assignment restore (during restore execution) are observable via `OperationRun` with stable failure codes, normalized `reason_code`, correct counters, and exactly one audit log entry per assignment restore execution.
|
||
|
||
**Independent Test**: Trigger (a) backup creation with assignments included and (b) a restore that applies assignments; verify `operation_runs` rows exist and `audit_logs` contains exactly one assignment-restore summary entry.
|
||
|
||
### Tests for User Story 1 (write first)
|
||
|
||
- [X] T007 [P] [US1] Add test for assignment restore OperationRun lifecycle + counters in `tests/Feature/Operations/AssignmentRestoreOperationRunTest.php`
|
||
- [X] T008 [P] [US1] Add test for stable failure `code` + normalized `reason_code` on assignment restore failure in `tests/Feature/Operations/AssignmentRestoreOperationRunFailureTest.php`
|
||
- [X] T009 [P] [US1] Add test that assignment restore writes exactly one audit log entry per restore execution in `tests/Feature/Audit/AssignmentRestoreAuditSummaryTest.php`
|
||
- [X] T010 [P] [US1] Add test that backup capture with assignments included records an assignment-fetch OperationRun entry for the resulting backup item in `tests/Feature/Operations/AssignmentFetchOperationRunTest.php`
|
||
- [X] T035 [P] [US1] Add regression test for Monitoring pages being DB-only at render time (no outbound calls) when viewing Operations list + detail in `tests/Feature/Monitoring/OperationsDbOnlyRenderTest.php`
|
||
|
||
### Implementation for User Story 1
|
||
|
||
- [X] T011 [US1] Add assignment-restore OperationRun creation + dedupe by `restore_run_id` in `app/Services/Intune/RestoreService.php` (use `OperationRunService::ensureRunWithIdentity()`; identity inputs include `restore_run_id`)
|
||
- [X] T012 [US1] Update assignment-restore OperationRun summary counts (`total` attempted, `processed` succeeded, `failed` failed) in `app/Services/Intune/RestoreService.php` and `app/Services/OperationRunService.php`
|
||
- [X] T013 [US1] Record stable failure `code` namespaces + normalized `reason_code` + sanitized messages for assignment restore failures in `app/Services/Intune/RestoreService.php` (leverage `OperationRunService` failure sanitization)
|
||
- [X] T014 [US1] Remove per-assignment audit log emission from `app/Services/AssignmentRestoreService.php` (replace per-item `auditLogger->log(...)` calls with in-memory aggregation only)
|
||
- [X] T015 [US1] Add exactly one audit log entry per assignment restore execution in `app/Services/Intune/RestoreService.php` using `app/Services/Intune/AuditLogger.php` (resourceType `restore_run`, resourceId = restore run id)
|
||
- [X] T016 [US1] Add assignment-fetch OperationRun creation + dedupe keyed by `backup_item_id` in `app/Services/AssignmentBackupService.php` (wrap the Graph fetch inside `enrichWithAssignments()` using `OperationRunService::ensureRunWithIdentity()`)
|
||
- [X] T017 [US1] Ensure assignment-related job classes can be OperationRun-tracked if they are used in the future by adding `public ?OperationRun $operationRun` + `middleware()` returning `TrackOperationRun` in `app/Jobs/FetchAssignmentsJob.php` and `app/Jobs/RestoreAssignmentsJob.php`
|
||
- [X] T036 [US1] Identify and document all start surfaces that can trigger assignment fetch/restore, and ensure each path creates/reuses the same run identity (avoid “tracked in one path, untracked in another” gaps)
|
||
|
||
**Checkpoint**: US1 delivers Monitoring visibility + auditability for assignment restore and assignment fetch during backup capture.
|
||
|
||
---
|
||
|
||
## Phase 4: User Story 2 — Enforce correct access control semantics on affected admin surfaces (Priority: P2)
|
||
|
||
**Goal**: Cross-plane access returns 404, non-member access returns 404, and member-without-capability returns 403. Remove any authorization bypasses.
|
||
|
||
**Independent Test**: Access the affected routes/surfaces with wrong guard and with insufficient permissions; assert 404 vs 403 semantics.
|
||
|
||
### Tests for User Story 2 (write first)
|
||
|
||
- [X] T018 [P] [US2] Add regression test for `/admin/w/{workspace}` guard enforcement (cross-plane 404) in `tests/Feature/Guards/AdminWorkspaceRoutesGuardTest.php`
|
||
- [X] T019 [P] [US2] Add regression test ensuring Provider Connection create CTA does not bypass authorization in `tests/Feature/ProviderConnections/ProviderConnectionListAuthorizationTest.php`
|
||
- [X] T020 [P] [US2] Add regression test for membership (404) before capability (403) in backup items relation manager in `tests/Feature/Rbac/BackupItemsRelationManagerSemanticsTest.php`
|
||
|
||
### Implementation for User Story 2
|
||
|
||
- [X] T021 [US2] Add missing `ensure-correct-guard:web` middleware to the `/admin/w/{workspace}` route group in `routes/web.php`
|
||
- [X] T022 [US2] Remove the `->authorize(fn (): bool => true)` bypass from header and empty-state create actions in `app/Filament/Resources/ProviderConnectionResource/Pages/ListProviderConnections.php`
|
||
- [X] T023 [US2] Fix membership (404) vs capability (403) ordering for backup items under a backup set in `app/Filament/Resources/BackupSetResource/RelationManagers/BackupItemsRelationManager.php`
|
||
- [X] T024 [US2] Swap legacy enforcement helper imports to canonical RBAC helper in `app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php` and `app/Filament/Resources/RestoreRunResource/Pages/ListRestoreRuns.php`
|
||
- [X] T037 [US2] Verify any destructive-like Provider Connections actions still require explicit confirmation (no regressions), and that server authorization remains enforced for both header and empty-state CTAs
|
||
|
||
**Checkpoint**: US2 closes cross-plane leak + removes bypasses + restores correct 404/403 semantics.
|
||
|
||
---
|
||
|
||
## Phase 5: User Story 3 — Validate assignment operations safely in non-production contexts (Priority: P3)
|
||
|
||
**Goal**: Assignment-related Graph services depend only on `GraphClientInterface`, enabling tests to run with `NullGraphClient` or fakes without concrete client coupling.
|
||
|
||
**Independent Test**: Resolve the assignment Graph services from the container with Graph disabled and run a minimal flow that exercises the interface contract.
|
||
|
||
### Tests for User Story 3 (write first)
|
||
|
||
- [X] T025 [P] [US3] Add unit test asserting assignment Graph services resolve with `GraphClientInterface` binding in `tests/Feature/Graph/AssignmentGraphServiceResolutionTest.php`
|
||
- [X] T026 [P] [US3] Add test using a fake `GraphClientInterface` to simulate assignment fetch failures without real HTTP in `tests/Feature/Operations/AssignmentFetchOperationRunFailureTest.php`
|
||
|
||
### Implementation for User Story 3
|
||
|
||
- [X] T027 [P] [US3] Replace `MicrosoftGraphClient` constructor type-hint with `GraphClientInterface` in `app/Services/Graph/AssignmentFetcher.php`
|
||
- [X] T028 [P] [US3] Replace `MicrosoftGraphClient` constructor type-hint with `GraphClientInterface` in `app/Services/Graph/GroupResolver.php`
|
||
- [X] T029 [P] [US3] Replace `MicrosoftGraphClient` constructor type-hint with `GraphClientInterface` in `app/Services/Graph/AssignmentFilterResolver.php`
|
||
- [X] T030 [US3] Confirm container bindings remain canonical and no concrete client is injected directly by reviewing `app/Providers/AppServiceProvider.php`
|
||
|
||
**Checkpoint**: US3 enables deterministic tests and non-prod validation for assignment operations.
|
||
|
||
---
|
||
|
||
## Phase 6: Polish & Cross-Cutting Concerns
|
||
|
||
**Purpose**: Formatting + targeted verification commands + confidence checks.
|
||
|
||
- [X] T031 [P] Run formatting for touched files via `./vendor/bin/sail bin pint --dirty` (see `specs/094-assignment-ops-observability-hardening/quickstart.md`)
|
||
- [X] T032 Run targeted tests via `./vendor/bin/sail artisan test --compact tests/Feature/Operations tests/Feature/Rbac tests/Feature/Guards tests/Feature/Audit` (see `specs/094-assignment-ops-observability-hardening/quickstart.md`)
|
||
- [X] T033 Run the full test suite via `./vendor/bin/sail artisan test --compact` (see `specs/094-assignment-ops-observability-hardening/quickstart.md`)
|
||
- [X] T038 Update any requirement references in tasks/spec if FR numbering changes (keep traceability from FR-001..FR-012 to the task IDs)
|
||
|
||
---
|
||
|
||
## Dependencies & Execution Order
|
||
|
||
### Phase Dependencies
|
||
|
||
- **Setup (Phase 1)**: No dependencies.
|
||
- **Foundational (Phase 2)**: Depends on Phase 1.
|
||
- **US1 (Phase 3)**: Depends on Phase 2. No dependency on US2/US3.
|
||
- **US2 (Phase 4)**: Depends on Phase 2. Independent from US1/US3.
|
||
- **US3 (Phase 5)**: Depends on Phase 2. Independent from US1/US2.
|
||
- **Polish (Phase 6)**: Depends on completing the desired stories.
|
||
|
||
### User Story Dependencies
|
||
|
||
- **US1 (P1)**: Standalone MVP.
|
||
- **US2 (P2)**: Standalone hardening.
|
||
- **US3 (P3)**: Standalone testability hardening.
|
||
|
||
---
|
||
|
||
## Parallel Example: User Story 1
|
||
|
||
Parallelizable test tasks: T007, T008, T009, T010 (different files under `tests/Feature/...`).
|
||
|
||
---
|
||
|
||
## Parallel Example: User Story 2
|
||
|
||
Parallelizable test tasks: T018, T019, T020 (different files under `tests/Feature/...`).
|
||
|
||
Parallelizable implementation tasks (after tests land): T021 + T022 (different files under `routes/` vs `app/Filament/...`).
|
||
|
||
---
|
||
|
||
## Parallel Example: User Story 3
|
||
|
||
Parallelizable implementation tasks: T027, T028, T029 (different files under `app/Services/Graph/...`).
|
||
|
||
---
|
||
|
||
## Implementation Strategy
|
||
|
||
### MVP First (User Story 1 Only)
|
||
|
||
1. Complete Phase 1 (Setup) + Phase 2 (Foundational)
|
||
2. Complete Phase 3 (US1) including tests
|
||
3. Validate Monitoring visibility + audit log semantics
|
||
|
||
### Incremental Delivery
|
||
|
||
1. US1 → deploy/demo
|
||
2. US2 → deploy/demo
|
||
3. US3 → deploy/demo
|