From 6bf0d254c1d9757a9547751aa5bcde868709b5d0 Mon Sep 17 00:00:00 2001 From: Oliver Bartsch <bo@cedev.de> Date: Thu, 6 Jul 2023 15:49:24 +0200 Subject: [PATCH] [BUGFIX] Properly handle user logged in tasks with MFA enabled In case a user has MFA enabled, the same handling is now done as for users, not having MFA enabled. This is e.g. dispatching the AfterUserLoggedInEvent. Resolves: #99864 Resolves: #100128 Releases: main, 12.4 Change-Id: I1df7a23e2b1a1ae09ac3b0217364478155af6bca Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/79807 Tested-by: core-ci <typo3@b13.com> Tested-by: Benjamin Franzke <ben@bnf.dev> Reviewed-by: Benjamin Franzke <ben@bnf.dev> --- .../Classes/Controller/MfaController.php | 6 +- .../BackendUserAuthentication.php | 60 ++++++++++++------- 2 files changed, 41 insertions(+), 25 deletions(-) diff --git a/typo3/sysext/backend/Classes/Controller/MfaController.php b/typo3/sysext/backend/Classes/Controller/MfaController.php index 1faf759161fe..adebc0f7c39f 100644 --- a/typo3/sysext/backend/Classes/Controller/MfaController.php +++ b/typo3/sysext/backend/Classes/Controller/MfaController.php @@ -120,7 +120,8 @@ class MfaController extends AbstractMfaController */ protected function verifyAction(ServerRequestInterface $request, MfaProviderManifestInterface $mfaProvider): ResponseInterface { - $propertyManager = MfaProviderPropertyManager::create($mfaProvider, $this->getBackendUser()); + $backendUser = $this->getBackendUser(); + $propertyManager = MfaProviderPropertyManager::create($mfaProvider, $backendUser); // Check if the provider can process the request and is not temporarily blocked if (!$mfaProvider->canProcess($request) || $mfaProvider->isLocked($propertyManager)) { @@ -150,7 +151,8 @@ class MfaController extends AbstractMfaController $this->log('Multi-factor authentication successful for user ###USERNAME###'); // If verified, store this information in the session // and initiate a redirect back to the login view. - $this->getBackendUser()->setAndSaveSessionData('mfa', true); + $backendUser->setAndSaveSessionData('mfa', true); + $backendUser->handleUserLoggedIn($request); return new RedirectResponse( $this->uriBuilder->buildUriWithRedirect('login', [], RouteRedirect::createFromRequest($request)) ); diff --git a/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php b/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php index d1f47b6b48c9..733754ef0d48 100644 --- a/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php +++ b/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php @@ -2092,30 +2092,44 @@ class BackendUserAuthentication extends AbstractUserAuthentication $this->fetchGroupData(); // Setting the UC array. It's needed with fetchGroupData first, due to default/overriding of values. $this->backendSetUC(); - if ($this->loginSessionStarted) { - // Also, if there is a recovery link set, unset it now - // this will be moved into its own Event at a later stage. - // If a token was set previously, this is now unset, as it was now possible to log-in - if ($this->user['password_reset_token'] ?? '') { - GeneralUtility::makeInstance(ConnectionPool::class) - ->getConnectionForTable($this->user_table) - ->update($this->user_table, ['password_reset_token' => ''], ['uid' => $this->user['uid']]); - } + if ($this->loginSessionStarted && !($this->getSessionData('mfa') ?? false)) { + // Handling user logged in. By checking for the mfa session key, it's ensured, the + // handling is only done once, since MfaController does the handling on its own. + $this->handleUserLoggedIn(); + } + } - $event = new AfterUserLoggedInEvent($this, $request); - GeneralUtility::makeInstance(EventDispatcherInterface::class)->dispatch($event); - // Process hooks - $hooks = $GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_userauthgroup.php']['backendUserLogin'] ?? []; - if (!empty($hooks)) { - trigger_error( - '$GLOBALS[\'TYPO3_CONF_VARS\'][\'SC_OPTIONS\'][\'t3lib/class.t3lib_userauthgroup.php\'][\'backendUserLogin\'] will be removed in TYPO3 v13.0. Use the PSR-14 "AfterUserLoggedInEvent" instead.', - E_USER_DEPRECATED - ); - } - foreach ($hooks as $_funcRef) { - $_params = ['user' => $this->user]; - GeneralUtility::callUserFunction($_funcRef, $_params, $this); - } + /** + * Is called after a user has sucesfully logged in. So either by using only one factor + * (e.g. username/password) or after the multi-factor authentication process has been passed. + * + * @internal + */ + public function handleUserLoggedIn(ServerRequestInterface $request = null): void + { + // Also, if there is a recovery link set, unset it now + // this will be moved into its own Event at a later stage. + // If a token was set previously, this is now unset, as it was now possible to log-in + if ($this->user['password_reset_token'] ?? '') { + GeneralUtility::makeInstance(ConnectionPool::class) + ->getConnectionForTable($this->user_table) + ->update($this->user_table, ['password_reset_token' => ''], ['uid' => $this->user['uid']]); + } + + $event = new AfterUserLoggedInEvent($this, $request); + GeneralUtility::makeInstance(EventDispatcherInterface::class)->dispatch($event); + + // Process hooks + $hooks = $GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_userauthgroup.php']['backendUserLogin'] ?? []; + if (!empty($hooks)) { + trigger_error( + '$GLOBALS[\'TYPO3_CONF_VARS\'][\'SC_OPTIONS\'][\'t3lib/class.t3lib_userauthgroup.php\'][\'backendUserLogin\'] will be removed in TYPO3 v13.0. Use the PSR-14 "AfterUserLoggedInEvent" instead.', + E_USER_DEPRECATED + ); + } + foreach ($hooks as $_funcRef) { + $_params = ['user' => $this->user]; + GeneralUtility::callUserFunction($_funcRef, $_params, $this); } } -- GitLab