Merge pull request #11837 from owncloud/fix-11328

unify count filters and introduce display name attribute detection

apps/user_ldap/ajax/wizard.php

@@ -62,6 +62,7 @@ switch($action) {
 	case 'guessPortAndTLS':
 	case 'guessBaseDN':
 	case 'detectEmailAttribute':
+	case 'detectUserDisplayNameAttribute':
 	case 'determineGroupMemberAssoc':
 	case 'determineUserObjectClasses':
 	case 'determineGroupObjectClasses':
@@ -115,4 +116,3 @@ switch($action) {
 		//TODO: return 4xx error
 		break;
 }
-

apps/user_ldap/js/ldapFilter.js

@@ -8,6 +8,7 @@ function LdapFilter(target, determineModeCallback) {
 	this.determineModeCallback = determineModeCallback;
 	this.foundFeatures = false;
 	this.activated = false;
+	this.countPending = false;
 
 	if( target === 'User' ||
 		target === 'Login' ||
@@ -25,9 +26,13 @@ LdapFilter.prototype.activate = function() {
 	this.determineMode();
 };
 
-LdapFilter.prototype.compose = function(callback) {
+LdapFilter.prototype.compose = function(updateCount) {
 	var action;
 
+	if(updateCount === true) {
+		this.countPending = updateCount;
+	}
+
 	if(this.locked) {
 		this.lazyRunCompose = true;
 		return false;
@@ -54,22 +59,36 @@ LdapFilter.prototype.compose = function(callback) {
 
 	LdapWizard.ajax(param,
 		function(result) {
-			LdapWizard.applyChanges(result);
-			filter.updateCount();
-			if(filter.target === 'Group') {
-				LdapWizard.detectGroupMemberAssoc();
-			}
-			if(typeof callback !== 'undefined') {
-				callback();
-			}
+			filter.afterComposeSuccess(result);
 		},
 		function () {
+			filter.countPending = false;
 			console.log('LDAP Wizard: could not compose filter. '+
 				'Please check owncloud.log');
 		}
 	);
 };
 
+/**
+ * this function is triggered after attribute detectors have completed in
+ * LdapWizard
+ */
+LdapFilter.prototype.afterDetectorsRan = function() {
+	this.updateCount();
+};
+
+/**
+ * this function is triggered after LDAP filters have been composed successfully
+ * @param {object} result returned by the ajax call
+ */
+LdapFilter.prototype.afterComposeSuccess = function(result) {
+	LdapWizard.applyChanges(result);
+	if(this.countPending) {
+		this.countPending = false;
+		this.updateCount();
+	}
+};
+
 LdapFilter.prototype.determineMode = function() {
 	var param = 'action=get'+encodeURIComponent(this.target)+'FilterMode'+
 		'&ldap_serverconfig_chooser='+
@@ -145,10 +164,26 @@ LdapFilter.prototype.findFeatures = function() {
 	}
 };
 
+/**
+ * this function is triggered before user and group counts are executed
+ * resolving the passed status variable will fire up counting
+ * @param {object} status an instance of $.Deferred
+ */
+LdapFilter.prototype.beforeUpdateCount = function() {
+	var status = $.Deferred();
+	LdapWizard.runDetectors(this.target, function() {
+		status.resolve();
+	});
+	return status;
+};
+
 LdapFilter.prototype.updateCount = function(doneCallback) {
-	if(this.target === 'User') {
-		LdapWizard.countUsers(doneCallback);
-	} else if (this.target === 'Group') {
-		LdapWizard.countGroups(doneCallback);
-	}
+	var filter = this;
+	$.when(this.beforeUpdateCount()).done(function() {
+		if(filter.target === 'User') {
+			LdapWizard.countUsers(doneCallback);
+		} else if (filter.target === 'Group') {
+			LdapWizard.countGroups(doneCallback);
+		}
+	});
 };

apps/user_ldap/js/settings.js

@@ -151,8 +151,10 @@ var LdapWizard = {
 	ajaxRequests: {},
 
 	ajax: function(param, fnOnSuccess, fnOnError, reqID) {
-		if(reqID !== undefined) {
+		if(!_.isUndefined(reqID)) {
 			if(LdapWizard.ajaxRequests.hasOwnProperty(reqID)) {
+				console.log('aborting ' + reqID);
+				console.log(param);
 				LdapWizard.ajaxRequests[reqID].abort();
 			}
 		}
@@ -167,9 +169,10 @@ var LdapWizard = {
 				}
 			}
 		);
-		if(reqID !== undefined) {
+		if(!_.isUndefined(reqID)) {
 			LdapWizard.ajaxRequests[reqID] = request;
 		}
+		return request;
 	},
 
 	applyChanges: function (result) {
@@ -342,7 +345,7 @@ var LdapWizard = {
 	},
 
 	_countThings: function(method, spinnerID, doneCallback) {
-		param = 'action='+method+
+		var param = 'action='+method+
 				'&ldap_serverconfig_chooser='+
 				encodeURIComponent($('#ldap_serverconfig_chooser').val());
 
@@ -351,14 +354,14 @@ var LdapWizard = {
 			function(result) {
 				LdapWizard.applyChanges(result);
 				LdapWizard.hideSpinner(spinnerID);
-				if(doneCallback !== undefined) {
+				if(!_.isUndefined(doneCallback)) {
 					doneCallback(method);
 				}
 			},
 			function (result) {
 				OC.Notification.show('Counting the entries failed with, ' + result.message);
 				LdapWizard.hideSpinner(spinnerID);
-				if(doneCallback !== undefined) {
+				if(!_.isUndefined(doneCallback)) {
 					doneCallback(method);
 				}
 			},
@@ -374,12 +377,54 @@ var LdapWizard = {
 		LdapWizard._countThings('countUsers', '#ldap_user_count', doneCallback);
 	},
 
+	/**
+	 * called after detectors have run
+	 * @callback runDetectorsCallback
+	 */
+
+	/**
+	 * runs detectors to determine appropriate attributes, e.g. displayName
+	 * @param {string} type either "User" or "Group"
+	 * @param {runDetectorsCallback} triggered after all detectors have completed
+	 */
+	runDetectors: function(type, callback) {
+		if(type === 'Group') {
+			$.when(LdapWizard.detectGroupMemberAssoc())
+				.then(callback, callback);
+			if(   LdapWizard.admin.isExperienced
+			   && !(LdapWizard.detectorsRunInXPMode & LdapWizard.groupDetectors)) {
+				LdapWizard.detectorsRunInXPMode += LdapWizard.groupDetectors;
+			}
+		} else if(type === 'User') {
+			var req1 = LdapWizard.detectUserDisplayNameAttribute();
+			var req2 = LdapWizard.detectEmailAttribute();
+			$.when(req1, req2)
+				.then(callback, callback);
+			if(   LdapWizard.admin.isExperienced
+			   && !(LdapWizard.detectorsRunInXPMode & LdapWizard.userDetectors)) {
+				LdapWizard.detectorsRunInXPMode += LdapWizard.userDetectors;
+			}
+		}
+	},
+
+	/**
+	 * runs detector to find out a fitting user display name attribute
+	 */
+	detectUserDisplayNameAttribute: function() {
+		var param = 'action=detectUserDisplayNameAttribute' +
+			'&ldap_serverconfig_chooser='+
+			encodeURIComponent($('#ldap_serverconfig_chooser').val());
+
+		//runs in the background, no callbacks necessary
+		return LdapWizard.ajax(param, LdapWizard.applyChanges, function(){}, 'detectUserDisplayNameAttribute');
+	},
+
 	detectEmailAttribute: function() {
-		param = 'action=detectEmailAttribute'+
+		var param = 'action=detectEmailAttribute'+
 				'&ldap_serverconfig_chooser='+
 				encodeURIComponent($('#ldap_serverconfig_chooser').val());
 		//runs in the background, no callbacks necessary
-		LdapWizard.ajax(param, LdapWizard.applyChanges, function(){}, 'detectEmailAttribute');
+		return LdapWizard.ajax(param, LdapWizard.applyChanges, function(){}, 'detectEmailAttribute');
 	},
 
 	detectGroupMemberAssoc: function() {
@@ -387,7 +432,7 @@ var LdapWizard = {
 				'&ldap_serverconfig_chooser='+
 				encodeURIComponent($('#ldap_serverconfig_chooser').val());
 
-		LdapWizard.ajax(param,
+		return LdapWizard.ajax(param,
 			function(result) {
 				//pure background story
 			},
@@ -409,7 +454,7 @@ var LdapWizard = {
 				$('#ldap_loginfilter_attributes').find('option').remove();
 				for (var i in result.options['ldap_loginfilter_attributes']) {
 					//FIXME: move HTML into template
-					attr = result.options['ldap_loginfilter_attributes'][i];
+					var attr = result.options['ldap_loginfilter_attributes'][i];
 					$('#ldap_loginfilter_attributes').append(
 								"<option value='"+attr+"'>"+attr+"</option>");
 				}
@@ -566,8 +611,12 @@ var LdapWizard = {
 	},
 
 	isConfigurationActiveControlLocked: true,
+	detectorsRunInXPMode: 0,
+	userDetectors: 1,
+	groupDetectors: 2,
 
 	init: function() {
+		LdapWizard.detectorsRunInXPMode = 0;
 		LdapWizard.instantiateFilters();
 		LdapWizard.admin.setExperienced($('#ldap_experienced_admin').is(':checked'));
 		LdapWizard.basicStatusCheck();
@@ -620,8 +669,9 @@ var LdapWizard = {
 	instantiateFilters: function() {
 		delete LdapWizard.userFilter;
 		LdapWizard.userFilter = new LdapFilter('User', function(mode) {
-			if(mode === LdapWizard.filterModeAssisted) {
-				LdapWizard.groupFilter.updateCount();
+			if( !LdapWizard.admin.isExperienced()
+			   || mode === LdapWizard.filterModeAssisted) {
+				LdapWizard.userFilter.updateCount();
 			}
 			LdapWizard.userFilter.findFeatures();
 		});
@@ -630,7 +680,6 @@ var LdapWizard = {
 			$('#ldap_user_count').text('');
 			LdapWizard.showSpinner('#rawUserFilterContainer .ldapGetEntryCount');
 			LdapWizard.userFilter.updateCount(LdapWizard.hideTestSpinner);
-			LdapWizard.detectEmailAttribute();
 			$('#ldap_user_count').removeClass('hidden');
 		});
 
@@ -641,7 +690,8 @@ var LdapWizard = {
 
 		delete LdapWizard.groupFilter;
 		LdapWizard.groupFilter = new LdapFilter('Group', function(mode) {
-			if(mode === LdapWizard.filterModeAssisted) {
+			if( !LdapWizard.admin.isExperienced()
+			   || mode === LdapWizard.filterModeAssisted) {
 				LdapWizard.groupFilter.updateCount();
 			}
 			LdapWizard.groupFilter.findFeatures();
@@ -651,7 +701,6 @@ var LdapWizard = {
 			$('#ldap_group_count').text('');
 			LdapWizard.showSpinner('#rawGroupFilterContainer .ldapGetEntryCount');
 			LdapWizard.groupFilter.updateCount(LdapWizard.hideTestSpinner);
-			LdapWizard.detectGroupMemberAssoc();
 			$('#ldap_group_count').removeClass('hidden');
 		});
 	},
@@ -668,7 +717,7 @@ var LdapWizard = {
 	postInitUserFilter: function() {
 		if(LdapWizard.userFilterObjectClassesHasRun &&
 			LdapWizard.userFilterAvailableGroupsHasRun) {
-			LdapWizard.userFilter.compose(LdapWizard.detectEmailAttribute);
+			LdapWizard.userFilter.compose();
 		}
 	},
 
@@ -679,7 +728,7 @@ var LdapWizard = {
 			//do not allow to switch tabs as long as a save process is active
 			return false;
 		}
-		newTabIndex = 0;
+		var newTabIndex = 0;
 		if(ui.newTab[0].id === '#ldapWizard2') {
 			LdapWizard.initUserFilter();
 			newTabIndex = 1;
@@ -691,9 +740,23 @@ var LdapWizard = {
 			newTabIndex = 3;
 		}
 
-		curTabIndex = $('#ldapSettings').tabs('option', 'active');
+		var curTabIndex = $('#ldapSettings').tabs('option', 'active');
 		if(curTabIndex >= 0 && curTabIndex <= 3) {
 			LdapWizard.controlUpdate(newTabIndex);
+			//run detectors in XP mode, when "Test Filter" button has not been
+			//clicked in order to make sure that email, displayname, member-
+			//group association attributes are properly set.
+			if(   curTabIndex === 1
+			   && LdapWizard.admin.isExperienced
+			   && !(LdapWizard.detecorsRunInXPMode & LdapWizard.userDetectors)
+			) {
+				LdapWizard.runDetectors('User', function(){});
+			} else if(   curTabIndex === 3
+			          && LdapWizard.admin.isExperienced
+			          && !(LdapWizard.detecorsRunInXPMode & LdapWizard.groupDetectors)
+			) {
+				LdapWizard.runDetectors('Group', function(){});
+			}
 		}
 	},
 
@@ -711,15 +774,15 @@ var LdapWizard = {
 			}
 		}
 
-		if(triggerObj.id === 'ldap_userlist_filter' && !LdapWizard.admin.isExperienced()) {
-			LdapWizard.detectEmailAttribute();
-		} else if(triggerObj.id === 'ldap_group_filter' && !LdapWizard.admin.isExperienced()) {
-			LdapWizard.detectGroupMemberAssoc();
-		}
-
 		if(triggerObj.id === 'ldap_loginfilter_username'
 		   || triggerObj.id === 'ldap_loginfilter_email') {
 			LdapWizard.loginFilter.compose();
+		} else if (!LdapWizard.admin.isExperienced()) {
+			if(triggerObj.id === 'ldap_userlist_filter') {
+				LdapWizard.userFilter.updateCount();
+			} else if (triggerObj.id === 'ldap_group_filter') {
+				LdapWizard.groupFilter.updateCount();
+			}
 		}
 
 		if($('#ldapSettings').tabs('option', 'active') == 0) {
@@ -749,7 +812,7 @@ var LdapWizard = {
 		LdapWizard._save($('#'+originalObj)[0], $.trim(values));
 		if(originalObj === 'ldap_userfilter_objectclass'
 		   || originalObj === 'ldap_userfilter_groups') {
-			LdapWizard.userFilter.compose(LdapWizard.detectEmailAttribute);
+			LdapWizard.userFilter.compose(true);
 			//when user filter is changed afterwards, login filter needs to
 			//be adjusted, too
 			if(!LdapWizard.loginFilter) {
@@ -760,7 +823,7 @@ var LdapWizard = {
 			LdapWizard.loginFilter.compose();
 		} else if(originalObj === 'ldap_groupfilter_objectclass'
 		   || originalObj === 'ldap_groupfilter_groups') {
-			LdapWizard.groupFilter.compose();
+			LdapWizard.groupFilter.compose(true);
 		}
 	},
 
@@ -830,10 +893,10 @@ var LdapWizard = {
 			LdapWizard._save({ id: modeKey }, LdapWizard.filterModeAssisted);
 			if(isUser) {
 				LdapWizard.blacklistRemove('ldap_userlist_filter');
-				LdapWizard.userFilter.compose(LdapWizard.detectEmailAttribute);
+				LdapWizard.userFilter.compose(true);
 			} else {
 				LdapWizard.blacklistRemove('ldap_group_filter');
-				LdapWizard.groupFilter.compose();
+				LdapWizard.groupFilter.compose(true);
 			}
 		}
 	},

apps/user_ldap/lib/access.php

@@ -813,7 +813,7 @@ class Access extends LDAPUtility implements user\IUserTools {
 		}
 
 		//check whether paged search should be attempted
-		$pagedSearchOK = $this->initPagedSearch($filter, $base, $attr, $limit, $offset);
+		$pagedSearchOK = $this->initPagedSearch($filter, $base, $attr, intval($limit), $offset);
 
 		$linkResources = array_pad(array(), count($base), $cr);
 		$sr = $this->ldap->search($linkResources, $base, $filter, $attr);
@@ -887,8 +887,9 @@ class Access extends LDAPUtility implements user\IUserTools {
 	private function count($filter, $base, $attr = null, $limit = null, $offset = null, $skipHandling = false) {
 		\OCP\Util::writeLog('user_ldap', 'Count filter:  '.print_r($filter, true), \OCP\Util::DEBUG);
 
-		if(is_null($limit) || $limit <= 0) {
-			$limit = intval($this->connection->ldapPagingSize);
+		$limitPerPage = intval($this->connection->ldapPagingSize);
+		if(!is_null($limit) && $limit < $limitPerPage && $limit > 0) {
+			$limitPerPage = $limit;
 		}
 
 		$counter = 0;
@@ -898,19 +899,19 @@ class Access extends LDAPUtility implements user\IUserTools {
 		do {
 			$continue = false;
 			$search = $this->executeSearch($filter, $base, $attr,
-										   $limit, $offset);
+										   $limitPerPage, $offset);
 			if($search === false) {
 				return $counter > 0 ? $counter : false;
 			}
 			list($sr, $pagedSearchOK) = $search;
 
-			$count = $this->countEntriesInSearchResults($sr, $limit, $continue);
+			$count = $this->countEntriesInSearchResults($sr, $limitPerPage, $continue);
 			$counter += $count;
 
-			$this->processPagedSearchStatus($sr, $filter, $base, $count, $limit,
+			$this->processPagedSearchStatus($sr, $filter, $base, $count, $limitPerPage,
 										$offset, $pagedSearchOK, $skipHandling);
-			$offset += $limit;
-		} while($continue);
+			$offset += $limitPerPage;
+		} while($continue && (is_null($limit) || $limit <= 0 || $limit > $counter));
 
 		return $counter;
 	}
@@ -1167,6 +1168,19 @@ class Access extends LDAPUtility implements user\IUserTools {
 		return $this->combineFilterWithOr($filter);
 	}
 
+	/**
+	 * returns the filter used for counting users
+	 * @return string
+	 */
+	public function getFilterForUserCount() {
+		$filter = $this->combineFilterWithAnd(array(
+			$this->connection->ldapUserFilter,
+			$this->connection->ldapUserDisplayName . '=*'
+		));
+
+		return $filter;
+	}
+
 	/**
 	 * @param string $name
 	 * @param string $password
@@ -1457,7 +1471,7 @@ class Access extends LDAPUtility implements user\IUserTools {
 	 */
 	private function initPagedSearch($filter, $bases, $attr, $limit, $offset) {
 		$pagedSearchOK = false;
-		if($this->connection->hasPagedResultSupport && !is_null($limit)) {
+		if($this->connection->hasPagedResultSupport && ($limit !== 0)) {
 			$offset = intval($offset); //can be null
 			\OCP\Util::writeLog('user_ldap',
 				'initializing paged search for  Filter '.$filter.' base '.print_r($bases, true)

apps/user_ldap/lib/wizard.php

@@ -56,7 +56,7 @@ class Wizard extends LDAPUtility {
 			Wizard::$l = \OC::$server->getL10N('user_ldap');
 		}
 		$this->access = $access;
-		$this->result = new WizardResult;
+		$this->result = new WizardResult();
 	}
 
 	public function  __destruct() {
@@ -120,7 +120,7 @@ class Wizard extends LDAPUtility {
 	 * @throws \Exception
 	 */
 	public function countUsers() {
-		$filter = $this->configuration->ldapUserFilter;
+		$filter = $this->access->getFilterForUserCount();
 
 		$usersTotal = $this->countEntries($filter, 'users');
 		$usersTotal = ($usersTotal !== false) ? $usersTotal : 0;
@@ -132,9 +132,10 @@ class Wizard extends LDAPUtility {
 	/**
 	 * counts users with a specified attribute
 	 * @param string $attr
+	 * @param bool $existsCheck
 	 * @return int|bool
 	 */
-	public function countUsersWithAttribute($attr) {
+	public function countUsersWithAttribute($attr, $existsCheck = false) {
 		if(!$this->checkRequirements(array('ldapHost',
 										   'ldapPort',
 										   'ldapBase',
@@ -148,7 +149,51 @@ class Wizard extends LDAPUtility {
 			$attr . '=*'
 		));
 
-		return $this->access->countUsers($filter);
+		$limit = ($existsCheck === false) ? null : 1;
+
+		return $this->access->countUsers($filter, array('dn'), $limit);
+	}
+
+	/**
+	 * detects the display name attribute. If a setting is already present that
+	 * returns at least one hit, the detection will be canceled.
+	 * @return WizardResult|bool
+	 * @throws \Exception
+	 */
+	public function detectUserDisplayNameAttribute() {
+		if(!$this->checkRequirements(array('ldapHost',
+										'ldapPort',
+										'ldapBase',
+										'ldapUserFilter',
+										))) {
+			return  false;
+		}
+
+		$attr = $this->configuration->ldapUserDisplayName;
+		if($attr !== 'displayName' && !empty($attr)) {
+			// most likely not the default value with upper case N,
+			// verify it still produces a result
+			$count = intval($this->countUsersWithAttribute($attr, true));
+			if($count > 0) {
+				//no change, but we sent it back to make sure the user interface
+				//is still correct, even if the ajax call was cancelled inbetween
+				$this->result->addChange('ldap_display_name', $attr);
+				return $this->result;
+			}
+		}
+
+		// first attribute that has at least one result wins
+		$displayNameAttrs = array('displayname', 'cn');
+		foreach ($displayNameAttrs as $attr) {
+			$count = intval($this->countUsersWithAttribute($attr, true));
+
+			if($count > 0) {
+				$this->applyFind('ldap_display_name', $attr);
+				return $this->result;
+			}
+		};
+
+		throw new \Exception(self::$l->t('Could not detect user display name attribute. Please specify it yourself in advanced ldap settings.'));
 	}
 
 	/**
@@ -168,7 +213,7 @@ class Wizard extends LDAPUtility {
 
 		$attr = $this->configuration->ldapEmailAttribute;
 		if(!empty($attr)) {
-			$count = intval($this->countUsersWithAttribute($attr));
+			$count = intval($this->countUsersWithAttribute($attr, true));
 			if($count > 0) {
 				return false;
 			}
@@ -189,7 +234,7 @@ class Wizard extends LDAPUtility {
 		}
 
 		if($winner !== '') {
-			$this->result->addChange('ldap_email_attr', $winner);
+			$this->applyFind('ldap_email_attr', $winner);
 			if($writeLog) {
 				\OCP\Util::writeLog('user_ldap', 'The mail attribute has ' .
 					'automatically been reset, because the original value ' .

apps/user_ldap/tests/user_ldap.php

@@ -552,23 +552,9 @@ class Test_User_Ldap_Direct extends \Test\TestCase {
 	public function testCountUsers() {
 		$access = $this->getAccessMock();
 
-		$access->connection->expects($this->once())
-			   ->method('__get')
-			   ->will($this->returnCallback(function($name) {
-					if($name === 'ldapLoginFilter') {
-						return 'uid=%uid';
-					}
-					return null;
-			   }));
-
 		$access->expects($this->once())
 			   ->method('countUsers')
-			   ->will($this->returnCallback(function($filter, $a, $b, $c) {
-				   if($filter !== 'uid=*') {
-					   return false;
-				   }
-				   return 5;
-			   }));
+			   ->will($this->returnValue(5));
 
 		$backend = new UserLDAP($access);
 
@@ -579,23 +565,9 @@ class Test_User_Ldap_Direct extends \Test\TestCase {
 	public function testCountUsersFailing() {
 		$access = $this->getAccessMock();
 
-		$access->connection->expects($this->once())
-			   ->method('__get')
-			   ->will($this->returnCallback(function($name) {
-					if($name === 'ldapLoginFilter') {
-						return 'invalidFilter';
-					}
-					return null;
-			   }));
-
 		$access->expects($this->once())
 			   ->method('countUsers')
-			   ->will($this->returnCallback(function($filter, $a, $b, $c) {
-				   if($filter !== 'uid=*') {
-					   return false;
-				   }
-				   return 5;
-			   }));
+			   ->will($this->returnValue(false));
 
 		$backend = new UserLDAP($access);
 

apps/user_ldap/user_ldap.php

@@ -290,8 +290,7 @@ class USER_LDAP extends BackendUtility implements \OCP\UserInterface {
 	 * @return int|bool
 	 */
 	public function countUsers() {
-		$filter = \OCP\Util::mb_str_replace(
-			'%uid', '*', $this->access->connection->ldapLoginFilter, 'UTF-8');
+		$filter = $this->access->getFilterForUserCount();
 		$entries = $this->access->countUsers($filter);
 		return $entries;
 	}