Skip to content

Commit

Permalink
[FIX] Several problems with read-only rooms and muted users (#11311)
Browse files Browse the repository at this point in the history
  • Loading branch information
Hudell authored and rodrigok committed May 16, 2019
1 parent cb4680d commit 8b81b17
Show file tree
Hide file tree
Showing 19 changed files with 182 additions and 72 deletions.
22 changes: 15 additions & 7 deletions app/authorization/client/hasPermission.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import { Template } from 'meteor/templating';
import { ChatPermissions } from './lib/ChatPermissions';
import * as Models from '../../models';

function atLeastOne(permissions = [], scope) {
function atLeastOne(permissions = [], scope, userId) {
userId = userId || Meteor.userId();

return permissions.some((permissionId) => {
const permission = ChatPermissions.findOne(permissionId, { fields: { roles: 1 } });
const roles = (permission && permission.roles) || [];
Expand All @@ -14,12 +16,14 @@ function atLeastOne(permissions = [], scope) {
const roleScope = role && role.scope;
const model = Models[roleScope];

return model && model.isUserInRole && model.isUserInRole(Meteor.userId(), roleName, scope);
return model && model.isUserInRole && model.isUserInRole(userId, roleName, scope);
});
});
}

function all(permissions = [], scope) {
function all(permissions = [], scope, userId) {
userId = userId || Meteor.userId();

return permissions.every((permissionId) => {
const permission = ChatPermissions.findOne(permissionId, { fields: { roles: 1 } });
const roles = (permission && permission.roles) || [];
Expand All @@ -29,13 +33,13 @@ function all(permissions = [], scope) {
const roleScope = role && role.scope;
const model = Models[roleScope];

return model && model.isUserInRole && model.isUserInRole(Meteor.userId(), roleName, scope);
return model && model.isUserInRole && model.isUserInRole(userId, roleName, scope);
});
});
}

function _hasPermission(permissions, scope, strategy) {
const userId = Meteor.userId();
function _hasPermission(permissions, scope, strategy, userId) {
userId = userId || Meteor.userId();
if (!userId) {
return false;
}
Expand All @@ -45,13 +49,17 @@ function _hasPermission(permissions, scope, strategy) {
}

permissions = [].concat(permissions);
return strategy(permissions, scope);
return strategy(permissions, scope, userId);
}

Template.registerHelper('hasPermission', function(permission, scope) {
return _hasPermission(permission, scope, atLeastOne);
});
Template.registerHelper('userHasAllPermission', function(userId, permission, scope) {
return _hasPermission(permission, scope, all, userId);
});

export const hasAllPermission = (permissions, scope) => _hasPermission(permissions, scope, all);
export const hasAtLeastOnePermission = (permissions, scope) => _hasPermission(permissions, scope, atLeastOne);
export const userHasAllPermission = (permissions, scope, userId) => _hasPermission(permissions, scope, all, userId);
export const hasPermission = hasAllPermission;
3 changes: 2 additions & 1 deletion app/authorization/client/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { hasAllPermission, hasAtLeastOnePermission, hasPermission } from './hasPermission';
import { hasAllPermission, hasAtLeastOnePermission, hasPermission, userHasAllPermission } from './hasPermission';
import { hasRole } from './hasRole';
import './usersNameChanged';
import './requiresPermission.html';
Expand All @@ -14,4 +14,5 @@ export {
hasAtLeastOnePermission,
hasRole,
hasPermission,
userHasAllPermission,
};
21 changes: 21 additions & 0 deletions app/authorization/server/functions/canSendMessage.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import { Meteor } from 'meteor/meteor';
import { TAPi18n } from 'meteor/tap:i18n';
import { Random } from 'meteor/random';

import { canAccessRoom } from './canAccessRoom';
import { hasPermission } from './hasPermission';
import { Notifications } from '../../../notifications';
import { Rooms, Subscriptions } from '../../../models';


export const canSendMessage = (rid, { uid, username }, extraData) => {
const room = Rooms.findOneById(rid);

Expand All @@ -15,6 +20,22 @@ export const canSendMessage = (rid, { uid, username }, extraData) => {
throw new Meteor.Error('room_is_blocked');
}

if (room.ro === true) {
if (!hasPermission(Meteor.userId(), 'post-readonly', room._id)) {
// Unless the user was manually unmuted
if (!(room.unmuted || []).includes(username)) {
Notifications.notifyUser(Meteor.userId(), 'message', {
_id: Random.id(),
rid: room._id,
ts: new Date(),
msg: TAPi18n.__('room_is_read_only'),
});

throw new Meteor.Error('You can\'t send messages because the room is readonly.');
}
}
}

if ((room.muted || []).includes(username)) {
throw new Meteor.Error('You_have_been_muted');
}
Expand Down
7 changes: 0 additions & 7 deletions app/lib/server/functions/addUserToDefaultChannels.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
import { Rooms, Subscriptions, Messages } from '../../../models';
import { hasPermission } from '../../../authorization';
import { callbacks } from '../../../callbacks';

export const addUserToDefaultChannels = function(user, silenced) {
callbacks.run('beforeJoinDefaultChannels', user);
const defaultRooms = Rooms.findByDefaultAndTypes(true, ['c', 'p'], { fields: { usernames: 0 } }).fetch();
defaultRooms.forEach((room) => {
// put user in default rooms
const muted = room.ro && !hasPermission(user._id, 'post-readonly');
if (muted) {
Rooms.muteUsernameByRoomId(room._id, user.username);
}

if (!Subscriptions.findOneByRoomIdAndUserId(room._id, user._id)) {
// Add a subscription to this user
Subscriptions.createWithRoomAndUser(room, user, {
Expand Down
6 changes: 0 additions & 6 deletions app/lib/server/functions/addUserToRoom.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Meteor } from 'meteor/meteor';

import { Rooms, Subscriptions, Messages } from '../../../models';
import { hasPermission } from '../../../authorization';
import { callbacks } from '../../../callbacks';

export const addUserToRoom = function(rid, user, inviter, silenced) {
Expand All @@ -22,11 +21,6 @@ export const addUserToRoom = function(rid, user, inviter, silenced) {
callbacks.run('beforeJoinRoom', user, room);
}

const muted = room.ro && !hasPermission(user._id, 'post-readonly');
if (muted) {
Rooms.muteUsernameByRoomId(rid, user.username);
}

Subscriptions.createWithRoomAndUser(room, user, {
ts: now,
open: true,
Expand Down
8 changes: 1 addition & 7 deletions app/lib/server/functions/createRoom.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import s from 'underscore.string';

import { Users, Rooms, Subscriptions } from '../../../models';
import { callbacks } from '../../../callbacks';
import { hasPermission, addUserRoles } from '../../../authorization';
import { addUserRoles } from '../../../authorization';
import { getValidRoomName } from '../../../utils';
import { Apps } from '../../../apps/server';

Expand Down Expand Up @@ -132,16 +132,10 @@ export const createRoom = function(type, name, owner, members, readOnly, extraDa

for (const username of members) {
const member = Users.findOneByUsername(username, { fields: { username: 1, 'settings.preferences': 1 } });
const isTheOwner = username === owner.username;
if (!member) {
continue;
}

// make all room members (Except the owner) muted by default, unless they have the post-readonly permission
if (readOnly === true && !hasPermission(member._id, 'post-readonly') && !isTheOwner) {
Rooms.muteUsernameByRoomId(room._id, username);
}

const extra = options.subscriptionExtra || {};

extra.open = true;
Expand Down
15 changes: 12 additions & 3 deletions app/lib/server/functions/notifications/mobile.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,17 @@ async function getBadgeCount(userId) {
return total;
}

function canSendMessageToRoom(room, username) {
return !(room.muted || []).includes(username);
function enableNotificationReplyButton(room, username) {
// Some users may have permission to send messages even on readonly rooms, but we're ok with false negatives here in exchange of better perfomance
if (room.ro === true) {
return false;
}

if (!room.muted) {
return true;
}

return !room.muted.includes(username);
}

export async function sendSinglePush({ room, message, userId, receiverUsername, senderUsername, senderName, notificationMessage }) {
Expand All @@ -61,7 +70,7 @@ export async function sendSinglePush({ room, message, userId, receiverUsername,
usersTo: {
userId,
},
category: canSendMessageToRoom(room, receiverUsername) ? CATEGORY_MESSAGE : CATEGORY_MESSAGE_NOREPLY,
category: enableNotificationReplyButton(room, receiverUsername) ? CATEGORY_MESSAGE : CATEGORY_MESSAGE_NOREPLY,
});
}

Expand Down
13 changes: 12 additions & 1 deletion app/lib/server/lib/processDirectEmail.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import moment from 'moment';
import { settings } from '../../../settings';
import { Rooms, Messages, Users, Subscriptions } from '../../../models';
import { metrics } from '../../../metrics';
import { hasPermission } from '../../../authorization';
import { sendMessage as _sendMessage } from '../functions';

export const processDirectEmail = function(email) {
Expand Down Expand Up @@ -87,10 +88,20 @@ export const processDirectEmail = function(email) {
}

if ((room.muted || []).includes(user.username)) {
// room is muted
// user is muted
return false;
}

// room is readonly
if (room.ro === true) {
if (!hasPermission(Meteor.userId(), 'post-readonly', room._id)) {
// Check if the user was manually unmuted
if (!(room.unmuted || []).includes(user.username)) {
return false;
}
}
}

if (message.alias == null && settings.get('Message_SetNameToAliasEnabled')) {
message.alias = user.name;
}
Expand Down
23 changes: 6 additions & 17 deletions app/models/server/models/Rooms.js
Original file line number Diff line number Diff line change
Expand Up @@ -544,25 +544,8 @@ export class Rooms extends Base {
const update = {
$set: {
ro: readOnly,
muted: [],
},
};
if (readOnly) {
Subscriptions.findByRoomIdWhenUsernameExists(_id, { fields: { 'u._id': 1, 'u.username': 1 } }).forEach(function({ u: user }) {
if (hasPermission(user._id, 'post-readonly')) {
return;
}
return update.$set.muted.push(user.username);
});
} else {
update.$unset = {
muted: '',
};
}

if (update.$set.muted.length === 0) {
delete update.$set.muted;
}

return this.update(query, update);
}
Expand Down Expand Up @@ -1177,6 +1160,9 @@ export class Rooms extends Base {
$addToSet: {
muted: username,
},
$pull: {
unmuted: username,
},
};

return this.update(query, update);
Expand All @@ -1189,6 +1175,9 @@ export class Rooms extends Base {
$pull: {
muted: username,
},
$addToSet: {
unmuted: username,
},
};

return this.update(query, update);
Expand Down
26 changes: 22 additions & 4 deletions app/reactions/client/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@ Template.room.events({
const user = Meteor.user();
const room = Rooms.findOne({ _id: rid });

if (Array.isArray(room.muted) && room.muted.indexOf(user.username) !== -1 && !room.reactWhenReadOnly) {
if (room.ro && !room.reactWhenReadOnly) {
if (!Array.isArray(room.unmuted) || room.unmuted.indexOf(user.username) === -1) {
return false;
}
}

if (Array.isArray(room.muted) && room.muted.indexOf(user.username) !== -1) {
return false;
}

Expand Down Expand Up @@ -68,11 +74,23 @@ Meteor.startup(function() {

if (!room) {
return false;
} if (Array.isArray(room.muted) && room.muted.indexOf(user.username) !== -1 && !room.reactWhenReadOnly) {
}

if (room.ro && !room.reactWhenReadOnly) {
if (!Array.isArray(room.unmuted) || room.unmuted.indexOf(user.username) === -1) {
return false;
}
}

if (Array.isArray(room.muted) && room.muted.indexOf(user.username) !== -1) {
return false;
} if (!Subscriptions.findOne({ rid: message.rid })) {
}

if (!Subscriptions.findOne({ rid: message.rid })) {
return false;
} if (message.private) {
}

if (message.private) {
return false;
}

Expand Down
20 changes: 16 additions & 4 deletions app/reactions/client/methods/setReaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,25 @@ Meteor.methods({
const message = Messages.findOne({ _id: messageId });
const room = Rooms.findOne({ _id: message.rid });

if (Array.isArray(room.muted) && room.muted.indexOf(user.username) !== -1 && !room.reactWhenReadOnly) {
if (room.ro && !room.reactWhenReadOnly) {
if (!Array.isArray(room.unmuted) || room.unmuted.indexOf(user.username) === -1) {
return false;
}
}

if (Array.isArray(room.muted) && room.muted.indexOf(user.username) !== -1) {
return false;
} if (!Subscriptions.findOne({ rid: message.rid })) {
}

if (!Subscriptions.findOne({ rid: message.rid })) {
return false;
} if (message.private) {
}

if (message.private) {
return false;
} if (!emoji.list[reaction] && EmojiCustom.findByNameOrAlias(reaction).count() === 0) {
}

if (!emoji.list[reaction] && EmojiCustom.findByNameOrAlias(reaction).count() === 0) {
return false;
}

Expand Down
8 changes: 7 additions & 1 deletion app/reactions/server/setReaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ export function setReaction(room, user, message, reaction, shouldReact) {
throw new Meteor.Error('error-not-allowed', 'Invalid emoji provided.', { method: 'setReaction' });
}

if (Array.isArray(room.muted) && room.muted.indexOf(user.username) !== -1 && !room.reactWhenReadOnly) {
if (room.ro && !room.reactWhenReadOnly) {
if (!Array.isArray(room.unmuted) || room.unmuted.indexOf(user.username) === -1) {
return false;
}
}

if (Array.isArray(room.muted) && room.muted.indexOf(user.username) !== -1) {
Notifications.notifyUser(Meteor.userId(), 'message', {
_id: Random.id(),
rid: room._id,
Expand Down
1 change: 1 addition & 0 deletions app/ui-admin/server/publications/adminRooms.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ Meteor.publish('adminRooms', function(filter, types, limit) {
usernames: 1,
usersCount: 1,
muted: 1,
unmuted: 1,
ro: 1,
default: 1,
topic: 1,
Expand Down
5 changes: 4 additions & 1 deletion app/ui-flextab/client/tabs/membersList.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ Template.membersList.helpers({
const roomUsers = Template.instance().users.get();
const room = ChatRoom.findOne(this.rid);
const roomMuted = (room != null ? room.muted : undefined) || [];
const roomUnmuted = (room != null ? room.unmuted : undefined) || [];
const userUtcOffset = Meteor.user() && Meteor.user().utcOffset;
let totalOnline = 0;
let users = roomUsers;
Expand Down Expand Up @@ -66,10 +67,12 @@ Template.membersList.helpers({
}
}

const muted = (room.ro && !roomUnmuted.includes(user.username)) || roomMuted.includes(user.username);

return {
user,
status: onlineUsers[user.username] != null ? onlineUsers[user.username].status : 'offline',
muted: Array.from(roomMuted).includes(user.username),
muted,
utcOffset,
};
});
Expand Down
Loading

0 comments on commit 8b81b17

Please sign in to comment.