From 2473d5d873e1658837a1e51b40f595bf93f244e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Mon, 22 Nov 2021 23:20:31 -0500 Subject: [PATCH] fix: #10027, properly auto confirm first user --- src/user/create.js | 13 ++-- test/authentication.js | 157 +++++++++++++++++++---------------------- test/flags.js | 4 +- test/helpers/index.js | 10 +-- 4 files changed, 86 insertions(+), 98 deletions(-) diff --git a/src/user/create.js b/src/user/create.js index 869db6b162..a278297db7 100644 --- a/src/user/create.js +++ b/src/user/create.js @@ -77,9 +77,6 @@ module.exports = function (User) { const isFirstUser = uid === 1; userData.uid = uid; - if (isFirstUser) { - userData['email:confirmed'] = 1; - } await db.setObject(`user:${uid}`, userData); const bulkAdd = [ @@ -97,20 +94,20 @@ module.exports = function (User) { bulkAdd.push(['fullname:sorted', 0, `${userData.fullname.toLowerCase()}:${userData.uid}`]); } - const groupsToJoin = ['registered-users'].concat( - isFirstUser ? 'verified-users' : 'unverified-users' - ); - await Promise.all([ db.incrObjectField('global', 'userCount'), analytics.increment('registrations'), db.sortedSetAddBulk(bulkAdd), - groups.join(groupsToJoin, userData.uid), + groups.join(['registered-users', 'unverified-users'], userData.uid), User.notifications.sendWelcomeNotification(userData.uid), storePassword(userData.uid, data.password), User.updateDigestSetting(userData.uid, meta.config.dailyDigestFreq), ]); + if (userData.email && isFirstUser) { + await User.email.confirmByUid(userData.uid); + } + if (userData.email && userData.uid > 1) { User.email.sendValidationEmail(userData.uid, { email: userData.email, diff --git a/test/authentication.js b/test/authentication.js index b65c95f2dd..1b796f5797 100644 --- a/test/authentication.js +++ b/test/authentication.js @@ -16,44 +16,35 @@ const privileges = require('../src/privileges'); const helpers = require('./helpers'); describe('authentication', () => { - function loginUser(username, password, callback) { - const jar = request.jar(); - request({ - url: `${nconf.get('url')}/api/config`, - json: true, - jar: jar, - }, (err, response, body) => { - if (err) { - return callback(err); - } - - request.post(`${nconf.get('url')}/login`, { - form: { - username: username, - password: password, - }, - json: true, - jar: jar, - headers: { - 'x-csrf-token': body.csrf_token, - }, - }, (err, response, body) => { - callback(err, response, body, jar); - }); - }); - } - const loginUserPromisified = util.promisify(loginUser); - const jar = request.jar(); let regularUid; before((done) => { user.create({ username: 'regular', password: 'regularpwd', email: 'regular@nodebb.org' }, (err, uid) => { assert.ifError(err); regularUid = uid; + assert.strictEqual(uid, 1); done(); }); }); + it('should allow login with email for uid 1', async () => { + const oldValue = meta.config.allowLoginWith; + meta.config.allowLoginWith = 'email'; + const { res } = await helpers.loginUser('regular@nodebb.org', 'regularpwd'); + assert.strictEqual(res.statusCode, 200); + meta.config.allowLoginWith = oldValue; + }); + + it('second user should fail to login with email since email is not confirmed', async () => { + const oldValue = meta.config.allowLoginWith; + meta.config.allowLoginWith = 'email'; + const uid = await user.create({ username: '2nduser', password: '2ndpassword', email: '2nduser@nodebb.org' }); + const { res, body } = await helpers.loginUser('2nduser@nodebb.org', '2ndpassword'); + assert.strictEqual(res.statusCode, 403); + assert.strictEqual(body, '[[error:invalid-login-credentials]]'); + meta.config.allowLoginWith = oldValue; + }); + it('should fail to create user if username is too short', (done) => { helpers.registerUser({ username: 'a', @@ -164,13 +155,13 @@ describe('authentication', () => { }); it('should login a user', (done) => { - loginUser('regular', 'regularpwd', (err, response, body, jar) => { + helpers.loginUser('regular', 'regularpwd', (err, data) => { assert.ifError(err); - assert(body); + assert(data.body); request({ url: `${nconf.get('url')}/api/self`, json: true, - jar: jar, + jar: data.jar, }, (err, response, body) => { assert.ifError(err); assert(body); @@ -245,37 +236,37 @@ describe('authentication', () => { }); it('should fail to login if user does not exist', (done) => { - loginUser('doesnotexist', 'nopassword', (err, response, body) => { + helpers.loginUser('doesnotexist', 'nopassword', (err, data) => { assert.ifError(err); - assert.equal(response.statusCode, 403); - assert.equal(body, '[[error:invalid-login-credentials]]'); + assert.equal(data.res.statusCode, 403); + assert.equal(data.body, '[[error:invalid-login-credentials]]'); done(); }); }); it('should fail to login if username is empty', (done) => { - loginUser('', 'some password', (err, response, body) => { + helpers.loginUser('', 'some password', (err, data) => { assert.ifError(err); - assert.equal(response.statusCode, 403); - assert.equal(body, '[[error:invalid-username-or-password]]'); + assert.equal(data.res.statusCode, 403); + assert.equal(data.body, '[[error:invalid-username-or-password]]'); done(); }); }); it('should fail to login if password is empty', (done) => { - loginUser('someuser', '', (err, response, body) => { + helpers.loginUser('someuser', '', (err, data) => { assert.ifError(err); - assert.equal(response.statusCode, 403); - assert.equal(body, '[[error:invalid-username-or-password]]'); + assert.equal(data.res.statusCode, 403); + assert.equal(data.body, '[[error:invalid-username-or-password]]'); done(); }); }); it('should fail to login if username and password are empty', (done) => { - loginUser('', '', (err, response, body) => { + helpers.loginUser('', '', (err, data) => { assert.ifError(err); - assert.equal(response.statusCode, 403); - assert.equal(body, '[[error:invalid-username-or-password]]'); + assert.equal(data.res.statusCode, 403); + assert.equal(data.body, '[[error:invalid-username-or-password]]'); done(); }); }); @@ -283,10 +274,10 @@ describe('authentication', () => { it('should fail to login if user does not have password field in db', (done) => { user.create({ username: 'hasnopassword', email: 'no@pass.org' }, (err, uid) => { assert.ifError(err); - loginUser('hasnopassword', 'doesntmatter', (err, response, body) => { + helpers.loginUser('hasnopassword', 'doesntmatter', (err, data) => { assert.ifError(err); - assert.equal(response.statusCode, 403); - assert.equal(body, '[[error:invalid-login-credentials]]'); + assert.equal(data.res.statusCode, 403); + assert.equal(data.body, '[[error:invalid-login-credentials]]'); done(); }); }); @@ -297,10 +288,10 @@ describe('authentication', () => { for (let i = 0; i < 5000; i++) { longPassword += 'a'; } - loginUser('someuser', longPassword, (err, response, body) => { + helpers.loginUser('someuser', longPassword, (err, data) => { assert.ifError(err); - assert.equal(response.statusCode, 403); - assert.equal(body, '[[error:password-too-long]]'); + assert.equal(data.res.statusCode, 403); + assert.equal(data.body, '[[error:password-too-long]]'); done(); }); }); @@ -308,10 +299,10 @@ describe('authentication', () => { it('should fail to login if local login is disabled', (done) => { privileges.global.rescind(['groups:local:login'], 'registered-users', (err) => { assert.ifError(err); - loginUser('regular', 'regularpwd', (err, response, body) => { + helpers.loginUser('regular', 'regularpwd', (err, data) => { assert.ifError(err); - assert.equal(response.statusCode, 403); - assert.equal(body, '[[error:local-login-disabled]]'); + assert.equal(data.res.statusCode, 403); + assert.equal(data.body, '[[error:local-login-disabled]]'); privileges.global.give(['groups:local:login'], 'registered-users', done); }); }); @@ -396,17 +387,17 @@ describe('authentication', () => { it('should be able to login with email', async () => { const uid = await user.create({ username: 'ginger', password: '123456', email: 'ginger@nodebb.org' }); await user.email.confirmByUid(uid); - const response = await loginUserPromisified('ginger@nodebb.org', '123456'); - assert.equal(response.statusCode, 200); + const { res } = await helpers.loginUser('ginger@nodebb.org', '123456'); + assert.equal(res.statusCode, 200); }); it('should fail to login if login type is username and an email is sent', (done) => { meta.config.allowLoginWith = 'username'; - loginUser('ginger@nodebb.org', '123456', (err, response, body) => { + helpers.loginUser('ginger@nodebb.org', '123456', (err, data) => { meta.config.allowLoginWith = 'username-email'; assert.ifError(err); - assert.equal(response.statusCode, 400); - assert.equal(body, '[[error:wrong-login-type-username]]'); + assert.equal(data.res.statusCode, 400); + assert.equal(data.body, '[[error:wrong-login-type-username]]'); done(); }); }); @@ -450,28 +441,28 @@ describe('authentication', () => { it('should prevent banned user from logging in', (done) => { user.bans.ban(bannedUser.uid, 0, 'spammer', (err) => { assert.ifError(err); - loginUser(bannedUser.username, bannedUser.pw, (err, res, body) => { + helpers.loginUser(bannedUser.username, bannedUser.pw, (err, data) => { assert.ifError(err); - assert.equal(res.statusCode, 403); - delete body.timestamp; - assert.deepStrictEqual(body, { + assert.equal(data.res.statusCode, 403); + delete data.body.timestamp; + assert.deepStrictEqual(data.body, { banned_until: 0, banned_until_readable: '', expiry: 0, expiry_readable: '', reason: 'spammer', - uid: 6, + uid: bannedUser.uid, }); user.bans.unban(bannedUser.uid, (err) => { assert.ifError(err); const expiry = Date.now() + 10000; user.bans.ban(bannedUser.uid, expiry, '', (err) => { assert.ifError(err); - loginUser(bannedUser.username, bannedUser.pw, (err, res, body) => { + helpers.loginUser(bannedUser.username, bannedUser.pw, (err, data) => { assert.ifError(err); - assert.equal(res.statusCode, 403); - assert(body.banned_until); - assert(body.reason, '[[user:info.banned-no-reason]]'); + assert.equal(data.res.statusCode, 403); + assert(data.body.banned_until); + assert(data.body.reason, '[[user:info.banned-no-reason]]'); done(); }); }); @@ -482,14 +473,14 @@ describe('authentication', () => { it('should allow banned user to log in if the "banned-users" group has "local-login" privilege', async () => { await privileges.global.give(['groups:local:login'], 'banned-users'); - const res = await loginUserPromisified(bannedUser.username, bannedUser.pw); + const { res } = await helpers.loginUser(bannedUser.username, bannedUser.pw); assert.strictEqual(res.statusCode, 200); }); it('should allow banned user to log in if the user herself has "local-login" privilege', async () => { await privileges.global.rescind(['groups:local:login'], 'banned-users'); await privileges.categories.give(['local:login'], 0, bannedUser.uid); - const res = await loginUserPromisified(bannedUser.username, bannedUser.pw); + const { res } = await helpers.loginUser(bannedUser.username, bannedUser.pw); assert.strictEqual(res.statusCode, 200); }); }); @@ -503,26 +494,26 @@ describe('authentication', () => { }, function (_uid, next) { uid = _uid; - loginUser('lockme', 'abcdef', next); + helpers.loginUser('lockme', 'abcdef', next); }, - function (res, body, jar, next) { - loginUser('lockme', 'abcdef', next); + function (data, next) { + helpers.loginUser('lockme', 'abcdef', next); }, - function (res, body, jar, next) { - loginUser('lockme', 'abcdef', next); + function (data, next) { + helpers.loginUser('lockme', 'abcdef', next); }, - function (res, body, jar, next) { - loginUser('lockme', 'abcdef', next); + function (data, next) { + helpers.loginUser('lockme', 'abcdef', next); }, - function (res, body, jar, next) { + function (data, next) { meta.config.loginAttempts = 5; - assert.equal(res.statusCode, 403); - assert.equal(body, '[[error:account-locked]]'); - loginUser('lockme', 'abcdef', next); + assert.equal(data.res.statusCode, 403); + assert.equal(data.body, '[[error:account-locked]]'); + helpers.loginUser('lockme', 'abcdef', next); }, - function (res, body, jar, next) { - assert.equal(res.statusCode, 403); - assert.equal(body, '[[error:account-locked]]'); + function (data, next) { + assert.equal(data.res.statusCode, 403); + assert.equal(data.body, '[[error:account-locked]]'); db.exists(`lockout:${uid}`, next); }, function (locked, next) { @@ -534,7 +525,7 @@ describe('authentication', () => { it('should clear all reset tokens upon successful login', async () => { const code = await user.reset.generate(regularUid); - await loginUserPromisified('regular', 'regularpwd'); + await helpers.loginUser('regular', 'regularpwd'); const valid = await user.reset.validate(code); assert.strictEqual(valid, false); }); diff --git a/test/flags.js b/test/flags.js index 9e07974bc4..784af0630f 100644 --- a/test/flags.js +++ b/test/flags.js @@ -673,8 +673,8 @@ describe('Flags', () => { throw err; } - // 1 for the new event appended, 1 for username change (email not changed immediately) - assert.strictEqual(entries + 2, history.length); + // 1 for the new event appended, 2 for username/email change + assert.strictEqual(entries + 3, history.length); done(); }); }); diff --git a/test/helpers/index.js b/test/helpers/index.js index 800e3b7d0b..eab17d186e 100644 --- a/test/helpers/index.js +++ b/test/helpers/index.js @@ -43,7 +43,7 @@ helpers.loginUser = function (username, password, callback) { if (err || res.statusCode !== 200) { return callback(err || new Error('[[error:invalid-response]]')); } - + const { csrf_token } = body; request.post(`${nconf.get('url')}/login`, { form: { username: username, @@ -52,13 +52,13 @@ helpers.loginUser = function (username, password, callback) { json: true, jar: jar, headers: { - 'x-csrf-token': body.csrf_token, + 'x-csrf-token': csrf_token, }, - }, (err, res) => { - if (err || res.statusCode !== 200) { + }, (err, res, body) => { + if (err) { return callback(err || new Error('[[error:invalid-response]]')); } - callback(null, { jar, csrf_token: body.csrf_token }); + callback(null, { jar, res, body, csrf_token: csrf_token }); }); }); };