Commit d4ad0dd8 authored by Benni Mack's avatar Benni Mack Committed by Andreas Fernandez
Browse files

[!!!][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's avatarChristian Kuhn <lolli@schwarzbu.ch>
Tested-by: default avatarTYPO3com <noreply@typo3.com>
Tested-by: Andreas Fernandez's avatarAndreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Christian Kuhn's avatarChristian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Andreas Fernandez's avatarAndreas Fernandez <a.fernandez@scripting-base.de>
parent e052f903
...@@ -44,7 +44,7 @@ class ConditionMatcherTest extends FunctionalTestCase ...@@ -44,7 +44,7 @@ class ConditionMatcherTest extends FunctionalTestCase
$backendUser = new BackendUserAuthentication(); $backendUser = new BackendUserAuthentication();
$backendUser->user['uid'] = 13; $backendUser->user['uid'] = 13;
$backendUser->user['admin'] = true; $backendUser->user['admin'] = true;
$backendUser->groupList = '13,14,15'; $backendUser->userGroupsUID = [13, 14, 15];
GeneralUtility::makeInstance(Context::class)->setAspect('backend.user', new UserAspect($backendUser)); GeneralUtility::makeInstance(Context::class)->setAspect('backend.user', new UserAspect($backendUser));
} }
......
...@@ -105,7 +105,7 @@ class UserInformationService ...@@ -105,7 +105,7 @@ class UserInformationService
$data = [ $data = [
'user' => $user->user ?? [], 'user' => $user->user ?? [],
'groups' => [ 'groups' => [
'inherit' => GeneralUtility::trimExplode(',', $user->groupList, true), 'inherit' => $user->userGroupsUID,
'direct' => GeneralUtility::trimExplode(',', $user->user['usergroup'], true), 'direct' => GeneralUtility::trimExplode(',', $user->user['usergroup'], true),
], ],
]; ];
......
...@@ -94,12 +94,6 @@ class BackendUserAuthentication extends AbstractUserAuthentication ...@@ -94,12 +94,6 @@ class BackendUserAuthentication extends AbstractUserAuthentication
*/ */
public $userGroupsUID = []; public $userGroupsUID = [];
/**
* This is $this->userGroupsUID imploded to a comma list... Will correspond to the 'usergroup_cached_list'
* @var string
*/
public $groupList = '';
/** /**
* User workspace. * User workspace.
* -99 is ERROR (none available) * -99 is ERROR (none available)
...@@ -115,13 +109,6 @@ class BackendUserAuthentication extends AbstractUserAuthentication ...@@ -115,13 +109,6 @@ class BackendUserAuthentication extends AbstractUserAuthentication
*/ */
public $workspaceRec = []; 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 * @var array Parsed user TSconfig
*/ */
...@@ -287,18 +274,18 @@ class BackendUserAuthentication extends AbstractUserAuthentication ...@@ -287,18 +274,18 @@ class BackendUserAuthentication extends AbstractUserAuthentication
/** /**
* Returns TRUE if the current user is a member of group $groupId * 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. * 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 * @return bool
* @internal should only be used from within TYPO3 Core, use Context API for quicker access * @internal should only be used from within TYPO3 Core, use Context API for quicker access
*/ */
public function isMemberOfGroup($groupId) public function isMemberOfGroup($groupId)
{ {
$groupId = (int)$groupId; $groupId = (int)$groupId;
if ($this->groupList && $groupId) { if (!empty($this->userGroupsUID) && $groupId) {
return GeneralUtility::inList($this->groupList, (string)$groupId); return in_array($groupId, $this->userGroupsUID, true);
} }
return false; return false;
} }
...@@ -515,12 +502,12 @@ class BackendUserAuthentication extends AbstractUserAuthentication ...@@ -515,12 +502,12 @@ class BackendUserAuthentication extends AbstractUserAuthentication
); );
// Group (if any is set) // Group (if any is set)
if ($this->groupList) { if (!empty($this->userGroupsUID)) {
$constraint->add( $constraint->add(
$expressionBuilder->andX( $expressionBuilder->andX(
$expressionBuilder->in( $expressionBuilder->in(
'pages.perms_groupid', 'pages.perms_groupid',
GeneralUtility::intExplode(',', $this->groupList) $this->userGroupsUID
), ),
$expressionBuilder->comparison( $expressionBuilder->comparison(
$expressionBuilder->bitAnd('pages.perms_group', $perms), $expressionBuilder->bitAnd('pages.perms_group', $perms),
...@@ -567,7 +554,7 @@ class BackendUserAuthentication extends AbstractUserAuthentication ...@@ -567,7 +554,7 @@ class BackendUserAuthentication extends AbstractUserAuthentication
$out = Permission::NOTHING; $out = Permission::NOTHING;
if ( if (
isset($row['perms_userid']) && isset($row['perms_user']) && isset($row['perms_groupid']) 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']) { if ($this->user['uid'] == $row['perms_userid']) {
$out |= $row['perms_user']; $out |= $row['perms_user'];
...@@ -1260,12 +1247,11 @@ class BackendUserAuthentication extends AbstractUserAuthentication ...@@ -1260,12 +1247,11 @@ class BackendUserAuthentication extends AbstractUserAuthentication
$this->fetchGroups($this->user[$this->usergroup_column]); $this->fetchGroups($this->user[$this->usergroup_column]);
} }
// Populating the $this->userGroupsUID -array with the groups in the order in which they were LAST included.!! // 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!) // 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, // and without duplicates (duplicates are presented with their last entrance in the list,
// which thus reflects the order of the TypoScript in TSconfig) // which thus reflects the order of the TypoScript in TSconfig)
$this->groupList = implode(',', $this->userGroupsUID); $this->setCachedList(implode(',', $this->userGroupsUID));
$this->setCachedList($this->groupList);
$this->prepareUserTsConfig(); $this->prepareUserTsConfig();
...@@ -1359,7 +1345,7 @@ TCAdefaults.sys_note.author = ' . $this->user['realName'] . ' ...@@ -1359,7 +1345,7 @@ TCAdefaults.sys_note.author = ' . $this->user['realName'] . '
TCAdefaults.sys_note.email = ' . $this->user['email']; TCAdefaults.sys_note.email = ' . $this->user['email'];
// Loop through all groups and add their 'TSconfig' fields // 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'] ?? ''; $collectedUserTSconfig['group_' . $groupId] = $this->userGroups[$groupId]['TSconfig'] ?? '';
} }
...@@ -1439,7 +1425,7 @@ TCAdefaults.sys_note.email = ' . $this->user['email']; ...@@ -1439,7 +1425,7 @@ TCAdefaults.sys_note.email = ' . $this->user['email'];
$this->fetchGroups($theList, $idList . ',' . $uid); $this->fetchGroups($theList, $idList . ',' . $uid);
} }
// Add the group uid, current list to the internal arrays. // Add the group uid, current list to the internal arrays.
$this->includeGroupArray[] = $uid; $this->userGroupsUID[] = (int)$uid;
// Mount group database-mounts // Mount group database-mounts
if ($mountOptions->shouldUserIncludePageMountsFromAssociatedGroups()) { if ($mountOptions->shouldUserIncludePageMountsFromAssociatedGroups()) {
$this->groupData['webmounts'] .= ',' . $row['db_mountpoints']; $this->groupData['webmounts'] .= ',' . $row['db_mountpoints'];
......
...@@ -20,7 +20,6 @@ namespace TYPO3\CMS\Core\Context; ...@@ -20,7 +20,6 @@ namespace TYPO3\CMS\Core\Context;
use TYPO3\CMS\Core\Authentication\AbstractUserAuthentication; use TYPO3\CMS\Core\Authentication\AbstractUserAuthentication;
use TYPO3\CMS\Core\Authentication\BackendUserAuthentication; use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
use TYPO3\CMS\Core\Context\Exception\AspectPropertyNotFoundException; use TYPO3\CMS\Core\Context\Exception\AspectPropertyNotFoundException;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication; use TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication;
/** /**
...@@ -140,7 +139,7 @@ class UserAspect implements AspectInterface ...@@ -140,7 +139,7 @@ class UserAspect implements AspectInterface
return $this->groups; return $this->groups;
} }
if ($this->user instanceof BackendUserAuthentication) { if ($this->user instanceof BackendUserAuthentication) {
return GeneralUtility::intExplode(',', $this->user->groupList, true); return $this->user->userGroupsUID;
} }
$groups = []; $groups = [];
if ($this->user instanceof FrontendUserAuthentication) { if ($this->user instanceof FrontendUserAuthentication) {
......
.. 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
...@@ -83,7 +83,7 @@ class BackendUserAuthenticationTest extends FunctionalTestCase ...@@ -83,7 +83,7 @@ class BackendUserAuthenticationTest extends FunctionalTestCase
$GLOBALS['TYPO3_CONF_VARS']['BE']['defaultUserTSconfig'] = "custom.generic = installation-wide-configuration\ncustom.property = from configuration"; $GLOBALS['TYPO3_CONF_VARS']['BE']['defaultUserTSconfig'] = "custom.generic = installation-wide-configuration\ncustom.property = from configuration";
$this->subject->user['realName'] = 'Test user'; $this->subject->user['realName'] = 'Test user';
$this->subject->user['TSconfig'] = 'custom.property = from 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->userGroups[13]['TSconfig'] = "custom.property = from group\ncustom.groupProperty = 13";
$this->subject->fetchGroupData(); $this->subject->fetchGroupData();
$result = $this->subject->getTSConfig(); $result = $this->subject->getTSConfig();
...@@ -127,4 +127,16 @@ class BackendUserAuthenticationTest extends FunctionalTestCase ...@@ -127,4 +127,16 @@ class BackendUserAuthenticationTest extends FunctionalTestCase
$folder = $this->subject->getDefaultUploadFolder(); $folder = $this->subject->getDefaultUploadFolder();
self::assertEquals('/' . $path . '/', $folder->getIdentifier()); 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.']);
}
} }
...@@ -12,4 +12,76 @@ ...@@ -12,4 +12,76 @@
<hidden>0</hidden> <hidden>0</hidden>
<cruser_id>1</cruser_id> <cruser_id>1</cruser_id>
</be_groups> </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> </dataset>
...@@ -39,4 +39,24 @@ ...@@ -39,4 +39,24 @@
<workspace_id>0</workspace_id> <workspace_id>0</workspace_id>
<usergroup>1</usergroup> <usergroup>1</usergroup>
</be_users> </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> </dataset>
...@@ -712,26 +712,26 @@ class BackendUserAuthenticationTest extends UnitTestCase ...@@ -712,26 +712,26 @@ class BackendUserAuthenticationTest extends UnitTestCase
'for admin' => [ 'for admin' => [
1, 1,
true, true,
'', [],
' 1=1' ' 1=1'
], ],
'for admin with groups' => [ 'for admin with groups' => [
11, 11,
true, true,
'1,2', [1, 2],
' 1=1' ' 1=1'
], ],
'for user' => [ 'for user' => [
2, 2,
false, false,
'', [],
' ((`pages`.`perms_everybody` & 2 = 2) OR' . ' ((`pages`.`perms_everybody` & 2 = 2) OR' .
' ((`pages`.`perms_userid` = 123) AND (`pages`.`perms_user` & 2 = 2)))' ' ((`pages`.`perms_userid` = 123) AND (`pages`.`perms_user` & 2 = 2)))'
], ],
'for user with groups' => [ 'for user with groups' => [
8, 8,
false, false,
'1,2', [1, 2],
' ((`pages`.`perms_everybody` & 8 = 8) OR' . ' ((`pages`.`perms_everybody` & 8 = 8) OR' .
' ((`pages`.`perms_userid` = 123) AND (`pages`.`perms_user` & 8 = 8))' . ' ((`pages`.`perms_userid` = 123) AND (`pages`.`perms_user` & 8 = 8))' .
' OR ((`pages`.`perms_groupid` IN (1, 2)) AND (`pages`.`perms_group` & 8 = 8)))' ' OR ((`pages`.`perms_groupid` IN (1, 2)) AND (`pages`.`perms_group` & 8 = 8)))'
...@@ -744,10 +744,10 @@ class BackendUserAuthenticationTest extends UnitTestCase ...@@ -744,10 +744,10 @@ class BackendUserAuthenticationTest extends UnitTestCase
* @dataProvider getPagePermissionsClauseWithValidUserDataProvider * @dataProvider getPagePermissionsClauseWithValidUserDataProvider
* @param int $perms * @param int $perms
* @param bool $admin * @param bool $admin
* @param string $groups * @param array $groups
* @param string $expected * @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 // We only need to setup the mocking for the non-admin cases
// If this setup is done for admin cases the FIFO behavior // If this setup is done for admin cases the FIFO behavior
...@@ -785,7 +785,7 @@ class BackendUserAuthenticationTest extends UnitTestCase ...@@ -785,7 +785,7 @@ class BackendUserAuthenticationTest extends UnitTestCase
->willReturn($admin); ->willReturn($admin);
$subject->user = ['uid' => 123]; $subject->user = ['uid' => 123];
$subject->groupList = $groups; $subject->userGroupsUID = $groups;
self::assertEquals($expected, $subject->getPagePermsClause($perms)); self::assertEquals($expected, $subject->getPagePermsClause($perms));
} }
......
...@@ -785,4 +785,14 @@ return [ ...@@ -785,4 +785,14 @@ return [
'Breaking-93047-RemovedPropertySendNoCacheHeadersInAbstractUserAuthentication.rst', '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',
],
],
]; ];
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment