From 0d797637f3c87fc5c1100f23cd50322adb5b4e34 Mon Sep 17 00:00:00 2001 From: Alex Weirig Date: Thu, 14 Jan 2016 13:26:40 +0100 Subject: [PATCH 1/5] code changes for user_ldap Dynamic Group Membership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added new setting of “Dynamic Group Member URL” (ldapDynamicGroupMemberURL) - see LDAP settings advanced tab. Added public function getDynamicGroupMembers. Updated function _groupMembers. Updated function getUserGroups. --- apps/user_ldap/group_ldap.php | 93 ++++++++++++++++++- apps/user_ldap/js/wizard/wizardTabAdvanced.js | 13 +++ apps/user_ldap/lib/configuration.php | 6 +- apps/user_ldap/templates/settings.php | 1 + 4 files changed, 108 insertions(+), 5 deletions(-) diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index 76152e1780..add66d8e87 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -12,6 +12,7 @@ * @author Robin McCorkell * @author Thomas Müller * @author Vincent Petry + * @author Richard Bentley * * @copyright Copyright (c) 2016, ownCloud, Inc. * @license AGPL-3.0 @@ -146,6 +147,46 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { return $isInGroup; } + /** + * @param string $dnGroup + * @return array + * + * For a group that has user membership defined by an LDAP search url attribute returns the users + * that match the search url otherwise returns an empty array. + */ + public function getDynamicGroupMembers($dnGroup) { + $dynamicGroupMemberURL = strtolower($this->access->connection->ldapDynamicGroupMemberURL); + + if (empty($dynamicGroupMemberURL)) { + return array(); + } + + $dynamicMembers = array(); + $memberURLs = $this->access->readAttribute( + $dnGroup, + $dynamicGroupMemberURL, + $this->access->connection->ldapGroupFilter + ); + if ($memberURLs !== false) { + // this group has the 'memberURL' attribute so this is a dynamic group + // example 1: ldap:///cn=users,cn=accounts,dc=dcsubbase,dc=dcbase??one?(o=HeadOffice) + // example 2: ldap:///cn=users,cn=accounts,dc=dcsubbase,dc=dcbase??one?(&(o=HeadOffice)(uidNumber>=500)) + $pos = strpos($memberURLs[0], '('); + if ($pos !== false) { + $memberUrlFilter = substr($memberURLs[0], $pos); + $foundMembers = $this->access->searchUsers($memberUrlFilter,'dn'); + $dynamicMembers = array(); + foreach($foundMembers as $value) { + $dynamicMembers[$value['dn'][0]] = 1; + } + } else { + \OCP\Util::writeLog('user_ldap', 'No search filter found on member url '. + 'of group ' . $dnGroup, \OCP\Util::DEBUG); + } + } + return $dynamicMembers; + } + /** * @param string $dnGroup * @param array|null &$seen @@ -180,6 +221,9 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { } } } + + $allMembers = array_merge($allMembers, $this->getDynamicGroupMembers($dnGroup)); + $this->access->connection->writeToCache($cacheKey, $allMembers); return $allMembers; } @@ -387,6 +431,8 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { * * This function fetches all groups a user belongs to. It does not check * if the user exists at all. + * + * This function includes groups based on dynamic group membership. */ public function getUserGroups($uid) { if(!$this->enabled) { @@ -405,6 +451,8 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { $groups = []; $primaryGroup = $this->getUserPrimaryGroup($userDN); + $dynamicGroupMemberURL = strtolower($this->access->connection->ldapDynamicGroupMemberURL); + // if possible, read out membership via memberOf. It's far faster than // performing a search, which still is a fallback later. if(intval($this->access->connection->hasMemberOfFilterSupport) === 1 @@ -422,11 +470,15 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { } } - if($primaryGroup !== false) { - $groups[] = $primaryGroup; + if (empty($dynamicGroupMemberURL)) { + // if dynamic group membership is not enabled then we can return + // straight away + if($primaryGroup !== false) { + $groups[] = $primaryGroup; + } + $this->access->connection->writeToCache($cacheKey, $groups); + return $groups; } - $this->access->connection->writeToCache($cacheKey, $groups); - return $groups; } //uniqueMember takes DN, memberuid the uid, so we need to distinguish @@ -458,6 +510,39 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { $groups[] = $primaryGroup; } + if (!empty($dynamicGroupMemberURL)) { + // look through dynamic groups to add them to the result array if needed + $groupsToMatch = $this->access->fetchListOfGroups( + $this->access->connection->ldapGroupFilter,array('dn',$dynamicGroupMemberURL)); + foreach($groupsToMatch as $value) { + if (!array_key_exists($dynamicGroupMemberURL, $value)) { + continue; + } + $pos = strpos($value[$dynamicGroupMemberURL][0], '('); + if ($pos !== false) { + $memberUrlFilter = substr($value[$dynamicGroupMemberURL][0],$pos); + // apply filter via ldap search to see if this user is in this + // dynamic group + $userMatch = $this->access->readAttribute( + $uid, + $this->access->connection->ldapUserDisplayName, + $memberUrlFilter + ); + if ($userMatch !== false) { + // match found so this user is in this group + $pos = strpos($value['dn'][0], ','); + if ($pos !== false) { + $membershipGroup = substr($value['dn'][0],3,$pos-3); + $groups[] = $membershipGroup; + } + } + } else { + \OCP\Util::writeLog('user_ldap', 'No search filter found on member url '. + 'of group ' . $dnGroup, \OCP\Util::DEBUG); + } + } + } + $groups = array_unique($groups, SORT_LOCALE_STRING); $this->access->connection->writeToCache($cacheKey, $groups); diff --git a/apps/user_ldap/js/wizard/wizardTabAdvanced.js b/apps/user_ldap/js/wizard/wizardTabAdvanced.js index 7367bfe87a..9e898ba2fc 100644 --- a/apps/user_ldap/js/wizard/wizardTabAdvanced.js +++ b/apps/user_ldap/js/wizard/wizardTabAdvanced.js @@ -79,6 +79,10 @@ OCA = OCA || {}; $element: $('#ldap_group_member_assoc_attribute'), setMethod: 'setGroupMemberAssociationAttribute' }, + ldap_dynamic_group_member_url: { + $element: $('#ldap_dynamic_group_member_url'), + setMethod: 'setDynamicGroupMemberURL' + }, ldap_nested_groups: { $element: $('#ldap_nested_groups'), setMethod: 'setUseNestedGroups' @@ -244,6 +248,15 @@ OCA = OCA || {}; this.setElementValue(this.managedItems.ldap_group_member_assoc_attribute.$element, attribute); }, + /** + * sets the dynamic group member url attribute + * + * @param {string} attribute + */ + setDynamicGroupMemberURL: function(attribute) { + this.setElementValue(this.managedItems.ldap_dynamic_group_member_url.$element, attribute); + }, + /** * enabled or disables the use of nested groups (groups in groups in * groups…) diff --git a/apps/user_ldap/lib/configuration.php b/apps/user_ldap/lib/configuration.php index 75d244255c..11090f1301 100644 --- a/apps/user_ldap/lib/configuration.php +++ b/apps/user_ldap/lib/configuration.php @@ -7,6 +7,7 @@ * @author Lukas Reschke * @author Morris Jobke * @author Robin McCorkell + * @author Richard Bentley * * @copyright Copyright (c) 2016, ownCloud, Inc. * @license AGPL-3.0 @@ -83,6 +84,7 @@ class Configuration { 'lastJpegPhotoLookup' => null, 'ldapNestedGroups' => false, 'ldapPagingSize' => null, + 'ldapDynamicGroupMemberURL' => null, ); /** @@ -439,6 +441,7 @@ class Configuration { 'ldap_nested_groups' => 0, 'ldap_paging_size' => 500, 'ldap_experienced_admin' => 0, + 'ldap_dynamic_group_member_url' => '', ); } @@ -492,7 +495,8 @@ class Configuration { 'last_jpegPhoto_lookup' => 'lastJpegPhotoLookup', 'ldap_nested_groups' => 'ldapNestedGroups', 'ldap_paging_size' => 'ldapPagingSize', - 'ldap_experienced_admin' => 'ldapExperiencedAdmin' + 'ldap_experienced_admin' => 'ldapExperiencedAdmin', + 'ldap_dynamic_group_member_url' => 'ldapDynamicGroupMemberURL', ); return $array; } diff --git a/apps/user_ldap/templates/settings.php b/apps/user_ldap/templates/settings.php index 88900e22bf..1c0c9a8acb 100644 --- a/apps/user_ldap/templates/settings.php +++ b/apps/user_ldap/templates/settings.php @@ -90,6 +90,7 @@ style('user_ldap', 'settings');

+

From 39aac2c66b2088153925cf75dca7f9f695571330 Mon Sep 17 00:00:00 2001 From: alexweirig Date: Thu, 28 Jan 2016 14:43:55 +0100 Subject: [PATCH 2/5] Refactored code to avoid performance problem Moved the dynamic group processing to the top and removed condition in memberOf processing. Also, changed variable name $value to $memberUrl --- apps/user_ldap/group_ldap.php | 78 +++++++++++++++++------------------ 1 file changed, 37 insertions(+), 41 deletions(-) diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index add66d8e87..2fcbcba08f 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -453,6 +453,39 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { $dynamicGroupMemberURL = strtolower($this->access->connection->ldapDynamicGroupMemberURL); + if (!empty($dynamicGroupMemberURL)) { + // look through dynamic groups to add them to the result array if needed + $groupsToMatch = $this->access->fetchListOfGroups( + $this->access->connection->ldapGroupFilter,array('dn',$dynamicGroupMemberURL)); + foreach($groupsToMatch as $memberUrl) { + if (!array_key_exists($dynamicGroupMemberURL, $memberUrl)) { + continue; + } + $pos = strpos($memberUrl[$dynamicGroupMemberURL][0], '('); + if ($pos !== false) { + $memberUrlFilter = substr($memberUrl[$dynamicGroupMemberURL][0],$pos); + // apply filter via ldap search to see if this user is in this + // dynamic group + $userMatch = $this->access->readAttribute( + $uid, + $this->access->connection->ldapUserDisplayName, + $memberUrlFilter + ); + if ($userMatch !== false) { + // match found so this user is in this group + $pos = strpos($memberUrl['dn'][0], ','); + if ($pos !== false) { + $membershipGroup = substr($memberUrl['dn'][0],3,$pos-3); + $groups[] = $membershipGroup; + } + } + } else { + \OCP\Util::writeLog('user_ldap', 'No search filter found on member url '. + 'of group ' . $dnGroup, \OCP\Util::DEBUG); + } + } + } + // if possible, read out membership via memberOf. It's far faster than // performing a search, which still is a fallback later. if(intval($this->access->connection->hasMemberOfFilterSupport) === 1 @@ -470,15 +503,11 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { } } - if (empty($dynamicGroupMemberURL)) { - // if dynamic group membership is not enabled then we can return - // straight away - if($primaryGroup !== false) { - $groups[] = $primaryGroup; - } - $this->access->connection->writeToCache($cacheKey, $groups); - return $groups; + if($primaryGroup !== false) { + $groups[] = $primaryGroup; } + $this->access->connection->writeToCache($cacheKey, $groups); + return $groups; } //uniqueMember takes DN, memberuid the uid, so we need to distinguish @@ -510,39 +539,6 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { $groups[] = $primaryGroup; } - if (!empty($dynamicGroupMemberURL)) { - // look through dynamic groups to add them to the result array if needed - $groupsToMatch = $this->access->fetchListOfGroups( - $this->access->connection->ldapGroupFilter,array('dn',$dynamicGroupMemberURL)); - foreach($groupsToMatch as $value) { - if (!array_key_exists($dynamicGroupMemberURL, $value)) { - continue; - } - $pos = strpos($value[$dynamicGroupMemberURL][0], '('); - if ($pos !== false) { - $memberUrlFilter = substr($value[$dynamicGroupMemberURL][0],$pos); - // apply filter via ldap search to see if this user is in this - // dynamic group - $userMatch = $this->access->readAttribute( - $uid, - $this->access->connection->ldapUserDisplayName, - $memberUrlFilter - ); - if ($userMatch !== false) { - // match found so this user is in this group - $pos = strpos($value['dn'][0], ','); - if ($pos !== false) { - $membershipGroup = substr($value['dn'][0],3,$pos-3); - $groups[] = $membershipGroup; - } - } - } else { - \OCP\Util::writeLog('user_ldap', 'No search filter found on member url '. - 'of group ' . $dnGroup, \OCP\Util::DEBUG); - } - } - } - $groups = array_unique($groups, SORT_LOCALE_STRING); $this->access->connection->writeToCache($cacheKey, $groups); From f4d4bed10ba2fd1465d54ab6a185fd2ac07d2879 Mon Sep 17 00:00:00 2001 From: alexweirig Date: Fri, 29 Jan 2016 08:27:59 +0100 Subject: [PATCH 3/5] Fixed undefined variable $dnGroup and variable name I renamed $memberURL into $dynamicGroup and print that variable in the writeLog. I think this makes more sense. --- apps/user_ldap/group_ldap.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index 2fcbcba08f..33f848da7e 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -457,13 +457,13 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { // look through dynamic groups to add them to the result array if needed $groupsToMatch = $this->access->fetchListOfGroups( $this->access->connection->ldapGroupFilter,array('dn',$dynamicGroupMemberURL)); - foreach($groupsToMatch as $memberUrl) { - if (!array_key_exists($dynamicGroupMemberURL, $memberUrl)) { + foreach($groupsToMatch as $dynamicGroup) { + if (!array_key_exists($dynamicGroupMemberURL, $dynamicGroup)) { continue; } - $pos = strpos($memberUrl[$dynamicGroupMemberURL][0], '('); + $pos = strpos($dynamicGroup[$dynamicGroupMemberURL][0], '('); if ($pos !== false) { - $memberUrlFilter = substr($memberUrl[$dynamicGroupMemberURL][0],$pos); + $memberUrlFilter = substr($dynamicGroup[$dynamicGroupMemberURL][0],$pos); // apply filter via ldap search to see if this user is in this // dynamic group $userMatch = $this->access->readAttribute( @@ -473,15 +473,15 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { ); if ($userMatch !== false) { // match found so this user is in this group - $pos = strpos($memberUrl['dn'][0], ','); + $pos = strpos($dynamicGroup['dn'][0], ','); if ($pos !== false) { - $membershipGroup = substr($memberUrl['dn'][0],3,$pos-3); + $membershipGroup = substr($dynamicGroup['dn'][0],3,$pos-3); $groups[] = $membershipGroup; } } } else { \OCP\Util::writeLog('user_ldap', 'No search filter found on member url '. - 'of group ' . $dnGroup, \OCP\Util::DEBUG); + 'of group ' . $dynamicGroup, \OCP\Util::DEBUG); } } } From cd19b648c644df4c5e28b0a50ffe3f9cbbc8a9fb Mon Sep 17 00:00:00 2001 From: alexweirig Date: Fri, 29 Jan 2016 14:21:20 +0100 Subject: [PATCH 4/5] change error logging replaced variable with print_r call --- apps/user_ldap/group_ldap.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index 33f848da7e..05ab9ddfaa 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -481,7 +481,7 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { } } else { \OCP\Util::writeLog('user_ldap', 'No search filter found on member url '. - 'of group ' . $dynamicGroup, \OCP\Util::DEBUG); + 'of group ' . print_r($dynamicGroup, true), \OCP\Util::DEBUG); } } } From e370b601db702ae9d6d57f961b651c401be4310f Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 3 Feb 2016 22:45:33 +0100 Subject: [PATCH 5/5] fix unit tests --- apps/user_ldap/tests/group_ldap.php | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/apps/user_ldap/tests/group_ldap.php b/apps/user_ldap/tests/group_ldap.php index 5f9ded878c..667a1c3acb 100644 --- a/apps/user_ldap/tests/group_ldap.php +++ b/apps/user_ldap/tests/group_ldap.php @@ -67,10 +67,13 @@ class Test_Group_Ldap extends \Test\TestCase { private function enableGroups($access) { $access->connection->expects($this->any()) - ->method('__get') - ->will($this->returnCallback(function() { - return 1; - })); + ->method('__get') + ->will($this->returnCallback(function($name) { + if($name === 'ldapDynamicGroupMemberURL') { + return ''; + } + return 1; + })); } public function testCountEmptySearchString() { @@ -430,6 +433,8 @@ class Test_Group_Ldap extends \Test\TestCase { ->will($this->returnCallback(function($name) { if($name === 'useMemberOfToDetectMembership') { return 0; + } else if($name === 'ldapDynamicGroupMemberURL') { + return ''; } return 1; }));