[stable30] fix: Return correct list of managers for a user

apps/provisioning_api/lib/Controller/AUserData.php

@@ -8,7 +8,7 @@
  */
 namespace OCA\Provisioning_API\Controller;
 
-use OC\Group\Manager;
+use OC\Group\Manager as GroupManager;
 use OC\User\Backend;
 use OC\User\NoUserException;
 use OC_Helper;
@@ -20,9 +20,10 @@
 use OCP\AppFramework\OCS\OCSNotFoundException;
 use OCP\AppFramework\OCSController;
 use OCP\Files\NotFoundException;
+use OCP\Group\ISubAdmin;
 use OCP\IConfig;
-use OCP\IGroupManager;
 use OCP\IRequest;
+use OCP\IUser;
 use OCP\IUserManager;
 use OCP\IUserSession;
 use OCP\L10N\IFactory;
@@ -45,35 +46,18 @@
  public const USER_FIELD_MANAGER = 'manager';
  public const USER_FIELD_NOTIFICATION_EMAIL = 'notify_email';
 
- /** @var IUserManager */
- protected $userManager;
- /** @var IConfig */
- protected $config;
- /** @var Manager */
- protected $groupManager;
- /** @var IUserSession */
- protected $userSession;
- /** @var IAccountManager */
- protected $accountManager;
- /** @var IFactory */
- protected $l10nFactory;
-
- public function __construct(string $appName,
+ public function __construct(
+ string $appName,
  IRequest $request,
- IUserManager $userManager,
- IConfig $config,
- IGroupManager $groupManager,
- IUserSession $userSession,
- IAccountManager $accountManager,
- IFactory $l10nFactory) {
+ protected IUserManager $userManager,
+ protected IConfig $config,
+ protected GroupManager $groupManager,
+ protected IUserSession $userSession,
+ protected IAccountManager $accountManager,
+ protected ISubAdmin $subAdminManager,
+ protected IFactory $l10nFactory,
+ ) {
  parent::__construct($appName, $request);
-
- $this->userManager = $userManager;
- $this->config = $config;
- $this->groupManager = $groupManager;
- $this->userSession = $userSession;
- $this->accountManager = $accountManager;
- $this->l10nFactory = $l10nFactory;
  }
 
  /**
@@ -136,8 +120,8 @@
  $data['backend'] = $targetUserObject->getBackendClassName();
  $data['subadmin'] = $this->getUserSubAdminGroupsData($targetUserObject->getUID());
  $data[self::USER_FIELD_QUOTA] = $this->fillStorageInfo($targetUserObject->getUID());
- $managerUids = $targetUserObject->getManagerUids();
- $data[self::USER_FIELD_MANAGER] = empty($managerUids) ? '' : $managerUids[0];
+ $managers = $this->getManagers($targetUserObject);
+ $data[self::USER_FIELD_MANAGER] = empty($managers) ? '' : $managers[0];
 
  try {
  if ($includeScopes) {
@@ -205,6 +189,34 @@
  return $data;
  }
 
+ /**
+ * @return string[]
+ */
+ protected function getManagers(IUser $user): array {
+ $currentLoggedInUser = $this->userSession->getUser();
+
+ $managerUids = $user->getManagerUids();
+ if ($this->groupManager->isAdmin($currentLoggedInUser->getUID()) || $this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID())) {
+ return $managerUids;
+ }
+
+ if ($this->subAdminManager->isSubAdmin($currentLoggedInUser)) {
+ $accessibleManagerUids = array_values(array_filter(
+ $managerUids,
+ function (string $managerUid) use ($currentLoggedInUser) {
+ $manager = $this->userManager->get($managerUid);
+ if (!($manager instanceof IUser)) {
+ return false;
+ }
+ return $this->subAdminManager->isUserAccessible($currentLoggedInUser, $manager);
+ },
+ ));
+ return $accessibleManagerUids;
+ }
+
+ return [];
+ }
+
  /**
  * Get the groups a user is a subadmin of
  *

apps/provisioning_api/lib/Controller/GroupsController.php

@@ -21,6 +21,7 @@
 use OCP\AppFramework\OCS\OCSForbiddenException;
 use OCP\AppFramework\OCS\OCSNotFoundException;
 use OCP\AppFramework\OCSController;
+use OCP\Group\ISubAdmin;
 use OCP\IConfig;
 use OCP\IGroup;
 use OCP\IGroupManager;
@@ -47,6 +48,7 @@ public function __construct(string $appName,
  IGroupManager $groupManager,
  IUserSession $userSession,
  IAccountManager $accountManager,
+ ISubAdmin $subAdminManager,
  IFactory $l10nFactory,
  LoggerInterface $logger) {
  parent::__construct($appName,
@@ -56,6 +58,7 @@ public function __construct(string $appName,
  $groupManager,
  $userSession,
  $accountManager,
+ $subAdminManager,
  $l10nFactory
  );
 

apps/provisioning_api/lib/Controller/UsersController.php

@@ -31,6 +31,7 @@
 use OCP\AppFramework\OCS\OCSNotFoundException;
 use OCP\AppFramework\OCSController;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Group\ISubAdmin;
 use OCP\HintException;
 use OCP\IConfig;
 use OCP\IGroup;
@@ -63,6 +64,7 @@ public function __construct(
  IGroupManager $groupManager,
  IUserSession $userSession,
  IAccountManager $accountManager,
+ ISubAdmin $subAdminManager,
  IFactory $l10nFactory,
  private IURLGenerator $urlGenerator,
  private LoggerInterface $logger,
@@ -81,6 +83,7 @@ public function __construct(
  $groupManager,
  $userSession,
  $accountManager,
+ $subAdminManager,
  $l10nFactory
  );
 

apps/provisioning_api/tests/Controller/GroupsControllerTest.php

@@ -8,10 +8,10 @@
 namespace OCA\Provisioning_API\Tests\Controller;
 
 use OC\Group\Manager;
-use OC\SubAdmin;
 use OC\User\NoUserException;
 use OCA\Provisioning_API\Controller\GroupsController;
 use OCP\Accounts\IAccountManager;
+use OCP\Group\ISubAdmin;
 use OCP\IConfig;
 use OCP\IRequest;
 use OCP\IUser;
@@ -34,13 +34,12 @@ class GroupsControllerTest extends \Test\TestCase {
  protected $userSession;
  /** @var IAccountManager|\PHPUnit\Framework\MockObject\MockObject */
  protected $accountManager;
+ /** @var ISubAdmin|\PHPUnit\Framework\MockObject\MockObject */
+ protected $subAdminManager;
  /** @var IFactory|\PHPUnit\Framework\MockObject\MockObject */
  protected $l10nFactory;
  /** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */
  protected $logger;
- /** @var SubAdmin|\PHPUnit\Framework\MockObject\MockObject */
- protected $subAdminManager;
-
  /** @var GroupsController|\PHPUnit\Framework\MockObject\MockObject */
  protected $api;
 
@@ -54,11 +53,10 @@ protected function setUp(): void {
  $this->groupManager = $this->createMock(Manager::class);
  $this->userSession = $this->createMock(IUserSession::class);
  $this->accountManager = $this->createMock(IAccountManager::class);
+ $this->subAdminManager = $this->createMock(ISubAdmin::class);
  $this->l10nFactory = $this->createMock(IFactory::class);
  $this->logger = $this->createMock(LoggerInterface::class);
 
- $this->subAdminManager = $this->createMock(SubAdmin::class);
-
  $this->groupManager
  ->method('getSubAdmin')
  ->willReturn($this->subAdminManager);
@@ -72,6 +70,7 @@ protected function setUp(): void {
  $this->groupManager,
  $this->userSession,
  $this->accountManager,
+ $this->subAdminManager,
  $this->l10nFactory,
  $this->logger
  ])

apps/provisioning_api/tests/Controller/UsersControllerTest.php

@@ -23,6 +23,7 @@
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCS\OCSException;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Group\ISubAdmin;
 use OCP\IConfig;
 use OCP\IGroup;
 use OCP\IL10N;
@@ -57,6 +58,8 @@ class UsersControllerTest extends TestCase {
  protected $api;
  /** @var IAccountManager|MockObject */
  protected $accountManager;
+ /** @var ISubAdmin|MockObject */
+ protected $subAdminManager;
  /** @var IURLGenerator|MockObject */
  protected $urlGenerator;
  /** @var IRequest|MockObject */
@@ -86,6 +89,7 @@ protected function setUp(): void {
  $this->logger = $this->createMock(LoggerInterface::class);
  $this->request = $this->createMock(IRequest::class);
  $this->accountManager = $this->createMock(IAccountManager::class);
+ $this->subAdminManager = $this->createMock(ISubAdmin::class);
  $this->urlGenerator = $this->createMock(IURLGenerator::class);
  $this->l10nFactory = $this->createMock(IFactory::class);
  $this->newUserMailHelper = $this->createMock(NewUserMailHelper::class);
@@ -108,6 +112,7 @@ protected function setUp(): void {
  $this->groupManager,
  $this->userSession,
  $this->accountManager,
+ $this->subAdminManager,
  $this->l10nFactory,
  $this->urlGenerator,
  $this->logger,
@@ -376,6 +381,7 @@ public function testAddUserSuccessfulWithDisplayName() {
  $this->groupManager,
  $this->userSession,
  $this->accountManager,
+ $this->subAdminManager,
  $this->l10nFactory,
  $this->urlGenerator,
  $this->logger,
@@ -931,7 +937,6 @@ public function testGetUserDataAsAdmin() {
  ->disableOriginalConstructor()
  ->getMock();
  $loggedInUser
- ->expects($this->exactly(2))
  ->method('getUID')
  ->willReturn('admin');
  $targetUser = $this->getMockBuilder(IUser::class)
@@ -941,16 +946,13 @@ public function testGetUserDataAsAdmin() {
  ->method('getSystemEMailAddress')
  ->willReturn('[email protected]');
  $this->userSession
- ->expects($this->once())
  ->method('getUser')
  ->willReturn($loggedInUser);
  $this->userManager
- ->expects($this->exactly(2))
  ->method('get')
  ->with('UID')
  ->willReturn($targetUser);
  $this->groupManager
- ->expects($this->once())
  ->method('isAdmin')
  ->with('admin')
  ->willReturn(true);
@@ -1077,7 +1079,6 @@ public function testGetUserDataAsSubAdminAndUserIsAccessible() {
  ->disableOriginalConstructor()
  ->getMock();
  $loggedInUser
- ->expects($this->exactly(2))
  ->method('getUID')
  ->willReturn('subadmin');
  $targetUser = $this->getMockBuilder(IUser::class)
@@ -1088,16 +1089,13 @@ public function testGetUserDataAsSubAdminAndUserIsAccessible() {
  ->method('getSystemEMailAddress')
  ->willReturn('[email protected]');
  $this->userSession
- ->expects($this->once())
  ->method('getUser')
  ->willReturn($loggedInUser);
  $this->userManager
- ->expects($this->exactly(2))
  ->method('get')
  ->with('UID')
  ->willReturn($targetUser);
  $this->groupManager
- ->expects($this->once())
  ->method('isAdmin')
  ->with('subadmin')
  ->willReturn(false);
@@ -1263,23 +1261,19 @@ public function testGetUserDataAsSubAdminSelfLookup() {
  ->disableOriginalConstructor()
  ->getMock();
  $loggedInUser
- ->expects($this->exactly(3))
  ->method('getUID')
  ->willReturn('UID');
  $targetUser = $this->getMockBuilder(IUser::class)
  ->disableOriginalConstructor()
  ->getMock();
  $this->userSession
- ->expects($this->once())
  ->method('getUser')
  ->willReturn($loggedInUser);
  $this->userManager
- ->expects($this->exactly(2))
  ->method('get')
  ->with('UID')
  ->willReturn($targetUser);
  $this->groupManager
- ->expects($this->once())
  ->method('isAdmin')
  ->with('UID')
  ->willReturn(false);
@@ -3659,6 +3653,7 @@ public function testGetCurrentUserLoggedIn() {
  $this->groupManager,
  $this->userSession,
  $this->accountManager,
+ $this->subAdminManager,
  $this->l10nFactory,
  $this->urlGenerator,
  $this->logger,
@@ -3746,6 +3741,7 @@ public function testGetUser() {
  $this->groupManager,
  $this->userSession,
  $this->accountManager,
+ $this->subAdminManager,
  $this->l10nFactory,
  $this->urlGenerator,
  $this->logger,

lib/private/SubAdmin.php

@@ -259,6 +259,9 @@ public function isSubAdmin(IUser $user): bool {
  * @return bool
  */
  public function isUserAccessible(IUser $subadmin, IUser $user): bool {
+ if ($subadmin->getUID() === $user->getUID()) {
+ return true;
+ }
  if (!$this->isSubAdmin($subadmin)) {
  return false;
  }