do not construct user DN, but use DN from search
The proper way to construct a user DN is not by appending LDAP_BIND_PREFIX or LDAP_BIND_SUFFIX to LDAP_USERNAME_FIELD
Instead use DN from ldap_search
The reason is that you might have multiple suffixes for user DNs in LDAP ie: ou=People,dc=example,dc=com ou=People,dc=sub,dc=example,dc=com
Not sure that this PR is necessary as the current version already supports authenticating the user by its DN and is more flexible in other senarios. To use DN with the current version use following settings:
- Define
LDAP_USERNAME_FIELDas"dn"or"distinguishedname" - Leave
LDAP_BIND_PREFIXandLDAP_BIND_SUFFIXcommented
Yes i've tried setting: LDAP_USERNAME_FIELD as "dn" but later in your code you use $user = $result[0][LDAP_USERNAME_FIELD][0];
which is not valid for ['dn'] all entries have $result[0]['attr'][0] except dn (see ps at end)
I deal with ldap servers and ldap-authenticated application for many years now, most of them do the following ldap_bind/ldap_search to find user_dn ldap_bind with user_dn to check user auth
Constructing user_dn with ldap_base and other fields is ad-hoc
best regards
G ps. see example from ldap_get_entries:
Array( [count] => 1 [0] => Array ( [mail] => Array ( [count] => 1 [0] => [email protected] )
[0] => mail
[count] => 1
[dn] => uid=bilias,ou=People,dc=example,dc=com
)
Sorry I didn't saw that dn doesn't have an array. The following code should fix that and keeps compatibility
if (in_array('group', array_values($result[0]['objectclass']), true)) {
// do not login as group
return null;
}
if (!isset($result[0][LDAP_USERNAME_FIELD])) {
@trigger_error(__('ldapno03') . ' "' . LDAP_USERNAME_FIELD . '" ' . __('ldapresults03'));
return null;
}
if (!is_array($result[0][LDAP_USERNAME_FIELD])) {
$user = $result[0][LDAP_USERNAME_FIELD];
} elseif (isset($result[0][LDAP_USERNAME_FIELD][0])) {
$user = $result[0][LDAP_USERNAME_FIELD][0];
} else {
@trigger_error(__('ldapno03') . ' "' . LDAP_USERNAME_FIELD . '" ' . __('ldapresults03'));
return null;
}
if (defined('LDAP_BIND_PREFIX')) {
$user = LDAP_BIND_PREFIX . $user;
}
if (defined('LDAP_BIND_SUFFIX')) {
$user .= LDAP_BIND_SUFFIX;
}
Yes it does :) but it's still ad-hoc, adds complexity and I believe it's prune to errors.
You're doing all that to "construct" the user_dn php-ldap already has a function to do that. use it and don't do all those checks and ninja coding
Maybe there is something I don't understand. Do you use LDAP_USERNAME_FIELD for anything else except user_dn?
(no offence intended) :-)
I think there was once a feature request for constucting the username like that but I have to look it up
I just checked and couldn't find such a feature request. But since there still is the possibility that someone uses this I think we should stick with the current version for 1.2 and apply this pr for 2.x
I agree with @Skywalker-11 : this pr should be rebased on develop and applied logic moved to MailWatch\Ldap class