From cb3af1dce294ab058162b3c4d1a0feb84465dd0b Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 24 Oct 2014 18:26:48 +0200 Subject: [PATCH 01/14] detect user display name attribute and return user count depending on its presence --- apps/user_ldap/ajax/wizard.php | 2 +- apps/user_ldap/js/settings.js | 15 ++++++++--- apps/user_ldap/lib/wizard.php | 49 ++++++++++++++++++++++++++++++++-- 3 files changed, 59 insertions(+), 7 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/settings.js b/apps/user_ldap/js/settings.js index fa40aba73b4..93fd97c1e73 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(typeof reqID !== 'undefined') { if(LdapWizard.ajaxRequests.hasOwnProperty(reqID)) { + console.log('aborting ' + reqID); + console.log(param); LdapWizard.ajaxRequests[reqID].abort(); } } @@ -167,7 +169,7 @@ var LdapWizard = { } } ); - if(reqID !== undefined) { + if(typeof reqID !== 'undefined') { LdapWizard.ajaxRequests[reqID] = request; } }, @@ -342,7 +344,7 @@ var LdapWizard = { }, _countThings: function(method, spinnerID, doneCallback) { - param = 'action='+method+ + var param = 'action='+method+ '&ldap_serverconfig_chooser='+ encodeURIComponent($('#ldap_serverconfig_chooser').val()); @@ -371,7 +373,12 @@ var LdapWizard = { }, countUsers: function(doneCallback) { - LdapWizard._countThings('countUsers', '#ldap_user_count', doneCallback); + //we make user counting depending on having a display name attribute + var param = 'action=detectUserDisplayNameAttribute' + + '&ldap_serverconfig_chooser='+ + encodeURIComponent($('#ldap_serverconfig_chooser').val()); + + LdapWizard._countThings('countUsers', '#ldap_user_count', doneCallback); }, detectEmailAttribute: function() { diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php index 1d7701440e9..c5db4c66cc0 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,11 @@ class Wizard extends LDAPUtility { * @throws \Exception */ public function countUsers() { - $filter = $this->configuration->ldapUserFilter; + $this->detectUserDisplayNameAttribute(); + $filter = $this->access->combineFilterWithAnd(array( + $this->configuration->ldapUserFilter, + $this->configuration->ldapUserDisplayName . '=*' + )); $usersTotal = $this->countEntries($filter, 'users'); $usersTotal = ($usersTotal !== false) ? $usersTotal : 0; @@ -151,6 +155,47 @@ class Wizard extends LDAPUtility { return $this->access->countUsers($filter); } + /** + * 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 + */ + 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)); + 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)); + + if($count > 0) { + $this->applyFind('ldap_display_name', $attr); + return $this->result; + } + }; + + throw new \Exception(self::$t->l('Could not detect user display name attribute. Please specify it yourself in advanced ldap settings.')); + } + /** * detects the most often used email attribute for users applying to the * user list filter. If a setting is already present that returns at least From f725cc66a385311d9995ec4215f57448b59f124e Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 27 Oct 2014 23:39:30 +0100 Subject: [PATCH 02/14] consolidate user count filter in wizard and user back end --- apps/user_ldap/lib/access.php | 12 +++++++++++ apps/user_ldap/lib/wizard.php | 5 +---- apps/user_ldap/tests/user_ldap.php | 32 ++---------------------------- apps/user_ldap/user_ldap.php | 3 +-- 4 files changed, 16 insertions(+), 36 deletions(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index d89029abf17..0d51fc51143 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -1167,6 +1167,18 @@ class Access extends LDAPUtility implements user\IUserTools { return $this->combineFilterWithOr($filter); } + /** + * returns the filter used for counting users + */ + public function getFilterForUserCount() { + $filter = $this->combineFilterWithAnd(array( + $this->connection->ldapUserFilter, + $this->connection->ldapUserDisplayName . '=*' + )); + + return $filter; + } + /** * @param string $name * @param string $password diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php index c5db4c66cc0..f373fabb69c 100644 --- a/apps/user_ldap/lib/wizard.php +++ b/apps/user_ldap/lib/wizard.php @@ -121,10 +121,7 @@ class Wizard extends LDAPUtility { */ public function countUsers() { $this->detectUserDisplayNameAttribute(); - $filter = $this->access->combineFilterWithAnd(array( - $this->configuration->ldapUserFilter, - $this->configuration->ldapUserDisplayName . '=*' - )); + $filter = $this->access->getFilterForUserCount(); $usersTotal = $this->countEntries($filter, 'users'); $usersTotal = ($usersTotal !== false) ? $usersTotal : 0; diff --git a/apps/user_ldap/tests/user_ldap.php b/apps/user_ldap/tests/user_ldap.php index c89edc33fa9..d975fd56c51 100644 --- a/apps/user_ldap/tests/user_ldap.php +++ b/apps/user_ldap/tests/user_ldap.php @@ -550,23 +550,9 @@ class Test_User_Ldap_Direct extends \PHPUnit_Framework_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); @@ -577,23 +563,9 @@ class Test_User_Ldap_Direct extends \PHPUnit_Framework_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; } From 71944a59a5279b34b57aa68c5b7ad0173e4a6793 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 28 Oct 2014 23:04:03 +0100 Subject: [PATCH 03/14] detectors (email, displayname..) are now started in one place, triggered from only 2 places. more reliable structure and flow, saves requests --- apps/user_ldap/js/ldapFilter.js | 41 +++++++++++------- apps/user_ldap/js/settings.js | 73 ++++++++++++++++++++++++--------- apps/user_ldap/lib/wizard.php | 1 - 3 files changed, 81 insertions(+), 34 deletions(-) diff --git a/apps/user_ldap/js/ldapFilter.js b/apps/user_ldap/js/ldapFilter.js index bb66c1df2ee..2bb62ad1a9a 100644 --- a/apps/user_ldap/js/ldapFilter.js +++ b/apps/user_ldap/js/ldapFilter.js @@ -25,7 +25,7 @@ LdapFilter.prototype.activate = function() { this.determineMode(); }; -LdapFilter.prototype.compose = function(callback) { +LdapFilter.prototype.compose = function() { var action; if(this.locked) { @@ -54,14 +54,7 @@ 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 () { console.log('LDAP Wizard: could not compose filter. '+ @@ -70,6 +63,15 @@ LdapFilter.prototype.compose = function(callback) { ); }; +LdapFilter.prototype.afterDetectorsRan = function() { + this.updateCount(); +}; + +LdapFilter.prototype.afterComposeSuccess = function(result) { + LdapWizard.applyChanges(result); + LdapWizard.runDetectors(this.target, this.afterDetectorsRan); +}; + LdapFilter.prototype.determineMode = function() { var param = 'action=get'+encodeURIComponent(this.target)+'FilterMode'+ '&ldap_serverconfig_chooser='+ @@ -145,10 +147,21 @@ LdapFilter.prototype.findFeatures = function() { } }; +LdapFilter.prototype.beforeUpdateCount = function(status) { + return LdapWizard.runDetectors(this.target, function() { + status.resolve(); + }); +}; + LdapFilter.prototype.updateCount = function(doneCallback) { - if(this.target === 'User') { - LdapWizard.countUsers(doneCallback); - } else if (this.target === 'Group') { - LdapWizard.countGroups(doneCallback); - } + var beforeUpdateCountDone = $.Deferred(); + this.beforeUpdateCount(beforeUpdateCountDone); + var filter = this; + $.when(beforeUpdateCountDone).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 93fd97c1e73..f8cb3d84cea 100644 --- a/apps/user_ldap/js/settings.js +++ b/apps/user_ldap/js/settings.js @@ -172,6 +172,7 @@ var LdapWizard = { if(typeof reqID !== 'undefined') { LdapWizard.ajaxRequests[reqID] = request; } + return request; }, applyChanges: function (result) { @@ -373,20 +374,44 @@ var LdapWizard = { }, countUsers: function(doneCallback) { - //we make user counting depending on having a display name attribute + LdapWizard._countThings('countUsers', '#ldap_user_count', doneCallback); + }, + + 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; + } + } + }, + + detectUserDisplayNameAttribute: function() { var param = 'action=detectUserDisplayNameAttribute' + '&ldap_serverconfig_chooser='+ encodeURIComponent($('#ldap_serverconfig_chooser').val()); - LdapWizard._countThings('countUsers', '#ldap_user_count', doneCallback); + //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() { @@ -394,7 +419,7 @@ var LdapWizard = { '&ldap_serverconfig_chooser='+ encodeURIComponent($('#ldap_serverconfig_chooser').val()); - LdapWizard.ajax(param, + return LdapWizard.ajax(param, function(result) { //pure background story }, @@ -416,7 +441,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( ""); } @@ -573,8 +598,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(); @@ -637,7 +666,6 @@ var LdapWizard = { $('#ldap_user_count').text(''); LdapWizard.showSpinner('#rawUserFilterContainer .ldapGetEntryCount'); LdapWizard.userFilter.updateCount(LdapWizard.hideTestSpinner); - LdapWizard.detectEmailAttribute(); $('#ldap_user_count').removeClass('hidden'); }); @@ -658,7 +686,6 @@ var LdapWizard = { $('#ldap_group_count').text(''); LdapWizard.showSpinner('#rawGroupFilterContainer .ldapGetEntryCount'); LdapWizard.groupFilter.updateCount(LdapWizard.hideTestSpinner); - LdapWizard.detectGroupMemberAssoc(); $('#ldap_group_count').removeClass('hidden'); }); }, @@ -675,7 +702,7 @@ var LdapWizard = { postInitUserFilter: function() { if(LdapWizard.userFilterObjectClassesHasRun && LdapWizard.userFilterAvailableGroupsHasRun) { - LdapWizard.userFilter.compose(LdapWizard.detectEmailAttribute); + LdapWizard.userFilter.compose(); } }, @@ -686,7 +713,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; @@ -698,9 +725,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(){}); + } } }, @@ -718,12 +759,6 @@ 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(); @@ -756,7 +791,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(); //when user filter is changed afterwards, login filter needs to //be adjusted, too if(!LdapWizard.loginFilter) { @@ -837,7 +872,7 @@ var LdapWizard = { LdapWizard._save({ id: modeKey }, LdapWizard.filterModeAssisted); if(isUser) { LdapWizard.blacklistRemove('ldap_userlist_filter'); - LdapWizard.userFilter.compose(LdapWizard.detectEmailAttribute); + LdapWizard.userFilter.compose(); } else { LdapWizard.blacklistRemove('ldap_group_filter'); LdapWizard.groupFilter.compose(); diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php index f373fabb69c..59911fe77d1 100644 --- a/apps/user_ldap/lib/wizard.php +++ b/apps/user_ldap/lib/wizard.php @@ -120,7 +120,6 @@ class Wizard extends LDAPUtility { * @throws \Exception */ public function countUsers() { - $this->detectUserDisplayNameAttribute(); $filter = $this->access->getFilterForUserCount(); $usersTotal = $this->countEntries($filter, 'users'); From f9b4f5f4e548715b06feec14236d68d330e865b2 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 29 Oct 2014 10:24:48 +0100 Subject: [PATCH 04/14] to reassure that selected attributes still work, do not count all matching entries but limit it to 1 in order to make it faster --- apps/user_ldap/lib/access.php | 14 ++++++++------ apps/user_ldap/lib/wizard.php | 13 ++++++++----- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 0d51fc51143..3a118891f40 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -887,8 +887,10 @@ 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); + $limitPerPage = (intval($limit) < intval($this->connection->ldapPagingSize)) ? + intval($limit) : intval($this->connection->ldapPagingSize); if(is_null($limit) || $limit <= 0) { - $limit = intval($this->connection->ldapPagingSize); + $limitPerPage = intval($this->connection->ldapPagingSize); } $counter = 0; @@ -898,19 +900,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; } diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php index 59911fe77d1..63b7e22ce8c 100644 --- a/apps/user_ldap/lib/wizard.php +++ b/apps/user_ldap/lib/wizard.php @@ -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,9 @@ class Wizard extends LDAPUtility { $attr . '=*' )); - return $this->access->countUsers($filter); + $limit = ($existsCheck === false) ? null : 1; + + return $this->access->countUsers($filter, array('dn'), $limit); } /** @@ -169,7 +172,7 @@ class Wizard extends LDAPUtility { 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)); + $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 @@ -181,7 +184,7 @@ class Wizard extends LDAPUtility { // first attribute that has at least one result wins $displayNameAttrs = array('displayname', 'cn'); foreach ($displayNameAttrs as $attr) { - $count = intval($this->countUsersWithAttribute($attr)); + $count = intval($this->countUsersWithAttribute($attr, true)); if($count > 0) { $this->applyFind('ldap_display_name', $attr); @@ -209,7 +212,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; } From 4a3fe42b1672f5110076c75582edb5a43b7ea577 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 29 Oct 2014 10:27:17 +0100 Subject: [PATCH 05/14] a corrected email attribute needs to be saved, not only returned --- apps/user_ldap/lib/wizard.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php index 63b7e22ce8c..2f8d97c89da 100644 --- a/apps/user_ldap/lib/wizard.php +++ b/apps/user_ldap/lib/wizard.php @@ -233,7 +233,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 ' . From c9e865629ebb1e83626ff049244318b8769b4502 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 29 Oct 2014 11:05:02 +0100 Subject: [PATCH 06/14] JS doc --- apps/user_ldap/js/ldapFilter.js | 14 ++++++++++++++ apps/user_ldap/js/settings.js | 13 +++++++++++++ 2 files changed, 27 insertions(+) diff --git a/apps/user_ldap/js/ldapFilter.js b/apps/user_ldap/js/ldapFilter.js index 2bb62ad1a9a..780aa10fe3f 100644 --- a/apps/user_ldap/js/ldapFilter.js +++ b/apps/user_ldap/js/ldapFilter.js @@ -63,12 +63,21 @@ LdapFilter.prototype.compose = function() { ); }; +/** + * 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); + //best time to run attribute detectors LdapWizard.runDetectors(this.target, this.afterDetectorsRan); }; @@ -147,6 +156,11 @@ 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(status) { return LdapWizard.runDetectors(this.target, function() { status.resolve(); diff --git a/apps/user_ldap/js/settings.js b/apps/user_ldap/js/settings.js index f8cb3d84cea..be1db28648f 100644 --- a/apps/user_ldap/js/settings.js +++ b/apps/user_ldap/js/settings.js @@ -377,6 +377,16 @@ 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()) @@ -397,6 +407,9 @@ var LdapWizard = { } }, + /** + * runs detector to find out a fitting user display name attribute + */ detectUserDisplayNameAttribute: function() { var param = 'action=detectUserDisplayNameAttribute' + '&ldap_serverconfig_chooser='+ From d8bb8afee33d96943fedbe619bfb2d7668cb6bb6 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 29 Oct 2014 11:08:20 +0100 Subject: [PATCH 07/14] this happens already before counting, no need anymore --- apps/user_ldap/js/ldapFilter.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/apps/user_ldap/js/ldapFilter.js b/apps/user_ldap/js/ldapFilter.js index 780aa10fe3f..4db8555f639 100644 --- a/apps/user_ldap/js/ldapFilter.js +++ b/apps/user_ldap/js/ldapFilter.js @@ -77,8 +77,6 @@ LdapFilter.prototype.afterDetectorsRan = function() { */ LdapFilter.prototype.afterComposeSuccess = function(result) { LdapWizard.applyChanges(result); - //best time to run attribute detectors - LdapWizard.runDetectors(this.target, this.afterDetectorsRan); }; LdapFilter.prototype.determineMode = function() { From 0e6d47123a9b47b977b821bd8fbeb92b3c8893a1 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 29 Oct 2014 19:20:32 +0100 Subject: [PATCH 08/14] use underscore.js for undefined checks --- apps/user_ldap/js/settings.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/user_ldap/js/settings.js b/apps/user_ldap/js/settings.js index be1db28648f..f6fcaa2849e 100644 --- a/apps/user_ldap/js/settings.js +++ b/apps/user_ldap/js/settings.js @@ -151,7 +151,7 @@ var LdapWizard = { ajaxRequests: {}, ajax: function(param, fnOnSuccess, fnOnError, reqID) { - if(typeof reqID !== 'undefined') { + if(!_.isUndefined(reqID)) { if(LdapWizard.ajaxRequests.hasOwnProperty(reqID)) { console.log('aborting ' + reqID); console.log(param); @@ -169,7 +169,7 @@ var LdapWizard = { } } ); - if(typeof reqID !== 'undefined') { + if(!_.isUndefined(reqID)) { LdapWizard.ajaxRequests[reqID] = request; } return request; @@ -354,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); } }, From 031d6c179f289140888ac4fb5fae698ee5bc37f3 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 29 Oct 2014 19:30:49 +0100 Subject: [PATCH 09/14] better readbility, no effective changes --- apps/user_ldap/js/ldapFilter.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/user_ldap/js/ldapFilter.js b/apps/user_ldap/js/ldapFilter.js index 4db8555f639..63baec24ecc 100644 --- a/apps/user_ldap/js/ldapFilter.js +++ b/apps/user_ldap/js/ldapFilter.js @@ -159,17 +159,17 @@ LdapFilter.prototype.findFeatures = function() { * resolving the passed status variable will fire up counting * @param {object} status an instance of $.Deferred */ -LdapFilter.prototype.beforeUpdateCount = function(status) { - return LdapWizard.runDetectors(this.target, function() { +LdapFilter.prototype.beforeUpdateCount = function() { + var status = $.Deferred(); + LdapWizard.runDetectors(this.target, function() { status.resolve(); }); + return status; }; LdapFilter.prototype.updateCount = function(doneCallback) { - var beforeUpdateCountDone = $.Deferred(); - this.beforeUpdateCount(beforeUpdateCountDone); var filter = this; - $.when(beforeUpdateCountDone).done(function() { + $.when(this.beforeUpdateCount()).done(function() { if(filter.target === 'User') { LdapWizard.countUsers(doneCallback); } else if (filter.target === 'Group') { From 6b6147dafd19965856b650cafe321c7347831992 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 20 Nov 2014 18:03:47 +0100 Subject: [PATCH 10/14] phpdoc and mixed up letters --- apps/user_ldap/lib/access.php | 1 + apps/user_ldap/lib/wizard.php | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 3a118891f40..a0ec64b3f60 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -1171,6 +1171,7 @@ class Access extends LDAPUtility implements user\IUserTools { /** * returns the filter used for counting users + * @return string */ public function getFilterForUserCount() { $filter = $this->combineFilterWithAnd(array( diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php index 2f8d97c89da..578a920f00e 100644 --- a/apps/user_ldap/lib/wizard.php +++ b/apps/user_ldap/lib/wizard.php @@ -158,6 +158,7 @@ class Wizard extends LDAPUtility { * 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', @@ -192,7 +193,7 @@ class Wizard extends LDAPUtility { } }; - throw new \Exception(self::$t->l('Could not detect user display name attribute. Please specify it yourself in advanced ldap settings.')); + throw new \Exception(self::$l->t('Could not detect user display name attribute. Please specify it yourself in advanced ldap settings.')); } /** From b5e707b1bfbe9cddafe2edb0232bc032cb892a2e Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 20 Nov 2014 18:31:24 +0100 Subject: [PATCH 11/14] trigger count on the correct filter --- apps/user_ldap/js/settings.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user_ldap/js/settings.js b/apps/user_ldap/js/settings.js index f6fcaa2849e..15c63614f50 100644 --- a/apps/user_ldap/js/settings.js +++ b/apps/user_ldap/js/settings.js @@ -670,7 +670,7 @@ var LdapWizard = { delete LdapWizard.userFilter; LdapWizard.userFilter = new LdapFilter('User', function(mode) { if(mode === LdapWizard.filterModeAssisted) { - LdapWizard.groupFilter.updateCount(); + LdapWizard.userFilter.updateCount(); } LdapWizard.userFilter.findFeatures(); }); From c07c338c908653d694df4457dc1739122a1f594c Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 21 Nov 2014 14:51:20 +0100 Subject: [PATCH 12/14] fix counting when ldapPagingSize is 0 --- apps/user_ldap/lib/access.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index a0ec64b3f60..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,10 +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); - $limitPerPage = (intval($limit) < intval($this->connection->ldapPagingSize)) ? - intval($limit) : intval($this->connection->ldapPagingSize); - if(is_null($limit) || $limit <= 0) { - $limitPerPage = intval($this->connection->ldapPagingSize); + $limitPerPage = intval($this->connection->ldapPagingSize); + if(!is_null($limit) && $limit < $limitPerPage && $limit > 0) { + $limitPerPage = $limit; } $counter = 0; @@ -1472,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) From 503de94392c0863e29cc8f1a46637f78ce8523ce Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 21 Nov 2014 16:23:56 +0100 Subject: [PATCH 13/14] make updateCount work properly with new xp'd mode as well as without --- apps/user_ldap/js/ldapFilter.js | 12 +++++++++++- apps/user_ldap/js/settings.js | 20 ++++++++++++++------ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/apps/user_ldap/js/ldapFilter.js b/apps/user_ldap/js/ldapFilter.js index 63baec24ecc..4fe8e4aebdf 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() { +LdapFilter.prototype.compose = function(updateCount = false) { var action; + if(updateCount) { + this.countPending = updateCount; + } + if(this.locked) { this.lazyRunCompose = true; return false; @@ -57,6 +62,7 @@ LdapFilter.prototype.compose = function() { filter.afterComposeSuccess(result); }, function () { + filter.countPending = false; console.log('LDAP Wizard: could not compose filter. '+ 'Please check owncloud.log'); } @@ -77,6 +83,10 @@ LdapFilter.prototype.afterDetectorsRan = function() { */ LdapFilter.prototype.afterComposeSuccess = function(result) { LdapWizard.applyChanges(result); + if(this.countPending) { + this.countPending = false; + this.updateCount(); + } }; LdapFilter.prototype.determineMode = function() { diff --git a/apps/user_ldap/js/settings.js b/apps/user_ldap/js/settings.js index 15c63614f50..9987f6fd015 100644 --- a/apps/user_ldap/js/settings.js +++ b/apps/user_ldap/js/settings.js @@ -669,7 +669,8 @@ var LdapWizard = { instantiateFilters: function() { delete LdapWizard.userFilter; LdapWizard.userFilter = new LdapFilter('User', function(mode) { - if(mode === LdapWizard.filterModeAssisted) { + if( !LdapWizard.admin.isExperienced() + || mode === LdapWizard.filterModeAssisted) { LdapWizard.userFilter.updateCount(); } LdapWizard.userFilter.findFeatures(); @@ -689,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(); @@ -775,6 +777,12 @@ var LdapWizard = { 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) { @@ -804,7 +812,7 @@ var LdapWizard = { LdapWizard._save($('#'+originalObj)[0], $.trim(values)); if(originalObj === 'ldap_userfilter_objectclass' || originalObj === 'ldap_userfilter_groups') { - LdapWizard.userFilter.compose(); + LdapWizard.userFilter.compose(!LdapWizard.admin.isExperienced()); //when user filter is changed afterwards, login filter needs to //be adjusted, too if(!LdapWizard.loginFilter) { @@ -815,7 +823,7 @@ var LdapWizard = { LdapWizard.loginFilter.compose(); } else if(originalObj === 'ldap_groupfilter_objectclass' || originalObj === 'ldap_groupfilter_groups') { - LdapWizard.groupFilter.compose(); + LdapWizard.groupFilter.compose(!LdapWizard.admin.isExperienced()); } }, @@ -885,10 +893,10 @@ var LdapWizard = { LdapWizard._save({ id: modeKey }, LdapWizard.filterModeAssisted); if(isUser) { LdapWizard.blacklistRemove('ldap_userlist_filter'); - LdapWizard.userFilter.compose(); + LdapWizard.userFilter.compose(!LdapWizard.admin.isExperienced()); } else { LdapWizard.blacklistRemove('ldap_group_filter'); - LdapWizard.groupFilter.compose(); + LdapWizard.groupFilter.compose(!LdapWizard.admin.isExperienced()); } } }, From 9aef83b5798733d718e03a4ef0edc7279db43e59 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Sat, 22 Nov 2014 00:51:41 +0100 Subject: [PATCH 14/14] make scrutinizer happier and always count users on assisted mode, even with xp'ed mode (would be a regression otherwise) --- apps/user_ldap/js/ldapFilter.js | 4 ++-- apps/user_ldap/js/settings.js | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/apps/user_ldap/js/ldapFilter.js b/apps/user_ldap/js/ldapFilter.js index 4fe8e4aebdf..0f7d240adac 100644 --- a/apps/user_ldap/js/ldapFilter.js +++ b/apps/user_ldap/js/ldapFilter.js @@ -26,10 +26,10 @@ LdapFilter.prototype.activate = function() { this.determineMode(); }; -LdapFilter.prototype.compose = function(updateCount = false) { +LdapFilter.prototype.compose = function(updateCount) { var action; - if(updateCount) { + if(updateCount === true) { this.countPending = updateCount; } diff --git a/apps/user_ldap/js/settings.js b/apps/user_ldap/js/settings.js index 9987f6fd015..6db210fe435 100644 --- a/apps/user_ldap/js/settings.js +++ b/apps/user_ldap/js/settings.js @@ -812,7 +812,7 @@ var LdapWizard = { LdapWizard._save($('#'+originalObj)[0], $.trim(values)); if(originalObj === 'ldap_userfilter_objectclass' || originalObj === 'ldap_userfilter_groups') { - LdapWizard.userFilter.compose(!LdapWizard.admin.isExperienced()); + LdapWizard.userFilter.compose(true); //when user filter is changed afterwards, login filter needs to //be adjusted, too if(!LdapWizard.loginFilter) { @@ -823,7 +823,7 @@ var LdapWizard = { LdapWizard.loginFilter.compose(); } else if(originalObj === 'ldap_groupfilter_objectclass' || originalObj === 'ldap_groupfilter_groups') { - LdapWizard.groupFilter.compose(!LdapWizard.admin.isExperienced()); + LdapWizard.groupFilter.compose(true); } }, @@ -893,10 +893,10 @@ var LdapWizard = { LdapWizard._save({ id: modeKey }, LdapWizard.filterModeAssisted); if(isUser) { LdapWizard.blacklistRemove('ldap_userlist_filter'); - LdapWizard.userFilter.compose(!LdapWizard.admin.isExperienced()); + LdapWizard.userFilter.compose(true); } else { LdapWizard.blacklistRemove('ldap_group_filter'); - LdapWizard.groupFilter.compose(!LdapWizard.admin.isExperienced()); + LdapWizard.groupFilter.compose(true); } } },