Merge pull request #9662 from nextcloud/backport/9640/stable13

[stable13] check user state when fetching to avoid dealing with offline objects

2 files changed, 56 insertions, 7 deletions

diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php
index f2fc43ed9b8..02f69715f4c 100644
--- a/apps/user_ldap/lib/Access.php
+++ b/apps/user_ldap/lib/Access.php
@@ -651,6 +651,7 @@ class Access extends LDAPUtility implements IUserTools {
 	 * @param array $ldapObjects as returned by fetchList()
 	 * @param bool $isUsers
 	 * @return array
+	 * @throws \Exception
 	 */
 	private function ldap2NextcloudNames($ldapObjects, $isUsers) {
 		if($isUsers) {
@@ -659,7 +660,7 @@ class Access extends LDAPUtility implements IUserTools {
 		} else {
 			$nameAttribute = $this->connection->ldapGroupDisplayName;
 		}
-		$nextcloudNames = array();
+		$nextcloudNames = [];
 
 		foreach($ldapObjects as $ldapObject) {
 			$nameByLDAP = null;
@@ -675,6 +676,7 @@ class Access extends LDAPUtility implements IUserTools {
 			if($ncName) {
 				$nextcloudNames[] = $ncName;
 				if($isUsers) {
+					$this->updateUserState($ncName);
 					//cache the user names so it does not need to be retrieved
 					//again later (e.g. sharing dialogue).
 					if(is_null($nameByLDAP)) {
@@ -690,6 +692,19 @@ class Access extends LDAPUtility implements IUserTools {
 	}
 
 	/**
+	 * removes the deleted-flag of a user if it was set
+	 *
+	 * @param string $ncname
+	 * @throws \Exception
+	 */
+	public function updateUserState($ncname) {
+		$user = $this->userManager->get($ncname);
+		if($user instanceof OfflineUser) {
+			$user->unmark();
+		}
+	}
+
+	/**
 	 * caches the user display name
 	 * @param string $ocName the internal Nextcloud username
 	 * @param string|false $home the home directory path
@@ -862,7 +877,9 @@ class Access extends LDAPUtility implements IUserTools {
 	 * provided with an array of LDAP user records the method will fetch the
 	 * user object and requests it to process the freshly fetched attributes and
 	 * and their values
+	 *
 	 * @param array $ldapRecords
+	 * @throws \Exception
 	 */
 	public function batchApplyUserAttributes(array $ldapRecords){
 		$displayNameAttribute = strtolower($this->connection->ldapUserDisplayName);
@@ -875,11 +892,8 @@ class Access extends LDAPUtility implements IUserTools {
 			if($ocName === false) {
 				continue;
 			}
+			$this->updateUserState($ocName);
 			$user = $this->userManager->get($ocName);
-			if($user instanceof OfflineUser) {
-				$user->unmark();
-				$user = $this->userManager->get($ocName);
-			}
 			if ($user !== null) {
 				$user->processAttributes($userRecord);
 			} else {
diff --git a/apps/user_ldap/tests/AccessTest.php b/apps/user_ldap/tests/AccessTest.php
index 336b92af04f..14750e13328 100644
--- a/apps/user_ldap/tests/AccessTest.php
+++ b/apps/user_ldap/tests/AccessTest.php
@@ -43,6 +43,7 @@ use OCA\User_LDAP\LDAP;
 use OCA\User_LDAP\LogWrapper;
 use OCA\User_LDAP\Mapping\UserMapping;
 use OCA\User_LDAP\User\Manager;
+use OCA\User_LDAP\User\OfflineUser;
 use OCA\User_LDAP\User\User;
 use OCP\IAvatarManager;
 use OCP\IConfig;
@@ -312,7 +313,7 @@ class AccessTest extends TestCase {
 		$userMock->expects($this->exactly(count($data)))
 			->method('processAttributes');
 
-		$this->userManager->expects($this->exactly(count($data)))
+		$this->userManager->expects($this->exactly(count($data) * 2))
 			->method('get')
 			->will($this->returnValue($userMock));
 
@@ -394,7 +395,7 @@ class AccessTest extends TestCase {
 		$userMock->expects($this->exactly(count($data)))
 			->method('processAttributes');
 
-		$this->userManager->expects($this->exactly(count($data)))
+		$this->userManager->expects($this->exactly(count($data) * 2))
 			->method('get')
 			->will($this->returnValue($userMock));
 
@@ -662,6 +663,40 @@ class AccessTest extends TestCase {
 		$this->assertSame($expected, $sanitizedName);
 	}
 
+	public function testUserStateUpdate() {
+		$this->connection->expects($this->any())
+			->method('__get')
+			->willReturnMap([
+				[ 'ldapUserDisplayName', 'displayName' ],
+				[ 'ldapUserDisplayName2', null],
+			]);
+
+		$offlineUserMock = $this->createMock(OfflineUser::class);
+		$offlineUserMock->expects($this->once())
+			->method('unmark');
+
+		$regularUserMock = $this->createMock(User::class);
+
+		$this->userManager->expects($this->atLeastOnce())
+			->method('get')
+			->with('detta')
+			->willReturnOnConsecutiveCalls($offlineUserMock, $regularUserMock);
+
+		/** @var UserMapping|\PHPUnit_Framework_MockObject_MockObject $mapperMock */
+		$mapperMock = $this->createMock(UserMapping::class);
+		$mapperMock->expects($this->any())
+			->method('getNameByDN')
+			->with('uid=detta,ou=users,dc=hex,dc=ample')
+			->willReturn('detta');
+		$this->access->setUserMapper($mapperMock);
 
+		$records = [
+			[
+				'dn' => ['uid=detta,ou=users,dc=hex,dc=ample'],
+				'displayName' => ['Detta Detkova'],
+			]
+		];
+		$this->access->nextcloudUserNames($records);
+	}
 
 }