From d3bc8b771c78ca881c127b6f614eb3fc9c4107ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 27 Feb 2025 16:21:30 +0100 Subject: [PATCH 1/3] fix: Correctly count disabled users for SAML groups subadmins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If too many users return -1 as for LDAP so that link is shown Signed-off-by: Côme Chilliet --- .../lib/Controller/UsersController.php | 16 +++-------- lib/private/User/Manager.php | 27 +++++++++++++++++-- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index 9fe1fe40cf4b5..fbf0789694dc0 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -148,20 +148,12 @@ public function usersList(): TemplateResponse { }, 0); } else { // User is subadmin ! - // Map group list to ids to retrieve the countDisabledUsersOfGroups - $userGroups = $this->groupManager->getUserGroups($user); - $groupsIds = []; - - foreach ($groups as $key => $group) { - // $userCount += (int)$group['usercount']; - $groupsIds[] = $group['id']; - } - - $userCount += $this->userManager->countUsersOfGroups($groupsInfo->getGroups()); - $disabledUsers = $this->userManager->countDisabledUsersOfGroups($groupsIds); + [$userCount,$disabledUsers] = $this->userManager->countUsersAndDisabledUsersOfGroups($groupsInfo->getGroups(), 999); } - $userCount -= $disabledUsers; + if ($disabledUsers > 0) { + $userCount -= $disabledUsers; + } } $recentUsersGroup = [ diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 501f13e98c635..9324810e9075a 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -492,8 +492,7 @@ public function countUsers() { * returns how many users per backend exist in the requested groups (if supported by backend) * * @param IGroup[] $groups an array of gid to search in - * @return array|int an array of backend class as key and count number as value - * if $hasLoggedIn is true only an int is returned + * @return int */ public function countUsersOfGroups(array $groups) { $users = []; @@ -506,6 +505,30 @@ public function countUsersOfGroups(array $groups) { return count(array_unique($users)); } + /** + * returns how many users per backend exist in the requested groups (if supported by backend) + * + * @param IGroup[] $groups an array of groups to search in + * @param int $limit limit to stop counting + * @return array{int,int} total number of users, and number of disabled users in the given groups, below $limit. If limit is reached, -1 is returned for number of disabled users + */ + public function countUsersAndDisabledUsersOfGroups(array $groups, int $limit): array { + $users = []; + $disabled = []; + foreach ($groups as $group) { + foreach ($group->getUsers() as $user) { + $users[$user->getUID()] = 1; + if (!$user->isEnabled()) { + $disabled[$user->getUID()] = 1; + } + if (count($users) >= $limit) { + return [count($users),-1]; + } + } + } + return [count($users),count($disabled)]; + } + /** * The callback is executed for each user on each backend. * If the callback returns false no further users will be retrieved. From 107c18dff2ef4ca9b3cd871493dd8c52b8b0000b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 27 Feb 2025 16:23:01 +0100 Subject: [PATCH 2/3] chore: Remove now unused methods from User manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/User/Manager.php | 47 ------------------------------------ 1 file changed, 47 deletions(-) diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 9324810e9075a..229abb273ff12 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -488,23 +488,6 @@ public function countUsers() { return $userCountStatistics; } - /** - * returns how many users per backend exist in the requested groups (if supported by backend) - * - * @param IGroup[] $groups an array of gid to search in - * @return int - */ - public function countUsersOfGroups(array $groups) { - $users = []; - foreach ($groups as $group) { - $usersIds = array_map(function ($user) { - return $user->getUID(); - }, $group->getUsers()); - $users = array_merge($users, $usersIds); - } - return count(array_unique($users)); - } - /** * returns how many users per backend exist in the requested groups (if supported by backend) * @@ -592,36 +575,6 @@ public function countDisabledUsers(): int { return $count; } - /** - * returns how many users are disabled in the requested groups - * - * @param array $groups groupids to search - * @return int - * @since 14.0.0 - */ - public function countDisabledUsersOfGroups(array $groups): int { - $queryBuilder = \OC::$server->getDatabaseConnection()->getQueryBuilder(); - $queryBuilder->select($queryBuilder->createFunction('COUNT(DISTINCT ' . $queryBuilder->getColumnName('uid') . ')')) - ->from('preferences', 'p') - ->innerJoin('p', 'group_user', 'g', $queryBuilder->expr()->eq('p.userid', 'g.uid')) - ->where($queryBuilder->expr()->eq('appid', $queryBuilder->createNamedParameter('core'))) - ->andWhere($queryBuilder->expr()->eq('configkey', $queryBuilder->createNamedParameter('enabled'))) - ->andWhere($queryBuilder->expr()->eq('configvalue', $queryBuilder->createNamedParameter('false'), IQueryBuilder::PARAM_STR)) - ->andWhere($queryBuilder->expr()->in('gid', $queryBuilder->createNamedParameter($groups, IQueryBuilder::PARAM_STR_ARRAY))); - - $result = $queryBuilder->execute(); - $count = $result->fetchOne(); - $result->closeCursor(); - - if ($count !== false) { - $count = (int)$count; - } else { - $count = 0; - } - - return $count; - } - /** * returns how many users have logged in once * From 3d33ac2d34a526a6307993890e8a42aac29a2436 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 6 Mar 2025 16:36:47 +0100 Subject: [PATCH 3/3] chore: Move magic number into a documented const MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/settings/lib/Controller/UsersController.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index fbf0789694dc0..a784178701309 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -51,6 +51,8 @@ #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] class UsersController extends Controller { + /** Limit for counting users for subadmins, to avoid spending too much time */ + private const COUNT_LIMIT_FOR_SUBADMINS = 999; public function __construct( string $appName, @@ -148,7 +150,7 @@ public function usersList(): TemplateResponse { }, 0); } else { // User is subadmin ! - [$userCount,$disabledUsers] = $this->userManager->countUsersAndDisabledUsersOfGroups($groupsInfo->getGroups(), 999); + [$userCount,$disabledUsers] = $this->userManager->countUsersAndDisabledUsersOfGroups($groupsInfo->getGroups(), self::COUNT_LIMIT_FOR_SUBADMINS); } if ($disabledUsers > 0) {