[FIX] Quoted messages from message links when user has no permission #20815
Conversation
…annels in which they are in
sampaiodiego
changed the title
|
|
||
| // validates if user can see the message | ||
| // user has to belong to the room the message was first wrote in | ||
| const canAccessRoom = Meteor.call('canAccessRoom', jumpToMessage.rid, Meteor.userId()); |
sampaiodiego
Feb 19, 2021
Member
please try to avoid using Meteor methods, specially on server code. always check if there is a service that does the same and if you can't used async functions (like here), look for alternative functions or use Promise.await.
to check if a user has permission to access a room you can use canAccessRoom
please try to avoid using Meteor methods, specially on server code. always check if there is a service that does the same and if you can't used async functions (like here), look for alternative functions or use Promise.await.
to check if a user has permission to access a room you can use canAccessRoom
KevLehman
Feb 19, 2021
Author
Contributor
Done!
Done!
Co-authored-by: Diego Sampaio <chinello@gmail.com>
|
|
||
| // validates if user can see the message | ||
| // user has to belong to the room the message was first wrote in | ||
| const canAccessRoomForUser = canAccessRoom({ _id: jumpToMessage.rid }, Meteor.user()); |
sampaiodiego
Feb 19, 2021
Member
I think you'll need to send a complete Room object to canAccessRoom..
I'd also recommend to store Meteor.user() in a variable instead of calling it inside the loop.
I think you'll need to send a complete Room object to canAccessRoom..
I'd also recommend to store Meteor.user() in a variable instead of calling it inside the loop.
KevLehman
Feb 19, 2021
Author
Contributor
Regarding this one, I noticed that in other places they were retrieving Rooms.findById(...., { _id: 1 }). That will exclude other fields and returning just the ID as an object. This mimics the same behavior to avoid calling the database n times. (And it looks like its working haha)
Regarding the Meteor.user I didn't know it was an expensive operation, I'll move it to the top.
Regarding this one, I noticed that in other places they were retrieving Rooms.findById(...., { _id: 1 }). That will exclude other fields and returning just the ID as an object. This mimics the same behavior to avoid calling the database n times. (And it looks like its working haha)
Regarding the Meteor.user I didn't know it was an expensive operation, I'll move it to the top.
| // validates if user can see the message | ||
| // user has to belong to the room the message was first wrote in | ||
| const canAccessRoomForUser = canAccessRoom({ _id: jumpToMessage.rid }, Meteor.user()); | ||
| if (jumpToMessage && canAccessRoomForUser) { |
sampaiodiego
Feb 19, 2021
Member
why not having a shortcut as others here? 😬
Suggested change
if (jumpToMessage && canAccessRoomForUser) {
if (!canAccessRoomForUser) {
return;
}
why not having a shortcut as others here?
| if (jumpToMessage && canAccessRoomForUser) { | |
| if (!canAccessRoomForUser) { | |
| return; | |
| } |
KevLehman
Feb 19, 2021
Author
Contributor
Ah, yes 🤔 since we are checking for jumpToMessage some lines above, it makes sense to use the shortcircuit here. Nice catch!
Ah, yes jumpToMessage some lines above, it makes sense to use the shortcircuit here. Nice catch!
sampaiodiego
changed the title
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments