Fix LDAP authentication by searching UID - #4674

I tested this against a real OpenLDAP directory, comparing develop (de21b4c) with this branch (09a99a7). The main change works, but I found one regression for setups without a bind user.

Setup

• phpIPAM in the phpipam/phpipam-www Docker image (Apache, PHP 8.3.36), MariaDB loaded from db/SCHEMA.sql, OpenLDAP 2.4.57 (osixia/openldap:1.5.0), no TLS.
• LDAP method: base DN ou=people,dc=example,dc=org, UID attribute uid, bind user set to a read-only account. The directory denies anonymous search (that image's default ACL).
• Directory entries:
  • uid=alice,ou=people,dc=example,dc=org (fits the old uid=<user>,<base_dn> pattern)
  • uid=bob,ou=engineering,ou=people,dc=example,dc=org (nested OU)
  • cn=Carol Jones,ou=staff,ou=people,dc=example,dc=org with uid: carol (RDN is not the uid)
  • uid=dup under both ou=engineering and ou=staff
• phpIPAM users with the LDAP method for each of those, plus users literally named *, b*, bob) and *)(uid=* for the filter tests.

Results with a bind user configured

The slapd log shows the username escaped in the filter: (uid=\2A), (uid=b\2A), (uid=bob\29), (uid=\2A\29\28uid=\2A). With the Users DN set, the search base is the Users DN, as expected. No PHP warnings or errors in the web server log during any of these.

Problems

1. Logins that work today break when no bind user is set and the directory denies anonymous search. The form describes the bind user as optional. With it left empty, alice logs in on develop but is rejected with this PR: the search runs anonymously, returns nothing (slapd: err=32 nentries=0), and the login stops there with "Invalid username or password". If the directory allows anonymous read, the PR works without a bind user (alice, bob and carol all log in).

Suggestion: only reject outright when the search finds more than one entry, and fall back to the old DN when it finds none or cannot run. I tried this on top of the PR. Every login in the table above gave the same result (I did not rerun the Users DN row), and without a bind user alice logs in again (bob is still rejected, since there is no way to find his DN):

        // Never authenticate an ambiguous UID.
        if ($entries !== false && $entries['count'] > 1) {
            // ... same rejection as now ...
        }

        if ($entries === false || $entries['count'] === 0 || empty($entries[0]['dn'])) {
            // Search not possible (e.g. no bind user and anonymous search is denied) or no match:
            // fall back to the previous DN pattern and let the bind decide.
            $userDn = $uidAttr.'='.ldap_escape($username, '', LDAP_ESCAPE_DN).','.$searchBase;
        } else {
            $userDn = $entries[0]['dn'];
        }

2. The bind user is now actually used, which changes behaviour for existing configs. On develop the check looks at admin_username / admin_password, so the bind user is never used (slapd shows no bind as the read-only account on develop). The camelCase fix here is right, but an install with wrong bind credentials saved goes from working to "Error: Bind to Active Directory failed. Check the login credentials and/or server details." on the login page. I confirmed that with alice: OK on develop, that error with the PR. Probably worth a line in the description or the release notes.

3. Formatting. The diff undoes the Pint formatting in the functions it touches (double quotes, # comments, a space before the argument list, one-line if/elseif, the missing blank line before the NetIQ docblock). ./vendor/bin/pint --test functions/classes/class.User.php fails on this branch; running pint on the file changes about 25 lines and would shrink the diff to the real change. CONTRIBUTING.md asks to avoid formatting changes.

4. Small things. The description says the PR adds a require_once, but the current diff does not contain one; develop already has it in directory_connect(). A successful login now opens two LDAP connections and binds as the bind user three times, because directory_authenticate() connects again; passing the first connection through would avoid that. Not a blocker.

I only tested on PHP 8.3. With problem 1 addressed I would be glad to see this merged, and I am happy to retest.