diff --git a/CHANGELOG.md b/CHANGELOG.md index d3953313c7..89a9b5fe38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,37 @@ +#### v2.8.1 (2022-12-30) + +##### Chores + +* fallbacks for new language string (8a69e740) +* remove extraneous lines from changelog (bbaf26ce) +* incrementing version number - v2.8.0 (8e77673d) +* update changelog for v2.8.0 (a5c2edb9) +* incrementing version number - v2.7.0 (96cc0617) +* incrementing version number - v2.6.1 (7e52a7a5) +* incrementing version number - v2.6.0 (e7fcf482) +* incrementing version number - v2.5.8 (dec0e7de) +* incrementing version number - v2.5.7 (5836bf4a) +* incrementing version number - v2.5.6 (c7bd7dbf) +* incrementing version number - v2.5.5 (3509ed94) +* incrementing version number - v2.5.4 (e83260ca) +* incrementing version number - v2.5.3 (7e922936) +* incrementing version number - v2.5.2 (babcd17e) +* incrementing version number - v2.5.1 (ce3aa950) +* incrementing version number - v2.5.0 (01d276cb) +* incrementing version number - v2.4.5 (dd3e1a28) +* incrementing version number - v2.4.4 (d5525c87) +* incrementing version number - v2.4.3 (9c647c6c) +* incrementing version number - v2.4.2 (3aa7b855) +* incrementing version number - v2.4.1 (60cbd148) +* incrementing version number - v2.4.0 (4834cde3) +* incrementing version number - v2.3.1 (d2425942) +* incrementing version number - v2.3.0 (046ea120) + +##### Bug Fixes + +* vulnerability in socket.io nested namespaces (#11117) (586eed14) +* lock post/reply similar to user.create (1ea9481a) + #### v2.8.0 (2022-12-21) ##### Chores diff --git a/install/package.json b/install/package.json index 25aeb0e78b..374d22d5e0 100644 --- a/install/package.json +++ b/install/package.json @@ -2,7 +2,7 @@ "name": "nodebb", "license": "GPL-3.0", "description": "NodeBB Forum", - "version": "2.8.0", + "version": "2.8.1", "homepage": "http://www.nodebb.org", "repository": { "type": "git", diff --git a/src/socket.io/user.js b/src/socket.io/user.js index 4f5c616114..8215af9616 100644 --- a/src/socket.io/user.js +++ b/src/socket.io/user.js @@ -62,7 +62,7 @@ SocketUser.reset.send = async function (socket, email) { } catch (err) { await logEvent(err.message); await sleep(2500 + ((Math.random() * 500) - 250)); - const internalErrors = ['[[error:invalid-email]]', '[[error:reset-rate-limited]]']; + const internalErrors = ['[[error:invalid-email]]']; if (!internalErrors.includes(err.message)) { throw err; } diff --git a/src/user/reset.js b/src/user/reset.js index b8a77a4d08..557a83285c 100644 --- a/src/user/reset.js +++ b/src/user/reset.js @@ -17,6 +17,8 @@ const UserReset = module.exports; const twoHours = 7200000; +UserReset.minSecondsBetweenEmails = 60; + UserReset.validate = async function (code) { const uid = await db.getObjectField('reset:uid', code); if (!uid) { @@ -39,31 +41,45 @@ UserReset.generate = async function (uid) { return code; }; -async function canGenerate(uid) { - const score = await db.sortedSetScore('reset:issueDate:uid', uid); - if (score > Date.now() - (1000 * 60)) { - throw new Error('[[error:reset-rate-limited]]'); - } -} - UserReset.send = async function (email) { const uid = await user.getUidByEmail(email); if (!uid) { throw new Error('[[error:invalid-email]]'); } - await canGenerate(uid); - await db.sortedSetAdd('reset:issueDate:uid', Date.now(), uid); - const code = await UserReset.generate(uid); - await emailer.send('reset', uid, { - reset_link: `${nconf.get('url')}/reset/${code}`, - subject: '[[email:password-reset-requested]]', - template: 'reset', - uid: uid, - }).catch(err => winston.error(`[emailer.send] ${err.stack}`)); - - return code; + await lockReset(uid, '[[error:reset-rate-limited]]'); + try { + await canGenerate(uid); + await db.sortedSetAdd('reset:issueDate:uid', Date.now(), uid); + const code = await UserReset.generate(uid); + await emailer.send('reset', uid, { + reset_link: `${nconf.get('url')}/reset/${code}`, + subject: '[[email:password-reset-requested]]', + template: 'reset', + uid: uid, + }).catch(err => winston.error(`[emailer.send] ${err.stack}`)); + + return code; + } finally { + db.deleteObjectField('locks', `reset${uid}`); + } }; +async function lockReset(uid, error) { + const value = `reset${uid}`; + const count = await db.incrObjectField('locks', value); + if (count > 1) { + throw new Error(error); + } + return value; +} + +async function canGenerate(uid) { + const score = await db.sortedSetScore('reset:issueDate:uid', uid); + if (score > Date.now() - (UserReset.minSecondsBetweenEmails * 1000)) { + throw new Error('[[error:reset-rate-limited]]'); + } +} + UserReset.commit = async function (code, password) { user.isPasswordValid(password); const validated = await UserReset.validate(code); @@ -122,16 +138,13 @@ UserReset.updateExpiry = async function (uid) { }; UserReset.clean = async function () { - const [tokens, uids] = await Promise.all([ - db.getSortedSetRangeByScore('reset:issueDate', 0, -1, '-inf', Date.now() - twoHours), - db.getSortedSetRangeByScore('reset:issueDate:uid', 0, -1, '-inf', Date.now() - twoHours), - ]); - if (!tokens.length && !uids.length) { + const tokens = await db.getSortedSetRangeByScore('reset:issueDate', 0, -1, '-inf', Date.now() - twoHours); + if (!tokens.length) { return; } winston.verbose(`[UserReset.clean] Removing ${tokens.length} reset tokens from database`); - await cleanTokensAndUids(tokens, uids); + await cleanTokens(tokens); }; UserReset.cleanByUid = async function (uid) { @@ -153,13 +166,15 @@ UserReset.cleanByUid = async function (uid) { } winston.verbose(`[UserReset.cleanByUid] Found ${tokensToClean.length} token(s), removing...`); - await cleanTokensAndUids(tokensToClean, uid); + await Promise.all([ + cleanTokens(tokensToClean), + db.deleteObjectField('locks', `reset${uid}`), + ]); }; -async function cleanTokensAndUids(tokens, uids) { +async function cleanTokens(tokens) { await Promise.all([ db.deleteObjectFields('reset:uid', tokens), db.sortedSetRemove('reset:issueDate', tokens), - db.sortedSetRemove('reset:issueDate:uid', uids), ]); } diff --git a/test/socket.io.js b/test/socket.io.js index 1a94a38f7c..5bb8a6d010 100644 --- a/test/socket.io.js +++ b/test/socket.io.js @@ -733,7 +733,7 @@ describe('socket.io', () => { it('should not generate code if rate limited', (done) => { socketUser.reset.send({ uid: 0 }, 'regular@test.com', (err) => { - assert.ifError(err); + assert(err); async.parallel({ count: async.apply(db.sortedSetCount.bind(db), 'reset:issueDate', 0, Date.now()), diff --git a/test/user.js b/test/user.js index 990122796a..cb02df20a1 100644 --- a/test/user.js +++ b/test/user.js @@ -15,7 +15,6 @@ const User = require('../src/user'); const Topics = require('../src/topics'); const Categories = require('../src/categories'); const Posts = require('../src/posts'); -const Password = require('../src/password'); const groups = require('../src/groups'); const messaging = require('../src/messaging'); const helpers = require('./helpers'); @@ -587,106 +586,6 @@ describe('User', () => { }); }); - describe('passwordReset', () => { - let uid; - let code; - before(async () => { - uid = await User.create({ username: 'resetuser', password: '123456' }); - await User.setUserField(uid, 'email', 'reset@me.com'); - await User.email.confirmByUid(uid); - }); - - it('.generate() should generate a new reset code', (done) => { - User.reset.generate(uid, (err, _code) => { - assert.ifError(err); - assert(_code); - - code = _code; - done(); - }); - }); - - it('.generate() should invalidate a previous generated reset code', async () => { - const _code = await User.reset.generate(uid); - const valid = await User.reset.validate(code); - assert.strictEqual(valid, false); - - code = _code; - }); - - it('.validate() should ensure that this new code is valid', (done) => { - User.reset.validate(code, (err, valid) => { - assert.ifError(err); - assert.strictEqual(valid, true); - done(); - }); - }); - - it('.validate() should correctly identify an invalid code', (done) => { - User.reset.validate(`${code}abcdef`, (err, valid) => { - assert.ifError(err); - assert.strictEqual(valid, false); - done(); - }); - }); - - it('.send() should create a new reset code and reset password', async () => { - code = await User.reset.send('reset@me.com'); - }); - - it('.commit() should update the user\'s password and confirm their email', (done) => { - User.reset.commit(code, 'newpassword', (err) => { - assert.ifError(err); - - async.parallel({ - userData: function (next) { - User.getUserData(uid, next); - }, - password: function (next) { - db.getObjectField(`user:${uid}`, 'password', next); - }, - }, (err, results) => { - assert.ifError(err); - Password.compare('newpassword', results.password, true, (err, match) => { - assert.ifError(err); - assert(match); - assert.strictEqual(results.userData['email:confirmed'], 1); - done(); - }); - }); - }); - }); - - it('.should error if same password is used for reset', async () => { - const uid = await User.create({ username: 'badmemory', email: 'bad@memory.com', password: '123456' }); - const code = await User.reset.generate(uid); - let err; - try { - await User.reset.commit(code, '123456'); - } catch (_err) { - err = _err; - } - assert.strictEqual(err.message, '[[error:reset-same-password]]'); - }); - - it('should not validate email if password reset is due to expiry', async () => { - const uid = await User.create({ username: 'resetexpiry', email: 'reset@expiry.com', password: '123456' }); - let confirmed = await User.getUserField(uid, 'email:confirmed'); - let [verified, unverified] = await groups.isMemberOfGroups(uid, ['verified-users', 'unverified-users']); - assert.strictEqual(confirmed, 0); - assert.strictEqual(verified, false); - assert.strictEqual(unverified, true); - await User.setUserField(uid, 'passwordExpiry', Date.now()); - const code = await User.reset.generate(uid); - await User.reset.commit(code, '654321'); - confirmed = await User.getUserField(uid, 'email:confirmed'); - [verified, unverified] = await groups.isMemberOfGroups(uid, ['verified-users', 'unverified-users']); - assert.strictEqual(confirmed, 0); - assert.strictEqual(verified, false); - assert.strictEqual(unverified, true); - }); - }); - describe('hash methods', () => { it('should return uid from email', (done) => { User.getUidByEmail('john@example.com', (err, uid) => { diff --git a/test/user/reset.js b/test/user/reset.js new file mode 100644 index 0000000000..36f80b2dda --- /dev/null +++ b/test/user/reset.js @@ -0,0 +1,163 @@ +'use strict'; + +const assert = require('assert'); +const async = require('async'); + +const db = require('../mocks/databasemock'); + +const user = require('../../src/user'); +const groups = require('../../src/groups'); +const password = require('../../src/password'); +const utils = require('../../src/utils'); + +const socketUser = require('../../src/socket.io/user'); + +describe('Password reset (library methods)', () => { + let uid; + let code; + before(async () => { + uid = await user.create({ username: 'resetuser', password: '123456' }); + await user.setUserField(uid, 'email', 'reset@me.com'); + await user.email.confirmByUid(uid); + }); + + it('.generate() should generate a new reset code', (done) => { + user.reset.generate(uid, (err, _code) => { + assert.ifError(err); + assert(_code); + + code = _code; + done(); + }); + }); + + it('.generate() should invalidate a previous generated reset code', async () => { + const _code = await user.reset.generate(uid); + const valid = await user.reset.validate(code); + assert.strictEqual(valid, false); + + code = _code; + }); + + it('.validate() should ensure that this new code is valid', (done) => { + user.reset.validate(code, (err, valid) => { + assert.ifError(err); + assert.strictEqual(valid, true); + done(); + }); + }); + + it('.validate() should correctly identify an invalid code', (done) => { + user.reset.validate(`${code}abcdef`, (err, valid) => { + assert.ifError(err); + assert.strictEqual(valid, false); + done(); + }); + }); + + it('.send() should create a new reset code and reset password', async () => { + code = await user.reset.send('reset@me.com'); + }); + + it('.commit() should update the user\'s password and confirm their email', (done) => { + user.reset.commit(code, 'newpassword', (err) => { + assert.ifError(err); + + async.parallel({ + userData: function (next) { + user.getUserData(uid, next); + }, + password: function (next) { + db.getObjectField(`user:${uid}`, 'password', next); + }, + }, (err, results) => { + assert.ifError(err); + password.compare('newpassword', results.password, true, (err, match) => { + assert.ifError(err); + assert(match); + assert.strictEqual(results.userData['email:confirmed'], 1); + done(); + }); + }); + }); + }); + + it('.should error if same password is used for reset', async () => { + const uid = await user.create({ username: 'badmemory', email: 'bad@memory.com', password: '123456' }); + const code = await user.reset.generate(uid); + let err; + try { + await user.reset.commit(code, '123456'); + } catch (_err) { + err = _err; + } + assert.strictEqual(err.message, '[[error:reset-same-password]]'); + }); + + it('should not validate email if password reset is due to expiry', async () => { + const uid = await user.create({ username: 'resetexpiry', email: 'reset@expiry.com', password: '123456' }); + let confirmed = await user.getUserField(uid, 'email:confirmed'); + let [verified, unverified] = await groups.isMemberOfGroups(uid, ['verified-users', 'unverified-users']); + assert.strictEqual(confirmed, 0); + assert.strictEqual(verified, false); + assert.strictEqual(unverified, true); + await user.setUserField(uid, 'passwordExpiry', Date.now()); + const code = await user.reset.generate(uid); + await user.reset.commit(code, '654321'); + confirmed = await user.getUserField(uid, 'email:confirmed'); + [verified, unverified] = await groups.isMemberOfGroups(uid, ['verified-users', 'unverified-users']); + assert.strictEqual(confirmed, 0); + assert.strictEqual(verified, false); + assert.strictEqual(unverified, true); + }); +}); + +describe.only('locks', () => { + let uid; + let email; + beforeEach(async () => { + const [username, password] = [utils.generateUUID().slice(0, 10), utils.generateUUID()]; + uid = await user.create({ username, password }); + email = `${username}@nodebb.org`; + await user.setUserField(uid, 'email', email); + await user.email.confirmByUid(uid); + }); + + it('should disallow reset request if one was made within the minute', async () => { + await user.reset.send(email); + await assert.rejects(user.reset.send(email), { + message: '[[error:reset-rate-limited]]', + }); + }); + + it('should not allow multiple calls to the reset method at the same time', async () => { + await assert.rejects(Promise.all([ + user.reset.send(email), + user.reset.send(email), + ]), { + message: '[[error:reset-rate-limited]]', + }); + }); + + it('should not allow multiple socket calls to the reset method either', async () => { + await assert.rejects(Promise.all([ + socketUser.reset.send({ uid: 0 }, email), + socketUser.reset.send({ uid: 0 }, email), + ]), { + message: '[[error:reset-rate-limited]]', + }); + }); + + it('should properly unlock user reset', async () => { + await user.reset.send(email); + await assert.rejects(user.reset.send(email), { + message: '[[error:reset-rate-limited]]', + }); + user.reset.minSecondsBetweenEmails = 3; + const util = require('util'); + const sleep = util.promisify(setTimeout); + await sleep(4 * 1000); // wait 4 seconds + await user.reset.send(email); + user.reset.minSecondsBetweenEmails = 60; + }); +});