From 017ca09b7f38f04e9878c1454f45e0de63aa8535 Mon Sep 17 00:00:00 2001 From: Mikael Hammarin Date: Mon, 28 Oct 2019 15:48:46 +0100 Subject: [PATCH] Patch to optimize for large installations (>5000 users >20000 groups) where subadmins have access to many of groups (>250) - Current implementation in isUserAccessible() iterates over each group a subadmin is member of - UsersController:editUser() calls isUserAccessible() even if the user is admin This fix reduces API calls to editUser (ex change locale/display name) from >2 minutes (!) to ~3 seconds per call in average. Signed-off-by: Mikael Hammarin --- .../lib/Controller/UsersController.php | 4 ++-- lib/private/SubAdmin.php | 23 +++++++++++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index c69c7d6260570..1ac6477075039 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -499,8 +499,8 @@ public function editUser(string $userId, string $key, string $value): DataRespon } else { // Check if admin / subadmin $subAdminManager = $this->groupManager->getSubAdmin(); - if ($subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser) - || $this->groupManager->isAdmin($currentLoggedInUser->getUID())) { + if ($this->groupManager->isAdmin($currentLoggedInUser->getUID()) + || $subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) { // They have permissions over the user $permittedFields[] = 'display'; $permittedFields[] = AccountManager::PROPERTY_DISPLAYNAME; diff --git a/lib/private/SubAdmin.php b/lib/private/SubAdmin.php index f79a0b1ae5adb..b87b06afd5a2a 100644 --- a/lib/private/SubAdmin.php +++ b/lib/private/SubAdmin.php @@ -254,13 +254,22 @@ public function isUserAccessible(IUser $subadmin, IUser $user): bool { if($this->groupManager->isAdmin($user->getUID())) { return false; } - $accessibleGroups = $this->getSubAdminsGroups($subadmin); - foreach($accessibleGroups as $accessibleGroup) { - if($accessibleGroup->inGroup($user)) { - return true; - } - } - return false; + + $qb = $this->dbConn->getQueryBuilder(); + + $result = $qb->select('gm.uid') + ->from('group_admin', 'ga') + ->join('ga', 'group_admin', 'gm', 'ga.gid = gm.gid') + ->andWhere($qb->expr()->eq('ga.uid', $qb->createNamedParameter($subadmin->getUID()))) + ->andWhere($qb->expr()->eq('gm.uid', $qb->createNamedParameter($user->getUID()))) + ->setMaxResults(1) + ->execute(); + ; + + $isUserAccessible = $result->fetch(); + $result->closeCursor(); + + return $isUserAccessible !== false; } /**