From 602a7a34826e770fe89fda7870b4d52e1130fa59 Mon Sep 17 00:00:00 2001 From: Dave Bolenbaugh Date: Thu, 25 Jan 2024 07:16:34 -0600 Subject: [PATCH] Configuration setting SESSION_NOT_SESSION_COOKIE was deprecated and removed --- CHANGELOG.md | 11 +++ README.md | 1 - debug/README.md | 13 ++- debug/cookie.js | 167 +++++++++++++-------------------- debug/modules/import-config.js | 6 -- docs/deployment.html | 15 ++- docs/env.html | 1 - server/app.js | 15 +-- server/config/index.js | 17 ++-- 9 files changed, 100 insertions(+), 146 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f09d2d9..f497bcc 100644 --- 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 index 2527a00..e2a856c 100644 --- 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 index a58d6cf..cbf11e6 100644 --- 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 index 6565ef6..a6c2fb6 100644 --- 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 "Resource Decision"'); + assert.ok(chain.responseRawData.indexOf('Resource Decision') >= 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 "Resource Decision"'); - assert.ok(chain.responseRawData.indexOf('Resource Decision') >= 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 index 43b6db5..4c1e323 100644 --- 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 index da4dd3a..6cdc315 100644 --- a/docs/deployment.html +++ b/docs/deployment.html @@ -576,20 +576,19 @@
Cookie/Session Configuration

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.

- 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. 

 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
index 00e788a..76650ab 100644
--- a/docs/env.html
+++ b/docs/env.html
@@ -78,7 +78,6 @@
 
             
             SESSION_SET_ROLLING_COOKIEfalse
-            SESSION_NOT_SESSION_COOKIEfalse
             SESSION_EXPIRE_SEC3600(1 hour)
             SESSION_PRUNE_INTERVAL_SEC3600(1 hour)
             SESSION_SECRETA Secret That Should Be Changed
diff --git a/server/app.js b/server/app.js
index 8b24ba5..755b08e 100644
--- 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
index 0c2bdfc..eb5a6d2 100644
--- 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'),