Skip to content

Add layer on top of LDAP methods to catch server disconnects and attempt re-connection #4992

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bline wants to merge 4 commits into from
Closed

Add layer on top of LDAP methods to catch server disconnects and attempt re-connection #4992

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ab209cc
add layer on top of ldap methods to catch server disconnects and atte…
bline May 20, 2017
a92e08c
special case for controlPagedResultResponse. It would be nice if ther…
bline May 20, 2017
69d447e
merged
bline May 20, 2017
77141c7
moved to something a little less invasive. back to passing CR around.
bline May 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
special case for controlPagedResultResponse. It would be nice if ther… 
…e was a generic way to pass by reference with call_user_func_array..
  • Loading branch information
@bline
bline committed May 20, 2017
commit a92e08c89368d3a47db534be4437a001bd24f124
111 changes: 78 additions & 33 deletions apps/user_ldap/lib/Access.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
use OCA\User_LDAP\User\OfflineUser;
use OCA\User_LDAP\Mapping\AbstractMapping;

use OC\ServerNotAvailableException;

/**
* Class Access
* @package OCA\User_LDAP
Expand Down Expand Up @@ -167,12 +169,6 @@ public function readAttribute($dn, $attr, $filter = 'objectClass=*') {
\OCP\Util::WARN);
return false;
}
$cr = $this->connection->getConnectionResource();
if(!$this->ldap->isResource($cr)) {
//LDAP not available
\OCP\Util::writeLog('user_ldap', 'LDAP resource not available.', \OCP\Util::DEBUG);
return false;
}
//Cancel possibly running Paged Results operation, otherwise we run in
//LDAP protocol errors
$this->abandonPagedSearch();
Expand All @@ -190,7 +186,7 @@ public function readAttribute($dn, $attr, $filter = 'objectClass=*') {
$values = [];
$isRangeRequest = false;
do {
$result = $this->executeRead($cr, $dn, $attrToRead, $filter, $maxResults);
$result = $this->executeRead($dn, $attrToRead, $filter, $maxResults);
if(is_bool($result)) {
// when an exists request was run and it was successful, an empty
// array must be returned
Expand Down Expand Up @@ -232,7 +228,6 @@ public function readAttribute($dn, $attr, $filter = 'objectClass=*') {
/**
* Runs an read operation against LDAP
*
* @param resource $cr the LDAP connection
* @param string $dn
* @param string $attribute
* @param string $filter
Expand All @@ -241,10 +236,10 @@ public function readAttribute($dn, $attr, $filter = 'objectClass=*') {
* was performed and the requested DN found, array with the
* returned data on a successful usual operation
*/
public function executeRead($cr, $dn, $attribute, $filter, $maxResults) {
public function executeRead($dn, $attribute, $filter, $maxResults) {
$this->initPagedSearch($filter, array($dn), array($attribute), $maxResults, 0);
$dn = $this->helper->DNasBaseParameter($dn);
$rr = @$this->ldap->read($cr, $dn, $filter, array($attribute));
$rr = @$this->invokeLDAPMethod('read', $dn, $filter, array($attribute));
if (!$this->ldap->isResource($rr)) {
if ($attribute !== '') {
//do not throw this message on userExists check, irritates
Expand All @@ -253,18 +248,18 @@ public function executeRead($cr, $dn, $attribute, $filter, $maxResults) {
//in case an error occurs , e.g. object does not exist
return false;
}
if ($attribute === '' && ($filter === 'objectclass=*' || $this->ldap->countEntries($cr, $rr) === 1)) {
if ($attribute === '' && ($filter === 'objectclass=*' || $this->invokeLDAPMethod('countEntries', $rr) === 1)) {
\OCP\Util::writeLog('user_ldap', 'readAttribute: ' . $dn . ' found', \OCP\Util::DEBUG);
return true;
}
$er = $this->ldap->firstEntry($cr, $rr);
$er = $this->invokeLDAPMethod('firstEntry', $rr);
if (!$this->ldap->isResource($er)) {
//did not match the filter, return false
return false;
}
//LDAP attributes are not case sensitive
$result = \OCP\Util::mb_array_change_key_case(
$this->ldap->getAttributes($cr, $er), MB_CASE_LOWER, 'UTF-8');
$this->invokeLDAPMethod('getAttributes', $er), MB_CASE_LOWER, 'UTF-8');

return $result;
}
Expand Down Expand Up @@ -337,15 +332,8 @@ public function setPassword($userDN, $password) {
if(intval($this->connection->turnOnPasswordChange) !== 1) {
throw new \Exception('LDAP password changes are disabled.');
}
$cr = $this->connection->getConnectionResource();
if(!$this->ldap->isResource($cr)) {
//LDAP not available
\OCP\Util::writeLog('user_ldap', 'LDAP resource not available.', \OCP\Util::DEBUG);
return false;
}

try {
return $this->ldap->modReplace($cr, $userDN, $password);
return $this->invokeLDAPMethod('modReplace', $userDN, $password);
} catch(ConstraintViolationException $e) {
throw new HintException('Password change rejected.', \OC::$server->getL10N('user_ldap')->t('Password change rejected. Hint: ').$e->getMessage(), $e->getCode());
}
Expand Down Expand Up @@ -937,6 +925,52 @@ public function countObjects($limit = null, $offset = null) {
return $this->count('objectclass=*', $this->connection->ldapBase, array('dn'), $limit, $offset);
}

/**
* @return mixed
*/
private function invokeLDAPMethod() {
$arguments = func_get_args();
$command = array_shift($arguments);
if (!method_exists($this->ldap, $command)) {
return null;
}
$cr = $this->connection->getConnectionResource();
if(!$this->ldap->isResource($cr)) {
// Seems like we didn't find any resource.
\OCP\Util::writeLog('user_ldap', "Could not $command, because resource is missing.", \OCP\Util::DEBUG);
return false;
}
array_unshift($arguments, $cr);
// php no longer supports call-time pass-by-reference
// make special case for controlPagedResultResponse as the third argument is a reference
$doMethod = function () use ($command, &$arguments) {
if ($command == 'controlPagedResultResponse') {
return $this->ldap->controlPagedResultResponse($arguments[0], $arguments[1], $arguments[2]);
} else {
return call_user_func_array(array($this->ldap, $command), $arguments);
}
};
try {
$ret = $doMethod();
} catch (ServerNotAvailableException $e) {
/* Server connection lost, attempt to reestablish it
* Maybe implement exponential backoff?
* This was enough to get solr indexer working which has large delays between LDAP fetches.
*/
\OCP\Util::writeLog('user_ldap', "Connection lost on $command, attempting to reestablish.", \OCP\Util::DEBUG);
$this->connection = clone $this->connection;
$cr = $this->connection->getConnectionResource();
if(!$this->ldap->isResource($cr)) {
// Seems like we didn't find any resource.
\OCP\Util::writeLog('user_ldap', "Could not $command, because resource is missing.", \OCP\Util::DEBUG);
return false;
Copy link
Member

@blizzz blizzz May 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throw $e instead

}
$arguments[0] = $cr;
$ret = $doMethod();
}
return $ret;
}

/**
* retrieved. Results will according to the order in the array.
* @param int $limit optional, maximum results to be counted
Expand All @@ -962,7 +996,23 @@ private function executeSearch($filter, $base, &$attr = null, $limit = null, $of
$pagedSearchOK = $this->initPagedSearch($filter, $base, $attr, intval($limit), $offset);

$linkResources = array_pad(array(), count($base), $cr);
$sr = $this->ldap->search($linkResources, $base, $filter, $attr);
try {
$sr = $this->ldap->search($linkResources, $base, $filter, $attr);
} catch (ServerNotAvailableException $e) {
/* Server connection lost, attempt to reestablish it
* According to MS docs, LDAP cookies survive reconnects
*/
\OCP\Util::writeLog('user_ldap', "Connection lost on search, attempting to reestablish.", \OCP\Util::DEBUG);
$this->connection = clone $this->connection;
$cr = $this->connection->getConnectionResource();
if(!$this->ldap->isResource($cr)) {
// Seems like we didn't find any resource.
\OCP\Util::writeLog('user_ldap', "Could not search, because resource is missing.", \OCP\Util::DEBUG);
return false;
}
$linkResources = array_pad(array(), count($base), $cr);
$sr = $this->ldap->search($linkResources, $base, $filter, $attr);
}
$error = $this->ldap->errno($cr);
if(!is_array($sr) || $error !== 0) {
\OCP\Util::writeLog('user_ldap', 'Attempt for Paging? '.print_r($pagedSearchOK, true), \OCP\Util::ERROR);
Expand Down Expand Up @@ -990,7 +1040,7 @@ private function processPagedSearchStatus($sr, $filter, $base, $iFoundItems, $li
if($pagedSearchOK) {
$cr = $this->connection->getConnectionResource();
foreach($sr as $key => $res) {
if($this->ldap->controlPagedResultResponse($cr, $res, $cookie)) {
if($this->invokeLDAPMethod('controlPagedResultResponse', $res, $cookie)) {
$this->setPagedResultCookie($base[$key], $filter, $limit, $offset, $cookie);
}
}
Expand Down Expand Up @@ -1079,7 +1129,7 @@ private function countEntriesInSearchResults($searchResults) {
$counter = 0;

foreach($searchResults as $res) {
$count = intval($this->ldap->countEntries($cr, $res));
$count = intval($this->invokeLDAPMethod('countEntries', $res));
$counter += $count;
}

Expand Down Expand Up @@ -1129,7 +1179,7 @@ public function search($filter, $base, $attr = null, $limit = null, $offset = nu
}

foreach($sr as $res) {
$findings = array_merge($findings, $this->ldap->getEntries($cr , $res ));
$findings = array_merge($findings, $this->invokeLDAPMethod('getEntries', $res));
}

$continue = $this->processPagedSearchStatus($sr, $filter, $base, $findings['count'],
Expand Down Expand Up @@ -1691,8 +1741,7 @@ public function isDNPartOfBase($dn, $bases) {
*/
private function abandonPagedSearch() {
if($this->connection->hasPagedResultSupport) {
$cr = $this->connection->getConnectionResource();
$this->ldap->controlPagedResult($cr, 0, false, $this->lastCookie);
$this->invokeLDAPMethod('controlPagedResult', 0, false, $this->lastCookie);
$this->getPagedSearchResultState();
$this->lastCookie = '';
$this->cookies = array();
Expand Down Expand Up @@ -1818,9 +1867,7 @@ private function initPagedSearch($filter, $bases, $attr, $limit, $offset) {
if(!is_null($cookie)) {
//since offset = 0, this is a new search. We abandon other searches that might be ongoing.
$this->abandonPagedSearch();
$pagedSearchOK = $this->ldap->controlPagedResult(
$this->connection->getConnectionResource(), $limit,
false, $cookie);
$pagedSearchOK = $this->invokeLDAPMethod('controlPagedResult', $limit, false, $cookie);
if(!$pagedSearchOK) {
return false;
}
Expand All @@ -1846,9 +1893,7 @@ private function initPagedSearch($filter, $bases, $attr, $limit, $offset) {
// in case someone set it to 0 … use 500, otherwise no results will
// be returned.
$pageSize = intval($this->connection->ldapPagingSize) > 0 ? intval($this->connection->ldapPagingSize) : 500;
$pagedSearchOK = $this->ldap->controlPagedResult(
$this->connection->getConnectionResource(), $pageSize, false, ''
);
$pagedSearchOK = $this->invokeLDAPMethod('controlPagedResult', $pageSize, false, '');
}

return $pagedSearchOK;
Expand Down