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.

Sign up for GitHub

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

[NEW] Add permission view-broadcast-member-list #10753

Merged
merged 14 commits into from
May 21, 2018
Merged

[NEW] Add permission view-broadcast-member-list #10753

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
fc145d9
Add view-broadcast-member-list permission
cardoso May 13, 2018
082b21a
Add view-broadcast-member-list to i18n file
cardoso May 13, 2018
8c5a5d0
Fix not looking up view-broadcast-member-list permission per room
cardoso May 13, 2018
bdf4410
REST API: Don't allow users without permission to fetch broadcast roo…
cardoso May 13, 2018
f0b7922
Remove trailing space
cardoso May 13, 2018
7a235a2
Merge branch 'develop' of https://github.com/RocketChat/Rocket.Chat i…
cardoso May 13, 2018
885a208
Merge branch 'develop' into imp/broadcast_permissions
cardoso May 15, 2018
78b30ab
DDP: getUsersOfRoom disallow users without permission to fetch broadc…
cardoso May 15, 2018
4bbd7a6
Merge branch 'imp/broadcast_permissions' of https://github.com/cardos…
cardoso May 15, 2018
f36aa43
Merge branch 'develop' into imp/broadcast_permissions
cardoso May 15, 2018
a5d30b8
Fix code style
cardoso May 15, 2018
d74cf48
Merge branch 'develop' into imp/broadcast_permissions
cardoso May 16, 2018
47410cb
Merge branch 'develop' into imp/broadcast_permissions
cardoso May 18, 2018
9d01a6b
Merge branch 'develop' into imp/broadcast_permissions
cardoso May 21, 2018
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
4 changes: 4 additions & 0 deletions packages/rocketchat-api/server/v1/channels.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,10 @@ RocketChat.API.v1.addRoute('channels.members', { authRequired: true }, {
returnUsernames: true
});

if (findResult.broadcast && !RocketChat.authz.hasPermission(this.userId, 'view-broadcast-member-list')) {
return RocketChat.API.v1.unauthorized();
}

const { offset, count } = this.getPaginationItems();
const { sort } = this.parseJsonQuery();

Expand Down
5 changes: 5 additions & 0 deletions packages/rocketchat-api/server/v1/groups.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,11 @@ RocketChat.API.v1.addRoute('groups.listAll', { authRequired: true }, {
RocketChat.API.v1.addRoute('groups.members', { authRequired: true }, {
get() {
const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId });

if (findResult._room.broadcast && !RocketChat.authz.hasPermission(this.userId, 'view-broadcast-member-list')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we use a "constant" on the name of the permission? Looks like we're repeating it many times.

Copy link
Contributor Author

@cardoso cardoso May 14, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd have to do that for all other permissions too.

But there wouldn't be much benefit with this being Javascript anyway. ¯\_(ツ)_/¯

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main benefit would be not repeating yourself :-)

return RocketChat.API.v1.unauthorized();
}

const { offset, count } = this.getPaginationItems();
const { sort } = this.parseJsonQuery();

Expand Down
3 changes: 2 additions & 1 deletion packages/rocketchat-authorization/server/startup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ Meteor.startup(function() {
{ _id: 'view-statistics', roles : ['admin'] },
{ _id: 'view-user-administration', roles : ['admin'] },
{ _id: 'preview-c-room', roles : ['admin', 'user', 'anonymous'] },
{ _id: 'view-outside-room', roles : ['admin', 'owner', 'moderator', 'user'] }
{ _id: 'view-outside-room', roles : ['admin', 'owner', 'moderator', 'user'] },
{ _id: 'view-broadcast-member-list', roles : ['admin', 'owner', 'moderator'] }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also repeating many times the roles here and the name of the permissions.

];

for (const permission of permissions) {
Expand Down
1 change: 1 addition & 0 deletions packages/rocketchat-i18n/i18n/en.i18n.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2322,6 +2322,7 @@
"Video_message": "Video message",
"Videocall_declined": "Video Call Declined.",
"Videocall_enabled": "Video Call Enabled",
"view-broadcast-member-list": "View Members List in Broadcast Room",
"view-c-room": "View Public Channel",
"view-c-room_description": "Permission to view public channels",
"view-d-room": "View Direct Messages",
Expand Down
2 changes: 1 addition & 1 deletion packages/rocketchat-lib/client/defaultTabBars.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ RocketChat.TabBar.addButton({
return true;
}

return RocketChat.authz.hasRole(Meteor.userId(), ['admin', 'moderator', 'owner'], rid);
return RocketChat.authz.hasAllPermission('view-broadcast-member-list', rid);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

}
});

Expand Down
4 changes: 4 additions & 0 deletions server/methods/getUsersOfRoom.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ Meteor.methods({
throw new Meteor.Error('error-invalid-room', 'Invalid room', { method: 'getUsersOfRoom' });
}

if (room.broadcast && !RocketChat.authz.hasPermission(Meteor.userId(), 'view-broadcast-member-list', roomId)) {
throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'getUsersOfRoom' });
}

const filter = (record) => {
if (!record._user) {
console.log('Subscription without user', record._id);
Expand Down