[jf-cbd]

Base information

Question	Answer
Related to a SourceForge thread / Another PR / a GitHub Issue / Combodo ticket?	N°10246
Type of change?	Bug fix

Symptom (bug) / Objective (enhancement)

When the LDAP servers setting contains at least one server without a uri, writing the configuration can fail. On PHP 8, this can raise a TypeError because array_key_exists() is called with its arguments in the wrong order. The intended behavior is to build the missing URI from that server’s host and port.

Reproduction procedure (bug)

1. On iTop 3.3.0
2. With PHP 8.3
3. Configure the authent-ldap module with a non-empty servers setting containing a server without a uri.
4. Trigger a configuration write that invokes AuthentLDAPInstaller::BeforeWritingConfig (e.g. by upgrading to iTop 3.3.0)
5. Observe the error or warning while the missing URI is processed, instead of the configuration being written successfully.

Cause (bug)

In datamodels/2.x/authent-ldap/module.authent-ldap.php, the arguments to array_key_exists() were reversed. The code also read host and port from the undefined $aServerParams variable instead of the current $aServer entry.

Proposed solution (bug and enhancement)

Corrected the array_key_exists() argument order and use $aServer to read the host and port when constructing a missing URI. Added unset($aServer) after the by-reference foreach to break its lingering reference.

Checklist before requesting a review

• If my PR is a bug fix or an enhancement, I opened an issue that got reviewed and approved by Combodo
• I have performed a self-review of my code
• I have tested all changes I made on an iTop instance
• I have added a unit test, otherwise I have explained why I couldn't — no unit test was added in this commit
• Is the PR clear and detailed enough so anyone can understand without digging in the code?

[Copilot]

🟢 Approval recommended

No active review issues are reported for the LDAP configuration-writing changes.

No open issues 🎉

What changed in this PR

Fixes LDAP configuration writing when a server entry lacks a URI, allowing the installer to construct it from that server’s host and port.

Changes:

• Corrects the array_key_exists() argument order to avoid a PHP 8 TypeError.
• Reads host and port from the current server entry instead of an undefined variable.
• Clears the lingering reference after the foreach loop.

File	Description
datamodels/​2.x/​authent-ldap/​module.authent-ldap.php	Fixes missing-URI handling and cleans up the loop reference before saving configuration.

_{Review effort: Balanced}

[greptile-apps]

Confidence Score: 5/5

[Low impact] The PR appears safe to merge.

Summary

Fixes configuration writes for LDAP servers missing a uri.

• LDAP server entries get a URI from their own host and port.

_{Reviews (1) · Last reviewed commit: "Fix LDAP server URI configuration" · Reviewed by Greptile}

[Molkobain]

Note: Mind to prefix the PR title with the ticket reference and to keep it when doing the "squash & merge", otherwise the information won't be synced with our iTop.

[jf-cbd]

Indeed @Molkobain, we noticed that the squash and merge default name won't be the PR current name but the initial PR name*. That explains why some community PRs I merge with default proposition didn't have the number included in the commit message. Good to know for next times! Thanks for the notice.

• [EDIT] on the first commit message

[Hipska]

That happens when there’s only 1 commit. As far as I know the template can be configured

[Molkobain]

That happens when there’s only 1 commit. As far as I know the template can be configured

Template of the default squash commit message?

[Molkobain]

Indeed, thanks Thomas!