diff --git a/app/Services/AssignmentRestoreService.php b/app/Services/AssignmentRestoreService.php index fcb290f..1505be7 100644 --- a/app/Services/AssignmentRestoreService.php +++ b/app/Services/AssignmentRestoreService.php @@ -51,12 +51,13 @@ public function restore( } $contract = $this->contracts->get($policyType); - $listPath = $this->resolvePath($contract['assignments_list_path'] ?? null, $policyId); - $deletePathTemplate = $contract['assignments_delete_path'] ?? null; $createPath = $this->resolvePath($contract['assignments_create_path'] ?? null, $policyId); $createMethod = strtoupper((string) ($contract['assignments_create_method'] ?? 'POST')); + $usesAssignAction = is_string($createPath) && str_ends_with($createPath, '/assign'); + $listPath = $this->resolvePath($contract['assignments_list_path'] ?? null, $policyId); + $deletePathTemplate = $contract['assignments_delete_path'] ?? null; - if (! $listPath || ! $createPath || ! $deletePathTemplate) { + if (! $createPath || (! $usesAssignAction && (! $listPath || ! $deletePathTemplate))) { $outcomes[] = $this->failureOutcome(null, 'Assignments endpoints are not configured for this policy type.'); $summary['failed']++; @@ -76,6 +77,138 @@ public function restore( 'restore_run_id' => $restoreRun?->id, ]; + $preparedAssignments = []; + $preparedMeta = []; + + foreach ($assignments as $assignment) { + if (! is_array($assignment)) { + continue; + } + + $groupId = $assignment['target']['groupId'] ?? null; + $mappedGroupId = $groupId && isset($groupMapping[$groupId]) ? $groupMapping[$groupId] : null; + + if ($mappedGroupId === 'SKIP') { + $outcomes[] = $this->skipOutcome($assignment, $groupId, $mappedGroupId); + $summary['skipped']++; + $this->logAssignmentOutcome( + status: 'skipped', + tenant: $tenant, + assignment: $assignment, + restoreRun: $restoreRun, + actorEmail: $actorEmail, + actorName: $actorName, + metadata: [ + 'policy_id' => $policyId, + 'policy_type' => $policyType, + 'group_id' => $groupId, + 'mapped_group_id' => $mappedGroupId, + ] + ); + + continue; + } + + $assignmentToRestore = $this->applyGroupMapping($assignment, $mappedGroupId); + $assignmentToRestore = $this->sanitizeAssignment($assignmentToRestore); + + $preparedAssignments[] = $assignmentToRestore; + $preparedMeta[] = [ + 'assignment' => $assignment, + 'group_id' => $groupId, + 'mapped_group_id' => $mappedGroupId, + ]; + } + + if ($preparedAssignments === []) { + return [ + 'outcomes' => $outcomes, + 'summary' => $summary, + ]; + } + + if ($usesAssignAction) { + $this->graphLogger->logRequest('restore_assignments_assign', $context + [ + 'method' => $createMethod, + 'endpoint' => $createPath, + 'assignments' => count($preparedAssignments), + ]); + + $assignResponse = $this->graphClient->request($createMethod, $createPath, [ + 'json' => ['assignments' => $preparedAssignments], + ] + $graphOptions); + + $this->graphLogger->logResponse('restore_assignments_assign', $assignResponse, $context + [ + 'method' => $createMethod, + 'endpoint' => $createPath, + 'assignments' => count($preparedAssignments), + ]); + + if ($assignResponse->successful()) { + foreach ($preparedMeta as $meta) { + $outcomes[] = $this->successOutcome( + $meta['assignment'], + $meta['group_id'], + $meta['mapped_group_id'] + ); + $summary['success']++; + $this->logAssignmentOutcome( + status: 'created', + tenant: $tenant, + assignment: $meta['assignment'], + restoreRun: $restoreRun, + actorEmail: $actorEmail, + actorName: $actorName, + metadata: [ + 'policy_id' => $policyId, + 'policy_type' => $policyType, + 'group_id' => $meta['group_id'], + 'mapped_group_id' => $meta['mapped_group_id'], + ] + ); + } + } else { + $reason = $assignResponse->meta['error_message'] ?? 'Graph assign failed'; + + if ($preparedMeta === []) { + $outcomes[] = $this->failureOutcome(null, $reason, null, null, $assignResponse); + $summary['failed']++; + } + + foreach ($preparedMeta as $meta) { + $outcomes[] = $this->failureOutcome( + $meta['assignment'], + $reason, + $meta['group_id'], + $meta['mapped_group_id'], + $assignResponse + ); + $summary['failed']++; + $this->logAssignmentOutcome( + status: 'failed', + tenant: $tenant, + assignment: $meta['assignment'], + restoreRun: $restoreRun, + actorEmail: $actorEmail, + actorName: $actorName, + metadata: [ + 'policy_id' => $policyId, + 'policy_type' => $policyType, + 'group_id' => $meta['group_id'], + 'mapped_group_id' => $meta['mapped_group_id'], + 'graph_error_message' => $assignResponse->meta['error_message'] ?? null, + 'graph_error_code' => $assignResponse->meta['error_code'] ?? null, + ], + ); + } + } + + return [ + 'outcomes' => $outcomes, + 'summary' => $summary, + ]; + } + $this->graphLogger->logRequest('restore_assignments_list', $context + [ 'method' => 'GET', 'endpoint' => $listPath, @@ -126,43 +259,18 @@ public function restore( } } - foreach ($assignments as $assignment) { - if (! is_array($assignment)) { + foreach ($preparedMeta as $index => $meta) { + $assignmentToRestore = $preparedAssignments[$index] ?? null; + + if (! is_array($assignmentToRestore)) { continue; } - $groupId = $assignment['target']['groupId'] ?? null; - $mappedGroupId = $groupId && isset($groupMapping[$groupId]) ? $groupMapping[$groupId] : null; - - if ($mappedGroupId === 'SKIP') { - $outcomes[] = $this->skipOutcome($assignment, $groupId, $mappedGroupId); - $summary['skipped']++; - $this->logAssignmentOutcome( - status: 'skipped', - tenant: $tenant, - assignment: $assignment, - restoreRun: $restoreRun, - actorEmail: $actorEmail, - actorName: $actorName, - metadata: [ - 'policy_id' => $policyId, - 'policy_type' => $policyType, - 'group_id' => $groupId, - 'mapped_group_id' => $mappedGroupId, - ] - ); - - continue; - } - - $assignmentToRestore = $this->applyGroupMapping($assignment, $mappedGroupId); - $assignmentToRestore = $this->sanitizeAssignment($assignmentToRestore); - $this->graphLogger->logRequest('restore_assignments_create', $context + [ 'method' => $createMethod, 'endpoint' => $createPath, - 'group_id' => $groupId, - 'mapped_group_id' => $mappedGroupId, + 'group_id' => $meta['group_id'], + 'mapped_group_id' => $meta['mapped_group_id'], ]); $createResponse = $this->graphClient->request($createMethod, $createPath, [ @@ -172,48 +280,48 @@ public function restore( $this->graphLogger->logResponse('restore_assignments_create', $createResponse, $context + [ 'method' => $createMethod, 'endpoint' => $createPath, - 'group_id' => $groupId, - 'mapped_group_id' => $mappedGroupId, + 'group_id' => $meta['group_id'], + 'mapped_group_id' => $meta['mapped_group_id'], ]); if ($createResponse->successful()) { - $outcomes[] = $this->successOutcome($assignment, $groupId, $mappedGroupId); + $outcomes[] = $this->successOutcome($meta['assignment'], $meta['group_id'], $meta['mapped_group_id']); $summary['success']++; $this->logAssignmentOutcome( status: 'created', tenant: $tenant, - assignment: $assignment, + assignment: $meta['assignment'], restoreRun: $restoreRun, actorEmail: $actorEmail, actorName: $actorName, metadata: [ 'policy_id' => $policyId, 'policy_type' => $policyType, - 'group_id' => $groupId, - 'mapped_group_id' => $mappedGroupId, + 'group_id' => $meta['group_id'], + 'mapped_group_id' => $meta['mapped_group_id'], ] ); } else { $outcomes[] = $this->failureOutcome( - $assignment, + $meta['assignment'], $createResponse->meta['error_message'] ?? 'Graph create failed', - $groupId, - $mappedGroupId, + $meta['group_id'], + $meta['mapped_group_id'], $createResponse ); $summary['failed']++; $this->logAssignmentOutcome( status: 'failed', tenant: $tenant, - assignment: $assignment, + assignment: $meta['assignment'], restoreRun: $restoreRun, actorEmail: $actorEmail, actorName: $actorName, metadata: [ 'policy_id' => $policyId, 'policy_type' => $policyType, - 'group_id' => $groupId, - 'mapped_group_id' => $mappedGroupId, + 'group_id' => $meta['group_id'], + 'mapped_group_id' => $meta['mapped_group_id'], 'graph_error_message' => $createResponse->meta['error_message'] ?? null, 'graph_error_code' => $createResponse->meta['error_code'] ?? null, ], diff --git a/config/graph_contracts.php b/config/graph_contracts.php index 35e6886..e77eff5 100644 --- a/config/graph_contracts.php +++ b/config/graph_contracts.php @@ -73,7 +73,7 @@ // Assignments CRUD (standard Graph pattern) 'assignments_list_path' => '/deviceManagement/configurationPolicies/{id}/assignments', - 'assignments_create_path' => '/deviceManagement/configurationPolicies/{id}/assignments', + 'assignments_create_path' => '/deviceManagement/configurationPolicies/{id}/assign', 'assignments_create_method' => 'POST', 'assignments_update_path' => '/deviceManagement/configurationPolicies/{id}/assignments/{assignmentId}', 'assignments_update_method' => 'PATCH', diff --git a/resources/views/filament/infolists/entries/restore-results.blade.php b/resources/views/filament/infolists/entries/restore-results.blade.php index c95c264..8c429e3 100644 --- a/resources/views/filament/infolists/entries/restore-results.blade.php +++ b/resources/views/filament/infolists/entries/restore-results.blade.php @@ -42,9 +42,14 @@ - @if (! empty($item['reason'])) + @php + $itemReason = $item['reason'] ?? null; + $itemGraphMessage = $item['graph_error_message'] ?? null; + @endphp + + @if (! empty($itemReason) && ($itemGraphMessage === null || $itemGraphMessage !== $itemReason))
- {{ $item['reason'] }} + {{ $itemReason }}
@endif @@ -95,9 +100,14 @@ @endif - @if (! empty($outcome['reason'])) + @php + $outcomeReason = $outcome['reason'] ?? null; + $outcomeGraphMessage = $outcome['graph_error_message'] ?? null; + @endphp + + @if (! empty($outcomeReason) && ($outcomeGraphMessage === null || $outcomeGraphMessage !== $outcomeReason))
- {{ $outcome['reason'] }} + {{ $outcomeReason }}
@endif diff --git a/resources/views/livewire/policy-version-assignments-widget.blade.php b/resources/views/livewire/policy-version-assignments-widget.blade.php index 59c3e31..fb6238b 100644 --- a/resources/views/livewire/policy-version-assignments-widget.blade.php +++ b/resources/views/livewire/policy-version-assignments-widget.blade.php @@ -53,23 +53,26 @@ @foreach($version->assignments as $assignment) @php $target = $assignment['target'] ?? []; - $type = $target['@odata.type'] ?? 'unknown'; + $type = $target['@odata.type'] ?? ''; + $typeKey = strtolower((string) $type); $intent = $assignment['intent'] ?? 'apply'; - $typeName = match($type) { - '#microsoft.graph.groupAssignmentTarget' => 'Include group', - '#microsoft.graph.exclusionGroupAssignmentTarget' => 'Exclude group', - '#microsoft.graph.allLicensedUsersAssignmentTarget' => 'All Users', - '#microsoft.graph.allDevicesAssignmentTarget' => 'All Devices', - default => 'Unknown' + $typeName = match (true) { + str_contains($typeKey, 'exclusiongroupassignmenttarget') => 'Exclude group', + str_contains($typeKey, 'groupassignmenttarget') => 'Include group', + str_contains($typeKey, 'alllicensedusersassignmenttarget') => 'All Users', + str_contains($typeKey, 'alldevicesassignmenttarget') => 'All Devices', + default => 'Unknown', }; $groupId = $target['groupId'] ?? null; $groupName = $target['group_display_name'] ?? null; $groupOrphaned = $target['group_orphaned'] ?? ($version->metadata['has_orphaned_assignments'] ?? false); $filterId = $target['deviceAndAppManagementAssignmentFilterId'] ?? null; - $filterType = $target['deviceAndAppManagementAssignmentFilterType'] ?? 'none'; + $filterTypeRaw = strtolower((string) ($target['deviceAndAppManagementAssignmentFilterType'] ?? 'none')); + $filterType = $filterTypeRaw !== '' ? $filterTypeRaw : 'none'; $filterName = $target['assignment_filter_name'] ?? null; + $filterLabel = $filterName ?? $filterId; @endphp
@@ -96,9 +99,9 @@ @endif @endif - @if($filterId && $filterType !== 'none') + @if($filterLabel) - Filter ({{ $filterType }}): {{ $filterName ?? $filterId }} + Filter{{ $filterType !== 'none' ? " ({$filterType})" : '' }}: {{ $filterLabel }} @endif diff --git a/specs/004-assignments-scope-tags/spec.md b/specs/004-assignments-scope-tags/spec.md index 7229547..2011a3c 100644 --- a/specs/004-assignments-scope-tags/spec.md +++ b/specs/004-assignments-scope-tags/spec.md @@ -28,7 +28,8 @@ ## Scope - **Policy Types**: `settingsCatalogPolicy` only (initially) - **Graph Endpoints**: - GET `/deviceManagement/configurationPolicies/{id}/assignments` - - POST/PATCH `/deviceManagement/configurationPolicies/{id}/assignments` + - POST `/deviceManagement/configurationPolicies/{id}/assign` (assign action, replaces assignments) + - DELETE `/deviceManagement/configurationPolicies/{id}/assignments/{assignmentId}` (fallback) - GET `/deviceManagement/roleScopeTags` (for reference data) - GET `/deviceManagement/assignmentFilters` (for filter names) - **Backup Behavior**: Optional at capture time with separate checkboxes ("Include assignments", "Include scope tags") on Add Policies and Capture Snapshot actions (defaults: true) @@ -190,10 +191,12 @@ ### Restore with Group Mapping 1. Replace source group IDs with mapped target group IDs in assignment objects 2. Skip assignments marked "Skip" in group mapping 3. Preserve include/exclude intent and filters -4. Execute restore via DELETE-then-CREATE pattern: - - Step 1: GET existing assignments from target policy - - Step 2: DELETE each existing assignment (via DELETE `/assignments/{id}`) - - Step 3: POST each new/mapped assignment (via POST `/assignments`) +4. Execute restore via assign action when supported: + - Step 1: POST `/assign` with `{ assignments: [...] }` to replace assignments + - Step 2 (fallback): If `/assign` is unsupported, use DELETE-then-CREATE: + - GET existing assignments from target policy + - DELETE each existing assignment (via DELETE `/assignments/{id}`) + - POST each new/mapped assignment (via POST `/assignments`) 5. Handle failures gracefully: - 204 No Content on DELETE = success - 201 Created on POST = success @@ -316,30 +319,32 @@ ### Endpoints to Add (Production-Tested Strategies) - Client-side filter to extract assignments - **Reason**: Known Graph API quirks with assignment expansion on certain template families -2. **Assignment CRUD Operations** (Standard Graph Pattern) +2. **Assignment Apply** (Assign action + fallback) - - **POST** `/deviceManagement/configurationPolicies/{id}/assignments` - - Body: Single assignment object - - Returns: 201 Created with assignment object + - **POST** `/deviceManagement/configurationPolicies/{id}/assign` + - Body: `{ "assignments": [ ... ] }` + - Returns: 200/204 on success (no per-assignment IDs) - Example: ```json { - "target": { - "@odata.type": "#microsoft.graph.groupAssignmentTarget", - "groupId": "abc-123-def" - }, - "intent": "apply" + "assignments": [ + { + "target": { + "@odata.type": "#microsoft.graph.groupAssignmentTarget", + "groupId": "abc-123-def" + }, + "intent": "apply" + } + ] } ``` + + - **Fallback** (when `/assign` is unsupported): + - **GET** `/deviceManagement/configurationPolicies/{id}/assignments` + - **DELETE** `/deviceManagement/configurationPolicies/{id}/assignments/{assignmentId}` + - **POST** `/deviceManagement/configurationPolicies/{id}/assignments` (single assignment object) - - **PATCH** `/deviceManagement/configurationPolicies/{id}/assignments/{assignmentId}` - - Body: Assignment object (partial update) - - Returns: 200 OK with updated assignment - - - **DELETE** `/deviceManagement/configurationPolicies/{id}/assignments/{assignmentId}` - - Returns: 204 No Content - - - **Restore Strategy**: DELETE all existing assignments, then POST new ones (best-effort; record per-assignment outcomes, no transactional rollback) + - **Restore Strategy**: Prefer `/assign`; if unsupported, delete existing assignments then POST new ones (best-effort; record outcomes, no transactional rollback). 3. **POST** `/directoryObjects/getByIds` (Stable Group Resolution) - Body: `{ "ids": ["id1", "id2"], "types": ["group"] }` @@ -372,7 +377,7 @@ ### Graph Contract Updates // Assignments CRUD (standard Graph pattern) 'assignments_list_path' => '/deviceManagement/configurationPolicies/{id}/assignments', - 'assignments_create_path' => '/deviceManagement/configurationPolicies/{id}/assignments', + 'assignments_create_path' => '/deviceManagement/configurationPolicies/{id}/assign', 'assignments_create_method' => 'POST', 'assignments_update_path' => '/deviceManagement/configurationPolicies/{id}/assignments/{assignmentId}', 'assignments_update_method' => 'PATCH', diff --git a/specs/004-assignments-scope-tags/tasks.md b/specs/004-assignments-scope-tags/tasks.md index 1229786..e464608 100644 --- a/specs/004-assignments-scope-tags/tasks.md +++ b/specs/004-assignments-scope-tags/tasks.md @@ -77,7 +77,7 @@ ### Tasks **1.9** [X] ⭐ Update `config/graph_contracts.php` with assignments endpoints - Add `assignments_list_path` (GET) -- Add `assignments_create_path` (POST) +- Add `assignments_create_path` (POST `/assign` for settingsCatalogPolicy) - Add `assignments_delete_path` (DELETE) - Add `supports_scope_tags: true` - Add `scope_tag_field: 'roleScopeTagIds'` @@ -351,15 +351,15 @@ ### Tasks **5.8** Create service: `AssignmentRestoreService` - File: `app/Services/AssignmentRestoreService.php` - Method: `restore(string $policyId, array $assignments, array $groupMapping): array` -- Implement DELETE-then-CREATE pattern +- Prefer `/assign` action when supported; fallback to DELETE-then-CREATE pattern -**5.9** Implement DELETE existing assignments +**5.9** Implement DELETE existing assignments (fallback) - Step 1: GET `/assignments` for target policy - Step 2: Loop and DELETE each assignment - Handle 204 No Content (success) - Log warnings on failure, continue -**5.10** Implement CREATE new assignments with mapping +**5.10** Implement CREATE new assignments with mapping (fallback) - Step 3: Loop through source assignments - Apply group mapping: replace source group IDs with target IDs - Skip assignments marked `"SKIP"` in mapping @@ -367,7 +367,7 @@ ### Tasks - Handle 201 Created (success) - Log per-assignment outcome -**5.11** Add rate limit protection +**5.11** Add rate limit protection (fallback only) - Add 100ms delay between sequential POST calls: `usleep(100000)` - Log request IDs for failed calls diff --git a/tests/Feature/RestoreAssignmentApplicationTest.php b/tests/Feature/RestoreAssignmentApplicationTest.php index b8d529b..f2a2b4b 100644 --- a/tests/Feature/RestoreAssignmentApplicationTest.php +++ b/tests/Feature/RestoreAssignmentApplicationTest.php @@ -67,10 +67,7 @@ public function request(string $method, string $path, array $options = []): Grap test('restore applies assignments with mapped groups', function () { $applyResponse = new GraphResponse(true, []); $requestResponses = [ - new GraphResponse(true, ['value' => [['id' => 'assign-old-1']]]), // list - new GraphResponse(true, [], 204), // delete - new GraphResponse(true, ['id' => 'assign-new-1'], 201), // create 1 - new GraphResponse(true, ['id' => 'assign-new-2'], 201), // create 2 + new GraphResponse(true, []), // assign action ]; $client = new RestoreAssignmentGraphClient($applyResponse, $requestResponses); @@ -153,23 +150,25 @@ public function request(string $method, string $path, array $options = []): Grap ->filter(fn (array $call) => $call['method'] === 'POST') ->values(); - expect($postCalls)->toHaveCount(2); - expect($postCalls[0]['payload']['target']['groupId'])->toBe('target-group-1'); - expect($postCalls[0]['payload'])->not->toHaveKey('id'); + expect($postCalls)->toHaveCount(1); + expect($postCalls[0]['path'])->toBe('/deviceManagement/configurationPolicies/scp-1/assign'); + + $payloadAssignments = $postCalls[0]['payload']['assignments'] ?? []; + $groupIds = collect($payloadAssignments)->pluck('target.groupId')->all(); + + expect($groupIds)->toBe(['target-group-1', 'target-group-2']); + expect($payloadAssignments[0])->not->toHaveKey('id'); }); test('restore handles assignment failures gracefully', function () { $applyResponse = new GraphResponse(true, []); $requestResponses = [ - new GraphResponse(true, ['value' => [['id' => 'assign-old-1']]]), // list - new GraphResponse(true, [], 204), // delete - new GraphResponse(true, ['id' => 'assign-new-1'], 201), // create 1 new GraphResponse(false, ['error' => ['message' => 'Bad request']], 400, [ ['code' => 'BadRequest', 'message' => 'Bad request'], ], [], [ 'error_code' => 'BadRequest', 'error_message' => 'Bad request', - ]), // create 2 fails + ]), // assign action fails ]; $client = new RestoreAssignmentGraphClient($applyResponse, $requestResponses); @@ -244,7 +243,7 @@ public function request(string $method, string $path, array $options = []): Grap $summary = $run->results[0]['assignment_summary'] ?? null; expect($summary)->not->toBeNull(); - expect($summary['success'])->toBe(1); - expect($summary['failed'])->toBe(1); + expect($summary['success'])->toBe(0); + expect($summary['failed'])->toBe(2); expect($run->results[0]['status'])->toBe('partial'); });