From 023e58cfe627dc39554dc7f354fb1f4c09c08b39 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Wed, 22 Dec 2021 23:42:19 -0500
Subject: [PATCH] express-session: Set a finite cookie lifetime

---
 CHANGELOG.md               |  4 ++++
 settings.json.template     | 22 +++++++++++++++++++++-
 src/node/hooks/express.js  |  1 +
 src/node/utils/Settings.js |  1 +
 4 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 58daed814..5f78af33d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@
   * `express_sid` cookies and `sessionstorage:*` database records are no longer
     created unless `requireAuthentication` is `true` (or a plugin causes them to
     be created).
+  * Login sessions now have a finite lifetime by default (10 days).
   * `sessionstorage:*` database records are automatically deleted when the login
     session expires (with some exceptions that will be fixed in the future).
   * Requests for static content (e.g., `/robots.txt`) and special pages (e.g.,
@@ -45,6 +46,9 @@
 
 ### Compatibility changes
 
+* The default login session expiration (applicable if `requireAuthentication` is
+  `true`) changed from never to 10 days.
+
 #### For plugin authors
 
 * The `client` context property for the `handleMessageSecurity` and
diff --git a/settings.json.template b/settings.json.template
index b802248f1..54372c59d 100644
--- a/settings.json.template
+++ b/settings.json.template
@@ -375,7 +375,27 @@
      * significant usability drawbacks vs. "Lax". See
      * https://stackoverflow.com/q/41841880 for discussion.
      */
-    "sameSite": "Lax"
+    "sameSite": "Lax",
+
+    /*
+     * How long (in milliseconds) a session lasts before the user is required to
+     * log in again. (The express_sid cookie is set to expire at time now +
+     * sessionLifetime when first created.) If requireAuthentication is false
+     * then this value does not really matter.
+     *
+     * The "best" value depends on your users' usage patterns and the amount of
+     * convenience you desire. A long lifetime is more convenient (users won't
+     * have to log back in as often) but has some drawbacks:
+     *   - It increases the amount of state kept in the database.
+     *   - It might weaken security somewhat: Once a user has accessed a pad,
+     *     the user can continue to use the pad until the session expires.
+     *
+     * Session lifetime can be set to infinity (not recommended) by setting this
+     * to null or 0. Note that if the session does not expire, most browsers
+     * will delete the cookie when the browser exits, but a session record is
+     * kept in the database forever.
+     */
+    "sessionLifetime": 864000000 // = 10d * 24h/d * 60m/h * 60s/m * 1000ms/s
   },
 
   /*
diff --git a/src/node/hooks/express.js b/src/node/hooks/express.js
index 18f026463..3e231bdde 100644
--- a/src/node/hooks/express.js
+++ b/src/node/hooks/express.js
@@ -186,6 +186,7 @@ exports.restartServer = async () => {
     // cleaner :)
     name: 'express_sid',
     cookie: {
+      maxAge: settings.cookie.sessionLifetime || null, // Convert 0 to null.
       sameSite: settings.cookie.sameSite,
 
       // The automatic express-session mechanism for determining if the application is being served
diff --git a/src/node/utils/Settings.js b/src/node/utils/Settings.js
index 42d29edb8..cd4e5c8fe 100644
--- a/src/node/utils/Settings.js
+++ b/src/node/utils/Settings.js
@@ -322,6 +322,7 @@ exports.cookie = {
    * https://stackoverflow.com/q/41841880 for discussion.
    */
   sameSite: 'Lax',
+  sessionLifetime: 10 * 24 * 60 * 60 * 1000,
 };
 
 /*