ignore non existing users when retrieving details of group members

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

3 files changed, 102 insertions, 19 deletions

diff --git a/apps/provisioning_api/lib/Controller/AUserData.php b/apps/provisioning_api/lib/Controller/AUserData.php
index 864ed65e69e..780d4f7a761 100644
--- a/apps/provisioning_api/lib/Controller/AUserData.php
+++ b/apps/provisioning_api/lib/Controller/AUserData.php
@@ -23,6 +23,7 @@ namespace OCA\Provisioning_API\Controller;
 
 use OC\Accounts\AccountManager;
 use OC\User\Backend;
+use OC\User\NoUserException;
 use OCP\AppFramework\OCS\OCSException;
 use OCP\AppFramework\OCS\OCSNotFoundException;
 use OCP\AppFramework\OCSController;
@@ -79,7 +80,9 @@ abstract class AUserData extends OCSController {
 	 *
 	 * @param string $userId
 	 * @return array
+	 * @throws NotFoundException
 	 * @throws OCSException
+	 * @throws OCSNotFoundException
 	 */
 	protected function getUserData(string $userId): array {
 		$currentLoggedInUser = $this->userSession->getUser();
@@ -111,9 +114,17 @@ abstract class AUserData extends OCSController {
 			$gids[] = $group->getGID();
 		}
 
+		try {
+			# might be thrown by LDAP due to handling of users disappears
+			# from the external source (reasons unknown to us)
+			# cf. https://github.com/nextcloud/server/issues/12991
+			$data['storageLocation'] = $targetUserObject->getHome();
+		} catch (NoUserException $e) {
+			throw new OCSNotFoundException($e->getMessage(), $e);
+		}
+
 		// Find the data
 		$data['id'] = $targetUserObject->getUID();
-		$data['storageLocation'] = $targetUserObject->getHome();
 		$data['lastLogin'] = $targetUserObject->getLastLogin() * 1000;
 		$data['backend'] = $targetUserObject->getBackendClassName();
 		$data['subadmin'] = $this->getUserSubAdminGroupsData($targetUserObject->getUID());
diff --git a/apps/provisioning_api/lib/Controller/GroupsController.php b/apps/provisioning_api/lib/Controller/GroupsController.php
index e52929df9c6..7c4447ac4eb 100644
--- a/apps/provisioning_api/lib/Controller/GroupsController.php
+++ b/apps/provisioning_api/lib/Controller/GroupsController.php
@@ -203,16 +203,20 @@ class GroupsController extends AUserData {
 			// Extract required number
 			$usersDetails = [];
 			foreach ($users as $user) {
-				/** @var IUser $user */
-				$userId = (string) $user->getUID();
-				$userData = $this->getUserData($userId);
-				// Do not insert empty entry
-				if(!empty($userData)) {
-					$usersDetails[$userId] = $userData;
-				} else {
-					// Logged user does not have permissions to see this user
-					// only showing its id
-					$usersDetails[$userId] = ['id' => $userId];
+				try {
+					/** @var IUser $user */
+					$userId = (string)$user->getUID();
+					$userData = $this->getUserData($userId);
+					// Do not insert empty entry
+					if (!empty($userData)) {
+						$usersDetails[$userId] = $userData;
+					} else {
+						// Logged user does not have permissions to see this user
+						// only showing its id
+						$usersDetails[$userId] = ['id' => $userId];
+					}
+				} catch(OCSNotFoundException $e) {
+					// continue if a users ceased to exist.
 				}
 			}
 			return new DataResponse(['users' => $usersDetails]);
diff --git a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php
index 2ed62a67841..b22953ca043 100644
--- a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php
+++ b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php
@@ -29,6 +29,7 @@ namespace OCA\Provisioning_API\Tests\Controller;
 use OC\Accounts\AccountManager;
 use OC\Group\Manager;
 use OC\SubAdmin;
+use OC\User\NoUserException;
 use OCA\Provisioning_API\Controller\GroupsController;
 use OCP\IConfig;
 use OCP\ILogger;
@@ -36,27 +37,31 @@ use OCP\IRequest;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\IUserSession;
+use OCP\UserInterface;
 
 class GroupsControllerTest extends \Test\TestCase {
 
-	/** @var IRequest|PHPUnit_Framework_MockObject_MockObject */
+	/** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */
 	protected $request;
-	/** @var IUserManager|PHPUnit_Framework_MockObject_MockObject */
+	/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
 	protected $userManager;
-	/** @var IConfig|PHPUnit_Framework_MockObject_MockObject */
+	/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
 	protected $config;
-	/** @var Manager|PHPUnit_Framework_MockObject_MockObject */
+	/** @var Manager|\PHPUnit_Framework_MockObject_MockObject */
 	protected $groupManager;
-	/** @var IUserSession|PHPUnit_Framework_MockObject_MockObject */
+	/** @var IUserSession|\PHPUnit_Framework_MockObject_MockObject */
 	protected $userSession;
-	/** @var AccountManager|PHPUnit_Framework_MockObject_MockObject */
+	/** @var AccountManager|\PHPUnit_Framework_MockObject_MockObject */
 	protected $accountManager;
-	/** @var ILogger|PHPUnit_Framework_MockObject_MockObject */
+	/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */
 	protected $logger;
+	/** @var  SubAdmin|\PHPUnit_Framework_MockObject_MockObject */
+	protected $subAdminManager;
 
-	/** @var GroupsController|PHPUnit_Framework_MockObject_MockObject */
+	/** @var GroupsController|\PHPUnit_Framework_MockObject_MockObject */
 	protected $api;
 
+
 	protected function setUp() {
 		parent::setUp();
 
@@ -126,6 +131,10 @@ class GroupsControllerTest extends \Test\TestCase {
 		$user
 			->method('getUID')
 			->willReturn($uid);
+		$backendMock = $this->createMock(UserInterface::class);
+		$user
+			->method('getBackend')
+			->willReturn($backendMock);
 		return $user;
 	}
 
@@ -164,6 +173,19 @@ class GroupsControllerTest extends \Test\TestCase {
 			}));
 	}
 
+	private function useAccountManager() {
+		$this->accountManager->expects($this->any())
+			->method('getUser')
+			->willReturnCallback(function(IUser $user) {
+				return [
+					AccountManager::PROPERTY_PHONE => ['value' => '0800-call-' . $user->getUID()],
+					AccountManager::PROPERTY_ADDRESS => ['value' => 'Holzweg 99, 0601 Herrera, Panama'],
+					AccountManager::PROPERTY_WEBSITE => ['value' => 'https://' . $user->getUid() . '.pa'],
+					AccountManager::PROPERTY_TWITTER => ['value' => '@' . $user->getUID()],
+				];
+			});
+	}
+
 	public function dataGetGroups() {
 		return [
 			[null, 0, 0],
@@ -454,4 +476,50 @@ class GroupsControllerTest extends \Test\TestCase {
 
 		$this->api->deleteGroup('ExistingGroup');
 	}
+
+	public function testGetGroupUsersDetails() {
+		$gid = 'ncg1';
+
+		$this->asAdmin();
+		$this->useAccountManager();
+
+		$users = [
+			'ncu1' => $this->createUser('ncu1'), # regular
+			'ncu2' => $this->createUser('ncu2'), # the zombie
+		];
+		$users['ncu2']->expects($this->atLeastOnce())
+			->method('getHome')
+			->willThrowException(new NoUserException());
+
+		$this->userManager->expects($this->any())
+			->method('get')
+			->willReturnCallback(function(string $uid) use ($users) {
+				return isset($users[$uid]) ? $users[$uid] : null;
+			});
+
+		$group = $this->createGroup($gid);
+		$group->expects($this->once())
+			->method('searchUsers')
+			->with('', null, 0)
+			->willReturn(array_values($users));
+
+		$this->groupManager
+			->method('get')
+			->with($gid)
+			->willReturn($group);
+		$this->groupManager->expects($this->any())
+			->method('getUserGroups')
+			->willReturn([$group]);
+
+		/** @var \PHPUnit_Framework_MockObject_MockObject */
+		$this->subAdminManager->expects($this->any())
+			->method('isSubAdminOfGroup')
+			->willReturn(false);
+		$this->subAdminManager->expects($this->any())
+			->method('getSubAdminsGroups')
+			->willReturn([]);
+
+
+		$this->api->getGroupUsersDetails($gid);
+	}
 }