diff --git a/.specify/plan.md b/.specify/plan.md index 2159414..fd2a1c2 100644 --- a/.specify/plan.md +++ b/.specify/plan.md @@ -24,7 +24,7 @@ ## Completed Workstreams (no new action needed) - **US1 Inventory (Phase 3)**: Filament policy listing with type/category/platform filters; tenant-scoped. - **US2 Backups (Phase 4)**: Backup sets/items in JSONB, immutable snapshots, audit logging, relation manager UX for attaching policies, soft-delete rules with restore-run guard. - **US3 Versions/Diffs (Phase 5)**: Version capture, timelines, human+JSON diffs, soft-deletes with audit. -- **US4 Restore (Phase 6)**: Preview, selective execution, conflict warnings, per-type restore level (enabled vs preview-only), PowerShell decode/encode respected, audit of outcomes; settings catalog fallback creates a new policy when the settings endpoint is unsupported, recording the new policy id and a manual cleanup warning. +- **US4 Restore (Phase 6)**: Preview, selective execution, conflict warnings, per-type restore level (enabled vs preview-only), PowerShell decode/encode respected, audit of outcomes; settings catalog fallback creates a new policy when the settings endpoint is unsupported, retrying metadata-only creation if settings are not accepted, recording the new policy id and manual warnings. - **US6 Tenant Setup & Highlander (Phases 8 & 12)**: Tenant CRUD/verify, INTUNE_TENANT_ID override, `is_current` unique enforcement, “Make current” action, block deactivated tenants. - **US6 Permissions/Health (Phase 9)**: Required permissions list, compare/check service, Verify action updates status and audit, permissions panel in Tenant detail. - **US1b Settings Display (Phase 13)**: PolicyNormalizer + SnapshotValidator, warnings for malformed snapshots, normalized settings and pretty JSON on policy/version detail, list badges, README section. diff --git a/.specify/spec.md b/.specify/spec.md index bc89b99..6ba9f44 100644 --- a/.specify/spec.md +++ b/.specify/spec.md @@ -362,7 +362,7 @@ ### Functional Requirements - **FR-020**: For PowerShell script objects (`deviceManagementScript` in `scope.supported_types`), the `scriptContent` MUST be base64-decoded when stored in backups/versions for readability/diffing and encoded again when sent back to Graph during restore. - **FR-021**: Restore behavior MUST follow the per-type configuration in `scope.restore_matrix`: `backup` determines full vs metadata-only snapshots; `restore` determines whether automated restore is enabled or preview-only; `risk` informs warning/confirmation UX. - **FR-022**: For high-risk types with `restore: preview-only` in `scope.restore_matrix` (e.g., `conditionalAccessPolicy`, `enrollmentRestriction`), TenantPilot MUST provide full backups, version history, and diffs plus detailed restore previews, but MUST NOT expose direct Graph apply actions; restore is manual, guided by the preview. -- **FR-036**: When `settingsCatalogPolicy` settings apply fails because the Graph settings endpoint is unsupported (route missing / method not allowed), the system MUST attempt a safe fallback by creating a new policy from the snapshot (including settings), record the new policy id, and mark the restore item as partial with a manual cleanup warning. +- **FR-036**: When `settingsCatalogPolicy` settings apply fails because the Graph settings endpoint is unsupported (route missing / method not allowed), the system MUST attempt a safe fallback by creating a new policy from the snapshot and record the new policy id. If creating with settings is not supported, the system MUST retry with a metadata-only payload, mark the restore item as partial, and surface a manual settings-apply warning. ### Key Entities *(include if feature involves data)* diff --git a/.specify/tasks.md b/.specify/tasks.md index f48cab7..d369dae 100644 --- a/.specify/tasks.md +++ b/.specify/tasks.md @@ -710,12 +710,12 @@ ### Implementation 2. **Sanitizer:** In `GraphContractRegistry` allow and preserve `@odata.type` inside `settingInstance` and nested children (recursively); continue to strip read-only/meta fields and `id`. 3. **RestoreService:** Build `settingsPayload = sanitizeSettingsApplyPayload(snapshot['settings'])` and `POST` to the contract path; on failure mark item `manual_required` and persist Graph meta (`request_id`, `client_request_id`, error message). 4. **UI:** RestoreRun Results view shows clear admin message when `manual_required` due to settings_apply, including request ids. -5. **Fallback create:** If the settings apply call fails with route missing / method not allowed, create a new Settings Catalog policy via `POST deviceManagement/configurationPolicies` using a sanitized payload that includes the settings. Record the new policy id in restore results and mark the item as partial with a manual cleanup warning. +5. **Fallback create:** If the settings apply call fails with route missing / method not allowed, create a new Settings Catalog policy via `POST deviceManagement/configurationPolicies` using a sanitized payload that includes the settings. If Graph returns `NotSupported`, retry with a metadata-only payload (no settings) and mark the item as partial with a manual settings apply warning. Record the new policy id in restore results. ### Tests (Pest) - Unit: `tests/Unit/GraphContractRegistrySettingsApplySanitizerTest.php` (preserve `@odata.type`, strip ids) - Feature: `tests/Feature/Filament/SettingsCatalogRestoreApplySettingsTest.php` (mock Graph, assert POST body includes `@odata.type` and success/failure flows) -- Feature: add a restore test that simulates a settings apply route-missing error and verifies fallback policy creation + new policy id recorded. +- Feature: add a restore test that simulates a settings apply route-missing error and verifies fallback policy creation + new policy id recorded, including metadata-only retry when create returns `NotSupported`. ### Verification - `./vendor/bin/pest tests/Unit/GraphContractRegistrySettingsApplySanitizerTest.php` diff --git a/app/Services/Intune/RestoreService.php b/app/Services/Intune/RestoreService.php index 993c4cb..1486068 100644 --- a/app/Services/Intune/RestoreService.php +++ b/app/Services/Intune/RestoreService.php @@ -150,6 +150,7 @@ public function execute( $settings = []; $resultReason = null; $createdPolicyId = null; + $createdPolicyMode = null; if ($item->policy_type === 'settingsCatalogPolicy') { $settings = $this->extractSettingsCatalogSettings($originalPayload); @@ -182,11 +183,17 @@ public function execute( if ($createOutcome['success']) { $createdPolicyId = $createOutcome['policy_id']; + $createdPolicyMode = $createOutcome['mode'] ?? null; $itemStatus = 'partial'; - $resultReason = 'Settings endpoint unsupported; created new policy. Manual cleanup required.'; + $mode = $createOutcome['mode'] ?? 'settings'; + + $resultReason = $mode === 'metadata_only' + ? 'Settings endpoint unsupported; created metadata-only policy. Manual settings apply required.' + : 'Settings endpoint unsupported; created new policy. Manual cleanup required.'; if ($settingsApply !== null && $createdPolicyId) { $settingsApply['created_policy_id'] = $createdPolicyId; + $settingsApply['created_policy_mode'] = $mode; } } elseif ($settingsApply !== null && $createOutcome['response']) { $settingsApply['issues'][] = [ @@ -259,6 +266,10 @@ public function execute( $result['created_policy_id'] = $createdPolicyId; } + if ($createdPolicyMode) { + $result['created_policy_mode'] = $createdPolicyMode; + } + if ($resultReason !== null) { $result['reason'] = $resultReason; } elseif ($itemStatus !== 'applied') { @@ -612,7 +623,7 @@ private function shouldAttemptSettingsCatalogCreate(array $settingsApply): bool } /** - * @return array{success:bool,policy_id:?string,response:?object} + * @return array{success:bool,policy_id:?string,response:?object,mode:string} */ private function createSettingsCatalogPolicy( array $originalPayload, @@ -629,10 +640,11 @@ private function createSettingsCatalogPolicy( 'success' => false, 'policy_id' => null, 'response' => null, + 'mode' => 'failed', ]; } - $payload = $this->buildSettingsCatalogCreatePayload($originalPayload, $sanitizedSettings, $fallbackName); + $payload = $this->buildSettingsCatalogCreatePayload($originalPayload, $sanitizedSettings, $fallbackName, true); $this->graphLogger->logRequest('create_settings_catalog_policy', $context + [ 'endpoint' => $resource, @@ -648,24 +660,67 @@ private function createSettingsCatalogPolicy( 'settings_count' => count($sanitizedSettings), ]); - $policyId = null; - if ($response->successful() && isset($response->data['id']) && is_string($response->data['id'])) { - $policyId = $response->data['id']; + $policyId = $this->extractCreatedPolicyId($response); + $mode = 'settings'; + + if ($response->failed() && $this->shouldRetrySettingsCatalogCreateWithoutSettings($response)) { + $fallbackPayload = $this->buildSettingsCatalogCreatePayload($originalPayload, $sanitizedSettings, $fallbackName, false); + + $this->graphLogger->logRequest('create_settings_catalog_policy_fallback', $context + [ + 'endpoint' => $resource, + 'method' => 'POST', + ]); + + $response = $this->graphClient->request('POST', $resource, ['json' => $fallbackPayload] + Arr::except($graphOptions, ['platform'])); + + $this->graphLogger->logResponse('create_settings_catalog_policy_fallback', $response, $context + [ + 'endpoint' => $resource, + 'method' => 'POST', + ]); + + $policyId = $this->extractCreatedPolicyId($response); + $mode = 'metadata_only'; } return [ 'success' => $response->successful(), 'policy_id' => $policyId, 'response' => $response, + 'mode' => $mode, ]; } + private function shouldRetrySettingsCatalogCreateWithoutSettings(object $response): bool + { + $code = strtolower((string) ($response->meta['error_code'] ?? '')); + $message = strtolower((string) ($response->meta['error_message'] ?? '')); + + if ($code === 'notsupported' || str_contains($code, 'notsupported')) { + return true; + } + + return str_contains($message, 'not supported'); + } + + private function extractCreatedPolicyId(object $response): ?string + { + if ($response->successful() && isset($response->data['id']) && is_string($response->data['id'])) { + return $response->data['id']; + } + + return null; + } + /** * @param array $settings * @return array */ - private function buildSettingsCatalogCreatePayload(array $originalPayload, array $settings, string $fallbackName): array - { + private function buildSettingsCatalogCreatePayload( + array $originalPayload, + array $settings, + string $fallbackName, + bool $includeSettings, + ): array { $payload = []; $name = $this->resolvePayloadString($originalPayload, ['name', 'displayName']); @@ -696,7 +751,7 @@ private function buildSettingsCatalogCreatePayload(array $originalPayload, array $payload['templateReference'] = $this->stripOdataAndReadOnly($templateReference); } - if ($settings !== []) { + if ($includeSettings && $settings !== []) { $payload['settings'] = $settings; } diff --git a/resources/views/filament/infolists/entries/restore-results.blade.php b/resources/views/filament/infolists/entries/restore-results.blade.php index a9ff440..2f15b6f 100644 --- a/resources/views/filament/infolists/entries/restore-results.blade.php +++ b/resources/views/filament/infolists/entries/restore-results.blade.php @@ -49,8 +49,14 @@ @endif @if (! empty($item['created_policy_id'])) + @php + $createdMode = $item['created_policy_mode'] ?? null; + $createdMessage = $createdMode === 'metadata_only' + ? 'New policy created (metadata only). Apply settings manually.' + : 'New policy created (manual cleanup required).'; + @endphp
- New policy created: {{ $item['created_policy_id'] }} (manual cleanup required). + {{ $createdMessage }} ID: {{ $item['created_policy_id'] }}
@endif diff --git a/tests/Feature/Filament/SettingsCatalogRestoreTest.php b/tests/Feature/Filament/SettingsCatalogRestoreTest.php index 04a80c4..24a7a79 100644 --- a/tests/Feature/Filament/SettingsCatalogRestoreTest.php +++ b/tests/Feature/Filament/SettingsCatalogRestoreTest.php @@ -434,7 +434,21 @@ public function request(string $method, string $path, array $options = []): Grap ], ); - $createResponse = new GraphResponse( + $createFailResponse = new GraphResponse( + success: false, + data: ['error' => ['code' => 'NotSupported', 'message' => 'NotSupported']], + status: 400, + errors: [['code' => 'NotSupported', 'message' => 'NotSupported']], + warnings: [], + meta: [ + 'error_code' => 'NotSupported', + 'error_message' => 'NotSupported', + 'request_id' => 'req-create-fail', + 'client_request_id' => 'client-create-fail', + ], + ); + + $createSuccessResponse = new GraphResponse( success: true, data: ['id' => 'new-policy-123'], status: 201, @@ -443,7 +457,7 @@ public function request(string $method, string $path, array $options = []): Grap meta: ['request_id' => 'req-create', 'client_request_id' => 'client-create'], ); - $client = new SettingsCatalogRestoreGraphClient($policyResponse, [$settingsResponse, $createResponse]); + $client = new SettingsCatalogRestoreGraphClient($policyResponse, [$settingsResponse, $createFailResponse, $createSuccessResponse]); app()->instance(GraphClientInterface::class, $client); @@ -513,11 +527,16 @@ public function request(string $method, string $path, array $options = []): Grap expect($run->status)->toBe('partial'); expect($run->results[0]['status'])->toBe('partial'); expect($run->results[0]['created_policy_id'])->toBe('new-policy-123'); + expect($run->results[0]['created_policy_mode'])->toBe('metadata_only'); expect($run->results[0]['settings_apply']['created_policy_id'])->toBe('new-policy-123'); + expect($run->results[0]['settings_apply']['created_policy_mode'])->toBe('metadata_only'); - expect($client->requestCalls)->toHaveCount(2); + expect($client->requestCalls)->toHaveCount(3); expect($client->requestCalls[0]['path'])->toBe('deviceManagement/configurationPolicies/scp-5/settings'); expect($client->requestCalls[1]['path'])->toBe('deviceManagement/configurationPolicies'); expect($client->requestCalls[1]['payload'])->toHaveKey('settings'); expect($client->requestCalls[1]['payload'])->toHaveKey('name'); + expect($client->requestCalls[2]['path'])->toBe('deviceManagement/configurationPolicies'); + expect($client->requestCalls[2]['payload'])->not->toHaveKey('settings'); + expect($client->requestCalls[2]['payload'])->toHaveKey('name'); });