Configuration setting SESSION_NOT_SESSION_COOKIE was deprecated and r…

…emoved
cotarr · Jan 25, 2024 · 602a7a3 · 602a7a3
1 parent 52814e8
commit 602a7a3
Show file tree

Hide file tree

Showing 9 changed files with 100 additions and 146 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,17 @@ More descriptive variables names were also used.
 - In server/auth.js - Added code to the passport localStrategy 'local' callback function to add login timestamp to the session.
 - In server/session-auth.js - In the auth.check() function, refactored check for session expiration to match other changes.
 
+Deprecated and removed configuration variable SESSION_NOT_SESSION_COOKIE. 
+This was not compatible with some of the middleware libraries.
+After removal, the server will act as if SESSION_NOT_SESSION_COOKIE=true.
+
+Cookies and sessions now have 2 options:
+
+- SESSION_SET_ROLLING_COOKIE=false - The session will expire at a fixed time after user login.
+- SESSION_SET_ROLLING_COOKIE=true - The session expiration time will be updated with each request.
+
+The session expiration time is configured using SESSION_EXPIRE_SEC in the .env file.
+
 ## [v0.0.23](https://github.com/cotarr/collab-auth/releases/tag/v0.0.23) 2024-01-17
 
 This update added the capability to disable client accounts in the client database.

diff --git a/README.md b/README.md
@@ -141,7 +141,6 @@ SERVER_LOG_ROTATE_SIZE=
 SERVER_LOG_FILTER=
 
 SESSION_SET_ROLLING_COOKIE=false
-SESSION_NOT_SESSION_COOKIE=true
 SESSION_EXPIRE_SEC=3600
 SESSION_PRUNE_INTERVAL_SEC=3600
 SESSION_SECRET="A Secret That Should Be Changed"

diff --git a/debug/README.md b/debug/README.md
@@ -116,13 +116,12 @@ The following different authorization server configuration options
 are supported and should execute the tests without error
 
 
-| Configuration Option               |   |   |   |   |   |   |   |   |   |   |   |   |
-| ---------------------------------- | - | - | - | - | - | - | - | - | - | - | - | - |
-| .env DATABASE_ENABLE_POSTGRES      | F | F | F | T | T | T | F | F | F | T | T | T |
-| .env SESSION_ENABLE_POSTGRES       | F | F | F | T | T | T | F | F | F | T | T | T |
-| .env SESSION_NOT_SESSION_COOKIE    | F | T | T | F | T | T | F | T | T | F | T | T |
-| .env SESSION_SET_ROLLING_COOKIE    | F | F | T | F | F | T | F | F | T | F | F | T |
-| clients-db.json trustedClient      | F | F | F | F | F | F | T | T | T | T | T | T |
+| Configuration Option               |   |   |   |   |   |   |   |   |
+| ---------------------------------- | - | - | - | - | - | - | - | - |
+| .env DATABASE_ENABLE_POSTGRES      | F | F | T | T | F | F | T | T |
+| .env SESSION_ENABLE_POSTGRES       | F | F | T | T | F | F | T | T |
+| .env SESSION_SET_ROLLING_COOKIE    | F | T | F | T | F | T | F | T |
+| clients-db.json trustedClient      | F | F | F | F | T | T | T | T |
 
 Cookie and access_token expiration time may be tested by configuring the 
 following expiration times in seconds in the .env file.

diff --git a/debug/cookie.js b/debug/cookie.js
@@ -146,13 +146,6 @@ setup(chainObj)
   //
   // This is the second request of the series. the user's session
   // cookie from test #1 is submitted with the request.
-  // Depending on the authorization server configuration,
-  // a set-cookie header may or may not be returned.
-  //
-  // SESSION_NOT_SESSION_COOKIE   false   true   true   false
-  // SESSION_SET_ROLLING_COOKIE   false   false  true   true
-  //                              -----   -----  -----  -----
-  // Expect set-cookie header:    no      yes    yes    ERROR
   //
   // This HTML form is unique because the GET /login route
   // will create a new record in the authentication server
@@ -179,19 +172,15 @@ setup(chainObj)
 
     console.log('\tExpect: status === 200');
     assert.strictEqual(chain.responseStatus, 200);
-    if (config.session.notSessionCookie) {
-      console.log('\tExpect: set-cookie header present (SESSION_NOT_SESSION_COOKIE=true)');
-      assert.ok(((chain.parsedSetCookieHeader != null) &&
-        (chain.parsedSetCookieHeader.length > 0)));
-      console.log('\tExpect: cookie matches previous cookie from previous test');
-      assert.strictEqual(
-        chain.tempLastSessionCookie,
-        chain.parsedSetCookieHeader);
-    } else {
-      // case of session cookies do not expire
-      console.log('\tExpect: set-cookie header not present (SESSION_NOT_SESSION_COOKIE=false)');
-      assert.ok(chain.parsedSetCookieHeader === null);
-    }
+
+    console.log('\tExpect: set-cookie header present');
+    assert.ok(((chain.parsedSetCookieHeader != null) &&
+      (chain.parsedSetCookieHeader.length > 0)));
+    console.log('\tExpect: cookie matches previous cookie from previous test');
+    assert.strictEqual(
+      chain.tempLastSessionCookie,
+      chain.parsedSetCookieHeader);
+
     // Temporary variable no longer needed
     delete chain.tempLastSessionCookie;
     //
@@ -725,25 +714,20 @@ setup(chainObj)
     } else {
       logRequest(chain);
       // console.log(JSON.stringify(chain.responseRawData, null, 2));
-      if (config.session.notSessionCookie === false) {
+      const expiresDelta = chain.currentSessionCookieExpires - chain.tempLastSessionCookieExpires;
+      if (config.session.rollingCookie === true) {
         console.log('\tExpect: status === 200');
         assert.strictEqual(chain.responseStatus, 200);
+        console.log('\tExpect: set-cookie header (because rollingCookie=true)');
+        assert.ok((chain.parsedSetCookieHeader != null) &&
+          (chain.parsedSetCookieHeader.length > 0));
+        console.log('\tExpect: Cookie not changed');
+        assert.strictEqual(chain.tempLastSessionCookie, chain.currentSessionCookie);
+        console.log('\tExpect: Cookie expires value incremented by 3 seconds after time delay');
+        assert.ok((expiresDelta >= 2) && (expiresDelta <= 4));
       } else {
-        const expiresDelta = chain.currentSessionCookieExpires - chain.tempLastSessionCookieExpires;
-        if (config.session.rollingCookie === true) {
-          console.log('\tExpect: status === 200');
-          assert.strictEqual(chain.responseStatus, 200);
-          console.log('\tExpect: set-cookie header (because rollingCookie=true)');
-          assert.ok((chain.parsedSetCookieHeader != null) &&
-            (chain.parsedSetCookieHeader.length > 0));
-          console.log('\tExpect: Cookie not changed');
-          assert.strictEqual(chain.tempLastSessionCookie, chain.currentSessionCookie);
-          console.log('\tExpect: Cookie expires value incremented by 3 seconds after time delay');
-          assert.ok((expiresDelta >= 2) && (expiresDelta <= 4));
-        } else {
-          console.log('\tExpect: status === 200');
-          assert.strictEqual(chain.responseStatus, 200);
-        }
+        console.log('\tExpect: status === 200');
+        assert.strictEqual(chain.responseStatus, 200);
       }
       return Promise.resolve(chain);
     }
@@ -781,25 +765,20 @@ setup(chainObj)
       return Promise.resolve(chain);
     } else {
       logRequest(chain);
-      if (config.session.notSessionCookie === false) {
+      const expiresDelta = chain.currentSessionCookieExpires - chain.tempLastSessionCookieExpires;
+      if (config.session.rollingCookie === true) {
         console.log('\tExpect: status === 200');
         assert.strictEqual(chain.responseStatus, 200);
+        console.log('\tExpect: set-cookie header (because rollingCookie=true)');
+        assert.ok((chain.parsedSetCookieHeader != null) &&
+          (chain.parsedSetCookieHeader.length > 0));
+        console.log('\tExpect: Cookie not changed');
+        assert.strictEqual(chain.tempLastSessionCookie, chain.currentSessionCookie);
+        console.log('\tExpect: Cookie expires value incremented by 6 seconds after time delay');
+        assert.ok((expiresDelta >= 5) && (expiresDelta <= 7));
       } else {
-        const expiresDelta = chain.currentSessionCookieExpires - chain.tempLastSessionCookieExpires;
-        if (config.session.rollingCookie === true) {
-          console.log('\tExpect: status === 200');
-          assert.strictEqual(chain.responseStatus, 200);
-          console.log('\tExpect: set-cookie header (because rollingCookie=true)');
-          assert.ok((chain.parsedSetCookieHeader != null) &&
-            (chain.parsedSetCookieHeader.length > 0));
-          console.log('\tExpect: Cookie not changed');
-          assert.strictEqual(chain.tempLastSessionCookie, chain.currentSessionCookie);
-          console.log('\tExpect: Cookie expires value incremented by 6 seconds after time delay');
-          assert.ok((expiresDelta >= 5) && (expiresDelta <= 7));
-        } else {
-          console.log('\tExpect: status === 200');
-          assert.strictEqual(chain.responseStatus, 200);
-        }
+        console.log('\tExpect: status === 200');
+        assert.strictEqual(chain.responseStatus, 200);
       }
       return Promise.resolve(chain);
     }
@@ -838,25 +817,20 @@ setup(chainObj)
       return Promise.resolve(chain);
     } else {
       logRequest(chain, { ignoreErrorStatus: 401 });
-      if (config.session.notSessionCookie === false) {
+      const expiresDelta = chain.currentSessionCookieExpires - chain.tempLastSessionCookieExpires;
+      if (config.session.rollingCookie === true) {
+        console.log('\tExpect: status === 200');
+        assert.strictEqual(chain.responseStatus, 200);
+        console.log('\tExpect: set-cookie header (because rollingCookie=true)');
+        assert.ok((chain.parsedSetCookieHeader != null) &&
+          (chain.parsedSetCookieHeader.length > 0));
+        console.log('\tExpect: Cookie not changed');
+        assert.strictEqual(chain.tempLastSessionCookie, chain.currentSessionCookie);
+        console.log('\tExpect: Cookie expires value incremented by 10 seconds after time delay');
+        assert.ok((expiresDelta >= 9) && (expiresDelta <= 11));
+      } else {
         console.log('\tExpect: status === 401');
         assert.strictEqual(chain.responseStatus, 401);
-      } else {
-        const expiresDelta = chain.currentSessionCookieExpires - chain.tempLastSessionCookieExpires;
-        if (config.session.rollingCookie === true) {
-          console.log('\tExpect: status === 200');
-          assert.strictEqual(chain.responseStatus, 200);
-          console.log('\tExpect: set-cookie header (because rollingCookie=true)');
-          assert.ok((chain.parsedSetCookieHeader != null) &&
-            (chain.parsedSetCookieHeader.length > 0));
-          console.log('\tExpect: Cookie not changed');
-          assert.strictEqual(chain.tempLastSessionCookie, chain.currentSessionCookie);
-          console.log('\tExpect: Cookie expires value incremented by 10 seconds after time delay');
-          assert.ok((expiresDelta >= 9) && (expiresDelta <= 11));
-        } else {
-          console.log('\tExpect: status === 401');
-          assert.strictEqual(chain.responseStatus, 401);
-        }
       }
       return Promise.resolve(chain);
     }
@@ -1227,41 +1201,34 @@ setup(chainObj)
       return Promise.resolve(chain);
     } else {
       logRequest(chain);
-      if (config.session.notSessionCookie === false) {
+      if (config.session.rollingCookie === true) {
+        if (testEnv.trustedClient) {
+          console.log('\tExpect: status === 302');
+          assert.strictEqual(chain.responseStatus, 302);
+          console.log('\tExpect: redirect location match redirectURI');
+          assert.strictEqual(
+            testEnv.redirectURI,
+            chain.parsedLocationHeader.split('?')[0]);
+        } else {
+          console.log('\tExpect: status === 200');
+          assert.strictEqual(chain.responseStatus, 200);
+          console.log('\tExpect: body contains "<title>Resource Decision</title>"');
+          assert.ok(chain.responseRawData.indexOf('<title>Resource Decision</title>') >= 0);
+        }
+        console.log('\tExpect: set-cookie header (because rollingCookie=true)');
+        assert.ok((chain.parsedSetCookieHeader != null) &&
+        (chain.parsedSetCookieHeader.length > 0));
+        console.log('\tExpect: Cookie not changed');
+        assert.strictEqual(chain.tempLastSessionCookie, chain.currentSessionCookie);
+        const expiresDelta =
+          chain.currentSessionCookieExpires - chain.tempLastSessionCookieExpires;
+        console.log('\tExpect: Cookie expires value incremented by 10 seconds after time delay');
+        assert.ok((expiresDelta >= 9) && (expiresDelta <= 11));
+      } else {
         console.log('\tExpect: status === 302');
         assert.strictEqual(chain.responseStatus, 302);
         console.log('\tExpect: Location header redirects to GET /login');
         assert.strictEqual(chain.parsedLocationHeader, '/login');
-      } else {
-        if (config.session.rollingCookie === true) {
-          if (testEnv.trustedClient) {
-            console.log('\tExpect: status === 302');
-            assert.strictEqual(chain.responseStatus, 302);
-            console.log('\tExpect: redirect location match redirectURI');
-            assert.strictEqual(
-              testEnv.redirectURI,
-              chain.parsedLocationHeader.split('?')[0]);
-          } else {
-            console.log('\tExpect: status === 200');
-            assert.strictEqual(chain.responseStatus, 200);
-            console.log('\tExpect: body contains "<title>Resource Decision</title>"');
-            assert.ok(chain.responseRawData.indexOf('<title>Resource Decision</title>') >= 0);
-          }
-          console.log('\tExpect: set-cookie header (because rollingCookie=true)');
-          assert.ok((chain.parsedSetCookieHeader != null) &&
-          (chain.parsedSetCookieHeader.length > 0));
-          console.log('\tExpect: Cookie not changed');
-          assert.strictEqual(chain.tempLastSessionCookie, chain.currentSessionCookie);
-          const expiresDelta =
-            chain.currentSessionCookieExpires - chain.tempLastSessionCookieExpires;
-          console.log('\tExpect: Cookie expires value incremented by 10 seconds after time delay');
-          assert.ok((expiresDelta >= 9) && (expiresDelta <= 11));
-        } else {
-          console.log('\tExpect: status === 302');
-          assert.strictEqual(chain.responseStatus, 302);
-          console.log('\tExpect: Location header redirects to GET /login');
-          assert.strictEqual(chain.parsedLocationHeader, '/login');
-        }
       }
       return Promise.resolve(chain);
     }

diff --git a/debug/modules/import-config.js b/debug/modules/import-config.js
@@ -12,12 +12,6 @@ const fs = require('fs');
 const config = require('../../server/config/index.js');
 // console.log('config', JSON.stringify(config, null, 2));
 
-if ((config.session.rollingCookie) && (!config.session.notSessionCookie)) {
-  console.log('Configuration error');
-  console.log('rollingCookie=true not compatible with notSessionCookie=false');
-  process.exit(1);
-}
-
 let clients = [];
 try {
   clients = JSON.parse(fs.readFileSync('./clients-db.json', 'utf8'));

diff --git a/docs/deployment.html b/docs/deployment.html
@@ -576,20 +576,19 @@
         <div class="section-title">Cookie/Session Configuration</div>
         <p>
           By default, browser cookies for the collab-auth authorization server
-          are created as session cookies. The cookie will not be issued an 
-          expiration date, but remain valid until the browser is closed.
+          are set to expire at a fixed time after user login. The permitted age
+          of the session is defined by providing a configuration value 
+          for SESSION_EXPIRE_SEC.
         </p>
         <p>
-          Setting SESSION_NOT_SESSION_COOKIE=true will instruct the server to 
-          add a cookie expiration time as specified in SESSION_EXPIRE_SEC.
-          Optionally, this can become a rolling cookie by setting 
-          SESSION_SET_ROLLING_COOKIE=true so a new cookie expiration date 
-          is issued with each browser interaction.
+          Setting SESSION_SET_ROLLING_COOKIE=true will instruct the server to 
+          reset the cookie expiration with each browser request.
+          In this case, the session will expire after a time period where no
+          requests are received in the time period specified by SESSION_EXPIRE_SEC.
         </p>
 
 <pre class="pre-div">
 SESSION_SET_ROLLING_COOKIE=false
-SESSION_NOT_SESSION_COOKIE=false
 SESSION_EXPIRE_SEC=3600
 SESSION_PRUNE_INTERVAL_SEC=3600
 SESSION_SECRET="A Secret That Should Be Changed"

diff --git a/docs/env.html b/docs/env.html
@@ -78,7 +78,6 @@
 
 
             <tr><td>SESSION_SET_ROLLING_COOKIE</td><td>false</td><td></td></tr>
-            <tr><td>SESSION_NOT_SESSION_COOKIE</td><td>false</td><td></td></tr>
             <tr><td>SESSION_EXPIRE_SEC</td><td>3600</td><td>(1 hour)</td></tr>
             <tr><td>SESSION_PRUNE_INTERVAL_SEC</td><td>3600</td><td>(1 hour)</td></tr>
             <tr><td>SESSION_SECRET</td><td>A Secret That Should Be Changed</td><td></td></tr>

diff --git a/server/app.js b/server/app.js
@@ -203,36 +203,27 @@ const sessionOptions = {
   secret: config.session.secret,
   cookie: {
     path: '/',
-    maxAge: null,
+    maxAge: config.session.maxAge,
     secure: (config.server.tls), // When TLS enabled, require secure cookies
     httpOnly: true,
     sameSite: 'Lax'
   }
 };
-// Session cookie clears when browser is closed.
-if (config.session.notSessionCookie) {
-  // express-session takes cookie.maxAge in milliseconds
-  sessionOptions.cookie.maxAge = config.session.maxAge;
-}
 
 if (config.session.enablePgSessionStore) {
   // SQL queries
   // List:       SELECT sid, expire FROM session;
   // Clear all:  DELETE FROM session;
   console.log('Using PostgresSQL connect-pg-simple for session storage');
-  // disable touch for fixed expiration cookie configuration
-  const disableTouch = ((config.session.notSessionCookie === true) &&
-    (config.session.rollingCookie === false));
-
   const pgPool = require('./db/pg-pool');
   const PgSessionStore = require('connect-pg-simple')(session);
   sessionOptions.store = new PgSessionStore({
     pool: pgPool,
     // connect-pg-simple ttl is in seconds
     ttl: config.session.ttl,
-    tableName: 'session',
     // disable touch for fixed expiration cookie configuration
-    disableTouch: disableTouch,
+    disableTouch: (config.session.rollingCookie === false),
+    tableName: 'session',
     // Connect-pg-simple takes prune time in seconds
     pruneSessionInterval: config.session.pruneInterval
   });

diff --git a/server/config/index.js b/server/config/index.js
@@ -38,20 +38,15 @@ exports.server = {
   logFilter: process.env.SERVER_LOG_FILTER || ''
 };
 
-const rollingCookie = (process.env.SESSION_SET_ROLLING_COOKIE === 'true') || false;
-let notSessionCookie = (process.env.SESSION_NOT_SESSION_COOKIE === 'true') || false;
-if ((rollingCookie === true) && (notSessionCookie === false)) {
-  notSessionCookie = true;
-  console.log('--------------------------------------------------------------------------');
-  console.log('Config Warning: Session cookies do not have an expiration.');
-  console.log('Due to configuration setting SESSION_SET_ROLLING_COOKIE=true, the setting');
-  console.log('SESSION_NOT_SESSION_COOKIE has been changed to true.');
-  console.log('--------------------------------------------------------------------------');
+if (Object.hasOwn(process.env, 'SESSION_NOT_SESSION_COOKIE')) {
+  console.log('\n------------------------------------------------------');
+  console.log('Please check the configuration environment variables.');
+  console.log('SESSION_NOT_SESSION_COOKIE was deprecated in v0.0.24');
+  console.log('------------------------------------------------------\n');
 }
 
 exports.session = {
-  rollingCookie: rollingCookie,
-  notSessionCookie: notSessionCookie,
+  rollingCookie: (process.env.SESSION_SET_ROLLING_COOKIE === 'true') || false,
   maxAge: parseInt(process.env.SESSION_EXPIRE_SEC || '3600') * 1000,
   ttl: parseInt(process.env.SESSION_EXPIRE_SEC || '3600'),
   pruneInterval: parseInt(process.env.SESSION_PRUNE_INTERVAL_SEC || '3600'),