Commit 22b32dfa authored by Benni Mack's avatar Benni Mack Committed by Oliver Hader
Browse files

[!!!][TASK] Rework user group fetching for frontend users

This change adapts the frontend user resolving to use
the same functionality as in backend users, which wasn't
the case in the past 15 years.

The newly introduced GroupResolver now also resolves
groups for frontend users recursively, making the "getGroupsFE"
and "authGroupsFE" functionality obsolete.

The AuthenticationService->getGroups methods are removed,
also the properties userTS, userData_change and TSdataArray have
been made protected, as it is only used for internal purposes.

The "userTSUpdated" flag has been removed and the property $TSdataArray
has been marked as internal.

Resolves: #93108
Releases: master
Change-Id: I630be39e855d45802c0236a0a38acbc07e0dd812
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/67177


Tested-by: Andreas Fernandez's avatarAndreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: default avatarTYPO3com <noreply@typo3.com>
Tested-by: Christian Kuhn's avatarChristian Kuhn <lolli@schwarzbu.ch>
Tested-by: Oliver Bartsch's avatarOliver Bartsch <bo@cedev.de>
Tested-by: Oliver Hader's avatarOliver Hader <oliver.hader@typo3.org>
Reviewed-by: Andreas Fernandez's avatarAndreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Christian Kuhn's avatarChristian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Oliver Bartsch's avatarOliver Bartsch <bo@cedev.de>
Reviewed-by: Oliver Hader's avatarOliver Hader <oliver.hader@typo3.org>
parent 794f3e61
......@@ -64,13 +64,6 @@ class AbstractAuthenticationService implements LoggerAwareInterface
*/
public $db_user = [];
/**
* Usergroups db table definition
*
* @var array
*/
public $db_groups = [];
/**
* If the writelog() functions is called if a login-attempt has be tried without success
*
......@@ -99,7 +92,6 @@ class AbstractAuthenticationService implements LoggerAwareInterface
$this->login = $loginData;
$this->authInfo = $authInfo;
$this->db_user = $this->getServiceOption('db_user', $authInfo['db_user'] ?? [], false);
$this->db_groups = $this->getServiceOption('db_groups', $authInfo['db_groups'] ?? [], false);
$this->writeAttemptLog = $this->pObj->writeAttemptLog ?? true;
}
......
......@@ -180,6 +180,11 @@ abstract class AbstractUserAuthentication implements LoggerAwareInterface
*/
public $user;
/**
* This array will hold the groups that the user is a member of
*/
public array $userGroups = [];
/**
* Will prevent the setting of the session cookie
* @var bool
......@@ -1115,7 +1120,6 @@ abstract class AbstractUserAuthentication implements LoggerAwareInterface
$authInfo['db_user']['userid_column'] = $this->userid_column;
$authInfo['db_user']['username_column'] = $this->username_column;
$authInfo['db_user']['userident_column'] = $this->userident_column;
$authInfo['db_user']['usergroup_column'] = $this->usergroup_column;
$authInfo['db_user']['enable_clause'] = $this->userConstraints()->buildExpression(
[$this->user_table => $this->user_table],
$expressionBuilder
......@@ -1130,7 +1134,6 @@ abstract class AbstractUserAuthentication implements LoggerAwareInterface
$authInfo['db_user']['checkPidList'] = '';
$authInfo['db_user']['check_pid_clause'] = '';
}
$authInfo['db_groups']['table'] = $this->usergroup_table;
return $authInfo;
}
......
......@@ -17,7 +17,6 @@ namespace TYPO3\CMS\Core\Authentication;
use TYPO3\CMS\Core\Crypto\PasswordHashing\InvalidPasswordHashException;
use TYPO3\CMS\Core\Crypto\PasswordHashing\PasswordHashFactory;
use TYPO3\CMS\Core\Database\Connection;
use TYPO3\CMS\Core\Database\ConnectionPool;
use TYPO3\CMS\Core\SysLog\Action\Login as SystemLogLoginAction;
use TYPO3\CMS\Core\SysLog\Error as SystemLogErrorClassification;
......@@ -172,106 +171,6 @@ class AuthenticationService extends AbstractAuthenticationService
return 200;
}
/**
* Find usergroup records, currently only for frontend
*
* @param array $user Data of user.
* @param array $knownGroups Group data array of already known groups. This is handy if you want select other related groups. Keys in this array are unique IDs of those groups.
* @return mixed Groups array, keys = uid which must be unique
*/
public function getGroups($user, $knownGroups)
{
// Attention: $knownGroups is not used within this method, but other services can use it.
// This parameter should not be removed!
// The FrontendUserAuthentication call getGroups and handover the previous detected groups.
$groupDataArr = [];
if ($this->mode === 'getGroupsFE') {
$groups = [];
if ($user[$this->db_user['usergroup_column']] ?? false) {
$groupList = $user[$this->db_user['usergroup_column']];
$groups = [];
$this->getSubGroups($groupList, '', $groups);
}
$groups = array_unique($groups);
if (!empty($groups)) {
$this->logger->debug('Get usergroups with id: ' . implode(',', $groups));
$queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)
->getQueryBuilderForTable($this->db_groups['table']);
$res = $queryBuilder->select('*')
->from($this->db_groups['table'])
->where(
$queryBuilder->expr()->in(
'uid',
$queryBuilder->createNamedParameter($groups, Connection::PARAM_INT_ARRAY)
)
)
->execute();
while ($row = $res->fetch()) {
$groupDataArr[$row['uid']] = $row;
}
} else {
$this->logger->debug('No usergroups found.');
}
}
return $groupDataArr;
}
/**
* Fetches subgroups of groups. Function is called recursively for each subgroup.
* Function was previously copied from
* \TYPO3\CMS\Core\Authentication\BackendUserAuthentication->fetchGroups and has been slightly modified.
*
* @param string $grList Commalist of fe_groups uid numbers
* @param string $idList List of already processed fe_groups-uids so the function will not fall into an eternal recursion.
* @param array $groups
* @internal
*/
public function getSubGroups($grList, $idList, &$groups)
{
// Fetching records of the groups in $grList:
$queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable('fe_groups');
$res = $queryBuilder
->select('uid', 'subgroup')
->from($this->db_groups['table'])
->where(
$queryBuilder->expr()->in(
'uid',
$queryBuilder->createNamedParameter(
GeneralUtility::intExplode(',', $grList, true),
Connection::PARAM_INT_ARRAY
)
)
)
->execute();
// Internal group record storage
$groupRows = [];
// The groups array is filled
while ($row = $res->fetch()) {
if (!in_array($row['uid'], $groups)) {
$groups[] = $row['uid'];
}
$groupRows[$row['uid']] = $row;
}
// Traversing records in the correct order
$include_staticArr = GeneralUtility::intExplode(',', $grList);
// traversing list
foreach ($include_staticArr as $uid) {
// Get row:
$row = $groupRows[$uid];
// Must be an array and $uid should not be in the idList, because then it is somewhere previously in the grouplist
if (is_array($row) && !GeneralUtility::inList($idList, (string)$uid) && trim($row['subgroup'])) {
// Make integer list
$theList = implode(',', GeneralUtility::intExplode(',', $row['subgroup']));
// Call recursively, pass along list of already processed groups so they are not processed again.
$this->getSubGroups($theList, $idList . ',' . $uid, $groups);
}
}
}
/**
* Method updates a FE/BE user record - in this case a new password string will be set.
*
......
......@@ -82,12 +82,6 @@ class BackendUserAuthentication extends AbstractUserAuthentication
'filemounts' => []
];
/**
* This array will hold the groups that the user is a member of
* @var array
*/
public $userGroups = [];
/**
* This array holds the uid's of the groups in the listed order
* @var array
......
......@@ -146,8 +146,8 @@ class UserAspect implements AspectInterface
if ($this->isLoggedIn()) {
// If a user is logged in, always add "-2"
$groups = [0, -2];
if (!empty($this->user->groupData['uid'])) {
$groups = array_merge($groups, array_map('intval', $this->user->groupData['uid']));
if (!empty($this->user->userGroups)) {
$groups = array_merge($groups, array_keys($this->user->userGroups));
}
} else {
$groups = [0, -1];
......@@ -164,10 +164,7 @@ class UserAspect implements AspectInterface
public function getGroupNames(): array
{
$groupNames = [];
if ($this->user instanceof FrontendUserAuthentication) {
$groupNames = $this->user->groupData['title'];
}
if ($this->user instanceof BackendUserAuthentication) {
if ($this->user instanceof AbstractUserAuthentication) {
foreach ($this->user->userGroups as $userGroup) {
$groupNames[] = $userGroup['title'];
}
......
.. include:: ../../Includes.txt
===========================================================================
Breaking: #93108 - Reworked internal user group fetching for frontend users
===========================================================================
See :issue:`93108`
Description
===========
Frontend users now support the same loading mechanism for usergroups as
backend users, making it easier to exchange functionality by unifying the
code base.
In previous versions, the Authentication Service was used to fetch groups and
enable groups, which can be achieved via the `AfterGroupsResolved` PSR-14 event.
Fetching groups and permissions belongs to authorization, and not authentication
(identities), where this removal is conceptually suited outside of
authentication services.
The respective methods and properties
* :php:`TYPO3\CMS\Core\Authentication\AuthenticationService->getGroups()`
* :php:`TYPO3\CMS\Core\Authentication\AuthenticationService->getSubGroups()`
* :php:`TYPO3\CMS\Core\Authentication\AuthenticationService->db_groups`
have been removed.
At the same time, much of the PHP 4-based code base from frontend users
within `FrontendUserAuthentication` has been marked as internal or removed
completely, allowing this information not to be read or modified from the
outside anymore.
* :php:`TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->TSdataArray`
* :php:`TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->userTS`
* :php:`TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->userTSUpdated`
* :php:`TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->userData_change`
Impact
======
The authentication services "subtype", "getGroupsFE" and "authGroupsFE" are never
executed anymore.
Accessing the properties will trigger a PHP warning.
Affected Installations
======================
TYPO3 installations with custom extensions handling group related authentication
services, e.g. LDAP extensions.
Migration
=========
Use the mentioned PSR-14 event to load custom groups from different sources or
based on rules, or use a custom PSR-15 middleware to inject custom groups,
not based on a specific user, but related to a request.
It is possible to keep extensions compatible with TYPO3 v10 and v11 by keeping
the AuthenticationService "getGroupsFE" subtype, and adding the PSR-14 event to
an extension.
.. index:: Frontend, FullyScanned, ext:frontend
......@@ -98,7 +98,7 @@ class UserAspectTest extends UnitTestCase
$user->user = [
'uid' => 13
];
$user->groupData['uid'] = [1, 5, 7];
$user->userGroups = [1 => ['uid' => 1], 5 => ['uid' => 5], 7 => ['uid' => 7]];
$subject = new UserAspect($user);
self::assertTrue($subject->isLoggedIn());
}
......@@ -125,7 +125,7 @@ class UserAspectTest extends UnitTestCase
$user->user = [
'uid' => 13
];
$user->groupData['uid'] = [23, 54];
$user->userGroups = [23 => ['uid' => 23], 54 => ['uid' => 54]];
$subject = new UserAspect($user);
self::assertEquals([0, -2, 23, 54], $subject->getGroupIds());
}
......@@ -137,7 +137,7 @@ class UserAspectTest extends UnitTestCase
{
$user = new FrontendUserAuthentication();
// Not used, because overridden with 33
$user->groupData['uid'] = [23, 54];
$user->userGroups = [23 => ['uid' => 23], 54 => ['uid' => 54]];
$subject = new UserAspect($user, [33]);
self::assertEquals([33], $subject->getGroupIds());
}
......@@ -204,7 +204,9 @@ class UserAspectTest extends UnitTestCase
if ($userId) {
$user->user['uid'] = $userId;
}
$user->groupData['uid'] = $userGroups;
foreach ($userGroups ?? [] as $groupId) {
$user->userGroups[$groupId] = ['uid' => $groupId];
}
$subject = new UserAspect($user, $overriddenGroups);
self::assertEquals($expectedResult, $subject->isUserOrGroupSet());
}
......
......@@ -38,7 +38,7 @@ unset($extractorRegistry);
[
'title' => 'User authentication',
'description' => 'Authentication with username/password.',
'subtype' => 'getUserBE,getUserFE,authUserBE,authUserFE,getGroupsFE,processLoginDataBE,processLoginDataFE',
'subtype' => 'getUserBE,getUserFE,authUserBE,authUserFE,processLoginDataBE,processLoginDataFE',
'available' => true,
'priority' => 50,
'quality' => 50,
......
......@@ -88,12 +88,12 @@ class RedirectModeHandler
public function redirectModeGroupLogin(): string
{
// taken from dkd_redirect_at_login written by Ingmar Schlecht; database-field changed
$groupData = $this->userService->getFeUserGroupData();
$groups = $this->userService->getFeUserGroupData();
$groupUids = array_unique(array_map('intval', $groupData['uid']));
if (empty($groupData['uid'])) {
if (empty($groups)) {
return '';
}
$groupUids = array_keys($groups);
// take the first group with a redirect page
$redirectPageId = $this->frontendUserGroupRepository->findRedirectPageIdByGroupId(
......
......@@ -56,7 +56,7 @@ class UserService
public function getFeUserGroupData(): array
{
return $this->feUser->groupData;
return $this->feUser->userGroups;
}
public function getFeUserTable(): string
......
......@@ -44,9 +44,9 @@ class IfHasRoleViewHelperTest extends ViewHelperBaseTestcase
$this->context = GeneralUtility::makeInstance(Context::class);
$user = new FrontendUserAuthentication();
$user->user['uid'] = 13;
$user->groupData = [
'uid' => [1, 2],
'title' => ['Editor', 'OtherRole']
$user->userGroups = [
1 => ['uid' => 1, 'title' => 'Editor'],
2 => ['uid' => 2, 'title' => 'OtherRole']
];
$this->context->setAspect('frontend.user', new UserAspect($user, [1, 2]));
$this->viewHelper = new IfHasRoleViewHelper();
......
......@@ -16,7 +16,7 @@
namespace TYPO3\CMS\Frontend\Authentication;
use TYPO3\CMS\Core\Authentication\AbstractUserAuthentication;
use TYPO3\CMS\Core\Authentication\AuthenticationService;
use TYPO3\CMS\Core\Authentication\GroupResolver;
use TYPO3\CMS\Core\Context\UserAspect;
use TYPO3\CMS\Core\Database\ConnectionPool;
use TYPO3\CMS\Core\Session\UserSession;
......@@ -124,22 +124,17 @@ class FrontendUserAuthentication extends AbstractUserAuthentication
* Used to accumulate the TSconfig data of the user
* @var array
*/
public $TSdataArray = [];
protected $TSdataArray = [];
/**
* @var array
*/
public $userTS = [];
protected $userTS = [];
/**
* @var bool
*/
public $userTSUpdated = false;
/**
* @var bool
*/
public $userData_change = false;
protected $userData_change = false;
/**
* @var bool
......@@ -264,17 +259,15 @@ class FrontendUserAuthentication extends AbstractUserAuthentication
}
/**
* Will select all fe_groups records that the current fe_user is member of
* and which groups are also allowed in the current domain.
* It also accumulates the TSconfig for the fe_user/fe_groups in ->TSdataArray
* Will select all fe_groups records that the current fe_user is member of.
*
* @return int Returns the number of usergroups for the frontend users (if the internal user record exists and the usergroup field contains a value)
* It also accumulates the TSconfig for the fe_user/fe_groups in ->TSdataArray
*/
public function fetchGroupData()
{
$this->TSdataArray = [];
$this->userTS = [];
$this->userTSUpdated = false;
$this->userGroups = [];
$this->groupData = [
'title' => [],
'uid' => [],
......@@ -282,68 +275,34 @@ class FrontendUserAuthentication extends AbstractUserAuthentication
];
// Setting default configuration:
$this->TSdataArray[] = $GLOBALS['TYPO3_CONF_VARS']['FE']['defaultUserTSconfig'];
// Get the info data for auth services
$authInfo = $this->getAuthInfoArray();
$groupDataArr = [];
if (is_array($this->user)) {
$this->logger->debug('Get usergroups for user', [
$this->userid_column => $this->user[$this->userid_column],
$this->username_column => $this->user[$this->username_column]
]);
} else {
$this->logger->debug('Get usergroups for "anonymous" user');
}
$groupDataArr = [];
// Use 'auth' service to find the groups for the user
$subType = 'getGroups' . $this->loginType;
/** @var AuthenticationService $serviceObj */
foreach ($this->getAuthServices($subType, [], $authInfo) as $serviceObj) {
$groupData = $serviceObj->getGroups($this->user, $groupDataArr);
if (is_array($groupData) && !empty($groupData)) {
// Keys in $groupData should be unique ids of the groups (like "uid") so this function will override groups.
$groupDataArr = $groupData + $groupDataArr;
}
$groupDataArr = GeneralUtility::makeInstance(GroupResolver::class)->resolveGroupsForUser($this->user, $this->usergroup_table);
}
if (empty($groupDataArr)) {
$this->logger->debug('No usergroups found by services');
}
if (!empty($groupDataArr)) {
$this->logger->debug(count($groupDataArr) . ' usergroup records found by services');
$this->logger->debug('No usergroups found');
} else {
$this->logger->debug(count($groupDataArr) . ' usergroup records found');
}
// Use 'auth' service to check the usergroups if they are really valid
foreach ($groupDataArr as $groupData) {
// By default a group is valid
$validGroup = true;
$subType = 'authGroups' . $this->loginType;
foreach ($this->getAuthServices($subType, [], $authInfo) as $serviceObj) {
// we assume that the service defines the authGroup function
if (!$serviceObj->authGroup($this->user, $groupData)) {
$validGroup = false;
$this->logger->debug($subType . ' auth service did not auth group', [
'uid ' => $groupData['uid'],
'title' => $groupData['title'],
]);
break;
}
}
if ($validGroup && (string)$groupData['uid'] !== '') {
$this->groupData['title'][$groupData['uid']] = $groupData['title'];
$this->groupData['uid'][$groupData['uid']] = $groupData['uid'];
$this->groupData['pid'][$groupData['uid']] = $groupData['pid'];
$this->groupData['TSconfig'][$groupData['uid']] = $groupData['TSconfig'];
}
}
if (!empty($this->groupData) && !empty($this->groupData['TSconfig'])) {
// TSconfig: collect it in the order it was collected
foreach ($this->groupData['TSconfig'] as $TSdata) {
$this->TSdataArray[] = $TSdata;
}
$this->TSdataArray[] = $this->user['TSconfig'];
// Sort information
ksort($this->groupData['title']);
ksort($this->groupData['uid']);
ksort($this->groupData['pid']);
$groupId = (int)$groupData['uid'];
$this->groupData['title'][$groupId] = $groupData['title'];
$this->groupData['uid'][$groupId] = $groupData['uid'];
$this->groupData['pid'][$groupId] = $groupData['pid'];
$this->TSdataArray[] = $groupData['TSconfig'];
$this->userGroups[$groupId] = $groupData;
}
return !empty($this->groupData['uid']) ? count($this->groupData['uid']) : 0;
$this->TSdataArray[] = $this->user['TSconfig'] ?? '';
// Sort information
ksort($this->groupData['title']);
ksort($this->groupData['uid']);
ksort($this->groupData['pid']);
}
/**
......@@ -356,19 +315,19 @@ class FrontendUserAuthentication extends AbstractUserAuthentication
public function createUserAspect(bool $respectUserGroups = true): UserAspect
{
$userGroups = [0];
$isUserAndGroupSet = is_array($this->user) && !empty($this->groupData['uid']);
$isUserAndGroupSet = is_array($this->user) && !empty($this->userGroups);
if ($isUserAndGroupSet) {
// group -2 is not an existing group, but denotes a 'default' group when a user IS logged in.
// This is used to let elements be shown for all logged in users!
$userGroups[] = -2;
$groupsFromUserRecord = $this->groupData['uid'];
$groupsFromUserRecord = array_keys($this->userGroups);
} else {
// group -1 is not an existing group, but denotes a 'default' group when not logged in.
// This is used to let elements be hidden, when a user is logged in!
$userGroups[] = -1;
if ($respectUserGroups) {
// For cases where logins are not banned from a branch usergroups can be set based on IP masks so we should add the usergroups uids.
$groupsFromUserRecord = $this->groupData['uid'];
$groupsFromUserRecord = array_keys($this->userGroups);
} else {
// Set to blank since we will NOT risk any groups being set when no logins are allowed!
$groupsFromUserRecord = [];
......@@ -378,7 +337,7 @@ class FrontendUserAuthentication extends AbstractUserAuthentication
$groupsFromUserRecord = array_unique($groupsFromUserRecord);
if ($respectUserGroups && !empty($groupsFromUserRecord)) {
sort($groupsFromUserRecord);
$userGroups = array_merge($userGroups, array_map('intval', $groupsFromUserRecord));
$userGroups = array_merge($userGroups, $groupsFromUserRecord);
}
// For every 60 seconds the is_online timestamp for a logged-in user is updated
......@@ -397,16 +356,15 @@ class FrontendUserAuthentication extends AbstractUserAuthentication
*/
public function getUserTSconf()
{
if (!$this->userTSUpdated) {
if ($this->userTS === [] && !empty($this->TSdataArray)) {
// Parsing the user TS (or getting from cache)
$this->TSdataArray = TypoScriptParser::checkIncludeLines_array($this->TSdataArray);
$userTS = implode(LF . '[GLOBAL]' . LF, $this->TSdataArray);
$parseObj = GeneralUtility::makeInstance(TypoScriptParser::class);
$parseObj->parse($userTS);
$this->userTS = $parseObj->setup;
$this->userTSUpdated = true;
}
return $this->userTS;
return $this->userTS ?? [];
}
/*****************************************
......
......@@ -499,7 +499,9 @@ class ConditionMatcherTest extends FunctionalTestCase
{
$frontendUser = $GLOBALS['TSFE']->fe_user;
$frontendUser->user['uid'] = empty($groups) ? 0 : 13;
$frontendUser->groupData['uid'] = $groups;
foreach ($groups as $groupId) {
$frontendUser->userGroups[$groupId] = ['uid' => $groupId];
}
GeneralUtility::makeInstance(Context::class)->setAspect('frontend.user', new UserAspect($frontendUser, $groups));
}
......
......@@ -4704,4 +4704,18 @@ return [
'Deprecation-93093-DeprecateMethodNameInShortcutPHPAPI.rst'
],
],