From d248ef95cd5cb97ad280fe46033b4c7ec8a04d54 Mon Sep 17 00:00:00 2001 From: Dominic Lennon Date: Tue, 10 Jan 2017 11:48:27 +0000 Subject: [PATCH 1/9] Fix for issue #5334. SessionStore now uses the correct value --- src/database/mongo.js | 71 +++++++++++++++++++++++-------------------- src/database/redis.js | 26 +++++++++++----- src/start.js | 3 ++ 3 files changed, 59 insertions(+), 41 deletions(-) diff --git a/src/database/mongo.js b/src/database/mongo.js index 2e09047da1..47f7a0596e 100644 --- a/src/database/mongo.js +++ b/src/database/mongo.js @@ -9,6 +9,7 @@ var session = require('express-session'); var _ = require('underscore'); var semver = require('semver'); + var meta = require('../meta'); var db; _.mixin(require('underscore.deep')); @@ -47,7 +48,7 @@ module.helpers.mongo = require('./mongo/helpers'); module.init = function (callback) { - callback = callback || function () {}; + callback = callback || function () { }; var mongoClient; try { mongoClient = require('mongodb').MongoClient; @@ -111,38 +112,42 @@ if (err) { return callback(err); } - createSessionStore(); + callback(); }); } else { winston.warn('You have no mongo password setup!'); - createSessionStore(); - } - - function createSessionStore() { - var sessionStore; - if (nconf.get('redis')) { - sessionStore = require('connect-redis')(session); - var rdb = require('./redis'); - rdb.client = rdb.connect(); - - module.sessionStore = new sessionStore({ - client: rdb.client, - ttl: 60 * 60 * 24 * 14 - }); - } else if (nconf.get('mongo')) { - sessionStore = require('connect-mongo')(session); - module.sessionStore = new sessionStore({ - db: db - }); - } callback(); - } + } }); }; + module.initSessionStore = function (callback) { + var meta = require('../meta'); + var sessionStore; + if (nconf.get('redis')) { + sessionStore = require('connect-redis')(session); + var rdb = require('./redis'); + rdb.client = rdb.connect(); + + var ttlDays = 1000 * 60 * 60 * 24 * (parseInt(meta.config.loginDays, 10) || 14); + + module.sessionStore = new sessionStore({ + client: rdb.client, + ttl: ttlDays + }); + } else if (nconf.get('mongo')) { + sessionStore = require('connect-mongo')(session); + module.sessionStore = new sessionStore({ + db: db + }); + } + + callback(); + }; + module.createIndices = function (callback) { function createIndex(collection, index, options, callback) { - module.client.collection(collection).createIndex(index, options, callback); + module.client.collection(collection).createIndex(index, options, callback); } if (!module.client) { @@ -152,9 +157,9 @@ winston.info('[database] Checking database indices.'); async.series([ - async.apply(createIndex, 'objects', {_key: 1, score: -1}, {background: true}), - async.apply(createIndex, 'objects', {_key: 1, value: -1}, {background: true, unique: true, sparse: true}), - async.apply(createIndex, 'objects', {expireAt: 1}, {expireAfterSeconds: 0, background: true}) + async.apply(createIndex, 'objects', { _key: 1, score: -1 }, { background: true }), + async.apply(createIndex, 'objects', { _key: 1, value: -1 }, { background: true, unique: true, sparse: true }), + async.apply(createIndex, 'objects', { expireAt: 1 }, { expireAfterSeconds: 0, background: true }) ], function (err) { if (err) { winston.error('Error creating index ' + err.message); @@ -162,16 +167,16 @@ } winston.info('[database] Checking database indices done!'); callback(); - }); + }); }; module.checkCompatibility = function (callback) { var mongoPkg = require.main.require('./node_modules/mongodb/package.json'); - + if (semver.lt(mongoPkg.version, '2.0.0')) { return callback(new Error('The `mongodb` package is out-of-date, please run `./nodebb setup` again.')); } - + callback(); }; @@ -181,10 +186,10 @@ } async.parallel({ serverStatus: function (next) { - db.command({'serverStatus': 1}, next); + db.command({ 'serverStatus': 1 }, next); }, stats: function (next) { - db.command({'dbStats': 1}, next); + db.command({ 'dbStats': 1 }, next); }, listCollections: function (next) { db.listCollections().toArray(function (err, items) { @@ -239,4 +244,4 @@ db.close(); }; -}(exports)); +} (exports)); diff --git a/src/database/redis.js b/src/database/redis.js index f1c00a2316..153d2e3058 100644 --- a/src/database/redis.js +++ b/src/database/redis.js @@ -38,7 +38,6 @@ module.init = function (callback) { try { redis = require('redis'); - connectRedis = require('connect-redis')(session); } catch (err) { winston.error('Unable to initialize Redis! Is Redis installed? Error :' + err.message); process.exit(); @@ -48,11 +47,6 @@ module.client = redisClient; - module.sessionStore = new connectRedis({ - client: redisClient, - ttl: 60 * 60 * 24 * 14 - }); - require('./redis/main')(redisClient, module); require('./redis/hash')(redisClient, module); require('./redis/sets')(redisClient, module); @@ -64,6 +58,22 @@ } }; + module.initSessionStore = function (callback) { + var meta = require('../meta'); + connectRedis = require('connect-redis')(session); + + var ttlDays = 1000 * 60 * 60 * 24 * (parseInt(meta.config.loginDays, 10) || 14); + + module.sessionStore = new connectRedis({ + client: redisClient, + ttl: ttlDays + }); + + if (typeof callback === 'function') { + callback(); + } + }; + module.connect = function (options) { var redis_socket_or_host = nconf.get('redis:host'); var cxn; @@ -97,7 +107,7 @@ var dbIdx = parseInt(nconf.get('redis:database'), 10); if (dbIdx) { cxn.select(dbIdx, function (error) { - if(error) { + if (error) { winston.error("NodeBB could not connect to your Redis database. Redis returned the following error: " + error.message); process.exit(); } @@ -156,5 +166,5 @@ module.helpers = module.helpers || {}; module.helpers.redis = require('./redis/helpers'); -}(exports)); +} (exports)); diff --git a/src/start.js b/src/start.js index f44b90643d..8c09b31fee 100644 --- a/src/start.js +++ b/src/start.js @@ -40,6 +40,9 @@ start.start = function () { next(err); }); }, + function(next) { + db.initSessionStore(next); + }, function (next) { var webserver = require('./webserver'); require('./socket.io').init(webserver.server); From ed19454ecaecee663534159e6504190e1f09cf33 Mon Sep 17 00:00:00 2001 From: Dominic Lennon Date: Wed, 11 Jan 2017 10:51:41 +0000 Subject: [PATCH 2/9] Adding more specific timing for session timings --- src/database/mongo.js | 10 +++++++--- src/database/redis.js | 20 ++++++++++++-------- src/views/admin/settings/user.tpl | 28 ++++++++++++++++++++++------ src/webserver.js | 14 ++++++++++---- 4 files changed, 51 insertions(+), 21 deletions(-) diff --git a/src/database/mongo.js b/src/database/mongo.js index 47f7a0596e..9e1c3a5a8f 100644 --- a/src/database/mongo.js +++ b/src/database/mongo.js @@ -129,11 +129,15 @@ var rdb = require('./redis'); rdb.client = rdb.connect(); - var ttlDays = 1000 * 60 * 60 * 24 * (parseInt(meta.config.loginDays, 10) || 14); + var ttlDays = 1000 * 60 * 60 * 24 * (parseInt(meta.config.loginDays, 10) || 0); + var ttlHours = 1000 * 60 * 60 * (parseInt(meta.config.loginHours, 10) || 0); + var ttlMinutes = 1000 * 60 * (parseInt(meta.config.loginMinutes, 10) || 0); + var ttlSeconds = 1000 * (parseInt(meta.config.loginSeconds, 10) || 0); + var ttlTotal = (ttlDays + ttlHours + ttlMinutes + ttlSeconds) || 1209600000; // Default to 14 days module.sessionStore = new sessionStore({ client: rdb.client, - ttl: ttlDays + ttl: ttlTotal }); } else if (nconf.get('mongo')) { sessionStore = require('connect-mongo')(session); @@ -141,7 +145,7 @@ db: db }); } - + callback(); }; diff --git a/src/database/redis.js b/src/database/redis.js index 153d2e3058..7d89054b26 100644 --- a/src/database/redis.js +++ b/src/database/redis.js @@ -59,14 +59,18 @@ }; module.initSessionStore = function (callback) { - var meta = require('../meta'); - connectRedis = require('connect-redis')(session); - - var ttlDays = 1000 * 60 * 60 * 24 * (parseInt(meta.config.loginDays, 10) || 14); - - module.sessionStore = new connectRedis({ - client: redisClient, - ttl: ttlDays + var meta = require('../meta'); + var sessionStore = require('connect-redis')(session); + + var ttlDays = 1000 * 60 * 60 * 24 * (parseInt(meta.config.loginDays, 10) || 0); + var ttlHours = 1000 * 60 * 60 * (parseInt(meta.config.loginHours, 10) || 0); + var ttlMinutes = 1000 * 60 * (parseInt(meta.config.loginMinutes, 10) || 0); + var ttlSeconds = 1000 * (parseInt(meta.config.loginSeconds, 10) || 0); + var ttlTotal = (ttlDays + ttlHours + ttlMinutes + ttlSeconds) || 1209600000; // Default to 14 days + + module.sessionStore = new sessionStore({ + client: module.client, + ttl: ttlTotal }); if (typeof callback === 'function') { diff --git a/src/views/admin/settings/user.tpl b/src/views/admin/settings/user.tpl index b10ed37da9..440b34edbb 100644 --- a/src/views/admin/settings/user.tpl +++ b/src/views/admin/settings/user.tpl @@ -20,7 +20,8 @@
- +
@@ -103,10 +104,6 @@ -
- - -
@@ -115,6 +112,25 @@
+
+
Session time +
+
+
+
+ + + + + + + + +
+
+
+
+
[[admin/settings/user:registration]]
@@ -259,4 +275,4 @@
- + \ No newline at end of file diff --git a/src/webserver.js b/src/webserver.js index fec42974c6..686451aef7 100644 --- a/src/webserver.js +++ b/src/webserver.js @@ -52,7 +52,7 @@ server.on('error', function (err) { }); module.exports.listen = function (callback) { - callback = callback || function () {}; + callback = callback || function () { }; emailer.registerApp(app); setupExpressApp(app); @@ -139,7 +139,7 @@ function setupExpressApp(app) { app.use(relativePath + '/apple-touch-icon', middleware.routeTouchIcon); - app.use(bodyParser.urlencoded({extended: true})); + app.use(bodyParser.urlencoded({ extended: true })); app.use(bodyParser.json()); app.use(cookieParser()); app.use(useragent.express()); @@ -170,8 +170,14 @@ function setupFavicon(app) { } function setupCookie() { + var ttlDays = 1000 * 60 * 60 * 24 * (parseInt(meta.config.loginDays, 10) || 0); + var ttlHours = 1000 * 60 * 60 * (parseInt(meta.config.loginHours, 10) || 0); + var ttlMinutes = 1000 * 60 * (parseInt(meta.config.loginMinutes, 10) || 0); + var ttlSeconds = 1000 * (parseInt(meta.config.loginSeconds, 10) || 0); + var ttlTotal = (ttlDays + ttlHours + ttlMinutes + ttlSeconds) || 1209600000; // Default to 14 days + var cookie = { - maxAge: 1000 * 60 * 60 * 24 * (parseInt(meta.config.loginDays, 10) || 14) + maxAge: ttlTotal }; if (nconf.get('cookieDomain') || meta.config.cookieDomain) { @@ -191,7 +197,7 @@ function setupCookie() { } function listen(callback) { - callback = callback || function () {}; + callback = callback || function () { }; var port = parseInt(nconf.get('port'), 10); var isSocket = isNaN(port); var socketPath = isSocket ? nconf.get('port') : ''; From 04e5707143bad0b0f67e217308da493a98cfe57d Mon Sep 17 00:00:00 2001 From: Dominic Lennon Date: Wed, 11 Jan 2017 11:24:27 +0000 Subject: [PATCH 3/9] Fixing lint issues --- src/start.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/start.js b/src/start.js index 8c09b31fee..d314596036 100644 --- a/src/start.js +++ b/src/start.js @@ -40,7 +40,7 @@ start.start = function () { next(err); }); }, - function(next) { + function (next) { db.initSessionStore(next); }, function (next) { From a06e39528f4c2fc60d95bc883ce3648fc7bdd7f1 Mon Sep 17 00:00:00 2001 From: Dominic Lennon Date: Mon, 16 Jan 2017 08:48:53 +0000 Subject: [PATCH 4/9] Code review change --- src/database/mongo.js | 7 ++----- src/database/redis.js | 6 ++---- src/views/admin/settings/user.tpl | 9 ++++----- src/webserver.js | 6 ++---- 4 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/database/mongo.js b/src/database/mongo.js index 9e1c3a5a8f..b91acde0cb 100644 --- a/src/database/mongo.js +++ b/src/database/mongo.js @@ -9,7 +9,6 @@ var session = require('express-session'); var _ = require('underscore'); var semver = require('semver'); - var meta = require('../meta'); var db; _.mixin(require('underscore.deep')); @@ -130,14 +129,12 @@ rdb.client = rdb.connect(); var ttlDays = 1000 * 60 * 60 * 24 * (parseInt(meta.config.loginDays, 10) || 0); - var ttlHours = 1000 * 60 * 60 * (parseInt(meta.config.loginHours, 10) || 0); - var ttlMinutes = 1000 * 60 * (parseInt(meta.config.loginMinutes, 10) || 0); var ttlSeconds = 1000 * (parseInt(meta.config.loginSeconds, 10) || 0); - var ttlTotal = (ttlDays + ttlHours + ttlMinutes + ttlSeconds) || 1209600000; // Default to 14 days + var ttl = ttlSeconds || ttlDays || 1209600000; // Default to 14 days module.sessionStore = new sessionStore({ client: rdb.client, - ttl: ttlTotal + ttl: ttl }); } else if (nconf.get('mongo')) { sessionStore = require('connect-mongo')(session); diff --git a/src/database/redis.js b/src/database/redis.js index 7d89054b26..8519b57bae 100644 --- a/src/database/redis.js +++ b/src/database/redis.js @@ -63,14 +63,12 @@ var sessionStore = require('connect-redis')(session); var ttlDays = 1000 * 60 * 60 * 24 * (parseInt(meta.config.loginDays, 10) || 0); - var ttlHours = 1000 * 60 * 60 * (parseInt(meta.config.loginHours, 10) || 0); - var ttlMinutes = 1000 * 60 * (parseInt(meta.config.loginMinutes, 10) || 0); var ttlSeconds = 1000 * (parseInt(meta.config.loginSeconds, 10) || 0); - var ttlTotal = (ttlDays + ttlHours + ttlMinutes + ttlSeconds) || 1209600000; // Default to 14 days + var ttl = ttlSeconds || ttlDays || 1209600000; // Default to 14 days module.sessionStore = new sessionStore({ client: module.client, - ttl: ttlTotal + ttl: ttl }); if (typeof callback === 'function') { diff --git a/src/views/admin/settings/user.tpl b/src/views/admin/settings/user.tpl index 440b34edbb..25de94b6d4 100644 --- a/src/views/admin/settings/user.tpl +++ b/src/views/admin/settings/user.tpl @@ -113,19 +113,18 @@
-
Session time +
+ Session time
- - - - +

Note that only one of these values will be used. If there is no seconds value we fall back to days. If + there is no days value we default to 14 days.

diff --git a/src/webserver.js b/src/webserver.js index 686451aef7..4e103d4a11 100644 --- a/src/webserver.js +++ b/src/webserver.js @@ -171,13 +171,11 @@ function setupFavicon(app) { function setupCookie() { var ttlDays = 1000 * 60 * 60 * 24 * (parseInt(meta.config.loginDays, 10) || 0); - var ttlHours = 1000 * 60 * 60 * (parseInt(meta.config.loginHours, 10) || 0); - var ttlMinutes = 1000 * 60 * (parseInt(meta.config.loginMinutes, 10) || 0); var ttlSeconds = 1000 * (parseInt(meta.config.loginSeconds, 10) || 0); - var ttlTotal = (ttlDays + ttlHours + ttlMinutes + ttlSeconds) || 1209600000; // Default to 14 days + var ttl = ttlSeconds || ttlDays || 1209600000; // Default to 14 days var cookie = { - maxAge: ttlTotal + maxAge: ttl }; if (nconf.get('cookieDomain') || meta.config.cookieDomain) { From 3507e4ce0e72a5e463ce4d0c168c0042f94ccc68 Mon Sep 17 00:00:00 2001 From: Dominic Lennon Date: Mon, 16 Jan 2017 09:33:33 +0000 Subject: [PATCH 5/9] Adding closing

--- src/views/admin/settings/user.tpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/views/admin/settings/user.tpl b/src/views/admin/settings/user.tpl index 25de94b6d4..c7373fbeb7 100644 --- a/src/views/admin/settings/user.tpl +++ b/src/views/admin/settings/user.tpl @@ -124,7 +124,7 @@

Note that only one of these values will be used. If there is no seconds value we fall back to days. If - there is no days value we default to 14 days. + there is no days value we default to 14 days.

From f0add97cf918a091692c73d668c808248df8911a Mon Sep 17 00:00:00 2001 From: Dominic Lennon Date: Mon, 16 Jan 2017 14:37:14 +0000 Subject: [PATCH 6/9] Fixing tests --- test/mocks/databasemock.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/mocks/databasemock.js b/test/mocks/databasemock.js index 44de8c7122..b73a1face2 100644 --- a/test/mocks/databasemock.js +++ b/test/mocks/databasemock.js @@ -100,6 +100,9 @@ function (next) { meta.configs.init(next); }, + function(next) { + db.initSessionStore(next); + }, function (next) { meta.dependencies.check(next); }, From 691b46d38b90904f42da74e95201df938da8e51c Mon Sep 17 00:00:00 2001 From: Dominic Lennon Date: Mon, 16 Jan 2017 14:43:34 +0000 Subject: [PATCH 7/9] Fixing lint issues --- test/mocks/databasemock.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/mocks/databasemock.js b/test/mocks/databasemock.js index b73a1face2..67d896b255 100644 --- a/test/mocks/databasemock.js +++ b/test/mocks/databasemock.js @@ -100,7 +100,7 @@ function (next) { meta.configs.init(next); }, - function(next) { + function (next) { db.initSessionStore(next); }, function (next) { From b1f0f8fa8bcc5e24bb96f827dd9410fa42ffe77d Mon Sep 17 00:00:00 2001 From: Dominic Lennon Date: Mon, 16 Jan 2017 14:52:25 +0000 Subject: [PATCH 8/9] Adding ttl value to the MongoStore options --- src/database/mongo.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/database/mongo.js b/src/database/mongo.js index b91acde0cb..c9323df23a 100644 --- a/src/database/mongo.js +++ b/src/database/mongo.js @@ -139,7 +139,8 @@ } else if (nconf.get('mongo')) { sessionStore = require('connect-mongo')(session); module.sessionStore = new sessionStore({ - db: db + db: db, + ttl: ttl }); } From 3f116e76362e36fcee234cb06860d431db4a9a5d Mon Sep 17 00:00:00 2001 From: Dominic Lennon Date: Mon, 16 Jan 2017 14:58:27 +0000 Subject: [PATCH 9/9] Fixing null ttl issue for MongoSession --- src/database/mongo.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/database/mongo.js b/src/database/mongo.js index c9323df23a..3a3331e900 100644 --- a/src/database/mongo.js +++ b/src/database/mongo.js @@ -123,15 +123,16 @@ module.initSessionStore = function (callback) { var meta = require('../meta'); var sessionStore; + + var ttlDays = 1000 * 60 * 60 * 24 * (parseInt(meta.config.loginDays, 10) || 0); + var ttlSeconds = 1000 * (parseInt(meta.config.loginSeconds, 10) || 0); + var ttl = ttlSeconds || ttlDays || 1209600000; // Default to 14 days + if (nconf.get('redis')) { sessionStore = require('connect-redis')(session); var rdb = require('./redis'); rdb.client = rdb.connect(); - var ttlDays = 1000 * 60 * 60 * 24 * (parseInt(meta.config.loginDays, 10) || 0); - var ttlSeconds = 1000 * (parseInt(meta.config.loginSeconds, 10) || 0); - var ttl = ttlSeconds || ttlDays || 1209600000; // Default to 14 days - module.sessionStore = new sessionStore({ client: rdb.client, ttl: ttl