Merge pull request #13644 from nextcloud/fix/12991/catch-nouserexception-groupusersdetails
ignore non existing users when retrieving details of group members
This commit is contained in:
Morris Jobke
GitHub
commit
6cdb392ec5
3 changed files with 102 additions and 19 deletions
|
|
@ -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());
|
||||
|
|
|
|||
|
|
@ -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]);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue