From 97f9017909cf9cf8385ddb34a5df2134ae7261ce Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Sat, 10 Jan 2026 02:02:40 +0100 Subject: [PATCH] feat: hydrate configuration policy assignments for dependency edges --- .../Inventory/DependencyExtractionService.php | 76 ++++++++++++--- .../Inventory/InventorySyncService.php | 75 ++++++++++++++- app/Support/Enums/RelationshipType.php | 6 ++ .../contracts/dependency-edge.schema.json | 5 +- .../data-model.md | 6 ++ .../042-inventory-dependencies-graph/spec.md | 6 +- .../042-inventory-dependencies-graph/tasks.md | 2 +- .../DependencyExtractionFeatureTest.php | 93 +++++++++++++++++++ 8 files changed, 250 insertions(+), 19 deletions(-) diff --git a/app/Services/Inventory/DependencyExtractionService.php b/app/Services/Inventory/DependencyExtractionService.php index 54ad580..b197564 100644 --- a/app/Services/Inventory/DependencyExtractionService.php +++ b/app/Services/Inventory/DependencyExtractionService.php @@ -22,12 +22,14 @@ public function extractForPolicyData(InventoryItem $item, array $policyData): ar $warnings = []; $edges = collect(); - $edges = $edges->merge($this->extractAssignedTo($item, $policyData, $warnings)); + $edges = $edges->merge($this->extractAssignments($item, $policyData, $warnings)); $edges = $edges->merge($this->extractScopedBy($item, $policyData)); // Enforce max 50 outbound edges by priority: assigned_to > scoped_by > others $priorities = [ - RelationshipType::AssignedTo->value => 1, + RelationshipType::AssignedToInclude->value => 1, + RelationshipType::AssignedToExclude->value => 2, + RelationshipType::UsesAssignmentFilter->value => 3, RelationshipType::ScopedBy->value => 2, RelationshipType::Targets->value => 3, RelationshipType::DependsOn->value => 4, @@ -67,7 +69,7 @@ public function extractForPolicyData(InventoryItem $item, array $policyData): ar * @param array $policyData * @return Collection> */ - private function extractAssignedTo(InventoryItem $item, array $policyData, array &$warnings): Collection + private function extractAssignments(InventoryItem $item, array $policyData, array &$warnings): Collection { $assignments = Arr::get($policyData, 'assignments'); if (! is_array($assignments)) { @@ -81,32 +83,78 @@ private function extractAssignedTo(InventoryItem $item, array $policyData, array continue; } - // Known shapes: ['target' => ['groupId' => '...']] or direct ['groupId' => '...'] + $policyId = (string) ($policyData['id'] ?? $item->external_id); + + $target = Arr::get($assignment, 'target'); + $odataType = is_array($target) ? (Arr::get($target, '@odata.type') ?? Arr::get($target, '@OData.Type')) : null; + $odataType = is_string($odataType) ? strtolower($odataType) : null; + $groupId = Arr::get($assignment, 'target.groupId') ?? Arr::get($assignment, 'groupId'); + $groupId = is_string($groupId) ? trim($groupId) : null; + + $filterId = Arr::get($assignment, 'deviceAndAppManagementAssignmentFilterId') + ?? Arr::get($assignment, 'assignmentFilterId') + ?? Arr::get($assignment, 'target.deviceAndAppManagementAssignmentFilterId'); + $filterId = is_string($filterId) ? trim($filterId) : null; + + $filterType = Arr::get($assignment, 'deviceAndAppManagementAssignmentFilterType') + ?? Arr::get($assignment, 'assignmentFilterType') + ?? Arr::get($assignment, 'target.deviceAndAppManagementAssignmentFilterType'); + $filterType = is_string($filterType) ? strtolower(trim($filterType)) : null; + $filterMode = in_array($filterType, ['include', 'exclude'], true) ? $filterType : null; + + if (is_string($filterId) && $filterId !== '') { + $edges[] = [ + 'tenant_id' => (int) $item->tenant_id, + 'source_type' => 'inventory_item', + 'source_id' => (string) $item->external_id, + 'target_type' => 'foundation_object', + 'target_id' => $filterId, + 'relationship_type' => RelationshipType::UsesAssignmentFilter->value, + 'metadata' => array_filter([ + 'last_known_name' => null, + 'foundation_type' => 'assignment_filter', + 'filter_mode' => $filterMode, + ], fn ($v) => $v !== null), + ]; + } + if (is_string($groupId) && $groupId !== '') { + $relationshipType = RelationshipType::AssignedToInclude->value; + if (is_string($odataType) && str_contains($odataType, 'exclusion')) { + $relationshipType = RelationshipType::AssignedToExclude->value; + } + $edges[] = [ 'tenant_id' => (int) $item->tenant_id, 'source_type' => 'inventory_item', 'source_id' => (string) $item->external_id, 'target_type' => 'foundation_object', 'target_id' => $groupId, - 'relationship_type' => RelationshipType::AssignedTo->value, + 'relationship_type' => $relationshipType, 'metadata' => [ 'last_known_name' => null, 'foundation_type' => 'aad_group', ], ]; - } else { - $warning = [ - 'type' => 'unsupported_reference', - 'policy_id' => (string) ($policyData['id'] ?? $item->external_id), - 'raw_ref' => $assignment, - 'reason' => 'unsupported_assignment_target_shape', - ]; - $warnings[] = $warning; - Log::info('Unsupported reference shape encountered', $warning); + continue; } + + // Known non-group targets (e.g. allDevices/allLicensedUsers) are out-of-scope for edges. + if (is_string($odataType) && (str_contains($odataType, 'alldevices') || str_contains($odataType, 'alllicensedusers') || str_contains($odataType, 'allusers'))) { + continue; + } + + $warning = [ + 'type' => 'unsupported_reference', + 'policy_id' => $policyId, + 'raw_ref' => $assignment, + 'reason' => 'unsupported_assignment_target_shape', + ]; + + $warnings[] = $warning; + Log::info('Unsupported reference shape encountered', $warning); } return collect($edges); diff --git a/app/Services/Inventory/InventorySyncService.php b/app/Services/Inventory/InventorySyncService.php index e581016..75c7a57 100644 --- a/app/Services/Inventory/InventorySyncService.php +++ b/app/Services/Inventory/InventorySyncService.php @@ -12,6 +12,7 @@ use Carbon\CarbonImmutable; use Illuminate\Contracts\Cache\Lock; use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Facades\Log; use Throwable; class InventorySyncService @@ -290,6 +291,18 @@ private function executeRun(InventorySyncRun $run, Tenant $tenant, array $normal $observed++; + $includeDeps = (bool) ($normalizedSelection['include_dependencies'] ?? true); + + if ($includeDeps && $this->shouldHydrateAssignments($policyType)) { + $existingAssignments = $policyData['assignments'] ?? null; + if (! is_array($existingAssignments)) { + $hydratedAssignments = $this->fetchAssignmentsForPolicyType($policyType, $tenant, $externalId, $warnings); + if (is_array($hydratedAssignments)) { + $policyData['assignments'] = $hydratedAssignments; + } + } + } + $displayName = $policyData['displayName'] ?? $policyData['name'] ?? null; $displayName = is_string($displayName) ? $displayName : null; @@ -327,7 +340,6 @@ private function executeRun(InventorySyncRun $run, Tenant $tenant, array $normal $upserted++; // Extract dependencies if requested in selection - $includeDeps = (bool) ($normalizedSelection['include_dependencies'] ?? true); if ($includeDeps) { $warnings = array_merge( $warnings, @@ -386,6 +398,67 @@ private function shouldSkipPolicyForSelectedType(string $selectedPolicyType, arr return $this->resolveConfigurationPolicyType($policyData) !== $selectedPolicyType; } + private function shouldHydrateAssignments(string $policyType): bool + { + return in_array($policyType, ['settingsCatalogPolicy', 'endpointSecurityPolicy', 'securityBaselinePolicy'], true); + } + + /** + * @param array> $warnings + * @return null|array + */ + private function fetchAssignmentsForPolicyType(string $policyType, Tenant $tenant, string $externalId, array &$warnings): ?array + { + $pathTemplate = config("graph_contracts.types.{$policyType}.assignments_list_path"); + if (! is_string($pathTemplate) || $pathTemplate === '') { + return null; + } + + $path = str_replace('{id}', $externalId, $pathTemplate); + + $options = [ + 'tenant' => $tenant->tenant_id ?? $tenant->external_id, + 'client_id' => $tenant->app_client_id, + 'client_secret' => $tenant->app_client_secret, + ]; + + $maxAttempts = 3; + + for ($attempt = 1; $attempt <= $maxAttempts; $attempt++) { + $response = $this->graphClient->request('GET', $path, $options); + + if (! $response->failed()) { + $data = $response->data; + if (is_array($data) && array_key_exists('value', $data) && is_array($data['value'])) { + return $data['value']; + } + + if (is_array($data)) { + return $data; + } + + return null; + } + + $status = (int) ($response->status ?? 0); + if (! in_array($status, [429, 503], true)) { + break; + } + } + + $warning = [ + 'type' => 'assignments_fetch_failed', + 'policy_id' => $externalId, + 'policy_type' => $policyType, + 'reason' => 'graph_assignments_list_failed', + ]; + + $warnings[] = $warning; + Log::info('Failed to fetch policy assignments', $warning); + + return null; + } + private function resolveConfigurationPolicyType(array $policyData): string { $templateReference = $policyData['templateReference'] ?? null; diff --git a/app/Support/Enums/RelationshipType.php b/app/Support/Enums/RelationshipType.php index 2c1f101..69ced31 100644 --- a/app/Support/Enums/RelationshipType.php +++ b/app/Support/Enums/RelationshipType.php @@ -5,6 +5,9 @@ enum RelationshipType: string { case AssignedTo = 'assigned_to'; + case AssignedToInclude = 'assigned_to_include'; + case AssignedToExclude = 'assigned_to_exclude'; + case UsesAssignmentFilter = 'uses_assignment_filter'; case ScopedBy = 'scoped_by'; case Targets = 'targets'; case DependsOn = 'depends_on'; @@ -13,6 +16,9 @@ public function label(): string { return match ($this) { self::AssignedTo => 'Assigned to', + self::AssignedToInclude => 'Assigned to (include)', + self::AssignedToExclude => 'Assigned to (exclude)', + self::UsesAssignmentFilter => 'Uses assignment filter', self::ScopedBy => 'Scoped by', self::Targets => 'Targets', self::DependsOn => 'Depends on', diff --git a/specs/042-inventory-dependencies-graph/contracts/dependency-edge.schema.json b/specs/042-inventory-dependencies-graph/contracts/dependency-edge.schema.json index 922a2a2..d4a91cf 100644 --- a/specs/042-inventory-dependencies-graph/contracts/dependency-edge.schema.json +++ b/specs/042-inventory-dependencies-graph/contracts/dependency-edge.schema.json @@ -17,14 +17,15 @@ "source_id": { "type": "string" }, "target_type": { "type": "string", "enum": ["inventory_item", "foundation_object", "missing"] }, "target_id": { "type": ["string", "null"] }, - "relationship_type": { "type": "string", "enum": ["assigned_to", "scoped_by", "targets", "depends_on"] }, + "relationship_type": { "type": "string", "enum": ["assigned_to", "assigned_to_include", "assigned_to_exclude", "uses_assignment_filter", "scoped_by", "targets", "depends_on"] }, "metadata": { "type": ["object", "null"], "additionalProperties": true, "properties": { "last_known_name": { "type": ["string", "null"] }, "raw_ref": {}, - "foundation_type": { "type": "string", "enum": ["aad_group", "scope_tag", "device_category"] } + "foundation_type": { "type": "string", "enum": ["aad_group", "scope_tag", "device_category", "assignment_filter"] }, + "filter_mode": { "type": ["string", "null"], "enum": ["include", "exclude", null] } } }, "created_at": { "type": ["string", "null"], "description": "ISO-8601 timestamp" }, diff --git a/specs/042-inventory-dependencies-graph/data-model.md b/specs/042-inventory-dependencies-graph/data-model.md index 61e7b7d..785163d 100644 --- a/specs/042-inventory-dependencies-graph/data-model.md +++ b/specs/042-inventory-dependencies-graph/data-model.md @@ -52,10 +52,16 @@ #### InventoryLink.metadata Required when `target_type='foundation_object'`: - `foundation_type` (string enum-like): `aad_group` | `scope_tag` | `device_category` +Additional metadata (when applicable): +- `filter_mode` (string enum-like): `include` | `exclude` (for `foundation_type='assignment_filter'`) + ## Enums ### RelationshipType - `assigned_to` +- `assigned_to_include` +- `assigned_to_exclude` +- `uses_assignment_filter` - `scoped_by` - `targets` - `depends_on` diff --git a/specs/042-inventory-dependencies-graph/spec.md b/specs/042-inventory-dependencies-graph/spec.md index 9ba0913..655336f 100644 --- a/specs/042-inventory-dependencies-graph/spec.md +++ b/specs/042-inventory-dependencies-graph/spec.md @@ -58,7 +58,10 @@ ## Functional Requirements - **FR1: Relationship taxonomy** Define a normalized set of relationship types covering inventory→inventory and inventory→foundation edges. Supported types (MVP): - - `assigned_to` (Policy → AAD Group) + - `assigned_to` (Policy → AAD Group) *(legacy/general)* + - `assigned_to_include` (Policy → AAD Group; include assignment) + - `assigned_to_exclude` (Policy → AAD Group; exclude assignment) + - `uses_assignment_filter` (Policy → Assignment Filter; metadata `filter_mode=include|exclude`) - `scoped_by` (Policy → Scope Tag) - `targets` (Update Policy → Device Category, conditional logic) - `depends_on` (Generic prerequisite, e.g., Compliance Policy referenced by Conditional Access) @@ -85,6 +88,7 @@ ## Functional Requirements - AAD Groups (`aad_group`) - Scope Tags (`scope_tag`) - Device Categories (`device_category`) + - Assignment Filters (`assignment_filter`) **Out-of-scope foundation types** (for this iteration): Conditional Access Policies, Compliance Policies as foundation nodes (only as inventory items). diff --git a/specs/042-inventory-dependencies-graph/tasks.md b/specs/042-inventory-dependencies-graph/tasks.md index 780e294..75c4481 100644 --- a/specs/042-inventory-dependencies-graph/tasks.md +++ b/specs/042-inventory-dependencies-graph/tasks.md @@ -36,7 +36,7 @@ ## Phase 2: Foundational (Blocking Prerequisites) - [x] T004 Ensure `inventory_links` schema (unique key + indexes) matches spec in `database/migrations/2026_01_07_150000_create_inventory_links_table.php` - [x] T005 [P] Ensure `InventoryLink` casts and tenant-safe query patterns in `app/Models/InventoryLink.php` - [x] T006 [P] Ensure factory coverage for dependency edges in `database/factories/InventoryLinkFactory.php` -- [x] T007 Align idempotent upsert semantics for edges in `app/Services/Inventory/DependencyExtractionService.php` +- [x] T007 Align idempotent upsert semantics for edges in `app/Services/Inventory/DependencyExtractionService.php` (including hydrated assignments when not present in list payloads) - [x] T008 Implement warning-only handling for unknown/unsupported reference shapes in `app/Services/Inventory/DependencyExtractionService.php` - [x] T009 Persist warnings on the sync run record at `InventorySyncRun.error_context.warnings[]` via `app/Services/Inventory/InventorySyncService.php` - [x] T010 Implement limit-only inbound/outbound queries with optional relationship filter in `app/Services/Inventory/DependencyQueryService.php` (ordered by `created_at DESC`) diff --git a/tests/Feature/DependencyExtractionFeatureTest.php b/tests/Feature/DependencyExtractionFeatureTest.php index cbfc0ac..c2e35e0 100644 --- a/tests/Feature/DependencyExtractionFeatureTest.php +++ b/tests/Feature/DependencyExtractionFeatureTest.php @@ -273,3 +273,96 @@ public function request(string $method, string $path, array $options = []): Grap expect($inbound[1]->source_id)->toBe('bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb'); expect($inbound[0]->created_at->greaterThan($inbound[1]->created_at))->toBeTrue(); }); + +it('hydrates settings catalog assignments and extracts include/exclude/filter edges', function () { + $tenant = Tenant::factory()->create(); + + $this->app->bind(GraphClientInterface::class, function () { + return new class implements GraphClientInterface + { + public function listPolicies(string $policyType, array $options = []): GraphResponse + { + return new GraphResponse(true, [[ + 'id' => 'sc-1', + 'name' => 'Settings Catalog Policy', + 'roleScopeTagIds' => ['scope-tag-1'], + // assignments omitted intentionally (must be hydrated via /assignments) + ]], 200); + } + + public function request(string $method, string $path, array $options = []): GraphResponse + { + if ($method === 'GET' && $path === '/deviceManagement/configurationPolicies/sc-1/assignments') { + return new GraphResponse(true, [ + 'value' => [ + [ + 'target' => [ + '@odata.type' => '#microsoft.graph.groupAssignmentTarget', + 'groupId' => 'group-inc-1', + ], + ], + [ + 'target' => [ + '@odata.type' => '#microsoft.graph.exclusionGroupAssignmentTarget', + 'groupId' => 'group-exc-1', + ], + ], + [ + 'target' => [ + '@odata.type' => '#microsoft.graph.groupAssignmentTarget', + 'groupId' => 'group-inc-2', + ], + 'deviceAndAppManagementAssignmentFilterId' => 'filter-1', + 'deviceAndAppManagementAssignmentFilterType' => 'include', + ], + ], + ], 200); + } + + return new GraphResponse(true, [], 200); + } + + public function getPolicy(string $policyType, string $policyId, array $options = []): GraphResponse + { + return new GraphResponse(true, [], 200); + } + + public function getOrganization(array $options = []): GraphResponse + { + return new GraphResponse(true, [], 200); + } + + public function applyPolicy(string $policyType, string $policyId, array $payload, array $options = []): GraphResponse + { + return new GraphResponse(true, [], 200); + } + + public function getServicePrincipalPermissions(array $options = []): GraphResponse + { + return new GraphResponse(true, [], 200); + } + }; + }); + + $svc = app(InventorySyncService::class); + + $run = $svc->syncNow($tenant, [ + 'policy_types' => ['settingsCatalogPolicy'], + 'categories' => [], + 'include_foundations' => false, + 'include_dependencies' => true, + ]); + + expect($run->status)->toBe('success'); + + $edges = InventoryLink::query()->where('tenant_id', $tenant->getKey())->get(); + expect($edges->where('relationship_type', 'scoped_by'))->toHaveCount(1); + expect($edges->where('relationship_type', 'assigned_to_include'))->toHaveCount(2); + expect($edges->where('relationship_type', 'assigned_to_exclude'))->toHaveCount(1); + expect($edges->where('relationship_type', 'uses_assignment_filter'))->toHaveCount(1); + + $filterEdge = $edges->firstWhere('relationship_type', 'uses_assignment_filter'); + expect($filterEdge)->not->toBeNull(); + expect($filterEdge->metadata['foundation_type'] ?? null)->toBe('assignment_filter'); + expect($filterEdge->metadata['filter_mode'] ?? null)->toBe('include'); +});