From 72bfc307ca3b0cda7ddc9dc5be518dad65ec9e90 Mon Sep 17 00:00:00 2001 From: haraldwitt <64258274+haraldwitt@users.noreply.github.com> Date: Thu, 19 Sep 2024 18:03:10 +0200 Subject: [PATCH 1/3] Update CheckPermissions.php The break statement in the foreach loop should only be executed if $feGroups comtains at least one element. Otherwise the loop should continue until a group is found or the rootline ends. --- Classes/Security/CheckPermissions.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Classes/Security/CheckPermissions.php b/Classes/Security/CheckPermissions.php index e173f32..8f8d22c 100644 --- a/Classes/Security/CheckPermissions.php +++ b/Classes/Security/CheckPermissions.php @@ -230,7 +230,9 @@ public function getPermissions(ResourceInterface $resource): string if ($folderRecord['fe_groups']) { $feGroups = ArrayUtility::keepItemsInArray($feGroups, $folderRecord['fe_groups']); } - break; + if (count($feGroups)) { + break; + } } } } catch (FolderDoesNotExistException $e) { From 183a20550680739f940e122d50f8c85249487da1 Mon Sep 17 00:00:00 2001 From: haraldwitt <64258274+haraldwitt@users.noreply.github.com> Date: Thu, 19 Sep 2024 18:06:43 +0200 Subject: [PATCH 2/3] Update CheckPermissions.php->getPermissions() If a file resource has a fe_group assigned but not any folder in the rootline, $feGroups will obviousely be empty before and after ArrayUtility::keepItemsInArray(). In such a case the fe_groups of the file resource have to be overtaken directly. --- Classes/Security/CheckPermissions.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Classes/Security/CheckPermissions.php b/Classes/Security/CheckPermissions.php index 8f8d22c..7c5c0dc 100644 --- a/Classes/Security/CheckPermissions.php +++ b/Classes/Security/CheckPermissions.php @@ -238,7 +238,11 @@ public function getPermissions(ResourceInterface $resource): string } catch (FolderDoesNotExistException $e) { } if ($resource instanceof FileInterface && $resource->getProperty('fe_groups')) { - $feGroups = ArrayUtility::keepItemsInArray($feGroups, $resource->getProperty('fe_groups')); + if (count($feGroups)) { + $feGroups = ArrayUtility::keepItemsInArray($feGroups, $resource->getProperty('fe_groups')); + } else { + $feGroups = GeneralUtility::trimExplode(',', $resource->getProperty('fe_groups')); + } } $resource->getStorage()->setEvaluatePermissions($currentPermissionsCheck); return implode(',', $feGroups); From 6e2006b3fc7dcf79a73fb73dd2d6e1d52c2bfb3b Mon Sep 17 00:00:00 2001 From: haraldwitt <64258274+haraldwitt@users.noreply.github.com> Date: Thu, 19 Sep 2024 18:15:56 +0200 Subject: [PATCH 3/3] Update CheckPermissions.php->getPermissions() Take care of group inheritance? Imagine groups inside $feGroups are subgroups of other groups. So those other groups should also be granted access. So $feGroups should be enriched by these other groups. And this should be done for both - the $feGroups of the folder and for the file resource - before they are merged together with ArrayUtility::keepItemsInArray(). --- Classes/Security/CheckPermissions.php | 74 ++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 2 deletions(-) diff --git a/Classes/Security/CheckPermissions.php b/Classes/Security/CheckPermissions.php index 7c5c0dc..89a06ef 100644 --- a/Classes/Security/CheckPermissions.php +++ b/Classes/Security/CheckPermissions.php @@ -231,6 +231,9 @@ public function getPermissions(ResourceInterface $resource): string $feGroups = ArrayUtility::keepItemsInArray($feGroups, $folderRecord['fe_groups']); } if (count($feGroups)) { + // only stop searching if something was found + $parentGroups = $this->fetchParentGroupsRecursive($feGroups); + $feGroups = array_merge($feGroups, $parentGroups); break; } } @@ -238,10 +241,17 @@ public function getPermissions(ResourceInterface $resource): string } catch (FolderDoesNotExistException $e) { } if ($resource instanceof FileInterface && $resource->getProperty('fe_groups')) { + $resourceGroups = GeneralUtility::trimExplode(',', $resource->getProperty('fe_groups')); + $resourceParentGroups = $this->fetchParentGroupsRecursive($resourceGroups); + $resourceGroups = array_merge($resourceGroups, $resourceParentGroups); + if (count($feGroups)) { - $feGroups = ArrayUtility::keepItemsInArray($feGroups, $resource->getProperty('fe_groups')); + $feGroups = ArrayUtility::keepItemsInArray($feGroups, $resourceGroups); + // @todo: if $feGroups is empty now and was not before, ensure nobody has access + // But solrfal will kick out any nonexisting group. So we leave as it is for now } else { - $feGroups = GeneralUtility::trimExplode(',', $resource->getProperty('fe_groups')); + // if no frontend groups were found in rootline overtake file properties + $feGroups = $resourceGroups; } } $resource->getStorage()->setEvaluatePermissions($currentPermissionsCheck); @@ -319,4 +329,64 @@ public function matchFeGroupsWithFeUser(string $groups, $userFeGroups): bool return false; } + + /** + * Load a list of group uids, and take into account if groups have been loaded before as part of recursive detection. + * This method very is similar to that in \TYPO3\CMS\Core\Authentication\GroupResolver, but that one is protected + * + * @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. + * This method very is similar to that in \TYPO3\CMS\Core\Authentication\GroupResolver, but that one is protected + */ + protected function fetchParentGroupsFromDatabase(array $subgroupIds): array + { + $queryBuilder = GeneralUtility::makeInstance(\TYPO3\CMS\Core\Database\ConnectionPool::class)->getQueryBuilderForTable('fe_groups'); + $queryBuilder + ->select('*') + ->from('fe_groups'); + + $constraints = []; + foreach ($subgroupIds as $subgroupId) { + $constraints[] = $queryBuilder->expr()->inSet('subgroup', (string)$subgroupId); + } + + $result = $queryBuilder + ->where( + $queryBuilder->expr()->or(...$constraints) + ) + ->executeQuery(); + + $groups = []; + while ($row = $result->fetchAssociative()) { + $groups[(int)$row['uid']] = $row; + } + return $groups; + } + }