From 69806662e63b8e7215c1946e123522f9a28dc714 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Thu, 5 Jan 2023 14:10:02 -0500 Subject: [PATCH] Session Timeout if "Remember Me" is not checked (#11125) * fix: convert loginDays and loginSeconds to number inputs * feat: configurable session timeout for when "Remember Me" is not checked closes #11124 * test: addition tests to check loginDays and sessionDuration settings * test: also test loginSeconds override --- install/data/defaults.json | 1 + .../language/en-GB/admin/settings/user.json | 2 + src/controllers/authentication.js | 5 +- src/views/admin/settings/user.tpl | 11 +- test/authentication.js | 124 +++++++++++++++--- 5 files changed, 123 insertions(+), 20 deletions(-) diff --git a/install/data/defaults.json b/install/data/defaults.json index e7bf3f65f4..eb3ccdf19a 100644 --- a/install/data/defaults.json +++ b/install/data/defaults.json @@ -4,6 +4,7 @@ "defaultLang": "en-GB", "loginDays": 14, "loginSeconds": 0, + "sessionDuration": 0, "loginAttempts": 5, "lockoutDuration": 60, "adminReloginDuration": 60, diff --git a/public/language/en-GB/admin/settings/user.json b/public/language/en-GB/admin/settings/user.json index 1d5d460472..c8c418a206 100644 --- a/public/language/en-GB/admin/settings/user.json +++ b/public/language/en-GB/admin/settings/user.json @@ -29,6 +29,8 @@ "session-time-days": "Days", "session-time-seconds": "Seconds", "session-time-help": "These values are used to govern how long a user stays logged in when they check "Remember Me" on login. 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.", + "session-duration": "Session length if \"Remember Me\" is not checked (seconds)", + "session-duration-help": "By default — or if set to 0 — a user will stay logged in for the duration of the session (e.g. however long the browser window/tab remains open). Set this value to explicitly invalidate the session after the specified number of seconds.", "online-cutoff": "Minutes after user is considered inactive", "online-cutoff-help": "If user performs no actions for this duration, they are considered inactive and they do not receive realtime updates.", "registration": "User Registration", diff --git a/src/controllers/authentication.js b/src/controllers/authentication.js index 2a006b321c..81ba2bcceb 100644 --- a/src/controllers/authentication.js +++ b/src/controllers/authentication.js @@ -294,8 +294,9 @@ function continueLogin(strategy, req, res, next) { req.session.cookie.maxAge = duration; req.session.cookie.expires = new Date(Date.now() + duration); } else { - req.session.cookie.maxAge = false; - req.session.cookie.expires = false; + const duration = meta.config.sessionDuration * 1000; + req.session.cookie.maxAge = duration || false; + req.session.cookie.expires = duration ? new Date(Date.now() + duration) : false; } plugins.hooks.fire('action:login.continue', { req, strategy, userData, error: null }); diff --git a/src/views/admin/settings/user.tpl b/src/views/admin/settings/user.tpl index 8a04d13524..bab3c943a0 100644 --- a/src/views/admin/settings/user.tpl +++ b/src/views/admin/settings/user.tpl @@ -126,13 +126,13 @@
- +
- +
@@ -141,6 +141,13 @@

+ +
+ + +

[[admin/settings/user:session-duration-help]]

+
+
diff --git a/test/authentication.js b/test/authentication.js index a4d15cbe1f..0dd702698f 100644 --- a/test/authentication.js +++ b/test/authentication.js @@ -158,22 +158,6 @@ describe('authentication', () => { }); }); - it('should login a user', async () => { - const { jar, body: loginBody } = await helpers.loginUser('regular', 'regularpwd'); - assert(loginBody); - const body = await requestAsync({ - url: `${nconf.get('url')}/api/self`, - json: true, - jar, - }); - assert(body); - assert.equal(body.username, 'regular'); - assert.equal(body.email, 'regular@nodebb.org'); - const sessions = await db.getObject(`uid:${regularUid}:sessionUUID:sessionId`); - assert(sessions); - assert(Object.keys(sessions).length > 0); - }); - it('should regenerate the session identifier on successful login', async () => { const matchRegexp = /express\.sid=s%3A(.+?);/; const { hostname, path } = url.parse(nconf.get('url')); @@ -202,6 +186,114 @@ describe('authentication', () => { }); }); + describe('login', () => { + let username; + let password; + let uid; + + function getCookieExpiry(res) { + assert(res.headers['set-cookie']); + assert.strictEqual(res.headers['set-cookie'][0].includes('Expires'), true); + + const values = res.headers['set-cookie'][0].split(';'); + return values.reduce((memo, cur) => { + if (!memo) { + const [name, value] = cur.split('='); + if (name === ' Expires') { + memo = new Date(value); + } + } + + return memo; + }, undefined); + } + + beforeEach(async () => { + ([username, password] = [utils.generateUUID().slice(0, 10), utils.generateUUID()]); + uid = await user.create({ username, password }); + }); + + it('should login a user', async () => { + const { jar, body: loginBody } = await helpers.loginUser(username, password); + assert(loginBody); + const body = await requestAsync({ + url: `${nconf.get('url')}/api/self`, + json: true, + jar, + }); + assert(body); + assert.equal(body.username, username); + const sessions = await db.getObject(`uid:${uid}:sessionUUID:sessionId`); + assert(sessions); + assert(Object.keys(sessions).length > 0); + }); + + it('should set a cookie that only lasts for the life of the browser session', async () => { + const { res } = await helpers.loginUser(username, password); + + assert(res.headers); + assert(res.headers['set-cookie']); + assert.strictEqual(res.headers['set-cookie'][0].includes('Expires'), false); + }); + + it('should set a different expiry if sessionDuration is set', async () => { + const _sessionDuration = meta.config.sessionDuration; + const days = 1; + meta.config.sessionDuration = days * 24 * 60 * 60; + + const { res } = await helpers.loginUser(username, password); + + const expiry = getCookieExpiry(res); + const expected = new Date(); + expected.setUTCDate(expected.getUTCDate() + days); + + assert.strictEqual(expiry.getUTCDate(), expected.getUTCDate()); + + meta.config.sessionDuration = _sessionDuration; + }); + + it('should set a cookie that lasts for x days where x is loginDays setting, if asked to remember', async () => { + const { res } = await helpers.loginUser(username, password, { remember: 'on' }); + + const expiry = getCookieExpiry(res); + const expected = new Date(); + expected.setUTCDate(expected.getUTCDate() + meta.config.loginDays); + + assert.strictEqual(expiry.getUTCDate(), expected.getUTCDate()); + }); + + it('should set the cookie expiry properly if loginDays setting is changed', async () => { + const _loginDays = meta.config.loginDays; + meta.config.loginDays = 5; + + const { res } = await helpers.loginUser(username, password, { remember: 'on' }); + + const expiry = getCookieExpiry(res); + const expected = new Date(); + expected.setUTCDate(expected.getUTCDate() + meta.config.loginDays); + + assert.strictEqual(expiry.getUTCDate(), expected.getUTCDate()); + + meta.config.loginDays = _loginDays; + }); + + it('should ignore loginDays if loginSeconds is truthy', async () => { + const _loginSeconds = meta.config.loginSeconds; + meta.config.loginSeconds = 60; + + const { res } = await helpers.loginUser(username, password, { remember: 'on' }); + + const expiry = getCookieExpiry(res); + const expected = new Date(); + expected.setUTCSeconds(expected.getUTCSeconds() + meta.config.loginSeconds); + + assert.strictEqual(expiry.getUTCDate(), expected.getUTCDate()); + assert.strictEqual(expiry.getUTCMinutes(), expected.getUTCMinutes()); + + meta.config.loginSeconds = _loginSeconds; + }); + }); + it('should fail to login if ip address is invalid', (done) => { const jar = request.jar(); request({