GDPR8 (#1416)

* Use very short session before successful auth. Session "bleeding" briefly mentioned at #1411 . This is "expanded" after successful auth.
* Output `originalMaxAge` for sync check in *express-session* via MongoDB
* Don't easily expose improper/expired callbacks. Part of #37
* Remove some currently unneeded `return` statements already captured by block braces

Related to #604 #1201 #1202 and #1393 

Auto-merge

Author: Martii

app.js

@@ -318,7 +318,7 @@ app.use(session({
   saveUninitialized: false,
   unset: 'destroy',
   cookie: {
-    maxAge: 6 * 60 * 60 * 1000 // hours in ms
+    maxAge: 5 * 60 * 1000 // minutes in ms NOTE: Expanded after successful auth
   },
   rolling: true,
   secret: sessionSecret,

controllers/admin.js

@@ -397,6 +397,7 @@ exports.adminSessionActiveView = function (aReq, aRes, aNext) {
         if (data && data.user) {
           options.session.push({
             _id: aElement._id,
+            originalMaxAge: data.cookie.originalMaxAge,
             expires: data.cookie.expires,
             name: data.user.name
           });

controllers/auth.js

@@ -27,6 +27,7 @@ var strategyInstances = require('../libs/passportLoader').strategyInstances;
 var verifyPassport = require('../libs/passportVerify').verify;
 var cleanFilename = require('../libs/helpers').cleanFilename;
 var addSession = require('../libs/modifySessions').add;
+var expandSession = require('../libs/modifySessions').expand;
 
 //--- Configuration inclusions
 var allStrategies = require('./strategies.json');
@@ -187,9 +188,9 @@ exports.callback = function (aReq, aRes, aNext) {
   var strategyInstance = null;
   var doneUri = aReq.session.user ? '/user/preferences' : '/';
 
-  // The callback was called improperly
+  // The callback was called improperly or sesssion expired
   if (!strategy || !username) {
-    aNext();
+    aRes.redirect(doneUri + (doneUri === '/' ? 'login' : ''));
     return;
   }
 
@@ -295,16 +296,16 @@ exports.callback = function (aReq, aRes, aNext) {
         if (newstrategy && newstrategy !== strategy) {
           // Allow a user to link to another account
           aRes.redirect('/auth/' + newstrategy); // NOTE: Watchpoint... careful with encoding
-          return;
         } else {
           // Delete the username that was temporarily stored
           delete aReq.session.username;
           delete aReq.session.newstrategy;
           doneUri = aReq.session.redirectTo;
           delete aReq.session.redirectTo;
 
-          aRes.redirect(doneUri);
-          return;
+          expandSession(aReq, aUser, function () {
+            aRes.redirect(doneUri);
+          });
         }
       });
     });

libs/modifySessions.js

@@ -5,6 +5,9 @@ var isPro = require('../libs/debug').isPro;
 var isDev = require('../libs/debug').isDev;
 var isDbg = require('../libs/debug').isDbg;
 
+//--- Library inclusions
+var moment = require('moment');
+
 //
 // This library allows for the modifications of user sessions
 var async = require('async');
@@ -62,6 +65,22 @@ exports.add = function (aReq, aUser, aCallback) {
   }
 };
 
+// Expand a single session
+exports.expand = function (aReq, aUser, aCallback) {
+  var expiry = null;
+
+  if (!aUser) {
+    aCallback('No User');
+    return;
+  }
+
+  // NOTE: Expanded minus initial. Keep initial in sync with app.js
+  expiry = moment(aReq.session.cookie.expires).add(6, 'h').subtract(5, 'm');
+
+  aReq.session.cookie.expires = expiry.toDate();
+  aReq.session.save(aCallback);
+};
+
 // Extend a single session
 exports.extend = function (aReq, aUser, aCallback) {
   if (!aUser) {
@@ -75,7 +94,7 @@ exports.extend = function (aReq, aUser, aCallback) {
   }
 
   // NOTE: Currently allow on any session with
-  //   no additional User restrictions yet
+  //   no additional User restrictions yet...
 
   aReq.session.cookie.expires = false;
   aReq.session.save(aCallback);