From e010b5e24040438b49cf87596163264360d0dbfc Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Sun, 11 Jan 2026 22:14:25 +0100 Subject: [PATCH] feat: cached Entra group label resolver --- app/Filament/Resources/RestoreRunResource.php | 20 +-- app/Filament/Resources/TenantResource.php | 130 +++--------------- .../PolicyVersionAssignmentsWidget.php | 68 +++++++++ .../Directory/EntraGroupLabelResolver.php | 97 +++++++++++++ .../entries/restore-results.blade.php | 27 +++- ...olicy-version-assignments-widget.blade.php | 17 ++- .../051-entra-group-directory-cache/tasks.md | 12 +- .../EntraGroupLabelResolverTest.php | 50 +++++++ 8 files changed, 287 insertions(+), 134 deletions(-) create mode 100644 app/Services/Directory/EntraGroupLabelResolver.php create mode 100644 tests/Unit/DirectoryGroups/EntraGroupLabelResolverTest.php diff --git a/app/Filament/Resources/RestoreRunResource.php b/app/Filament/Resources/RestoreRunResource.php index 9dbc2d6..87e03b7 100644 --- a/app/Filament/Resources/RestoreRunResource.php +++ b/app/Filament/Resources/RestoreRunResource.php @@ -13,6 +13,7 @@ use App\Models\Tenant; use App\Rules\SkipOrUuidRule; use App\Services\BulkOperationService; +use App\Services\Directory\EntraGroupLabelResolver; use App\Services\Intune\AuditLogger; use App\Services\Intune\RestoreDiffGenerator; use App\Services\Intune\RestoreRiskChecker; @@ -120,7 +121,7 @@ public static function form(Schema $schema): Schema ->placeholder('SKIP or target group Object ID (GUID)') ->rules([new SkipOrUuidRule]) ->required() - ->helperText('Paste the target Entra ID group Object ID (GUID). Names are not resolved in this phase. Use SKIP to omit the assignment.'); + ->helperText('Paste the target Entra ID group Object ID (GUID). Labels use the cached directory groups only (no live Graph lookups). Use SKIP to omit the assignment.'); }, $unresolved); }) ->visible(function (Get $get): bool { @@ -1541,10 +1542,16 @@ private static function unresolvedGroups(?int $backupSetId, ?array $selectedItem return []; } - return array_map(function (string $groupId) use ($sourceNames): array { + $resolver = app(EntraGroupLabelResolver::class); + $cached = $resolver->lookupMany($tenant, $groupIds); + + return array_map(function (string $groupId) use ($sourceNames, $cached): array { + $cachedName = $cached[strtolower($groupId)] ?? null; + $fallbackName = $cachedName ?? ($sourceNames[$groupId] ?? null); + return [ 'id' => $groupId, - 'label' => static::formatGroupLabel($sourceNames[$groupId] ?? null, $groupId), + 'label' => EntraGroupLabelResolver::formatLabel($fallbackName, $groupId), ]; }, $groupIds); } @@ -1653,11 +1660,4 @@ private static function normalizeGroupMapping(mixed $mapping): array return array_filter($result, static fn (?string $value): bool => is_string($value) && $value !== ''); } - - private static function formatGroupLabel(?string $displayName, string $id): string - { - $suffix = '…'.mb_substr($id, -8); - - return trim(($displayName ?: 'Security group').' ('.$suffix.')'); - } } diff --git a/app/Filament/Resources/TenantResource.php b/app/Filament/Resources/TenantResource.php index f32bd90..f5efe27 100644 --- a/app/Filament/Resources/TenantResource.php +++ b/app/Filament/Resources/TenantResource.php @@ -6,9 +6,11 @@ use App\Http\Controllers\RbacDelegatedAuthController; use App\Jobs\BulkTenantSyncJob; use App\Jobs\SyncPoliciesJob; +use App\Models\EntraGroup; use App\Models\Tenant; use App\Models\User; use App\Services\BulkOperationService; +use App\Services\Directory\EntraGroupLabelResolver; use App\Services\Graph\GraphClientInterface; use App\Services\Intune\AuditLogger; use App\Services\Intune\RbacHealthService; @@ -585,10 +587,7 @@ public static function rbacAction(): Actions\Action ->placeholder('Search security groups') ->visible(fn (Get $get) => $get('scope') === 'scope_group') ->required(fn (Get $get) => $get('scope') === 'scope_group') - ->disabled(fn (?Tenant $record) => static::delegatedToken($record) === null) ->helperText(fn (?Tenant $record) => static::groupSearchHelper($record)) - ->hintAction(fn (?Tenant $record) => static::loginToSearchGroupsAction($record)) - ->hint(fn (?Tenant $record) => static::groupSearchHelper($record)) ->getSearchResultsUsing(fn (string $search, ?Tenant $record) => static::groupSearchOptions($record, $search)) ->getOptionLabelUsing(fn (?string $value, ?Tenant $record) => static::resolveGroupLabel($record, $value)) ->noSearchResultsMessage('No security groups found') @@ -615,10 +614,7 @@ public static function rbacAction(): Actions\Action ->placeholder('Search security groups') ->visible(fn (Get $get) => $get('group_mode') === 'existing') ->required(fn (Get $get) => $get('group_mode') === 'existing') - ->disabled(fn (?Tenant $record) => static::delegatedToken($record) === null) ->helperText(fn (?Tenant $record) => static::groupSearchHelper($record)) - ->hintAction(fn (?Tenant $record) => static::loginToSearchGroupsAction($record)) - ->hint(fn (?Tenant $record) => static::groupSearchHelper($record)) ->getSearchResultsUsing(fn (string $search, ?Tenant $record) => static::groupSearchOptions($record, $search)) ->getOptionLabelUsing(fn (?string $value, ?Tenant $record) => static::resolveGroupLabel($record, $value)) ->noSearchResultsMessage('No security groups found') @@ -928,6 +924,11 @@ private static function formatRoleLabel(?string $displayName, string $id): strin return trim(($displayName ?: 'RBAC role').$suffix); } + private static function escapeOdataValue(string $value): string + { + return str_replace("'", "''", $value); + } + private static function notifyRoleLookupFailure(): void { Notification::make() @@ -980,65 +981,30 @@ private static function loginToSearchGroupsAction(?Tenant $tenant): ?Actions\Act public static function groupSearchHelper(?Tenant $tenant): ?string { - return static::delegatedToken($tenant) ? null : 'Login to search groups'; + if (! $tenant) { + return null; + } + + return 'Uses cached directory groups only (no live Graph lookups). Run “Sync Groups” if results are empty.'; } /** * @return array */ public static function groupSearchOptions(?Tenant $tenant, string $search): array - { - return static::searchSecurityGroups($tenant, $search); - } - - /** - * @return array - */ - private static function searchSecurityGroups(?Tenant $tenant, string $search): array { if (! $tenant || mb_strlen($search) < 2) { return []; } - $token = static::delegatedToken($tenant); - - if (! $token) { - return []; - } - - try { - $response = app(GraphClientInterface::class)->request( - 'GET', - 'groups', - [ - 'query' => [ - '$filter' => sprintf( - "securityEnabled eq true and startswith(displayName,'%s')", - static::escapeOdataValue($search) - ), - '$select' => 'id,displayName', - '$top' => 20, - ], - ] + $tenant->graphOptions() + [ - 'access_token' => $token, - ] - ); - } catch (Throwable) { - static::notifyGroupLookupFailure(); - - return []; - } - - if ($response->failed()) { - static::notifyGroupLookupFailure(); - - return []; - } - - return collect($response->data['value'] ?? []) - ->filter(fn (array $group) => filled($group['id'] ?? null)) - ->mapWithKeys(fn (array $group) => [ - $group['id'] => static::formatGroupLabel($group['displayName'] ?? null, $group['id']), + return EntraGroup::query() + ->where('tenant_id', $tenant->getKey()) + ->where('display_name', 'ilike', '%'.str_replace('%', '\\%', $search).'%') + ->orderBy('display_name') + ->limit(20) + ->get(['entra_id', 'display_name']) + ->mapWithKeys(fn (EntraGroup $group) => [ + (string) $group->entra_id => EntraGroupLabelResolver::formatLabel($group->display_name, (string) $group->entra_id), ]) ->all(); } @@ -1049,61 +1015,7 @@ private static function resolveGroupLabel(?Tenant $tenant, ?string $groupId): ?s return $groupId; } - $token = static::delegatedToken($tenant); - - if (! $token) { - return $groupId; - } - - try { - $response = app(GraphClientInterface::class)->request( - 'GET', - "groups/{$groupId}", - [ - 'query' => [ - '$select' => 'id,displayName', - ], - ] + $tenant->graphOptions() + [ - 'access_token' => $token, - ] - ); - } catch (Throwable) { - static::notifyGroupLookupFailure(); - - return $groupId; - } - - if ($response->failed()) { - static::notifyGroupLookupFailure(); - - return $groupId; - } - - $displayName = $response->data['displayName'] ?? null; - $id = $response->data['id'] ?? $groupId; - - return static::formatGroupLabel($displayName, $id); - } - - private static function formatGroupLabel(?string $displayName, string $id): string - { - $suffix = sprintf(' (%s)', Str::limit($id, 8, '')); - - return trim(($displayName ?: 'Security group').$suffix); - } - - private static function escapeOdataValue(string $value): string - { - return str_replace("'", "''", $value); - } - - private static function notifyGroupLookupFailure(): void - { - Notification::make() - ->title('Group lookup failed') - ->body('Delegated session may have expired. Login again to search security groups.') - ->danger() - ->send(); + return app(EntraGroupLabelResolver::class)->resolveOne($tenant, $groupId); } public static function verifyTenant( diff --git a/app/Livewire/PolicyVersionAssignmentsWidget.php b/app/Livewire/PolicyVersionAssignmentsWidget.php index 4067708..12d66de 100644 --- a/app/Livewire/PolicyVersionAssignmentsWidget.php +++ b/app/Livewire/PolicyVersionAssignmentsWidget.php @@ -3,6 +3,8 @@ namespace App\Livewire; use App\Models\PolicyVersion; +use App\Models\Tenant; +use App\Services\Directory\EntraGroupLabelResolver; use Livewire\Component; class PolicyVersionAssignmentsWidget extends Component @@ -19,9 +21,75 @@ public function render(): \Illuminate\Contracts\View\View return view('livewire.policy-version-assignments-widget', [ 'version' => $this->version, 'compliance' => $this->complianceNotifications(), + 'groupLabels' => $this->groupLabels(), ]); } + /** + * @return array + */ + private function groupLabels(): array + { + $assignments = $this->version->assignments; + + if (! is_array($assignments) || $assignments === []) { + return []; + } + + $tenant = rescue(fn () => Tenant::current(), null); + + if (! $tenant instanceof Tenant) { + return []; + } + + $groupIds = []; + $sourceNames = []; + + foreach ($assignments as $assignment) { + if (! is_array($assignment)) { + continue; + } + + $target = $assignment['target'] ?? null; + + if (! is_array($target)) { + continue; + } + + $groupId = $target['groupId'] ?? null; + + if (! is_string($groupId) || $groupId === '') { + continue; + } + + $groupIds[] = $groupId; + + $displayName = $target['group_display_name'] ?? null; + + if (is_string($displayName) && $displayName !== '') { + $sourceNames[$groupId] = $displayName; + } + } + + $groupIds = array_values(array_unique($groupIds)); + + if ($groupIds === []) { + return []; + } + + $resolver = app(EntraGroupLabelResolver::class); + $cached = $resolver->lookupMany($tenant, $groupIds); + + $labels = []; + + foreach ($groupIds as $groupId) { + $cachedName = $cached[strtolower($groupId)] ?? null; + $labels[$groupId] = EntraGroupLabelResolver::formatLabel($cachedName ?? ($sourceNames[$groupId] ?? null), $groupId); + } + + return $labels; + } + /** * @return array{total:int,templates:array,items:array} */ diff --git a/app/Services/Directory/EntraGroupLabelResolver.php b/app/Services/Directory/EntraGroupLabelResolver.php new file mode 100644 index 0000000..0691983 --- /dev/null +++ b/app/Services/Directory/EntraGroupLabelResolver.php @@ -0,0 +1,97 @@ +resolveMany($tenant, [$groupId]); + + return $labels[$groupId] ?? self::formatLabel(null, $groupId); + } + + /** + * @param array $groupIds + * @return array + */ + public function resolveMany(Tenant $tenant, array $groupIds): array + { + $groupIds = array_values(array_unique(array_filter($groupIds, fn ($id) => is_string($id) && $id !== ''))); + + if ($groupIds === []) { + return []; + } + + $displayNames = $this->lookupMany($tenant, $groupIds); + + $labels = []; + + foreach ($groupIds as $groupId) { + $lookupId = Str::isUuid($groupId) ? strtolower($groupId) : $groupId; + $labels[$groupId] = self::formatLabel($displayNames[$lookupId] ?? null, $groupId); + } + + return $labels; + } + + /** + * @param array $groupIds + * @return array Map of groupId (lowercased UUID) => display_name + */ + public function lookupMany(Tenant $tenant, array $groupIds): array + { + $uuids = []; + + foreach ($groupIds as $groupId) { + if (! is_string($groupId) || $groupId === '') { + continue; + } + + if (! Str::isUuid($groupId)) { + continue; + } + + $uuids[] = strtolower($groupId); + } + + $uuids = array_values(array_unique($uuids)); + + if ($uuids === []) { + return []; + } + + return EntraGroup::query() + ->where('tenant_id', $tenant->getKey()) + ->whereIn('entra_id', $uuids) + ->pluck('display_name', 'entra_id') + ->mapWithKeys(fn (string $displayName, string $entraId) => [strtolower($entraId) => $displayName]) + ->all(); + } + + public static function formatLabel(?string $displayName, string $id): string + { + $name = filled($displayName) ? $displayName : 'Unresolved'; + + return sprintf('%s (%s)', trim($name), self::shortToken($id)); + } + + private static function shortToken(string $id): string + { + $normalized = preg_replace('/[^a-zA-Z0-9]/', '', $id); + + if (! is_string($normalized) || $normalized === '') { + return 'unknown'; + } + + if (mb_strlen($normalized) <= 8) { + return $normalized; + } + + return '…'.mb_substr($normalized, -8); + } +} diff --git a/resources/views/filament/infolists/entries/restore-results.blade.php b/resources/views/filament/infolists/entries/restore-results.blade.php index 96a6b81..8171bde 100644 --- a/resources/views/filament/infolists/entries/restore-results.blade.php +++ b/resources/views/filament/infolists/entries/restore-results.blade.php @@ -13,6 +13,26 @@ $foundationItems = collect($results)->filter($isFoundationEntry); $policyItems = collect($results)->reject($isFoundationEntry); } + + $tenant = rescue(fn () => \App\Models\Tenant::current(), null); + $groupLabelResolver = $tenant ? app(\App\Services\Directory\EntraGroupLabelResolver::class) : null; + + $formatGroupId = function ($groupId, $fallbackName = null) use ($tenant, $groupLabelResolver) { + if (! is_string($groupId) || $groupId === '') { + return null; + } + + $cachedName = null; + + if ($tenant && $groupLabelResolver) { + $cached = $groupLabelResolver->lookupMany($tenant, [$groupId]); + $cachedName = $cached[strtolower($groupId)] ?? null; + } + + $name = is_string($fallbackName) && $fallbackName !== '' ? $fallbackName : null; + + return \App\Services\Directory\EntraGroupLabelResolver::formatLabel($cachedName ?? $name, $groupId); + }; @endphp @if ($foundationItems->isEmpty() && $policyItems->isEmpty()) @@ -152,12 +172,15 @@ }; $assignmentGroupId = $outcome['group_id'] ?? ($outcome['assignment']['target']['groupId'] ?? null); + $assignmentGroupLabel = $formatGroupId(is_string($assignmentGroupId) ? $assignmentGroupId : null); + $mappedGroupId = $outcome['mapped_group_id'] ?? null; + $mappedGroupLabel = $formatGroupId(is_string($mappedGroupId) ? $mappedGroupId : null); @endphp
- Assignment {{ $assignmentGroupId ?? 'unknown group' }} + Assignment {{ $assignmentGroupLabel ?? ($assignmentGroupId ?? 'unknown group') }}
{{ $outcomeStatus }} @@ -166,7 +189,7 @@ @if (! empty($outcome['mapped_group_id']))
- Mapped to: {{ $outcome['mapped_group_id'] }} + Mapped to: {{ $mappedGroupLabel ?? $outcome['mapped_group_id'] }}
@endif diff --git a/resources/views/livewire/policy-version-assignments-widget.blade.php b/resources/views/livewire/policy-version-assignments-widget.blade.php index 5a593c4..65006bf 100644 --- a/resources/views/livewire/policy-version-assignments-widget.blade.php +++ b/resources/views/livewire/policy-version-assignments-widget.blade.php @@ -66,21 +66,24 @@ {{ $typeName }} @if($groupId) + @php + $groupLabel = $groupLabels[$groupId] ?? \App\Services\Directory\EntraGroupLabelResolver::formatLabel( + is_string($groupName) ? $groupName : null, + (string) $groupId, + ); + @endphp : @if($groupOrphaned) - ⚠️ Unknown group (ID: {{ $groupId }}) + ⚠️ {{ $groupLabel }} - @elseif($groupName) + @elseif($groupLabel) - {{ $groupName }} - - - ({{ $groupId }}) + {{ $groupLabel }} @else - Group ID: {{ $groupId }} + {{ $groupId }} @endif @endif diff --git a/specs/051-entra-group-directory-cache/tasks.md b/specs/051-entra-group-directory-cache/tasks.md index 915ef8e..51ba8cc 100644 --- a/specs/051-entra-group-directory-cache/tasks.md +++ b/specs/051-entra-group-directory-cache/tasks.md @@ -111,16 +111,16 @@ ## Phase 5: User Story 3 - Name resolution across the suite (Priority: P3) ### Tests for User Story 3 (REQUIRED) ⚠️ -- [ ] T026 [P] [US3] Add unit test for label resolver formatting + fallbacks in tests/Unit/DirectoryGroups/EntraGroupLabelResolverTest.php +- [x] T026 [P] [US3] Add unit test for label resolver formatting + fallbacks in tests/Unit/DirectoryGroups/EntraGroupLabelResolverTest.php - [x] T027 [P] [US3] Add feature test ensuring Tenant/Restore/PolicyVersion UI renders without Graph calls during render in tests/Feature/DirectoryGroups/NoLiveGraphOnRenderTest.php ### Implementation for User Story 3 -- [ ] T028 [US3] Implement DB-backed label resolver in app/Services/Directory/EntraGroupLabelResolver.php (resolveOne/resolveMany, tenant-scoped, stable formatting) -- [ ] T029 [US3] Refactor group label rendering in app/Filament/Resources/TenantResource.php to use EntraGroupLabelResolver instead of Graph lookups -- [ ] T030 [US3] Refactor Livewire assignments widget to use cached labels: app/Livewire/PolicyVersionAssignmentsWidget.php and resources/views/livewire/policy-version-assignments-widget.blade.php -- [ ] T031 [US3] Refactor restore results to show cached labels where possible: resources/views/filament/infolists/entries/restore-results.blade.php -- [ ] T032 [US3] Refactor restore group-mapping inputs/labels to prefer cached labels: app/Filament/Resources/RestoreRunResource.php +- [x] T028 [US3] Implement DB-backed label resolver in app/Services/Directory/EntraGroupLabelResolver.php (resolveOne/resolveMany, tenant-scoped, stable formatting) +- [x] T029 [US3] Refactor group label rendering in app/Filament/Resources/TenantResource.php to use EntraGroupLabelResolver instead of Graph lookups +- [x] T030 [US3] Refactor Livewire assignments widget to use cached labels: app/Livewire/PolicyVersionAssignmentsWidget.php and resources/views/livewire/policy-version-assignments-widget.blade.php +- [x] T031 [US3] Refactor restore results to show cached labels where possible: resources/views/filament/infolists/entries/restore-results.blade.php +- [x] T032 [US3] Refactor restore group-mapping inputs/labels to prefer cached labels: app/Filament/Resources/RestoreRunResource.php **Checkpoint**: US3 complete — name resolution is consistent and render-safe across key pages. diff --git a/tests/Unit/DirectoryGroups/EntraGroupLabelResolverTest.php b/tests/Unit/DirectoryGroups/EntraGroupLabelResolverTest.php new file mode 100644 index 0000000..189f9eb --- /dev/null +++ b/tests/Unit/DirectoryGroups/EntraGroupLabelResolverTest.php @@ -0,0 +1,50 @@ +toBe('Unresolved (…55555555)'); +}); + +it('resolves labels from the tenant cache (tenant-scoped)', function () { + $tenantA = Tenant::factory()->create(); + $tenantB = Tenant::factory()->create(); + + $entraId = '11111111-2222-3333-4444-555555555555'; + + EntraGroup::factory()->create([ + 'tenant_id' => $tenantA->getKey(), + 'entra_id' => $entraId, + 'display_name' => 'Alpha Team', + ]); + + EntraGroup::factory()->create([ + 'tenant_id' => $tenantB->getKey(), + 'entra_id' => $entraId, + 'display_name' => 'Beta Team', + ]); + + $resolver = app(EntraGroupLabelResolver::class); + + expect($resolver->resolveOne($tenantA, $entraId)) + ->toBe('Alpha Team (…55555555)') + ->and($resolver->resolveOne($tenantB, $entraId)) + ->toBe('Beta Team (…55555555)'); +}); + +it('returns a fallback without querying invalid UUIDs', function () { + $tenant = Tenant::factory()->create(); + + $resolver = app(EntraGroupLabelResolver::class); + + expect($resolver->resolveOne($tenant, 'group-123')) + ->toBe('Unresolved (group123)'); +});