From 59bbede8c78c191d8d0e9221a84fd3eb983f8a83 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?=
 <barisusakli@gmail.com>
Date: Tue, 13 Oct 2020 22:08:09 -0400
Subject: [PATCH] fix: cant join system groups

---
 src/controllers/write/groups.js |  2 +-
 src/groups/index.js             |  6 +++---
 src/socket.io/groups.js         |  2 +-
 test/groups.js                  | 37 +++++++++++++++++++++++++++++++--
 4 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/src/controllers/write/groups.js b/src/controllers/write/groups.js
index 9f95f5361a1c..a8b16e7136dd 100644
--- a/src/controllers/write/groups.js
+++ b/src/controllers/write/groups.js
@@ -70,7 +70,7 @@ Groups.join = async (req, res) => {
 
 	if (!res.locals.privileges.isAdmin) {
 		// Admin and privilege groups unjoinable client-side
-		if (group.name === 'administrators' || groups.isPrivilegeGroup(group.name)) {
+		if (groups.systemGroups.includes(group.name) || groups.isPrivilegeGroup(group.name)) {
 			throw new Error('[[error:not-allowed]]');
 		}
 
diff --git a/src/groups/index.js b/src/groups/index.js
index eb3f507518e5..4140e558f9a0 100644
--- a/src/groups/index.js
+++ b/src/groups/index.js
@@ -38,9 +38,9 @@ Groups.getEphemeralGroup = function (groupName) {
 		name: groupName,
 		slug: slugify(groupName),
 		description: '',
-		deleted: '0',
-		hidden: '0',
-		system: '1',
+		deleted: 0,
+		hidden: 0,
+		system: 1,
 	};
 };
 
diff --git a/src/socket.io/groups.js b/src/socket.io/groups.js
index e46983e1835b..9aebf738cfa1 100644
--- a/src/socket.io/groups.js
+++ b/src/socket.io/groups.js
@@ -30,7 +30,7 @@ SocketGroups.join = async (socket, data) => {
 		throw new Error('[[error:invalid-group-name]]');
 	}
 
-	if (data.groupName === 'administrators' || groups.isPrivilegeGroup(data.groupName)) {
+	if (groups.systemGroups.includes(data.groupName) || groups.isPrivilegeGroup(data.groupName)) {
 		throw new Error('[[error:not-allowed]]');
 	}
 
diff --git a/test/groups.js b/test/groups.js
index afbad945b032..a388a5a929a4 100644
--- a/test/groups.js
+++ b/test/groups.js
@@ -48,6 +48,16 @@ describe('Groups', function () {
 					disableLeave: 1,
 				});
 			},
+			async () => {
+				await Groups.create({
+					name: 'Global Moderators',
+					userTitle: 'Global Moderator',
+					description: 'Forum wide moderators',
+					hidden: 0,
+					private: 1,
+					disableJoinRequests: 1,
+				});
+			},
 			function (next) {
 				// Create a new user
 				User.create({
@@ -72,8 +82,8 @@ describe('Groups', function () {
 			},
 		], function (err, results) {
 			assert.ifError(err);
-			testUid = results[4];
-			adminUid = results[5];
+			testUid = results[5];
+			adminUid = results[6];
 			Groups.join('administrators', adminUid, done);
 		});
 	});
@@ -699,6 +709,29 @@ describe('Groups', function () {
 				});
 			});
 		});
+
+		it('should fail to add user to system group', async function () {
+			const uid = await User.create({ username: 'eviluser' });
+			const oldValue = meta.config.allowPrivateGroups;
+			meta.config.allowPrivateGroups = 0;
+			async function test(groupName) {
+				let err;
+				try {
+					await socketGroups.join({ uid: uid }, { groupName: groupName });
+					const isMember = await Groups.isMember(uid, groupName);
+					assert.strictEqual(isMember, false);
+				} catch (_err) {
+					err = _err;
+				}
+				assert.strictEqual(err.message, '[[error:not-allowed]]');
+			}
+			const groups = ['Global Moderators', 'verified-users', 'unverified-users'];
+			for (const g of groups) {
+				// eslint-disable-next-line no-await-in-loop
+				await test(g);
+			}
+			meta.config.allowPrivateGroups = oldValue;
+		});
 	});
 
 	describe('.leave()', function () {