Project

General

Profile

Actions

Bug #93421

closed

Reconstitution of frontend sessions not possible in case of multiple session persistence procedures within one request

Added by Ralf Zimmermann about 3 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Locking / Session Handling
Target version:
-
Start date:
2021-02-03
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
11
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

In short:

If some session data are stored for a new anonymous user (no `fe_sessions` record exists) within the application layer and `\TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->storeSessionData()` is called within the application layer, then the persisted `fe_sessions` record can never be reconstituted in further requests.
This is because the `FrontendUserAuthenticator` middleware wants to update the already persisted (through the application layer) user session record.
Since the update procedure does not handle the `ses_iplock` property, this property will be updated as an empty string.
A further request will try to reconstitute the persisted session data and compares the `ses_iplock` value from the persisted record (empty string) with the configured `[FE][lockIP]` value (which will be internal transformed to '[DISABLED]' in case of '0').
Since the comparison of `'' === '[DISABLED]'` never matches, a new anonymous session will be created and the previous persisted session data leaves as garbage.

In detail:

The ipLock property from the UserSession model is some kind of imutable and has the default value `''`.
This property is always set from the "outside" (example 1 , example 2 , example 3).
If an anonymous session will be persisted through UserSessionManager::fixateAnonymousSession() , the session data to persist will be enriched with `ses_iplock` with a value of `[DISABLED]` or some parts of an ip address.
If the setting `[FE][lockIP]` is set to 0 ('Do not lock Backend User sessions to their IP address at all'), then the persisted `fe_sessions` record contains the string '[DISABLED]' as the value for the column `ses_iplock`.
If an already persisted anonymous session will be updated through UserSessionManager::updateSession , then the `UserSession` model will be converted to an array and this array data will be persisted.
Since the UserSession model was created by UserSession::createNonFixated() , the property "ipLock" has always an empty string as value within the model data.
Therefore, the update procedure overwrite the value '[DISABLED]' (written by an previous call of 'UserSessionManager::fixateAnonymousSession()') with an empty string.
A further request will compare the `ses_iplock` value from the persisted record (now '' instead of '[DISABLED]') with the configured value ('[DISABLED]') within UserSessionManager::getSessionFromSessionId().
Since the comparison of `'' === '[DISABLED]'` never matches, a new anonymous session will be created and the previous persisted session data leaves as garbage.

Let's take a look into some examples.

This is a working example:

We assume that the `fe_sessions` table is empty or no fe cookie exists and the setting `[FE][lockIP]` is set to 0 (default configuration).

We start with the middleware layer in incoming direction which creates the user session.

\TYPO3\CMS\Frontend\Middleware\FrontendUserAuthenticator->process() Line 63
    \TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->start() Line 261
        \TYPO3\CMS\Core\Session\UserSessionManager->createFromGlobalCookieOrAnonymous() Line 102
            \TYPO3\CMS\Core\Session\UserSessionManager->createAnonymousSession() Line 113

The user session model is created by `UserSession::createNonFixated()` -> the property "ipLock" has an empty string as value within the model data.

Now we are within the application layer, let's say an extbase controller and we do something like this

$this->getFrontendUser()->setKey('ses', 'foo_1', 'bar_1');

The data will be set to the `UserSession` model (but the model is still not persisted to the `fe_sessions` table).

Now we leave the application layer and enter the middleware layer again in outgoing direction.

\TYPO3\CMS\Frontend\Middleware\FrontendUserAuthenticator->process() Line 78
    \TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->storeSessionData() Line 339
        \TYPO3\CMS\Core\Session\UserSessionManager->fixateAnonymousSession() Line 168

The `FrontendUserAuthentication::storeSessionData()` procedure determines that the session data was never persisted before and call `UserSessionManager::fixateAnonymousSession()` to persist them.
`UserSessionManager::fixateAnonymousSession()` transform the `UserSession` model to an array, add the `ses_iplock` value and persist them.

        $sessionRecord = $session->toArray();
        $sessionRecord['ses_iplock'] = $sessionIpLock;
        ...
        $this->sessionBackend->set($session->getIdentifier(), $sessionRecord);

The `ses_iplock` column (table `fe_sessions`) contains the string '[DISABLED]'.

Now we perform an new request.

\TYPO3\CMS\Frontend\Middleware\FrontendUserAuthenticator->process() Line 63
    \TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->start() Line 261
        \TYPO3\CMS\Core\Session\UserSessionManager->createFromGlobalCookieOrAnonymous() Line 102
            \TYPO3\CMS\Core\Session\UserSessionManager->getSessionFromSessionId()

`UserSessionManager::getSessionFromSessionId()` is able to reconstitute the user session from the persistence layer, because of the `$this->ipLocker->validateRemoteAddressAgainstSessionIpLock()` check succeed.

This is a not working example:

We assume that the `fe_sessions` table is empty or no fe cookie exists and the setting `[FE][lockIP]` is set to 0 (default configuration).

We start with the middleware layer in incoming direction which creates the user session.

\TYPO3\CMS\Frontend\Middleware\FrontendUserAuthenticator->process() Line 63
    \TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->start() Line 261
        \TYPO3\CMS\Core\Session\UserSessionManager->createFromGlobalCookieOrAnonymous() Line 102
            \TYPO3\CMS\Core\Session\UserSessionManager->createAnonymousSession() Line 113

The user session model is created by `UserSession::createNonFixated()` -> the property "ipLock" has an empty string as value within the model data.

Now we are within the application layer, let's say an extbase controller and we do something like this

$this->getFrontendUser()->setKey('ses', 'foo_1', 'bar_1');
$this->getFrontendUser()->setAndSaveSessionData('foo_2', 'bar_2');

The data will be set to the `UserSession` model.
The call to `FrontendUserAuthentication::setAndSaveSessionData()` results in the following calls

    \TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->storeSessionData() Line 339
        \TYPO3\CMS\Core\Session\UserSessionManager->fixateAnonymousSession() Line 168

`UserSessionManager::fixateAnonymousSession()` transform the `UserSession` model to an array, add the `ses_iplock` value and persist them.

        $sessionRecord = $session->toArray();
        $sessionRecord['ses_iplock'] = $sessionIpLock;
        ...
        $this->sessionBackend->set($session->getIdentifier(), $sessionRecord);

So far everything is going as expected

Now we leave the application layer and enter the middleware layer again in outgoing direction.

\TYPO3\CMS\Frontend\Middleware\FrontendUserAuthenticator->process() Line 78
    \TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->storeSessionData() Line 404
        \TYPO3\CMS\Core\Session\UserSessionManager->updateSession() Line 267

The `FrontendUserAuthentication::storeSessionData()` procedure determines that the session data was already persisted and call `UserSessionManager::updateSession()` to update them.
We remember that the user session model was created by `UserSession::createNonFixated()` -> the property "ipLock" has an empty string as value within the model data.
`UserSessionManager::updateSession()` converts the `UserSession` model to an array and persist this data, without doing anything with the 'ipLock' property.
Since `ipLock` is an empty string within the model an empty string will be persisted for the `ses_iplock` column (table `fe_sessions`).

Now we perform an new request.

\TYPO3\CMS\Frontend\Middleware\FrontendUserAuthenticator->process() Line 63
    \TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->start() Line 261
        \TYPO3\CMS\Core\Session\UserSessionManager->createFromGlobalCookieOrAnonymous() Line 102
            \TYPO3\CMS\Core\Session\UserSessionManager->getSessionFromSessionId()

`UserSessionManager::getSessionFromSessionId()` is not able to reconstitute the user session from the persistence layer, because of the `$this->ipLocker->validateRemoteAddressAgainstSessionIpLock()` check will fail (comparison of `'' === '[DISABLED]'`).
Therefore a new anonymous session will be created and the previous persisted session data leaves as garbage.

Since the usage of the `FlashMessagesViewHelper` or calls of `FrontendFormProtection::persistSessionToken()` results in `FrontendUserAuthentication::setAndSaveSessionData()` calls, this wrong behaviour can also happen if a developer does not call 'FrontendUserAuthentication::setAndSaveSessionData()' by its own.
It is enough to call `FrontendUserAuthentication::setKey()` within the application layer (a controller or something like this) and make use of a `<f:flashMessages />` ViewHelper within an template (it doesn't matter if flash messages exists or not).

A quick test can be performed with an controller like this which will reduces the issue scenarios a little bit

class SessionTestController extends ActionController
{
    public function workingAction()
    {
        dump($this->getFrontendUser()->getKey('ses', 'foo_1'));

        $this->getFrontendUser()->setKey('ses', 'foo_1', 'bar_1');
    }

    public function notworkingAction()
    {
        dump($this->getFrontendUser()->getKey('ses', 'foo_1'));

        $this->getFrontendUser()->setKey('ses', 'foo_1', 'bar_1');
        $this->getFrontendUser()->setAndSaveSessionData('foo_2', 'bar_2');
    }

    /**
     * @return FrontendUserAuthentication
     */
    protected function getFrontendUser(): FrontendUserAuthentication
    {
        return $GLOBALS['TSFE']->fe_user;
    }
}

A request to `notworkingAction()` creates a new `fe_sessions` record everytime you call it and `$this->getFrontendUser()->getKey('ses', 'foo_1')` will always return null.

The provided patch try to avoid the wrong behavior by re-set the `ses_iplock` value from the persisted data to the data to persist.
Since this is done within `UserSessionManager::updateSession()` i'm not quite sure if this is the right place because this method is used by FE and BE context.
Maybe this must be handled only within `FrontendUserAuthentication::storeSessionData()`.


Related issues 1 (0 open1 closed)

Related to TYPO3 Core - Bug #93386: SessionNotCreatedException when using storeSessionData() and user not authenticatedClosed2021-01-29

Actions
Actions #1

Updated by Gerrit Code Review about 3 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67637

Actions #2

Updated by Markus Klein about 3 years ago

  • Description updated (diff)
Actions #3

Updated by Laurent Foulloy about 3 years ago

  • Related to Bug #93386: SessionNotCreatedException when using storeSessionData() and user not authenticated added
Actions #4

Updated by Gerrit Code Review about 3 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67637

Actions #5

Updated by Gerrit Code Review about 3 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67637

Actions #6

Updated by Gerrit Code Review about 3 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67637

Actions #7

Updated by Gerrit Code Review about 3 years ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67637

Actions #8

Updated by Gerrit Code Review about 3 years ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67637

Actions #9

Updated by Gerrit Code Review about 3 years ago

Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67637

Actions #10

Updated by Gerrit Code Review about 3 years ago

Patch set 8 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67637

Actions #11

Updated by Gerrit Code Review about 3 years ago

Patch set 9 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67637

Actions #12

Updated by Gerrit Code Review almost 3 years ago

Patch set 10 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67637

Actions #13

Updated by Gerrit Code Review almost 3 years ago

Patch set 11 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67637

Actions #14

Updated by Markus Klein almost 3 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions #15

Updated by Benni Mack over 2 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF