Conversation
pabzm
changed the title
bin/gc.sh
Outdated
| $session_lifetime = $rcmail->config->get('session_lifetime', 0) * 60 * 2; | ||
|
|
||
| // Clean expired SQL sessions | ||
| if ($session_driver == 'db' && $session_lifetime) { |
There was a problem hiding this comment.
Seems session_lifetime=0 might have been a valid use-case. However, I'm not sure how useful is that, so maybe we just drop it.
There was a problem hiding this comment.
I'd prefer to do that in another, dedicated PR, to make the change publicly and not "hidden" in another PR (just in case there are people out there who would like to jump in and explain their use case and why it shouldn't be removed).
program/lib/Roundcube/session/db.php
Outdated
| base64_encode($newvars), $key); | ||
| } elseif ($ts - $this->changed > $this->lifetime / 2) { | ||
| $this->db->query("UPDATE {$this->table_name} SET `changed` = {$now}" | ||
| } elseif ($this->expires_at - $ts > $this->lifetime / 2) { |
There was a problem hiding this comment.
Considering lifetime can be quite a big interval now. Maybe forcing an update at least every hour would make sense.
On the other hand. Isn't "Remember login for up to # days" imply that the session expiration should be not auto-extended? Also, is "up to" needed here? I guess these are some general questions we need to think about.
There was a problem hiding this comment.
In my understanding the feature is "X days after the last activity", which I tried to make clear with that wording.
pabzm
force-pushed
the
extended-session-lifetime
branch
from
24f16a5 to
c263389
Compare
|
Rebased onto #9990 |
pabzm
force-pushed
the
extended-session-lifetime
branch
from
c263389 to
065e3f4
Compare
pabzm
force-pushed
the
extended-session-lifetime
branch
from
b3c084d to
1b02902
Compare
|
Rebased again onto the latest changes from #9990 |
pabzm
force-pushed
the
extended-session-lifetime
branch
2 times, most recently
from
1748a3f to
125d63c
Compare
aka Persisted Login plugin functionality in core code. Allows admins to set `$config['session_lifetime_extension_days']`, which allows users to switch on an extended session lifetime in the login form. In effect, these user sessions are valid for the configured number of days after the last activity, even across network outages, closed browsers (as long as they keep their cookies), etc.
pabzm
force-pushed
the
extended-session-lifetime
branch
from
125d63c to
ce56fd9
Compare
|
Squashed and rebased onto the latest state of the "master" branch. |
| // Warning: This reduces the effectiveness of Roundcube's session highjacking | ||
| // mitigation, since a stolen session cookie will be valid for much longer than | ||
| // without this option. | ||
| $config['session_lifetime_extension_days'] = 1; |
There was a problem hiding this comment.
In the code the default value is 0.
|
|
||
| // Ignore checkboxes, they are prettified well enough by pretty_checkbox() already. | ||
| if (input.attr('type') === 'checkbox') { | ||
| return; |
There was a problem hiding this comment.
If we return here the tr will not get "form-group row" class, which makes it looking better (adds margin). Also, it would be good to align this row content to the center, imo.
|
|
||
| if ($this->config->session_lifetime_extension_days() > 0) { | ||
| $session_lifetime_extension_hidden_field = new html_hiddenfield(['name' => '_session_lifetime_extension', 'value' => '0']); | ||
| $form_content['hidden']['session_lifetime_extension'] = $session_lifetime_extension_hidden_field->show(); |
There was a problem hiding this comment.
This hidden field isn't used, is it?
| $session_lifetime_extension_text = str_replace('#', $this->config->session_lifetime_extension_days(), $this->app->gettext('session_lifetime_extension_switch_text')); | ||
| $session_lifetime_extension_checkbox = new html_checkbox(['name' => '_session_lifetime_extension', 'id' => '_session_lifetime_extension', 'title' => $session_lifetime_extension_text]); | ||
| $form_content['inputs']['session_lifetime_extension'] = [ | ||
| 'content' => html::label(['for' => '_session_lifetime_extension'], [$session_lifetime_extension_checkbox->show(), $session_lifetime_extension_text]), |
There was a problem hiding this comment.
I see the checkbox is checked by default, it shouldn't.
2 participants
aka Persisted Login plugin functionality in core code.
Allows admins to set
$config['extended_session_lifetime_days'], which allows users to switch on an extended sessionlifetime in the login form. In effect, these user sessions are valid for the configured number of days even across
network outages, closed browsers (as long as they keep their cookies), etc.
Based on #9990
This would implement #5050