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

[Ernesto Baschny [cron IT]]

Lars Houmark schrieb am 14.05.2010 17:36:

> Ernesto Baschny [cron IT] wrote:
> 
>> I think the session handling has nothing to do with "locking" the
>> install tool. At least at the current state.
> 
> Well, tbh I really do not see the purpose of the extra wrapper function
> then. A part of the purpose of having your own destroy handler is
> exactly to be able to do extra stuff when doing it, but let the dev
> still call the PHP core function.

No, that is not the purpose of having the extra Session-class. It is
there because we want to have a exchangeable session handling layer. How
internally the session of the installer works isn't to be exposed to the
"outside".

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.

Since we already have it, we should use it.

> So to use it the way it was done, we
> should have that extra code within the destroy() function of the session
> class. And since we now all agree that a part of destroying a session
> and make the whole thing safe, is also deleting the lock file, this is
> of course a part of the destroy of a session and should be implemented
> accordingly.

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.

> I really think you misunderstood the purpose of using the
> session_set_save_handler, and you could as well have not used it at all
> and done all the session handling code you self,

I have done that. I had implemented my own session handling before
because I didn't want to make the install tool dependant on PHP
sessions. But then someone came in "why don't we use PHP sessions" as
the TYPO3 backend doesn't work without it anyway.

I did that and it was very easy to do, because the session handling was
all in tx_install_session class. So I just had to change some places.

But then: php sessions are by default stored in /tmp, you cannot really
"expire" a session file very easily and there is no concept of
authenticated sessions. So I used the possibility to overwrite PHP's
mechanism, which is what is the current state.

> instead of having the
> extra overhead of inheriting the PHP core functions, when you want them
> wrapped by own functions anyways - without taking advantage of it!
> Reading your code, it's very limited how much code from the PHP session
> core functions you actually use.

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.

> Just my 2 cents - and nothing blocking the commit, but will for sure
> mean extra redundant code lines at some point - besides the ones already
> there.

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.

Cheers,
Ernesto