Merge pull request #10543 from nextcloud/ignore-deactivated-users

Do not show deactivated users in sharees and contacts

apps/dav/lib/CardDAV/SyncService.php

@@ -270,18 +270,22 @@ class SyncService {
 
 		$cardId = "$name:$userId.vcf";
 		$card = $this->backend->getCard($addressBookId, $cardId);
-		if ($card === false) {
-			$vCard = $converter->createCardFromUser($user);
-			if ($vCard !== null) {
-				$this->backend->createCard($addressBookId, $cardId, $vCard->serialize());
+		if ($user->isEnabled()) {
+			if ($card === false) {
+				$vCard = $converter->createCardFromUser($user);
+				if ($vCard !== null) {
+					$this->backend->createCard($addressBookId, $cardId, $vCard->serialize());
+				}
+			} else {
+				$vCard = $converter->createCardFromUser($user);
+				if (is_null($vCard)) {
+					$this->backend->deleteCard($addressBookId, $cardId);
+				} else {
+					$this->backend->updateCard($addressBookId, $cardId, $vCard->serialize());
+				}
 			}
 		} else {
-			$vCard = $converter->createCardFromUser($user);
-			if (is_null($vCard)) {
-				$this->backend->deleteCard($addressBookId, $cardId);
-			} else {
-				$this->backend->updateCard($addressBookId, $cardId, $vCard->serialize());
-			}
+			$this->backend->deleteCard($addressBookId, $cardId);
 		}
 	}
 

apps/dav/tests/unit/CardDAV/SyncServiceTest.php

@@ -84,14 +84,30 @@ class SyncServiceTest extends TestCase {
 		$ss->ensureSystemAddressBookExists('principals/users/adam', 'contacts', []);
 	}
 
-	public function testUpdateAndDeleteUser() {
+	public function dataActivatedUsers() {
+		return [
+			[true, 1, 1, 1],
+			[false, 0, 0, 3],
+		];
+	}
+
+	/**
+	 * @dataProvider dataActivatedUsers
+	 *
+	 * @param boolean $activated
+	 * @param integer $createCalls
+	 * @param integer $updateCalls
+	 * @param integer $deleteCalls
+	 * @return void
+	 */
+	public function testUpdateAndDeleteUser($activated, $createCalls, $updateCalls, $deleteCalls) {
 		/** @var CardDavBackend | \PHPUnit_Framework_MockObject_MockObject $backend */
 		$backend = $this->getMockBuilder(CardDavBackend::class)->disableOriginalConstructor()->getMock();
 		$logger = $this->getMockBuilder(ILogger::class)->disableOriginalConstructor()->getMock();
 
-		$backend->expects($this->once())->method('createCard');
-		$backend->expects($this->once())->method('updateCard');
-		$backend->expects($this->once())->method('deleteCard');
+		$backend->expects($this->exactly($createCalls))->method('createCard');
+		$backend->expects($this->exactly($updateCalls))->method('updateCard');
+		$backend->expects($this->exactly($deleteCalls))->method('deleteCard');
 
 		$backend->method('getCard')->willReturnOnConsecutiveCalls(false, [
 			'carddata' => "BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.4.8//EN\r\nUID:test-user\r\nFN:test-user\r\nN:test-user;;;;\r\nEND:VCARD\r\n\r\n"
@@ -106,6 +122,7 @@ class SyncServiceTest extends TestCase {
 		$user->method('getUID')->willReturn('test-user');
 		$user->method('getCloudId')->willReturn('cloudId');
 		$user->method('getDisplayName')->willReturn('test-user');
+		$user->method('isEnabled')->willReturn($activated);
 		$accountManager = $this->getMockBuilder(AccountManager::class)->disableOriginalConstructor()->getMock();
 		$accountManager->expects($this->any())->method('getUser')
 			->willReturn([

lib/private/Collaboration/Collaborators/UserPlugin.php

@@ -79,7 +79,9 @@ class UserPlugin implements ISearchPlugin {
 			$usersTmp = $this->userManager->searchDisplayName($search, $limit, $offset);
 
 			foreach ($usersTmp as $user) {
-				$users[$user->getUID()] = $user->getDisplayName();
+				if ($user->isEnabled()) { // Don't keep deactivated users
+					$users[$user->getUID()] = $user->getDisplayName();
+				}
 			}
 		}
 

tests/lib/Collaboration/Collaborators/UserPluginTest.php

@@ -89,7 +89,7 @@ class UserPluginTest extends TestCase {
 		);
 	}
 
-	public function getUserMock($uid, $displayName) {
+	public function getUserMock($uid, $displayName, $enabled = true) {
 		$user = $this->createMock(IUser::class);
 
 		$user->expects($this->any())
@@ -100,6 +100,10 @@ class UserPluginTest extends TestCase {
 			->method('getDisplayName')
 			->willReturn($displayName);
 
+		$user->expects($this->any())
+			->method('isEnabled')
+			->willReturn($enabled);
+
 		return $user;
 	}