[TYPO3-core] RFC #14367: Install Tool does not provide a logout possibility

[Lars Houmark]

Hi Ernesto,

Ernesto Baschny [cron IT] wrote:
> That we currently use PHP sessions and handlers is just the current
> *internal* implementation detail. So either we keep it that way
> (internal) and let the session handling class be enchangeable (which I
> think is nice, because it for example allows better unit testing). Or
> don't use it at all. A "mix up" isn't really nice.

I understand, and I can go with the extra wrapper function in order to 
have the extra internal layer, which would make it easier to change the 
session handling without having to replace the calls many places, and at 
the same time use the same name-space for working with the session.

> The lock file has nothing to do with the session. We agree that logging
> out should clear the lock file, but I don't agree that destroying a
> session should "know" about this aspect that is unrelated to session
> management.

But I don't agree that the lock file has nothing to do with the logout 
routine. Think about it this way. Any logout button/link no matter where 
will be placed will try to remove that file and redirect when done. Any 
logout done by triggering some incident (I mean something potential 
evil) will do the same. I cannot think of a situation where you would 
want to logout, meaning do session_destroy but not redirect. And since 
we have agreed, that the lock file should also be deleted, this will 
also be done in any of the above mentioned situations.

So why no centralize all that code? Otherwise you are speaking against 
your own request to centralize the session functions and having it easy 
changeable. We have the exact same situation several other places, you 
need to look no further than the lock file itself. The code for 
create/delete is not centralized and therefore *very* redundant and this 
is giving some problems/discussions now. The same goes for sorting of 
extension versions in the EM. This should also be centralized, and 
because it was not, and still isn't, the same problem came back over 3 bugs.

> True, but that is because PHP session handling is not secure by default.
> And we wanted the install tool more secure. After all the session
> handling was added in a security release and not as a regular feature.

I understand and appreciate your work :). I think the implementation in 
general was very nice, but I cannot understand, why you think the code 
should not be centralized. I feels like you are separating *too* much.

> Hope you understood the motivation behind the session class. While it
> might not have been necessary, we have it, and I think it provides a
> clean separation of tasks.

But maybe too clean? Afterall the session class is called 
tx_install_session so it's not that this class will be used for anything 
else, so then let's at least keep the redundant code to a minimum.

-- 
Lars Houmark