Properly nest groups when using memberOf to detect group membership, fixes #17759

2 files changed, 39 insertions, 7 deletions

diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php
index 1b83817151c..44b0ceac7eb 100644
--- a/apps/user_ldap/group_ldap.php
+++ b/apps/user_ldap/group_ldap.php
@@ -182,6 +182,39 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
 	}
 
 	/**
+	 * @param string $dnGroup
+	 * @param array &$seen
+	 * @return array
+	 */
+	private function _getGroupDNsFromMemberOf($DN, &$seen = null) {
+		if ($seen === null) {
+			$seen = array();
+		}
+		if (array_key_exists($DN, $seen)) {
+			// avoid loops
+			return array();
+		}
+		$seen[$DN] = 1;
+		$groups = $this->access->readAttribute($DN, 'memberOf');
+		if (is_array($groups)) {
+			$groups = $this->access->groupsMatchFilter($groups);
+			$allGroups =  $groups;
+			foreach ($groups as $group) {
+				$nestedGroups = $this->access->connection->ldapNestedGroups;
+				if (!empty($nestedGroups)) {
+					$subGroups = $this->_getGroupDNsFromMemberOf($group, $seen);
+					if ($subGroups) {
+						$allGroups = array_merge($allGroups, $subGroups);
+					}
+				}
+			}	
+			return $allGroups;	
+		} else {
+			return array();
+		}
+	}
+
+	/**
 	 * translates a primary group ID into an ownCloud internal name
 	 * @param string $gid as given by primaryGroupID on AD
 	 * @param string $dn a DN that belongs to the same domain as the group
@@ -377,14 +410,14 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
 		if(intval($this->access->connection->hasMemberOfFilterSupport) === 1
 			&& intval($this->access->connection->useMemberOfToDetectMembership) === 1
 		) {
-			$groupDNs = $this->access->readAttribute($userDN, 'memberOf');
-
+			$groupDNs = $this->_getGroupDNsFromMemberOf($userDN);
+			
 			if (is_array($groupDNs)) {
-				$groupDNs = $this->access->groupsMatchFilter($groupDNs);
 				foreach ($groupDNs as $dn) {
 					$groups[] = $this->access->dn2groupname($dn);
 				}
 			}
+			
 			if($primaryGroup !== false) {
 				$groups[] = $primaryGroup;
 			}
diff --git a/apps/user_ldap/tests/group_ldap.php b/apps/user_ldap/tests/group_ldap.php
index f716618ce48..805238e7d37 100644
--- a/apps/user_ldap/tests/group_ldap.php
+++ b/apps/user_ldap/tests/group_ldap.php
@@ -395,16 +395,15 @@ class Test_Group_Ldap extends \Test\TestCase {
 			->method('username2dn')
 			->will($this->returnValue($dn));
 
-		$access->expects($this->once())
+		$access->expects($this->exactly(3))
 			->method('readAttribute')
-			->with($dn, 'memberOf')
-			->will($this->returnValue(['cn=groupA,dc=foobar', 'cn=groupB,dc=foobar']));
+			->will($this->onConsecutiveCalls(['cn=groupA,dc=foobar', 'cn=groupB,dc=foobar'], [], []));
 
 		$access->expects($this->exactly(2))
 			->method('dn2groupname')
 			->will($this->returnArgument(0));
 
-		$access->expects($this->once())
+		$access->expects($this->exactly(3))
 			->method('groupsMatchFilter')
 			->will($this->returnArgument(0));