From 6ffde74928651db451a8ca23ba440f16f8512681 Mon Sep 17 00:00:00 2001
From: Mikael Hammarin <mikael@try2.se>
Date: Mon, 28 Oct 2019 15:48:46 +0100
Subject: [PATCH 1/2] Patch to optimize for large installations (>5000 users
 >20000 groups) where subadmins have access to many of groups (>250) -
 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 <mikael@try2.se>
---
 apps/provisioning_api/lib/Controller/UsersController.php | 4 ++--
 lib/private/SubAdmin.php                                 | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php
index bd327ffe441bc..07a1514dd1f18 100644
--- a/apps/provisioning_api/lib/Controller/UsersController.php
+++ b/apps/provisioning_api/lib/Controller/UsersController.php
@@ -504,8 +504,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 d292e998ab94a..9a758ac4423c4 100644
--- a/lib/private/SubAdmin.php
+++ b/lib/private/SubAdmin.php
@@ -255,6 +255,7 @@ 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)) {

From e24e9ec0a9cf7081eea0f9fd41ac27f29a674c31 Mon Sep 17 00:00:00 2001
From: Joas Schilling <coding@schilljs.com>
Date: Mon, 27 Apr 2020 08:37:53 +0200
Subject: [PATCH 2/2] Don't loop over all groups to check for subadmins

Signed-off-by: Joas Schilling <coding@schilljs.com>
---
 lib/private/SubAdmin.php | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/lib/private/SubAdmin.php b/lib/private/SubAdmin.php
index 9a758ac4423c4..890bcf67b3bcb 100644
--- a/lib/private/SubAdmin.php
+++ b/lib/private/SubAdmin.php
@@ -110,6 +110,25 @@ public function deleteSubAdmin(IUser $user, IGroup $group): void {
 	 * @return IGroup[]
 	 */
 	public function getSubAdminsGroups(IUser $user): array {
+		$groupIds = $this->getSubAdminsGroupIds($user);
+
+		$groups = [];
+		foreach ($groupIds as $groupId) {
+			$group = $this->groupManager->get($groupId);
+			if ($group !== null) {
+				$groups[$group->getGID()] = $group;
+			}
+		}
+
+		return $groups;
+	}
+
+	/**
+	 * Get group ids of a SubAdmin
+	 * @param IUser $user the SubAdmin
+	 * @return string[]
+	 */
+	public function getSubAdminsGroupIds(IUser $user): array {
 		$qb = $this->dbConn->getQueryBuilder();
 
 		$result = $qb->select('gid')
@@ -119,10 +138,7 @@ public function getSubAdminsGroups(IUser $user): array {
 
 		$groups = [];
 		while ($row = $result->fetch()) {
-			$group = $this->groupManager->get($row['gid']);
-			if (!is_null($group)) {
-				$groups[$group->getGID()] = $group;
-			}
+			$groups[] = $row['gid'];
 		}
 		$result->closeCursor();
 
@@ -256,13 +272,10 @@ public function isUserAccessible(IUser $subadmin, IUser $user): bool {
 			return false;
 		}
 
-		$accessibleGroups = $this->getSubAdminsGroups($subadmin);
-		foreach ($accessibleGroups as $accessibleGroup) {
-			if ($accessibleGroup->inGroup($user)) {
-				return true;
-			}
-		}
-		return false;
+		$accessibleGroups = $this->getSubAdminsGroupIds($subadmin);
+		$userGroups = $this->groupManager->getUserGroupIds($user);
+
+		return !empty(array_intersect($accessibleGroups, $userGroups));
 	}
 
 	/**