diff options
author | Morris Jobke <hey@morrisjobke.de> | 2014-11-24 18:13:01 +0100 |
---|---|---|
committer | Morris Jobke <hey@morrisjobke.de> | 2014-11-24 18:13:01 +0100 |
commit | 02095d4f2052350132631970467c5aedd7cffe0c (patch) | |
tree | eea292d29f6681eadb2ed860a5a303b07dac048b /apps/user_ldap | |
parent | 949d7b3312e51cb7076c6f51852cc7f04ea5ca58 (diff) | |
parent | 9aef83b5798733d718e03a4ef0edc7279db43e59 (diff) | |
download | nextcloud-server-02095d4f2052350132631970467c5aedd7cffe0c.tar.gz nextcloud-server-02095d4f2052350132631970467c5aedd7cffe0c.zip |
Merge pull request #11837 from owncloud/fix-11328
unify count filters and introduce display name attribute detection
Diffstat (limited to 'apps/user_ldap')
-rw-r--r-- | apps/user_ldap/ajax/wizard.php | 2 | ||||
-rw-r--r-- | apps/user_ldap/js/ldapFilter.js | 63 | ||||
-rw-r--r-- | apps/user_ldap/js/settings.js | 117 | ||||
-rw-r--r-- | apps/user_ldap/lib/access.php | 32 | ||||
-rw-r--r-- | apps/user_ldap/lib/wizard.php | 57 | ||||
-rw-r--r-- | apps/user_ldap/tests/user_ldap.php | 32 | ||||
-rw-r--r-- | apps/user_ldap/user_ldap.php | 3 |
7 files changed, 217 insertions, 89 deletions
diff --git a/apps/user_ldap/ajax/wizard.php b/apps/user_ldap/ajax/wizard.php index ef1241b9147..6d7d70c8c5a 100644 --- a/apps/user_ldap/ajax/wizard.php +++ b/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; } - diff --git a/apps/user_ldap/js/ldapFilter.js b/apps/user_ldap/js/ldapFilter.js index bb66c1df2ee..0f7d240adac 100644 --- a/apps/user_ldap/js/ldapFilter.js +++ b/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); + } + }); }; diff --git a/apps/user_ldap/js/settings.js b/apps/user_ldap/js/settings.js index fa40aba73b4..6db210fe435 100644 --- a/apps/user_ldap/js/settings.js +++ b/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); } } }, diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index d89029abf17..6f28a87d30c 100644 --- a/apps/user_ldap/lib/access.php +++ b/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; } @@ -1168,6 +1169,19 @@ class Access extends LDAPUtility implements user\IUserTools { } /** + * 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 * @return bool @@ -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) diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php index 1d7701440e9..578a920f00e 100644 --- a/apps/user_ldap/lib/wizard.php +++ b/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 ' . diff --git a/apps/user_ldap/tests/user_ldap.php b/apps/user_ldap/tests/user_ldap.php index e91404d47f2..33cec0247b6 100644 --- a/apps/user_ldap/tests/user_ldap.php +++ b/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); diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php index 6e244311d4a..c2f87ebeb22 100644 --- a/apps/user_ldap/user_ldap.php +++ b/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; } |