From 6827b49db2e0120f7e3daf54353aac7c59e984c2 Mon Sep 17 00:00:00 2001 From: Curtis Conard Date: Thu, 5 Nov 2026 19:25:14 -0500 Subject: [PATCH 1/4] drop ldap can_support_pagesize and handle automatically --- CHANGELOG.md | 2 + .../update_12.0.x_to_13.0.0/authldap.php | 53 +++++ src/AuthLDAP.php | 193 ++++++------------ src/User.php | 56 ++--- templates/pages/setup/ldap/adv_info.html.twig | 33 ++- tests/functional/UserTest.php | 2 - 6 files changed, 161 insertions(+), 178 deletions(-) create mode 100644 install/migrations/update_12.0.x_to_13.0.0/authldap.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 579a8009b99c..d0ec20bce5c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ The present file will list all changes made to the project; according to the ### Removed - `itemtype` criteria for Ticket and Change rules. This has been non-operational since 0.85 (over 11 years ago). - POP protocol support for mails receivers and mail authentication servers. Existing mails receivers and authentication servers using it are disabled during the update. +- "Used paged results" option for LDAP authentication. If an LDAP server is not able to handle paged results, it will automatically be handled now. ### API changes @@ -38,6 +39,7 @@ The present file will list all changes made to the project; according to the - `Html::getConfirmationOnActionScript()`. Use the `data-glpi-confirm=""` attribute instead. - `Html::link()`. - `confirmaction` option of `Html::submit()`. Confirmation is now handled through the `data-glpi-confirm` attribute, so no additional action can be run on confirmation. +- `AuthLDAP::isLdapPageSizeAvailable()`. The LDAP page size is always available now (with an automatic fallback at least). ## [12.0.2] unreleased diff --git a/install/migrations/update_12.0.x_to_13.0.0/authldap.php b/install/migrations/update_12.0.x_to_13.0.0/authldap.php new file mode 100644 index 000000000000..176e5cee6362 --- /dev/null +++ b/install/migrations/update_12.0.x_to_13.0.0/authldap.php @@ -0,0 +1,53 @@ +. + * + * --------------------------------------------------------------------- + */ + +/** + * @var Migration $migration + * @var DBmysql $DB + */ + +// can_support_pagesize has not been needed since GLPI stopped supporting PHP versions older than 5.4 +$migration->dropField('glpi_authldaps', 'can_support_pagesize'); + +// pagesize is always available now (at least with an automatic fallback), so we can set it to 1000 for all existing LDAP authentication servers where it is set to 0 or null +$iterator = $DB->request([ + 'SELECT' => ['id'], + 'FROM' => 'glpi_authldaps', + 'WHERE' => [ + 'pagesize' => ['IN', [0, null]], + ], +]); +foreach ($iterator as $data) { + $DB->update('glpi_authldaps', ['pagesize' => 1000], ['id' => $data['id']]); +} diff --git a/src/AuthLDAP.php b/src/AuthLDAP.php index fbbf19266a6f..db1fe5f5c10a 100644 --- a/src/AuthLDAP.php +++ b/src/AuthLDAP.php @@ -207,8 +207,7 @@ public function post_getEmpty() $this->fields['use_bind'] = 1; $this->fields['picture_field'] = ''; $this->fields['responsible_field'] = ''; - $this->fields['can_support_pagesize'] = 0; - $this->fields['pagesize'] = 0; + $this->fields['pagesize'] = 1000; $this->fields['ldap_maxlimit'] = 0; $this->fields['begin_date_field'] = ''; $this->fields['end_date_field'] = ''; @@ -249,7 +248,6 @@ public function preconfig($type) $this->fields['comment_field'] = 'info'; $this->fields['title_field'] = 'title'; $this->fields['use_dn'] = 1; - $this->fields['can_support_pagesize'] = 1; $this->fields['pagesize'] = 1000; $this->fields['picture_field'] = ''; $this->fields['responsible_field'] = 'manager'; @@ -279,7 +277,6 @@ public function preconfig($type) $this->fields['comment_field'] = 'description'; $this->fields['title_field'] = 'title'; $this->fields['use_dn'] = 1; - $this->fields['can_support_pagesize'] = 1; $this->fields['pagesize'] = 1000; $this->fields['picture_field'] = 'jpegphoto'; $this->fields['responsible_field'] = 'manager'; @@ -489,7 +486,7 @@ public function showForm($ID, array $options = []) 'realname_field', 'registration_number_field', 'title_field', 'use_dn', 'use_tls', 'picture_field', 'responsible_field', 'begin_date_field', 'end_date_field', 'category_field', 'language_field', 'location_field', - 'can_support_pagesize', 'pagesize', + 'pagesize', ]; } @@ -537,7 +534,6 @@ public function showFormAdvancedConfig() TemplateRenderer::getInstance()->display('pages/setup/ldap/adv_info.html.twig', [ 'item' => $this, - 'page_size_available' => self::isLdapPageSizeAvailable(false, false), 'gmt_values' => Dropdown::getGMTValues(), 'params' => [ 'formfooter' => false, @@ -1739,60 +1735,41 @@ public static function searchForUsers( &$ldap_users_by_dn = null ) { - // If paged results cannot be used (PHP < 5.4) $cookie = ''; // Cookie used to perform query using pages $count = 0; // Store the number of results ldap_search do { - if (self::isLdapPageSizeAvailable($config_ldap)) { - $controls = [ - [ - 'oid' => LDAP_CONTROL_PAGEDRESULTS, - 'iscritical' => true, - 'value' => [ - 'size' => $config_ldap->fields['pagesize'], - 'cookie' => $cookie, - ], + // The paged results control is not critical: if the server does not support it (or restricts it), + // it will ignore it and return all results at once, without any page cookie. + $controls = [ + [ + 'oid' => LDAP_CONTROL_PAGEDRESULTS, + 'iscritical' => false, + 'value' => [ + 'size' => $config_ldap->fields['pagesize'], + 'cookie' => $cookie, ], - ]; - $sr = @ldap_search($ds, $values['basedn'], $filter, $attrs, 0, -1, -1, LDAP_DEREF_NEVER, $controls); - if ( - $sr === false - || @ldap_parse_result($ds, $sr, $errcode, $matcheddn, $errmsg, $referrals, $controls) === false // @phpstan-ignore theCodingMachineSafe.function - ) { - // 32 = LDAP_NO_SUCH_OBJECT => This error can be silented as it just means that search produces no result. - if (ldap_errno($ds) !== 32) { - trigger_error( - static::buildError( - $ds, - sprintf('LDAP search with base DN `%s` and filter `%s` failed', $values['basedn'], $filter) - ), - E_USER_WARNING - ); - } - return false; - } - if (isset($controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie'])) { - $cookie = $controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie']; - } else { - $cookie = ''; - } - } else { - $sr = @ldap_search($ds, $values['basedn'], $filter, $attrs); - if ($sr === false) { - // 32 = LDAP_NO_SUCH_OBJECT => This error can be silented as it just means that search produces no result. - if (ldap_errno($ds) !== 32) { - trigger_error( - static::buildError( - $ds, - sprintf('LDAP search with base DN `%s` and filter `%s` failed', $values['basedn'], $filter) - ), - E_USER_WARNING - ); - } - return false; + ], + ]; + $sr = @ldap_search($ds, $values['basedn'], $filter, $attrs, 0, -1, -1, LDAP_DEREF_NEVER, $controls); + if ( + $sr === false + || @ldap_parse_result($ds, $sr, $errcode, $matcheddn, $errmsg, $referrals, $controls) === false // @phpstan-ignore theCodingMachineSafe.function + ) { + // 32 = LDAP_NO_SUCH_OBJECT => This error can be silented as it just means that search produces no result. + if (ldap_errno($ds) !== 32) { + trigger_error( + static::buildError( + $ds, + sprintf('LDAP search with base DN `%s` and filter `%s` failed', $values['basedn'], $filter) + ), + E_USER_WARNING + ); } + return false; } + // No page cookie in the response means that there are no more pages (or that paging is not supported) + $cookie = $controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie'] ?? ''; if (in_array(ldap_errno($ds), [4,11])) { // openldap return 4 for Size limit exceeded @@ -1806,11 +1783,10 @@ public static function searchForUsers( } $count += $info['count']; - //If page results are enabled and the number of results is greater than the maximum allowed + //If the number of results is greater than the maximum allowed //warn user that limit is exceeded and stop search if ( - self::isLdapPageSizeAvailable($config_ldap) - && $config_ldap->fields['ldap_maxlimit'] + $config_ldap->fields['ldap_maxlimit'] && ($count > $config_ldap->fields['ldap_maxlimit']) ) { $limitexceeded = true; @@ -2418,55 +2394,37 @@ public static function getGroupsFromLDAP( $cookie = ''; $count = 0; do { - if (self::isLdapPageSizeAvailable($config_ldap)) { - $controls = [ - [ - 'oid' => LDAP_CONTROL_PAGEDRESULTS, - 'iscritical' => true, - 'value' => [ - 'size' => $config_ldap->fields['pagesize'], - 'cookie' => $cookie, - ], + // The paged results control is not critical: if the server does not support it (or restricts it), + // it will ignore it and return all results at once, without any page cookie. + $controls = [ + [ + 'oid' => LDAP_CONTROL_PAGEDRESULTS, + 'iscritical' => false, + 'value' => [ + 'size' => $config_ldap->fields['pagesize'], + 'cookie' => $cookie, ], - ]; - $sr = @ldap_search($ldap_connection, $config_ldap->fields['basedn'], $filter, $attrs, 0, -1, -1, LDAP_DEREF_NEVER, $controls); - if ( - $sr === false - || @ldap_parse_result($ldap_connection, $sr, $errcode, $matcheddn, $errmsg, $referrals, $controls) === false // @phpstan-ignore theCodingMachineSafe.function - ) { - // 32 = LDAP_NO_SUCH_OBJECT => This error can be silented as it just means that search produces no result. - if (ldap_errno($ldap_connection) !== 32) { - trigger_error( - static::buildError( - $ldap_connection, - sprintf('LDAP search with base DN `%s` and filter `%s` failed', $config_ldap->fields['basedn'], $filter) - ), - E_USER_WARNING - ); - } - return $groups; - } - if (isset($controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie'])) { - $cookie = $controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie']; - } else { - $cookie = ''; - } - } else { - $sr = @ldap_search($ldap_connection, $config_ldap->fields['basedn'], $filter, $attrs); - if ($sr === false) { - // 32 = LDAP_NO_SUCH_OBJECT => This error can be silented as it just means that search produces no result. - if (ldap_errno($ldap_connection) !== 32) { - trigger_error( - static::buildError( - $ldap_connection, - sprintf('LDAP search with base DN `%s` and filter `%s` failed', $config_ldap->fields['basedn'], $filter) - ), - E_USER_WARNING - ); - } - return $groups; + ], + ]; + $sr = @ldap_search($ldap_connection, $config_ldap->fields['basedn'], $filter, $attrs, 0, -1, -1, LDAP_DEREF_NEVER, $controls); + if ( + $sr === false + || @ldap_parse_result($ldap_connection, $sr, $errcode, $matcheddn, $errmsg, $referrals, $controls) === false // @phpstan-ignore theCodingMachineSafe.function + ) { + // 32 = LDAP_NO_SUCH_OBJECT => This error can be silented as it just means that search produces no result. + if (ldap_errno($ldap_connection) !== 32) { + trigger_error( + static::buildError( + $ldap_connection, + sprintf('LDAP search with base DN `%s` and filter `%s` failed', $config_ldap->fields['basedn'], $filter) + ), + E_USER_WARNING + ); } + return $groups; } + // No page cookie in the response means that there are no more pages (or that paging is not supported) + $cookie = $controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie'] ?? ''; if (in_array(ldap_errno($ldap_connection), [4,11])) { // openldap return 4 for Size limit exceeded @@ -2480,11 +2438,10 @@ public static function getGroupsFromLDAP( } $count += $infos['count']; - // If page results are enabled and the number of results is greater than the maximum allowed + // If the number of results is greater than the maximum allowed // warn user that limit is exceeded and stop search if ( - self::isLdapPageSizeAvailable($config_ldap) - && $config_ldap->fields['ldap_maxlimit'] + $config_ldap->fields['ldap_maxlimit'] && ($count > $config_ldap->fields['ldap_maxlimit']) ) { $limitexceeded = true; @@ -3899,8 +3856,8 @@ public function post_addItem() public function prepareInputForAdd($input) { - if (empty($input['can_support_pagesize'] ?? '')) { - $input['can_support_pagesize'] = 0; + if (empty($input['pagesize'] ?? '')) { + $input['pagesize'] = 1000; } if (!empty($input["rootdn_passwd"])) { @@ -4106,28 +4063,6 @@ public static function getAllReplicateForAMaster($master_id) return $replicates; } - /** - * Check if ldap results can be paged or not - * This functionality is available for PHP 5.4 and higher - * - * @since 0.84 - * - * @param ($check_config_value is true ? object : false) $config_ldap LDAP configuration. May only be false if $check_config_value is also false. - * @param bool $check_config_value Whether to check config values - * - * @return bool true if maxPageSize can be used, false otherwise - */ - public static function isLdapPageSizeAvailable($config_ldap, $check_config_value = true) - { - return ( - extension_loaded('ldap') - && ( - !$check_config_value - || $config_ldap->fields['can_support_pagesize'] - ) - ); - } - /** * Does LDAP user already exists in the database? * diff --git a/src/User.php b/src/User.php index 8d0045ac9f35..3d236f10e755 100644 --- a/src/User.php +++ b/src/User.php @@ -2419,6 +2419,11 @@ private function getFromLDAPGroupDiscretCached( ? $ldap_method['condition'] : '(objectClass=*)'; + $page_size = (int) ($ldap_method['pagesize'] ?? 0); + if ($page_size <= 0) { + $page_size = 1000; + } + $groups_iterator = $DB->request([ 'SELECT' => ['id', 'ldap_group_dn'], 'FROM' => 'glpi_groups', @@ -2437,34 +2442,31 @@ private function getFromLDAPGroupDiscretCached( do { $last_db_activity = self::keepDbConnectionAlive($last_db_activity); - if (!empty($ldap_method['pagesize'])) { - $controls = [[ - 'oid' => LDAP_CONTROL_PAGEDRESULTS, - 'iscritical' => true, - 'value' => ['size' => (int) $ldap_method['pagesize'], 'cookie' => $cookie], - ]]; - $sr = @ldap_search( - $ldap_connection, - $ldap_method['basedn'], - $filter, - ['dn'], - 0, - -1, - -1, - LDAP_DEREF_NEVER, - $controls - ); - if ( - $sr instanceof Result - && @ldap_parse_result($ldap_connection, $sr, $errcode, $matcheddn, $errmsg, $referrals, $controls) !== false // @phpstan-ignore theCodingMachineSafe.function - && isset($controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie']) - ) { - $cookie = $controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie']; - } else { - $cookie = ''; - } + // The paged results control is not critical: if the server does not support it (or restricts it), + // it will ignore it and return all results at once, without any page cookie. + $controls = [[ + 'oid' => LDAP_CONTROL_PAGEDRESULTS, + 'iscritical' => false, + 'value' => ['size' => $page_size, 'cookie' => $cookie], + ]]; + $sr = @ldap_search( + $ldap_connection, + $ldap_method['basedn'], + $filter, + ['dn'], + 0, + -1, + -1, + LDAP_DEREF_NEVER, + $controls + ); + if ( + $sr instanceof Result + && @ldap_parse_result($ldap_connection, $sr, $errcode, $matcheddn, $errmsg, $referrals, $controls) !== false // @phpstan-ignore theCodingMachineSafe.function + ) { + // No page cookie in the response means that there are no more pages (or that paging is not supported) + $cookie = $controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie'] ?? ''; } else { - $sr = @ldap_search($ldap_connection, $ldap_method['basedn'], $filter, ['dn']); $cookie = ''; } diff --git a/templates/pages/setup/ldap/adv_info.html.twig b/templates/pages/setup/ldap/adv_info.html.twig index 4ef63183fb8e..9a0c6edcffa6 100644 --- a/templates/pages/setup/ldap/adv_info.html.twig +++ b/templates/pages/setup/ldap/adv_info.html.twig @@ -37,26 +37,19 @@ {{ fields.dropdownYesNo('use_tls', item.fields['use_tls'], __('Use TLS')) }} {{ fields.dropdownArrayField('time_offset', item.fields['time_offset'], gmt_values, __('LDAP directory time zone')) }} - {% if page_size_available %} - {{ fields.dropdownYesNo('can_support_pagesize', item.fields['can_support_pagesize'], __('Use paged results')) }} - {{ fields.dropdownNumberField('pagesize', item.fields['pagesize'], __('Page size'), { - min: 100, - max: 100000, - step: 100 - }) }} - {{ fields.dropdownNumberField('ldap_maxlimit', item.fields['ldap_maxlimit'], __('Maximum number of results'), { - min: 100, - max: 999999, - step: 100, - toadd: { - 0: __('Unlimited') - } - }) }} - {% else %} - - - - {% endif %} + {{ fields.dropdownNumberField('pagesize', item.fields['pagesize'], __('Page size'), { + min: 100, + max: 100000, + step: 100 + }) }} + {{ fields.dropdownNumberField('ldap_maxlimit', item.fields['ldap_maxlimit'], __('Maximum number of results'), { + min: 100, + max: 999999, + step: 100, + toadd: { + 0: __('Unlimited') + } + }) }} {{ fields.dropdownArrayField('deref_option', item.fields['deref_option'], { (constant('LDAP_DEREF_NEVER')): __('Never dereferenced (default)'), diff --git a/tests/functional/UserTest.php b/tests/functional/UserTest.php index bd8d965cae1d..e1d84fcebf6b 100644 --- a/tests/functional/UserTest.php +++ b/tests/functional/UserTest.php @@ -3040,7 +3040,6 @@ public function testGetFromLDAPGroupDiscretChainOidRouting(): void 'id' => $auth->getID(), 'basedn' => 'dc=example,dc=com', 'condition' => '', - 'pagesize' => 0, 'group_field' => 'member', 'use_dn' => 1, 'group_condition' => '', @@ -3100,7 +3099,6 @@ public function testGetFromLDAPGroupDiscretCachedUsesStaticCache(): void 'id' => $auth->getID(), 'basedn' => 'dc=example,dc=com', 'condition' => '', - 'pagesize' => 0, 'group_field' => 'member', 'group_member_field' => 'member:' . AuthLDAP::MATCHING_RULE_IN_CHAIN_OID, 'use_dn' => 1, From 179101492e3778603bcb53401e8b06ee573e7b35 Mon Sep 17 00:00:00 2001 From: Curtis Conard Date: Thu, 5 Nov 2026 19:40:22 -0500 Subject: [PATCH 2/4] fixes --- install/mysql/glpi-empty.sql | 1 - src/AuthLDAP.php | 16 ++++- tests/LDAP/AuthLdapTest.php | 133 +++++++++++++++-------------------- 3 files changed, 70 insertions(+), 80 deletions(-) diff --git a/install/mysql/glpi-empty.sql b/install/mysql/glpi-empty.sql index b562040c27ec..dd969d1fcfe8 100644 --- a/install/mysql/glpi-empty.sql +++ b/install/mysql/glpi-empty.sql @@ -109,7 +109,6 @@ CREATE TABLE `glpi_authldaps` ( `responsible_field` varchar(255) DEFAULT NULL, `pagesize` int NOT NULL DEFAULT '0', `ldap_maxlimit` int NOT NULL DEFAULT '0', - `can_support_pagesize` tinyint NOT NULL DEFAULT '0', `picture_field` varchar(255) DEFAULT NULL, `begin_date_field` varchar(255) DEFAULT NULL, `end_date_field` varchar(255) DEFAULT NULL, diff --git a/src/AuthLDAP.php b/src/AuthLDAP.php index db1fe5f5c10a..1ef3e8f602b1 100644 --- a/src/AuthLDAP.php +++ b/src/AuthLDAP.php @@ -292,6 +292,9 @@ public function preconfig($type) public function prepareInputForUpdate($input) { + if (isset($input['pagesize']) && ((int) $input['pagesize'] <= 0)) { + unset($input['pagesize']); + } if (isset($input["rootdn_passwd"])) { if (empty($input["rootdn_passwd"])) { unset($input["rootdn_passwd"]); @@ -1737,6 +1740,10 @@ public static function searchForUsers( $cookie = ''; // Cookie used to perform query using pages $count = 0; // Store the number of results ldap_search + $page_size = (int) ($config_ldap->fields['pagesize'] ?? 0); + if ($page_size <= 0) { + $page_size = 1000; + } do { // The paged results control is not critical: if the server does not support it (or restricts it), @@ -1746,7 +1753,7 @@ public static function searchForUsers( 'oid' => LDAP_CONTROL_PAGEDRESULTS, 'iscritical' => false, 'value' => [ - 'size' => $config_ldap->fields['pagesize'], + 'size' => $page_size, 'cookie' => $cookie, ], ], @@ -2393,6 +2400,11 @@ public static function getGroupsFromLDAP( } $cookie = ''; $count = 0; + $page_size = (int) ($config_ldap->fields['pagesize'] ?? 0); + if ($page_size <= 0) { + $page_size = 1000; + } + do { // The paged results control is not critical: if the server does not support it (or restricts it), // it will ignore it and return all results at once, without any page cookie. @@ -2401,7 +2413,7 @@ public static function getGroupsFromLDAP( 'oid' => LDAP_CONTROL_PAGEDRESULTS, 'iscritical' => false, 'value' => [ - 'size' => $config_ldap->fields['pagesize'], + 'size' => $page_size, 'cookie' => $cookie, ], ], diff --git a/tests/LDAP/AuthLdapTest.php b/tests/LDAP/AuthLdapTest.php index dc97689c3dd0..465f2235e95b 100644 --- a/tests/LDAP/AuthLdapTest.php +++ b/tests/LDAP/AuthLdapTest.php @@ -3325,52 +3325,43 @@ public function testSearchForUsersIndexesUsersByDn(): void public static function searchForUsersErrorsProvider(): iterable { - // error messages should be identical whether pagesize support is enabled or not - $configs = [ - [ - 'can_support_pagesize' => 0, - ], - [ - 'can_support_pagesize' => 1, - 'pagesize' => 100, + // invalid base DN + yield [ + 'config_fields' => [ + 'pagesize' => 100, ], + 'basedn' => 'notavalidbasedn', + 'filter' => '(objectclass=inetOrgPerson)', + 'error' => implode( + "\n", + [ + 'LDAP search with base DN `notavalidbasedn` and filter `(objectclass=inetOrgPerson)` failed', + 'error: Invalid DN syntax (34)', + 'extended error: invalid DN', + 'err string: invalid DN', + ] + ), ]; - foreach ($configs as $config_fields) { - // invalid base DN - yield [ - 'config_fields' => $config_fields, - 'basedn' => 'notavalidbasedn', - 'filter' => '(objectclass=inetOrgPerson)', - 'error' => implode( - "\n", - [ - 'LDAP search with base DN `notavalidbasedn` and filter `(objectclass=inetOrgPerson)` failed', - 'error: Invalid DN syntax (34)', - 'extended error: invalid DN', - 'err string: invalid DN', - ] - ), - ]; - // invalid filter - yield [ - 'config_fields' => $config_fields, - 'basedn' => 'dc=glpi,dc=org', - 'filter' => 'notavalidfilter', - 'error' => implode( - "\n", - [ - 'LDAP search with base DN `dc=glpi,dc=org` and filter `notavalidfilter` failed', - 'error: Bad search filter (-7)', - ] - ), - ]; - } + // invalid filter + yield [ + 'config_fields' => [ + 'pagesize' => 100, + ], + 'basedn' => 'dc=glpi,dc=org', + 'filter' => 'notavalidfilter', + 'error' => implode( + "\n", + [ + 'LDAP search with base DN `dc=glpi,dc=org` and filter `notavalidfilter` failed', + 'error: Bad search filter (-7)', + ] + ), + ]; // invalid pagesize yield [ 'config_fields' => [ - 'can_support_pagesize' => 1, 'pagesize' => 0, ], 'basedn' => 'dc=glpi,dc=org', @@ -3475,50 +3466,38 @@ public function testSearchUserDnErrorMessages( public static function getGroupsFromLDAPErrorsProvider(): iterable { - // error messages should be identical whether pagesize support is enabled or not - $configs = [ - [ - 'can_support_pagesize' => 0, - ], - [ - 'can_support_pagesize' => 1, - 'pagesize' => 100, - ], + // invalid base DN + yield [ + 'config_fields' => ['pagesize' => 100, 'basedn' => 'notavalidbasedn'], + 'filter' => '(objectclass=inetOrgPerson)', + 'error' => implode( + "\n", + [ + 'LDAP search with base DN `notavalidbasedn` and filter `(objectclass=inetOrgPerson)` failed', + 'error: Invalid DN syntax (34)', + 'extended error: invalid DN', + 'err string: invalid DN', + ] + ), + ]; + + // invalid filter + yield [ + 'config_fields' => ['pagesize' => 100], + 'filter' => 'notavalidfilter', + 'error' => implode( + "\n", + [ + 'LDAP search with base DN `dc=glpi,dc=org` and filter `notavalidfilter` failed', + 'error: Bad search filter (-7)', + ] + ), ]; - foreach ($configs as $config_fields) { - // invalid base DN - yield [ - 'config_fields' => $config_fields + ['basedn' => 'notavalidbasedn'], - 'filter' => '(objectclass=inetOrgPerson)', - 'error' => implode( - "\n", - [ - 'LDAP search with base DN `notavalidbasedn` and filter `(objectclass=inetOrgPerson)` failed', - 'error: Invalid DN syntax (34)', - 'extended error: invalid DN', - 'err string: invalid DN', - ] - ), - ]; - // invalid filter - yield [ - 'config_fields' => $config_fields, - 'filter' => 'notavalidfilter', - 'error' => implode( - "\n", - [ - 'LDAP search with base DN `dc=glpi,dc=org` and filter `notavalidfilter` failed', - 'error: Bad search filter (-7)', - ] - ), - ]; - } // invalid pagesize yield [ 'config_fields' => [ - 'can_support_pagesize' => 1, 'pagesize' => 0, ], 'filter' => '(objectclass=groupOfNames)', From d4d3e1ad90cc2e51b2b8f4d68a40fe951478642a Mon Sep 17 00:00:00 2001 From: Curtis Conard Date: Thu, 5 Nov 2026 19:50:54 -0500 Subject: [PATCH 3/4] update phpstan baseline --- .phpstan-baseline.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.phpstan-baseline.php b/.phpstan-baseline.php index 6880c6fa8af4..bebdf393250b 100644 --- a/.phpstan-baseline.php +++ b/.phpstan-baseline.php @@ -1204,7 +1204,7 @@ $ignoreErrors[] = [ 'message' => '#^Access to an undefined property object\\:\\:\\$fields\\.$#', 'identifier' => 'property.notFound', - 'count' => 16, + 'count' => 11, 'path' => __DIR__ . '/src/AuthLDAP.php', ]; $ignoreErrors[] = [ From bacfce1964c3cef101c1cbd2a766f085986b9e01 Mon Sep 17 00:00:00 2001 From: Curtis Conard Date: Thu, 5 Nov 2026 20:21:16 -0500 Subject: [PATCH 4/4] fix tests --- tests/LDAP/AuthLdapTest.php | 34 +--------------------------------- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/tests/LDAP/AuthLdapTest.php b/tests/LDAP/AuthLdapTest.php index 465f2235e95b..3366dbb2c8b4 100644 --- a/tests/LDAP/AuthLdapTest.php +++ b/tests/LDAP/AuthLdapTest.php @@ -151,7 +151,7 @@ public function testPost_getEmpty() { $ldap = new AuthLDAP(); $ldap->post_getEmpty(); - $this->assertCount(30, $ldap->fields); + $this->assertCount(29, $ldap->fields); } public function testUnsetUndisclosedFields() @@ -3358,22 +3358,6 @@ public static function searchForUsersErrorsProvider(): iterable ] ), ]; - - // invalid pagesize - yield [ - 'config_fields' => [ - 'pagesize' => 0, - ], - 'basedn' => 'dc=glpi,dc=org', - 'filter' => '(objectclass=inetOrgPerson)', - 'error' => implode( - "\n", - [ - 'LDAP search with base DN `dc=glpi,dc=org` and filter `(objectclass=inetOrgPerson)` failed', - 'error: Bad parameter to an ldap routine (-9)', - ] - ), - ]; } #[RequiresPhpExtension('ldap')] @@ -3493,22 +3477,6 @@ public static function getGroupsFromLDAPErrorsProvider(): iterable ] ), ]; - - - // invalid pagesize - yield [ - 'config_fields' => [ - 'pagesize' => 0, - ], - 'filter' => '(objectclass=groupOfNames)', - 'error' => implode( - "\n", - [ - 'LDAP search with base DN `dc=glpi,dc=org` and filter `(objectclass=groupOfNames)` failed', - 'error: Bad parameter to an ldap routine (-9)', - ] - ), - ]; } #[RequiresPhpExtension('ldap')]