From 1ed61328991ce1071fc956a18ea3159405b0e6f3 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 27 Nov 2015 23:46:26 +0100 Subject: [PATCH] LDAP: do not attempt to process user records without display name, fixes fatal error --- apps/user_ldap/lib/access.php | 8 +++ ...ntegrationtestbatchapplyuserattributes.php | 72 +++++++++++++++++++ .../createUsersWithoutDisplayName.php | 59 +++++++++++++++ apps/user_ldap/tests/user_ldap.php | 2 +- apps/user_ldap/user_ldap.php | 1 + 5 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 apps/user_ldap/tests/integration/lib/integrationtestbatchapplyuserattributes.php create mode 100644 apps/user_ldap/tests/integration/setup-scripts/createUsersWithoutDisplayName.php diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 7d2d355f9b..b1e3a4c044 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -706,8 +706,16 @@ class Access extends LDAPUtility implements user\IUserTools { * @param array $ldapRecords */ public function batchApplyUserAttributes(array $ldapRecords){ + $displayNameAttribute = strtolower($this->connection->ldapUserDisplayName); foreach($ldapRecords as $userRecord) { + if(!isset($userRecord[$displayNameAttribute])) { + // displayName is obligatory + continue; + } $ocName = $this->dn2ocname($userRecord['dn'][0]); + if($ocName === false) { + continue; + } $this->cacheUserExists($ocName); $user = $this->userManager->get($ocName); if($user instanceof OfflineUser) { diff --git a/apps/user_ldap/tests/integration/lib/integrationtestbatchapplyuserattributes.php b/apps/user_ldap/tests/integration/lib/integrationtestbatchapplyuserattributes.php new file mode 100644 index 0000000000..f83b0efd0c --- /dev/null +++ b/apps/user_ldap/tests/integration/lib/integrationtestbatchapplyuserattributes.php @@ -0,0 +1,72 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OCA\user_ldap\tests\integration\lib; + +use OCA\User_LDAP\Mapping\UserMapping; +use OCA\user_ldap\tests\integration\AbstractIntegrationTest; + +require_once __DIR__ . '/../../../../../lib/base.php'; + +class IntegrationTestBatchApplyUserAttributes extends AbstractIntegrationTest { + /** + * prepares the LDAP environment and sets up a test configuration for + * the LDAP backend. + */ + public function init() { + require(__DIR__ . '/../setup-scripts/createExplicitUsers.php'); + require(__DIR__ . '/../setup-scripts/createUsersWithoutDisplayName.php'); + parent::init(); + + $this->mapping = new UserMapping(\OC::$server->getDatabaseConnection()); + $this->mapping->clear(); + $this->access->setUserMapper($this->mapping); + } + + /** + * sets up the LDAP configuration to be used for the test + */ + protected function initConnection() { + parent::initConnection(); + $this->connection->setConfiguration([ + 'ldapUserDisplayName' => 'displayname', + ]); + } + + /** + * indirectly tests whether batchApplyUserAttributes does it job properly, + * when a user without display name is included in the result set from LDAP. + * + * @return bool + */ + protected function case1() { + $result = $this->access->fetchListOfUsers('objectclass=person', 'dn'); + // on the original issue, PHP would emit a fatal error + // – cannot catch it here, but will render the test as unsuccessful + return is_array($result) && !empty($result); + } + +} + +require_once(__DIR__ . '/../setup-scripts/config.php'); +$test = new IntegrationTestBatchApplyUserAttributes($host, $port, $adn, $apwd, $bdn); +$test->init(); +$test->run(); diff --git a/apps/user_ldap/tests/integration/setup-scripts/createUsersWithoutDisplayName.php b/apps/user_ldap/tests/integration/setup-scripts/createUsersWithoutDisplayName.php new file mode 100644 index 0000000000..7f474aaa0e --- /dev/null +++ b/apps/user_ldap/tests/integration/setup-scripts/createUsersWithoutDisplayName.php @@ -0,0 +1,59 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ +if(php_sapi_name() !== 'cli') { + print('Only via CLI, please.'); + exit(1); +} + +include __DIR__ . '/config.php'; + +$cr = ldap_connect($host, $port); +ldap_set_option($cr, LDAP_OPT_PROTOCOL_VERSION, 3); +$ok = ldap_bind($cr, $adn, $apwd); + +if (!$ok) { + die(ldap_error($cr)); +} + +$ouName = 'Users'; +$ouDN = 'ou=' . $ouName . ',' . $bdn; + +$users = ['robot']; + +foreach ($users as $uid) { + $newDN = 'uid=' . $uid . ',' . $ouDN; + $fn = ucfirst($uid); + $sn = ucfirst(str_shuffle($uid)); // not so explicit but it's OK. + + $entry = []; + $entry['cn'] = ucfirst($uid); + $entry['objectclass'][] = 'inetOrgPerson'; + $entry['objectclass'][] = 'person'; + $entry['sn'] = $sn; + $entry['userPassword'] = $uid; + + $ok = ldap_add($cr, $newDN, $entry); + if ($ok) { + echo('created user ' . ': ' . $entry['cn'] . PHP_EOL); + } else { + die(ldap_error($cr)); + } +} diff --git a/apps/user_ldap/tests/user_ldap.php b/apps/user_ldap/tests/user_ldap.php index 3aec2a5ce5..4d1aa146fe 100644 --- a/apps/user_ldap/tests/user_ldap.php +++ b/apps/user_ldap/tests/user_ldap.php @@ -313,7 +313,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { $access->expects($this->any()) ->method('combineFilterWithAnd') ->will($this->returnCallback(function($param) { - return $param[1]; + return $param[2]; })); $access->expects($this->any()) diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php index c568905f3d..50b0345dd7 100644 --- a/apps/user_ldap/user_ldap.php +++ b/apps/user_ldap/user_ldap.php @@ -172,6 +172,7 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn } $filter = $this->access->combineFilterWithAnd(array( $this->access->connection->ldapUserFilter, + $this->access->connection->ldapUserDisplayName . '=*', $this->access->getFilterPartForUserSearch($search) ));