LDAP: do not attempt to process user records without display name, fixes fatal error

Author: blizzz

apps/user_ldap/lib/access.php

@@ -706,8 +706,16 @@ public function fetchListOfUsers($filter, $attr, $limit = null, $offset = null)
 	 * @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) {

apps/user_ldap/tests/integration/lib/integrationtestbatchapplyuserattributes.php

@@ -0,0 +1,72 @@
+<?php
+/**
+ * @author Arthur Schiwon <blizzz@owncloud.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>
+ *
+ */
+
+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();

apps/user_ldap/tests/integration/setup-scripts/createUsersWithoutDisplayName.php

@@ -0,0 +1,59 @@
+<?php
+/**
+ * @author Arthur Schiwon <blizzz@owncloud.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>
+ *
+ */
+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));
+	}
+}

apps/user_ldap/tests/user_ldap.php

@@ -294,7 +294,7 @@ private function prepareAccessForGetUsers(&$access) {
 		$access->expects($this->any())
 			   ->method('combineFilterWithAnd')
 			   ->will($this->returnCallback(function($param) {
-					return $param[1];
+					return $param[2];
 			   }));
 
 		$access->expects($this->any())

apps/user_ldap/user_ldap.php

@@ -170,6 +170,7 @@ public function getUsers($search = '', $limit = 10, $offset = 0) {
 		}
 		$filter = $this->access->combineFilterWithAnd(array(
 			$this->access->connection->ldapUserFilter,
+			$this->access->connection->ldapUserDisplayName . '=*',
 			$this->access->getFilterPartForUserSearch($search)
 		));