Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Several problems with read-only rooms and muted users #11311

Merged
merged 27 commits into from
May 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
9e41ef9
Fixed post-readonly permission
pierre-lehnen-rc Jul 2, 2018
65fa8ba
Merge branch 'develop' into fix.post-readonly-permission
pierre-lehnen-rc Aug 20, 2018
8a4fadc
Merge branch 'develop' into fix.post-readonly-permission
pierre-lehnen-rc Aug 20, 2018
0514dee
Merge branch 'fix.post-readonly-permission' of github.com:RocketChat/…
pierre-lehnen-rc Aug 20, 2018
95755ef
Simplified code
pierre-lehnen-rc Aug 20, 2018
493cf50
Merge branch 'develop' into fix.post-readonly-permission
pierre-lehnen-rc Aug 20, 2018
7c9e0f3
Merge branch 'develop' into fix.post-readonly-permission
pierre-lehnen-rc Oct 9, 2018
04ddbd4
Use the room muted list only for manually muted users
pierre-lehnen-rc Oct 11, 2018
6f9504a
Merge branch 'develop' into fix.post-readonly-permission
pierre-lehnen-rc Oct 15, 2018
18c93e5
Added migration to remove users from muted list on old readonly rooms
pierre-lehnen-rc Oct 15, 2018
aa0c6fb
Merge branch 'develop' into fix.post-readonly-permission
pierre-lehnen-rc Mar 12, 2019
2f89654
Merge remote-tracking branch 'origin/develop' into fix.post-readonly-…
pierre-lehnen-rc Mar 13, 2019
4aa9385
Added scope to permission checks
pierre-lehnen-rc Mar 13, 2019
e3afd8d
Stop overriding permissions for room owners
pierre-lehnen-rc Mar 13, 2019
ab86f18
List of unmuted users on a readonly room
pierre-lehnen-rc Mar 13, 2019
dcf7a38
Fixed permission checking
pierre-lehnen-rc Mar 13, 2019
87ad368
Adding option to unmute user
pierre-lehnen-rc Mar 13, 2019
cfe07ba
Merge branch 'develop' into fix.post-readonly-permission
pierre-lehnen-rc Mar 18, 2019
9babc83
Mute and Unmute users on readonly rooms
pierre-lehnen-rc Mar 20, 2019
3290f54
Removed default param values
pierre-lehnen-rc May 7, 2019
eecfc1a
Merge branch 'develop' into fix.post-readonly-permission
pierre-lehnen-rc May 7, 2019
7cd8c6f
Fixed import paths
pierre-lehnen-rc May 7, 2019
18fb2dc
Merge branch 'develop' into fix.post-readonly-permission
Hudell May 7, 2019
0897479
Merge branch 'develop' into fix.post-readonly-permission
pierre-lehnen-rc May 14, 2019
2e5a5a6
Merge branch 'develop' into fix.post-readonly-permission
pierre-lehnen-rc May 15, 2019
ee3602b
Fixed file imported wrong
pierre-lehnen-rc May 15, 2019
4918d63
Merge branch 'develop' into fix.post-readonly-permission
rodrigok May 16, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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