From 1488d0ab2a7fe7dd462030c758da7dd844072907 Mon Sep 17 00:00:00 2001 From: f2cmb <2480194+f2cmb@users.noreply.github.com> Date: Fri, 6 Mar 2026 16:01:41 +0100 Subject: [PATCH 01/11] test sync / dryrun --- ajax/syncexecute.php | 66 ++++++++++ src/Inventory/LdapSyncExecutor.php | 88 ++++++++++++- src/SyncFilter.php | 43 +++++-- templates/sync_execution.html.twig | 195 +++++++++++++++++++++++++++++ 4 files changed, 382 insertions(+), 10 deletions(-) create mode 100644 ajax/syncexecute.php create mode 100644 templates/sync_execution.html.twig diff --git a/ajax/syncexecute.php b/ajax/syncexecute.php new file mode 100644 index 0000000..ecd7ee4 --- /dev/null +++ b/ajax/syncexecute.php @@ -0,0 +1,66 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2018-2023 by Teclib'. + * @license GPLv3+ https://www.gnu.org/licenses/gpl-3.0.html + * @link https://services.glpi-network.com + * ------------------------------------------------------------------------- + */ + +use Glpi\Exception\Http\BadRequestHttpException; +use GlpiPlugin\Advancedldap\Inventory\LdapSyncExecutor; +use GlpiPlugin\Advancedldap\SyncFilter; + +use function Safe\json_encode; + +header('Content-Type: application/json'); + +Session::checkLoginUser(); + +$action = $_POST['action'] ?? null; +$syncfilters_id = (int)($_POST['syncfilters_id'] ?? 0); + +$syncfilter = new SyncFilter(); +if ($syncfilters_id <= 0 || !$syncfilter->getFromDB($syncfilters_id)) { + throw new BadRequestHttpException('Invalid SyncFilter'); +} + +$required_right = ($action === 'execute') ? UPDATE : READ; +$syncfilter->check($syncfilters_id, $required_right); + +$executor = new LdapSyncExecutor(); + +switch ($action) { + case 'execute': + session_write_close(); + $results = $executor->executeSingleFilter($syncfilter); + echo json_encode(['success' => true, 'results' => $results]); + break; + case 'dry_run': + $preview = $executor->previewSyncFilter($syncfilter); + echo json_encode(['success' => true, 'preview' => $preview]); + break; + default: + throw new BadRequestHttpException('Unknown action'); +} diff --git a/src/Inventory/LdapSyncExecutor.php b/src/Inventory/LdapSyncExecutor.php index d830df3..bf9b3ac 100644 --- a/src/Inventory/LdapSyncExecutor.php +++ b/src/Inventory/LdapSyncExecutor.php @@ -99,6 +99,83 @@ public function executeForConnection(AuthLDAP $authldap): array return $this->results; } + /** + * Execute synchronization for a single SyncFilter using its linked AuthLDAP. + * + * @param SyncFilter $syncfilter The sync filter to execute + * + * @return array{created: int, updated: int, errors: int, skipped: int} Sync results + */ + public function executeSingleFilter(SyncFilter $syncfilter): array + { + $this->resetResults(); + + $authldap = $syncfilter->getLinkedAuthLdap(); + if ($authldap === null) { + return $this->results; + } + + $this->executeSyncFilter($authldap, $syncfilter); + + return $this->results; + } + + /** + * Preview synchronization for a single SyncFilter without injecting data. + * + * @param SyncFilter $syncfilter The sync filter to preview + * + * @return array{first_entry: array|null, would_create: int, would_update: int, total: int} + */ + public function previewSyncFilter(SyncFilter $syncfilter): array + { + $authldap = $syncfilter->getLinkedAuthLdap(); + $result = ['first_entry' => null, 'would_create' => 0, 'would_update' => 0, 'total' => 0]; + + if ($authldap === null) { + return $result; + } + + $builder = $this->loadBuilderMapping($syncfilter); + if (!$builder instanceof AbstractBuilderMapping) { + return $result; + } + + $sections = $builder->getAllSections(); + $ldap_attrs = $this->extractLdapAttributes($sections); + + $ldap_entries = $this->performLdapSearch($authldap, $syncfilter, $ldap_attrs); + + if (!is_array($ldap_entries) || $ldap_entries === []) { + return $result; + } + + foreach ($ldap_entries as $index => $ldap_entry) { + $inventory_data = $this->buildInventoryJson($sections, $ldap_entry, $syncfilter); + if ($inventory_data === null) { + continue; + } + + $result['total']++; + + if ($index === 0) { + $result['first_entry'] = $inventory_data; + } + + $deviceid = isset($inventory_data['deviceid']) && is_string($inventory_data['deviceid']) + ? $inventory_data['deviceid'] : ''; + + $agent = new \Agent(); + if ($deviceid !== '' && $agent->getFromDBByCrit(['deviceid' => $deviceid])) { + $result['would_update']++; + } else { + $result['would_create']++; + } + } + + return $result; + } + /** * Execute synchronization for a single SyncFilter. * @@ -572,11 +649,16 @@ private function injectInventory(array $inventory_data): void return; } + $agent = new \Agent(); + $agent_exists = $deviceid !== 'unknown' && $agent->getFromDBByCrit(['deviceid' => $deviceid]); + $inventory->doInventory(); - // TODO: Determine if item was created or updated based on Inventory results - // For now, assume created - $this->results['created']++; + if ($agent_exists) { + $this->results['updated']++; + } else { + $this->results['created']++; + } } /** diff --git a/src/SyncFilter.php b/src/SyncFilter.php index fac591b..1768ce9 100644 --- a/src/SyncFilter.php +++ b/src/SyncFilter.php @@ -218,26 +218,41 @@ public function getTabNameForItem(CommonGLPI $item, $withtemplate = 0) return ''; } - // Only show tab if a BuilderMapping is associated + $tabs = []; + $builder_itemtype = $item->fields['builder_itemtype'] ?? null; $builder_items_id = $item->fields['builder_items_id'] ?? 0; - if (empty($builder_itemtype) || $builder_items_id <= 0) { - return ''; + if (!empty($builder_itemtype) && $builder_items_id > 0) { + $tabs[1] = self::createTabEntry( + __('Builder Mapping', 'advancedldap'), + 0, + $item::class, + 'ti ti-code', + ); } - return self::createTabEntry( - __('Builder Mapping', 'advancedldap'), + $tabs[2] = self::createTabEntry( + __('Synchronize', 'advancedldap'), 0, $item::class, - 'ti ti-code', + 'ti ti-refresh', ); + + return $tabs; } public static function displayTabContentForItem(CommonGLPI $item, $tabnum = 1, $withtemplate = 0) { if ($item instanceof self) { - $item->showBuilderMappingTab(); + switch ($tabnum) { + case 1: + $item->showBuilderMappingTab(); + break; + case 2: + $item->showSyncTab(); + break; + } return true; } @@ -302,6 +317,20 @@ public function showBuilderMappingTab(): void ]); } + /** + * Display the Synchronize tab content. + */ + public function showSyncTab(): void + { + $authldap = $this->getLinkedAuthLdap(); + + TemplateRenderer::getInstance()->display('@advancedldap/sync_execution.html.twig', [ + 'syncfilters_id' => $this->getID(), + 'has_authldap' => $authldap !== null, + 'authldap_name' => $authldap?->getName(), + ]); + } + public function post_addItem() { parent::post_addItem(); diff --git a/templates/sync_execution.html.twig b/templates/sync_execution.html.twig new file mode 100644 index 0000000..af87a86 --- /dev/null +++ b/templates/sync_execution.html.twig @@ -0,0 +1,195 @@ +{# + # ------------------------------------------------------------------------- + # advancedldap plugin for GLPI + # ------------------------------------------------------------------------- + # + # LICENSE + # + # This file is part of advancedldap. + # + # AdvancedLDAP is free software; you can redistribute it and/or modify + # it under the terms of the GNU General Public License as published by + # the Free Software Foundation; either version 2 of the License, or + # (at your option) any later version. + # + # AdvancedLDAP is distributed in the hope that it will be useful, + # but WITHOUT ANY WARRANTY; without even the implied warranty of + # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + # GNU General Public License for more details. + # + # You should have received a copy of the GNU General Public License + # along with AdvancedLDAP. If not, see . + # ------------------------------------------------------------------------- + # @copyright Copyright (C) 2018-2023 by Teclib'. + # @license GPLv3+ https://www.gnu.org/licenses/gpl-3.0.html + # @link https://services.glpi-network.com + # ------------------------------------------------------------------------- + #} + +
+ {% if not has_authldap %} +
+ + {{ __('No LDAP connection linked to this filter. Associate an AuthLDAP connection first.', 'advancedldap') }} +
+ {% else %} +
+ + {{ __('LDAP connection:', 'advancedldap') }} {{ authldap_name }} +
+ {% endif %} + +
+ + +
+ +
+
+ {{ __('Loading...') }} +
+ {{ __('Processing...', 'advancedldap') }} +
+ + {# Sync results #} +
+

{{ __('Synchronization results', 'advancedldap') }}

+
+ + + 0 {{ __('created', 'advancedldap') }} + + + + 0 {{ __('updated', 'advancedldap') }} + + + + 0 {{ __('errors', 'advancedldap') }} + + + + 0 {{ __('skipped', 'advancedldap') }} + +
+
+ + {# Dry-run results #} +
+

{{ __('Preview results', 'advancedldap') }}

+
+ + + 0 {{ __('would be created', 'advancedldap') }} + + + + 0 {{ __('would be updated', 'advancedldap') }} + + + + 0 {{ __('total', 'advancedldap') }} + +
+
+

{{ __('First entry (JSON preview):', 'advancedldap') }}

+

+        
+
+ +
+
+ + From bfd3a4dece35b2ff38b171751315a06e678e8f60 Mon Sep 17 00:00:00 2001 From: f2cmb <2480194+f2cmb@users.noreply.github.com> Date: Thu, 11 Jun 2026 16:26:15 +0200 Subject: [PATCH 02/11] fix(sync): green the Handle sync gate (phpstan/rector) - ajax/syncexecute.php: import Safe\session_write_close and narrow POST id with is_numeric before int cast (avoids cast.int on mixed) - src/Inventory/LdapSyncExecutor.php, src/SyncFilter.php: rector FlipTypeControlToUseExclusiveTypeRector + NewlineAfterStatementRector - front/authldapsyncfilter.form.php: drop always-true null guard and ignore argument.type on add() (phpstan drift from GLPI core bump) --- ajax/syncexecute.php | 6 ++++-- front/authldapsyncfilter.form.php | 4 +--- src/Inventory/LdapSyncExecutor.php | 9 +++++---- src/SyncFilter.php | 15 ++++++--------- 4 files changed, 16 insertions(+), 18 deletions(-) diff --git a/ajax/syncexecute.php b/ajax/syncexecute.php index ecd7ee4..cd6715f 100644 --- a/ajax/syncexecute.php +++ b/ajax/syncexecute.php @@ -33,13 +33,15 @@ use GlpiPlugin\Advancedldap\SyncFilter; use function Safe\json_encode; +use function Safe\session_write_close; header('Content-Type: application/json'); Session::checkLoginUser(); -$action = $_POST['action'] ?? null; -$syncfilters_id = (int)($_POST['syncfilters_id'] ?? 0); +$action = $_POST['action'] ?? null; +$raw_syncfilters_id = $_POST['syncfilters_id'] ?? 0; +$syncfilters_id = is_numeric($raw_syncfilters_id) ? (int) $raw_syncfilters_id : 0; $syncfilter = new SyncFilter(); if ($syncfilters_id <= 0 || !$syncfilter->getFromDB($syncfilters_id)) { diff --git a/front/authldapsyncfilter.form.php b/front/authldapsyncfilter.form.php index 9510714..ed3550b 100644 --- a/front/authldapsyncfilter.form.php +++ b/front/authldapsyncfilter.form.php @@ -37,9 +37,7 @@ if (isset($_POST["add"])) { $input = $_POST; $relation->check(-1, CREATE, $input); // @phpstan-ignore argument.type ($_POST keys are always strings) - if ($input !== null) { - $relation->add($input); - } + $relation->add($input); // @phpstan-ignore argument.type ($_POST keys are always strings) } Html::back(); diff --git a/src/Inventory/LdapSyncExecutor.php b/src/Inventory/LdapSyncExecutor.php index bf9b3ac..514d3be 100644 --- a/src/Inventory/LdapSyncExecutor.php +++ b/src/Inventory/LdapSyncExecutor.php @@ -33,6 +33,7 @@ namespace GlpiPlugin\Advancedldap\Inventory; +use Agent; use Throwable; use LDAP\Result; use AuthLDAP; @@ -111,7 +112,7 @@ public function executeSingleFilter(SyncFilter $syncfilter): array $this->resetResults(); $authldap = $syncfilter->getLinkedAuthLdap(); - if ($authldap === null) { + if (!$authldap instanceof AuthLDAP) { return $this->results; } @@ -132,7 +133,7 @@ public function previewSyncFilter(SyncFilter $syncfilter): array $authldap = $syncfilter->getLinkedAuthLdap(); $result = ['first_entry' => null, 'would_create' => 0, 'would_update' => 0, 'total' => 0]; - if ($authldap === null) { + if (!$authldap instanceof AuthLDAP) { return $result; } @@ -165,7 +166,7 @@ public function previewSyncFilter(SyncFilter $syncfilter): array $deviceid = isset($inventory_data['deviceid']) && is_string($inventory_data['deviceid']) ? $inventory_data['deviceid'] : ''; - $agent = new \Agent(); + $agent = new Agent(); if ($deviceid !== '' && $agent->getFromDBByCrit(['deviceid' => $deviceid])) { $result['would_update']++; } else { @@ -649,7 +650,7 @@ private function injectInventory(array $inventory_data): void return; } - $agent = new \Agent(); + $agent = new Agent(); $agent_exists = $deviceid !== 'unknown' && $agent->getFromDBByCrit(['deviceid' => $deviceid]); $inventory->doInventory(); diff --git a/src/SyncFilter.php b/src/SyncFilter.php index 1768ce9..2c0f2e5 100644 --- a/src/SyncFilter.php +++ b/src/SyncFilter.php @@ -245,14 +245,11 @@ public function getTabNameForItem(CommonGLPI $item, $withtemplate = 0) public static function displayTabContentForItem(CommonGLPI $item, $tabnum = 1, $withtemplate = 0) { if ($item instanceof self) { - switch ($tabnum) { - case 1: - $item->showBuilderMappingTab(); - break; - case 2: - $item->showSyncTab(); - break; - } + match ($tabnum) { + 1 => $item->showBuilderMappingTab(), + 2 => $item->showSyncTab(), + default => true, + }; return true; } @@ -326,7 +323,7 @@ public function showSyncTab(): void TemplateRenderer::getInstance()->display('@advancedldap/sync_execution.html.twig', [ 'syncfilters_id' => $this->getID(), - 'has_authldap' => $authldap !== null, + 'has_authldap' => $authldap instanceof AuthLDAP, 'authldap_name' => $authldap?->getName(), ]); } From 03c90f75931f5bb1b90573fdca50cc344de9d570 Mon Sep 17 00:00:00 2001 From: f2cmb <2480194+f2cmb@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:14:08 +0200 Subject: [PATCH 03/11] fix(sync): address critical & major review findings on Handle sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Critical: - SyncFilter: use AuthLDAP field 'deref_option' (not 'deref', which never exists and silently fell back to 0); robust is_numeric cast in both spots. Major: - LdapSyncExecutor: stop silently returning empty results — log + skipped++ when a SyncFilter has no linked AuthLDAP (executeSingleFilter) and log the null-AuthLDAP/null-builder preview paths. Prevents a false 'disappeared' signal for future lifecycle handling. - LdapSyncExecutor::replacePlaceholders: substitute LDAP values inside the PHP structure (string leaves only) instead of into a re-serialized JSON string with a hand-rolled addcslashes escaper. Closes JSON key-injection and control-character corruption from attacker-controlled LDAP values. - AuthLdapSyncFilter::prepareInputForAdd: whitelist the two relation foreign keys (array_intersect_key) to prevent mass assignment. - SyncFilter::showBuilderMappingTab: render alerts via a Twig partial using core alerts_macros and pass Monaco CSS as a template var instead of echoing raw HTML from a class method. - LdapSyncExecutor: trim verbose narrating class PHPDoc. Tests: - previewSyncFilter without linked AuthLDAP (result + log); - replacePlaceholders cannot inject inventory keys / preserves control chars; - prepareInputForAdd strips unknown keys. Note: Hooks::CSRF_COMPLIANT intentionally NOT added — deprecated since GLPI 11.0 (CSRF enforced by CheckCsrfListener); declaring it would fail phpstan. --- setup.php | 3 + src/AuthLdapSyncFilter.php | 4 ++ src/Inventory/LdapSyncExecutor.php | 77 +++++++++++++++-------- src/SyncFilter.php | 27 ++++---- templates/builder_mapping.html.twig | 4 ++ templates/builder_mapping_alert.html.twig | 35 +++++++++++ tests/AuthLdapSyncFilterTest.php | 37 +++++++++++ tests/LdapSyncExecutorTest.php | 40 ++++++++++++ 8 files changed, 189 insertions(+), 38 deletions(-) create mode 100644 templates/builder_mapping_alert.html.twig diff --git a/setup.php b/setup.php index f40ec75..9a1b287 100644 --- a/setup.php +++ b/setup.php @@ -48,6 +48,9 @@ function plugin_init_advancedldap(): void /** @var array>> $PLUGIN_HOOKS */ global $PLUGIN_HOOKS; + // Note: Hooks::CSRF_COMPLIANT is deprecated since GLPI 11.0 — CSRF is enforced + // automatically by the CheckCsrfListener middleware, so it is intentionally not declared. + $PLUGIN_HOOKS[Hooks::ITEM_PURGE]['advancedldap'] = [ AuthLDAP::class => 'plugin_advancedldap_item_purge', SyncFilter::class => 'plugin_advancedldap_item_purge', diff --git a/src/AuthLdapSyncFilter.php b/src/AuthLdapSyncFilter.php index f75a0ab..1618e52 100644 --- a/src/AuthLdapSyncFilter.php +++ b/src/AuthLdapSyncFilter.php @@ -371,6 +371,7 @@ public function prepareInputForAdd($input) return false; } + $authldap_fk = getForeignKeyFieldForItemType(AuthLDAP::class); $syncfilter_fk = getForeignKeyFieldForItemType(SyncFilter::class); $syncfilter_value = $input[$syncfilter_fk] ?? 0; $syncfilters_id = is_numeric($syncfilter_value) ? (int) $syncfilter_value : 0; @@ -380,6 +381,9 @@ public function prepareInputForAdd($input) $this->deleteExistingLinkForSyncFilter($syncfilters_id); } + // N'autoriser que les deux clés étrangères de la relation (anti mass-assignment) + $input = array_intersect_key($input, array_flip([$authldap_fk, $syncfilter_fk])); + return parent::prepareInputForAdd($input); } diff --git a/src/Inventory/LdapSyncExecutor.php b/src/Inventory/LdapSyncExecutor.php index 514d3be..7a78797 100644 --- a/src/Inventory/LdapSyncExecutor.php +++ b/src/Inventory/LdapSyncExecutor.php @@ -51,14 +51,7 @@ use function Safe\preg_replace_callback; /** - * Orchestrator for LDAP to GLPI inventory synchronization. - * - * Responsibilities: - * 1. Retrieve active SyncFilters for a given LDAP connection - * 2. Perform LDAP searches using filter criteria - * 3. Instantiate the appropriate InventoryBuilder based on itemtype - * 4. Inject built inventory JSON into Glpi\Inventory\Inventory - * 5. Log results + * Orchestrates LDAP-to-GLPI inventory synchronization for a SyncFilter. */ class LdapSyncExecutor { @@ -113,6 +106,11 @@ public function executeSingleFilter(SyncFilter $syncfilter): array $authldap = $syncfilter->getLinkedAuthLdap(); if (!$authldap instanceof AuthLDAP) { + Toolbox::logDebug(sprintf( + 'AdvancedLDAP: SyncFilter %d has no linked AuthLDAP, nothing to synchronize', + $syncfilter->getID(), + )); + $this->results['skipped']++; return $this->results; } @@ -134,11 +132,19 @@ public function previewSyncFilter(SyncFilter $syncfilter): array $result = ['first_entry' => null, 'would_create' => 0, 'would_update' => 0, 'total' => 0]; if (!$authldap instanceof AuthLDAP) { + Toolbox::logDebug(sprintf( + 'AdvancedLDAP: SyncFilter %d has no linked AuthLDAP, cannot preview', + $syncfilter->getID(), + )); return $result; } $builder = $this->loadBuilderMapping($syncfilter); if (!$builder instanceof AbstractBuilderMapping) { + Toolbox::logDebug(sprintf( + 'AdvancedLDAP: SyncFilter %d has no BuilderMapping, cannot preview', + $syncfilter->getID(), + )); return $result; } @@ -403,27 +409,48 @@ protected function removeEmptyKeys(array $data): array */ protected function replacePlaceholders(array $data, array $ldap_entry): array { - $json_string = json_encode($data, JSON_UNESCAPED_UNICODE); - - $json_string = preg_replace_callback( - self::PLACEHOLDER_PATTERN, - function ($matches) use ($ldap_entry) { - $attr_raw = $matches[1] ?? ''; - $attr_name = strtolower(is_string($attr_raw) ? $attr_raw : ''); - $value = $this->getLdapValue($ldap_entry, $attr_name); - // Escape JSON special characters to prevent invalid JSON - return addcslashes($value, "\"\\/\n\r\t"); - }, - $json_string, - ); - - $decoded = json_decode($json_string, true); - + // Substitute directly within the PHP structure (string leaves only). LDAP values + // never touch the JSON-string layer raw, so they cannot break out of their context + // or inject arbitrary keys into the inventory payload. /** @var array $result */ - $result = is_array($decoded) ? $decoded : []; + $result = $this->substitutePlaceholders($data, $ldap_entry); return $result; } + /** + * Recursively replace {{ ldap.xxx }} placeholders inside string leaves of a structure. + * + * @param mixed $value The value to process (array, string or scalar) + * @param array $ldap_entry The LDAP entry data + * + * @return mixed The value with placeholders substituted + */ + private function substitutePlaceholders(mixed $value, array $ldap_entry): mixed + { + if (is_array($value)) { + $result = []; + foreach ($value as $key => $item) { + $result[$key] = $this->substitutePlaceholders($item, $ldap_entry); + } + + return $result; + } + + if (is_string($value)) { + return preg_replace_callback( + self::PLACEHOLDER_PATTERN, + function ($matches) use ($ldap_entry) { + $attr_raw = $matches[1] ?? ''; + $attr_name = strtolower(is_string($attr_raw) ? $attr_raw : ''); + return $this->getLdapValue($ldap_entry, $attr_name); + }, + $value, + ); + } + + return $value; + } + /** * Get a value from LDAP entry, handling the LDAP array structure. * diff --git a/src/SyncFilter.php b/src/SyncFilter.php index 2c0f2e5..4a3eaf0 100644 --- a/src/SyncFilter.php +++ b/src/SyncFilter.php @@ -272,24 +272,23 @@ public function showBuilderMappingTab(): void || !class_exists($builder_itemtype) || !is_subclass_of($builder_itemtype, AbstractBuilderMapping::class) ) { - echo '
'; - echo __s('No Builder Mapping associated with this SyncFilter.', 'advancedldap'); - echo '
'; + TemplateRenderer::getInstance()->display('@advancedldap/builder_mapping_alert.html.twig', [ + 'type' => 'warning', + 'message' => __('No Builder Mapping associated with this SyncFilter.', 'advancedldap'), + ]); return; } /** @var class-string $builder_itemtype */ $builder = new $builder_itemtype(); if (!$builder->getFromDB((int) $builder_items_id)) { - echo '
'; - echo __s('Failed to load Builder Mapping.', 'advancedldap'); - echo '
'; + TemplateRenderer::getInstance()->display('@advancedldap/builder_mapping_alert.html.twig', [ + 'type' => 'danger', + 'message' => __('Failed to load Builder Mapping.', 'advancedldap'), + ]); return; } - // Load Monaco CSS (required for AJAX-loaded content) - echo Html::css("lib/monaco.css"); - // Prepare sections data for template $sections = []; $section_names = $builder_itemtype::getSectionNames(); @@ -311,6 +310,8 @@ public function showBuilderMappingTab(): void 'sections' => $sections, 'completions' => $this->getLdapCompletions($this->getID()), 'authldap_status' => $this->getAuthLdapStatus(), + // Monaco CSS, required for AJAX-loaded content (link tag built by Html::css) + 'monaco_css' => Html::css('lib/monaco.css'), ]); } @@ -515,7 +516,7 @@ private function fetchAttributesFromLdap(AuthLDAP $authldap, SyncFilter $syncfil $rootdn = $authldap->fields['rootdn'] ?? ''; $rootdn_passwd = $authldap->fields['rootdn_passwd'] ?? ''; $use_tls = $authldap->fields['use_tls'] ?? false; - $deref = $authldap->fields['deref'] ?? 0; + $deref = $authldap->fields['deref_option'] ?? 0; $decrypted_passwd = (new GLPIKey())->decrypt(is_string($rootdn_passwd) ? $rootdn_passwd : ''); @@ -526,7 +527,7 @@ private function fetchAttributesFromLdap(AuthLDAP $authldap, SyncFilter $syncfil is_string($rootdn) ? $rootdn : '', is_string($decrypted_passwd) ? $decrypted_passwd : '', (bool) $use_tls, - is_int($deref) ? $deref : 0, + is_numeric($deref) ? (int) $deref : 0, ); if ($ds === false) { @@ -676,7 +677,7 @@ public function getAuthLdapStatus(): array $rootdn = $authldap->fields['rootdn'] ?? ''; $rootdn_passwd = $authldap->fields['rootdn_passwd'] ?? ''; $use_tls = $authldap->fields['use_tls'] ?? false; - $deref = $authldap->fields['deref'] ?? 0; + $deref = $authldap->fields['deref_option'] ?? 0; $decrypted_passwd = (new GLPIKey())->decrypt(is_string($rootdn_passwd) ? $rootdn_passwd : ''); @@ -687,7 +688,7 @@ public function getAuthLdapStatus(): array is_string($rootdn) ? $rootdn : '', is_string($decrypted_passwd) ? $decrypted_passwd : '', (bool) $use_tls, - is_int($deref) ? $deref : 0, + is_numeric($deref) ? (int) $deref : 0, ); if ($ds === false) { diff --git a/templates/builder_mapping.html.twig b/templates/builder_mapping.html.twig index c16522e..0ff27b9 100644 --- a/templates/builder_mapping.html.twig +++ b/templates/builder_mapping.html.twig @@ -26,6 +26,10 @@ # ------------------------------------------------------------------------- #} +{% if monaco_css is defined %} + {{ monaco_css|raw }} +{% endif %} +

diff --git a/templates/builder_mapping_alert.html.twig b/templates/builder_mapping_alert.html.twig new file mode 100644 index 0000000..e585ff4 --- /dev/null +++ b/templates/builder_mapping_alert.html.twig @@ -0,0 +1,35 @@ +{# + # ------------------------------------------------------------------------- + # advancedldap plugin for GLPI + # ------------------------------------------------------------------------- + # + # LICENSE + # + # This file is part of advancedldap. + # + # AdvancedLDAP is free software; you can redistribute it and/or modify + # it under the terms of the GNU General Public License as published by + # the Free Software Foundation; either version 2 of the License, or + # (at your option) any later version. + # + # AdvancedLDAP is distributed in the hope that it will be useful, + # but WITHOUT ANY WARRANTY; without even the implied warranty of + # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + # GNU General Public License for more details. + # + # You should have received a copy of the GNU General Public License + # along with AdvancedLDAP. If not, see . + # ------------------------------------------------------------------------- + # @copyright Copyright (C) 2018-2023 by Teclib'. + # @license GPLv3+ https://www.gnu.org/licenses/gpl-3.0.html + # @link https://services.glpi-network.com + # ------------------------------------------------------------------------- + #} + +{% import 'components/alerts_macros.html.twig' as alerts %} + +{% if type == 'danger' %} + {{ alerts.alert_danger(message) }} +{% else %} + {{ alerts.alert_warning(message) }} +{% endif %} diff --git a/tests/AuthLdapSyncFilterTest.php b/tests/AuthLdapSyncFilterTest.php index 6ee5b6a..2654d25 100644 --- a/tests/AuthLdapSyncFilterTest.php +++ b/tests/AuthLdapSyncFilterTest.php @@ -41,6 +41,9 @@ * @method void assertEquals($expected, $actual, string $message = '') * @method void assertNotFalse($condition, string $message = '') * @method void assertGreaterThan($expected, $actual, string $message = '') + * @method void assertIsArray($actual, string $message = '') + * @method void assertArrayHasKey($key, $array, string $message = '') + * @method void assertArrayNotHasKey($key, $array, string $message = '') */ final class AuthLdapSyncFilterTest extends DbTestCase { @@ -196,4 +199,38 @@ public function testPurgeSyncFilterCleansRelations(): void [$syncfilter_fk => $syncfilter_id], )); } + + public function testPrepareInputForAddStripsUnknownKeys(): void + { + $authldap = $this->createItem(AuthLDAP::class, [ + 'name' => 'Test LDAP Server', + 'host' => 'ldap.example.com', + 'basedn' => 'dc=example,dc=com', + 'is_active' => 1, + ]); + + $syncfilter = $this->createItem(SyncFilter::class, [ + 'name' => 'Test Sync Filter', + 'connection_filter' => '(objectClass=computer)', + 'basedn' => 'ou=computers,dc=example,dc=com', + 'itemtype' => 'Computer', + ]); + + $authldap_fk = getForeignKeyFieldForItemType(AuthLDAP::class); + $syncfilter_fk = getForeignKeyFieldForItemType(SyncFilter::class); + + $relation = new AuthLdapSyncFilter(); + $prepared = $relation->prepareInputForAdd([ + $authldap_fk => $authldap->getID(), + $syncfilter_fk => $syncfilter->getID(), + 'id' => 999, + 'evil_field' => 'injected', + ]); + + $this->assertIsArray($prepared); + $this->assertArrayHasKey($authldap_fk, $prepared); + $this->assertArrayHasKey($syncfilter_fk, $prepared); + $this->assertArrayNotHasKey('evil_field', $prepared); + $this->assertArrayNotHasKey('id', $prepared); + } } diff --git a/tests/LdapSyncExecutorTest.php b/tests/LdapSyncExecutorTest.php index fb95ece..ac1c3a2 100644 --- a/tests/LdapSyncExecutorTest.php +++ b/tests/LdapSyncExecutorTest.php @@ -223,6 +223,28 @@ public function testReplacePlaceholdersEscapesJsonSpecialCharacters(): void $this->assertEquals('PC "test"', $result['name']); } + public function testReplacePlaceholdersCannotInjectInventoryKeys(): void + { + // A hostile LDAP value full of JSON metacharacters must remain a plain string + // value: it cannot break out of its context nor inject sibling keys. + $data = ['name' => '{{ ldap.cn }}']; + $entry = ['cn' => ['count' => 1, 0 => '", "injected": "evil']]; + $result = $this->executor->callReplacePlaceholders($data, $entry); + $this->assertEquals('", "injected": "evil', $result['name']); + $this->assertArrayNotHasKey('injected', $result); + $this->assertCount(1, $result); + } + + public function testReplacePlaceholdersPreservesControlCharactersInValue(): void + { + // Control characters (e.g. NUL, vertical tab) must be preserved verbatim, where a + // hand-rolled JSON escaper would have corrupted the payload. + $data = ['name' => '{{ ldap.cn }}']; + $entry = ['cn' => ['count' => 1, 0 => "line1\x00\x0bline2"]]; + $result = $this->executor->callReplacePlaceholders($data, $entry); + $this->assertEquals("line1\x00\x0bline2", $result['name']); + } + // --- extractLdapAttributes --- public function testExtractLdapAttributesFindsPlaceholders(): void @@ -357,6 +379,24 @@ public function testBuildInventoryJsonStripsEmptyValuesFromPayload(): void $this->assertArrayNotHasKey('tag', $result); } + // --- previewSyncFilter --- + + public function testPreviewSyncFilterWithoutLinkedAuthLdapReturnsZeroAndLogs(): void + { + $syncfilter = $this->createSyncFilter(); + + $result = $this->executor->previewSyncFilter($syncfilter); + + $this->hasPhpLogRecordThatContains( + 'AdvancedLDAP: SyncFilter ' . $syncfilter->getID() . ' has no linked AuthLDAP, cannot preview', + 'Debug', + ); + $this->assertNull($result['first_entry']); + $this->assertEquals(0, $result['would_create']); + $this->assertEquals(0, $result['would_update']); + $this->assertEquals(0, $result['total']); + } + // --- helpers --- private function createSyncFilter(): SyncFilter From ae7a2e96ebb9ed663413a93f73a84cf1048b4931 Mon Sep 17 00:00:00 2001 From: f2cmb <2480194+f2cmb@users.noreply.github.com> Date: Thu, 11 Jun 2026 18:00:46 +0200 Subject: [PATCH 04/11] docs(changelog): add Handle sync entry --- CHANGELOG.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb8a6d8..22bfe04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,4 +5,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). -## [unreleased] - \ No newline at end of file +## [unreleased] + +### Added + +- Manual LDAP to GLPI inventory synchronization from a sync filter, with a dry-run preview and an execute mode (Computer itemtype) + +### Fixed + +- Read the correct `deref_option` field from the LDAP directory configuration +- Prevent JSON injection from LDAP attribute values when building the inventory payload +- Prevent mass assignment when creating an AuthLDAP / SyncFilter relation \ No newline at end of file From fd76ad1081da1ad0d9667c8ba9e5adb2d4d85d9a Mon Sep 17 00:00:00 2001 From: f2cmb <2480194+f2cmb@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:32:06 +0200 Subject: [PATCH 05/11] refactor(executor): extract connectToLdap() from performLdapSearch() --- src/Inventory/LdapSyncExecutor.php | 63 ++++++++++++++++++------------ 1 file changed, 37 insertions(+), 26 deletions(-) diff --git a/src/Inventory/LdapSyncExecutor.php b/src/Inventory/LdapSyncExecutor.php index 7a78797..6486ea4 100644 --- a/src/Inventory/LdapSyncExecutor.php +++ b/src/Inventory/LdapSyncExecutor.php @@ -531,35 +531,14 @@ protected function generateDeviceId(SyncFilter $syncfilter, array $ldap_entry): } /** - * Perform LDAP search using filter criteria. + * Open a connection to the LDAP server using AuthLDAP credentials. * - * @param AuthLDAP $authldap The LDAP connection - * @param SyncFilter $syncfilter The sync filter with search criteria - * @param array $ldap_attrs LDAP attributes to fetch + * @param AuthLDAP $authldap The LDAP connection configuration * - * @return array>|false Array of LDAP entries or false on error + * @return \LDAP\Connection|false The LDAP link or false on failure */ - private function performLdapSearch(AuthLDAP $authldap, SyncFilter $syncfilter, array $ldap_attrs): array|false + protected function connectToLdap(AuthLDAP $authldap): \LDAP\Connection|false { - $connection_filter = $syncfilter->fields['connection_filter'] ?? ''; - $basedn = $syncfilter->fields['basedn'] ?? ''; - - if (!is_string($connection_filter) || ($connection_filter === '' || $connection_filter === '0') || !is_string($basedn) || ($basedn === '' || $basedn === '0')) { - Toolbox::logDebug(sprintf( - 'AdvancedLDAP: Missing filter or basedn for SyncFilter %d', - $syncfilter->getID(), - )); - return false; - } - - Toolbox::logDebug(sprintf( - 'AdvancedLDAP: Searching LDAP - Filter: "%s", BaseDN: "%s", Attrs: [%s]', - $connection_filter, - $basedn, - implode(', ', $ldap_attrs), - )); - - // Connect to LDAP using AuthLDAP credentials $host = is_string($authldap->fields['host'] ?? null) ? $authldap->fields['host'] : ''; $port = is_string($authldap->fields['port'] ?? null) ? $authldap->fields['port'] : '389'; $rootdn = is_string($authldap->fields['rootdn'] ?? null) ? $authldap->fields['rootdn'] : ''; @@ -574,7 +553,7 @@ private function performLdapSearch(AuthLDAP $authldap, SyncFilter $syncfilter, a $timeout = is_numeric($timeout_raw) ? (int) $timeout_raw : 10; $tls_version = is_string($authldap->fields['tls_version'] ?? null) ? $authldap->fields['tls_version'] : ''; - $ds = AuthLDAP::connectToServer( + return AuthLDAP::connectToServer( $host, $port, $rootdn, @@ -587,6 +566,38 @@ private function performLdapSearch(AuthLDAP $authldap, SyncFilter $syncfilter, a $timeout, $tls_version, ); + } + + /** + * Perform LDAP search using filter criteria. + * + * @param AuthLDAP $authldap The LDAP connection + * @param SyncFilter $syncfilter The sync filter with search criteria + * @param array $ldap_attrs LDAP attributes to fetch + * + * @return array>|false Array of LDAP entries or false on error + */ + private function performLdapSearch(AuthLDAP $authldap, SyncFilter $syncfilter, array $ldap_attrs): array|false + { + $connection_filter = $syncfilter->fields['connection_filter'] ?? ''; + $basedn = $syncfilter->fields['basedn'] ?? ''; + + if (!is_string($connection_filter) || ($connection_filter === '' || $connection_filter === '0') || !is_string($basedn) || ($basedn === '' || $basedn === '0')) { + Toolbox::logDebug(sprintf( + 'AdvancedLDAP: Missing filter or basedn for SyncFilter %d', + $syncfilter->getID(), + )); + return false; + } + + Toolbox::logDebug(sprintf( + 'AdvancedLDAP: Searching LDAP - Filter: "%s", BaseDN: "%s", Attrs: [%s]', + $connection_filter, + $basedn, + implode(', ', $ldap_attrs), + )); + + $ds = $this->connectToLdap($authldap); if ($ds === false) { Toolbox::logDebug(sprintf( From 752b073c55a6809988e90b728d911d961b9937c7 Mon Sep 17 00:00:00 2001 From: f2cmb <2480194+f2cmb@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:33:23 +0200 Subject: [PATCH 06/11] feat(executor): add pure page-collection loop with completeness flag --- src/Inventory/LdapSyncExecutor.php | 47 +++++++++++++++++++++ tests/LdapSyncExecutorTest.php | 68 ++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+) diff --git a/src/Inventory/LdapSyncExecutor.php b/src/Inventory/LdapSyncExecutor.php index 6486ea4..fb4d9e9 100644 --- a/src/Inventory/LdapSyncExecutor.php +++ b/src/Inventory/LdapSyncExecutor.php @@ -73,6 +73,53 @@ class LdapSyncExecutor 'skipped' => 0, ]; + /** + * Whether the last LDAP search retrieved the full result set. + * Set to false when the search failed, was truncated (size limit exceeded) + * or the connection could not be established. + */ + protected bool $last_search_complete = true; + + /** + * Whether the last LDAP search retrieved the full result set. + */ + public function wasLastSearchComplete(): bool + { + return $this->last_search_complete; + } + + /** + * Iterate over LDAP result pages until the server returns an empty cookie. + * + * The page fetcher receives the pagination cookie ('' for the first page) and + * must return ['entries' => ..., 'next_cookie' => ...] or false on error. + * On fetcher failure, the whole collection fails and the search is flagged + * as incomplete: partial results must never be mistaken for full ones. + * + * @param callable(string): (array{entries: array>, next_cookie: string}|false) $page_fetcher + * + * @return array>|false All entries, or false on error + */ + protected function collectAllPages(callable $page_fetcher): array|false + { + $entries = []; + $cookie = ''; + + do { + $page = $page_fetcher($cookie); + + if ($page === false) { + $this->last_search_complete = false; + return false; + } + + $entries = array_merge($entries, $page['entries']); + $cookie = $page['next_cookie']; + } while ($cookie !== ''); + + return $entries; + } + /** * Execute synchronization for all active SyncFilters linked to an LDAP connection. * diff --git a/tests/LdapSyncExecutorTest.php b/tests/LdapSyncExecutorTest.php index ac1c3a2..d878642 100644 --- a/tests/LdapSyncExecutorTest.php +++ b/tests/LdapSyncExecutorTest.php @@ -97,6 +97,20 @@ public function callGetLdapValue(array $ldap_entry, string $attr_name): string { return $this->getLdapValue($ldap_entry, $attr_name); } + + /** + * @param callable(string): (array{entries: array>, next_cookie: string}|false) $page_fetcher + * @return array>|false + */ + public function callCollectAllPages(callable $page_fetcher): array|false + { + return $this->collectAllPages($page_fetcher); + } + + public function callGetPageSize(\AuthLDAP $authldap): int + { + return $this->getPageSize($authldap); + } } /** @@ -397,6 +411,60 @@ public function testPreviewSyncFilterWithoutLinkedAuthLdapReturnsZeroAndLogs(): $this->assertEquals(0, $result['total']); } + // --- collectAllPages --- + + public function testCollectAllPagesSinglePage(): void + { + $result = $this->executor->callCollectAllPages(function (string $cookie) { + $this->assertEquals('', $cookie); + return ['entries' => [['dn' => 'cn=a']], 'next_cookie' => '']; + }); + $this->assertEquals([['dn' => 'cn=a']], $result); + } + + public function testCollectAllPagesConcatenatesPagesInOrder(): void + { + $pages = [ + '' => ['entries' => [['dn' => 'cn=a'], ['dn' => 'cn=b']], 'next_cookie' => 'C1'], + 'C1' => ['entries' => [['dn' => 'cn=c']], 'next_cookie' => 'C2'], + 'C2' => ['entries' => [['dn' => 'cn=d']], 'next_cookie' => ''], + ]; + $result = $this->executor->callCollectAllPages(fn (string $cookie) => $pages[$cookie]); + $this->assertEquals( + [['dn' => 'cn=a'], ['dn' => 'cn=b'], ['dn' => 'cn=c'], ['dn' => 'cn=d']], + $result, + ); + } + + public function testCollectAllPagesEmptyResult(): void + { + $result = $this->executor->callCollectAllPages( + fn (string $cookie) => ['entries' => [], 'next_cookie' => ''], + ); + $this->assertEquals([], $result); + } + + public function testCollectAllPagesReturnsFalseOnMidPaginationFailure(): void + { + $pages = [ + '' => ['entries' => [['dn' => 'cn=a']], 'next_cookie' => 'C1'], + 'C1' => false, + ]; + $result = $this->executor->callCollectAllPages(fn (string $cookie) => $pages[$cookie]); + $this->assertFalse($result); + } + + public function testWasLastSearchCompleteDefaultsToTrue(): void + { + $this->assertTrue($this->executor->wasLastSearchComplete()); + } + + public function testFailedPageCollectionMarksSearchIncomplete(): void + { + $this->executor->callCollectAllPages(fn (string $cookie) => false); + $this->assertFalse($this->executor->wasLastSearchComplete()); + } + // --- helpers --- private function createSyncFilter(): SyncFilter From b9fb7638f3b321838f398dbc7ffb5813071a9f36 Mon Sep 17 00:00:00 2001 From: f2cmb <2480194+f2cmb@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:34:10 +0200 Subject: [PATCH 07/11] feat(executor): resolve LDAP page size from AuthLDAP configuration --- src/Inventory/LdapSyncExecutor.php | 18 ++++++++++++++++++ tests/LdapSyncExecutorTest.php | 28 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/src/Inventory/LdapSyncExecutor.php b/src/Inventory/LdapSyncExecutor.php index fb4d9e9..06cf508 100644 --- a/src/Inventory/LdapSyncExecutor.php +++ b/src/Inventory/LdapSyncExecutor.php @@ -615,6 +615,24 @@ protected function connectToLdap(AuthLDAP $authldap): \LDAP\Connection|false ); } + /** + * Get the page size to use for LDAP searches on this connection. + * + * @param AuthLDAP $authldap The LDAP connection configuration + * + * @return int Page size, or 0 when pagination is not available + */ + protected function getPageSize(AuthLDAP $authldap): int + { + if (!AuthLDAP::isLdapPageSizeAvailable($authldap)) { + return 0; + } + + $pagesize = $authldap->fields['pagesize'] ?? 0; + + return is_numeric($pagesize) ? max(0, (int) $pagesize) : 0; + } + /** * Perform LDAP search using filter criteria. * diff --git a/tests/LdapSyncExecutorTest.php b/tests/LdapSyncExecutorTest.php index d878642..8a5fc1a 100644 --- a/tests/LdapSyncExecutorTest.php +++ b/tests/LdapSyncExecutorTest.php @@ -465,6 +465,34 @@ public function testFailedPageCollectionMarksSearchIncomplete(): void $this->assertFalse($this->executor->wasLastSearchComplete()); } + // --- getPageSize --- + + public function testGetPageSizeReturnsConfiguredValueWhenPaginationSupported(): void + { + $authldap = $this->createItem(\AuthLDAP::class, [ + 'name' => 'paged ldap', + 'host' => 'ldap.example.com', + 'basedn' => 'dc=example,dc=com', + 'port' => 389, + 'can_support_pagesize' => 1, + 'pagesize' => 500, + ]); + $this->assertEquals(500, $this->executor->callGetPageSize($authldap)); + } + + public function testGetPageSizeReturnsZeroWhenPaginationNotSupported(): void + { + $authldap = $this->createItem(\AuthLDAP::class, [ + 'name' => 'unpaged ldap', + 'host' => 'ldap.example.com', + 'basedn' => 'dc=example,dc=com', + 'port' => 389, + 'can_support_pagesize' => 0, + 'pagesize' => 500, + ]); + $this->assertEquals(0, $this->executor->callGetPageSize($authldap)); + } + // --- helpers --- private function createSyncFilter(): SyncFilter From 9e8b3aed7a17ef5e7d27f4a5693940c7a274f90d Mon Sep 17 00:00:00 2001 From: f2cmb <2480194+f2cmb@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:38:23 +0200 Subject: [PATCH 08/11] feat(executor): fetch all LDAP result pages via LDAP_CONTROL_PAGEDRESULTS --- src/Inventory/LdapSyncExecutor.php | 177 ++++++++++++++++++++--------- 1 file changed, 124 insertions(+), 53 deletions(-) diff --git a/src/Inventory/LdapSyncExecutor.php b/src/Inventory/LdapSyncExecutor.php index 06cf508..68c121b 100644 --- a/src/Inventory/LdapSyncExecutor.php +++ b/src/Inventory/LdapSyncExecutor.php @@ -634,69 +634,85 @@ protected function getPageSize(AuthLDAP $authldap): int } /** - * Perform LDAP search using filter criteria. + * Perform a single (possibly paged) LDAP search request. * - * @param AuthLDAP $authldap The LDAP connection - * @param SyncFilter $syncfilter The sync filter with search criteria - * @param array $ldap_attrs LDAP attributes to fetch + * @param \LDAP\Connection $ds The LDAP link + * @param string $basedn Search base DN + * @param string $filter LDAP filter + * @param array $ldap_attrs Attributes to fetch + * @param string $cookie Pagination cookie ('' for the first page) + * @param int $pagesize Page size (0 = pagination disabled) * - * @return array>|false Array of LDAP entries or false on error + * @return array{entries: array>, next_cookie: string}|false */ - private function performLdapSearch(AuthLDAP $authldap, SyncFilter $syncfilter, array $ldap_attrs): array|false - { - $connection_filter = $syncfilter->fields['connection_filter'] ?? ''; - $basedn = $syncfilter->fields['basedn'] ?? ''; - - if (!is_string($connection_filter) || ($connection_filter === '' || $connection_filter === '0') || !is_string($basedn) || ($basedn === '' || $basedn === '0')) { - Toolbox::logDebug(sprintf( - 'AdvancedLDAP: Missing filter or basedn for SyncFilter %d', - $syncfilter->getID(), - )); - return false; - } - - Toolbox::logDebug(sprintf( - 'AdvancedLDAP: Searching LDAP - Filter: "%s", BaseDN: "%s", Attrs: [%s]', - $connection_filter, - $basedn, - implode(', ', $ldap_attrs), - )); - - $ds = $this->connectToLdap($authldap); - - if ($ds === false) { - Toolbox::logDebug(sprintf( - 'AdvancedLDAP: Failed to connect to LDAP server for AuthLDAP %d', - $authldap->getID(), - )); - return false; + protected function fetchLdapPage( + \LDAP\Connection $ds, + string $basedn, + string $filter, + array $ldap_attrs, + string $cookie, + int $pagesize, + ): array|false { + $controls = []; + if ($pagesize > 0) { + $controls = [ + [ + 'oid' => LDAP_CONTROL_PAGEDRESULTS, + 'iscritical' => true, + 'value' => [ + 'size' => $pagesize, + 'cookie' => $cookie, + ], + ], + ]; } - // Perform the LDAP search - $sr = @ldap_search($ds, $basedn, $connection_filter, $ldap_attrs); + $sr = @ldap_search($ds, $basedn, $filter, $ldap_attrs, 0, -1, -1, LDAP_DEREF_NEVER, $controls); if ($sr === false) { $errno = ldap_errno($ds); // 32 = LDAP_NO_SUCH_OBJECT (no results, not an error) - if ($errno !== 32) { - Toolbox::logDebug(sprintf( - 'AdvancedLDAP: LDAP search failed - Error %d: %s', - $errno, - ldap_error($ds), - )); - return false; + if ($errno === 32) { + Toolbox::logDebug('AdvancedLDAP: LDAP search returned no results (LDAP_NO_SUCH_OBJECT)'); + return ['entries' => [], 'next_cookie' => '']; } - Toolbox::logDebug('AdvancedLDAP: LDAP search returned no results (LDAP_NO_SUCH_OBJECT)'); - return []; + Toolbox::logDebug(sprintf( + 'AdvancedLDAP: LDAP search failed - Error %d: %s', + $errno, + ldap_error($ds), + )); + return false; } - // Get entries - $sr is guaranteed to be LDAP\Result after the false check above if (!$sr instanceof Result) { Toolbox::logDebug('AdvancedLDAP: Unexpected LDAP search result type'); return false; } + $next_cookie = ''; + if ($pagesize > 0) { + // Same pattern as core AuthLDAP::searchForUsers(): read the pagination cookie + // from the parsed result controls. Narrow each offset (the by-ref output is mixed). + $errcode = $matcheddn = $errmsg = $referrals = null; + $parsed_controls = []; + $parsed = @ldap_parse_result($ds, $sr, $errcode, $matcheddn, $errmsg, $referrals, $parsed_controls); // @phpstan-ignore theCodingMachineSafe.function + + $paged = is_array($parsed_controls) ? ($parsed_controls[LDAP_CONTROL_PAGEDRESULTS] ?? null) : null; + $paged_value = is_array($paged) ? ($paged['value'] ?? null) : null; + $cookie = is_array($paged_value) ? ($paged_value['cookie'] ?? null) : null; + + if ($parsed !== false && (is_string($cookie) || is_int($cookie))) { + $next_cookie = (string) $cookie; + } + } + + // 4 (openldap) / 11 = size limit exceeded: the server refused to return everything + if (in_array(ldap_errno($ds), [4, 11], true)) { + Toolbox::logDebug('AdvancedLDAP: LDAP size limit exceeded, result set is truncated'); + $this->last_search_complete = false; + } + try { $entries = ldap_get_entries($ds, $sr); } catch (Throwable $throwable) { @@ -708,23 +724,78 @@ private function performLdapSearch(AuthLDAP $authldap, SyncFilter $syncfilter, a } $count = isset($entries['count']) && is_int($entries['count']) ? $entries['count'] : 0; - Toolbox::logDebug(sprintf( - 'AdvancedLDAP: LDAP search found %d entries', - $count, - )); // Convert LDAP entries to clean array (remove 'count' key and numeric indexes) - /** @var array> $results */ - $results = []; + /** @var array> $page_entries */ + $page_entries = []; for ($i = 0; $i < $count; $i++) { if (isset($entries[$i]) && is_array($entries[$i])) { /** @var array $entry */ $entry = $entries[$i]; - $results[] = $entry; + $page_entries[] = $entry; } } - return $results; + return ['entries' => $page_entries, 'next_cookie' => $next_cookie]; + } + + /** + * Perform LDAP search using filter criteria, fetching all result pages. + * + * @param AuthLDAP $authldap The LDAP connection + * @param SyncFilter $syncfilter The sync filter with search criteria + * @param array $ldap_attrs LDAP attributes to fetch + * + * @return array>|false Array of LDAP entries or false on error + */ + protected function performLdapSearch(AuthLDAP $authldap, SyncFilter $syncfilter, array $ldap_attrs): array|false + { + $this->last_search_complete = true; + + $connection_filter = $syncfilter->fields['connection_filter'] ?? ''; + $basedn = $syncfilter->fields['basedn'] ?? ''; + + if (!is_string($connection_filter) || ($connection_filter === '' || $connection_filter === '0') || !is_string($basedn) || ($basedn === '' || $basedn === '0')) { + Toolbox::logDebug(sprintf( + 'AdvancedLDAP: Missing filter or basedn for SyncFilter %d', + $syncfilter->getID(), + )); + $this->last_search_complete = false; + return false; + } + + $ds = $this->connectToLdap($authldap); + + if ($ds === false) { + Toolbox::logDebug(sprintf( + 'AdvancedLDAP: Failed to connect to LDAP server for AuthLDAP %d', + $authldap->getID(), + )); + $this->last_search_complete = false; + return false; + } + + $pagesize = $this->getPageSize($authldap); + + Toolbox::logDebug(sprintf( + 'AdvancedLDAP: Searching LDAP - Filter: "%s", BaseDN: "%s", PageSize: %d, Attrs: [%s]', + $connection_filter, + $basedn, + $pagesize, + implode(', ', $ldap_attrs), + )); + + $entries = $this->collectAllPages( + fn (string $cookie): array|false => $this->fetchLdapPage($ds, $basedn, $connection_filter, $ldap_attrs, $cookie, $pagesize), + ); + + if ($entries === false) { + return false; + } + + Toolbox::logDebug(sprintf('AdvancedLDAP: LDAP search found %d entries', count($entries))); + + return $entries; } /** From 149f02b6c45754161b5d42f6f9db2afd92c721e7 Mon Sep 17 00:00:00 2001 From: f2cmb <2480194+f2cmb@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:40:48 +0200 Subject: [PATCH 09/11] feat(executor): expose ldap_complete flag in sync results --- src/Inventory/LdapSyncExecutor.php | 28 +++++++++++++++++----------- tests/LdapSyncExecutorTest.php | 19 +++++++++++++++++++ 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/Inventory/LdapSyncExecutor.php b/src/Inventory/LdapSyncExecutor.php index 68c121b..1f552d2 100644 --- a/src/Inventory/LdapSyncExecutor.php +++ b/src/Inventory/LdapSyncExecutor.php @@ -64,13 +64,14 @@ class LdapSyncExecutor /** * Results of the last synchronization. * - * @var array{created: int, updated: int, errors: int, skipped: int} + * @var array{created: int, updated: int, errors: int, skipped: int, ldap_complete: int} */ private array $results = [ - 'created' => 0, - 'updated' => 0, - 'errors' => 0, - 'skipped' => 0, + 'created' => 0, + 'updated' => 0, + 'errors' => 0, + 'skipped' => 0, + 'ldap_complete' => 1, ]; /** @@ -125,7 +126,7 @@ protected function collectAllPages(callable $page_fetcher): array|false * * @param AuthLDAP $authldap The LDAP connection * - * @return array{created: int, updated: int, errors: int, skipped: int} Sync results + * @return array{created: int, updated: int, errors: int, skipped: int, ldap_complete: int} Sync results */ public function executeForConnection(AuthLDAP $authldap): array { @@ -145,7 +146,7 @@ public function executeForConnection(AuthLDAP $authldap): array * * @param SyncFilter $syncfilter The sync filter to execute * - * @return array{created: int, updated: int, errors: int, skipped: int} Sync results + * @return array{created: int, updated: int, errors: int, skipped: int, ldap_complete: int} Sync results */ public function executeSingleFilter(SyncFilter $syncfilter): array { @@ -265,6 +266,10 @@ public function executeSyncFilter(AuthLDAP $authldap, SyncFilter $syncfilter): v // 3. Perform LDAP search $ldap_entries = $this->performLdapSearch($authldap, $syncfilter, $ldap_attrs); + if (!$this->last_search_complete) { + $this->results['ldap_complete'] = 0; + } + if ($ldap_entries === false) { $this->results['errors']++; return; @@ -888,10 +893,11 @@ private function getSyncFiltersForConnection(AuthLDAP $authldap): array private function resetResults(): void { $this->results = [ - 'created' => 0, - 'updated' => 0, - 'errors' => 0, - 'skipped' => 0, + 'created' => 0, + 'updated' => 0, + 'errors' => 0, + 'skipped' => 0, + 'ldap_complete' => 1, ]; } diff --git a/tests/LdapSyncExecutorTest.php b/tests/LdapSyncExecutorTest.php index 8a5fc1a..e0cac86 100644 --- a/tests/LdapSyncExecutorTest.php +++ b/tests/LdapSyncExecutorTest.php @@ -493,6 +493,25 @@ public function testGetPageSizeReturnsZeroWhenPaginationNotSupported(): void $this->assertEquals(0, $this->executor->callGetPageSize($authldap)); } + // --- ldap_complete propagation --- + + public function testExecuteSingleFilterResultsContainLdapCompleteFlag(): void + { + // No linked AuthLDAP: executor returns early, no network access. + $syncfilter = $this->createSyncFilter(); + + $results = $this->executor->executeSingleFilter($syncfilter); + + // executeSingleFilter logs the missing-AuthLDAP path (gate-hardening pass). + $this->hasPhpLogRecordThatContains( + 'AdvancedLDAP: SyncFilter ' . $syncfilter->getID() . ' has no linked AuthLDAP, nothing to synchronize', + 'Debug', + ); + + $this->assertArrayHasKey('ldap_complete', $results); + $this->assertEquals(1, $results['ldap_complete']); + } + // --- helpers --- private function createSyncFilter(): SyncFilter From 1905a1ceed2a228c82b96dc83e5324896a244d75 Mon Sep 17 00:00:00 2001 From: f2cmb <2480194+f2cmb@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:43:20 +0200 Subject: [PATCH 10/11] chore(executor): use Safe\ldap_parse_result and apply rector/lint formatting --- src/Inventory/LdapSyncExecutor.php | 29 +++++++++++++++++++---------- tests/LdapSyncExecutorTest.php | 15 ++++++++------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/Inventory/LdapSyncExecutor.php b/src/Inventory/LdapSyncExecutor.php index 1f552d2..71f93a3 100644 --- a/src/Inventory/LdapSyncExecutor.php +++ b/src/Inventory/LdapSyncExecutor.php @@ -33,6 +33,7 @@ namespace GlpiPlugin\Advancedldap\Inventory; +use LDAP\Connection; use Agent; use Throwable; use LDAP\Result; @@ -47,6 +48,7 @@ use function Safe\json_decode; use function Safe\json_encode; use function Safe\ldap_get_entries; +use function Safe\ldap_parse_result; use function Safe\preg_match_all; use function Safe\preg_replace_callback; @@ -587,9 +589,9 @@ protected function generateDeviceId(SyncFilter $syncfilter, array $ldap_entry): * * @param AuthLDAP $authldap The LDAP connection configuration * - * @return \LDAP\Connection|false The LDAP link or false on failure + * @return Connection|false The LDAP link or false on failure */ - protected function connectToLdap(AuthLDAP $authldap): \LDAP\Connection|false + protected function connectToLdap(AuthLDAP $authldap): Connection|false { $host = is_string($authldap->fields['host'] ?? null) ? $authldap->fields['host'] : ''; $port = is_string($authldap->fields['port'] ?? null) ? $authldap->fields['port'] : '389'; @@ -641,7 +643,7 @@ protected function getPageSize(AuthLDAP $authldap): int /** * Perform a single (possibly paged) LDAP search request. * - * @param \LDAP\Connection $ds The LDAP link + * @param Connection $ds The LDAP link * @param string $basedn Search base DN * @param string $filter LDAP filter * @param array $ldap_attrs Attributes to fetch @@ -651,7 +653,7 @@ protected function getPageSize(AuthLDAP $authldap): int * @return array{entries: array>, next_cookie: string}|false */ protected function fetchLdapPage( - \LDAP\Connection $ds, + Connection $ds, string $basedn, string $filter, array $ldap_attrs, @@ -697,17 +699,24 @@ protected function fetchLdapPage( $next_cookie = ''; if ($pagesize > 0) { - // Same pattern as core AuthLDAP::searchForUsers(): read the pagination cookie - // from the parsed result controls. Narrow each offset (the by-ref output is mixed). - $errcode = $matcheddn = $errmsg = $referrals = null; + // Read the pagination cookie from the parsed result controls (same pattern as + // core AuthLDAP::searchForUsers()). Narrow each offset: the by-ref output is mixed. + $errcode = null; + $matcheddn = null; + $errmsg = null; + $referrals = null; $parsed_controls = []; - $parsed = @ldap_parse_result($ds, $sr, $errcode, $matcheddn, $errmsg, $referrals, $parsed_controls); // @phpstan-ignore theCodingMachineSafe.function + try { + ldap_parse_result($ds, $sr, $errcode, $matcheddn, $errmsg, $referrals, $parsed_controls); + } catch (Throwable) { + $parsed_controls = []; + } $paged = is_array($parsed_controls) ? ($parsed_controls[LDAP_CONTROL_PAGEDRESULTS] ?? null) : null; $paged_value = is_array($paged) ? ($paged['value'] ?? null) : null; $cookie = is_array($paged_value) ? ($paged_value['cookie'] ?? null) : null; - if ($parsed !== false && (is_string($cookie) || is_int($cookie))) { + if (is_string($cookie) || is_int($cookie)) { $next_cookie = (string) $cookie; } } @@ -791,7 +800,7 @@ protected function performLdapSearch(AuthLDAP $authldap, SyncFilter $syncfilter, )); $entries = $this->collectAllPages( - fn (string $cookie): array|false => $this->fetchLdapPage($ds, $basedn, $connection_filter, $ldap_attrs, $cookie, $pagesize), + fn(string $cookie): array|false => $this->fetchLdapPage($ds, $basedn, $connection_filter, $ldap_attrs, $cookie, $pagesize), ); if ($entries === false) { diff --git a/tests/LdapSyncExecutorTest.php b/tests/LdapSyncExecutorTest.php index e0cac86..7bd4c94 100644 --- a/tests/LdapSyncExecutorTest.php +++ b/tests/LdapSyncExecutorTest.php @@ -30,6 +30,7 @@ namespace GlpiPlugin\Advancedldap\Tests; +use AuthLDAP; use Computer; use Glpi\Tests\DbTestCase; use GlpiPlugin\Advancedldap\Inventory\LdapSyncExecutor; @@ -107,7 +108,7 @@ public function callCollectAllPages(callable $page_fetcher): array|false return $this->collectAllPages($page_fetcher); } - public function callGetPageSize(\AuthLDAP $authldap): int + public function callGetPageSize(AuthLDAP $authldap): int { return $this->getPageSize($authldap); } @@ -429,7 +430,7 @@ public function testCollectAllPagesConcatenatesPagesInOrder(): void 'C1' => ['entries' => [['dn' => 'cn=c']], 'next_cookie' => 'C2'], 'C2' => ['entries' => [['dn' => 'cn=d']], 'next_cookie' => ''], ]; - $result = $this->executor->callCollectAllPages(fn (string $cookie) => $pages[$cookie]); + $result = $this->executor->callCollectAllPages(fn(string $cookie) => $pages[$cookie]); $this->assertEquals( [['dn' => 'cn=a'], ['dn' => 'cn=b'], ['dn' => 'cn=c'], ['dn' => 'cn=d']], $result, @@ -439,7 +440,7 @@ public function testCollectAllPagesConcatenatesPagesInOrder(): void public function testCollectAllPagesEmptyResult(): void { $result = $this->executor->callCollectAllPages( - fn (string $cookie) => ['entries' => [], 'next_cookie' => ''], + fn(string $cookie) => ['entries' => [], 'next_cookie' => ''], ); $this->assertEquals([], $result); } @@ -450,7 +451,7 @@ public function testCollectAllPagesReturnsFalseOnMidPaginationFailure(): void '' => ['entries' => [['dn' => 'cn=a']], 'next_cookie' => 'C1'], 'C1' => false, ]; - $result = $this->executor->callCollectAllPages(fn (string $cookie) => $pages[$cookie]); + $result = $this->executor->callCollectAllPages(fn(string $cookie) => $pages[$cookie]); $this->assertFalse($result); } @@ -461,7 +462,7 @@ public function testWasLastSearchCompleteDefaultsToTrue(): void public function testFailedPageCollectionMarksSearchIncomplete(): void { - $this->executor->callCollectAllPages(fn (string $cookie) => false); + $this->executor->callCollectAllPages(fn(string $cookie) => false); $this->assertFalse($this->executor->wasLastSearchComplete()); } @@ -469,7 +470,7 @@ public function testFailedPageCollectionMarksSearchIncomplete(): void public function testGetPageSizeReturnsConfiguredValueWhenPaginationSupported(): void { - $authldap = $this->createItem(\AuthLDAP::class, [ + $authldap = $this->createItem(AuthLDAP::class, [ 'name' => 'paged ldap', 'host' => 'ldap.example.com', 'basedn' => 'dc=example,dc=com', @@ -482,7 +483,7 @@ public function testGetPageSizeReturnsConfiguredValueWhenPaginationSupported(): public function testGetPageSizeReturnsZeroWhenPaginationNotSupported(): void { - $authldap = $this->createItem(\AuthLDAP::class, [ + $authldap = $this->createItem(AuthLDAP::class, [ 'name' => 'unpaged ldap', 'host' => 'ldap.example.com', 'basedn' => 'dc=example,dc=com', From 0acfef701062f0234810e22529c07b72567ff0f1 Mon Sep 17 00:00:00 2001 From: f2cmb <2480194+f2cmb@users.noreply.github.com> Date: Thu, 11 Jun 2026 18:01:22 +0200 Subject: [PATCH 11/11] docs(changelog): add LDAP paged results entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22bfe04..94a8a0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added - Manual LDAP to GLPI inventory synchronization from a sync filter, with a dry-run preview and an execute mode (Computer itemtype) +- Fetch all LDAP result pages using paged results (`LDAP_CONTROL_PAGEDRESULTS`), honouring the directory page size configuration +- Flag truncated or failed LDAP searches through an `ldap_complete` completeness signal ### Fixed