[Tickets #12136] Re: Session Timeout not enforced

[noreply at bugs.horde.org]

DO NOT REPLY TO THIS MESSAGE. THIS EMAIL ADDRESS IS NOT MONITORED.

Ticket URL: http://bugs.horde.org/ticket/12136
------------------------------------------------------------------------------
  Ticket             | 12136
  Updated By         | Jan Schneider <jan at horde.org>
  Summary            | Session Timeout not enforced
  Queue              | Horde Framework Packages
  Version            | Git master
  Type               | Bug
  State              | Feedback
  Priority           | 2. Medium
  Milestone          |
  Patch              |
  Owners             |
------------------------------------------------------------------------------


Jan Schneider <jan at horde.org> (2013-05-02 09:48) wrote:

> So may I contribute my 2c here? To be honest I didn't look at the  
> code, I'd ratjer like to formulate my expectations to how I would  
> love to see  session expiry working.
>
> Sidenote: For the time being it's irrelevant to me whether we  
> distinguish between real-user activity or mailbox polling. I think  
> this is a seperate issue. More on that later.
>
> So I'd like to have horde this way:
>
> 1. cookie_timeout: When I close the browser the cookie should expire  
> (means cookie_timeout should be 0). This means that when I'm using a  
> normal browser that respects this setting I should not anymore be  
> logged in, when I access the webmail the next time I start the  
> browser. This is quite a crucial setting as it is the most efficient  
> way to address forgotten logouts on shared computers. If I set  
> cookie_timeout to something bigger thant 0, my cookie should live  
> that long and also survive browser restarts and so on.
>
> 2. max_session_lifetime: This should be the maximum time I want  
> users to be able to be logged in. Users are forced to log in again  
> after that time. No matter how much activity happens within their  
> session. Main reasons for that is to enforce users to run their  
> login tasks.
>
> 3.  session_timeout: This should be the maximum time a session  
> should live without any further requests. This means for example, if  
> this is set to 30min. and I close my webmail tab (but not the whole  
> browser) and try access webmail after 31 minutes again I need to log  
> in again.
>
> As I mentioned above for simplicity I do not want to distinguish  
> currently between automated/background polls and real user  
> interaction. This means that for point 3, if I leave my webmail tab  
> open, I will be logged in also after 35 minutes. But still if I  
> close my tab and so no polling happens I am logged out. Which is  
> something that you, Michael, completely miss in comment #8.
>
> Do we agree on these user stories or did I miss something, get  
> something wrong?

Almost. We have different views on whether in story 3, the session  
must timeout 31 minutes after the last activity, or "some time" after  
30 minutes of inactivity. We think the latter is sufficient too.

> Implementation brainstorming (as I said I have no idea how it is  
> done currently):
>
> Point 1 is a setting the timeout value of the cookie. That's not a  
> big deal: copy setting value to cookie value.

Correct, and this is what has been done since ever.

> Point 2: If I record the time the session got created and compare  
> that value with max_session_lifetime each time I validate the  
> session, I will perfectly know when I have to not accept the session  
> anymore.

Correct and already implemented this way.

> Point 3: If I record the time of the current request in the session  
> and on the next request I check if that value is more than  
> session_timeout ago and hence do not accept the session as valid, if  
> it's beyond that time, I also have user story 3 implemented. Right?

Yes, but so is it if using PHP's session garbage collection.

> So point 1 is unrelated to point 2 & 3 and hence an independent  
> setting. Implementing point 2 means to store the creation time and  
> compare it on each request. Point 3 is to store on each request the  
> current time and compare the previous value to the current time. I  
> don't expect this to be that expensive as I expect horde to write  
> much more things on each request and session storage is usually  
> something that should be written quickly.

I can't tell about the cost of storing the session if not necessary,  
but Michael has put a lot of work into only writing to the session if  
really necessary, so I trust him that there are very good reason to  
not write to the session on every request.
As a compromise I could imagine only doing this on explicit demand by  
the administrator, per configuration, at the expense of performance  
penalty. But I don't really see this being necessary.

> The first answer in  
> http://stackoverflow.com/questions/1236374/session-timeouts-in-php-best-practices provides an in my opinion a pretty good explanation how something like that should look like and why you can't count on gc_maxlifetime. The only thing in my opinion gc is for, is to garbage collect sessions that did not get closed by the user itself (read user logged out).  As mentioned before the gc probability can be quite low and so sessions can live much longer than expected. And well if longer would only be 5 minutes it would probably not be a that big issue, but if it's 2 to 3 times longer than what is set in session_timeout it's simply not working anymore as one would expect. Also there is no way to make that more controllable, it's purely  
> random.

It's not purely random, but it's the administrator's responsibility to  
tweak the PHP GC settings so that sessions will be cleaned up in a  
timeframe acceptable for his security requirements and, more  
important, matching his site's traffic.
I'm with Michael that it's perfectly fine to not being able to set the  
session timeout on the minute, but rely on PHP's session garbage  
collection which exact purpose it to kill outdated sessions. If it  
doesn't do this quick enough for you, and you accept the additional  
performance penalty (which you do, see above), set a higher probability.

> I didn't yet get why it should be a bad idea to write to the session  
> on each request. The only thing I can think of is that the write  
> operation is expensive. But I think horde is probably doing much  
> more expensive operations during a request than updating a session.  
> But I'm not an expert.

I'll leave this to Michael to explain why he has such a strong opinion  
about this.