[FIX] ReadOnly Rooms permission checks

app/api/server/v1/chat.js

@@ -5,6 +5,8 @@ import { Messages } from '../../../models';
 import { canAccessRoom, hasPermission } from '../../../authorization';
 import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser';
 import { processWebhookMessage } from '../../../lib/server';
+import { executeSendMessage } from '../../../lib/server/methods/sendMessage';
+import { executeSetReaction } from '../../../reactions/server/setReaction';
 import { API } from '../api';
 import Rooms from '../../../models/server/models/Rooms';
 import Users from '../../../models/server/models/Users';
@@ -172,8 +174,7 @@ API.v1.addRoute('chat.sendMessage', { authRequired: true }, {
 			throw new Meteor.Error('error-invalid-params', 'The "message" parameter must be provided.');
 		}
 
-		const sent = Meteor.runAsUser(this.userId, () => Meteor.call('sendMessage', this.bodyParams.message));
-
+		const sent = executeSendMessage(this.userId, this.bodyParams.message);
 		const [message] = normalizeMessagesForUser([sent], this.userId);
 
 		return API.v1.success({
@@ -294,7 +295,7 @@ API.v1.addRoute('chat.react', { authRequired: true }, {
 			throw new Meteor.Error('error-emoji-param-not-provided', 'The required "emoji" param is missing.');
 		}
 
-		Meteor.runAsUser(this.userId, () => Meteor.call('setReaction', emoji, msg._id, this.bodyParams.shouldReact));
+		Meteor.runAsUser(this.userId, () => Promise.await(executeSetReaction(emoji, msg._id, this.bodyParams.shouldReact)));
 
 		return API.v1.success();
 	},

app/authorization/server/functions/canDeleteMessage.js

@@ -1,5 +1,6 @@
 import { hasPermissionAsync } from './hasPermission';
 import { getValue } from '../../../settings/server/raw';
+import { Rooms } from '../../../models';
 
 const elapsedTime = (ts) => {
 	const dif = Date.now() - ts;
@@ -30,12 +31,20 @@ export const canDeleteMessageAsync = async (uid, { u, rid, ts }) => {
 	}
 	const blockDeleteInMinutes = await getValue('Message_AllowDeleting_BlockDeleteInMinutes');
 
-	if (!blockDeleteInMinutes) {
-		return true;
+	if (blockDeleteInMinutes) {
+		const timeElapsedForMessage = elapsedTime(ts);
+		return timeElapsedForMessage <= blockDeleteInMinutes;
+	}
+
+	const room = await Rooms.findOneById(rid, { fields: { ro: 1, unmuted: 1 } });
+	if (room.ro === true && !await hasPermissionAsync(uid, 'post-readonly', rid)) {
+		// Unless the user was manually unmuted
+		if (!(room.unmuted || []).includes(u.username)) {
+			throw new Error('You can\'t delete messages because the room is readonly.');
+		}
 	}
 
-	const timeElapsedForMessage = elapsedTime(ts);
-	return timeElapsedForMessage <= blockDeleteInMinutes;
+	return true;
 };
 
 export const canDeleteMessage = (uid, { u, rid, ts }) => Promise.await(canDeleteMessageAsync(uid, { u, rid, ts }));

app/authorization/server/functions/canSendMessage.js

@@ -10,21 +10,19 @@ const subscriptionOptions = {
 	},
 };
 
-export const canSendMessageAsync = async (rid, { uid, username, type }, extraData) => {
-	const room = await Rooms.findOneById(rid);
-
+export const validateRoomMessagePermissionsAsync = async (room, { uid, username, type }, extraData) => {
 	if (type !== 'app' && !await canAccessRoomAsync(room, { _id: uid, username }, extraData)) {
 		throw new Error('error-not-allowed');
 	}
 
 	if (roomTypes.getConfig(room.t).allowMemberAction(room, RoomMemberActions.BLOCK)) {
-		const subscription = await Subscriptions.findOneByRoomIdAndUserId(rid, uid, subscriptionOptions);
+		const subscription = await Subscriptions.findOneByRoomIdAndUserId(room._id, uid, subscriptionOptions);
 		if (subscription && (subscription.blocked || subscription.blocker)) {
 			throw new Error('room_is_blocked');
 		}
 	}
 
-	if (room.ro === true && !await hasPermissionAsync(uid, 'post-readonly', rid)) {
+	if (room.ro === true && !await hasPermissionAsync(uid, 'post-readonly', room._id)) {
 		// Unless the user was manually unmuted
 		if (!(room.unmuted || []).includes(username)) {
 			throw new Error('You can\'t send messages because the room is readonly.');
@@ -34,8 +32,13 @@ export const canSendMessageAsync = async (rid, { uid, username, type }, extraDat
 	if ((room.muted || []).includes(username)) {
 		throw new Error('You_have_been_muted');
 	}
+};
 
+export const canSendMessageAsync = async (rid, { uid, username, type }, extraData) => {
+	const room = await Rooms.findOneById(rid);
+	await validateRoomMessagePermissionsAsync(room, { uid, username, type }, extraData);
 	return room;
 };
 
 export const canSendMessage = (rid, { uid, username, type }, extraData) => Promise.await(canSendMessageAsync(rid, { uid, username, type }, extraData));
+export const validateRoomMessagePermissions = (room, { uid, username, type }, extraData) => Promise.await(validateRoomMessagePermissionsAsync(room, { uid, username, type }, extraData));

app/authorization/server/index.js

@@ -4,7 +4,7 @@ import {
 	canAccessRoom,
 	roomAccessValidators,
 } from './functions/canAccessRoom';
-import { canSendMessage } from './functions/canSendMessage';
+import { canSendMessage, validateRoomMessagePermissions } from './functions/canSendMessage';
 import { getRoles } from './functions/getRoles';
 import { getUsersInRole } from './functions/getUsersInRole';
 import {
@@ -30,6 +30,7 @@ export {
 	hasRole,
 	removeUserFromRoles,
 	canSendMessage,
+	validateRoomMessagePermissions,
 	addRoomAccessValidator,
 	roomAccessValidators,
 	addUserRoles,

app/lib/server/functions/processWebhookMessage.js

@@ -4,6 +4,7 @@ import s from 'underscore.string';
 
 import { getRoomByNameOrIdWithOptionToJoin } from './getRoomByNameOrIdWithOptionToJoin';
 import { sendMessage } from './sendMessage';
+import { validateRoomMessagePermissions } from '../../../authorization/server/functions/canSendMessage';
 import { Subscriptions } from '../../../models';
 import { getDirectMessageByIdWithOptionToJoin, getDirectMessageByNameOrIdWithOptionToJoin } from './getDirectMessageByNameOrIdWithOptionToJoin';
 
@@ -82,6 +83,8 @@ export const processWebhookMessage = function(messageObj, user, defaultValues =
 			}
 		}
 
+		validateRoomMessagePermissions(room, { uid: user._id, ...user });
+
 		const messageReturn = sendMessage(user, message, room);
 		sentData.push({ channel, message: messageReturn });
 	}

app/lib/server/methods/sendMessage.js

@@ -72,18 +72,21 @@ export function executeSendMessage(uid, message) {
 		metrics.messagesSent.inc(); // TODO This line needs to be moved to it's proper place. See the comments on: https://github.com/RocketChat/Rocket.Chat/pull/5736
 		return sendMessage(user, message, room, false);
 	} catch (error) {
-		if (error === 'error-not-allowed') {
-			throw new Meteor.Error('error-not-allowed');
-		}
-
 		SystemLogger.error('Error sending message:', error);
 
+		const errorMessage = typeof error === 'string' ? error : error.error || error.message;
 		Notifications.notifyUser(uid, 'message', {
 			_id: Random.id(),
 			rid: message.rid,
 			ts: new Date(),
-			msg: TAPi18n.__(error, {}, user.language),
+			msg: TAPi18n.__(errorMessage, {}, user.language),
 		});
+
+		if (typeof error === 'string') {
+			throw new Error(error);
+		}
+
+		throw error;
 	}
 }
 
@@ -98,7 +101,15 @@ Meteor.methods({
 			});
 		}
 
-		return executeSendMessage(uid, message);
+		try {
+			return executeSendMessage(uid, message);
+		} catch (error) {
+			if ((error.error || error.message) === 'error-not-allowed') {
+				throw new Meteor.Error(error.error || error.message, error.reason, {
+					method: 'sendMessage',
+				});
+			}
+		}
 	},
 });
 // Limit a user, who does not have the "bot" role, to sending 5 msgs/second

app/lib/server/methods/updateMessage.js

@@ -4,7 +4,7 @@ import moment from 'moment';
 
 import { Messages } from '../../../models';
 import { settings } from '../../../settings';
-import { hasPermission } from '../../../authorization';
+import { hasPermission, canSendMessage } from '../../../authorization/server';
 import { updateMessage } from '../functions';
 
 Meteor.methods({
@@ -47,6 +47,9 @@ Meteor.methods({
 			}
 		}
 
+		const user = Meteor.users.findOne(Meteor.userId());
+		canSendMessage(message.rid, { uid: user._id, ...user });
+
 		// It is possible to have an empty array as the attachments property, so ensure both things exist
 		if (originalMessage.attachments && originalMessage.attachments.length > 0 && originalMessage.attachments[0].description !== undefined) {
 			message.attachments = originalMessage.attachments;

app/reactions/server/setReaction.js

@@ -33,13 +33,9 @@ async function setReaction(room, user, message, reaction, shouldReact) {
 	}
 
 	if (Array.isArray(room.muted) && room.muted.indexOf(user.username) !== -1) {
-		Notifications.notifyUser(Meteor.userId(), 'message', {
-			_id: Random.id(),
+		throw new Meteor.Error('error-not-allowed', TAPi18n.__('You_have_been_muted', {}, user.language), {
 			rid: room._id,
-			ts: new Date(),
-			msg: TAPi18n.__('You_have_been_muted', {}, user.language),
 		});
-		return false;
 	}
 
 	const userAlreadyReacted = Boolean(message.reactions) && Boolean(message.reactions[reaction]) && message.reactions[reaction].usernames.indexOf(user.username) !== -1;
@@ -88,26 +84,45 @@ async function setReaction(room, user, message, reaction, shouldReact) {
 	msgStream.emit(message.rid, message);
 }
 
-Meteor.methods({
-	setReaction(reaction, messageId, shouldReact) {
-		const user = Meteor.user();
+export const executeSetReaction = async function(reaction, messageId, shouldReact) {
+	const user = Meteor.user();
 
-		if (!user) {
-			throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'setReaction' });
-		}
+	if (!user) {
+		throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'setReaction' });
+	}
 
-		const message = Messages.findOneById(messageId);
+	const message = Messages.findOneById(messageId);
 
-		if (!message) {
-			throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'setReaction' });
-		}
+	if (!message) {
+		throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'setReaction' });
+	}
 
-		const room = Meteor.call('canAccessRoom', message.rid, Meteor.userId());
+	const room = Meteor.call('canAccessRoom', message.rid, Meteor.userId());
 
-		if (!room) {
-			throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'setReaction' });
-		}
+	if (!room) {
+		throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'setReaction' });
+	}
 
-		setReaction(room, user, message, reaction, shouldReact);
+	return setReaction(room, user, message, reaction, shouldReact);
+};
+
+Meteor.methods({
+	setReaction(reaction, messageId, shouldReact) {
+		try {
+			return Promise.await(executeSetReaction(reaction, messageId, shouldReact));
+		} catch (e) {
+			if (e.error === 'error-not-allowed' && e.reason && e.details && e.details.rid) {
+				Notifications.notifyUser(Meteor.userId(), 'message', {
+					_id: Random.id(),
+					rid: e.details.rid,
+					ts: new Date(),
+					msg: e.reason,
+				});
+
+				return false;
+			}
+
+			throw e;
+		}
 	},
 });

tests/end-to-end/api/05-chat.js

@@ -702,6 +702,31 @@ describe('[Chat]', function() {
 		describe('Read only channel', () => {
 			let readOnlyChannel;
 
+			const userCredentials = {};
+			let user;
+			before((done) => {
+				const username = `user.test.readonly.${ Date.now() }`;
+				const email = `${ username }@rocket.chat`;
+				request.post(api('users.create'))
+					.set(credentials)
+					.send({ email, name: username, username, password })
+					.end((err, res) => {
+						user = res.body.user;
+						request.post(api('login'))
+							.send({
+								user: username,
+								password,
+							})
+							.expect('Content-Type', 'application/json')
+							.expect(200)
+							.expect((res) => {
+								userCredentials['X-Auth-Token'] = res.body.data.authToken;
+								userCredentials['X-User-Id'] = res.body.data.userId;
+							})
+							.end(done);
+					});
+			});
+
 			it('Creating a read-only channel', (done) => {
 				request.post(api('channels.create'))
 					.set(credentials)
@@ -734,6 +759,60 @@ describe('[Chat]', function() {
 					})
 					.end(done);
 			});
+			it('Inviting regular user to read-only channel', (done) => {
+				request.post(api('channels.invite'))
+					.set(credentials)
+					.send({
+						roomId: readOnlyChannel._id,
+						userId: user._id,
+					})
+					.expect('Content-Type', 'application/json')
+					.expect(200)
+					.expect((res) => {
+						expect(res.body).to.have.property('success', true);
+					})
+					.end(() => {
+						done();
+					});
+			});
+
+			it('should fail to send message when the user lacks permission', (done) => {
+				request.post(api('chat.sendMessage'))
+					.set(userCredentials)
+					.send({
+						message: {
+							rid: readOnlyChannel._id,
+							msg: 'Sample blocked message',
+						},
+					})
+					.expect('Content-Type', 'application/json')
+					.expect(400)
+					.expect((res) => {
+						expect(res.body).to.have.property('success', false);
+						expect(res.body).to.have.property('error');
+					})
+					.end(done);
+			});
+
+			it('should send a message when the user has permission to send messages on readonly channels', (done) => {
+				updatePermission('post-readonly', ['user']).then(() => {
+					request.post(api('chat.sendMessage'))
+						.set(userCredentials)
+						.send({
+							message: {
+								rid: readOnlyChannel._id,
+								msg: 'Sample message overwriting readonly status',
+							},
+						})
+						.expect('Content-Type', 'application/json')
+						.expect(200)
+						.expect((res) => {
+							expect(res.body).to.have.property('success', true);
+							expect(res.body).to.have.property('message').and.to.be.an('object');
+						})
+						.end(done);
+				});
+			});
 		});
 	});