Merge branch 'master' of https://github.com/NodeBB/NodeBB

src/database/redis/hash.js

@@ -154,7 +154,11 @@ module.exports = function (module) {
 	};
 
 	module.deleteObjectFields = async function (key, fields) {
-		if (!Array.isArray(fields) || !fields.length) {
+		if (!key || !Array.isArray(fields) || !fields.length) {
+			return;
+		}
+		fields = fields.filter(Boolean);
+		if (!fields.length) {
 			return;
 		}
 		await module.client.async.hdel(key, fields);

src/user/create.js

@@ -14,10 +14,30 @@ module.exports = function (User) {
 		if (data.email !== undefined) {
 			data.email = String(data.email).trim();
 		}
-		const timestamp = data.timestamp || Date.now();
 
 		await User.isDataValid(data);
 
+		try {
+			await lock(data.username, '[[error:username-taken]]');
+			if (data.email) {
+				await lock(data.email, '[[error:email-taken]]');
+			}
+			return await create(data);
+		} finally {
+			await db.deleteObjectFields('locks', [data.username, data.email]);
+		}
+	};
+
+	async function lock(value, error) {
+		const count = await db.incrObjectField('locks', value);
+		if (count > 1) {
+			throw new Error(error);
+		}
+	}
+
+	async function create(data) {
+		const timestamp = data.timestamp || Date.now();
+
 		let userData = {
 			username: data.username,
 			userslug: data.userslug,
@@ -92,7 +112,7 @@ module.exports = function (User) {
 		}
 		plugins.fireHook('action:user.create', { user: userData, data: data });
 		return userData.uid;
-	};
+	}
 
 	async function storePassword(uid, password) {
 		if (!password) {

test/database/hash.js

@@ -435,6 +435,10 @@ describe('Hash methods', function () {
 			});
 		});
 
+		it('should not error if one of the fields is undefined', async function () {
+			await db.deleteObjectFields('someKey', ['best', undefined]);
+		});
+
 		it('should not error if field is null', function (done) {
 			db.deleteObjectField('someKey', null, function (err) {
 				assert.ifError(err);

test/user.js

@@ -100,6 +100,40 @@ describe('User', function () {
 				done();
 			});
 		});
+
+		it('should error if username is already taken', async function () {
+			let err;
+			async function tryCreate(data) {
+				try {
+					return await User.create(data);
+				} catch (_err) {
+					err = _err;
+				}
+			}
+
+			await Promise.all([
+				tryCreate({ username: 'dupe1' }),
+				tryCreate({ username: 'dupe1' }),
+			]);
+			assert.strictEqual(err.message, '[[error:username-taken]]');
+		});
+
+		it('should error if email is already taken', async function () {
+			let err;
+			async function tryCreate(data) {
+				try {
+					return await User.create(data);
+				} catch (_err) {
+					err = _err;
+				}
+			}
+
+			await Promise.all([
+				tryCreate({ username: 'notdupe1', email: 'dupe@dupe.com' }),
+				tryCreate({ username: 'notdupe2', email: 'dupe@dupe.com' }),
+			]);
+			assert.strictEqual(err.message, '[[error:email-taken]]');
+		});
 	});
 
 	describe('.uniqueUsername()', function () {