From 98ac45fa78ad663511c9f6c565c0adf1c241d9f9 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 25 Jun 2014 14:04:06 +0200 Subject: [PATCH] Backport of #9214 Wizad: email attribute detection remove Access as hard dependency, inject it instead add unit test for mail detection write log message, if original value was changed undo falsely changed log file trigger email detection by Wizard upon any user filter filter change. before it happenend only upon user automatic list filter creation, but not on manual editing adjust static method vars as well --- apps/user_ldap/ajax/wizard.php | 20 +++- apps/user_ldap/js/ldapFilter.js | 5 +- apps/user_ldap/js/settings.js | 18 +++- apps/user_ldap/lib/wizard.php | 108 +++++++++++++++------ apps/user_ldap/tests/wizard.php | 163 ++++++++++++++++++++++++++++++-- 5 files changed, 273 insertions(+), 41 deletions(-) diff --git a/apps/user_ldap/ajax/wizard.php b/apps/user_ldap/ajax/wizard.php index ad75a384369..adbcf6ff414 100644 --- a/apps/user_ldap/ajax/wizard.php +++ b/apps/user_ldap/ajax/wizard.php @@ -39,13 +39,29 @@ if(!isset($_POST['ldap_serverconfig_chooser'])) { } $prefix = $_POST['ldap_serverconfig_chooser']; -$ldapWrapper = new OCA\user_ldap\lib\LDAP(); +$ldapWrapper = new \OCA\user_ldap\lib\LDAP(); $configuration = new \OCA\user_ldap\lib\Configuration($prefix); -$wizard = new \OCA\user_ldap\lib\Wizard($configuration, $ldapWrapper); + +$con = new \OCA\user_ldap\lib\Connection($ldapWrapper, '', null); +$con->setConfiguration($configuration->getConfiguration()); +$con->ldapConfigurationActive = true; +$con->setIgnoreValidation(true); + +$userManager = new \OCA\user_ldap\lib\user\Manager( + \OC::$server->getConfig(), + new \OCA\user_ldap\lib\FilesystemHelper(), + new \OCA\user_ldap\lib\LogWrapper(), + \OC::$server->getAvatarManager(), + new \OCP\Image()); + +$access = new \OCA\user_ldap\lib\Access($con, $ldapWrapper, $userManager); + +$wizard = new \OCA\user_ldap\lib\Wizard($configuration, $ldapWrapper, $access); switch($action) { case 'guessPortAndTLS': case 'guessBaseDN': + case 'detectEmailAttribute': case 'determineGroupMemberAssoc': case 'determineUserObjectClasses': case 'determineGroupObjectClasses': diff --git a/apps/user_ldap/js/ldapFilter.js b/apps/user_ldap/js/ldapFilter.js index df3bd67aec2..e9f60e7ba3c 100644 --- a/apps/user_ldap/js/ldapFilter.js +++ b/apps/user_ldap/js/ldapFilter.js @@ -14,7 +14,7 @@ function LdapFilter(target) { } } -LdapFilter.prototype.compose = function() { +LdapFilter.prototype.compose = function(callback) { var action; if(this.locked) { @@ -50,6 +50,9 @@ LdapFilter.prototype.compose = function() { LdapWizard.countGroups(); LdapWizard.detectGroupMemberAssoc(); } + if(typeof callback !== 'undefined') { + callback(); + } }, function () { console.log('LDAP Wizard: could not compose filter. '+ diff --git a/apps/user_ldap/js/settings.js b/apps/user_ldap/js/settings.js index 87d755697cb..fd84ca1980b 100644 --- a/apps/user_ldap/js/settings.js +++ b/apps/user_ldap/js/settings.js @@ -340,6 +340,14 @@ var LdapWizard = { LdapWizard._countThings('countUsers'); }, + detectEmailAttribute: function() { + param = 'action=detectEmailAttribute'+ + '&ldap_serverconfig_chooser='+ + encodeURIComponent($('#ldap_serverconfig_chooser').val()); + //runs in the background, no callbacks necessary + LdapWizard.ajax(param, LdapWizard.applyChanges, function(){}); + }, + detectGroupMemberAssoc: function() { param = 'action=determineGroupMemberAssoc'+ '&ldap_serverconfig_chooser='+ @@ -577,7 +585,7 @@ var LdapWizard = { postInitUserFilter: function() { if(LdapWizard.userFilterObjectClassesHasRun && LdapWizard.userFilterAvailableGroupsHasRun) { - LdapWizard.userFilter.compose(); + LdapWizard.userFilter.compose(LdapWizard.detectEmailAttribute); LdapWizard.countUsers(); } }, @@ -619,6 +627,7 @@ var LdapWizard = { if(triggerObj.id == 'ldap_userlist_filter') { LdapWizard.countUsers(); + LdapWizard.detectEmailAttribute(); } else if(triggerObj.id == 'ldap_group_filter') { LdapWizard.countGroups(); LdapWizard.detectGroupMemberAssoc(); @@ -656,9 +665,12 @@ var LdapWizard = { LdapWizard._save($('#'+originalObj)[0], $.trim(values)); if(originalObj == 'ldap_userfilter_objectclass' || originalObj == 'ldap_userfilter_groups') { - LdapWizard.userFilter.compose(); + LdapWizard.userFilter.compose(LdapWizard.detectEmailAttribute); //when user filter is changed afterwards, login filter needs to //be adjusted, too + if(!LdapWizard.loginFilter) { + LdapWizard.initLoginFilter(); + } LdapWizard.loginFilter.compose(); } else if(originalObj == 'ldap_loginfilter_attributes') { LdapWizard.loginFilter.compose(); @@ -720,7 +732,7 @@ var LdapWizard = { LdapWizard._save({ id: modeKey }, LdapWizard.filterModeAssisted); if(moc.indexOf('user') >= 0) { LdapWizard.blacklistRemove('ldap_userlist_filter'); - LdapWizard.userFilter.compose(); + LdapWizard.userFilter.compose(LdapWizard.detectEmailAttribute); } 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 0cec493d9ed..4118b45bc35 100644 --- a/apps/user_ldap/lib/wizard.php +++ b/apps/user_ldap/lib/wizard.php @@ -25,6 +25,7 @@ namespace OCA\user_ldap\lib; class Wizard extends LDAPUtility { static protected $l; + protected $access; protected $cr; protected $configuration; protected $result; @@ -48,12 +49,13 @@ class Wizard extends LDAPUtility { * @param Configuration $configuration an instance of Configuration * @param ILDAPWrapper $ldap an instance of ILDAPWrapper */ - public function __construct(Configuration $configuration, ILDAPWrapper $ldap) { + public function __construct(Configuration $configuration, ILDAPWrapper $ldap, Access $access) { parent::__construct($ldap); $this->configuration = $configuration; if(is_null(Wizard::$l)) { Wizard::$l = \OC_L10N::get('user_ldap'); } + $this->access = $access; $this->result = new WizardResult; } @@ -78,11 +80,10 @@ class Wizard extends LDAPUtility { throw new \Exception('Requirements not met', 400); } - $ldapAccess = $this->getAccess(); if($type === 'groups') { - $result = $ldapAccess->countGroups($filter); + $result = $this->access->countGroups($filter); } else if($type === 'users') { - $result = $ldapAccess->countUsers($filter); + $result = $this->access->countUsers($filter); } else { throw new \Exception('internal error: invald object type', 500); } @@ -128,6 +129,77 @@ class Wizard extends LDAPUtility { return $this->result; } + /** + * counts users with a specified attribute + * @param string $attr + * @return int|bool + */ + public function countUsersWithAttribute($attr) { + if(!$this->checkRequirements(array('ldapHost', + 'ldapPort', + 'ldapBase', + 'ldapUserFilter', + ))) { + return false; + } + + $filter = $this->access->combineFilterWithAnd(array( + $this->configuration->ldapUserFilter, + $attr . '=*' + )); + + return $this->access->countUsers($filter); + } + + /** + * 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 + * one hit, the detection will be canceled. + * @return WizardResult|bool + */ + public function detectEmailAttribute() { + if(!$this->checkRequirements(array('ldapHost', + 'ldapPort', + 'ldapBase', + 'ldapUserFilter', + ))) { + return false; + } + + $attr = $this->configuration->ldapEmailAttribute; + if(!empty($attr)) { + $count = intval($this->countUsersWithAttribute($attr)); + if($count > 0) { + return false; + } + $writeLog = true; + } else { + $writeLog = false; + } + + $emailAttributes = array('mail', 'mailPrimaryAddress'); + $winner = ''; + $maxUsers = 0; + foreach($emailAttributes as $attr) { + $count = $this->countUsersWithAttribute($attr); + if($count > $maxUsers) { + $maxUsers = $count; + $winner = $attr; + } + } + + if($winner !== '') { + $this->result->addChange('ldap_email_attr', $winner); + if($writeLog) { + \OCP\Util::writeLog('user_ldap', 'The mail attribute has ' . + 'automatically been reset, because the original value ' . + 'did not return any results.', \OCP\Util::INFO); + } + } + + return $this->result; + } + /** * @return WizardResult * @throws \Exception @@ -289,7 +361,6 @@ class Wizard extends LDAPUtility { */ public function fetchGroups($dbKey, $confKey) { $obclasses = array('posixGroup', 'group', 'zimbraDistributionList', 'groupOfNames'); - $ldapAccess = $this->getAccess(); $filterParts = array(); foreach($obclasses as $obclass) { @@ -298,15 +369,15 @@ class Wizard extends LDAPUtility { //we filter for everything //- that looks like a group and //- has the group display name set - $filter = $ldapAccess->combineFilterWithOr($filterParts); - $filter = $ldapAccess->combineFilterWithAnd(array($filter, 'cn=*')); + $filter = $this->access->combineFilterWithOr($filterParts); + $filter = $this->access->combineFilterWithAnd(array($filter, 'cn=*')); $groupNames = array(); $groupEntries = array(); $limit = 400; $offset = 0; do { - $result = $ldapAccess->searchGroups($filter, array('cn','dn'), $limit, $offset); + $result = $this->access->searchGroups($filter, array('cn'), $limit, $offset); foreach($result as $item) { $groupNames[] = $item['cn']; $groupEntries[] = $item; @@ -1104,27 +1175,6 @@ class Wizard extends LDAPUtility { } } - /** - * creates and returns an Access instance - * @return \OCA\user_ldap\lib\Access - */ - private function getAccess() { - $con = new Connection($this->ldap, '', null); - $con->setConfiguration($this->configuration->getConfiguration()); - $con->ldapConfigurationActive = true; - $con->setIgnoreValidation(true); - - $userManager = new user\Manager( - \OC::$server->getConfig(), - new FilesystemHelper(), - new LogWrapper(), - \OC::$server->getAvatarManager(), - new \OCP\Image()); - - $ldapAccess = new Access($con, $this->ldap, $userManager); - return $ldapAccess; - } - /** * @return bool|mixed */ diff --git a/apps/user_ldap/tests/wizard.php b/apps/user_ldap/tests/wizard.php index 50461432c42..1f420f9ee8a 100644 --- a/apps/user_ldap/tests/wizard.php +++ b/apps/user_ldap/tests/wizard.php @@ -43,16 +43,29 @@ class Test_Wizard extends \PHPUnit_Framework_TestCase { } private function getWizardAndMocks() { - static $conMethods; - - if(is_null($conMethods)) { - $conMethods = get_class_methods('\OCA\user_ldap\lib\Configuration'); + static $confMethods; + static $connMethods; + static $accMethods; + + if(is_null($confMethods)) { + $confMethods = get_class_methods('\OCA\user_ldap\lib\Configuration'); + $connMethods = get_class_methods('\OCA\user_ldap\lib\Connection'); + $accMethods = get_class_methods('\OCA\user_ldap\lib\Access'); } $lw = $this->getMock('\OCA\user_ldap\lib\ILDAPWrapper'); $conf = $this->getMock('\OCA\user_ldap\lib\Configuration', - $conMethods, + $confMethods, array($lw, null, null)); - return array(new Wizard($conf, $lw), $conf, $lw); + + $connector = $this->getMock('\OCA\user_ldap\lib\Connection', + $connMethods, array($lw, null, null)); + $um = $this->getMockBuilder('\OCA\user_ldap\lib\user\Manager') + ->disableOriginalConstructor() + ->getMock(); + $access = $this->getMock('\OCA\user_ldap\lib\Access', + $accMethods, array($connector, $lw, $um)); + + return array(new Wizard($conf, $lw, $access), $conf, $lw, $access); } private function prepareLdapWrapperForConnections(&$ldap) { @@ -207,6 +220,144 @@ class Test_Wizard extends \PHPUnit_Framework_TestCase { unset($uidnumber); } + public function testDetectEmailAttributeAlreadySet() { + list($wizard, $configuration, $ldap, $access) + = $this->getWizardAndMocks(); + + $configuration->expects($this->any()) + ->method('__get') + ->will($this->returnCallback(function ($name) { + if($name === 'ldapEmailAttribute') { + return 'myEmailAttribute'; + } else { + //for requirement checks + return 'let me pass'; + } + })); + + $access->expects($this->once()) + ->method('countUsers') + ->will($this->returnValue(42)); + + $wizard->detectEmailAttribute(); + } + + public function testDetectEmailAttributeOverrideSet() { + list($wizard, $configuration, $ldap, $access) + = $this->getWizardAndMocks(); + + $configuration->expects($this->any()) + ->method('__get') + ->will($this->returnCallback(function ($name) { + if($name === 'ldapEmailAttribute') { + return 'myEmailAttribute'; + } else { + //for requirement checks + return 'let me pass'; + } + })); + + $access->expects($this->exactly(3)) + ->method('combineFilterWithAnd') + ->will($this->returnCallback(function ($filterParts) { + return str_replace('=*', '', array_pop($filterParts)); + })); + + $access->expects($this->exactly(3)) + ->method('countUsers') + ->will($this->returnCallback(function ($filter) { + if($filter === 'myEmailAttribute') { + return 0; + } else if($filter === 'mail') { + return 3; + } else if($filter === 'mailPrimaryAddress') { + return 17; + } + var_dump($filter); + })); + + $result = $wizard->detectEmailAttribute()->getResultArray(); + $this->assertSame('mailPrimaryAddress', + $result['changes']['ldap_email_attr']); + } + + public function testDetectEmailAttributeFind() { + list($wizard, $configuration, $ldap, $access) + = $this->getWizardAndMocks(); + + $configuration->expects($this->any()) + ->method('__get') + ->will($this->returnCallback(function ($name) { + if($name === 'ldapEmailAttribute') { + return ''; + } else { + //for requirement checks + return 'let me pass'; + } + })); + + $access->expects($this->exactly(2)) + ->method('combineFilterWithAnd') + ->will($this->returnCallback(function ($filterParts) { + return str_replace('=*', '', array_pop($filterParts)); + })); + + $access->expects($this->exactly(2)) + ->method('countUsers') + ->will($this->returnCallback(function ($filter) { + if($filter === 'myEmailAttribute') { + return 0; + } else if($filter === 'mail') { + return 3; + } else if($filter === 'mailPrimaryAddress') { + return 17; + } + var_dump($filter); + })); + + $result = $wizard->detectEmailAttribute()->getResultArray(); + $this->assertSame('mailPrimaryAddress', + $result['changes']['ldap_email_attr']); + } + + public function testDetectEmailAttributeFindNothing() { + list($wizard, $configuration, $ldap, $access) + = $this->getWizardAndMocks(); + + $configuration->expects($this->any()) + ->method('__get') + ->will($this->returnCallback(function ($name) { + if($name === 'ldapEmailAttribute') { + return 'myEmailAttribute'; + } else { + //for requirement checks + return 'let me pass'; + } + })); + + $access->expects($this->exactly(3)) + ->method('combineFilterWithAnd') + ->will($this->returnCallback(function ($filterParts) { + return str_replace('=*', '', array_pop($filterParts)); + })); + + $access->expects($this->exactly(3)) + ->method('countUsers') + ->will($this->returnCallback(function ($filter) { + if($filter === 'myEmailAttribute') { + return 0; + } else if($filter === 'mail') { + return 0; + } else if($filter === 'mailPrimaryAddress') { + return 0; + } + var_dump($filter); + })); + + $result = $wizard->detectEmailAttribute(); + $this->assertSame(false, $result->hasChanges()); + } + public function testCumulativeSearchOnAttributeSkipReadDN() { // tests that there is no infinite loop, when skipping already processed // DNs (they can be returned multiple times for multiple filters ) -- 2.39.5