From d4ad0dd8cc94571a67233f3c11c0fcc7e50ca486 Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Fri, 11 Dec 2020 20:23:14 +0100
Subject: [PATCH] [!!!][TASK] Remove group-related properties in BE_USER
The "$BE_USER->groupList" property is a comma-separated list
of userGroupUID, whereas "$BE_USER->includeGroupArray"
is a list of groups added to the includeGroupArray
which contains groups possibly added multiple times due
to recursion.
This is not needed, and therefore cleaned up,
the properties are removed.
Resolves: #93062
Releases: master
Change-Id: Ic358c1be56253acdbe3be4222196a074aab1e98c
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/67099
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
---
.../ConditionMatcherTest.php | 2 +-
.../Service/UserInformationService.php | 2 +-
.../BackendUserAuthentication.php | 36 +++-------
.../core/Classes/Context/UserAspect.php | 3 +-
...elatedPublicPropertiesInBE_USERRemoved.rst | 47 ++++++++++++
.../BackendUserAuthenticationTest.php | 14 +++-
.../Authentication/Fixtures/be_groups.xml | 72 +++++++++++++++++++
.../Authentication/Fixtures/be_users.xml | 20 ++++++
.../BackendUserAuthenticationTest.php | 14 ++--
.../Php/PropertyPublicMatcher.php | 10 +++
10 files changed, 183 insertions(+), 37 deletions(-)
create mode 100644 typo3/sysext/core/Documentation/Changelog/master/Breaking-93062-VariousGroup-relatedPublicPropertiesInBE_USERRemoved.rst
diff --git a/typo3/sysext/backend/Tests/Functional/Configuration/TypoScript/ConditionMatching/ConditionMatcherTest.php b/typo3/sysext/backend/Tests/Functional/Configuration/TypoScript/ConditionMatching/ConditionMatcherTest.php
index 450d94795b70..50b886dd1608 100644
--- a/typo3/sysext/backend/Tests/Functional/Configuration/TypoScript/ConditionMatching/ConditionMatcherTest.php
+++ b/typo3/sysext/backend/Tests/Functional/Configuration/TypoScript/ConditionMatching/ConditionMatcherTest.php
@@ -44,7 +44,7 @@ class ConditionMatcherTest extends FunctionalTestCase
$backendUser = new BackendUserAuthentication();
$backendUser->user['uid'] = 13;
$backendUser->user['admin'] = true;
- $backendUser->groupList = '13,14,15';
+ $backendUser->userGroupsUID = [13, 14, 15];
GeneralUtility::makeInstance(Context::class)->setAspect('backend.user', new UserAspect($backendUser));
}
diff --git a/typo3/sysext/beuser/Classes/Service/UserInformationService.php b/typo3/sysext/beuser/Classes/Service/UserInformationService.php
index 93c9dce96279..604e213c5ec2 100644
--- a/typo3/sysext/beuser/Classes/Service/UserInformationService.php
+++ b/typo3/sysext/beuser/Classes/Service/UserInformationService.php
@@ -105,7 +105,7 @@ class UserInformationService
$data = [
'user' => $user->user ?? [],
'groups' => [
- 'inherit' => GeneralUtility::trimExplode(',', $user->groupList, true),
+ 'inherit' => $user->userGroupsUID,
'direct' => GeneralUtility::trimExplode(',', $user->user['usergroup'], true),
],
];
diff --git a/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php b/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php
index 3bde2d76580f..e8b3dbd4b14b 100644
--- a/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php
+++ b/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php
@@ -94,12 +94,6 @@ class BackendUserAuthentication extends AbstractUserAuthentication
*/
public $userGroupsUID = [];
- /**
- * This is $this->userGroupsUID imploded to a comma list... Will correspond to the 'usergroup_cached_list'
- * @var string
- */
- public $groupList = '';
-
/**
* User workspace.
* -99 is ERROR (none available)
@@ -115,13 +109,6 @@ class BackendUserAuthentication extends AbstractUserAuthentication
*/
public $workspaceRec = [];
- /**
- * List of group_id's in the order they are processed.
- * @var array
- * @internal should only be used from within TYPO3 Core
- */
- public $includeGroupArray = [];
-
/**
* @var array Parsed user TSconfig
*/
@@ -287,18 +274,18 @@ class BackendUserAuthentication extends AbstractUserAuthentication
/**
* Returns TRUE if the current user is a member of group $groupId
- * $groupId must be set. $this->groupList must contain groups
+ * $groupId must be set. $this->userGroupsUID must contain groups
* Will return TRUE also if the user is a member of a group through subgroups.
*
- * @param int $groupId Group ID to look for in $this->groupList
+ * @param int $groupId Group ID to look for in $this->userGroupsUID
* @return bool
* @internal should only be used from within TYPO3 Core, use Context API for quicker access
*/
public function isMemberOfGroup($groupId)
{
$groupId = (int)$groupId;
- if ($this->groupList && $groupId) {
- return GeneralUtility::inList($this->groupList, (string)$groupId);
+ if (!empty($this->userGroupsUID) && $groupId) {
+ return in_array($groupId, $this->userGroupsUID, true);
}
return false;
}
@@ -515,12 +502,12 @@ class BackendUserAuthentication extends AbstractUserAuthentication
);
// Group (if any is set)
- if ($this->groupList) {
+ if (!empty($this->userGroupsUID)) {
$constraint->add(
$expressionBuilder->andX(
$expressionBuilder->in(
'pages.perms_groupid',
- GeneralUtility::intExplode(',', $this->groupList)
+ $this->userGroupsUID
),
$expressionBuilder->comparison(
$expressionBuilder->bitAnd('pages.perms_group', $perms),
@@ -567,7 +554,7 @@ class BackendUserAuthentication extends AbstractUserAuthentication
$out = Permission::NOTHING;
if (
isset($row['perms_userid']) && isset($row['perms_user']) && isset($row['perms_groupid'])
- && isset($row['perms_group']) && isset($row['perms_everybody']) && isset($this->groupList)
+ && isset($row['perms_group']) && isset($row['perms_everybody']) && !empty($this->userGroupsUID)
) {
if ($this->user['uid'] == $row['perms_userid']) {
$out |= $row['perms_user'];
@@ -1260,12 +1247,11 @@ class BackendUserAuthentication extends AbstractUserAuthentication
$this->fetchGroups($this->user[$this->usergroup_column]);
}
// 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->includeGroupArray)));
+ $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->groupList = implode(',', $this->userGroupsUID);
- $this->setCachedList($this->groupList);
+ $this->setCachedList(implode(',', $this->userGroupsUID));
$this->prepareUserTsConfig();
@@ -1359,7 +1345,7 @@ TCAdefaults.sys_note.author = ' . $this->user['realName'] . '
TCAdefaults.sys_note.email = ' . $this->user['email'];
// Loop through all groups and add their 'TSconfig' fields
- foreach ($this->includeGroupArray as $groupId) {
+ foreach ($this->userGroupsUID as $groupId) {
$collectedUserTSconfig['group_' . $groupId] = $this->userGroups[$groupId]['TSconfig'] ?? '';
}
@@ -1439,7 +1425,7 @@ TCAdefaults.sys_note.email = ' . $this->user['email'];
$this->fetchGroups($theList, $idList . ',' . $uid);
}
// Add the group uid, current list to the internal arrays.
- $this->includeGroupArray[] = $uid;
+ $this->userGroupsUID[] = (int)$uid;
// Mount group database-mounts
if ($mountOptions->shouldUserIncludePageMountsFromAssociatedGroups()) {
$this->groupData['webmounts'] .= ',' . $row['db_mountpoints'];
diff --git a/typo3/sysext/core/Classes/Context/UserAspect.php b/typo3/sysext/core/Classes/Context/UserAspect.php
index 7441a0efc15a..6acf27267af8 100644
--- a/typo3/sysext/core/Classes/Context/UserAspect.php
+++ b/typo3/sysext/core/Classes/Context/UserAspect.php
@@ -20,7 +20,6 @@ namespace TYPO3\CMS\Core\Context;
use TYPO3\CMS\Core\Authentication\AbstractUserAuthentication;
use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
use TYPO3\CMS\Core\Context\Exception\AspectPropertyNotFoundException;
-use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication;
/**
@@ -140,7 +139,7 @@ class UserAspect implements AspectInterface
return $this->groups;
}
if ($this->user instanceof BackendUserAuthentication) {
- return GeneralUtility::intExplode(',', $this->user->groupList, true);
+ return $this->user->userGroupsUID;
}
$groups = [];
if ($this->user instanceof FrontendUserAuthentication) {
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Breaking-93062-VariousGroup-relatedPublicPropertiesInBE_USERRemoved.rst b/typo3/sysext/core/Documentation/Changelog/master/Breaking-93062-VariousGroup-relatedPublicPropertiesInBE_USERRemoved.rst
new file mode 100644
index 000000000000..e6c878a49171
--- /dev/null
+++ b/typo3/sysext/core/Documentation/Changelog/master/Breaking-93062-VariousGroup-relatedPublicPropertiesInBE_USERRemoved.rst
@@ -0,0 +1,47 @@
+.. include:: ../../Includes.txt
+
+=============================================================================
+Breaking: #93062 - Various group-related public properties in BE_USER removed
+=============================================================================
+
+See :issue:`93062`
+
+Description
+===========
+
+The PHP API class `BackendUserAuthentication` was built back in
+PHP4 days and had a few public properties which have been removed.
+
+Their purpose was to store data between methods while resolving
+groups, where there are other methods containing all group-related
+information already anyways.
+
+- :php:`TYPO3\CMS\Core\Authentication\BackendUserAuthentication->groupList`
+- :php:`TYPO3\CMS\Core\Authentication\BackendUserAuthentication->includeGroupArray`
+
+
+Impact
+======
+
+Accessing or setting these properties have no effect anymore.
+
+
+Affected Installations
+======================
+
+TYPO3 installations with third-party extensions accessing these
+`BackendUserAuthentication` properties, which is highly unlikely,
+or because they were built 10 years ago, still accessing these properties.
+
+
+Migration
+=========
+
+Use `BackendUserAuthentication->userGroupsUID` (array of group UIDs) instead,
+which contains the groups in the proper order on how they were resolved.
+
+If this is not needed directly, it is usually highly recommended to use the
+Context API's "backend.user" aspect to retrieve a user's groups of a
+backend user.
+
+.. index:: PHP-API, FullyScanned, ext:core
diff --git a/typo3/sysext/core/Tests/Functional/Authentication/BackendUserAuthenticationTest.php b/typo3/sysext/core/Tests/Functional/Authentication/BackendUserAuthenticationTest.php
index d0e4e24abdbe..5ebef3f6f8d5 100644
--- a/typo3/sysext/core/Tests/Functional/Authentication/BackendUserAuthenticationTest.php
+++ b/typo3/sysext/core/Tests/Functional/Authentication/BackendUserAuthenticationTest.php
@@ -83,7 +83,7 @@ class BackendUserAuthenticationTest extends FunctionalTestCase
$GLOBALS['TYPO3_CONF_VARS']['BE']['defaultUserTSconfig'] = "custom.generic = installation-wide-configuration\ncustom.property = from configuration";
$this->subject->user['realName'] = 'Test user';
$this->subject->user['TSconfig'] = 'custom.property = from user';
- $this->subject->includeGroupArray[] = 13;
+ $this->subject->userGroupsUID[] = 13;
$this->subject->userGroups[13]['TSconfig'] = "custom.property = from group\ncustom.groupProperty = 13";
$this->subject->fetchGroupData();
$result = $this->subject->getTSConfig();
@@ -127,4 +127,16 @@ class BackendUserAuthenticationTest extends FunctionalTestCase
$folder = $this->subject->getDefaultUploadFolder();
self::assertEquals('/' . $path . '/', $folder->getIdentifier());
}
+
+ /**
+ * @test
+ */
+ public function loadGroupsWithProperSettingsAndOrder(): void
+ {
+ $subject = $this->setUpBackendUser(3);
+ $subject->fetchGroupData();
+ self::assertEquals('web_info,web_layout,web_list,file_filelist', $subject->groupData['modules']);
+ self::assertEquals(['1', '4', '5', '3', '2', '6'], $subject->userGroupsUID);
+ self::assertEquals(['groupValue' => 'from_group_6', 'userValue' => 'from_user_3'], $subject->getTSConfig()['test.']['default.']);
+ }
}
diff --git a/typo3/sysext/core/Tests/Functional/Authentication/Fixtures/be_groups.xml b/typo3/sysext/core/Tests/Functional/Authentication/Fixtures/be_groups.xml
index 4fe85136250f..047f0608e717 100644
--- a/typo3/sysext/core/Tests/Functional/Authentication/Fixtures/be_groups.xml
+++ b/typo3/sysext/core/Tests/Functional/Authentication/Fixtures/be_groups.xml
@@ -12,4 +12,76 @@
<hidden>0</hidden>
<cruser_id>1</cruser_id>
</be_groups>
+ <be_groups>
+ <uid>2</uid>
+ <pid>0</pid>
+ <title>first level group</title>
+ <workspace_perms>0</workspace_perms>
+ <db_mountpoints>40</db_mountpoints>
+ <tstamp>1544454571</tstamp>
+ <crdate>1542360853</crdate>
+ <deleted>0</deleted>
+ <hidden>0</hidden>
+ <subgroup>3</subgroup>
+ <cruser_id>1</cruser_id>
+ </be_groups>
+ <be_groups>
+ <uid>3</uid>
+ <pid>0</pid>
+ <title>first level subgroup</title>
+ <workspace_perms>0</workspace_perms>
+ <db_mountpoints>40</db_mountpoints>
+ <tstamp>1544454571</tstamp>
+ <TSconfig>test.default.groupValue = from_group_3
+test.default.userValue = never_seen</TSconfig>
+ <groupMods>web_layout,web_list</groupMods>
+ <subgroup>2,5</subgroup>
+ <crdate>1542360853</crdate>
+ <deleted>0</deleted>
+ <hidden>0</hidden>
+ <cruser_id>1</cruser_id>
+ </be_groups>
+ <be_groups>
+ <uid>4</uid>
+ <pid>0</pid>
+ <title>random group for filelist</title>
+ <workspace_perms>0</workspace_perms>
+ <db_mountpoints></db_mountpoints>
+ <TSconfig>test.default.groupValue = from_group_4</TSconfig>
+ <tstamp>1544454571</tstamp>
+ <groupMods>file_filelist</groupMods>
+ <crdate>1542360853</crdate>
+ <deleted>0</deleted>
+ <hidden>0</hidden>
+ <cruser_id>1</cruser_id>
+ </be_groups>
+ <be_groups>
+ <uid>5</uid>
+ <pid>0</pid>
+ <title>subgroup level 2</title>
+ <workspace_perms>0</workspace_perms>
+ <db_mountpoints></db_mountpoints>
+ <TSconfig>test.default.groupValue = from_group_5</TSconfig>
+ <tstamp>1544454571</tstamp>
+ <groupMods>web_info</groupMods>
+ <crdate>1542360853</crdate>
+ <deleted>0</deleted>
+ <hidden>0</hidden>
+ <cruser_id>1</cruser_id>
+ </be_groups>
+ <be_groups>
+ <uid>6</uid>
+ <pid>0</pid>
+ <title>random group for filelist</title>
+ <workspace_perms>0</workspace_perms>
+ <db_mountpoints></db_mountpoints>
+ <tstamp>1544454571</tstamp>
+ <TSconfig>test.default.groupValue = from_group_6</TSconfig>
+ <groupMods>file_filelist</groupMods>
+ <crdate>1542360853</crdate>
+ <deleted>0</deleted>
+ <hidden>0</hidden>
+ <subgroup>2</subgroup>
+ <cruser_id>1</cruser_id>
+ </be_groups>
</dataset>
diff --git a/typo3/sysext/core/Tests/Functional/Authentication/Fixtures/be_users.xml b/typo3/sysext/core/Tests/Functional/Authentication/Fixtures/be_users.xml
index 706fefad5e04..c2854189d1b5 100644
--- a/typo3/sysext/core/Tests/Functional/Authentication/Fixtures/be_users.xml
+++ b/typo3/sysext/core/Tests/Functional/Authentication/Fixtures/be_users.xml
@@ -39,4 +39,24 @@
<workspace_id>0</workspace_id>
<usergroup>1</usergroup>
</be_users>
+ <be_users>
+ <uid>3</uid>
+ <pid>0</pid>
+ <tstamp>1366642540</tstamp>
+ <username>editor_with_groups</username>
+ <password>$1$tCrlLajZ$C0sikFQQ3SWaFAZ1Me0Z/1</password> <!-- password -->
+ <admin>0</admin>
+ <disable>0</disable>
+ <starttime>0</starttime>
+ <endtime>0</endtime>
+ <options>3</options>
+ <crdate>1366642540</crdate>
+ <cruser_id>0</cruser_id>
+ <workspace_perms>1</workspace_perms>
+ <deleted>0</deleted>
+ <TSconfig>test.default.userValue = from_user_3</TSconfig>
+ <lastlogin>1371033743</lastlogin>
+ <workspace_id>0</workspace_id>
+ <usergroup>1,2,4,6</usergroup>
+ </be_users>
</dataset>
diff --git a/typo3/sysext/core/Tests/Unit/Authentication/BackendUserAuthenticationTest.php b/typo3/sysext/core/Tests/Unit/Authentication/BackendUserAuthenticationTest.php
index e6d1731ec95c..fa8cc7c76d9b 100644
--- a/typo3/sysext/core/Tests/Unit/Authentication/BackendUserAuthenticationTest.php
+++ b/typo3/sysext/core/Tests/Unit/Authentication/BackendUserAuthenticationTest.php
@@ -712,26 +712,26 @@ class BackendUserAuthenticationTest extends UnitTestCase
'for admin' => [
1,
true,
- '',
+ [],
' 1=1'
],
'for admin with groups' => [
11,
true,
- '1,2',
+ [1, 2],
' 1=1'
],
'for user' => [
2,
false,
- '',
+ [],
' ((`pages`.`perms_everybody` & 2 = 2) OR' .
' ((`pages`.`perms_userid` = 123) AND (`pages`.`perms_user` & 2 = 2)))'
],
'for user with groups' => [
8,
false,
- '1,2',
+ [1, 2],
' ((`pages`.`perms_everybody` & 8 = 8) OR' .
' ((`pages`.`perms_userid` = 123) AND (`pages`.`perms_user` & 8 = 8))' .
' OR ((`pages`.`perms_groupid` IN (1, 2)) AND (`pages`.`perms_group` & 8 = 8)))'
@@ -744,10 +744,10 @@ class BackendUserAuthenticationTest extends UnitTestCase
* @dataProvider getPagePermissionsClauseWithValidUserDataProvider
* @param int $perms
* @param bool $admin
- * @param string $groups
+ * @param array $groups
* @param string $expected
*/
- public function getPagePermissionsClauseWithValidUser(int $perms, bool $admin, string $groups, string $expected): void
+ public function getPagePermissionsClauseWithValidUser(int $perms, bool $admin, array $groups, string $expected): void
{
// We only need to setup the mocking for the non-admin cases
// If this setup is done for admin cases the FIFO behavior
@@ -785,7 +785,7 @@ class BackendUserAuthenticationTest extends UnitTestCase
->willReturn($admin);
$subject->user = ['uid' => 123];
- $subject->groupList = $groups;
+ $subject->userGroupsUID = $groups;
self::assertEquals($expected, $subject->getPagePermsClause($perms));
}
diff --git a/typo3/sysext/install/Configuration/ExtensionScanner/Php/PropertyPublicMatcher.php b/typo3/sysext/install/Configuration/ExtensionScanner/Php/PropertyPublicMatcher.php
index 2c291990bfa0..a216c07337be 100644
--- a/typo3/sysext/install/Configuration/ExtensionScanner/Php/PropertyPublicMatcher.php
+++ b/typo3/sysext/install/Configuration/ExtensionScanner/Php/PropertyPublicMatcher.php
@@ -785,4 +785,14 @@ return [
'Breaking-93047-RemovedPropertySendNoCacheHeadersInAbstractUserAuthentication.rst',
],
],
+ 'TYPO3\CMS\Core\Authentication\BackendUserAuthentication->groupList' => [
+ 'restFiles' => [
+ 'Breaking-93062-VariousGroup-relatedPublicPropertiesInBE_USERRemoved.rst',
+ ],
+ ],
+ 'TYPO3\CMS\Core\Authentication\BackendUserAuthentication->includeGroupArray' => [
+ 'restFiles' => [
+ 'Breaking-93062-VariousGroup-relatedPublicPropertiesInBE_USERRemoved.rst',
+ ],
+ ],
];
--
GitLab