fix(messenger_communities): block messages and reactions to token gated or spectated communities #4064
Conversation
|
Hey @shinnok, and thank you so much for making your first pull request in status-go! ❤️ Please help us make your experience better by filling out this brief questionnaire https://goo.gl/forms/uWqNcVpVz7OIopXg2 |
Jenkins BuildsClick to see older builds (96)
|
shinnok
changed the title
shinnok
changed the title
There was a problem hiding this comment.
The line in CanPost seems like the right culprit for the Sender side.
We should also do a similar check on the receiver side.
The entry point for that is in messenger_handlers.go > dispatchToHandler.
That's where we dispatch the received message to the right handler.
In this case, it would be handleChatMessageProtobuf.
Pro tip: using fmt.Println("...") is a good way to add logs to status-go that will show up in the console when running Status Desktop.
But even better is using the Go tests. You'll find a lot of useful tests in community_test.go. For example TestCanPost. TestValidateRequestToJoin might be a good one to validate that messages only work for those joined, but maybe it's better to create a new test case.
shinnok
changed the title
|
I can't get the Not sure how critical it is to get the |
There was a problem hiding this comment.
Code looks good, though there is a small cleanup needed.
As we discussed in our 1-1, the Pin message test can be added in another PR, because it seems like the Setting to let users pin in a community is not being saved correctly on the receiver side, so it's worth another issue.
I believe the root cause is different: #4064 (comment) |
Yeah I know. When I was in the 1-1 with him, we did add the Or maybe we do not publish community settings with |
The |
shinnok
force-pushed
the
fix/11915/send-message-to-gated-community
branch
from
39789d9 to
dace1b2
Compare
|
I've created issue #4138 to further investigate the channel message pin issue and forked the new test method The current squashed commit contains the sufficient changes to block messages, reactions and pins to channels for which the user is not a member, both for him and other members. Please review again. |
shinnok
force-pushed
the
fix/11915/send-message-to-gated-community
branch
from
dace1b2 to
372bc11
Compare
jrainville
force-pushed
the
fix/11915/send-message-to-gated-community
branch
from
fd675b8 to
67badfc
Compare
shinnok
force-pushed
the
fix/11915/send-message-to-gated-community
branch
from
3d5ab9c to
0453246
Compare
shinnok
force-pushed
the
fix/11915/send-message-to-gated-community
branch
from
48ddf61 to
5eb9234
Compare
shinnok
force-pushed
the
fix/11915/send-message-to-gated-community
branch
from
5f30482 to
b69d5b0
Compare
|
Attaching a video showing the changes in effect, the top left Status instance has adjustments in this PR in place, the bottom right doesn't, but instead has the QML UI checks disabled for sending, reacting and pinning. This effectively verifies that the receiving side effectively blocks non-member actions, however, once applied, this patch also denies non-member actions on the sender's side for efficiency. Testing this exact same workflow without this patch on either side, but with the UI checks disabled, will allow a non-member to send, react and pin before being approved in a on-request community or before spectating for no-request community. output.mp4
|
Which specifies that if a user is not a community member & a chat member, he can't post, react or pin messages in that chat. Notes: - also fix&cleanup associated failing tests. - refactor Community.CanPost() to reflect the new requirement. - grant code is not fully implemented and is to be removed later. Fixes #11915
shinnok
force-pushed
the
fix/11915/send-message-to-gated-community
branch
from
55ba45e to
4fe5a2c
Compare
Right now, if the QML UI control checks are disabled, the user can send messages to spectated, non-joined and, possibly, token-gated communities with invalid tokens?
Right now the PR is in draft stage with possible code culprits.
Closes status-im/status-desktop#11915