if possible, getUserGroups should get memberships using memberOf virtual attribute

4 files changed, 96 insertions, 2 deletions

diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php
index 4c5c01743aa..0395a4a80e3 100644
--- a/apps/user_ldap/group_ldap.php
+++ b/apps/user_ldap/group_ldap.php
@@ -251,7 +251,14 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
 	 * @return string|bool
 	 */
 	public function getUserPrimaryGroupIDs($dn) {
-		return $this->getEntryGroupID($dn, 'primaryGroupID');
+		$primaryGroupID = false;
+		if($this->access->connection->hasPrimaryGroups) {
+			$primaryGroupID = $this->getEntryGroupID($dn, 'primaryGroupID');
+			if($primaryGroupID === false) {
+				$this->access->connection->hasPrimaryGroups = false;
+			}
+		}
+		return $primaryGroupID;
 	}
 
 	/**
@@ -362,6 +369,27 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
 			return array();
 		}
 
+		$groups = [];
+		$primaryGroup = $this->getUserPrimaryGroup($userDN);
+
+		// 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
+			&& intval($this->access->connection->useMemberOfToDetectMembership) === 1
+		) {
+			$groupDNs = $this->access->readAttribute($userDN, 'memberOf');
+			if (is_array($groupDNs)) {
+				foreach ($groupDNs as $dn) {
+					$groups[] = $this->access->dn2groupname($dn);;
+				}
+			}
+			if($primaryGroup !== false) {
+				$groups[] = $primaryGroup;
+			}
+			$this->access->connection->writeToCache($cacheKey, $groups);
+			return $groups;
+		}
+
 		//uniqueMember takes DN, memberuid the uid, so we need to distinguish
 		if((strtolower($this->access->connection->ldapGroupMemberAssocAttr) === 'uniquemember')
 			|| (strtolower($this->access->connection->ldapGroupMemberAssocAttr) === 'member')
@@ -387,7 +415,6 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
 			$this->cachedGroupsByMember[$uid] = $groups;
 		}
 
-		$primaryGroup = $this->getUserPrimaryGroup($userDN);
 		if($primaryGroup !== false) {
 			$groups[] = $primaryGroup;
 		}
diff --git a/apps/user_ldap/lib/configuration.php b/apps/user_ldap/lib/configuration.php
index 373c5b48417..0af819ff66f 100644
--- a/apps/user_ldap/lib/configuration.php
+++ b/apps/user_ldap/lib/configuration.php
@@ -76,6 +76,7 @@ class Configuration {
 		'homeFolderNamingRule' => null,
 		'hasPagedResultSupport' => false,
 		'hasMemberOfFilterSupport' => false,
+		'useMemberOfToDetectMembership' => true,
 		'ldapExpertUsernameAttr' => null,
 		'ldapExpertUUIDUserAttr' => null,
 		'ldapExpertUUIDGroupAttr' => null,
@@ -395,6 +396,7 @@ class Configuration {
 			'ldap_expert_uuid_user_attr'        => '',
 			'ldap_expert_uuid_group_attr'       => '',
 			'has_memberof_filter_support'       => 0,
+			'use_memberof_to_detect_membership' => 1,
 			'last_jpegPhoto_lookup'             => 0,
 			'ldap_nested_groups'                => 0,
 			'ldap_paging_size'                  => 500,
@@ -449,6 +451,7 @@ class Configuration {
 			'ldap_expert_uuid_user_attr'        => 'ldapExpertUUIDUserAttr',
 			'ldap_expert_uuid_group_attr'       => 'ldapExpertUUIDGroupAttr',
 			'has_memberof_filter_support'       => 'hasMemberOfFilterSupport',
+			'use_memberof_to_detect_membership' => 'useMemberOfToDetectMembership',
 			'last_jpegPhoto_lookup'             => 'lastJpegPhotoLookup',
 			'ldap_nested_groups'                => 'ldapNestedGroups',
 			'ldap_paging_size'                  => 'ldapPagingSize',
diff --git a/apps/user_ldap/lib/connection.php b/apps/user_ldap/lib/connection.php
index d6f4bdcde04..0328259d78f 100644
--- a/apps/user_ldap/lib/connection.php
+++ b/apps/user_ldap/lib/connection.php
@@ -53,6 +53,11 @@ class Connection extends LDAPUtility {
 	private $dontDestruct = false;
 	private $hasPagedResultSupport = true;
 
+	/**
+	 * @var bool runtime flag that indicates whether supported primary groups are available
+	 */
+	public $hasPrimaryGroups = true;
+
 	//cache handler
 	protected $cache;
 
diff --git a/apps/user_ldap/tests/group_ldap.php b/apps/user_ldap/tests/group_ldap.php
index d91f1503abd..aeb306174f0 100644
--- a/apps/user_ldap/tests/group_ldap.php
+++ b/apps/user_ldap/tests/group_ldap.php
@@ -383,4 +383,63 @@ class Test_Group_Ldap extends \Test\TestCase {
 		$this->assertSame(4, $users);
 	}
 
+	public function testGetUserGroupsMemberOf() {
+		$access = $this->getAccessMock();
+		$this->enableGroups($access);
+
+		$dn = 'cn=userX,dc=foobar';
+
+		$access->connection->hasPrimaryGroups = false;
+
+		$access->expects($this->once())
+			->method('username2dn')
+			->will($this->returnValue($dn));
+
+		$access->expects($this->once())
+			->method('readAttribute')
+			->with($dn, 'memberOf')
+			->will($this->returnValue(['cn=groupA,dc=foobar', 'cn=groupB,dc=foobar']));
+
+		$access->expects($this->exactly(2))
+			->method('dn2groupname')
+			->will($this->returnArgument(0));
+
+		$groupBackend = new GroupLDAP($access);
+		$groups = $groupBackend->getUserGroups('userX');
+
+		$this->assertSame(2, count($groups));
+	}
+
+	public function testGetUserGroupsMemberOfDisabled() {
+		$access = $this->getAccessMock();
+
+		$access->connection->expects($this->any())
+			->method('__get')
+			->will($this->returnCallback(function($name) {
+				if($name === 'useMemberOfToDetectMembership') {
+					return 0;
+				}
+				return 1;
+			}));
+
+		$dn = 'cn=userX,dc=foobar';
+
+		$access->connection->hasPrimaryGroups = false;
+
+		$access->expects($this->once())
+			->method('username2dn')
+			->will($this->returnValue($dn));
+
+		$access->expects($this->never())
+			->method('readAttribute')
+			->with($dn, 'memberOf');
+
+		$access->expects($this->once())
+			->method('ownCloudGroupNames')
+			->will($this->returnValue([]));
+
+		$groupBackend = new GroupLDAP($access);
+		$groupBackend->getUserGroups('userX');
+	}
+
 }