New plugin: Persisted login

[pabzm]

This plugin adds a toggle switch into the login form of Roundcubemail's "elastic" skin, that makes the session live for a configured number of days.

Adapted from #8689

Hat tip and thanks to @Github-Citizen

[pabzm]

The E2E-tests are green locally. I'll fix them in the CI next week.

[pabzm]

Thank you for the review! I changed the styling as requested and added support for derivate skins.

[pabzm]

The PHP8.5 deprecation warning in the Unit tests is fixed via DASPRiD/Enum#18

[pabzm]

I fixed the Browser tests by changing the plugin behaviour a little bit: It now uses a default of 7 days, and a minimum of 1 day. Setting persisted_login_days to 0 doesn't work anymore to disable the plugin. Instead disabling should be done via config.inc.php. Additionally the config file is included as config.inc.php.dist, as it is in other plugins.
I think it's even more logical this way.

[alecpl]

A composer.json file is missing.

ps. I don't like one thing about this plugin. It changes session lifetime for everyone. Which means all session records are garbage collected after X days. It probably should be fixed with a "expires at" column in the session table. It's not a stopper, I suppose.

[Github-Citizen]

ps. I don't like one thing about this plugin. It changes session lifetime for everyone. Which means all session records are garbage collected after X days.

Yes, but isn't that a necessary evil? If you don't extend the 'session_lifetime' then GC will remove the server side session even though the browser cookie is saved in the browser for x number of days.

It probably should be fixed with a "expires at" column in the session table. It's not a stopper, I suppose.

But doesn't that require a fundamental change to the core of roundcube and outside of the scope of a plugin?

[pabzm]

I also don't really like to rely on the clients to respect the expiry-timestamp of the cookie, either, but decided to go with it for now.

But as we agree on that part maybe we should do it properly and implement this features in core code, using an expire_at database column and memcache/redis key. Because having it in a plugin while changing core code explicitly for it feels wrong to me.

@alecpl Do you agree?

[pabzm]

And as far as I see we don't need the changed column anymore, and should remove it – or rather rename it, instead of adding and removing.

[alecpl]

And as far as I see we don't need the changed column anymore, and should remove it – or rather rename it, instead of adding and removing.

What do you mean we don't need it? It is a very important column. We'd need both changed and expires_at.

[pabzm]

From the code in rcube_session_db I figured that the purpose of changed is to mark the latest use of a session, so expired sessions can be filtered and removed. That could be done using expires_at instead, I think. What other purpose is there to changed?

[pabzm]

In c69f02f (branch extended-session-lifetime) I built a proof of concept of the feature in core code, which works without changed. It's not polished, but it works. Could you let me know which problems you see or would expect?

[alecpl]

You might be right that one column would suffice. However, "time since the last change" is not the same as "time to expiry"

It would be easier to review if I could see the changed -> expires_at change separately from the persistent login.

[pabzm]

Here's a PR with only the change to the session expiry without the new feature: #9990

"time since the last change" is not the same as "time to expiry"

You're right, but as far as I can tell we use the timestamp only for expiry, and not for anything else.