diff --git a/apps/provisioning_api/lib/Controller/AUserData.php b/apps/provisioning_api/lib/Controller/AUserData.php index 25e5ab5c86..eaef07770c 100644 --- a/apps/provisioning_api/lib/Controller/AUserData.php +++ b/apps/provisioning_api/lib/Controller/AUserData.php @@ -22,6 +22,7 @@ declare(strict_types=1); namespace OCA\Provisioning_API\Controller; use OC\Accounts\AccountManager; +use OC\User\NoUserException; use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; @@ -76,7 +77,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(); @@ -108,9 +111,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 e52929df9c..7c4447ac4e 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 2ed62a6784..f07f29031c 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(); @@ -164,6 +169,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 +472,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); + } }