Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.When using bind_method==LDAP_SERVERS_BIND_METHOD_USER, the dn and password are already sent (and checked) during the initial non-anonymous bind, but later in the code, when it comes time to check the password for the anonymous method, the password is checked again either way. The unnecessary network round-trip to the server can be eliminated thus:
In file modules/ldap/ldap_authentication/ldap_authentication.inc around line 364:
$credentials_pass = FALSE;
if ($sso_login) {
/* [...] */
$credentials_pass = (boolean)($ldap_user);
}
+ elseif ($ldap_server->bind_method == LDAP_SERVERS_BIND_METHOD_USER) {
+ // With user bind method, the only way we can reach this part of the code is when the pw
+ // has already been checked and $ldap_user could be loaded, so we're good to go.
+ $credentials_pass = true;
+ }
else {
$credentials_pass = ($ldap_server->bind($ldap_user['dn'], $pass) == LDAP_SUCCESS);
}
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | password_unnecessarily-1885710-4.patch | 849 bytes | humansky |
Comments
Comment #1
johnbarclay CreditAttribution: johnbarclay commentedComment #2
johnbarclay CreditAttribution: johnbarclay commentedThis looks good, but I don't want to break anything at this point, so am postponing to version 8.
Comment #3
johnbarclay CreditAttribution: johnbarclay commentedComment #4
humansky CreditAttribution: humansky as a volunteer commentedThank you @alexanderperlis, this worked like a charm. My work's LDAP server would refuse to bind multiple times during a single connection. I used your patch and now it works. I've attached your code in patch form. All credit goes to @alexanderperlis.
Patch works for latest 7.x dev release.
Comment #5
queenvictoria CreditAttribution: queenvictoria at Holly commentedTO CHECK does this occur in the current branch?
Comment #6
grahlComment #7
grahlComment #8
notmike CreditAttribution: notmike commented@grahl The patch in #4 is still needed for the 7.x branch (yesterday, we successfully applied it to 7.x-2.0-beta12 on a production site). What is the protocol for re-opening this issue? The issue was switched from the 7.x version to the 8.x version awhile back, but the patch is only for the 7.x branch (the ldap_authentication.inc file doesn't exist in the 8.x branch).
Comment #9
grahlHi @notmike
Thanks for your feedback. The policy on reopening is to reopen if the problem isn't solved yet :-)
Comment #13
grahlThe patch to 8 differs a bit in its implementation, would be great to get feedback from others on this though in my limited testing it worked as expected.