diff --git a/typo3/sysext/backend/Classes/Utility/BackendUtility.php b/typo3/sysext/backend/Classes/Utility/BackendUtility.php index c2b15668aad427fb63827a874ef11c5db5cd7fe3..3e2cc5f859c845228d04a8017b49ac7ecdb32d64 100644 --- a/typo3/sysext/backend/Classes/Utility/BackendUtility.php +++ b/typo3/sysext/backend/Classes/Utility/BackendUtility.php @@ -742,12 +742,12 @@ class BackendUtility * Returns an array with be_users records of all user NOT DELETED sorted by their username * Keys in the array is the be_users uid * - * @param string $fields Optional $fields list (default: username,usergroup,usergroup_cached_list,uid) can be used to set the selected fields + * @param string $fields Optional $fields list (default: username,usergroup,uid) can be used to set the selected fields * @param string $where Optional $where clause (fx. "AND username='pete'") can be used to limit query * @return array * @internal should only be used from within TYPO3 Core, use a direct SQL query instead to ensure proper DBAL where statements */ - public static function getUserNames($fields = 'username,usergroup,usergroup_cached_list,uid', $where = '') + public static function getUserNames($fields = 'username,usergroup,uid', $where = '') { return self::getRecordsSortedByTitle( GeneralUtility::trimExplode(',', $fields, true), diff --git a/typo3/sysext/belog/Classes/Domain/Repository/LogEntryRepository.php b/typo3/sysext/belog/Classes/Domain/Repository/LogEntryRepository.php index 3ef418cf840fdbdf977bb4af722ace1710380683..4ce76800108a79d99ce50c1734c22db6e65e82f8 100644 --- a/typo3/sysext/belog/Classes/Domain/Repository/LogEntryRepository.php +++ b/typo3/sysext/belog/Classes/Domain/Repository/LogEntryRepository.php @@ -16,10 +16,10 @@ namespace TYPO3\CMS\Belog\Domain\Repository; use TYPO3\CMS\Backend\Tree\View\PageTreeView; -use TYPO3\CMS\Backend\Utility\BackendUtility; use TYPO3\CMS\Belog\Domain\Model\Constraint; use TYPO3\CMS\Belog\Domain\Model\LogEntry; use TYPO3\CMS\Belog\Domain\Model\Workspace; +use TYPO3\CMS\Core\Authentication\GroupResolver; use TYPO3\CMS\Core\Database\ConnectionPool; use TYPO3\CMS\Core\Type\Bitmask\Permission; use TYPO3\CMS\Core\Utility\GeneralUtility; @@ -33,19 +33,11 @@ use TYPO3\CMS\Extbase\Persistence\Repository; */ class LogEntryRepository extends Repository { - /** - * Backend users, with UID as key - * - * @var array - */ - protected $beUserList = []; - /** * Initialize some local variables to be used during creation of objects */ public function initializeObject() { - $this->beUserList = $this->getBackendUsers(); /** @var \TYPO3\CMS\Extbase\Persistence\Generic\QuerySettingsInterface $defaultQuerySettings */ $defaultQuerySettings = $this->objectManager->get(QuerySettingsInterface::class); $defaultQuerySettings->setRespectStoragePage(false); @@ -146,13 +138,11 @@ class LogEntryRepository extends Repository // Constraint for a group if (strpos($userOrGroup, 'gr-') === 0) { $groupId = (int)substr($userOrGroup, 3); - $userIds = []; - foreach ($this->beUserList as $userId => $userData) { - if (GeneralUtility::inList($userData['usergroup_cached_list'], (string)$groupId)) { - $userIds[] = $userId; - } - } + $groupResolver = GeneralUtility::makeInstance(GroupResolver::class); + $userIds = $groupResolver->findAllUsersInGroups([$groupId], 'be_groups', 'be_users'); if (!empty($userIds)) { + $userIds = array_column($userIds, 'uid'); + $userIds = array_map('intval', $userIds); $queryConstraints[] = $query->in('userid', $userIds); } else { // If there are no group members -> use -1 as constraint to not find anything @@ -186,14 +176,4 @@ class LogEntryRepository extends Repository ->where(...$constraints) ->execute(); } - - /** - * Get a list of all backend users that are not deleted - * - * @return array - */ - protected function getBackendUsers() - { - return BackendUtility::getUserNames(); - } } diff --git a/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php b/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php index 4eecb2fbdb13f0a1c29195ea7675e2bb0b6088c0..d0b392d45abe7c073eb1f3c0a386033c9f8a61ac 100644 --- a/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php +++ b/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php @@ -1274,11 +1274,10 @@ class BackendUserAuthentication extends AbstractUserAuthentication } // Populating the $this->userGroupsUID -array with the groups in the order in which they were LAST included.!! - $this->userGroupsUID = array_reverse(array_unique(array_reverse($this->userGroupsUID))); // Finally this is the list of group_uid's in the order they are parsed (including subgroups!) // and without duplicates (duplicates are presented with their last entrance in the list, // which thus reflects the order of the TypoScript in TSconfig) - $this->setCachedList(implode(',', $this->userGroupsUID)); + $this->userGroupsUID = array_reverse(array_unique(array_reverse($this->userGroupsUID))); $this->prepareUserTsConfig(); @@ -1395,27 +1394,6 @@ TCAdefaults.sys_note.email = ' . $this->user['email']; } } - /** - * Updates the field be_users.usergroup_cached_list if the groupList of the user - * has changed/is different from the current list. - * The field "usergroup_cached_list" contains the list of groups which the user is a member of. - * After authentication (where these functions are called...) one can depend on this list being - * a representation of the exact groups/subgroups which the BE_USER has membership with. - * - * @param string $cList The newly compiled group-list which must be compared with the current list in the user record and possibly stored if a difference is detected. - * @internal - */ - public function setCachedList($cList) - { - if ((string)$cList != (string)$this->user['usergroup_cached_list']) { - GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable('be_users')->update( - 'be_users', - ['usergroup_cached_list' => $cList], - ['uid' => (int)$this->user['uid']] - ); - } - } - /** * Sets up all file storages for a user. * Needs to be called AFTER the groups have been loaded. diff --git a/typo3/sysext/core/Classes/Authentication/GroupResolver.php b/typo3/sysext/core/Classes/Authentication/GroupResolver.php index 673121fea3e3412a236582db2dae4b4ecf3cdde9..44fd58e0b3d104ee15f76a84e1444e2b65dfdc9a 100644 --- a/typo3/sysext/core/Classes/Authentication/GroupResolver.php +++ b/typo3/sysext/core/Classes/Authentication/GroupResolver.php @@ -127,4 +127,107 @@ class GroupResolver } return $groups; } + + /** + * This works the other way around: Find all users that belong to some groups. Because groups are nested, + * we need to find all groups and subgroups first, because maybe a user is only part of a higher group, + * instead of a "All editors" group. + * + * @param int[] $groupIds a list of IDs of groups + * @param string $sourceTable e.g. be_groups or fe_groups + * @param string $userSourceTable e.g. be_users or fe_users + * @return array full user records + */ + public function findAllUsersInGroups(array $groupIds, string $sourceTable, string $userSourceTable): array + { + $this->sourceTable = $sourceTable; + + // Ensure the given groups exist + $mainGroups = $this->fetchRowsFromDatabase($groupIds); + $groupIds = array_map('intval', array_column($mainGroups, 'uid')); + if (empty($groupIds)) { + return []; + } + $parentGroupIds = $this->fetchParentGroupsRecursive($groupIds, $groupIds); + $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable($userSourceTable); + $queryBuilder + ->select('*') + ->from($userSourceTable); + + $constraints = []; + foreach ($groupIds as $groupUid) { + $constraints[] = $queryBuilder->expr()->inSet($this->sourceField, (string)$groupUid); + } + foreach ($parentGroupIds as $groupUid) { + $constraints[] = $queryBuilder->expr()->inSet($this->sourceField, (string)$groupUid); + } + + $users = $queryBuilder + ->where( + $queryBuilder->expr()->orX(...$constraints) + ) + ->execute() + ->fetchAll(); + return !empty($users) ? $users : []; + } + + /** + * Load a list of group uids, and take into account if groups have been loaded before as part of recursive detection. + * + * @param int[] $groupIds a list of groups to find THEIR ancestors + * @param array $processedGroupIds helper function to avoid recursive detection + * @return array a list of parent groups and thus, grand grand parent groups as well + */ + protected function fetchParentGroupsRecursive(array $groupIds, array $processedGroupIds = []): array + { + if (empty($groupIds)) { + return []; + } + $parentGroups = $this->fetchParentGroupsFromDatabase($groupIds); + $validParentGroupIds = []; + foreach ($parentGroups as $parentGroup) { + $parentGroupId = (int)$parentGroup['uid']; + // Record was already processed, continue to avoid adding this group again + if (in_array($parentGroupId, $processedGroupIds, true)) { + continue; + } + $processedGroupIds[] = $parentGroupId; + $validParentGroupIds[] = $parentGroupId; + } + + $grandParentGroups = $this->fetchParentGroupsRecursive($validParentGroupIds, $processedGroupIds); + return array_merge($validParentGroupIds, $grandParentGroups); + } + + /** + * Find all groups that have a FIND_IN_SET(subgroups, [$subgroupIds]) => the parent groups + * via one SQL query. + * + * @param array $subgroupIds + * @return array + */ + protected function fetchParentGroupsFromDatabase(array $subgroupIds): array + { + $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable($this->sourceTable); + $queryBuilder + ->select('*') + ->from($this->sourceTable); + + $constraints = []; + foreach ($subgroupIds as $subgroupId) { + $constraints[] = $queryBuilder->expr()->inSet($this->recursiveSourceField, (string)$subgroupId); + } + + $result = $queryBuilder + ->where( + $queryBuilder->expr()->orX(...$constraints) + ) + ->execute(); + + $groups = []; + while ($row = $result->fetch()) { + $groups[(int)$row['uid']] = $row; + } + return $groups; + } } diff --git a/typo3/sysext/core/Documentation/Changelog/master/Breaking-79565-RemovedUsergroup_cached_listDatabaseField.rst b/typo3/sysext/core/Documentation/Changelog/master/Breaking-79565-RemovedUsergroup_cached_listDatabaseField.rst new file mode 100644 index 0000000000000000000000000000000000000000..534314d8abebd77e70fc6991700b8416aea08d67 --- /dev/null +++ b/typo3/sysext/core/Documentation/Changelog/master/Breaking-79565-RemovedUsergroup_cached_listDatabaseField.rst @@ -0,0 +1,38 @@ +.. include:: ../../Includes.txt + +================================================================= +Breaking: #79565 - Removed "usergroup_cached_list" database field +================================================================= + +See :issue:`79565` + +Description +=========== + +The database field `be_users.usergroup_cached_list` has been +removed. It was populated by a list of all groups (including +subgroups) the user belongs to, and stored when a user logged in, +but was never updated when an admin added or removed a group +from the users' group list. + + +Impact +====== + +The mentioned database field is removed, any direct SQL queries +accessing or writing this field will result in a database error. + + +Affected Installations +====================== + +TYPO3 installations using or querying this database field +with third-party extensions. + + +Migration +========= + +Use the GroupResolver class to fetch all groups of a user directly. + +.. index:: Backend, PHP-API, NotScanned, ext:core \ No newline at end of file diff --git a/typo3/sysext/core/Tests/Functional/Authentication/GroupResolverTest.php b/typo3/sysext/core/Tests/Functional/Authentication/GroupResolverTest.php new file mode 100644 index 0000000000000000000000000000000000000000..83fa6a26330586f5448dd11d21a81237d3c864fa --- /dev/null +++ b/typo3/sysext/core/Tests/Functional/Authentication/GroupResolverTest.php @@ -0,0 +1,79 @@ +<?php + +declare(strict_types=1); + +/* + * This file is part of the TYPO3 CMS project. + * + * It is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License, either version 2 + * of the License, or any later version. + * + * For the full copyright and license information, please read the + * LICENSE.txt file that was distributed with this source code. + * + * The TYPO3 project - inspiring people to share! + */ + +namespace TYPO3\CMS\Core\Tests\Functional\Authentication; + +use TYPO3\CMS\Core\Authentication\GroupResolver; +use TYPO3\CMS\Core\Utility\GeneralUtility; +use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase; + +class GroupResolverTest extends FunctionalTestCase +{ + public function setUp(): void + { + parent::setUp(); + $this->importDataSet(__DIR__ . '/Fixtures/be_users.xml'); + $this->importDataSet(__DIR__ . '/Fixtures/be_groups.xml'); + } + + public function findAllUsersOfGroupsHandlesRecursiveCallsDataProvider(): array + { + return [ + 'invalid group' => [ + [238], + [] + ], + 'direct group with multiple users' => [ + [1], + [2, 3] + ], + 'direct group with one users' => [ + [4], + [3] + ], + 'direct and indirect subgroup with one users' => [ + [2], + [3] + ], + 'subgroup with no direct reference' => [ + [5], + [3] + ], + 'subgroup and direct with no direct reference' => [ + [5, 2, 3], + [3] + ], + 'no group given' => [ + [], + [] + ], + ]; + } + + /** + * @test + * @dataProvider findAllUsersOfGroupsHandlesRecursiveCallsDataProvider + * @param int[] $groupIds + * @param array $expectedUsers + */ + public function findAllUsersOfGroupsHandlesRecursiveCalls(array $groupIds, array $expectedUsers): void + { + $subject = GeneralUtility::makeInstance(GroupResolver::class); + $users = $subject->findAllUsersInGroups($groupIds, 'be_groups', 'be_users'); + self::assertEquals($expectedUsers, array_map('intval', array_column($users, 'uid'))); + } +} diff --git a/typo3/sysext/core/ext_tables.sql b/typo3/sysext/core/ext_tables.sql index 0061c5b7661a94fc5ec9c277cac839711d5c4673..c5cb7ec53667a6704e4677081caca8a6b9e65994 100644 --- a/typo3/sysext/core/ext_tables.sql +++ b/typo3/sysext/core/ext_tables.sql @@ -56,7 +56,6 @@ CREATE TABLE be_users ( workspace_perms tinyint(3) DEFAULT '1' NOT NULL, TSconfig text, lastlogin int(10) unsigned DEFAULT '0' NOT NULL, - usergroup_cached_list text, workspace_id int(11) DEFAULT '0' NOT NULL, category_perms text, KEY username (username) diff --git a/typo3/sysext/workspaces/Classes/Service/StagesService.php b/typo3/sysext/workspaces/Classes/Service/StagesService.php index 3ddfd8562a13c3c3bd9b8d776dab57e7f820cdc4..95362515d4e0fa325feb74146a87b1ce63590471 100644 --- a/typo3/sysext/workspaces/Classes/Service/StagesService.php +++ b/typo3/sysext/workspaces/Classes/Service/StagesService.php @@ -17,8 +17,7 @@ namespace TYPO3\CMS\Workspaces\Service; use TYPO3\CMS\Backend\Utility\BackendUtility; use TYPO3\CMS\Core\Authentication\BackendUserAuthentication; -use TYPO3\CMS\Core\Database\Connection; -use TYPO3\CMS\Core\Database\ConnectionPool; +use TYPO3\CMS\Core\Authentication\GroupResolver; use TYPO3\CMS\Core\Localization\LanguageService; use TYPO3\CMS\Core\SingletonInterface; use TYPO3\CMS\Core\Utility\GeneralUtility; @@ -62,16 +61,6 @@ class StagesService implements SingletonInterface */ protected $workspaceStageAllowedCache = []; - /** - * @var array - */ - protected $fetchGroupsCache = []; - - /** - * @var array - */ - protected $userGroups = []; - /** * Getter for current workspace id * @@ -452,19 +441,6 @@ class StagesService implements SingletonInterface return $recipientArray; } - /** - * Gets backend user ids from a mixed list of backend users - * and backend users groups. This is used for notifying persons - * responsible for a particular stage or workspace. - * - * @param string $stageRespValue Responsible_person value from stage record - * @return string List of backend user ids - */ - public function getResponsibleUser($stageRespValue) - { - return implode(',', $this->resolveBackendUserIds($stageRespValue)); - } - /** * Resolves backend user ids from a mixed list of backend users * and backend user groups (e.g. "be_users_1,be_groups_3,be_users_4,...") @@ -483,23 +459,17 @@ class StagesService implements SingletonInterface // Current value is a uid of a be_user record $backendUserIds[] = str_replace('be_users_', '', $element); } elseif (strpos($element, 'be_groups_') === 0) { - $backendGroupIds[] = str_replace('be_groups_', '', $element); + $backendGroupIds[] = (int)str_replace('be_groups_', '', $element); } elseif ((int)$element) { $backendUserIds[] = (int)$element; } } if (!empty($backendGroupIds)) { - $allBeUserArray = BackendUtility::getUserNames(); - $backendGroupList = implode(',', $backendGroupIds); - $this->userGroups = []; - $backendGroups = $this->fetchGroups($backendGroupList); - foreach ($backendGroups as $backendGroup) { - foreach ($allBeUserArray as $backendUserId => $backendUser) { - if (GeneralUtility::inList($backendUser['usergroup_cached_list'], $backendGroup['uid'])) { - $backendUserIds[] = $backendUserId; - } - } + $groupResolver = GeneralUtility::makeInstance(GroupResolver::class); + $backendUsersInGroups = $groupResolver->findAllUsersInGroups($backendGroupIds, 'be_groups', 'be_users'); + foreach ($backendUsersInGroups as $backendUsers) { + $backendUserIds[] = (int)$backendUsers['uid']; } } @@ -553,90 +523,6 @@ class StagesService implements SingletonInterface return WorkspaceRecord::get($this->getWorkspaceId()); } - /** - * @param string $grList - * @param string $idList - * @return array - */ - private function fetchGroups($grList, $idList = '') - { - $cacheKey = md5($grList . $idList); - if (isset($this->fetchGroupsCache[$cacheKey])) { - return $this->fetchGroupsCache[$cacheKey]; - } - if ($idList === '') { - // we're at the beginning of the recursion and therefore we need to reset the userGroups member - $this->userGroups = []; - } - $groupList = $this->fetchGroupsRecursive($grList); - $this->fetchGroupsCache[$cacheKey] = $groupList; - return $groupList; - } - - /** - * @param array $groups - */ - private function fetchGroupsFromDB(array $groups) - { - // The userGroups array is filled - $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable('be_groups'); - - $result = $queryBuilder - ->select('*') - ->from('be_groups') - ->where( - $queryBuilder->expr()->in( - 'uid', - $queryBuilder->createNamedParameter($groups, Connection::PARAM_INT_ARRAY) - ) - ) - ->execute(); - - while ($row = $result->fetch()) { - $this->userGroups[$row['uid']] = $row; - } - } - - /** - * Fetches particular groups recursively. - * - * @param string $grList - * @param string $idList - * @return array - */ - private function fetchGroupsRecursive($grList, $idList = '') - { - $requiredGroups = GeneralUtility::intExplode(',', $grList, true); - $existingGroups = array_keys($this->userGroups); - $missingGroups = array_diff($requiredGroups, $existingGroups); - if (!empty($missingGroups)) { - $this->fetchGroupsFromDB($missingGroups); - } - // Traversing records in the correct order - foreach ($requiredGroups as $uid) { - // traversing list - // Get row: - $row = $this->userGroups[$uid]; - if (is_array($row) && !GeneralUtility::inList($idList, (string)$uid)) { - // Must be an array and $uid should not be in the idList, because then it is somewhere previously in the grouplist - // If the localconf.php option isset the user of the sub- sub- groups will also be used - if ($GLOBALS['TYPO3_CONF_VARS']['BE']['customStageShowRecipientRecursive'] == 1) { - // Include sub groups - if (trim($row['subgroup'])) { - // Make integer list - $theList = implode(',', GeneralUtility::intExplode(',', $row['subgroup'])); - // Get the subgroups - $subGroups = $this->fetchGroups($theList, $idList . ',' . $uid); - // Merge the subgroups to the already existing userGroups array - $subUid = key($subGroups); - $this->userGroups[$subUid] = $subGroups[$subUid]; - } - } - } - } - return $this->userGroups; - } - /** * Gets a property of a workspaces stage. *