Use precise session lifetime for garbage collector

[Github-Citizen]

program/lib/Roundcube/rcube.php

458:  $lifetime      = $this->config->get('session_lifetime', 0) * 60;
.
.
475:  ini_set('session.gc_maxlifetime', $lifetime * 2);

Why is $lifetime being doubled? If you set $config['session_lifetime'] = 7*24*60; intending to keep sessions for one week the garbage collection is being told to wait for two weeks before clearing old sessions?

Is there a technical reason for this or am i misunderstanding what is happening in the code?

[alecpl]

I'm not sure, but we didn't consider such long lifetime of the session. Also, we used to not invalidate outdated sessions, so you could use it until it has been removed from the database. This is not the case anymore.

I think the purpose was to not invalidate the session too soon in case of some temporary connection issues on the user side (which would case the keep-alive requests to fail). I'm thinking that nowadays we could remove that multiplier and see how that works.

[Github-Citizen]

I have put together a stay logged in plugin that adds a toggle switch on the login page. By default, the sessions last 10 mins server side, and the cookie last only for the browser session. The plugin modifies the cookie expiration date to be how many number of days configured in the plugin.

Only issue is that database garbage collection is based on $config['session_lifetime']. For the plugin to work, for users to have the option to stay logged in for a week, then $config['session_lifetime'] also needs to be set = 7*24*60 which as the code stands means gc wouldn't happen for two weeks. I don't know if that becomes a problem for high volume high traffic servers.

I would like the plugin to become part roundcubes core plugins. I have seen lots of people desire this feature and always wondered why a stay-logged-in feature wasn't already built in.

The plugin needs more language files contributed to, i made a small handful of them using google translate and have no idea how accurate the translations are.

Are you interested? Can i do a PR adding the plugin to roundcube? It works as is, as long as you set $config['session_lifetime'] to match. However i think it could be improved if some changes were also made to roundcube core code.

For example, plugins hook config_get should work in the session_init() allowing a stay logged in plugin to make sure those sessions aren't being garbage collected.

Ideally i think it would be better if RC changed how it manages sessions. Instead of time stamping last activity as time(), then doing gc based on time_stamp + $config['session_lifetime'] < time(), i think it would be better to save the expire time. Every page load when the session is updated save the time stamp as time() + $config['session_lifetime'] then for gc you only have to do time_stamp < time().

Not only would this guarantee stay-logged-in sessions don't accidentally get gc, it also allows for shorter gc on sessions who have not chosen to stay logged in, those sessions can still get gc in 10 mins.

Please let me know if you are interested and how interested. Just add the plugin and leave RC code as is, or also making changes to RC code.

Thank you.

[Github-Citizen]

In relation to sessions and gc, i would like to suggest renaming $config['session_lifetime'] to something more descriptive of what it actually does like $config['gc_maxlifetime']. Or explain it better to users in the defaults file comments.

When i was new to RC i assumed it meant browser sessions, i thought if i set the value to 7 days then users would stay logged in for 7 days, because session lifetime = 7 days. But in reality, sessions are hard coded to only last until the browser is closed and session_lifetime only controls when gc happens which can happen even before the user closes their browser, if they are sitting on a page that doesn't check for new mail.

[Github-Citizen]

[alecpl]

Create a PR and we'll consider that.

[Github-Citizen]

Created PR #8689 for the plugin.

[alecpl]

Done.