Merge pull request #27515 from nextcloud/enh/noid/read-multi-value-user-attribute

Add method to read multi-value attributes from ldap

2 files changed, 214 insertions, 13 deletions

diff --git a/apps/user_ldap/lib/LDAPProvider.php b/apps/user_ldap/lib/LDAPProvider.php
index 1d2354026d6..dd86ce486ac 100644
--- a/apps/user_ldap/lib/LDAPProvider.php
+++ b/apps/user_ldap/lib/LDAPProvider.php
@@ -309,32 +309,42 @@ class LDAPProvider implements ILDAPProvider, IDeletionFlagSupport {
 
 	/**
 	 * Get an LDAP attribute for a nextcloud user
-	 * @param string $uid the nextcloud user id to get the attribute for
-	 * @param string $attribute the name of the attribute to read
-	 * @return string|null
+	 *
 	 * @throws \Exception if user id was not found in LDAP
 	 */
 	public function getUserAttribute(string $uid, string $attribute): ?string {
+		$values = $this->getMultiValueUserAttribute($uid, $attribute);
+		if (count($values) === 0) {
+			return null;
+		}
+		return current($values);
+	}
+
+	/**
+	 * Get a multi-value LDAP attribute for a nextcloud user
+	 *
+	 * @throws \Exception if user id was not found in LDAP
+	 */
+	public function getMultiValueUserAttribute(string $uid, string $attribute): array {
 		if (!$this->userBackend->userExists($uid)) {
 			throw new \Exception('User id not found in LDAP');
 		}
+
 		$access = $this->userBackend->getLDAPAccess($uid);
 		$connection = $access->getConnection();
-		$key = $uid . "::" . $attribute;
-		$cached = $connection->getFromCache($key);
+		$key = $uid . '-' . $attribute;
 
-		if ($cached !== null) {
+		$cached = $connection->getFromCache($key);
+		if (is_array($cached)) {
 			return $cached;
 		}
 
-		$value = $access->readAttribute($access->username2dn($uid), $attribute);
-		if (is_array($value) && count($value) > 0) {
-			$value = current($value);
-		} else {
-			return null;
+		$values = $access->readAttribute($access->username2dn($uid), $attribute);
+		if ($values === false) {
+			$values = [];
 		}
-		$connection->writeToCache($key, $value);
 
-		return $value;
+		$connection->writeToCache($key, $values);
+		return $values;
 	}
 }
diff --git a/apps/user_ldap/tests/LDAPProviderTest.php b/apps/user_ldap/tests/LDAPProviderTest.php
index 607cf8f4293..59f51f204bf 100644
--- a/apps/user_ldap/tests/LDAPProviderTest.php
+++ b/apps/user_ldap/tests/LDAPProviderTest.php
@@ -31,8 +31,10 @@ namespace OCA\User_LDAP\Tests;
 use OC\User\Manager;
 use OCA\User_LDAP\Access;
 use OCA\User_LDAP\Connection;
+use OCA\User_LDAP\Group_LDAP;
 use OCA\User_LDAP\IGroupLDAP;
 use OCA\User_LDAP\IUserLDAP;
+use OCA\User_LDAP\User_LDAP;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\ICacheFactory;
 use OCP\IConfig;
@@ -697,4 +699,193 @@ class LDAPProviderTest extends \Test\TestCase {
 		$ldapProvider = $this->getLDAPProvider($server);
 		$this->assertEquals('assoc_type', $ldapProvider->getLDAPGroupMemberAssoc('existing_group'));
 	}
+
+	public function testGetMultiValueUserAttributeUserNotFound() {
+		$this->expectException(\Exception::class);
+		$this->expectExceptionMessage('User id not found in LDAP');
+
+		$userBackend = $this->createMock(User_LDAP::class);
+		$userBackend->expects(self::once())
+			->method('userExists')
+			->with('admin')
+			->willReturn(false);
+		$groupBackend = $this->createMock(Group_LDAP::class);
+		$server = $this->getServerMock($userBackend, $groupBackend);
+
+		$ldapProvider = $this->getLDAPProvider($server);
+		$ldapProvider->getMultiValueUserAttribute('admin', 'mailAlias');
+	}
+
+	public function testGetMultiValueUserAttributeCacheHit() {
+		$connection = $this->createMock(Connection::class);
+		$connection->expects(self::once())
+			->method('getFromCache')
+			->with('admin-mailAlias')
+			->willReturn(['aliasA@test.local', 'aliasB@test.local']);
+		$access = $this->createMock(Access::class);
+		$access->expects(self::once())
+			->method('getConnection')
+			->willReturn($connection);
+		$userBackend = $this->createMock(User_LDAP::class);
+		$userBackend->expects(self::once())
+			->method('userExists')
+			->with('admin')
+			->willReturn(true);
+		$userBackend->expects(self::once())
+			->method('getLDAPAccess')
+			->willReturn($access);
+		$groupBackend = $this->createMock(Group_LDAP::class);
+		$server = $this->getServerMock($userBackend, $groupBackend);
+
+		$ldapProvider = $this->getLDAPProvider($server);
+		$ldapProvider->getMultiValueUserAttribute('admin', 'mailAlias');
+	}
+
+	public function testGetMultiValueUserAttributeLdapError() {
+		$connection = $this->createMock(Connection::class);
+		$connection->expects(self::once())
+			->method('getFromCache')
+			->with('admin-mailAlias')
+			->willReturn(null);
+		$access = $this->createMock(Access::class);
+		$access->expects(self::once())
+			->method('getConnection')
+			->willReturn($connection);
+		$access->expects(self::once())
+			->method('username2dn')
+			->with('admin')
+			->willReturn('admin');
+		$access->expects(self::once())
+			->method('readAttribute')
+			->with('admin', 'mailAlias')
+			->willReturn(false);
+		$userBackend = $this->getMockBuilder(User_LDAP::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$userBackend->method('userExists')
+			->with('admin')
+			->willReturn(true);
+		$userBackend->method('getLDAPAccess')
+			->willReturn($access);
+		$groupBackend = $this->getMockBuilder(Group_LDAP::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$server = $this->getServerMock($userBackend, $groupBackend);
+
+		$ldapProvider = $this->getLDAPProvider($server);
+		$values = $ldapProvider->getMultiValueUserAttribute('admin', 'mailAlias');
+
+		self::assertCount(0, $values);
+	}
+
+	public function testGetMultiValueUserAttribute() {
+		$connection = $this->createMock(Connection::class);
+		$connection->expects(self::once())
+			->method('getFromCache')
+			->with('admin-mailAlias')
+			->willReturn(null);
+		$access = $this->createMock(Access::class);
+		$access->expects(self::once())
+			->method('getConnection')
+			->willReturn($connection);
+		$access->expects(self::once())
+			->method('username2dn')
+			->with('admin')
+			->willReturn('admin');
+		$access->expects(self::once())
+			->method('readAttribute')
+			->with('admin', 'mailAlias')
+			->willReturn(['aliasA@test.local', 'aliasB@test.local']);
+		$userBackend = $this->getMockBuilder(User_LDAP::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$userBackend->method('userExists')
+			->with('admin')
+			->willReturn(true);
+		$userBackend->method('getLDAPAccess')
+			->willReturn($access);
+		$groupBackend = $this->getMockBuilder(Group_LDAP::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$server = $this->getServerMock($userBackend, $groupBackend);
+
+		$ldapProvider = $this->getLDAPProvider($server);
+		$values = $ldapProvider->getMultiValueUserAttribute('admin', 'mailAlias');
+
+		self::assertCount(2, $values);
+	}
+
+	public function testGetUserAttributeLdapError() {
+		$connection = $this->createMock(Connection::class);
+		$connection->expects(self::once())
+			->method('getFromCache')
+			->with('admin-mailAlias')
+			->willReturn(null);
+		$access = $this->createMock(Access::class);
+		$access->expects(self::once())
+			->method('getConnection')
+			->willReturn($connection);
+		$access->expects(self::once())
+			->method('username2dn')
+			->with('admin')
+			->willReturn('admin');
+		$access->expects(self::once())
+			->method('readAttribute')
+			->with('admin', 'mailAlias')
+			->willReturn(false);
+		$userBackend = $this->getMockBuilder(User_LDAP::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$userBackend->method('userExists')
+			->with('admin')
+			->willReturn(true);
+		$userBackend->method('getLDAPAccess')
+			->willReturn($access);
+		$groupBackend = $this->getMockBuilder(Group_LDAP::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$server = $this->getServerMock($userBackend, $groupBackend);
+
+		$ldapProvider = $this->getLDAPProvider($server);
+		$value = $ldapProvider->getUserAttribute('admin', 'mailAlias');
+
+		self::assertNull($value);
+	}
+
+	public function testGetUserAttribute() {
+		$connection = $this->createMock(Connection::class);
+		$connection->expects(self::once())
+			->method('getFromCache')
+			->with('admin-mailAlias')
+			->willReturn(null);
+		$access = $this->createMock(Access::class);
+		$access->expects(self::once())
+			->method('getConnection')
+			->willReturn($connection);
+		$access->expects(self::once())
+			->method('username2dn')
+			->with('admin')
+			->willReturn('admin');
+		$access->expects(self::once())
+			->method('readAttribute')
+			->with('admin', 'mailAlias')
+			->willReturn(['aliasA@test.local', 'aliasB@test.local']);
+		$userBackend = $this->getMockBuilder(User_LDAP::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$userBackend->method('userExists')
+			->with('admin')
+			->willReturn(true);
+		$userBackend->method('getLDAPAccess')
+			->willReturn($access);
+		$groupBackend = $this->getMockBuilder(Group_LDAP::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$server = $this->getServerMock($userBackend, $groupBackend);
+
+		$ldapProvider = $this->getLDAPProvider($server);
+		$value = $ldapProvider->getUserAttribute('admin', 'mailAlias');
+
+		self::assertEquals('aliasA@test.local', $value);
+	}
 }