Skip to content

Commit

Permalink
Cache LDAP connections: MDL-18130 Properly handle open LDAP connections.
Browse files Browse the repository at this point in the history
Both CAS and LDAP auth plugins open new connections to the LDAP server
to get the user account details. While this is the desired behaviour
for regular logins (we probably don't have an already open connection
to the LDAP server), this is a ressource hog when we are doing user
synchronization, as the closed connections remain in the TCP_WAIT
state for a while before the server can reuse them. If we are syncing
a lot of users, we can make the server run out of available TCP
ressources.

So we cache the connection the first time we establish it and return
the same connection handle everytime, unless we've closed all the
'open' connections, or the auth object is destroyed.

In addition to that, there were a few missing calls to ldap_close().
  • Loading branch information
iarenaza committed Feb 15, 2009
1 parent a33b386 commit b743169
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 13 deletions.
33 changes: 31 additions & 2 deletions auth/cas/auth.php
Expand Up @@ -405,7 +405,7 @@ function get_userinfo($username) {
$result[$key] = $ldapval; $result[$key] = $ldapval;
} }
} }
@ldap_close($ldapconnection); $this->ldap_close($ldapconnection);
return $result; return $result;
} }
/** /**
Expand Down Expand Up @@ -435,6 +435,16 @@ function get_userinfo_asobj($username) {
* @return connection result * @return connection result
*/ */
function ldap_connect($binddn='',$bindpwd='') { function ldap_connect($binddn='',$bindpwd='') {
// Cache ldap connections (they are expensive to set up
// and can drain the TCP/IP ressources on the server if we
// are syncing a lot of users (as we try to open a new connection
// to get the user details). This is the least invasive way
// to reuse existing connections without greater code surgery.
if(!empty($this->ldapconnection)) {
$this->ldapconns++;
return $this->ldapconnection;
}

//Select bind password, With empty values use //Select bind password, With empty values use
//ldap_bind_* variables or anonymous bind if ldap_bind_* are empty //ldap_bind_* variables or anonymous bind if ldap_bind_* are empty
if ($binddn == '' and $bindpwd == '') { if ($binddn == '' and $bindpwd == '') {
Expand Down Expand Up @@ -469,6 +479,10 @@ function ldap_connect($binddn='',$bindpwd='') {
ldap_set_option($connresult, LDAP_OPT_DEREF, $this->config->opt_deref); ldap_set_option($connresult, LDAP_OPT_DEREF, $this->config->opt_deref);
} }
if ($bindresult) { if ($bindresult) {
// Set the connection counter so we can call PHP's ldap_close()
// when we call $this->ldap_close() for the last 'open' connection.
$this->ldapconns = 1;
$this->ldapconnection = $connresult;
return $connresult; return $connresult;
} }
$debuginfo .= "<br/>Server: '$server' <br/> Connection: '$connresult'<br/> Bind result: '$bindresult'</br>"; $debuginfo .= "<br/>Server: '$server' <br/> Connection: '$connresult'<br/> Bind result: '$bindresult'</br>";
Expand All @@ -477,6 +491,18 @@ function ldap_connect($binddn='',$bindpwd='') {
print_error('auth_ldap_noconnect_all','auth',$this->config->user_type); print_error('auth_ldap_noconnect_all','auth',$this->config->user_type);
return false; return false;
} }
/**
* disconnects from a ldap server
*
*/
function ldap_close() {
$this->ldapconns--;
if($this->ldapconns == 0) {
@ldap_close($this->ldapconnection);
unset($this->ldapconnection);
}
}

/** /**
* retuns user attribute mappings between moodle and ldap * retuns user attribute mappings between moodle and ldap
* *
Expand Down Expand Up @@ -629,7 +655,7 @@ function sync_users ($bulk_insert_records = 1000, $do_updates = true) {
print "Connecting to ldap...\n"; print "Connecting to ldap...\n";
$ldapconnection = $this->ldap_connect(); $ldapconnection = $this->ldap_connect();
if (!$ldapconnection) { if (!$ldapconnection) {
@ldap_close($ldapconnection); $this->ldap_close($ldapconnection);
print get_string('auth_ldap_noconnect','auth',$this->config->host_url); print get_string('auth_ldap_noconnect','auth',$this->config->host_url);
exit; exit;
} }
Expand Down Expand Up @@ -878,6 +904,7 @@ function sync_users ($bulk_insert_records = 1000, $do_updates = true) {
} else { } else {
print "No users to be added\n"; print "No users to be added\n";
} }
$this->ldap_close();
return true; return true;
} }
/** /**
Expand Down Expand Up @@ -1016,6 +1043,7 @@ function ldap_isgroupmember($extusername='', $groupdns='') {
} }
} }
} }
$this->ldap_close();
return $result; return $result;
} }
/** /**
Expand Down Expand Up @@ -1055,6 +1083,7 @@ function ldap_get_userlist($filter="*") {
array_push($fresult, ($users[$i][$this->config->user_attribute][0]) ); array_push($fresult, ($users[$i][$this->config->user_attribute][0]) );
} }
} }
$this->ldap_close();
return $fresult; return $fresult;
} }
/** /**
Expand Down
49 changes: 38 additions & 11 deletions auth/ldap/auth.php
Expand Up @@ -109,19 +109,19 @@ function user_login($username, $password) {


//if ldap_user_dn is empty, user does not exist //if ldap_user_dn is empty, user does not exist
if (!$ldap_user_dn) { if (!$ldap_user_dn) {
ldap_close($ldapconnection); $this->ldap_close();
return false; return false;
} }


// Try to bind with current username and password // Try to bind with current username and password
$ldap_login = @ldap_bind($ldapconnection, $ldap_user_dn, $extpassword); $ldap_login = @ldap_bind($ldapconnection, $ldap_user_dn, $extpassword);
ldap_close($ldapconnection); $this->ldap_close();
if ($ldap_login) { if ($ldap_login) {
return true; return true;
} }
} }
else { else {
@ldap_close($ldapconnection); $this->ldap_close();
print_error('auth_ldap_noconnect','auth','',$this->config->host_url); print_error('auth_ldap_noconnect','auth','',$this->config->host_url);
} }
return false; return false;
Expand Down Expand Up @@ -195,7 +195,7 @@ function get_userinfo($username) {
} }
} }


@ldap_close($ldapconnection); $this->ldap_close();
return $result; return $result;
} }


Expand Down Expand Up @@ -301,7 +301,7 @@ function user_create($userobject, $plainpass) {
print_error('auth_ldap_unsupportedusertype','auth','',$this->config->user_type); print_error('auth_ldap_unsupportedusertype','auth','',$this->config->user_type);
} }
$uadd = ldap_add($ldapconnection, $this->config->user_attribute.'="'.$this->ldap_addslashes($userobject->username).','.$this->config->create_context.'"', $newuser); $uadd = ldap_add($ldapconnection, $this->config->user_attribute.'="'.$this->ldap_addslashes($userobject->username).','.$this->config->create_context.'"', $newuser);
ldap_close($ldapconnection); $this->ldap_close();
return $uadd; return $uadd;


} }
Expand Down Expand Up @@ -416,7 +416,7 @@ function sync_users ($bulk_insert_records = 1000, $do_updates = true) {
$ldapconnection = $this->ldap_connect(); $ldapconnection = $this->ldap_connect();


if (!$ldapconnection) { if (!$ldapconnection) {
@ldap_close($ldapconnection); $this->ldap_close();
print get_string('auth_ldap_noconnect','auth',$this->config->host_url); print get_string('auth_ldap_noconnect','auth',$this->config->host_url);
exit; exit;
} }
Expand Down Expand Up @@ -700,6 +700,7 @@ function sync_users ($bulk_insert_records = 1000, $do_updates = true) {
} else { } else {
print "No users to be added\n"; print "No users to be added\n";
} }
$this->ldap_close();
return true; return true;
} }


Expand Down Expand Up @@ -794,7 +795,7 @@ function user_activate($username) {
error ('auth: ldap user_activate() does not support selected usertype:"'.$this->config->user_type.'" (..yet)'); error ('auth: ldap user_activate() does not support selected usertype:"'.$this->config->user_type.'" (..yet)');
} }
$result = ldap_modify($ldapconnection, $userdn, $newinfo); $result = ldap_modify($ldapconnection, $userdn, $newinfo);
ldap_close($ldapconnection); $this->ldap_close();
return $result; return $result;
} }


Expand All @@ -819,7 +820,7 @@ function user_activate($username) {
error ('auth: ldap user_disable() does not support selected usertype (..yet)'); error ('auth: ldap user_disable() does not support selected usertype (..yet)');
} }
$result = ldap_modify($ldapconnection, $userdn, $newinfo); $result = ldap_modify($ldapconnection, $userdn, $newinfo);
ldap_close($ldapconnection); $this->ldap_close();
return $result; return $result;
}*/ }*/


Expand Down Expand Up @@ -1004,11 +1005,11 @@ function user_update($olduser, $newuser) {
} }
} else { } else {
error_log("ERROR:No user found in LDAP"); error_log("ERROR:No user found in LDAP");
@ldap_close($ldapconnection); $this->ldap_close();
return false; return false;
} }


@ldap_close($ldapconnection); $this->ldap_close();


return true; return true;


Expand Down Expand Up @@ -1132,7 +1133,7 @@ function user_update_password($user, $newpassword) {


} }


@ldap_close($ldapconnection); $this->ldap_close();
return $result; return $result;
} }


Expand Down Expand Up @@ -1365,6 +1366,16 @@ function ldap_isgroupmember($extusername='', $groupdns='') {
* @return connection result * @return connection result
*/ */
function ldap_connect($binddn='',$bindpwd='') { function ldap_connect($binddn='',$bindpwd='') {
// Cache ldap connections (they are expensive to set up
// and can drain the TCP/IP ressources on the server if we
// are syncing a lot of users (as we try to open a new connection
// to get the user details). This is the least invasive way
// to reuse existing connections without greater code surgery.
if(!empty($this->ldapconnection)) {
$this->ldapconns++;
return $this->ldapconnection;
}

//Select bind password, With empty values use //Select bind password, With empty values use
//ldap_bind_* variables or anonymous bind if ldap_bind_* are empty //ldap_bind_* variables or anonymous bind if ldap_bind_* are empty
if ($binddn == '' and $bindpwd == '') { if ($binddn == '' and $bindpwd == '') {
Expand Down Expand Up @@ -1411,6 +1422,10 @@ function ldap_connect($binddn='',$bindpwd='') {
} }


if ($bindresult) { if ($bindresult) {
// Set the connection counter so we can call PHP's ldap_close()
// when we call $this->ldap_close() for the last 'open' connection.
$this->ldapconns = 1;
$this->ldapconnection = $connresult;
return $connresult; return $connresult;
} }


Expand All @@ -1422,6 +1437,18 @@ function ldap_connect($binddn='',$bindpwd='') {
return false; return false;
} }


/**
* disconnects from a ldap server
*
*/
function ldap_close() {
$this->ldapconns--;
if($this->ldapconns == 0) {
@ldap_close($this->ldapconnection);
unset($this->ldapconnection);
}
}

/** /**
* retuns dn of username * retuns dn of username
* *
Expand Down

0 comments on commit b743169

Please sign in to comment.