[cconard96]

Checklist before requesting a review

• I have read the CONTRIBUTING document.
• I have performed a self-review of my code.

Description

Virtually every modern LDAP server natively supports paged results. It is an extension of LDAPv3 which has been around since 2000. There are only rare edge-cases where GLPI would need to handle non-paged results and this can be handled automatically by GLPI by requesting paged results, non-critically, and then checking if the server responds with a paged result or not.

If needed, we can re-introduce allowing admins to explicitly disable paging by treating a pagesize of 0 as disabled (critical true).

[qodo-free-for-open-source-projects]

Code Review by Qodo

View medium (1)

🟠 **Medium**

1. Automatic LDAP fallback has no test 🐞 Bug

Description

The new noncritical-control paths in searchForUsers() and getGroupsFromLDAP() have no regression
test for a response without a paging cookie. The test change only removes zero page sizes from two
fixtures, while existing LDAP cases still toggle the now-ignored can_support_pagesize, so they do
not exercise the advertised fallback.

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The feature's unsupported-paging fallback has no test that distinguishes it from the prior behavior.
## Fix Focus Areas
- src/AuthLDAP.php[1742-1772]
- src/AuthLDAP.php[2397-2427]
- tests/LDAP/AuthLdapTest.php[3326-3340]
- tests/functional/UserTest.php[3037-3048]
## Recommended Fix
Add an LDAP integration regression test in which the server ignores the noncritical paging control and returns results without a cookie. Verify that user and group searches consume those results and terminate, and remove reliance on the obsolete capability flag in relevant test cases.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Resolved findings

1. Upgrades report a false schema failure ✓ Resolved

Description

The LDAP migration drops can_support_pagesize, but install/mysql/glpi-empty.sql still declares
the column. After an upgrade, the schema-integrity check compares the migrated database with that
file, and the database update command exits with an error despite completing the migration.

2. LDAP searches fail after zero-size updates ✓ Resolved

Description

prepareInputForUpdate() accepts pagesize = 0, while the new unconditional user and group
searches pass that value into the LDAP paging control. An update that saves zero therefore reaches
the bad-parameter error exercised by the LDAP tests, instead of using the previous unpaged path when
paging was disabled.

[trasher]

Seem correct from a code point of view; and there is less code.
Change itself seem globally correct to me. Maybe should we make sure it still can work on a non compliant implementation?

[cconard96]

Maybe should we make sure it still can work on a non compliant implementation?

I'm not sure I can. I only have, and have experience with, Active Directory and this is a core, hard-coded feature that doesn't seem to be able to be disabled.

I think this is something we may only identify issues with after it is used in the wild with obscure LDAP servers that refuse to follow the LDAP spec(s) (much like email clients refusing to adhere to specs).