fix: dont allow searching by ip/banned/flagged for regular users

NodeBB · Jul 4, 2020 · 02ac44c · 02ac44c
1 parent 057b783
commit 02ac44c
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 17 deletions.
diff --git a/src/socket.io/user/search.js b/src/socket.io/user/search.js
@@ -9,8 +9,12 @@ module.exports = function (SocketUser) {
 		if (!data) {
 			throw new Error('[[error:invalid-data]]');
 		}
-		const allowed = await privileges.global.can('search:users', socket.uid);
-		if (!allowed) {
+		const [allowed, isPrivileged] = await Promise.all([
+			privileges.global.can('search:users', socket.uid),
+			user.isPrivileged(socket.uid),
+		]);
+
+		if (!allowed || ((data.searchBy === 'ip' || data.bannedOnly || data.flaggedOnly) && !isPrivileged)) {
 			throw new Error('[[error:no-privileges]]');
 		}
 		const result = await user.search({

diff --git a/test/user.js b/test/user.js
@@ -314,7 +314,13 @@ describe('User', function () {
 	});
 
 	describe('.search()', function () {
-		var uid;
+		let adminUid;
+		let uid;
+		before(async () => {
+			adminUid = await User.create({ username: 'noteadmin' });
+			await groups.join('administrators', adminUid);
+		});
+
 		it('should return an object containing an array of matching users', function (done) {
 			User.search({ query: 'john' }, function (err, searchData) {
 				assert.ifError(err);
@@ -347,22 +353,36 @@ describe('User', function () {
 			});
 		});
 
-		it('should search users by ip', function (done) {
-			User.create({ username: 'ipsearch' }, function (err, uid) {
-				assert.ifError(err);
-				db.sortedSetAdd('ip:1.1.1.1:uid', [1, 1], [testUid, uid], function (err) {
-					assert.ifError(err);
-					socketUser.search({ uid: testUid }, { query: '1.1.1.1', searchBy: 'ip' }, function (err, data) {
-						assert.ifError(err);
-						assert(Array.isArray(data.users));
-						assert.equal(data.users.length, 2);
-						done();
-					});
-				});
+		it('should error for unprivileged user', function (done) {
+			socketUser.search({ uid: testUid }, { searchBy: 'ip', query: '123' }, function (err) {
+				assert.equal(err.message, '[[error:no-privileges]]');
+				done();
 			});
 		});
 
-		it('should search users by ip', function (done) {
+		it('should error for unprivileged user', function (done) {
+			socketUser.search({ uid: testUid }, { bannedOnly: true, query: '123' }, function (err) {
+				assert.equal(err.message, '[[error:no-privileges]]');
+				done();
+			});
+		});
+
+		it('should error for unprivileged user', function (done) {
+			socketUser.search({ uid: testUid }, { flaggedOnly: true, query: '123' }, function (err) {
+				assert.equal(err.message, '[[error:no-privileges]]');
+				done();
+			});
+		});
+
+		it('should search users by ip', async function () {
+			const uid = await User.create({ username: 'ipsearch' });
+			await db.sortedSetAdd('ip:1.1.1.1:uid', [1, 1], [testUid, uid]);
+			const data = await socketUser.search({ uid: adminUid }, { query: '1.1.1.1', searchBy: 'ip' });
+			assert(Array.isArray(data.users));
+			assert.equal(data.users.length, 2);
+		});
+
+		it('should search users by uid', function (done) {
 			socketUser.search({ uid: testUid }, { query: uid, searchBy: 'uid' }, function (err, data) {
 				assert.ifError(err);
 				assert(Array.isArray(data.users));
@@ -384,7 +404,7 @@ describe('User', function () {
 				assert.ifError(err);
 				User.setUserFields(uid, { banned: 1, flags: 10 }, function (err) {
 					assert.ifError(err);
-					socketUser.search({ uid: testUid }, {
+					socketUser.search({ uid: adminUid }, {
 						query: 'ipsearch',
 						onlineOnly: true,
 						bannedOnly: true,