fix: #10027, properly auto confirm first user

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,

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(data.res.statusCode, 403);
-			assert.equal(body, '[[error:invalid-login-credentials]]');
+			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(data.res.statusCode, 403);
-			assert.equal(body, '[[error:invalid-username-or-password]]');
+			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(data.res.statusCode, 403);
-			assert.equal(body, '[[error:invalid-username-or-password]]');
+			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(data.res.statusCode, 403);
-			assert.equal(body, '[[error:invalid-username-or-password]]');
+			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(data.res.statusCode, 403);
-				assert.equal(body, '[[error:invalid-login-credentials]]');
+				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(data.res.statusCode, 403);
-			assert.equal(body, '[[error:password-too-long]]');
+			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(data.res.statusCode, 403);
-				assert.equal(body, '[[error:local-login-disabled]]');
+				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');
+		const { res } = await helpers.loginUser('ginger@nodebb.org', '123456');
-		assert.equal(response.statusCode, 200);
+		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(data.res.statusCode, 400);
-			assert.equal(body, '[[error:wrong-login-type-username]]');
+			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);
+					assert.equal(data.res.statusCode, 403);
-					delete body.timestamp;
+					delete data.body.timestamp;
-					assert.deepStrictEqual(body, {
+					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.equal(data.res.statusCode, 403);
-								assert(body.banned_until);
+								assert(data.body.banned_until);
-								assert(body.reason, '[[user:info.banned-no-reason]]');
+								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) {
+			function (data, next) {
-				loginUser('lockme', 'abcdef', next);
+				helpers.loginUser('lockme', 'abcdef', next);
 			},
-			function (res, body, jar, next) {
+			function (data, next) {
-				loginUser('lockme', 'abcdef', next);
+				helpers.loginUser('lockme', 'abcdef', next);
 			},
-			function (res, body, jar, next) {
+			function (data, next) {
-				loginUser('lockme', 'abcdef', 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(data.res.statusCode, 403);
-				assert.equal(body, '[[error:account-locked]]');
+				assert.equal(data.body, '[[error:account-locked]]');
-				loginUser('lockme', 'abcdef', next);
+				helpers.loginUser('lockme', 'abcdef', next);
 			},
-			function (res, body, jar, next) {
+			function (data, next) {
-				assert.equal(res.statusCode, 403);
+				assert.equal(data.res.statusCode, 403);
-				assert.equal(body, '[[error:account-locked]]');
+				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);
 	});

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)
+					// 1 for the new event appended, 2 for username/email change
-					assert.strictEqual(entries + 2, history.length);
+					assert.strictEqual(entries + 3, history.length);
 					done();
 				});
 			});

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) => {
+		}, (err, res, body) => {
-			if (err || res.statusCode !== 200) {
+			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 });
 		});
 	});
 };