From 1a020e0696b5b2876044f419c7f02525b0bb9942 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Sun, 18 Aug 2013 13:33:59 +0200 Subject: [PATCH 01/16] Resolve merge conflict --- apps/user_ldap/lib/connection.php | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/apps/user_ldap/lib/connection.php b/apps/user_ldap/lib/connection.php index e5d9b4d5b40..42139274666 100644 --- a/apps/user_ldap/lib/connection.php +++ b/apps/user_ldap/lib/connection.php @@ -23,7 +23,7 @@ namespace OCA\user_ldap\lib; -class Connection { +class Connection extends BackendBase { private $ldapConnectionRes = null; private $configPrefix; private $configID; @@ -78,6 +78,7 @@ class Connection { * @param $configID a string with the value for the appid column (appconfig table) or null for on-the-fly connections */ public function __construct($configPrefix = '', $configID = 'user_ldap') { + parent::__construct(); $this->configPrefix = $configPrefix; $this->configID = $configID; $memcache = new \OC\Memcache\Factory(); @@ -86,13 +87,13 @@ class Connection { } else { $this->cache = \OC_Cache::getGlobalCache(); } - $this->config['hasPagedResultSupport'] = (function_exists('ldap_control_paged_result') - && function_exists('ldap_control_paged_result_response')); + $this->config['hasPagedResultSupport'] = + $this->ldap->hasPagedResultSupport(); } public function __destruct() { if(!$this->dontDestruct && is_resource($this->ldapConnectionRes)) { - @ldap_unbind($this->ldapConnectionRes); + @$this->ldap->unbind($this->ldapConnectionRes); }; } @@ -603,7 +604,7 @@ class Connection { return false; } if(!$this->ldapConnectionRes) { - if(!function_exists('ldap_connect')) { + if(!$this->ldap->areLDAPFunctionsAvailable()) { $phpLDAPinstalled = false; \OCP\Util::writeLog('user_ldap', 'function ldap_connect is not available. Make sure that the PHP ldap module is installed.', @@ -653,11 +654,11 @@ class Connection { //ldap_connect ignores port paramater when URLs are passed $host .= ':' . $port; } - $this->ldapConnectionRes = ldap_connect($host, $port); - if(ldap_set_option($this->ldapConnectionRes, LDAP_OPT_PROTOCOL_VERSION, 3)) { - if(ldap_set_option($this->ldapConnectionRes, LDAP_OPT_REFERRALS, 0)) { + $this->ldapConnectionRes = $this->ldap->connect($host, $port); + if($this->ldap->set_option($this->ldapConnectionRes, LDAP_OPT_PROTOCOL_VERSION, 3)) { + if($this->ldap->set_option($this->ldapConnectionRes, LDAP_OPT_REFERRALS, 0)) { if($this->config['ldapTLS']) { - ldap_start_tls($this->ldapConnectionRes); + $this->ldap->start_tls($this->ldapConnectionRes); } } } @@ -681,10 +682,10 @@ class Connection { if(!is_resource($cr)) { return false; } - $ldapLogin = @ldap_bind($cr, $this->config['ldapAgentName'], $this->config['ldapAgentPassword']); + $ldapLogin = @$this->ldap->bind($cr, $this->config['ldapAgentName'], $this->config['ldapAgentPassword']); if(!$ldapLogin) { \OCP\Util::writeLog('user_ldap', - 'Bind failed: ' . ldap_errno($cr) . ': ' . ldap_error($cr), + 'Bind failed: ' . $this->ldap->errno($cr) . ': ' . $this->ldap->error($cr), \OCP\Util::ERROR); $this->ldapConnectionRes = null; return false; From d34fbf3a8647806779d908b599068b9000d0c4e4 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 12 Jun 2013 02:08:02 +0200 Subject: [PATCH 02/16] LDAP: move PHP LDAP functions calls to an LDAP Wrapper for better isolation and mock testing --- apps/user_ldap/lib/access.php | 22 ++++---- apps/user_ldap/lib/backendbase.php | 50 +++++++++++++++++ apps/user_ldap/lib/ldap.php | 86 ++++++++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 11 deletions(-) create mode 100644 apps/user_ldap/lib/backendbase.php create mode 100644 apps/user_ldap/lib/ldap.php diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 52aa39012fd..d09571a8a66 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -23,7 +23,7 @@ namespace OCA\user_ldap\lib; -abstract class Access { +abstract class Access extends BackendBase { protected $connection; //never ever check this var directly, always use getPagedSearchResultState protected $pagedSearchedSuccessful; @@ -60,7 +60,7 @@ abstract class Access { return false; } $dn = $this->DNasBaseParameter($dn); - $rr = @ldap_read($cr, $dn, $filter, array($attr)); + $rr = @$this->ldap->read($cr, $dn, $filter, array($attr)); if(!is_resource($rr)) { if(!empty($attr)) { //do not throw this message on userExists check, irritates @@ -73,13 +73,13 @@ abstract class Access { \OCP\Util::writeLog('user_ldap', 'readAttribute: '.$dn.' found', \OCP\Util::DEBUG); return array(); } - $er = ldap_first_entry($cr, $rr); + $er = $this->ldap->first_entry($cr, $rr); if(!is_resource($er)) { //did not match the filter, return false return false; } //LDAP attributes are not case sensitive - $result = \OCP\Util::mb_array_change_key_case(ldap_get_attributes($cr, $er), MB_CASE_LOWER, 'UTF-8'); + $result = \OCP\Util::mb_array_change_key_case($this->ldap->get_attributes($cr, $er), MB_CASE_LOWER, 'UTF-8'); $attr = mb_strtolower($attr, 'UTF-8'); if(isset($result[$attr]) && $result[$attr]['count'] > 0) { @@ -664,11 +664,11 @@ abstract class Access { $pagedSearchOK = $this->initPagedSearch($filter, $base, $attr, $limit, $offset); $linkResources = array_pad(array(), count($base), $link_resource); - $sr = ldap_search($linkResources, $base, $filter, $attr); - $error = ldap_errno($link_resource); + $sr = $this->ldap->search($linkResources, $base, $filter, $attr); + $error = $this->ldap->errno($link_resource); if(!is_array($sr) || $error !== 0) { \OCP\Util::writeLog('user_ldap', - 'Error when searching: '.ldap_error($link_resource).' code '.ldap_errno($link_resource), + 'Error when searching: '.$this->ldap->error($link_resource).' code '.$this->ldap->errno($link_resource), \OCP\Util::ERROR); \OCP\Util::writeLog('user_ldap', 'Attempt for Paging? '.print_r($pagedSearchOK, true), \OCP\Util::ERROR); return array(); @@ -677,19 +677,19 @@ abstract class Access { // Do the server-side sorting foreach(array_reverse($attr) as $sortAttr){ foreach($sr as $searchResource) { - ldap_sort($link_resource, $searchResource, $sortAttr); + $this->ldap->sort($link_resource, $searchResource, $sortAttr); } } $findings = array(); foreach($sr as $key => $res) { - $findings = array_merge($findings, ldap_get_entries($link_resource, $res )); + $findings = array_merge($findings, $this->ldap->get_entries($link_resource, $res )); } if($pagedSearchOK) { \OCP\Util::writeLog('user_ldap', 'Paged search successful', \OCP\Util::INFO); foreach($sr as $key => $res) { $cookie = null; - if(ldap_control_paged_result_response($link_resource, $res, $cookie)) { + if($this->ldap->control_paged_result_response($link_resource, $res, $cookie)) { \OCP\Util::writeLog('user_ldap', 'Set paged search cookie', \OCP\Util::INFO); $this->setPagedResultCookie($base[$key], $filter, $limit, $offset, $cookie); } @@ -1103,7 +1103,7 @@ abstract class Access { if($offset > 0) { \OCP\Util::writeLog('user_ldap', 'Cookie '.$cookie, \OCP\Util::INFO); } - $pagedSearchOK = ldap_control_paged_result($this->connection->getConnectionResource(), + $pagedSearchOK = $this->ldap->control_paged_result($this->connection->getConnectionResource(), $limit, false, $cookie); if(!$pagedSearchOK) { return false; diff --git a/apps/user_ldap/lib/backendbase.php b/apps/user_ldap/lib/backendbase.php new file mode 100644 index 00000000000..7ed079b3418 --- /dev/null +++ b/apps/user_ldap/lib/backendbase.php @@ -0,0 +1,50 @@ +. + * + */ + +namespace OCA\user_ldap\lib; + +abstract class BackendBase { + protected $ldap; + + public function __construct() { + $this->ldap = new LDAP(); + } + + /** + * @brief sets the LDAP Wrapper to be used + * + * @param $ldapWrapper an instance of the Wrapper + * @return true on success, otherwise false + * + * The LDAP Wrapper must implement the PHP LDAP functions, which are used + * in the LDAP backend + */ + public function setLDAPWrapper($ldapWrapper) { + if(is_object($ldapWrapper)) { + unset($this->ldap); + $this->ldap = $ldapWrapper; + return true; + } + return false; + } +} \ No newline at end of file diff --git a/apps/user_ldap/lib/ldap.php b/apps/user_ldap/lib/ldap.php new file mode 100644 index 00000000000..86c57c37d8a --- /dev/null +++ b/apps/user_ldap/lib/ldap.php @@ -0,0 +1,86 @@ +. + * + */ + +namespace OCA\user_ldap\lib; + +class LDAP { + protected $curFunc = ''; + protected $curArgs = array(); + + //Simple wrapper for the ldap functions + public function __call($name, $arguments) { + $func = 'ldap_' . $name; + if(function_exists($func)) { + $this->preFunctionCall($func, $arguments); + $result = call_user_func_array($func, $arguments); + $this->postFunctionCall(); + return $result; + } + } + + public function control_paged_result_response($linkResource, $resultResource, &$cookie) { + $this->preFunctionCall('ldap_control_paged_result_response', + array($linkResource, $resultResource, $cookie)); + $result = ldap_control_paged_result_response( + $linkResource, $resultResource, $cookie); + $this->postFunctionCall(); + + return $result; + } + + public function areLDAPFunctionsAvailable() { + return function_exists('ldap_connect'); + } + + public function hasPagedResultSupport() { + $hasSupport = function_exists('ldap_control_paged_result') + && function_exists('ldap_control_paged_result_response'); + return $hasSupport; + } + + private function preFunctionCall($functionName, $args) { + $this->curFunc = $functionName; + $this->curArgs = $args; + } + + private function postFunctionCall() { + if(is_resource($this->curArgs[0])) { + $errorCode = ldap_errno($this->curArgs[0]); + $errorMsg = ldap_error($this->curArgs[0]); + if($errorCode !== 0) { + if($this->curFunc === 'ldap_sort' && $errorCode === -4) { + //You can safely ignore that decoding error. + //… says https://bugs.php.net/bug.php?id=18023 + } else if($this->curFunc === 'ldap_get_entries' && $errorCode === -4) { + } else if ($errorCode === 32) { + //for now + } else { + throw new \Exception('LDAP error '.$errorMsg.' (' .$errorCode.') after calling '.$this->curFunc.' with arguments '.print_r($this->curArgs, true)); + } + } + } + + $this->curFunc = ''; + $this->curArgs = array(); + } +} \ No newline at end of file From 52454e39b7e04869f2cb3253d6978bbef5ece7fc Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 20 Aug 2013 12:39:24 +0200 Subject: [PATCH 03/16] LDAP: move is_resource check to ldap wrapper to make it mockable --- apps/user_ldap/lib/access.php | 8 ++++---- apps/user_ldap/lib/connection.php | 9 +++++---- apps/user_ldap/lib/ldap.php | 10 ++++++++++ 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index d09571a8a66..6cdeff36677 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -54,14 +54,14 @@ abstract class Access extends BackendBase { return false; } $cr = $this->connection->getConnectionResource(); - if(!is_resource($cr)) { + if(!$this->ldap->isResource($cr)) { //LDAP not available \OCP\Util::writeLog('user_ldap', 'LDAP resource not available.', \OCP\Util::DEBUG); return false; } $dn = $this->DNasBaseParameter($dn); $rr = @$this->ldap->read($cr, $dn, $filter, array($attr)); - if(!is_resource($rr)) { + if(!$this->ldap->isResource($rr)) { if(!empty($attr)) { //do not throw this message on userExists check, irritates \OCP\Util::writeLog('user_ldap', 'readAttribute failed for DN '.$dn, \OCP\Util::DEBUG); @@ -74,7 +74,7 @@ abstract class Access extends BackendBase { return array(); } $er = $this->ldap->first_entry($cr, $rr); - if(!is_resource($er)) { + if(!$this->ldap->isResource($er)) { //did not match the filter, return false return false; } @@ -653,7 +653,7 @@ abstract class Access extends BackendBase { // See if we have a resource, in case not cancel with message $link_resource = $this->connection->getConnectionResource(); - if(!is_resource($link_resource)) { + if(!$this->ldap->isResource($link_resource)) { // Seems like we didn't find any resource. // Return an empty array just like before. \OCP\Util::writeLog('user_ldap', 'Could not search, because resource is missing.', \OCP\Util::DEBUG); diff --git a/apps/user_ldap/lib/connection.php b/apps/user_ldap/lib/connection.php index 42139274666..6a0d00405c0 100644 --- a/apps/user_ldap/lib/connection.php +++ b/apps/user_ldap/lib/connection.php @@ -92,7 +92,8 @@ class Connection extends BackendBase { } public function __destruct() { - if(!$this->dontDestruct && is_resource($this->ldapConnectionRes)) { + if(!$this->dontDestruct && + $this->ldap->isResource($this->ldapConnectionRes)) { @$this->ldap->unbind($this->ldapConnectionRes); }; } @@ -149,7 +150,7 @@ class Connection extends BackendBase { public function getConnectionResource() { if(!$this->ldapConnectionRes) { $this->init(); - } else if(!is_resource($this->ldapConnectionRes)) { + } else if(!$this->ldap->isResource($this->ldapConnectionRes)) { $this->ldapConnectionRes = null; $this->establishConnection(); } @@ -624,7 +625,7 @@ class Connection extends BackendBase { if(!$this->config['ldapOverrideMainServer'] && !$this->getFromCache('overrideMainServer')) { $this->doConnect($this->config['ldapHost'], $this->config['ldapPort']); $bindStatus = $this->bind(); - $error = is_resource($this->ldapConnectionRes) ? ldap_errno($this->ldapConnectionRes) : -1; + $error = $this->ldap->isResource($this->ldapConnectionRes) ? ldap_errno($this->ldapConnectionRes) : -1; } else { $bindStatus = false; $error = null; @@ -679,7 +680,7 @@ class Connection extends BackendBase { $getConnectionResourceAttempt = true; $cr = $this->getConnectionResource(); $getConnectionResourceAttempt = false; - if(!is_resource($cr)) { + if(!$this->ldap->isResource($cr)) { return false; } $ldapLogin = @$this->ldap->bind($cr, $this->config['ldapAgentName'], $this->config['ldapAgentPassword']); diff --git a/apps/user_ldap/lib/ldap.php b/apps/user_ldap/lib/ldap.php index 86c57c37d8a..1de901aec12 100644 --- a/apps/user_ldap/lib/ldap.php +++ b/apps/user_ldap/lib/ldap.php @@ -58,6 +58,16 @@ class LDAP { return $hasSupport; } + /** + * Checks whether the submitted parameter is a resource + * + * @param $resource the resource variable to check + * @return true if it is a resource, false otherwise + */ + public function isResource($resource) { + return is_resource($resource); + } + private function preFunctionCall($functionName, $args) { $this->curFunc = $functionName; $this->curArgs = $args; From 6e850e0bee24f1fcea6c59e074d576bee1ec3e5b Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 20 Aug 2013 14:23:49 +0200 Subject: [PATCH 04/16] LDAP: establish wrapper interface to allow proper mocking --- apps/user_ldap/lib/access.php | 13 +- apps/user_ldap/lib/connection.php | 6 +- apps/user_ldap/lib/ildapwrapper.php | 179 ++++++++++++++++++++++++++++ apps/user_ldap/lib/ldap.php | 141 ++++++++++++++++------ 4 files changed, 293 insertions(+), 46 deletions(-) create mode 100644 apps/user_ldap/lib/ildapwrapper.php diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 6cdeff36677..4b7920b7162 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -73,13 +73,13 @@ abstract class Access extends BackendBase { \OCP\Util::writeLog('user_ldap', 'readAttribute: '.$dn.' found', \OCP\Util::DEBUG); return array(); } - $er = $this->ldap->first_entry($cr, $rr); + $er = $this->ldap->firstEntry($cr, $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->get_attributes($cr, $er), MB_CASE_LOWER, 'UTF-8'); + $result = \OCP\Util::mb_array_change_key_case($this->ldap->getAttributes($cr, $er), MB_CASE_LOWER, 'UTF-8'); $attr = mb_strtolower($attr, 'UTF-8'); if(isset($result[$attr]) && $result[$attr]['count'] > 0) { @@ -683,13 +683,13 @@ abstract class Access extends BackendBase { $findings = array(); foreach($sr as $key => $res) { - $findings = array_merge($findings, $this->ldap->get_entries($link_resource, $res )); + $findings = array_merge($findings, $this->ldap->getEntries($link_resource, $res )); } if($pagedSearchOK) { \OCP\Util::writeLog('user_ldap', 'Paged search successful', \OCP\Util::INFO); foreach($sr as $key => $res) { $cookie = null; - if($this->ldap->control_paged_result_response($link_resource, $res, $cookie)) { + if($this->ldap->controlPagedResultResponse($link_resource, $res, $cookie)) { \OCP\Util::writeLog('user_ldap', 'Set paged search cookie', \OCP\Util::INFO); $this->setPagedResultCookie($base[$key], $filter, $limit, $offset, $cookie); } @@ -1103,8 +1103,9 @@ abstract class Access extends BackendBase { if($offset > 0) { \OCP\Util::writeLog('user_ldap', 'Cookie '.$cookie, \OCP\Util::INFO); } - $pagedSearchOK = $this->ldap->control_paged_result($this->connection->getConnectionResource(), - $limit, false, $cookie); + $pagedSearchOK = $this->ldap->controlPagedResult( + $this->connection->getConnectionResource(), $limit, + false, $cookie); if(!$pagedSearchOK) { return false; } diff --git a/apps/user_ldap/lib/connection.php b/apps/user_ldap/lib/connection.php index 6a0d00405c0..6850169f2df 100644 --- a/apps/user_ldap/lib/connection.php +++ b/apps/user_ldap/lib/connection.php @@ -656,10 +656,10 @@ class Connection extends BackendBase { $host .= ':' . $port; } $this->ldapConnectionRes = $this->ldap->connect($host, $port); - if($this->ldap->set_option($this->ldapConnectionRes, LDAP_OPT_PROTOCOL_VERSION, 3)) { - if($this->ldap->set_option($this->ldapConnectionRes, LDAP_OPT_REFERRALS, 0)) { + if($this->ldap->setOption($this->ldapConnectionRes, LDAP_OPT_PROTOCOL_VERSION, 3)) { + if($this->ldap->setOption($this->ldapConnectionRes, LDAP_OPT_REFERRALS, 0)) { if($this->config['ldapTLS']) { - $this->ldap->start_tls($this->ldapConnectionRes); + $this->ldap->startTls($this->ldapConnectionRes); } } } diff --git a/apps/user_ldap/lib/ildapwrapper.php b/apps/user_ldap/lib/ildapwrapper.php new file mode 100644 index 00000000000..56f5cfb4a2d --- /dev/null +++ b/apps/user_ldap/lib/ildapwrapper.php @@ -0,0 +1,179 @@ +. + * + */ + +namespace OCA\user_ldap\lib; + +interface ILDAPWrapper { + + //LDAP functions in use + + /** + * @brief Bind to LDAP directory + * @param $link LDAP link resource + * @param $dn an RDN to log in with + * @param $password the password + * @return true on success, false otherwise + * + * with $dn and $password as null a anonymous bind is attempted. + */ + public function bind($link, $dn, $password); + + /** + * @brief connect to an LDAP server + * @param $host The host to connect to + * @param $port The port to connect to + * @return a link resource on success, otherwise false + */ + public function connect($host, $port); + + /** + * @brief Send LDAP pagination control + * @param $link LDAP link resource + * @param $pagesize number of results per page + * @param $isCritical Indicates whether the pagination is critical of not. + * @param $cookie structure sent by LDAP server + * @return true on success, false otherwise + */ + public function controlPagedResult($link, $pagesize, $isCritical, $cookie); + + /** + * @brief Retrieve the LDAP pagination cookie + * @param $link LDAP link resource + * @param $result LDAP result resource + * @param $cookie structure sent by LDAP server + * + * Corresponds to ldap_control_paged_result_response + */ + public function controlPagedResultResponse($link, $result, &$cookie); + + /** + * @brief Return the LDAP error number of the last LDAP command + * @param $link LDAP link resource + * @return error message as string + */ + public function errno($link); + + /** + * @brief Return the LDAP error message of the last LDAP command + * @param $link LDAP link resource + * @return error code as integer + */ + public function error($link); + + /** + * @brief Return first result id + * @param $link LDAP link resource + * @param $result LDAP result resource + * @return an LDAP search result resource + * */ + public function firstEntry($link, $result); + + /** + * @brief Get attributes from a search result entry + * @param $link LDAP link resource + * @param $result LDAP result resource + * @return array containing the results, false on error + * */ + public function getAttributes($link, $result); + + /** + * @brief Get all result entries + * @param $link LDAP link resource + * @param $result LDAP result resource + * @return array containing the results, false on error + */ + public function getEntries($link, $result); + + /** + * @brief Read an entry + * @param $link LDAP link resource + * @param $baseDN The DN of the entry to read from + * @param $filter An LDAP filter + * @param $attr array of the attributes to read + * @return an LDAP search result resource + */ + public function read($link, $baseDN, $filter, $attr); + + /** + * @brief Search LDAP tree + * @param $link LDAP link resource + * @param $baseDN The DN of the entry to read from + * @param $filter An LDAP filter + * @param $attr array of the attributes to read + * @return an LDAP search result resource, false on error + */ + public function search($link, $baseDN, $filter, $attr); + + /** + * @brief Sets the value of the specified option to be $value + * @param $link LDAP link resource + * @param $option a defined LDAP Server option + * @param $value the new value for the option + * @return true on success, false otherwise + */ + public function setOption($link, $option, $value); + + /** + * @brief establish Start TLS + * @param $link LDAP link resource + * @return true on success, false otherwise + */ + public function startTls($link); + + /** + * @brief Sort the result of a LDAP search + * @param $link LDAP link resource + * @param $result LDAP result resource + * @param $sortfilter attribute to use a key in sort + */ + public function sort($link, $result, $sortfilter); + + /** + * @brief Unbind from LDAP directory + * @param $link LDAP link resource + * @return true on success, false otherwise + */ + public function unbind($link); + + //additional required methods in owncloud + + /** + * @brief Checks whether the server supports LDAP + * @return true if it the case, false otherwise + * */ + public function areLDAPFunctionsAvailable(); + + /** + * @brief Checks whether PHP supports LDAP Paged Results + * @return true if it the case, false otherwise + * */ + public function hasPagedResultSupport(); + + /** + * @brief Checks whether the submitted parameter is a resource + * @param $resource the resource variable to check + * @return true if it is a resource, false otherwise + */ + public function isResource($resource); + +} \ No newline at end of file diff --git a/apps/user_ldap/lib/ldap.php b/apps/user_ldap/lib/ldap.php index 1de901aec12..0f082147b14 100644 --- a/apps/user_ldap/lib/ldap.php +++ b/apps/user_ldap/lib/ldap.php @@ -23,13 +23,107 @@ namespace OCA\user_ldap\lib; -class LDAP { +class LDAP implements ILDAPWrapper { protected $curFunc = ''; protected $curArgs = array(); - //Simple wrapper for the ldap functions - public function __call($name, $arguments) { - $func = 'ldap_' . $name; + public function bind($link, $dn, $password) { + return $this->invokeLDAPMethod('bind', $link, $dn, $password); + } + + public function connect($host, $port) { + return $this->invokeLDAPMethod('connect', $host, $port); + } + + public function controlPagedResultResponse($link, $result, &$cookie) { + $this->preFunctionCall('ldap_control_paged_result_response', + array($link, $result, $cookie)); + $result = ldap_control_paged_result_response($link, $result, $cookie); + $this->postFunctionCall(); + + return $result; + } + + public function controlPagedResult($link, $pagesize, $isCritical, $cookie) { + return $this->invokeLDAPMethod('control_paged_result', $link, $pagesize, + $isCritical, $cookie); + } + + public function errno($link) { + return $this->invokeLDAPMethod('errno', $link); + } + + public function error($link) { + return $this->invokeLDAPMethod('error', $link); + } + + public function firstEntry($link, $result) { + return $this->invokeLDAPMethod('first_entry', $link, $result); + } + + public function getAttributes($link, $result) { + return $this->invokeLDAPMethod('get_attributes', $link, $result); + } + + public function getEntries($link, $result) { + return $this->invokeLDAPMethod('get_entries', $link, $result); + } + + public function read($link, $baseDN, $filter, $attr) { + return $this->invokeLDAPMethod('read', $link, $baseDN, $filter, $attr); + } + + public function search($link, $baseDN, $filter, $attr) { + return $this->invokeLDAPMethod('search', $link, $baseDN, + $filter, $attr); + } + + public function setOption($link, $option, $value) { + $this->invokeLDAPMethod('set_option', $link, $option, $value); + } + + public function sort($link, $result, $sortfilter) { + return $this->invokeLDAPMethod('sort', $link, $result, $sortfilter); + } + + public function startTls($link) { + return $this->invokeLDAPMethod('start_tls', $link); + } + + public function unbind($link) { + return $this->invokeLDAPMethod('unbind', $link); + } + + /** + * @brief Checks whether the server supports LDAP + * @return true if it the case, false otherwise + * */ + public function areLDAPFunctionsAvailable() { + return function_exists('ldap_connect'); + } + + /** + * @brief Checks whether PHP supports LDAP Paged Results + * @return true if it the case, false otherwise + * */ + public function hasPagedResultSupport() { + $hasSupport = function_exists('ldap_control_paged_result') + && function_exists('ldap_control_paged_result_response'); + return $hasSupport; + } + + /** + * @brief Checks whether the submitted parameter is a resource + * @param $resource the resource variable to check + * @return true if it is a resource, false otherwise + */ + public function isResource($resource) { + return is_resource($resource); + } + + private function invokeLDAPMethod() { + $arguments = func_get_args(); + $func = 'ldap_' . array_shift($arguments); if(function_exists($func)) { $this->preFunctionCall($func, $arguments); $result = call_user_func_array($func, $arguments); @@ -38,54 +132,27 @@ class LDAP { } } - public function control_paged_result_response($linkResource, $resultResource, &$cookie) { - $this->preFunctionCall('ldap_control_paged_result_response', - array($linkResource, $resultResource, $cookie)); - $result = ldap_control_paged_result_response( - $linkResource, $resultResource, $cookie); - $this->postFunctionCall(); - - return $result; - } - - public function areLDAPFunctionsAvailable() { - return function_exists('ldap_connect'); - } - - public function hasPagedResultSupport() { - $hasSupport = function_exists('ldap_control_paged_result') - && function_exists('ldap_control_paged_result_response'); - return $hasSupport; - } - - /** - * Checks whether the submitted parameter is a resource - * - * @param $resource the resource variable to check - * @return true if it is a resource, false otherwise - */ - public function isResource($resource) { - return is_resource($resource); - } - private function preFunctionCall($functionName, $args) { $this->curFunc = $functionName; $this->curArgs = $args; } private function postFunctionCall() { - if(is_resource($this->curArgs[0])) { + if($this->isResource($this->curArgs[0])) { $errorCode = ldap_errno($this->curArgs[0]); $errorMsg = ldap_error($this->curArgs[0]); if($errorCode !== 0) { if($this->curFunc === 'ldap_sort' && $errorCode === -4) { //You can safely ignore that decoding error. //… says https://bugs.php.net/bug.php?id=18023 - } else if($this->curFunc === 'ldap_get_entries' && $errorCode === -4) { + } else if($this->curFunc === 'ldap_get_entries' + && $errorCode === -4) { } else if ($errorCode === 32) { //for now } else { - throw new \Exception('LDAP error '.$errorMsg.' (' .$errorCode.') after calling '.$this->curFunc.' with arguments '.print_r($this->curArgs, true)); + throw new \Exception('LDAP error '.$errorMsg.' (' . + $errorCode.') after calling '.$this->curFunc. + ' with arguments '.print_r($this->curArgs, true)); } } } From 5090ca3eb472b4d6b14aa6d7034cb6fc6f688649 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 21 Aug 2013 23:59:39 +0200 Subject: [PATCH 05/16] LDAP: add phpdoc --- apps/user_ldap/lib/ildapwrapper.php | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/user_ldap/lib/ildapwrapper.php b/apps/user_ldap/lib/ildapwrapper.php index 56f5cfb4a2d..be35cebe945 100644 --- a/apps/user_ldap/lib/ildapwrapper.php +++ b/apps/user_ldap/lib/ildapwrapper.php @@ -61,6 +61,7 @@ interface ILDAPWrapper { * @param $link LDAP link resource * @param $result LDAP result resource * @param $cookie structure sent by LDAP server + * @return true on success, false otherwise * * Corresponds to ldap_control_paged_result_response */ From b9cd22cf7845e6192206c2f997123ba3a0bb098e Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 6 Sep 2013 19:31:41 +0200 Subject: [PATCH 06/16] LDAP: use wrapper, not direct function call --- apps/user_ldap/lib/connection.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user_ldap/lib/connection.php b/apps/user_ldap/lib/connection.php index 6850169f2df..dd627a4e1b1 100644 --- a/apps/user_ldap/lib/connection.php +++ b/apps/user_ldap/lib/connection.php @@ -625,7 +625,7 @@ class Connection extends BackendBase { if(!$this->config['ldapOverrideMainServer'] && !$this->getFromCache('overrideMainServer')) { $this->doConnect($this->config['ldapHost'], $this->config['ldapPort']); $bindStatus = $this->bind(); - $error = $this->ldap->isResource($this->ldapConnectionRes) ? ldap_errno($this->ldapConnectionRes) : -1; + $error = $this->ldap->isResource($this->ldapConnectionRes) ? $this->ldap->errno($this->ldapConnectionRes) : -1; } else { $bindStatus = false; $error = null; From f04aa1af5d53759dfc838057a5289106489c4a25 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 6 Sep 2013 20:52:26 +0200 Subject: [PATCH 07/16] LDAP: don't check var type when you can specify the type in the parameter list --- apps/user_ldap/lib/backendbase.php | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/apps/user_ldap/lib/backendbase.php b/apps/user_ldap/lib/backendbase.php index 7ed079b3418..5ed73a0a5b1 100644 --- a/apps/user_ldap/lib/backendbase.php +++ b/apps/user_ldap/lib/backendbase.php @@ -39,12 +39,8 @@ abstract class BackendBase { * The LDAP Wrapper must implement the PHP LDAP functions, which are used * in the LDAP backend */ - public function setLDAPWrapper($ldapWrapper) { - if(is_object($ldapWrapper)) { - unset($this->ldap); - $this->ldap = $ldapWrapper; - return true; - } - return false; + public function setLDAPWrapper(ILDAPWrapper $ldapWrapper) { + $this->ldap = $ldapWrapper; + return true; } } \ No newline at end of file From b9cc2ad660fb7a43f2b0a12e1290527fcebb995a Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 9 Sep 2013 23:19:05 +0200 Subject: [PATCH 08/16] Whitespaces --- apps/user_ldap/user_ldap.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php index 850ca0df995..ab3b57001b6 100644 --- a/apps/user_ldap/user_ldap.php +++ b/apps/user_ldap/user_ldap.php @@ -144,14 +144,12 @@ class USER_LDAP extends lib\Access implements \OCP\UserInterface { if($this->connection->isCached('userExists'.$uid)) { return $this->connection->getFromCache('userExists'.$uid); } - //getting dn, if false the user does not exist. If dn, he may be mapped only, requires more checking. $dn = $this->username2dn($uid); if(!$dn) { $this->connection->writeToCache('userExists'.$uid, false); return false; } - //check if user really still exists by reading its entry if(!is_array($this->readAttribute($dn, ''))) { $this->connection->writeToCache('userExists'.$uid, false); From d4f92494a23747af545d7ebb70eaf3e417f46eaa Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 10 Sep 2013 17:11:02 +0200 Subject: [PATCH 09/16] LDAP: make Access be a dependency to the user and group backend instead of inheriting it. --- apps/user_ldap/ajax/getConfiguration.php | 3 +- apps/user_ldap/ajax/setConfiguration.php | 3 +- apps/user_ldap/ajax/testConfiguration.php | 3 +- apps/user_ldap/appinfo/app.php | 14 +- apps/user_ldap/group_ldap.php | 129 +++++++++--------- apps/user_ldap/group_proxy.php | 10 +- apps/user_ldap/lib/access.php | 7 +- apps/user_ldap/lib/backendutility.php | 38 ++++++ apps/user_ldap/lib/connection.php | 22 +-- apps/user_ldap/lib/jobs.php | 11 +- apps/user_ldap/lib/ldap.php | 2 + .../lib/{backendbase.php => ldaputility.php} | 20 +-- apps/user_ldap/lib/proxy.php | 27 ++-- apps/user_ldap/settings.php | 7 +- apps/user_ldap/user_ldap.php | 94 +++++++------ apps/user_ldap/user_proxy.php | 10 +- 16 files changed, 226 insertions(+), 174 deletions(-) create mode 100644 apps/user_ldap/lib/backendutility.php rename apps/user_ldap/lib/{backendbase.php => ldaputility.php} (65%) diff --git a/apps/user_ldap/ajax/getConfiguration.php b/apps/user_ldap/ajax/getConfiguration.php index baca588976f..fc51b459a25 100644 --- a/apps/user_ldap/ajax/getConfiguration.php +++ b/apps/user_ldap/ajax/getConfiguration.php @@ -27,5 +27,6 @@ OCP\JSON::checkAppEnabled('user_ldap'); OCP\JSON::callCheck(); $prefix = $_POST['ldap_serverconfig_chooser']; -$connection = new \OCA\user_ldap\lib\Connection($prefix); +$ldapWrapper = new OCA\user_ldap\lib\LDAP(); +$connection = new \OCA\user_ldap\lib\Connection($ldapWrapper, $prefix); OCP\JSON::success(array('configuration' => $connection->getConfiguration())); diff --git a/apps/user_ldap/ajax/setConfiguration.php b/apps/user_ldap/ajax/setConfiguration.php index d850bda2470..94de8835fbc 100644 --- a/apps/user_ldap/ajax/setConfiguration.php +++ b/apps/user_ldap/ajax/setConfiguration.php @@ -27,7 +27,8 @@ OCP\JSON::checkAppEnabled('user_ldap'); OCP\JSON::callCheck(); $prefix = $_POST['ldap_serverconfig_chooser']; -$connection = new \OCA\user_ldap\lib\Connection($prefix); +$ldapWrapper = new OCA\user_ldap\lib\LDAP(); +$connection = new \OCA\user_ldap\lib\Connection($ldapWrapper, $prefix); $connection->setConfiguration($_POST); $connection->saveConfiguration(); OCP\JSON::success(); diff --git a/apps/user_ldap/ajax/testConfiguration.php b/apps/user_ldap/ajax/testConfiguration.php index 7ce1258a796..0b8e4ccfe20 100644 --- a/apps/user_ldap/ajax/testConfiguration.php +++ b/apps/user_ldap/ajax/testConfiguration.php @@ -28,7 +28,8 @@ OCP\JSON::callCheck(); $l=OC_L10N::get('user_ldap'); -$connection = new \OCA\user_ldap\lib\Connection('', null); +$ldapWrapper = new OCA\user_ldap\lib\LDAP(); +$connection = new \OCA\user_ldap\lib\Connection($ldapWrapper, '', null); if($connection->setConfiguration($_POST)) { //Configuration is okay if($connection->bind()) { diff --git a/apps/user_ldap/appinfo/app.php b/apps/user_ldap/appinfo/app.php index 593e846bc03..9d6327181af 100644 --- a/apps/user_ldap/appinfo/app.php +++ b/apps/user_ldap/appinfo/app.php @@ -24,15 +24,15 @@ OCP\App::registerAdmin('user_ldap', 'settings'); $configPrefixes = OCA\user_ldap\lib\Helper::getServerConfigurationPrefixes(true); +$ldapWrapper = new OCA\user_ldap\lib\LDAP(); if(count($configPrefixes) === 1) { - $connector = new OCA\user_ldap\lib\Connection($configPrefixes[0]); - $userBackend = new OCA\user_ldap\USER_LDAP(); - $userBackend->setConnector($connector); - $groupBackend = new OCA\user_ldap\GROUP_LDAP(); - $groupBackend->setConnector($connector); + $connector = new OCA\user_ldap\lib\Connection($ldapWrapper, $configPrefixes[0]); + $ldapAccess = new OCA\user_ldap\lib\Access($connector, $ldapWrapper); + $userBackend = new OCA\user_ldap\USER_LDAP($ldapAccess); + $groupBackend = new OCA\user_ldap\GROUP_LDAP($ldapAccess); } else { - $userBackend = new OCA\user_ldap\User_Proxy($configPrefixes); - $groupBackend = new OCA\user_ldap\Group_Proxy($configPrefixes); + $userBackend = new OCA\user_ldap\User_Proxy($configPrefixes, $ldapWrapper); + $groupBackend = new OCA\user_ldap\Group_Proxy($configPrefixes, $ldapWrapper); } if(count($configPrefixes) > 0) { diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index 04ff392f920..80b988594de 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -23,13 +23,16 @@ namespace OCA\user_ldap; -class GROUP_LDAP extends lib\Access implements \OCP\GroupInterface { +use OCA\user_ldap\lib\Access; +use OCA\user_ldap\lib\BackendUtility; + +class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { protected $enabled = false; - public function setConnector(lib\Connection &$connection) { - parent::setConnector($connection); - $filter = $this->connection->ldapGroupFilter; - $gassoc = $this->connection->ldapGroupMemberAssocAttr; + public function __construct(Access $access) { + parent::__construct($access); + $filter = $this->access->connection->ldapGroupFilter; + $gassoc = $this->access->connection->ldapGroupMemberAssocAttr; if(!empty($filter) && !empty($gassoc)) { $this->enabled = true; } @@ -47,30 +50,30 @@ class GROUP_LDAP extends lib\Access implements \OCP\GroupInterface { if(!$this->enabled) { return false; } - if($this->connection->isCached('inGroup'.$uid.':'.$gid)) { - return $this->connection->getFromCache('inGroup'.$uid.':'.$gid); + if($this->access->connection->isCached('inGroup'.$uid.':'.$gid)) { + return $this->access->connection->getFromCache('inGroup'.$uid.':'.$gid); } - $dn_user = $this->username2dn($uid); - $dn_group = $this->groupname2dn($gid); + $dn_user = $this->access->username2dn($uid); + $dn_group = $this->access->groupname2dn($gid); // just in case if(!$dn_group || !$dn_user) { - $this->connection->writeToCache('inGroup'.$uid.':'.$gid, false); + $this->access->connection->writeToCache('inGroup'.$uid.':'.$gid, false); return false; } //usually, LDAP attributes are said to be case insensitive. But there are exceptions of course. - $members = $this->readAttribute($dn_group, $this->connection->ldapGroupMemberAssocAttr); + $members = $this->access->readAttribute($dn_group, $this->access->connection->ldapGroupMemberAssocAttr); if(!$members) { - $this->connection->writeToCache('inGroup'.$uid.':'.$gid, false); + $this->access->connection->writeToCache('inGroup'.$uid.':'.$gid, false); return false; } //extra work if we don't get back user DNs //TODO: this can be done with one LDAP query - if(strtolower($this->connection->ldapGroupMemberAssocAttr) === 'memberuid') { + if(strtolower($this->access->connection->ldapGroupMemberAssocAttr) === 'memberuid') { $dns = array(); foreach($members as $mid) { - $filter = str_replace('%uid', $mid, $this->connection->ldapLoginFilter); - $ldap_users = $this->fetchListOfUsers($filter, 'dn'); + $filter = str_replace('%uid', $mid, $this->access->connection->ldapLoginFilter); + $ldap_users = $this->access->fetchListOfUsers($filter, 'dn'); if(count($ldap_users) < 1) { continue; } @@ -80,7 +83,7 @@ class GROUP_LDAP extends lib\Access implements \OCP\GroupInterface { } $isInGroup = in_array($dn_user, $members); - $this->connection->writeToCache('inGroup'.$uid.':'.$gid, $isInGroup); + $this->access->connection->writeToCache('inGroup'.$uid.':'.$gid, $isInGroup); return $isInGroup; } @@ -98,35 +101,35 @@ class GROUP_LDAP extends lib\Access implements \OCP\GroupInterface { return array(); } $cacheKey = 'getUserGroups'.$uid; - if($this->connection->isCached($cacheKey)) { - return $this->connection->getFromCache($cacheKey); + if($this->access->connection->isCached($cacheKey)) { + return $this->access->connection->getFromCache($cacheKey); } - $userDN = $this->username2dn($uid); + $userDN = $this->access->username2dn($uid); if(!$userDN) { - $this->connection->writeToCache($cacheKey, array()); + $this->access->connection->writeToCache($cacheKey, array()); return array(); } //uniqueMember takes DN, memberuid the uid, so we need to distinguish - if((strtolower($this->connection->ldapGroupMemberAssocAttr) === 'uniquemember') - || (strtolower($this->connection->ldapGroupMemberAssocAttr) === 'member') + if((strtolower($this->access->connection->ldapGroupMemberAssocAttr) === 'uniquemember') + || (strtolower($this->access->connection->ldapGroupMemberAssocAttr) === 'member') ) { $uid = $userDN; - } else if(strtolower($this->connection->ldapGroupMemberAssocAttr) === 'memberuid') { - $result = $this->readAttribute($userDN, 'uid'); + } else if(strtolower($this->access->connection->ldapGroupMemberAssocAttr) === 'memberuid') { + $result = $this->access->readAttribute($userDN, 'uid'); $uid = $result[0]; } else { // just in case $uid = $userDN; } - $filter = $this->combineFilterWithAnd(array( - $this->connection->ldapGroupFilter, - $this->connection->ldapGroupMemberAssocAttr.'='.$uid + $filter = $this->access->combineFilterWithAnd(array( + $this->access->connection->ldapGroupFilter, + $this->access->connection->ldapGroupMemberAssocAttr.'='.$uid )); - $groups = $this->fetchListOfGroups($filter, array($this->connection->ldapGroupDisplayName, 'dn')); - $groups = array_unique($this->ownCloudGroupNames($groups), SORT_LOCALE_STRING); - $this->connection->writeToCache($cacheKey, $groups); + $groups = $this->access->fetchListOfGroups($filter, array($this->access->connection->ldapGroupDisplayName, 'dn')); + $groups = array_unique($this->access->ownCloudGroupNames($groups), SORT_LOCALE_STRING); + $this->access->connection->writeToCache($cacheKey, $groups); return $groups; } @@ -144,70 +147,70 @@ class GROUP_LDAP extends lib\Access implements \OCP\GroupInterface { } $cachekey = 'usersInGroup-'.$gid.'-'.$search.'-'.$limit.'-'.$offset; // check for cache of the exact query - $groupUsers = $this->connection->getFromCache($cachekey); + $groupUsers = $this->access->connection->getFromCache($cachekey); if(!is_null($groupUsers)) { return $groupUsers; } // check for cache of the query without limit and offset - $groupUsers = $this->connection->getFromCache('usersInGroup-'.$gid.'-'.$search); + $groupUsers = $this->access->connection->getFromCache('usersInGroup-'.$gid.'-'.$search); if(!is_null($groupUsers)) { $groupUsers = array_slice($groupUsers, $offset, $limit); - $this->connection->writeToCache($cachekey, $groupUsers); + $this->access->connection->writeToCache($cachekey, $groupUsers); return $groupUsers; } if($limit === -1) { $limit = null; } - $groupDN = $this->groupname2dn($gid); + $groupDN = $this->access->groupname2dn($gid); if(!$groupDN) { // group couldn't be found, return empty resultset - $this->connection->writeToCache($cachekey, array()); + $this->access->connection->writeToCache($cachekey, array()); return array(); } - $members = $this->readAttribute($groupDN, $this->connection->ldapGroupMemberAssocAttr); + $members = $this->access->readAttribute($groupDN, $this->access->connection->ldapGroupMemberAssocAttr); if(!$members) { //in case users could not be retrieved, return empty resultset - $this->connection->writeToCache($cachekey, array()); + $this->access->connection->writeToCache($cachekey, array()); return array(); } $groupUsers = array(); - $isMemberUid = (strtolower($this->connection->ldapGroupMemberAssocAttr) === 'memberuid'); + $isMemberUid = (strtolower($this->access->connection->ldapGroupMemberAssocAttr) === 'memberuid'); foreach($members as $member) { if($isMemberUid) { //we got uids, need to get their DNs to 'tranlsate' them to usernames - $filter = $this->combineFilterWithAnd(array( + $filter = $this->access->combineFilterWithAnd(array( \OCP\Util::mb_str_replace('%uid', $member, - $this->connection->ldapLoginFilter, 'UTF-8'), - $this->getFilterPartForUserSearch($search) + $this->access->connection->ldapLoginFilter, 'UTF-8'), + $this->access->getFilterPartForUserSearch($search) )); - $ldap_users = $this->fetchListOfUsers($filter, 'dn'); + $ldap_users = $this->access->fetchListOfUsers($filter, 'dn'); if(count($ldap_users) < 1) { continue; } - $groupUsers[] = $this->dn2username($ldap_users[0]); + $groupUsers[] = $this->access->dn2username($ldap_users[0]); } else { //we got DNs, check if we need to filter by search or we can give back all of them if(!empty($search)) { - if(!$this->readAttribute($member, - $this->connection->ldapUserDisplayName, - $this->getFilterPartForUserSearch($search))) { + if(!$this->access->readAttribute($member, + $this->access->connection->ldapUserDisplayName, + $this->access->getFilterPartForUserSearch($search))) { continue; } } // dn2username will also check if the users belong to the allowed base - if($ocname = $this->dn2username($member)) { + if($ocname = $this->access->dn2username($member)) { $groupUsers[] = $ocname; } } } natsort($groupUsers); - $this->connection->writeToCache('usersInGroup-'.$gid.'-'.$search, $groupUsers); + $this->access->connection->writeToCache('usersInGroup-'.$gid.'-'.$search, $groupUsers); $groupUsers = array_slice($groupUsers, $offset, $limit); - $this->connection->writeToCache($cachekey, $groupUsers); + $this->access->connection->writeToCache($cachekey, $groupUsers); return $groupUsers; } @@ -245,7 +248,7 @@ class GROUP_LDAP extends lib\Access implements \OCP\GroupInterface { //Check cache before driving unnecessary searches \OCP\Util::writeLog('user_ldap', 'getGroups '.$cachekey, \OCP\Util::DEBUG); - $ldap_groups = $this->connection->getFromCache($cachekey); + $ldap_groups = $this->access->connection->getFromCache($cachekey); if(!is_null($ldap_groups)) { return $ldap_groups; } @@ -255,16 +258,16 @@ class GROUP_LDAP extends lib\Access implements \OCP\GroupInterface { if($limit <= 0) { $limit = null; } - $filter = $this->combineFilterWithAnd(array( - $this->connection->ldapGroupFilter, - $this->getFilterPartForGroupSearch($search) + $filter = $this->access->combineFilterWithAnd(array( + $this->access->connection->ldapGroupFilter, + $this->access->getFilterPartForGroupSearch($search) )); \OCP\Util::writeLog('user_ldap', 'getGroups Filter '.$filter, \OCP\Util::DEBUG); - $ldap_groups = $this->fetchListOfGroups($filter, array($this->connection->ldapGroupDisplayName, 'dn'), + $ldap_groups = $this->access->fetchListOfGroups($filter, array($this->access->connection->ldapGroupDisplayName, 'dn'), $limit, $offset); - $ldap_groups = $this->ownCloudGroupNames($ldap_groups); + $ldap_groups = $this->access->ownCloudGroupNames($ldap_groups); - $this->connection->writeToCache($cachekey, $ldap_groups); + $this->access->connection->writeToCache($cachekey, $ldap_groups); return $ldap_groups; } @@ -278,25 +281,25 @@ class GROUP_LDAP extends lib\Access implements \OCP\GroupInterface { * @return bool */ public function groupExists($gid) { - if($this->connection->isCached('groupExists'.$gid)) { - return $this->connection->getFromCache('groupExists'.$gid); + if($this->access->connection->isCached('groupExists'.$gid)) { + return $this->access->connection->getFromCache('groupExists'.$gid); } //getting dn, if false the group does not exist. If dn, it may be mapped only, requires more checking. - $dn = $this->groupname2dn($gid); + $dn = $this->access->groupname2dn($gid); if(!$dn) { - $this->connection->writeToCache('groupExists'.$gid, false); + $this->access->connection->writeToCache('groupExists'.$gid, false); return false; } //if group really still exists, we will be able to read its objectclass - $objcs = $this->readAttribute($dn, 'objectclass'); + $objcs = $this->access->readAttribute($dn, 'objectclass'); if(!$objcs || empty($objcs)) { - $this->connection->writeToCache('groupExists'.$gid, false); + $this->access->connection->writeToCache('groupExists'.$gid, false); return false; } - $this->connection->writeToCache('groupExists'.$gid, true); + $this->access->connection->writeToCache('groupExists'.$gid, true); return true; } diff --git a/apps/user_ldap/group_proxy.php b/apps/user_ldap/group_proxy.php index eb6f176c58c..78a616854c7 100644 --- a/apps/user_ldap/group_proxy.php +++ b/apps/user_ldap/group_proxy.php @@ -23,6 +23,8 @@ namespace OCA\user_ldap; +use OCA\user_ldap\lib\ILDAPWrapper; + class Group_Proxy extends lib\Proxy implements \OCP\GroupInterface { private $backends = array(); private $refBackend = null; @@ -31,12 +33,10 @@ class Group_Proxy extends lib\Proxy implements \OCP\GroupInterface { * @brief Constructor * @param $serverConfigPrefixes array containing the config Prefixes */ - public function __construct($serverConfigPrefixes) { - parent::__construct(); + public function __construct($serverConfigPrefixes, ILDAPWrapper $ldap) { + parent::__construct($ldap); foreach($serverConfigPrefixes as $configPrefix) { - $this->backends[$configPrefix] = new \OCA\user_ldap\GROUP_LDAP(); - $connector = $this->getConnector($configPrefix); - $this->backends[$configPrefix]->setConnector($connector); + $this->backends[$configPrefix] = new \OCA\user_ldap\GROUP_LDAP($this->getAccess($configPrefix)); if(is_null($this->refBackend)) { $this->refBackend = &$this->backends[$configPrefix]; } diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 4b7920b7162..3d791755aba 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -23,12 +23,13 @@ namespace OCA\user_ldap\lib; -abstract class Access extends BackendBase { - protected $connection; +class Access extends LDAPUtility { + public $connection; //never ever check this var directly, always use getPagedSearchResultState protected $pagedSearchedSuccessful; - public function setConnector(Connection &$connection) { + public function __construct(Connection $connection, ILDAPWrapper $ldap) { + parent::__construct($ldap); $this->connection = $connection; } diff --git a/apps/user_ldap/lib/backendutility.php b/apps/user_ldap/lib/backendutility.php new file mode 100644 index 00000000000..f279b1e997e --- /dev/null +++ b/apps/user_ldap/lib/backendutility.php @@ -0,0 +1,38 @@ +. + * + */ + +namespace OCA\user_ldap\lib; + +use OCA\user_ldap\lib\Access; + +abstract class BackendUtility { + protected $access; + + /** + * @brief constructor, make sure the subclasses call this one! + * @param $access an instance of Access for LDAP interaction + */ + public function __construct(Access $access) { + $this->access = $access; + } +} \ No newline at end of file diff --git a/apps/user_ldap/lib/connection.php b/apps/user_ldap/lib/connection.php index dd627a4e1b1..0bf13a19377 100644 --- a/apps/user_ldap/lib/connection.php +++ b/apps/user_ldap/lib/connection.php @@ -23,7 +23,7 @@ namespace OCA\user_ldap\lib; -class Connection extends BackendBase { +class Connection extends LDAPUtility { private $ldapConnectionRes = null; private $configPrefix; private $configID; @@ -60,7 +60,7 @@ class Connection extends BackendBase { 'ldapQuotaDefault' => null, 'ldapEmailAttribute' => null, 'ldapCacheTTL' => null, - 'ldapUuidAttribute' => null, + 'ldapUuidAttribute' => 'auto', 'ldapOverrideUuidAttribute' => null, 'ldapOverrideMainServer' => false, 'ldapConfigurationActive' => false, @@ -77,8 +77,8 @@ class Connection extends BackendBase { * @param $configPrefix a string with the prefix for the configkey column (appconfig table) * @param $configID a string with the value for the appid column (appconfig table) or null for on-the-fly connections */ - public function __construct($configPrefix = '', $configID = 'user_ldap') { - parent::__construct(); + public function __construct(ILDAPWrapper $ldap, $configPrefix = '', $configID = 'user_ldap') { + parent::__construct($ldap); $this->configPrefix = $configPrefix; $this->configID = $configID; $memcache = new \OC\Memcache\Factory(); @@ -363,6 +363,14 @@ class Connection extends BackendBase { && $params[$parameter] === 'homeFolderNamingRule')) && !empty($value)) { $value = 'attr:'.$value; + } else if (strpos($parameter, 'ldapBase') !== false + || (isset($params[$parameter]) + && strpos($params[$parameter], 'ldapBase') !== false)) { + $this->readBase($params[$parameter], $value); + if(is_array($setParameters)) { + $setParameters[] = $parameter; + } + continue; } if(isset($this->config[$parameter])) { $this->config[$parameter] = $value; @@ -433,10 +441,6 @@ class Connection extends BackendBase { $config[$dbKey] = ''; } continue; - } else if((strpos($classKey, 'ldapBase') !== false) - || (strpos($classKey, 'ldapAttributes') !== false)) { - $config[$dbKey] = implode("\n", $this->config[$classKey]); - continue; } $config[$dbKey] = $this->config[$classKey]; } @@ -553,7 +557,7 @@ class Connection extends BackendBase { * @returns an associative array with the default values. Keys are correspond * to config-value entries in the database table */ - public function getDefaults() { + static public function getDefaults() { return array( 'ldap_host' => '', 'ldap_port' => '389', diff --git a/apps/user_ldap/lib/jobs.php b/apps/user_ldap/lib/jobs.php index 6b7666d4ca1..2f90da3bfb6 100644 --- a/apps/user_ldap/lib/jobs.php +++ b/apps/user_ldap/lib/jobs.php @@ -139,13 +139,14 @@ class Jobs extends \OC\BackgroundJob\TimedJob { return self::$groupBE; } $configPrefixes = Helper::getServerConfigurationPrefixes(true); - if(count($configPrefixes) == 1) { + $ldapWrapper = new OCA\user_ldap\lib\LDAP(); + if(count($configPrefixes) === 1) { //avoid the proxy when there is only one LDAP server configured - $connector = new Connection($configPrefixes[0]); - self::$groupBE = new \OCA\user_ldap\GROUP_LDAP(); - self::$groupBE->setConnector($connector); + $connector = new OCA\user_ldap\lib\Connection($ldapWrapper, $configPrefixes[0]); + $ldapAccess = new OCA\user_ldap\lib\Access($connector, $ldapWrapper); + self::$groupBE = new OCA\user_ldap\GROUP_LDAP($ldapAccess); } else { - self::$groupBE = new \OCA\user_ldap\Group_Proxy($configPrefixes); + self::$groupBE = new \OCA\user_ldap\Group_Proxy($configPrefixes, $ldapWrapper); } return self::$groupBE; diff --git a/apps/user_ldap/lib/ldap.php b/apps/user_ldap/lib/ldap.php index 0f082147b14..ee2e336cc95 100644 --- a/apps/user_ldap/lib/ldap.php +++ b/apps/user_ldap/lib/ldap.php @@ -149,6 +149,8 @@ class LDAP implements ILDAPWrapper { && $errorCode === -4) { } else if ($errorCode === 32) { //for now + } else if ($errorCode === 10) { + //referrals, we switch them off, but then there is AD :) } else { throw new \Exception('LDAP error '.$errorMsg.' (' . $errorCode.') after calling '.$this->curFunc. diff --git a/apps/user_ldap/lib/backendbase.php b/apps/user_ldap/lib/ldaputility.php similarity index 65% rename from apps/user_ldap/lib/backendbase.php rename to apps/user_ldap/lib/ldaputility.php index 5ed73a0a5b1..975df7d1de0 100644 --- a/apps/user_ldap/lib/backendbase.php +++ b/apps/user_ldap/lib/ldaputility.php @@ -1,7 +1,7 @@ ldap = new LDAP(); - } - /** - * @brief sets the LDAP Wrapper to be used - * - * @param $ldapWrapper an instance of the Wrapper - * @return true on success, otherwise false - * - * The LDAP Wrapper must implement the PHP LDAP functions, which are used - * in the LDAP backend + * @brief constructor, make sure the subclasses call this one! + * @param $ldapWrapper an instance of an ILDAPWrapper */ - public function setLDAPWrapper(ILDAPWrapper $ldapWrapper) { + public function __construct(ILDAPWrapper $ldapWrapper) { $this->ldap = $ldapWrapper; - return true; } } \ No newline at end of file diff --git a/apps/user_ldap/lib/proxy.php b/apps/user_ldap/lib/proxy.php index ae3e3be7361..c74b357bdd2 100644 --- a/apps/user_ldap/lib/proxy.php +++ b/apps/user_ldap/lib/proxy.php @@ -23,26 +23,27 @@ namespace OCA\user_ldap\lib; -abstract class Proxy { - static private $connectors = array(); +use OCA\user_ldap\lib\Access; - public function __construct() { +abstract class Proxy { + static private $accesses = array(); + private $ldap = null; + + public function __construct(ILDAPWrapper $ldap) { + $this->ldap = $ldap; $this->cache = \OC_Cache::getGlobalCache(); } - private function addConnector($configPrefix) { - self::$connectors[$configPrefix] = new \OCA\user_ldap\lib\Connection($configPrefix); + private function addAccess($configPrefix) { + $connector = new Connection($this->ldap, $configPrefix); + self::$accesses[$configPrefix] = new Access($connector, $this->ldap); } - protected function getConnector($configPrefix) { - if(!isset(self::$connectors[$configPrefix])) { - $this->addConnector($configPrefix); + protected function getAccess($configPrefix) { + if(!isset(self::$accesses[$configPrefix])) { + $this->addAccess($configPrefix); } - return self::$connectors[$configPrefix]; - } - - protected function getConnectors() { - return self::$connectors; + return self::$accesses[$configPrefix]; } protected function getUserCacheKey($uid) { diff --git a/apps/user_ldap/settings.php b/apps/user_ldap/settings.php index 7169192a18e..b7070f23183 100644 --- a/apps/user_ldap/settings.php +++ b/apps/user_ldap/settings.php @@ -49,14 +49,9 @@ $tmpl->assign('serverConfigurationPrefixes', $prefixes); $tmpl->assign('serverConfigurationHosts', $hosts); // assign default values -if(!isset($ldap)) { - $ldap = new \OCA\user_ldap\lib\Connection(); -} -$defaults = $ldap->getDefaults(); +$defaults = \OCA\user_ldap\lib\Connection::getDefaults(); foreach($defaults as $key => $default) { $tmpl->assign($key.'_default', $default); } -// $tmpl->assign(); - return $tmpl->fetchPage(); diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php index ab3b57001b6..f2e99d45794 100644 --- a/apps/user_ldap/user_ldap.php +++ b/apps/user_ldap/user_ldap.php @@ -25,37 +25,46 @@ namespace OCA\user_ldap; -class USER_LDAP extends lib\Access implements \OCP\UserInterface { +use OCA\user_ldap\lib\ILDAPWrapper; +use OCA\user_ldap\lib\BackendUtility; + +class USER_LDAP extends BackendUtility implements \OCP\UserInterface { private function updateQuota($dn) { $quota = null; - $quotaDefault = $this->connection->ldapQuotaDefault; - $quotaAttribute = $this->connection->ldapQuotaAttribute; + $quotaDefault = $this->access->connection->ldapQuotaDefault; + $quotaAttribute = $this->access->connection->ldapQuotaAttribute; if(!empty($quotaDefault)) { $quota = $quotaDefault; } if(!empty($quotaAttribute)) { - $aQuota = $this->readAttribute($dn, $quotaAttribute); + $aQuota = $this->access->readAttribute($dn, $quotaAttribute); if($aQuota && (count($aQuota) > 0)) { $quota = $aQuota[0]; } } if(!is_null($quota)) { - \OCP\Config::setUserValue($this->dn2username($dn), 'files', 'quota', \OCP\Util::computerFileSize($quota)); + \OCP\Config::setUserValue( $this->access->dn2username($dn), + 'files', + 'quota', + \OCP\Util::computerFileSize($quota)); } } private function updateEmail($dn) { $email = null; - $emailAttribute = $this->connection->ldapEmailAttribute; + $emailAttribute = $this->access->connection->ldapEmailAttribute; if(!empty($emailAttribute)) { - $aEmail = $this->readAttribute($dn, $emailAttribute); + $aEmail = $this->access->readAttribute($dn, $emailAttribute); if($aEmail && (count($aEmail) > 0)) { $email = $aEmail[0]; } if(!is_null($email)) { - \OCP\Config::setUserValue($this->dn2username($dn), 'settings', 'email', $email); + \OCP\Config::setUserValue( $this->access->dn2username($dn), + 'settings', + 'email', + $email); } } } @@ -70,15 +79,15 @@ class USER_LDAP extends lib\Access implements \OCP\UserInterface { */ public function checkPassword($uid, $password) { //find out dn of the user name - $filter = \OCP\Util::mb_str_replace('%uid', $uid, $this->connection->ldapLoginFilter, 'UTF-8'); - $ldap_users = $this->fetchListOfUsers($filter, 'dn'); + $filter = \OCP\Util::mb_str_replace('%uid', $uid, $this->access->connection->ldapLoginFilter, 'UTF-8'); + $ldap_users = $this->access->fetchListOfUsers($filter, 'dn'); if(count($ldap_users) < 1) { return false; } $dn = $ldap_users[0]; //do we have a username for him/her? - $ocname = $this->dn2username($dn); + $ocname = $this->access->dn2username($dn); if($ocname) { //update some settings, if necessary @@ -86,7 +95,7 @@ class USER_LDAP extends lib\Access implements \OCP\UserInterface { $this->updateEmail($dn); //are the credentials OK? - if(!$this->areCredentialsValid($dn, $password)) { + if(!$this->access->areCredentialsValid($dn, $password)) { return false; } @@ -107,7 +116,7 @@ class USER_LDAP extends lib\Access implements \OCP\UserInterface { $cachekey = 'getUsers-'.$search.'-'.$limit.'-'.$offset; //check if users are cached, if so return - $ldap_users = $this->connection->getFromCache($cachekey); + $ldap_users = $this->access->connection->getFromCache($cachekey); if(!is_null($ldap_users)) { return $ldap_users; } @@ -117,21 +126,23 @@ class USER_LDAP extends lib\Access implements \OCP\UserInterface { if($limit <= 0) { $limit = null; } - $filter = $this->combineFilterWithAnd(array( - $this->connection->ldapUserFilter, - $this->getFilterPartForUserSearch($search) + $filter = $this->access->combineFilterWithAnd(array( + $this->access->connection->ldapUserFilter, + $this->access->getFilterPartForUserSearch($search) )); \OCP\Util::writeLog('user_ldap', 'getUsers: Options: search '.$search.' limit '.$limit.' offset '.$offset.' Filter: '.$filter, \OCP\Util::DEBUG); //do the search and translate results to owncloud names - $ldap_users = $this->fetchListOfUsers($filter, array($this->connection->ldapUserDisplayName, 'dn'), + $ldap_users = $this->access->fetchListOfUsers( + $filter, + array($this->access->connection->ldapUserDisplayName, 'dn'), $limit, $offset); - $ldap_users = $this->ownCloudUserNames($ldap_users); + $ldap_users = $this->access->ownCloudUserNames($ldap_users); \OCP\Util::writeLog('user_ldap', 'getUsers: '.count($ldap_users). ' Users found', \OCP\Util::DEBUG); - $this->connection->writeToCache($cachekey, $ldap_users); + $this->access->connection->writeToCache($cachekey, $ldap_users); return $ldap_users; } @@ -141,22 +152,24 @@ class USER_LDAP extends lib\Access implements \OCP\UserInterface { * @return boolean */ public function userExists($uid) { - if($this->connection->isCached('userExists'.$uid)) { - return $this->connection->getFromCache('userExists'.$uid); + if($this->access->connection->isCached('userExists'.$uid)) { + return $this->access->connection->getFromCache('userExists'.$uid); } //getting dn, if false the user does not exist. If dn, he may be mapped only, requires more checking. - $dn = $this->username2dn($uid); + $dn = $this->access->username2dn($uid); if(!$dn) { - $this->connection->writeToCache('userExists'.$uid, false); + \OCP\Util::writeLog('user_ldap', 'No DN found for '.$uid.' on '.$this->access->connection->ldapHost, \OCP\Util::DEBUG); + $this->access->connection->writeToCache('userExists'.$uid, false); return false; } //check if user really still exists by reading its entry - if(!is_array($this->readAttribute($dn, ''))) { - $this->connection->writeToCache('userExists'.$uid, false); + if(!is_array($this->access->readAttribute($dn, ''))) { + \OCP\Util::writeLog('user_ldap', 'LDAP says no user '.$dn, \OCP\Util::DEBUG); + $this->access->connection->writeToCache('userExists'.$uid, false); return false; } - $this->connection->writeToCache('userExists'.$uid, true); + $this->access->connection->writeToCache('userExists'.$uid, true); $this->updateQuota($dn); return true; } @@ -184,12 +197,13 @@ class USER_LDAP extends lib\Access implements \OCP\UserInterface { } $cacheKey = 'getHome'.$uid; - if($this->connection->isCached($cacheKey)) { - return $this->connection->getFromCache($cacheKey); + if($this->access->connection->isCached($cacheKey)) { + return $this->access->connection->getFromCache($cacheKey); } - if(strpos($this->connection->homeFolderNamingRule, 'attr:') === 0) { - $attr = substr($this->connection->homeFolderNamingRule, strlen('attr:')); - $homedir = $this->readAttribute($this->username2dn($uid), $attr); + if(strpos($this->access->connection->homeFolderNamingRule, 'attr:') === 0) { + $attr = substr($this->access->connection->homeFolderNamingRule, strlen('attr:')); + $homedir = $this->access->readAttribute( + $this->access->username2dn($uid), $attr); if($homedir && isset($homedir[0])) { $path = $homedir[0]; //if attribute's value is an absolute path take this, otherwise append it to data dir @@ -204,13 +218,13 @@ class USER_LDAP extends lib\Access implements \OCP\UserInterface { $homedir = \OCP\Config::getSystemValue('datadirectory', \OC::$SERVERROOT.'/data' ) . '/' . $homedir[0]; } - $this->connection->writeToCache($cacheKey, $homedir); + $this->access->connection->writeToCache($cacheKey, $homedir); return $homedir; } } //false will apply default behaviour as defined and done by OC_User - $this->connection->writeToCache($cacheKey, false); + $this->access->connection->writeToCache($cacheKey, false); return false; } @@ -225,16 +239,16 @@ class USER_LDAP extends lib\Access implements \OCP\UserInterface { } $cacheKey = 'getDisplayName'.$uid; - if(!is_null($displayName = $this->connection->getFromCache($cacheKey))) { + if(!is_null($displayName = $this->access->connection->getFromCache($cacheKey))) { return $displayName; } - $displayName = $this->readAttribute( - $this->username2dn($uid), - $this->connection->ldapUserDisplayName); + $displayName = $this->access->readAttribute( + $this->access->username2dn($uid), + $this->access->connection->ldapUserDisplayName); if($displayName && (count($displayName) > 0)) { - $this->connection->writeToCache($cacheKey, $displayName[0]); + $this->access->connection->writeToCache($cacheKey, $displayName[0]); return $displayName[0]; } @@ -249,7 +263,7 @@ class USER_LDAP extends lib\Access implements \OCP\UserInterface { */ public function getDisplayNames($search = '', $limit = null, $offset = null) { $cacheKey = 'getDisplayNames-'.$search.'-'.$limit.'-'.$offset; - if(!is_null($displayNames = $this->connection->getFromCache($cacheKey))) { + if(!is_null($displayNames = $this->access->connection->getFromCache($cacheKey))) { return $displayNames; } @@ -258,7 +272,7 @@ class USER_LDAP extends lib\Access implements \OCP\UserInterface { foreach ($users as $user) { $displayNames[$user] = $this->getDisplayName($user); } - $this->connection->writeToCache($cacheKey, $displayNames); + $this->access->connection->writeToCache($cacheKey, $displayNames); return $displayNames; } diff --git a/apps/user_ldap/user_proxy.php b/apps/user_ldap/user_proxy.php index 0722d8871a4..c7b469c8224 100644 --- a/apps/user_ldap/user_proxy.php +++ b/apps/user_ldap/user_proxy.php @@ -23,6 +23,8 @@ namespace OCA\user_ldap; +use OCA\user_ldap\lib\ILDAPWrapper; + class User_Proxy extends lib\Proxy implements \OCP\UserInterface { private $backends = array(); private $refBackend = null; @@ -31,12 +33,10 @@ class User_Proxy extends lib\Proxy implements \OCP\UserInterface { * @brief Constructor * @param $serverConfigPrefixes array containing the config Prefixes */ - public function __construct($serverConfigPrefixes) { - parent::__construct(); + public function __construct($serverConfigPrefixes, ILDAPWrapper $ldap) { + parent::__construct($ldap); foreach($serverConfigPrefixes as $configPrefix) { - $this->backends[$configPrefix] = new \OCA\user_ldap\USER_LDAP(); - $connector = $this->getConnector($configPrefix); - $this->backends[$configPrefix]->setConnector($connector); + $this->backends[$configPrefix] = new \OCA\user_ldap\USER_LDAP($this->getAccess($configPrefix)); if(is_null($this->refBackend)) { $this->refBackend = &$this->backends[$configPrefix]; } From bb162b1f94c3966457e8b08191cd85620478c940 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 10 Sep 2013 17:39:29 +0200 Subject: [PATCH 10/16] LDAP: get rid of some PHP Warnings --- apps/user_ldap/lib/connection.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/user_ldap/lib/connection.php b/apps/user_ldap/lib/connection.php index 0bf13a19377..053ec7e2355 100644 --- a/apps/user_ldap/lib/connection.php +++ b/apps/user_ldap/lib/connection.php @@ -396,7 +396,7 @@ class Connection extends LDAPUtility { public function saveConfiguration() { $trans = array_flip($this->getConfigTranslationArray()); foreach($this->config as $key => $value) { - \OCP\Util::writeLog('user_ldap', 'LDAP: storing key '.$key.' value '.$value, \OCP\Util::DEBUG); + \OCP\Util::writeLog('user_ldap', 'LDAP: storing key '.$key.' value '.print_r($value, true), \OCP\Util::DEBUG); switch ($key) { case 'ldapAgentPassword': $value = base64_encode($value); @@ -441,6 +441,11 @@ class Connection extends LDAPUtility { $config[$dbKey] = ''; } continue; + } else if((strpos($classKey, 'ldapBase') !== false + || strpos($classKey, 'ldapAttributes') !== false) + && is_array($this->config[$classKey])) { + $config[$dbKey] = implode("\n", $this->config[$classKey]); + continue; } $config[$dbKey] = $this->config[$classKey]; } From 84f522f406d2bec8dff5acb0c7defc54b8e7d79a Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 11 Sep 2013 18:51:45 +0200 Subject: [PATCH 11/16] LDAP: simple tests for the user backend --- apps/user_ldap/tests/user_ldap_direct.php | 313 ++++++++++++++++++++++ 1 file changed, 313 insertions(+) create mode 100644 apps/user_ldap/tests/user_ldap_direct.php diff --git a/apps/user_ldap/tests/user_ldap_direct.php b/apps/user_ldap/tests/user_ldap_direct.php new file mode 100644 index 00000000000..ab0563f12d6 --- /dev/null +++ b/apps/user_ldap/tests/user_ldap_direct.php @@ -0,0 +1,313 @@ +. +* +*/ + +namespace OCA\user_ldap\tests; + +use \OCA\user_ldap\USER_LDAP as UserLDAP; +use \OCA\user_ldap\lib\Access; +use \OCA\user_ldap\lib\Connection; +use \OCA\user_ldap\lib\ILDAPWrapper; + +class Test_User_Ldap_Direct extends \PHPUnit_Framework_TestCase { + protected $backend; + + public function setUp() { + \OC_User::clearBackends(); + \OC_Group::clearBackends(); + } + + private function getAccessMock() { + static $conMethods; + static $accMethods; + + if(is_null($conMethods) || is_null($accMethods)) { + $conMethods = get_class_methods('\OCA\user_ldap\lib\Connection'); + $accMethods = get_class_methods('\OCA\user_ldap\lib\Access'); + } + $lw = $this->getMock('\OCA\user_ldap\lib\ILDAPWrapper'); + $connector = $this->getMock('\OCA\user_ldap\lib\Connection', + $conMethods, + array($lw, null, null)); + $access = $this->getMock('\OCA\user_ldap\lib\Access', + $accMethods, + array($connector, $lw)); + + return $access; + } + + private function prepareMockForUserExists(&$access) { + $access->expects($this->any()) + ->method('username2dn') + ->will($this->returnCallback(function($uid) { + switch ($uid) { + case 'gunslinger': + return 'dnOfRoland'; + break; + case 'formerUser': + return 'dnOfFormerUser'; + break; + case 'newyorker': + return 'dnOfNewYorker'; + break; + case 'ladyofshadows': + return 'dnOfLadyOfShadows'; + break; + defautl: + return false; + } + })); + + + } + + public function testCheckPassword() { + $access = $this->getAccessMock(); + $backend = new UserLDAP($access); + \OC_User::useBackend($backend); + + $access->connection->expects($this->any()) + ->method('__get') + ->will($this->returnCallback(function($name) { + if($name === 'ldapLoginFilter') { + return '%uid'; + } + return null; + })); + + $access->expects($this->any()) + ->method('fetchListOfUsers') + ->will($this->returnCallback(function($filter) { + if($filter === 'roland') { + return array('dnOfRoland'); + } + return array(); + })); + + $access->expects($this->any()) + ->method('dn2username') + ->with($this->equalTo('dnOfRoland')) + ->will($this->returnValue('gunslinger')); + + $access->expects($this->any()) + ->method('areCredentialsValid') + ->will($this->returnCallback(function($dn, $pwd) { + if($pwd === 'dt19') { + return true; + } + return false; + })); + + $result = $backend->checkPassword('roland', 'dt19'); + $this->assertEquals('gunslinger', $result); + + $result = $backend->checkPassword('roland', 'wrong'); + $this->assertFalse($result); + + $result = $backend->checkPassword('mallory', 'evil'); + $this->assertFalse($result); + } + + public function testGetUsers() { + $access = $this->getAccessMock(); + $backend = new UserLDAP($access); + + $access->expects($this->any()) + ->method('getFilterPartForUserSearch') + ->will($this->returnCallback(function($search) { + return $search; + })); + + $access->expects($this->any()) + ->method('combineFilterWithAnd') + ->will($this->returnCallback(function($param) { + return $param[1]; + })); + + $access->expects($this->any()) + ->method('fetchListOfUsers') + ->will($this->returnCallback(function($search, $a, $l, $o) { + $users = array('gunslinger', 'newyorker', 'ladyofshadows'); + if(empty($search)) { + $result = $users; + } else { + $result = array(); + foreach($users as $user) { + if(stripos($user, $search) !== false) { + $result[] = $user; + } + } + } + if(!is_null($l) || !is_null($o)) { + $result = array_slice($result, $o, $l); + } + return $result; + })); + + $access->expects($this->any()) + ->method('ownCloudUserNames') + ->will($this->returnArgument(0)); + + $result = $backend->getUsers(); + $this->assertEquals(3, count($result)); + + $result = $backend->getUsers('', 1, 2); + $this->assertEquals(1, count($result)); + + $result = $backend->getUsers('', 2, 1); + $this->assertEquals(2, count($result)); + + $result = $backend->getUsers('yo'); + $this->assertEquals(2, count($result)); + + $result = $backend->getUsers('nix'); + $this->assertEquals(0, count($result)); + } + + public function testUserExists() { + $access = $this->getAccessMock(); + $backend = new UserLDAP($access); + $this->prepareMockForUserExists($access); + + $access->expects($this->any()) + ->method('readAttribute') + ->will($this->returnCallback(function($dn) { + if($dn === 'dnOfRoland') { + return array(); + } + return false; + })); + + //test for existing user + $result = $backend->userExists('gunslinger'); + $this->assertTrue($result); + + //test for deleted user + $result = $backend->userExists('formerUser'); + $this->assertFalse($result); + + //test for never-existing user + $result = $backend->userExists('mallory'); + $this->assertFalse($result); + } + + public function testDeleteUser() { + $access = $this->getAccessMock(); + $backend = new UserLDAP($access); + + //we do not support deleting users at all + $result = $backend->deleteUser('gunslinger'); + $this->assertFalse($result); + } + + public function testGetHome() { + $access = $this->getAccessMock(); + $backend = new UserLDAP($access); + $this->prepareMockForUserExists($access); + + $access->connection->expects($this->any()) + ->method('__get') + ->will($this->returnCallback(function($name) { + if($name === 'homeFolderNamingRule') { + return 'attr:testAttribute'; + } + return null; + })); + + $access->expects($this->any()) + ->method('readAttribute') + ->will($this->returnCallback(function($dn, $attr) { + switch ($dn) { + case 'dnOfRoland': + if($attr === 'testAttribute') { + return array('/tmp/rolandshome/'); + } + return array(); + break; + case 'dnOfLadyOfShadows': + if($attr === 'testAttribute') { + return array('susannah/'); + } + return array(); + break; + default: + return false; + } + })); + + //absolut path + $result = $backend->getHome('gunslinger'); + $this->assertEquals('/tmp/rolandshome/', $result); + + //datadir-relativ path + $result = $backend->getHome('ladyofshadows'); + $datadir = \OCP\Config::getSystemValue('datadirectory', + \OC::$SERVERROOT.'/data'); + $this->assertEquals($datadir.'/susannah/', $result); + + //no path at all – triggers OC default behaviour + $result = $backend->getHome('newyorker'); + $this->assertFalse($result); + } + + public function testGetDisplayName() { + $access = $this->getAccessMock(); + $backend = new UserLDAP($access); + $this->prepareMockForUserExists($access); + + $access->connection->expects($this->any()) + ->method('__get') + ->will($this->returnCallback(function($name) { + if($name === 'ldapUserDisplayName') { + return 'displayname'; + } + return null; + })); + + $access->expects($this->any()) + ->method('readAttribute') + ->will($this->returnCallback(function($dn, $attr) { + switch ($dn) { + case 'dnOfRoland': + if($attr === 'displayname') { + return array('Roland Deschain'); + } + return array(); + break; + + default: + return false; + } + })); + + //with displayName + $result = $backend->getDisplayName('gunslinger'); + $this->assertEquals('Roland Deschain', $result); + + //empty displayname retrieved + $result = $backend->getDisplayName('newyorker'); + $this->assertEquals(null, $result); + } + + //no test for getDisplayNames, because it just invokes getUsers and + //getDisplayName + +} \ No newline at end of file From 81cf4a22efb4dd7303a3aa3451e29992562b19f0 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 11 Sep 2013 19:42:08 +0200 Subject: [PATCH 12/16] LDAP: coding style --- apps/user_ldap/group_ldap.php | 18 ++++++++++++------ apps/user_ldap/group_proxy.php | 3 ++- apps/user_ldap/lib/access.php | 6 ++++-- apps/user_ldap/lib/backendutility.php | 2 +- apps/user_ldap/lib/connection.php | 10 +++++++--- apps/user_ldap/lib/ildapwrapper.php | 2 +- apps/user_ldap/lib/ldaputility.php | 2 +- apps/user_ldap/user_ldap.php | 6 ++++-- apps/user_ldap/user_proxy.php | 3 ++- 9 files changed, 34 insertions(+), 18 deletions(-) diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index 80b988594de..32e2cec5960 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -61,7 +61,8 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { return false; } //usually, LDAP attributes are said to be case insensitive. But there are exceptions of course. - $members = $this->access->readAttribute($dn_group, $this->access->connection->ldapGroupMemberAssocAttr); + $members = $this->access->readAttribute($dn_group, + $this->access->connection->ldapGroupMemberAssocAttr); if(!$members) { $this->access->connection->writeToCache('inGroup'.$uid.':'.$gid, false); return false; @@ -127,7 +128,8 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { $this->access->connection->ldapGroupFilter, $this->access->connection->ldapGroupMemberAssocAttr.'='.$uid )); - $groups = $this->access->fetchListOfGroups($filter, array($this->access->connection->ldapGroupDisplayName, 'dn')); + $groups = $this->access->fetchListOfGroups($filter, + array($this->access->connection->ldapGroupDisplayName, 'dn')); $groups = array_unique($this->access->ownCloudGroupNames($groups), SORT_LOCALE_STRING); $this->access->connection->writeToCache($cacheKey, $groups); @@ -170,7 +172,8 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { return array(); } - $members = $this->access->readAttribute($groupDN, $this->access->connection->ldapGroupMemberAssocAttr); + $members = $this->access->readAttribute($groupDN, + $this->access->connection->ldapGroupMemberAssocAttr); if(!$members) { //in case users could not be retrieved, return empty resultset $this->access->connection->writeToCache($cachekey, array()); @@ -263,8 +266,10 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { $this->access->getFilterPartForGroupSearch($search) )); \OCP\Util::writeLog('user_ldap', 'getGroups Filter '.$filter, \OCP\Util::DEBUG); - $ldap_groups = $this->access->fetchListOfGroups($filter, array($this->access->connection->ldapGroupDisplayName, 'dn'), - $limit, $offset); + $ldap_groups = $this->access->fetchListOfGroups($filter, + array($this->access->connection->ldapGroupDisplayName, 'dn'), + $limit, + $offset); $ldap_groups = $this->access->ownCloudGroupNames($ldap_groups); $this->access->connection->writeToCache($cachekey, $ldap_groups); @@ -285,7 +290,8 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { return $this->access->connection->getFromCache('groupExists'.$gid); } - //getting dn, if false the group does not exist. If dn, it may be mapped only, requires more checking. + //getting dn, if false the group does not exist. If dn, it may be mapped + //only, requires more checking. $dn = $this->access->groupname2dn($gid); if(!$dn) { $this->access->connection->writeToCache('groupExists'.$gid, false); diff --git a/apps/user_ldap/group_proxy.php b/apps/user_ldap/group_proxy.php index 78a616854c7..acc563c9532 100644 --- a/apps/user_ldap/group_proxy.php +++ b/apps/user_ldap/group_proxy.php @@ -36,7 +36,8 @@ class Group_Proxy extends lib\Proxy implements \OCP\GroupInterface { public function __construct($serverConfigPrefixes, ILDAPWrapper $ldap) { parent::__construct($ldap); foreach($serverConfigPrefixes as $configPrefix) { - $this->backends[$configPrefix] = new \OCA\user_ldap\GROUP_LDAP($this->getAccess($configPrefix)); + $this->backends[$configPrefix] = + new \OCA\user_ldap\GROUP_LDAP($this->getAccess($configPrefix)); if(is_null($this->refBackend)) { $this->refBackend = &$this->backends[$configPrefix]; } diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 3d791755aba..fdf9c24612d 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -80,7 +80,8 @@ class Access extends LDAPUtility { 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'); + $result = \OCP\Util::mb_array_change_key_case( + $this->ldap->getAttributes($cr, $er), MB_CASE_LOWER, 'UTF-8'); $attr = mb_strtolower($attr, 'UTF-8'); if(isset($result[$attr]) && $result[$attr]['count'] > 0) { @@ -669,7 +670,8 @@ class Access extends LDAPUtility { $error = $this->ldap->errno($link_resource); if(!is_array($sr) || $error !== 0) { \OCP\Util::writeLog('user_ldap', - 'Error when searching: '.$this->ldap->error($link_resource).' code '.$this->ldap->errno($link_resource), + 'Error when searching: '.$this->ldap->error($link_resource). + ' code '.$this->ldap->errno($link_resource), \OCP\Util::ERROR); \OCP\Util::writeLog('user_ldap', 'Attempt for Paging? '.print_r($pagedSearchOK, true), \OCP\Util::ERROR); return array(); diff --git a/apps/user_ldap/lib/backendutility.php b/apps/user_ldap/lib/backendutility.php index f279b1e997e..815757a1a11 100644 --- a/apps/user_ldap/lib/backendutility.php +++ b/apps/user_ldap/lib/backendutility.php @@ -35,4 +35,4 @@ abstract class BackendUtility { public function __construct(Access $access) { $this->access = $access; } -} \ No newline at end of file +} diff --git a/apps/user_ldap/lib/connection.php b/apps/user_ldap/lib/connection.php index 053ec7e2355..a53022c27b3 100644 --- a/apps/user_ldap/lib/connection.php +++ b/apps/user_ldap/lib/connection.php @@ -396,7 +396,8 @@ class Connection extends LDAPUtility { public function saveConfiguration() { $trans = array_flip($this->getConfigTranslationArray()); foreach($this->config as $key => $value) { - \OCP\Util::writeLog('user_ldap', 'LDAP: storing key '.$key.' value '.print_r($value, true), \OCP\Util::DEBUG); + \OCP\Util::writeLog('user_ldap', 'LDAP: storing key '.$key. + ' value '.print_r($value, true), \OCP\Util::DEBUG); switch ($key) { case 'ldapAgentPassword': $value = base64_encode($value); @@ -634,7 +635,8 @@ class Connection extends LDAPUtility { if(!$this->config['ldapOverrideMainServer'] && !$this->getFromCache('overrideMainServer')) { $this->doConnect($this->config['ldapHost'], $this->config['ldapPort']); $bindStatus = $this->bind(); - $error = $this->ldap->isResource($this->ldapConnectionRes) ? $this->ldap->errno($this->ldapConnectionRes) : -1; + $error = $this->ldap->isResource($this->ldapConnectionRes) ? + $this->ldap->errno($this->ldapConnectionRes) : -1; } else { $bindStatus = false; $error = null; @@ -692,7 +694,9 @@ class Connection extends LDAPUtility { if(!$this->ldap->isResource($cr)) { return false; } - $ldapLogin = @$this->ldap->bind($cr, $this->config['ldapAgentName'], $this->config['ldapAgentPassword']); + $ldapLogin = @$this->ldap->bind($cr, + $this->config['ldapAgentName'], + $this->config['ldapAgentPassword']); if(!$ldapLogin) { \OCP\Util::writeLog('user_ldap', 'Bind failed: ' . $this->ldap->errno($cr) . ': ' . $this->ldap->error($cr), diff --git a/apps/user_ldap/lib/ildapwrapper.php b/apps/user_ldap/lib/ildapwrapper.php index be35cebe945..9e6bd56ef2a 100644 --- a/apps/user_ldap/lib/ildapwrapper.php +++ b/apps/user_ldap/lib/ildapwrapper.php @@ -177,4 +177,4 @@ interface ILDAPWrapper { */ public function isResource($resource); -} \ No newline at end of file +} diff --git a/apps/user_ldap/lib/ldaputility.php b/apps/user_ldap/lib/ldaputility.php index 975df7d1de0..7fffd9c88d1 100644 --- a/apps/user_ldap/lib/ldaputility.php +++ b/apps/user_ldap/lib/ldaputility.php @@ -33,4 +33,4 @@ abstract class LDAPUtility { public function __construct(ILDAPWrapper $ldapWrapper) { $this->ldap = $ldapWrapper; } -} \ No newline at end of file +} diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php index f2e99d45794..6f52bbdf233 100644 --- a/apps/user_ldap/user_ldap.php +++ b/apps/user_ldap/user_ldap.php @@ -79,7 +79,8 @@ class USER_LDAP extends BackendUtility implements \OCP\UserInterface { */ public function checkPassword($uid, $password) { //find out dn of the user name - $filter = \OCP\Util::mb_str_replace('%uid', $uid, $this->access->connection->ldapLoginFilter, 'UTF-8'); + $filter = \OCP\Util::mb_str_replace( + '%uid', $uid, $this->access->connection->ldapLoginFilter, 'UTF-8'); $ldap_users = $this->access->fetchListOfUsers($filter, 'dn'); if(count($ldap_users) < 1) { return false; @@ -158,7 +159,8 @@ class USER_LDAP extends BackendUtility implements \OCP\UserInterface { //getting dn, if false the user does not exist. If dn, he may be mapped only, requires more checking. $dn = $this->access->username2dn($uid); if(!$dn) { - \OCP\Util::writeLog('user_ldap', 'No DN found for '.$uid.' on '.$this->access->connection->ldapHost, \OCP\Util::DEBUG); + \OCP\Util::writeLog('user_ldap', 'No DN found for '.$uid.' on '. + $this->access->connection->ldapHost, \OCP\Util::DEBUG); $this->access->connection->writeToCache('userExists'.$uid, false); return false; } diff --git a/apps/user_ldap/user_proxy.php b/apps/user_ldap/user_proxy.php index c7b469c8224..092fdbf7c78 100644 --- a/apps/user_ldap/user_proxy.php +++ b/apps/user_ldap/user_proxy.php @@ -36,7 +36,8 @@ class User_Proxy extends lib\Proxy implements \OCP\UserInterface { public function __construct($serverConfigPrefixes, ILDAPWrapper $ldap) { parent::__construct($ldap); foreach($serverConfigPrefixes as $configPrefix) { - $this->backends[$configPrefix] = new \OCA\user_ldap\USER_LDAP($this->getAccess($configPrefix)); + $this->backends[$configPrefix] = + new \OCA\user_ldap\USER_LDAP($this->getAccess($configPrefix)); if(is_null($this->refBackend)) { $this->refBackend = &$this->backends[$configPrefix]; } From a1aff3e8a1b956e0e42ceca2b6c7b6233b195432 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 13 Sep 2013 19:27:07 +0200 Subject: [PATCH 13/16] LDAP: don't throw exceptions in the wrapper, errors are handled in the code. nevertheless, log unexpected errors --- apps/user_ldap/lib/ldap.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/user_ldap/lib/ldap.php b/apps/user_ldap/lib/ldap.php index ee2e336cc95..b63e969912a 100644 --- a/apps/user_ldap/lib/ldap.php +++ b/apps/user_ldap/lib/ldap.php @@ -152,9 +152,11 @@ class LDAP implements ILDAPWrapper { } else if ($errorCode === 10) { //referrals, we switch them off, but then there is AD :) } else { - throw new \Exception('LDAP error '.$errorMsg.' (' . - $errorCode.') after calling '.$this->curFunc. - ' with arguments '.print_r($this->curArgs, true)); + \OCP\Util::writeLog('user_ldap', + 'LDAP error '.$errorMsg.' (' . + $errorCode.') after calling '. + $this->curFunc, + \OCP\Util::DEBUG); } } } From 2cebd8b92e35232b50fe8c9083b718e895ad3211 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 25 Sep 2013 22:14:02 +0200 Subject: [PATCH 14/16] added user_ldap tests via public api calls --- apps/user_ldap/tests/user_ldap_direct.php | 126 +++++++++++++++++++--- 1 file changed, 112 insertions(+), 14 deletions(-) diff --git a/apps/user_ldap/tests/user_ldap_direct.php b/apps/user_ldap/tests/user_ldap_direct.php index ab0563f12d6..6b9b8b3e185 100644 --- a/apps/user_ldap/tests/user_ldap_direct.php +++ b/apps/user_ldap/tests/user_ldap_direct.php @@ -75,15 +75,14 @@ class Test_User_Ldap_Direct extends \PHPUnit_Framework_TestCase { return false; } })); - - } - public function testCheckPassword() { - $access = $this->getAccessMock(); - $backend = new UserLDAP($access); - \OC_User::useBackend($backend); - + /** + * @brief Prepares the Access mock for checkPassword tests + * @param $access mock of \OCA\user_ldap\lib\Access + * @return void + */ + private function prepareAccessForCheckPassword(&$access) { $access->connection->expects($this->any()) ->method('__get') ->will($this->returnCallback(function($name) { @@ -115,6 +114,13 @@ class Test_User_Ldap_Direct extends \PHPUnit_Framework_TestCase { } return false; })); + } + + public function testCheckPassword() { + $access = $this->getAccessMock(); + $this->prepareAccessForCheckPassword($access); + $backend = new UserLDAP($access); + \OC_User::useBackend($backend); $result = $backend->checkPassword('roland', 'dt19'); $this->assertEquals('gunslinger', $result); @@ -126,10 +132,28 @@ class Test_User_Ldap_Direct extends \PHPUnit_Framework_TestCase { $this->assertFalse($result); } - public function testGetUsers() { + public function testCheckPasswordPublicAPI() { $access = $this->getAccessMock(); + $this->prepareAccessForCheckPassword($access); $backend = new UserLDAP($access); + \OC_User::useBackend($backend); + $result = \OCP\User::checkPassword('roland', 'dt19'); + $this->assertEquals('gunslinger', $result); + + $result = \OCP\User::checkPassword('roland', 'wrong'); + $this->assertFalse($result); + + $result = \OCP\User::checkPassword('mallory', 'evil'); + $this->assertFalse($result); + } + + /** + * @brief Prepares the Access mock for getUsers tests + * @param $access mock of \OCA\user_ldap\lib\Access + * @return void + */ + private function prepareAccessForGetUsers(&$access) { $access->expects($this->any()) ->method('getFilterPartForUserSearch') ->will($this->returnCallback(function($search) { @@ -165,6 +189,12 @@ class Test_User_Ldap_Direct extends \PHPUnit_Framework_TestCase { $access->expects($this->any()) ->method('ownCloudUserNames') ->will($this->returnArgument(0)); + } + + public function testGetUsers() { + $access = $this->getAccessMock(); + $this->prepareAccessForGetUsers($access); + $backend = new UserLDAP($access); $result = $backend->getUsers(); $this->assertEquals(3, count($result)); @@ -182,6 +212,28 @@ class Test_User_Ldap_Direct extends \PHPUnit_Framework_TestCase { $this->assertEquals(0, count($result)); } + public function testGetUsersViaAPI() { + $access = $this->getAccessMock(); + $this->prepareAccessForGetUsers($access); + $backend = new UserLDAP($access); + \OC_User::useBackend($backend); + + $result = \OCP\User::getUsers(); + $this->assertEquals(3, count($result)); + + $result = \OCP\User::getUsers('', 1, 2); + $this->assertEquals(1, count($result)); + + $result = \OCP\User::getUsers('', 2, 1); + $this->assertEquals(2, count($result)); + + $result = \OCP\User::getUsers('yo'); + $this->assertEquals(2, count($result)); + + $result = \OCP\User::getUsers('nix'); + $this->assertEquals(0, count($result)); + } + public function testUserExists() { $access = $this->getAccessMock(); $backend = new UserLDAP($access); @@ -209,6 +261,34 @@ class Test_User_Ldap_Direct extends \PHPUnit_Framework_TestCase { $this->assertFalse($result); } + public function testUserExistsPublicAPI() { + $access = $this->getAccessMock(); + $backend = new UserLDAP($access); + $this->prepareMockForUserExists($access); + \OC_User::useBackend($backend); + + $access->expects($this->any()) + ->method('readAttribute') + ->will($this->returnCallback(function($dn) { + if($dn === 'dnOfRoland') { + return array(); + } + return false; + })); + + //test for existing user + $result = \OCP\User::userExists('gunslinger'); + $this->assertTrue($result); + + //test for deleted user + $result = \OCP\User::userExists('formerUser'); + $this->assertFalse($result); + + //test for never-existing user + $result = \OCP\User::userExists('mallory'); + $this->assertFalse($result); + } + public function testDeleteUser() { $access = $this->getAccessMock(); $backend = new UserLDAP($access); @@ -268,11 +348,7 @@ class Test_User_Ldap_Direct extends \PHPUnit_Framework_TestCase { $this->assertFalse($result); } - public function testGetDisplayName() { - $access = $this->getAccessMock(); - $backend = new UserLDAP($access); - $this->prepareMockForUserExists($access); - + private function prepareAccessForGetDisplayName(&$access) { $access->connection->expects($this->any()) ->method('__get') ->will($this->returnCallback(function($name) { @@ -297,6 +373,13 @@ class Test_User_Ldap_Direct extends \PHPUnit_Framework_TestCase { return false; } })); + } + + public function testGetDisplayName() { + $access = $this->getAccessMock(); + $this->prepareAccessForGetDisplayName($access); + $backend = new UserLDAP($access); + $this->prepareMockForUserExists($access); //with displayName $result = $backend->getDisplayName('gunslinger'); @@ -307,7 +390,22 @@ class Test_User_Ldap_Direct extends \PHPUnit_Framework_TestCase { $this->assertEquals(null, $result); } + public function testGetDisplayNamePublicAPI() { + $access = $this->getAccessMock(); + $this->prepareAccessForGetDisplayName($access); + $backend = new UserLDAP($access); + $this->prepareMockForUserExists($access); + \OC_User::useBackend($backend); + + //with displayName + $result = \OCP\User::getDisplayName('gunslinger'); + $this->assertEquals('Roland Deschain', $result); + + //empty displayname retrieved + $result = \OCP\User::getDisplayName('newyorker'); + $this->assertEquals('newyorker', $result); + } + //no test for getDisplayNames, because it just invokes getUsers and //getDisplayName - } \ No newline at end of file From 05ee6606f10d7b6e7b0c5558aa28c6c8c6380923 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 25 Sep 2013 22:14:35 +0200 Subject: [PATCH 15/16] give test file a proper name --- apps/user_ldap/tests/{user_ldap_direct.php => user_ldap.php} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename apps/user_ldap/tests/{user_ldap_direct.php => user_ldap.php} (100%) diff --git a/apps/user_ldap/tests/user_ldap_direct.php b/apps/user_ldap/tests/user_ldap.php similarity index 100% rename from apps/user_ldap/tests/user_ldap_direct.php rename to apps/user_ldap/tests/user_ldap.php From 3e1bdd8a041886319daea4ad7867d1994724b311 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 25 Sep 2013 22:16:07 +0200 Subject: [PATCH 16/16] remove outdated (thus failing) and anyway insufficient group tests - for now --- apps/user_ldap/tests/group_ldap.php | 46 ----------------------------- 1 file changed, 46 deletions(-) delete mode 100644 apps/user_ldap/tests/group_ldap.php diff --git a/apps/user_ldap/tests/group_ldap.php b/apps/user_ldap/tests/group_ldap.php deleted file mode 100644 index ae635597b71..00000000000 --- a/apps/user_ldap/tests/group_ldap.php +++ /dev/null @@ -1,46 +0,0 @@ -. -* -*/ - -class Test_Group_Ldap extends PHPUnit_Framework_TestCase { - function setUp() { - OC_Group::clearBackends(); - } - - function testSingleBackend() { - OC_Group::useBackend(new OCA\user_ldap\GROUP_LDAP()); - $group_ldap = new OCA\user_ldap\GROUP_LDAP(); - - $this->assertIsA(OC_Group::getGroups(), gettype(array())); - $this->assertIsA($group_ldap->getGroups(), gettype(array())); - - $this->assertFalse(OC_Group::inGroup('john', 'dosers'), gettype(false)); - $this->assertFalse($group_ldap->inGroup('john', 'dosers'), gettype(false)); - //TODO: check also for expected true result. This backend won't be able to do any modifications, maybe use a dummy for this. - - $this->assertIsA(OC_Group::getUserGroups('john doe'), gettype(array())); - $this->assertIsA($group_ldap->getUserGroups('john doe'), gettype(array())); - - $this->assertIsA(OC_Group::usersInGroup('campers'), gettype(array())); - $this->assertIsA($group_ldap->usersInGroup('campers'), gettype(array())); - } - -}