[TASK] Streamline userid/username handling and system-maintainer checks

The new methods AbstractUserAuthentication::getUserName() and AbstractUserAuthentication::getUserId() can be used to resolve the corresponding values (instead of using the $user->user array). In addition, the pure system-maintainer checks have been moved into to central BackendUserAuthentication::isSystemMaintainer(). The term "pure" refers to ignoring the development context and not applying any fallbacks in case the setting is empty. Resolves: #103323 Releases: main, 12.4 Change-Id: Ia7db222dac32acc2ef13a34ded4545ba1aedefc3 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/83440 Tested-by: core-ci <typo3@b13.com> Tested-by: Oliver Hader <oliver.hader@typo3.org> Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
TYPO3 · Mar 12, 2024 · 6055db0 · 6055db0
1 parent b00cc35
commit 6055db0
Show file tree

Hide file tree

Showing 19 changed files with 71 additions and 58 deletions.
diff --git a/typo3/sysext/backend/Classes/ContextMenu/ItemProviders/RecordProvider.php b/typo3/sysext/backend/Classes/ContextMenu/ItemProviders/RecordProvider.php
@@ -619,8 +619,7 @@ protected function isRecordATranslation(): bool
      */
     protected function isRecordCurrentBackendUser(): bool
     {
-        return $this->table === 'be_users'
-            && (int)($this->record['uid'] ?? 0) === (int)$this->backendUser->user[$this->backendUser->userid_column];
+        return $this->table === 'be_users' && (int)($this->record['uid'] ?? 0) === $this->backendUser->getUserId();
     }
 
     /**

diff --git a/typo3/sysext/backend/Classes/Controller/EditDocumentController.php b/typo3/sysext/backend/Classes/Controller/EditDocumentController.php
@@ -522,7 +522,7 @@ protected function processData(ModuleTemplate $view, ServerRequestInterface $req
             $tce->process_cmdmap();
 
             // Update the module menu for the current backend user, as they updated their UI language
-            $currentUserId = (int)($beUser->user[$beUser->userid_column] ?? 0);
+            $currentUserId = $beUser->getUserId();
             if ($currentUserId
                 && (string)($this->data['be_users'][$currentUserId]['lang'] ?? '') !== ''
                 && $this->data['be_users'][$currentUserId]['lang'] !== $beUser->user['lang']
@@ -1849,8 +1849,7 @@ protected function getDisableDelete(): bool
     protected function isRecordCurrentBackendUser(): bool
     {
         $backendUser = $this->getBackendUser();
-        return $this->firstEl['table'] === 'be_users'
-            && (int)($this->firstEl['uid'] ?? 0) === (int)$backendUser->user[$backendUser->userid_column];
+        return $this->firstEl['table'] === 'be_users' && (int)($this->firstEl['uid'] ?? 0) === $backendUser->getUserId();
     }
 
     /**

diff --git a/typo3/sysext/backend/Classes/Controller/MfaAjaxController.php b/typo3/sysext/backend/Classes/Controller/MfaAjaxController.php
@@ -83,7 +83,7 @@ public function handleRequest(ServerRequestInterface $request): ResponseInterfac
     protected function deactivateAction(ServerRequestInterface $request, AbstractUserAuthentication $user): array
     {
         $lang = $this->getLanguageService();
-        $userName = (string)($user->user[$user->username_column] ?? '');
+        $userName = $user->getUserName() ?? '';
         $providerToDeactivate = (string)($request->getParsedBody()['provider'] ?? '');
 
         if ($providerToDeactivate === '') {
@@ -191,13 +191,12 @@ protected function isAllowedToPerformAction(string $action, AbstractUserAuthenti
                 return false;
             }
             // Providers from system maintainers can only be deactivated by system maintainers.
-            // This check is however only be necessary if the target is a backend user.
-            if ($user instanceof BackendUserAuthentication) {
-                $systemMaintainers = array_map('intval', $GLOBALS['TYPO3_CONF_VARS']['SYS']['systemMaintainers'] ?? []);
-                $isTargetUserSystemMaintainer = $user->isAdmin() && in_array((int)$user->user[$user->userid_column], $systemMaintainers, true);
-                if ($isTargetUserSystemMaintainer && !$this->getBackendUser()->isSystemMaintainer()) {
-                    return false;
-                }
+            // However, this check is only necessary if the target is a backend user.
+            if (($user instanceof BackendUserAuthentication)
+                && $user->isSystemMaintainer(true)
+                && !$this->getBackendUser()->isSystemMaintainer()
+            ) {
+                return false;
             }
             return true;
         }

diff --git a/typo3/sysext/backend/Classes/Controller/MfaController.php b/typo3/sysext/backend/Classes/Controller/MfaController.php
@@ -194,10 +194,10 @@ protected function log(
         int $error = SystemLogErrorClassification::MESSAGE
     ): void {
         $user = $this->getBackendUser();
-        $username = $user->user[$user->username_column];
+        $username = $user->getUserName();
         $context = [
             'user' => [
-                'uid' => $user->user[$user->userid_column],
+                'uid' => $user->getUserId(),
                 'username' => $username,
             ],
         ];

diff --git a/typo3/sysext/backend/Classes/Controller/MfaSetupController.php b/typo3/sysext/backend/Classes/Controller/MfaSetupController.php
@@ -264,8 +264,8 @@ protected function log(string $message, ?MfaProviderManifestInterface $mfaProvid
         $user = $this->getBackendUser();
         $context = [
             'user' => [
-                'uid' => $user->user[$user->userid_column],
-                'username' => $user->user[$user->username_column],
+                'uid' => $user->getUserId(),
+                'username' => $user->getUserName(),
             ],
         ];
         if ($mfaProvider !== null) {

diff --git a/typo3/sysext/backend/Classes/Controller/RecordListDownloadController.php b/typo3/sysext/backend/Classes/Controller/RecordListDownloadController.php
@@ -226,7 +226,7 @@ protected function jsonDownloadAction(
                         'table' => $this->table,
                         'page' => $this->id,
                         'timestamp' => GeneralUtility::makeInstance(Context::class)->getPropertyFromAspect('date', 'timestamp'),
-                        'user' => $user->user[$user->username_column] ?? '',
+                        'user' => $user->getUserName() ?? '',
                         'site' => $GLOBALS['TYPO3_CONF_VARS']['SYS']['sitename'] ?? '',
                         'options' => [
                             'columns' => array_values($headerRow),

diff --git a/typo3/sysext/backend/Classes/Controller/SwitchUserController.php b/typo3/sysext/backend/Classes/Controller/SwitchUserController.php
@@ -65,8 +65,8 @@ public function switchUserAction(ServerRequestInterface $request): ResponseInter
         $targetUserId = (int)($request->getParsedBody()['targetUser'] ?? 0);
 
         if (!$targetUserId
-            || $targetUserId === (int)($currentUser->user[$currentUser->userid_column] ?? 0)
             || !$currentUser->isAdmin()
+            || $targetUserId === $currentUser->getUserId()
             || $currentUser->getOriginalUserIdWhenInSwitchUserMode() !== null
         ) {
             return $this->jsonResponse(['success' => false]);
@@ -86,13 +86,13 @@ public function switchUserAction(ServerRequestInterface $request): ResponseInter
 
         // Write user switch to log
         $currentUser->writelog(Type::LOGIN, 2, 0, 1, 'User %s switched to user %s (be_users:%s)', [
-            $currentUser->user[$currentUser->username_column] ?? '',
+            $currentUser->getUserName() ?? '',
             $targetUser['username'] ?? '',
             $targetUserId,
         ]);
 
         $sessionObject = $currentUser->getSession();
-        $sessionObject->set('backuserid', (int)($currentUser->user[$currentUser->userid_column] ?? 0));
+        $sessionObject->set('backuserid', $currentUser->getUserId() ?? 0);
         $sessionRecord = $sessionObject->toArray();
         $sessionRecord['ses_userid'] = $targetUserId;
         $this->sessionBackend->update($sessionObject->getIdentifier(), $sessionRecord);

diff --git a/typo3/sysext/backend/Classes/Form/Element/MfaInfoElement.php b/typo3/sysext/backend/Classes/Form/Element/MfaInfoElement.php
@@ -70,13 +70,12 @@ public function render(): array
 
         $isDeactivationAllowed = true;
         // Providers from system maintainers can only be deactivated by system maintainers.
-        // This check is however only be necessary if the target is a backend user.
-        if ($targetUser instanceof BackendUserAuthentication) {
-            $systemMaintainers = array_map('intval', $GLOBALS['TYPO3_CONF_VARS']['SYS']['systemMaintainers'] ?? []);
-            $isTargetUserSystemMaintainer = $targetUser->isAdmin() && in_array($userId, $systemMaintainers, true);
-            if ($isTargetUserSystemMaintainer && !$currentBackendUser->isSystemMaintainer()) {
-                $isDeactivationAllowed = false;
-            }
+        // However, this check is only necessary if the target is a backend user.
+        if (($targetUser instanceof BackendUserAuthentication)
+            && $targetUser->isSystemMaintainer(true)
+            && !$currentBackendUser->isSystemMaintainer()
+        ) {
+            $isDeactivationAllowed = false;
         }
 
         // Fetch providers from the mfa field

diff --git a/typo3/sysext/backend/Classes/Form/FieldInformation/AdminIsSystemMaintainer.php b/typo3/sysext/backend/Classes/Form/FieldInformation/AdminIsSystemMaintainer.php
@@ -49,7 +49,7 @@ public function render(): array
             return $resultArray;
         }
 
-        // False if current user is not in system maintainer list or if switch to user mode is active
+        // False, if the current user is not in the list of system maintainers, or if the switch to user mode is active
         $isCurrentUserSystemMaintainer = $this->getBackendUser()->isSystemMaintainer();
         $systemMaintainers = array_map('intval', $GLOBALS['TYPO3_CONF_VARS']['SYS']['systemMaintainers'] ?? []);
         $isTargetUserInSystemMaintainerList = in_array((int)$this->data['vanillaUid'], $systemMaintainers, true);

diff --git a/typo3/sysext/backend/Classes/Search/EventListener/AddLiveSearchResultActionsListener.php b/typo3/sysext/backend/Classes/Search/EventListener/AddLiveSearchResultActionsListener.php
@@ -65,9 +65,9 @@ protected function addSwitchUserAction(ResultItem $resultItem): void
 
         if (
             $backendUserIsActive
-            && (int)(($currentUser->user[$currentUser->userid_column] ?? 0) !== $resultItem->getExtraData()['uid'])
             && $currentUser->isAdmin()
             && $currentUser->getOriginalUserIdWhenInSwitchUserMode() === null
+            && (int)$currentUser->getUserId() !== (int)$resultItem->getExtraData()['uid']
         ) {
             $switchUserAction = (new ResultItemAction('switch_backend_user'))
                 ->setLabel($this->languageService->sL('LLL:EXT:beuser/Resources/Private/Language/locallang.xlf:switchBackMode'))

diff --git a/typo3/sysext/beuser/Classes/ViewHelpers/SwitchUserViewHelper.php b/typo3/sysext/beuser/Classes/ViewHelpers/SwitchUserViewHelper.php
@@ -50,7 +50,7 @@ public function render(): string
         $currentUser = self::getBackendUserAuthentication();
         $iconFactory = GeneralUtility::makeInstance(IconFactory::class);
 
-        if ((int)$targetUser->getUid() === (int)($currentUser->user[$currentUser->userid_column] ?? 0)
+        if ((int)$targetUser->getUid() === (int)$currentUser->getUserId()
             || !$targetUser->isActive()
             || !$currentUser->isAdmin()
             || $currentUser->getOriginalUserIdWhenInSwitchUserMode() !== null

diff --git a/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php b/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php
@@ -585,8 +585,8 @@ public function checkAuthentication(ServerRequestInterface $request)
                 // The login session is started.
                 $this->loginSessionStarted = true;
                 $this->logger->debug('User session finally read', [
-                    $this->userid_column => $this->user[$this->userid_column],
-                    $this->username_column => $this->user[$this->username_column],
+                    $this->userid_column => $this->getUserId(),
+                    $this->username_column => $this->getUserName(),
                 ]);
             } else {
                 // if we come here the current session is for sure not anonymous as this is a pre-condition for $authenticated = true
@@ -1013,15 +1013,16 @@ protected function userConstraints(): QueryRestrictionContainerInterface
      */
     public function writeUC()
     {
-        if (is_array($this->user) && $this->user[$this->userid_column]) {
+        $userId = $this->getUserId();
+        if ($userId) {
             $this->logger->debug('writeUC: {userid_column}={value}', [
                 'userid_column' => $this->userid_column,
-                'value' => $this->user[$this->userid_column],
+                'value' => $userId,
             ]);
             GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable($this->user_table)->update(
                 $this->user_table,
                 ['uc' => serialize($this->uc)],
-                [$this->userid_column => (int)$this->user[$this->userid_column]],
+                [$this->userid_column => $userId],
                 ['uc' => Connection::PARAM_LOB]
             );
         }
@@ -1323,6 +1324,22 @@ public function getRawUserByName($name)
         return $query->executeQuery()->fetchAssociative();
     }
 
+    public function getUserId(): ?int
+    {
+        if (isset($this->user[$this->userid_column])) {
+            return (int)$this->user[$this->userid_column];
+        }
+        return null;
+    }
+
+    public function getUserName(): ?string
+    {
+        if (isset($this->user[$this->username_column])) {
+            return (string)$this->user[$this->username_column];
+        }
+        return null;
+    }
+
     public function getSession(): UserSession
     {
         return $this->userSession;

diff --git a/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php b/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php
@@ -430,17 +430,19 @@ public function modAccess($conf)
      * Checks if the user is in the valid list of allowed system maintainers. if the list is not set,
      * then all admins are system maintainers. If the list is empty, no one is system maintainer (good for production
      * systems). If the currently logged in user is in "switch user" mode, this method will return false.
+     *
+     * @param bool $pure Whether to apply pure behavior (ignore development & skip fallback for empty setting)
      */
-    public function isSystemMaintainer(): bool
+    public function isSystemMaintainer(bool $pure = false): bool
     {
         if (!$this->isAdmin()) {
             return false;
         }
 
-        if ($GLOBALS['BE_USER']->getOriginalUserIdWhenInSwitchUserMode()) {
+        if (!$pure && $GLOBALS['BE_USER']->getOriginalUserIdWhenInSwitchUserMode()) {
             return false;
         }
-        if (Environment::getContext()->isDevelopment()) {
+        if (!$pure && Environment::getContext()->isDevelopment()) {
             return true;
         }
         $systemMaintainers = $GLOBALS['TYPO3_CONF_VARS']['SYS']['systemMaintainers'] ?? [];
@@ -451,11 +453,10 @@ public function isSystemMaintainer(): bool
         // No system maintainers set up yet, so any admin is allowed to access the modules
         // but explicitly no system maintainers allowed (empty string in TYPO3_CONF_VARS).
         // @todo: this needs to be adjusted once system maintainers can log into the install tool with their credentials
-        if (isset($GLOBALS['TYPO3_CONF_VARS']['SYS']['systemMaintainers'])
-            && empty($GLOBALS['TYPO3_CONF_VARS']['SYS']['systemMaintainers'])) {
-            return false;
+        if (!$pure && !isset($GLOBALS['TYPO3_CONF_VARS']['SYS']['systemMaintainers'])) {
+            return true;
         }
-        return true;
+        return false;
     }
 
     /**
@@ -2276,8 +2277,8 @@ protected function evaluateMfaRequirements(): void
         // In case the current session is a "switch-user" session, MFA is not required
         if ($this->getOriginalUserIdWhenInSwitchUserMode() !== null) {
             $this->logger->debug('MFA is skipped in switch user mode', [
-                $this->userid_column => $this->user[$this->userid_column],
-                $this->username_column => $this->user[$this->username_column],
+                $this->userid_column => $this->getUserId(),
+                $this->username_column => $this->getUserName(),
             ]);
             return;
         }

diff --git a/typo3/sysext/core/Classes/Authentication/Mfa/MfaProviderPropertyManager.php b/typo3/sysext/core/Classes/Authentication/Mfa/MfaProviderPropertyManager.php
@@ -161,16 +161,16 @@ protected function storeProperties(): bool
         $this->logger->debug('MFA properties updated', [
             'provider' => $this->providerIdentifier,
             'user' => [
-                'uid' => $this->user->user[$this->user->userid_column],
-                'username' => $this->user->user[$this->user->username_column],
+                'uid' => $this->user->getUserId(),
+                'username' => $this->user->getUserName(),
             ],
         ]);
 
         // Store updated mfa properties in the database
         return (bool)GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable($this->user->user_table)->update(
             $this->user->user_table,
             [self::DATABASE_FIELD_NAME => $mfa],
-            [$this->user->userid_column => (int)$this->user->user[$this->user->userid_column]],
+            [$this->user->userid_column => (int)$this->user->getUserId()],
             [self::DATABASE_FIELD_NAME => Connection::PARAM_LOB]
         );
     }

diff --git a/typo3/sysext/core/Classes/Hooks/TcaDisplayConditions.php b/typo3/sysext/core/Classes/Hooks/TcaDisplayConditions.php
@@ -49,7 +49,7 @@ public function isExtensionInstalled(array $parameters): bool
     public function isRecordCurrentUser(array $parameters): bool
     {
         $backendUser = $this->getBackendUser();
-        $isCurrentUser = (int)($parameters['record']['uid'] ?? 0) === (int)$backendUser->user[$backendUser->userid_column];
+        $isCurrentUser = (int)($parameters['record']['uid'] ?? 0) === (int)$backendUser->getUserId();
         return strtolower($parameters['conditionParameters'][0] ?? 'true') !== 'true' ? !$isCurrentUser : $isCurrentUser;
     }
 

diff --git a/typo3/sysext/frontend/Classes/Authentication/FrontendUserAuthentication.php b/typo3/sysext/frontend/Classes/Authentication/FrontendUserAuthentication.php
@@ -302,8 +302,8 @@ public function fetchGroupData(ServerRequestInterface $request)
         $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],
+                $this->userid_column => $this->getUserId(),
+                $this->username_column => $this->getUserName(),
             ]);
             $groupDataArr = GeneralUtility::makeInstance(GroupResolver::class)->resolveGroupsForUser($this->user, $this->usergroup_table);
         }

diff --git a/typo3/sysext/setup/Classes/Controller/SetupModuleController.php b/typo3/sysext/setup/Classes/Controller/SetupModuleController.php
@@ -776,9 +776,8 @@ protected function getFieldsFromShowItem()
         }
 
         $backendUser = $this->getBackendUser();
-        $systemMaintainers = array_map('intval', $GLOBALS['TYPO3_CONF_VARS']['SYS']['systemMaintainers'] ?? []);
-        if ($backendUser->getOriginalUserIdWhenInSwitchUserMode() && in_array((int)$backendUser->user['uid'], $systemMaintainers, true)) {
-            // DataHandler denies changing password of system maintainer users in switch user mode.
+        if ($backendUser->getOriginalUserIdWhenInSwitchUserMode() && $backendUser->isSystemMaintainer(true)) {
+            // DataHandler denies changing the password of system maintainer users in switch user mode.
             // Do not show the password fields is this case.
             $key = array_search('password', $allowedFields);
             if ($key !== false) {

diff --git a/typo3/sysext/sys_note/Classes/Renderer/NoteRenderer.php b/typo3/sysext/sys_note/Classes/Renderer/NoteRenderer.php
@@ -50,13 +50,13 @@ public function renderList(ServerRequestInterface $request, int $pid, int $posit
     {
         $backendUser = $this->getBackendUser();
         if ($pid <= 0
-            || empty($backendUser->user[$backendUser->userid_column])
+            || empty($backendUser->getUserId())
             || !$backendUser->check('tables_select', 'sys_note')
         ) {
             return '';
         }
 
-        $notes = $this->sysNoteRepository->findByPidAndAuthorId($pid, (int)$backendUser->user[$backendUser->userid_column], $position);
+        $notes = $this->sysNoteRepository->findByPidAndAuthorId($pid, (int)$backendUser->getUserId(), $position);
         if (!$notes) {
             return '';
         }

diff --git a/typo3/sysext/webhooks/Classes/Message/MfaVerificationErrorOccurredMessage.php b/typo3/sysext/webhooks/Classes/Message/MfaVerificationErrorOccurredMessage.php
@@ -55,8 +55,8 @@ public static function createFromEvent(MfaVerificationFailedEvent $event): self
             $event->getRequest()->getUri(),
             [
                 'user' => [
-                    'id' => $user->user[$user->userid_column],
-                    'name' => $user->user[$user->username_column],
+                    'id' => $user->getUserId(),
+                    'name' => $user->getUserName(),
                 ],
                 'provider' => $event->getProviderIdentifier(),
                 'isLocked' => $event->isProviderLocked(),